Improve handling of qr timeout

Prior to this change, the application would not check if the qr code was expired. If the user scanned an expired qr code, it would try to see if it was invalid.
This resulted in errors in the logs, because it should not even try to see if the scanned qr code is valid if it is expired.
This change checks if the qr code is expired before attempting to check its validity.

See https://www.pivotaltracker.com/n/projects/1163646/stories/188205272

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;
     }
 
     /**