diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AppConfig.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AppConfig.scala index 601711423..5f97c0cb3 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AppConfig.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AppConfig.scala @@ -122,7 +122,8 @@ object AppConfig { config.getString("version"), config.getString("baseUrl"), // Must be a valid UTC datetime string in ISO 8601 format ex: 2007-12-03T10:15:30.00Z - Instant.parse(config.getString("rollingAcceptanceWindowExpirationDatetime")) + Instant.parse(config.getString("rollingAcceptanceWindowExpirationDatetime")), + config.getString("rollingAcceptanceWindowPreviousTosVersion") ) } diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/TermsOfServiceConfig.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/TermsOfServiceConfig.scala index c63e98fb4..853bd917d 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/TermsOfServiceConfig.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/TermsOfServiceConfig.scala @@ -13,6 +13,10 @@ import java.time.Instant * The expiration time for the rolling acceptance window. If the user has not accepted the new ToS by this time, * they will be denied access to the system. Must be a valid UTC datetime string in ISO 8601 format * example: 2007-12-03T10:15:30.00Z + * + * @param rollingAcceptanceWindowPreviousTosVersion + * The previous version of the ToS that the user must have accepted in order to be granted access to the system under + * the rolling window. */ -case class TermsOfServiceConfig(isTosEnabled: Boolean, isGracePeriodEnabled: Boolean, version: String, baseUrl: String, rollingAcceptanceWindowExpiration: Instant) +case class TermsOfServiceConfig(isTosEnabled: Boolean, isGracePeriodEnabled: Boolean, version: String, baseUrl: String, rollingAcceptanceWindowExpiration: Instant, rollingAcceptanceWindowPreviousTosVersion: String) diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/DirectoryDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/DirectoryDAO.scala index 35a8f5295..d62cd34ac 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/DirectoryDAO.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/DirectoryDAO.scala @@ -75,7 +75,8 @@ 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 getLatestUserTos(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[SamUserTos]] + def getUserTos(userId: WorkbenchUserId, tosVersion: String, samRequestContext: SamRequestContext): IO[Option[SamUserTos]] def createPetManagedIdentity(petManagedIdentity: PetManagedIdentity, samRequestContext: SamRequestContext): IO[PetManagedIdentity] def loadPetManagedIdentity(petManagedIdentityId: PetManagedIdentityId, samRequestContext: SamRequestContext): IO[Option[PetManagedIdentity]] diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAO.scala index 876b6dd74..5d0e57aa3 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAO.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAO.scala @@ -647,8 +647,8 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val } } - override def getUserTos(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[SamUserTos]] = - readOnlyTransaction("getUserTos", samRequestContext) { implicit session => + override def getLatestUserTos(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[SamUserTos]] = + readOnlyTransaction("getLatestUserTos", samRequestContext) { implicit session => val tosTable = TosTable.syntax val column = TosTable.column @@ -663,6 +663,22 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val userTosRecordOpt.map(TosTable.unmarshalUserRecord) } + override def getUserTos(userId: WorkbenchUserId, tosVersion: String, samRequestContext: SamRequestContext): IO[Option[SamUserTos]] = + readOnlyTransaction("getUserTos", 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} and ${column.version} = ${tosVersion} + order by ${column.createdAt} desc + limit 1""" + + val userTosRecordOpt: Option[TosRecord] = loadUserTosQuery.map(TosTable(tosTable)).first().apply() + userTosRecordOpt.map(TosTable.unmarshalUserRecord) + } + override def isEnabled(subject: WorkbenchSubject, samRequestContext: SamRequestContext): IO[Boolean] = readOnlyTransaction("isEnabled", samRequestContext) { implicit session => val userIdOpt = subject match { diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/TosService.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/TosService.scala index 247aaaf92..5dc199843 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/TosService.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/TosService.scala @@ -47,27 +47,29 @@ class TosService(val directoryDao: DirectoryDAO, val tosConfig: TermsOfServiceCo @Deprecated def getTosDetails(samUser: SamUser, samRequestContext: SamRequestContext): IO[TermsOfServiceDetails] = - directoryDao.getUserTos(samUser.id, samRequestContext).map { tos => + directoryDao.getLatestUserTos(samUser.id, samRequestContext).map { tos => TermsOfServiceDetails(isEnabled = true, tosConfig.isGracePeriodEnabled, tosConfig.version, tos.map(_.version)) } - def getTosComplianceStatus(samUser: SamUser, samRequestContext: SamRequestContext): IO[TermsOfServiceComplianceStatus] = - directoryDao.getUserTos(samUser.id, samRequestContext).map { tos => - val userHasAcceptedLatestVersion = userHasAcceptedLatestTosVersion(tos) - val permitsSystemUsage = tosAcceptancePermitsSystemUsage(samUser, tos) - TermsOfServiceComplianceStatus(samUser.id, userHasAcceptedLatestVersion, permitsSystemUsage) - } + def getTosComplianceStatus(samUser: SamUser, samRequestContext: SamRequestContext): IO[TermsOfServiceComplianceStatus] = for { + latestUserTos <- directoryDao.getLatestUserTos(samUser.id, samRequestContext) + previousUserTos <- directoryDao.getUserTos(samUser.id, tosConfig.rollingAcceptanceWindowPreviousTosVersion, samRequestContext) + userHasAcceptedLatestVersion = userHasAcceptedLatestTosVersion(latestUserTos) + permitsSystemUsage = tosAcceptancePermitsSystemUsage(samUser, latestUserTos, previousUserTos) + } yield TermsOfServiceComplianceStatus(samUser.id, userHasAcceptedLatestVersion, permitsSystemUsage) /** If grace period enabled, don't check ToS, return true If ToS disabled, return true Otherwise return true if user has accepted ToS, or is a service account */ - private def tosAcceptancePermitsSystemUsage(user: SamUser, userTos: Option[SamUserTos]): Boolean = { + private def tosAcceptancePermitsSystemUsage(user: SamUser, userTos: Option[SamUserTos], previousUserTos: Option[SamUserTos]): Boolean = { val now = Instant.now() val userIsServiceAccount = StandardSamUserDirectives.SAdomain.matches(user.email.value) // Service Account users do not need to accept ToS val userIsPermitted = userTos.exists { tos => val userHasAcceptedLatestVersion = userHasAcceptedLatestTosVersion(Option(tos)) val userCanUseSystemUnderGracePeriod = tosConfig.isGracePeriodEnabled && tos.action == TosTable.ACCEPT val tosDisabled = !tosConfig.isTosEnabled - val userInsideOfRollingAcceptanceWindow = tosConfig.rollingAcceptanceWindowExpiration.isAfter(now) + + val userHasAcceptedPreviousVersion = userHasAcceptedPreviousTosVersion(previousUserTos) + val userInsideOfRollingAcceptanceWindow = tosConfig.rollingAcceptanceWindowExpiration.isAfter(now) && userHasAcceptedPreviousVersion userHasAcceptedLatestVersion || userInsideOfRollingAcceptanceWindow || userCanUseSystemUnderGracePeriod || tosDisabled @@ -79,6 +81,9 @@ class TosService(val directoryDao: DirectoryDAO, val tosConfig: TermsOfServiceCo userTos.exists { tos => tos.version.contains(tosConfig.version) && tos.action == TosTable.ACCEPT } + + private def userHasAcceptedPreviousTosVersion(previousUserTos: Option[SamUserTos]): Boolean = + previousUserTos.exists(tos => tos.action == TosTable.ACCEPT) } trait TermsOfServiceDocument { diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockDirectoryDAO.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockDirectoryDAO.scala index 689df11cb..3472df795 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockDirectoryDAO.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockDirectoryDAO.scala @@ -340,7 +340,7 @@ class MockDirectoryDAO(val groups: mutable.Map[WorkbenchGroupIdentity, Workbench true } - override def getUserTos(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[SamUserTos]] = + override def getLatestUserTos(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[SamUserTos]] = loadUser(userId, samRequestContext).map { case None => None case Some(_) => diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAOSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAOSpec.scala index 123b12003..da09d7617 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAOSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAOSpec.scala @@ -1486,7 +1486,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B dao.acceptTermsOfService(defaultUser.id, tosConfig.version, samRequestContext).unsafeRunSync() shouldBe true // Assert - val userTos = dao.getUserTos(defaultUser.id, samRequestContext).unsafeRunSync() + val userTos = dao.getLatestUserTos(defaultUser.id, samRequestContext).unsafeRunSync() userTos should not be empty userTos.get.createdAt should beAround(Instant.now()) userTos.get.action shouldBe TosTable.ACCEPT @@ -1501,7 +1501,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B dao.acceptTermsOfService(defaultUser.id, "2", samRequestContext).unsafeRunSync() shouldBe true // Assert - val userTos = dao.getUserTos(defaultUser.id, samRequestContext).unsafeRunSync() + val userTos = dao.getLatestUserTos(defaultUser.id, samRequestContext).unsafeRunSync() userTos should not be empty userTos.get.createdAt should beAround(Instant.now()) userTos.get.action shouldBe TosTable.ACCEPT @@ -1516,7 +1516,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B dao.rejectTermsOfService(user.id, tosConfig.version, samRequestContext).unsafeRunSync() shouldBe true // Assert - val userTos = dao.getUserTos(user.id, samRequestContext).unsafeRunSync() + val userTos = dao.getLatestUserTos(user.id, samRequestContext).unsafeRunSync() userTos should not be empty userTos.get.createdAt should beAround(Instant.now()) userTos.get.action shouldBe TosTable.REJECT @@ -1530,7 +1530,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B dao.rejectTermsOfService(user.id, tosConfig.version, samRequestContext).unsafeRunSync() shouldBe true // Assert - val userTos = dao.getUserTos(user.id, samRequestContext).unsafeRunSync() + val userTos = dao.getLatestUserTos(user.id, samRequestContext).unsafeRunSync() userTos should not be empty userTos.get.createdAt should beAround(Instant.now()) userTos.get.action shouldBe TosTable.REJECT @@ -1544,7 +1544,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B dao.createUser(user, samRequestContext).unsafeRunSync() // Assert - val userTos = dao.getUserTos(user.id, samRequestContext).unsafeRunSync() + val userTos = dao.getLatestUserTos(user.id, samRequestContext).unsafeRunSync() userTos should be(None) } } diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/TosServiceSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/TosServiceSpec.scala index 6b4b6e5ef..7f6b56828 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/TosServiceSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/TosServiceSpec.scala @@ -87,7 +87,7 @@ class TosServiceSpec(_system: ActorSystem) val tosVersion = "2" val tosService = new TosService(dirDAO, TestSupport.tosConfig.copy(version = tosVersion)) - when(dirDAO.getUserTos(serviceAccountUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(serviceAccountUser.id, samRequestContext)) .thenReturn(IO.pure(Some(SamUserTos(serviceAccountUser.id, tosVersion, TosTable.ACCEPT, Instant.now())))) val complianceStatus = tosService.getTosComplianceStatus(serviceAccountUser, samRequestContext).unsafeRunSync() complianceStatus.permitsSystemUsage shouldBe true @@ -120,14 +120,14 @@ class TosServiceSpec(_system: ActorSystem) "when the user has not accepted any ToS version" - { "says the user has not accepted the latest version" in { - when(dirDAO.getUserTos(defaultUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(defaultUser.id, samRequestContext)) .thenReturn(IO.pure(None)) val complianceStatus = tosServiceV2.getTosComplianceStatus(defaultUser, samRequestContext).unsafeRunSync() complianceStatus.userHasAcceptedLatestTos shouldBe false } withoutGracePeriod - { cannotUseTheSystem in { - when(dirDAO.getUserTos(defaultUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(defaultUser.id, samRequestContext)) .thenReturn(IO.pure(None)) // CASE 1 val complianceStatus = tosServiceV2.getTosComplianceStatus(defaultUser, samRequestContext).unsafeRunSync() @@ -136,7 +136,7 @@ class TosServiceSpec(_system: ActorSystem) } withGracePeriod - { cannotUseTheSystem in { - when(dirDAO.getUserTos(defaultUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(defaultUser.id, samRequestContext)) .thenReturn(IO.pure(None)) // CASE 4 val complianceStatus = tosServiceV2GracePeriodEnabled.getTosComplianceStatus(defaultUser, samRequestContext).unsafeRunSync() @@ -146,14 +146,14 @@ class TosServiceSpec(_system: ActorSystem) } "when the user has accepted a non-current ToS version" - { "says the user has not accepted the latest version" in { - when(dirDAO.getUserTos(defaultUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(defaultUser.id, samRequestContext)) .thenReturn(IO.pure(None)) val complianceStatus = tosServiceV2.getTosComplianceStatus(defaultUser, samRequestContext).unsafeRunSync() complianceStatus.userHasAcceptedLatestTos shouldBe false } withoutGracePeriod - { cannotUseTheSystem in { - when(dirDAO.getUserTos(defaultUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(defaultUser.id, samRequestContext)) .thenReturn(IO.pure(None)) // CASE 2 val complianceStatus = tosServiceV2.getTosComplianceStatus(defaultUser, samRequestContext).unsafeRunSync() @@ -162,7 +162,7 @@ class TosServiceSpec(_system: ActorSystem) } withGracePeriod - { canUseTheSystem in { - when(dirDAO.getUserTos(defaultUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(defaultUser.id, samRequestContext)) .thenReturn(IO.pure(Option(SamUserTos(defaultUser.id, tosVersion, TosTable.ACCEPT, Instant.now())))) // CASE 5 val complianceStatus = tosServiceV2GracePeriodEnabled.getTosComplianceStatus(defaultUser, samRequestContext).unsafeRunSync() @@ -172,14 +172,14 @@ class TosServiceSpec(_system: ActorSystem) } "when the user has accepted the current ToS version" - { "says the user has accepted the latest version" in { - when(dirDAO.getUserTos(defaultUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(defaultUser.id, samRequestContext)) .thenReturn(IO.pure(Option(SamUserTos(defaultUser.id, tosVersion, TosTable.ACCEPT, Instant.now())))) val complianceStatus = tosServiceV2.getTosComplianceStatus(defaultUser, samRequestContext).unsafeRunSync() complianceStatus.userHasAcceptedLatestTos shouldBe true } withoutGracePeriod - { canUseTheSystem in { - when(dirDAO.getUserTos(defaultUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(defaultUser.id, samRequestContext)) .thenReturn(IO.pure(Option(SamUserTos(defaultUser.id, tosVersion, TosTable.ACCEPT, Instant.now())))) // CASE 3 val complianceStatus = tosServiceV2.getTosComplianceStatus(defaultUser, samRequestContext).unsafeRunSync() @@ -188,7 +188,7 @@ class TosServiceSpec(_system: ActorSystem) } withGracePeriod - { canUseTheSystem in { - when(dirDAO.getUserTos(defaultUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(defaultUser.id, samRequestContext)) .thenReturn(IO.pure(Option(SamUserTos(defaultUser.id, tosVersion, TosTable.ACCEPT, Instant.now())))) // CASE 6 val complianceStatus = tosServiceV2GracePeriodEnabled.getTosComplianceStatus(defaultUser, samRequestContext).unsafeRunSync() @@ -199,14 +199,14 @@ class TosServiceSpec(_system: ActorSystem) "when the user has rejected the latest ToS version" - { "says the user has rejected the latest version" in { - when(dirDAO.getUserTos(defaultUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(defaultUser.id, samRequestContext)) .thenReturn(IO.pure(Option(SamUserTos(defaultUser.id, tosVersion, TosTable.REJECT, Instant.now())))) val complianceStatus = tosServiceV2.getTosComplianceStatus(defaultUser, samRequestContext).unsafeRunSync() complianceStatus.userHasAcceptedLatestTos shouldBe false } withoutGracePeriod - { cannotUseTheSystem in { - when(dirDAO.getUserTos(defaultUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(defaultUser.id, samRequestContext)) .thenReturn(IO.pure(Option(SamUserTos(defaultUser.id, tosVersion, TosTable.REJECT, Instant.now())))) // CASE 1 val complianceStatus = tosServiceV2.getTosComplianceStatus(defaultUser, samRequestContext).unsafeRunSync() @@ -215,7 +215,7 @@ class TosServiceSpec(_system: ActorSystem) } withGracePeriod - { cannotUseTheSystem in { - when(dirDAO.getUserTos(defaultUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(defaultUser.id, samRequestContext)) .thenReturn(IO.pure(Option(SamUserTos(defaultUser.id, tosVersion, TosTable.REJECT, Instant.now())))) // CASE 4 val complianceStatus = tosServiceV2GracePeriodEnabled.getTosComplianceStatus(defaultUser, samRequestContext).unsafeRunSync() @@ -225,7 +225,7 @@ class TosServiceSpec(_system: ActorSystem) } "when a service account is using the api" - { "let it use the api regardless of tos status" in { - when(dirDAO.getUserTos(serviceAccountUser.id, samRequestContext)) + when(dirDAO.getLatestUserTos(serviceAccountUser.id, samRequestContext)) .thenReturn(IO.pure(None)) val complianceStatus = tosServiceV2.getTosComplianceStatus(serviceAccountUser, samRequestContext).unsafeRunSync() complianceStatus.permitsSystemUsage shouldBe true