From 189f9770c6e1314a997c837e723e4f3527633ebd Mon Sep 17 00:00:00 2001 From: Lars Date: Mon, 27 Jan 2025 13:36:07 +0100 Subject: [PATCH] feat: patch user scim v2 endpoint (#9219) # Which Problems Are Solved * Adds support for the patch user SCIM v2 endpoint # How the Problems Are Solved * Adds support for the patch user SCIM v2 endpoint under `PATCH /scim/v2/{orgID}/Users/{id}` # Additional Context Part of #8140 --- internal/api/scim/authz.go | 3 + .../testdata/users_update_test_full.json | 139 +++ .../integration_test/users_create_test.go | 1 + .../integration_test/users_replace_test.go | 1 + .../integration_test/users_update_test.go | 281 +++++ internal/api/scim/metadata/metadata.go | 69 +- .../resources/filter/attribute_resolver.go | 110 ++ .../filter/attribute_resolver_test.go | 137 +++ .../scim/resources/filter/filter_evaluator.go | 338 ++++++ .../api/scim/resources/filter/path_parser.go | 107 ++ .../scim/resources/filter/path_parser_test.go | 160 +++ internal/api/scim/resources/patch/patch.go | 273 +++++ .../api/scim/resources/patch/patch_add.go | 112 ++ .../api/scim/resources/patch/patch_remove.go | 73 ++ .../api/scim/resources/patch/patch_replace.go | 105 ++ .../api/scim/resources/resource_handler.go | 2 + .../resources/resource_handler_adapter.go | 25 + internal/api/scim/resources/user.go | 34 +- internal/api/scim/resources/user_mapping.go | 206 ++-- internal/api/scim/resources/user_metadata.go | 7 +- internal/api/scim/resources/user_patch.go | 111 ++ .../api/scim/resources/user_patch_test.go | 984 ++++++++++++++++++ internal/api/scim/schemas/schemas.go | 1 + internal/api/scim/serrors/errors.go | 22 + internal/api/scim/server.go | 1 + internal/command/phone.go | 16 + internal/command/user_v2_human.go | 37 +- internal/command/user_v2_model.go | 35 +- internal/command/user_v2_model_test.go | 313 +++++- internal/integration/scim/assertions.go | 5 +- internal/integration/scim/client.go | 18 +- 31 files changed, 3601 insertions(+), 125 deletions(-) create mode 100644 internal/api/scim/integration_test/testdata/users_update_test_full.json create mode 100644 internal/api/scim/integration_test/users_update_test.go create mode 100644 internal/api/scim/resources/filter/attribute_resolver.go create mode 100644 internal/api/scim/resources/filter/attribute_resolver_test.go create mode 100644 internal/api/scim/resources/filter/filter_evaluator.go create mode 100644 internal/api/scim/resources/filter/path_parser.go create mode 100644 internal/api/scim/resources/filter/path_parser_test.go create mode 100644 internal/api/scim/resources/patch/patch.go create mode 100644 internal/api/scim/resources/patch/patch_add.go create mode 100644 internal/api/scim/resources/patch/patch_remove.go create mode 100644 internal/api/scim/resources/patch/patch_replace.go create mode 100644 internal/api/scim/resources/user_patch.go create mode 100644 internal/api/scim/resources/user_patch_test.go diff --git a/internal/api/scim/authz.go b/internal/api/scim/authz.go index 55b29a974..245c8c7a5 100644 --- a/internal/api/scim/authz.go +++ b/internal/api/scim/authz.go @@ -22,6 +22,9 @@ var AuthMapping = authz.MethodMapping{ "PUT:/scim/v2/" + http.OrgIdInPathVariable + "/Users/{id}": { Permission: domain.PermissionUserWrite, }, + "PATCH:/scim/v2/" + http.OrgIdInPathVariable + "/Users/{id}": { + Permission: domain.PermissionUserWrite, + }, "DELETE:/scim/v2/" + http.OrgIdInPathVariable + "/Users/{id}": { Permission: domain.PermissionUserDelete, }, 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 new file mode 100644 index 000000000..33b05a1f7 --- /dev/null +++ b/internal/api/scim/integration_test/testdata/users_update_test_full.json @@ -0,0 +1,139 @@ +{ + "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + "Operations": [ + // add without path + { + "op": "add", + "value": { + "emails":[ + { + "value":"babs@example.com", + "type":"home", + "primary": true + } + ], + "nickname":"added-babs" + } + }, + // add complex attribute with path + { + "op": "add", + "path": "name", + "value": { + "formatted": "added-formatted", + "familyName": "added-family-name", + "givenName": "added-given-name", + "middleName": "added-middle-name", + "honorificPrefix": "added-honorific-prefix", + "honorificSuffix": "added-honorific-suffix" + } + }, + // add complex attribute value + { + "op": "add", + "path": "name.middlename", + "value": "added-middle-name-2" + }, + // add single to multi valued attribute + { + "op": "add", + "path": "entitlements", + "value": { "value": "added-entitlement-1" } + }, + // add single already existing to multi valued attribute + // (should be replaced) + { + "op": "add", + "path": "entitlements", + "value": { + "value": "my-entitlement-1", + "display": "added-entitlement-1", + "type": "added-entitlement-1", + "primary": true + } + }, + // add multiple to multi valued attribute, + // with one item already existing (should be replaced) + { + "op": "add", + "path": "entitlements", + "value": [ + { "value": "added-entitlement-2" }, + { "value": "added-entitlement-3", "primary": true } + ] + }, + // remove single valued attribute + { + "op": "remove", + "path": "nickname" + }, + // remove multi valued attribute + { + "op": "remove", + "path": "roles" + }, + // remove multi valued attribute with filter + { + "op": "remove", + "path": "photos[type eq \"thumbnail\"]" + }, + // remove attribute of multi valued attribute with filter + { + "op": "remove", + "path": "ims[type eq \"X\"].type" + }, + // remove multi valued attribute with non-matching filter + { + "op": "remove", + "path": "ims[type eq \"fooBar\"].type" + }, + // replace without path + { + "op": "replace", + "value": { + "displayName": "replaced-display-name" + } + }, + // replace nested with path + { + "op": "replace", + "path": "name.honorificSuffix", + "value": "replaced-honorific-suffix" + }, + // replace complex multi attribute + { + "op": "replace", + "path": "addresses", + "value": [ + { + "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 + } + ] + }, + // replace phone + { + "op": "replace", + "path": "phonenumbers[primary eq true].value", + "value": "+41711234567" + }, + // replace externalID + { + "op": "replace", + "path": "externalid", + "value": "fooBAR" + }, + // replace password + { + "op": "replace", + "path": "password", + "value": "Password2!" + } + ] +} \ No newline at end of file diff --git a/internal/api/scim/integration_test/users_create_test.go b/internal/api/scim/integration_test/users_create_test.go index 8e70c91af..7763fdc17 100644 --- a/internal/api/scim/integration_test/users_create_test.go +++ b/internal/api/scim/integration_test/users_create_test.go @@ -265,6 +265,7 @@ func TestCreateUser(t *testing.T) { createdUser, err := Instance.Client.SCIM.Users.Create(ctx, Instance.DefaultOrg.Id, tt.body) if (err != nil) != tt.wantErr { t.Errorf("CreateUser() error = %v, wantErr %v", err, tt.wantErr) + return } if err != nil { diff --git a/internal/api/scim/integration_test/users_replace_test.go b/internal/api/scim/integration_test/users_replace_test.go index 69f1535e9..e5b0e1443 100644 --- a/internal/api/scim/integration_test/users_replace_test.go +++ b/internal/api/scim/integration_test/users_replace_test.go @@ -233,6 +233,7 @@ func TestReplaceUser(t *testing.T) { if statusCode == 0 { statusCode = http.StatusBadRequest } + scimErr := scim.RequireScimError(t, statusCode, err) assert.Equal(t, tt.scimErrorType, scimErr.Error.ScimType) if tt.zitadelErrID != "" { diff --git a/internal/api/scim/integration_test/users_update_test.go b/internal/api/scim/integration_test/users_update_test.go new file mode 100644 index 000000000..9fc78b928 --- /dev/null +++ b/internal/api/scim/integration_test/users_update_test.go @@ -0,0 +1,281 @@ +//go:build integration + +package integration_test + +import ( + "context" + _ "embed" + "fmt" + "net/http" + "regexp" + "testing" + "time" + + "github.com/brianvoe/gofakeit/v6" + "github.com/muhlemmer/gu" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/text/language" + + "github.com/zitadel/zitadel/internal/api/scim/resources" + "github.com/zitadel/zitadel/internal/api/scim/schemas" + "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 ( + //go:embed testdata/users_update_test_full.json + fullUserUpdateJson []byte + + minimalUserUpdateJson []byte = simpleReplacePatchBody("nickname", "foo") + + // remove comments in the json, as the default golang json unmarshaler cannot handle them + // the test file is much easier to maintain with comments + removeCommentsRegex = regexp.MustCompile("(?s)//.*?\n|/\\*.*?\\*/") +) + +func init() { + fullUserUpdateJson = removeComments(fullUserUpdateJson) +} + +func removeComments(json []byte) []byte { + return removeCommentsRegex.ReplaceAll(json, nil) +} + +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) + }() + + iamOwnerCtx := Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner) + secondaryOrg := Instance.CreateOrganization(iamOwnerCtx, gofakeit.Name(), gofakeit.Email()) + + tests := []struct { + name string + body []byte + ctx context.Context + orgID string + userID string + want *resources.ScimUser + wantErr bool + scimErrorType string + errorStatus int + }{ + { + name: "not authenticated", + 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: "other org", + orgID: secondaryOrg.OrganizationId, + body: minimalUserUpdateJson, + wantErr: true, + errorStatus: http.StatusNotFound, + }, + { + name: "invalid patch json", + body: simpleReplacePatchBody("nickname", "10"), + wantErr: true, + scimErrorType: "invalidValue", + }, + { + name: "password complexity violation", + 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 time zone", + body: simpleReplacePatchBody("timezone", `"foobar"`), + wantErr: true, + scimErrorType: "invalidValue", + }, + { + name: "invalid locale", + body: simpleReplacePatchBody("locale", `"foobar"`), + wantErr: true, + scimErrorType: "invalidValue", + }, + { + name: "unknown user id", + 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: "babs@example.com", + Primary: true, + }, + }, + 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: gu.Ptr(true), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.ctx == nil { + tt.ctx = CTX + } + + if tt.orgID == "" { + tt.orgID = Instance.DefaultOrg.Id + } + + if tt.userID == "" { + tt.userID = fullUserCreated.ID + } + + err := Instance.Client.SCIM.Users.Update(tt.ctx, tt.orgID, tt.userID, tt.body) + + if tt.wantErr { + require.Error(t, err) + + statusCode := tt.errorStatus + if statusCode == 0 { + statusCode = http.StatusBadRequest + } + + scimErr := scim.RequireScimError(t, statusCode, err) + assert.Equal(t, tt.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) + + fetchedUser.Resource = nil + fetchedUser.ID = "" + if tt.want != nil && !test.PartiallyDeepEqual(tt.want, fetchedUser) { + ttt.Errorf("got = %#v, want = %#v", fetchedUser, tt.want) + } + }, retryDuration, tick) + }) + } +} + +func simpleReplacePatchBody(path, value string) []byte { + return []byte(fmt.Sprintf( + `{ + "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + "Operations": [ + { + "op": "replace", + "path": "%s", + "value": %s + } + ] + }`, + path, + value, + )) +} diff --git a/internal/api/scim/metadata/metadata.go b/internal/api/scim/metadata/metadata.go index 626d93823..09391cab3 100644 --- a/internal/api/scim/metadata/metadata.go +++ b/internal/api/scim/metadata/metadata.go @@ -3,6 +3,8 @@ package metadata import ( "context" "strings" + + "github.com/zitadel/zitadel/internal/query" ) type Key string @@ -30,21 +32,40 @@ const ( KeyRoles Key = KeyPrefix + "roles" ) -var ScimUserRelevantMetadataKeys = []Key{ - KeyExternalId, - KeyMiddleName, - KeyHonorificPrefix, - KeyHonorificSuffix, - KeyProfileUrl, - KeyTitle, - KeyLocale, - KeyTimezone, - KeyIms, - KeyPhotos, - KeyAddresses, - KeyEntitlements, - KeyRoles, -} +var ( + ScimUserRelevantMetadataKeys = []Key{ + KeyExternalId, + KeyMiddleName, + KeyHonorificPrefix, + KeyHonorificSuffix, + KeyProfileUrl, + KeyTitle, + KeyLocale, + KeyTimezone, + KeyIms, + KeyPhotos, + KeyAddresses, + KeyEntitlements, + KeyRoles, + } + + AttributePathToMetadataKeys = map[string][]Key{ + "externalid": {KeyExternalId}, + "name": {KeyMiddleName, KeyHonorificPrefix, KeyHonorificSuffix}, + "name.middlename": {KeyMiddleName}, + "name.honorificprefix": {KeyHonorificPrefix}, + "name.honorificsuffix": {KeyHonorificSuffix}, + "profileurl": {KeyProfileUrl}, + "title": {KeyTitle}, + "locale": {KeyLocale}, + "timezone": {KeyTimezone}, + "ims": {KeyIms}, + "photos": {KeyPhotos}, + "addresses": {KeyAddresses}, + "entitlements": {KeyEntitlements}, + "roles": {KeyRoles}, + } +) func ScopeExternalIdKey(provisioningDomain string) ScopedKey { return ScopedKey(strings.Replace(keyScopedExternalIdTemplate, externalIdProvisioningDomainPlaceholder, provisioningDomain, 1)) @@ -58,3 +79,21 @@ func ScopeKey(ctx context.Context, key Key) ScopedKey { return ScopedKey(key) } + +func MapToScopedKeyMap(md map[string][]byte) map[ScopedKey][]byte { + result := make(map[ScopedKey][]byte, len(md)) + for k, v := range md { + result[ScopedKey(k)] = v + } + + return result +} + +func MapListToScopedKeyMap(metadataList []*query.UserMetadata) map[ScopedKey][]byte { + metadataMap := make(map[ScopedKey][]byte, len(metadataList)) + for _, entry := range metadataList { + metadataMap[ScopedKey(entry.Key)] = entry.Value + } + + return metadataMap +} diff --git a/internal/api/scim/resources/filter/attribute_resolver.go b/internal/api/scim/resources/filter/attribute_resolver.go new file mode 100644 index 000000000..484705410 --- /dev/null +++ b/internal/api/scim/resources/filter/attribute_resolver.go @@ -0,0 +1,110 @@ +package filter + +import ( + "reflect" + "strings" + "sync" + + "github.com/zitadel/zitadel/internal/api/scim/schemas" + "github.com/zitadel/zitadel/internal/zerrors" +) + +// jsonFieldCache Cache storing JSON tag to field mappings for each reflect.Type +// keep this in memory forever, as the fields / json tags are constant at runtime and never change. +// reflect.Type => structFieldCache +var jsonFieldCache sync.Map + +type structFieldCache map[string]reflect.StructField + +type AttributeResolver struct { + schema schemas.ScimSchemaType +} + +func (c structFieldCache) get(name string) (reflect.StructField, error) { + field, ok := c[strings.ToLower(name)] + if !ok { + return reflect.StructField{}, zerrors.ThrowInvalidArgumentf(nil, "SCIM-attr12", "SCIM Attribute not found %s", name) + } + + return field, nil +} + +func (c structFieldCache) set(field reflect.StructField) { + jsonTag := field.Tag.Get("json") + + // Skip fields explicitly excluded + if jsonTag == "-" { + return + } + + // use field name as default + fieldName := field.Name + if jsonTag != "" { + // strip other options such as omitempty + fieldName = strings.Split(jsonTag, ",")[0] + } + + c[strings.ToLower(fieldName)] = field +} + +func newAttributeResolver(schema schemas.ScimSchemaType) *AttributeResolver { + return &AttributeResolver{ + schema: schema, + } +} + +func (r *AttributeResolver) resolveAttrPath(item reflect.Value, attrPath *AttrPath) ([]string, reflect.Value, error) { + if err := attrPath.validateSchema(r.schema); err != nil { + return nil, reflect.Value{}, err + } + + segments := attrPath.Segments() + for _, segment := range segments { + var err error + item, err = r.resolveField(item, segment) + if err != nil { + return nil, reflect.Value{}, err + } + } + + return segments, item, nil +} + +func (r *AttributeResolver) resolveField(item reflect.Value, fieldName string) (reflect.Value, error) { + if item.Kind() == reflect.Ptr { + item = item.Elem() + } + + fields, err := r.getOrBuildFieldMap(item.Type()) + if err != nil { + return reflect.Value{}, err + } + + field, err := fields.get(fieldName) + if err != nil { + return reflect.Value{}, err + } + + resolvedField := item.FieldByName(field.Name) + if !resolvedField.IsValid() { + return reflect.Value{}, zerrors.ThrowInvalidArgumentf(nil, "SCIM-attr13", "SCIM Attribute not found %s", fieldName) + } + + return resolvedField, nil +} + +func (r *AttributeResolver) getOrBuildFieldMap(t reflect.Type) (structFieldCache, error) { + if cached, ok := jsonFieldCache.Load(t); ok { + return cached.(structFieldCache), nil + } + + // cache miss, build json name field map + fieldMap := make(structFieldCache, t.NumField()) + for i := 0; i < t.NumField(); i++ { + fieldMap.set(t.Field(i)) + } + + // Cache the result for future use. + jsonFieldCache.Store(t, fieldMap) + return fieldMap, nil +} diff --git a/internal/api/scim/resources/filter/attribute_resolver_test.go b/internal/api/scim/resources/filter/attribute_resolver_test.go new file mode 100644 index 000000000..119024291 --- /dev/null +++ b/internal/api/scim/resources/filter/attribute_resolver_test.go @@ -0,0 +1,137 @@ +package filter + +import ( + "reflect" + "testing" + + "github.com/muhlemmer/gu" + + "github.com/zitadel/zitadel/internal/api/scim/schemas" +) + +type attributeResolverTestType struct { + StringValue string + Nested struct { + IntValue int + } +} + +func TestAttributeResolver_resolveAttrPath(t *testing.T) { + tests := []struct { + name string + schema schemas.ScimSchemaType + item interface{} + attrPath *AttrPath + wantSegments []string + wantValue interface{} + wantErr bool + }{ + { + name: "simple", + schema: "fooBar", + item: &attributeResolverTestType{StringValue: "value"}, + attrPath: &AttrPath{ + AttrName: "StringValue", + }, + wantSegments: []string{"stringvalue"}, + wantValue: "value", + }, + { + name: "simple case insensitive", + schema: "fooBar", + item: &attributeResolverTestType{StringValue: "value"}, + attrPath: &AttrPath{ + AttrName: "stringvalue", + }, + wantSegments: []string{"stringvalue"}, + wantValue: "value", + }, + { + name: "invalid attribute", + schema: "fooBar", + item: &attributeResolverTestType{StringValue: "value"}, + attrPath: &AttrPath{ + AttrName: "StringValue2", + }, + wantErr: true, + }, + { + name: "with SubAttr", + schema: "fooBar", + item: &attributeResolverTestType{Nested: struct{ IntValue int }{IntValue: 42}}, + attrPath: &AttrPath{ + AttrName: "Nested", + SubAttr: gu.Ptr("IntValue"), + }, + wantSegments: []string{"nested", "intvalue"}, + wantValue: 42, + }, + { + name: "with case-insensitive SubAttr", + schema: "fooBar", + item: &attributeResolverTestType{Nested: struct{ IntValue int }{IntValue: 42}}, + attrPath: &AttrPath{ + AttrName: "NESTED", + SubAttr: gu.Ptr("intvalue"), + }, + wantSegments: []string{"nested", "intvalue"}, + wantValue: 42, + }, + { + name: "with invalid SubAttr", + schema: "fooBar", + item: &attributeResolverTestType{Nested: struct{ IntValue int }{IntValue: 42}}, + attrPath: &AttrPath{ + AttrName: "NESTED", + SubAttr: gu.Ptr("xxxxx"), + }, + wantErr: true, + }, + { + name: "with SubAttr and urn", + schema: "fooBar", + item: &attributeResolverTestType{Nested: struct{ IntValue int }{IntValue: 42}}, + attrPath: &AttrPath{ + UrnAttributePrefix: gu.Ptr("fooBar:"), + AttrName: "Nested", + SubAttr: gu.Ptr("IntValue"), + }, + wantSegments: []string{"nested", "intvalue"}, + wantValue: 42, + }, + { + name: "with SubAttr and invalid urn", + schema: "fooBar", + item: &attributeResolverTestType{Nested: struct{ IntValue int }{IntValue: 42}}, + attrPath: &AttrPath{ + UrnAttributePrefix: gu.Ptr("fooBaz:"), + AttrName: "Nested", + SubAttr: gu.Ptr("IntValue"), + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := newAttributeResolver(tt.schema) + + gotSegments, gotValue, err := r.resolveAttrPath(reflect.ValueOf(tt.item), tt.attrPath) + if (err != nil) != tt.wantErr { + t.Errorf("resolveAttrPath() error = %#v, wantErr %#v", err, tt.wantErr) + return + } + + if err != nil { + return + } + + if !reflect.DeepEqual(gotSegments, tt.wantSegments) { + t.Errorf("resolveAttrPath() gotSegments = %#v, want %#v", gotSegments, tt.wantSegments) + } + + if !reflect.DeepEqual(gotValue.Interface(), tt.wantValue) { + t.Errorf("resolveAttrPath() gotValue = %#v, want %#v", gotValue, tt.wantValue) + } + }) + } +} diff --git a/internal/api/scim/resources/filter/filter_evaluator.go b/internal/api/scim/resources/filter/filter_evaluator.go new file mode 100644 index 000000000..7b00b21b5 --- /dev/null +++ b/internal/api/scim/resources/filter/filter_evaluator.go @@ -0,0 +1,338 @@ +package filter + +import ( + "reflect" + "strings" + + "github.com/zitadel/logging" + + "github.com/zitadel/zitadel/internal/api/scim/schemas" + "github.com/zitadel/zitadel/internal/zerrors" +) + +type Evaluator struct { + schema schemas.ScimSchemaType + attributeResolver *AttributeResolver +} + +type SimpleValueEvaluationResult struct { + PathSegments []string + Value reflect.Value +} + +type FilteredValuesEvaluationResult struct { + PathSegments []string + Source reflect.Value + Matches []*FilteredValuesEvaluationResultMatch +} + +type FilteredValuesEvaluationResultMatch struct { + // Value the selected value of the Element. Can be the Element itself or an attribute of the Element. + Value reflect.Value + Element reflect.Value + SourceIndex int +} + +type EvaluationResult interface{} + +func NewEvaluator(schema schemas.ScimSchemaType) *Evaluator { + return &Evaluator{ + schema: schema, + attributeResolver: newAttributeResolver(schema), + } +} + +func (e *Evaluator) Evaluate(v reflect.Value, path *Path) (EvaluationResult, error) { + switch { + case path == nil: + return &SimpleValueEvaluationResult{ + PathSegments: nil, + Value: v, + }, nil + case path.AttrPath != nil: + segments, value, err := e.attributeResolver.resolveAttrPath(v, path.AttrPath) + if err != nil { + return nil, err + } + + return &SimpleValueEvaluationResult{ + PathSegments: segments, + Value: value, + }, nil + } + + return e.filterByValuePath(v, path.ValuePath) +} + +func (e *Evaluator) filterByValuePath(v reflect.Value, valuePath *ValuePathWithSubAttr) (*FilteredValuesEvaluationResult, error) { + segments, value, err := e.attributeResolver.resolveAttrPath(v, &valuePath.ValuePath.AttrPath) + if err != nil { + return nil, err + } + + // filtering does only work on slices + if value.Kind() != reflect.Slice { + return nil, zerrors.ThrowInvalidArgument(nil, "SCIM-21x2", "value path target must be a slice") + } + + // apply the filter + var matches []*FilteredValuesEvaluationResultMatch + for i := 0; i < value.Len(); i++ { + element := value.Index(i) + if match, err := e.evaluateOr(element, &valuePath.ValuePath.ValFilter); err != nil { + return nil, err + } else if match && valuePath.SubAttr != nil { + subElement, err := e.attributeResolver.resolveField(element, *valuePath.SubAttr) + if err != nil { + return nil, err + } + + matches = append(matches, &FilteredValuesEvaluationResultMatch{ + Value: subElement, + Element: element, + SourceIndex: i, + }) + } else if match { + matches = append(matches, &FilteredValuesEvaluationResultMatch{ + Value: element, + Element: element, + SourceIndex: i, + }) + } + } + + if valuePath.SubAttr != nil { + segments = append(segments, *valuePath.SubAttr) + } + + return &FilteredValuesEvaluationResult{ + PathSegments: segments, + Source: value, + Matches: matches, + }, nil +} + +func (e *Evaluator) evaluateOr(item reflect.Value, or *OrLogExp) (bool, error) { + if match, err := e.evaluateAnd(item, &or.Left); err != nil { + return false, err + } else if match { + return true, nil + } + + if or.Right != nil { + return e.evaluateOr(item, or.Right) + } + + return false, nil +} + +func (e *Evaluator) evaluateAnd(item reflect.Value, and *AndLogExp) (bool, error) { + if match, err := e.evaluateValueAtom(item, &and.Left); err != nil { + return false, err + } else if !match { + return false, nil + } + + if and.Right != nil { + return e.evaluateAnd(item, and.Right) + } + + return true, nil +} + +func (e *Evaluator) evaluateValueAtom(item reflect.Value, atom *ValueAtom) (bool, error) { + switch { + case atom.SubFilter != nil: + return e.evaluateOr(item, &atom.SubFilter.OrExp) + case atom.Negation != nil: + if match, err := e.evaluateOr(item, &atom.Negation.OrExp); err != nil { + return false, err + } else { + return !match, nil + } + case atom.AttrExp != nil: + return e.evaluateAttrExp(item, atom.AttrExp) + } + + // atom.ValuePath is not evaluated since nested valuePaths are not supported + logging.Warn("Encountered unsupported nested value path") + return false, zerrors.ThrowInvalidArgument(nil, "SCIM-21x7", "nested value paths are not supported") +} + +func (e *Evaluator) evaluateAttrExp(item reflect.Value, attrExp *AttrExp) (bool, error) { + if attrExp.UnaryCondition != nil { + return e.evaluateUnaryCondition(item, attrExp.UnaryCondition) + } + + return e.evaluateBinaryCondition(item, attrExp.BinaryCondition) +} + +func (e *Evaluator) evaluateUnaryCondition(item reflect.Value, condition *UnaryCondition) (bool, error) { + _, field, err := e.attributeResolver.resolveAttrPath(item, &condition.Left) + if err != nil { + return false, err + } + + if field.Kind() == reflect.Ptr { + return !field.IsZero(), nil + } + + return !field.IsZero(), nil +} + +func (e *Evaluator) evaluateBinaryCondition(item reflect.Value, condition *BinaryCondition) (bool, error) { + _, field, err := e.attributeResolver.resolveAttrPath(item, &condition.Left) + if err != nil { + return false, err + } + + return e.compareValues(field, &condition.Operator, &condition.Right) +} + +func (e *Evaluator) compareValues(value reflect.Value, op *CompareOp, compValue *CompValue) (bool, error) { + if (op.Equal) && compValue.Null { + return value.IsNil() || value.IsZero(), nil + } + + if op.NotEqual && compValue.Null { + return !value.IsNil() && !value.IsZero(), nil + } + + //nolint:exhaustive + switch value.Kind() { + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + return e.compareUIntValue(value, op, compValue) + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + return e.compareIntValue(value, op, compValue) + case reflect.Float64, reflect.Float32: + return e.compareFloatValue(value, op, compValue) + case reflect.String: + return e.compareString(value, op, compValue) + case reflect.Bool: + return e.compareBool(value, op, compValue) + + // for complex attributes, compare the "value" by default as required by the rfc + case reflect.Struct: + if defaultValue, err := e.attributeResolver.resolveField(value, "value"); err == nil { + return e.compareValues(defaultValue, op, compValue) + } + } + + logging.WithFields("kind", value.Kind()).Warn("Encountered unsupported nested value path") + return false, zerrors.ThrowInvalidArgument(nil, "SCIM-23x8", "unsupported filter path value type") +} + +func (e *Evaluator) compareUIntValue(value reflect.Value, op *CompareOp, compValue *CompValue) (bool, error) { + if compValue.Int == nil { + return false, zerrors.ThrowInvalidArgument(nil, "SCIM-23x8", "Invalid comparison type, expected integer") + } + + intCompValue := uint64(*compValue.Int) + v := value.Uint() + switch { + case op.Equal: + return v == intCompValue, nil + case op.NotEqual: + return v != intCompValue, nil + case op.GreaterThan: + return v > intCompValue, nil + case op.GreaterThanOrEqual: + return v >= intCompValue, nil + case op.LessThan: + return v < intCompValue, nil + case op.LessThanOrEqual: + return v <= intCompValue, nil + } + + return false, zerrors.ThrowInvalidArgumentf(nil, "SCIM-24x8", "Invalid comparison operator %s, not supported for number comparisons", op.String()) +} + +func (e *Evaluator) compareIntValue(value reflect.Value, op *CompareOp, compValue *CompValue) (bool, error) { + if compValue.Int == nil { + return false, zerrors.ThrowInvalidArgument(nil, "SCIM-23x9", "Invalid comparison type, expected integer") + } + + intCompValue := int64(*compValue.Int) + v := value.Int() + switch { + case op.Equal: + return v == intCompValue, nil + case op.NotEqual: + return v != intCompValue, nil + case op.GreaterThan: + return v > intCompValue, nil + case op.GreaterThanOrEqual: + return v >= intCompValue, nil + case op.LessThan: + return v < intCompValue, nil + case op.LessThanOrEqual: + return v <= intCompValue, nil + } + + return false, zerrors.ThrowInvalidArgumentf(nil, "SCIM-24x7", "Invalid comparison operator %s, not supported for number comparisons", op.String()) +} + +func (e *Evaluator) compareFloatValue(value reflect.Value, op *CompareOp, compValue *CompValue) (bool, error) { + var floatCompValue float64 + switch { + case compValue.Float != nil: + floatCompValue = *compValue.Float + case compValue.Int != nil: + floatCompValue = float64(*compValue.Int) + default: + return false, zerrors.ThrowInvalidArgument(nil, "SCIM-24x1", "Invalid comparison type, expected number") + } + + v := value.Float() + switch { + case op.Equal: + return v == floatCompValue, nil + case op.NotEqual: + return v != floatCompValue, nil + case op.GreaterThan: + return v > floatCompValue, nil + case op.GreaterThanOrEqual: + return v >= floatCompValue, nil + case op.LessThan: + return v < floatCompValue, nil + case op.LessThanOrEqual: + return v <= floatCompValue, nil + } + + return false, zerrors.ThrowInvalidArgumentf(nil, "SCIM-24x6", "Invalid comparison operator %s, not supported for number comparisons", op.String()) +} + +func (e *Evaluator) compareString(value reflect.Value, op *CompareOp, compValue *CompValue) (bool, error) { + if compValue.StringValue == nil { + return false, zerrors.ThrowInvalidArgument(nil, "SCIM-24x2", "Invalid comparison type, expected string") + } + + strCompValue := *compValue.StringValue + v := value.String() + switch { + case op.Equal: + return v == strCompValue, nil + case op.NotEqual: + return v != strCompValue, nil + case op.Contains: + return strings.Contains(v, strCompValue), nil + case op.StartsWith: + return strings.HasPrefix(v, strCompValue), nil + case op.EndsWith: + return strings.HasSuffix(v, strCompValue), nil + } + + return false, zerrors.ThrowInvalidArgumentf(nil, "SCIM-24x5", "Invalid comparison operator %s, not supported for string comparisons", op.String()) +} + +func (e *Evaluator) compareBool(value reflect.Value, op *CompareOp, compValue *CompValue) (bool, error) { + if !compValue.BooleanTrue && !compValue.BooleanFalse { + return false, zerrors.ThrowInvalidArgument(nil, "SCIM-24x3", "Invalid comparison type, expected bool") + } + + if !op.Equal && !op.NotEqual { + return false, zerrors.ThrowInvalidArgumentf(nil, "SCIM-24x4", "Invalid comparison operator %s, expected eq or ne", op.String()) + } + + return value.Bool() == (op.Equal && compValue.BooleanTrue), nil +} diff --git a/internal/api/scim/resources/filter/path_parser.go b/internal/api/scim/resources/filter/path_parser.go new file mode 100644 index 000000000..8e1cde3e1 --- /dev/null +++ b/internal/api/scim/resources/filter/path_parser.go @@ -0,0 +1,107 @@ +package filter + +import ( + "encoding/json" + "fmt" + + "github.com/zitadel/logging" + + "github.com/zitadel/zitadel/internal/api/scim/schemas" + "github.com/zitadel/zitadel/internal/api/scim/serrors" +) + +// Path The root ast node for the path grammar +// PATH = attrPath / valuePath [subAttr] +type Path struct { + ValuePath *ValuePathWithSubAttr `parser:"@@ |"` + AttrPath *AttrPath `parser:"@@"` +} + +type ValuePathWithSubAttr struct { + ValuePath ValuePath `parser:"@@"` + SubAttr *string `parser:"('.' @AttrName)?"` +} + +var scimPathParser = buildParser[Path]() + +func ParsePath(path string) (*Path, error) { + if path == "" { + return nil, nil + } + + if len(path) > maxInputLength { + logging.WithFields("len", len(path)).Infof("scim: path exceeds maximum allowed length: %d", maxInputLength) + return nil, serrors.ThrowInvalidFilter(fmt.Errorf("path exceeds maximum allowed length: %d", maxInputLength)) + } + + parsedPath, err := scimPathParser.ParseString("", path) + if err != nil { + logging.WithError(err).Info("scim: failed to parse path") + return nil, serrors.ThrowInvalidFilter(err) + } + + return parsedPath, nil +} + +func (p *Path) UnmarshalJSON(data []byte) error { + var rawPath string + if err := json.Unmarshal(data, &rawPath); err != nil { + return err + } + + if rawPath == "" { + return nil + } + + parsedPath, err := ParsePath(rawPath) + if err != nil { + return err + } + + *p = *parsedPath + return nil +} + +func (p *Path) String() string { + if p.ValuePath != nil { + return p.ValuePath.String() + } + + return p.AttrPath.String() +} + +func (p *Path) IsZero() bool { + return p == nil || *p == Path{} +} + +func (p *Path) Segments(schema schemas.ScimSchemaType) ([]string, error) { + if p.ValuePath != nil { + return p.ValuePath.Segments(schema) + } + + if err := p.AttrPath.validateSchema(schema); err != nil { + return nil, err + } + return p.AttrPath.Segments(), nil +} + +func (v *ValuePathWithSubAttr) String() string { + if v.SubAttr != nil { + return v.ValuePath.String() + "." + *v.SubAttr + } + + return v.ValuePath.String() +} + +func (v *ValuePathWithSubAttr) Segments(schema schemas.ScimSchemaType) ([]string, error) { + if err := v.ValuePath.AttrPath.validateSchema(schema); err != nil { + return nil, err + } + + segments := v.ValuePath.AttrPath.Segments() + if v.SubAttr != nil { + segments = append(segments, *v.SubAttr) + } + + return segments, nil +} diff --git a/internal/api/scim/resources/filter/path_parser_test.go b/internal/api/scim/resources/filter/path_parser_test.go new file mode 100644 index 000000000..81f2d8781 --- /dev/null +++ b/internal/api/scim/resources/filter/path_parser_test.go @@ -0,0 +1,160 @@ +package filter + +import ( + "reflect" + "testing" + + "github.com/muhlemmer/gu" +) + +func TestParsePath(t *testing.T) { + tests := []struct { + name string + path string + want *Path + wantErr bool + }{ + { + name: "empty", + path: "", + }, + { + name: "too long", + path: longString, + wantErr: true, + }, + { + name: "invalid syntax", + path: "fooBar[['baz']]", + wantErr: true, + }, + + // test cases from https://datatracker.ietf.org/doc/html/rfc7644#section-3.5.2 + { + name: "simple", + path: "members", + want: &Path{ + AttrPath: &AttrPath{ + AttrName: "members", + }, + }, + }, + { + name: "nested", + path: "name.familyName", + want: &Path{ + AttrPath: &AttrPath{ + AttrName: "name", + SubAttr: gu.Ptr("familyName"), + }, + }, + }, + { + name: "with filter", + path: `addresses[type eq "work"]`, + want: &Path{ + ValuePath: &ValuePathWithSubAttr{ + ValuePath: ValuePath{ + AttrPath: AttrPath{ + AttrName: "addresses", + }, + ValFilter: OrLogExp{ + Left: AndLogExp{ + Left: ValueAtom{ + AttrExp: &AttrExp{ + BinaryCondition: &BinaryCondition{ + Left: AttrPath{ + AttrName: "type", + }, + Operator: CompareOp{ + Equal: true, + }, + Right: CompValue{ + StringValue: gu.Ptr("work"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "with filter and submember", + path: `members[value pr].displayName`, + want: &Path{ + ValuePath: &ValuePathWithSubAttr{ + ValuePath: ValuePath{ + AttrPath: AttrPath{ + AttrName: "members", + }, + ValFilter: OrLogExp{ + Left: AndLogExp{ + Left: ValueAtom{ + AttrExp: &AttrExp{ + UnaryCondition: &UnaryCondition{ + Left: AttrPath{ + AttrName: "value", + }, + Operator: UnaryConditionOperator{ + Present: true, + }, + }, + }, + }, + }, + }, + }, + SubAttr: gu.Ptr("displayName"), + }, + }, + }, + { + name: "with binary filter", + path: `entitlements[primary eq true]`, + want: &Path{ + ValuePath: &ValuePathWithSubAttr{ + ValuePath: ValuePath{ + AttrPath: AttrPath{ + AttrName: "entitlements", + }, + ValFilter: OrLogExp{ + Left: AndLogExp{ + Left: ValueAtom{ + AttrExp: &AttrExp{ + BinaryCondition: &BinaryCondition{ + Left: AttrPath{ + AttrName: "primary", + }, + Operator: CompareOp{ + Equal: true, + }, + Right: CompValue{ + BooleanTrue: true, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParsePath(tt.path) + if (err != nil) != tt.wantErr { + t.Errorf("ParsePath() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ParsePath() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/api/scim/resources/patch/patch.go b/internal/api/scim/resources/patch/patch.go new file mode 100644 index 000000000..208e23e3a --- /dev/null +++ b/internal/api/scim/resources/patch/patch.go @@ -0,0 +1,273 @@ +package patch + +import ( + "encoding/json" + "reflect" + "slices" + "strings" + + "github.com/zitadel/logging" + + "github.com/zitadel/zitadel/internal/api/scim/resources/filter" + "github.com/zitadel/zitadel/internal/api/scim/schemas" + "github.com/zitadel/zitadel/internal/api/scim/serrors" + "github.com/zitadel/zitadel/internal/zerrors" +) + +type OperationRequest struct { + Schemas []schemas.ScimSchemaType `json:"Schemas"` + Operations []*Operation `json:"Operations"` +} + +type Operation struct { + Operation OperationType `json:"op"` + Path *filter.Path `json:"path"` + Value json.RawMessage `json:"value"` + valueIsArray bool +} + +type OperationCollection []*Operation + +type OperationType string + +const ( + OperationTypeAdd OperationType = "add" + OperationTypeRemove OperationType = "remove" + OperationTypeReplace OperationType = "replace" + + fieldNamePrimary = "Primary" + fieldNameValue = "Value" +) + +type ResourcePatcher interface { + FilterEvaluator() *filter.Evaluator + Added(attributePath []string) error + Replaced(attributePath []string) error + Removed(attributePath []string) error +} + +func (req *OperationRequest) Validate() error { + if !slices.Contains(req.Schemas, schemas.IdPatchOperation) { + return serrors.ThrowInvalidSyntax(zerrors.ThrowInvalidArgumentf(nil, "SCIM-xy1schema", "Expected schema %v is not provided", schemas.IdPatchOperation)) + } + + for _, op := range req.Operations { + if err := op.validate(); err != nil { + return err + } + } + + return nil +} + +func (op *Operation) validate() error { + if !op.Operation.isValid() { + return serrors.ThrowInvalidValue(zerrors.ThrowInvalidArgumentf(nil, "SCIM-opty1", "Patch op %s not supported", op.Operation)) + } + + // json deserialization initializes this field if an empty string is provided + // to not special case this in the further code, + // set it to nil here. + if op.Path.IsZero() { + op.Path = nil + } + + op.valueIsArray = strings.HasPrefix(strings.TrimPrefix(string(op.Value), " "), "[") + return nil +} + +func (ops OperationCollection) Apply(patcher ResourcePatcher, value interface{}) error { + for _, op := range ops { + if err := op.validate(); err != nil { + return err + } + + if err := op.apply(patcher, value); err != nil { + return err + } + } + + return nil +} + +func (op *Operation) apply(patcher ResourcePatcher, value interface{}) error { + switch op.Operation { + case OperationTypeRemove: + return applyRemovePatch(patcher, op, value) + case OperationTypeReplace: + return applyReplacePatch(patcher, op, value) + case OperationTypeAdd: + return applyAddPatch(patcher, op, value) + } + + return zerrors.ThrowInvalidArgumentf(nil, "SCIM-opty3", "SCIM patch: Invalid operation %v", op.Operation) +} + +func (o OperationType) isValid() bool { + switch o { + case OperationTypeAdd, OperationTypeRemove, OperationTypeReplace: + return true + default: + return false + } +} + +func flattenAndApplyPatchOperations(patcher ResourcePatcher, op *Operation, value interface{}) error { + ops, err := flattenPatchOperations(op) + if err != nil { + return err + } + + for _, flattenedOperation := range ops { + if err = flattenedOperation.apply(patcher, value); err != nil { + return err + } + } + + return nil +} + +// flattenPatchOperations flattens patch operations without a path +// it converts an op { "operation": "add", "value": { "path1": "value1", "path2": "value2" } } +// into [ { "operation": "add", "path": "path1", "value": "value1" }, { "operation": "add", "path": "path2", "value": "value2" } ] +func flattenPatchOperations(op *Operation) ([]*Operation, error) { + if op.Path != nil { + panic("Only operations without a path can be flattened") + } + + patches := map[string]json.RawMessage{} + if err := json.Unmarshal(op.Value, &patches); err != nil { + logging.WithError(err).Error("SCIM: Invalid patch value while flattening") + return nil, zerrors.ThrowInvalidArgument(err, "SCIM-ioyl1", "Invalid patch value") + } + + result := make([]*Operation, 0, len(patches)) + for path, value := range patches { + result = append(result, &Operation{ + Operation: op.Operation, + Path: &filter.Path{ + AttrPath: &filter.AttrPath{ + AttrName: path, + }, + }, + Value: value, + valueIsArray: strings.HasPrefix(string(value), "["), + }) + } + + return result, nil +} + +// unmarshalPatchValuesSlice unmarshal the raw json value (a scalar value, object or array) into a new slice +func unmarshalPatchValuesSlice(elementTypePtr reflect.Type, value json.RawMessage, valueIsArray bool) (reflect.Value, error) { + if elementTypePtr.Kind() != reflect.Ptr { + logging.Panicf("elementType must be a pointer to a struct, but is %s", elementTypePtr.Name()) + return reflect.Value{}, nil + } + + if !valueIsArray { + newElement := reflect.New(elementTypePtr.Elem()) + if err := unmarshalPatchValue(value, newElement); err != nil { + return reflect.Value{}, err + } + + newSlice := reflect.MakeSlice(reflect.SliceOf(elementTypePtr), 1, 1) + newSlice.Index(0).Set(newElement) + return newSlice, nil + } + + newSlicePtr := reflect.New(reflect.SliceOf(elementTypePtr)) + newSlice := newSlicePtr.Elem() + if err := json.Unmarshal(value, newSlicePtr.Interface()); err != nil { + logging.WithError(err).Error("SCIM: Invalid patch values") + return reflect.Value{}, zerrors.ThrowInvalidArgument(err, "SCIM-opxx8", "Invalid patch values") + } + return newSlice, nil +} + +func unmarshalPatchValue(newValue json.RawMessage, targetElement reflect.Value) error { + if targetElement.Kind() != reflect.Ptr { + targetElement = targetElement.Addr() + } + + if targetElement.IsNil() { + targetElement.Set(reflect.New(targetElement.Type().Elem())) + } + + if err := json.Unmarshal(newValue, targetElement.Interface()); err != nil { + logging.WithError(err).Error("SCIM: Invalid patch value") + return zerrors.ThrowInvalidArgument(err, "SCIM-opty9", "Invalid patch value") + } + + return nil +} + +// ensureSinglePrimary ensures the modification on a slice results in max one primary element. +// modifiedSlice contains the patched slice. +// modifiedElementsSlice contains only the modified elements. +// if a new element has Primary set to true and an existing is also Primary, the existing Primary flag is set to false. +// returns an error if multiple modifiedElements have a primary value of true. +func ensureSinglePrimary(modifiedSlice reflect.Value, modifiedElementsSlice []reflect.Value, modifiedElementIndexes map[int]bool) error { + if len(modifiedElementsSlice) == 0 { + return nil + } + + hasPrimary, err := isAnyPrimary(modifiedElementsSlice) + if err != nil || !hasPrimary { + return err + } + + for i := 0; i < modifiedSlice.Len(); i++ { + if mod, ok := modifiedElementIndexes[i]; ok && mod { + continue + } + + sliceElement := modifiedSlice.Index(i) + if sliceElement.Kind() == reflect.Ptr { + sliceElement = sliceElement.Elem() + } + + sliceElementPrimaryField := sliceElement.FieldByName(fieldNamePrimary) + if !sliceElementPrimaryField.IsValid() || !sliceElementPrimaryField.Bool() { + continue + } + + sliceElementPrimaryField.SetBool(false) + + // we can stop at the first primary, + // since there can only be one primary in a slice. + return nil + } + + return nil +} + +func isAnyPrimary(elements []reflect.Value) (bool, error) { + foundPrimary := false + for _, element := range elements { + if !isPrimary(element) { + continue + } + + if foundPrimary { + return true, zerrors.ThrowInvalidArgument(nil, "SCIM-1d23", "Cannot add multiple primary values in one patch operation") + } + + foundPrimary = true + } + + return foundPrimary, nil +} + +func isPrimary(element reflect.Value) bool { + if element.Kind() == reflect.Ptr { + element = element.Elem() + } + + if element.Kind() != reflect.Struct { + return false + } + + primaryField := element.FieldByName(fieldNamePrimary) + return primaryField.IsValid() && primaryField.Bool() +} diff --git a/internal/api/scim/resources/patch/patch_add.go b/internal/api/scim/resources/patch/patch_add.go new file mode 100644 index 000000000..09e1154a3 --- /dev/null +++ b/internal/api/scim/resources/patch/patch_add.go @@ -0,0 +1,112 @@ +package patch + +import ( + "reflect" + + "github.com/zitadel/zitadel/internal/api/scim/resources/filter" + "github.com/zitadel/zitadel/internal/api/scim/serrors" + "github.com/zitadel/zitadel/internal/zerrors" +) + +func applyAddPatch(patcher ResourcePatcher, op *Operation, value interface{}) error { + if op.Path == nil { + return flattenAndApplyPatchOperations(patcher, op, value) + } + + result, err := patcher.FilterEvaluator().Evaluate(reflect.ValueOf(value), op.Path) + if err != nil { + return serrors.ThrowInvalidPath(zerrors.ThrowInvalidArgument(err, "SCIM-opzz8", "Failed to evaluate path")) + } + + evaluationResult, ok := result.(*filter.SimpleValueEvaluationResult) + if !ok { + return serrors.ThrowInvalidPath(zerrors.ThrowInvalidArgument(nil, "SCIM-opty8", "Filtered paths are not allowed for add patch operations")) + } + + if evaluationResult.Value.Kind() != reflect.Slice { + return applyReplacePatchSimple(patcher, evaluationResult, op.Value, op.valueIsArray) + } + + elementType := evaluationResult.Value.Type().Elem() + newElementsSlice, err := unmarshalPatchValuesSlice(elementType, op.Value, op.valueIsArray) + if err != nil { + return err + } + + oldLen := evaluationResult.Value.Len() + newSlice := reflect.MakeSlice(reflect.SliceOf(elementType), oldLen, oldLen+newElementsSlice.Len()) + + // copy over existing values + reflect.Copy(newSlice, evaluationResult.Value) + + // according to the RFC only "new" values should be added + // existing values should be replaced + newSlice, modifiedIndexes := addOrReplaceByValue(newSlice, newElementsSlice) + + evaluationResult.Value.Set(newSlice) + if err = ensureSinglePrimaryAdded(evaluationResult.Value, newElementsSlice, modifiedIndexes); err != nil { + return err + } + + return patcher.Added(evaluationResult.PathSegments) +} + +func ensureSinglePrimaryAdded(resultSlice, newSlice reflect.Value, modifiedIndexes map[int]bool) error { + modifiedValues := make([]reflect.Value, newSlice.Len()) + for i := 0; i < newSlice.Len(); i++ { + modifiedValues[i] = newSlice.Index(i) + } + + return ensureSinglePrimary(resultSlice, modifiedValues, modifiedIndexes) +} + +func addOrReplaceByValue(entries, newEntries reflect.Value) (reflect.Value, map[int]bool) { + modifiedIndexes := make(map[int]bool, newEntries.Len()) + if entries.Len() == 0 { + for i := 0; i < newEntries.Len(); i++ { + modifiedIndexes[i] = true + } + + return newEntries, modifiedIndexes + } + + valueField := entries.Index(0).Elem().FieldByName(fieldNameValue) + if !valueField.IsValid() || valueField.Kind() != reflect.String { + entriesLen := entries.Len() + for i := 0; i < newEntries.Len(); i++ { + modifiedIndexes[i+entriesLen] = true + } + + return reflect.AppendSlice(entries, newEntries), modifiedIndexes + } + + existingValueIndexes := make(map[string]int, entries.Len()) + for i := 0; i < entries.Len(); i++ { + value := entries.Index(i).Elem().FieldByName(fieldNameValue).String() + if _, ok := existingValueIndexes[value]; ok { + continue + } + + existingValueIndexes[value] = i + } + + entriesLen := entries.Len() + for i := 0; i < newEntries.Len(); i++ { + newEntry := newEntries.Index(i) + value := newEntry.Elem().FieldByName(fieldNameValue).String() + index, valueExists := existingValueIndexes[value] + + // according to the rfc if the value already exists it should be replaced + if valueExists { + entries.Index(index).Set(newEntry) + modifiedIndexes[index] = true + continue + } + + entries = reflect.Append(entries, newEntry) + modifiedIndexes[entriesLen] = true + entriesLen++ + } + + return entries, modifiedIndexes +} diff --git a/internal/api/scim/resources/patch/patch_remove.go b/internal/api/scim/resources/patch/patch_remove.go new file mode 100644 index 000000000..99be895f2 --- /dev/null +++ b/internal/api/scim/resources/patch/patch_remove.go @@ -0,0 +1,73 @@ +package patch + +import ( + "reflect" + + "github.com/zitadel/logging" + + "github.com/zitadel/zitadel/internal/api/scim/resources/filter" + "github.com/zitadel/zitadel/internal/api/scim/serrors" + "github.com/zitadel/zitadel/internal/zerrors" +) + +func applyRemovePatch(patcher ResourcePatcher, op *Operation, value interface{}) error { + // the root cannot be removed + if op.Path == nil { + logging.Info("SCIM: remove patch without path") + return serrors.ThrowNoTarget(zerrors.ThrowInvalidArgument(nil, "SCIM-ozzy54", "Remove patch without path is not supported")) + } + + result, err := patcher.FilterEvaluator().Evaluate(reflect.ValueOf(value), op.Path) + if err != nil { + return serrors.ThrowInvalidPath(zerrors.ThrowInvalidArgument(err, "SCIM-sd41", "Failed to evaluate path")) + } + + switch filterResult := result.(type) { + case *filter.SimpleValueEvaluationResult: + return applyRemovePatchSimple(patcher, filterResult) + case *filter.FilteredValuesEvaluationResult: + return applyRemovePatchFiltered(patcher, filterResult) + } + + logging.Errorf("SCIM remove patch: unsupported filter type %T", result) + return serrors.ThrowInvalidPath(zerrors.ThrowInvalidArgument(err, "SCIM-12syw", "Invalid patch path")) +} + +func applyRemovePatchSimple(patcher ResourcePatcher, filterResult *filter.SimpleValueEvaluationResult) error { + filterResult.Value.Set(reflect.Zero(filterResult.Value.Type())) + return patcher.Removed(filterResult.PathSegments) +} + +func applyRemovePatchFiltered(patcher ResourcePatcher, filterResult *filter.FilteredValuesEvaluationResult) error { + if len(filterResult.Matches) == 0 { + return nil + } + + // if a subattribute is selected, set that one to nil instead of removing the elements from the slice + if len(filterResult.PathSegments) > 1 { + for _, match := range filterResult.Matches { + match.Value.Set(reflect.Zero(match.Value.Type())) + } + + return patcher.Removed(filterResult.PathSegments) + } + + slice := filterResult.Source + sliceLen := slice.Len() + + // if all elements are matched, set the field to nil + if sliceLen == len(filterResult.Matches) { + filterResult.Source.Set(reflect.Zero(slice.Type())) + return patcher.Removed(filterResult.PathSegments) + } + + // start at the very last matched value to keep correct indexing + for i := len(filterResult.Matches) - 1; i >= 0; i-- { + match := filterResult.Matches[i] + slice = reflect.AppendSlice(slice.Slice(0, match.SourceIndex), slice.Slice(match.SourceIndex+1, sliceLen)) + sliceLen-- + } + + filterResult.Source.Set(slice) + return patcher.Removed(filterResult.PathSegments) +} diff --git a/internal/api/scim/resources/patch/patch_replace.go b/internal/api/scim/resources/patch/patch_replace.go new file mode 100644 index 000000000..f02b56216 --- /dev/null +++ b/internal/api/scim/resources/patch/patch_replace.go @@ -0,0 +1,105 @@ +package patch + +import ( + "encoding/json" + "reflect" + + "github.com/zitadel/logging" + + "github.com/zitadel/zitadel/internal/api/scim/resources/filter" + "github.com/zitadel/zitadel/internal/api/scim/serrors" + "github.com/zitadel/zitadel/internal/zerrors" +) + +func applyReplacePatch(patcher ResourcePatcher, op *Operation, value interface{}) error { + if op.Path == nil { + return flattenAndApplyPatchOperations(patcher, op, value) + } + + result, err := patcher.FilterEvaluator().Evaluate(reflect.ValueOf(value), op.Path) + if err != nil { + return serrors.ThrowInvalidPath(zerrors.ThrowInvalidArgument(err, "SCIM-i2o3", "Failed to evaluate path")) + } + + switch filterResult := result.(type) { + case *filter.SimpleValueEvaluationResult: + return applyReplacePatchSimple(patcher, filterResult, op.Value, op.valueIsArray) + case *filter.FilteredValuesEvaluationResult: + return applyReplacePatchFiltered(patcher, filterResult, op.Value) + } + + logging.Errorf("SCIM replace patch: unsupported filter type %T", result) + return serrors.ThrowInvalidPath(zerrors.ThrowInvalidArgument(err, "SCIM-optu9", "Invalid patch path")) +} + +func applyReplacePatchSimple(patcher ResourcePatcher, evaluationResult *filter.SimpleValueEvaluationResult, newValueRaw json.RawMessage, valueIsArray bool) error { + // patch value is an array + // or it is a scalar or an object but the target is a slice + // unmarshal it as a slice and set it on the target, clearing all existing entries + if valueIsArray || evaluationResult.Value.Kind() == reflect.Slice { + return applyReplacePatchSimpleSlice(patcher, evaluationResult, newValueRaw, valueIsArray) + } + + // patch value and target is a scalar or object + if err := unmarshalPatchValue(newValueRaw, evaluationResult.Value); err != nil { + return err + } + + return patcher.Replaced(evaluationResult.PathSegments) +} + +func applyReplacePatchSimpleSlice(patcher ResourcePatcher, evaluationResult *filter.SimpleValueEvaluationResult, newValueRaw json.RawMessage, valueIsArray bool) error { + if evaluationResult.Value.Kind() != reflect.Slice { + return zerrors.ThrowInvalidArgument(nil, "SCIM-E345X", "Cannot apply array patch value to single value attribute") + } + + values, err := unmarshalPatchValuesSlice(evaluationResult.Value.Type().Elem(), newValueRaw, valueIsArray) + if err != nil { + return err + } + + evaluationResult.Value.Set(values) + modifiedIndexes := make(map[int]bool, values.Len()) + for i := 0; i < values.Len(); i++ { + modifiedIndexes[i] = true + } + + if err = ensureSinglePrimaryAdded(values, values, modifiedIndexes); err != nil { + return err + } + + return patcher.Replaced(evaluationResult.PathSegments) +} + +func applyReplacePatchFiltered(patcher ResourcePatcher, result *filter.FilteredValuesEvaluationResult, newValueRaw json.RawMessage) error { + if len(result.Matches) == 0 { + return serrors.ThrowNoTarget(zerrors.ThrowInvalidArgument(nil, "SCIM-4513", "Path evaluation resulted in no matches")) + } + + for _, match := range result.Matches { + if err := unmarshalPatchValue(newValueRaw, match.Value); err != nil { + return err + } + } + + if err := ensureSinglePrimaryBasedOnMatches(result.Source, result.Matches); err != nil { + return err + } + + return patcher.Replaced(result.PathSegments) +} + +func ensureSinglePrimaryBasedOnMatches(slice reflect.Value, matches []*filter.FilteredValuesEvaluationResultMatch) error { + if len(matches) == 0 { + return nil + } + + modifiedElements := make([]reflect.Value, 0, len(matches)) + modifiedIndexes := make(map[int]bool, len(matches)) + for _, match := range matches { + modifiedElements = append(modifiedElements, match.Element) + modifiedIndexes[match.SourceIndex] = true + } + + return ensureSinglePrimary(slice, modifiedElements, modifiedIndexes) +} diff --git a/internal/api/scim/resources/resource_handler.go b/internal/api/scim/resources/resource_handler.go index c6245c7b7..b7e8bd04e 100644 --- a/internal/api/scim/resources/resource_handler.go +++ b/internal/api/scim/resources/resource_handler.go @@ -8,6 +8,7 @@ import ( "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/api/http" + "github.com/zitadel/zitadel/internal/api/scim/resources/patch" "github.com/zitadel/zitadel/internal/api/scim/schemas" "github.com/zitadel/zitadel/internal/domain" ) @@ -20,6 +21,7 @@ type ResourceHandler[T ResourceHolder] interface { Create(ctx context.Context, resource T) (T, error) Replace(ctx context.Context, id string, resource T) (T, error) + Update(ctx context.Context, id string, operations patch.OperationCollection) error Delete(ctx context.Context, id string) error Get(ctx context.Context, id string) (T, error) List(ctx context.Context, request *ListRequest) (*ListResponse[T], error) diff --git a/internal/api/scim/resources/resource_handler_adapter.go b/internal/api/scim/resources/resource_handler_adapter.go index d0dc21db3..07a32fa1a 100644 --- a/internal/api/scim/resources/resource_handler_adapter.go +++ b/internal/api/scim/resources/resource_handler_adapter.go @@ -7,6 +7,7 @@ import ( "github.com/gorilla/mux" + "github.com/zitadel/zitadel/internal/api/scim/resources/patch" "github.com/zitadel/zitadel/internal/api/scim/serrors" "github.com/zitadel/zitadel/internal/zerrors" ) @@ -40,6 +41,30 @@ func (adapter *ResourceHandlerAdapter[T]) Replace(r *http.Request) (T, error) { return adapter.handler.Replace(r.Context(), id, entity) } +func (adapter *ResourceHandlerAdapter[T]) Update(r *http.Request) error { + request := new(patch.OperationRequest) + err := json.NewDecoder(r.Body).Decode(request) + if err != nil { + if zerrors.IsZitadelError(err) { + return err + } + + return serrors.ThrowInvalidSyntax(zerrors.ThrowInvalidArgumentf(nil, "SCIM-ucrjson2", "Could not deserialize json: %v", err.Error())) + } + + err = request.Validate() + if err != nil { + return err + } + + if len(request.Operations) == 0 { + return nil + } + + id := mux.Vars(r)["id"] + return adapter.handler.Update(r.Context(), id, request.Operations) +} + func (adapter *ResourceHandlerAdapter[T]) Delete(r *http.Request) error { id := mux.Vars(r)["id"] return adapter.handler.Delete(r.Context(), id) diff --git a/internal/api/scim/resources/user.go b/internal/api/scim/resources/user.go index 0e8bfc008..b56e16165 100644 --- a/internal/api/scim/resources/user.go +++ b/internal/api/scim/resources/user.go @@ -7,17 +7,22 @@ import ( "github.com/zitadel/zitadel/internal/api/authz" scim_config "github.com/zitadel/zitadel/internal/api/scim/config" + "github.com/zitadel/zitadel/internal/api/scim/resources/filter" + "github.com/zitadel/zitadel/internal/api/scim/resources/patch" scim_schemas "github.com/zitadel/zitadel/internal/api/scim/schemas" "github.com/zitadel/zitadel/internal/command" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/query" + "github.com/zitadel/zitadel/internal/zerrors" ) type UsersHandler struct { - command *command.Commands - query *query.Queries - userCodeAlg crypto.EncryptionAlgorithm - config *scim_config.Config + command *command.Commands + query *query.Queries + userCodeAlg crypto.EncryptionAlgorithm + config *scim_config.Config + filterEvaluator *filter.Evaluator } type ScimUser struct { @@ -105,7 +110,7 @@ func NewUsersHandler( query *query.Queries, userCodeAlg crypto.EncryptionAlgorithm, config *scim_config.Config) ResourceHandler[*ScimUser] { - return &UsersHandler{command, query, userCodeAlg, config} + return &UsersHandler{command, query, userCodeAlg, config, filter.NewEvaluator(scim_schemas.IdUser)} } func (h *UsersHandler) ResourceNameSingular() scim_schemas.ScimResourceTypeSingular { @@ -160,6 +165,21 @@ func (h *UsersHandler) Replace(ctx context.Context, id string, user *ScimUser) ( return user, nil } +func (h *UsersHandler) Update(ctx context.Context, id string, operations patch.OperationCollection) error { + userWM, err := h.command.UserHumanWriteModel(ctx, id, true, true, true, true, false, false, true) + if err != nil { + return err + } + + user := h.mapWriteModelToScimUser(ctx, userWM) + changeHuman, err := h.applyPatchesToChangeHuman(ctx, user, operations) + if err != nil { + return err + } + + return h.command.ChangeUserHuman(ctx, changeHuman, h.userCodeAlg) +} + func (h *UsersHandler) Delete(ctx context.Context, id string) error { memberships, grants, err := h.queryUserDependencies(ctx, id) if err != nil { @@ -176,6 +196,10 @@ func (h *UsersHandler) Get(ctx context.Context, id string) (*ScimUser, error) { return nil, err } + if user.Type != domain.UserTypeHuman { + return nil, zerrors.ThrowNotFound(nil, "SCIM-USRT1", "Errors.Users.NotFound") + } + metadata, err := h.queryMetadataForUser(ctx, id) if err != nil { return nil, err diff --git a/internal/api/scim/resources/user_mapping.go b/internal/api/scim/resources/user_mapping.go index eeb5962b7..321029244 100644 --- a/internal/api/scim/resources/user_mapping.go +++ b/internal/api/scim/resources/user_mapping.go @@ -14,6 +14,7 @@ import ( "github.com/zitadel/zitadel/internal/command" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/query" + "github.com/zitadel/zitadel/internal/zerrors" ) func (h *UsersHandler) mapToAddHuman(ctx context.Context, scimUser *ScimUser) (*command.AddHuman, error) { @@ -27,8 +28,10 @@ func (h *UsersHandler) mapToAddHuman(ctx context.Context, scimUser *ScimUser) (* human.SetInactive = true } - if email := h.mapPrimaryEmail(scimUser); email != nil { - human.Email = *email + if email, err := h.mapPrimaryEmail(scimUser); err != nil { + return nil, err + } else { + human.Email = email } if phone := h.mapPrimaryPhone(scimUser); phone != nil { @@ -76,10 +79,19 @@ func (h *UsersHandler) mapToChangeHuman(ctx context.Context, scimUser *ScimUser) NickName: &scimUser.NickName, DisplayName: &scimUser.DisplayName, }, - Email: h.mapPrimaryEmail(scimUser), Phone: h.mapPrimaryPhone(scimUser), } + if human.Phone == nil { + human.Phone = &command.Phone{Remove: true} + } + + if email, err := h.mapPrimaryEmail(scimUser); err != nil { + return nil, err + } else { + human.Email = &email + } + if scimUser.Active != nil { if *scimUser.Active { human.State = gu.Ptr(domain.UserStateActive) @@ -114,6 +126,12 @@ func (h *UsersHandler) mapToChangeHuman(ctx context.Context, scimUser *ScimUser) // update user to match the actual stored value scimUser.Name.Formatted = *human.Profile.DisplayName } + + if scimUser.Name.GivenName == "" || scimUser.Name.FamilyName == "" { + return nil, zerrors.ThrowInvalidArgument(nil, "SCIM-USN1", "The name of a user is mandatory") + } + } else { + return nil, zerrors.ThrowInvalidArgument(nil, "SCIM-USN2", "The name of a user is mandatory") } if err := domain.LanguageIsDefined(scimUser.PreferredLanguage); err != nil { @@ -124,19 +142,19 @@ func (h *UsersHandler) mapToChangeHuman(ctx context.Context, scimUser *ScimUser) return human, nil } -func (h *UsersHandler) mapPrimaryEmail(scimUser *ScimUser) *command.Email { +func (h *UsersHandler) mapPrimaryEmail(scimUser *ScimUser) (command.Email, error) { for _, email := range scimUser.Emails { if !email.Primary { continue } - return &command.Email{ + return command.Email{ Address: domain.EmailAddress(email.Value), Verified: h.config.EmailVerified, - } + }, nil } - return nil + return command.Email{}, zerrors.ThrowInvalidArgument(nil, "SCIM-EM19", "Errors.User.Email.Empty") } func (h *UsersHandler) mapPrimaryPhone(scimUser *ScimUser) *command.Phone { @@ -224,95 +242,126 @@ func (h *UsersHandler) mapToScimUsers(ctx context.Context, users []*query.User, func (h *UsersHandler) mapToScimUser(ctx context.Context, user *query.User, md map[metadata.ScopedKey][]byte) *ScimUser { scimUser := &ScimUser{ - Resource: h.buildResourceForQuery(ctx, user), - ID: user.ID, - ExternalID: extractScalarMetadata(ctx, md, metadata.KeyExternalId), - UserName: user.Username, - ProfileUrl: extractHttpURLMetadata(ctx, md, metadata.KeyProfileUrl), - Title: extractScalarMetadata(ctx, md, metadata.KeyTitle), - Locale: extractScalarMetadata(ctx, md, metadata.KeyLocale), - Timezone: extractScalarMetadata(ctx, md, metadata.KeyTimezone), - Active: gu.Ptr(user.State.IsEnabled()), - Ims: make([]*ScimIms, 0), - Addresses: make([]*ScimAddress, 0), - Photos: make([]*ScimPhoto, 0), - Entitlements: make([]*ScimEntitlement, 0), - Roles: make([]*ScimRole, 0), + Resource: h.buildResourceForQuery(ctx, user), + ID: user.ID, + UserName: user.Username, + DisplayName: user.Human.DisplayName, + NickName: user.Human.NickName, + PreferredLanguage: user.Human.PreferredLanguage, + Name: &ScimUserName{ + Formatted: user.Human.DisplayName, + FamilyName: user.Human.LastName, + GivenName: user.Human.FirstName, + }, + Active: gu.Ptr(user.State.IsEnabled()), } - if scimUser.Locale != "" { - _, err := language.Parse(scimUser.Locale) - if err != nil { - logging.OnError(err).Warn("Failed to load locale of scim user") - scimUser.Locale = "" + if string(user.Human.Email) != "" { + scimUser.Emails = []*ScimEmail{ + { + Value: string(user.Human.Email), + Primary: true, + }, } } - if scimUser.Timezone != "" { - _, err := time.LoadLocation(scimUser.Timezone) - if err != nil { - logging.OnError(err).Warn("Failed to load timezone of scim user") - scimUser.Timezone = "" + if string(user.Human.Phone) != "" { + scimUser.PhoneNumbers = []*ScimPhoneNumber{ + { + Value: string(user.Human.Phone), + Primary: true, + }, } } - if err := extractJsonMetadata(ctx, md, metadata.KeyIms, &scimUser.Ims); err != nil { - logging.OnError(err).Warn("Could not deserialize scim ims metadata") - } - - if err := extractJsonMetadata(ctx, md, metadata.KeyAddresses, &scimUser.Addresses); err != nil { - logging.OnError(err).Warn("Could not deserialize scim addresses metadata") - } - - if err := extractJsonMetadata(ctx, md, metadata.KeyPhotos, &scimUser.Photos); err != nil { - logging.OnError(err).Warn("Could not deserialize scim photos metadata") - } - - if err := extractJsonMetadata(ctx, md, metadata.KeyEntitlements, &scimUser.Entitlements); err != nil { - logging.OnError(err).Warn("Could not deserialize scim entitlements metadata") - } - - if err := extractJsonMetadata(ctx, md, metadata.KeyRoles, &scimUser.Roles); err != nil { - logging.OnError(err).Warn("Could not deserialize scim roles metadata") - } - - if user.Human != nil { - mapHumanToScimUser(ctx, user.Human, scimUser, md) - } - + h.mapAndValidateMetadata(ctx, scimUser, md) return scimUser } -func mapHumanToScimUser(ctx context.Context, human *query.Human, user *ScimUser, md map[metadata.ScopedKey][]byte) { - user.DisplayName = human.DisplayName - user.NickName = human.NickName - user.PreferredLanguage = human.PreferredLanguage - user.Name = &ScimUserName{ - Formatted: human.DisplayName, - FamilyName: human.LastName, - GivenName: human.FirstName, - MiddleName: extractScalarMetadata(ctx, md, metadata.KeyMiddleName), - HonorificPrefix: extractScalarMetadata(ctx, md, metadata.KeyHonorificPrefix), - HonorificSuffix: extractScalarMetadata(ctx, md, metadata.KeyHonorificSuffix), +func (h *UsersHandler) mapWriteModelToScimUser(ctx context.Context, user *command.UserV2WriteModel) *ScimUser { + scimUser := &ScimUser{ + Resource: h.buildResourceForWriteModel(ctx, user), + ID: user.AggregateID, + UserName: user.UserName, + DisplayName: user.DisplayName, + NickName: user.NickName, + PreferredLanguage: user.PreferredLanguage, + Name: &ScimUserName{ + Formatted: user.DisplayName, + FamilyName: user.LastName, + GivenName: user.FirstName, + }, + Active: gu.Ptr(user.UserState.IsEnabled()), } - if string(human.Email) != "" { - user.Emails = []*ScimEmail{ + if string(user.Email) != "" { + scimUser.Emails = []*ScimEmail{ { - Value: string(human.Email), + Value: string(user.Email), Primary: true, }, } } - if string(human.Phone) != "" { - user.PhoneNumbers = []*ScimPhoneNumber{ + if string(user.Phone) != "" { + scimUser.PhoneNumbers = []*ScimPhoneNumber{ { - Value: string(human.Phone), + Value: string(user.Phone), Primary: true, }, } } + + md := metadata.MapToScopedKeyMap(user.Metadata) + h.mapAndValidateMetadata(ctx, scimUser, md) + return scimUser +} + +func (h *UsersHandler) mapAndValidateMetadata(ctx context.Context, user *ScimUser, md map[metadata.ScopedKey][]byte) { + user.ExternalID = extractScalarMetadata(ctx, md, metadata.KeyExternalId) + user.ProfileUrl = extractHttpURLMetadata(ctx, md, metadata.KeyProfileUrl) + user.Title = extractScalarMetadata(ctx, md, metadata.KeyTitle) + user.Locale = extractScalarMetadata(ctx, md, metadata.KeyLocale) + user.Timezone = extractScalarMetadata(ctx, md, metadata.KeyTimezone) + user.Name.MiddleName = extractScalarMetadata(ctx, md, metadata.KeyMiddleName) + user.Name.HonorificPrefix = extractScalarMetadata(ctx, md, metadata.KeyHonorificPrefix) + user.Name.HonorificSuffix = extractScalarMetadata(ctx, md, metadata.KeyHonorificSuffix) + + if user.Locale != "" { + _, err := language.Parse(user.Locale) + if err != nil { + logging.OnError(err).Warn("Failed to load locale of scim user") + user.Locale = "" + } + } + + if user.Timezone != "" { + _, err := time.LoadLocation(user.Timezone) + if err != nil { + logging.OnError(err).Warn("Failed to load timezone of scim user") + user.Timezone = "" + } + } + + if err := extractJsonMetadata(ctx, md, metadata.KeyIms, &user.Ims); err != nil { + logging.OnError(err).Warn("Could not deserialize scim ims metadata") + } + + if err := extractJsonMetadata(ctx, md, metadata.KeyAddresses, &user.Addresses); err != nil { + logging.OnError(err).Warn("Could not deserialize scim addresses metadata") + } + + if err := extractJsonMetadata(ctx, md, metadata.KeyPhotos, &user.Photos); err != nil { + logging.OnError(err).Warn("Could not deserialize scim photos metadata") + } + + if err := extractJsonMetadata(ctx, md, metadata.KeyEntitlements, &user.Entitlements); err != nil { + logging.OnError(err).Warn("Could not deserialize scim entitlements metadata") + } + + if err := extractJsonMetadata(ctx, md, metadata.KeyRoles, &user.Roles); err != nil { + logging.OnError(err).Warn("Could not deserialize scim roles metadata") + } } func (h *UsersHandler) buildResourceForQuery(ctx context.Context, user *query.User) *Resource { @@ -328,6 +377,19 @@ func (h *UsersHandler) buildResourceForQuery(ctx context.Context, user *query.Us } } +func (h *UsersHandler) buildResourceForWriteModel(ctx context.Context, user *command.UserV2WriteModel) *Resource { + return &Resource{ + Schemas: []schemas.ScimSchemaType{schemas.IdUser}, + Meta: &ResourceMeta{ + ResourceType: schemas.UserResourceType, + Created: user.CreationDate.UTC(), + LastModified: user.ChangeDate.UTC(), + Version: strconv.FormatUint(user.ProcessedSequence, 10), + Location: buildLocation(ctx, h, user.AggregateID), + }, + } +} + func cascadingMemberships(memberships []*query.Membership) []*command.CascadingMembership { cascades := make([]*command.CascadingMembership, len(memberships)) for i, membership := range memberships { diff --git a/internal/api/scim/resources/user_metadata.go b/internal/api/scim/resources/user_metadata.go index d1faf05fa..69d85e40e 100644 --- a/internal/api/scim/resources/user_metadata.go +++ b/internal/api/scim/resources/user_metadata.go @@ -50,12 +50,7 @@ func (h *UsersHandler) queryMetadataForUser(ctx context.Context, id string) (map return nil, err } - metadataMap := make(map[metadata.ScopedKey][]byte, len(md.Metadata)) - for _, entry := range md.Metadata { - metadataMap[metadata.ScopedKey(entry.Key)] = entry.Value - } - - return metadataMap, nil + return metadata.MapListToScopedKeyMap(md.Metadata), nil } func (h *UsersHandler) buildMetadataQueries(ctx context.Context) *query.UserMetadataSearchQueries { diff --git a/internal/api/scim/resources/user_patch.go b/internal/api/scim/resources/user_patch.go new file mode 100644 index 000000000..5022ed669 --- /dev/null +++ b/internal/api/scim/resources/user_patch.go @@ -0,0 +1,111 @@ +package resources + +import ( + "context" + "strings" + + "github.com/zitadel/zitadel/internal/api/scim/metadata" + "github.com/zitadel/zitadel/internal/api/scim/resources/filter" + "github.com/zitadel/zitadel/internal/api/scim/resources/patch" + "github.com/zitadel/zitadel/internal/command" + "github.com/zitadel/zitadel/internal/domain" +) + +type userPatcher struct { + ctx context.Context + user *ScimUser + metadataChanges map[metadata.Key]*domain.Metadata + metadataKeysToRemove map[metadata.Key]bool + handler *UsersHandler +} + +func (h *UsersHandler) applyPatchesToChangeHuman(ctx context.Context, user *ScimUser, operations patch.OperationCollection) (*command.ChangeHuman, error) { + patcher := &userPatcher{ + ctx: ctx, + user: user, + metadataChanges: make(map[metadata.Key]*domain.Metadata), + metadataKeysToRemove: make(map[metadata.Key]bool), + handler: h, + } + + if err := operations.Apply(patcher, user); err != nil { + return nil, err + } + + // we rely on the change detection of the write model to only execute commands that really change data + changeCommand, err := h.mapToChangeHuman(ctx, user) + if err != nil { + return nil, err + } + + patcher.applyMetadataChangesToCommand(changeCommand) + return changeCommand, nil +} + +func (p *userPatcher) FilterEvaluator() *filter.Evaluator { + return p.handler.filterEvaluator +} + +func (p *userPatcher) Added(attributePath []string) error { + return p.updateMetadata(attributePath) +} + +func (p *userPatcher) Replaced(attributePath []string) error { + return p.updateMetadata(attributePath) +} + +func (p *userPatcher) Removed(attributePath []string) error { + return p.updateMetadata(attributePath) +} + +func (p *userPatcher) applyMetadataChangesToCommand(command *command.ChangeHuman) { + command.MetadataKeysToRemove = make([]string, 0, len(p.metadataKeysToRemove)) + for key := range p.metadataKeysToRemove { + command.MetadataKeysToRemove = append(command.MetadataKeysToRemove, string(key)) + } + + command.Metadata = make([]*domain.Metadata, 0, len(p.metadataChanges)) + for _, update := range p.metadataChanges { + command.Metadata = append(command.Metadata, update) + } +} + +func (p *userPatcher) updateMetadata(attributePath []string) error { + if len(attributePath) == 0 { + return nil + } + + // try full path first (e.g. name.middleName) + // try root only if full path did not match (e.g. for entitlements.value only entitlements is mapped) + var ok bool + var keys []metadata.Key + if len(attributePath) > 1 { + keys, ok = metadata.AttributePathToMetadataKeys[strings.Join(attributePath, ".")] + } + + if !ok { + keys, ok = metadata.AttributePathToMetadataKeys[attributePath[0]] + if !ok { + return nil + } + } + + for _, key := range keys { + value, err := getValueForMetadataKey(p.user, key) + if err != nil { + return err + } + + if len(value) > 0 { + delete(p.metadataKeysToRemove, key) + p.metadataChanges[key] = &domain.Metadata{ + Key: string(metadata.ScopeKey(p.ctx, key)), + Value: value, + } + } else { + p.metadataKeysToRemove[key] = true + delete(p.metadataChanges, key) + } + } + return nil +} diff --git a/internal/api/scim/resources/user_patch_test.go b/internal/api/scim/resources/user_patch_test.go new file mode 100644 index 000000000..5ed83b94a --- /dev/null +++ b/internal/api/scim/resources/user_patch_test.go @@ -0,0 +1,984 @@ +package resources + +import ( + "context" + "encoding/json" + "reflect" + "strings" + "testing" + + "github.com/muhlemmer/gu" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/text/language" + + "github.com/zitadel/zitadel/internal/api/scim/metadata" + "github.com/zitadel/zitadel/internal/api/scim/resources/filter" + "github.com/zitadel/zitadel/internal/api/scim/resources/patch" + "github.com/zitadel/zitadel/internal/api/scim/schemas" + "github.com/zitadel/zitadel/internal/domain" + "github.com/zitadel/zitadel/internal/test" +) + +func TestOperationCollection_Apply(t *testing.T) { + tests := []struct { + name string + op *patch.Operation + prepare func(user *ScimUser) + want *ScimUser + wantFn func(user *ScimUser) + wantModifications []string + wantErr bool + }{ + { + name: "add unknown path", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Path: test.Must(filter.ParsePath("fooBar")), + Value: json.RawMessage(`{ "userName": "hans.muster", "nickname": "hansi" }`), + }, + wantErr: true, + }, + { + name: "add without path", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Value: json.RawMessage(`{ "userName": "hans.muster", "nickname": "hansi" }`), + }, + want: &ScimUser{ + UserName: "hans.muster", + NickName: "hansi", + }, + }, + { + name: "add without path (sample from rfc)", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Value: json.RawMessage(` + { + "emails":[ + { + "value":"babs@jensen.org", + "type":"home", + "primary": true + } + ], + "nickname":"Babs" + }`), + }, + want: &ScimUser{ + NickName: "Babs", + Emails: []*ScimEmail{ + { + Value: "jeanie.pendleton@example.com", + Primary: false, + }, + { + Value: "babs@jensen.org", + Primary: true, + }, + }, + }, + }, + { + name: "add with path value which is nil", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Path: test.Must(filter.ParsePath("externalid")), + Value: json.RawMessage(`"externalid-1"`), + }, + want: &ScimUser{ + ExternalID: "externalid-1", + }, + wantModifications: []string{"rep:externalid"}, + }, + { + name: "add complex attribute", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Path: test.Must(filter.ParsePath("name")), + Value: json.RawMessage(`{ + "Formatted": "added-formatted", + "FamilyName": "added-family-name", + "GivenName": "added-given-name", + "MiddleName": "added-middle-name", + "HonorificPrefix": "added-honorific-prefix", + "HonorificSuffix": "added-honorific-suffix" + }`), + }, + want: &ScimUser{ + Name: &ScimUserName{ + Formatted: "added-formatted", + FamilyName: "added-family-name", + GivenName: "added-given-name", + MiddleName: "added-middle-name", + HonorificPrefix: "added-honorific-prefix", + HonorificSuffix: "added-honorific-suffix", + }, + }, + wantModifications: []string{"rep:name"}, + }, + { + name: "add complex attribute value", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Path: test.Must(filter.ParsePath("name.formatted")), + Value: json.RawMessage(`"added-formatted"`), + }, + want: &ScimUser{ + Name: &ScimUserName{ + Formatted: "added-formatted", + }, + }, + wantModifications: []string{"rep:name.formatted"}, + }, + { + name: "add single to multi valued empty attribute", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Path: test.Must(filter.ParsePath("entitlements")), + Value: json.RawMessage(`{ + "value": "added-entitlement" + }`), + }, + prepare: func(user *ScimUser) { + user.Entitlements = nil + }, + want: &ScimUser{ + Entitlements: []*ScimEntitlement{ + { + Value: "added-entitlement", + Primary: false, + }, + }, + }, + wantModifications: []string{"add:entitlements"}, + }, + { + name: "add single to multi valued attribute", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Path: test.Must(filter.ParsePath("entitlements")), + Value: json.RawMessage(`{ + "value": "added-entitlement" + }`), + }, + want: &ScimUser{ + Entitlements: []*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, + }, + { + Value: "added-entitlement", + Primary: false, + }, + }, + }, + wantModifications: []string{"add:entitlements"}, + }, + { + name: "add single primary to multi valued attribute", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Path: test.Must(filter.ParsePath("entitlements")), + Value: json.RawMessage(`{ + "value": "added-entitlement", + "primary": true + }`), + }, + want: &ScimUser{ + Entitlements: []*ScimEntitlement{ + { + Value: "my-entitlement-1", + Display: "Entitlement 1", + Type: "main-entitlement", + Primary: false, + }, + { + Value: "my-entitlement-2", + Display: "Entitlement 2", + Type: "secondary-entitlement", + Primary: false, + }, + { + Value: "added-entitlement", + Primary: true, + }, + }, + }, + wantModifications: []string{"add:entitlements"}, + }, + { + name: "add unique valued item in multi valued attribute", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Path: test.Must(filter.ParsePath("entitlements")), + Value: json.RawMessage(` + { + "value": "my-entitlement-1", + "display": "entitlement-1-patched", + "primary": true + }`), + }, + want: &ScimUser{ + Entitlements: []*ScimEntitlement{ + { + Value: "my-entitlement-1", + Display: "entitlement-1-patched", + Primary: true, + }, + { + Value: "my-entitlement-2", + Display: "Entitlement 2", + Type: "secondary-entitlement", + Primary: false, + }, + }, + }, + wantModifications: []string{"add:entitlements"}, + }, + { + name: "add unique valued item and additional item in multi valued attribute", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Path: test.Must(filter.ParsePath("entitlements")), + Value: json.RawMessage(`[ + { + "value": "my-entitlement-1", + "display": "entitlement-1-patched", + "primary": true + }, + { + "value": "my-entitlement-3", + "display": "entitlement-3", + "primary": false + } + ]`), + }, + want: &ScimUser{ + Entitlements: []*ScimEntitlement{ + { + Value: "my-entitlement-1", + Display: "entitlement-1-patched", + Primary: true, + }, + { + Value: "my-entitlement-2", + Display: "Entitlement 2", + Type: "secondary-entitlement", + Primary: false, + }, + { + Value: "my-entitlement-3", + Display: "entitlement-3", + Primary: false, + }, + }, + }, + wantModifications: []string{"add:entitlements"}, + }, + { + name: "add unique valued items in multi valued attribute", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Path: test.Must(filter.ParsePath("entitlements")), + Value: json.RawMessage(`[ + { + "value": "my-entitlement-1", + "display": "entitlement-1-patched", + "primary": true + }, + { + "value": "my-entitlement-2", + "display": "entitlement-2-patched", + "primary": false + } + ]`), + }, + want: &ScimUser{ + Entitlements: []*ScimEntitlement{ + { + Value: "my-entitlement-1", + Display: "entitlement-1-patched", + Primary: true, + }, + { + Value: "my-entitlement-2", + Display: "entitlement-2-patched", + Primary: false, + }, + }, + }, + wantModifications: []string{"add:entitlements"}, + }, + { + name: "add multiple to multi valued attribute", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Path: test.Must(filter.ParsePath("entitlements")), + Value: json.RawMessage(` [ + { + "value": "added-entitlement", + "primary": true + }, + { + "value": "added-entitlement-2" + } + ]`), + }, + want: &ScimUser{ + Entitlements: []*ScimEntitlement{ + { + Value: "my-entitlement-1", + Display: "Entitlement 1", + Type: "main-entitlement", + Primary: false, + }, + { + Value: "my-entitlement-2", + Display: "Entitlement 2", + Type: "secondary-entitlement", + Primary: false, + }, + { + Value: "added-entitlement", + Primary: true, + }, + { + Value: "added-entitlement-2", + Primary: false, + }, + }, + }, + wantModifications: []string{"add:entitlements"}, + }, + { + name: "add multiple primaries to multi valued attribute", + op: &patch.Operation{ + Operation: patch.OperationTypeAdd, + Path: test.Must(filter.ParsePath("entitlements")), + Value: json.RawMessage(` [ + { + "value": "added-entitlement", + "primary": true + }, + { + "value": "added-entitlement-2", + "primary": true + } + ]`), + }, + wantErr: true, + }, + { + name: "remove unknown path", + op: &patch.Operation{ + Operation: patch.OperationTypeReplace, + Path: test.Must(filter.ParsePath("fooBar")), + }, + wantErr: true, + }, + { + name: "remove without a path", + op: &patch.Operation{ + Operation: patch.OperationTypeRemove, + }, + wantErr: true, + }, + { + name: "remove single valued attribute", + op: &patch.Operation{ + Operation: patch.OperationTypeRemove, + Path: test.Must(filter.ParsePath("nickname")), + }, + wantFn: func(user *ScimUser) { + assert.Equal(t, user.NickName, "") + }, + wantModifications: []string{"rem:nickname"}, + }, + { + name: "remove multi valued attribute", + op: &patch.Operation{ + Operation: patch.OperationTypeRemove, + Path: test.Must(filter.ParsePath("entitlements")), + }, + wantFn: func(user *ScimUser) { + assert.Len(t, user.Entitlements, 0) + }, + wantModifications: []string{"rem:entitlements"}, + }, + { + name: "remove multi valued attribute with filter", + op: &patch.Operation{ + Operation: patch.OperationTypeRemove, + Path: test.Must(filter.ParsePath(`entitlements[display ew "1"]`)), + }, + want: &ScimUser{ + Entitlements: []*ScimEntitlement{ + { + Value: "my-entitlement-2", + Display: "Entitlement 2", + Type: "secondary-entitlement", + Primary: false, + }, + }, + }, + wantModifications: []string{"rem:entitlements"}, + }, + { + name: "remove multi valued attribute with filter matches all", + op: &patch.Operation{ + Operation: patch.OperationTypeRemove, + Path: test.Must(filter.ParsePath(`entitlements[value pr]`)), + }, + wantFn: func(user *ScimUser) { + assert.Len(t, user.Entitlements, 0) + }, + wantModifications: []string{"rem:entitlements"}, + }, + { + name: "remove attribute of multi valued attribute with filter", + op: &patch.Operation{ + Operation: patch.OperationTypeRemove, + Path: test.Must(filter.ParsePath(`entitlements[display ew "1"].display`)), + }, + want: &ScimUser{ + Entitlements: []*ScimEntitlement{ + { + Value: "my-entitlement-1", + Display: "", + Type: "main-entitlement", + Primary: true, + }, + { + Value: "my-entitlement-2", + Display: "Entitlement 2", + Type: "secondary-entitlement", + Primary: false, + }, + }, + }, + wantModifications: []string{"rem:entitlements.display"}, + }, + { + // this should not fail + // according to the rfc only the replace operation fails without a matching target + name: "remove multi valued attribute with filter and no matches", + op: &patch.Operation{ + Operation: patch.OperationTypeRemove, + Path: test.Must(filter.ParsePath(`entitlements[display eq "FOOBAR"]`)), + }, + want: &ScimUser{ + Entitlements: []*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: "replace unknown path", + op: &patch.Operation{ + Operation: patch.OperationTypeReplace, + Path: test.Must(filter.ParsePath("fooBar")), + Value: json.RawMessage(`"fooBar"`), + }, + wantErr: true, + }, + { + name: "replace without path", + op: &patch.Operation{ + Operation: patch.OperationTypeReplace, + Value: json.RawMessage(`{ "userName": "hans.muster", "nickname": "hansi" }`), + }, + want: &ScimUser{ + UserName: "hans.muster", + NickName: "hansi", + }, + }, + { + name: "replace single valued attribute with path", + op: &patch.Operation{ + Operation: patch.OperationTypeReplace, + Path: test.Must(filter.ParsePath("nickname")), + Value: json.RawMessage(`"fooBar"`), + }, + want: &ScimUser{ + NickName: "fooBar", + }, + }, + { + name: "replace with path value which is nil", + op: &patch.Operation{ + Operation: patch.OperationTypeReplace, + Path: test.Must(filter.ParsePath("externalid")), + Value: json.RawMessage(`"externalid-1"`), + }, + want: &ScimUser{ + ExternalID: "externalid-1", + }, + wantModifications: []string{"rep:externalid"}, + }, + { + name: "replace complex attribute", + op: &patch.Operation{ + Operation: patch.OperationTypeReplace, + Path: test.Must(filter.ParsePath("name")), + Value: json.RawMessage(`{ + "Formatted": "added-formatted", + "FamilyName": "added-family-name", + "GivenName": "added-given-name", + "MiddleName": "added-middle-name", + "HonorificPrefix": "added-honorific-prefix", + "HonorificSuffix": "added-honorific-suffix" + }`), + }, + want: &ScimUser{ + Name: &ScimUserName{ + Formatted: "added-formatted", + FamilyName: "added-family-name", + GivenName: "added-given-name", + MiddleName: "added-middle-name", + HonorificPrefix: "added-honorific-prefix", + HonorificSuffix: "added-honorific-suffix", + }, + }, + wantModifications: []string{"rep:name"}, + }, + { + name: "replace complex multi attribute", + op: &patch.Operation{ + Operation: patch.OperationTypeReplace, + Path: test.Must(filter.ParsePath("entitlements")), + Value: json.RawMessage(`{ + "value": "entitlement patched" + }`), + }, + want: &ScimUser{ + Entitlements: []*ScimEntitlement{ + { + Value: "entitlement patched", + }, + }, + }, + wantModifications: []string{"rep:entitlements"}, + }, + { + name: "replace complex multi attribute with multiple", + op: &patch.Operation{ + Operation: patch.OperationTypeReplace, + Path: test.Must(filter.ParsePath("entitlements")), + Value: json.RawMessage(`[ + { + "value": "entitlement patched" + }, + { + "value": "entitlement patched2" + } + ]`), + }, + want: &ScimUser{ + Entitlements: []*ScimEntitlement{ + { + Value: "entitlement patched", + }, + { + Value: "entitlement patched2", + }, + }, + }, + wantModifications: []string{"rep:entitlements"}, + }, + { + name: "replace complex multi attribute with multiple primary", + op: &patch.Operation{ + Operation: patch.OperationTypeReplace, + Path: test.Must(filter.ParsePath("entitlements")), + Value: json.RawMessage(`[ + { + "value": "entitlement patched", + "primary": true + }, + { + "value": "entitlement patched2", + "primary": true + } + ]`), + }, + wantErr: true, + }, + { + name: "replace filter no match", + op: &patch.Operation{ + Operation: patch.OperationTypeReplace, + Path: test.Must(filter.ParsePath(`entitlements[value eq "foobar"]`)), + }, + wantErr: true, + }, + { + name: "replace filter complex subattribute", + op: &patch.Operation{ + Operation: patch.OperationTypeReplace, + Path: test.Must(filter.ParsePath(`entitlements[value eq "my-entitlement-1"].display`)), + Value: json.RawMessage(`"updated display"`), + }, + want: &ScimUser{ + Entitlements: []*ScimEntitlement{ + { + Value: "my-entitlement-1", + Display: "updated display", + Type: "main-entitlement", + Primary: true, + }, + }, + }, + wantModifications: []string{"rep:entitlements.display"}, + }, + { + name: "replace filter complex subattribute primary", + op: &patch.Operation{ + Operation: patch.OperationTypeReplace, + Path: test.Must(filter.ParsePath(`entitlements[value eq "my-entitlement-2"].primary`)), + Value: json.RawMessage(`true`), + }, + want: &ScimUser{ + Entitlements: []*ScimEntitlement{ + { + Value: "my-entitlement-1", + Display: "Entitlement 1", + Type: "main-entitlement", + Primary: false, + }, + { + Value: "my-entitlement-2", + Display: "Entitlement 2", + Type: "secondary-entitlement", + Primary: true, + }, + }, + }, + wantModifications: []string{"rep:entitlements.primary"}, + }, + { + name: "replace filter complex subattribute multiple primary", + op: &patch.Operation{ + Operation: patch.OperationTypeReplace, + Path: test.Must(filter.ParsePath(`roles[primary ne true].primary`)), + Value: json.RawMessage(`true`), + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + user := &ScimUser{ + ID: "1", + UserName: "username-1", + Name: &ScimUserName{ + Formatted: "Dr. Jeanie R. Pendleton III", + FamilyName: "Pendleton", + GivenName: "Jeanie", + MiddleName: "Rebecca", + HonorificPrefix: "Dr.", + HonorificSuffix: "III", + }, + DisplayName: "Jeanie Pendleton", + NickName: "Jenny", + Title: "Ms.", + ProfileUrl: test.Must(schemas.ParseHTTPURL("https://example.com/profile.gif")), + PreferredLanguage: language.MustParse("en-US"), + Locale: "en-US", + Timezone: "America/New_York", + Active: gu.Ptr(true), + Emails: []*ScimEmail{ + { + Value: "jeanie.pendleton@example.com", + Primary: true, + }, + }, + PhoneNumbers: []*ScimPhoneNumber{ + { + Value: "+1 775-599-5252", + Primary: true, + }, + }, + Ims: []*ScimIms{ + { + Value: "jeeeeeny91", + Type: "icq", + }, + }, + Addresses: []*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, + }, + }, + Photos: []*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: []*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, + }, + { + Value: "my-role-3", + Display: "Rolle 3", + Type: "third-role", + Primary: false, + }, + }, + Entitlements: []*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, + }, + }, + } + + if tt.prepare != nil { + tt.prepare(user) + } + + patcher := new(simplePatcher) + operations := patch.OperationCollection{tt.op} + err := operations.Apply(patcher, user) + if tt.wantErr { + require.Error(t, err) + return + } else { + require.NoError(t, err) + } + + if !test.PartiallyDeepEqual(tt.want, user) { + t.Errorf("apply() got = %#v, want %#v", user, tt.want) + } + + if tt.wantModifications != nil { + assert.EqualValues(t, tt.wantModifications, patcher.modifications) + } + + if tt.wantFn != nil { + tt.wantFn(user) + } + }) + } +} + +func Test_userPatcher_updateMetadata(t *testing.T) { + tests := []struct { + name string + metadataPath []string + metadataChanges map[metadata.Key]*domain.Metadata + metadataKeysToRemove map[metadata.Key]bool + wantErr bool + wantMetadataChanges map[metadata.Key]*domain.Metadata + wantMetadataKeysToRemove map[metadata.Key]bool + }{ + { + name: "empty path", + }, + { + name: "unknown attribute", + metadataPath: []string{"fooBar"}, + }, + { + name: "unknown nested attribute", + metadataPath: []string{"foo", "bar"}, + }, + { + name: "simple attribute", + metadataPath: []string{"title"}, + wantMetadataChanges: map[metadata.Key]*domain.Metadata{ + metadata.KeyTitle: { + Key: string(metadata.KeyTitle), + Value: []byte("Mr."), + }, + }, + }, + { + name: "simple attribute with previous deletion", + metadataPath: []string{"title"}, + metadataKeysToRemove: map[metadata.Key]bool{ + metadata.KeyTitle: true, + }, + wantMetadataChanges: map[metadata.Key]*domain.Metadata{ + metadata.KeyTitle: { + Key: string(metadata.KeyTitle), + Value: []byte("Mr."), + }, + }, + }, + { + name: "nested attribute", + metadataPath: []string{"name", "middlename"}, + wantMetadataChanges: map[metadata.Key]*domain.Metadata{ + metadata.KeyMiddleName: { + Key: string(metadata.KeyMiddleName), + Value: []byte("middle name"), + }, + }, + }, + { + name: "nested attribute with previous deletion", + metadataPath: []string{"name", "middlename"}, + metadataKeysToRemove: map[metadata.Key]bool{ + metadata.KeyMiddleName: true, + }, + wantMetadataChanges: map[metadata.Key]*domain.Metadata{ + metadata.KeyMiddleName: { + Key: string(metadata.KeyMiddleName), + Value: []byte("middle name"), + }, + }, + }, + { + name: "complex object root", + metadataPath: []string{"name"}, + wantMetadataChanges: map[metadata.Key]*domain.Metadata{ + metadata.KeyMiddleName: { + Key: string(metadata.KeyMiddleName), + Value: []byte("middle name"), + }, + metadata.KeyHonorificPrefix: { + Key: string(metadata.KeyHonorificPrefix), + Value: []byte("Adel"), + }, + metadata.KeyHonorificSuffix: { + Key: string(metadata.KeyHonorificSuffix), + Value: []byte("III"), + }, + }, + }, + { + name: "delete previous modified attribute", + metadataPath: []string{"locale"}, + metadataChanges: map[metadata.Key]*domain.Metadata{ + metadata.KeyLocale: { + Key: string(metadata.KeyLocale), + Value: []byte("edited locale"), + }, + }, + wantMetadataKeysToRemove: map[metadata.Key]bool{ + metadata.KeyLocale: true, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := &userPatcher{ + ctx: context.Background(), + user: &ScimUser{ + Title: "Mr.", + Name: &ScimUserName{ + MiddleName: "middle name", + HonorificPrefix: "Adel", + HonorificSuffix: "III", + }, + }, + metadataChanges: tt.metadataChanges, + metadataKeysToRemove: tt.metadataKeysToRemove, + } + + if p.metadataChanges == nil { + p.metadataChanges = make(map[metadata.Key]*domain.Metadata) + } + + if p.metadataKeysToRemove == nil { + p.metadataKeysToRemove = make(map[metadata.Key]bool) + } + + err := p.updateMetadata(tt.metadataPath) + if tt.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + if tt.wantMetadataChanges != nil { + if !reflect.DeepEqual(p.metadataChanges, tt.wantMetadataChanges) { + t.Errorf("updateMetadata() got = %#v, want %#v", p.metadataChanges, tt.wantMetadataChanges) + } + } + + if tt.metadataKeysToRemove != nil { + if !reflect.DeepEqual(p.metadataKeysToRemove, tt.metadataKeysToRemove) { + t.Errorf("updateMetadata() got = %#v, want %#v", p.metadataKeysToRemove, tt.metadataKeysToRemove) + } + } + }) + } +} + +type simplePatcher struct { + modifications []string +} + +func (s *simplePatcher) FilterEvaluator() *filter.Evaluator { + return filter.NewEvaluator(schemas.IdUser) +} + +func (s *simplePatcher) Added(attributePath []string) error { + return s.modified("add", attributePath) +} + +func (s *simplePatcher) Replaced(attributePath []string) error { + return s.modified("rep", attributePath) +} + +func (s *simplePatcher) Removed(attributePath []string) error { + return s.modified("rem", attributePath) +} + +func (s *simplePatcher) modified(op string, attributePath []string) error { + s.modifications = append(s.modifications, op+":"+strings.Join(attributePath, ".")) + return nil +} diff --git a/internal/api/scim/schemas/schemas.go b/internal/api/scim/schemas/schemas.go index b93c664b1..f940657d2 100644 --- a/internal/api/scim/schemas/schemas.go +++ b/internal/api/scim/schemas/schemas.go @@ -11,6 +11,7 @@ const ( IdUser ScimSchemaType = idPrefixCore + "User" IdListResponse ScimSchemaType = idPrefixMessages + "ListResponse" + IdPatchOperation ScimSchemaType = idPrefixMessages + "PatchOp" IdError ScimSchemaType = idPrefixMessages + "Error" IdZitadelErrorDetail ScimSchemaType = idPrefixZitadelMessages + "ErrorDetail" diff --git a/internal/api/scim/serrors/errors.go b/internal/api/scim/serrors/errors.go index e548b4f04..a56d88503 100644 --- a/internal/api/scim/serrors/errors.go +++ b/internal/api/scim/serrors/errors.go @@ -54,6 +54,14 @@ const ( // specified attribute and filter comparison combination is not supported. ScimTypeInvalidFilter scimErrorType = "invalidFilter" + // ScimTypeInvalidPath The "path" attribute was invalid or malformed. + ScimTypeInvalidPath scimErrorType = "invalidPath" + + // ScimTypeNoTarget The specified "path" did not + // yield an attribute or attribute value that could be operated on. + // This occurs when the specified "path" value contains a filter that yields no match. + ScimTypeNoTarget scimErrorType = "noTarget" + // ScimTypeUniqueness One or more of the attribute values are already in use or are reserved. ScimTypeUniqueness scimErrorType = "uniqueness" ) @@ -99,6 +107,20 @@ func ThrowInvalidFilter(parent error) error { } } +func ThrowInvalidPath(parent error) error { + return &wrappedScimError{ + Parent: parent, + ScimType: ScimTypeInvalidPath, + } +} + +func ThrowNoTarget(parent error) error { + return &wrappedScimError{ + Parent: parent, + ScimType: ScimTypeNoTarget, + } +} + func IsScimOrZitadelError(err error) bool { return IsScimError(err) || zerrors.IsZitadelError(err) } diff --git a/internal/api/scim/server.go b/internal/api/scim/server.go index 7e56e8241..d589b480c 100644 --- a/internal/api/scim/server.go +++ b/internal/api/scim/server.go @@ -58,6 +58,7 @@ func mapResource[T sresources.ResourceHolder](router *mux.Router, mw zhttp_middl resourceRouter.Handle("/.search", mw(handleJsonResponse(adapter.List))).Methods(http.MethodPost) resourceRouter.Handle("/{id}", mw(handleResourceResponse(adapter.Get))).Methods(http.MethodGet) resourceRouter.Handle("/{id}", mw(handleResourceResponse(adapter.Replace))).Methods(http.MethodPut) + resourceRouter.Handle("/{id}", mw(handleEmptyResponse(adapter.Update))).Methods(http.MethodPatch) resourceRouter.Handle("/{id}", mw(handleEmptyResponse(adapter.Delete))).Methods(http.MethodDelete) } diff --git a/internal/command/phone.go b/internal/command/phone.go index 8d971e2f9..5afed5ee7 100644 --- a/internal/command/phone.go +++ b/internal/command/phone.go @@ -6,16 +6,32 @@ import ( "github.com/zitadel/zitadel/internal/command/preparation" "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/domain" + "github.com/zitadel/zitadel/internal/zerrors" ) type Phone struct { Number domain.PhoneNumber + Remove bool Verified bool // ReturnCode is used if the Verified field is false ReturnCode bool } +func (p *Phone) Validate() (err error) { + if p.Remove && p.Number != "" { + return zerrors.ThrowInvalidArgumentf(nil, "USRP2-12881", "Cannot update and remove the phone number at the same time") + } + + if p.Number != "" { + if p.Number, err = p.Number.Normalize(); err != nil { + return err + } + } + + return nil +} + // newPhoneCode generates a new code to be sent out to via SMS or // returns the ID of the external code provider (e.g. when using Twilio verification API) func (c *Commands) newPhoneCode(ctx context.Context, filter preparation.FilterToQueryReducer, secretGeneratorType domain.SecretGeneratorType, alg crypto.EncryptionAlgorithm, defaultConfig *crypto.GeneratorConfig) (*EncryptedCode, string, error) { diff --git a/internal/command/user_v2_human.go b/internal/command/user_v2_human.go index fa627ec66..6ef61f575 100644 --- a/internal/command/user_v2_human.go +++ b/internal/command/user_v2_human.go @@ -14,12 +14,13 @@ import ( ) type ChangeHuman struct { - ID string - State *domain.UserState - Username *string - Profile *Profile - Email *Email - Phone *Phone + ID string + State *domain.UserState + Username *string + Profile *Profile + Email *Email + Phone *Phone + Metadata []*domain.Metadata MetadataKeysToRemove []string @@ -61,8 +62,8 @@ func (h *ChangeHuman) Validate(hasher *crypto.Hasher) (err error) { } } - if h.Phone != nil && h.Phone.Number != "" { - if h.Phone.Number, err = h.Phone.Number.Normalize(); err != nil { + if h.Phone != nil { + if err := h.Phone.Validate(); err != nil { return err } } @@ -263,7 +264,7 @@ func (c *Commands) ChangeUserHuman(ctx context.Context, human *ChangeHuman, alg return err } - existingHuman, err := c.userHumanWriteModel( + existingHuman, err := c.UserHumanWriteModel( ctx, human.ID, human.Profile != nil, @@ -272,13 +273,11 @@ func (c *Commands) ChangeUserHuman(ctx context.Context, human *ChangeHuman, alg human.Password != nil, false, // avatar not updateable false, // IDPLinks not updateable + len(human.Metadata) > 0 || len(human.MetadataKeysToRemove) > 0, ) if err != nil { return err } - if !isUserStateExists(existingHuman.UserState) { - return zerrors.ThrowNotFound(nil, "COMMAND-ugjs0upun6", "Errors.User.NotFound") - } if human.Changed() { if err := c.checkPermissionUpdateUser(ctx, existingHuman.ResourceOwner, existingHuman.AggregateID); err != nil { @@ -415,6 +414,10 @@ func (c *Commands) changeUserPhone(ctx context.Context, cmds []eventstore.Comman ctx, span := tracing.NewSpan(ctx) defer func() { span.End() }() + if phone.Remove { + return append(cmds, user.NewHumanPhoneRemovedEvent(ctx, &wm.Aggregate().Aggregate)), nil, nil + } + if phone.Number != "" && phone.Number != wm.Phone { cmds = append(cmds, user.NewHumanPhoneChangedEvent(ctx, &wm.Aggregate().Aggregate, phone.Number)) @@ -432,6 +435,7 @@ func (c *Commands) changeUserPhone(ctx context.Context, cmds []eventstore.Comman return cmds, code, nil } } + // only create separate event of verified if email was not changed if phone.Verified && wm.IsPhoneVerified != phone.Verified { return append(cmds, user.NewHumanPhoneVerifiedEvent(ctx, &wm.Aggregate().Aggregate)), code, nil @@ -499,14 +503,19 @@ func (c *Commands) userExistsWriteModel(ctx context.Context, userID string) (wri return writeModel, nil } -func (c *Commands) userHumanWriteModel(ctx context.Context, userID string, profileWM, emailWM, phoneWM, passwordWM, avatarWM, idpLinksWM bool) (writeModel *UserV2WriteModel, err error) { +func (c *Commands) UserHumanWriteModel(ctx context.Context, userID string, profileWM, emailWM, phoneWM, passwordWM, avatarWM, idpLinksWM, metadataWM bool) (writeModel *UserV2WriteModel, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() - writeModel = NewUserHumanWriteModel(userID, "", profileWM, emailWM, phoneWM, passwordWM, avatarWM, idpLinksWM) + writeModel = NewUserHumanWriteModel(userID, "", profileWM, emailWM, phoneWM, passwordWM, avatarWM, idpLinksWM, metadataWM) err = c.eventstore.FilterToQueryReducer(ctx, writeModel) if err != nil { return nil, err } + + if !isUserStateExists(writeModel.UserState) { + return nil, zerrors.ThrowNotFound(nil, "COMMAND-ugjs0upun6", "Errors.User.NotFound") + } + return writeModel, nil } diff --git a/internal/command/user_v2_model.go b/internal/command/user_v2_model.go index 25150ab65..214a2a5f9 100644 --- a/internal/command/user_v2_model.go +++ b/internal/command/user_v2_model.go @@ -15,6 +15,8 @@ import ( type UserV2WriteModel struct { eventstore.WriteModel + CreationDate time.Time + UserName string MachineWriteModel bool @@ -73,6 +75,9 @@ type UserV2WriteModel struct { IDPLinkWriteModel bool IDPLinks []*domain.UserIDPLink + + MetadataWriteModel bool + Metadata map[string][]byte } func NewUserExistsWriteModel(userID, resourceOwner string) *UserV2WriteModel { @@ -87,7 +92,7 @@ func NewUserRemoveWriteModel(userID, resourceOwner string) *UserV2WriteModel { return newUserV2WriteModel(userID, resourceOwner, WithHuman(), WithMachine(), WithState(), WithIDPLinks()) } -func NewUserHumanWriteModel(userID, resourceOwner string, profileWM, emailWM, phoneWM, passwordWM, avatarWM, idpLinks bool) *UserV2WriteModel { +func NewUserHumanWriteModel(userID, resourceOwner string, profileWM, emailWM, phoneWM, passwordWM, avatarWM, idpLinks, metadataListWM bool) *UserV2WriteModel { opts := []UserV2WMOption{WithHuman(), WithState()} if profileWM { opts = append(opts, WithProfile()) @@ -107,6 +112,9 @@ func NewUserHumanWriteModel(userID, resourceOwner string, profileWM, emailWM, ph if idpLinks { opts = append(opts, WithIDPLinks()) } + if metadataListWM { + opts = append(opts, WithMetadata()) + } return newUserV2WriteModel(userID, resourceOwner, opts...) } @@ -172,6 +180,12 @@ func WithIDPLinks() UserV2WMOption { } } +func WithMetadata() UserV2WMOption { + return func(o *UserV2WriteModel) { + o.MetadataWriteModel = true + } +} + func (wm *UserV2WriteModel) Reduce() error { for _, event := range wm.Events { switch e := event.(type) { @@ -281,6 +295,17 @@ func (wm *UserV2WriteModel) Reduce() error { wm.RemoveIDPLink(e.IDPConfigID, e.ExternalUserID) case *user.UserIDPLinkCascadeRemovedEvent: wm.RemoveIDPLink(e.IDPConfigID, e.ExternalUserID) + case *user.MetadataSetEvent: + if wm.Metadata == nil { + wm.Metadata = make(map[string][]byte) + } + + wm.Metadata[e.Key] = e.Value + case *user.MetadataRemovedEvent: + wm.Metadata[e.Key] = nil + delete(wm.Metadata, e.Key) + case *user.MetadataRemovedAllEvent: + wm.Metadata = nil } } return wm.WriteModel.Reduce() @@ -457,6 +482,13 @@ func (wm *UserV2WriteModel) Query() *eventstore.SearchQueryBuilder { ) } + if wm.MetadataWriteModel { + eventTypes = append(eventTypes, + user.MetadataSetType, + user.MetadataRemovedType, + user.MetadataRemovedAllType) + } + query := eventstore.NewSearchQueryBuilder(eventstore.ColumnsEvent). AddQuery(). AggregateTypes(user.AggregateType). @@ -482,6 +514,7 @@ func (wm *UserV2WriteModel) reduceHumanAddedEvent(e *user.HumanAddedEvent) { wm.UserState = domain.UserStateActive wm.PasswordEncodedHash = crypto.SecretOrEncodedHash(e.Secret, e.EncodedHash) wm.PasswordChangeRequired = e.ChangeRequired + wm.CreationDate = e.Creation } func (wm *UserV2WriteModel) reduceHumanRegisteredEvent(e *user.HumanRegisteredEvent) { diff --git a/internal/command/user_v2_model_test.go b/internal/command/user_v2_model_test.go index 2476d27d5..5bbc17a54 100644 --- a/internal/command/user_v2_model_test.go +++ b/internal/command/user_v2_model_test.go @@ -567,7 +567,7 @@ func TestCommandSide_userHumanWriteModel_profile(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore(t), } - wm, err := r.userHumanWriteModel(tt.args.ctx, tt.args.userID, true, false, false, false, false, false) + wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, true, false, false, false, false, false, false) if tt.res.err == nil { if !assert.NoError(t, err) { t.FailNow() @@ -912,7 +912,7 @@ func TestCommandSide_userHumanWriteModel_email(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore(t), } - wm, err := r.userHumanWriteModel(tt.args.ctx, tt.args.userID, false, true, false, false, false, false) + wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, false, true, false, false, false, false, false) if tt.res.err == nil { if !assert.NoError(t, err) { t.FailNow() @@ -1344,7 +1344,7 @@ func TestCommandSide_userHumanWriteModel_phone(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore(t), } - wm, err := r.userHumanWriteModel(tt.args.ctx, tt.args.userID, false, false, true, false, false, false) + wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, false, false, true, false, false, false, false) if tt.res.err == nil { if !assert.NoError(t, err) { t.FailNow() @@ -1605,7 +1605,7 @@ func TestCommandSide_userHumanWriteModel_password(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore(t), } - wm, err := r.userHumanWriteModel(tt.args.ctx, tt.args.userID, false, false, false, true, false, false) + wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, false, false, false, true, false, false, false) if tt.res.err == nil { if !assert.NoError(t, err) { t.FailNow() @@ -2132,7 +2132,7 @@ func TestCommandSide_userHumanWriteModel_avatar(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore(t), } - wm, err := r.userHumanWriteModel(tt.args.ctx, tt.args.userID, false, false, false, false, true, false) + wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, false, false, false, false, true, false, false) if tt.res.err == nil { if !assert.NoError(t, err) { t.FailNow() @@ -2456,3 +2456,306 @@ func TestCommandSide_userHumanWriteModel_idpLinks(t *testing.T) { }) } } + +func TestCommandSide_userHumanWriteModel_metadata(t *testing.T) { + type fields struct { + eventstore func(t *testing.T) *eventstore.Eventstore + } + type args struct { + ctx context.Context + userID string + } + type res struct { + want *UserV2WriteModel + err func(error) bool + } + + userAgg := user.NewAggregate("user1", "org1") + + tests := []struct { + name string + fields fields + args args + res res + }{ + { + name: "user added with metadata", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + newAddHumanEvent("$plain$x$password", true, true, "", language.English), + ), + eventFromEventPusher( + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key", + []byte("value"), + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + }, + res: res{ + want: &UserV2WriteModel{ + HumanWriteModel: true, + StateWriteModel: true, + MetadataWriteModel: true, + WriteModel: eventstore.WriteModel{ + AggregateID: "user1", + Events: []eventstore.Event{}, + ProcessedSequence: 0, + ResourceOwner: "org1", + }, + UserName: "username", + FirstName: "firstname", + LastName: "lastname", + DisplayName: "firstname lastname", + PreferredLanguage: language.English, + PasswordEncodedHash: "$plain$x$password", + PasswordChangeRequired: true, + Email: "email@test.ch", + IsEmailVerified: false, + UserState: domain.UserStateActive, + Metadata: map[string][]byte{ + "key": []byte("value"), + }, + }, + }, + }, + { + name: "user added with multiple metadata", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + newAddHumanEvent("$plain$x$password", true, true, "", language.English), + ), + eventFromEventPusher( + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key1", + []byte("value1"), + ), + ), + eventFromEventPusher( + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key2", + []byte("value2"), + ), + ), + eventFromEventPusher( + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key3", + []byte("value3"), + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + }, + res: res{ + want: &UserV2WriteModel{ + HumanWriteModel: true, + StateWriteModel: true, + MetadataWriteModel: true, + WriteModel: eventstore.WriteModel{ + AggregateID: "user1", + Events: []eventstore.Event{}, + ProcessedSequence: 0, + ResourceOwner: "org1", + }, + UserName: "username", + FirstName: "firstname", + LastName: "lastname", + DisplayName: "firstname lastname", + PreferredLanguage: language.English, + PasswordEncodedHash: "$plain$x$password", + PasswordChangeRequired: true, + Email: "email@test.ch", + IsEmailVerified: false, + UserState: domain.UserStateActive, + Metadata: map[string][]byte{ + "key1": []byte("value1"), + "key2": []byte("value2"), + "key3": []byte("value3"), + }, + }, + }, + }, + { + name: "user added with metadata add and remove", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + newAddHumanEvent("$plain$x$password", true, true, "", language.English), + ), + eventFromEventPusher( + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key1", + []byte("value1"), + ), + ), + eventFromEventPusher( + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key2", + []byte("name2"), + ), + ), + eventFromEventPusher( + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key3", + []byte("value3"), + ), + ), + eventFromEventPusher( + user.NewMetadataRemovedEvent( + context.Background(), + &userAgg.Aggregate, + "key2", + ), + ), + eventFromEventPusher( + user.NewMetadataRemovedEvent( + context.Background(), + &userAgg.Aggregate, + "key3", + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + }, + res: res{ + want: &UserV2WriteModel{ + HumanWriteModel: true, + StateWriteModel: true, + MetadataWriteModel: true, + WriteModel: eventstore.WriteModel{ + AggregateID: "user1", + Events: []eventstore.Event{}, + ProcessedSequence: 0, + ResourceOwner: "org1", + }, + UserName: "username", + FirstName: "firstname", + LastName: "lastname", + DisplayName: "firstname lastname", + PreferredLanguage: language.English, + PasswordEncodedHash: "$plain$x$password", + PasswordChangeRequired: true, + Email: "email@test.ch", + IsEmailVerified: false, + UserState: domain.UserStateActive, + Metadata: map[string][]byte{ + "key1": []byte("value1"), + }, + }, + }, + }, + { + name: "user added with added metadata and removed all", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + newAddHumanEvent("$plain$x$password", true, true, "", language.English), + ), + eventFromEventPusher( + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key1", + []byte("value1"), + ), + ), + eventFromEventPusher( + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key2", + []byte("value2"), + ), + ), + eventFromEventPusher( + user.NewMetadataRemovedAllEvent( + context.Background(), + &userAgg.Aggregate, + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + }, + res: res{ + want: &UserV2WriteModel{ + HumanWriteModel: true, + StateWriteModel: true, + MetadataWriteModel: true, + WriteModel: eventstore.WriteModel{ + AggregateID: "user1", + Events: []eventstore.Event{}, + ProcessedSequence: 0, + ResourceOwner: "org1", + }, + UserName: "username", + FirstName: "firstname", + LastName: "lastname", + DisplayName: "firstname lastname", + PreferredLanguage: language.English, + PasswordEncodedHash: "$plain$x$password", + PasswordChangeRequired: true, + Email: "email@test.ch", + IsEmailVerified: false, + UserState: domain.UserStateActive, + Metadata: nil, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := &Commands{ + eventstore: tt.fields.eventstore(t), + } + wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, false, false, false, false, false, false, true) + if tt.res.err == nil { + if !assert.NoError(t, err) { + t.FailNow() + } + } else if !tt.res.err(err) { + t.Errorf("got wrong err: %v ", err) + return + } + if tt.res.err == nil { + assert.Equal(t, tt.res.want, wm) + } + }) + } +} diff --git a/internal/integration/scim/assertions.go b/internal/integration/scim/assertions.go index a91c33da8..f725a0bdd 100644 --- a/internal/integration/scim/assertions.go +++ b/internal/integration/scim/assertions.go @@ -4,7 +4,6 @@ import ( "errors" "strconv" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -16,7 +15,7 @@ func RequireScimError(t require.TestingT, httpStatus int, err error) AssertedSci require.Error(t, err) var scimErr *ScimError - assert.True(t, errors.As(err, &scimErr)) - assert.Equal(t, strconv.Itoa(httpStatus), scimErr.Status) + require.True(t, errors.As(err, &scimErr)) + require.Equal(t, strconv.Itoa(httpStatus), scimErr.Status) return AssertedScimError{scimErr} // wrap it, otherwise error handling is enforced } diff --git a/internal/integration/scim/client.go b/internal/integration/scim/client.go index 3c1bdb4b2..b51ce33d6 100644 --- a/internal/integration/scim/client.go +++ b/internal/integration/scim/client.go @@ -101,6 +101,15 @@ func (c *ResourceClient[T]) Replace(ctx context.Context, orgID, id string, body return c.doWithBody(ctx, http.MethodPut, orgID, id, bytes.NewReader(body)) } +func (c *ResourceClient[T]) Update(ctx context.Context, orgID, id string, body []byte) error { + req, err := http.NewRequestWithContext(ctx, http.MethodPatch, c.buildURL(orgID, id), bytes.NewReader(body)) + if err != nil { + return err + } + + return c.doReq(req, nil) +} + func (c *ResourceClient[T]) List(ctx context.Context, orgID string, req *ListRequest) (*ListResponse[*T], error) { if req.SendAsPost { listReq, err := json.Marshal(req) @@ -181,15 +190,16 @@ func (c *ResourceClient[T]) doReq(req *http.Request, responseEntity interface{}) addTokenAsHeader(req) resp, err := c.client.Do(req) - defer func() { - err := resp.Body.Close() - logging.OnError(err).Error("Failed to close response body") - }() if err != nil { return err } + defer func() { + err := resp.Body.Close() + logging.OnError(err).Error("Failed to close response body") + }() + if (resp.StatusCode / 100) != 2 { return readScimError(resp) }