Merge pull request #68 from mrc-ide/mrc-5324-update-endpoints-return-…

…data

Mrc-5324 Update return values of Create/update/delete + add role/user-update endpoint

api/app/src/main/kotlin/packit/controllers/LoginController.kt

@@ -8,15 +8,18 @@ import packit.AppConfig
 import packit.exceptions.PackitException
 import packit.model.dto.LoginWithPassword
 import packit.model.dto.LoginWithToken
+import packit.model.dto.UpdatePassword
 import packit.service.BasicLoginService
 import packit.service.GithubAPILoginService
+import packit.service.UserService
 
 @RestController
 @RequestMapping("/auth")
 class LoginController(
     val gitApiLoginService: GithubAPILoginService,
     val basicLoginService: BasicLoginService,
-    val config: AppConfig
+    val config: AppConfig,
+    val userService: UserService,
 )
 {
     @PostMapping("/login/api")
@@ -58,4 +61,20 @@ class LoginController(
         )
         return ResponseEntity.ok(authConfig)
     }
+
+    @PostMapping("/{username}/basic/password")
+    fun updatePassword(
+        @PathVariable username: String,
+        @RequestBody @Validated updatePassword: UpdatePassword
+    ): ResponseEntity<Unit>
+    {
+        if (!config.authEnableBasicLogin)
+        {
+            throw PackitException("basicLoginDisabled", HttpStatus.FORBIDDEN)
+        }
+
+        userService.updatePassword(username, updatePassword)
+
+        return ResponseEntity.noContent().build()
+    }
 }

api/app/src/main/kotlin/packit/controllers/RoleController.kt

@@ -8,41 +8,55 @@ import org.springframework.web.bind.annotation.*
 import packit.model.dto.CreateRole
 import packit.model.dto.RoleDto
 import packit.model.dto.UpdateRolePermissions
+import packit.model.dto.UpdateRoleUsers
 import packit.model.toDto
 import packit.service.RoleService
+import packit.service.UserRoleService
+import java.net.URI
 
 @Controller
 @PreAuthorize("hasAuthority('user.manage')")
 @RequestMapping("/role")
-class RoleController(private val roleService: RoleService)
+class RoleController(private val roleService: RoleService, private val userRoleService: UserRoleService)
 {
     @PostMapping()
-    fun createRole(@RequestBody @Validated createRole: CreateRole): ResponseEntity<Map<String, String?>>
+    fun createRole(@RequestBody @Validated createRole: CreateRole): ResponseEntity<RoleDto>
     {
-        roleService.createRole(createRole)
+        val role = roleService.createRole(createRole)
 
-        return ResponseEntity.ok(mapOf("message" to "Role created"))
+        return ResponseEntity.created(URI.create("/role/${role.id}")).body(role.toDto())
     }
 
     @DeleteMapping("/{roleName}")
     fun deleteRole(
         @PathVariable roleName: String
-    ): ResponseEntity<Map<String, String?>>
+    ): ResponseEntity<Unit>
     {
         roleService.deleteRole(roleName)
 
         return ResponseEntity.noContent().build()
     }
 
-    @PutMapping("/update-permissions/{roleName}")
+    @PutMapping("/{roleName}/permissions")
     fun updatePermissionsToRole(
         @RequestBody @Validated updateRolePermissions: UpdateRolePermissions,
         @PathVariable roleName: String
-    ): ResponseEntity<Unit>
+    ): ResponseEntity<RoleDto>
     {
-        roleService.updatePermissionsToRole(roleName, updateRolePermissions)
+        val updatedRole = roleService.updatePermissionsToRole(roleName, updateRolePermissions)
 
-        return ResponseEntity.noContent().build()
+        return ResponseEntity.ok(updatedRole.toDto())
+    }
+
+    @PutMapping("{roleName}/users")
+    fun updateUsersToRole(
+        @RequestBody @Validated usersToUpdate: UpdateRoleUsers,
+        @PathVariable roleName: String
+    ): ResponseEntity<RoleDto>
+    {
+        val updatedRole = userRoleService.updateRoleUsers(roleName, usersToUpdate)
+
+        return ResponseEntity.ok(updatedRole.toDto())
     }
 
     @GetMapping("/names")

api/app/src/main/kotlin/packit/controllers/UserController.kt

@@ -10,46 +10,54 @@ import packit.AppConfig
 import packit.exceptions.PackitException
 import packit.model.dto.CreateBasicUser
 import packit.model.dto.UpdateUserRoles
+import packit.model.dto.UserDto
+import packit.model.toDto
+import packit.service.UserRoleService
 import packit.service.UserService
+import java.net.URI
 
 @Controller
 @PreAuthorize("hasAuthority('user.manage')")
 @RequestMapping("/user")
