Skip to content

Commit

Permalink
fix: made portal db the source of truth on user creation (#387)
Browse files Browse the repository at this point in the history
  • Loading branch information
kamilczaja authored Dec 3, 2024
1 parent 37c52dd commit 82d1e1d
Show file tree
Hide file tree
Showing 11 changed files with 129 additions and 57 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
- 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)

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

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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)
}
Expand Down Expand Up @@ -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 }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
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 resourceAlreadyExists(message: String = ""): Nothing {
throw WebApplicationException(
"Resource 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 @@ -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
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 @@ -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 {
Expand Down Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 82d1e1d

Please sign in to comment.