Fix #9718: Fix user staying authorized despite authKey change

docs/guide/security-authentication.md

@@ -42,11 +42,10 @@ the following methods:
   an instance of the identity class using the specified access token. This method is used when you need
   to authenticate a user by a single secret token (e.g. in a stateless RESTful application).
 * [[yii\web\IdentityInterface::getId()|getId()]]: it returns the ID of the user represented by this identity instance.
-* [[yii\web\IdentityInterface::getAuthKey()|getAuthKey()]]: it returns a key used to verify cookie-based login.
+* [[yii\web\IdentityInterface::getAuthKey()|getAuthKey()]]: it returns a key used to validate session and auto-login in
-  The key is stored in the login cookie and will be later compared with the server-side version to make
+  case it is enabled.
-  sure the login cookie is valid.
 * [[yii\web\IdentityInterface::validateAuthKey()|validateAuthKey()]]: it implements the logic for verifying
-  the cookie-based login key.
+  authentication key.
 
 If a particular method is not needed, you may implement it with an empty body. For example, if your application
 is a pure stateless RESTful application, you would only need to implement [[yii\web\IdentityInterface::findIdentityByAccessToken()|findIdentityByAccessToken()]]
@@ -117,8 +116,7 @@ class User extends ActiveRecord implements IdentityInterface
 }
 ```
 
-As explained previously, you only need to implement `getAuthKey()` and `validateAuthKey()` if your application
+You may use the following code to generate an auth key for each
-uses cookie-based login feature. In this case, you may use the following code to generate an auth key for each
 user and store it in the `user` table:
 
 ```php

framework/CHANGELOG.md

@@ -20,6 +20,7 @@ Yii Framework 2 Change Log
 - Enh #18487: Allow creating URLs for non-GET-verb rules (bizley)
 - Bug #8750: Fix MySQL support when running in `ANSI`/`ANSI_QUOTES` modes (brandonkelly)
 - Bug #18505: Fixed `yii\helpers\ArrayHelper::getValue()` for ArrayAccess objects with explicitly defined properties (samdark)
+- Bug #9718: Fix user staying authorized despite authKey change (kidol, Charlie Jack, Kunal Mhaske, samdark)
 - Bug #18508: Fix Postgres SQL query for load table indexes with correct column order (insolita)
 - Enh #18518: Add support for ngrok’s `X-Original-Host` header (brandonkelly)
 - Bug #18529: Fix asset files path with `appendTimestamp` option for non-root-relative base URLs (bizley)

framework/UPGRADE.md

@@ -51,6 +51,17 @@ if you want to upgrade from version A to version C and there is
 version B between A and C, you need to follow the instructions
 for both A and B.
 
+Upgrade from Yii 2.0.40
+-----------------------
+
+* The methods `getAuthKey()` and `validateAuthKey()` of `yii\web\IdentityInterface` are now also used to validate active
+  sessions (previously these methods were only used for cookie-based login). If your identity class does not properly
+  implement these methods yet, you should update it accordingly (an example can be found in the guide under
+  `Security` -> `Authentication`). Alternatively, you can simply return `null` in the `getAuthKey()` method to keep
+  the old behavior (that is, no validation of active sessions). Applications that change the underlying `authKey` of
+  an authenticated identity, should now call `yii\web\User::switchIdentity()`, `yii\web\User::login()`
+  or `yii\web\User::logout()` to recreate the active session with the new `authKey`.
+
 Upgrade from Yii 2.0.39.3
 -------------------------
 

framework/web/IdentityInterface.php

@@ -82,8 +82,7 @@ interface IdentityInterface
      *
      * The space of such keys should be big enough to defeat potential identity attacks.
      *
-     * This is required if [[User::enableAutoLogin]] is enabled. The returned key will be stored on the
+     * The returned key is used to validate session and auto-login (if [[User::enableAutoLogin]] is enabled).
-     * client side as a cookie and will be used to authenticate user even if PHP session has been expired.
      *
      * Make sure to invalidate earlier issued authKeys when you implement force user logout, password change and
      * other scenarios, that require forceful access revocation for old sessions.
@@ -96,7 +95,6 @@ interface IdentityInterface
     /**
      * Validates the given auth key.
      *
-     * This is required if [[User::enableAutoLogin]] is enabled.
      * @param string $authKey the given auth key
      * @return bool whether the given auth key is valid.
      * @see getAuthKey()

framework/web/User.php

@@ -131,6 +131,11 @@ class User extends Component
      * @var string the session variable name used to store the value of [[id]].
      */
     public $idParam = '__id';
