Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SAML 2.0 Login should allow loginProcessingUrl without {registrationId} when providing an AuthenticationConverter #10176

Closed
sathishkumar294 opened this issue Aug 15, 2021 · 6 comments · Fixed by #10307
Assignees
Labels
in: saml2 An issue in SAML2 modules status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@sathishkumar294
Copy link

Assert.state(loginProcessingUrl.contains("{registrationId}"), "{registrationId} path variable is required");

Since we are allowed to use a custom assertionConsumerLocation when registering a relying party, I do not understand why the loginProcessingUrl should contain the {registrationId}?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 15, 2021
@marcusdacoregio marcusdacoregio added in: saml2 An issue in SAML2 modules and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 17, 2021
@marcusdacoregio
Copy link
Contributor

Hi @sathishkumar294, thanks for reaching out.

The registrationId is an arbitrary value that you choose for differentiating between registrations. There could be many different RelyingPartyRegistrations and each one will be identified by the registrationId.

When the request comes in, we already know which registration it is referring to since we have the variable in the URL.

I am closing this but feel free to continue the discussion.

@sathishkumar294
Copy link
Author

HI @marcusdacoregio thanks for your explanation. I agree with your answer considering how it will be really beneficial to new software solutions.
I am in the process of migrating an existing tool written in 2014 to latest version of spring security. Since the URLs related to login/assertion are already in use by several users for years now, we wanted to retain the same URLs in the new application so that the experience would look seamless for the users. Since the registrationId is mandatory in the login processing url, I am not able to re-use the previous login URL. I would suggest the need to use spring-security's conventions for URLs to my business team and try to convince them it is necessary for our future.

Thanks.

@marcusdacoregio
Copy link
Contributor

Hi @sathishkumar294.

I was talking with the team, and the filterProcessingUrl should not be required to contain the {registrationId} when you provide an AuthenticationConverter to resolve the RelyingPartyRegistration.

Do you mind if I change the title of this issue to SAML 2.0 Login should allow filterProcessingUrl without {registrationId} when providing an AuthenticationConverter?

@marcusdacoregio marcusdacoregio added the type: bug A general bug label Sep 20, 2021
@marcusdacoregio marcusdacoregio changed the title Why is {registrationId} mandatory when declaring a loginProcessingURL? SAML 2.0 Login should allow filterProcessingUrl without {registrationId} when providing an AuthenticationConverter Sep 20, 2021
@marcusdacoregio marcusdacoregio changed the title SAML 2.0 Login should allow filterProcessingUrl without {registrationId} when providing an AuthenticationConverter SAML 2.0 Login should allow filterProcessingUrl without {registrationId} when providing an AuthenticationConverter Sep 20, 2021
@marcusdacoregio marcusdacoregio changed the title SAML 2.0 Login should allow filterProcessingUrl without {registrationId} when providing an AuthenticationConverter SAML 2.0 Login should allow loginProcessingUrl without {registrationId} when providing an AuthenticationConverter Sep 20, 2021
@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Sep 20, 2021

As a workaround, you can use the ObjectPostProcessor to set the filterProcessingUrl in Saml2WebSsoAuthenticationFilter.

http
    .csrf((csrf) -> csrf.ignoringRequestMatchers(new AntPathRequestMatcher("/login/saml2/sso"))) // If using POST-binding
    .saml2Login((saml2) -> saml2
        .authenticationConverter(myAuthenticationConverter)
        .withObjectPostProcessor(new ObjectPostProcessor<Saml2WebSsoAuthenticationFilter>() {
            @Override
	    public<O extends Saml2WebSsoAuthenticationFilter> O postProcess(O object) {
	        object.setFilterProcessesUrl("/login/saml2/sso");
                return object;
	    }
        })
)

Remember that you have to provide the AuthenticationConverter since the default implementation relies on the registrationId to be present in the URL.

@sathishkumar294
Copy link
Author

Thanks, I will try this recommendation and update here.

@spring-projects-issues
Copy link

Fixed via 816e847

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: saml2 An issue in SAML2 modules status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
3 participants