Skip to content

Commit 811b59b

Browse files
oharstaphavekes
oharsta
authored and
phavekes
committed
Fixes #633 
1 parent be8a48a commit 811b59b

File tree

10 files changed

+101
-35
lines changed

10 files changed

+101
-35
lines changed

‎server/src/main/java/invite/eduid/EduID.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package invite.eduid;
22

3+
import invite.exception.RemoteException;
34
import org.apache.commons.logging.Log;
45
import org.apache.commons.logging.LogFactory;
56
import org.springframework.beans.factory.annotation.Value;
@@ -29,14 +30,18 @@ public EduID(@Value("${myconext.uri}") String uri,
2930
this.headers = initHttpHeaders();
3031
}
3132

32-
public Optional<String> provisionEduid(EduIDProvision eduIDProvision) {
33+
public String provisionEduid(EduIDProvision eduIDProvision) {
3334
HttpEntity<EduIDProvision> requestEntity = new HttpEntity<>(eduIDProvision, headers);
3435
try {
3536
ResponseEntity<EduIDProvision> responseEntity = restTemplate.exchange(uri, HttpMethod.POST, requestEntity, EduIDProvision.class);
36-
return Optional.of(responseEntity.getBody().getEduIDValue());
37+
String eduIDValue = responseEntity.getBody().getEduIDValue();
38+
39+
LOG.info(String.format("eduID %s for institution %s is eduID %s after eduID provisioning",
40+
eduIDProvision.getEduIDValue(), eduIDProvision.getInstitutionGUID(), eduIDValue));
41+
42+
return eduIDValue;
3743
} catch (RuntimeException e) {
38-
LOG.error("Error in provisionEduid", e);
39-
return Optional.empty();
44+
throw new RemoteException(HttpStatus.BAD_REQUEST, "Error in provisionEduid", e);
4045
}
4146
}
4247

‎server/src/main/java/invite/provision/ProvisioningServiceDefault.java‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,10 @@ public Optional<GraphResponse> newUserRequest(User user) {
109109
.filter(provisioning -> this.remoteProvisionedUserRepository.findByManageProvisioningIdAndUser(provisioning.getId(), user)
110110
.isEmpty())
111111
.forEach(provisioning -> {
112-
UserRequest request = new UserRequest(user, provisioning);
113-
if (ScimUserIdentifier.eduID.equals(provisioning.getScimUserIdentifier()) &&
114-
request.getUserName().equals(user.getEduId())) {
115-
//No fallback for failure
116-
this.eduID.provisionEduid(new EduIDProvision(user.getEduId(), provisioning.getInstitutionGUID()));
117-
}
118-
String userRequest = prettyJson(request);
119-
Optional<ProvisioningResponse> provisioningResponse = this.newRequest(provisioning, userRequest, user);
112+
UserRequest userRequest = new UserRequest(user, provisioning);
113+
this.resolveInstitutionalEduID(user, provisioning, userRequest);
114+
String userRequestJson = prettyJson(userRequest);
115+
Optional<ProvisioningResponse> provisioningResponse = this.newRequest(provisioning, userRequestJson, user);
120116
provisioningResponse.ifPresent(response -> {
121117
if (!response.isErrorResponse() && StringUtils.hasText(response.remoteIdentifier())) {
122118
RemoteProvisionedUser remoteProvisionedUser = new RemoteProvisionedUser(user, response.remoteIdentifier(), provisioning.getId());
@@ -130,6 +126,17 @@ public Optional<GraphResponse> newUserRequest(User user) {
130126
return Optional.ofNullable(graphResponseReference.get());
131127
}
132128

129+
private void resolveInstitutionalEduID(User user, Provisioning provisioning, UserRequest request) {
130+
if (ScimUserIdentifier.eduID.equals(provisioning.getScimUserIdentifier()) &&
131+
request.getUserName().equals(user.getEduId())) {
132+
//No fallback for failure
133+
EduIDProvision eduIDProvision = new EduIDProvision(user.getEduId(), provisioning.getInstitutionGUID());
134+
String eduIdProvisionResponse = this.eduID.provisionEduid(eduIDProvision);
135+
//Empty eduIdProvisionResponse is handled by invite.eduid.EduID
136+
request.setInsitutionalEduID(eduIdProvisionResponse);
137+
}
138+
}
139+
133140
@Override
134141
public void updateUserRequest(User user) {
135142
List<Provisioning> userProvisionings = getProvisionings(user);
@@ -144,8 +151,10 @@ public void updateUserRequest(User user) {
144151
Optional<RemoteProvisionedUser> provisionedUserOptional =
145152
this.remoteProvisionedUserRepository.findByManageProvisioningIdAndUser(provisioning.getId(), user);
146153
provisionedUserOptional.ifPresent(remoteProvisionedUser -> {
147-
String userRequest = prettyJson(new UserRequest(user, provisioning, remoteProvisionedUser.getRemoteIdentifier()));
148-
this.updateRequest(provisioning, userRequest, APIType.USER_API, remoteProvisionedUser.getRemoteIdentifier(), HttpMethod.PUT);
154+
UserRequest userRequest = new UserRequest(user, provisioning, remoteProvisionedUser.getRemoteIdentifier());
155+
this.resolveInstitutionalEduID(user, provisioning, userRequest);
156+
String userRequestJson = prettyJson(userRequest);
157+
this.updateRequest(provisioning, userRequestJson, APIType.USER_API, remoteProvisionedUser.getRemoteIdentifier(), HttpMethod.PUT);
149158
});
150159
}
151160
});
@@ -198,8 +207,10 @@ public void deleteUserRequest(User user) {
198207
if (provisionedUserOptional.isPresent()) {
199208
RemoteProvisionedUser remoteProvisionedUser = provisionedUserOptional.get();
200209
String remoteIdentifier = remoteProvisionedUser.getRemoteIdentifier();
201-
String userRequest = prettyJson(new UserRequest(user, provisioning, remoteIdentifier));
202-
this.deleteRequest(provisioning, userRequest, user, remoteIdentifier);
210+
UserRequest userRequest = new UserRequest(user, provisioning, remoteIdentifier);
211+
this.resolveInstitutionalEduID(user, provisioning, userRequest);
212+
String userRequestJson = prettyJson(userRequest);
213+
this.deleteRequest(provisioning, userRequestJson, user, remoteIdentifier);
203214
this.remoteProvisionedUserRepository.delete(remoteProvisionedUser);
204215
}
205216
});

‎server/src/main/java/invite/provision/scim/UserRequest.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ public class UserRequest implements Serializable {
2828
private static final Log LOG = LogFactory.getLog(UserRequest.class);
2929

3030
private final List<String> schemas = Collections.singletonList("urn:ietf:params:scim:schemas:core:2.0:User");
31-
private final String externalId;
32-
private final String userName;
31+
private String externalId;
32+
private String userName;
3333
private final Name name;
3434
private String id;
3535
private final String displayName;
@@ -56,6 +56,12 @@ public UserRequest(User user, Provisioning provisioning, String remoteScimIdenti
5656
this.id = remoteScimIdentifier;
5757
}
5858

59+
public void setInsitutionalEduID(String eduIDValue) {
60+
this.userName = eduIDValue;
61+
this.externalId = eduIDValue;
62+
}
63+
64+
5965
private String resolveUserName(User user, Provisioning provisioning) {
6066
ScimUserIdentifier scimUserIdentifier = provisioning.getScimUserIdentifier();
6167
//Backward compatibility for older Provisionings without default

‎server/src/test/java/invite/AbstractTest.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package invite;
22

33
import invite.config.HashGenerator;
4+
import invite.eduid.EduIDProvision;
45
import invite.manage.EntityType;
56
import invite.manage.LocalManage;
67
import invite.model.*;
@@ -142,7 +143,7 @@ public abstract class AbstractTest {
142143
protected LocalManage localManage;
143144

144145
@RegisterExtension
145-
WireMockExtension mockServer = new WireMockExtension(8081);
146+
protected WireMockExtension mockServer = new WireMockExtension(8081);
146147

147148
@LocalServerPort
148149
protected int port;
@@ -345,14 +346,20 @@ protected JWTClaimsSet getJwtClaimsSet(String clientId, String sub, String redir
345346
return builder.build();
346347
}
347348

349+
protected void stubForProvisionEduID(String eduIdForInstitution) throws JsonProcessingException {
350+
EduIDProvision eduIDProvision = new EduIDProvision(eduIdForInstitution, "semantically-not-used");
351+
stubFor(post(urlPathMatching("/myconext/api/invite/provision-eduid")).willReturn(aResponse()
352+
.withHeader("Content-Type", "application/json")
353+
.withBody(objectMapper.writeValueAsString(eduIDProvision))));
354+
}
355+
348356
protected void stubForManageProvisioning(List<String> applicationIdentifiers) throws JsonProcessingException {
349357
List<Map<String, Object>> providers = localManage.provisioning(applicationIdentifiers);
350358
String body = writeValueAsString(providers);
351359
stubFor(post(urlPathMatching("/manage/api/internal/provisioning"))
352360
.willReturn(aResponse().withHeader("Content-Type", "application/json")
353361
.withBody(body)
354362
.withStatus(200)));
355-
356363
}
357364

358365
protected void stubForManageProvidersAllowedByIdP(String organisationGUID) throws JsonProcessingException {

‎server/src/test/java/invite/api/InvitationControllerTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,7 @@ void acceptForUpgradingExistingUserRole() throws Exception {
364364
.findFirst().get().getAuthority();
365365
assertEquals(Authority.GUEST, authority);
366366

367+
super.stubForProvisionEduID(UUID.randomUUID().toString());
367368
//Because the user is changed and provisionings are queried
368369
stubForManageProvisioning(List.of());
369370
AccessCookieFilter accessCookieFilter = openIDConnectFlow("/api/v1/users/login", GUEST_SUB);

‎server/src/test/java/invite/api/RoleControllerTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ void updateApplications() throws Exception {
243243

244244
AccessCookieFilter accessCookieFilter = openIDConnectFlow("/api/v1/users/login", INSTITUTION_ADMIN_SUB);
245245

246+
super.stubForProvisionEduID(UUID.randomUUID().toString());
246247
super.stubForManagerProvidersByIdIn(EntityType.SAML20_SP, List.of("1", "2", "4"));
247248
super.stubForManageProvisioning(List.of("1", "2", "4"));
248249
super.stubForCreateGraphUser();

‎server/src/test/java/invite/api/UserControllerTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,7 @@ void meUpdateScim() throws Exception {
636636
remoteProvisionedUserRepository.save(remoteProvisionedUser);
637637

638638
super.stubForManageProvisioning(List.of("1", "4", "5"));
639+
super.stubForProvisionEduID(UUID.randomUUID().toString());
639640
super.stubForUpdateScimUser();
640641

641642
//This will trigger the SCIM update request, see CustomOidcUserService#loadUser

‎server/src/test/java/invite/api/UserRoleControllerTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,7 @@ private void doUserRoleProvisioning(UserRoleProvisioning userRoleProvisioning, S
557557
super.stubForManagerProvidersByIdIn(EntityType.SAML20_SP, List.of("1", "2"));
558558
super.stubForManageProvidersAllowedByIdP(ORGANISATION_GUID);
559559
super.stubForManageProvisioning(List.of("1", "2"));
560-
560+
super.stubForProvisionEduID(UUID.randomUUID().toString());
561561
super.stubForCreateScimUser();
562562
super.stubForCreateGraphUser();
563563
super.stubForCreateScimRole();

‎server/src/test/java/invite/eduid/EduIDTest.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import invite.WireMockExtension;
44
import com.fasterxml.jackson.databind.ObjectMapper;
5+
import invite.exception.RemoteException;
56
import lombok.SneakyThrows;
67
import org.junit.jupiter.api.Test;
78
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -10,8 +11,7 @@
1011
import java.util.UUID;
1112

1213
import static com.github.tomakehurst.wiremock.client.WireMock.*;
13-
import static org.junit.jupiter.api.Assertions.assertEquals;
14-
import static org.junit.jupiter.api.Assertions.assertTrue;
14+
import static org.junit.jupiter.api.Assertions.*;
1515

1616
class EduIDTest {
1717

@@ -30,8 +30,8 @@ void provisionEduid() {
3030
stubFor(post(urlPathMatching("/myconext/api/invite/provision-eduid")).willReturn(aResponse()
3131
.withHeader("Content-Type", "application/json")
3232
.withBody(objectMapper.writeValueAsString(eduIDProvision))));
33-
Optional<String> eduid = eduID.provisionEduid(eduIDProvision);
34-
assertEquals(eduIDProvision.getEduIDValue(), eduid.get());
33+
String eduid = eduID.provisionEduid(eduIDProvision);
34+
assertEquals(eduIDProvision.getEduIDValue(), eduid);
3535
}
3636

3737
@SneakyThrows
@@ -41,7 +41,6 @@ void provisionEduid404() {
4141
stubFor(post(urlPathMatching("/myconext/api/invite/provision-eduid")).willReturn(aResponse()
4242
.withHeader("Content-Type", "application/json")
4343
.withStatus(404)));
44-
Optional<String> eduid = eduID.provisionEduid(eduIDProvision);
45-
assertTrue(eduid.isEmpty());
44+
assertThrows(RemoteException.class, ()-> eduID.provisionEduid(eduIDProvision)) ;
4645
}
4746
}

‎server/src/test/java/invite/provision/ProvisioningServiceDefaultTest.java‎

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,21 @@
11
package invite.provision;
22

33
import com.fasterxml.jackson.core.JsonProcessingException;
4+
import com.fasterxml.jackson.core.type.TypeReference;
45
import com.github.tomakehurst.wiremock.stubbing.ServeEvent;
56
import com.github.tomakehurst.wiremock.verification.LoggedRequest;
67
import invite.AbstractTest;
7-
import invite.eduid.EduIDProvision;
88
import invite.exception.RemoteException;
9-
import invite.model.*;
9+
import invite.model.Authority;
10+
import invite.model.RemoteProvisionedGroup;
11+
import invite.model.RemoteProvisionedUser;
12+
import invite.model.Role;
13+
import invite.model.User;
14+
import invite.model.UserRole;
1015
import invite.provision.scim.OperationType;
11-
import invite.provision.scim.UserRequest;
1216
import org.junit.jupiter.api.Test;
1317
import org.springframework.beans.factory.annotation.Autowired;
18+
import org.springframework.http.HttpStatus;
1419

1520
import java.util.List;
1621
import java.util.Map;
@@ -49,19 +54,28 @@ void newUserRequestWithInvalidRemoteResponse() throws JsonProcessingException {
4954
@Test
5055
void newUserRequestWithEduIDProvisioning() throws JsonProcessingException {
5156
User user = userRepository.findBySubIgnoreCase(GUEST_SUB).get();
52-
5357
this.stubForManageProvisioning(List.of("1"));
5458

55-
EduIDProvision eduIDProvision = new EduIDProvision(user.getEduId(), UUID.randomUUID().toString());
56-
stubFor(post(urlPathMatching("/myconext/api/invite/provision-eduid")).willReturn(aResponse()
57-
.withHeader("Content-Type", "application/json")
58-
.withBody(objectMapper.writeValueAsString(eduIDProvision))));
59+
String eduIdForInstitution = UUID.randomUUID().toString();
60+
super.stubForProvisionEduID(eduIdForInstitution);
5961

6062
String remoteScimIdentifier = this.stubForCreateScimUser();
6163
provisioningService.newUserRequest(user);
64+
List<ServeEvent> events = this.mockServer.getAllServeEvents();
65+
ServeEvent serveEvent = events.stream()
66+
.filter(event -> event.getRequest().getUrl().equalsIgnoreCase("/api/scim/v2/Users"))
67+
.findFirst()
68+
.orElseThrow(IllegalArgumentException::new);
69+
//UserRequest can not be deserialized from String due to missing constructors and setters
70+
Map<String, Object> userRequest = objectMapper.readValue(serveEvent.getRequest().getBodyAsString(), new TypeReference<>() {
71+
});
72+
assertEquals(eduIdForInstitution, userRequest.get("userName"));
73+
assertEquals(eduIdForInstitution, userRequest.get("externalId"));
74+
6275
List<RemoteProvisionedUser> remoteProvisionedUsers = remoteProvisionedUserRepository.findAll();
6376
assertEquals(1, remoteProvisionedUsers.size());
6477
assertEquals(remoteScimIdentifier, remoteProvisionedUsers.get(0).getRemoteIdentifier());
78+
6579
}
6680

6781
@Test
@@ -93,6 +107,7 @@ void updateUserRequest() throws JsonProcessingException {
93107
remoteProvisionedUserRepository.save(remoteProvisionedUser);
94108
this.stubForManageProvisioning(List.of("1", "4", "5"));
95109
this.stubForUpdateScimUser();
110+
super.stubForProvisionEduID(UUID.randomUUID().toString());
96111
provisioningService.updateUserRequest(user);
97112
List<LoggedRequest> loggedRequests = findAll(putRequestedFor(urlPathMatching(String.format("/api/scim/v2/Users/(.*)"))));
98113

@@ -101,6 +116,26 @@ void updateUserRequest() throws JsonProcessingException {
101116
assertEquals(remoteScimIdentifier, userRequest.get("id"));
102117
}
103118

119+
@Test
120+
void updateUserRequestWithEduIDProvisioningError() throws JsonProcessingException {
121+
User user = userRepository.findBySubIgnoreCase(GUEST_SUB).get();
122+
//Need to ensure the user is updated, therefore the remote needs to exists and provisioning is scimn
123+
String remoteScimIdentifier = UUID.randomUUID().toString();
124+
RemoteProvisionedUser remoteProvisionedUser = new RemoteProvisionedUser(user, remoteScimIdentifier, "7");
125+
remoteProvisionedUserRepository.save(remoteProvisionedUser);
126+
this.stubForManageProvisioning(List.of("1", "4", "5"));
127+
this.stubForUpdateScimUser();
128+
try {
129+
provisioningService.updateUserRequest(user);
130+
fail("Expected RemoteException");
131+
} catch (RemoteException e) {
132+
assertTrue(e.getMessage().contains("Error in provisionEduid"));
133+
assertEquals(HttpStatus.BAD_REQUEST, e.getStatusCode());
134+
assertNotNull(e.getReference());
135+
}
136+
137+
}
138+
104139
@Test
105140
void deleteUserRequest() throws JsonProcessingException {
106141
User user = userRepository.findBySubIgnoreCase(GUEST_SUB).get();

0 commit comments

Comments
 (0)