Skip to content

Commit c946d04

Browse files
committed
[PROD-13730] Add pre-condition checks for add/update runner ACL
1 parent 66b4e08 commit c946d04

File tree

3 files changed

+51
-6
lines changed

3 files changed

+51
-6
lines changed

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

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import com.cosmotech.organization.domain.OrganizationAccessControl
3636
import com.cosmotech.organization.domain.OrganizationSecurity
3737
import com.cosmotech.runner.RunnerApiServiceInterface
3838
import com.cosmotech.runner.domain.*
39+
import com.cosmotech.runner.domain.RunnerRole
3940
import com.cosmotech.solution.api.SolutionApiService
4041
import com.cosmotech.solution.domain.*
4142
import com.cosmotech.workspace.api.WorkspaceApiService
@@ -524,7 +525,7 @@ class RunnerServiceIntegrationTest : CsmRedisTestBase() {
524525
}
525526

526527
@Test
527-
fun `access control list shouldn't contain more than one time each user on ACL addition`() {
528+
fun `access control list can't add an existing user`() {
528529
organizationSaved =
529530
organizationApiService.registerOrganization(makeOrganization("organization"))
530531
solutionSaved = solutionApiService.createSolution(organizationSaved.id!!, makeSolution())
@@ -538,11 +539,44 @@ class RunnerServiceIntegrationTest : CsmRedisTestBase() {
538539
organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!)
539540
assertEquals(2, runnerSavedSecurityUsers.size)
540541

541-
runnerApiService.addRunnerAccessControl(
542-
organizationSaved.id!!,
543-
workspaceSaved.id!!,
544-
runnerSaved.id!!,
545-
RunnerAccessControl(defaultName, ROLE_EDITOR))
542+
assertThrows<IllegalArgumentException> {
543+
runnerApiService.addRunnerAccessControl(
544+
organizationSaved.id!!,
545+
workspaceSaved.id!!,
546+
runnerSaved.id!!,
547+
RunnerAccessControl(defaultName, ROLE_EDITOR))
548+
}
549+
550+
val runnerSecurityUsers =
551+
runnerApiService.getRunnerSecurityUsers(
552+
organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!)
553+
assertEquals(2, runnerSecurityUsers.size)
554+
assert(runnerSavedSecurityUsers == runnerSecurityUsers)
555+
}
556+
557+
@Test
558+
fun `access control list can't update a non-existing user`() {
559+
organizationSaved =
560+
organizationApiService.registerOrganization(makeOrganization("organization"))
561+
solutionSaved = solutionApiService.createSolution(organizationSaved.id!!, makeSolution())
562+
workspaceSaved = workspaceApiService.createWorkspace(organizationSaved.id!!, makeWorkspace())
563+
val workingRunner = makeRunner()
564+
runnerSaved =
565+
runnerApiService.createRunner(organizationSaved.id!!, workspaceSaved.id!!, workingRunner)
566+
567+
val runnerSavedSecurityUsers =
568+
runnerApiService.getRunnerSecurityUsers(
569+
organizationSaved.id!!, workspaceSaved.id!!, runnerSaved.id!!)
570+
assertEquals(2, runnerSavedSecurityUsers.size)
571+
572+
assertThrows<CsmResourceNotFoundException> {
573+
runnerApiService.updateRunnerAccessControl(
574+
organizationSaved.id!!,
575+
workspaceSaved.id!!,
576+
runnerSaved.id!!,
577+
"invalid user",
578+
RunnerRole(ROLE_VIEWER))
579+
}
546580

547581
val runnerSecurityUsers =
548582
runnerApiService.getRunnerSecurityUsers(

runner/src/main/kotlin/com/cosmotech/runner/service/RunnerApiServiceImpl.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,11 @@ internal class RunnerApiServiceImpl(
114114
val runnerInstance =
115115
runnerService.getInstance(runnerId).userHasPermission(PERMISSION_WRITE_SECURITY)
116116

117+
val users = getRunnerSecurityUsers(organizationId, workspaceId, runnerId)
118+
if (users.contains(runnerAccessControl.id)) {
119+
throw IllegalArgumentException("User is already in this Runner security")
120+
}
121+
117122
runnerInstance.setAccessControl(runnerAccessControl)
118123
runnerService.saveInstance(runnerInstance)
119124

@@ -143,6 +148,7 @@ internal class RunnerApiServiceImpl(
143148
val runnerService = getRunnerService().inOrganization(organizationId).inWorkspace(workspaceId)
144149
val runnerInstance =
145150
runnerService.getInstance(runnerId).userHasPermission(PERMISSION_WRITE_SECURITY)
151+
runnerInstance.checkUserExists(identityId)
146152

147153
val runnerAccessControl = RunnerAccessControl(identityId, runnerRole.role)
148154
runnerInstance.setAccessControl(runnerAccessControl)

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,11 @@ class RunnerService(
264264
this.removeAccessControlToDatasets(userId)
265265
}
266266

267+
fun checkUserExists(userId: String) {
268+
csmRbac.checkUserExists(
269+
runner.getRbac(), userId, "User '$userId' not found in runner ${runner.id}")
270+
}
271+
267272
private fun getRbacSecurity(): RbacSecurity {
268273
return extractRbacSecurity(this.runner)!!
269274
}

0 commit comments

Comments
 (0)