From 926365911623216c4a6fb945d6fddc3622f00df7 Mon Sep 17 00:00:00 2001 From: memleakd <121398829+memleakd@users.noreply.github.com> Date: Sat, 30 May 2026 08:47:23 +0200 Subject: [PATCH] fix(auth): require login actions for magic links - Start configured login actions during magic-link verification - Preserve magicLogin notification until pending actions complete - Cover normal and conditional login-action magic-link flows - Document that magic-link login finishes after required login actions Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com> --- docs/references/magic_link_login.md | 2 + src/Authentication/Authenticators/Session.php | 27 +++++ src/Controllers/MagicLinkController.php | 13 ++- tests/Controllers/MagicLinkTest.php | 103 ++++++++++++++++++ 4 files changed, 144 insertions(+), 1 deletion(-) diff --git a/docs/references/magic_link_login.md b/docs/references/magic_link_login.md index e18edbd4e..bd1518fbd 100644 --- a/docs/references/magic_link_login.md +++ b/docs/references/magic_link_login.md @@ -35,6 +35,8 @@ Some apps or devices may try to be "too helpful" by automatically visiting links Magic Link logins allow a user that has forgotten their password to have an email sent with a unique, one-time login link. Once they've logged in you can decide how to respond. In some cases, you might want to redirect them to a special page where they must choose a new password. In other cases, you might simply want to display a one-time message prompting them to go to their account page and choose a new password. +If a login auth action is configured, such as Email-based Two Factor Authentication, the user must complete that action before the magic link login is finished. + ### Session Notification You can detect if a user has finished the magic link login by checking for a session value, `magicLogin`. If they have recently completed the flow, it will exist and have a value of `true`. diff --git a/src/Authentication/Authenticators/Session.php b/src/Authentication/Authenticators/Session.php index 08df82407..05e342244 100644 --- a/src/Authentication/Authenticators/Session.php +++ b/src/Authentication/Authenticators/Session.php @@ -57,6 +57,10 @@ class Session implements AuthenticatorInterface private const STATE_PENDING = 2; // 2FA or Activation required. private const STATE_LOGGED_IN = 3; + // Magic-link session markers + private const MAGIC_LOGIN_TEMP_DATA = 'magicLogin'; + private const PENDING_MAGIC_LINK_LOGIN = 'auth_action_magic_login'; + /** * Authenticated or authenticating (pending login) User */ @@ -270,6 +274,29 @@ public function completeLogin(User $user): void // a successful login Events::trigger('login', $user); + + // Complete the magic-link notification after any pending login action. + $this->completeMagicLinkLoginIfPending(); + } + + /** + * Marks the pending login action as originating from a magic-link login. + */ + public function markPendingLoginAsMagicLink(): void + { + $this->setSessionUserKey(self::PENDING_MAGIC_LINK_LOGIN, self::MAGIC_LOGIN_TEMP_DATA); + } + + private function completeMagicLinkLoginIfPending(): void + { + if ($this->getSessionUserKey(self::PENDING_MAGIC_LINK_LOGIN) !== self::MAGIC_LOGIN_TEMP_DATA) { + return; + } + + $this->removeSessionUserKey(self::PENDING_MAGIC_LINK_LOGIN); + session()->setTempdata(self::MAGIC_LOGIN_TEMP_DATA, true); + + Events::trigger('magicLogin'); } /** diff --git a/src/Controllers/MagicLinkController.php b/src/Controllers/MagicLinkController.php index cd399803d..53c285ad9 100644 --- a/src/Controllers/MagicLinkController.php +++ b/src/Controllers/MagicLinkController.php @@ -20,6 +20,7 @@ use CodeIgniter\HTTP\RedirectResponse; use CodeIgniter\I18n\Time; use CodeIgniter\Shield\Authentication\Authenticators\Session; +use CodeIgniter\Shield\Entities\User; use CodeIgniter\Shield\Models\LoginModel; use CodeIgniter\Shield\Models\UserIdentityModel; use CodeIgniter\Shield\Models\UserModel; @@ -205,7 +206,17 @@ public function verify(): RedirectResponse return redirect()->route('auth-action-show')->with('error', lang('Auth.needActivate')); } - // Log the user in + $user = $this->provider->findById($identity->user_id); + + // Start any login action that has been defined. + if ($user instanceof User && $authenticator->startUpAction('login', $user) && $authenticator->hasAction($user->id)) { + $this->recordLoginAttempt($identifier, true, $user->id); + $authenticator->markPendingLoginAsMagicLink(); + + return redirect()->route('auth-action-show'); + } + + // Log the user in. $authenticator->loginById($identity->user_id); $user = $authenticator->getUser(); diff --git a/tests/Controllers/MagicLinkTest.php b/tests/Controllers/MagicLinkTest.php index 83d031d57..e39eb1431 100644 --- a/tests/Controllers/MagicLinkTest.php +++ b/tests/Controllers/MagicLinkTest.php @@ -16,6 +16,7 @@ use CodeIgniter\Config\Factories; use CodeIgniter\Exceptions\PageNotFoundException; use CodeIgniter\I18n\Time; +use CodeIgniter\Shield\Authentication\Actions\Email2FA; use CodeIgniter\Shield\Authentication\Actions\EmailActivator; use CodeIgniter\Shield\Authentication\Authenticators\Session; use CodeIgniter\Shield\Entities\User; @@ -24,6 +25,7 @@ use CodeIgniter\Test\DatabaseTestTrait; use CodeIgniter\Test\FeatureTestTrait; use Config\Services; +use Tests\Support\AdminEmail2FA; use Tests\Support\AdminEmailActivator; use Tests\Support\FakeUser; use Tests\Support\TestCase; @@ -175,6 +177,85 @@ public function testMagicLinkVerifyPendingConditionalRegistrationActivation(): v $this->assertFalse(auth()->loggedIn()); } + public function testMagicLinkVerifyStartsLoginAction(): void + { + setting('Auth.actions', ['login' => Email2FA::class, 'register' => null]); + + /** @var User $user */ + $user = fake(UserModel::class); + $user->createEmailIdentity(['email' => 'foo@example.com', 'password' => 'secret123']); + + $this->insertMagicLinkIdentity($user, 'validtoken123'); + + $result = $this->get(route_to('verify-magic-link') . '?token=validtoken123'); + + $result->assertRedirect(); + $this->assertSame(site_url('/auth/a/show'), $result->getRedirectUrl()); + $this->assertPendingLoginAction($user, Email2FA::class); + $this->seeInDatabase(config('Auth')->tables['identities'], [ + 'user_id' => $user->id, + 'type' => Session::ID_TYPE_EMAIL_2FA, + 'name' => 'login', + 'extra' => lang('Auth.need2FA'), + ]); + $result->assertSessionMissing('magicLogin'); + $this->assertFalse(auth()->loggedIn()); + + $identity = model(UserIdentityModel::class)->getIdentityByType($user, Session::ID_TYPE_EMAIL_2FA); + $this->assertNotNull($identity); + + $result = $this->withSession()->post('/auth/a/verify', [ + 'token' => $identity->secret, + ]); + + $result->assertRedirectTo(config('Auth')->loginRedirect()); + $result->assertSessionHas('user', ['id' => $user->id]); + $result->assertSessionHas('magicLogin', true); + $this->assertTrue(auth()->loggedIn()); + } + + public function testMagicLinkVerifyStartsConditionalLoginActionWhenItApplies(): void + { + setting('Auth.actions', ['login' => AdminEmail2FA::class, 'register' => null]); + + /** @var User $user */ + $user = fake(UserModel::class); + $user->addGroup('admin'); + $user->createEmailIdentity(['email' => 'foo@example.com', 'password' => 'secret123']); + + $this->insertMagicLinkIdentity($user, 'validtoken123'); + + $result = $this->get(route_to('verify-magic-link') . '?token=validtoken123'); + + $result->assertRedirect(); + $this->assertSame(site_url('/auth/a/show'), $result->getRedirectUrl()); + $this->assertPendingLoginAction($user, AdminEmail2FA::class); + $result->assertSessionMissing('magicLogin'); + $this->assertFalse(auth()->loggedIn()); + } + + public function testMagicLinkVerifySkipsConditionalLoginActionWhenItDoesNotApply(): void + { + setting('Auth.actions', ['login' => AdminEmail2FA::class, 'register' => null]); + + /** @var User $user */ + $user = fake(UserModel::class); + $user->createEmailIdentity(['email' => 'foo@example.com', 'password' => 'secret123']); + + $this->insertMagicLinkIdentity($user, 'validtoken123'); + + $result = $this->get(route_to('verify-magic-link') . '?token=validtoken123'); + + $result->assertRedirectTo(config('Auth')->loginRedirect()); + $result->assertSessionHas('user', ['id' => $user->id]); + $result->assertSessionMissing('auth_action'); + $this->assertTrue(auth()->loggedIn()); + $this->dontSeeInDatabase(config('Auth')->tables['identities'], [ + 'user_id' => $user->id, + 'type' => Session::ID_TYPE_EMAIL_2FA, + ]); + } + public function testBackToLoginLinkOnPage(): void { $result = $this->get('/login/magic-link'); @@ -252,4 +333,26 @@ public function testMagicLinkVerifyReturns404ForRobotUserAgent(): void $this->get(route_to('verify-magic-link') . '?token=validtoken123'); } + + private function insertMagicLinkIdentity(User $user, string $token): void + { + model(UserIdentityModel::class)->insert([ + 'user_id' => $user->id, + 'type' => Session::ID_TYPE_MAGIC_LINK, + 'secret' => $token, + 'expires' => Time::now()->addMinutes(60), + ]); + } + + /** + * @param class-string $action + */ + private function assertPendingLoginAction(User $user, string $action): void + { + $sessionUser = session('user'); + $this->assertIsArray($sessionUser); + $this->assertSame($user->id, $sessionUser['id'] ?? null); + $this->assertSame($action, $sessionUser['auth_action'] ?? null); + $this->assertSame(lang('Auth.need2FA'), $sessionUser['auth_action_message'] ?? null); + } }