diff --git a/CHANGELOG.md b/CHANGELOG.md
index 84f54dec..edb631d6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,14 @@
- [#936](https://github.com/oauth2-proxy/oauth2-proxy/pull/936) `--user-id-claim` option is deprecated and replaced by `--oidc-email-claim`
- [#630](https://github.com/oauth2-proxy/oauth2-proxy/pull/630) Gitlab projects needs a Gitlab application with the extra `read_api` enabled
+- [#849](https://github.com/oauth2-proxy/oauth2-proxy/pull/849) `/oauth2/auth` `allowed_groups` querystring parameter can be paired with the `allowed-groups` configuration option.
+ - The `allowed_groups` querystring parameter can specify multiple comma delimited groups.
+ - In this scenario, the user must have a group (from their multiple groups) present in both lists to not get a 401 or 403 response code.
+ - Example:
+ - OAuth2-Proxy globally sets the `allowed_groups` as `engineering`.
+ - An application using Kubernetes ingress uses the `/oauth2/auth` endpoint with `allowed_groups` querystring set to `backend`.
+ - A user must have a session with the groups `["engineering", "backend"]` to pass authorization.
+ - Another user with the groups `["engineering", "frontend"]` would fail the querystring authorization portion.
- [#905](https://github.com/oauth2-proxy/oauth2-proxy/pull/905) Existing sessions from v6.0.0 or earlier are no longer valid. They will trigger a reauthentication.
- [#826](https://github.com/oauth2-proxy/oauth2-proxy/pull/826) `skip-auth-strip-headers` now applies to all requests, not just those where authentication would be skipped.
- [#797](https://github.com/oauth2-proxy/oauth2-proxy/pull/797) The behavior of the Google provider Groups restriction changes with this
@@ -19,7 +27,8 @@
- [#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.
- Sessions from v6.0.0 or later had a graceful conversion to SHA256 that resulted in no reauthentication
- Upgrading from v5.1.1 or earlier will result in a reauthentication
-- [#616](https://github.com/oauth2-proxy/oauth2-proxy/pull/616) Ensure you have configured oauth2-proxy to use the `groups` scope. The user may be logged out initially as they may not currently have the `groups` claim however after going back through login process wil be authenticated.
+- [#616](https://github.com/oauth2-proxy/oauth2-proxy/pull/616) Ensure you have configured oauth2-proxy to use the `groups` scope.
+ - The user may be logged out initially as they may not currently have the `groups` claim however after going back through login process wil be authenticated.
- [#839](https://github.com/oauth2-proxy/oauth2-proxy/pull/839) Enables complex data structures for group claim entries, which are output as Json by default.
## Breaking Changes
@@ -50,12 +59,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_groups` querystring (@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)
- [#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 693d5a7a..f97af98b 100644
--- a/oauthproxy.go
+++ b/oauthproxy.go
@@ -744,7 +744,7 @@ func (p *OAuthProxy) serveHTTP(rw http.ResponseWriter, req *http.Request) {
case path == p.OAuthCallbackPath:
p.OAuthCallback(rw, req)
case path == p.AuthOnlyPath:
- p.AuthenticateOnly(rw, req)
+ p.AuthOnly(rw, req)
case path == p.UserInfoPath:
p.UserInfo(rw, req)
default:
@@ -925,11 +925,19 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
}
}
-// AuthenticateOnly checks whether the user is currently logged in
-func (p *OAuthProxy) AuthenticateOnly(rw http.ResponseWriter, req *http.Request) {
+// AuthOnly checks whether the user is currently logged in (both authentication
+// and optional authorization).
+func (p *OAuthProxy) AuthOnly(rw http.ResponseWriter, req *http.Request) {
session, err := p.getAuthenticatedSession(rw, req)
if err != nil {
- http.Error(rw, "unauthorized request", http.StatusUnauthorized)
+ http.Error(rw, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
+ return
+ }
+
+ // 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
}
@@ -1016,6 +1024,53 @@ func (p *OAuthProxy) getAuthenticatedSession(rw http.ResponseWriter, req *http.R
return session, nil
}
+// authOnlyAuthorize handles special authorization logic that is only done
+// on the AuthOnly endpoint for use with Nginx subrequest architectures.
+//
+// TODO (@NickMeves): This method is a placeholder to be extended but currently
+// fails the linter. Remove the nolint when functionality expands.
+//
+//nolint:S1008
+func authOnlyAuthorize(req *http.Request, s *sessionsapi.SessionState) bool {
+ // Allow secondary group restrictions based on the `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 s.Groups {
+ if _, ok := allowedGroups[group]; ok {
+ return true
+ }
+ }
+
+ return false
+}
+
+func extractAllowedGroups(req *http.Request) map[string]struct{} {
+ groups := map[string]struct{}{}
+
+ query := req.URL.Query()
+ for _, allowedGroups := range query["allowed_groups"] {
+ for _, group := range strings.Split(allowedGroups, ",") {
+ if group != "" {
+ groups[group] = struct{}{}
+ }
+ }
+ }
+
+ return groups
+}
+
// addHeadersForProxying adds the appropriate headers the request / response for proxying
func (p *OAuthProxy) addHeadersForProxying(rw http.ResponseWriter, req *http.Request, session *sessionsapi.SessionState) {
if session.Email == "" {
diff --git a/oauthproxy_test.go b/oauthproxy_test.go
index 866109ca..56e046c2 100644
--- a/oauthproxy_test.go
+++ b/oauthproxy_test.go
@@ -1197,18 +1197,20 @@ func TestUserInfoEndpointUnauthorizedOnNoCookieSetError(t *testing.T) {
assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
}
-func NewAuthOnlyEndpointTest(modifiers ...OptionsModifier) (*ProcessCookieTest, error) {
+func NewAuthOnlyEndpointTest(querystring string, modifiers ...OptionsModifier) (*ProcessCookieTest, error) {
pcTest, err := NewProcessCookieTestWithOptionsModifiers(modifiers...)
if err != nil {
return nil, err
}
- pcTest.req, _ = http.NewRequest("GET",
- pcTest.opts.ProxyPrefix+"/auth", nil)
+ pcTest.req, _ = http.NewRequest(
+ "GET",
+ fmt.Sprintf("%s/auth%s", pcTest.opts.ProxyPrefix, querystring),
+ nil)
return pcTest, nil
}
func TestAuthOnlyEndpointAccepted(t *testing.T) {
- test, err := NewAuthOnlyEndpointTest()
+ test, err := NewAuthOnlyEndpointTest("")
if err != nil {
t.Fatal(err)
}
@@ -1226,7 +1228,7 @@ func TestAuthOnlyEndpointAccepted(t *testing.T) {
}
func TestAuthOnlyEndpointUnauthorizedOnNoCookieSetError(t *testing.T) {
- test, err := NewAuthOnlyEndpointTest()
+ test, err := NewAuthOnlyEndpointTest("")
if err != nil {
t.Fatal(err)
}
@@ -1234,11 +1236,11 @@ func TestAuthOnlyEndpointUnauthorizedOnNoCookieSetError(t *testing.T) {
test.proxy.ServeHTTP(test.rw, test.req)
assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
bodyBytes, _ := ioutil.ReadAll(test.rw.Body)
- assert.Equal(t, "unauthorized request\n", string(bodyBytes))
+ assert.Equal(t, "Unauthorized\n", string(bodyBytes))
}
func TestAuthOnlyEndpointUnauthorizedOnExpiration(t *testing.T) {
- test, err := NewAuthOnlyEndpointTest(func(opts *options.Options) {
+ test, err := NewAuthOnlyEndpointTest("", func(opts *options.Options) {
opts.Cookie.Expire = time.Duration(24) * time.Hour
})
if err != nil {
@@ -1254,11 +1256,11 @@ func TestAuthOnlyEndpointUnauthorizedOnExpiration(t *testing.T) {
test.proxy.ServeHTTP(test.rw, test.req)
assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
bodyBytes, _ := ioutil.ReadAll(test.rw.Body)
- assert.Equal(t, "unauthorized request\n", string(bodyBytes))
+ assert.Equal(t, "Unauthorized\n", string(bodyBytes))
}
func TestAuthOnlyEndpointUnauthorizedOnEmailValidationFailure(t *testing.T) {
- test, err := NewAuthOnlyEndpointTest()
+ test, err := NewAuthOnlyEndpointTest("")
if err != nil {
t.Fatal(err)
}
@@ -1273,7 +1275,7 @@ func TestAuthOnlyEndpointUnauthorizedOnEmailValidationFailure(t *testing.T) {
test.proxy.ServeHTTP(test.rw, test.req)
assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
bodyBytes, _ := ioutil.ReadAll(test.rw.Body)
- assert.Equal(t, "unauthorized request\n", string(bodyBytes))
+ assert.Equal(t, "Unauthorized\n", string(bodyBytes))
}
func TestAuthOnlyEndpointSetXAuthRequestHeaders(t *testing.T) {
@@ -1960,7 +1962,7 @@ func TestGetJwtSession(t *testing.T) {
verifier := oidc.NewVerifier("https://issuer.example.com", keyset,
&oidc.Config{ClientID: "https://test.myapp.com", SkipExpiryCheck: true})
- test, err := NewAuthOnlyEndpointTest(func(opts *options.Options) {
+ test, err := NewAuthOnlyEndpointTest("", func(opts *options.Options) {
opts.InjectRequestHeaders = []options.Header{
{
Name: "Authorization",
@@ -2028,7 +2030,6 @@ func TestGetJwtSession(t *testing.T) {
},
},
}
-
opts.SkipJwtBearerTokens = true
opts.SetJWTBearerVerifiers(append(opts.GetJWTBearerVerifiers(), verifier))
})
@@ -2692,32 +2693,106 @@ func TestProxyAllowedGroups(t *testing.T) {
}
func TestAuthOnlyAllowedGroups(t *testing.T) {
- tests := []struct {
+ testCases := []struct {
name string
allowedGroups []string
groups []string
- expectUnauthorized bool
+ querystring string
+ expectedStatusCode int
}{
- {"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: "",
+ expectedStatusCode: http.StatusAccepted,
+ },
+ {
+ name: "NoAllowedGroupsUserHasGroups",
+ allowedGroups: []string{},
+ groups: []string{"a", "b"},
+ querystring: "",
+ expectedStatusCode: http.StatusAccepted,
+ },
+ {
+ name: "UserInAllowedGroup",
+ allowedGroups: []string{"a"},
+ groups: []string{"a", "b"},
+ querystring: "",
+ expectedStatusCode: http.StatusAccepted,
+ },
+ {
+ name: "UserNotInAllowedGroup",
+ allowedGroups: []string{"a"},
+ groups: []string{"c"},
+ querystring: "",
+ expectedStatusCode: http.StatusUnauthorized,
+ },
+ {
+ name: "UserInQuerystringGroup",
+ allowedGroups: []string{"a", "b"},
+ groups: []string{"a", "c"},
+ querystring: "?allowed_groups=a",
+ expectedStatusCode: http.StatusAccepted,
+ },
+ {
+ name: "UserInMultiParamQuerystringGroup",
+ allowedGroups: []string{"a", "b"},
+ groups: []string{"b"},
+ querystring: "?allowed_groups=a&allowed_groups=b,d",
+ expectedStatusCode: http.StatusAccepted,
+ },
+ {
+ name: "UserInOnlyQuerystringGroup",
+ allowedGroups: []string{},
+ groups: []string{"a", "c"},
+ querystring: "?allowed_groups=a,b",
+ expectedStatusCode: http.StatusAccepted,
+ },
+ {
+ name: "UserInDelimitedQuerystringGroup",
+ allowedGroups: []string{"a", "b", "c"},
+ groups: []string{"c"},
+ querystring: "?allowed_groups=a,c",
+ expectedStatusCode: http.StatusAccepted,
+ },
+ {
+ name: "UserNotInQuerystringGroup",
+ allowedGroups: []string{},
+ groups: []string{"c"},
+ querystring: "?allowed_groups=a,b",
+ expectedStatusCode: http.StatusForbidden,
+ },
+ {
+ name: "UserInConfigGroupNotInQuerystringGroup",
+ allowedGroups: []string{"a", "b", "c"},
+ groups: []string{"c"},
+ querystring: "?allowed_groups=a,b",
+ expectedStatusCode: http.StatusForbidden,
+ },
+ {
+ name: "UserInQuerystringGroupNotInConfigGroup",
+ allowedGroups: []string{"a", "b"},
+ groups: []string{"c"},
+ querystring: "?allowed_groups=b,c",
+ expectedStatusCode: http.StatusUnauthorized,
+ },
}
- for _, tt := range tests {
- t.Run(tt.name, func(t *testing.T) {
+ for _, tc := range testCases {
+ t.Run(tc.name, func(t *testing.T) {
emailAddress := "test"
created := time.Now()
session := &sessions.SessionState{
- Groups: tt.groups,
+ Groups: tc.groups,
Email: emailAddress,
AccessToken: "oauth_token",
CreatedAt: &created,
}
- test, err := NewAuthOnlyEndpointTest(func(opts *options.Options) {
- opts.AllowedGroups = tt.allowedGroups
+ test, err := NewAuthOnlyEndpointTest(tc.querystring, func(opts *options.Options) {
+ opts.AllowedGroups = tc.allowedGroups
})
if err != nil {
t.Fatal(err)
@@ -2728,11 +2803,7 @@ func TestAuthOnlyAllowedGroups(t *testing.T) {
test.proxy.ServeHTTP(test.rw, test.req)
- if tt.expectUnauthorized {
- assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
- } else {
- assert.Equal(t, http.StatusAccepted, test.rw.Code)
- }
+ assert.Equal(t, tc.expectedStatusCode, test.rw.Code)
})
}
}