Skip to content

Commit 82e825a

Browse files
datamwebdatamweb
authored
Merge pull request codeigniter4#1329 from memleakd/fix/magic-link-login-actions
fix: require login actions for magic links
2 parents 5577529 + 272142c commit 82e825a

4 files changed

Lines changed: 150 additions & 7 deletions

File tree

‎docs/references/magic_link_login.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ Some apps or devices may try to be "too helpful" by automatically visiting links
3535

3636
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.
3737

38+
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.
39+
3840
### Session Notification
3941

4042
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`.

‎src/Authentication/Authenticators/Session.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ class Session implements AuthenticatorInterface
5757
private const STATE_PENDING = 2; // 2FA or Activation required.
5858
private const STATE_LOGGED_IN = 3;
5959

60+
// Passwordless login session markers
61+
private const MAGIC_LOGIN_TEMP_DATA = 'magicLogin';
62+
private const PENDING_LOGIN_METHOD = 'auth_action_login_method';
63+
6064
/**
6165
* Authenticated or authenticating (pending login) User
6266
*/
@@ -270,6 +274,34 @@ public function completeLogin(User $user): void
270274

271275
// a successful login
272276
Events::trigger('login', $user);
277+
278+
// Complete the passwordless login notification after any pending login action.
279+
$this->completePendingLoginMethod();
280+
}
281+
282+
/**
283+
* Marks the pending login action as originating from a passwordless login method.
284+
*/
285+
public function setPendingLoginMethod(string $method): void
286+
{
287+
$this->setSessionUserKey(self::PENDING_LOGIN_METHOD, $method);
288+
}
289+
290+
private function completePendingLoginMethod(): void
291+
{
292+
$method = $this->getSessionUserKey(self::PENDING_LOGIN_METHOD);
293+
294+
if ($method === null) {
295+
return;
296+
}
297+
298+
$this->removeSessionUserKey(self::PENDING_LOGIN_METHOD);
299+
300+
if ($method === self::ID_TYPE_MAGIC_LINK) {
301+
session()->setTempdata(self::MAGIC_LOGIN_TEMP_DATA, true);
302+
303+
Events::trigger('magicLogin');
304+
}
273305
}
274306

275307
/**

‎src/Controllers/MagicLinkController.php‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use CodeIgniter\HTTP\RedirectResponse;
2121
use CodeIgniter\I18n\Time;
2222
use CodeIgniter\Shield\Authentication\Authenticators\Session;
23+
use CodeIgniter\Shield\Entities\User;
2324
use CodeIgniter\Shield\Models\LoginModel;
2425
use CodeIgniter\Shield\Models\UserIdentityModel;
2526
use CodeIgniter\Shield\Models\UserModel;
@@ -205,19 +206,25 @@ public function verify(): RedirectResponse
205206
return redirect()->route('auth-action-show')->with('error', lang('Auth.needActivate'));
206207
}
207208

208-
// Log the user in
209+
$user = $this->provider->findById($identity->user_id);
210+
211+
// Start any login action that has been defined.
212+
if ($user instanceof User && $authenticator->startUpAction('login', $user) && $authenticator->hasAction($user->id)) {
213+
$this->recordLoginAttempt($identifier, true, $user->id);
214+
$authenticator->setPendingLoginMethod(Session::ID_TYPE_MAGIC_LINK);
215+
216+
return redirect()->route('auth-action-show');
217+
}
218+
219+
$authenticator->setPendingLoginMethod(Session::ID_TYPE_MAGIC_LINK);
220+
221+
// Log the user in.
209222
$authenticator->loginById($identity->user_id);
210223

211224
$user = $authenticator->getUser();
212225

213226
$this->recordLoginAttempt($identifier, true, $user->id);
214227

215-
// Give the developer a way to know the user
216-
// logged in via a magic link.
217-
session()->setTempdata('magicLogin', true);
218-
219-
Events::trigger('magicLogin');
220-
221228
// Get our login redirect url
222229
return redirect()->to(config('Auth')->loginRedirect());
223230
}

‎tests/Controllers/MagicLinkTest.php‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use CodeIgniter\Config\Factories;
1717
use CodeIgniter\Exceptions\PageNotFoundException;
1818
use CodeIgniter\I18n\Time;
19+
use CodeIgniter\Shield\Authentication\Actions\Email2FA;
1920
use CodeIgniter\Shield\Authentication\Actions\EmailActivator;
2021
use CodeIgniter\Shield\Authentication\Authenticators\Session;
2122
use CodeIgniter\Shield\Entities\User;
@@ -24,6 +25,7 @@
2425
use CodeIgniter\Test\DatabaseTestTrait;
2526
use CodeIgniter\Test\FeatureTestTrait;
2627
use Config\Services;
28+
use Tests\Support\AdminEmail2FA;
2729
use Tests\Support\AdminEmailActivator;
2830
use Tests\Support\FakeUser;
2931
use Tests\Support\TestCase;
@@ -175,6 +177,84 @@ public function testMagicLinkVerifyPendingConditionalRegistrationActivation(): v
175177
$this->assertFalse(auth()->loggedIn());
176178
}
177179

180+
public function testMagicLinkVerifyStartsLoginAction(): void
181+
{
182+
setting('Auth.actions', ['login' => Email2FA::class, 'register' => null]);
183+
184+
/** @var User $user */
185+
$user = fake(UserModel::class);
186+
$user->createEmailIdentity(['email' => 'foo@example.com', 'password' => 'secret123']);
187+
188+
$this->insertMagicLinkIdentity($user, 'validtoken123');
189+
190+
$result = $this->get(route_to('verify-magic-link') . '?token=validtoken123');
191+
192+
$result->assertRedirect();
193+
$this->assertSame(site_url('/auth/a/show'), $result->getRedirectUrl());
194+
$this->assertPendingLoginAction($user, Email2FA::class);
195+
$this->seeInDatabase(config('Auth')->tables['identities'], [
196+
'user_id' => $user->id,
197+
'type' => Session::ID_TYPE_EMAIL_2FA,
198+
'name' => 'login',
199+
]);
200+
$result->assertSessionMissing('magicLogin');
201+
$this->assertFalse(auth()->loggedIn());
202+
203+
$identity = model(UserIdentityModel::class)->getIdentityByType($user, Session::ID_TYPE_EMAIL_2FA);
204+
$this->assertNotNull($identity);
205+
206+
$result = $this->withSession()->post('/auth/a/verify', [
207+
'token' => $identity->secret,
208+
]);
209+
210+
$result->assertRedirectTo(config('Auth')->loginRedirect());
211+
$result->assertSessionHas('user', ['id' => $user->id]);
212+
$result->assertSessionHas('magicLogin', true);
213+
$this->assertTrue(auth()->loggedIn());
214+
}
215+
216+
public function testMagicLinkVerifyStartsConditionalLoginActionWhenItApplies(): void
217+
{
218+
setting('Auth.actions', ['login' => AdminEmail2FA::class, 'register' => null]);
219+
220+
/** @var User $user */
221+
$user = fake(UserModel::class);
222+
$user->addGroup('admin');
223+
$user->createEmailIdentity(['email' => 'foo@example.com', 'password' => 'secret123']);
224+
225+
$this->insertMagicLinkIdentity($user, 'validtoken123');
226+
227+
$result = $this->get(route_to('verify-magic-link') . '?token=validtoken123');
228+
229+
$result->assertRedirect();
230+
$this->assertSame(site_url('/auth/a/show'), $result->getRedirectUrl());
231+
$this->assertPendingLoginAction($user, AdminEmail2FA::class);
232+
$result->assertSessionMissing('magicLogin');
233+
$this->assertFalse(auth()->loggedIn());
234+
}
235+
236+
public function testMagicLinkVerifySkipsConditionalLoginActionWhenItDoesNotApply(): void
237+
{
238+
setting('Auth.actions', ['login' => AdminEmail2FA::class, 'register' => null]);
239+
240+
/** @var User $user */
241+
$user = fake(UserModel::class);
242+
$user->createEmailIdentity(['email' => 'foo@example.com', 'password' => 'secret123']);
243+
244+
$this->insertMagicLinkIdentity($user, 'validtoken123');
245+
246+
$result = $this->get(route_to('verify-magic-link') . '?token=validtoken123');
247+
248+
$result->assertRedirectTo(config('Auth')->loginRedirect());
249+
$result->assertSessionHas('user', ['id' => $user->id]);
250+
$result->assertSessionMissing('auth_action');
251+
$this->assertTrue(auth()->loggedIn());
252+
$this->dontSeeInDatabase(config('Auth')->tables['identities'], [
253+
'user_id' => $user->id,
254+
'type' => Session::ID_TYPE_EMAIL_2FA,
255+
]);
256+
}
257+
178258
public function testBackToLoginLinkOnPage(): void
179259
{
180260
$result = $this->get('/login/magic-link');
@@ -252,4 +332,26 @@ public function testMagicLinkVerifyReturns404ForRobotUserAgent(): void
252332

253333
$this->get(route_to('verify-magic-link') . '?token=validtoken123');
254334
}
335+
336+
private function insertMagicLinkIdentity(User $user, string $token): void
337+
{
338+
model(UserIdentityModel::class)->insert([
339+
'user_id' => $user->id,
340+
'type' => Session::ID_TYPE_MAGIC_LINK,
341+
'secret' => $token,
342+
'expires' => Time::now()->addMinutes(60),
343+
]);
344+
}
345+
346+
/**
347+
* @param class-string $action
348+
*/
349+
private function assertPendingLoginAction(User $user, string $action): void
350+
{
351+
$sessionUser = session('user');
352+
$this->assertIsArray($sessionUser);
353+
$this->assertSame($user->id, $sessionUser['id'] ?? null);
354+
$this->assertSame($action, $sessionUser['auth_action'] ?? null);
355+
$this->assertSame(lang('Auth.need2FA'), $sessionUser['auth_action_message'] ?? null);
356+
}
255357
}

0 commit comments

Comments
 (0)