ID-792 Load Terms of Service docs from remote sources (#1201)

* ID-792 Load Terms of Service docs from remote sources

env/local.env

@@ -36,7 +36,7 @@ export SUPER_ADMINS_GROUP="sam-super-admins@test.firecloud.org"
 export TERRA_GOOGLE_ORG_NUMBER="400176686919"
 export TOS_ENABLED="true"
 export TOS_GRACE_PERIOD_ENABLED="true"
-export TOS_URL="app.terra.bio/#terms-of-service"
+export TOS_BASE_URL="https://raw.githubusercontent.com/broadinstitute/terra-terms-of-service/main/versions"
 export TOS_VERSION="1"
 
 export SAM_LOG_APPENDER="Console-Standard"

env/test.env

@@ -31,7 +31,7 @@ export SUPER_ADMINS_GROUP="sam-super-admins@test.firecloud.org"
 export TERRA_GOOGLE_ORG_NUMBER="mock-org-number"
 export TOS_ENABLED="true"
 export TOS_GRACE_PERIOD_ENABLED="true"
-export TOS_URL="app.terra.bio/#terms-of-service"
-export TOS_VERSION="0"
+export TOS_BASE_URL="classpath:tos"
+export TOS_VERSION="1"
 
 export SAM_LOG_APPENDER="Console-Standard"

src/main/resources/reference.conf

@@ -1451,6 +1451,7 @@ blockedEmailDomains = ["qwiklabs-gsuite.net", "qwiklabs.net"]
 
 termsOfService {
   isGracePeriodEnabled = false
+  baseUrl: "classpath:tos"
 }
 
 prometheus {

src/main/resources/sam.conf

@@ -40,7 +40,7 @@ termsOfService {
   isTosEnabled = ${?TOS_ENABLED}
   isGracePeriodEnabled = ${?TOS_GRACE_PERIOD_ENABLED}
   version = ${?TOS_VERSION}
-  url = ${?TOS_URL}
+  baseUrl = ${?TOS_BASE_URL}
 }
 
 oidc {

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

@@ -51,15 +51,16 @@ trait SamUserDirectives {
 
   def asAdminServiceUser: Directive0
 
+  val oldTermsOfServiceUrl = "app.terra.bio/#terms-of-service"
   def withTermsOfServiceAcceptance: Directive0 =
     Directives.mapInnerRoute { r =>
-      handleRejections(termsOfServiceRejectionHandler(termsOfServiceConfig.url)) {
+      handleRejections(termsOfServiceRejectionHandler(oldTermsOfServiceUrl)) {
         requestEntityPresent {
           entity(as[TermsOfServiceAcceptance]) { tos =>
-            if (tos.value.equalsIgnoreCase(termsOfServiceConfig.url)) r
+            if (tos.value.equalsIgnoreCase(oldTermsOfServiceUrl)) r
             else
               reject(
-                MalformedRequestContentRejection(s"Invalid ToS acceptance", new WorkbenchException(s"ToS URL did not match ${termsOfServiceConfig.url}"))
+                MalformedRequestContentRejection(s"Invalid ToS acceptance", new WorkbenchException(s"ToS URL did not match ${oldTermsOfServiceUrl}"))
               )
           }
         }

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

@@ -15,7 +15,7 @@ trait TermsOfServiceRoutes {
       path("text") {
         pathEndOrSingleSlash {
           get {
-            complete(tosService.getTosText)
+            complete(tosService.termsOfServiceText)
           }
         }
       }
@@ -24,7 +24,7 @@ trait TermsOfServiceRoutes {
         path("text") {
           pathEndOrSingleSlash {
             get {
-              complete(tosService.getPrivacyText)
+              complete(tosService.privacyPolicyText)
             }
           }
         }

src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AppConfig.scala

@@ -119,7 +119,7 @@ object AppConfig {
       config.getAs[Boolean]("isTosEnabled").getOrElse(true),
       config.getBoolean("isGracePeriodEnabled"),
       config.getString("version"),
-      config.getString("url")
+      config.getString("baseUrl")
     )
   }
 

src/main/scala/org/broadinstitute/dsde/workbench/sam/config/TermsOfServiceConfig.scala

@@ -9,4 +9,4 @@ package org.broadinstitute.dsde.workbench.sam.config
   *   The url to the Terra Terms of Service. Used for validation and will be displayed to user in error messages
   */
 
-case class TermsOfServiceConfig(isTosEnabled: Boolean, isGracePeriodEnabled: Boolean, version: String, url: String)
+case class TermsOfServiceConfig(isTosEnabled: Boolean, isGracePeriodEnabled: Boolean, version: String, baseUrl: String)

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

@@ -1,9 +1,13 @@
 package org.broadinstitute.dsde.workbench.sam.service
-
-import akka.http.scaladsl.model.StatusCodes
+import akka.actor.ActorSystem
+import akka.http.scaladsl.Http
+import akka.http.scaladsl.client.RequestBuilding.Get
+import akka.http.scaladsl.model.{StatusCodes, Uri}
+import akka.http.scaladsl.unmarshalling.Unmarshal
 import cats.effect.IO
 import com.typesafe.scalalogging.LazyLogging
 import org.broadinstitute.dsde.workbench.sam.util.AsyncLogging.IOWithLogging
+import org.broadinstitute.dsde.workbench.sam.util.AsyncLogging.FutureWithLogging
 import org.broadinstitute.dsde.workbench.model.{ErrorReport, WorkbenchExceptionWithErrorReport, WorkbenchUserId}
 import org.broadinstitute.dsde.workbench.sam.api.StandardSamUserDirectives
 import org.broadinstitute.dsde.workbench.sam.dataAccess.DirectoryDAO
@@ -13,13 +17,22 @@ import org.broadinstitute.dsde.workbench.sam.config.TermsOfServiceConfig
 import org.broadinstitute.dsde.workbench.sam.db.tables.TosTable
 import org.broadinstitute.dsde.workbench.sam.model.{SamUser, SamUserTos, TermsOfServiceComplianceStatus, TermsOfServiceDetails}
 
-import scala.concurrent.ExecutionContext
 import java.io.{FileNotFoundException, IOException}
+import scala.concurrent.{Await, ExecutionContext}
+import java.util.concurrent.TimeUnit
+import scala.concurrent.duration.Duration
 import scala.io.Source
 
-class TosService(val directoryDao: DirectoryDAO, val tosConfig: TermsOfServiceConfig)(implicit val executionContext: ExecutionContext) extends LazyLogging {
-  val termsOfServiceFile = s"tos/termsOfService-${tosConfig.version}.md"
-  val privacyPolicyFile = s"tos/privacyPolicy-${tosConfig.version}.md"
+class TosService(val directoryDao: DirectoryDAO, val tosConfig: TermsOfServiceConfig)(
+    implicit val executionContext: ExecutionContext,
+    implicit val actorSystem: ActorSystem
+) extends LazyLogging {
+
+  private val termsOfServiceUri = s"${tosConfig.baseUrl}/${tosConfig.version}/termsOfService.md"
+  private val privacyPolicyUri = s"${tosConfig.baseUrl}/${tosConfig.version}/privacyPolicy.md"
+
+  val termsOfServiceText = TermsOfServiceDocument(termsOfServiceUri)
+  val privacyPolicyText = TermsOfServiceDocument(privacyPolicyUri)
 
   def acceptTosStatus(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Boolean] =
     directoryDao
@@ -63,34 +76,58 @@ class TosService(val directoryDao: DirectoryDAO, val tosConfig: TermsOfServiceCo
     userTos.exists { tos =>
       tos.version.contains(tosConfig.version) && tos.action == TosTable.ACCEPT
     }
+}
+
+trait TermsOfServiceDocument {
+  def apply(uri: Uri)(implicit actorSystem: ActorSystem, executionContext: ExecutionContext): String
+}
+
+object TermsOfServiceDocument extends TermsOfServiceDocument with LazyLogging {
+  override def apply(uri: Uri)(implicit actorSystem: ActorSystem, executionContext: ExecutionContext): String =
+    if (uri.scheme.equalsIgnoreCase("classpath")) {
+      getTextFromResource(uri)
+    } else {
+      getTextFromWeb(uri)
+    }
 
-  /** Get the terms of service text and send it to the caller
+  /** Get the contents of an HTTP resource.
+    * @param uri
+    *   HTTP(s) URI of a resource
     * @return
-    *   terms of service text
+    *   The text of the document at the provided uri.
     */
-  def getText(file: String, prettyTitle: String): String = {
+  private def getTextFromWeb(uri: Uri)(implicit actorSystem: ActorSystem, executionContext: ExecutionContext): String = {
+    val future = for {
+      response <- Http().singleRequest(Get(uri))
+      text <- Unmarshal(response).to[String]
+    } yield text
+
+    Await.result(future.withInfoLogMessage(s"Retrieved Terms of Service doc from $uri"), Duration.apply(10, TimeUnit.SECONDS))
+  }
+
+  /** Get the contents of a resource on the classpath. Used for BEE Environments
+    * @param resourceUri
+    *   classpath resource uri
+    * @return
+    *   The text of a document in the classpath
+    */
+  private def getTextFromResource(resourceUri: Uri): String = {
     val fileStream =
       try {
-        logger.debug(s"Reading $prettyTitle")
-        Source.fromResource(file)
+        logger.debug(s"Reading $resourceUri")
+        Source.fromResource(resourceUri.path.toString())
       } catch {
         case e: FileNotFoundException =>
-          logger.error(s"$prettyTitle file not found", e)
+          logger.error(s"$resourceUri file not found", e)
           throw new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, e))
         case e: IOException =>
-          logger.error(s"Failed to read $prettyTitle file due to IO exception", e)
+          logger.error(s"Failed to read $resourceUri file due to IO exception", e)
           throw new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, e))
       }
-    logger.debug(s"$prettyTitle file found")
+    logger.debug(s"$resourceUri file found")
     try
       fileStream.mkString
     finally
       fileStream.close
   }
-
-  def getPrivacyText: String =
-    getText(privacyPolicyFile, "Privacy Policy")
-
-  def getTosText: String =
-    getText(termsOfServiceFile, "Terms of Service")
 }

src/test/resources/tos/2/privacyPolicy.md

@@ -0,0 +1 @@
+# Test Privacy Policy

src/test/resources/tos/2/termsOfService.md

@@ -0,0 +1 @@
+# Test Terms of Service

src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureServiceSpec.scala

@@ -1,6 +1,8 @@
 package org.broadinstitute.dsde.workbench.sam.azure
 
+import akka.actor.ActorSystem
 import akka.http.scaladsl.model.StatusCodes
+import akka.testkit.TestKit
 import com.azure.resourcemanager.managedapplications.models.Plan
 import org.broadinstitute.dsde.workbench.model.{WorkbenchEmail, WorkbenchExceptionWithErrorReport}
 import org.broadinstitute.dsde.workbench.sam.Generator.genWorkbenchUserAzure
@@ -10,16 +12,27 @@ import org.broadinstitute.dsde.workbench.sam.model.{UserStatus, UserStatusDetail
 import org.broadinstitute.dsde.workbench.sam.service.{NoExtensions, TosService, UserService}
 import org.broadinstitute.dsde.workbench.sam.{ConnectedTest, Generator}
 import org.mockito.Mockito.when
+import org.scalatest.BeforeAndAfterAll
 import org.scalatest.concurrent.ScalaFutures
-import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.flatspec.AnyFlatSpecLike
 import org.scalatest.matchers.should.Matchers
 
 import scala.jdk.CollectionConverters._
 
-class AzureServiceSpec extends AnyFlatSpec with Matchers with ScalaFutures {
+class AzureServiceSpec(_system: ActorSystem) extends TestKit(_system) with AnyFlatSpecLike with Matchers with ScalaFutures with BeforeAndAfterAll {
   implicit val ec = scala.concurrent.ExecutionContext.global
   implicit val ioRuntime = cats.effect.unsafe.IORuntime.global
 
+  def this() = this(ActorSystem("AzureServiceSpec"))
+
+  override def beforeAll(): Unit =
+    super.beforeAll()
+
+  override def afterAll(): Unit = {
+    TestKit.shutdownActorSystem(system)
+    super.afterAll()
+  }
+
   "AzureService" should "create a pet managed identity" taggedAs ConnectedTest in {
     val azureServicesConfig = appConfig.azureServicesConfig
 

src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockAccessPolicyDAOSpec.scala

@@ -1,12 +1,14 @@
 package org.broadinstitute.dsde.workbench.sam.dataAccess
 
+import akka.actor.ActorSystem
+import akka.testkit.TestKit
 import cats.effect.unsafe.implicits.global
 import org.broadinstitute.dsde.workbench.model._
 import org.broadinstitute.dsde.workbench.sam.TestSupport.{databaseEnabled, databaseEnabledClue}
 import org.broadinstitute.dsde.workbench.sam.model._
 import org.broadinstitute.dsde.workbench.sam.service._
 import org.broadinstitute.dsde.workbench.sam.{Generator, TestSupport}
-import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.flatspec.AnyFlatSpecLike
 import org.scalatest.matchers.should.Matchers
 import org.scalatest.{BeforeAndAfter, BeforeAndAfterAll}
 
@@ -15,12 +17,25 @@ import scala.language.reflectiveCalls
 
 /** Created by dvoet on 6/26/17.
   */
-class MockAccessPolicyDAOSpec extends AnyFlatSpec with Matchers with TestSupport with BeforeAndAfter with BeforeAndAfterAll {
+class MockAccessPolicyDAOSpec(_system: ActorSystem)
+    extends TestKit(_system)
+    with AnyFlatSpecLike
+    with Matchers
+    with TestSupport
+    with BeforeAndAfter
+    with BeforeAndAfterAll {
   private val dummyUser = Generator.genWorkbenchUserGoogle.sample.get
 
-  override protected def beforeAll(): Unit =
+  def this() = this(ActorSystem("MockAccessPolicyDAOSpec"))
+
+  override def beforeAll(): Unit =
     super.beforeAll()
 
+  override def afterAll(): Unit = {
+    TestKit.shutdownActorSystem(system)
+    super.afterAll()
+  }
+
   before {
     TestSupport.truncateAll
   }

src/test/scala/org/broadinstitute/dsde/workbench/sam/service/TosServiceSpec.scala

@@ -1,5 +1,7 @@
 package org.broadinstitute.dsde.workbench.sam.service
 
+import akka.actor.ActorSystem
+import akka.testkit.TestKit
 import cats.effect.IO
 import cats.effect.unsafe.implicits.{global => globalEc}
 import org.broadinstitute.dsde.workbench.model.WorkbenchUserId
@@ -11,23 +13,36 @@ import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext
 import org.broadinstitute.dsde.workbench.sam.{Generator, PropertyBasedTesting, TestSupport}
 import org.mockito.Mockito.RETURNS_SMART_NULLS
 import org.mockito.scalatest.MockitoSugar
-import org.scalatest.freespec.AnyFreeSpec
+import org.scalatest.freespec.AnyFreeSpecLike
 import org.scalatest.{BeforeAndAfter, BeforeAndAfterAll}
 
 import java.time.Instant
 import scala.concurrent.ExecutionContext.Implicits.global
 
-class TosServiceSpec extends AnyFreeSpec with TestSupport with BeforeAndAfterAll with BeforeAndAfter with PropertyBasedTesting with MockitoSugar {
+class TosServiceSpec(_system: ActorSystem)
+    extends TestKit(_system)
+    with AnyFreeSpecLike
+    with TestSupport
+    with BeforeAndAfterAll
+    with BeforeAndAfter
+    with PropertyBasedTesting
+    with MockitoSugar {
 
-  lazy val dirDAO = mock[DirectoryDAO](RETURNS_SMART_NULLS)
-
-  val defaultUser = Generator.genWorkbenchUserBoth.sample.get
-  val serviceAccountUser = Generator.genWorkbenchUserServiceAccount.sample.get
+  def this() = this(ActorSystem("TosServiceSpec"))
 
   override protected def beforeAll(): Unit = {
     super.beforeAll()
     TestSupport.truncateAll
   }
+  override def afterAll(): Unit = {
+    TestKit.shutdownActorSystem(system)
+    super.afterAll()
+  }
+
+  lazy val dirDAO = mock[DirectoryDAO](RETURNS_SMART_NULLS)
+
+  val defaultUser = Generator.genWorkbenchUserBoth.sample.get
+  val serviceAccountUser = Generator.genWorkbenchUserServiceAccount.sample.get
 
   before {
     clearDatabase()
@@ -78,6 +93,12 @@ class TosServiceSpec extends AnyFreeSpec with TestSupport with BeforeAndAfterAll
       complianceStatus.permitsSystemUsage shouldBe true
     }
 
+    "loads the Terms of Service text when TosService is instantiated" in {
+      val tosService = new TosService(dirDAO, TestSupport.tosConfig.copy(version = "2"))
+      tosService.termsOfServiceText contains "Test Terms of Service"
+      tosService.privacyPolicyText contains "Test Privacy Policy"
+    }
+
     val tosVersion = "2"
     val withoutGracePeriod = "without the grace period enabled"
     val withGracePeriod = " with the grace period enabled"