Support a caller-supplied redirect on /reset-remember-wayf

CHANGELOG.md

@@ -26,6 +26,10 @@ Features:
 * 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`.
+* The `/reset-remember-wayf` endpoint accepts a `redirect` query parameter. Only `https` URLs with a host listed in the
+  new parameter `wayf.reset_choice_allowed_redirect_hosts` are accepted, other values fall back to
+  `wayf.reset_choice_per_idp_redirect`. The endpoint also appends `wayfReset=removed` or `wayfReset=none` to the
+  redirect URL.
 
 ## 7.2.1
 

config/packages/parameters.yml.dist

@@ -200,6 +200,13 @@ parameters:
     ## 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/'
+    ## Hostnames that the /reset-remember-wayf endpoint is allowed to redirect back to via
+    ## its `redirect` query parameter (e.g. so Profile can send the user back to the page
+    ## they came from). When the query parameter is missing, malformed, or points at a host
+    ## that is not in this list, the endpoint falls back to
+    ## `wayf.reset_choice_per_idp_redirect` instead.
+    wayf.reset_choice_allowed_redirect_hosts:
+        - profile.dev.openconext.local
 
     ## Toggle the default IdP quick link banner on the WAYF.
     wayf.display_default_idp_banner_on_wayf: true

config/services/controllers/authentication.yml

@@ -79,6 +79,7 @@ services:
             $rememberedIdpCookie: '@OpenConext\EngineBlock\Service\Wayf\RememberedIdpCookie'
             $logger: '@engineblock.compat.logger'
             $redirectUrl: '%wayf.reset_choice_per_idp_redirect%'
+            $allowedRedirectHosts: '%wayf.reset_choice_allowed_redirect_hosts%'
 
     OpenConext\EngineBlock\Service\RequestAccessMailer:
         arguments:

docs/wayf_remember_choice.md

@@ -105,6 +105,22 @@ The endpoint removes the `rememberedidps` cookie and redirects the user (HTTP 30
 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.
 
+The caller can choose where the user lands with the `redirect` query parameter, for example
+`/reset-remember-wayf?redirect=https://profile.dev.openconext.local/my-choices`. To prevent an open redirect, the
+`redirect` value is only used when it is an absolute `https` URL whose host is listed in:
+
+    # Hosts that may be used in the redirect query parameter
+    wayf.reset_choice_allowed_redirect_hosts:
+        - profile.dev.openconext.local
+
+A missing, malformed or unlisted `redirect` falls back to `wayf.reset_choice_per_idp_redirect`. Plain `http` URLs, URLs
+with credentials (`user@host`), and URLs containing backslashes, whitespace or control characters are rejected as well,
+because browsers interpret those differently from the host check. The value in `parameters.yml.dist` is the local
+development host. Replace it with the hosts of your own deployment.
+
+The endpoint appends the query parameter `wayfReset` to the final URL, before any fragment. Its value is `removed` when
+a cookie was removed and `none` when there was nothing to remove, so the target page can show a matching message.
+
 ## Switching modes
 
 Cookies written in one mode are not read in the other. After switching, users have to make their choice once more.

src/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfController.php

@@ -34,6 +34,7 @@ public function __construct(
         private readonly RememberedIdpCookie $rememberedIdpCookie,
         private readonly LoggerInterface $logger,
         private readonly string $redirectUrl,
+        private readonly array $allowedRedirectHosts = [],
     ) {
         if ($this->redirectUrl === '') {
             throw new InvalidArgumentException(
@@ -56,6 +57,54 @@ public function __invoke(Request $request): RedirectResponse
             ));
         }
 
