From 808b098b58ee959d9ebdba5f4e5f54010a97f8ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20=C4=B0mzal=C4=B1?= Date: Sat, 30 Sep 2023 10:57:26 +0300 Subject: [PATCH 1/3] supportOldDangerousPassword support removed --- src/Authentication/Authenticators/Session.php | 14 ++-------- src/Authentication/Passwords.php | 15 ---------- src/Config/Auth.php | 10 ------- .../SessionAuthenticatorTest.php | 28 ------------------- 4 files changed, 2 insertions(+), 65 deletions(-) diff --git a/src/Authentication/Authenticators/Session.php b/src/Authentication/Authenticators/Session.php index c5e9ba138..9ac01efca 100644 --- a/src/Authentication/Authenticators/Session.php +++ b/src/Authentication/Authenticators/Session.php @@ -336,30 +336,20 @@ 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; + } // 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); } diff --git a/src/Authentication/Passwords.php b/src/Authentication/Passwords.php index 3a5b7b041..87c0f1f60 100644 --- a/src/Authentication/Passwords.php +++ b/src/Authentication/Passwords.php @@ -81,21 +81,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. */ diff --git a/src/Config/Auth.php b/src/Config/Auth.php index e7ad04189..ac2d5dc08 100644 --- a/src/Config/Auth.php +++ b/src/Config/Auth.php @@ -363,16 +363,6 @@ class Auth extends BaseConfig */ public int $hashCost = 10; - /** - * 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 diff --git a/tests/Authentication/Authenticators/SessionAuthenticatorTest.php b/tests/Authentication/Authenticators/SessionAuthenticatorTest.php index 662b32fd0..b0109a83f 100644 --- a/tests/Authentication/Authenticators/SessionAuthenticatorTest.php +++ b/tests/Authentication/Authenticators/SessionAuthenticatorTest.php @@ -304,34 +304,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([ From 6cdc44a389194b9ea904e0f479d6bf44ab1f2127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20=C4=B0mzal=C4=B1?= Date: Sat, 30 Sep 2023 11:10:00 +0300 Subject: [PATCH 2/3] supportOldDangerousPassword support removed && style fixes according to style guide --- src/Authentication/Authenticators/Session.php | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/src/Authentication/Authenticators/Session.php b/src/Authentication/Authenticators/Session.php index 9ac01efca..2071a9a30 100644 --- a/src/Authentication/Authenticators/Session.php +++ b/src/Authentication/Authenticators/Session.php @@ -95,8 +95,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\'.' ); } } @@ -130,7 +130,7 @@ public function attempt(array $credentials): Result $result = $this->check($credentials); // Credentials mismatch. - if (! $result->isOK()) { + if (!$result->isOK()) { // Always record a login attempt, whether success or not. $this->recordLoginAttempt($credentials, false, $ipAddress, $userAgent); @@ -173,7 +173,7 @@ public function attempt(array $credentials): Result $this->issueRememberMeToken(); - if (! $this->hasAction()) { + if (!$this->hasAction()) { $this->completeLogin($user); } @@ -284,10 +284,10 @@ private function recordLoginAttempt( $field = array_pop($field); - if (! in_array($field, ['email', 'username'], true)) { + if (!in_array($field, ['email', 'username'], true)) { $idType = $field; } else { - $idType = (! isset($credentials['email']) && isset($credentials['username'])) + $idType = (!isset($credentials['email']) && isset($credentials['username'])) ? self::ID_TYPE_USERNAME : self::ID_TYPE_EMAIL_PASSWORD; } @@ -337,12 +337,11 @@ public function check(array $credentials): Result $passwords = service('passwords'); // Now, try matching the passwords. - if (! $passwords->verify($givenPassword, $user->password_hash)) { - return new Result([ - 'success' => false, - 'reason' => lang('Auth.invalidPassword'), - ]); - + if (!$passwords->verify($givenPassword, $user->password_hash)) { + return new Result([ + 'success' => false, + 'reason' => lang('Auth.invalidPassword'), + ]); } // Check to see if the password needs to be rehashed. @@ -644,10 +643,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 ); } @@ -732,18 +731,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 ); } @@ -886,7 +885,7 @@ public function getPendingUser(): ?User */ public function recordActiveDate(): void { - if (! $this->user instanceof User) { + if (!$this->user instanceof User) { throw new InvalidArgumentException( __METHOD__ . '() requires logged in user before calling.' ); From 157f21f1bdef4bc92f84e02a75334d75a406b53c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20=C4=B0mzal=C4=B1?= Date: Sat, 30 Sep 2023 16:20:39 +0300 Subject: [PATCH 3/3] PHPStan changed to 8 and composer fix --- phpstan-baseline.php | 2 +- src/Authentication/Authenticators/Session.php | 12 ++++++------ .../Authenticators/SessionAuthenticatorTest.php | 1 - 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/phpstan-baseline.php b/phpstan-baseline.php index 2e880e9ff..65396c867 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -318,7 +318,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[] = [ diff --git a/src/Authentication/Authenticators/Session.php b/src/Authentication/Authenticators/Session.php index 2071a9a30..a4b0f5248 100644 --- a/src/Authentication/Authenticators/Session.php +++ b/src/Authentication/Authenticators/Session.php @@ -130,7 +130,7 @@ public function attempt(array $credentials): Result $result = $this->check($credentials); // Credentials mismatch. - if (!$result->isOK()) { + if (! $result->isOK()) { // Always record a login attempt, whether success or not. $this->recordLoginAttempt($credentials, false, $ipAddress, $userAgent); @@ -173,7 +173,7 @@ public function attempt(array $credentials): Result $this->issueRememberMeToken(); - if (!$this->hasAction()) { + if (! $this->hasAction()) { $this->completeLogin($user); } @@ -284,10 +284,10 @@ private function recordLoginAttempt( $field = array_pop($field); - if (!in_array($field, ['email', 'username'], true)) { + if (! in_array($field, ['email', 'username'], true)) { $idType = $field; } else { - $idType = (!isset($credentials['email']) && isset($credentials['username'])) + $idType = (! isset($credentials['email']) && isset($credentials['username'])) ? self::ID_TYPE_USERNAME : self::ID_TYPE_EMAIL_PASSWORD; } @@ -337,7 +337,7 @@ public function check(array $credentials): Result $passwords = service('passwords'); // Now, try matching the passwords. - if (!$passwords->verify($givenPassword, $user->password_hash)) { + if (! $passwords->verify($givenPassword, $user->password_hash)) { return new Result([ 'success' => false, 'reason' => lang('Auth.invalidPassword'), @@ -885,7 +885,7 @@ public function getPendingUser(): ?User */ public function recordActiveDate(): void { - if (!$this->user instanceof User) { + if (! $this->user instanceof User) { throw new InvalidArgumentException( __METHOD__ . '() requires logged in user before calling.' ); diff --git a/tests/Authentication/Authenticators/SessionAuthenticatorTest.php b/tests/Authentication/Authenticators/SessionAuthenticatorTest.php index b0109a83f..2a3249f00 100644 --- a/tests/Authentication/Authenticators/SessionAuthenticatorTest.php +++ b/tests/Authentication/Authenticators/SessionAuthenticatorTest.php @@ -12,7 +12,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;