Merge pull request #9311 from airadier/autoonboard-and-custom-user-claim

Add options for automatic onboarding and username claim

src/common/config/metadata/metadatalist.go

@@ -140,7 +140,9 @@ var (
 		{Name: common.OIDCClientSecret, Scope: UserScope, Group: OIDCGroup, ItemType: &PasswordType{}},
 		{Name: common.OIDCGroupsClaim, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}},
 		{Name: common.OIDCScope, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}},
+		{Name: common.OIDCUserClaim, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}},
 		{Name: common.OIDCVerifyCert, Scope: UserScope, Group: OIDCGroup, DefaultValue: "true", ItemType: &BoolType{}},
+		{Name: common.OIDCAutoOnboard, Scope: UserScope, Group: OIDCGroup, DefaultValue: "false", ItemType: &BoolType{}},
 
 		{Name: common.WithChartMuseum, Scope: SystemScope, Group: BasicGroup, EnvKey: "WITH_CHARTMUSEUM", DefaultValue: "false", ItemType: &BoolType{}, Editable: true},
 		{Name: common.WithClair, Scope: SystemScope, Group: BasicGroup, EnvKey: "WITH_CLAIR", DefaultValue: "false", ItemType: &BoolType{}, Editable: true},

src/common/const.go

@@ -106,7 +106,9 @@ const (
 	OIDCClientSecret                 = "oidc_client_secret"
 	OIDCVerifyCert                   = "oidc_verify_cert"
 	OIDCGroupsClaim                  = "oidc_groups_claim"
+	OIDCAutoOnboard                  = "oidc_auto_onboard"
 	OIDCScope                        = "oidc_scope"
+	OIDCUserClaim                    = "oidc_user_claim"
 
 	CfgDriverDB                       = "db"
 	NewHarborAdminName                = "admin@harbor.local"

src/common/models/config.go

@@ -81,11 +81,13 @@ type OIDCSetting struct {
 	Name         string   `json:"name"`
 	Endpoint     string   `json:"endpoint"`
 	VerifyCert   bool     `json:"verify_cert"`
+	AutoOnboard  bool     `json:"auto_onboard"`
 	ClientID     string   `json:"client_id"`
 	ClientSecret string   `json:"client_secret"`
 	GroupsClaim  string   `json:"groups_claim"`
 	RedirectURL  string   `json:"redirect_url"`
 	Scope        []string `json:"scope"`
+	UserClaim    string   `json:"user_claim"`
 }
 
 // QuotaSetting wraps the settings for Quota

src/common/utils/oidc/helper.go

@@ -19,16 +19,17 @@ import (
 	"crypto/tls"
 	"errors"
 	"fmt"
-	gooidc "github.com/coreos/go-oidc"
-	"github.com/goharbor/harbor/src/common/models"
-	"github.com/goharbor/harbor/src/core/config"
-	"github.com/goharbor/harbor/src/lib/log"
-	"golang.org/x/oauth2"
 	"net/http"
 	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
+
+	gooidc "github.com/coreos/go-oidc"
+	"github.com/goharbor/harbor/src/common/models"
+	"github.com/goharbor/harbor/src/core/config"
+	"github.com/goharbor/harbor/src/lib/log"
+	"golang.org/x/oauth2"
 )
 
 const (
@@ -294,7 +295,7 @@ func userInfoFromRemote(ctx context.Context, token *Token, setting models.OIDCSe
 	if err != nil {
 		return nil, err
 	}
-	return userInfoFromClaims(u, setting.GroupsClaim)
+	return userInfoFromClaims(u, setting.GroupsClaim, setting.UserClaim)
 }
 
 func userInfoFromIDToken(ctx context.Context, token *Token, setting models.OIDCSetting) (*UserInfo, error) {
@@ -305,14 +306,28 @@ func userInfoFromIDToken(ctx context.Context, token *Token, setting models.OIDCS
 	if err != nil {
 		return nil, err
 	}
-	return userInfoFromClaims(idt, setting.GroupsClaim)
+
+	return userInfoFromClaims(idt, setting.GroupsClaim, setting.UserClaim)
 }
 
-func userInfoFromClaims(c claimsProvider, g string) (*UserInfo, error) {
+func userInfoFromClaims(c claimsProvider, g, u string) (*UserInfo, error) {
 	res := &UserInfo{}
 	if err := c.Claims(res); err != nil {
 		return nil, err
 	}
+	if u != "" {
+		allClaims := make(map[string]interface{})
+		if err := c.Claims(&allClaims); err != nil {
+			return nil, err
+		}
+
+		username, ok := allClaims[u].(string)
+		if !ok {
+			return nil, fmt.Errorf("OIDC. Failed to recover Username from claim. Claim '%s' is invalid or not a string", u)
+		}
+		res.Username = username
+
+	}
 	res.Groups, res.hasGroupClaim = GroupsFromClaims(c, g)
 	return res, nil
 }

src/common/utils/oidc/helper_test.go

@@ -175,6 +175,7 @@ func TestUserInfoFromClaims(t *testing.T) {
 	s := []struct {
 		input      map[string]interface{}
 		groupClaim string
+		userClaim  string
 		expect     *UserInfo
 	}{
 		{
@@ -184,6 +185,7 @@ func TestUserInfoFromClaims(t *testing.T) {
 				"groups": []interface{}{"g1", "g2"},
 			},
 			groupClaim: "grouplist",
+			userClaim:  "",
 			expect: &UserInfo{
 				Issuer:        "",
 				Subject:       "",
@@ -200,6 +202,7 @@ func TestUserInfoFromClaims(t *testing.T) {
 				"groups": []interface{}{"g1", "g2"},
 			},
 			groupClaim: "groups",
+			userClaim:  "",
 			expect: &UserInfo{
 				Issuer:        "",
 				Subject:       "",
@@ -218,6 +221,7 @@ func TestUserInfoFromClaims(t *testing.T) {
 				"groupclaim": []interface{}{},
 			},
 			groupClaim: "groupclaim",
+			userClaim:  "",
 			expect: &UserInfo{
 				Issuer:        "issuer",
 				Subject:       "subject000",
@@ -227,9 +231,26 @@ func TestUserInfoFromClaims(t *testing.T) {
 				hasGroupClaim: true,
 			},
 		},
+		{
+			input: map[string]interface{}{
+				"name":   "Alvaro",
+				"email":  "airadier@gmail.com",
+				"groups": []interface{}{"g1", "g2"},
+			},
+			groupClaim: "grouplist",
+			userClaim:  "email",
+			expect: &UserInfo{
+				Issuer:        "",
+				Subject:       "",
+				Username:      "airadier@gmail.com",
+				Email:         "airadier@gmail.com",
+				Groups:        []string{},
+				hasGroupClaim: false,
+			},
+		},
 	}
 	for _, tc := range s {
-		out, err := userInfoFromClaims(&fakeClaims{tc.input}, tc.groupClaim)
+		out, err := userInfoFromClaims(&fakeClaims{tc.input}, tc.groupClaim, tc.userClaim)
 		assert.Nil(t, err)
 		assert.Equal(t, *tc.expect, *out)
 	}

src/core/config/config.go

@@ -440,11 +440,13 @@ func OIDCSetting() (*models.OIDCSetting, error) {
 		Name:         cfgMgr.Get(common.OIDCName).GetString(),
 		Endpoint:     cfgMgr.Get(common.OIDCEndpoint).GetString(),
 		VerifyCert:   cfgMgr.Get(common.OIDCVerifyCert).GetBool(),
+		AutoOnboard:  cfgMgr.Get(common.OIDCAutoOnboard).GetBool(),
 		ClientID:     cfgMgr.Get(common.OIDCCLientID).GetString(),
 		ClientSecret: cfgMgr.Get(common.OIDCClientSecret).GetString(),
 		GroupsClaim:  cfgMgr.Get(common.OIDCGroupsClaim).GetString(),
 		RedirectURL:  extEndpoint + common.OIDCCallbackPath,
 		Scope:        scope,
+		UserClaim:    cfgMgr.Get(common.OIDCUserClaim).GetString(),
 	}, nil
 }
 

src/core/config/config_test.go

@@ -253,8 +253,10 @@ func TestOIDCSetting(t *testing.T) {
 		common.OIDCName:         "test",
 		common.OIDCEndpoint:     "https://oidc.test",
 		common.OIDCVerifyCert:   "true",
+		common.OIDCAutoOnboard:  "false",
 		common.OIDCScope:        "openid, profile",
 		common.OIDCGroupsClaim:  "my_group",
+		common.OIDCUserClaim:    "username",
 		common.OIDCCLientID:     "client",
 		common.OIDCClientSecret: "secret",
 		common.ExtEndpoint:      "https://harbor.test",
@@ -266,8 +268,10 @@ func TestOIDCSetting(t *testing.T) {
 	assert.Equal(t, "https://oidc.test", v.Endpoint)
 	assert.True(t, v.VerifyCert)
 	assert.Equal(t, "my_group", v.GroupsClaim)
+	assert.False(t, v.AutoOnboard)
 	assert.Equal(t, "client", v.ClientID)
 	assert.Equal(t, "secret", v.ClientSecret)
 	assert.Equal(t, "https://harbor.test/c/oidc/callback", v.RedirectURL)
 	assert.ElementsMatch(t, []string{"openid", "profile"}, v.Scope)
+	assert.Equal(t, "username", v.UserClaim)
 }

src/core/controllers/oidc.go

@@ -17,10 +17,11 @@ package controllers
 import (
 	"encoding/json"
 	"fmt"
-	"github.com/goharbor/harbor/src/common/dao/group"
 	"net/http"
 	"strings"
 
+	"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"
@@ -122,11 +123,37 @@ func (oc *OIDCController) Callback() {
 	}
 	oc.SetSession(tokenKey, tokenBytes)
 
+	oidcSettings, err := config.OIDCSetting()
+	if err != nil {
+		oc.SendInternalServerError(err)
+		return
+	}
+
 	if u == nil {
-		oc.SetSession(userInfoKey, string(ouDataStr))
+		// Recover the username from d.Username by default
-		oc.Controller.Redirect(fmt.Sprintf("/oidc-onboard?username=%s", strings.Replace(info.Username, " ", "_", -1)),
+		username := info.Username
-			http.StatusFound)
+
+		// Fix blanks in username
+		username = strings.Replace(username, " ", "_", -1)
+
+		// If automatic onboard is enabled, skip the onboard page
+		if oidcSettings.AutoOnboard {
+			log.Debug("Doing automatic onboarding\n")
+			user, onboarded := userOnboard(oc, info, username, tokenBytes)
+			if onboarded == false {
+				log.Error("User not onboarded\n")
+				return
+			}
+			log.Debug("User automatically onboarded\n")
+			u = user
 		} else {
+			oc.SetSession(userInfoKey, string(ouDataStr))
+			oc.Controller.Redirect(fmt.Sprintf("/oidc-onboard?username=%s", username), http.StatusFound)
+			// Once redirected, no further actions are done
+			return
+		}
+	}
+
 	gids, err := group.PopulateGroup(models.UserGroupsFromName(info.Groups, common.OIDCGroupType))
 	if err != nil {
 		log.Warningf("Failed to populate groups, error: %v, user will have empty group list, username: %s", err, info.Username)
@@ -145,7 +172,50 @@ func (oc *OIDCController) Callback() {
 	}
 	oc.PopulateUserSession(*u)
 	oc.Controller.Redirect("/", http.StatusFound)
+
+}
+
+func userOnboard(oc *OIDCController, info *oidc.UserInfo, username string, tokenBytes []byte) (*models.User, bool) {
+	s, t, err := secretAndToken(tokenBytes)
+	if err != nil {
+		oc.SendInternalServerError(err)
+		return nil, false
 	}
+
+	gids, err := group.PopulateGroup(models.UserGroupsFromName(info.Groups, common.OIDCGroupType))
+	if err != nil {
+		log.Warningf("Failed to populate group user will have empty group list. username: %s", username)
+	}
+
+	oidcUser := models.OIDCUser{
+		SubIss: info.Subject + info.Issuer,
+		Secret: s,
+		Token:  t,
+	}
+
+	user := models.User{
+		Username:     username,
+		Realname:     username,
+		Email:        info.Email,
+		GroupIDs:     gids,
+		OIDCUserMeta: &oidcUser,
+		Comment:      oidcUserComment,
+	}
+
+	log.Debugf("User created: %+v\n", user)
+
+	err = dao.OnBoardOIDCUser(&user)
+	if err != nil {
+		if strings.Contains(err.Error(), dao.ErrDupUser.Error()) {
+			oc.RenderError(http.StatusConflict, "Conflict, the user with same username or email has been onboarded.")
+			return nil, false
+		}
+
+		oc.SendInternalServerError(err)
+		return nil, false
+	}
+
+	return &user, true
 }
 
 // Onboard handles the request to onboard a user authenticated via OIDC provider
@@ -176,51 +246,20 @@ func (oc *OIDCController) Onboard() {
 		oc.SendBadRequestError(errors.New("Failed to get OIDC token from session"))
 		return
 	}
-	s, t, err := secretAndToken(tb)
+
-	if err != nil {
-		oc.SendInternalServerError(err)
-		return
-	}
 	d := &oidc.UserInfo{}
-	err = json.Unmarshal([]byte(userInfoStr), &d)
+	err := json.Unmarshal([]byte(userInfoStr), &d)
 	if err != nil {
 		oc.SendInternalServerError(err)
 		return
 	}
-	gids, err := group.PopulateGroup(models.UserGroupsFromName(d.Groups, common.OIDCGroupType))
-	if err != nil {
-		log.Warningf("Failed to populate group user will have empty group list. username: %s", username)
-	}
-	oidcUser := models.OIDCUser{
-		SubIss: d.Subject + d.Issuer,
-		Secret: s,
-		Token:  t,
-	}
-
-	email := d.Email
-	user := models.User{
-		Username:     username,
-		Realname:     d.Username,
-		Email:        email,
-		GroupIDs:     gids,
-		OIDCUserMeta: &oidcUser,
-		Comment:      oidcUserComment,
-	}
-
-	err = dao.OnBoardOIDCUser(&user)
-	if err != nil {
-		if strings.Contains(err.Error(), dao.ErrDupUser.Error()) {
-			oc.RenderError(http.StatusConflict, "Conflict, the user with same username or email has been onboarded.")
-			return
-		}
-		oc.SendInternalServerError(err)
-		oc.DelSession(userInfoKey)
-		return
-	}
 
+	if user, onboarded := userOnboard(oc, d, username, tb); onboarded {
 		user.OIDCUserMeta = nil
 		oc.DelSession(userInfoKey)
-	oc.PopulateUserSession(user)
+		oc.PopulateUserSession(*user)
+	}
+
 }
 
 func secretAndToken(tokenBytes []byte) (string, string, error) {

src/portal/src/app/config/auth/config-auth.component.html

@@ -390,8 +390,37 @@
                     [(ngModel)]="currentConfig.oidc_verify_cert.value" />
                 </clr-checkbox-wrapper>
         </clr-checkbox-container>
+        <clr-checkbox-container>
+            <label for="oidcAutoOnboard">{{'CONFIG.OIDC.OIDC_AUTOONBOARD' | translate}}
+                    <clr-tooltip>
+                            <clr-icon clrTooltipTrigger shape="info-circle" size="24"></clr-icon>
+                            <clr-tooltip-content clrPosition="top-right" clrSize="lg" *clrIfOpen>
+                                <span>{{'TOOLTIP.OIDC_AUTOONBOARD' | translate}}</span>
+                            </clr-tooltip-content>
+                        </clr-tooltip>
+            </label>
+            <clr-checkbox-wrapper>
+                <input type="checkbox" clrCheckbox name="oidcAutoOnboard" id="oidcAutoOnboard"
+                [disabled]="disabled(currentConfig.oidc_auto_onboard)"
+                [(ngModel)]="currentConfig.oidc_auto_onboard.value" />
+            </clr-checkbox-wrapper>
+        </clr-checkbox-container>         
+        <clr-input-container>
+            <label  for="oidcUserClaim">{{'CONFIG.OIDC.USER_CLAIM' | translate}}
+                <clr-tooltip>
+                    <clr-icon clrTooltipTrigger shape="info-circle" size="24"></clr-icon>
+                    <clr-tooltip-content clrPosition="top-right" clrSize="lg" *clrIfOpen>
+                        <span>{{'TOOLTIP.OIDC_USER_CLAIM' | translate}}</span>
+                    </clr-tooltip-content>
+                </clr-tooltip>
+            </label>
+            <input clrInput name="oidcUserClaim" type="text" #oidcUserClaimInput="ngModel"
+                [(ngModel)]="currentConfig.oidc_user_claim.value" id="oidcUserClaim" size="40"
+                [disabled]="disabled(currentConfig.oidc_user_claim)" pattern="^[a-zA-Z0-9_-]*$">
+        </clr-input-container>
         <div class="oidc-tip">{{ 'CONFIG.OIDC.OIDC_REDIREC_URL' | translate}}
-            <span>{{redirectUrl}}/c/oidc/callback</span></div>
+            <span>{{redirectUrl}}/c/oidc/callback</span>
+        </div>
     </section>
 </form>
 <div>

src/portal/src/i18n/lang/en-us-lang.json

@@ -102,6 +102,8 @@
         "OIDC_VERIFYCERT": "Uncheck this box if your OIDC server is hosted via self-signed certificate.",
         "OIDC_GROUP_CLAIM": "The name of Claim in the ID token whose value is the list of group names.",
         "OIDC_GROUP_CLAIM_WARNING": "It can only contain letters, numbers, underscores, and the input length is no more than 256 characters.",
+        "OIDC_AUTOONBOARD": "Skip the onboarding screen, so user cannot change its username. Username is provided from ID Token",
+        "OIDC_USER_CLAIM": "The name of the claim in the ID Token where the username is retrieved from. If not specified, it will default to 'name'",
         "NEW_SECRET": "The secret must longer than 8 chars with at least 1 uppercase letter, 1 lowercase letter and 1 number"
     },
     "PLACEHOLDER": {
@@ -911,6 +913,8 @@
             "CLIENTSECRET": "OIDC Client Secret",
             "SCOPE": "OIDC Scope",
             "OIDC_VERIFYCERT": "Verify Certificate",
+            "OIDC_AUTOONBOARD": "Automatic onboarding",
+            "USER_CLAIM": "Username Claim",    
             "OIDC_SETNAME": "Set OIDC Username",
             "OIDC_SETNAMECONTENT": "You must create a Harbor username the first time when authenticating via a third party(OIDC).This will be used within Harbor to be associated with projects, roles, etc.",
             "OIDC_USERNAME": "Username",

src/portal/src/lib/components/config/config.ts

@@ -98,7 +98,9 @@ export class Configuration {
     oidc_client_id?: StringValueItem;
     oidc_client_secret?: StringValueItem;
     oidc_verify_cert?: BoolValueItem;
+    oidc_auto_onboard?: BoolValueItem;
     oidc_scope?: StringValueItem;
+    oidc_user_claim?: StringValueItem;
     count_per_project: NumberValueItem;
     storage_per_project: NumberValueItem;
     cfg_expiration: NumberValueItem;
@@ -155,8 +157,10 @@ export class Configuration {
         this.oidc_client_id = new StringValueItem('', true);
         this.oidc_client_secret = new StringValueItem('', true);
         this.oidc_verify_cert = new BoolValueItem(false, true);
+        this.oidc_auto_onboard = new BoolValueItem(false, true);
         this.oidc_scope = new StringValueItem('', true);
         this.oidc_groups_claim = new StringValueItem('', true);
+        this.oidc_user_claim = new StringValueItem('', true);
         this.count_per_project = new NumberValueItem(-1, true);
         this.storage_per_project = new NumberValueItem(-1, true);
     }