Merge pull request #9142 from reasonerjt/oidc-groups

Support OIDC groups

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"

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"`

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)
+	}
+}

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 {
+	} 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: common.HTTPGroupType})
+		ugs, err := group.QueryUserGroup(models.UserGroup{GroupName: request.MemberGroup.GroupName, GroupType: request.MemberGroup.GroupType})
 		if err != nil {
 			return 0, err
 		}

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...)

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)
 	}

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{})
 }

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{})
 }

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{})
+}

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)
+}

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)
@@ -203,6 +210,7 @@ func (oc *OIDCController) Onboard() {
 		Username:     username,
 		Realname:     d.Username,
 		Email:        email,
+		GroupIDs:     d.GroupIDs,
 		OIDCUserMeta: &oidcUser,
 		Comment:      oidcUserComment,
 	}

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
 			}
 		}

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"