git.iko.soy/typeable/oauth2-proxy
on master help sign in
Improve AllowedRoute test table formatting
commitb7b7ade7c4aaa95f2da051961c3bda6d7e8103d5 authorNick Meves <nick.meves@greenhouse.io> dateMon 5 Oct 2020, 12:39 (6 years ago) parent89a8ac8
4 files changed+87−68
compare with master
unifiedsplit
oauthproxy.go+7−7 oauthproxy_test.go+74−56 pkg/validation/allowlist.go+4−4 pkg/validation/allowlist_test.go+2−1
oauthproxy.go+7−7
@@ -76,7 +76,7 @@ type OAuthProxy struct {
7676 AuthOnlyPath string
7777 UserInfoPath string
7878
79− allowedRoutes []*allowedRoute
79+ allowedRoutes []allowedRoute
8080 redirectURL *url.URL // the url to receive requests at
8181 whitelistDomains []string
8282 provider providers.Provider
@@ -285,8 +285,8 @@ func buildSignInMessage(opts *options.Options) string {
285285// buildRoutesAllowlist builds an []allowedRoute list from either the legacy
286286// SkipAuthRegex option (paths only support) or newer SkipAuthRoutes option
287287// (method=path support)
288−func buildRoutesAllowlist(opts *options.Options) ([]*allowedRoute, error) {
289− routes := make([]*allowedRoute, 0, len(opts.SkipAuthRegex)+len(opts.SkipAuthRoutes))
288+func buildRoutesAllowlist(opts *options.Options) ([]allowedRoute, error) {
289+ routes := make([]allowedRoute, 0, len(opts.SkipAuthRegex)+len(opts.SkipAuthRoutes))
290290
291291 for _, path := range opts.SkipAuthRegex {
292292 compiledRegex, err := regexp.Compile(path)
@@ -294,7 +294,7 @@ func buildRoutesAllowlist(opts *options.Options) ([]*allowedRoute, error) {
294294 return nil, err
295295 }
296296 logger.Printf("Skipping auth - Method: ALL | Path: %s", path)
297− routes = append(routes, &allowedRoute{
297+ routes = append(routes, allowedRoute{
298298 method: "",
299299 pathRegex: compiledRegex,
300300 })
@@ -306,13 +306,13 @@ func buildRoutesAllowlist(opts *options.Options) ([]*allowedRoute, error) {
306306 path string
307307 )
308308
309− parts := strings.Split(methodPath, "=")
309+ parts := strings.SplitN(methodPath, "=", 2)
310310 if len(parts) == 1 {
311311 method = ""
312312 path = parts[0]
313313 } else {
314314 method = strings.ToUpper(parts[0])
315− path = strings.Join(parts[1:], "=")
315+ path = parts[1]
316316 }
317317
318318 compiledRegex, err := regexp.Compile(path)
@@ -320,7 +320,7 @@ func buildRoutesAllowlist(opts *options.Options) ([]*allowedRoute, error) {
320320 return nil, err
321321 }
322322 logger.Printf("Skipping auth - Method: %s | Path: %s", method, path)
323− routes = append(routes, &allowedRoute{
323+ routes = append(routes, allowedRoute{
324324 method: method,
325325 pathRegex: compiledRegex,
326326 })
oauthproxy_test.go+74−56
@@ -2242,21 +2242,24 @@ func TestTrustedIPs(t *testing.T) {
22422242}
22432243
22442244func Test_buildRoutesAllowlist(t *testing.T) {
2245+ type expectedAllowedRoute struct {
2246+ method string
2247+ regexString string
2248+ }
2249+
22452250 testCases := []struct {
2246− name string
2247− skipAuthRegex []string
2248− skipAuthRoutes []string
2249− expectedMethods []string
2250− expectedRegexes []string
2251− shouldError bool
2251+ name string
2252+ skipAuthRegex []string
2253+ skipAuthRoutes []string
2254+ expectedRoutes []expectedAllowedRoute
2255+ shouldError bool
22522256 }{
22532257 {
2254− name: "No skip auth configured",
2255− skipAuthRegex: []string{},
2256− skipAuthRoutes: []string{},
2257− expectedMethods: []string{},
2258− expectedRegexes: []string{},
2259− shouldError: false,
2258+ name: "No skip auth configured",
2259+ skipAuthRegex: []string{},
2260+ skipAuthRoutes: []string{},
2261+ expectedRoutes: []expectedAllowedRoute{},
2262+ shouldError: false,
22602263 },
22612264 {
22622265 name: "Only skipAuthRegex configured",
@@ -2265,13 +2268,15 @@ func Test_buildRoutesAllowlist(t *testing.T) {
22652268 "^/baz/[0-9]+/thing",
22662269 },
22672270 skipAuthRoutes: []string{},
2268− expectedMethods: []string{
2269− "",
2270− "",
2271− },
2272− expectedRegexes: []string{
2273− "^/foo/bar",
2274− "^/baz/[0-9]+/thing",
2271+ expectedRoutes: []expectedAllowedRoute{
2272+ {
2273+ method: "",
2274+ regexString: "^/foo/bar",
2275+ },
2276+ {
2277+ method: "",
2278+ regexString: "^/baz/[0-9]+/thing",
2279+ },
22752280 },
22762281 shouldError: false,
22772282 },
@@ -2285,19 +2290,27 @@ func Test_buildRoutesAllowlist(t *testing.T) {
22852290 "WEIRD=^/methods/are/allowed",
22862291 "PATCH=/second/equals?are=handled&just=fine",
22872292 },
2288− expectedMethods: []string{
2289− "GET",
2290− "POST",
2291− "",
2292− "WEIRD",
2293− "PATCH",
2294− },
2295− expectedRegexes: []string{
2296− "^/foo/bar",
2297− "^/baz/[0-9]+/thing",
2298− "^/all/methods$",
2299− "^/methods/are/allowed",
2300− "/second/equals?are=handled&just=fine",
2293+ expectedRoutes: []expectedAllowedRoute{
2294+ {
2295+ method: "GET",
2296+ regexString: "^/foo/bar",
2297+ },
2298+ {
2299+ method: "POST",
2300+ regexString: "^/baz/[0-9]+/thing",
2301+ },
2302+ {
2303+ method: "",
2304+ regexString: "^/all/methods$",
2305+ },
2306+ {
2307+ method: "WEIRD",
2308+ regexString: "^/methods/are/allowed",
2309+ },
2310+ {
2311+ method: "PATCH",
2312+ regexString: "/second/equals?are=handled&just=fine",
2313+ },
23012314 },
23022315 shouldError: false,
23032316 },
@@ -2312,19 +2325,27 @@ func Test_buildRoutesAllowlist(t *testing.T) {
23122325 "POST=^/baz/[0-9]+/thing",
23132326 "^/all/methods$",
23142327 },
2315− expectedMethods: []string{
2316− "",
2317− "",
2318− "GET",
2319− "POST",
2320− "",
2321− },
2322− expectedRegexes: []string{
2323− "^/foo/bar/regex",
2324− "^/baz/[0-9]+/thing/regex",
2325− "^/foo/bar",
2326− "^/baz/[0-9]+/thing",
2327− "^/all/methods$",
2328+ expectedRoutes: []expectedAllowedRoute{
2329+ {
2330+ method: "",
2331+ regexString: "^/foo/bar/regex",
2332+ },
2333+ {
2334+ method: "",
2335+ regexString: "^/baz/[0-9]+/thing/regex",
2336+ },
2337+ {
2338+ method: "GET",
2339+ regexString: "^/foo/bar",
2340+ },
2341+ {
2342+ method: "POST",
2343+ regexString: "^/baz/[0-9]+/thing",
2344+ },
2345+ {
2346+ method: "",
2347+ regexString: "^/all/methods$",
2348+ },
23282349 },
23292350 shouldError: false,
23302351 },
@@ -2335,10 +2356,9 @@ func Test_buildRoutesAllowlist(t *testing.T) {
23352356 "^/baz/[0-9]+/thing",
23362357 "(bad[regex",
23372358 },
2338− skipAuthRoutes: []string{},
2339− expectedMethods: []string{},
2340− expectedRegexes: []string{},
2341− shouldError: true,
2359+ skipAuthRoutes: []string{},
2360+ expectedRoutes: []expectedAllowedRoute{},
2361+ shouldError: true,
23422362 },
23432363 {
23442364 name: "Invalid skipAuthRoutes entry",
@@ -2349,9 +2369,8 @@ func Test_buildRoutesAllowlist(t *testing.T) {
23492369 "^/all/methods$",
23502370 "PUT=(bad[regex",
23512371 },
2352− expectedMethods: []string{},
2353− expectedRegexes: []string{},
2354− shouldError: true,
2372+ expectedRoutes: []expectedAllowedRoute{},
2373+ shouldError: true,
23552374 },
23562375 }
23572376
@@ -2369,10 +2388,9 @@ func Test_buildRoutesAllowlist(t *testing.T) {
23692388 assert.NoError(t, err)
23702389
23712390 for i, route := range routes {
2372− assert.Greater(t, len(tc.expectedMethods), i)
2373− assert.Equal(t, route.method, tc.expectedMethods[i])
2374− assert.Greater(t, len(tc.expectedRegexes), i)
2375− assert.Equal(t, route.pathRegex.String(), tc.expectedRegexes[i])
2391+ assert.Greater(t, len(tc.expectedRoutes), i)
2392+ assert.Equal(t, route.method, tc.expectedRoutes[i].method)
2393+ assert.Equal(t, route.pathRegex.String(), tc.expectedRoutes[i].regexString)
23762394 }
23772395 })
23782396 }
pkg/validation/allowlist.go+4−4
@@ -6,8 +6,8 @@ import (
66 "regexp"
77 "strings"
88
9− "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"
10− "github.com/oauth2-proxy/oauth2-proxy/pkg/ip"
9+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
10+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/ip"
1111)
1212
1313func validateAllowlists(o *options.Options) []string {
@@ -32,11 +32,11 @@ func validateRoutes(o *options.Options) []string {
3232 msgs := []string{}
3333 for _, route := range o.SkipAuthRoutes {
3434 var regex string
35− parts := strings.Split(route, "=")
35+ parts := strings.SplitN(route, "=", 2)
3636 if len(parts) == 1 {
3737 regex = parts[0]
3838 } else {
39− regex = strings.Join(parts[1:], "=")
39+ regex = parts[1]
4040 }
4141 _, err := regexp.Compile(regex)
4242 if err != nil {
pkg/validation/allowlist_test.go+2−1
@@ -1,10 +1,11 @@
11package validation
22
33import (
4− "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"
54 . "github.com/onsi/ginkgo"
65 . "github.com/onsi/ginkgo/extensions/table"
76 . "github.com/onsi/gomega"
7+
8+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
89)
910
1011var _ = Describe("Allowlist", func() {