diff --git a/Makefile b/Makefile index b5145cef3d..b415abe08c 100644 --- a/Makefile +++ b/Makefile @@ -112,7 +112,7 @@ core_unit_test: .PHONY: core_integration_db_up core_integration_db_up: - docker compose -f internal/integration/config/docker-compose.yaml up --pull always --wait cache + docker compose -f internal/integration/config/docker-compose.yaml up --pull always --wait .PHONY: core_integration_db_down core_integration_db_down: diff --git a/internal/api/scim/integration_test/bulk_test.go b/internal/api/scim/integration_test/bulk_test.go index 660b10f4fd..c2f430c1b7 100644 --- a/internal/api/scim/integration_test/bulk_test.go +++ b/internal/api/scim/integration_test/bulk_test.go @@ -17,6 +17,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/text/language" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/zitadel/zitadel/internal/api/scim/resources" "github.com/zitadel/zitadel/internal/api/scim/schemas" @@ -478,7 +480,7 @@ func TestBulk(t *testing.T) { }, { name: "fail on errors", - body: bulkFailOnErrorsJson, + body: withUsername(bulkFailOnErrorsJson, gofakeit.Username()), want: &scim.BulkResponse{ Schemas: []schemas.ScimSchemaType{schemas.IdBulkResponse}, Operations: []*scim.BulkResponseOperation{ @@ -579,7 +581,6 @@ func TestBulk(t *testing.T) { response, err := Instance.Client.SCIM.Bulk(ctx, orgID, tt.body) createdUserIDs := buildCreatedIDs(response) - defer deleteUsers(t, createdUserIDs) if tt.wantErr != nil { statusCode := tt.wantErr.status @@ -656,17 +657,6 @@ func buildCreatedIDs(response *scim.BulkResponse) []string { return createdIds } -func deleteUsers(t require.TestingT, ids []string) { - for _, id := range ids { - err := Instance.Client.SCIM.Users.Delete(CTX, Instance.DefaultOrg.Id, id) - - // only not found errors are ok (if the user is deleted in a later on bulk request) - if err != nil { - scim.RequireScimError(t, http.StatusNotFound, err) - } - } -} - func buildMinimalUpdateRequest(userID string) *scim.BulkRequest { return &scim.BulkRequest{ Schemas: []schemas.ScimSchemaType{schemas.IdBulkRequest}, @@ -690,7 +680,7 @@ func buildTooManyOperationsRequest() *scim.BulkRequest { req.Operations[i] = &scim.BulkRequestOperation{ Method: http.MethodPost, Path: "/Users", - Data: minimalUserJson, + Data: withUsername(minimalUserJson, gofakeit.Username()), } } @@ -720,8 +710,11 @@ func ensureMetadataProjected(t require.TestingT, userID, key, value string) { Id: userID, Key: key, }) - require.NoError(tt, err) - require.Equal(tt, value, string(md.Metadata.Value)) + if !assert.NoError(tt, err) { + require.Equal(tt, status.Code(err), codes.NotFound) + return + } + assert.Equal(tt, value, string(md.Metadata.Value)) }, retryDuration, tick) } diff --git a/internal/api/scim/integration_test/testdata/bulk_test_fail_on_errors.json b/internal/api/scim/integration_test/testdata/bulk_test_fail_on_errors.json index bc9ed1346a..d1d0c88fe6 100644 --- a/internal/api/scim/integration_test/testdata/bulk_test_fail_on_errors.json +++ b/internal/api/scim/integration_test/testdata/bulk_test_fail_on_errors.json @@ -32,14 +32,14 @@ "urn:ietf:params:scim:schemas:core:2.0:User" ], "externalId": "scim-bulk-created-user-0", - "userName": "scim-bulk-created-user-0", + "userName": "{{ .Username }}", "name": { "familyName": "scim-bulk-created-user-0-family-name", "givenName": "scim-bulk-created-user-0-given-name" }, "emails": [ { - "value": "scim-bulk-created-user-0@example.com", + "value": "{{ .Username }}@example.com", "primary": true } ], diff --git a/internal/api/scim/integration_test/testdata/users_create_test_full.json b/internal/api/scim/integration_test/testdata/users_create_test_full.json index 7879ecf160..cfc786a7e3 100644 --- a/internal/api/scim/integration_test/testdata/users_create_test_full.json +++ b/internal/api/scim/integration_test/testdata/users_create_test_full.json @@ -1,7 +1,7 @@ { "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], "externalId": "701984", - "userName": "bjensen@example.com", + "userName": "{{ .Username }}@example.com", "name": { "formatted": "Ms. Barbara J Jensen, III", "familyName": "Jensen", @@ -15,12 +15,12 @@ "profileUrl": "http://login.example.com/bjensen", "emails": [ { - "value": "bjensen@example.com", + "value": "{{ .Username }}@example.com", "type": "work", "primary": true }, { - "value": "babs@jensen.org", + "value": "{{ .Username }}+1@example.com", "type": "home" } ], diff --git a/internal/api/scim/integration_test/testdata/users_create_test_minimal.json b/internal/api/scim/integration_test/testdata/users_create_test_minimal.json index c51f416bc7..bdceb3e45f 100644 --- a/internal/api/scim/integration_test/testdata/users_create_test_minimal.json +++ b/internal/api/scim/integration_test/testdata/users_create_test_minimal.json @@ -2,14 +2,14 @@ "schemas": [ "urn:ietf:params:scim:schemas:core:2.0:User" ], - "userName": "acmeUser1", + "userName": "{{ .Username }}", "name": { "familyName": "Ross", "givenName": "Bethany" }, "emails": [ { - "value": "user1@example.com", + "value": "{{ .Username }}@example.com", "primary": true } ] diff --git a/internal/api/scim/integration_test/testdata/users_create_test_minimal_inactive.json b/internal/api/scim/integration_test/testdata/users_create_test_minimal_inactive.json index 11650674a6..95ca1246d5 100644 --- a/internal/api/scim/integration_test/testdata/users_create_test_minimal_inactive.json +++ b/internal/api/scim/integration_test/testdata/users_create_test_minimal_inactive.json @@ -2,14 +2,14 @@ "schemas": [ "urn:ietf:params:scim:schemas:core:2.0:User" ], - "userName": "acmeUser1", + "userName": "acmeUser1-inactive", "name": { "familyName": "Ross", "givenName": "Bethany" }, "emails": [ { - "value": "user1@example.com", + "value": "user1-inactive@example.com", "primary": true } ], diff --git a/internal/api/scim/integration_test/testdata/users_create_test_no_primary_email_phone.json b/internal/api/scim/integration_test/testdata/users_create_test_no_primary_email_phone.json index 20c67c4715..f2b4bf2e4c 100644 --- a/internal/api/scim/integration_test/testdata/users_create_test_no_primary_email_phone.json +++ b/internal/api/scim/integration_test/testdata/users_create_test_no_primary_email_phone.json @@ -2,14 +2,14 @@ "schemas": [ "urn:ietf:params:scim:schemas:core:2.0:User" ], - "userName": "acmeUser1", + "userName": "acmeUser1-no-primary-email-phone", "name": { "familyName": "Ross", "givenName": "Bethany" }, "emails": [ { - "value": "user1@example.com" + "value": "user1-no-primary-email-phone@example.com" } ], "phoneNumbers": [ diff --git a/internal/api/scim/integration_test/testdata/users_replace_test_minimal_with_email_type.json b/internal/api/scim/integration_test/testdata/users_replace_test_minimal_with_email_type.json index b7e8d87590..33d78a2e3a 100644 --- a/internal/api/scim/integration_test/testdata/users_replace_test_minimal_with_email_type.json +++ b/internal/api/scim/integration_test/testdata/users_replace_test_minimal_with_email_type.json @@ -2,14 +2,14 @@ "schemas": [ "urn:ietf:params:scim:schemas:core:2.0:User" ], - "userName": "acmeUser1-minimal-replaced", + "userName": "{{ .Username }}", "name": { "familyName": "Ross-replaced", "givenName": "Bethany-replaced" }, "emails": [ { - "value": "user1-minimal-replaced@example.com", + "value": "{{ .Username }}@example.com", "primary": true, "type": "work" } diff --git a/internal/api/scim/integration_test/testdata/users_update_test_full.json b/internal/api/scim/integration_test/testdata/users_update_test_full.json index 23403f3e5b..f79a0b5332 100644 --- a/internal/api/scim/integration_test/testdata/users_update_test_full.json +++ b/internal/api/scim/integration_test/testdata/users_update_test_full.json @@ -7,7 +7,7 @@ "value": { "emails":[ { - "value":"babs@example.com", + "value":"{{ .Username }}+2@example.com", "type":"home", "primary": true } diff --git a/internal/api/scim/integration_test/users_create_test.go b/internal/api/scim/integration_test/users_create_test.go index 35d5297878..71356c7b43 100644 --- a/internal/api/scim/integration_test/users_create_test.go +++ b/internal/api/scim/integration_test/users_create_test.go @@ -3,17 +3,22 @@ package integration_test import ( + "bytes" "context" _ "embed" + "fmt" "net/http" "path" "testing" + "text/template" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/text/language" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/zitadel/zitadel/internal/api/scim/resources" "github.com/zitadel/zitadel/internal/api/scim/schemas" @@ -157,7 +162,18 @@ var ( } ) +func withUsername(fixture []byte, username string) []byte { + buf := new(bytes.Buffer) + template.Must(template.New("").Parse(string(fixture))).Execute(buf, &struct { + Username string + }{ + Username: username, + }) + return buf.Bytes() +} + func TestCreateUser(t *testing.T) { + minimalUsername := gofakeit.Username() tests := []struct { name string body []byte @@ -171,16 +187,16 @@ func TestCreateUser(t *testing.T) { }{ { name: "minimal user", - body: minimalUserJson, + body: withUsername(minimalUserJson, minimalUsername), want: &resources.ScimUser{ - UserName: "acmeUser1", + UserName: minimalUsername, Name: &resources.ScimUserName{ FamilyName: "Ross", GivenName: "Bethany", }, Emails: []*resources.ScimEmail{ { - Value: "user1@example.com", + Value: minimalUsername + "@example.com", Primary: true, }, }, @@ -195,7 +211,7 @@ func TestCreateUser(t *testing.T) { }, { name: "full user", - body: fullUserJson, + body: withUsername(fullUserJson, "bjensen"), want: fullUser, }, { @@ -204,7 +220,7 @@ func TestCreateUser(t *testing.T) { want: &resources.ScimUser{ Emails: []*resources.ScimEmail{ { - Value: "user1@example.com", + Value: "user1-no-primary-email-phone@example.com", Primary: true, }, }, @@ -262,21 +278,21 @@ func TestCreateUser(t *testing.T) { }, { name: "not authenticated", - body: minimalUserJson, + body: withUsername(minimalUserJson, gofakeit.Username()), ctx: context.Background(), wantErr: true, errorStatus: http.StatusUnauthorized, }, { name: "no permissions", - body: minimalUserJson, + body: withUsername(minimalUserJson, gofakeit.Username()), ctx: Instance.WithAuthorization(CTX, integration.UserTypeNoPermission), wantErr: true, errorStatus: http.StatusNotFound, }, { name: "another org", - body: minimalUserJson, + body: withUsername(minimalUserJson, gofakeit.Username()), orgID: SecondaryOrganization.OrganizationId, wantErr: true, errorStatus: http.StatusNotFound, @@ -315,11 +331,6 @@ func TestCreateUser(t *testing.T) { } assert.NotEmpty(t, createdUser.ID) - defer func() { - _, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: createdUser.ID}) - assert.NoError(t, err) - }() - assert.EqualValues(t, []schemas.ScimSchemaType{"urn:ietf:params:scim:schemas:core:2.0:User"}, createdUser.Resource.Schemas) assert.Equal(t, schemas.ScimResourceTypeSingular("User"), createdUser.Resource.Meta.ResourceType) assert.Equal(t, "http://"+Instance.Host()+path.Join(schemas.HandlerPrefix, orgID, "Users", createdUser.ID), createdUser.Resource.Meta.Location) @@ -345,10 +356,11 @@ func TestCreateUser(t *testing.T) { } func TestCreateUser_duplicate(t *testing.T) { - createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, minimalUserJson) + parsedMinimalUserJson := withUsername(minimalUserJson, gofakeit.Username()) + createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, parsedMinimalUserJson) require.NoError(t, err) - _, err = Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, minimalUserJson) + _, err = Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, parsedMinimalUserJson) scimErr := scim.RequireScimError(t, http.StatusConflict, err) assert.Equal(t, "User already exists", scimErr.Error.Detail) assert.Equal(t, "uniqueness", scimErr.Error.ScimType) @@ -358,14 +370,10 @@ func TestCreateUser_duplicate(t *testing.T) { } func TestCreateUser_metadata(t *testing.T) { - createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, fullUserJson) + username := gofakeit.Username() + createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, username)) require.NoError(t, err) - defer func() { - _, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: createdUser.ID}) - require.NoError(t, err) - }() - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) require.EventuallyWithT(t, func(tt *assert.CollectT) { md, err := Instance.Client.Mgmt.ListUserMetadata(CTX, &management.ListUserMetadataRequest{ @@ -391,38 +399,111 @@ func TestCreateUser_metadata(t *testing.T) { test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:locale", "en-US") test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:ims", `[{"value":"someaimhandle","type":"aim"},{"value":"twitterhandle","type":"X"}]`) test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:roles", `[{"value":"my-role-1","display":"Rolle 1","type":"main-role","primary":true},{"value":"my-role-2","display":"Rolle 2","type":"secondary-role"}]`) - test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:emails", `[{"value":"bjensen@example.com","primary":true,"type":"work"},{"value":"babs@jensen.org","primary":false,"type":"home"}]`) + test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:emails", fmt.Sprintf(`[{"value":"%s@example.com","primary":true,"type":"work"},{"value":"%s+1@example.com","primary":false,"type":"home"}]`, username, username)) }, retryDuration, tick) } func TestCreateUser_scopedExternalID(t *testing.T) { - setProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID, "fooBar") - - createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, fullUserJson) + callingUserId, callingUserPat, err := Instance.CreateMachineUserPATWithMembership(CTX, "ORG_OWNER") require.NoError(t, err) - - defer func() { - _, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: createdUser.ID}) - require.NoError(t, err) - - removeProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID) - }() - - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) + ctx := integration.WithAuthorizationToken(CTX, callingUserPat) + setProvisioningDomain(t, callingUserId, "fooBar") + createdUser, err := Instance.Client.SCIM.Users.Create(ctx, Instance.DefaultOrg.Id, withUsername(fullUserJson, gofakeit.Username())) + require.NoError(t, err) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) require.EventuallyWithT(t, func(tt *assert.CollectT) { // unscoped externalID should not exist - _, err = Instance.Client.Mgmt.GetUserMetadata(CTX, &management.GetUserMetadataRequest{ + unscoped, err := Instance.Client.Mgmt.GetUserMetadata(ctx, &management.GetUserMetadataRequest{ Id: createdUser.ID, Key: "urn:zitadel:scim:externalId", }) integration.AssertGrpcStatus(tt, codes.NotFound, err) + unscoped = unscoped // scoped externalID should exist - md, err := Instance.Client.Mgmt.GetUserMetadata(CTX, &management.GetUserMetadataRequest{ + md, err := Instance.Client.Mgmt.GetUserMetadata(ctx, &management.GetUserMetadataRequest{ Id: createdUser.ID, Key: "urn:zitadel:scim:fooBar:externalId", }) - require.NoError(tt, err) + if !assert.NoError(tt, err) { + require.Equal(tt, status.Code(err), codes.NotFound) + return + } assert.Equal(tt, "701984", string(md.Metadata.Value)) }, retryDuration, tick) } + +func TestCreateUser_ignorePasswordOnCreate(t *testing.T) { + t.Parallel() + tests := []struct { + name string + ignorePassword string + scimErrorType string + scimErrorDetail string + wantUser *resources.ScimUser + wantErr bool + }{ + { + name: "ignorePasswordOnCreate set to false", + ignorePassword: "false", + wantErr: true, + scimErrorType: "invalidValue", + scimErrorDetail: "Password is too short", + }, + { + name: "ignorePasswordOnCreate set to an invalid value", + ignorePassword: "random", + wantErr: true, + scimErrorType: "invalidValue", + scimErrorDetail: "Invalid value for metadata key urn:zitadel:scim:ignorePasswordOnCreate: random", + }, + { + name: "ignorePasswordOnCreate set to true", + ignorePassword: "true", + wantUser: &resources.ScimUser{ + UserName: "acmeUser1", + Name: &resources.ScimUserName{ + FamilyName: "Ross", + GivenName: "Bethany", + }, + Emails: []*resources.ScimEmail{ + { + Value: "user1@example.com", + Primary: true, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + // create a machine user + callingUserId, callingUserPat, err := Instance.CreateMachineUserPATWithMembership(CTX, "ORG_OWNER") + require.NoError(t, err) + ctx := integration.WithAuthorizationToken(CTX, callingUserPat) + + // set urn:zitadel:scim:ignorePasswordOnCreate metadata for the machine user + setAndEnsureMetadata(t, callingUserId, "urn:zitadel:scim:ignorePasswordOnCreate", tt.ignorePassword) + + // create a user with an invalid password + createdUser, err := Instance.Client.SCIM.Users.Create(ctx, Instance.DefaultOrg.Id, withUsername(invalidPasswordUserJson, "acmeUser1")) + require.Equal(t, tt.wantErr, err != nil) + if err != nil { + scimErr := scim.RequireScimError(t, http.StatusBadRequest, err) + assert.Equal(t, tt.scimErrorType, scimErr.Error.ScimType) + assert.Equal(t, tt.scimErrorDetail, scimErr.Error.Detail) + return + } + + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) + require.EventuallyWithT(t, func(ttt *assert.CollectT) { + // ensure the user is really stored and not just returned to the caller + fetchedUser, err := Instance.Client.SCIM.Users.Get(CTX, Instance.DefaultOrg.Id, createdUser.ID) + require.NoError(ttt, err) + assert.True(ttt, test.PartiallyDeepEqual(tt.wantUser, fetchedUser)) + }, retryDuration, tick) + }) + } +} diff --git a/internal/api/scim/integration_test/users_get_test.go b/internal/api/scim/integration_test/users_get_test.go index 8a1bab6c93..08e09946fb 100644 --- a/internal/api/scim/integration_test/users_get_test.go +++ b/internal/api/scim/integration_test/users_get_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/text/language" @@ -18,256 +19,260 @@ 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/user/v2" ) func TestGetUser(t *testing.T) { - tests := []struct { - name string - orgID string - buildUserID func() string - cleanup func(userID string) + type testCase struct { ctx context.Context + orgID string + userID string want *resources.ScimUser wantErr bool errorStatus int + } + tests := []struct { + name string + setup func(t *testing.T) testCase }{ { - name: "not authenticated", - ctx: context.Background(), - errorStatus: http.StatusUnauthorized, - wantErr: true, + name: "not authenticated", + setup: func(t *testing.T) testCase { + return testCase{ + ctx: context.Background(), + errorStatus: http.StatusUnauthorized, + wantErr: true, + } + }, }, { - name: "no permissions", - ctx: Instance.WithAuthorization(CTX, integration.UserTypeNoPermission), - errorStatus: http.StatusNotFound, - wantErr: true, + name: "no permissions", + setup: func(t *testing.T) testCase { + return testCase{ + ctx: Instance.WithAuthorization(CTX, integration.UserTypeNoPermission), + errorStatus: http.StatusNotFound, + wantErr: true, + } + }, }, { - name: "another org", - orgID: SecondaryOrganization.OrganizationId, - errorStatus: http.StatusNotFound, - wantErr: true, + name: "another org", + setup: func(t *testing.T) testCase { + return testCase{ + orgID: SecondaryOrganization.OrganizationId, + errorStatus: http.StatusNotFound, + wantErr: true, + } + }, }, { - name: "another org with permissions", - orgID: SecondaryOrganization.OrganizationId, - ctx: Instance.WithAuthorization(CTX, integration.UserTypeNoPermission), - errorStatus: http.StatusNotFound, - wantErr: true, + name: "another org with permissions", + setup: func(t *testing.T) testCase { + return testCase{ + ctx: Instance.WithAuthorization(CTX, integration.UserTypeNoPermission), + orgID: SecondaryOrganization.OrganizationId, + errorStatus: http.StatusNotFound, + wantErr: true, + } + }, }, { name: "unknown user id", - buildUserID: func() string { - return "unknown" + setup: func(t *testing.T) testCase { + return testCase{ + userID: "unknown", + errorStatus: http.StatusNotFound, + wantErr: true, + } }, - errorStatus: http.StatusNotFound, - wantErr: true, }, { name: "created via grpc", - want: &resources.ScimUser{ - Name: &resources.ScimUserName{ - FamilyName: "Mouse", - GivenName: "Mickey", - }, - PreferredLanguage: language.MustParse("nl"), - PhoneNumbers: []*resources.ScimPhoneNumber{ - { - Value: "+41791234567", - Primary: true, + setup: func(t *testing.T) testCase { + return testCase{ + want: &resources.ScimUser{ + Name: &resources.ScimUserName{ + FamilyName: "Mouse", + GivenName: "Mickey", + }, + PreferredLanguage: language.MustParse("nl"), + PhoneNumbers: []*resources.ScimPhoneNumber{ + { + Value: "+41791234567", + Primary: true, + }, + }, }, - }, + } }, }, { name: "created via scim", - buildUserID: func() string { - createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, fullUserJson) + setup: func(t *testing.T) testCase { + username := gofakeit.Username() + createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, username)) require.NoError(t, err) - return createdUser.ID - }, - cleanup: func(userID string) { - _, err := Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: userID}) - require.NoError(t, err) - }, - want: &resources.ScimUser{ - ExternalID: "701984", - UserName: "bjensen@example.com", - Name: &resources.ScimUserName{ - Formatted: "Babs Jensen", // DisplayName takes precedence - FamilyName: "Jensen", - GivenName: "Barbara", - MiddleName: "Jane", - HonorificPrefix: "Ms.", - HonorificSuffix: "III", - }, - DisplayName: "Babs Jensen", - NickName: "Babs", - ProfileUrl: test.Must(schemas.ParseHTTPURL("http://login.example.com/bjensen")), - Title: "Tour Guide", - PreferredLanguage: language.Make("en-US"), - Locale: "en-US", - Timezone: "America/Los_Angeles", - Active: schemas.NewRelaxedBool(true), - Emails: []*resources.ScimEmail{ - { - Value: "bjensen@example.com", - Primary: true, - Type: "work", + return testCase{ + userID: createdUser.ID, + want: &resources.ScimUser{ + ExternalID: "701984", + UserName: username + "@example.com", + Name: &resources.ScimUserName{ + Formatted: "Babs Jensen", // DisplayName takes precedence + FamilyName: "Jensen", + GivenName: "Barbara", + MiddleName: "Jane", + HonorificPrefix: "Ms.", + HonorificSuffix: "III", + }, + DisplayName: "Babs Jensen", + NickName: "Babs", + ProfileUrl: test.Must(schemas.ParseHTTPURL("http://login.example.com/bjensen")), + Title: "Tour Guide", + PreferredLanguage: language.Make("en-US"), + Locale: "en-US", + Timezone: "America/Los_Angeles", + Active: schemas.NewRelaxedBool(true), + Emails: []*resources.ScimEmail{ + { + Value: username + "@example.com", + Primary: true, + Type: "work", + }, + }, + PhoneNumbers: []*resources.ScimPhoneNumber{ + { + Value: "+415555555555", + Primary: true, + }, + }, + Ims: []*resources.ScimIms{ + { + Value: "someaimhandle", + Type: "aim", + }, + { + Value: "twitterhandle", + Type: "X", + }, + }, + Addresses: []*resources.ScimAddress{ + { + Type: "work", + StreetAddress: "100 Universal City Plaza", + Locality: "Hollywood", + Region: "CA", + PostalCode: "91608", + Country: "USA", + Formatted: "100 Universal City Plaza\nHollywood, CA 91608 USA", + Primary: true, + }, + { + Type: "home", + StreetAddress: "456 Hollywood Blvd", + Locality: "Hollywood", + Region: "CA", + PostalCode: "91608", + Country: "USA", + Formatted: "456 Hollywood Blvd\nHollywood, CA 91608 USA", + }, + }, + Photos: []*resources.ScimPhoto{ + { + Value: *test.Must(schemas.ParseHTTPURL("https://photos.example.com/profilephoto/72930000000Ccne/F")), + Type: "photo", + }, + { + Value: *test.Must(schemas.ParseHTTPURL("https://photos.example.com/profilephoto/72930000000Ccne/T")), + Type: "thumbnail", + }, + }, + Roles: []*resources.ScimRole{ + { + Value: "my-role-1", + Display: "Rolle 1", + Type: "main-role", + Primary: true, + }, + { + Value: "my-role-2", + Display: "Rolle 2", + Type: "secondary-role", + Primary: false, + }, + }, + Entitlements: []*resources.ScimEntitlement{ + { + Value: "my-entitlement-1", + Display: "Entitlement 1", + Type: "main-entitlement", + Primary: true, + }, + { + Value: "my-entitlement-2", + Display: "Entitlement 2", + Type: "secondary-entitlement", + Primary: false, + }, + }, }, - }, - PhoneNumbers: []*resources.ScimPhoneNumber{ - { - Value: "+415555555555", - Primary: true, - }, - }, - Ims: []*resources.ScimIms{ - { - Value: "someaimhandle", - Type: "aim", - }, - { - Value: "twitterhandle", - Type: "X", - }, - }, - Addresses: []*resources.ScimAddress{ - { - Type: "work", - StreetAddress: "100 Universal City Plaza", - Locality: "Hollywood", - Region: "CA", - PostalCode: "91608", - Country: "USA", - Formatted: "100 Universal City Plaza\nHollywood, CA 91608 USA", - Primary: true, - }, - { - Type: "home", - StreetAddress: "456 Hollywood Blvd", - Locality: "Hollywood", - Region: "CA", - PostalCode: "91608", - Country: "USA", - Formatted: "456 Hollywood Blvd\nHollywood, CA 91608 USA", - }, - }, - Photos: []*resources.ScimPhoto{ - { - Value: *test.Must(schemas.ParseHTTPURL("https://photos.example.com/profilephoto/72930000000Ccne/F")), - Type: "photo", - }, - { - Value: *test.Must(schemas.ParseHTTPURL("https://photos.example.com/profilephoto/72930000000Ccne/T")), - Type: "thumbnail", - }, - }, - Roles: []*resources.ScimRole{ - { - Value: "my-role-1", - Display: "Rolle 1", - Type: "main-role", - Primary: true, - }, - { - Value: "my-role-2", - Display: "Rolle 2", - Type: "secondary-role", - Primary: false, - }, - }, - Entitlements: []*resources.ScimEntitlement{ - { - Value: "my-entitlement-1", - Display: "Entitlement 1", - Type: "main-entitlement", - Primary: true, - }, - { - Value: "my-entitlement-2", - Display: "Entitlement 2", - Type: "secondary-entitlement", - Primary: false, - }, - }, + } }, }, { name: "scoped externalID", - buildUserID: func() string { - // create user without provisioning domain - createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, fullUserJson) + setup: func(t *testing.T) testCase { + createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, gofakeit.Username())) require.NoError(t, err) - - // set provisioning domain of service user - setProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID, "fooBar") - - // set externalID for provisioning domain + callingUserId, callingUserPat, err := Instance.CreateMachineUserPATWithMembership(CTX, "ORG_OWNER") + require.NoError(t, err) + setProvisioningDomain(t, callingUserId, "fooBar") 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) - - removeProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID) - }, - want: &resources.ScimUser{ - ExternalID: "100-scopedExternalId", + return testCase{ + ctx: integration.WithAuthorizationToken(CTX, callingUserPat), + userID: createdUser.ID, + want: &resources.ScimUser{ + ExternalID: "100-scopedExternalId", + }, + } }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctx := tt.ctx - if ctx == nil { - ctx = CTX + ttt := tt.setup(t) + if ttt.userID == "" { + ttt.userID = Instance.CreateHumanUser(CTX).UserId } - - var userID string - if tt.buildUserID != nil { - userID = tt.buildUserID() - } else { - createUserResp := Instance.CreateHumanUser(CTX) - userID = createUserResp.UserId + if ttt.ctx == nil { + ttt.ctx = CTX } - - orgID := tt.orgID - if orgID == "" { - orgID = Instance.DefaultOrg.Id + if ttt.orgID == "" { + ttt.orgID = Instance.DefaultOrg.Id } - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) - var fetchedUser *resources.ScimUser - var err error - require.EventuallyWithT(t, func(ttt *assert.CollectT) { - fetchedUser, err = Instance.Client.SCIM.Users.Get(ctx, orgID, userID) - if tt.wantErr { - statusCode := tt.errorStatus + require.EventuallyWithT(t, func(collect *assert.CollectT) { + fetchedUser, err := Instance.Client.SCIM.Users.Get(ttt.ctx, ttt.orgID, ttt.userID) + if ttt.wantErr { + statusCode := ttt.errorStatus if statusCode == 0 { statusCode = http.StatusBadRequest } - - scim.RequireScimError(ttt, statusCode, err) + scim.RequireScimError(collect, statusCode, err) return } - - assert.Equal(ttt, userID, fetchedUser.ID) - assert.EqualValues(ttt, []schemas.ScimSchemaType{"urn:ietf:params:scim:schemas:core:2.0:User"}, fetchedUser.Schemas) - assert.Equal(ttt, schemas.ScimResourceTypeSingular("User"), fetchedUser.Resource.Meta.ResourceType) - assert.Equal(ttt, "http://"+Instance.Host()+path.Join(schemas.HandlerPrefix, orgID, "Users", fetchedUser.ID), fetchedUser.Resource.Meta.Location) - assert.Nil(ttt, fetchedUser.Password) - if !test.PartiallyDeepEqual(tt.want, fetchedUser) { - ttt.Errorf("GetUser() got = %#v, want %#v", fetchedUser, tt.want) + if !assert.NoError(collect, err) { + scim.RequireScimError(collect, http.StatusNotFound, err) + return + } + assert.Equal(collect, ttt.userID, fetchedUser.ID) + assert.EqualValues(collect, []schemas.ScimSchemaType{"urn:ietf:params:scim:schemas:core:2.0:User"}, fetchedUser.Schemas) + assert.Equal(collect, schemas.ScimResourceTypeSingular("User"), fetchedUser.Resource.Meta.ResourceType) + assert.Equal(collect, "http://"+Instance.Host()+path.Join(schemas.HandlerPrefix, ttt.orgID, "Users", fetchedUser.ID), fetchedUser.Resource.Meta.Location) + assert.Nil(collect, fetchedUser.Password) + if !test.PartiallyDeepEqual(ttt.want, fetchedUser) { + collect.Errorf("GetUser() got = %#v, want %#v", fetchedUser, ttt.want) } }, retryDuration, tick) - - if tt.cleanup != nil { - tt.cleanup(fetchedUser.ID) - } }) } } diff --git a/internal/api/scim/integration_test/users_list_test.go b/internal/api/scim/integration_test/users_list_test.go index 8c6ccb80ef..81fbf1a5bc 100644 --- a/internal/api/scim/integration_test/users_list_test.go +++ b/internal/api/scim/integration_test/users_list_test.go @@ -23,15 +23,6 @@ var totalCountOfHumanUsers = 13 /* func TestListUser(t *testing.T) { createdUserIDs := createUsers(t, CTX, Instance.DefaultOrg.Id) - defer func() { - // only the full user needs to be deleted, all others have random identification data - // fullUser is always the first one. - _, err := Instance.Client.UserV2.DeleteUser(CTX, &user_v2.DeleteUserRequest{ - UserId: createdUserIDs[0], - }) - require.NoError(t, err) - }() - // secondary organization with same set of users, // these should never be modified. // This allows testing list requests without filters. @@ -451,7 +442,7 @@ func createUsers(t *testing.T, ctx context.Context, orgID string) []string { // create the full scim user if on primary org if orgID == Instance.DefaultOrg.Id { - fullUserCreatedResp, err := Instance.Client.SCIM.Users.Create(ctx, orgID, fullUserJson) + fullUserCreatedResp, err := Instance.Client.SCIM.Users.Create(ctx, orgID, withUsername(fullUserJson, gofakeit.Username())) require.NoError(t, err) createdUserIDs = append(createdUserIDs, fullUserCreatedResp.ID) count-- diff --git a/internal/api/scim/integration_test/users_replace_test.go b/internal/api/scim/integration_test/users_replace_test.go index 1c99592b01..7896ed4e00 100644 --- a/internal/api/scim/integration_test/users_replace_test.go +++ b/internal/api/scim/integration_test/users_replace_test.go @@ -5,11 +5,13 @@ package integration_test import ( "context" _ "embed" + "fmt" "net/http" "path" "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/text/language" @@ -20,7 +22,6 @@ import ( "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" ) var ( @@ -199,28 +200,28 @@ func TestReplaceUser(t *testing.T) { }, { name: "not authenticated", - body: minimalUserJson, + body: withUsername(minimalUserJson, gofakeit.Username()), ctx: context.Background(), wantErr: true, errorStatus: http.StatusUnauthorized, }, { name: "no permissions", - body: minimalUserJson, + body: withUsername(minimalUserJson, gofakeit.Username()), ctx: Instance.WithAuthorization(CTX, integration.UserTypeNoPermission), wantErr: true, errorStatus: http.StatusNotFound, }, { name: "another org", - body: minimalUserJson, + body: withUsername(minimalUserJson, gofakeit.Username()), replaceUserOrgID: SecondaryOrganization.OrganizationId, wantErr: true, errorStatus: http.StatusNotFound, }, { name: "another org with permissions", - body: minimalUserJson, + body: withUsername(minimalUserJson, gofakeit.Username()), replaceUserOrgID: SecondaryOrganization.OrganizationId, ctx: Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner), wantErr: true, @@ -230,14 +231,9 @@ func TestReplaceUser(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // use iam owner => we don't want to test permissions of the create endpoint. - createdUser, err := Instance.Client.SCIM.Users.Create(Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner), Instance.DefaultOrg.Id, fullUserJson) + createdUser, err := Instance.Client.SCIM.Users.Create(Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner), Instance.DefaultOrg.Id, withUsername(fullUserJson, gofakeit.Username())) require.NoError(t, err) - defer func() { - _, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: createdUser.ID}) - assert.NoError(t, err) - }() - ctx := tt.ctx if ctx == nil { ctx = CTX @@ -294,10 +290,11 @@ func TestReplaceUser(t *testing.T) { func TestReplaceUser_removeOldMetadata(t *testing.T) { // ensure old metadata is removed correctly - createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, fullUserJson) + username := gofakeit.Username() + createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, username)) require.NoError(t, err) - _, err = Instance.Client.SCIM.Users.Replace(CTX, Instance.DefaultOrg.Id, createdUser.ID, minimalUserJson) + _, err = Instance.Client.SCIM.Users.Replace(CTX, Instance.DefaultOrg.Id, createdUser.ID, withUsername(minimalUserJson, username)) require.NoError(t, err) retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) @@ -312,20 +309,17 @@ func TestReplaceUser_removeOldMetadata(t *testing.T) { for i := range md.Result { mdMap[md.Result[i].Key] = string(md.Result[i].Value) } - - test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:emails", "[{\"value\":\"user1@example.com\",\"primary\":true}]") + test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:emails", fmt.Sprintf("[{\"value\":\"%s@example.com\",\"primary\":true}]", username)) }, retryDuration, tick) - - _, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: createdUser.ID}) - require.NoError(t, err) } func TestReplaceUser_emailType(t *testing.T) { // ensure old metadata is removed correctly - createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, fullUserJson) + createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, gofakeit.Username())) require.NoError(t, err) - _, err = Instance.Client.SCIM.Users.Replace(CTX, Instance.DefaultOrg.Id, createdUser.ID, minimalUserWithEmailTypeReplaceJson) + replacedUsername := gofakeit.Username() + _, err = Instance.Client.SCIM.Users.Replace(CTX, Instance.DefaultOrg.Id, createdUser.ID, withUsername(minimalUserWithEmailTypeReplaceJson, replacedUsername)) require.NoError(t, err) retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) @@ -341,28 +335,26 @@ func TestReplaceUser_emailType(t *testing.T) { mdMap[md.Result[i].Key] = string(md.Result[i].Value) } - test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:emails", "[{\"value\":\"user1-minimal-replaced@example.com\",\"primary\":true,\"type\":\"work\"}]") + test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:emails", fmt.Sprintf("[{\"value\":\"%s@example.com\",\"primary\":true,\"type\":\"work\"}]", replacedUsername)) }, retryDuration, tick) - - _, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: createdUser.ID}) - require.NoError(t, err) } func TestReplaceUser_scopedExternalID(t *testing.T) { - // create user without provisioning domain set - createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, fullUserJson) + createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, gofakeit.Username())) require.NoError(t, err) - + callingUserId, callingUserPat, err := Instance.CreateMachineUserPATWithMembership(CTX, "ORG_OWNER") + require.NoError(t, err) + ctx := integration.WithAuthorizationToken(CTX, callingUserPat) // set provisioning domain of service user - setProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID, "fooBazz") + setProvisioningDomain(t, callingUserId, "fooBazz") // replace the user with provisioning domain set - _, err = Instance.Client.SCIM.Users.Replace(CTX, Instance.DefaultOrg.Id, createdUser.ID, minimalUserWithExternalIDJson) + _, err = Instance.Client.SCIM.Users.Replace(ctx, Instance.DefaultOrg.Id, createdUser.ID, minimalUserWithExternalIDJson) require.NoError(t, err) - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) require.EventuallyWithT(t, func(tt *assert.CollectT) { - md, err := Instance.Client.Mgmt.ListUserMetadata(CTX, &management.ListUserMetadataRequest{ + md, err := Instance.Client.Mgmt.ListUserMetadata(ctx, &management.ListUserMetadataRequest{ Id: createdUser.ID, }) require.NoError(tt, err) @@ -376,9 +368,4 @@ func TestReplaceUser_scopedExternalID(t *testing.T) { test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:externalId", "701984") test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:fooBazz:externalId", "replaced-external-id") }, retryDuration, tick) - - _, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: createdUser.ID}) - require.NoError(t, err) - - removeProvisioningDomain(t, Instance.Users.Get(integration.UserTypeOrgOwner).ID) } diff --git a/internal/api/scim/integration_test/users_update_test.go b/internal/api/scim/integration_test/users_update_test.go index 77e55bac60..c798b0fac3 100644 --- a/internal/api/scim/integration_test/users_update_test.go +++ b/internal/api/scim/integration_test/users_update_test.go @@ -5,11 +5,13 @@ package integration_test import ( "context" _ "embed" + "encoding/json" "fmt" "net/http" "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/text/language" @@ -19,7 +21,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/user/v2" ) var ( @@ -34,15 +35,7 @@ func init() { } func TestUpdateUser(t *testing.T) { - fullUserCreated, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, fullUserJson) - require.NoError(t, err) - - defer func() { - _, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: fullUserCreated.ID}) - require.NoError(t, err) - }() - - tests := []struct { + type testCase struct { name string body []byte ctx context.Context @@ -52,215 +45,296 @@ func TestUpdateUser(t *testing.T) { wantErr bool scimErrorType string errorStatus int + } + tests := []struct { + name string + setup func(t *testing.T) testCase }{ { - name: "not authenticated", - ctx: context.Background(), - body: minimalUserUpdateJson, - wantErr: true, - errorStatus: http.StatusUnauthorized, + name: "not authenticated", + setup: func(t *testing.T) testCase { + username := gofakeit.Username() + created, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, username)) + require.NoError(t, err) + return testCase{ + userID: created.ID, + ctx: context.Background(), + body: minimalUserUpdateJson, + wantErr: true, + errorStatus: http.StatusUnauthorized, + } + }, }, { - name: "no permissions", - ctx: Instance.WithAuthorization(CTX, integration.UserTypeNoPermission), - body: minimalUserUpdateJson, - wantErr: true, - errorStatus: http.StatusNotFound, + name: "no permissions", + setup: func(t *testing.T) testCase { + username := gofakeit.Username() + created, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, username)) + require.NoError(t, err) + return testCase{ + userID: created.ID, + ctx: Instance.WithAuthorization(CTX, integration.UserTypeNoPermission), + body: minimalUserUpdateJson, + wantErr: true, + errorStatus: http.StatusNotFound, + } + }, }, { - name: "other org", - orgID: SecondaryOrganization.OrganizationId, - body: minimalUserUpdateJson, - wantErr: true, - errorStatus: http.StatusNotFound, + name: "other org", + setup: func(t *testing.T) testCase { + username := gofakeit.Username() + created, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, username)) + require.NoError(t, err) + return testCase{ + userID: created.ID, + orgID: SecondaryOrganization.OrganizationId, + body: minimalUserUpdateJson, + wantErr: true, + errorStatus: http.StatusNotFound, + } + }, }, { - name: "other org with permissions", - ctx: Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner), - orgID: SecondaryOrganization.OrganizationId, - body: minimalUserUpdateJson, - wantErr: true, - errorStatus: http.StatusNotFound, + name: "other org with permissions", + setup: func(t *testing.T) testCase { + username := gofakeit.Username() + created, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, username)) + require.NoError(t, err) + return testCase{ + userID: created.ID, + ctx: Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner), + orgID: SecondaryOrganization.OrganizationId, + body: minimalUserUpdateJson, + wantErr: true, + errorStatus: http.StatusNotFound, + } + }, }, { - name: "invalid patch json", - body: simpleReplacePatchBody("nickname", "10"), - wantErr: true, - scimErrorType: "invalidValue", + name: "invalid patch json", + setup: func(t *testing.T) testCase { + username := gofakeit.Username() + created, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, username)) + require.NoError(t, err) + return testCase{ + userID: created.ID, + body: simpleReplacePatchBody("nickname", "10"), + wantErr: true, + scimErrorType: "invalidValue", + } + }, }, { - name: "password complexity violation", - body: simpleReplacePatchBody("password", `"fooBar"`), - wantErr: true, - scimErrorType: "invalidValue", + name: "password complexity violation", + setup: func(t *testing.T) testCase { + username := gofakeit.Username() + created, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, username)) + require.NoError(t, err) + return testCase{ + userID: created.ID, + body: simpleReplacePatchBody("password", `"fooBar"`), + wantErr: true, + scimErrorType: "invalidValue", + } + }, }, { - name: "invalid profile url", - body: simpleReplacePatchBody("profileUrl", `"ftp://example.com/profiles"`), - wantErr: true, - scimErrorType: "invalidValue", + name: "invalid profile url", + setup: func(t *testing.T) testCase { + username := gofakeit.Username() + created, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, username)) + require.NoError(t, err) + return testCase{ + userID: created.ID, + body: simpleReplacePatchBody("profileUrl", `"ftp://example.com/profiles"`), + wantErr: true, + scimErrorType: "invalidValue", + } + }, }, { - name: "invalid time zone", - body: simpleReplacePatchBody("timezone", `"foobar"`), - wantErr: true, - scimErrorType: "invalidValue", + name: "invalid time zone", + setup: func(t *testing.T) testCase { + username := gofakeit.Username() + created, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, username)) + require.NoError(t, err) + return testCase{ + userID: created.ID, + body: simpleReplacePatchBody("timezone", `"foobar"`), + wantErr: true, + scimErrorType: "invalidValue", + } + }, }, { - name: "invalid locale", - body: simpleReplacePatchBody("locale", `"foobar"`), - wantErr: true, - scimErrorType: "invalidValue", + name: "invalid locale", + setup: func(t *testing.T) testCase { + username := gofakeit.Username() + created, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, username)) + require.NoError(t, err) + return testCase{ + userID: created.ID, + body: simpleReplacePatchBody("locale", `"foobar"`), + wantErr: true, + scimErrorType: "invalidValue", + } + }, }, { - name: "unknown user id", - body: simpleReplacePatchBody("nickname", `"foo"`), - userID: "fooBar", - wantErr: true, - errorStatus: http.StatusNotFound, + name: "unknown user id", + setup: func(t *testing.T) testCase { + return testCase{ + body: simpleReplacePatchBody("nickname", `"foo"`), + userID: "fooBar", + wantErr: true, + errorStatus: http.StatusNotFound, + } + }, }, { name: "full", - body: fullUserUpdateJson, - want: &resources.ScimUser{ - ExternalID: "fooBAR", - UserName: "bjensen@example.com", - Name: &resources.ScimUserName{ - Formatted: "replaced-display-name", - FamilyName: "added-family-name", - GivenName: "added-given-name", - MiddleName: "added-middle-name-2", - HonorificPrefix: "added-honorific-prefix", - HonorificSuffix: "replaced-honorific-suffix", - }, - DisplayName: "replaced-display-name", - NickName: "", - ProfileUrl: test.Must(schemas.ParseHTTPURL("http://login.example.com/bjensen")), - Emails: []*resources.ScimEmail{ - { - Value: "bjensen@example.com", - Type: "work", + setup: func(t *testing.T) testCase { + username := gofakeit.Username() + created, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, withUsername(fullUserJson, username)) + require.NoError(t, err) + return testCase{ + userID: created.ID, + body: withUsername(fullUserUpdateJson, username), + want: &resources.ScimUser{ + ExternalID: "fooBAR", + UserName: username + "@example.com", + Name: &resources.ScimUserName{ + Formatted: "replaced-display-name", + FamilyName: "added-family-name", + GivenName: "added-given-name", + MiddleName: "added-middle-name-2", + HonorificPrefix: "added-honorific-prefix", + HonorificSuffix: "replaced-honorific-suffix", + }, + DisplayName: "replaced-display-name", + NickName: "", + ProfileUrl: test.Must(schemas.ParseHTTPURL("http://login.example.com/bjensen")), + Emails: []*resources.ScimEmail{ + { + Value: username + "@example.com", + Type: "work", + }, + { + Value: username + "+1@example.com", + Type: "home", + }, + { + Value: username + "+2@example.com", + Primary: true, + Type: "home", + }, + }, + Addresses: []*resources.ScimAddress{ + { + Type: "replaced-work", + StreetAddress: "replaced-100 Universal City Plaza", + Locality: "replaced-Hollywood", + Region: "replaced-CA", + PostalCode: "replaced-91608", + Country: "replaced-USA", + Formatted: "replaced-100 Universal City Plaza\nHollywood, CA 91608 USA", + Primary: true, + }, + }, + PhoneNumbers: []*resources.ScimPhoneNumber{ + { + Value: "+41711234567", + Primary: true, + }, + }, + Ims: []*resources.ScimIms{ + { + Value: "someaimhandle", + Type: "aim", + }, + { + Value: "twitterhandle", + Type: "", + }, + }, + Photos: []*resources.ScimPhoto{ + { + Value: *test.Must(schemas.ParseHTTPURL("https://photos.example.com/profilephoto/72930000000Ccne/F")), + Type: "photo", + }, + }, + Roles: nil, + Entitlements: []*resources.ScimEntitlement{ + { + Value: "my-entitlement-1", + Display: "added-entitlement-1", + Type: "added-entitlement-1", + Primary: false, + }, + { + Value: "my-entitlement-2", + Display: "Entitlement 2", + Type: "secondary-entitlement", + Primary: false, + }, + { + Value: "added-entitlement-1", + Primary: false, + }, + { + Value: "added-entitlement-2", + Primary: false, + }, + { + Value: "added-entitlement-3", + Primary: true, + }, + }, + Title: "Tour Guide", + PreferredLanguage: language.MustParse("en"), + Locale: "en-US", + Timezone: "America/Los_Angeles", + Active: schemas.NewRelaxedBool(true), }, - { - Value: "babs@jensen.org", - Type: "home", - }, - { - Value: "babs@example.com", - Primary: true, - Type: "home", - }, - }, - Addresses: []*resources.ScimAddress{ - { - Type: "replaced-work", - StreetAddress: "replaced-100 Universal City Plaza", - Locality: "replaced-Hollywood", - Region: "replaced-CA", - PostalCode: "replaced-91608", - Country: "replaced-USA", - Formatted: "replaced-100 Universal City Plaza\nHollywood, CA 91608 USA", - Primary: true, - }, - }, - PhoneNumbers: []*resources.ScimPhoneNumber{ - { - Value: "+41711234567", - Primary: true, - }, - }, - Ims: []*resources.ScimIms{ - { - Value: "someaimhandle", - Type: "aim", - }, - { - Value: "twitterhandle", - Type: "", - }, - }, - Photos: []*resources.ScimPhoto{ - { - Value: *test.Must(schemas.ParseHTTPURL("https://photos.example.com/profilephoto/72930000000Ccne/F")), - Type: "photo", - }, - }, - Roles: nil, - Entitlements: []*resources.ScimEntitlement{ - { - Value: "my-entitlement-1", - Display: "added-entitlement-1", - Type: "added-entitlement-1", - Primary: false, - }, - { - Value: "my-entitlement-2", - Display: "Entitlement 2", - Type: "secondary-entitlement", - Primary: false, - }, - { - Value: "added-entitlement-1", - Primary: false, - }, - { - Value: "added-entitlement-2", - Primary: false, - }, - { - Value: "added-entitlement-3", - Primary: true, - }, - }, - Title: "Tour Guide", - PreferredLanguage: language.MustParse("en-US"), - Locale: "en-US", - Timezone: "America/Los_Angeles", - Active: schemas.NewRelaxedBool(true), + } }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.ctx == nil { - tt.ctx = CTX + ttt := tt.setup(t) + if ttt.orgID == "" { + ttt.orgID = Instance.DefaultOrg.Id } - - if tt.orgID == "" { - tt.orgID = Instance.DefaultOrg.Id + if ttt.ctx == nil { + ttt.ctx = CTX } - - if tt.userID == "" { - tt.userID = fullUserCreated.ID - } - - err := Instance.Client.SCIM.Users.Update(tt.ctx, tt.orgID, tt.userID, tt.body) - - if tt.wantErr { + err := Instance.Client.SCIM.Users.Update(ttt.ctx, ttt.orgID, ttt.userID, ttt.body) + if ttt.wantErr { require.Error(t, err) - - statusCode := tt.errorStatus + statusCode := ttt.errorStatus if statusCode == 0 { statusCode = http.StatusBadRequest } - scimErr := scim.RequireScimError(t, statusCode, err) - assert.Equal(t, tt.scimErrorType, scimErr.Error.ScimType) + assert.Equal(t, ttt.scimErrorType, scimErr.Error.ScimType) return } require.NoError(t, err) - - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) - require.EventuallyWithT(t, func(ttt *assert.CollectT) { - fetchedUser, err := Instance.Client.SCIM.Users.Get(tt.ctx, tt.orgID, fullUserCreated.ID) - require.NoError(ttt, err) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, 3*time.Minute) + require.EventuallyWithT(t, func(collect *assert.CollectT) { + fetchedUser, err := Instance.Client.SCIM.Users.Get(ttt.ctx, ttt.orgID, ttt.userID) + require.NoError(collect, err) fetchedUser.Resource = nil fetchedUser.ID = "" - if tt.want != nil && !test.PartiallyDeepEqual(tt.want, fetchedUser) { - ttt.Errorf("got = %#v, want = %#v", fetchedUser, tt.want) - } + fetched, err := json.Marshal(fetchedUser) + require.NoError(collect, err) + want, err := json.Marshal(ttt.want) + require.NoError(collect, err) + assert.JSONEq(collect, string(want), string(fetched)) }, retryDuration, tick) }) } diff --git a/internal/api/scim/metadata/context.go b/internal/api/scim/metadata/context.go index dd8ef54d8e..2d0d76b928 100644 --- a/internal/api/scim/metadata/context.go +++ b/internal/api/scim/metadata/context.go @@ -15,6 +15,7 @@ var scimContextKey scimContextKeyType type ScimContextData struct { ProvisioningDomain string + IgnorePasswordOnCreate bool ExternalIDScopedMetadataKey ScopedKey bulkIDMapping map[string]string } diff --git a/internal/api/scim/metadata/metadata.go b/internal/api/scim/metadata/metadata.go index 28e42290d1..eb20284821 100644 --- a/internal/api/scim/metadata/metadata.go +++ b/internal/api/scim/metadata/metadata.go @@ -13,8 +13,9 @@ type ScopedKey string const ( externalIdProvisioningDomainPlaceholder = "{provisioningDomain}" - KeyPrefix = "urn:zitadel:scim:" - KeyProvisioningDomain Key = KeyPrefix + "provisioningDomain" + KeyPrefix = "urn:zitadel:scim:" + KeyProvisioningDomain Key = KeyPrefix + "provisioningDomain" + KeyIgnorePasswordOnCreate Key = KeyPrefix + "ignorePasswordOnCreate" KeyExternalId Key = KeyPrefix + "externalId" keyScopedExternalIdTemplate = KeyPrefix + externalIdProvisioningDomainPlaceholder + ":externalId" diff --git a/internal/api/scim/middleware/scim_context_middleware.go b/internal/api/scim/middleware/scim_context_middleware.go index 1ec917e18b..d63e490c08 100644 --- a/internal/api/scim/middleware/scim_context_middleware.go +++ b/internal/api/scim/middleware/scim_context_middleware.go @@ -3,10 +3,15 @@ package middleware import ( "context" "net/http" + "strconv" + "strings" + + "github.com/zitadel/logging" "github.com/zitadel/zitadel/internal/api/authz" zhttp "github.com/zitadel/zitadel/internal/api/http/middleware" smetadata "github.com/zitadel/zitadel/internal/api/scim/metadata" + sresources "github.com/zitadel/zitadel/internal/api/scim/resources" "github.com/zitadel/zitadel/internal/query" "github.com/zitadel/zitadel/internal/zerrors" ) @@ -29,22 +34,43 @@ func initScimContext(ctx context.Context, q *query.Queries) (context.Context, er ctx = smetadata.SetScimContextData(ctx, data) userID := authz.GetCtxData(ctx).UserID - metadata, err := q.GetUserMetadataByKey(ctx, false, userID, string(smetadata.KeyProvisioningDomain), false) + + // get the provisioningDomain and ignorePasswordOnCreate metadata keys associated with the service user + metadataKeys := []smetadata.Key{ + smetadata.KeyProvisioningDomain, + smetadata.KeyIgnorePasswordOnCreate, + } + queries := sresources.BuildMetadataQueries(ctx, metadataKeys) + + metadataList, err := q.SearchUserMetadata(ctx, false, userID, queries, false) if err != nil { if zerrors.IsNotFound(err) { return ctx, nil } - return ctx, err } - if metadata == nil { + if metadataList == nil || len(metadataList.Metadata) == 0 { return ctx, nil } - data.ProvisioningDomain = string(metadata.Value) - if data.ProvisioningDomain != "" { - data.ExternalIDScopedMetadataKey = smetadata.ScopeExternalIdKey(data.ProvisioningDomain) + for _, metadata := range metadataList.Metadata { + switch metadata.Key { + case string(smetadata.KeyProvisioningDomain): + data.ProvisioningDomain = string(metadata.Value) + if data.ProvisioningDomain != "" { + data.ExternalIDScopedMetadataKey = smetadata.ScopeExternalIdKey(data.ProvisioningDomain) + } + case string(smetadata.KeyIgnorePasswordOnCreate): + ignorePasswordOnCreate, parseErr := strconv.ParseBool(strings.TrimSpace(string(metadata.Value))) + if parseErr != nil { + return ctx, + zerrors.ThrowInvalidArgumentf(nil, "SMCM-yvw2rt", "Invalid value for metadata key %s: %s", smetadata.KeyIgnorePasswordOnCreate, metadata.Value) + } + data.IgnorePasswordOnCreate = ignorePasswordOnCreate + default: + logging.WithFields("user_metadata_key", metadata.Key).Warn("unexpected metadata key") + } } return smetadata.SetScimContextData(ctx, data), nil } diff --git a/internal/api/scim/resources/user_mapping.go b/internal/api/scim/resources/user_mapping.go index 260e50846a..0aac888173 100644 --- a/internal/api/scim/resources/user_mapping.go +++ b/internal/api/scim/resources/user_mapping.go @@ -45,7 +45,12 @@ func (h *UsersHandler) mapToAddHuman(ctx context.Context, scimUser *ScimUser) (* } human.Metadata = md - if scimUser.Password != nil { + // Okta sends a random password during SCIM provisioning + // irrespective of whether the Sync Password option is enabled or disabled on Okta. + // This password does not comply with Zitadel's password complexity, and + // the following workaround ignores the random password as it does not add any value. + ignorePasswordOnCreate := metadata.GetScimContextData(ctx).IgnorePasswordOnCreate + if scimUser.Password != nil && !ignorePasswordOnCreate { human.Password = scimUser.Password.String() scimUser.Password = nil } diff --git a/internal/api/scim/resources/user_metadata.go b/internal/api/scim/resources/user_metadata.go index 3e018507fe..ced5d195c3 100644 --- a/internal/api/scim/resources/user_metadata.go +++ b/internal/api/scim/resources/user_metadata.go @@ -21,7 +21,7 @@ import ( ) func (h *UsersHandler) queryMetadataForUsers(ctx context.Context, userIds []string) (map[string]map[metadata.ScopedKey][]byte, error) { - queries := h.buildMetadataQueries(ctx) + queries := BuildMetadataQueries(ctx, metadata.ScimUserRelevantMetadataKeys) md, err := h.query.SearchUserMetadataForUsers(ctx, false, userIds, queries) if err != nil { @@ -43,7 +43,7 @@ func (h *UsersHandler) queryMetadataForUsers(ctx context.Context, userIds []stri } func (h *UsersHandler) queryMetadataForUser(ctx context.Context, id string) (map[metadata.ScopedKey][]byte, error) { - queries := h.buildMetadataQueries(ctx) + queries := BuildMetadataQueries(ctx, metadata.ScimUserRelevantMetadataKeys) md, err := h.query.SearchUserMetadata(ctx, false, id, queries, false) if err != nil { @@ -53,9 +53,9 @@ func (h *UsersHandler) queryMetadataForUser(ctx context.Context, id string) (map return metadata.MapListToScopedKeyMap(md.Metadata), nil } -func (h *UsersHandler) buildMetadataQueries(ctx context.Context) *query.UserMetadataSearchQueries { - keyQueries := make([]query.SearchQuery, len(metadata.ScimUserRelevantMetadataKeys)) - for i, key := range metadata.ScimUserRelevantMetadataKeys { +func BuildMetadataQueries(ctx context.Context, metadataKeys []metadata.Key) *query.UserMetadataSearchQueries { + keyQueries := make([]query.SearchQuery, len(metadataKeys)) + for i, key := range metadataKeys { keyQueries[i] = buildMetadataKeyQuery(ctx, key) } diff --git a/internal/command/org.go b/internal/command/org.go index a018a90c82..9b22703d60 100644 --- a/internal/command/org.go +++ b/internal/command/org.go @@ -321,7 +321,7 @@ func (c *Commands) addOrgWithIDAndMember(ctx context.Context, name, userID, reso if err != nil { return nil, err } - err = c.checkUserExists(ctx, userID, resourceOwner) + _, err = c.checkUserExists(ctx, userID, resourceOwner) if err != nil { return nil, err } diff --git a/internal/command/project_grant_member.go b/internal/command/project_grant_member.go index f7ea887475..00b089eb2b 100644 --- a/internal/command/project_grant_member.go +++ b/internal/command/project_grant_member.go @@ -21,7 +21,7 @@ func (c *Commands) AddProjectGrantMember(ctx context.Context, member *domain.Pro if len(domain.CheckForInvalidRoles(member.Roles, domain.ProjectGrantRolePrefix, c.zitadelRoles)) > 0 { return nil, zerrors.ThrowInvalidArgument(nil, "PROJECT-m9gKK", "Errors.Project.Grant.Member.Invalid") } - err = c.checkUserExists(ctx, member.UserID, "") + _, err = c.checkUserExists(ctx, member.UserID, "") if err != nil { return nil, err } diff --git a/internal/command/project_member.go b/internal/command/project_member.go index a2e4fae553..af7b41cb72 100644 --- a/internal/command/project_member.go +++ b/internal/command/project_member.go @@ -45,7 +45,7 @@ func (c *Commands) addProjectMember(ctx context.Context, projectAgg *eventstore. return nil, zerrors.ThrowInvalidArgument(nil, "PROJECT-3m9ds", "Errors.Project.Member.Invalid") } - err = c.checkUserExists(ctx, addedMember.UserID, "") + _, err = c.checkUserExists(ctx, addedMember.UserID, "") if err != nil { return nil, err } diff --git a/internal/command/user.go b/internal/command/user.go index 6b65aa83ec..7ab2936729 100644 --- a/internal/command/user.go +++ b/internal/command/user.go @@ -327,18 +327,18 @@ func (c *Commands) UserDomainClaimedSent(ctx context.Context, orgID, userID stri return err } -func (c *Commands) checkUserExists(ctx context.Context, userID, resourceOwner string) (err error) { +func (c *Commands) checkUserExists(ctx context.Context, userID, resourceOwner string) (_ string, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() existingUser, err := c.userWriteModelByID(ctx, userID, resourceOwner) if err != nil { - return err + return "", err } if !isUserStateExists(existingUser.UserState) { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-uXHNj", "Errors.User.NotFound") + return "", zerrors.ThrowPreconditionFailed(nil, "COMMAND-uXHNj", "Errors.User.NotFound") } - return nil + return existingUser.ResourceOwner, nil } func (c *Commands) userWriteModelByID(ctx context.Context, userID, resourceOwner string) (writeModel *UserWriteModel, err error) { diff --git a/internal/command/user_grant.go b/internal/command/user_grant.go index 6bb4a20b0a..d0d303b5de 100644 --- a/internal/command/user_grant.go +++ b/internal/command/user_grant.go @@ -299,7 +299,7 @@ func (c *Commands) checkUserGrantPreCondition(ctx context.Context, usergrant *do ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() - if err := c.checkUserExists(ctx, usergrant.UserID, ""); err != nil { + if _, err := c.checkUserExists(ctx, usergrant.UserID, ""); err != nil { return err } existingRoleKeys, err := c.searchUserGrantPreConditionState(ctx, usergrant, resourceOwner) diff --git a/internal/command/user_human_otp.go b/internal/command/user_human_otp.go index 97596aabd8..fca762cbab 100644 --- a/internal/command/user_human_otp.go +++ b/internal/command/user_human_otp.go @@ -26,7 +26,7 @@ func (c *Commands) ImportHumanTOTP(ctx context.Context, userID, userAgentID, res if err != nil { return err } - if err = c.checkUserExists(ctx, userID, resourceOwner); err != nil { + if _, err = c.checkUserExists(ctx, userID, resourceOwner); err != nil { return err } diff --git a/internal/command/user_idp_link.go b/internal/command/user_idp_link.go index 432d2e0b90..e3484c9753 100644 --- a/internal/command/user_idp_link.go +++ b/internal/command/user_idp_link.go @@ -56,7 +56,7 @@ func (c *Commands) BulkAddedUserIDPLinks(ctx context.Context, userID, resourceOw return zerrors.ThrowInvalidArgument(nil, "COMMAND-Ek9s", "Errors.User.ExternalIDP.MinimumExternalIDPNeeded") } - if err := c.checkUserExists(ctx, userID, resourceOwner); err != nil { + if _, err := c.checkUserExists(ctx, userID, resourceOwner); err != nil { return err } diff --git a/internal/command/user_metadata.go b/internal/command/user_metadata.go index d47c5b61d0..1600d1c9e8 100644 --- a/internal/command/user_metadata.go +++ b/internal/command/user_metadata.go @@ -1,6 +1,7 @@ package command import ( + "bytes" "context" "github.com/zitadel/zitadel/internal/domain" @@ -14,12 +15,21 @@ func (c *Commands) SetUserMetadata(ctx context.Context, metadata *domain.Metadat ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() - err = c.checkUserExists(ctx, userID, resourceOwner) + userResourceOwner, err := c.checkUserExists(ctx, userID, resourceOwner) + if err != nil { + return nil, err + } + + setMetadata, err := c.getUserMetadataModelByID(ctx, userID, userResourceOwner, metadata.Key) if err != nil { return nil, err } - setMetadata := NewUserMetadataWriteModel(userID, resourceOwner, metadata.Key) userAgg := UserAggregateFromWriteModel(&setMetadata.WriteModel) + // return if no change in the metadata + if bytes.Equal(setMetadata.Value, metadata.Value) { + return writeModelToUserMetadata(setMetadata), nil + } + event, err := c.setUserMetadata(ctx, userAgg, metadata) if err != nil { return nil, err @@ -40,20 +50,31 @@ func (c *Commands) BulkSetUserMetadata(ctx context.Context, userID, resourceOwne if len(metadatas) == 0 { return nil, zerrors.ThrowPreconditionFailed(nil, "META-9mm2d", "Errors.Metadata.NoData") } - err = c.checkUserExists(ctx, userID, resourceOwner) + userResourceOwner, err := c.checkUserExists(ctx, userID, resourceOwner) if err != nil { return nil, err } - events := make([]eventstore.Command, len(metadatas)) - setMetadata := NewUserMetadataListWriteModel(userID, resourceOwner) + events := make([]eventstore.Command, 0) + setMetadata, err := c.getUserMetadataListModelByID(ctx, userID, userResourceOwner) + if err != nil { + return nil, err + } userAgg := UserAggregateFromWriteModel(&setMetadata.WriteModel) - for i, data := range metadatas { + for _, data := range metadatas { + // if no change to metadata no event has to be pushed + if existingValue, ok := setMetadata.metadataList[data.Key]; ok && bytes.Equal(existingValue, data.Value) { + continue + } event, err := c.setUserMetadata(ctx, userAgg, data) if err != nil { return nil, err } - events[i] = event + events = append(events, event) + } + // no changes for the metadata + if len(events) == 0 { + return writeModelToObjectDetails(&setMetadata.WriteModel), nil } pushedEvents, err := c.eventstore.Push(ctx, events...) @@ -84,11 +105,12 @@ func (c *Commands) RemoveUserMetadata(ctx context.Context, metadataKey, userID, if metadataKey == "" { return nil, zerrors.ThrowInvalidArgument(nil, "META-2n0fs", "Errors.Metadata.Invalid") } - err = c.checkUserExists(ctx, userID, resourceOwner) + userResourceOwner, err := c.checkUserExists(ctx, userID, resourceOwner) if err != nil { return nil, err } - removeMetadata, err := c.getUserMetadataModelByID(ctx, userID, resourceOwner, metadataKey) + + removeMetadata, err := c.getUserMetadataModelByID(ctx, userID, userResourceOwner, metadataKey) if err != nil { return nil, err } @@ -116,13 +138,13 @@ func (c *Commands) BulkRemoveUserMetadata(ctx context.Context, userID, resourceO if len(metadataKeys) == 0 { return nil, zerrors.ThrowPreconditionFailed(nil, "META-9mm2d", "Errors.Metadata.NoData") } - err = c.checkUserExists(ctx, userID, resourceOwner) + userResourceOwner, err := c.checkUserExists(ctx, userID, resourceOwner) if err != nil { return nil, err } events := make([]eventstore.Command, len(metadataKeys)) - removeMetadata, err := c.getUserMetadataListModelByID(ctx, userID, resourceOwner) + removeMetadata, err := c.getUserMetadataListModelByID(ctx, userID, userResourceOwner) if err != nil { return nil, err } @@ -153,24 +175,6 @@ func (c *Commands) BulkRemoveUserMetadata(ctx context.Context, userID, resourceO return writeModelToObjectDetails(&removeMetadata.WriteModel), nil } -func (c *Commands) removeUserMetadataFromOrg(ctx context.Context, resourceOwner string) ([]eventstore.Command, error) { - existingUserMetadata, err := c.getUserMetadataByOrgListModelByID(ctx, resourceOwner) - if err != nil { - return nil, err - } - if len(existingUserMetadata.UserMetadata) == 0 { - return nil, nil - } - events := make([]eventstore.Command, 0) - for key, value := range existingUserMetadata.UserMetadata { - if len(value) == 0 { - continue - } - events = append(events, user.NewMetadataRemovedAllEvent(ctx, &user.NewAggregate(key, resourceOwner).Aggregate)) - } - return events, nil -} - func (c *Commands) removeUserMetadata(ctx context.Context, userAgg *eventstore.Aggregate, metadataKey string) (command eventstore.Command, err error) { command = user.NewMetadataRemovedEvent( ctx, @@ -197,12 +201,3 @@ func (c *Commands) getUserMetadataListModelByID(ctx context.Context, userID, res } return userMetadataWriteModel, nil } - -func (c *Commands) getUserMetadataByOrgListModelByID(ctx context.Context, resourceOwner string) (*UserMetadataByOrgListWriteModel, error) { - userMetadataWriteModel := NewUserMetadataByOrgListWriteModel(resourceOwner) - err := c.eventstore.FilterToQueryReducer(ctx, userMetadataWriteModel) - if err != nil { - return nil, err - } - return userMetadataWriteModel, nil -} diff --git a/internal/command/user_metadata_test.go b/internal/command/user_metadata_test.go index b3ffa7b823..86b4867051 100644 --- a/internal/command/user_metadata_test.go +++ b/internal/command/user_metadata_test.go @@ -16,7 +16,8 @@ import ( func TestCommandSide_SetUserMetadata(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(t *testing.T) *eventstore.Eventstore + checkPermission domain.PermissionCheck } type ( args struct { @@ -39,10 +40,10 @@ func TestCommandSide_SetUserMetadata(t *testing.T) { { name: "user not existing, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter(), ), + checkPermission: newMockPermissionCheckAllowed(), }, args: args{ ctx: context.Background(), @@ -60,8 +61,7 @@ func TestCommandSide_SetUserMetadata(t *testing.T) { { name: "invalid metadata, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -78,7 +78,17 @@ func TestCommandSide_SetUserMetadata(t *testing.T) { ), ), ), + expectFilter( + eventFromEventPusher( + user.NewMetadataSetEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "key", + []byte("value"), + ), + ), + ), ), + checkPermission: newMockPermissionCheckAllowed(), }, args: args{ ctx: context.Background(), @@ -95,8 +105,7 @@ func TestCommandSide_SetUserMetadata(t *testing.T) { { name: "add metadata, ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -113,6 +122,7 @@ func TestCommandSide_SetUserMetadata(t *testing.T) { ), ), ), + expectFilter(), expectPush( user.NewMetadataSetEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, @@ -121,6 +131,164 @@ func TestCommandSide_SetUserMetadata(t *testing.T) { ), ), ), + checkPermission: newMockPermissionCheckAllowed(), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + userID: "user1", + metadata: &domain.Metadata{ + Key: "key", + Value: []byte("value"), + }, + }, + res: res{ + want: &domain.Metadata{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "user1", + ResourceOwner: "org1", + }, + Key: "key", + Value: []byte("value"), + State: domain.MetadataStateActive, + }, + }, + }, + { + name: "add metadata, reset, invalid", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "", + "firstname lastname", + language.Und, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + user.NewMetadataSetEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "key", + []byte("value"), + ), + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + userID: "user1", + metadata: &domain.Metadata{ + Key: "key", + }, + }, + res: res{ + err: zerrors.IsErrorInvalidArgument, + }, + }, + { + name: "add metadata, reset, ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "", + "firstname lastname", + language.Und, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + user.NewMetadataSetEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "key", + []byte("value"), + ), + ), + ), + expectPush( + user.NewMetadataSetEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "key", + []byte("value2"), + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + userID: "user1", + metadata: &domain.Metadata{ + Key: "key", + Value: []byte("value2"), + }, + }, + res: res{ + want: &domain.Metadata{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "user1", + ResourceOwner: "org1", + }, + Key: "key", + Value: []byte("value2"), + State: domain.MetadataStateActive, + }, + }, + }, + { + name: "add metadata, reset, no change", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "", + "firstname lastname", + language.Und, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + user.NewMetadataSetEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "key", + []byte("value"), + ), + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), }, args: args{ ctx: context.Background(), @@ -147,7 +315,8 @@ func TestCommandSide_SetUserMetadata(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), + checkPermission: tt.fields.checkPermission, } got, err := r.SetUserMetadata(tt.args.ctx, tt.args.metadata, tt.args.userID, tt.args.orgID) if tt.res.err == nil { @@ -165,7 +334,8 @@ func TestCommandSide_SetUserMetadata(t *testing.T) { func TestCommandSide_BulkSetUserMetadata(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(t *testing.T) *eventstore.Eventstore + checkPermission domain.PermissionCheck } type ( args struct { @@ -188,9 +358,7 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) { { name: "empty meta data list, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -204,8 +372,7 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) { { name: "user not existing, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter(), ), }, @@ -225,8 +392,7 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) { { name: "invalid metadata, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -243,7 +409,9 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) { ), ), ), + expectFilter(), ), + checkPermission: newMockPermissionCheckAllowed(), }, args: args{ ctx: context.Background(), @@ -261,8 +429,7 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) { { name: "add metadata, ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -279,6 +446,7 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) { ), ), ), + expectFilter(), expectPush( user.NewMetadataSetEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, @@ -292,6 +460,7 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) { ), ), ), + checkPermission: newMockPermissionCheckAllowed(), }, args: args{ ctx: context.Background(), @@ -312,7 +481,8 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), + checkPermission: tt.fields.checkPermission, } got, err := r.BulkSetUserMetadata(tt.args.ctx, tt.args.userID, tt.args.orgID, tt.args.metadataList...) if tt.res.err == nil { @@ -330,7 +500,8 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) { func TestCommandSide_UserRemoveMetadata(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(t *testing.T) *eventstore.Eventstore + checkPermission domain.PermissionCheck } type ( args struct { @@ -353,8 +524,7 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) { { name: "user not existing, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter(), ), }, @@ -371,9 +541,7 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) { { name: "invalid metadata, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -388,8 +556,7 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) { { name: "meta data not existing, not found error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -408,6 +575,7 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) { ), expectFilter(), ), + checkPermission: newMockPermissionCheckAllowed(), }, args: args{ ctx: context.Background(), @@ -422,8 +590,7 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) { { name: "remove metadata, ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -456,6 +623,7 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) { ), ), ), + checkPermission: newMockPermissionCheckAllowed(), }, args: args{ ctx: context.Background(), @@ -473,7 +641,8 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), + checkPermission: tt.fields.checkPermission, } got, err := r.RemoveUserMetadata(tt.args.ctx, tt.args.metadataKey, tt.args.userID, tt.args.orgID) if tt.res.err == nil { @@ -491,7 +660,8 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) { func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(t *testing.T) *eventstore.Eventstore + checkPermission domain.PermissionCheck } type ( args struct { @@ -514,9 +684,7 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) { { name: "empty meta data list, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -530,8 +698,7 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) { { name: "user not existing, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter(), ), }, @@ -548,8 +715,7 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) { { name: "remove metadata keys not existing, precondition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -576,6 +742,7 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) { ), ), ), + checkPermission: newMockPermissionCheckAllowed(), }, args: args{ ctx: context.Background(), @@ -590,8 +757,7 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) { { name: "invalid metadata, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -625,6 +791,7 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) { ), ), ), + checkPermission: newMockPermissionCheckAllowed(), }, args: args{ ctx: context.Background(), @@ -639,8 +806,7 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) { { name: "remove metadata, ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -684,6 +850,7 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) { ), ), ), + checkPermission: newMockPermissionCheckAllowed(), }, args: args{ ctx: context.Background(), @@ -701,7 +868,8 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), + checkPermission: tt.fields.checkPermission, } got, err := r.BulkRemoveUserMetadata(tt.args.ctx, tt.args.userID, tt.args.orgID, tt.args.metadataList...) if tt.res.err == nil {