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>
This commit is contained in:
Tim Möhlmann 2024-04-04 08:41:44 +03:00 committed by GitHub
parent f862e43ede
commit 9b3f3e4cd9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 79 additions and 11 deletions

View File

@ -38,7 +38,12 @@ func (s *Session) FetchUser(ctx context.Context) (user idp.User, err error) {
return nil, err return nil, err
} }
} }
info, err := rp.Userinfo[*oidc.UserInfo](ctx,
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.AccessToken,
s.Tokens.TokenType, s.Tokens.TokenType,
s.Tokens.IDTokenClaims.GetSubject(), s.Tokens.IDTokenClaims.GetSubject(),
@ -47,8 +52,6 @@ func (s *Session) FetchUser(ctx context.Context) (user idp.User, err error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
if s.Provider.useIDToken {
info = s.Tokens.IDTokenClaims.GetUserInfo()
} }
u := s.Provider.userInfoMapper(info) u := s.Provider.userInfoMapper(info)
return u, nil return u, nil

View File

@ -54,6 +54,7 @@ func TestSession_FetchUser(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
fields fields fields fields
opts []ProviderOpts
want want want want
}{ }{
{ {
@ -198,6 +199,70 @@ func TestSession_FetchUser(t *testing.T) {
profile: "profile", 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", name: "successful fetch with token exchange",
fields: fields{ fields: fields{
@ -260,7 +325,7 @@ func TestSession_FetchUser(t *testing.T) {
tt.fields.httpMock(tt.fields.issuer) tt.fields.httpMock(tt.fields.issuer)
a := assert.New(t) 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) require.NoError(t, err)
session := &Session{ session := &Session{
@ -275,7 +340,7 @@ func TestSession_FetchUser(t *testing.T) {
a.Fail("invalid error", "expected %v, got %v", tt.want.err, err) a.Fail("invalid error", "expected %v, got %v", tt.want.err, err)
} }
if tt.want.err == nil { if tt.want.err == nil {
a.NoError(err) require.NoError(t, err)
a.Equal(tt.want.id, user.GetID()) a.Equal(tt.want.id, user.GetID())
a.Equal(tt.want.firstName, user.GetFirstName()) a.Equal(tt.want.firstName, user.GetFirstName())
a.Equal(tt.want.lastName, user.GetLastName()) a.Equal(tt.want.lastName, user.GetLastName())