diff --git a/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/HttpGoogleServicesDAO.scala b/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/HttpGoogleServicesDAO.scala index 02be80c486..8539cc0cf9 100644 --- a/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/HttpGoogleServicesDAO.scala +++ b/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/HttpGoogleServicesDAO.scala @@ -6,7 +6,7 @@ import java.util.UUID import akka.actor.{ActorRef, ActorSystem} import akka.http.scaladsl.Http import akka.http.scaladsl.client.RequestBuilding -import akka.http.scaladsl.model.headers.OAuth2BearerToken +import akka.http.scaladsl.model.headers.{Authorization, OAuth2BearerToken} import akka.http.scaladsl.model.{StatusCode, StatusCodes} import akka.stream.Materializer import com.google.api.client.auth.oauth2.{Credential, TokenResponse} @@ -44,7 +44,7 @@ import org.broadinstitute.dsde.rawls.metrics.GoogleInstrumentedService._ import org.broadinstitute.dsde.rawls.model.UserAuthJsonSupport._ import org.broadinstitute.dsde.rawls.model.WorkspaceAccessLevels._ import org.broadinstitute.dsde.rawls.model._ -import org.broadinstitute.dsde.rawls.util.{FutureSupport, Retry} +import org.broadinstitute.dsde.rawls.util.{FutureSupport, HttpClientUtilsStandard, Retry} import org.broadinstitute.dsde.rawls.{RawlsException, RawlsExceptionWithErrorReport} import org.broadinstitute.dsde.workbench.model.{WorkbenchEmail, WorkbenchGroup} import org.joda.time @@ -77,6 +77,9 @@ class HttpGoogleServicesDAO( override val workbenchMetricBaseName: String, proxyNamePrefix: String)(implicit val system: ActorSystem, val materializer: Materializer, implicit val executionContext: ExecutionContext ) extends GoogleServicesDAO(groupsPrefix) with FutureSupport with GoogleUtilities { + val http = Http(system) + val httpClientUtils = HttpClientUtilsStandard() + val groupMemberRole = "MEMBER" // the Google Group role corresponding to a member (note that this is distinct from the GCS roles defined in WorkspaceAccessLevel) val API_SERVICE_MANAGEMENT = "ServiceManagement" val API_CLOUD_RESOURCE_MANAGER = "CloudResourceManager" @@ -715,17 +718,25 @@ class HttpGoogleServicesDAO( } def diagnosticBucketRead(userInfo: UserInfo, bucketName: String): Future[Option[ErrorReport]] = { - implicit val service = GoogleInstrumentedService.Storage - Future { - val getter = getStorage(getUserCredential(userInfo)).buckets().get(bucketName) - try { - blocking { - executeGoogleRequest(getter) - } - None - } catch { - case t: HttpResponseException => Some(ErrorReport(StatusCode.int2StatusCode(t.getStatusCode), t.getMessage)) + // we make requests to the target bucket as the default pet, if the user only has read access. + // the default pet is created in a project without APIs enabled. Due to Google issue #16062674, we cannot + // call the GCS JSON API as the default pet; we must use the XML API. In turn, this means we cannot use + // the GCS client library (which internally calls the JSON API); we must hand-code a call to the XML API. + + // the "proper" request to make into the XML API is HEAD-bucket; see https://cloud.google.com/storage/docs/xml-api/overview. + // however, the akka-http client is vulnerable to connection-pool exhaustion with HEAD requests; see akka/akka-http#1495. + // therefore, we make a request to GET /?storageClass. Since all we care about is the 200 vs. 40x status code + // in the response, this is an equivalent request. + val bucketUrl = s"https://$bucketName.storage.googleapis.com/?storageClass" + val bucketRequest = httpClientUtils.addHeader(RequestBuilding.Head(bucketUrl), Authorization(userInfo.accessToken)) + + httpClientUtils.executeRequest(http, bucketRequest) map { httpResponse => + httpResponse.status match { + case StatusCodes.OK => None + case x => Some(ErrorReport(x, x.defaultMessage())) } + } recover { + case t:Throwable => Some(ErrorReport(t)) } } diff --git a/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/HttpSamDAO.scala b/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/HttpSamDAO.scala index 5edd545605..1d30dc763c 100644 --- a/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/HttpSamDAO.scala +++ b/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/HttpSamDAO.scala @@ -174,6 +174,10 @@ class HttpSamDAO(baseSamServiceURL: String, serviceAccountCreds: Credential)(imp retry(when401or500) { () => asRawlsSAPipeline[String] apply RequestBuilding.Get(url) } } + override def getDefaultPetServiceAccountKeyForUser(userInfo: UserInfo): Future[String] = { + val url = samServiceURL + "/api/google/v1/user/petServiceAccount/key" + retry(when401or500) { () => pipeline[String](userInfo) apply RequestBuilding.Get(url) } + } //managed group apis diff --git a/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/SamDAO.scala b/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/SamDAO.scala index cdaf3fee5f..fd295a3bb7 100644 --- a/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/SamDAO.scala +++ b/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/SamDAO.scala @@ -53,6 +53,7 @@ trait SamDAO extends ErrorReportable { * @return a json blob */ def getPetServiceAccountKeyForUser(googleProject: String, userEmail: RawlsUserEmail): Future[String] + def getDefaultPetServiceAccountKeyForUser(userInfo: UserInfo): Future[String] def getStatus(): Future[SubsystemStatus] } diff --git a/core/src/main/scala/org/broadinstitute/dsde/rawls/workspace/WorkspaceService.scala b/core/src/main/scala/org/broadinstitute/dsde/rawls/workspace/WorkspaceService.scala index db415b3fe7..087dee5364 100644 --- a/core/src/main/scala/org/broadinstitute/dsde/rawls/workspace/WorkspaceService.scala +++ b/core/src/main/scala/org/broadinstitute/dsde/rawls/workspace/WorkspaceService.scala @@ -1666,6 +1666,17 @@ class WorkspaceService(protected val userInfo: UserInfo, val dataSource: SlickDa } } + /* + If the user only has read access, check the bucket using the default pet. + If the user has a higher level of access, check the bucket using the pet for this workspace's project. + + We use the default pet when possible because the default pet is created in a per-user shell project, i.e. not in + this workspace's project. This prevents proliferation of service accounts within this workspace's project. For + FireCloud's common read-only public workspaces, this is an important safeguard; else those common projects + would constantly hit limits on the number of allowed service accounts. + + If the user has write access, we need to use the pet for this workspace's project in order to get accurate results. + */ def checkBucketReadAccess(workspaceName: WorkspaceName) = { for { (workspace, maxAccessLevel) <- dataSource.inTransaction { dataAccess => @@ -1676,20 +1687,18 @@ class WorkspaceService(protected val userInfo: UserInfo, val dataSource: SlickDa } } - petKey <- samDAO.getPetServiceAccountKeyForUser(workspace.namespace, userInfo.userEmail) + petKey <- if (maxAccessLevel >= WorkspaceAccessLevels.Write) + samDAO.getPetServiceAccountKeyForUser(workspace.namespace, userInfo.userEmail) + else + samDAO.getDefaultPetServiceAccountKeyForUser(userInfo) accessToken <- gcsDAO.getAccessTokenUsingJson(petKey) - //if the user only has read access, it doesn't matter if their pet doesn't have access because it will never be used. so we skip over it. - resultsForPet <- if (maxAccessLevel >= WorkspaceAccessLevels.Write) { - gcsDAO.diagnosticBucketRead(UserInfo(userInfo.userEmail, OAuth2BearerToken(accessToken), 60, userInfo.userSubjectId), workspace.bucketName) - } else Future.successful(None) - resultsForUser <- gcsDAO.diagnosticBucketRead(userInfo, workspace.bucketName) + resultsForPet <- gcsDAO.diagnosticBucketRead(UserInfo(userInfo.userEmail, OAuth2BearerToken(accessToken), 60, userInfo.userSubjectId), workspace.bucketName) } yield { - (resultsForUser, resultsForPet) match { - case (None, None) => RequestComplete(StatusCodes.OK) - case (Some(report), _) => RequestComplete(report) // report actual user does not have access first - case (_, Some(report)) => RequestComplete(report) + resultsForPet match { + case None => RequestComplete(StatusCodes.OK) + case Some(report) => RequestComplete(report) } } } diff --git a/core/src/test/scala/org/broadinstitute/dsde/rawls/mock/RemoteServicesMockServer.scala b/core/src/test/scala/org/broadinstitute/dsde/rawls/mock/RemoteServicesMockServer.scala index 45f4920934..b77a2d4fa0 100644 --- a/core/src/test/scala/org/broadinstitute/dsde/rawls/mock/RemoteServicesMockServer.scala +++ b/core/src/test/scala/org/broadinstitute/dsde/rawls/mock/RemoteServicesMockServer.scala @@ -978,6 +978,19 @@ class RemoteServicesMockServer(port:Int) extends RawlsTestUtils { .withStatusCode(StatusCodes.Created.intValue) ) + mockServer.when( + request() + .withMethod("GET") + .withPath("/api/google/v1/user/petServiceAccount/key") + ).respond( + response() + .withHeaders(jsonHeader) + .withBody( + """{"client_email": "pet-110347448408766049948@broad-dsde-dev.iam.gserviceaccount.com"}""".stripMargin + ) + .withStatusCode(StatusCodes.OK.intValue) + ) + mockServer.when( request() .withMethod("GET")