Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ticket/12183] Update user_newpasswd column in users table for passwords manager #2025

Merged
merged 8 commits into from Feb 21, 2014

Conversation

@marc1706
Copy link
Member

marc1706 commented Feb 10, 2014

The user_newpasswd column wasn't update in the passwords manager migrations file. This column is needed for reseting users' passwords. As it will contain a password hash, it has to be updated to the new size of the user_password column.

Ticket: http://tracker.phpbb.com/browse/PHPBB3-12183

marc1706 added 4 commits Feb 10, 2014
PHPBB3-12183
…_test

The forgot_password_test disables the password reset functionalty but doesn't
enable it again afterwards.

PHPBB3-12183
'email' => 'nobody@example.com',
));
$crawler = self::submit($form);
$this->assertContainsLang('PASSWORD_UPDATED', $crawler->text());

This comment has been minimized.

Copy link
@bantu

bantu Feb 11, 2014

Member

Shouldn't this test also:

  • receive the new password and activation key
  • activate the new password
  • make sure that the old password can not be used to login
  • make sure that the new password can be used to login
    ?

This comment has been minimized.

Copy link
@marc1706

marc1706 Feb 11, 2014

Author Member

Not sure how we'd be able to test receiving the new password and activating it. Suggestions?

This comment has been minimized.

Copy link
@bantu

bantu Feb 15, 2014

Member

SQL query against user table.

This comment has been minimized.

Copy link
@marc1706

marc1706 Feb 17, 2014

Author Member

Added the tests that were suggested above.

@@ -1173,7 +1173,7 @@
'user_jabber' => array('VCHAR_UNI', ''),
'user_website' => array('VCHAR_UNI:200', ''),
'user_actkey' => array('VCHAR:32', ''),
'user_newpasswd' => array('VCHAR_UNI:40', ''),
'user_newpasswd' => array('VCHAR_UNI', ''),

This comment has been minimized.

Copy link
@nickvergessen

nickvergessen Feb 11, 2014

Contributor

move just below user password so they are next to each others and similar mistakes cna be spotted easier in future?

This comment has been minimized.

Copy link
@bantu

bantu Feb 11, 2014

Member

user_actkey belongs to that feature as well

$crawler = self::submit($form);
$this->assertContainsLang('PASSWORD_UPDATED', $crawler->text());

// Make sure we know the password

This comment has been minimized.

Copy link
@EXreaction

EXreaction Feb 19, 2014

Contributor

You should check that the new password was actually setup here before setting it to an arbitrary value for later tests.

{
$this->get_user_data();
$this->assertNotNull($this->user_data['user_actkey']);
$this->assertNotNull($this->user_data['user_newpasswd']);

This comment has been minimized.

Copy link
@EXreaction

EXreaction Feb 19, 2014

Contributor

These assertions should be made before you set the new password to some arbitrary value

$this->get_user_data();
$this->assertNotNull($this->user_data['user_actkey']);
$this->assertNotNull($this->user_data['user_newpasswd']);
$this->login('reset-password-test-user');

This comment has been minimized.

Copy link
@EXreaction

EXreaction Feb 19, 2014

Contributor

This login isn't required. Next task is activating a password, which shouldn't require being logged in first (you should actually make sure the user is logged out).

EXreaction added a commit that referenced this pull request Feb 21, 2014
[ticket/12183] Update user_newpasswd column in users table for passwords manager
@EXreaction EXreaction merged commit 2df2032 into phpbb:develop Feb 21, 2014
1 check passed
1 check passed
default The Travis CI build passed
Details
@marc1706 marc1706 deleted the marc1706:ticket/12183 branch Jan 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.