From 4111312c8a57d6c3344cdac6772c8f4b7e4e8047 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Kleinb=C3=B6lting?= Date: Wed, 6 Mar 2024 21:15:00 +0100 Subject: [PATCH] refactor: Prevent illegal updates with `KnoraUserService` (#3098) --- .../admin/api/service/UsersRestService.scala | 22 ++++---- .../domain/service/KnoraUserService.scala | 50 +++++++++++++------ 2 files changed, 45 insertions(+), 27 deletions(-) diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/api/service/UsersRestService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/api/service/UsersRestService.scala index ff9715c935..5fdf2a623e 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/api/service/UsersRestService.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/api/service/UsersRestService.scala @@ -36,7 +36,6 @@ import org.knora.webapi.slice.admin.domain.service.KnoraUserService import org.knora.webapi.slice.admin.domain.service.KnoraUserToUserConverter import org.knora.webapi.slice.admin.domain.service.PasswordService import org.knora.webapi.slice.admin.domain.service.ProjectADMService -import org.knora.webapi.slice.admin.domain.service.UserChangeRequest import org.knora.webapi.slice.admin.domain.service.UserService import org.knora.webapi.slice.common.api.AuthorizationRestService import org.knora.webapi.slice.common.api.KnoraResponseRenderer @@ -135,14 +134,14 @@ final case class UsersRestService( _ <- ensureNotABuiltInUser(userIri) _ <- ensureSelfUpdateOrSystemAdmin(userIri, requestingUser) user <- getKnoraUserOrNotFound(userIri) - theChange = UserChangeRequest( - username = changeRequest.username, - email = changeRequest.email, - givenName = changeRequest.givenName, - familyName = changeRequest.familyName, - lang = changeRequest.lang, - ) - updated <- knoraUserService.updateUser(user, theChange) + updated <- knoraUserService.updateUser( + user, + changeRequest.username, + changeRequest.email, + changeRequest.givenName, + changeRequest.familyName, + changeRequest.lang, + ) response <- asExternalUserResponseADM(requestingUser, updated) } yield response @@ -177,7 +176,7 @@ final case class UsersRestService( _ <- ensureNotABuiltInUser(userIri) _ <- ensureSelfUpdateOrSystemAdmin(userIri, requestingUser) user <- getKnoraUserOrNotFound(userIri) - updated <- knoraUserService.updateUser(user, UserChangeRequest(status = Some(changeRequest.status))) + updated <- knoraUserService.updateUserStatus(user, changeRequest.status) response <- asExternalUserResponseADM(requestingUser, updated) } yield response @@ -190,8 +189,7 @@ final case class UsersRestService( _ <- ensureNotABuiltInUser(userIri) _ <- auth.ensureSystemAdmin(requestingUser) user <- getKnoraUserOrNotFound(userIri) - theUpdate = UserChangeRequest(systemAdmin = Some(changeRequest.systemAdmin)) - updated <- knoraUserService.updateUser(user, theUpdate) + updated <- knoraUserService.updateSystemAdminStatus(user, changeRequest.systemAdmin) response <- asExternalUserResponseADM(requestingUser, updated) } yield response diff --git a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala index 9d29d2ed50..d8ffcf9f54 100644 --- a/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala +++ b/webapi/src/main/scala/org/knora/webapi/slice/admin/domain/service/KnoraUserService.scala @@ -31,22 +31,9 @@ import org.knora.webapi.slice.admin.domain.model.User import org.knora.webapi.slice.admin.domain.model.UserStatus import org.knora.webapi.slice.admin.domain.model.Username import org.knora.webapi.slice.admin.domain.service.KnoraUserService.Errors.UserServiceError +import org.knora.webapi.slice.admin.domain.service.KnoraUserService.UserChangeRequest import org.knora.webapi.store.cache.CacheService -final case class UserChangeRequest( - username: Option[Username] = None, - email: Option[Email] = None, - givenName: Option[GivenName] = None, - familyName: Option[FamilyName] = None, - status: Option[UserStatus] = None, - lang: Option[LanguageCode] = None, - passwordHash: Option[PasswordHash] = None, - projects: Option[Chunk[ProjectIri]] = None, - projectsAdmin: Option[Chunk[ProjectIri]] = None, - groups: Option[Chunk[GroupIri]] = None, - systemAdmin: Option[SystemAdmin] = None, -) - case class KnoraUserService( private val userRepo: KnoraUserRepo, private val iriService: IriService, @@ -54,7 +41,26 @@ case class KnoraUserService( private val cacheService: CacheService, ) { - def updateUser(kUser: KnoraUser, update: UserChangeRequest): Task[KnoraUser] = { + def updateSystemAdminStatus(knoraUser: KnoraUser, status: SystemAdmin): Task[KnoraUser] = + updateUser(knoraUser, UserChangeRequest(systemAdmin = Some(status))) + + def updateUserStatus(knoraUser: KnoraUser, status: UserStatus): Task[KnoraUser] = + updateUser(knoraUser, UserChangeRequest(status = Some(status))) + + def updateUser( + knoraUser: KnoraUser, + username: Option[Username] = None, + email: Option[Email], + givenName: Option[GivenName] = None, + familyName: Option[FamilyName] = None, + lang: Option[LanguageCode] = None, + ): Task[KnoraUser] = { + val theUpdate = + UserChangeRequest(username = username, email = email, givenName = givenName, familyName = familyName, lang = lang) + updateUser(knoraUser, theUpdate) + } + + private def updateUser(kUser: KnoraUser, update: UserChangeRequest): Task[KnoraUser] = { val updatedUser = kUser.copy( username = update.username.getOrElse(kUser.username), email = update.email.getOrElse(kUser.email), @@ -210,6 +216,20 @@ case class KnoraUserService( } object KnoraUserService { + private final case class UserChangeRequest( + username: Option[Username] = None, + email: Option[Email] = None, + givenName: Option[GivenName] = None, + familyName: Option[FamilyName] = None, + status: Option[UserStatus] = None, + lang: Option[LanguageCode] = None, + passwordHash: Option[PasswordHash] = None, + projects: Option[Chunk[ProjectIri]] = None, + projectsAdmin: Option[Chunk[ProjectIri]] = None, + groups: Option[Chunk[GroupIri]] = None, + systemAdmin: Option[SystemAdmin] = None, + ) + object Errors { final case class UserServiceError(message: String) }