From 60e3668d433315f82874aacd2fbf5d84f3f45474 Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Wed, 9 Dec 2020 20:25:58 +0800 Subject: [PATCH] Support admin group in http authproxy This commit adds admin_groups into the configuration of http_auth settings, it's a string in the form of "group1, group2". If the token review result shows the user is in one of the groups in the setting he will have the administrator role in Harbor. Signed-off-by: Daniel Jiang --- src/common/config/metadata/metadatalist.go | 1 + src/common/const.go | 1 + src/common/models/config.go | 11 +++-- src/core/auth/authproxy/auth.go | 2 +- src/core/config/config.go | 17 +++++-- src/core/config/config_test.go | 34 +++++++++++++ src/pkg/authproxy/http.go | 15 +++++- src/pkg/authproxy/http_test.go | 49 ++++++++++++++----- src/server/middleware/security/auth_proxy.go | 3 +- .../middleware/security/auth_proxy_test.go | 1 + 10 files changed, 109 insertions(+), 25 deletions(-) diff --git a/src/common/config/metadata/metadatalist.go b/src/common/config/metadata/metadatalist.go index 845bc8f80..77f4f335b 100644 --- a/src/common/config/metadata/metadatalist.go +++ b/src/common/config/metadata/metadatalist.go @@ -129,6 +129,7 @@ var ( {Name: common.HTTPAuthProxyEndpoint, Scope: UserScope, Group: HTTPAuthGroup, ItemType: &StringType{}}, {Name: common.HTTPAuthProxyTokenReviewEndpoint, Scope: UserScope, Group: HTTPAuthGroup, ItemType: &StringType{}}, + {Name: common.HTTPAuthProxyAdminGroups, Scope: UserScope, Group: HTTPAuthGroup, ItemType: &StringType{}}, {Name: common.HTTPAuthProxyVerifyCert, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "true", ItemType: &BoolType{}}, {Name: common.HTTPAuthProxySkipSearch, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "false", ItemType: &BoolType{}}, {Name: common.HTTPAuthProxyServerCertificate, Scope: UserScope, Group: HTTPAuthGroup, ItemType: &StringType{}}, diff --git a/src/common/const.go b/src/common/const.go index cc9819e72..aec7424b8 100755 --- a/src/common/const.go +++ b/src/common/const.go @@ -96,6 +96,7 @@ const ( UAAVerifyCert = "uaa_verify_cert" HTTPAuthProxyEndpoint = "http_authproxy_endpoint" HTTPAuthProxyTokenReviewEndpoint = "http_authproxy_tokenreview_endpoint" + HTTPAuthProxyAdminGroups = "http_authproxy_admin_groups" HTTPAuthProxyVerifyCert = "http_authproxy_verify_cert" HTTPAuthProxySkipSearch = "http_authproxy_skip_search" HTTPAuthProxyServerCertificate = "http_authproxy_server_certificate" diff --git a/src/common/models/config.go b/src/common/models/config.go index c970dd167..2019ce8b1 100644 --- a/src/common/models/config.go +++ b/src/common/models/config.go @@ -69,11 +69,12 @@ type Email struct { // HTTPAuthProxy wraps the settings for HTTP auth proxy type HTTPAuthProxy struct { - Endpoint string `json:"endpoint"` - TokenReviewEndpoint string `json:"tokenreivew_endpoint"` - VerifyCert bool `json:"verify_cert"` - SkipSearch bool `json:"skip_search"` - ServerCertificate string `json:"server_certificate"` + Endpoint string `json:"endpoint"` + TokenReviewEndpoint string `json:"tokenreivew_endpoint"` + AdminGroups []string `json:"admin_groups"` + VerifyCert bool `json:"verify_cert"` + SkipSearch bool `json:"skip_search"` + ServerCertificate string `json:"server_certificate"` } // OIDCSetting wraps the settings for OIDC auth endpoint diff --git a/src/core/auth/authproxy/auth.go b/src/core/auth/authproxy/auth.go index c5fc40209..0a88e5bf9 100644 --- a/src/core/auth/authproxy/auth.go +++ b/src/core/auth/authproxy/auth.go @@ -116,7 +116,7 @@ func (a *Auth) tokenReview(sessionID string) (*models.User, error) { if err != nil { return nil, err } - u, err := authproxy.UserFromReviewStatus(reviewStatus) + u, err := authproxy.UserFromReviewStatus(reviewStatus, httpAuthProxySetting.AdminGroups) if err != nil { return nil, err } diff --git a/src/core/config/config.go b/src/core/config/config.go index 0596e1923..f642e9474 100755 --- a/src/core/config/config.go +++ b/src/core/config/config.go @@ -391,6 +391,7 @@ func HTTPAuthProxySetting() (*models.HTTPAuthProxy, error) { return &models.HTTPAuthProxy{ Endpoint: cfgMgr.Get(common.HTTPAuthProxyEndpoint).GetString(), TokenReviewEndpoint: cfgMgr.Get(common.HTTPAuthProxyTokenReviewEndpoint).GetString(), + AdminGroups: splitAndTrim(cfgMgr.Get(common.HTTPAuthProxyAdminGroups).GetString(), ","), VerifyCert: cfgMgr.Get(common.HTTPAuthProxyVerifyCert).GetBool(), SkipSearch: cfgMgr.Get(common.HTTPAuthProxySkipSearch).GetBool(), ServerCertificate: cfgMgr.Get(common.HTTPAuthProxyServerCertificate).GetString(), @@ -405,11 +406,7 @@ func OIDCSetting() (*models.OIDCSetting, error) { } scopeStr := cfgMgr.Get(common.OIDCScope).GetString() extEndpoint := strings.TrimSuffix(cfgMgr.Get(common.ExtEndpoint).GetString(), "/") - scope := []string{} - for _, s := range strings.Split(scopeStr, ",") { - scope = append(scope, strings.TrimSpace(s)) - } - + scope := splitAndTrim(scopeStr, ",") return &models.OIDCSetting{ Name: cfgMgr.Get(common.OIDCName).GetString(), Endpoint: cfgMgr.Get(common.OIDCEndpoint).GetString(), @@ -479,3 +476,13 @@ func Metric() *models.Metric { Path: cfgMgr.Get(common.MetricPath).GetString(), } } + +func splitAndTrim(s, sep string) []string { + res := make([]string, 0) + for _, s := range strings.Split(s, sep) { + if e := strings.TrimSpace(s); len(e) > 0 { + res = append(res, e) + } + } + return res +} diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index 6e130ef63..438367f2f 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -238,6 +238,7 @@ y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk= m := map[string]interface{}{ common.HTTPAuthProxySkipSearch: "true", common.HTTPAuthProxyVerifyCert: "true", + common.HTTPAuthProxyAdminGroups: "group1, group2", common.HTTPAuthProxyEndpoint: "https://auth.proxy/suffix", common.HTTPAuthProxyServerCertificate: certificate, } @@ -246,6 +247,7 @@ y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk= assert.Nil(t, e) assert.Equal(t, *v, models.HTTPAuthProxy{ Endpoint: "https://auth.proxy/suffix", + AdminGroups: []string{"group1", "group2"}, SkipSearch: true, VerifyCert: true, ServerCertificate: certificate, @@ -279,3 +281,35 @@ func TestOIDCSetting(t *testing.T) { assert.ElementsMatch(t, []string{"openid", "profile"}, v.Scope) assert.Equal(t, "username", v.UserClaim) } + +func TestSplitAndTrim(t *testing.T) { + cases := []struct { + s string + sep string + expect []string + }{ + { + s: "abc", + sep: ",", + expect: []string{"abc"}, + }, + { + s: "a, b, c", + sep: ",", + expect: []string{"a", "b", "c"}, + }, + { + s: "a,b,c ", + sep: ".", + expect: []string{"a,b,c"}, + }, + { + s: "", + sep: ",", + expect: []string{}, + }, + } + for _, c := range cases { + assert.Equal(t, c.expect, splitAndTrim(c.s, c.sep)) + } +} diff --git a/src/pkg/authproxy/http.go b/src/pkg/authproxy/http.go index fd0a5d156..6bc327490 100644 --- a/src/pkg/authproxy/http.go +++ b/src/pkg/authproxy/http.go @@ -79,7 +79,8 @@ func getTLSConfig(config *models.HTTPAuthProxy) rest.TLSClientConfig { // UserFromReviewStatus transform a review status to a user model. // Group entries will be populated if needed. -func UserFromReviewStatus(status k8s_api_v1beta1.TokenReviewStatus) (*models.User, error) { +func UserFromReviewStatus(status k8s_api_v1beta1.TokenReviewStatus, adminGroups []string) (*models.User, error) { + if !status.Authenticated { return nil, fmt.Errorf("failed to authenticate the token, error in status: %s", status.Error) } @@ -94,6 +95,18 @@ func UserFromReviewStatus(status k8s_api_v1beta1.TokenReviewStatus) (*models.Use } log.Debugf("current user's group ID list is %+v", groupIDList) user.GroupIDs = groupIDList + if len(adminGroups) > 0 { + agm := make(map[string]struct{}) + for _, ag := range adminGroups { + agm[ag] = struct{}{} + } + for _, ug := range status.User.Groups { + if _, ok := agm[ug]; ok { + user.AdminRoleInAuth = true + break + } + } + } } return user, nil diff --git a/src/pkg/authproxy/http_test.go b/src/pkg/authproxy/http_test.go index 1c6274b73..58cb53e63 100644 --- a/src/pkg/authproxy/http_test.go +++ b/src/pkg/authproxy/http_test.go @@ -21,19 +21,22 @@ func TestMain(m *testing.M) { func TestUserFromReviewStatus(t *testing.T) { type result struct { - hasErr bool - username string - groupLen int + hasErr bool + username string + groupLen int + adminInAuth bool } cases := []struct { - input v1beta1.TokenReviewStatus - expect result + input v1beta1.TokenReviewStatus + adminGroups []string + expect result }{ { input: v1beta1.TokenReviewStatus{ Authenticated: false, Error: "connection error", }, + adminGroups: []string{"admin"}, expect: result{ hasErr: true, }, @@ -46,10 +49,12 @@ func TestUserFromReviewStatus(t *testing.T) { UID: "u-1", }, }, + adminGroups: []string{"admin"}, expect: result{ - hasErr: false, - username: "jack", - groupLen: 0, + hasErr: false, + username: "jack", + groupLen: 0, + adminInAuth: false, }, }, { @@ -61,21 +66,41 @@ func TestUserFromReviewStatus(t *testing.T) { }, Error: "", }, + adminGroups: []string{"group2", "admin"}, expect: result{ - hasErr: false, - username: "daniel", - groupLen: 2, + hasErr: false, + username: "daniel", + groupLen: 2, + adminInAuth: true, + }, + }, + { + input: v1beta1.TokenReviewStatus{ + Authenticated: true, + User: v1beta1.UserInfo{ + Username: "daniel", + Groups: []string{"group1", "group2"}, + }, + Error: "", + }, + adminGroups: []string{}, + expect: result{ + hasErr: false, + username: "daniel", + groupLen: 2, + adminInAuth: false, }, }, } for _, c := range cases { - u, err := UserFromReviewStatus(c.input) + u, err := UserFromReviewStatus(c.input, c.adminGroups) if c.expect.hasErr == true { assert.NotNil(t, err) } else { assert.Nil(t, err) assert.Equal(t, c.expect.username, u.Username) assert.Equal(t, c.expect.groupLen, len(u.GroupIDs)) + assert.Equal(t, c.expect.adminInAuth, u.AdminRoleInAuth) } if u != nil { for _, gid := range u.GroupIDs { diff --git a/src/server/middleware/security/auth_proxy.go b/src/server/middleware/security/auth_proxy.go index a8f1e6477..8decd3910 100644 --- a/src/server/middleware/security/auth_proxy.go +++ b/src/server/middleware/security/auth_proxy.go @@ -86,12 +86,13 @@ func (a *authProxy) Generate(req *http.Request) security.Context { return nil } } - u2, err := authproxy.UserFromReviewStatus(tokenReviewStatus) + u2, err := authproxy.UserFromReviewStatus(tokenReviewStatus, httpAuthProxyConf.AdminGroups) if err != nil { log.Errorf("failed to get user information from token review status: %v", err) return nil } user.GroupIDs = u2.GroupIDs + user.AdminRoleInAuth = u2.AdminRoleInAuth log.Debugf("an auth proxy security context generated for request %s %s", req.Method, req.URL.Path) return local.NewSecurityContext(user) } diff --git a/src/server/middleware/security/auth_proxy_test.go b/src/server/middleware/security/auth_proxy_test.go index 32b4ed068..16f30c174 100644 --- a/src/server/middleware/security/auth_proxy_test.go +++ b/src/server/middleware/security/auth_proxy_test.go @@ -57,6 +57,7 @@ func TestAuthProxy(t *testing.T) { SkipSearch: true, VerifyCert: false, TokenReviewEndpoint: server.URL, + AdminGroups: []string{}, }) // No onboard