Support a caller-supplied redirect on /reset-remember-wayf - #2097
Open
kayjoosten wants to merge 1 commit into
Open
kayjoosten wants to merge 1 commit into
kayjoosten wants to merge 1 commit into
Conversation
Let /reset-remember-wayf accept an optional `redirect` query parameter so callers such as Profile can send the user back to the page they came from once their remembered per-SP WAYF choices are cleared, instead of always sending everyone to the single operator-configured URL from #2085. Every redirect (whether the caller-supplied target or the configured fallback) now also carries a `wayfReset=removed|none` query parameter, so the caller can tell whether a cookie actually existed and was cleared. - New config wayf.reset_choice_allowed_redirect_hosts is a host allowlist (defaulting to profile.dev.openconext.local in dev/CI): the `redirect` parameter is only honoured when it parses to an http(s) URL whose host appears in this list, otherwise the endpoint silently falls back to the existing wayf.reset_choice_per_idp_redirect target. This mirrors the "parse and check an allowed value" shape of the existing AllowedSchemeValidator rather than introducing a new general-purpose URL validation abstraction for a single call site. - Resolving the redirect target and appending the wayfReset signal are both extracted into small private methods so the __invoke method reads as a straight sequence: clear/log the cookie if present, resolve where to send the user, tell them what happened. - The entry-count/log-message behaviour from #2085 is unchanged: a present-but-unparseable cookie is still treated the same as a valid one for logging and for the wayfReset signal (both are cleared and reported), since from the caller's perspective the cookie no longer exists either way. - Existing unit/functional tests were updated for the new `?wayfReset=` suffix on every redirect, and new cases cover an allowed redirect host, a redirect with its own query string, a disallowed host, a malformed URL, and a disallowed scheme. Needed by OpenConext/OpenConext-profile#345, which links back to Profile's "my personal data" page after resetting a user's remembered WAYF choices.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Extends
GET /reset-remember-wayf(introduced in #2085 / #2094) so itaccepts an optional
redirectquery parameter, and always appends awayfReset=removed|nonequery parameter to whichever redirect target isused. This is a prerequisite for
OpenConext/OpenConext-profile#345,
which wants to link from Profile's "my personal data" page to this
endpoint and land the user back on that same page afterwards, knowing
whether a cookie was actually cleared.
Stacked on #2094
This branches from
wayf-reset-endpoint(#2094, not yet merged), since itdirectly extends
ResetRememberedWayfControlleradded there. The diffshown here is the incremental change only; once #2094 merges to
main,this PR's base should be retargeted to
main.Details
wayf.reset_choice_allowed_redirect_hostsis a hostallowlist (defaulting to
profile.dev.openconext.localin dev/CI). Theredirectparameter is only honoured when it parses to anhttp(s)URLwhose host appears in this list; otherwise the endpoint silently falls
back to the existing
wayf.reset_choice_per_idp_redirecttarget. Thismirrors the "parse and check an allowed value" shape of the existing
AllowedSchemeValidatorrather than introducing a new general-purposeURL validation abstraction for a single call site.
wayfResetsignal areextracted into small private methods, so
__invokereads as a straightsequence: clear/log the cookie if present, resolve where to send the
user, tell them what happened.
present-but-unparseable cookie is still treated the same as a valid one
for logging and for the
wayfResetsignal (both are cleared andreported), since from the caller's perspective the cookie no longer
exists either way.
?wayfReset=suffix on every redirect, and new cases cover an allowed redirect host,
a redirect with its own query string, a disallowed host, a malformed
URL, and a disallowed scheme.
Testing
All run inside the Docker dev container (PHP 8.5):
phpmd/phpcs/phpcs-legacy/docheader: clean.eb4: 244/244.unit: 1008/1008 (1003 baseline + 5 new).functional(
APP_ENV=test): 123/123 (121 baseline + 2 new).integration:105/105.
defaultsuite: 300/300 scenarios, 5557/5557 steps (unchanged —no new scenarios needed, this is pure PHPUnit-covered logic).
Refs: OpenConext/OpenConext-profile#345