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

Provide ability to customize the SAML authnRequest URL of OpenSamlAuthenticationRequestResolver #11875

Closed
scoldwell opened this issue Sep 16, 2022 · 5 comments
Assignees
Labels
status: invalid An issue that we don't feel is valid type: enhancement A general enhancement

Comments

@scoldwell
Copy link

Currently there is no way to customize the SAML authn request processing URL without copying entire classes of existing Spring Security code which is not good for code maintenance.

We need to be able to customize the SAML authn request processing URL used for creating a request matcher in the package private OpenSamlAuthenticationRequestResolver and the classes that use it: OpenSaml4AuthenticationRequestResolver and OpenSaml3AuthenticationRequestResolver.

Without this ability to customize the URL on OpenSamlAuthenticationRequestResolver and its associated classes, it would require us to copy all the code from OpenSamlAuthenticationRequestResolver, OpenSaml3AuthenticationRequestResolver and OpenSaml4AuthenticationRequestResolver just to change a simple URL as these classes are all either marked as final or are package private and cannot be subclassed.

@scoldwell scoldwell added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Sep 16, 2022
@scoldwell
Copy link
Author

I should have mentioned we are using spring-security 5.7.3. I would be happy to issue a PR to fix this, but I want to make sure I follow the correct procedure and fork the correct branch and such.

@sjohnr
Copy link
Member

sjohnr commented Sep 16, 2022

Thanks for reaching out, @scoldwell!

Please note that the OpenSaml4AuthenticationRequestResolver class has a setRequestMatcher() method that can be used to customize request matcher, which in turn sets this on the internal OpenSamlAuthenticationRequestResolver. The docs cover how to customize the Saml2AuthenticationRequestResolver. I believe this is what you're looking for, but please correct me if I'm wrong.

Thanks for reaching out first, as it's always better to discuss it before submitting a PR. In many cases, classes that are hidden or final will have other ways of influencing their behavior/configuration without requiring making them public. With that in mind, I'm going to close this issue as answered. If I've misunderstood anything, please add a comment and we can re-open if necessary.

@sjohnr sjohnr closed this as completed Sep 16, 2022
@sjohnr sjohnr self-assigned this Sep 16, 2022
@sjohnr sjohnr added status: declined A suggestion or change that we don't feel we should currently apply status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged status: declined A suggestion or change that we don't feel we should currently apply labels Sep 16, 2022
@scoldwell
Copy link
Author

scoldwell commented Sep 16, 2022

Hi @sjohnr. It looks like that change is only in the 5.8 branch not the 5.7 branch. Any chance of getting that ported to 5.7?

@sjohnr
Copy link
Member

sjohnr commented Sep 16, 2022

Hi @scoldwell. Typically, we don't backport new features, and it looks like this was indeed introduced in 5.8 as you say in gh-10840. If there's no other option, you could temporarily copy the class from 5.8, place it in the same package (org.springframework.security.saml2.provider.service.web.authentication) and delete the class once 5.8 is released and you have upgraded to it.

This comment may also be helpful to you.

@scoldwell
Copy link
Author

@sjohnr Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants