From 9b3f3e4cd9bd79767f5fb8286f52c7bf22d4ce41 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= <tim+github@zitadel.com>
Date: Thu, 4 Apr 2024 08:41:44 +0300
Subject: [PATCH] fix(idp): do not call userinfo when mapping from ID token is
 configured (#7696)

* fix(idp): do not call userinfo when mapping from ID token is configured

This change prevents the call of the Userinfo endpoint of a OIDC IDP if the IDP is configured to use the ID token for user information instead.
A unit test has been added to confirm the corrected behavior.

Closes #7373

* video for e2e

---------

Co-authored-by: Livio Spring <livio.a@gmail.com>
---
 internal/idp/providers/oidc/session.go      | 21 ++++---
 internal/idp/providers/oidc/session_test.go | 69 ++++++++++++++++++++-
 2 files changed, 79 insertions(+), 11 deletions(-)

diff --git a/internal/idp/providers/oidc/session.go b/internal/idp/providers/oidc/session.go
index bb44fb1146..bd6303f2e5 100644
--- a/internal/idp/providers/oidc/session.go
+++ b/internal/idp/providers/oidc/session.go
@@ -38,17 +38,20 @@ func (s *Session) FetchUser(ctx context.Context) (user idp.User, err error) {
 			return nil, err
 		}
 	}
-	info, err := rp.Userinfo[*oidc.UserInfo](ctx,
-		s.Tokens.AccessToken,
-		s.Tokens.TokenType,
-		s.Tokens.IDTokenClaims.GetSubject(),
-		s.Provider.RelyingParty,
-	)
-	if err != nil {
-		return nil, err
-	}
+
+	var info *oidc.UserInfo
 	if s.Provider.useIDToken {
 		info = s.Tokens.IDTokenClaims.GetUserInfo()
+	} else {
+		info, err = rp.Userinfo[*oidc.UserInfo](ctx,
+			s.Tokens.AccessToken,
+			s.Tokens.TokenType,
+			s.Tokens.IDTokenClaims.GetSubject(),
+			s.Provider.RelyingParty,
+		)
+		if err != nil {
+			return nil, err
+		}
 	}
 	u := s.Provider.userInfoMapper(info)
 	return u, nil
diff --git a/internal/idp/providers/oidc/session_test.go b/internal/idp/providers/oidc/session_test.go
index 200dac8fc4..9d1a41ed3c 100644
--- a/internal/idp/providers/oidc/session_test.go
+++ b/internal/idp/providers/oidc/session_test.go
@@ -54,6 +54,7 @@ func TestSession_FetchUser(t *testing.T) {
 	tests := []struct {
 		name   string
 		fields fields
+		opts   []ProviderOpts
 		want   want
 	}{
 		{
@@ -198,6 +199,70 @@ func TestSession_FetchUser(t *testing.T) {
 				profile:           "profile",
 			},
 		},
+		{
+			name: "use ID token",
+			fields: fields{
+				name:         "oidc",
+				issuer:       "https://issuer.com",
+				clientID:     "clientID",
+				clientSecret: "clientSecret",
+				redirectURI:  "redirectURI",
+				scopes:       []string{"openid"},
+				userMapper:   DefaultMapper,
+				httpMock: func(issuer string) {
+					gock.New(issuer).
+						Get(oidc.DiscoveryEndpoint).
+						Reply(200).
+						JSON(&oidc.DiscoveryConfiguration{
+							Issuer:                issuer,
+							AuthorizationEndpoint: issuer + "/authorize",
+							TokenEndpoint:         issuer + "/token",
+							UserinfoEndpoint:      issuer + "/userinfo",
+						})
+				},
+				authURL: "https://issuer.com/authorize?client_id=clientID&redirect_uri=redirectURI&response_type=code&scope=openid&state=testState",
+				tokens: &oidc.Tokens[*oidc.IDTokenClaims]{
+					Token: &oauth2.Token{
+						AccessToken: "accessToken",
+						TokenType:   oidc.BearerToken,
+					},
+					IDTokenClaims: func() *oidc.IDTokenClaims {
+						claims := oidc.NewIDTokenClaims(
+							"https://issuer.com",
+							"sub",
+							[]string{"clientID"},
+							time.Now().Add(1*time.Hour),
+							time.Now().Add(-1*time.Second),
+							"nonce",
+							"",
+							nil,
+							"clientID",
+							0,
+						)
+						claims.SetUserInfo(userinfo())
+						return claims
+					}(),
+				},
+			},
+			opts: []ProviderOpts{
+				WithIDTokenMapping(),
+			},
+			want: want{
+				id:                "sub",
+				firstName:         "firstname",
+				lastName:          "lastname",
+				displayName:       "firstname lastname",
+				nickName:          "nickname",
+				preferredUsername: "username",
+				email:             "email",
+				isEmailVerified:   true,
+				phone:             "phone",
+				isPhoneVerified:   true,
+				preferredLanguage: language.English,
+				avatarURL:         "picture",
+				profile:           "profile",
+			},
+		},
 		{
 			name: "successful fetch with token exchange",
 			fields: fields{
@@ -260,7 +325,7 @@ func TestSession_FetchUser(t *testing.T) {
 			tt.fields.httpMock(tt.fields.issuer)
 			a := assert.New(t)
 
-			provider, err := New(tt.fields.name, tt.fields.issuer, tt.fields.clientID, tt.fields.clientSecret, tt.fields.redirectURI, tt.fields.scopes, tt.fields.userMapper)
+			provider, err := New(tt.fields.name, tt.fields.issuer, tt.fields.clientID, tt.fields.clientSecret, tt.fields.redirectURI, tt.fields.scopes, tt.fields.userMapper, tt.opts...)
 			require.NoError(t, err)
 
 			session := &Session{
@@ -275,7 +340,7 @@ func TestSession_FetchUser(t *testing.T) {
 				a.Fail("invalid error", "expected %v, got %v", tt.want.err, err)
 			}
 			if tt.want.err == nil {
-				a.NoError(err)
+				require.NoError(t, err)
 				a.Equal(tt.want.id, user.GetID())
 				a.Equal(tt.want.firstName, user.GetFirstName())
 				a.Equal(tt.want.lastName, user.GetLastName())