From 82d1e1d6b2650041523d4b74812d04614e09d6f2 Mon Sep 17 00:00:00 2001 From: Kamil Czaja <46053356+kamilczaja@users.noreply.github.com> Date: Tue, 3 Dec 2024 11:03:32 +0100 Subject: [PATCH] fix: made portal db the source of truth on user creation (#387) --- CHANGELOG.md | 1 + .../OrganizationInvitationApiService.kt | 30 +++++------ .../registration/RegistrationApiService.kt | 28 +++++----- .../UserInvitationApiService.kt | 8 +++ .../web/services/UserService.kt | 9 ++++ .../thirdparty/keycloak/KeycloakService.kt | 52 ++++++++++++++----- .../uptimekuma/model/ComponentStatus.kt | 3 ++ .../web/utils/HttpExceptionUtils.kt | 10 ++++ .../services/FirstUserRegistrationTest.kt | 5 +- .../services/RegistrationApiServiceTest.kt | 30 +++++++++-- .../OrganizationInvitationApiServiceTest.kt | 10 ++-- 11 files changed, 129 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02a90df29..d9f651b71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ please see [changelog_updates.md](docs/dev/changelog_updates.md). - 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)) +- Fixed an issue wherein a user registration could fail due to a mismatch of the internal database and the Keycloak database - Fixed an issue where entries in the connector overview would randomly switch places ([PR #386](https://github.com/sovity/authority-portal/pull/386)) ### Known issues diff --git a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/organizationmanagement/OrganizationInvitationApiService.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/organizationmanagement/OrganizationInvitationApiService.kt index a0abaf16f..304bcf48c 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/organizationmanagement/OrganizationInvitationApiService.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/organizationmanagement/OrganizationInvitationApiService.kt @@ -23,6 +23,7 @@ import de.sovity.authorityportal.web.thirdparty.keycloak.KeycloakService import de.sovity.authorityportal.web.thirdparty.keycloak.model.OrganizationRole import de.sovity.authorityportal.web.utils.TimeUtils import de.sovity.authorityportal.web.utils.idmanagement.OrganizationIdUtils +import de.sovity.authorityportal.web.utils.resourceAlreadyExists import io.quarkus.logging.Log import jakarta.enterprise.context.ApplicationScoped @@ -36,8 +37,20 @@ class OrganizationInvitationApiService( ) { fun inviteOrganization(invitationInformation: InviteOrganizationRequest, adminUserId: String): IdResponse { + if (userService.userExistsInDb(invitationInformation.userEmail)) { + resourceAlreadyExists("User with email ${invitationInformation.userEmail} already exists.") + } + val organizationId = organizationIdUtils.generateOrganizationId() - val userId = createKeycloakUserAndOrganization(organizationId, invitationInformation) + val userId = keycloakService.createKeycloakUserAndOrganization( + organizationId = organizationId, + userEmail = invitationInformation.userEmail, + userFirstName = invitationInformation.userFirstName, + userLastName = invitationInformation.userLastName, + userOrganizationRole = OrganizationRole.PARTICIPANT_ADMIN, + userPassword = null + ) + createDbUserAndOrganization(userId, organizationId, invitationInformation) keycloakService.sendInvitationEmailWithPasswordReset(userId) @@ -46,21 +59,6 @@ class OrganizationInvitationApiService( return IdResponse(organizationId, timeUtils.now()) } - private fun createKeycloakUserAndOrganization( - organizationId: String, - invitationInformation: InviteOrganizationRequest - ): String { - val userId = keycloakService.createUser( - invitationInformation.userEmail, - invitationInformation.userFirstName, - invitationInformation.userLastName - ) - keycloakService.createOrganization(organizationId) - keycloakService.joinOrganization(userId, organizationId, OrganizationRole.PARTICIPANT_ADMIN) - - return userId - } - private fun createDbUserAndOrganization( userId: String, organizationId: String, diff --git a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/registration/RegistrationApiService.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/registration/RegistrationApiService.kt index 7e74b23db..3798cb9d4 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/registration/RegistrationApiService.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/registration/RegistrationApiService.kt @@ -27,6 +27,7 @@ import de.sovity.authorityportal.web.thirdparty.keycloak.KeycloakService import de.sovity.authorityportal.web.thirdparty.keycloak.model.OrganizationRole import de.sovity.authorityportal.web.utils.TimeUtils import de.sovity.authorityportal.web.utils.idmanagement.OrganizationIdUtils +import de.sovity.authorityportal.web.utils.resourceAlreadyExists import io.quarkus.logging.Log import jakarta.enterprise.context.ApplicationScoped @@ -41,8 +42,20 @@ class RegistrationApiService( ) { fun registerUserAndOrganization(registrationRequest: RegistrationRequestDto): IdResponse { + if (userService.userExistsInDb(registrationRequest.userEmail)) { + resourceAlreadyExists("User with email ${registrationRequest.userEmail} already exists.") + } + val organizationId = organizationIdUtils.generateOrganizationId() - val userId = createKeycloakUserAndOrganization(organizationId, registrationRequest) + val userId = keycloakService.createKeycloakUserAndOrganization( + organizationId = organizationId, + userEmail = registrationRequest.userEmail, + userFirstName = registrationRequest.userFirstName, + userLastName = registrationRequest.userLastName, + userOrganizationRole = OrganizationRole.PARTICIPANT_ADMIN, + userPassword = registrationRequest.userPassword, + ) + createDbUserAndOrganization(userId, organizationId, registrationRequest) keycloakService.sendInvitationEmail(userId) firstUserService.setupFirstUserIfRequired(userId, organizationId) @@ -52,19 +65,6 @@ class RegistrationApiService( return IdResponse(userId, timeUtils.now()) } - private fun createKeycloakUserAndOrganization(organizationId: String, registrationRequest: RegistrationRequestDto): String { - val userId = keycloakService.createUser( - email = registrationRequest.userEmail, - firstName = registrationRequest.userFirstName, - lastName = registrationRequest.userLastName, - password = registrationRequest.userPassword - ) - keycloakService.createOrganization(organizationId) - keycloakService.joinOrganization(userId, organizationId, OrganizationRole.PARTICIPANT_ADMIN) - - return userId - } - private fun createDbUserAndOrganization( userId: String, organizationId: String, diff --git a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/usermanagement/UserInvitationApiService.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/usermanagement/UserInvitationApiService.kt index 6789892cd..c984a1a22 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/usermanagement/UserInvitationApiService.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/pages/usermanagement/UserInvitationApiService.kt @@ -19,6 +19,7 @@ import de.sovity.authorityportal.db.jooq.enums.UserOnboardingType import de.sovity.authorityportal.web.services.UserService import de.sovity.authorityportal.web.thirdparty.keycloak.KeycloakService import de.sovity.authorityportal.web.utils.TimeUtils +import de.sovity.authorityportal.web.utils.resourceAlreadyExists import io.quarkus.logging.Log import jakarta.enterprise.context.ApplicationScoped @@ -35,6 +36,13 @@ class UserInvitationApiService( organizationId: String, adminUserId: String ): IdResponse { + if (userService.userExistsInDb(userInformation.email)) { + resourceAlreadyExists("User with email ${userInformation.email} already exists.") + } + + // DB is source of truth, so we delete a potentially existing user in Keycloak + keycloakService.getUserIdByEmail(userInformation.email)?.let { keycloakService.deleteUser(it) } + val userId = keycloakService.createUser(userInformation.email, userInformation.firstName, userInformation.lastName) keycloakService.sendInvitationEmailWithPasswordReset(userId) diff --git a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/services/UserService.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/services/UserService.kt index d24b76f37..7c33dd0eb 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/services/UserService.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/services/UserService.kt @@ -65,6 +65,15 @@ class UserService( .fetchMap(u.ORGANIZATION_ID, DSL.count()) } + fun userExistsInDb(email: String): Boolean { + val u = Tables.USER + + return dsl.fetchExists( + dsl.selectFrom(u) + .where(u.EMAIL.eq(email)) + ) + } + private fun getUser(userId: String): UserRecord? { val u = Tables.USER diff --git a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/thirdparty/keycloak/KeycloakService.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/thirdparty/keycloak/KeycloakService.kt index adcc4185e..2aeec4b2a 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/thirdparty/keycloak/KeycloakService.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/thirdparty/keycloak/KeycloakService.kt @@ -19,7 +19,6 @@ import de.sovity.authorityportal.web.thirdparty.keycloak.model.OrganizationRole import de.sovity.authorityportal.web.thirdparty.keycloak.model.RequiredAction import io.quarkus.logging.Log import jakarta.enterprise.context.ApplicationScoped -import jakarta.inject.Inject import jakarta.ws.rs.WebApplicationException import jakarta.ws.rs.core.Response import org.eclipse.microprofile.config.inject.ConfigProperty @@ -29,22 +28,19 @@ import org.keycloak.representations.idm.GroupRepresentation import org.keycloak.representations.idm.UserRepresentation @ApplicationScoped -class KeycloakService { - - @Inject - lateinit var keycloak: Keycloak - - @Inject - lateinit var keycloakUserMapper: KeycloakUserMapper +class KeycloakService( + val keycloak: Keycloak, + val keycloakUserMapper: KeycloakUserMapper, @ConfigProperty(name = "quarkus.keycloak.admin-client.realm") - lateinit var keycloakRealm: String + val keycloakRealm: String, @ConfigProperty(name = "quarkus.keycloak.admin-client.client-id") - lateinit var keycloakClientId: String + val keycloakClientId: String, @ConfigProperty(name = "authority-portal.base-url") - lateinit var baseUrl: String + val baseUrl: String +) { fun createUser(email: String, firstName: String, lastName: String, password: String? = null): String { val user = UserRepresentation().also { @@ -60,7 +56,7 @@ class KeycloakService { if (password != null) { it.credentials = listOf( - CredentialRepresentation().also {credentials -> + CredentialRepresentation().also { credentials -> credentials.isTemporary = false credentials.type = CredentialRepresentation.PASSWORD credentials.value = password @@ -77,6 +73,36 @@ class KeycloakService { return keycloak.realm(keycloakRealm).users().search(email).first().id } + fun createKeycloakUserAndOrganization( + organizationId: String, + userEmail: String, + userFirstName: String, + userLastName: String, + userOrganizationRole: OrganizationRole, + userPassword: String? + ): String { + // To avoid syncing issues, we assume our DB to be the source of truth, so we need to delete the potentially existing user in KC + val userIdIfExists = getUserIdByEmail(userEmail) + if (userIdIfExists != null) { + deleteUser(userIdIfExists) + } + + val userId = createUser( + email = userEmail, + firstName = userFirstName, + lastName = userLastName, + password = userPassword + ) + createOrganization(organizationId) + joinOrganization(userId, organizationId, userOrganizationRole) + + return userId + } + + fun getUserIdByEmail(email: String): String? { + return keycloak.realm(keycloakRealm).users().search(email).firstOrNull()?.id + } + fun deleteUser(userId: String) { keycloak.realm(keycloakRealm).users().delete(userId) } @@ -278,7 +304,7 @@ class KeycloakService { keycloak.realm(keycloakRealm).users().get(userId).executeActionsEmail(keycloakClientId, baseUrl, actions) } - private fun getSubGroupIds(groupId: String) = + private fun getSubGroupIds(groupId: String) = keycloak.realm(keycloakRealm).groups().query("", true) .first { it.id == groupId }.subGroups.associate { it.name to it.id } } diff --git a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/thirdparty/uptimekuma/model/ComponentStatus.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/thirdparty/uptimekuma/model/ComponentStatus.kt index 2b53b7932..c013a6d37 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/thirdparty/uptimekuma/model/ComponentStatus.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/thirdparty/uptimekuma/model/ComponentStatus.kt @@ -13,6 +13,9 @@ package de.sovity.authorityportal.web.thirdparty.uptimekuma.model +import de.sovity.authorityportal.web.thirdparty.uptimekuma.model.ComponentStatus.entries + + enum class ComponentStatus(val intValue: Int) { UP(1), DOWN(0), diff --git a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/utils/HttpExceptionUtils.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/utils/HttpExceptionUtils.kt index 748a2b851..6607d2a27 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/utils/HttpExceptionUtils.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/utils/HttpExceptionUtils.kt @@ -15,6 +15,7 @@ package de.sovity.authorityportal.web.utils import jakarta.ws.rs.NotAuthorizedException import jakarta.ws.rs.NotFoundException +import jakarta.ws.rs.WebApplicationException import jakarta.ws.rs.core.Response /** @@ -37,3 +38,12 @@ fun notFound(message: String = ""): Nothing { .build() ) } + +fun resourceAlreadyExists(message: String = ""): Nothing { + throw WebApplicationException( + "Resource already exists. $message", + Response.status(Response.Status.CONFLICT) + .header("WWW-Authenticate", "Bearer realm=\"authority-portal\"") + .build() + ) +} diff --git a/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/FirstUserRegistrationTest.kt b/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/FirstUserRegistrationTest.kt index 463122ba8..fb45a9505 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/FirstUserRegistrationTest.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/FirstUserRegistrationTest.kt @@ -93,10 +93,9 @@ class FirstUserRegistrationTest { useMockNow(now) doNothing().whenever(keycloakService).sendInvitationEmail(any()) - doNothing().whenever(keycloakService).createOrganization(any()) doNothing().whenever(keycloakService).joinOrganization(any(), any(), any()) - whenever(keycloakService.createUser( - eq(request.userEmail), eq(request.userFirstName), eq(request.userLastName), eq(request.userPassword) + whenever(keycloakService.createKeycloakUserAndOrganization( + any(), eq(request.userEmail), eq(request.userFirstName), eq(request.userLastName), any(), eq(request.userPassword) )).thenReturn(dummyDevUserUuid(0)) // act diff --git a/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/RegistrationApiServiceTest.kt b/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/RegistrationApiServiceTest.kt index b82e37c34..5b3c8bddf 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/RegistrationApiServiceTest.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/RegistrationApiServiceTest.kt @@ -33,7 +33,9 @@ import io.quarkus.test.InjectMock import io.quarkus.test.TestTransaction import io.quarkus.test.junit.QuarkusTest import jakarta.inject.Inject +import jakarta.ws.rs.WebApplicationException import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy import org.jooq.DSLContext import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith @@ -41,6 +43,7 @@ import org.mockito.junit.jupiter.MockitoExtension import org.mockito.kotlin.any import org.mockito.kotlin.doNothing import org.mockito.kotlin.eq +import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import java.time.OffsetDateTime @@ -95,10 +98,8 @@ class RegistrationApiServiceTest { useMockNow(now) doNothing().whenever(keycloakService).sendInvitationEmail(any()) - doNothing().whenever(keycloakService).createOrganization(any()) - doNothing().whenever(keycloakService).joinOrganization(any(), any(), any()) - whenever(keycloakService.createUser( - eq(request.userEmail), eq(request.userFirstName), eq(request.userLastName), eq(request.userPassword) + whenever(keycloakService.createKeycloakUserAndOrganization( + any(), eq(request.userEmail), eq(request.userFirstName), eq(request.userLastName), any(), eq(request.userPassword) )).thenReturn(dummyDevUserUuid(1)) ScenarioData().apply { @@ -174,4 +175,25 @@ class RegistrationApiServiceTest { .withStrictTypeChecking() .isEqualTo(expectedUser.copy()) } + + @Test + @TestTransaction + fun `registration fails because user is already in db`() { + // arrange + useUnauthenticated() + + ScenarioData().apply { + organization(0, 0) + user(0, 0) { + it.email = request.userEmail + } + scenarioInstaller.install(this) + } + + // act & assert + assertThatThrownBy { uiResource.registerUser(request) } + .isInstanceOf(WebApplicationException::class.java) + .extracting { (it as WebApplicationException).response.status } + .isEqualTo(409) + } } diff --git a/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/organization/OrganizationInvitationApiServiceTest.kt b/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/organization/OrganizationInvitationApiServiceTest.kt index 628199b56..b5229fe1e 100644 --- a/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/organization/OrganizationInvitationApiServiceTest.kt +++ b/authority-portal-backend/authority-portal-quarkus/src/test/kotlin/de/sovity/authorityportal/web/tests/services/organization/OrganizationInvitationApiServiceTest.kt @@ -97,13 +97,9 @@ class OrganizationInvitationApiServiceTest { useMockNow(now) whenever(organizationIdUtils.generateOrganizationId()).thenReturn(dummyDevOrganizationId(1)) - whenever(keycloakService.createUser( - eq("new.user@test.sovity.io"), - eq("New"), - eq("User"), - isNull())).thenReturn(dummyDevUserUuid(1)) - doNothing().whenever(keycloakService).createOrganization(eq(dummyDevOrganizationId(1))) - doNothing().whenever(keycloakService).joinOrganization(eq(dummyDevUserUuid(1)), eq(dummyDevOrganizationId(1)), any()) + whenever(keycloakService.createKeycloakUserAndOrganization( + any(), eq("new.user@test.sovity.io"), eq("New"), eq("User"), any(), eq(null) + )).thenReturn(dummyDevUserUuid(1)) ScenarioData().apply { user(0, 0)