Merge pull request #976 from kenjis/remove-supportOldDangerousPassword

refactor: remove supportOldDangerousPassword

Author: kenjis

UPGRADING.md

@@ -1,5 +1,13 @@
 # Upgrade Guide
 
+## Version 1.0.0-beta.8 to 1.0.0
+
+## Removed Deprecated Items
+
+The [$supportOldDangerousPassword](#if-you-want-to-allow-login-with-existing-passwords)
+feature for backward compatiblity has been removed. The old passwords saved in
+Shield v1.0.0-beta.3 or earlier are no longer supported.
+
 ## Version 1.0.0-beta.7 to 1.0.0-beta.8
 
 ### Mandatory Config Changes

phpstan-baseline.php

@@ -348,7 +348,7 @@
 ];
 $ignoreErrors[] = [
 	'message' => '#^Call to method PHPUnit\\\\Framework\\\\Assert\\:\\:assertInstanceOf\\(\\) with \'CodeIgniter\\\\\\\\Shield\\\\\\\\Result\' and CodeIgniter\\\\Shield\\\\Result will always evaluate to true\\.$#',
-	'count' => 9,
+	'count' => 8,
 	'path' => __DIR__ . '/tests/Authentication/Authenticators/SessionAuthenticatorTest.php',
 ];
 $ignoreErrors[] = [

src/Authentication/Authenticators/Session.php

@@ -102,8 +102,8 @@ private function checkSecurityConfig(): void
         if ($securityConfig->csrfProtection === 'cookie') {
             throw new SecurityException(
                 'Config\Security::$csrfProtection is set to \'cookie\'.'
-                . ' Same-site attackers may bypass the CSRF protection.'
-                . ' Please set it to \'session\'.'
+                    . ' Same-site attackers may bypass the CSRF protection.'
+                    . ' Please set it to \'session\'.'
             );
         }
     }
@@ -343,30 +343,19 @@ public function check(array $credentials): Result
         /** @var Passwords $passwords */
         $passwords = service('passwords');
 
-        // This is only for supportOldDangerousPassword.
-        $needsRehash = false;
-
         // Now, try matching the passwords.
         if (! $passwords->verify($givenPassword, $user->password_hash)) {
-            if (
-                ! setting('Auth.supportOldDangerousPassword')
-                || ! $passwords->verifyDanger($givenPassword, $user->password_hash) // @phpstan-ignore-line
-            ) {
-                return new Result([
-                    'success' => false,
-                    'reason'  => lang('Auth.invalidPassword'),
-                ]);
-            }
-
-            // Passed with old dangerous password.
-            $needsRehash = true;
+            return new Result([
+                'success' => false,
+                'reason'  => lang('Auth.invalidPassword'),
+            ]);
         }
 
         // Check to see if the password needs to be rehashed.
         // This would be due to the hash algorithm or hash
         // cost changing since the last time that a user
         // logged in.
