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.

Sign up for GitHub

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

Jump to bottom

SAML Logout move onload script to body tag #11879

Closed
ugrave opened this issue Sep 19, 2022 · 1 comment
Closed

SAML Logout move onload script to body tag #11879

ugrave opened this issue Sep 19, 2022 · 1 comment
Assignees
@marcusdacoregio
Labels
in: saml2 An issue in SAML2 modules type: bug A general bug
Milestone
5.8.0-RC1

Comments

@ugrave
Copy link
Contributor

ugrave commented Sep 19, 2022 •
edited

With PR #11676 the submit of the form was moved fron body to inline script. But it was no removed on all onLoad body tags:

spring-security/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2LogoutRequestFilter.java

Line 222 in e4e24c6

html.append(" <body onload=\"document.forms[0].submit()\">\n");

and

spring-security/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/Saml2RelyingPartyInitiatedLogoutSuccessHandler.java

Line 126 in e4e24c6

html.append(" <body onload=\"document.forms[0].submit()\">\n");

still have the onLoad on the body and also now in the inlince script section.

Is there a reason/benefit to put the script outside of the head and body tag?
@ugrave ugrave added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Sep 19, 2022
@marcusdacoregio marcusdacoregio added in: saml2 An issue in SAML2 modules and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 19, 2022
@marcusdacoregio marcusdacoregio self-assigned this Sep 19, 2022
@marcusdacoregio marcusdacoregio added this to the 5.8.0-RC1 milestone Sep 19, 2022
@marcusdacoregio marcusdacoregio changed the title SAML Post class submit in body and inlince script SAML Logout remove onload from body tag Sep 19, 2022
@marcusdacoregio
Copy link
Contributor

Thanks for the report @ugrave.

Is there a reason/benefit to put the script outside of the head and body tag?

No, there isn't. It is even recommended to have the scripts inside the <head> or <body> tags, emphasis by me:

The SCRIPT element places a script within a document. This element may appear any number of times in the HEAD or BODY of an HTML document.

@marcusdacoregio marcusdacoregio mentioned this issue Sep 19, 2022
Closed
@marcusdacoregio marcusdacoregio changed the title SAML Logout remove onload from body tag SAML Logout move onload script to body tag Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@marcusdacoregio marcusdacoregio

Labels
in: saml2 An issue in SAML2 modules type: bug A general bug
Projects
None yet
Milestone
5.8.0-RC1
Development

No branches or pull requests
2 participants
@ugrave @marcusdacoregio