diff --git a/oauthproxy.go b/oauthproxy.go index ae5231f6..e64ffe91 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -76,7 +76,7 @@ type OAuthProxy struct { AuthOnlyPath string UserInfoPath string - allowedRoutes []*allowedRoute + allowedRoutes []allowedRoute redirectURL *url.URL // the url to receive requests at whitelistDomains []string provider providers.Provider @@ -285,8 +285,8 @@ func buildSignInMessage(opts *options.Options) string { // 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)) +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) @@ -294,7 +294,7 @@ func buildRoutesAllowlist(opts *options.Options) ([]*allowedRoute, error) { return nil, err } logger.Printf("Skipping auth - Method: ALL | Path: %s", path) - routes = append(routes, &allowedRoute{ + routes = append(routes, allowedRoute{ method: "", pathRegex: compiledRegex, }) @@ -306,13 +306,13 @@ func buildRoutesAllowlist(opts *options.Options) ([]*allowedRoute, error) { path string ) - parts := strings.Split(methodPath, "=") + parts := strings.SplitN(methodPath, "=", 2) if len(parts) == 1 { method = "" path = parts[0] } else { method = strings.ToUpper(parts[0]) - path = strings.Join(parts[1:], "=") + path = parts[1] } compiledRegex, err := regexp.Compile(path) @@ -320,7 +320,7 @@ func buildRoutesAllowlist(opts *options.Options) ([]*allowedRoute, error) { return nil, err } logger.Printf("Skipping auth - Method: %s | Path: %s", method, path) - routes = append(routes, &allowedRoute{ + routes = append(routes, allowedRoute{ method: method, pathRegex: compiledRegex, }) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index e42033d1..d0fd9481 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -2242,21 +2242,24 @@ 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 - expectedMethods []string - expectedRegexes []string - shouldError bool + name string + skipAuthRegex []string + skipAuthRoutes []string + expectedRoutes []expectedAllowedRoute + shouldError bool }{ { - name: "No skip auth configured", - skipAuthRegex: []string{}, - skipAuthRoutes: []string{}, - expectedMethods: []string{}, - expectedRegexes: []string{}, - shouldError: false, + name: "No skip auth configured", + skipAuthRegex: []string{}, + skipAuthRoutes: []string{}, + expectedRoutes: []expectedAllowedRoute{}, + shouldError: false, }, { name: "Only skipAuthRegex configured", @@ -2265,13 +2268,15 @@ func Test_buildRoutesAllowlist(t *testing.T) { "^/baz/[0-9]+/thing", }, skipAuthRoutes: []string{}, - expectedMethods: []string{ - "", - "", - }, - expectedRegexes: []string{ - "^/foo/bar", - "^/baz/[0-9]+/thing", + expectedRoutes: []expectedAllowedRoute{ + { + method: "", + regexString: "^/foo/bar", + }, + { + method: "", + regexString: "^/baz/[0-9]+/thing", + }, }, shouldError: false, }, @@ -2285,19 +2290,27 @@ func Test_buildRoutesAllowlist(t *testing.T) { "WEIRD=^/methods/are/allowed", "PATCH=/second/equals?are=handled&just=fine", }, - expectedMethods: []string{ - "GET", - "POST", - "", - "WEIRD", - "PATCH", - }, - expectedRegexes: []string{ - "^/foo/bar", - "^/baz/[0-9]+/thing", - "^/all/methods$", - "^/methods/are/allowed", - "/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, }, @@ -2312,19 +2325,27 @@ func Test_buildRoutesAllowlist(t *testing.T) { "POST=^/baz/[0-9]+/thing", "^/all/methods$", }, - expectedMethods: []string{ - "", - "", - "GET", - "POST", - "", - }, - expectedRegexes: []string{ - "^/foo/bar/regex", - "^/baz/[0-9]+/thing/regex", - "^/foo/bar", - "^/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, }, @@ -2335,10 +2356,9 @@ func Test_buildRoutesAllowlist(t *testing.T) { "^/baz/[0-9]+/thing", "(bad[regex", }, - skipAuthRoutes: []string{}, - expectedMethods: []string{}, - expectedRegexes: []string{}, - shouldError: true, + skipAuthRoutes: []string{}, + expectedRoutes: []expectedAllowedRoute{}, + shouldError: true, }, { name: "Invalid skipAuthRoutes entry", @@ -2349,9 +2369,8 @@ func Test_buildRoutesAllowlist(t *testing.T) { "^/all/methods$", "PUT=(bad[regex", }, - expectedMethods: []string{}, - expectedRegexes: []string{}, - shouldError: true, + expectedRoutes: []expectedAllowedRoute{}, + shouldError: true, }, } @@ -2369,10 +2388,9 @@ func Test_buildRoutesAllowlist(t *testing.T) { assert.NoError(t, err) for i, route := range routes { - assert.Greater(t, len(tc.expectedMethods), i) - assert.Equal(t, route.method, tc.expectedMethods[i]) - assert.Greater(t, len(tc.expectedRegexes), i) - assert.Equal(t, route.pathRegex.String(), tc.expectedRegexes[i]) + 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) } }) } diff --git a/pkg/validation/allowlist.go b/pkg/validation/allowlist.go index 254983d8..56a3fd4c 100644 --- a/pkg/validation/allowlist.go +++ b/pkg/validation/allowlist.go @@ -6,8 +6,8 @@ import ( "regexp" "strings" - "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options" - "github.com/oauth2-proxy/oauth2-proxy/pkg/ip" + "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 { @@ -32,11 +32,11 @@ func validateRoutes(o *options.Options) []string { msgs := []string{} for _, route := range o.SkipAuthRoutes { var regex string - parts := strings.Split(route, "=") + parts := strings.SplitN(route, "=", 2) if len(parts) == 1 { regex = parts[0] } else { - regex = strings.Join(parts[1:], "=") + regex = parts[1] } _, err := regexp.Compile(regex) if err != nil { diff --git a/pkg/validation/allowlist_test.go b/pkg/validation/allowlist_test.go index 1dd48288..4600a718 100644 --- a/pkg/validation/allowlist_test.go +++ b/pkg/validation/allowlist_test.go @@ -1,10 +1,11 @@ package validation import ( - "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options" . "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() {