From 5f669d51b4b8b59bb203131b3bd980c069545fa4 Mon Sep 17 00:00:00 2001
From: Michiel Kodde <mkodde@ibuildings.nl>
Date: Mon, 2 Sep 2024 14:52:02 +0200
Subject: [PATCH] Move Authn Status to own controller

Some additional bootstrapping was required to allow for this (in
services.yaml).
---
 config/services.yaml                          |   9 +-
 src/Controller/AuthenticationController.php   | 114 --------------
 .../AuthenticationStatusController.php        | 143 ++++++++++++++++++
 3 files changed, 150 insertions(+), 116 deletions(-)
 create mode 100644 src/Controller/AuthenticationStatusController.php

diff --git a/config/services.yaml b/config/services.yaml
index 3edf3b34..d1b7ac86 100644
--- a/config/services.yaml
+++ b/config/services.yaml
@@ -26,7 +26,13 @@ services:
         resource: '../src/*'
         exclude: '../src/{DependencyInjection,Entity,Migrations,Tests,Kernel.php}'
 
-    # add more service definitions when explicit configuration is needed
+    Surfnet\Tiqr\Controller\:
+        tags: [ 'controller.service_arguments' ]
+        resource: '../src/Controller/'
+        autowire: true
+        autoconfigure: true
+
+  # add more service definitions when explicit configuration is needed
     # please note that last definitions always *replace* previous ones
 
     Symfony\Component\DependencyInjection\Container:
@@ -51,6 +57,5 @@ services:
         $pattern: '%mobile_app_user_agent_pattern%'
 
     Surfnet\Tiqr\Controller\ExceptionController:
-      tags: ['controller.service_arguments']
       arguments:
         $errorPageHelper: '@Surfnet\Tiqr\Service\ErrorPageHelper'
diff --git a/src/Controller/AuthenticationController.php b/src/Controller/AuthenticationController.php
index ee66432b..e0cf98a4 100644
--- a/src/Controller/AuthenticationController.php
+++ b/src/Controller/AuthenticationController.php
@@ -151,120 +151,6 @@ public function authentication(Request $request): Response
         ]);
     }
 
