From b502eb6d08342612b3919d92e908d8304dad5975 Mon Sep 17 00:00:00 2001 From: "Max Kalashnikoff | maksy.eth" Date: Fri, 12 Jul 2024 22:47:56 +0200 Subject: [PATCH] feat(sessions): implementing permission revoking (#699) * feat(sessions): implementing permission revoking * feat: irn operation type to enum * feat: return Ok in case of item not found * fix: operation type as a parameter for add_irn_latency --- integration/sessions.test.ts | 58 ++++++++++++++++++++++++ src/handlers/sessions/context.rs | 9 +++- src/handlers/sessions/create.rs | 9 +++- src/handlers/sessions/get.rs | 9 +++- src/handlers/sessions/list.rs | 7 ++- src/handlers/sessions/mod.rs | 9 ++++ src/handlers/sessions/revoke.rs | 77 ++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/metrics.rs | 5 ++- src/storage/irn/mod.rs | 26 +++++++++++ 10 files changed, 200 insertions(+), 10 deletions(-) create mode 100644 src/handlers/sessions/revoke.rs diff --git a/integration/sessions.test.ts b/integration/sessions.test.ts index abf693607..9cfd53aad 100644 --- a/integration/sessions.test.ts +++ b/integration/sessions.test.ts @@ -15,7 +15,9 @@ describe('Sessions/Permissions', () => { onChainValidated: false } } + // New session PCI let new_pci: string; + // New session signing (private) key let signing_key: string; it('create new session', async () => { @@ -95,4 +97,60 @@ describe('Sessions/Permissions', () => { ) expect(resp.status).toBe(200) }) + + it('revoke PCI permission', async () => { + // Check PCI is exists + let resp = await httpClient.get( + `${baseUrl}/v1/sessions/${address}` + ) + expect(resp.status).toBe(200) + expect(resp.data.pci.length).toBe(1) + expect(resp.data.pci[0]).toBe(new_pci) + + // Create a signature + const privateKey = createPrivateKey({ + key: Buffer.from(signing_key, 'base64'), + format: 'der', + type: 'sec1', + }); + + // Create a bad signature and try to revoke PCI + let bad_signature = createSign('SHA256'); + bad_signature.update('bad_pci'); + bad_signature.end(); + let signature = bad_signature.sign(privateKey, 'base64'); + let payload = { + pci: new_pci, + signature, + } + resp = await httpClient.post( + `${baseUrl}/v1/sessions/${address}/revoke`, + payload + ) + expect(resp.status).toBe(401) + + // Create a good signature and try to revoke PCI + const good_signature = createSign('SHA256'); + good_signature.update(new_pci); + good_signature.end(); + signature = good_signature.sign(privateKey, 'base64'); + payload = { + pci: new_pci, + signature, + } + + // Revoke PCI + resp = await httpClient.post( + `${baseUrl}/v1/sessions/${address}/revoke`, + payload + ) + expect(resp.status).toBe(200) + + // check PCI is revoked + resp = await httpClient.get( + `${baseUrl}/v1/sessions/${address}` + ) + expect(resp.status).toBe(200) + expect(resp.data.pci.length).toBe(0) + }) }) diff --git a/src/handlers/sessions/context.rs b/src/handlers/sessions/context.rs index c328006a9..c3044ddb9 100644 --- a/src/handlers/sessions/context.rs +++ b/src/handlers/sessions/context.rs @@ -3,6 +3,7 @@ use { crate::{ error::RpcError, state::AppState, + storage::irn::OperationType, utils::crypto::{disassemble_caip10, json_canonicalize, verify_ecdsa_signature}, }, axum::{ @@ -41,7 +42,9 @@ async fn handler_internal( .hget(address.clone(), request_payload.pci.clone()) .await? .ok_or_else(|| RpcError::PermissionNotFound(request_payload.pci.clone()))?; - state.metrics.add_irn_latency(irn_call_start, "hget".into()); + state + .metrics + .add_irn_latency(irn_call_start, OperationType::Hget); let mut storage_permissions_item = serde_json::from_str::(&storage_permissions_item)?; @@ -66,7 +69,9 @@ async fn handler_internal( serde_json::to_string(&storage_permissions_item)?.into(), ) .await?; - state.metrics.add_irn_latency(irn_call_start, "hset".into()); + state + .metrics + .add_irn_latency(irn_call_start, OperationType::Hset); Ok(Json(storage_permissions_item).into_response()) } diff --git a/src/handlers/sessions/create.rs b/src/handlers/sessions/create.rs index d9ee78ffd..1b1974c8c 100644 --- a/src/handlers/sessions/create.rs +++ b/src/handlers/sessions/create.rs @@ -1,6 +1,9 @@ use { super::{super::HANDLER_TASK_METRICS, NewPermissionPayload, StoragePermissionsItem}, - crate::{error::RpcError, state::AppState, utils::crypto::disassemble_caip10}, + crate::{ + error::RpcError, state::AppState, storage::irn::OperationType, + utils::crypto::disassemble_caip10, + }, axum::{ extract::{Path, State}, response::{IntoResponse, Response}, @@ -81,7 +84,9 @@ async fn handler_internal( serde_json::to_string(&storage_permissions_item)?.into(), ) .await?; - state.metrics.add_irn_latency(irn_call_start, "hset".into()); + state + .metrics + .add_irn_latency(irn_call_start, OperationType::Hset); let response = NewPermissionResponse { pci, diff --git a/src/handlers/sessions/get.rs b/src/handlers/sessions/get.rs index d5124a3bd..279c0250b 100644 --- a/src/handlers/sessions/get.rs +++ b/src/handlers/sessions/get.rs @@ -1,6 +1,9 @@ use { super::{super::HANDLER_TASK_METRICS, GetPermissionsRequest, StoragePermissionsItem}, - crate::{error::RpcError, state::AppState, utils::crypto::disassemble_caip10}, + crate::{ + error::RpcError, state::AppState, storage::irn::OperationType, + utils::crypto::disassemble_caip10, + }, axum::{ extract::{Path, State}, response::{IntoResponse, Response}, @@ -34,7 +37,9 @@ async fn handler_internal( .hget(request.address.clone(), request.pci.clone()) .await? .ok_or_else(|| RpcError::PermissionNotFound(request.pci))?; - state.metrics.add_irn_latency(irn_call_start, "hget".into()); + state + .metrics + .add_irn_latency(irn_call_start, OperationType::Hget); let storage_permissions_item = serde_json::from_str::(&storage_permissions_item)?; diff --git a/src/handlers/sessions/list.rs b/src/handlers/sessions/list.rs index 5d608dac9..6de2a27fe 100644 --- a/src/handlers/sessions/list.rs +++ b/src/handlers/sessions/list.rs @@ -1,6 +1,9 @@ use { super::super::HANDLER_TASK_METRICS, - crate::{error::RpcError, state::AppState, utils::crypto::disassemble_caip10}, + crate::{ + error::RpcError, state::AppState, storage::irn::OperationType, + utils::crypto::disassemble_caip10, + }, axum::{ extract::{Path, State}, response::{IntoResponse, Response}, @@ -40,7 +43,7 @@ async fn handler_internal( let pci = irn_client.hfields(address).await?; state .metrics - .add_irn_latency(irn_call_start, "hfields".into()); + .add_irn_latency(irn_call_start, OperationType::Hfields); let response = ListPermissionResponse { pci }; Ok(Json(response).into_response()) diff --git a/src/handlers/sessions/mod.rs b/src/handlers/sessions/mod.rs index f51eb8ddd..28320e79e 100644 --- a/src/handlers/sessions/mod.rs +++ b/src/handlers/sessions/mod.rs @@ -4,6 +4,7 @@ pub mod context; pub mod create; pub mod get; pub mod list; +pub mod revoke; /// Payload to create a new permission #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -75,3 +76,11 @@ pub struct StoragePermissionsItem { context: Option, verification_key: String, } + +/// Permission revoke request schema +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PermissionRevokeRequest { + pci: String, + signature: String, +} diff --git a/src/handlers/sessions/revoke.rs b/src/handlers/sessions/revoke.rs new file mode 100644 index 000000000..4b45e3f17 --- /dev/null +++ b/src/handlers/sessions/revoke.rs @@ -0,0 +1,77 @@ +use { + super::{super::HANDLER_TASK_METRICS, PermissionRevokeRequest, StoragePermissionsItem}, + crate::{ + error::RpcError, + state::AppState, + storage::irn::OperationType, + utils::crypto::{disassemble_caip10, verify_ecdsa_signature}, + }, + axum::{ + extract::{Path, State}, + response::Response, + Json, + }, + std::{sync::Arc, time::SystemTime}, + tracing::warn, + wc::future::FutureExt, +}; + +pub async fn handler( + state: State>, + address: Path, + Json(request_payload): Json, +) -> Result { + handler_internal(state, address, request_payload) + .with_metrics(HANDLER_TASK_METRICS.with_name("sessions_revoke")) + .await +} + +#[tracing::instrument(skip(state), level = "debug")] +async fn handler_internal( + state: State>, + Path(address): Path, + request_payload: PermissionRevokeRequest, +) -> Result { + let irn_client = state.irn.as_ref().ok_or(RpcError::IrnNotConfigured)?; + + // Checking the CAIP-10 address format + disassemble_caip10(&address)?; + + // Get the PCI object from the IRN + let irn_call_start = SystemTime::now(); + let irn_client_result = irn_client + .hget(address.clone(), request_payload.pci.clone()) + .await?; + state + .metrics + .add_irn_latency(irn_call_start, OperationType::Hget); + let storage_permissions_item = match irn_client_result { + Some(item) => item, + // Return Success if the item is not found for idempotency + None => { + warn!( + "Permission item not found in the storage for address: {} and PCI: {}", + address, request_payload.pci + ); + return Ok(Response::default()); + } + }; + let storage_permissions_item = + serde_json::from_str::(&storage_permissions_item)?; + + // Check the signature + verify_ecdsa_signature( + &request_payload.pci, + &request_payload.signature, + &storage_permissions_item.verification_key, + )?; + + // Remove the session/permission item from the IRN + let irn_call_start = SystemTime::now(); + irn_client.hdel(address, request_payload.pci).await?; + state + .metrics + .add_irn_latency(irn_call_start, OperationType::Hdel); + + Ok(Response::default()) +} diff --git a/src/lib.rs b/src/lib.rs index 99b152513..3e24a5020 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -339,6 +339,7 @@ pub async fn bootstrap(config: Config) -> RpcResult<()> { .route("/v1/sessions/:address", get(handlers::sessions::list::handler)) .route("/v1/sessions/:address/:pci", get(handlers::sessions::get::handler)) .route("/v1/sessions/:address/context", post(handlers::sessions::context::handler)) + .route("/v1/sessions/:address/revoke", post(handlers::sessions::revoke::handler)) // Health .route("/health", get(handlers::health::handler)) .route_layer(tracing_and_metrics_layer) diff --git a/src/metrics.rs b/src/metrics.rs index aa5968e50..724aa4c55 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -3,6 +3,7 @@ use { database::helpers::get_account_names_stats, handlers::identity::IdentityLookupSource, providers::{ProviderKind, RpcProvider}, + storage::irn::OperationType, }, hyper::http, sqlx::PgPool, @@ -537,14 +538,14 @@ impl Metrics { .record(&otel::Context::new(), entry, &[]); } - pub fn add_irn_latency(&self, start: SystemTime, operation: String) { + pub fn add_irn_latency(&self, start: SystemTime, operation: OperationType) { self.irn_latency_tracker.record( &otel::Context::new(), start .elapsed() .unwrap_or(Duration::from_secs(0)) .as_secs_f64(), - &[otel::KeyValue::new("operation", operation)], + &[otel::KeyValue::new("operation", operation.as_str())], ); } diff --git a/src/storage/irn/mod.rs b/src/storage/irn/mod.rs index 6637d2fe2..ed5bcdf5e 100644 --- a/src/storage/irn/mod.rs +++ b/src/storage/irn/mod.rs @@ -15,6 +15,32 @@ const CONNECTION_TIMEOUT: Duration = Duration::from_secs(3); const RECORDS_TTL: Duration = Duration::from_secs(60 * 60 * 24 * 30); // 30 days const UDP_SOCKET_COUNT: usize = 1; +/// IRN storage operation type +#[derive(Debug)] +pub enum OperationType { + Hset, + Hget, + Hfields, + Hdel, +} + +impl OperationType { + pub fn as_str(&self) -> &'static str { + match self { + OperationType::Hset => "hset", + OperationType::Hget => "hget", + OperationType::Hfields => "hfields", + OperationType::Hdel => "hdel", + } + } +} + +impl From for String { + fn from(value: OperationType) -> Self { + value.as_str().to_string() + } +} + #[derive(Debug, Clone, Deserialize, Eq, PartialEq)] pub struct Config { pub node: Option,