Skip to content

Commit

Permalink
Add the session id derived id to polling requests
Browse files Browse the repository at this point in the history
The polling async requests should include the correlation id based on
the session id of the user that is placing them.

That way we can correlate between the access  logs and the application
logs.

See: https://www.pivotaltracker.com/story/show/188205232
  • Loading branch information
MKodde authored and johanib committed Nov 14, 2024
1 parent be129cb commit 31ed973
Show file tree
Hide file tree
Showing 15 changed files with 83 additions and 27 deletions.
10 changes: 7 additions & 3 deletions assets/typescript/Client/StatusClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@ export interface PendingRequest {
}

export class StatusClient {
constructor(private apiUrl: string) {

constructor(
private apiUrl: string,
private correlationLoggingId: string,
) {
}

/**
* Request status form the API.
*/
public request(callback: (status: string) => void, errorCallback: (error: unknown) => void): PendingRequest {
return jQuery.get(this.apiUrl, callback).fail(errorCallback);
const url = this.apiUrl + (this.apiUrl.includes('?') ? '&' : '?') + 'correlation-id=' + this.correlationLoggingId;

return jQuery.get(url, callback).fail(errorCallback);
}
}
6 changes: 3 additions & 3 deletions assets/typescript/authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import jQuery from 'jquery';

declare global {
interface Window {
bootstrapAuthentication: (statusApiUrl: string, notificationApiUrl: string) => AuthenticationPageService;
bootstrapAuthentication: (statusApiUrl: string, notificationApiUrl: string, correlationLoggingId: string) => AuthenticationPageService;
}
}

window.bootstrapAuthentication = (statusApiUrl: string, notificationApiUrl: string) => {
const statusClient = new StatusClient(statusApiUrl);
window.bootstrapAuthentication = (statusApiUrl: string, notificationApiUrl: string, correlationLoggingId: string) => {
const statusClient = new StatusClient(statusApiUrl, correlationLoggingId);
const notificationClient = new NotificationClient(notificationApiUrl);
const pollingService = new StatusPollService(statusClient);

Expand Down
10 changes: 7 additions & 3 deletions assets/typescript/registration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,16 @@ import jQuery from 'jquery';

declare global {
interface Window {
bootstrapRegistration: (statusApiUrl: string, notificationApiUrl: string) => RegistrationStateMachine;
bootstrapRegistration: (
statusApiUrl: string,
notificationApiUrl: string,
correlationLoggingId: string
) => RegistrationStateMachine;
}
}

window.bootstrapRegistration = (statusApiUrl: string, finalizedUrl: string) => {
const statusClient = new StatusClient(statusApiUrl);
window.bootstrapRegistration = (statusApiUrl: string, finalizedUrl: string, correlationLoggingId: string) => {
const statusClient = new StatusClient(statusApiUrl, correlationLoggingId);
const pollingService = new StatusPollService(statusClient);
const machine = new RegistrationStateMachine(
pollingService,
Expand Down
6 changes: 4 additions & 2 deletions src/Controller/AuthenticationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use Surfnet\Tiqr\Exception\UserNotFoundException;
use Surfnet\Tiqr\Exception\UserPermanentlyBlockedException;
use Surfnet\Tiqr\Exception\UserTemporarilyBlockedException;
use Surfnet\Tiqr\Service\SessionCorrelationIdService;
use Surfnet\Tiqr\Tiqr\AuthenticationRateLimitServiceInterface;
use Surfnet\Tiqr\Tiqr\Exception\UserNotExistsException;
use Surfnet\Tiqr\Tiqr\Response\AuthenticationResponse;
Expand All @@ -53,6 +54,7 @@ public function __construct(
private readonly TiqrServiceInterface $tiqrService,
private readonly TiqrUserRepositoryInterface $userRepository,
private readonly AuthenticationRateLimitServiceInterface $authenticationRateLimitService,
private readonly SessionCorrelationIdService $correlationIdService,
private readonly LoggerInterface $logger
) {
}
Expand Down Expand Up @@ -145,8 +147,8 @@ public function __invoke(Request $request): Response
$logger->info('Return authentication page with QR code');

return $this->render('default/authentication.html.twig', [
// TODO: Add something identifying the authentication session to the authenticateUrl
'authenticateUrl' => $this->tiqrService->authenticationUrl()
'authenticateUrl' => $this->tiqrService->authenticationUrl(),
'correlationLoggingId' => $this->correlationIdService->generateCorrelationId(),
]);
}

Expand Down
3 changes: 3 additions & 0 deletions src/Controller/RegistrationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use Surfnet\GsspBundle\Service\StateHandlerInterface;
use Surfnet\Tiqr\Attribute\RequiresActiveSession;
use Surfnet\Tiqr\Exception\NoActiveAuthenrequestException;
use Surfnet\Tiqr\Service\SessionCorrelationIdService;
use Surfnet\Tiqr\Tiqr\Legacy\TiqrService;
use Surfnet\Tiqr\Tiqr\TiqrServiceInterface;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
Expand All @@ -38,6 +39,7 @@ public function __construct(
private readonly RegistrationService $registrationService,
private readonly StateHandlerInterface $stateHandler,
private readonly TiqrServiceInterface $tiqrService,
private readonly SessionCorrelationIdService $correlationIdService,
private readonly LoggerInterface $logger
) {
}
Expand Down Expand Up @@ -81,6 +83,7 @@ public function registration(Request $request): Response
'default/registration.html.twig',
[
'metadataUrl' => sprintf("tiqrenroll://%s", $metadataUrl),
'correlationLoggingId' => $this->correlationIdService->generateCorrelationId(),
'enrollmentKey' => $key
]
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@
private string $sessionName;

public function __construct(
private LoggerInterface $logger,
private LoggerInterface $logger,
private SessionCorrelationIdService $sessionCorrelationIdService,
/** @var array<string, string> */
private array $sessionOptions,
) {
if (!array_key_exists('name', $this->sessionOptions)) {
Expand Down
1 change: 1 addition & 0 deletions src/EventSubscriber/SessionStateListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
public function __construct(
private LoggerInterface $logger,
private SessionCorrelationIdService $sessionCorrelationIdService,
/** @var array<string, string> */
private array $sessionOptions,
) {
if (!array_key_exists('name', $this->sessionOptions)) {
Expand Down
1 change: 1 addition & 0 deletions src/Service/SessionCorrelationIdService.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

public function __construct(
private RequestStack $requestStack,
/** @var array<string, string> */
private array $sessionOptions,
private string $sessionCorrelationSalt,
) {
Expand Down
1 change: 0 additions & 1 deletion src/Tiqr/Legacy/TiqrService.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
use Tiqr_Service;
use Tiqr_StateStorage_StateStorageInterface;


/**
* Wrapper around the legacy Tiqr service.
*
Expand Down
3 changes: 2 additions & 1 deletion templates/default/authentication.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
*/
var authenticationPageService = window.bootstrapAuthentication(
"{{ path('app_identity_authentication_status') | escape('js') }}",
"{{ path('app_identity_authentication_notification') | escape('js') }}"
"{{ path('app_identity_authentication_notification') | escape('js') }}",
"{{ correlationLoggingId }}"
);
</script>
{% endblock %}
Expand Down
3 changes: 2 additions & 1 deletion templates/default/registration.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
<script>
var registrationStateMachine = window.bootstrapRegistration(
"{{ path('app_identity_registration_status') | escape('js') }}",
"{{ path('app_identity_registration') | escape('js') }}"
"{{ path('app_identity_registration') | escape('js') }}",
"{{ correlationLoggingId }}"
);
</script>
{% endblock %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,14 @@ public function testControllersWithoutTheRequiresActiveSessionAttributeAreIgnore
$mockLogger = Mockery::mock(LoggerInterface::class);
$mockLogger->shouldNotReceive('log');

$listener = new RequiresActiveSessionAttributeListener($mockLogger, new SessionCorrelationIdService($requestStack));
$listener = new RequiresActiveSessionAttributeListener(
$mockLogger,
new SessionCorrelationIdService(
$requestStack,
['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'
),
['name' => 'PHPSESSID'],
);

$dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelControllerArguments']);
$dispatcher->dispatch($event, KernelEvents::REQUEST);
Expand Down Expand Up @@ -103,7 +110,11 @@ public function testItDeniesAccessWhenThereIsNoActiveSession(): void
['correlationId' => '', 'route' => '/route']
);

$listener = new RequiresActiveSessionAttributeListener($mockLogger, new SessionCorrelationIdService($requestStack));
$listener = new RequiresActiveSessionAttributeListener(
$mockLogger,
new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
['name' => 'PHPSESSID'],
);

$dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelControllerArguments']);
$dispatcher->dispatch($event, KernelEvents::REQUEST);
Expand Down Expand Up @@ -144,7 +155,11 @@ public function testItDeniesAccessWhenThereIsNoSessionCookie(): void
['correlationId' => '', 'route' => '/route']
);

$listener = new RequiresActiveSessionAttributeListener($mockLogger, new SessionCorrelationIdService($requestStack));
$listener = new RequiresActiveSessionAttributeListener(
$mockLogger,
new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
['name' => 'PHPSESSID'],
);

$dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelControllerArguments']);
$dispatcher->dispatch($event, KernelEvents::REQUEST);
Expand Down Expand Up @@ -185,7 +200,11 @@ public function testItDeniesAccessWhenTheActiveSessionDoesNotMatchTheSessionCook
['correlationId' => 'f6e7cfb6f0861f577c48f171e27542236b1184f7a599dde82aca1640d86da961', 'route' => '/route']
);

$listener = new RequiresActiveSessionAttributeListener($mockLogger, new SessionCorrelationIdService($requestStack));
$listener = new RequiresActiveSessionAttributeListener(
$mockLogger,
new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
['name' => 'PHPSESSID'],
);

$dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelControllerArguments']);
$dispatcher->dispatch($event, KernelEvents::REQUEST);
Expand Down Expand Up @@ -219,7 +238,11 @@ public function testItDoesNotThrowWhenTheActiveSessionMatchesTheSessionCookie():
$mockLogger = Mockery::mock(LoggerInterface::class);
$mockLogger->shouldNotReceive('log');

$listener = new RequiresActiveSessionAttributeListener($mockLogger, new SessionCorrelationIdService($requestStack));
$listener = new RequiresActiveSessionAttributeListener(
$mockLogger,
new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
['name' => 'PHPSESSID'],
);

$dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelControllerArguments']);
$dispatcher->dispatch($event, KernelEvents::REQUEST);
Expand Down
24 changes: 20 additions & 4 deletions tests/Unit/EventSubscriber/SessionStateListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ public function testItLogsWhenUserHasNoSessionCookie(): void
->once()
->with(LogLevel::INFO, 'User made a request without a session cookie.', ['correlationId' => '', 'route' => '/route']);

$listener = new SessionStateListener($mockLogger, new SessionCorrelationIdService($requestStack));
$listener = new SessionStateListener(
$mockLogger,
new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
['name' => 'PHPSESSIONID'],
);

$dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelRequest']);
$dispatcher->dispatch($event, KernelEvents::REQUEST);
Expand Down Expand Up @@ -85,7 +89,11 @@ public function testItLogsWhenUserHasNoSession(): void
->once()
->with(LogLevel::INFO, 'Session not found.', ['correlationId' => 'f6e7cfb6f0861f577c48f171e27542236b1184f7a599dde82aca1640d86da961', 'route' => '/route']);

$listener = new SessionStateListener($mockLogger, new SessionCorrelationIdService($requestStack));
$listener = new SessionStateListener(
$mockLogger,
new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
['name' => 'PHPSESSID'],
);

$dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelRequest']);
$dispatcher->dispatch($event, KernelEvents::REQUEST);
Expand Down Expand Up @@ -121,7 +129,11 @@ public function testItLogsAnErrorWhenTheSessionIdDoesNotMatchTheSessionCookie():
->once()
->with(LogLevel::ERROR, 'The session cookie does not match the session id.', ['correlationId' => 'f6e7cfb6f0861f577c48f171e27542236b1184f7a599dde82aca1640d86da961', 'route' => '/route']);

$listener = new SessionStateListener($mockLogger, new SessionCorrelationIdService($requestStack));
$listener = new SessionStateListener(
$mockLogger,
new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
['name' => 'PHPSESSID'],
);

$dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelRequest']);
$dispatcher->dispatch($event, KernelEvents::REQUEST);
Expand Down Expand Up @@ -158,7 +170,11 @@ public function testTheUserSessionMatchesTheSessionCookie(): void
->once()
->with(LogLevel::INFO, 'User session matches the session cookie.', ['correlationId' => 'f6e7cfb6f0861f577c48f171e27542236b1184f7a599dde82aca1640d86da961', 'route' => '/route']);

$listener = new SessionStateListener($mockLogger, new SessionCorrelationIdService($requestStack));
$listener = new SessionStateListener(
$mockLogger,
new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
['name' => 'PHPSESSID'],
);

$dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelRequest']);
$dispatcher->dispatch($event, KernelEvents::REQUEST);
Expand Down
4 changes: 2 additions & 2 deletions tests/Unit/Service/SessionCorrelationIdServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public function testItHasNoCorrelationIdWhenThereIsNoSessionCookie(): void
$requestStack = new RequestStack();
$requestStack->push($request);

$service = new SessionCorrelationIdService($requestStack);
$service = new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ');

$this->assertNull($service->generateCorrelationId());
}
Expand All @@ -41,7 +41,7 @@ public function testItGeneratesACorrelationIdBasedOnTheSessionCookie(): void
$requestStack = new RequestStack();
$requestStack->push($request);

$service = new SessionCorrelationIdService($requestStack);
$service = new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ');

$this->assertSame('f6e7cfb6f0861f577c48f171e27542236b1184f7a599dde82aca1640d86da961', $service->generateCorrelationId());
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Unit/Session/LoggingSessionFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function testItLogsWheneverASessionIsCreated(): void
$requestStack,
$this->createStub(SessionStorageFactoryInterface::class),
$mockLogger,
new SessionCorrelationIdService($requestStack),
new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
);

$this->assertInstanceOf(SessionInterface::class, $sessionFactory->createSession());
Expand Down

0 comments on commit 31ed973

Please sign in to comment.