Skip to content

Commit 91c7acb

Browse files
Leopold-CramerLeopold-Cramer
committed
feat: apply PR review comments
1 parent c4b05c7 commit 91c7acb

File tree

8 files changed

+103
-60
lines changed

8 files changed

+103
-60
lines changed

‎dataset/src/main/kotlin/com/cosmotech/dataset/DatasetApiServiceInterface.kt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,12 @@ interface DatasetApiServiceInterface : DatasetApiService {
4343
identity: String,
4444
role: String
4545
): DatasetAccessControl
46+
47+
/**
48+
* Update the default security of the dataset passed in parameter
49+
* @param organizationId an organization id
50+
* @param dataset a dataset to update
51+
* @param role new dataset role
52+
*/
53+
fun updateDefaultSecurity(organizationId: String, dataset: Dataset, role: String)
4654
}

‎dataset/src/main/kotlin/com/cosmotech/dataset/service/DatasetServiceImpl.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,6 +1064,12 @@ class DatasetServiceImpl(
10641064
return dataset.security as DatasetSecurity
10651065
}
10661066

1067+
override fun updateDefaultSecurity(organizationId: String, dataset: Dataset, role: String) {
1068+
val rbacSecurity = csmRbac.setDefault(dataset.getRbac(), role)
1069+
dataset.setRbac(rbacSecurity)
1070+
datasetRepository.save(dataset)
1071+
}
1072+
10671073
override fun addDatasetAccessControl(
10681074
organizationId: String,
10691075
datasetId: String,

‎runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceIntegrationTest.kt‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import com.cosmotech.api.utils.getCurrentAuthenticatedUserName
2020
import com.cosmotech.connector.api.ConnectorApiService
2121
import com.cosmotech.connector.domain.Connector
2222
import com.cosmotech.connector.domain.IoTypesEnum
23-
import com.cosmotech.dataset.api.DatasetApiService
23+
import com.cosmotech.dataset.DatasetApiServiceInterface
2424
import com.cosmotech.dataset.domain.*
2525
import com.cosmotech.dataset.repository.DatasetRepository
2626
import com.cosmotech.organization.api.OrganizationApiService
@@ -39,6 +39,7 @@ import com.cosmotech.workspace.domain.WorkspaceSolution
3939
import com.ninjasquad.springmockk.SpykBean
4040
import com.redis.om.spring.RediSearchIndexer
4141
import com.redis.testcontainers.RedisStackContainer
42+
import io.mockk.clearAllMocks
4243
import io.mockk.every
4344
import io.mockk.junit5.MockKExtension
4445
import io.mockk.mockkStatic
@@ -80,7 +81,7 @@ class RunnerServiceIntegrationTest : CsmRedisTestBase() {
8081
@Autowired lateinit var rediSearchIndexer: RediSearchIndexer
8182
@Autowired lateinit var connectorApiService: ConnectorApiService
8283
@Autowired lateinit var organizationApiService: OrganizationApiService
83-
@SpykBean @Autowired lateinit var datasetApiService: DatasetApiService
84+
@SpykBean @Autowired lateinit var datasetApiService: DatasetApiServiceInterface
8485
@Autowired lateinit var datasetRepository: DatasetRepository
8586
@Autowired lateinit var solutionApiService: SolutionApiService
8687
@Autowired lateinit var workspaceApiService: WorkspaceApiService
@@ -128,6 +129,7 @@ class RunnerServiceIntegrationTest : CsmRedisTestBase() {
128129

129130
@BeforeEach
130131
fun setUp() {
132+
clearAllMocks()
131133
mockkStatic("com.cosmotech.api.utils.SecurityUtilsKt")
132134
every { getCurrentAccountIdentifier(any()) } returns CONNECTED_ADMIN_USER
133135
every { getCurrentAuthenticatedUserName(csmPlatformProperties) } returns "test.user"
@@ -1099,51 +1101,53 @@ class RunnerServiceIntegrationTest : CsmRedisTestBase() {
10991101
@Test
11001102
fun `when sharing a runner, the linked dataset default security should be set to at least viewer if the dataset isn't main`() {
11011103
runnerSaved.datasetList!!.removeLast()
1104+
val linkedDatasets = mutableListOf<Dataset>()
11021105
listOf(ROLE_NONE, ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).forEach { role ->
1103-
var linkedDataset =
1106+
linkedDatasets.add(
11041107
datasetApiService.createDataset(
1105-
organizationSaved.id!!, makeDataset(isMain = false, default = role))
1106-
runnerSaved.datasetList!!.add(linkedDataset.id!!)
1107-
linkedDataset =
1108+
organizationSaved.id!!, makeDataset(isMain = false, default = role)))
1109+
linkedDatasets.add(
11081110
datasetApiService.createDataset(
1109-
organizationSaved.id!!, makeDataset(isMain = true, default = role))
1110-
runnerSaved.datasetList!!.add(linkedDataset.id!!)
1111+
organizationSaved.id!!, makeDataset(isMain = true, default = role)))
11111112
}
1113+
linkedDatasets.forEach { runnerSaved.datasetList!!.add(it.id!!) }
11121114
runnerSaved =
11131115
runnerApiService.updateRunner(
11141116
organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!, runnerSaved)
1117+
every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit
1118+
every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returnsMany
1119+
linkedDatasets
11151120

11161121
runnerApiService.setRunnerDefaultSecurity(
11171122
organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!, RunnerRole(ROLE_EDITOR))
11181123

1119-
verify(exactly = 1) {
1120-
datasetApiService.setDatasetDefaultSecurity(any(), any(), DatasetRole(ROLE_VIEWER))
1121-
}
1124+
verify(exactly = 1) { datasetApiService.updateDefaultSecurity(any(), any(), any()) }
11221125
}
11231126

11241127
@Test
11251128
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`() {
11261129
runnerSaved.datasetList!!.removeLast()
1130+
val linkedDatasets = mutableListOf<Dataset>()
11271131
listOf(ROLE_NONE, ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).forEach { role ->
1128-
var linkedDataset =
1132+
linkedDatasets.add(
11291133
datasetApiService.createDataset(
1130-
organizationSaved.id!!, makeDataset(isMain = false, default = role))
1131-
runnerSaved.datasetList!!.add(linkedDataset.id!!)
1132-
linkedDataset =
1134+
organizationSaved.id!!, makeDataset(isMain = false, default = role)))
1135+
linkedDatasets.add(
11331136
datasetApiService.createDataset(
1134-
organizationSaved.id!!, makeDataset(isMain = true, default = role))
1135-
runnerSaved.datasetList!!.add(linkedDataset.id!!)
1137+
organizationSaved.id!!, makeDataset(isMain = true, default = role)))
11361138
}
1139+
linkedDatasets.forEach { runnerSaved.datasetList!!.add(it.id!!) }
11371140
runnerSaved =
11381141
runnerApiService.updateRunner(
11391142
organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!, runnerSaved)
1143+
every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit
1144+
every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returnsMany
1145+
linkedDatasets
11401146

11411147
runnerApiService.setRunnerDefaultSecurity(
11421148
organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!, RunnerRole(ROLE_NONE))
11431149

1144-
verify(exactly = 1) {
1145-
datasetApiService.setDatasetDefaultSecurity(any(), any(), DatasetRole(ROLE_NONE))
1146-
}
1150+
verify { datasetApiService.updateDefaultSecurity(any(), any(), any()) }
11471151
}
11481152

11491153
private fun makeConnector(name: String = "name"): Connector {

‎runner/src/integrationTest/kotlin/com/cosmotech/runner/service/RunnerServiceRBACTest.kt‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import com.cosmotech.api.utils.getCurrentAuthenticatedUserName
2525
import com.cosmotech.connector.api.ConnectorApiService
2626
import com.cosmotech.connector.domain.Connector
2727
import com.cosmotech.connector.domain.IoTypesEnum
28-
import com.cosmotech.dataset.api.DatasetApiService
28+
import com.cosmotech.dataset.DatasetApiServiceInterface
2929
import com.cosmotech.dataset.domain.Dataset
3030
import com.cosmotech.dataset.domain.DatasetAccessControl
3131
import com.cosmotech.dataset.domain.DatasetConnector
@@ -96,7 +96,7 @@ class RunnerServiceRBACTest : CsmRedisTestBase() {
9696
@Autowired lateinit var rediSearchIndexer: RediSearchIndexer
9797
@Autowired lateinit var connectorApiService: ConnectorApiService
9898
@Autowired lateinit var organizationApiService: OrganizationApiService
99-
@SpykBean @Autowired lateinit var datasetApiService: DatasetApiService
99+
@SpykBean @Autowired lateinit var datasetApiService: DatasetApiServiceInterface
100100
@Autowired lateinit var solutionApiService: SolutionApiService
101101
@Autowired lateinit var workspaceApiService: WorkspaceApiService
102102
@Autowired lateinit var runnerApiService: RunnerApiService
@@ -2369,6 +2369,9 @@ class RunnerServiceRBACTest : CsmRedisTestBase() {
23692369
datasetRepository.save(
23702370
datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS })
23712371
materializeTwingraph(datasetSaved)
2372+
every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit
2373+
every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns
2374+
datasetSaved
23722375
val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN)
23732376
val solutionSaved =
23742377
solutionApiService.createSolution(organizationSaved.id!!, solution)
@@ -2440,6 +2443,9 @@ class RunnerServiceRBACTest : CsmRedisTestBase() {
24402443
datasetRepository.save(
24412444
datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS })
24422445
materializeTwingraph(datasetSaved)
2446+
every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit
2447+
every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns
2448+
datasetSaved
24432449
val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN)
24442450
val solutionSaved =
24452451
solutionApiService.createSolution(organizationSaved.id!!, solution)
@@ -2511,6 +2517,9 @@ class RunnerServiceRBACTest : CsmRedisTestBase() {
25112517
datasetRepository.save(
25122518
datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS })
25132519
materializeTwingraph(datasetSaved)
2520+
every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit
2521+
every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns
2522+
datasetSaved
25142523
val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, role)
25152524
val solutionSaved =
25162525
solutionApiService.createSolution(organizationSaved.id!!, solution)
@@ -2581,6 +2590,9 @@ class RunnerServiceRBACTest : CsmRedisTestBase() {
25812590
datasetSaved =
25822591
datasetRepository.save(
25832592
datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS })
2593+
every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit
2594+
every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns
2595+
datasetSaved
25842596
val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN)
25852597
val solutionSaved =
25862598
solutionApiService.createSolution(organizationSaved.id!!, solution)
@@ -2647,6 +2659,9 @@ class RunnerServiceRBACTest : CsmRedisTestBase() {
26472659
datasetSaved =
26482660
datasetRepository.save(
26492661
datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS })
2662+
every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit
2663+
every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returns
2664+
datasetSaved
26502665
val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN)
26512666
val solutionSaved =
26522667
solutionApiService.createSolution(organizationSaved.id!!, solution)

‎runner/src/main/kotlin/com/cosmotech/runner/service/RunnerService.kt‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import com.cosmotech.api.utils.getCurrentAuthenticatedRoles
2727
import com.cosmotech.api.utils.getCurrentAuthenticatedUserName
2828
import com.cosmotech.dataset.DatasetApiServiceInterface
2929
import com.cosmotech.dataset.domain.Dataset
30-
import com.cosmotech.dataset.domain.DatasetRole
3130
import com.cosmotech.dataset.service.getRbac
3231
import com.cosmotech.organization.OrganizationApiServiceInterface
3332
import com.cosmotech.organization.domain.Organization
@@ -508,32 +507,26 @@ class RunnerService(
508507
// create a rbacSecurity object from runner Rbac by changing default value
509508
val rbacSecurity = csmRbac.setDefault(this.getRbacSecurity(), role, this.roleDefinition)
510509
this.setRbacSecurity(rbacSecurity)
511-
// this.runner.datasetList!!
512-
// .mapNotNull {
513-
// datasetApiService.findByOrganizationIdAndDatasetId(this.runner.organizationId!!,
514-
// it)
515-
// }
516-
// .forEach { dataset ->
517510
updateLinkedDatasetDefaultSecurity(role)
518-
// }
519511
}
520512

521513
private fun updateLinkedDatasetDefaultSecurity(role: String) {
522514
var datasetRole = ROLE_NONE
523515
if (role != ROLE_NONE) datasetRole = ROLE_VIEWER
524516
this.runner.datasetList!!.forEach { datasetId ->
525517
val linkedDataset =
526-
datasetApiService.findDatasetById(this.runner.organizationId!!, datasetId)
518+
datasetApiService.findByOrganizationIdAndDatasetId(
519+
this.runner.organizationId!!, datasetId)
527520
// We do not want to lower the default security if it's higher than viewer
528-
if (linkedDataset.security!!.default != ROLE_NONE && datasetRole == ROLE_VIEWER)
521+
if (linkedDataset!!.security!!.default != ROLE_NONE && datasetRole == ROLE_VIEWER)
529522
return@forEach Unit
530523
if (linkedDataset.security!!.default != ROLE_NONE && datasetRole == ROLE_NONE)
531524
return@forEach Unit
532525
// Filter on dataset copy (because we do not want to update main dataset as it can be shared
533-
// between scenarios)
526+
// between runners)
534527
if (linkedDataset.main != true) {
535-
datasetApiService.setDatasetDefaultSecurity(
536-
this.runner.organizationId!!, datasetId, DatasetRole(datasetRole))
528+
datasetApiService.updateDefaultSecurity(
529+
this.runner.organizationId!!, linkedDataset, datasetRole)
537530
}
538531
}
539532
}

‎scenario/src/integrationTest/kotlin/com/cosmotech/scenario/service/ScenarioServiceIntegrationTest.kt‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import com.ninjasquad.springmockk.MockkBean
5050
import com.ninjasquad.springmockk.SpykBean
5151
import com.redis.om.spring.RediSearchIndexer
5252
import com.redis.testcontainers.RedisStackContainer
53+
import io.mockk.clearAllMocks
5354
import io.mockk.every
5455
import io.mockk.impl.annotations.MockK
5556
import io.mockk.junit5.MockKExtension
@@ -143,6 +144,7 @@ class ScenarioServiceIntegrationTest : CsmRedisTestBase() {
143144

144145
@BeforeEach
145146
fun setUp() {
147+
clearAllMocks()
146148
mockkStatic("com.cosmotech.api.utils.SecurityUtilsKt")
147149
every { getCurrentAccountIdentifier(any()) } returns CONNECTED_ADMIN_USER
148150
every { getCurrentAuthenticatedUserName(csmPlatformProperties) } returns "test.user"
@@ -1261,51 +1263,53 @@ class ScenarioServiceIntegrationTest : CsmRedisTestBase() {
12611263
@Test
12621264
fun `when sharing a scenario, the linked dataset default security should be set to at least viewer if the dataset isn't main`() {
12631265
scenarioSaved.datasetList!!.removeLast()
1266+
val linkedDatasets = mutableListOf<Dataset>()
12641267
listOf(ROLE_NONE, ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).forEach { role ->
1265-
var linkedDataset =
1268+
linkedDatasets.add(
12661269
datasetApiService.createDataset(
1267-
organizationSaved.id!!, makeDataset(isMain = false, default = role))
1268-
scenarioSaved.datasetList!!.add(linkedDataset.id!!)
1269-
linkedDataset =
1270+
organizationSaved.id!!, makeDataset(isMain = false, default = role)))
1271+
linkedDatasets.add(
12701272
datasetApiService.createDataset(
1271-
organizationSaved.id!!, makeDataset(isMain = true, default = role))
1272-
scenarioSaved.datasetList!!.add(linkedDataset.id!!)
1273+
organizationSaved.id!!, makeDataset(isMain = true, default = role)))
12731274
}
1275+
linkedDatasets.forEach { scenarioSaved.datasetList!!.add(it.id!!) }
12741276
scenarioSaved =
12751277
scenarioApiService.updateScenario(
12761278
organizationSaved.id!!, workspaceSaved.id!!, scenarioSaved.id!!, scenarioSaved)
1279+
every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit
1280+
every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returnsMany
1281+
linkedDatasets
12771282

12781283
scenarioApiService.setScenarioDefaultSecurity(
12791284
organizationSaved.id!!, workspaceSaved.id!!, scenarioSaved.id!!, ScenarioRole(ROLE_EDITOR))
12801285

1281-
verify(exactly = 1) {
1282-
datasetApiService.setDatasetDefaultSecurity(any(), any(), DatasetRole(ROLE_VIEWER))
1283-
}
1286+
verify(exactly = 1) { datasetApiService.updateDefaultSecurity(any(), any(), any()) }
12841287
}
12851288

