From fef02d1e7554813e60724f450ad274d9bdb93965 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/4] 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 98e4a29a0..8802b7e9e 100644 --- a/src/Authentication/Authenticators/Session.php +++ b/src/Authentication/Authenticators/Session.php @@ -343,30 +343,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 994c192a8..42f9fe0ed 100644 --- a/src/Authentication/Passwords.php +++ b/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. */ diff --git a/src/Config/Auth.php b/src/Config/Auth.php index 22a052b01..bf4c9ca02 100644 --- a/src/Config/Auth.php +++ b/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 diff --git a/tests/Authentication/Authenticators/SessionAuthenticatorTest.php b/tests/Authentication/Authenticators/SessionAuthenticatorTest.php index ca1a8b53e..d181d398f 100644 --- a/tests/Authentication/Authenticators/SessionAuthenticatorTest.php +++ b/tests/Authentication/Authenticators/SessionAuthenticatorTest.php @@ -313,34 +313,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 f54d7bf40aac6459ac27c492c92ab7c2cc119431 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/4] 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 8802b7e9e..6d040b494 100644 --- a/src/Authentication/Authenticators/Session.php +++ b/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\'.' ); } } @@ -137,7 +137,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); @@ -180,7 +180,7 @@ public function attempt(array $credentials): Result $this->issueRememberMeToken(); - if (! $this->hasAction()) { + if (!$this->hasAction()) { $this->completeLogin($user); } @@ -291,10 +291,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; } @@ -344,12 +344,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. @@ -651,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 ); } @@ -739,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 ); } @@ -893,7 +892,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 c528352d3c3c8c03f15e05bac4e941da47ac5699 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/4] 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 d6c3d7580..dd4fafb64 100644 --- a/phpstan-baseline.php +++ b/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[] = [ diff --git a/src/Authentication/Authenticators/Session.php b/src/Authentication/Authenticators/Session.php index 6d040b494..c480dae5c 100644 --- a/src/Authentication/Authenticators/Session.php +++ b/src/Authentication/Authenticators/Session.php @@ -137,7 +137,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); @@ -180,7 +180,7 @@ public function attempt(array $credentials): Result $this->issueRememberMeToken(); - if (!$this->hasAction()) { + if (! $this->hasAction()) { $this->completeLogin($user); } @@ -291,10 +291,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; } @@ -344,7 +344,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'), @@ -892,7 +892,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 d181d398f..3fb04a386 100644 --- a/tests/Authentication/Authenticators/SessionAuthenticatorTest.php +++ b/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; From 2d4c315aab439952253a2d028b19b73b5085bf63 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 9 Dec 2023 07:08:33 +0900 Subject: [PATCH 4/4] docs: add UPGRADING.md --- UPGRADING.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/UPGRADING.md b/UPGRADING.md index e004c9d2c..c0248a97f 100644 --- a/UPGRADING.md +++ b/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