Currently if a user tries to login and their credentials aren't right they get a prompt to reset their password. They click the link and get to user/password and the form has forgotten their username.

It would be nice if we didn't require them to enter that information again.

This feature could also be useful in other places, so I made it a url parameter instead of a cookie. I don't see any way this could be abused.

Files: 
CommentFileSizeAuthor
#45 1952196_easier_password_recovery_44.patch2.55 KBgreggles
PASSED: [[SimpleTest]]: [MySQL] 40,344 pass(es).
[ View ]
#39 1952196_easier_password_recovery_d7-no-not-test.patch1.25 KBgreggles
PASSED: [[SimpleTest]]: [MySQL] 40,409 pass(es).
[ View ]
#36 1952196_easier_password_recovery_36.patch2.46 KBgreggles
PASSED: [[SimpleTest]]: [MySQL] 55,257 pass(es).
[ View ]
#32 1952196_easier_password_recovery_32.patch2.46 KBgreggles
FAILED: [[SimpleTest]]: [MySQL] 55,028 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#30 1952196_easier_password_recovery_30.patch2.46 KBgreggles
PASSED: [[SimpleTest]]: [MySQL] 55,002 pass(es).
[ View ]
#29 1952196_easier_password_recovery_26.patch2.52 KBgreggles
PASSED: [[SimpleTest]]: [MySQL] 54,820 pass(es).
[ View ]
#28 1952196_easier_password_recovery_26.patch2.52 KBgreggles
PASSED: [[SimpleTest]]: [MySQL] 54,799 pass(es).
[ View ]
#19 1952196_easier_password_recovery_19.patch2.47 KBgreggles
PASSED: [[SimpleTest]]: [MySQL] 54,744 pass(es).
[ View ]
#18 userpassword-reset-namefill-1952196-17.patch4.89 KBsandhya.m
PASSED: [[SimpleTest]]: [MySQL] 54,471 pass(es).
[ View ]
#16 username-autofill-resetpw-1952196-16.patch2.46 KBsandhya.m
FAILED: [[SimpleTest]]: [MySQL] 54,454 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
#13 username-autofill-resetpw-1952196-13.patch1.13 KBsandhya.m
FAILED: [[SimpleTest]]: [MySQL] 54,492 pass(es), 5 fail(s), and 0 exception(s).
[ View ]
#11 username-autofill-resetpw-1952196-11.patch1.11 KBsandhya.m
FAILED: [[SimpleTest]]: [MySQL] 54,468 pass(es), 8 fail(s), and 0 exception(s).
[ View ]
#10 1952196-username-autofill-resetpw.patch1.11 KBsandhya.m
FAILED: [[SimpleTest]]: [MySQL] 54,311 pass(es), 8 fail(s), and 0 exception(s).
[ View ]
#1 1952196_easier_password_recovery.patch1.32 KBgreggles
PASSED: [[SimpleTest]]: [MySQL] 53,293 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new1.32 KB
PASSED: [[SimpleTest]]: [MySQL] 53,293 pass(es).
[ View ]

Status:Needs review» Needs work

The last submitted patch, 1952196_easier_password_recovery.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Reviewed & tested by the community

Patch applied, works good.
Better add Usability tag to it, and talk to guys at #drupal-usability on IRC, to get your patch applied.
Thanks.

Issue tags:+Usability

Sounds good, lets get this in!

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs tests

I think we can add a test for this?

Sure, a test for displaying the message or a test that the default from the url works or both?

For the default value. Thanks! :)

Assigned:Unassigned» sandhya.m
Status:Needs work» Needs review
StatusFileSize
new1.11 KB
FAILED: [[SimpleTest]]: [MySQL] 54,311 pass(es), 8 fail(s), and 0 exception(s).
[ View ]

Status:Needs work» Needs review
StatusFileSize
new1.11 KB
FAILED: [[SimpleTest]]: [MySQL] 54,468 pass(es), 8 fail(s), and 0 exception(s).
[ View ]

Changed the patch file name.

Status:Needs review» Needs work

The last submitted patch, username-autofill-resetpw-1952196-11.patch, failed testing.

Status:Needs review» Needs work
StatusFileSize
new1.13 KB
FAILED: [[SimpleTest]]: [MySQL] 54,492 pass(es), 5 fail(s), and 0 exception(s).
[ View ]

Changed patch attached.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, username-autofill-resetpw-1952196-13.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.46 KB
FAILED: [[SimpleTest]]: [MySQL] 54,454 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, username-autofill-resetpw-1952196-16.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.89 KB
PASSED: [[SimpleTest]]: [MySQL] 54,471 pass(es).
[ View ]

StatusFileSize
new2.47 KB
PASSED: [[SimpleTest]]: [MySQL] 54,744 pass(es).
[ View ]

#18 looks pretty solid to me. It adds a test for the "Sorry, unrecognized username or password" which we weren't testing before. However, it wasn't testing that the name gets added into the link and I think as long as we're going to test that message we should ensure the name is in there. So, attached is an updated patch based on #18 that uses assertRaw to look for the proper link.

@xjm - care to review/rtbc?

Oh, I also removed an extra * on the closing comment tag of the docblock of the test and tried to make the comment in the docblock of the test a little more descriptive.

One more note: in case you are curious why sandhya.m's file is about twice as large as mine - it's because it's utf16 (thanks to heine for helping me understand that).

Status:Needs review» Reviewed & tested by the community

Hey, why not just do this again...

Status:Reviewed & tested by the community» Needs work

