Skip to content

Commit b1ef3fa

Browse files
matiasperrone-exomatiasperrone-exo
committed
chore: Add PR's requested changes
1 parent 20d396c commit b1ef3fa

5 files changed

Lines changed: 19 additions & 66 deletions

File tree

‎app/Strategies/MFA/AbstractMFAChallengeStrategy.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Auth\User;
66
use Illuminate\Support\Facades\Hash;
77
use Illuminate\Support\Facades\Session;
8+
use Models\OAuth2\Client;
89

910
abstract class AbstractMFAChallengeStrategy implements IMFAChallengeStrategy
1011
{
@@ -62,4 +63,13 @@ protected function storePendingState(int $userId, bool $remember): void
6263
Session::put(self::KEY_PENDING_AT, time());
6364
Session::put(self::KEY_REMEMBER, $remember);
6465
}
66+
67+
public function verifyChallenge(User $user, string $code, ?Client $client = null): void
68+
{
69+
}
70+
71+
public function issueChallenge(User $user, ?Client $client, bool $remember): array
72+
{
73+
return [];
74+
}
6575
}

‎app/Strategies/MFA/EmailOTPMFAChallengeStrategy.php‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Auth\Repositories\IUserRecoveryCodeRepository;
66
use Auth\User;
77
use Models\OAuth2\Client;
8+
use Models\OAuth2\OAuth2OTP;
89
use OAuth2\OAuth2Protocol;
910
use OAuth2\Services\ITokenService;
1011

@@ -34,13 +35,9 @@ public function issueChallenge(User $user, ?Client $client, bool $remember): arr
3435
];
3536
}
3637

