From b99b3a75691bfc7e0a8f92e593e3e33c76b0f61f Mon Sep 17 00:00:00 2001 From: Okke Harsta Date: Wed, 17 Dec 2025 14:24:51 +0100 Subject: [PATCH] Fixes #1090 --- .../provision/ProvisioningServiceDefault.java | 2 +- .../invite/provision/scim/UserRequest.java | 36 ++++++++++++------- server/src/test/java/invite/AbstractTest.java | 4 +-- .../invite/api/InvitationControllerTest.java | 11 +++--- .../provision/scim/UserRequestTest.java | 11 ++++++ 5 files changed, 43 insertions(+), 21 deletions(-) diff --git a/server/src/main/java/invite/provision/ProvisioningServiceDefault.java b/server/src/main/java/invite/provision/ProvisioningServiceDefault.java index a51ca115..a696f671 100644 --- a/server/src/main/java/invite/provision/ProvisioningServiceDefault.java +++ b/server/src/main/java/invite/provision/ProvisioningServiceDefault.java @@ -111,7 +111,7 @@ public Optional newUserRequest(User user) { .forEach(provisioning -> { UserRequest request = new UserRequest(user, provisioning); if (ScimUserIdentifier.eduID.equals(provisioning.getScimUserIdentifier()) && - request.getExternalId().equals(user.getEduId())) { + request.getUserName().equals(user.getEduId())) { //No fallback for failure this.eduID.provisionEduid(new EduIDProvision(user.getEduId(), provisioning.getInstitutionGUID())); } diff --git a/server/src/main/java/invite/provision/scim/UserRequest.java b/server/src/main/java/invite/provision/scim/UserRequest.java index 85110065..c0131c23 100644 --- a/server/src/main/java/invite/provision/scim/UserRequest.java +++ b/server/src/main/java/invite/provision/scim/UserRequest.java @@ -13,6 +13,15 @@ import java.util.Collections; import java.util.List; +/** + * The SCIM client (e.g., the Invite server) generates and owns the externalId. It also supplies the userName, + * whose uniqueness and semantics are enforced by the SCIM service provider (e.g. the educational institutions). The + * SCIM service provider generates the immutable id and returns it in the response to a 'create' request. The SCIM + * client stores this id and uses it in subsequent PUT or PATCH requests to address the user resource. + * + * The username which is used by the Invite server can be configured in Manage with the `scim_user_identifier` + * attribute, and defaults to the EPPN. + */ @Getter public class UserRequest implements Serializable { @@ -29,8 +38,9 @@ public class UserRequest implements Serializable { private final List phoneNumbers; public UserRequest(User user, Provisioning provisioning) { - this.externalId = this.resolveExternalId(user, provisioning); - this.userName = user.getEduPersonPrincipalName(); + String resolvedUserName = this.resolveUserName(user, provisioning); + this.userName = resolvedUserName; + this.externalId = resolvedUserName; this.name = new Name(user.getName(), user.getFamilyName(), user.getGivenName()); this.displayName = user.getName(); this.emails = List.of(new Email("other",user.getEmail())); @@ -46,39 +56,39 @@ public UserRequest(User user, Provisioning provisioning, String remoteScimIdenti this.id = remoteScimIdentifier; } - private String resolveExternalId(User user, Provisioning provisioning) { + private String resolveUserName(User user, Provisioning provisioning) { ScimUserIdentifier scimUserIdentifier = provisioning.getScimUserIdentifier(); //Backward compatibility for older Provisionings without default - String defaultExternalId = user.getEduPersonPrincipalName(); + String defaultUserName = user.getEduPersonPrincipalName(); if (scimUserIdentifier == null) { - return defaultExternalId; + return defaultUserName; } boolean missingScimUserIdentifierValue = false; - String externalIdIdentifier = switch (scimUserIdentifier) { + String configuredUserName = switch (scimUserIdentifier) { case subject_id -> { missingScimUserIdentifierValue = !StringUtils.hasText(user.getSubjectId()); - yield missingScimUserIdentifierValue ? defaultExternalId : user.getSubjectId(); + yield missingScimUserIdentifierValue ? defaultUserName : user.getSubjectId(); } case uids -> { missingScimUserIdentifierValue = !StringUtils.hasText(user.getUid()); - yield missingScimUserIdentifierValue ? defaultExternalId : user.getUid(); + yield missingScimUserIdentifierValue ? defaultUserName : user.getUid(); } case email -> { missingScimUserIdentifierValue = !StringUtils.hasText(user.getEmail()); - yield missingScimUserIdentifierValue ? defaultExternalId : user.getEmail(); + yield missingScimUserIdentifierValue ? defaultUserName : user.getEmail(); } case eduID -> { missingScimUserIdentifierValue = !StringUtils.hasText(user.getEduId()); - yield missingScimUserIdentifierValue ? defaultExternalId : user.getEduId(); + yield missingScimUserIdentifierValue ? defaultUserName : user.getEduId(); } - default -> defaultExternalId; + default -> defaultUserName; }; if (missingScimUserIdentifierValue) { LOG.warn(String.format( "Missing attribute %s for SCIM provisioning to %s for user %s. Return defaultExternalId %s", - scimUserIdentifier, provisioning.getEntityId(), user.getSub(), defaultExternalId) + scimUserIdentifier, provisioning.getEntityId(), user.getSub(), defaultUserName) ); } - return externalIdIdentifier; + return configuredUserName; } } diff --git a/server/src/test/java/invite/AbstractTest.java b/server/src/test/java/invite/AbstractTest.java index 5579b24e..399ab474 100644 --- a/server/src/test/java/invite/AbstractTest.java +++ b/server/src/test/java/invite/AbstractTest.java @@ -336,8 +336,8 @@ protected JWTClaimsSet getJwtClaimsSet(String clientId, String sub, String redir .subject(StringUtils.hasText(sub) ? sub : "sub") .notBeforeTime(new Date(System.currentTimeMillis())) .claim("redirect_uri", redirectURI) - .claim("eduperson_principal_name", sub) - .claim("email", sub) + .claim("eduperson_principal_name", userInfo.getOrDefault("eduperson_principal_name", sub)) + .claim("email", userInfo.getOrDefault("email", sub)) .claim("given_name", userInfo.get("given_name")) .claim("family_name", userInfo.get("family_name")) .claim("nonce", nonce) diff --git a/server/src/test/java/invite/api/InvitationControllerTest.java b/server/src/test/java/invite/api/InvitationControllerTest.java index 4ade39a4..312da87b 100644 --- a/server/src/test/java/invite/api/InvitationControllerTest.java +++ b/server/src/test/java/invite/api/InvitationControllerTest.java @@ -518,15 +518,15 @@ void acceptPatch() throws Exception { @Test void acceptPatchForDifferentScimIdentifier() throws Exception { - String externalId = "subject@example.com"; + String subjectId = "subject@example.com"; AccessCookieFilter accessCookieFilter = openIDConnectFlow("/api/v1/users/login", "new@prov.com", m -> { - - m.put("subject_id", externalId); + m.put("subject_id", subjectId); + m.put("eduperson_principal_name", "eppn@example.com"); return m; }); Invitation invitation = invitationRepository.findByHash(Authority.GUEST.name()).get(); - //Provisioning with id=4 hs different scim_user_identifier + //Provisioning with an application(id=4) has the '' as scim_user_identifier stubForManageProvisioning(List.of("4")); stubForCreateScimUser(); stubForCreateScimRole(); @@ -549,7 +549,8 @@ void acceptPatchForDifferentScimIdentifier() throws Exception { Map request = objectMapper.readValue(requests.get(0).getBodyAsString(), new TypeReference<>() { }); - assertEquals(externalId, request.get("externalId")); + assertEquals(subjectId, request.get("userName")); + assertEquals(subjectId, request.get("externalId")); } @Test diff --git a/server/src/test/java/invite/provision/scim/UserRequestTest.java b/server/src/test/java/invite/provision/scim/UserRequestTest.java index 29b10fcb..5fb07690 100644 --- a/server/src/test/java/invite/provision/scim/UserRequestTest.java +++ b/server/src/test/java/invite/provision/scim/UserRequestTest.java @@ -35,6 +35,7 @@ void beforeEach() { void externalIdEduPersonPrincipalName() { Provisioning provisioning = getProvisioning(ScimUserIdentifier.eduperson_principal_name); UserRequest userRequest = new UserRequest(user, provisioning); + assertEquals(user.getEduPersonPrincipalName(), userRequest.getUserName()); assertEquals(user.getEduPersonPrincipalName(), userRequest.getExternalId()); } @@ -42,10 +43,12 @@ void externalIdEduPersonPrincipalName() { void externalIdEduId() { Provisioning provisioning = getProvisioning(ScimUserIdentifier.eduID); UserRequest userRequest = new UserRequest(user, provisioning); + assertEquals(user.getEduId(), userRequest.getUserName()); assertEquals(user.getEduId(), userRequest.getExternalId()); ReflectionTestUtils.setField(user, "eduId", null); userRequest = new UserRequest(user, provisioning); + assertEquals(user.getEduPersonPrincipalName(), userRequest.getUserName()); assertEquals(user.getEduPersonPrincipalName(), userRequest.getExternalId()); } @@ -53,10 +56,12 @@ void externalIdEduId() { void externalIdEmail() { Provisioning provisioning = getProvisioning(ScimUserIdentifier.email); UserRequest userRequest = new UserRequest(user, provisioning); + assertEquals(user.getEmail(), userRequest.getUserName()); assertEquals(user.getEmail(), userRequest.getExternalId()); ReflectionTestUtils.setField(user, "email", null); userRequest = new UserRequest(user, provisioning); + assertEquals(user.getEduPersonPrincipalName(), userRequest.getUserName()); assertEquals(user.getEduPersonPrincipalName(), userRequest.getExternalId()); } @@ -64,10 +69,12 @@ void externalIdEmail() { void externalIdUid() { Provisioning provisioning = getProvisioning(ScimUserIdentifier.uids); UserRequest userRequest = new UserRequest(user, provisioning); + assertEquals(user.getUid(), userRequest.getUserName()); assertEquals(user.getUid(), userRequest.getExternalId()); ReflectionTestUtils.setField(user, "uid", null); userRequest = new UserRequest(user, provisioning); + assertEquals(user.getEduPersonPrincipalName(), userRequest.getUserName()); assertEquals(user.getEduPersonPrincipalName(), userRequest.getExternalId()); } @@ -75,10 +82,12 @@ void externalIdUid() { void externalIdSubjectId() { Provisioning provisioning = getProvisioning(ScimUserIdentifier.subject_id); UserRequest userRequest = new UserRequest(user, provisioning); + assertEquals(user.getSubjectId(), userRequest.getUserName()); assertEquals(user.getSubjectId(), userRequest.getExternalId()); ReflectionTestUtils.setField(user, "subjectId", null); userRequest = new UserRequest(user, provisioning); + assertEquals(user.getEduPersonPrincipalName(), userRequest.getUserName()); assertEquals(user.getEduPersonPrincipalName(), userRequest.getExternalId()); } @@ -91,6 +100,7 @@ void externalIdDefault() { "scim_password", "secret" )); UserRequest userRequest = new UserRequest(user, provisioning); + assertEquals(user.getEduPersonPrincipalName(), userRequest.getUserName()); assertEquals(user.getEduPersonPrincipalName(), userRequest.getExternalId()); } @@ -104,6 +114,7 @@ void externalIdAvoidNullPointer() { )); ReflectionTestUtils.setField(provisioning, "scimUserIdentifier", null); UserRequest userRequest = new UserRequest(user, provisioning); + assertEquals(user.getEduPersonPrincipalName(), userRequest.getUserName()); assertEquals(user.getEduPersonPrincipalName(), userRequest.getExternalId()); }