-        return new RedirectResponse($this->redirectUrl, Response::HTTP_FOUND);
+        $target = $this->resolveRedirectTarget($request->query->get('redirect'));
+        $location = $this->appendQueryParameter($target, 'wayfReset', $raw !== null ? 'removed' : 'none');
+
+        return new RedirectResponse($location, Response::HTTP_FOUND);
+    }
+
+    private function resolveRedirectTarget(?string $redirect): string
+    {
+        if ($redirect === null || $redirect === '' || !$this->isAllowedRedirect($redirect)) {
+            return $this->redirectUrl;
+        }
+
+        return $redirect;
+    }
+
+    private function isAllowedRedirect(string $redirect): bool
+    {
+        // Browsers normalise backslashes and drop control characters and whitespace, parse_url() does not.
+        // Such a value could pass the host check here and still send the browser to another host.
+        if (preg_match('/[\x00-\x20\x7f\\\\]/', $redirect) === 1) {
+            return false;
+        }
+
+        $parts = parse_url($redirect);
+
+        if ($parts === false || ($parts['scheme'] ?? null) !== 'https' || !isset($parts['host'])) {
+            return false;
+        }
+
+        if (isset($parts['user']) || isset($parts['pass'])) {
+            return false;
+        }
+
+        return in_array($parts['host'], $this->allowedRedirectHosts, true);
+    }
+
+    private function appendQueryParameter(string $url, string $key, string $value): string
+    {
+        $fragment = '';
+        $hashPosition = strpos($url, '#');
+
+        if ($hashPosition !== false) {
+            $fragment = substr($url, $hashPosition);
+            $url = substr($url, 0, $hashPosition);
+        }
+
+        $separator = str_contains($url, '?') ? '&' : '?';
+
+        return $url . $separator . $key . '=' . rawurlencode($value) . $fragment;
     }
 }

tests/functional/OpenConext/EngineBlockBundle/Controller/ResetRememberedWayfControllerTest.php

@@ -46,7 +46,7 @@ public function visiting_the_endpoint_with_a_remembered_idp_cookie_clears_it_and
         $response = $client->getResponse();
         $this->assertSame(Response::HTTP_FOUND, $response->getStatusCode());
         $this->assertSame(
-            self::getContainer()->getParameter('wayf.reset_choice_per_idp_redirect'),
+            self::getContainer()->getParameter('wayf.reset_choice_per_idp_redirect') . '?wayfReset=removed',
             $response->headers->get('Location')
         );
     }
@@ -61,7 +61,45 @@ public function visiting_the_endpoint_without_a_remembered_idp_cookie_still_redi
         $response = $client->getResponse();
         $this->assertSame(Response::HTTP_FOUND, $response->getStatusCode());
         $this->assertSame(
-            self::getContainer()->getParameter('wayf.reset_choice_per_idp_redirect'),
+            self::getContainer()->getParameter('wayf.reset_choice_per_idp_redirect') . '?wayfReset=none',
+            $response->headers->get('Location')
+        );
+    }
+
+    #[Test]
+    public function a_redirect_parameter_pointing_at_an_allowed_host_is_honoured(): void
+    {
+        $client = self::createClient();
+
+        $client->request(
+            'GET',
+            'https://engine.dev.openconext.local/reset-remember-wayf'
+            . '?redirect=' . urlencode('https://profile.dev.openconext.local/my-profile')
+        );
+
+        $response = $client->getResponse();
+        $this->assertSame(Response::HTTP_FOUND, $response->getStatusCode());
+        $this->assertSame(
+            'https://profile.dev.openconext.local/my-profile?wayfReset=none',
+            $response->headers->get('Location')
+        );
+    }
+
+    #[Test]
+    public function a_redirect_parameter_pointing_at_a_disallowed_host_falls_back_to_the_configured_redirect(): void
+    {
+        $client = self::createClient();
+
+        $client->request(
+            'GET',
+            'https://engine.dev.openconext.local/reset-remember-wayf'
+            . '?redirect=' . urlencode('https://evil.example.org/phishing')
+        );
+
+        $response = $client->getResponse();
+        $this->assertSame(Response::HTTP_FOUND, $response->getStatusCode());
+        $this->assertSame(
+            self::getContainer()->getParameter('wayf.reset_choice_per_idp_redirect') . '?wayfReset=none',
             $response->headers->get('Location')
         );
     }