Skip to content

Commit

Permalink
SHIBUI-1848
Browse files Browse the repository at this point in the history
finished updates for USER-GROUP items
  • Loading branch information
chasegawa committed Jul 1, 2021
1 parent 038b9b4 commit 2d024da
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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}")
Expand All @@ -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)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
Expand All @@ -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'

Expand Down Expand Up @@ -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'() {
Expand Down Expand Up @@ -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'
Expand Down

0 comments on commit 2d024da

Please sign in to comment.