fix(oidc): don't push introspection client events (#8481)

# Which Problems Are Solved

Do not push secret succeeded and failed events for API and OIDC clients
on the introspection endpoint.
On instances where introspection was fequently called, the pushed events
created issues on duplicate primary keys, due to collisions on the
`sequence` column in the eventstore. As the event pusher retries on this
collision and we pushed above mentioned events async, it would create a
backpressure of concurrent pushers and effectively cripple an instance.

We considered that pushing these events have little value with regards
to the audit trail, as we do not push similar events when client
assertion is used. Also, before #7657 the events were defined, but not
pushed.

# How the Problems Are Solved

- Removed API secret check succeeded and faild event definitions
- Removed OIDC secret check succeeded and faild event definitions
- Push only Hash Updated event when needed

# Additional Changes

- None

# Additional Context

- Fixes https://github.com/zitadel/zitadel/issues/8479
- Closes https://github.com/zitadel/zitadel/issues/8430
- Intoduced in https://github.com/zitadel/zitadel/pull/7657
This commit is contained in:
Tim Möhlmann 2024-08-28 20:19:50 +02:00 committed by GitHub
parent ca8f82423a
commit 90b908c361
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 38 additions and 256 deletions

View File

@ -1035,10 +1035,11 @@ func (s *Server) verifyClientSecret(ctx context.Context, client *query.OIDCClien
updated, err := s.hasher.Verify(client.HashedSecret, secret)
spanPasswordComparison.EndWithError(err)
if err != nil {
s.command.OIDCSecretCheckFailed(ctx, client.AppID, client.ProjectID, client.Settings.ResourceOwner)
return oidc.ErrInvalidClient().WithParent(err).WithReturnParentToClient(authz.GetFeatures(ctx).DebugOIDCParentError).WithDescription("invalid secret")
}
s.command.OIDCSecretCheckSucceeded(ctx, client.AppID, client.ProjectID, client.Settings.ResourceOwner, updated)
if updated != "" {
s.command.OIDCUpdateSecret(ctx, client.AppID, client.ProjectID, client.Settings.ResourceOwner, updated)
}
return nil
}

View File

