fix: Fix UserIri and allow existing values (DEV-3194) (#2997)

Co-authored-by: Marcin Procyk <marcin.procyk@dasch.swiss>

integration/src/test/scala/org/knora/webapi/core/LayersTest.scala

@@ -33,7 +33,7 @@ import org.knora.webapi.slice.admin.api.service.MaintenanceRestService
 import org.knora.webapi.slice.admin.api.service.PermissionsRestService
 import org.knora.webapi.slice.admin.api.service.ProjectADMRestService
 import org.knora.webapi.slice.admin.api.service.ProjectsADMRestServiceLive
-import org.knora.webapi.slice.admin.api.service.UsersADMRestServiceLive
+import org.knora.webapi.slice.admin.api.service.UsersRestService
 import org.knora.webapi.slice.admin.domain.service.*
 import org.knora.webapi.slice.admin.repo.service.KnoraProjectRepoLive
 import org.knora.webapi.slice.common.api.*
@@ -217,7 +217,7 @@ object LayersTest {
       TapirToPekkoInterpreter.layer,
       TestClientService.layer,
       TriplestoreServiceLive.layer,
-      UsersADMRestServiceLive.layer,
+      UsersRestService.layer,
       UsersEndpoints.layer,
       UsersEndpointsHandler.layer,
       UsersResponderADMLive.layer,

webapi/src/main/scala/org/knora/webapi/core/LayersLive.scala

@@ -34,7 +34,7 @@ import org.knora.webapi.slice.admin.api.service.MaintenanceRestService
 import org.knora.webapi.slice.admin.api.service.PermissionsRestService
 import org.knora.webapi.slice.admin.api.service.ProjectADMRestService
 import org.knora.webapi.slice.admin.api.service.ProjectsADMRestServiceLive
-import org.knora.webapi.slice.admin.api.service.UsersADMRestServiceLive
+import org.knora.webapi.slice.admin.api.service.UsersRestService
 import org.knora.webapi.slice.admin.domain.service.*
 import org.knora.webapi.slice.admin.repo.service.KnoraProjectRepoLive
 import org.knora.webapi.slice.common.api.*
@@ -166,7 +166,7 @@ object LayersLive {
       StringFormatter.live,
       TapirToPekkoInterpreter.layer,
       TriplestoreServiceLive.layer,
-      UsersADMRestServiceLive.layer,
+      UsersRestService.layer,
       UsersEndpoints.layer,
       UsersEndpointsHandler.layer,
       UsersResponderADMLive.layer,

webapi/src/main/scala/org/knora/webapi/messages/admin/responder/usersmessages/UsersMessagesADM.scala

@@ -474,7 +474,7 @@ case class UserGroupMembershipsGetResponseADM(groups: Seq[GroupADM]) extends Kno
  *
  * @param user the new user profile of the created/modified user.
  */
-case class UserOperationResponseADM(user: User) extends KnoraResponseADM {
+case class UserOperationResponseADM(user: User) extends AdminKnoraResponseADM {
   def toJsValue: JsValue = UsersADMJsonProtocol.userOperationResponseADMFormat.write(this)
 }
 

webapi/src/main/scala/org/knora/webapi/responders/admin/UsersResponderADM.scala

@@ -57,6 +57,24 @@ import org.knora.webapi.util.ZioHelper
 @accessible
 trait UsersResponderADM {
   def getAllUserADMRequest(requestingUser: User): Task[UsersGetResponseADM]
+
+  /**
+   * Change the user's status (active / inactive).
+   *
+   * @param userIri        the IRI of the existing user that we want to update.
+   * @param status         the new status.
+   * @param requestingUser the requesting user.
+   * @param apiRequestID   the unique api request ID.
+   * @return a task containing a [[UserOperationResponseADM]].
+   *         fails with a [[BadRequestException]] if necessary parameters are not supplied.
+   *         fails with a [[ForbiddenException]] if the requestingUser doesn't hold the necessary permission for the operation.
+   */
+  def changeUserStatusADM(
+    userIri: IRI,
+    status: UserStatus,
+    requestingUser: User,
+    apiRequestID: UUID
+  ): Task[UserOperationResponseADM]
 }
 
 final case class UsersResponderADMLive(
@@ -601,19 +619,7 @@ final case class UsersResponderADMLive(
     )
   }
 
-  /**
-   * Change the user's status (active / inactive).
-   *
-   * @param userIri              the IRI of the existing user that we want to update.
-   * @param status               the new status.
-   *
-   * @param requestingUser       the requesting user.
-   * @param apiRequestID         the unique api request ID.
-   * @return a future containing a [[UserOperationResponseADM]].
-   *         fails with a [[BadRequestException]] if necessary parameters are not supplied.
-   *         fails with a [[ForbiddenException]] if the user doesn't hold the necessary permission for the operation.
-   */
-  private def changeUserStatusADM(
+  override def changeUserStatusADM(
     userIri: IRI,
     status: UserStatus,
     requestingUser: User,

webapi/src/main/scala/org/knora/webapi/routing/admin/UsersRouteADM.scala

@@ -16,19 +16,13 @@ import org.knora.webapi.core.MessageRelay
 import org.knora.webapi.messages.StringFormatter
 import org.knora.webapi.messages.admin.responder.usersmessages.UsersADMJsonProtocol.*
 import org.knora.webapi.messages.admin.responder.usersmessages.*
-import org.knora.webapi.messages.util.KnoraSystemInstances.Users.AnonymousUser
-import org.knora.webapi.messages.util.KnoraSystemInstances.Users.SystemUser
 import org.knora.webapi.routing.Authenticator
 import org.knora.webapi.routing.RouteUtilADM
 import org.knora.webapi.routing.RouteUtilADM.getIriUser
 import org.knora.webapi.routing.RouteUtilADM.getIriUserUuid
 import org.knora.webapi.routing.RouteUtilADM.getUserUuid
 import org.knora.webapi.routing.RouteUtilADM.runJsonRouteZ
-import org.knora.webapi.slice.admin.domain.model.Email
-import org.knora.webapi.slice.admin.domain.model.SystemAdmin
-import org.knora.webapi.slice.admin.domain.model.UserIri
-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.model.*
 
 /**
  * Provides an pekko-http-routing function for API routes that deal with users.
@@ -47,7 +41,6 @@ final case class UsersRouteADM()(
       changeUserBasicInformation() ~
       changeUserPassword() ~
       changeUserStatus() ~
-      deleteUser() ~
       changeUserSystemAdminMembership() ~
       getUsersProjectMemberships() ~
       addUserToProjectMembership() ~
@@ -122,7 +115,7 @@ final case class UsersRouteADM()(
       put {
         entity(as[ChangeUserApiRequestADM]) { apiRequest => requestContext =>
           val task = for {
-            checkedUserIri <- validateAndEscapeUserIri(userIri)
+            checkedUserIri <- validateUserIriAndEnsureRegularUser(userIri)
             r              <- getUserUuid(requestContext)
             payload <- UserUpdateBasicInformationPayloadADM
                          .make(apiRequest)
@@ -143,7 +136,7 @@ final case class UsersRouteADM()(
       put {
         entity(as[ChangeUserPasswordApiRequestADM]) { apiRequest => requestContext =>
           val task = for {
-            checkedUserIri <- validateAndEscapeUserIri(userIri)
+            checkedUserIri <- validateUserIriAndEnsureRegularUser(userIri)
             payload        <- UserUpdatePasswordPayloadADM.make(apiRequest).mapError(BadRequestException(_)).toZIO
             r              <- getUserUuid(requestContext)
           } yield UserChangePasswordRequestADM(checkedUserIri, payload, r.user, r.uuid)
@@ -163,27 +156,14 @@ final case class UsersRouteADM()(
             newStatus <- ZIO
                            .fromOption(apiRequest.status.map(UserStatus.from))
                            .orElseFail(BadRequestException("The status is missing."))
-            checkedUserIri <- validateAndEscapeUserIri(userIri)
+            checkedUserIri <- validateUserIriAndEnsureRegularUser(userIri)
             r              <- getUserUuid(requestContext)
           } yield UserChangeStatusRequestADM(checkedUserIri, newStatus, r.user, r.uuid)
           runJsonRouteZ(task, requestContext)
         }
       }
     }
 
-  /**
-   * delete a user identified by iri (change status to false).
-   */
-  private def deleteUser(): Route = path(usersBasePath / "iri" / Segment) { userIri =>
-    delete { requestContext =>
-      val task = for {
-        checkedUserIri <- validateAndEscapeUserIri(userIri)
-        r              <- getUserUuid(requestContext)
-      } yield UserChangeStatusRequestADM(checkedUserIri, UserStatus.from(false), r.user, r.uuid)
-      runJsonRouteZ(task, requestContext)
-    }
-  }
-
   /**
    * Change user's SystemAdmin membership.
    */
@@ -192,7 +172,7 @@ final case class UsersRouteADM()(
       put {
         entity(as[ChangeUserApiRequestADM]) { apiRequest => requestContext =>
           val task = for {
-            checkedUserIri <- validateAndEscapeUserIri(userIri)
+            checkedUserIri <- validateUserIriAndEnsureRegularUser(userIri)
             r              <- getUserUuid(requestContext)
             newSystemAdmin <- ZIO
                                 .fromOption(apiRequest.systemAdmin.map(SystemAdmin.from))
@@ -210,7 +190,7 @@ final case class UsersRouteADM()(
     path(usersBasePath / "iri" / Segment / "project-memberships") { userIri =>
       get { requestContext =>
         val requestTask = for {
-          checkedUserIri <- validateAndEscapeUserIri(userIri)
+          checkedUserIri <- validateUserIriAndEnsureRegularUser(userIri)
           requestingUser <- Authenticator.getUserADM(requestContext)
         } yield UserProjectMembershipsGetRequestADM(checkedUserIri, requestingUser)
         runJsonRouteZ(requestTask, requestContext)
@@ -224,24 +204,18 @@ final case class UsersRouteADM()(
     path(usersBasePath / "iri" / Segment / "project-memberships" / Segment) { (userIri, projectIri) =>
       post { requestContext =>
         val task = for {
-          checkedUserIri <- validateAndEscapeUserIri(userIri)
-          r              <- getIriUserUuid(projectIri, requestContext)
-        } yield UserProjectMembershipAddRequestADM(checkedUserIri, r.iri, r.user, r.uuid)
+          userIri <- validateUserIriAndEnsureRegularUser(userIri)
+          r       <- getIriUserUuid(projectIri, requestContext)
+        } yield UserProjectMembershipAddRequestADM(userIri, r.iri, r.user, r.uuid)
         runJsonRouteZ(task, requestContext)
       }
     }
 
-  private def validateAndEscapeUserIri(userIri: String): ZIO[StringFormatter, BadRequestException, String] =
-    for {
-      nonEmptyIri <- ZIO.succeed(userIri).filterOrFail(_.nonEmpty)(BadRequestException("User IRI cannot be empty"))
-      checkedUserIri <-
-        ZIO
-          .fromOption(Iri.validateAndEscapeUserIri(nonEmptyIri))
-          .orElseFail(BadRequestException(s"Invalid user IRI $userIri"))
-          .filterOrFail(isNotBuildInUser)(BadRequestException("Changes to built-in users are not allowed."))
-    } yield checkedUserIri
-
-  private def isNotBuildInUser(it: String) = !it.equals(SystemUser.id) && !it.equals(AnonymousUser.id)
+  private def validateUserIriAndEnsureRegularUser(userIri: String) =
+    ZIO
+      .fromEither(UserIri.from(userIri))
+      .filterOrFail(_.isRegularUser)("Changes to built-in users are not allowed.")
+      .mapBoth(BadRequestException.apply, _.value)
 
   private def validateAndEscapeGroupIri(groupIri: String) =
     Iri
@@ -256,7 +230,7 @@ final case class UsersRouteADM()(
     path(usersBasePath / "iri" / Segment / "project-memberships" / Segment) { (userIri, projectIri) =>
       delete { requestContext =>
         val task = for {
-          checkedUserIri <- validateAndEscapeUserIri(userIri)
+          checkedUserIri <- validateUserIriAndEnsureRegularUser(userIri)
           r              <- getIriUserUuid(projectIri, requestContext)
         } yield UserProjectMembershipRemoveRequestADM(checkedUserIri, r.iri, r.user, r.uuid)
         runJsonRouteZ(task, requestContext)
@@ -284,7 +258,7 @@ final case class UsersRouteADM()(
     path(usersBasePath / "iri" / Segment / "project-admin-memberships" / Segment) { (userIri, projectIri) =>
       post { ctx =>
         val task = for {
-          checkedUserIri <- validateAndEscapeUserIri(userIri)
+          checkedUserIri <- validateUserIriAndEnsureRegularUser(userIri)
           r              <- getIriUserUuid(projectIri, ctx)
         } yield UserProjectAdminMembershipAddRequestADM(checkedUserIri, r.iri, r.user, r.uuid)
         runJsonRouteZ(task, ctx)
@@ -298,7 +272,7 @@ final case class UsersRouteADM()(
     path(usersBasePath / "iri" / Segment / "project-admin-memberships" / Segment) { (userIri, projectIri) =>
       delete { requestContext =>
         val task = for {
-          checkedUserIri <- validateAndEscapeUserIri(userIri)
+          checkedUserIri <- validateUserIriAndEnsureRegularUser(userIri)
           r              <- getIriUserUuid(projectIri, requestContext)
         } yield UserProjectAdminMembershipRemoveRequestADM(checkedUserIri, r.iri, r.user, r.uuid)
         runJsonRouteZ(task, requestContext)
@@ -326,7 +300,7 @@ final case class UsersRouteADM()(
     path(usersBasePath / "iri" / Segment / "group-memberships" / Segment) { (userIri, groupIri) =>
       post { requestContext =>
         val task = for {
-          checkedUserIri  <- validateAndEscapeUserIri(userIri)
+          checkedUserIri  <- validateUserIriAndEnsureRegularUser(userIri)
           checkedGroupIri <- validateAndEscapeGroupIri(groupIri)
           r               <- getIriUserUuid(groupIri, requestContext)
         } yield UserGroupMembershipAddRequestADM(checkedUserIri, checkedGroupIri, r.user, r.uuid)
@@ -341,7 +315,7 @@ final case class UsersRouteADM()(
     path(usersBasePath / "iri" / Segment / "group-memberships" / Segment) { (userIri, groupIri) =>
       delete { requestContext =>
         val task = for {
-          checkedUserIri  <- validateAndEscapeUserIri(userIri)
+          checkedUserIri  <- validateUserIriAndEnsureRegularUser(userIri)
           checkedGroupIri <- validateAndEscapeGroupIri(groupIri)
           r               <- getIriUserUuid(groupIri, requestContext)
         } yield UserGroupMembershipRemoveRequestADM(checkedUserIri, checkedGroupIri, r.user, r.uuid)

webapi/src/main/scala/org/knora/webapi/slice/admin/api/Codecs.scala

@@ -12,6 +12,7 @@ import zio.json.JsonCodec
 import dsp.valueobjects.V2
 import org.knora.webapi.slice.admin.api.model.MaintenanceRequests.AssetId
 import org.knora.webapi.slice.admin.domain.model.KnoraProject.*
+import org.knora.webapi.slice.admin.domain.model.UserIri
 import org.knora.webapi.slice.common.Value.BooleanValue
 import org.knora.webapi.slice.common.Value.StringValue
 import org.knora.webapi.slice.common.domain.SparqlEncodedString
@@ -40,6 +41,7 @@ object Codecs {
     implicit val shortname: StringCodec[Shortname]                     = stringCodec(Shortname.from)
     implicit val sparqlEncodedString: StringCodec[SparqlEncodedString] = stringCodec(SparqlEncodedString.from)
     implicit val status: StringCodec[Status]                           = booleanCodec(Status.from)
+    implicit val userIri: StringCodec[UserIri]                         = stringCodec(UserIri.from)
   }
 
   object ZioJsonCodec {

webapi/src/main/scala/org/knora/webapi/slice/admin/api/UsersEndpoints.scala

@@ -10,10 +10,17 @@ import sttp.tapir.generic.auto.*
 import sttp.tapir.json.spray.jsonBody as sprayJsonBody
 import zio.*
 
+import org.knora.webapi.messages.admin.responder.usersmessages.UserOperationResponseADM
 import org.knora.webapi.messages.admin.responder.usersmessages.UsersADMJsonProtocol.*
 import org.knora.webapi.messages.admin.responder.usersmessages.UsersGetResponseADM
+import org.knora.webapi.slice.admin.api.Codecs.TapirCodec.userIri
+import org.knora.webapi.slice.admin.domain.model.UserIri
 import org.knora.webapi.slice.common.api.BaseEndpoints
 
+object PathVars {
+  val userIriPathVar: EndpointInput.PathCapture[UserIri] =
+    path[UserIri].description("The user IRI. Must be URL-encoded.")
+}
 final case class UsersEndpoints(baseEndpoints: BaseEndpoints) {
   private val base = "admin" / "users"
   private val tags = List("Users", "Admin API")
@@ -24,7 +31,13 @@ final case class UsersEndpoints(baseEndpoints: BaseEndpoints) {
     .description("Returns all users.")
     .tags(tags)
 
-  val endpoints: Seq[AnyEndpoint] = Seq(getUsers).map(_.endpoint)
+  val deleteUser = baseEndpoints.securedEndpoint.delete
+    .in(base / "iri" / PathVars.userIriPathVar)
+    .out(sprayJsonBody[UserOperationResponseADM])
+    .description("Delete a user identified by IRI (change status to false).")
+    .tags(tags)
+
+  val endpoints: Seq[AnyEndpoint] = Seq(getUsers, deleteUser).map(_.endpoint)
 }
 
 object UsersEndpoints {

webapi/src/main/scala/org/knora/webapi/slice/admin/api/UsersEndpointsHandler.scala

@@ -7,27 +7,35 @@ package org.knora.webapi.slice.admin.api
 
 import zio.ZLayer
 
+import org.knora.webapi.messages.admin.responder.usersmessages.UserOperationResponseADM
 import org.knora.webapi.messages.admin.responder.usersmessages.UsersGetResponseADM
-import org.knora.webapi.slice.admin.api.service.UsersADMRestService
+import org.knora.webapi.slice.admin.api.service.UsersRestService
+import org.knora.webapi.slice.admin.domain.model.UserIri
 import org.knora.webapi.slice.common.api.HandlerMapper
 import org.knora.webapi.slice.common.api.SecuredEndpointAndZioHandler
 
 case class UsersEndpointsHandler(
   usersEndpoints: UsersEndpoints,
-  restService: UsersADMRestService,
+  restService: UsersRestService,
   mapper: HandlerMapper
 ) {
 
-  val getUsersHandler =
+  private val getUsersHandler =
     SecuredEndpointAndZioHandler[
       Unit,
       UsersGetResponseADM
-    ](usersEndpoints.getUsers, user => { case (_: Unit) => restService.listAllUsers(user) })
-
-//  private val handlers        = List().map(mapper.mapEndpointAndHandler)
-  private val securedHandlers = List(getUsersHandler).map(mapper.mapEndpointAndHandler(_))
-
-  val allHanders = /* handlers ++ */ securedHandlers
+    ](
+      usersEndpoints.getUsers,
+      requestingUser => _ => restService.listAllUsers(requestingUser)
+    )
+
+  private val deleteUserByIriHandler =
+    SecuredEndpointAndZioHandler[UserIri, UserOperationResponseADM](
+      usersEndpoints.deleteUser,
+      requestingUser => { case (userIri: UserIri) => restService.deleteUser(requestingUser, userIri) }
+    )
+
+  val allHanders = List(getUsersHandler, deleteUserByIriHandler).map(mapper.mapEndpointAndHandler(_))
 }
 
 object UsersEndpointsHandler {