From 3b140a67c8ff07e112be6be93e08ff5664bdb23d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Thu, 12 Sep 2024 15:36:33 +0300 Subject: [PATCH] fix(oidc): always set sub claim (#8598) # Which Problems Are Solved When the `openid` scope was not requested, as is possible in machine authentication, we didn't set the `sub` (subject) claim to tokens and possibly also userInfo and introspection. This fix always sets the `sub` claim for all cases. # How the Problems Are Solved Set the `Subject` field to regardless of passed scopes. # Additional Changes - none # Additional Context According to standards: - [RFC9068 - JSON Web Token (JWT) Profile for OAuth 2.0 Access Tokens](https://datatracker.ietf.org/doc/html/rfc9068#name-data-structure) this claim is **required**. - [RFC7667 - OAuth 2.0 Token Introspection](https://datatracker.ietf.org/doc/html/rfc7662#section-2.2) the claim is optional, however there is no correlation to the `openid` or OpenID Connect. Therefore it doesn't harm to always return this claim. - [OpenID connect, User Info Response](https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse): "The sub (subject) Claim **MUST** always be returned in the UserInfo Response." Closes https://github.com/zitadel/zitadel/issues/8591 --- internal/api/oidc/userinfo.go | 6 ++--- internal/api/oidc/userinfo_test.go | 36 ++++++++++++++++++++++++------ 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/internal/api/oidc/userinfo.go b/internal/api/oidc/userinfo.go index e023d7a63..542ea6a08 100644 --- a/internal/api/oidc/userinfo.go +++ b/internal/api/oidc/userinfo.go @@ -163,11 +163,11 @@ func prepareRoles(ctx context.Context, scope []string, projectID string, project } func userInfoToOIDC(user *query.OIDCUserInfo, userInfoAssertion bool, scope []string, assetPrefix string) *oidc.UserInfo { - out := new(oidc.UserInfo) + out := &oidc.UserInfo{ + Subject: user.User.ID, + } for _, s := range scope { switch s { - case oidc.ScopeOpenID: - out.Subject = user.User.ID case oidc.ScopeEmail: if !userInfoAssertion { continue diff --git a/internal/api/oidc/userinfo_test.go b/internal/api/oidc/userinfo_test.go index 741f7eed3..2646cc903 100644 --- a/internal/api/oidc/userinfo_test.go +++ b/internal/api/oidc/userinfo_test.go @@ -280,14 +280,18 @@ func Test_userInfoToOIDC(t *testing.T) { args: args{ user: humanUserInfo, }, - want: &oidc.UserInfo{}, + want: &oidc.UserInfo{ + Subject: "human1", + }, }, { name: "machine, empty", args: args{ user: machineUserInfo, }, - want: &oidc.UserInfo{}, + want: &oidc.UserInfo{ + Subject: "machine1", + }, }, { name: "human, scope openid", @@ -317,6 +321,7 @@ func Test_userInfoToOIDC(t *testing.T) { scope: []string{oidc.ScopeEmail}, }, want: &oidc.UserInfo{ + Subject: "human1", UserInfoEmail: oidc.UserInfoEmail{ Email: "foo@bar.com", EmailVerified: true, @@ -329,7 +334,9 @@ func Test_userInfoToOIDC(t *testing.T) { user: humanUserInfo, scope: []string{oidc.ScopeEmail}, }, - want: &oidc.UserInfo{}, + want: &oidc.UserInfo{ + Subject: "human1", + }, }, { name: "machine, scope email, profileInfoAssertion", @@ -338,7 +345,7 @@ func Test_userInfoToOIDC(t *testing.T) { scope: []string{oidc.ScopeEmail}, }, want: &oidc.UserInfo{ - UserInfoEmail: oidc.UserInfoEmail{}, + Subject: "machine1", }, }, { @@ -349,6 +356,7 @@ func Test_userInfoToOIDC(t *testing.T) { scope: []string{oidc.ScopeProfile}, }, want: &oidc.UserInfo{ + Subject: "human1", UserInfoProfile: oidc.UserInfoProfile{ Name: "xxx", GivenName: "user", @@ -370,6 +378,7 @@ func Test_userInfoToOIDC(t *testing.T) { scope: []string{oidc.ScopeProfile}, }, want: &oidc.UserInfo{ + Subject: "machine1", UserInfoProfile: oidc.UserInfoProfile{ Name: "machine", UpdatedAt: oidc.FromTime(time.Unix(567, 890)), @@ -383,7 +392,9 @@ func Test_userInfoToOIDC(t *testing.T) { user: machineUserInfo, scope: []string{oidc.ScopeProfile}, }, - want: &oidc.UserInfo{}, + want: &oidc.UserInfo{ + Subject: "machine1", + }, }, { name: "human, scope phone, profileInfoAssertion", @@ -393,6 +404,7 @@ func Test_userInfoToOIDC(t *testing.T) { scope: []string{oidc.ScopePhone}, }, want: &oidc.UserInfo{ + Subject: "human1", UserInfoPhone: oidc.UserInfoPhone{ PhoneNumber: "+31123456789", PhoneNumberVerified: true, @@ -405,7 +417,9 @@ func Test_userInfoToOIDC(t *testing.T) { user: humanUserInfo, scope: []string{oidc.ScopePhone}, }, - want: &oidc.UserInfo{}, + want: &oidc.UserInfo{ + Subject: "human1", + }, }, { name: "machine, scope phone", @@ -414,6 +428,7 @@ func Test_userInfoToOIDC(t *testing.T) { scope: []string{oidc.ScopePhone}, }, want: &oidc.UserInfo{ + Subject: "machine1", UserInfoPhone: oidc.UserInfoPhone{}, }, }, @@ -424,6 +439,8 @@ func Test_userInfoToOIDC(t *testing.T) { scope: []string{ScopeUserMetaData}, }, want: &oidc.UserInfo{ + Subject: "human1", + UserInfoEmail: oidc.UserInfoEmail{}, Claims: map[string]any{ ClaimUserMetaData: map[string]string{ "key1": base64.RawURLEncoding.EncodeToString([]byte{1, 2, 3}), @@ -438,7 +455,9 @@ func Test_userInfoToOIDC(t *testing.T) { user: machineUserInfo, scope: []string{ScopeUserMetaData}, }, - want: &oidc.UserInfo{}, + want: &oidc.UserInfo{ + Subject: "machine1", + }, }, { name: "machine, scope resource owner", @@ -447,6 +466,7 @@ func Test_userInfoToOIDC(t *testing.T) { scope: []string{ScopeResourceOwner}, }, want: &oidc.UserInfo{ + Subject: "machine1", Claims: map[string]any{ ClaimResourceOwnerID: "orgID", ClaimResourceOwnerName: "orgName", @@ -461,6 +481,7 @@ func Test_userInfoToOIDC(t *testing.T) { scope: []string{domain.OrgDomainPrimaryScope + "foo.com"}, }, want: &oidc.UserInfo{ + Subject: "human1", Claims: map[string]any{ domain.OrgDomainPrimaryClaim: "foo.com", }, @@ -473,6 +494,7 @@ func Test_userInfoToOIDC(t *testing.T) { scope: []string{domain.OrgIDScope + "orgID"}, }, want: &oidc.UserInfo{ + Subject: "machine1", Claims: map[string]any{ domain.OrgIDClaim: "orgID", ClaimResourceOwnerID: "orgID",