Adding oidc email scope check

This helps to avoid "no email claim" error if email scope was not specified.

Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>
This commit is contained in:
m.nabokikh 2019-12-28 12:18:51 +04:00
parent 789272a0c1
commit 383c2fe8b6
2 changed files with 67 additions and 17 deletions

View File

@ -262,15 +262,25 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I
if !found { if !found {
return identity, fmt.Errorf("missing \"%s\" claim", userNameKey) return identity, fmt.Errorf("missing \"%s\" claim", userNameKey)
} }
hasEmailScope := false
for _, s := range c.oauth2Config.Scopes {
if s == "email" {
hasEmailScope = true
break
}
}
email, found := claims["email"].(string) email, found := claims["email"].(string)
if !found { if !found && hasEmailScope {
return identity, errors.New("missing \"email\" claim") return identity, errors.New("missing \"email\" claim")
} }
emailVerified, found := claims["email_verified"].(bool) emailVerified, found := claims["email_verified"].(bool)
if !found { if !found {
if c.insecureSkipEmailVerified { if c.insecureSkipEmailVerified {
emailVerified = true emailVerified = true
} else { } else if hasEmailScope {
return identity, errors.New("missing \"email_verified\" claim") return identity, errors.New("missing \"email_verified\" claim")
} }
} }

View File

@ -50,8 +50,10 @@ func TestHandleCallback(t *testing.T) {
userIDKey string userIDKey string
userNameKey string userNameKey string
insecureSkipEmailVerified bool insecureSkipEmailVerified bool
scopes []string
expectUserID string expectUserID string
expectUserName string expectUserName string
expectedEmailField string
token map[string]interface{} token map[string]interface{}
}{ }{
{ {
@ -60,6 +62,7 @@ func TestHandleCallback(t *testing.T) {
userNameKey: "", // not configured userNameKey: "", // not configured
expectUserID: "subvalue", expectUserID: "subvalue",
expectUserName: "namevalue", expectUserName: "namevalue",
expectedEmailField: "emailvalue",
token: map[string]interface{}{ token: map[string]interface{}{
"sub": "subvalue", "sub": "subvalue",
"name": "namevalue", "name": "namevalue",
@ -72,6 +75,7 @@ func TestHandleCallback(t *testing.T) {
insecureSkipEmailVerified: true, insecureSkipEmailVerified: true,
expectUserID: "subvalue", expectUserID: "subvalue",
expectUserName: "namevalue", expectUserName: "namevalue",
expectedEmailField: "emailvalue",
token: map[string]interface{}{ token: map[string]interface{}{
"sub": "subvalue", "sub": "subvalue",
"name": "namevalue", "name": "namevalue",
@ -83,6 +87,7 @@ func TestHandleCallback(t *testing.T) {
userIDKey: "name", userIDKey: "name",
expectUserID: "namevalue", expectUserID: "namevalue",
expectUserName: "namevalue", expectUserName: "namevalue",
expectedEmailField: "emailvalue",
token: map[string]interface{}{ token: map[string]interface{}{
"sub": "subvalue", "sub": "subvalue",
"name": "namevalue", "name": "namevalue",
@ -95,6 +100,7 @@ func TestHandleCallback(t *testing.T) {
userNameKey: "user_name", userNameKey: "user_name",
expectUserID: "subvalue", expectUserID: "subvalue",
expectUserName: "username", expectUserName: "username",
expectedEmailField: "emailvalue",
token: map[string]interface{}{ token: map[string]interface{}{
"sub": "subvalue", "sub": "subvalue",
"user_name": "username", "user_name": "username",
@ -102,6 +108,33 @@ func TestHandleCallback(t *testing.T) {
"email_verified": true, "email_verified": true,
}, },
}, },
{
name: "emptyEmailScope",
expectUserID: "subvalue",
expectUserName: "namevalue",
expectedEmailField: "",
scopes: []string{"groups"},
insecureSkipEmailVerified: true,
token: map[string]interface{}{
"sub": "subvalue",
"name": "namevalue",
"user_name": "username",
},
},
{
name: "emptyEmailScopeButEmailProvided",
expectUserID: "subvalue",
expectUserName: "namevalue",
expectedEmailField: "emailvalue",
scopes: []string{"groups"},
insecureSkipEmailVerified: true,
token: map[string]interface{}{
"sub": "subvalue",
"name": "namevalue",
"user_name": "username",
"email": "emailvalue",
},
},
} }
for _, tc := range tests { for _, tc := range tests {
@ -111,13 +144,20 @@ func TestHandleCallback(t *testing.T) {
t.Fatal("failed to setup test server", err) t.Fatal("failed to setup test server", err)
} }
defer testServer.Close() defer testServer.Close()
var scopes []string
if len(tc.scopes) > 0 {
scopes = tc.scopes
} else {
scopes = []string{"email", "groups"}
}
serverURL := testServer.URL serverURL := testServer.URL
basicAuth := true basicAuth := true
config := Config{ config := Config{
Issuer: serverURL, Issuer: serverURL,
ClientID: "clientID", ClientID: "clientID",
ClientSecret: "clientSecret", ClientSecret: "clientSecret",
Scopes: []string{"groups"}, Scopes: scopes,
RedirectURI: fmt.Sprintf("%s/callback", serverURL), RedirectURI: fmt.Sprintf("%s/callback", serverURL),
UserIDKey: tc.userIDKey, UserIDKey: tc.userIDKey,
UserNameKey: tc.userNameKey, UserNameKey: tc.userNameKey,
@ -142,7 +182,7 @@ func TestHandleCallback(t *testing.T) {
expectEquals(t, identity.UserID, tc.expectUserID) expectEquals(t, identity.UserID, tc.expectUserID)
expectEquals(t, identity.Username, tc.expectUserName) expectEquals(t, identity.Username, tc.expectUserName)
expectEquals(t, identity.Email, "emailvalue") expectEquals(t, identity.Email, tc.expectedEmailField)
expectEquals(t, identity.EmailVerified, true) expectEquals(t, identity.EmailVerified, true)
}) })
} }