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

Add FormRedirectStrategy to enable POST OIDC Logout #16214

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

candrews
Copy link
Contributor

@candrews candrews commented Dec 4, 2024

FormRedirectStrategy redirects using an autosubmitting HTML form using the POST method versus DefaultRedirectStrategy which redirects using the GET method.

Can be used to implement POST binding for relying party initiated OIDC logout by setting FormRedirectStrategy as the redirection strategy on OidcClientInitiatedLogoutSuccessHandler.

Closes gh-13002

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 4, 2024
@candrews candrews force-pushed the gh-13002 branch 2 times, most recently from cd8b993 to 2c41332 Compare December 4, 2024 18:07
@candrews
Copy link
Contributor Author

candrews commented Dec 4, 2024

I need to register the filter that handles /form-redirect.js, but I'm unsure where to do so and would greatly appreciate assistance.

/default-ui.css is registered at

and (for servlet) and
http.addFilterBefore(DefaultResourcesWebFilter.css(), SecurityWebFiltersOrder.LOGIN_PAGE_GENERATING);
(for reactive). The registration is conditional on the login page being configured. However, FormRedirectStrategy would be set up by the user not as bean or via any other special method, so I don't see a way to conditionally register the handler for /form-redirect.js. Should it be unconditionally registered? Should some kind of configuration be added to conditionally register it?

@candrews candrews force-pushed the gh-13002 branch 3 times, most recently from 2b6f36f to 95babac Compare December 4, 2024 19:04
FormRedirectStrategy redirects using an autosubmitting HTML form using the POST method versus DefaultRedirectStrategy which redirects using the GET method.

Can be used to implement POST binding for relying party initiated OIDC logout by setting FormRedirectStrategy as the redirection strategy on OidcClientInitiatedLogoutSuccessHandler.

Closes spring-projectsgh-13002

Signed-off-by: Craig Andrews <[email protected]>
@sjohnr sjohnr self-assigned this Dec 5, 2024
@sjohnr sjohnr added in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 5, 2024
@sjohnr
Copy link
Member

sjohnr commented Dec 5, 2024

@candrews thanks for the PR!

I need to register the filter that handles /form-redirect.js, but I'm unsure where to do so and would greatly appreciate assistance.

The registration is conditional on the login page being configured.

For this reason and and because of the simplicity of the javascript, I feel that registering a static resource for the javascript is not necessary in this case. Would you please consider adding inline javascript at the end of the page instead?

I think this case should similarly have its own CSS inline as well. I think it would be best to not rely on registering additional filters in order for the FormRedirectStrategy to be successfully plugged into the filter chain in various places.

@candrews
Copy link
Contributor Author

candrews commented Dec 5, 2024

Would you please consider adding inline javascript at the end of the page instead?

Inline javascript requires the CSP to allow unsafe-inline, and that's not something I want to do or that Spring Security should encourage.

Quoting https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#unsafe_inline_script

Allowing all inline scripts is considered a security risk

Many organizations prohibit unsafe-inline.

@sjohnr
Copy link
Member

sjohnr commented Dec 5, 2024

Inline javascript requires the CSP to allow unsafe-inline

Thanks. That's a good point. However, it does make this issue more difficult. Requiring additional configuration to add the javascript necessary for this simple page is not ideal. We might need to think about this a bit more and see if there's a more "all-inclusive" way to simply register the strategy and have the configuration work. I don't have anything in mind for what that would look like though.

@sjohnr
Copy link
Member

sjohnr commented Dec 5, 2024

Could we generate a Content-Security-Policy using a hash-source as outlined in that document? e.g.

Content-Security-Policy: script-src 'sha256-B2yPHKaXnvFWtRChIbabYmUBFZdVfKKXHbWtWidDVF8='

This seems like it would be very easy to do and well supported by browsers.

@candrews
Copy link
Contributor Author

candrews commented Dec 5, 2024

Could we generate a Content-Security-Policy using a hash-source as outlined in that document? e.g.

Content-Security-Policy: script-src 'sha256-B2yPHKaXnvFWtRChIbabYmUBFZdVfKKXHbWtWidDVF8='

This seems like it would be very easy to do and well supported by browsers.

We could - but I don't see anywhere in Spring Security where we could set that CSP in that way and not impact existing users.

@sjohnr
Copy link
Member

sjohnr commented Dec 5, 2024

I don't see anywhere in Spring Security where we could set that CSP in that way and not impact existing users.

By that, do you mean this would impact existing headers written by the HeaderWriterFilter, specifically from ContentSecurityPolicyHeaderWriter? Since this would be an opt-in configuration, it should be passive. Or do you mean something else?

@candrews
Copy link
Contributor Author

candrews commented Dec 6, 2024

By that, do you mean this would impact existing headers written by the HeaderWriterFilter, specifically from ContentSecurityPolicyHeaderWriter?

Yes. If a project uses ContentSecurityPolicyHeaderWriter then I believe it would overwrite the CSP described here or vice versa.

Since this would be an opt-in configuration, it should be passive. Or do you mean something else?

I think it would be automatic: if FormRedirectStrategy is used, then it should Just Work ™️ without any further setup required.

@sjohnr
Copy link
Member

sjohnr commented Dec 6, 2024

If a project uses ContentSecurityPolicyHeaderWriter then I believe it would overwrite the CSP described here or vice versa.

The ContentSecurityPolicyHeaderWriter does not write the header if it has already been written/added. So one option (not sure yet whether it's a good one) would be to just write the header in FormRedirectStrategy and it would win (but only if written first).

Another option might be to provide a mechanism to influence the CSP header that's written by the writer.

In any case, I think it would be nice to explore some alternative options because configuring static assets served by the filter chain seems like something we would only really do for things like the default login page, not collaborators/strategies like this one.

I think it would be automatic: if FormRedirectStrategy is used, then it should Just Work ™️ without any further setup required.

Yes, I agree. That would be ideal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Projects
Status: Prioritized
Development

Successfully merging this pull request may close these issues.

Add POST Binding for RP-initiated OIDC Logout
3 participants