Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/references/magic_link_login.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ in the **app/Config/Auth.php** file.
public int $magicLinkLifetime = HOUR;
```

### Bot Detection

Some apps or devices may try to be "too helpful" by automatically visiting links - for example, to check if they're safe or to prepare for read-aloud features. Since this is a one-time magic link, such automated visits could invalidate it. To prevent this, Shield relies on the framework's `UserAgents::robots` config property (**app/Config/UserAgents.php**) to filter out requests that are likely initiated by non-human agents.

## Responding to Magic Link Logins

!!! note
Expand Down
12 changes: 12 additions & 0 deletions phpstan-baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,18 @@
'count' => 1,
'path' => __DIR__ . '/tests/Authentication/HasAccessTokensTest.php',
];
$ignoreErrors[] = [
'rawMessage' => 'Accessing offset \'HTTP_USER_AGENT\' directly on $_SERVER is discouraged.',
'identifier' => 'codeigniter.superglobalAccess',
'count' => 1,
'path' => __DIR__ . '/tests/Controllers/MagicLinkTest.php',
];
$ignoreErrors[] = [
'rawMessage' => 'Assigning \'Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)\' directly on offset \'HTTP_USER_AGENT\' of $_SERVER is discouraged.',
'identifier' => 'codeigniter.superglobalAccessAssign',
'count' => 1,
'path' => __DIR__ . '/tests/Controllers/MagicLinkTest.php',
];
$ignoreErrors[] = [
'rawMessage' => 'Only booleans are allowed in a ternary operator condition, string|null given.',
'identifier' => 'ternary.condNotBoolean',
Expand Down
10 changes: 8 additions & 2 deletions rector.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@
use Rector\DeadCode\Rector\Cast\RecastingRemovalRector;
use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPromotedPropertyRector;
use Rector\DeadCode\Rector\If_\UnwrapFutureCompatibleIfPhpVersionRector;
use Rector\DeadCode\Rector\MethodCall\RemoveNullArgOnNullDefaultParamRector;
use Rector\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector;
use Rector\EarlyReturn\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector;
use Rector\EarlyReturn\Rector\If_\ChangeIfElseValueAssignToEarlyReturnRector;
use Rector\EarlyReturn\Rector\If_\RemoveAlwaysElseRector;
use Rector\EarlyReturn\Rector\Return_\PreparedValueToEarlyReturnRector;
use Rector\Php55\Rector\String_\StringClassNameToClassConstantRector;
use Rector\Php73\Rector\FuncCall\StringifyStrNeedlesRector;
use Rector\Php81\Rector\ClassMethod\NewInInitializerRector;
use Rector\PHPUnit\AnnotationsToAttributes\Rector\Class_\AnnotationWithValueToAttributeRector;
use Rector\PHPUnit\CodeQuality\Rector\Class_\YieldDataProviderRector;
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\AssertEmptyNullableObjectToAssertInstanceofRector;
Expand All @@ -52,7 +54,6 @@
use Rector\Set\ValueObject\LevelSetList;
use Rector\Set\ValueObject\SetList;
use Rector\Strict\Rector\Empty_\DisallowedEmptyRuleFixerRector;
use Rector\Strict\Rector\If_\BooleanInIfConditionRuleFixerRector;
use Rector\TypeDeclaration\Rector\Empty_\EmptyOnNullableObjectToInstanceOfRector;
use Rector\ValueObject\PhpVersion;

Expand Down Expand Up @@ -135,6 +136,12 @@

// Ignore some PHPUnit rules
AssertEmptyNullableObjectToAssertInstanceofRector::class,

// Ignore to prevent BC break
NewInInitializerRector::class,

// Ignore for readability
RemoveNullArgOnNullDefaultParamRector::class,
]);

// auto import fully qualified class names
Expand Down Expand Up @@ -166,7 +173,6 @@
$rectorConfig->rule(StringClassNameToClassConstantRector::class);
$rectorConfig->rule(PrivatizeFinalClassPropertyRector::class);
$rectorConfig->rule(CompleteDynamicPropertiesRector::class);
$rectorConfig->rule(BooleanInIfConditionRuleFixerRector::class);
$rectorConfig->rule(SingleInArrayToCompareRector::class);
$rectorConfig->rule(VersionCompareFuncCallToConstantRector::class);
$rectorConfig->rule(ExplicitBoolCompareRector::class);
Expand Down
5 changes: 5 additions & 0 deletions src/Controllers/MagicLinkController.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

use App\Controllers\BaseController;
use CodeIgniter\Events\Events;
use CodeIgniter\Exceptions\PageNotFoundException;
use CodeIgniter\HTTP\IncomingRequest;
use CodeIgniter\HTTP\RedirectResponse;
use CodeIgniter\I18n\Time;
Expand Down Expand Up @@ -160,6 +161,10 @@ public function verify(): RedirectResponse
return redirect()->route('login')->with('error', lang('Auth.magicLinkDisabled'));
}

if ($this->request->getUserAgent()->isRobot()) {
throw PageNotFoundException::forPageNotFound();
}

$token = $this->request->getGet('token');

/** @var UserIdentityModel $identityModel */
Expand Down
2 changes: 1 addition & 1 deletion src/Models/GroupModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public function getGroupsByUserIds(array $userIds): array
->getResultArray();

return array_map(
'array_keys',
array_keys(...),
array_reduce($groups, static function ($carry, $item) {
$carry[$item['user_id']][$item['group']] = true;

Expand Down
2 changes: 1 addition & 1 deletion src/Models/PermissionModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function getPermissionsByUserIds(array $userIds): array
->getResultArray();

return array_map(
'array_keys',
array_keys(...),
array_reduce($permissions, static function ($carry, $item) {
$carry[$item['user_id']][$item['permission']] = true;

Expand Down
4 changes: 2 additions & 2 deletions tests/Collectors/AuthTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public function testDisplayNotLoggedIn(): void
public function testDisplayLoggedIn(): void
{
$authenticator = service('auth')->getAuthenticator();
assert($authenticator instanceof Session);
$this->assertInstanceOf(Session::class, $authenticator);
$authenticator->login($this->user);
$this->user->addGroup('admin', 'beta');
$this->user->addPermission('users.create', 'users.edit');
Expand All @@ -68,7 +68,7 @@ public function testDisplayLoggedIn(): void
public function testDisplayNotLoggedInAfterLogout(): void
{
$authenticator = service('auth')->getAuthenticator();
assert($authenticator instanceof Session);
$this->assertInstanceOf(Session::class, $authenticator);
$authenticator->login($this->user);

$authenticator->logout();
Expand Down
33 changes: 33 additions & 0 deletions tests/Controllers/MagicLinkTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
namespace Tests\Controllers;

use CodeIgniter\Config\Factories;
use CodeIgniter\Exceptions\PageNotFoundException;
use CodeIgniter\I18n\Time;
use CodeIgniter\Shield\Authentication\Actions\EmailActivator;
use CodeIgniter\Shield\Authentication\Authenticators\Session;
Expand Down Expand Up @@ -47,6 +48,14 @@ protected function setUp(): void
Services::injectMock('routes', $routes);
}

protected function tearDown(): void
{
parent::tearDown();

// Clean up any robot user agent set in tests
unset($_SERVER['HTTP_USER_AGENT']);
}

public function testAfterLoggedInNotAllowDisplayMagicLink(): void
{
$this->user->createEmailIdentity([
Expand Down Expand Up @@ -177,4 +186,28 @@ public function testMagicLinkVerifyRedirectsIfNotAllowed(): void
lang('Auth.magicLinkDisabled'),
);
}

public function testMagicLinkVerifyReturns404ForRobotUserAgent(): void
{
$this->expectException(PageNotFoundException::class);

/** @var User $user */
$user = fake(UserModel::class);
$user->createEmailIdentity(['email' => 'foo@example.com', 'password' => 'secret123']);

$identities = model(UserIdentityModel::class);

// Insert User Identity for Magic link login
$identities->insert([
'user_id' => $user->id,
'type' => Session::ID_TYPE_MAGIC_LINK,
'secret' => 'validtoken123',
'expires' => Time::now()->addMinutes(60),
]);

// Simulate a robot user agent
$_SERVER['HTTP_USER_AGENT'] = 'Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)';

$this->get(route_to('verify-magic-link') . '?token=validtoken123');
}
}
Loading