Skip to content

Commit

Permalink
HAI-1875 Add audit logging for new users and tokens
Browse files Browse the repository at this point in the history
Add logging for creating permissions, hankekayttaja and hanketunniste.

Adding the audit reasonably required some refactoring:
- In PermissionService, make the old setPermission function be
  explicitly for creating new permisisions, so logging can be added.
- Move updating the kayttooikeustaso of a permission to
  PermissionService to centralize modifications and logging.
- In HankeKayttajaService, make the saveUser function to handle creating
  new users explicitly so that logging can be added.
- Rename kayttooikeustaso in PermissionEntity to kayttooikeustasoEntity
  and add a new getter for the actual kayttooikeustaso, so that
  `permissionEntity.kayttooikeustaso` is required instead of the old
  `permissionEntity.kayttooikeustaso.kayttooikeustaso`.
- Change references to permissionEntity.kayttooikeustasoEntity to use
  either the new getter for reads or PermissionService functions for
  modifications where ever possible.
- Move creating the permission for a founder inside addHankeFounder,
  since it seemed like a better packaging and made testing the audit
  logs easier.
- Move the static hasPermission from PermissionService's companion
  object to PermissionEntity and KayttooikeustasoEntity.
- Add userIds to several calls se that the logging can use it.
- Move Permission logging to PermissionLoggingService from
  HankeKayttajaLoggingService.
- In tests, use custom asserts to make audit log tests more readable and
  props to give better error messages when a test fails.
  • Loading branch information
