git.iko.soy/typeable/oauth2-proxy
on master help sign in
Support negating for skip auth routes
commitf53754808be745af4dda8868eed4576cb8920546 authorIan Serpa <ian.serpa@imc.com> dateFri 19 Aug 2022, 12:46 (4 years ago) parent037cb04
5 files changed+250−5
CHANGELOG.md+2−1 docs/docs/configuration/overview.md+1−1 oauthproxy.go+19−2 oauthproxy_test.go+227 pkg/apis/options/options.go+1−1
CHANGELOG.md+2−1
@@ -26,7 +26,6 @@ N/A
2626
2727- [#1774](https://github.com/oauth2-proxy/oauth2-proxy/pull/1774) Fix vulnerabilities CVE-2022-27191, CVE-2021-44716 and CVE-2022-29526
2828
29−
3029- [#1667](https://github.com/oauth2-proxy/oauth2-proxy/issues/1667) Rename configuration file flag for PKCE
3130to remain consistent with CLI flags. You should specify `code_challenge_method` in your configuration instead of
3231`force_code_challenge_method`.
@@ -36,6 +35,8 @@ to remain consistent with CLI flags. You should specify `code_challenge_method`
3635 This feature allows parallel callbacks and by default it is disabled.
3736 - Add flag "--cookie-csrf-expire" to define a different expiration time for the CSRF cookie. By default, it is 15 minutes.
3837
38+- [#1762](https://github.com/oauth2-proxy/oauth2-proxy/pull/1762) Support negating for skip auth routes
39+
3940# V7.3.0
4041
4142## Release Highlights
docs/docs/configuration/overview.md+1−1
@@ -186,7 +186,7 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/
186186| `--silence-ping-logging` | bool | disable logging of requests to ping endpoint | false |
187187| `--skip-auth-preflight` | bool | will skip authentication for OPTIONS requests | false |
188188| `--skip-auth-regex` | string \| list | (DEPRECATED for `--skip-auth-route`) bypass authentication for requests paths that match (may be given multiple times) | |
189−| `--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 | |
189+| `--skip-auth-route` | string \| list | bypass authentication for requests that match the method & path. Format: method=path_regex OR method!=path_regex. For all methods: path_regex OR !=path_regex | |
190190| `--skip-auth-strip-headers` | bool | strips `X-Forwarded-*` style authentication headers & `Authorization` header if they would be set by oauth2-proxy | true |
191191| `--skip-jwt-bearer-tokens` | bool | will skip requests that have verified JWT bearer tokens (the token must have [`aud`](https://en.wikipedia.org/wiki/JSON_Web_Token#Standard_fields) that matches this client id or one of the extras from `extra-jwt-issuers`) | false |
192192| `--skip-oidc-discovery` | bool | bypass OIDC endpoint discovery. `--login-url`, `--redeem-url` and `--oidc-jwks-url` must be configured in this case | false |
oauthproxy.go+19−2
@@ -64,6 +64,7 @@ var (
6464// allowedRoute manages method + path based allowlists
6565type allowedRoute struct {
6666 method string
67+ negate bool
6768 pathRegex *regexp.Regexp
6869}
6970
@@ -445,9 +446,10 @@ func buildRoutesAllowlist(opts *options.Options) ([]allowedRoute, error) {
445446 var (
446447 method string
447448 path string
449+ negate = strings.Contains(methodPath, "!=")
448450 )
449451
450− parts := strings.SplitN(methodPath, "=", 2)
452+ parts := regexp.MustCompile("!?=").Split(methodPath, 2)
451453 if len(parts) == 1 {
452454 method = ""
453455 path = parts[0]
@@ -463,6 +465,7 @@ func buildRoutesAllowlist(opts *options.Options) ([]allowedRoute, error) {
463465 logger.Printf("Skipping auth - Method: %s | Path: %s", method, path)
464466 routes = append(routes, allowedRoute{
465467 method: method,
468+ negate: negate,
466469 pathRegex: compiledRegex,
467470 })
468471 }
@@ -516,10 +519,24 @@ func (p *OAuthProxy) IsAllowedRequest(req *http.Request) bool {
516519 return isPreflightRequestAllowed || p.isAllowedRoute(req) || p.isTrustedIP(req)
517520}
518521
522+func isAllowedMethod(req *http.Request, route allowedRoute) bool {
523+ return route.method == "" || req.Method == route.method
524+}
525+
526+func isAllowedPath(req *http.Request, route allowedRoute) bool {
527+ matches := route.pathRegex.MatchString(req.URL.Path)
528+
529+ if route.negate {
530+ return !matches
531+ }
532+
533+ return matches
534+}
535+
519536// IsAllowedRoute is used to check if the request method & path is allowed without auth
520537func (p *OAuthProxy) isAllowedRoute(req *http.Request) bool {
521538 for _, route := range p.allowedRoutes {
522− if (route.method == "" || req.Method == route.method) && route.pathRegex.MatchString(req.URL.Path) {
539+ if isAllowedMethod(req, route) && isAllowedPath(req, route) {
523540 return true
524541 }
525542 }
oauthproxy_test.go+227
@@ -2248,6 +2248,7 @@ func TestTrustedIPs(t *testing.T) {
22482248func Test_buildRoutesAllowlist(t *testing.T) {
22492249 type expectedAllowedRoute struct {
22502250 method string
2251+ negate bool
22512252 regexString string
22522253 }
22532254
@@ -2275,10 +2276,12 @@ func Test_buildRoutesAllowlist(t *testing.T) {
22752276 expectedRoutes: []expectedAllowedRoute{
22762277 {
22772278 method: "",
2279+ negate: false,
22782280 regexString: "^/foo/bar",
22792281 },
22802282 {
22812283 method: "",
2284+ negate: false,
22822285 regexString: "^/baz/[0-9]+/thing",
22832286 },
22842287 },
@@ -2293,28 +2296,45 @@ func Test_buildRoutesAllowlist(t *testing.T) {
22932296 "^/all/methods$",
22942297 "WEIRD=^/methods/are/allowed",
22952298 "PATCH=/second/equals?are=handled&just=fine",
2299+ "!=^/api",
2300+ "METHOD!=^/api",
22962301 },
22972302 expectedRoutes: []expectedAllowedRoute{
22982303 {
22992304 method: "GET",
2305+ negate: false,
23002306 regexString: "^/foo/bar",
23012307 },
23022308 {
23032309 method: "POST",
2310+ negate: false,
23042311 regexString: "^/baz/[0-9]+/thing",
23052312 },
23062313 {
23072314 method: "",
2315+ negate: false,
23082316 regexString: "^/all/methods$",
23092317 },
23102318 {
23112319 method: "WEIRD",
2320+ negate: false,
23122321 regexString: "^/methods/are/allowed",
23132322 },
23142323 {
23152324 method: "PATCH",
2325+ negate: false,
23162326 regexString: "/second/equals?are=handled&just=fine",
23172327 },
2328+ {
2329+ method: "",
2330+ negate: true,
2331+ regexString: "^/api",
2332+ },
2333+ {
2334+ method: "METHOD",
2335+ negate: true,
2336+ regexString: "^/api",
2337+ },
23182338 },
23192339 shouldError: false,
23202340 },
@@ -2394,6 +2414,7 @@ func Test_buildRoutesAllowlist(t *testing.T) {
23942414 for i, route := range routes {
23952415 assert.Greater(t, len(tc.expectedRoutes), i)
23962416 assert.Equal(t, route.method, tc.expectedRoutes[i].method)
2417+ assert.Equal(t, route.negate, tc.expectedRoutes[i].negate)
23972418 assert.Equal(t, route.pathRegex.String(), tc.expectedRoutes[i].regexString)
23982419 }
23992420 })
@@ -2475,6 +2496,212 @@ func TestAllowedRequest(t *testing.T) {
24752496 url: "/skip/auth/routes/wrong/path",
24762497 allowed: false,
24772498 },
2499+ {
2500+ name: "Route denied with wrong path and method",
2501+ method: "POST",
2502+ url: "/skip/auth/routes/wrong/path",
2503+ allowed: false,
2504+ },
2505+ }
2506+
2507+ for _, tc := range testCases {
2508+ t.Run(tc.name, func(t *testing.T) {
2509+ req, err := http.NewRequest(tc.method, tc.url, nil)
2510+ assert.NoError(t, err)
2511+ assert.Equal(t, tc.allowed, proxy.isAllowedRoute(req))
2512+
2513+ rw := httptest.NewRecorder()
2514+ proxy.ServeHTTP(rw, req)
2515+
2516+ if tc.allowed {
2517+ assert.Equal(t, 200, rw.Code)
2518+ assert.Equal(t, "Allowed Request", rw.Body.String())
2519+ } else {
2520+ assert.Equal(t, 403, rw.Code)
2521+ }
2522+ })
2523+ }
2524+}
2525+
2526+func TestAllowedRequestNegateWithoutMethod(t *testing.T) {
2527+ upstreamServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
2528+ w.WriteHeader(200)
2529+ _, err := w.Write([]byte("Allowed Request"))
2530+ if err != nil {
2531+ t.Fatal(err)
2532+ }
2533+ }))
2534+ t.Cleanup(upstreamServer.Close)
2535+
2536+ opts := baseTestOptions()
2537+ opts.UpstreamServers = options.UpstreamConfig{
2538+ Upstreams: []options.Upstream{
2539+ {
2540+ ID: upstreamServer.URL,
2541+ Path: "/",
2542+ URI: upstreamServer.URL,
2543+ },
2544+ },
2545+ }
2546+ opts.SkipAuthRoutes = []string{
2547+ "!=^/api", // any non-api routes
2548+ "POST=^/api/public-entity/?$",
2549+ }
2550+ err := validation.Validate(opts)
2551+ assert.NoError(t, err)
2552+ proxy, err := NewOAuthProxy(opts, func(_ string) bool { return true })
2553+ if err != nil {
2554+ t.Fatal(err)
2555+ }
2556+
2557+ testCases := []struct {
2558+ name string
2559+ method string
2560+ url string
2561+ allowed bool
2562+ }{
2563+ {
2564+ name: "Some static file allowed",
2565+ method: "GET",
2566+ url: "/static/file.txt",
2567+ allowed: true,
2568+ },
2569+ {
2570+ name: "POST to contact form allowed",
2571+ method: "POST",
2572+ url: "/contact",
2573+ allowed: true,
2574+ },
2575+ {
2576+ name: "Regex POST allowed",
2577+ method: "POST",
2578+ url: "/api/public-entity",
2579+ allowed: true,
2580+ },
2581+ {
2582+ name: "Regex POST with trailing slash allowed",
2583+ method: "POST",
2584+ url: "/api/public-entity/",
2585+ allowed: true,
2586+ },
2587+ {
2588+ name: "Regex GET api route denied",
2589+ method: "GET",
2590+ url: "/api/users",
2591+ allowed: false,
2592+ },
2593+ {
2594+ name: "Regex POST api route denied",
2595+ method: "POST",
2596+ url: "/api/users",
2597+ allowed: false,
2598+ },
2599+ {
2600+ name: "Regex DELETE api route denied",
2601+ method: "DELETE",
2602+ url: "/api/users/1",
2603+ allowed: false,
2604+ },
2605+ }
2606+
2607+ for _, tc := range testCases {
2608+ t.Run(tc.name, func(t *testing.T) {
2609+ req, err := http.NewRequest(tc.method, tc.url, nil)
2610+ assert.NoError(t, err)
2611+ assert.Equal(t, tc.allowed, proxy.isAllowedRoute(req))
2612+
2613+ rw := httptest.NewRecorder()
2614+ proxy.ServeHTTP(rw, req)
2615+
2616+ if tc.allowed {
2617+ assert.Equal(t, 200, rw.Code)
2618+ assert.Equal(t, "Allowed Request", rw.Body.String())
2619+ } else {
2620+ assert.Equal(t, 403, rw.Code)
2621+ }
2622+ })
2623+ }
2624+}
2625+
2626+func TestAllowedRequestNegateWithMethod(t *testing.T) {
2627+ upstreamServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
2628+ w.WriteHeader(200)
2629+ _, err := w.Write([]byte("Allowed Request"))
2630+ if err != nil {
2631+ t.Fatal(err)
2632+ }
2633+ }))
2634+ t.Cleanup(upstreamServer.Close)
2635+
2636+ opts := baseTestOptions()
2637+ opts.UpstreamServers = options.UpstreamConfig{
2638+ Upstreams: []options.Upstream{
2639+ {
2640+ ID: upstreamServer.URL,
2641+ Path: "/",
2642+ URI: upstreamServer.URL,
2643+ },
2644+ },
2645+ }
2646+ opts.SkipAuthRoutes = []string{
2647+ "GET!=^/api", // any non-api routes
2648+ "POST=^/api/public-entity/?$",
2649+ }
2650+ err := validation.Validate(opts)
2651+ assert.NoError(t, err)
2652+ proxy, err := NewOAuthProxy(opts, func(_ string) bool { return true })
2653+ if err != nil {
2654+ t.Fatal(err)
2655+ }
2656+
2657+ testCases := []struct {
2658+ name string
2659+ method string
2660+ url string
2661+ allowed bool
2662+ }{
2663+ {
2664+ name: "Some static file allowed",
2665+ method: "GET",
2666+ url: "/static/file.txt",
2667+ allowed: true,
2668+ },
2669+ {
2670+ name: "POST to contact form not allowed",
2671+ method: "POST",
2672+ url: "/contact",
2673+ allowed: false,
2674+ },
2675+ {
2676+ name: "Regex POST allowed",
2677+ method: "POST",
2678+ url: "/api/public-entity",
2679+ allowed: true,
2680+ },
2681+ {
2682+ name: "Regex POST with trailing slash allowed",
2683+ method: "POST",
2684+ url: "/api/public-entity/",
2685+ allowed: true,
2686+ },
2687+ {
2688+ name: "Regex GET api route denied",
2689+ method: "GET",
2690+ url: "/api/users",
2691+ allowed: false,
2692+ },
2693+ {
2694+ name: "Regex POST api route denied",
2695+ method: "POST",
2696+ url: "/api/users",
2697+ allowed: false,
2698+ },
2699+ {
2700+ name: "Regex DELETE api route denied",
2701+ method: "DELETE",
2702+ url: "/api/users/1",
2703+ allowed: false,
2704+ },
24782705 }
24792706
24802707 for _, tc := range testCases {
pkg/apis/options/options.go+1−1
@@ -115,7 +115,7 @@ func NewFlagSet() *pflag.FlagSet {
115115 flagSet.Bool("force-https", false, "force HTTPS redirect for HTTP requests")
116116 flagSet.String("redirect-url", "", "the OAuth Redirect URL. ie: \"https://internalapp.yourcompany.com/oauth2/callback\"")
117117 flagSet.StringSlice("skip-auth-regex", []string{}, "(DEPRECATED for --skip-auth-route) bypass authentication for requests path's that match (may be given multiple times)")
118− 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")
118+ flagSet.StringSlice("skip-auth-route", []string{}, "bypass authentication for requests that match the method & path. Format: method=path_regex OR method!=path_regex. For all methods: path_regex OR !=path_regex")
119119 flagSet.Bool("skip-provider-button", false, "will skip sign-in-page to directly reach the next step: oauth/start")
120120 flagSet.Bool("skip-auth-preflight", false, "will skip authentication for OPTIONS requests")
121121 flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS providers")