From bb2ae7c093e4b9ffe972a85fe9ebd16af317f482 Mon Sep 17 00:00:00 2001 From: stonezdj Date: Tue, 16 Jul 2019 15:38:44 +0800 Subject: [PATCH] Add HTTP group feature Signed-off-by: stonezdj --- docs/swagger.yaml | 6 +- .../postgresql/0005_1.8.2_schema.up.sql | 25 ++++ src/common/config/metadata/metadatalist.go | 4 +- src/common/const.go | 7 +- src/common/dao/base.go | 1 + src/common/dao/config.go | 2 +- src/common/dao/dao_test.go | 3 - src/common/dao/group/usergroup.go | 45 ++++++- src/common/dao/group/usergroup_test.go | 78 ++++++++++--- src/common/dao/project_test.go | 21 ---- src/common/dao/testutils.go | 13 +++ src/common/models/config.go | 2 +- src/common/security/local/context_test.go | 4 +- src/common/utils/test/test.go | 7 +- src/core/api/api_test.go | 11 ++ src/core/api/email_test.go | 2 + src/core/api/projectmember.go | 39 ++++++- src/core/api/projectmember_test.go | 76 ++++++++++++ src/core/api/usergroup.go | 80 ++++++++----- src/core/api/usergroup_test.go | 46 +++++++- src/core/auth/authproxy/auth.go | 110 +++++++++++++++--- src/core/auth/authproxy/auth_test.go | 27 +++-- src/core/auth/authproxy/test/server.go | 11 ++ src/core/auth/ldap/ldap.go | 2 +- src/core/auth/ldap/ldap_test.go | 7 +- src/core/config/config.go | 4 +- src/core/config/config_test.go | 13 ++- src/core/filter/security.go | 57 +-------- src/core/filter/security_test.go | 9 +- src/pkg/authproxy/http.go | 65 +++++++++++ src/portal/lib/src/config/config.ts | 4 +- .../config/auth/config-auth.component.html | 12 +- src/portal/src/i18n/lang/en-us-lang.json | 2 +- src/portal/src/i18n/lang/es-es-lang.json | 2 +- src/portal/src/i18n/lang/fr-fr-lang.json | 2 +- src/portal/src/i18n/lang/pt-br-lang.json | 2 +- src/portal/src/i18n/lang/zh-cn-lang.json | 2 +- 37 files changed, 599 insertions(+), 204 deletions(-) create mode 100644 make/migrations/postgresql/0005_1.8.2_schema.up.sql create mode 100644 src/pkg/authproxy/http.go diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 804d4a44c..bfabcc98b 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -516,7 +516,7 @@ paths: '403': description: User in session does not have permission to the project. '409': - description: An LDAP user group with same DN already exist. + description: A user group with same group name already exist or an LDAP user group with same DN already exist. '500': description: Unexpected internal errors. '/projects/{project_id}/members/{mid}': @@ -2575,7 +2575,7 @@ paths: '403': description: User in session does not have permission to the user group. '409': - description: An LDAP user group with same DN already exist. + description: A user group with same group name already exist, or an LDAP user group with same DN already exist. '500': description: Unexpected internal errors. '/usergroups/{group_id}': @@ -4584,7 +4584,7 @@ definitions: description: The name of the user group group_type: type: integer - description: 'The group type, 1 for LDAP group.' + description: 'The group type, 1 for LDAP group, 2 for HTTP group.' ldap_group_dn: type: string description: The DN of the LDAP group if group type is 1 (LDAP group). diff --git a/make/migrations/postgresql/0005_1.8.2_schema.up.sql b/make/migrations/postgresql/0005_1.8.2_schema.up.sql new file mode 100644 index 000000000..0bf72529f --- /dev/null +++ b/make/migrations/postgresql/0005_1.8.2_schema.up.sql @@ -0,0 +1,25 @@ +/* +Rename the duplicate names before adding "UNIQUE" constraint +*/ +DO $$ +BEGIN + WHILE EXISTS (SELECT count(*) FROM user_group GROUP BY group_name HAVING count(*) > 1) LOOP + UPDATE user_group AS r + SET group_name = ( + /* + truncate the name if it is too long after appending the sequence number + */ + CASE WHEN (length(group_name)+length(v.seq::text)+1) > 256 + THEN + substring(group_name from 1 for (255-length(v.seq::text))) || '_' || v.seq + ELSE + group_name || '_' || v.seq + END + ) + FROM (SELECT id, row_number() OVER (PARTITION BY group_name ORDER BY id) AS seq FROM user_group) AS v + WHERE r.id = v.id AND v.seq > 1; + END LOOP; +END $$; + +ALTER TABLE user_group ADD CONSTRAINT unique_group_name UNIQUE (group_name); + diff --git a/src/common/config/metadata/metadatalist.go b/src/common/config/metadata/metadatalist.go index 202f426b7..d019081ea 100644 --- a/src/common/config/metadata/metadatalist.go +++ b/src/common/config/metadata/metadatalist.go @@ -91,7 +91,7 @@ var ( {Name: common.LDAPBaseDN, Scope: UserScope, Group: LdapBasicGroup, EnvKey: "LDAP_BASE_DN", DefaultValue: "", ItemType: &NonEmptyStringType{}, Editable: false}, {Name: common.LDAPFilter, Scope: UserScope, Group: LdapBasicGroup, EnvKey: "LDAP_FILTER", DefaultValue: "", ItemType: &StringType{}, Editable: false}, {Name: common.LDAPGroupBaseDN, Scope: UserScope, Group: LdapGroupGroup, EnvKey: "LDAP_GROUP_BASE_DN", DefaultValue: "", ItemType: &StringType{}, Editable: false}, - {Name: common.LdapGroupAdminDn, Scope: UserScope, Group: LdapGroupGroup, EnvKey: "LDAP_GROUP_ADMIN_DN", DefaultValue: "", ItemType: &StringType{}, Editable: false}, + {Name: common.LDAPGroupAdminDn, Scope: UserScope, Group: LdapGroupGroup, EnvKey: "LDAP_GROUP_ADMIN_DN", DefaultValue: "", ItemType: &StringType{}, Editable: false}, {Name: common.LDAPGroupAttributeName, Scope: UserScope, Group: LdapGroupGroup, EnvKey: "LDAP_GROUP_GID", DefaultValue: "", ItemType: &StringType{}, Editable: false}, {Name: common.LDAPGroupSearchFilter, Scope: UserScope, Group: LdapGroupGroup, EnvKey: "LDAP_GROUP_FILTER", DefaultValue: "", ItemType: &StringType{}, Editable: false}, {Name: common.LDAPGroupSearchScope, Scope: UserScope, Group: LdapGroupGroup, EnvKey: "LDAP_GROUP_SCOPE", DefaultValue: "2", ItemType: &LdapScopeType{}, Editable: false}, @@ -133,7 +133,7 @@ var ( {Name: common.HTTPAuthProxyEndpoint, Scope: UserScope, Group: HTTPAuthGroup, ItemType: &StringType{}}, {Name: common.HTTPAuthProxyTokenReviewEndpoint, Scope: UserScope, Group: HTTPAuthGroup, ItemType: &StringType{}}, {Name: common.HTTPAuthProxyVerifyCert, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "true", ItemType: &BoolType{}}, - {Name: common.HTTPAuthProxyAlwaysOnboard, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "false", ItemType: &BoolType{}}, + {Name: common.HTTPAuthProxySkipSearch, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "false", ItemType: &BoolType{}}, {Name: common.OIDCName, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}}, {Name: common.OIDCEndpoint, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}}, diff --git a/src/common/const.go b/src/common/const.go index 532e7960f..3826d1d2d 100644 --- a/src/common/const.go +++ b/src/common/const.go @@ -100,7 +100,7 @@ const ( HTTPAuthProxyEndpoint = "http_authproxy_endpoint" HTTPAuthProxyTokenReviewEndpoint = "http_authproxy_tokenreview_endpoint" HTTPAuthProxyVerifyCert = "http_authproxy_verify_cert" - HTTPAuthProxyAlwaysOnboard = "http_authproxy_always_onboard" + HTTPAuthProxySkipSearch = "http_authproxy_skip_search" OIDCName = "oidc_name" OIDCEndpoint = "oidc_endpoint" OIDCCLientID = "oidc_client_id" @@ -120,8 +120,9 @@ const ( NotaryURL = "notary_url" DefaultCoreEndpoint = "http://core:8080" DefaultNotaryEndpoint = "http://notary-server:4443" - LdapGroupType = 1 - LdapGroupAdminDn = "ldap_group_admin_dn" + LDAPGroupType = 1 + HTTPGroupType = 2 + LDAPGroupAdminDn = "ldap_group_admin_dn" LDAPGroupMembershipAttribute = "ldap_group_membership_attribute" DefaultRegistryControllerEndpoint = "http://registryctl:8080" WithChartMuseum = "with_chartmuseum" diff --git a/src/common/dao/base.go b/src/common/dao/base.go index 3e04867da..f1d3d2386 100644 --- a/src/common/dao/base.go +++ b/src/common/dao/base.go @@ -183,6 +183,7 @@ func paginateForQuerySetter(qs orm.QuerySeter, page, size int64) orm.QuerySeter // Escape .. func Escape(str string) string { + str = strings.Replace(str, `\`, `\\`, -1) str = strings.Replace(str, `%`, `\%`, -1) str = strings.Replace(str, `_`, `\_`, -1) return str diff --git a/src/common/dao/config.go b/src/common/dao/config.go index 65ec6e195..eea49cb30 100644 --- a/src/common/dao/config.go +++ b/src/common/dao/config.go @@ -54,7 +54,7 @@ func GetConfigEntries() ([]*models.ConfigEntry, error) { func SaveConfigEntries(entries []models.ConfigEntry) error { o := GetOrmer() for _, entry := range entries { - if entry.Key == common.LdapGroupAdminDn { + if entry.Key == common.LDAPGroupAdminDn { entry.Value = utils.TrimLower(entry.Value) } tempEntry := models.ConfigEntry{} diff --git a/src/common/dao/dao_test.go b/src/common/dao/dao_test.go index 646634226..0610fd305 100644 --- a/src/common/dao/dao_test.go +++ b/src/common/dao/dao_test.go @@ -302,9 +302,6 @@ func TestListUsers(t *testing.T) { if err != nil { t.Errorf("Error occurred in ListUsers: %v", err) } - if len(users) != 1 { - t.Errorf("Expect one user in list, but the acutal length is %d, the list: %+v", len(users), users) - } users2, err := ListUsers(&models.UserQuery{Username: username}) if len(users2) != 1 { t.Errorf("Expect one user in list, but the acutal length is %d, the list: %+v", len(users), users) diff --git a/src/common/dao/group/usergroup.go b/src/common/dao/group/usergroup.go index 95feb2d0b..f17be1003 100644 --- a/src/common/dao/group/usergroup.go +++ b/src/common/dao/group/usergroup.go @@ -15,24 +15,38 @@ package group import ( + "strings" "time" "github.com/goharbor/harbor/src/common/utils" + "fmt" + + "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/log" + "github.com/pkg/errors" ) +// ErrGroupNameDup ... +var ErrGroupNameDup = errors.New("duplicated user group name") + // AddUserGroup - Add User Group func AddUserGroup(userGroup models.UserGroup) (int, error) { + userGroupList, err := QueryUserGroup(models.UserGroup{GroupName: userGroup.GroupName, GroupType: common.HTTPGroupType}) + if err != nil { + return 0, ErrGroupNameDup + } + if len(userGroupList) > 0 { + return 0, ErrGroupNameDup + } o := dao.GetOrmer() - sql := "insert into user_group (group_name, group_type, ldap_group_dn, creation_time, update_time) values (?, ?, ?, ?, ?) RETURNING id" var id int now := time.Now() - err := o.Raw(sql, userGroup.GroupName, userGroup.GroupType, utils.TrimLower(userGroup.LdapGroupDN), now, now).QueryRow(&id) + err = o.Raw(sql, userGroup.GroupName, userGroup.GroupType, utils.TrimLower(userGroup.LdapGroupDN), now, now).QueryRow(&id) if err != nil { return 0, err } @@ -45,10 +59,10 @@ func QueryUserGroup(query models.UserGroup) ([]*models.UserGroup, error) { o := dao.GetOrmer() sql := `select id, group_name, group_type, ldap_group_dn from user_group where 1=1 ` sqlParam := make([]interface{}, 1) - groups := []*models.UserGroup{} + var groups []*models.UserGroup if len(query.GroupName) != 0 { - sql += ` and group_name like ? ` - sqlParam = append(sqlParam, `%`+dao.Escape(query.GroupName)+`%`) + sql += ` and group_name = ? ` + sqlParam = append(sqlParam, query.GroupName) } if query.GroupType != 0 { @@ -84,6 +98,27 @@ func GetUserGroup(id int) (*models.UserGroup, error) { return nil, nil } +// GetGroupIDByGroupName - Return the group ID by given group name. it is possible less group ID than the given group name if some group doesn't exist. +func GetGroupIDByGroupName(groupName []string, groupType int) ([]int, error) { + var retGroupID []int + var conditions []string + if len(groupName) == 0 { + return retGroupID, nil + } + for _, gName := range groupName { + con := "'" + gName + "'" + conditions = append(conditions, con) + } + sql := fmt.Sprintf("select id from user_group where group_name in ( %s ) and group_type = %v", strings.Join(conditions, ","), groupType) + o := dao.GetOrmer() + cnt, err := o.Raw(sql).QueryRows(&retGroupID) + if err != nil { + return retGroupID, err + } + log.Debugf("Found rows %v", cnt) + return retGroupID, nil +} + // DeleteUserGroup ... func DeleteUserGroup(id int) error { userGroup := models.UserGroup{ID: id} diff --git a/src/common/dao/group/usergroup_test.go b/src/common/dao/group/usergroup_test.go index f4801beea..1cd8919e1 100644 --- a/src/common/dao/group/usergroup_test.go +++ b/src/common/dao/group/usergroup_test.go @@ -17,6 +17,7 @@ package group import ( "fmt" "os" + "reflect" "testing" "github.com/goharbor/harbor/src/common" @@ -46,10 +47,13 @@ func TestMain(m *testing.M) { // Extract to test utils 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 harbor_user (username, email, password, realname) values ('grouptestu09', 'grouptestu09@example.com', '123456', 'grouptestu09')", "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')", + "insert into user_group (group_name, group_type, ldap_group_dn) values ('test_http_group', 2, '')", + "insert into user_group (group_name, group_type, ldap_group_dn) values ('test_myhttp_group', 2, '')", "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)", "insert into project_member (project_id, entity_id, entity_type, role) values ( (select project_id from project where name = 'member_test_01') , (select id from user_group where group_name = 'test_group_01'), 'g', 1)", @@ -59,11 +63,12 @@ func TestMain(m *testing.M) { "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 harbor_user where username='member_test_01' or username='pm_sample' or username='grouptestu09'", "delete from user_group", "delete from project_member", } - dao.PrepareTestData(clearSqls, initSqls) + dao.ExecuteBatchSQL(initSqls) + defer dao.ExecuteBatchSQL(clearSqls) result = m.Run() @@ -84,7 +89,7 @@ func TestAddUserGroup(t *testing.T) { want int wantErr bool }{ - {"Insert an ldap user group", args{userGroup: models.UserGroup{GroupName: "sample_group", GroupType: common.LdapGroupType, LdapGroupDN: "sample_ldap_dn_string"}}, 0, false}, + {"Insert an ldap user group", args{userGroup: models.UserGroup{GroupName: "sample_group", GroupType: common.LDAPGroupType, LdapGroupDN: "sample_ldap_dn_string"}}, 0, false}, {"Insert other user group", args{userGroup: models.UserGroup{GroupName: "other_group", GroupType: 3, LdapGroupDN: "other information"}}, 0, false}, } for _, tt := range tests { @@ -112,8 +117,8 @@ func TestQueryUserGroup(t *testing.T) { wantErr bool }{ {"Query all user group", args{query: models.UserGroup{GroupName: "test_group_01"}}, 1, false}, - {"Query all ldap group", args{query: models.UserGroup{GroupType: common.LdapGroupType}}, 2, false}, - {"Query ldap group with group property", args{query: models.UserGroup{GroupType: common.LdapGroupType, LdapGroupDN: "CN=harbor_users,OU=sample,OU=vmware,DC=harbor,DC=com"}}, 1, false}, + {"Query all ldap group", args{query: models.UserGroup{GroupType: common.LDAPGroupType}}, 2, false}, + {"Query ldap group with group property", args{query: models.UserGroup{GroupType: common.LDAPGroupType, LdapGroupDN: "CN=harbor_users,OU=sample,OU=vmware,DC=harbor,DC=com"}}, 1, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -130,7 +135,7 @@ func TestQueryUserGroup(t *testing.T) { } func TestGetUserGroup(t *testing.T) { - userGroup := models.UserGroup{GroupName: "insert_group", GroupType: common.LdapGroupType, LdapGroupDN: "ldap_dn_string"} + userGroup := models.UserGroup{GroupName: "insert_group", GroupType: common.LDAPGroupType, LdapGroupDN: "ldap_dn_string"} result, err := AddUserGroup(userGroup) if err != nil { t.Errorf("Error occurred when AddUserGroup: %v", err) @@ -235,13 +240,18 @@ func TestOnBoardUserGroup(t *testing.T) { args{g: &models.UserGroup{ GroupName: "harbor_example", LdapGroupDN: "cn=harbor_example,ou=groups,dc=example,dc=com", - GroupType: common.LdapGroupType}}, + GroupType: common.LDAPGroupType}}, false}, {"OnBoardUserGroup second time", args{g: &models.UserGroup{ GroupName: "harbor_example", LdapGroupDN: "cn=harbor_example,ou=groups,dc=example,dc=com", - GroupType: common.LdapGroupType}}, + GroupType: common.LDAPGroupType}}, + false}, + {"OnBoardUserGroup HTTP user group", + args{g: &models.UserGroup{ + GroupName: "test_myhttp_group", + GroupType: common.HTTPGroupType}}, false}, } for _, tt := range tests { @@ -254,12 +264,6 @@ func TestOnBoardUserGroup(t *testing.T) { } func TestGetGroupProjects(t *testing.T) { - userID, err := dao.Register(models.User{ - Username: "grouptestu09", - Email: "grouptest09@example.com", - Password: "Harbor123456", - }) - defer dao.DeleteUser(int(userID)) projectID1, err := dao.AddProject(models.Project{ Name: "grouptest01", OwnerID: 1, @@ -277,7 +281,7 @@ func TestGetGroupProjects(t *testing.T) { } defer dao.DeleteProject(projectID2) groupID, err := AddUserGroup(models.UserGroup{ - GroupName: "test_group_01", + GroupName: "test_group_03", GroupType: 1, LdapGroupDN: "cn=harbor_users,ou=groups,dc=example,dc=com", }) @@ -344,7 +348,7 @@ func TestGetTotalGroupProjects(t *testing.T) { } defer dao.DeleteProject(projectID2) groupID, err := AddUserGroup(models.UserGroup{ - GroupName: "test_group_01", + GroupName: "test_group_05", GroupType: 1, LdapGroupDN: "cn=harbor_users,ou=groups,dc=example,dc=com", }) @@ -428,3 +432,45 @@ func TestGetRolesByLDAPGroup(t *testing.T) { }) } } + +func TestGetGroupIDByGroupName(t *testing.T) { + groupList, err := QueryUserGroup(models.UserGroup{GroupName: "test_http_group", GroupType: 2}) + if err != nil { + t.Error(err) + } + if len(groupList) < 0 { + t.Error(err) + } + groupList2, err := QueryUserGroup(models.UserGroup{GroupName: "test_myhttp_group", GroupType: 2}) + if err != nil { + t.Error(err) + } + if len(groupList2) < 0 { + t.Error(err) + } + var expectGroupID []int + type args struct { + groupName []string + } + tests := []struct { + name string + args args + want []int + wantErr bool + }{ + {"empty query", args{groupName: []string{}}, expectGroupID, false}, + {"normal query", args{groupName: []string{"test_http_group", "test_myhttp_group"}}, []int{groupList[0].ID, groupList2[0].ID}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := GetGroupIDByGroupName(tt.args.groupName, common.HTTPGroupType) + if (err != nil) != tt.wantErr { + t.Errorf("GetHTTPGroupIDByGroupName() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetHTTPGroupIDByGroupName() = %#v, want %#v", got, tt.want) + } + }) + } +} diff --git a/src/common/dao/project_test.go b/src/common/dao/project_test.go index d3cc1d469..b35200047 100644 --- a/src/common/dao/project_test.go +++ b/src/common/dao/project_test.go @@ -118,27 +118,6 @@ func Test_projectQueryConditions(t *testing.T) { } } -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')`, - `insert into harbor_user (username, email, password, realname) values ('sample01', 'sample01@example.com', 'harbor12345', 'sample01')`, - `insert into project (name, owner_id) values ('group_project', 1)`, - `insert into project (name, owner_id) values ('group_project_private', 1)`, - `insert into project_metadata (project_id, name, value) values ((select project_id from project where name = 'group_project'), 'public', 'false')`, - `insert into project_metadata (project_id, name, value) values ((select project_id from project where name = 'group_project_private'), 'public', 'false')`, - `insert into project_member (project_id, entity_id, entity_type, role) values ((select project_id from project where name = 'group_project'), (select id from user_group where group_name = 'harbor_group_01'),'g', 2)`, - } - - clearSqls := []string{ - `delete from project_metadata where project_id in (select project_id from project where name in ('group_project', 'group_project_private'))`, - `delete from project where name in ('group_project', 'group_project_private')`, - `delete from project_member where project_id in (select project_id from project where name in ('group_project', 'group_project_private'))`, - `delete from user_group where group_name = 'harbor_group_01'`, - `delete from harbor_user where username = 'sample01'`, - } - PrepareTestData(clearSqls, initSqls) -} - func TestProjetExistsByName(t *testing.T) { name := "project_exist_by_name_test" exist := ProjectExistsByName(name) diff --git a/src/common/dao/testutils.go b/src/common/dao/testutils.go index 95d6c3ab7..910d5af72 100644 --- a/src/common/dao/testutils.go +++ b/src/common/dao/testutils.go @@ -120,6 +120,19 @@ func PrepareTestData(clearSqls []string, initSqls []string) { } } +// ExecuteBatchSQL ... +func ExecuteBatchSQL(sqls []string) { + o := GetOrmer() + + for _, sql := range sqls { + fmt.Printf("Exec sql:%v\n", sql) + _, err := o.Raw(sql).Exec() + if err != nil { + fmt.Printf("failed to execute batch sql, sql:%v, error: %v", sql, err) + } + } +} + // ArrayEqual ... func ArrayEqual(arrayA, arrayB []int) bool { if len(arrayA) != len(arrayB) { diff --git a/src/common/models/config.go b/src/common/models/config.go index cbcb3f810..588e7dd84 100644 --- a/src/common/models/config.go +++ b/src/common/models/config.go @@ -70,7 +70,7 @@ type HTTPAuthProxy struct { Endpoint string `json:"endpoint"` TokenReviewEndpoint string `json:"tokenreivew_endpoint"` VerifyCert bool `json:"verify_cert"` - AlwaysOnBoard bool `json:"always_onboard"` + SkipSearch bool `json:"skip_search"` } // OIDCSetting wraps the settings for OIDC auth endpoint diff --git a/src/common/security/local/context_test.go b/src/common/security/local/context_test.go index 36436010a..30159d559 100644 --- a/src/common/security/local/context_test.go +++ b/src/common/security/local/context_test.go @@ -255,7 +255,7 @@ func TestHasPushPullPermWithGroup(t *testing.T) { t.Errorf("Error occurred when GetUser: %v", err) } - userGroups, err := group.QueryUserGroup(models.UserGroup{GroupType: common.LdapGroupType, 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) } @@ -340,7 +340,7 @@ func TestSecurityContext_GetRolesByGroup(t *testing.T) { if err != nil { t.Errorf("Error occurred when GetUser: %v", err) } - userGroups, err := group.QueryUserGroup(models.UserGroup{GroupType: common.LdapGroupType, 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) } diff --git a/src/common/utils/test/test.go b/src/common/utils/test/test.go index 28046e5db..73e107741 100644 --- a/src/common/utils/test/test.go +++ b/src/common/utils/test/test.go @@ -22,10 +22,11 @@ import ( "strings" "fmt" - "github.com/goharbor/harbor/src/common" - "github.com/gorilla/mux" "os" "sort" + + "github.com/goharbor/harbor/src/common" + "github.com/gorilla/mux" ) // RequestHandlerMapping is a mapping between request and its handler @@ -120,7 +121,7 @@ func GetUnitTestConfig() map[string]interface{} { common.LDAPGroupBaseDN: "dc=example,dc=com", common.LDAPGroupAttributeName: "cn", common.LDAPGroupSearchScope: 2, - common.LdapGroupAdminDn: "cn=harbor_users,ou=groups,dc=example,dc=com", + common.LDAPGroupAdminDn: "cn=harbor_users,ou=groups,dc=example,dc=com", common.WithNotary: "false", common.WithChartMuseum: "false", common.SelfRegistration: "true", diff --git a/src/core/api/api_test.go b/src/core/api/api_test.go index 8b9d3bfaf..f8e1ccdd0 100644 --- a/src/core/api/api_test.go +++ b/src/core/api/api_test.go @@ -207,6 +207,17 @@ func TestMain(m *testing.M) { if err := prepare(); err != nil { panic(err) } + dao.ExecuteBatchSQL([]string{ + "insert into user_group (group_name, group_type, ldap_group_dn) values ('test_group_01_api', 1, 'cn=harbor_users,ou=sample,ou=vmware,dc=harbor,dc=com')", + "insert into user_group (group_name, group_type, ldap_group_dn) values ('vsphere.local\\administrators', 2, '')", + }) + + defer dao.ExecuteBatchSQL([]string{ + "delete from harbor_label", + "delete from robot", + "delete from user_group", + "delete from project_member", + }) ret := m.Run() clean() diff --git a/src/core/api/email_test.go b/src/core/api/email_test.go index c38fbbb29..7fff60776 100644 --- a/src/core/api/email_test.go +++ b/src/core/api/email_test.go @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +// +build !darwin + package api import ( diff --git a/src/core/api/projectmember.go b/src/core/api/projectmember.go index 0ede80dd4..5495ac26a 100644 --- a/src/core/api/projectmember.go +++ b/src/core/api/projectmember.go @@ -23,11 +23,13 @@ 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" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/auth" + "github.com/goharbor/harbor/src/core/config" ) // ProjectMemberAPI handles request to /api/projects/{}/members/{} @@ -37,6 +39,7 @@ type ProjectMemberAPI struct { entityID int entityType string project *models.Project + groupType int } // ErrDuplicateProjectMember ... @@ -84,6 +87,15 @@ func (pma *ProjectMemberAPI) Prepare() { return } pma.id = int(pmid) + authMode, err := config.AuthMode() + if err != nil { + pma.SendInternalServerError(fmt.Errorf("failed to get authentication mode")) + } + if authMode == common.LDAPAuth { + pma.groupType = common.LDAPGroupType + } else if authMode == common.HTTPAuth { + pma.groupType = common.HTTPGroupType + } } func (pma *ProjectMemberAPI) requireAccess(action rbac.Action) bool { @@ -131,7 +143,7 @@ func (pma *ProjectMemberAPI) Get() { return } if len(memberList) == 0 { - pma.SendNotFoundError(fmt.Errorf("The project member does not exit, pmid:%v", pma.id)) + pma.SendNotFoundError(fmt.Errorf("The project member does not exist, pmid:%v", pma.id)) return } @@ -161,10 +173,10 @@ func (pma *ProjectMemberAPI) Post() { pma.SendBadRequestError(fmt.Errorf("Failed to add project member, error: %v", err)) return } else if err == auth.ErrDuplicateLDAPGroup { - pma.SendConflictError(fmt.Errorf("Failed to add project member, already exist LDAP group or project member, groupDN:%v", request.MemberGroup.LdapGroupDN)) + pma.SendConflictError(fmt.Errorf("Failed to add project member, already exist group or project member, groupDN:%v", request.MemberGroup.LdapGroupDN)) return } else if err == ErrDuplicateProjectMember { - pma.SendConflictError(fmt.Errorf("Failed to add project member, already exist LDAP group or project member, groupMemberID:%v", request.MemberGroup.ID)) + pma.SendConflictError(fmt.Errorf("Failed to add project member, already exist group or project member, groupMemberID:%v", request.MemberGroup.ID)) return } else if err == ErrInvalidRole { pma.SendBadRequestError(fmt.Errorf("Invalid role ID, role ID %v", request.Role)) @@ -220,12 +232,13 @@ func AddProjectMember(projectID int64, request models.MemberReq) (int, error) { var member models.Member member.ProjectID = projectID member.Role = request.Role + member.EntityType = common.GroupMember + if request.MemberUser.UserID > 0 { member.EntityID = request.MemberUser.UserID member.EntityType = common.UserMember } else if request.MemberGroup.ID > 0 { member.EntityID = request.MemberGroup.ID - member.EntityType = common.GroupMember } else if len(request.MemberUser.Username) > 0 { var userID int member.EntityType = common.UserMember @@ -243,14 +256,28 @@ func AddProjectMember(projectID int64, request models.MemberReq) (int, error) { } member.EntityID = userID } else if len(request.MemberGroup.LdapGroupDN) > 0 { - + request.MemberGroup.GroupType = common.LDAPGroupType // If groupname provided, use the provided groupname to name this group groupID, err := auth.SearchAndOnBoardGroup(request.MemberGroup.LdapGroupDN, request.MemberGroup.GroupName) if err != nil { return 0, err } member.EntityID = groupID - member.EntityType = common.GroupMember + } else if len(request.MemberGroup.GroupName) > 0 && request.MemberGroup.GroupType == common.HTTPGroupType { + ugs, err := group.QueryUserGroup(models.UserGroup{GroupName: request.MemberGroup.GroupName, GroupType: common.HTTPGroupType}) + if err != nil { + return 0, err + } + if len(ugs) == 0 { + groupID, err := auth.SearchAndOnBoardGroup(request.MemberGroup.GroupName, "") + if err != nil { + return 0, err + } + member.EntityID = groupID + } else { + member.EntityID = ugs[0].ID + } + } if member.EntityID <= 0 { return 0, fmt.Errorf("Can not get valid member entity, request: %+v", request) diff --git a/src/core/api/projectmember_test.go b/src/core/api/projectmember_test.go index bd7b7d043..88e47851f 100644 --- a/src/core/api/projectmember_test.go +++ b/src/core/api/projectmember_test.go @@ -20,6 +20,7 @@ import ( "testing" "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" ) @@ -94,6 +95,21 @@ func TestProjectMemberAPI_Post(t *testing.T) { t.Errorf("Error occurred when create user: %v", err) } + ugList, err := group.QueryUserGroup(models.UserGroup{GroupType: 1, LdapGroupDN: "cn=harbor_users,ou=sample,ou=vmware,dc=harbor,dc=com"}) + if err != nil { + t.Errorf("Failed to query the user group") + } + if len(ugList) <= 0 { + t.Errorf("Failed to query the user group") + } + httpUgList, err := group.QueryUserGroup(models.UserGroup{GroupType: 2, GroupName: "vsphere.local\\administrators"}) + if err != nil { + t.Errorf("Failed to query the user group") + } + if len(httpUgList) <= 0 { + t.Errorf("Failed to query the user group") + } + cases := []*codeCheckingCase{ // 401 { @@ -167,6 +183,66 @@ func TestProjectMemberAPI_Post(t *testing.T) { }, code: http.StatusOK, }, + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/projects/1/members", + credential: admin, + bodyJSON: &models.MemberReq{ + Role: 1, + MemberGroup: models.UserGroup{ + GroupType: 1, + LdapGroupDN: "cn=harbor_users,ou=groups,dc=example,dc=com", + }, + }, + }, + code: http.StatusBadRequest, + }, + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/projects/1/members", + credential: admin, + bodyJSON: &models.MemberReq{ + Role: 1, + MemberGroup: models.UserGroup{ + GroupType: 2, + ID: httpUgList[0].ID, + }, + }, + }, + code: http.StatusCreated, + }, + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/projects/1/members", + credential: admin, + bodyJSON: &models.MemberReq{ + Role: 1, + MemberGroup: models.UserGroup{ + GroupType: 1, + ID: ugList[0].ID, + }, + }, + }, + code: http.StatusCreated, + }, + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/projects/1/members", + credential: admin, + bodyJSON: &models.MemberReq{ + Role: 1, + MemberGroup: models.UserGroup{ + GroupType: 2, + GroupName: "vsphere.local/users", + }, + }, + }, + code: http.StatusBadRequest, + }, } runCodeCheckingCases(t, cases...) } diff --git a/src/core/api/usergroup.go b/src/core/api/usergroup.go index 317ab4362..3bfd2d34e 100644 --- a/src/core/api/usergroup.go +++ b/src/core/api/usergroup.go @@ -27,12 +27,14 @@ import ( "github.com/goharbor/harbor/src/common/utils/ldap" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/auth" + "github.com/goharbor/harbor/src/core/config" ) // UserGroupAPI ... type UserGroupAPI struct { BaseController - id int + id int + groupType int } const ( @@ -61,6 +63,15 @@ func (uga *UserGroupAPI) Prepare() { uga.SendForbiddenError(errors.New(uga.SecurityCtx.GetUsername())) return } + authMode, err := config.AuthMode() + if err != nil { + uga.SendInternalServerError(errors.New("failed to get authentication mode")) + } + if authMode == common.LDAPAuth { + uga.groupType = common.LDAPGroupType + } else if authMode == common.HTTPAuth { + uga.groupType = common.HTTPGroupType + } } // Get ... @@ -69,7 +80,7 @@ func (uga *UserGroupAPI) Get() { uga.Data["json"] = make([]models.UserGroup, 0) if ID == 0 { // user group id not set, return all user group - query := models.UserGroup{GroupType: common.LdapGroupType} // Current query LDAP group only + query := models.UserGroup{GroupType: uga.groupType} userGroupList, err := group.QueryUserGroup(query) if err != nil { uga.SendInternalServerError(fmt.Errorf("failed to query database for user group list, error: %v", err)) @@ -103,41 +114,50 @@ func (uga *UserGroupAPI) Post() { } userGroup.ID = 0 - userGroup.GroupType = common.LdapGroupType + if userGroup.GroupType == 0 { + userGroup.GroupType = uga.groupType + } userGroup.LdapGroupDN = strings.TrimSpace(userGroup.LdapGroupDN) userGroup.GroupName = strings.TrimSpace(userGroup.GroupName) if len(userGroup.GroupName) == 0 { uga.SendBadRequestError(errors.New(userNameEmptyMsg)) return } - query := models.UserGroup{GroupType: userGroup.GroupType, LdapGroupDN: userGroup.LdapGroupDN} - result, err := group.QueryUserGroup(query) - if err != nil { - uga.SendInternalServerError(fmt.Errorf("error occurred in add user group, error: %v", err)) - return - } - if len(result) > 0 { - uga.SendConflictError(errors.New("error occurred in add user group, duplicate user group exist")) - return - } - // User can not add ldap group when the ldap server is offline - ldapGroup, err := auth.SearchGroup(userGroup.LdapGroupDN) - if err == ldap.ErrNotFound || ldapGroup == nil { - uga.SendBadRequestError(fmt.Errorf("LDAP Group DN is not found: DN:%v", userGroup.LdapGroupDN)) - return - } - if err == ldap.ErrDNSyntax { - uga.SendBadRequestError(fmt.Errorf("invalid DN syntax. DN: %v", userGroup.LdapGroupDN)) - return - } - if err != nil { - uga.SendInternalServerError(fmt.Errorf("Error occurred in search user group. error: %v", err)) - return + + if userGroup.GroupType == common.LDAPGroupType { + query := models.UserGroup{GroupType: userGroup.GroupType, LdapGroupDN: userGroup.LdapGroupDN} + result, err := group.QueryUserGroup(query) + if err != nil { + uga.SendInternalServerError(fmt.Errorf("error occurred in add user group, error: %v", err)) + return + } + if len(result) > 0 { + uga.SendConflictError(errors.New("error occurred in add user group, duplicate user group exist")) + return + } + // User can not add ldap group when the ldap server is offline + ldapGroup, err := auth.SearchGroup(userGroup.LdapGroupDN) + if err == ldap.ErrNotFound || ldapGroup == nil { + uga.SendBadRequestError(fmt.Errorf("LDAP Group DN is not found: DN:%v", userGroup.LdapGroupDN)) + return + } + if err == ldap.ErrDNSyntax { + uga.SendBadRequestError(fmt.Errorf("invalid DN syntax. DN: %v", userGroup.LdapGroupDN)) + return + } + if err != nil { + uga.SendInternalServerError(fmt.Errorf("error occurred in search user group. error: %v", err)) + return + } } groupID, err := group.AddUserGroup(userGroup) if err != nil { - uga.SendInternalServerError(fmt.Errorf("Error occurred in add user group, error: %v", err)) + if err == group.ErrGroupNameDup { + uga.SendConflictError(fmt.Errorf("duplicated user group name %s", userGroup.GroupName)) + return + } + uga.SendInternalServerError(fmt.Errorf("error occurred in add user group, error: %v", err)) return } uga.Redirect(http.StatusCreated, strconv.FormatInt(int64(groupID), 10)) @@ -150,13 +170,17 @@ func (uga *UserGroupAPI) Put() { uga.SendBadRequestError(err) return } + if userGroup.GroupType == common.HTTPGroupType { + uga.SendBadRequestError(errors.New("HTTP group is not allowed to update")) + return + } ID := uga.id userGroup.GroupName = strings.TrimSpace(userGroup.GroupName) if len(userGroup.GroupName) == 0 { uga.SendBadRequestError(errors.New(userNameEmptyMsg)) return } - userGroup.GroupType = common.LdapGroupType + userGroup.GroupType = common.LDAPGroupType log.Debugf("Updated user group %v", userGroup) err := group.UpdateUserGroupName(ID, userGroup.GroupName) if err != nil { diff --git a/src/core/api/usergroup_test.go b/src/core/api/usergroup_test.go index ebeeefb4d..dad91080e 100644 --- a/src/core/api/usergroup_test.go +++ b/src/core/api/usergroup_test.go @@ -35,7 +35,7 @@ func TestUserGroupAPI_GetAndDelete(t *testing.T) { groupID, err := group.AddUserGroup(models.UserGroup{ GroupName: "harbor_users", LdapGroupDN: "cn=harbor_users,ou=groups,dc=example,dc=com", - GroupType: common.LdapGroupType, + GroupType: common.LDAPGroupType, }) if err != nil { @@ -88,7 +88,7 @@ func TestUserGroupAPI_Post(t *testing.T) { groupID, err := group.AddUserGroup(models.UserGroup{ GroupName: "harbor_group", LdapGroupDN: "cn=harbor_group,ou=groups,dc=example,dc=com", - GroupType: common.LdapGroupType, + GroupType: common.LDAPGroupType, }) if err != nil { t.Errorf("Error occurred when AddUserGroup: %v", err) @@ -104,7 +104,32 @@ func TestUserGroupAPI_Post(t *testing.T) { bodyJSON: &models.UserGroup{ GroupName: "harbor_group", LdapGroupDN: "cn=harbor_group,ou=groups,dc=example,dc=com", - GroupType: common.LdapGroupType, + GroupType: common.LDAPGroupType, + }, + credential: admin, + }, + code: http.StatusConflict, + }, + // 201 + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/usergroups", + bodyJSON: &models.UserGroup{ + GroupName: "vsphere.local\\guest", + GroupType: common.HTTPGroupType, + }, + credential: admin, + }, + code: http.StatusCreated, + }, + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/usergroups", + bodyJSON: &models.UserGroup{ + GroupName: "vsphere.local\\guest", + GroupType: common.HTTPGroupType, }, credential: admin, }, @@ -118,7 +143,7 @@ func TestUserGroupAPI_Put(t *testing.T) { groupID, err := group.AddUserGroup(models.UserGroup{ GroupName: "harbor_group", LdapGroupDN: "cn=harbor_groups,ou=groups,dc=example,dc=com", - GroupType: common.LdapGroupType, + GroupType: common.LDAPGroupType, }) defer group.DeleteUserGroup(groupID) @@ -149,6 +174,19 @@ func TestUserGroupAPI_Put(t *testing.T) { }, code: http.StatusOK, }, + // 400 + { + request: &testingRequest{ + method: http.MethodPut, + url: fmt.Sprintf("/api/usergroups/%d", groupID), + bodyJSON: &models.UserGroup{ + GroupName: "my_group", + GroupType: common.HTTPGroupType, + }, + credential: admin, + }, + code: http.StatusBadRequest, + }, } runCodeCheckingCases(t, cases...) } diff --git a/src/core/auth/authproxy/auth.go b/src/core/auth/authproxy/auth.go index e388cbad6..9081ad8ea 100644 --- a/src/core/auth/authproxy/auth.go +++ b/src/core/auth/authproxy/auth.go @@ -16,18 +16,24 @@ package authproxy import ( "crypto/tls" + "encoding/json" "fmt" + "io/ioutil" + "net/http" + "strings" + "sync" + "time" + + "github.com/goharbor/harbor/src/common/dao/group" + "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/log" "github.com/goharbor/harbor/src/core/auth" "github.com/goharbor/harbor/src/core/config" - "io/ioutil" - "net/http" - "strings" - "sync" - "time" + "github.com/goharbor/harbor/src/pkg/authproxy" + k8s_api_v1beta1 "k8s.io/api/authentication/v1beta1" ) const refreshDuration = 2 * time.Second @@ -45,11 +51,16 @@ var insecureTransport = &http.Transport{ type Auth struct { auth.DefaultAuthenticateHelper sync.Mutex - Endpoint string - SkipCertVerify bool - AlwaysOnboard bool - settingTimeStamp time.Time - client *http.Client + Endpoint string + TokenReviewEndpoint string + SkipCertVerify bool + SkipSearch bool + settingTimeStamp time.Time + client *http.Client +} + +type session struct { + SessionID string `json:"session_id,omitempty"` } // Authenticate issues http POST request to Endpoint if it returns 200 the authentication is considered success. @@ -72,7 +83,39 @@ func (a *Auth) Authenticate(m models.AuthModel) (*models.User, error) { } defer resp.Body.Close() if resp.StatusCode == http.StatusOK { - return &models.User{Username: m.Principal}, nil + user := &models.User{Username: m.Principal} + data, err := ioutil.ReadAll(resp.Body) + if err != nil { + log.Warningf("Failed to read response body, error: %v", err) + return nil, auth.ErrAuth{} + } + s := session{} + err = json.Unmarshal(data, &s) + if err != nil { + log.Errorf("failed to read session %v", err) + } + + reviewResponse, err := a.tokenReview(s.SessionID) + if err != nil { + return nil, err + } + if reviewResponse == nil { + return nil, auth.ErrAuth{} + } + + // Attach user group ID information + ugList := reviewResponse.Status.User.Groups + log.Debugf("user groups %+v", ugList) + if len(ugList) > 0 { + groupIDList, err := group.GetGroupIDByGroupName(ugList, common.HTTPGroupType) + if err != nil { + return nil, err + } + log.Debugf("current user's group ID list is %+v", groupIDList) + user.GroupIDs = groupIDList + } + return user, nil + } else if resp.StatusCode == http.StatusUnauthorized { return nil, auth.ErrAuth{} } else { @@ -81,10 +124,19 @@ func (a *Auth) Authenticate(m models.AuthModel) (*models.User, error) { log.Warningf("Failed to read response body, error: %v", err) } return nil, fmt.Errorf("failed to authenticate, status code: %d, text: %s", resp.StatusCode, string(data)) + } } +func (a *Auth) tokenReview(sessionID string) (*k8s_api_v1beta1.TokenReview, error) { + httpAuthProxySetting, err := config.HTTPAuthProxySetting() + if err != nil { + return nil, err + } + return authproxy.TokenReview(sessionID, httpAuthProxySetting) +} + // OnBoardUser delegates to dao pkg to insert/update data in DB. func (a *Auth) OnBoardUser(u *models.User) error { return dao.OnBoardUser(u) @@ -102,14 +154,14 @@ func (a *Auth) PostAuthenticate(u *models.User) error { } // SearchUser returns nil as authproxy does not have such capability. -// When AlwaysOnboard is set it always return the default model. +// When SkipSearch is set it always return the default model. func (a *Auth) SearchUser(username string) (*models.User, error) { err := a.ensure() if err != nil { log.Warningf("Failed to refresh configuration for HTTP Auth Proxy Authenticator, error: %v, the default settings will be used", err) } var u *models.User - if a.AlwaysOnboard { + if a.SkipSearch { u = &models.User{Username: username} if err := a.fillInModel(u); err != nil { return nil, err @@ -118,6 +170,35 @@ func (a *Auth) SearchUser(username string) (*models.User, error) { return u, nil } +// 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) (*models.UserGroup, error) { + err := a.ensure() + if err != nil { + log.Warningf("Failed to refresh configuration for HTTP Auth Proxy Authenticator, error: %v, the default settings will be used", err) + } + var ug *models.UserGroup + if a.SkipSearch { + ug = &models.UserGroup{ + GroupName: groupKey, + GroupType: common.HTTPGroupType, + } + return ug, nil + } + return nil, nil +} + +// 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 err != nil { + return err + } + u.ID = userGroup.ID + return nil +} + func (a *Auth) fillInModel(u *models.User) error { if strings.TrimSpace(u.Username) == "" { return fmt.Errorf("username cannot be empty") @@ -145,8 +226,9 @@ func (a *Auth) ensure() error { return err } a.Endpoint = setting.Endpoint + a.TokenReviewEndpoint = setting.TokenReviewEndpoint a.SkipCertVerify = !setting.VerifyCert - a.AlwaysOnboard = setting.AlwaysOnBoard + a.SkipSearch = setting.SkipSearch } if a.SkipCertVerify { a.client.Transport = insecureTransport diff --git a/src/core/auth/authproxy/auth_test.go b/src/core/auth/authproxy/auth_test.go index 0e45b7388..07c035211 100644 --- a/src/core/auth/authproxy/auth_test.go +++ b/src/core/auth/authproxy/auth_test.go @@ -15,18 +15,20 @@ package authproxy import ( + "net/http/httptest" + "os" + "testing" + "time" + "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" cut "github.com/goharbor/harbor/src/common/utils/test" "github.com/goharbor/harbor/src/core/auth" "github.com/goharbor/harbor/src/core/auth/authproxy/test" "github.com/goharbor/harbor/src/core/config" "github.com/stretchr/testify/assert" - "net/http/httptest" - "os" - "testing" - "time" ) var mockSvr *httptest.Server @@ -42,15 +44,16 @@ func TestMain(m *testing.M) { mockSvr = test.NewMockServer(map[string]string{"jt": "pp", "Admin@vsphere.local": "Admin!23"}) defer mockSvr.Close() a = &Auth{ - Endpoint: mockSvr.URL + "/test/login", - SkipCertVerify: true, + Endpoint: mockSvr.URL + "/test/login", + TokenReviewEndpoint: mockSvr.URL + "/test/tokenreview", + SkipCertVerify: true, // So it won't require mocking the cfgManager settingTimeStamp: time.Now(), } conf := map[string]interface{}{ - common.HTTPAuthProxyEndpoint: "dummy", - common.HTTPAuthProxyTokenReviewEndpoint: "dummy", - common.HTTPAuthProxyVerifyCert: "false", + common.HTTPAuthProxyEndpoint: a.Endpoint, + common.HTTPAuthProxyTokenReviewEndpoint: a.TokenReviewEndpoint, + common.HTTPAuthProxyVerifyCert: !a.SkipCertVerify, } config.InitWithSettings(conf) @@ -64,6 +67,10 @@ func TestMain(m *testing.M) { } func TestAuth_Authenticate(t *testing.T) { + groupIDs, err := group.GetGroupIDByGroupName([]string{"vsphere.local\\users", "vsphere.local\\administrators"}, common.HTTPGroupType) + if err != nil { + t.Fatal("Failed to get groupIDs") + } t.Log("auth endpoint: ", a.Endpoint) type output struct { user models.User @@ -80,6 +87,7 @@ func TestAuth_Authenticate(t *testing.T) { expect: output{ user: models.User{ Username: "jt", + GroupIDs: groupIDs, }, err: nil, }, @@ -92,6 +100,7 @@ func TestAuth_Authenticate(t *testing.T) { expect: output{ user: models.User{ Username: "Admin@vsphere.local", + GroupIDs: groupIDs, // Email: "Admin@placeholder.com", // Password: pwd, // Comment: fmt.Sprintf(cmtTmpl, path.Join(mockSvr.URL, "/test/login")), diff --git a/src/core/auth/authproxy/test/server.go b/src/core/auth/authproxy/test/server.go index b11ec17aa..6fead0196 100644 --- a/src/core/auth/authproxy/test/server.go +++ b/src/core/auth/authproxy/test/server.go @@ -41,9 +41,20 @@ func (ah *authHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { } } +type reviewTokenHandler struct { +} + +func (rth *reviewTokenHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { + if req.Method != http.MethodPost { + http.Error(rw, "", http.StatusMethodNotAllowed) + } + rw.Write([]byte(`{"apiVersion": "authentication.k8s.io/v1beta1", "kind": "TokenReview", "status": {"authenticated": true, "user": {"username": "administrator@vsphere.local", "groups": ["vsphere.local\\users", "vsphere.local\\administrators", "vsphere.local\\caadmins", "vsphere.local\\systemconfiguration.bashshelladministrators", "vsphere.local\\systemconfiguration.administrators", "vsphere.local\\licenseservice.administrators", "vsphere.local\\everyone"], "extra": {"method": ["basic"]}}}}`)) +} + // NewMockServer creates the mock server for testing func NewMockServer(creds map[string]string) *httptest.Server { mux := http.NewServeMux() mux.Handle("/test/login", &authHandler{m: creds}) + mux.Handle("/test/tokenreview", &reviewTokenHandler{}) return httptest.NewTLSServer(mux) } diff --git a/src/core/auth/ldap/ldap.go b/src/core/auth/ldap/ldap.go index 9009d3e23..cf1963f7c 100644 --- a/src/core/auth/ldap/ldap.go +++ b/src/core/auth/ldap/ldap.go @@ -205,7 +205,7 @@ func (l *Auth) OnBoardGroup(u *models.UserGroup, altGroupName string) error { if len(altGroupName) > 0 { u.GroupName = altGroupName } - u.GroupType = common.LdapGroupType + u.GroupType = common.LDAPGroupType // Check duplicate LDAP DN in usergroup, if usergroup exist, return error userGroupList, err := group.QueryUserGroup(models.UserGroup{LdapGroupDN: u.LdapGroupDN}) if err != nil { diff --git a/src/core/auth/ldap/ldap_test.go b/src/core/auth/ldap/ldap_test.go index 5eb852fbe..498ffee2a 100644 --- a/src/core/auth/ldap/ldap_test.go +++ b/src/core/auth/ldap/ldap_test.go @@ -55,7 +55,7 @@ var ldapTestConfig = map[string]interface{}{ common.LDAPGroupBaseDN: "dc=example,dc=com", common.LDAPGroupAttributeName: "cn", common.LDAPGroupSearchScope: 2, - common.LdapGroupAdminDn: "cn=harbor_users,ou=groups,dc=example,dc=com", + common.LDAPGroupAdminDn: "cn=harbor_users,ou=groups,dc=example,dc=com", } func TestMain(m *testing.M) { @@ -92,8 +92,8 @@ func TestMain(m *testing.M) { "delete from user_group", "delete from project_member", } - dao.PrepareTestData(clearSqls, initSqls) - + dao.ExecuteBatchSQL(initSqls) + defer dao.ExecuteBatchSQL(clearSqls) retCode := m.Run() os.Exit(retCode) } @@ -405,6 +405,7 @@ func TestAddProjectMemberWithLdapGroup(t *testing.T) { ProjectID: currentProject.ProjectID, MemberGroup: models.UserGroup{ LdapGroupDN: "cn=harbor_users,ou=groups,dc=example,dc=com", + GroupType: 1, }, Role: models.PROJECTADMIN, } diff --git a/src/core/config/config.go b/src/core/config/config.go index 31ed8cadc..149fc3dd2 100644 --- a/src/core/config/config.go +++ b/src/core/config/config.go @@ -224,7 +224,7 @@ func LDAPGroupConf() (*models.LdapGroupConf, error) { LdapGroupFilter: cfgMgr.Get(common.LDAPGroupSearchFilter).GetString(), LdapGroupNameAttribute: cfgMgr.Get(common.LDAPGroupAttributeName).GetString(), LdapGroupSearchScope: cfgMgr.Get(common.LDAPGroupSearchScope).GetInt(), - LdapGroupAdminDN: cfgMgr.Get(common.LdapGroupAdminDn).GetString(), + LdapGroupAdminDN: cfgMgr.Get(common.LDAPGroupAdminDn).GetString(), LdapGroupMembershipAttribute: cfgMgr.Get(common.LDAPGroupMembershipAttribute).GetString(), }, nil } @@ -482,7 +482,7 @@ func HTTPAuthProxySetting() (*models.HTTPAuthProxy, error) { Endpoint: cfgMgr.Get(common.HTTPAuthProxyEndpoint).GetString(), TokenReviewEndpoint: cfgMgr.Get(common.HTTPAuthProxyTokenReviewEndpoint).GetString(), VerifyCert: cfgMgr.Get(common.HTTPAuthProxyVerifyCert).GetBool(), - AlwaysOnBoard: cfgMgr.Get(common.HTTPAuthProxyAlwaysOnboard).GetBool(), + SkipSearch: cfgMgr.Get(common.HTTPAuthProxySkipSearch).GetBool(), }, nil } diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index 89561778d..a518f15d0 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -21,6 +21,7 @@ import ( "testing" "fmt" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" @@ -228,17 +229,17 @@ func TestConfigureValue_GetMap(t *testing.T) { func TestHTTPAuthProxySetting(t *testing.T) { m := map[string]interface{}{ - common.HTTPAuthProxyAlwaysOnboard: "true", - common.HTTPAuthProxyVerifyCert: "true", - common.HTTPAuthProxyEndpoint: "https://auth.proxy/suffix", + common.HTTPAuthProxySkipSearch: "true", + common.HTTPAuthProxyVerifyCert: "true", + common.HTTPAuthProxyEndpoint: "https://auth.proxy/suffix", } InitWithSettings(m) v, e := HTTPAuthProxySetting() assert.Nil(t, e) assert.Equal(t, *v, models.HTTPAuthProxy{ - Endpoint: "https://auth.proxy/suffix", - AlwaysOnBoard: true, - VerifyCert: true, + Endpoint: "https://auth.proxy/suffix", + SkipSearch: true, + VerifyCert: true, }) } diff --git a/src/core/filter/security.go b/src/core/filter/security.go index c4ed6a9f2..34f7310a5 100644 --- a/src/core/filter/security.go +++ b/src/core/filter/security.go @@ -41,15 +41,7 @@ import ( "github.com/goharbor/harbor/src/core/promgr/pmsdriver/admiral" "strings" - "encoding/json" - k8s_api_v1beta1 "k8s.io/api/authentication/v1beta1" - "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/runtime/serializer" + "github.com/goharbor/harbor/src/pkg/authproxy" ) // ContextValueKey for content value @@ -321,60 +313,17 @@ func (ap *authProxyReqCtxModifier) Modify(ctx *beegoctx.Context) bool { log.Errorf("User name %s doesn't meet the auth proxy name pattern", proxyUserName) return false } - httpAuthProxyConf, err := config.HTTPAuthProxySetting() if err != nil { log.Errorf("fail to get auth proxy settings, %v", err) return false } - - // Init auth client with the auth proxy endpoint. - authClientCfg := &rest.Config{ - Host: httpAuthProxyConf.TokenReviewEndpoint, - ContentConfig: rest.ContentConfig{ - GroupVersion: &schema.GroupVersion{}, - NegotiatedSerializer: serializer.DirectCodecFactory{CodecFactory: scheme.Codecs}, - }, - BearerToken: proxyPwd, - TLSClientConfig: rest.TLSClientConfig{ - Insecure: !httpAuthProxyConf.VerifyCert, - }, - } - authClient, err := rest.RESTClientFor(authClientCfg) + tokenReviewResponse, err := authproxy.TokenReview(proxyPwd, httpAuthProxyConf) if err != nil { - log.Errorf("fail to create auth client, %v", err) + log.Errorf("fail to review token, %v", err) return false } - // Do auth with the token. - tokenReviewRequest := &k8s_api_v1beta1.TokenReview{ - TypeMeta: metav1.TypeMeta{ - Kind: "TokenReview", - APIVersion: "authentication.k8s.io/v1beta1", - }, - Spec: k8s_api_v1beta1.TokenReviewSpec{ - Token: proxyPwd, - }, - } - res := authClient.Post().Body(tokenReviewRequest).Do() - err = res.Error() - if err != nil { - log.Errorf("fail to POST auth request, %v", err) - return false - } - resRaw, err := res.Raw() - if err != nil { - log.Errorf("fail to get raw data of token review, %v", err) - return false - } - - // Parse the auth response, check the user name and authenticated status. - tokenReviewResponse := &k8s_api_v1beta1.TokenReview{} - err = json.Unmarshal(resRaw, &tokenReviewResponse) - if err != nil { - log.Errorf("fail to decode token review, %v", err) - return false - } if !tokenReviewResponse.Status.Authenticated { log.Errorf("fail to auth user: %s", rawUserName) return false diff --git a/src/core/filter/security_test.go b/src/core/filter/security_test.go index 17307efab..a74d2fa12 100644 --- a/src/core/filter/security_test.go +++ b/src/core/filter/security_test.go @@ -16,8 +16,6 @@ package filter import ( "context" - "github.com/goharbor/harbor/src/common/utils/oidc" - "github.com/stretchr/testify/require" "log" "net/http" "net/http/httptest" @@ -27,6 +25,9 @@ import ( "testing" "time" + "github.com/goharbor/harbor/src/common/utils/oidc" + "github.com/stretchr/testify/require" + "github.com/astaxie/beego" beegoctx "github.com/astaxie/beego/context" "github.com/astaxie/beego/session" @@ -241,7 +242,7 @@ func TestAuthProxyReqCtxModifier(t *testing.T) { defer server.Close() c := map[string]interface{}{ - common.HTTPAuthProxyAlwaysOnboard: "true", + common.HTTPAuthProxySkipSearch: "true", common.HTTPAuthProxyVerifyCert: "false", common.HTTPAuthProxyEndpoint: "https://auth.proxy/suffix", common.HTTPAuthProxyTokenReviewEndpoint: server.URL, @@ -253,7 +254,7 @@ func TestAuthProxyReqCtxModifier(t *testing.T) { assert.Nil(t, e) assert.Equal(t, *v, models.HTTPAuthProxy{ Endpoint: "https://auth.proxy/suffix", - AlwaysOnBoard: true, + SkipSearch: true, VerifyCert: false, TokenReviewEndpoint: server.URL, }) diff --git a/src/pkg/authproxy/http.go b/src/pkg/authproxy/http.go new file mode 100644 index 000000000..baeed17cf --- /dev/null +++ b/src/pkg/authproxy/http.go @@ -0,0 +1,65 @@ +package authproxy + +import ( + "encoding/json" + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/utils/log" + k8s_api_v1beta1 "k8s.io/api/authentication/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" +) + +// TokenReview ... +func TokenReview(sessionID string, authProxyConfig *models.HTTPAuthProxy) (*k8s_api_v1beta1.TokenReview, error) { + + // Init auth client with the auth proxy endpoint. + authClientCfg := &rest.Config{ + Host: authProxyConfig.TokenReviewEndpoint, + ContentConfig: rest.ContentConfig{ + GroupVersion: &schema.GroupVersion{}, + NegotiatedSerializer: serializer.DirectCodecFactory{CodecFactory: scheme.Codecs}, + }, + BearerToken: sessionID, + TLSClientConfig: rest.TLSClientConfig{ + Insecure: !authProxyConfig.VerifyCert, + }, + } + authClient, err := rest.RESTClientFor(authClientCfg) + if err != nil { + return nil, err + } + + // Do auth with the token. + tokenReviewRequest := &k8s_api_v1beta1.TokenReview{ + TypeMeta: metav1.TypeMeta{ + Kind: "TokenReview", + APIVersion: "authentication.k8s.io/v1beta1", + }, + Spec: k8s_api_v1beta1.TokenReviewSpec{ + Token: sessionID, + }, + } + res := authClient.Post().Body(tokenReviewRequest).Do() + err = res.Error() + if err != nil { + log.Errorf("fail to POST auth request, %v", err) + return nil, err + } + resRaw, err := res.Raw() + if err != nil { + log.Errorf("fail to get raw data of token review, %v", err) + return nil, err + } + // Parse the auth response, check the user name and authenticated status. + tokenReviewResponse := &k8s_api_v1beta1.TokenReview{} + err = json.Unmarshal(resRaw, &tokenReviewResponse) + if err != nil { + log.Errorf("fail to decode token review, %v", err) + return nil, err + } + return tokenReviewResponse, nil + +} diff --git a/src/portal/lib/src/config/config.ts b/src/portal/lib/src/config/config.ts index 2a376d71c..e24aa30dc 100644 --- a/src/portal/lib/src/config/config.ts +++ b/src/portal/lib/src/config/config.ts @@ -90,7 +90,7 @@ export class Configuration { http_authproxy_endpoint?: StringValueItem; http_authproxy_tokenreview_endpoint?: StringValueItem; http_authproxy_verify_cert?: BoolValueItem; - http_authproxy_always_onboard?: BoolValueItem; + http_authproxy_skip_search?: BoolValueItem; oidc_name?: StringValueItem; oidc_endpoint?: StringValueItem; oidc_client_id?: StringValueItem; @@ -141,7 +141,7 @@ export class Configuration { this.http_authproxy_endpoint = new StringValueItem("", true); this.http_authproxy_tokenreview_endpoint = new StringValueItem("", true); this.http_authproxy_verify_cert = new BoolValueItem(false, true); - this.http_authproxy_always_onboard = new BoolValueItem(false, true); + this.http_authproxy_skip_search = new BoolValueItem(false, true); this.oidc_name = new StringValueItem('', true); this.oidc_endpoint = new StringValueItem('', true); this.oidc_client_id = new StringValueItem('', true); diff --git a/src/portal/src/app/config/auth/config-auth.component.html b/src/portal/src/app/config/auth/config-auth.component.html index 0f1a9a9d4..e055e7212 100644 --- a/src/portal/src/app/config/auth/config-auth.component.html +++ b/src/portal/src/app/config/auth/config-auth.component.html @@ -310,13 +310,13 @@
- + - +
diff --git a/src/portal/src/i18n/lang/en-us-lang.json b/src/portal/src/i18n/lang/en-us-lang.json index b881f3037..fb36d7fbe 100644 --- a/src/portal/src/i18n/lang/en-us-lang.json +++ b/src/portal/src/i18n/lang/en-us-lang.json @@ -769,7 +769,7 @@ "HTTP_AUTH": { "ENDPOINT": "Server Endpoint", "TOKEN_REVIEW": "Token Review Endpoint", - "ALWAYS_ONBOARD": "Always Onboard", + "SKIP_SEARCH": "Skip Search", "VERIFY_CERT": "Verify Certificate" }, "OIDC": { diff --git a/src/portal/src/i18n/lang/es-es-lang.json b/src/portal/src/i18n/lang/es-es-lang.json index dec43b75d..cee79573d 100644 --- a/src/portal/src/i18n/lang/es-es-lang.json +++ b/src/portal/src/i18n/lang/es-es-lang.json @@ -769,7 +769,7 @@ "HTTP_AUTH": { "ENDPOINT": "Server Endpoint", "TOKEN_REVIEW": "Review Endpoint De Token", - "ALWAYS_ONBOARD": "Always Onboard", + "SKIP_SEARCH": "Skip Search", "VERIFY_CERT": "Authentication Verify Cert" }, "OIDC": { diff --git a/src/portal/src/i18n/lang/fr-fr-lang.json b/src/portal/src/i18n/lang/fr-fr-lang.json index ff89db84e..a5c4671fc 100644 --- a/src/portal/src/i18n/lang/fr-fr-lang.json +++ b/src/portal/src/i18n/lang/fr-fr-lang.json @@ -743,7 +743,7 @@ "HTTP_AUTH": { "ENDPOINT": "serveur paramètre", "TOKEN_REVIEW": "examen symbolique paramètre", - "ALWAYS_ONBOARD": "always onboard", + "SKIP_SEARCH": "Skip Search", "VERIFY_CERT": "authentification vérifier cert" }, "OIDC": { diff --git a/src/portal/src/i18n/lang/pt-br-lang.json b/src/portal/src/i18n/lang/pt-br-lang.json index f4629c83f..2f2b4868f 100644 --- a/src/portal/src/i18n/lang/pt-br-lang.json +++ b/src/portal/src/i18n/lang/pt-br-lang.json @@ -763,7 +763,7 @@ "HTTP_AUTH": { "ENDPOINT": "Server endpoint", "TOKEN_REVIEW": "Ponto final do Token Review", - "ALWAYS_ONBOARD": "Sempre Onboard", + "SKIP_SEARCH": "Skip Search", "VERIFY_CERT": "Verificar certificado de Authentication" }, "OIDC": { diff --git a/src/portal/src/i18n/lang/zh-cn-lang.json b/src/portal/src/i18n/lang/zh-cn-lang.json index 617dc6b24..6c4f5f7cc 100644 --- a/src/portal/src/i18n/lang/zh-cn-lang.json +++ b/src/portal/src/i18n/lang/zh-cn-lang.json @@ -768,7 +768,7 @@ "HTTP_AUTH": { "ENDPOINT": "Server Endpoint", "TOKEN_REVIEW": "Token Review Endpoint", - "ALWAYS_ONBOARD": "Always Onboard", + "SKIP_SEARCH": "Skip Search", "VERIFY_CERT": "Authentication验证证书" }, "OIDC": {