refactor: Migrate GET /admin/users/iri/<userIri> to tapir (#3010)

dasch-swiss · Jan 29, 2024 · 34d2d7a · 34d2d7a
1 parent a931357
commit 34d2d7a
Show file tree

Hide file tree

Showing 12 changed files with 99 additions and 143 deletions.
diff --git a/integration/src/test/scala/org/knora/webapi/e2e/v2/AuthenticationV2E2ESpec.scala b/integration/src/test/scala/org/knora/webapi/e2e/v2/AuthenticationV2E2ESpec.scala
@@ -373,21 +373,6 @@ class AuthenticationV2E2ESpec extends E2ESpec with AuthenticationV2JsonProtocol
       token.set(lr.token)
     }
 
-    "allow access using URL parameters with token from v2" in {
-      /* Correct token */
-      val request  = Get(baseApiUrl + s"/admin/users/iri/$rootIriEnc?token=${token.get}")
-      val response = singleAwaitingRequest(request)
-      assert(response.status === StatusCodes.OK)
-    }
-
-    "fail with authentication using URL parameters with wrong token" in {
-      /* Wrong token */
-      val request = Get(baseApiUrl + s"/admin/users/iri/$rootIriEnc?token=wrong")
-
-      val response = singleAwaitingRequest(request)
-      assert(response.status === StatusCodes.Unauthorized)
-    }
-
     "allow access using HTTP Bearer Auth header with token from v2" in {
       /* Correct token */
       val request =

diff --git a/integration/src/test/scala/org/knora/webapi/responders/admin/UsersResponderADMSpec.scala b/integration/src/test/scala/org/knora/webapi/responders/admin/UsersResponderADMSpec.scala
@@ -31,6 +31,7 @@ import org.knora.webapi.routing.Authenticator
 import org.knora.webapi.routing.UnsafeZioRun
 import org.knora.webapi.sharedtestdata.SharedTestDataADM
 import org.knora.webapi.slice.admin.domain.model.*
+import org.knora.webapi.util.ZioScalaTestUtil.*
 
 /**
  * This spec is used to test the messages received by the [[UsersResponderADM]] actor.
@@ -98,15 +99,6 @@ class UsersResponderADMSpec extends CoreSpec with ImplicitSender {
         expectMsg(Some(rootUser.ofType(UserInformationTypeADM.Full)))
       }
 
-      "return 'NotFoundException' when the user is unknown" in {
-        appActor ! UserGetByIriRequestADM(
-          identifier = UserIri.unsafeFrom("http://rdfh.ch/users/notexisting"),
-          userInformationTypeADM = UserInformationTypeADM.Full,
-          requestingUser = KnoraSystemInstances.Users.SystemUser
-        )
-        expectMsg(Failure(NotFoundException(s"User 'http://rdfh.ch/users/notexisting' not found")))
-      }
-
       "return 'None' when the user is unknown" in {
         appActor ! UserGetByIriADM(
           identifier = UserIri.unsafeFrom("http://rdfh.ch/users/notexisting"),

diff --git a/...main/scala/org/knora/webapi/messages/admin/responder/usersmessages/UsersMessagesADM.scala b/...main/scala/org/knora/webapi/messages/admin/responder/usersmessages/UsersMessagesADM.scala
@@ -185,19 +185,6 @@ case class UserGetByEmailADM(
   requestingUser: User
 ) extends UsersResponderRequestADM
 
-/**
- * A message that requests a user's profile by IRI. A successful response will be a [[UserResponseADM]].
- *
- * @param identifier             the IRI of the user to be queried.
- * @param userInformationTypeADM the extent of the information returned.
- * @param requestingUser         the user initiating the request.
- */
-case class UserGetByIriRequestADM(
-  identifier: UserIri,
-  userInformationTypeADM: UserInformationTypeADM = UserInformationTypeADM.Short,
-  requestingUser: User
-) extends UsersResponderRequestADM
-
 /**
  * A message that requests a user's profile by email. A successful response will be a [[UserResponseADM]].
  *
@@ -438,7 +425,7 @@ case class UsersGetResponseADM(users: Seq[User]) extends AdminKnoraResponseADM {
  *
  * @param user the user's information of the requested type.
  */
-case class UserResponseADM(user: User) extends KnoraResponseADM {
+case class UserResponseADM(user: User) extends AdminKnoraResponseADM {
   def toJsValue: JsValue = UsersADMJsonProtocol.userProfileResponseADMFormat.write(this)
 }
 

diff --git a/webapi/src/main/scala/org/knora/webapi/messages/util/UserUtilADM.scala b/webapi/src/main/scala/org/knora/webapi/messages/util/UserUtilADM.scala
@@ -8,11 +8,11 @@ package org.knora.webapi.messages.util
 import zio.*
 
 import dsp.errors.ForbiddenException
+import dsp.errors.NotFoundException
 import org.knora.webapi.IRI
-import org.knora.webapi.core.MessageRelay
 import org.knora.webapi.messages.admin.responder.usersmessages.UserInformationTypeADM.Full
-import org.knora.webapi.messages.admin.responder.usersmessages.*
 import org.knora.webapi.messages.util.KnoraSystemInstances.Users.SystemUser
+import org.knora.webapi.responders.admin.UsersResponderADM
 import org.knora.webapi.slice.admin.domain.model.User
 import org.knora.webapi.slice.admin.domain.model.UserIri
 
@@ -36,19 +36,18 @@ object UserUtilADM {
     requestingUser: User,
     requestedUserIri: IRI,
     projectIri: IRI
-  ): ZIO[MessageRelay, Throwable, User] =
-    if (requestingUser.id == requestedUserIri) {
-      ZIO.succeed(requestingUser)
-    } else if (!(requestingUser.permissions.isSystemAdmin || requestingUser.permissions.isProjectAdmin(projectIri))) {
-      val forbiddenMsg =
-        s"You are logged in as ${requestingUser.username}, but only a system administrator or project administrator can perform an operation as another user"
-      ZIO.fail(ForbiddenException(forbiddenMsg))
-    } else {
-      for {
-        userResponse <-
-          MessageRelay.ask[UserResponseADM](
-            UserGetByIriRequestADM(UserIri.unsafeFrom(requestedUserIri), Full, SystemUser)
-          )
-      } yield userResponse.user
+  ): ZIO[UsersResponderADM, Throwable, User] = {
+    val userIri = UserIri.unsafeFrom(requestedUserIri)
+    requestingUser match {
+      case _ if requestingUser.id == userIri.value => ZIO.succeed(requestingUser)
+      case _ if !(requestingUser.permissions.isSystemAdmin || requestingUser.permissions.isProjectAdmin(projectIri)) =>
+        val msg =
+          s"You are logged in as ${requestingUser.username}, but only a system administrator or project administrator can perform an operation as another user"
+        ZIO.fail(ForbiddenException(msg))
+      case _ =>
+        UsersResponderADM
+          .findUserByIri(userIri, Full, SystemUser)
+          .someOrFail(NotFoundException(s"User '${userIri.value}' not found"))
     }
+  }
 }
diff --git a/...in/scala/org/knora/webapi/messages/v2/responder/resourcemessages/ResourceMessagesV2.scala b/...in/scala/org/knora/webapi/messages/v2/responder/resourcemessages/ResourceMessagesV2.scala
@@ -37,6 +37,7 @@ import org.knora.webapi.messages.v2.responder.resourcemessages.CreateResourceReq
 import org.knora.webapi.messages.v2.responder.resourcemessages.CreateResourceRequestV2.AssetIngestState.AssetInTemp
 import org.knora.webapi.messages.v2.responder.standoffmessages.MappingXMLtoStandoff
 import org.knora.webapi.messages.v2.responder.valuemessages.*
+import org.knora.webapi.responders.admin.UsersResponderADM
 import org.knora.webapi.slice.admin.domain.model.User
 import org.knora.webapi.slice.resourceinfo.domain.IriConverter
 import org.knora.webapi.store.iiif.api.SipiService
@@ -683,7 +684,11 @@ object CreateResourceRequestV2 {
     apiRequestID: UUID,
     requestingUser: User,
     ingestState: AssetIngestState = AssetInTemp
-  ): ZIO[IriConverter & SipiService & StringFormatter & MessageRelay, Throwable, CreateResourceRequestV2] =
+  ): ZIO[
+    IriConverter & MessageRelay & SipiService & StringFormatter & UsersResponderADM,
+    Throwable,
+    CreateResourceRequestV2
+  ] =
     ZIO.serviceWithZIO[StringFormatter] { implicit stringFormatter =>
       val validationFun: (String, => Nothing) => String =
         (s, errorFun) => Iri.toSparqlEncodedString(s).getOrElse(errorFun)

diff --git a/webapi/src/main/scala/org/knora/webapi/responders/admin/UsersResponderADM.scala b/webapi/src/main/scala/org/knora/webapi/responders/admin/UsersResponderADM.scala
@@ -75,6 +75,28 @@ trait UsersResponderADM {
     requestingUser: User,
     apiRequestID: UUID
   ): Task[UserOperationResponseADM]
+
+  /**
+   * ~ CACHED ~
+   * Gets information about a Knora user, and returns it as a [[User]].
+   * If possible, tries to retrieve it from the cache. If not, it retrieves
+   * it from the triplestore, and then writes it to the cache. Writes to the
+   * cache are always `UserInformationTypeADM.FULL`.
+   *
+   * @param identifier          the IRI of the user.
+   * @param userInformationType the type of the requested profile (restricted
+   *                            of full).
+   * @param requestingUser      the user initiating the request.
+   * @param skipCache           the flag denotes to skip the cache and instead
+   *                            get data from the triplestore
+   * @return a [[User]] describing the user.
+   */
+  def findUserByIri(
+    identifier: UserIri,
+    userInformationType: UserInformationTypeADM,
+    requestingUser: User,
+    skipCache: Boolean = false
+  ): Task[Option[User]]
 }
 
 final case class UsersResponderADMLive(
@@ -101,13 +123,11 @@ final case class UsersResponderADMLive(
     case UsersGetRequestADM(_, requestingUser) =>
       getAllUserADMRequest(requestingUser)
     case UserGetByIriADM(identifier, userInformationTypeADM, requestingUser) =>
-      getSingleUserByIriADM(identifier, userInformationTypeADM, requestingUser)
+      findUserByIri(identifier, userInformationTypeADM, requestingUser)
     case UserGetByEmailADM(email, userInformationTypeADM, requestingUser) =>
       getSingleUserByEmailADM(email, userInformationTypeADM, requestingUser)
     case UserGetByUsernameADM(username, userInformationTypeADM, requestingUser) =>
       getSingleUserByUsernameADM(username, userInformationTypeADM, requestingUser)
-    case UserGetByIriRequestADM(identifier, userInformationTypeADM, requestingUser) =>
-      getSingleUserByIriADMRequest(identifier, userInformationTypeADM, requestingUser)
     case UserGetByEmailRequestADM(email, userInformationTypeADM, requestingUser) =>
       getSingleUserByEmailADMRequest(email, userInformationTypeADM, requestingUser)
     case UserGetByUsernameRequestADM(username, userInformationTypeADM, requestingUser) =>
@@ -290,22 +310,7 @@ final case class UsersResponderADMLive(
                 else ZIO.fail(NotFoundException(s"No users found"))
     } yield result
 
-  /**
-   * ~ CACHED ~
-   * Gets information about a Knora user, and returns it as a [[User]].
-   * If possible, tries to retrieve it from the cache. If not, it retrieves
-   * it from the triplestore, and then writes it to the cache. Writes to the
-   * cache are always `UserInformationTypeADM.FULL`.
-   *
-   * @param identifier           the IRI of the user.
-   * @param userInformationType  the type of the requested profile (restricted
-   *                             of full).
-   * @param requestingUser       the user initiating the request.
-   * @param skipCache            the flag denotes to skip the cache and instead
-   *                             get data from the triplestore
-   * @return a [[User]] describing the user.
-   */
-  private def getSingleUserByIriADM(
+  override def findUserByIri(
     identifier: UserIri,
     userInformationType: UserInformationTypeADM,
     requestingUser: User,
@@ -400,29 +405,6 @@ final case class UsersResponderADMLive(
       _            <- ZIO.logDebug(s"getSingleUserByIriADM - retrieved user '${username.value}': ${finalResponse.nonEmpty}")
     } yield finalResponse
 
-  /**
-   * Gets information about a Knora user, and returns it as a [[UserResponseADM]].
-   *
-   * @param identifier          the IRI of the user.
-   * @param userInformationType the type of the requested profile (restricted of full).
-   * @param requestingUser      the user initiating the request.
-   * @return a [[UserResponseADM]]
-   */
-  private def getSingleUserByIriADMRequest(
-    identifier: UserIri,
-    userInformationType: UserInformationTypeADM,
-    requestingUser: User
-  ): Task[UserResponseADM] =
-    for {
-      maybeUserADM <- getSingleUserByIriADM(identifier, userInformationType, requestingUser)
-      result <- ZIO
-                  .fromOption(maybeUserADM)
-                  .mapBoth(
-                    _ => NotFoundException(s"User '${identifier.value}' not found"),
-                    user => UserResponseADM(user = user)
-                  )
-    } yield result
-
   /**
    * Gets information about a Knora user, and returns it as a [[UserResponseADM]].
    *
@@ -510,7 +492,7 @@ final case class UsersResponderADMLive(
              )
 
         // get current user information
-        currentUserInformation <- getSingleUserByIriADM(
+        currentUserInformation <- findUserByIri(
                                     identifier = UserIri.unsafeFrom(userIri),
                                     userInformationType = UserInformationTypeADM.Full,
                                     requestingUser = KnoraSystemInstances.Users.SystemUser
@@ -720,7 +702,7 @@ final case class UsersResponderADMLive(
    * @return a sequence of [[ProjectADM]]
    */
   private def userProjectMembershipsGetADM(userIri: IRI) =
-    getSingleUserByIriADM(
+    findUserByIri(
       UserIri.unsafeFrom(userIri),
       UserInformationTypeADM.Full,
       KnoraSystemInstances.Users.SystemUser
@@ -1113,7 +1095,7 @@ final case class UsersResponderADMLive(
    * @return a sequence of [[GroupADM]].
    */
   private def userGroupMembershipsGetADM(userIri: IRI) =
-    getSingleUserByIriADM(
+    findUserByIri(
       UserIri.unsafeFrom(userIri),
       UserInformationTypeADM.Full,
       KnoraSystemInstances.Users.SystemUser
@@ -1145,7 +1127,7 @@ final case class UsersResponderADMLive(
     ): Task[UserOperationResponseADM] =
       for {
         // check if user exists
-        maybeUser <- getSingleUserByIriADM(
+        maybeUser <- findUserByIri(
                        UserIri.unsafeFrom(userIri),
                        UserInformationTypeADM.Full,
                        KnoraSystemInstances.Users.SystemUser,
@@ -1304,7 +1286,7 @@ final case class UsersResponderADMLive(
     for {
 
       // get current user
-      maybeCurrentUser <- getSingleUserByIriADM(
+      maybeCurrentUser <- findUserByIri(
                             identifier = UserIri.unsafeFrom(userIri),
                             requestingUser = requestingUser,
                             userInformationType = UserInformationTypeADM.Full,
@@ -1395,7 +1377,7 @@ final case class UsersResponderADMLive(
       _ <- invalidateCachedUserADM(maybeCurrentUser) *> triplestore.query(Update(updateUserSparql))
 
       /* Verify that the user was updated */
-      maybeUpdatedUserADM <- getSingleUserByIriADM(
+      maybeUpdatedUserADM <- findUserByIri(
                                identifier = UserIri.unsafeFrom(userIri),
                                requestingUser = KnoraSystemInstances.Users.SystemUser,
                                userInformationType = UserInformationTypeADM.Full,
@@ -1502,7 +1484,7 @@ final case class UsersResponderADMLive(
     }
 
     for {
-      maybeCurrentUser <- getSingleUserByIriADM(
+      maybeCurrentUser <- findUserByIri(
                             identifier = UserIri.unsafeFrom(userIri),
                             requestingUser = requestingUser,
                             userInformationType = UserInformationTypeADM.Full,
@@ -1521,7 +1503,7 @@ final case class UsersResponderADMLive(
       _ <- triplestore.query(Update(updateUserSparql))
 
       /* Verify that the password was updated. */
-      maybeUpdatedUserADM <- getSingleUserByIriADM(
+      maybeUpdatedUserADM <- findUserByIri(
                                identifier = UserIri.unsafeFrom(userIri),
                                requestingUser = requestingUser,
                                userInformationType = UserInformationTypeADM.Full,
@@ -1634,7 +1616,7 @@ final case class UsersResponderADMLive(
         _ <- triplestore.query(Update(createNewUserSparql))
 
         // try to retrieve newly created user (will also add to cache)
-        maybeNewUserADM <- getSingleUserByIriADM(
+        maybeNewUserADM <- findUserByIri(
                              identifier = UserIri.unsafeFrom(userIri),
                              requestingUser = KnoraSystemInstances.Users.SystemUser,
                              userInformationType = UserInformationTypeADM.Full,

diff --git a/webapi/src/main/scala/org/knora/webapi/routing/ApiRoutes.scala b/webapi/src/main/scala/org/knora/webapi/routing/ApiRoutes.scala
@@ -21,6 +21,7 @@ import org.knora.webapi.core.MessageRelay
 import org.knora.webapi.http.directives.DSPApiDirectives
 import org.knora.webapi.http.version.ServerVersion
 import org.knora.webapi.messages.StringFormatter
+import org.knora.webapi.responders.admin.UsersResponderADM
 import org.knora.webapi.responders.v2.SearchResponderV2
 import org.knora.webapi.responders.v2.ValuesResponderV2
 import org.knora.webapi.routing
@@ -47,7 +48,7 @@ object ApiRoutes {
    * All routes composed together.
    */
   val layer: URLayer[
-    ActorSystem & AdminApiRoutes & AppConfig & AppRouter & IriConverter & KnoraProjectRepo & MessageRelay & ProjectADMRestService & ProjectsEndpointsHandler & ResourceInfoRoutes & RestCardinalityService & RestResourceInfoService & SearchApiRoutes & SearchResponderV2 & SipiService & StringFormatter & ValuesResponderV2 & core.State & routing.Authenticator,
+    ActorSystem & AdminApiRoutes & AppConfig & AppRouter & core.State & IriConverter & KnoraProjectRepo & MessageRelay & ProjectADMRestService & ProjectsEndpointsHandler & ResourceInfoRoutes & RestCardinalityService & RestResourceInfoService & routing.Authenticator & SearchApiRoutes & SearchResponderV2 & SipiService & StringFormatter & UsersResponderADM & ValuesResponderV2,
     ApiRoutes
   ] =
     ZLayer {
@@ -61,7 +62,7 @@ object ApiRoutes {
         routeData          <- ZIO.succeed(KnoraRouteData(sys.system, router.ref, appConfig))
         runtime <-
           ZIO.runtime[
-            AppConfig & IriConverter & KnoraProjectRepo & MessageRelay & ProjectADMRestService & RestCardinalityService & RestResourceInfoService & SearchResponderV2 & SearchApiRoutes & SipiService & StringFormatter & ValuesResponderV2 & core.State & routing.Authenticator
+            AppConfig & core.State & IriConverter & KnoraProjectRepo & MessageRelay & ProjectADMRestService & RestCardinalityService & RestResourceInfoService & routing.Authenticator & SearchApiRoutes & SearchResponderV2 & SipiService & StringFormatter & UsersResponderADM & ValuesResponderV2
           ]
       } yield ApiRoutesImpl(routeData, adminApiRoutes, resourceInfoRoutes, searchApiRoutes, appConfig, runtime)
     }
@@ -81,8 +82,7 @@ private final case class ApiRoutesImpl(
   searchApiRoutes: SearchApiRoutes,
   appConfig: AppConfig,
   implicit val runtime: Runtime[
-    AppConfig & IriConverter & KnoraProjectRepo & MessageRelay & ProjectADMRestService & RestCardinalityService &
-      RestResourceInfoService & SearchResponderV2 & SipiService & StringFormatter & ValuesResponderV2 & core.State & routing.Authenticator
+    AppConfig & core.State & IriConverter & KnoraProjectRepo & MessageRelay & ProjectADMRestService & RestCardinalityService & RestResourceInfoService & routing.Authenticator & SearchResponderV2 & SipiService & StringFormatter & UsersResponderADM & ValuesResponderV2
   ]
 ) extends ApiRoutes
     with AroundDirectives {

diff --git a/webapi/src/main/scala/org/knora/webapi/routing/admin/UsersRouteADM.scala b/webapi/src/main/scala/org/knora/webapi/routing/admin/UsersRouteADM.scala
@@ -35,7 +35,6 @@ final case class UsersRouteADM()(
 
   def makeRoute: Route =
     addUser() ~
-      getUserByIri ~
       getUserByEmail ~
       getUserByUsername ~
       changeUserBasicInformation() ~
@@ -65,20 +64,6 @@ final case class UsersRouteADM()(
     }
   }
 
-  /**
-   * return a single user identified by iri
-   */
-  private def getUserByIri: Route =
-    path(usersBasePath / "iri" / Segment)(userIri =>
-      ctx => {
-        val task = for {
-          requestingUser <- Authenticator.getUserADM(ctx)
-          iri            <- ZIO.fromEither(UserIri.from(userIri)).mapError(BadRequestException(_))
-        } yield UserGetByIriRequestADM(iri, UserInformationTypeADM.Restricted, requestingUser)
-        runJsonRouteZ(task, ctx)
-      }
-    )
-
   /**
    * return a single user identified by email
    */