diff --git a/CHANGELOG.md b/CHANGELOG.md index ef5a77aa..504222d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,13 +54,13 @@ - [#938](https://github.com/oauth2-proxy/oauth2-proxy/pull/938) Cleanup missed provider renaming refactor methods (@NickMeves) - [#816](https://github.com/oauth2-proxy/oauth2-proxy/pull/816) (via [#936](https://github.com/oauth2-proxy/oauth2-proxy/pull/936)) Support non-list group claims (@loafoe) - [#936](https://github.com/oauth2-proxy/oauth2-proxy/pull/936) Refactor OIDC Provider and support groups from Profile URL (@NickMeves) +- [#869](https://github.com/oauth2-proxy/oauth2-proxy/pull/869) Streamline provider interface method names and signatures (@NickMeves) +- [#849](https://github.com/oauth2-proxy/oauth2-proxy/pull/849) Support group authorization on `oauth2/auth` endpoint via `allowed_group` & `allowed_groups` querystring parameters (@NickMeves) - [#925](https://github.com/oauth2-proxy/oauth2-proxy/pull/925) Fix basic auth legacy header conversion (@JoelSpeed) - [#916](https://github.com/oauth2-proxy/oauth2-proxy/pull/916) Add AlphaOptions struct to prepare for alpha config loading (@JoelSpeed) - [#923](https://github.com/oauth2-proxy/oauth2-proxy/pull/923) Support TLS 1.3 (@aajisaka) - [#918](https://github.com/oauth2-proxy/oauth2-proxy/pull/918) Fix log header output (@JoelSpeed) - [#911](https://github.com/oauth2-proxy/oauth2-proxy/pull/911) Validate provider type on startup. -- [#869](https://github.com/oauth2-proxy/oauth2-proxy/pull/869) Streamline provider interface method names and signatures (@NickMeves) -- [#849](https://github.com/oauth2-proxy/oauth2-proxy/pull/849) Support group authorization on `oauth2/auth` endpoint via `allowed_group` & `allowed_groups` querystring parameters (@NickMeves) - [#906](https://github.com/oauth2-proxy/oauth2-proxy/pull/906) Set up v6.1.x versioned documentation as default documentation (@JoelSpeed) - [#905](https://github.com/oauth2-proxy/oauth2-proxy/pull/905) Remove v5 legacy sessions support (@NickMeves) - [#904](https://github.com/oauth2-proxy/oauth2-proxy/pull/904) Set `skip-auth-strip-headers` to `true` by default (@NickMeves) diff --git a/oauthproxy.go b/oauthproxy.go index 0520593b..c11b83b1 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -926,7 +926,7 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { } // AuthOnly checks whether the user is currently logged in (both authentication -// and optional authorization via `allowed_groups` querystring). +// and optional authorization). func (p *OAuthProxy) AuthOnly(rw http.ResponseWriter, req *http.Request) { session, err := p.getAuthenticatedSession(rw, req) if err != nil { @@ -934,9 +934,9 @@ func (p *OAuthProxy) AuthOnly(rw http.ResponseWriter, req *http.Request) { return } - // Allow secondary group restrictions based on the `allowed_group` or - // `allowed_groups` querystring parameter - if !checkAllowedGroups(req, session) { + // Unauthorized cases need to return 403 to prevent infinite redirects with + // subrequest architectures + if !authOnlyAuthorize(req, session) { http.Error(rw, http.StatusText(http.StatusForbidden), http.StatusForbidden) return } @@ -1024,13 +1024,25 @@ func (p *OAuthProxy) getAuthenticatedSession(rw http.ResponseWriter, req *http.R return session, nil } -func checkAllowedGroups(req *http.Request, session *sessionsapi.SessionState) bool { +// authOnlyAuthorize handles special authorization logic that is only done +// on the AuthOnly endpoint for use with Nginx subrequest architectures. +func authOnlyAuthorize(req *http.Request, s *sessionsapi.SessionState) bool { + // Allow secondary group restrictions based on the `allowed_group` or + // `allowed_groups` querystring parameter + if !checkAllowedGroups(req, s) { + return false + } + + return true +} + +func checkAllowedGroups(req *http.Request, s *sessionsapi.SessionState) bool { allowedGroups := extractAllowedGroups(req) if len(allowedGroups) == 0 { return true } - for _, group := range session.Groups { + for _, group := range s.Groups { if _, ok := allowedGroups[group]; ok { return true }