Whitelist domains

[JoelSpeed]

This PR replaces: bitly/oauth2_proxy#464
Fixes: #12

Description

Adds a whitelist-domain flag that can be used to whitelist a set of domains for the redirect parameter in the authentication request

Motivation and Context

I wrote this to fix #399.

It allows you to set a whitelist of redirect domains which is useful when you wish to run 1 copy of the oauth2_proxy but protect multiple different endpoints (used particularly in the nginx auth_request mode)

How Has This Been Tested?

This has been running for us in production for nearly a year.

Checklist:

• My change requires a change to the documentation or CHANGELOG.
• I have updated the documentation/CHANGELOG accordingly.
• I have created a feature (non-master) branch for my PR.

[danihodovic]

Doesn't the cookie-domains option already allow you to do this? What is the difference?

[JoelSpeed]

Doesn't the cookie-domains option already allow you to do this? What is the difference?

It does not. There is a security check in place as part of any OAuth2 flow that you should have a predefined list of approved "redirect URLs". The initial request to the OAuth2 Proxy will have an rd query parameter that tells the proxy where to redirect to after it has completed the authentication flow.

This PR allows you to set the redirect URL to be on a different domain than the OAuth2 proxy is hosted. With the current behaviour, if I have the OAuth2 proxy hosted at auth.example.com and my app at app.example.com, then I would want ?rd=app.example.com/some/page but the OAuth2 proxy expects to be on the same domain and so would replace this with ?rd=/ and redirect back to auth.example.com once the authentication flow is completed.

As I say, it is standard to be able to redirect to a different domain as part of OAuth2 but there has to be a validated list, which is what we are implementing in this PR.

As for the cookie domain, that is only used when returning the Set-Cookie header, arguably we could leverage this flag to guess the redirects but that is less flexible, if cookie-domain: .example.com then we would have to allow redirects to any subdomain of example.com, which may not be desired, with this approach we can set whitelist-domains: foo.example.com, bar.example.com and only allow the separate proxy to protect those two subdomains.

Does that answer your question @danihodovic?

[danihodovic]

Yes, thanks! 👍 I've worked around this problem by having example.com as a sort of index page for the internal tools and set cookie domain to cookie-domain = .example.com, but your solution looks more elegant.

[Miouge1]

This is great! It's one of the most popular PR in the bitly project, I'm looking forward to see this in push/oauth2_proxy.

[loshz]

LGTM once conflicts are fixed :)

[jaybe78]

Hey @JoelSpeed ,

I've tested the branch. It works so good! Thanks for the great job.
Will this be merged anytime soon ?

Thx

[JoelSpeed]

I was just waiting on a sanity check after my rebase but since the tests didn't break I don't think I've changed the behaviour, which is good to know 😅

I'll get this merged shortly

[JoelSpeed]

@syscll Got a moment to re-approve this? Pushing to the branch dismissed the review

[loshz]

LGTM 👍

[Miouge1]

I would love to see this released in 3.1.0 :)

[JoelSpeed]

My current plan is to get #16 wrapped up and then cut a new release at that point, hopefully within the next week or so 😄

[Miouge1]

3.1.0 is now release with white-list domain support. Well done everyone 🎉 !

[ghostsquad]

The rd= parameter is not really explained anywhere. I'd like to know what's the best way to allow redirecting to arbitrary subdomains.