Add link to reset remembered WAYF login-method choices - #353
Open
kayjoosten wants to merge 3 commits into
Open
kayjoosten wants to merge 3 commits into
kayjoosten wants to merge 3 commits into
Conversation
Add a "Login method choices" section to the bottom of the "my personal
data" page that lets a user reset every per-application login-method
choice they've made in the WAYF, behind a confirmation popup. Confirming
sends the user to EngineBlock's /reset-remember-wayf endpoint (#2085),
which clears the cookie and redirects back here.
- WayfResetLinkBuilder is a small, pure service that appends a `redirect`
query parameter (the absolute URL of this same page) to the configured
wayf_reset_url. It's factored out of MyProfileController and unit
tested in isolation because this repo has no controller/functional
test infrastructure yet, and the interesting logic (building the
link) doesn't need a kernel to test.
- New wayf_reset_url parameter follows the existing pattern of simple
string config values in parameters.yaml(.dist), pointing at
EngineBlock's reset endpoint.
- The new section reuses the shared modal.html.twig confirmation
component (the same one used for deleting a connection), wrapped in a
single-iteration `for` loop: the helper unconditionally derives its
element ids from `loop.index`, which only exists inside a loop, and
its one existing caller is always used from within a real loop over
services.
- SCSS mirrors the structure/naming of the existing "Download" section
on the SURFconext dashboard (mySurfConext__download*), including its
IE11 fallback rules, since both are a text explanation next to a
single button.
- New translation keys were added under the existing
profile.my_profile key in all three locales (en/nl/pt). Since exact
copy from design isn't available, the wording is a best-effort draft
that should be reviewed against the actual mockups; the confirm
button text ("Reset all") is the one piece of copy specified by the
ticket itself.
This depends on OpenConext/OpenConext-engineblock#2097 adding `redirect`
support to /reset-remember-wayf; without it the endpoint still works but
always redirects to its own configured default instead of back here.
Refs: #345
# If applied, this commit will Stop the 'reset login method choices' action from being triggerable by a plain, unauthenticated cross-site GET request. # Why is this change needed? Prior to this change, the confirm button in the reset modal was a plain <a href> pointing directly at EngineBlock's cross-origin /reset-remember-wayf endpoint. The confirmation modal was a client-side-only gate; the underlying action was a GET with no CSRF token, so it could be triggered by cross-site navigation or an auto-submitting link, silently clearing a user's remembered WAYF choices without their intent. # How does it address the issue? This change replaces the link with a same-origin POST form carrying a Symfony CSRF token. A new route validates the token and only then redirects the browser on to EngineBlock's existing reset endpoint, preserving the same redirect-back behavior for legitimate same-origin submissions. # Provide links to any relevant tickets, articles or other resources Found during review of PR #353.
Contributor
Author
|
We have decided to use the same design as the remove consent. |
johanib
requested changes
Sep 24, 2026
johanib
left a comment
johanib
left a comment
Contributor
There was a problem hiding this comment.
Two small issues, then good to merge!
Looks nice.
The modal has some potential a11y issues but that is pre-existing, not in scope for this pr.
| # The EngineBlock endpoint that clears a user's remembered per-SP WAYF IdP | ||
| # choices. Profile links to this with a `redirect` query parameter so the | ||
| # user is sent back here afterwards. See OpenConext-engineblock#2085/#345. | ||
| wayf_reset_url: "https://engine.dev.openconext.local/reset-remember-wayf" |
Contributor
There was a problem hiding this comment.
Update chaneglog.md with instructions that this param must be set.
| @include ie11Only('.myProfile__wayfResetButton') { | ||
| height: auto; | ||
| min-width: 14rem; | ||
| } |
Contributor
There was a problem hiding this comment.
Modal styling of the button is not sound?
I ran yarn & yarn encode dev, no browser caching.
Contributor
Author
There was a problem hiding this comment.
hmmm let me take a look
# If applied, this commit will... Fix the reset-choice confirmation button's layout in the modal and add changelog guidance for the new required parameter. # Why is this change needed? Prior to this change, the modal button used a width:100% rule on the form wrapper that broke the button's intended layout, and the CHANGELOG did not mention that the new wayf_reset_url parameter must be set by deployments. # How does it address the issue? This change replaces the width override with display:contents on the form wrapper so the button retains its natural sizing, and adds CHANGELOG entries describing the feature and the required parameter. # Provide links to any relevant tickets, articles or other resources #353
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
2 participants
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
Adds a "Login method choices" section to the bottom of the "my personal
data" page (
/my-profile) that lets a user reset every per-applicationWAYF login-method choice they've made, behind a confirmation popup.
Confirming sends the user to EngineBlock's
/reset-remember-wayfendpoint (OpenConext-engineblock#2085), which clears the cookie and
redirects back to this same page.
Closes #345.
Depends on OpenConext-engineblock#2097
/reset-remember-wayfneeds to accept aredirectquery parameter andsend a result signal back for this to fully work end-to-end; that's
OpenConext/OpenConext-engineblock#2097,
stacked on the already-open OpenConext-engineblock#2094. Without it,
this page's link still works, but the user is sent to EngineBlock's own
configured default afterwards instead of back here.
Scope note: this PR covers #345 only. #350 ("cookies have been
removed" feedback) and OpenConext-profile#346/#348 are intentionally out
of scope and deferred pending re-confirmation, per the epic's recommended
order.
Details
WayfResetLinkBuilderis a small, pure service that appends aredirectquery parameter (the absolute URL of this same page) to theconfigured
wayf_reset_url. It's factored out ofMyProfileControllerand unit tested in isolation, since this repo has no
controller/functional test infrastructure and the interesting logic
(building the link) doesn't need a kernel to test.
wayf_reset_urlparameter follows the existing pattern of simplestring config values in
parameters.yaml(.dist), pointing atEngineBlock's reset endpoint.
modal.html.twigconfirmationcomponent (the same one used for deleting a connection), wrapped in a
single-iteration
forloop: the helper unconditionally derives itselement ids from
loop.index, which only exists inside a loop, and itsone existing caller is always used from within a real loop over
services.
the SURFconext dashboard (
mySurfConext__download*), including itsIE11 fallback rules, since both are a text explanation next to a single
button.
profile.my_profilekey in all three locales (en/nl/pt). Exact copyfrom design mockups wasn't available to me, so the wording is a
best-effort draft that should be reviewed against the actual designs
before merging; the confirm button text ("Reset all") is the one piece
of copy specified by the ticket itself.
Testing
All run inside the CI Docker image
(
ghcr.io/openconext/openconext-basecontainers/php85-apache2-node24):composer check(validate, phplint, phpmd, phpcs, docheader, phpunit,phpstan): clean. 146/146 tests pass, including the new
WayfResetLinkBuilderTest(100% coverage).bin/console lint:twig: 57/57 templates valid.yarn build(Encore/Sass): compiles cleanly (only pre-existing Sassdeprecation warnings, unrelated to this change).
No controller/functional test was added for
MyProfileControlleritself, consistent with this repo's existing convention of not having
kernel/controller tests anywhere.