From 8933ab8074a08ef45b1dcea45ab8b680791e0f3e Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Tue, 5 Nov 2019 19:35:25 +0800 Subject: [PATCH] Add configuration "case sensitive" to HTTP auth proxy This commit make case sensitivity configurable when the authentication backend is auth proxy. When the "http_authproxy_case_sensitive" is set to false, the name of user/group will be converted to lower-case when onboarded to Harbor, so as long as the authentication is successful there's no difference regardless upper or lower case is used. It will be mapped to one entry in Harbor's User/Group table. Similar to auth_mode, there is limitation that once there are users onboarded to Harbor's DB this attribute is not configurable. Signed-off-by: Daniel Jiang --- src/common/config/metadata/metadatalist.go | 1 + src/common/const.go | 1 + src/common/models/config.go | 1 + src/core/api/config.go | 37 ++++++++++++++-------- src/core/auth/authproxy/auth.go | 30 +++++++++++++++--- src/core/auth/authproxy/auth_test.go | 17 ++++++---- src/core/config/config.go | 2 +- src/core/config/config_test.go | 14 ++++---- src/core/filter/security_test.go | 1 + 9 files changed, 73 insertions(+), 31 deletions(-) diff --git a/src/common/config/metadata/metadatalist.go b/src/common/config/metadata/metadatalist.go index 60a235bb4..85a5dfa4e 100644 --- a/src/common/config/metadata/metadatalist.go +++ b/src/common/config/metadata/metadatalist.go @@ -139,6 +139,7 @@ var ( {Name: common.HTTPAuthProxyTokenReviewEndpoint, 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.HTTPAuthProxyCaseSensitive, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "true", ItemType: &BoolType{}}, {Name: common.OIDCName, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}}, {Name: common.OIDCEndpoint, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}}, diff --git a/src/common/const.go b/src/common/const.go index 3a7f6b9bf..9ae956d11 100755 --- a/src/common/const.go +++ b/src/common/const.go @@ -105,6 +105,7 @@ const ( HTTPAuthProxyTokenReviewEndpoint = "http_authproxy_tokenreview_endpoint" HTTPAuthProxyVerifyCert = "http_authproxy_verify_cert" HTTPAuthProxySkipSearch = "http_authproxy_skip_search" + HTTPAuthProxyCaseSensitive = "http_authproxy_case_sensitive" OIDCName = "oidc_name" OIDCEndpoint = "oidc_endpoint" OIDCCLientID = "oidc_client_id" diff --git a/src/common/models/config.go b/src/common/models/config.go index 3f22e1b94..7094613aa 100644 --- a/src/common/models/config.go +++ b/src/common/models/config.go @@ -73,6 +73,7 @@ type HTTPAuthProxy struct { TokenReviewEndpoint string `json:"tokenreivew_endpoint"` VerifyCert bool `json:"verify_cert"` SkipSearch bool `json:"skip_search"` + CaseSensitive bool `json:"case_sensitive"` } // OIDCSetting wraps the settings for OIDC auth endpoint diff --git a/src/core/api/config.go b/src/core/api/config.go index 8a7e4f710..6e6338d27 100644 --- a/src/core/api/config.go +++ b/src/core/api/config.go @@ -121,21 +121,32 @@ func (c *ConfigAPI) Put() { } func (c *ConfigAPI) validateCfg(cfgs map[string]interface{}) (bool, error) { - mode := c.cfgManager.Get(common.AUTHMode).GetString() - if value, ok := cfgs[common.AUTHMode]; ok { - flag, err := authModeCanBeModified() - if err != nil { - return true, err - } - if mode != fmt.Sprintf("%v", value) && !flag { - return false, fmt.Errorf("%s can not be modified as new users have been inserted into database", common.AUTHMode) - } - } - err := c.cfgManager.ValidateCfg(cfgs) + flag, err := authModeCanBeModified() if err != nil { - return false, err + return true, err } - return false, nil + if !flag { + if failedKeys := checkUnmodifiable(c.cfgManager, cfgs, common.AUTHMode, common.HTTPAuthProxyCaseSensitive); len(failedKeys) > 0 { + return false, fmt.Errorf("the keys %v can not be modified as new users have been inserted into database", failedKeys) + } + } + err = c.cfgManager.ValidateCfg(cfgs) + return false, err +} + +func checkUnmodifiable(mgr *config.CfgManager, cfgs map[string]interface{}, keys ...string) (failed []string) { + if mgr == nil || cfgs == nil || keys == nil { + return + } + for _, k := range keys { + v := mgr.Get(k).GetString() + if nv, ok := cfgs[k]; ok { + if v != fmt.Sprintf("%v", nv) { + failed = append(failed, k) + } + } + } + return } // delete sensitive attrs and add editable field to every attr diff --git a/src/core/auth/authproxy/auth.go b/src/core/auth/authproxy/auth.go index 26467bac6..e9a1414b9 100644 --- a/src/core/auth/authproxy/auth.go +++ b/src/core/auth/authproxy/auth.go @@ -40,11 +40,14 @@ import ( const refreshDuration = 2 * time.Second const userEntryComment = "By Authproxy" -var secureTransport = &http.Transport{} +var secureTransport = &http.Transport{ + Proxy: http.ProxyFromEnvironment, +} var insecureTransport = &http.Transport{ TLSClientConfig: &tls.Config{ InsecureSkipVerify: true, }, + Proxy: http.ProxyFromEnvironment, } // Auth implements HTTP authenticator the required attributes. @@ -56,8 +59,12 @@ type Auth struct { TokenReviewEndpoint string SkipCertVerify bool SkipSearch bool - settingTimeStamp time.Time - client *http.Client + // When this attribute is set to false, the name of user/group will be converted to lower-case when onboarded to Harbor, so + // as long as the authentication is successful there's no difference in terms of upper or lower case that is used. + // It will be mapped to one entry in Harbor's User/Group table. + CaseSensitive bool + settingTimeStamp time.Time + client *http.Client } type session struct { @@ -84,7 +91,8 @@ func (a *Auth) Authenticate(m models.AuthModel) (*models.User, error) { } defer resp.Body.Close() if resp.StatusCode == http.StatusOK { - user := &models.User{Username: m.Principal} + name := a.normalizeName(m.Principal) + user := &models.User{Username: name} data, err := ioutil.ReadAll(resp.Body) if err != nil { log.Warningf("Failed to read response body, error: %v", err) @@ -141,6 +149,7 @@ func (a *Auth) tokenReview(sessionID string) (*k8s_api_v1beta1.TokenReview, erro // OnBoardUser delegates to dao pkg to insert/update data in DB. func (a *Auth) OnBoardUser(u *models.User) error { + u.Username = a.normalizeName(u.Username) return dao.OnBoardUser(u) } @@ -156,12 +165,14 @@ func (a *Auth) PostAuthenticate(u *models.User) error { } // SearchUser returns nil as authproxy does not have such capability. -// When SkipSearch is set it always return the default model. +// When SkipSearch is set it always return the default model, +// the username will be switch to lowercase if it's configured as case-insensitive func (a *Auth) SearchUser(username string) (*models.User, error) { err := a.ensure() if err != nil { log.Warningf("Failed to refresh configuration for HTTP Auth Proxy Authenticator, error: %v, the default settings will be used", err) } + username = a.normalizeName(username) var u *models.User if a.SkipSearch { u = &models.User{Username: username} @@ -178,6 +189,7 @@ func (a *Auth) SearchGroup(groupKey string) (*models.UserGroup, error) { if err != nil { log.Warningf("Failed to refresh configuration for HTTP Auth Proxy Authenticator, error: %v, the default settings will be used", err) } + groupKey = a.normalizeName(groupKey) var ug *models.UserGroup if a.SkipSearch { ug = &models.UserGroup{ @@ -196,6 +208,7 @@ func (a *Auth) OnBoardGroup(u *models.UserGroup, altGroupName string) error { return errors.New("Should provide a group name") } u.GroupType = common.HTTPGroupType + u.GroupName = a.normalizeName(u.GroupName) err := group.OnBoardUserGroup(u) if err != nil { return err @@ -241,6 +254,13 @@ func (a *Auth) ensure() error { return nil } +func (a *Auth) normalizeName(n string) string { + if !a.CaseSensitive { + return strings.ToLower(n) + } + return n +} + func init() { auth.Register(common.HTTPAuth, &Auth{}) } diff --git a/src/core/auth/authproxy/auth_test.go b/src/core/auth/authproxy/auth_test.go index 86cfebb15..126c2c68d 100644 --- a/src/core/auth/authproxy/auth_test.go +++ b/src/core/auth/authproxy/auth_test.go @@ -43,11 +43,11 @@ func TestMain(m *testing.M) { } mockSvr = test.NewMockServer(map[string]string{"jt": "pp", "Admin@vsphere.local": "Admin!23"}) defer mockSvr.Close() - defer dao.ExecuteBatchSQL([]string{"delete from user_group where group_name='OnBoardTest'"}) a = &Auth{ Endpoint: mockSvr.URL + "/test/login", TokenReviewEndpoint: mockSvr.URL + "/test/tokenreview", SkipCertVerify: true, + CaseSensitive: false, // So it won't require mocking the cfgManager settingTimeStamp: time.Now(), } @@ -56,6 +56,7 @@ func TestMain(m *testing.M) { common.HTTPAuthProxyEndpoint: a.Endpoint, common.HTTPAuthProxyTokenReviewEndpoint: a.TokenReviewEndpoint, common.HTTPAuthProxyVerifyCert: !a.SkipCertVerify, + common.HTTPAuthProxyCaseSensitive: a.CaseSensitive, common.PostGreSQLSSLMode: cfgMap[common.PostGreSQLSSLMode], common.PostGreSQLUsername: cfgMap[common.PostGreSQLUsername], common.PostGreSQLPort: cfgMap[common.PostGreSQLPort], @@ -65,6 +66,7 @@ func TestMain(m *testing.M) { } config.InitWithSettings(conf) + defer dao.ExecuteBatchSQL([]string{"delete from user_group where group_name='onboardtest'"}) rc := m.Run() if err := dao.ClearHTTPAuthProxyUsers(); err != nil { panic(err) @@ -117,7 +119,7 @@ func TestAuth_Authenticate(t *testing.T) { }, expect: output{ user: models.User{ - Username: "Admin@vsphere.local", + Username: "admin@vsphere.local", GroupIDs: groupIDs, // Email: "Admin@placeholder.com", // Password: pwd, @@ -172,12 +174,12 @@ func TestAuth_PostAuthenticate(t *testing.T) { }, { input: &models.User{ - Username: "Admin@vsphere.local", + Username: "admin@vsphere.local", }, expect: models.User{ - Username: "Admin@vsphere.local", - Email: "Admin@vsphere.local", - Realname: "Admin@vsphere.local", + Username: "admin@vsphere.local", + Email: "admin@vsphere.local", + Realname: "admin@vsphere.local", Password: pwd, Comment: userEntryComment, }, @@ -201,6 +203,9 @@ func TestAuth_OnBoardGroup(t *testing.T) { a.OnBoardGroup(input, "") assert.True(t, input.ID > 0, "The OnBoardGroup should have a valid group ID") + g, er := group.GetUserGroup(input.ID) + assert.Nil(t, er) + assert.Equal(t, "onboardtest", g.GroupName) emptyGroup := &models.UserGroup{} err := a.OnBoardGroup(emptyGroup, "") diff --git a/src/core/config/config.go b/src/core/config/config.go index 409396e03..fe0e2b7a4 100755 --- a/src/core/config/config.go +++ b/src/core/config/config.go @@ -475,8 +475,8 @@ func HTTPAuthProxySetting() (*models.HTTPAuthProxy, error) { TokenReviewEndpoint: cfgMgr.Get(common.HTTPAuthProxyTokenReviewEndpoint).GetString(), VerifyCert: cfgMgr.Get(common.HTTPAuthProxyVerifyCert).GetBool(), SkipSearch: cfgMgr.Get(common.HTTPAuthProxySkipSearch).GetBool(), + CaseSensitive: cfgMgr.Get(common.HTTPAuthProxyCaseSensitive).GetBool(), }, nil - } // OIDCSetting returns the setting of OIDC provider, currently there's only one OIDC provider allowed for Harbor and it's diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index 72309af2c..b4e5f4bd3 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -217,17 +217,19 @@ func currPath() string { func TestHTTPAuthProxySetting(t *testing.T) { m := map[string]interface{}{ - common.HTTPAuthProxySkipSearch: "true", - common.HTTPAuthProxyVerifyCert: "true", - common.HTTPAuthProxyEndpoint: "https://auth.proxy/suffix", + common.HTTPAuthProxySkipSearch: "true", + common.HTTPAuthProxyVerifyCert: "true", + common.HTTPAuthProxyCaseSensitive: "false", + common.HTTPAuthProxyEndpoint: "https://auth.proxy/suffix", } InitWithSettings(m) v, e := HTTPAuthProxySetting() assert.Nil(t, e) assert.Equal(t, *v, models.HTTPAuthProxy{ - Endpoint: "https://auth.proxy/suffix", - SkipSearch: true, - VerifyCert: true, + Endpoint: "https://auth.proxy/suffix", + SkipSearch: true, + VerifyCert: true, + CaseSensitive: false, }) } diff --git a/src/core/filter/security_test.go b/src/core/filter/security_test.go index 5c23dd7ec..069af33f6 100644 --- a/src/core/filter/security_test.go +++ b/src/core/filter/security_test.go @@ -257,6 +257,7 @@ func TestAuthProxyReqCtxModifier(t *testing.T) { Endpoint: "https://auth.proxy/suffix", SkipSearch: true, VerifyCert: false, + CaseSensitive: true, TokenReviewEndpoint: server.URL, })