diff --git a/docs/references/magic_link_login.md b/docs/references/magic_link_login.md index 942f08aa8..e18edbd4e 100644 --- a/docs/references/magic_link_login.md +++ b/docs/references/magic_link_login.md @@ -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 diff --git a/phpstan-baseline.php b/phpstan-baseline.php index d0463dd69..ce455efb9 100644 --- a/phpstan-baseline.php +++ b/phpstan-baseline.php @@ -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', diff --git a/rector.php b/rector.php index 82f12653b..7058e982f 100644 --- a/rector.php +++ b/rector.php @@ -37,6 +37,7 @@ 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; @@ -44,6 +45,7 @@ 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; @@ -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; @@ -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 @@ -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); diff --git a/src/Controllers/MagicLinkController.php b/src/Controllers/MagicLinkController.php index bb203f9c6..cd399803d 100644 --- a/src/Controllers/MagicLinkController.php +++ b/src/Controllers/MagicLinkController.php @@ -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; @@ -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 */ diff --git a/src/Models/GroupModel.php b/src/Models/GroupModel.php index ea41d02a8..d0324f6ae 100644 --- a/src/Models/GroupModel.php +++ b/src/Models/GroupModel.php @@ -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; diff --git a/src/Models/PermissionModel.php b/src/Models/PermissionModel.php index 553818640..3f2e0f004 100644 --- a/src/Models/PermissionModel.php +++ b/src/Models/PermissionModel.php @@ -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; diff --git a/tests/Collectors/AuthTest.php b/tests/Collectors/AuthTest.php index cba81aacc..47e856abe 100644 --- a/tests/Collectors/AuthTest.php +++ b/tests/Collectors/AuthTest.php @@ -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'); @@ -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(); diff --git a/tests/Controllers/MagicLinkTest.php b/tests/Controllers/MagicLinkTest.php index fce1d4600..ac7bab979 100644 --- a/tests/Controllers/MagicLinkTest.php +++ b/tests/Controllers/MagicLinkTest.php @@ -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; @@ -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([ @@ -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'); + } }