ID-787 Get user/self terms of service history. (#1254)

* ID-787 Get user/self terms of service history.

* Add to api-docs.yaml

* Refactor Tos -> TermsOfService

* format

* fix test

* address kai comment

* last touches

* Only return acceptances from `getTermsOfServiceDetailsForUser`

* Address tlangs comments

* post merge last touches

src/main/resources/swagger/api-docs.yaml

@@ -3370,6 +3370,58 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/ErrorReport'
+  /termsOfService/v1/user/self/history:
+    get:
+      tags:
+        - TermsOfService
+      summary: gets a user's own terms of service acceptance/rejection action history
+      operationId: userTermsOfServiceSelfGetHistory
+      parameters:
+        - name: limit
+          in: query
+          description: limit on how many records to return, defaults to 100
+          required: false
+          schema:
+            type: string
+      responses:
+        200:
+          description: user has a history of terms of service actions
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/UserTermsOfServiceHistory'
+        404:
+          description: user has no history of terms of service actions
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorReport'
+  /termsOfService/v1/user/{sam_user_id}/history:
+    get:
+      tags:
+        - TermsOfService
+      summary: gets a user's terms of service acceptance/rejection action history, can only get another user's history if the requester is an admin.
+      operationId: userTermsOfServiceGetUserHistory
+      parameters:
+        - name: sam_user_id
+          in: path
+          description: the id of the sam user to get terms of service status for
+          required: true
+          schema:
+            type: string
+        - name: limit
+          in: query
+          description: limit on how many records to return, defaults to 100
+          required: false
+          schema:
+            type: string
+      responses:
+        200:
+          description: user has a history of terms of service actions
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/UserTermsOfServiceHistory'
   /version:
     get:
       tags:
@@ -4091,6 +4143,33 @@ components:
           type: boolean
           description: based on the user's currently accepted terms of service version and when the terms of service were last changed,
             should the user still be permitted to use the system?
+    UserTermsOfServiceHistory:
+      required:
+        - history
+      type: object
+      description: a user's terms of service action history
+      properties:
+        history:
+          type: array
+          items:
+            $ref: '#/components/schemas/UserTermsOfServiceHistoryRecord'
+    UserTermsOfServiceHistoryRecord:
+      required:
+        - action
+        - version
+        - timestamp
+      type: object
+      properties:
+        action:
+          type: string
+          description: the terms of service action the user took
+        version:
+          type: string
+          description: the version of the terms of service the user accepted/rejected
+        timestamp:
+          type: string
+          format: date-time
+          description: the timestamp of the action
     ManagedResourceGroupCoordinates:
       required:
         - tenantId

src/main/scala/org/broadinstitute/dsde/workbench/sam/api/OldUserRoutes.scala

@@ -66,7 +66,7 @@ trait OldUserRoutes extends SamUserDirectives with SamRequestContextDirectives {
               get {
                 withUserAllowInactive(samRequestContext) { samUser =>
                   complete {
-                    tosService.getTosComplianceStatus(samUser, samRequestContext).map { tosAcceptanceStatus =>
+                    tosService.getTermsOfServiceComplianceStatus(samUser, samRequestContext).map { tosAcceptanceStatus =>
                       StatusCodes.OK -> Option(JsBoolean(tosAcceptanceStatus.permitsSystemUsage))
                     }
                   }
@@ -144,7 +144,7 @@ trait OldUserRoutes extends SamUserDirectives with SamRequestContextDirectives {
             } ~
             path("termsOfServiceComplianceStatus") {
               get {
-                complete(tosService.getTosComplianceStatus(user, samRequestContext))
+                complete(tosService.getTermsOfServiceComplianceStatus(user, samRequestContext))
               }
             }
           }

src/main/scala/org/broadinstitute/dsde/workbench/sam/api/TermsOfServiceRoutes.scala

@@ -97,6 +97,17 @@ trait TermsOfServiceRoutes extends SamUserDirectives with SamRequestContextDirec
                     complete(tosService.rejectCurrentTermsOfService(samUser.id, samRequestContext).map(_ => StatusCodes.NoContent))
                   }
                 }
+              } ~
+              pathPrefix("history") { // api/termsOfService/v1/user/{userId}/history
+                pathEndOrSingleSlash {
+                  get {
+                    parameters("limit".as[Integer].withDefault(100)) { (limit: Int) =>
+                      complete {
+                        tosService.getTermsOfServiceHistoryForUser(samUser.id, samRequestContext, limit)
+                      }
+                    }
+                  }
+                }
               }
             } ~
             // The {user_id} route must be last otherwise it will try to parse the other routes incorrectly as user id's
