Skip to content

Commit 93091f3

Browse files
authored
feat: ignore robots when accessing magic link (#1294)
1 parent 8bf6f42 commit 93091f3

File tree

8 files changed

+66
-6
lines changed

8 files changed

+66
-6
lines changed

docs/references/magic_link_login.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ in the **app/Config/Auth.php** file.
2323
public int $magicLinkLifetime = HOUR;
2424
```
2525

26+
### Bot Detection
27+
28+
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.
29+
2630
## Responding to Magic Link Logins
2731

2832
!!! note

phpstan-baseline.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,18 @@
480480
'count' => 1,
481481
'path' => __DIR__ . '/tests/Authentication/HasAccessTokensTest.php',
482482
];
483+
$ignoreErrors[] = [
484+
'rawMessage' => 'Accessing offset \'HTTP_USER_AGENT\' directly on $_SERVER is discouraged.',
485+
'identifier' => 'codeigniter.superglobalAccess',
486+
'count' => 1,
487+
'path' => __DIR__ . '/tests/Controllers/MagicLinkTest.php',
488+
];
489+
$ignoreErrors[] = [
490+
'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.',
491+
'identifier' => 'codeigniter.superglobalAccessAssign',
492+
'count' => 1,
493+
'path' => __DIR__ . '/tests/Controllers/MagicLinkTest.php',
494+
];
483495
$ignoreErrors[] = [
484496
'rawMessage' => 'Only booleans are allowed in a ternary operator condition, string|null given.',
485497
'identifier' => 'ternary.condNotBoolean',

rector.php

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,15 @@
3737
use Rector\DeadCode\Rector\Cast\RecastingRemovalRector;
3838
use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPromotedPropertyRector;
3939
use Rector\DeadCode\Rector\If_\UnwrapFutureCompatibleIfPhpVersionRector;
40+
use Rector\DeadCode\Rector\MethodCall\RemoveNullArgOnNullDefaultParamRector;
4041
use Rector\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector;
4142
use Rector\EarlyReturn\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector;
4243
use Rector\EarlyReturn\Rector\If_\ChangeIfElseValueAssignToEarlyReturnRector;
4344
use Rector\EarlyReturn\Rector\If_\RemoveAlwaysElseRector;
4445
use Rector\EarlyReturn\Rector\Return_\PreparedValueToEarlyReturnRector;
4546
use Rector\Php55\Rector\String_\StringClassNameToClassConstantRector;
4647
use Rector\Php73\Rector\FuncCall\StringifyStrNeedlesRector;
48+
use Rector\Php81\Rector\ClassMethod\NewInInitializerRector;
4749
use Rector\PHPUnit\AnnotationsToAttributes\Rector\Class_\AnnotationWithValueToAttributeRector;
4850
use Rector\PHPUnit\CodeQuality\Rector\Class_\YieldDataProviderRector;
4951
use Rector\PHPUnit\CodeQuality\Rector\MethodCall\AssertEmptyNullableObjectToAssertInstanceofRector;
@@ -52,7 +54,6 @@
5254
use Rector\Set\ValueObject\LevelSetList;
5355
use Rector\Set\ValueObject\SetList;
5456
use Rector\Strict\Rector\Empty_\DisallowedEmptyRuleFixerRector;
55-
use Rector\Strict\Rector\If_\BooleanInIfConditionRuleFixerRector;
5657
use Rector\TypeDeclaration\Rector\Empty_\EmptyOnNullableObjectToInstanceOfRector;
5758
use Rector\ValueObject\PhpVersion;
5859

@@ -135,6 +136,12 @@
135136

136137
// Ignore some PHPUnit rules
137138
AssertEmptyNullableObjectToAssertInstanceofRector::class,
139+
140+
// Ignore to prevent BC break
141+
NewInInitializerRector::class,
142+
143+
// Ignore for readability
144+
RemoveNullArgOnNullDefaultParamRector::class,
138145
]);
139146

140147
// auto import fully qualified class names
@@ -166,7 +173,6 @@
166173
$rectorConfig->rule(StringClassNameToClassConstantRector::class);
167174
$rectorConfig->rule(PrivatizeFinalClassPropertyRector::class);
168175
$rectorConfig->rule(CompleteDynamicPropertiesRector::class);
169-
$rectorConfig->rule(BooleanInIfConditionRuleFixerRector::class);
170176
$rectorConfig->rule(SingleInArrayToCompareRector::class);
171177
$rectorConfig->rule(VersionCompareFuncCallToConstantRector::class);
172178
$rectorConfig->rule(ExplicitBoolCompareRector::class);

src/Controllers/MagicLinkController.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
use App\Controllers\BaseController;
1717
use CodeIgniter\Events\Events;
18+
use CodeIgniter\Exceptions\PageNotFoundException;
1819
use CodeIgniter\HTTP\IncomingRequest;
1920
use CodeIgniter\HTTP\RedirectResponse;
2021
use CodeIgniter\I18n\Time;
@@ -160,6 +161,10 @@ public function verify(): RedirectResponse
160161
return redirect()->route('login')->with('error', lang('Auth.magicLinkDisabled'));
161162
}
162163

164+
if ($this->request->getUserAgent()->isRobot()) {
165+
throw PageNotFoundException::forPageNotFound();
166+
}
167+
163168
$token = $this->request->getGet('token');
164169

165170
/** @var UserIdentityModel $identityModel */

src/Models/GroupModel.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public function getGroupsByUserIds(array $userIds): array
9898
->getResultArray();
9999

100100
return array_map(
101-
'array_keys',
101+
array_keys(...),
102102
array_reduce($groups, static function ($carry, $item) {
103103
$carry[$item['user_id']][$item['group']] = true;
104104

src/Models/PermissionModel.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public function getPermissionsByUserIds(array $userIds): array
8888
->getResultArray();
8989

9090
return array_map(
91-
'array_keys',
91+
array_keys(...),
9292
array_reduce($permissions, static function ($carry, $item) {
9393
$carry[$item['user_id']][$item['permission']] = true;
9494

tests/Collectors/AuthTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public function testDisplayNotLoggedIn(): void
5252
public function testDisplayLoggedIn(): void
5353
{
5454
$authenticator = service('auth')->getAuthenticator();
55-
assert($authenticator instanceof Session);
55+
$this->assertInstanceOf(Session::class, $authenticator);
5656
$authenticator->login($this->user);
5757
$this->user->addGroup('admin', 'beta');
5858
$this->user->addPermission('users.create', 'users.edit');
@@ -68,7 +68,7 @@ public function testDisplayLoggedIn(): void
6868
public function testDisplayNotLoggedInAfterLogout(): void
6969
{
7070
$authenticator = service('auth')->getAuthenticator();
71-
assert($authenticator instanceof Session);
71+
$this->assertInstanceOf(Session::class, $authenticator);
7272
$authenticator->login($this->user);
7373

7474
$authenticator->logout();

tests/Controllers/MagicLinkTest.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
namespace Tests\Controllers;
1515

1616
use CodeIgniter\Config\Factories;
17+
use CodeIgniter\Exceptions\PageNotFoundException;
1718
use CodeIgniter\I18n\Time;
1819
use CodeIgniter\Shield\Authentication\Actions\EmailActivator;
1920
use CodeIgniter\Shield\Authentication\Authenticators\Session;
@@ -47,6 +48,14 @@ protected function setUp(): void
4748
Services::injectMock('routes', $routes);
4849
}
4950

51+
protected function tearDown(): void
52+
{
53+
parent::tearDown();
54+
55+
// Clean up any robot user agent set in tests
56+
unset($_SERVER['HTTP_USER_AGENT']);
57+
}
58+
5059
public function testAfterLoggedInNotAllowDisplayMagicLink(): void
5160
{
5261
$this->user->createEmailIdentity([
@@ -177,4 +186,28 @@ public function testMagicLinkVerifyRedirectsIfNotAllowed(): void
177186
lang('Auth.magicLinkDisabled'),
178187
);
179188
}
189+
190+
public function testMagicLinkVerifyReturns404ForRobotUserAgent(): void
191+
{
192+
$this->expectException(PageNotFoundException::class);
193+
194+
/** @var User $user */
195+
$user = fake(UserModel::class);
196+
$user->createEmailIdentity(['email' => 'foo@example.com', 'password' => 'secret123']);
197+
198+
$identities = model(UserIdentityModel::class);
199+
200+
// Insert User Identity for Magic link login
201+
$identities->insert([
202+
'user_id' => $user->id,
203+
'type' => Session::ID_TYPE_MAGIC_LINK,
204+
'secret' => 'validtoken123',
205+
'expires' => Time::now()->addMinutes(60),
206+
]);
207+
208+
// Simulate a robot user agent
209+
$_SERVER['HTTP_USER_AGENT'] = 'Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)';
210+
211+
$this->get(route_to('verify-magic-link') . '?token=validtoken123');
212+
}
180213
}

0 commit comments

Comments
 (0)