While the initial one-time login link is correct and works, any further attempt to reset the password fails, because the link looks like this:

http://d5.o1.chicago.aegir.cc/user/reset/%252F1304465335/900801b2b9931d18b3d3a656f4cb1ae3

The correct, working URL should look like:

http://d5.o1.chicago.aegir.cc/user/reset/1/1304465256/de427d7814d2dd903d4650e81e893fcb

This was reproduced many times on Pressflow 5.23.50 platforms.

CommentFileSizeAuthor
#8 1146238_reroll_reroll.patch823 bytesmig5
#8 Selection_002.png39.89 KBmig5
#6 1146238-reroll.patch686 bytesmig5
#1 1146238.patch748 bytesmig5

Comments

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Needs review
StatusFileSize
new 1146238.patch748 bytes

We were calling user_load() in a way that, while works in D6/D7 due to them checking for a single integer argument 'just in case', does not work in Drupal 5 as it expects a proper array as an arg.

I've fixed this and tested it resetting a password in a D5, D6 and D7 site, but would appreciate someone else confirming it works (Steven?)

Cheers

Mig

Anonymous’s picture

Anonymous (not verified) commented

Aaaaand, I just accidentally pushed that commit when pushing a fix for a separate issue :( oops!!! I'm a bit rusty..

I'm pretty confident in the fix though, but still would appreciate some testing..

Sorry about that..

steven jones’s picture

steven jones commented
Version: 6.x-0.4-alpha3 » 6.x-1.x-dev
Status: Needs review » Needs work

Hmm...I'm fairly sure this won't work on D7, looking at the arguments to user_load, I think we might be better off using a call to drush_drupal_major_version and call the function appropriately.

Anonymous’s picture

Anonymous (not verified) commented

Strange, it worked for me in D7 somehow. I'll test again

Anonymous’s picture

Anonymous (not verified) commented

Ahh missed this message:

array_flip(): Can only flip STRING and INTEGER values! entity.inc:178

Login url: http://test1.drupal7.dev/user/reset/1/1321303510/wt-cXE_XieclA_-yBA3iTrHx-2v5jy7qGk2yMqXQ3OM

Somehow the reset still works, but yes, we shouldn't have this error.

I'll revert this patch that I accidentally committed (I'm back, can you tell! :( )

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review
StatusFileSize
new 1146238-reroll.patch686 bytes

reroll, please test!

steven jones’s picture

steven jones commented
Status: Needs review » Needs work

Okay, going to be annoying here, but we should actually test that we loaded an account, we'd have spotted the D5 bug much sooner if we had.

Something like:
<?php
if (empty($account)) {
// abort the drush command.
return drush_set_error();
}

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review
StatusFileSize
new Selection_002.png39.89 KB
new 1146238_reroll_reroll.patch823 bytes

OK, new patch attached..

We seem to use drush_set_error() about 10 different ways, I went with your example from the tests/ dir.

Screenshot also attached of a forced error (deleted uid 1 from the test site). But would appreciate a full test across drupal 5, 6 and 7 (perhaps we should reset a login in our provision tests on each platform), as I no longer trust myself here..

steven jones’s picture

steven jones commented
Status: Needs review » Fixed

Tested, cleaned up and pushed!

Thanks very much.

Status: Fixed » Closed (fixed)

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