@@ -113,7 +124,9 @@ trait TermsOfServiceRoutes extends SamUserDirectives with SamRequestContextDirec
                 pathPrefix("history") { // api/termsOfService/v1/user/{userId}/history
                   pathEndOrSingleSlash {
                     getWithTelemetry(samRequestContext, userIdParam(requestUserId)) {
-                      complete(StatusCodes.NotImplemented)
+                      parameters("limit".as[Integer].withDefault(100)) { (limit: Int) =>
+                        complete(tosService.getTermsOfServiceHistoryForUser(requestUserId, samRequestContext, limit))
+                      }
                     }
                   }
                 }

src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/DirectoryDAO.scala

@@ -76,8 +76,14 @@ trait DirectoryDAO {
 
   def acceptTermsOfService(userId: WorkbenchUserId, tosVersion: String, samRequestContext: SamRequestContext): IO[Boolean]
   def rejectTermsOfService(userId: WorkbenchUserId, tosVersion: String, samRequestContext: SamRequestContext): IO[Boolean]
-  def getUserTos(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[SamUserTos]]
-  def getUserTosVersion(userId: WorkbenchUserId, tosVersion: Option[String], samRequestContext: SamRequestContext): IO[Option[SamUserTos]]
+  def getUserTermsOfService(userId: WorkbenchUserId, samRequestContext: SamRequestContext, action: Option[String] = None): IO[Option[SamUserTos]]
+  def getUserTermsOfServiceVersion(
+      userId: WorkbenchUserId,
+      tosVersion: Option[String],
+      samRequestContext: SamRequestContext,
+      action: Option[String] = None
+  ): IO[Option[SamUserTos]]
+  def getUserTermsOfServiceHistory(userId: WorkbenchUserId, samRequestContext: SamRequestContext, limit: Integer): IO[List[SamUserTos]]
   def createPetManagedIdentity(petManagedIdentity: PetManagedIdentity, samRequestContext: SamRequestContext): IO[PetManagedIdentity]
   def loadPetManagedIdentity(petManagedIdentityId: PetManagedIdentityId, samRequestContext: SamRequestContext): IO[Option[PetManagedIdentity]]
   def getUserFromPetManagedIdentity(petManagedIdentityObjectId: ManagedIdentityObjectId, samRequestContext: SamRequestContext): IO[Option[SamUser]]

src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAO.scala

@@ -649,27 +649,51 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val
   }
 
   // When no tosVersion is specified, return the latest TosRecord for the user
