fix: concurrency issues while requesting CaaS (#384)

Co-authored-by: Richard Treier <[email protected]>
sovity · Dec 2, 2024 · fc6e2e5 · fc6e2e5
1 parent d5c1282
commit fc6e2e5
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,9 +24,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
@@ -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/...rity-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/seeds/utils/ScenarioData.kt b/...rity-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<ContractOfferRecord>()
     private val crawlerEventLogEntries = mutableListOf<CrawlerEventLogRecord>()
 
+    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-quarkus/src/main/kotlin/de/sovity/authorityportal/web/UiResourceImpl.kt b/.../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/...otlin/de/sovity/authorityportal/web/pages/connectormanagement/CaasManagementApiService.kt b/...otlin/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/...in/de/sovity/authorityportal/web/tests/services/connector/CaasManagementApiServiceTest.kt b/...in/de/sovity/authorityportal/web/tests/services/connector/CaasManagementApiServiceTest.kt
@@ -38,16 +38,23 @@ 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
 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)
+    }
 }