From 3c2f92294d247308f3a332e7e050c5adb5dff5b5 Mon Sep 17 00:00:00 2001 From: stonezdj Date: Thu, 18 Jul 2019 17:22:21 +0800 Subject: [PATCH] Fix OnBoardGroup issue Signed-off-by: stonezdj Fix issue when adding a HTTP user group to a project member, returns HTTP 500 error. --- src/common/dao/group/usergroup.go | 15 ++++++++++----- src/common/dao/group/usergroup_test.go | 2 +- src/core/auth/authproxy/auth.go | 9 ++++++--- src/core/auth/authproxy/auth_test.go | 24 ++++++++++++++++++++++++ src/core/auth/ldap/ldap.go | 2 +- 5 files changed, 42 insertions(+), 10 deletions(-) diff --git a/src/common/dao/group/usergroup.go b/src/common/dao/group/usergroup.go index f17be1003..a6eedfec1 100644 --- a/src/common/dao/group/usergroup.go +++ b/src/common/dao/group/usergroup.go @@ -144,11 +144,7 @@ func UpdateUserGroupName(id int, groupName string) error { return err } -// OnBoardUserGroup will check if a usergroup exists in usergroup table, if not insert the usergroup and -// put the id in the pointer of usergroup model, if it does exist, return the usergroup's profile. -// This is used for ldap and uaa authentication, such the usergroup can have an ID in Harbor. -// the keyAttribute and combinedKeyAttribute are key columns used to check duplicate usergroup in harbor -func OnBoardUserGroup(g *models.UserGroup, keyAttribute string, combinedKeyAttributes ...string) error { +func onBoardCommonUserGroup(g *models.UserGroup, keyAttribute string, combinedKeyAttributes ...string) error { g.LdapGroupDN = utils.TrimLower(g.LdapGroupDN) o := dao.GetOrmer() @@ -172,3 +168,12 @@ func OnBoardUserGroup(g *models.UserGroup, keyAttribute string, combinedKeyAttri return nil } + +// OnBoardUserGroup will check if a usergroup exists in usergroup table, if not insert the usergroup and +// put the id in the pointer of usergroup model, if it does exist, return the usergroup's profile. +func OnBoardUserGroup(g *models.UserGroup) error { + if g.GroupType == common.LDAPGroupType { + return onBoardCommonUserGroup(g, "LdapGroupDN", "GroupType") + } + return onBoardCommonUserGroup(g, "GroupName", "GroupType") +} diff --git a/src/common/dao/group/usergroup_test.go b/src/common/dao/group/usergroup_test.go index 1cd8919e1..b4ad3897d 100644 --- a/src/common/dao/group/usergroup_test.go +++ b/src/common/dao/group/usergroup_test.go @@ -256,7 +256,7 @@ func TestOnBoardUserGroup(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := OnBoardUserGroup(tt.args.g, "LdapGroupDN", "GroupType"); (err != nil) != tt.wantErr { + if err := OnBoardUserGroup(tt.args.g); (err != nil) != tt.wantErr { t.Errorf("OnBoardUserGroup() error = %v, wantErr %v", err, tt.wantErr) } }) diff --git a/src/core/auth/authproxy/auth.go b/src/core/auth/authproxy/auth.go index 9081ad8ea..f3efae49d 100644 --- a/src/core/auth/authproxy/auth.go +++ b/src/core/auth/authproxy/auth.go @@ -17,6 +17,7 @@ package authproxy import ( "crypto/tls" "encoding/json" + "errors" "fmt" "io/ioutil" "net/http" @@ -190,12 +191,14 @@ func (a *Auth) SearchGroup(groupKey string) (*models.UserGroup, error) { // OnBoardGroup create user group entity in Harbor DB, altGroupName is not used. func (a *Auth) OnBoardGroup(u *models.UserGroup, altGroupName string) error { // if group name provided, on board the user group - userGroup := &models.UserGroup{GroupName: u.GroupName, GroupType: common.HTTPGroupType} - err := group.OnBoardUserGroup(u, "GroupName", "GroupType") + if len(u.GroupName) == 0 { + return errors.New("Should provide a group name") + } + u.GroupType = common.HTTPGroupType + err := group.OnBoardUserGroup(u) if err != nil { return err } - u.ID = userGroup.ID return nil } diff --git a/src/core/auth/authproxy/auth_test.go b/src/core/auth/authproxy/auth_test.go index 07c035211..2dbcdf061 100644 --- a/src/core/auth/authproxy/auth_test.go +++ b/src/core/auth/authproxy/auth_test.go @@ -43,6 +43,7 @@ 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", @@ -50,10 +51,17 @@ func TestMain(m *testing.M) { // So it won't require mocking the cfgManager settingTimeStamp: time.Now(), } + cfgMap := cut.GetUnitTestConfig() conf := map[string]interface{}{ common.HTTPAuthProxyEndpoint: a.Endpoint, common.HTTPAuthProxyTokenReviewEndpoint: a.TokenReviewEndpoint, common.HTTPAuthProxyVerifyCert: !a.SkipCertVerify, + common.PostGreSQLSSLMode: cfgMap[common.PostGreSQLSSLMode], + common.PostGreSQLUsername: cfgMap[common.PostGreSQLUsername], + common.PostGreSQLPort: cfgMap[common.PostGreSQLPort], + common.PostGreSQLHOST: cfgMap[common.PostGreSQLHOST], + common.PostGreSQLPassword: cfgMap[common.PostGreSQLPassword], + common.PostGreSQLDatabase: cfgMap[common.PostGreSQLDatabase], } config.InitWithSettings(conf) @@ -174,3 +182,19 @@ func TestAuth_PostAuthenticate(t *testing.T) { } } + +func TestAuth_OnBoardGroup(t *testing.T) { + input := &models.UserGroup{ + GroupName: "OnBoardTest", + GroupType: common.HTTPGroupType, + } + a.OnBoardGroup(input, "") + + assert.True(t, input.ID > 0, "The OnBoardGroup should have a valid group ID") + + emptyGroup := &models.UserGroup{} + err := a.OnBoardGroup(emptyGroup, "") + if err == nil { + t.Fatal("Empty user group should failed to OnBoard") + } +} diff --git a/src/core/auth/ldap/ldap.go b/src/core/auth/ldap/ldap.go index cf1963f7c..15a44da12 100644 --- a/src/core/auth/ldap/ldap.go +++ b/src/core/auth/ldap/ldap.go @@ -214,7 +214,7 @@ func (l *Auth) OnBoardGroup(u *models.UserGroup, altGroupName string) error { if len(userGroupList) > 0 { return auth.ErrDuplicateLDAPGroup } - return group.OnBoardUserGroup(u, "LdapGroupDN", "GroupType") + return group.OnBoardUserGroup(u) } // PostAuthenticate -- If user exist in harbor DB, sync email address, if not exist, call OnBoardUser