-class UserController(private val config: AppConfig, private val userService: UserService)
+class UserController(
+    private val config: AppConfig,
+    private val userService: UserService,
+    private val userRoleService: UserRoleService
+)
 {
     @PostMapping("/basic")
     fun createBasicUser(
         @RequestBody @Validated createBasicUser: CreateBasicUser
-    ): ResponseEntity<Map<String, String?>>
+    ): ResponseEntity<UserDto>
     {
         if (!config.authEnableBasicLogin)
         {
             throw PackitException("basicLoginDisabled", HttpStatus.FORBIDDEN)
         }
 
-        userService.createBasicUser(createBasicUser)
+        val user = userService.createBasicUser(createBasicUser)
 
-        return ResponseEntity.ok(mapOf("message" to "User created"))
+        return ResponseEntity.created(URI.create("/user/${user.id}")).body(user.toDto())
     }
 
-    @PutMapping("/update-roles/{username}")
+    @PutMapping("/{username}/roles")
     fun updateUserRoles(
         @RequestBody @Validated updateUserRoles: UpdateUserRoles,
         @PathVariable username: String
-    ): ResponseEntity<Unit>
+    ): ResponseEntity<UserDto>
     {
-        userService.updateUserRoles(username, updateUserRoles)
+        val updatedUser = userRoleService.updateUserRoles(username, updateUserRoles)
 
-        return ResponseEntity.noContent().build()
+        return ResponseEntity.ok(updatedUser.toDto())
     }
 
     @DeleteMapping("/{username}")
     fun deleteUser(
         @PathVariable username: String
-    ): ResponseEntity<Map<String, String?>>
+    ): ResponseEntity<Unit>
     {
         userService.deleteUser(username)
 
-        return ResponseEntity.ok(mapOf("message" to "User deleted"))
+        return ResponseEntity.noContent().build()
     }
 }

api/app/src/main/kotlin/packit/exceptions/PackitExceptionHandler.kt

@@ -4,6 +4,7 @@ import org.springframework.http.HttpStatus
 import org.springframework.http.ResponseEntity
 import org.springframework.http.converter.HttpMessageNotReadableException
 import org.springframework.security.access.AccessDeniedException
+import org.springframework.security.authentication.InternalAuthenticationServiceException
 import org.springframework.security.core.AuthenticationException
 import org.springframework.web.bind.MethodArgumentNotValidException
 import org.springframework.web.bind.annotation.ControllerAdvice
@@ -67,6 +68,13 @@ class PackitExceptionHandler
             .toResponseEntity()
     }
 