-    /**
-     * @throws InvalidArgumentException
-     *
-     * Requires session cookie to be set to a valid session.
-     */
-    #[Route(path: '/authentication/status', name: 'app_identity_authentication_status', methods: ['GET'])]
-    public function authenticationStatus(): JsonResponse
-    {
-        $nameId = $this->authenticationService->getNameId();
-        $sari = $this->stateHandler->getRequestId();
-        $logger = WithContextLogger::from($this->logger, ['nameId' => $nameId, 'sari' => $sari]);
-        $logger->info('Request for authentication status');
-
-        // Do we have a valid GSSP authentication AuthnRequest in this session?
-        if (!$this->authenticationService->authenticationRequired()) {
-            $logger->error('There is no pending authentication request from SP');
-            return $this->refreshAuthenticationPage();
-        }
-
-        // Check if the user is authenticated. This check will work if the user has completed the authentication
-        // within the last Tiqr_Service::LOGIN_EXPIRE seconds (1 hour).
-        // Uses our SessionId.
-        $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();
-    }
-
-    /**
-     * Check if the authentication challenge is expired.
-     *
-     * If the challenge is expired, the page should be refreshed so a new
-     * challenge and QR code is generated.
-     *
-     * @return bool
-     */
-    private function authenticationChallengeIsExpired(): bool
-    {
-        // The use of authenticationUrl() here is a hack, because it depends on an implementation detail
-        // of this function.
-        // Effectively this does a $this->_stateStorage->getValue(self::PREFIX_CHALLENGE . $sessionKey);
-        // To check that the session key still exists in the Tiqr_Service's state storage
-
-        // TODO: Refactor this so it does not depend on the implementation of the TiqrService
-        //       i.e. we should keep track of authentication timeout ourselves
-        //       Uses this method creates noise in the logs because it logs errors because keys are not found
-        //       in the StateStorage. But this is not an error here, it is expected bacause expired keys
-        //       cannot be found in the statestorage by definition.
-        //       If you were truly interested in the authenticationUrl(), then logging the errors is correct.
-        try {
-            $this->tiqrService->authenticationUrl();
-        } catch (Exception) {
-            return true;
-        }
-        return false;
-    }
-
-    /**
-     * Authentication is pending, schedule a new poll action.
-     *
-     * @return JsonResponse
-     */
-    private function scheduleNextPollOnAuthenticationPage(): JsonResponse
-    {
-        return $this->generateAuthenticationStatusResponse('pending');
-    }
-
-    /**
-     * Generate a response for authentication.html: Ask the user to retry.
-     *
-     * @return JsonResponse
-     */
-    private function timeoutNeedsManualRetry(): JsonResponse
-    {
-        return $this->generateAuthenticationStatusResponse('challenge-expired');
-    }
-
-    /**
-     * Generate a response for authentication.html: refresh the page.
-     *
-     * @return JsonResponse
-     */
-    private function refreshAuthenticationPage(): JsonResponse
-    {
-        return $this->generateAuthenticationStatusResponse('needs-refresh');
-    }
-
-    /**
-     * Generate a status response for authentication.html.
-     *
-     * The javascript in the authentication page expects one of three 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.
-     *
-     * @return JsonResponse
-     */
-    private function generateAuthenticationStatusResponse(string $status): JsonResponse
-    {
-        return new JsonResponse($status);
-    }
-
     /**
      * Generate a notification response for authentication.html.
      *
diff --git a/src/Controller/AuthenticationStatusController.php b/src/Controller/AuthenticationStatusController.php
new file mode 100644
index 00000000..ade35c09
--- /dev/null
+++ b/src/Controller/AuthenticationStatusController.php
@@ -0,0 +1,143 @@
+<?php
+
+declare(strict_types = 1);
+
+/**
+ * Copyright 2018 SURFnet B.V.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+namespace Surfnet\Tiqr\Controller;
+
+use Exception;
+use InvalidArgumentException;
+use Psr\Log\LoggerInterface;
+use Surfnet\GsspBundle\Service\AuthenticationService;
+use Surfnet\GsspBundle\Service\StateHandlerInterface;
+use Surfnet\Tiqr\Tiqr\TiqrServiceInterface;
+use Surfnet\Tiqr\WithContextLogger;
+use Symfony\Component\HttpFoundation\JsonResponse;
+use Symfony\Component\Routing\Attribute\Route;
+
+class AuthenticationStatusController
+{
+    public function __construct(
+        private readonly AuthenticationService $authenticationService,
+        private readonly StateHandlerInterface $stateHandler,
+        private readonly TiqrServiceInterface $tiqrService,
+        private readonly LoggerInterface $logger
+    ) {
+    }
+
+    /**
+     * @throws InvalidArgumentException
+     */
+    #[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();
+        }
+
+        $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:
+     *
+     *  - 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.
+     *
+     * @return JsonResponse
+     */
+    private function generateAuthenticationStatusResponse(string $status): JsonResponse
+    {
+        return new JsonResponse($status);
+    }
+
+    /**
+     * Generate a response for authentication.html: refresh the page.
+     *
+     * @return JsonResponse
+     */
+    private function refreshAuthenticationPage(): JsonResponse
+    {
+        return $this->generateAuthenticationStatusResponse('needs-refresh');
+    }
+
+    /**
+     * Check if the authentication challenge is expired.
+     *
+     * If the challenge is expired, the page should be refreshed so a new
+     * challenge and QR code is generated.
+     *
+     * @return bool
+     */
+    private function authenticationChallengeIsExpired(): bool
+    {
+        // The use of authenticationUrl() here is a hack, because it depends on an implementation detail
+        // of this function.
+        // Effectively this does a $this->_stateStorage->getValue(self::PREFIX_CHALLENGE . $sessionKey);
+        // To check that the session key still exists in the Tiqr_Service's state storage
+        try {
+            $this->tiqrService->authenticationUrl();
+        } catch (Exception) {
+            return true;
+        }
+        return false;
+    }
+
+    /**
+     * Generate a response for authentication.html: Ask the user to retry.
+     *
+     * @return JsonResponse
+     */
+    private function timeoutNeedsManualRetry(): JsonResponse
+    {
+        return $this->generateAuthenticationStatusResponse('challenge-expired');
+    }
+
+    /**
+     * Authentication is pending, schedule a new poll action.
+     *
+     * @return JsonResponse
+     */
+    private function scheduleNextPollOnAuthenticationPage(): JsonResponse
+    {
+        return $this->generateAuthenticationStatusResponse('pending');
+    }
+}