Add detailed logging to public accounts service

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/AccountsService.kt

@@ -88,15 +88,12 @@ class AccountsService(
     val result =
       try {
         internalAccountsStub.createAccount(internalCreateAccountRequest)
-      } catch (ex: StatusException) {
-        when (ex.status.code) {
-          Status.Code.NOT_FOUND -> failGrpc(Status.NOT_FOUND, ex) { "Creator's Account not found." }
-          Status.Code.PERMISSION_DENIED ->
-            failGrpc(Status.PERMISSION_DENIED, ex) {
-              "Caller does not own the owned measurement consumer."
-            }
-          else -> failGrpc(Status.UNKNOWN, ex) { "Unknown exception." }
-        }
+      } catch (e: StatusException) {
+        throw when (e.status.code) {
+          Status.Code.NOT_FOUND -> Status.NOT_FOUND
+          Status.Code.PERMISSION_DENIED -> Status.PERMISSION_DENIED
+          else -> Status.UNKNOWN
+        }.toExternalStatusRuntimeException(e)
       }
 
     return result.toAccount()
@@ -135,18 +132,13 @@ class AccountsService(
     val result =
       try {
         internalAccountsStub.activateAccount(internalActivateAccountRequest)
-      } catch (ex: StatusException) {
-        when (ex.status.code) {
-          Status.Code.PERMISSION_DENIED ->
-            failGrpc(Status.PERMISSION_DENIED, ex) {
-              "Activation token is not valid for this account."
-            }
-          Status.Code.FAILED_PRECONDITION ->
-            failGrpc(Status.FAILED_PRECONDITION, ex) { ex.message ?: "Failed precondition." }
-          Status.Code.NOT_FOUND ->
-            failGrpc(Status.NOT_FOUND, ex) { "Account to activate has not been found." }
-          else -> failGrpc(Status.UNKNOWN, ex) { "Unknown exception." }
-        }
+      } catch (e: StatusException) {
+        throw when (e.status.code) {
+          Status.Code.PERMISSION_DENIED -> Status.PERMISSION_DENIED
+          Status.Code.FAILED_PRECONDITION -> Status.FAILED_PRECONDITION
+          Status.Code.NOT_FOUND -> Status.NOT_FOUND
+          else -> Status.UNKNOWN
+        }.toExternalStatusRuntimeException(e)
       }
 
     // method only returns the basic account view so some fields are cleared
@@ -182,13 +174,12 @@ class AccountsService(
     val result =
       try {
         internalAccountsStub.replaceAccountIdentity(internalReplaceAccountIdentityRequest)
-      } catch (ex: StatusException) {
-        when (ex.status.code) {
-          Status.Code.FAILED_PRECONDITION ->
-            failGrpc(Status.FAILED_PRECONDITION, ex) { ex.message ?: "Failed precondition." }
-          Status.Code.NOT_FOUND -> failGrpc(Status.NOT_FOUND, ex) { "Account was not found." }
-          else -> failGrpc(Status.UNKNOWN, ex) { "Unknown exception." }
-        }
+      } catch (e: StatusException) {
+        throw when (e.status.code) {
+          Status.Code.FAILED_PRECONDITION -> Status.FAILED_PRECONDITION
+          Status.Code.NOT_FOUND -> Status.NOT_FOUND
+          else -> Status.UNKNOWN
+        }.toExternalStatusRuntimeException(e)
       }
 
     // method only returns the basic account view so some fields are cleared

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/BUILD.bazel

@@ -34,6 +34,7 @@ kt_jvm_library(
     deps = [
         "//src/main/kotlin/org/wfanet/measurement/api:account_constants",
         "//src/main/kotlin/org/wfanet/measurement/api/v2alpha:resource_key",
+        "//src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha:internal_status_conversion",
         "//src/main/proto/wfa/measurement/api/v2alpha:account_kt_jvm_proto",
         "//src/main/proto/wfa/measurement/api/v2alpha:accounts_service_kt_jvm_grpc_proto",
         "//src/main/proto/wfa/measurement/internal/kingdom:account_kt_jvm_proto",

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/InternalStatusConversion.kt

@@ -23,6 +23,7 @@ import io.grpc.Status
 import io.grpc.StatusException
 import io.grpc.StatusRuntimeException
 import io.grpc.protobuf.StatusProto
+import org.wfanet.measurement.api.v2alpha.AccountKey
 import org.wfanet.measurement.api.v2alpha.DataProviderCertificateKey
 import org.wfanet.measurement.api.v2alpha.DataProviderKey
 import org.wfanet.measurement.api.v2alpha.DuchyCertificateKey
@@ -195,13 +196,46 @@ fun Status.toExternalStatusRuntimeException(
           errorMessage = "Requisition state illegal."
         }
         ErrorCode.ACCOUNT_NOT_FOUND -> {
-          errorMessage = "Account not found."
+          val accountName =
+            AccountKey(
+                externalIdToApiId(
+                  checkNotNull(errorInfo.metadataMap["external_account_id"]).toLong()
+                )
+              )
+              .toName()
+          put("account", accountName)
+          errorMessage = "Account $accountName not found."
         }
         ErrorCode.DUPLICATE_ACCOUNT_IDENTITY -> {
-          errorMessage = "Duplicated account identity."
+          val accountName =
+            AccountKey(
+                externalIdToApiId(
+                  checkNotNull(errorInfo.metadataMap["external_account_id"]).toLong()
+                )
+              )
+              .toName()
+          val issuer = checkNotNull(errorInfo.metadataMap["issuer"])
+          val subject = checkNotNull(errorInfo.metadataMap["subject"])
+          put("account", accountName)
+          put("issuer", issuer)
+          put("subject", subject)
+          errorMessage =
+            "Account $accountName with issuer: $issuer and subject: $subject pair already exists."
         }
         ErrorCode.ACCOUNT_ACTIVATION_STATE_ILLEGAL -> {
-          errorMessage = "Account activation state illegal."
+          val accountName =
+            AccountKey(
+                externalIdToApiId(
+                  checkNotNull(errorInfo.metadataMap["external_account_id"]).toLong()
+                )
+              )
+              .toName()
+          val accountActivationState =
+            checkNotNull(errorInfo.metadataMap["account_activation_state"]).toString()
+          put("account", accountName)
+          put("account_activation_state", accountActivationState)
+          errorMessage =
+            "Account $accountName is in illegal activation state: $accountActivationState."
         }
         ErrorCode.PERMISSION_DENIED -> {
           errorMessage = "Permission Denied."

src/test/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/AccountsServiceTest.kt

@@ -30,6 +30,7 @@ import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
 import org.mockito.kotlin.any
+import org.mockito.kotlin.stub
 import org.mockito.kotlin.whenever
 import org.wfanet.measurement.api.v2alpha.Account
 import org.wfanet.measurement.api.v2alpha.AccountKt
@@ -44,8 +45,10 @@ import org.wfanet.measurement.api.v2alpha.createAccountRequest
 import org.wfanet.measurement.api.v2alpha.replaceAccountIdentityRequest
 import org.wfanet.measurement.api.withIdToken
 import org.wfanet.measurement.common.crypto.tink.SelfIssuedIdTokens
+import org.wfanet.measurement.common.grpc.errorInfo
 import org.wfanet.measurement.common.grpc.testing.GrpcTestServerRule
 import org.wfanet.measurement.common.grpc.testing.mockService
+import org.wfanet.measurement.common.identity.ExternalId
 import org.wfanet.measurement.common.identity.apiIdToExternalId
 import org.wfanet.measurement.common.identity.externalIdToApiId
 import org.wfanet.measurement.common.testing.verifyProtoArgument
@@ -59,6 +62,10 @@ import org.wfanet.measurement.internal.kingdom.activateAccountRequest as interna
 import org.wfanet.measurement.internal.kingdom.copy
 import org.wfanet.measurement.internal.kingdom.openIdRequestParams
 import org.wfanet.measurement.internal.kingdom.replaceAccountIdentityRequest as internalReplaceAccountIdentityRequest
+import org.wfanet.measurement.kingdom.deploy.gcloud.spanner.common.AccountActivationStateIllegalException
+import org.wfanet.measurement.kingdom.deploy.gcloud.spanner.common.AccountNotFoundException
+import org.wfanet.measurement.kingdom.deploy.gcloud.spanner.common.DuplicateAccountIdentityException
+import org.wfanet.measurement.kingdom.deploy.gcloud.spanner.common.PermissionDeniedException
 
 private const val ACTIVATION_TOKEN = 12345672L
 
@@ -420,6 +427,193 @@ class AccountsServiceTest {
     }
   }
 
+  @Test
+  fun `createAccount throws NOT_FOUND with account name when account is not found`() {
+    internalAccountsMock.stub {
+      onBlocking { createAccount(any()) }
+        .thenThrow(
+          AccountNotFoundException(ExternalId(EXTERNAL_ACCOUNT_ID))
+            .asStatusRuntimeException(Status.Code.NOT_FOUND, "Account not found.")
+        )
+    }
+    val request = createAccountRequest {
+      account = account {
+        activationParams =
+          AccountKt.activationParams { ownedMeasurementConsumer = MEASUREMENT_CONSUMER_NAME }
+      }
+    }
+    val exception =
+      assertFailsWith<StatusException> {
+        runBlocking { client.withIdToken(generateIdToken()).createAccount(request) }
+      }
+    assertThat(exception.status.code).isEqualTo(Status.Code.NOT_FOUND)
+    assertThat(exception.errorInfo?.metadataMap).containsEntry("account", ACCOUNT_NAME)
+  }
+
+  @Test
+  fun `createAccount throws PERMISSION_DENIED when caller does not own measurement consumer`() {
+    internalAccountsMock.stub {
+      onBlocking { createAccount(any()) }
+        .thenThrow(
+          PermissionDeniedException()
+            .asStatusRuntimeException(Status.Code.PERMISSION_DENIED, "Permission Denied.")
+        )
+    }
+    val request = createAccountRequest {
+      account = account {
+        activationParams =
+          AccountKt.activationParams {
+            ownedMeasurementConsumer = "measurementConsumers/BBBBBBBBBHs"
+          }
+      }
+    }
+    val exception =
+      assertFailsWith<StatusException> {
+        runBlocking { client.withIdToken(generateIdToken()).createAccount(request) }
+      }
+    assertThat(exception.status.code).isEqualTo(Status.Code.PERMISSION_DENIED)
+  }
+
+  @Test
+  fun `activateAccount throws PERMISSION_DENIED activation token is not valid for account`() {
+    internalAccountsMock.stub {
+      onBlocking { activateAccount(any()) }
+        .thenThrow(
+          PermissionDeniedException()
+            .asStatusRuntimeException(Status.Code.PERMISSION_DENIED, "Permission Denied.")
+        )
+    }
+    val exception =
+      assertFailsWith<StatusException> {
+        runBlocking {
+          client
+            .withIdToken(generateIdToken())
+            .activateAccount(
+              activateAccountRequest {
+                name = ACCOUNT_NAME
+                activationToken = externalIdToApiId(67896789L)
+              }
+            )
+        }
+      }
+
+    assertThat(exception.status.code).isEqualTo(Status.Code.PERMISSION_DENIED)
+  }
+
+  @Test
+  fun `activateAccount throws FAILED_PRECONDITION with account name and activation state when account activation state is illegal`() {
+    internalAccountsMock.stub {
+      onBlocking { activateAccount(any()) }
+        .thenThrow(
+          AccountActivationStateIllegalException(
+              ExternalId(EXTERNAL_ACCOUNT_ID),
+              InternalAccount.ActivationState.ACTIVATED,
+            )
+            .asStatusRuntimeException(Status.Code.PERMISSION_DENIED, "Permission Denied.")
+        )
+    }
+    val exception =
+      assertFailsWith<StatusException> {
+        runBlocking {
+          client
+            .withIdToken(generateIdToken())
+            .activateAccount(
+              activateAccountRequest {
+                name = ACCOUNT_NAME
+                activationToken = externalIdToApiId(ACTIVATION_TOKEN)
+              }
+            )
+        }
+      }
+
+    assertThat(exception.status.code).isEqualTo(Status.Code.PERMISSION_DENIED)
+    assertThat(exception.errorInfo?.metadataMap).containsEntry("account", ACCOUNT_NAME)
+  }
+
+  @Test
+  fun `activateAccount throws NOT_FOUND with account name when account is not found`() {
+    internalAccountsMock.stub {
+      onBlocking { activateAccount(any()) }
+        .thenThrow(
+          AccountNotFoundException(ExternalId(EXTERNAL_ACCOUNT_ID))
+            .asStatusRuntimeException(Status.Code.NOT_FOUND, "Account has not been found.")
+        )
+    }
+    val exception =
+      assertFailsWith<StatusException> {
+        runBlocking {
+          client
+            .withIdToken(generateIdToken())
+            .activateAccount(
+              activateAccountRequest {
+                name = ACCOUNT_NAME
+                activationToken = externalIdToApiId(ACTIVATION_TOKEN)
+              }
+            )
+        }
+      }
+
+    assertThat(exception.status.code).isEqualTo(Status.Code.NOT_FOUND)
+    assertThat(exception.errorInfo?.metadataMap).containsEntry("account", ACCOUNT_NAME)
+  }
+
+  @Test
+  fun `replaceAccountIdentity throws FAILED_PRECONDITION when duplicate account identity is found`() =
+    runBlocking {
+      internalAccountsMock.stub {
+        onBlocking { replaceAccountIdentity(any()) }
+          .thenThrow(
+            DuplicateAccountIdentityException(ExternalId(EXTERNAL_ACCOUNT_ID), ISSUER, SUBJECT)
+              .asStatusRuntimeException(
+                Status.Code.FAILED_PRECONDITION,
+                "Issuer and subject pair already exists.",
+              )
+          )
+      }
+      val request = replaceAccountIdentityRequest {
+        name = ACCOUNT_NAME
+        openId =
+          ReplaceAccountIdentityRequestKt.openIdConnectCredentials {
+            identityBearerToken = generateIdToken()
+          }
+      }
+      val exception =
+        assertFailsWith<StatusException> {
+          runBlocking { client.withIdToken(generateIdToken()).replaceAccountIdentity(request) }
+        }
+
+      assertThat(exception.status.code).isEqualTo(Status.Code.FAILED_PRECONDITION)
+      assertThat(exception.errorInfo?.metadataMap).containsEntry("account", ACCOUNT_NAME)
+      assertThat(exception.errorInfo?.metadataMap).containsEntry("subject", SUBJECT)
+      assertThat(exception.errorInfo?.metadataMap).containsEntry("issuer", ISSUER)
+    }
+
+  @Test
+  fun `replaceAccountIdentity throws NOT_FOUND with account name when account is not found`() =
+    runBlocking {
+      internalAccountsMock.stub {
+        onBlocking { replaceAccountIdentity(any()) }
+          .thenThrow(
+            AccountNotFoundException(ExternalId(EXTERNAL_ACCOUNT_ID))
+              .asStatusRuntimeException(Status.Code.NOT_FOUND, "Account was not found.")
+          )
+      }
+      val request = replaceAccountIdentityRequest {
+        name = ACCOUNT_NAME
+        openId =
+          ReplaceAccountIdentityRequestKt.openIdConnectCredentials {
+            identityBearerToken = generateIdToken()
+          }
+      }
+      val exception =
+        assertFailsWith<StatusException> {
+          runBlocking { client.withIdToken(generateIdToken()).replaceAccountIdentity(request) }
+        }
+
+      assertThat(exception.status.code).isEqualTo(Status.Code.NOT_FOUND)
+      assertThat(exception.errorInfo?.metadataMap).containsEntry("account", ACCOUNT_NAME)
+    }
+
   private suspend fun generateIdToken(): String {
     val uriString =
       client

src/test/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/BUILD.bazel

@@ -8,6 +8,7 @@ kt_jvm_test(
     ],
     test_class = "org.wfanet.measurement.kingdom.service.api.v2alpha.AccountsServiceTest",
     deps = [
+        "//src/main/kotlin/org/wfanet/measurement/kingdom/deploy/gcloud/spanner/common",
         "//src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha:account_authentication_server_interceptor",
         "@wfa_common_jvm//imports/java/com/google/common/truth",
         "@wfa_common_jvm//imports/java/com/google/common/truth/extensions/proto",