TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2024-12-13 11:34:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(oidc): roles in userinfo for client credentials token (#7763)

Browse Source 
* fix(oidc): roles in userinfo for client credentials token

When tokens were obtained using the client credentials grant,
with audience and role scopes, userinfo would not return the role claims. This had multiple causes:

1. There is no auth request flow, so for legacy userinfo project data was never attached to the token
2. For optimized userinfo, there is no client ID that maps to an application. The client ID for client credentials is the machine user's name. There we can't obtain a project ID. When the project ID remained empty, we always ignored the roleAudience.

This PR fixes situation 2, by always taking the roleAudience into account, even when the projectID is empty. The code responsible for the bug is also refactored to be more readable and understandable, including additional godoc.

The fix only applies to the optimized userinfo code introduced in #7706 and released in v2.50 (currently in RC). Therefore it can't be back-ported to earlier versions.

Fixes #6662

* chore(deps): update all go deps (#7764)

This change updates all go modules, including oidc, a major version of go-jose and the go 1.22 release.

* Revert "chore(deps): update all go deps" (#7772)

Revert "chore(deps): update all go deps (#7764)"

This reverts commit 6893e7d060.

---------

Co-authored-by: Livio Spring <livio.a@gmail.com>
(cherry picked from commit 9ccbbe05bc)
This commit is contained in:
Tim Möhlmann 2024-04-16 16:02:38 +03:00 committed by Livio Spring
parent e4843d7692
commit 8054e6753a
No known key found for this signature in database
GPG Key ID: 26BB1C2FA5952CF0
7 changed files with 218 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
internal/api/oidc/client_integration_test.go
View File

@ -348,7 +348,7 @@ func createInvalidKeyData(t testing.TB, client *management.AddOIDCAppResponse) [
}
func TestServer_CreateAccessToken_ClientCredentials(t *testing.T) {
clientID, clientSecret, err := Tester.CreateOIDCCredentialsClient(CTX)
_, clientID, clientSecret, err := Tester.CreateOIDCCredentialsClient(CTX)
require.NoError(t, err)
type clientDetails struct {

2
internal/api/oidc/introspect.go
View File

@ -99,7 +99,7 @@ func (s *Server) Introspect(ctx context.Context, r *op.Request[op.IntrospectionR
if err = validateIntrospectionAudience(token.audience, client.clientID, client.projectID); err != nil {
return nil, err
}
userInfo, err := s.userInfo(ctx, token.userID, client.projectID, client.projectRoleAssertion, token.scope, []string{client.projectID})
userInfo, err := s.userInfo(ctx, token.userID, token.scope, client.projectID, client.projectRoleAssertion, true)
if err != nil {
return nil, err
}

2
internal/api/oidc/token_exchange.go
View File

@ -216,7 +216,7 @@ func (s *Server) createExchangeTokens(ctx context.Context, tokenType oidc.TokenT
)
if slices.Contains(scopes, oidc.ScopeOpenID) || tokenType == oidc.JWTTokenType || tokenType == oidc.IDTokenType {
projectID := client.client.ProjectID
userInfo, err = s.userInfo(ctx, subjectToken.userID, projectID, client.client.ProjectRoleAssertion, scopes, []string{projectID})
userInfo, err = s.userInfo(ctx, subjectToken.userID, scopes, projectID, client.client.ProjectRoleAssertion, false)
if err != nil {
return nil, err
}

50
internal/api/oidc/userinfo.go
View File

@ -53,18 +53,28 @@ func (s *Server) UserInfo(ctx context.Context, r *op.Request[oidc.UserInfoReques
}
}
userInfo, err := s.userInfo(ctx, token.userID, projectID, assertion, token.scope, nil)
userInfo, err := s.userInfo(ctx, token.userID, token.scope, projectID, assertion, false)
if err != nil {
return nil, err
}
return op.NewResponse(userInfo), nil
}
func (s *Server) userInfo(ctx context.Context, userID, projectID string, projectRoleAssertion bool, scope, roleAudience []string) (_ *oidc.UserInfo, err error) {
// userInfo gets the user's data based on the scope.
// The returned UserInfo contains standard and reserved claims, documented
// here: https://zitadel.com/docs/apis/openidoauth/claims.
//
// projectID is an optional parameter which defines the default audience when there are any (or all) role claims requested.
// projectRoleAssertion sets the default of returning all project roles, only if no specific roles were requested in the scope.
//
// currentProjectOnly can be set to use the current project ID only and ignore the audience from the scope.
// It should be set in cases where the client doesn't need to know roles outside its own project,
// for example an introspection client.
func (s *Server) userInfo(ctx context.Context, userID string, scope []string, projectID string, projectRoleAssertion, currentProjectOnly bool) (_ *oidc.UserInfo, err error) {
ctx, span := tracing.NewSpan(ctx)
defer func() { span.EndWithError(err) }()
roleAudience, requestedRoles := prepareRoles(ctx, projectID, projectRoleAssertion, scope, roleAudience)
roleAudience, requestedRoles := prepareRoles(ctx, scope, projectID, projectRoleAssertion, currentProjectOnly)
qu, err := s.query.GetOIDCUserInfo(ctx, userID, roleAudience)
if err != nil {
return nil, err
@ -74,31 +84,35 @@ func (s *Server) userInfo(ctx context.Context, userID, projectID string, project
return userInfo, s.userinfoFlows(ctx, qu, userInfo)
}
// prepareRoles scans the requested scopes, appends to roleAudience and returns the requestedRoles.
// prepareRoles scans the requested scopes and builds the requested roles
// and the audience for which roles need to be asserted.
//
// Scopes with [ScopeProjectRolePrefix] are added to requestedRoles.
// When [ScopeProjectsRoles] is present and roleAudience was empty,
// project IDs with the [domain.ProjectIDScope] prefix are added to the roleAudience.
// When [ScopeProjectsRoles] is present project IDs with the [domain.ProjectIDScope]
// prefix are added to the returned audience.
//
// If projectRoleAssertion is true and the resulting requestedRoles or roleAudience are not empty,
// the current projectID will always be parts or roleAudience.
// Else nil, nil is returned.
func prepareRoles(ctx context.Context, projectID string, projectRoleAssertion bool, scope, roleAudience []string) (ra, requestedRoles []string) {
// if all roles are requested take the audience for those from the scopes
if slices.Contains(scope, ScopeProjectsRoles) && len(roleAudience) == 0 {
roleAudience = domain.AddAudScopeToAudience(ctx, roleAudience, scope)
}
requestedRoles = make([]string, 0, len(scope))
// If projectRoleAssertion is true and there were no specific roles requested,
// the current projectID will always be parts of the returned audience.
func prepareRoles(ctx context.Context, scope []string, projectID string, projectRoleAssertion, currentProjectOnly bool) (roleAudience, requestedRoles []string) {
for _, s := range scope {
if role, ok := strings.CutPrefix(s, ScopeProjectRolePrefix); ok {
requestedRoles = append(requestedRoles, role)
}
}
if !projectRoleAssertion && len(requestedRoles) == 0 && len(roleAudience) == 0 {
return nil, nil
// If roles are requested take the audience for those from the scopes,
// when currentProjectOnly is not set.
if !currentProjectOnly && (len(requestedRoles) > 0 || slices.Contains(scope, ScopeProjectsRoles)) {
roleAudience = domain.AddAudScopeToAudience(ctx, roleAudience, scope)
}
if projectID != "" && !slices.Contains(roleAudience, projectID) {
// When either:
// - Project role assertion is set;
// - Roles for the current project (only) are requested;
// - There is already a roleAudience requested through scope;
// - There are requested roles through the scope;
// and the projectID is not empty, projectID must be part of the roleAudience.
if (projectRoleAssertion || currentProjectOnly || len(roleAudience) > 0 || len(requestedRoles) > 0) && projectID != "" && !slices.Contains(roleAudience, projectID) {
roleAudience = append(roleAudience, projectID)
}
return roleAudience, requestedRoles

104
internal/api/oidc/userinfo_integration_test.go
View File

@ -15,6 +15,7 @@ import (
"golang.org/x/oauth2"
oidc_api "github.com/zitadel/zitadel/internal/api/oidc"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/integration"
feature "github.com/zitadel/zitadel/pkg/grpc/feature/v2beta"
"github.com/zitadel/zitadel/pkg/grpc/management"
@ -66,20 +67,13 @@ func TestServer_UserInfo(t *testing.T) {
// testServer_UserInfo is the actual userinfo integration test,
// which calls the userinfo endpoint with different client configurations, roles and token scopes.
func testServer_UserInfo(t *testing.T) {
const role = "testUserRole"
const (
roleFoo = "foo"
roleBar = "bar"
)
clientID, projectID := createClient(t)
_, err := Tester.Client.Mgmt.AddProjectRole(CTX, &management.AddProjectRoleRequest{
ProjectId: projectID,
RoleKey: role,
DisplayName: "test",
})
require.NoError(t, err)
_, err = Tester.Client.Mgmt.AddUserGrant(CTX, &management.AddUserGrantRequest{
UserId: User.GetUserId(),
ProjectId: projectID,
RoleKeys: []string{role},
})
require.NoError(t, err)
addProjectRolesGrants(t, User.GetUserId(), projectID, roleFoo, roleBar)
tests := []struct {
name string
@ -149,18 +143,34 @@ func testServer_UserInfo(t *testing.T) {
assertions: []func(*testing.T, *oidc.UserInfo){
assertUserinfo,
func(t *testing.T, ui *oidc.UserInfo) {
assertProjectRoleClaims(t, projectID, ui.Claims, role)
assertProjectRoleClaims(t, projectID, ui.Claims, true, roleFoo, roleBar)
},
},
},
{
name: "projects roles scope",
name: "project role scope",
prepare: getTokens,
scope: []string{oidc.ScopeProfile, oidc.ScopeOpenID, oidc.ScopeEmail, oidc.ScopeOfflineAccess, oidc_api.ScopeProjectRolePrefix + role},
scope: []string{oidc.ScopeProfile, oidc.ScopeOpenID, oidc.ScopeEmail, oidc.ScopeOfflineAccess,
oidc_api.ScopeProjectRolePrefix + roleFoo,
},
assertions: []func(*testing.T, *oidc.UserInfo){
assertUserinfo,
func(t *testing.T, ui *oidc.UserInfo) {
assertProjectRoleClaims(t, projectID, ui.Claims, role)
assertProjectRoleClaims(t, projectID, ui.Claims, true, roleFoo)
},
},
},
{
name: "project role and audience scope",
prepare: getTokens,
scope: []string{oidc.ScopeProfile, oidc.ScopeOpenID, oidc.ScopeEmail, oidc.ScopeOfflineAccess,
oidc_api.ScopeProjectRolePrefix + roleFoo,
domain.ProjectIDScope + projectID + domain.AudSuffix,
},
assertions: []func(*testing.T, *oidc.UserInfo){
assertUserinfo,
func(t *testing.T, ui *oidc.UserInfo) {
assertProjectRoleClaims(t, projectID, ui.Claims, true, roleFoo)
},
},
},
@ -211,6 +221,57 @@ func testServer_UserInfo(t *testing.T) {
}
}
// https://github.com/zitadel/zitadel/issues/6662
func TestServer_UserInfo_Issue6662(t *testing.T) {
const (
roleFoo = "foo"
roleBar = "bar"
)
project, err := Tester.CreateProject(CTX)
projectID := project.GetId()
require.NoError(t, err)
userID, clientID, clientSecret, err := Tester.CreateOIDCCredentialsClient(CTX)
require.NoError(t, err)
addProjectRolesGrants(t, userID, projectID, roleFoo, roleBar)
scope := []string{oidc.ScopeProfile, oidc.ScopeOpenID, oidc.ScopeEmail, oidc.ScopeOfflineAccess,
oidc_api.ScopeProjectRolePrefix + roleFoo,
domain.ProjectIDScope + projectID + domain.AudSuffix,
}
provider, err := rp.NewRelyingPartyOIDC(CTX, Tester.OIDCIssuer(), clientID, clientSecret, redirectURI, scope)
require.NoError(t, err)
tokens, err := rp.ClientCredentials(CTX, provider, nil)
require.NoError(t, err)
userinfo, err := rp.Userinfo[*oidc.UserInfo](CTX, tokens.AccessToken, tokens.TokenType, userID, provider)
require.NoError(t, err)
assertProjectRoleClaims(t, projectID, userinfo.Claims, false, roleFoo)
}
func addProjectRolesGrants(t *testing.T, userID, projectID string, roles ...string) {
t.Helper()
bulkRoles := make([]*management.BulkAddProjectRolesRequest_Role, len(roles))
for i, role := range roles {
bulkRoles[i] = &management.BulkAddProjectRolesRequest_Role{
Key: role,
DisplayName: role,
}
}
_, err := Tester.Client.Mgmt.BulkAddProjectRoles(CTX, &management.BulkAddProjectRolesRequest{
ProjectId: projectID,
Roles: bulkRoles,
})
require.NoError(t, err)
_, err = Tester.Client.Mgmt.AddUserGrant(CTX, &management.AddUserGrantRequest{
UserId: userID,
ProjectId: projectID,
RoleKeys: roles,
})
require.NoError(t, err)
}
func getTokens(t *testing.T, clientID string, scope []string) *oidc.Tokens[*oidc.IDTokenClaims] {
authRequestID := createAuthRequest(t, clientID, redirectURI, scope...)
sessionID, sessionToken, startTime, changeTime := Tester.CreateVerifiedWebAuthNSession(t, CTXLOGIN, User.GetUserId())
@ -255,10 +316,13 @@ func assertNoReservedScopes(t *testing.T, claims map[string]any) {
}
}
func assertProjectRoleClaims(t *testing.T, projectID string, claims map[string]any, roles ...string) {
func assertProjectRoleClaims(t *testing.T, projectID string, claims map[string]any, claimProjectRole bool, roles ...string) {
t.Helper()
projectIDRoleClaim := fmt.Sprintf(oidc_api.ClaimProjectRolesFormat, projectID)
for _, claim := range []string{oidc_api.ClaimProjectRoles, projectIDRoleClaim} {
projectRoleClaims := []string{fmt.Sprintf(oidc_api.ClaimProjectRolesFormat, projectID)}
if claimProjectRole {
projectRoleClaims = append(projectRoleClaims, oidc_api.ClaimProjectRoles)
}
for _, claim := range projectRoleClaims {
roleMap, ok := claims[claim].(map[string]any)
require.Truef(t, ok, "claim %s not found or wrong type %T", claim, claims[claim])
for _, roleKey := range roles {

119
internal/api/oidc/userinfo_test.go
View File

@ -18,25 +18,25 @@ import (
func Test_prepareRoles(t *testing.T) {
type args struct {
projectID string
projectRoleAssertion bool
scope []string
roleAudience []string
projectRoleAssertion bool
currentProjectOnly bool
}
tests := []struct {
name string
args args
wantRa []string
wantRoleAudience []string
wantRequestedRoles []string
}{
{
name: "empty scope and roleAudience",
name: "empty scope",
args: args{
projectID: "projID",
projectRoleAssertion: false,
scope: nil,
roleAudience: nil,
projectRoleAssertion: false,
currentProjectOnly: false,
},
wantRa: nil,
wantRoleAudience: nil,
wantRequestedRoles: nil,
},
{
@ -45,50 +45,121 @@ func Test_prepareRoles(t *testing.T) {
projectID: "projID",
projectRoleAssertion: true,
scope: nil,
roleAudience: nil,
currentProjectOnly: false,
},
wantRa: []string{"projID"},
wantRequestedRoles: []string{},
wantRoleAudience: []string{"projID"},
wantRequestedRoles: nil,
},
{
name: "some scope and roleAudience",
name: "some scope, current project only",
args: args{
projectID: "projID",
projectRoleAssertion: false,
scope: []string{"openid", "profile"},
roleAudience: []string{"project2"},
currentProjectOnly: true,
},
wantRa: []string{"project2", "projID"},
wantRequestedRoles: []string{},
wantRoleAudience: []string{"projID"},
wantRequestedRoles: nil,
},
{
name: "scope projects roles",
args: args{
projectID: "projID",
projectRoleAssertion: false,
scope: []string{ScopeProjectsRoles, domain.ProjectIDScope + "project2" + domain.AudSuffix},
roleAudience: nil,
scope: []string{
"openid", "profile",
ScopeProjectsRoles,
domain.ProjectIDScope + "project2" + domain.AudSuffix,
},
currentProjectOnly: false,
},
wantRa: []string{"project2", "projID"},
wantRequestedRoles: []string{},
wantRoleAudience: []string{"project2", "projID"},
wantRequestedRoles: nil,
},
{
name: "scope projects roles ignored, current project only",
args: args{
projectID: "projID",
projectRoleAssertion: false,
scope: []string{
"openid", "profile",
ScopeProjectsRoles,
domain.ProjectIDScope + "project2" + domain.AudSuffix,
},
currentProjectOnly: true,
},
wantRoleAudience: []string{"projID"},
wantRequestedRoles: nil,
},
{
name: "scope project role prefix",
args: args{
projectID: "projID",
projectRoleAssertion: false,
scope: []string{"openid", "profile", ScopeProjectRolePrefix + "foo", ScopeProjectRolePrefix + "bar"},
roleAudience: nil,
scope: []string{
"openid", "profile",
ScopeProjectRolePrefix + "foo",
ScopeProjectRolePrefix + "bar",
},
currentProjectOnly: false,
},
wantRa: []string{"projID"},
wantRoleAudience: []string{"projID"},
wantRequestedRoles: []string{"foo", "bar"},
},
{
name: "scope project role prefix and audience",
args: args{
projectID: "projID",
projectRoleAssertion: false,
scope: []string{
"openid", "profile",
ScopeProjectRolePrefix + "foo",
ScopeProjectRolePrefix + "bar",
domain.ProjectIDScope + "project2" + domain.AudSuffix,
},
currentProjectOnly: false,
},
wantRoleAudience: []string{"projID", "project2"},
wantRequestedRoles: []string{"foo", "bar"},
},
{
name: "scope project role prefix and audience ignored, current project only",
args: args{
projectID: "projID",
projectRoleAssertion: false,
scope: []string{
"openid", "profile",
ScopeProjectRolePrefix + "foo",
ScopeProjectRolePrefix + "bar",
domain.ProjectIDScope + "project2" + domain.AudSuffix,
},
currentProjectOnly: true,
},
wantRoleAudience: []string{"projID"},
wantRequestedRoles: []string{"foo", "bar"},
},
{
name: "no projectID, scope project role prefix and audience",
args: args{
projectID: "",
projectRoleAssertion: false,
scope: []string{
"openid", "profile",
ScopeProjectRolePrefix + "foo",
ScopeProjectRolePrefix + "bar",
domain.ProjectIDScope + "project2" + domain.AudSuffix,
},
currentProjectOnly: false,
},
wantRoleAudience: []string{"project2"},
wantRequestedRoles: []string{"foo", "bar"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotRa, gotRequestedRoles := prepareRoles(context.Background(), tt.args.projectID, tt.args.projectRoleAssertion, tt.args.scope, tt.args.roleAudience)
assert.Equal(t, tt.wantRa, gotRa, "roleAudience")
assert.Equal(t, tt.wantRequestedRoles, gotRequestedRoles, "requestedRoles")
gotRoleAudience, gotRequestedRoles := prepareRoles(context.Background(), tt.args.scope, tt.args.projectID, tt.args.projectRoleAssertion, tt.args.currentProjectOnly)
assert.ElementsMatch(t, tt.wantRoleAudience, gotRoleAudience, "roleAudience")
assert.ElementsMatch(t, tt.wantRequestedRoles, gotRequestedRoles, "requestedRoles")
})
}
}

8
internal/integration/oidc.go
View File

@ -281,7 +281,7 @@ func CheckRedirect(req *http.Request) (*url.URL, error) {
return resp.Location()
}
func (s *Tester) CreateOIDCCredentialsClient(ctx context.Context) (string, string, error) {
func (s *Tester) CreateOIDCCredentialsClient(ctx context.Context) (userID, clientID, clientSecret string, err error) {
name := gofakeit.Username()
user, err := s.Client.Mgmt.AddMachineUser(ctx, &management.AddMachineUserRequest{
Name: name,
@ -289,13 +289,13 @@ func (s *Tester) CreateOIDCCredentialsClient(ctx context.Context) (string, strin
AccessTokenType: user.AccessTokenType_ACCESS_TOKEN_TYPE_JWT,
})
if err != nil {
return "", "", err
return "", "", "", err
}
secret, err := s.Client.Mgmt.GenerateMachineSecret(ctx, &management.GenerateMachineSecretRequest{
UserId: user.GetUserId(),
})
if err != nil {
return "", "", err
return "", "", "", err
}
return secret.GetClientId(), secret.GetClientSecret(), nil
return user.GetUserId(), secret.GetClientId(), secret.GetClientSecret(), nil
}