From 6ec0fd3b9e1a7f839814b7e110c2803e47878f7d Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Tue, 15 Sep 2026 11:59:39 +0200 Subject: [PATCH 1/4] Add endpoint to reset the per-SP WAYF remember-choice cookie Introduce GET /reset-remember-wayf, a public endpoint that lets a user forget their remembered per-SP IdP choices set up in #2065. Visiting the page clears the 'rememberedidps' cookie (if present) and redirects to an operator-configured URL. - ResetRememberedWayfController reuses the existing RememberedIdpCookie service rather than hand-rolling cookie-clearing logic: normalize() computes the valid entry count for logging, clear() removes the cookie with the same SameSite/secure flags used when it was written. Both only run when the cookie is actually present, so a first-time visitor with no cookie is redirected without any log noise. - The controller validates its configured redirect URL at construction time and refuses to serve requests when it is blank, so a missing wayf.reset_choice_per_idp_redirect parameter fails fast instead of silently sending users nowhere. - New config wayf.reset_choice_per_idp_redirect defaults to a working dev URL (matching the convention of other wayf.* dev defaults in parameters.yml.dist) so the endpoint works out of the box in dev/CI, while still requiring operators to set a real destination in production. - The functional test asserts the redirect target against the configured parameter rather than merely checking for a Location header: the app's global exception listener also turns uncaught exceptions into a 302, which would otherwise let a broken configuration pass unnoticed. Refs: #2085 --- config/packages/parameters.yml.dist | 5 + .../services/controllers/authentication.yml | 6 + .../ResetRememberedWayfController.php | 61 +++++++ .../ResetRememberedWayfControllerTest.php | 78 +++++++++ .../ResetRememberedWayfControllerTest.php | 154 ++++++++++++++++++ 5 files changed, 304 insertions(+) create mode 100644 src/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfController.php create mode 100644 tests/functional/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php create mode 100644 tests/unit/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php diff --git a/config/packages/parameters.yml.dist b/config/packages/parameters.yml.dist index cd1431b0ff..3e87245d69 100644 --- a/config/packages/parameters.yml.dist +++ b/config/packages/parameters.yml.dist @@ -189,6 +189,11 @@ parameters: ## oldest-expiring entries are evicted first; the newly added choice is always kept and no ## error is raised (see RememberedIdpCookie::pruneOverLimit()). wayf.remember_choice_per_idp_max: 16 + ## URL to redirect the user to after visiting /reset-remember-wayf and having their + ## per-SP remembered IdP choices cookie removed. Operators should override this with a + ## real destination for their deployment; it must never be left empty, as the endpoint + ## refuses to serve requests (failing fast at construction time) when it is blank. + wayf.reset_choice_per_idp_redirect: 'https://engine.dev.openconext.local/' ## Toggle the default IdP quick link banner on the WAYF. wayf.display_default_idp_banner_on_wayf: true diff --git a/config/services/controllers/authentication.yml b/config/services/controllers/authentication.yml index 21b07e7ccc..73ec6fe5e3 100644 --- a/config/services/controllers/authentication.yml +++ b/config/services/controllers/authentication.yml @@ -74,6 +74,12 @@ services: - '@twig' - '@OpenConext\EngineBlock\Service\SsoSessionService' + OpenConext\EngineBlockBundle\Controller\ResetRememberedWayfController: + arguments: + $rememberedIdpCookie: '@OpenConext\EngineBlock\Service\Wayf\RememberedIdpCookie' + $logger: '@engineblock.compat.logger' + $redirectUrl: '%wayf.reset_choice_per_idp_redirect%' + OpenConext\EngineBlock\Service\RequestAccessMailer: arguments: - '@symfony.mailer' diff --git a/src/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfController.php b/src/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfController.php new file mode 100644 index 0000000000..8dae9e8303 --- /dev/null +++ b/src/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfController.php @@ -0,0 +1,61 @@ +redirectUrl === '') { + throw new InvalidArgumentException( + 'The "wayf.reset_choice_per_idp_redirect" parameter must be configured with a redirect URL' + ); + } + } + + #[Route(path: '/reset-remember-wayf', name: 'reset_remember_wayf', methods: ['GET'])] + public function __invoke(Request $request): RedirectResponse + { + $raw = $request->cookies->get(RememberedIdpCookie::NAME); + + if ($raw !== null) { + $entryCount = count($this->rememberedIdpCookie->normalize($raw)['entries']); + $this->rememberedIdpCookie->clear(); + $this->logger->info(sprintf( + 'WAYF-remember-my-choice cookie removed (had %d entries)', + $entryCount + )); + } + + return new RedirectResponse($this->redirectUrl, Response::HTTP_FOUND); + } +} diff --git a/tests/functional/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php b/tests/functional/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php new file mode 100644 index 0000000000..2f67773ac9 --- /dev/null +++ b/tests/functional/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php @@ -0,0 +1,78 @@ +getCookieJar()->set(new Cookie( + RememberedIdpCookie::NAME, + self::encodeEntries([ + 'https://sp.example.org' => ['idp' => 'https://idp.example.org', 'expires' => time() + 3600], + ]), + null, + '/', + 'engine.dev.openconext.local' + )); + + $client->request('GET', 'https://engine.dev.openconext.local/reset-remember-wayf'); + + $response = $client->getResponse(); + $this->assertSame(Response::HTTP_FOUND, $response->getStatusCode()); + // Asserting the exact configured redirect target (rather than merely "some Location + // header") is deliberate: it's the only way this test would fail if the controller's + // constructor validation ever throws (e.g. due to a blank redirect URL), since the + // app's global exception listener also turns uncaught exceptions into a 302 elsewhere. + $this->assertSame( + self::getContainer()->getParameter('wayf.reset_choice_per_idp_redirect'), + $response->headers->get('Location') + ); + } + + #[Test] + public function visiting_the_endpoint_without_a_remembered_idp_cookie_still_redirects(): void + { + $client = self::createClient(); + + $client->request('GET', 'https://engine.dev.openconext.local/reset-remember-wayf'); + + $response = $client->getResponse(); + $this->assertSame(Response::HTTP_FOUND, $response->getStatusCode()); + $this->assertSame( + self::getContainer()->getParameter('wayf.reset_choice_per_idp_redirect'), + $response->headers->get('Location') + ); + } + + /** @param array $entries */ + private static function encodeEntries(array $entries): string + { + return base64_encode((string) gzdeflate((string) json_encode($entries))); + } +} diff --git a/tests/unit/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php b/tests/unit/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php new file mode 100644 index 0000000000..99690f8f5f --- /dev/null +++ b/tests/unit/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php @@ -0,0 +1,154 @@ +clearCookieWithSameSite(Phake::anyParameters())->thenReturn(true); + + $rememberedIdpCookie = $this->buildRememberedIdpCookie($cookieService); + $raw = $rememberedIdpCookie->encode([ + 'https://sp1.example.org' => ['idp' => 'https://idp1.example.org', 'expires' => time() + 3600], + 'https://sp2.example.org' => ['idp' => 'https://idp2.example.org', 'expires' => time() + 3600], + ]); + + $logger = Mockery::mock(LoggerInterface::class); + $logger->shouldReceive('info') + ->once() + ->with('WAYF-remember-my-choice cookie removed (had 2 entries)'); + + $controller = new ResetRememberedWayfController($rememberedIdpCookie, $logger, self::REDIRECT_URL); + + $response = $controller($this->buildRequest($raw)); + + $this->assertSame(Response::HTTP_FOUND, $response->getStatusCode()); + $this->assertSame(self::REDIRECT_URL, $response->getTargetUrl()); + Phake::verify($cookieService)->clearCookieWithSameSite( + RememberedIdpCookie::NAME, + self::COOKIE_PATH, + self::COOKIE_DOMAIN, + true, + true, + 'None' + ); + } + + #[Test] + public function invalid_cookie_is_still_cleared_and_logged_with_zero_entries(): void + { + $cookieService = Phake::mock(CookieService::class); + Phake::when($cookieService)->clearCookieWithSameSite(Phake::anyParameters())->thenReturn(true); + + $rememberedIdpCookie = $this->buildRememberedIdpCookie($cookieService); + + $logger = Mockery::mock(LoggerInterface::class); + $logger->shouldReceive('info') + ->once() + ->with('WAYF-remember-my-choice cookie removed (had 0 entries)'); + + $controller = new ResetRememberedWayfController($rememberedIdpCookie, $logger, self::REDIRECT_URL); + + $response = $controller($this->buildRequest('not valid base64 or deflated data!')); + + $this->assertSame(Response::HTTP_FOUND, $response->getStatusCode()); + $this->assertSame(self::REDIRECT_URL, $response->getTargetUrl()); + Phake::verify($cookieService)->clearCookieWithSameSite(Phake::anyParameters()); + } + + #[Test] + public function missing_cookie_is_not_cleared_or_logged_but_still_redirects(): void + { + $cookieService = Phake::mock(CookieService::class); + $rememberedIdpCookie = $this->buildRememberedIdpCookie($cookieService); + + $logger = Mockery::mock(LoggerInterface::class); + $logger->shouldNotReceive('info'); + + $controller = new ResetRememberedWayfController($rememberedIdpCookie, $logger, self::REDIRECT_URL); + + $response = $controller($this->buildRequest(null)); + + $this->assertSame(Response::HTTP_FOUND, $response->getStatusCode()); + $this->assertSame(self::REDIRECT_URL, $response->getTargetUrl()); + Phake::verifyNoInteraction($cookieService); + } + + #[Test] + public function constructor_rejects_an_empty_redirect_url(): void + { + $this->expectException(InvalidArgumentException::class); + + new ResetRememberedWayfController( + $this->buildRememberedIdpCookie(Phake::mock(CookieService::class)), + Mockery::mock(LoggerInterface::class), + '' + ); + } + + private function buildRememberedIdpCookie(CookieService $cookieService): RememberedIdpCookie + { + return new RememberedIdpCookie( + new TimeProvider(), + $cookieService, + self::LIFETIME, + self::MAX_ENTRIES, + self::COOKIE_DOMAIN, + self::COOKIE_PATH, + true, + ); + } + + private function buildRequest(?string $rememberedIdpsCookie): Request + { + $request = Request::create('/reset-remember-wayf'); + if ($rememberedIdpsCookie !== null) { + $request->cookies->set(RememberedIdpCookie::NAME, $rememberedIdpsCookie); + } + + return $request; + } +} From 1f300bc6a695c17eab8b2941ef1f32dbc331794f Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Tue, 22 Sep 2026 16:09:34 +0200 Subject: [PATCH 2/4] Remove explanatory prose comment restating the assertion below it --- .../Controller/ResetRememberedWayfControllerTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/functional/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php b/tests/functional/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php index 2f67773ac9..fd86703c6a 100644 --- a/tests/functional/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php +++ b/tests/functional/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php @@ -45,10 +45,6 @@ public function visiting_the_endpoint_with_a_remembered_idp_cookie_clears_it_and $response = $client->getResponse(); $this->assertSame(Response::HTTP_FOUND, $response->getStatusCode()); - // Asserting the exact configured redirect target (rather than merely "some Location - // header") is deliberate: it's the only way this test would fail if the controller's - // constructor validation ever throws (e.g. due to a blank redirect URL), since the - // app's global exception listener also turns uncaught exceptions into a 302 elsewhere. $this->assertSame( self::getContainer()->getParameter('wayf.reset_choice_per_idp_redirect'), $response->headers->get('Location') From 6997c26db21a2d7f8c9523307b049b8d4c9c1936 Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Thu, 8 Oct 2026 10:46:35 +0200 Subject: [PATCH 3/4] Document the reset endpoint for the per-SP remembered choice # Why is this change needed? Prior to this change, /reset-remember-wayf and the wayf.reset_choice_per_idp_redirect parameter were only described in the comments of parameters.yml.dist. This is an open source project, so operators and integrators need to find the endpoint in the docs and the release notes. # How does it address the issue? This change adds a section to docs/wayf_remember_choice.md and an entry to the CHANGELOG. --- CHANGELOG.md | 3 +++ docs/wayf_remember_choice.md | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e414bd49b..f277ef4c24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ Features: * The per-SP mode does not require `wayf.remember_choice`. Leave `wayf.remember_choice` set to `false` when enabling it. * The cookie removal page `/authentication/idp/remove-cookies` is now also available in the per-SP mode and lists the `rememberedidps` cookie. Before, it was only available when `wayf.remember_choice` was `true`. +* Added the endpoint `/reset-remember-wayf`, which removes the per-SP `rememberedidps` cookie and redirects to the URL + configured in `wayf.reset_choice_per_idp_redirect`. This new parameter must not be empty. See + `docs/wayf_remember_choice.md`. Updated: diff --git a/docs/wayf_remember_choice.md b/docs/wayf_remember_choice.md index cd1a8a0761..330f378517 100644 --- a/docs/wayf_remember_choice.md +++ b/docs/wayf_remember_choice.md @@ -90,6 +90,21 @@ The page `/authentication/idp/remove-cookies` lets a user inspect and remove the including `rememberchoice` and `rememberedidps`. The page is available when either mode is enabled, and returns a 404 when both are disabled. +## Resetting the remembered choices + +In the per-SP mode, other applications (for example a profile page) can let users forget all their remembered choices +by sending them to: + + https:///reset-remember-wayf + +The endpoint removes the `rememberedidps` cookie and redirects the user (HTTP 302) to the URL configured in: + + # Where to send the user after the reset. Required, must not be empty. + wayf.reset_choice_per_idp_redirect: 'https://engine.dev.openconext.local/' + +The endpoint only accepts `GET` requests and needs no authentication, because it only clears a cookie in the user's own +browser. It does nothing when the cookie is not present. The `rememberchoice` cookie of the global mode is not touched. + ## Switching modes Cookies written in one mode are not read in the other. After switching, users have to make their choice once more. From 1e00df13b792c685e3e7dfa0b7327b4645ef7f9e Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Mon, 12 Oct 2026 15:08:40 +0200 Subject: [PATCH 4/4] Replace Phake with Mockery in ResetRememberedWayfControllerTest # If applied, this commit will... Convert the remaining Phake-based mocking in ResetRememberedWayfControllerTest to Mockery, matching the rest of the test suite. # Why is this change needed? Prior to this change, the test mixed Phake (for CookieService) with Mockery (for LoggerInterface) in the same class. Phake is deprecated in this codebase and only used in a handful of legacy test files, and mixing mocking libraries in one test class adds unnecessary cognitive overhead. # How does it address the issue? This change replaces Phake::mock()/when()/verify() calls with declarative Mockery::mock()->shouldReceive()->with()->andReturn() expectations set up before the action under test runs. The "no interaction expected" case relies on Mockery's default strict behaviour, which throws automatically on any unstubbed method call, so no explicit verification call is needed. # Provide links to any relevant tickets, articles or other resources Addresses review comment on PR #2094. --- .../ResetRememberedWayfControllerTest.php | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/tests/unit/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php b/tests/unit/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php index 99690f8f5f..8b4e3cffed 100644 --- a/tests/unit/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php +++ b/tests/unit/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php @@ -27,7 +27,6 @@ use OpenConext\EngineBlock\Service\TimeProvider\TimeProvider; use OpenConext\EngineBlock\Service\Wayf\RememberedIdpCookie; use OpenConext\EngineBlockBundle\Controller\ResetRememberedWayfController; -use Phake; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; @@ -47,8 +46,18 @@ class ResetRememberedWayfControllerTest extends TestCase #[Test] public function cookie_with_valid_entries_is_cleared_and_logged_with_entry_count(): void { - $cookieService = Phake::mock(CookieService::class); - Phake::when($cookieService)->clearCookieWithSameSite(Phake::anyParameters())->thenReturn(true); + $cookieService = Mockery::mock(CookieService::class); + $cookieService->shouldReceive('clearCookieWithSameSite') + ->once() + ->with( + RememberedIdpCookie::NAME, + self::COOKIE_PATH, + self::COOKIE_DOMAIN, + true, + true, + 'None' + ) + ->andReturn(true); $rememberedIdpCookie = $this->buildRememberedIdpCookie($cookieService); $raw = $rememberedIdpCookie->encode([ @@ -67,21 +76,13 @@ public function cookie_with_valid_entries_is_cleared_and_logged_with_entry_count $this->assertSame(Response::HTTP_FOUND, $response->getStatusCode()); $this->assertSame(self::REDIRECT_URL, $response->getTargetUrl()); - Phake::verify($cookieService)->clearCookieWithSameSite( - RememberedIdpCookie::NAME, - self::COOKIE_PATH, - self::COOKIE_DOMAIN, - true, - true, - 'None' - ); } #[Test] public function invalid_cookie_is_still_cleared_and_logged_with_zero_entries(): void { - $cookieService = Phake::mock(CookieService::class); - Phake::when($cookieService)->clearCookieWithSameSite(Phake::anyParameters())->thenReturn(true); + $cookieService = Mockery::mock(CookieService::class); + $cookieService->shouldReceive('clearCookieWithSameSite')->once()->andReturn(true); $rememberedIdpCookie = $this->buildRememberedIdpCookie($cookieService); @@ -96,13 +97,12 @@ public function invalid_cookie_is_still_cleared_and_logged_with_zero_entries(): $this->assertSame(Response::HTTP_FOUND, $response->getStatusCode()); $this->assertSame(self::REDIRECT_URL, $response->getTargetUrl()); - Phake::verify($cookieService)->clearCookieWithSameSite(Phake::anyParameters()); } #[Test] public function missing_cookie_is_not_cleared_or_logged_but_still_redirects(): void { - $cookieService = Phake::mock(CookieService::class); + $cookieService = Mockery::mock(CookieService::class); $rememberedIdpCookie = $this->buildRememberedIdpCookie($cookieService); $logger = Mockery::mock(LoggerInterface::class); @@ -114,7 +114,6 @@ public function missing_cookie_is_not_cleared_or_logged_but_still_redirects(): v $this->assertSame(Response::HTTP_FOUND, $response->getStatusCode()); $this->assertSame(self::REDIRECT_URL, $response->getTargetUrl()); - Phake::verifyNoInteraction($cookieService); } #[Test] @@ -123,7 +122,7 @@ public function constructor_rejects_an_empty_redirect_url(): void $this->expectException(InvalidArgumentException::class); new ResetRememberedWayfController( - $this->buildRememberedIdpCookie(Phake::mock(CookieService::class)), + $this->buildRememberedIdpCookie(Mockery::mock(CookieService::class)), Mockery::mock(LoggerInterface::class), '' );