TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2025-08-12 00:27:31 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(oidc): nil check for client secret (#7115)

Browse Source 
This fixes a nil pointer panic when client basic auth is attempted on a client without secret in introspection.
This commit is contained in:
Tim Möhlmann
2023-12-28 15:31:41 +02:00
committed by GitHub
parent 9d5d1cf3ea
commit 45ccdcfa99
3 changed files with 128 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -61,46 +61,116 @@ func TestServer_Introspect(t *testing.T) {
require.NoError(t, err)
app, err := Tester.CreateOIDCNativeClient(CTX, redirectURI, logoutRedirectURI, project.GetId(), false)
require.NoError(t, err)
api, err := Tester.CreateAPIClient(CTX, project.GetId())
require.NoError(t, err)
keyResp, err := Tester.Client.Mgmt.AddAppKey(CTX, &management.AddAppKeyRequest{
ProjectId: project.GetId(),
AppId: api.GetAppId(),
Type: authn.KeyType_KEY_TYPE_JSON,
ExpirationDate: nil,
})
require.NoError(t, err)
resourceServer, err := Tester.CreateResourceServer(CTX, keyResp.GetKeyDetails())
require.NoError(t, err)
scope := []string{oidc.ScopeOpenID, oidc.ScopeProfile, oidc.ScopeEmail, oidc.ScopeOfflineAccess, oidc_api.ScopeResourceOwner}
authRequestID := createAuthRequest(t, app.GetClientId(), redirectURI, scope...)
sessionID, sessionToken, startTime, changeTime := Tester.CreateVerifiedWebAuthNSession(t, CTXLOGIN, User.GetUserId())
linkResp, err := Tester.Client.OIDCv2.CreateCallback(CTXLOGIN, &oidc_pb.CreateCallbackRequest{
AuthRequestId: authRequestID,
CallbackKind: &oidc_pb.CreateCallbackRequest_Session{
Session: &oidc_pb.Session{
SessionId: sessionID,
SessionToken: sessionToken,
wantAudience := []string{app.GetClientId(), project.GetId()}
tests := []struct {
name string
api func(*testing.T) (apiID string, resourceServer rs.ResourceServer)
wantErr bool
}{
{
name: "client assertion",
api: func(t *testing.T) (string, rs.ResourceServer) {
api, err := Tester.CreateAPIClientJWT(CTX, project.GetId())
require.NoError(t, err)
keyResp, err := Tester.Client.Mgmt.AddAppKey(CTX, &management.AddAppKeyRequest{
ProjectId: project.GetId(),
AppId: api.GetAppId(),
Type: authn.KeyType_KEY_TYPE_JSON,
ExpirationDate: nil,
})
require.NoError(t, err)
resourceServer, err := Tester.CreateResourceServerJWTProfile(CTX, keyResp.GetKeyDetails())
require.NoError(t, err)
return api.GetClientId(), resourceServer
},
},
})
require.NoError(t, err)
{
name: "client credentials",
api: func(t *testing.T) (string, rs.ResourceServer) {
api, err := Tester.CreateAPIClientBasic(CTX, project.GetId())
require.NoError(t, err)
resourceServer, err := Tester.CreateResourceServerClientCredentials(CTX, api.GetClientId(), api.GetClientSecret())
require.NoError(t, err)
return api.GetClientId(), resourceServer
},
},
{
name: "client invalid id, error",
api: func(t *testing.T) (string, rs.ResourceServer) {
api, err := Tester.CreateAPIClientBasic(CTX, project.GetId())
require.NoError(t, err)
resourceServer, err := Tester.CreateResourceServerClientCredentials(CTX, "xxxxx", api.GetClientSecret())
require.NoError(t, err)
return api.GetClientId(), resourceServer
},
wantErr: true,
},
{
name: "client invalid secret, error",
api: func(t *testing.T) (string, rs.ResourceServer) {
api, err := Tester.CreateAPIClientBasic(CTX, project.GetId())
require.NoError(t, err)
resourceServer, err := Tester.CreateResourceServerClientCredentials(CTX, api.GetClientId(), "xxxxx")
require.NoError(t, err)
return api.GetClientId(), resourceServer
},
wantErr: true,
},
{
name: "client credentials on jwt client, error",
api: func(t *testing.T) (string, rs.ResourceServer) {
api, err := Tester.CreateAPIClientJWT(CTX, project.GetId())
require.NoError(t, err)
resourceServer, err := Tester.CreateResourceServerClientCredentials(CTX, api.GetClientId(), "xxxxx")
require.NoError(t, err)
return api.GetClientId(), resourceServer
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
apiID, resourceServer := tt.api(t)
// wantAudience grows for every API we add to the project.
wantAudience = append(wantAudience, apiID)
// code exchange
code := assertCodeResponse(t, linkResp.GetCallbackUrl())
tokens, err := exchangeTokens(t, app.GetClientId(), code)
require.NoError(t, err)
assertTokens(t, tokens, true)
assertIDTokenClaims(t, tokens.IDTokenClaims, armPasskey, startTime, changeTime)
scope := []string{oidc.ScopeOpenID, oidc.ScopeProfile, oidc.ScopeEmail, oidc.ScopeOfflineAccess, oidc_api.ScopeResourceOwner}
authRequestID := createAuthRequest(t, app.GetClientId(), redirectURI, scope...)
sessionID, sessionToken, startTime, changeTime := Tester.CreateVerifiedWebAuthNSession(t, CTXLOGIN, User.GetUserId())
linkResp, err := Tester.Client.OIDCv2.CreateCallback(CTXLOGIN, &oidc_pb.CreateCallbackRequest{
AuthRequestId: authRequestID,
CallbackKind: &oidc_pb.CreateCallbackRequest_Session{
Session: &oidc_pb.Session{
SessionId: sessionID,
SessionToken: sessionToken,
},
},
})
require.NoError(t, err)
// test actual introspection
introspection, err := rs.Introspect[*oidc.IntrospectionResponse](context.Background(), resourceServer, tokens.AccessToken)
require.NoError(t, err)
assertIntrospection(t, introspection,
Tester.OIDCIssuer(), app.GetClientId(),
scope, []string{app.GetClientId(), api.GetClientId(), project.GetId()},
tokens.Expiry, tokens.Expiry.Add(-12*time.Hour))
// code exchange
code := assertCodeResponse(t, linkResp.GetCallbackUrl())
tokens, err := exchangeTokens(t, app.GetClientId(), code)
require.NoError(t, err)
assertTokens(t, tokens, true)
assertIDTokenClaims(t, tokens.IDTokenClaims, armPasskey, startTime, changeTime)
// test actual introspection
introspection, err := rs.Introspect[*oidc.IntrospectionResponse](context.Background(), resourceServer, tokens.AccessToken)
if tt.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
assertIntrospection(t, introspection,
Tester.OIDCIssuer(), app.GetClientId(),
scope, wantAudience,
tokens.Expiry, tokens.Expiry.Add(-12*time.Hour))
})
}
}
func assertUserinfo(t *testing.T, userinfo *oidc.UserInfo) {

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

@@ -72,7 +72,7 @@ func (s *Server) Introspect(ctx context.Context, r *op.Request[op.IntrospectionR
return nil, err
}
// remaining errors shoudn't be returned to the client,
// remaining errors shouldn't be returned to the client,
// so we catch errors here, log them and return the response
// with active: false
defer func() {
@@ -122,6 +122,8 @@ type introspectionClientResult struct {
err error
}
var errNoClientSecret = errors.New("client has no configured secret")
func (s *Server) introspectionClientAuth(ctx context.Context, cc *op.ClientCredentials, rc chan<- *introspectionClientResult) {
ctx, span := tracing.NewSpan(ctx)
@@ -136,13 +138,16 @@ func (s *Server) introspectionClientAuth(ctx context.Context, cc *op.ClientCrede
if _, err := op.VerifyJWTAssertion(ctx, cc.ClientAssertion, verifier); err != nil {
return "", "", oidc.ErrUnauthorizedClient().WithParent(err)
}
} else {
return client.ClientID, client.ProjectID, nil
}
if client.ClientSecret != nil {
if err := crypto.CompareHash(client.ClientSecret, []byte(cc.ClientSecret), s.hashAlg); err != nil {
return "", "", oidc.ErrUnauthorizedClient().WithParent(err)
}
return client.ClientID, client.ProjectID, nil
}
return client.ClientID, client.ProjectID, nil
return "", "", oidc.ErrUnauthorizedClient().WithParent(errNoClientSecret)
}()
span.EndWithError(err)