From 2e772897d636e31f653351b6a63ffe2454bc2926 Mon Sep 17 00:00:00 2001 From: Kamil Czaja <46053356+kamilczaja@users.noreply.github.com> Date: Mon, 2 Dec 2024 08:52:54 +0100 Subject: [PATCH] fix: concurrency issues while requesting CaaS (#384) Co-authored-by: Richard Treier (cherry picked from commit fc6e2e5c42fd6b55547c10907721f24ce65af94c) --- CHANGELOG.md | 3 +- .../authority-portal-quarkus/build.gradle.kts | 1 + .../seeds/utils/ScenarioData.kt | 17 +++++ .../authorityportal/web/UiResourceImpl.kt | 5 +- .../CaasManagementApiService.kt | 2 + .../connector/CaasManagementApiServiceTest.kt | 67 ++++++++++++++++++- 6 files changed, 90 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12a30f697..199b60223 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,9 +22,10 @@ please see [changelog_updates.md](docs/dev/changelog_updates.md). - Fixed final step not showing when registering a central component ([#305](https://github.com/sovity/authority-portal/issues/305)) - Fixed My Organization page not updated when switching between environments ([#255](https://github.com/sovity/authority-portal/issues/255)) - Added live update when deactivating/reactivating users ([#287](https://github.com/sovity/authority-portal/issues/287)) -- Fixed Website title not updating in some scenarios [#237](https://github.com/sovity/authority-portal/issues/237) +- Fixed Website title not updating in some scenarios ([#237](https://github.com/sovity/authority-portal/issues/237)) - Fixed security vulnerabilities - Fixed the user not being redirected to the correct URL after login ([#324](https://github.com/sovity/authority-portal/issues/324)) +- Fixed an issue wherein it was possible to bypass the CaaS request limit in an organization ([PR #384](https://github.com/sovity/authority-portal/pull/384)) ### Known issues diff --git a/authority-portal-backend/authority-portal-quarkus/build.gradle.kts b/authority-portal-backend/authority-portal-quarkus/build.gradle.kts index 0c47d4081..f72d96331 100644 --- a/authority-portal-backend/authority-portal-quarkus/build.gradle.kts +++ b/authority-portal-backend/authority-portal-quarkus/build.gradle.kts @@ -61,6 +61,7 @@ dependencies { // https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.0#fixation-of-the-mockito-subclass-mockmaker exclude(group = libs.mockito.subclass.get().group, module = libs.mockito.subclass.get().name) } + testImplementation("io.quarkus:quarkus-test-security") testImplementation(libs.bundles.assertj) testImplementation(libs.awaitility) testImplementation(libs.bundles.mockito) diff --git a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/seeds/utils/ScenarioData.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/seeds/utils/ScenarioData.kt index b8de08fe7..bfe3e1be6 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/seeds/utils/ScenarioData.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/seeds/utils/ScenarioData.kt @@ -14,6 +14,7 @@ package de.sovity.authorityportal.seeds.utils import com.fasterxml.jackson.databind.ObjectMapper +import de.sovity.authorityportal.db.jooq.Tables import de.sovity.authorityportal.db.jooq.enums.ConnectorContractOffersExceeded import de.sovity.authorityportal.db.jooq.enums.ConnectorDataOffersExceeded import de.sovity.authorityportal.db.jooq.enums.ConnectorOnlineStatus @@ -57,6 +58,22 @@ class ScenarioData { private val contractOffers = mutableListOf() private val crawlerEventLogEntries = mutableListOf() + companion object { + fun uninstall(dsl: DSLContext) { + dsl.deleteFrom(Tables.CRAWLER_EVENT_LOG).execute() + dsl.deleteFrom(Tables.CONTRACT_OFFER).execute() + dsl.deleteFrom(Tables.DATA_OFFER_VIEW_COUNT).execute() + dsl.deleteFrom(Tables.DATA_OFFER).execute() + dsl.deleteFrom(Tables.COMPONENT).execute() + dsl.deleteFrom(Tables.CONNECTOR).execute() + dsl.update(Tables.USER) + .set(Tables.USER.ORGANIZATION_ID, null as String?) + .execute() + dsl.deleteFrom(Tables.ORGANIZATION).execute() + dsl.deleteFrom(Tables.USER).execute() + } + } + fun install(dsl: DSLContext) { val userOrgMap = users.associate { it.id to it.organizationId } users.forEach { 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 a115d7f6e..ce5e2a2e8 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 @@ -64,11 +64,12 @@ import de.sovity.authorityportal.web.pages.usermanagement.UserInvitationApiServi import de.sovity.authorityportal.web.pages.usermanagement.UserRoleApiService import de.sovity.authorityportal.web.pages.usermanagement.UserUpdateApiService import de.sovity.authorityportal.web.pages.userregistration.UserRegistrationApiService +import io.quarkus.arc.Lock import jakarta.annotation.security.PermitAll import jakarta.enterprise.context.ApplicationScoped import jakarta.transaction.Transactional -@PermitAll // auth checks will be in code in this unit +@PermitAll @ApplicationScoped class UiResourceImpl( val authUtils: AuthUtils, @@ -412,7 +413,7 @@ class UiResourceImpl( ) } - @Transactional + @Lock override fun createCaas(environmentId: String, caasRequest: CreateCaasRequest): CreateConnectorResponse { authUtils.requiresRole(Roles.UserRoles.PARTICIPANT_CURATOR) authUtils.requiresMemberOfAnyOrganization() diff --git a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/connectormanagement/CaasManagementApiService.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/connectormanagement/CaasManagementApiService.kt index 833deef90..918189dfb 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/connectormanagement/CaasManagementApiService.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/connectormanagement/CaasManagementApiService.kt @@ -31,6 +31,7 @@ import de.sovity.authorityportal.web.utils.idmanagement.ClientIdUtils import de.sovity.authorityportal.web.utils.idmanagement.DataspaceComponentIdUtils import io.quarkus.logging.Log import jakarta.enterprise.context.ApplicationScoped +import jakarta.transaction.Transactional import org.eclipse.microprofile.config.inject.ConfigProperty import kotlin.jvm.optionals.getOrNull @@ -48,6 +49,7 @@ class CaasManagementApiService( @ConfigProperty(name = "quarkus.oidc-client.sovity.client-enabled") val isCaasClientEnabled: Boolean ) { + @Transactional(Transactional.TxType.REQUIRES_NEW) fun createCaas( organizationId: String, userId: String, diff --git a/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/connector/CaasManagementApiServiceTest.kt b/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/connector/CaasManagementApiServiceTest.kt index c5aa8c4b9..ad4f121bc 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/connector/CaasManagementApiServiceTest.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/connector/CaasManagementApiServiceTest.kt @@ -38,9 +38,12 @@ import de.sovity.authorityportal.web.utils.idmanagement.ClientIdUtils import io.quarkus.test.InjectMock import io.quarkus.test.TestTransaction import io.quarkus.test.junit.QuarkusTest +import io.quarkus.test.security.TestSecurity import jakarta.inject.Inject import org.assertj.core.api.Assertions.assertThat +import org.flywaydb.core.Flyway import org.jooq.DSLContext +import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith import org.mockito.junit.jupiter.MockitoExtension @@ -48,6 +51,10 @@ import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.whenever import java.time.OffsetDateTime +import java.util.concurrent.CountDownLatch +import java.util.concurrent.ExecutorService +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger @QuarkusTest @ExtendWith(MockitoExtension::class) @@ -65,11 +72,18 @@ class CaasManagementApiServiceTest { @Inject lateinit var clientIdUtils: ClientIdUtils + @Inject + lateinit var executorService: ExecutorService + @InjectMock lateinit var caasClient: CaasClient + @AfterEach + fun cleanup() { + ScenarioData.uninstall(dsl) + } + @Test - @TestTransaction fun `create caas creates connector with caas configuration`() { // arrange val now = OffsetDateTime.now() @@ -143,7 +157,6 @@ class CaasManagementApiServiceTest { } @Test - @TestTransaction fun `check free caas slots returns the correct amount`() { // arrange useDevUser(0, 0, setOf(Roles.UserRoles.PARTICIPANT_CURATOR)) @@ -166,4 +179,54 @@ class CaasManagementApiServiceTest { assertThat(result.limit).isEqualTo(0) assertThat(result.current).isEqualTo(0) } + + @Test + @TestSecurity(authorizationEnabled = false) + fun `create caas endpoint should lock correctly`() { + // arrange + val now = OffsetDateTime.now() + val i = AtomicInteger(0) + + useDevUser(0, 0, setOf(Roles.UserRoles.PARTICIPANT_CURATOR)) + useMockNow(now) + + ScenarioData().apply { + organization(0, 0) + user(0, 0) + + scenarioInstaller.install(this) + } + + whenever(caasClient.validateSubdomain(any())).thenReturn(true) + whenever(caasClient.requestCaas(any())).thenReturn( + CaasPortalResponse().apply { + value = CaasDetails(connectorId = dummyDevConnectorId(0, 0)) + } + ) + + // act + val count = 10 + val latch = CountDownLatch(count) + + repeat(count) { + executorService.execute { + uiResource.createCaas( + "test", CreateCaasRequest( + connectorSubdomain = "test-caas-${i.addAndGet(1)}", + connectorTitle = "Test CaaS-${i.addAndGet(1)}", + connectorDescription = "Connector-as-a-service for testing purposes" + ) + ) + latch.countDown() + } + } + latch.await(20, TimeUnit.SECONDS) + + // assert + val connectors = dsl.selectFrom(Tables.CONNECTOR) + .where(Tables.CONNECTOR.TYPE.eq(ConnectorType.CAAS)) + .fetch() + + assertThat(connectors).hasSize(1) + } }