From d2eab5e590ea67366fcf6359b5b3fa126598264e Mon Sep 17 00:00:00 2001 From: Enrico Marconi Date: Tue, 28 Nov 2023 11:58:06 +0100 Subject: [PATCH] Slightly more structured error handling --- bindings/grpc/Cargo.toml | 1 + bindings/grpc/src/main.rs | 6 +-- bindings/grpc/src/server.rs | 8 ++-- bindings/grpc/src/services/credential.rs | 40 ++++++++++++------- bindings/grpc/src/services/mod.rs | 4 +- .../tests/api/credential_revocation_check.rs | 28 ++++++++++++- bindings/grpc/tests/api/main.rs | 2 +- 7 files changed, 62 insertions(+), 27 deletions(-) diff --git a/bindings/grpc/Cargo.toml b/bindings/grpc/Cargo.toml index d284954442..23a4a88aa8 100644 --- a/bindings/grpc/Cargo.toml +++ b/bindings/grpc/Cargo.toml @@ -28,6 +28,7 @@ iota-sdk = { version = "1.1.2", features = ["stronghold"] } serde_json = { version = "1.0.108", features = ["alloc"] } thiserror = "1.0.50" rand = "0.8.5" +serde = { version = "1.0.193", features = ["derive", "alloc"] } [build-dependencies] tonic-build = "0.10" diff --git a/bindings/grpc/src/main.rs b/bindings/grpc/src/main.rs index fb00f96919..37eeb353a6 100644 --- a/bindings/grpc/src/main.rs +++ b/bindings/grpc/src/main.rs @@ -5,16 +5,14 @@ const API_ENDPOINT: &str = "http://127.0.0.1:14265"; #[tokio::main] async fn main() -> anyhow::Result<()> { - let client: Client = Client::builder() + let client: Client = Client::builder() .with_primary_node(API_ENDPOINT, None)? .finish() .await?; let addr = "127.0.0.1:50051".parse()?; println!("gRPC server listening on {}", addr); - GRpcServer::new(client) - .serve(addr) - .await?; + GRpcServer::new(client).serve(addr).await?; Ok(()) } diff --git a/bindings/grpc/src/server.rs b/bindings/grpc/src/server.rs index e03544c548..9f1923325d 100644 --- a/bindings/grpc/src/server.rs +++ b/bindings/grpc/src/server.rs @@ -1,7 +1,7 @@ use std::net::SocketAddr; -use tonic::transport::server::{Router, Server}; use iota_sdk::client::Client; +use tonic::transport::server::{Router, Server}; use crate::services; @@ -13,9 +13,7 @@ pub struct GRpcServer { impl GRpcServer { pub fn new(client: Client) -> Self { let router = Server::builder().add_routes(services::routes(client)); - Self { - router, - } + Self { router } } pub async fn serve(self, addr: SocketAddr) -> Result<(), tonic::transport::Error> { self.router.serve(addr).await @@ -23,4 +21,4 @@ impl GRpcServer { pub fn into_router(self) -> Router { self.router } -} \ No newline at end of file +} diff --git a/bindings/grpc/src/services/credential.rs b/bindings/grpc/src/services/credential.rs index 70a85b2d57..f41a05e475 100644 --- a/bindings/grpc/src/services/credential.rs +++ b/bindings/grpc/src/services/credential.rs @@ -7,6 +7,8 @@ use identity_iota::{ prelude::{IotaDocument, Resolver}, }; use iota_sdk::client::Client; +use prost::bytes::Bytes; +use serde::{Deserialize, Serialize}; use thiserror::Error; use tonic::{self, Request, Response}; @@ -46,7 +48,8 @@ mod credential_verification { } } -#[derive(Debug, Error)] +#[derive(Debug, Error, Serialize, Deserialize)] +#[serde(tag = "error_type", content = "reason")] pub enum RevocationCheckError { #[error("Unknown revocation type {0}")] UnknownRevocationType(String), @@ -54,12 +57,12 @@ pub enum RevocationCheckError { InvalidRevocationUrl(String), #[error("Properties isn't a valid JSON object")] MalformedPropertiesObject, - #[error("Invalid credential status")] - InvalidCredentialStatus(#[source] credential::Error), - #[error("Issuer's DID resolution error")] - ResolutionError(#[source] identity_iota::resolver::Error), + #[error("Invalid credential status: {0}")] + InvalidCredentialStatus(String), + #[error("Issuer's DID resolution error: {0}")] + ResolutionError(String), #[error("Revocation map not found")] - RevocationMapNotFound(#[source] credential::JwtValidationError), + RevocationMapNotFound, } impl From for tonic::Status { @@ -67,14 +70,22 @@ impl From for tonic::Status { let message = e.to_string(); let code = match &e { RevocationCheckError::InvalidCredentialStatus(_) - | RevocationCheckError::MalformedPropertiesObject - | RevocationCheckError::UnknownRevocationType(_) - | RevocationCheckError::InvalidRevocationUrl(_) => tonic::Code::InvalidArgument, + | RevocationCheckError::MalformedPropertiesObject + | RevocationCheckError::UnknownRevocationType(_) + | RevocationCheckError::InvalidRevocationUrl(_) => tonic::Code::InvalidArgument, RevocationCheckError::ResolutionError(_) => tonic::Code::Internal, - RevocationCheckError::RevocationMapNotFound(_) => tonic::Code::NotFound, + RevocationCheckError::RevocationMapNotFound => tonic::Code::NotFound, }; + let error_json = serde_json::to_vec(&e).unwrap_or_default(); - tonic::Status::new(code, message) + tonic::Status::with_details(code, message, Bytes::from(error_json)) + } +} + +impl TryFrom for RevocationCheckError { + type Error = (); + fn try_from(value: tonic::Status) -> Result { + serde_json::from_slice(value.details()).map_err(|_| ()) } } @@ -99,14 +110,15 @@ impl CredentialRevocation for CredentialVerifier { ) -> Result, tonic::Status> { let credential_revocation_status = { let revocation_status = credential::Status::try_from(req.into_inner())?; - RevocationBitmapStatus::try_from(revocation_status).map_err(RevocationCheckError::InvalidCredentialStatus)? + RevocationBitmapStatus::try_from(revocation_status) + .map_err(|e| RevocationCheckError::InvalidCredentialStatus(e.to_string()))? }; let issuer_did = credential_revocation_status.id().unwrap(); // Safety: already parsed as a valid URL let issuer_doc = self .resolver .resolve(issuer_did.did()) .await - .map_err(RevocationCheckError::ResolutionError)?; + .map_err(|e| RevocationCheckError::ResolutionError(e.to_string()))?; if let Err(e) = JwtCredentialValidatorUtils::check_revocation_bitmap_status(&issuer_doc, credential_revocation_status) @@ -115,7 +127,7 @@ impl CredentialRevocation for CredentialVerifier { JwtValidationError::Revoked => Ok(Response::new(RevocationCheckResponse { status: RevocationStatus::Revoked.into(), })), - _ => Err(RevocationCheckError::RevocationMapNotFound(e).into()), + _ => Err(RevocationCheckError::RevocationMapNotFound.into()), } } else { Ok(Response::new(RevocationCheckResponse { diff --git a/bindings/grpc/src/services/mod.rs b/bindings/grpc/src/services/mod.rs index 574e4cba3e..8c940c3e0b 100644 --- a/bindings/grpc/src/services/mod.rs +++ b/bindings/grpc/src/services/mod.rs @@ -1,5 +1,5 @@ -mod health_check; -mod credential; +pub mod credential; +pub mod health_check; use iota_sdk::client::Client; use tonic::transport::server::{Routes, RoutesBuilder}; diff --git a/bindings/grpc/tests/api/credential_revocation_check.rs b/bindings/grpc/tests/api/credential_revocation_check.rs index 777233f06c..a752fe6bad 100644 --- a/bindings/grpc/tests/api/credential_revocation_check.rs +++ b/bindings/grpc/tests/api/credential_revocation_check.rs @@ -3,6 +3,7 @@ use identity_iota::{ credential::{self, RevocationBitmap, RevocationBitmapStatus}, did::DID, }; +use serde_json::json; use crate::{ credential_revocation_check::credentials::RevocationCheckRequest, @@ -51,7 +52,6 @@ async fn checking_status_of_credential_works() -> anyhow::Result<()> { .map(|(k, v)| (k, v.to_string().trim_matches('"').to_owned())) .collect(), }; - dbg!(&req); let res = grpc_client.check(tonic::Request::new(req.clone())).await?.into_inner(); assert_eq!(res.status(), RevocationStatus::Valid); @@ -68,3 +68,29 @@ async fn checking_status_of_credential_works() -> anyhow::Result<()> { Ok(()) } + +#[tokio::test] +async fn checking_status_of_valid_but_unresolvable_url_fails() -> anyhow::Result<()> { + use identity_grpc::services::credential::RevocationCheckError; + let server = TestServer::new().await; + + let mut grpc_client = CredentialRevocationClient::connect(server.endpoint()).await?; + let properties = json!({ + "revocationBitmapIndex": "3" + }); + let req = RevocationCheckRequest { + r#type: RevocationBitmap::TYPE.to_owned(), + url: "did:example:1234567890#my-revocation-service".to_owned(), + properties: properties + .as_object() + .unwrap() + .into_iter() + .map(|(k, v)| (k.clone(), v.to_string().trim_matches('"').to_owned())) + .collect(), + }; + let res_error = grpc_client.check(tonic::Request::new(req.clone())).await; + + assert!(res_error.is_err_and(|e| matches!(e.try_into().unwrap(), RevocationCheckError::ResolutionError(_)))); + + Ok(()) +} diff --git a/bindings/grpc/tests/api/main.rs b/bindings/grpc/tests/api/main.rs index f49d97416f..7f7011f804 100644 --- a/bindings/grpc/tests/api/main.rs +++ b/bindings/grpc/tests/api/main.rs @@ -1,3 +1,3 @@ +mod credential_revocation_check; mod health_check; mod helpers; -mod credential_revocation_check; \ No newline at end of file