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

Added allowed-url option for secure allowance of custom redirection URLs #544

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Onlinehead
Copy link

That is a small change which adding one option - "allowed-url" which able user to set regex for validate "rd" option instead of just replacing it to '/' if it contain absolute path.

Why?

Because if you have many services on different domains (as example - kubernetes cluster with tens of services) and want to protect them, you don't want to deploy many oauth2_proxy one per domain, you want to use one oauth2_proxy and validate redirection URLs for make redirection secure.

Example of usage

Adding option --allowed-url=.+\.internals\.example\.com will allow you to use one proxy for all services in subdomain .internals.example.com.

Is it tested?

I built a container with that change - onlinehead/oauth2_proxy:2.2.1 and tested it on my K8s cluster in pair of GitLab as Oauth provider. And looks like it working OK.

P.S. I am not sure that name of the option is right.

@ploxiln
Copy link
Contributor

ploxiln commented Feb 10, 2018

This is indeed a popular request. The current best implementation might be in #464

@Onlinehead
Copy link
Author

@ploxiln thanks for a link!
But, I think my implementation with regexp is a bit more useful because allow to set up a mask once instead of updating settings for every service and making possible to use some automation which creating new services with new FQDNs without pain with updating of relations like auth proxy.

@Onlinehead
Copy link
Author

But, I definitely have to add a test for my implementation and add support of set an option from the environment variable, looks like many peoples need it.

@ploxiln
Copy link
Contributor

ploxiln commented Feb 10, 2018

The implementation I linked to does support subdomain wildcard. That should cover most use cases.

I don't think supporting an environment variable is important. It's important for secret values, but not for other misc config options.

@Onlinehead
Copy link
Author

@ploxiln yes, you are right, I found that check. And I see that change in 'OAuthCallback' function is missed, because it has a same block with a check or redirection.
Anyway, I will add tests to my commit and, maybe, move detection to a separate function, for making that tests simpler.
Although I like regex and shorter implementations more than just strings with hidden magic in code, the choice is yours:)
In my cases, it will work anyway, at least now.

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

Successfully merging this pull request may close these issues.

None yet

2 participants