[SHIBUI-1031]

Added tests for POST/PUT.

backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersController.java

@@ -1,25 +1,29 @@
 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 org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
-import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
-import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.transaction.annotation.Transactional;
 import org.springframework.web.bind.annotation.DeleteMapping;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.PutMapping;
+import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestMapping;
-import org.springframework.web.bind.annotation.RequestParam;
 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;
 
@@ -29,69 +33,81 @@
  * @author Dmitriy Kopylenko
  */
 @RestController
-@RequestMapping("/api/admin")
+@RequestMapping("/api/admin/users")
 public class UsersController {
 
+    private static final Logger logger = LoggerFactory.getLogger(UsersController.class);
+
     private UserRepository userRepository;
+    private RoleRepository roleRepository;
 
-    public UsersController(UserRepository userRepository) {
+    public UsersController(UserRepository userRepository, RoleRepository roleRepository) {
         this.userRepository = userRepository;
+        this.roleRepository = roleRepository;
     }
 
     @Transactional(readOnly = true)
-    @GetMapping("/users")
+    @GetMapping
     public List<User> getAll() {
         return userRepository.findAll();
     }
 
     @Transactional(readOnly = true)
-    @GetMapping("/users/{username}")
+    @GetMapping("/{username}")
     public ResponseEntity<?> getOne(@PathVariable String username) {
         return ResponseEntity.ok(findUserOrThrowHttp404(username));
     }
 
     @Transactional
-    @DeleteMapping("/users/{username}")
+    @DeleteMapping("/{username}")
     public ResponseEntity<?> deleteOne(@PathVariable String username) {
         User user = findUserOrThrowHttp404(username);
         userRepository.delete(user);
         return ResponseEntity.noContent().build();
     }
 
     @Transactional
-    @PostMapping("/users")
-    ResponseEntity<?> saveOne(@RequestParam User user) {
+    @PostMapping
+    ResponseEntity<?> saveOne(@RequestBody User user) {
         Optional<User> persistedUser = userRepository.findByUsername(user.getUsername());
         if (persistedUser.isPresent()) {
             return ResponseEntity
                     .status(HttpStatus.CONFLICT)
                     .body(new ErrorResponse(String.valueOf(HttpStatus.CONFLICT.value()),
-                            String.format("A user with username [ %s ] already exists within the system.", user.getUsername())));
+                            String.format("A user with username [%s] already exists within the system.", user.getUsername())));
         }
-        //TODO: encrypt password?
+        user.setRoles(getPersistedRoles(user.getUsername(), user.getRoles()));
+        //TODO: encrypt password? Or is it sent to us encrypted?
         User savedUser = userRepository.save(user);
         return ResponseEntity.ok(savedUser);
     }
 
     @Transactional
-    @PutMapping("/users/{username}")
-    ResponseEntity<?> updateOne(@PathVariable(value = "username") String username, @RequestParam User user) {
-        Optional<User> userSearchResult = userRepository.findByUsername(username);
-        if (!userSearchResult.isPresent()) {
-            return ResponseEntity.status(HttpStatus.BAD_REQUEST)
-                    .body(new ErrorResponse(String.valueOf(HttpStatus.BAD_REQUEST.value()),
-                            String.format("No user with username [ %s ] exists within the system.", username)));
-        }
-        PasswordEncoder passwordEncoder = new BCryptPasswordEncoder();
-        User persistedUser = userSearchResult.get();
-        persistedUser.setPassword(passwordEncoder.encode(user.getPassword()));
+    @PutMapping("/{username}")
+    ResponseEntity<?> updateOne(@PathVariable(value = "username") String username, @RequestBody User user) {
+        User persistedUser = findUserOrThrowHttp404(username);
+        persistedUser.setPassword(user.getPassword()); //TODO: encrypt password?
         persistedUser.setFirstName(user.getFirstName());
         persistedUser.setLastName(user.getLastName());
-        persistedUser.setRoles(user.getRoles());
+        persistedUser.setEmailAddress(user.getEmailAddress());
+        persistedUser.setRoles(getPersistedRoles(user.getUsername(), user.getRoles()));
         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)));

backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Role.java

@@ -29,6 +29,10 @@
 @ToString(exclude = "users")
 public class Role extends AbstractAuditable {
 
+    public Role(String name) {
+        this.name = name;
+    }
+
     @Column(unique = true)
     private String name;
 

backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java

@@ -1,9 +1,12 @@
 package edu.internet2.tier.shibboleth.admin.ui.security.model;
 
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
-import com.fasterxml.jackson.annotation.JsonProperty;
 import edu.internet2.tier.shibboleth.admin.ui.domain.AbstractAuditable;
-import lombok.*;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+import lombok.Setter;
+import lombok.ToString;
 
 import javax.persistence.CascadeType;
 import javax.persistence.Column;
@@ -30,7 +33,8 @@ public class User extends AbstractAuditable {
     @Column(nullable = false, unique = true)
     private String username;
 
-    @JsonProperty(access = JsonProperty.Access.WRITE_ONLY)
+    //TODO: Need to figure out the right way to protect this property
+    //@JsonProperty(access = JsonProperty.Access.WRITE_ONLY)
     @Column(nullable = false)
     private String password;
 

backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersControllerIntegrationTests.groovy

@@ -1,8 +1,14 @@
 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 org.springframework.beans.factory.annotation.Autowired
 import org.springframework.boot.test.context.SpringBootTest
 import org.springframework.boot.test.web.client.TestRestTemplate
+import org.springframework.http.HttpEntity
+import org.springframework.http.HttpHeaders
 import org.springframework.test.annotation.DirtiesContext
 import org.springframework.test.context.ActiveProfiles
 import spock.lang.Specification
@@ -19,6 +25,13 @@ class UsersControllerIntegrationTests extends Specification {
 
     static RESOURCE_URI = '/api/admin/users'
 
+    ObjectMapper mapper
+
+    def setup() {
+        mapper = new ObjectMapper()
+        mapper.enable(SerializationFeature.INDENT_OUTPUT)
+    }
+
     def 'GET ALL users (when there are existing users)'() {
         when: 'GET request is made for ALL users in the system, and system has users in it'
         def result = this.restTemplate.getForEntity(RESOURCE_URI, Object)
@@ -64,4 +77,84 @@ class UsersControllerIntegrationTests extends Specification {
         then: 'The deleted user is gone'
         result.statusCodeValue == 404
     }
+
+    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
+        }
+
+        when:
+        def result = this.restTemplate.postForEntity("$RESOURCE_URI", createRequestHttpEntityFor { mapper.writeValueAsString(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
+        }
+
+        when:
+        this.restTemplate.postForEntity("$RESOURCE_URI", createRequestHttpEntityFor { mapper.writeValueAsString(newUser) }, Map)
+        def result = this.restTemplate.postForEntity("$RESOURCE_URI", createRequestHttpEntityFor { mapper.writeValueAsString(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
+        }
+
+        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)
+
+        then:
+        result.statusCodeValue == 200
+    }
+
+    def 'PUT detects unknown username'() {
+        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
+        }
+
+        when:
+        def result = this.restTemplate.exchange("$RESOURCE_URI/$newUser.username", org.springframework.http.HttpMethod.PUT, createRequestHttpEntityFor { mapper.writeValueAsString(newUser) }, Map)
+
+        then:
+        result.statusCodeValue == 404
+    }
+
+    private HttpEntity<String> createRequestHttpEntityFor(Closure jsonBodySupplier) {
+        new HttpEntity<String>(jsonBodySupplier(), ['Content-Type': 'application/json'] as HttpHeaders)
+    }
 }