d8b0d90 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Merge pull request #789 from grnhse/skip-auth-http-method

Support HTTP method based allowlists

commitd8b0d9059da0ba7d20b60803b847be67c1466549 authorJoel Speed <Joel.speed@hotmail.co.uk> dateWed 7 Oct 2020, 18:23 (6 years ago) parentfcb83c4 b7b7ade

9 files changed+558−145

compare with master

unifiedsplit

CHANGELOG.md+4 docs/configuration/configuration.md+3−2 oauthproxy.go+70−18 oauthproxy_test.go+271−20 pkg/apis/options/options.go+4−6 pkg/validation/allowlist.go+70 pkg/validation/allowlist_test.go+125 pkg/validation/options.go+2−22 pkg/validation/options_test.go+9−77

CHANGELOG.md+4

@@ -4,6 +4,9 @@

55## Important Notes

7+- [#789](https://github.com/oauth2-proxy/oauth2-proxy/pull/789) `--skip-auth-route` is (almost) backwards compatible with `--skip-auth-regex`

8+ - We are marking `--skip-auth-regex` as DEPRECATED and will remove it in the next major version.

9+ - If your regex contains an `=` and you want it for all methods, you will need to add a leading `=` (this is the area where `--skip-auth-regex` doesn't port perfectly)

710- [#575](https://github.com/oauth2-proxy/oauth2-proxy/pull/575) Sessions from v5.1.1 or earlier will no longer validate since they were not signed with SHA1.

811 - Sessions from v6.0.0 or later had a graceful conversion to SHA256 that resulted in no reauthentication

912 - Upgrading from v5.1.1 or earlier will result in a reauthentication

@@ -23,6 +26,7 @@

2326## Changes since v6.1.1

2427

2528- [#753](https://github.com/oauth2-proxy/oauth2-proxy/pull/753) Pass resource parameter in login url (@codablock)

29+- [#789](https://github.com/oauth2-proxy/oauth2-proxy/pull/789) Add `--skip-auth-route` configuration option for `METHOD=pathRegex` based allowlists (@NickMeves)

2630- [#575](https://github.com/oauth2-proxy/oauth2-proxy/pull/575) Stop accepting legacy SHA1 signed cookies (@NickMeves)

2731- [#722](https://github.com/oauth2-proxy/oauth2-proxy/pull/722) Validate Redis configuration options at startup (@NickMeves)

2832- [#791](https://github.com/oauth2-proxy/oauth2-proxy/pull/791) Remove GetPreferredUsername method from provider interface (@NickMeves)

docs/configuration/configuration.md+3−2

@@ -119,8 +119,9 @@ An example [oauth2-proxy.cfg]({{ site.gitweb }}/contrib/oauth2-proxy.cfg.example

124+| `--skip-auth-strip-headers` | bool | strips `X-Forwarded-*` style authentication headers & `Authorization` header if they would be set by oauth2-proxy for allowlisted requests (`--skip-auth-route`, `--skip-auth-regex`, `--skip-auth-preflight`, `--trusted-ip`) | false |

oauthproxy.go+70−18

@@ -48,6 +48,12 @@ var (

4848 invalidRedirectRegex = regexp.MustCompile(`[/\\](?:[\s\v]*|\.{1,2})[/\\]`)

4949)

5050

51+// allowedRoute manages method + path based allowlists

52+type allowedRoute struct {

53+ method string

54+ pathRegex *regexp.Regexp

55+}

56+

5157// OAuthProxy is the main authentication proxy

5258type OAuthProxy struct {

5359 CookieSeed string

@@ -70,6 +76,7 @@ type OAuthProxy struct {

7076 AuthOnlyPath string

7177 UserInfoPath string

7278

79+ allowedRoutes []allowedRoute

7380 redirectURL *url.URL // the url to receive requests at

7481 whitelistDomains []string

7582 provider providers.Provider

@@ -90,13 +97,11 @@ type OAuthProxy struct {

9097 SetAuthorization bool

9198 PassAuthorization bool

9299 PreferEmailToUser bool

93− skipAuthRegex []string

94100 skipAuthPreflight bool

95101 skipAuthStripHeaders bool

96102 skipJwtBearerTokens bool

97103 mainJwtBearerVerifier *oidc.IDTokenVerifier

98104 extraJwtBearerVerifiers []*oidc.IDTokenVerifier

99− compiledRegex []*regexp.Regexp

100105 templates *template.Template

101106 realClientIPParser ipapi.RealClientIPParser

102107 trustedIPs *ip.NetSet

@@ -121,10 +126,6 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr

121126 return nil, fmt.Errorf("error initialising upstream proxy: %v", err)

122127 }

123128

124− for _, u := range opts.GetCompiledRegex() {

125− logger.Printf("compiled skip-auth-regex => %q", u)

126− }

127−

128129 if opts.SkipJwtBearerTokens {

129130 logger.Printf("Skipping JWT tokens from configured OIDC issuer: %q", opts.OIDCIssuerURL)

130131 for _, issuer := range opts.ExtraJwtIssuers {

@@ -163,6 +164,11 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr

163164 }

164165 }

165166

167+ allowedRoutes, err := buildRoutesAllowlist(opts)

168+ if err != nil {

169+ return nil, err

170+ }

171+

166172 sessionChain := buildSessionChain(opts, sessionStore, basicAuthValidator)

167173

168174 return &OAuthProxy{

@@ -192,14 +198,13 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr

192198 sessionStore: sessionStore,

193199 serveMux: upstreamProxy,

194200 redirectURL: redirectURL,

201+ allowedRoutes: allowedRoutes,

195202 whitelistDomains: opts.WhitelistDomains,

196− skipAuthRegex: opts.SkipAuthRegex,

197203 skipAuthPreflight: opts.SkipAuthPreflight,

198204 skipAuthStripHeaders: opts.SkipAuthStripHeaders,

199205 skipJwtBearerTokens: opts.SkipJwtBearerTokens,

200206 mainJwtBearerVerifier: opts.GetOIDCVerifier(),

201207 extraJwtBearerVerifiers: opts.GetJWTBearerVerifiers(),

202− compiledRegex: opts.GetCompiledRegex(),

203208 realClientIPParser: opts.GetRealClientIPParser(),

204209 SetXAuthRequest: opts.SetXAuthRequest,

205210 PassBasicAuth: opts.PassBasicAuth,

@@ -277,6 +282,53 @@ func buildSignInMessage(opts *options.Options) string {

277282 return msg

278283}

279284

285+// buildRoutesAllowlist builds an []allowedRoute list from either the legacy

286+// SkipAuthRegex option (paths only support) or newer SkipAuthRoutes option

287+// (method=path support)

288+func buildRoutesAllowlist(opts *options.Options) ([]allowedRoute, error) {

289+ routes := make([]allowedRoute, 0, len(opts.SkipAuthRegex)+len(opts.SkipAuthRoutes))

290+

291+ for _, path := range opts.SkipAuthRegex {

292+ compiledRegex, err := regexp.Compile(path)

293+ if err != nil {

294+ return nil, err

295+ }

296+ logger.Printf("Skipping auth - Method: ALL | Path: %s", path)

297+ routes = append(routes, allowedRoute{

298+ method: "",

299+ pathRegex: compiledRegex,

300+ })

301+ }

302+

303+ for _, methodPath := range opts.SkipAuthRoutes {

304+ var (

305+ method string

306+ path string

307+ )

308+

309+ parts := strings.SplitN(methodPath, "=", 2)

310+ if len(parts) == 1 {

311+ method = ""

312+ path = parts[0]

313+ } else {

314+ method = strings.ToUpper(parts[0])

315+ path = parts[1]

316+ }

317+

318+ compiledRegex, err := regexp.Compile(path)

319+ if err != nil {

320+ return nil, err

321+ }

322+ logger.Printf("Skipping auth - Method: %s | Path: %s", method, path)

323+ routes = append(routes, allowedRoute{

324+ method: method,

325+ pathRegex: compiledRegex,

326+ })

327+ }

328+

329+ return routes, nil

330+}

331+

280332// GetRedirectURI returns the redirectURL that the upstream OAuth Provider will

281333// redirect clients to once authenticated

282334func (p *OAuthProxy) GetRedirectURI(host string) string {

@@ -584,16 +636,16 @@ func (p *OAuthProxy) IsValidRedirect(redirect string) bool {

584636 }

585637}

586638

587−// IsWhitelistedRequest is used to check if auth should be skipped for this request

588−func (p *OAuthProxy) IsWhitelistedRequest(req *http.Request) bool {

639+// IsAllowedRequest is used to check if auth should be skipped for this request

640+func (p *OAuthProxy) IsAllowedRequest(req *http.Request) bool {

589641 isPreflightRequestAllowed := p.skipAuthPreflight && req.Method == "OPTIONS"

590− return isPreflightRequestAllowed || p.IsWhitelistedPath(req.URL.Path) || p.IsTrustedIP(req)

642+ return isPreflightRequestAllowed || p.isAllowedRoute(req) || p.IsTrustedIP(req)

591643}

592644

593−// IsWhitelistedPath is used to check if the request path is allowed without auth

594−func (p *OAuthProxy) IsWhitelistedPath(path string) bool {

595− for _, u := range p.compiledRegex {

596− if u.MatchString(path) {

645+// IsAllowedRoute is used to check if the request method & path is allowed without auth

646+func (p *OAuthProxy) isAllowedRoute(req *http.Request) bool {

647+ for _, route := range p.allowedRoutes {

648+ if (route.method == "" || req.Method == route.method) && route.pathRegex.MatchString(req.URL.Path) {

597649 return true

598650 }

599651 }

@@ -643,7 +695,7 @@ func (p *OAuthProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {

643695 switch path := req.URL.Path; {

644696 case path == p.RobotsPath:

645697 p.RobotsTxt(rw)

646− case p.IsWhitelistedRequest(req):

698+ case p.IsAllowedRequest(req):

647699 p.SkipAuthProxy(rw, req)

648700 case path == p.SignInPath:

649701 p.SignIn(rw, req)

@@ -831,7 +883,7 @@ func (p *OAuthProxy) AuthenticateOnly(rw http.ResponseWriter, req *http.Request)

831883 rw.WriteHeader(http.StatusAccepted)

832884}

833885

834−// SkipAuthProxy proxies whitelisted requests and skips authentication

886+// SkipAuthProxy proxies allowlisted requests and skips authentication

835887func (p *OAuthProxy) SkipAuthProxy(rw http.ResponseWriter, req *http.Request) {

836888 if p.skipAuthStripHeaders {

837889 p.stripAuthHeaders(req)

@@ -1026,7 +1078,7 @@ func (p *OAuthProxy) addHeadersForProxying(rw http.ResponseWriter, req *http.Req

10261078 }

10271079}

10281080

1029−// stripAuthHeaders removes Auth headers for whitelisted routes from skipAuthRegex

1081+// stripAuthHeaders removes Auth headers for allowlisted routes from skipAuthRegex

10301082func (p *OAuthProxy) stripAuthHeaders(req *http.Request) {

10311083 if p.PassBasicAuth {

10321084 req.Header.Del("X-Forwarded-User")

oauthproxy_test.go+271−20

@@ -1482,28 +1482,28 @@ func TestAuthOnlyEndpointSetBasicAuthFalseRequestHeaders(t *testing.T) {

14821482}

14831483

14841484func TestAuthSkippedForPreflightRequests(t *testing.T) {

1485− upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

1485+ upstreamServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

14861486 w.WriteHeader(200)

14871487 _, err := w.Write([]byte("response"))

14881488 if err != nil {

14891489 t.Fatal(err)

14901490 }

14911491 }))

1492− t.Cleanup(upstream.Close)

1492+ t.Cleanup(upstreamServer.Close)

14931493

14941494 opts := baseTestOptions()

14951495 opts.UpstreamServers = options.Upstreams{

14961496 {

1497− ID: upstream.URL,

1497+ ID: upstreamServer.URL,

14981498 Path: "/",

1499− URI: upstream.URL,

1499+ URI: upstreamServer.URL,

15001500 },

15011501 }

15021502 opts.SkipAuthPreflight = true

15031503 err := validation.Validate(opts)

15041504 assert.NoError(t, err)

15051505

1506− upstreamURL, _ := url.Parse(upstream.URL)

1506+ upstreamURL, _ := url.Parse(upstreamServer.URL)

15071507 opts.SetProvider(NewTestProvider(upstreamURL, ""))

15081508

15091509 proxy, err := NewOAuthProxy(opts, func(string) bool { return false })

@@ -1561,17 +1561,17 @@ func NewSignatureTest() (*SignatureTest, error) {

15611561 opts.EmailDomains = []string{"acm.org"}

15621562

15631563 authenticator := &SignatureAuthenticator{}

1564− upstream := httptest.NewServer(

1564+ upstreamServer := httptest.NewServer(

15651565 http.HandlerFunc(authenticator.Authenticate))

1566− upstreamURL, err := url.Parse(upstream.URL)

1566+ upstreamURL, err := url.Parse(upstreamServer.URL)

15671567 if err != nil {

15681568 return nil, err

15691569 }

15701570 opts.UpstreamServers = options.Upstreams{

15711571 {

1572− ID: upstream.URL,

1572+ ID: upstreamServer.URL,

15731573 Path: "/",

1574− URI: upstream.URL,

1574+ URI: upstreamServer.URL,

15751575 },

15761576 }

15771577

@@ -1590,7 +1590,7 @@ func NewSignatureTest() (*SignatureTest, error) {

15901590

15911591 return &SignatureTest{

15921592 opts,

1593− upstream,

1593+ upstreamServer,

15941594 upstreamURL.Host,

15951595 provider,

15961596 make(http.Header),

@@ -1974,20 +1974,20 @@ func Test_prepareNoCache(t *testing.T) {

19741974}

19751975

19761976func Test_noCacheHeaders(t *testing.T) {

1977− upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

1977+ upstreamServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

19781978 _, err := w.Write([]byte("upstream"))

19791979 if err != nil {

19801980 t.Error(err)

19811981 }

19821982 }))

1983− t.Cleanup(upstream.Close)

1983+ t.Cleanup(upstreamServer.Close)

19841984

19851985 opts := baseTestOptions()

19861986 opts.UpstreamServers = options.Upstreams{

19871987 {

1988− ID: upstream.URL,

1988+ ID: upstreamServer.URL,

19891989 Path: "/",

1990− URI: upstream.URL,

1990+ URI: upstreamServer.URL,

19911991 },

19921992 }

19931993 opts.SkipAuthRegex = []string{".*"}

@@ -2224,7 +2224,8 @@ func TestTrustedIPs(t *testing.T) {

22242224 opts.TrustedIPs = tt.trustedIPs

22252225 opts.ReverseProxy = tt.reverseProxy

22262226 opts.RealClientIPHeader = tt.realClientIPHeader

2227− validation.Validate(opts)

2227+ err := validation.Validate(opts)

2228+ assert.NoError(t, err)

22282229

22292230 proxy, err := NewOAuthProxy(opts, func(string) bool { return true })

22302231 assert.NoError(t, err)

@@ -2240,6 +2241,255 @@ func TestTrustedIPs(t *testing.T) {

22402241 }

22412242}

22422243

2244+func Test_buildRoutesAllowlist(t *testing.T) {

2245+ type expectedAllowedRoute struct {

2246+ method string

2247+ regexString string

2248+ }

2249+

2250+ testCases := []struct {

2251+ name string

2252+ skipAuthRegex []string

2253+ skipAuthRoutes []string

2254+ expectedRoutes []expectedAllowedRoute

2255+ shouldError bool

2256+ }{

2257+ {

2258+ name: "No skip auth configured",

2259+ skipAuthRegex: []string{},

2260+ skipAuthRoutes: []string{},

2261+ expectedRoutes: []expectedAllowedRoute{},

2262+ shouldError: false,

2263+ },

2264+ {

2265+ name: "Only skipAuthRegex configured",

2266+ skipAuthRegex: []string{

2267+ "^/foo/bar",

2268+ "^/baz/[0-9]+/thing",

2269+ },

2270+ skipAuthRoutes: []string{},

2271+ expectedRoutes: []expectedAllowedRoute{

2272+ {

2273+ method: "",

2274+ regexString: "^/foo/bar",

2275+ },

2276+ {

2277+ method: "",

2278+ regexString: "^/baz/[0-9]+/thing",

2279+ },

2280+ },

2281+ shouldError: false,

2282+ },

2283+ {

2284+ name: "Only skipAuthRoutes configured",

2285+ skipAuthRegex: []string{},

2286+ skipAuthRoutes: []string{

2287+ "GET=^/foo/bar",

2288+ "POST=^/baz/[0-9]+/thing",

2289+ "^/all/methods$",

2290+ "WEIRD=^/methods/are/allowed",

2291+ "PATCH=/second/equals?are=handled&just=fine",

2292+ },

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+ },

2314+ },

2315+ shouldError: false,

2316+ },

2317+ {

2318+ name: "Both skipAuthRegexes and skipAuthRoutes configured",

2319+ skipAuthRegex: []string{

2320+ "^/foo/bar/regex",

2321+ "^/baz/[0-9]+/thing/regex",

2322+ },

2323+ skipAuthRoutes: []string{

2324+ "GET=^/foo/bar",

2325+ "POST=^/baz/[0-9]+/thing",

2326+ "^/all/methods$",

2327+ },

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+ },

2349+ },

2350+ shouldError: false,

2351+ },

2352+ {

2353+ name: "Invalid skipAuthRegex entry",

2354+ skipAuthRegex: []string{

2355+ "^/foo/bar",

2356+ "^/baz/[0-9]+/thing",

2357+ "(bad[regex",

2358+ },

2359+ skipAuthRoutes: []string{},

2360+ expectedRoutes: []expectedAllowedRoute{},

2361+ shouldError: true,

2362+ },

2363+ {

2364+ name: "Invalid skipAuthRoutes entry",

2365+ skipAuthRegex: []string{},

2366+ skipAuthRoutes: []string{

2367+ "GET=^/foo/bar",

2368+ "POST=^/baz/[0-9]+/thing",

2369+ "^/all/methods$",

2370+ "PUT=(bad[regex",

2371+ },

2372+ expectedRoutes: []expectedAllowedRoute{},

2373+ shouldError: true,

2374+ },

2375+ }

2376+

2377+ for _, tc := range testCases {

2378+ t.Run(tc.name, func(t *testing.T) {

2379+ opts := &options.Options{

2380+ SkipAuthRegex: tc.skipAuthRegex,

2381+ SkipAuthRoutes: tc.skipAuthRoutes,

2382+ }

2383+ routes, err := buildRoutesAllowlist(opts)

2384+ if tc.shouldError {

2385+ assert.Error(t, err)

2386+ return

2387+ }

2388+ assert.NoError(t, err)

2389+

2390+ for i, route := range routes {

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)

2394+ }

2395+ })

2396+ }

2397+}

2398+

2399+func TestAllowedRequest(t *testing.T) {

2400+ upstreamServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

2401+ w.WriteHeader(200)

2402+ _, err := w.Write([]byte("Allowed Request"))

2403+ if err != nil {

2404+ t.Fatal(err)

2405+ }

2406+ }))

2407+ t.Cleanup(upstreamServer.Close)

2408+

2409+ opts := baseTestOptions()

2410+ opts.UpstreamServers = options.Upstreams{

2411+ {

2412+ ID: upstreamServer.URL,

2413+ Path: "/",

2414+ URI: upstreamServer.URL,

2415+ },

2416+ }

2417+ opts.SkipAuthRegex = []string{

2418+ "^/skip/auth/regex$",

2419+ }

2420+ opts.SkipAuthRoutes = []string{

2421+ "GET=^/skip/auth/routes/get",

2422+ }

2423+ err := validation.Validate(opts)

2424+ assert.NoError(t, err)

2425+ proxy, err := NewOAuthProxy(opts, func(_ string) bool { return true })

2426+ if err != nil {

2427+ t.Fatal(err)

2428+ }

2429+

2430+ testCases := []struct {

2431+ name string

2432+ method string

2433+ url string

2434+ allowed bool

2435+ }{

2436+ {

2437+ name: "Regex GET allowed",

2438+ method: "GET",

2439+ url: "/skip/auth/regex",

2440+ allowed: true,

2441+ },

2442+ {

2443+ name: "Regex POST allowed ",

2444+ method: "POST",

2445+ url: "/skip/auth/regex",

2446+ allowed: true,

2447+ },

2448+ {

2449+ name: "Regex denied",

2450+ method: "GET",

2451+ url: "/wrong/denied",

2452+ allowed: false,

2453+ },

2454+ {

2455+ name: "Route allowed",

2456+ method: "GET",

2457+ url: "/skip/auth/routes/get",

2458+ allowed: true,

2459+ },

2460+ {

2461+ name: "Route denied with wrong method",

2462+ method: "PATCH",

2463+ url: "/skip/auth/routes/get",

2464+ allowed: false,

2465+ },

2466+ {

2467+ name: "Route denied with wrong path",

2468+ method: "GET",

2469+ url: "/skip/auth/routes/wrong/path",

2470+ allowed: false,

2471+ },

2472+ }

2473+

2474+ for _, tc := range testCases {

2475+ t.Run(tc.name, func(t *testing.T) {

2476+ req, err := http.NewRequest(tc.method, tc.url, nil)

2477+ assert.NoError(t, err)

2478+ assert.Equal(t, tc.allowed, proxy.isAllowedRoute(req))

2479+

2480+ rw := httptest.NewRecorder()

2481+ proxy.ServeHTTP(rw, req)

2482+

2483+ if tc.allowed {

2484+ assert.Equal(t, 200, rw.Code)

2485+ assert.Equal(t, "Allowed Request", rw.Body.String())

2486+ } else {

2487+ assert.Equal(t, 403, rw.Code)

2488+ }

2489+ })

2490+ }

2491+}

2492+

22432493func TestProxyAllowedGroups(t *testing.T) {

22442494 tests := []struct {

22452495 name string

@@ -2265,18 +2515,18 @@ func TestProxyAllowedGroups(t *testing.T) {

22652515 CreatedAt: &created,

22662516 }

22672517

2268− upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

2518+ upstreamServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

22692519 w.WriteHeader(200)

22702520 }))

2271− t.Cleanup(upstream.Close)

2521+ t.Cleanup(upstreamServer.Close)

22722522

22732523 test, err := NewProcessCookieTestWithOptionsModifiers(func(opts *options.Options) {

22742524 opts.AllowedGroups = tt.allowedGroups

22752525 opts.UpstreamServers = options.Upstreams{

22762526 {

2277− ID: upstream.URL,

2527+ ID: upstreamServer.URL,

22782528 Path: "/",

2279− URI: upstream.URL,

2529+ URI: upstreamServer.URL,

22802530 },

22812531 }

22822532 })

@@ -2287,7 +2537,8 @@ func TestProxyAllowedGroups(t *testing.T) {

22872537 test.req, _ = http.NewRequest("GET", "/", nil)

22882538

22892539 test.req.Header.Add("accept", applicationJSON)

2290− test.SaveSession(session)

2540+ err = test.SaveSession(session)

2541+ assert.NoError(t, err)

22912542 test.proxy.ServeHTTP(test.rw, test.req)

22922543

22932544 if tt.expectUnauthorized {

pkg/apis/options/options.go+4−6

@@ -3,7 +3,6 @@ package options

33import (

44 "crypto"

55 "net/url"

6− "regexp"

87 oidc "github.com/coreos/go-oidc"

98 ipapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/ip"

@@ -67,6 +66,7 @@ type Options struct {

6766 UpstreamServers Upstreams `cfg:",internal"`

6867

6968 SkipAuthRegex []string `flag:"skip-auth-regex" cfg:"skip_auth_regex"`

69+ SkipAuthRoutes []string `flag:"skip-auth-route" cfg:"skip_auth_routes"`

7070 SkipAuthStripHeaders bool `flag:"skip-auth-strip-headers" cfg:"skip_auth_strip_headers"`

7171 SkipJwtBearerTokens bool `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens"`

7272 ExtraJwtIssuers []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers"`

@@ -114,7 +114,6 @@ type Options struct {

114114

115115 // internal values that are set after config validation

116116 redirectURL *url.URL

117− compiledRegex []*regexp.Regexp

118117 provider providers.Provider

119118 signatureData *SignatureData

120119 oidcVerifier *oidc.IDTokenVerifier

@@ -124,7 +123,6 @@ type Options struct {

124123

125124// Options for Getting internal values

126125func (o *Options) GetRedirectURL() *url.URL { return o.redirectURL }

127−func (o *Options) GetCompiledRegex() []*regexp.Regexp { return o.compiledRegex }

128126func (o *Options) GetProvider() providers.Provider { return o.provider }

129127func (o *Options) GetSignatureData() *SignatureData { return o.signatureData }

130128func (o *Options) GetOIDCVerifier() *oidc.IDTokenVerifier { return o.oidcVerifier }

@@ -133,7 +131,6 @@ func (o *Options) GetRealClientIPParser() ipapi.RealClientIPParser { return o.re

133131

134132// Options for Setting internal values

135133func (o *Options) SetRedirectURL(s *url.URL) { o.redirectURL = s }

136−func (o *Options) SetCompiledRegex(s []*regexp.Regexp) { o.compiledRegex = s }

137134func (o *Options) SetProvider(s providers.Provider) { o.provider = s }

138135func (o *Options) SetSignatureData(s *SignatureData) { o.signatureData = s }

139136func (o *Options) SetOIDCVerifier(s *oidc.IDTokenVerifier) { o.oidcVerifier = s }

@@ -195,8 +192,9 @@ func NewFlagSet() *pflag.FlagSet {

195192 flagSet.Bool("pass-access-token", false, "pass OAuth access_token to upstream via X-Forwarded-Access-Token header")

196193 flagSet.Bool("pass-authorization-header", false, "pass the Authorization Header to upstream")

197194 flagSet.Bool("set-authorization-header", false, "set Authorization response headers (useful in Nginx auth_request mode)")

198− flagSet.StringSlice("skip-auth-regex", []string{}, "bypass authentication for requests path's that match (may be given multiple times)")

199− flagSet.Bool("skip-auth-strip-headers", false, "strips X-Forwarded-* style authentication headers & Authorization header if they would be set by oauth2-proxy for request paths in --skip-auth-regex")

195+ flagSet.StringSlice("skip-auth-regex", []string{}, "(DEPRECATED for --skip-auth-route) bypass authentication for requests path's that match (may be given multiple times)")

196+ 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")

197+ flagSet.Bool("skip-auth-strip-headers", false, "strips `X-Forwarded-*` style authentication headers & `Authorization` header if they would be set by oauth2-proxy for allowlisted requests (`--skip-auth-route`, `--skip-auth-regex`, `--skip-auth-preflight`, `--trusted-ip`)")

200198 flagSet.Bool("skip-provider-button", false, "will skip sign-in-page to directly reach the next step: oauth/start")

201199 flagSet.Bool("skip-auth-preflight", false, "will skip authentication for OPTIONS requests")

202200 flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS providers")

pkg/validation/allowlist.gonew file+70

@@ -0,0 +1,70 @@

1+package validation

3+import (

4+ "fmt"

5+ "os"

6+ "regexp"

7+ "strings"

9+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"

10+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/ip"

11+)

12+

13+func validateAllowlists(o *options.Options) []string {

14+ msgs := []string{}

15+

16+ msgs = append(msgs, validateRoutes(o)...)

17+ msgs = append(msgs, validateRegexes(o)...)

18+ msgs = append(msgs, validateTrustedIPs(o)...)

19+

20+ if len(o.TrustedIPs) > 0 && o.ReverseProxy {

21+ _, err := fmt.Fprintln(os.Stderr, "WARNING: mixing --trusted-ip with --reverse-proxy is a potential security vulnerability. An attacker can inject a trusted IP into an X-Real-IP or X-Forwarded-For header if they aren't properly protected outside of oauth2-proxy")

22+ if err != nil {

23+ panic(err)

24+ }

25+ }

26+

27+ return msgs

28+}

29+

30+// validateRoutes validates method=path routes passed with options.SkipAuthRoutes

31+func validateRoutes(o *options.Options) []string {

32+ msgs := []string{}

33+ for _, route := range o.SkipAuthRoutes {

34+ var regex string

35+ parts := strings.SplitN(route, "=", 2)

36+ if len(parts) == 1 {

37+ regex = parts[0]

38+ } else {

39+ regex = parts[1]

40+ }

41+ _, err := regexp.Compile(regex)

42+ if err != nil {

43+ msgs = append(msgs, fmt.Sprintf("error compiling regex /%s/: %v", regex, err))

44+ }

45+ }

46+ return msgs

47+}

48+

49+// validateRegex validates regex paths passed with options.SkipAuthRegex

50+func validateRegexes(o *options.Options) []string {

51+ msgs := []string{}

52+ for _, regex := range o.SkipAuthRegex {

53+ _, err := regexp.Compile(regex)

54+ if err != nil {

55+ msgs = append(msgs, fmt.Sprintf("error compiling regex /%s/: %v", regex, err))

56+ }

57+ }

58+ return msgs

59+}

60+

61+// validateTrustedIPs validates IP/CIDRs for IP based allowlists

62+func validateTrustedIPs(o *options.Options) []string {

63+ msgs := []string{}

64+ for i, ipStr := range o.TrustedIPs {

65+ if nil == ip.ParseIPNet(ipStr) {

66+ msgs = append(msgs, fmt.Sprintf("trusted_ips[%d] (%s) could not be recognized", i, ipStr))

67+ }

68+ }

69+ return msgs

70+}

pkg/validation/allowlist_test.gonew file+125

@@ -0,0 +1,125 @@

1+package validation

3+import (

4+ . "github.com/onsi/ginkgo"

5+ . "github.com/onsi/ginkgo/extensions/table"

6+ . "github.com/onsi/gomega"

8+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"

9+)

10+

11+var _ = Describe("Allowlist", func() {

12+ type validateRoutesTableInput struct {

13+ routes []string

14+ errStrings []string

15+ }

16+

17+ type validateRegexesTableInput struct {

18+ regexes []string

19+ errStrings []string

20+ }

21+

22+ type validateTrustedIPsTableInput struct {

23+ trustedIPs []string

24+ errStrings []string

25+ }

26+

27+ DescribeTable("validateRoutes",

28+ func(r *validateRoutesTableInput) {

29+ opts := &options.Options{

30+ SkipAuthRoutes: r.routes,

31+ }

32+ Expect(validateRoutes(opts)).To(ConsistOf(r.errStrings))

33+ },

34+ Entry("Valid regex routes", &validateRoutesTableInput{

35+ routes: []string{

36+ "/foo",

37+ "POST=/foo/bar",

38+ "PUT=^/foo/bar$",

39+ "DELETE=/crazy/(?:regex)?/[^/]+/stuff$",

40+ },

41+ errStrings: []string{},

42+ }),

43+ Entry("Bad regexes do not compile", &validateRoutesTableInput{

44+ routes: []string{

45+ "POST=/(foo",

46+ "OPTIONS=/foo/bar)",

47+ "GET=^]/foo/bar[$",

48+ "GET=^]/foo/bar[$",

49+ },

50+ errStrings: []string{

51+ "error compiling regex //(foo/: error parsing regexp: missing closing ): `/(foo`",

52+ "error compiling regex //foo/bar)/: error parsing regexp: unexpected ): `/foo/bar)`",

53+ "error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",

54+ "error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",

55+ },

56+ }),

57+ )

58+

59+ DescribeTable("validateRegexes",

60+ func(r *validateRegexesTableInput) {

61+ opts := &options.Options{

62+ SkipAuthRegex: r.regexes,

63+ }

64+ Expect(validateRegexes(opts)).To(ConsistOf(r.errStrings))

65+ },

66+ Entry("Valid regex routes", &validateRegexesTableInput{

67+ regexes: []string{

68+ "/foo",

69+ "/foo/bar",

70+ "^/foo/bar$",

71+ "/crazy/(?:regex)?/[^/]+/stuff$",

72+ },

73+ errStrings: []string{},

74+ }),

75+ Entry("Bad regexes do not compile", &validateRegexesTableInput{

76+ regexes: []string{

77+ "/(foo",

78+ "/foo/bar)",

79+ "^]/foo/bar[$",

80+ "^]/foo/bar[$",

81+ },

82+ errStrings: []string{

83+ "error compiling regex //(foo/: error parsing regexp: missing closing ): `/(foo`",

84+ "error compiling regex //foo/bar)/: error parsing regexp: unexpected ): `/foo/bar)`",

85+ "error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",

86+ "error compiling regex /^]/foo/bar[$/: error parsing regexp: missing closing ]: `[$`",

87+ },

88+ }),

89+ )

90+

91+ DescribeTable("validateTrustedIPs",

92+ func(t *validateTrustedIPsTableInput) {

93+ opts := &options.Options{

94+ TrustedIPs: t.trustedIPs,

95+ }

96+ Expect(validateTrustedIPs(opts)).To(ConsistOf(t.errStrings))

97+ },

98+ Entry("Non-overlapping valid IPs", &validateTrustedIPsTableInput{

99+ trustedIPs: []string{

100+ "127.0.0.1",

101+ "10.32.0.1/32",

102+ "43.36.201.0/24",

103+ "::1",

104+ "2a12:105:ee7:9234:0:0:0:0/64",

105+ },

106+ errStrings: []string{},

107+ }),

108+ Entry("Overlapping valid IPs", &validateTrustedIPsTableInput{

109+ trustedIPs: []string{

110+ "135.180.78.199",

111+ "135.180.78.199/32",

112+ "d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4",

113+ "d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4/128",

114+ },

115+ errStrings: []string{},

116+ }),

117+ Entry("Invalid IPs", &validateTrustedIPsTableInput{

118+ trustedIPs: []string{"[::1]", "alkwlkbn/32"},

119+ errStrings: []string{

120+ "trusted_ips[0] ([::1]) could not be recognized",

121+ "trusted_ips[1] (alkwlkbn/32) could not be recognized",

122+ },

123+ }),

124+ )

125+})

pkg/validation/options.go+2−22

@@ -9,7 +9,6 @@ import (

99 "net/http"

1010 "net/url"

1111 "os"

12− "regexp"

1312 "strings"

1413

1514 "github.com/coreos/go-oidc"

@@ -184,15 +183,6 @@ func Validate(o *options.Options) error {

184183 o.SetRedirectURL(redirectURL)

185184

186185 msgs = append(msgs, validateUpstreams(o.UpstreamServers)...)

187−

188− for _, u := range o.SkipAuthRegex {

189− compiledRegex, err := regexp.Compile(u)

190− if err != nil {

191− msgs = append(msgs, fmt.Sprintf("error compiling regex=%q %s", u, err))

192− continue

193− }

194− o.SetCompiledRegex(append(o.GetCompiledRegex(), compiledRegex))

195− }

196186 msgs = parseProviderInfo(o, msgs)

197187

198188 if len(o.GoogleGroups) > 0 || o.GoogleAdminEmail != "" || o.GoogleServiceAccountJSON != "" {

@@ -223,18 +213,8 @@ func Validate(o *options.Options) error {

223213 })

224214 }

225215

226− if len(o.TrustedIPs) > 0 && o.ReverseProxy {

227− _, err := fmt.Fprintln(os.Stderr, "WARNING: trusting of IPs with --reverse-proxy poses risks if a header spoofing attack is possible.")

228− if err != nil {

229− panic(err)

230− }

231− }

232−

233− for i, ipStr := range o.TrustedIPs {

234− if nil == ip.ParseIPNet(ipStr) {

235− msgs = append(msgs, fmt.Sprintf("trusted_ips[%d] (%s) could not be recognized", i, ipStr))

236− }

237− }

216+ // Do this after ReverseProxy validation for TrustedIP coordinated checks

217+ msgs = append(msgs, validateAllowlists(o)...)

238218

239219 if len(msgs) != 0 {

240220 return fmt.Errorf("invalid configuration:\n %s",

pkg/validation/options_test.go+9−77

@@ -2,7 +2,6 @@ package validation

33import (

44 "crypto"

5− "errors"

65 "io/ioutil"

76 "net/url"

87 "os"

@@ -78,12 +77,19 @@ func TestClientSecretFileOption(t *testing.T) {

7877 if err != nil {

7978 t.Fatalf("failed to create temp file: %v", err)

8079 }

81− f.WriteString("testcase")

80+ _, err = f.WriteString("testcase")

81+ if err != nil {

82+ t.Fatalf("failed to write to temp file: %v", err)

83+ }

8284 if err := f.Close(); err != nil {

8385 t.Fatalf("failed to close temp file: %v", err)

8486 }

8587 clientSecretFileName := f.Name()

86− defer os.Remove(clientSecretFileName)

88+ defer func(t *testing.T) {

89+ if err := os.Remove(clientSecretFileName); err != nil {

90+ t.Fatalf("failed to delete temp file: %v", err)

91+ }

92+ }(t)

8793

8894 o := options.NewOptions()

8995 o.Cookie.Secret = cookieSecret

@@ -144,41 +150,6 @@ func TestRedirectURL(t *testing.T) {

144150 assert.Equal(t, expected, o.GetRedirectURL())

145151}

146152

147−func TestCompiledRegex(t *testing.T) {

148− o := testOptions()

149− regexps := []string{"/foo/.*", "/ba[rz]/quux"}

150− o.SkipAuthRegex = regexps

151− assert.Equal(t, nil, Validate(o))

152− actual := make([]string, 0)

153− for _, regex := range o.GetCompiledRegex() {

154− actual = append(actual, regex.String())

155− }

156− assert.Equal(t, regexps, actual)

157−}

158−

159−func TestCompiledRegexError(t *testing.T) {

160− o := testOptions()

161− o.SkipAuthRegex = []string{"(foobaz", "barquux)"}

162− err := Validate(o)

163− assert.NotEqual(t, nil, err)

164−

165− expected := errorMsg([]string{

166− "error compiling regex=\"(foobaz\" error parsing regexp: " +

167− "missing closing ): `(foobaz`",

168− "error compiling regex=\"barquux)\" error parsing regexp: " +

169− "unexpected ): `barquux)`"})

170− assert.Equal(t, expected, err.Error())

171−

172− o.SkipAuthRegex = []string{"foobaz", "barquux)"}

173− err = Validate(o)

174− assert.NotEqual(t, nil, err)

175−

176− expected = errorMsg([]string{

177− "error compiling regex=\"barquux)\" error parsing regexp: " +

178− "unexpected ): `barquux)`"})

179− assert.Equal(t, expected, err.Error())

180−}

181−

182153func TestDefaultProviderApiSettings(t *testing.T) {

183154 o := testOptions()

184155 assert.Equal(t, nil, Validate(o))

@@ -337,45 +308,6 @@ func TestRealClientIPHeader(t *testing.T) {

337308 assert.Nil(t, o.GetRealClientIPParser())

338309}

339310

340−func TestIPCIDRSetOption(t *testing.T) {

341− tests := []struct {

342− name string

343− trustedIPs []string

344− err error

345− }{

346− {

347− "TestSomeIPs",

348− []string{"127.0.0.1", "10.32.0.1/32", "43.36.201.0/24", "::1", "2a12:105:ee7:9234:0:0:0:0/64"},

349− nil,

350− }, {

351− "TestOverlappingIPs",

352− []string{"135.180.78.199", "135.180.78.199/32", "d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4", "d910:a5a1:16f8:ddf5:e5b9:5cef:a65e:41f4/128"},

353− nil,

354− }, {

355− "TestInvalidIPs",

356− []string{"[::1]", "alkwlkbn/32"},

357− errors.New(

358− "invalid configuration:\n" +

359− " trusted_ips[0] ([::1]) could not be recognized\n" +

360− " trusted_ips[1] (alkwlkbn/32) could not be recognized",

361− ),

362− },

363− }

364−

365− for _, tt := range tests {

366− t.Run(tt.name, func(t *testing.T) {

367− o := testOptions()

368− o.TrustedIPs = tt.trustedIPs

369− err := Validate(o)

370− if tt.err == nil {

371− assert.Nil(t, err)

372− } else {

373− assert.Equal(t, tt.err.Error(), err.Error())

374− }

375− })

376− }

377−}

378−

379311func TestProviderCAFilesError(t *testing.T) {

380312 file, err := ioutil.TempFile("", "absent.*.crt")

381313 assert.NoError(t, err)