12861289
@Test
12871290
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`() {
12881291
scenarioSaved.datasetList!!.removeLast()
1292+
val linkedDatasets = mutableListOf<Dataset>()
12891293
listOf(ROLE_NONE, ROLE_VIEWER, ROLE_USER, ROLE_EDITOR, ROLE_ADMIN).forEach { role ->
1290-
var linkedDataset =
1294+
linkedDatasets.add(
12911295
datasetApiService.createDataset(
1292-
organizationSaved.id!!, makeDataset(isMain = false, default = role))
1293-
scenarioSaved.datasetList!!.add(linkedDataset.id!!)
1294-
linkedDataset =
1296+
organizationSaved.id!!, makeDataset(isMain = false, default = role)))
1297+
linkedDatasets.add(
12951298
datasetApiService.createDataset(
1296-
organizationSaved.id!!, makeDataset(isMain = true, default = role))
1297-
scenarioSaved.datasetList!!.add(linkedDataset.id!!)
1299+
organizationSaved.id!!, makeDataset(isMain = true, default = role)))
12981300
}
1301+
linkedDatasets.forEach { scenarioSaved.datasetList!!.add(it.id!!) }
12991302
scenarioSaved =
13001303
scenarioApiService.updateScenario(
13011304
organizationSaved.id!!, workspaceSaved.id!!, scenarioSaved.id!!, scenarioSaved)
1305+
every { datasetApiService.updateDefaultSecurity(any(), any(), any()) } returns Unit
1306+
every { datasetApiService.findByOrganizationIdAndDatasetId(any(), any()) } returnsMany
1307+
linkedDatasets
13021308

13031309
scenarioApiService.setScenarioDefaultSecurity(
13041310
organizationSaved.id!!, workspaceSaved.id!!, scenarioSaved.id!!, ScenarioRole(ROLE_NONE))
13051311

1306-
verify(exactly = 1) {
1307-
datasetApiService.setDatasetDefaultSecurity(any(), any(), DatasetRole(ROLE_NONE))
1308-
}
1312+
verify { datasetApiService.updateDefaultSecurity(any(), any(), any()) }
13091313
}
13101314

13111315
private fun makeWorkspaceEventHubInfo(eventHubAvailable: Boolean): WorkspaceEventHubInfo {

0 commit comments

Comments
 (0)