From 4dc7a58a256727169daf3ba3d2f9b1b40262f0ca Mon Sep 17 00:00:00 2001 From: Lars Date: Wed, 5 Feb 2025 16:40:56 +0100 Subject: [PATCH] fix: ensure metadata is projected for scim tests to ensure stable tests (#9305) # Which Problems Are Solved - SCIM tests are flaky due to metadata being set by the tests while shortly after being read by the application, resulting in a race condition # How the Problems Are Solved - whenever metadata is set, the projection is awaited # Additional Context Part of #8140 Co-authored-by: Stefan Benz <46600784+stebenz@users.noreply.github.com> --- .../api/scim/integration_test/bulk_test.go | 37 +++++++++++++++++++ .../integration_test/users_create_test.go | 13 +------ .../scim/integration_test/users_get_test.go | 21 ++--------- .../scim/integration_test/users_list_test.go | 21 ++--------- .../integration_test/users_replace_test.go | 13 +------ 5 files changed, 47 insertions(+), 58 deletions(-) diff --git a/internal/api/scim/integration_test/bulk_test.go b/internal/api/scim/integration_test/bulk_test.go index 2c9f95a262..660b10f4fd 100644 --- a/internal/api/scim/integration_test/bulk_test.go +++ b/internal/api/scim/integration_test/bulk_test.go @@ -23,6 +23,7 @@ import ( "github.com/zitadel/zitadel/internal/integration" "github.com/zitadel/zitadel/internal/integration/scim" "github.com/zitadel/zitadel/internal/test" + "github.com/zitadel/zitadel/pkg/grpc/management" ) var ( @@ -695,3 +696,39 @@ func buildTooManyOperationsRequest() *scim.BulkRequest { return req } + +func setProvisioningDomain(t require.TestingT, userID, provisioningDomain string) { + setAndEnsureMetadata(t, userID, "urn:zitadel:scim:provisioningDomain", provisioningDomain) +} + +func setAndEnsureMetadata(t require.TestingT, userID, key, value string) { + _, err := Instance.Client.Mgmt.SetUserMetadata(CTX, &management.SetUserMetadataRequest{ + Id: userID, + Key: key, + Value: []byte(value), + }) + require.NoError(t, err) + + // ensure metadata is projected + ensureMetadataProjected(t, userID, key, value) +} + +func ensureMetadataProjected(t require.TestingT, userID, key, value string) { + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) + require.EventuallyWithT(t, func(tt *assert.CollectT) { + md, err := Instance.Client.Mgmt.GetUserMetadata(CTX, &management.GetUserMetadataRequest{ + Id: userID, + Key: key, + }) + require.NoError(tt, err) + require.Equal(tt, value, string(md.Metadata.Value)) + }, retryDuration, tick) +} + +func removeProvisioningDomain(t require.TestingT, userID string) { + _, err := Instance.Client.Mgmt.RemoveUserMetadata(CTX, &management.RemoveUserMetadataRequest{ + Id: userID, + Key: "urn:zitadel:scim:provisioningDomain", + }) + require.NoError(t, err) +} diff --git a/internal/api/scim/integration_test/users_create_test.go b/internal/api/scim/integration_test/users_create_test.go index 1202add3d4..8b6986666c 100644 --- a/internal/api/scim/integration_test/users_create_test.go +++ b/internal/api/scim/integration_test/users_create_test.go @@ -395,12 +395,7 @@ func TestCreateUser_metadata(t *testing.T) { } func TestCreateUser_scopedExternalID(t *testing.T) { - _, err := Instance.Client.Mgmt.SetUserMetadata(CTX, &management.SetUserMetadataRequest{ - Id: Instance.Users.Get(integration.UserTypeOrgOwner).ID, - Key: "urn:zitadel:scim:provisioningDomain", - Value: []byte("fooBar"), - }) - require.NoError(t, err) + setProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID, "fooBar") createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, fullUserJson) require.NoError(t, err) @@ -409,11 +404,7 @@ func TestCreateUser_scopedExternalID(t *testing.T) { _, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: createdUser.ID}) require.NoError(t, err) - _, err = Instance.Client.Mgmt.RemoveUserMetadata(CTX, &management.RemoveUserMetadataRequest{ - Id: Instance.Users.Get(integration.UserTypeOrgOwner).ID, - Key: "urn:zitadel:scim:provisioningDomain", - }) - require.NoError(t, err) + removeProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID) }() retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) diff --git a/internal/api/scim/integration_test/users_get_test.go b/internal/api/scim/integration_test/users_get_test.go index 7651952b43..0106e47ca2 100644 --- a/internal/api/scim/integration_test/users_get_test.go +++ b/internal/api/scim/integration_test/users_get_test.go @@ -18,7 +18,6 @@ import ( "github.com/zitadel/zitadel/internal/integration" "github.com/zitadel/zitadel/internal/integration/scim" "github.com/zitadel/zitadel/internal/test" - "github.com/zitadel/zitadel/pkg/grpc/management" "github.com/zitadel/zitadel/pkg/grpc/user/v2" ) @@ -203,31 +202,17 @@ func TestGetUser(t *testing.T) { require.NoError(t, err) // set provisioning domain of service user - _, err = Instance.Client.Mgmt.SetUserMetadata(CTX, &management.SetUserMetadataRequest{ - Id: Instance.Users.Get(integration.UserTypeOrgOwner).ID, - Key: "urn:zitadel:scim:provisioningDomain", - Value: []byte("fooBar"), - }) - require.NoError(t, err) + setProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID, "fooBar") // set externalID for provisioning domain - _, err = Instance.Client.Mgmt.SetUserMetadata(CTX, &management.SetUserMetadataRequest{ - Id: createdUser.ID, - Key: "urn:zitadel:scim:fooBar:externalId", - Value: []byte("100-scopedExternalId"), - }) - require.NoError(t, err) + setAndEnsureMetadata(t, createdUser.ID, "urn:zitadel:scim:fooBar:externalId", "100-scopedExternalId") return createdUser.ID }, cleanup: func(userID string) { _, err := Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: userID}) require.NoError(t, err) - _, err = Instance.Client.Mgmt.RemoveUserMetadata(CTX, &management.RemoveUserMetadataRequest{ - Id: Instance.Users.Get(integration.UserTypeOrgOwner).ID, - Key: "urn:zitadel:scim:provisioningDomain", - }) - require.NoError(t, err) + removeProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID) }, want: &resources.ScimUser{ ExternalID: "100-scopedExternalId", diff --git a/internal/api/scim/integration_test/users_list_test.go b/internal/api/scim/integration_test/users_list_test.go index d364fa219d..7945d2039d 100644 --- a/internal/api/scim/integration_test/users_list_test.go +++ b/internal/api/scim/integration_test/users_list_test.go @@ -20,7 +20,6 @@ import ( "github.com/zitadel/zitadel/internal/integration" "github.com/zitadel/zitadel/internal/integration/scim" "github.com/zitadel/zitadel/internal/test" - "github.com/zitadel/zitadel/pkg/grpc/management" "github.com/zitadel/zitadel/pkg/grpc/object/v2" user_v2 "github.com/zitadel/zitadel/pkg/grpc/user/v2" ) @@ -372,20 +371,10 @@ func TestListUser(t *testing.T) { resp := createHumanUser(t, CTX, Instance.DefaultOrg.Id, 102) // set provisioning domain of service user - _, err := Instance.Client.Mgmt.SetUserMetadata(CTX, &management.SetUserMetadataRequest{ - Id: Instance.Users.Get(integration.UserTypeOrgOwner).ID, - Key: "urn:zitadel:scim:provisioningDomain", - Value: []byte("fooBar"), - }) - require.NoError(t, err) + setProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID, "fooBar") // set externalID for provisioning domain - _, err = Instance.Client.Mgmt.SetUserMetadata(CTX, &management.SetUserMetadataRequest{ - Id: resp.UserId, - Key: "urn:zitadel:scim:fooBar:externalId", - Value: []byte("100-scopedExternalId"), - }) - require.NoError(t, err) + setAndEnsureMetadata(t, resp.UserId, "urn:zitadel:scim:fooBar:externalId", "100-scopedExternalId") return &scim.ListRequest{ Filter: gu.Ptr(fmt.Sprintf(`id eq "%s"`, resp.UserId)), } @@ -396,11 +385,7 @@ func TestListUser(t *testing.T) { }, cleanup: func(t require.TestingT) { // delete provisioning domain of service user - _, err := Instance.Client.Mgmt.RemoveUserMetadata(CTX, &management.RemoveUserMetadataRequest{ - Id: Instance.Users.Get(integration.UserTypeOrgOwner).ID, - Key: "urn:zitadel:scim:provisioningDomain", - }) - require.NoError(t, err) + removeProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID) }, }, } diff --git a/internal/api/scim/integration_test/users_replace_test.go b/internal/api/scim/integration_test/users_replace_test.go index 9f88621872..770ed06959 100644 --- a/internal/api/scim/integration_test/users_replace_test.go +++ b/internal/api/scim/integration_test/users_replace_test.go @@ -316,12 +316,7 @@ func TestReplaceUser_scopedExternalID(t *testing.T) { require.NoError(t, err) // set provisioning domain of service user - _, err = Instance.Client.Mgmt.SetUserMetadata(CTX, &management.SetUserMetadataRequest{ - Id: Instance.Users.Get(integration.UserTypeOrgOwner).ID, - Key: "urn:zitadel:scim:provisioningDomain", - Value: []byte("fooBazz"), - }) - require.NoError(t, err) + setProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID, "fooBazz") // replace the user with provisioning domain set _, err = Instance.Client.SCIM.Users.Replace(CTX, Instance.DefaultOrg.Id, createdUser.ID, minimalUserWithExternalIDJson) @@ -347,9 +342,5 @@ func TestReplaceUser_scopedExternalID(t *testing.T) { _, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: createdUser.ID}) require.NoError(t, err) - _, err = Instance.Client.Mgmt.RemoveUserMetadata(CTX, &management.RemoveUserMetadataRequest{ - Id: Instance.Users.Get(integration.UserTypeOrgOwner).ID, - Key: "urn:zitadel:scim:provisioningDomain", - }) - require.NoError(t, err) + removeProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID) }