From 06f2414d1ce15edd7bce2abda75c311de2edb00f Mon Sep 17 00:00:00 2001 From: He Weiwei Date: Thu, 2 Sep 2021 19:05:35 +0800 Subject: [PATCH] fix: use ctx from http request for middlewares (#15523) 1. Use ctx from http request for the readonly middleware. 2. Refactor the AuthenticateHelper to let it get orm from ctx of the http request. 3. Change to use ctx from http request for oidc and authproxy http handlers. Signed-off-by: He Weiwei --- src/controller/gc/callback.go | 18 +++- src/controller/member/controller.go | 7 +- src/controller/usergroup/controller.go | 3 +- src/core/auth/auth_test.go | 13 +-- src/core/auth/authenticator.go | 78 ++++++++-------- src/core/auth/authproxy/auth.go | 55 ++++++------ src/core/auth/authproxy/auth_test.go | 8 +- src/core/auth/db/db.go | 13 +-- src/core/auth/db/db_test.go | 12 +-- src/core/auth/ldap/ldap.go | 48 +++++----- src/core/auth/ldap/ldap_test.go | 88 ++++++++++++------- src/core/auth/oidc/oidc.go | 9 +- src/core/auth/oidc/oidc_test.go | 12 +-- src/core/auth/uaa/uaa.go | 23 +++-- src/core/auth/uaa/uaa_test.go | 28 +++--- src/core/controllers/authproxy_redirect.go | 9 +- src/core/controllers/base.go | 6 +- src/core/controllers/oidc.go | 3 +- src/pkg/ldap/manager.go | 3 +- src/server/middleware/readonly/readonly.go | 7 +- src/server/middleware/repoproxy/proxy.go | 15 ++-- src/server/middleware/security/auth_proxy.go | 5 +- .../middleware/security/auth_proxy_test.go | 13 ++- src/server/middleware/security/basic_auth.go | 2 +- .../middleware/security/basic_auth_test.go | 9 +- src/server/middleware/security/security.go | 5 +- .../middleware/security/security_test.go | 12 ++- 27 files changed, 267 insertions(+), 237 deletions(-) diff --git a/src/controller/gc/callback.go b/src/controller/gc/callback.go index b3379629c..8a9cfaf92 100644 --- a/src/controller/gc/callback.go +++ b/src/controller/gc/callback.go @@ -1,13 +1,27 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package gc import ( "context" "encoding/json" "fmt" - "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/controller/quota" "github.com/goharbor/harbor/src/jobservice/job" + "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg/scheduler" @@ -30,7 +44,7 @@ func gcCallback(ctx context.Context, p string) error { if err := json.Unmarshal([]byte(p), param); err != nil { return fmt.Errorf("failed to unmarshal the param: %v", err) } - _, err := Ctl.Start(orm.Context(), *param, task.ExecutionTriggerSchedule) + _, err := Ctl.Start(ctx, *param, task.ExecutionTriggerSchedule) return err } diff --git a/src/controller/member/controller.go b/src/controller/member/controller.go index ed51976bd..86db65286 100644 --- a/src/controller/member/controller.go +++ b/src/controller/member/controller.go @@ -17,6 +17,7 @@ package member import ( "context" "fmt" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/core/auth" "github.com/goharbor/harbor/src/lib/errors" @@ -142,7 +143,7 @@ func (c *controller) Create(ctx context.Context, projectNameOrID interface{}, re if u != nil { userID = u.UserID } else { - userID, err = auth.SearchAndOnBoardUser(req.MemberUser.Username) + userID, err = auth.SearchAndOnBoardUser(ctx, req.MemberUser.Username) if err != nil { return 0, err } @@ -160,7 +161,7 @@ func (c *controller) Create(ctx context.Context, projectNameOrID interface{}, re member.EntityType = common.GroupMember } else { // If groupname provided, use the provided groupname to name this group - groupID, err := auth.SearchAndOnBoardGroup(req.MemberGroup.LdapGroupDN, req.MemberGroup.GroupName) + groupID, err := auth.SearchAndOnBoardGroup(ctx, req.MemberGroup.LdapGroupDN, req.MemberGroup.GroupName) if err != nil { return 0, err } @@ -174,7 +175,7 @@ func (c *controller) Create(ctx context.Context, projectNameOrID interface{}, re return 0, err } if len(ugs) == 0 { - groupID, err := auth.SearchAndOnBoardGroup(req.MemberGroup.GroupName, "") + groupID, err := auth.SearchAndOnBoardGroup(ctx, req.MemberGroup.GroupName, "") if err != nil { return 0, err } diff --git a/src/controller/usergroup/controller.go b/src/controller/usergroup/controller.go index 5cf899d6c..1d4e9f65d 100644 --- a/src/controller/usergroup/controller.go +++ b/src/controller/usergroup/controller.go @@ -16,6 +16,7 @@ package usergroup import ( "context" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/core/auth" "github.com/goharbor/harbor/src/lib/errors" @@ -87,7 +88,7 @@ func (c *controller) Update(ctx context.Context, id int, groupName string) error func (c *controller) Create(ctx context.Context, group model.UserGroup) (int, error) { if group.GroupType == common.LDAPGroupType { - ldapGroup, err := auth.SearchGroup(group.LdapGroupDN) + ldapGroup, err := auth.SearchGroup(ctx, group.LdapGroupDN) if err == ldap.ErrNotFound || ldapGroup == nil { return 0, errors.BadRequestError(nil).WithMessage("LDAP Group DN is not found: DN:%v", group.LdapGroupDN) } diff --git a/src/core/auth/auth_test.go b/src/core/auth/auth_test.go index dd44d590d..23fcbb234 100644 --- a/src/core/auth/auth_test.go +++ b/src/core/auth/auth_test.go @@ -14,11 +14,12 @@ package auth import ( - "github.com/goharbor/harbor/src/pkg/usergroup/model" + "context" "testing" "time" "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/pkg/usergroup/model" "github.com/stretchr/testify/assert" ) @@ -45,7 +46,7 @@ func TestLock(t *testing.T) { func TestDefaultAuthenticate(t *testing.T) { authHelper := DefaultAuthenticateHelper{} m := models.AuthModel{} - user, err := authHelper.Authenticate(m) + user, err := authHelper.Authenticate(context.TODO(), m) if user != nil || err == nil { t.Fatal("Default implementation should return nil") } @@ -54,7 +55,7 @@ func TestDefaultAuthenticate(t *testing.T) { func TestDefaultOnBoardUser(t *testing.T) { user := &models.User{} authHelper := DefaultAuthenticateHelper{} - err := authHelper.OnBoardUser(user) + err := authHelper.OnBoardUser(context.TODO(), user) if err == nil { t.Fatal("Default implementation should return error") } @@ -62,17 +63,17 @@ func TestDefaultOnBoardUser(t *testing.T) { func TestDefaultMethods(t *testing.T) { authHelper := DefaultAuthenticateHelper{} - _, err := authHelper.SearchUser("sample") + _, err := authHelper.SearchUser(context.TODO(), "sample") if err == nil { t.Fatal("Default implementation should return error") } - _, err = authHelper.SearchGroup("sample") + _, err = authHelper.SearchGroup(context.TODO(), "sample") if err == nil { t.Fatal("Default implementation should return error") } - err = authHelper.OnBoardGroup(&model.UserGroup{}, "sample") + err = authHelper.OnBoardGroup(context.TODO(), &model.UserGroup{}, "sample") if err == nil { t.Fatal("Default implementation should return error") } diff --git a/src/core/auth/authenticator.go b/src/core/auth/authenticator.go index ed15a6d5a..d0bd6795f 100644 --- a/src/core/auth/authenticator.go +++ b/src/core/auth/authenticator.go @@ -25,7 +25,6 @@ import ( "github.com/goharbor/harbor/src/lib/config" libErrors "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" - "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg/user" "github.com/goharbor/harbor/src/pkg/usergroup/model" ) @@ -69,19 +68,19 @@ func NewErrAuth(msg string) ErrAuth { type AuthenticateHelper interface { // Authenticate authenticate the user based on data in m. Only when the error returned is an instance // of ErrAuth, it will be considered a bad credentials, other errors will be treated as server side error. - Authenticate(m models.AuthModel) (*models.User, error) + Authenticate(ctx context.Context, m models.AuthModel) (*models.User, error) // OnBoardUser will check if a user exists in user table, if not insert the user and // put the id in the pointer of user model, if it does exist, fill in the user model based // on the data record of the user - OnBoardUser(u *models.User) error + OnBoardUser(ctx context.Context, u *models.User) error // OnBoardGroup Create a group in harbor DB, if altGroupName is not empty, take the altGroupName as groupName in harbor DB. - OnBoardGroup(g *model.UserGroup, altGroupName string) error + OnBoardGroup(ctx context.Context, g *model.UserGroup, altGroupName string) error // SearchUser Get user information from account repository - SearchUser(username string) (*models.User, error) + SearchUser(ctx context.Context, username string) (*models.User, error) // SearchGroup Search a group based on specific authentication - SearchGroup(groupDN string) (*model.UserGroup, error) + SearchGroup(ctx context.Context, groupDN string) (*model.UserGroup, error) // PostAuthenticate Update user information after authenticate, such as Onboard or sync info etc - PostAuthenticate(u *models.User) error + PostAuthenticate(ctx context.Context, u *models.User) error } // DefaultAuthenticateHelper - default AuthenticateHelper implementation @@ -89,35 +88,35 @@ type DefaultAuthenticateHelper struct { } // Authenticate ... -func (d *DefaultAuthenticateHelper) Authenticate(m models.AuthModel) (*models.User, error) { +func (d *DefaultAuthenticateHelper) Authenticate(ctx context.Context, m models.AuthModel) (*models.User, error) { return nil, ErrNotSupported } // OnBoardUser will check if a user exists in user table, if not insert the user and // put the id in the pointer of user model, if it does exist, fill in the user model based // on the data record of the user -func (d *DefaultAuthenticateHelper) OnBoardUser(u *models.User) error { +func (d *DefaultAuthenticateHelper) OnBoardUser(ctx context.Context, u *models.User) error { return ErrNotSupported } // SearchUser - Get user information from account repository -func (d *DefaultAuthenticateHelper) SearchUser(username string) (*models.User, error) { +func (d *DefaultAuthenticateHelper) SearchUser(ctx context.Context, username string) (*models.User, error) { log.Errorf("Not support searching user, username: %s", username) return nil, libErrors.NotFoundError(ErrNotSupported).WithMessage("%s not found", username) } // PostAuthenticate - Update user information after authenticate, such as OnBoard or sync info etc -func (d *DefaultAuthenticateHelper) PostAuthenticate(u *models.User) error { +func (d *DefaultAuthenticateHelper) PostAuthenticate(ctx context.Context, u *models.User) error { return nil } // OnBoardGroup - OnBoardGroup, it will set the ID of the user group, if altGroupName is not empty, take the altGroupName as groupName in harbor DB. -func (d *DefaultAuthenticateHelper) OnBoardGroup(u *model.UserGroup, altGroupName string) error { +func (d *DefaultAuthenticateHelper) OnBoardGroup(ctx context.Context, u *model.UserGroup, altGroupName string) error { return ErrNotSupported } // SearchGroup - Search ldap group by group key, groupKey is the unique attribute of group in authenticator, for LDAP, the key is group DN -func (d *DefaultAuthenticateHelper) SearchGroup(groupKey string) (*model.UserGroup, error) { +func (d *DefaultAuthenticateHelper) SearchGroup(ctx context.Context, groupKey string) (*model.UserGroup, error) { log.Errorf("Not support searching group, group key: %s", groupKey) return nil, libErrors.NotFoundError(ErrNotSupported).WithMessage("%s not found", groupKey) } @@ -135,8 +134,7 @@ func Register(name string, h AuthenticateHelper) { } // Login authenticates user credentials based on setting. -func Login(m models.AuthModel) (*models.User, error) { - ctx := orm.Context() +func Login(ctx context.Context, m models.AuthModel) (*models.User, error) { authMode, err := config.AuthMode(ctx) if err != nil { return nil, err @@ -154,7 +152,7 @@ func Login(m models.AuthModel) (*models.User, error) { log.Debugf("%s is locked due to login failure, login failed", m.Principal) return nil, nil } - user, err := authenticator.Authenticate(m) + user, err := authenticator.Authenticate(ctx, m) if err != nil { if _, ok = err.(ErrAuth); ok { log.Debugf("Login failed, locking %s, and sleep for %v", m.Principal, frozenTime) @@ -163,12 +161,12 @@ func Login(m models.AuthModel) (*models.User, error) { } return nil, err } - err = authenticator.PostAuthenticate(user) + err = authenticator.PostAuthenticate(ctx, user) return user, err } -func getHelper() (AuthenticateHelper, error) { - authMode, err := config.AuthMode(orm.Context()) +func getHelper(ctx context.Context) (AuthenticateHelper, error) { + authMode, err := config.AuthMode(ctx) if err != nil { return nil, err } @@ -181,52 +179,52 @@ func getHelper() (AuthenticateHelper, error) { // OnBoardUser will check if a user exists in user table, if not insert the user and // put the id in the pointer of user model, if it does exist, return the user's profile. -func OnBoardUser(user *models.User) error { +func OnBoardUser(ctx context.Context, user *models.User) error { log.Debugf("OnBoardUser, user %+v", user) - helper, err := getHelper() + helper, err := getHelper(ctx) if err != nil { return err } - return helper.OnBoardUser(user) + return helper.OnBoardUser(ctx, user) } // SearchUser -- -func SearchUser(username string) (*models.User, error) { - helper, err := getHelper() +func SearchUser(ctx context.Context, username string) (*models.User, error) { + helper, err := getHelper(ctx) if err != nil { return nil, err } - return helper.SearchUser(username) + return helper.SearchUser(ctx, username) } // OnBoardGroup - Create a user group in harbor db, if altGroupName is not empty, take the altGroupName as groupName in harbor DB -func OnBoardGroup(userGroup *model.UserGroup, altGroupName string) error { - helper, err := getHelper() +func OnBoardGroup(ctx context.Context, userGroup *model.UserGroup, altGroupName string) error { + helper, err := getHelper(ctx) if err != nil { return err } - return helper.OnBoardGroup(userGroup, altGroupName) + return helper.OnBoardGroup(ctx, userGroup, altGroupName) } // SearchGroup -- Search group in authenticator, groupKey is the unique attribute of group in authenticator, for LDAP, the key is group DN -func SearchGroup(groupKey string) (*model.UserGroup, error) { - helper, err := getHelper() +func SearchGroup(ctx context.Context, groupKey string) (*model.UserGroup, error) { + helper, err := getHelper(ctx) if err != nil { return nil, err } - return helper.SearchGroup(groupKey) + return helper.SearchGroup(ctx, groupKey) } // SearchAndOnBoardUser ... Search user and OnBoard user, if user exist, return the ID of current user. -func SearchAndOnBoardUser(username string) (int, error) { - user, err := SearchUser(username) +func SearchAndOnBoardUser(ctx context.Context, username string) (int, error) { + user, err := SearchUser(ctx, username) if err != nil { return 0, err } if user == nil { return 0, libErrors.NotFoundError(nil).WithMessage(fmt.Sprintf("user %s is not found", username)) } - err = OnBoardUser(user) + err = OnBoardUser(ctx, user) if err != nil { return 0, err } @@ -234,8 +232,8 @@ func SearchAndOnBoardUser(username string) (int, error) { } // SearchAndOnBoardGroup ... if altGroupName is not empty, take the altGroupName as groupName in harbor DB -func SearchAndOnBoardGroup(groupKey, altGroupName string) (int, error) { - userGroup, err := SearchGroup(groupKey) +func SearchAndOnBoardGroup(ctx context.Context, groupKey, altGroupName string) (int, error) { + userGroup, err := SearchGroup(ctx, groupKey) if err != nil { return 0, err } @@ -243,18 +241,18 @@ func SearchAndOnBoardGroup(groupKey, altGroupName string) (int, error) { return 0, ErrorGroupNotExist } if userGroup != nil { - err = OnBoardGroup(userGroup, altGroupName) + err = OnBoardGroup(ctx, userGroup, altGroupName) } return userGroup.ID, err } // PostAuthenticate - -func PostAuthenticate(u *models.User) error { - helper, err := getHelper() +func PostAuthenticate(ctx context.Context, u *models.User) error { + helper, err := getHelper(ctx) if err != nil { return err } - return helper.PostAuthenticate(u) + return helper.PostAuthenticate(ctx, u) } // IsSuperUser checks if the user is super user(conventionally id == 1) of Harbor diff --git a/src/core/auth/authproxy/auth.go b/src/core/auth/authproxy/auth.go index 143da39a3..4aaeb507e 100644 --- a/src/core/auth/authproxy/auth.go +++ b/src/core/auth/authproxy/auth.go @@ -15,6 +15,7 @@ package authproxy import ( + "context" "crypto/tls" "crypto/x509" "encoding/json" @@ -26,20 +27,18 @@ import ( "sync" "time" - "github.com/goharbor/harbor/src/jobservice/logger" - "github.com/goharbor/harbor/src/lib/config" - cfgModels "github.com/goharbor/harbor/src/lib/config/models" - harborErrors "github.com/goharbor/harbor/src/lib/errors" - "github.com/goharbor/harbor/src/lib/orm" - "github.com/goharbor/harbor/src/pkg/usergroup/model" - "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/controller/usergroup" "github.com/goharbor/harbor/src/core/auth" + "github.com/goharbor/harbor/src/jobservice/logger" + "github.com/goharbor/harbor/src/lib/config" + cfgModels "github.com/goharbor/harbor/src/lib/config/models" + harborErrors "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/pkg/authproxy" "github.com/goharbor/harbor/src/pkg/user" + "github.com/goharbor/harbor/src/pkg/usergroup/model" ) const refreshDuration = 2 * time.Second @@ -67,8 +66,8 @@ type session struct { } // Authenticate issues http POST request to Endpoint if it returns 200 the authentication is considered success. -func (a *Auth) Authenticate(m models.AuthModel) (*models.User, error) { - err := a.ensure() +func (a *Auth) Authenticate(ctx context.Context, m models.AuthModel) (*models.User, error) { + err := a.ensure(ctx) if err != nil { if a.Endpoint == "" { return nil, fmt.Errorf("failed to initialize HTTP Auth Proxy Authenticator, error: %v", err) @@ -96,7 +95,7 @@ func (a *Auth) Authenticate(m models.AuthModel) (*models.User, error) { if err != nil { return nil, auth.NewErrAuth(fmt.Sprintf("failed to read session %v", err)) } - user, err := a.tokenReview(s.SessionID) + user, err := a.tokenReview(ctx, s.SessionID) if err != nil { return nil, auth.NewErrAuth(fmt.Sprintf("failed to do token review, error: %v", err)) } @@ -112,8 +111,8 @@ func (a *Auth) Authenticate(m models.AuthModel) (*models.User, error) { } } -func (a *Auth) tokenReview(sessionID string) (*models.User, error) { - httpAuthProxySetting, err := config.HTTPAuthProxySetting(orm.Context()) +func (a *Auth) tokenReview(ctx context.Context, sessionID string) (*models.User, error) { + httpAuthProxySetting, err := config.HTTPAuthProxySetting(ctx) if err != nil { return nil, err } @@ -129,26 +128,26 @@ func (a *Auth) tokenReview(sessionID string) (*models.User, error) { } // VerifyToken reviews the token to generate the user model -func (a *Auth) VerifyToken(token string) (*models.User, error) { - if err := a.ensure(); err != nil { +func (a *Auth) VerifyToken(ctx context.Context, token string) (*models.User, error) { + if err := a.ensure(ctx); err != nil { return nil, err } - return a.tokenReview(token) + return a.tokenReview(ctx, token) } // OnBoardUser delegates to dao pkg to insert/update data in DB. -func (a *Auth) OnBoardUser(u *models.User) error { - return a.userMgr.Onboard(orm.Context(), u) +func (a *Auth) OnBoardUser(ctx context.Context, u *models.User) error { + return a.userMgr.Onboard(ctx, u) } // PostAuthenticate generates the user model and on board the user. -func (a *Auth) PostAuthenticate(u *models.User) error { - _, err := a.userMgr.GetByName(orm.Context(), u.Username) +func (a *Auth) PostAuthenticate(ctx context.Context, u *models.User) error { + _, err := a.userMgr.GetByName(ctx, u.Username) if harborErrors.IsNotFoundErr(err) { if err2 := a.fillInModel(u); err2 != nil { return err2 } - return a.OnBoardUser(u) + return a.OnBoardUser(ctx, u) } else if err != nil { return err } @@ -159,8 +158,8 @@ 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, // 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() +func (a *Auth) SearchUser(ctx context.Context, username string) (*models.User, error) { + err := a.ensure(ctx) if err != nil { log.Warningf("Failed to refresh configuration for HTTP Auth Proxy Authenticator, error: %v, the default settings will be used", err) } @@ -175,8 +174,8 @@ func (a *Auth) SearchUser(username string) (*models.User, error) { } // SearchGroup search group exist in the authentication provider, for HTTP auth, if SkipSearch is true, it assume this group exist in authentication provider. -func (a *Auth) SearchGroup(groupKey string) (*model.UserGroup, error) { - err := a.ensure() +func (a *Auth) SearchGroup(ctx context.Context, groupKey string) (*model.UserGroup, error) { + err := a.ensure(ctx) if err != nil { log.Warningf("Failed to refresh configuration for HTTP Auth Proxy Authenticator, error: %v, the default settings will be used", err) } @@ -192,13 +191,13 @@ func (a *Auth) SearchGroup(groupKey string) (*model.UserGroup, error) { } // OnBoardGroup create user group entity in Harbor DB, altGroupName is not used. -func (a *Auth) OnBoardGroup(u *model.UserGroup, altGroupName string) error { +func (a *Auth) OnBoardGroup(ctx context.Context, u *model.UserGroup, altGroupName string) error { // if group name provided, on board the user group if len(u.GroupName) == 0 { return errors.New("should provide a group name") } u.GroupType = common.HTTPGroupType - err := usergroup.Ctl.Ensure(orm.Context(), u) + err := usergroup.Ctl.Ensure(ctx, u) if err != nil { return err } @@ -218,14 +217,14 @@ func (a *Auth) fillInModel(u *models.User) error { return nil } -func (a *Auth) ensure() error { +func (a *Auth) ensure(ctx context.Context) error { a.Lock() defer a.Unlock() if a.client == nil { a.client = &http.Client{} } if time.Now().Sub(a.settingTimeStamp) >= refreshDuration { - setting, err := config.HTTPAuthProxySetting(orm.Context()) + setting, err := config.HTTPAuthProxySetting(ctx) if err != nil { return err } diff --git a/src/core/auth/authproxy/auth_test.go b/src/core/auth/authproxy/auth_test.go index bcea9c2a4..266937a67 100644 --- a/src/core/auth/authproxy/auth_test.go +++ b/src/core/auth/authproxy/auth_test.go @@ -139,7 +139,7 @@ func TestAuth_Authenticate(t *testing.T) { } assert := assert.New(t) for _, c := range suite { - r, e := a.Authenticate(c.input) + r, e := a.Authenticate(orm.Context(), c.input) if c.expect.err == nil { assert.Nil(e) assert.Equal(c.expect.user, *r) @@ -185,7 +185,7 @@ func TestAuth_PostAuthenticate(t *testing.T) { }, } for _, c := range suite { - a.PostAuthenticate(c.input) + a.PostAuthenticate(orm.Context(), c.input) assert.Equal(t, c.expect.Username, c.input.Username) assert.Equal(t, c.expect.Email, c.input.Email) assert.Equal(t, c.expect.Realname, c.input.Realname) @@ -199,7 +199,7 @@ func TestAuth_OnBoardGroup(t *testing.T) { GroupName: "OnBoardTest", GroupType: common.HTTPGroupType, } - a.OnBoardGroup(input, "") + a.OnBoardGroup(orm.Context(), input, "") assert.True(t, input.ID > 0, "The OnBoardGroup should have a valid group ID") g, er := usergroup.Mgr.Get(orm.Context(), input.ID) @@ -207,7 +207,7 @@ func TestAuth_OnBoardGroup(t *testing.T) { assert.Equal(t, "OnBoardTest", g.GroupName) emptyGroup := &model.UserGroup{} - err := a.OnBoardGroup(emptyGroup, "") + err := a.OnBoardGroup(orm.Context(), emptyGroup, "") if err == nil { t.Fatal("Empty user group should failed to OnBoard") } diff --git a/src/core/auth/db/db.go b/src/core/auth/db/db.go index a55bee155..715c06705 100644 --- a/src/core/auth/db/db.go +++ b/src/core/auth/db/db.go @@ -15,11 +15,12 @@ package db import ( + "context" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/core/auth" "github.com/goharbor/harbor/src/lib/errors" - "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg/user" ) @@ -30,8 +31,8 @@ type Auth struct { } // Authenticate calls dao to authenticate user. -func (d *Auth) Authenticate(m models.AuthModel) (*models.User, error) { - u, err := d.userMgr.MatchLocalPassword(orm.Context(), m.Principal, m.Password) +func (d *Auth) Authenticate(ctx context.Context, m models.AuthModel) (*models.User, error) { + u, err := d.userMgr.MatchLocalPassword(ctx, m.Principal, m.Password) if err != nil { return nil, err } @@ -42,8 +43,8 @@ func (d *Auth) Authenticate(m models.AuthModel) (*models.User, error) { } // SearchUser - Check if user exist in local db -func (d *Auth) SearchUser(username string) (*models.User, error) { - u, err := d.userMgr.GetByName(orm.Context(), username) +func (d *Auth) SearchUser(ctx context.Context, username string) (*models.User, error) { + u, err := d.userMgr.GetByName(ctx, username) if errors.IsNotFoundErr(err) { return nil, nil } else if err != nil { @@ -53,7 +54,7 @@ func (d *Auth) SearchUser(username string) (*models.User, error) { } // OnBoardUser - -func (d *Auth) OnBoardUser(u *models.User) error { +func (d *Auth) OnBoardUser(ctx context.Context, u *models.User) error { return nil } diff --git a/src/core/auth/db/db_test.go b/src/core/auth/db/db_test.go index ffbccad4c..b8933352b 100644 --- a/src/core/auth/db/db_test.go +++ b/src/core/auth/db/db_test.go @@ -14,23 +14,15 @@ package db import ( - "os" + "context" "testing" "github.com/goharbor/harbor/src/common/models" - "github.com/goharbor/harbor/src/common/utils/test" "github.com/goharbor/harbor/src/testing/mock" testinguserpkg "github.com/goharbor/harbor/src/testing/pkg/user" - testifymock "github.com/stretchr/testify/mock" ) -func TestMain(m *testing.M) { - test.InitDatabaseFromEnv() - retCode := m.Run() - os.Exit(retCode) -} - func TestSearchUser(t *testing.T) { user := &models.User{ UserID: 123, @@ -49,7 +41,7 @@ func TestSearchUser(t *testing.T) { return name == "existuser" })).Return(user, nil) - newUser, err := auth.SearchUser("existuser") + newUser, err := auth.SearchUser(context.TODO(), "existuser") if err != nil { t.Fatalf("Failed to search user, error %v", err) } diff --git a/src/core/auth/ldap/ldap.go b/src/core/auth/ldap/ldap.go index 73c24ec7c..3672f3c24 100644 --- a/src/core/auth/ldap/ldap.go +++ b/src/core/auth/ldap/ldap.go @@ -20,25 +20,21 @@ import ( "regexp" "strings" + goldap "github.com/go-ldap/ldap/v3" + "github.com/goharbor/harbor/src/common" + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/utils" + ldapCtl "github.com/goharbor/harbor/src/controller/ldap" + ugCtl "github.com/goharbor/harbor/src/controller/usergroup" + "github.com/goharbor/harbor/src/core/auth" "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/errors" - "github.com/goharbor/harbor/src/lib/orm" + "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/lib/q" + "github.com/goharbor/harbor/src/pkg/ldap" "github.com/goharbor/harbor/src/pkg/ldap/model" "github.com/goharbor/harbor/src/pkg/user" ugModel "github.com/goharbor/harbor/src/pkg/usergroup/model" - - goldap "github.com/go-ldap/ldap/v3" - "github.com/goharbor/harbor/src/common" - "github.com/goharbor/harbor/src/common/utils" - - "github.com/goharbor/harbor/src/common/models" - ldapCtl "github.com/goharbor/harbor/src/controller/ldap" - ugCtl "github.com/goharbor/harbor/src/controller/usergroup" - "github.com/goharbor/harbor/src/pkg/ldap" - - "github.com/goharbor/harbor/src/core/auth" - "github.com/goharbor/harbor/src/lib/log" ) // Auth implements AuthenticateHelper interface to authenticate against LDAP @@ -50,8 +46,7 @@ type Auth struct { // Authenticate checks user's credential against LDAP based on basedn template and LDAP URL, // if the check is successful a dummy record will be inserted into DB, such that this user can // be associated to other entities in the system. -func (l *Auth) Authenticate(m models.AuthModel) (*models.User, error) { - ctx := orm.Context() +func (l *Auth) Authenticate(ctx context.Context, m models.AuthModel) (*models.User, error) { p := m.Principal if len(strings.TrimSpace(p)) == 0 { log.Debugf("LDAP authentication failed for empty user id.") @@ -135,7 +130,7 @@ func (l *Auth) attachLDAPGroup(ctx context.Context, ldapUsers []model.User, u *m } userGroups = append(userGroups, ugModel.UserGroup{GroupName: lGroups[0].Name, LdapGroupDN: dn, GroupType: common.LDAPGroupType}) } - u.GroupIDs, err = ugCtl.Ctl.Populate(orm.Context(), userGroups) + u.GroupIDs, err = ugCtl.Ctl.Populate(ctx, userGroups) if err != nil { log.Warningf("Failed to fetch ldap group configuration:%v", err) } @@ -155,7 +150,7 @@ func (l *Auth) syncUserInfoFromDB(ctx context.Context, u *models.User) { // OnBoardUser will check if a user exists in user table, if not insert the user and // put the id in the pointer of user model, if it does exist, return the user's profile. -func (l *Auth) OnBoardUser(u *models.User) error { +func (l *Auth) OnBoardUser(ctx context.Context, u *models.User) error { if u.Email == "" { if strings.Contains(u.Username, "@") { u.Email = u.Username @@ -164,13 +159,13 @@ func (l *Auth) OnBoardUser(u *models.User) error { u.Password = "12345678AbC" // Password is not kept in local db u.Comment = "from LDAP." // Source is from LDAP - return l.userMgr.Onboard(orm.Context(), u) + return l.userMgr.Onboard(ctx, u) } // SearchUser -- Search user in ldap -func (l *Auth) SearchUser(username string) (*models.User, error) { +func (l *Auth) SearchUser(ctx context.Context, username string) (*models.User, error) { var user models.User - s, err := ldapCtl.Ctl.Session(orm.Context()) + s, err := ldapCtl.Ctl.Session(ctx) if err != nil { return nil, err } @@ -201,11 +196,11 @@ func (l *Auth) SearchUser(username string) (*models.User, error) { } // SearchGroup -- Search group in ldap authenticator, groupKey is LDAP group DN. -func (l *Auth) SearchGroup(groupKey string) (*ugModel.UserGroup, error) { +func (l *Auth) SearchGroup(ctx context.Context, groupKey string) (*ugModel.UserGroup, error) { if _, err := goldap.ParseDN(groupKey); err != nil { return nil, auth.ErrInvalidLDAPGroupDN } - s, err := ldapCtl.Ctl.Session(orm.Context()) + s, err := ldapCtl.Ctl.Session(ctx) if err != nil { return nil, fmt.Errorf("can not load system ldap config: %v", err) @@ -234,8 +229,7 @@ func (l *Auth) SearchGroup(groupKey string) (*ugModel.UserGroup, error) { } // OnBoardGroup -- Create Group in harbor DB, if altGroupName is not empty, take the altGroupName as groupName in harbor DB. -func (l *Auth) OnBoardGroup(u *ugModel.UserGroup, altGroupName string) error { - ctx := orm.Context() +func (l *Auth) OnBoardGroup(ctx context.Context, u *ugModel.UserGroup, altGroupName string) error { if _, err := goldap.ParseDN(u.LdapGroupDN); err != nil { return auth.ErrInvalidLDAPGroupDN } @@ -255,9 +249,7 @@ func (l *Auth) OnBoardGroup(u *ugModel.UserGroup, altGroupName string) error { } // PostAuthenticate -- If user exist in harbor DB, sync email address, if not exist, call OnBoardUser -func (l *Auth) PostAuthenticate(u *models.User) error { - - ctx := orm.Context() +func (l *Auth) PostAuthenticate(ctx context.Context, u *models.User) error { query := q.New(q.KeyWords{"Username": u.Username}) n, err := l.userMgr.Count(ctx, query) if err != nil { @@ -288,7 +280,7 @@ func (l *Auth) PostAuthenticate(u *models.User) error { return nil } - err = auth.OnBoardUser(u) + err = auth.OnBoardUser(ctx, u) if err != nil { return err } diff --git a/src/core/auth/ldap/ldap_test.go b/src/core/auth/ldap/ldap_test.go index 53069f0ee..1cc15682a 100644 --- a/src/core/auth/ldap/ldap_test.go +++ b/src/core/auth/ldap/ldap_test.go @@ -14,28 +14,26 @@ package ldap import ( - "github.com/goharbor/harbor/src/pkg/project" "os" "testing" - "github.com/goharbor/harbor/src/lib/config" - "github.com/goharbor/harbor/src/lib/orm" - _ "github.com/goharbor/harbor/src/pkg/config/db" - _ "github.com/goharbor/harbor/src/pkg/config/inmemory" - userpkg "github.com/goharbor/harbor/src/pkg/user" - "github.com/goharbor/harbor/src/pkg/usergroup" - ugModel "github.com/goharbor/harbor/src/pkg/usergroup/model" - "github.com/stretchr/testify/assert" - "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/utils/test" + "github.com/goharbor/harbor/src/core/auth" + "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/log" + "github.com/goharbor/harbor/src/lib/orm" + _ "github.com/goharbor/harbor/src/pkg/config/db" + _ "github.com/goharbor/harbor/src/pkg/config/inmemory" "github.com/goharbor/harbor/src/pkg/member" memberModels "github.com/goharbor/harbor/src/pkg/member/models" - - "github.com/goharbor/harbor/src/core/auth" + "github.com/goharbor/harbor/src/pkg/project" + userpkg "github.com/goharbor/harbor/src/pkg/user" + "github.com/goharbor/harbor/src/pkg/usergroup" + ugModel "github.com/goharbor/harbor/src/pkg/usergroup/model" + "github.com/stretchr/testify/assert" ) var ldapTestConfig = map[string]interface{}{ @@ -111,10 +109,12 @@ func TestMain(m *testing.M) { } func TestAuthenticate(t *testing.T) { + ctx := orm.Context() + var person models.AuthModel person.Principal = "test" person.Password = "123456" - user, err := authHelper.Authenticate(person) + user, err := authHelper.Authenticate(ctx, person) if err != nil { t.Errorf("unexpected ldap authenticate fail: %v", err) } @@ -123,14 +123,14 @@ func TestAuthenticate(t *testing.T) { } person.Principal = "test" person.Password = "1" - user, err = authHelper.Authenticate(person) + user, err = authHelper.Authenticate(ctx, person) if _, ok := err.(auth.ErrAuth); !ok { t.Errorf("Expected an ErrAuth on wrong password, but got: %v", err) } person.Principal = "" person.Password = "" - user, err = authHelper.Authenticate(person) + user, err = authHelper.Authenticate(ctx, person) if _, ok := err.(auth.ErrAuth); !ok { t.Errorf("Expected an ErrAuth on empty credentials, but got: %v", err) } @@ -139,7 +139,7 @@ func TestAuthenticate(t *testing.T) { Principal: "test", Password: "123456", } - user2, err := authHelper.Authenticate(person2) + user2, err := authHelper.Authenticate(ctx, person2) if err != nil { t.Errorf("unexpected ldap error: %v", err) @@ -151,8 +151,10 @@ func TestAuthenticate(t *testing.T) { } func TestSearchUser(t *testing.T) { + ctx := orm.Context() + var username = "test" - user, err := authHelper.SearchUser(username) + user, err := authHelper.SearchUser(ctx, username) if err != nil { t.Errorf("Search user failed %v", err) } @@ -161,10 +163,12 @@ func TestSearchUser(t *testing.T) { } } func TestAuthenticateWithAdmin(t *testing.T) { + ctx := orm.Context() + var person models.AuthModel person.Principal = "mike" person.Password = "zhu88jie" - user, err := authHelper.Authenticate(person) + user, err := authHelper.Authenticate(ctx, person) if err != nil { t.Errorf("unexpected ldap authenticate fail: %v", err) } @@ -176,10 +180,12 @@ func TestAuthenticateWithAdmin(t *testing.T) { } } func TestAuthenticateWithoutAdmin(t *testing.T) { + ctx := orm.Context() + var person models.AuthModel person.Principal = "user001" person.Password = "Test1@34" - user, err := authHelper.Authenticate(person) + user, err := authHelper.Authenticate(ctx, person) if err != nil { t.Errorf("unexpected ldap authenticate fail: %v", err) } @@ -191,8 +197,10 @@ func TestAuthenticateWithoutAdmin(t *testing.T) { } } func TestSearchUser_02(t *testing.T) { + ctx := orm.Context() + var username = "nonexist" - user, _ := authHelper.SearchUser(username) + user, _ := authHelper.SearchUser(ctx, username) if user != nil { t.Errorf("Should failed to search nonexist user") } @@ -200,12 +208,14 @@ func TestSearchUser_02(t *testing.T) { } func TestOnBoardUser(t *testing.T) { + ctx := orm.Context() + user := &models.User{ Username: "sample", Email: "sample@example.com", Realname: "Sample", } - err := authHelper.OnBoardUser(user) + err := authHelper.OnBoardUser(ctx, user) if err != nil { t.Errorf("Failed to onboard user") } @@ -216,11 +226,13 @@ func TestOnBoardUser(t *testing.T) { } func TestOnBoardUser_02(t *testing.T) { + ctx := orm.Context() + user := &models.User{ Username: "sample02", Realname: "Sample02", } - err := authHelper.OnBoardUser(user) + err := authHelper.OnBoardUser(ctx, user) if err != nil { t.Errorf("Failed to onboard user") } @@ -233,11 +245,13 @@ func TestOnBoardUser_02(t *testing.T) { } func TestOnBoardUser_03(t *testing.T) { + ctx := orm.Context() + user := &models.User{ Username: "sample03@example.com", Realname: "Sample03", } - err := authHelper.OnBoardUser(user) + err := authHelper.OnBoardUser(ctx, user) if err != nil { t.Errorf("Failed to onboard user") } @@ -250,13 +264,15 @@ func TestOnBoardUser_03(t *testing.T) { } func TestAuthenticateHelperOnBoardUser(t *testing.T) { + ctx := orm.Context() + user := models.User{ Username: "test01", Realname: "test01", Email: "test01@example.com", } - err := auth.OnBoardUser(&user) + err := auth.OnBoardUser(ctx, &user) if err != nil { t.Errorf("Failed to onboard user error: %v", err) } @@ -268,12 +284,14 @@ func TestAuthenticateHelperOnBoardUser(t *testing.T) { } func TestOnBoardGroup(t *testing.T) { + ctx := orm.Context() + group := ugModel.UserGroup{ GroupName: "harbor_group2", LdapGroupDN: "cn=harbor_group2,ou=groups,dc=example,dc=com", } newGroupName := "group_name123" - err := auth.OnBoardGroup(&group, newGroupName) + err := auth.OnBoardGroup(ctx, &group, newGroupName) if err != nil { t.Errorf("Failed to OnBoardGroup, %+v", group) } @@ -283,8 +301,9 @@ func TestOnBoardGroup(t *testing.T) { } func TestAuthenticateHelperSearchUser(t *testing.T) { + ctx := orm.Context() - user, err := auth.SearchUser("test") + user, err := auth.SearchUser(ctx, "test") if err != nil { t.Error("Failed to search user, test") } @@ -295,6 +314,7 @@ func TestAuthenticateHelperSearchUser(t *testing.T) { } func TestPostAuthentication(t *testing.T) { + ctx := orm.Context() assert := assert.New(t) user1 := &models.User{ @@ -305,7 +325,7 @@ func TestPostAuthentication(t *testing.T) { queryUsername := "test003" - err := auth.OnBoardUser(user1) + err := auth.OnBoardUser(ctx, user1) assert.Nil(err) user2 := &models.User{ @@ -313,8 +333,8 @@ func TestPostAuthentication(t *testing.T) { Email: "234invalidmail@@@@@", } - auth.PostAuthenticate(user2) - ctx := orm.Context() + auth.PostAuthenticate(ctx, user2) + dbUser, err := userpkg.Mgr.GetByName(ctx, queryUsername) if err != nil { t.Fatalf("Failed to get user, error %v", err) @@ -325,7 +345,7 @@ func TestPostAuthentication(t *testing.T) { Username: "test003", } - auth.PostAuthenticate(user3) + auth.PostAuthenticate(ctx, user3) dbUser, err = userpkg.Mgr.GetByName(ctx, queryUsername) if err != nil { t.Fatalf("Failed to get user, error %v", err) @@ -337,7 +357,7 @@ func TestPostAuthentication(t *testing.T) { Email: "test003@example.com", } - auth.PostAuthenticate(user4) + auth.PostAuthenticate(ctx, user4) dbUser, err = userpkg.Mgr.GetByName(ctx, queryUsername) if err != nil { t.Fatalf("Failed to get user, error %v", err) @@ -347,7 +367,9 @@ func TestPostAuthentication(t *testing.T) { } func TestSearchAndOnBoardUser(t *testing.T) { - userID, err := auth.SearchAndOnBoardUser("mike02") + ctx := orm.Context() + + userID, err := auth.SearchAndOnBoardUser(ctx, "mike02") defer dao.CleanUser(int64(userID)) if err != nil { t.Errorf("Error occurred when SearchAndOnBoardUser: %v", err) @@ -363,7 +385,7 @@ func TestAddProjectMemberWithLdapUser(t *testing.T) { if err != nil { t.Errorf("Error occurred when GetProjectByName: %v", err) } - userID, err := auth.SearchAndOnBoardUser("mike") + userID, err := auth.SearchAndOnBoardUser(ctx, "mike") member := memberModels.Member{ ProjectID: currentProject.ProjectID, EntityType: common.UserMember, diff --git a/src/core/auth/oidc/oidc.go b/src/core/auth/oidc/oidc.go index fe71cb638..eb9ff1c58 100644 --- a/src/core/auth/oidc/oidc.go +++ b/src/core/auth/oidc/oidc.go @@ -1,10 +1,11 @@ package oidc import ( + "context" "fmt" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/core/auth" - "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg/usergroup" "github.com/goharbor/harbor/src/pkg/usergroup/model" ) @@ -15,7 +16,7 @@ type Auth struct { } // SearchGroup is skipped in OIDC mode, so it makes sure any group will be onboarded. -func (a *Auth) SearchGroup(groupKey string) (*model.UserGroup, error) { +func (a *Auth) SearchGroup(ctx context.Context, groupKey string) (*model.UserGroup, error) { return &model.UserGroup{ GroupName: groupKey, GroupType: common.OIDCGroupType, @@ -23,12 +24,12 @@ func (a *Auth) SearchGroup(groupKey string) (*model.UserGroup, error) { } // OnBoardGroup create user group entity in Harbor DB, altGroupName is not used. -func (a *Auth) OnBoardGroup(u *model.UserGroup, altGroupName string) error { +func (a *Auth) OnBoardGroup(ctx context.Context, u *model.UserGroup, altGroupName string) error { // if group name provided, on board the user group if len(u.GroupName) == 0 || u.GroupType != common.OIDCGroupType { return fmt.Errorf("invalid input group for OIDC mode: %v", *u) } - return usergroup.Mgr.Onboard(orm.Context(), u) + return usergroup.Mgr.Onboard(ctx, u) } func init() { diff --git a/src/core/auth/oidc/oidc_test.go b/src/core/auth/oidc/oidc_test.go index 6d2579922..b704e967c 100644 --- a/src/core/auth/oidc/oidc_test.go +++ b/src/core/auth/oidc/oidc_test.go @@ -1,11 +1,13 @@ package oidc import ( + "context" + "os" + "testing" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/pkg/usergroup/model" "github.com/stretchr/testify/assert" - "os" - "testing" ) func TestMain(m *testing.M) { @@ -15,7 +17,7 @@ func TestMain(m *testing.M) { func TestAuth_SearchGroup(t *testing.T) { a := Auth{} - res, err := a.SearchGroup("grp") + res, err := a.SearchGroup(context.TODO(), "grp") assert.Nil(t, err) assert.Equal(t, model.UserGroup{GroupName: "grp", GroupType: common.OIDCGroupType}, *res) } @@ -23,9 +25,9 @@ func TestAuth_SearchGroup(t *testing.T) { func TestAuth_OnBoardGroup(t *testing.T) { a := Auth{} g1 := &model.UserGroup{GroupName: "", GroupType: common.OIDCGroupType} - err1 := a.OnBoardGroup(g1, "") + err1 := a.OnBoardGroup(context.TODO(), g1, "") assert.NotNil(t, err1) g2 := &model.UserGroup{GroupName: "group", GroupType: common.LDAPGroupType} - err2 := a.OnBoardGroup(g2, "") + err2 := a.OnBoardGroup(context.TODO(), g2, "") assert.NotNil(t, err2) } diff --git a/src/core/auth/uaa/uaa.go b/src/core/auth/uaa/uaa.go index 12447ad16..6c923f232 100644 --- a/src/core/auth/uaa/uaa.go +++ b/src/core/auth/uaa/uaa.go @@ -15,6 +15,7 @@ package uaa import ( + "context" "fmt" "os" "strings" @@ -27,7 +28,6 @@ import ( "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" - "github.com/goharbor/harbor/src/lib/orm" userpkg "github.com/goharbor/harbor/src/pkg/user" ) @@ -40,8 +40,8 @@ type Auth struct { } // Authenticate ... -func (u *Auth) Authenticate(m models.AuthModel) (*models.User, error) { - if err := u.ensureClient(); err != nil { +func (u *Auth) Authenticate(ctx context.Context, m models.AuthModel) (*models.User, error) { + if err := u.ensureClient(ctx); err != nil { return nil, err } t, err := u.client.PasswordAuth(m.Principal, m.Password) @@ -63,7 +63,7 @@ func (u *Auth) Authenticate(m models.AuthModel) (*models.User, error) { // OnBoardUser will check if a user exists in user table, if not insert the user and // put the id in the pointer of user model, if it does exist, return the user's profile. -func (u *Auth) OnBoardUser(user *models.User) error { +func (u *Auth) OnBoardUser(ctx context.Context, user *models.User) error { user.Username = strings.TrimSpace(user.Username) if len(user.Username) == 0 { return fmt.Errorf("the Username is empty") @@ -73,7 +73,7 @@ func (u *Auth) OnBoardUser(user *models.User) error { } fillEmailRealName(user) user.Comment = "From UAA" - return u.userMgr.Onboard(orm.Context(), user) + return u.userMgr.Onboard(ctx, user) } func fillEmailRealName(user *models.User) { @@ -86,11 +86,10 @@ func fillEmailRealName(user *models.User) { } // PostAuthenticate will check if user exists in DB, if not on Board user, if he does, update the profile. -func (u *Auth) PostAuthenticate(user *models.User) error { - ctx := orm.Context() +func (u *Auth) PostAuthenticate(ctx context.Context, user *models.User) error { dbUser, err := u.userMgr.GetByName(ctx, user.Username) if errors.IsNotFoundErr(err) { - return u.OnBoardUser(user) + return u.OnBoardUser(ctx, user) } else if err != nil { return err } @@ -104,8 +103,8 @@ func (u *Auth) PostAuthenticate(user *models.User) error { } // SearchUser search user on uaa server, transform it to Harbor's user model -func (u *Auth) SearchUser(username string) (*models.User, error) { - if err := u.ensureClient(); err != nil { +func (u *Auth) SearchUser(ctx context.Context, username string) (*models.User, error) { + if err := u.ensureClient(ctx); err != nil { return nil, err } l, err := u.client.SearchUser(username) @@ -129,9 +128,9 @@ func (u *Auth) SearchUser(username string) (*models.User, error) { }, nil } -func (u *Auth) ensureClient() error { +func (u *Auth) ensureClient(ctx context.Context) error { var cfg *uaa.ClientConfig - UAASettings, err := config.UAASettings(orm.Context()) + UAASettings, err := config.UAASettings(ctx) // log.Debugf("Uaa settings: %+v", UAASettings) if err != nil { log.Warningf("Failed to get UAA setting from Admin Server, error: %v", err) diff --git a/src/core/auth/uaa/uaa_test.go b/src/core/auth/uaa/uaa_test.go index aac8cfe72..2b47ae043 100644 --- a/src/core/auth/uaa/uaa_test.go +++ b/src/core/auth/uaa/uaa_test.go @@ -59,13 +59,14 @@ func TestMain(m *testing.M) { func TestEnsureClient(t *testing.T) { assert := assert.New(t) auth := Auth{client: nil} - err := auth.ensureClient() + err := auth.ensureClient(orm.Context()) assert.Nil(err) assert.NotNil(auth.client) } func TestAuthenticate(t *testing.T) { assert := assert.New(t) + ctx := orm.Context() client := &uaa.FakeClient{ Username: "user1", Password: "password1", @@ -78,7 +79,7 @@ func TestAuthenticate(t *testing.T) { Principal: "user1", Password: "password1", } - u1, err1 := auth.Authenticate(m1) + u1, err1 := auth.Authenticate(ctx, m1) assert.Nil(err1) assert.NotNil(u1) assert.Equal("fake@fake.com", u1.Email) @@ -86,7 +87,7 @@ func TestAuthenticate(t *testing.T) { Principal: "wrong", Password: "wrong", } - u2, err2 := auth.Authenticate(m2) + u2, err2 := auth.Authenticate(ctx, m2) assert.NotNil(err2) assert.Nil(u2) err3 := dao.ClearTable(models.UserTable) @@ -102,14 +103,14 @@ func TestOnBoardUser(t *testing.T) { um1 := &models.User{ Username: " ", } - err1 := auth.OnBoardUser(um1) + err1 := auth.OnBoardUser(ctx, um1) assert.NotNil(err1) um2 := &models.User{ Username: "test ", } user2, _ := auth.userMgr.GetByName(ctx, "test") assert.Nil(user2) - err2 := auth.OnBoardUser(um2) + err2 := auth.OnBoardUser(ctx, um2) assert.Nil(err2) user, _ := auth.userMgr.GetByName(ctx, "test") assert.Equal("test", user.Realname) @@ -121,24 +122,24 @@ func TestOnBoardUser(t *testing.T) { func TestPostAuthenticate(t *testing.T) { assert := assert.New(t) + ctx := orm.Context() auth := Auth{ userMgr: userpkg.New(), } um := &models.User{ Username: "test", } - err := auth.PostAuthenticate(um) + err := auth.PostAuthenticate(ctx, um) // need a new user model to simulate a login case... um2 := &models.User{ Username: "test", } - ctx := orm.Context() assert.Nil(err) user, _ := auth.userMgr.GetByName(ctx, "test") assert.Equal("", user.Email) um2.Email = "newEmail@new.com" um2.Realname = "newName" - err2 := auth.PostAuthenticate(um2) + err2 := auth.PostAuthenticate(ctx, um2) assert.Equal(user.UserID, um2.UserID) assert.Nil(err2) user2, _ := auth.userMgr.GetByName(ctx, "test") @@ -148,7 +149,7 @@ func TestPostAuthenticate(t *testing.T) { um3 := &models.User{ Username: "test", } - err3 := auth.PostAuthenticate(um3) + err3 := auth.PostAuthenticate(ctx, um3) assert.Nil(err3) user3, _ := auth.userMgr.GetByName(ctx, "test") assert.Equal(user3.UserID, um3.UserID) @@ -160,19 +161,20 @@ func TestPostAuthenticate(t *testing.T) { func TestSearchUser(t *testing.T) { assert := assert.New(t) + ctx := orm.Context() client := &uaa.FakeClient{ Username: "user1", Password: "password1", } auth := Auth{client: client} - _, err0 := auth.SearchUser("error") + _, err0 := auth.SearchUser(ctx, "error") assert.NotNil(err0) - u1, err1 := auth.SearchUser("one") + u1, err1 := auth.SearchUser(ctx, "one") assert.Nil(err1) assert.Equal("one@email.com", u1.Email) - _, err2 := auth.SearchUser("two") + _, err2 := auth.SearchUser(ctx, "two") assert.NotNil(err2) - user3, err3 := auth.SearchUser("none") + user3, err3 := auth.SearchUser(ctx, "none") assert.Nil(user3) assert.Nil(err3) } diff --git a/src/core/controllers/authproxy_redirect.go b/src/core/controllers/authproxy_redirect.go index 6c6a680cb..36c2d049b 100644 --- a/src/core/controllers/authproxy_redirect.go +++ b/src/core/controllers/authproxy_redirect.go @@ -2,13 +2,12 @@ package controllers import ( "fmt" - "github.com/goharbor/harbor/src/lib/config" - "github.com/goharbor/harbor/src/lib/orm" "net/http" "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/core/api" "github.com/goharbor/harbor/src/core/auth/authproxy" + "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/log" ) @@ -26,7 +25,7 @@ type AuthProxyController struct { // Prepare checks the auth mode and fail early func (apc *AuthProxyController) Prepare() { - am, err := config.AuthMode(orm.Context()) + am, err := config.AuthMode(apc.Context()) if err != nil { apc.SendInternalServerError(err) return @@ -45,13 +44,13 @@ func (apc *AuthProxyController) HandleRedirect() { apc.Ctx.Redirect(http.StatusMovedPermanently, "/") return } - u, err := helper.VerifyToken(token) + u, err := helper.VerifyToken(apc.Context(), token) if err != nil { log.Errorf("Failed to verify token, error: %v", err) apc.Ctx.Redirect(http.StatusMovedPermanently, "/") return } - if err := helper.PostAuthenticate(u); err != nil { + if err := helper.PostAuthenticate(apc.Context(), u); err != nil { log.Errorf("Failed to onboard user, error: %v", err) apc.Ctx.Redirect(http.StatusMovedPermanently, "/") return diff --git a/src/core/controllers/base.go b/src/core/controllers/base.go index ecec6d7f2..b3811fa80 100644 --- a/src/core/controllers/base.go +++ b/src/core/controllers/base.go @@ -21,7 +21,6 @@ import ( "strings" "github.com/astaxie/beego" - o "github.com/astaxie/beego/orm" "github.com/beego/i18n" "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/models" @@ -32,7 +31,6 @@ import ( "github.com/goharbor/harbor/src/lib" "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/log" - "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/lib/q" ) @@ -97,7 +95,7 @@ func (cc *CommonController) Login() { return } - user, err := auth.Login(models.AuthModel{ + user, err := auth.Login(cc.Context(), models.AuthModel{ Principal: principal, Password: password, }) @@ -119,7 +117,7 @@ func (cc *CommonController) LogOut() { // UserExists checks if user exists when user input value in sign in form. func (cc *CommonController) UserExists() { - ctx := orm.NewContext(cc.Ctx.Request.Context(), o.NewOrm()) + ctx := cc.Context() flag, err := config.SelfRegistration(ctx) if err != nil { log.Errorf("Failed to get the status of self registration flag, error: %v, disabling user existence check", err) diff --git a/src/core/controllers/oidc.go b/src/core/controllers/oidc.go index ac3f863f9..6038c2794 100644 --- a/src/core/controllers/oidc.go +++ b/src/core/controllers/oidc.go @@ -29,7 +29,6 @@ import ( "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" - "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg/oidc" ) @@ -49,7 +48,7 @@ type onboardReq struct { // Prepare include public code path for call request handler of OIDCController func (oc *OIDCController) Prepare() { - if mode, _ := config.AuthMode(orm.Context()); mode != common.OIDCAuth { + if mode, _ := config.AuthMode(oc.Context()); mode != common.OIDCAuth { oc.SendPreconditionFailedError(fmt.Errorf("auth mode: %s is not OIDC based", mode)) return } diff --git a/src/pkg/ldap/manager.go b/src/pkg/ldap/manager.go index 77418d254..14d3ebb07 100644 --- a/src/pkg/ldap/manager.go +++ b/src/pkg/ldap/manager.go @@ -3,6 +3,7 @@ package ldap import ( "context" "fmt" + goldap "github.com/go-ldap/ldap/v3" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/core/auth" @@ -104,7 +105,7 @@ func (m *manager) ImportUser(ctx context.Context, sess *Session, ldapImportUsers user.Username = ldapUsers[0].Username user.Realname = ldapUsers[0].Realname user.Email = ldapUsers[0].Email - err = auth.OnBoardUser(&user) + err = auth.OnBoardUser(ctx, &user) if err != nil || user.UserID <= 0 { u.UID = tempUID diff --git a/src/server/middleware/readonly/readonly.go b/src/server/middleware/readonly/readonly.go index cfa5d26a9..cde12074c 100644 --- a/src/server/middleware/readonly/readonly.go +++ b/src/server/middleware/readonly/readonly.go @@ -15,12 +15,11 @@ package readonly import ( - "github.com/goharbor/harbor/src/lib/config" - lib_http "github.com/goharbor/harbor/src/lib/http" - "github.com/goharbor/harbor/src/lib/orm" "net/http" + "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/errors" + lib_http "github.com/goharbor/harbor/src/lib/http" "github.com/goharbor/harbor/src/server/middleware" ) @@ -34,7 +33,7 @@ var ( // DefaultConfig default readonly config DefaultConfig = Config{ ReadOnly: func(r *http.Request) bool { - return config.ReadOnly(orm.Context()) + return config.ReadOnly(r.Context()) }, } diff --git a/src/server/middleware/repoproxy/proxy.go b/src/server/middleware/repoproxy/proxy.go index 9e07e68f1..d39ae70e4 100644 --- a/src/server/middleware/repoproxy/proxy.go +++ b/src/server/middleware/repoproxy/proxy.go @@ -17,6 +17,10 @@ package repoproxy import ( "context" "fmt" + "io" + "net/http" + "time" + "github.com/goharbor/harbor/src/common/security" "github.com/goharbor/harbor/src/common/security/proxycachesecret" "github.com/goharbor/harbor/src/controller/project" @@ -30,9 +34,6 @@ import ( proModels "github.com/goharbor/harbor/src/pkg/project/models" "github.com/goharbor/harbor/src/pkg/reg/model" "github.com/goharbor/harbor/src/server/middleware" - "io" - "net/http" - "time" ) const ( @@ -59,7 +60,7 @@ func handleBlob(w http.ResponseWriter, r *http.Request, next http.Handler) error if err != nil { return err } - if !canProxy(p) || proxyCtl.UseLocalBlob(ctx, art) { + if !canProxy(r.Context(), p) || proxyCtl.UseLocalBlob(ctx, art) { next.ServeHTTP(w, r) return nil } @@ -111,7 +112,7 @@ func handleManifest(w http.ResponseWriter, r *http.Request, next http.Handler) e if err != nil { return err } - if !canProxy(p) { + if !canProxy(r.Context(), p) { next.ServeHTTP(w, r) return nil } @@ -173,11 +174,11 @@ func proxyManifestGet(ctx context.Context, w http.ResponseWriter, ctl proxy.Cont return nil } -func canProxy(p *proModels.Project) bool { +func canProxy(ctx context.Context, p *proModels.Project) bool { if p.RegistryID < 1 { return false } - reg, err := registry.Ctl.Get(orm.Context(), p.RegistryID) + reg, err := registry.Ctl.Get(ctx, p.RegistryID) if err != nil { log.Errorf("failed to get registry, error:%v", err) return false diff --git a/src/server/middleware/security/auth_proxy.go b/src/server/middleware/security/auth_proxy.go index f985a4db6..57cac826d 100644 --- a/src/server/middleware/security/auth_proxy.go +++ b/src/server/middleware/security/auth_proxy.go @@ -26,7 +26,6 @@ import ( "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" - "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg/authproxy" pkguser "github.com/goharbor/harbor/src/pkg/user" ) @@ -51,7 +50,7 @@ func (a *authProxy) Generate(req *http.Request) security.Context { log.Errorf("user name %s doesn't meet the auth proxy name pattern", proxyUserName) return nil } - httpAuthProxyConf, err := config.HTTPAuthProxySetting(orm.Context()) + httpAuthProxyConf, err := config.HTTPAuthProxySetting(req.Context()) if err != nil { log.Errorf("failed to get auth proxy settings: %v", err) return nil @@ -68,7 +67,7 @@ func (a *authProxy) Generate(req *http.Request) security.Context { user, err := pkguser.Mgr.GetByName(req.Context(), rawUserName) if errors.IsNotFoundErr(err) { // onboard user if it's not yet onboarded. - uid, err2 := auth.SearchAndOnBoardUser(rawUserName) + uid, err2 := auth.SearchAndOnBoardUser(req.Context(), rawUserName) if err2 != nil { log.Errorf("failed to search and onboard user %s: %v", rawUserName, err) return nil diff --git a/src/server/middleware/security/auth_proxy_test.go b/src/server/middleware/security/auth_proxy_test.go index 8f67eb8d4..947737c6f 100644 --- a/src/server/middleware/security/auth_proxy_test.go +++ b/src/server/middleware/security/auth_proxy_test.go @@ -17,8 +17,13 @@ package security import ( "encoding/json" "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + "net/url" + "testing" + "github.com/goharbor/harbor/src/common" - "github.com/goharbor/harbor/src/common/utils/test" _ "github.com/goharbor/harbor/src/core/auth/authproxy" "github.com/goharbor/harbor/src/lib" "github.com/goharbor/harbor/src/lib/config" @@ -28,17 +33,11 @@ import ( _ "github.com/goharbor/harbor/src/pkg/config/inmemory" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "io/ioutil" "k8s.io/api/authentication/v1beta1" - "net/http" - "net/http/httptest" - "net/url" - "testing" ) func TestAuthProxy(t *testing.T) { config.Init() - test.InitDatabaseFromEnv() authProxy := &authProxy{} server, err := newAuthProxyTestServer() diff --git a/src/server/middleware/security/basic_auth.go b/src/server/middleware/security/basic_auth.go index 7f91a48aa..390979bf6 100644 --- a/src/server/middleware/security/basic_auth.go +++ b/src/server/middleware/security/basic_auth.go @@ -32,7 +32,7 @@ func (b *basicAuth) Generate(req *http.Request) security.Context { if !ok { return nil } - user, err := auth.Login(models.AuthModel{ + user, err := auth.Login(req.Context(), models.AuthModel{ Principal: username, Password: password, }) diff --git a/src/server/middleware/security/basic_auth_test.go b/src/server/middleware/security/basic_auth_test.go index dd3e23e97..eb4ab31cc 100644 --- a/src/server/middleware/security/basic_auth_test.go +++ b/src/server/middleware/security/basic_auth_test.go @@ -15,11 +15,13 @@ package security import ( - _ "github.com/goharbor/harbor/src/core/auth/db" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "net/http" "testing" + + _ "github.com/goharbor/harbor/src/core/auth/db" + "github.com/goharbor/harbor/src/lib/orm" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestBasicAuth(t *testing.T) { @@ -27,6 +29,7 @@ func TestBasicAuth(t *testing.T) { req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/api/projects/", nil) require.Nil(t, err) req.SetBasicAuth("admin", "Harbor12345") + req = req.WithContext(orm.Context()) ctx := basicAuth.Generate(req) assert.NotNil(t, ctx) } diff --git a/src/server/middleware/security/security.go b/src/server/middleware/security/security.go index c26ab6513..8ecefc65f 100644 --- a/src/server/middleware/security/security.go +++ b/src/server/middleware/security/security.go @@ -15,12 +15,11 @@ package security import ( - "github.com/goharbor/harbor/src/lib/config" - "github.com/goharbor/harbor/src/lib/orm" "net/http" "github.com/goharbor/harbor/src/common/security" "github.com/goharbor/harbor/src/lib" + "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/server/middleware" ) @@ -48,7 +47,7 @@ type generator interface { func Middleware(skippers ...middleware.Skipper) func(http.Handler) http.Handler { return middleware.New(func(w http.ResponseWriter, r *http.Request, next http.Handler) { log := log.G(r.Context()) - mode, err := config.AuthMode(orm.Context()) + mode, err := config.AuthMode(r.Context()) if err == nil { r = r.WithContext(lib.WithAuthMode(r.Context(), mode)) } else { diff --git a/src/server/middleware/security/security_test.go b/src/server/middleware/security/security_test.go index 5f2a1f5ed..f0ab2b454 100644 --- a/src/server/middleware/security/security_test.go +++ b/src/server/middleware/security/security_test.go @@ -15,13 +15,21 @@ package security import ( + "net/http" + "os" + "testing" + "github.com/goharbor/harbor/src/common/security" + "github.com/goharbor/harbor/src/common/utils/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "net/http" - "testing" ) +func TestMain(m *testing.M) { + test.InitDatabaseFromEnv() + os.Exit(m.Run()) +} + func TestSecurity(t *testing.T) { var ctx security.Context var exist bool