Merge branch 'tl_ID-1242_azure_private_container_registry_resource' o…

…f github.com:broadinstitute/sam into tl_ID-1242_azure_private_container_registry_resource

pact4s/src/test/resources/reference.conf

@@ -119,8 +119,6 @@ testStuff = {
 oidc {
   authorityEndpoint = "https://accounts.google.com"
   oidcClientId = "some-client"
-  oidcClientSecret = "some-secret"
-  legacyGoogleClientId = "another-client"
 }
 
 liquibase {

project/Dependencies.scala

@@ -11,14 +11,14 @@ object Dependencies {
   val postgresDriverVersion = "42.7.2"
   val sentryVersion = "6.15.0"
 
-  val workbenchLibV = "1c0cf92" // If updating this, make sure googleStorageLocal in test dependencies is up-to-date
+  val workbenchLibV = "d16cba9" // If updating this, make sure googleStorageLocal in test dependencies is up-to-date
   val workbenchUtilV = s"0.10-$workbenchLibV"
   val workbenchUtil2V = s"0.9-$workbenchLibV"
   val workbenchModelV = s"0.19-$workbenchLibV"
   val workbenchGoogleV = s"0.30-$workbenchLibV"
   val workbenchGoogle2V = s"0.36-$workbenchLibV"
   val workbenchNotificationsV = s"0.6-$workbenchLibV"
-  val workbenchOauth2V = s"0.5-$workbenchLibV"
+  val workbenchOauth2V = s"0.7-$workbenchLibV"
   val monocleVersion = "2.0.5"
   val crlVersion = "1.2.30-SNAPSHOT"
   val tclVersion = "1.0.5-SNAPSHOT"

src/main/resources/sam.conf

@@ -37,8 +37,6 @@ termsOfService {
 oidc {
   authorityEndpoint = ${?OIDC_AUTHORITY_ENDPOINT}
   oidcClientId = ${?OIDC_CLIENT_ID}
-  oidcClientSecret = ${?OIDC_CLIENT_SECRET}
-  legacyGoogleClientId = ${?LEGACY_GOOGLE_CLIENT_ID}
 }
 
 schemaLock {

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

@@ -11,14 +11,8 @@ info:
 servers:
   - url: /
 security:
-  - googleoauth:
-      - openid
-      - email
-      - profile
   - oidc:
       - openid
-      - email
-      - profile
 paths:
   /api/admin/v1/user/{userId}:
     get:
@@ -4638,22 +4632,7 @@ components:
           items:
             type: string
   securitySchemes:
-    googleoauth:
-      type: oauth2
-      flows:
-        implicit:
-          authorizationUrl: https://accounts.google.com/o/oauth2/auth
-          scopes:
-            openid: open id authorization
-            email: email authorization
-            profile: profile authorization
     oidc:
-      type: oauth2
-      flows:
-        authorizationCode:
-          authorizationUrl: /oauth2/authorize
-          tokenUrl: /oauth2/token
-          scopes:
-            openid: open id authorization
-            email: email authorization
-            profile: profile authorization
+      type: openIdConnect
+      openIdConnectUrl: OPEN_ID_CONNECT_URL
+      x-tokenName: id_token

src/main/scala/org/broadinstitute/dsde/workbench/sam/Boot.scala

@@ -34,7 +34,7 @@ import org.broadinstitute.dsde.workbench.google.{
 }
 import org.broadinstitute.dsde.workbench.google2.{GoogleStorageInterpreter, GoogleStorageService}
 import org.broadinstitute.dsde.workbench.model.WorkbenchEmail
-import org.broadinstitute.dsde.workbench.oauth2.{ClientId, ClientSecret, OpenIDConnectConfiguration}
+import org.broadinstitute.dsde.workbench.oauth2.{ClientId, OpenIDConnectConfiguration}
 import org.broadinstitute.dsde.workbench.sam.api.{LivenessRoutes, SamRoutes, StandardSamUserDirectives}
 import org.broadinstitute.dsde.workbench.sam.azure.{AzureService, CrlService}
 import org.broadinstitute.dsde.workbench.sam.config.AppConfig.AdminConfig
@@ -140,8 +140,6 @@ object Boot extends IOApp with LazyLogging {
         OpenIDConnectConfiguration[IO](
           appConfig.oidcConfig.authorityEndpoint,
           ClientId(appConfig.oidcConfig.clientId),
-          oidcClientSecret = appConfig.oidcConfig.clientSecret.map(ClientSecret),
-          extraGoogleClientId = appConfig.oidcConfig.legacyGoogleClientId.map(ClientId),
           extraAuthParams = Some("prompt=login")
         )
       )
@@ -423,7 +421,14 @@ object Boot extends IOApp with LazyLogging {
     )
     val tosService = new TosService(cloudExtensionsInitializer.cloudExtensions, directoryDAO, config.termsOfServiceConfig)
     val userService =
-      new UserService(directoryDAO, cloudExtensionsInitializer.cloudExtensions, config.blockedEmailDomains, tosService, config.azureServicesConfig)
+      new UserService(
+        directoryDAO,
+        cloudExtensionsInitializer.cloudExtensions,
+        config.blockedEmailDomains,
+        tosService,
+        config.azureServicesConfig,
+        Seq(config.emailDomain)
+      )
     val statusService =
       new StatusService(directoryDAO, cloudExtensionsInitializer.cloudExtensions, 10 seconds, 1 minute)
     val managedGroupService =

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

@@ -37,9 +37,7 @@ object AppConfig {
   implicit val oidcReader: ValueReader[OidcConfig] = ValueReader.relative { config =>
     OidcConfig(
       config.getString("authorityEndpoint"),
-      config.getString("oidcClientId"),
-      config.as[Option[String]]("oidcClientSecret"),
-      config.as[Option[String]]("legacyGoogleClientId")
+      config.getString("oidcClientId")
     )
   }
 

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

@@ -1,3 +1,3 @@
 package org.broadinstitute.dsde.workbench.sam.config
 
-case class OidcConfig(authorityEndpoint: String, clientId: String, clientSecret: Option[String], legacyGoogleClientId: Option[String])
+case class OidcConfig(authorityEndpoint: String, clientId: String)

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

@@ -50,7 +50,13 @@ class ManagedGroupService(
       )
 
     validateGroupName(groupId.value)
+    val groupEmail = WorkbenchEmail(constructEmail(groupId.value))
     for {
+      _ <- directoryDAO.loadSubjectFromEmail(groupEmail, samRequestContext).flatMap {
+        case Some(_) =>
+          IO.raiseError(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.Conflict, s"subject with email $groupEmail already exists")))
+        case None => IO.pure(())
+      }
       managedGroup <- resourceService.createResource(
         managedGroupType,
         groupId,

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

@@ -29,7 +29,8 @@ class UserService(
     val cloudExtensions: CloudExtensions,
     blockedEmailDomains: Seq[String],
     tosService: TosService,
-    azureConfig: Option[AzureServicesConfig] = None
+    azureConfig: Option[AzureServicesConfig] = None,
+    nonInvitableDomains: Seq[String] = Seq.empty
 )(implicit
     val executionContext: ExecutionContext
 ) extends LazyLogging {
@@ -247,7 +248,7 @@ class UserService(
 
   def inviteUser(inviteeEmail: WorkbenchEmail, samRequestContext: SamRequestContext): IO[UserStatusDetails] =
     for {
-      _ <- validateEmailAddress(inviteeEmail, blockedEmailDomains)
+      _ <- validateEmailAddress(inviteeEmail, blockedEmailDomains, nonInvitableDomains)
       existingSubject <- directoryDAO.loadSubjectFromEmail(inviteeEmail, samRequestContext)
       createdUser <- existingSubject match {
         case None => createUserInternal(SamUser(genWorkbenchUserId(System.currentTimeMillis()), None, inviteeEmail, None, false), samRequestContext)
@@ -436,14 +437,26 @@ class UserService(
 
   // moved this method from the UserService companion object into this class
   // because Mockito would not let us spy/mock the static method
-  def validateEmailAddress(email: WorkbenchEmail, blockedEmailDomains: Seq[String]): IO[Unit] =
+  def validateEmailAddress(email: WorkbenchEmail, blockedEmailDomains: Seq[String], nonInvitableDomain: Seq[String]): IO[Unit] =
     email.value match {
-      case emailString if blockedEmailDomains.exists(domain => emailString.endsWith("@" + domain) || emailString.endsWith("." + domain)) =>
+      case emailString if matchesBadDomain(emailString, blockedEmailDomains) =>
         IO.raiseError(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.BadRequest, s"email domain not permitted [${email.value}]")))
+      case emailString if matchesBadDomain(emailString, nonInvitableDomain) =>
+        IO.raiseError(
+          new WorkbenchExceptionWithErrorReport(
+            ErrorReport(
+              StatusCodes.BadRequest,
+              s"Email domain cannot be invited [${email.value}]. If you are trying to invite a group, please make sure that group exists before adding it to a resource policy."
+            )
+          )
+        )
       case UserService.emailRegex() => IO.unit
       case _ => IO.raiseError(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.BadRequest, s"invalid email address [${email.value}]")))
     }
 
+  private def matchesBadDomain(emailString: String, badDomains: Seq[String]): Boolean =
+    badDomains.exists(domain => emailString.endsWith("@" + domain) || emailString.endsWith("." + domain))
+
   def getUserAllowances(samUser: SamUser, samRequestContext: SamRequestContext): IO[SamUserAllowances] =
     for {
       tosStatus <- tosService.getTermsOfServiceComplianceStatus(samUser, samRequestContext)

src/test/resources/reference.conf

@@ -111,8 +111,6 @@ testStuff = {
 oidc {
   authorityEndpoint = "https://accounts.google.com"
   oidcClientId = "some-client"
-  oidcClientSecret = "some-secret"
-  legacyGoogleClientId = "another-client"
 }
 
 liquibase {

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

@@ -152,7 +152,7 @@ class ManagedGroupServiceSpec
     val exception = intercept[WorkbenchExceptionWithErrorReport] {
       runAndWait(managedGroupService.createManagedGroup(ResourceId(groupName), dummyUser, samRequestContext = samRequestContext))
     }
-    exception.getMessage should include("A resource of this type and name already exists")
+    exception.getMessage should include(s"subject with email $groupName@$testDomain already exists")
     managedGroupService.loadManagedGroup(resourceId, samRequestContext).unsafeRunSync() shouldEqual None
   }
 

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

@@ -8,7 +8,7 @@ import cats.effect.IO
 import cats.effect.unsafe.implicits.{global => globalEc}
 import org.broadinstitute.dsde.workbench.model._
 import org.broadinstitute.dsde.workbench.sam.Generator.{arbNonPetEmail => _, _}
-import org.broadinstitute.dsde.workbench.sam.TestSupport.{databaseEnabled, databaseEnabledClue}
+import org.broadinstitute.dsde.workbench.sam.TestSupport.{databaseEnabled, databaseEnabledClue, truncateAll}
 import org.broadinstitute.dsde.workbench.sam.dataAccess.{DirectoryDAO, PostgresDirectoryDAO}
 import org.broadinstitute.dsde.workbench.sam.google.GoogleExtensions
 import org.broadinstitute.dsde.workbench.sam.matchers.BeSameUserMatcher.beSameUserAs
@@ -35,7 +35,7 @@ import scala.concurrent.duration._
 
 // TODO: continue breaking down old UserServiceSpec tests into nested suites
 // See: https://www.scalatest.org/scaladoc/3.2.3/org/scalatest/Suite.html
-class UserServiceSpec(_system: ActorSystem) extends TestKit(_system) with Suite with BeforeAndAfterAll {
+class UserServiceSpec(_system: ActorSystem) extends TestKit(_system) with Suite with BeforeAndAfterAll with BeforeAndAfterEach {
   override def nestedSuites: IndexedSeq[Suite] =
     IndexedSeq(
       new CreateUserSpec,
@@ -54,6 +54,11 @@ class UserServiceSpec(_system: ActorSystem) extends TestKit(_system) with Suite
     TestKit.shutdownActorSystem(system)
     super.afterAll()
   }
+
+  override def beforeEach(): Unit = {
+    truncateAll
+    super.beforeEach()
+  }
 }
 
 // This test suite is deprecated.  It is still used and still has valid tests in it, but it should be broken out
@@ -641,7 +646,7 @@ class OldUserServiceSpec(_system: ActorSystem)
     implicit val arbEmail: Arbitrary[WorkbenchEmail] = Arbitrary(genEmail)
 
     forAll { email: WorkbenchEmail =>
-      assert(service.validateEmailAddress(email, Seq.empty).attempt.unsafeRunSync().isRight)
+      assert(service.validateEmailAddress(email, Seq.empty, Seq.empty).attempt.unsafeRunSync().isRight)
     }
   }
 
@@ -662,7 +667,7 @@ class OldUserServiceSpec(_system: ActorSystem)
     implicit val arbEmail: Arbitrary[WorkbenchEmail] = Arbitrary(genEmail)
 
     forAll { email: WorkbenchEmail =>
-      assert(service.validateEmailAddress(email, Seq.empty).attempt.unsafeRunSync().isLeft)
+      assert(service.validateEmailAddress(email, Seq.empty, Seq.empty).attempt.unsafeRunSync().isLeft)
     }
   }
 
@@ -683,7 +688,7 @@ class OldUserServiceSpec(_system: ActorSystem)
     implicit val arbEmail: Arbitrary[WorkbenchEmail] = Arbitrary(genEmail)
 
     forAll { email: WorkbenchEmail =>
-      assert(service.validateEmailAddress(email, Seq.empty).attempt.unsafeRunSync().isLeft)
+      assert(service.validateEmailAddress(email, Seq.empty, Seq.empty).attempt.unsafeRunSync().isLeft)
     }
   }
 
@@ -697,12 +702,17 @@ class OldUserServiceSpec(_system: ActorSystem)
     implicit val arbEmail: Arbitrary[WorkbenchEmail] = Arbitrary(genEmail)
 
     forAll { email: WorkbenchEmail =>
-      assert(service.validateEmailAddress(email, Seq.empty).attempt.unsafeRunSync().isLeft)
+      assert(service.validateEmailAddress(email, Seq.empty, Seq.empty).attempt.unsafeRunSync().isLeft)
     }
   }
 
   it should "reject blocked email domain" in {
-    assert(service.validateEmailAddress(WorkbenchEmail("[email protected]"), Seq("bar.com")).attempt.unsafeRunSync().isLeft)
-    assert(service.validateEmailAddress(WorkbenchEmail("[email protected]"), Seq("bar.com")).attempt.unsafeRunSync().isLeft)
+    assert(service.validateEmailAddress(WorkbenchEmail("[email protected]"), Seq("bar.com"), Seq.empty).attempt.unsafeRunSync().isLeft)
+    assert(service.validateEmailAddress(WorkbenchEmail("[email protected]"), Seq("bar.com"), Seq.empty).attempt.unsafeRunSync().isLeft)
+  }
+
+  it should "reject an un-invitable email domain" in {
+    assert(service.validateEmailAddress(WorkbenchEmail("[email protected]"), Seq.empty, Seq("bar.com")).attempt.unsafeRunSync().isLeft)
+    assert(service.validateEmailAddress(WorkbenchEmail("[email protected]"), Seq.empty, Seq("bar.com")).attempt.unsafeRunSync().isLeft)
   }
 }

src/test/scala/org/broadinstitute/dsde/workbench/sam/service/UserServiceSpecs/CreateUserSpec.scala

@@ -383,6 +383,23 @@ class CreateUserSpec extends UserServiceTestTraits {
 
   describe("An invited User") {
 
+    it("should not be able to be invited with a non-invitable domain") {
+      // Arrange
+      val nonInvitableDomain = "non-invitable-domain.com"
+      val invitedGoogleUser = genWorkbenchUserGoogle.sample.get.copy(email = WorkbenchEmail(s"user@$nonInvitableDomain"))
+      val directoryDAO = MockDirectoryDaoBuilder(allUsersGroup).build
+      val cloudExtensions = MockCloudExtensionsBuilder(allUsersGroup).build
+      val userService = new UserService(directoryDAO, cloudExtensions, Seq.empty, defaultTosService, None, Seq(nonInvitableDomain))
+
+      // Act
+      val exception = intercept[WorkbenchExceptionWithErrorReport] {
+        runAndWait(userService.inviteUser(invitedGoogleUser.email, samRequestContext))
+      }
+
+      // Assert
+      exception.errorReport.message should include("Email domain cannot be invited")
+    }
+
     it("should be able to be invited with no marketing consent") {
       // Arrange
       val invitedGoogleUser = genWorkbenchUserGoogle.sample.get