Merge pull request #1456 from srenatus/sr/post-1448/fix-1455/restore-error-semantics

connectors/oidc: truely ignore "email_verified" claim if configured that way
This commit is contained in:
Stephan Renatus 2019-05-28 16:23:00 +02:00 committed by GitHub
commit dfb2dfd333
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 57 additions and 30 deletions

View File

@ -219,7 +219,11 @@ func (c *oidcConnector) HandleCallback(s connector.Scopes, r *http.Request) (ide
} }
emailVerified, found := claims["email_verified"].(bool) emailVerified, found := claims["email_verified"].(bool)
if !found { if !found {
return identity, errors.New("missing \"email_verified\" claim") if c.insecureSkipEmailVerified {
emailVerified = true
} else {
return identity, errors.New("missing \"email_verified\" claim")
}
} }
hostedDomain, _ := claims["hd"].(string) hostedDomain, _ := claims["hd"].(string)
@ -237,10 +241,6 @@ func (c *oidcConnector) HandleCallback(s connector.Scopes, r *http.Request) (ide
} }
} }
if c.insecureSkipEmailVerified {
emailVerified = true
}
if c.getUserInfo { if c.getUserInfo {
userInfo, err := c.provider.UserInfo(r.Context(), oauth2.StaticTokenSource(token)) userInfo, err := c.provider.UserInfo(r.Context(), oauth2.StaticTokenSource(token))
if err != nil { if err != nil {

View File

@ -45,29 +45,62 @@ func TestHandleCallback(t *testing.T) {
t.Helper() t.Helper()
tests := []struct { tests := []struct {
name string name string
userIDKey string userIDKey string
expectUserID string insecureSkipEmailVerified bool
expectUserID string
token map[string]interface{}
}{ }{
{"simpleCase", "", "sub"}, {
{"withUserIDKey", "name", "name"}, name: "simpleCase",
userIDKey: "", // not configured
expectUserID: "subvalue",
token: map[string]interface{}{
"sub": "subvalue",
"name": "namevalue",
"email": "emailvalue",
"email_verified": true,
},
},
{
name: "email_verified not in claims, configured to be skipped",
insecureSkipEmailVerified: true,
expectUserID: "subvalue",
token: map[string]interface{}{
"sub": "subvalue",
"name": "namevalue",
"email": "emailvalue",
},
},
{
name: "withUserIDKey",
userIDKey: "name",
expectUserID: "namevalue",
token: map[string]interface{}{
"sub": "subvalue",
"name": "namevalue",
"email": "emailvalue",
"email_verified": true,
},
},
} }
for _, tc := range tests { for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
testServer, err := setupServer() testServer, err := setupServer(tc.token)
if err != nil { if err != nil {
t.Fatal("failed to setup test server", err) t.Fatal("failed to setup test server", err)
} }
defer testServer.Close() defer testServer.Close()
serverURL := testServer.URL serverURL := testServer.URL
config := Config{ config := Config{
Issuer: serverURL, Issuer: serverURL,
ClientID: "clientID", ClientID: "clientID",
ClientSecret: "clientSecret", ClientSecret: "clientSecret",
Scopes: []string{"groups"}, Scopes: []string{"groups"},
RedirectURI: fmt.Sprintf("%s/callback", serverURL), RedirectURI: fmt.Sprintf("%s/callback", serverURL),
UserIDKey: tc.userIDKey, UserIDKey: tc.userIDKey,
InsecureSkipEmailVerified: tc.insecureSkipEmailVerified,
} }
conn, err := newConnector(config) conn, err := newConnector(config)
@ -86,14 +119,14 @@ func TestHandleCallback(t *testing.T) {
} }
expectEquals(t, identity.UserID, tc.expectUserID) expectEquals(t, identity.UserID, tc.expectUserID)
expectEquals(t, identity.Username, "name") expectEquals(t, identity.Username, "namevalue")
expectEquals(t, identity.Email, "email") expectEquals(t, identity.Email, "emailvalue")
expectEquals(t, identity.EmailVerified, true) expectEquals(t, identity.EmailVerified, true)
}) })
} }
} }
func setupServer() (*httptest.Server, error) { func setupServer(tok map[string]interface{}) (*httptest.Server, error) {
key, err := rsa.GenerateKey(rand.Reader, 1024) key, err := rsa.GenerateKey(rand.Reader, 1024)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to generate rsa key: %v", err) return nil, fmt.Errorf("failed to generate rsa key: %v", err)
@ -121,16 +154,10 @@ func setupServer() (*httptest.Server, error) {
mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) { mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) {
url := fmt.Sprintf("http://%s", r.Host) url := fmt.Sprintf("http://%s", r.Host)
tok["iss"] = url
token, err := newToken(&jwk, map[string]interface{}{ tok["exp"] = time.Now().Add(time.Hour).Unix()
"iss": url, tok["aud"] = "clientID"
"aud": "clientID", token, err := newToken(&jwk, tok)
"exp": time.Now().Add(time.Hour).Unix(),
"sub": "sub",
"name": "name",
"email": "email",
"email_verified": true,
})
if err != nil { if err != nil {
w.WriteHeader(http.StatusInternalServerError) w.WriteHeader(http.StatusInternalServerError)
} }