Skip to content

Commit

Permalink
fix: made portal db the source of truth on user creation
Browse files Browse the repository at this point in the history
  • Loading branch information
kamilczaja committed Dec 2, 2024
1 parent fc6e2e5 commit dc2a7c2
Show file tree
Hide file tree
Showing 10 changed files with 128 additions and 42 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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)
}
Expand Down Expand Up @@ -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 }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand All @@ -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()
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,17 @@ 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
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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ class OrganizationInvitationApiServiceTest {
eq("New"),
eq("User"),
isNull())).thenReturn(dummyDevUserUuid(1))
whenever(keycloakService.createKeycloakUserAndOrganization(
any(), eq("[email protected]"), 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())

Expand Down

0 comments on commit dc2a7c2

Please sign in to comment.