From b21f9dc6f10dec60597d7a279c1d5f4b999dcba7 Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Tue, 17 Sep 2019 09:52:34 +0800 Subject: [PATCH] Support OIDC groups This commit enable project admin to add group as project member when Harbor is configured against OIDC as AuthN backend. It populates the information of groups from ID Token based on the claim that is set in OIDC settings. Signed-off-by: Daniel Jiang --- src/common/const.go | 1 + src/common/utils/oidc/helper.go | 39 ++++++++++++++++++++++ src/common/utils/oidc/helper_test.go | 48 ++++++++++++++++++++++++++++ src/core/api/projectmember.go | 4 +-- src/core/api/projectmember_test.go | 4 +-- src/core/auth/authenticator.go | 6 ++-- src/core/auth/db/db.go | 3 +- src/core/auth/ldap/ldap.go | 2 +- src/core/auth/oidc/oidc.go | 35 ++++++++++++++++++++ src/core/auth/oidc/oidc_test.go | 31 ++++++++++++++++++ src/core/controllers/oidc.go | 13 ++++++-- src/core/filter/security.go | 7 +++- src/core/main.go | 1 + 13 files changed, 181 insertions(+), 13 deletions(-) create mode 100644 src/core/auth/oidc/oidc.go create mode 100644 src/core/auth/oidc/oidc_test.go diff --git a/src/common/const.go b/src/common/const.go index bec1d261c..45c2e2692 100755 --- a/src/common/const.go +++ b/src/common/const.go @@ -126,6 +126,7 @@ const ( DefaultNotaryEndpoint = "http://notary-server:4443" LDAPGroupType = 1 HTTPGroupType = 2 + OIDCGroupType = 3 LDAPGroupAdminDn = "ldap_group_admin_dn" LDAPGroupMembershipAttribute = "ldap_group_membership_attribute" DefaultRegistryControllerEndpoint = "http://registryctl:8080" diff --git a/src/common/utils/oidc/helper.go b/src/common/utils/oidc/helper.go index 30f14e209..2d59e09ed 100644 --- a/src/common/utils/oidc/helper.go +++ b/src/common/utils/oidc/helper.go @@ -207,6 +207,45 @@ func RefreshToken(ctx context.Context, token *Token) (*Token, error) { return &Token{Token: *t, IDToken: it}, nil } +// GroupsFromToken returns the list of group name in the token, the claim of the group list is set in OIDCSetting. +// It's designed not to return errors, in case of unexpected situation it will log and return empty list. +func GroupsFromToken(token *gooidc.IDToken) []string { + if token == nil { + log.Warning("Return empty list for nil token") + return []string{} + } + setting := provider.setting.Load().(models.OIDCSetting) + if len(setting.GroupsClaim) == 0 { + log.Warning("Group claim is not set in OIDC setting returning empty group list.") + return []string{} + } + var c map[string]interface{} + err := token.Claims(&c) + if err != nil { + log.Warningf("Failed to get claims map, error: %v", err) + return []string{} + } + return groupsFromClaim(c, setting.GroupsClaim) +} + +func groupsFromClaim(claimMap map[string]interface{}, k string) []string { + var res []string + g, ok := claimMap[k].([]interface{}) + if !ok { + log.Warningf("Unable to get groups from claims, claims: %+v, groups claim key: %s", claimMap, k) + return res + } + for _, e := range g { + s, ok := e.(string) + if !ok { + log.Warningf("Element in group list is not string: %v, list: %v", e, g) + continue + } + res = append(res, s) + } + return res +} + // Conn wraps connection info of an OIDC endpoint type Conn struct { URL string `json:"url"` diff --git a/src/common/utils/oidc/helper_test.go b/src/common/utils/oidc/helper_test.go index d706836b8..8270b44f7 100644 --- a/src/common/utils/oidc/helper_test.go +++ b/src/common/utils/oidc/helper_test.go @@ -15,6 +15,7 @@ package oidc import ( + gooidc "github.com/coreos/go-oidc" "github.com/goharbor/harbor/src/common" config2 "github.com/goharbor/harbor/src/common/config" "github.com/goharbor/harbor/src/common/models" @@ -110,3 +111,50 @@ func TestTestEndpoint(t *testing.T) { assert.Nil(t, TestEndpoint(c1)) assert.NotNil(t, TestEndpoint(c2)) } + +func TestGroupsFromToken(t *testing.T) { + res := GroupsFromToken(nil) + assert.Equal(t, []string{}, res) + res = GroupsFromToken(&gooidc.IDToken{}) + assert.Equal(t, []string{}, res) +} + +func TestGroupsFromClaim(t *testing.T) { + in := map[string]interface{}{ + "user": "user1", + "groups": []interface{}{"group1", "group2"}, + "groups_2": []interface{}{"group1", "group2", 2}, + } + + m := []struct { + input map[string]interface{} + key string + expect []string + }{ + { + in, + "user", + nil, + }, + { + in, + "prg", + nil, + }, + { + in, + "groups", + []string{"group1", "group2"}, + }, + { + in, + "groups_2", + []string{"group1", "group2"}, + }, + } + + for _, tc := range m { + r := groupsFromClaim(tc.input, tc.key) + assert.Equal(t, tc.expect, r) + } +} diff --git a/src/core/api/projectmember.go b/src/core/api/projectmember.go index c836016f7..b704ad5c6 100644 --- a/src/core/api/projectmember.go +++ b/src/core/api/projectmember.go @@ -251,8 +251,8 @@ func AddProjectMember(projectID int64, request models.MemberReq) (int, error) { return 0, err } member.EntityID = groupID - } 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}) + } else if len(request.MemberGroup.GroupName) > 0 && request.MemberGroup.GroupType == common.HTTPGroupType || request.MemberGroup.GroupType == common.OIDCGroupType { + ugs, err := group.QueryUserGroup(models.UserGroup{GroupName: request.MemberGroup.GroupName, GroupType: request.MemberGroup.GroupType}) if err != nil { return 0, err } diff --git a/src/core/api/projectmember_test.go b/src/core/api/projectmember_test.go index 88e47851f..89aab304b 100644 --- a/src/core/api/projectmember_test.go +++ b/src/core/api/projectmember_test.go @@ -196,7 +196,7 @@ func TestProjectMemberAPI_Post(t *testing.T) { }, }, }, - code: http.StatusBadRequest, + code: http.StatusInternalServerError, }, { request: &testingRequest{ @@ -241,7 +241,7 @@ func TestProjectMemberAPI_Post(t *testing.T) { }, }, }, - code: http.StatusBadRequest, + code: http.StatusInternalServerError, }, } runCodeCheckingCases(t, cases...) diff --git a/src/core/auth/authenticator.go b/src/core/auth/authenticator.go index 48641b37b..46788ead4 100644 --- a/src/core/auth/authenticator.go +++ b/src/core/auth/authenticator.go @@ -230,12 +230,12 @@ func SearchAndOnBoardUser(username string) (int, error) { // SearchAndOnBoardGroup ... if altGroupName is not empty, take the altGroupName as groupName in harbor DB func SearchAndOnBoardGroup(groupKey, altGroupName string) (int, error) { userGroup, err := SearchGroup(groupKey) - if userGroup == nil { - return 0, ErrorGroupNotExist - } if err != nil { return 0, err } + if userGroup == nil { + return 0, ErrorGroupNotExist + } if userGroup != nil { err = OnBoardGroup(userGroup, altGroupName) } diff --git a/src/core/auth/db/db.go b/src/core/auth/db/db.go index dd9bdf1e5..405bf0d8a 100644 --- a/src/core/auth/db/db.go +++ b/src/core/auth/db/db.go @@ -15,6 +15,7 @@ package db import ( + "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/core/auth" @@ -52,5 +53,5 @@ func (d *Auth) OnBoardUser(u *models.User) error { } func init() { - auth.Register("db_auth", &Auth{}) + auth.Register(common.DBAuth, &Auth{}) } diff --git a/src/core/auth/ldap/ldap.go b/src/core/auth/ldap/ldap.go index c5fd86d29..904aafc66 100644 --- a/src/core/auth/ldap/ldap.go +++ b/src/core/auth/ldap/ldap.go @@ -265,5 +265,5 @@ func (l *Auth) PostAuthenticate(u *models.User) error { } func init() { - auth.Register("ldap_auth", &Auth{}) + auth.Register(common.LDAPAuth, &Auth{}) } diff --git a/src/core/auth/oidc/oidc.go b/src/core/auth/oidc/oidc.go new file mode 100644 index 000000000..6f7398845 --- /dev/null +++ b/src/core/auth/oidc/oidc.go @@ -0,0 +1,35 @@ +package oidc + +import ( + "fmt" + "github.com/goharbor/harbor/src/common" + "github.com/goharbor/harbor/src/common/dao/group" + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/core/auth" +) + +// Auth of OIDC mode only implements the funcs for onboarding group +type Auth struct { + auth.DefaultAuthenticateHelper +} + +// SearchGroup is skipped in OIDC mode, so it makes sure any group will be onboarded. +func (a *Auth) SearchGroup(groupKey string) (*models.UserGroup, error) { + return &models.UserGroup{ + GroupName: groupKey, + GroupType: common.OIDCGroupType, + }, 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 + if len(u.GroupName) == 0 || u.GroupType != common.OIDCGroupType { + return fmt.Errorf("invalid input group for OIDC mode: %v", *u) + } + return group.OnBoardUserGroup(u) +} + +func init() { + auth.Register(common.OIDCAuth, &Auth{}) +} diff --git a/src/core/auth/oidc/oidc_test.go b/src/core/auth/oidc/oidc_test.go new file mode 100644 index 000000000..cb2a13fc4 --- /dev/null +++ b/src/core/auth/oidc/oidc_test.go @@ -0,0 +1,31 @@ +package oidc + +import ( + "github.com/goharbor/harbor/src/common" + "github.com/goharbor/harbor/src/common/models" + "github.com/stretchr/testify/assert" + "os" + "testing" +) + +func TestMain(m *testing.M) { + retCode := m.Run() + os.Exit(retCode) +} + +func TestAuth_SearchGroup(t *testing.T) { + a := Auth{} + res, err := a.SearchGroup("grp") + assert.Nil(t, err) + assert.Equal(t, models.UserGroup{GroupName: "grp", GroupType: common.OIDCGroupType}, *res) +} + +func TestAuth_OnBoardGroup(t *testing.T) { + a := Auth{} + g1 := &models.UserGroup{GroupName: "", GroupType: common.OIDCGroupType} + err1 := a.OnBoardGroup(g1, "") + assert.NotNil(t, err1) + g2 := &models.UserGroup{GroupName: "group", GroupType: common.LDAPGroupType} + err2 := a.OnBoardGroup(g2, "") + assert.NotNil(t, err2) +} diff --git a/src/core/controllers/oidc.go b/src/core/controllers/oidc.go index 3bc5d1e35..c9241235c 100644 --- a/src/core/controllers/oidc.go +++ b/src/core/controllers/oidc.go @@ -17,6 +17,7 @@ package controllers import ( "encoding/json" "fmt" + "github.com/goharbor/harbor/src/common/dao/group" "net/http" "strings" @@ -50,12 +51,13 @@ type oidcUserData struct { Subject string `json:"sub"` Username string `json:"name"` Email string `json:"email"` + GroupIDs []int `json:"group_ids"` } // Prepare include public code path for call request handler of OIDCController func (oc *OIDCController) Prepare() { if mode, _ := config.AuthMode(); mode != common.OIDCAuth { - oc.SendPreconditionFailedError(fmt.Errorf("Auth Mode: %s is not OIDC based", mode)) + oc.SendPreconditionFailedError(fmt.Errorf("auth mode: %s is not OIDC based", mode)) return } } @@ -114,6 +116,10 @@ func (oc *OIDCController) Callback() { oc.SendInternalServerError(err) return } + d.GroupIDs, err = group.GetGroupIDByGroupName(oidc.GroupsFromToken(idToken), common.OIDCGroupType) + if err != nil { + log.Warningf("Failed to get group ID list, due to error: %v, setting empty list into user model.", err) + } ouDataStr, err := json.Marshal(d) if err != nil { oc.SendInternalServerError(err) @@ -137,6 +143,7 @@ func (oc *OIDCController) Callback() { oc.Controller.Redirect(fmt.Sprintf("/oidc-onboard?username=%s", strings.Replace(d.Username, " ", "_", -1)), http.StatusFound) } else { + u.GroupIDs = d.GroupIDs oidcUser, err := dao.GetOIDCUserByUserID(u.UserID) if err != nil { oc.SendInternalServerError(err) @@ -175,6 +182,7 @@ func (oc *OIDCController) Onboard() { oc.SendBadRequestError(errors.New("Failed to get OIDC user info from session")) return } + defer oc.DelSession(userInfoKey) log.Debugf("User info string: %s\n", userInfoStr) tb, ok := oc.GetSession(tokenKey).([]byte) if !ok { @@ -203,6 +211,7 @@ func (oc *OIDCController) Onboard() { Username: username, Realname: d.Username, Email: email, + GroupIDs: d.GroupIDs, OIDCUserMeta: &oidcUser, Comment: oidcUserComment, } @@ -214,13 +223,11 @@ func (oc *OIDCController) Onboard() { return } oc.SendInternalServerError(err) - oc.DelSession(userInfoKey) return } user.OIDCUserMeta = nil oc.SetSession(userKey, user) - oc.DelSession(userInfoKey) } func secretAndToken(tokenBytes []byte) (string, string, error) { diff --git a/src/core/filter/security.go b/src/core/filter/security.go index 34f7310a5..785fbba72 100644 --- a/src/core/filter/security.go +++ b/src/core/filter/security.go @@ -25,6 +25,7 @@ import ( "github.com/docker/distribution/reference" "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" secstore "github.com/goharbor/harbor/src/common/secret" "github.com/goharbor/harbor/src/common/security" @@ -284,6 +285,10 @@ func (it *idTokenReqCtxModifier) Modify(ctx *beegoctx.Context) bool { log.Warning("User matches token's claims is not onboarded.") return false } + u.GroupIDs, err = group.GetGroupIDByGroupName(oidc.GroupsFromToken(claims), common.OIDCGroupType) + if err != nil { + log.Errorf("Failed to get group ID list for OIDC user: %s, error: %v", u.Username, err) + } pm := config.GlobalProjectMgr sc := local.NewSecurityContext(u, pm) setSecurCtxAndPM(ctx.Request, sc, pm) @@ -457,7 +462,7 @@ func (s *sessionReqCtxModifier) Modify(ctx *beegoctx.Context) bool { if ou != nil { // If user does not have OIDC metadata, it means he is not onboarded via OIDC authn, // so we can skip checking the token. if err := oidc.VerifyAndPersistToken(ctx.Request.Context(), ou); err != nil { - log.Errorf("Failed to verify secret, error: %v", err) + log.Errorf("Failed to verify token, error: %v", err) return false } } diff --git a/src/core/main.go b/src/core/main.go index 837d4be28..ceaece635 100755 --- a/src/core/main.go +++ b/src/core/main.go @@ -35,6 +35,7 @@ import ( _ "github.com/goharbor/harbor/src/core/auth/authproxy" _ "github.com/goharbor/harbor/src/core/auth/db" _ "github.com/goharbor/harbor/src/core/auth/ldap" + _ "github.com/goharbor/harbor/src/core/auth/oidc" _ "github.com/goharbor/harbor/src/core/auth/uaa" quota "github.com/goharbor/harbor/src/core/api/quota"