Consolidate simple feedback routes into generic feedbackAction

Prior to this change, there were many 'empty' identical twig templates in the error process.
This change replaces them with a 'generic-error' twig template.

Resolves #1758

Author: johanib

config/services/ci/controllers.yml

@@ -26,6 +26,7 @@ services:
         arguments:
             - '@twig'
             - '@OpenConext\EngineBlock\Service\FeedbackStateHelper'
+            - '@OpenConext\EngineBlockBundle\Controller\FeedbackController'
 
     engineblock.functional_test.controller.consent:
         class: OpenConext\EngineBlockFunctionalTestingBundle\Controllers\ConsentController

src/OpenConext/EngineBlockBundle/Controller/FeedbackController.php

@@ -66,13 +66,73 @@ public function unableToReceiveMessageAction()
     #[Route(
         path: '/authentication/feedback/unsolicited-response',
         name: 'authentication_feedback_unsolicited_response',
+        defaults: [
+            'pageIdentifier' => 'unsolicited-response',
+            'statusCode' => 400
+        ],
         methods: ['GET']
     )]
-    public function unsolicitedResponseAction(): Response
+    #[Route(
+        path: '/authentication/feedback/session-lost',
+        name: 'authentication_feedback_session_lost',
+        defaults: [
+            'pageIdentifier' => 'session-lost',
+            'statusCode' => 400
+        ],
+        methods: ['GET']
+    )]
+    #[Route(
+        path: '/authentication/feedback/session-not-started',
+        name: 'authentication_feedback_session_not_started',
+        defaults: [
+            'pageIdentifier' => 'session-not-started',
+            'statusCode' => 400
+        ],
+        methods: ['GET']
+    )]
+    #[Route(
+        path: '/authentication/feedback/invalid-acs-binding',
+        name: 'authentication_feedback_invalid_acs_binding',
+        defaults: [
+            'pageIdentifier' => 'invalid-acs-binding',
+            'statusCode' => 400
+        ],
+        methods: ['GET']
+    )]
+    #[Route(
+        path: '/authentication/feedback/received-error-status-code',
+        name: 'authentication_feedback_received_error_status_code',
+        defaults: [
+            'pageIdentifier' => 'received-error-status-code',
+            'statusCode' => 400
+        ],
+        methods: ['GET']
+    )]
+    #[Route(
+        path: '/authentication/feedback/unknown_requesterid_in_authnrequest',
+        name: 'authentication_feedback_unknown_requesterid_in_authnrequest',
+        defaults: [
+            'pageIdentifier' => 'unknown-requesterid-in-authnrequest',
+            'statusCode' => 400
+        ],
+        methods: ['GET']
+    )]
+    #[Route(
+        path: '/authentication/feedback/authentication-limit-exceeded',
+        name: 'authentication_feedback_authentication_limit_exceeded',
+        defaults: [
+            'pageIdentifier' => 'authentication-limit-exceeded',
+            'statusCode' => 429
+        ],
+        methods: ['GET']
+    )]
+    public function feedbackAction(string $pageIdentifier, int $statusCode): Response
     {
         return new Response(
-            $this->twig->render('@theme/Authentication/View/Feedback/unsolicited-response.html.twig'),
-            400
+            $this->twig->render('@theme/Authentication/View/Feedback/generic-error.html.twig', [
+                'pageIdentifier' => $pageIdentifier,
+            ]),
+            $statusCode
         );
     }
 
@@ -85,19 +145,6 @@ public function unknownErrorAction()
         );
     }
 
-
-    #[Route(path: '/authentication/feedback/session-lost', name: 'authentication_feedback_session_lost', methods: ['GET'])]
-    public function sessionLostAction()
-    {
-        return new Response($this->twig->render('@theme/Authentication/View/Feedback/session-lost.html.twig'), 400);
-    }
-
-    #[Route(path: '/authentication/feedback/session-not-started', name: 'authentication_feedback_session_not_started', methods: ['GET'])]
-    public function sessionNotStartedAction()
-    {
-        return new Response($this->twig->render('@theme/Authentication/View/Feedback/session-not-started.html.twig'), 400);
-    }
-
     #[Route(path: '/authentication/feedback/no-idps', name: 'authentication_feedback_no_idps', methods: ['GET'])]
     public function noIdpsAction()
     {
@@ -310,26 +357,6 @@ public function customAction(Request $request)
         );
     }
 
-    #[Route(path: '/authentication/feedback/invalid-acs-binding', name: 'authentication_feedback_invalid_acs_binding', methods: ['GET'])]
-    public function invalidAcsBindingAction()
-    {
-        // @todo Send 4xx or 5xx header depending on invalid binding came from request or configured metadata
-        return new Response($this->twig->render('@theme/Authentication/View/Feedback/invalid-acs-binding.html.twig'));
-    }
-
-    #[Route(
-        path: '/authentication/feedback/received-error-status-code',
-        name: 'authentication_feedback_received_error_status_code',
-        methods: ['GET']
-    )]
-    public function receivedErrorStatusCodeAction()
-    {
-        // @todo Send 4xx or 5xx header?
-        return new Response(
-            $this->twig->render('@theme/Authentication/View/Feedback/received-error-status-code.html.twig')
-        );
-    }
-
     #[Route(
         path: '/authentication/feedback/received-invalid-signed-response',
         name: 'authentication_feedback_signature_verification_failed',
@@ -352,20 +379,6 @@ public function receivedInvalidResponseAction()
         );
     }
 
