diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java index da6d5edae..4062ec080 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java @@ -52,7 +52,7 @@ public ResponseEntity update(@RequestBody Group group) { if (g == null) { HttpHeaders headers = new HttpHeaders(); - headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/admin/groups").build().toUri()); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/admin/groups/{resourceId}").build().toUri()); return ResponseEntity.status(HttpStatus.NOT_FOUND).headers(headers) .body(new ErrorResponse(String.valueOf(HttpStatus.NOT_FOUND.value()), @@ -99,13 +99,13 @@ public ResponseEntity delete(@PathVariable String resourceId) { .body(new ErrorResponse(String.valueOf(HttpStatus.NOT_FOUND.value()), String.format("Unable to find group with resource id: [%s]", resourceId))); } - if (!g.getUsers().isEmpty()) { + if (!g.getUsers().isEmpty() || !g.getEntityDescriptors().isEmpty()) { HttpHeaders headers = new HttpHeaders(); - headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/admin/groups").build().toUri()); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/admin/groups/{resourceId}").build().toUri()); return ResponseEntity.status(HttpStatus.CONFLICT).headers(headers) .body(new ErrorResponse(String.valueOf(HttpStatus.CONFLICT.value()), String.format( - "Unable to delete group with resource id: [%s] - remove all users from group first", + "Unable to delete group with resource id: [%s] - remove all users and entities from group first", resourceId))); } groupService.deleteDefinition(g); diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersController.java index db8924242..3b75b865c 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersController.java @@ -1,13 +1,16 @@ package edu.internet2.tier.shibboleth.admin.ui.security.controller; import edu.internet2.tier.shibboleth.admin.ui.controller.ErrorResponse; +import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; import edu.internet2.tier.shibboleth.admin.ui.security.model.User; +import edu.internet2.tier.shibboleth.admin.ui.security.repository.GroupsRepository; import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository; import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository; import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService; +import groovy.util.logging.Slf4j; + import org.apache.commons.lang.StringUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; @@ -31,25 +34,35 @@ /** * Implementation of the REST resource endpoints exposing system users. - * - * @author Dmitriy Kopylenko */ @RestController @RequestMapping("/api/admin/users") +@Slf4j public class UsersController { - - private static final Logger logger = LoggerFactory.getLogger(UsersController.class); - + @Autowired + private GroupsRepository groupRepo; private UserRepository userRepository; - private RoleRepository roleRepository; private UserService userService; public UsersController(UserRepository userRepository, RoleRepository roleRepository, UserService userService) { this.userRepository = userRepository; - this.roleRepository = roleRepository; this.userService = userService; } + @PreAuthorize("hasRole('ADMIN')") + @Transactional + @DeleteMapping("/{username}") + public ResponseEntity deleteOne(@PathVariable String username) { + User user = findUserOrThrowHttp404(username); + userRepository.delete(user); + return ResponseEntity.noContent().build(); + } + + private User findUserOrThrowHttp404(String username) { + return userRepository.findByUsername(username) + .orElseThrow(() -> new HttpClientErrorException(NOT_FOUND, String.format("User with username [%s] not found", username))); + } + @PreAuthorize("hasRole('ADMIN')") @Transactional(readOnly = true) @GetMapping @@ -78,15 +91,6 @@ public ResponseEntity getUsersWithRole(@PathVariable String rolename) { return ResponseEntity.ok(userRepository.findByRoles_Name(rolename)); } - @PreAuthorize("hasRole('ADMIN')") - @Transactional - @DeleteMapping("/{username}") - public ResponseEntity deleteOne(@PathVariable String username) { - User user = findUserOrThrowHttp404(username); - userRepository.delete(user); - return ResponseEntity.noContent().build(); - } - @PreAuthorize("hasRole('ADMIN')") @Transactional @PostMapping @@ -101,10 +105,22 @@ ResponseEntity saveOne(@RequestBody User user) { //TODO: modify this such that additional encoders can be used user.setPassword(BCrypt.hashpw(user.getPassword(), BCrypt.gensalt())); userService.updateUserRole(user); + findAndSetGroup(user); + User savedUser = userRepository.save(user); return ResponseEntity.ok(savedUser); } + private User findAndSetGroup(User user) { + // Ensure we have the full group detail from the db + if (user.getGroupId() != null || user.getGroup() != null) { + String resourceId = user.getGroupId() == null ? user.getGroup().getResourceId() : user.getGroupId(); + Group groupFromDb = groupRepo.findByResourceId(resourceId); + user.setGroup(groupFromDb); + } + return user; + } + @PreAuthorize("hasRole('ADMIN')") @Transactional @PatchMapping("/{username}") @@ -126,12 +142,8 @@ ResponseEntity updateOne(@PathVariable(value = "username") String username, @ persistedUser.setRole(user.getRole()); userService.updateUserRole(persistedUser); } + persistedUser.setGroup(findAndSetGroup(user).getGroup()); User savedUser = userRepository.save(persistedUser); return ResponseEntity.ok(savedUser); } - - private User findUserOrThrowHttp404(String username) { - return userRepository.findByUsername(username) - .orElseThrow(() -> new HttpClientErrorException(NOT_FOUND, String.format("User with username [%s] not found", username))); - } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java index 9d58422e7..8acb53a45 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java @@ -47,6 +47,10 @@ public class User extends AbstractAuditable { @EqualsAndHashCode.Exclude private Group group; + @Transient + @EqualsAndHashCode.Exclude + private String groupId; // simplifies the ui/api + private String lastName; @JsonProperty(access = JsonProperty.Access.WRITE_ONLY) diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy index 3b514c065..a1199bf21 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy @@ -24,6 +24,8 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilder import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status +import javax.persistence.EntityManager + @SpringBootTest @AutoConfigureMockMvc @ActiveProfiles(["no-auth", "dev"]) @@ -32,6 +34,9 @@ class GroupsControllerIntegrationTests extends Specification { @Autowired private MockMvc mockMvc + @Autowired + EntityManager entityManager + static RESOURCE_URI = '/api/admin/groups' static USERS_RESOURCE_URI = '/api/admin/users' @@ -147,35 +152,7 @@ class GroupsControllerIntegrationTests extends Specification { then: 'The group not found' nonexistentGroupRequest.andExpect(status().isNotFound()) } - -// @Rollback -// @WithMockUser(value = "admin", roles = ["ADMIN"]) -// def 'DELETE ONE existing group'() { -// when: 'GET request for a single specific group in a system that has groups' -// def result = mockMvc.perform(get("$RESOURCE_URI/BBB")) -// -// then: 'GET request for a single specific group completed with HTTP 200' -// result.andExpect(status().isOk()) -// -// when: 'DELETE request is made' -// result = mockMvc.perform(delete("$RESOURCE_URI/BBB")) -// -// then: 'DELETE was successful' -// result.andExpect(status().isNoContent()) -// -// when: 'GET request for a single specific group just deleted' -// result = mockMvc.perform(get("$RESOURCE_URI/BBB")) -// -// then: 'The group not found' -// result.andExpect(status().isNotFound()) -// -// when: 'DELETE request for a single specific group that does not exist' -// result = mockMvc.perform(delete("$RESOURCE_URI/CCCC")) -// -// then: 'The group not found' -// result.andExpect(status().isNotFound()) -// } - + @Rollback @WithMockUser(value = "admin", roles = ["ADMIN"]) def 'DELETE performs correctly when group attached to a user'() { @@ -218,6 +195,8 @@ class GroupsControllerIntegrationTests extends Specification { when: 'DELETE request is made' + entityManager.flush() + entityManager.clear() result = mockMvc.perform(delete("$RESOURCE_URI/$group.resourceId")) then: 'DELETE was not successful'