diff --git a/config/areas/account/drawers.php b/config/areas/account/drawers.php index db81d6e1a4..1ca6763397 100644 --- a/config/areas/account/drawers.php +++ b/config/areas/account/drawers.php @@ -11,6 +11,10 @@ ...$drawers['user.security.method.code'], 'pattern' => '(account)/security/method/code', ], + 'account.security.challenge.email' => [ + ...$drawers['user.security.challenge.email'], + 'pattern' => '(account)/security/challenge/email', + ], 'account.security.challenge.totp' => [ ...$drawers['user.security.challenge.totp'], 'pattern' => '(account)/security/challenge/totp', diff --git a/config/areas/users/drawers.php b/config/areas/users/drawers.php index 468d8641b1..ff20967b11 100644 --- a/config/areas/users/drawers.php +++ b/config/areas/users/drawers.php @@ -2,6 +2,7 @@ use Kirby\Panel\Controller\Drawer\FieldDrawerController; use Kirby\Panel\Controller\Drawer\SectionDrawerController; +use Kirby\Panel\Controller\Drawer\UserEmailChallengeDrawerController; use Kirby\Panel\Controller\Drawer\UserSecurityCodeMethodDrawerController; use Kirby\Panel\Controller\Drawer\UserSecurityDrawerController; use Kirby\Panel\Controller\Drawer\UserTotpDrawerController; @@ -16,6 +17,10 @@ 'pattern' => 'users/(:any)/security/method/code', 'action' => UserSecurityCodeMethodDrawerController::class ], + 'user.security.challenge.email' => [ + 'pattern' => 'users/(:any)/security/challenge/email', + 'action' => UserEmailChallengeDrawerController::class + ], 'user.security.challenge.totp' => [ 'pattern' => 'users/(:any)/security/challenge/totp', 'action' => UserTotpDrawerController::class diff --git a/i18n/translations/en.json b/i18n/translations/en.json index 3322e22ade..181af0716b 100644 --- a/i18n/translations/en.json +++ b/i18n/translations/en.json @@ -552,6 +552,12 @@ "lock.isUnlocked": "Was unlocked by another user", "login": "Log in", + "login.challenge.email.description": "Send a one‑time code to your email address that is used as a second factor when signing into your account.", + "login.challenge.email.disable.confirm": "Do you really want to disable codes via email for {user}?", + "login.challenge.email.disable.label": "Disable codes via email", + "login.challenge.email.empty": "Codes via email not set up yet", + "login.challenge.email.enable.label": "Enable codes via email", + "login.challenge.email.help": "Enter the code just sent to your email inbox.", "login.challenge.email.label": "Code via email", "login.challenge.totp.label": "Authenticator app", "login.challenges.help": "Two-factor methods can add an additional layer of security by requiring more than just a password to sign in.", diff --git a/panel/src/components/Drawers/UserEmailChallengeDrawer.vue b/panel/src/components/Drawers/UserEmailChallengeDrawer.vue new file mode 100644 index 0000000000..51e8677eea --- /dev/null +++ b/panel/src/components/Drawers/UserEmailChallengeDrawer.vue @@ -0,0 +1,168 @@ + + + diff --git a/panel/src/components/Drawers/UserTotpDrawer.vue b/panel/src/components/Drawers/UserTotpDrawer.vue index cce60d4a90..bb5e54002a 100644 --- a/panel/src/components/Drawers/UserTotpDrawer.vue +++ b/panel/src/components/Drawers/UserTotpDrawer.vue @@ -7,7 +7,12 @@ @cancel="$emit('cancel')" @submit="$emit('cancel')" > -
+ @@ -149,6 +154,13 @@ export default { }, onSubmit: (password) => this.request("remove", { password }) }); + }, + onSubmit() { + if (this.isAccount === false) { + return; + } + + this.isEnabled ? this.disable() : this.create(); } } }; diff --git a/panel/src/components/Drawers/UserWebauthnDrawer.vue b/panel/src/components/Drawers/UserWebauthnDrawer.vue index 8e586f7623..a82090ff37 100644 --- a/panel/src/components/Drawers/UserWebauthnDrawer.vue +++ b/panel/src/components/Drawers/UserWebauthnDrawer.vue @@ -7,7 +7,12 @@ @cancel="$emit('cancel')" @submit="$emit('cancel')" > - + this.request("remove", { id, authorization }), (error) => this.$panel.notification.error(error) ); + }, + onSubmit() { + if (this.isAccount === true) { + this.create(); + } } } }; diff --git a/panel/src/components/Drawers/index.js b/panel/src/components/Drawers/index.js index 3b427b469e..5c594ec3d8 100644 --- a/panel/src/components/Drawers/index.js +++ b/panel/src/components/Drawers/index.js @@ -7,6 +7,7 @@ import StateDrawer from "./StateDrawer.vue"; import FormDrawer from "./FormDrawer.vue"; import StructureDrawer from "./StructureDrawer.vue"; import TextDrawer from "./TextDrawer.vue"; +import UserEmailChallengeDrawer from "./UserEmailChallengeDrawer.vue"; import UserSecurityDrawer from "./UserSecurityDrawer.vue"; import UserTotpDrawer from "./UserTotpDrawer.vue"; import UserWebauthnDrawer from "./UserWebauthnDrawer.vue"; @@ -22,6 +23,7 @@ export default { app.component("k-form-drawer", FormDrawer); app.component("k-structure-drawer", StructureDrawer); app.component("k-text-drawer", TextDrawer); + app.component("k-user-email-challenge-drawer", UserEmailChallengeDrawer); app.component("k-user-security-drawer", UserSecurityDrawer); app.component("k-user-totp-drawer", UserTotpDrawer); app.component("k-user-webauthn-drawer", UserWebauthnDrawer); diff --git a/src/Auth/Challenge/EmailChallenge.php b/src/Auth/Challenge/EmailChallenge.php index 112bcd318a..9547b638af 100644 --- a/src/Auth/Challenge/EmailChallenge.php +++ b/src/Auth/Challenge/EmailChallenge.php @@ -53,6 +53,26 @@ public static function icon(): string return 'email-unread'; } + /** + * As a second factor, the email challenge is opt-in per user, + * so that users are not forced into it just by having an email + * address. For every other purpose it stays always available. + */ + public static function isAvailable(User $user, string $mode): bool + { + if ($mode !== '2fa') { + return true; + } + + if ($user->secret('email') === true) { + return true; + } + + // enforced 2FA needs a factor for every user, so email stays + // the baseline for those who have not set up anything else + return $user->kirby()->auth()->methods()->hasAnyRequiring2FA(); + } + /** * Sends the email with the code to the user */ @@ -105,10 +125,9 @@ public static function settings(User $user): array { return [ new Button( - icon: static::icon(), - text: static::i18n('login.challenge.email.label'), - dialog: $user->panel()->url(true) . '/changeEmail', - disabled: !$user->permissions()->can('changeEmail') + icon: static::icon(), + text: static::i18n('login.challenge.email.label'), + drawer: $user->panel()->url(true) . '/security/challenge/email' ) ]; } diff --git a/src/Auth/Challenges.php b/src/Auth/Challenges.php index 1e8785eaa5..f5871bd899 100644 --- a/src/Auth/Challenges.php +++ b/src/Auth/Challenges.php @@ -192,6 +192,15 @@ public function get( ); } + /** + * Checks whether at least one challenge is available + * for the user and purpose/mode + */ + public function hasAvailable(User $user, string $mode): bool + { + return $this->available($user, $mode) !== []; + } + /** * Writes challenge state into the session */ diff --git a/src/Auth/Method.php b/src/Auth/Method.php index a4928d3158..18df1eaac8 100644 --- a/src/Auth/Method.php +++ b/src/Auth/Method.php @@ -69,16 +69,6 @@ public static function isEnabled( return true; } - /** - * Checks if this method uses challenges - */ - public static function isUsingChallenges( - Auth $auth, - array $options = [] - ): bool { - return false; - } - /** * Returns the config options for this method */ diff --git a/src/Auth/Method/BasicAuthMethod.php b/src/Auth/Method/BasicAuthMethod.php index 631baa0c48..dbbb666ea4 100644 --- a/src/Auth/Method/BasicAuthMethod.php +++ b/src/Auth/Method/BasicAuthMethod.php @@ -13,6 +13,11 @@ /** * HTTP basic authentication * + * Validates the same email + password credentials as `PasswordMethod`. + * As it cannot run a challenge, it defers the 2FA policy + * to `PasswordMethod::has2FA()` and rejects users that would be + * challenged there instead of skipping their second factor. + * * @copyright Bastian Allgeier * @license https://getkirby.com/license * @since 6.0.0 @@ -24,6 +29,7 @@ class BasicAuthMethod extends Method * so `$long` isn't relevant here * * @throws InvalidArgumentException If the password is missing + * @throws PermissionException If the user has a second factor set up */ public function authenticate( string|null $email, @@ -37,8 +43,23 @@ public function authenticate( ); } - return $this->auth->validatePassword($email, $password) + $user = $this->auth->validatePassword($email, $password) ?? throw new PermissionException(message: 'Invalid password'); // @codeCoverageIgnore + + /** + * @var PasswordMethod $method + */ + $method = $this->auth->methods()->get('password'); + + // users without a second factor (e.g. dedicated + // API accounts) can keep using basic auth + if ($method->has2FA($user) === true) { + throw new PermissionException( + message: 'Basic authentication cannot be used with 2FA' + ); + } + + return $user; } /** @@ -114,7 +135,11 @@ public static function isEnabled( // if any login method requires 2FA, // basic auth without 2FA would be a weakness - if (in_array(true, array_column($methods, '2fa'), true) === true) { + // + // without enforced 2FA this general gate stays open, as it + // cannot know which user is authenticating; users who set + // up a second factor are rejected in `::authenticate()` + if ($auth->methods()->hasAnyRequiring2FA() === true) { if ($fail === true) { throw new PermissionException( message: 'Basic authentication cannot be used with 2FA' diff --git a/src/Auth/Method/CodeMethod.php b/src/Auth/Method/CodeMethod.php index 9167ffecf5..993f1175d0 100644 --- a/src/Auth/Method/CodeMethod.php +++ b/src/Auth/Method/CodeMethod.php @@ -62,19 +62,12 @@ public static function isEnabled(Auth $auth, array $options = []): bool return true; } - public static function isUsingChallenges( - Auth $auth, - array $options = [] - ): bool { - return true; - } - /** * Don't allow to circumvent 2FA by 1FA code method */ protected static function isWithoutAny2FA(Auth $auth): bool { - if (in_array(true, array_column($auth->methods()->config(), '2fa'), true) === true) { + if ($auth->methods()->hasAnyRequiring2FA() === true) { throw new InvalidArgumentException( message: 'The "' . static::type() . '" login method cannot be enabled when 2FA is required' ); diff --git a/src/Auth/Method/PasswordMethod.php b/src/Auth/Method/PasswordMethod.php index 8ebce8ed28..2586d4acc3 100644 --- a/src/Auth/Method/PasswordMethod.php +++ b/src/Auth/Method/PasswordMethod.php @@ -3,7 +3,6 @@ namespace Kirby\Auth\Method; use InvalidArgumentException; -use Kirby\Auth\Auth; use Kirby\Auth\Method; use Kirby\Auth\Status; use Kirby\Cms\User; @@ -68,27 +67,19 @@ public function form(): Component } /** - * Checks whether a second-factor is required + * Checks whether a second factor is required for this user */ - protected function has2FA(User $user): bool + public function has2FA(User $user): bool { - return static::isUsingChallenges( - $this->auth, - $this->options - ); - } - - public static function isUsingChallenges( - Auth $auth, - array $options = [] - ): bool { - $option = $options['2fa'] ?? null; - - if ($option === true) { + // enforced: every user needs a second factor, + // even if they have not set one up yet + if (($this->options['2fa'] ?? null) === true) { return true; } - return false; + // otherwise only challenge users who have any + // second factor set up (= available) for them + return $this->auth->challenges()->hasAvailable($user, '2fa'); } public static function settings(User $user): array diff --git a/src/Auth/Method/WebauthnMethod.php b/src/Auth/Method/WebauthnMethod.php index 0cd4f01d7f..39e09bf564 100644 --- a/src/Auth/Method/WebauthnMethod.php +++ b/src/Auth/Method/WebauthnMethod.php @@ -2,7 +2,6 @@ namespace Kirby\Auth\Method; -use Kirby\Auth\Auth; use Kirby\Auth\Exception\LoginNotPermittedException; use Kirby\Auth\Method; use Kirby\Auth\Service\Webauthn; @@ -125,13 +124,6 @@ public static function icon(): string return 'fingerprint'; } - public static function isUsingChallenges( - Auth $auth, - array $options = [] - ): bool { - return false; - } - public static function settings(User $user): array { return [ diff --git a/src/Auth/Methods.php b/src/Auth/Methods.php index ebd50d285b..ea294a889a 100644 --- a/src/Auth/Methods.php +++ b/src/Auth/Methods.php @@ -179,18 +179,11 @@ public function has(string $type): bool } /** - * Checks if any method is using challenges + * Checks if any method requires 2FA */ - public function hasAnyUsingChallenges(): bool + public function hasAnyRequiring2FA(): bool { - foreach ($this->enabled() as $method => $options) { - $class = $this->class($method); - - if ($class::isUsingChallenges($this->auth, $options) === true) { - return true; - } - } - - return false; + return in_array(true, array_column($this->config(), '2fa'), true); } + } diff --git a/src/Cms/System.php b/src/Cms/System.php index 928403fedc..090d190dab 100644 --- a/src/Cms/System.php +++ b/src/Cms/System.php @@ -235,14 +235,19 @@ public function init(): void /** * Check if the Panel has 2FA activated + * + * @deprecated 6.0.0 Use `$kirby->auth()->methods()->hasAnyRequiring2FA()` instead */ public function is2FA(): bool { - return ($this->loginMethods()['password']['2fa'] ?? null) === true; + return $this->app->auth()->methods()->hasAnyRequiring2FA(); } /** * Check if the Panel has 2FA with TOTP activated + * + * @deprecated 6.0.0 Use `$kirby->auth()->methods()->hasAnyRequiring2FA()` and + * `$kirby->auth()->enabledChallenges()` instead */ public function is2FAWithTOTP(): bool { diff --git a/src/Panel/Controller/Drawer/UserCredentialDrawerController.php b/src/Panel/Controller/Drawer/UserCredentialDrawerController.php index fc640fae77..cd6e5b025c 100644 --- a/src/Panel/Controller/Drawer/UserCredentialDrawerController.php +++ b/src/Panel/Controller/Drawer/UserCredentialDrawerController.php @@ -7,6 +7,7 @@ use Kirby\Cms\User; use Kirby\Exception\InvalidArgumentException; use Kirby\Exception\PermissionException; +use Throwable; /** * Shared base for drawers that manage a removable login credential @@ -42,7 +43,12 @@ protected function authorization(): mixed $this->kirby->session()->set( 'kirby.security.authorize.' . $this->user->id(), - $pending->toArray() + [ + ...$pending->toArray(), + // the stored code is only valid for the challenge's + // lifetime, so a leaked session cannot reuse it forever + 'expires' => time() + $challenge->timeout() + ] ); return $pending->public(); @@ -91,15 +97,50 @@ protected function authorizeAdmin(): void */ protected function authorizeCurrentUser(): void { + $key = 'kirby.security.authorize.' . $this->user->id(); + $session = $this->kirby->session(); + $limits = $this->kirby->auth()->limits(); + $email = $this->user->email(); + + // block once the shared auth rate limit is exhausted, so that the + // secret/code cannot be brute-forced from a hijacked session + $limits->ensure($email); + $challenge = $this->challenge(); - $pending = $this->kirby->session()->pull('kirby.security.authorize.' . $this->user->id()) ?? []; - $pending = Pending::from($pending); + $stored = $session->get($key) ?? []; + $input = $this->request->get('authorization'); - $input = $this->request->get('authorization'); + // a stored code is only valid for the challenge's lifetime + $expires = $stored['expires'] ?? null; - if ($challenge->verify($input, $pending) !== true) { + if (is_int($expires) === true && $expires < time()) { + $session->remove($key); throw new InvalidArgumentException(key: 'access.code'); } + + try { + if ($challenge->verify($input, Pending::from($stored)) !== true) { + throw new InvalidArgumentException(key: 'access.code'); + } + } catch (Throwable $e) { + // count the failed attempt against the rate limit + $limits->track($email, triggerHook: false); + + // a single-use challenge signs a one-time nonce that must be + // invalidated even after a failed attempt (e.g. WebAuthn); a + // reusable code is kept so the account owner can retry + // with the correct code within its lifetime + // instead of being locked out by a single typo + if ($challenge->isSingleUse() === true) { + $session->remove($key); + } + + throw $e; + } + + // the action is authorized: consume the pending so the same + // code or nonce cannot be replayed for another change + $session->remove($key); } /** diff --git a/src/Panel/Controller/Drawer/UserEmailChallengeDrawerController.php b/src/Panel/Controller/Drawer/UserEmailChallengeDrawerController.php new file mode 100644 index 0000000000..82a374ee2e --- /dev/null +++ b/src/Panel/Controller/Drawer/UserEmailChallengeDrawerController.php @@ -0,0 +1,87 @@ +authorizeCurrentUser(); + + return $this->user->changeSecret('email', true); + } + + protected function isEnabled(): bool + { + return $this->user->secret('email') === true; + } + + public function load(): Drawer + { + return new Drawer( + component: 'k-user-email-challenge-drawer', + icon: EmailChallenge::icon(), + title: $this->i18n('login.challenge.email.label'), + isAccount: $this->isCurrentUser(), + isEnabled: $this->isEnabled(), + size: 'tiny', + user: $this->user->panel()->info() + ); + } + + protected function remove(): User + { + $this->authorize(); + return $this->user->changeSecret('email', null); + } + + /** + * Sends a one-time code to the user's email address and + * remembers it for the following create/remove action. + */ + protected function send(): User + { + $this->authorization(); + return $this->user; + } + + public function submit(): bool + { + $this->user = match ($action = $this->request->get('action')) { + 'code' => $this->send(), + 'create' => $this->create(), + 'remove' => $this->remove(), + default => throw new InvalidArgumentException( + message: 'Invalid action: ' . $action + ) + }; + + return true; + } + +} diff --git a/src/Panel/Controller/Drawer/UserSecurityDrawerController.php b/src/Panel/Controller/Drawer/UserSecurityDrawerController.php index 402f3bac09..2c211b45e7 100644 --- a/src/Panel/Controller/Drawer/UserSecurityDrawerController.php +++ b/src/Panel/Controller/Drawer/UserSecurityDrawerController.php @@ -25,12 +25,6 @@ public function auth(): Auth public function challenges(): array { - $methods = $this->auth()->methods(); - - if ($methods->hasAnyUsingChallenges() === false) { - return []; - } - $buttons = []; $challenges = $this->auth()->challenges(); @@ -73,7 +67,7 @@ public function methods(): array $methods = $this->auth()->methods()->enabled(); - foreach ($methods as $type => $options) { + foreach (array_keys($methods) as $type) { $method = $this->auth()->methods()->class($type); $buttons = [ ...$buttons, diff --git a/tests/Auth/AuthChallengeTest.php b/tests/Auth/AuthChallengeTest.php index 3db41156f2..7d5572f5fc 100644 --- a/tests/Auth/AuthChallengeTest.php +++ b/tests/Auth/AuthChallengeTest.php @@ -52,7 +52,12 @@ protected function setUp(): void ] ]); - F::write(static::TMP . '/site/accounts/marge/.htpasswd', self::$password); + // the email challenge is opt-in as a second factor, + // so enable it for the user that tests the 2FA flow + F::write( + static::TMP . '/site/accounts/marge/.htpasswd', + self::$password . "\n" . '{"email":true}' + ); $this->auth = $this->app->auth(); } diff --git a/tests/Auth/Challenge/EmailChallengeTest.php b/tests/Auth/Challenge/EmailChallengeTest.php index 38e36f0d6c..64d13a7a72 100644 --- a/tests/Auth/Challenge/EmailChallengeTest.php +++ b/tests/Auth/Challenge/EmailChallengeTest.php @@ -203,7 +203,43 @@ public function testForm(): void public function testIsAvailable(): void { $this->assertTrue(EmailChallenge::isAvailable($this->user, 'login')); - $this->assertTrue(EmailChallenge::isAvailable($this->user, '2fa')); + $this->assertTrue(EmailChallenge::isAvailable($this->user, 'password-reset')); + } + + public function testIsAvailable2FA(): void + { + // as a second factor, the challenge is opt-in per user + $this->assertFalse(EmailChallenge::isAvailable($this->user, '2fa')); + + $this->app->impersonate('kirby'); + $user = $this->user->changeSecret('email', true); + + $this->assertTrue(EmailChallenge::isAvailable($user, '2fa')); + } + + public function testIsAvailable2FAEnforced(): void + { + // enforced 2FA needs a factor for every user, so email stays + // available for those who have not opted in + $this->app = $this->app->clone([ + 'options' => [ + 'auth' => [ + 'methods' => ['password' => ['2fa' => true]] + ] + ] + ]); + + $user = $this->app->user('marge'); + $this->assertTrue(EmailChallenge::isAvailable($user, '2fa')); + } + + public function testIsAvailable2FAWithoutOptIn(): void + { + // the opt-in must not be satisfied by any other truthy value + $this->app->impersonate('kirby'); + $user = $this->user->changeSecret('email', 'yes'); + + $this->assertFalse(EmailChallenge::isAvailable($user, '2fa')); } public function testIsEnabled(): void @@ -227,6 +263,13 @@ public function testSettings(): void { $settings = EmailChallenge::settings($this->user); $this->assertCount(1, $settings); + + $props = $settings[0]->render()['props']; + $this->assertSame('email-unread', $props['icon']); + $this->assertSame( + $this->user->panel()->url(true) . '/security/challenge/email', + $props['drawer'] + ); } public function testTimeout(): void diff --git a/tests/Auth/ChallengesTest.php b/tests/Auth/ChallengesTest.php index c23a4c520f..dc826bcb10 100644 --- a/tests/Auth/ChallengesTest.php +++ b/tests/Auth/ChallengesTest.php @@ -249,6 +249,16 @@ public function testGet(): void $this->assertSame(123, $challenge->timeout()); } + public function testHasAvailable(): void + { + $user = $this->app->user('marge'); + + $this->assertTrue($this->challenges->hasAvailable($user, 'login')); + + DummyChallenge::$available = false; + $this->assertFalse($this->challenges->hasAvailable($user, 'login')); + } + public function testSwitch(): void { $this->app = $this->app->clone([ diff --git a/tests/Auth/Method/BasicAuthMethodTest.php b/tests/Auth/Method/BasicAuthMethodTest.php index 184aa2242e..58e630e8c1 100644 --- a/tests/Auth/Method/BasicAuthMethodTest.php +++ b/tests/Auth/Method/BasicAuthMethodTest.php @@ -26,12 +26,6 @@ public function __construct( protected bool $ssl, protected object|null $header ) { - parent::__construct(options: [ - 'url' => [ - 'scheme' => $ssl ? 'https' : 'http', - 'host' => 'example.com' - ] - ]); } public function ssl(): bool @@ -52,46 +46,50 @@ protected function auth( bool $allowInsecure = false, bool $hasPassword = true, bool $hasAnyWith2FA = false, + bool $userHas2FA = false, object|null $header = null ): Auth&Stub { - $kirby = $this->createStub(App::class); - $request = $this->request($isSsl, $header); - $kirby->method('request')->willReturn($request); + $kirby = $this->createConfiguredStub(App::class, [ + 'request' => $this->request($isSsl, $header), + ]); $kirby->method('option')->willReturnCallback( - function (string $key) use ($auth, $allowInsecure) { - return match ($key) { - 'api.basicAuth' => $auth, - 'api.allowInsecure' => $allowInsecure, - default => null - }; + fn (string $key) => match ($key) { + 'api.basicAuth' => $auth, + 'api.allowInsecure' => $allowInsecure, + default => null } ); - $methods = $this->createStub(Methods::class); - $config = $hasPassword === true ? ['password' => []] : []; + $config = $hasPassword ? ['password' => []] : []; - if ($hasAnyWith2FA === true && $hasPassword === true) { + if ($hasAnyWith2FA && $hasPassword) { $config['password']['2fa'] = true; } - $methods->method('config')->willReturn($config); + $password = $this->createConfiguredStub(PasswordMethod::class, [ + 'has2FA' => $userHas2FA, + ]); - $auth = $this->createStub(Auth::class); - $auth->method('kirby')->willReturn($kirby); - $auth->method('methods')->willReturn($methods); + $methods = $this->createConfiguredStub(Methods::class, [ + 'config' => $config, + 'get' => $password, + 'hasAnyRequiring2FA' => $hasAnyWith2FA, + ]); - return $auth; + return $this->createConfiguredStub(Auth::class, [ + 'kirby' => $kirby, + 'methods' => $methods, + ]); } protected function header( string $user = 'kirby', string $password = 'secret' ): BasicAuth { - $header = $this->createStub(BasicAuth::class); - $header->method('username')->willReturn($user); - $header->method('password')->willReturn($password); - - return $header; + return $this->createConfiguredStub(BasicAuth::class, [ + 'username' => $user, + 'password' => $password, + ]); } public function testAuthenticate(): void @@ -109,6 +107,36 @@ public function testAuthenticate(): void $this->assertSame($user, $result); } + public function testAuthenticateWithUser2FA(): void + { + // the user would be challenged on regular password login, + // so basic auth must not let them skip the second factor + $auth = $this->auth(userHas2FA: true, header: $this->header()); + $auth->method('validatePassword') + ->willReturn($this->createStub(User::class)); + + $this->expectException(PermissionException::class); + $this->expectExceptionMessage('Basic authentication cannot be used with 2FA'); + + $method = new BasicAuthMethod(auth: $auth); + $method->authenticate('kirby@getkirby.com', 'topsecret'); + } + + public function testAuthenticateWithoutUser2FA(): void + { + // dedicated API accounts without a second factor + // can still use basic auth when 2FA is not enforced + $user = $this->createStub(User::class); + $auth = $this->auth(userHas2FA: false, header: $this->header()); + $auth->method('validatePassword')->willReturn($user); + + $method = new BasicAuthMethod(auth: $auth); + $this->assertSame( + $user, + $method->authenticate('kirby@getkirby.com', 'topsecret') + ); + } + public function testAuthenticateWithoutPassword(): void { $this->expectException(InvalidArgumentException::class); diff --git a/tests/Auth/Method/CodeMethodTest.php b/tests/Auth/Method/CodeMethodTest.php index e960b793a7..40c2a860f4 100644 --- a/tests/Auth/Method/CodeMethodTest.php +++ b/tests/Auth/Method/CodeMethodTest.php @@ -32,6 +32,7 @@ protected function auth( } $methods->method('config')->willReturn($config); + $methods->method('hasAnyRequiring2FA')->willReturn($has2fa === true); $auth = $this->createStub(Auth::class); $auth->method('methods')->willReturn($methods); @@ -108,12 +109,6 @@ public function testIsEnabledWithPasswordReset(): void CodeMethod::isEnabled($auth); } - public function testIsUsingChallenges(): void - { - $auth = $this->auth(); - $this->assertTrue(CodeMethod::isUsingChallenges($auth)); - } - public function testSettings(): void { $user = $this->createStub(User::class); diff --git a/tests/Auth/Method/MethodTest.php b/tests/Auth/Method/MethodTest.php index deef6ef6ef..39b845d5e9 100644 --- a/tests/Auth/Method/MethodTest.php +++ b/tests/Auth/Method/MethodTest.php @@ -15,10 +15,4 @@ public function testIsEnabled(): void $auth = $this->createStub(Auth::class); $this->assertTrue(Method::isEnabled($auth)); } - - public function testIsUsingChallenges(): void - { - $auth = $this->createStub(Auth::class); - $this->assertFalse(Method::isUsingChallenges($auth)); - } } diff --git a/tests/Auth/Method/PasswordMethodTest.php b/tests/Auth/Method/PasswordMethodTest.php index a708df7a18..72ec8af5c9 100644 --- a/tests/Auth/Method/PasswordMethodTest.php +++ b/tests/Auth/Method/PasswordMethodTest.php @@ -4,6 +4,7 @@ use InvalidArgumentException; use Kirby\Auth\Auth; +use Kirby\Auth\Challenges; use Kirby\Auth\Method; use Kirby\Auth\Status; use Kirby\Cms\User; @@ -89,6 +90,75 @@ public function testAuthenticateWith2FA(): void $this->assertSame(['marge@simpsons.com', false, '2fa'], $challenge); } + public function testAuthenticateWithoutSecondFactor(): void + { + $login = []; + $user = $this->createStub(User::class); + $user->method('loginPasswordless') + ->willReturnCallback(function ($options) use (&$login) { + $login[] = $options; + }); + + $validate = null; + $challenges = $this->createStub(Challenges::class); + $challenges->method('hasAvailable')->willReturn(false); + + $auth = $this->createStub(Auth::class); + $auth->method('validatePassword') + ->willReturnCallback(function (...$args) use (&$validate, $user) { + $validate = $args; + return $user; + }); + $auth->method('challenges')->willReturn($challenges); + $auth->method('createChallenge') + ->willReturnCallback(function () { + throw new RuntimeException('createChallenge should not be called'); + }); + + // 2FA is not enforced and the user has no second factor + $method = new PasswordMethod(auth: $auth); + $result = $method->authenticate('marge@simpsons.com', 'springfield123', true); + + $this->assertInstanceOf(User::class, $result); + $this->assertSame($user, $result); + $this->assertSame(['marge@simpsons.com', 'springfield123'], $validate); + $this->assertSame([[ + 'createMode' => 'cookie', + 'long' => true + ]], $login); + } + + public function testAuthenticateWithSecondFactor(): void + { + $status = $this->createStub(Status::class); + $user = $this->createStub(User::class); + $validate = null; + $challenge = null; + $challenges = $this->createStub(Challenges::class); + $challenges->method('hasAvailable')->willReturn(true); + + $auth = $this->createStub(Auth::class); + $auth->method('validatePassword') + ->willReturnCallback(function (...$args) use (&$validate, $user) { + $validate = $args; + return $user; + }); + $auth->method('challenges')->willReturn($challenges); + $auth->method('createChallenge') + ->willReturnCallback(function (...$args) use (&$challenge, $status) { + $challenge = $args; + return $status; + }); + + // 2FA is not enforced, but the user opted into a second factor + $method = new PasswordMethod(auth: $auth); + $result = $method->authenticate('marge@simpsons.com', 'springfield123'); + + $this->assertSame($status, $result); + $this->assertSame(['marge@simpsons.com', 'springfield123'], $validate); + $this->assertSame(['marge@simpsons.com', false, '2fa'], $challenge); + } + public function testAuthenticateWithoutPassword(): void { $this->expectException(InvalidArgumentException::class); @@ -111,14 +181,6 @@ public function testIsEnabled(): void $this->assertTrue(PasswordMethod::isEnabled($auth)); } - public function testIsUsingChallenges(): void - { - $auth = $this->createStub(Auth::class); - - $this->assertFalse(PasswordMethod::isUsingChallenges($auth)); - $this->assertTrue(PasswordMethod::isUsingChallenges($auth, ['2fa' => true])); - } - public function testOptions(): void { $auth = $this->createStub(Auth::class); diff --git a/tests/Auth/Method/PasswordResetMethodTest.php b/tests/Auth/Method/PasswordResetMethodTest.php index ea15a9a82c..0b067f9bdd 100644 --- a/tests/Auth/Method/PasswordResetMethodTest.php +++ b/tests/Auth/Method/PasswordResetMethodTest.php @@ -26,6 +26,7 @@ protected function auth(bool $has2fa = false): Auth } $methods->method('config')->willReturn($config); + $methods->method('hasAnyRequiring2FA')->willReturn($has2fa === true); $auth = $this->createStub(Auth::class); $auth->method('methods')->willReturn($methods); @@ -92,12 +93,6 @@ public function testIsEnabledWith2FA(): void PasswordResetMethod::isEnabled($auth); } - public function testIsUsingChallenges(): void - { - $auth = $this->auth(); - $this->assertTrue(PasswordResetMethod::isUsingChallenges($auth)); - } - public function testSettings(): void { $user = $this->createStub(User::class); diff --git a/tests/Auth/Method/WebauthnMethodTest.php b/tests/Auth/Method/WebauthnMethodTest.php index f2f2785885..e7c6e249ab 100644 --- a/tests/Auth/Method/WebauthnMethodTest.php +++ b/tests/Auth/Method/WebauthnMethodTest.php @@ -106,11 +106,6 @@ public function testIcon(): void $this->assertSame('fingerprint', WebauthnMethod::icon()); } - public function testIsUsingChallenges(): void - { - $this->assertFalse(WebauthnMethod::isUsingChallenges($this->app->auth())); - } - public function testSettings(): void { $settings = WebauthnMethod::settings($this->app->user('marge')); diff --git a/tests/Auth/MethodsTest.php b/tests/Auth/MethodsTest.php index 7a561047ac..f190133b34 100644 --- a/tests/Auth/MethodsTest.php +++ b/tests/Auth/MethodsTest.php @@ -268,44 +268,6 @@ public function testHasEnabledMethods(): void $this->assertFalse($methods->has('password-reset')); } - public function testHasAnyUsingChallenges(): void - { - $app = $this->app->clone([ - 'options' => [ - 'auth' => [ - 'methods' => [ - 'password' => ['2fa' => true], - ] - ] - ] - ]); - - $methods = $app->auth()->methods(); - $this->assertTrue($methods->hasAnyUsingChallenges()); - - $app = $this->app->clone([ - 'options' => [ - 'auth' => [ - 'methods' => ['password', 'code'] - ] - ] - ]); - - $methods = $app->auth()->methods(); - $this->assertTrue($methods->hasAnyUsingChallenges()); - - $app = $this->app->clone([ - 'options' => [ - 'auth' => [ - 'methods' => ['password'] - ] - ] - ]); - - $methods = $app->auth()->methods(); - $this->assertFalse($methods->hasAnyUsingChallenges()); - } - public function testHasWithDisabled(): void { $app = $this->app->clone([ diff --git a/tests/Panel/Areas/AccountDrawersTest.php b/tests/Panel/Areas/AccountDrawersTest.php new file mode 100644 index 0000000000..a1ca80e602 --- /dev/null +++ b/tests/Panel/Areas/AccountDrawersTest.php @@ -0,0 +1,30 @@ + $drawer) { + $mirror = 'account.' . substr($key, strlen('user.')); + + $this->assertArrayHasKey( + $mirror, + $account, + 'Missing account drawer for ' . $key + ); + $this->assertSame($drawer['action'], $account[$mirror]['action']); + } + } +} diff --git a/tests/Panel/Controller/Drawer/UserCredentialDrawerControllerTest.php b/tests/Panel/Controller/Drawer/UserCredentialDrawerControllerTest.php index 6356ee10f5..325b546cbd 100644 --- a/tests/Panel/Controller/Drawer/UserCredentialDrawerControllerTest.php +++ b/tests/Panel/Controller/Drawer/UserCredentialDrawerControllerTest.php @@ -4,9 +4,11 @@ use Kirby\Auth\Challenge; use Kirby\Auth\Challenge\TotpChallenge; +use Kirby\Auth\Exception\RateLimitException; use Kirby\Auth\Pending; use Kirby\Cms\User; use Kirby\Exception\InvalidArgumentException; +use Kirby\Exception\PermissionException; use Kirby\Panel\TestCase; use Kirby\Toolkit\Totp; use PHPUnit\Framework\Attributes\CoversClass; @@ -31,6 +33,14 @@ public function verify(mixed $input, Pending $data): bool } } +class DummySingleUseCredentialChallenge extends DummyCredentialChallenge +{ + public function isSingleUse(): bool + { + return true; + } +} + class DummyUserCredentialDrawerController extends UserCredentialDrawerController { public function __construct(User $user, string $type = 'totp') @@ -48,6 +58,11 @@ public function authorize(): void parent::authorize(); } + public function create(): User + { + return parent::create(); + } + public function challenge(): Challenge { return parent::challenge(); @@ -75,7 +90,8 @@ protected function setUp(): void $this->app = $this->app->clone([ 'authChallenges' => [ - 'dummy' => DummyCredentialChallenge::class + 'dummy' => DummyCredentialChallenge::class, + 'dummy-single-use' => DummySingleUseCredentialChallenge::class ], 'users' => [ [ @@ -117,10 +133,13 @@ public function testAuthorization(): void $result = $controller->authorization(); $this->assertNull($result); - $this->assertSame( - ['public' => null, 'secret' => null], - $this->app->session()->get('kirby.security.authorize.test') - ); + + // the pending is stored with an expiry so a leaked session + // cannot reuse the code indefinitely + $stored = $this->app->session()->get('kirby.security.authorize.test'); + $this->assertNull($stored['public']); + $this->assertNull($stored['secret']); + $this->assertGreaterThan(time(), $stored['expires']); } public function testAuthorizationForOtherUser(): void @@ -144,13 +163,11 @@ public function testAuthorizationWithChallengeData(): void $result = $controller->authorization(); $this->assertSame(['id' => 'pending-public'], $result); - $this->assertSame( - [ - 'public' => ['id' => 'pending-public'], - 'secret' => 'pending-secret' - ], - $this->app->session()->get('kirby.security.authorize.test') - ); + + $stored = $this->app->session()->get('kirby.security.authorize.test'); + $this->assertSame(['id' => 'pending-public'], $stored['public']); + $this->assertSame('pending-secret', $stored['secret']); + $this->assertGreaterThan(time(), $stored['expires']); } public function testAuthorizeAsAdmin(): void @@ -214,6 +231,112 @@ public function testAuthorizeAsCurrentUserWithInvalidCode(): void $controller->authorize(); } + public function testAuthorizeAsCurrentUserKeepsReusableCodeOnFailure(): void + { + // a reusable code (challenge is not single-use) must survive a + // failed attempt, so the account owner can retry with the correct + // one instead of being locked out after a single typo + $this->setRequest(['authorization' => 'wrong-secret']); + $this->app->impersonate('test'); + + $this->app->session()->set('kirby.security.authorize.test', [ + 'public' => null, + 'secret' => 'pending-secret' + ]); + + $controller = new DummyUserCredentialDrawerController($this->app->user('test'), 'dummy'); + + try { + $controller->authorize(); + $this->fail('Expected InvalidArgumentException was not thrown'); + } catch (InvalidArgumentException $e) { + $this->assertSame('error.access.code', $e->getCode()); + } + + // the stored pending is untouched and ready for the next attempt + $this->assertSame( + ['public' => null, 'secret' => 'pending-secret'], + $this->app->session()->get('kirby.security.authorize.test') + ); + } + + public function testAuthorizeAsCurrentUserClearsSingleUseCodeOnFailure(): void + { + // a single-use nonce must be invalidated even after a failed + // attempt so that it cannot be replayed within its lifetime + $this->setRequest(['authorization' => 'wrong-secret']); + $this->app->impersonate('test'); + + $this->app->session()->set('kirby.security.authorize.test', [ + 'public' => null, + 'secret' => 'pending-secret' + ]); + + $controller = new DummyUserCredentialDrawerController($this->app->user('test'), 'dummy-single-use'); + + try { + $controller->authorize(); + $this->fail('Expected InvalidArgumentException was not thrown'); + } catch (InvalidArgumentException) { + // expected + } + + $this->assertNull( + $this->app->session()->get('kirby.security.authorize.test') + ); + } + + public function testAuthorizeAsCurrentUserWithExpiredCode(): void + { + // a stored code past its lifetime is rejected and cleared, + // even if the input itself would have matched + $this->setRequest(['authorization' => 'pending-secret']); + $this->app->impersonate('test'); + + $this->app->session()->set('kirby.security.authorize.test', [ + 'public' => null, + 'secret' => 'pending-secret', + 'expires' => 1 + ]); + + $controller = new DummyUserCredentialDrawerController($this->app->user('test'), 'dummy'); + + try { + $controller->authorize(); + $this->fail('Expected InvalidArgumentException was not thrown'); + } catch (InvalidArgumentException $e) { + $this->assertSame('error.access.code', $e->getCode()); + } + + $this->assertNull( + $this->app->session()->get('kirby.security.authorize.test') + ); + } + + public function testAuthorizeAsCurrentUserRateLimited(): void + { + // once the shared auth rate limit is hit, no further code + // can be tried until the limit resets + $this->app = $this->app->clone([ + 'options' => ['auth' => ['trials' => 1]] + ]); + $this->setRequest(['authorization' => 'pending-secret']); + $this->app->impersonate('test'); + + // exhaust the limit for this visitor + $this->app->auth()->limits()->track('test@getkirby.com'); + + $this->app->session()->set('kirby.security.authorize.test', [ + 'public' => null, + 'secret' => 'pending-secret' + ]); + + $controller = new DummyUserCredentialDrawerController($this->app->user('test'), 'dummy'); + + $this->expectException(RateLimitException::class); + $controller->authorize(); + } + public function testChallenge(): void { $controller = new DummyUserCredentialDrawerController($this->app->user('test'), 'totp'); @@ -224,6 +347,19 @@ public function testChallenge(): void $this->assertTrue($challenge->user()->is($this->app->user('test'))); } + public function testCreateForOtherUser(): void + { + // adding a login credential is only ever allowed for one's own + // account: an admin managing another user is rejected here, before + // any challenge verification (`authorizeCurrentUser()`) can run + $this->app->impersonate('admin'); + + $controller = new DummyUserCredentialDrawerController($this->app->user('test')); + + $this->expectException(PermissionException::class); + $controller->create(); + } + public function testIsCurrentUser(): void { $this->app->impersonate('test'); diff --git a/tests/Panel/Controller/Drawer/UserEmailChallengeDrawerControllerTest.php b/tests/Panel/Controller/Drawer/UserEmailChallengeDrawerControllerTest.php new file mode 100644 index 0000000000..c5154b967a --- /dev/null +++ b/tests/Panel/Controller/Drawer/UserEmailChallengeDrawerControllerTest.php @@ -0,0 +1,307 @@ +app = $this->app->clone([ + // the challenge derives its sender address from the host + 'server' => [ + 'SERVER_NAME' => 'getkirby.com' + ], + 'users' => [ + [ + 'id' => 'test', + 'name' => 'Test User', + 'email' => 'test@getkirby.com', + 'role' => 'admin', + 'password' => User::hashPassword('password123') + ], + [ + 'id' => 'admin', + 'email' => 'admin@getkirby.com', + 'role' => 'admin', + 'password' => User::hashPassword('adminpass123') + ] + ], + 'site' => [ + 'title' => 'Test Site' + ] + ]); + + $this->app->impersonate('kirby'); + } + + protected function tearDown(): void + { + Email::$debug = false; + Email::$emails = []; + + parent::tearDown(); + } + + /** + * Opts the test user into the email challenge + */ + protected function enableEmailChallenge(): void + { + $this->app->user('test')->changeSecret('email', true); + } + + /** + * Stores a pending challenge for the test user, just like + * the `code` action would have done before the user submits + */ + protected function storeCode(string $code = '123456'): void + { + $this->app->session()->set('kirby.security.authorize.test', [ + 'public' => null, + 'secret' => User::hashPassword($code) + ]); + } + + public function testFactory(): void + { + $controller = UserEmailChallengeDrawerController::factory('test'); + $this->assertInstanceOf(UserEmailChallengeDrawerController::class, $controller); + } + + public function testLoad(): void + { + $this->app->impersonate('test'); + + $user = $this->app->user('test'); + $controller = new UserEmailChallengeDrawerController($user); + $drawer = $controller->load(); + + $this->assertInstanceOf(Drawer::class, $drawer); + $this->assertSame('k-user-email-challenge-drawer', $drawer->component); + $this->assertSame('email-unread', $drawer->icon); + $this->assertNotNull($drawer->title); + + $props = $drawer->props(); + $this->assertTrue($props['isAccount']); + $this->assertFalse($props['isEnabled']); + $this->assertSame( + ['avatar' => null, 'email' => 'test@getkirby.com', 'name' => 'Test User'], + $props['user'] + ); + + // opening the drawer must not send an email on its own + $this->assertSame([], Email::$emails); + } + + public function testLoadForOtherUser(): void + { + $this->enableEmailChallenge(); + $this->app->impersonate('admin'); + + $controller = new UserEmailChallengeDrawerController($this->app->user('test')); + $props = $controller->load()->props(); + + $this->assertFalse($props['isAccount']); + $this->assertTrue($props['isEnabled']); + } + + public function testSubmitCode(): void + { + $this->setRequest(['action' => 'code']); + $this->app->impersonate('test'); + + $result = (new UserEmailChallengeDrawerController($this->app->user('test')))->submit(); + + $this->assertTrue($result); + $this->assertCount(1, Email::$emails); + $this->assertSame( + ['test@getkirby.com' => 'Test User'], + Email::$emails[0]->to() + ); + + // the emailed code is kept for the following create/remove action + $body = Email::$emails[0]->body()->text(); + preg_match('/[0-9]{3} [0-9]{3}/', $body, $code); + $this->assertNotEmpty($code[0]); + + $pending = $this->app->session()->get('kirby.security.authorize.test'); + $this->assertTrue( + password_verify(str_replace(' ', '', $code[0]), $pending['secret']) + ); + } + + public function testSubmitCodeForOtherUser(): void + { + // an admin authorizes with their own password instead, + // so no code must be sent to the other user + $this->setRequest(['action' => 'code']); + $this->app->impersonate('admin'); + + $controller = new UserEmailChallengeDrawerController($this->app->user('test')); + + $this->assertTrue($controller->submit()); + $this->assertSame([], Email::$emails); + } + + public function testSubmitCreate(): void + { + // the code is entered as it was formatted in the email + $this->setRequest([ + 'action' => 'create', + 'authorization' => '123 456' + ]); + $this->app->impersonate('test'); + $this->storeCode(); + + $result = (new UserEmailChallengeDrawerController($this->app->user('test')))->submit(); + + $this->assertTrue($result); + $this->assertTrue($this->app->user('test')->secret('email')); + } + + public function testSubmitCreateForOtherUser(): void + { + // an admin must not opt another user into the challenge: + // only the account owner can prove their address is reachable + $this->setRequest(['action' => 'create']); + $this->app->impersonate('admin'); + + $controller = new UserEmailChallengeDrawerController($this->app->user('test')); + + try { + $controller->submit(); + $this->fail('Expected PermissionException was not thrown'); + } catch (PermissionException) { + $this->assertNull($this->app->user('test')->secret('email')); + } + } + + public function testSubmitCreateWithWrongCode(): void + { + $this->setRequest([ + 'action' => 'create', + 'authorization' => '000 000' + ]); + $this->app->impersonate('test'); + $this->storeCode(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionCode('error.access.code'); + + (new UserEmailChallengeDrawerController($this->app->user('test')))->submit(); + } + + public function testSubmitCreateWithoutCode(): void + { + // the code action must run first, otherwise there is + // nothing to verify the user's input against + $this->setRequest([ + 'action' => 'create', + 'authorization' => '000 000' + ]); + $this->app->impersonate('test'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionCode('error.access.code'); + + (new UserEmailChallengeDrawerController($this->app->user('test')))->submit(); + } + + public function testSubmitRemoveAsAccount(): void + { + // the account owner opts out by entering a fresh code, + // proving they still control the address + $this->enableEmailChallenge(); + + $this->setRequest([ + 'action' => 'remove', + 'authorization' => '123 456' + ]); + $this->app->impersonate('test'); + $this->storeCode(); + + $result = (new UserEmailChallengeDrawerController($this->app->user('test')))->submit(); + + $this->assertTrue($result); + $this->assertNull($this->app->user('test')->secret('email')); + } + + public function testSubmitRemoveAsAccountWithWrongCode(): void + { + $this->enableEmailChallenge(); + + $this->setRequest([ + 'action' => 'remove', + 'authorization' => '000 000' + ]); + $this->app->impersonate('test'); + $this->storeCode(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionCode('error.access.code'); + + (new UserEmailChallengeDrawerController($this->app->user('test')))->submit(); + } + + public function testSubmitRemoveAsAdmin(): void + { + // an admin opts another user out with their own password, + // e.g. when that user lost access to their mailbox + $this->enableEmailChallenge(); + + $this->setRequest([ + 'action' => 'remove', + 'password' => 'adminpass123' + ]); + $this->app->impersonate('admin'); + + $result = (new UserEmailChallengeDrawerController($this->app->user('test')))->submit(); + + $this->assertTrue($result); + $this->assertNull($this->app->user('test')->secret('email')); + } + + public function testSubmitRemoveAsAdminWithWrongPassword(): void + { + $this->enableEmailChallenge(); + + $this->setRequest([ + 'action' => 'remove', + 'password' => 'wrongpass' + ]); + $this->app->impersonate('admin'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionCode('error.user.password.wrong'); + + (new UserEmailChallengeDrawerController($this->app->user('test')))->submit(); + } + + public function testSubmitWithInvalidAction(): void + { + $this->setRequest(['action' => 'nope']); + $this->app->impersonate('kirby'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid action: nope'); + + (new UserEmailChallengeDrawerController($this->app->user('test')))->submit(); + } +} diff --git a/tests/Panel/Controller/Drawer/UserSecurityDrawerControllerTest.php b/tests/Panel/Controller/Drawer/UserSecurityDrawerControllerTest.php index 7fe64621fd..d3d9a2c50b 100644 --- a/tests/Panel/Controller/Drawer/UserSecurityDrawerControllerTest.php +++ b/tests/Panel/Controller/Drawer/UserSecurityDrawerControllerTest.php @@ -32,10 +32,12 @@ protected function setUp(): void public function testChallenges(): void { + // no challenge is enabled on this site $this->app = $this->app->clone([ 'options' => [ 'auth' => [ - 'methods' => ['password'] + 'methods' => ['password'], + 'challenges' => [] ] ] ]); @@ -44,6 +46,25 @@ public function testChallenges(): void $controller = new UserSecurityDrawerController($user); $this->assertSame([], $controller->challenges()); + // without enforced 2FA users can still opt into a second factor + $this->app = $this->app->clone([ + 'options' => [ + 'auth' => [ + 'methods' => ['password'], + 'challenges' => ['totp'] + ] + ] + ]); + + $this->app->impersonate('test'); + + $user = $this->app->user('test'); + $controller = new UserSecurityDrawerController($user); + $challenges = $controller->challenges(); + + $this->assertCount(1, $challenges); + $this->assertSame('qr-code', $challenges[0]['icon']); + $this->app = $this->app->clone([ 'options' => [ @@ -63,7 +84,7 @@ public function testChallenges(): void $this->assertCount(2, $challenges); $this->assertSame('email-unread', $challenges[0]['icon']); - $this->assertSame($user->panel()->url(true) . '/changeEmail', $challenges[0]['dialog']); + $this->assertSame($user->panel()->url(true) . '/security/challenge/email', $challenges[0]['drawer']); $this->assertSame('qr-code', $challenges[1]['icon']); $this->assertSame($user->panel()->url(true) . '/security/challenge/totp', $challenges[1]['drawer']); }