git.iko.soy/typeable/oauth2-proxy
on master help sign in
Improve AllowedRoute test table formatting
b7b7adeNick Meves
oauthproxy.go+7−7
compare with master
unifiedsplit
@@ -76,7 +76,7 @@ type OAuthProxy struct {
76 AuthOnlyPath string
76 AuthOnlyPath string
77 UserInfoPath string
77 UserInfoPath string
78
78
79− allowedRoutes []*allowedRoute
79+ allowedRoutes []allowedRoute
80 redirectURL *url.URL // the url to receive requests at
80 redirectURL *url.URL // the url to receive requests at
81 whitelistDomains []string
81 whitelistDomains []string
82 provider providers.Provider
82 provider providers.Provider
@@ -285,8 +285,8 @@ func buildSignInMessage(opts *options.Options) string {
285// buildRoutesAllowlist builds an []allowedRoute list from either the legacy
285// buildRoutesAllowlist builds an []allowedRoute list from either the legacy
286// SkipAuthRegex option (paths only support) or newer SkipAuthRoutes option
286// SkipAuthRegex option (paths only support) or newer SkipAuthRoutes option
287// (method=path support)
287// (method=path support)
288−func buildRoutesAllowlist(opts *options.Options) ([]*allowedRoute, error) {
288+func buildRoutesAllowlist(opts *options.Options) ([]allowedRoute, error) {
289− routes := make([]*allowedRoute, 0, len(opts.SkipAuthRegex)+len(opts.SkipAuthRoutes))
289+ routes := make([]allowedRoute, 0, len(opts.SkipAuthRegex)+len(opts.SkipAuthRoutes))
290
290
291 for _, path := range opts.SkipAuthRegex {
291 for _, path := range opts.SkipAuthRegex {
292 compiledRegex, err := regexp.Compile(path)
292 compiledRegex, err := regexp.Compile(path)
@@ -294,7 +294,7 @@ func buildRoutesAllowlist(opts *options.Options) ([]*allowedRoute, error) {
294 return nil, err
294 return nil, err
295 }
295 }
296 logger.Printf("Skipping auth - Method: ALL | Path: %s", path)
296 logger.Printf("Skipping auth - Method: ALL | Path: %s", path)
297− routes = append(routes, &allowedRoute{
297+ routes = append(routes, allowedRoute{
298 method: "",
298 method: "",
299 pathRegex: compiledRegex,
299 pathRegex: compiledRegex,
300 })
300 })
@@ -306,13 +306,13 @@ func buildRoutesAllowlist(opts *options.Options) ([]*allowedRoute, error) {
306 path string
306 path string
307 )
307 )
308
308
309− parts := strings.Split(methodPath, "=")
309+ parts := strings.SplitN(methodPath, "=", 2)
310 if len(parts) == 1 {
310 if len(parts) == 1 {
311 method = ""
311 method = ""
312 path = parts[0]
312 path = parts[0]
313 } else {
313 } else {
314 method = strings.ToUpper(parts[0])
314 method = strings.ToUpper(parts[0])
315− path = strings.Join(parts[1:], "=")
315+ path = parts[1]
316 }
316 }
317
317
318 compiledRegex, err := regexp.Compile(path)
318 compiledRegex, err := regexp.Compile(path)
@@ -320,7 +320,7 @@ func buildRoutesAllowlist(opts *options.Options) ([]*allowedRoute, error) {
320 return nil, err
320 return nil, err
321 }
321 }
322 logger.Printf("Skipping auth - Method: %s | Path: %s", method, path)
322 logger.Printf("Skipping auth - Method: %s | Path: %s", method, path)
323− routes = append(routes, &allowedRoute{
323+ routes = append(routes, allowedRoute{
324 method: method,
324 method: method,
325 pathRegex: compiledRegex,
325 pathRegex: compiledRegex,
326 })
326 })
oauthproxy_test.go+74−56
@@ -2242,21 +2242,24 @@ func TestTrustedIPs(t *testing.T) {
2242}
2242}
2243
2243
2244func Test_buildRoutesAllowlist(t *testing.T) {
2244func Test_buildRoutesAllowlist(t *testing.T) {
2245+ type expectedAllowedRoute struct {
2246+ method string
2247+ regexString string
2248+ }
2249+
2245 testCases := []struct {
2250 testCases := []struct {
2246− name string
2251+ name string
2247− skipAuthRegex []string
2252+ skipAuthRegex []string
2248− skipAuthRoutes []string
2253+ skipAuthRoutes []string
2249− expectedMethods []string
2254+ expectedRoutes []expectedAllowedRoute
2250− expectedRegexes []string
2251− shouldError bool
2255+ shouldError bool
2252 }{
2256 }{
2253 {
2257 {
2254− name: "No skip auth configured",
2258+ name: "No skip auth configured",
2255− skipAuthRegex: []string{},
2259+ skipAuthRegex: []string{},
2256− skipAuthRoutes: []string{},
2260+ skipAuthRoutes: []string{},
2257− expectedMethods: []string{},
2261+ expectedRoutes: []expectedAllowedRoute{},
2258− expectedRegexes: []string{},
2259− shouldError: false,
2262+ shouldError: false,
2260 },
2263 },
2261 {
2264 {
2262 name: "Only skipAuthRegex configured",
2265 name: "Only skipAuthRegex configured",
@@ -2265,13 +2268,15 @@ func Test_buildRoutesAllowlist(t *testing.T) {
2265 "^/baz/[0-9]+/thing",
2268 "^/baz/[0-9]+/thing",
2266 },
2269 },
2267 skipAuthRoutes: []string{},
2270 skipAuthRoutes: []string{},
2268− expectedMethods: []string{
2271+ expectedRoutes: []expectedAllowedRoute{
2269− "",
2272+ {
2270− "",
2273+ method: "",
2271− },
2272− expectedRegexes: []string{
2273− "^/foo/bar",
2274+ regexString: "^/foo/bar",
2275+ },
2276+ {
2277+ method: "",
2274− "^/baz/[0-9]+/thing",
2278+ regexString: "^/baz/[0-9]+/thing",
2279+ },
2275 },
2280 },
2276 shouldError: false,
2281 shouldError: false,
2277 },
2282 },
@@ -2285,19 +2290,27 @@ func Test_buildRoutesAllowlist(t *testing.T) {
2285 "WEIRD=^/methods/are/allowed",
2290 "WEIRD=^/methods/are/allowed",
2286 "PATCH=/second/equals?are=handled&just=fine",
2291 "PATCH=/second/equals?are=handled&just=fine",
2287 },
2292 },
2288− expectedMethods: []string{
2293+ expectedRoutes: []expectedAllowedRoute{
2294+ {
2289− "GET",
2295+ method: "GET",
2290− "POST",
2291− "",
2292− "WEIRD",
2293− "PATCH",
2294− },
2295− expectedRegexes: []string{
2296− "^/foo/bar",
2296+ regexString: "^/foo/bar",
2297+ },
2298+ {
2299+ method: "POST",
2297− "^/baz/[0-9]+/thing",
2300+ regexString: "^/baz/[0-9]+/thing",
2301+ },
2302+ {
2303+ method: "",
2298− "^/all/methods$",
2304+ regexString: "^/all/methods$",
2305+ },
2306+ {
2307+ method: "WEIRD",
2299− "^/methods/are/allowed",
2308+ regexString: "^/methods/are/allowed",
2309+ },
2310+ {
2311+ method: "PATCH",
2300− "/second/equals?are=handled&just=fine",
2312+ regexString: "/second/equals?are=handled&just=fine",
2313+ },
2301 },
2314 },
2302 shouldError: false,
2315 shouldError: false,
2303 },
2316 },
@@ -2312,19 +2325,27 @@ func Test_buildRoutesAllowlist(t *testing.T) {
2312 "POST=^/baz/[0-9]+/thing",
2325 "POST=^/baz/[0-9]+/thing",
2313 "^/all/methods$",
2326 "^/all/methods$",
2314 },
2327 },
2315− expectedMethods: []string{
2328+ expectedRoutes: []expectedAllowedRoute{
2316− "",
2329+ {
2317− "",
2330+ method: "",
2331+ regexString: "^/foo/bar/regex",
2332+ },
2333+ {
2334+ method: "",
2335+ regexString: "^/baz/[0-9]+/thing/regex",
2336+ },
2337+ {
2318− "GET",
2338+ method: "GET",
2339+ regexString: "^/foo/bar",
2340+ },
2341+ {
2319− "POST",
2342+ method: "POST",
2320− "",
2321− },
2322− expectedRegexes: []string{
2323− "^/foo/bar/regex",
2324− "^/baz/[0-9]+/thing/regex",
2343+ regexString: "^/baz/[0-9]+/thing",
2325− "^/foo/bar",
2344+ },
2326− "^/baz/[0-9]+/thing",
2345+ {
2346+ method: "",
2327− "^/all/methods$",
2347+ regexString: "^/all/methods$",
2348+ },
2328 },
2349 },
2329 shouldError: false,
2350 shouldError: false,
2330 },
2351 },
@@ -2335,10 +2356,9 @@ func Test_buildRoutesAllowlist(t *testing.T) {
2335 "^/baz/[0-9]+/thing",
2356 "^/baz/[0-9]+/thing",
2336 "(bad[regex",
2357 "(bad[regex",
2337 },
2358 },
2338− skipAuthRoutes: []string{},
2359+ skipAuthRoutes: []string{},
2339− expectedMethods: []string{},
2360+ expectedRoutes: []expectedAllowedRoute{},
2340− expectedRegexes: []string{},
2341− shouldError: true,
2361+ shouldError: true,
2342 },
2362 },
2343 {
2363 {
2344 name: "Invalid skipAuthRoutes entry",
2364 name: "Invalid skipAuthRoutes entry",
@@ -2349,9 +2369,8 @@ func Test_buildRoutesAllowlist(t *testing.T) {
2349 "^/all/methods$",
2369 "^/all/methods$",
2350 "PUT=(bad[regex",
2370 "PUT=(bad[regex",
2351 },
2371 },
2352− expectedMethods: []string{},
2372+ expectedRoutes: []expectedAllowedRoute{},
2353− expectedRegexes: []string{},
2354− shouldError: true,
2373+ shouldError: true,
2355 },
2374 },
2356 }
2375 }
2357
2376
@@ -2369,10 +2388,9 @@ func Test_buildRoutesAllowlist(t *testing.T) {
2369 assert.NoError(t, err)
2388 assert.NoError(t, err)
2370
2389
2371 for i, route := range routes {
2390 for i, route := range routes {
2372− assert.Greater(t, len(tc.expectedMethods), i)
2391+ assert.Greater(t, len(tc.expectedRoutes), i)
2373− assert.Equal(t, route.method, tc.expectedMethods[i])
2392+ assert.Equal(t, route.method, tc.expectedRoutes[i].method)
2374− assert.Greater(t, len(tc.expectedRegexes), i)
2375− assert.Equal(t, route.pathRegex.String(), tc.expectedRegexes[i])
2393+ assert.Equal(t, route.pathRegex.String(), tc.expectedRoutes[i].regexString)
2376 }
2394 }
2377 })
2395 })
2378 }
2396 }
pkg/validation/allowlist.go+4−4
@@ -6,8 +6,8 @@ import (
6 "regexp"
6 "regexp"
7 "strings"
7 "strings"
8
8
9− "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"
9+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
10− "github.com/oauth2-proxy/oauth2-proxy/pkg/ip"
10+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/ip"
11)
11)
12
12
13func validateAllowlists(o *options.Options) []string {
13func validateAllowlists(o *options.Options) []string {
@@ -32,11 +32,11 @@ func validateRoutes(o *options.Options) []string {
32 msgs := []string{}
32 msgs := []string{}
33 for _, route := range o.SkipAuthRoutes {
33 for _, route := range o.SkipAuthRoutes {
34 var regex string
34 var regex string
35− parts := strings.Split(route, "=")
35+ parts := strings.SplitN(route, "=", 2)
36 if len(parts) == 1 {
36 if len(parts) == 1 {
37 regex = parts[0]
37 regex = parts[0]
38 } else {
38 } else {
39− regex = strings.Join(parts[1:], "=")
39+ regex = parts[1]
40 }
40 }
41 _, err := regexp.Compile(regex)
41 _, err := regexp.Compile(regex)
42 if err != nil {
42 if err != nil {
pkg/validation/allowlist_test.go+2−1
@@ -1,10 +1,11 @@
1package validation
1package validation
2
2
3import (
3import (
4− "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"
5 . "github.com/onsi/ginkgo"
4 . "github.com/onsi/ginkgo"
6 . "github.com/onsi/ginkgo/extensions/table"
5 . "github.com/onsi/ginkgo/extensions/table"
7 . "github.com/onsi/gomega"
6 . "github.com/onsi/gomega"
7+
8+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
8)
9)
9
10
10var _ = Describe("Allowlist", func() {
11var _ = Describe("Allowlist", func() {