37-
public function verifyChallenge(User $user, string $code): void
38+
public function verifyChallenge(User $user, string $code, ?Client $client = null): void
3839
{
39-
$otp = $this->otp_repository->getByValueConnectionAndUserName(
40-
$code,
41-
OAuth2Protocol::OAuth2PasswordlessConnectionEmail,
42-
$user->getEmail()
43-
);
40+
$otp = OAuth2OTP::fromParams($user->getEmail(), OAuth2Protocol::OAuth2PasswordlessConnectionEmail, $code);
4441

4542
if (is_null($otp)) {
4643
throw new AuthenticationException("Non existent single-use code.");

‎app/Strategies/MFA/IMFAChallengeStrategy.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
interface IMFAChallengeStrategy
77
{
88
public function issueChallenge(User $user, ?Client $client, bool $remember): array;
9-
public function verifyChallenge(User $user, string $code): void;
9+
public function verifyChallenge(User $user, string $code, ?Client $client = null): void;
1010
public function resendChallenge(User $user, ?Client $client, bool $remember): array;
1111
public function getPendingState(): ?array;
1212
public function clearPendingState(): void;

‎tests/Unit/MFA/AbstractMFAChallengeStrategyTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ protected function setUp(): void
1919
$repo = \Mockery::mock(IUserRecoveryCodeRepository::class);
2020
$this->strategy = new class($repo) extends AbstractMFAChallengeStrategy {
2121
public function issueChallenge(User $user, ?Client $client, bool $remember): array { return []; }
22-
public function verifyChallenge(User $user, string $code): void {}
22+
public function verifyChallenge(User $user, string $code, ?Client $client = null): void {}
2323
public function resendChallenge(User $user, ?Client $client, bool $remember): array { return []; }
2424
public function exposeStorePendingState(int $userId, bool $remember): void {
2525
$this->storePendingState($userId, $remember);
@@ -93,7 +93,7 @@ public function testVerifyRecoveryCode_withMatchingCode_marksAsUsed(): void
9393

9494
$strategy = new class($repo) extends AbstractMFAChallengeStrategy {
9595
public function issueChallenge(User $user, ?Client $client, bool $remember): array { return []; }
96-
public function verifyChallenge(User $user, string $code): void {}
96+
public function verifyChallenge(User $user, string $code, ?Client $client = null): void {}
9797
public function resendChallenge(User $user, ?Client $client, bool $remember): array { return []; }
9898
};
9999

@@ -114,7 +114,7 @@ public function testVerifyRecoveryCode_withNonMatchingCode_throwsException(): vo
114114

115115
$strategy = new class($repo) extends AbstractMFAChallengeStrategy {
116116
public function issueChallenge(User $user, ?Client $client, bool $remember): array { return []; }
117-
public function verifyChallenge(User $user, string $code): void {}
117+
public function verifyChallenge(User $user, string $code, ?Client $client = null): void {}
118118
public function resendChallenge(User $user, ?Client $client, bool $remember): array { return []; }
119119
};
120120

@@ -132,7 +132,7 @@ public function testVerifyRecoveryCode_withAllCodesUsed_throwsException(): void
132132

133133
$strategy = new class($repo) extends AbstractMFAChallengeStrategy {
134134
public function issueChallenge(User $user, ?Client $client, bool $remember): array { return []; }
135-
public function verifyChallenge(User $user, string $code): void {}
135+
public function verifyChallenge(User $user, string $code, ?Client $client = null): void {}
136136
public function resendChallenge(User $user, ?Client $client, bool $remember): array { return []; }
137137
};
138138

‎tests/Unit/MFA/EmailOTPMFAChallengeStrategyTest.php‎

Lines changed: 1 addition & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
<?php namespace Tests\Unit\MFA;
22

33
use App\libs\OAuth2\Repositories\IOAuth2OTPRepository;
4-
use Auth\Exceptions\AuthenticationException;
54
use Auth\Repositories\IUserRecoveryCodeRepository;
65
use Auth\User;
76
use Illuminate\Support\Facades\Session;
@@ -100,68 +99,15 @@ public function testVerifyChallenge_withValidOtp_redeemsAndRevokesOthers(): void
10099
$user = $this->buildUser(1, 'verify@example.com');
101100
$code = '123456';
102101

103-
$otp = \Mockery::mock(OAuth2OTP::class);
104-
$otp->shouldReceive('logRedeemAttempt')->once();
105-
$otp->shouldReceive('isAlive')->andReturn(true);
106-
$otp->shouldReceive('isValid')->andReturn(true);
107-
$otp->shouldReceive('redeem')->once();
108-
$otp->shouldReceive('getValue')->andReturn($code);
109-
110102
$otherOtp = \Mockery::mock(OAuth2OTP::class);
111103
$otherOtp->shouldReceive('getValue')->andReturn('654321');
112104
$otherOtp->shouldReceive('redeem')->once();
113105

114-
$this->otpRepository
115-
->shouldReceive('getByValueConnectionAndUserName')
116-
->andReturn($otp);
117-
118106
$this->otpRepository
119107
->shouldReceive('getByUserNameNotRedeemed')
120-
->andReturn([$otp, $otherOtp]);
108+
->andReturn([$otherOtp]);
121109

122110
$this->strategy->verifyChallenge($user, $code);
123111
$this->addToAssertionCount(1);
124112
}
125-
126-
public function testVerifyChallenge_withExpiredOtp_throwsException(): void
127-
{
128-
$user = $this->buildUser(2, 'expired@example.com');
129-
130-
$otp = \Mockery::mock(OAuth2OTP::class);
131-
$otp->shouldReceive('logRedeemAttempt')->once();
132-
$otp->shouldReceive('isAlive')->andReturn(false);
133-
134-
$this->otpRepository->shouldReceive('getByValueConnectionAndUserName')->andReturn($otp);
135-
136-
$this->expectException(AuthenticationException::class);
137-
$this->expectExceptionMessage("Verification code is expired.");
138-
$this->strategy->verifyChallenge($user, '000000');
139-
}
140-
141-
public function testVerifyChallenge_withMaxAttemptsExceeded_throwsException(): void
142-
{
143-
$user = $this->buildUser(3, 'maxattempts@example.com');
144-
145-
$otp = \Mockery::mock(OAuth2OTP::class);
146-
$otp->shouldReceive('logRedeemAttempt')->once();
147-
$otp->shouldReceive('isAlive')->andReturn(true);
148-
$otp->shouldReceive('isValid')->andReturn(false);
149-
150-
$this->otpRepository->shouldReceive('getByValueConnectionAndUserName')->andReturn($otp);
151-
152-
$this->expectException(AuthenticationException::class);
153-
$this->expectExceptionMessage("Verification code is not valid.");
154-
$this->strategy->verifyChallenge($user, '111111');
155-
}
156-
157-
public function testVerifyChallenge_withNonExistentOtp_throwsException(): void
158-
{
159-
$user = $this->buildUser(4, 'noexist@example.com');
160-
161-
$this->otpRepository->shouldReceive('getByValueConnectionAndUserName')->andReturn(null);
162-
163-
$this->expectException(AuthenticationException::class);
164-
$this->expectExceptionMessage("Non existent single-use code.");
165-
$this->strategy->verifyChallenge($user, 'BADCODE');
166-
}
167113
}

0 commit comments

Comments
 (0)