Skip to content

Commit

Permalink
Merge branch 'develop' into SWARM-CA-240
Browse files Browse the repository at this point in the history
# Conflicts:
#	src/main/resources/reference.conf
  • Loading branch information
gpolumbo-broad committed May 29, 2019
2 parents 374fa5f + 0384656 commit 27a385b
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ object SamConfig extends CommonConfig {
object GCS extends CommonGCS {
val appsDomain = gcsConfig.getString("appsDomain")
val pathToSamTestFirestoreAccountPath = gcsConfig.getString("firestoreAccountPath")
val serviceProject = gcsConfig.getString("serviceProject")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,15 @@ class SamApiSpec extends FreeSpec with BillingFixtures with Matchers with ScalaF
.getOrElse(Set.empty) should contain theSameElementsAs Set(allUsersGroupEmail.value),
5.minutes, 5.seconds)
}

"should not allow pet creation in a project that belongs to an external org" in {
val userAuthToken = UserPool.chooseAnyUser.makeAuthToken()
val restException = intercept[RestException] {
Sam.user.petServiceAccountEmail(gcsConfig.serviceProject)(userAuthToken)
}
import spray.json._
restException.message.parseJson.asJsObject.fields("statusCode") shouldBe JsNumber(400)
}
}

private def getFieldFromJson(jsonKey: String, field: String): String = {
Expand Down
10 changes: 5 additions & 5 deletions project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ object Dependencies {
val catsEffectV = "1.2.0"

val workbenchUtilV = "0.5-6942040"
val workbenchModelV = "0.13-7e86fba"
val workbenchGoogleV = "0.18-5941525"
val workbenchModelV = "0.13-d4e0782"
val workbenchGoogleV = "0.18-d4e0782"
val workbenchGoogle2V = "0.1-8328aae"
val workbenchNotificationsV = "0.1-f2a0020"
val monocleVersion = "1.5.1-cats"
Expand Down Expand Up @@ -58,10 +58,10 @@ object Dependencies {

// All of workbench-libs pull in Akka; exclude it since we provide our own Akka dependency.
// workbench-google pulls in workbench-{util, model, metrics}; exclude them so we can control the library versions individually.
val workbenchUtil: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-util" % workbenchUtilV
val workbenchUtil: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-util" % workbenchUtilV excludeAll(excludeWorkbenchModel)
val workbenchModel: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-model" % workbenchModelV
val workbenchGoogle: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-google" % workbenchGoogleV
val workbenchGoogle2: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-google2" % workbenchGoogle2V
val workbenchGoogle: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-google" % workbenchGoogleV excludeAll(excludeWorkbenchModel, excludeWorkbenchUtil)
val workbenchGoogle2: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-google2" % workbenchGoogle2V excludeAll(excludeWorkbenchModel, excludeWorkbenchUtil)
val workbenchNotifications: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-notifications" % workbenchNotificationsV excludeAll(excludeWorkbenchGoogle, excludeWorkbenchModel)
val workbenchGoogleTests: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-google" % workbenchGoogleV % "test" classifier "tests" excludeAll(excludeWorkbenchUtil, excludeWorkbenchModel)
val workbenchGoogle2Tests: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-google2" % workbenchGoogle2V % "test" classifier "tests" excludeAll(excludeWorkbenchUtil, excludeWorkbenchModel)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,10 +232,7 @@ object Boot extends IOApp with LazyLogging {
)
val googleProjectDAO = new HttpGoogleProjectDAO(
config.googleServicesConfig.appName,
Pem(
config.googleServicesConfig.billingPemEmail,
new File(config.googleServicesConfig.pathToBillingPem),
Option(config.googleServicesConfig.billingEmail)),
Pem(WorkbenchEmail(config.googleServicesConfig.serviceAccountClientId), new File(config.googleServicesConfig.pemFile)),
"google"
)
val googleKeyCache =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ final case class GoogleServicesConfig(
serviceAccountClientProject: GoogleProject,
subEmail: WorkbenchEmail,
projectServiceAccount: WorkbenchEmail,
pathToBillingPem: String,
billingPemEmail: WorkbenchEmail,
billingEmail: WorkbenchEmail,
groupSyncPubSubProject: String,
groupSyncPollInterval: FiniteDuration,
groupSyncPollJitter: FiniteDuration,
Expand All @@ -38,7 +35,8 @@ final case class GoogleServicesConfig(
googleKeyCacheConfig: GoogleKeyCacheConfig,
resourceNamePrefix: Option[String],
adminSdkServiceAccounts: Option[NonEmptyList[ServiceAccountConfig]],
googleKms: GoogleKmsConfig
googleKms: GoogleKmsConfig,
terraGoogleOrgNumber: String
)

object GoogleServicesConfig {
Expand Down Expand Up @@ -87,9 +85,6 @@ object GoogleServicesConfig {
GoogleProject(config.getString("serviceAccountClientProject")),
WorkbenchEmail(config.getString("subEmail")),
WorkbenchEmail(config.getString("projectServiceAccount")),
config.getString("billing.pathToBillingPem"),
WorkbenchEmail(config.getString("billing.billingPemEmail")),
WorkbenchEmail(config.getString("billing.billingEmail")),
config.getString("groupSync.pubSubProject"),
org.broadinstitute.dsde.workbench.util.toScalaDuration(config.getDuration("groupSync.pollInterval")),
org.broadinstitute.dsde.workbench.util.toScalaDuration(config.getDuration("groupSync.pollJitter")),
Expand All @@ -100,7 +95,8 @@ object GoogleServicesConfig {
config.as[GoogleKeyCacheConfig]("googleKeyCache"),
config.as[Option[String]]("resourceNamePrefix"),
config.as[Option[NonEmptyList[ServiceAccountConfig]]]("adminSdkServiceAccounts"),
config.as[GoogleKmsConfig]("kms")
config.as[GoogleKmsConfig]("kms"),
config.getString("terraGoogleOrgNumber")
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import akka.http.scaladsl.model.headers.OAuth2BearerToken
import cats.effect.{ContextShift, IO}
import cats.implicits._
import com.google.api.client.googleapis.json.GoogleJsonResponseException
import com.google.api.client.http.HttpResponseException
import com.google.api.gax.rpc.AlreadyExistsException
import com.google.auth.oauth2.ServiceAccountCredentials
import com.google.protobuf.{Duration, Timestamp}
Expand Down Expand Up @@ -329,6 +330,7 @@ class GoogleExtensions(
// SA does not exist in google, create it and add it to the proxy group
case None =>
for {
_ <- assertProjectInTerraOrg(project)
sa <- IO.fromFuture(IO(googleIamDAO.createServiceAccount(project, petSaName, petSaDisplayName)))
r <- IO.fromFuture(IO(withProxyEmail(user.id) { proxyEmail =>
googleDirectoryDAO.addMemberToGroup(proxyEmail, sa.email)
Expand Down Expand Up @@ -362,6 +364,23 @@ class GoogleExtensions(
} yield p
}

private def assertProjectInTerraOrg(project: GoogleProject): IO[Unit] = {
val validOrg = IO.fromFuture(IO(googleProjectDAO.getAncestry(project.value).map { ancestry =>
ancestry.exists { ancestor =>
ancestor.getResourceId.getType == GoogleResourceTypes.Organization.value && ancestor.getResourceId.getId == googleServicesConfig.terraGoogleOrgNumber
}
})).recoverWith {
// if the getAncestry call results in a 403 error the project can't be in the right org
case e: HttpResponseException if e.getStatusCode == StatusCodes.Forbidden.intValue =>
IO.raiseError(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.BadRequest, s"Access denied from google accessing project ${project.value}, is it a Terra project?", e)))
}

validOrg.flatMap {
case true => IO.unit
case false => IO.raiseError(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.BadRequest, s"Project ${project.value} must be in Terra Organization")))
}
}

private def retrievePetAndSA(
userId: WorkbenchUserId,
petServiceAccountName: ServiceAccountName,
Expand Down Expand Up @@ -402,7 +421,7 @@ class GoogleExtensions(
private def getDefaultServiceAccountForShellProject(user: WorkbenchUser): Future[String] = {
val projectName = s"fc-${googleServicesConfig.environment.substring(0, Math.min(googleServicesConfig.environment.length(), 5))}-${user.id.value}" //max 30 characters. subject ID is 21
for {
creationOperationId <- googleProjectDAO.createProject(projectName).map(opId => Option(opId)) recover {
creationOperationId <- googleProjectDAO.createProject(projectName, googleServicesConfig.terraGoogleOrgNumber, GoogleResourceTypes.Organization).map(opId => Option(opId)) recover {
case gjre: GoogleJsonResponseException if gjre.getDetails.getCode == StatusCodes.Conflict.intValue => None
}
_ <- creationOperationId match {
Expand Down
1 change: 1 addition & 0 deletions src/test/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ googleServices {
serviceAccountClientProject = "broad-dsde-dev"
subEmail = "google@dev.test.firecloud.org"
projectServiceAccount = "broad-dsde-dev@gs-project-accounts.iam.gserviceaccount.com"
terraGoogleOrgNumber = "mock-org-number" # This org number needs to match what is specified in workbench-libs/google/src/main/scala/org/broadinstitute/dsde/workbench/google/HttpGoogleProjectDAO.scala.getAncestry

billing {
pathToBillingPem = "/etc/billing-account.pem"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ object TestSupport extends TestSupport {
val policyDAO = policyAccessDAO.getOrElse(new MockAccessPolicyDAO())
val pubSubDAO = new MockGooglePubSubDAO()
val googleStorageDAO = new MockGoogleStorageDAO()
val googleProjectDAO = new MockGoogleProjectDAO()
val notificationDAO = new PubSubNotificationDAO(pubSubDAO, "foo")
val cloudKeyCache = new GoogleKeyCache(fakeDistributedLock, googleIamDAO, googleStorageDAO, FakeGoogleStorageInterpreter, pubSubDAO, googleServicesConfig, petServiceAccountConfig)
val googleExt = cloudExtensions.getOrElse(new GoogleExtensions(
Expand All @@ -122,7 +123,7 @@ object TestSupport extends TestSupport {
pubSubDAO,
googleIamDAO,
googleStorageDAO,
null,
googleProjectDAO,
cloudKeyCache,
notificationDAO,
FakeGoogleKmsInterpreter,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import akka.testkit.TestKit
import cats.effect.IO
import cats.implicits._
import cats.data.NonEmptyList
import com.google.api.client.http.{HttpHeaders, HttpResponseException}
import com.google.api.services.cloudresourcemanager.model.Ancestor
import com.google.api.services.groupssettings.model.Groups
import com.unboundid.ldap.sdk.{LDAPConnection, LDAPConnectionPool}
import org.broadinstitute.dsde.workbench.dataaccess.PubSubNotificationDAO
Expand Down Expand Up @@ -355,8 +357,9 @@ class GoogleExtensionSpec(_system: ActorSystem) extends TestKit(_system) with Fl

val mockGoogleIamDAO = new MockGoogleIamDAO
val mockGoogleDirectoryDAO = new MockGoogleDirectoryDAO
val mockGoogleProjectDAO = new MockGoogleProjectDAO

val googleExtensions = new GoogleExtensions(TestSupport.fakeDistributedLock, dirDAO, null, mockGoogleDirectoryDAO, null, mockGoogleIamDAO, null, null, null, null, null, googleServicesConfig, petServiceAccountConfig, configResourceTypes)
val googleExtensions = new GoogleExtensions(TestSupport.fakeDistributedLock, dirDAO, null, mockGoogleDirectoryDAO, null, mockGoogleIamDAO, null, mockGoogleProjectDAO, null, null, null, googleServicesConfig, petServiceAccountConfig, configResourceTypes)
val service = new UserService(dirDAO, googleExtensions)

val defaultUserId = WorkbenchUserId("newuser123")
Expand Down Expand Up @@ -749,10 +752,11 @@ class GoogleExtensionSpec(_system: ActorSystem) extends TestKit(_system) with Fl
val mockGoogleDirectoryDAO = new MockGoogleDirectoryDAO
val mockGooglePubSubDAO = new MockGooglePubSubDAO
val mockGoogleStorageDAO = new MockGoogleStorageDAO
val mockGoogleProjectDAO = new MockGoogleProjectDAO
val notificationDAO = new PubSubNotificationDAO(mockGooglePubSubDAO, "foo")
val googleKeyCache = new GoogleKeyCache(TestSupport.fakeDistributedLock, mockGoogleIamDAO, mockGoogleStorageDAO, FakeGoogleStorageInterpreter, mockGooglePubSubDAO, googleServicesConfig, petServiceAccountConfig)

val googleExtensions = new GoogleExtensions(TestSupport.fakeDistributedLock, dirDAO, null, mockGoogleDirectoryDAO, null, mockGoogleIamDAO, mockGoogleStorageDAO, null, googleKeyCache, notificationDAO, null, googleServicesConfig, petServiceAccountConfig, configResourceTypes)
val googleExtensions = new GoogleExtensions(TestSupport.fakeDistributedLock, dirDAO, null, mockGoogleDirectoryDAO, null, mockGoogleIamDAO, mockGoogleStorageDAO, mockGoogleProjectDAO, googleKeyCache, notificationDAO, null, googleServicesConfig, petServiceAccountConfig, configResourceTypes)
val service = new UserService(dirDAO, googleExtensions)

(googleExtensions, service)
Expand Down Expand Up @@ -1121,4 +1125,63 @@ class GoogleExtensionSpec(_system: ActorSystem) extends TestKit(_system) with Fl
val constrained = synchronizer.isConstrainable(resource.fullyQualifiedId, accessPolicy)
constrained shouldEqual false
}

"createUserPetServiceAccount" should "return a failed IO when the project is not in the Terra Google Org" in {
val dirURI = new URI(directoryConfig.directoryUrl)
val connectionPool = new LDAPConnectionPool(new LDAPConnection(dirURI.getHost, dirURI.getPort, directoryConfig.user, directoryConfig.password), directoryConfig.connectionPoolSize)
val dirDAO = new LdapDirectoryDAO(connectionPool, directoryConfig, TestSupport.blockingEc, TestSupport.testMemberOfCache)
val schemaDao = new JndiSchemaDAO(directoryConfig, schemaLockConfig)

runAndWait(schemaDao.clearDatabase())
runAndWait(schemaDao.init())
runAndWait(schemaDao.createOrgUnits())

val mockGoogleIamDAO = new MockGoogleIamDAO
val mockGoogleDirectoryDAO = new MockGoogleDirectoryDAO
val mockGoogleProjectDAO = new MockGoogleProjectDAO
val garbageOrgGoogleServicesConfig = TestSupport.googleServicesConfig.copy(terraGoogleOrgNumber = "garbage")
val googleExtensions = new GoogleExtensions(TestSupport.fakeDistributedLock, dirDAO, null, mockGoogleDirectoryDAO, null, mockGoogleIamDAO, null, mockGoogleProjectDAO, null, null, null, garbageOrgGoogleServicesConfig, petServiceAccountConfig, configResourceTypes)

val defaultUserId = WorkbenchUserId("newuser123")
val defaultUserEmail = WorkbenchEmail("newuser@new.com")
val defaultUser = WorkbenchUser(defaultUserId, Some(GoogleSubjectId(defaultUserId.value)), defaultUserEmail)

val googleProject = GoogleProject("testproject")
val report = intercept[WorkbenchExceptionWithErrorReport] {
googleExtensions.createUserPetServiceAccount(defaultUser, googleProject).unsafeRunSync()
}

report.errorReport.statusCode shouldEqual Some(StatusCodes.BadRequest)
}

it should "return a failed IO when google returns a 403" in {
val dirURI = new URI(directoryConfig.directoryUrl)
val connectionPool = new LDAPConnectionPool(new LDAPConnection(dirURI.getHost, dirURI.getPort, directoryConfig.user, directoryConfig.password), directoryConfig.connectionPoolSize)
val dirDAO = new LdapDirectoryDAO(connectionPool, directoryConfig, TestSupport.blockingEc, TestSupport.testMemberOfCache)
val schemaDao = new JndiSchemaDAO(directoryConfig, schemaLockConfig)

runAndWait(schemaDao.clearDatabase())
runAndWait(schemaDao.init())
runAndWait(schemaDao.createOrgUnits())

val mockGoogleIamDAO = new MockGoogleIamDAO
val mockGoogleDirectoryDAO = new MockGoogleDirectoryDAO
val mockGoogleProjectDAO = new MockGoogleProjectDAO {
override def getAncestry(projectName: String): Future[Seq[Ancestor]] = {
Future.failed(new HttpResponseException.Builder(403, "Made up error message", new HttpHeaders()).build())
}
}
val googleExtensions = new GoogleExtensions(TestSupport.fakeDistributedLock, dirDAO, null, mockGoogleDirectoryDAO, null, mockGoogleIamDAO, null, mockGoogleProjectDAO, null, null, null, googleServicesConfig, petServiceAccountConfig, configResourceTypes)

val defaultUserId = WorkbenchUserId("newuser123")
val defaultUserEmail = WorkbenchEmail("newuser@new.com")
val defaultUser = WorkbenchUser(defaultUserId, Some(GoogleSubjectId(defaultUserId.value)), defaultUserEmail)

val googleProject = GoogleProject("testproject")
val report = intercept[WorkbenchExceptionWithErrorReport] {
googleExtensions.createUserPetServiceAccount(defaultUser, googleProject).unsafeRunSync()
}

report.errorReport.statusCode shouldEqual Some(StatusCodes.BadRequest)
}
}

0 comments on commit 27a385b

Please sign in to comment.