-        if ($passwords->needsRehash($user->password_hash) || $needsRehash) {
+        if ($passwords->needsRehash($user->password_hash)) {
             $user->password_hash = $passwords->hash($givenPassword);
             $this->provider->save($user);
         }
@@ -661,10 +650,10 @@ public function startLogin(User $user): void
         if ($userId !== null) {
             throw new LogicException(
                 'The user has User Info in Session, so already logged in or in pending login state.'
-                . ' If a logged in user logs in again with other account, the session data of the previous'
-                . ' user will be used as the new user.'
-                . ' Fix your code to prevent users from logging in without logging out or delete the session data.'
-                . ' user_id: ' . $userId
+                    . ' If a logged in user logs in again with other account, the session data of the previous'
+                    . ' user will be used as the new user.'
+                    . ' Fix your code to prevent users from logging in without logging out or delete the session data.'
+                    . ' user_id: ' . $userId
             );
         }
 
@@ -749,18 +738,18 @@ public function login(User $user): void
         if ($this->getIdentitiesForAction($user) !== []) {
             throw new LogicException(
                 'The user has identities for action, so cannot complete login.'
-                . ' If you want to start to login with auth action, use startLogin() instead.'
-                . ' Or delete identities for action in database.'
-                . ' user_id: ' . $user->id
+                    . ' If you want to start to login with auth action, use startLogin() instead.'
+                    . ' Or delete identities for action in database.'
+                    . ' user_id: ' . $user->id
             );
         }
         // Check auth_action in Session
         if ($this->getSessionKey('auth_action')) {
             throw new LogicException(
                 'The user has auth action in session, so cannot complete login.'
-                . ' If you want to start to login with auth action, use startLogin() instead.'
-                . ' Or delete `auth_action` and `auth_action_message` in session data.'
-                . ' user_id: ' . $user->id
+                    . ' If you want to start to login with auth action, use startLogin() instead.'
+                    . ' Or delete `auth_action` and `auth_action_message` in session data.'
+                    . ' user_id: ' . $user->id
             );
         }
 

src/Authentication/Passwords.php

@@ -90,21 +90,6 @@ public function verify(string $password, string $hash): bool
         return password_verify($password, $hash);
     }
 
-    /**
-     * Verifies a password against a previously hashed password.
-     *
-     * @param string $password The password we're checking
-     * @param string $hash     The previously hashed password
-     *
-     * @deprecated This is only for backward compatibility.
-     */
-    public function verifyDanger(string $password, string $hash): bool
-    {
-        return password_verify(base64_encode(
-            hash('sha384', $password, true)
-        ), $hash);
-    }
-
     /**
      * Checks to see if a password should be rehashed.
      */

src/Config/Auth.php

@@ -374,16 +374,6 @@ class Auth extends BaseConfig
      */
     public int $hashCost = 12;
 
-    /**
-     * If you need to support passwords saved in versions prior to Shield v1.0.0-beta.4.
-     * set this to true.
-     *
-     * See https://github.com/codeigniter4/shield/security/advisories/GHSA-c5vj-f36q-p9vg
-     *
-     * @deprecated This is only for backward compatibility.
-     */
-    public bool $supportOldDangerousPassword = false;
-
     /**
      * ////////////////////////////////////////////////////////////////////
      * OTHER SETTINGS

tests/Authentication/Authenticators/SessionAuthenticatorTest.php

@@ -21,7 +21,6 @@
 use CodeIgniter\Shield\Entities\User;
 use CodeIgniter\Shield\Exceptions\LogicException;
 use CodeIgniter\Shield\Models\RememberModel;
-use CodeIgniter\Shield\Models\UserIdentityModel;
 use CodeIgniter\Shield\Models\UserModel;
 use CodeIgniter\Shield\Result;
 use CodeIgniter\Test\Mock\MockEvents;
@@ -313,34 +312,6 @@ public function testCheckSuccess(): void
         $this->assertSame($this->user->id, $foundUser->id);
     }
 
-    public function testCheckSuccessOldDangerousPassword(): void
-    {
-        /** @var Auth $config */
-        $config                              = config('Auth');
-        $config->supportOldDangerousPassword = true; // @phpstan-ignore-line
-
-        fake(
-            UserIdentityModel::class,
-            [
-                'user_id' => $this->user->id,
-                'type'    => Session::ID_TYPE_EMAIL_PASSWORD,
-                'secret'  => 'foo@example.com',
-                'secret2' => '$2y$10$WswjNNcR24cJvsXvBc5TveVVVQ9/EYC0eq.Ad9e/2cVnmeSEYBOEm',
-            ]
-        );
-
-        $result = $this->auth->check([
-            'email'    => 'foo@example.com',
-            'password' => 'passw0rd!',
-        ]);
-
-        $this->assertInstanceOf(Result::class, $result);
-        $this->assertTrue($result->isOK());
-
-        $foundUser = $result->extraInfo();
-        $this->assertSame($this->user->id, $foundUser->id);
-    }
-
     public function testAttemptCannotFindUser(): void
     {
         $result = $this->auth->attempt([