From cf45fd62de5b25c855e1df6f63c810e2ef6efa42 Mon Sep 17 00:00:00 2001 From: Marco Raddatz Date: Tue, 17 Sep 2024 12:31:15 +0200 Subject: [PATCH] bypass superpin environment --- config/totp-login.php | 42 +++++++++---- src/Events/LoggedInViaTotp.php | 4 +- src/Events/LoginRequestViaTotp.php | 4 +- src/Exceptions/MissingCode.php | 4 +- src/Exceptions/MissingSessionInformation.php | 4 +- src/Jobs/CreateAndSendLoginCode.php | 4 +- src/Jobs/ResetLoginCode.php | 4 +- src/Notifications/LoginCode.php | 4 +- src/Requests/CodeRequest.php | 18 +++++- .../Controllers/HandleCodeRequestTest.php | 62 ++++++++++++++++++- tests/Unit/HandleCodeRequestTest.php | 45 ++++++++++++++ 11 files changed, 159 insertions(+), 36 deletions(-) create mode 100644 tests/Unit/HandleCodeRequestTest.php diff --git a/config/totp-login.php b/config/totp-login.php index 4c0becc..62197e3 100644 --- a/config/totp-login.php +++ b/config/totp-login.php @@ -13,7 +13,7 @@ */ 'notification' => \Empuxa\TotpLogin\Notifications\LoginCode::class, - 'columns' => [ + 'columns' => [ /** * The main identifier of the user model. * We will use this column to authenticate the user and to send the PIN to. @@ -34,7 +34,7 @@ 'code_valid_until' => 'login_totp_code_valid_until', ], - 'route' => [ + 'route' => [ /** * The middleware to use for the route. * Default: ['web', 'guest'] @@ -48,7 +48,7 @@ 'prefix' => 'login', ], - 'identifier' => [ + 'identifier' => [ /** * The maximum number of attempts to get the user per minute. * Afterward, the user gets blocked for 60 seconds. @@ -71,7 +71,7 @@ 'enable_throttling' => true, ], - 'code' => [ + 'code' => [ /** * The length of the PIN. * Keep in mind that longer PINs might break the layout. @@ -108,21 +108,37 @@ 'enable_throttling' => true, ], - /** - * Enable the "superpin" feature. - * When enabled, any user can also sign in with the PIN of your choice on non-production environments. - * Set the environment variable `TOTP_LOGIN_SUPERPIN` to the PIN you want to use. - * Default: env('TOTP_LOGIN_SUPERPIN', false) - */ - 'superpin' => env('TOTP_LOGIN_SUPERPIN', false), + 'superpin' => [ + /** + * Enable the "superpin" feature. + * When enabled, any user can also sign in with the PIN of your choice. + * Set the environment variable `TOTP_LOGIN_SUPERPIN` to the PIN you want to use. + * Default: env('TOTP_LOGIN_SUPERPIN', false) + */ + 'pin' => env('TOTP_LOGIN_SUPERPIN', false), + + /** + * The environments where the superpin is allowed. + * This is an extra security layer to prevent the superpin from being used in production. + * Default: ['local', 'testing'] + */ + 'environments' => ['local', 'testing'], + + /** + * The identifiers that can bypass the environment check. + * This is useful for testing the superpin in production or providing test accounts to vendors. + * Default: [] + */ + 'bypassing_identifiers' => [], + ], /** * The redirect path after a successful login. * Default: '/' */ - 'redirect' => '/', + 'redirect' => '/', - 'events' => [ + 'events' => [ /** * This event is fired when a user submits a TOTP. * Default: \Empuxa\TotpLogin\Events\PinRequested::class diff --git a/src/Events/LoggedInViaTotp.php b/src/Events/LoggedInViaTotp.php index f658e41..00855fc 100644 --- a/src/Events/LoggedInViaTotp.php +++ b/src/Events/LoggedInViaTotp.php @@ -12,7 +12,5 @@ class LoggedInViaTotp use InteractsWithSockets; use SerializesModels; - public function __construct(public $user, public $request) - { - } + public function __construct(public $user, public $request) {} } diff --git a/src/Events/LoginRequestViaTotp.php b/src/Events/LoginRequestViaTotp.php index 16a5e68..d1670ae 100644 --- a/src/Events/LoginRequestViaTotp.php +++ b/src/Events/LoginRequestViaTotp.php @@ -12,7 +12,5 @@ class LoginRequestViaTotp use InteractsWithSockets; use SerializesModels; - public function __construct(public $user, public $request) - { - } + public function __construct(public $user, public $request) {} } diff --git a/src/Exceptions/MissingCode.php b/src/Exceptions/MissingCode.php index 8cf61d5..742362e 100644 --- a/src/Exceptions/MissingCode.php +++ b/src/Exceptions/MissingCode.php @@ -4,6 +4,4 @@ use Exception; -class MissingCode extends Exception -{ -} +class MissingCode extends Exception {} diff --git a/src/Exceptions/MissingSessionInformation.php b/src/Exceptions/MissingSessionInformation.php index 18d7ea9..bbe8139 100644 --- a/src/Exceptions/MissingSessionInformation.php +++ b/src/Exceptions/MissingSessionInformation.php @@ -4,6 +4,4 @@ use Exception; -class MissingSessionInformation extends Exception -{ -} +class MissingSessionInformation extends Exception {} diff --git a/src/Jobs/CreateAndSendLoginCode.php b/src/Jobs/CreateAndSendLoginCode.php index 51f2a39..41b4ea5 100644 --- a/src/Jobs/CreateAndSendLoginCode.php +++ b/src/Jobs/CreateAndSendLoginCode.php @@ -11,9 +11,7 @@ class CreateAndSendLoginCode use Dispatchable; use SerializesModels; - public function __construct(public $user, public readonly string $ip = '') - { - } + public function __construct(public $user, public readonly string $ip = '') {} /** * @throws \Exception diff --git a/src/Jobs/ResetLoginCode.php b/src/Jobs/ResetLoginCode.php index a6ac665..56acb77 100644 --- a/src/Jobs/ResetLoginCode.php +++ b/src/Jobs/ResetLoginCode.php @@ -10,9 +10,7 @@ class ResetLoginCode use Dispatchable; use SerializesModels; - public function __construct(public $user) - { - } + public function __construct(public $user) {} /** * @throws \Exception diff --git a/src/Notifications/LoginCode.php b/src/Notifications/LoginCode.php index a509369..0b39294 100644 --- a/src/Notifications/LoginCode.php +++ b/src/Notifications/LoginCode.php @@ -10,9 +10,7 @@ */ class LoginCode extends Notification { - public function __construct(protected readonly string $code, protected readonly string $ip) - { - } + public function __construct(protected readonly string $code, protected readonly string $ip) {} /** * @param array $notifiable diff --git a/src/Requests/CodeRequest.php b/src/Requests/CodeRequest.php index 2ba36a8..7389670 100644 --- a/src/Requests/CodeRequest.php +++ b/src/Requests/CodeRequest.php @@ -102,6 +102,16 @@ public function ensureCodeIsNotExpired(): void ]); } + public static function runsOnAllowedEnvironment(string $environment): bool + { + return in_array($environment, config('totp-login.superpin.environments'), true); + } + + public static function bypassesEnvironment(string $identifier): bool + { + return in_array($identifier, config('totp-login.superpin.bypassing_identifiers'), true); + } + /** * @throws \Illuminate\Validation\ValidationException */ @@ -109,7 +119,13 @@ public function validateCode(): void { $this->formatCode(); - if ($this->formattedCode === (string) config('totp-login.superpin') && ! app()->isProduction()) { + $codeMatchesSuperPin = $this->formattedCode === (string) config('totp-login.superpin.pin'); + + if ($codeMatchesSuperPin && self::runsOnAllowedEnvironment(app()->environment())) { + return; + } + + if ($codeMatchesSuperPin && self::bypassesEnvironment($this->user->{config('totp-login.columns.identifier')})) { return; } diff --git a/tests/Feature/Controllers/HandleCodeRequestTest.php b/tests/Feature/Controllers/HandleCodeRequestTest.php index caccb0a..256b062 100644 --- a/tests/Feature/Controllers/HandleCodeRequestTest.php +++ b/tests/Feature/Controllers/HandleCodeRequestTest.php @@ -198,7 +198,7 @@ public function test_can_login_with_superpin(): void { Notification::fake(); - Config::set('totp-login.superpin', 333333); + Config::set('totp-login.superpin.pin', 333333); $user = $this->createUser([ config('totp-login.columns.code_valid_until') => now()->addMinutes(10), @@ -220,4 +220,64 @@ public function test_can_login_with_superpin(): void Notification::assertNothingSent(); } + + public function test_cannot_login_with_superpin_on_wrong_environment(): void + { + Notification::fake(); + + Config::set('totp-login.superpin.pin', 333333); + Config::set('totp-login.superpin.environments', ['production']); + + $user = $this->createUser([ + config('totp-login.columns.code_valid_until') => now()->addMinutes(10), + ]); + + $response = $this + ->withSession([ + config('totp-login.columns.identifier') => $user->{config('totp-login.columns.identifier')}, + ]) + ->post(route('totp-login.code.handle'), [ + 'code' => [3, 3, 3, 3, 3, 3], + ]); + + $response->assertRedirect(); + + $response->assertSessionHasErrors('code', __('controllers/session.store.error.totp_wrong', [ + 'attempts_left' => config('totp-login.code.max_attempts') - 1, + ])); + + $this->assertGuest(); + + Notification::assertNothingSent(); + } + + public function test_can_login_with_superpin_on_wrong_environment_with_bypassing_identifier(): void + { + Notification::fake(); + + Config::set('totp-login.superpin.pin', 333333); + Config::set('totp-login.superpin.environments', ['production']); + Config::set('totp-login.superpin.bypassing_identifiers', ['test@example.com']); + + $user = $this->createUser([ + config('totp-login.columns.identifier') => 'test@example.com', + config('totp-login.columns.code_valid_until') => now()->addMinutes(10), + ]); + + $response = $this + ->withSession([ + config('totp-login.columns.identifier') => $user->{config('totp-login.columns.identifier')}, + ]) + ->post(route('totp-login.code.handle'), [ + 'code' => [3, 3, 3, 3, 3, 3], + ]); + + $response->assertSessionHasNoErrors(); + + $response->assertRedirect(config('totp-login.redirect')); + + $this->assertAuthenticatedAs($user); + + Notification::assertNothingSent(); + } } diff --git a/tests/Unit/HandleCodeRequestTest.php b/tests/Unit/HandleCodeRequestTest.php new file mode 100644 index 0000000..9c085a6 --- /dev/null +++ b/tests/Unit/HandleCodeRequestTest.php @@ -0,0 +1,45 @@ + true, + 'prod*' => false, + 'staging' => false, + 'testing' => false, + 'local' => false, + ]; + + foreach ($data as $environment => $expected) { + $this->assertEquals($expected, CodeRequest::runsOnAllowedEnvironment($environment), $environment); + } + } + + public function test_bypasses_environment(): void + { + Config::set('totp-login.superpin.pin', 333333); + Config::set('totp-login.superpin.environments', ['non-existing']); + Config::set('totp-login.superpin.bypassing_identifiers', ['test@example.com']); + + $data = [ + 'test@example.com' => true, + 'test@*' => false, + 'test2@example.com' => false, + ]; + + foreach ($data as $email => $expected) { + $this->assertEquals($expected, CodeRequest::bypassesEnvironment($email), $email); + } + } +}