diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java index fb60112f2..8af5129d6 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java @@ -7,6 +7,7 @@ import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolversPositionOrderContainerRepository; import edu.internet2.tier.shibboleth.admin.ui.scheduled.EntityDescriptorFilesScheduledTasks; import edu.internet2.tier.shibboleth.admin.ui.scheduled.MetadataProvidersScheduledTasks; +import edu.internet2.tier.shibboleth.admin.ui.security.service.UserRoleService; import edu.internet2.tier.shibboleth.admin.ui.service.DefaultMetadataResolversPositionOrderContainerService; import edu.internet2.tier.shibboleth.admin.ui.service.DirectoryService; import edu.internet2.tier.shibboleth.admin.ui.service.DirectoryServiceImpl; @@ -194,4 +195,9 @@ public Module stringTrimModule() { public ModelRepresentationConversions modelRepresentationConversions() { return new ModelRepresentationConversions(customPropertiesConfiguration()); } + + @Bean + public UserRoleService userRoleService() { + return new UserRoleService(); + } } 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 bd479b1c8..4f3e67c11 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,10 +1,10 @@ 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.Role; import edu.internet2.tier.shibboleth.admin.ui.security.model.User; 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.UserRoleService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.http.HttpStatus; @@ -20,10 +20,8 @@ import org.springframework.web.bind.annotation.RestController; import org.springframework.web.client.HttpClientErrorException; -import java.util.HashSet; import java.util.List; import java.util.Optional; -import java.util.Set; import static org.springframework.http.HttpStatus.NOT_FOUND; @@ -40,10 +38,12 @@ public class UsersController { private UserRepository userRepository; private RoleRepository roleRepository; + private UserRoleService userRoleService; - public UsersController(UserRepository userRepository, RoleRepository roleRepository) { + public UsersController(UserRepository userRepository, RoleRepository roleRepository, UserRoleService userRoleService) { this.userRepository = userRepository; this.roleRepository = roleRepository; + this.userRoleService = userRoleService; } @Transactional(readOnly = true) @@ -76,7 +76,7 @@ ResponseEntity saveOne(@RequestBody User user) { .body(new ErrorResponse(String.valueOf(HttpStatus.CONFLICT.value()), String.format("A user with username [%s] already exists within the system.", user.getUsername()))); } - user.setRoles(getPersistedRoles(user.getUsername(), user.getRoles())); + userRoleService.updateUserRole(user); //TODO: encrypt password? Or is it sent to us encrypted? User savedUser = userRepository.save(user); return ResponseEntity.ok(savedUser); @@ -90,24 +90,11 @@ ResponseEntity updateOne(@PathVariable(value = "username") String username, @ persistedUser.setFirstName(user.getFirstName()); persistedUser.setLastName(user.getLastName()); persistedUser.setEmailAddress(user.getEmailAddress()); - persistedUser.setRoles(getPersistedRoles(user.getUsername(), user.getRoles())); + userRoleService.updateUserRole(persistedUser); User savedUser = userRepository.save(persistedUser); return ResponseEntity.ok(savedUser); } - private Set getPersistedRoles(String username, Set userRolesToBeUpdated) { - Set newRoles = new HashSet<>(); - for (Role role : userRolesToBeUpdated) { - Optional persistedRole = roleRepository.findByName(role.getName()); - if (!persistedRole.isPresent()) { - logger.warn("Role [%s] is not present in the system. Not setting role for user [%s].", role.getName(), username); - continue; - } - newRoles.add(persistedRole.get()); - } - return newRoles; - } - 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 992b55413..bf1f9cb4f 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 @@ -1,12 +1,14 @@ package edu.internet2.tier.shibboleth.admin.ui.security.model; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonProperty; import edu.internet2.tier.shibboleth.admin.ui.domain.AbstractAuditable; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; import lombok.ToString; +import org.apache.commons.lang.StringUtils; import javax.persistence.CascadeType; import javax.persistence.Column; @@ -14,6 +16,7 @@ import javax.persistence.JoinColumn; import javax.persistence.JoinTable; import javax.persistence.ManyToMany; +import javax.persistence.Transient; import java.util.HashSet; import java.util.Set; @@ -33,8 +36,7 @@ public class User extends AbstractAuditable { @Column(nullable = false, unique = true) private String username; - //TODO: Need to figure out the right way to protect this property - //@JsonProperty(access = JsonProperty.Access.WRITE_ONLY) + @JsonProperty(access = JsonProperty.Access.WRITE_ONLY) @Column(nullable = false) private String password; @@ -44,9 +46,24 @@ public class User extends AbstractAuditable { private String emailAddress; + @Transient + private String role; + //Ignore properties annotation here is to prevent stack overflow recursive error during JSON serialization - @JsonIgnoreProperties("users") + @JsonIgnore +// @JsonIgnoreProperties("users") @ManyToMany(cascade = CascadeType.ALL) @JoinTable(name = "user_role", joinColumns = @JoinColumn(name = "user_id"), inverseJoinColumns = @JoinColumn(name = "role_id")) private Set roles = new HashSet<>(); + + public String getRole() { + Set roles = this.getRoles(); + if (roles.size() != 1) { + if (StringUtils.isNotBlank(this.role)) { + return this.role; + } + throw new RuntimeException(String.format("User with username [%s] does not have exactly one role!", this.getUsername())); + } + return roles.iterator().next().getName(); + } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserRoleService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserRoleService.java new file mode 100644 index 000000000..5f9312ebe --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserRoleService.java @@ -0,0 +1,43 @@ +package edu.internet2.tier.shibboleth.admin.ui.security.service; + +import edu.internet2.tier.shibboleth.admin.ui.security.model.Role; +import edu.internet2.tier.shibboleth.admin.ui.security.model.User; +import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository; +import org.apache.commons.lang.StringUtils; +import org.springframework.beans.factory.annotation.Autowired; + +import java.util.HashSet; +import java.util.Optional; +import java.util.Set; + +/** + * @author Bill Smith (wsmith@unicon.net) + */ +public class UserRoleService { + + @Autowired + RoleRepository roleRepository; + + /** + * Given a user with a defined User.role, update the User.roles collection with that role. + * + * This currently exists because users should only ever have one role in the system at this time. However, user + * roles are persisted as a set of roles (for future-proofing). Once we start allowing a user to have multiple roles, + * this method and User.role can go away. + * @param user + */ + public void updateUserRole(User user) { + if (StringUtils.isNotBlank(user.getRole())) { + Optional userRole = roleRepository.findByName(user.getRole()); + if (userRole.isPresent()) { + Set userRoles = new HashSet<>(); + userRoles.add(userRole.get()); + user.setRoles(userRoles); + } else { + throw new RuntimeException(String.format("User with username [%s] is defined with role [%s] which does not exist in the system!", user.getUsername(), user.getRole())); + } + } else { + throw new RuntimeException(String.format("User with username [%s] has no role defined and therefor cannot be updated!", user.getUsername())); + } + } +} diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy index 9faec1da1..6aae4231d 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy @@ -2,8 +2,7 @@ package edu.internet2.tier.shibboleth.admin.ui.security.controller import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.SerializationFeature -import edu.internet2.tier.shibboleth.admin.ui.security.model.Role -import edu.internet2.tier.shibboleth.admin.ui.security.model.User +import groovy.json.JsonOutput import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.context.SpringBootTest import org.springframework.boot.test.web.client.TestRestTemplate @@ -39,7 +38,7 @@ class UsersControllerIntegrationTests extends Specification { then: 'Request completed with HTTP 200 and returned a list of users' result.statusCodeValue == 200 result.body[0].username == 'admin' - result.body[0].roles[0].name == 'ROLE_ADMIN' + result.body[0].role == 'ROLE_ADMIN' } def 'GET ONE existing user'() { @@ -49,7 +48,7 @@ class UsersControllerIntegrationTests extends Specification { then: 'Request completed with HTTP 200 and returned one user' result.statusCodeValue == 200 result.body.username == 'admin' - result.body.roles[0].name == 'ROLE_ADMIN' + result.body.role == 'ROLE_ADMIN' } def 'GET ONE NON-existing user'() { @@ -80,37 +79,32 @@ class UsersControllerIntegrationTests extends Specification { def 'POST new user persists properly'() { given: - def newUser = new User().with { - it.firstName = 'Foo' - it.lastName = 'Bar' - it.username = 'FooBar' - it.password = 'somepass' - it.roles = [new Role().with {it.name = 'ROLE_USER'}] as Set - it - } + def newUser = [firstName: 'Foo', + lastName: 'Bar', + username: 'FooBar', + password: 'somepass', + emailAddress: 'foo@institution.edu', + role: 'ROLE_USER'] when: - def result = this.restTemplate.postForEntity("$RESOURCE_URI", createRequestHttpEntityFor { mapper.writeValueAsString(newUser) }, Map) + def result = this.restTemplate.postForEntity("$RESOURCE_URI", createRequestHttpEntityFor { JsonOutput.toJson(newUser) }, Map) then: result.statusCodeValue == 200 - //TODO: Compare body? Or do that in a service-level unit test? } def 'POST new duplicate username returns 409'() { given: - def newUser = new User().with { - it.firstName = 'Foo' - it.lastName = 'Bar' - it.username = 'DuplicateUser' - it.password = 'somepass' - it.roles = [new Role().with {it.name = 'ROLE_USER'}] as Set - it - } + def newUser = [firstName: 'Foo', + lastName: 'Bar', + username: 'DuplicateUser', + password: 'somepass', + emailAddress: 'foo@institution.edu', + role: 'ROLE_USER'] when: - this.restTemplate.postForEntity("$RESOURCE_URI", createRequestHttpEntityFor { mapper.writeValueAsString(newUser) }, Map) - def result = this.restTemplate.postForEntity("$RESOURCE_URI", createRequestHttpEntityFor { mapper.writeValueAsString(newUser) }, Map) + this.restTemplate.postForEntity("$RESOURCE_URI", createRequestHttpEntityFor { JsonOutput.toJson(newUser) }, Map) + def result = this.restTemplate.postForEntity("$RESOURCE_URI", createRequestHttpEntityFor { JsonOutput.toJson(newUser) }, Map) then: result.statusCodeValue == 409 @@ -118,19 +112,17 @@ class UsersControllerIntegrationTests extends Specification { def 'PUT updates user properly'() { given: - def newUser = new User().with { - it.firstName = 'Foo' - it.lastName = 'Bar' - it.username = 'FooBar' - it.password = 'somepass' - it.roles = [new Role().with {it.name = 'ROLE_USER'}] as Set - it - } + def newUser = [firstName: 'Foo', + lastName: 'Bar', + username: 'FooBar', + password: 'somepass', + emailAddress: 'foo@institution.edu', + role: 'ROLE_USER'] when: - this.restTemplate.postForEntity("$RESOURCE_URI", createRequestHttpEntityFor { mapper.writeValueAsString(newUser) }, Map) - newUser.setFirstName('Bob') - def result = this.restTemplate.exchange("$RESOURCE_URI/$newUser.username", org.springframework.http.HttpMethod.PUT, createRequestHttpEntityFor { mapper.writeValueAsString(newUser) }, Map) + this.restTemplate.postForEntity("$RESOURCE_URI", createRequestHttpEntityFor { JsonOutput.toJson(newUser) }, Map) + newUser['firstName'] = 'Bob' + def result = this.restTemplate.exchange("$RESOURCE_URI/$newUser.username", org.springframework.http.HttpMethod.PUT, createRequestHttpEntityFor { JsonOutput.toJson(newUser) }, Map) then: result.statusCodeValue == 200 @@ -138,14 +130,12 @@ class UsersControllerIntegrationTests extends Specification { def 'PUT detects unknown username'() { given: - def newUser = new User().with { - it.firstName = 'Foo' - it.lastName = 'Bar' - it.username = 'UnknownUsername' - it.password = 'somepass' - it.roles = [new Role().with {it.name = 'ROLE_USER'}] as Set - it - } + def newUser = [firstName: 'Foo', + lastName: 'Bar', + username: 'UnknownUser', + password: 'somepass', + emailAddress: 'foo@institution.edu', + role: 'ROLE_USER'] when: def result = this.restTemplate.exchange("$RESOURCE_URI/$newUser.username", org.springframework.http.HttpMethod.PUT, createRequestHttpEntityFor { mapper.writeValueAsString(newUser) }, Map)