From 58c615072a343317915f450ef09522824ec2e68c Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Wed, 1 May 2024 07:59:06 +0100 Subject: [PATCH 01/25] feat: get updates working --- .../packit/controllers/RoleController.kt | 29 +++++++- .../main/kotlin/packit/model/PacketGroup.kt | 2 +- .../kotlin/packit/model/RolePermission.kt | 16 +++++ .../packit/model/dto/UpdateRolePermission.kt | 21 ++++++ .../packit/repository/PermissionRepository.kt | 1 + .../repository/RolePermissionRepository.kt | 17 +++++ .../packit/repository/RoleRepository.kt | 3 + .../packit/service/RolePermissionService.kt | 66 +++++++++++++++++++ .../main/kotlin/packit/service/RoleService.kt | 42 ++++++++++-- .../src/main/resources/errorBundle.properties | 7 +- 10 files changed, 195 insertions(+), 9 deletions(-) create mode 100644 api/app/src/main/kotlin/packit/model/dto/UpdateRolePermission.kt create mode 100644 api/app/src/main/kotlin/packit/repository/RolePermissionRepository.kt create mode 100644 api/app/src/main/kotlin/packit/service/RolePermissionService.kt diff --git a/api/app/src/main/kotlin/packit/controllers/RoleController.kt b/api/app/src/main/kotlin/packit/controllers/RoleController.kt index 20b2ff51..160c64f6 100644 --- a/api/app/src/main/kotlin/packit/controllers/RoleController.kt +++ b/api/app/src/main/kotlin/packit/controllers/RoleController.kt @@ -9,6 +9,7 @@ import org.springframework.web.bind.annotation.PostMapping import org.springframework.web.bind.annotation.RequestBody import org.springframework.web.bind.annotation.RequestMapping import packit.model.dto.CreateRole +import packit.model.dto.UpdateRolePermission import packit.service.RoleService @Controller @@ -24,13 +25,35 @@ class RoleController(private val roleService: RoleService) return ResponseEntity.ok(mapOf("message" to "Role created")) } - @PostMapping("/delete/{name}") + @PostMapping("/delete/{roleName}") fun deleteRole( - @PathVariable name: String + @PathVariable roleName: String ): ResponseEntity> { - roleService.deleteRole(name) + roleService.deleteRole(roleName) return ResponseEntity.ok(mapOf("message" to "Role deleted")) } + + @PostMapping("/add-permissions/{roleName}") + fun addPermissionsToRole( + @RequestBody @Validated addRolePermissions: List, + @PathVariable roleName: String + ): ResponseEntity> + { + roleService.addPermissionsToRole(roleName, addRolePermissions) + + return ResponseEntity.ok(mapOf("message" to "Permissions added")) + } + + @PostMapping("/remove-permissions/{roleName}") + fun removePermissionsFromRole( + @RequestBody @Validated removeRolePermissions: List, + @PathVariable roleName: String + ): ResponseEntity> + { + roleService.removePermissionsFromRole(roleName, removeRolePermissions) + + return ResponseEntity.ok(mapOf("message" to "Permissions removed")) + } } diff --git a/api/app/src/main/kotlin/packit/model/PacketGroup.kt b/api/app/src/main/kotlin/packit/model/PacketGroup.kt index 81a22edc..48ba3349 100644 --- a/api/app/src/main/kotlin/packit/model/PacketGroup.kt +++ b/api/app/src/main/kotlin/packit/model/PacketGroup.kt @@ -6,7 +6,7 @@ import jakarta.persistence.* @Table(name = "packet_group") class PacketGroup( var name: String, - @OneToMany(mappedBy = "packetGroup") + @OneToMany(mappedBy = "packetGroup", cascade = [CascadeType.ALL]) var rolePermissions: MutableList = mutableListOf(), @Id @GeneratedValue(strategy = GenerationType.IDENTITY) diff --git a/api/app/src/main/kotlin/packit/model/RolePermission.kt b/api/app/src/main/kotlin/packit/model/RolePermission.kt index c7e4b826..7124455c 100644 --- a/api/app/src/main/kotlin/packit/model/RolePermission.kt +++ b/api/app/src/main/kotlin/packit/model/RolePermission.kt @@ -29,3 +29,19 @@ class RolePermission( @GeneratedValue(strategy = GenerationType.IDENTITY) var id: Int? = null, ) +{ + override fun equals(other: Any?): Boolean + { + return when + { + this === other -> true + other !is RolePermission -> false + role.name != other.role.name -> false + permission.name != other.permission.name -> false + packet?.id != other.packet?.id -> false + packetGroup?.id != other.packetGroup?.id -> false + tag?.id != other.tag?.id -> false + else -> true + } + } +} diff --git a/api/app/src/main/kotlin/packit/model/dto/UpdateRolePermission.kt b/api/app/src/main/kotlin/packit/model/dto/UpdateRolePermission.kt new file mode 100644 index 00000000..3ec26ff1 --- /dev/null +++ b/api/app/src/main/kotlin/packit/model/dto/UpdateRolePermission.kt @@ -0,0 +1,21 @@ +package packit.model.dto + +import org.jetbrains.annotations.NotNull + +data class UpdateRolePermission( + @field:NotNull + val permission: String, + val packetId: String? = null, + val tagId: Int? = null, + val packetGroupId: Int? = null +) +{ + init + { + val nonNullFields = listOf(packetId, tagId, packetGroupId).count { it != null } + require(nonNullFields <= 1) { + "Either all of packetId, tagId, packetGroupId should be null or only one of them should be not null" + } + } + +} \ No newline at end of file diff --git a/api/app/src/main/kotlin/packit/repository/PermissionRepository.kt b/api/app/src/main/kotlin/packit/repository/PermissionRepository.kt index e3cea30f..2d292a6f 100644 --- a/api/app/src/main/kotlin/packit/repository/PermissionRepository.kt +++ b/api/app/src/main/kotlin/packit/repository/PermissionRepository.kt @@ -8,4 +8,5 @@ import packit.model.Permission interface PermissionRepository : JpaRepository { fun findByNameIn(names: List): List + fun findByName(name: String): Permission? } diff --git a/api/app/src/main/kotlin/packit/repository/RolePermissionRepository.kt b/api/app/src/main/kotlin/packit/repository/RolePermissionRepository.kt new file mode 100644 index 00000000..08d4d8ef --- /dev/null +++ b/api/app/src/main/kotlin/packit/repository/RolePermissionRepository.kt @@ -0,0 +1,17 @@ +package packit.repository + +import org.springframework.data.jpa.repository.JpaRepository +import org.springframework.data.jpa.repository.Modifying +import org.springframework.data.jpa.repository.Query +import org.springframework.stereotype.Repository +import org.springframework.transaction.annotation.Transactional +import packit.model.RolePermission + +@Repository +interface RolePermissionRepository : JpaRepository +{ + @Modifying + @Transactional + @Query("DELETE FROM RolePermission rp WHERE rp.id IN :ids") + fun deleteAllByIdIn(ids: List) +} \ No newline at end of file diff --git a/api/app/src/main/kotlin/packit/repository/RoleRepository.kt b/api/app/src/main/kotlin/packit/repository/RoleRepository.kt index 5a1b7d64..39b2c9fb 100644 --- a/api/app/src/main/kotlin/packit/repository/RoleRepository.kt +++ b/api/app/src/main/kotlin/packit/repository/RoleRepository.kt @@ -2,6 +2,7 @@ package packit.repository import org.springframework.data.jpa.repository.JpaRepository import org.springframework.stereotype.Repository +import org.springframework.transaction.annotation.Transactional import packit.model.Role @Repository @@ -10,5 +11,7 @@ interface RoleRepository : JpaRepository fun findByName(name: String): Role? fun existsByName(name: String): Boolean fun findByNameIn(names: List): List + + @Transactional fun deleteByName(name: String) } diff --git a/api/app/src/main/kotlin/packit/service/RolePermissionService.kt b/api/app/src/main/kotlin/packit/service/RolePermissionService.kt new file mode 100644 index 00000000..0e95631b --- /dev/null +++ b/api/app/src/main/kotlin/packit/service/RolePermissionService.kt @@ -0,0 +1,66 @@ +package packit.service + +import org.springframework.http.HttpStatus +import org.springframework.stereotype.Service +import packit.exceptions.PackitException +import packit.model.Role +import packit.model.RolePermission +import packit.model.dto.UpdateRolePermission +import packit.repository.* + + +interface RolePermissionService +{ + fun getRolePermissionsToUpdate(role: Role, addRolePermissions: List): List + fun removeRolePermissionsFromRole(role: Role, removeRolePermissions: List) +} + +@Service +class BaseRolePermissionService( + private val permissionRepository: PermissionRepository, + private val packetRepository: PacketRepository, + private val packetGroupRepository: PacketGroupRepository, + private val tagRepository: TagRepository, + private val rolePermissionRepository: RolePermissionRepository +) : RolePermissionService +{ + override fun getRolePermissionsToUpdate( + role: Role, + addRolePermissions: List + ): List + { + return addRolePermissions.map { addRolePermission -> + val permission = permissionRepository.findByName(addRolePermission.permission) + ?: throw PackitException("permissionNotFound", HttpStatus.BAD_REQUEST) + RolePermission( + role = role, + permission = permission, + packet = addRolePermission.packetId?.let { + packetRepository.findById(it) + .orElseThrow { PackitException("packetNotFound", HttpStatus.BAD_REQUEST) } + }, + packetGroup = addRolePermission.packetGroupId?.let { + packetGroupRepository.findById(it) + .orElseThrow { PackitException("packetGroupNotFound", HttpStatus.BAD_REQUEST) } + }, + tag = addRolePermission.tagId?.let { + tagRepository.findById(it).orElseThrow { PackitException("tagNotFound", HttpStatus.BAD_REQUEST) } + } + ) + } + } + + + override fun removeRolePermissionsFromRole(role: Role, removeRolePermissions: List) + { + val rolePermissionsToRemove = getRolePermissionsToUpdate(role, removeRolePermissions) + + val matchedRolePermissionsToRemove = rolePermissionsToRemove.map { rolePermissionsToRemove -> + val matchedPermission = role.rolePermissions.find { rolePermissionsToRemove == it } + ?: throw PackitException("rolePermissionDoesNotExist", HttpStatus.BAD_REQUEST) + matchedPermission + } + + rolePermissionRepository.deleteAllByIdIn(matchedRolePermissionsToRemove.map { it.id!! }) + } +} \ No newline at end of file diff --git a/api/app/src/main/kotlin/packit/service/RoleService.kt b/api/app/src/main/kotlin/packit/service/RoleService.kt index 8c94bab3..901c1477 100644 --- a/api/app/src/main/kotlin/packit/service/RoleService.kt +++ b/api/app/src/main/kotlin/packit/service/RoleService.kt @@ -10,6 +10,7 @@ import packit.model.Permission import packit.model.Role import packit.model.RolePermission import packit.model.dto.CreateRole +import packit.model.dto.UpdateRolePermission import packit.repository.RoleRepository interface RoleService @@ -20,12 +21,15 @@ interface RoleService fun getGrantedAuthorities(roles: List): MutableList fun createRole(createRole: CreateRole) fun deleteRole(roleName: String) + fun addPermissionsToRole(roleName: String, addRolePermissions: List) + fun removePermissionsFromRole(roleName: String, removeRolePermissions: List) } @Service class BaseRoleService( private val roleRepository: RoleRepository, - private val permissionService: PermissionService + private val permissionService: PermissionService, + private val rolePermissionService: BaseRolePermissionService ) : RoleService { override fun getUsernameRole(username: String): Role @@ -56,11 +60,9 @@ class BaseRoleService( saveRole(createRole.name, permissions) } - - @Transactional + override fun deleteRole(roleName: String) { - if (!roleRepository.existsByName(roleName)) { throw PackitException("roleNotFound", HttpStatus.BAD_REQUEST) @@ -68,6 +70,38 @@ class BaseRoleService( roleRepository.deleteByName(roleName) } + override fun addPermissionsToRole(roleName: String, addRolePermissions: List) + { + val role = roleRepository.findByName(roleName) + ?: throw PackitException("roleNotFound", HttpStatus.BAD_REQUEST) + + val rolePermissionsToAdd = rolePermissionService.getRolePermissionsToUpdate(role, addRolePermissions) + if (rolePermissionsToAdd.any { role.rolePermissions.contains(it) }) + { + throw PackitException("rolePermissionAlreadyExists", HttpStatus.BAD_REQUEST) + } + + role.rolePermissions.addAll(rolePermissionsToAdd) + roleRepository.save(role) + } + + + override fun removePermissionsFromRole(roleName: String, removeRolePermissions: List) + { + val role = roleRepository.findByName(roleName) + ?: throw PackitException("roleNotFound", HttpStatus.BAD_REQUEST) +// match them to role permissions and then delete those ones!!! + rolePermissionService.removeRolePermissionsFromRole(role, removeRolePermissions) + +// if (rolePermissionsToRemove.any { !role.rolePermissions.contains(it) }) +// { +// throw PackitException("rolePermissionDoesNotExist", HttpStatus.BAD_REQUEST) +// } +// +// role.rolePermissions.removeAll(rolePermissionsToRemove) +// roleRepository.save(role) + } + internal fun saveRole(roleName: String, permissions: List = listOf()) { if (roleRepository.existsByName(roleName)) diff --git a/api/app/src/main/resources/errorBundle.properties b/api/app/src/main/resources/errorBundle.properties index 0151e043..28f84d08 100644 --- a/api/app/src/main/resources/errorBundle.properties +++ b/api/app/src/main/resources/errorBundle.properties @@ -14,4 +14,9 @@ invalidPermissionsProvided=Invalid permissions provided userAlreadyExists=User already exists userNotFound=User not found roleAlreadyExists=Role already exists -roleNotFound=Role not found \ No newline at end of file +roleNotFound=Role not found +rolePermissionAlreadyExists=Role permission already exists +packetNotFound=Packet not found +packetGroupNotFound=Packet group not found +tagNotFound=Tag not found +rolePermissionDoesNotExist=Permission does not exist on Role \ No newline at end of file From 9580f708b20be17c6b2aaba68a67e2429fb29e62 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Wed, 1 May 2024 09:49:32 +0100 Subject: [PATCH 02/25] Remove unused code in RoleService --- .../src/main/kotlin/packit/service/RoleService.kt | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/api/app/src/main/kotlin/packit/service/RoleService.kt b/api/app/src/main/kotlin/packit/service/RoleService.kt index 901c1477..995ab477 100644 --- a/api/app/src/main/kotlin/packit/service/RoleService.kt +++ b/api/app/src/main/kotlin/packit/service/RoleService.kt @@ -4,7 +4,6 @@ import org.springframework.http.HttpStatus import org.springframework.security.core.GrantedAuthority import org.springframework.security.core.authority.SimpleGrantedAuthority import org.springframework.stereotype.Service -import org.springframework.transaction.annotation.Transactional import packit.exceptions.PackitException import packit.model.Permission import packit.model.Role @@ -60,7 +59,7 @@ class BaseRoleService( saveRole(createRole.name, permissions) } - + override fun deleteRole(roleName: String) { if (!roleRepository.existsByName(roleName)) @@ -90,16 +89,7 @@ class BaseRoleService( { val role = roleRepository.findByName(roleName) ?: throw PackitException("roleNotFound", HttpStatus.BAD_REQUEST) -// match them to role permissions and then delete those ones!!! rolePermissionService.removeRolePermissionsFromRole(role, removeRolePermissions) - -// if (rolePermissionsToRemove.any { !role.rolePermissions.contains(it) }) -// { -// throw PackitException("rolePermissionDoesNotExist", HttpStatus.BAD_REQUEST) -// } -// -// role.rolePermissions.removeAll(rolePermissionsToRemove) -// roleRepository.save(role) } internal fun saveRole(roleName: String, permissions: List = listOf()) From 49dd4aa194d8b6d45bf9214f96f9766f20794c12 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Wed, 1 May 2024 11:29:50 +0100 Subject: [PATCH 03/25] test: test logic --- .../main/kotlin/packit/service/RoleService.kt | 2 +- .../controllers/RoleControllerTest.kt | 58 ++++++ .../packit/unit/model/RolePermissionTest.kt | 94 +++++++++ .../unit/model/UpdateRolePermissionTest.kt | 49 +++++ .../unit/service/RolePermissionServiceTest.kt | 183 ++++++++++++++++++ .../packit/unit/service/RoleServiceTest.kt | 91 ++++++++- 6 files changed, 474 insertions(+), 3 deletions(-) create mode 100644 api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt create mode 100644 api/app/src/test/kotlin/packit/unit/model/UpdateRolePermissionTest.kt create mode 100644 api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt diff --git a/api/app/src/main/kotlin/packit/service/RoleService.kt b/api/app/src/main/kotlin/packit/service/RoleService.kt index 995ab477..6560f9af 100644 --- a/api/app/src/main/kotlin/packit/service/RoleService.kt +++ b/api/app/src/main/kotlin/packit/service/RoleService.kt @@ -28,7 +28,7 @@ interface RoleService class BaseRoleService( private val roleRepository: RoleRepository, private val permissionService: PermissionService, - private val rolePermissionService: BaseRolePermissionService + private val rolePermissionService: RolePermissionService ) : RoleService { override fun getUsernameRole(username: String): Role diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index 520fd3da..7e5d0eab 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -8,7 +8,10 @@ import org.springframework.test.context.jdbc.Sql import packit.integration.IntegrationTest import packit.integration.WithAuthenticatedUser import packit.model.Role +import packit.model.RolePermission import packit.model.dto.CreateRole +import packit.model.dto.UpdateRolePermission +import packit.repository.PermissionRepository import packit.repository.RoleRepository import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -19,12 +22,23 @@ class RoleControllerTest : IntegrationTest() { @Autowired private lateinit var roleRepository: RoleRepository + + @Autowired + private lateinit var permissionsRepository: PermissionRepository + private val createTestRoleBody = ObjectMapper().writeValueAsString( CreateRole( name = "testRole", permissions = listOf("packet.run", "packet.read") ) ) + private val updateRolePermission = ObjectMapper().writeValueAsString( + listOf( + UpdateRolePermission( + permission = "packet.run" + ) + ) + ) @Test @WithAuthenticatedUser(authorities = ["user.manage"]) @@ -81,4 +95,48 @@ class RoleControllerTest : IntegrationTest() assertSuccess(result) assertNull(roleRepository.findByName("testRole")) } + + @Test + @WithAuthenticatedUser(authorities = ["user.manage"]) + fun `users with manage authority can add permissions to roles`() + { + roleRepository.save(Role(name = "testRole")) + + val result = restTemplate.postForEntity( + "/role/add-permissions/testRole", + getTokenizedHttpEntity(data = updateRolePermission), + String::class.java + ) + + assertSuccess(result) + val role = roleRepository.findByName("testRole")!! + assertEquals(1, role.rolePermissions.size) + assertEquals("packet.run", role.rolePermissions.first().permission.name) + } + + @Test + @WithAuthenticatedUser(authorities = ["user.manage"]) + fun `users with manage authority can remove permissions from roles`() + { + val roleName = "testRole" + val baseRole = roleRepository.save(Role(name = roleName)) + val permission = permissionsRepository.findByName("packet.run")!! + baseRole.rolePermissions = mutableListOf( + RolePermission( + baseRole, + permission + ) + ) + roleRepository.save(baseRole) + + val result = restTemplate.postForEntity( + "/role/remove-permissions/testRole", + getTokenizedHttpEntity(data = updateRolePermission), + String::class.java + ) + + assertSuccess(result) + val role = roleRepository.findByName("testRole")!! + assertEquals(0, role.rolePermissions.size) + } } diff --git a/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt b/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt new file mode 100644 index 00000000..2ffd0a06 --- /dev/null +++ b/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt @@ -0,0 +1,94 @@ +package packit.unit.model + +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import packit.model.* +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotEquals + + +class RolePermissionTest +{ + private val mockRoles = listOf( + Role("role1"), + Role("role2"), + ) + private val mockPermissions = listOf( + Permission("permission1", description = "d1"), + Permission("permission2", description = "d2"), + ) + + @Test + fun `equals returns true when comparing same instance`() + { + val rolePermission = RolePermission(mockRoles.first(), mockPermissions.first()) + assertEquals(rolePermission, rolePermission) + } + + @Test + fun `equals returns false when comparing with non RolePermission instance`() + { + val rolePermission = RolePermission(mockRoles.first(), mockPermissions.first()) + assertFalse(rolePermission.equals("not a RolePermission")) + } + + @Test + fun `equals returns false when role names are different`() + { + val rolePermission1 = RolePermission(mockRoles.first(), mockPermissions.first()) + val rolePermission2 = RolePermission(mockRoles.last(), mockPermissions.first()) + assertNotEquals(rolePermission1, rolePermission2) + } + + @Test + fun `equals returns false when permission names are different`() + { + val rolePermission1 = RolePermission(mockRoles.first(), mockPermissions.first()) + val rolePermission2 = RolePermission(mockRoles.first(), mockPermissions.last()) + assertNotEquals(rolePermission1, rolePermission2) + } + + @Test + fun `equals returns true when all properties are equal`() + { + val rolePermission1 = RolePermission(mockRoles.first(), mockPermissions.first()) + val rolePermission2 = RolePermission(mockRoles.first(), mockPermissions.first()) + assertEquals(rolePermission1, rolePermission2) + } + + @Test + fun `equals returns false when packet ids are different`() + { + + val rolePermission1 = + RolePermission(mockRoles.first(), mockPermissions.first(), mock { on { id } doReturn "2024111" }) + val rolePermission2 = + RolePermission(mockRoles.first(), mockPermissions.first(), mock { on { id } doReturn "2023344" }) + assertNotEquals(rolePermission1, rolePermission2) + } + + + @Test + fun `equals returns false when packetGroup ids are different`() + { + val rolePermission1 = + RolePermission(mockRoles.first(), mockPermissions.first(), null, mock { on { id } doReturn 1 }) + val rolePermission2 = + RolePermission(mockRoles.first(), mockPermissions.first(), null, mock { on { id } doReturn 2 }) + assertNotEquals(rolePermission1, rolePermission2) + } + + + @Test + fun `equals returns false when tag ids are different`() + { + val rolePermission1 = + RolePermission(mockRoles.first(), mockPermissions.first(), null, null, mock { on { id } doReturn 1 }) + val rolePermission2 = + RolePermission(mockRoles.first(), mockPermissions.first(), null, null, mock { on { id } doReturn 2 }) + assertNotEquals(rolePermission1, rolePermission2) + } + +} \ No newline at end of file diff --git a/api/app/src/test/kotlin/packit/unit/model/UpdateRolePermissionTest.kt b/api/app/src/test/kotlin/packit/unit/model/UpdateRolePermissionTest.kt new file mode 100644 index 00000000..2c46490b --- /dev/null +++ b/api/app/src/test/kotlin/packit/unit/model/UpdateRolePermissionTest.kt @@ -0,0 +1,49 @@ +package packit.unit.model + +import org.junit.jupiter.api.assertDoesNotThrow +import org.junit.jupiter.api.assertThrows +import packit.model.dto.UpdateRolePermission +import kotlin.test.Test + +class UpdateRolePermissionTest +{ + @Test + fun `constructor throws when more than one field is non-null`() + { + assertThrows { + UpdateRolePermission("permission", "packetId", 1, 1) + } + } + + @Test + fun `constructor does not throw when all fields are null`() + { + assertDoesNotThrow { + UpdateRolePermission("permission", null, null, null) + } + } + + @Test + fun `constructor does not throw when only packetId is non-null`() + { + assertDoesNotThrow { + UpdateRolePermission("permission", "packetId", null, null) + } + } + + @Test + fun `constructor does not throw when only tagId is non-null`() + { + assertDoesNotThrow { + UpdateRolePermission("permission", null, 1, null) + } + } + + @Test + fun `constructor does not throw when only packetGroupId is non-null`() + { + assertDoesNotThrow { + UpdateRolePermission("permission", null, null, 1) + } + } +} \ No newline at end of file diff --git a/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt new file mode 100644 index 00000000..b4fa418e --- /dev/null +++ b/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt @@ -0,0 +1,183 @@ +package packit.unit.service + +import org.junit.jupiter.api.assertThrows +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.springframework.http.HttpStatus +import packit.exceptions.PackitException +import packit.model.* +import packit.model.dto.UpdateRolePermission +import packit.repository.* +import packit.service.BaseRolePermissionService +import java.util.* +import kotlin.test.Test +import kotlin.test.assertEquals + +class RolePermissionServiceTest +{ + + private val permissionRepository: PermissionRepository = mock() + private val packetRepository: PacketRepository = mock() + private val packetGroupRepository: PacketGroupRepository = mock() + private val tagRepository: TagRepository = mock() + private val rolePermissionRepository: RolePermissionRepository = mock() + + private val service = BaseRolePermissionService( + permissionRepository, + packetRepository, + packetGroupRepository, + tagRepository, + rolePermissionRepository + ) + + @Test + fun `getRolePermissionsToUpdate returns RolePermission list when all entities are found`() + { + val role = Role("role1") + val mockPacket = mock() + val updateRolePermission = UpdateRolePermission("permission1", "id-1") + whenever(permissionRepository.findByName(any())).thenReturn(mock()) + whenever(packetRepository.findById(any())).thenReturn(Optional.of(mockPacket)) + + val result = service.getRolePermissionsToUpdate(role, listOf(updateRolePermission)) + + assertEquals(1, result.size) + assertEquals(role, result[0].role) + assertEquals(mockPacket, result[0].packet) + } + + @Test + fun `getRolePermissionsToUpdate returns RolePermission list when packetGroup is found`() + { + val role = Role("role1") + val mockPacketGroup = mock() + val updateRolePermission = UpdateRolePermission("permission1", packetGroupId = 1) + whenever(permissionRepository.findByName(any())).thenReturn(mock()) + whenever(packetGroupRepository.findById(any())).thenReturn(Optional.of(mockPacketGroup)) + + val result = service.getRolePermissionsToUpdate(role, listOf(updateRolePermission)) + + assertEquals(1, result.size) + assertEquals(role, result[0].role) + assertEquals(mockPacketGroup, result[0].packetGroup) + } + + @Test + fun `getRolePermissionsToUpdate returns RolePermission list when tag is found`() + { + val role = Role("role1") + val mockTag = mock() + val updateRolePermission = UpdateRolePermission("permission1", tagId = 1) + whenever(permissionRepository.findByName(any())).thenReturn(mock()) + whenever(tagRepository.findById(any())).thenReturn(Optional.of(mockTag)) + + val result = service.getRolePermissionsToUpdate(role, listOf(updateRolePermission)) + + assertEquals(1, result.size) + assertEquals(role, result[0].role) + assertEquals(mockTag, result[0].tag) + } + + @Test + fun `getRolePermissionsToUpdate throws PackitException when permission is not found`() + { + val role = Role("role1") + val updateRolePermission = UpdateRolePermission("permission1") + whenever(permissionRepository.findByName(any())).thenReturn(null) + + assertThrows { + service.getRolePermissionsToUpdate(role, listOf(updateRolePermission)) + }.apply { + assertEquals("permissionNotFound", key) + assertEquals(HttpStatus.BAD_REQUEST, httpStatus) + } + } + + @Test + fun `getRolePermissionsToUpdate throws PackitException when packet is not found`() + { + val role = Role("role1") + val updateRolePermission = UpdateRolePermission("permission1", "packet1") + whenever(permissionRepository.findByName(any())).thenReturn(mock()) + whenever(packetRepository.findById(any())).thenReturn(Optional.empty()) + + assertThrows { + service.getRolePermissionsToUpdate(role, listOf(updateRolePermission)) + }.apply { + assertEquals("packetNotFound", key) + assertEquals(HttpStatus.BAD_REQUEST, httpStatus) + } + } + + @Test + fun `getRolePermissionsToUpdate throws PackitException when packetGroup is not found`() + { + val role = Role("role1") + val updateRolePermission = UpdateRolePermission("permission1", packetGroupId = 1) + whenever(permissionRepository.findByName(any())).thenReturn(mock()) + whenever(packetGroupRepository.findById(any())).thenReturn(Optional.empty()) + + assertThrows { + service.getRolePermissionsToUpdate(role, listOf(updateRolePermission)) + }.apply { + assertEquals("packetGroupNotFound", key) + assertEquals(HttpStatus.BAD_REQUEST, httpStatus) + } + } + + @Test + fun `getRolePermissionsToUpdate throws PackitException when tag is not found`() + { + val role = Role("role1") + val updateRolePermission = UpdateRolePermission("permission1", tagId = 1) + whenever(permissionRepository.findByName(any())).thenReturn(mock()) + whenever(tagRepository.findById(any())).thenReturn(Optional.empty()) + + assertThrows { + service.getRolePermissionsToUpdate(role, listOf(updateRolePermission)) + }.apply { + assertEquals("tagNotFound", key) + assertEquals(HttpStatus.BAD_REQUEST, httpStatus) + } + } + + @Test + fun `removeRolePermissionsFromRole removes role permissions when they exist`() + { + val role = Role("role1") + val permission1 = Permission("permission1", "d1") + val updateRolePermissions = listOf(UpdateRolePermission(permission1.name)) + role.rolePermissions = mutableListOf( + RolePermission(role, permission1, id = 1), + RolePermission(role, Permission("permission2", "d2"), id = 2) + ) + whenever(permissionRepository.findByName(any())).thenReturn(permission1) + + service.removeRolePermissionsFromRole(role, updateRolePermissions) + + verify(rolePermissionRepository).deleteAllByIdIn(listOf(role.rolePermissions.first().id!!)) + } + + @Test + fun `removeRolePermissionsFromRole throws PackitException when role permission does not exist`() + { + val role = Role("role1") + val permission1 = Permission("permission1", "d1") + val updateRolePermissions = + listOf(UpdateRolePermission(permission1.name)) + role.rolePermissions = mutableListOf( + RolePermission(role, Permission("permission2", "d2"), id = 2) + ) + whenever(permissionRepository.findByName(any())).thenReturn(permission1) + + assertThrows { + service.removeRolePermissionsFromRole(role, updateRolePermissions) + }.apply { + assertEquals("rolePermissionDoesNotExist", key) + assertEquals(HttpStatus.BAD_REQUEST, httpStatus) + } + } + +} \ No newline at end of file diff --git a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt index 940de7fe..db4561b7 100644 --- a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt @@ -4,7 +4,9 @@ import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertThrows import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows import org.mockito.kotlin.* +import org.springframework.http.HttpStatus import org.springframework.security.core.authority.SimpleGrantedAuthority import packit.exceptions.PackitException import packit.model.Packet @@ -15,6 +17,7 @@ import packit.model.dto.CreateRole import packit.repository.RoleRepository import packit.service.BaseRoleService import packit.service.PermissionService +import packit.service.RolePermissionService import kotlin.test.assertTrue class RoleServiceTest @@ -22,13 +25,15 @@ class RoleServiceTest private lateinit var roleRepository: RoleRepository private lateinit var roleService: BaseRoleService private lateinit var permissionService: PermissionService + private lateinit var rolePermissionService: RolePermissionService @BeforeEach fun setup() { roleRepository = mock() permissionService = mock() - roleService = BaseRoleService(roleRepository, permissionService) + rolePermissionService = mock() + roleService = BaseRoleService(roleRepository, permissionService, rolePermissionService) } @Test @@ -229,11 +234,93 @@ class RoleServiceTest val roleName = "nonExistingRole" whenever(roleRepository.existsByName(roleName)).thenReturn(false) - assertThrows(PackitException::class.java) { + assertThrows { roleService.deleteRole(roleName) } } + @Test + fun `addPermissionsToRole throws exception when role does not exist`() + { + val roleName = "nonExistingRole" + whenever(roleRepository.findByName(roleName)).thenReturn(null) + + assertThrows { + roleService.addPermissionsToRole(roleName, listOf()) + }.apply { + assertEquals("roleNotFound", key) + assertEquals(HttpStatus.BAD_REQUEST, httpStatus) + } + } + + @Test + fun `addPermissionsToRole throws exception when role permission already exists`() + { + val roleName = "roleName" + val permissionName = "permission1" + val role = createRoleWithPermission(roleName, permissionName) + whenever(roleRepository.findByName(roleName)).thenReturn(role) + whenever(rolePermissionService.getRolePermissionsToUpdate(role, listOf())).thenReturn( + listOf( + createRoleWithPermission(roleName, permissionName).rolePermissions.first() + ) + ) + + assertThrows { + roleService.addPermissionsToRole(roleName, listOf()) + }.apply { + assertEquals("rolePermissionAlreadyExists", key) + assertEquals(HttpStatus.BAD_REQUEST, httpStatus) + } + } + + @Test + fun `addPermissionsToRole calls getRolePermissionsToUpdate and saves role with added role permission`() + { + val roleName = "roleName" + val permissionName = "permission1" + val role = createRoleWithPermission(roleName, permissionName) + whenever(roleRepository.findByName(roleName)).thenReturn(role) + whenever(rolePermissionService.getRolePermissionsToUpdate(role, listOf())).thenReturn( + listOf( + createRoleWithPermission(roleName, "differentPermission").rolePermissions.first() + ) + ) + + roleService.addPermissionsToRole(roleName, listOf()) + + verify(roleRepository).save(argThat { + this == role + this.rolePermissions.size == 2 + }) + } + + @Test + fun `removePermissionsFromRole throws exception when role does not exist`() + { + val roleName = "nonExistingRole" + whenever(roleRepository.findByName(roleName)).thenReturn(null) + + assertThrows { + roleService.removePermissionsFromRole(roleName, listOf()) + }.apply { + assertEquals("roleNotFound", key) + assertEquals(HttpStatus.BAD_REQUEST, httpStatus) + } + } + + @Test + fun `removePermissionsFromRole calls removeRolePermissionsFromRole`() + { + val roleName = "roleName" + val role = Role(name = roleName) + whenever(roleRepository.findByName(roleName)).thenReturn(role) + + roleService.removePermissionsFromRole(roleName, listOf()) + + verify(rolePermissionService).removeRolePermissionsFromRole(role, listOf()) + } + private fun createRoleWithPermission( roleName: String, permissionName: String, From 8baee2c40cc73ac63d38a48856a1e20306d027dd Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Wed, 1 May 2024 12:16:31 +0100 Subject: [PATCH 04/25] Fix formatting and remove unnecessary code --- .../kotlin/packit/model/RolePermission.kt | 11 ++++ .../packit/model/dto/UpdateRolePermission.kt | 3 +- .../repository/RolePermissionRepository.kt | 2 +- .../packit/service/RolePermissionService.kt | 4 +- .../main/kotlin/packit/service/RoleService.kt | 1 - .../packit/unit/model/RolePermissionTest.kt | 58 +++++++++++++++++-- .../unit/model/UpdateRolePermissionTest.kt | 2 +- .../unit/service/RolePermissionServiceTest.kt | 3 +- .../packit/unit/service/RoleServiceTest.kt | 6 +- 9 files changed, 74 insertions(+), 16 deletions(-) diff --git a/api/app/src/main/kotlin/packit/model/RolePermission.kt b/api/app/src/main/kotlin/packit/model/RolePermission.kt index 7124455c..34c196af 100644 --- a/api/app/src/main/kotlin/packit/model/RolePermission.kt +++ b/api/app/src/main/kotlin/packit/model/RolePermission.kt @@ -44,4 +44,15 @@ class RolePermission( else -> true } } + + override fun hashCode(): Int + { + val prime = 31 + var result = role.name.hashCode() + result = prime * result + permission.name.hashCode() + result = prime * result + packet?.id.hashCode() + result = prime * result + packetGroup?.id.hashCode() + result = prime * result + tag?.id.hashCode() + return result + } } diff --git a/api/app/src/main/kotlin/packit/model/dto/UpdateRolePermission.kt b/api/app/src/main/kotlin/packit/model/dto/UpdateRolePermission.kt index 3ec26ff1..f81c5a5e 100644 --- a/api/app/src/main/kotlin/packit/model/dto/UpdateRolePermission.kt +++ b/api/app/src/main/kotlin/packit/model/dto/UpdateRolePermission.kt @@ -17,5 +17,4 @@ data class UpdateRolePermission( "Either all of packetId, tagId, packetGroupId should be null or only one of them should be not null" } } - -} \ No newline at end of file +} diff --git a/api/app/src/main/kotlin/packit/repository/RolePermissionRepository.kt b/api/app/src/main/kotlin/packit/repository/RolePermissionRepository.kt index 08d4d8ef..921e251a 100644 --- a/api/app/src/main/kotlin/packit/repository/RolePermissionRepository.kt +++ b/api/app/src/main/kotlin/packit/repository/RolePermissionRepository.kt @@ -14,4 +14,4 @@ interface RolePermissionRepository : JpaRepository @Transactional @Query("DELETE FROM RolePermission rp WHERE rp.id IN :ids") fun deleteAllByIdIn(ids: List) -} \ No newline at end of file +} diff --git a/api/app/src/main/kotlin/packit/service/RolePermissionService.kt b/api/app/src/main/kotlin/packit/service/RolePermissionService.kt index 0e95631b..06fd470b 100644 --- a/api/app/src/main/kotlin/packit/service/RolePermissionService.kt +++ b/api/app/src/main/kotlin/packit/service/RolePermissionService.kt @@ -8,7 +8,6 @@ import packit.model.RolePermission import packit.model.dto.UpdateRolePermission import packit.repository.* - interface RolePermissionService { fun getRolePermissionsToUpdate(role: Role, addRolePermissions: List): List @@ -50,7 +49,6 @@ class BaseRolePermissionService( } } - override fun removeRolePermissionsFromRole(role: Role, removeRolePermissions: List) { val rolePermissionsToRemove = getRolePermissionsToUpdate(role, removeRolePermissions) @@ -63,4 +61,4 @@ class BaseRolePermissionService( rolePermissionRepository.deleteAllByIdIn(matchedRolePermissionsToRemove.map { it.id!! }) } -} \ No newline at end of file +} diff --git a/api/app/src/main/kotlin/packit/service/RoleService.kt b/api/app/src/main/kotlin/packit/service/RoleService.kt index 6560f9af..60b25947 100644 --- a/api/app/src/main/kotlin/packit/service/RoleService.kt +++ b/api/app/src/main/kotlin/packit/service/RoleService.kt @@ -84,7 +84,6 @@ class BaseRoleService( roleRepository.save(role) } - override fun removePermissionsFromRole(roleName: String, removeRolePermissions: List) { val role = roleRepository.findByName(roleName) diff --git a/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt b/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt index 2ffd0a06..b1cd0c8a 100644 --- a/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt +++ b/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt @@ -8,7 +8,6 @@ import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotEquals - class RolePermissionTest { private val mockRoles = listOf( @@ -69,7 +68,6 @@ class RolePermissionTest assertNotEquals(rolePermission1, rolePermission2) } - @Test fun `equals returns false when packetGroup ids are different`() { @@ -80,7 +78,6 @@ class RolePermissionTest assertNotEquals(rolePermission1, rolePermission2) } - @Test fun `equals returns false when tag ids are different`() { @@ -91,4 +88,57 @@ class RolePermissionTest assertNotEquals(rolePermission1, rolePermission2) } -} \ No newline at end of file + @Test + fun `hashCode returns same hashcode for identical RolePermission instances`() + { + val rolePermission1 = RolePermission(mockRoles.first(), mockPermissions.first()) + val rolePermission2 = RolePermission(mockRoles.first(), mockPermissions.first()) + assertEquals(rolePermission1.hashCode(), rolePermission2.hashCode()) + } + + @Test + fun `hashCode returns different hash codes for RolePermission instances with different roles`() + { + val rolePermission1 = RolePermission(mockRoles.first(), mockPermissions.first()) + val rolePermission2 = RolePermission(mockRoles.last(), mockPermissions.first()) + assertNotEquals(rolePermission1.hashCode(), rolePermission2.hashCode()) + } + + @Test + fun `hashCode returns different hash codes for RolePermission instances with different permissions`() + { + val rolePermission1 = RolePermission(mockRoles.first(), mockPermissions.first()) + val rolePermission2 = RolePermission(mockRoles.first(), mockPermissions.last()) + assertNotEquals(rolePermission1.hashCode(), rolePermission2.hashCode()) + } + + @Test + fun `hashCode returns different hash codes for RolePermission instances with different packets`() + { + val rolePermission1 = + RolePermission(mockRoles.first(), mockPermissions.first(), mock { on { id } doReturn "2024111" }) + val rolePermission2 = + RolePermission(mockRoles.first(), mockPermissions.first(), mock { on { id } doReturn "2023344" }) + assertNotEquals(rolePermission1.hashCode(), rolePermission2.hashCode()) + } + + @Test + fun `hashCode returns different hash codes for RolePermission instances with different packetGroups`() + { + val rolePermission1 = + RolePermission(mockRoles.first(), mockPermissions.first(), null, mock { on { id } doReturn 1 }) + val rolePermission2 = + RolePermission(mockRoles.first(), mockPermissions.first(), null, mock { on { id } doReturn 2 }) + assertNotEquals(rolePermission1.hashCode(), rolePermission2.hashCode()) + } + + @Test + fun `hashCode returns different hash codes for RolePermission instances with different tags`() + { + val rolePermission1 = + RolePermission(mockRoles.first(), mockPermissions.first(), null, null, mock { on { id } doReturn 1 }) + val rolePermission2 = + RolePermission(mockRoles.first(), mockPermissions.first(), null, null, mock { on { id } doReturn 2 }) + assertNotEquals(rolePermission1.hashCode(), rolePermission2.hashCode()) + } +} diff --git a/api/app/src/test/kotlin/packit/unit/model/UpdateRolePermissionTest.kt b/api/app/src/test/kotlin/packit/unit/model/UpdateRolePermissionTest.kt index 2c46490b..479798f0 100644 --- a/api/app/src/test/kotlin/packit/unit/model/UpdateRolePermissionTest.kt +++ b/api/app/src/test/kotlin/packit/unit/model/UpdateRolePermissionTest.kt @@ -46,4 +46,4 @@ class UpdateRolePermissionTest UpdateRolePermission("permission", null, null, 1) } } -} \ No newline at end of file +} diff --git a/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt index b4fa418e..30f3b025 100644 --- a/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt @@ -179,5 +179,4 @@ class RolePermissionServiceTest assertEquals(HttpStatus.BAD_REQUEST, httpStatus) } } - -} \ No newline at end of file +} diff --git a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt index db4561b7..b318a7c0 100644 --- a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt @@ -289,10 +289,12 @@ class RoleServiceTest roleService.addPermissionsToRole(roleName, listOf()) - verify(roleRepository).save(argThat { + verify(roleRepository).save( + argThat { this == role this.rolePermissions.size == 2 - }) + } + ) } @Test From ce58d17bcc538709900e9b51b05d33002a10de4c Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Wed, 1 May 2024 14:33:29 +0100 Subject: [PATCH 05/25] Update RoleController endpoints --- .../src/main/kotlin/packit/controllers/RoleController.kt | 6 +++--- .../packit/integration/controllers/RoleControllerTest.kt | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/api/app/src/main/kotlin/packit/controllers/RoleController.kt b/api/app/src/main/kotlin/packit/controllers/RoleController.kt index 9a347627..739d35d3 100644 --- a/api/app/src/main/kotlin/packit/controllers/RoleController.kt +++ b/api/app/src/main/kotlin/packit/controllers/RoleController.kt @@ -22,7 +22,7 @@ class RoleController(private val roleService: RoleService) return ResponseEntity.ok(mapOf("message" to "Role created")) } - @DeleteMapping("/{name}") + @DeleteMapping("/{roleName}") fun deleteRole( @PathVariable roleName: String ): ResponseEntity> @@ -32,7 +32,7 @@ class RoleController(private val roleService: RoleService) return ResponseEntity.ok(mapOf("message" to "Role deleted")) } - @PostMapping("/add-permissions/{roleName}") + @PutMapping("/add-permissions/{roleName}") fun addPermissionsToRole( @RequestBody @Validated addRolePermissions: List, @PathVariable roleName: String @@ -43,7 +43,7 @@ class RoleController(private val roleService: RoleService) return ResponseEntity.ok(mapOf("message" to "Permissions added")) } - @PostMapping("/remove-permissions/{roleName}") + @PutMapping("/remove-permissions/{roleName}") fun removePermissionsFromRole( @RequestBody @Validated removeRolePermissions: List, @PathVariable roleName: String diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index 28a09367..3c78d954 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -104,8 +104,9 @@ class RoleControllerTest : IntegrationTest() { roleRepository.save(Role(name = "testRole")) - val result = restTemplate.postForEntity( + val result = restTemplate.exchange( "/role/add-permissions/testRole", + HttpMethod.PUT, getTokenizedHttpEntity(data = updateRolePermission), String::class.java ) @@ -131,8 +132,9 @@ class RoleControllerTest : IntegrationTest() ) roleRepository.save(baseRole) - val result = restTemplate.postForEntity( + val result = restTemplate.exchange( "/role/remove-permissions/testRole", + HttpMethod.PUT, getTokenizedHttpEntity(data = updateRolePermission), String::class.java ) From bd0145b42ad08df309c195caeff94a4a7e9e13f6 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Thu, 2 May 2024 07:26:17 +0100 Subject: [PATCH 06/25] Refactor variable name in RolePermissionService.kt --- .../src/main/kotlin/packit/service/RolePermissionService.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/app/src/main/kotlin/packit/service/RolePermissionService.kt b/api/app/src/main/kotlin/packit/service/RolePermissionService.kt index 06fd470b..d08cc3f2 100644 --- a/api/app/src/main/kotlin/packit/service/RolePermissionService.kt +++ b/api/app/src/main/kotlin/packit/service/RolePermissionService.kt @@ -53,8 +53,8 @@ class BaseRolePermissionService( { val rolePermissionsToRemove = getRolePermissionsToUpdate(role, removeRolePermissions) - val matchedRolePermissionsToRemove = rolePermissionsToRemove.map { rolePermissionsToRemove -> - val matchedPermission = role.rolePermissions.find { rolePermissionsToRemove == it } + val matchedRolePermissionsToRemove = rolePermissionsToRemove.map { rolePermissionToRemove -> + val matchedPermission = role.rolePermissions.find { rolePermissionToRemove == it } ?: throw PackitException("rolePermissionDoesNotExist", HttpStatus.BAD_REQUEST) matchedPermission } From 1095f2c2c4d83577d21d4155da2d90370705184f Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Thu, 2 May 2024 07:54:05 +0100 Subject: [PATCH 07/25] Add exception handler for IllegalArgumentException and add validation in RolePermission constructor --- .../exceptions/PackitExceptionHandler.kt | 7 +++++ .../kotlin/packit/model/RolePermission.kt | 8 ++++++ .../packit/unit/model/RolePermissionTest.kt | 26 +++++++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/api/app/src/main/kotlin/packit/exceptions/PackitExceptionHandler.kt b/api/app/src/main/kotlin/packit/exceptions/PackitExceptionHandler.kt index 07ebee20..ccb9c0b5 100644 --- a/api/app/src/main/kotlin/packit/exceptions/PackitExceptionHandler.kt +++ b/api/app/src/main/kotlin/packit/exceptions/PackitExceptionHandler.kt @@ -53,6 +53,13 @@ class PackitExceptionHandler .toResponseEntity() } + @ExceptionHandler(IllegalArgumentException::class) + fun handleIllegalArgumentException(e: Exception): ResponseEntity + { + return ErrorDetail(HttpStatus.BAD_REQUEST, e.message ?: "Invalid argument") + .toResponseEntity() + } + @ExceptionHandler(AccessDeniedException::class, AuthenticationException::class) fun handleAccessDenied(e: Exception): ResponseEntity { diff --git a/api/app/src/main/kotlin/packit/model/RolePermission.kt b/api/app/src/main/kotlin/packit/model/RolePermission.kt index 34c196af..d84530ef 100644 --- a/api/app/src/main/kotlin/packit/model/RolePermission.kt +++ b/api/app/src/main/kotlin/packit/model/RolePermission.kt @@ -30,6 +30,14 @@ class RolePermission( var id: Int? = null, ) { + init + { + val nonNullFields = listOf(packet, tag, packetGroup).count { it != null } + require(nonNullFields <= 1) { + "Either all of packet, tag, packetGroup should be null or only one of them should be not null" + } + } + override fun equals(other: Any?): Boolean { return when diff --git a/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt b/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt index b1cd0c8a..0d67bbc7 100644 --- a/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt +++ b/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt @@ -1,5 +1,7 @@ package packit.unit.model +import org.junit.jupiter.api.Assertions.assertDoesNotThrow +import org.junit.jupiter.api.assertThrows import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock import packit.model.* @@ -141,4 +143,28 @@ class RolePermissionTest RolePermission(mockRoles.first(), mockPermissions.first(), null, null, mock { on { id } doReturn 2 }) assertNotEquals(rolePermission1.hashCode(), rolePermission2.hashCode()) } + + @Test + fun `constructor throws when more than one scope field is non-null`() + { + assertThrows { + RolePermission(Role("r1"), Permission("p1", "d1"), mock(), mock()) + } + } + + @Test + fun `constructor does not throw when all scope fields are null`() + { + assertDoesNotThrow { + RolePermission(Role("r1"), Permission("p1", "d1")) + } + } + + @Test + fun `constructor does not throw when only single scope field is non-null`() + { + assertDoesNotThrow { + RolePermission(Role("r1"), Permission("p1", "d1"), mock()) + } + } } From 5cf418789ba83acf4f489c78e96e7f266ab11af8 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Thu, 2 May 2024 09:22:44 +0100 Subject: [PATCH 08/25] feat: get reads working --- .../packit/controllers/RoleController.kt | 22 +++++++++++++++++++ .../src/main/kotlin/packit/model/Packet.kt | 3 +++ .../main/kotlin/packit/model/PacketGroup.kt | 3 +++ api/app/src/main/kotlin/packit/model/Role.kt | 8 ++++++- .../kotlin/packit/model/RolePermission.kt | 9 ++++++++ api/app/src/main/kotlin/packit/model/Tag.kt | 3 +++ api/app/src/main/kotlin/packit/model/User.kt | 3 +++ .../kotlin/packit/model/dto/BasicPacketDto.kt | 6 +++++ .../kotlin/packit/model/dto/PacketGroupDto.kt | 4 ++++ .../main/kotlin/packit/model/dto/RoleDto.kt | 8 +++++++ .../packit/model/dto/RolePermissionDto.kt | 12 ++++++++++ .../main/kotlin/packit/model/dto/TagDto.kt | 3 +++ .../main/kotlin/packit/model/dto/UserDto.kt | 5 +++++ .../main/kotlin/packit/service/RoleService.kt | 19 ++++++++++++++++ 14 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 api/app/src/main/kotlin/packit/model/dto/BasicPacketDto.kt create mode 100644 api/app/src/main/kotlin/packit/model/dto/PacketGroupDto.kt create mode 100644 api/app/src/main/kotlin/packit/model/dto/RoleDto.kt create mode 100644 api/app/src/main/kotlin/packit/model/dto/RolePermissionDto.kt create mode 100644 api/app/src/main/kotlin/packit/model/dto/TagDto.kt create mode 100644 api/app/src/main/kotlin/packit/model/dto/UserDto.kt diff --git a/api/app/src/main/kotlin/packit/controllers/RoleController.kt b/api/app/src/main/kotlin/packit/controllers/RoleController.kt index 739d35d3..3d2d6d7f 100644 --- a/api/app/src/main/kotlin/packit/controllers/RoleController.kt +++ b/api/app/src/main/kotlin/packit/controllers/RoleController.kt @@ -6,7 +6,9 @@ import org.springframework.stereotype.Controller import org.springframework.validation.annotation.Validated import org.springframework.web.bind.annotation.* import packit.model.dto.CreateRole +import packit.model.dto.RoleDto import packit.model.dto.UpdateRolePermission +import packit.model.toDto import packit.service.RoleService @Controller @@ -53,4 +55,24 @@ class RoleController(private val roleService: RoleService) return ResponseEntity.ok(mapOf("message" to "Permissions removed")) } + + @GetMapping + fun getRoleNames(): ResponseEntity> + { + return ResponseEntity.ok(roleService.getRoleNames()) + } + + @GetMapping("/complete") + fun getRolesWithRelationships(): ResponseEntity> + { + val roles = roleService.getRolesWithRelationships() + return ResponseEntity.ok(roles.map { it.toDto() }) + } + + @GetMapping("/{roleName}") + fun getRole(@PathVariable roleName: String): ResponseEntity + { + val role = roleService.getRole(roleName) + return ResponseEntity.ok(role.toDto()) + } } diff --git a/api/app/src/main/kotlin/packit/model/Packet.kt b/api/app/src/main/kotlin/packit/model/Packet.kt index 4cae07d3..b4d101e2 100644 --- a/api/app/src/main/kotlin/packit/model/Packet.kt +++ b/api/app/src/main/kotlin/packit/model/Packet.kt @@ -3,6 +3,7 @@ package packit.model import jakarta.persistence.* import org.hibernate.annotations.JdbcTypeCode import org.hibernate.type.SqlTypes +import packit.model.dto.BasicPacketDto import packit.model.dto.PacketDto @Entity @@ -32,3 +33,5 @@ class Packet( fun Packet.toDto() = PacketDto( id, name, displayName, parameters, published, importTime, startTime, endTime ) + +fun Packet.toBasicDto() = BasicPacketDto(id, name) \ No newline at end of file diff --git a/api/app/src/main/kotlin/packit/model/PacketGroup.kt b/api/app/src/main/kotlin/packit/model/PacketGroup.kt index 48ba3349..f852699e 100644 --- a/api/app/src/main/kotlin/packit/model/PacketGroup.kt +++ b/api/app/src/main/kotlin/packit/model/PacketGroup.kt @@ -1,6 +1,7 @@ package packit.model import jakarta.persistence.* +import packit.model.dto.PacketGroupDto @Entity @Table(name = "packet_group") @@ -12,3 +13,5 @@ class PacketGroup( @GeneratedValue(strategy = GenerationType.IDENTITY) var id: Int? = null ) + +fun PacketGroup.toDto() = PacketGroupDto(name, id!!) diff --git a/api/app/src/main/kotlin/packit/model/Role.kt b/api/app/src/main/kotlin/packit/model/Role.kt index 790f4a34..a3d15450 100644 --- a/api/app/src/main/kotlin/packit/model/Role.kt +++ b/api/app/src/main/kotlin/packit/model/Role.kt @@ -1,6 +1,7 @@ package packit.model import jakarta.persistence.* +import packit.model.dto.RoleDto @Entity @Table(name = "`role`") @@ -8,9 +9,14 @@ class Role( var name: String, @OneToMany(mappedBy = "role", fetch = FetchType.EAGER, cascade = [CascadeType.ALL]) var rolePermissions: MutableList = mutableListOf(), - @ManyToMany(mappedBy = "roles", fetch = FetchType.LAZY) + @ManyToMany(mappedBy = "roles", fetch = FetchType.EAGER) var users: MutableList = mutableListOf(), @Id @GeneratedValue(strategy = GenerationType.IDENTITY) var id: Int? = null, ) + +fun Role.toDto() = + RoleDto( + name, rolePermissions.map { it.toDto() }, users.map { it.toDto() }, id!! + ) diff --git a/api/app/src/main/kotlin/packit/model/RolePermission.kt b/api/app/src/main/kotlin/packit/model/RolePermission.kt index d84530ef..d76f5fc5 100644 --- a/api/app/src/main/kotlin/packit/model/RolePermission.kt +++ b/api/app/src/main/kotlin/packit/model/RolePermission.kt @@ -1,6 +1,7 @@ package packit.model import jakarta.persistence.* +import packit.model.dto.RolePermissionDto @Entity @Table(name = "role_permission") @@ -64,3 +65,11 @@ class RolePermission( return result } } + +fun RolePermission.toDto() = RolePermissionDto( + permission.name, + packet?.toBasicDto(), + tag?.toDto(), + packetGroup?.toDto(), + id!! +) \ No newline at end of file diff --git a/api/app/src/main/kotlin/packit/model/Tag.kt b/api/app/src/main/kotlin/packit/model/Tag.kt index 9b1fd26b..15359dea 100644 --- a/api/app/src/main/kotlin/packit/model/Tag.kt +++ b/api/app/src/main/kotlin/packit/model/Tag.kt @@ -1,6 +1,7 @@ package packit.model import jakarta.persistence.* +import packit.model.dto.TagDto @Entity @Table(name = "tag") @@ -14,3 +15,5 @@ class Tag( @GeneratedValue(strategy = GenerationType.IDENTITY) val id: Int? = null, ) + +fun Tag.toDto() = TagDto(name, id!!) diff --git a/api/app/src/main/kotlin/packit/model/User.kt b/api/app/src/main/kotlin/packit/model/User.kt index e9f0ec27..716a3b00 100644 --- a/api/app/src/main/kotlin/packit/model/User.kt +++ b/api/app/src/main/kotlin/packit/model/User.kt @@ -1,6 +1,7 @@ package packit.model import jakarta.persistence.* +import packit.model.dto.UserDto import java.time.Instant import java.util.* @@ -25,3 +26,5 @@ class User( @GeneratedValue(strategy = GenerationType.UUID) val id: UUID? = null ) + +fun User.toDto() = UserDto(username, id!!) \ No newline at end of file diff --git a/api/app/src/main/kotlin/packit/model/dto/BasicPacketDto.kt b/api/app/src/main/kotlin/packit/model/dto/BasicPacketDto.kt new file mode 100644 index 00000000..a38b0f51 --- /dev/null +++ b/api/app/src/main/kotlin/packit/model/dto/BasicPacketDto.kt @@ -0,0 +1,6 @@ +package packit.model.dto + +data class BasicPacketDto( + val name: String, + val id: String, +) diff --git a/api/app/src/main/kotlin/packit/model/dto/PacketGroupDto.kt b/api/app/src/main/kotlin/packit/model/dto/PacketGroupDto.kt new file mode 100644 index 00000000..1de74c73 --- /dev/null +++ b/api/app/src/main/kotlin/packit/model/dto/PacketGroupDto.kt @@ -0,0 +1,4 @@ +package packit.model.dto + + +data class PacketGroupDto(val name: String, val id: Int) \ No newline at end of file diff --git a/api/app/src/main/kotlin/packit/model/dto/RoleDto.kt b/api/app/src/main/kotlin/packit/model/dto/RoleDto.kt new file mode 100644 index 00000000..95da0a5a --- /dev/null +++ b/api/app/src/main/kotlin/packit/model/dto/RoleDto.kt @@ -0,0 +1,8 @@ +package packit.model.dto + +data class RoleDto( + var name: String, + var rolePermissions: List = listOf(), + var usernames: List = listOf(), + var id: Int +) \ No newline at end of file diff --git a/api/app/src/main/kotlin/packit/model/dto/RolePermissionDto.kt b/api/app/src/main/kotlin/packit/model/dto/RolePermissionDto.kt new file mode 100644 index 00000000..56b29884 --- /dev/null +++ b/api/app/src/main/kotlin/packit/model/dto/RolePermissionDto.kt @@ -0,0 +1,12 @@ +package packit.model.dto + + +data class RolePermissionDto( + val permission: String, + val packet: BasicPacketDto? = null, + val tag: TagDto? = null, + val packetGroup: PacketGroupDto? = null, + val id: Int, +) + + diff --git a/api/app/src/main/kotlin/packit/model/dto/TagDto.kt b/api/app/src/main/kotlin/packit/model/dto/TagDto.kt new file mode 100644 index 00000000..0e97c8e0 --- /dev/null +++ b/api/app/src/main/kotlin/packit/model/dto/TagDto.kt @@ -0,0 +1,3 @@ +package packit.model.dto + +data class TagDto(val name: String, val id: Int) \ No newline at end of file diff --git a/api/app/src/main/kotlin/packit/model/dto/UserDto.kt b/api/app/src/main/kotlin/packit/model/dto/UserDto.kt new file mode 100644 index 00000000..ac3d020a --- /dev/null +++ b/api/app/src/main/kotlin/packit/model/dto/UserDto.kt @@ -0,0 +1,5 @@ +package packit.model.dto + +import java.util.* + +data class UserDto(val username: String, val id: UUID) diff --git a/api/app/src/main/kotlin/packit/service/RoleService.kt b/api/app/src/main/kotlin/packit/service/RoleService.kt index b9076dcd..b532415f 100644 --- a/api/app/src/main/kotlin/packit/service/RoleService.kt +++ b/api/app/src/main/kotlin/packit/service/RoleService.kt @@ -22,6 +22,9 @@ interface RoleService fun deleteRole(roleName: String) fun addPermissionsToRole(roleName: String, addRolePermissions: List) fun removePermissionsFromRole(roleName: String, removeRolePermissions: List) + fun getRoleNames(): List + fun getRolesWithRelationships(): List + fun getRole(roleName: String): Role } @Service @@ -91,6 +94,22 @@ class BaseRoleService( rolePermissionService.removeRolePermissionsFromRole(role, removeRolePermissions) } + override fun getRoleNames(): List + { + return roleRepository.findAll().map { it.name } + } + + override fun getRolesWithRelationships(): List + { + return roleRepository.findAll() + } + + override fun getRole(roleName: String): Role + { + return roleRepository.findByName(roleName) + ?: throw PackitException("roleNotFound", HttpStatus.BAD_REQUEST) + } + internal fun saveRole(roleName: String, permissions: List = listOf()) { if (roleRepository.existsByName(roleName)) From 98e30624cd19b0c0b9b3309983028f839f4d2484 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Thu, 2 May 2024 10:14:03 +0100 Subject: [PATCH 09/25] test: unit + integration tests --- .../src/main/kotlin/packit/model/Packet.kt | 2 +- .../main/kotlin/packit/model/dto/RoleDto.kt | 2 +- .../controllers/RoleControllerTest.kt | 53 +++++++++++++++++++ .../packit/unit/model/PacketGroupTest.kt | 19 +++++++ .../kotlin/packit/unit/model/PacketTest.kt | 44 +++++++++++++++ .../packit/unit/model/RolePermissionTest.kt | 15 ++++++ .../test/kotlin/packit/unit/model/RoleTest.kt | 29 ++++++++++ .../test/kotlin/packit/unit/model/TagTest.kt | 26 +++++++++ .../test/kotlin/packit/unit/model/UserTest.kt | 21 ++++++++ .../packit/unit/service/RoleServiceTest.kt | 42 +++++++++++++-- 10 files changed, 248 insertions(+), 5 deletions(-) create mode 100644 api/app/src/test/kotlin/packit/unit/model/PacketGroupTest.kt create mode 100644 api/app/src/test/kotlin/packit/unit/model/PacketTest.kt create mode 100644 api/app/src/test/kotlin/packit/unit/model/RoleTest.kt create mode 100644 api/app/src/test/kotlin/packit/unit/model/TagTest.kt create mode 100644 api/app/src/test/kotlin/packit/unit/model/UserTest.kt diff --git a/api/app/src/main/kotlin/packit/model/Packet.kt b/api/app/src/main/kotlin/packit/model/Packet.kt index b4d101e2..33b3132d 100644 --- a/api/app/src/main/kotlin/packit/model/Packet.kt +++ b/api/app/src/main/kotlin/packit/model/Packet.kt @@ -34,4 +34,4 @@ fun Packet.toDto() = PacketDto( id, name, displayName, parameters, published, importTime, startTime, endTime ) -fun Packet.toBasicDto() = BasicPacketDto(id, name) \ No newline at end of file +fun Packet.toBasicDto() = BasicPacketDto(name, id) \ No newline at end of file diff --git a/api/app/src/main/kotlin/packit/model/dto/RoleDto.kt b/api/app/src/main/kotlin/packit/model/dto/RoleDto.kt index 95da0a5a..c59f7fbb 100644 --- a/api/app/src/main/kotlin/packit/model/dto/RoleDto.kt +++ b/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 = listOf(), - var usernames: List = listOf(), + var users: List = listOf(), var id: Int ) \ No newline at end of file diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index 3c78d954..fd25f594 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -12,6 +12,7 @@ import packit.model.Role import packit.model.RolePermission import packit.model.dto.CreateRole import packit.model.dto.UpdateRolePermission +import packit.model.toDto import packit.repository.PermissionRepository import packit.repository.RoleRepository import kotlin.test.assertEquals @@ -143,4 +144,56 @@ class RoleControllerTest : IntegrationTest() val role = roleRepository.findByName("testRole")!! assertEquals(0, role.rolePermissions.size) } + + @Test + @WithAuthenticatedUser(authorities = ["user.manage"]) + fun `users with manage authority can get role names`() + { + roleRepository.save(Role(name = "testRole")) + + val result = restTemplate.exchange( + "/role", + HttpMethod.GET, + getTokenizedHttpEntity(), + String::class.java + ) + + assertSuccess(result) + + assertEquals(ObjectMapper().writeValueAsString(listOf("ADMIN", "testRole")), result.body) + } + + @Test + @WithAuthenticatedUser(authorities = ["user.manage"]) + fun `users can get roles with relationships`() + { + val roleDto = roleRepository.findByName("ADMIN")!!.toDto() + val result = restTemplate.exchange( + "/role/complete", + HttpMethod.GET, + getTokenizedHttpEntity(), + String::class.java + ) + + assertSuccess(result) + + assertEquals(ObjectMapper().writeValueAsString(listOf(roleDto)), result.body) + } + + @Test + @WithAuthenticatedUser(authorities = ["user.manage"]) + fun `users can get specific with relationships`() + { + val roleDto = roleRepository.findByName("ADMIN")!!.toDto() + val result = restTemplate.exchange( + "/role/ADMIN", + HttpMethod.GET, + getTokenizedHttpEntity(), + String::class.java + ) + + assertSuccess(result) + + assertEquals(ObjectMapper().writeValueAsString(roleDto), result.body) + } } diff --git a/api/app/src/test/kotlin/packit/unit/model/PacketGroupTest.kt b/api/app/src/test/kotlin/packit/unit/model/PacketGroupTest.kt new file mode 100644 index 00000000..85df1e07 --- /dev/null +++ b/api/app/src/test/kotlin/packit/unit/model/PacketGroupTest.kt @@ -0,0 +1,19 @@ +package packit.unit.model + +import packit.model.PacketGroup +import packit.model.toDto +import kotlin.test.Test +import kotlin.test.assertEquals + +class PacketGroupTest +{ + @Test + fun `toDto returns correct PacketGroupDto for given PacketGroup`() + { + val packetGroup = PacketGroup("group1") + packetGroup.id = 1 + val packetGroupDto = packetGroup.toDto() + assertEquals("group1", packetGroupDto.name) + assertEquals(1, packetGroupDto.id) + } +} \ No newline at end of file diff --git a/api/app/src/test/kotlin/packit/unit/model/PacketTest.kt b/api/app/src/test/kotlin/packit/unit/model/PacketTest.kt new file mode 100644 index 00000000..19aa6631 --- /dev/null +++ b/api/app/src/test/kotlin/packit/unit/model/PacketTest.kt @@ -0,0 +1,44 @@ +package packit.unit.model + +import packit.model.Packet +import packit.model.toBasicDto +import packit.model.toDto +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class PacketTest +{ + @Test + fun `toDto returns correct PacketDto for given Packet`() + { + val packet = Packet("id1", "name1", "displayName1", emptyMap(), true, 1.0, 2.0, 3.0) + val packetDto = packet.toDto() + assertEquals("id1", packetDto.id) + assertEquals("name1", packetDto.name) + assertEquals("displayName1", packetDto.displayName) + assertTrue(packetDto.parameters.isEmpty()) + assertTrue(packetDto.published) + assertEquals(1.0, packetDto.importTime) + assertEquals(2.0, packetDto.startTime) + assertEquals(3.0, packetDto.endTime) + } + + @Test + fun `toDto returns correct PacketDto for Packet with non-empty parameters`() + { + val parameters = mapOf("param1" to "value1") + val packet = Packet("id1", "name1", "displayName1", parameters, true, 1.0, 2.0, 3.0) + val packetDto = packet.toDto() + assertEquals(parameters, packetDto.parameters) + } + + @Test + fun `toBasicDto returns correct BasicPacketDto for given Packet`() + { + val packet = Packet("id1", "name1", "displayName1", emptyMap(), true, 1.0, 2.0, 3.0) + val basicPacketDto = packet.toBasicDto() + assertEquals("id1", basicPacketDto.id) + assertEquals("name1", basicPacketDto.name) + } +} \ No newline at end of file diff --git a/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt b/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt index 0d67bbc7..bb8c156a 100644 --- a/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt +++ b/api/app/src/test/kotlin/packit/unit/model/RolePermissionTest.kt @@ -167,4 +167,19 @@ class RolePermissionTest RolePermission(Role("r1"), Permission("p1", "d1"), mock()) } } + + @Test + fun `toDto returns correct RolePermissionDto for given RolePermission`() + { + val permission = Permission("permission1", "d1") + val tag = Tag("tag1", id = 1) + val rolePermission = RolePermission(Role("roleName"), permission, tag = tag, id = 1) + + val rolePermissionDto = rolePermission.toDto() + + assertEquals("permission1", rolePermissionDto.permission) + assertEquals("tag1", rolePermissionDto.tag!!.name) + assertEquals(1, rolePermissionDto.tag!!.id) + assertEquals(1, rolePermissionDto.id) + } } diff --git a/api/app/src/test/kotlin/packit/unit/model/RoleTest.kt b/api/app/src/test/kotlin/packit/unit/model/RoleTest.kt new file mode 100644 index 00000000..c3e5d620 --- /dev/null +++ b/api/app/src/test/kotlin/packit/unit/model/RoleTest.kt @@ -0,0 +1,29 @@ +package packit.unit.model + +import packit.model.* +import java.util.* +import kotlin.test.Test +import kotlin.test.assertEquals + +class RoleTest +{ + @Test + fun `toDto returns correct RoleDto for given Role`() + { + val user = User("user1", mutableListOf(), false, "source1", "displayName", id = UUID.randomUUID()) + val role = Role("role1", users = mutableListOf(user), id = 1) + val permission = Permission("permission1", "d1") + val tag = Tag("tag1", id = 1) + val rolePermission = RolePermission(role, permission, tag = tag, id = 1) + role.rolePermissions = mutableListOf(rolePermission) + + val roleDto = role.toDto() + + assertEquals("role1", roleDto.name) + assertEquals(1, roleDto.id) + assertEquals("permission1", roleDto.rolePermissions.first().permission) + assertEquals("user1", roleDto.users.first().username) + assertEquals("tag1", roleDto.rolePermissions.first().tag!!.name) + assertEquals(1, roleDto.rolePermissions.first().tag!!.id) + } +} \ No newline at end of file diff --git a/api/app/src/test/kotlin/packit/unit/model/TagTest.kt b/api/app/src/test/kotlin/packit/unit/model/TagTest.kt new file mode 100644 index 00000000..09a47298 --- /dev/null +++ b/api/app/src/test/kotlin/packit/unit/model/TagTest.kt @@ -0,0 +1,26 @@ +package packit.unit.model + +import org.junit.jupiter.api.assertThrows +import packit.model.Tag +import packit.model.toDto +import kotlin.test.Test +import kotlin.test.assertEquals + +class TagTest +{ + @Test + fun `toDto returns correct TagDto for given Tag`() + { + val tag = Tag("tag1", id = 1) + val tagDto = tag.toDto() + assertEquals("tag1", tagDto.name) + assertEquals(1, tagDto.id) + } + + @Test + fun `toDto throws NullPointerException for Tag with null id`() + { + val tag = Tag("tag1") + assertThrows { tag.toDto() } + } +} \ No newline at end of file diff --git a/api/app/src/test/kotlin/packit/unit/model/UserTest.kt b/api/app/src/test/kotlin/packit/unit/model/UserTest.kt new file mode 100644 index 00000000..76d8c405 --- /dev/null +++ b/api/app/src/test/kotlin/packit/unit/model/UserTest.kt @@ -0,0 +1,21 @@ +package packit.unit.model + +import packit.model.Role +import packit.model.User +import packit.model.toDto +import java.util.* +import kotlin.test.Test +import kotlin.test.assertEquals + +class UserTest +{ + @Test + fun `toDto returns correct UserDto for given User`() + { + val role = Role("role1") + val user = User("user1", mutableListOf(role), false, "source1", "displayName1", id = UUID.randomUUID()) + val userDto = user.toDto() + assertEquals("user1", userDto.username) + assertEquals(user.id, userDto.id) + } +} \ No newline at end of file diff --git a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt index b318a7c0..11e0353a 100644 --- a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt @@ -291,9 +291,9 @@ class RoleServiceTest verify(roleRepository).save( argThat { - this == role - this.rolePermissions.size == 2 - } + this == role + this.rolePermissions.size == 2 + } ) } @@ -323,6 +323,42 @@ class RoleServiceTest verify(rolePermissionService).removeRolePermissionsFromRole(role, listOf()) } + @Test + fun `getRoleNames returns role names`() + { + val roles = listOf(Role(name = "role1"), Role(name = "role2")) + whenever(roleRepository.findAll()).thenReturn(roles) + + val result = roleService.getRoleNames() + + assertEquals(2, result.size) + assertTrue(result.containsAll(listOf("role1", "role2"))) + } + + @Test + fun `getRolesWithRelationships returns all roles`() + { + val roles = listOf(Role(name = "role1"), Role(name = "role2")) + whenever(roleRepository.findAll()).thenReturn(roles) + + val result = roleService.getRolesWithRelationships() + + assertEquals(2, result.size) + assertTrue(result.containsAll(roles)) + } + + @Test + fun `getRole returns role by name`() + { + val roleName = "roleName" + val role = Role(name = roleName) + whenever(roleRepository.findByName(roleName)).thenReturn(role) + + val result = roleService.getRole(roleName) + + assertEquals(role, result) + } + private fun createRoleWithPermission( roleName: String, permissionName: String, From 27d5b0e17f05fd1c2b49b3536070b852348d9fa9 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Fri, 3 May 2024 09:11:29 +0100 Subject: [PATCH 10/25] Refactor model classes --- api/app/src/main/kotlin/packit/model/Packet.kt | 2 +- api/app/src/main/kotlin/packit/model/RolePermission.kt | 2 +- api/app/src/main/kotlin/packit/model/User.kt | 2 +- api/app/src/main/kotlin/packit/model/dto/PacketGroupDto.kt | 3 +-- api/app/src/main/kotlin/packit/model/dto/RoleDto.kt | 2 +- api/app/src/main/kotlin/packit/model/dto/RolePermissionDto.kt | 3 --- api/app/src/main/kotlin/packit/model/dto/TagDto.kt | 2 +- api/app/src/test/kotlin/packit/unit/model/PacketGroupTest.kt | 2 +- api/app/src/test/kotlin/packit/unit/model/PacketTest.kt | 2 +- api/app/src/test/kotlin/packit/unit/model/RoleTest.kt | 2 +- api/app/src/test/kotlin/packit/unit/model/TagTest.kt | 2 +- api/app/src/test/kotlin/packit/unit/model/UserTest.kt | 2 +- 12 files changed, 11 insertions(+), 15 deletions(-) diff --git a/api/app/src/main/kotlin/packit/model/Packet.kt b/api/app/src/main/kotlin/packit/model/Packet.kt index 33b3132d..ef9e6ff0 100644 --- a/api/app/src/main/kotlin/packit/model/Packet.kt +++ b/api/app/src/main/kotlin/packit/model/Packet.kt @@ -34,4 +34,4 @@ fun Packet.toDto() = PacketDto( id, name, displayName, parameters, published, importTime, startTime, endTime ) -fun Packet.toBasicDto() = BasicPacketDto(name, id) \ No newline at end of file +fun Packet.toBasicDto() = BasicPacketDto(name, id) diff --git a/api/app/src/main/kotlin/packit/model/RolePermission.kt b/api/app/src/main/kotlin/packit/model/RolePermission.kt index d76f5fc5..4824068f 100644 --- a/api/app/src/main/kotlin/packit/model/RolePermission.kt +++ b/api/app/src/main/kotlin/packit/model/RolePermission.kt @@ -72,4 +72,4 @@ fun RolePermission.toDto() = RolePermissionDto( tag?.toDto(), packetGroup?.toDto(), id!! -) \ No newline at end of file +) diff --git a/api/app/src/main/kotlin/packit/model/User.kt b/api/app/src/main/kotlin/packit/model/User.kt index 716a3b00..abc935a3 100644 --- a/api/app/src/main/kotlin/packit/model/User.kt +++ b/api/app/src/main/kotlin/packit/model/User.kt @@ -27,4 +27,4 @@ class User( val id: UUID? = null ) -fun User.toDto() = UserDto(username, id!!) \ No newline at end of file +fun User.toDto() = UserDto(username, id!!) diff --git a/api/app/src/main/kotlin/packit/model/dto/PacketGroupDto.kt b/api/app/src/main/kotlin/packit/model/dto/PacketGroupDto.kt index 1de74c73..a8f6c8c9 100644 --- a/api/app/src/main/kotlin/packit/model/dto/PacketGroupDto.kt +++ b/api/app/src/main/kotlin/packit/model/dto/PacketGroupDto.kt @@ -1,4 +1,3 @@ package packit.model.dto - -data class PacketGroupDto(val name: String, val id: Int) \ No newline at end of file +data class PacketGroupDto(val name: String, val id: Int) diff --git a/api/app/src/main/kotlin/packit/model/dto/RoleDto.kt b/api/app/src/main/kotlin/packit/model/dto/RoleDto.kt index c59f7fbb..09843780 100644 --- a/api/app/src/main/kotlin/packit/model/dto/RoleDto.kt +++ b/api/app/src/main/kotlin/packit/model/dto/RoleDto.kt @@ -5,4 +5,4 @@ data class RoleDto( var rolePermissions: List = listOf(), var users: List = listOf(), var id: Int -) \ No newline at end of file +) diff --git a/api/app/src/main/kotlin/packit/model/dto/RolePermissionDto.kt b/api/app/src/main/kotlin/packit/model/dto/RolePermissionDto.kt index 56b29884..1895695c 100644 --- a/api/app/src/main/kotlin/packit/model/dto/RolePermissionDto.kt +++ b/api/app/src/main/kotlin/packit/model/dto/RolePermissionDto.kt @@ -1,6 +1,5 @@ package packit.model.dto - data class RolePermissionDto( val permission: String, val packet: BasicPacketDto? = null, @@ -8,5 +7,3 @@ data class RolePermissionDto( val packetGroup: PacketGroupDto? = null, val id: Int, ) - - diff --git a/api/app/src/main/kotlin/packit/model/dto/TagDto.kt b/api/app/src/main/kotlin/packit/model/dto/TagDto.kt index 0e97c8e0..e359f932 100644 --- a/api/app/src/main/kotlin/packit/model/dto/TagDto.kt +++ b/api/app/src/main/kotlin/packit/model/dto/TagDto.kt @@ -1,3 +1,3 @@ package packit.model.dto -data class TagDto(val name: String, val id: Int) \ No newline at end of file +data class TagDto(val name: String, val id: Int) diff --git a/api/app/src/test/kotlin/packit/unit/model/PacketGroupTest.kt b/api/app/src/test/kotlin/packit/unit/model/PacketGroupTest.kt index 85df1e07..7ad05d10 100644 --- a/api/app/src/test/kotlin/packit/unit/model/PacketGroupTest.kt +++ b/api/app/src/test/kotlin/packit/unit/model/PacketGroupTest.kt @@ -16,4 +16,4 @@ class PacketGroupTest assertEquals("group1", packetGroupDto.name) assertEquals(1, packetGroupDto.id) } -} \ No newline at end of file +} diff --git a/api/app/src/test/kotlin/packit/unit/model/PacketTest.kt b/api/app/src/test/kotlin/packit/unit/model/PacketTest.kt index 19aa6631..af8e97b4 100644 --- a/api/app/src/test/kotlin/packit/unit/model/PacketTest.kt +++ b/api/app/src/test/kotlin/packit/unit/model/PacketTest.kt @@ -41,4 +41,4 @@ class PacketTest assertEquals("id1", basicPacketDto.id) assertEquals("name1", basicPacketDto.name) } -} \ No newline at end of file +} diff --git a/api/app/src/test/kotlin/packit/unit/model/RoleTest.kt b/api/app/src/test/kotlin/packit/unit/model/RoleTest.kt index c3e5d620..504f813b 100644 --- a/api/app/src/test/kotlin/packit/unit/model/RoleTest.kt +++ b/api/app/src/test/kotlin/packit/unit/model/RoleTest.kt @@ -26,4 +26,4 @@ class RoleTest assertEquals("tag1", roleDto.rolePermissions.first().tag!!.name) assertEquals(1, roleDto.rolePermissions.first().tag!!.id) } -} \ No newline at end of file +} diff --git a/api/app/src/test/kotlin/packit/unit/model/TagTest.kt b/api/app/src/test/kotlin/packit/unit/model/TagTest.kt index 09a47298..434300bc 100644 --- a/api/app/src/test/kotlin/packit/unit/model/TagTest.kt +++ b/api/app/src/test/kotlin/packit/unit/model/TagTest.kt @@ -23,4 +23,4 @@ class TagTest val tag = Tag("tag1") assertThrows { tag.toDto() } } -} \ No newline at end of file +} diff --git a/api/app/src/test/kotlin/packit/unit/model/UserTest.kt b/api/app/src/test/kotlin/packit/unit/model/UserTest.kt index 76d8c405..a5cd03d2 100644 --- a/api/app/src/test/kotlin/packit/unit/model/UserTest.kt +++ b/api/app/src/test/kotlin/packit/unit/model/UserTest.kt @@ -18,4 +18,4 @@ class UserTest assertEquals("user1", userDto.username) assertEquals(user.id, userDto.id) } -} \ No newline at end of file +} From bf920c5b91004b213cb4c9c35037afa4f4138fc9 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Fri, 3 May 2024 10:52:28 +0100 Subject: [PATCH 11/25] Add getUsernameRoles and getNonUsernameRoles endpoints --- .../packit/controllers/RoleController.kt | 14 ++++++++ .../packit/repository/RoleRepository.kt | 1 + .../main/kotlin/packit/service/RoleService.kt | 7 ++++ .../controllers/RoleControllerTest.kt | 35 +++++++++++++++++++ .../packit/unit/service/RoleServiceTest.kt | 14 +++++++- 5 files changed, 70 insertions(+), 1 deletion(-) diff --git a/api/app/src/main/kotlin/packit/controllers/RoleController.kt b/api/app/src/main/kotlin/packit/controllers/RoleController.kt index 3d2d6d7f..00e5f3bd 100644 --- a/api/app/src/main/kotlin/packit/controllers/RoleController.kt +++ b/api/app/src/main/kotlin/packit/controllers/RoleController.kt @@ -69,6 +69,20 @@ class RoleController(private val roleService: RoleService) return ResponseEntity.ok(roles.map { it.toDto() }) } + @GetMapping("/complete/usernames") + fun getUsernameRoles(): ResponseEntity> + { + val roles = roleService.getRolesWithRelationships(true) + return ResponseEntity.ok(roles.map { it.toDto() }) + } + + @GetMapping("/complete/non-usernames") + fun getNonUsernameRoles(): ResponseEntity> + { + val roles = roleService.getRolesWithRelationships(false) + return ResponseEntity.ok(roles.map { it.toDto() }) + } + @GetMapping("/{roleName}") fun getRole(@PathVariable roleName: String): ResponseEntity { diff --git a/api/app/src/main/kotlin/packit/repository/RoleRepository.kt b/api/app/src/main/kotlin/packit/repository/RoleRepository.kt index 39b2c9fb..ac14e290 100644 --- a/api/app/src/main/kotlin/packit/repository/RoleRepository.kt +++ b/api/app/src/main/kotlin/packit/repository/RoleRepository.kt @@ -11,6 +11,7 @@ interface RoleRepository : JpaRepository fun findByName(name: String): Role? fun existsByName(name: String): Boolean fun findByNameIn(names: List): List + fun findAllByIsUsername(isUsername: Boolean): List @Transactional fun deleteByName(name: String) diff --git a/api/app/src/main/kotlin/packit/service/RoleService.kt b/api/app/src/main/kotlin/packit/service/RoleService.kt index b4006e49..2e7b6a22 100644 --- a/api/app/src/main/kotlin/packit/service/RoleService.kt +++ b/api/app/src/main/kotlin/packit/service/RoleService.kt @@ -24,6 +24,8 @@ interface RoleService fun removePermissionsFromRole(roleName: String, removeRolePermissions: List) fun getRoleNames(): List fun getRolesWithRelationships(): List + fun getRolesWithRelationships(isUsernames: Boolean): List + fun getRole(roleName: String): Role } @@ -104,6 +106,11 @@ class BaseRoleService( return roleRepository.findAll() } + override fun getRolesWithRelationships(isUsernames: Boolean): List + { + return roleRepository.findAllByIsUsername(isUsernames) + } + override fun getRole(roleName: String): Role { return roleRepository.findByName(roleName) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index fd25f594..8a52f3df 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -180,6 +180,41 @@ class RoleControllerTest : IntegrationTest() assertEquals(ObjectMapper().writeValueAsString(listOf(roleDto)), result.body) } + @Test + @WithAuthenticatedUser(authorities = ["user.manage"]) + fun `users can get username roles with relationships `() + { + val roleDto = roleRepository.save(Role("randomUser", isUsername = true))!!.toDto() + val result = restTemplate.exchange( + "/role/complete/usernames", + HttpMethod.GET, + getTokenizedHttpEntity(), + String::class.java + ) + + assertSuccess(result) + + assertEquals(ObjectMapper().writeValueAsString(listOf(roleDto)), result.body) + } + + @Test + @WithAuthenticatedUser(authorities = ["user.manage"]) + fun `users can get non username roles with relationships `() + { + roleRepository.save(Role("randomUser", isUsername = true))!!.toDto() + val adminRole = roleRepository.findByName("ADMIN")!!.toDto() + val result = restTemplate.exchange( + "/role/complete/non-usernames", + HttpMethod.GET, + getTokenizedHttpEntity(), + String::class.java + ) + + assertSuccess(result) + + assertEquals(ObjectMapper().writeValueAsString(listOf(adminRole)), result.body) + } + @Test @WithAuthenticatedUser(authorities = ["user.manage"]) fun `users can get specific with relationships`() diff --git a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt index fd05c99e..4061f589 100644 --- a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt @@ -337,7 +337,7 @@ class RoleServiceTest } @Test - fun `getRolesWithRelationships returns all roles`() + fun `getRolesWithRelationships returns all roles when no isUsernamesflag set`() { val roles = listOf(Role(name = "role1"), Role(name = "role2")) whenever(roleRepository.findAll()).thenReturn(roles) @@ -348,6 +348,18 @@ class RoleServiceTest assertTrue(result.containsAll(roles)) } + @Test + fun `getRolesWithRelationships returns roles with isUsername flag`() + { + val roles = listOf(Role(name = "username1", isUsername = true), Role(name = "username2", isUsername = true)) + whenever(roleRepository.findAllByIsUsername(true)).thenReturn(roles) + + val result = roleService.getRolesWithRelationships(true) + + assertEquals(roles, result) + verify(roleRepository).findAllByIsUsername(true) + } + @Test fun `getRole returns role by name`() { From ecb871ea0973b06ced14c55ae7526c9c62d0199b Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Fri, 3 May 2024 11:52:53 +0100 Subject: [PATCH 12/25] Fix roleDto saving in RoleControllerTest --- .../packit/integration/controllers/RoleControllerTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index 8a52f3df..9d4b9190 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -184,7 +184,7 @@ class RoleControllerTest : IntegrationTest() @WithAuthenticatedUser(authorities = ["user.manage"]) fun `users can get username roles with relationships `() { - val roleDto = roleRepository.save(Role("randomUser", isUsername = true))!!.toDto() + val roleDto = roleRepository.save(Role("randomUser", isUsername = true)).toDto() val result = restTemplate.exchange( "/role/complete/usernames", HttpMethod.GET, @@ -201,7 +201,7 @@ class RoleControllerTest : IntegrationTest() @WithAuthenticatedUser(authorities = ["user.manage"]) fun `users can get non username roles with relationships `() { - roleRepository.save(Role("randomUser", isUsername = true))!!.toDto() + roleRepository.save(Role("randomUser", isUsername = true)).toDto() val adminRole = roleRepository.findByName("ADMIN")!!.toDto() val result = restTemplate.exchange( "/role/complete/non-usernames", From 1ce3d7d47d4b833ce0519850f4409a28567bbe54 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Fri, 3 May 2024 12:25:25 +0100 Subject: [PATCH 13/25] Refactor test case for getting non-username roles with relationships --- .../packit/integration/controllers/RoleControllerTest.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index 9d4b9190..e12167f9 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -199,7 +199,7 @@ class RoleControllerTest : IntegrationTest() @Test @WithAuthenticatedUser(authorities = ["user.manage"]) - fun `users can get non username roles with relationships `() + fun `users can get non username roles with relationships`() { roleRepository.save(Role("randomUser", isUsername = true)).toDto() val adminRole = roleRepository.findByName("ADMIN")!!.toDto() @@ -212,6 +212,8 @@ class RoleControllerTest : IntegrationTest() assertSuccess(result) + print(result.body) + assertEquals(ObjectMapper().writeValueAsString(listOf(adminRole)), result.body) } From 169ea1dd3aff6d77fbb6499ba9d7e37ff162de5a Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Fri, 3 May 2024 12:34:23 +0100 Subject: [PATCH 14/25] Refactor RoleControllerTest to improve readability and add assertions --- .../packit/integration/controllers/RoleControllerTest.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index e12167f9..5da395ef 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -193,8 +193,8 @@ class RoleControllerTest : IntegrationTest() ) assertSuccess(result) - - assertEquals(ObjectMapper().writeValueAsString(listOf(roleDto)), result.body) + print(result.body) + assert(result.body?.contains(ObjectMapper().writeValueAsString(listOf(roleDto))) ?: false) } @Test @@ -212,8 +212,6 @@ class RoleControllerTest : IntegrationTest() assertSuccess(result) - print(result.body) - assertEquals(ObjectMapper().writeValueAsString(listOf(adminRole)), result.body) } From 5765f229b8334b7e6210486202c181d97a0e1c66 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Fri, 3 May 2024 12:39:55 +0100 Subject: [PATCH 15/25] Refactor RoleControllerTest to use usernameRole instead of roleDto --- .../packit/integration/controllers/RoleControllerTest.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index 5da395ef..4ee5a466 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -184,7 +184,7 @@ class RoleControllerTest : IntegrationTest() @WithAuthenticatedUser(authorities = ["user.manage"]) fun `users can get username roles with relationships `() { - val roleDto = roleRepository.save(Role("randomUser", isUsername = true)).toDto() + val usernameRole = roleRepository.save(Role("username", isUsername = true)) val result = restTemplate.exchange( "/role/complete/usernames", HttpMethod.GET, @@ -193,8 +193,8 @@ class RoleControllerTest : IntegrationTest() ) assertSuccess(result) - print(result.body) - assert(result.body?.contains(ObjectMapper().writeValueAsString(listOf(roleDto))) ?: false) + + assertEquals(ObjectMapper().writeValueAsString(listOf(usernameRole.toDto())), result.body) } @Test From a4130333a99bc9ae25408c3fad997be324ed7864 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Fri, 3 May 2024 12:41:51 +0100 Subject: [PATCH 16/25] Refactor RoleControllerTest to improve username role retrieval --- .../packit/integration/controllers/RoleControllerTest.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index 4ee5a466..2a29f254 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -184,7 +184,8 @@ class RoleControllerTest : IntegrationTest() @WithAuthenticatedUser(authorities = ["user.manage"]) fun `users can get username roles with relationships `() { - val usernameRole = roleRepository.save(Role("username", isUsername = true)) + roleRepository.save(Role("username", isUsername = true)) + val allUsernameRoles = roleRepository.findAllByIsUsername(true).map { it.toDto() } val result = restTemplate.exchange( "/role/complete/usernames", HttpMethod.GET, @@ -194,7 +195,7 @@ class RoleControllerTest : IntegrationTest() assertSuccess(result) - assertEquals(ObjectMapper().writeValueAsString(listOf(usernameRole.toDto())), result.body) + assertEquals(ObjectMapper().writeValueAsString(allUsernameRoles), result.body) } @Test From c6d2bc567165988f78ed03c266de0fbd73d14915 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Tue, 7 May 2024 11:08:59 +0100 Subject: [PATCH 17/25] update as per pr coments --- .../packit/service/RolePermissionService.kt | 17 ++++++++ .../main/kotlin/packit/service/RoleService.kt | 6 +-- .../controllers/RoleControllerTest.kt | 40 +++++++++++++++++-- .../unit/service/RolePermissionServiceTest.kt | 32 +++++++++++++++ .../packit/unit/service/RoleServiceTest.kt | 31 +++----------- 5 files changed, 91 insertions(+), 35 deletions(-) diff --git a/api/app/src/main/kotlin/packit/service/RolePermissionService.kt b/api/app/src/main/kotlin/packit/service/RolePermissionService.kt index d08cc3f2..7b8e8219 100644 --- a/api/app/src/main/kotlin/packit/service/RolePermissionService.kt +++ b/api/app/src/main/kotlin/packit/service/RolePermissionService.kt @@ -12,6 +12,7 @@ interface RolePermissionService { fun getRolePermissionsToUpdate(role: Role, addRolePermissions: List): List fun removeRolePermissionsFromRole(role: Role, removeRolePermissions: List) + fun getAddRolePermissionsFromRole(role: Role, addRolePermissions: List): List } @Service @@ -31,6 +32,7 @@ class BaseRolePermissionService( return addRolePermissions.map { addRolePermission -> val permission = permissionRepository.findByName(addRolePermission.permission) ?: throw PackitException("permissionNotFound", HttpStatus.BAD_REQUEST) + RolePermission( role = role, permission = permission, @@ -56,9 +58,24 @@ class BaseRolePermissionService( val matchedRolePermissionsToRemove = rolePermissionsToRemove.map { rolePermissionToRemove -> val matchedPermission = role.rolePermissions.find { rolePermissionToRemove == it } ?: throw PackitException("rolePermissionDoesNotExist", HttpStatus.BAD_REQUEST) + matchedPermission } rolePermissionRepository.deleteAllByIdIn(matchedRolePermissionsToRemove.map { it.id!! }) } + + override fun getAddRolePermissionsFromRole( + role: Role, + addRolePermissions: List + ): List + { + val rolePermissionsToAdd = getRolePermissionsToUpdate(role, addRolePermissions) + if (rolePermissionsToAdd.any { role.rolePermissions.contains(it) }) + { + throw PackitException("rolePermissionAlreadyExists", HttpStatus.BAD_REQUEST) + } + + return rolePermissionsToAdd + } } diff --git a/api/app/src/main/kotlin/packit/service/RoleService.kt b/api/app/src/main/kotlin/packit/service/RoleService.kt index f71da09c..b6260af2 100644 --- a/api/app/src/main/kotlin/packit/service/RoleService.kt +++ b/api/app/src/main/kotlin/packit/service/RoleService.kt @@ -69,11 +69,7 @@ class BaseRoleService( val role = roleRepository.findByName(roleName) ?: throw PackitException("roleNotFound", HttpStatus.BAD_REQUEST) - val rolePermissionsToAdd = rolePermissionService.getRolePermissionsToUpdate(role, addRolePermissions) - if (rolePermissionsToAdd.any { role.rolePermissions.contains(it) }) - { - throw PackitException("rolePermissionAlreadyExists", HttpStatus.BAD_REQUEST) - } + val rolePermissionsToAdd = rolePermissionService.getAddRolePermissionsFromRole(role, addRolePermissions) role.rolePermissions.addAll(rolePermissionsToAdd) roleRepository.save(role) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index 7ea0eaf9..d1e0d1bc 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -98,6 +98,21 @@ class RoleControllerTest : IntegrationTest() assertNull(roleRepository.findByName("testRole")) } + @Test + @WithAuthenticatedUser(authorities = ["none"]) + fun `user without user manage permission cannot delete roles`() + { + roleRepository.save(Role(name = "testRole")) + + val result = restTemplate.postForEntity( + "/role/testRole", + getTokenizedHttpEntity(data = createTestRoleBody), + String::class.java + ) + + assertEquals(result.statusCode, HttpStatus.UNAUTHORIZED) + } + @Test @WithAuthenticatedUser(authorities = ["user.manage"]) fun `users with manage authority can add permissions to roles`() @@ -144,18 +159,35 @@ class RoleControllerTest : IntegrationTest() assertEquals(0, role.rolePermissions.size) } + @Test @WithAuthenticatedUser(authorities = ["none"]) - fun `user without user manage permission cannot delete roles`() + fun `user without user manage permission cannot add roles to permissions`() { roleRepository.save(Role(name = "testRole")) - val result = restTemplate.postForEntity( - "/role/testRole", - getTokenizedHttpEntity(data = createTestRoleBody), + val result = restTemplate.exchange( + "/role/add-permissions/testRole", + HttpMethod.PUT, + getTokenizedHttpEntity(data = updateRolePermission), String::class.java ) assertEquals(result.statusCode, HttpStatus.UNAUTHORIZED) + } + @Test + @WithAuthenticatedUser(authorities = ["none"]) + fun `user without user manage permission cannot remove roles from permissions`() + { + roleRepository.save(Role(name = "testRole")) + + val result = restTemplate.exchange( + "/role/remove-permissions/testRole", + HttpMethod.PUT, + getTokenizedHttpEntity(data = updateRolePermission), + String::class.java + ) + + assertEquals(result.statusCode, HttpStatus.UNAUTHORIZED) } } diff --git a/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt index 30f3b025..7cbe5b9d 100644 --- a/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt @@ -179,4 +179,36 @@ class RolePermissionServiceTest assertEquals(HttpStatus.BAD_REQUEST, httpStatus) } } + + @Test + fun `getAddRolePermissionsFromRole returns role permissions to add when they do not exist in role`() + { + val role = Role("role1") + val permission1 = Permission("permission1", "d1") + val addRolePermissions = listOf(UpdateRolePermission(permission1.name)) + whenever(permissionRepository.findByName(any())).thenReturn(permission1) + + val result = service.getAddRolePermissionsFromRole(role, addRolePermissions) + + assertEquals(1, result.size) + assertEquals(role, result[0].role) + assertEquals(permission1, result[0].permission) + } + + @Test + fun `getAddRolePermissionsFromRole throws exception when role permission already exists in role`() + { + val role = Role("role1") + val permission1 = Permission("permission1", "d1") + val addRolePermissions = listOf(UpdateRolePermission(permission1.name)) + role.rolePermissions = mutableListOf(RolePermission(role, permission1, id = 1)) + whenever(permissionRepository.findByName(any())).thenReturn(permission1) + + assertThrows { + service.getAddRolePermissionsFromRole(role, addRolePermissions) + }.apply { + assertEquals("rolePermissionAlreadyExists", key) + assertEquals(HttpStatus.BAD_REQUEST, httpStatus) + } + } } diff --git a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt index d878c1c4..d7965469 100644 --- a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt @@ -256,34 +256,13 @@ class RoleServiceTest } @Test - fun `addPermissionsToRole throws exception when role permission already exists`() + fun `addPermissionsToRole calls getAddRolePermissionsFromRole and saves role with added role permission`() { val roleName = "roleName" val permissionName = "permission1" val role = createRoleWithPermission(roleName, permissionName) whenever(roleRepository.findByName(roleName)).thenReturn(role) - whenever(rolePermissionService.getRolePermissionsToUpdate(role, listOf())).thenReturn( - listOf( - createRoleWithPermission(roleName, permissionName).rolePermissions.first() - ) - ) - - assertThrows { - roleService.addPermissionsToRole(roleName, listOf()) - }.apply { - assertEquals("rolePermissionAlreadyExists", key) - assertEquals(HttpStatus.BAD_REQUEST, httpStatus) - } - } - - @Test - fun `addPermissionsToRole calls getRolePermissionsToUpdate and saves role with added role permission`() - { - val roleName = "roleName" - val permissionName = "permission1" - val role = createRoleWithPermission(roleName, permissionName) - whenever(roleRepository.findByName(roleName)).thenReturn(role) - whenever(rolePermissionService.getRolePermissionsToUpdate(role, listOf())).thenReturn( + whenever(rolePermissionService.getAddRolePermissionsFromRole(role, listOf())).thenReturn( listOf( createRoleWithPermission(roleName, "differentPermission").rolePermissions.first() ) @@ -293,9 +272,9 @@ class RoleServiceTest verify(roleRepository).save( argThat { - this == role - this.rolePermissions.size == 2 - } + this == role + this.rolePermissions.size == 2 + } ) } From 27d0103d3c1302822a0fc66122286e07ceffe74b Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Tue, 7 May 2024 11:19:48 +0100 Subject: [PATCH 18/25] Refactor RoleControllerTest*** --- .../controllers/RoleControllerTest.kt | 60 ++++++++++++------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index 6dc5205a..128d0f59 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -1,9 +1,6 @@ package packit.integration.controllers import com.fasterxml.jackson.databind.ObjectMapper -import kotlin.test.assertEquals -import kotlin.test.assertNotNull -import kotlin.test.assertNull import org.junit.jupiter.api.Test import org.springframework.beans.factory.annotation.Autowired import org.springframework.http.HttpMethod @@ -18,6 +15,9 @@ import packit.model.dto.UpdateRolePermission import packit.model.toDto import packit.repository.PermissionRepository import packit.repository.RoleRepository +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull @Sql("/delete-test-users.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD) class RoleControllerTest : IntegrationTest() @@ -161,11 +161,45 @@ class RoleControllerTest : IntegrationTest() } @Test - @WithAuthenticatedUser(authorities = ["user.manage"]) + @WithAuthenticatedUser(authorities = ["none"]) + fun `user without user manage permission cannot remove roles from permissions`() + { + roleRepository.save(Role(name = "testRole")) + + val result = + restTemplate.exchange( + "/role/remove-permissions/testRole", + HttpMethod.PUT, + getTokenizedHttpEntity(data = updateRolePermission), + String::class.java + ) + + assertEquals(result.statusCode, HttpStatus.UNAUTHORIZED) + } + + @Test + @WithAuthenticatedUser(authorities = ["none"]) fun `user without user manage permission cannot add roles to permissions`() { roleRepository.save(Role(name = "testRole")) + val result = + restTemplate.exchange( + "/role/add-permissions/testRole", + HttpMethod.PUT, + getTokenizedHttpEntity(data = updateRolePermission), + String::class.java + ) + + assertEquals(result.statusCode, HttpStatus.UNAUTHORIZED) + } + + @Test + @WithAuthenticatedUser(authorities = ["user.manage"]) + fun `user with manage authority can read role names`() + { + roleRepository.save(Role(name = "testRole")) + val result = restTemplate.exchange( "/role", @@ -179,7 +213,7 @@ class RoleControllerTest : IntegrationTest() assertEquals(ObjectMapper().writeValueAsString(listOf("ADMIN", "testRole")), result.body) } - + @Test @WithAuthenticatedUser(authorities = ["user.manage"]) fun `users can get roles with relationships`() @@ -254,20 +288,4 @@ class RoleControllerTest : IntegrationTest() assertEquals(ObjectMapper().writeValueAsString(roleDto), result.body) } - @Test - @WithAuthenticatedUser(authorities = ["none"]) - fun `user without user manage permission cannot remove roles from permissions`() - { - roleRepository.save(Role(name = "testRole")) - - val result = - restTemplate.exchange( - "/role/remove-permissions/testRole", - HttpMethod.PUT, - getTokenizedHttpEntity(data = updateRolePermission), - String::class.java - ) - - assertEquals(result.statusCode, HttpStatus.UNAUTHORIZED) - } } From 163e5c0512cda897ee6672b7aaaa72ad3af5db11 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Tue, 7 May 2024 12:06:44 +0100 Subject: [PATCH 19/25] Refactor RoleController and RoleService --- .../packit/controllers/RoleController.kt | 22 ++++--------------- .../main/kotlin/packit/service/RoleService.kt | 14 +++++------- .../controllers/RoleControllerTest.kt | 8 +++---- .../packit/unit/service/RoleServiceTest.kt | 4 ++-- 4 files changed, 16 insertions(+), 32 deletions(-) diff --git a/api/app/src/main/kotlin/packit/controllers/RoleController.kt b/api/app/src/main/kotlin/packit/controllers/RoleController.kt index b3764445..468d7c83 100644 --- a/api/app/src/main/kotlin/packit/controllers/RoleController.kt +++ b/api/app/src/main/kotlin/packit/controllers/RoleController.kt @@ -56,30 +56,16 @@ class RoleController(private val roleService: RoleService) return ResponseEntity.ok(mapOf("message" to "Permissions removed")) } - @GetMapping + @GetMapping("/names") fun getRoleNames(): ResponseEntity> { return ResponseEntity.ok(roleService.getRoleNames()) } - @GetMapping("/complete") - fun getRolesWithRelationships(): ResponseEntity> - { - val roles = roleService.getRolesWithRelationships() - return ResponseEntity.ok(roles.map { it.toDto() }) - } - - @GetMapping("/complete/usernames") - fun getUsernameRoles(): ResponseEntity> - { - val roles = roleService.getRolesWithRelationships(true) - return ResponseEntity.ok(roles.map { it.toDto() }) - } - - @GetMapping("/complete/non-usernames") - fun getNonUsernameRoles(): ResponseEntity> + @GetMapping + fun getRolesWithRelationships(@RequestParam isUsername: Boolean?): ResponseEntity> { - val roles = roleService.getRolesWithRelationships(false) + val roles = roleService.getRoles(isUsername) return ResponseEntity.ok(roles.map { it.toDto() }) } diff --git a/api/app/src/main/kotlin/packit/service/RoleService.kt b/api/app/src/main/kotlin/packit/service/RoleService.kt index 737dcd3e..bcefc465 100644 --- a/api/app/src/main/kotlin/packit/service/RoleService.kt +++ b/api/app/src/main/kotlin/packit/service/RoleService.kt @@ -23,9 +23,7 @@ interface RoleService fun addPermissionsToRole(roleName: String, addRolePermissions: List) fun removePermissionsFromRole(roleName: String, removeRolePermissions: List) fun getRoleNames(): List - fun getRolesWithRelationships(): List - fun getRolesWithRelationships(isUsernames: Boolean): List - + fun getRoles(isUsernames: Boolean?): List fun getRole(roleName: String): Role } @@ -92,13 +90,13 @@ class BaseRoleService( return roleRepository.findAll().map { it.name } } - override fun getRolesWithRelationships(): List + override fun getRoles(isUsernames: Boolean?): List { - return roleRepository.findAll() - } + if (isUsernames == null) + { + return roleRepository.findAll() + } - override fun getRolesWithRelationships(isUsernames: Boolean): List - { return roleRepository.findAllByIsUsername(isUsernames) } diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index 128d0f59..79d03a5d 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -202,7 +202,7 @@ class RoleControllerTest : IntegrationTest() val result = restTemplate.exchange( - "/role", + "/role/names", HttpMethod.GET, getTokenizedHttpEntity(), String::class.java @@ -221,7 +221,7 @@ class RoleControllerTest : IntegrationTest() val roleDto = roleRepository.findByName("ADMIN")!!.toDto() val result = restTemplate.exchange( - "/role/complete", + "/role", HttpMethod.GET, getTokenizedHttpEntity(), String::class.java @@ -240,7 +240,7 @@ class RoleControllerTest : IntegrationTest() val allUsernameRoles = roleRepository.findAllByIsUsername(true).map { it.toDto() } val result = restTemplate.exchange( - "/role/complete/usernames", + "/role?isUsername=true", HttpMethod.GET, getTokenizedHttpEntity(), String::class.java @@ -259,7 +259,7 @@ class RoleControllerTest : IntegrationTest() val adminRole = roleRepository.findByName("ADMIN")!!.toDto() val result = restTemplate.exchange( - "/role/complete/non-usernames", + "/role?isUsername=false", HttpMethod.GET, getTokenizedHttpEntity(), String::class.java diff --git a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt index 8f81b6a4..6a69bc1f 100644 --- a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt @@ -322,7 +322,7 @@ class RoleServiceTest val roles = listOf(Role(name = "role1"), Role(name = "role2")) whenever(roleRepository.findAll()).thenReturn(roles) - val result = roleService.getRolesWithRelationships() + val result = roleService.getRoles(null) assertEquals(2, result.size) assertTrue(result.containsAll(roles)) @@ -334,7 +334,7 @@ class RoleServiceTest val roles = listOf(Role(name = "username1", isUsername = true), Role(name = "username2", isUsername = true)) whenever(roleRepository.findAllByIsUsername(true)).thenReturn(roles) - val result = roleService.getRolesWithRelationships(true) + val result = roleService.getRoles(true) assertEquals(roles, result) verify(roleRepository).findAllByIsUsername(true) From 30d168bfa40c6b07415bce13e07d215bc69848e2 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Tue, 7 May 2024 12:07:03 +0100 Subject: [PATCH 20/25] Remove empty lines in RoleControllerTest --- .../kotlin/packit/integration/controllers/RoleControllerTest.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index 79d03a5d..0611bd00 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -213,7 +213,6 @@ class RoleControllerTest : IntegrationTest() assertEquals(ObjectMapper().writeValueAsString(listOf("ADMIN", "testRole")), result.body) } - @Test @WithAuthenticatedUser(authorities = ["user.manage"]) fun `users can get roles with relationships`() @@ -287,5 +286,4 @@ class RoleControllerTest : IntegrationTest() assertEquals(ObjectMapper().writeValueAsString(roleDto), result.body) } - } From bc05370cc5a671bf377438cfa8dbcbfd50e642c7 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Wed, 8 May 2024 11:28:44 +0100 Subject: [PATCH 21/25] feat: make add/remove into update --- .../packit/controllers/RoleController.kt | 25 ++----- .../packit/model/dto/UpdateRolePermission.kt | 5 ++ .../packit/service/RolePermissionService.kt | 2 +- .../main/kotlin/packit/service/RoleService.kt | 23 ++++--- .../controllers/RoleControllerTest.kt | 66 ++++++------------- .../unit/service/RolePermissionServiceTest.kt | 2 +- .../packit/unit/service/RoleServiceTest.kt | 38 +++-------- 7 files changed, 55 insertions(+), 106 deletions(-) diff --git a/api/app/src/main/kotlin/packit/controllers/RoleController.kt b/api/app/src/main/kotlin/packit/controllers/RoleController.kt index 35905e73..dbaf2f55 100644 --- a/api/app/src/main/kotlin/packit/controllers/RoleController.kt +++ b/api/app/src/main/kotlin/packit/controllers/RoleController.kt @@ -6,7 +6,7 @@ import org.springframework.stereotype.Controller import org.springframework.validation.annotation.Validated import org.springframework.web.bind.annotation.* import packit.model.dto.CreateRole -import packit.model.dto.UpdateRolePermission +import packit.model.dto.UpdateRolePermissions import packit.service.RoleService @Controller @@ -32,25 +32,14 @@ class RoleController(private val roleService: RoleService) return ResponseEntity.noContent().build() } - @PutMapping("/add-permissions/{roleName}") - fun addPermissionsToRole( - @RequestBody @Validated addRolePermissions: List, + @PutMapping("/update-permissions/{roleName}") + fun updatePermissionsToRole( + @RequestBody @Validated updateRolePermissions: UpdateRolePermissions, @PathVariable roleName: String - ): ResponseEntity> + ): ResponseEntity { - roleService.addPermissionsToRole(roleName, addRolePermissions) + roleService.updatePermissionsToRole(roleName, updateRolePermissions) - return ResponseEntity.ok(mapOf("message" to "Permissions added")) - } - - @PutMapping("/remove-permissions/{roleName}") - fun removePermissionsFromRole( - @RequestBody @Validated removeRolePermissions: List, - @PathVariable roleName: String - ): ResponseEntity> - { - roleService.removePermissionsFromRole(roleName, removeRolePermissions) - - return ResponseEntity.ok(mapOf("message" to "Permissions removed")) + return ResponseEntity.noContent().build() } } diff --git a/api/app/src/main/kotlin/packit/model/dto/UpdateRolePermission.kt b/api/app/src/main/kotlin/packit/model/dto/UpdateRolePermission.kt index f81c5a5e..8fa5b2a3 100644 --- a/api/app/src/main/kotlin/packit/model/dto/UpdateRolePermission.kt +++ b/api/app/src/main/kotlin/packit/model/dto/UpdateRolePermission.kt @@ -2,6 +2,11 @@ package packit.model.dto import org.jetbrains.annotations.NotNull +data class UpdateRolePermissions( + val addPermissions: List = listOf(), + val removePermissions: List = listOf() +) + data class UpdateRolePermission( @field:NotNull val permission: String, diff --git a/api/app/src/main/kotlin/packit/service/RolePermissionService.kt b/api/app/src/main/kotlin/packit/service/RolePermissionService.kt index 7b8e8219..677ef3ec 100644 --- a/api/app/src/main/kotlin/packit/service/RolePermissionService.kt +++ b/api/app/src/main/kotlin/packit/service/RolePermissionService.kt @@ -31,7 +31,7 @@ class BaseRolePermissionService( { return addRolePermissions.map { addRolePermission -> val permission = permissionRepository.findByName(addRolePermission.permission) - ?: throw PackitException("permissionNotFound", HttpStatus.BAD_REQUEST) + ?: throw PackitException("invalidPermissionsProvided", HttpStatus.BAD_REQUEST) RolePermission( role = role, diff --git a/api/app/src/main/kotlin/packit/service/RoleService.kt b/api/app/src/main/kotlin/packit/service/RoleService.kt index b6260af2..60921e1d 100644 --- a/api/app/src/main/kotlin/packit/service/RoleService.kt +++ b/api/app/src/main/kotlin/packit/service/RoleService.kt @@ -10,6 +10,7 @@ import packit.model.Role import packit.model.RolePermission import packit.model.dto.CreateRole import packit.model.dto.UpdateRolePermission +import packit.model.dto.UpdateRolePermissions import packit.repository.RoleRepository interface RoleService @@ -20,8 +21,7 @@ interface RoleService fun getGrantedAuthorities(roles: List): MutableList fun createRole(createRole: CreateRole) fun deleteRole(roleName: String) - fun addPermissionsToRole(roleName: String, addRolePermissions: List) - fun removePermissionsFromRole(roleName: String, removeRolePermissions: List) + fun updatePermissionsToRole(roleName: String, updateRolePermissions: UpdateRolePermissions) } @Service @@ -64,22 +64,25 @@ class BaseRoleService( roleRepository.deleteByName(roleName) } - override fun addPermissionsToRole(roleName: String, addRolePermissions: List) + override fun updatePermissionsToRole(roleName: String, updateRolePermissions: UpdateRolePermissions) { val role = roleRepository.findByName(roleName) ?: throw PackitException("roleNotFound", HttpStatus.BAD_REQUEST) - val rolePermissionsToAdd = rolePermissionService.getAddRolePermissionsFromRole(role, addRolePermissions) + val roleAfterPermissionAdd = addRolePermissionsToRole(role, updateRolePermissions.addPermissions) - role.rolePermissions.addAll(rolePermissionsToAdd) - roleRepository.save(role) + rolePermissionService.removeRolePermissionsFromRole( + roleAfterPermissionAdd, + updateRolePermissions.removePermissions + ) } - override fun removePermissionsFromRole(roleName: String, removeRolePermissions: List) + internal fun addRolePermissionsToRole(role: Role, addRolePermissions: List): Role { - val role = roleRepository.findByName(roleName) - ?: throw PackitException("roleNotFound", HttpStatus.BAD_REQUEST) - rolePermissionService.removeRolePermissionsFromRole(role, removeRolePermissions) + val rolePermissionsToAdd = + rolePermissionService.getAddRolePermissionsFromRole(role, addRolePermissions) + role.rolePermissions.addAll(rolePermissionsToAdd) + return roleRepository.save(role) } internal fun saveRole(roleName: String, permissions: List = listOf()) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index d1e0d1bc..fbc5f001 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -12,6 +12,7 @@ import packit.model.Role import packit.model.RolePermission import packit.model.dto.CreateRole import packit.model.dto.UpdateRolePermission +import packit.model.dto.UpdateRolePermissions import packit.repository.PermissionRepository import packit.repository.RoleRepository import kotlin.test.assertEquals @@ -33,10 +34,17 @@ class RoleControllerTest : IntegrationTest() permissionNames = listOf("packet.run", "packet.read") ) ) - private val updateRolePermission = ObjectMapper().writeValueAsString( - listOf( - UpdateRolePermission( - permission = "packet.run" + private val updateRolePermissions = ObjectMapper().writeValueAsString( + UpdateRolePermissions( + addPermissions = listOf( + UpdateRolePermission( + permission = "packet.read" + ) + ), + removePermissions = listOf( + UpdateRolePermission( + permission = "packet.run" + ) ) ) ) @@ -115,26 +123,7 @@ class RoleControllerTest : IntegrationTest() @Test @WithAuthenticatedUser(authorities = ["user.manage"]) - fun `users with manage authority can add permissions to roles`() - { - roleRepository.save(Role(name = "testRole")) - - val result = restTemplate.exchange( - "/role/add-permissions/testRole", - HttpMethod.PUT, - getTokenizedHttpEntity(data = updateRolePermission), - String::class.java - ) - - assertSuccess(result) - val role = roleRepository.findByName("testRole")!! - assertEquals(1, role.rolePermissions.size) - assertEquals("packet.run", role.rolePermissions.first().permission.name) - } - - @Test - @WithAuthenticatedUser(authorities = ["user.manage"]) - fun `users with manage authority can remove permissions from roles`() + fun `users with manage authority can update role permissions`() { val roleName = "testRole" val baseRole = roleRepository.save(Role(name = roleName)) @@ -148,43 +137,28 @@ class RoleControllerTest : IntegrationTest() roleRepository.save(baseRole) val result = restTemplate.exchange( - "/role/remove-permissions/testRole", + "/role/update-permissions/testRole", HttpMethod.PUT, - getTokenizedHttpEntity(data = updateRolePermission), + getTokenizedHttpEntity(data = updateRolePermissions), String::class.java ) assertSuccess(result) val role = roleRepository.findByName("testRole")!! - assertEquals(0, role.rolePermissions.size) - } - - @Test - @WithAuthenticatedUser(authorities = ["none"]) - fun `user without user manage permission cannot add roles to permissions`() - { - roleRepository.save(Role(name = "testRole")) - - val result = restTemplate.exchange( - "/role/add-permissions/testRole", - HttpMethod.PUT, - getTokenizedHttpEntity(data = updateRolePermission), - String::class.java - ) - - assertEquals(result.statusCode, HttpStatus.UNAUTHORIZED) + assertEquals(1, role.rolePermissions.size) + assertEquals("packet.read", role.rolePermissions.first().permission.name) } @Test @WithAuthenticatedUser(authorities = ["none"]) - fun `user without user manage permission cannot remove roles from permissions`() + fun `user without user manage permission cannot update role permissions`() { roleRepository.save(Role(name = "testRole")) val result = restTemplate.exchange( - "/role/remove-permissions/testRole", + "/role/update-permissions/testRole", HttpMethod.PUT, - getTokenizedHttpEntity(data = updateRolePermission), + getTokenizedHttpEntity(data = updateRolePermissions), String::class.java ) diff --git a/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt index 7cbe5b9d..ed2c3568 100644 --- a/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt @@ -90,7 +90,7 @@ class RolePermissionServiceTest assertThrows { service.getRolePermissionsToUpdate(role, listOf(updateRolePermission)) }.apply { - assertEquals("permissionNotFound", key) + assertEquals("invalidPermissionsProvided", key) assertEquals(HttpStatus.BAD_REQUEST, httpStatus) } } diff --git a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt index d7965469..15166884 100644 --- a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt @@ -14,6 +14,7 @@ import packit.model.Permission import packit.model.Role import packit.model.RolePermission import packit.model.dto.CreateRole +import packit.model.dto.UpdateRolePermissions import packit.repository.RoleRepository import packit.service.BaseRoleService import packit.service.PermissionService @@ -242,33 +243,20 @@ class RoleServiceTest } @Test - fun `addPermissionsToRole throws exception when role does not exist`() - { - val roleName = "nonExistingRole" - whenever(roleRepository.findByName(roleName)).thenReturn(null) - - assertThrows { - roleService.addPermissionsToRole(roleName, listOf()) - }.apply { - assertEquals("roleNotFound", key) - assertEquals(HttpStatus.BAD_REQUEST, httpStatus) - } - } - - @Test - fun `addPermissionsToRole calls getAddRolePermissionsFromRole and saves role with added role permission`() + fun `updatePermissionsToRole calls correct methods and saves role`() { val roleName = "roleName" val permissionName = "permission1" val role = createRoleWithPermission(roleName, permissionName) whenever(roleRepository.findByName(roleName)).thenReturn(role) + whenever(roleRepository.save(any())).thenAnswer { it.getArgument(0) } whenever(rolePermissionService.getAddRolePermissionsFromRole(role, listOf())).thenReturn( listOf( createRoleWithPermission(roleName, "differentPermission").rolePermissions.first() ) ) - roleService.addPermissionsToRole(roleName, listOf()) + roleService.updatePermissionsToRole(roleName, UpdateRolePermissions()) verify(roleRepository).save( argThat { @@ -276,34 +264,24 @@ class RoleServiceTest this.rolePermissions.size == 2 } ) + verify(rolePermissionService).removeRolePermissionsFromRole(role, listOf()) + verify(rolePermissionService).getAddRolePermissionsFromRole(role, listOf()) } @Test - fun `removePermissionsFromRole throws exception when role does not exist`() + fun `updatePermissionsToRole throws exception when role does not exist`() { val roleName = "nonExistingRole" whenever(roleRepository.findByName(roleName)).thenReturn(null) assertThrows { - roleService.removePermissionsFromRole(roleName, listOf()) + roleService.updatePermissionsToRole(roleName, UpdateRolePermissions()) }.apply { assertEquals("roleNotFound", key) assertEquals(HttpStatus.BAD_REQUEST, httpStatus) } } - @Test - fun `removePermissionsFromRole calls removeRolePermissionsFromRole`() - { - val roleName = "roleName" - val role = Role(name = roleName) - whenever(roleRepository.findByName(roleName)).thenReturn(role) - - roleService.removePermissionsFromRole(roleName, listOf()) - - verify(rolePermissionService).removeRolePermissionsFromRole(role, listOf()) - } - private fun createRoleWithPermission( roleName: String, permissionName: String, From 1ea9487f32dcb6555998dcd8c67938142ffc567d Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Wed, 8 May 2024 11:39:03 +0100 Subject: [PATCH 22/25] Refactor imports in RoleControllerTest --- .../packit/integration/controllers/RoleControllerTest.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index 7bc6feff..ea7d9372 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -12,8 +12,8 @@ import packit.model.Role import packit.model.RolePermission import packit.model.dto.CreateRole import packit.model.dto.UpdateRolePermission -import packit.model.toDto import packit.model.dto.UpdateRolePermissions +import packit.model.toDto import packit.repository.PermissionRepository import packit.repository.RoleRepository import kotlin.test.assertEquals @@ -49,7 +49,7 @@ class RoleControllerTest : IntegrationTest() ) ) ) - + @Test @WithAuthenticatedUser(authorities = ["user.manage"]) fun `users with manage authority can create roles`() @@ -144,7 +144,7 @@ class RoleControllerTest : IntegrationTest() String::class.java ) - assertSuccess(result) + assertEquals(result.statusCode, HttpStatus.NO_CONTENT) val role = roleRepository.findByName("testRole")!! assertEquals(1, role.rolePermissions.size) assertEquals("packet.read", role.rolePermissions.first().permission.name) From 771eead6dde2e97c9cac716772d5d0ca16472ace Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Wed, 8 May 2024 11:40:09 +0100 Subject: [PATCH 23/25] Update assertSuccess to assertEquals for result status code --- .../kotlin/packit/integration/controllers/RoleControllerTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt index fbc5f001..d839b42f 100644 --- a/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt +++ b/api/app/src/test/kotlin/packit/integration/controllers/RoleControllerTest.kt @@ -143,7 +143,7 @@ class RoleControllerTest : IntegrationTest() String::class.java ) - assertSuccess(result) + assertEquals(result.statusCode, HttpStatus.NO_CONTENT) val role = roleRepository.findByName("testRole")!! assertEquals(1, role.rolePermissions.size) assertEquals("packet.read", role.rolePermissions.first().permission.name) From a98ea9e92ae754f352697b355edefb7960acc347 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Wed, 8 May 2024 11:42:02 +0100 Subject: [PATCH 24/25] Refactor imports in RoleController.kt --- api/app/src/main/kotlin/packit/controllers/RoleController.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/app/src/main/kotlin/packit/controllers/RoleController.kt b/api/app/src/main/kotlin/packit/controllers/RoleController.kt index dfb58b26..d4a91816 100644 --- a/api/app/src/main/kotlin/packit/controllers/RoleController.kt +++ b/api/app/src/main/kotlin/packit/controllers/RoleController.kt @@ -7,9 +7,8 @@ import org.springframework.validation.annotation.Validated import org.springframework.web.bind.annotation.* import packit.model.dto.CreateRole import packit.model.dto.RoleDto -import packit.model.dto.UpdateRolePermission -import packit.model.toDto import packit.model.dto.UpdateRolePermissions +import packit.model.toDto import packit.service.RoleService @Controller From 813099982c1b115d1dcc2eae286d97bf86ace945 Mon Sep 17 00:00:00 2001 From: anmol thapar Date: Mon, 13 May 2024 15:04:30 +0100 Subject: [PATCH 25/25] Refactor role permission service and role service --- .../kotlin/packit/service/RolePermissionService.kt | 11 +++++------ api/app/src/main/kotlin/packit/service/RoleService.kt | 2 +- .../packit/unit/service/RolePermissionServiceTest.kt | 4 ++-- .../kotlin/packit/unit/service/RoleServiceTest.kt | 4 ++-- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/api/app/src/main/kotlin/packit/service/RolePermissionService.kt b/api/app/src/main/kotlin/packit/service/RolePermissionService.kt index 677ef3ec..d1e08651 100644 --- a/api/app/src/main/kotlin/packit/service/RolePermissionService.kt +++ b/api/app/src/main/kotlin/packit/service/RolePermissionService.kt @@ -10,9 +10,8 @@ import packit.repository.* interface RolePermissionService { - fun getRolePermissionsToUpdate(role: Role, addRolePermissions: List): List fun removeRolePermissionsFromRole(role: Role, removeRolePermissions: List) - fun getAddRolePermissionsFromRole(role: Role, addRolePermissions: List): List + fun getRolePermissionsToAdd(role: Role, addRolePermissions: List): List } @Service @@ -24,12 +23,12 @@ class BaseRolePermissionService( private val rolePermissionRepository: RolePermissionRepository ) : RolePermissionService { - override fun getRolePermissionsToUpdate( + internal fun getRolePermissionsToUpdate( role: Role, - addRolePermissions: List + updateRolePermissions: List ): List { - return addRolePermissions.map { addRolePermission -> + return updateRolePermissions.map { addRolePermission -> val permission = permissionRepository.findByName(addRolePermission.permission) ?: throw PackitException("invalidPermissionsProvided", HttpStatus.BAD_REQUEST) @@ -65,7 +64,7 @@ class BaseRolePermissionService( rolePermissionRepository.deleteAllByIdIn(matchedRolePermissionsToRemove.map { it.id!! }) } - override fun getAddRolePermissionsFromRole( + override fun getRolePermissionsToAdd( role: Role, addRolePermissions: List ): List diff --git a/api/app/src/main/kotlin/packit/service/RoleService.kt b/api/app/src/main/kotlin/packit/service/RoleService.kt index 60921e1d..2c4e47e2 100644 --- a/api/app/src/main/kotlin/packit/service/RoleService.kt +++ b/api/app/src/main/kotlin/packit/service/RoleService.kt @@ -80,7 +80,7 @@ class BaseRoleService( internal fun addRolePermissionsToRole(role: Role, addRolePermissions: List): Role { val rolePermissionsToAdd = - rolePermissionService.getAddRolePermissionsFromRole(role, addRolePermissions) + rolePermissionService.getRolePermissionsToAdd(role, addRolePermissions) role.rolePermissions.addAll(rolePermissionsToAdd) return roleRepository.save(role) } diff --git a/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt index ed2c3568..e24fe454 100644 --- a/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/RolePermissionServiceTest.kt @@ -188,7 +188,7 @@ class RolePermissionServiceTest val addRolePermissions = listOf(UpdateRolePermission(permission1.name)) whenever(permissionRepository.findByName(any())).thenReturn(permission1) - val result = service.getAddRolePermissionsFromRole(role, addRolePermissions) + val result = service.getRolePermissionsToAdd(role, addRolePermissions) assertEquals(1, result.size) assertEquals(role, result[0].role) @@ -205,7 +205,7 @@ class RolePermissionServiceTest whenever(permissionRepository.findByName(any())).thenReturn(permission1) assertThrows { - service.getAddRolePermissionsFromRole(role, addRolePermissions) + service.getRolePermissionsToAdd(role, addRolePermissions) }.apply { assertEquals("rolePermissionAlreadyExists", key) assertEquals(HttpStatus.BAD_REQUEST, httpStatus) diff --git a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt index 15166884..d03d76f6 100644 --- a/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt +++ b/api/app/src/test/kotlin/packit/unit/service/RoleServiceTest.kt @@ -250,7 +250,7 @@ class RoleServiceTest val role = createRoleWithPermission(roleName, permissionName) whenever(roleRepository.findByName(roleName)).thenReturn(role) whenever(roleRepository.save(any())).thenAnswer { it.getArgument(0) } - whenever(rolePermissionService.getAddRolePermissionsFromRole(role, listOf())).thenReturn( + whenever(rolePermissionService.getRolePermissionsToAdd(role, listOf())).thenReturn( listOf( createRoleWithPermission(roleName, "differentPermission").rolePermissions.first() ) @@ -265,7 +265,7 @@ class RoleServiceTest } ) verify(rolePermissionService).removeRolePermissionsFromRole(role, listOf()) - verify(rolePermissionService).getAddRolePermissionsFromRole(role, listOf()) + verify(rolePermissionService).getRolePermissionsToAdd(role, listOf()) } @Test