-    #[Route(
-        path: '/authentication/feedback/unknown_requesterid_in_authnrequest',
-        name: 'authentication_feedback_unknown_requesterid_in_authnrequest',
-        methods: ['GET']
-    )]
-    public function unknownRequesterIdInAuthnRequestAction()
-    {
-        return new Response(
-            $this->twig->render('@theme/Authentication/View/Feedback/unknown-requesterid-in-authnrequest.html.twig'),
-            400
-        );
-    }
-
-
     #[Route(path: '/authentication/feedback/authorization-policy-violation', name: 'authentication_feedback_pep_violation', methods: ['GET'])]
     public function authorizationPolicyViolationAction(Request $request)
     {
@@ -442,19 +455,6 @@ public function stuckInAuthenticationLoopAction()
         );
     }
 
-    #[Route(
-        path: '/authentication/feedback/authentication-limit-exceeded',
-        name: 'authentication_feedback_authentication_limit_exceeded',
-        methods: ['GET']
-    )]
-    public function authenticationLimitExceededAction()
-    {
-        return new Response(
-            $this->twig->render('@theme/Authentication/View/Feedback/authentication-limit-exceeded.html.twig'),
-            429
-        );
-    }
-
     #[Route(
         path: '/authentication/feedback/invalid-request-method-on-sso',
         name: 'authentication_feedback_no_authentication_request_received',

src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/FeedbackController.php

@@ -19,6 +19,7 @@
 namespace OpenConext\EngineBlockFunctionalTestingBundle\Controllers;
 
 use OpenConext\EngineBlock\Service\FeedbackStateHelperInterface;
+use OpenConext\EngineBlockBundle\Controller\FeedbackController as RealFeedbackController;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
@@ -35,12 +36,16 @@ class FeedbackController extends AbstractController
 
     private FeedbackStateHelperInterface $feedbackStateHelper;
 
+    private RealFeedbackController $feedbackController;
+
     public function __construct(
         Environment $twig,
-        FeedbackStateHelperInterface $feedbackStateHelper
+        FeedbackStateHelperInterface $feedbackStateHelper,
+        RealFeedbackController $feedbackController
     ) {
         $this->twig = $twig;
         $this->feedbackStateHelper = $feedbackStateHelper;
+        $this->feedbackController = $feedbackController;
     }
 
     /**
@@ -56,12 +61,16 @@ public function feedbackAction(Request $request)
         $feedbackInfo = $this->getFeedbackInfo($request);
         $parameters = $this->getTemplateParameters($request);
 
+        $this->feedbackStateHelper->storeFeedbackInfo($feedbackInfo);
+
         $template = sprintf(
             '@theme/Authentication/View/Feedback/%s.html.twig',
             $key
         );
 
-        $this->feedbackStateHelper->storeFeedbackInfo($feedbackInfo);
+        if (!$this->twig->getLoader()->exists($template)) {
+            return $this->feedbackController->feedbackAction($key, 200);
+        }
 
         return new Response($this->twig->render($template, $parameters), 200);
     }
@@ -106,14 +115,12 @@ private function getFeedbackInfo(Request $request)
      * @param Request $request
      * @return mixed|string
      */
-    private function getTemplateParameters(Request $request)
+    private function getTemplateParameters(Request $request): array
     {
         $default = '{}';
 
         $parameters = $request->query->getString('parameters', $default);
 
-        $parameters = json_decode($parameters, true);
-
-        return $parameters;
+        return json_decode($parameters, true);
     }
 }

src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature

@@ -49,6 +49,10 @@ Feature:
     When I go to Engineblock URL "/functional-testing/feedback?template=session-lost&feedback-info=%7B%22requestId%22%3A%22test-abc%22%2C%22ipAddress%22%3A%221.2.3.4%22%2C%22artCode%22%3A%2231914%22%7D"
     Then I should see "your session was lost"
 
+  Scenario: The session-lost feedback route renders correctly
+    When I go to Engineblock URL "/authentication/feedback/session-lost"
+    Then I should see "your session was lost"
+
   Scenario: When a IdP specific error page is shown and a translation is not configured the support emailaddress of the IdP should be hidden
     Given The clock on the IdP "Dummy Idp" is ahead
     And I have configured the following translations:

…/authentication-limit-exceeded.html.twig …on/View/Feedback/generic-error.html.twigtheme/base/templates/modules/Authentication/View/Feedback/authentication-limit-exceeded.html.twig renamed to theme/base/templates/modules/Authentication/View/Feedback/generic-error.html.twig theme/base/templates/modules/Authentication/View/Feedback/authentication-limit-exceeded.html.twig renamed to theme/base/templates/modules/Authentication/View/Feedback/generic-error.html.twig

@@ -1,8 +1,9 @@
 {% extends '@theme/Default/View/Error/error.html.twig' %}
 
-{% set pageTitle = 'error_authentication_limit_exceeded'|trans %}
+{% set _key = 'error_' ~ pageIdentifier|replace({'-': '_'}) %}
+{% set pageTitle = (_key)|trans %}
 {% block pageTitle %}{{ pageTitle }}{% endblock %}
 {% block title %}{{ parent() }}{% endblock %}
 {% block pageHeading %}{{ pageTitle }}{% endblock %}
 
-{% block errorMessage %}{{ 'error_authentication_limit_exceeded_desc'|trans }}{% endblock %}
+{% block errorMessage %}{{ (_key ~ '_desc')|trans }}{% endblock %}