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 CSRF protection? #10

Open
callahad opened this issue Oct 27, 2016 · 2 comments
Open

Add CSRF protection? #10

callahad opened this issue Oct 27, 2016 · 2 comments

Comments

@callahad
Copy link
Member

Dubious as to the value in the case of the RP -- a malicious actor could just hit /auth on the Broker directly to initiate a login... but at least it wouldn't consume as many cycles on the RP?

...but probably still a good idea.

@vr2262
Copy link

vr2262 commented Nov 17, 2016

I just ran into this when creating a dummy application using Tornado. Is there any security risk with disabling the CSRF protection on the /verify endpoint? Or is there some way to have the response from the broker include an _xsrf argument that I supply?

@stephank
Copy link
Member

stephank commented Mar 9, 2020

Sorry for the very late update on this. I think this is simply a docs issue, so we can make this issue about improving docs. (Feel free to unsub if this is noise.)

There is no issue with disabling CSRF for the redirect_uri, because the authentication process contains a similar token already (the nonce). The original issue description no longer applies; a malicious actor cannot perform a successful login by using /auth directly.

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

No branches or pull requests

3 participants