From c4b05c73791d36bae5cd262f70dbfa9e21eb3448 Mon Sep 17 00:00:00 2001 From: Leopold Cramer Date: Mon, 24 Nov 2025 15:09:59 +0100 Subject: [PATCH 1/2] feat: add linked datasets default security update on scenario default security update --- .../service/RunnerServiceIntegrationTest.kt | 58 +++++++++++++++++- .../cosmotech/runner/service/RunnerService.kt | 30 ++++++++++ .../service/ScenarioServiceIntegrationTest.kt | 59 ++++++++++++++++++- .../scenario/service/ScenarioServiceImpl.kt | 24 ++++++++ 4 files changed, 166 insertions(+), 5 deletions(-) diff --git a/runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceIntegrationTest.kt b/runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceIntegrationTest.kt index 312e980fe..74d2dec3a 100644 --- a/runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceIntegrationTest.kt +++ b/runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceIntegrationTest.kt @@ -42,6 +42,7 @@ import com.redis.testcontainers.RedisStackContainer import io.mockk.every import io.mockk.junit5.MockKExtension import io.mockk.mockkStatic +import io.mockk.verify import java.time.Instant import java.util.* import kotlin.test.* @@ -1095,6 +1096,56 @@ class RunnerServiceIntegrationTest : CsmRedisTestBase() { } } + @Test + fun `when sharing a runner, the linked dataset default security should be set to at least viewer if the dataset isn't main`() { + runnerSaved.datasetList!!.removeLast() + listOf(ROLE_NONE, ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).forEach { role -> + var linkedDataset = + datasetApiService.createDataset( + organizationSaved.id!!, makeDataset(isMain = false, default = role)) + runnerSaved.datasetList!!.add(linkedDataset.id!!) + linkedDataset = + datasetApiService.createDataset( + organizationSaved.id!!, makeDataset(isMain = true, default = role)) + runnerSaved.datasetList!!.add(linkedDataset.id!!) + } + runnerSaved = + runnerApiService.updateRunner( + organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!, runnerSaved) + + runnerApiService.setRunnerDefaultSecurity( + organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!, RunnerRole(ROLE_EDITOR)) + + verify(exactly = 1) { + datasetApiService.setDatasetDefaultSecurity(any(), any(), DatasetRole(ROLE_VIEWER)) + } + } + + @Test + fun `when stopping sharing a scenario, the dataset default security should be set to none it's not higher than viewer and the dataset isn't main`() { + runnerSaved.datasetList!!.removeLast() + listOf(ROLE_NONE, ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).forEach { role -> + var linkedDataset = + datasetApiService.createDataset( + organizationSaved.id!!, makeDataset(isMain = false, default = role)) + runnerSaved.datasetList!!.add(linkedDataset.id!!) + linkedDataset = + datasetApiService.createDataset( + organizationSaved.id!!, makeDataset(isMain = true, default = role)) + runnerSaved.datasetList!!.add(linkedDataset.id!!) + } + runnerSaved = + runnerApiService.updateRunner( + organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!, runnerSaved) + + runnerApiService.setRunnerDefaultSecurity( + organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!, RunnerRole(ROLE_NONE)) + + verify(exactly = 1) { + datasetApiService.setDatasetDefaultSecurity(any(), any(), DatasetRole(ROLE_NONE)) + } + } + private fun makeConnector(name: String = "name"): Connector { return Connector( key = UUID.randomUUID().toString(), @@ -1107,7 +1158,9 @@ class RunnerServiceIntegrationTest : CsmRedisTestBase() { fun makeDataset( organizationId: String = organizationSaved.id!!, name: String = "name", - connector: Connector = connectorSaved + connector: Connector = connectorSaved, + isMain: Boolean = true, + default: String = ROLE_NONE ): Dataset { return Dataset( name = name, @@ -1120,9 +1173,10 @@ class RunnerServiceIntegrationTest : CsmRedisTestBase() { name = connector.name, version = connector.version, ), + main = isMain, security = DatasetSecurity( - default = ROLE_NONE, + default = default, accessControlList = mutableListOf( DatasetAccessControl(id = CONNECTED_ADMIN_USER, role = ROLE_ADMIN), diff --git a/runner/src/main/kotlin/com/cosmotech/runner/service/RunnerService.kt b/runner/src/main/kotlin/com/cosmotech/runner/service/RunnerService.kt index beca1aead..6ea55577d 100644 --- a/runner/src/main/kotlin/com/cosmotech/runner/service/RunnerService.kt +++ b/runner/src/main/kotlin/com/cosmotech/runner/service/RunnerService.kt @@ -15,6 +15,7 @@ import com.cosmotech.api.rbac.PERMISSION_READ_SECURITY import com.cosmotech.api.rbac.ROLE_NONE import com.cosmotech.api.rbac.ROLE_USER import com.cosmotech.api.rbac.ROLE_VALIDATOR +import com.cosmotech.api.rbac.ROLE_VIEWER import com.cosmotech.api.rbac.RolesDefinition import com.cosmotech.api.rbac.getScenarioRolesDefinition import com.cosmotech.api.rbac.model.RbacAccessControl @@ -26,6 +27,7 @@ import com.cosmotech.api.utils.getCurrentAuthenticatedRoles import com.cosmotech.api.utils.getCurrentAuthenticatedUserName import com.cosmotech.dataset.DatasetApiServiceInterface import com.cosmotech.dataset.domain.Dataset +import com.cosmotech.dataset.domain.DatasetRole import com.cosmotech.dataset.service.getRbac import com.cosmotech.organization.OrganizationApiServiceInterface import com.cosmotech.organization.domain.Organization @@ -506,6 +508,34 @@ class RunnerService( // create a rbacSecurity object from runner Rbac by changing default value val rbacSecurity = csmRbac.setDefault(this.getRbacSecurity(), role, this.roleDefinition) this.setRbacSecurity(rbacSecurity) + // this.runner.datasetList!! + // .mapNotNull { + // datasetApiService.findByOrganizationIdAndDatasetId(this.runner.organizationId!!, + // it) + // } + // .forEach { dataset -> + updateLinkedDatasetDefaultSecurity(role) + // } + } + + private fun updateLinkedDatasetDefaultSecurity(role: String) { + var datasetRole = ROLE_NONE + if (role != ROLE_NONE) datasetRole = ROLE_VIEWER + this.runner.datasetList!!.forEach { datasetId -> + val linkedDataset = + datasetApiService.findDatasetById(this.runner.organizationId!!, datasetId) + // We do not want to lower the default security if it's higher than viewer + if (linkedDataset.security!!.default != ROLE_NONE && datasetRole == ROLE_VIEWER) + return@forEach Unit + if (linkedDataset.security!!.default != ROLE_NONE && datasetRole == ROLE_NONE) + return@forEach Unit + // Filter on dataset copy (because we do not want to update main dataset as it can be shared + // between scenarios) + if (linkedDataset.main != true) { + datasetApiService.setDatasetDefaultSecurity( + this.runner.organizationId!!, datasetId, DatasetRole(datasetRole)) + } + } } } diff --git a/scenario/src/integrationTest/kotlin/com/cosmotech/scenario/service/ScenarioServiceIntegrationTest.kt b/scenario/src/integrationTest/kotlin/com/cosmotech/scenario/service/ScenarioServiceIntegrationTest.kt index 000d6e31f..48e56bd41 100644 --- a/scenario/src/integrationTest/kotlin/com/cosmotech/scenario/service/ScenarioServiceIntegrationTest.kt +++ b/scenario/src/integrationTest/kotlin/com/cosmotech/scenario/service/ScenarioServiceIntegrationTest.kt @@ -54,6 +54,7 @@ import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.junit5.MockKExtension import io.mockk.mockkStatic +import io.mockk.verify import java.time.Instant import java.util.* import kotlin.test.assertEquals @@ -1257,6 +1258,56 @@ class ScenarioServiceIntegrationTest : CsmRedisTestBase() { } } + @Test + fun `when sharing a scenario, the linked dataset default security should be set to at least viewer if the dataset isn't main`() { + scenarioSaved.datasetList!!.removeLast() + listOf(ROLE_NONE, ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).forEach { role -> + var linkedDataset = + datasetApiService.createDataset( + organizationSaved.id!!, makeDataset(isMain = false, default = role)) + scenarioSaved.datasetList!!.add(linkedDataset.id!!) + linkedDataset = + datasetApiService.createDataset( + organizationSaved.id!!, makeDataset(isMain = true, default = role)) + scenarioSaved.datasetList!!.add(linkedDataset.id!!) + } + scenarioSaved = + scenarioApiService.updateScenario( + organizationSaved.id!!, workspaceSaved.id!!, scenarioSaved.id!!, scenarioSaved) + + scenarioApiService.setScenarioDefaultSecurity( + organizationSaved.id!!, workspaceSaved.id!!, scenarioSaved.id!!, ScenarioRole(ROLE_EDITOR)) + + verify(exactly = 1) { + datasetApiService.setDatasetDefaultSecurity(any(), any(), DatasetRole(ROLE_VIEWER)) + } + } + + @Test + fun `when stopping sharing a scenario, the dataset default security should be set to none it's not higher than viewer and the dataset isn't main`() { + scenarioSaved.datasetList!!.removeLast() + listOf(ROLE_NONE, ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).forEach { role -> + var linkedDataset = + datasetApiService.createDataset( + organizationSaved.id!!, makeDataset(isMain = false, default = role)) + scenarioSaved.datasetList!!.add(linkedDataset.id!!) + linkedDataset = + datasetApiService.createDataset( + organizationSaved.id!!, makeDataset(isMain = true, default = role)) + scenarioSaved.datasetList!!.add(linkedDataset.id!!) + } + scenarioSaved = + scenarioApiService.updateScenario( + organizationSaved.id!!, workspaceSaved.id!!, scenarioSaved.id!!, scenarioSaved) + + scenarioApiService.setScenarioDefaultSecurity( + organizationSaved.id!!, workspaceSaved.id!!, scenarioSaved.id!!, ScenarioRole(ROLE_NONE)) + + verify(exactly = 1) { + datasetApiService.setDatasetDefaultSecurity(any(), any(), DatasetRole(ROLE_NONE)) + } + } + private fun makeWorkspaceEventHubInfo(eventHubAvailable: Boolean): WorkspaceEventHubInfo { return WorkspaceEventHubInfo( eventHubNamespace = "eventHubNamespace", @@ -1284,7 +1335,8 @@ class ScenarioServiceIntegrationTest : CsmRedisTestBase() { name: String = "name", connector: Connector = connectorSaved, sourceType: DatasetSourceType = DatasetSourceType.Twincache, - isMain: Boolean = true + isMain: Boolean = true, + default: String = ROLE_NONE ): Dataset { return Dataset( name = name, @@ -1301,7 +1353,7 @@ class ScenarioServiceIntegrationTest : CsmRedisTestBase() { main = isMain, security = DatasetSecurity( - default = ROLE_NONE, + default = default, accessControlList = mutableListOf( DatasetAccessControl(id = CONNECTED_ADMIN_USER, role = ROLE_ADMIN)))) @@ -1397,6 +1449,7 @@ class ScenarioServiceIntegrationTest : CsmRedisTestBase() { name: String = "name", datasetList: MutableList? = mutableListOf(), parentId: String? = null, + defaultSecurity: String = ROLE_NONE, userName: String = "roleName", role: String = ROLE_USER, validationStatus: ScenarioValidationStatus = ScenarioValidationStatus.Draft, @@ -1416,7 +1469,7 @@ class ScenarioServiceIntegrationTest : CsmRedisTestBase() { parametersValues = parametersValues, security = ScenarioSecurity( - ROLE_NONE, + defaultSecurity, mutableListOf( ScenarioAccessControl(CONNECTED_ADMIN_USER, ROLE_ADMIN), ScenarioAccessControl(userName, role)))) diff --git a/scenario/src/main/kotlin/com/cosmotech/scenario/service/ScenarioServiceImpl.kt b/scenario/src/main/kotlin/com/cosmotech/scenario/service/ScenarioServiceImpl.kt index 88f20e7ae..d91206ad9 100644 --- a/scenario/src/main/kotlin/com/cosmotech/scenario/service/ScenarioServiceImpl.kt +++ b/scenario/src/main/kotlin/com/cosmotech/scenario/service/ScenarioServiceImpl.kt @@ -27,6 +27,7 @@ import com.cosmotech.api.rbac.PERMISSION_DELETE import com.cosmotech.api.rbac.PERMISSION_READ_SECURITY import com.cosmotech.api.rbac.PERMISSION_WRITE import com.cosmotech.api.rbac.PERMISSION_WRITE_SECURITY +import com.cosmotech.api.rbac.ROLE_NONE import com.cosmotech.api.rbac.ROLE_USER import com.cosmotech.api.rbac.ROLE_VALIDATOR import com.cosmotech.api.rbac.ROLE_VIEWER @@ -40,6 +41,7 @@ import com.cosmotech.api.utils.findAllPaginated import com.cosmotech.api.utils.getCurrentAccountIdentifier import com.cosmotech.api.utils.getCurrentAuthenticatedUserName import com.cosmotech.dataset.DatasetApiServiceInterface +import com.cosmotech.dataset.domain.DatasetRole import com.cosmotech.dataset.domain.IngestionStatusEnum import com.cosmotech.dataset.domain.SubDatasetGraphQuery import com.cosmotech.dataset.service.getRbac @@ -996,6 +998,7 @@ internal class ScenarioServiceImpl( csmRbac.setDefault(scenario.getRbac(), scenarioRole.role, scenarioPermissions) scenario.setRbac(rbacSecurity) upsertScenarioData(scenario) + setLinkedDatasetDefaultSecurity(organizationId, scenario, scenarioRole.role) return scenario.security as ScenarioSecurity } @@ -1158,6 +1161,27 @@ internal class ScenarioServiceImpl( } } + fun setLinkedDatasetDefaultSecurity( + organizationId: String, + scenario: Scenario, + scenarioRole: String + ) { + var datasetRole = ROLE_NONE + if (scenarioRole != ROLE_NONE) datasetRole = ROLE_VIEWER + scenario.datasetList!!.forEach { datasetId -> + val dataset = datasetService.findDatasetById(organizationId, datasetId) + // We do not want to lower the default security if it's higher than viewer + if (dataset.security!!.default != ROLE_NONE && datasetRole == ROLE_VIEWER) return Unit + if (dataset.security!!.default != ROLE_NONE && datasetRole == ROLE_NONE) return Unit + // Filter on dataset copy (because we do not want to update main dataset as it can be shared + // between scenarios) + if (dataset.main != true) { + datasetService.setDatasetDefaultSecurity( + organizationId, datasetId, DatasetRole(datasetRole)) + } + } + } + override fun getScenarioSecurityUsers( organizationId: String, workspaceId: String, From 3ad3b88c372a2a8f46e009b7faca311a7e8ab13d Mon Sep 17 00:00:00 2001 From: Leopold Cramer Date: Mon, 24 Nov 2025 17:20:55 +0100 Subject: [PATCH 2/2] feat: apply PR review comments --- .../dataset/DatasetApiServiceInterface.kt | 8 ++++ .../dataset/service/DatasetServiceImpl.kt | 6 +++ .../service/RunnerServiceIntegrationTest.kt | 46 +++++++++++-------- .../runner/service/RunnerServiceRBACTest.kt | 21 ++++++++- .../cosmotech/runner/service/RunnerService.kt | 37 +++++++-------- .../service/ScenarioServiceIntegrationTest.kt | 42 +++++++++-------- .../service/ScenarioServiceRBACTest.kt | 19 +++++++- .../scenario/service/ScenarioServiceImpl.kt | 17 +++---- 8 files changed, 123 insertions(+), 73 deletions(-) diff --git a/dataset/src/main/kotlin/com/cosmotech/dataset/DatasetApiServiceInterface.kt b/dataset/src/main/kotlin/com/cosmotech/dataset/DatasetApiServiceInterface.kt index fbf47104d..eda437a47 100644 --- a/dataset/src/main/kotlin/com/cosmotech/dataset/DatasetApiServiceInterface.kt +++ b/dataset/src/main/kotlin/com/cosmotech/dataset/DatasetApiServiceInterface.kt @@ -43,4 +43,12 @@ interface DatasetApiServiceInterface : DatasetApiService { identity: String, role: String ): DatasetAccessControl + + /** + * Update the default security of the dataset passed in parameter + * @param organizationId an organization id + * @param dataset a dataset to update + * @param role new dataset role + */ + fun updateDefaultSecurity(organizationId: String, dataset: Dataset, role: String) } diff --git a/dataset/src/main/kotlin/com/cosmotech/dataset/service/DatasetServiceImpl.kt b/dataset/src/main/kotlin/com/cosmotech/dataset/service/DatasetServiceImpl.kt index 9cd087824..7d7a68d1b 100644 --- a/dataset/src/main/kotlin/com/cosmotech/dataset/service/DatasetServiceImpl.kt +++ b/dataset/src/main/kotlin/com/cosmotech/dataset/service/DatasetServiceImpl.kt @@ -1064,6 +1064,12 @@ class DatasetServiceImpl( return dataset.security as DatasetSecurity } + override fun updateDefaultSecurity(organizationId: String, dataset: Dataset, role: String) { + val rbacSecurity = csmRbac.setDefault(dataset.getRbac(), role) + dataset.setRbac(rbacSecurity) + datasetRepository.save(dataset) + } + override fun addDatasetAccessControl( organizationId: String, datasetId: String, diff --git a/runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceIntegrationTest.kt b/runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceIntegrationTest.kt index 74d2dec3a..03de261fa 100644 --- a/runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceIntegrationTest.kt +++ b/runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceIntegrationTest.kt @@ -20,7 +20,7 @@ import com.cosmotech.api.utils.getCurrentAuthenticatedUserName import com.cosmotech.connector.api.ConnectorApiService import com.cosmotech.connector.domain.Connector import com.cosmotech.connector.domain.IoTypesEnum -import com.cosmotech.dataset.api.DatasetApiService +import com.cosmotech.dataset.DatasetApiServiceInterface import com.cosmotech.dataset.domain.* import com.cosmotech.dataset.repository.DatasetRepository import com.cosmotech.organization.api.OrganizationApiService @@ -39,6 +39,7 @@ import com.cosmotech.workspace.domain.WorkspaceSolution import com.ninjasquad.springmockk.SpykBean import com.redis.om.spring.RediSearchIndexer import com.redis.testcontainers.RedisStackContainer +import io.mockk.clearAllMocks import io.mockk.every import io.mockk.junit5.MockKExtension import io.mockk.mockkStatic @@ -80,7 +81,7 @@ class RunnerServiceIntegrationTest : CsmRedisTestBase() { @Autowired lateinit var rediSearchIndexer: RediSearchIndexer @Autowired lateinit var connectorApiService: ConnectorApiService @Autowired lateinit var organizationApiService: OrganizationApiService - @SpykBean @Autowired lateinit var datasetApiService: DatasetApiService + @SpykBean @Autowired lateinit var datasetApiService: DatasetApiServiceInterface @Autowired lateinit var datasetRepository: DatasetRepository @Autowired lateinit var solutionApiService: SolutionApiService @Autowired lateinit var workspaceApiService: WorkspaceApiService @@ -128,6 +129,7 @@ class RunnerServiceIntegrationTest : CsmRedisTestBase() { @BeforeEach fun setUp() { + clearAllMocks() mockkStatic("com.cosmotech.api.utils.SecurityUtilsKt") every { getCurrentAccountIdentifier(any()) } returns CONNECTED_ADMIN_USER every { getCurrentAuthenticatedUserName(csmPlatformProperties) } returns "test.user" @@ -1099,51 +1101,55 @@ class RunnerServiceIntegrationTest : CsmRedisTestBase() { @Test fun `when sharing a runner, the linked dataset default security should be set to at least viewer if the dataset isn't main`() { runnerSaved.datasetList!!.removeLast() + val linkedDatasets = mutableListOf() listOf(ROLE_NONE, ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).forEach { role -> - var linkedDataset = + linkedDatasets.add( datasetApiService.createDataset( - organizationSaved.id!!, makeDataset(isMain = false, default = role)) - runnerSaved.datasetList!!.add(linkedDataset.id!!) - linkedDataset = + organizationSaved.id!!, makeDataset(isMain = false, default = role))) + linkedDatasets.add( datasetApiService.createDataset( - organizationSaved.id!!, makeDataset(isMain = true, default = role)) - runnerSaved.datasetList!!.add(linkedDataset.id!!) + organizationSaved.id!!, makeDataset(isMain = true, default = role))) } + linkedDatasets.forEach { runnerSaved.datasetList!!.add(it.id!!) } runnerSaved = runnerApiService.updateRunner( organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!, runnerSaved) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returnsMany + linkedDatasets runnerApiService.setRunnerDefaultSecurity( organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!, RunnerRole(ROLE_EDITOR)) - verify(exactly = 1) { - datasetApiService.setDatasetDefaultSecurity(any(), any(), DatasetRole(ROLE_VIEWER)) - } + // Only one result correspond, the dataset with default security none and main=false + verify(exactly = 1) { datasetApiService.updateDefaultSecurity(any(), any(), ROLE_VIEWER) } } @Test fun `when stopping sharing a scenario, the dataset default security should be set to none it's not higher than viewer and the dataset isn't main`() { runnerSaved.datasetList!!.removeLast() + val linkedDatasets = mutableListOf() listOf(ROLE_NONE, ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).forEach { role -> - var linkedDataset = + linkedDatasets.add( datasetApiService.createDataset( - organizationSaved.id!!, makeDataset(isMain = false, default = role)) - runnerSaved.datasetList!!.add(linkedDataset.id!!) - linkedDataset = + organizationSaved.id!!, makeDataset(isMain = false, default = role))) + linkedDatasets.add( datasetApiService.createDataset( - organizationSaved.id!!, makeDataset(isMain = true, default = role)) - runnerSaved.datasetList!!.add(linkedDataset.id!!) + organizationSaved.id!!, makeDataset(isMain = true, default = role))) } + linkedDatasets.forEach { runnerSaved.datasetList!!.add(it.id!!) } runnerSaved = runnerApiService.updateRunner( organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!, runnerSaved) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returnsMany + linkedDatasets runnerApiService.setRunnerDefaultSecurity( organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!, RunnerRole(ROLE_NONE)) - verify(exactly = 1) { - datasetApiService.setDatasetDefaultSecurity(any(), any(), DatasetRole(ROLE_NONE)) - } + // Only one result correspond, the dataset with default security viewer and main=false + verify(exactly = 1) { datasetApiService.updateDefaultSecurity(any(), any(), ROLE_NONE) } } private fun makeConnector(name: String = "name"): Connector { diff --git a/runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceRBACTest.kt b/runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceRBACTest.kt index 482122452..20f8ef9ca 100644 --- a/runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceRBACTest.kt +++ b/runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceRBACTest.kt @@ -25,7 +25,7 @@ import com.cosmotech.api.utils.getCurrentAuthenticatedUserName import com.cosmotech.connector.api.ConnectorApiService import com.cosmotech.connector.domain.Connector import com.cosmotech.connector.domain.IoTypesEnum -import com.cosmotech.dataset.api.DatasetApiService +import com.cosmotech.dataset.DatasetApiServiceInterface import com.cosmotech.dataset.domain.Dataset import com.cosmotech.dataset.domain.DatasetAccessControl import com.cosmotech.dataset.domain.DatasetConnector @@ -56,6 +56,7 @@ import com.cosmotech.workspace.domain.WorkspaceSolution import com.ninjasquad.springmockk.SpykBean import com.redis.om.spring.RediSearchIndexer import com.redis.testcontainers.RedisStackContainer +import io.mockk.clearAllMocks import io.mockk.every import io.mockk.junit5.MockKExtension import io.mockk.mockk @@ -96,7 +97,7 @@ class RunnerServiceRBACTest : CsmRedisTestBase() { @Autowired lateinit var rediSearchIndexer: RediSearchIndexer @Autowired lateinit var connectorApiService: ConnectorApiService @Autowired lateinit var organizationApiService: OrganizationApiService - @SpykBean @Autowired lateinit var datasetApiService: DatasetApiService + @SpykBean @Autowired lateinit var datasetApiService: DatasetApiServiceInterface @Autowired lateinit var solutionApiService: SolutionApiService @Autowired lateinit var workspaceApiService: WorkspaceApiService @Autowired lateinit var runnerApiService: RunnerApiService @@ -106,6 +107,7 @@ class RunnerServiceRBACTest : CsmRedisTestBase() { @BeforeEach fun setUp() { + clearAllMocks() mockkStatic("com.cosmotech.api.utils.SecurityUtilsKt") every { getCurrentAccountIdentifier(any()) } returns CONNECTED_ADMIN_USER every { getCurrentAuthenticatedUserName(csmPlatformProperties) } returns "test.user" @@ -2369,6 +2371,9 @@ class RunnerServiceRBACTest : CsmRedisTestBase() { datasetRepository.save( datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS }) materializeTwingraph(datasetSaved) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns + datasetSaved val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN) val solutionSaved = solutionApiService.createSolution(organizationSaved.id!!, solution) @@ -2440,6 +2445,9 @@ class RunnerServiceRBACTest : CsmRedisTestBase() { datasetRepository.save( datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS }) materializeTwingraph(datasetSaved) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns + datasetSaved val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN) val solutionSaved = solutionApiService.createSolution(organizationSaved.id!!, solution) @@ -2511,6 +2519,9 @@ class RunnerServiceRBACTest : CsmRedisTestBase() { datasetRepository.save( datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS }) materializeTwingraph(datasetSaved) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns + datasetSaved val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, role) val solutionSaved = solutionApiService.createSolution(organizationSaved.id!!, solution) @@ -2581,6 +2592,9 @@ class RunnerServiceRBACTest : CsmRedisTestBase() { datasetSaved = datasetRepository.save( datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS }) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns + datasetSaved val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN) val solutionSaved = solutionApiService.createSolution(organizationSaved.id!!, solution) @@ -2647,6 +2661,9 @@ class RunnerServiceRBACTest : CsmRedisTestBase() { datasetSaved = datasetRepository.save( datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS }) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns + datasetSaved val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN) val solutionSaved = solutionApiService.createSolution(organizationSaved.id!!, solution) diff --git a/runner/src/main/kotlin/com/cosmotech/runner/service/RunnerService.kt b/runner/src/main/kotlin/com/cosmotech/runner/service/RunnerService.kt index 6ea55577d..df9c2bd49 100644 --- a/runner/src/main/kotlin/com/cosmotech/runner/service/RunnerService.kt +++ b/runner/src/main/kotlin/com/cosmotech/runner/service/RunnerService.kt @@ -27,7 +27,6 @@ import com.cosmotech.api.utils.getCurrentAuthenticatedRoles import com.cosmotech.api.utils.getCurrentAuthenticatedUserName import com.cosmotech.dataset.DatasetApiServiceInterface import com.cosmotech.dataset.domain.Dataset -import com.cosmotech.dataset.domain.DatasetRole import com.cosmotech.dataset.service.getRbac import com.cosmotech.organization.OrganizationApiServiceInterface import com.cosmotech.organization.domain.Organization @@ -508,33 +507,29 @@ class RunnerService( // create a rbacSecurity object from runner Rbac by changing default value val rbacSecurity = csmRbac.setDefault(this.getRbacSecurity(), role, this.roleDefinition) this.setRbacSecurity(rbacSecurity) - // this.runner.datasetList!! - // .mapNotNull { - // datasetApiService.findByOrganizationIdAndDatasetId(this.runner.organizationId!!, - // it) - // } - // .forEach { dataset -> updateLinkedDatasetDefaultSecurity(role) - // } } private fun updateLinkedDatasetDefaultSecurity(role: String) { - var datasetRole = ROLE_NONE - if (role != ROLE_NONE) datasetRole = ROLE_VIEWER + var datasetRole = ROLE_VIEWER + if (role == ROLE_NONE) datasetRole = ROLE_NONE this.runner.datasetList!!.forEach { datasetId -> val linkedDataset = - datasetApiService.findDatasetById(this.runner.organizationId!!, datasetId) - // We do not want to lower the default security if it's higher than viewer - if (linkedDataset.security!!.default != ROLE_NONE && datasetRole == ROLE_VIEWER) - return@forEach Unit - if (linkedDataset.security!!.default != ROLE_NONE && datasetRole == ROLE_NONE) - return@forEach Unit + datasetApiService.findByOrganizationIdAndDatasetId( + this.runner.organizationId!!, datasetId) + if (linkedDataset!!.main == true) return@forEach + // If the dataset default security is different from none, + // Then it's already viewer or higher and doesn't need to change + if (datasetRole == ROLE_VIEWER && linkedDataset.security!!.default != ROLE_NONE) + return@forEach + // If the dataset default security is different from viewer, + // Then it's either already none or higher than viewer and doesn't need to change + if (datasetRole == ROLE_NONE && linkedDataset.security!!.default != ROLE_VIEWER) + return@forEach // Filter on dataset copy (because we do not want to update main dataset as it can be shared - // between scenarios) - if (linkedDataset.main != true) { - datasetApiService.setDatasetDefaultSecurity( - this.runner.organizationId!!, datasetId, DatasetRole(datasetRole)) - } + // between runners) + datasetApiService.updateDefaultSecurity( + this.runner.organizationId!!, linkedDataset, datasetRole) } } } diff --git a/scenario/src/integrationTest/kotlin/com/cosmotech/scenario/service/ScenarioServiceIntegrationTest.kt b/scenario/src/integrationTest/kotlin/com/cosmotech/scenario/service/ScenarioServiceIntegrationTest.kt index 48e56bd41..c42c1a7a5 100644 --- a/scenario/src/integrationTest/kotlin/com/cosmotech/scenario/service/ScenarioServiceIntegrationTest.kt +++ b/scenario/src/integrationTest/kotlin/com/cosmotech/scenario/service/ScenarioServiceIntegrationTest.kt @@ -50,6 +50,7 @@ import com.ninjasquad.springmockk.MockkBean import com.ninjasquad.springmockk.SpykBean import com.redis.om.spring.RediSearchIndexer import com.redis.testcontainers.RedisStackContainer +import io.mockk.clearAllMocks import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.junit5.MockKExtension @@ -143,6 +144,7 @@ class ScenarioServiceIntegrationTest : CsmRedisTestBase() { @BeforeEach fun setUp() { + clearAllMocks() mockkStatic("com.cosmotech.api.utils.SecurityUtilsKt") every { getCurrentAccountIdentifier(any()) } returns CONNECTED_ADMIN_USER every { getCurrentAuthenticatedUserName(csmPlatformProperties) } returns "test.user" @@ -1261,51 +1263,55 @@ class ScenarioServiceIntegrationTest : CsmRedisTestBase() { @Test fun `when sharing a scenario, the linked dataset default security should be set to at least viewer if the dataset isn't main`() { scenarioSaved.datasetList!!.removeLast() + val linkedDatasets = mutableListOf() listOf(ROLE_NONE, ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).forEach { role -> - var linkedDataset = + linkedDatasets.add( datasetApiService.createDataset( - organizationSaved.id!!, makeDataset(isMain = false, default = role)) - scenarioSaved.datasetList!!.add(linkedDataset.id!!) - linkedDataset = + organizationSaved.id!!, makeDataset(isMain = false, default = role))) + linkedDatasets.add( datasetApiService.createDataset( - organizationSaved.id!!, makeDataset(isMain = true, default = role)) - scenarioSaved.datasetList!!.add(linkedDataset.id!!) + organizationSaved.id!!, makeDataset(isMain = true, default = role))) } + linkedDatasets.forEach { scenarioSaved.datasetList!!.add(it.id!!) } scenarioSaved = scenarioApiService.updateScenario( organizationSaved.id!!, workspaceSaved.id!!, scenarioSaved.id!!, scenarioSaved) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returnsMany + linkedDatasets scenarioApiService.setScenarioDefaultSecurity( organizationSaved.id!!, workspaceSaved.id!!, scenarioSaved.id!!, ScenarioRole(ROLE_EDITOR)) - verify(exactly = 1) { - datasetApiService.setDatasetDefaultSecurity(any(), any(), DatasetRole(ROLE_VIEWER)) - } + // Only one result correspond, the dataset with default security none and main=false + verify(exactly = 1) { datasetApiService.updateDefaultSecurity(any(), any(), ROLE_VIEWER) } } @Test fun `when stopping sharing a scenario, the dataset default security should be set to none it's not higher than viewer and the dataset isn't main`() { scenarioSaved.datasetList!!.removeLast() + val linkedDatasets = mutableListOf() listOf(ROLE_NONE, ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).forEach { role -> - var linkedDataset = + linkedDatasets.add( datasetApiService.createDataset( - organizationSaved.id!!, makeDataset(isMain = false, default = role)) - scenarioSaved.datasetList!!.add(linkedDataset.id!!) - linkedDataset = + organizationSaved.id!!, makeDataset(isMain = false, default = role))) + linkedDatasets.add( datasetApiService.createDataset( - organizationSaved.id!!, makeDataset(isMain = true, default = role)) - scenarioSaved.datasetList!!.add(linkedDataset.id!!) + organizationSaved.id!!, makeDataset(isMain = true, default = role))) } + linkedDatasets.forEach { scenarioSaved.datasetList!!.add(it.id!!) } scenarioSaved = scenarioApiService.updateScenario( organizationSaved.id!!, workspaceSaved.id!!, scenarioSaved.id!!, scenarioSaved) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returnsMany + linkedDatasets scenarioApiService.setScenarioDefaultSecurity( organizationSaved.id!!, workspaceSaved.id!!, scenarioSaved.id!!, ScenarioRole(ROLE_NONE)) - verify(exactly = 1) { - datasetApiService.setDatasetDefaultSecurity(any(), any(), DatasetRole(ROLE_NONE)) - } + // Only one result correspond, the dataset with default security viewer and main=false + verify(exactly = 1) { datasetApiService.updateDefaultSecurity(any(), any(), ROLE_NONE) } } private fun makeWorkspaceEventHubInfo(eventHubAvailable: Boolean): WorkspaceEventHubInfo { diff --git a/scenario/src/integrationTest/kotlin/com/cosmotech/scenario/service/ScenarioServiceRBACTest.kt b/scenario/src/integrationTest/kotlin/com/cosmotech/scenario/service/ScenarioServiceRBACTest.kt index 746f8e666..50014de91 100644 --- a/scenario/src/integrationTest/kotlin/com/cosmotech/scenario/service/ScenarioServiceRBACTest.kt +++ b/scenario/src/integrationTest/kotlin/com/cosmotech/scenario/service/ScenarioServiceRBACTest.kt @@ -27,7 +27,7 @@ import com.cosmotech.api.utils.getCurrentAuthenticatedUserName import com.cosmotech.connector.api.ConnectorApiService import com.cosmotech.connector.domain.Connector import com.cosmotech.connector.domain.IoTypesEnum -import com.cosmotech.dataset.api.DatasetApiService +import com.cosmotech.dataset.DatasetApiServiceInterface import com.cosmotech.dataset.domain.* import com.cosmotech.dataset.repository.DatasetRepository import com.cosmotech.organization.api.OrganizationApiService @@ -100,7 +100,7 @@ class ScenarioServiceRBACTest : CsmRedisTestBase() { @Autowired lateinit var rediSearchIndexer: RediSearchIndexer @Autowired lateinit var connectorApiService: ConnectorApiService @Autowired lateinit var organizationApiService: OrganizationApiService - @SpykBean @Autowired lateinit var datasetApiService: DatasetApiService + @SpykBean @Autowired lateinit var datasetApiService: DatasetApiServiceInterface @Autowired lateinit var solutionApiService: SolutionApiService @Autowired lateinit var workspaceApiService: WorkspaceApiService @Autowired lateinit var scenarioApiService: ScenarioApiService @@ -4887,6 +4887,9 @@ class ScenarioServiceRBACTest : CsmRedisTestBase() { datasetRepository.save( datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS }) materializeTwingraph(datasetSaved) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns + datasetSaved val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN) val solutionSaved = solutionApiService.createSolution(organizationSaved.id!!, solution) @@ -4959,6 +4962,9 @@ class ScenarioServiceRBACTest : CsmRedisTestBase() { datasetRepository.save( datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS }) materializeTwingraph(datasetSaved) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns + datasetSaved val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN) val solutionSaved = solutionApiService.createSolution(organizationSaved.id!!, solution) @@ -5031,6 +5037,9 @@ class ScenarioServiceRBACTest : CsmRedisTestBase() { datasetRepository.save( datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS }) materializeTwingraph(datasetSaved) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns + datasetSaved val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, role) val solutionSaved = solutionApiService.createSolution(organizationSaved.id!!, solution) @@ -5102,6 +5111,9 @@ class ScenarioServiceRBACTest : CsmRedisTestBase() { datasetSaved = datasetRepository.save( datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS }) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns + datasetSaved val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN) val solutionSaved = solutionApiService.createSolution(organizationSaved.id!!, solution) @@ -5169,6 +5181,9 @@ class ScenarioServiceRBACTest : CsmRedisTestBase() { datasetSaved = datasetRepository.save( datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS }) + every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit + every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns + datasetSaved val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN) val solutionSaved = solutionApiService.createSolution(organizationSaved.id!!, solution) diff --git a/scenario/src/main/kotlin/com/cosmotech/scenario/service/ScenarioServiceImpl.kt b/scenario/src/main/kotlin/com/cosmotech/scenario/service/ScenarioServiceImpl.kt index d91206ad9..a6f0f443c 100644 --- a/scenario/src/main/kotlin/com/cosmotech/scenario/service/ScenarioServiceImpl.kt +++ b/scenario/src/main/kotlin/com/cosmotech/scenario/service/ScenarioServiceImpl.kt @@ -41,7 +41,6 @@ import com.cosmotech.api.utils.findAllPaginated import com.cosmotech.api.utils.getCurrentAccountIdentifier import com.cosmotech.api.utils.getCurrentAuthenticatedUserName import com.cosmotech.dataset.DatasetApiServiceInterface -import com.cosmotech.dataset.domain.DatasetRole import com.cosmotech.dataset.domain.IngestionStatusEnum import com.cosmotech.dataset.domain.SubDatasetGraphQuery import com.cosmotech.dataset.service.getRbac @@ -1166,19 +1165,17 @@ internal class ScenarioServiceImpl( scenario: Scenario, scenarioRole: String ) { - var datasetRole = ROLE_NONE - if (scenarioRole != ROLE_NONE) datasetRole = ROLE_VIEWER + var datasetRole = ROLE_VIEWER + if (scenarioRole == ROLE_NONE) datasetRole = ROLE_NONE scenario.datasetList!!.forEach { datasetId -> - val dataset = datasetService.findDatasetById(organizationId, datasetId) + val dataset = datasetService.findByOrganizationIdAndDatasetId(organizationId, datasetId) + if (dataset!!.main == true) return@forEach // We do not want to lower the default security if it's higher than viewer - if (dataset.security!!.default != ROLE_NONE && datasetRole == ROLE_VIEWER) return Unit - if (dataset.security!!.default != ROLE_NONE && datasetRole == ROLE_NONE) return Unit + if (datasetRole == ROLE_VIEWER && dataset.security!!.default != ROLE_NONE) return@forEach + if (datasetRole == ROLE_NONE && dataset.security!!.default != ROLE_VIEWER) return@forEach // Filter on dataset copy (because we do not want to update main dataset as it can be shared // between scenarios) - if (dataset.main != true) { - datasetService.setDatasetDefaultSecurity( - organizationId, datasetId, DatasetRole(datasetRole)) - } + datasetService.updateDefaultSecurity(organizationId, dataset, datasetRole) } }