Skip to content

Preserve deeplink target after SAML login - #356

Merged
kayjoosten merged 1 commit into
mainfrom
issue-325-deeplink-preserve
Oct 1, 2026
Merged

kayjoosten merged 1 commit into
mainfrom
issue-325-deeplink-preserve

Conversation

@kayjoosten

Copy link
Copy Markdown
Contributor

What

Browsing to a specific page (e.g. /attribute-support) while unauthenticated, logging in via WAYF/the IdP, and returning through the SAML ACS endpoint always landed the user on / instead of the page they originally asked for.

Why

SamlAuthenticator is registered in security.yaml as a Symfony custom_authenticator rather than through one of Symfony's built-in authenticator factories (e.g. form_login). Symfony only calls AuthenticationSuccessHandler::setFirewallName() automatically for its own built-in factories, so the firewall name was never set on our SuccessHandler. Without a firewall name, DefaultAuthenticationSuccessHandler can't look up the deeplink stored in the session before authentication started, and always falls back to the default target path (/).

Fix

Explicitly configure the SuccessHandler service with the saml_based firewall name in config/services.yaml, restoring Symfony's normal deeplink-preservation behavior.

Testing

Added src/OpenConext/ProfileBundle/Tests/Security/SamlAuthenticationSuccessHandlerTest.php, which:

  • asserts the success handler is wired with the saml_based firewall name
  • asserts a deeplink stored in the session before login is used as the post-login redirect target
  • asserts the default target path is still used when no deeplink was stored

Verified these tests fail without the fix and pass with it. Ran the full composer check suite (composer-validate, phplint, phpmd, phpcs, license-headers, phpunit, phpstan) locally, all green.

Closes #325

@kayjoosten
kayjoosten force-pushed the issue-325-deeplink-preserve branch from 8a5cfe6 to 97a1cc2 Compare September 29, 2026 07:32
# If applied, this commit will...
Make the profile app redirect users back to the page they originally
requested after logging in via SAML, instead of always sending them to
the homepage.

# Why is this change needed?
Prior to this change, browsing to a specific page (e.g. /attribute-support)
while unauthenticated, logging in via WAYF/the IdP, and returning through
the SAML ACS endpoint always landed the user on "/" instead of the page
they originally asked for.

The SamlAuthenticator is registered in security.yaml as a Symfony
"custom_authenticator" rather than through one of Symfony's built-in
authenticator factories (e.g. form_login). Symfony only calls
AuthenticationSuccessHandler::setFirewallName() automatically for its own
built-in factories, so the firewall name was never set on our
SuccessHandler. Without a firewall name, DefaultAuthenticationSuccessHandler
is unable to look up the deeplink that was stored in the session before
authentication started, and always falls back to the default target path.

# How does it address the issue?
This change explicitly configures the SuccessHandler service with the
"saml_based" firewall name, so the URL that was stored in the session
before login is used again as the post-login redirect target. Tests
covering the wiring and the redirect behavior, with and without a stored
deeplink, are included.

# Provide links to any relevant tickets, articles or other resources
#325
@kayjoosten
kayjoosten force-pushed the issue-325-deeplink-preserve branch from 97a1cc2 to b7c34fb Compare September 29, 2026 07:55

@johanib johanib left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 This might have been a regression from a previous symfony upgrade.

This inserts the missing piece in the chain. tested 💯

@kayjoosten
kayjoosten merged commit 8ff7491 into main Oct 1, 2026
1 check passed
@kayjoosten
kayjoosten deleted the issue-325-deeplink-preserve branch October 1, 2026 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deeplink not preserved after login

2 participants