Merge "SECURITY: Move 'UserGetRights' call before application of Session::getAllowedU...
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Tue, 23 Aug 2016 04:34:38 +0000 (04:34 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Tue, 23 Aug 2016 04:34:38 +0000 (04:34 +0000)
1  2 
includes/user/User.php

diff --combined includes/user/User.php
@@@ -3134,6 -3134,7 +3134,7 @@@ class User implements IDBAccessObject 
        public function getRights() {
                if ( is_null( $this->mRights ) ) {
                        $this->mRights = self::getGroupPermissions( $this->getEffectiveGroups() );
+                       Hooks::run( 'UserGetRights', [ $this, &$this->mRights ] );
  
                        // Deny any rights denied by the user's session, unless this
                        // endpoint has no sessions.
                                }
                        }
  
-                       Hooks::run( 'UserGetRights', [ $this, &$this->mRights ] );
                        // Force reindexation of rights when a hook has unset one of them
                        $this->mRights = array_values( array_unique( $this->mRights ) );
  
                $noPass = PasswordFactory::newInvalidPassword()->toString();
  
                $dbw = wfGetDB( DB_MASTER );
 -              $inWrite = $dbw->writesOrCallbacksPending();
                $seqVal = $dbw->nextSequenceValue( 'user_user_id_seq' );
                $dbw->insert( 'user',
                        [
                        [ 'IGNORE' ]
                );
                if ( !$dbw->affectedRows() ) {
 -                      // The queries below cannot happen in the same REPEATABLE-READ snapshot.
 -                      // Handle this by COMMIT, if possible, or by LOCK IN SHARE MODE otherwise.
 -                      if ( $inWrite ) {
 -                              // Can't commit due to pending writes that may need atomicity.
 -                              // This may cause some lock contention unlike the case below.
 -                              $options = [ 'LOCK IN SHARE MODE' ];
 -                              $flags = self::READ_LOCKING;
 -                      } else {
 -                              // Often, this case happens early in views before any writes when
 -                              // using CentralAuth. It's should be OK to commit and break the snapshot.
 -                              $dbw->commit( __METHOD__, 'flush' );
 -                              $options = [];
 -                              $flags = self::READ_LATEST;
 -                      }
 -                      $this->mId = $dbw->selectField( 'user', 'user_id',
 -                              [ 'user_name' => $this->mName ], __METHOD__, $options );
 +                      // Use locking reads to bypass any REPEATABLE-READ snapshot.
 +                      $this->mId = $dbw->selectField(
 +                              'user',
 +                              'user_id',
 +                              [ 'user_name' => $this->mName ],
 +                              __METHOD__,
 +                              [ 'LOCK IN SHARE MODE' ]
 +                      );
                        $loaded = false;
                        if ( $this->mId ) {
 -                              if ( $this->loadFromDatabase( $flags ) ) {
 +                              if ( $this->loadFromDatabase( self::READ_LOCKING ) ) {
                                        $loaded = true;
                                }
                        }