Merge pull request #15 from pusher/whitelist-domains

Whitelist domains

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

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

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{}
@@ -46,6 +47,7 @@ func main() {
 	flagSet.Duration("flush-interval", time.Duration(1)*time.Second, "period between response flushing when streaming responses")
 
 	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. 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")

oauthproxy.go

@@ -70,6 +70,7 @@ type OAuthProxy struct {
 	AuthOnlyPath      string
 
 	redirectURL         *url.URL // the url to receive requests at
+	whitelistDomains    []string
 	provider            providers.Provider
 	ProxyPrefix         string
 	SignInMessage       string
@@ -224,6 +225,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,
@@ -567,7 +569,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 = "/"
@@ -577,6 +579,27 @@ 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://") || strings.HasPrefix(redirect, "https://"):
+		redirectURL, err := url.Parse(redirect)
+		if err != nil {
+			return false
+		}
+		for _, domain := range p.whitelistDomains {
+			if (redirectURL.Host == domain) || (strings.HasPrefix(domain, ".") && strings.HasSuffix(redirectURL.Host, 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"
@@ -713,7 +736,7 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	if !strings.HasPrefix(redirect, "/") || strings.HasPrefix(redirect, "//") {
+	if !p.IsValidRedirect(redirect) {
 		redirect = "/"
 	}
 

oauthproxy_test.go

@@ -93,6 +93,57 @@ 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"
+	// 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 })
+
+	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://foo.bar/redirect")
+	assert.Equal(t, true, validHTTP)
+
+	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)
+
+	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

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-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"`