From 821338b9771bf7b36af1b6286627b2213a1bc939 Mon Sep 17 00:00:00 2001 From: Topias Heinonen Date: Fri, 1 Sep 2023 12:58:20 +0300 Subject: [PATCH] HAI-1527 Audit logging for permission updates (#393) --- .../permissions/HankeKayttajaServiceITest.kt | 65 ++++++++ .../haitaton/hanke/logging/AuditLogEntry.kt | 8 +- .../logging/HankeKayttajaLoggingService.kt | 40 +++++ .../hanke/permissions/HankeKayttajaService.kt | 9 +- .../hanke/permissions/KayttajaTunniste.kt | 15 ++ .../haitaton/hanke/permissions/Permissions.kt | 15 +- .../hanke/factory/KayttajaTunnisteFactory.kt | 24 +++ .../hanke/factory/PermissionFactory.kt | 19 +++ .../HankeKayttajaLoggingServiceTest.kt | 139 ++++++++++++++++++ .../hanke/logging/HankeLoggingServiceTest.kt | 3 +- 10 files changed, 331 insertions(+), 6 deletions(-) create mode 100644 services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/logging/HankeKayttajaLoggingService.kt create mode 100644 services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/KayttajaTunnisteFactory.kt create mode 100644 services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/PermissionFactory.kt create mode 100644 services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/logging/HankeKayttajaLoggingServiceTest.kt diff --git a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt index d106528fa..67ee4e0af 100644 --- a/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt +++ b/services/hanke-service/src/integrationTest/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaServiceITest.kt @@ -7,6 +7,7 @@ import assertk.assertThat import assertk.assertions.containsExactly import assertk.assertions.containsExactlyInAnyOrder import assertk.assertions.each +import assertk.assertions.first import assertk.assertions.hasClass import assertk.assertions.hasSize import assertk.assertions.isEmpty @@ -23,7 +24,12 @@ import fi.hel.haitaton.hanke.factory.HankeFactory import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withGeneratedOmistaja import fi.hel.haitaton.hanke.factory.HankeFactory.Companion.withYhteystiedot import fi.hel.haitaton.hanke.factory.HankeYhteystietoFactory +import fi.hel.haitaton.hanke.logging.AuditLogRepository +import fi.hel.haitaton.hanke.logging.ObjectType +import fi.hel.haitaton.hanke.logging.Operation +import fi.hel.haitaton.hanke.logging.UserRole import fi.hel.haitaton.hanke.test.Asserts.isRecent +import fi.hel.haitaton.hanke.toChangeLogJsonString import java.time.OffsetDateTime import java.util.UUID import org.junit.jupiter.api.Nested @@ -51,6 +57,7 @@ class HankeKayttajaServiceITest : DatabaseTest() { @Autowired private lateinit var hankeKayttajaRepository: HankeKayttajaRepository @Autowired private lateinit var permissionRepository: PermissionRepository @Autowired private lateinit var roleRepository: RoleRepository + @Autowired private lateinit var auditLogRepository: AuditLogRepository @Autowired private lateinit var permissionService: PermissionService @@ -391,6 +398,36 @@ class HankeKayttajaServiceITest : DatabaseTest() { } } + @Test + fun `Writes permission update to audit log`() { + val hanke = hankeFactory.save() + val kayttaja = saveUserAndPermission(hanke.id!!) + val updates = mapOf(kayttaja.id to Role.HANKEMUOKKAUS) + auditLogRepository.deleteAll() + + hankeKayttajaService.updatePermissions(hanke, updates, false, USERNAME) + + val logs = auditLogRepository.findAll() + assertThat(logs).hasSize(1) + assertThat(logs) + .first() + .transform { it.message.auditEvent } + .all { + transform { it.target.type }.isEqualTo(ObjectType.PERMISSION) + transform { it.target.id }.isEqualTo(kayttaja.permission?.id.toString()) + transform { it.operation }.isEqualTo(Operation.UPDATE) + transform { it.actor.role }.isEqualTo(UserRole.USER) + transform { it.actor.userId }.isEqualTo(USERNAME) + val permission = kayttaja.permission!!.toDomain() + transform { it.target.objectBefore } + .isEqualTo(permission.toChangeLogJsonString()) + transform { it.target.objectAfter } + .isEqualTo( + permission.copy(role = Role.HANKEMUOKKAUS).toChangeLogJsonString() + ) + } + } + @Test fun `Updates role to tunniste if permission doesn't exist`() { val hanke = hankeFactory.save() @@ -406,6 +443,34 @@ class HankeKayttajaServiceITest : DatabaseTest() { } } + @Test + fun `Writes tunniste update to audit log`() { + val hanke = hankeFactory.save() + val kayttaja = saveUserAndToken(hanke.id!!, "Toinen Tohelo", "urho@kekkonen.test") + val updates = mapOf(kayttaja.id to Role.HANKEMUOKKAUS) + auditLogRepository.deleteAll() + + hankeKayttajaService.updatePermissions(hanke, updates, false, USERNAME) + + val logs = auditLogRepository.findAll() + assertThat(logs).hasSize(1) + assertThat(logs) + .first() + .transform { it.message.auditEvent } + .all { + transform { it.target.type }.isEqualTo(ObjectType.KAYTTAJA_TUNNISTE) + transform { it.target.id }.isEqualTo(kayttaja.kayttajaTunniste?.id.toString()) + transform { it.operation }.isEqualTo(Operation.UPDATE) + transform { it.actor.role }.isEqualTo(UserRole.USER) + transform { it.actor.userId }.isEqualTo(USERNAME) + val tunniste = + kayttaja.kayttajaTunniste!!.toDomain().copy(hankeKayttajaId = kayttaja.id) + transform { it.target.objectBefore }.isEqualTo(tunniste.toChangeLogJsonString()) + transform { it.target.objectAfter } + .isEqualTo(tunniste.copy(role = Role.HANKEMUOKKAUS).toChangeLogJsonString()) + } + } + @Test fun `Updates role to only permission if both permission and tunniste exist`() { val hanke = hankeFactory.save() diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/logging/AuditLogEntry.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/logging/AuditLogEntry.kt index 4788019dd..ec8661b13 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/logging/AuditLogEntry.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/logging/AuditLogEntry.kt @@ -43,13 +43,15 @@ enum class Status { } enum class ObjectType { - YHTEYSTIETO, - ALLU_CUSTOMER, + APPLICATION, ALLU_CONTACT, + ALLU_CUSTOMER, GDPR_RESPONSE, HANKE, HANKE_KAYTTAJA, - APPLICATION, + KAYTTAJA_TUNNISTE, + PERMISSION, + YHTEYSTIETO, } enum class UserRole { diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/logging/HankeKayttajaLoggingService.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/logging/HankeKayttajaLoggingService.kt new file mode 100644 index 000000000..b301b22be --- /dev/null +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/logging/HankeKayttajaLoggingService.kt @@ -0,0 +1,40 @@ +package fi.hel.haitaton.hanke.logging + +import fi.hel.haitaton.hanke.permissions.KayttajaTunniste +import fi.hel.haitaton.hanke.permissions.Permission +import fi.hel.haitaton.hanke.permissions.Role +import org.springframework.stereotype.Service +import org.springframework.transaction.annotation.Propagation +import org.springframework.transaction.annotation.Transactional + +@Service +class HankeKayttajaLoggingService(private val auditLogService: AuditLogService) { + + @Transactional(propagation = Propagation.MANDATORY) + fun logUpdate(roleBefore: Role, permissionAfter: Permission, userId: String) { + val permissionBefore = permissionAfter.copy(role = roleBefore) + + AuditLogService.updateEntry( + userId, + ObjectType.PERMISSION, + permissionBefore, + permissionAfter, + ) + ?.let { auditLogService.create(it) } + } + + @Transactional(propagation = Propagation.MANDATORY) + fun logUpdate( + kayttajaTunnisteBefore: KayttajaTunniste, + kayttajaTunnisteAfter: KayttajaTunniste, + userId: String + ) { + AuditLogService.updateEntry( + userId, + ObjectType.KAYTTAJA_TUNNISTE, + kayttajaTunnisteBefore, + kayttajaTunnisteAfter, + ) + ?.let { auditLogService.create(it) } + } +} diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt index c2f931cc6..2afbe196c 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/HankeKayttajaService.kt @@ -6,6 +6,7 @@ import fi.hel.haitaton.hanke.configuration.Feature import fi.hel.haitaton.hanke.configuration.FeatureFlags import fi.hel.haitaton.hanke.domain.Hanke import fi.hel.haitaton.hanke.domain.Perustaja +import fi.hel.haitaton.hanke.logging.HankeKayttajaLoggingService import java.util.UUID import mu.KotlinLogging import org.springframework.stereotype.Service @@ -20,6 +21,7 @@ class HankeKayttajaService( private val roleRepository: RoleRepository, private val permissionService: PermissionService, private val featureFlags: FeatureFlags, + private val logService: HankeKayttajaLoggingService, ) { @Transactional(readOnly = true) @@ -100,9 +102,14 @@ class HankeKayttajaService( kayttajat.forEach { kayttaja -> if (kayttaja.permission != null) { + val roleBefore = kayttaja.permission.role.role kayttaja.permission.role = roleRepository.findOneByRole(updates[kayttaja.id]!!) + logService.logUpdate(roleBefore, kayttaja.permission.toDomain(), userId) } else { - kayttaja.kayttajaTunniste!!.role = updates[kayttaja.id]!! + val kayttajaTunnisteBefore = kayttaja.kayttajaTunniste!!.toDomain() + kayttaja.kayttajaTunniste.role = updates[kayttaja.id]!! + val kayttajaTunnisteAfter = kayttaja.kayttajaTunniste.toDomain() + logService.logUpdate(kayttajaTunnisteBefore, kayttajaTunnisteAfter, userId) } } diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/KayttajaTunniste.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/KayttajaTunniste.kt index 893314a55..3f7d25c99 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/KayttajaTunniste.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/KayttajaTunniste.kt @@ -1,5 +1,8 @@ package fi.hel.haitaton.hanke.permissions +import com.fasterxml.jackson.annotation.JsonView +import fi.hel.haitaton.hanke.ChangeLogView +import fi.hel.haitaton.hanke.domain.HasId import fi.hel.haitaton.hanke.getCurrentTimeUTC import jakarta.persistence.Column import jakarta.persistence.Entity @@ -15,6 +18,16 @@ import kotlin.streams.asSequence import org.springframework.data.jpa.repository.JpaRepository import org.springframework.stereotype.Repository +@JsonView(ChangeLogView::class) +data class KayttajaTunniste( + override val id: UUID, + val tunniste: String, + val createdAt: OffsetDateTime, + val sentAt: OffsetDateTime?, + var role: Role, + val hankeKayttajaId: UUID? +) : HasId + @Entity @Table(name = "kayttaja_tunniste") class KayttajaTunnisteEntity( @@ -26,6 +39,8 @@ class KayttajaTunnisteEntity( @OneToOne(mappedBy = "kayttajaTunniste") val hankeKayttaja: HankeKayttajaEntity? ) { + fun toDomain() = KayttajaTunniste(id, tunniste, createdAt, sentAt, role, hankeKayttaja?.id) + companion object { private const val tokenLength: Int = 24 private val charPool: List = ('a'..'z') + ('A'..'Z') + ('0'..'9') diff --git a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/Permissions.kt b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/Permissions.kt index d7b9bac81..a14e2a728 100644 --- a/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/Permissions.kt +++ b/services/hanke-service/src/main/kotlin/fi/hel/haitaton/hanke/permissions/Permissions.kt @@ -1,5 +1,8 @@ package fi.hel.haitaton.hanke.permissions +import com.fasterxml.jackson.annotation.JsonView +import fi.hel.haitaton.hanke.ChangeLogView +import fi.hel.haitaton.hanke.domain.HasId import jakarta.persistence.Entity import jakarta.persistence.FetchType import jakarta.persistence.GeneratedValue @@ -52,4 +55,14 @@ class PermissionEntity( @ManyToOne(optional = false, fetch = FetchType.EAGER) @JoinColumn(name = "roleid") var role: RoleEntity, -) +) { + fun toDomain() = Permission(id, userId, hankeId, role.role) +} + +@JsonView(ChangeLogView::class) +data class Permission( + override val id: Int, + val userId: String, + val hankeId: Int, + var role: Role, +) : HasId diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/KayttajaTunnisteFactory.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/KayttajaTunnisteFactory.kt new file mode 100644 index 000000000..54b7e1a0b --- /dev/null +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/KayttajaTunnisteFactory.kt @@ -0,0 +1,24 @@ +package fi.hel.haitaton.hanke.factory + +import fi.hel.haitaton.hanke.permissions.KayttajaTunniste +import fi.hel.haitaton.hanke.permissions.Role +import java.time.OffsetDateTime +import java.util.UUID + +object KayttajaTunnisteFactory { + val TUNNISTE_ID: UUID = UUID.fromString("b514795c-d982-430a-836b-91371829db51") + const val TUNNISTE: String = "K6NqNdCJOrNRh45aCP08e9wc" + val CREATED_AT: OffsetDateTime = OffsetDateTime.parse("2023-08-31T14:25:13Z") + val SENT_AT: OffsetDateTime = OffsetDateTime.parse("2023-08-31T14:25:14Z") + val ROLE: Role = Role.KATSELUOIKEUS + val KAYTTAJA_ID: UUID = UUID.fromString("597431b3-3be1-4594-a07a-bef77c8167df") + + fun create( + id: UUID = TUNNISTE_ID, + tunniste: String = TUNNISTE, + createdAt: OffsetDateTime = CREATED_AT, + sentAt: OffsetDateTime? = SENT_AT, + role: Role = ROLE, + hankeKayttajaId: UUID? = KAYTTAJA_ID, + ) = KayttajaTunniste(id, tunniste, createdAt, sentAt, role, hankeKayttajaId) +} diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/PermissionFactory.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/PermissionFactory.kt new file mode 100644 index 000000000..a470fbaf6 --- /dev/null +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/factory/PermissionFactory.kt @@ -0,0 +1,19 @@ +package fi.hel.haitaton.hanke.factory + +import fi.hel.haitaton.hanke.permissions.Permission +import fi.hel.haitaton.hanke.permissions.Role + +object PermissionFactory { + + const val PERMISSION_ID = 65110 + const val USER_ID = "permissionUser" + const val HANKE_ID = 984141 + val ROLE = Role.KATSELUOIKEUS + + fun create( + id: Int = PERMISSION_ID, + userId: String = USER_ID, + hankeId: Int = HANKE_ID, + role: Role = ROLE, + ) = Permission(id, userId, hankeId, role) +} diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/logging/HankeKayttajaLoggingServiceTest.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/logging/HankeKayttajaLoggingServiceTest.kt new file mode 100644 index 000000000..504de195c --- /dev/null +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/logging/HankeKayttajaLoggingServiceTest.kt @@ -0,0 +1,139 @@ +package fi.hel.haitaton.hanke.logging + +import assertk.all +import assertk.assertThat +import assertk.assertions.contains +import assertk.assertions.isEqualTo +import assertk.assertions.isNotNull +import assertk.assertions.isNull +import fi.hel.haitaton.hanke.factory.KayttajaTunnisteFactory +import fi.hel.haitaton.hanke.factory.PermissionFactory +import fi.hel.haitaton.hanke.permissions.Role +import io.mockk.called +import io.mockk.checkUnnecessaryStub +import io.mockk.clearAllMocks +import io.mockk.confirmVerified +import io.mockk.mockk +import io.mockk.verify +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test + +class HankeKayttajaLoggingServiceTest { + private val userId = "test" + + private val auditLogService: AuditLogService = mockk(relaxed = true) + private val loggingService = HankeKayttajaLoggingService(auditLogService) + + @BeforeEach + fun clearMocks() { + clearAllMocks() + } + + @AfterEach + fun cleanUp() { + checkUnnecessaryStub() + confirmVerified(auditLogService) + } + + @Nested + inner class LogPermissionUpdate { + @Test + fun `Creates audit log entry for updated permission`() { + val roleBefore = Role.KATSELUOIKEUS + val permissionAfter = PermissionFactory.create(role = Role.HANKEMUOKKAUS) + + loggingService.logUpdate(roleBefore, permissionAfter, userId) + + verify { + auditLogService.create( + withArg { entry -> + assertThat(entry.operation).isEqualTo(Operation.UPDATE) + assertThat(entry.status).isEqualTo(Status.SUCCESS) + assertThat(entry.failureDescription).isNull() + assertThat(entry.userId).isEqualTo(userId) + assertThat(entry.userRole).isEqualTo(UserRole.USER) + assertThat(entry.objectId) + .isEqualTo(PermissionFactory.PERMISSION_ID.toString()) + assertThat(entry.objectType).isEqualTo(ObjectType.PERMISSION) + assertThat(entry.objectBefore).isNotNull().all { + contains(PermissionFactory.PERMISSION_ID.toString()) + contains(PermissionFactory.USER_ID) + contains(PermissionFactory.HANKE_ID.toString()) + contains("KATSELUOIKEUS") + } + assertThat(entry.objectAfter).isNotNull().all { + contains(PermissionFactory.PERMISSION_ID.toString()) + contains(PermissionFactory.USER_ID) + contains(PermissionFactory.HANKE_ID.toString()) + contains("HANKEMUOKKAUS") + } + } + ) + } + } + + @Test + fun `Doesn't create audit log entry if permission not updated`() { + val roleBefore = PermissionFactory.ROLE + val permissionAfter = PermissionFactory.create() + + loggingService.logUpdate(roleBefore, permissionAfter, userId) + + verify { auditLogService wasNot called } + } + } + + @Nested + inner class LogTunnisteUpdate { + @Test + fun `Creates audit log entry for updated kayttajatunniste`() { + val kayttajaTunnisteBefore = KayttajaTunnisteFactory.create(sentAt = null) + val kayttajaTunnisteAfter = KayttajaTunnisteFactory.create(role = Role.HANKEMUOKKAUS) + + loggingService.logUpdate(kayttajaTunnisteBefore, kayttajaTunnisteAfter, userId) + + verify { + auditLogService.create( + withArg { entry -> + assertThat(entry.operation).isEqualTo(Operation.UPDATE) + assertThat(entry.status).isEqualTo(Status.SUCCESS) + assertThat(entry.failureDescription).isNull() + assertThat(entry.userId).isEqualTo(userId) + assertThat(entry.userRole).isEqualTo(UserRole.USER) + assertThat(entry.objectId) + .isEqualTo(KayttajaTunnisteFactory.TUNNISTE_ID.toString()) + assertThat(entry.objectType).isEqualTo(ObjectType.KAYTTAJA_TUNNISTE) + assertThat(entry.objectBefore).isNotNull().all { + contains(KayttajaTunnisteFactory.TUNNISTE_ID.toString()) + contains(KayttajaTunnisteFactory.TUNNISTE) + contains(KayttajaTunnisteFactory.CREATED_AT.toString()) + contains("null") + contains(KayttajaTunnisteFactory.KAYTTAJA_ID.toString()) + contains("KATSELUOIKEUS") + } + assertThat(entry.objectAfter).isNotNull().all { + contains(KayttajaTunnisteFactory.TUNNISTE_ID.toString()) + contains(KayttajaTunnisteFactory.TUNNISTE) + contains(KayttajaTunnisteFactory.CREATED_AT.toString()) + contains(KayttajaTunnisteFactory.SENT_AT.toString()) + contains(KayttajaTunnisteFactory.KAYTTAJA_ID.toString()) + contains("HANKEMUOKKAUS") + } + } + ) + } + } + + @Test + fun `Doesn't create audit log entry if permission not updated`() { + val kayttajaTunnisteBefore = KayttajaTunnisteFactory.create() + val kayttajaTunnisteAfter = KayttajaTunnisteFactory.create() + + loggingService.logUpdate(kayttajaTunnisteBefore, kayttajaTunnisteAfter, userId) + + verify { auditLogService wasNot called } + } + } +} diff --git a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/logging/HankeLoggingServiceTest.kt b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/logging/HankeLoggingServiceTest.kt index 28dd5b124..539cda953 100644 --- a/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/logging/HankeLoggingServiceTest.kt +++ b/services/hanke-service/src/test/kotlin/fi/hel/haitaton/hanke/logging/HankeLoggingServiceTest.kt @@ -24,7 +24,7 @@ import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test -internal class HankeLoggingServiceTest { +class HankeLoggingServiceTest { private val userId = "test" private val auditLogService: AuditLogService = mockk(relaxed = true) @@ -146,6 +146,7 @@ internal class HankeLoggingServiceTest { ) } } + @Test fun `logUpdate doesn't create audit log entry if hanke not changed`() { val hankeBefore = HankeFactory.create(version = 1)