diff --git a/docs/references/authentication/auth_actions.md b/docs/references/authentication/auth_actions.md index 24aad2290..817b65297 100644 --- a/docs/references/authentication/auth_actions.md +++ b/docs/references/authentication/auth_actions.md @@ -86,4 +86,52 @@ and provides feedback. In the `Email2FA` class, it verifies the code against wha database and either sends them back to the previous form to try again or redirects the user to the page that a `login` task would have redirected them to anyway. -All methods should return either a `Response` or a view string (e.g. using the `view()` function). \ No newline at end of file +All methods should return either a `Response` or a view string (e.g. using the `view()` function). + +## Conditional Actions + +Some applications only need an action for certain users. For example, you may +want email-based 2FA for administrators, but not for every user. + +To make an action conditional, implement `ConditionalActionInterface`: + +```php +inGroup('admin', 'superadmin'); + } +} +``` + +Then register your conditional action in **app/Config/Auth.php**: + +```php +public array $actions = [ + 'register' => null, + 'login' => \App\Authentication\Actions\AdminEmail2FA::class, +]; +``` + +When `appliesTo()` returns `true`, Shield starts the action as usual and +discovers any stored identity for that action. When it returns `false`, Shield +does not start the action and ignores stored identities for that action while +the condition remains false. The exception is activation: if a user is already +inactive and has a stored activation identity, Shield continues to require that +activation before login can complete. + +The `appliesTo()` method may be called more than once while Shield checks for +actions, so keep it deterministic, free of side effects, and fail closed when +the condition cannot be determined. It is not a replacement for authorization. + +Once an action is already pending in the session, Shield continues that pending +action instead of rechecking the condition. diff --git a/docs/references/authorization.md b/docs/references/authorization.md index 81c1e6a14..6208f9f1a 100644 --- a/docs/references/authorization.md +++ b/docs/references/authorization.md @@ -256,6 +256,7 @@ if ($user->isActivated()) { !!! note If no activator is specified in the `Auth` config file, `actions['register']` property, then this will always return `true`. + If a conditional activator does not apply during registration, the newly registered user is activated immediately. You can check if a user has not been activated yet via the `isNotActivated()` method. diff --git a/src/Authentication/Actions/ConditionalActionInterface.php b/src/Authentication/Actions/ConditionalActionInterface.php new file mode 100644 index 000000000..edd305d93 --- /dev/null +++ b/src/Authentication/Actions/ConditionalActionInterface.php @@ -0,0 +1,30 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Shield\Authentication\Actions; + +use CodeIgniter\Shield\Entities\User; + +/** + * Allows an authentication action to decide if it applies to a user. + */ +interface ConditionalActionInterface +{ + /** + * Determines if this action applies to the given user. + * + * This method may be called while Shield starts or discovers pending actions. + * It should be deterministic and free of side effects. + */ + public function appliesTo(User $user): bool; +} diff --git a/src/Authentication/Authenticators/Session.php b/src/Authentication/Authenticators/Session.php index 2b800c2a5..08df82407 100644 --- a/src/Authentication/Authenticators/Session.php +++ b/src/Authentication/Authenticators/Session.php @@ -19,6 +19,7 @@ use CodeIgniter\HTTP\Response; use CodeIgniter\I18n\Time; use CodeIgniter\Shield\Authentication\Actions\ActionInterface; +use CodeIgniter\Shield\Authentication\Actions\ConditionalActionInterface; use CodeIgniter\Shield\Authentication\AuthenticationException; use CodeIgniter\Shield\Authentication\AuthenticatorInterface; use CodeIgniter\Shield\Authentication\Passwords; @@ -185,11 +186,11 @@ public function attempt(array $credentials): Result } /** - * If an action has been defined, start it up. + * If an action has been defined and applies to the user, start it up. * * @param string $type 'register', 'login' * - * @return bool If the action has been defined or not. + * @return bool If the action was started or not. */ public function startUpAction(string $type, User $user): bool { @@ -202,6 +203,10 @@ public function startUpAction(string $type, User $user): bool /** @var ActionInterface $action */ $action = Factories::actions($actionClass); // @phpstan-ignore-line + if (! $this->actionAppliesToUser($action, $user)) { + return false; + } + // Create identity for the action. $action->createIdentity($user); @@ -472,7 +477,7 @@ private function setAuthAction(): bool $authActions = setting('Auth.actions'); - foreach ($authActions as $actionClass) { + foreach ($authActions as $type => $actionClass) { if ($actionClass === null || $actionClass === '') { continue; } @@ -480,6 +485,13 @@ private function setAuthAction(): bool /** @var ActionInterface $action */ $action = Factories::actions($actionClass); // @phpstan-ignore-line + if ( + ! $this->actionAppliesToUser($action, $this->user) + && ! $this->inactiveUserNeedsRegisterAction($type, $this->user) + ) { + continue; + } + $identity = $this->userIdentityModel->getIdentityByType($this->user, $action->getType()); if ($identity instanceof UserIdentity) { @@ -504,31 +516,49 @@ private function getIdentitiesForAction(User $user): array { return $this->userIdentityModel->getIdentitiesByTypes( $user, - $this->getActionTypes(), + $this->getActionTypes($user), ); } /** * @return list */ - private function getActionTypes(): array + private function getActionTypes(User $user): array { $actions = setting('Auth.actions'); $types = []; - foreach ($actions as $actionClass) { + foreach ($actions as $type => $actionClass) { if ($actionClass === null || $actionClass === '') { continue; } /** @var ActionInterface $action */ - $action = Factories::actions($actionClass); // @phpstan-ignore-line + $action = Factories::actions($actionClass); // @phpstan-ignore-line + + if ( + ! $this->actionAppliesToUser($action, $user) + && ! $this->inactiveUserNeedsRegisterAction($type, $user) + ) { + continue; + } + $types[] = $action->getType(); } return $types; } + private function actionAppliesToUser(ActionInterface $action, User $user): bool + { + return ! $action instanceof ConditionalActionInterface || $action->appliesTo($user); + } + + private function inactiveUserNeedsRegisterAction(int|string $type, User $user): bool + { + return $type === 'register' && ! $user->active; + } + /** * Checks if the user is currently in pending login state. * They need to do an auth action. diff --git a/src/Config/Auth.php b/src/Config/Auth.php index c6c27bf1e..db2a0a4a7 100644 --- a/src/Config/Auth.php +++ b/src/Config/Auth.php @@ -98,6 +98,7 @@ class Auth extends BaseConfig * Custom Actions and Requirements: * * - All actions must implement \CodeIgniter\Shield\Authentication\Actions\ActionInterface. + * - Actions may implement \CodeIgniter\Shield\Authentication\Actions\ConditionalActionInterface to apply only to certain users. * - Custom actions for "register" must have a class name that ends with the suffix "Activator" (e.g., `CustomSmsActivator`) ensure proper functionality. * * @var array|null> diff --git a/src/Filters/SessionAuth.php b/src/Filters/SessionAuth.php index 2063f240d..c2d2ca9bc 100644 --- a/src/Filters/SessionAuth.php +++ b/src/Filters/SessionAuth.php @@ -74,6 +74,11 @@ public function before(RequestInterface $request, $arguments = null) return redirect()->route('auth-action-show') ->with('error', lang('Auth.activationBlocked')); } + + $authenticator->logout(); + + return redirect()->to(config('Auth')->logoutRedirect()) + ->with('error', lang('Auth.activationBlocked')); } return; diff --git a/tests/Authentication/Filters/SessionFilterTest.php b/tests/Authentication/Filters/SessionFilterTest.php index 10c8c21b9..34493aae7 100644 --- a/tests/Authentication/Filters/SessionFilterTest.php +++ b/tests/Authentication/Filters/SessionFilterTest.php @@ -14,9 +14,12 @@ namespace Tests\Authentication\Filters; use CodeIgniter\I18n\Time; +use CodeIgniter\Shield\Authentication\Authenticators\Session; use CodeIgniter\Shield\Filters\SessionAuth; +use CodeIgniter\Shield\Models\UserIdentityModel; use CodeIgniter\Shield\Models\UserModel; use CodeIgniter\Test\DatabaseTestTrait; +use Tests\Support\AdminEmailActivator; /** * @internal @@ -94,6 +97,47 @@ public function testBlocksInactiveUsersAndRedirectsToAuthAction(): void setting('Auth.actions', ['register' => null]); } + public function testBlocksInactiveUsersWhenConditionalActivatorDoesNotApply(): void + { + $user = fake(UserModel::class, ['active' => false]); + + setting('Auth.actions', ['register' => AdminEmailActivator::class]); + + $result = $this->actingAs($user) + ->get('protected-route'); + + $result->assertRedirectTo(config('Auth')->logoutRedirect()); + $result->assertSessionHas('error', lang('Auth.activationBlocked')); + $this->assertNull(auth('session')->id()); + + setting('Auth.actions', ['register' => null]); + } + + public function testRedirectsInactiveUsersToStoredConditionalActivationAction(): void + { + $user = fake(UserModel::class, ['active' => false]); + + setting('Auth.actions', ['register' => AdminEmailActivator::class]); + + model(UserIdentityModel::class)->insert([ + 'user_id' => $user->id, + 'type' => Session::ID_TYPE_EMAIL_ACTIVATE, + 'secret' => '123456', + 'name' => 'register', + 'extra' => lang('Auth.needVerification'), + ]); + + /** @var Session $authenticator */ + $authenticator = auth('session')->getAuthenticator(); + $this->assertTrue($authenticator->hasAction($user->id)); + + $result = $this->get('protected-route'); + + $result->assertRedirectTo('/auth/a/show'); + + setting('Auth.actions', ['register' => null]); + } + public function testStoreRedirectsToEntraceUrlIntoSession(): void { $result = $this->call('get', 'protected-route'); diff --git a/tests/Controllers/LoginTest.php b/tests/Controllers/LoginTest.php index acdb1e894..09de25823 100644 --- a/tests/Controllers/LoginTest.php +++ b/tests/Controllers/LoginTest.php @@ -17,9 +17,12 @@ use CodeIgniter\I18n\Time; use CodeIgniter\Shield\Authentication\Actions\Email2FA; use CodeIgniter\Shield\Config\Auth; +use CodeIgniter\Shield\Models\UserIdentityModel; use CodeIgniter\Test\FeatureTestTrait; +use CodeIgniter\Test\TestResponse; use Config\Services; use Config\Validation; +use Tests\Support\AdminEmail2FA; use Tests\Support\DatabaseTestCase; use Tests\Support\FakeUser; @@ -256,4 +259,90 @@ public function testLoginRedirectsToActionIfDefined(): void $result->assertSessionMissing('errors'); $this->assertSame(site_url('auth/a/show'), $result->getRedirectUrl()); } + + public function testLoginRedirectsToConditionalActionWhenItApplies(): void + { + $this->enableAdminEmail2FA(); + + $this->user->addGroup('admin'); + $this->createUserEmailIdentity(); + + $result = $this->loginUser(); + + $result->assertStatus(302); + $result->assertRedirect(); + $this->assertSame(site_url('auth/a/show'), $result->getRedirectUrl()); + } + + public function testLoginSkipsConditionalActionWhenItDoesNotApply(): void + { + $this->enableAdminEmail2FA(); + $this->createUserEmailIdentity(); + + $result = $this->loginUser(); + + $result->assertStatus(302); + $result->assertRedirect(); + $this->assertSame(site_url(), $result->getRedirectUrl()); + } + + public function testLoginIgnoresStoredConditionalActionIdentityWhenItDoesNotApply(): void + { + $this->enableAdminEmail2FA(); + $this->createUserEmailIdentity(); + + model(UserIdentityModel::class)->insert([ + 'user_id' => $this->user->id, + 'type' => 'email_2fa', + 'name' => 'login', + 'secret' => '123456', + 'extra' => lang('Auth.need2FA'), + ]); + + $result = $this->loginUser(); + + $result->assertStatus(302); + $result->assertRedirect(); + $this->assertSame(site_url(), $result->getRedirectUrl()); + } + + public function testLoginKeepsExistingPendingConditionalActionInSession(): void + { + $this->enableAdminEmail2FA(); + $this->createUserEmailIdentity(); + + $result = $this->withSession([ + 'user' => [ + 'id' => $this->user->id, + 'auth_action' => AdminEmail2FA::class, + ], + ])->get('/login'); + + $result->assertStatus(302); + $result->assertRedirect(); + $this->assertSame(site_url('auth/a/show'), $result->getRedirectUrl()); + } + + private function enableAdminEmail2FA(): void + { + $config = config('Auth'); + $config->actions['login'] = AdminEmail2FA::class; + Factories::injectMock('config', 'Auth', $config); + } + + private function createUserEmailIdentity(): void + { + $this->user->createEmailIdentity([ + 'email' => 'foo@example.com', + 'password' => 'secret123', + ]); + } + + private function loginUser(): TestResponse + { + return $this->post('/login', [ + 'email' => 'foo@example.com', + 'password' => 'secret123', + ]); + } } diff --git a/tests/Controllers/MagicLinkTest.php b/tests/Controllers/MagicLinkTest.php index 97a4ebbd2..83d031d57 100644 --- a/tests/Controllers/MagicLinkTest.php +++ b/tests/Controllers/MagicLinkTest.php @@ -24,6 +24,7 @@ use CodeIgniter\Test\DatabaseTestTrait; use CodeIgniter\Test\FeatureTestTrait; use Config\Services; +use Tests\Support\AdminEmailActivator; use Tests\Support\FakeUser; use Tests\Support\TestCase; @@ -133,6 +134,47 @@ public function testMagicLinkVerifyPendingRegistrationActivation(): void $this->assertFalse(auth()->loggedIn()); } + public function testMagicLinkVerifyPendingConditionalRegistrationActivation(): void + { + $config = config('Auth'); + $config->actions['register'] = AdminEmailActivator::class; + Factories::injectMock('config', 'Auth', $config); + + /** @var User $user */ + $user = fake(UserModel::class, ['active' => false]); + $user->createEmailIdentity(['email' => 'foo@example.com', 'password' => 'secret123']); + + $identities = model(UserIdentityModel::class); + + $identities->insert([ + 'user_id' => $user->id, + 'type' => Session::ID_TYPE_EMAIL_ACTIVATE, + 'secret' => '123456', + 'name' => 'register', + 'extra' => lang('Auth.needVerification'), + ]); + $identities->insert([ + 'user_id' => $user->id, + 'type' => Session::ID_TYPE_MAGIC_LINK, + 'secret' => 'validtoken123', + 'expires' => Time::now()->addMinutes(60), + ]); + + $result = $this->get(route_to('verify-magic-link') . '?token=validtoken123'); + + $result->assertRedirectTo(route_to('auth-action-show')); + $result->assertSessionHas('error', lang('Auth.needActivate')); + $result->assertSessionHas( + 'user', + [ + 'id' => $user->id, + 'auth_action' => AdminEmailActivator::class, + 'auth_action_message' => lang('Auth.needVerification'), + ], + ); + $this->assertFalse(auth()->loggedIn()); + } + public function testBackToLoginLinkOnPage(): void { $result = $this->get('/login/magic-link'); diff --git a/tests/Controllers/RegisterTest.php b/tests/Controllers/RegisterTest.php index 9a56f7bca..bfc92b2bf 100644 --- a/tests/Controllers/RegisterTest.php +++ b/tests/Controllers/RegisterTest.php @@ -22,6 +22,7 @@ use CodeIgniter\Shield\Models\UserModel; use CodeIgniter\Test\FeatureTestTrait; use Config\Services; +use Tests\Support\AdminEmailActivator; use Tests\Support\DatabaseTestCase; use Tests\Support\FakeUser; @@ -205,6 +206,53 @@ public function testRegisterRedirectsToActionIfDefined(): void ]); } + public function testRegisterSkipsConditionalActionWhenItDoesNotApply(): void + { + $config = config('Auth'); + $config->actions['register'] = AdminEmailActivator::class; + Factories::injectMock('config', 'Auth', $config); + + $result = $this->post('/register', [ + 'email' => 'foo@example.com', + 'username' => 'foo', + 'password' => 'abkdhflkjsdflkjasd;lkjf', + 'password_confirm' => 'abkdhflkjsdflkjasd;lkjf', + ]); + + $result->assertRedirect(); + $this->assertSame(site_url(), $result->getRedirectUrl()); + + $this->seeInDatabase($this->tables['users'], [ + 'username' => 'foo', + 'active' => 1, + ]); + } + + public function testRegisterRedirectsToConditionalActionWhenItApplies(): void + { + $authConfig = config('Auth'); + $authConfig->actions['register'] = AdminEmailActivator::class; + Factories::injectMock('config', 'Auth', $authConfig); + + $groupsConfig = config('AuthGroups'); + $groupsConfig->defaultGroup = 'admin'; + Factories::injectMock('config', 'AuthGroups', $groupsConfig); + + $result = $this->post('/register', [ + 'email' => 'foo@example.com', + 'username' => 'foo', + 'password' => 'abkdhflkjsdflkjasd;lkjf', + 'password_confirm' => 'abkdhflkjsdflkjasd;lkjf', + ]); + + $result->assertRedirectTo('/auth/a/show'); + + $this->seeInDatabase($this->tables['users'], [ + 'username' => 'foo', + 'active' => 0, + ]); + } + public function testRegisteredButNotActivatedAndLogin(): void { // Ensure our action is defined diff --git a/tests/_support/AdminEmail2FA.php b/tests/_support/AdminEmail2FA.php new file mode 100644 index 000000000..68755b1d0 --- /dev/null +++ b/tests/_support/AdminEmail2FA.php @@ -0,0 +1,26 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace Tests\Support; + +use CodeIgniter\Shield\Authentication\Actions\ConditionalActionInterface; +use CodeIgniter\Shield\Authentication\Actions\Email2FA; +use CodeIgniter\Shield\Entities\User; + +final class AdminEmail2FA extends Email2FA implements ConditionalActionInterface +{ + public function appliesTo(User $user): bool + { + return $user->inGroup('admin', 'superadmin'); + } +} diff --git a/tests/_support/AdminEmailActivator.php b/tests/_support/AdminEmailActivator.php new file mode 100644 index 000000000..9c5f5fcc5 --- /dev/null +++ b/tests/_support/AdminEmailActivator.php @@ -0,0 +1,26 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace Tests\Support; + +use CodeIgniter\Shield\Authentication\Actions\ConditionalActionInterface; +use CodeIgniter\Shield\Authentication\Actions\EmailActivator; +use CodeIgniter\Shield\Entities\User; + +final class AdminEmailActivator extends EmailActivator implements ConditionalActionInterface +{ + public function appliesTo(User $user): bool + { + return $user->inGroup('admin', 'superadmin'); + } +}