ColorRecipesExplorer
/
Backend
1
1
Fork 0
Code Issues 7 Pull Requests Projects 1 Releases 2 Wiki Activity

#30 Random group token UUIDs to prevent critical security problem.
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is passing Details

This commit is contained in:
william 2022-07-22 08:46:16 -04:00
parent 69c5d86d45
commit b235b5a845
7 changed files with 109 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/main/kotlin/dev/fyloz/colorrecipesexplorer/config/security/filters/JwtAuthenticationFilter.kt
View File

@ -18,7 +18,7 @@ import javax.servlet.http.HttpServletResponse
abstract class JwtAuthenticationFilter(
filterProcessesUrl: String,
private val securityProperties: CreSecurityProperties,
protected val jwtLogic: JwtLogic
private val jwtLogic: JwtLogic
) :
AbstractAuthenticationProcessingFilter(
AntPathRequestMatcher(filterProcessesUrl, HttpMethod.POST.toString())
@ -73,4 +73,4 @@ abstract class JwtAuthenticationFilter(
private const val AUTHORIZATION_COOKIE_SAME_SITE = true
private const val AUTHORIZATION_COOKIE_PATH = Constants.ControllerPaths.BASE_PATH
}
}
}

2
src/main/kotlin/dev/fyloz/colorrecipesexplorer/dtos/account/GroupTokenDto.kt
View File

@ -10,6 +10,8 @@ data class GroupTokenDto(
val enabled: Boolean,
val isDeleted: Boolean,
val group: GroupDto
)

26
src/main/kotlin/dev/fyloz/colorrecipesexplorer/logic/account/GroupTokenLogic.kt
View File

