Merge pull request #789 from grnhse/skip-auth-http-method

Support HTTP method based allowlists

CHANGELOG.md

@@ -4,6 +4,9 @@
 
 ## Important Notes
 
+- [#789](https://github.com/oauth2-proxy/oauth2-proxy/pull/789) `--skip-auth-route` is (almost) backwards compatible with `--skip-auth-regex`
+  - We are marking `--skip-auth-regex` as DEPRECATED and will remove it in the next major version.
+  - If your regex contains an `=` and you want it for all methods, you will need to add a leading `=` (this is the area where `--skip-auth-regex` doesn't port perfectly)
 - [#575](https://github.com/oauth2-proxy/oauth2-proxy/pull/575) Sessions from v5.1.1 or earlier will no longer validate since they were not signed with SHA1.
   - Sessions from v6.0.0 or later had a graceful conversion to SHA256 that resulted in no reauthentication
   - Upgrading from v5.1.1 or earlier will result in a reauthentication
@@ -23,6 +26,7 @@
 ## Changes since v6.1.1
 
 - [#753](https://github.com/oauth2-proxy/oauth2-proxy/pull/753) Pass resource parameter in login url (@codablock)
+- [#789](https://github.com/oauth2-proxy/oauth2-proxy/pull/789) Add `--skip-auth-route` configuration option for `METHOD=pathRegex` based allowlists (@NickMeves)
 - [#575](https://github.com/oauth2-proxy/oauth2-proxy/pull/575) Stop accepting legacy SHA1 signed cookies (@NickMeves)
 - [#722](https://github.com/oauth2-proxy/oauth2-proxy/pull/722) Validate Redis configuration options at startup (@NickMeves)
 - [#791](https://github.com/oauth2-proxy/oauth2-proxy/pull/791) Remove GetPreferredUsername method from provider interface (@NickMeves)

docs/configuration/configuration.md

@@ -119,8 +119,9 @@ An example [oauth2-proxy.cfg]({{ site.gitweb }}/contrib/oauth2-proxy.cfg.example
 | `--signature-key` | string | GAP-Signature request signature key (algorithm:secretkey) | |
 | `--silence-ping-logging` | bool | disable logging of requests to ping endpoint | false |
 | `--skip-auth-preflight` | bool | will skip authentication for OPTIONS requests | false |
-| `--skip-auth-regex` | string | bypass authentication for requests paths that match (may be given multiple times) | |
+| `--skip-auth-regex` | string \| list | (DEPRECATED for `--skip-auth-route`) bypass authentication for requests paths that match (may be given multiple times) | |
-| `--skip-auth-strip-headers` | bool | strips `X-Forwarded-*` style authentication headers & `Authorization` header if they would be set by oauth2-proxy for request paths in `--skip-auth-regex` | false |
+| `--skip-auth-route` | string \| list | bypass authentication for requests that match the method & path. Format: method=path_regex OR path_regex alone for all methods | |
+| `--skip-auth-strip-headers` | bool | strips `X-Forwarded-*` style authentication headers & `Authorization` header if they would be set by oauth2-proxy for allowlisted requests (`--skip-auth-route`, `--skip-auth-regex`, `--skip-auth-preflight`, `--trusted-ip`) | false |
 | `--skip-jwt-bearer-tokens` | bool | will skip requests that have verified JWT bearer tokens | false |
 | `--skip-oidc-discovery` | bool | bypass OIDC endpoint discovery. `--login-url`, `--redeem-url` and `--oidc-jwks-url` must be configured in this case | false |
 | `--skip-provider-button` | bool | will skip sign-in-page to directly reach the next step: oauth/start | false |

oauthproxy.go

@@ -48,6 +48,12 @@ var (
 	invalidRedirectRegex = regexp.MustCompile(`[/\\](?:[\s\v]*|\.{1,2})[/\\]`)
 )
 
+// allowedRoute manages method + path based allowlists
+type allowedRoute struct {
+	method    string
+	pathRegex *regexp.Regexp
+}
+
 // OAuthProxy is the main authentication proxy
 type OAuthProxy struct {
 	CookieSeed     string
@@ -70,6 +76,7 @@ type OAuthProxy struct {
 	AuthOnlyPath      string
 	UserInfoPath      string
 
+	allowedRoutes           []allowedRoute
 	redirectURL             *url.URL // the url to receive requests at
 	whitelistDomains        []string
 	provider                providers.Provider
@@ -90,13 +97,11 @@ type OAuthProxy struct {
 	SetAuthorization        bool
 	PassAuthorization       bool
 	PreferEmailToUser       bool
-	skipAuthRegex           []string
 	skipAuthPreflight       bool
 	skipAuthStripHeaders    bool
 	skipJwtBearerTokens     bool
 	mainJwtBearerVerifier   *oidc.IDTokenVerifier
 	extraJwtBearerVerifiers []*oidc.IDTokenVerifier
-	compiledRegex           []*regexp.Regexp
 	templates               *template.Template
 	realClientIPParser      ipapi.RealClientIPParser
 	trustedIPs              *ip.NetSet
@@ -121,10 +126,6 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
 		return nil, fmt.Errorf("error initialising upstream proxy: %v", err)
 	}
 
-	for _, u := range opts.GetCompiledRegex() {
-		logger.Printf("compiled skip-auth-regex => %q", u)
-	}
-
 	if opts.SkipJwtBearerTokens {
 		logger.Printf("Skipping JWT tokens from configured OIDC issuer: %q", opts.OIDCIssuerURL)
 		for _, issuer := range opts.ExtraJwtIssuers {
@@ -163,6 +164,11 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
 		}
 	}
 
+	allowedRoutes, err := buildRoutesAllowlist(opts)
+	if err != nil {
+		return nil, err
+	}
+
 	sessionChain := buildSessionChain(opts, sessionStore, basicAuthValidator)
 
 	return &OAuthProxy{
@@ -192,14 +198,13 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
 		sessionStore:            sessionStore,
 		serveMux:                upstreamProxy,
 		redirectURL:             redirectURL,
+		allowedRoutes:           allowedRoutes,
 		whitelistDomains:        opts.WhitelistDomains,
-		skipAuthRegex:           opts.SkipAuthRegex,
 		skipAuthPreflight:       opts.SkipAuthPreflight,
 		skipAuthStripHeaders:    opts.SkipAuthStripHeaders,
 		skipJwtBearerTokens:     opts.SkipJwtBearerTokens,
 		mainJwtBearerVerifier:   opts.GetOIDCVerifier(),
 		extraJwtBearerVerifiers: opts.GetJWTBearerVerifiers(),
-		compiledRegex:           opts.GetCompiledRegex(),
 		realClientIPParser:      opts.GetRealClientIPParser(),
 		SetXAuthRequest:         opts.SetXAuthRequest,
 		PassBasicAuth:           opts.PassBasicAuth,
@@ -277,6 +282,53 @@ func buildSignInMessage(opts *options.Options) string {
 	return msg
 }
 
+// buildRoutesAllowlist builds an []allowedRoute  list from either the legacy
+// SkipAuthRegex option (paths only support) or newer SkipAuthRoutes option
+// (method=path support)
+func buildRoutesAllowlist(opts *options.Options) ([]allowedRoute, error) {
+	routes := make([]allowedRoute, 0, len(opts.SkipAuthRegex)+len(opts.SkipAuthRoutes))
+
+	for _, path := range opts.SkipAuthRegex {
+		compiledRegex, err := regexp.Compile(path)
+		if err != nil {
+			return nil, err
+		}
+		logger.Printf("Skipping auth - Method: ALL | Path: %s", path)
+		routes = append(routes, allowedRoute{
+			method:    "",
+			pathRegex: compiledRegex,
+		})
+	}
+
+	for _, methodPath := range opts.SkipAuthRoutes {
+		var (
+			method string
+			path   string
+		)
+
+		parts := strings.SplitN(methodPath, "=", 2)
+		if len(parts) == 1 {
+			method = ""
+			path = parts[0]
+		} else {
+			method = strings.ToUpper(parts[0])
+			path = parts[1]
+		}
+
+		compiledRegex, err := regexp.Compile(path)
+		if err != nil {
+			return nil, err
+		}
+		logger.Printf("Skipping auth - Method: %s | Path: %s", method, path)
+		routes = append(routes, allowedRoute{
+			method:    method,
+			pathRegex: compiledRegex,
+		})
+	}
+
+	return routes, nil
+}
+
 // GetRedirectURI returns the redirectURL that the upstream OAuth Provider will
 // redirect clients to once authenticated
 func (p *OAuthProxy) GetRedirectURI(host string) string {
@@ -584,16 +636,16 @@ func (p *OAuthProxy) IsValidRedirect(redirect string) bool {
 	}
 }
 
-// IsWhitelistedRequest is used to check if auth should be skipped for this request
+// IsAllowedRequest is used to check if auth should be skipped for this request
-func (p *OAuthProxy) IsWhitelistedRequest(req *http.Request) bool {
+func (p *OAuthProxy) IsAllowedRequest(req *http.Request) bool {
 	isPreflightRequestAllowed := p.skipAuthPreflight && req.Method == "OPTIONS"
-	return isPreflightRequestAllowed || p.IsWhitelistedPath(req.URL.Path) || p.IsTrustedIP(req)
+	return isPreflightRequestAllowed || p.isAllowedRoute(req) || p.IsTrustedIP(req)
 }
 
-// IsWhitelistedPath is used to check if the request path is allowed without auth
+// IsAllowedRoute is used to check if the request method & path is allowed without auth
-func (p *OAuthProxy) IsWhitelistedPath(path string) bool {
+func (p *OAuthProxy) isAllowedRoute(req *http.Request) bool {
-	for _, u := range p.compiledRegex {
+	for _, route := range p.allowedRoutes {
-		if u.MatchString(path) {
+		if (route.method == "" || req.Method == route.method) && route.pathRegex.MatchString(req.URL.Path) {
 			return true
 		}
 	}
@@ -643,7 +695,7 @@ func (p *OAuthProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 	switch path := req.URL.Path; {
 	case path == p.RobotsPath:
 		p.RobotsTxt(rw)
-	case p.IsWhitelistedRequest(req):
+	case p.IsAllowedRequest(req):
 		p.SkipAuthProxy(rw, req)
 	case path == p.SignInPath:
 		p.SignIn(rw, req)
@@ -831,7 +883,7 @@ func (p *OAuthProxy) AuthenticateOnly(rw http.ResponseWriter, req *http.Request)
 	rw.WriteHeader(http.StatusAccepted)
 }
 
-// SkipAuthProxy proxies whitelisted requests and skips authentication
+// SkipAuthProxy proxies allowlisted requests and skips authentication
 func (p *OAuthProxy) SkipAuthProxy(rw http.ResponseWriter, req *http.Request) {
 	if p.skipAuthStripHeaders {
 		p.stripAuthHeaders(req)
@@ -1026,7 +1078,7 @@ func (p *OAuthProxy) addHeadersForProxying(rw http.ResponseWriter, req *http.Req
 	}
 }
 
-// stripAuthHeaders removes Auth headers for whitelisted routes from skipAuthRegex
+// stripAuthHeaders removes Auth headers for allowlisted routes from skipAuthRegex
 func (p *OAuthProxy) stripAuthHeaders(req *http.Request) {
 	if p.PassBasicAuth {
 		req.Header.Del("X-Forwarded-User")

oauthproxy_test.go

@@ -1482,28 +1482,28 @@ func TestAuthOnlyEndpointSetBasicAuthFalseRequestHeaders(t *testing.T) {
 }
 
 func TestAuthSkippedForPreflightRequests(t *testing.T) {
-	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	upstreamServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(200)
 		_, err := w.Write([]byte("response"))
 		if err != nil {
 			t.Fatal(err)
 		}
 	}))
-	t.Cleanup(upstream.Close)
+	t.Cleanup(upstreamServer.Close)
 
 	opts := baseTestOptions()
 	opts.UpstreamServers = options.Upstreams{
 		{
-			ID:   upstream.URL,
+			ID:   upstreamServer.URL,
 			Path: "/",
-			URI:  upstream.URL,
+			URI:  upstreamServer.URL,
 		},
 	}
 	opts.SkipAuthPreflight = true
 	err := validation.Validate(opts)
 	assert.NoError(t, err)
 
-	upstreamURL, _ := url.Parse(upstream.URL)
+	upstreamURL, _ := url.Parse(upstreamServer.URL)
 	opts.SetProvider(NewTestProvider(upstreamURL, ""))
 
 	proxy, err := NewOAuthProxy(opts, func(string) bool { return false })
@@ -1561,17 +1561,17 @@ func NewSignatureTest() (*SignatureTest, error) {
 	opts.EmailDomains = []string{"acm.org"}
 
 	authenticator := &SignatureAuthenticator{}
-	upstream := httptest.NewServer(
+	upstreamServer := httptest.NewServer(
 		http.HandlerFunc(authenticator.Authenticate))
-	upstreamURL, err := url.Parse(upstream.URL)
+	upstreamURL, err := url.Parse(upstreamServer.URL)
 	if err != nil {
 		return nil, err
 	}
 	opts.UpstreamServers = options.Upstreams{
 		{
-			ID:   upstream.URL,
+			ID:   upstreamServer.URL,
 			Path: "/",
-			URI:  upstream.URL,
+			URI:  upstreamServer.URL,
 		},
 	}
 
@@ -1590,7 +1590,7 @@ func NewSignatureTest() (*SignatureTest, error) {
 
 	return &SignatureTest{
 		opts,
-		upstream,
+		upstreamServer,
 		upstreamURL.Host,
 		provider,
 		make(http.Header),
@@ -1974,20 +1974,20 @@ func Test_prepareNoCache(t *testing.T) {
 }
 
 func Test_noCacheHeaders(t *testing.T) {
-	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	upstreamServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		_, err := w.Write([]byte("upstream"))
 		if err != nil {
 			t.Error(err)
 		}
 	}))
-	t.Cleanup(upstream.Close)
+	t.Cleanup(upstreamServer.Close)
 
 	opts := baseTestOptions()
 	opts.UpstreamServers = options.Upstreams{
 		{
-			ID:   upstream.URL,
+			ID:   upstreamServer.URL,
 			Path: "/",
-			URI:  upstream.URL,
+			URI:  upstreamServer.URL,
 		},
 	}
 	opts.SkipAuthRegex = []string{".*"}
@@ -2224,7 +2224,8 @@ func TestTrustedIPs(t *testing.T) {
 			opts.TrustedIPs = tt.trustedIPs
 			opts.ReverseProxy = tt.reverseProxy
 			opts.RealClientIPHeader = tt.realClientIPHeader
-			validation.Validate(opts)
+			err := validation.Validate(opts)
+			assert.NoError(t, err)
 
 			proxy, err := NewOAuthProxy(opts, func(string) bool { return true })
 			assert.NoError(t, err)
@@ -2240,6 +2241,255 @@ func TestTrustedIPs(t *testing.T) {
 	}
 }
 
+func Test_buildRoutesAllowlist(t *testing.T) {
+	type expectedAllowedRoute struct {
+		method      string
+		regexString string
+	}
+
+	testCases := []struct {
+		name           string
+		skipAuthRegex  []string
+		skipAuthRoutes []string
+		expectedRoutes []expectedAllowedRoute
+		shouldError    bool
+	}{
+		{
+			name:           "No skip auth configured",
+			skipAuthRegex:  []string{},
+			skipAuthRoutes: []string{},
+			expectedRoutes: []expectedAllowedRoute{},
+			shouldError:    false,
+		},
+		{
+			name: "Only skipAuthRegex configured",
+			skipAuthRegex: []string{
+				"^/foo/bar",
+				"^/baz/[0-9]+/thing",
+			},
+			skipAuthRoutes: []string{},
+			expectedRoutes: []expectedAllowedRoute{
+				{
+					method:      "",
+					regexString: "^/foo/bar",
+				},
+				{
+					method:      "",
+					regexString: "^/baz/[0-9]+/thing",
+				},
+			},
+			shouldError: false,
+		},
+		{
+			name:          "Only skipAuthRoutes configured",
+			skipAuthRegex: []string{},
+			skipAuthRoutes: []string{
+				"GET=^/foo/bar",
+				"POST=^/baz/[0-9]+/thing",
+				"^/all/methods$",
+				"WEIRD=^/methods/are/allowed",
+				"PATCH=/second/equals?are=handled&just=fine",
+			},
+			expectedRoutes: []expectedAllowedRoute{
+				{
+					method:      "GET",
+					regexString: "^/foo/bar",
+				},
+				{
+					method:      "POST",
+					regexString: "^/baz/[0-9]+/thing",
+				},
+				{
+					method:      "",
+					regexString: "^/all/methods$",
+				},
+				{
+					method:      "WEIRD",
+					regexString: "^/methods/are/allowed",
+				},
+				{
+					method:      "PATCH",
+					regexString: "/second/equals?are=handled&just=fine",
+				},
+			},
+			shouldError: false,
+		},
+		{
+			name: "Both skipAuthRegexes and skipAuthRoutes configured",
+			skipAuthRegex: []string{
+				"^/foo/bar/regex",
+				"^/baz/[0-9]+/thing/regex",
+			},
+			skipAuthRoutes: []string{
+				"GET=^/foo/bar",
+				"POST=^/baz/[0-9]+/thing",
+				"^/all/methods$",
+			},
+			expectedRoutes: []expectedAllowedRoute{
+				{
+					method:      "",
+					regexString: "^/foo/bar/regex",
+				},
+				{
+					method:      "",
+					regexString: "^/baz/[0-9]+/thing/regex",
+				},
+				{
+					method:      "GET",
+					regexString: "^/foo/bar",
+				},
+				{
+					method:      "POST",
+					regexString: "^/baz/[0-9]+/thing",
+				},
+				{
+					method:      "",
+					regexString: "^/all/methods$",
+				},
+			},
+			shouldError: false,
+		},
+		{
+			name: "Invalid skipAuthRegex entry",
+			skipAuthRegex: []string{
+				"^/foo/bar",
+				"^/baz/[0-9]+/thing",
+				"(bad[regex",
+			},
+			skipAuthRoutes: []string{},
+			expectedRoutes: []expectedAllowedRoute{},
+			shouldError:    true,
+		},
+		{
+			name:          "Invalid skipAuthRoutes entry",
+			skipAuthRegex: []string{},
+			skipAuthRoutes: []string{
+				"GET=^/foo/bar",
+				"POST=^/baz/[0-9]+/thing",
+				"^/all/methods$",
+				"PUT=(bad[regex",
+			},
+			expectedRoutes: []expectedAllowedRoute{},
+			shouldError:    true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			opts := &options.Options{
+				SkipAuthRegex:  tc.skipAuthRegex,
+				SkipAuthRoutes: tc.skipAuthRoutes,
+			}
+			routes, err := buildRoutesAllowlist(opts)
+			if tc.shouldError {
+				assert.Error(t, err)
+				return
+			}
+			assert.NoError(t, err)
+
+			for i, route := range routes {
+				assert.Greater(t, len(tc.expectedRoutes), i)
+				assert.Equal(t, route.method, tc.expectedRoutes[i].method)
+				assert.Equal(t, route.pathRegex.String(), tc.expectedRoutes[i].regexString)
+			}
+		})
+	}
+}
+
+func TestAllowedRequest(t *testing.T) {
+	upstreamServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(200)
+		_, err := w.Write([]byte("Allowed Request"))
+		if err != nil {
+			t.Fatal(err)
+		}
+	}))
+	t.Cleanup(upstreamServer.Close)
+
+	opts := baseTestOptions()
+	opts.UpstreamServers = options.Upstreams{
+		{
+			ID:   upstreamServer.URL,
+			Path: "/",
+			URI:  upstreamServer.URL,
+		},
+	}
+	opts.SkipAuthRegex = []string{
+		"^/skip/auth/regex$",
+	}
+	opts.SkipAuthRoutes = []string{
+		"GET=^/skip/auth/routes/get",
+	}
+	err := validation.Validate(opts)
+	assert.NoError(t, err)
+	proxy, err := NewOAuthProxy(opts, func(_ string) bool { return true })
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	testCases := []struct {
+		name    string
+		method  string
+		url     string
+		allowed bool
+	}{
+		{
+			name:    "Regex GET allowed",
+			method:  "GET",
+			url:     "/skip/auth/regex",
+			allowed: true,
+		},
+		{
+			name:    "Regex POST allowed ",
+			method:  "POST",
+			url:     "/skip/auth/regex",
+			allowed: true,
+		},
+		{
+			name:    "Regex denied",
+			method:  "GET",
+			url:     "/wrong/denied",
+			allowed: false,
+		},
+		{
+			name:    "Route allowed",
+			method:  "GET",
+			url:     "/skip/auth/routes/get",
+			allowed: true,
+		},
+		{
+			name:    "Route denied with wrong method",
+			method:  "PATCH",
+			url:     "/skip/auth/routes/get",
+			allowed: false,
+		},
+		{
+			name:    "Route denied with wrong path",
+			method:  "GET",
+			url:     "/skip/auth/routes/wrong/path",
+			allowed: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			req, err := http.NewRequest(tc.method, tc.url, nil)
+			assert.NoError(t, err)
+			assert.Equal(t, tc.allowed, proxy.isAllowedRoute(req))
+
+			rw := httptest.NewRecorder()
+			proxy.ServeHTTP(rw, req)
+
+			if tc.allowed {
+				assert.Equal(t, 200, rw.Code)
+				assert.Equal(t, "Allowed Request", rw.Body.String())
+			} else {
+				assert.Equal(t, 403, rw.Code)
+			}
+		})
+	}
+}
+
 func TestProxyAllowedGroups(t *testing.T) {
 	tests := []struct {
 		name               string
@@ -2265,18 +2515,18 @@ func TestProxyAllowedGroups(t *testing.T) {
 				CreatedAt:   &created,
 			}
 
-			upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			upstreamServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(200)
 			}))
-			t.Cleanup(upstream.Close)
+			t.Cleanup(upstreamServer.Close)
 
 			test, err := NewProcessCookieTestWithOptionsModifiers(func(opts *options.Options) {
 				opts.AllowedGroups = tt.allowedGroups
 				opts.UpstreamServers = options.Upstreams{
 					{
-						ID:   upstream.URL,
+						ID:   upstreamServer.URL,
 						Path: "/",
-						URI:  upstream.URL,
+						URI:  upstreamServer.URL,
 					},
 				}
 			})
@@ -2287,7 +2537,8 @@ func TestProxyAllowedGroups(t *testing.T) {
 			test.req, _ = http.NewRequest("GET", "/", nil)
 
 			test.req.Header.Add("accept", applicationJSON)
-			test.SaveSession(session)
+			err = test.SaveSession(session)
+			assert.NoError(t, err)
 			test.proxy.ServeHTTP(test.rw, test.req)
 
 			if tt.expectUnauthorized {

pkg/apis/options/options.go

@@ -3,7 +3,6 @@ package options
 import (
 	"crypto"
 	"net/url"
-	"regexp"
 
 	oidc "github.com/coreos/go-oidc"
 	ipapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/ip"
@@ -67,6 +66,7 @@ type Options struct {
 	UpstreamServers Upstreams `cfg:",internal"`
 
 	SkipAuthRegex         []string `flag:"skip-auth-regex" cfg:"skip_auth_regex"`
+	SkipAuthRoutes        []string `flag:"skip-auth-route" cfg:"skip_auth_routes"`
 	SkipAuthStripHeaders  bool     `flag:"skip-auth-strip-headers" cfg:"skip_auth_strip_headers"`
 	SkipJwtBearerTokens   bool     `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens"`
 	ExtraJwtIssuers       []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers"`
@@ -114,7 +114,6 @@ type Options struct {
 
 	// internal values that are set after config validation
 	redirectURL        *url.URL
-	compiledRegex      []*regexp.Regexp
 	provider           providers.Provider
 	signatureData      *SignatureData
 	oidcVerifier       *oidc.IDTokenVerifier
@@ -124,7 +123,6 @@ type Options struct {
 
 // Options for Getting internal values
 func (o *Options) GetRedirectURL() *url.URL                        { return o.redirectURL }
-func (o *Options) GetCompiledRegex() []*regexp.Regexp              { return o.compiledRegex }
 func (o *Options) GetProvider() providers.Provider                 { return o.provider }
 func (o *Options) GetSignatureData() *SignatureData                { return o.signatureData }
 func (o *Options) GetOIDCVerifier() *oidc.IDTokenVerifier          { return o.oidcVerifier }
@@ -133,7 +131,6 @@ func (o *Options) GetRealClientIPParser() ipapi.RealClientIPParser { return o.re
 
 // Options for Setting internal values
 func (o *Options) SetRedirectURL(s *url.URL)                        { o.redirectURL = s }
-func (o *Options) SetCompiledRegex(s []*regexp.Regexp)              { o.compiledRegex = s }
 func (o *Options) SetProvider(s providers.Provider)                 { o.provider = s }
 func (o *Options) SetSignatureData(s *SignatureData)                { o.signatureData = s }
 func (o *Options) SetOIDCVerifier(s *oidc.IDTokenVerifier)          { o.oidcVerifier = s }
@@ -195,8 +192,9 @@ func NewFlagSet() *pflag.FlagSet {
 	flagSet.Bool("pass-access-token", false, "pass OAuth access_token to upstream via X-Forwarded-Access-Token header")
 	flagSet.Bool("pass-authorization-header", false, "pass the Authorization Header to upstream")
 	flagSet.Bool("set-authorization-header", false, "set Authorization response headers (useful in Nginx auth_request mode)")
-	flagSet.StringSlice("skip-auth-regex", []string{}, "bypass authentication for requests path's that match (may be given multiple times)")
+	flagSet.StringSlice("skip-auth-regex", []string{}, "(DEPRECATED for --skip-auth-route) bypass authentication for requests path's that match (may be given multiple times)")
-	flagSet.Bool("skip-auth-strip-headers", false, "strips X-Forwarded-* style authentication headers & Authorization header if they would be set by oauth2-proxy for request paths in --skip-auth-regex")
+	flagSet.StringSlice("skip-auth-route", []string{}, "bypass authentication for requests that match the method & path. Format: method=path_regex OR path_regex alone for all methods")
+	flagSet.Bool("skip-auth-strip-headers", false, "strips `X-Forwarded-*` style authentication headers & `Authorization` header if they would be set by oauth2-proxy for allowlisted requests (`--skip-auth-route`, `--skip-auth-regex`, `--skip-auth-preflight`, `--trusted-ip`)")
 	flagSet.Bool("skip-provider-button", false, "will skip sign-in-page to directly reach the next step: oauth/start")
 	flagSet.Bool("skip-auth-preflight", false, "will skip authentication for OPTIONS requests")
 	flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS providers")

pkg/validation/allowlist.go

@@ -0,0 +1,70 @@
+package validation
+
+import (
+	"fmt"
+	"os"
+	"regexp"
+	"strings"
+
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/ip"
+)
+
+func validateAllowlists(o *options.Options) []string {
+	msgs := []string{}
+
+	msgs = append(msgs, validateRoutes(o)...)
+	msgs = append(msgs, validateRegexes(o)...)
+	msgs = append(msgs, validateTrustedIPs(o)...)
+
+	if len(o.TrustedIPs) > 0 && o.ReverseProxy {
+		_, err := fmt.Fprintln(os.Stderr, "WARNING: mixing --trusted-ip with --reverse-proxy is a potential security vulnerability. An attacker can inject a trusted IP into an X-Real-IP or X-Forwarded-For header if they aren't properly protected outside of oauth2-proxy")
+		if err != nil {
+			panic(err)
+		}
+	}
+
+	return msgs
+}
+
+// validateRoutes validates method=path routes passed with options.SkipAuthRoutes
+func validateRoutes(o *options.Options) []string {
+	msgs := []string{}
+	for _, route := range o.SkipAuthRoutes {
+		var regex string
+		parts := strings.SplitN(route, "=", 2)
+		if len(parts) == 1 {
+			regex = parts[0]
+		} else {
+			regex = parts[1]
+		}
+		_, err := regexp.Compile(regex)
+		if err != nil {
+			msgs = append(msgs, fmt.Sprintf("error compiling regex /%s/: %v", regex, err))
+		}
+	}
+	return msgs
+}
+
+// validateRegex validates regex paths passed with options.SkipAuthRegex
+func validateRegexes(o *options.Options) []string {
+	msgs := []string{}
+	for _, regex := range o.SkipAuthRegex {
+		_, err := regexp.Compile(regex)
+		if err != nil {
+			msgs = append(msgs, fmt.Sprintf("error compiling regex /%s/: %v", regex, err))
+		}
+	}
+	return msgs
+}
+
+// validateTrustedIPs validates IP/CIDRs for IP based allowlists
+func validateTrustedIPs(o *options.Options) []string {
+	msgs := []string{}
+	for i, ipStr := range o.TrustedIPs {
+		if nil == ip.ParseIPNet(ipStr) {
+			msgs = append(msgs, fmt.Sprintf("trusted_ips[%d] (%s) could not be recognized", i, ipStr))
+		}
+	}
+	return msgs
+}

pkg/validation/allowlist_test.go

@@ -0,0 +1,125 @@
+package validation
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/gomega"
+
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
+)
+
+var _ = Describe("Allowlist", func() {
+	type validateRoutesTableInput struct {
+		routes     []string
+		errStrings []string
+	}
+
+	type validateRegexesTableInput struct {
+		regexes    []string
+		errStrings []string
+	}
+
+	type validateTrustedIPsTableInput struct {
+		trustedIPs []string
+		errStrings []string
+	}
+
+	DescribeTable("validateRoutes",
+		func(r *validateRoutesTableInput) {
+			opts := &options.Options{
+				SkipAuthRoutes: r.routes,
+			}
+			Expect(validateRoutes(opts)).To(ConsistOf(r.errStrings))
+		},
+		Entry("Valid regex routes", &validateRoutesTableInput{
+			routes: []string{
+				"/foo",
+				"POST=/foo/bar",
+				"PUT=^/foo/bar$",
+				"DELETE=/crazy/(?:regex)?/[^/]+/stuff$",
+			},
+			errStrings: []string{},
+		}),
+		Entry("Bad regexes do not compile", &validateRoutesTableInput{
+			routes: []string{
+				"POST=/(foo",
+				"OPTIONS=/foo/bar)",
+				"GET=^]/foo/bar[$",
+				"GET=^]/foo/bar[$",
+			},
+			errStrings: []string{
+				"error compiling regex //(foo/: error parsing regexp: missing closing ): `/(foo`",
+				"error compiling regex //foo/bar)/: error parsing regexp: unexpected ): `/foo/bar)`",
+				"error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",
+				"error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",
+			},
+		}),
+	)
+
+	DescribeTable("validateRegexes",
+		func(r *validateRegexesTableInput) {
+			opts := &options.Options{
+				SkipAuthRegex: r.regexes,
+			}
+			Expect(validateRegexes(opts)).To(ConsistOf(r.errStrings))
+		},
+		Entry("Valid regex routes", &validateRegexesTableInput{
+			regexes: []string{
+				"/foo",
+				"/foo/bar",
+				"^/foo/bar$",
+				"/crazy/(?:regex)?/[^/]+/stuff$",
+			},
+			errStrings: []string{},
+		}),
+		Entry("Bad regexes do not compile", &validateRegexesTableInput{
+			regexes: []string{
+				"/(foo",
+				"/foo/bar)",
+				"^]/foo/bar[$",
+				"^]/foo/bar[$",
+			},
+			errStrings: []string{
+				"error compiling regex //(foo/: error parsing regexp: missing closing ): `/(foo`",
+				"error compiling regex //foo/bar)/: error parsing regexp: unexpected ): `/foo/bar)`",
+				"error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",
+				"error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",
+			},
+		}),
+	)
+
+	DescribeTable("validateTrustedIPs",
+		func(t *validateTrustedIPsTableInput) {
+			opts := &options.Options{
+				TrustedIPs: t.trustedIPs,
+			}
+			Expect(validateTrustedIPs(opts)).To(ConsistOf(t.errStrings))
+		},
+		Entry("Non-overlapping valid IPs", &validateTrustedIPsTableInput{
+			trustedIPs: []string{
+				"127.0.0.1",
+				"10.32.0.1/32",
+				"43.36.201.0/24",
+				"::1",
+				"2a12:105:ee7:9234:0:0:0:0/64",
+			},
+			errStrings: []string{},
+		}),
+		Entry("Overlapping valid IPs", &validateTrustedIPsTableInput{
+			trustedIPs: []string{
+				"135.180.78.199",
+				"135.180.78.199/32",
+				"d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4",
+				"d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4/128",
+			},
+			errStrings: []string{},
+		}),
+		Entry("Invalid IPs", &validateTrustedIPsTableInput{
+			trustedIPs: []string{"[::1]", "alkwlkbn/32"},
+			errStrings: []string{
+				"trusted_ips[0] ([::1]) could not be recognized",
+				"trusted_ips[1] (alkwlkbn/32) could not be recognized",
+			},
+		}),
+	)
+})

pkg/validation/options.go

@@ -9,7 +9,6 @@ import (
 	"net/http"
 	"net/url"
 	"os"
-	"regexp"
 	"strings"
 
 	"github.com/coreos/go-oidc"
@@ -184,15 +183,6 @@ func Validate(o *options.Options) error {
 	o.SetRedirectURL(redirectURL)
 
 	msgs = append(msgs, validateUpstreams(o.UpstreamServers)...)
-
-	for _, u := range o.SkipAuthRegex {
-		compiledRegex, err := regexp.Compile(u)
-		if err != nil {
-			msgs = append(msgs, fmt.Sprintf("error compiling regex=%q %s", u, err))
-			continue
-		}
-		o.SetCompiledRegex(append(o.GetCompiledRegex(), compiledRegex))
-	}
 	msgs = parseProviderInfo(o, msgs)
 
 	if len(o.GoogleGroups) > 0 || o.GoogleAdminEmail != "" || o.GoogleServiceAccountJSON != "" {
@@ -223,18 +213,8 @@ func Validate(o *options.Options) error {
 		})
 	}
 
-	if len(o.TrustedIPs) > 0 && o.ReverseProxy {
+	// Do this after ReverseProxy validation for TrustedIP coordinated checks
-		_, err := fmt.Fprintln(os.Stderr, "WARNING: trusting of IPs with --reverse-proxy poses risks if a header spoofing attack is possible.")
+	msgs = append(msgs, validateAllowlists(o)...)
-		if err != nil {
-			panic(err)
-		}
-	}
-
-	for i, ipStr := range o.TrustedIPs {
-		if nil == ip.ParseIPNet(ipStr) {
-			msgs = append(msgs, fmt.Sprintf("trusted_ips[%d] (%s) could not be recognized", i, ipStr))
-		}
-	}
 
 	if len(msgs) != 0 {
 		return fmt.Errorf("invalid configuration:\n  %s",

pkg/validation/options_test.go

@@ -2,7 +2,6 @@ package validation
 
 import (
 	"crypto"
-	"errors"
 	"io/ioutil"
 	"net/url"
 	"os"
@@ -78,12 +77,19 @@ func TestClientSecretFileOption(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed to create temp file: %v", err)
 	}
-	f.WriteString("testcase")
+	_, err = f.WriteString("testcase")
+	if err != nil {
+		t.Fatalf("failed to write to temp file: %v", err)
+	}
 	if err := f.Close(); err != nil {
 		t.Fatalf("failed to close temp file: %v", err)
 	}
 	clientSecretFileName := f.Name()
-	defer os.Remove(clientSecretFileName)
+	defer func(t *testing.T) {
+		if err := os.Remove(clientSecretFileName); err != nil {
+			t.Fatalf("failed to delete temp file: %v", err)
+		}
+	}(t)
 
 	o := options.NewOptions()
 	o.Cookie.Secret = cookieSecret
@@ -144,41 +150,6 @@ func TestRedirectURL(t *testing.T) {
 	assert.Equal(t, expected, o.GetRedirectURL())
 }
 
-func TestCompiledRegex(t *testing.T) {
-	o := testOptions()
-	regexps := []string{"/foo/.*", "/ba[rz]/quux"}
-	o.SkipAuthRegex = regexps
-	assert.Equal(t, nil, Validate(o))
-	actual := make([]string, 0)
-	for _, regex := range o.GetCompiledRegex() {
-		actual = append(actual, regex.String())
-	}
-	assert.Equal(t, regexps, actual)
-}
-
-func TestCompiledRegexError(t *testing.T) {
-	o := testOptions()
-	o.SkipAuthRegex = []string{"(foobaz", "barquux)"}
-	err := Validate(o)
-	assert.NotEqual(t, nil, err)
-
-	expected := errorMsg([]string{
-		"error compiling regex=\"(foobaz\" error parsing regexp: " +
-			"missing closing ): `(foobaz`",
-		"error compiling regex=\"barquux)\" error parsing regexp: " +
-			"unexpected ): `barquux)`"})
-	assert.Equal(t, expected, err.Error())
-
-	o.SkipAuthRegex = []string{"foobaz", "barquux)"}
-	err = Validate(o)
-	assert.NotEqual(t, nil, err)
-
-	expected = errorMsg([]string{
-		"error compiling regex=\"barquux)\" error parsing regexp: " +
-			"unexpected ): `barquux)`"})
-	assert.Equal(t, expected, err.Error())
-}
-
 func TestDefaultProviderApiSettings(t *testing.T) {
 	o := testOptions()
 	assert.Equal(t, nil, Validate(o))
@@ -337,45 +308,6 @@ func TestRealClientIPHeader(t *testing.T) {
 	assert.Nil(t, o.GetRealClientIPParser())
 }
 
-func TestIPCIDRSetOption(t *testing.T) {
-	tests := []struct {
-		name       string
-		trustedIPs []string
-		err        error
-	}{
-		{
-			"TestSomeIPs",
-			[]string{"127.0.0.1", "10.32.0.1/32", "43.36.201.0/24", "::1", "2a12:105:ee7:9234:0:0:0:0/64"},
-			nil,
-		}, {
-			"TestOverlappingIPs",
-			[]string{"135.180.78.199", "135.180.78.199/32", "d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4", "d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4/128"},
-			nil,
-		}, {
-			"TestInvalidIPs",
-			[]string{"[::1]", "alkwlkbn/32"},
-			errors.New(
-				"invalid configuration:\n" +
-					"  trusted_ips[0] ([::1]) could not be recognized\n" +
-					"  trusted_ips[1] (alkwlkbn/32) could not be recognized",
-			),
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			o := testOptions()
-			o.TrustedIPs = tt.trustedIPs
-			err := Validate(o)
-			if tt.err == nil {
-				assert.Nil(t, err)
-			} else {
-				assert.Equal(t, tt.err.Error(), err.Error())
-			}
-		})
-	}
-}
-
 func TestProviderCAFilesError(t *testing.T) {
 	file, err := ioutil.TempFile("", "absent.*.crt")
 	assert.NoError(t, err)