Skip to content

Commit 19e1ecf

Browse files
oharstaoharsta
committed
Fixes #474 
1 parent 1484652 commit 19e1ecf

9 files changed

Lines changed: 44 additions & 19 deletions

File tree

‎server/src/main/java/access/manage/LocalManage.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,16 +84,16 @@ public Map<String, Object> providerById(EntityType entityType, String id) {
8484
}
8585

8686
@Override
87-
public List<Map<String, Object>> provisioning(Collection<String> ids) {
88-
LOG.debug("provisioning for : " + ids);
87+
public List<Map<String, Object>> provisioning(Collection<String> applicationIdentifiers) {
88+
LOG.debug("provisioning for : " + applicationIdentifiers);
8989

90-
if (CollectionUtils.isEmpty(ids)) {
90+
if (CollectionUtils.isEmpty(applicationIdentifiers)) {
9191
return Collections.emptyList();
9292
}
9393
return providers(EntityType.PROVISIONING).stream()
9494
.filter(map -> {
9595
List<Map<String, String>> applications = (List<Map<String, String>>) map.get("applications");
96-
return applications.stream().anyMatch(m -> ids.contains(m.get("id")));
96+
return applications.stream().anyMatch(m -> applicationIdentifiers.contains(m.get("id")));
9797
})
9898
.collect(Collectors.toList());
9999
}

‎server/src/main/java/access/manage/Manage.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public interface Manage {
1919

2020
List<Map<String, Object>> providersByIdIn(EntityType entityType, List<String> identifiers);
2121

22-
List<Map<String, Object>> provisioning(Collection<String> ids);
22+
List<Map<String, Object>> provisioning(Collection<String> applicationIdentifiers);
2323

2424
List<Map<String, Object>> providersAllowedByIdP(Map<String, Object> identityProvider);
2525

‎server/src/main/java/access/manage/RemoteManage.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,14 @@ public Map<String, Object> providerById(EntityType entityType, String id) {
7575

7676

7777
@Override
78-
public List<Map<String, Object>> provisioning(Collection<String> ids) {
78+
public List<Map<String, Object>> provisioning(Collection<String> applicationIdentifiers) {
7979
LOG.debug("provisionings for identifiers");
8080

81-
if (CollectionUtils.isEmpty(ids)) {
81+
if (CollectionUtils.isEmpty(applicationIdentifiers)) {
8282
return emptyList();
8383
}
8484
String queryUrl = String.format("%s/manage/api/internal/provisioning", url);
85-
return transformProvider(restTemplate.postForObject(queryUrl, ids, List.class));
85+
return transformProvider(restTemplate.postForObject(queryUrl, applicationIdentifiers, List.class));
8686
}
8787

8888
@Override

‎server/src/main/java/access/provision/DefaultProvisioningResponse.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,5 @@ public boolean isGraphResponse() {
1111
public boolean isErrorResponse() {
1212
return false;
1313
}
14+
1415
}

‎server/src/main/java/access/provision/ProvisioningResponse.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ public interface ProvisioningResponse {
77
String remoteIdentifier();
88

99
boolean isErrorResponse();
10+
1011
}

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ private Optional<ProvisioningResponse> newRequest(Provisioning provisioning, Str
389389
boolean isUser = provisionable instanceof User;
390390
String apiType = isUser ? USER_API : GROUP_API;
391391
RequestEntity<String> requestEntity = null;
392+
boolean requiresRemoteIdentifier = false;
392393
if (hasEvaHook(provisioning) && isUser) {
393394
LOG.info(String.format("Provisioning new eva account for user %s and provisioning %s",
394395
((User) provisionable).getEmail(), provisioning.getEntityId()));
@@ -397,6 +398,7 @@ private Optional<ProvisioningResponse> newRequest(Provisioning provisioning, Str
397398
LOG.info(String.format("Provisioning new SCIM account for provisionable %s and provisioning %s",
398399
provisionable.getName(), provisioning.getEntityId()));
399400
URI uri = this.provisioningUri(provisioning, apiType, Optional.empty());
401+
requiresRemoteIdentifier = true;
400402
requestEntity = new RequestEntity<>(request, httpHeaders(provisioning), HttpMethod.POST, uri);
401403
} else if (hasGraphHook(provisioning) && isUser) {
402404
LOG.info(String.format("Provisioning new Graph user for provisionable %s and provisioning %s",
@@ -406,7 +408,15 @@ private Optional<ProvisioningResponse> newRequest(Provisioning provisioning, Str
406408
}
407409
if (requestEntity != null) {
408410
Map<String, Object> results = doExchange(requestEntity, apiType, mapParameterizedTypeReference, provisioning);
409-
return Optional.of(new DefaultProvisioningResponse(String.valueOf(results.get("id"))));
411+
String id = String.valueOf(results.get("id"));
412+
if (!StringUtils.hasText(id) && requiresRemoteIdentifier) {
413+
String errorMessage = String.format("Error in %s response %s send to entityID %s. ID is required, but empty SCIM request.",
414+
apiType,
415+
results,
416+
provisioning.getEntityId());
417+
throw new RemoteException(HttpStatus.BAD_REQUEST, errorMessage, null);
418+
}
419+
return Optional.of(new DefaultProvisioningResponse(id));
410420
}
411421
return Optional.empty();
412422

@@ -475,14 +485,13 @@ private <T, S> T doExchange(RequestEntity<S> requestEntity,
475485
provisioning.getEntityId()));
476486
return restTemplate.exchange(requestEntity, typeReference).getBody();
477487
} catch (RestClientException e) {
478-
LOG.error(String.format("Error from %s with original stack-trace", provisioning.getEntityId()), e);
479-
480488
String errorMessage = String.format("Error %s SCIM request (entityID %s) to %s with %s httpMethod and body %s",
481489
api,
482490
provisioning.getEntityId(),
483491
requestEntity.getUrl(),
484492
requestEntity.getMethod(),
485493
requestEntity.getBody());
494+
LOG.error(errorMessage, e);
486495
throw new RemoteException(HttpStatus.BAD_REQUEST, errorMessage, e);
487496
}
488497
}
@@ -531,7 +540,7 @@ private HttpHeaders httpHeaders(Provisioning provisioning) {
531540
} else if (StringUtils.hasText(provisioning.getScimBearerToken())) {
532541
String decryptedScimBearerToken = this.decryptScimBearerToken(provisioning);
533542
//For testing only, remove before prod
534-
LOG.debug(String.format("Inserting header Authorization: Bearer %s ",decryptedScimBearerToken));
543+
LOG.debug(String.format("Inserting header Authorization: Bearer %s ", decryptedScimBearerToken));
535544
headers.add(HttpHeaders.AUTHORIZATION, String.format("Bearer %s", decryptedScimBearerToken));
536545
}
537546
headers.setContentType(MediaType.APPLICATION_JSON);

‎server/src/main/java/access/provision/graph/GraphResponse.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,5 @@ public boolean isGraphResponse() {
1414
public boolean isErrorResponse() {
1515
return errorResponse;
1616
}
17+
1718
}

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -342,8 +342,8 @@ protected JWTClaimsSet getJwtClaimsSet(String clientId, String sub, String redir
342342
return builder.build();
343343
}
344344

345-
protected void stubForManageProvisioning(List<String> identifiers) throws JsonProcessingException {
346-
List<Map<String, Object>> providers = localManage.provisioning(identifiers);
345+
protected void stubForManageProvisioning(List<String> applicationIdentifiers) throws JsonProcessingException {
346+
List<Map<String, Object>> providers = localManage.provisioning(applicationIdentifiers);
347347
String body = writeValueAsString(providers);
348348
stubFor(post(urlPathMatching("/manage/api/internal/provisioning"))
349349
.willReturn(aResponse().withHeader("Content-Type", "application/json")
@@ -468,12 +468,16 @@ protected String stubForCreateScimRole() throws JsonProcessingException {
468468

469469
protected String stubForCreateScimUser() throws JsonProcessingException {
470470
String value = UUID.randomUUID().toString();
471-
String body = objectMapper.writeValueAsString(Map.of("id", value));
471+
return stubForCreateScimUser(value);
472+
}
473+
474+
protected String stubForCreateScimUser(String idValue) throws JsonProcessingException {
475+
String body = objectMapper.writeValueAsString(Map.of("id", idValue));
472476
stubFor(post(urlPathMatching("/api/scim/v2/users"))
473477
.willReturn(aResponse()
474478
.withHeader("Content-Type", "application/json")
475479
.withBody(body)));
476-
return value;
480+
return idValue;
477481
}
478482

479483
protected String stubForCreateEvaUser() throws JsonProcessingException {

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

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

33
import access.AbstractTest;
44
import access.eduid.EduIDProvision;
5+
import access.exception.RemoteException;
56
import access.model.*;
67
import access.provision.scim.OperationType;
78
import com.fasterxml.jackson.core.JsonProcessingException;
@@ -15,8 +16,7 @@
1516
import java.util.UUID;
1617

1718
import static com.github.tomakehurst.wiremock.client.WireMock.*;
18-
import static org.junit.jupiter.api.Assertions.assertEquals;
19-
import static org.junit.jupiter.api.Assertions.assertTrue;
19+
import static org.junit.jupiter.api.Assertions.*;
2020

2121
class ProvisioningServiceDefaultTest extends AbstractTest {
2222

@@ -27,14 +27,23 @@ class ProvisioningServiceDefaultTest extends AbstractTest {
2727
void newUserRequest() throws JsonProcessingException {
2828
User user = userRepository.findBySubIgnoreCase(GUEST_SUB).get();
2929
//See server/src/main/resources/manage/provisioning.json, applicationId="3"
30-
this.stubForManageProvisioning(List.of("3"));
30+
this.stubForManageProvisioning(List.of("7"));
3131
String remoteScimIdentifier = this.stubForCreateEvaUser();
3232
provisioningService.newUserRequest(user);
3333
List<RemoteProvisionedUser> remoteProvisionedUsers = remoteProvisionedUserRepository.findAll();
3434
assertEquals(1, remoteProvisionedUsers.size());
3535
assertEquals(remoteScimIdentifier, remoteProvisionedUsers.get(0).getRemoteIdentifier());
3636
}
3737

38+
@Test
39+
void newUserRequestWithInvalidRemoteResponse() throws JsonProcessingException {
40+
User user = userRepository.findBySubIgnoreCase(GUEST_SUB).get();
41+
//We will return a SCIM provisioning app
42+
this.stubForManageProvisioning(List.of("1"));
43+
this.stubForCreateScimUser("");
44+
assertThrows(RemoteException.class, () -> provisioningService.newUserRequest(user));
45+
}
46+
3847
@Test
3948
void newUserRequestWithEduIDProvisioning() throws JsonProcessingException {
4049
User user = userRepository.findBySubIgnoreCase(GUEST_SUB).get();

0 commit comments

Comments
 (0)