corvidian committed Sep 5, 2023
1 parent 86012b9 commit 741ecf0
Show file tree
Hide file tree
Showing 22 changed files with 859 additions and 334 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,8 @@ class ApplicationServiceITest : DatabaseTest() {
val otherUser = "otherUser"
val hanke = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1234"))
val hanke2 = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1235"))
permissionService.setPermission(hanke.id!!, USERNAME, Kayttooikeustaso.HAKEMUSASIOINTI)
permissionService.setPermission(hanke2.id!!, "otherUser", Kayttooikeustaso.HAKEMUSASIOINTI)
permissionService.create(hanke.id!!, USERNAME, Kayttooikeustaso.HAKEMUSASIOINTI)
permissionService.create(hanke2.id!!, "otherUser", Kayttooikeustaso.HAKEMUSASIOINTI)

alluDataFactory.saveApplicationEntities(3, USERNAME, hanke = hanke) { _, application ->
application.userId = USERNAME
Expand Down Expand Up @@ -376,8 +376,8 @@ class ApplicationServiceITest : DatabaseTest() {
val hanke = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1234"))
val hanke2 = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1235"))
val hanke3 = hankeRepository.save(HankeEntity(hankeTunnus = "HAI-1236"))
permissionService.setPermission(hanke.id!!, USERNAME, Kayttooikeustaso.HAKEMUSASIOINTI)
permissionService.setPermission(hanke2.id!!, USERNAME, Kayttooikeustaso.HAKEMUSASIOINTI)
permissionService.create(hanke.id!!, USERNAME, Kayttooikeustaso.HAKEMUSASIOINTI)
permissionService.create(hanke2.id!!, USERNAME, Kayttooikeustaso.HAKEMUSASIOINTI)
val application1 = alluDataFactory.saveApplicationEntity(username = USERNAME, hanke = hanke)
val application2 =
alluDataFactory.saveApplicationEntity(username = "secondUser", hanke = hanke2)
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,15 +1,33 @@
package fi.hel.haitaton.hanke.permissions

import assertk.all
import assertk.assertThat
import assertk.assertions.first
import assertk.assertions.hasSize
import assertk.assertions.isFalse
import assertk.assertions.isTrue
import assertk.assertions.isEmpty
import assertk.assertions.isEqualTo
import assertk.assertions.isNull
import assertk.assertions.key
import assertk.assertions.prop
import fi.hel.haitaton.hanke.DatabaseTest
import fi.hel.haitaton.hanke.HankeService
import fi.hel.haitaton.hanke.factory.HankeFactory
import fi.hel.haitaton.hanke.logging.AuditLogEvent
import fi.hel.haitaton.hanke.logging.AuditLogRepository
import fi.hel.haitaton.hanke.logging.AuditLogTarget
import fi.hel.haitaton.hanke.logging.ObjectType
import fi.hel.haitaton.hanke.logging.Operation
import fi.hel.haitaton.hanke.test.AuditLogEntryEntityAsserts.auditEvent
import fi.hel.haitaton.hanke.test.AuditLogEntryEntityAsserts.hasId
import fi.hel.haitaton.hanke.test.AuditLogEntryEntityAsserts.hasObjectAfter
import fi.hel.haitaton.hanke.test.AuditLogEntryEntityAsserts.hasObjectBefore
import fi.hel.haitaton.hanke.test.AuditLogEntryEntityAsserts.hasUserActor
import fi.hel.haitaton.hanke.test.AuditLogEntryEntityAsserts.isSuccess
import fi.hel.haitaton.hanke.test.AuditLogEntryEntityAsserts.withTarget
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.Arguments
Expand All @@ -20,17 +38,21 @@ import org.springframework.security.test.context.support.WithMockUser
import org.springframework.test.context.ActiveProfiles
import org.testcontainers.junit.jupiter.Testcontainers

private const val CURRENT_USER: String = "test7358"

@Testcontainers
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@ActiveProfiles("test")
@WithMockUser(username = "test7358")
@WithMockUser(username = CURRENT_USER)
class PermissionServiceITest : DatabaseTest() {

val username = "user"

@Autowired lateinit var permissionService: PermissionService
@Autowired lateinit var permissionRepository: PermissionRepository
@Autowired lateinit var hankeService: HankeService
@Autowired lateinit var hankeFactory: HankeFactory
@Autowired lateinit var auditLogRepository: AuditLogRepository

companion object {
@JvmStatic
Expand Down Expand Up @@ -74,18 +96,12 @@ class PermissionServiceITest : DatabaseTest() {
) {
val kayttooikeustasoEntity = permissionService.findKayttooikeustaso(kayttooikeustaso)

allowedPermissions.forEach { code ->
assertThat(code)
.transform { PermissionService.hasPermission(kayttooikeustasoEntity, it) }
.isTrue()
val permissions =
PermissionCode.values().associateWith { kayttooikeustasoEntity.hasPermission(it) }

PermissionCode.values().forEach {
assertThat(permissions).key(it).isEqualTo(allowedPermissions.contains(it))
}
PermissionCode.values()
.filter { !allowedPermissions.contains(it) }
.forEach { code ->
assertThat(code)
.transform { PermissionService.hasPermission(kayttooikeustasoEntity, it) }
.isFalse()
}
}

@Test
Expand All @@ -97,18 +113,14 @@ class PermissionServiceITest : DatabaseTest() {

@Test
fun `getAllowedHankeIds with permissions returns list of IDs`() {
val kaikkiOikeudet =
permissionService.findKayttooikeustaso(Kayttooikeustaso.KAIKKI_OIKEUDET)
val hankkeet = saveSeveralHanke(3)
val hankkeet = hankeFactory.saveSeveralMinimal(3)
hankkeet
.map { it.id!! }
.forEach {
permissionRepository.save(
PermissionEntity(
userId = username,
hankeId = it,
kayttooikeustaso = kaikkiOikeudet,
)
permissionService.create(
userId = username,
hankeId = it,
kayttooikeustaso = Kayttooikeustaso.KAIKKI_OIKEUDET,
)
}

Expand All @@ -119,23 +131,15 @@ class PermissionServiceITest : DatabaseTest() {

@Test
fun `getAllowedHankeIds return ids with correct permissions`() {
val kaikkiOikeudet =
permissionService.findKayttooikeustaso(Kayttooikeustaso.KAIKKI_OIKEUDET)
val hankemuokkaus = permissionService.findKayttooikeustaso(Kayttooikeustaso.HANKEMUOKKAUS)
val hakemusasiointi =
permissionService.findKayttooikeustaso(Kayttooikeustaso.HAKEMUSASIOINTI)
val katseluoikeus = permissionService.findKayttooikeustaso(Kayttooikeustaso.KATSELUOIKEUS)
val hankkeet = saveSeveralHanke(4)
val kaikkiOikeudet = Kayttooikeustaso.KAIKKI_OIKEUDET
val hankemuokkaus = Kayttooikeustaso.HANKEMUOKKAUS
val hakemusasiointi = Kayttooikeustaso.HAKEMUSASIOINTI
val katseluoikeus = Kayttooikeustaso.KATSELUOIKEUS
val hankkeet = hankeFactory.saveSeveralMinimal(4)
listOf(kaikkiOikeudet, hankemuokkaus, hakemusasiointi, katseluoikeus).zip(hankkeet) {
kayttooikeustaso,
hanke ->
permissionRepository.save(
PermissionEntity(
userId = username,
hankeId = hanke.id!!,
kayttooikeustaso = kayttooikeustaso,
)
)
permissionService.create(hanke.id!!, username, kayttooikeustaso)
}

val response = permissionService.getAllowedHankeIds(username, PermissionCode.EDIT)
Expand All @@ -150,79 +154,127 @@ class PermissionServiceITest : DatabaseTest() {

@Test
fun `hasPermission with correct permission`() {
val kaikkiOikeudet =
permissionService.findKayttooikeustaso(Kayttooikeustaso.KAIKKI_OIKEUDET)
val hankeId = saveSeveralHanke(1)[0].id!!
permissionRepository.save(
PermissionEntity(
userId = username,
hankeId = hankeId,
kayttooikeustaso = kaikkiOikeudet
)
)
val hankeId = hankeFactory.saveMinimal().id!!
permissionService.create(hankeId, username, Kayttooikeustaso.KAIKKI_OIKEUDET)

assertTrue(permissionService.hasPermission(hankeId, username, PermissionCode.EDIT))
}

@Test
fun `hasPermission with insufficient permissions`() {
val hakemusasiointi =
permissionService.findKayttooikeustaso(Kayttooikeustaso.HAKEMUSASIOINTI)
val hankeId = saveSeveralHanke(1)[0].id!!
permissionRepository.save(
PermissionEntity(
userId = username,
hankeId = hankeId,
kayttooikeustaso = hakemusasiointi
)
)
val hankeId = hankeFactory.saveMinimal().id!!
permissionService.create(hankeId, username, Kayttooikeustaso.HAKEMUSASIOINTI)

assertFalse(permissionService.hasPermission(hankeId, username, PermissionCode.EDIT))
}

@Test
fun `setPermission creates a new permission`() {
val hankeId = saveSeveralHanke(1)[0].id!!
permissionRepository.deleteAll() // remove permission created in hanke creation

permissionService.setPermission(hankeId, username, Kayttooikeustaso.KATSELUOIKEUS)

val permissions = permissionRepository.findAll()
assertThat(permissions).hasSize(1)
assertEquals(hankeId, permissions[0].hankeId)
assertEquals(
Kayttooikeustaso.KATSELUOIKEUS,
permissions[0].kayttooikeustaso.kayttooikeustaso
)
@Nested
inner class Create {

@Test
fun `Creates a new permission`() {
val hankeId = hankeFactory.saveMinimal().id!!

val result = permissionService.create(hankeId, username, Kayttooikeustaso.KATSELUOIKEUS)

val permissions = permissionRepository.findAll()
assertThat(permissions).hasSize(1)
assertThat(permissions).first().all {
prop(PermissionEntity::id).isEqualTo(result.id)
prop(PermissionEntity::hankeId).isEqualTo(hankeId)
prop(PermissionEntity::userId).isEqualTo(username)
prop(PermissionEntity::kayttooikeustaso).isEqualTo(Kayttooikeustaso.KATSELUOIKEUS)
}
}

@Test
fun `Writes to audit log`() {
val hankeId = hankeFactory.saveMinimal().id!!
assertThat(auditLogRepository.findAll()).isEmpty()

val permission =
permissionService.create(hankeId, username, Kayttooikeustaso.KATSELUOIKEUS)

val logs = auditLogRepository.findAll()
assertThat(logs).hasSize(1)
val expectedObject =
Permission(
id = permission.id,
userId = username,
hankeId = hankeId,
kayttooikeustaso = Kayttooikeustaso.KATSELUOIKEUS,
)
assertThat(logs).first().auditEvent {
withTarget {
prop(AuditLogTarget::type).isEqualTo(ObjectType.PERMISSION)
hasId(permission.id)
prop(AuditLogTarget::objectBefore).isNull()
hasObjectAfter(expectedObject)
}
prop(AuditLogEvent::operation).isEqualTo(Operation.CREATE)
hasUserActor(CURRENT_USER)
}
}
}

@Test
fun `setPermission updates an existing permission`() {
val hankeId = saveSeveralHanke(1)[0].id!!
permissionRepository.deleteAll() // remove permission created in hanke creation
val kayttooikeustaso =
permissionService.findKayttooikeustaso(Kayttooikeustaso.KATSELUOIKEUS)
permissionRepository.save(
PermissionEntity(
userId = username,
hankeId = hankeId,
kayttooikeustaso = kayttooikeustaso
@Nested
inner class UpdateKayttooikeustaso {

@Test
fun `updateKayttooikeustaso updates an existing permission`() {
val hankeId = hankeFactory.saveMinimal().id!!
val permission =
permissionService.create(hankeId, username, Kayttooikeustaso.KATSELUOIKEUS)

permissionService.updateKayttooikeustaso(
permission,
Kayttooikeustaso.HAKEMUSASIOINTI,
username,
)
)

permissionService.setPermission(hankeId, username, Kayttooikeustaso.HAKEMUSASIOINTI)
val permissions = permissionRepository.findAll()
assertThat(permissions).hasSize(1)
assertThat(permissions).first().all {
prop(PermissionEntity::id).isEqualTo(permission.id)
prop(PermissionEntity::hankeId).isEqualTo(hankeId)
prop(PermissionEntity::userId).isEqualTo(username)
prop(PermissionEntity::kayttooikeustaso).isEqualTo(Kayttooikeustaso.HAKEMUSASIOINTI)
}
}

val permissions = permissionRepository.findAll()
assertThat(permissions).hasSize(1)
assertEquals(hankeId, permissions[0].hankeId)
assertEquals(
Kayttooikeustaso.HAKEMUSASIOINTI,
permissions[0].kayttooikeustaso.kayttooikeustaso
)
}
@Test
fun `updateKayttooikeustaso writes to audit log`() {
val hankeId = hankeFactory.saveMinimal().id!!
val permission =
permissionService.create(hankeId, username, Kayttooikeustaso.KATSELUOIKEUS)
auditLogRepository.deleteAll()

private fun saveSeveralHanke(n: Int) =
createSeveralHanke(n).map { hankeService.createHanke(it) }
permissionService.updateKayttooikeustaso(
permission,
Kayttooikeustaso.HAKEMUSASIOINTI,
CURRENT_USER,
)

private fun createSeveralHanke(n: Int) = (1..n).map { HankeFactory.create(id = null) }
val logs = auditLogRepository.findAll()
assertThat(logs).hasSize(1)
val expectedObject =
Permission(
id = permission.id,
userId = username,
hankeId = hankeId,
kayttooikeustaso = Kayttooikeustaso.KATSELUOIKEUS,
)
assertThat(logs).first().isSuccess(Operation.UPDATE) {
withTarget {
prop(AuditLogTarget::type).isEqualTo(ObjectType.PERMISSION)
hasId(permission.id)
hasObjectBefore(expectedObject)
hasObjectAfter(
expectedObject.copy(kayttooikeustaso = Kayttooikeustaso.HAKEMUSASIOINTI)
)
}
hasUserActor(CURRENT_USER)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import fi.hel.haitaton.hanke.logging.HankeLoggingService
import fi.hel.haitaton.hanke.logging.Operation
import fi.hel.haitaton.hanke.logging.YhteystietoLoggingEntryHolder
import fi.hel.haitaton.hanke.permissions.HankeKayttajaService
import fi.hel.haitaton.hanke.permissions.Kayttooikeustaso
import fi.hel.haitaton.hanke.permissions.PermissionService
import fi.hel.haitaton.hanke.tormaystarkastelu.TormaystarkasteluLaskentaService
import fi.hel.haitaton.hanke.tormaystarkastelu.TormaystarkasteluTulos
Expand Down Expand Up @@ -216,7 +215,7 @@ open class HankeServiceImpl(
postProcessAndSaveLogging(loggingEntryHolder, savedHankeEntity, userId)

return createHankeDomainObjectFromEntity(entity).also {
hankeKayttajaService.saveNewTokensFromHanke(it)
hankeKayttajaService.saveNewTokensFromHanke(it, userId)
hankeLoggingService.logUpdate(hankeBeforeUpdate, it, userId)
}
}
Expand Down Expand Up @@ -246,10 +245,8 @@ open class HankeServiceImpl(

private fun initAccessForCreatedHanke(hanke: Hanke, perustaja: Perustaja?, userId: String) {
val hankeId = hanke.id!!
val permissionAll =
permissionService.setPermission(hankeId, userId, Kayttooikeustaso.KAIKKI_OIKEUDET)
perustaja?.let { hankeKayttajaService.addHankeFounder(hankeId, it, permissionAll) }
hankeKayttajaService.saveNewTokensFromHanke(hanke)
hankeKayttajaService.addHankeFounder(hankeId, perustaja, userId)
hankeKayttajaService.saveNewTokensFromHanke(hanke, userId)
}

// TODO: functions to remove and invalidate Hanke's tormaystarkastelu-data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ open class ApplicationService(
return application.toApplication()
}

hankeKayttajaService.saveNewTokensFromApplication(application, hanke.id!!)
hankeKayttajaService.saveNewTokensFromApplication(application, hanke.id!!, userId)

// The application should no longer be a draft
application.applicationData = application.applicationData.copy(pendingOnClient = false)
Expand Down
Loading

0 comments on commit 741ecf0

Please sign in to comment.