Using ErrorFactories for error dispatching

digital-asset · Oct 19, 2021 · 11d41a1 · 11d41a1
1 parent 87b18b2
commit 11d41a1
Show file tree

Hide file tree

Showing 6 changed files with 101 additions and 93 deletions.
diff --git a/ledger/ledger-api-auth/src/main/scala/com/digitalasset/ledger/api/auth/Authorizer.scala b/ledger/ledger-api-auth/src/main/scala/com/digitalasset/ledger/api/auth/Authorizer.scala
@@ -3,13 +3,15 @@
 
 package com.daml.ledger.api.auth
 
-import com.daml.error.{ContextualizedErrorLogger, DamlContextualizedErrorLogger, ErrorCodesVersionSwitcher, NoLogging}
-import com.daml.error.definitions.LedgerApiErrors
+import com.daml.error.{
+  ContextualizedErrorLogger,
+  DamlContextualizedErrorLogger,
+  ErrorCodesVersionSwitcher,
+}
 import com.daml.ledger.api.auth.interceptor.AuthorizationInterceptor
 import com.daml.ledger.api.v1.transaction_filter.TransactionFilter
 import com.daml.logging.{ContextualizedLogger, LoggingContext}
 import com.daml.platform.server.api.validation.ErrorFactories
-import io.grpc.StatusRuntimeException
 import io.grpc.stub.{ServerCallStreamObserver, StreamObserver}
 
 import java.time.Instant
@@ -27,8 +29,8 @@ final class Authorizer(
     errorCodesVersionSwitcher: ErrorCodesVersionSwitcher,
 ) {
   private val logger = ContextualizedLogger.get(this.getClass)
-  // TODO error codes: Enable logging
-  private implicit val contextualizedErrorLogger: ContextualizedErrorLogger = NoLogging
+  private val errorFactories = ErrorFactories(errorCodesVersionSwitcher)
+
   /** Validates all properties of claims that do not depend on the request,
     * such as expiration time or ledger ID.
     */
@@ -184,17 +186,22 @@ final class Authorizer(
       scso,
       claims,
       _.notExpired(now()),
-      authorizationError => permissionDenied(authorizationError.reason),
+      authorizationError => {
+        val errorLogger = new DamlContextualizedErrorLogger(logger, loggingContext, None)
+        errorFactories.permissionDenied(authorizationError.reason)(errorLogger)
+      },
     )
 
   private def authenticatedClaimsFromContext()(implicit
       loggingContext: LoggingContext
   ): Try[ClaimSet.Claims] = {
+    implicit val errorLogger: ContextualizedErrorLogger =
+      new DamlContextualizedErrorLogger(logger, loggingContext, None)
     AuthorizationInterceptor
       .extractClaimSetFromContext()
-      .fold[Try[ClaimSet.Claims]](Failure(unauthenticated())) {
+      .fold[Try[ClaimSet.Claims]](Failure(errorFactories.unauthenticatedMissingJwtToken())) {
         case ClaimSet.Unauthenticated =>
-          Failure(unauthenticated())
+          Failure(errorFactories.unauthenticatedMissingJwtToken())
         case claims: ClaimSet.Claims => Success(claims)
       }
   }
@@ -224,7 +231,10 @@ final class Authorizer(
                     scso,
                 )
               case Left(authorizationError) =>
-                observer.onError(permissionDenied(authorizationError.reason))
+                val errorLogger = new DamlContextualizedErrorLogger(logger, loggingContext, None)
+                observer.onError(
+                  errorFactories.permissionDenied(authorizationError.reason)(errorLogger)
+                )
             },
         )
     }
@@ -246,29 +256,11 @@ final class Authorizer(
             authorized(claims) match {
               case Right(_) => call(request)
               case Left(authorizationError) =>
-                Future.failed(permissionDenied(authorizationError.reason))
+                val errorLogger = new DamlContextualizedErrorLogger(logger, loggingContext, None)
+                Future.failed(
+                  errorFactories.permissionDenied(authorizationError.reason)(errorLogger)
+                )
             },
         )
     }
