From c0ed55445d54e911a9ddbc04cfcb4a4a8c9ef9ab Mon Sep 17 00:00:00 2001 From: stonezdj Date: Tue, 2 Jul 2019 17:43:51 +0800 Subject: [PATCH] Refactor LDAP group Signed-off-by: stonezdj --- src/common/dao/group/usergroup.go | 19 ---- src/common/dao/group/usergroup_test.go | 111 ++++++++++--------- src/common/dao/project.go | 38 ++++--- src/common/dao/project/projectmember.go | 13 --- src/common/dao/project/projectmember_test.go | 27 ----- src/common/dao/project_test.go | 97 ---------------- src/common/dao/utils.go | 11 ++ src/common/dao/utils_test.go | 24 ++++ src/common/models/project.go | 6 +- src/common/models/user.go | 14 +-- src/common/security/local/context.go | 12 +- src/common/security/local/context_test.go | 22 +++- src/core/auth/ldap/ldap.go | 15 +-- src/core/promgr/pmsdriver/local/local.go | 12 +- 14 files changed, 158 insertions(+), 263 deletions(-) create mode 100644 src/common/dao/utils.go create mode 100644 src/common/dao/utils_test.go diff --git a/src/common/dao/group/usergroup.go b/src/common/dao/group/usergroup.go index e0aa1d226..95feb2d0b 100644 --- a/src/common/dao/group/usergroup.go +++ b/src/common/dao/group/usergroup.go @@ -15,10 +15,8 @@ package group import ( - "strings" "time" - "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/utils" "github.com/goharbor/harbor/src/common/dao" @@ -139,20 +137,3 @@ func OnBoardUserGroup(g *models.UserGroup, keyAttribute string, combinedKeyAttri return nil } - -// GetGroupDNQueryCondition get the part of IN ('XXX', 'XXX') condition -func GetGroupDNQueryCondition(userGroupList []*models.UserGroup) string { - result := make([]string, 0) - count := 0 - for _, userGroup := range userGroupList { - if userGroup.GroupType == common.LdapGroupType { - result = append(result, "'"+userGroup.LdapGroupDN+"'") - count++ - } - } - // No LDAP Group found - if count == 0 { - return "" - } - return strings.Join(result, ",") -} diff --git a/src/common/dao/group/usergroup_test.go b/src/common/dao/group/usergroup_test.go index 91603e64d..f4801beea 100644 --- a/src/common/dao/group/usergroup_test.go +++ b/src/common/dao/group/usergroup_test.go @@ -47,6 +47,8 @@ func TestMain(m *testing.M) { initSqls := []string{ "insert into harbor_user (username, email, password, realname) values ('member_test_01', 'member_test_01@example.com', '123456', 'member_test_01')", "insert into project (name, owner_id) values ('member_test_01', 1)", + `insert into project (name, owner_id) values ('group_project2', 1)`, + `insert into project (name, owner_id) values ('group_project_private', 1)`, "insert into user_group (group_name, group_type, ldap_group_dn) values ('test_group_01', 1, 'cn=harbor_users,ou=sample,ou=vmware,dc=harbor,dc=com')", "update project set owner_id = (select user_id from harbor_user where username = 'member_test_01') where name = 'member_test_01'", "insert into project_member (project_id, entity_id, entity_type, role) values ( (select project_id from project where name = 'member_test_01') , (select user_id from harbor_user where username = 'member_test_01'), 'u', 1)", @@ -55,6 +57,8 @@ func TestMain(m *testing.M) { clearSqls := []string{ "delete from project where name='member_test_01'", + "delete from project where name='group_project2'", + "delete from project where name='group_project_private'", "delete from harbor_user where username='member_test_01' or username='pm_sample'", "delete from user_group", "delete from project_member", @@ -175,7 +179,7 @@ func TestUpdateUserGroup(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fmt.Printf("id=%v", createdUserGroupID) + fmt.Printf("id=%v\n", createdUserGroupID) if err := UpdateUserGroupName(tt.args.id, tt.args.groupName); (err != nil) != tt.wantErr { t.Errorf("UpdateUserGroup() error = %v, wantErr %v", err, tt.wantErr) userGroup, err := GetUserGroup(tt.args.id) @@ -249,40 +253,6 @@ func TestOnBoardUserGroup(t *testing.T) { } } -func TestGetGroupDNQueryCondition(t *testing.T) { - userGroupList := []*models.UserGroup{ - { - GroupName: "sample1", - GroupType: 1, - LdapGroupDN: "cn=sample1_users,ou=groups,dc=example,dc=com", - }, - { - GroupName: "sample2", - GroupType: 1, - LdapGroupDN: "cn=sample2_users,ou=groups,dc=example,dc=com", - }, - { - GroupName: "sample3", - GroupType: 0, - LdapGroupDN: "cn=sample3_users,ou=groups,dc=example,dc=com", - }, - } - - groupQueryConditions := GetGroupDNQueryCondition(userGroupList) - expectedConditions := `'cn=sample1_users,ou=groups,dc=example,dc=com','cn=sample2_users,ou=groups,dc=example,dc=com'` - if groupQueryConditions != expectedConditions { - t.Errorf("Failed to GetGroupDNQueryCondition, expected %v, actual %v", expectedConditions, groupQueryConditions) - } - var userGroupList2 []*models.UserGroup - groupQueryCondition2 := GetGroupDNQueryCondition(userGroupList2) - if len(groupQueryCondition2) > 0 { - t.Errorf("Failed to GetGroupDNQueryCondition, expected %v, actual %v", "", groupQueryCondition2) - } - groupQueryCondition3 := GetGroupDNQueryCondition(nil) - if len(groupQueryCondition3) > 0 { - t.Errorf("Failed to GetGroupDNQueryCondition, expected %v, actual %v", "", groupQueryCondition3) - } -} func TestGetGroupProjects(t *testing.T) { userID, err := dao.Register(models.User{ Username: "grouptestu09", @@ -322,8 +292,7 @@ func TestGetGroupProjects(t *testing.T) { }) defer project.DeleteProjectMemberByID(pmid) type args struct { - groupDNCondition string - query *models.ProjectQueryParam + query *models.ProjectQueryParam } member := &models.MemberQuery{ Name: "grouptestu09", @@ -335,19 +304,17 @@ func TestGetGroupProjects(t *testing.T) { wantErr bool }{ {"Query with group DN", - args{"'cn=harbor_users,ou=groups,dc=example,dc=com'", - &models.ProjectQueryParam{ - Member: member, - }}, + args{&models.ProjectQueryParam{ + Member: member, + }}, 1, false}, {"Query without group DN", - args{"", - &models.ProjectQueryParam{}}, + args{&models.ProjectQueryParam{}}, 1, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := dao.GetGroupProjects(tt.args.groupDNCondition, tt.args.query) + got, err := dao.GetGroupProjects([]int{groupID}, tt.args.query) if (err != nil) != tt.wantErr { t.Errorf("GetGroupProjects() error = %v, wantErr %v", err, tt.wantErr) return @@ -392,8 +359,7 @@ func TestGetTotalGroupProjects(t *testing.T) { }) defer project.DeleteProjectMemberByID(pmid) type args struct { - groupDNCondition string - query *models.ProjectQueryParam + query *models.ProjectQueryParam } tests := []struct { name string @@ -401,18 +367,16 @@ func TestGetTotalGroupProjects(t *testing.T) { wantSize int wantErr bool }{ - {"Query with group DN", - args{"'cn=harbor_users,ou=groups,dc=example,dc=com'", - &models.ProjectQueryParam{}}, + {"Query with group ID", + args{&models.ProjectQueryParam{}}, 1, false}, - {"Query without group DN", - args{"", - &models.ProjectQueryParam{}}, + {"Query without group ID", + args{&models.ProjectQueryParam{}}, 1, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := dao.GetTotalGroupProjects(tt.args.groupDNCondition, tt.args.query) + got, err := dao.GetTotalGroupProjects([]int{groupID}, tt.args.query) if (err != nil) != tt.wantErr { t.Errorf("GetGroupProjects() error = %v, wantErr %v", err, tt.wantErr) return @@ -423,3 +387,44 @@ func TestGetTotalGroupProjects(t *testing.T) { }) } } +func TestGetRolesByLDAPGroup(t *testing.T) { + + userGroupList, err := QueryUserGroup(models.UserGroup{LdapGroupDN: "cn=harbor_users,ou=sample,ou=vmware,dc=harbor,dc=com", GroupType: 1}) + if err != nil || len(userGroupList) < 1 { + t.Errorf("failed to query user group, err %v", err) + } + project, err := dao.GetProjectByName("member_test_01") + if err != nil { + t.Errorf("Error occurred when Get project by name: %v", err) + } + privateProject, err := dao.GetProjectByName("group_project_private") + if err != nil { + t.Errorf("Error occurred when Get project by name: %v", err) + } + + type args struct { + projectID int64 + groupIDs []int + } + tests := []struct { + name string + args args + wantSize int + wantErr bool + }{ + {"Check normal", args{projectID: project.ProjectID, groupIDs: []int{userGroupList[0].ID}}, 1, false}, + {"Check non exist", args{projectID: privateProject.ProjectID, groupIDs: []int{9999}}, 0, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := dao.GetRolesByGroupID(tt.args.projectID, tt.args.groupIDs) + if (err != nil) != tt.wantErr { + t.Errorf("TestGetRolesByLDAPGroup() error = %v, wantErr %v", err, tt.wantErr) + return + } + if len(got) != tt.wantSize { + t.Errorf("TestGetRolesByLDAPGroup() = %v, want %v", len(got), tt.wantSize) + } + }) + } +} diff --git a/src/common/dao/project.go b/src/common/dao/project.go index 423b6b23b..c96af6537 100644 --- a/src/common/dao/project.go +++ b/src/common/dao/project.go @@ -156,18 +156,19 @@ func GetProjects(query *models.ProjectQueryParam) ([]*models.Project, error) { // GetGroupProjects - Get user's all projects, including user is the user member of this project // and the user is in the group which is a group member of this project. -func GetGroupProjects(groupDNCondition string, query *models.ProjectQueryParam) ([]*models.Project, error) { +func GetGroupProjects(groupIDs []int, query *models.ProjectQueryParam) ([]*models.Project, error) { sql, params := projectQueryConditions(query) sql = `select distinct p.project_id, p.name, p.owner_id, p.creation_time, p.update_time ` + sql - if len(groupDNCondition) > 0 { + groupIDCondition := JoinNumberConditions(groupIDs) + if len(groupIDs) > 0 { sql = fmt.Sprintf( `%s union select distinct p.project_id, p.name, p.owner_id, p.creation_time, p.update_time from project p left join project_member pm on p.project_id = pm.project_id - left join user_group ug on ug.id = pm.entity_id and pm.entity_type = 'g' and ug.group_type = 1 - where ug.ldap_group_dn in ( %s ) order by name`, - sql, groupDNCondition) + left join user_group ug on ug.id = pm.entity_id and pm.entity_type = 'g' + where ug.id in ( %s ) order by name`, + sql, groupIDCondition) } sqlStr, queryParams := CreatePagination(query, sql, params) log.Debugf("query sql:%v", sql) @@ -178,10 +179,11 @@ func GetGroupProjects(groupDNCondition string, query *models.ProjectQueryParam) // GetTotalGroupProjects - Get the total count of projects, including user is the member of this project and the // user is in the group, which is the group member of this project. -func GetTotalGroupProjects(groupDNCondition string, query *models.ProjectQueryParam) (int, error) { +func GetTotalGroupProjects(groupIDs []int, query *models.ProjectQueryParam) (int, error) { var sql string sqlCondition, params := projectQueryConditions(query) - if len(groupDNCondition) == 0 { + groupIDCondition := JoinNumberConditions(groupIDs) + if len(groupIDs) == 0 { sql = `select count(1) ` + sqlCondition } else { sql = fmt.Sprintf( @@ -189,9 +191,9 @@ func GetTotalGroupProjects(groupDNCondition string, query *models.ProjectQueryPa from ( select p.project_id %s union select p.project_id from project p left join project_member pm on p.project_id = pm.project_id - left join user_group ug on ug.id = pm.entity_id and pm.entity_type = 'g' and ug.group_type = 1 - where ug.ldap_group_dn in ( %s )) t`, - sqlCondition, groupDNCondition) + left join user_group ug on ug.id = pm.entity_id and pm.entity_type = 'g' + where ug.id in ( %s )) t`, + sqlCondition, groupIDCondition) } log.Debugf("query sql:%v", sql) var count int @@ -291,24 +293,24 @@ func DeleteProject(id int64) error { return err } -// GetRolesByLDAPGroup - Get Project roles of the -// specified group DN is a member of current project -func GetRolesByLDAPGroup(projectID int64, groupDNCondition string) ([]int, error) { +// GetRolesByGroupID - Get Project roles of the +// specified group is a member of current project +func GetRolesByGroupID(projectID int64, groupIDs []int) ([]int, error) { var roles []int - if len(groupDNCondition) == 0 { + if len(groupIDs) == 0 { return roles, nil } + groupIDCondition := JoinNumberConditions(groupIDs) o := GetOrmer() - // Because an LDAP user can be memberof multiple groups, // the role is in descent order (1-admin, 2-developer, 3-guest, 4-master), use min to select the max privilege role. sql := fmt.Sprintf( `select min(pm.role) from project_member pm left join user_group ug on pm.entity_type = 'g' and pm.entity_id = ug.id - where ug.ldap_group_dn in ( %s ) and pm.project_id = ? `, - groupDNCondition) + where ug.id in ( %s ) and pm.project_id = ?`, + groupIDCondition) log.Debugf("sql:%v", sql) if _, err := o.Raw(sql, projectID).QueryRows(&roles); err != nil { - log.Warningf("Error in GetRolesByLDAPGroup, error: %v", err) + log.Warningf("Error in GetRolesByGroupID, error: %v", err) return nil, err } // If there is no row selected, the min returns an empty row, to avoid return 0 as role diff --git a/src/common/dao/project/projectmember.go b/src/common/dao/project/projectmember.go index f9a81e706..6776143ad 100644 --- a/src/common/dao/project/projectmember.go +++ b/src/common/dao/project/projectmember.go @@ -148,16 +148,3 @@ func SearchMemberByName(projectID int64, entityName string) ([]*models.Member, e _, err := o.Raw(sql, queryParam).QueryRows(&members) return members, err } - -// GetRolesByGroup -- Query group roles -func GetRolesByGroup(projectID int64, groupDNCondition string) []int { - var roles []int - o := dao.GetOrmer() - sql := `select role from project_member pm - left join user_group ug on pm.project_id = ? - where ug.group_type = 1 and ug.ldap_group_dn in (` + groupDNCondition + `)` - if _, err := o.Raw(sql, projectID).QueryRows(&roles); err != nil { - return roles - } - return roles -} diff --git a/src/common/dao/project/projectmember_test.go b/src/common/dao/project/projectmember_test.go index 66de3b6a8..f19738009 100644 --- a/src/common/dao/project/projectmember_test.go +++ b/src/common/dao/project/projectmember_test.go @@ -305,30 +305,3 @@ func PrepareGroupTest() { } dao.PrepareTestData(clearSqls, initSqls) } -func TestGetRolesByGroup(t *testing.T) { - PrepareGroupTest() - - project, err := dao.GetProjectByName("group_project") - if err != nil { - t.Errorf("Error occurred when GetProjectByName : %v", err) - } - type args struct { - projectID int64 - groupDNCondition string - } - tests := []struct { - name string - args args - want []int - }{ - {"Query group with role", args{project.ProjectID, "'cn=harbor_user,dc=example,dc=com'"}, []int{2}}, - {"Query group no role", args{project.ProjectID, "'cn=another_user,dc=example,dc=com'"}, []int{}}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := GetRolesByGroup(tt.args.projectID, tt.args.groupDNCondition); !dao.ArrayEqual(got, tt.want) { - t.Errorf("GetRolesByGroup() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/src/common/dao/project_test.go b/src/common/dao/project_test.go index 7358840b9..d3cc1d469 100644 --- a/src/common/dao/project_test.go +++ b/src/common/dao/project_test.go @@ -118,36 +118,6 @@ func Test_projectQueryConditions(t *testing.T) { } } -func TestGetGroupProjects(t *testing.T) { - prepareGroupTest() - query := &models.ProjectQueryParam{Member: &models.MemberQuery{Name: "sample_group"}} - type args struct { - groupDNCondition string - query *models.ProjectQueryParam - } - tests := []struct { - name string - args args - wantSize int - wantErr bool - }{ - {"Verify correct sql", args{groupDNCondition: "'cn=harbor_user,dc=example,dc=com'", query: query}, 1, false}, - {"Verify missed sql", args{groupDNCondition: "'cn=another_user,dc=example,dc=com'", query: query}, 0, false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := GetGroupProjects(tt.args.groupDNCondition, tt.args.query) - if (err != nil) != tt.wantErr { - t.Errorf("GetGroupProjects() error = %v, wantErr %v", err, tt.wantErr) - return - } - if len(got) != tt.wantSize { - t.Errorf("GetGroupProjects() = %v, want %v", got, tt.wantSize) - } - }) - } -} - func prepareGroupTest() { initSqls := []string{ `insert into user_group (group_name, group_type, ldap_group_dn) values ('harbor_group_01', 1, 'cn=harbor_user,dc=example,dc=com')`, @@ -169,73 +139,6 @@ func prepareGroupTest() { PrepareTestData(clearSqls, initSqls) } -func TestGetTotalGroupProjects(t *testing.T) { - prepareGroupTest() - query := &models.ProjectQueryParam{Member: &models.MemberQuery{Name: "sample_group"}} - type args struct { - groupDNCondition string - query *models.ProjectQueryParam - } - tests := []struct { - name string - args args - want int - wantErr bool - }{ - {"Verify correct sql", args{groupDNCondition: "'cn=harbor_user,dc=example,dc=com'", query: query}, 1, false}, - {"Verify missed sql", args{groupDNCondition: "'cn=another_user,dc=example,dc=com'", query: query}, 0, false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := GetTotalGroupProjects(tt.args.groupDNCondition, tt.args.query) - if (err != nil) != tt.wantErr { - t.Errorf("GetTotalGroupProjects() error = %v, wantErr %v", err, tt.wantErr) - return - } - if got != tt.want { - t.Errorf("GetTotalGroupProjects() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestGetRolesByLDAPGroup(t *testing.T) { - prepareGroupTest() - project, err := GetProjectByName("group_project") - if err != nil { - t.Errorf("Error occurred when Get project by name: %v", err) - } - privateProject, err := GetProjectByName("group_project_private") - if err != nil { - t.Errorf("Error occurred when Get project by name: %v", err) - } - type args struct { - projectID int64 - groupDNCondition string - } - tests := []struct { - name string - args args - wantSize int - wantErr bool - }{ - {"Check normal", args{project.ProjectID, "'cn=harbor_user,dc=example,dc=com'"}, 1, false}, - {"Check non exist", args{privateProject.ProjectID, "'cn=not_harbor_user,dc=example,dc=com'"}, 0, false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := GetRolesByLDAPGroup(tt.args.projectID, tt.args.groupDNCondition) - if (err != nil) != tt.wantErr { - t.Errorf("TestGetRolesByLDAPGroup() error = %v, wantErr %v", err, tt.wantErr) - return - } - if len(got) != tt.wantSize { - t.Errorf("TestGetRolesByLDAPGroup() = %v, want %v", len(got), tt.wantSize) - } - }) - } -} - func TestProjetExistsByName(t *testing.T) { name := "project_exist_by_name_test" exist := ProjectExistsByName(name) diff --git a/src/common/dao/utils.go b/src/common/dao/utils.go new file mode 100644 index 000000000..489f43e45 --- /dev/null +++ b/src/common/dao/utils.go @@ -0,0 +1,11 @@ +package dao + +import ( + "fmt" + "strings" +) + +// JoinNumberConditions - To join number condition into string,used in sql query +func JoinNumberConditions(ids []int) string { + return strings.Trim(strings.Replace(fmt.Sprint(ids), " ", ",", -1), "[]") +} diff --git a/src/common/dao/utils_test.go b/src/common/dao/utils_test.go new file mode 100644 index 000000000..78f2f4a3a --- /dev/null +++ b/src/common/dao/utils_test.go @@ -0,0 +1,24 @@ +package dao + +import "testing" + +func TestJoinNumberConditions(t *testing.T) { + type args struct { + ids []int + } + tests := []struct { + name string + args args + want string + }{ + {name: "normal test", args: args{[]int{1, 2, 3}}, want: "1,2,3"}, + {name: "dummy test", args: args{[]int{}}, want: ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := JoinNumberConditions(tt.args.ids); got != tt.want { + t.Errorf("JoinNumberConditions() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/src/common/models/project.go b/src/common/models/project.go index bebadcdd1..91b83d87f 100644 --- a/src/common/models/project.go +++ b/src/common/models/project.go @@ -128,9 +128,9 @@ type ProjectQueryParam struct { // MemberQuery filter by member's username and role type MemberQuery struct { - Name string // the username of member - Role int // the role of the member has to the project - GroupList []*UserGroup // the group list of current user + Name string // the username of member + Role int // the role of the member has to the project + GroupIDs []int // the group ID of current user belongs to } // Pagination ... diff --git a/src/common/models/user.go b/src/common/models/user.go index 9b224bd80..77fac1a83 100644 --- a/src/common/models/user.go +++ b/src/common/models/user.go @@ -35,13 +35,13 @@ type User struct { // to it. Role int `orm:"-" json:"role_id"` // RoleList []Role `json:"role_list"` - HasAdminRole bool `orm:"column(sysadmin_flag)" json:"has_admin_role"` - ResetUUID string `orm:"column(reset_uuid)" json:"reset_uuid"` - Salt string `orm:"column(salt)" json:"-"` - CreationTime time.Time `orm:"column(creation_time);auto_now_add" json:"creation_time"` - UpdateTime time.Time `orm:"column(update_time);auto_now" json:"update_time"` - GroupList []*UserGroup `orm:"-" json:"-"` - OIDCUserMeta *OIDCUser `orm:"-" json:"oidc_user_meta,omitempty"` + HasAdminRole bool `orm:"column(sysadmin_flag)" json:"has_admin_role"` + ResetUUID string `orm:"column(reset_uuid)" json:"reset_uuid"` + Salt string `orm:"column(salt)" json:"-"` + CreationTime time.Time `orm:"column(creation_time);auto_now_add" json:"creation_time"` + UpdateTime time.Time `orm:"column(update_time);auto_now" json:"update_time"` + GroupIDs []int `orm:"-" json:"-"` + OIDCUserMeta *OIDCUser `orm:"-" json:"oidc_user_meta,omitempty"` } // UserQuery ... diff --git a/src/common/security/local/context.go b/src/common/security/local/context.go index 655fe34b1..c63c5edab 100644 --- a/src/common/security/local/context.go +++ b/src/common/security/local/context.go @@ -17,7 +17,6 @@ package local import ( "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" - "github.com/goharbor/harbor/src/common/dao/group" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/rbac/project" @@ -140,12 +139,11 @@ func (s *SecurityContext) GetRolesByGroup(projectIDOrName interface{}) []int { user := s.user project, err := s.pm.Get(projectIDOrName) // No user, group or project info - if err != nil || project == nil || user == nil || len(user.GroupList) == 0 { + if err != nil || project == nil || user == nil || len(user.GroupIDs) == 0 { return roles } - // Get role by LDAP group - groupDNConditions := group.GetGroupDNQueryCondition(user.GroupList) - roles, err = dao.GetRolesByLDAPGroup(project.ProjectID, groupDNConditions) + // Get role by Group ID + roles, err = dao.GetRolesByGroupID(project.ProjectID, user.GroupIDs) if err != nil { return nil } @@ -157,8 +155,8 @@ func (s *SecurityContext) GetMyProjects() ([]*models.Project, error) { result, err := s.pm.List( &models.ProjectQueryParam{ Member: &models.MemberQuery{ - Name: s.GetUsername(), - GroupList: s.user.GroupList, + Name: s.GetUsername(), + GroupIDs: s.user.GroupIDs, }, }) if err != nil { diff --git a/src/common/security/local/context_test.go b/src/common/security/local/context_test.go index 955c041cc..36436010a 100644 --- a/src/common/security/local/context_test.go +++ b/src/common/security/local/context_test.go @@ -20,6 +20,7 @@ import ( "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/dao/group" "github.com/goharbor/harbor/src/common/dao/project" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" @@ -253,9 +254,16 @@ func TestHasPushPullPermWithGroup(t *testing.T) { if err != nil { t.Errorf("Error occurred when GetUser: %v", err) } - developer.GroupList = []*models.UserGroup{ - {GroupName: "test_group", GroupType: 1, LdapGroupDN: "cn=harbor_user,dc=example,dc=com"}, + + userGroups, err := group.QueryUserGroup(models.UserGroup{GroupType: common.LdapGroupType, LdapGroupDN: "cn=harbor_user,dc=example,dc=com"}) + if err != nil { + t.Errorf("Failed to query user group %v", err) } + if len(userGroups) < 1 { + t.Errorf("Failed to retrieve user group") + } + + developer.GroupIDs = []int{userGroups[0].ID} resource := rbac.NewProjectNamespace(project.Name).Resource(rbac.ResourceRepository) @@ -332,9 +340,15 @@ func TestSecurityContext_GetRolesByGroup(t *testing.T) { if err != nil { t.Errorf("Error occurred when GetUser: %v", err) } - developer.GroupList = []*models.UserGroup{ - {GroupName: "test_group", GroupType: 1, LdapGroupDN: "cn=harbor_user,dc=example,dc=com"}, + userGroups, err := group.QueryUserGroup(models.UserGroup{GroupType: common.LdapGroupType, LdapGroupDN: "cn=harbor_user,dc=example,dc=com"}) + if err != nil { + t.Errorf("Failed to query user group %v", err) } + if len(userGroups) < 1 { + t.Errorf("Failed to retrieve user group") + } + + developer.GroupIDs = []int{userGroups[0].ID} type fields struct { user *models.User pm promgr.ProjectManager diff --git a/src/core/auth/ldap/ldap.go b/src/core/auth/ldap/ldap.go index 4d8b63fdf..9009d3e23 100644 --- a/src/core/auth/ldap/ldap.go +++ b/src/core/auth/ldap/ldap.go @@ -20,11 +20,11 @@ import ( "strings" "github.com/goharbor/harbor/src/common" + "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/dao/group" "github.com/goharbor/harbor/src/common/utils" goldap "gopkg.in/ldap.v2" - "github.com/goharbor/harbor/src/common/dao" - "github.com/goharbor/harbor/src/common/dao/group" "github.com/goharbor/harbor/src/common/models" ldapUtils "github.com/goharbor/harbor/src/common/utils/ldap" "github.com/goharbor/harbor/src/common/utils/log" @@ -79,7 +79,7 @@ func (l *Auth) Authenticate(m models.AuthModel) (*models.User, error) { u.Username = ldapUsers[0].Username u.Email = strings.TrimSpace(ldapUsers[0].Email) u.Realname = ldapUsers[0].Realname - userGroups := make([]*models.UserGroup, 0) + ugIDs := []int{} dn := ldapUsers[0].DN if err = ldapSession.Bind(dn, m.Password); err != nil { @@ -95,6 +95,7 @@ func (l *Auth) Authenticate(m models.AuthModel) (*models.User, error) { for _, groupDN := range ldapUsers[0].GroupDNList { groupDN = utils.TrimLower(groupDN) + // Attach LDAP group admin if len(groupAdminDN) > 0 && groupAdminDN == groupDN { u.HasAdminRole = true } @@ -103,16 +104,16 @@ func (l *Auth) Authenticate(m models.AuthModel) (*models.User, error) { GroupType: 1, LdapGroupDN: groupDN, } - userGroupList, err := group.QueryUserGroup(userGroupQuery) + userGroups, err := group.QueryUserGroup(userGroupQuery) if err != nil { continue } - if len(userGroupList) == 0 { + if len(userGroups) == 0 { continue } - userGroups = append(userGroups, userGroupList[0]) + ugIDs = append(ugIDs, userGroups[0].ID) } - u.GroupList = userGroups + u.GroupIDs = ugIDs return &u, nil } diff --git a/src/core/promgr/pmsdriver/local/local.go b/src/core/promgr/pmsdriver/local/local.go index b02b19cbd..4706f3f43 100644 --- a/src/core/promgr/pmsdriver/local/local.go +++ b/src/core/promgr/pmsdriver/local/local.go @@ -20,7 +20,6 @@ import ( "time" "github.com/goharbor/harbor/src/common/dao" - "github.com/goharbor/harbor/src/common/dao/group" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/utils" errutil "github.com/goharbor/harbor/src/common/utils/error" @@ -132,19 +131,16 @@ func (d *driver) Update(projectIDOrName interface{}, func (d *driver) List(query *models.ProjectQueryParam) (*models.ProjectQueryResult, error) { var total int64 var projects []*models.Project - var groupDNCondition string - - // List with LDAP group projects + var groupIDs []int if query != nil && query.Member != nil { - groupDNCondition = group.GetGroupDNQueryCondition(query.Member.GroupList) + groupIDs = query.Member.GroupIDs } - - count, err := dao.GetTotalGroupProjects(groupDNCondition, query) + count, err := dao.GetTotalGroupProjects(groupIDs, query) if err != nil { return nil, err } total = int64(count) - projects, err = dao.GetGroupProjects(groupDNCondition, query) + projects, err = dao.GetGroupProjects(groupIDs, query) if err != nil { return nil, err }