From dc2a7c2220eccb5b8ced9c1b4f457fb8b8667e84 Mon Sep 17 00:00:00 2001 From: Kamil Czaja Date: Mon, 2 Dec 2024 15:49:36 +0100 Subject: [PATCH] fix: made portal db the source of truth on user creation --- CHANGELOG.md | 1 + .../OrganizationInvitationApiService.kt | 27 ++++------ .../registration/RegistrationApiService.kt | 25 ++++----- .../UserInvitationApiService.kt | 8 +++ .../web/services/UserService.kt | 13 +++++ .../thirdparty/keycloak/KeycloakService.kt | 51 ++++++++++++++----- .../web/utils/HttpExceptionUtils.kt | 10 ++++ .../services/FirstUserRegistrationTest.kt | 4 ++ .../services/RegistrationApiServiceTest.kt | 28 ++++++++++ .../OrganizationInvitationApiServiceTest.kt | 3 ++ 10 files changed, 128 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a32fbba33..bb15d8795 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 ### 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..ec9334621 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 @@ -36,8 +36,18 @@ class OrganizationInvitationApiService( ) { fun inviteOrganization(invitationInformation: InviteOrganizationRequest, adminUserId: String): IdResponse { + userService.assertUserDoesNotExistInDbOrThrow(invitationInformation.userEmail) + 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 +56,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..c97d2484d 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 @@ -41,8 +41,18 @@ class RegistrationApiService( ) { fun registerUserAndOrganization(registrationRequest: RegistrationRequestDto): IdResponse { + userService.assertUserDoesNotExistInDbOrThrow(registrationRequest.userEmail) + 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 +62,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..92ac2c9bb 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 @@ -35,6 +35,14 @@ class UserInvitationApiService( organizationId: String, adminUserId: String ): IdResponse { + userService.assertUserDoesNotExistInDbOrThrow(userInformation.email) + + // DB is source of truth, so we delete an existing user in Keycloak + val maybeExistingUserId = keycloakService.getUserIdByEmail(userInformation.email) + if (maybeExistingUserId != null) { + keycloakService.deleteUser(maybeExistingUserId) + } + 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..7ef456f58 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 @@ -20,6 +20,7 @@ import de.sovity.authorityportal.db.jooq.enums.UserRegistrationStatus import de.sovity.authorityportal.db.jooq.tables.records.UserRecord import de.sovity.authorityportal.web.model.CreateUserData import de.sovity.authorityportal.web.utils.TimeUtils +import de.sovity.authorityportal.web.utils.conflict import jakarta.enterprise.context.ApplicationScoped import org.jooq.DSLContext import org.jooq.impl.DSL @@ -65,6 +66,18 @@ class UserService( .fetchMap(u.ORGANIZATION_ID, DSL.count()) } + fun assertUserDoesNotExistInDbOrThrow(email: String) { + val u = Tables.USER + + val user = dsl.selectFrom(u) + .where(u.EMAIL.eq(email)) + .fetchOne() + + if (user != null) { + conflict("User with this email already exists") + } + } + 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..a37e9b8a1 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 @@ -29,22 +29,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 +57,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 +74,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 maybeExists = getUserIdByEmail(userEmail) + if (maybeExists != null) { + deleteUser(maybeExists) + } + + 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 +305,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/utils/HttpExceptionUtils.kt b/authority-portal-backend/authority-portal-quarkus/src/main/kotlin/de/sovity/authorityportal/web/utils/HttpExceptionUtils.kt index 748a2b851..dbcd01c1a 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 conflict(message: String = ""): Nothing { + throw WebApplicationException( + "User 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..49b76aeec 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 @@ -95,9 +95,13 @@ class FirstUserRegistrationTest { doNothing().whenever(keycloakService).sendInvitationEmail(any()) doNothing().whenever(keycloakService).createOrganization(any()) doNothing().whenever(keycloakService).joinOrganization(any(), any(), any()) + whenever(keycloakService.getUserIdByEmail(any())).thenReturn(null) whenever(keycloakService.createUser( eq(request.userEmail), eq(request.userFirstName), eq(request.userLastName), eq(request.userPassword) )).thenReturn(dummyDevUserUuid(0)) + whenever(keycloakService.createKeycloakUserAndOrganization( + any(), eq(request.userEmail), eq(request.userFirstName), eq(request.userLastName), any(), eq(request.userPassword) + )).thenReturn(dummyDevUserUuid(0)) // act val result = uiResource.registerUser(request) 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..a6dd941a6 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 @@ -100,6 +103,9 @@ class RegistrationApiServiceTest { whenever(keycloakService.createUser( eq(request.userEmail), eq(request.userFirstName), eq(request.userLastName), eq(request.userPassword) )).thenReturn(dummyDevUserUuid(1)) + whenever(keycloakService.createKeycloakUserAndOrganization( + any(), eq(request.userEmail), eq(request.userFirstName), eq(request.userLastName), any(), eq(request.userPassword) + )).thenReturn(dummyDevUserUuid(1)) ScenarioData().apply { organization(0, 0) @@ -174,4 +180,26 @@ 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..1f3de8240 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 @@ -102,6 +102,9 @@ class OrganizationInvitationApiServiceTest { eq("New"), eq("User"), isNull())).thenReturn(dummyDevUserUuid(1)) + whenever(keycloakService.createKeycloakUserAndOrganization( + any(), eq("new.user@test.sovity.io"), eq("New"), eq("User"), any(), eq(null) + )).thenReturn(dummyDevUserUuid(1)) doNothing().whenever(keycloakService).createOrganization(eq(dummyDevOrganizationId(1))) doNothing().whenever(keycloakService).joinOrganization(eq(dummyDevUserUuid(1)), eq(dummyDevOrganizationId(1)), any())