feat: Add AuthService validateCredentials method

matiasperrone-exo · matiasperrone-exo · commit 4f356635a104 · 2026-05-21T18:01:52.000Z
- test: cover canLogin()=false branch in validateCredentials() unit tests
- docs: document known double-query cost in validateCredentials()
- fix: use consistent error message in validateCredentials()
diff --git a/app/libs/Auth/AuthService.php b/app/libs/Auth/AuthService.php
@@ -1,4 +1,5 @@
-<?php namespace Auth;
+<?php
+namespace Auth;
 /**
  * Copyright 2016 OpenStack Foundation
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -97,21 +98,19 @@ final class AuthService extends AbstractService implements IAuthService
      * @param IAuthUserService $auth_user_service
      * @param ISecurityContextService $security_context_service
      * @param ITransactionService $tx_service
-     * @params ISecurityContextService $security_context_service
      */
     public function __construct
     (
-        IUserRepository         $user_repository,
-        IOAuth2OTPRepository    $otp_repository,
-        IPrincipalService       $principal_service,
-        IUserService            $user_service,
-        IUserActionService      $user_action_service,
-        ICacheService           $cache_service,
-        IAuthUserService        $auth_user_service,
+        IUserRepository $user_repository,
+        IOAuth2OTPRepository $otp_repository,
+        IPrincipalService $principal_service,
+        IUserService $user_service,
+        IUserActionService $user_action_service,
+        ICacheService $cache_service,
+        IAuthUserService $auth_user_service,
         ISecurityContextService $security_context_service,
-        ITransactionService     $tx_service
-    )
-    {
+        ITransactionService $tx_service
+    ) {
         parent::__construct($tx_service);
         $this->user_repository = $user_repository;
         $this->principal_service = $principal_service;
@@ -131,6 +130,17 @@ public function isUserLogged()
         return Auth::check();
     }
 