+    /**
+     * @var string the session variable name used to store authentication key.
+     * @since 2.0.41
+     */
+    public $authKeyParam = '__authKey';
     /**
      * @var string the session variable name used to store the value of expiration timestamp of the authenticated state.
      * This is used when [[authTimeout]] is set.
@@ -599,7 +604,8 @@ class User extends Component
                 if (!$identity instanceof IdentityInterface) {
                     throw new InvalidValueException("$class::findIdentity() must return an object implementing IdentityInterface.");
                 } elseif (!$identity->validateAuthKey($authKey)) {
-                    Yii::warning("Invalid auth key attempted for user '$id': $authKey", __METHOD__);
+                    $ip = Yii::$app->getRequest()->getUserIP();
+                    Yii::warning("Invalid cookie auth key attempted for user '$id' from $ip: $authKey", __METHOD__);
                 } else {
                     return ['identity' => $identity, 'duration' => $duration];
                 }
@@ -654,9 +660,11 @@ class User extends Component
         }
         $session->remove($this->idParam);
         $session->remove($this->authTimeoutParam);
+        $session->remove($this->authKeyParam);
 
         if ($identity) {
             $session->set($this->idParam, $identity->getId());
+            $session->set($this->authKeyParam, $identity->getAuthKey());
             if ($this->authTimeout !== null) {
                 $session->set($this->authTimeoutParam, time() + $this->authTimeout);
             }
@@ -692,6 +700,15 @@ class User extends Component
             $identity = $class::findIdentity($id);
         }
 
+        if ($identity !== null) {
+            $authKey = $session->get($this->authKeyParam);
+            if ($authKey !== null && !$identity->validateAuthKey($authKey)) {
+                $identity = null;
+                $ip = Yii::$app->getRequest()->getUserIP();
+                Yii::warning("Invalid session auth key attempted for user '$id' from $ip: $authKey", __METHOD__);
+            }
+        }
+
         $this->setIdentity($identity);
 
         if ($identity !== null && ($this->authTimeout !== null || $this->absoluteAuthTimeout !== null)) {
@@ -757,7 +774,7 @@ class User extends Component
     protected function checkRedirectAcceptable()
     {
         $acceptableTypes = Yii::$app->getRequest()->getAcceptableContentTypes();
-        if (empty($acceptableTypes) || count($acceptableTypes) === 1 && array_keys($acceptableTypes)[0] === '*/*') {
+        if (empty($acceptableTypes) || (count($acceptableTypes) === 1 && array_keys($acceptableTypes)[0] === '*/*')) {
             return true;
         }
 

tests/framework/filters/stubs/UserIdentity.php

@@ -8,7 +8,6 @@
 namespace yiiunit\framework\filters\stubs;
 
 use yii\base\Component;
-use yii\base\NotSupportedException;
 use yii\web\IdentityInterface;
 
 /**
@@ -61,11 +60,11 @@ class UserIdentity extends Component implements IdentityInterface
 
     public function getAuthKey()
     {
-        throw new NotSupportedException();
+        return null;
     }
 
     public function validateAuthKey($authKey)
     {
-        throw new NotSupportedException();
+        return true;
     }
 }

tests/framework/web/UserTest.php

@@ -451,6 +451,77 @@ class UserTest extends TestCase
         $this->expectException('\yii\base\InvalidValueException');
         Yii::$app->user->setIdentity(new \stdClass());
     }
+
+    public function testSessionAuthWithNonExistingId()
+    {
+        $appConfig = [
+            'components' => [
+                'user' => [
+                    'identityClass' => UserIdentity::className(),
+                ],
+            ],
+        ];
+
+        $this->mockWebApplication($appConfig);
+
+        Yii::$app->session->set('__id', '1');
+
+        $this->assertNull(Yii::$app->user->getIdentity());
+    }
+
+    public function testSessionAuthWithMissingKey()
+    {
+        $appConfig = [
+            'components' => [
+                'user' => [
+                    'identityClass' => UserIdentity::className(),
+                ],
+            ],
+        ];
+
+        $this->mockWebApplication($appConfig);
+
+        Yii::$app->session->set('__id', 'user1');
+
+        $this->assertNotNull(Yii::$app->user->getIdentity());
+    }
+
+    public function testSessionAuthWithInvalidKey()
+    {
+        $appConfig = [
+            'components' => [
+                'user' => [
+                    'identityClass' => UserIdentity::className(),
+                ],
+            ],
+        ];
+
+        $this->mockWebApplication($appConfig);
+
+        Yii::$app->session->set('__id', 'user1');
+        Yii::$app->session->set('__authKey', 'invalid');
+
+
+        $this->assertNull(Yii::$app->user->getIdentity());
+    }
+
+    public function testSessionAuthWithValidKey()
+    {
+        $appConfig = [
+            'components' => [
+                'user' => [
+                    'identityClass' => UserIdentity::className(),
+                ],
+            ],
+        ];
+
+        $this->mockWebApplication($appConfig);
+
+        Yii::$app->session->set('__id', 'user1');
+        Yii::$app->session->set('__authKey', 'ABCD1234');
+
+        $this->assertNotNull(Yii::$app->user->getIdentity());
+    }
 }
 
 static $cookiesMock;