perf(oidc): disable push of user token meta-event (#8691)

# Which Problems Are Solved

When executing many concurrent authentication requests on a single
machine user, there were performance issues. As the same aggregate is
being searched and written to concurrently, we traced it down to a
locking issue on the used index.
We already optimized the token endpoint by creating a separate OIDC
aggregate.

At the time we decided to push a single event to the user aggregate, for
the user audit log. See [technical advisory
10010](https://zitadel.com/docs/support/advisory/a10010) for more
details.

However, a recent security fix introduced an additional search query on
the user aggregate, causing the locking issue we found.

# How the Problems Are Solved

Add a feature flag which disables pushing of the `user.token.v2.added`.
The event has no importance and was only added for informational
purposes on the user objects. The `oidc_session.access_token.added` is
the actual payload event and is pushed on the OIDC session aggregate and
can still be used for audit trail.

# Additional Changes

- Fix an event mapper type for
`SystemOIDCSingleV1SessionTerminationEventType`

# Additional Context

- Reported by support request
- https://github.com/zitadel/zitadel/pull/7822 changed the token
aggregate
- https://github.com/zitadel/zitadel/pull/8631 introduced user state
check

Load test trace graph with `user.token.v2.added` **enabled**. Query
times are steadily increasing:


![image](https://github.com/user-attachments/assets/4aa25055-8721-4e93-b695-625560979909)

Load test trace graph with `user.token.v2.added` **disabled**. Query
times constant:


![image](https://github.com/user-attachments/assets/a7657f6c-0c55-401b-8291-453da5d5caf9)

---------

Co-authored-by: Livio Spring <livio.a@gmail.com>
This commit is contained in:
Tim Möhlmann
2024-09-26 15:55:41 +02:00
committed by GitHub
parent 7247f62006
commit 63d733b3a2
20 changed files with 334 additions and 14 deletions

View File

@@ -26,6 +26,7 @@ type InstanceFeatures struct {
WebKey *bool
DebugOIDCParentError *bool
OIDCSingleV1SessionTermination *bool
DisableUserTokenEvent *bool
}
func (m *InstanceFeatures) isEmpty() bool {
@@ -39,7 +40,8 @@ func (m *InstanceFeatures) isEmpty() bool {
m.ImprovedPerformance == nil &&
m.WebKey == nil &&
m.DebugOIDCParentError == nil &&
m.OIDCSingleV1SessionTermination == nil
m.OIDCSingleV1SessionTermination == nil &&
m.DisableUserTokenEvent == nil
}
func (c *Commands) SetInstanceFeatures(ctx context.Context, f *InstanceFeatures) (*domain.ObjectDetails, error) {

View File

@@ -70,6 +70,7 @@ func (m *InstanceFeaturesWriteModel) Query() *eventstore.SearchQueryBuilder {
feature_v2.InstanceWebKeyEventType,
feature_v2.InstanceDebugOIDCParentErrorEventType,
feature_v2.InstanceOIDCSingleV1SessionTerminationEventType,
feature_v2.InstanceDisableUserTokenEvent,
).
Builder().ResourceOwner(m.ResourceOwner)
}
@@ -112,6 +113,9 @@ func reduceInstanceFeature(features *InstanceFeatures, key feature.Key, value an
case feature.KeyOIDCSingleV1SessionTermination:
v := value.(bool)
features.OIDCSingleV1SessionTermination = &v
case feature.KeyDisableUserTokenEvent:
v := value.(bool)
features.DisableUserTokenEvent = &v
}
}
@@ -128,5 +132,6 @@ func (wm *InstanceFeaturesWriteModel) setCommands(ctx context.Context, f *Instan
cmds = appendFeatureUpdate(ctx, cmds, aggregate, wm.WebKey, f.WebKey, feature_v2.InstanceWebKeyEventType)
cmds = appendFeatureUpdate(ctx, cmds, aggregate, wm.DebugOIDCParentError, f.DebugOIDCParentError, feature_v2.InstanceDebugOIDCParentErrorEventType)
cmds = appendFeatureUpdate(ctx, cmds, aggregate, wm.OIDCSingleV1SessionTermination, f.OIDCSingleV1SessionTermination, feature_v2.InstanceOIDCSingleV1SessionTerminationEventType)
cmds = appendFeatureUpdate(ctx, cmds, aggregate, wm.DisableUserTokenEvent, f.DisableUserTokenEvent, feature_v2.InstanceDisableUserTokenEvent)
return cmds
}

View File

@@ -423,10 +423,10 @@ func (c *OIDCSessionEvents) AddAccessToken(ctx context.Context, scope []string,
return err
}
c.accessTokenID = AccessTokenPrefix + accessTokenID
c.events = append(c.events,
oidcsession.NewAccessTokenAddedEvent(ctx, c.oidcSessionWriteModel.aggregate, c.accessTokenID, scope, c.accessTokenLifetime, reason, actor),
user.NewUserTokenV2AddedEvent(ctx, &user.NewAggregate(userID, resourceOwner).Aggregate, c.accessTokenID), // for user audit log
)
c.events = append(c.events, oidcsession.NewAccessTokenAddedEvent(ctx, c.oidcSessionWriteModel.aggregate, c.accessTokenID, scope, c.accessTokenLifetime, reason, actor))
if !authz.GetFeatures(ctx).DisableUserTokenEvent {
c.events = append(c.events, user.NewUserTokenV2AddedEvent(ctx, &user.NewAggregate(userID, resourceOwner).Aggregate, c.accessTokenID))
}
return nil
}

View File

@@ -18,6 +18,7 @@ import (
"github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/eventstore"
"github.com/zitadel/zitadel/internal/feature"
"github.com/zitadel/zitadel/internal/id"
"github.com/zitadel/zitadel/internal/id/mock"
"github.com/zitadel/zitadel/internal/repository/authrequest"
@@ -436,6 +437,144 @@ func TestCommands_CreateOIDCSessionFromAuthRequest(t *testing.T) {
state: "state",
},
},
{
"disable user token event",
fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
authrequest.NewAddedEvent(context.Background(), &authrequest.NewAggregate("V2_authRequestID", "instanceID").Aggregate,
"loginClient",
"clientID",
"redirectURI",
"state",
"nonce",
[]string{"openid", "offline_access"},
[]string{"audience"},
domain.OIDCResponseTypeCode,
domain.OIDCResponseModeQuery,
&domain.OIDCCodeChallenge{
Challenge: "challenge",
Method: domain.CodeChallengeMethodS256,
},
[]domain.Prompt{domain.PromptNone},
[]string{"en", "de"},
gu.Ptr(time.Duration(0)),
gu.Ptr("loginHint"),
gu.Ptr("hintUserID"),
true,
),
),
eventFromEventPusher(
authrequest.NewCodeAddedEvent(context.Background(), &authrequest.NewAggregate("V2_authRequestID", "instanceID").Aggregate),
),
eventFromEventPusher(
authrequest.NewSessionLinkedEvent(context.Background(), &authrequest.NewAggregate("V2_authRequestID", "instanceID").Aggregate,
"sessionID",
"userID",
testNow,
[]domain.UserAuthMethodType{domain.UserAuthMethodTypePassword},
),
),
),
expectFilter(
eventFromEventPusher(
session.NewAddedEvent(context.Background(),
&session.NewAggregate("sessionID", "instance1").Aggregate,
&domain.UserAgent{
FingerprintID: gu.Ptr("fp1"),
IP: net.ParseIP("1.2.3.4"),
Description: gu.Ptr("firefox"),
Header: http.Header{"foo": []string{"bar"}},
},
),
),
eventFromEventPusher(
session.NewUserCheckedEvent(context.Background(), &session.NewAggregate("sessionID", "instanceID").Aggregate,
"userID", "org1", testNow, &language.Afrikaans),
),
eventFromEventPusher(
session.NewPasswordCheckedEvent(context.Background(), &session.NewAggregate("sessionID", "instanceID").Aggregate,
testNow),
),
),
expectFilter(
user.NewHumanAddedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.Afrikaans,
domain.GenderUnspecified,
"email",
false,
),
),
expectFilter(), // token lifetime
expectPush(
authrequest.NewCodeExchangedEvent(context.Background(), &authrequest.NewAggregate("V2_authRequestID", "instanceID").Aggregate),
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
"userID", "org1", "sessionID", "clientID", []string{"audience"}, []string{"openid", "offline_access"},
[]domain.UserAuthMethodType{domain.UserAuthMethodTypePassword}, testNow, "nonce", &language.Afrikaans,
&domain.UserAgent{
FingerprintID: gu.Ptr("fp1"),
IP: net.ParseIP("1.2.3.4"),
Description: gu.Ptr("firefox"),
Header: http.Header{"foo": []string{"bar"}},
},
),
oidcsession.NewAccessTokenAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
"at_accessTokenID", []string{"openid", "offline_access"}, time.Hour, domain.TokenReasonAuthRequest, nil),
oidcsession.NewRefreshTokenAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
"rt_refreshTokenID", 7*24*time.Hour, 24*time.Hour),
authrequest.NewSucceededEvent(context.Background(), &authrequest.NewAggregate("V2_authRequestID", "instanceID").Aggregate),
),
),
idGenerator: mock.NewIDGeneratorExpectIDs(t, "oidcSessionID", "accessTokenID", "refreshTokenID"),
defaultAccessTokenLifetime: time.Hour,
defaultRefreshTokenLifetime: 7 * 24 * time.Hour,
defaultRefreshTokenIdleLifetime: 24 * time.Hour,
keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args{
ctx: authz.WithFeatures(
authz.WithInstanceID(context.Background(), "instanceID"),
feature.Features{
DisableUserTokenEvent: true,
},
),
authRequestID: "V2_authRequestID",
complianceCheck: mockAuthRequestComplianceChecker(nil),
needRefreshToken: true,
},
res{
session: &OIDCSession{
SessionID: "sessionID",
TokenID: "V2_oidcSessionID-at_accessTokenID",
ClientID: "clientID",
UserID: "userID",
Audience: []string{"audience"},
Expiration: time.Time{}.Add(time.Hour),
Scope: []string{"openid", "offline_access"},
AuthMethods: []domain.UserAuthMethodType{domain.UserAuthMethodTypePassword},
AuthTime: testNow,
Nonce: "nonce",
PreferredLanguage: &language.Afrikaans,
UserAgent: &domain.UserAgent{
FingerprintID: gu.Ptr("fp1"),
IP: net.ParseIP("1.2.3.4"),
Description: gu.Ptr("firefox"),
Header: http.Header{"foo": []string{"bar"}},
},
Reason: domain.TokenReasonAuthRequest,
RefreshToken: "VjJfb2lkY1Nlc3Npb25JRC1ydF9yZWZyZXNoVG9rZW5JRDp1c2VySUQ", //V2_oidcSessionID-rt_refreshTokenID:userID
},
state: "state",
},
},
{
"without ID token only (implicit)",
fields{
@@ -800,6 +939,106 @@ func TestCommands_CreateOIDCSession(t *testing.T) {
},
},
},
{
name: "disable user token event",
fields: fields{
eventstore: expectEventstore(
expectFilter(
user.NewHumanAddedEvent(
context.Background(),
&user.NewAggregate("userID", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.Afrikaans,
domain.GenderUnspecified,
"email",
false,
),
),
expectFilter(), // token lifetime
expectPush(
oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
"userID", "org1", "", "clientID", []string{"audience"}, []string{"openid", "offline_access"},
[]domain.UserAuthMethodType{domain.UserAuthMethodTypePassword}, testNow, "nonce", &language.Afrikaans,
&domain.UserAgent{
FingerprintID: gu.Ptr("fp1"),
IP: net.ParseIP("1.2.3.4"),
Description: gu.Ptr("firefox"),
Header: http.Header{"foo": []string{"bar"}},
},
),
oidcsession.NewAccessTokenAddedEvent(context.Background(),
&oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate,
"at_accessTokenID", []string{"openid", "offline_access"}, time.Hour, domain.TokenReasonAuthRequest,
&domain.TokenActor{
UserID: "user2",
Issuer: "foo.com",
},
),
),
),
idGenerator: mock.NewIDGeneratorExpectIDs(t, "oidcSessionID", "accessTokenID"),
defaultAccessTokenLifetime: time.Hour,
defaultRefreshTokenLifetime: 7 * 24 * time.Hour,
defaultRefreshTokenIdleLifetime: 24 * time.Hour,
keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args: args{
ctx: authz.WithFeatures(
authz.WithInstanceID(context.Background(), "instanceID"),
feature.Features{
DisableUserTokenEvent: true,
},
),
userID: "userID",
resourceOwner: "org1",
clientID: "clientID",
audience: []string{"audience"},
scope: []string{"openid", "offline_access"},
authMethods: []domain.UserAuthMethodType{domain.UserAuthMethodTypePassword},
authTime: testNow,
nonce: "nonce",
preferredLanguage: &language.Afrikaans,
userAgent: &domain.UserAgent{
FingerprintID: gu.Ptr("fp1"),
IP: net.ParseIP("1.2.3.4"),
Description: gu.Ptr("firefox"),
Header: http.Header{"foo": []string{"bar"}},
},
reason: domain.TokenReasonAuthRequest,
actor: &domain.TokenActor{
UserID: "user2",
Issuer: "foo.com",
},
needRefreshToken: false,
},
want: &OIDCSession{
TokenID: "V2_oidcSessionID-at_accessTokenID",
ClientID: "clientID",
UserID: "userID",
Audience: []string{"audience"},
Expiration: time.Time{}.Add(time.Hour),
Scope: []string{"openid", "offline_access"},
AuthMethods: []domain.UserAuthMethodType{domain.UserAuthMethodTypePassword},
AuthTime: testNow,
Nonce: "nonce",
PreferredLanguage: &language.Afrikaans,
UserAgent: &domain.UserAgent{
FingerprintID: gu.Ptr("fp1"),
IP: net.ParseIP("1.2.3.4"),
Description: gu.Ptr("firefox"),
Header: http.Header{"foo": []string{"bar"}},
},
Reason: domain.TokenReasonAuthRequest,
Actor: &domain.TokenActor{
UserID: "user2",
Issuer: "foo.com",
},
},
},
{
name: "with refresh token",
fields: fields{

View File

@@ -18,6 +18,7 @@ type SystemFeatures struct {
Actions *bool
ImprovedPerformance []feature.ImprovedPerformanceType
OIDCSingleV1SessionTermination *bool
DisableUserTokenEvent *bool
}
func (m *SystemFeatures) isEmpty() bool {
@@ -29,7 +30,8 @@ func (m *SystemFeatures) isEmpty() bool {
m.Actions == nil &&
// nil check to allow unset improvements
m.ImprovedPerformance == nil &&
m.OIDCSingleV1SessionTermination == nil
m.OIDCSingleV1SessionTermination == nil &&
m.DisableUserTokenEvent == nil
}
func (c *Commands) SetSystemFeatures(ctx context.Context, f *SystemFeatures) (*domain.ObjectDetails, error) {

View File

@@ -61,6 +61,7 @@ func (m *SystemFeaturesWriteModel) Query() *eventstore.SearchQueryBuilder {
feature_v2.SystemActionsEventType,
feature_v2.SystemImprovedPerformanceEventType,
feature_v2.SystemOIDCSingleV1SessionTerminationEventType,
feature_v2.SystemDisableUserTokenEvent,
).
Builder().ResourceOwner(m.ResourceOwner)
}
@@ -96,6 +97,9 @@ func reduceSystemFeature(features *SystemFeatures, key feature.Key, value any) {
case feature.KeyOIDCSingleV1SessionTermination:
v := value.(bool)
features.OIDCSingleV1SessionTermination = &v
case feature.KeyDisableUserTokenEvent:
v := value.(bool)
features.DisableUserTokenEvent = &v
}
}
@@ -110,6 +114,7 @@ func (wm *SystemFeaturesWriteModel) setCommands(ctx context.Context, f *SystemFe
cmds = appendFeatureUpdate(ctx, cmds, aggregate, wm.Actions, f.Actions, feature_v2.SystemActionsEventType)
cmds = appendFeatureSliceUpdate(ctx, cmds, aggregate, wm.ImprovedPerformance, f.ImprovedPerformance, feature_v2.SystemImprovedPerformanceEventType)
cmds = appendFeatureUpdate(ctx, cmds, aggregate, wm.OIDCSingleV1SessionTermination, f.OIDCSingleV1SessionTermination, feature_v2.SystemOIDCSingleV1SessionTerminationEventType)
cmds = appendFeatureUpdate(ctx, cmds, aggregate, wm.DisableUserTokenEvent, f.DisableUserTokenEvent, feature_v2.SystemDisableUserTokenEvent)
return cmds
}