Skip to content

Commit 8f541b9

Browse files
jreynard-codejreynard-code
committed
[RMZ-413] Fixes after cherry pick (from 3.1)
1 parent 5c0bf50 commit 8f541b9

File tree

5 files changed

+97
-120
lines changed

5 files changed

+97
-120
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,7 @@ class DatasetServiceImpl(
579579
val dataset = getVerifiedDataset(organizationId, datasetId, PERMISSION_DELETE)
580580

581581
if (useGraphModule && unifiedJedis.exists(dataset.twingraphId!!)) {
582-
unifiedJedis.del(dataset.twingraphId!!)
582+
unifiedJedis.del(dataset.twingraphId!!)
583583
}
584584

585585
dataset.linkedWorkspaceIdList?.forEach { unlinkWorkspace(organizationId, datasetId, it) }

‎dataset/src/test/kotlin/com/cosmotech/dataset/service/DatasetServiceImplTests.kt‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -413,12 +413,16 @@ class DatasetServiceImplTests {
413413

414414
@Test
415415
fun `deleteDataset do not throw error - rbac is disabled`() {
416-
val dataset = baseDataset().apply { twingraphId = "mytwingraphId" }
416+
val twingraphIdValue = "mytwingraphId"
417+
val dataset = baseDataset().apply { twingraphId = twingraphIdValue }
417418
every { organizationService.getVerifiedOrganization(ORGANIZATION_ID) } returns Organization()
418419
every { datasetRepository.findBy(ORGANIZATION_ID, DATASET_ID) } returns Optional.of(dataset)
420+
every { datasetRepository.delete(dataset) } returns Unit
421+
every { unifiedJedis.exists(twingraphIdValue) } returns true
422+
every { unifiedJedis.del(twingraphIdValue) } returns 1L
419423
every { getCurrentAuthenticatedUserName(csmPlatformProperties) } returns "my.account-tester"
420424
datasetService.deleteDataset(ORGANIZATION_ID, DATASET_ID)
421-
verify(exactly = 1) { datasetRepository.delete(any()) }
425+
verify(exactly = 1) { datasetRepository.delete(dataset) }
422426
}
423427

424428
@Test
@@ -465,20 +469,6 @@ class DatasetServiceImplTests {
465469
verify { unifiedJedis.graphQuery("graphId", graphQuery, 0) }
466470
}
467471

468-
@Test
469-
fun `test processCSV - should create cypher requests by line`() {
470-
val fileName = this::class.java.getResource("/Users.csv")?.file
471-
val file = File(fileName!!)
472-
val query =
473-
DatasetTwinGraphQuery(
474-
"CREATE (:Person {id: toInteger(\$id), name: \$name, rank: toInteger(\$rank), object: \$object})")
475-
val result = TwinGraphBatchResult(0, 0, mutableListOf())
476-
datasetService.processCSVBatch(file.inputStream(), query, result) { result.processedLines++ }
477-
assertEquals(9, result.totalLines)
478-
assertEquals(9, result.processedLines)
479-
assertEquals(0, result.errors.size)
480-
}
481-
482472
@Test
483473
fun `test bulkQueryGraphs as Admin - should call query and set data to Redis`() {
484474
every { getCurrentAuthenticatedRoles(any()) } returns listOf("Platform.Admin")

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -565,8 +565,7 @@ class RunnerServiceIntegrationTest : CsmRedisTestBase() {
565565
RunnerRole(ROLE_EDITOR))
566566
assertEquals(ROLE_EDITOR, runnerAccessControlRegistered.role)
567567

568-
logger.info(
569-
"should let the Access Control as-is cause dataset ACL already contains info for this user")
568+
logger.info("Should not change the datasets access control because ACL already exist on it")
570569
runnerSaved.datasetList!!.forEach {
571570
assertEquals(
572571
ROLE_VIEWER,

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

Lines changed: 77 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ import io.mockk.mockkStatic
6262
import java.util.*
6363
import kotlin.test.Test
6464
import kotlin.test.assertEquals
65+
import kotlin.test.assertTrue
6566
import org.junit.jupiter.api.BeforeEach
6667
import org.junit.jupiter.api.DynamicTest.dynamicTest
6768
import org.junit.jupiter.api.TestFactory
@@ -2770,83 +2771,92 @@ class RunnerServiceRBACTest : CsmRedisTestBase() {
27702771
}
27712772

27722773
@TestFactory
2773-
fun `test Dataset RBAC addRunnerAccessControl`() =
2774+
fun `test Dataset RBAC modification when addRunnerAccessControl is called on a runner`() =
27742775
listOf(
27752776
ROLE_VIEWER,
27762777
ROLE_EDITOR,
27772778
ROLE_VALIDATOR,
2778-
ROLE_USER,
2779-
ROLE_NONE,
27802779
ROLE_ADMIN,
27812780
)
27822781
.map { role ->
2783-
dynamicTest("Test Dataset RBAC addRunnerAccessControl : $role") {
2784-
every { getCurrentAccountIdentifier(any()) } returns CONNECTED_ADMIN_USER
2785-
val connector = makeConnector()
2786-
val connectorSaved = connectorApiService.registerConnector(connector)
2787-
val organization = makeOrganizationWithRole(id = TEST_USER_MAIL, role = ROLE_ADMIN)
2788-
val organizationSaved = organizationApiService.registerOrganization(organization)
2789-
val dataset =
2790-
makeDataset(organizationSaved.id!!, connectorSaved, TEST_USER_MAIL, role)
2791-
var datasetSaved = datasetApiService.createDataset(organizationSaved.id!!, dataset)
2792-
datasetSaved =
2793-
datasetRepository.save(
2794-
datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS })
2795-
every { datasetApiService.createSubDataset(any(), any(), any()) } returns datasetSaved
2796-
val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN)
2797-
val solutionSaved =
2798-
solutionApiService.createSolution(organizationSaved.id!!, solution)
2799-
val workspace =
2800-
makeWorkspaceWithRole(
2801-
organizationSaved.id!!,
2802-
solutionSaved.id!!,
2803-
id = TEST_USER_MAIL,
2804-
role = ROLE_ADMIN)
2805-
val workspaceSaved =
2806-
workspaceApiService.createWorkspace(organizationSaved.id!!, workspace)
2807-
val runner =
2808-
makeRunnerWithRole(
2809-
organizationSaved.id!!,
2810-
workspaceSaved.id!!,
2811-
solutionSaved.id!!,
2812-
mutableListOf(datasetSaved.id!!),
2813-
id = TEST_USER_MAIL,
2814-
role = ROLE_ADMIN)
2815-
val runnerSaved =
2816-
runnerApiService.createRunner(organizationSaved.id!!, workspaceSaved.id!!, runner)
2782+
dynamicTest(
2783+
"Check Dataset RBAC modification " +
2784+
"when addRunnerAccessControl is called on a runner with role : $role") {
2785+
every { getCurrentAccountIdentifier(any()) } returns CONNECTED_ADMIN_USER
2786+
val connector = makeConnector()
2787+
val connectorSaved = connectorApiService.registerConnector(connector)
2788+
val organization =
2789+
makeOrganizationWithRole(id = TEST_USER_MAIL, role = ROLE_ADMIN)
2790+
val organizationSaved = organizationApiService.registerOrganization(organization)
2791+
val dataset =
2792+
makeDataset(organizationSaved.id!!, connectorSaved, TEST_USER_MAIL, role)
2793+
var datasetSaved =
2794+
datasetApiService.createDataset(organizationSaved.id!!, dataset)
2795+
datasetSaved =
2796+
datasetRepository.save(
2797+
datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS })
2798+
every { datasetApiService.createSubDataset(any(), any(), any()) } returns
2799+
datasetSaved
2800+
val solution = makeSolution(organizationSaved.id!!, TEST_USER_MAIL, ROLE_ADMIN)
2801+
val solutionSaved =
2802+
solutionApiService.createSolution(organizationSaved.id!!, solution)
2803+
val workspace =
2804+
makeWorkspaceWithRole(
2805+
organizationSaved.id!!,
2806+
solutionSaved.id!!,
2807+
id = TEST_USER_MAIL,
2808+
role = ROLE_ADMIN)
2809+
val workspaceSaved =
2810+
workspaceApiService.createWorkspace(organizationSaved.id!!, workspace)
2811+
val runner =
2812+
makeRunnerWithRole(
2813+
organizationSaved.id!!,
2814+
workspaceSaved.id!!,
2815+
solutionSaved.id!!,
2816+
mutableListOf(datasetSaved.id!!),
2817+
id = TEST_USER_MAIL,
2818+
role = ROLE_ADMIN)
2819+
val runnerSaved =
2820+
runnerApiService.createRunner(
2821+
organizationSaved.id!!, workspaceSaved.id!!, runner)
28172822

2818-
assertDoesNotThrow {
2819-
assertEquals(
2820-
true,
2821-
datasetSaved.security
2822-
?.accessControlList
2823-
?.filter { datasetAccessControl ->
2824-
datasetAccessControl.id == TEST_USER_MAIL
2825-
}
2826-
?.any { datasetAccessControl -> datasetAccessControl.role == role })
2827-
2828-
runnerApiService.addRunnerAccessControl(
2829-
organizationSaved.id!!,
2830-
workspaceSaved.id!!,
2831-
runnerSaved.id!!,
2832-
RunnerAccessControl(TEST_USER_MAIL, ROLE_ADMIN))
2833-
2834-
val datasetWithUpgradedACL =
2835-
datasetApiService.findDatasetById(organizationSaved.id!!, datasetSaved.id!!)
2836-
assertEquals(
2837-
true,
2838-
datasetWithUpgradedACL.security
2839-
?.accessControlList
2840-
?.filter { datasetAccessControl ->
2841-
datasetAccessControl.id == TEST_USER_MAIL
2842-
}
2843-
?.any { datasetAccessControl -> datasetAccessControl.role == role })
2844-
}
2845-
}
2823+
assertDoesNotThrow {
2824+
assertTrue(
2825+
datasetSaved.security
2826+
?.accessControlList
2827+
?.filter { datasetAccessControl ->
2828+
datasetAccessControl.id == "[email protected]"
2829+
}
2830+
.isNullOrEmpty())
2831+
2832+
runnerApiService.addRunnerAccessControl(
2833+
organizationSaved.id!!,
2834+
workspaceSaved.id!!,
2835+
runnerSaved.id!!,
2836+
RunnerAccessControl("[email protected]", role))
2837+
2838+
val datasetWithUpgradedACL =
2839+
datasetApiService.findDatasetById(organizationSaved.id!!, datasetSaved.id!!)
2840+
var datasetRole = role
2841+
if (role == ROLE_VALIDATOR) {
2842+
datasetRole = ROLE_USER
2843+
}
2844+
assertEquals(
2845+
true,
2846+
datasetWithUpgradedACL.security
2847+
?.accessControlList
2848+
?.filter { datasetAccessControl ->
2849+
datasetAccessControl.id == "[email protected]"
2850+
}
2851+
?.any { datasetAccessControl ->
2852+
datasetAccessControl.role == datasetRole
2853+
})
2854+
}
2855+
}
28462856
}
28472857

28482858
@Test
2849-
fun `test Dataset RBAC addRunnerAccessControl with new ACL entry`() {
2859+
fun `test addRunnerAccessControl when called on a runner with an user that do not exist in Dataset RBAC`() {
28502860
every { getCurrentAccountIdentifier(any()) } returns CONNECTED_ADMIN_USER
28512861
val connector = makeConnector()
28522862
val connectorSaved = connectorApiService.registerConnector(connector)
@@ -2857,8 +2867,7 @@ class RunnerServiceRBACTest : CsmRedisTestBase() {
28572867
organizationSaved.id!!, connectorSaved, id = "[email protected]", role = ROLE_NONE)
28582868
var datasetSaved = datasetApiService.createDataset(organizationSaved.id!!, dataset)
28592869
datasetSaved =
2860-
datasetRepository.save(
2861-
datasetSaved.apply { ingestionStatus = Dataset.IngestionStatus.SUCCESS })
2870+
datasetRepository.save(datasetSaved.apply { ingestionStatus = IngestionStatusEnum.SUCCESS })
28622871
every { datasetApiService.createSubDataset(any(), any(), any()) } returns datasetSaved
28632872

28642873
assertEquals(

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

Lines changed: 12 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@ import com.cosmotech.api.utils.getCurrentAccountIdentifier
2424
import com.cosmotech.api.utils.getCurrentAuthenticatedRoles
2525
import com.cosmotech.api.utils.getCurrentAuthenticatedUserName
2626
import com.cosmotech.dataset.DatasetApiServiceInterface
27-
import com.cosmotech.dataset.domain.DatasetAccessControl
28-
import com.cosmotech.dataset.domain.DatasetRole
27+
import com.cosmotech.dataset.domain.Dataset
2928
import com.cosmotech.dataset.service.getRbac
3029
import com.cosmotech.organization.OrganizationApiServiceInterface
3130
import com.cosmotech.organization.domain.Organization
@@ -249,13 +248,9 @@ class RunnerService(
249248
?.mapNotNull {
250249
datasetApiService.findByOrganizationIdAndDatasetId(organization!!.id!!, it)
251250
}
252-
?.forEach { newDataset ->
251+
?.forEach { dataset ->
253252
this.runner.security?.accessControlList?.forEach { roleDefinition ->
254-
val newDatasetAcl = newDataset.getRbac().accessControlList
255-
if (newDatasetAcl.none { it.id == roleDefinition.id }) {
256-
datasetApiService.addOrUpdateAccessControl(
257-
organization!!.id!!, newDataset, roleDefinition.id, roleDefinition.role)
258-
}
253+
addUserAccessControlOnDataset(dataset, roleDefinition)
259254
}
260255
}
261256
}
@@ -371,11 +366,7 @@ class RunnerService(
371366
this.runner.datasetList!!
372367
.mapNotNull { datasetApiService.findByOrganizationIdAndDatasetId(organizationId, it) }
373368
.forEach { dataset ->
374-
val datasetAcl = dataset.getRbac().accessControlList
375-
if (datasetAcl.none { it.id == userId }) {
376-
datasetApiService.addOrUpdateAccessControl(
377-
organizationId, dataset, userId, datasetRole)
378-
}
369+
addUserAccessControlOnDataset(dataset, RunnerAccessControl(userId, datasetRole))
379370
}
380371
}
381372

@@ -473,26 +464,6 @@ class RunnerService(
473464
return parametersValuesList
474465
}
475466

476-
private fun propagateAccessControlToDatasets(userId: String, role: String) {
477-
this.runner.datasetList?.forEach { datasetId ->
478-
propagateAccessControlToDataset(datasetId, userId, role)
479-
}
480-
}
481-
482-
private fun propagateAccessControlToDataset(datasetId: String, userId: String, role: String) {
483-
val datasetRole = role.takeUnless { it == ROLE_VALIDATOR } ?: ROLE_USER
484-
val organizationId = this.runner.organizationId!!
485-
486-
val datasetUsers = datasetApiService.getDatasetSecurityUsers(organizationId, datasetId)
487-
if (datasetUsers.contains(userId)) {
488-
datasetApiService.updateDatasetAccessControl(
489-
organizationId, datasetId, userId, DatasetRole(datasetRole))
490-
} else {
491-
datasetApiService.addDatasetAccessControl(
492-
organizationId, datasetId, DatasetAccessControl(userId, datasetRole))
493-
}
494-
}
495-
496467
private fun removeAccessControlToDatasets(userId: String) {
497468
val organizationId = this.runner.organizationId!!
498469
this.runner.datasetList?.forEach { datasetId ->
@@ -514,4 +485,12 @@ class RunnerService(
514485
this.setRbacSecurity(rbacSecurity)
515486
}
516487
}
488+
489+
private fun addUserAccessControlOnDataset(dataset: Dataset, roleDefinition: RunnerAccessControl) {
490+
val newDatasetAcl = dataset.getRbac().accessControlList
491+
if (newDatasetAcl.none { it.id == roleDefinition.id }) {
492+
datasetApiService.addOrUpdateAccessControl(
493+
organization!!.id!!, dataset, roleDefinition.id, roleDefinition.role)
494+
}
495+
}
517496
}

0 commit comments

Comments
 (0)