[9.x] Improve password checks (#42248)

* Added tests for auth providers with multiple passwords * Improve password checks * formatting * remove method Co-authored-by: Taylor Otwell <taylor@laravel.com>
laravel · May 4, 2022 · 8542414 · 8542414
1 parent 3be7023
commit 8542414
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 21 deletions.
diff --git a/src/Illuminate/Auth/DatabaseUserProvider.php b/src/Illuminate/Auth/DatabaseUserProvider.php
@@ -99,9 +99,13 @@ public function updateRememberToken(UserContract $user, $token)
      */
     public function retrieveByCredentials(array $credentials)
     {
-        if (empty($credentials) ||
-           (count($credentials) === 1 &&
-            array_key_exists('password', $credentials))) {
+        $credentials = array_filter(
+            $credentials,
+            fn ($key) => ! str_contains($key, 'password'),
+            ARRAY_FILTER_USE_KEY
+        );
+
+        if (empty($credentials)) {
             return;
         }
 

diff --git a/src/Illuminate/Auth/EloquentUserProvider.php b/src/Illuminate/Auth/EloquentUserProvider.php
@@ -105,9 +105,13 @@ public function updateRememberToken(UserContract $user, $token)
      */
     public function retrieveByCredentials(array $credentials)
     {
-        if (empty($credentials) ||
-           (count($credentials) === 1 &&
-            str_contains($this->firstCredentialKey($credentials), 'password'))) {
+        $credentials = array_filter(
+            $credentials,
+            fn ($key) => ! str_contains($key, 'password'),
+            ARRAY_FILTER_USE_KEY
+        );
+
+        if (empty($credentials)) {
             return;
         }
 
@@ -117,10 +121,6 @@ public function retrieveByCredentials(array $credentials)
         $query = $this->newModelQuery();
 
         foreach ($credentials as $key => $value) {
-            if (str_contains($key, 'password')) {
-                continue;
-            }
-
             if (is_array($value) || $value instanceof Arrayable) {
                 $query->whereIn($key, $value);
             } elseif ($value instanceof Closure) {
@@ -133,17 +133,6 @@ public function retrieveByCredentials(array $credentials)
         return $query->first();
     }
 
-    /**
-     * Get the first key from the credential array.
-     *
-     * @param  array  $credentials
-     * @return string|null
-     */
-    protected function firstCredentialKey(array $credentials)
-    {
-        return array_key_first($credentials);
-    }
-
     /**
      * Validate a user against the given credentials.
      *

diff --git a/tests/Auth/AuthDatabaseUserProviderTest.php b/tests/Auth/AuthDatabaseUserProviderTest.php
@@ -135,6 +135,19 @@ public function testRetrieveByCredentialsReturnsNullWhenUserIsFound()
         $this->assertNull($user);
     }
 
+    public function testRetrieveByCredentialsWithMultiplyPasswordsReturnsNull()
+    {
+        $conn = m::mock(Connection::class);
+        $hasher = m::mock(Hasher::class);
+        $provider = new DatabaseUserProvider($conn, $hasher, 'foo');
+        $user = $provider->retrieveByCredentials([
+            'password' => 'dayle',
+            'password2' => 'night'
+        ]);
+
+        $this->assertNull($user);
+    }
+
     public function testCredentialValidation()
     {
         $conn = m::mock(Connection::class);

diff --git a/tests/Auth/AuthEloquentUserProviderTest.php b/tests/Auth/AuthEloquentUserProviderTest.php
@@ -117,6 +117,17 @@ public function testRetrieveByCredentialsAcceptsCallback()
         $this->assertSame('bar', $user);
     }
 
+    public function testRetrieveByCredentialsWithMultiplyPasswordsReturnsNull()
+    {
+        $provider = $this->getProviderMock();
+        $user = $provider->retrieveByCredentials([
+            'password' => 'dayle',
+            'password2' => 'night'
+        ]);
+
+        $this->assertNull($user);
+    }
+
     public function testCredentialValidation()
     {
         $hasher = m::mock(Hasher::class);