From 0dc5c2a8a5b91e083242a13212f18d8c4753e406 Mon Sep 17 00:00:00 2001 From: yatharthranjan Date: Tue, 22 Jul 2025 13:07:39 +0100 Subject: [PATCH 1/8] add check for duplicate external user ids (service ids) --- .../service/RestSourceUserService.kt | 33 +++++++++++++++++ ...add_unique_constraint_external_user_id.xml | 37 +++++++++++++++++++ .../changelog/changes/db.changelog-master.xml | 1 + 3 files changed, 71 insertions(+) create mode 100644 authorizer-app-backend/src/main/resources/db/changelog/changes/20240607120000_add_unique_constraint_external_user_id.xml diff --git a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt index abdde814..fe2158a9 100644 --- a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt +++ b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt @@ -40,6 +40,22 @@ class RestSourceUserService( suspend fun create(userDto: RestSourceUserDTO): RestSourceUserDTO { userDto.ensure() + + // Check if the service user ID is already in use for the same source type. + userDto.serviceUserId?.let { serviceUserId -> + val existingExternalUser = userRepository.findByExternalId( + externalId = serviceUserId, + sourceType = userDto.sourceType, + ) + if (existingExternalUser != null) { + val response = Response.status(Response.Status.CONFLICT) + .entity(mapOf("status" to 409, "message" to "Account with this service user ID $serviceUserId already exists for source type ${userDto.sourceType}.", "user" to userMapper.fromEntity(existingExternalUser))) + .build() + + throw WebApplicationException(response) + } + } + val existingUser = userRepository.findByUserIdProjectIdSourceType( userId = userDto.userId!!, projectId = userDto.projectId!!, @@ -52,6 +68,7 @@ class RestSourceUserService( throw WebApplicationException(response) } + val user = userRepository.create(userDto) return userMapper.fromEntity(user) } @@ -69,6 +86,22 @@ class RestSourceUserService( suspend fun update(userId: Long, user: RestSourceUserDTO): RestSourceUserDTO { user.ensure() + + // Check if the service user ID is already in use for the same source type but different user. + user.serviceUserId?.let { serviceUserId -> + val existingExternalUser = userRepository.findByExternalId( + externalId = serviceUserId, + sourceType = user.sourceType, + ) + if (existingExternalUser != null && existingExternalUser.id != userId) { + val response = Response.status(Response.Status.CONFLICT) + .entity(mapOf("status" to 409, "message" to "Account with this service user ID $serviceUserId already exists for source type ${user.sourceType}.", "user" to userMapper.fromEntity(existingExternalUser))) + .build() + + throw WebApplicationException(response) + } + } + return userMapper.fromEntity( runLocked(userId) { userRepository.update(userId, user) diff --git a/authorizer-app-backend/src/main/resources/db/changelog/changes/20240607120000_add_unique_constraint_external_user_id.xml b/authorizer-app-backend/src/main/resources/db/changelog/changes/20240607120000_add_unique_constraint_external_user_id.xml new file mode 100644 index 00000000..ec92a961 --- /dev/null +++ b/authorizer-app-backend/src/main/resources/db/changelog/changes/20240607120000_add_unique_constraint_external_user_id.xml @@ -0,0 +1,37 @@ + + + + + + + + SELECT COUNT(*) FROM ( + SELECT external_user_id FROM rest_source_user + WHERE external_user_id IS NOT NULL + GROUP BY external_user_id + HAVING COUNT(*) > 1 + ) t; + + + + UPDATE rest_source_user a + SET external_user_id = 'DUPLICATE_INVESTIGATE_' || a.external_user_id + FROM rest_source_user b + WHERE a.external_user_id = b.external_user_id + AND a.id > b.id + AND a.external_user_id IS NOT NULL + AND b.id = ( + SELECT MIN(id) FROM rest_source_user c + WHERE c.external_user_id = a.external_user_id + ); + + + + \ No newline at end of file diff --git a/authorizer-app-backend/src/main/resources/db/changelog/changes/db.changelog-master.xml b/authorizer-app-backend/src/main/resources/db/changelog/changes/db.changelog-master.xml index cc8e142c..d392157e 100644 --- a/authorizer-app-backend/src/main/resources/db/changelog/changes/db.changelog-master.xml +++ b/authorizer-app-backend/src/main/resources/db/changelog/changes/db.changelog-master.xml @@ -11,4 +11,5 @@ + From 7d48d610f3abd4e66411a8613e3350fb2011bd96 Mon Sep 17 00:00:00 2001 From: yatharthranjan Date: Tue, 22 Jul 2025 15:23:54 +0100 Subject: [PATCH 2/8] add unique constraint at JPA layer --- .../java/org/radarbase/authorizer/doa/entity/RestSourceUser.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/doa/entity/RestSourceUser.kt b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/doa/entity/RestSourceUser.kt index 261ea776..2216532b 100644 --- a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/doa/entity/RestSourceUser.kt +++ b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/doa/entity/RestSourceUser.kt @@ -26,6 +26,7 @@ import jakarta.persistence.Id import jakarta.persistence.OneToMany import jakarta.persistence.SequenceGenerator import jakarta.persistence.Table +import jakarta.persistence.UniqueConstraint import org.hibernate.annotations.BatchSize import org.hibernate.annotations.Cache import org.hibernate.annotations.CacheConcurrencyStrategy @@ -36,7 +37,7 @@ import java.util.Objects import java.util.UUID @Entity -@Table(name = "rest_source_user") +@Table(name = "rest_source_user", uniqueConstraints = [UniqueConstraint(columnNames = ["external_user_id", "source_type"])]) @Cache(usage = CacheConcurrencyStrategy.TRANSACTIONAL) class RestSourceUser( @Id From a9e0ad111aa144e8ae8dfdb0cb432edc3462c2f6 Mon Sep 17 00:00:00 2001 From: Pauline Date: Mon, 1 Sep 2025 18:12:38 +0100 Subject: [PATCH 3/8] Update db changelog --- ...add_unique_constraint_external_user_id.xml | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/authorizer-app-backend/src/main/resources/db/changelog/changes/20240607120000_add_unique_constraint_external_user_id.xml b/authorizer-app-backend/src/main/resources/db/changelog/changes/20240607120000_add_unique_constraint_external_user_id.xml index ec92a961..12b4e4ea 100644 --- a/authorizer-app-backend/src/main/resources/db/changelog/changes/20240607120000_add_unique_constraint_external_user_id.xml +++ b/authorizer-app-backend/src/main/resources/db/changelog/changes/20240607120000_add_unique_constraint_external_user_id.xml @@ -7,31 +7,35 @@ - - - SELECT COUNT(*) FROM ( - SELECT external_user_id FROM rest_source_user - WHERE external_user_id IS NOT NULL - GROUP BY external_user_id - HAVING COUNT(*) > 1 - ) t; - + + + + SELECT COUNT(*) FROM ( + SELECT external_user_id, source_type FROM rest_source_user + WHERE external_user_id IS NOT NULL + GROUP BY external_user_id, source_type + HAVING COUNT(*) > 1 + ) t; + + UPDATE rest_source_user a - SET external_user_id = 'DUPLICATE_INVESTIGATE_' || a.external_user_id + SET external_user_id = 'DUPLICATE_INVESTIGATE_' || a.id || '_' || a.external_user_id FROM rest_source_user b WHERE a.external_user_id = b.external_user_id + AND a.source_type = b.source_type AND a.id > b.id AND a.external_user_id IS NOT NULL AND b.id = ( SELECT MIN(id) FROM rest_source_user c WHERE c.external_user_id = a.external_user_id + AND c.source_type = a.source_type ); + columnNames="external_user_id,source_type" + constraintName="uk_rest_source_user_external_user_id_source_type"/> \ No newline at end of file From f8fa7bdcc4ceba1ce793e2e5c1ea690a7e63bc8a Mon Sep 17 00:00:00 2001 From: Pauline Date: Mon, 1 Sep 2025 18:49:43 +0100 Subject: [PATCH 4/8] Add separate existing external user id check when updating the user token --- .../resources/RegistrationResource.kt | 2 +- .../service/RestSourceUserService.kt | 59 +++++++++---------- 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/resources/RegistrationResource.kt b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/resources/RegistrationResource.kt index 06050e48..6e0de275 100644 --- a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/resources/RegistrationResource.kt +++ b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/resources/RegistrationResource.kt @@ -154,7 +154,7 @@ class RegistrationResource( ) = asyncService.runAsCoroutine(asyncResponse) { val registration = registrationService.ensureRegistration(token) val accessToken = authorizationService.requestAccessToken(payload, registration.user.sourceType) - val user = userRepository.updateToken(accessToken, registration.user) + val user = restSourceUserService.updateUserToken(accessToken, registration.user) val project = registration.user.projectId?.let { projectService.project(it).toProject() } diff --git a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt index fe2158a9..05845f10 100644 --- a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt +++ b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt @@ -5,6 +5,7 @@ import jakarta.ws.rs.core.Context import jakarta.ws.rs.core.Response import org.radarbase.auth.authorization.EntityDetails import org.radarbase.auth.authorization.Permission +import org.radarbase.authorizer.api.RestOauth2AccessToken import org.radarbase.authorizer.api.RestSourceUserDTO import org.radarbase.authorizer.api.RestSourceUserMapper import org.radarbase.authorizer.api.TokenDTO @@ -13,6 +14,7 @@ import org.radarbase.authorizer.doa.entity.RestSourceUser import org.radarbase.jersey.auth.AuthService import org.radarbase.jersey.exception.HttpApplicationException import org.radarbase.jersey.exception.HttpBadRequestException +import org.radarbase.jersey.exception.HttpConflictException import org.radarbase.jersey.exception.HttpNotFoundException import kotlin.time.Duration.Companion.seconds @@ -41,21 +43,6 @@ class RestSourceUserService( suspend fun create(userDto: RestSourceUserDTO): RestSourceUserDTO { userDto.ensure() - // Check if the service user ID is already in use for the same source type. - userDto.serviceUserId?.let { serviceUserId -> - val existingExternalUser = userRepository.findByExternalId( - externalId = serviceUserId, - sourceType = userDto.sourceType, - ) - if (existingExternalUser != null) { - val response = Response.status(Response.Status.CONFLICT) - .entity(mapOf("status" to 409, "message" to "Account with this service user ID $serviceUserId already exists for source type ${userDto.sourceType}.", "user" to userMapper.fromEntity(existingExternalUser))) - .build() - - throw WebApplicationException(response) - } - } - val existingUser = userRepository.findByUserIdProjectIdSourceType( userId = userDto.userId!!, projectId = userDto.projectId!!, @@ -87,21 +74,6 @@ class RestSourceUserService( suspend fun update(userId: Long, user: RestSourceUserDTO): RestSourceUserDTO { user.ensure() - // Check if the service user ID is already in use for the same source type but different user. - user.serviceUserId?.let { serviceUserId -> - val existingExternalUser = userRepository.findByExternalId( - externalId = serviceUserId, - sourceType = user.sourceType, - ) - if (existingExternalUser != null && existingExternalUser.id != userId) { - val response = Response.status(Response.Status.CONFLICT) - .entity(mapOf("status" to 409, "message" to "Account with this service user ID $serviceUserId already exists for source type ${user.sourceType}.", "user" to userMapper.fromEntity(existingExternalUser))) - .build() - - throw WebApplicationException(response) - } - } - return userMapper.fromEntity( runLocked(userId) { userRepository.update(userId, user) @@ -139,6 +111,31 @@ class RestSourceUserService( ) } + /** + * Validates that an external user ID is not already in use by another user. + * Should be called before updating a user's token with an external user ID. + */ + private suspend fun validateExternalUserId(token: RestOauth2AccessToken?, user: RestSourceUser) { + if (token?.externalUserId != null) { + val existingUser = userRepository.findByExternalId(token.externalUserId, user.sourceType) + if (existingUser != null && existingUser.id != user.id) { + throw HttpConflictException( + "external_user_id_already_exists", + "External user ID ${token.externalUserId} is already registered for another user of source type ${user.sourceType}", + ) + } + } + } + + /** + * Updates a user's token after validating the external user ID. + * This method ensures no duplicate external user IDs exist in the system. + */ + suspend fun updateUserToken(token: RestOauth2AccessToken?, user: RestSourceUser): RestSourceUser { + validateExternalUserId(token, user) + return userRepository.updateToken(token, user) + } + suspend fun ensureToken(userId: Long): TokenDTO { ensureUser(userId, Permission.MEASUREMENT_CREATE) return runLocked(userId) { user -> @@ -184,7 +181,7 @@ class RestSourceUserService( } val token = authorizationService.refreshToken(user) - val updatedUser = userRepository.updateToken(token, user) + val updatedUser = updateUserToken(token, user) if (!updatedUser.authorized) { throw HttpApplicationException( From 4a3d76186feede8a2791744e79a99857bccc50ac Mon Sep 17 00:00:00 2001 From: Pauline Date: Mon, 1 Sep 2025 18:55:51 +0100 Subject: [PATCH 5/8] Remove unnecessary spaces --- .../org/radarbase/authorizer/service/RestSourceUserService.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt index 05845f10..f4ea2da7 100644 --- a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt +++ b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt @@ -42,7 +42,6 @@ class RestSourceUserService( suspend fun create(userDto: RestSourceUserDTO): RestSourceUserDTO { userDto.ensure() - val existingUser = userRepository.findByUserIdProjectIdSourceType( userId = userDto.userId!!, projectId = userDto.projectId!!, @@ -55,7 +54,6 @@ class RestSourceUserService( throw WebApplicationException(response) } - val user = userRepository.create(userDto) return userMapper.fromEntity(user) } @@ -73,7 +71,6 @@ class RestSourceUserService( suspend fun update(userId: Long, user: RestSourceUserDTO): RestSourceUserDTO { user.ensure() - return userMapper.fromEntity( runLocked(userId) { userRepository.update(userId, user) From 49109acfab50ecf88a4030350f5bfcef20eecb61 Mon Sep 17 00:00:00 2001 From: Pauline Date: Tue, 2 Sep 2025 11:50:10 +0100 Subject: [PATCH 6/8] Show subjectId in error message for duplicate service ids --- .../org/radarbase/authorizer/service/RestSourceUserService.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt index f4ea2da7..1c8d7e54 100644 --- a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt +++ b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt @@ -118,7 +118,7 @@ class RestSourceUserService( if (existingUser != null && existingUser.id != user.id) { throw HttpConflictException( "external_user_id_already_exists", - "External user ID ${token.externalUserId} is already registered for another user of source type ${user.sourceType}", + "External user ID ${token.externalUserId} is already registered for another user of source type ${user.sourceType} and user id ${existingUser.userId}", ) } } From fc94f825cc57386a306ffd65a7cafbce2abeb582 Mon Sep 17 00:00:00 2001 From: Pauline Date: Fri, 3 Oct 2025 10:30:22 +0100 Subject: [PATCH 7/8] Remove unused userRepository in RegistrationResource --- .../org/radarbase/authorizer/resources/RegistrationResource.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/resources/RegistrationResource.kt b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/resources/RegistrationResource.kt index 6e0de275..6a5f5a2b 100644 --- a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/resources/RegistrationResource.kt +++ b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/resources/RegistrationResource.kt @@ -22,7 +22,6 @@ import org.radarbase.authorizer.api.StateCreateDTO import org.radarbase.authorizer.api.TokenSecret import org.radarbase.authorizer.api.toProject import org.radarbase.authorizer.doa.RegistrationRepository -import org.radarbase.authorizer.doa.RestSourceUserRepository import org.radarbase.authorizer.service.RegistrationService import org.radarbase.authorizer.service.RestSourceAuthorizationService import org.radarbase.authorizer.service.RestSourceUserService @@ -45,7 +44,6 @@ class RegistrationResource( @Context private val registrationRepository: RegistrationRepository, @Context private val restSourceUserService: RestSourceUserService, @Context private val authorizationService: RestSourceAuthorizationService, - @Context private val userRepository: RestSourceUserRepository, @Context private val registrationService: RegistrationService, @Context private val projectService: RadarProjectService, @Context private val authService: AuthService, From 071de94936570a11e72199bd44fa6383c306fbe7 Mon Sep 17 00:00:00 2001 From: Pauline Date: Fri, 3 Oct 2025 10:31:44 +0100 Subject: [PATCH 8/8] Throw error when external user id is null when validating this --- .../radarbase/authorizer/service/RestSourceUserService.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt index 1c8d7e54..ae1e0fbe 100644 --- a/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt +++ b/authorizer-app-backend/src/main/java/org/radarbase/authorizer/service/RestSourceUserService.kt @@ -121,6 +121,11 @@ class RestSourceUserService( "External user ID ${token.externalUserId} is already registered for another user of source type ${user.sourceType} and user id ${existingUser.userId}", ) } + } else { + throw HttpBadRequestException( + "missing_external_user_id", + "External user ID cannot be empty", + ) } }