From 5b40af79f0d74c2d475cb74930c80e768f975bce Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Tue, 17 Sep 2024 15:21:49 +0200 Subject: [PATCH] fix: correctly check user state (#8631) # Which Problems Are Solved ZITADEL's user account deactivation mechanism did not work correctly with service accounts. Deactivated service accounts retained the ability to request tokens, which could lead to unauthorized access to applications and resources. # How the Problems Are Solved Additionally to checking the user state on the session API and login UI, the state is checked on all oidc session methods resulting in a new token or when returning the user information (userinfo, introspection, id_token / access_token and saml attributes) --- internal/api/oidc/client.go | 3 + .../token_client_credentials_test.go | 10 + internal/api/oidc/userinfo.go | 5 +- internal/api/saml/storage.go | 6 + internal/command/device_auth.go | 2 +- internal/command/device_auth_test.go | 115 +++++- internal/command/oidc_session.go | 22 +- internal/command/oidc_session_test.go | 362 +++++++++++++++++- internal/integration/oidc.go | 26 ++ internal/query/userinfo_by_id.sql | 2 +- 10 files changed, 520 insertions(+), 33 deletions(-) diff --git a/internal/api/oidc/client.go b/internal/api/oidc/client.go index 1e6fecda5..08ed8c31b 100644 --- a/internal/api/oidc/client.go +++ b/internal/api/oidc/client.go @@ -330,6 +330,9 @@ func (o *OPStorage) setUserinfo(ctx context.Context, userInfo *oidc.UserInfo, us if err != nil { return err } + if user.State != domain.UserStateActive { + return zerrors.ThrowUnauthenticated(nil, "OIDC-S3tha", "Errors.Users.NotActive") + } var allRoles bool roles := make([]string, 0) for _, scope := range scopes { diff --git a/internal/api/oidc/integration_test/token_client_credentials_test.go b/internal/api/oidc/integration_test/token_client_credentials_test.go index 5316202e3..372baf163 100644 --- a/internal/api/oidc/integration_test/token_client_credentials_test.go +++ b/internal/api/oidc/integration_test/token_client_credentials_test.go @@ -24,6 +24,9 @@ func TestServer_ClientCredentialsExchange(t *testing.T) { machine, name, clientID, clientSecret, err := Instance.CreateOIDCCredentialsClient(CTX) require.NoError(t, err) + _, _, clientIDInactive, clientSecretInactive, err := Instance.CreateOIDCCredentialsClientInactive(CTX) + require.NoError(t, err) + type claims struct { name string username string @@ -71,6 +74,13 @@ func TestServer_ClientCredentialsExchange(t *testing.T) { scope: []string{oidc.ScopeOpenID}, wantErr: true, }, + { + name: "inactive machine user error", + clientID: clientIDInactive, + clientSecret: clientSecretInactive, + scope: []string{oidc.ScopeOpenID}, + wantErr: true, + }, { name: "wrong secret error", clientID: clientID, diff --git a/internal/api/oidc/userinfo.go b/internal/api/oidc/userinfo.go index 542ea6a08..b2121a73a 100644 --- a/internal/api/oidc/userinfo.go +++ b/internal/api/oidc/userinfo.go @@ -66,7 +66,10 @@ func (s *Server) UserInfo(ctx context.Context, r *op.Request[oidc.UserInfoReques false, )(ctx, true, domain.TriggerTypePreUserinfoCreation) if err != nil { - return nil, err + if !zerrors.IsNotFound(err) { + return nil, err + } + return nil, op.NewStatusError(oidc.ErrAccessDenied().WithDescription("no active user").WithParent(err).WithReturnParentToClient(authz.GetFeatures(ctx).DebugOIDCParentError), http.StatusUnauthorized) } return op.NewResponse(userInfo), nil } diff --git a/internal/api/saml/storage.go b/internal/api/saml/storage.go index 8f33e1089..76173c259 100644 --- a/internal/api/saml/storage.go +++ b/internal/api/saml/storage.go @@ -131,6 +131,9 @@ func (p *Storage) SetUserinfoWithUserID(ctx context.Context, applicationID strin if err != nil { return err } + if user.State != domain.UserStateActive { + return zerrors.ThrowPreconditionFailed(nil, "SAML-S3gFd", "Errors.User.NotActive") + } userGrants, err := p.getGrants(ctx, userID, applicationID) if err != nil { @@ -157,6 +160,9 @@ func (p *Storage) SetUserinfoWithLoginName(ctx context.Context, userinfo models. if err != nil { return err } + if user.State != domain.UserStateActive { + return zerrors.ThrowPreconditionFailed(nil, "SAML-FJ262", "Errors.User.NotActive") + } setUserinfo(user, userinfo, attributes, map[string]*customAttribute{}) return nil diff --git a/internal/command/device_auth.go b/internal/command/device_auth.go index 1a151437b..a2754650e 100644 --- a/internal/command/device_auth.go +++ b/internal/command/device_auth.go @@ -144,7 +144,7 @@ func (c *Commands) CreateOIDCSessionFromDeviceAuth(ctx context.Context, deviceCo return nil, DeviceAuthStateError(deviceAuthModel.State) } - cmd, err := c.newOIDCSessionAddEvents(ctx, deviceAuthModel.UserOrgID) + cmd, err := c.newOIDCSessionAddEvents(ctx, deviceAuthModel.UserID, deviceAuthModel.UserOrgID) if err != nil { return nil, err } diff --git a/internal/command/device_auth_test.go b/internal/command/device_auth_test.go index 500c7b8f5..f25be7053 100644 --- a/internal/command/device_auth_test.go +++ b/internal/command/device_auth_test.go @@ -126,7 +126,7 @@ func TestCommands_ApproveDeviceAuth(t *testing.T) { pushErr := errors.New("pushErr") type fields struct { - eventstore *eventstore.Eventstore + eventstore func(*testing.T) *eventstore.Eventstore } type args struct { ctx context.Context @@ -149,7 +149,7 @@ func TestCommands_ApproveDeviceAuth(t *testing.T) { { name: "not found error", fields: fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter(), ), }, @@ -169,7 +169,7 @@ func TestCommands_ApproveDeviceAuth(t *testing.T) { { name: "push error", fields: fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter(eventFromEventPusherWithInstanceID( "instance1", deviceauth.NewAddedEvent( @@ -211,7 +211,7 @@ func TestCommands_ApproveDeviceAuth(t *testing.T) { { name: "success", fields: fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter(eventFromEventPusherWithInstanceID( "instance1", deviceauth.NewAddedEvent( @@ -256,7 +256,7 @@ func TestCommands_ApproveDeviceAuth(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), } gotDetails, err := c.ApproveDeviceAuth(tt.args.ctx, tt.args.id, tt.args.userID, tt.args.userOrgID, tt.args.authMethods, tt.args.authTime, tt.args.preferredLanguage, tt.args.userAgent, tt.args.sessionID) require.ErrorIs(t, err, tt.wantErr) @@ -271,7 +271,7 @@ func TestCommands_CancelDeviceAuth(t *testing.T) { pushErr := errors.New("pushErr") type fields struct { - eventstore *eventstore.Eventstore + eventstore func(*testing.T) *eventstore.Eventstore } type args struct { ctx context.Context @@ -288,7 +288,7 @@ func TestCommands_CancelDeviceAuth(t *testing.T) { { name: "not found error", fields: fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter(), ), }, @@ -298,7 +298,7 @@ func TestCommands_CancelDeviceAuth(t *testing.T) { { name: "push error", fields: fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter(eventFromEventPusherWithInstanceID( "instance1", deviceauth.NewAddedEvent( @@ -323,7 +323,7 @@ func TestCommands_CancelDeviceAuth(t *testing.T) { { name: "success/denied", fields: fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter(eventFromEventPusherWithInstanceID( "instance1", deviceauth.NewAddedEvent( @@ -350,7 +350,7 @@ func TestCommands_CancelDeviceAuth(t *testing.T) { { name: "success/expired", fields: fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter(eventFromEventPusherWithInstanceID( "instance1", deviceauth.NewAddedEvent( @@ -378,7 +378,7 @@ func TestCommands_CancelDeviceAuth(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), } gotDetails, err := c.CancelDeviceAuth(tt.args.ctx, tt.args.id, tt.args.reason) require.ErrorIs(t, err, tt.wantErr) @@ -586,6 +586,69 @@ func TestCommands_CreateOIDCSessionFromDeviceAuth(t *testing.T) { }, wantErr: DeviceAuthStateError(domain.DeviceAuthStateDone), }, + { + name: "user not active", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusherWithInstanceID( + "instance1", + deviceauth.NewAddedEvent( + ctx, + deviceauth.NewAggregate("123", "instance1"), + "clientID", "123", "456", time.Now().Add(-time.Minute), + []string{"openid", "offline_access"}, + []string{"audience"}, false, + ), + ), + eventFromEventPusherWithInstanceID( + "instance1", + deviceauth.NewApprovedEvent(ctx, + deviceauth.NewAggregate("123", "instance1"), + "userID", "org1", + []domain.UserAuthMethodType{domain.UserAuthMethodTypePassword}, + testNow, &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"}}, + }, + "sessionID", + ), + ), + ), + expectFilter( + user.NewHumanAddedEvent( + ctx, + &user.NewAggregate("userID", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.English, + domain.GenderUnspecified, + "email", + false, + ), + user.NewUserDeactivatedEvent( + ctx, + &user.NewAggregate("userID", "org1").Aggregate, + ), + ), + ), + idGenerator: mock.NewIDGeneratorExpectIDs(t), + defaultAccessTokenLifetime: time.Hour, + defaultRefreshTokenLifetime: 7 * 24 * time.Hour, + defaultRefreshTokenIdleLifetime: 24 * time.Hour, + keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + args: args{ + ctx, + "123", + }, + wantErr: zerrors.ThrowPreconditionFailed(nil, "OIDCS-kj3g2", "Errors.User.NotActive"), + }, { name: "approved, success", fields: fields{ @@ -617,6 +680,21 @@ func TestCommands_CreateOIDCSessionFromDeviceAuth(t *testing.T) { ), ), ), + expectFilter( + user.NewHumanAddedEvent( + ctx, + &user.NewAggregate("userID", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.English, + domain.GenderUnspecified, + "email", + false, + ), + ), expectFilter(), // token lifetime expectPush( oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, @@ -699,6 +777,21 @@ func TestCommands_CreateOIDCSessionFromDeviceAuth(t *testing.T) { ), ), ), + expectFilter( + user.NewHumanAddedEvent( + ctx, + &user.NewAggregate("userID", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.English, + domain.GenderUnspecified, + "email", + false, + ), + ), expectFilter(), // token lifetime expectPush( oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, diff --git a/internal/command/oidc_session.go b/internal/command/oidc_session.go index ae201feb7..1ad46ba7d 100644 --- a/internal/command/oidc_session.go +++ b/internal/command/oidc_session.go @@ -80,7 +80,7 @@ func (c *Commands) CreateOIDCSessionFromAuthRequest(ctx context.Context, authReq return nil, "", err } - cmd, err := c.newOIDCSessionAddEvents(ctx, sessionModel.UserResourceOwner) + cmd, err := c.newOIDCSessionAddEvents(ctx, sessionModel.UserID, sessionModel.UserResourceOwner) if err != nil { return nil, "", err } @@ -141,7 +141,7 @@ func (c *Commands) CreateOIDCSession(ctx context.Context, ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() - cmd, err := c.newOIDCSessionAddEvents(ctx, resourceOwner) + cmd, err := c.newOIDCSessionAddEvents(ctx, userID, resourceOwner) if err != nil { return nil, err } @@ -265,7 +265,14 @@ func (c *Commands) RevokeOIDCSessionToken(ctx context.Context, token, clientID s return c.pushAppendAndReduce(ctx, writeModel, oidcsession.NewAccessTokenRevokedEvent(ctx, writeModel.aggregate)) } -func (c *Commands) newOIDCSessionAddEvents(ctx context.Context, resourceOwner string, pending ...eventstore.Command) (*OIDCSessionEvents, error) { +func (c *Commands) newOIDCSessionAddEvents(ctx context.Context, userID, resourceOwner string, pending ...eventstore.Command) (*OIDCSessionEvents, error) { + userStateModel, err := c.userStateWriteModel(ctx, userID) + if err != nil { + return nil, err + } + if !userStateModel.UserState.IsEnabled() { + return nil, zerrors.ThrowPreconditionFailed(nil, "OIDCS-kj3g2", "Errors.User.NotActive") + } accessTokenLifetime, refreshTokenLifeTime, refreshTokenIdleLifetime, err := c.tokenTokenLifetimes(ctx) if err != nil { return nil, err @@ -281,6 +288,7 @@ func (c *Commands) newOIDCSessionAddEvents(ctx context.Context, resourceOwner st encryptionAlg: c.keyAlgorithm, events: pending, oidcSessionWriteModel: NewOIDCSessionWriteModel(sessionID, resourceOwner), + userStateModel: userStateModel, accessTokenLifetime: accessTokenLifetime, refreshTokenLifeTime: refreshTokenLifeTime, refreshTokenIdleLifetime: refreshTokenIdleLifetime, @@ -321,6 +329,13 @@ func (c *Commands) newOIDCSessionUpdateEvents(ctx context.Context, refreshToken if err = sessionWriteModel.CheckRefreshToken(refreshTokenID); err != nil { return nil, err } + userStateWriteModel, err := c.userStateWriteModel(ctx, sessionWriteModel.UserID) + if err != nil { + return nil, err + } + if !userStateWriteModel.UserState.IsEnabled() { + return nil, zerrors.ThrowPreconditionFailed(nil, "OIDCS-J39h2", "Errors.User.NotActive") + } accessTokenLifetime, refreshTokenLifeTime, refreshTokenIdleLifetime, err := c.tokenTokenLifetimes(ctx) if err != nil { return nil, err @@ -342,6 +357,7 @@ type OIDCSessionEvents struct { encryptionAlg crypto.EncryptionAlgorithm events []eventstore.Command oidcSessionWriteModel *OIDCSessionWriteModel + userStateModel *UserV2WriteModel accessTokenLifetime time.Duration refreshTokenLifeTime time.Duration diff --git a/internal/command/oidc_session_test.go b/internal/command/oidc_session_test.go index a3af86d25..6d9ee6e32 100644 --- a/internal/command/oidc_session_test.go +++ b/internal/command/oidc_session_test.go @@ -205,6 +205,103 @@ func TestCommands_CreateOIDCSessionFromAuthRequest(t *testing.T) { err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-Flk38", "Errors.Session.NotExisting"), }, }, + { + "user not active", + 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, + ), + user.NewUserDeactivatedEvent( + context.Background(), + &user.NewAggregate("userID", "org1").Aggregate, + ), + ), + ), + idGenerator: mock.NewIDGeneratorExpectIDs(t), + defaultAccessTokenLifetime: time.Hour, + defaultRefreshTokenLifetime: 7 * 24 * time.Hour, + defaultRefreshTokenIdleLifetime: 24 * time.Hour, + keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + args{ + ctx: authz.WithInstanceID(context.Background(), "instanceID"), + authRequestID: "V2_authRequestID", + complianceCheck: mockAuthRequestComplianceChecker(nil), + needRefreshToken: true, + }, + res{ + err: zerrors.ThrowPreconditionFailed(nil, "OIDCS-kj3g2", "Errors.User.NotActive"), + }, + }, { "add successful", fields{ @@ -266,6 +363,21 @@ func TestCommands_CreateOIDCSessionFromAuthRequest(t *testing.T) { 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), @@ -382,6 +494,21 @@ func TestCommands_CreateOIDCSessionFromAuthRequest(t *testing.T) { 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( oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, @@ -521,10 +648,81 @@ func TestCommands_CreateOIDCSession(t *testing.T) { }, wantErr: io.ErrClosedPipe, }, + { + name: "not active user", + 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, + ), + user.NewUserDeactivatedEvent( + context.Background(), + &user.NewAggregate("userID", "org1").Aggregate, + ), + ), + ), + idGenerator: mock.NewIDGeneratorExpectIDs(t), + defaultAccessTokenLifetime: time.Hour, + defaultRefreshTokenLifetime: 7 * 24 * time.Hour, + defaultRefreshTokenIdleLifetime: 24 * time.Hour, + keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + args: args{ + ctx: context.Background(), + 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, + }, + wantErr: zerrors.ThrowPreconditionFailed(nil, "OIDCS-kj3g2", "Errors.User.NotActive"), + }, { name: "without refresh token", 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, @@ -606,6 +804,21 @@ func TestCommands_CreateOIDCSession(t *testing.T) { name: "with refresh token", 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, @@ -689,6 +902,21 @@ func TestCommands_CreateOIDCSession(t *testing.T) { name: "with sessionID", 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, @@ -772,6 +1000,21 @@ func TestCommands_CreateOIDCSession(t *testing.T) { name: "impersonation not allowed", 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 ), idGenerator: mock.NewIDGeneratorExpectIDs(t, "oidcSessionID"), @@ -813,6 +1056,21 @@ func TestCommands_CreateOIDCSession(t *testing.T) { name: "impersonation allowed", 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( user.NewUserImpersonatedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, "clientID", &domain.TokenActor{ @@ -1067,6 +1325,63 @@ func TestCommands_ExchangeOIDCSessionRefreshAndAccessToken(t *testing.T) { err: zerrors.ThrowPreconditionFailed(nil, "OIDCS-3jt2w", "Errors.OIDCSession.RefreshTokenInvalid"), }, }, + { + "user not active", + fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusherWithCreationDateNow( + oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, + "userID", "org1", "sessionID", "clientID", []string{"audience"}, []string{"openid", "profile", "offline_access"}, + []domain.UserAuthMethodType{domain.UserAuthMethodTypePassword}, testNow, "nonce", &language.Afrikaans, + &domain.UserAgent{FingerprintID: gu.Ptr("browserFP")}, + ), + ), + eventFromEventPusherWithCreationDateNow( + oidcsession.NewAccessTokenAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, + "at_accessTokenID", []string{"openid", "profile", "offline_access"}, time.Hour, domain.TokenReasonAuthRequest, nil), + ), + eventFromEventPusherWithCreationDateNow( + oidcsession.NewRefreshTokenAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, + "rt_refreshTokenID", 7*24*time.Hour, 24*time.Hour), + ), + ), + expectFilter( + user.NewHumanAddedEvent( + context.Background(), + &user.NewAggregate("userID", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.Afrikaans, + domain.GenderUnspecified, + "email", + false, + ), + user.NewUserDeactivatedEvent( + context.Background(), + &user.NewAggregate("userID", "org1").Aggregate, + ), + ), + ), + idGenerator: mock.NewIDGeneratorExpectIDs(t), + defaultAccessTokenLifetime: time.Hour, + defaultRefreshTokenLifetime: 7 * 24 * time.Hour, + defaultRefreshTokenIdleLifetime: 24 * time.Hour, + keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + args{ + ctx: authz.WithInstanceID(context.Background(), "instanceID"), + refreshToken: "VjJfb2lkY1Nlc3Npb25JRC1ydF9yZWZyZXNoVG9rZW5JRDp1c2VySUQ", //V2_oidcSessionID:rt_refreshTokenID:userID + scope: []string{"openid", "offline_access"}, + complianceCheck: mockRefreshTokenComplianceChecker(nil), + }, + res{ + err: zerrors.ThrowPreconditionFailed(nil, "OIDCS-J39h2", "Errors.User.NotActive"), + }, + }, { "refresh successful", fields{ @@ -1088,6 +1403,21 @@ func TestCommands_ExchangeOIDCSessionRefreshAndAccessToken(t *testing.T) { "rt_refreshTokenID", 7*24*time.Hour, 24*time.Hour), ), ), + 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.NewAccessTokenAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, @@ -1153,7 +1483,7 @@ func TestCommands_ExchangeOIDCSessionRefreshAndAccessToken(t *testing.T) { func TestCommands_OIDCSessionByRefreshToken(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(*testing.T) *eventstore.Eventstore idGenerator id.Generator defaultAccessTokenLifetime time.Duration defaultRefreshTokenLifetime time.Duration @@ -1177,7 +1507,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) { { "invalid refresh token format error", fields{ - eventstore: eventstoreExpect(t), + eventstore: expectEventstore(), keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), }, args{ @@ -1191,7 +1521,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) { { "inactive session error", fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter(), ), keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), @@ -1207,7 +1537,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) { { "invalid refresh token error", fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, @@ -1235,7 +1565,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) { { "expired refresh token error", fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, @@ -1267,7 +1597,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) { { "get successful", fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter( eventFromEventPusherWithCreationDateNow( oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, @@ -1316,7 +1646,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), idGenerator: tt.fields.idGenerator, defaultAccessTokenLifetime: tt.fields.defaultAccessTokenLifetime, defaultRefreshTokenLifetime: tt.fields.defaultRefreshTokenLifetime, @@ -1348,7 +1678,7 @@ func TestCommands_OIDCSessionByRefreshToken(t *testing.T) { func TestCommands_RevokeOIDCSessionToken(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(*testing.T) *eventstore.Eventstore keyAlgorithm crypto.EncryptionAlgorithm } type args struct { @@ -1368,7 +1698,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) { { "invalid token", fields{ - eventstore: eventstoreExpect(t), + eventstore: expectEventstore(), keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), }, args{ @@ -1382,7 +1712,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) { { "refresh_token inactive", fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, @@ -1407,7 +1737,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) { { "refresh_token invalid client", fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, @@ -1432,7 +1762,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) { { "refresh_token revoked", fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, @@ -1468,7 +1798,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) { { "access_token inactive session", fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, @@ -1493,7 +1823,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) { { "access_token invalid client", fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, @@ -1518,7 +1848,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) { { "access_token revoked", fields{ - eventstore: eventstoreExpect(t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( oidcsession.NewAddedEvent(context.Background(), &oidcsession.NewAggregate("V2_oidcSessionID", "org1").Aggregate, @@ -1555,7 +1885,7 @@ func TestCommands_RevokeOIDCSessionToken(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), keyAlgorithm: tt.fields.keyAlgorithm, } err := c.RevokeOIDCSessionToken(tt.args.ctx, tt.args.token, tt.args.clientID) diff --git a/internal/integration/oidc.go b/internal/integration/oidc.go index 3afd262a3..f6d779de9 100644 --- a/internal/integration/oidc.go +++ b/internal/integration/oidc.go @@ -22,6 +22,7 @@ import ( "github.com/zitadel/zitadel/pkg/grpc/authn" "github.com/zitadel/zitadel/pkg/grpc/management" "github.com/zitadel/zitadel/pkg/grpc/user" + user_v2 "github.com/zitadel/zitadel/pkg/grpc/user/v2" ) func (i *Instance) CreateOIDCClient(ctx context.Context, redirectURI, logoutRedirectURI, projectID string, appType app.OIDCAppType, authMethod app.OIDCAuthMethodType, devMode bool, grantTypes ...app.OIDCGrantType) (*management.AddOIDCAppResponse, error) { @@ -355,6 +356,31 @@ func (i *Instance) CreateOIDCCredentialsClient(ctx context.Context) (machine *ma return machine, name, secret.GetClientId(), secret.GetClientSecret(), nil } +func (i *Instance) CreateOIDCCredentialsClientInactive(ctx context.Context) (machine *management.AddMachineUserResponse, name, clientID, clientSecret string, err error) { + name = gofakeit.Username() + machine, err = i.Client.Mgmt.AddMachineUser(ctx, &management.AddMachineUserRequest{ + Name: name, + UserName: name, + AccessTokenType: user.AccessTokenType_ACCESS_TOKEN_TYPE_JWT, + }) + if err != nil { + return nil, "", "", "", err + } + secret, err := i.Client.Mgmt.GenerateMachineSecret(ctx, &management.GenerateMachineSecretRequest{ + UserId: machine.GetUserId(), + }) + if err != nil { + return nil, "", "", "", err + } + _, err = i.Client.UserV2.DeactivateUser(ctx, &user_v2.DeactivateUserRequest{ + UserId: machine.GetUserId(), + }) + if err != nil { + return nil, "", "", "", err + } + return machine, name, secret.GetClientId(), secret.GetClientSecret(), nil +} + func (i *Instance) CreateOIDCJWTProfileClient(ctx context.Context) (machine *management.AddMachineUserResponse, name string, keyData []byte, err error) { name = gofakeit.Username() machine, err = i.Client.Mgmt.AddMachineUser(ctx, &management.AddMachineUserRequest{ diff --git a/internal/query/userinfo_by_id.sql b/internal/query/userinfo_by_id.sql index 93db8ab1b..cd7f301c7 100644 --- a/internal/query/userinfo_by_id.sql +++ b/internal/query/userinfo_by_id.sql @@ -2,7 +2,7 @@ with usr as ( select u.id, u.creation_date, u.change_date, u.sequence, u.state, u.resource_owner, u.username, n.login_name as preferred_login_name from projections.users13 u left join projections.login_names3 n on u.id = n.user_id and u.instance_id = n.instance_id - where u.id = $1 + where u.id = $1 and u.state = 1 -- only allow active users and u.instance_id = $2 and n.is_primary = true ),