From 78bf962f49ec8b4e308b1771fef8cc2914b658b4 Mon Sep 17 00:00:00 2001 From: Okke Harsta Date: Thu, 5 Dec 2024 11:33:42 +0100 Subject: [PATCH] Fixes #637 --- .../java/myconext/api/UserController.java | 2 +- .../src/main/java/myconext/manage/Manage.java | 3 +- .../java/myconext/manage/RemoteManage.java | 8 +++- .../myconext/model/ExternalLinkedAccount.java | 2 + .../java/myconext/model/IdentityProvider.java | 4 +- .../RemoteCreationController.java | 10 ++++- .../GuestIdpAuthenticationRequestFilter.java | 27 ++++++++++--- .../java/myconext/verify/AttributeMapper.java | 38 ++++++++++++++----- .../RemoteCreationControllerTest.java | 9 +++-- .../myconext/verify/AttributeMapperTest.java | 3 +- 10 files changed, 81 insertions(+), 25 deletions(-) diff --git a/myconext-server/src/main/java/myconext/api/UserController.java b/myconext-server/src/main/java/myconext/api/UserController.java index 0c09a975..38880b5f 100644 --- a/myconext-server/src/main/java/myconext/api/UserController.java +++ b/myconext-server/src/main/java/myconext/api/UserController.java @@ -309,7 +309,7 @@ public int successfullyLoggedIn(@RequestParam("id") String id) { @GetMapping("/sp/institution/names") public ResponseEntity institutionNames(@RequestParam(value = "schac_home") String schacHome) { return ResponseEntity.ok(manage.findIdentityProviderByDomainName(schacHome) - .orElse(new IdentityProvider(new RemoteProvider(schacHome, schacHome, schacHome, null, null), null))); + .orElse(new IdentityProvider(new RemoteProvider(schacHome, schacHome, schacHome, null, null), null, null))); } @Operation(summary = "User details", description = "Retrieve the attributes of the current user") diff --git a/myconext-server/src/main/java/myconext/manage/Manage.java b/myconext-server/src/main/java/myconext/manage/Manage.java index 75dbd7ae..909a160c 100644 --- a/myconext-server/src/main/java/myconext/manage/Manage.java +++ b/myconext-server/src/main/java/myconext/manage/Manage.java @@ -87,7 +87,8 @@ default Map identityProvider(Map map) Map metaDataFields = metaDataFields(map); IdentityProvider identityProvider = new IdentityProvider( remoteProvider, - metaDataFields.get("coin:institution_brin") + metaDataFields.get("coin:institution_brin"), + metaDataFields.get("shibmd:scope:0:allowed") ); Map results = new HashMap<>(); diff --git a/myconext-server/src/main/java/myconext/manage/RemoteManage.java b/myconext-server/src/main/java/myconext/manage/RemoteManage.java index 50317ea0..15585904 100644 --- a/myconext-server/src/main/java/myconext/manage/RemoteManage.java +++ b/myconext-server/src/main/java/myconext/manage/RemoteManage.java @@ -165,13 +165,17 @@ private Optional searchIdentityProvider(String metaDataField, "REQUESTED_ATTRIBUTES", Arrays.asList( "metaDataFields.coin:institution_brin", "metaDataFields.logo:0:url", - "metaDataFields.coin:institution_guid")); + "metaDataFields.coin:institution_guid", + "metaDataFields.shibmd:scope:0:allowed")); HttpEntity> requestEntity = new HttpEntity<>(requestBody, this.headers); return restTemplate.exchange(manageBaseUrl + "/manage/api/internal/search/saml20_idp", HttpMethod.POST, requestEntity, typeReference) .getBody() .stream() - .map(m -> new IdentityProvider(remoteProvider(m), metaDataFields(m).get("coin:institution_brin"))) + .map(m -> new IdentityProvider( + remoteProvider(m), + metaDataFields(m).get("coin:institution_brin"), + metaDataFields(m).get("shibmd:scope:0:allowed"))) .findFirst(); } diff --git a/myconext-server/src/main/java/myconext/model/ExternalLinkedAccount.java b/myconext-server/src/main/java/myconext/model/ExternalLinkedAccount.java index 4669c4a4..64666778 100644 --- a/myconext-server/src/main/java/myconext/model/ExternalLinkedAccount.java +++ b/myconext-server/src/main/java/myconext/model/ExternalLinkedAccount.java @@ -31,6 +31,8 @@ public class ExternalLinkedAccount implements Serializable, ProvisionedLinkedAcc private String subjectIssuer; @Setter private List brinCodes; + @Setter + private List affiliations; private String initials; private String chosenName; diff --git a/myconext-server/src/main/java/myconext/model/IdentityProvider.java b/myconext-server/src/main/java/myconext/model/IdentityProvider.java index 8ec3cf14..a1e444e0 100644 --- a/myconext-server/src/main/java/myconext/model/IdentityProvider.java +++ b/myconext-server/src/main/java/myconext/model/IdentityProvider.java @@ -9,10 +9,12 @@ @Getter public class IdentityProvider extends RemoteProvider { + private String domainName; private String institutionBrin; - public IdentityProvider(RemoteProvider remoteProvider, String institutionBrin) { + public IdentityProvider(RemoteProvider remoteProvider, String institutionBrin, String domainName) { super(remoteProvider.getEntityId(), remoteProvider.getName(), remoteProvider.getNameNl(), remoteProvider.getInstitutionGuid(), remoteProvider.getLogoUrl()); this.institutionBrin = institutionBrin; + this.domainName = domainName; } } diff --git a/myconext-server/src/main/java/myconext/remotecreation/RemoteCreationController.java b/myconext-server/src/main/java/myconext/remotecreation/RemoteCreationController.java index dc558c33..94afd736 100644 --- a/myconext-server/src/main/java/myconext/remotecreation/RemoteCreationController.java +++ b/myconext-server/src/main/java/myconext/remotecreation/RemoteCreationController.java @@ -27,10 +27,13 @@ import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.annotation.AuthenticationPrincipal; +import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.*; +import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.UUID; @@ -266,6 +269,7 @@ public ResponseEntity updateEduID(@Parameter(hidden = true) optionalExternalLinkedAccount.ifPresentOrElse(externalLinkedAccount -> { //Not all external attributes can be changed externalLinkedAccount.setVerification(externalEduID.getVerification()); + externalLinkedAccount.setAffiliations(AttributeMapper.externalAffiliations(externalEduID.getBrinCodes(), manage)); externalLinkedAccount.setBrinCodes(externalEduID.getBrinCodes()); externalLinkedAccount.setDateOfBirth(AttributeMapper.parseDate(externalEduID.getDateOfBirth())); }, () -> { @@ -310,7 +314,7 @@ public ResponseEntity deleteEduID(@Parameter(hidden = true) @Authenticatio return ResponseEntity.status(HttpStatus.NO_CONTENT).build(); } - private static RemoteProvider getRemoteProvider(RemoteUser remoteUser, String remoteUserName) { + private RemoteProvider getRemoteProvider(RemoteUser remoteUser, String remoteUserName) { return new RemoteProvider( null, remoteUserName, @@ -319,4 +323,8 @@ private static RemoteProvider getRemoteProvider(RemoteUser remoteUser, String re String.format("https://static.surfconext.nl/logos/org/%s.png", remoteUser.getInstitutionGUID())); } + private List brinCodesSorted(List brinCodes) { + return CollectionUtils.isEmpty(brinCodes) ? brinCodes : brinCodes.stream().sorted().toList(); + } + } diff --git a/myconext-server/src/main/java/myconext/security/GuestIdpAuthenticationRequestFilter.java b/myconext-server/src/main/java/myconext/security/GuestIdpAuthenticationRequestFilter.java index bb5a171a..1f571e10 100644 --- a/myconext-server/src/main/java/myconext/security/GuestIdpAuthenticationRequestFilter.java +++ b/myconext-server/src/main/java/myconext/security/GuestIdpAuthenticationRequestFilter.java @@ -772,18 +772,35 @@ protected List attributes(User user, String requesterEntityId) { values.forEach(value -> attributes.add(attribute(key, value)))); - List scopedAffiliations = linkedAccounts.stream() + //we need a mutable list + List scopedAffiliations = new ArrayList<>(linkedAccounts.stream() .map(linkedAccount -> linkedAccount.getEduPersonAffiliations().stream() .map(affiliation -> affiliation.contains("@") ? affiliation : String.format("%s@%s", affiliation, linkedAccount.getSchacHomeOrganization())) .collect(toList())) - .flatMap(Collection::stream).distinct().collect(Collectors.toList()); + .flatMap(Collection::stream) + .distinct() + .toList()); scopedAffiliations.add("affiliate@eduid.nl"); scopedAffiliations.forEach(aff -> attributes.add(attribute("urn:mace:dir:attribute-def:eduPersonScopedAffiliation", aff))); - List affiliations = scopedAffiliations.stream().map(affiliation -> affiliation.substring(0, affiliation.indexOf("@"))) - .distinct().collect(toList()); - affiliations.forEach(aff -> attributes.add(attribute("urn:mace:dir:attribute-def:eduPersonAffiliation", aff))); + scopedAffiliations.stream() + .map(affiliation -> affiliation.substring(0, affiliation.indexOf("@"))) + .distinct() + .forEach(aff -> attributes.add(attribute("urn:mace:dir:attribute-def:eduPersonAffiliation", aff))); + + List studentAffiliations = user.getExternalLinkedAccounts().stream() + .map(ExternalLinkedAccount::getAffiliations) + .filter(externalLinkedAccountAffiliations -> !CollectionUtils.isEmpty(externalLinkedAccountAffiliations)) + .flatMap(Collection::stream) + .distinct() + .toList(); + studentAffiliations.forEach(aff -> attributes.add(attribute("urn:mace:dir:attribute-def:eduPersonScopedAffiliation", aff))); + if (!studentAffiliations.isEmpty() && attributes.stream() + .noneMatch(samlAttribute -> samlAttribute.getName().equals("urn:mace:dir:attribute-def:eduPersonAffiliation") && + samlAttribute.getValue().equals("student"))) { + attributes.add(attribute("urn:mace:dir:attribute-def:eduPersonAffiliation", "student")); + } return attributes; } diff --git a/myconext-server/src/main/java/myconext/verify/AttributeMapper.java b/myconext-server/src/main/java/myconext/verify/AttributeMapper.java index 4baa9715..0326a189 100644 --- a/myconext-server/src/main/java/myconext/verify/AttributeMapper.java +++ b/myconext-server/src/main/java/myconext/verify/AttributeMapper.java @@ -1,26 +1,29 @@ package myconext.verify; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.databind.ObjectMapper; import lombok.SneakyThrows; +import myconext.manage.Manage; import myconext.model.ExternalLinkedAccount; import myconext.model.IdpScoping; import myconext.model.Verification; import myconext.model.VerifyIssuer; import myconext.remotecreation.NewExternalEduID; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.annotation.Transient; import org.springframework.stereotype.Service; +import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; -import java.time.*; +import java.time.DateTimeException; +import java.time.Instant; +import java.time.LocalDate; +import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; import java.time.temporal.ChronoUnit; -import java.time.temporal.TemporalAccessor; -import java.util.Date; -import java.util.HashMap; -import java.util.Map; -import java.util.Objects; +import java.util.*; import java.util.regex.Pattern; import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; @@ -37,10 +40,12 @@ public class AttributeMapper { ); private final ObjectMapper objectMapper; + private final Manage manage; @Autowired - public AttributeMapper(ObjectMapper objectMapper) { + public AttributeMapper(ObjectMapper objectMapper, Manage manage) { this.objectMapper = objectMapper; + this.manage = manage; } public ExternalLinkedAccount externalLinkedAccountFromAttributes( @@ -145,7 +150,7 @@ public ExternalLinkedAccount externalLinkedAccountFromAttributes( } public ExternalLinkedAccount createExternalLinkedAccount(NewExternalEduID eduID, IdpScoping idpScoping) { - return new ExternalLinkedAccount( + ExternalLinkedAccount externalLinkedAccount = new ExternalLinkedAccount( //String subjectId eduID.getIdentifier(), //IdpScoping idpScoping @@ -188,9 +193,9 @@ public ExternalLinkedAccount createExternalLinkedAccount(NewExternalEduID eduID, Date.from(Instant.now().plus(DEFAULT_EXPIRATION_YEARS * 365, ChronoUnit.DAYS)), //boolean external true - ); - + externalLinkedAccount.setAffiliations(externalAffiliations(eduID.getBrinCodes(), manage)); + return externalLinkedAccount; } @SneakyThrows @@ -253,6 +258,19 @@ public static Date parseDate(String dateString) { return null; } + public static List externalAffiliations(List brinCodes, Manage manage) { + return CollectionUtils.isEmpty(brinCodes) ? Collections.emptyList() : + brinCodes.stream() + .map(manage::findIdentityProviderByBrinCode) + .filter(Optional::isPresent) + .map(Optional::get) + .filter(idp -> StringUtils.hasText(idp.getDomainName())) + .map(idp -> String.format("student@%s", idp.getDomainName())) + .toList(); + } + + + private String getAttribute(Map attributes, String key) { return (String) attributes.get(key); } diff --git a/myconext-server/src/test/java/myconext/remotecreation/RemoteCreationControllerTest.java b/myconext-server/src/test/java/myconext/remotecreation/RemoteCreationControllerTest.java index 408e737e..07799d6b 100644 --- a/myconext-server/src/test/java/myconext/remotecreation/RemoteCreationControllerTest.java +++ b/myconext-server/src/test/java/myconext/remotecreation/RemoteCreationControllerTest.java @@ -153,7 +153,7 @@ void createEduIDHappyFlow() { "19880327", UUID.randomUUID().toString(), Verification.Decentraal, - null + List.of("ST42") ); UpdateExternalEduID externalEduIDResult = given() .when() @@ -177,7 +177,9 @@ void createEduIDHappyFlow() { assertEquals(institutionGUID, newEduID.getServices().get(0).getInstitutionGuid()); assertEquals(1, user.getExternalLinkedAccounts().size()); - assertEquals(IdpScoping.studielink, user.getExternalLinkedAccounts().get(0).getIdpScoping()); + ExternalLinkedAccount externalLinkedAccount = user.getExternalLinkedAccounts().get(0); + assertEquals(IdpScoping.studielink, externalLinkedAccount.getIdpScoping()); + assertEquals("student@aap.nl", externalLinkedAccount.getAffiliations().getFirst()); } @Test @@ -310,7 +312,7 @@ void updateEduIDHappyFlowWithNewUser() { .extract() .as(new TypeRef<>() { }); - externalEduIDResult.setBrinCodes(List.of("QWER")); + externalEduIDResult.setBrinCodes(List.of("ST42")); externalEduIDResult.setVerification(Verification.Geverifieerd); given() .when() @@ -329,6 +331,7 @@ void updateEduIDHappyFlowWithNewUser() { assertEquals(1, user.getExternalLinkedAccounts().size()); ExternalLinkedAccount externalLinkedAccount = user.getExternalLinkedAccounts().get(0); assertEquals(externalEduIDResult.getBrinCodes(), externalLinkedAccount.getBrinCodes()); + assertEquals("student@aap.nl", externalLinkedAccount.getAffiliations().getFirst()); assertEquals(Verification.Geverifieerd, externalLinkedAccount.getVerification()); } diff --git a/myconext-server/src/test/java/myconext/verify/AttributeMapperTest.java b/myconext-server/src/test/java/myconext/verify/AttributeMapperTest.java index 52cb4a8b..f1024acb 100644 --- a/myconext-server/src/test/java/myconext/verify/AttributeMapperTest.java +++ b/myconext-server/src/test/java/myconext/verify/AttributeMapperTest.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import lombok.SneakyThrows; +import myconext.manage.MockManage; import myconext.model.ExternalLinkedAccount; import myconext.model.IdpScoping; import myconext.model.VerifyIssuer; @@ -22,7 +23,7 @@ class AttributeMapperTest { private final ObjectMapper objectMapper = new ObjectMapper(); - private final AttributeMapper attributeMapper = new AttributeMapper(objectMapper); + private final AttributeMapper attributeMapper = new AttributeMapper(objectMapper, new MockManage(objectMapper)); @SneakyThrows @Test