diff --git a/server/src/main/java/access/api/RoleController.java b/server/src/main/java/access/api/RoleController.java index b42d2ec7..d19f8009 100644 --- a/server/src/main/java/access/api/RoleController.java +++ b/server/src/main/java/access/api/RoleController.java @@ -14,6 +14,8 @@ import access.repository.ApplicationUsageRepository; import access.repository.RoleRepository; import access.security.RemoteUser; +import access.security.RemoteUserPermissions; +import access.security.Scope; import access.security.UserPermissions; import access.validation.URLFormatValidator; import io.swagger.v3.oas.annotations.Parameter; @@ -145,19 +147,37 @@ public ResponseEntity> search(@RequestParam(value = "query") String q public ResponseEntity newRole(@Validated @RequestBody Role role, @Parameter(hidden = true) User user, @Parameter(hidden = true) @AuthenticationPrincipal RemoteUser remoteUser) { - //TODO differentiate between RemoteUser and User - UserPermissions.assertAuthority(user, Authority.INSTITUTION_ADMIN); + if (user != null) { + //OpenID connect login with User + UserPermissions.assertAuthority(user, Authority.INSTITUTION_ADMIN); + } else { + //API user with Basic Authentication + RemoteUserPermissions.assertScopeAccess(remoteUser, Scope.sp_dashboard); + } + role.setShortName(GroupURN.sanitizeRoleShortName(role.getShortName())); role.setIdentifier(UUID.randomUUID().toString()); - LOG.debug(String.format("New role '%s' by user %s", role.getName(), user.getEduPersonPrincipalName())); - return saveOrUpdate(role, user); + Provisionable provisionable = user != null ? user : remoteUser; + + LOG.debug(String.format("New role '%s' by user %s", role.getName(), provisionable.getName())); + + return saveOrUpdate(role, user, remoteUser); } @PutMapping("") - public ResponseEntity updateRole(@Validated @RequestBody Role role, @Parameter(hidden = true) User user) { - UserPermissions.assertAuthority(user, Authority.MANAGER); + public ResponseEntity updateRole(@Validated @RequestBody Role role, + @Parameter(hidden = true) User user, + @Parameter(hidden = true) @AuthenticationPrincipal RemoteUser remoteUser) { + if (user != null) { + //OpenID connect login with User + UserPermissions.assertAuthority(user, Authority.MANAGER); + } else { + //API user with Basic Authentication + RemoteUserPermissions.assertScopeAccess(remoteUser, Scope.sp_dashboard); + } LOG.debug(String.format("Update role '%s' by user %s", role.getName(), user.getEduPersonPrincipalName())); - return saveOrUpdate(role, user); + + return saveOrUpdate(role, user, remoteUser); } @DeleteMapping("/{id}") @@ -175,7 +195,7 @@ public ResponseEntity deleteRole(@PathVariable("id") Long id, @Parameter(h return Results.deleteResult(); } - private ResponseEntity saveOrUpdate(Role role, User user) { + private ResponseEntity saveOrUpdate(Role role, User user, RemoteUser remoteUser) { if (CollectionUtils.isEmpty(role.getApplicationUsages())) { throw new InvalidInputException("applicationUsages are required"); } @@ -186,10 +206,12 @@ private ResponseEntity saveOrUpdate(Role role, User user) { }); manage.addManageMetaData(List.of(role)); - UserPermissions.assertRoleAccess(user, role, Authority.MANAGER); + if (user != null) { + UserPermissions.assertRoleAccess(user, role, Authority.MANAGER); + } boolean isNew = role.getId() == null; List previousApplicationIdentifiers = new ArrayList<>(); - Optional optionalUserRole = user.userRoleForRole(role); + Optional optionalUserRole = user != null ? user.userRoleForRole(role) : Optional.empty(); boolean immutableApplicationUsages = optionalUserRole.isPresent() && optionalUserRole.get().getAuthority().equals(Authority.MANAGER); boolean nameChanged = false; if (!isNew) { @@ -229,7 +251,10 @@ private ResponseEntity saveOrUpdate(Role role, User user) { } else { provisioningService.updateGroupRequest(previousApplicationIdentifiers, saved, nameChanged); } - AccessLogger.role(LOG, isNew ? Event.Created : Event.Updated, user, role); + + Provisionable provisionable = user != null ? user : remoteUser; + AccessLogger.role(LOG, isNew ? Event.Created : Event.Updated, provisionable, role); + return ResponseEntity.ok(saved); } diff --git a/server/src/main/java/access/logging/AccessLogger.java b/server/src/main/java/access/logging/AccessLogger.java index 223f6bd0..aa553e19 100644 --- a/server/src/main/java/access/logging/AccessLogger.java +++ b/server/src/main/java/access/logging/AccessLogger.java @@ -13,10 +13,10 @@ public class AccessLogger { private AccessLogger() { } - public static void role(Log log, Event event, User user, Role role) { + public static void role(Log log, Event event, Provisionable provisionable, Role role) { MDC.setContextMap(Map.of( "type", String.format("%s Role", event), - "userId", user.getSub(), + "userId", provisionable.getName(), "applications", applications(role), "roleId", role.getId().toString() )); diff --git a/server/src/main/java/access/security/RemoteUser.java b/server/src/main/java/access/security/RemoteUser.java index c8396b77..6d7ead41 100644 --- a/server/src/main/java/access/security/RemoteUser.java +++ b/server/src/main/java/access/security/RemoteUser.java @@ -1,5 +1,6 @@ package access.security; +import access.model.Provisionable; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; @@ -15,11 +16,11 @@ @Getter @Setter @NoArgsConstructor -public class RemoteUser implements UserDetails, CredentialsContainer { +public class RemoteUser implements UserDetails, CredentialsContainer, Provisionable { private String username; private String password; - private List scopes; + private List scopes; public RemoteUser(RemoteUser remoteUser) { this.username = remoteUser.username; @@ -31,28 +32,13 @@ public RemoteUser(RemoteUser remoteUser) { public Collection getAuthorities() { //Convention dictates upperCase Role names to be used in @PreAuthorize annotations return scopes.stream() - .map(scope -> new SimpleGrantedAuthority("ROLE_" + scope.toUpperCase())) + .map(scope -> new SimpleGrantedAuthority("ROLE_" + scope.name().toUpperCase())) .collect(Collectors.toList()); } @Override - public boolean isAccountNonExpired() { - return true; - } - - @Override - public boolean isAccountNonLocked() { - return true; - } - - @Override - public boolean isCredentialsNonExpired() { - return true; - } - - @Override - public boolean isEnabled() { - return true; + public String getName() { + return username; } @Override diff --git a/server/src/main/java/access/security/RemoteUserPermissions.java b/server/src/main/java/access/security/RemoteUserPermissions.java index 48939400..3116a1bf 100644 --- a/server/src/main/java/access/security/RemoteUserPermissions.java +++ b/server/src/main/java/access/security/RemoteUserPermissions.java @@ -8,7 +8,7 @@ public class RemoteUserPermissions { private RemoteUserPermissions() { } - public static void assertScopeAccess(RemoteUser remoteUser, String... scopes) { + public static void assertScopeAccess(RemoteUser remoteUser, Scope... scopes) { if (remoteUser == null) { throw new UserRestrictionException(); } diff --git a/server/src/main/java/access/security/Scope.java b/server/src/main/java/access/security/Scope.java new file mode 100644 index 00000000..e182609a --- /dev/null +++ b/server/src/main/java/access/security/Scope.java @@ -0,0 +1,5 @@ +package access.security; + +public enum Scope { + voot, teams, attribute_aggregation, lifecycle, profile, sp_dashboard +} diff --git a/server/src/test/java/access/api/RoleControllerTest.java b/server/src/test/java/access/api/RoleControllerTest.java index 1d18baf1..34f73c59 100644 --- a/server/src/test/java/access/api/RoleControllerTest.java +++ b/server/src/test/java/access/api/RoleControllerTest.java @@ -442,17 +442,16 @@ void createWithAPIUser() throws Exception { super.stubForManageProvisioning(List.of("1")); super.stubForCreateScimRole(); - given() + Role newRole = given() .when() .auth().preemptive().basic("sp_dashboard", "secret") .accept(ContentType.JSON) .contentType(ContentType.JSON) .body(role) .post("/api/external/v1/sp_dashboard/roles") - .then() - .statusCode(400); - //TODO -// assertNotNull(result.get("id")); + .as(new TypeRef<>() { + }); + assertNotNull(newRole.getId()); } @Test diff --git a/server/src/test/java/access/profile/ProfileControllerTest.java b/server/src/test/java/access/profile/ProfileControllerTest.java index c5bce4f4..42d6a83d 100644 --- a/server/src/test/java/access/profile/ProfileControllerTest.java +++ b/server/src/test/java/access/profile/ProfileControllerTest.java @@ -25,7 +25,7 @@ void roles() { .accept(ContentType.JSON) .contentType(ContentType.JSON) .queryParam("collabPersonId", GUEST_SUB) - .get("/api/profile") + .get("/api/external/v1/profile") .as(new TypeRef<>() { }); assertEquals(3, roles.size()); @@ -54,7 +54,7 @@ void rolesGuestRoleIncluded() { .accept(ContentType.JSON) .contentType(ContentType.JSON) .queryParam("collabPersonId", MANAGE_SUB) - .get("/api/profile") + .get("/api/external/v1/profile") .as(new TypeRef<>() { }); assertEquals(1, roles.size()); @@ -68,7 +68,7 @@ void rolesNotExistentUser() { .accept(ContentType.JSON) .contentType(ContentType.JSON) .queryParam("collabPersonId", "nope") - .get("/api/profile") + .get("/api/external/v1/profile") .as(new TypeRef<>() { }); assertEquals(0, roles.size()); diff --git a/server/src/test/java/access/teams/TeamsControllerTest.java b/server/src/test/java/access/teams/TeamsControllerTest.java index e1bf5384..1e99bb29 100644 --- a/server/src/test/java/access/teams/TeamsControllerTest.java +++ b/server/src/test/java/access/teams/TeamsControllerTest.java @@ -88,7 +88,7 @@ void migrateTeam() throws JsonProcessingException { .accept(ContentType.JSON) .contentType(ContentType.JSON) .pathParam("sub", harryDoe.getPerson().getUrn()) - .get("/api/voot/{sub}") + .get("/api/external/v1/voot/{sub}") .as(new TypeRef<>() { }); assertEquals(1, groups.size());