Skip to content

Commit 5f3c28e

Browse files
matiasperrone-exomatiasperrone-exo
committed
chore: Add PR's requested changes
1 parent 9fc38a0 commit 5f3c28e

2 files changed

Lines changed: 79 additions & 157 deletions

File tree

‎app/libs/Auth/AuthService.php‎

Lines changed: 49 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
<?php
2-
namespace Auth;
1+
<?php namespace Auth;
32
/**
43
* Copyright 2016 OpenStack Foundation
54
* Licensed under the Apache License, Version 2.0 (the "License");
@@ -97,19 +96,21 @@ final class AuthService extends AbstractService implements IAuthService
9796
* @param IAuthUserService $auth_user_service
9897
* @param ISecurityContextService $security_context_service
9998
* @param ITransactionService $tx_service
99+
* @params ISecurityContextService $security_context_service
100100
*/
101101
public function __construct
102102
(
103-
IUserRepository $user_repository,
104-
IOAuth2OTPRepository $otp_repository,
105-
IPrincipalService $principal_service,
106-
IUserService $user_service,
107-
IUserActionService $user_action_service,
108-
ICacheService $cache_service,
109-
IAuthUserService $auth_user_service,
103+
IUserRepository $user_repository,
104+
IOAuth2OTPRepository $otp_repository,
105+
IPrincipalService $principal_service,
106+
IUserService $user_service,
107+
IUserActionService $user_action_service,
108+
ICacheService $cache_service,
109+
IAuthUserService $auth_user_service,
110110
ISecurityContextService $security_context_service,
111-
ITransactionService $tx_service
112-
) {
111+
ITransactionService $tx_service
112+
)
113+
{
113114
parent::__construct($tx_service);
114115
$this->user_repository = $user_repository;
115116
$this->principal_service = $principal_service;
@@ -134,11 +135,7 @@ public function isUserLogged()
134135
*/
135136
public function getCurrentUser(): ?User
136137
{
137-
$user = Auth::user();
138-
if ($user instanceof User) {
139-
return $user;
140-
}
141-
return null;
138+
return Auth::user();
142139
}
143140

144141
/**
@@ -152,19 +149,20 @@ public function login(string $username, string $password, bool $remember_me): bo
152149
{
153150
Log::debug("AuthService::login");
154151

152+
$this->last_login_error = "";
155153
if (!Auth::attempt(['username' => $username, 'password' => $password], $remember_me)) {
156154
throw new AuthenticationException
157155
(
158-
"username or password does not match an existing record."
156+
"We are sorry, your username or password does not match an existing record."
159157
);
160158
}
161159
Log::debug("AuthService::login: clearing principal");
162160
$this->principal_service->clear();
163161
$current_user = $this->getCurrentUser();
164-
if (is_null($current_user) || !$current_user->canLogin())
162+
if(is_null($current_user) || !$current_user->canLogin())
165163
throw new AuthenticationException
166164
(
167-
"username or password does not match an existing record."
165+
"We are sorry, your username or password does not match an existing record."
168166
);
169167
$this->principal_service->register
170168
(
@@ -178,35 +176,23 @@ public function login(string $username, string $password, bool $remember_me): bo
178176
/**
179177
* @param string $username
180178
* @param string $password
181-
* @return User
179+
* @return User|null
182180
* @throws AuthenticationException
183181
*/
184182
public function validateCredentials(string $username, string $password): User
185183
{
186184
Log::debug("AuthService::validateCredentials");
187185

188-
// retrieveByCredentials swallows AuthenticationLockedUserLoginAttempt and returns null,
189-
// so pre-check lock state here to surface a distinct message for locked accounts.
190-
$existing = $this->user_repository->getByEmailOrName($username);
191-
if (!is_null($existing) && !$existing->isActive()) {
192-
throw new AuthenticationException(
193-
sprintf("User %s is locked.", $username)
194-
);
186+
/**
187+
* @var User|null $user
188+
*/
189+
$user = Auth::getProvider()->retrieveByCredentials(['username' => $username, 'password' => $password]);
190+
if (!$user) {
191+
throw new AuthenticationException();
195192
}
196193

197-
// Known cost: retrieveByCredentials() calls user_repository->getByEmailOrName() internally
198-
// (CustomAuthProvider line ~122), duplicating the query above. Eliminating it would require
199-
// either changing the provider API to accept a pre-fetched User, or moving
200-
// LockUserCounterMeasure checkpoint logic out of the provider — both out of scope here.
201-
$user = Auth::getProvider()->retrieveByCredentials([
202-
'username' => $username,
203-
'password' => $password,
204-
]);
205-
206-
if (is_null($user) || !$user instanceof User || !$user->canLogin()) {
207-
throw new AuthenticationException(
208-
"username or password does not match an existing record."
209-
);
194+
if (!$user->isActive()) {
195+
throw new AuthenticationException("User is locked.");
210196
}
211197

212198
return $user;
@@ -220,6 +206,7 @@ public function validateCredentials(string $username, string $password): User
220206
public function loginUser(User $user, bool $remember): void
221207
{
222208
Log::debug("AuthService::loginUser");
209+
if (!$user->canLogin()) throw new AuthenticationException("User is not active or cannot login.");
223210
Auth::login($user, $remember);
224211
}
225212

@@ -278,13 +265,11 @@ public function loginWithOTP(OAuth2OTP $otpClaim, ?Client $client = null, bool $
278265
throw new AuthenticationException("Single-use code mismatch.");
279266
}
280267

281-
if (!empty($otpClaim->getScope()) && !$otp->allowScope($otpClaim->getScope()))
268+
if(!empty($otpClaim->getScope()) && !$otp->allowScope($otpClaim->getScope()))
282269
throw new InvalidOTPException("Single-use code requested scopes escalates former scopes.");
283270

284-
if (
285-
($otp->hasClient() && is_null($client)) ||
286-
($otp->hasClient() && !is_null($client) && $client->getClientId() != $otp->getClient()->getClientId())
287-
) {
271+
if (($otp->hasClient() && is_null($client)) ||
272+
($otp->hasClient() && !is_null($client) && $client->getClientId() != $otp->getClient()->getClientId())) {
288273
throw new AuthenticationException("Single-use code audience mismatch.");
289274
}
290275

@@ -306,16 +291,17 @@ public function loginWithOTP(OAuth2OTP $otpClaim, ?Client $client = null, bool $
306291
],
307292
$otp
308293
);
309-
} else {
310-
if ($user->isActive()) {
294+
}
295+
else{
296+
if($user->isActive()) {
311297
// verify email
312298
$user->verifyEmail(false);
313299
}
314300
}
315301

316-
if (!$user->canLogin()) {
302+
if(!$user->canLogin()){
317303
Log::warning(sprintf("AuthService::loginWithOTP user %s cannot login ( is not active ).", $user->getId()));
318-
throw new AuthenticationException("username or password does not match an existing record.");
304+
throw new AuthenticationException("We are sorry, your username or password does not match an existing record.");
319305
}
320306

321307
$otp->setAuthTime(time());
@@ -329,7 +315,7 @@ public function loginWithOTP(OAuth2OTP $otpClaim, ?Client $client = null, bool $
329315
$client
330316
);
331317

332-
foreach ($grants2Revoke as $otp2Revoke) {
318+
foreach ($grants2Revoke as $otp2Revoke){
333319
try {
334320
Log::debug(sprintf("AuthService::loginWithOTP revoking otp %s ", $otp2Revoke->getValue()));
335321
if ($otp2Revoke->getValue() !== $otpClaim->getValue())
@@ -350,12 +336,12 @@ public function loginWithOTP(OAuth2OTP $otpClaim, ?Client $client = null, bool $
350336
* @param bool $clear_security_ctx
351337
* @return void
352338
*/
353-
public function logout(bool $clear_security_ctx = true): void
339+
public function logout(bool $clear_security_ctx = true):void
354340
{
355341
Log::debug("AuthService::logout");
356342
$current_user = $this->getCurrentUser();
357343
// check if we have user on session
358-
if (!is_null($current_user)) {
344+
if(!is_null($current_user)) {
359345
$ip = IPHelper::getUserIp();
360346
Log::debug(sprintf("AuthService::logout we have user %s from ip %s", $current_user->getId(), $ip));
361347
$this->user_action_service->addUserAction
@@ -369,7 +355,7 @@ public function logout(bool $clear_security_ctx = true): void
369355
// regular flow
370356
$this->invalidateSession();
371357
$this->principal_service->clear();
372-
if ($clear_security_ctx)
358+
if($clear_security_ctx)
373359
$this->security_context_service->clear();
374360
Auth::logout();
375361
// put in past
@@ -489,7 +475,8 @@ public function unwrapUserId(string $user_id): string
489475
$unwrapped_name = $this->decrypt($user_id);
490476
$parts = explode(':', $unwrapped_name);
491477
return intval($parts[1]);
492-
} catch (Exception $ex) {
478+
}
479+
catch (Exception $ex){
493480
Log::warning($ex);
494481
}
495482
return $user_id;
@@ -552,8 +539,7 @@ public function registerRPLogin(string $client_id): void
552539
$rps = $zlib->uncompress($rps);
553540
$rps .= '|';
554541
}
555-
if (is_null($rps))
556-
$rps = "";
542+
if (is_null($rps)) $rps = "";
557543

558544
if (!str_contains($rps, $client_id))
559545
$rps .= $client_id;
@@ -592,7 +578,8 @@ public function getLoggedRPs(): array
592578
$rps = $zlib->uncompress($rps);
593579
return explode('|', $rps);
594580
}
595-
} catch (Exception $ex) {
581+
}
582+
catch (Exception $ex){
596583
Log::warning($ex);
597584
}
598585
return [];
@@ -659,17 +646,14 @@ public function invalidateSession(): void
659646
public function postLoginUserActions(int $user_id): void
660647
{
661648
Log::debug(sprintf("AuthService::postLoginUserActions user %s", $user_id));
662-
$this->tx_service->transaction(function () use ($user_id) {
649+
$this->tx_service->transaction(function () use($user_id){
663650
$user = $this->user_repository->getById($user_id);
664-
if (!$user instanceof User)
665-
return;
651+
if(!$user instanceof User) return;
666652

667653
if (!$user->isActive()) {
668-
Log::warning(sprintf("AuthService::postLoginUserActions user %s is not active.", $user_id));
669-
throw new AuthenticationLockedUserLoginAttempt(
670-
$user->getEmail(),
671-
sprintf("User %s is locked.", $user->getEmail())
672-
);
654+
Log::warning(sprintf("AuthService::postLoginUserActions user %s is not active.", $user_id));
655+
throw new AuthenticationLockedUserLoginAttempt($user->getEmail(),
656+
sprintf("User %s is locked.", $user->getEmail()));
673657
}
674658

675659
//update user fields

0 commit comments

Comments
 (0)