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
Prior to this change, when a polling action from the browser hit the
server, and the server returned a 500, or some other error, it was not
possible to trace which session the browser belonged to.
This change adds a correlation id to the polling mechanism, so that id
can be used to search te logs for the origin of the session.

This should help retrace errors.

See: https://www.pivotaltracker.com/story/show/188205232
  • Loading branch information
MKodde authored and johanib committed Nov 21, 2024
1 parent 76473db commit b30b7b4
Show file tree
Hide file tree
Showing 11 changed files with 34 additions and 151 deletions.
15 changes: 12 additions & 3 deletions assets/typescript/Client/StatusClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,23 @@ 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);
let url;
if(this.correlationLoggingId !== ''){
url = this.apiUrl + (this.apiUrl.includes('?') ? '&' : '?') + 'correlation-id=' + this.correlationLoggingId;
}else{
url = this.apiUrl;
}

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
1 change: 0 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
"khanamiryan/qrcode-detector-decoder": "^2.0",
"league/csv": "^9.13",
"malukenho/docheader": "^1",
"mockery/mockery": "^1.6",
"overtrue/phplint": "*",
"phpmd/phpmd": "^2.15",
"phpstan/phpstan": "^1.10",
Expand Down
136 changes: 1 addition & 135 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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 All @@ -72,7 +74,6 @@ public function __invoke(Request $request): Response

$logger->info('Verifying if there is a pending authentication request from SP');

// Do have a valid sample AuthnRequest?
if (!$this->authenticationService->authenticationRequired()) {
$logger->error('There is no pending authentication request from SP');

Expand Down Expand Up @@ -145,7 +146,8 @@ public function __invoke(Request $request): Response
$logger->info('Return authentication page with QR code');

return $this->render('default/authentication.html.twig', [
'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
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 @@ -17,7 +17,6 @@

namespace Unit\EventSubscriber;

use Mockery;
use Psr\Log\LoggerInterface;
use Psr\Log\LogLevel;
use Surfnet\Tiqr\Attribute\RequiresActiveSession;
Expand Down
1 change: 0 additions & 1 deletion tests/Unit/EventSubscriber/SessionStateListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

namespace Unit\EventSubscriber;

use Mockery;
use Psr\Log\LoggerInterface;
use Psr\Log\LogLevel;
use Surfnet\Tiqr\EventSubscriber\SessionStateListener;
Expand Down

0 comments on commit b30b7b4

Please sign in to comment.