+    /**
+     * @return User|null
+     */
+    public function getCurrentUser(): ?User
+    {
+        $user = Auth::user();
+        if ($user instanceof User) {
+            return $user;
+        }
+    }
+
     /**
      * Finds the OTP by value/connection/username, logs the redeem attempt (TX-A),
      * then validates lifecycle / value / scope / audience (TX-B).
@@ -140,13 +150,12 @@ public function isUserLogged()
      * @throws InvalidOTPException
      */
     private function findAndValidateOTP(
-        string  $otp_value,
-        string  $user_name,
-        string  $otp_conn,
+        string $otp_value,
+        string $user_name,
+        string $otp_conn,
         ?string $otp_required_scopes,
         ?Client $client
-    ): OAuth2OTP
-    {
+    ): OAuth2OTP {
         // TX-A: find + log attempt (committed before any validation can throw)
         $otp = $this->tx_service->transaction(function () use ($otp_value, $otp_conn, $user_name, $client) {
 
@@ -189,8 +198,10 @@ private function findAndValidateOTP(
                 throw new InvalidOTPException("Single-use code requested scopes escalates former scopes.");
             }
 
-            if (($otp->hasClient() && is_null($client)) ||
-                ($otp->hasClient() && !is_null($client) && $client->getClientId() != $otp->getClient()->getClientId())) {
+            if (
+                ($otp->hasClient() && is_null($client)) ||
+                ($otp->hasClient() && !is_null($client) && $client->getClientId() != $otp->getClient()->getClientId())
+            ) {
                 throw new AuthenticationException("Single-use code audience mismatch.");
             }
 
@@ -319,8 +330,7 @@ public function verifyOTPChallenge(
         OAuth2OTP $otpClaim,
         User $sessionUser,
         ?Client $client = null
-    ): OAuth2OTP
-    {
+    ): OAuth2OTP {
         Log::debug(sprintf(
             "AuthService::verifyOTPChallenge otp %s session user %s",
             $otpClaim->getValue(),
@@ -384,11 +394,10 @@ public function login(string $username, string $password, bool $remember_me): bo
     {
         Log::debug("AuthService::login");
 
-        $this->last_login_error = "";
         if (!Auth::attempt(['username' => $username, 'password' => $password], $remember_me)) {
             throw new AuthenticationException
             (
-                "We are sorry, your username or password does not match an existing record."
+                "username or password does not match an existing record."
             );
         }
         Log::debug("AuthService::login: clearing principal");
@@ -397,7 +406,7 @@ public function login(string $username, string $password, bool $remember_me): bo
         if (is_null($current_user) || !$current_user->canLogin())
             throw new AuthenticationException
             (
-                "We are sorry, your username or password does not match an existing record."
+                "username or password does not match an existing record."
             );
         $this->principal_service->register
         (
@@ -409,11 +418,51 @@ public function login(string $username, string $password, bool $remember_me): bo
     }
 
     /**
-     * @return User|null
+     * @param string $username
+     * @param string $password
+     * @return User
+     * @throws AuthenticationException
      */
-    public function getCurrentUser(): ?User
+    public function validateCredentials(string $username, string $password): User
     {
-        return Auth::user();
+        Log::debug("AuthService::validateCredentials");
+
+        // retrieveByCredentials swallows AuthenticationLockedUserLoginAttempt and returns null,
+        // so pre-check lock state here to surface a distinct message for locked accounts.
+        $existing = $this->user_repository->getByEmailOrName($username);
+        if (!is_null($existing) && !$existing->isActive()) {
+            throw new AuthenticationException(
+                sprintf("User %s is locked.", $username)
+            );
+        }
+
+        // Known cost: retrieveByCredentials() calls user_repository->getByEmailOrName() internally
+        // (CustomAuthProvider line ~122), duplicating the query above. Eliminating it would require
+        // either changing the provider API to accept a pre-fetched User, or moving
+        // LockUserCounterMeasure checkpoint logic out of the provider — both out of scope here.
+        $user = Auth::getProvider()->retrieveByCredentials([
+            'username' => $username,
+            'password' => $password,
+        ]);
+
+        if (is_null($user) || !$user instanceof User || !$user->canLogin()) {
+            throw new AuthenticationException(
+                "username or password does not match an existing record."
+            );
+        }
+
+        return $user;
+    }
+
+    /**
+     * @param User $user
+     * @param bool $remember
+     * @return void
+     */
+    public function loginUser(User $user, bool $remember): void
+    {
+        Log::debug("AuthService::loginUser");
+        Auth::login($user, $remember);
     }
 
     /**
@@ -618,7 +667,8 @@ public function registerRPLogin(string $client_id): void
                 $rps = $zlib->uncompress($rps);
                 $rps .= '|';
             }
-            if (is_null($rps)) $rps = "";
+            if (is_null($rps))
+                $rps = "";
 
             if (!str_contains($rps, $client_id))
                 $rps .= $client_id;
@@ -720,12 +770,15 @@ public function postLoginUserActions(int $user_id): void
         Log::debug(sprintf("AuthService::postLoginUserActions user %s", $user_id));
         $this->tx_service->transaction(function () use ($user_id) {
             $user = $this->user_repository->getById($user_id);
-            if (!$user instanceof User) return;
+            if (!$user instanceof User)
+                return;
 
             if (!$user->isActive()) {
                 Log::warning(sprintf("AuthService::postLoginUserActions user %s is not active.", $user_id));
-                throw new AuthenticationLockedUserLoginAttempt($user->getEmail(),
-                    sprintf("User %s is locked.", $user->getEmail()));
+                throw new AuthenticationLockedUserLoginAttempt(
+                    $user->getEmail(),
+                    sprintf("User %s is locked.", $user->getEmail())
+                );
             }
 
             //update user fields
@@ -736,4 +789,4 @@ public function postLoginUserActions(int $user_id): void
 
         });
     }
-}
+}
diff --git a/app/libs/Utils/Services/IAuthService.php b/app/libs/Utils/Services/IAuthService.php
@@ -57,6 +57,28 @@ public function getCurrentUser():?User;
      */
     public function login(string $username, string $password, bool $remember_me): bool;
 
+    /**
+     * Validates the supplied credentials without establishing a session.
+     * Delegates to CustomAuthProvider::retrieveByCredentials() so security
+     * checkpoints (LockUserCounterMeasure, etc.) still fire on failure.
+     *
+     * @param string $username
+     * @param string $password
+     * @return User
+     * @throws AuthenticationException on invalid credentials, missing user, or locked account.
+     */
+    public function validateCredentials(string $username, string $password): User;
+
+    /**
+     * Establishes a Laravel session for an already-authenticated user.
+     * Used by the 2FA flow after the second factor is verified.
+     *
+     * @param User $user
+     * @param bool $remember
+     * @return void
+     */
+    public function loginUser(User $user, bool $remember): void;
+
     /**
      * @param OAuth2OTP $otpClaim
      * @param Client|null $client
diff --git a/tests/AuthServiceValidateCredentialsIntegrationTest.php b/tests/AuthServiceValidateCredentialsIntegrationTest.php
@@ -0,0 +1,106 @@
+<?php namespace Tests;
+/**
+ * Copyright 2026 OpenStack Foundation
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ **/
+
+use Auth\Exceptions\AuthenticationException;
+use Auth\Repositories\IUserRepository;
+use Auth\User;
+use Illuminate\Support\Facades\Auth;
+use LaravelDoctrine\ORM\Facades\EntityManager;
+use Utils\Services\IAuthService;
+use Utils\Services\UtilsServiceCatalog;
+
+/**
+ * Class AuthServiceValidateCredentialsIntegrationTest
+ * Exercises AuthService::validateCredentials() against the real database and
+ * security-checkpoint stack to verify that failed attempts increment the
+ * user's login_failed_attempt counter (via LockUserCounterMeasure) and that
+ * no session is established on either success or failure.
+ */
+final class AuthServiceValidateCredentialsIntegrationTest extends OpenStackIDBaseTestCase
+{
+    // CustomAuthProvider looks up users via IUserRepository::getByEmailOrName(),
+    // which currently matches only on the email column — so login uses the email
+    // as the "username".
+    private const SEEDED_USERNAME = 'sebastian@tipit.net';
+    private const SEEDED_PASSWORD = '1Qaz2wsx!';
+
+    private IAuthService $auth_service;
+
+    protected function prepareForTests(): void
+    {
+        parent::prepareForTests();
+        $this->auth_service = $this->app[UtilsServiceCatalog::AuthenticationService];
+    }
+
+    /**
+     * A failed validateCredentials() call must:
+     *  - throw AuthenticationException,
+     *  - NOT establish a session (Auth::check() stays false),
+     *  - trigger LockUserCounterMeasure so the user's login_failed_attempt counter increments.
+     */
+    public function testFailedAttempt_incrementsLoginFailedAttemptCounter(): void
+    {
+        $initial_attempts = $this->getLoginFailedAttempt(self::SEEDED_USERNAME);
+        $this->assertFalse(Auth::check(), 'precondition: no authenticated user');
+
+        $threw = false;
+        try {
+            $this->auth_service->validateCredentials(self::SEEDED_USERNAME, 'wrong-password');
+        } catch (AuthenticationException $ex) {
+            $threw = true;
+        }
+
+        $this->assertTrue($threw, 'Expected AuthenticationException on wrong password');
+        $this->assertFalse(Auth::check(), 'No session should be established after a failed attempt');
+
+        $new_attempts = $this->getLoginFailedAttempt(self::SEEDED_USERNAME);
+        $this->assertSame(
+            $initial_attempts + 1,
+            $new_attempts,
+            'login_failed_attempt counter must increment via LockUserCounterMeasure'
+        );
+    }
+
+    /**
+     * A successful validateCredentials() call must return the user without
+     * establishing a session — Auth::check() must remain false afterwards.
+     */
+    public function testSuccessfulValidation_doesNotEstablishSession(): void
+    {
+        $this->assertFalse(Auth::check(), 'precondition: no authenticated user');
+
+        $user = $this->auth_service->validateCredentials(
+            self::SEEDED_USERNAME,
+            self::SEEDED_PASSWORD
+        );
+
+        $this->assertInstanceOf(User::class, $user);
+        $this->assertFalse(
+            Auth::check(),
+            'validateCredentials() must NOT call Auth::login() on success'
+        );
+    }
+
+    private function getLoginFailedAttempt(string $username): int
+    {
+        // Clear Doctrine's identity map so we read fresh state from the DB,
+        // not a cached in-memory entity from a prior transaction.
+        EntityManager::clear();
+        $repo = EntityManager::getRepository(User::class);
+        /** @var IUserRepository $repo */
+        $user = $repo->getByEmailOrName($username);
+        $this->assertInstanceOf(User::class, $user, "Seeded user {$username} not found");
+        return $user->getLoginFailedAttempt();
+    }
+}
diff --git a/tests/unit/AuthServiceValidateCredentialsTest.php b/tests/unit/AuthServiceValidateCredentialsTest.php