Since this was RTBCed by the security team lead, I assume this is not a problem, but:

+++ b/core/modules/user/user.pages.incundefined
@@ -38,6 +38,9 @@ function user_pass() {
+  else {
+    $form['name']['#default_value'] = isset($_GET['name']) ? $_GET['name'] : '';
+  }

Is that cool? Don't we need check_plain() at least?

Also, (in D8 at least, if not also D7) referencing $GET directly is a no-no. I'm not sure what the Symfonic equivalent of it is anymore... maybe Drupal::request()->getSomething?

Oh, that said, this is an awesome improvement! :)

Is that cool? Don't we need check_plain() at least?

I don't think so, but happy to be corrected. I make mistakes. There are lots of places where we use user supplied text into a textfield's default value without any sanitizing e.g. editing of node titles, text fields etc.

Berdir said the right thing to use is "Drupal::request() if it's not within a class/service that can get it injected."

I don't know how to judge if it should get injected, so I guess it's good this is up to sandhya.m.

Issue tags:-Needs tests

That's a nice improvement!

+++ b/core/modules/user/lib/Drupal/user/Tests/UserPasswordResetTest.phpundefined
@@ -108,4 +108,21 @@ public function getResetURL() {
+    $this->assertFieldByName('name',$edit['name'],'User name found.');

Missing spaces after the ","

+++ b/core/modules/user/lib/Drupal/user/Tests/UserPasswordResetTest.phpundefined
@@ -108,4 +108,21 @@ public function getResetURL() {
+    $edit = array(
+      'name' => $this->randomName(),
+      'pass' => $this->randomName(),
+      );

indent is off

+++ b/core/modules/user/user.pages.incundefined
@@ -38,6 +38,9 @@ function user_pass() {
+    $form['name']['#default_value'] = isset($_GET['name']) ? $_GET['name'] : '';

should be
$form['name']['#default_value'] = Drupal::request()->query->get('name', '');

No need for the ternary:) you can pass the default value..if NULL is appropriate here, you can omit it

Status:Needs work» Needs review
Issue tags:+Needs tests
StatusFileSize
new2.52 KB
PASSED: [[SimpleTest]]: [MySQL] 54,799 pass(es).
[ View ]

After getting 2 incorrect answers in #drupal-contribute a shining angel came to my rescue and gave me the advice I needed. Thank you, shining angel, for the pointer to the get function in http://api.symfony.com/2.0/Symfony/Component/HttpFoundation/Request.html

StatusFileSize
new2.52 KB
PASSED: [[SimpleTest]]: [MySQL] 54,820 pass(es).
[ View ]

Thanks dawehener and ParisLiakos for your reviews!

Incorporated into this patch.

StatusFileSize
new2.46 KB
PASSED: [[SimpleTest]]: [MySQL] 55,002 pass(es).
[ View ]

+++ b/core/modules/user/user.pages.incundefined
@@ -38,6 +38,9 @@ function user_pass() {
+    $form['name']['#default_value'] = Drupal::Request()->get('name', '');

Doesnt request()->query->get() work?
Right from the link you posted:
http://api.symfony.com/2.0/Symfony/Component/HttpFoundation/Request.html...

Order of precedence: GET, PATH, POST, COOKIE
Avoid using this method in controllers:
* slow
* prefer to get from a "named" source

The query property holds $_GET:)

StatusFileSize
new2.46 KB
FAILED: [[SimpleTest]]: [MySQL] 55,028 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Indeed it does work. Thanks for the advice ParisLiakos.

Status:Needs review» Reviewed & tested by the community

looks good to go:)

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 1952196_easier_password_recovery_32.patch, failed testing.

+++ b/core/modules/user/user.pages.incundefined
@@ -38,6 +38,9 @@ function user_pass() {
+  else {
+    $form['name']['#default_value'] = Drupal::Request()->query->get('foo');
+  }

Erm. :) I think 'name', not 'foo' :)

Status:Needs work» Needs review
StatusFileSize
new2.46 KB
PASSED: [[SimpleTest]]: [MySQL] 55,257 pass(es).
[ View ]

Zoinks.

I was testing to see if a non existant value would throw a notice and forgot to change it back. Whoops.

Edit: Oh, and tests passed locally this time so it should be all good.

Status:Needs review» Reviewed & tested by the community

Yay tests!

D7?

Issue tags:-Needs tests+needs backport to D7
StatusFileSize
new1.25 KB
PASSED: [[SimpleTest]]: [MySQL] 40,409 pass(es).
[ View ]

Yes, I think a backport makes sense though it's a bit early. Here's a patch for d7 from when I first worked on this issue. It will need the tests merged in, but that should be easy.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 1952196_easier_password_recovery_d7-no-not-test.patch, failed testing.

Status:Needs work» Reviewed & tested by the community

I swear I named that right...

'n' and 'd' are far away from each other. It must have been the keyboard demon ;)

Thanx Greg!

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Active

Committed b8abe39 and pushed to 8.x. Thanks!

Status:Active» Needs review

StatusFileSize
new2.55 KB
PASSED: [[SimpleTest]]: [MySQL] 40,344 pass(es).
[ View ]

And a straight D7 port with passing tests.

simplytest.me manual tested and it is green ;)

Tested on simplytest.me its green

Status:Needs review» Reviewed & tested by the community

Issue tags:-needs backport to D7

Backport is done, so I think this tag should be removed.

Status:Reviewed & tested by the community» Fixed
Issue tags:+7.23 release notes

Thanx! ;)

Automatically closed -- issue fixed for 2 weeks with no activity.