Merge branch 'feature/SHIBUI-1031' of bitbucket.org:unicon/shib-idp-u…

…i into feature/SHIBUI-1031
TIER · Jan 7, 2019 · 325d382 · 325d382
2 parents 14bed36 + fbe3075
commit 325d382
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 66 deletions.
diff --git a/...in/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java b/...in/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/...main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersController.java b/...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<Role> getPersistedRoles(String username, Set<Role> userRolesToBeUpdated) {
-        Set<Role> newRoles = new HashSet<>();
-        for (Role role : userRolesToBeUpdated) {
-            Optional<Role> 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
@@ -1,19 +1,22 @@
 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;
 import javax.persistence.Entity;
 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<Role> roles = new HashSet<>();
+
+    public String getRole() {
+        Set<Role> 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/...rc/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserRoleService.java b/...rc/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<Role> userRole = roleRepository.findByName(user.getRole());
+            if (userRole.isPresent()) {
+                Set<Role> 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/...rnet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy b/...rnet2/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,72 +79,63 @@ 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<Role>
-            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<Role>
-            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
     }
 
     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<Role>
-            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
     }
 
     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<Role>
-            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)