Skip to content

Commit

Permalink
refactored graph redirect URL to fetch the user without guessing
Browse files Browse the repository at this point in the history
  • Loading branch information
@oharsta
oharsta committed Oct 16, 2023
1 parent abab91a commit 97a93b8
Show file tree
Hide file tree
Showing 11 changed files with 131 additions and 81 deletions.
6 changes: 6 additions & 0 deletions provisioning-mock/src/main/resources/application.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,9 @@ management:
info:
git:
mode: full

# used by the git plugin
info:
build:
artifact: "@project.artifactId@"
version: "@project.version@"
13 changes: 7 additions & 6 deletions server/src/main/java/access/api/UserController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,15 @@ public ResponseEntity<Map<String, Integer>> logout(HttpServletRequest request) {
return Results.okResult();
}

@GetMapping("ms-accept-return/{sub}")
public View msAcceptReturn(@PathVariable("sub") String sub) {
Optional<RemoteProvisionedUser> remoteProvisionedUserOptional = remoteProvisionedUserRepository.findByUserSub(sub);
@GetMapping("ms-accept-return/{manageId}/{userId}")
public View msAcceptReturn(@PathVariable("manageId") String manageId, @PathVariable("userId") Long userId) {
User user = userRepository.findById(userId).orElseThrow(NotFoundException::new);
Map<String, Object> provisioningMap = manage.providerById(EntityType.PROVISIONING, manageId);
Provisioning provisioning = new Provisioning(provisioningMap);
AtomicReference<String> redirectReference = new AtomicReference<>(this.config.getWelcomeUrl());
Optional<RemoteProvisionedUser> remoteProvisionedUserOptional = remoteProvisionedUserRepository
.findByManageProvisioningIdAndUser(manageId, user);
remoteProvisionedUserOptional.ifPresent(remoteProvisionedUser -> {
User user = remoteProvisionedUser.getUser();
Map<String, Object> provisioningMap = manage.providerById(EntityType.PROVISIONING, remoteProvisionedUser.getManageProvisioningId());
Provisioning provisioning = new Provisioning(provisioningMap);
graphClient.updateUserRequest(user, provisioning, remoteProvisionedUser.getRemoteIdentifier());
String invitationHash = invitationRepository.findTopBySubInviteeOrderByCreatedAtDesc(user.getSub())
.map(Invitation::getHash).orElse("");
Expand Down
2 changes: 2 additions & 0 deletions server/src/main/java/access/model/Provisionable.java
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
package access.model;

public interface Provisionable {

String getName();
}
51 changes: 29 additions & 22 deletions server/src/main/java/access/provision/ProvisioningServiceDefault.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public Optional<GraphResponse> newUserRequest(User user) {
RemoteProvisionedUser remoteProvisionedUser = new RemoteProvisionedUser(user, response.remoteIdentifier(), provisioning.getId());
this.remoteProvisionedUserRepository.save(remoteProvisionedUser);
if (response.isGraphResponse()) {
graphResponseReference.set((GraphResponse) response);
graphResponseReference.set((GraphResponse) response);
}
});
});
Expand All @@ -117,9 +117,9 @@ public void deleteUserRequest(User user) {
.findByManageProvisioningIdAndUser(provisioning.getId(), user);
if (provisionedUserOptional.isPresent()) {
RemoteProvisionedUser remoteProvisionedUser = provisionedUserOptional.get();
String remoteScimIdentifier = remoteProvisionedUser.getRemoteIdentifier();
String userRequest = prettyJson(new UserRequest(user, remoteScimIdentifier));
this.deleteRequest(provisioning, userRequest, user, remoteScimIdentifier);
String remoteIdentifier = remoteProvisionedUser.getRemoteIdentifier();
String userRequest = prettyJson(new UserRequest(user, remoteIdentifier));
this.deleteRequest(provisioning, userRequest, user, remoteIdentifier);
this.remoteProvisionedUserRepository.delete(remoteProvisionedUser);
}
});
Expand Down Expand Up @@ -220,17 +220,16 @@ public void updateGroupRequest(UserRole userRole, OperationType operationType) {
public void deleteGroupRequest(Role role) {
List<Provisioning> provisionings = getProvisionings(role);
//Delete the group to all provisionings in Manage where the group is known
provisionings.forEach(provisioning -> {
this.remoteProvisionedGroupRepository
.findByManageProvisioningIdAndRole(provisioning.getId(), role)
.ifPresent(remoteProvisionedGroup -> {
String remoteScimIdentifier = remoteProvisionedGroup.getRemoteIdentifier();
String externalId = GroupURN.urnFromRole(groupUrnPrefix, role);
String groupRequest = prettyJson(new GroupRequest(externalId, remoteScimIdentifier, role.getName(), Collections.emptyList()));
this.deleteRequest(provisioning, groupRequest, role, remoteScimIdentifier);
this.remoteProvisionedGroupRepository.delete(remoteProvisionedGroup);
});
});
provisionings.forEach(provisioning ->
this.remoteProvisionedGroupRepository
.findByManageProvisioningIdAndRole(provisioning.getId(), role)
.ifPresent(remoteProvisionedGroup -> {
String remoteIdentifier = remoteProvisionedGroup.getRemoteIdentifier();
String externalId = GroupURN.urnFromRole(groupUrnPrefix, role);
String groupRequest = prettyJson(new GroupRequest(externalId, remoteIdentifier, role.getName(), Collections.emptyList()));
this.deleteRequest(provisioning, groupRequest, role, remoteIdentifier);
this.remoteProvisionedGroupRepository.delete(remoteProvisionedGroup);
}));
}

private String constructGroupRequest(Role role, String remoteGroupScimIdentifier, List<String> remoteUserScimIdentifiers) {
Expand Down Expand Up @@ -258,11 +257,17 @@ private Optional<ProvisioningResponse> newRequest(Provisioning provisioning, Str
String apiType = isUser ? USER_API : GROUP_API;
RequestEntity<String> requestEntity = null;
if (hasEvaHook(provisioning) && isUser) {
LOG.info(String.format("Provisioning new eva account for user %s and provisioning %s",
((User) provisionable).getEmail(), provisioning.getEntityId()));
requestEntity = this.evaClient.newUserRequest(provisioning, (User) provisionable);
} else if (hasScimHook(provisioning)) {
LOG.info(String.format("Provisioning new SCIM account for provisionable %s and provisioning %s",
provisionable.getName(), provisioning.getEntityId()));
URI uri = this.provisioningUri(provisioning, apiType, Optional.empty());
requestEntity = new RequestEntity<>(request, httpHeaders(provisioning), HttpMethod.POST, uri);
} else if (hasGraphHook(provisioning) && isUser) {
LOG.info(String.format("Provisioning new Graph user for provisionable %s and provisioning %s",
((User) provisionable).getEmail(), provisioning.getEntityId()));
GraphResponse graphResponse = this.graphClient.newUserRequest(provisioning, (User) provisionable);
return Optional.of(graphResponse);
}
Expand All @@ -278,10 +283,10 @@ private Optional<ProvisioningResponse> newRequest(Provisioning provisioning, Str
private void updateRequest(Provisioning provisioning,
String request,
String apiType,
String remoteScimIdentifier,
String remoteIdentifier,
HttpMethod httpMethod) {
if (hasScimHook(provisioning)) {
URI uri = this.provisioningUri(provisioning, apiType, Optional.ofNullable(remoteScimIdentifier));
URI uri = this.provisioningUri(provisioning, apiType, Optional.ofNullable(remoteIdentifier));
RequestEntity<String> requestEntity = new RequestEntity<>(request, httpHeaders(provisioning), httpMethod, uri);
doExchange(requestEntity, apiType, mapParameterizedTypeReference, provisioning);
}
Expand All @@ -301,18 +306,20 @@ private List<Provisioning> getProvisionings(Role role) {
private void deleteRequest(Provisioning provisioning,
String request,
Provisionable provisionable,
String remoteScimIdentifier) {
String remoteIdentifier) {
boolean isUser = provisionable instanceof User;
String apiType = isUser ? USER_API : GROUP_API;
RequestEntity<String> requestEntity = null;
if (hasEvaHook(provisioning) && isUser) {
String url = provisioning.getEvaUrl() + "/api/v1/guest/disable/" + remoteScimIdentifier;
String url = provisioning.getEvaUrl() + "/api/v1/guest/disable/" + remoteIdentifier;
requestEntity = new RequestEntity(httpHeaders(provisioning), HttpMethod.POST, URI.create(url));
} else if (hasScimHook(provisioning)) {
URI uri = this.provisioningUri(provisioning, apiType, Optional.ofNullable(remoteScimIdentifier));
URI uri = this.provisioningUri(provisioning, apiType, Optional.ofNullable(remoteIdentifier));
HttpHeaders headers = new HttpHeaders();
headers.setBasicAuth(provisioning.getScimUser(), provisioning.getScimPassword());
requestEntity = new RequestEntity<>(request, headers, HttpMethod.DELETE, uri);
} else if (hasGraphHook(provisioning) && isUser) {
this.graphClient.deleteUser((User) provisionable, provisioning, remoteIdentifier);
}
if (requestEntity != null) {
doExchange(requestEntity, apiType, stringParameterizedTypeReference, provisioning);
Expand Down Expand Up @@ -355,8 +362,8 @@ private boolean hasGraphHook(Provisioning provisioning) {
return provisioning.getProvisioningType().equals(ProvisioningType.graph);
}

private URI provisioningUri(Provisioning provisioning, String objectType, Optional<String> remoteScimIdentifier) {
String postFix = remoteScimIdentifier.map(identifier -> "/" + identifier).orElse("");
private URI provisioningUri(Provisioning provisioning, String objectType, Optional<String> remoteIdentifier) {
String postFix = remoteIdentifier.map(identifier -> "/" + identifier).orElse("");
return URI.create(String.format("%s/%s%s",
provisioning.getScimUrl(),
objectType,
Expand Down
25 changes: 22 additions & 3 deletions server/src/main/java/access/provision/graph/GraphClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.microsoft.graph.requests.GraphServiceClient;
import com.microsoft.graph.requests.InvitationCollectionRequest;
import com.microsoft.graph.requests.UserRequest;
import com.microsoft.graph.requests.UserRequestBuilder;
import okhttp3.Request;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
Expand Down Expand Up @@ -44,7 +45,8 @@ public GraphResponse newUserRequest(Provisioning provisioning, User user) {
com.microsoft.graph.models.Invitation invitation = new com.microsoft.graph.models.Invitation();
invitation.invitedUserEmailAddress = eduidIdpSchacHomeOrganization.equalsIgnoreCase(user.getSchacHomeOrganization()) ? user.getEduPersonPrincipalName() : user.getEmail();
invitation.invitedUserDisplayName = user.getName();
invitation.inviteRedirectUrl = String.format("%s/api/v1/invitations/ms-accept-return/%s", serverUrl, user.getSub());
invitation.inviteRedirectUrl = String.format("%s/api/v1/invitations/ms-accept-return/%s/%s",
serverUrl, provisioning.getId(),user.getId());
invitation.sendInvitationMessage = false;
invitation.invitedUserType = "Guest";

Expand All @@ -53,10 +55,8 @@ public GraphResponse newUserRequest(Provisioning provisioning, User user) {
provisioning.getGraphClientId(),
user.getEduPersonPrincipalName()));
try {

com.microsoft.graph.models.Invitation newInvitation = buildRequest.post(invitation);
return new GraphResponse(newInvitation.invitedUser.id, newInvitation.inviteRedeemUrl);

} catch (ClientException e) {
String errorMessage = String.format("Error Graph request (entityID %s) to %s for user %s",
provisioning.getEntityId(),
Expand Down Expand Up @@ -86,6 +86,25 @@ public void updateUserRequest(User user, Provisioning provisioning, String remot
}
}

public void deleteUser(User user, Provisioning provisioning, String remoteUserIdentifier) {
GraphServiceClient<Request> graphServiceClient = getRequestGraphServiceClient(provisioning);
UserRequest userRequest = graphServiceClient.users(remoteUserIdentifier).buildRequest();

String graphUrl = provisioning.getGraphUrl();
replaceGraphUrl(graphUrl, userRequest);

try {
userRequest.delete();
} catch (ClientException e) {
String errorMessage = String.format("Error Graph delete (entityID %s) to %s for user %s",
provisioning.getEntityId(),
graphUrl,
user.getEmail());
throw new RemoteException(HttpStatus.BAD_REQUEST, errorMessage, e);
}

}

private GraphServiceClient<Request> getRequestGraphServiceClient(Provisioning provisioning) {
ClientSecretCredential credential = new ClientSecretCredentialBuilder()
.clientId(provisioning.getGraphClientId())
Expand Down
2 changes: 0 additions & 2 deletions server/src/main/java/access/repository/RemoteProvisionedUserRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,4 @@ public interface RemoteProvisionedUserRepository extends JpaRepository<RemotePro

Optional<RemoteProvisionedUser> findByManageProvisioningIdAndUser(String manageId, User user);

Optional<RemoteProvisionedUser> findByUserSub(String sub);

}
4 changes: 4 additions & 0 deletions server/src/main/resources/db/mysql/migration/V8_0__remote_provisioned_created_at.sql
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
ALTER TABLE `remote_provisioned_groups`
add `created_at` datetime DEFAULT CURRENT_TIMESTAMP;
ALTER TABLE `remote_provisioned_users`
add `created_at` datetime DEFAULT CURRENT_TIMESTAMP;
7 changes: 6 additions & 1 deletion server/src/test/java/access/AbstractTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,17 @@ protected void stubForDeleteScimUser() {
.withStatus(201)));
}

protected void stubForDeleteEvaUser() throws JsonProcessingException {
protected void stubForDeleteEvaUser() {
stubFor(post(urlPathMatching(String.format("/eva/api/v1/guest/disable/(.*)")))
.willReturn(aResponse()
.withStatus(201)));
}

protected void stubForDeleteGraphUser() {
stubFor(delete(urlPathMatching(String.format("/graph/users")))
.willReturn(aResponse()
.withStatus(201)));
}

protected void stubForDeleteScimRole() {
stubFor(delete(urlPathMatching("/api/scim/v2/groups/(.*)"))
Expand Down
13 changes: 9 additions & 4 deletions server/src/test/java/access/api/UserControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import access.AbstractTest;
import access.AccessCookieFilter;
import access.exception.NotFoundException;
import access.manage.EntityType;
import access.model.Authority;
import access.model.User;
Expand All @@ -18,12 +19,14 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Stream;

import static access.Seed.*;
import static com.github.tomakehurst.wiremock.client.WireMock.*;
import static io.restassured.RestAssured.given;
import static org.junit.jupiter.api.Assertions.*;

@SuppressWarnings("unchecked")
class UserControllerTest extends AbstractTest {

@Test
Expand Down Expand Up @@ -116,7 +119,7 @@ void institutionAdminProvision() throws Exception {
assertTrue(user.isInstitutionAdmin());
assertEquals(ORGANISATION_GUID, user.getOrganizationGUID());
assertEquals(3, user.getApplications().size());
user.getApplications().stream().forEach(application -> assertEquals(ORGANISATION_GUID,
user.getApplications().forEach(application -> assertEquals(ORGANISATION_GUID,
((Map) ((Map) application.get("data")).get("metaDataFields")).get("coin:institution_guid")));

Map res = given()
Expand Down Expand Up @@ -145,7 +148,7 @@ void meWithRoles() throws Exception {
.contentType(ContentType.JSON)
.get(accessCookieFilter.apiURL())
.as(User.class);
List<String> roleNames = List.of("Mail", "Calendar").stream().sorted().toList();
List<String> roleNames = Stream.of("Mail", "Calendar").sorted().toList();

assertEquals(roleNames, user.getUserRoles().stream().map(userRole -> userRole.getRole().getName()).sorted().toList());
List<Object> applicationIdentifiers = user.getUserRoles().stream()
Expand Down Expand Up @@ -344,11 +347,13 @@ void msAcceptReturn() throws Exception {
super.stubForUpdateGraphUser(GUEST_SUB);
super.stubForManageProviderById(EntityType.PROVISIONING, "9");

User user = userRepository.findBySubIgnoreCase(GUEST_SUB).orElseThrow(NotFoundException::new);
given().redirects()
.follow(false)
.when()
.pathParams("sub", GUEST_SUB)
.get("/api/v1/users/ms-accept-return/{sub}")
.pathParams("manageId", "9")
.pathParams("userId", user.getId())
.get("/api/v1/users/ms-accept-return/{manageId}/{userId}")
.then()
.statusCode(302)
.header("Location", "http://localhost:4000/proceed?hash=GUEST&isRedirect=true");
Expand Down
16 changes: 16 additions & 0 deletions server/src/test/java/access/provision/ProvisioningServiceDefaultTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,22 @@ void deleteUserRequest() throws JsonProcessingException {
assertEquals(0, remoteProvisionedUsers.size());
}

@Test
void deleteGraphUserRequest() throws JsonProcessingException {
User user = userRepository.findBySubIgnoreCase(GUEST_SUB).get();

//Mock that this user was provisioned earlier
RemoteProvisionedUser remoteProvisionedUser = new RemoteProvisionedUser(user, UUID.randomUUID().toString(), "9");
remoteProvisionedUserRepository.save(remoteProvisionedUser);

this.stubForManageProvisioning(List.of("2", "6"));
this.stubForDeleteGraphUser();

provisioningService.deleteUserRequest(user);
List<RemoteProvisionedUser> remoteProvisionedUsers = remoteProvisionedUserRepository.findAll();
assertEquals(0, remoteProvisionedUsers.size());
}

@Test
void updateGroupRequest() {
//We only provision GUEST users
Expand Down
Loading

0 comments on commit 97a93b8

Please sign in to comment.