diff --git a/config/openconext/parameters.yaml.dist b/config/openconext/parameters.yaml.dist index 80e724d5..a508e874 100644 --- a/config/openconext/parameters.yaml.dist +++ b/config/openconext/parameters.yaml.dist @@ -4,7 +4,7 @@ parameters: app_secret: SeTW1ThY0uR0WnS3cR37 application_name: OpenConext Middleware # IP addresses of any HTTP proxies that are sitting in front of the application - # See: http://symfony.com/doc/current/request/load_balancer_reverse_proxy.html + # See: https://symfony.com/doc/current/deployment/proxies.html trusted_proxies: ~ database_driver: pdo_mysql diff --git a/config/packages/framework.yaml b/config/packages/framework.yaml index 3fa121f0..ca23e49f 100644 --- a/config/packages/framework.yaml +++ b/config/packages/framework.yaml @@ -1,5 +1,6 @@ framework: secret: '"%secret%"' + trusted_proxies: '%trusted_proxies%' #csrf_protection: true #http_method_override: true diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Logger/SecurityContextProcessor.php b/src/Surfnet/StepupMiddleware/ApiBundle/Logger/SecurityContextProcessor.php new file mode 100644 index 00000000..2955be98 --- /dev/null +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Logger/SecurityContextProcessor.php @@ -0,0 +1,65 @@ + 'request'])] +class SecurityContextProcessor implements ProcessorInterface +{ + public function __construct( + private readonly RequestStack $requestStack, + private readonly Security $security, + ) { + } + + public function __invoke(LogRecord $record): LogRecord + { + $extra = $record->extra; + + $request = $this->requestStack->getCurrentRequest(); + if ($request !== null) { + $extra['http_method'] = $request->getMethod(); + $extra['path'] = $request->getPathInfo(); + + $route = $request->attributes->get('_route'); + if (is_string($route) && $route !== '') { + $extra['route'] = $route; + } + + $clientIp = $request->getClientIp(); + if ($clientIp !== null && $clientIp !== '') { + $extra['client_ip'] = $clientIp; + } + } + + $user = $this->security->getUser(); + if ($user !== null) { + $extra['user'] = $user->getUserIdentifier(); + } + + return $record->with(extra: $extra); + } +} diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Logger/SecurityContextProcessorTest.php b/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Logger/SecurityContextProcessorTest.php new file mode 100644 index 00000000..6809bcaa --- /dev/null +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Logger/SecurityContextProcessorTest.php @@ -0,0 +1,179 @@ + '192.168.1.100'] + ); + $request->attributes->set('_route', 'api_identity_resolve'); + + $user = m::mock(UserInterface::class); + $user->shouldReceive('getUserIdentifier')->andReturn('user-12345'); + + $requestStack->shouldReceive('getCurrentRequest')->andReturn($request); + $security->shouldReceive('getUser')->andReturn($user); + + $processor = new SecurityContextProcessor($requestStack, $security); + + $record = new LogRecord( + datetime: new DateTimeImmutable(), + channel: 'security', + level: Level::Debug, + message: 'Authenticator set no success response: request continues.', + context: ['authenticator' => 'CustomAuthenticator'], + extra: ['existing' => 'value'] + ); + + $processed = $processor($record); + + $this->assertSame('value', $processed->extra['existing']); + $this->assertSame('GET', $processed->extra['http_method']); + $this->assertSame('/api/identity/resolve', $processed->extra['path']); + $this->assertSame('api_identity_resolve', $processed->extra['route']); + $this->assertSame('192.168.1.100', $processed->extra['client_ip']); + $this->assertSame('user-12345', $processed->extra['user']); + } + + #[Test] + public function it_enriches_log_record_when_request_is_present_without_user(): void + { + $requestStack = m::mock(RequestStack::class); + $security = m::mock(Security::class); + + $request = Request::create( + uri: '/api/identity/resolve', + method: 'GET', + server: ['REMOTE_ADDR' => '192.168.1.100'] + ); + $request->attributes->set('_route', 'api_identity_resolve'); + + $requestStack->shouldReceive('getCurrentRequest')->andReturn($request); + $security->shouldReceive('getUser')->andReturnNull(); + + $processor = new SecurityContextProcessor($requestStack, $security); + + $record = new LogRecord( + datetime: new DateTimeImmutable(), + channel: 'request', + level: Level::Info, + message: 'Unauthenticated request received', + context: [], + extra: ['existing' => 'value'] + ); + + $processed = $processor($record); + + $this->assertSame('value', $processed->extra['existing']); + $this->assertSame('GET', $processed->extra['http_method']); + $this->assertSame('/api/identity/resolve', $processed->extra['path']); + $this->assertSame('api_identity_resolve', $processed->extra['route']); + $this->assertSame('192.168.1.100', $processed->extra['client_ip']); + $this->assertArrayNotHasKey('user', $processed->extra); + } + + #[Test] + public function it_omits_route_when_route_attribute_is_missing(): void + { + $requestStack = m::mock(RequestStack::class); + $security = m::mock(Security::class); + + $request = Request::create( + uri: '/api/identity/resolve', + method: 'POST', + server: ['REMOTE_ADDR' => '192.168.1.100'] + ); + + $user = m::mock(UserInterface::class); + $user->shouldReceive('getUserIdentifier')->andReturn('user-12345'); + + $requestStack->shouldReceive('getCurrentRequest')->andReturn($request); + $security->shouldReceive('getUser')->andReturn($user); + + $processor = new SecurityContextProcessor($requestStack, $security); + + $record = new LogRecord( + datetime: new DateTimeImmutable(), + channel: 'request', + level: Level::Info, + message: 'Request with unmatched route', + context: [], + extra: [] + ); + + $processed = $processor($record); + + $this->assertSame('POST', $processed->extra['http_method']); + $this->assertSame('/api/identity/resolve', $processed->extra['path']); + $this->assertSame('192.168.1.100', $processed->extra['client_ip']); + $this->assertSame('user-12345', $processed->extra['user']); + $this->assertArrayNotHasKey('route', $processed->extra); + } + + #[Test] + public function it_handles_missing_request_and_missing_user(): void + { + $requestStack = m::mock(RequestStack::class); + $security = m::mock(Security::class); + + $requestStack->shouldReceive('getCurrentRequest')->andReturnNull(); + $security->shouldReceive('getUser')->andReturnNull(); + + $processor = new SecurityContextProcessor($requestStack, $security); + + $record = new LogRecord( + datetime: new DateTimeImmutable(), + channel: 'app', + level: Level::Info, + message: 'CLI task executed', + context: [], + extra: [] + ); + + $processed = $processor($record); + + $this->assertSame([], $processed->extra); + } +}