+    @ExceptionHandler(InternalAuthenticationServiceException::class)
+    fun handleInternalAuthenticationServiceException(e: Exception): ResponseEntity<String>
+    {
+        return ErrorDetail(HttpStatus.UNAUTHORIZED, e.message ?: "Authentication failed")
+            .toResponseEntity()
+    }
+
     @ExceptionHandler(OutpackServerException::class)
     fun handleOutpackServerException(e: OutpackServerException): ResponseEntity<String>
     {

api/app/src/main/kotlin/packit/model/Role.kt

@@ -1,6 +1,7 @@
 package packit.model
 
 import jakarta.persistence.*
+import packit.model.dto.BasicRoleDto
 import packit.model.dto.RoleDto
 
 @Entity
@@ -38,5 +39,7 @@ class Role(
 
 fun Role.toDto() =
     RoleDto(
-        name, rolePermissions.map { it.toDto() }, users.map { it.toDto() }, id!!
+        name, rolePermissions.map { it.toDto() }, users.map { it.toBasicDto() }, id!!
     )
+
+fun Role.toBasicDto() = BasicRoleDto(name, id!!)

api/app/src/main/kotlin/packit/model/User.kt

@@ -1,6 +1,7 @@
 package packit.model
 
 import jakarta.persistence.*
+import packit.model.dto.BasicUserDto
 import packit.model.dto.UserDto
 import java.time.Instant
 import java.util.*
@@ -20,11 +21,14 @@ class User(
     val userSource: String,
     val displayName: String?,
     val email: String? = null,
-    val password: String? = null,
+    var password: String? = null,
     var lastLoggedIn: Instant? = null,
     @Id
     @GeneratedValue(strategy = GenerationType.UUID)
     val id: UUID? = null
 )
 
-fun User.toDto() = UserDto(username, id!!)
+fun User.toBasicDto() = BasicUserDto(username, id!!)
+
+fun User.toDto() =
+    UserDto(username, roles.map { it.toBasicDto() }, disabled, userSource, displayName, email, id!!)

api/app/src/main/kotlin/packit/model/dto/BasicRoleDto.kt

@@ -0,0 +1,3 @@
+package packit.model.dto
+
+data class BasicRoleDto(val name: String, val id: Int)

api/app/src/main/kotlin/packit/model/dto/BasicUserDto.kt

@@ -0,0 +1,5 @@
+package packit.model.dto
+
+import java.util.*
+
+data class BasicUserDto(val username: String, val id: UUID)

api/app/src/main/kotlin/packit/model/dto/RoleDto.kt

@@ -3,6 +3,6 @@ package packit.model.dto
 data class RoleDto(
     var name: String,
     var rolePermissions: List<RolePermissionDto> = listOf(),
-    var users: List<UserDto> = listOf(),
+    var users: List<BasicUserDto> = listOf(),
     var id: Int
 )

api/app/src/main/kotlin/packit/model/dto/UpdatePassword.kt

@@ -0,0 +1,16 @@
+package packit.model.dto
+
+import jakarta.validation.constraints.Size
+
+data class UpdatePassword(
+    @field:Size(min = 8, message = "Password must be at least 8 characters long")
+    val currentPassword: String,
+    @field:Size(min = 8, message = "Password must be at least 8 characters long")
+    val newPassword: String
+)
+{
+    init
+    {
+        require(currentPassword != newPassword) { "New password must be different from the current password" }
+    }
+}

api/app/src/main/kotlin/packit/model/dto/UpdateRoleUsers.kt

@@ -0,0 +1,6 @@
+package packit.model.dto
+
+data class UpdateRoleUsers(
+    val usernamesToAdd: List<String> = listOf(),
+    val usernamesToRemove: List<String> = listOf()
+)

api/app/src/main/kotlin/packit/model/dto/UserDto.kt

@@ -2,4 +2,12 @@ package packit.model.dto
 
 import java.util.*
 
-data class UserDto(val username: String, val id: UUID)
+data class UserDto(
+    val username: String,
+    val roles: List<BasicRoleDto> = listOf(),
+    val disabled: Boolean,
+    val userSource: String,
+    val displayName: String?,
+    val email: String?,
+    val id: UUID
+)

api/app/src/main/kotlin/packit/repository/UserRepository.kt

@@ -9,9 +9,9 @@ import java.util.*
 interface UserRepository : JpaRepository<User, UUID>
 {
     fun findByUsername(username: String): User?
-
     fun deleteByEmail(email: String)
     fun findByEmail(email: String): User?
     fun existsByUsername(username: String): Boolean
     fun existsByEmail(email: String): Boolean
+    fun findByUsernameIn(usernames: List<String>): List<User>
 }

api/app/src/main/kotlin/packit/service/BasicLoginService.kt

@@ -12,7 +12,8 @@ import packit.security.provider.JwtIssuer
 @Component
 class BasicLoginService(
     val jwtIssuer: JwtIssuer,
-    val authenticationManager: AuthenticationManager
+    val authenticationManager: AuthenticationManager,
+    val userService: UserService
 )
 {
     fun authenticateAndIssueToken(loginRequest: LoginWithPassword): Map<String, String>
@@ -28,6 +29,7 @@ class BasicLoginService(
                 loginRequest.password
             )
         )
+        userService.checkAndUpdateLastLoggedIn(loginRequest.email)
 
         val userDetails = (authentication.principal as BasicUserDetails)
         val token = jwtIssuer.issue(userDetails.principal)

api/app/src/main/kotlin/packit/service/RolePermissionService.kt

@@ -10,7 +10,7 @@ import packit.repository.*
 
 interface RolePermissionService
 {
-    fun removeRolePermissionsFromRole(role: Role, removeRolePermissions: List<UpdateRolePermission>)
+    fun removeRolePermissionsFromRole(role: Role, removeRolePermissions: List<UpdateRolePermission>): Role
     fun getRolePermissionsToAdd(role: Role, addRolePermissions: List<UpdateRolePermission>): List<RolePermission>
 }
 
@@ -50,18 +50,22 @@ class BaseRolePermissionService(
         }
     }
 
-    override fun removeRolePermissionsFromRole(role: Role, removeRolePermissions: List<UpdateRolePermission>)
+    override fun removeRolePermissionsFromRole(role: Role, removeRolePermissions: List<UpdateRolePermission>): Role
     {
         val rolePermissionsToRemove = getRolePermissionsToUpdate(role, removeRolePermissions)
+        var rolePermissionsToRemoveIds = mutableListOf<Int>()
 
-        val matchedRolePermissionsToRemove = rolePermissionsToRemove.map { rolePermissionToRemove ->
-            val matchedPermission = role.rolePermissions.find { rolePermissionToRemove == it }
+        for (rolePermission in rolePermissionsToRemove)
+        {
+            val matchedRolePermission = role.rolePermissions.find { rolePermission == it }
                 ?: throw PackitException("rolePermissionDoesNotExist", HttpStatus.BAD_REQUEST)
 
-            matchedPermission
+            rolePermissionsToRemoveIds.add(matchedRolePermission.id!!)
+            role.rolePermissions.remove(matchedRolePermission)
         }
 
-        rolePermissionRepository.deleteAllByIdIn(matchedRolePermissionsToRemove.map { it.id!! })
+        rolePermissionRepository.deleteAllByIdIn(rolePermissionsToRemoveIds)
+        return role
     }
 
     override fun getRolePermissionsToAdd(