Get rid of case-sensitivity in authproxy setting

This commit removes the attribute to control case-sensitivity from
authproxy setting.
The result in token review status will be used as the single source of
truth, regardless the case of the letters in group names and user names.

Signed-off-by: Daniel Jiang <jiangd@vmware.com>

src/common/config/metadata/metadatalist.go

@@ -140,7 +140,6 @@ var (
 		{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{}},
-		{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,7 +105,6 @@ const (
 	HTTPAuthProxyTokenReviewEndpoint = "http_authproxy_tokenreview_endpoint"
 	HTTPAuthProxyVerifyCert          = "http_authproxy_verify_cert"
 	HTTPAuthProxySkipSearch          = "http_authproxy_skip_search"
-	HTTPAuthProxyCaseSensitive       = "http_authproxy_case_sensitive"
 	HTTPAuthProxyServerCertificate   = "http_authproxy_server_certificate"
 	OIDCName                         = "oidc_name"
 	OIDCEndpoint                     = "oidc_endpoint"

src/common/models/config.go

@@ -74,7 +74,6 @@ type HTTPAuthProxy struct {
 	VerifyCert          bool   `json:"verify_cert"`
 	SkipSearch          bool   `json:"skip_search"`
 	ServerCertificate   string `json:"server_certificate"`
-	CaseSensitive       bool   `json:"case_sensitive"`
 }
 
 // OIDCSetting wraps the settings for OIDC auth endpoint

src/core/api/config.go

@@ -126,7 +126,7 @@ func (c *ConfigAPI) validateCfg(cfgs map[string]interface{}) (bool, error) {
 		return true, err
 	}
 	if !flag {
-		if failedKeys := checkUnmodifiable(c.cfgManager, cfgs, common.AUTHMode, common.HTTPAuthProxyCaseSensitive); len(failedKeys) > 0 {
+		if failedKeys := checkUnmodifiable(c.cfgManager, cfgs, common.AUTHMode); len(failedKeys) > 0 {
 			return false, fmt.Errorf("the keys %v can not be modified as new users have been inserted into database", failedKeys)
 		}
 	}

src/core/auth/authproxy/auth.go

@@ -53,12 +53,8 @@ type Auth struct {
 	Endpoint            string
 	TokenReviewEndpoint string
 	SkipSearch          bool
-	// When this attribute is set to false, the name of user/group will be converted to lower-case when onboarded to Harbor, so
+	settingTimeStamp    time.Time
-	// as long as the authentication is successful there's no difference in terms of upper or lower case that is used.
+	client              *http.Client
-	// It will be mapped to one entry in Harbor's User/Group table.
-	CaseSensitive    bool
-	settingTimeStamp time.Time
-	client           *http.Client
 }
 
 type session struct {
@@ -83,26 +79,25 @@ func (a *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 	if err != nil {
 		return nil, err
 	}
+	data, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		log.Warningf("Failed to read response body, error: %v", err)
+		return nil, auth.ErrAuth{}
+	}
 	defer resp.Body.Close()
 	if resp.StatusCode == http.StatusOK {
-		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)
-			return nil, auth.ErrAuth{}
-		}
 		s := session{}
 		err = json.Unmarshal(data, &s)
 		if err != nil {
 			return nil, auth.NewErrAuth(fmt.Sprintf("failed to read session %v", err))
 		}
-		if err := a.tokenReview(s.SessionID, user); err != nil {
+		user, err := a.tokenReview(s.SessionID)
-			return nil, auth.NewErrAuth(err.Error())
+		if err != nil {
+			return nil, auth.NewErrAuth(fmt.Sprintf("failed to do token review, error: %v", err))
 		}
 		return user, nil
 	} else if resp.StatusCode == http.StatusUnauthorized {
-		return nil, auth.ErrAuth{}
+		return nil, auth.NewErrAuth(string(data))
 	} else {
 		data, err := ioutil.ReadAll(resp.Body)
 		if err != nil {
@@ -112,26 +107,24 @@ func (a *Auth) Authenticate(m models.AuthModel) (*models.User, error) {
 	}
 }
 
-func (a *Auth) tokenReview(sessionID string, user *models.User) error {
+func (a *Auth) tokenReview(sessionID string) (*models.User, error) {
 	httpAuthProxySetting, err := config.HTTPAuthProxySetting()
 	if err != nil {
-		return err
+		return nil, err
 	}
 	reviewStatus, err := authproxy.TokenReview(sessionID, httpAuthProxySetting)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	u2, err := authproxy.UserFromReviewStatus(reviewStatus)
+	u, err := authproxy.UserFromReviewStatus(reviewStatus)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	user.GroupIDs = u2.GroupIDs
+	return u, nil
-	return nil
 }
 
 // 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)
 }
 
@@ -154,7 +147,6 @@ func (a *Auth) SearchUser(username string) (*models.User, error) {
 	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}
@@ -171,7 +163,6 @@ 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{
@@ -190,7 +181,6 @@ 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
@@ -248,13 +238,6 @@ func getTLSConfig(setting *models.HTTPAuthProxy) (*tls.Config, error) {
 	return &tls.Config{InsecureSkipVerify: !setting.VerifyCert}, 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

@@ -44,13 +44,11 @@ func TestMain(m *testing.M) {
 	a = &Auth{
 		Endpoint:            mockSvr.URL + "/test/login",
 		TokenReviewEndpoint: mockSvr.URL + "/test/tokenreview",
-		CaseSensitive:       false,
 	}
 	cfgMap := cut.GetUnitTestConfig()
 	conf := map[string]interface{}{
 		common.HTTPAuthProxyEndpoint:            a.Endpoint,
 		common.HTTPAuthProxyTokenReviewEndpoint: a.TokenReviewEndpoint,
-		common.HTTPAuthProxyCaseSensitive:       a.CaseSensitive,
 		common.HTTPAuthProxyVerifyCert:          false,
 		common.PostGreSQLSSLMode:                cfgMap[common.PostGreSQLSSLMode],
 		common.PostGreSQLUsername:               cfgMap[common.PostGreSQLUsername],
@@ -200,7 +198,7 @@ func TestAuth_OnBoardGroup(t *testing.T) {
 	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)
+	assert.Equal(t, "OnBoardTest", g.GroupName)
 
 	emptyGroup := &models.UserGroup{}
 	err := a.OnBoardGroup(emptyGroup, "")
@@ -226,7 +224,6 @@ func TestGetTLSConfig(t *testing.T) {
 				VerifyCert:          false,
 				SkipSearch:          false,
 				ServerCertificate:   "",
-				CaseSensitive:       false,
 			},
 			expect: result{
 				hasError:  false,
@@ -241,7 +238,6 @@ func TestGetTLSConfig(t *testing.T) {
 				VerifyCert:          false,
 				SkipSearch:          false,
 				ServerCertificate:   "This does not look like a cert",
-				CaseSensitive:       false,
 			},
 			expect: result{
 				hasError:  false,
@@ -256,7 +252,6 @@ func TestGetTLSConfig(t *testing.T) {
 				VerifyCert:          true,
 				SkipSearch:          false,
 				ServerCertificate:   "This does not look like a cert",
-				CaseSensitive:       false,
 			},
 			expect: result{
 				hasError: true,
@@ -269,7 +264,6 @@ func TestGetTLSConfig(t *testing.T) {
 				VerifyCert:          true,
 				SkipSearch:          false,
 				ServerCertificate:   "",
-				CaseSensitive:       false,
 			},
 			expect: result{
 				hasError:  false,
@@ -315,7 +309,6 @@ pkgODrJUf0p5dhcnLyA2nZolRV1rtwlgJstnEV4JpG1MwtmAZYZUilLvnfpVxTtA
 y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk=
 -----END CERTIFICATE-----
 `,
-				CaseSensitive: false,
 			},
 			expect: result{
 				hasError:  false,

src/core/auth/authproxy/test/server.go

@@ -15,14 +15,30 @@
 package test
 
 import (
+	"encoding/json"
+	"fmt"
+	"github.com/goharbor/harbor/src/common/utils"
+	"io/ioutil"
+	"k8s.io/api/authentication/v1beta1"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 )
 
-type authHandler struct {
+type userEntry struct {
-	m map[string]string
+	username     string
+	password     string
+	sessionID    string
+	reviewStatus string
 }
 
+type authHandler struct {
+	entries []userEntry
+	m       map[string]string
+}
+
+var reviewStatusTpl = `{"apiVersion": "authentication.k8s.io/v1beta1", "kind": "TokenReview", "status": {"authenticated": true, "user": {"username": "%s", "groups": ["vsphere.local\\users", "vsphere.local\\administrators", "vsphere.local\\caadmins", "vsphere.local\\systemconfiguration.bashshelladministrators", "vsphere.local\\systemconfiguration.administrators", "vsphere.local\\licenseservice.administrators", "vsphere.local\\everyone"], "extra": {"method": ["basic"]}}}}`
+
 // ServeHTTP handles HTTP requests
 func (ah *authHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 	if req.Method != http.MethodPost {
@@ -34,27 +50,64 @@ func (ah *authHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 	} else if pass, ok := ah.m[u]; !ok || pass != p {
 		http.Error(rw, "", http.StatusUnauthorized)
 	} else {
-		_, e := rw.Write([]byte(`{"session_id": "hgx59wuWI3b0jcbtidv5mU1YCp-DOQ9NKR1iYKACdKCvbVn7"}`))
+		for _, e := range ah.entries {
-		if e != nil {
+			if e.username == strings.ToLower(u) {
-			panic(e)
+				_, err := rw.Write([]byte(fmt.Sprintf(`{"session_id": "%s"}`, e.sessionID)))
+				if err != nil {
+					panic(err)
+				} else {
+					return
+				}
+			}
 		}
+		http.Error(rw, fmt.Sprintf("Do not find entry in entrylist, username: %s", u), http.StatusUnauthorized)
 	}
 }
 
 type reviewTokenHandler struct {
+	entries []userEntry
 }
 
 func (rth *reviewTokenHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 	if req.Method != http.MethodPost {
 		http.Error(rw, "", http.StatusMethodNotAllowed)
 	}
-	rw.Write([]byte(`{"apiVersion": "authentication.k8s.io/v1beta1", "kind": "TokenReview", "status": {"authenticated": true, "user": {"username": "administrator@vsphere.local", "groups": ["vsphere.local\\users", "vsphere.local\\administrators", "vsphere.local\\caadmins", "vsphere.local\\systemconfiguration.bashshelladministrators", "vsphere.local\\systemconfiguration.administrators", "vsphere.local\\licenseservice.administrators", "vsphere.local\\everyone"], "extra": {"method": ["basic"]}}}}`))
+	bodyBytes, err := ioutil.ReadAll(req.Body)
+	if err != nil {
+		http.Error(rw, fmt.Sprintf("failed to read request body, error: %v", err), http.StatusBadRequest)
+	}
+	reviewData := &v1beta1.TokenReview{}
+	if err := json.Unmarshal(bodyBytes, reviewData); err != nil {
+		http.Error(rw, fmt.Sprintf("failed to decode request body, error: %v", err), http.StatusBadRequest)
+	}
+	defer req.Body.Close()
+	for _, e := range rth.entries {
+		if reviewData.Spec.Token == e.sessionID {
+			_, err := rw.Write([]byte(fmt.Sprintf(reviewStatusTpl, e.username)))
+			if err != nil {
+				panic(err)
+			} else {
+				return
+			}
+		}
+	}
+	http.Error(rw, fmt.Sprintf("failed to match token: %s, entrylist: %+v", reviewData.Spec.Token, rth.entries), http.StatusUnauthorized)
 }
 
 // NewMockServer creates the mock server for testing
 func NewMockServer(creds map[string]string) *httptest.Server {
 	mux := http.NewServeMux()
-	mux.Handle("/test/login", &authHandler{m: creds})
+	entryList := []userEntry{}
-	mux.Handle("/test/tokenreview", &reviewTokenHandler{})
+	for user, pwd := range creds {
+		e := userEntry{
+			username:     strings.ToLower(user),
+			password:     pwd,
+			sessionID:    utils.GenerateRandomString(),
+			reviewStatus: fmt.Sprintf(reviewStatusTpl, user),
+		}
+		entryList = append(entryList, e)
+	}
+	mux.Handle("/test/login", &authHandler{m: creds, entries: entryList})
+	mux.Handle("/test/tokenreview", &reviewTokenHandler{entries: entryList})
 	return httptest.NewTLSServer(mux)
 }

src/core/config/config.go

@@ -476,7 +476,6 @@ func HTTPAuthProxySetting() (*models.HTTPAuthProxy, error) {
 		VerifyCert:          cfgMgr.Get(common.HTTPAuthProxyVerifyCert).GetBool(),
 		SkipSearch:          cfgMgr.Get(common.HTTPAuthProxySkipSearch).GetBool(),
 		ServerCertificate:   cfgMgr.Get(common.HTTPAuthProxyServerCertificate).GetString(),
-		CaseSensitive:       cfgMgr.Get(common.HTTPAuthProxyCaseSensitive).GetBool(),
 	}, nil
 }
 

src/core/config/config_test.go

@@ -251,7 +251,6 @@ y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk=
 	m := map[string]interface{}{
 		common.HTTPAuthProxySkipSearch:        "true",
 		common.HTTPAuthProxyVerifyCert:        "true",
-		common.HTTPAuthProxyCaseSensitive:     "false",
 		common.HTTPAuthProxyEndpoint:          "https://auth.proxy/suffix",
 		common.HTTPAuthProxyServerCertificate: certificate,
 	}
@@ -262,7 +261,6 @@ y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk=
 		Endpoint:          "https://auth.proxy/suffix",
 		SkipSearch:        true,
 		VerifyCert:        true,
-		CaseSensitive:     false,
 		ServerCertificate: certificate,
 	})
 }

src/core/filter/security_test.go

@@ -257,7 +257,6 @@ func TestAuthProxyReqCtxModifier(t *testing.T) {
 		Endpoint:            "https://auth.proxy/suffix",
 		SkipSearch:          true,
 		VerifyCert:          false,
-		CaseSensitive:       true,
 		TokenReviewEndpoint: server.URL,
 	})
 

src/pkg/authproxy/http_test.go

@@ -132,7 +132,6 @@ y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk=
 				VerifyCert:          false,
 				SkipSearch:          false,
 				ServerCertificate:   "",
-				CaseSensitive:       false,
 			},
 			expect: rest.TLSClientConfig{
 				Insecure: true,