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

Create a state id without restart url #26

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

Conversation

sigmunau
Copy link
Contributor

@sigmunau sigmunau commented Sep 5, 2019

The restart url only makes sense for saml idps, and it will
potentially leak quite a bit of information that was encoded in the
url to the OP. Also the shorted state parameter makes for nice urls.

The restart url only makes sense for saml idps, and it will
potentially leak quite a bit of information that was encoded in the
url to the OP. Also the shorted state parameter makes for nice urls.
@jaimeperez
Copy link

The problem with removing the restart URL is that if something goes wrong, it's impossible to recover from that...

@sigmunau
Copy link
Contributor Author

sigmunau commented Sep 6, 2019

As far as I understand it, the restart url is only used to trigger a "idp initiated login" in saml flows if the state was somehow missing. OpenID Connect and OAuth2 has no such thing. In fact, if a oauth2 client did respond to a unsolicited login response, this would be considered a CSRF vulnerability in the client and it is explicitly forbidden by the oauth2 specs. What error scenarios were you thinking of that could be recovered by having this url in the state?

@pradtke
Copy link
Contributor

pradtke commented Sep 16, 2019

Could you make this a configuration option? I understand your point about it being forbidden by oauth2 spec but it seems that CSRF is a "feature" of saml since there is IDP initiated logins and SP have request initiated logins, both with can be initiated by any party.

The error scenario that is most common for us, where having the restart url could be useful, is when users decided to click back in the browser and Google/etc sends us a new code. Currently we just tell the user not to click back, but we may want to handle it with the restart url in the future.

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.

3 participants