mirror of
https://github.com/andwati/zitadel.git
synced 2026-10-18 05:52:51 +00:00
Merge commit from fork
* fix: added client and scope validation for token exchange Verify that the token exchange is not cross client Only yield scopes which were already on the initial token * comments * added test
This commit is contained in:
@@ -248,22 +248,14 @@ func TestServer_TokenExchange(t *testing.T) {
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "EXCHANGE: alternate scope for refresh token",
|
||||
name: "EXCHANGE: alternate scope for refresh token is rejected when not subset of subject scopes",
|
||||
args: args{
|
||||
SubjectToken: noPermPAT,
|
||||
SubjectTokenType: oidc.AccessTokenType,
|
||||
RequestedTokenType: oidc.AccessTokenType,
|
||||
Scopes: []string{oidc.ScopeOpenID, oidc.ScopeOfflineAccess, "profile"},
|
||||
},
|
||||
want: result{
|
||||
issuedTokenType: oidc.AccessTokenType,
|
||||
tokenType: oidc.BearerToken,
|
||||
expiresIn: 43100,
|
||||
scopes: []string{oidc.ScopeOpenID, oidc.ScopeOfflineAccess, "profile"},
|
||||
verifyAccessToken: accessTokenVerifier(ctx, resourceServer, serviceUserID, ""),
|
||||
verifyIDToken: idTokenVerifier(ctx, relyingParty, serviceUserID, ""),
|
||||
verifyRefreshToken: refreshTokenVerifier(ctx, relyingParty, "", ""),
|
||||
},
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "EXCHANGE: access token, requested token type not supported error",
|
||||
@@ -322,6 +314,57 @@ func TestServer_TokenExchange(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestServer_TokenExchange_GHSA_vrh8_c9cm_wh8v(t *testing.T) {
|
||||
instance := integration.NewInstance(CTX)
|
||||
ctx := instance.WithAuthorization(CTX, integration.UserTypeIAMOwner)
|
||||
|
||||
t.Run("SECURITY: client should not exchange access tokens of other clients", func(t *testing.T) {
|
||||
// Create two separate token-exchange clients (different projects).
|
||||
clientA, keyDataA, err := instance.CreateOIDCTokenExchangeClient(ctx, t)
|
||||
require.NoError(t, err)
|
||||
signerA, err := rp.SignerFromKeyFile(keyDataA)()
|
||||
require.NoError(t, err)
|
||||
exchangerA, err := tokenexchange.NewTokenExchangerJWTProfile(ctx, instance.OIDCIssuer(), clientA.GetClientId(), signerA)
|
||||
require.NoError(t, err)
|
||||
|
||||
clientB, keyDataB, err := instance.CreateOIDCTokenExchangeClient(ctx, t)
|
||||
require.NoError(t, err)
|
||||
signerB, err := rp.SignerFromKeyFile(keyDataB)()
|
||||
require.NoError(t, err)
|
||||
exchangerB, err := tokenexchange.NewTokenExchangerJWTProfile(ctx, instance.OIDCIssuer(), clientB.GetClientId(), signerB)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Mint a valid access token for clientA to use as subject_token.
|
||||
_, servicePAT := createMachineUserPATWithMembership(ctx, t, instance)
|
||||
source, err := tokenexchange.ExchangeToken(ctx, exchangerA, servicePAT, oidc.AccessTokenType, "", "", nil, nil, []string{oidc.ScopeOpenID}, oidc.AccessTokenType)
|
||||
require.NoError(t, err)
|
||||
|
||||
// RFC 8693 requires that the exchanged token belongs to the requesting client.
|
||||
_, err = tokenexchange.ExchangeToken(ctx, exchangerB, source.AccessToken, oidc.AccessTokenType, "", "", nil, nil, nil, oidc.AccessTokenType)
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "subject_token invalid")
|
||||
})
|
||||
|
||||
t.Run("SECURITY: requested scopes must be subset of subject token scopes", func(t *testing.T) {
|
||||
clientA, keyDataA, err := instance.CreateOIDCTokenExchangeClient(ctx, t)
|
||||
require.NoError(t, err)
|
||||
signerA, err := rp.SignerFromKeyFile(keyDataA)()
|
||||
require.NoError(t, err)
|
||||
exchangerA, err := tokenexchange.NewTokenExchangerJWTProfile(ctx, instance.OIDCIssuer(), clientA.GetClientId(), signerA)
|
||||
require.NoError(t, err)
|
||||
|
||||
_, servicePAT := createMachineUserPATWithMembership(ctx, t, instance)
|
||||
limited, err := tokenexchange.ExchangeToken(ctx, exchangerA, servicePAT, oidc.AccessTokenType, "", "", nil, nil, nil, oidc.AccessTokenType)
|
||||
require.NoError(t, err)
|
||||
assert.NotContains(t, limited.Scopes, oidc.ScopeOfflineAccess)
|
||||
|
||||
// Asking for offline_access escalates beyond the subject token's scope set and must fail.
|
||||
_, err = tokenexchange.ExchangeToken(ctx, exchangerA, limited.AccessToken, oidc.AccessTokenType, "", "", nil, nil, []string{oidc.ScopeOpenID, oidc.ScopeOfflineAccess}, oidc.AccessTokenType)
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "invalid_scope")
|
||||
})
|
||||
}
|
||||
|
||||
func TestServer_TokenExchangeImpersonation(t *testing.T) {
|
||||
instance := integration.NewInstance(CTX)
|
||||
ctx := instance.WithAuthorization(CTX, integration.UserTypeIAMOwner)
|
||||
@@ -499,23 +542,22 @@ func TestServer_TokenExchangeImpersonation(t *testing.T) {
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "ORG IMPERSONATION: subject: access token, actor: access token, with refresh token, success",
|
||||
name: "ORG IMPERSONATION: subject: access token, actor: access token, reduced scopes success",
|
||||
args: args{
|
||||
SubjectToken: teResp.AccessToken,
|
||||
SubjectTokenType: oidc.AccessTokenType,
|
||||
RequestedTokenType: oidc.AccessTokenType,
|
||||
ActorToken: orgImpersonatorPAT,
|
||||
ActorTokenType: oidc.AccessTokenType,
|
||||
Scopes: []string{oidc.ScopeOpenID, oidc.ScopeOfflineAccess},
|
||||
Scopes: []string{oidc.ScopeOpenID},
|
||||
},
|
||||
want: result{
|
||||
issuedTokenType: oidc.AccessTokenType,
|
||||
tokenType: oidc.BearerToken,
|
||||
expiresIn: 43100,
|
||||
scopes: []string{oidc.ScopeOpenID, oidc.ScopeOfflineAccess},
|
||||
verifyAccessToken: accessTokenVerifier(ctx, resourceServer, serviceUserID, orgUserID),
|
||||
verifyIDToken: idTokenVerifier(ctx, relyingParty, serviceUserID, orgUserID),
|
||||
verifyRefreshToken: refreshTokenVerifier(ctx, relyingParty, serviceUserID, orgUserID),
|
||||
issuedTokenType: oidc.AccessTokenType,
|
||||
tokenType: oidc.BearerToken,
|
||||
expiresIn: 43100,
|
||||
scopes: oidc.SpaceDelimitedArray{oidc.ScopeOpenID},
|
||||
verifyAccessToken: accessTokenVerifier(ctx, resourceServer, serviceUserID, orgUserID),
|
||||
verifyIDToken: idTokenVerifier(ctx, relyingParty, serviceUserID, orgUserID),
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
@@ -107,6 +107,11 @@ func (s *Server) verifyExchangeToken(ctx context.Context, client *Client, token
|
||||
if err != nil {
|
||||
return nil, zerrors.ThrowPermissionDenied(err, "OIDC-Osh3t", "Errors.TokenExchange.Token.Invalid")
|
||||
}
|
||||
if !token.isPAT {
|
||||
if err = validateIntrospectionAudience(token.audience, client.GetID(), client.client.ProjectID); err != nil {
|
||||
return nil, zerrors.ThrowPermissionDenied(err, "OIDC-zi9Y0", "Errors.TokenExchange.Token.Invalid")
|
||||
}
|
||||
}
|
||||
if token.isPAT {
|
||||
if err = s.assertClientScopesForPAT(ctx, token, client.GetID(), client.client.ProjectID); err != nil {
|
||||
return nil, err
|
||||
@@ -175,16 +180,21 @@ func (s *Server) jwtProfileUserCheck(ctx context.Context, resourceOwner *string,
|
||||
|
||||
func validateTokenExchangeScopes(client *Client, requestedScopes, subjectScopes, actorScopes []string) ([]string, error) {
|
||||
// Scope always has 1 empty string if the space delimited array was an empty string.
|
||||
scopes := slices.DeleteFunc(requestedScopes, func(s string) bool {
|
||||
requestedScopes = slices.DeleteFunc(requestedScopes, func(s string) bool {
|
||||
return s == ""
|
||||
})
|
||||
if len(scopes) == 0 {
|
||||
scopes = subjectScopes
|
||||
|
||||
if len(requestedScopes) == 0 {
|
||||
return op.ValidateAuthReqScopes(client, actorScopes)
|
||||
}
|
||||
if len(scopes) == 0 {
|
||||
scopes = actorScopes
|
||||
|
||||
for _, scope := range requestedScopes {
|
||||
if !slices.Contains(subjectScopes, scope) || !slices.Contains(actorScopes, scope) {
|
||||
return nil, oidc.ErrInvalidScope().WithDescription("scope %q not found in subject or actor token", scope)
|
||||
}
|
||||
}
|
||||
return op.ValidateAuthReqScopes(client, scopes)
|
||||
|
||||
return op.ValidateAuthReqScopes(client, requestedScopes)
|
||||
}
|
||||
|
||||
func validateTokenExchangeAudience(requestedAudience, subjectAudience, actorAudience []string) ([]string, error) {
|
||||
|
||||
Reference in New Issue
Block a user