[MediaWiki-commits] [Gerrit] mediawiki...CentralAuth[wmf/1.30.0-wmf.7]: Fix handling of password hash upgrade on login

2017-07-07 Thread jenkins-bot (Code Review) 
jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/363891 )

Change subject: Fix handling of password hash upgrade on login
..


Fix handling of password hash upgrade on login

Ie0fbf797 caused the user to be immediately logged out if their password
hash was upgraded on login, by moving the auth token reset to post-send
where it can't update the user's session cookie.

This is fixed by not bothering with the auth token reset in that code
path.

I5e22a184 caused the password hash upgrade to fail, accidentally, by
moving most processing to a non-master CentralAuthUser instance so
->setPassword() was being called on a master instance that hadn't yet
had its data loaded. Thus, the call to ->getId() which did trigger that
loading was overwriting ->mPassword.

This is fixed by making sure ->setPassword() loads the data before it
starts setting fields.

Bug: T169261
Change-Id: I7f01c02ed7c369e71f748ac9142edd1709ea6514
(cherry picked from commit 80eeb133cc707426c9fcca0cc1e6183d50f29211)
---
M includes/CentralAuthUser.php
1 file changed, 10 insertions(+), 1 deletion(-)

Approvals:
  Legoktm: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/CentralAuthUser.php b/includes/CentralAuthUser.php
index a85c1dd..903138d 100644
--- a/includes/CentralAuthUser.php
+++ b/includes/CentralAuthUser.php
@@ -1903,7 +1903,12 @@
 
$centralUser = 
CentralAuthUser::newMasterInstanceFromId( $this->getId() );
if ( $centralUser ) {
-   $centralUser->setPassword( 
$password );
+   // Don't bother resetting the 
auth token for a hash
+   // upgrade. It's not really a 
password *change*, and
+   // since this is being done 
post-send it'll cause the
+   // user to be logged out when 
they just tried to log in
+   // since it can't update the 
just-sent session cookies.
+   $centralUser->setPassword( 
$password, false );
$centralUser->saveSettings();
}
} );
@@ -2537,6 +2542,10 @@
 */
function setPassword( $password, $resetAuthToken = true ) {
$this->checkWriteMode();
+
+   // Make sure state is loaded before updating ->mPassword
+   $this->loadState();
+
list( $salt, $hash ) = $this->saltedPassword( $password );
 
$this->mPassword = $hash;

-- 
To view, visit https://gerrit.wikimedia.org/r/363891
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I7f01c02ed7c369e71f748ac9142edd1709ea6514
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/CentralAuth
Gerrit-Branch: wmf/1.30.0-wmf.7
Gerrit-Owner: Legoktm 
Gerrit-Reviewer: Anomie 
Gerrit-Reviewer: Legoktm 
Gerrit-Reviewer: jenkins-bot <>

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

[MediaWiki-commits] [Gerrit] mediawiki...CentralAuth[wmf/1.30.0-wmf.7]: Fix handling of password hash upgrade on login

2017-07-07 Thread Legoktm (Code Review) 
Legoktm has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/363891 )

Change subject: Fix handling of password hash upgrade on login
..

Fix handling of password hash upgrade on login

Ie0fbf797 caused the user to be immediately logged out if their password
hash was upgraded on login, by moving the auth token reset to post-send
where it can't update the user's session cookie.

This is fixed by not bothering with the auth token reset in that code
path.

I5e22a184 caused the password hash upgrade to fail, accidentally, by
moving most processing to a non-master CentralAuthUser instance so
->setPassword() was being called on a master instance that hadn't yet
had its data loaded. Thus, the call to ->getId() which did trigger that
loading was overwriting ->mPassword.

This is fixed by making sure ->setPassword() loads the data before it
starts setting fields.

Bug: T169261
Change-Id: I7f01c02ed7c369e71f748ac9142edd1709ea6514
(cherry picked from commit 80eeb133cc707426c9fcca0cc1e6183d50f29211)
---
M includes/CentralAuthUser.php
1 file changed, 10 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CentralAuth 
refs/changes/91/363891/1

diff --git a/includes/CentralAuthUser.php b/includes/CentralAuthUser.php
index a85c1dd..903138d 100644
--- a/includes/CentralAuthUser.php
+++ b/includes/CentralAuthUser.php
@@ -1903,7 +1903,12 @@
 
$centralUser = 
CentralAuthUser::newMasterInstanceFromId( $this->getId() );
if ( $centralUser ) {
-   $centralUser->setPassword( 
$password );
+   // Don't bother resetting the 
auth token for a hash
+   // upgrade. It's not really a 
password *change*, and
+   // since this is being done 
post-send it'll cause the
+   // user to be logged out when 
they just tried to log in
+   // since it can't update the 
just-sent session cookies.
+   $centralUser->setPassword( 
$password, false );
$centralUser->saveSettings();
}
} );
@@ -2537,6 +2542,10 @@
 */
function setPassword( $password, $resetAuthToken = true ) {
$this->checkWriteMode();
+
+   // Make sure state is loaded before updating ->mPassword
+   $this->loadState();
+
list( $salt, $hash ) = $this->saltedPassword( $password );
 
$this->mPassword = $hash;

-- 
To view, visit https://gerrit.wikimedia.org/r/363891
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7f01c02ed7c369e71f748ac9142edd1709ea6514
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CentralAuth
Gerrit-Branch: wmf/1.30.0-wmf.7
Gerrit-Owner: Legoktm 
Gerrit-Reviewer: Anomie 

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits