diff --git a/backend/v3/domain/session_check_idp_intent.go b/backend/v3/domain/session_check_idp_intent.go index bac694c15..762a003cf 100644 --- a/backend/v3/domain/session_check_idp_intent.go +++ b/backend/v3/domain/session_check_idp_intent.go @@ -24,8 +24,9 @@ type IDPIntentCheckCommand struct { InstanceID string EncAlgo crypto.EncryptionAlgorithm - FetchedUser User - IsCheckComplete bool + FetchedUser User + IsCheckComplete bool + IntentLastVerified time.Time } // NewIDPIntentCheckCommand returns an IDPIntentCheckCommand initialized with the input values. @@ -53,7 +54,7 @@ func (i *IDPIntentCheckCommand) Events(ctx context.Context, opts *InvokeOpts) ([ } return []eventstore.Command{ - session.NewIntentCheckedEvent(ctx, &session.NewAggregate(i.SessionID, i.InstanceID).Aggregate, time.Now()), + session.NewIntentCheckedEvent(ctx, &session.NewAggregate(i.SessionID, i.InstanceID).Aggregate, i.IntentLastVerified), idpintent.NewConsumedEvent(ctx, &idpintent.NewAggregate(i.CheckIntent.ID, "").Aggregate), }, nil } @@ -65,15 +66,54 @@ func (i *IDPIntentCheckCommand) Execute(ctx context.Context, opts *InvokeOpts) ( } intentRepo := opts.idpIntentRepo + sessionRepo := opts.sessionRepo + + beginner, ok := opts.DB().(database.Beginner) + if !ok { + return zerrors.ThrowInternal(nil, "DOM-jiuIbh", "database doesn't implement database.Beginner") + } + + tx, txErr := beginner.Begin(ctx, nil) + if txErr != nil { + return zerrors.ThrowInternal(txErr, "DOM-Msuwn2", "failed starting transaction") + } + + defer func() { + if endErr := tx.End(ctx, txErr); endErr != nil { + err = endErr + } + }() removedRows, err := intentRepo.Delete(ctx, opts.DB(), intentRepo.PrimaryKeyCondition(i.InstanceID, i.CheckIntent.ID)) if err != nil { - return zerrors.ThrowInternal(err, "DOM-j1s5Eu", "failed deleting IDP intent") + txErr = zerrors.ThrowInternal(err, "DOM-j1s5Eu", "failed deleting IDP intent") + return txErr } if removedRows != 1 { - return zerrors.ThrowInternal(NewRowsReturnedMismatchError(1, removedRows), "DOM-3CBpdB", "unexpected number of rows deleted") + txErr = zerrors.ThrowInternal(NewRowsReturnedMismatchError(1, removedRows), "DOM-3CBpdB", "unexpected number of rows deleted") + return txErr } + idpFactor := &SessionFactorIdentityProviderIntent{LastVerifiedAt: time.Now()} + updateCount, updateErr := sessionRepo.Update(ctx, opts.DB(), + sessionRepo.PrimaryKeyCondition(i.InstanceID, i.SessionID), + sessionRepo.SetFactor(idpFactor), + ) + if updateErr != nil { + txErr = zerrors.ThrowInternal(updateErr, "DOM-pec0al", "failed updating session") + return txErr + } + + if updateCount == 0 { + txErr = zerrors.ThrowNotFound(nil, "DOM-CopO4e", "session not found") + return txErr + } + if updateCount > 1 { + txErr = zerrors.ThrowInternal(NewMultipleObjectsUpdatedError(1, updateCount), "DOM-mlbibw", "unexpected number of rows updated") + return txErr + } + + i.IntentLastVerified = idpFactor.LastVerifiedAt i.IsCheckComplete = true return nil diff --git a/backend/v3/domain/session_check_idp_intent_test.go b/backend/v3/domain/session_check_idp_intent_test.go index 5ab9f4ff2..8df83091e 100644 --- a/backend/v3/domain/session_check_idp_intent_test.go +++ b/backend/v3/domain/session_check_idp_intent_test.go @@ -1,7 +1,6 @@ package domain_test import ( - "context" "encoding/base64" "errors" "testing" @@ -48,10 +47,11 @@ func TestIDPIntentCheckCommand_Events(t *testing.T) { { name: "valid command returns events", command: &domain.IDPIntentCheckCommand{ - CheckIntent: &domain.CheckIDPIntentType{ID: "intent-123"}, - SessionID: "session-456", - InstanceID: "instance-789", - IsCheckComplete: true, + CheckIntent: &domain.CheckIDPIntentType{ID: "intent-123"}, + SessionID: "session-456", + InstanceID: "instance-789", + IsCheckComplete: true, + IntentLastVerified: time.Now(), }, expectedEvents: []eventstore.Command{ @@ -65,7 +65,7 @@ func TestIDPIntentCheckCommand_Events(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() // Test - events, err := tc.command.Events(context.Background(), &domain.InvokeOpts{}) + events, err := tc.command.Events(t.Context(), &domain.InvokeOpts{}) // Verify assert.NoError(t, err) @@ -73,12 +73,12 @@ func TestIDPIntentCheckCommand_Events(t *testing.T) { require.Len(t, events, len(tc.expectedEvents)) for i, expectedType := range tc.expectedEvents { - assert.IsType(t, expectedType, events[i]) - switch expectedAssertedType := expectedType.(type) { + require.IsType(t, expectedType, events[i]) + switch expectedType.(type) { case *session.IntentCheckedEvent: actualAssertedType, ok := events[i].(*session.IntentCheckedEvent) require.True(t, ok) - assert.InDelta(t, expectedAssertedType.CheckedAt.UnixMilli(), actualAssertedType.CheckedAt.UnixMilli(), 10) + assert.Equal(t, tc.command.IntentLastVerified, actualAssertedType.CheckedAt) case *idpintent.ConsumedEvent: _, ok := events[i].(*idpintent.ConsumedEvent) require.True(t, ok) @@ -739,10 +739,12 @@ func TestIDPIntentCheckCommand_Validate(t *testing.T) { func TestIDPIntentCheckCommand_Execute(t *testing.T) { t.Parallel() deleteErr := errors.New("delete error") + updateErr := errors.New("update error") tt := []struct { testName string idpIntentRepo func(ctrl *gomock.Controller) domain.IDPIntentRepository + sessionRepo func(ctrl *gomock.Controller) domain.SessionRepository cmd *domain.IDPIntentCheckCommand expectedError error expectComplete bool @@ -780,7 +782,7 @@ func TestIDPIntentCheckCommand_Execute(t *testing.T) { expectComplete: false, }, { - testName: "when intent is successfully deleted should mark complete", + testName: "when session update fails should return internal error", cmd: domain.NewIDPIntentCheckCommand(&domain.CheckIDPIntentType{ID: "intent-123"}, "session-1", "instance-1", nil), idpIntentRepo: func(ctrl *gomock.Controller) domain.IDPIntentRepository { repo := domainmock.NewIDPIntentRepo(ctrl) @@ -789,14 +791,81 @@ func TestIDPIntentCheckCommand_Execute(t *testing.T) { Return(int64(1), nil) return repo }, - expectedError: nil, + sessionRepo: func(ctrl *gomock.Controller) domain.SessionRepository { + repo := domainmock.NewSessionRepo(ctrl) + repo.EXPECT(). + Update(gomock.Any(), gomock.Any(), repo.PrimaryKeyCondition("instance-1", "session-1"), repo.SetFactor(&domain.SessionFactorIdentityProviderIntent{LastVerifiedAt: time.Now()})). + Times(1). + Return(int64(0), updateErr) + return repo + }, + expectedError: zerrors.ThrowInternal(updateErr, "DOM-pec0al", "failed updating session"), + }, + { + testName: "when session update returns no rows should return not found error", + cmd: domain.NewIDPIntentCheckCommand(&domain.CheckIDPIntentType{ID: "intent-123"}, "session-1", "instance-1", nil), + idpIntentRepo: func(ctrl *gomock.Controller) domain.IDPIntentRepository { + repo := domainmock.NewIDPIntentRepo(ctrl) + repo.EXPECT(). + Delete(gomock.Any(), gomock.Any(), repo.PrimaryKeyCondition("instance-1", "intent-123")). + Return(int64(1), nil) + return repo + }, + sessionRepo: func(ctrl *gomock.Controller) domain.SessionRepository { + repo := domainmock.NewSessionRepo(ctrl) + repo.EXPECT(). + Update(gomock.Any(), gomock.Any(), repo.PrimaryKeyCondition("instance-1", "session-1"), repo.SetFactor(&domain.SessionFactorIdentityProviderIntent{LastVerifiedAt: time.Now()})). + Times(1). + Return(int64(0), nil) + return repo + }, + expectedError: zerrors.ThrowNotFound(nil, "DOM-CopO4e", "session not found"), + }, + { + testName: "when session update returns no too many rows should return internal error", + cmd: domain.NewIDPIntentCheckCommand(&domain.CheckIDPIntentType{ID: "intent-123"}, "session-1", "instance-1", nil), + idpIntentRepo: func(ctrl *gomock.Controller) domain.IDPIntentRepository { + repo := domainmock.NewIDPIntentRepo(ctrl) + repo.EXPECT(). + Delete(gomock.Any(), gomock.Any(), repo.PrimaryKeyCondition("instance-1", "intent-123")). + Return(int64(1), nil) + return repo + }, + sessionRepo: func(ctrl *gomock.Controller) domain.SessionRepository { + repo := domainmock.NewSessionRepo(ctrl) + repo.EXPECT(). + Update(gomock.Any(), gomock.Any(), repo.PrimaryKeyCondition("instance-1", "session-1"), repo.SetFactor(&domain.SessionFactorIdentityProviderIntent{LastVerifiedAt: time.Now()})). + Times(1). + Return(int64(2), nil) + return repo + }, + expectedError: zerrors.ThrowInternal(domain.NewMultipleObjectsUpdatedError(1, 2), "DOM-mlbibw", "unexpected number of rows updated"), + }, + { + testName: "when session update returns 1 row updated should return no error", + cmd: domain.NewIDPIntentCheckCommand(&domain.CheckIDPIntentType{ID: "intent-123"}, "session-1", "instance-1", nil), + idpIntentRepo: func(ctrl *gomock.Controller) domain.IDPIntentRepository { + repo := domainmock.NewIDPIntentRepo(ctrl) + repo.EXPECT(). + Delete(gomock.Any(), gomock.Any(), repo.PrimaryKeyCondition("instance-1", "intent-123")). + Return(int64(1), nil) + return repo + }, + sessionRepo: func(ctrl *gomock.Controller) domain.SessionRepository { + repo := domainmock.NewSessionRepo(ctrl) + repo.EXPECT(). + Update(gomock.Any(), gomock.Any(), repo.PrimaryKeyCondition("instance-1", "session-1"), repo.SetFactor(&domain.SessionFactorIdentityProviderIntent{LastVerifiedAt: time.Now()})). + Times(1). + Return(int64(1), nil) + return repo + }, expectComplete: true, }, } for _, tc := range tt { t.Run(tc.testName, func(t *testing.T) { - // t.Parallel() + t.Parallel() // Given ctrl := gomock.NewController(t) ctx := authz.NewMockContext(tc.cmd.InstanceID, "", "") @@ -809,6 +878,9 @@ func TestIDPIntentCheckCommand_Execute(t *testing.T) { if tc.idpIntentRepo != nil { domain.WithIDPIntentRepo(tc.idpIntentRepo(ctrl))(opts) } + if tc.sessionRepo != nil { + domain.WithSessionRepo(tc.sessionRepo(ctrl))(opts) + } // Test err := tc.cmd.Execute(ctx, opts) @@ -816,6 +888,9 @@ func TestIDPIntentCheckCommand_Execute(t *testing.T) { // Verify assert.ErrorIs(t, err, tc.expectedError) assert.Equal(t, tc.expectComplete, tc.cmd.IsCheckComplete) + if tc.expectedError == nil && tc.expectComplete { + assert.NotZero(t, tc.cmd.IntentLastVerified) + } }) } } diff --git a/backend/v3/domain/session_check_password_test.go b/backend/v3/domain/session_check_password_test.go index d312c9276..7366fea77 100644 --- a/backend/v3/domain/session_check_password_test.go +++ b/backend/v3/domain/session_check_password_test.go @@ -17,7 +17,6 @@ import ( "github.com/zitadel/zitadel/backend/v3/storage/database/dbmock" noopdb "github.com/zitadel/zitadel/backend/v3/storage/database/dialect/noop" "github.com/zitadel/zitadel/internal/api/authz" - "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/repository/session" "github.com/zitadel/zitadel/internal/repository/user" @@ -1049,7 +1048,7 @@ func TestPasswordCheckCommand_Execute(t *testing.T) { } if tc.humanRepo != nil { humanRepo := tc.humanRepo(ctrl) - userRepo := domainmock.NewMockUserRepository(ctrl) + userRepo := domainmock.NewUserRepo(ctrl) userRepo.EXPECT().Human().Times(1).Return(humanRepo) domain.WithUserRepo(userRepo)(opts) } @@ -1071,7 +1070,6 @@ func TestPasswordCheckCommand_Execute(t *testing.T) { func TestPasswordCheckCommand_Events(t *testing.T) { t.Parallel() - domain.SetPasswordHasher(&crypto.Hasher{}) userAgg := user.NewAggregate("user-1", "org-1").Aggregate sessAgg := session.NewAggregate("session-1", "instance-1").Aggregate @@ -1206,7 +1204,7 @@ func TestPasswordCheckCommand_Events(t *testing.T) { case *session.PasswordCheckedEvent: actualAssertedType, ok := events[i].(*session.PasswordCheckedEvent) require.True(t, ok) - assert.InDelta(t, expectedAssertedType.CheckedAt.UnixMilli(), actualAssertedType.CheckedAt.UnixMilli(), 10) + assert.Equal(t, tc.cmd.CheckTime, actualAssertedType.CheckedAt) } } })