From cc69b1e9512b162fd2c6ba267c0f8c6fb5de406f Mon Sep 17 00:00:00 2001 From: "stonezdj(Daojun Zhang)" Date: Fri, 11 Nov 2022 09:55:11 +0800 Subject: [PATCH] Add OIDC group filter (#17736) Filter out the OIDC group which doesn't match the regular expression Fixes #17130 Signed-off-by: stonezdj Signed-off-by: stonezdj --- api/v2.0/swagger.yaml | 8 +++++++ src/common/const.go | 1 + src/lib/config/metadata/metadatalist.go | 1 + src/lib/config/models/model.go | 1 + src/lib/config/userconfig.go | 1 + src/pkg/oidc/helper.go | 29 ++++++++++++++++++++++++- src/pkg/oidc/helper_test.go | 22 +++++++++++++++++++ 7 files changed, 62 insertions(+), 1 deletion(-) diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml index c92f9f854..77720bed9 100644 --- a/api/v2.0/swagger.yaml +++ b/api/v2.0/swagger.yaml @@ -8600,6 +8600,9 @@ definitions: oidc_admin_group: $ref: '#/definitions/StringConfigItem' description: The OIDC group which has the harbor admin privileges + oidc_group_filter: + $ref: '#/definitions/StringConfigItem' + description: The OIDC group filter which filters out the group doesn't match the regular expression oidc_scope: $ref: '#/definitions/StringConfigItem' description: The scope of the OIDC provider @@ -8877,6 +8880,11 @@ definitions: description: The OIDC group which has the harbor admin privileges x-omitempty: true x-isnullable: true + oidc_group_filter: + type: string + description: The OIDC group filter which filters out the group name doesn't match the regular expression + x-omitempty: true + x-isnullable: true oidc_scope: type: string description: The scope of the OIDC provider diff --git a/src/common/const.go b/src/common/const.go index a99d8ddae..a2e214f62 100755 --- a/src/common/const.go +++ b/src/common/const.go @@ -111,6 +111,7 @@ const ( OIDCVerifyCert = "oidc_verify_cert" OIDCAdminGroup = "oidc_admin_group" OIDCGroupsClaim = "oidc_groups_claim" + OIDCGroupFilter = "oidc_group_filter" OIDCAutoOnboard = "oidc_auto_onboard" OIDCExtraRedirectParms = "oidc_extra_redirect_parms" OIDCScope = "oidc_scope" diff --git a/src/lib/config/metadata/metadatalist.go b/src/lib/config/metadata/metadatalist.go index 1c2e7c340..006ac796a 100644 --- a/src/lib/config/metadata/metadatalist.go +++ b/src/lib/config/metadata/metadatalist.go @@ -144,6 +144,7 @@ var ( {Name: common.OIDCClientSecret, Scope: UserScope, Group: OIDCGroup, ItemType: &PasswordType{}, Description: `The OIDC provider secret`}, {Name: common.OIDCGroupsClaim, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}, Description: `The attribute claims the group name`}, {Name: common.OIDCAdminGroup, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}, Description: `The OIDC group which has the harbor admin privileges`}, + {Name: common.OIDCGroupFilter, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}, Description: `The OIDC group filter to filter which groups could be onboarded to Harbor`}, {Name: common.OIDCScope, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}, Description: `The scope of the OIDC provider`}, {Name: common.OIDCUserClaim, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}, Description: `The attribute claims the username`}, {Name: common.OIDCVerifyCert, Scope: UserScope, Group: OIDCGroup, DefaultValue: "true", ItemType: &BoolType{}, Description: `Verify the OIDC provider's certificate'`}, diff --git a/src/lib/config/models/model.go b/src/lib/config/models/model.go index 187219f7b..eb2153eb7 100644 --- a/src/lib/config/models/model.go +++ b/src/lib/config/models/model.go @@ -51,6 +51,7 @@ type OIDCSetting struct { ClientSecret string `json:"client_secret"` GroupsClaim string `json:"groups_claim"` AdminGroup string `json:"admin_group"` + GroupFilter string `json:"group_filter"` RedirectURL string `json:"redirect_url"` Scope []string `json:"scope"` UserClaim string `json:"user_claim"` diff --git a/src/lib/config/userconfig.go b/src/lib/config/userconfig.go index 55086c975..485d3001e 100644 --- a/src/lib/config/userconfig.go +++ b/src/lib/config/userconfig.go @@ -184,6 +184,7 @@ func OIDCSetting(ctx context.Context) (*cfgModels.OIDCSetting, error) { ClientID: mgr.Get(ctx, common.OIDCCLientID).GetString(), ClientSecret: mgr.Get(ctx, common.OIDCClientSecret).GetString(), GroupsClaim: mgr.Get(ctx, common.OIDCGroupsClaim).GetString(), + GroupFilter: mgr.Get(ctx, common.OIDCGroupFilter).GetString(), AdminGroup: mgr.Get(ctx, common.OIDCAdminGroup).GetString(), RedirectURL: extEndpoint + common.OIDCCallbackPath, Scope: scope, diff --git a/src/pkg/oidc/helper.go b/src/pkg/oidc/helper.go index 96231e902..ec828f97f 100644 --- a/src/pkg/oidc/helper.go +++ b/src/pkg/oidc/helper.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "net/http" + "regexp" "strings" "sync" "sync/atomic" @@ -405,7 +406,33 @@ func groupsFromClaims(gp claimsProvider, k string) ([]string, bool) { type populate func(groupNames []string) ([]int, error) func populateGroupsDB(groupNames []string) ([]int, error) { - return usergroup.Mgr.Populate(orm.Context(), model.UserGroupsFromName(groupNames, common.OIDCGroupType)) + ctx := orm.Context() + cfg, err := config.OIDCSetting(ctx) + if err != nil { + return nil, err + } + log.Debugf("populateGroupsDB, group filter %v", cfg.GroupFilter) + return usergroup.Mgr.Populate(orm.Context(), model.UserGroupsFromName(filterGroup(groupNames, cfg.GroupFilter), common.OIDCGroupType)) +} + +// filterGroup filter group with a regular expression filter +func filterGroup(groupNames []string, filter string) []string { + if len(filter) == 0 { + return groupNames + } + pattern, err := regexp.Compile(filter) + if err != nil { + log.Errorf("failed to filter group, invalid filter %v", filter) + return groupNames + } + result := make([]string, 0) + for _, name := range groupNames { + if pattern.MatchString(name) { + result = append(result, name) + } + } + log.Debugf("filter is %v, result is %v", filter, result) + return result } // InjectGroupsToUser populates the group to DB and inject the group IDs to user model. diff --git a/src/pkg/oidc/helper_test.go b/src/pkg/oidc/helper_test.go index 7c65cd8e2..a71173077 100644 --- a/src/pkg/oidc/helper_test.go +++ b/src/pkg/oidc/helper_test.go @@ -537,3 +537,25 @@ func TestInjectGroupsToUser(t *testing.T) { assert.Equal(t, *c.new, *u) } } + +func Test_filterGroup(t *testing.T) { + type args struct { + groupNames []string + filter string + } + tests := []struct { + name string + args args + want []string + }{ + {"normal", args{[]string{"admin_user"}, "^admin.*"}, []string{"admin_user"}}, + {"multiple ", args{[]string{"admin_user", "harbor_admin"}, "^admin.*"}, []string{"admin_user"}}, + {"no match", args{[]string{"harbor_admin", "harbor_user", "sample_admin", "myadmin"}, "^admin.*"}, []string{}}, + {"empty filter", args{[]string{"harbor_admin", "harbor_user", "sample_admin", "myadmin"}, ""}, []string{"harbor_admin", "harbor_user", "sample_admin", "myadmin"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, filterGroup(tt.args.groupNames, tt.args.filter), "filterGroup(%v, %v)", tt.args.groupNames, tt.args.filter) + }) + } +}