From fd5c3cfbf57dfa7a611b6f6174f2c4da08a6f17a Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Mon, 1 Jan 2024 14:08:28 +0900 Subject: [PATCH] chore: refactor magic values --- src/notify_message.rs | 2 +- .../handlers/notify_delete.rs | 9 ++-- .../handlers/notify_subscribe.rs | 22 +++++++--- .../handlers/notify_update.rs | 9 ++-- .../handlers/notify_watch_subscriptions.rs | 13 +++--- src/spec.rs | 1 + tests/deployment.rs | 41 ++++++++++--------- 7 files changed, 59 insertions(+), 38 deletions(-) diff --git a/src/notify_message.rs b/src/notify_message.rs index 6a86cf6a..e734a7ba 100644 --- a/src/notify_message.rs +++ b/src/notify_message.rs @@ -34,7 +34,7 @@ pub fn sign_message( exp: add_ttl(now, NOTIFY_MESSAGE_TTL).timestamp(), iss: decoded_client_id.to_did_key(), // no `aud` because any client can receive this message - act: NOTIFY_MESSAGE_ACT.to_string(), + act: NOTIFY_MESSAGE_ACT.to_owned(), sub: account.to_did_pkh(), app: app.clone(), msg, diff --git a/src/services/websocket_server/handlers/notify_delete.rs b/src/services/websocket_server/handlers/notify_delete.rs index ced0d301..f247aed9 100644 --- a/src/services/websocket_server/handlers/notify_delete.rs +++ b/src/services/websocket_server/handlers/notify_delete.rs @@ -15,7 +15,10 @@ use { handlers::{decrypt_message, notify_watch_subscriptions::update_subscription_watchers}, NotifyDelete, NotifyRequest, NotifyResponse, ResponseAuth, }, - spec::{NOTIFY_DELETE_RESPONSE_TAG, NOTIFY_DELETE_RESPONSE_TTL}, + spec::{ + NOTIFY_DELETE_ACT, NOTIFY_DELETE_RESPONSE_ACT, NOTIFY_DELETE_RESPONSE_TAG, + NOTIFY_DELETE_RESPONSE_TTL, + }, state::{AppState, WebhookNotificationEvent}, types::{Envelope, EnvelopeType0}, utils::topic_from_key, @@ -71,7 +74,7 @@ pub async fn handle(msg: PublishedMessage, state: &AppState, client: &Client) -> } let (account, siwe_domain) = { - if sub_auth.shared_claims.act != "notify_delete" { + if sub_auth.shared_claims.act != NOTIFY_DELETE_ACT { return Err(AuthError::InvalidAct)?; } @@ -137,7 +140,7 @@ pub async fn handle(msg: PublishedMessage, state: &AppState, client: &Client) -> exp: add_ttl(now, NOTIFY_DELETE_RESPONSE_TTL).timestamp() as u64, iss: identity.to_did_key(), aud: sub_auth.shared_claims.iss, - act: "notify_delete_response".to_string(), + act: NOTIFY_DELETE_RESPONSE_ACT.to_owned(), mjv: "1".to_owned(), }, sub: account.to_did_pkh(), diff --git a/src/services/websocket_server/handlers/notify_subscribe.rs b/src/services/websocket_server/handlers/notify_subscribe.rs index 90915511..8137d78e 100644 --- a/src/services/websocket_server/handlers/notify_subscribe.rs +++ b/src/services/websocket_server/handlers/notify_subscribe.rs @@ -15,7 +15,10 @@ use { handlers::{decrypt_message, notify_watch_subscriptions::update_subscription_watchers}, NotifyRequest, NotifyResponse, NotifySubscribe, ResponseAuth, }, - spec::{NOTIFY_NOOP_TAG, NOTIFY_SUBSCRIBE_RESPONSE_TAG, NOTIFY_SUBSCRIBE_RESPONSE_TTL}, + spec::{ + NOTIFY_NOOP_TAG, NOTIFY_NOOP_TTL, NOTIFY_SUBSCRIBE_ACT, NOTIFY_SUBSCRIBE_RESPONSE_ACT, + NOTIFY_SUBSCRIBE_RESPONSE_TAG, NOTIFY_SUBSCRIBE_RESPONSE_TTL, + }, state::{AppState, WebhookNotificationEvent}, types::{parse_scope, Envelope, EnvelopeType0, EnvelopeType1}, utils::topic_from_key, @@ -28,7 +31,10 @@ use { domain::{DecodedClientId, Topic}, rpc::Publish, }, - std::{collections::HashSet, sync::Arc}, + std::{ + collections::HashSet, + sync::{Arc, OnceLock}, + }, tracing::{info, instrument}, x25519_dalek::{PublicKey, StaticSecret}, }; @@ -83,7 +89,7 @@ pub async fn handle(msg: PublishedMessage, state: &AppState) -> Result<()> { } let (account, siwe_domain) = { - if sub_auth.shared_claims.act != "notify_subscription" { + if sub_auth.shared_claims.act != NOTIFY_SUBSCRIBE_ACT { return Err(AuthError::InvalidAct)?; } @@ -125,7 +131,7 @@ pub async fn handle(msg: PublishedMessage, state: &AppState) -> Result<()> { exp: add_ttl(now, NOTIFY_SUBSCRIBE_RESPONSE_TTL).timestamp() as u64, iss: identity.to_did_key(), aud: sub_auth.shared_claims.iss.clone(), - act: "notify_subscription_response".to_string(), + act: NOTIFY_SUBSCRIBE_RESPONSE_ACT.to_owned(), mjv: "1".to_owned(), }, sub: account.to_did_pkh(), @@ -206,9 +212,13 @@ pub async fn handle(msg: PublishedMessage, state: &AppState) -> Result<()> { &state.relay_http_client, &Publish { topic: notify_topic, - message: "".into(), + message: { + // Extremely minor performance optimization with OnceLock to avoid allocating the same empty string everytime + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| "".into()).clone() + }, tag: NOTIFY_NOOP_TAG, - ttl_secs: 300, + ttl_secs: NOTIFY_NOOP_TTL.as_secs() as u32, prompt: false, }, state.metrics.as_ref(), diff --git a/src/services/websocket_server/handlers/notify_update.rs b/src/services/websocket_server/handlers/notify_update.rs index fc09ad12..8f2a4bbb 100644 --- a/src/services/websocket_server/handlers/notify_update.rs +++ b/src/services/websocket_server/handlers/notify_update.rs @@ -15,7 +15,10 @@ use { decode_key, handlers::decrypt_message, NotifyRequest, NotifyResponse, NotifyUpdate, ResponseAuth, }, - spec::{NOTIFY_UPDATE_RESPONSE_TAG, NOTIFY_UPDATE_RESPONSE_TTL}, + spec::{ + NOTIFY_UPDATE_ACT, NOTIFY_UPDATE_RESPONSE_ACT, NOTIFY_UPDATE_RESPONSE_TAG, + NOTIFY_UPDATE_RESPONSE_TTL, + }, state::AppState, types::{parse_scope, Envelope, EnvelopeType0}, utils::topic_from_key, @@ -70,7 +73,7 @@ pub async fn handle(msg: PublishedMessage, state: &AppState) -> Result<()> { } let (account, siwe_domain) = { - if sub_auth.shared_claims.act != "notify_update" { + if sub_auth.shared_claims.act != NOTIFY_UPDATE_ACT { return Err(AuthError::InvalidAct)?; } @@ -142,7 +145,7 @@ pub async fn handle(msg: PublishedMessage, state: &AppState) -> Result<()> { exp: add_ttl(now, NOTIFY_UPDATE_RESPONSE_TTL).timestamp() as u64, iss: identity.to_did_key(), aud: sub_auth.shared_claims.iss, - act: "notify_update_response".to_string(), + act: NOTIFY_UPDATE_RESPONSE_ACT.to_owned(), mjv: "1".to_owned(), }, sub: account.to_did_pkh(), diff --git a/src/services/websocket_server/handlers/notify_watch_subscriptions.rs b/src/services/websocket_server/handlers/notify_watch_subscriptions.rs index 8e2c34d1..6b3414c7 100644 --- a/src/services/websocket_server/handlers/notify_watch_subscriptions.rs +++ b/src/services/websocket_server/handlers/notify_watch_subscriptions.rs @@ -23,9 +23,10 @@ use { NotifySubscriptionsChanged, NotifyWatchSubscriptions, }, spec::{ - NOTIFY_SUBSCRIPTIONS_CHANGED_METHOD, NOTIFY_SUBSCRIPTIONS_CHANGED_TAG, - NOTIFY_SUBSCRIPTIONS_CHANGED_TTL, NOTIFY_WATCH_SUBSCRIPTIONS_RESPONSE_TAG, - NOTIFY_WATCH_SUBSCRIPTIONS_RESPONSE_TTL, + NOTIFY_SUBSCRIPTIONS_CHANGED_ACT, NOTIFY_SUBSCRIPTIONS_CHANGED_METHOD, + NOTIFY_SUBSCRIPTIONS_CHANGED_TAG, NOTIFY_SUBSCRIPTIONS_CHANGED_TTL, + NOTIFY_WATCH_SUBSCRIPTIONS_ACT, NOTIFY_WATCH_SUBSCRIPTIONS_RESPONSE_ACT, + NOTIFY_WATCH_SUBSCRIPTIONS_RESPONSE_TAG, NOTIFY_WATCH_SUBSCRIPTIONS_RESPONSE_TTL, }, state::AppState, types::{Envelope, EnvelopeType0, EnvelopeType1}, @@ -80,7 +81,7 @@ pub async fn handle(msg: PublishedMessage, state: &AppState) -> Result<()> { // Verify request let authorization = { - if request_auth.shared_claims.act != "notify_watch_subscriptions" { + if request_auth.shared_claims.act != NOTIFY_WATCH_SUBSCRIPTIONS_ACT { return Err(AuthError::InvalidAct)?; } @@ -151,7 +152,7 @@ pub async fn handle(msg: PublishedMessage, state: &AppState) -> Result<()> { exp: add_ttl(now, NOTIFY_WATCH_SUBSCRIPTIONS_RESPONSE_TTL).timestamp() as u64, iss: identity.to_did_key(), aud: request_auth.shared_claims.iss, - act: "notify_watch_subscriptions_response".to_string(), + act: NOTIFY_WATCH_SUBSCRIPTIONS_RESPONSE_ACT.to_owned(), mjv: "1".to_owned(), }, sub: request_auth.sub, @@ -287,7 +288,7 @@ pub async fn update_subscription_watchers( exp: add_ttl(now, NOTIFY_SUBSCRIPTIONS_CHANGED_TTL).timestamp() as u64, iss: notify_did_key.clone(), aud, - act: "notify_subscriptions_changed".to_string(), + act: NOTIFY_SUBSCRIPTIONS_CHANGED_ACT.to_owned(), mjv: "1".to_owned(), }, sub: did_pkh, diff --git a/src/spec.rs b/src/spec.rs index 90b69e4a..bb478947 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -42,6 +42,7 @@ pub const NOTIFY_WATCH_SUBSCRIPTIONS_TTL: Duration = T300; pub const NOTIFY_WATCH_SUBSCRIPTIONS_RESPONSE_TTL: Duration = T300; pub const NOTIFY_SUBSCRIPTIONS_CHANGED_TTL: Duration = T300; pub const NOTIFY_SUBSCRIPTIONS_CHANGED_RESPONSE_TTL: Duration = T300; +pub const NOTIFY_NOOP_TTL: Duration = T300; // acts // https://specs.walletconnect.com/2.0/specs/clients/notify/notify-authentication diff --git a/tests/deployment.rs b/tests/deployment.rs index 27289067..543d3fef 100644 --- a/tests/deployment.rs +++ b/tests/deployment.rs @@ -32,13 +32,16 @@ use { }, }, spec::{ - NOTIFY_DELETE_METHOD, NOTIFY_DELETE_RESPONSE_TAG, NOTIFY_DELETE_TAG, NOTIFY_DELETE_TTL, - NOTIFY_MESSAGE_TAG, NOTIFY_NOOP_TAG, NOTIFY_SUBSCRIBE_METHOD, - NOTIFY_SUBSCRIBE_RESPONSE_TAG, NOTIFY_SUBSCRIBE_TAG, NOTIFY_SUBSCRIBE_TTL, - NOTIFY_SUBSCRIPTIONS_CHANGED_TAG, NOTIFY_UPDATE_METHOD, NOTIFY_UPDATE_RESPONSE_TAG, - NOTIFY_UPDATE_TAG, NOTIFY_UPDATE_TTL, NOTIFY_WATCH_SUBSCRIPTIONS_METHOD, - NOTIFY_WATCH_SUBSCRIPTIONS_RESPONSE_TAG, NOTIFY_WATCH_SUBSCRIPTIONS_TAG, - NOTIFY_WATCH_SUBSCRIPTIONS_TTL, + NOTIFY_DELETE_ACT, NOTIFY_DELETE_METHOD, NOTIFY_DELETE_RESPONSE_ACT, + NOTIFY_DELETE_RESPONSE_TAG, NOTIFY_DELETE_TAG, NOTIFY_DELETE_TTL, NOTIFY_MESSAGE_ACT, + NOTIFY_MESSAGE_TAG, NOTIFY_NOOP_TAG, NOTIFY_SUBSCRIBE_ACT, NOTIFY_SUBSCRIBE_METHOD, + NOTIFY_SUBSCRIBE_RESPONSE_ACT, NOTIFY_SUBSCRIBE_RESPONSE_TAG, NOTIFY_SUBSCRIBE_TAG, + NOTIFY_SUBSCRIBE_TTL, NOTIFY_SUBSCRIPTIONS_CHANGED_ACT, + NOTIFY_SUBSCRIPTIONS_CHANGED_TAG, NOTIFY_UPDATE_ACT, NOTIFY_UPDATE_METHOD, + NOTIFY_UPDATE_RESPONSE_ACT, NOTIFY_UPDATE_RESPONSE_TAG, NOTIFY_UPDATE_TAG, + NOTIFY_UPDATE_TTL, NOTIFY_WATCH_SUBSCRIPTIONS_ACT, NOTIFY_WATCH_SUBSCRIPTIONS_METHOD, + NOTIFY_WATCH_SUBSCRIPTIONS_RESPONSE_ACT, NOTIFY_WATCH_SUBSCRIPTIONS_RESPONSE_TAG, + NOTIFY_WATCH_SUBSCRIPTIONS_TAG, NOTIFY_WATCH_SUBSCRIPTIONS_TTL, }, types::{encode_scope, Envelope, EnvelopeType0, EnvelopeType1, Notification}, utils::topic_from_key, @@ -236,7 +239,7 @@ async fn watch_subscriptions( iat: now.timestamp() as u64, exp: add_ttl(now, NOTIFY_SUBSCRIBE_TTL).timestamp() as u64, iss: identity_did_key.to_owned(), - act: "notify_watch_subscriptions".to_owned(), + act: NOTIFY_WATCH_SUBSCRIPTIONS_ACT.to_owned(), aud: DecodedClientId(authentication_key).to_did_key(), mjv: "0".to_owned(), }, @@ -318,7 +321,7 @@ async fn watch_subscriptions( let auth = from_jwt::(response_auth).unwrap(); assert_eq!( auth.shared_claims.act, - "notify_watch_subscriptions_response" + NOTIFY_WATCH_SUBSCRIPTIONS_RESPONSE_ACT ); assert_eq!( auth.shared_claims.iss, @@ -486,7 +489,7 @@ async fn run_test(statement: String, watch_subscriptions_all_domains: bool) { iat: now.timestamp() as u64, exp: add_ttl(now, NOTIFY_SUBSCRIBE_TTL).timestamp() as u64, iss: identity_did_key.clone(), - act: "notify_subscription".to_owned(), + act: NOTIFY_SUBSCRIBE_ACT.to_owned(), aud: dapp_did_key.clone(), mjv: "0".to_owned(), }, @@ -605,7 +608,7 @@ async fn run_test(statement: String, watch_subscriptions_all_domains: bool) { let subscribe_response_auth = from_jwt::(response_auth).unwrap(); assert_eq!( subscribe_response_auth.shared_claims.act, - "notify_subscription_response" + NOTIFY_SUBSCRIBE_RESPONSE_ACT ); let notify_key = { @@ -637,7 +640,7 @@ async fn run_test(statement: String, watch_subscriptions_all_domains: bool) { .as_str() .unwrap(); let auth = from_jwt::(response_auth).unwrap(); - assert_eq!(auth.shared_claims.act, "notify_subscriptions_changed"); + assert_eq!(auth.shared_claims.act, NOTIFY_SUBSCRIPTIONS_CHANGED_ACT); assert_eq!(auth.sbs.len(), 1); let sub = &auth.sbs[0]; assert_eq!(sub.scope, notification_types); @@ -736,7 +739,7 @@ async fn run_test(statement: String, watch_subscriptions_all_domains: bool) { assert!(claims.exp > chrono::Utc::now().timestamp() - JWT_LEEWAY); // TODO remove leeway assert_eq!(claims.app.as_ref(), app_domain); assert_eq!(claims.sub, did_pkh); - assert_eq!(claims.act, "notify_message"); + assert_eq!(claims.act, NOTIFY_MESSAGE_ACT); // TODO Notify receipt? // https://github.com/WalletConnect/walletconnect-docs/blob/main/docs/specs/clients/notify/notify-authentication.md#notify-receipt @@ -753,7 +756,7 @@ async fn run_test(statement: String, watch_subscriptions_all_domains: bool) { iat: now.timestamp() as u64, exp: add_ttl(now, NOTIFY_UPDATE_TTL).timestamp() as u64, iss: identity_did_key.clone(), - act: "notify_update".to_owned(), + act: NOTIFY_UPDATE_ACT.to_owned(), aud: dapp_did_key.clone(), mjv: "0".to_owned(), }, @@ -821,7 +824,7 @@ async fn run_test(statement: String, watch_subscriptions_all_domains: bool) { assert!((claims.shared_claims.exp as i64) > chrono::Utc::now().timestamp() - JWT_LEEWAY); // TODO remove leeway assert_eq!(claims.app, DidWeb::from_domain(app_domain.clone())); assert_eq!(claims.shared_claims.aud, identity_did_key); - assert_eq!(claims.shared_claims.act, "notify_update_response"); + assert_eq!(claims.shared_claims.act, NOTIFY_UPDATE_RESPONSE_ACT); { let resp = rx.recv().await.unwrap(); @@ -852,7 +855,7 @@ async fn run_test(statement: String, watch_subscriptions_all_domains: bool) { .as_str() .unwrap(); let auth = from_jwt::(response_auth).unwrap(); - assert_eq!(auth.shared_claims.act, "notify_subscriptions_changed"); + assert_eq!(auth.shared_claims.act, NOTIFY_SUBSCRIPTIONS_CHANGED_ACT); assert_eq!(auth.sbs.len(), 1); let subs = &auth.sbs[0]; assert_eq!(subs.scope, notification_types); @@ -867,7 +870,7 @@ async fn run_test(statement: String, watch_subscriptions_all_domains: bool) { exp: add_ttl(now, NOTIFY_DELETE_TTL).timestamp() as u64, iss: identity_did_key.clone(), aud: dapp_did_key.clone(), - act: "notify_delete".to_owned(), + act: NOTIFY_DELETE_ACT.to_owned(), mjv: "0".to_owned(), }, ksu: vars.keys_server_url.to_string(), @@ -933,7 +936,7 @@ async fn run_test(statement: String, watch_subscriptions_all_domains: bool) { assert!((claims.shared_claims.exp as i64) > chrono::Utc::now().timestamp() - JWT_LEEWAY); // TODO remove leeway assert_eq!(claims.app, DidWeb::from_domain(app_domain)); assert_eq!(claims.shared_claims.aud, identity_did_key); - assert_eq!(claims.shared_claims.act, "notify_delete_response"); + assert_eq!(claims.shared_claims.act, NOTIFY_DELETE_RESPONSE_ACT); { let resp = rx.recv().await.unwrap(); @@ -964,7 +967,7 @@ async fn run_test(statement: String, watch_subscriptions_all_domains: bool) { .as_str() .unwrap(); let auth = from_jwt::(response_auth).unwrap(); - assert_eq!(auth.shared_claims.act, "notify_subscriptions_changed"); + assert_eq!(auth.shared_claims.act, NOTIFY_SUBSCRIPTIONS_CHANGED_ACT); assert!(auth.sbs.is_empty()); }