diff --git a/src/common/config/metadata/metadatalist.go b/src/common/config/metadata/metadatalist.go index cce026488..07bbd0d0a 100644 --- a/src/common/config/metadata/metadatalist.go +++ b/src/common/config/metadata/metadatalist.go @@ -139,7 +139,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{}}, diff --git a/src/common/const.go b/src/common/const.go index 9e5a60707..1dc3937df 100755 --- a/src/common/const.go +++ b/src/common/const.go @@ -104,7 +104,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" diff --git a/src/common/models/config.go b/src/common/models/config.go index 87d5924eb..5397fc34a 100644 --- a/src/common/models/config.go +++ b/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 diff --git a/src/core/api/config.go b/src/core/api/config.go index 6e6338d27..eccc65eb6 100644 --- a/src/core/api/config.go +++ b/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) } } diff --git a/src/core/auth/authproxy/auth.go b/src/core/auth/authproxy/auth.go index a71ab3d32..bb4fcc7f7 100644 --- a/src/core/auth/authproxy/auth.go +++ b/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 - // 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 + 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 { - return nil, auth.NewErrAuth(err.Error()) + user, err := a.tokenReview(s.SessionID) + 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 nil + return u, 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{}) } diff --git a/src/core/auth/authproxy/auth_test.go b/src/core/auth/authproxy/auth_test.go index 4edfb7ca4..498d96142 100644 --- a/src/core/auth/authproxy/auth_test.go +++ b/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, diff --git a/src/core/auth/authproxy/test/server.go b/src/core/auth/authproxy/test/server.go index 6fead0196..f9af00e96 100644 --- a/src/core/auth/authproxy/test/server.go +++ b/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 { - m map[string]string +type userEntry struct { + 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"}`)) - if e != nil { - panic(e) + for _, e := range ah.entries { + if e.username == strings.ToLower(u) { + _, 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}) - mux.Handle("/test/tokenreview", &reviewTokenHandler{}) + entryList := []userEntry{} + 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) } diff --git a/src/core/config/config.go b/src/core/config/config.go index 1ba036476..daf66259e 100755 --- a/src/core/config/config.go +++ b/src/core/config/config.go @@ -409,7 +409,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 } diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index 4d62f6745..6974d86b5 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -241,7 +241,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, } @@ -252,7 +251,6 @@ y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk= Endpoint: "https://auth.proxy/suffix", SkipSearch: true, VerifyCert: true, - CaseSensitive: false, ServerCertificate: certificate, }) } diff --git a/src/core/filter/security_test.go b/src/core/filter/security_test.go index 069af33f6..5c23dd7ec 100644 --- a/src/core/filter/security_test.go +++ b/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, }) diff --git a/src/pkg/authproxy/http_test.go b/src/pkg/authproxy/http_test.go index f2d53ee1f..1c6274b73 100644 --- a/src/pkg/authproxy/http_test.go +++ b/src/pkg/authproxy/http_test.go @@ -132,7 +132,6 @@ y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk= VerifyCert: false, SkipSearch: false, ServerCertificate: "", - CaseSensitive: false, }, expect: rest.TLSClientConfig{ Insecure: true,