Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HAI-1527 Audit logging for permission updates #393

Merged
merged 4 commits into from
Sep 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand All @@ -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", "[email protected]")
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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all these functions unit tested?

Copy link
Contributor Author

@corvidian corvidian Aug 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, some of these were copied over and unchanged. Removing them.

I added unit tests for the ones that are still in use.


@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) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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<UUID>

@Entity
@Table(name = "kayttaja_tunniste")
class KayttajaTunnisteEntity(
Expand All @@ -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<Char> = ('a'..'z') + ('A'..'Z') + ('0'..'9')
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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<Int>
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
@@ -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)
}
Loading