@ -183,17 +183,13 @@ func (s *Server) introspectionClientAuth(ctx context.Context, cc *op.ClientCrede
var errNoAppType = errors.New("introspection client without app type")
func (s *Server) introspectionClientSecretAuth(ctx context.Context, client *query.IntrospectionClient, secret string) error {
var (
successCommand func(ctx context.Context, appID, projectID, resourceOwner, updated string)
failedCommand func(ctx context.Context, appID, projectID, resourceOwner string)
)
var updateCommand func(ctx context.Context, appID, projectID, resourceOwner, updated string)
switch client.AppType {
case query.AppTypeAPI:
successCommand = s.command.APISecretCheckSucceeded
failedCommand = s.command.APISecretCheckFailed
updateCommand = s.command.APIUpdateSecret
case query.AppTypeOIDC:
successCommand = s.command.OIDCSecretCheckSucceeded
failedCommand = s.command.OIDCSecretCheckFailed
updateCommand = s.command.OIDCUpdateSecret
default:
return zerrors.ThrowInternal(errNoAppType, "OIDC-ooD5Ot", "Errors.Internal")
}
@ -202,10 +198,11 @@ func (s *Server) introspectionClientSecretAuth(ctx context.Context, client *quer
updated, err := s.hasher.Verify(client.HashedSecret, secret)
spanPasswordComparison.EndWithError(err)
if err != nil {
failedCommand(ctx, client.AppID, client.ProjectID, client.ResourceOwner)
return err
}
successCommand(ctx, client.AppID, client.ProjectID, client.ResourceOwner, updated)
if updated != "" {
updateCommand(ctx, client.AppID, client.ProjectID, client.ResourceOwner, updated)
}
return nil
}

View File

@ -250,22 +250,18 @@ func (c *Commands) VerifyAPIClientSecret(ctx context.Context, projectID, appID,
ctx, spanPasswordComparison := tracing.NewNamedSpan(ctx, "passwap.Verify")
updated, err := c.secretHasher.Verify(app.HashedSecret, secret)
spanPasswordComparison.EndWithError(err)
if err == nil {
c.apiSecretCheckSucceeded(ctx, projectAgg, app.AppID, updated)
return err
if err != nil {
return zerrors.ThrowInvalidArgument(err, "COMMAND-SADfg", "Errors.Project.App.ClientSecretInvalid")
}
c.apiSecretCheckFailed(ctx, projectAgg, app.AppID)
return zerrors.ThrowInvalidArgument(err, "COMMAND-SADfg", "Errors.Project.App.ClientSecretInvalid")
if updated != "" {
c.apiUpdateSecret(ctx, projectAgg, app.AppID, updated)
}
return nil
}
func (c *Commands) APISecretCheckSucceeded(ctx context.Context, appID, projectID, resourceOwner, updated string) {
func (c *Commands) APIUpdateSecret(ctx context.Context, appID, projectID, resourceOwner, updated string) {
agg := project_repo.NewAggregate(projectID, resourceOwner)
c.apiSecretCheckSucceeded(ctx, &agg.Aggregate, appID, updated)
}
func (c *Commands) APISecretCheckFailed(ctx context.Context, appID, projectID, resourceOwner string) {
agg := project_repo.NewAggregate(projectID, resourceOwner)
c.apiSecretCheckFailed(ctx, &agg.Aggregate, appID)
c.apiUpdateSecret(ctx, &agg.Aggregate, appID, updated)
}
func (c *Commands) getAPIAppWriteModel(ctx context.Context, projectID, appID, resourceOwner string) (_ *APIApplicationWriteModel, err error) {
@ -280,17 +276,6 @@ func (c *Commands) getAPIAppWriteModel(ctx context.Context, projectID, appID, re
return appWriteModel, nil
}
func (c *Commands) apiSecretCheckSucceeded(ctx context.Context, agg *eventstore.Aggregate, appID, updated string) {
cmds := append(
make([]eventstore.Command, 0, 2),
project_repo.NewAPIConfigSecretCheckSucceededEvent(ctx, agg, appID),
)
if updated != "" {
cmds = append(cmds, project_repo.NewAPIConfigSecretHashUpdatedEvent(ctx, agg, appID, updated))
}
c.asyncPush(ctx, cmds...)
}
func (c *Commands) apiSecretCheckFailed(ctx context.Context, agg *eventstore.Aggregate, appID string) {
c.asyncPush(ctx, project_repo.NewAPIConfigSecretCheckFailedEvent(ctx, agg, appID))
func (c *Commands) apiUpdateSecret(ctx context.Context, agg *eventstore.Aggregate, appID, updated string) {
c.asyncPush(ctx, project_repo.NewAPIConfigSecretHashUpdatedEvent(ctx, agg, appID, updated))
}

View File

@ -837,9 +837,6 @@ func TestCommands_VerifyAPIClientSecret(t *testing.T) {
project.NewAPIConfigAddedEvent(context.Background(), &agg.Aggregate, "appID", "clientID", hashedSecret, domain.APIAuthMethodTypePrivateKeyJWT),
),
),
expectPush(
project.NewAPIConfigSecretCheckSucceededEvent(context.Background(), &agg.Aggregate, "appID"),
),
),
},
{
@ -854,9 +851,6 @@ func TestCommands_VerifyAPIClientSecret(t *testing.T) {
project.NewAPIConfigAddedEvent(context.Background(), &agg.Aggregate, "appID", "clientID", hashedSecret, domain.APIAuthMethodTypePrivateKeyJWT),
),
),
expectPush(
project.NewAPIConfigSecretCheckFailedEvent(context.Background(), &agg.Aggregate, "appID"),
),
),
wantErr: zerrors.ThrowInvalidArgument(err, "COMMAND-SADfg", "Errors.Project.App.ClientSecretInvalid"),
},

View File

@ -331,22 +331,18 @@ func (c *Commands) VerifyOIDCClientSecret(ctx context.Context, projectID, appID,
ctx, spanPasswordComparison := tracing.NewNamedSpan(ctx, "passwap.Verify")
updated, err := c.secretHasher.Verify(app.HashedSecret, secret)
spanPasswordComparison.EndWithError(err)
if err == nil {
c.oidcSecretCheckSucceeded(ctx, projectAgg, appID, updated)
return nil
if err != nil {
return zerrors.ThrowInvalidArgument(err, "COMMAND-Bz542", "Errors.Project.App.ClientSecretInvalid")
}
c.oidcSecretCheckFailed(ctx, projectAgg, appID)
return zerrors.ThrowInvalidArgument(err, "COMMAND-Bz542", "Errors.Project.App.ClientSecretInvalid")
if updated != "" {
c.oidcUpdateSecret(ctx, projectAgg, appID, updated)
}
return nil
}
func (c *Commands) OIDCSecretCheckSucceeded(ctx context.Context, appID, projectID, resourceOwner, updated string) {
func (c *Commands) OIDCUpdateSecret(ctx context.Context, appID, projectID, resourceOwner, updated string) {
agg := project_repo.NewAggregate(projectID, resourceOwner)
c.oidcSecretCheckSucceeded(ctx, &agg.Aggregate, appID, updated)
}
func (c *Commands) OIDCSecretCheckFailed(ctx context.Context, appID, projectID, resourceOwner string) {
agg := project_repo.NewAggregate(projectID, resourceOwner)
c.oidcSecretCheckFailed(ctx, &agg.Aggregate, appID)
c.oidcUpdateSecret(ctx, &agg.Aggregate, appID, updated)
}
func (c *Commands) getOIDCAppWriteModel(ctx context.Context, projectID, appID, resourceOwner string) (_ *OIDCApplicationWriteModel, err error) {
@ -382,17 +378,6 @@ func trimStringSliceWhiteSpaces(slice []string) []string {
return slice
}
func (c *Commands) oidcSecretCheckSucceeded(ctx context.Context, agg *eventstore.Aggregate, appID, updated string) {
cmds := append(
make([]eventstore.Command, 0, 2),
project_repo.NewOIDCConfigSecretCheckSucceededEvent(ctx, agg, appID),
)
if updated != "" {
cmds = append(cmds, project_repo.NewOIDCConfigSecretHashUpdatedEvent(ctx, agg, appID, updated))
}
c.asyncPush(ctx, cmds...)
}
func (c *Commands) oidcSecretCheckFailed(ctx context.Context, agg *eventstore.Aggregate, appID string) {
c.asyncPush(ctx, project_repo.NewOIDCConfigSecretCheckFailedEvent(ctx, agg, appID))
func (c *Commands) oidcUpdateSecret(ctx context.Context, agg *eventstore.Aggregate, appID, updated string) {
c.asyncPush(ctx, project_repo.NewOIDCConfigSecretHashUpdatedEvent(ctx, agg, appID, updated))
}

View File

@ -1363,9 +1363,6 @@ func TestCommands_VerifyOIDCClientSecret(t *testing.T) {
),
),
),
expectPush(
project.NewOIDCConfigSecretCheckSucceededEvent(context.Background(), &agg.Aggregate, "appID"),
),
),
},
{
@ -1400,9 +1397,6 @@ func TestCommands_VerifyOIDCClientSecret(t *testing.T) {
),
),
),
expectPush(
project.NewOIDCConfigSecretCheckFailedEvent(context.Background(), &agg.Aggregate, "appID"),
),
),
wantErr: zerrors.ThrowInvalidArgument(err, "COMMAND-Bz542", "Errors.Project.App.ClientSecretInvalid"),
},

