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

[ENHANCE] provide means to associate state with each saml request before the redirect to the idp #541

Closed
hcldan opened this issue Feb 18, 2021 · 5 comments

Comments

@hcldan
Copy link

hcldan commented Feb 18, 2021

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
When users have saved tabs in a browser, and they open their browser after a session has expired, it results in all tabs getting a redirect to log in. If the site uses a cookie to figure out where to return, then all tabs get clobbered.

I notice there is an inResponseTo attribute in the profile that comes back from the idp... can I hook somewhere to inspect this value before the request is made so that I can map the various saml requests to the redirect they should go to properly?

Describe the solution you'd like
passport.authenticate('saml', { state: (requestId) => { /* save reference to redirect keyed by requestId in session */ } })

@cjbarth
Copy link
Collaborator

cjbarth commented Feb 20, 2021

I'm not sure how this would add to what is already possible. I know many users of this library federate with multiple IdP without issue, in fact, we have a special class just for that. However, if you care to put together a PR, I'll have a look.

@hcldan
Copy link
Author

hcldan commented Feb 25, 2021

@cjbarth am I understanding the concept of inResponseTo correctly?

I'm not using multiple idps, but it would be useful to get an identifier for any outbound redirect to an IDP for each unique request to the IDP.

Again, imagine you open your browser to 10 github(configured with saml) PR request tabs. Each tab is a different PR. Each of those tabs is operating on the same expired authentication session with github. each of those tabs will redirect you to the IDP to log in.

If each of those redirects had a unique identifier saved in the github session, it could map those requests and understand where the correct redirect should go when the authentication succeds.

I'm not implying that github uses this software, just demonstrating the scenario.

@srd90
Copy link

srd90 commented Mar 1, 2021

@hcldan Your use case sound like use case of RelayState.

3.5.3 RelayState

RelayState data MAY be included with a SAML protocol message transmitted with this binding. The value MUST NOT exceed 80 bytes in length and SHOULD be integrity protected by the entity creating the message independent of any other protections that may or may not exist during message transmission.Signing is not realistic given the space limitation, but because the value is exposed to third-party tampering, the entity SHOULD ensure that the value has not been tampered with by using a checksum, a pseudo-random value, or similar means.

If a SAML request message is accompanied by RelayState data, then the SAML responder MUST return its SAML protocol response using a binding that also supports a RelayState mechanism, and it MUST place the exact data it received with the request into the corresponding RelayState parameter in the response.

If no such value is included with a SAML request message, or if the SAML response message is being generated without a corresponding request, then the SAML responder MAY include RelayState data to be interpreted by the recipient based on the use of a profile or prior agreement between the parties.

source: https://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf

If that is the case see additional more down to earth description about RelayState e.g. from

Current state of passport-saml's support for per request RelayState is documented by someone in following answer: https://stackoverflow.com/a/46555155

passport-saml's RelayState related issues/pull requests:
https://github.com/node-saml/passport-saml/search?q=relaystate&type=issues

@hcldan
Copy link
Author

hcldan commented Mar 5, 2021

I think that might work. I'm going to close this until I have a chance to play with that.

@hcldan hcldan closed this as completed Mar 5, 2021
@cjbarth
Copy link
Collaborator

cjbarth commented Mar 5, 2021

If you get it working using RelayState @hcldan , don't hesitate to update this projects readme file with a a PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants