TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2025-08-11 21:17:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(scim): add type attribute to ScimEmail (#9690)

Browse Source 
# Which Problems Are Solved

- SCIM PATCH operations for users from Entra ID for the `emails`
attribute fails due to missing `type` subattribute

# How the Problems Are Solved

- Adds the `type` attribute to the `ScimUser` struct and sets the
default value to `"work"` in the `mapWriteModelToScimUser()` method.

# Additional Changes

# Additional Context

The SCIM handlers for POST and PUT ignore multiple emails and only uses
the primary email for a given user, or falls back to the first email if
none are marked as primary. PATCH operations however, will attempt to
resolve the provided filter in `operations[].path`.

Some services, such as Entra ID, only support patching emails by
filtering for `emails[type eq "(work|home|other)"].value`, which fails
with Zitadel as the ScimUser struct (and thus the generated schema)
doesn't include the `type` field.

This commit adds the `type` field to work around this issue, while still
preserving compatibility with filters such as `emails[primary eq
true].value`.

-
https://discord.com/channels/927474939156643850/927866013545025566/1356556668527448191

---------

Co-authored-by: Christer Edvartsen <christer.edvartsen@nav.no>
Co-authored-by: Thomas Siegfried Krampl <thomas.siegfried.krampl@nav.no>
This commit is contained in:
Trong Huu Nguyen
2025-06-19 11:42:44 +02:00
committed by GitHub
parent 28f7218ea1
commit 3a4298c179
12 changed files with 135 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
internal/api/scim/integration_test/testdata/service_provider_config_expected_schemas.json vendored
View File

@@ -233,6 +233,17 @@
"mutability": "readWrite",
"returned": "always",
"uniqueness": "none"
},
{
"name": "type",
"description": "For details see RFC7643",
"type": "string",
"multiValued": false,
"required": false,
"caseExact": true,
"mutability": "readWrite",
"returned": "always",
"uniqueness": "none"
}
],
"multiValued": true,

11
internal/api/scim/integration_test/testdata/service_provider_config_expected_user_schema.json vendored
View File

@@ -225,6 +225,17 @@
"mutability": "readWrite",
"returned": "always",
"uniqueness": "none"
},
{
"name": "type",
"description": "For details see RFC7643",
"type": "string",
"multiValued": false,
"required": false,
"caseExact": true,
"mutability": "readWrite",
"returned": "always",
"uniqueness": "none"
}
],
"multiValued": true,

17
internal/api/scim/integration_test/testdata/users_replace_test_minimal_with_email_type.json vendored Normal file
View File

@@ -0,0 +1,17 @@
{
"schemas": [
"urn:ietf:params:scim:schemas:core:2.0:User"
],
"userName": "acmeUser1-minimal-replaced",
"name": {
"familyName": "Ross-replaced",
"givenName": "Bethany-replaced"
},
"emails": [
{
"value": "user1-minimal-replaced@example.com",
"primary": true,
"type": "work"
}
]
}

1
internal/api/scim/integration_test/users_create_test.go
View File

@@ -391,6 +391,7 @@ func TestCreateUser_metadata(t *testing.T) {
test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:locale", "en-US")
test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:ims", `[{"value":"someaimhandle","type":"aim"},{"value":"twitterhandle","type":"X"}]`)
test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:roles", `[{"value":"my-role-1","display":"Rolle 1","type":"main-role","primary":true},{"value":"my-role-2","display":"Rolle 2","type":"secondary-role"}]`)
test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:emails", `[{"value":"bjensen@example.com","primary":true,"type":"work"},{"value":"babs@jensen.org","primary":false,"type":"home"}]`)
}, retryDuration, tick)
}

1
internal/api/scim/integration_test/users_get_test.go
View File

@@ -115,6 +115,7 @@ func TestGetUser(t *testing.T) {
{
Value: "bjensen@example.com",
Primary: true,
Type: "work",
},
},
PhoneNumbers: []*resources.ScimPhoneNumber{

40
internal/api/scim/integration_test/users_replace_test.go
View File

@@ -27,6 +27,9 @@ var (
//go:embed testdata/users_replace_test_minimal_with_external_id.json
minimalUserWithExternalIDJson []byte
//go:embed testdata/users_replace_test_minimal_with_email_type.json
minimalUserWithEmailTypeReplaceJson []byte
//go:embed testdata/users_replace_test_minimal.json
minimalUserReplaceJson []byte
@@ -303,7 +306,42 @@ func TestReplaceUser_removeOldMetadata(t *testing.T) {
Id: createdUser.ID,
})
require.NoError(tt, err)
require.Equal(tt, 0, len(md.Result))
require.Equal(tt, 1, len(md.Result))
mdMap := make(map[string]string)
for i := range md.Result {
mdMap[md.Result[i].Key] = string(md.Result[i].Value)
}
test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:emails", "[{\"value\":\"user1@example.com\",\"primary\":true}]")
}, retryDuration, tick)
_, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: createdUser.ID})
require.NoError(t, err)
}
func TestReplaceUser_emailType(t *testing.T) {
// ensure old metadata is removed correctly
createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, fullUserJson)
require.NoError(t, err)
_, err = Instance.Client.SCIM.Users.Replace(CTX, Instance.DefaultOrg.Id, createdUser.ID, minimalUserWithEmailTypeReplaceJson)
require.NoError(t, err)
retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute)
require.EventuallyWithT(t, func(tt *assert.CollectT) {
md, err := Instance.Client.Mgmt.ListUserMetadata(CTX, &management.ListUserMetadataRequest{
Id: createdUser.ID,
})
require.NoError(tt, err)
require.Equal(tt, 1, len(md.Result))
mdMap := make(map[string]string)
for i := range md.Result {
mdMap[md.Result[i].Key] = string(md.Result[i].Value)
}
test.AssertMapContains(tt, mdMap, "urn:zitadel:scim:emails", "[{\"value\":\"user1-minimal-replaced@example.com\",\"primary\":true,\"type\":\"work\"}]")
}, retryDuration, tick)
_, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: createdUser.ID})

