Skip to content

Commit 6163af4

Browse files
jreynard-codejreynard-code
committed
Fix [SDCOSMO-2543]: getVerifiedWorkspace is now based on organizationId
1 parent 7b23e56 commit 6163af4

File tree

3 files changed

+55
-23
lines changed

3 files changed

+55
-23
lines changed

‎workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceIntegrationTest.kt

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import kotlin.test.assertNull
5454
import kotlin.test.assertTrue
5555
import org.junit.jupiter.api.BeforeEach
5656
import org.junit.jupiter.api.Test
57+
import org.junit.jupiter.api.assertDoesNotThrow
5758
import org.junit.jupiter.api.assertThrows
5859
import org.junit.jupiter.api.extension.ExtendWith
5960
import org.junit.runner.RunWith
@@ -636,6 +637,35 @@ class WorkspaceServiceIntegrationTest : CsmRedisTestBase() {
636637
assertEquals(createdWorkspace.webApp, updatedWorkspace.webApp)
637638
}
638639

640+
641+
@Test
642+
fun `SDCOSMO-2543 - test workspace is not accessible from another organization`() {
643+
val workspace = makeWorkspaceCreateRequest()
644+
workspaceSaved = workspaceApiService.createWorkspace(organizationSaved.id, workspace)
645+
646+
647+
val anotherOrganization = makeOrganizationCreateRequest("Another Organization")
648+
val anotherOrganizationSaved = organizationApiService.createOrganization(anotherOrganization)
649+
650+
assertDoesNotThrow {
651+
workspaceApiService.getWorkspace(
652+
organizationSaved.id,
653+
workspaceSaved.id
654+
)
655+
}
656+
657+
val exception = assertThrows<CsmResourceNotFoundException> {
658+
workspaceApiService.getWorkspace(
659+
anotherOrganizationSaved.id,
660+
workspaceSaved.id
661+
)
662+
}
663+
664+
assertEquals("Workspace ${workspaceSaved.id} not found in organization ${anotherOrganizationSaved.id}",
665+
exception.message)
666+
667+
}
668+
639669
fun makeOrganizationCreateRequest(
640670
id: String,
641671
userName: String = CONNECTED_ADMIN_USER,

‎workspace/src/main/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImpl.kt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,8 +454,10 @@ internal class WorkspaceServiceImpl(
454454
): Workspace {
455455
organizationService.getVerifiedOrganization(organizationId)
456456
val workspace =
457-
workspaceRepository.findByIdOrNull(workspaceId)
458-
?: throw CsmResourceNotFoundException("Workspace $workspaceId does not exist!")
457+
workspaceRepository.findBy(organizationId,workspaceId)
458+
.orElseThrow{
459+
CsmResourceNotFoundException("Workspace $workspaceId not found in organization $organizationId")
460+
}
459461
csmRbac.verify(workspace.security.toGenericSecurity(workspaceId), requiredPermission)
460462
return workspace
461463
}

‎workspace/src/test/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImplTests.kt

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ class WorkspaceServiceImplTests {
150150
every { workspace.id } returns WORKSPACE_ID
151151
every { workspace.name } returns "test workspace"
152152
every { workspace.security } returns WorkspaceSecurity(ROLE_ADMIN, mutableListOf())
153-
every { workspaceRepository.findByIdOrNull(any()) } returns workspace
153+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(workspace)
154154

155155
val file = mockk<Resource>(relaxed = true)
156156
every { file.filename } returns "my_file.txt"
@@ -171,7 +171,7 @@ class WorkspaceServiceImplTests {
171171
every { workspace.security } returns WorkspaceSecurity(ROLE_ADMIN, mutableListOf())
172172
every { organizationService.getVerifiedOrganization(ORGANIZATION_ID) } returns
173173
mockk<Organization>()
174-
every { workspaceRepository.findByIdOrNull(any()) } returns workspace
174+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(workspace)
175175

176176
val file = mockk<Resource>(relaxed = true)
177177
every { file.filename } returns "my_file.txt"
@@ -190,7 +190,7 @@ class WorkspaceServiceImplTests {
190190
every { workspace.id } returns WORKSPACE_ID
191191
every { workspace.name } returns "test workspace"
192192
every { workspace.security } returns WorkspaceSecurity(ROLE_ADMIN, mutableListOf())
193-
every { workspaceRepository.findByIdOrNull(any()) } returns workspace
193+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(workspace)
194194

195195
val file = mockk<Resource>(relaxed = true)
196196
every { file.filename } returns "my_file.txt"
@@ -212,7 +212,7 @@ class WorkspaceServiceImplTests {
212212
every { workspace.id } returns WORKSPACE_ID
213213
every { workspace.name } returns "test workspace"
214214
every { workspace.security } returns WorkspaceSecurity(ROLE_ADMIN, mutableListOf())
215-
every { workspaceRepository.findByIdOrNull(any()) } returns workspace
215+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(workspace)
216216

217217
val file = mockk<Resource>(relaxed = true)
218218
every { file.filename } returns "my_file.txt"
@@ -233,7 +233,7 @@ class WorkspaceServiceImplTests {
233233
every { workspace.id } returns WORKSPACE_ID
234234
every { workspace.name } returns "test workspace"
235235
every { workspace.security } returns WorkspaceSecurity(ROLE_ADMIN, mutableListOf())
236-
every { workspaceRepository.findByIdOrNull(any()) } returns workspace
236+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(workspace)
237237

238238
val file = mockk<Resource>(relaxed = true)
239239
every { file.filename } returns "my_file.txt"
@@ -292,7 +292,7 @@ class WorkspaceServiceImplTests {
292292
workspaceName = "my workspace name",
293293
roleName = CONNECTED_DEFAULT_USER,
294294
role = ROLE_ADMIN)
295-
every { workspaceRepository.findByIdOrNull(WORKSPACE_ID) } returns workspace
295+
every { workspaceRepository.findBy(any(),WORKSPACE_ID) } returns Optional.of(workspace)
296296
every { solutionService.getSolution(ORGANIZATION_ID, any()) } throws
297297
CsmResourceNotFoundException("Solution not found")
298298
assertThrows<CsmResourceNotFoundException> {
@@ -320,7 +320,7 @@ class WorkspaceServiceImplTests {
320320
ROLE_NONE to true)
321321
.map { (role, shouldThrow) ->
322322
rbacTest("Test RBAC read workspace: $role", role, shouldThrow) {
323-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
323+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
324324
workspaceServiceImpl.getVerifiedWorkspace(it.organization.id, it.workspace.id)
325325
}
326326
}
@@ -360,7 +360,7 @@ class WorkspaceServiceImplTests {
360360
ROLE_NONE to true)
361361
.map { (role, shouldThrow) ->
362362
rbacTest("Test RBAC delete all workspace files: $role", role, shouldThrow) {
363-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
363+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
364364
workspaceServiceImpl.deleteWorkspaceFiles(it.organization.id, it.workspace.id)
365365
}
366366
}
@@ -377,7 +377,7 @@ class WorkspaceServiceImplTests {
377377
.map { (role, shouldThrow) ->
378378
rbacTest("test RBAC update workspace: $role", role, shouldThrow) {
379379
every { solutionService.getSolution(any(), any()) } returns it.solution
380-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
380+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
381381
every { workspaceRepository.save(any()) } returns it.workspace
382382
workspaceServiceImpl.updateWorkspace(
383383
it.organization.id,
@@ -397,7 +397,7 @@ class WorkspaceServiceImplTests {
397397
ROLE_NONE to true)
398398
.map { (role, shouldThrow) ->
399399
rbacTest("Test RBAC delete workspace: $role", role, shouldThrow) {
400-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
400+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
401401
workspaceServiceImpl.deleteWorkspace(it.organization.id, it.workspace.id)
402402
}
403403
}
@@ -413,7 +413,7 @@ class WorkspaceServiceImplTests {
413413
ROLE_NONE to true)
414414
.map { (role, shouldThrow) ->
415415
rbacTest("Test RBAC delete workspace file: $role", role, shouldThrow) {
416-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
416+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
417417
workspaceServiceImpl.deleteWorkspaceFile(it.organization.id, it.workspace.id, "")
418418
}
419419
}
@@ -429,7 +429,7 @@ class WorkspaceServiceImplTests {
429429
ROLE_NONE to true)
430430
.map { (role, shouldThrow) ->
431431
rbacTest("Test RBAC download workspace file: $role", role, shouldThrow) {
432-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
432+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
433433
val filePath =
434434
Path.of(blobPersistencePath, it.organization.id, it.workspace.id, "name")
435435
Files.createDirectories(filePath.getParent())
@@ -450,7 +450,7 @@ class WorkspaceServiceImplTests {
450450
ROLE_NONE to true)
451451
.map { (role, shouldThrow) ->
452452
rbacTest("Test RBAC upload workspace file: $role", role, shouldThrow) {
453-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
453+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
454454
workspaceServiceImpl.createWorkspaceFile(
455455
it.organization.id, it.workspace.id, mockk(relaxed = true), true, "name")
456456
}
@@ -467,7 +467,7 @@ class WorkspaceServiceImplTests {
467467
ROLE_NONE to true)
468468
.map { (role, shouldThrow) ->
469469
rbacTest("Test RBAC findAllWorkspaceFiles: $role", role, shouldThrow) {
470-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
470+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
471471
workspaceServiceImpl.listWorkspaceFiles(it.organization.id, it.workspace.id)
472472
}
473473
}
@@ -483,7 +483,7 @@ class WorkspaceServiceImplTests {
483483
ROLE_NONE to true)
484484
.map { (role, shouldThrow) ->
485485
rbacTest("Test RBAC get workspace security: $role", role, shouldThrow) {
486-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
486+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
487487
workspaceServiceImpl.getWorkspaceSecurity(it.organization.id, it.workspace.id)
488488
}
489489
}
@@ -499,7 +499,7 @@ class WorkspaceServiceImplTests {
499499
ROLE_NONE to true)
500500
.map { (role, shouldThrow) ->
501501
rbacTest("Test RBAC set workspace default security: $role", role, shouldThrow) {
502-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
502+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
503503
every { workspaceRepository.save(any()) } returns it.workspace
504504
workspaceServiceImpl.updateWorkspaceDefaultSecurity(
505505
it.organization.id, it.workspace.id, WorkspaceRole(ROLE_NONE))
@@ -517,7 +517,7 @@ class WorkspaceServiceImplTests {
517517
ROLE_NONE to true)
518518
.map { (role, shouldThrow) ->
519519
rbacTest("test RBAC get workspace access control: $role", role, shouldThrow) {
520-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
520+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
521521
workspaceServiceImpl.getWorkspaceAccessControl(
522522
it.organization.id, it.workspace.id, CONNECTED_DEFAULT_USER)
523523
}
@@ -535,7 +535,7 @@ class WorkspaceServiceImplTests {
535535
.map { (role, shouldThrow) ->
536536
rbacTest("test RBAC add workspace access control: $role", role, shouldThrow) {
537537
every { workspaceRepository.save(any()) } returns it.workspace
538-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
538+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
539539
workspaceServiceImpl.createWorkspaceAccessControl(
540540
it.organization.id,
541541
it.workspace.id,
@@ -554,7 +554,7 @@ class WorkspaceServiceImplTests {
554554
ROLE_NONE to true)
555555
.map { (role, shouldThrow) ->
556556
rbacTest("test RBAC update workspace access control: $role", role, shouldThrow) {
557-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
557+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
558558
every { workspaceRepository.save(any()) } returns it.workspace
559559
workspaceServiceImpl.updateWorkspaceAccessControl(
560560
it.organization.id,
@@ -575,7 +575,7 @@ class WorkspaceServiceImplTests {
575575
ROLE_NONE to true)
576576
.map { (role, shouldThrow) ->
577577
rbacTest("test RBAC remove workspace access control: $role", role, shouldThrow) {
578-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
578+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
579579
every { workspaceRepository.save(any()) } returns it.workspace
580580
workspaceServiceImpl.deleteWorkspaceAccessControl(
581581
it.organization.id, it.workspace.id, "2$CONNECTED_DEFAULT_USER")
@@ -593,7 +593,7 @@ class WorkspaceServiceImplTests {
593593
ROLE_NONE to true)
594594
.map { (role, shouldThrow) ->
595595
rbacTest("test RBAC get workspace security users: $role", role, shouldThrow) {
596-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
596+
every { workspaceRepository.findBy(any(),any()) } returns Optional.of(it.workspace)
597597
every { workspaceRepository.save(any()) } returns it.workspace
598598
workspaceServiceImpl.listWorkspaceSecurityUsers(it.organization.id, it.workspace.id)
599599
}

0 commit comments

Comments
 (0)