Handle unknown statuses as an error

At first I opted to handle the 'invalid-request' manually. But having a
default switch-case to handle all unhandled stati as an error makes more
sense.

And before this commit, the invalid request was handled as a Push
Notification failure. But that was not my intention. I wanted to render
the error page, and for that, we need to call the switchToStatusRequestError
method instead.

Add a timeout helper class

That is added to isolate the timeout 'business' rules are implemented
correctly. And make them testable.

Check registration timeout occurences

Handle authentication timeout occurences

Test the new registration state change

The Jest test was not yet covering the possibility of a registration
timeout

assets/typescript/AuthenticationPageService.ts

@@ -106,12 +106,12 @@ export class AuthenticationPageService {
       case 'challenge-expired':
         this.switchToChallengeHasExpired();
         break;
-      case 'invalid-request':
-        this.switchToNotificationFailed();
-        break;
       case 'needs-refresh':
         this.reloadPage();
         break;
+      default:
+        this.switchToStatusRequestError();
+        break;
     }
   };
 

assets/typescript/Component/RegistrationStatusComponent.ts

@@ -49,6 +49,12 @@ export class RegistrationStatusComponent {
   public showUnknownErrorHappened() {
     this.show('div.status.error');
   }
+  /**
+   * Unknown error happened. Please try again by refreshing your browser.
+   */
+  public showTimeoutHappened() {
+    this.show('div.status.timeout');
+  }
 
   private hideAll() {
     jQuery('.status-container >').hide();

assets/typescript/RegistrationStateMachine.ts

@@ -12,6 +12,8 @@ export class RegistrationStateMachine {
    * Client-side only status.
    */
   public static readonly ERROR = 'ERROR';
+  public static readonly TIMEOUT = 'TIMEOUT';
+
   private previousStatus = RegistrationStateMachine.IDLE;
 
   constructor(private statusPollingService: StatusPollService,
@@ -62,6 +64,12 @@ export class RegistrationStateMachine {
         this.qrCode.hide();
         document.location.replace(this.finalizedUrl);
         break;
+      case RegistrationStateMachine.TIMEOUT:
+        this.qrCode.hide();
+        this.statusUi.showTimeoutHappened();
+        this.statusPollingService.stop();
+        this.previousStatus = RegistrationStateMachine.ERROR;
+        break;
       default:
         this.unknownError();
         return;

assets/typescript/__test__/AuthenticationPageService.test.ts

@@ -161,7 +161,7 @@ describe('AuthenticationPageService', () => {
       if (!successCallback || !errorCallback) {
         throw new Error('Should have started status request');
       }
-      const spy = jest.spyOn(context.authenticationPageService, 'switchToNotificationFailed');
+      const spy = jest.spyOn(context.authenticationPageService, 'switchToStatusRequestError');
       successCallback('invalid-request');
       expect(spy).toBeCalled();
     });

assets/typescript/__test__/RegistrationPageService.test.ts

@@ -141,6 +141,28 @@ describe('RegistrationPageService', () => {
     });
   });
 
+  describe('When timeout', () => {
+    beforeEach(() => {
+      context.authenticationPageService.start();
+      if (!statusCallback || !errorCallback) {
+        throw new Error('Should have started status request');
+      }
+      statusCallback(RegistrationStateMachine.TIMEOUT);
+    });
+
+    it('The qr code should be hidden', () => {
+      expect(context.qrComponent.isVisible()).toBeFalsy();
+    });
+
+    it('Polling should be disabled', () => {
+      expect(context.pollingService.enabled).toBeFalsy();
+    });
+
+    it('Show finalized', () => {
+      expect(context.statusUi.showTimeoutHappened).toBeCalled();
+    });
+  });
+
   describe('When connection error occurred', () => {
     beforeEach(() => {
       context.authenticationPageService.start();
@@ -227,6 +249,7 @@ describe('RegistrationPageService', () => {
       showAccountActivationHelp:jest.fn(),
       showOneMomentPlease: jest.fn(),
       showFinalized: jest.fn(),
+      showTimeoutHappened: jest.fn(),
       showUnknownErrorHappened: jest.fn(),
     };
 

ci/qa/phpstan-baseline.neon

@@ -1,5 +1,200 @@
 parameters:
 	ignoreErrors:
+		-
+			message: "#^Cannot access offset 'authenticationUrl' on mixed\\.$#"
+			count: 1
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Cannot access offset 'id' on mixed\\.$#"
+			count: 1
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Cannot access offset 'identities' on mixed\\.$#"
+			count: 2
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Cannot access offset 'ocraSuite' on mixed\\.$#"
+			count: 1
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Cannot access offset 'secret' on mixed\\.$#"
+			count: 1
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Cannot access offset int\\|string\\|false on mixed\\.$#"
+			count: 1
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Cannot access offset string on mixed\\.$#"
+			count: 1
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Parameter \\#1 \\$array of function array_keys expects array, mixed given\\.$#"
+			count: 1
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Parameter \\#1 \\$file of method Surfnet\\\\Tiqr\\\\Dev\\\\Command\\\\AuthenticationCommand\\:\\:readAuthenticationLinkFromFile\\(\\) expects string, mixed given\\.$#"
+			count: 1
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Parameter \\#1 \\$json of function json_decode expects string, string\\|false given\\.$#"
+			count: 1
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Parameter \\#1 \\$ocraSuite of static method OCRA\\:\\:generateOCRA\\(\\) expects string, mixed given\\.$#"
+			count: 1
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Parameter \\#1 \\$uri of method GuzzleHttp\\\\Client\\:\\:post\\(\\) expects Psr\\\\Http\\\\Message\\\\UriInterface\\|string, mixed given\\.$#"
+			count: 1
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Parameter \\#2 \\$key of static method OCRA\\:\\:generateOCRA\\(\\) expects string, mixed given\\.$#"
+			count: 1
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Parameter \\#2 \\.\\.\\.\\$values of function sprintf expects bool\\|float\\|int\\|string\\|null, mixed given\\.$#"
+			count: 1
+			path: ../../dev/Command/AuthenticationCommand.php
+
+		-
+			message: "#^Cannot access offset 'authenticationUrl' on mixed\\.$#"
+			count: 1
+			path: ../../dev/Command/RegistrationCommand.php
+
+		-
+			message: "#^Cannot access offset 'identities' on mixed\\.$#"
+			count: 2
+			path: ../../dev/Command/RegistrationCommand.php
+
+		-
+			message: "#^Cannot access offset 'ocraSuite' on mixed\\.$#"
+			count: 1
+			path: ../../dev/Command/RegistrationCommand.php
+
+		-
+			message: "#^Cannot access offset mixed on mixed\\.$#"
+			count: 5
+			path: ../../dev/Command/RegistrationCommand.php
+
+		-
+			message: "#^Cannot access property \\$service on mixed\\.$#"
+			count: 2
+			path: ../../dev/Command/RegistrationCommand.php
+
+		-
+			message: "#^Method Surfnet\\\\Tiqr\\\\Dev\\\\Command\\\\RegistrationCommand\\:\\:storeIdentity\\(\\) has parameter \\$metadata with no type specified\\.$#"
+			count: 1
+			path: ../../dev/Command/RegistrationCommand.php
+
+		-
+			message: "#^Method Surfnet\\\\Tiqr\\\\Dev\\\\Command\\\\RegistrationCommand\\:\\:storeIdentity\\(\\) has parameter \\$secret with no type specified\\.$#"
+			count: 1
+			path: ../../dev/Command/RegistrationCommand.php
+
+		-
+			message: "#^Parameter \\#1 \\$file of method Surfnet\\\\Tiqr\\\\Dev\\\\Command\\\\RegistrationCommand\\:\\:readRegistrationUrlFromFile\\(\\) expects string, mixed given\\.$#"
+			count: 1
+			path: ../../dev/Command/RegistrationCommand.php
+
+		-
+			message: "#^Parameter \\#1 \\$json of function json_decode expects string, string\\|false given\\.$#"
+			count: 1
+			path: ../../dev/Command/RegistrationCommand.php
+
+		-
+			message: "#^Call to an undefined method SAML2\\\\Message\\:\\:getStatus\\(\\)\\.$#"
+			count: 1
+			path: ../../dev/Controller/SPController.php
+
+		-
+			message: "#^Cannot call method getValue\\(\\) on SAML2\\\\XML\\\\saml\\\\Issuer\\|null\\.$#"
+			count: 1
+			path: ../../dev/Controller/SPController.php
+
+		-
+			message: "#^Cannot cast mixed to string\\.$#"
+			count: 1
+			path: ../../dev/Controller/SPController.php
+
+		-
+			message: "#^Method Surfnet\\\\Tiqr\\\\Dev\\\\Controller\\\\SPController\\:\\:signRequestQuery\\(\\) has parameter \\$queryParams with no value type specified in iterable type array\\.$#"
+			count: 1
+			path: ../../dev/Controller/SPController.php
+
+		-
+			message: "#^PHPDoc tag @var has invalid value \\(\\$securityKey\\)\\: Unexpected token \"\\$securityKey\", expected type at offset 10$#"
+			count: 1
+			path: ../../dev/Controller/SPController.php
+
+		-
+			message: "#^Parameter \\#1 \\$key of method SAML2\\\\Certificate\\\\PrivateKeyLoader\\:\\:loadPrivateKey\\(\\) expects SAML2\\\\Configuration\\\\PrivateKey, mixed given\\.$#"
+			count: 1
+			path: ../../dev/Controller/SPController.php
+
+		-
+			message: "#^Parameter \\#1 \\$nameId of method Surfnet\\\\SamlBundle\\\\SAML2\\\\AuthnRequest\\:\\:setSubject\\(\\) expects string, mixed given\\.$#"
+			count: 1
+			path: ../../dev/Controller/SPController.php
+
+		-
+			message: "#^Parameter \\#1 \\$source of method DOMDocument\\:\\:loadXML\\(\\) expects string, bool\\|string given\\.$#"
+			count: 1
+			path: ../../dev/Controller/SPController.php
+
+		-
+			message: "#^Parameter \\#1 \\$string of function base64_decode expects string, bool\\|float\\|int\\|string\\|null given\\.$#"
+			count: 1
+			path: ../../dev/Controller/SPController.php
+
+		-
+			message: "#^Parameter \\#1 \\$string of function base64_encode expects string, string\\|false given\\.$#"
+			count: 1
+			path: ../../dev/Controller/SPController.php
+
+		-
+			message: "#^Parameter \\#1 \\$xml of static method SAML2\\\\Message\\:\\:fromXML\\(\\) expects DOMElement, DOMElement\\|null given\\.$#"
+			count: 1
+			path: ../../dev/Controller/SPController.php
+
+		-
+			message: "#^Parameter \\#2 \\$values of method Symfony\\\\Component\\\\HttpFoundation\\\\ResponseHeaderBag\\:\\:set\\(\\) expects array\\<string\\>\\|string\\|null, mixed given\\.$#"
+			count: 1
+			path: ../../dev/Controller/SPController.php
+
+		-
+			message: "#^Method Surfnet\\\\Tiqr\\\\Dev\\\\FileLogger\\:\\:getLogs\\(\\) return type has no value type specified in iterable type array\\.$#"
+			count: 1
+			path: ../../dev/FileLogger.php
+
+		-
+			message: "#^Method Surfnet\\\\Tiqr\\\\Dev\\\\FileLogger\\:\\:log\\(\\) has parameter \\$context with no value type specified in iterable type array\\.$#"
+			count: 1
+			path: ../../dev/FileLogger.php
+
+		-
+			message: "#^Parameter \\#1 \\$record of method League\\\\Csv\\\\Writer\\:\\:insertOne\\(\\) expects array\\<float\\|int\\|string\\|Stringable\\|null\\>, array\\<int, mixed\\> given\\.$#"
+			count: 1
+			path: ../../dev/FileLogger.php
+
+		-
+			message: "#^Parameter \\#1 \\$stream of static method League\\\\Csv\\\\AbstractCsv\\:\\:createFromStream\\(\\) expects resource, resource\\|false given\\.$#"
+			count: 1
+			path: ../../dev/FileLogger.php
+
 		-
 			message: "#^Parameter \\#3 \\$response of method Surfnet\\\\Tiqr\\\\Tiqr\\\\AuthenticationRateLimitServiceInterface\\:\\:authenticate\\(\\) expects string, mixed given\\.$#"
 			count: 1

ci/qa/phpstan.neon

@@ -5,3 +5,4 @@ parameters:
 	level: 9
 	paths:
 		- ../../src
+		- ../../dev

dev/Command/AuthenticationCommand.php

@@ -88,7 +88,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         [$serviceId, $session, $challenge, $sp, $version] = explode('/', $authn);
 
         $userId = null;
-        if (strpos($serviceId, '@') >= 0) {
+        if (str_contains($serviceId, '@')) {
             [$userId, $serviceId] = explode('@', $serviceId);
         }
 
@@ -101,7 +101,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 'sp' => $sp,
                 'version' => $version,
                 'userId' => $userId,
-            ], JSON_PRETTY_PRINT)),
+            ], JSON_PRETTY_PRINT | JSON_THROW_ON_ERROR)),
         ]);
 
         $service = $this->getService($serviceId);
@@ -126,7 +126,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $service['id'] = $serviceId;
         $output->writeln([
             '<comment>Generate OCRA for service:</comment>',
-            $this->decorateResult(json_encode($service, JSON_PRETTY_PRINT)),
+            $this->decorateResult(json_encode($service, JSON_PRETTY_PRINT | JSON_THROW_ON_ERROR)),
         ]);
 
         $response = OCRA::generateOCRA($ocraSuite, $secret, '', $challenge, '', $session, '');
@@ -143,16 +143,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             'sessionKey' => $session,
             'userId' => $userId,
             'response' => $response,
-            'notificationType' => $input->getOption('notificationType', ''),
-            'notificationAddress' => $input->getOption('notificationAddress', ''),
+            'notificationType' => $input->getOption('notificationType'),
+            'notificationAddress' => $input->getOption('notificationAddress'),
         ];
 
         $output->writeln([
             sprintf(
                 '<comment>Send authentication data to "%s" with body:</comment>',
                 $authenticationUrl
             ),
-            $this->decorateResult(json_encode($authenticationBody, JSON_PRETTY_PRINT)),
+            $this->decorateResult(json_encode($authenticationBody, JSON_PRETTY_PRINT | JSON_THROW_ON_ERROR)),
         ]);
 
         $result = $this->client->post($authenticationUrl, [
@@ -166,22 +166,27 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         return Command::SUCCESS;
     }
 
-    protected function decorateResult($text): string
+    protected function decorateResult(string $text): string
     {
         return "<options=bold>$text</>";
     }
 
     protected function readAuthenticationLinkFromFile(string $file, OutputInterface $output): string
     {
         $qrcode = new QrReader(file_get_contents($file), QrReader::SOURCE_TYPE_BLOB);
+        /** @phpstan-var mixed $link */
         $link = $qrcode->text();
 
+        if (!is_string($link)) {
+            throw new RuntimeException('Unable to read a link from the QR code');
+        }
+
         $output->writeln([
             'Registration link result: ',
             $this->decorateResult($link),
         ]);
 
-        return $link->toString();
+        return $link;
     }
 
     /**