-
-  private def permissionDenied(
-      cause: String
-  )(implicit loggingContext: LoggingContext): StatusRuntimeException =
-    errorCodesVersionSwitcher.choose(
-      v1 = {
-        logger.warn(s"Permission denied. Reason: $cause.")
-        ErrorFactories.permissionDenied()
-      },
-      v2 = LedgerApiErrors.AuthorizationChecks.PermissionDenied
-        .Reject(cause)(new DamlContextualizedErrorLogger(logger, loggingContext, None))
-        .asGrpcError,
-    )
-
-  private def unauthenticated()(implicit loggingContext: LoggingContext): StatusRuntimeException =
-    errorCodesVersionSwitcher.choose(
-      v1 = ErrorFactories.unauthenticated(),
-      v2 = LedgerApiErrors.AuthorizationChecks.Unauthenticated
-        .MissingJwtToken()(new DamlContextualizedErrorLogger(logger, loggingContext, None))
-        .asGrpcError,
-    )
 }
diff --git a/...rc/main/scala/com/digitalasset/ledger/api/auth/interceptor/AuthorizationInterceptor.scala b/...rc/main/scala/com/digitalasset/ledger/api/auth/interceptor/AuthorizationInterceptor.scala
@@ -3,11 +3,10 @@
 
 package com.daml.ledger.api.auth.interceptor
 
-import com.daml.error.ErrorCode.ApiException
-import com.daml.error.definitions.LedgerApiErrors
-import com.daml.error.{DamlErrorCodeLoggingContext, ErrorCodesVersionSwitcher}
+import com.daml.error.{DamlContextualizedErrorLogger, ErrorCodesVersionSwitcher}
 import com.daml.ledger.api.auth.{AuthService, ClaimSet}
 import com.daml.logging.{ContextualizedLogger, LoggingContext}
+import com.daml.platform.server.api.validation.ErrorFactories
 import io.grpc._
 
 import scala.compat.java8.FutureConverters
