From 2a1691a99459bfd245ccf87ad7e0c831716c3bb9 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Fri, 29 Sep 2017 16:55:50 +0100 Subject: [PATCH 1/8] Add whitelist domains flag --- main.go | 2 ++ oauthproxy.go | 34 ++++++++++++++++++++++++++++++++-- options.go | 1 + 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index 5cfa1f5..9b70da2 100644 --- a/main.go +++ b/main.go @@ -18,6 +18,7 @@ func main() { flagSet := flag.NewFlagSet("oauth2_proxy", flag.ExitOnError) emailDomains := StringArray{} + whitelistDomains := StringArray{} upstreams := StringArray{} skipAuthRegex := StringArray{} googleGroups := StringArray{} @@ -45,6 +46,7 @@ func main() { flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS") flagSet.Var(&emailDomains, "email-domain", "authenticate emails with the specified domain (may be given multiple times). Use * to authenticate any email") + flagSet.Var(&whitelistDomains, "whitelist-domains", "allowed domains for redirection after authentication") flagSet.String("azure-tenant", "common", "go to a tenant-specific or common (tenant-independent) endpoint.") flagSet.String("github-org", "", "restrict logins to members of this organisation") flagSet.String("github-team", "", "restrict logins to members of this team") diff --git a/oauthproxy.go b/oauthproxy.go index 64503cb..8029eab 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -68,6 +68,7 @@ type OAuthProxy struct { AuthOnlyPath string redirectURL *url.URL // the url to receive requests at + whitelistDomains []string provider providers.Provider ProxyPrefix string SignInMessage string @@ -220,6 +221,7 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy { provider: opts.provider, serveMux: serveMux, redirectURL: redirectURL, + whitelistDomains: opts.WhitelistDomains, skipAuthRegex: opts.SkipAuthRegex, skipAuthPreflight: opts.SkipAuthPreflight, compiledRegex: opts.CompiledRegex, @@ -563,7 +565,7 @@ func (p *OAuthProxy) GetRedirect(req *http.Request) (redirect string, err error) } redirect = req.Form.Get("rd") - if redirect == "" || !strings.HasPrefix(redirect, "/") || strings.HasPrefix(redirect, "//") { + if !p.IsValidRedirect(redirect) { redirect = req.URL.Path if strings.HasPrefix(redirect, p.ProxyPrefix) { redirect = "/" @@ -573,6 +575,34 @@ func (p *OAuthProxy) GetRedirect(req *http.Request) (redirect string, err error) return } +// IsValidRedirect checks whether the redirect URL is whitelisted +func (p *OAuthProxy) IsValidRedirect(redirect string) bool { + switch { + case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//"): + return true + case strings.HasPrefix(redirect, "http://"): + redirect = strings.TrimPrefix(redirect, "http://") + redirect = strings.Split(redirect, "/")[0] + for _, domain := range p.whitelistDomains { + if strings.HasSuffix(redirect, domain) { + return true + } + } + return false + case strings.HasPrefix(redirect, "https://"): + redirect = strings.TrimPrefix(redirect, "https://") + redirect = strings.Split(redirect, "/")[0] + for _, domain := range p.whitelistDomains { + if strings.HasSuffix(redirect, domain) { + return true + } + } + return false + default: + return false + } +} + // IsWhitelistedRequest is used to check if auth should be skipped for this request func (p *OAuthProxy) IsWhitelistedRequest(req *http.Request) (ok bool) { isPreflightRequestAllowed := p.skipAuthPreflight && req.Method == "OPTIONS" @@ -709,7 +739,7 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { return } - if !strings.HasPrefix(redirect, "/") || strings.HasPrefix(redirect, "//") { + if !p.IsValidRedirect(redirect) { redirect = "/" } diff --git a/options.go b/options.go index 9696144..3306567 100644 --- a/options.go +++ b/options.go @@ -33,6 +33,7 @@ type Options struct { AuthenticatedEmailsFile string `flag:"authenticated-emails-file" cfg:"authenticated_emails_file"` AzureTenant string `flag:"azure-tenant" cfg:"azure_tenant"` EmailDomains []string `flag:"email-domain" cfg:"email_domains"` + WhitelistDomains []string `flag:"whitelist-domains" cfg:"whitelist_domains"` GitHubOrg string `flag:"github-org" cfg:"github_org"` GitHubTeam string `flag:"github-team" cfg:"github_team"` GoogleGroups []string `flag:"google-group" cfg:"google_group"` From 768a6ce989bead40a01b90abe956dac1d11ccd44 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 2 Oct 2017 10:19:24 +0100 Subject: [PATCH 2/8] Test IsValidRedirect method --- oauthproxy_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index adc3cfb..b53c79b 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -93,6 +93,44 @@ func TestRobotsTxt(t *testing.T) { assert.Equal(t, "User-agent: *\nDisallow: /", rw.Body.String()) } +func TestIsValidRedirect(t *testing.T) { + opts := NewOptions() + opts.ClientID = "bazquux" + opts.ClientSecret = "foobar" + opts.CookieSecret = "xyzzyplugh" + opts.WhitelistDomains = []string{"foo.bar"} + opts.Validate() + + proxy := NewOAuthProxy(opts, func(string) bool { return true }) + + noRD := proxy.IsValidRedirect("") + assert.Equal(t, false, noRD) + + singleSlash := proxy.IsValidRedirect("/redirect") + assert.Equal(t, true, singleSlash) + + doubleSlash := proxy.IsValidRedirect("//redirect") + assert.Equal(t, false, doubleSlash) + + validHTTP := proxy.IsValidRedirect("http://baz.foo.bar/redirect") + assert.Equal(t, true, validHTTP) + + validHTTPS := proxy.IsValidRedirect("https://baz.foo.bar/redirect") + assert.Equal(t, true, validHTTPS) + + invalidHTTP1 := proxy.IsValidRedirect("http://foo.bar.evil.corp/redirect") + assert.Equal(t, false, invalidHTTP1) + + invalidHTTPS1 := proxy.IsValidRedirect("https://foo.bar.evil.corp/redirect") + assert.Equal(t, false, invalidHTTPS1) + + invalidHTTP2 := proxy.IsValidRedirect("http://evil.corp/redirect?rd=foo.bar") + assert.Equal(t, false, invalidHTTP2) + + invalidHTTPS2 := proxy.IsValidRedirect("https://evil.corp/redirect?rd=foo.bar") + assert.Equal(t, false, invalidHTTPS2) +} + type TestProvider struct { *providers.ProviderData EmailAddress string From fd875fc663922cc5a6ed5698d82327ea0eb0bcfc Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 11 Dec 2017 09:12:28 +0000 Subject: [PATCH 3/8] Make option name singular --- main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.go b/main.go index 9b70da2..33ab07f 100644 --- a/main.go +++ b/main.go @@ -46,7 +46,7 @@ func main() { flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS") flagSet.Var(&emailDomains, "email-domain", "authenticate emails with the specified domain (may be given multiple times). Use * to authenticate any email") - flagSet.Var(&whitelistDomains, "whitelist-domains", "allowed domains for redirection after authentication") + flagSet.Var(&whitelistDomains, "whitelist-domain", "allowed domains for redirection after authentication") flagSet.String("azure-tenant", "common", "go to a tenant-specific or common (tenant-independent) endpoint.") flagSet.String("github-org", "", "restrict logins to members of this organisation") flagSet.String("github-team", "", "restrict logins to members of this team") From bc4d5941fc9c0ee3e7d27a56cee1e3c3a9fc42ec Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 11 Dec 2017 09:24:52 +0000 Subject: [PATCH 4/8] Remove duplicated logic --- oauthproxy.go | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index 8029eab..18984d4 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -580,20 +580,13 @@ func (p *OAuthProxy) IsValidRedirect(redirect string) bool { switch { case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//"): return true - case strings.HasPrefix(redirect, "http://"): - redirect = strings.TrimPrefix(redirect, "http://") - redirect = strings.Split(redirect, "/")[0] - for _, domain := range p.whitelistDomains { - if strings.HasSuffix(redirect, domain) { - return true - } + case strings.HasPrefix(redirect, "http://") || strings.HasPrefix(redirect, "https://"): + redirectURL, err := url.Parse(redirect) + if err != nil { + return false } - return false - case strings.HasPrefix(redirect, "https://"): - redirect = strings.TrimPrefix(redirect, "https://") - redirect = strings.Split(redirect, "/")[0] for _, domain := range p.whitelistDomains { - if strings.HasSuffix(redirect, domain) { + if (redirectURL.Host == domain) || (strings.HasPrefix(domain, ".") && strings.HasSuffix(redirectURL.Host, domain)) { return true } } From 81f77a55deeb7969288cd307123072fc83b8a90a Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 20 Jun 2018 12:05:17 +0100 Subject: [PATCH 5/8] Add note on subdomain behaviour --- README.md | 3 +++ main.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 910a671..c23484d 100644 --- a/README.md +++ b/README.md @@ -237,8 +237,11 @@ Usage of oauth2_proxy: -upstream value: the http url(s) of the upstream endpoint or file:// paths for static files. Routing is based on the path -validate-url string: Access token validation endpoint -version: print version string + -whitelist-domain: allowed domains for redirection after authentication. Prefix domain with a . to allow subdomains (eg .example.com) ``` +Note, when using the `whitelist-domain` option, any domain prefixed with a `.` will allow any subdomain of the specified domain as a valid redirect URL. + See below for provider specific options ### Upstreams Configuration diff --git a/main.go b/main.go index 33ab07f..2e3cbf4 100644 --- a/main.go +++ b/main.go @@ -46,7 +46,7 @@ func main() { flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS") flagSet.Var(&emailDomains, "email-domain", "authenticate emails with the specified domain (may be given multiple times). Use * to authenticate any email") - flagSet.Var(&whitelistDomains, "whitelist-domain", "allowed domains for redirection after authentication") + flagSet.Var(&whitelistDomains, "whitelist-domain", "allowed domains for redirection after authentication. Prefix domain with a . to allow subdomains (eg .example.com)") flagSet.String("azure-tenant", "common", "go to a tenant-specific or common (tenant-independent) endpoint.") flagSet.String("github-org", "", "restrict logins to members of this organisation") flagSet.String("github-team", "", "restrict logins to members of this team") From 9007d66559f566de6dbb98c397d7e2ff00d934f9 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 11 Dec 2017 09:33:52 +0000 Subject: [PATCH 6/8] Test explicit subdomain whitelisting --- oauthproxy_test.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index b53c79b..973fa4a 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -98,7 +98,8 @@ func TestIsValidRedirect(t *testing.T) { opts.ClientID = "bazquux" opts.ClientSecret = "foobar" opts.CookieSecret = "xyzzyplugh" - opts.WhitelistDomains = []string{"foo.bar"} + // Should match domains that are exactly foo.bar and any subdomain of bar.foo + opts.WhitelistDomains = []string{"foo.bar", ".bar.foo"} opts.Validate() proxy := NewOAuthProxy(opts, func(string) bool { return true }) @@ -112,12 +113,24 @@ func TestIsValidRedirect(t *testing.T) { doubleSlash := proxy.IsValidRedirect("//redirect") assert.Equal(t, false, doubleSlash) - validHTTP := proxy.IsValidRedirect("http://baz.foo.bar/redirect") + validHTTP := proxy.IsValidRedirect("http://foo.bar/redirect") assert.Equal(t, true, validHTTP) - validHTTPS := proxy.IsValidRedirect("https://baz.foo.bar/redirect") + validHTTPS := proxy.IsValidRedirect("https://foo.bar/redirect") assert.Equal(t, true, validHTTPS) + invalidHTTPSubdomain := proxy.IsValidRedirect("http://baz.foo.bar/redirect") + assert.Equal(t, false, invalidHTTPSubdomain) + + invalidHTTPSSubdomain := proxy.IsValidRedirect("https://baz.foo.bar/redirect") + assert.Equal(t, false, invalidHTTPSSubdomain) + + validHTTPSubdomain := proxy.IsValidRedirect("http://baz.bar.foo/redirect") + assert.Equal(t, true, validHTTPSubdomain) + + validHTTPSSubdomain := proxy.IsValidRedirect("https://baz.bar.foo/redirect") + assert.Equal(t, true, validHTTPSSubdomain) + invalidHTTP1 := proxy.IsValidRedirect("http://foo.bar.evil.corp/redirect") assert.Equal(t, false, invalidHTTP1) From 52b50a49edc55770a0b695942422a96eb1c0c59c Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Thu, 18 Jan 2018 10:20:50 +0000 Subject: [PATCH 7/8] Add env option --- options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/options.go b/options.go index 3306567..c54038c 100644 --- a/options.go +++ b/options.go @@ -33,7 +33,7 @@ type Options struct { AuthenticatedEmailsFile string `flag:"authenticated-emails-file" cfg:"authenticated_emails_file"` AzureTenant string `flag:"azure-tenant" cfg:"azure_tenant"` EmailDomains []string `flag:"email-domain" cfg:"email_domains"` - WhitelistDomains []string `flag:"whitelist-domains" cfg:"whitelist_domains"` + WhitelistDomains []string `flag:"whitelist-domain" cfg:"whitelist_domains" env:"OAUTH2_PROXY_WHITELIST_DOMAINS"` GitHubOrg string `flag:"github-org" cfg:"github_org"` GitHubTeam string `flag:"github-team" cfg:"github_team"` GoogleGroups []string `flag:"google-group" cfg:"google_group"` From 987b25fae74c16c5f65a21703b16f5756532f652 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Tue, 22 Jan 2019 12:01:37 +0000 Subject: [PATCH 8/8] Add whitelist domain to changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51086d0..5ced5d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ - Implement token refreshing in OIDC provider - Split cookies larger than 4k limit into multiple cookies - Implement token validation in OIDC provider +- [#15](https://github.com/pusher/oauth2_proxy/pull/21) WhitelistDomains (@joelspeed) + - Add `--whitelist-domain` flag to allow redirection to approved domains after OAuth flow - [#21](https://github.com/pusher/oauth2_proxy/pull/21) Docker Improvement (@yaegashi) - Move Docker base image from debian to alpine - Install ca-certificates in docker image