andwati/zitadel
1
0
Fork 0
mirror of https://github.com/andwati/zitadel.git synced 2026-10-18 05:52:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(domain): Fix session checks failing tests (#11881)

Browse Source 
# Which Problems Are Solved

2 tests were failing on the new session checks:

  - Password check (events) fails due to a race condition
- IDP intent check (events) (also password test) fails due to a flaky
time comparison

# How the Problems Are Solved

- Race condition: remove setting the password hasher in the test. It's
already globally set in `domain_test.go`
- Time comparison: use `assert.WithinRange` instead of `assert.InDelta`
and use 2 second range to make sure you don't get resolution issues

# Additional Context

- Closes #11878
This commit is contained in:
Marco A.
2026-03-20 06:57:36 +01:00
committed by GitHub
parent a592fd7293
commit 8d1a099deb
3 changed files with 134 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/v3/domain/session_check_idp_intent.go
+45 -5
View File
@@ -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
backend/v3/domain/session_check_idp_intent_test.go
+87 -12
View File
@@ -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)
}
})
}
}
backend/v3/domain/session_check_password_test.go
+2 -4
View File
@@ -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)
}
}
})