From 136a24372d3b0ff1897c82c16e023100d2e4c29d Mon Sep 17 00:00:00 2001 From: Michiel Kodde Date: Mon, 2 Sep 2024 15:43:34 +0200 Subject: [PATCH] When authn error occurs, send 'invalid-request' This error status was previously not supported. It is now. The uncaught errors are caught, and the invalid-request is sent back to the JS app. That in turn displays the user facing error page. --- .../typescript/AuthenticationPageService.ts | 3 + .../AuthenticationPageService.test.ts | 8 +++ .../AuthenticationStatusController.php | 62 +++++++++++-------- 3 files changed, 48 insertions(+), 25 deletions(-) diff --git a/assets/typescript/AuthenticationPageService.ts b/assets/typescript/AuthenticationPageService.ts index c141b46d..daf17f7b 100644 --- a/assets/typescript/AuthenticationPageService.ts +++ b/assets/typescript/AuthenticationPageService.ts @@ -106,6 +106,9 @@ export class AuthenticationPageService { case 'challenge-expired': this.switchToChallengeHasExpired(); break; + case 'invalid-request': + this.switchToNotificationFailed(); + break; case 'needs-refresh': this.reloadPage(); break; diff --git a/assets/typescript/__test__/AuthenticationPageService.test.ts b/assets/typescript/__test__/AuthenticationPageService.test.ts index 801694a5..7d3d9db7 100644 --- a/assets/typescript/__test__/AuthenticationPageService.test.ts +++ b/assets/typescript/__test__/AuthenticationPageService.test.ts @@ -157,6 +157,14 @@ describe('AuthenticationPageService', () => { successCallback('challenge-expired'); expect(spy).toBeCalled(); }); + it('Should handle authn error (invalid request)', () => { + if (!successCallback || !errorCallback) { + throw new Error('Should have started status request'); + } + const spy = jest.spyOn(context.authenticationPageService, 'switchToNotificationFailed'); + successCallback('invalid-request'); + expect(spy).toBeCalled(); + }); it('Should handle challenge expired', () => { if (!successCallback || !errorCallback) { diff --git a/src/Controller/AuthenticationStatusController.php b/src/Controller/AuthenticationStatusController.php index ade35c09..469981d0 100644 --- a/src/Controller/AuthenticationStatusController.php +++ b/src/Controller/AuthenticationStatusController.php @@ -46,41 +46,53 @@ public function __construct( #[Route(path: '/authentication/status', name: 'app_identity_authentication_status', methods: ['GET'])] public function __invoke(): JsonResponse { - $nameId = $this->authenticationService->getNameId(); - $sari = $this->stateHandler->getRequestId(); - $logger = WithContextLogger::from($this->logger, ['nameId' => $nameId, 'sari' => $sari]); - $logger->info('Request for authentication status'); - - if (!$this->authenticationService->authenticationRequired()) { - $logger->error('There is no pending authentication request from SP'); - return $this->refreshAuthenticationPage(); - } - - $isAuthenticated = $this->tiqrService->isAuthenticated(); - - if ($isAuthenticated) { - $logger->info('Send json response is authenticated'); - - return $this->refreshAuthenticationPage(); - } - - if ($this->authenticationChallengeIsExpired()) { - return $this->timeoutNeedsManualRetry(); + try { + $nameId = $this->authenticationService->getNameId(); + $sari = $this->stateHandler->getRequestId(); + $logger = WithContextLogger::from($this->logger, ['nameId' => $nameId, 'sari' => $sari]); + $logger->info('Request for authentication status'); + + if (!$this->authenticationService->authenticationRequired()) { + $logger->error('There is no pending authentication request from SP'); + return $this->refreshAuthenticationPage(); + } + + $isAuthenticated = $this->tiqrService->isAuthenticated(); + + if ($isAuthenticated) { + $logger->info('Send json response is authenticated'); + + return $this->refreshAuthenticationPage(); + } + + if ($this->authenticationChallengeIsExpired()) { + return $this->timeoutNeedsManualRetry(); + } + + $logger->info('Send json response is not authenticated'); + + return $this->scheduleNextPollOnAuthenticationPage(); + } catch (Exception $e) { + $this->logger->error( + sprintf( + 'An unexpected authentication error occurred. Responding with "invalid-request", '. + 'original exception message: "%s"', + $e->getMessage() + ) + ); + return $this->generateAuthenticationStatusResponse('invalid-request'); } - - $logger->info('Send json response is not authenticated'); - - return $this->scheduleNextPollOnAuthenticationPage(); } /** * Generate a status response for authentication.html. * - * The javascript in the authentication page expects one of three statuses: + * The javascript in the authentication page expects one of four statuses: * * - pending: waiting for user action, schedule next poll * - needs-refresh: refresh the page (the /authentication page will handle the success or error) * - challenge-expired: Message user challenge is expired, let the user give the option to retry. + * - invalid-request: There was a state issue, or another reason why authentication failed * * @return JsonResponse */