Make sure auto-onboard username claim has higher priority

This commit adjust the priority when consolidating data from userinfo
endpoint and id token, making sure the auto-onboard username claim from
ID token has highest priority.

fixes #15504

Signed-off-by: Daniel Jiang <jiangd@vmware.com>
This commit is contained in:
Daniel Jiang 2021-09-21 18:59:11 +08:00
parent 04cd65d179
commit 91131098c3
2 changed files with 129 additions and 81 deletions

View File

@ -132,6 +132,7 @@ type UserInfo struct {
Email string `json:"email"`
Groups []string `json:"groups"`
AdminGroupMember bool `json:"admin_group_member"`
autoOnboardUsername string
hasGroupClaim bool
}
@ -291,7 +292,13 @@ func mergeUserInfo(remote, local *UserInfo) *UserInfo {
// Used data from userinfo
Email: remote.Email,
}
if remote.Username != "" {
// priority for username (high to low):
// 1. Username based on the auto onboard claim from ID token
// 2. Username from response of userinfo endpoint
// 3. Username from the default "name" claim from ID token
if local.autoOnboardUsername != "" {
res.Username = local.autoOnboardUsername
} else if remote.Username != "" {
res.Username = remote.Username
} else {
res.Username = local.Username
@ -348,7 +355,7 @@ func userInfoFromClaims(c claimsProvider, setting cfgModels.OIDCSetting) (*UserI
}
if username, ok := allClaims[setting.UserClaim].(string); ok {
res.Username = username
res.autoOnboardUsername = username
} else {
log.Warningf("OIDC. Failed to recover Username from claim. Claim '%s' is invalid or not a string", setting.UserClaim)
}

View File

@ -213,6 +213,7 @@ func TestUserInfoFromClaims(t *testing.T) {
expect: &UserInfo{
Issuer: "",
Subject: "",
autoOnboardUsername: "",
Username: "Daniel",
Email: "daniel@gmail.com",
Groups: []string{},
@ -234,6 +235,7 @@ func TestUserInfoFromClaims(t *testing.T) {
expect: &UserInfo{
Issuer: "",
Subject: "",
autoOnboardUsername: "",
Username: "Daniel",
Email: "daniel@gmail.com",
Groups: []string{"g1", "g2"},
@ -258,6 +260,7 @@ func TestUserInfoFromClaims(t *testing.T) {
expect: &UserInfo{
Issuer: "issuer",
Subject: "subject000",
autoOnboardUsername: "",
Username: "jack",
Email: "jack@gmail.com",
Groups: []string{},
@ -280,7 +283,8 @@ func TestUserInfoFromClaims(t *testing.T) {
expect: &UserInfo{
Issuer: "",
Subject: "",
Username: "airadier@gmail.com",
autoOnboardUsername: "airadier@gmail.com",
Username: "Alvaro",
Email: "airadier@gmail.com",
Groups: []string{},
hasGroupClaim: false,
@ -305,6 +309,7 @@ func TestMergeUserInfo(t *testing.T) {
fromInfo: &UserInfo{
Issuer: "",
Subject: "",
autoOnboardUsername: "",
Username: "daniel",
Email: "daniel@gmail.com",
Groups: []string{},
@ -313,6 +318,7 @@ func TestMergeUserInfo(t *testing.T) {
fromIDToken: &UserInfo{
Issuer: "issuer-google",
Subject: "subject-daniel",
autoOnboardUsername: "",
Username: "daniel",
Email: "daniel@yahoo.com",
Groups: []string{"developers", "everyone"},
@ -331,6 +337,7 @@ func TestMergeUserInfo(t *testing.T) {
fromInfo: &UserInfo{
Issuer: "",
Subject: "",
autoOnboardUsername: "",
Username: "tom",
Email: "tom@gmail.com",
Groups: nil,
@ -339,6 +346,7 @@ func TestMergeUserInfo(t *testing.T) {
fromIDToken: &UserInfo{
Issuer: "issuer-okta",
Subject: "subject-jiangtan",
autoOnboardUsername: "",
Username: "tom",
Email: "tom@okta.com",
Groups: []string{"nouse"},
@ -357,6 +365,7 @@ func TestMergeUserInfo(t *testing.T) {
fromInfo: &UserInfo{
Issuer: "",
Subject: "",
autoOnboardUsername: "",
Username: "jim",
Email: "jim@gmail.com",
Groups: []string{},
@ -365,6 +374,7 @@ func TestMergeUserInfo(t *testing.T) {
fromIDToken: &UserInfo{
Issuer: "issuer-yahoo",
Subject: "subject-jim",
autoOnboardUsername: "",
Username: "jim",
Email: "jim@yaoo.com",
Groups: []string{"g1", "g2"},
@ -383,6 +393,7 @@ func TestMergeUserInfo(t *testing.T) {
fromInfo: &UserInfo{
Issuer: "",
Subject: "",
autoOnboardUsername: "",
Username: "",
Email: "kevin@whatever.com",
Groups: []string{},
@ -391,6 +402,7 @@ func TestMergeUserInfo(t *testing.T) {
fromIDToken: &UserInfo{
Issuer: "issuer-whatever",
Subject: "subject-kevin",
autoOnboardUsername: "",
Username: "kevin",
Email: "kevin@whatever.com",
Groups: []string{"g1", "g2"},
@ -405,6 +417,35 @@ func TestMergeUserInfo(t *testing.T) {
hasGroupClaim: true,
},
},
{
fromInfo: &UserInfo{
Issuer: "",
Subject: "",
// only the auto onboard username from token will be used
autoOnboardUsername: "info-jt",
Username: "",
Email: "jt@whatever.com",
Groups: []string{},
hasGroupClaim: false,
},
fromIDToken: &UserInfo{
Issuer: "issuer-whatever",
Subject: "subject-jt",
autoOnboardUsername: "token-jt",
Username: "jt",
Email: "jt@whatever.com",
Groups: []string{"g1", "g2"},
hasGroupClaim: true,
},
expected: &UserInfo{
Issuer: "issuer-whatever",
Subject: "subject-jt",
Username: "token-jt",
Email: "jt@whatever.com",
Groups: []string{"g1", "g2"},
hasGroupClaim: true,
},
},
}
for _, tc := range s {