@ -32,8 +32,7 @@ class DefaultGroupTokenLogic(
private val groupLogic: GroupLogic,
private val jwtLogic: JwtLogic,
private val enabledTokensCache: HashSet<String> = hashSetOf() // In constructor for unit testing
) :
GroupTokenLogic {
) : GroupTokenLogic {
private val typeName = Constants.ModelNames.GROUP_TOKEN
private val typeNameLowerCase = typeName.lowercase()
@ -49,8 +48,7 @@ class DefaultGroupTokenLogic(
override fun getById(id: UUID) = service.getById(id) ?: throw notFoundException(value = id)
override fun getIdForRequest(request: HttpServletRequest): UUID? {
val groupTokenCookie = getGroupTokenCookie(request)
?: return null
val groupTokenCookie = getGroupTokenCookie(request) ?: return null
val jwt = parseBearer(groupTokenCookie.value)
return jwtLogic.parseGroupTokenIdJwt(jwt)
@ -59,10 +57,9 @@ class DefaultGroupTokenLogic(
override fun save(dto: GroupTokenSaveDto): GroupTokenDto {
throwIfNameAlreadyExists(dto.name)
// We don't need to check for collision, because UUIDs with different names will be different
val id = generateUUIDForName(dto.name)
val id = generateRandomUUID()
val token = GroupTokenDto(
id, dto.name, true, groupLogic.getById(dto.groupId)
id, dto.name, enabled = true, isDeleted = false, group = groupLogic.getById(dto.groupId)
)
val savedToken = service.save(token)
@ -84,15 +81,26 @@ class DefaultGroupTokenLogic(
}
override fun deleteById(id: String) {
val token = getById(id).copy(enabled = false, isDeleted = true)
service.save(token)
enabledTokensCache.remove(id)
service.deleteById(UUID.fromString(id))
}
private fun setEnabled(id: String, enabled: Boolean) = with(getById(id)) {
service.save(this.copy(enabled = enabled))
}
private fun generateUUIDForName(name: String) = UUID.nameUUIDFromBytes(name.toByteArray())
private fun generateRandomUUID(): UUID {
var uuid = UUID.randomUUID()
// The UUID specification doesn't guarantee to prevent collisions
while (service.existsById(uuid)) {
uuid = UUID.randomUUID()
}
return uuid
}
private fun throwIfNameAlreadyExists(name: String) {
if (service.existsByName(name)) {

3
src/main/kotlin/dev/fyloz/colorrecipesexplorer/model/account/GroupToken.kt
View File

@ -20,6 +20,9 @@ data class GroupToken(
@Column(name = "is_valid")
val isValid: Boolean,
@Column(name = "deleted")
val isDeleted: Boolean,
@ManyToOne
@JoinColumn(name = "group_id")
val group: Group

10
src/main/kotlin/dev/fyloz/colorrecipesexplorer/repository/AccountRepository.kt
View File

@ -39,6 +39,12 @@ interface GroupRepository : JpaRepository<Group, Long> {
@Repository
interface GroupTokenRepository : JpaRepository<GroupToken, UUID> {
/** Checks if a token with the given [name] exists. */
fun existsByName(name: String): Boolean
/** Checks if a token that is not deleted with the given [name] exists. */
fun existsByNameAndIsDeletedIsFalse(name: String): Boolean
/** Finds all group tokens that are not deleted. */
fun findAllByIsDeletedIsFalse(): Collection<GroupToken>
/** Finds the group token with the given [id] if it is not deleted. */
fun findByIdAndIsDeletedIsFalse(id: UUID): GroupToken?
}

11
src/main/kotlin/dev/fyloz/colorrecipesexplorer/service/account/GroupTokenService.kt
View File

@ -4,7 +4,6 @@ import dev.fyloz.colorrecipesexplorer.config.annotations.ServiceComponent
import dev.fyloz.colorrecipesexplorer.dtos.account.GroupTokenDto
import dev.fyloz.colorrecipesexplorer.model.account.GroupToken
import dev.fyloz.colorrecipesexplorer.repository.GroupTokenRepository
import org.springframework.data.repository.findByIdOrNull
import java.util.UUID
interface GroupTokenService {
@ -23,12 +22,12 @@ interface GroupTokenService {
class DefaultGroupTokenService(private val repository: GroupTokenRepository, private val groupService: GroupService) :
GroupTokenService {
override fun existsById(id: UUID) = repository.existsById(id)
override fun existsByName(name: String) = repository.existsByName(name)
override fun existsByName(name: String) = repository.existsByNameAndIsDeletedIsFalse(name)
override fun getAll() = repository.findAll().map(::toDto)
override fun getAll() = repository.findAllByIsDeletedIsFalse().map(::toDto)
override fun getById(id: UUID): GroupTokenDto? {
val entity = repository.findByIdOrNull(id)
val entity = repository.findByIdAndIsDeletedIsFalse(id)
return if (entity != null) toDto(entity) else null
}
@ -41,8 +40,8 @@ class DefaultGroupTokenService(private val repository: GroupTokenRepository, pri
override fun deleteById(id: UUID) = repository.deleteById(id)
override fun toDto(entity: GroupToken) =
GroupTokenDto(entity.id, entity.name, entity.isValid, groupService.toDto(entity.group))
GroupTokenDto(entity.id, entity.name, entity.isValid, entity.isDeleted, groupService.toDto(entity.group))
override fun toEntity(dto: GroupTokenDto) =
GroupToken(dto.id, dto.name, dto.enabled, groupService.toEntity(dto.group))
GroupToken(dto.id, dto.name, dto.enabled, dto.isDeleted, groupService.toEntity(dto.group))
}

80
src/test/kotlin/dev/fyloz/colorrecipesexplorer/logic/account/DefaultGroupTokenLogicTest.kt
View File

@ -1,5 +1,6 @@
package dev.fyloz.colorrecipesexplorer.logic.account
import dev.fyloz.colorrecipesexplorer.Constants
import dev.fyloz.colorrecipesexplorer.dtos.account.GroupDto
import dev.fyloz.colorrecipesexplorer.dtos.account.GroupTokenDto
import dev.fyloz.colorrecipesexplorer.dtos.account.GroupTokenSaveDto
@ -10,22 +11,27 @@ import io.mockk.*
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows
import org.springframework.web.util.WebUtils
import java.util.*
import javax.servlet.http.Cookie
import javax.servlet.http.HttpServletRequest
import kotlin.test.*
class DefaultGroupTokenLogicTest {
private val groupTokenServiceMock = mockk<GroupTokenService>()
private val groupLogicMock = mockk<GroupLogic>()
private val jwtLogicMock = mockk<JwtLogic>()
private val enabledTokenCache = hashSetOf<String>()
private val groupTokenLogic = spyk(DefaultGroupTokenLogic(groupTokenServiceMock, groupLogicMock, enabledTokenCache))
private val groupTokenLogic =
spyk(DefaultGroupTokenLogic(groupTokenServiceMock, groupLogicMock, jwtLogicMock, enabledTokenCache))
private val groupTokenName = "Unit test token"
private val groupTokenId = UUID.nameUUIDFromBytes(groupTokenName.toByteArray())
private val groupTokenIdStr = groupTokenId.toString()
private val group = GroupDto(1L, "Unit test group", listOf(), listOf())
private val groupToken = GroupTokenDto(groupTokenId, groupTokenName, true, group)
private val groupToken = GroupTokenDto(groupTokenId, groupTokenName, true, false, group)
private val groupTokenSaveDto = GroupTokenSaveDto(groupTokenName, group.id)
@AfterEach
@ -106,15 +112,31 @@ class DefaultGroupTokenLogicTest {
assertThrows<NotFoundException> { groupTokenLogic.getById(groupTokenId) }
}
@Test
fun getIdForRequest_normalBehavior_returnsGroupTokenIdFromRequest() {
// Arrange
val request = mockk<HttpServletRequest>()
val cookie = mockk<Cookie> {
every { value } returns "Bearer$groupTokenIdStr"
}
mockkStatic(WebUtils::class) {
every { WebUtils.getCookie(any(), Constants.CookieNames.GROUP_TOKEN) } returns cookie
}
}
@Test
fun save_normalBehavior_callsSaveInService() {
// Arrange
every { groupTokenServiceMock.existsByName(any()) } returns false
every { groupTokenServiceMock.existsById(any()) } returns false
every { groupTokenServiceMock.save(any()) } returns groupToken
every { groupLogicMock.getById(any()) } returns group
// Act
groupTokenLogic.save(groupTokenSaveDto)
withMockRandomUUID {
groupTokenLogic.save(groupTokenSaveDto)
}
// Assert
verify {
@ -122,15 +144,41 @@ class DefaultGroupTokenLogicTest {
}
}
@Test
fun save_idAlreadyExists_generatesNewId() {
// Arrange
every { groupTokenServiceMock.existsByName(any()) } returns false
every { groupTokenServiceMock.existsById(any()) } returnsMany listOf(true, false)
every { groupTokenServiceMock.save(any()) } returns groupToken
every { groupLogicMock.getById(any()) } returns group
val anotherGroupTokenId = UUID.nameUUIDFromBytes("Another unit test token".toByteArray())
// Act
withMockRandomUUID(listOf(groupTokenId, anotherGroupTokenId)) {
groupTokenLogic.save(groupTokenSaveDto)
}
// Assert
verify {
groupTokenServiceMock.save(match {
it.id == anotherGroupTokenId
})
}
}
@Test
fun save_normalBehavior_addsIdToEnabledTokensCache() {
// Arrange
every { groupTokenServiceMock.existsByName(any()) } returns false
every { groupTokenServiceMock.existsById(any()) } returns false
every { groupTokenServiceMock.save(any()) } returns groupToken
every { groupLogicMock.getById(any()) } returns group
// Act
groupTokenLogic.save(groupTokenSaveDto)
withMockRandomUUID {
groupTokenLogic.save(groupTokenSaveDto)
}
// Assert
assertContains(enabledTokenCache, groupTokenIdStr)
@ -207,23 +255,27 @@ class DefaultGroupTokenLogicTest {
}
@Test
fun deleteById_normalBehavior_callsService() {
fun deleteById_normalBehavior_savesDeletedTokenInService() {
// Arrange
every { groupTokenServiceMock.deleteById(any()) } just runs
every { groupTokenLogic.getById(any<String>()) } answers { groupToken }
every { groupTokenServiceMock.save(any()) } answers { firstArg() }
// Act
groupTokenLogic.deleteById(groupTokenIdStr)
// Assert
verify {
groupTokenServiceMock.deleteById(groupTokenId)
groupTokenServiceMock.save(match {
it.id == groupTokenId && it.isDeleted
})
}
}
@Test
fun deleteById_normalBehavior_removesIdFromEnabledTokensCache() {
// Arrange
every { groupTokenServiceMock.deleteById(any()) } just runs
every { groupTokenLogic.getById(any<String>()) } answers { groupToken }
every { groupTokenServiceMock.save(any()) } answers { firstArg() }
// Act
groupTokenLogic.deleteById(groupTokenIdStr)
@ -231,4 +283,16 @@ class DefaultGroupTokenLogicTest {
// Assert
assertFalse(enabledTokenCache.contains(groupTokenIdStr))
}
private fun withMockRandomUUID(uuids: List<UUID>? = null, block: () -> Unit) {
mockkStatic(UUID::class) {
if (uuids == null) {
every { UUID.randomUUID() } returns groupTokenId
} else {
every { UUID.randomUUID() } returnsMany uuids
}
block()
}
}
}