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

Enabling multiple oauth2_proxy's to exist on the same domain #104

Closed
wants to merge 3 commits into from

Conversation

tonymeng
Copy link
Contributor

This PR enables multiple oauth2_proxy's to operate on the same domain while enforcing different levels of security.

The changes required involve namespacing the web template for the specific oauth2_proxy hosting it and namespacing the cookie's key.

@@ -300,7 +302,7 @@ func (p *OauthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code
rw.WriteHeader(code)

redirect_url := req.URL.RequestURI()
if redirect_url == signInPath {
if redirect_url == fmt.Sprintf(signInPathFormat, p.ProxyPrefix) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move these string formatting blocks to happen once when the &OauthProxy{...} is created? You can also drop the const's.

@jehiah
Copy link
Member

jehiah commented May 31, 2015

@tonymeng some comments on implementation, but this change in general sounds like it adds a bit of flexibility which is nice.

@@ -131,7 +132,7 @@ func NewOauthProxy(opts *Options, validator func(string) bool) *OauthProxy {
}

return &OauthProxy{
CookieKey: "_oauthproxy",
CookieKey: fmt.Sprintf("_oauthproxy_%s", opts.ProxyPrefix),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just make CookieKey a separate configurable not tied to ProxyPrefix

@tonymeng
Copy link
Contributor Author

tonymeng commented Jun 1, 2015

@jehiah thanks for getting back to me so fast, i've updated it per your comments, back to you :)

@jehiah
Copy link
Member

jehiah commented Jun 6, 2015

@tonymeng Thanks! I've merged this (with a few small docs changes and a fix to the cookie name) as c5ccd43

@jehiah jehiah closed this Jun 6, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

2 participants