Merge pull request #9860 from reasonerjt/authproxy-case-sensitive-master

Authproxy case sensitive master

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{}},

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"

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

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

src/core/api/harborapi_test.go

@@ -904,16 +904,11 @@ func (a testapi) UsersSearch(userName string, authInfo ...usrInfo) (int, []apili
 }
 
 // Get registered users by userid.
-func (a testapi) UsersGetByID(userName string, authInfo usrInfo, userID int) (int, apilib.User, error) {
+func (a testapi) UsersGetByID(userID int, authInfo usrInfo) (int, apilib.User, error) {
 	_sling := sling.New().Get(a.basePath)
 	// create path and map variables
 	path := "/api/users/" + fmt.Sprintf("%d", userID)
 	_sling = _sling.Path(path)
-	// body params
-	type QueryParams struct {
-		UserName string `url:"username, omitempty"`
-	}
-	_sling = _sling.QueryStruct(&QueryParams{UserName: userName})
 	httpStatusCode, body, err := request(_sling, jsonAcceptHeader, authInfo)
 	var successPayLoad apilib.User
 	if 200 == httpStatusCode && nil == err {

src/core/api/user.go

@@ -261,11 +261,12 @@ func (ua *UserAPI) Put() {
 		ua.SendForbiddenError(fmt.Errorf("User with ID %d cannot be modified", ua.userID))
 		return
 	}
-	user := models.User{UserID: ua.userID}
+	user := models.User{}
 	if err := ua.DecodeJSONReq(&user); err != nil {
 		ua.SendBadRequestError(err)
 		return
 	}
+	user.UserID = ua.userID
 	err := commonValidate(user)
 	if err != nil {
 		log.Warningf("Bad request in change user profile: %v", err)

src/core/api/user_test.go

@@ -257,7 +257,7 @@ func TestUsersGetByID(t *testing.T) {
 	apiTest := newHarborAPI()
 
 	// case 1: Get user2 with userID and his own auth, expect 200
-	code, user, err := apiTest.UsersGetByID(testUser0002.Username, *testUser0002Auth, testUser0002ID)
+	code, user, err := apiTest.UsersGetByID(testUser0002ID, *testUser0002Auth)
 	if err != nil {
 		t.Error("Error occurred while get users", err.Error())
 		t.Log(err)
@@ -280,7 +280,7 @@ func TestUsersGetByID(t *testing.T) {
 	}
 
 	testUser0003Auth = &usrInfo{"testUser0003", "testUser0003"}
-	code, user, err = apiTest.UsersGetByID(testUser0002.Username, *testUser0003Auth, testUser0002ID)
+	code, user, err = apiTest.UsersGetByID(testUser0002ID, *testUser0003Auth)
 	if err != nil {
 		t.Error("Error occurred while get users", err.Error())
 		t.Log(err)
@@ -288,7 +288,7 @@ func TestUsersGetByID(t *testing.T) {
 		assert.Equal(403, code, "Get users status should be 403")
 	}
 	// case 3: Get user that does not exist with user2 auth, expect 404 not found.
-	code, user, err = apiTest.UsersGetByID(testUser0002.Username, *testUser0002Auth, 1000)
+	code, user, err = apiTest.UsersGetByID(1000, *testUser0002Auth)
 	if err != nil {
 		t.Error("Error occurred while get users", err.Error())
 		t.Log(err)
@@ -320,7 +320,31 @@ func TestUsersPut(t *testing.T) {
 	} else {
 		assert.Equal(403, code, "Change user profile status should be 403")
 	}
-	// case 2: change user2 profile with user2 auth, but bad parameters format.
+	// case 2: change user3 profile with user2 auth
+	realname := "new realname"
+	email := "new_email@mydomain.com"
+	comment := "new comment"
+	profile.UserID = testUser0003ID
+	profile.Realname = realname
+	profile.Email = email
+	profile.Comment = comment
+	code, err = apiTest.UsersPut(testUser0002ID, profile, *testUser0002Auth)
+	if err != nil {
+		t.Error("Error occurred while change user profile", err.Error())
+		t.Log(err)
+	} else {
+		assert.Equal(200, code, "Change user profile status should be 200")
+	}
+	_, user, err := apiTest.UsersGetByID(testUser0003ID, *testUser0003Auth)
+	if err != nil {
+		t.Error("Error occurred while get user", err.Error())
+	} else {
+		assert.NotEqual(realname, user.Realname)
+		assert.NotEqual(email, user.Email)
+		assert.NotEqual(comment, user.Comment)
+	}
+	// case 3: change user2 profile with user2 auth, but bad parameters format.
+	profile = apilib.UserProfile{}
 	code, err = apiTest.UsersPut(testUser0002ID, profile, *testUser0002Auth)
 	if err != nil {
 		t.Error("Error occurred while change user profile", err.Error())
@@ -328,7 +352,7 @@ func TestUsersPut(t *testing.T) {
 	} else {
 		assert.Equal(400, code, "Change user profile status should be 400")
 	}
-	// case 3: change user2 profile with user2 auth, but duplicate email.
+	// case 4: change user2 profile with user2 auth, but duplicate email.
 	profile.Realname = "test user"
 	profile.Email = "testUser0003@mydomain.com"
 	profile.Comment = "change profile"
@@ -339,7 +363,7 @@ func TestUsersPut(t *testing.T) {
 	} else {
 		assert.Equal(409, code, "Change user profile status should be 409")
 	}
-	// case 4: change user2 profile with user2 auth, right parameters format.
+	// case 5: change user2 profile with user2 auth, right parameters format.
 	profile.Realname = "test user"
 	profile.Email = "testUser0002@vmware.com"
 	profile.Comment = "change profile"

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{})
 }

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, "")

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

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,
 	})
 }
 

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,
 	})
 

src/testing/apitests/apilib/user_profile.go

@@ -23,9 +23,8 @@
 package apilib
 
 type UserProfile struct {
-	Email string `json:"email,omitempty"`
-
+	UserID   int    `json:"user_id,omitempty"`
+	Email    string `json:"email,omitempty"`
 	Realname string `json:"realname,omitempty"`
-
-	Comment string `json:"comment,omitempty"`
+	Comment  string `json:"comment,omitempty"`
 }