-  override def getUserTos(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[SamUserTos]] =
-    getUserTosVersion(userId, None, samRequestContext)
-
-  override def getUserTosVersion(userId: WorkbenchUserId, tosVersion: Option[String], samRequestContext: SamRequestContext): IO[Option[SamUserTos]] =
-    readOnlyTransaction("getUserTos", samRequestContext) { implicit session =>
+  override def getUserTermsOfService(userId: WorkbenchUserId, samRequestContext: SamRequestContext, action: Option[String] = None): IO[Option[SamUserTos]] =
+    getUserTermsOfServiceVersion(userId, None, samRequestContext, action)
+
+  override def getUserTermsOfServiceVersion(
+      userId: WorkbenchUserId,
+      tosVersion: Option[String],
+      samRequestContext: SamRequestContext,
+      action: Option[String] = None
+  ): IO[Option[SamUserTos]] =
+    readOnlyTransaction("getUserTermsOfService", samRequestContext) { implicit session =>
       val tosTable = TosTable.syntax
       val column = TosTable.column
 
-      val versionConstraint = if (tosVersion.isDefined) samsqls"and ${column.version} = ${tosVersion.get}" else samsqls""
+      val versionConstraint = tosVersion.map(v => samsqls"and ${column.version} = $v").getOrElse(samsqls"")
+      val actionConstraint = action.map(a => samsqls"and ${column.action} = $a").getOrElse(samsqls"")
 
       val loadUserTosQuery =
         samsql"""select ${tosTable.resultAll}
               from ${TosTable as tosTable}
-              where ${column.samUserId} = $userId $versionConstraint
+              where ${column.samUserId} = $userId
+                $versionConstraint
+                $actionConstraint
               order by ${column.createdAt} desc
               limit 1"""
 
       val userTosRecordOpt: Option[TosRecord] = loadUserTosQuery.map(TosTable(tosTable)).first().apply()
       userTosRecordOpt.map(TosTable.unmarshalUserRecord)
     }
 
+  override def getUserTermsOfServiceHistory(userId: WorkbenchUserId, samRequestContext: SamRequestContext, limit: Integer): IO[List[SamUserTos]] =
+    readOnlyTransaction("getUserTermsOfServiceHistory", samRequestContext) { implicit session =>
+      val tosTable = TosTable.syntax
+      val column = TosTable.column
+
+      val loadUserTosQuery =
+        samsql"""select ${tosTable.resultAll}
+              from ${TosTable as tosTable}
+              where ${column.samUserId} = ${userId}
+              order by ${column.createdAt} desc
+              limit ${limit}"""
+
+      val userTosRecordOpt: List[TosRecord] = loadUserTosQuery.map(TosTable(tosTable)).list().apply()
+      userTosRecordOpt.map(TosTable.unmarshalUserRecord)
+    }
+
   override def isEnabled(subject: WorkbenchSubject, samRequestContext: SamRequestContext): IO[Boolean] =
     readOnlyTransaction("isEnabled", samRequestContext) { implicit session =>
       val userIdOpt = subject match {

src/main/scala/org/broadinstitute/dsde/workbench/sam/model/SamModel.scala

@@ -85,6 +85,8 @@ object UserStatusDetails {
 )
 
 @Lenses final case class TermsOfServiceDetails(latestAcceptedVersion: String, acceptedOn: Instant, permitsSystemUsage: Boolean, isCurrentVersion: Boolean)
+@Lenses final case class TermsOfServiceHistory(history: List[TermsOfServiceHistoryRecord])
+@Lenses final case class TermsOfServiceHistoryRecord(action: String, version: String, timestamp: Instant)
 
 @Lenses final case class ResourceActionPattern(value: String, description: String, authDomainConstrainable: Boolean) {
   def matches(other: ResourceAction) = value.r.pattern.matcher(other.value).matches()
@@ -254,6 +256,7 @@ final case class SamUserTos(id: WorkbenchUserId, version: String, action: String
       this.action == userTos.action
     case _ => false
   }
+  def toHistoryRecord: TermsOfServiceHistoryRecord = TermsOfServiceHistoryRecord(action, version, createdAt)
 }
 object SamLenses {
   val resourceIdentityAccessPolicy = AccessPolicy.id composeLens FullyQualifiedPolicyId.resource

src/main/scala/org/broadinstitute/dsde/workbench/sam/model/api/SamJsonSupport.scala

@@ -29,6 +29,8 @@ import org.broadinstitute.dsde.workbench.sam.model.{
   TermsOfServiceAcceptance,
   TermsOfServiceComplianceStatus,
   TermsOfServiceDetails,
+  TermsOfServiceHistory,
+  TermsOfServiceHistoryRecord,
   UserIdInfo,
   UserPolicyResponse,
   UserResourcesResponse,
@@ -73,6 +75,10 @@ object SamJsonSupport {
 
   implicit val TermsOfServiceDetailsFormat = jsonFormat4(TermsOfServiceDetails.apply)
 
+  implicit val termsOfServiceHistoryRecordFormat = jsonFormat3(TermsOfServiceHistoryRecord.apply)
+
+  implicit val termsOfServiceHistory = jsonFormat1(TermsOfServiceHistory.apply)
+
   implicit val SamUserTosFormat = jsonFormat4(SamUserTos.apply)
 
   implicit val termsOfAcceptanceStatusFormat = jsonFormat3(TermsOfServiceComplianceStatus.apply)

src/main/scala/org/broadinstitute/dsde/workbench/sam/service/TosService.scala

@@ -13,7 +13,14 @@ import org.broadinstitute.dsde.workbench.sam.dataAccess.DirectoryDAO
 import org.broadinstitute.dsde.workbench.sam.db.tables.TosTable
 import org.broadinstitute.dsde.workbench.sam.errorReportSource
 import org.broadinstitute.dsde.workbench.sam.model.api.{SamUser, TermsOfServiceConfigResponse}
-import org.broadinstitute.dsde.workbench.sam.model.{OldTermsOfServiceDetails, SamUserTos, TermsOfServiceComplianceStatus, TermsOfServiceDetails}
+import org.broadinstitute.dsde.workbench.sam.model.{
+  OldTermsOfServiceDetails,
+  SamUserTos,
+  TermsOfServiceComplianceStatus,
+  TermsOfServiceDetails,
+  TermsOfServiceHistory,
+  TermsOfServiceHistoryRecord
+}
 import org.broadinstitute.dsde.workbench.sam.util.AsyncLogging.{FutureWithLogging, IOWithLogging}
 import org.broadinstitute.dsde.workbench.sam.util.{SamRequestContext, SupportsAdmin}
 
@@ -79,7 +86,7 @@ class TosService(
 
   @Deprecated
   def getTosDetails(samUser: SamUser, samRequestContext: SamRequestContext): IO[OldTermsOfServiceDetails] =
-    directoryDao.getUserTos(samUser.id, samRequestContext).map { tos =>
+    directoryDao.getUserTermsOfService(samUser.id, samRequestContext).map { tos =>
       OldTermsOfServiceDetails(isEnabled = true, tosConfig.isGracePeriodEnabled, tosConfig.version, tos.map(_.version))
     }
 
@@ -100,7 +107,7 @@ class TosService(
     }
 
   private def ensureLatestTermsOfService(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[SamUserTos] = for {
-    maybeTermsOfServiceRecord <- directoryDao.getUserTos(userId, samRequestContext)
+    maybeTermsOfServiceRecord <- directoryDao.getUserTermsOfService(userId, samRequestContext, Option(TosTable.ACCEPT))
     latestUserTermsOfService <- maybeTermsOfServiceRecord
       .map(IO.pure)
       .getOrElse(
@@ -115,8 +122,20 @@ class TosService(
       case Some(samUser) => samUser
       case None => throw new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, s"Could not find user:${userId}"))
     }
-  def getTosComplianceStatus(samUser: SamUser, samRequestContext: SamRequestContext): IO[TermsOfServiceComplianceStatus] = for {
-    latestUserTos <- directoryDao.getUserTos(samUser.id, samRequestContext)
+
+  def getTermsOfServiceHistoryForUser(userId: WorkbenchUserId, samRequestContext: SamRequestContext, limit: Integer): IO[TermsOfServiceHistory] =
+    ensureAdminIfNeeded[TermsOfServiceHistory](userId, samRequestContext) {
+      directoryDao.getUserTermsOfServiceHistory(userId, samRequestContext, limit).map {
+        case samUserTosHistory if samUserTosHistory.isEmpty => TermsOfServiceHistory(List.empty)
+        case samUserTosHistory =>
+          TermsOfServiceHistory(
+            samUserTosHistory.map(historyRecord => TermsOfServiceHistoryRecord(historyRecord.action, historyRecord.version, historyRecord.createdAt))
+          )
+      }
+    }
+
+  def getTermsOfServiceComplianceStatus(samUser: SamUser, samRequestContext: SamRequestContext): IO[TermsOfServiceComplianceStatus] = for {
+    latestUserTos <- directoryDao.getUserTermsOfService(samUser.id, samRequestContext)
     userHasAcceptedLatestVersion = userHasAcceptedCurrentTermsOfService(latestUserTos)
     permitsSystemUsage = tosAcceptancePermitsSystemUsage(samUser, latestUserTos)
   } yield TermsOfServiceComplianceStatus(samUser.id, userHasAcceptedLatestVersion, permitsSystemUsage)

src/main/scala/org/broadinstitute/dsde/workbench/sam/service/UserService.scala

@@ -318,7 +318,7 @@ class UserService(
             allUsersStatus <- directoryDAO.isGroupMember(allUsersGroup.id, user.id, samRequestContext) recover { case _: NameNotFoundException =>
               false
             }
-            tosComplianceStatus <- tosService.getTosComplianceStatus(user, samRequestContext)
+            tosComplianceStatus <- tosService.getTermsOfServiceComplianceStatus(user, samRequestContext)
             adminEnabled <- directoryDAO.isEnabled(user.id, samRequestContext)
           } yield {
             // We are removing references to LDAP but this will require an API version change here, so we are leaving
@@ -360,14 +360,14 @@ class UserService(
   // Mixing up the endpoint to return user info AND status information is only causing problems and confusion
   def getUserStatusInfo(user: SamUser, samRequestContext: SamRequestContext): IO[UserStatusInfo] =
     for {
-      tosAcceptanceDetails <- tosService.getTosComplianceStatus(user, samRequestContext)
+      tosAcceptanceDetails <- tosService.getTermsOfServiceComplianceStatus(user, samRequestContext)
     } yield UserStatusInfo(user.id.value, user.email.value, tosAcceptanceDetails.permitsSystemUsage && user.enabled, user.enabled)
 
   def getUserStatusDiagnostics(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[UserStatusDiagnostics]] =
     directoryDAO.loadUser(userId, samRequestContext).flatMap {
       case Some(user) =>
         // pulled out of for comprehension to allow concurrent execution
-        val tosAcceptanceStatus = tosService.getTosComplianceStatus(user, samRequestContext)
+        val tosAcceptanceStatus = tosService.getTermsOfServiceComplianceStatus(user, samRequestContext)
         val adminEnabledStatus = directoryDAO.isEnabled(user.id, samRequestContext)
         val allUsersStatus = cloudExtensions.getOrCreateAllUsersGroup(directoryDAO, samRequestContext).flatMap { allUsersGroup =>
           directoryDAO.isGroupMember(allUsersGroup.id, user.id, samRequestContext) recover { case e: NameNotFoundException => false }
@@ -457,7 +457,7 @@ class UserService(
 
   def getUserAllowances(samUser: SamUser, samRequestContext: SamRequestContext): IO[SamUserAllowances] =
     for {
-      tosStatus <- tosService.getTosComplianceStatus(samUser, samRequestContext)
+      tosStatus <- tosService.getTermsOfServiceComplianceStatus(samUser, samRequestContext)
     } yield SamUserAllowances(
       allowed = samUser.enabled && tosStatus.permitsSystemUsage,
       enabled = samUser.enabled,

src/test/scala/org/broadinstitute/dsde/workbench/sam/api/MockSamRoutesBuilder.scala

@@ -5,6 +5,7 @@ import akka.http.scaladsl.server.Directives.{onSuccess, reject}
 import akka.http.scaladsl.server._
 import akka.stream.Materializer
 import org.broadinstitute.dsde.workbench.model.{ErrorReportSource, WorkbenchGroup}
+import org.broadinstitute.dsde.workbench.sam.model.TermsOfServiceHistory
 import org.broadinstitute.dsde.workbench.sam.model.api.{SamUser, SamUserAttributes}
 import org.broadinstitute.dsde.workbench.sam.service._
 import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext
@@ -74,6 +75,11 @@ class MockSamRoutesBuilder(allUsersGroup: WorkbenchGroup)(implicit system: Actor
     this
   }
 
+  def withTermsOfServiceHistoryForUser(samUser: SamUser, tosHistory: TermsOfServiceHistory): MockSamRoutesBuilder = {
+    mockTosServiceBuilder.withTermsOfServiceHistoryForUser(samUser, tosHistory)
+    this
+  }
+
   def withUserAttributes(samUser: SamUser, userAttributes: SamUserAttributes): MockSamRoutesBuilder = {
     userServiceBuilder.withUserAttributes(samUser, userAttributes)
     this