From eea6eab7a9e97d5eb851dfce678f7a34bb736af8 Mon Sep 17 00:00:00 2001 From: Kamil Czaja Date: Thu, 28 Nov 2024 18:32:47 +0100 Subject: [PATCH 1/4] feat: locked caas request endpoint to avoid race conditions --- .../kotlin/de/sovity/authorityportal/web/UiResourceImpl.kt | 3 ++- .../web/pages/connectormanagement/CaasManagementApiService.kt | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) 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..9862423ed 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,6 +64,7 @@ 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 @@ -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..589f45b6e 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 fun createCaas( organizationId: String, userId: String, From dfb7526e5be0fc867c87f387bd21c09ca896f56c Mon Sep 17 00:00:00 2001 From: Kamil Czaja Date: Thu, 28 Nov 2024 19:16:09 +0100 Subject: [PATCH 2/4] chore: tests --- .../authority-portal-quarkus/build.gradle.kts | 1 + .../authorityportal/web/UiResourceImpl.kt | 2 +- .../CaasManagementApiService.kt | 2 +- .../connector/CaasManagementApiServiceTest.kt | 71 +++++++++++++++++++ 4 files changed, 74 insertions(+), 2 deletions(-) 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/web/UiResourceImpl.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/UiResourceImpl.kt index 9862423ed..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 @@ -69,7 +69,7 @@ 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, 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 589f45b6e..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 @@ -49,7 +49,7 @@ class CaasManagementApiService( @ConfigProperty(name = "quarkus.oidc-client.sovity.client-enabled") val isCaasClientEnabled: Boolean ) { - @Transactional + @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..472811654 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 @@ -35,12 +35,17 @@ import de.sovity.authorityportal.web.thirdparty.caas.CaasClient import de.sovity.authorityportal.web.thirdparty.caas.model.CaasDetails import de.sovity.authorityportal.web.thirdparty.caas.model.CaasPortalResponse import de.sovity.authorityportal.web.utils.idmanagement.ClientIdUtils +import io.quarkus.security.identity.SecurityIdentity import io.quarkus.test.InjectMock import io.quarkus.test.TestTransaction import io.quarkus.test.junit.QuarkusTest +import io.quarkus.test.security.TestSecurity +import jakarta.enterprise.context.control.ActivateRequestContext 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 +53,11 @@ 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.Executors +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger @QuarkusTest @ExtendWith(MockitoExtension::class) @@ -65,9 +75,20 @@ class CaasManagementApiServiceTest { @Inject lateinit var clientIdUtils: ClientIdUtils + @Inject + lateinit var executorService: ExecutorService + + @Inject + lateinit var flyway: Flyway + @InjectMock lateinit var caasClient: CaasClient + @AfterEach + fun cleanup() { + flyway.clean() + } + @Test @TestTransaction fun `create caas creates connector with caas configuration`() { @@ -166,4 +187,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) + } } From 869d490f6562b5e7d9d7fc4a1c2fd201dd6d8b69 Mon Sep 17 00:00:00 2001 From: Kamil Czaja Date: Fri, 29 Nov 2024 15:10:59 +0100 Subject: [PATCH 3/4] fix: tests --- .../authorityportal/seeds/utils/ScenarioData.kt | 17 +++++++++++++++++ .../connector/CaasManagementApiServiceTest.kt | 10 +--------- 2 files changed, 18 insertions(+), 9 deletions(-) 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/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 472811654..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 @@ -35,12 +35,10 @@ import de.sovity.authorityportal.web.thirdparty.caas.CaasClient import de.sovity.authorityportal.web.thirdparty.caas.model.CaasDetails import de.sovity.authorityportal.web.thirdparty.caas.model.CaasPortalResponse import de.sovity.authorityportal.web.utils.idmanagement.ClientIdUtils -import io.quarkus.security.identity.SecurityIdentity import io.quarkus.test.InjectMock import io.quarkus.test.TestTransaction import io.quarkus.test.junit.QuarkusTest import io.quarkus.test.security.TestSecurity -import jakarta.enterprise.context.control.ActivateRequestContext import jakarta.inject.Inject import org.assertj.core.api.Assertions.assertThat import org.flywaydb.core.Flyway @@ -55,7 +53,6 @@ import org.mockito.kotlin.whenever import java.time.OffsetDateTime import java.util.concurrent.CountDownLatch import java.util.concurrent.ExecutorService -import java.util.concurrent.Executors import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicInteger @@ -78,19 +75,15 @@ class CaasManagementApiServiceTest { @Inject lateinit var executorService: ExecutorService - @Inject - lateinit var flyway: Flyway - @InjectMock lateinit var caasClient: CaasClient @AfterEach fun cleanup() { - flyway.clean() + ScenarioData.uninstall(dsl) } @Test - @TestTransaction fun `create caas creates connector with caas configuration`() { // arrange val now = OffsetDateTime.now() @@ -164,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)) From eba4ffedb1533b56b548d8b90eeccd654bc9dedd Mon Sep 17 00:00:00 2001 From: Kamil Czaja Date: Fri, 29 Nov 2024 18:32:58 +0100 Subject: [PATCH 4/4] chore: updated CHANGELOG --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bf374e8c..a32fbba33 100644 --- 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