fix: allow operator admins to delete all connectors (#408)

* fix: use correct endpoint when trying to delete connector as operator admin * fix: allow operator admins to delete all connectors * test: add tests for connector deletion * docs: update CHANGELOG.md
sovity · Jan 9, 2025 · f68cb2a · f68cb2a
1 parent 41340c1
commit f68cb2a
Show file tree

Hide file tree

Showing 6 changed files with 179 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,7 +15,8 @@ please see [changelog_updates.md](docs/dev/changelog_updates.md).
 
 #### Patch
 
-- Fixed Keycloak dev realm for local E2E development
+- Fixed Keycloak dev realm for local E2E development ([PR #405](https://github.com/sovity/authority-portal/pull/405))
+- Fixed Operator Admins being unable to delete connectors ([PR #408](https://github.com/sovity/authority-portal/pull/408))
 
 ### Known issues
 

diff --git a/.../authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/UiResourceImpl.kt b/.../authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/UiResourceImpl.kt
@@ -308,9 +308,15 @@ class UiResourceImpl(
 
     @Transactional
     override fun deleteProvidedConnector(connectorId: String): IdResponse {
-        authUtils.requiresRole(Roles.UserRoles.SERVICE_PARTNER_ADMIN)
-        authUtils.requiresMemberOfAnyOrganization()
-        return connectorManagementApiService.deleteOwnOrProvidedConnector(
+        authUtils.requiresAnyRole(Roles.UserRoles.SERVICE_PARTNER_ADMIN, Roles.UserRoles.OPERATOR_ADMIN)
+
+        if (!authUtils.hasRole(Roles.UserRoles.OPERATOR_ADMIN)) {
+            authUtils.requiresMemberOfProviderOrganization(connectorId)
+        } else {
+            authUtils.requiresMemberOfAnyOrganization()
+        }
+
+        return connectorManagementApiService.deleteConnectorById(
             connectorId,
             loggedInUser.organizationId!!,
             loggedInUser.userId
@@ -369,8 +375,8 @@ class UiResourceImpl(
     @Transactional
     override fun deleteOwnConnector(connectorId: String): IdResponse {
         authUtils.requiresRole(Roles.UserRoles.PARTICIPANT_CURATOR)
-        authUtils.requiresMemberOfAnyOrganization()
-        return connectorManagementApiService.deleteOwnOrProvidedConnector(
+        authUtils.requiresMemberOfOwningOrganization(connectorId)
+        return connectorManagementApiService.deleteConnectorById(
             connectorId,
             loggedInUser.organizationId!!,
             loggedInUser.userId

diff --git a/.../authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/auth/AuthUtils.kt b/.../authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/auth/AuthUtils.kt
@@ -15,24 +15,20 @@ package de.sovity.authorityportal.web.auth
 
 import de.sovity.authorityportal.db.jooq.enums.OrganizationRegistrationStatus
 import de.sovity.authorityportal.db.jooq.enums.UserRegistrationStatus
+import de.sovity.authorityportal.web.services.ConnectorService
 import de.sovity.authorityportal.web.services.OrganizationService
 import de.sovity.authorityportal.web.services.UserService
 import de.sovity.authorityportal.web.utils.unauthorized
 import io.quarkus.logging.Log
 import jakarta.enterprise.context.ApplicationScoped
-import jakarta.inject.Inject
 
 @ApplicationScoped
-class AuthUtils {
-    @Inject
-    lateinit var loggedInUser: LoggedInUser
-
-    @Inject
-    lateinit var userService: UserService
-
-    @Inject
-    lateinit var organizationService: OrganizationService
-
+class AuthUtils(
+    val loggedInUser: LoggedInUser,
+    val userService: UserService,
+    val organizationService: OrganizationService,
+    val connectorService: ConnectorService
+) {
     fun requiresAuthenticated() {
         if (!loggedInUser.authenticated) {
             Log.error("User is not authenticated.")
@@ -149,7 +145,6 @@ class AuthUtils {
     }
 
     fun requiresOrganizationRegistrationStatus(status: OrganizationRegistrationStatus) {
-        requiresAuthenticated()
         requiresMemberOfAnyOrganization()
 
         val organization = organizationService.getOrganizationOrThrow(loggedInUser.organizationId!!)
@@ -158,4 +153,24 @@ class AuthUtils {
             unauthorized("User can only perform the action if their organization is in the onboarding phase")
         }
     }
+
+    fun requiresMemberOfProviderOrganization(connectorId: String) {
+        requiresMemberOfAnyOrganization()
+
+        val connector = connectorService.getConnectorOrThrow(connectorId)
+        if (connector.providerOrganizationId != loggedInUser.organizationId!!) {
+            Log.error("User is not a member of the provider organization. userId=${loggedInUser.userId}, providerOrganizationId=${connector.providerOrganizationId}")
+            unauthorized("User is not a member of the provider organization")
+        }
+    }
+
+    fun requiresMemberOfOwningOrganization(connectorId: String) {
+        requiresMemberOfAnyOrganization()
+
+        val connector = connectorService.getConnectorOrThrow(connectorId)
+        if (!connectorId.startsWith(loggedInUser.organizationId!!)) {
+            Log.error("User is not a member of the owning organization. userId=${loggedInUser.userId}, organizationId=${connector.organizationId}")
+            unauthorized("User is not a member of the owning organization")
+        }
+    }
 }
diff --git a/.../de/sovity/authorityportal/web/pages/connectormanagement/ConnectorManagementApiService.kt b/.../de/sovity/authorityportal/web/pages/connectormanagement/ConnectorManagementApiService.kt
@@ -335,26 +335,21 @@ class ConnectorManagementApiService(
         return url.trim().removeSuffix("/")
     }
 
-    fun deleteOwnOrProvidedConnector(
+    fun deleteConnectorById(
         connectorId: String,
         organizationId: String,
         userId: String
     ): IdResponse {
         val connector = connectorService.getConnectorOrThrow(connectorId)
 
-        if (!connectorId.startsWith(organizationId) && connector.providerOrganizationId != organizationId) {
-            Log.error("To be deleted connector does not belong to user's organization and is not hosted by it. connectorId=$connectorId, organizationId=$organizationId, userId=$userId.")
-            error("Connector ID does not match the ID of the user's organization or host organization")
-        }
-
         deleteConnector(connector)
         Log.info("Connector unregistered. connectorId=$connectorId, organizationId=$organizationId, userId=$userId.")
 
         return IdResponse(connectorId, timeUtils.now())
     }
 
-    fun deleteAllOrganizationConnectors(organizationid: String) {
-        val connectors = connectorService.getConnectorsByOrganizationId(organizationid)
+    fun deleteAllOrganizationConnectors(organizationId: String) {
+        val connectors = connectorService.getConnectorsByOrganizationId(organizationId)
         connectors.forEach { deleteConnector(it) }
     }
 

diff --git a/.../sovity/authorityportal/web/tests/services/connector/ConnectorManagementApiServiceTest.kt b/.../sovity/authorityportal/web/tests/services/connector/ConnectorManagementApiServiceTest.kt
@@ -371,13 +371,6 @@ class ConnectorManagementApiServiceTest {
         assertThat(result).isNotNull
         assertThat(result.id).isEqualTo(connectorId)
 
-        fun count(table: TableLike<*>, condition: Condition): Int {
-            return dsl.selectCount()
-                .from(table)
-                .where(condition)
-                .fetchOne(0, Int::class.java) ?: 0
-        }
-
         assertThat(
             count(
                 Tables.CONNECTOR,
@@ -823,4 +816,139 @@ class ConnectorManagementApiServiceTest {
         }
             .isInstanceOf(NotAuthorizedException::class.java)
     }
+
+    @Test
+    @TestTransaction
+    fun `delete own connector fails because of missing membership to owning organization`() {
+        // arrange
+        useDevUser(0, 0, setOf(Roles.UserRoles.PARTICIPANT_CURATOR))
+
+        ScenarioData().apply {
+            organization(0, 0)
+            organization(1, 1)
+            user(0, 0)
+            user(1, 1)
+            connector(0, 1, 1)
+
+            scenarioInstaller.install(this)
+        }
+
+        // act && assert
+        assertThatThrownBy { uiResource.deleteOwnConnector(dummyDevConnectorId(1, 0)) }
+            .isInstanceOf(NotAuthorizedException::class.java)
+            .hasMessageContaining("User is not a member of the owning organization")
+    }
+
+    @Test
+    @TestTransaction
+    fun `delete provided connector fails because of missing membership to provider organization`() {
+        // arrange
+        useDevUser(0, 0, setOf(Roles.UserRoles.SERVICE_PARTNER_ADMIN))
+
+        ScenarioData().apply {
+            organization(0, 0)
+            organization(1, 1)
+            organization(2, 2)
+            user(0, 0)
+            user(1, 1)
+            user(2, 2)
+            connector(0, 1, 2) {
+                it.providerOrganizationId = dummyDevOrganizationId(2)
+            }
+
+            scenarioInstaller.install(this)
+        }
+
+        // act && assert
+        assertThatThrownBy { uiResource.deleteProvidedConnector(dummyDevConnectorId(1, 0)) }
+            .isInstanceOf(NotAuthorizedException::class.java)
+            .hasMessageContaining("User is not a member of the provider organization")
+    }
+
+    @Test
+    @TestTransaction
+    fun `delete provided connector as service partner admin`() {
+        // arrange
+        useDevUser(0, 0, setOf(Roles.UserRoles.SERVICE_PARTNER_ADMIN))
+
+        val dapsClient = mock<DapsClient>()
+        whenever(dapsClientService.forEnvironment(any())).thenReturn(dapsClient)
+        doNothing().whenever(dapsClient).deleteClient(any())
+
+        ScenarioData().apply {
+            organization(0, 0)
+            organization(1, 1)
+            organization(2, 2)
+            user(0, 0)
+            user(1, 1)
+            user(2, 2)
+            connector(0, 1, 0) {
+                it.providerOrganizationId = dummyDevOrganizationId(0)
+            }
+
+            scenarioInstaller.install(this)
+        }
+        val connectorId = dummyDevConnectorId(1, 0)
+
+        // act
+        val result = uiResource.deleteProvidedConnector(connectorId)
+
+        // assert
+        assertThat(result).isNotNull
+        assertThat(result.id).isEqualTo(connectorId)
+
+        assertThat(
+            count(
+                Tables.CONNECTOR,
+                Tables.CONNECTOR.CONNECTOR_ID.eq(connectorId)
+            )
+        ).isEqualTo(0)
+    }
+
+    @Test
+    @TestTransaction
+    fun `delete provided connector as operator admin`() {
+        // arrange
+        useDevUser(0, 0, setOf(Roles.UserRoles.OPERATOR_ADMIN))
+
+        val dapsClient = mock<DapsClient>()
+        whenever(dapsClientService.forEnvironment(any())).thenReturn(dapsClient)
+        doNothing().whenever(dapsClient).deleteClient(any())
+
+        ScenarioData().apply {
+            organization(0, 0)
+            organization(1, 1)
+            organization(2, 2)
+            user(0, 0)
+            user(1, 1)
+            user(2, 2)
+            connector(0, 1, 2) {
+                it.providerOrganizationId = dummyDevOrganizationId(2)
+            }
+
+            scenarioInstaller.install(this)
+        }
+        val connectorId = dummyDevConnectorId(1, 0)
+
+        // act
+        val result = uiResource.deleteProvidedConnector(connectorId)
+
+        // assert
+        assertThat(result).isNotNull
+        assertThat(result.id).isEqualTo(connectorId)
+
+        assertThat(
+            count(
+                Tables.CONNECTOR,
+                Tables.CONNECTOR.CONNECTOR_ID.eq(connectorId)
+            )
+        ).isEqualTo(0)
+    }
+
+    private fun count(table: TableLike<*>, condition: Condition): Int {
+        return dsl.selectCount()
+            .from(table)
+            .where(condition)
+            .fetchOne(0, Int::class.java) ?: 0
+    }
 }
diff --git a/...app/pages/authority-connector-list-page/state/authority-connector-list-page-state-impl.ts b/...app/pages/authority-connector-list-page/state/authority-connector-list-page-state-impl.ts
@@ -105,7 +105,7 @@ export class AuthorityConnectorListPageStateImpl {
     }
     ctx.patchState({busy: true});
 
-    return this.apiService.deleteOwnConnector(action.connectorId).pipe(
+    return this.apiService.deleteProvidedConnector(action.connectorId).pipe(
       switchMap(() => this.globalStateUtils.getDeploymentEnvironmentId()),
       switchMap((deploymentEnvironmentId) =>
         this.apiService.getAllConnectors(deploymentEnvironmentId),