From 9fbc943534dcef0844a411e2d469b30825f8982b Mon Sep 17 00:00:00 2001 From: Johan Kromhout Date: Wed, 29 Apr 2026 13:51:26 +0200 Subject: [PATCH 1/5] Fix feedbackInfo session bleed-through between auth flows (#1795) feedbackInfo was stored as a flat dict in $_SESSION['feedbackInfo'] and merged with existing data on each write. This meant info collected during one auth flow (e.g. which IdP was involved) could appear on error pages in a completely separate flow. Additionally, currentServiceProvider and currentIdentityProvider were never cleared from the session after a successful login, so they would show up on error pages for unrelated early errors. The fix: - feedbackInfo is now keyed per SAML request ID so each auth flow has its own isolated bucket with no merging across flows - clearFlowContext() is called by ProcessedAssertionConsumer after a successful auth, clearing all flow context from the session - All session access moved from raw $_SESSION to the Symfony session - Session ops for feedbackInfo and flow context are centralised in a new FeedbackStateHelper class, split out from ProcessingStateHelper - FeedbackStateHelper is wired through DiContainerRuntime instead of the legacy DiContainer --- config/services/ci/controllers.yml | 4 ++++ .../EngineBlockBundle/Bridge/DiContainerRuntime.php | 9 +++++++++ .../EngineBlock/Test/Corto/Module/BindingsTest.php | 6 ++++++ 3 files changed, 19 insertions(+) diff --git a/config/services/ci/controllers.yml b/config/services/ci/controllers.yml index 0c0b7b5cc..d41c2d374 100644 --- a/config/services/ci/controllers.yml +++ b/config/services/ci/controllers.yml @@ -25,6 +25,10 @@ services: class: OpenConext\EngineBlockFunctionalTestingBundle\Controllers\FeedbackController arguments: - '@twig' +<<<<<<< HEAD +======= + - '@engineblock.compat.logger' +>>>>>>> 41926c2bd (Fix feedbackInfo session bleed-through between auth flows (#1795)) - '@OpenConext\EngineBlock\Service\FeedbackStateHelper' engineblock.functional_test.controller.consent: diff --git a/src/OpenConext/EngineBlockBundle/Bridge/DiContainerRuntime.php b/src/OpenConext/EngineBlockBundle/Bridge/DiContainerRuntime.php index 47a0f71a4..e4c4f12f1 100644 --- a/src/OpenConext/EngineBlockBundle/Bridge/DiContainerRuntime.php +++ b/src/OpenConext/EngineBlockBundle/Bridge/DiContainerRuntime.php @@ -20,7 +20,10 @@ use OpenConext\EngineBlock\Service\FeedbackInfoCollectorInterface; use OpenConext\EngineBlock\Service\FeedbackStateHelperInterface; +<<<<<<< HEAD use OpenConext\EngineBlockBundle\Service\WayfRenderer; +======= +>>>>>>> 41926c2bd (Fix feedbackInfo session bleed-through between auth flows (#1795)) use Twig\Environment; /** @@ -34,6 +37,7 @@ public function __construct( public Environment $twig, +<<<<<<< HEAD public WayfRenderer $wayfRenderer, public FeedbackStateHelperInterface $feedbackStateHelper, public FeedbackInfoCollectorInterface $feedbackInfoCollector, @@ -44,5 +48,10 @@ public function __construct( public function getPreferredIdpEntityIds(): array { return $this->preferredIdpEntityIds; +======= + public FeedbackStateHelperInterface $feedbackStateHelper, + public FeedbackInfoCollectorInterface $feedbackInfoCollector, + ) { +>>>>>>> 41926c2bd (Fix feedbackInfo session bleed-through between auth flows (#1795)) } } diff --git a/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php b/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php index b71041e8c..9edc0279a 100644 --- a/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php +++ b/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php @@ -63,10 +63,16 @@ public function setUp(): void $engineBlock = \EngineBlock_ApplicationSingleton::getInstance(); $engineBlock->setDiContainerRuntime(new DiContainerRuntime( +<<<<<<< HEAD $this->createStub(Twig\Environment::class), $this->createStub(WayfRenderer::class), $this->createStub(FeedbackStateHelperInterface::class), $this->createStub(FeedbackInfoCollectorInterface::class), +======= + Phake::mock(Twig\Environment::class), + m::mock(FeedbackStateHelperInterface::class), + m::mock(FeedbackInfoCollectorInterface::class), +>>>>>>> 41926c2bd (Fix feedbackInfo session bleed-through between auth flows (#1795)) )); $this->bindings = new EngineBlock_Corto_Module_Bindings($this->proxyServer); From 0f39841f60b9a21341b47e759116f79659a3761f Mon Sep 17 00:00:00 2001 From: Johan Kromhout Date: Mon, 4 May 2026 16:25:29 +0200 Subject: [PATCH 2/5] 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 --- config/services/ci/controllers.yml | 1 + .../Controller/FeedbackController.php | 126 +++++++++--------- .../Controllers/FeedbackController.php | 19 ++- .../Features/FeedbackFooters.feature | 4 + ...eded.html.twig => generic-error.html.twig} | 5 +- .../received-error-status-code.html.twig | 8 -- .../View/Feedback/session-lost.html.twig | 8 -- .../Feedback/session-not-started.html.twig | 8 -- ...nown-requesterid-in-authnrequest.html.twig | 8 -- .../Feedback/unsolicited-response.html.twig | 8 -- .../View/Feedback/generic-error.html.twig} | 5 +- .../Feedback/invalid-acs-binding.html.twig | 8 -- .../received-error-status-code.html.twig | 8 -- .../View/Feedback/session-lost.html.twig | 8 -- .../Feedback/session-not-started.html.twig | 8 -- ...nown-requesterid-in-authnrequest.html.twig | 8 -- .../Feedback/unsolicited-response.html.twig | 8 -- 17 files changed, 87 insertions(+), 161 deletions(-) rename theme/base/templates/modules/Authentication/View/Feedback/{authentication-limit-exceeded.html.twig => generic-error.html.twig} (56%) delete mode 100644 theme/base/templates/modules/Authentication/View/Feedback/received-error-status-code.html.twig delete mode 100644 theme/base/templates/modules/Authentication/View/Feedback/session-lost.html.twig delete mode 100644 theme/base/templates/modules/Authentication/View/Feedback/session-not-started.html.twig delete mode 100644 theme/base/templates/modules/Authentication/View/Feedback/unknown-requesterid-in-authnrequest.html.twig delete mode 100644 theme/base/templates/modules/Authentication/View/Feedback/unsolicited-response.html.twig rename theme/{base/templates/modules/Authentication/View/Feedback/invalid-acs-binding.html.twig => openconext/templates/modules/Authentication/View/Feedback/generic-error.html.twig} (56%) delete mode 100644 theme/openconext/templates/modules/Authentication/View/Feedback/invalid-acs-binding.html.twig delete mode 100644 theme/openconext/templates/modules/Authentication/View/Feedback/received-error-status-code.html.twig delete mode 100644 theme/openconext/templates/modules/Authentication/View/Feedback/session-lost.html.twig delete mode 100644 theme/openconext/templates/modules/Authentication/View/Feedback/session-not-started.html.twig delete mode 100644 theme/openconext/templates/modules/Authentication/View/Feedback/unknown-requesterid-in-authnrequest.html.twig delete mode 100644 theme/openconext/templates/modules/Authentication/View/Feedback/unsolicited-response.html.twig diff --git a/config/services/ci/controllers.yml b/config/services/ci/controllers.yml index d41c2d374..4a0b5aa3d 100644 --- a/config/services/ci/controllers.yml +++ b/config/services/ci/controllers.yml @@ -30,6 +30,7 @@ services: - '@engineblock.compat.logger' >>>>>>> 41926c2bd (Fix feedbackInfo session bleed-through between auth flows (#1795)) - '@OpenConext\EngineBlock\Service\FeedbackStateHelper' + - '@OpenConext\EngineBlockBundle\Controller\FeedbackController' engineblock.functional_test.controller.consent: class: OpenConext\EngineBlockFunctionalTestingBundle\Controllers\ConsentController diff --git a/src/OpenConext/EngineBlockBundle/Controller/FeedbackController.php b/src/OpenConext/EngineBlockBundle/Controller/FeedbackController.php index 998f9cf6a..d30dba14b 100644 --- a/src/OpenConext/EngineBlockBundle/Controller/FeedbackController.php +++ b/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', diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/FeedbackController.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/FeedbackController.php index d37273856..63d8d0e04 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/FeedbackController.php +++ b/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); } } diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature index b136bc974..72f3765f5 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature +++ b/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: diff --git a/theme/base/templates/modules/Authentication/View/Feedback/authentication-limit-exceeded.html.twig b/theme/base/templates/modules/Authentication/View/Feedback/generic-error.html.twig similarity index 56% rename from theme/base/templates/modules/Authentication/View/Feedback/authentication-limit-exceeded.html.twig rename to theme/base/templates/modules/Authentication/View/Feedback/generic-error.html.twig index cbb1c57e9..b5bcfe5b2 100644 --- a/theme/base/templates/modules/Authentication/View/Feedback/authentication-limit-exceeded.html.twig +++ b/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 %} diff --git a/theme/base/templates/modules/Authentication/View/Feedback/received-error-status-code.html.twig b/theme/base/templates/modules/Authentication/View/Feedback/received-error-status-code.html.twig deleted file mode 100644 index f4e1d2ac0..000000000 --- a/theme/base/templates/modules/Authentication/View/Feedback/received-error-status-code.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_received_error_status_code'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }}{% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_received_error_status_code_desc'|trans }}{% endblock %} diff --git a/theme/base/templates/modules/Authentication/View/Feedback/session-lost.html.twig b/theme/base/templates/modules/Authentication/View/Feedback/session-lost.html.twig deleted file mode 100644 index afc1f3003..000000000 --- a/theme/base/templates/modules/Authentication/View/Feedback/session-lost.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_session_lost'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }}{% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_session_lost_desc'|trans }}{% endblock %} diff --git a/theme/base/templates/modules/Authentication/View/Feedback/session-not-started.html.twig b/theme/base/templates/modules/Authentication/View/Feedback/session-not-started.html.twig deleted file mode 100644 index 1f4b4d28f..000000000 --- a/theme/base/templates/modules/Authentication/View/Feedback/session-not-started.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_session_not_started'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }}{% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_session_not_started_desc'|trans }}{% endblock %} diff --git a/theme/base/templates/modules/Authentication/View/Feedback/unknown-requesterid-in-authnrequest.html.twig b/theme/base/templates/modules/Authentication/View/Feedback/unknown-requesterid-in-authnrequest.html.twig deleted file mode 100644 index 9fcc85b40..000000000 --- a/theme/base/templates/modules/Authentication/View/Feedback/unknown-requesterid-in-authnrequest.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_unknown_requesterid_in_authnrequest'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }}{% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_unknown_requesterid_in_authnrequest_desc'|trans }}{% endblock %} diff --git a/theme/base/templates/modules/Authentication/View/Feedback/unsolicited-response.html.twig b/theme/base/templates/modules/Authentication/View/Feedback/unsolicited-response.html.twig deleted file mode 100644 index e201837af..000000000 --- a/theme/base/templates/modules/Authentication/View/Feedback/unsolicited-response.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_unsolicited_response'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }}{% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_unsolicited_response_desc'|trans }}{% endblock %} diff --git a/theme/base/templates/modules/Authentication/View/Feedback/invalid-acs-binding.html.twig b/theme/openconext/templates/modules/Authentication/View/Feedback/generic-error.html.twig similarity index 56% rename from theme/base/templates/modules/Authentication/View/Feedback/invalid-acs-binding.html.twig rename to theme/openconext/templates/modules/Authentication/View/Feedback/generic-error.html.twig index e7e45a6fc..b5bcfe5b2 100644 --- a/theme/base/templates/modules/Authentication/View/Feedback/invalid-acs-binding.html.twig +++ b/theme/openconext/templates/modules/Authentication/View/Feedback/generic-error.html.twig @@ -1,8 +1,9 @@ {% extends '@theme/Default/View/Error/error.html.twig' %} -{% set pageTitle = 'error_invalid_acs_binding'|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_invalid_acs_binding_desc'|trans }}{% endblock %} +{% block errorMessage %}{{ (_key ~ '_desc')|trans }}{% endblock %} diff --git a/theme/openconext/templates/modules/Authentication/View/Feedback/invalid-acs-binding.html.twig b/theme/openconext/templates/modules/Authentication/View/Feedback/invalid-acs-binding.html.twig deleted file mode 100644 index 845484408..000000000 --- a/theme/openconext/templates/modules/Authentication/View/Feedback/invalid-acs-binding.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_invalid_acs_binding'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }} - {{ pageTitle }} {% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_invalid_acs_binding_desc'|trans }}{% endblock %} diff --git a/theme/openconext/templates/modules/Authentication/View/Feedback/received-error-status-code.html.twig b/theme/openconext/templates/modules/Authentication/View/Feedback/received-error-status-code.html.twig deleted file mode 100644 index 4d42a47b1..000000000 --- a/theme/openconext/templates/modules/Authentication/View/Feedback/received-error-status-code.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_received_error_status_code'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }} - {{ pageTitle }} {% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_received_error_status_code_desc'|trans }}{% endblock %} diff --git a/theme/openconext/templates/modules/Authentication/View/Feedback/session-lost.html.twig b/theme/openconext/templates/modules/Authentication/View/Feedback/session-lost.html.twig deleted file mode 100644 index 018bc04be..000000000 --- a/theme/openconext/templates/modules/Authentication/View/Feedback/session-lost.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_session_lost'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }} - {{ pageTitle }} {% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_session_lost_desc'|trans }}{% endblock %} diff --git a/theme/openconext/templates/modules/Authentication/View/Feedback/session-not-started.html.twig b/theme/openconext/templates/modules/Authentication/View/Feedback/session-not-started.html.twig deleted file mode 100644 index 1f153ec23..000000000 --- a/theme/openconext/templates/modules/Authentication/View/Feedback/session-not-started.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_session_not_started'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }} - {{ pageTitle }} {% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_session_not_started_desc'|trans }}{% endblock %} diff --git a/theme/openconext/templates/modules/Authentication/View/Feedback/unknown-requesterid-in-authnrequest.html.twig b/theme/openconext/templates/modules/Authentication/View/Feedback/unknown-requesterid-in-authnrequest.html.twig deleted file mode 100644 index 992ed8dac..000000000 --- a/theme/openconext/templates/modules/Authentication/View/Feedback/unknown-requesterid-in-authnrequest.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_unknown_requesterid_in_authnrequest'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }} - {{ pageTitle }} {% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_unknown_requesterid_in_authnrequest'|trans }}{% endblock %} diff --git a/theme/openconext/templates/modules/Authentication/View/Feedback/unsolicited-response.html.twig b/theme/openconext/templates/modules/Authentication/View/Feedback/unsolicited-response.html.twig deleted file mode 100644 index 44b8dce4a..000000000 --- a/theme/openconext/templates/modules/Authentication/View/Feedback/unsolicited-response.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_unsolicited_response'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }} - {{ pageTitle }} {% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_unsolicited_response_desc'|trans }}{% endblock %} From 5f2fa0519a475e5b604e0f657905ef3028f6d11c Mon Sep 17 00:00:00 2001 From: Johan Kromhout Date: Tue, 5 May 2026 09:50:43 +0200 Subject: [PATCH 3/5] Consolidate additional routes These templates were not using the 'new' translation key convention yet. (the old translation keys are not removed, because `invalid-acs-location` still references them). --- languages/messages.en.php | 6 +- languages/messages.nl.php | 6 +- languages/messages.pt.php | 6 +- .../Controller/FeedbackController.php | 55 ++++++++----------- .../Features/FeedbackFooters.feature | 12 ++++ .../View/Feedback/generic-error.html.twig | 6 ++ .../Feedback/stepup-callout-unknown.html.twig | 8 --- .../stepup-callout-user-cancelled.html.twig | 8 --- .../unable-to-receive-message.html.twig | 8 --- 9 files changed, 53 insertions(+), 62 deletions(-) delete mode 100644 theme/base/templates/modules/Authentication/View/Feedback/stepup-callout-unknown.html.twig delete mode 100644 theme/base/templates/modules/Authentication/View/Feedback/stepup-callout-user-cancelled.html.twig delete mode 100644 theme/base/templates/modules/Authentication/View/Feedback/unable-to-receive-message.html.twig diff --git a/languages/messages.en.php b/languages/messages.en.php index 22c77ef2d..4f4020887 100644 --- a/languages/messages.en.php +++ b/languages/messages.en.php @@ -178,6 +178,8 @@ 'error_authorization_policy_violation_info_no_idp_name' => 'Message from your %organisationNoun%: ', 'error_no_message' => 'Error - No message received', 'error_no_message_desc' => 'We were expecting a SAML message, but did not get one. Something went wrong. Please try again.', + 'error_unable_to_receive_message' => 'Error - No message received', + 'error_unable_to_receive_message_desc' => 'We were expecting a SAML message, but did not get one. Something went wrong. Please try again.', 'error_invalid_acs_location' => 'The given "Assertion Consumer Service" is unknown or invalid.', 'error_invalid_acs_binding' => 'Error - Invalid ACS binding type', 'error_invalid_acs_binding_desc' => 'The provided or configured "Assertion Consumer Service" Binding Type is unknown or invalid.', @@ -263,13 +265,13 @@ 'error_clock_issue_title' => 'Error - The Assertion is not yet valid or has expired', 'error_clock_issue_desc' => 'This is likely because the difference in time between %idpName% and %suiteName% it too large. Please verify that the time on the %organisationNoun% is correct.', 'error_clock_issue_desc_no_idp_name' => 'This is likely because the difference in time between %organisationNoun% and %suiteName% it too large. Please verify that the time on the IdP is correct.', - 'error_stepup_callout_unknown_title' => 'Error - Unknown strong authentication failure', + 'error_stepup_callout_unknown' => 'Error - Unknown strong authentication failure', 'error_stepup_callout_unknown_desc' => 'Logging in with strong authentication has failed and we don\'t know exactly why . Please try again first by going back to the service and logging in again . If this doesn\'t work, please contact the service desk of your %organisationNoun%.', 'error_stepup_callout_unmet_loa_title' => 'Error - No suitable token found', 'error_stepup_callout_unmet_loa_desc' => 'To continue to this service, a registered token with a certain level of assurance is required. Currently, you either haven\'t registered a token at all, or the level of assurance of the token you did register is too low. See the link below for more information about the registration process.', 'error_stepup_callout_unmet_loa_link_text' => 'Read more about the registration process.', 'error_stepup_callout_unmet_loa_link_target' => 'https://support.surfconext.nl/stepup-noauthncontext-en', - 'error_stepup_callout_user_cancelled_title' => 'Error - Logging in cancelled', + 'error_stepup_callout_user_cancelled' => 'Error - Logging in cancelled', 'error_stepup_callout_user_cancelled_desc' => 'You have aborted the login process. Go back to the service if you want to try again.', 'error_metadata_entity_id_not_found' => 'Metadata can not be generated', 'error_metadata_entity_id_not_found_desc' => 'The following error occurred: %message%', diff --git a/languages/messages.nl.php b/languages/messages.nl.php index f6e68e94e..91a215948 100644 --- a/languages/messages.nl.php +++ b/languages/messages.nl.php @@ -178,6 +178,8 @@ 'error_authorization_policy_violation_info_no_idp_name' => 'Bericht van je %organisationNoun%: ', 'error_no_message' => 'Fout - Geen bericht ontvangen', 'error_no_message_desc' => 'We verwachtten een SAML bericht, maar we hebben er geen ontvangen. Er is iets fout gegaan. Probeer het alstublieft opnieuw.', + 'error_unable_to_receive_message' => 'Fout - Geen bericht ontvangen', + 'error_unable_to_receive_message_desc' => 'We verwachtten een SAML bericht, maar we hebben er geen ontvangen. Er is iets fout gegaan. Probeer het alstublieft opnieuw.', 'error_invalid_acs_location' => 'De opgegeven "Assertion Consumer Service" is onjuist of bestaat niet.', 'error_invalid_acs_binding' => 'Fout - Onjuist ACS binding type', 'error_invalid_acs_binding_desc' => 'Het opgegeven of geconfigureerde "Assertion Consumer Service" Binding Type is onjuist of bestaat niet.', @@ -261,13 +263,13 @@ 'error_clock_issue_title' => 'Fout - De Assertion is nog niet geldig of is verlopen', 'error_clock_issue_desc' => 'Dit komt waarschijnlijk doordat de tijd tussen %idpName% en %suiteName% te ver uiteen loopt. Controleer de tijd op de %organisationNoun%.', 'error_clock_issue_desc_no_idp_name' => 'Dit komt waarschijnlijk doordat de tijd tussen de %organisationNoun% en %suiteName% te ver uiteen loopt. Controleer de tijd op de IdP.', - 'error_stepup_callout_unknown_title' => 'Fout - Onbekend sterke authenticatie probleem', + 'error_stepup_callout_unknown' => 'Fout - Onbekend sterke authenticatie probleem', 'error_stepup_callout_unknown_desc' => 'Inloggen met sterke authenticatie is niet gelukt en we weten niet precies waarom. Probeer het eerst eens opnieuw door terug te gaan naar de dienst en opnieuw in te loggen. Lukt dit niet, neem dan contact op met de helpdesk van je %organisationNoun%.', 'error_stepup_callout_unmet_loa_title' => 'Fout - Geen geschikt token gevonden', 'error_stepup_callout_unmet_loa_desc' => 'Om toegang te krijgen tot deze dienst heb je een geregistreerd token nodig met een bepaald zekerheidsniveau. Je hebt nu ofwel geen token geregistreerd, of het zekerheidsniveau van het token dat je hebt geregistreerd is te laag. Volg de link hieronder voor meer informatie over het registratieproces.', 'error_stepup_callout_unmet_loa_link_text' => 'Lees meer over het registratieproces.', 'error_stepup_callout_unmet_loa_link_target' => 'https://support.surfconext.nl/stepup-noauthncontext-nl', - 'error_stepup_callout_user_cancelled_title' => 'Fout - Inloggen afgebroken', + 'error_stepup_callout_user_cancelled' => 'Fout - Inloggen afgebroken', 'error_stepup_callout_user_cancelled_desc' => 'Je hebt het inloggen afgebroken. Ga terug naar de dienst als je het opnieuw wilt proberen.', 'error_metadata_entity_id_not_found' => 'Metadata kan niet gegenereerd worden', 'error_metadata_entity_id_not_found_desc' => 'De volgende fout is opgetreden: %message%', diff --git a/languages/messages.pt.php b/languages/messages.pt.php index 23350ae6a..525e5ff8b 100644 --- a/languages/messages.pt.php +++ b/languages/messages.pt.php @@ -176,6 +176,8 @@ 'error_authorization_policy_violation_info_no_idp_name' => 'Mensagem da sua %organisationNoun%: ', 'error_no_message' => 'Erro - Não foi recebido nenhuma mensagem', 'error_no_message_desc' => 'Estávamos a aguardar uma mensagem, mas não chegou nenhuma? Alguma coisa correu mal. Tente de novo por favor.', + 'error_unable_to_receive_message' => 'Erro - Não foi recebido nenhuma mensagem', + 'error_unable_to_receive_message_desc' => 'Estávamos a aguardar uma mensagem, mas não chegou nenhuma? Alguma coisa correu mal. Tente de novo por favor.', 'error_invalid_acs_location' => 'O "Serviço de Consumidor de Asserção" fornecido é desconhecido ou inválido.', 'error_invalid_acs_binding' => 'O ACS "Binding Type" é inválido', 'error_invalid_acs_binding_desc' => 'O "Binding Type" do "Serviço de Consumidor de Asserção" fornecido ou configurado é desconhecido ou inválido.', @@ -255,13 +257,13 @@ 'error_clock_issue_title' => 'Erro - A asserção ainda não é válida ou pode ter expirado', 'error_clock_issue_desc' => '

Por favor, verifique se a hora no IdP está correta.

', 'error_clock_issue_desc_no_idp_name' => '

Por favor, verifique se a hora no IdP está correta.

', - 'error_stepup_callout_unknown_title' => 'Erro - falha por autenticação forte desconhecida', + 'error_stepup_callout_unknown' => 'Erro - falha por autenticação forte desconhecida', 'error_stepup_callout_unknown_desc' => 'O login com autenticação forte falhou e não sabemos exatamente qual o motivo. Tente aceder de novo ao serviço e efetuar uma nova autenticação. Se voltar a não funcionar, entre em contato com o suporte técnico da sua %organisationNoun%.', 'error_stepup_callout_unmet_loa_title' => 'Erro - não foi encontrado nenhum token adequado', 'error_stepup_callout_unmet_loa_desc' => 'Para continuar neste serviço, é necessário que o token registado tenho um determinado nível de confiança. Atualmente, você não tem um token registado, ou o nível de confiança do seu token é muito baixo. Veja o endereço abaixo para mais informações sobre o processo de registo.', 'error_stepup_callout_unmet_loa_link_text' => 'Leia mais sobre o processo de registro.', 'error_stepup_callout_unmet_loa_link_target' => 'https://support.surfconext.nl/stepup-noauthncontext', - 'error_stepup_callout_user_cancelled_title' => 'Erro - Carregamento cancelado', + 'error_stepup_callout_user_cancelled' => 'Erro - Carregamento cancelado', 'error_stepup_callout_user_cancelled_desc' => 'Você cancelou o processo de autenticação. Volte ao serviço se você pretender tentar de novo.', 'error_metadata_entity_id_not_found' => 'Metadata can not be generated', 'error_metadata_entity_id_not_found_desc' => 'The following error occurred: %message%', diff --git a/src/OpenConext/EngineBlockBundle/Controller/FeedbackController.php b/src/OpenConext/EngineBlockBundle/Controller/FeedbackController.php index d30dba14b..e86c224a6 100644 --- a/src/OpenConext/EngineBlockBundle/Controller/FeedbackController.php +++ b/src/OpenConext/EngineBlockBundle/Controller/FeedbackController.php @@ -52,17 +52,12 @@ public function __construct( #[Route( path: '/authentication/feedback/unable-to-receive-message', name: 'authentication_feedback_unable_to_receive_message', + defaults: [ + 'pageIdentifier' => 'unable-to-receive-message', + 'statusCode' => 400 + ], methods: ['GET'] - ) - ] - public function unableToReceiveMessageAction() - { - return new Response( - $this->twig->render('@theme/Authentication/View/Feedback/unable-to-receive-message.html.twig'), - 400 - ); - } - + )] #[Route( path: '/authentication/feedback/unsolicited-response', name: 'authentication_feedback_unsolicited_response', @@ -126,6 +121,24 @@ public function unableToReceiveMessageAction() ], methods: ['GET'] )] + #[Route( + path: '/authentication/feedback/stepup-callout-unknown', + name: 'authentication_feedback_stepup_callout_unknown', + defaults: [ + 'pageIdentifier' => 'stepup-callout-unknown', + 'statusCode' => 400 + ], + methods: ['GET'] + )] + #[Route( + path: '/authentication/feedback/stepup-callout-user-cancelled', + name: 'authentication_feedback_stepup_callout_user_cancelled', + defaults: [ + 'pageIdentifier' => 'stepup-callout-user-cancelled', + 'statusCode' => 400 + ], + methods: ['GET'] + )] public function feedbackAction(string $pageIdentifier, int $statusCode): Response { return new Response( @@ -493,19 +506,6 @@ public function clockIssueAction() ); } - #[Route( - path: '/authentication/feedback/stepup-callout-user-cancelled', - name: 'authentication_feedback_stepup_callout_user_cancelled', - methods: ['GET'] - )] - public function stepupCalloutUserCancelledAction(Request $request) - { - return new Response( - $this->twig->render('@theme/Authentication/View/Feedback/stepup-callout-user-cancelled.html.twig'), - 400 - ); - } - #[Route(path: '/authentication/feedback/stepup-callout-unmet-loa', name: 'authentication_feedback_stepup_callout_unmet_loa', methods: ['GET'])] public function stepupCalloutUnmetLoaAction(Request $request) { @@ -515,15 +515,6 @@ public function stepupCalloutUnmetLoaAction(Request $request) ); } - #[Route(path: '/authentication/feedback/stepup-callout-unknown', name: 'authentication_feedback_stepup_callout_unknown', methods: ['GET'])] - public function stepupCalloutUnknownAction(Request $request) - { - return new Response( - $this->twig->render('@theme/Authentication/View/Feedback/stepup-callout-unknown.html.twig'), - 400 - ); - } - private function setFeedbackInformationOnSession(array $customFeedbackInfo): void { $this->feedbackStateHelper->mergeFeedbackInfo($customFeedbackInfo); diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature index 72f3765f5..1a459255e 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature @@ -53,6 +53,18 @@ Feature: When I go to Engineblock URL "/authentication/feedback/session-lost" Then I should see "your session was lost" + Scenario: The unable-to-receive-message feedback route renders correctly + When I go to Engineblock URL "/authentication/feedback/unable-to-receive-message" + Then I should see "No message received" + + Scenario: The stepup-callout-unknown feedback route renders correctly + When I go to Engineblock URL "/authentication/feedback/stepup-callout-unknown" + Then I should see "Unknown strong authentication failure" + + Scenario: The stepup-callout-user-cancelled feedback route renders correctly + When I go to Engineblock URL "/authentication/feedback/stepup-callout-user-cancelled" + Then I should see "Logging in cancelled" + 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: diff --git a/theme/base/templates/modules/Authentication/View/Feedback/generic-error.html.twig b/theme/base/templates/modules/Authentication/View/Feedback/generic-error.html.twig index b5bcfe5b2..024febeaa 100644 --- a/theme/base/templates/modules/Authentication/View/Feedback/generic-error.html.twig +++ b/theme/base/templates/modules/Authentication/View/Feedback/generic-error.html.twig @@ -1,5 +1,11 @@ {% extends '@theme/Default/View/Error/error.html.twig' %} +{# + # Possible translation keys: error_session_lost, error_session_not_started, error_unsolicited_response, + # error_invalid_acs_binding, error_received_error_status_code, error_unable_to_receive_message, + # error_unknown_requesterid_in_authnrequest, error_authentication_limit_exceeded, + # error_stepup_callout_unknown, error_stepup_callout_user_cancelled + #} {% set _key = 'error_' ~ pageIdentifier|replace({'-': '_'}) %} {% set pageTitle = (_key)|trans %} {% block pageTitle %}{{ pageTitle }}{% endblock %} diff --git a/theme/base/templates/modules/Authentication/View/Feedback/stepup-callout-unknown.html.twig b/theme/base/templates/modules/Authentication/View/Feedback/stepup-callout-unknown.html.twig deleted file mode 100644 index 60dbec0ed..000000000 --- a/theme/base/templates/modules/Authentication/View/Feedback/stepup-callout-unknown.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_stepup_callout_unknown_title'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }}{% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_stepup_callout_unknown_desc'|trans }}{% endblock %} diff --git a/theme/base/templates/modules/Authentication/View/Feedback/stepup-callout-user-cancelled.html.twig b/theme/base/templates/modules/Authentication/View/Feedback/stepup-callout-user-cancelled.html.twig deleted file mode 100644 index 275148c63..000000000 --- a/theme/base/templates/modules/Authentication/View/Feedback/stepup-callout-user-cancelled.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_stepup_callout_user_cancelled_title'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }}{% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_stepup_callout_user_cancelled_desc'|trans }}{% endblock %} diff --git a/theme/base/templates/modules/Authentication/View/Feedback/unable-to-receive-message.html.twig b/theme/base/templates/modules/Authentication/View/Feedback/unable-to-receive-message.html.twig deleted file mode 100644 index 1c0ff808a..000000000 --- a/theme/base/templates/modules/Authentication/View/Feedback/unable-to-receive-message.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_no_message'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }}{% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_no_message_desc'|trans }}{% endblock %} From 40b65715e6874e8f06406a9743c27d199b3db05f Mon Sep 17 00:00:00 2001 From: Johan Kromhout Date: Tue, 5 May 2026 10:39:55 +0200 Subject: [PATCH 4/5] Refactor the last remaining `error_no_message` to the new format. --- CHANGELOG.md | 40 +++++++++++++++++++ languages/messages.en.php | 5 +-- languages/messages.nl.php | 5 +-- languages/messages.pt.php | 5 +-- .../Controller/FeedbackController.php | 17 ++++---- .../Features/FeedbackFooters.feature | 4 ++ .../View/Feedback/generic-error.html.twig | 3 +- .../Feedback/invalid-acs-location.html.twig | 8 ---- 8 files changed, 61 insertions(+), 26 deletions(-) delete mode 100644 theme/base/templates/modules/Authentication/View/Feedback/invalid-acs-location.html.twig diff --git a/CHANGELOG.md b/CHANGELOG.md index feff76a89..cb0d2bde6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,46 @@ the EngineBlock wiki. Features: * Added `coin:azure_domain_hint` configuration option for IdPs. When set, EngineBlock appends a `whr=` query parameter to the HTTP-Redirect AuthnRequest sent to the IdP, allowing Microsoft Azure / EntraID to skip the account picker (#1864). +### Translation key changes + +The following translation keys have been renamed. If you have overridden any of these in your theme translations (`theme/{name}/translations/messages.*.php`), update the key names accordingly. + +| Old key | New key | +|---------------------------------------------|----------------------------------------| +| `error_no_message` | `error_unable_to_receive_message` | +| `error_no_message_desc` | `error_unable_to_receive_message_desc` | +| `error_stepup_callout_unknown_title` | `error_stepup_callout_unknown` | +| `error_stepup_callout_user_cancelled_title` | `error_stepup_callout_user_cancelled` | + +#### `error_invalid_acs_location` + +The `error_invalid_acs_location` translation key has changed meaning. Previously it held the **error description** text. It now holds the **page title**. + +If you have overridden this key in your theme translations (`theme/{name}/translations/messages.*.php`), rename it to `error_invalid_acs_location_desc` and add a new `error_invalid_acs_location` entry for the page title. + +**Before:** +```php +'error_invalid_acs_location' => 'Your custom description text.', +``` + +**After:** +```php +'error_invalid_acs_location' => 'Error - Invalid ACS location', +'error_invalid_acs_location_desc' => 'Your custom description text.', +``` + +See https://github.com/OpenConext/OpenConext-engineblock/issues/1758 + +### HTTP status code changes + +The following feedback pages previously returned HTTP **200 OK** and now return HTTP **400 Bad Request**. + +| URL | Before | After | +|-------------------------------------------------------|--------|-------| +| `/authentication/feedback/invalid-acs-binding` | 200 | 400 | +| `/authentication/feedback/received-error-status-code` | 200 | 400 | + + ## UNRELEASED 7.2.0 Upgrade to Symfony 7.4 Upgrade to `doctrine/dbal` 4 diff --git a/languages/messages.en.php b/languages/messages.en.php index 4f4020887..0eaf6113c 100644 --- a/languages/messages.en.php +++ b/languages/messages.en.php @@ -176,11 +176,10 @@ 'error_authorization_policy_violation_desc_no_name' => 'You cannot use this service because your %organisationNoun% limits access to this service (the "Service Provider") with an authorization policy. Please contact the helpdesk of your %organisationNoun% if you think you should be allowed access to this service.', 'error_authorization_policy_violation_info' => 'Message from %idpName%: ', 'error_authorization_policy_violation_info_no_idp_name' => 'Message from your %organisationNoun%: ', - 'error_no_message' => 'Error - No message received', - 'error_no_message_desc' => 'We were expecting a SAML message, but did not get one. Something went wrong. Please try again.', 'error_unable_to_receive_message' => 'Error - No message received', 'error_unable_to_receive_message_desc' => 'We were expecting a SAML message, but did not get one. Something went wrong. Please try again.', - 'error_invalid_acs_location' => 'The given "Assertion Consumer Service" is unknown or invalid.', + 'error_invalid_acs_location' => 'Error - Invalid ACS location', + 'error_invalid_acs_location_desc' => 'The given "Assertion Consumer Service" is unknown or invalid.', 'error_invalid_acs_binding' => 'Error - Invalid ACS binding type', 'error_invalid_acs_binding_desc' => 'The provided or configured "Assertion Consumer Service" Binding Type is unknown or invalid.', 'error_unsupported_signature_method' => 'Error - Signature method is not supported', diff --git a/languages/messages.nl.php b/languages/messages.nl.php index 91a215948..cb6d5d33c 100644 --- a/languages/messages.nl.php +++ b/languages/messages.nl.php @@ -176,11 +176,10 @@ 'error_authorization_policy_violation_desc_no_name' => 'Neem contact op met de helpdesk van je eigen %organisationNoun% als je toegang tot deze dienst wilt. Vermeld daarbij op welke dienst je probeerde in te loggen en dat je werd tegengehouden door een autorisatieregel van %suiteName%, geconfigureerd door jouw eigen %organisationNoun%.', 'error_authorization_policy_violation_info' => 'Bericht van %idpName%: ', 'error_authorization_policy_violation_info_no_idp_name' => 'Bericht van je %organisationNoun%: ', - 'error_no_message' => 'Fout - Geen bericht ontvangen', - 'error_no_message_desc' => 'We verwachtten een SAML bericht, maar we hebben er geen ontvangen. Er is iets fout gegaan. Probeer het alstublieft opnieuw.', 'error_unable_to_receive_message' => 'Fout - Geen bericht ontvangen', 'error_unable_to_receive_message_desc' => 'We verwachtten een SAML bericht, maar we hebben er geen ontvangen. Er is iets fout gegaan. Probeer het alstublieft opnieuw.', - 'error_invalid_acs_location' => 'De opgegeven "Assertion Consumer Service" is onjuist of bestaat niet.', + 'error_invalid_acs_location' => 'Fout - Ongeldige ACS locatie', + 'error_invalid_acs_location_desc' => 'De opgegeven "Assertion Consumer Service" is onjuist of bestaat niet.', 'error_invalid_acs_binding' => 'Fout - Onjuist ACS binding type', 'error_invalid_acs_binding_desc' => 'Het opgegeven of geconfigureerde "Assertion Consumer Service" Binding Type is onjuist of bestaat niet.', 'error_unsupported_signature_method' => 'Fout - Ondertekeningsmethode wordt niet ondersteund', diff --git a/languages/messages.pt.php b/languages/messages.pt.php index 525e5ff8b..94527836c 100644 --- a/languages/messages.pt.php +++ b/languages/messages.pt.php @@ -174,11 +174,10 @@ 'error_authorization_policy_violation_desc_no_name' => 'Você autenticu-se com sucesso na sua %organisationNoun%, mas infelizmente você não pode utilizar este serviço (o "Fornecedor de Serviço") porque não tem acesso. A sua %organisationNoun% limita o acesso a este serviço com uma política de autorização. Entre em contacto com o suporte da sua %organisationNoun% se acha que deve ser-lhe concedido acesso ao serviço.', 'error_authorization_policy_violation_info' => 'Mensagem da %idpName%: ', 'error_authorization_policy_violation_info_no_idp_name' => 'Mensagem da sua %organisationNoun%: ', - 'error_no_message' => 'Erro - Não foi recebido nenhuma mensagem', - 'error_no_message_desc' => 'Estávamos a aguardar uma mensagem, mas não chegou nenhuma? Alguma coisa correu mal. Tente de novo por favor.', 'error_unable_to_receive_message' => 'Erro - Não foi recebido nenhuma mensagem', 'error_unable_to_receive_message_desc' => 'Estávamos a aguardar uma mensagem, mas não chegou nenhuma? Alguma coisa correu mal. Tente de novo por favor.', - 'error_invalid_acs_location' => 'O "Serviço de Consumidor de Asserção" fornecido é desconhecido ou inválido.', + 'error_invalid_acs_location' => 'Erro - Localização ACS inválida', + 'error_invalid_acs_location_desc' => 'O "Serviço de Consumidor de Asserção" fornecido é desconhecido ou inválido.', 'error_invalid_acs_binding' => 'O ACS "Binding Type" é inválido', 'error_invalid_acs_binding_desc' => 'O "Binding Type" do "Serviço de Consumidor de Asserção" fornecido ou configurado é desconhecido ou inválido.', 'error_unsupported_signature_method' => 'O método de assinatura não é suportado', diff --git a/src/OpenConext/EngineBlockBundle/Controller/FeedbackController.php b/src/OpenConext/EngineBlockBundle/Controller/FeedbackController.php index e86c224a6..f2d34ae3c 100644 --- a/src/OpenConext/EngineBlockBundle/Controller/FeedbackController.php +++ b/src/OpenConext/EngineBlockBundle/Controller/FeedbackController.php @@ -139,6 +139,15 @@ public function __construct( ], methods: ['GET'] )] + #[Route( + path: '/authentication/feedback/invalidAcsLocation', + name: 'authentication_feedback_invalid_acs_location', + defaults: [ + 'pageIdentifier' => 'invalid-acs-location', + 'statusCode' => 400 + ], + methods: ['GET'] + )] public function feedbackAction(string $pageIdentifier, int $statusCode): Response { return new Response( @@ -166,14 +175,6 @@ public function noIdpsAction() return new Response($this->twig->render('@theme/Authentication/View/Feedback/no-idps.html.twig')); } - #[Route(path: '/authentication/feedback/invalidAcsLocation', name: 'authentication_feedback_invalid_acs_location', methods: ['GET'])] - public function invalidAcsLocationAction() - { - return new Response( - $this->twig->render('@theme/Authentication/View/Feedback/invalid-acs-location.html.twig'), - 400 - ); - } #[Route( path: '/authentication/feedback/unsupportedSignatureMethod', diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature index 1a459255e..782527fa4 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature @@ -65,6 +65,10 @@ Feature: When I go to Engineblock URL "/authentication/feedback/stepup-callout-user-cancelled" Then I should see "Logging in cancelled" + Scenario: The invalid-acs-location feedback route renders correctly + When I go to Engineblock URL "/authentication/feedback/invalidAcsLocation" + Then I should see "Invalid ACS location" + 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: diff --git a/theme/base/templates/modules/Authentication/View/Feedback/generic-error.html.twig b/theme/base/templates/modules/Authentication/View/Feedback/generic-error.html.twig index 024febeaa..fd24db5d9 100644 --- a/theme/base/templates/modules/Authentication/View/Feedback/generic-error.html.twig +++ b/theme/base/templates/modules/Authentication/View/Feedback/generic-error.html.twig @@ -4,7 +4,8 @@ # Possible translation keys: error_session_lost, error_session_not_started, error_unsolicited_response, # error_invalid_acs_binding, error_received_error_status_code, error_unable_to_receive_message, # error_unknown_requesterid_in_authnrequest, error_authentication_limit_exceeded, - # error_stepup_callout_unknown, error_stepup_callout_user_cancelled + # error_stepup_callout_unknown, error_stepup_callout_user_cancelled, + # error_invalid_acs_location #} {% set _key = 'error_' ~ pageIdentifier|replace({'-': '_'}) %} {% set pageTitle = (_key)|trans %} diff --git a/theme/base/templates/modules/Authentication/View/Feedback/invalid-acs-location.html.twig b/theme/base/templates/modules/Authentication/View/Feedback/invalid-acs-location.html.twig deleted file mode 100644 index 7e2258026..000000000 --- a/theme/base/templates/modules/Authentication/View/Feedback/invalid-acs-location.html.twig +++ /dev/null @@ -1,8 +0,0 @@ -{% extends '@theme/Default/View/Error/error.html.twig' %} - -{% set pageTitle = 'error_no_message'|trans %} -{% block pageTitle %}{{ pageTitle }}{% endblock %} -{% block title %}{{ parent() }}{% endblock %} -{% block pageHeading %}{{ pageTitle }}{% endblock %} - -{% block errorMessage %}{{ 'error_invalid_acs_location'|trans }}{% endblock %} From 5c9e561f7ec6eb883feac3edddc6cd446b3c856d Mon Sep 17 00:00:00 2001 From: Johan Kromhout Date: Tue, 5 May 2026 11:27:58 +0200 Subject: [PATCH 5/5] Add quick smoketest to assert the routes don't produce an error and the status code is correct --- ci/qa/phpunit.sh | 2 +- config/services/ci/controllers.yml | 4 - languages/messages.en.php | 2 +- .../Bridge/DiContainerRuntime.php | 9 -- .../Controllers/FeedbackController.php | 16 +-- .../Features/FeedbackFooters.feature | 4 +- .../Controller/FeedbackControllerTest.php | 114 ++++++++++++++++++ .../Test/Corto/Module/BindingsTest.php | 6 - 8 files changed, 124 insertions(+), 33 deletions(-) create mode 100644 tests/functional/OpenConext/EngineBlockBundle/Controller/FeedbackControllerTest.php diff --git a/ci/qa/phpunit.sh b/ci/qa/phpunit.sh index bd12241a7..9ec0e1288 100755 --- a/ci/qa/phpunit.sh +++ b/ci/qa/phpunit.sh @@ -18,7 +18,7 @@ XDEBUG_MODE=coverage ./vendor/bin/phpunit --configuration=./tests/phpunit.xml -- echo -e "\nPHPUnit unit tests\n" XDEBUG_MODE=coverage ./vendor/bin/phpunit --configuration=./tests/phpunit.xml --testsuite=unit --coverage-clover coverage.xml -echo -e "\nPHPUnit API acceptance tests\n" +echo -e "\nPHPUnit acceptance tests\n" ./bin/console cache:clear --env=test --no-warmup APP_ENV=test XDEBUG_MODE=coverage ./vendor/bin/phpunit --configuration=./tests/phpunit.xml --testsuite=functional --coverage-clover coverage.xml diff --git a/config/services/ci/controllers.yml b/config/services/ci/controllers.yml index 4a0b5aa3d..ce71b961f 100644 --- a/config/services/ci/controllers.yml +++ b/config/services/ci/controllers.yml @@ -25,10 +25,6 @@ services: class: OpenConext\EngineBlockFunctionalTestingBundle\Controllers\FeedbackController arguments: - '@twig' -<<<<<<< HEAD -======= - - '@engineblock.compat.logger' ->>>>>>> 41926c2bd (Fix feedbackInfo session bleed-through between auth flows (#1795)) - '@OpenConext\EngineBlock\Service\FeedbackStateHelper' - '@OpenConext\EngineBlockBundle\Controller\FeedbackController' diff --git a/languages/messages.en.php b/languages/messages.en.php index 0eaf6113c..e329b4bbe 100644 --- a/languages/messages.en.php +++ b/languages/messages.en.php @@ -265,7 +265,7 @@ 'error_clock_issue_desc' => 'This is likely because the difference in time between %idpName% and %suiteName% it too large. Please verify that the time on the %organisationNoun% is correct.', 'error_clock_issue_desc_no_idp_name' => 'This is likely because the difference in time between %organisationNoun% and %suiteName% it too large. Please verify that the time on the IdP is correct.', 'error_stepup_callout_unknown' => 'Error - Unknown strong authentication failure', - 'error_stepup_callout_unknown_desc' => 'Logging in with strong authentication has failed and we don\'t know exactly why . Please try again first by going back to the service and logging in again . If this doesn\'t work, please contact the service desk of your %organisationNoun%.', + 'error_stepup_callout_unknown_desc' => 'Logging in with strong authentication has failed and we don\'t know exactly why. Please try again first by going back to the service and logging in again. If this doesn\'t work, please contact the service desk of your %organisationNoun%.', 'error_stepup_callout_unmet_loa_title' => 'Error - No suitable token found', 'error_stepup_callout_unmet_loa_desc' => 'To continue to this service, a registered token with a certain level of assurance is required. Currently, you either haven\'t registered a token at all, or the level of assurance of the token you did register is too low. See the link below for more information about the registration process.', 'error_stepup_callout_unmet_loa_link_text' => 'Read more about the registration process.', diff --git a/src/OpenConext/EngineBlockBundle/Bridge/DiContainerRuntime.php b/src/OpenConext/EngineBlockBundle/Bridge/DiContainerRuntime.php index e4c4f12f1..47a0f71a4 100644 --- a/src/OpenConext/EngineBlockBundle/Bridge/DiContainerRuntime.php +++ b/src/OpenConext/EngineBlockBundle/Bridge/DiContainerRuntime.php @@ -20,10 +20,7 @@ use OpenConext\EngineBlock\Service\FeedbackInfoCollectorInterface; use OpenConext\EngineBlock\Service\FeedbackStateHelperInterface; -<<<<<<< HEAD use OpenConext\EngineBlockBundle\Service\WayfRenderer; -======= ->>>>>>> 41926c2bd (Fix feedbackInfo session bleed-through between auth flows (#1795)) use Twig\Environment; /** @@ -37,7 +34,6 @@ public function __construct( public Environment $twig, -<<<<<<< HEAD public WayfRenderer $wayfRenderer, public FeedbackStateHelperInterface $feedbackStateHelper, public FeedbackInfoCollectorInterface $feedbackInfoCollector, @@ -48,10 +44,5 @@ public function __construct( public function getPreferredIdpEntityIds(): array { return $this->preferredIdpEntityIds; -======= - public FeedbackStateHelperInterface $feedbackStateHelper, - public FeedbackInfoCollectorInterface $feedbackInfoCollector, - ) { ->>>>>>> 41926c2bd (Fix feedbackInfo session bleed-through between auth flows (#1795)) } } diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/FeedbackController.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/FeedbackController.php index 63d8d0e04..3b587cbdb 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/FeedbackController.php +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/FeedbackController.php @@ -58,10 +58,9 @@ public function __construct( public function feedbackAction(Request $request) { $key = $this->getTemplate($request); - $feedbackInfo = $this->getFeedbackInfo($request); $parameters = $this->getTemplateParameters($request); - $this->feedbackStateHelper->storeFeedbackInfo($feedbackInfo); + $this->feedbackStateHelper->storeFeedbackInfo($this->getFeedbackInfo($request)); $template = sprintf( '@theme/Authentication/View/Feedback/%s.html.twig', @@ -91,19 +90,14 @@ private function getTemplate(Request $request) /** * @param Request $request - * @return mixed|string + * @return array */ - private function getFeedbackInfo(Request $request) + private function getFeedbackInfo(Request $request): array { - $default = '{ - "requestId":"5cb4bd3879b49", - "ipAddress":"192.168.66.98", - "artCode":"31914" - }'; + $default = '{"requestId":"5cb4bd3879b49","ipAddress":"192.168.66.98","artCode":"31914"}'; - $feedbackInfo = $request->query->getString('feedback-info', $default); + $feedbackInfo = json_decode($request->query->getString('feedback-info', $default), true); - $feedbackInfo = json_decode($feedbackInfo, true); if (!empty($feedbackInfo['IdentityProvider']) || !empty($feedbackInfo['IdP'])) { $feedbackInfo['identityProviderName'] = 'OpenConext Identities Inc'; } diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature index 782527fa4..aa06df815 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FeedbackFooters.feature @@ -45,9 +45,11 @@ Feature: - Scenario: The functional-testing feedback page renders correctly with feedback-info + Scenario: The functional-testing feedback page renders feedback details 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" + And I should see "test-abc" + And I should see "1.2.3.4" Scenario: The session-lost feedback route renders correctly When I go to Engineblock URL "/authentication/feedback/session-lost" diff --git a/tests/functional/OpenConext/EngineBlockBundle/Controller/FeedbackControllerTest.php b/tests/functional/OpenConext/EngineBlockBundle/Controller/FeedbackControllerTest.php new file mode 100644 index 000000000..79aa32756 --- /dev/null +++ b/tests/functional/OpenConext/EngineBlockBundle/Controller/FeedbackControllerTest.php @@ -0,0 +1,114 @@ +assertFeedbackPage('/authentication/feedback/session-lost', Response::HTTP_BAD_REQUEST, 'your session was lost'); + } + + #[Test] + public function session_not_started_returns_400_with_expected_content(): void + { + $this->assertFeedbackPage('/authentication/feedback/session-not-started', Response::HTTP_BAD_REQUEST, 'No session found'); + } + + #[Test] + public function unsolicited_response_returns_400_with_expected_content(): void + { + $this->assertFeedbackPage('/authentication/feedback/unsolicited-response', Response::HTTP_BAD_REQUEST, 'Sign-in could not be completed'); + } + + #[Test] + public function invalid_acs_binding_returns_400_with_expected_content(): void + { + $this->assertFeedbackPage('/authentication/feedback/invalid-acs-binding', Response::HTTP_BAD_REQUEST, 'Invalid ACS binding type'); + } + + #[Test] + public function received_error_status_code_returns_400_with_expected_content(): void + { + $this->assertFeedbackPage('/authentication/feedback/received-error-status-code', Response::HTTP_BAD_REQUEST, 'Identity Provider error'); + } + + #[Test] + public function unable_to_receive_message_returns_400_with_expected_content(): void + { + $this->assertFeedbackPage('/authentication/feedback/unable-to-receive-message', Response::HTTP_BAD_REQUEST, 'No message received'); + } + + #[Test] + public function unknown_requesterid_in_authnrequest_returns_400_with_expected_content(): void + { + $this->assertFeedbackPage('/authentication/feedback/unknown_requesterid_in_authnrequest', Response::HTTP_BAD_REQUEST, 'Unknown service'); + } + + #[Test] + public function authentication_limit_exceeded_returns_429_with_expected_content(): void + { + $this->assertFeedbackPage('/authentication/feedback/authentication-limit-exceeded', Response::HTTP_TOO_MANY_REQUESTS, 'too many authentications in progress'); + } + + #[Test] + public function stepup_callout_unknown_returns_400_with_expected_content(): void + { + $this->assertFeedbackPage('/authentication/feedback/stepup-callout-unknown', Response::HTTP_BAD_REQUEST, 'Unknown strong authentication failure'); + } + + #[Test] + public function stepup_callout_user_cancelled_returns_400_with_expected_content(): void + { + $this->assertFeedbackPage('/authentication/feedback/stepup-callout-user-cancelled', Response::HTTP_BAD_REQUEST, 'Logging in cancelled'); + } + + #[Test] + public function invalid_acs_location_returns_400_with_expected_content(): void + { + $this->assertFeedbackPage('/authentication/feedback/invalidAcsLocation', Response::HTTP_BAD_REQUEST, 'Invalid ACS location'); + } + + #[Test] + public function feedback_data_from_session_is_rendered_on_the_real_route(): void + { + $client = self::createClient(); + + // First prime, the session, then visit the actual route + $client->request('GET', 'https://engine.dev.openconext.local/functional-testing/feedback?template=session-lost'); + $client->request('GET', 'https://engine.dev.openconext.local/authentication/feedback/session-lost'); + + $content = $client->getResponse()->getContent(); + $this->assertStringContainsString('feedback-info--requestid', $content); + $this->assertStringContainsString('feedback-info--ipaddress', $content); + } + + private function assertFeedbackPage(string $path, int $expectedStatus, string $expectedPhrase): void + { + $client = self::createClient(); + $client->request('GET', 'https://engine.dev.openconext.local' . $path); + + $this->assertEquals($expectedStatus, $client->getResponse()->getStatusCode()); + $this->assertStringContainsString($expectedPhrase, $client->getResponse()->getContent()); + } +} diff --git a/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php b/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php index 9edc0279a..b71041e8c 100644 --- a/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php +++ b/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php @@ -63,16 +63,10 @@ public function setUp(): void $engineBlock = \EngineBlock_ApplicationSingleton::getInstance(); $engineBlock->setDiContainerRuntime(new DiContainerRuntime( -<<<<<<< HEAD $this->createStub(Twig\Environment::class), $this->createStub(WayfRenderer::class), $this->createStub(FeedbackStateHelperInterface::class), $this->createStub(FeedbackInfoCollectorInterface::class), -======= - Phake::mock(Twig\Environment::class), - m::mock(FeedbackStateHelperInterface::class), - m::mock(FeedbackInfoCollectorInterface::class), ->>>>>>> 41926c2bd (Fix feedbackInfo session bleed-through between auth flows (#1795)) )); $this->bindings = new EngineBlock_Corto_Module_Bindings($this->proxyServer);