9
internal/api/scim/integration_test/users_update_test.go
View File

@@ -137,9 +137,18 @@ func TestUpdateUser(t *testing.T) {
NickName: "",
ProfileUrl: test.Must(schemas.ParseHTTPURL("http://login.example.com/bjensen")),
Emails: []*resources.ScimEmail{
{
Value: "bjensen@example.com",
Type: "work",
},
{
Value: "babs@jensen.org",
Type: "home",
},
{
Value: "babs@example.com",
Primary: true,
Type: "home",
},
},
Addresses: []*resources.ScimAddress{

3
internal/api/scim/metadata/metadata.go
View File

@@ -30,6 +30,7 @@ const (
KeyAddresses Key = KeyPrefix + "addresses"
KeyEntitlements Key = KeyPrefix + "entitlements"
KeyRoles Key = KeyPrefix + "roles"
KeyEmails Key = KeyPrefix + "emails"
)
var (
@@ -47,6 +48,7 @@ var (
KeyAddresses,
KeyEntitlements,
KeyRoles,
KeyEmails,
}
AttributePathToMetadataKeys = map[string][]Key{
@@ -64,6 +66,7 @@ var (
"addresses": {KeyAddresses},
"entitlements": {KeyEntitlements},
"roles": {KeyRoles},
"emails": {KeyEmails},
}
)

1
internal/api/scim/resources/user.go
View File

@@ -90,6 +90,7 @@ type ScimIms struct {
type ScimEmail struct {
Value string `json:"value" scim:"required"`
Primary bool `json:"primary"`
Type string `json:"type,omitempty"`
}
type ScimPhoneNumber struct {

4
internal/api/scim/resources/user_mapping.go
View File

@@ -382,6 +382,10 @@ func (h *UsersHandler) mapAndValidateMetadata(ctx context.Context, user *ScimUse
if err := extractJsonMetadata(ctx, md, metadata.KeyRoles, &user.Roles); err != nil {
logging.OnError(err).Warn("Could not deserialize scim roles metadata")
}
if err := extractJsonMetadata(ctx, md, metadata.KeyEmails, &user.Emails); err != nil {
logging.OnError(err).Warn("Could not deserialize scim emails metadata")
}
}
func (h *UsersHandler) buildResourceForQuery(ctx context.Context, user *query.User) *schemas.Resource {

5
internal/api/scim/resources/user_metadata.go
View File

@@ -129,7 +129,8 @@ func getValueForMetadataKey(user *ScimUser, key metadata.Key) ([]byte, error) {
metadata.KeyAddresses,
metadata.KeyEntitlements,
metadata.KeyIms,
metadata.KeyPhotos:
metadata.KeyPhotos,
metadata.KeyEmails:
val, err := json.Marshal(value)
if err != nil {
return nil, err
@@ -223,6 +224,8 @@ func getRawValueForMetadataKey(user *ScimUser, key metadata.Key) interface{} {
return user.Locale
case metadata.KeyTimezone:
return user.Timezone
case metadata.KeyEmails:
return user.Emails
case metadata.KeyProvisioningDomain:
break
}

34
internal/api/scim/resources/user_patch_test.go
View File

@@ -685,6 +685,39 @@ func TestOperationCollection_Apply(t *testing.T) {
},
wantErr: true,
},
{
name: "replace filter complex subattribute multiple emails primary value",
op: &patch.Operation{
Operation: patch.OperationTypeReplace,
Path: test.Must(filter.ParsePath(`emails[primary eq true].value`)),
Value: json.RawMessage(`"jeanie.rebecca.pendleton@example.com"`),
},
want: &ScimUser{
Emails: []*ScimEmail{
{
Value: "jeanie.rebecca.pendleton@example.com",
Primary: true,
},
},
},
},
{
name: "replace filter complex subattribute multiple emails type value",
op: &patch.Operation{
Operation: patch.OperationTypeReplace,
Path: test.Must(filter.ParsePath(`emails[type eq "work"].value`)),
Value: json.RawMessage(`"jeanie.rebecca.pendleton@example.com"`),
},
want: &ScimUser{
Emails: []*ScimEmail{
{
Value: "jeanie.rebecca.pendleton@example.com",
Primary: true,
Type: "work",
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@@ -711,6 +744,7 @@ func TestOperationCollection_Apply(t *testing.T) {
{
Value: "jeanie.pendleton@example.com",
Primary: true,
Type: "work",
},
},
PhoneNumbers: []*ScimPhoneNumber{