Improve AllowedRoute test table formatting

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) {
+func buildRoutesAllowlist(opts *options.Options) ([]allowedRoute, error) {
-	routes := make([]*allowedRoute, 0, len(opts.SkipAuthRegex)+len(opts.SkipAuthRoutes))
+	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,
 		})

oauthproxy_test.go

@@ -2242,20 +2242,23 @@ 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
+		expectedRoutes []expectedAllowedRoute
-		expectedRegexes []string
 		shouldError    bool
 	}{
 		{
 			name:           "No skip auth configured",
 			skipAuthRegex:  []string{},
 			skipAuthRoutes: []string{},
-			expectedMethods: []string{},
+			expectedRoutes: []expectedAllowedRoute{},
-			expectedRegexes: []string{},
 			shouldError:    false,
 		},
 		{
@@ -2265,13 +2268,15 @@ func Test_buildRoutesAllowlist(t *testing.T) {
 				"^/baz/[0-9]+/thing",
 			},
 			skipAuthRoutes: []string{},
-			expectedMethods: []string{
+			expectedRoutes: []expectedAllowedRoute{
-				"",
+				{
-				"",
+					method:      "",
+					regexString: "^/foo/bar",
+				},
+				{
+					method:      "",
+					regexString: "^/baz/[0-9]+/thing",
 				},
-			expectedRegexes: []string{
-				"^/foo/bar",
-				"^/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{
+			expectedRoutes: []expectedAllowedRoute{
-				"GET",
+				{
-				"POST",
+					method:      "GET",
-				"",
+					regexString: "^/foo/bar",
-				"WEIRD",
+				},
-				"PATCH",
+				{
+					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",
 				},
-			expectedRegexes: []string{
-				"^/foo/bar",
-				"^/baz/[0-9]+/thing",
-				"^/all/methods$",
-				"^/methods/are/allowed",
-				"/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{
+			expectedRoutes: []expectedAllowedRoute{
-				"",
+				{
-				"",
+					method:      "",
-				"GET",
+					regexString: "^/foo/bar/regex",
-				"POST",
+				},
-				"",
+				{
+					method:      "",
+					regexString: "^/baz/[0-9]+/thing/regex",
+				},
+				{
+					method:      "GET",
+					regexString: "^/foo/bar",
+				},
+				{
+					method:      "POST",
+					regexString: "^/baz/[0-9]+/thing",
+				},
+				{
+					method:      "",
+					regexString: "^/all/methods$",
 				},
-			expectedRegexes: []string{
-				"^/foo/bar/regex",
-				"^/baz/[0-9]+/thing/regex",
-				"^/foo/bar",
-				"^/baz/[0-9]+/thing",
-				"^/all/methods$",
 			},
 			shouldError: false,
 		},
@@ -2336,8 +2357,7 @@ func Test_buildRoutesAllowlist(t *testing.T) {
 				"(bad[regex",
 			},
 			skipAuthRoutes: []string{},
-			expectedMethods: []string{},
+			expectedRoutes: []expectedAllowedRoute{},
-			expectedRegexes: []string{},
 			shouldError:    true,
 		},
 		{
@@ -2349,8 +2369,7 @@ func Test_buildRoutesAllowlist(t *testing.T) {
 				"^/all/methods$",
 				"PUT=(bad[regex",
 			},
-			expectedMethods: []string{},
+			expectedRoutes: []expectedAllowedRoute{},
-			expectedRegexes: []string{},
 			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.Greater(t, len(tc.expectedRoutes), i)
-				assert.Equal(t, route.method, tc.expectedMethods[i])
+				assert.Equal(t, route.method, tc.expectedRoutes[i].method)
-				assert.Greater(t, len(tc.expectedRegexes), i)
+				assert.Equal(t, route.pathRegex.String(), tc.expectedRoutes[i].regexString)
-				assert.Equal(t, route.pathRegex.String(), tc.expectedRegexes[i])
 			}
 		})
 	}

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/v7/pkg/apis/options"
-	"github.com/oauth2-proxy/oauth2-proxy/pkg/ip"
+	"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 {

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() {