Don't override action in UI and REDIRECT responses
authorBrad Jorsch <bjorsch@wikimedia.org>
Fri, 3 Jun 2016 15:33:41 +0000 (11:33 -0400)
committerBrad Jorsch <bjorsch@wikimedia.org>
Fri, 3 Jun 2016 15:37:29 +0000 (11:37 -0400)
In Ic8caf57eb, we changed things so the requests returned in a UI or
REDIRECT response would have the action forced to that appropriate for
the action being peformed. But ResetPasswordSecondaryAuthenticationProvider
has a use case where a mismatch is necessary: it's run during the login
action, but it needs a PasswordAuthenticationResponse for a change
action.

Bug: T136894
Change-Id: I9d109a22c5b2d2064f664f584100ecaab43199c5

includes/auth/AuthManager.php
includes/auth/ResetPasswordSecondaryAuthenticationProvider.php

index 402ea96..2ed0d61 100644 (file)
@@ -558,7 +558,7 @@ class AuthManager implements LoggerAwareInterface {
                                        );
                                        $ret->neededRequests[] = $ret->createRequest;
                                }
-                               $this->fillRequests( $ret->neededRequests, self::ACTION_LOGIN, null );
+                               $this->fillRequests( $ret->neededRequests, self::ACTION_LOGIN, null, true );
                                $session->setSecret( 'AuthManager::authnState', [
                                        'reqs' => [], // Will be filled in later
                                        'primary' => null,
@@ -2056,7 +2056,7 @@ class AuthManager implements LoggerAwareInterface {
                }
 
                // Fill in reqs data
-               $this->fillRequests( $reqs, $providerAction, $options['username'] );
+               $this->fillRequests( $reqs, $providerAction, $options['username'], true );
 
                // For self::ACTION_CHANGE, filter out any that something else *doesn't* allow changing
                if ( $providerAction === self::ACTION_CHANGE || $providerAction === self::ACTION_REMOVE ) {
@@ -2073,10 +2073,13 @@ class AuthManager implements LoggerAwareInterface {
         * @param AuthenticationRequest[] &$reqs
         * @param string $action
         * @param string|null $username
+        * @param boolean $forceAction
         */
-       private function fillRequests( array &$reqs, $action, $username ) {
+       private function fillRequests( array &$reqs, $action, $username, $forceAction = false ) {
                foreach ( $reqs as $req ) {
-                       $req->action = $action;
+                       if ( !$req->action || $forceAction ) {
+                               $req->action = $action;
+                       }
                        if ( $req->username === null ) {
                                $req->username = $username;
                        }
index 2e51cf2..f87a762 100644 (file)
@@ -95,10 +95,8 @@ class ResetPasswordSecondaryAuthenticationProvider extends AbstractSecondaryAuth
                        }
                }
 
-               if ( isset( $data->req ) ) {
-                       $needReq = $data->req;
-               } else {
-                       $needReq = new PasswordAuthenticationRequest();
+               $needReq = isset( $data->req ) ? $data->req : new PasswordAuthenticationRequest();
+               if ( !$needReq->action ) {
                        $needReq->action = AuthManager::ACTION_CHANGE;
                }
                $needReqs = [ $needReq ];