checkBucketReadAccess / oauth scopes (#957)

always use pet token in checkBucketReadAccess

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))
     }
   }
 

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
 

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]
 }

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)
       }
     }
   }

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")