diff --git a/CHANGELOG.md b/CHANGELOG.md index f426f8bc0..0b803c1ea 100644 --- 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-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/UiResourceImpl.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/UiResourceImpl.kt index ce5e2a2e8..55bf7ee68 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/UiResourceImpl.kt +++ b/authority-portal-backend/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-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/auth/AuthUtils.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/auth/AuthUtils.kt index 7b3c23eea..304ce3c91 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/auth/AuthUtils.kt +++ b/authority-portal-backend/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/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/connectormanagement/ConnectorManagementApiService.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/connectormanagement/ConnectorManagementApiService.kt index 96fa3f67f..94253f5f5 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/connectormanagement/ConnectorManagementApiService.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/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/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/connector/ConnectorManagementApiServiceTest.kt b/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/connector/ConnectorManagementApiServiceTest.kt index 0f841979d..047c91b27 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/connector/ConnectorManagementApiServiceTest.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/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() + 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() + 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/authority-portal-frontend/src/app/pages/authority-connector-list-page/state/authority-connector-list-page-state-impl.ts b/authority-portal-frontend/src/app/pages/authority-connector-list-page/state/authority-connector-list-page-state-impl.ts index 11e635740..2391a05f2 100644 --- a/authority-portal-frontend/src/app/pages/authority-connector-list-page/state/authority-connector-list-page-state-impl.ts +++ b/authority-portal-frontend/src/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),