diff --git a/apps/api/project.json b/apps/api/project.json index d360ef155..32fd798ed 100644 --- a/apps/api/project.json +++ b/apps/api/project.json @@ -6,6 +6,7 @@ "sources": [ "{workspaceRoot}/cmd/**/*.go", "{workspaceRoot}/internal/**/*.go", + "{workspaceRoot}/backend/**/*.go", "{workspaceRoot}/proto/**/*.go", "{workspaceRoot}/pkg/**/*.go", "{workspaceRoot}/main.go" diff --git a/backend/v3/storage/database/condition.go b/backend/v3/storage/database/condition.go index b000018d0..4d9ada49a 100644 --- a/backend/v3/storage/database/condition.go +++ b/backend/v3/storage/database/condition.go @@ -1,6 +1,8 @@ package database -import "go.uber.org/mock/gomock" +import ( + "go.uber.org/mock/gomock" +) // Condition represents a SQL condition. // Its written after the WHERE keyword in a SQL statement. @@ -158,7 +160,7 @@ func IsNull(column Column) *isNull { // IsRestrictingColumn implements [Condition]. func (i isNull) IsRestrictingColumn(col Column) bool { - return true + return false } var _ Condition = (*isNull)(nil) @@ -194,7 +196,7 @@ func IsNotNull(column Column) *isNotNull { // IsRestrictingColumn implements [Condition]. func (i isNotNull) IsRestrictingColumn(col Column) bool { - return true + return false } var _ Condition = (*isNotNull)(nil) @@ -344,3 +346,21 @@ func (e existsCondition) IsRestrictingColumn(col Column) bool { } var _ Condition = (*existsCondition)(nil) + +type forcedRestrictingColumnCondition struct { + Condition + isRestricting bool +} + +func (c forcedRestrictingColumnCondition) IsRestrictingColumn(col Column) bool { + return c.isRestricting +} + +func ForceRestrictingColumn(cond Condition, isRestriction bool) Condition { + return forcedRestrictingColumnCondition{ + Condition: cond, + isRestricting: isRestriction, + } +} + +var _ Condition = (*forcedRestrictingColumnCondition)(nil) diff --git a/backend/v3/storage/database/condition_test.go b/backend/v3/storage/database/condition_test.go index 4ac1c78ae..01dac0b4d 100644 --- a/backend/v3/storage/database/condition_test.go +++ b/backend/v3/storage/database/condition_test.go @@ -174,13 +174,13 @@ func TestWrite(t *testing.T) { func TestIsRestrictingColumn(t *testing.T) { for _, test := range []struct { name string - col Column + arg Column cond Condition want bool }{ { name: "and with restricting column", - col: NewColumn("table", "column1"), + arg: NewColumn("table", "column1"), cond: And( NewColumnCondition(NewColumn("table", "column1"), NewColumn("other_table", "column2")), NewColumnCondition(NewColumn("table", "column1"), NewColumn("other_table", "column3")), @@ -189,7 +189,7 @@ func TestIsRestrictingColumn(t *testing.T) { }, { name: "and without restricting column", - col: NewColumn("table", "column1"), + arg: NewColumn("table", "column1"), cond: And( NewColumnCondition(NewColumn("table", "column2"), NewColumn("other_table", "column3")), IsNull(NewColumn("table", "column4")), @@ -199,7 +199,7 @@ func TestIsRestrictingColumn(t *testing.T) { }, { name: "or with restricting column", - col: NewColumn("table", "column1"), + arg: NewColumn("table", "column1"), cond: Or( NewColumnCondition(NewColumn("table", "column1"), NewColumn("other_table", "column2")), NewColumnCondition(NewColumn("table", "column1"), NewColumn("other_table", "column3")), @@ -208,7 +208,7 @@ func TestIsRestrictingColumn(t *testing.T) { }, { name: "or without restricting column", - col: NewColumn("table", "column1"), + arg: NewColumn("table", "column1"), cond: Or( NewColumnCondition(NewColumn("table", "column1"), NewColumn("other_table", "column3")), IsNotNull(NewColumn("table", "column4")), @@ -218,19 +218,19 @@ func TestIsRestrictingColumn(t *testing.T) { }, { name: "is null never restricts", - col: NewColumn("table", "column1"), + arg: NewColumn("table", "column1"), cond: IsNull(NewColumn("table", "column1")), want: false, }, { name: "is not null never restricts", - col: NewColumn("table", "column1"), + arg: NewColumn("table", "column1"), cond: IsNotNull(NewColumn("table", "column1")), want: false, }, { name: "exists with restricting column", - col: NewColumn("table", "column1"), + arg: NewColumn("table", "column1"), cond: Exists("table", And( NewColumnCondition(NewColumn("table", "column1"), NewColumn("other_table", "column2")), NewColumnCondition(NewColumn("table", "column1"), NewColumn("other_table", "column3")), @@ -239,7 +239,7 @@ func TestIsRestrictingColumn(t *testing.T) { }, { name: "exists without restricting column", - col: NewColumn("table", "column1"), + arg: NewColumn("table", "column1"), cond: Exists("table", Or( NewColumnCondition(NewColumn("table", "column1"), NewColumn("other_table", "column3")), IsNotNull(NewColumn("table", "column4")), @@ -249,7 +249,7 @@ func TestIsRestrictingColumn(t *testing.T) { }, } { t.Run(test.name, func(t *testing.T) { - isRestricting := test.cond.IsRestrictingColumn(test.col) + isRestricting := test.cond.IsRestrictingColumn(test.arg) assert.Equal(t, test.want, isRestricting) }) } diff --git a/backend/v3/storage/database/events_testing/settings_instance_test.go b/backend/v3/storage/database/events_testing/settings_instance_test.go index 87a9ce595..239626ad5 100644 --- a/backend/v3/storage/database/events_testing/settings_instance_test.go +++ b/backend/v3/storage/database/events_testing/settings_instance_test.go @@ -326,9 +326,9 @@ func TestServer_TestInstanceLabelSettingsReduces(t *testing.T) { settingsRepo := repository.BrandingSettingsRepository() - before := time.Now() + before := time.Now().Add(-time.Second) newInstance := integration.NewInstance(t.Context()) - after := time.Now() + after := time.Now().Add(time.Second) IAMCTX := newInstance.WithAuthorizationToken(t.Context(), integration.UserTypeIAMOwner) @@ -413,10 +413,10 @@ func TestServer_TestInstanceLabelSettingsReduces(t *testing.T) { t.Run("test label settings activated", func(t *testing.T) { // activate label - before := time.Now() + before := time.Now().Add(-time.Second) _, err := newInstance.Client.Admin.ActivateLabelPolicy(IAMCTX, &admin.ActivateLabelPolicyRequest{}) require.NoError(t, err) - after := time.Now() + after := time.Now().Add(time.Second) retryDuration, tick := integration.WaitForAndTickWithMaxDuration(IAMCTX, time.Second*20) assert.EventuallyWithT(t, func(collect *assert.CollectT) { @@ -436,9 +436,9 @@ func TestServer_TestInstanceLabelSettingsReduces(t *testing.T) { t.Run("test policy label logo light added", func(t *testing.T) { // set logo light - before := time.Now() + before := time.Now().Add(-time.Second) uploadInstanceAsset(IAMCTX, t, newInstance, "/instance/policy/label/logo", bytes.NewReader(picture)) - after := time.Now() + after := time.Now().Add(time.Second) retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Second*20) assert.EventuallyWithT(t, func(collect *assert.CollectT) { diff --git a/backend/v3/storage/database/repository/settings.go b/backend/v3/storage/database/repository/settings.go index 5496e1811..0af22e94e 100644 --- a/backend/v3/storage/database/repository/settings.go +++ b/backend/v3/storage/database/repository/settings.go @@ -86,11 +86,14 @@ func (s settings) PrimaryKeyCondition(instanceID, id string) database.Condition } func (s settings) UniqueCondition(instanceID string, orgID *string, typ domain.SettingType, state domain.SettingState) database.Condition { - return database.And( - s.InstanceIDCondition(instanceID), - s.OrganizationIDCondition(orgID), - s.TypeCondition(typ), - s.StateCondition(state), + return database.ForceRestrictingColumn( + database.And( + s.InstanceIDCondition(instanceID), + s.OrganizationIDCondition(orgID), + s.TypeCondition(typ), + s.StateCondition(state), + ), + true, ) } @@ -633,11 +636,6 @@ func (s brandingSettings) ActivateAt(ctx context.Context, client database.QueryE } func (s brandingSettings) activateAt(ctx context.Context, client database.QueryExecutor, condition database.Condition, updatedAt time.Time) (int64, error) { - condition = database.And( - condition, - s.TypeCondition(domain.SettingTypeBranding), - s.StateCondition(domain.SettingStatePreview), - ) // if you want to update the roles you have to have a unique condition, otherwise multiple branding settings get activated if err := checkRestrictingColumns(condition, s.UniqueColumns()...); err != nil { return 0, err diff --git a/backend/v3/storage/database/repository/settings_test.go b/backend/v3/storage/database/repository/settings_test.go index c8af17090..5c51da1d6 100644 --- a/backend/v3/storage/database/repository/settings_test.go +++ b/backend/v3/storage/database/repository/settings_test.go @@ -1252,34 +1252,42 @@ func TestActivateBrandingSettings(t *testing.T) { }{ { name: "activate instance", - condition: database.And( - repo.InstanceIDCondition(existingInstanceSettings.InstanceID), - repo.OrganizationIDCondition(nil), + condition: repo.UniqueCondition( + existingInstanceSettings.InstanceID, + nil, + domain.SettingTypeBranding, + domain.SettingStatePreview, ), wantRowsAffected: 1, }, { name: "activate organization", - condition: database.And( - repo.InstanceIDCondition(existingOrganizationSettings.InstanceID), - repo.OrganizationIDCondition(existingOrganizationSettings.OrganizationID), + condition: repo.UniqueCondition( + existingInstanceSettings.InstanceID, + existingOrganizationSettings.OrganizationID, + domain.SettingTypeBranding, + domain.SettingStatePreview, ), wantRowsAffected: 1, }, { name: "non-existing instance", - condition: database.And( - repo.InstanceIDCondition("foo"), - repo.OrganizationIDCondition(nil), + condition: repo.UniqueCondition( + "foo", + nil, + domain.SettingTypeBranding, + domain.SettingStatePreview, ), wantRowsAffected: 0, }, { name: "non-existing organization", - condition: database.And( - repo.InstanceIDCondition(existingOrganizationSettings.InstanceID), - repo.OrganizationIDCondition(gu.Ptr("foo")), + condition: repo.UniqueCondition( + existingInstanceSettings.InstanceID, + gu.Ptr("foo"), + domain.SettingTypeBranding, + domain.SettingStatePreview, ), wantRowsAffected: 0, }, @@ -1370,27 +1378,33 @@ func TestActivateAtBrandingSettings(t *testing.T) { }{ { name: "activate instance", - condition: database.And( - repo.InstanceIDCondition(existingInstanceSettings.InstanceID), - repo.OrganizationIDCondition(nil), + condition: repo.UniqueCondition( + existingInstanceSettings.InstanceID, + nil, + domain.SettingTypeBranding, + domain.SettingStatePreview, ), updatedAt: time.Now(), wantRowsAffected: 1, }, { name: "activate organization", - condition: database.And( - repo.InstanceIDCondition(existingOrganizationSettings.InstanceID), - repo.OrganizationIDCondition(existingOrganizationSettings.OrganizationID), + condition: repo.UniqueCondition( + existingOrganizationSettings.InstanceID, + existingOrganizationSettings.OrganizationID, + domain.SettingTypeBranding, + domain.SettingStatePreview, ), updatedAt: time.Now(), wantRowsAffected: 1, }, { name: "non-existing instance", - condition: database.And( - repo.InstanceIDCondition("foo"), - repo.OrganizationIDCondition(nil), + condition: repo.UniqueCondition( + "foo", + nil, + domain.SettingTypeBranding, + domain.SettingStatePreview, ), updatedAt: time.Now(), wantRowsAffected: 0, @@ -1398,9 +1412,11 @@ func TestActivateAtBrandingSettings(t *testing.T) { { name: "non-existing organization", - condition: database.And( - repo.InstanceIDCondition(existingOrganizationSettings.InstanceID), - repo.OrganizationIDCondition(gu.Ptr("foo")), + condition: repo.UniqueCondition( + existingOrganizationSettings.InstanceID, + gu.Ptr("foo"), + domain.SettingTypeBranding, + domain.SettingStatePreview, ), updatedAt: time.Now(), wantRowsAffected: 0, @@ -1454,9 +1470,11 @@ func TestDeleteBrandingSettings(t *testing.T) { require.NoError(t, err) _, err = repo.Activate(t.Context(), tx, - database.And( - repo.InstanceIDCondition(existingInstanceSettings.InstanceID), - repo.OrganizationIDCondition(existingInstanceSettings.OrganizationID), + repo.UniqueCondition( + existingInstanceSettings.InstanceID, + existingInstanceSettings.OrganizationID, + domain.SettingTypeBranding, + domain.SettingStatePreview, ), ) require.NoError(t, err) @@ -1492,9 +1510,11 @@ func TestDeleteBrandingSettings(t *testing.T) { require.NoError(t, err) _, err = repo.Activate(t.Context(), tx, - database.And( - repo.InstanceIDCondition(existingOrganizationSettings.InstanceID), - repo.OrganizationIDCondition(existingOrganizationSettings.OrganizationID), + repo.UniqueCondition( + existingOrganizationSettings.InstanceID, + existingOrganizationSettings.OrganizationID, + domain.SettingTypeBranding, + domain.SettingStatePreview, ), ) require.NoError(t, err) diff --git a/internal/query/projection/settings_relational.go b/internal/query/projection/settings_relational.go index 8409c3eae..2b7dffefd 100644 --- a/internal/query/projection/settings_relational.go +++ b/internal/query/projection/settings_relational.go @@ -8,7 +8,6 @@ import ( "github.com/muhlemmer/gu" "github.com/zitadel/zitadel/backend/v3/domain" - "github.com/zitadel/zitadel/backend/v3/storage/database" v3_sql "github.com/zitadel/zitadel/backend/v3/storage/database/dialect/sql" "github.com/zitadel/zitadel/backend/v3/storage/database/repository" "github.com/zitadel/zitadel/internal/eventstore" @@ -766,10 +765,7 @@ func (s *settingsRelationalProjection) reduceLabelActivated(event eventstore.Eve settingsRepo := repository.BrandingSettingsRepository() _, err := settingsRepo.ActivateAt(ctx, v3_sql.SQLTx(tx), - database.And( - settingsRepo.InstanceIDCondition(event.Aggregate().InstanceID), - settingsRepo.OrganizationIDCondition(orgId), - ), + settingsRepo.UniqueCondition(policyEvent.Aggregate().InstanceID, orgId, domain.SettingTypeBranding, domain.SettingStatePreview), policyEvent.Creation, ) return err