Skip to content

Commit 80972f0

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

1 file changed

Lines changed: 47 additions & 66 deletions

File tree

‎app/libs/Auth/AuthService.php‎

Lines changed: 47 additions & 66 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
(
@@ -185,28 +183,13 @@ 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-
);
195-
}
196-
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-
);
186+
/**
187+
* @var User|null $user
188+
*/
189+
$user = $this->user_repository->getByEmailOrName($username);
190+
$valid = Auth::getProvider()->validateCredentials($user, ['username' => $username, 'password' => $password]);
191+
if (!$valid) {
192+
throw new AuthenticationException();
210193
}
211194

212195
return $user;
@@ -220,6 +203,7 @@ public function validateCredentials(string $username, string $password): User
220203
public function loginUser(User $user, bool $remember): void
221204
{
222205
Log::debug("AuthService::loginUser");
206+
if (!$user->isActive() || !$user->canLogin()) throw new AuthenticationException("User is not active or cannot login.");
223207
Auth::login($user, $remember);
224208
}
225209

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

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

284-
if (
285-
($otp->hasClient() && is_null($client)) ||
286-
($otp->hasClient() && !is_null($client) && $client->getClientId() != $otp->getClient()->getClientId())
287-
) {
268+
if (($otp->hasClient() && is_null($client)) ||
269+
($otp->hasClient() && !is_null($client) && $client->getClientId() != $otp->getClient()->getClientId())) {
288270
throw new AuthenticationException("Single-use code audience mismatch.");
289271
}
290272

@@ -306,16 +288,17 @@ public function loginWithOTP(OAuth2OTP $otpClaim, ?Client $client = null, bool $
306288
],
307289
$otp
308290
);
309-
} else {
310-
if ($user->isActive()) {
291+
}
292+
else{
293+
if($user->isActive()) {
311294
// verify email
312295
$user->verifyEmail(false);
313296
}
314297
}
315298

316-
if (!$user->canLogin()) {
299+
if(!$user->canLogin()){
317300
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.");
301+
throw new AuthenticationException("We are sorry, your username or password does not match an existing record.");
319302
}
320303

321304
$otp->setAuthTime(time());
@@ -329,7 +312,7 @@ public function loginWithOTP(OAuth2OTP $otpClaim, ?Client $client = null, bool $
329312
$client
330313
);
331314

332-
foreach ($grants2Revoke as $otp2Revoke) {
315+
foreach ($grants2Revoke as $otp2Revoke){
333316
try {
334317
Log::debug(sprintf("AuthService::loginWithOTP revoking otp %s ", $otp2Revoke->getValue()));
335318
if ($otp2Revoke->getValue() !== $otpClaim->getValue())
@@ -350,12 +333,12 @@ public function loginWithOTP(OAuth2OTP $otpClaim, ?Client $client = null, bool $
350333
* @param bool $clear_security_ctx
351334
* @return void
352335
*/
353-
public function logout(bool $clear_security_ctx = true): void
336+
public function logout(bool $clear_security_ctx = true):void
354337
{
355338
Log::debug("AuthService::logout");
356339
$current_user = $this->getCurrentUser();
357340
// check if we have user on session
358-
if (!is_null($current_user)) {
341+
if(!is_null($current_user)) {
359342
$ip = IPHelper::getUserIp();
360343
Log::debug(sprintf("AuthService::logout we have user %s from ip %s", $current_user->getId(), $ip));
361344
$this->user_action_service->addUserAction
@@ -369,7 +352,7 @@ public function logout(bool $clear_security_ctx = true): void
369352
// regular flow
370353
$this->invalidateSession();
371354
$this->principal_service->clear();
372-
if ($clear_security_ctx)
355+
if($clear_security_ctx)
373356
$this->security_context_service->clear();
374357
Auth::logout();
375358
// put in past
@@ -489,7 +472,8 @@ public function unwrapUserId(string $user_id): string
489472
$unwrapped_name = $this->decrypt($user_id);
490473
$parts = explode(':', $unwrapped_name);
491474
return intval($parts[1]);
492-
} catch (Exception $ex) {
475+
}
476+
catch (Exception $ex){
493477
Log::warning($ex);
494478
}
495479
return $user_id;
@@ -552,8 +536,7 @@ public function registerRPLogin(string $client_id): void
552536
$rps = $zlib->uncompress($rps);
553537
$rps .= '|';
554538
}
555-
if (is_null($rps))
556-
$rps = "";
539+
if (is_null($rps)) $rps = "";
557540

558541
if (!str_contains($rps, $client_id))
559542
$rps .= $client_id;
@@ -592,7 +575,8 @@ public function getLoggedRPs(): array
592575
$rps = $zlib->uncompress($rps);
593576
return explode('|', $rps);
594577
}
595-
} catch (Exception $ex) {
578+
}
579+
catch (Exception $ex){
596580
Log::warning($ex);
597581
}
598582
return [];
@@ -659,17 +643,14 @@ public function invalidateSession(): void
659643
public function postLoginUserActions(int $user_id): void
660644
{
661645
Log::debug(sprintf("AuthService::postLoginUserActions user %s", $user_id));
662-
$this->tx_service->transaction(function () use ($user_id) {
646+
$this->tx_service->transaction(function () use($user_id){
663647
$user = $this->user_repository->getById($user_id);
664-
if (!$user instanceof User)
665-
return;
648+
if(!$user instanceof User) return;
666649

667650
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-
);
651+
Log::warning(sprintf("AuthService::postLoginUserActions user %s is not active.", $user_id));
652+
throw new AuthenticationLockedUserLoginAttempt($user->getEmail(),
653+
sprintf("User %s is locked.", $user->getEmail()));
673654
}
674655

675656
//update user fields

0 commit comments

Comments
 (0)