diff --git a/core/lib/Module/Module.php b/core/lib/Module/Module.php index 685265f..90e78d2 100644 --- a/core/lib/Module/Module.php +++ b/core/lib/Module/Module.php @@ -52,8 +52,15 @@ class Module extends ModuleInstanceAbstract implements ModuleConsoleInterface, M priority: 100, ); - foreach ([ + $this->events->listen( + 'core', AuthenticationSucceededEvent::class, + FirewallService::class, + 'logAuthenticationSuccess', + DeliveryMode::Deferred, + ); + + foreach ([ AccessDeniedEvent::class, BruteForceDetectedEvent::class, RateLimitExceededEvent::class, diff --git a/core/lib/Security/AuthenticationManager.php b/core/lib/Security/AuthenticationManager.php index b7ec9b4..15a9d55 100644 --- a/core/lib/Security/AuthenticationManager.php +++ b/core/lib/Security/AuthenticationManager.php @@ -9,6 +9,7 @@ use KTXC\Resource\ProviderManager; use KTXC\Security\Authentication\AuthenticationRequest; use KTXC\Security\Authentication\AuthenticationResponse; use KTXC\Security\Event\AuthenticationFailedEvent; +use KTXC\Security\Event\AuthenticationSucceededEvent; use KTXC\Service\TokenService; use KTXC\Service\UserAccountsService; use KTXC\Context\TenantContextInterface; @@ -616,7 +617,16 @@ class AuthenticationManager */ private function completeAuthentication(AuthenticationSession $session): AuthenticationResponse { - $userData = $this->userService->fetchByIdentifier($session->userIdentifier); + $userId = $session->userIdentifier; + if ($userId === null) { + return AuthenticationResponse::failed( + AuthenticationResponse::ERROR_INVALID_SESSION, + 'Authenticated user is missing', + 401, + ); + } + + $userData = $this->userService->fetchByIdentifier($userId); if ($userData === null) { return AuthenticationResponse::failed( @@ -633,6 +643,11 @@ class AuthenticationManager $this->deleteSession($session->id); + $this->events->dispatch(new AuthenticationSucceededEvent( + $userId, + $session->tenantIdentifier, + )); + return AuthenticationResponse::success( $this->buildUserData($user), $tokens diff --git a/core/lib/Security/Event/AuthenticationSucceededEvent.php b/core/lib/Security/Event/AuthenticationSucceededEvent.php index bf229c0..bfb0213 100644 --- a/core/lib/Security/Event/AuthenticationSucceededEvent.php +++ b/core/lib/Security/Event/AuthenticationSucceededEvent.php @@ -6,17 +6,12 @@ namespace KTXC\Security\Event; use KTXF\Event\Event; -final class AuthenticationSucceededEvent extends Event implements SecurityRequestEventInterface +final class AuthenticationSucceededEvent extends Event implements SecurityEventInterface { public function __construct( - private readonly string $ipAddress, private readonly string $userId, ?string $tenantId = null, - private readonly ?string $deviceFingerprint = null, ) { - if ($ipAddress === '') { - throw new \InvalidArgumentException('Successful authentication requires an IP address.'); - } if ($userId === '') { throw new \InvalidArgumentException('Successful authentication requires a user ID.'); } @@ -28,31 +23,6 @@ final class AuthenticationSucceededEvent extends Event implements SecurityReques ); } - public function getIpAddress(): string - { - return $this->ipAddress; - } - - public function getDeviceFingerprint(): ?string - { - return $this->deviceFingerprint; - } - - public function getUserAgent(): ?string - { - return null; - } - - public function getRequestPath(): ?string - { - return null; - } - - public function getRequestMethod(): ?string - { - return null; - } - public function getUserId(): string { return $this->userId; diff --git a/core/lib/Service/FirewallService.php b/core/lib/Service/FirewallService.php index 2cfbb96..74287d1 100644 --- a/core/lib/Service/FirewallService.php +++ b/core/lib/Service/FirewallService.php @@ -249,6 +249,14 @@ class FirewallService } } + public function logAuthenticationSuccess(AuthenticationSucceededEvent $event): void + { + $log = $this->securityLog($event, $this->requestContext->current()); + if ($log !== null) { + $this->store->createLog($log); + } + } + private function securityLog( SecurityEventInterface $event, ?Request $request = null, diff --git a/tests/php/Unit/Event/AuthenticationSucceededEventTest.php b/tests/php/Unit/Event/AuthenticationSucceededEventTest.php index 0e26bcc..8b9dd5e 100644 --- a/tests/php/Unit/Event/AuthenticationSucceededEventTest.php +++ b/tests/php/Unit/Event/AuthenticationSucceededEventTest.php @@ -6,6 +6,7 @@ namespace KTXT\Unit\Event; use KTXC\Security\Event\AuthenticationSucceededEvent; use KTXC\Security\Event\SecurityEventSeverity; +use KTXC\Security\Event\SecurityRequestEventInterface; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\TestDox; use PHPUnit\Framework\TestCase; @@ -17,35 +18,24 @@ final class AuthenticationSucceededEventTest extends TestCase public function constructsTypedState(): void { $event = new AuthenticationSucceededEvent( - '203.0.113.10', 'user-a', 'tenant-a', - 'device-a', ); self::assertSame(AuthenticationSucceededEvent::class, $event->getName()); - self::assertSame('203.0.113.10', $event->getIpAddress()); self::assertSame('user-a', $event->getUserId()); self::assertSame('tenant-a', $event->getTenantId()); - self::assertSame('device-a', $event->getDeviceFingerprint()); self::assertSame(['userId' => 'user-a'], $event->getData()); self::assertSame(SecurityEventSeverity::INFO, $event->getSeverity()); + self::assertNotInstanceOf(SecurityRequestEventInterface::class, $event); } #[Test] - #[TestDox('Successful authentication requires an IP address and user ID')] + #[TestDox('Successful authentication requires a user ID')] public function rejectsIncompleteAuthenticationContext(): void { - foreach ([ - ['', 'user-a'], - ['203.0.113.10', ''], - ] as $arguments) { - try { - new AuthenticationSucceededEvent(...$arguments); - self::fail('Incomplete successful-authentication context was accepted.'); - } catch (\InvalidArgumentException) { - $this->addToAssertionCount(1); - } - } + $this->expectException(\InvalidArgumentException::class); + + new AuthenticationSucceededEvent(''); } } diff --git a/tests/php/Unit/Module/CoreModuleTest.php b/tests/php/Unit/Module/CoreModuleTest.php index e64255f..ef64139 100644 --- a/tests/php/Unit/Module/CoreModuleTest.php +++ b/tests/php/Unit/Module/CoreModuleTest.php @@ -52,8 +52,14 @@ final class CoreModuleTest extends TestCase $registry->listeners(AuthenticationFailedEvent::class, DeliveryMode::Immediate)[0]->service, ); self::assertSame([], $registry->listeners(AuthenticationFailedEvent::class, DeliveryMode::Deferred)); - foreach ([ + $successListeners = $registry->listeners( AuthenticationSucceededEvent::class, + DeliveryMode::Deferred, + ); + self::assertCount(1, $successListeners); + self::assertSame(FirewallService::class, $successListeners[0]->service); + self::assertSame('logAuthenticationSuccess', $successListeners[0]->method); + foreach ([ AccessDeniedEvent::class, BruteForceDetectedEvent::class, RateLimitExceededEvent::class, diff --git a/tests/php/Unit/Security/AuthenticationManagerTest.php b/tests/php/Unit/Security/AuthenticationManagerTest.php index 105acf6..7c9120e 100644 --- a/tests/php/Unit/Security/AuthenticationManagerTest.php +++ b/tests/php/Unit/Security/AuthenticationManagerTest.php @@ -10,6 +10,7 @@ use KTXC\Resource\ProviderManager; use KTXC\Security\Authentication\AuthenticationRequest; use KTXC\Security\AuthenticationManager; use KTXC\Security\Event\AuthenticationFailedEvent; +use KTXC\Security\Event\AuthenticationSucceededEvent; use KTXC\Service\TokenService; use KTXC\Service\UserAccountsService; use KTXF\Cache\CacheScope; @@ -24,6 +25,80 @@ use PHPUnit\Framework\TestCase; final class AuthenticationManagerTest extends TestCase { + #[Test] + public function completedAuthenticationPublishesAuthenticationSucceededEvent(): void + { + $tenant = $this->createStub(TenantContextInterface::class); + $tenant->method('identifier')->willReturn('tenant-a'); + $tenant->method('configuration')->willReturn(new TenantConfiguration()); + + $session = new AuthenticationSession( + id: 'auth-session', + state: AuthenticationSession::STATE_IDENTIFIED, + tenantIdentifier: 'tenant-a', + userIdentifier: 'user-a', + userIdentity: 'person@example.com', + methodsAvailable: ['password'], + createdAt: time(), + expiresAt: time() + 300, + ); + + $cache = $this->createMock(EphemeralCacheInterface::class); + $cache->expects($this->once()) + ->method('get') + ->with('auth-session', CacheScope::Tenant, 'auth') + ->willReturn($session); + $cache->expects($this->once())->method('set')->willReturn(true); + $cache->expects($this->once()) + ->method('delete') + ->with('auth-session', CacheScope::Tenant, 'auth') + ->willReturn(true); + + $provider = new SuccessfulAuthenticationProvider(); + $providers = $this->createStub(ProviderManager::class); + $providers->method('resolve')->willReturn($provider); + + $users = $this->createMock(UserAccountsService::class); + $users->expects($this->once()) + ->method('fetchByIdentifier') + ->with('user-a') + ->willReturn([ + 'uid' => 'user-a', + 'identity' => 'person@example.com', + 'label' => 'Person', + 'permissions' => [], + ]); + + $tokens = $this->createStub(TokenService::class); + $tokens->method('createToken')->willReturn('token'); + + $events = $this->createMock(EventDispatcherInterface::class); + $events->expects($this->once()) + ->method('dispatch') + ->with(self::callback(static fn($event): bool => + $event instanceof AuthenticationSucceededEvent + && $event->getUserId() === 'user-a' + && $event->getTenantId() === 'tenant-a' + )); + + $manager = new AuthenticationManager( + $tenant, + $cache, + $providers, + $tokens, + $users, + $events, + ); + + $response = $manager->handle(AuthenticationRequest::verify( + 'auth-session', + 'password', + 'correct', + )); + + self::assertTrue($response->isSuccess()); + } + #[Test] public function rejectedCredentialPublishesAuthenticationFailedEvent(): void { @@ -145,3 +220,64 @@ final class AuthenticationManagerTest extends TestCase self::assertSame(1, $provider->verificationCount); } } + +final class SuccessfulAuthenticationProvider implements AuthenticationProviderInterface +{ + public function type(): string + { + return 'authentication'; + } + + public function identifier(): string + { + return 'password'; + } + + public function label(): string + { + return 'Password'; + } + + public function description(): string + { + return 'Successful test provider'; + } + + public function method(): string + { + return self::METHOD_CREDENTIAL; + } + + public function icon(): string + { + return ''; + } + + public function verify(ProviderContext $context, string $secret): ProviderResult + { + return ProviderResult::success(); + } + + public function beginChallenge(ProviderContext $context): ProviderResult + { + return ProviderResult::failed(); + } + + public function verifyChallenge(ProviderContext $context, string $code): ProviderResult + { + return ProviderResult::failed(); + } + + public function beginRedirect( + ProviderContext $context, + string $callbackUrl, + ?string $returnUrl = null, + ): ProviderResult { + return ProviderResult::failed(); + } + + public function completeRedirect(ProviderContext $context, array $params): ProviderResult + { + return ProviderResult::failed(); + } +} diff --git a/tests/php/Unit/Service/FirewallServiceTest.php b/tests/php/Unit/Service/FirewallServiceTest.php index 118f341..f247434 100644 --- a/tests/php/Unit/Service/FirewallServiceTest.php +++ b/tests/php/Unit/Service/FirewallServiceTest.php @@ -324,8 +324,7 @@ class FirewallServiceTest extends TestCase })) ->willReturnArgument(0); - $this->service->logSecurityEvent(new AuthenticationSucceededEvent( - '203.0.113.10', + $this->service->logAuthenticationSuccess(new AuthenticationSucceededEvent( 'user-a', 'tenant-a', ));