@@ -20,9 +19,12 @@ import scala.util.{Failure, Success}
 final class AuthorizationInterceptor(
     protected val authService: AuthService,
     ec: ExecutionContext,
-    errorCodesStatusSwitcher: ErrorCodesVersionSwitcher,
-) extends ServerInterceptor {
+    errorCodesVersionSwitcher: ErrorCodesVersionSwitcher,
+)(implicit loggingContext: LoggingContext)
+    extends ServerInterceptor {
   private val logger = ContextualizedLogger.get(getClass)
+  private val errorLogger = new DamlContextualizedErrorLogger(logger, loggingContext, None)
+  private val errorFactories = ErrorFactories(errorCodesVersionSwitcher)
 
   override def interceptCall[ReqT, RespT](
       call: ServerCall[ReqT, RespT],
@@ -42,7 +44,10 @@ final class AuthorizationInterceptor(
         .toScala(authService.decodeMetadata(headers))
         .onComplete {
           case Failure(exception) =>
-            val error = internalAuthenticationError(exception)
+            val error = errorFactories.internalAuthenticationError(
+              securitySafeMessage = "Failed to get claims from request metadata",
+              exception = exception,
+            )(errorLogger)
             call.close(error.getStatus, error.getTrailers)
             new ServerCall.Listener[Nothing]() {}
           case Success(claimSet) =>
@@ -56,26 +61,6 @@ final class AuthorizationInterceptor(
         }(ec)
     }
   }
-
-  private def internalAuthenticationError(
-      exception: Throwable
-  ): StatusRuntimeException =
-    LoggingContext.newLoggingContext { implicit loggingContext: LoggingContext =>
-      errorCodesStatusSwitcher.choose(
-        v1 = {
-          logger.warn(s"Failed to get claims from request metadata: ${exception.getMessage}")
-          new ApiException(
-            status = Status.INTERNAL.withDescription("Failed to get claims from request metadata"),
-            metadata = new Metadata(),
-          )
-        },
-        v2 = LedgerApiErrors.AuthorizationChecks.InternalAuthorizationError
-          .ClaimsFromMetadataExtractionFailed(exception)(
-            new DamlErrorCodeLoggingContext(logger, loggingContext, None)
-          )
-          .asGrpcError,
-      )
-    }
 }
 
 object AuthorizationInterceptor {
@@ -90,5 +75,7 @@ object AuthorizationInterceptor {
       ec: ExecutionContext,
       errorCodesStatusSwitcher: ErrorCodesVersionSwitcher,
   ): AuthorizationInterceptor =
-    new AuthorizationInterceptor(authService, ec, errorCodesStatusSwitcher)
+    LoggingContext.newLoggingContext { implicit loggingContext: LoggingContext =>
+      new AuthorizationInterceptor(authService, ec, errorCodesStatusSwitcher)
+    }
 }
diff --git a/.../src/test/suite/scala/com/digitalasset/ledger/api/auth/AuthorizationInterceptorSpec.scala b/.../src/test/suite/scala/com/digitalasset/ledger/api/auth/AuthorizationInterceptorSpec.scala
@@ -57,7 +57,7 @@ class AuthorizationInterceptorSpec
 
     val errorCodesStatusSwitcher = new ErrorCodesVersionSwitcher(usesSelfServiceErrorCodes)
     val authorizationInterceptor =
-      new AuthorizationInterceptor(authService, global, errorCodesStatusSwitcher)
+      AuthorizationInterceptor(authService, global, errorCodesStatusSwitcher)
 
     val statusCaptor = ArgCaptor[Status]
     val metadataCaptor = ArgCaptor[Metadata]

diff --git a/...ommon/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala b/...ommon/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala
@@ -3,19 +3,20 @@
 
 package com.daml.platform.server.api.validation
 
+import com.daml.error.ErrorCode.ApiException
+import com.daml.platform.server.api.{ApiException => NoStackTraceApiException}
 import com.daml.error.definitions.LedgerApiErrors
 import com.daml.error.{ContextualizedErrorLogger, ErrorCodesVersionSwitcher}
 import com.daml.ledger.api.domain.LedgerId
 import com.daml.ledger.grpc.GrpcStatuses
-import com.daml.platform.server.api.ApiException
 import com.daml.platform.server.api.validation.ErrorFactories.{
   addDefiniteAnswerDetails,
   definiteAnswers,
 }
 import com.google.protobuf.{Any => AnyProto}
 import com.google.rpc.{ErrorInfo, Status}
 import io.grpc.Status.Code
-import io.grpc.StatusRuntimeException
+import io.grpc.{Metadata, StatusRuntimeException}
 import io.grpc.protobuf.StatusProto
 import scalaz.syntax.tag._
 
@@ -167,30 +168,49 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch
   }
 
   // permission denied is intentionally without description to ensure we don't leak security relevant information by accident
