Skip to content
This repository has been archived by the owner on Jan 24, 2019. It is now read-only.

CSRF protection for OAuth flow. #360

Merged
merged 1 commit into from
Mar 29, 2017
Merged

Conversation

jehiah
Copy link
Member

@jehiah jehiah commented Mar 29, 2017

This implements CSRF protection for the initiation and termination of the OAuth flow. (thanks @arnottcr and @TheRook for the contribution).

Fixes #336, #321

@jehiah jehiah added the bug label Mar 29, 2017
@jehiah jehiah force-pushed the csrf_validation_360 branch 2 times, most recently from 6fb84e5 to 720baad Compare March 29, 2017 01:50
@jehiah jehiah force-pushed the csrf_validation_360 branch from f6aee61 to 55085d9 Compare March 29, 2017 13:31
@jehiah jehiah merged commit 4464655 into bitly:master Mar 29, 2017
@jehiah jehiah deleted the csrf_validation_360 branch April 24, 2017 16:25
@colemickens
Copy link

@jehiah / @arnottcr Can you help me understand specifically why this PR was submitted?

I'm using oauth2 proxy at https://auth.mydomain.tld to protect a resource https://super-secret.mydomain.tld.

RE: this specific function: 55085d9#diff-6fd8df33d9f8086bc31e28c375fbc0abL387

AFAICT, this code means that hitting https://auth.mydomain.tld/oauth2/start?rd=https://super-secret.mydomain.tld will never work, because the redirect is always reset to /.

Does it ever make sense to relax this?

@colemickens
Copy link

I may not have linked the exact right location in code, there are a couple functions that "filter" the redirect down to '/'.

It seems like this was done for security reasons. Would it be sufficient to accept a list of domains to respect? (For example, I am already having oauth2proxy set the cookie domain to mydomain.tld, can I also have a --redirect-whitelist=mydomain.tld or some such?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

2 participants