From fcc2db040e2da5781919f21577f721911a384419 Mon Sep 17 00:00:00 2001 From: Jacob Alberty Date: Sat, 8 Nov 2025 06:58:34 -0600 Subject: [PATCH] feat: add allowed_* constraint option to proxy endpoint query string (#2841) * Add check for constraints to the proxy endpoint * Add tests for allowed_groups query string * Add this feature to the changelog * Apply suggestions from code review Co-authored-by: Jan Larwig * Use explicit key names in TestProxyAllowedGroups * Document the query parameters on proxy endpoint * Comment was copied from the AuthOnly handler but on closer inspection is not relevant here replacing comment with one more relevant --------- Signed-off-by: Jan Larwig Co-authored-by: Jan Larwig --- CHANGELOG.md | 1 + docs/docs/features/endpoints.md | 12 ++++++ oauthproxy.go | 7 ++++ oauthproxy_test.go | 73 ++++++++++++++++++++++++++++++--- 4 files changed, 88 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb4904c5..7594b407 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - [#3237](https://github.com/oauth2-proxy/oauth2-proxy/pull/3237) - feat: add option to use organization id for preferred username in Google Provider (@pixeldrew) - [GHSA-vjrc-mh2v-45x6](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-vjrc-mh2v-45x6) fix: request header smuggling by stripping all normalized header variants (@tuunit) - [#1933](https://github.com/oauth2-proxy/oauth2-proxy/pull/1933) fix: validation of refreshed sessions using the access_token in the OIDC provider (@gysel / @tuunit) +- [#2841](https://github.com/oauth2-proxy/oauth2-proxy/pull/2841) feat: add allowed_* constraint option to proxy endpoint query string (@jacobalberty) # V7.12.0 diff --git a/docs/docs/features/endpoints.md b/docs/docs/features/endpoints.md index 3ec1e2aa..db00ba1e 100644 --- a/docs/docs/features/endpoints.md +++ b/docs/docs/features/endpoints.md @@ -5,6 +5,7 @@ title: Endpoints OAuth2 Proxy responds directly to the following endpoints. All other endpoints will be proxied upstream when authenticated. The `/oauth2` prefix can be changed with the `--proxy-prefix` config variable. +- / - the proxy endpoint provides authentication and returns the appropriate 40x error if not authenticated or authorized then passes the request upstream. - /robots.txt - returns a 200 OK response that disallows all User-agents from all paths; see [robotstxt.org](http://www.robotstxt.org/) for more info - /ping - returns a 200 OK response, which is intended for use with health checks - /ready - returns a 200 OK response if all the underlying connections (e.g., Redis store) are connected @@ -45,3 +46,14 @@ It can be configured using the following query parameters: - `allowed_groups`: comma separated list of allowed groups - `allowed_email_domains`: comma separated list of allowed email domains - `allowed_emails`: comma separated list of allowed emails + +### Proxy (/) + +This endpoint returns the upstream response if authenticated. +If unauthenticated it returns a 401 Unauthorized. If the authenticatd user +is not in one of the allowed groups, or emails then it returns a 403 forbidden + +It can be configured using the following query parameters: +- `allowed_groups`: comma separated list of allowed groups +- `allowed_email_domains`: comma separated list of allowed email domains +- `allowed_emails`: comma separated list of allowed emails diff --git a/oauthproxy.go b/oauthproxy.go index 7526d641..c6db18a7 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -1012,6 +1012,13 @@ func (p *OAuthProxy) Proxy(rw http.ResponseWriter, req *http.Request) { session, err := p.getAuthenticatedSession(rw, req) switch err { case nil: + // Check against our authorization constraints and return forbidden + // if this request fails to satisfy them. + if !authOnlyAuthorize(req, session) { + http.Error(rw, http.StatusText(http.StatusForbidden), http.StatusForbidden) + return + } + // we are authenticated p.addHeadersForProxying(rw, session) p.headersChain.Then(p.upstreamProxy).ServeHTTP(rw, req) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 488b8cea..e05396cd 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -2938,12 +2938,75 @@ func TestProxyAllowedGroups(t *testing.T) { name string allowedGroups []string groups []string + querystring string expectUnauthorized bool }{ - {"NoAllowedGroups", []string{}, []string{}, false}, - {"NoAllowedGroupsUserHasGroups", []string{}, []string{"a", "b"}, false}, - {"UserInAllowedGroup", []string{"a"}, []string{"a", "b"}, false}, - {"UserNotInAllowedGroup", []string{"a"}, []string{"c"}, true}, + { + name: "NoAllowedGroups", + allowedGroups: []string{}, + groups: []string{}, + querystring: "", + expectUnauthorized: false}, + { + name: "NoAllowedGroupsUserHasGroups", + allowedGroups: []string{}, + groups: []string{"a", "b"}, + querystring: "", + expectUnauthorized: false}, + { + name: "UserInAllowedGroup", + allowedGroups: []string{"a"}, + groups: []string{"a", "b"}, + querystring: "", + expectUnauthorized: false}, + { + name: "UserNotInAllowedGroup", + allowedGroups: []string{"a"}, + groups: []string{"c"}, + querystring: "", + expectUnauthorized: true}, + { + name: "UserInQuerystringGroup", + allowedGroups: []string{"a", "b"}, + groups: []string{"a", "c"}, + querystring: "?allowed_groups=a", + expectUnauthorized: false}, + { + name: "UserInMultiParamQuerystringGroup", + allowedGroups: []string{"a", "b"}, + groups: []string{"b"}, + querystring: "?allowed_groups=a&allowed_groups=b,d", + expectUnauthorized: false}, + { + name: "UserInOnlyQuerystringGroup", + allowedGroups: []string{}, + groups: []string{"a", "c"}, + querystring: "?allowed_groups=a,b", + expectUnauthorized: false}, + { + name: "UserInDelimitedQuerystringGroup", + allowedGroups: []string{"a", "b", "c"}, + groups: []string{"c"}, + querystring: "?allowed_groups=a,c", + expectUnauthorized: false}, + { + name: "UserNotInQuerystringGroup", + allowedGroups: []string{}, + groups: []string{"c"}, + querystring: "?allowed_groups=a,b", + expectUnauthorized: true}, + { + name: "UserInConfigGroupNotInQuerystringGroup", + allowedGroups: []string{"a", "b", "c"}, + groups: []string{"c"}, + querystring: "?allowed_groups=a,b", + expectUnauthorized: true}, + { + name: "UserInQuerystringGroupNotInConfigGroup", + allowedGroups: []string{"a", "b"}, + groups: []string{"c"}, + querystring: "?allowed_groups=b,c", + expectUnauthorized: true}, } for _, tt := range tests { @@ -2979,7 +3042,7 @@ func TestProxyAllowedGroups(t *testing.T) { t.Fatal(err) } - test.req, _ = http.NewRequest("GET", "/", nil) + test.req, _ = http.NewRequest("GET", fmt.Sprintf("/%s", tt.querystring), nil) test.req.Header.Add("accept", applicationJSON) err = test.SaveSession(session)