Skip to content

Commit 8838c3b

Browse files
jreynard-codejreynard-code
committed
Fix [SDCOSMO-2543]: getVerifiedWorkspace is now based on organizationId
1 parent abc05f0 commit 8838c3b

File tree

3 files changed

+48
-24
lines changed

3 files changed

+48
-24
lines changed

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import kotlin.test.assertNull
4949
import kotlin.test.assertTrue
5050
import org.junit.jupiter.api.BeforeEach
5151
import org.junit.jupiter.api.Test
52+
import org.junit.jupiter.api.assertDoesNotThrow
5253
import org.junit.jupiter.api.assertThrows
5354
import org.junit.jupiter.api.extension.ExtendWith
5455
import org.junit.runner.RunWith
@@ -500,6 +501,28 @@ class WorkspaceServiceIntegrationTest : CsmRedisTestBase() {
500501
}
501502
}
502503

504+
@Test
505+
fun `SDCOSMO-2543 - test workspace is not accessible from another organization`() {
506+
val workspace = makeWorkspace()
507+
workspaceSaved = workspaceApiService.createWorkspace(organizationSaved.id!!, workspace)
508+
509+
val anotherOrganization = makeOrganization("Another Organization")
510+
val anotherOrganizationSaved = organizationApiService.registerOrganization(anotherOrganization)
511+
512+
assertDoesNotThrow {
513+
workspaceApiService.findWorkspaceById(organizationSaved.id!!, workspaceSaved.id!!)
514+
}
515+
516+
val exception =
517+
assertThrows<CsmResourceNotFoundException> {
518+
workspaceApiService.findWorkspaceById(anotherOrganizationSaved.id!!, workspaceSaved.id!!)
519+
}
520+
521+
assertEquals(
522+
"Workspace ${workspaceSaved.id} not found in organization ${anotherOrganizationSaved.id}",
523+
exception.message)
524+
}
525+
503526
fun makeOrganization(
504527
id: String,
505528
userName: String = CONNECTED_ADMIN_USER,

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ import org.springframework.context.annotation.Scope
5757
import org.springframework.context.event.EventListener
5858
import org.springframework.core.io.Resource
5959
import org.springframework.data.domain.Pageable
60-
import org.springframework.data.repository.findByIdOrNull
6160
import org.springframework.scheduling.annotation.Async
6261
import org.springframework.stereotype.Service
6362

@@ -458,8 +457,10 @@ internal class WorkspaceServiceImpl(
458457
): Workspace {
459458
organizationService.getVerifiedOrganization(organizationId)
460459
val workspace =
461-
workspaceRepository.findByIdOrNull(workspaceId)
462-
?: throw CsmResourceNotFoundException("Workspace $workspaceId does not exist!")
460+
workspaceRepository.findBy(organizationId, workspaceId).orElseThrow {
461+
CsmResourceNotFoundException(
462+
"Workspace $workspaceId not found in organization $organizationId")
463+
}
463464
csmRbac.verify(workspace.getRbac(), requiredPermission)
464465
return workspace
465466
}

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

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ class WorkspaceServiceImplTests {
141141
every { workspace.id } returns WORKSPACE_ID
142142
every { workspace.name } returns "test workspace"
143143
every { workspace.security } returns WorkspaceSecurity(ROLE_ADMIN, mutableListOf())
144-
every { workspaceRepository.findByIdOrNull(any()) } returns workspace
144+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(workspace)
145145

146146
val file = mockk<Resource>(relaxed = true)
147147
every { file.filename } returns "my_file.txt"
@@ -173,7 +173,7 @@ class WorkspaceServiceImplTests {
173173
every { workspace.security } returns WorkspaceSecurity(ROLE_ADMIN, mutableListOf())
174174
every { organizationService.getVerifiedOrganization(ORGANIZATION_ID) } returns
175175
mockk<Organization>()
176-
every { workspaceRepository.findByIdOrNull(any()) } returns workspace
176+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(workspace)
177177

178178
val file = mockk<Resource>(relaxed = true)
179179
every { file.filename } returns "my_file.txt"
@@ -203,7 +203,7 @@ class WorkspaceServiceImplTests {
203203
every { workspace.id } returns WORKSPACE_ID
204204
every { workspace.name } returns "test workspace"
205205
every { workspace.security } returns WorkspaceSecurity(ROLE_ADMIN, mutableListOf())
206-
every { workspaceRepository.findByIdOrNull(any()) } returns workspace
206+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(workspace)
207207

208208
val file = mockk<Resource>(relaxed = true)
209209
every { file.filename } returns "my_file.txt"
@@ -235,7 +235,7 @@ class WorkspaceServiceImplTests {
235235
every { workspace.id } returns WORKSPACE_ID
236236
every { workspace.name } returns "test workspace"
237237
every { workspace.security } returns WorkspaceSecurity(ROLE_ADMIN, mutableListOf())
238-
every { workspaceRepository.findByIdOrNull(any()) } returns workspace
238+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(workspace)
239239

240240
val file = mockk<Resource>(relaxed = true)
241241
every { file.filename } returns "my_file.txt"
@@ -267,7 +267,7 @@ class WorkspaceServiceImplTests {
267267
every { workspace.id } returns WORKSPACE_ID
268268
every { workspace.name } returns "test workspace"
269269
every { workspace.security } returns WorkspaceSecurity(ROLE_ADMIN, mutableListOf())
270-
every { workspaceRepository.findByIdOrNull(any()) } returns workspace
270+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(workspace)
271271

272272
val file = mockk<Resource>(relaxed = true)
273273
every { file.filename } returns "my_file.txt"
@@ -356,7 +356,7 @@ class WorkspaceServiceImplTests {
356356
name = "my workspace name",
357357
solution = WorkspaceSolution(solutionId = "SOL-my-solution-id"),
358358
security = WorkspaceSecurity(ROLE_ADMIN, mutableListOf()))
359-
every { workspaceRepository.findByIdOrNull(WORKSPACE_ID) } returns workspace
359+
every { workspaceRepository.findBy(any(), WORKSPACE_ID) } returns Optional.of(workspace)
360360
every { solutionService.findSolutionById(ORGANIZATION_ID, any()) } throws
361361
CsmResourceNotFoundException("Solution not found")
362362
assertThrows<CsmResourceNotFoundException> {
@@ -383,7 +383,7 @@ class WorkspaceServiceImplTests {
383383
ROLE_NONE to true)
384384
.map { (role, shouldThrow) ->
385385
rbacTest("Test RBAC read workspace: $role", role, shouldThrow) {
386-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
386+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
387387
workspaceServiceImpl.getVerifiedWorkspace(it.organization.id!!, it.workspace.id!!)
388388
}
389389
}
@@ -420,7 +420,7 @@ class WorkspaceServiceImplTests {
420420
ROLE_NONE to true)
421421
.map { (role, shouldThrow) ->
422422
rbacTest("Test RBAC delete all workspace files: $role", role, shouldThrow) {
423-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
423+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
424424
workspaceServiceImpl.deleteAllWorkspaceFiles(it.organization.id!!, it.workspace.id!!)
425425
}
426426
}
@@ -437,7 +437,7 @@ class WorkspaceServiceImplTests {
437437
.map { (role, shouldThrow) ->
438438
rbacTest("test RBAC update workspace: $role", role, shouldThrow) {
439439
every { solutionService.findSolutionById(any(), any()) } returns it.solution
440-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
440+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
441441
every { workspaceRepository.save(any()) } returns it.workspace
442442
workspaceServiceImpl.updateWorkspace(
443443
it.organization.id!!, it.workspace.id!!, it.workspace)
@@ -455,7 +455,7 @@ class WorkspaceServiceImplTests {
455455
ROLE_NONE to true)
456456
.map { (role, shouldThrow) ->
457457
rbacTest("Test RBAC delete workspace: $role", role, shouldThrow) {
458-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
458+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
459459
every { secretManager.deleteSecret(any(), any()) } returns Unit
460460
workspaceServiceImpl.deleteWorkspace(it.organization.id!!, it.workspace.id!!)
461461
}
@@ -472,7 +472,7 @@ class WorkspaceServiceImplTests {
472472
ROLE_NONE to true)
473473
.map { (role, shouldThrow) ->
474474
rbacTest("Test RBAC delete workspace file: $role", role, shouldThrow) {
475-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
475+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
476476
every {
477477
azureStorageBlobServiceClient
478478
.getBlobContainerClient(any())
@@ -494,7 +494,7 @@ class WorkspaceServiceImplTests {
494494
ROLE_NONE to true)
495495
.map { (role, shouldThrow) ->
496496
rbacTest("Test RBAC download workspace file: $role", role, shouldThrow) {
497-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
497+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
498498
every { azureStorageBlobProtocolResolver.getResource(any()) } returns mockk()
499499
workspaceServiceImpl.downloadWorkspaceFile(
500500
it.organization.id!!, it.workspace.id!!, "")
@@ -512,7 +512,7 @@ class WorkspaceServiceImplTests {
512512
ROLE_NONE to true)
513513
.map { (role, shouldThrow) ->
514514
rbacTest("Test RBAC upload workspace file: $role", role, shouldThrow) {
515-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
515+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
516516
every { resource.filename } returns "name"
517517
every {
518518
azureStorageBlobServiceClient
@@ -538,7 +538,7 @@ class WorkspaceServiceImplTests {
538538
ROLE_NONE to true)
539539
.map { (role, shouldThrow) ->
540540
rbacTest("Test RBAC findAllWorkspaceFiles: $role", role, shouldThrow) {
541-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
541+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
542542
every { azureStorageBlobServiceClient.getBlobContainerClient(any()) } returns mockk()
543543
every { azureStorageBlobProtocolResolver.getResources(any()) } returns arrayOf()
544544
workspaceServiceImpl.findAllWorkspaceFiles(it.organization.id!!, it.workspace.id!!)
@@ -556,7 +556,7 @@ class WorkspaceServiceImplTests {
556556
ROLE_NONE to true)
557557
.map { (role, shouldThrow) ->
558558
rbacTest("Test RBAC get workspace security: $role", role, shouldThrow) {
559-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
559+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
560560
workspaceServiceImpl.getWorkspaceSecurity(it.organization.id!!, it.workspace.id!!)
561561
}
562562
}
@@ -572,7 +572,7 @@ class WorkspaceServiceImplTests {
572572
ROLE_NONE to true)
573573
.map { (role, shouldThrow) ->
574574
rbacTest("Test RBAC set workspace default security: $role", role, shouldThrow) {
575-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
575+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
576576
every { workspaceRepository.save(any()) } returns it.workspace
577577
workspaceServiceImpl.setWorkspaceDefaultSecurity(
578578
it.organization.id!!, it.workspace.id!!, WorkspaceRole(ROLE_NONE))
@@ -590,7 +590,7 @@ class WorkspaceServiceImplTests {
590590
ROLE_NONE to true)
591591
.map { (role, shouldThrow) ->
592592
rbacTest("test RBAC get workspace access control: $role", role, shouldThrow) {
593-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
593+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
594594
workspaceServiceImpl.getWorkspaceAccessControl(
595595
it.organization.id!!, it.workspace.id!!, CONNECTED_DEFAULT_USER)
596596
}
@@ -608,7 +608,7 @@ class WorkspaceServiceImplTests {
608608
.map { (role, shouldThrow) ->
609609
rbacTest("test RBAC add workspace access control: $role", role, shouldThrow) {
610610
every { workspaceRepository.save(any()) } returns it.workspace
611-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
611+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
612612
workspaceServiceImpl.addWorkspaceAccessControl(
613613
it.organization.id!!,
614614
it.workspace.id!!,
@@ -627,7 +627,7 @@ class WorkspaceServiceImplTests {
627627
ROLE_NONE to true)
628628
.map { (role, shouldThrow) ->
629629
rbacTest("test RBAC update workspace access control: $role", role, shouldThrow) {
630-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
630+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
631631
every { workspaceRepository.save(any()) } returns it.workspace
632632
workspaceServiceImpl.updateWorkspaceAccessControl(
633633
it.organization.id!!,
@@ -648,7 +648,7 @@ class WorkspaceServiceImplTests {
648648
ROLE_NONE to true)
649649
.map { (role, shouldThrow) ->
650650
rbacTest("test RBAC remove workspace access control: $role", role, shouldThrow) {
651-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
651+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
652652
every { workspaceRepository.save(any()) } returns it.workspace
653653
workspaceServiceImpl.removeWorkspaceAccessControl(
654654
it.organization.id!!, it.workspace.id!!, "2$CONNECTED_DEFAULT_USER")
@@ -665,7 +665,7 @@ class WorkspaceServiceImplTests {
665665
ROLE_NONE to true)
666666
.map { (role, shouldThrow) ->
667667
rbacTest("test RBAC get workspace security users: $role", role, shouldThrow) {
668-
every { workspaceRepository.findByIdOrNull(any()) } returns it.workspace
668+
every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace)
669669
every { workspaceRepository.save(any()) } returns it.workspace
670670
workspaceServiceImpl.getWorkspaceSecurityUsers(
671671
it.organization.id!!, it.workspace.id!!)

0 commit comments

Comments
 (0)