View File

@ -10,12 +10,10 @@ import (
)
const (
APIConfigAddedType = applicationEventTypePrefix + "config.api.added"
APIConfigChangedType = applicationEventTypePrefix + "config.api.changed"
APIConfigSecretChangedType = applicationEventTypePrefix + "config.api.secret.changed"
APIClientSecretCheckSucceededType = applicationEventTypePrefix + "api.secret.check.succeeded"
APIClientSecretCheckFailedType = applicationEventTypePrefix + "api.secret.check.failed"
APIConfigSecretHashUpdatedType = applicationEventTypePrefix + "config.api.secret.updated"
APIConfigAddedType = applicationEventTypePrefix + "config.api.added"
APIConfigChangedType = applicationEventTypePrefix + "config.api.changed"
APIConfigSecretChangedType = applicationEventTypePrefix + "config.api.secret.changed"
APIConfigSecretHashUpdatedType = applicationEventTypePrefix + "config.api.secret.updated"
)
type APIConfigAddedEvent struct {
@ -202,90 +200,6 @@ func APIConfigSecretChangedEventMapper(event eventstore.Event) (eventstore.Event
return e, nil
}
type APIConfigSecretCheckSucceededEvent struct {
eventstore.BaseEvent `json:"-"`
AppID string `json:"appId"`
}
func (e *APIConfigSecretCheckSucceededEvent) Payload() interface{} {
return e
}
func (e *APIConfigSecretCheckSucceededEvent) UniqueConstraints() []*eventstore.UniqueConstraint {
return nil
}
func NewAPIConfigSecretCheckSucceededEvent(
ctx context.Context,
aggregate *eventstore.Aggregate,
appID string,
) *APIConfigSecretCheckSucceededEvent {
return &APIConfigSecretCheckSucceededEvent{
BaseEvent: *eventstore.NewBaseEventForPush(
ctx,
aggregate,
APIClientSecretCheckSucceededType,
),
AppID: appID,
}
}
func APIConfigSecretCheckSucceededEventMapper(event eventstore.Event) (eventstore.Event, error) {
e := &APIConfigSecretCheckSucceededEvent{
BaseEvent: *eventstore.BaseEventFromRepo(event),
}
err := event.Unmarshal(e)
if err != nil {
return nil, zerrors.ThrowInternal(err, "API-837gV", "unable to unmarshal api config")
}
return e, nil
}
type APIConfigSecretCheckFailedEvent struct {
eventstore.BaseEvent `json:"-"`
AppID string `json:"appId"`
}
func (e *APIConfigSecretCheckFailedEvent) Payload() interface{} {
return e
}
func (e *APIConfigSecretCheckFailedEvent) UniqueConstraints() []*eventstore.UniqueConstraint {
return nil
}
func NewAPIConfigSecretCheckFailedEvent(
ctx context.Context,
aggregate *eventstore.Aggregate,
appID string,
) *APIConfigSecretCheckFailedEvent {
return &APIConfigSecretCheckFailedEvent{
BaseEvent: *eventstore.NewBaseEventForPush(
ctx,
aggregate,
APIClientSecretCheckFailedType,
),
AppID: appID,
}
}
func APIConfigSecretCheckFailedEventMapper(event eventstore.Event) (eventstore.Event, error) {
e := &APIConfigSecretCheckFailedEvent{
BaseEvent: *eventstore.BaseEventFromRepo(event),
}
err := event.Unmarshal(e)
if err != nil {
return nil, zerrors.ThrowInternal(err, "API-987g%", "unable to unmarshal api config")
}
return e, nil
}
type APIConfigSecretHashUpdatedEvent struct {
*eventstore.BaseEvent `json:"-"`

View File

@ -35,8 +35,6 @@ func init() {
eventstore.RegisterFilterEventMapper(AggregateType, OIDCConfigAddedType, OIDCConfigAddedEventMapper)
eventstore.RegisterFilterEventMapper(AggregateType, OIDCConfigChangedType, OIDCConfigChangedEventMapper)
eventstore.RegisterFilterEventMapper(AggregateType, OIDCConfigSecretChangedType, OIDCConfigSecretChangedEventMapper)
eventstore.RegisterFilterEventMapper(AggregateType, OIDCClientSecretCheckSucceededType, OIDCConfigSecretCheckSucceededEventMapper)
eventstore.RegisterFilterEventMapper(AggregateType, OIDCClientSecretCheckFailedType, OIDCConfigSecretCheckFailedEventMapper)
eventstore.RegisterFilterEventMapper(AggregateType, OIDCConfigSecretHashUpdatedType, eventstore.GenericEventMapper[OIDCConfigSecretHashUpdatedEvent])
eventstore.RegisterFilterEventMapper(AggregateType, APIConfigAddedType, APIConfigAddedEventMapper)
eventstore.RegisterFilterEventMapper(AggregateType, APIConfigChangedType, APIConfigChangedEventMapper)

View File

@ -11,12 +11,10 @@ import (
)
const (
OIDCConfigAddedType = applicationEventTypePrefix + "config.oidc.added"
OIDCConfigChangedType = applicationEventTypePrefix + "config.oidc.changed"
OIDCConfigSecretChangedType = applicationEventTypePrefix + "config.oidc.secret.changed"
OIDCClientSecretCheckSucceededType = applicationEventTypePrefix + "oidc.secret.check.succeeded"
OIDCClientSecretCheckFailedType = applicationEventTypePrefix + "oidc.secret.check.failed"
OIDCConfigSecretHashUpdatedType = applicationEventTypePrefix + "config.oidc.secret.updated"
OIDCConfigAddedType = applicationEventTypePrefix + "config.oidc.added"
OIDCConfigChangedType = applicationEventTypePrefix + "config.oidc.changed"
OIDCConfigSecretChangedType = applicationEventTypePrefix + "config.oidc.secret.changed"
OIDCConfigSecretHashUpdatedType = applicationEventTypePrefix + "config.oidc.secret.updated"
)
type OIDCConfigAddedEvent struct {
@ -409,90 +407,6 @@ func OIDCConfigSecretChangedEventMapper(event eventstore.Event) (eventstore.Even
return e, nil
}
type OIDCConfigSecretCheckSucceededEvent struct {
eventstore.BaseEvent `json:"-"`
AppID string `json:"appId"`
}
func (e *OIDCConfigSecretCheckSucceededEvent) Payload() interface{} {
return e
}
func (e *OIDCConfigSecretCheckSucceededEvent) UniqueConstraints() []*eventstore.UniqueConstraint {
return nil
}
func NewOIDCConfigSecretCheckSucceededEvent(
ctx context.Context,
aggregate *eventstore.Aggregate,
appID string,
) *OIDCConfigSecretCheckSucceededEvent {
return &OIDCConfigSecretCheckSucceededEvent{
BaseEvent: *eventstore.NewBaseEventForPush(
ctx,
aggregate,
OIDCClientSecretCheckSucceededType,
),
AppID: appID,
}
}
func OIDCConfigSecretCheckSucceededEventMapper(event eventstore.Event) (eventstore.Event, error) {
e := &OIDCConfigSecretCheckSucceededEvent{
BaseEvent: *eventstore.BaseEventFromRepo(event),
}
err := event.Unmarshal(e)
if err != nil {
return nil, zerrors.ThrowInternal(err, "OIDC-837gV", "unable to unmarshal oidc config")
}
return e, nil
}
type OIDCConfigSecretCheckFailedEvent struct {
eventstore.BaseEvent `json:"-"`
AppID string `json:"appId"`
}
func (e *OIDCConfigSecretCheckFailedEvent) Payload() interface{} {
return e
}
func (e *OIDCConfigSecretCheckFailedEvent) UniqueConstraints() []*eventstore.UniqueConstraint {
return nil
}
func NewOIDCConfigSecretCheckFailedEvent(
ctx context.Context,
aggregate *eventstore.Aggregate,
appID string,
) *OIDCConfigSecretCheckFailedEvent {
return &OIDCConfigSecretCheckFailedEvent{
BaseEvent: *eventstore.NewBaseEventForPush(
ctx,
aggregate,
OIDCClientSecretCheckFailedType,
),
AppID: appID,
}
}
func OIDCConfigSecretCheckFailedEventMapper(event eventstore.Event) (eventstore.Event, error) {
e := &OIDCConfigSecretCheckFailedEvent{
BaseEvent: *eventstore.BaseEventFromRepo(event),
}
err := event.Unmarshal(e)
if err != nil {
return nil, zerrors.ThrowInternal(err, "OIDC-987g%", "unable to unmarshal oidc config")
}
return e, nil
}
type OIDCConfigSecretHashUpdatedEvent struct {
*eventstore.BaseEvent `json:"-"`