diff --git a/src/main/kotlin/com/terraformation/backend/customer/model/IndividualUser.kt b/src/main/kotlin/com/terraformation/backend/customer/model/IndividualUser.kt index 2c267581f43..ea9c7df7563 100644 --- a/src/main/kotlin/com/terraformation/backend/customer/model/IndividualUser.kt +++ b/src/main/kotlin/com/terraformation/backend/customer/model/IndividualUser.kt @@ -668,8 +668,10 @@ data class IndividualUser( override fun canUpdatePlantingZone(plantingZoneId: PlantingZoneId) = isAdminOrHigher(parentStore.getOrganizationId(plantingZoneId)) - override fun canUpdateProject(projectId: ProjectId) = - isAdminOrHigher(parentStore.getOrganizationId(projectId)) + override fun canUpdateProject(projectId: ProjectId): Boolean { + val organizationId = parentStore.getOrganizationId(projectId) ?: return false + return isAdminOrHigher(organizationId) || isGlobalWriter(organizationId) + } override fun canUpdateProjectAcceleratorDetails(projectId: ProjectId): Boolean = isTFExpertOrHigher() @@ -789,11 +791,18 @@ data class IndividualUser( /** Returns true if one of the user's global roles allows them to read an organization. */ private fun isGlobalReader(organizationId: OrganizationId) = - GlobalRole.SuperAdmin in globalRoles || + isSuperAdmin() || (isReadOnlyOrHigher() && (parentStore.hasInternalTag(organizationId, InternalTagIds.Accelerator) || parentStore.hasApplications(organizationId))) + /** Returns true if one of the user's global roles allows them to write to an organization. */ + private fun isGlobalWriter(organizationId: OrganizationId) = + isSuperAdmin() || + (isTFExpertOrHigher() && + (parentStore.hasInternalTag(organizationId, InternalTagIds.Accelerator) || + parentStore.hasApplications(organizationId))) + private var isRecordingChecks: Boolean = false private fun recordPermissionCheck(check: PermissionCheck) { diff --git a/src/test/kotlin/com/terraformation/backend/customer/model/PermissionTest.kt b/src/test/kotlin/com/terraformation/backend/customer/model/PermissionTest.kt index 076bda11eea..2dcdfaa2a21 100644 --- a/src/test/kotlin/com/terraformation/backend/customer/model/PermissionTest.kt +++ b/src/test/kotlin/com/terraformation/backend/customer/model/PermissionTest.kt @@ -1646,7 +1646,7 @@ internal class PermissionTest : DatabaseTest() { removeTerraformationContact = true, ) - // Can access accelerator-related functions on all ogs. + // Can access accelerator-related functions on all orgs. permissions.expect( ProjectId(3000), ProjectId(4000), @@ -1662,6 +1662,7 @@ internal class PermissionTest : DatabaseTest() { readProjectVotes = true, updateDefaultVoters = true, updateInternalVariableWorkflowDetails = true, + updateProject = true, updateProjectAcceleratorDetails = true, updateProjectDocumentSettings = true, updateProjectScores = true, @@ -1880,6 +1881,7 @@ internal class PermissionTest : DatabaseTest() { readProjectScores = true, readProjectVotes = true, updateInternalVariableWorkflowDetails = true, + updateProject = true, updateProjectAcceleratorDetails = true, updateProjectDocumentSettings = true, updateProjectScores = true, @@ -2081,6 +2083,7 @@ internal class PermissionTest : DatabaseTest() { readProjectScores = true, readProjectVotes = true, updateInternalVariableWorkflowDetails = true, + updateProject = true, updateProjectAcceleratorDetails = true, updateProjectScores = true, updateProjectVotes = true, diff --git a/src/test/kotlin/com/terraformation/backend/documentproducer/db/VariableWorkflowStoreTest.kt b/src/test/kotlin/com/terraformation/backend/documentproducer/db/VariableWorkflowStoreTest.kt index 842fb9f1675..ba38a4a82bb 100644 --- a/src/test/kotlin/com/terraformation/backend/documentproducer/db/VariableWorkflowStoreTest.kt +++ b/src/test/kotlin/com/terraformation/backend/documentproducer/db/VariableWorkflowStoreTest.kt @@ -1,18 +1,19 @@ package com.terraformation.backend.documentproducer.db -import com.terraformation.backend.RunsAsUser +import com.terraformation.backend.RunsAsDatabaseUser import com.terraformation.backend.TestClock import com.terraformation.backend.TestEventPublisher +import com.terraformation.backend.customer.model.TerrawareUser import com.terraformation.backend.db.DatabaseTest import com.terraformation.backend.db.ProjectNotFoundException +import com.terraformation.backend.db.default_schema.GlobalRole +import com.terraformation.backend.db.default_schema.Role import com.terraformation.backend.db.docprod.VariableType import com.terraformation.backend.db.docprod.VariableWorkflowStatus import com.terraformation.backend.db.docprod.tables.records.VariableWorkflowHistoryRecord import com.terraformation.backend.db.docprod.tables.references.VARIABLE_WORKFLOW_HISTORY import com.terraformation.backend.documentproducer.event.QuestionsDeliverableReviewedEvent import com.terraformation.backend.documentproducer.model.ExistingVariableWorkflowHistoryModel -import com.terraformation.backend.mockUser -import io.mockk.every import java.time.Instant import java.util.UUID import org.junit.jupiter.api.Assertions.* @@ -24,8 +25,8 @@ import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.EnumSource import org.springframework.security.access.AccessDeniedException -class VariableWorkflowStoreTest : DatabaseTest(), RunsAsUser { - override val user = mockUser() +class VariableWorkflowStoreTest : DatabaseTest(), RunsAsDatabaseUser { + override lateinit var user: TerrawareUser private val clock = TestClock() private val eventPublisher = TestEventPublisher() @@ -37,6 +38,7 @@ class VariableWorkflowStoreTest : DatabaseTest(), RunsAsUser { fun setUp() { insertOrganization() insertProject() + insertApplication() insertDocumentTemplate() insertVariableManifest() insertDocument() @@ -47,9 +49,7 @@ class VariableWorkflowStoreTest : DatabaseTest(), RunsAsUser { insertValue(variableId = inserted.variableId) - every { user.canReadProject(any()) } returns true - every { user.canReadInternalVariableWorkflowDetails(any()) } returns true - every { user.canUpdateInternalVariableWorkflowDetails(any()) } returns true + insertUserGlobalRole(role = GlobalRole.AcceleratorAdmin) } @Nested @@ -160,7 +160,8 @@ class VariableWorkflowStoreTest : DatabaseTest(), RunsAsUser { @Test fun `does not populate internal comment if user lacks permission to see it`() { - every { user.canReadInternalVariableWorkflowDetails(inserted.projectId) } returns false + deleteUserGlobalRole(role = GlobalRole.AcceleratorAdmin) + insertOrganizationUser(role = Role.Manager) val historyId = insertVariableWorkflowHistory( @@ -189,7 +190,7 @@ class VariableWorkflowStoreTest : DatabaseTest(), RunsAsUser { @Test fun `throws exception if no permission to read project`() { - every { user.canReadProject(inserted.projectId) } returns false + deleteUserGlobalRole(role = GlobalRole.AcceleratorAdmin) assertThrows { store.fetchCurrentForProject(inserted.projectId) } } @@ -275,7 +276,9 @@ class VariableWorkflowStoreTest : DatabaseTest(), RunsAsUser { @Test fun `throws exception if no permission to read internal workflow details`() { - every { user.canReadInternalVariableWorkflowDetails(any()) } returns false + deleteUserGlobalRole(role = GlobalRole.AcceleratorAdmin) + insertOrganizationUser(role = Role.Admin) + assertThrows { store.fetchProjectVariableHistory(inserted.projectId, inserted.variableId) } @@ -365,7 +368,19 @@ class VariableWorkflowStoreTest : DatabaseTest(), RunsAsUser { } @Test - fun `allows non-admin to set status to In Review`() { + fun `allows organization admin to set status to In Review`() { + deleteUserGlobalRole(role = GlobalRole.AcceleratorAdmin) + insertOrganizationUser(role = Role.Admin) + + testSetStatusToInReview() + } + + @Test + fun `allows accelerator admin to set status to In Review`() { + testSetStatusToInReview() + } + + private fun testSetStatusToInReview() { val variableId = inserted.variableId val variableValueId = insertValue(variableId = variableId) val existingHistoryId = @@ -373,9 +388,6 @@ class VariableWorkflowStoreTest : DatabaseTest(), RunsAsUser { feedback = "Lazy answer! Do better next time", status = VariableWorkflowStatus.Rejected) - every { user.canUpdateInternalVariableWorkflowDetails(inserted.projectId) } returns false - every { user.canUpdateProject(inserted.projectId) } returns true - store.update( projectId = inserted.projectId, variableId = variableId, @@ -408,8 +420,8 @@ class VariableWorkflowStoreTest : DatabaseTest(), RunsAsUser { val variableId = inserted.variableId insertVariableWorkflowHistory(status = VariableWorkflowStatus.InReview) - every { user.canUpdateInternalVariableWorkflowDetails(inserted.projectId) } returns false - every { user.canUpdateProject(inserted.projectId) } returns true + deleteUserGlobalRole(role = GlobalRole.AcceleratorAdmin) + insertOrganizationUser(role = Role.Admin) assertThrows { store.update(