-  def permissionDenied()(implicit
+  def permissionDenied(cause: String)(implicit
       contextualizedErrorLogger: ContextualizedErrorLogger
   ): StatusRuntimeException = errorCodesVersionSwitcher.choose(
-    v1 = grpcError(
-      Status
-        .newBuilder()
-        .setCode(Code.PERMISSION_DENIED.value())
-        .build()
-    ),
-    v2 = LedgerApiErrors.AuthorizationChecks.PermissionDenied.Reject().asGrpcError,
+    v1 = {
+      contextualizedErrorLogger.warn(s"Permission denied. Reason: $cause.")
+      new ApiException(
+        io.grpc.Status.PERMISSION_DENIED,
+        new Metadata(),
+      )
+    },
+    v2 = LedgerApiErrors.AuthorizationChecks.PermissionDenied.Reject(cause).asGrpcError,
   )
 
-  def unauthenticated()(implicit
+  def unauthenticatedMissingJwtToken()(implicit
       contextualizedErrorLogger: ContextualizedErrorLogger
   ): StatusRuntimeException = errorCodesVersionSwitcher.choose(
-    v1 = grpcError(
-      Status
-        .newBuilder()
-        .setCode(Code.UNAUTHENTICATED.value())
-        .build()
+    v1 = new ApiException(
+      io.grpc.Status.UNAUTHENTICATED,
+      new Metadata(),
     ),
-    v2 = LedgerApiErrors.AuthorizationChecks.Unauthenticated.Reject().asGrpcError,
+    v2 = LedgerApiErrors.AuthorizationChecks.Unauthenticated
+      .MissingJwtToken()
+      .asGrpcError,
   )
 
+  def internalAuthenticationError(securitySafeMessage: String, exception: Throwable)(implicit
+      contextualizedErrorLogger: ContextualizedErrorLogger
+  ): StatusRuntimeException =
+    errorCodesVersionSwitcher.choose(
+      v1 = {
+        contextualizedErrorLogger.warn(
+          s"$securitySafeMessage: ${exception.getMessage}"
+        )
+        new ApiException(
+          io.grpc.Status.INTERNAL.withDescription(securitySafeMessage),
+          new Metadata(),
+        )
+      },
+      v2 = LedgerApiErrors.AuthorizationChecks.InternalAuthorizationError
+        .ClaimsFromMetadataExtractionFailed(exception)
+        .asGrpcError,
+    )
+
   /** @param definiteAnswer A flag that says whether it is a definite answer. Provided only in the context of command deduplication.
     * @return An exception with the [[Code.UNAVAILABLE]] status code.
     */
@@ -268,7 +288,7 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch
     * @param status A Protobuf [[Status]] object.
     * @return An exception without a stack trace.
     */
-  def grpcError(status: Status): StatusRuntimeException = new ApiException(
+  def grpcError(status: Status): StatusRuntimeException = new NoStackTraceApiException(
     StatusProto.toStatusRuntimeException(status)
   )
 }

diff --git a/...test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala b/...test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala
@@ -43,7 +43,7 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope
     }
 
     "return a permissionDenied error" in {
-      assertVersionedError(_.permissionDenied())(
+      assertVersionedError(_.permissionDenied("some cause"))(
         v1_code = Code.PERMISSION_DENIED,
         v1_message = "",
         v1_details = Seq.empty,
@@ -53,6 +53,30 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope
       )
     }
 
+    "return an unauthenticated error" in {
+      assertVersionedError(_.unauthenticatedMissingJwtToken())(
+        v1_code = Code.UNAUTHENTICATED,
+        v1_message = "",
+        v1_details = Seq.empty,
+        v2_code = Code.UNAUTHENTICATED,
+        v2_message =
+          s"An error occurred. Please contact the operator and inquire about the request $correlationId",
+      )
+    }
+
+    "return an internalAuthenticationError" in {
+      val someSecuritySafeMessage = "nothing security sensitive in here"
+      val someThrowable = new RuntimeException("some internal authentication error")
+      assertVersionedError(_.internalAuthenticationError(someSecuritySafeMessage, someThrowable))(
+        v1_code = Code.INTERNAL,
+        v1_message = someSecuritySafeMessage,
+        v1_details = Seq.empty,
+        v2_code = Code.INTERNAL,
+        v2_message =
+          s"An error occurred. Please contact the operator and inquire about the request $correlationId",
+      )
+    }
+
     "return a missingLedgerConfig error" in {
       val testCases = Table(
         ("definite answer", "expected details"),
@@ -109,17 +133,6 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope
       }
     }
 
-    "return an unauthenticated error" in {
-      assertVersionedError(_.unauthenticated())(
-        v1_code = Code.UNAUTHENTICATED,
-        v1_message = "",
-        v1_details = Seq.empty,
-        v2_code = Code.UNAUTHENTICATED,
-        v2_message =
-          s"An error occurred. Please contact the operator and inquire about the request $correlationId",
-      )
-    }
-
     "return a ledgerIdMismatch error" in {
       val testCases = Table(
         ("definite answer", "expected details"),

diff --git a/...on-api/src/main/scala/platform/apiserver/services/transaction/ApiTransactionService.scala b/...on-api/src/main/scala/platform/apiserver/services/transaction/ApiTransactionService.scala
@@ -6,11 +6,7 @@ package com.daml.platform.apiserver.services.transaction
 import akka.NotUsed
 import akka.stream.Materializer
 import akka.stream.scaladsl.Source
-<<<<<<< HEAD
 import com.daml.error.{DamlContextualizedErrorLogger, ErrorCodesVersionSwitcher}
-=======
-import com.daml.error.{DamlErrorCodeLoggingContext, ErrorCodesVersionSwitcher}
->>>>>>> [Self-service error codes] Implement V2 in Authorizer
 import com.daml.error.definitions.LedgerApiErrors
 import com.daml.grpc.adapter.ExecutionSequencerFactory
 import com.daml.ledger.api.domain.{