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 support for POST on a wrapped request #534

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sfc-gh-thardie
Copy link

@sfc-gh-thardie sfc-gh-thardie commented Sep 8, 2023

When a request is made to an endpoint with the method set to POST, if the user has no session currently, we start the SAML authentication flow. When the assertion comes back, we will then redirect to the URL where the POST request was made, which will do a GET to that URL with no body. This obviously breaks the original POST.

This PR adds storage of the method and post form storage to TrackedRequest , and once we are done with the SAML Assertion, if the original request was a POST, we use the same method that IdpAuthnRequest uses in WriteResponse to create a form that uses a tiny bit of javascript to POST back to the original endpoint.

Obviously this won't work for large forms, since the TrackedRequest will then generate a cookie which is too big, but it's better than making a GET to an endpoint expecting a POST.

In terms of the code that generates the HTML with the script, LMK if you think the function I added as well as WriteResponse should be factored out into some standalone, common function that both of these methods could use, and I could make an attempt at doing that.

sfc-gh-thardie and others added 4 commits September 8, 2023 12:45
In the case where you've used samlsp to wrap an endpoint that may get a POST request,
the previous implemetation just did a HTTP Redirect after the SAML assertion was done.
Comment on lines +238 to +249
text := fmt.Sprintf(`<html>`+
`<form method="post" action="%s" id="SAMLAfterAssertionRedirectForm">`, trackedRequest.URI)
for key, values := range trackedRequest.PostData {
for _, value := range values {
text = fmt.Sprintf(`%s<input type="hidden" name="%s" value="%s" />`, text, key, value)
}
}
text += `<input id="SAMLAfterAssertionRedirectSubmitButton" type="submit" value="Continue" />` +
`</form>` +
`<script>document.getElementById('SAMLAfterAssertionRedirectSubmitButton').style.visibility='hidden';</script>` +
`<script>document.getElementById('SAMLAfterAssertionRedirectForm').submit();</script>` +
`</html>`
Copy link

@patricsss patricsss Oct 28, 2023

Choose a reason for hiding this comment

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

This would be great to live in a template and then be rendered with the placeholders.

https://pkg.go.dev/text/template

That would also let you abstract this away into another file either as a constant-string-literal or as an actual text file that can be embedded and read from something like an embed.FS. I would strongly recommend the string literal though if you opt for this approach.

The template package supports the nested looping you're doing here "out of the box".

This would also make it considerably easier to write tests to ensure the form generated is as expected as the templating could be more easily tested than this method which has farm more state in it.

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.

2 participants