[WOR-1240] Don't log tokeninfo requests in GoogleServicesDao (#2521)

broadinstitute · Sep 6, 2023 · 9eea4c8 · 9eea4c8
1 parent 7eb89d3
commit 9eea4c8
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 15 deletions.
diff --git a/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/GoogleServicesDAO.scala b/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/GoogleServicesDAO.scala
@@ -195,8 +195,6 @@ abstract class GoogleServicesDAO(groupsPrefix: String) extends ErrorReportable {
 
   def getResourceBufferServiceAccountCredential: Credential
 
-  def getServiceAccountRawlsUser(): Future[RawlsUser]
-
   def getServiceAccountUserInfo(): Future[UserInfo]
 
   def getBucketDetails(bucket: String, project: GoogleProjectId): Future[WorkspaceBucketOptions]

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
@@ -1221,16 +1221,17 @@ class HttpGoogleServicesDAO(val clientSecrets: GoogleClientSecrets,
     }
   }
 
-  def getServiceAccountRawlsUser(): Future[RawlsUser] =
-    getRawlsUserForCreds(getBucketServiceAccountCredential)
-
   def getRawlsUserForCreds(creds: Credential): Future[RawlsUser] = {
     implicit val service = GoogleInstrumentedService.Groups
     val oauth2 = new Builder(httpTransport, jsonFactory, null).setApplicationName(appName).build()
     Future {
       creds.refreshToken()
-      val tokenInfo = executeGoogleRequest(oauth2.tokeninfo().setAccessToken(creds.getAccessToken))
+      val tokenInfo = executeGoogleRequest(oauth2.tokeninfo().setAccessToken(creds.getAccessToken), logRequest = false)
       RawlsUser(RawlsUserSubjectId(tokenInfo.getUserId), RawlsUserEmail(tokenInfo.getEmail))
+    }.recover { case e: GoogleJsonResponseException =>
+      throw new RawlsExceptionWithErrorReport(
+        ErrorReport(StatusCodes.InternalServerError, s"Failed to get token info: ${e.getMessage}")
+      )
     }
   }
 

diff --git a/core/src/test/scala/org/broadinstitute/dsde/rawls/dataaccess/MockGoogleServicesDAO.scala b/core/src/test/scala/org/broadinstitute/dsde/rawls/dataaccess/MockGoogleServicesDAO.scala
@@ -199,9 +199,6 @@ class MockGoogleServicesDAO(groupsPrefix: String,
 
   def toGoogleGroupName(groupName: RawlsGroupName): String = s"GROUP_${groupName.value}@dev.firecloud.org"
 
-  override def getServiceAccountRawlsUser(): Future[RawlsUser] =
-    Future.successful(RawlsUser(RawlsUserSubjectId("12345678000"), RawlsUserEmail("[email protected]")))
-
   def getServiceAccountUserInfo(): Future[UserInfo] = Future.successful(
     UserInfo(RawlsUserEmail("[email protected]"), OAuth2BearerToken("test_token"), 0, RawlsUserSubjectId("12345678000"))
   )

diff --git a/google/src/main/scala/org/broadinstitute/dsde/rawls/google/GoogleUtilities.scala b/google/src/main/scala/org/broadinstitute/dsde/rawls/google/GoogleUtilities.scala
@@ -64,8 +64,10 @@ trait GoogleUtilities extends LazyLogging with InstrumentedRetry with GoogleInst
   ): Future[T] =
     retryExponentially(when500or400orGoogleError)(() => Future(blocking(op())).recover(recover))
 
-  protected def executeGoogleRequest[T](request: AbstractGoogleClientRequest[T])(implicit counters: GoogleCounters): T =
-    executeGoogleCall(request) { response =>
+  protected def executeGoogleRequest[T](request: AbstractGoogleClientRequest[T], logRequest: Boolean = true)(implicit
+    counters: GoogleCounters
+  ): T =
+    executeGoogleCall(request, logRequest) { response =>
       response.parseAs(request.getResponseClass)
     }
 
@@ -86,25 +88,26 @@ trait GoogleUtilities extends LazyLogging with InstrumentedRetry with GoogleInst
     }
 
   protected def executeGoogleCall[A, B](
-    request: AbstractGoogleClientRequest[A]
+    request: AbstractGoogleClientRequest[A],
+    logRequest: Boolean = true
   )(processResponse: (com.google.api.client.http.HttpResponse) => B)(implicit counters: GoogleCounters): B = {
     val start = System.currentTimeMillis()
     Try {
       request.executeUnparsed()
     } match {
       case Success(response) =>
-        logGoogleRequest(request, start, response)
+        if (logRequest) logGoogleRequest(request, start, response)
         instrumentGoogleRequest(request, start, Right(response))
         try
           processResponse(response)
         finally
           response.disconnect()
       case Failure(httpRegrets: HttpResponseException) =>
-        logGoogleRequest(request, start, httpRegrets)
+        if (logRequest) logGoogleRequest(request, start, httpRegrets)
         instrumentGoogleRequest(request, start, Left(httpRegrets))
         throw httpRegrets
       case Failure(regrets) =>
-        logGoogleRequest(request, start, regrets)
+        if (logRequest) logGoogleRequest(request, start, regrets)
         instrumentGoogleRequest(request, start, Left(regrets))
         throw regrets
     }