From 444531f3ac0a91168441a8b0c0c66e4a0d07a156 Mon Sep 17 00:00:00 2001 From: Yuki Kishimoto Date: Mon, 12 May 2025 09:43:03 +0200 Subject: [PATCH] pool: add `AuthenticationMiddleware` - Add `AuthenticationMiddleware` trait - Move signer from `SharedState` to `Client` Closes https://github.com/rust-nostr/nostr/issues/739 Pull-Request: https://github.com/rust-nostr/nostr/pull/872 Signed-off-by: Yuki Kishimoto --- crates/nostr-relay-pool/src/policy.rs | 23 ++++- crates/nostr-relay-pool/src/pool/builder.rs | 11 +-- crates/nostr-relay-pool/src/pool/inner.rs | 2 +- crates/nostr-relay-pool/src/relay/error.rs | 3 + crates/nostr-relay-pool/src/relay/inner.rs | 22 +++-- crates/nostr-relay-pool/src/relay/mod.rs | 95 +++++++++++++++++---- crates/nostr-relay-pool/src/shared.rs | 43 ++-------- crates/nostr-sdk/src/client/error.rs | 3 + crates/nostr-sdk/src/client/middleware.rs | 33 +++++++ crates/nostr-sdk/src/client/mod.rs | 34 ++++++-- 10 files changed, 192 insertions(+), 77 deletions(-) create mode 100644 crates/nostr-sdk/src/client/middleware.rs diff --git a/crates/nostr-relay-pool/src/policy.rs b/crates/nostr-relay-pool/src/policy.rs index 772d7b57b..470b972ed 100644 --- a/crates/nostr-relay-pool/src/policy.rs +++ b/crates/nostr-relay-pool/src/policy.rs @@ -7,7 +7,7 @@ use std::fmt; use nostr::util::BoxedFuture; -use nostr::{Event, RelayUrl, SubscriptionId}; +use nostr::{Event, EventBuilder, RelayUrl, SubscriptionId}; /// Policy Error #[derive(Debug)] @@ -96,3 +96,24 @@ pub trait AdmitPolicy: fmt::Debug + Send + Sync { Box::pin(async move { Ok(AdmitStatus::Success) }) } } + +/// Authentication middleware +/// +/// +pub trait AuthenticationMiddleware: fmt::Debug + Send + Sync { + /// Check if the middleware is ready for authentication + /// + /// If the middleware doesn't have a signer yet, this will return `false`. + fn is_ready(&self) -> BoxedFuture<'_, bool>; + + /// Build authentication [`Event`]. + /// + /// Takes an [`EventBuilder`] and returns the signed [`Event`] for authenticating to the relay. + /// + /// + fn authenticate<'a>( + &'a self, + relay_url: &'a RelayUrl, + builder: EventBuilder, + ) -> BoxedFuture<'a, Result>; +} diff --git a/crates/nostr-relay-pool/src/pool/builder.rs b/crates/nostr-relay-pool/src/pool/builder.rs index e246f574f..57d4951bf 100644 --- a/crates/nostr-relay-pool/src/pool/builder.rs +++ b/crates/nostr-relay-pool/src/pool/builder.rs @@ -6,13 +6,12 @@ use std::sync::Arc; -use nostr::NostrSigner; use nostr_database::{MemoryDatabase, NostrDatabase}; use super::options::RelayPoolOptions; use super::RelayPool; use crate::monitor::Monitor; -use crate::policy::AdmitPolicy; +use crate::policy::{AdmitPolicy, AuthenticationMiddleware}; use crate::transport::websocket::{DefaultWebsocketTransport, WebSocketTransport}; /// Relay Pool builder @@ -22,6 +21,10 @@ pub struct RelayPoolBuilder { pub websocket_transport: Arc, /// Admission policy pub admit_policy: Option>, + /// Authentication middleware + /// + /// + pub auth_middleware: Option>, /// Relay monitor pub monitor: Option, /// Relay pool options @@ -29,8 +32,6 @@ pub struct RelayPoolBuilder { // Private stuff #[doc(hidden)] pub __database: Arc, - #[doc(hidden)] - pub __signer: Option>, } impl Default for RelayPoolBuilder { @@ -38,10 +39,10 @@ impl Default for RelayPoolBuilder { Self { websocket_transport: Arc::new(DefaultWebsocketTransport), admit_policy: None, + auth_middleware: None, monitor: None, opts: RelayPoolOptions::default(), __database: Arc::new(MemoryDatabase::default()), - __signer: None, } } } diff --git a/crates/nostr-relay-pool/src/pool/inner.rs b/crates/nostr-relay-pool/src/pool/inner.rs index 7089b5562..568ef2017 100644 --- a/crates/nostr-relay-pool/src/pool/inner.rs +++ b/crates/nostr-relay-pool/src/pool/inner.rs @@ -52,8 +52,8 @@ impl InnerRelayPool { state: SharedState::new( builder.__database, builder.websocket_transport, - builder.__signer, builder.admit_policy, + builder.auth_middleware, builder.opts.nip42_auto_authentication, builder.monitor, ), diff --git a/crates/nostr-relay-pool/src/relay/error.rs b/crates/nostr-relay-pool/src/relay/error.rs index ee46c4cb3..053508f87 100644 --- a/crates/nostr-relay-pool/src/relay/error.rs +++ b/crates/nostr-relay-pool/src/relay/error.rs @@ -114,6 +114,8 @@ pub enum Error { }, /// Auth failed AuthenticationFailed, + /// Authentication middleware not set + AuthMiddlewareNotSet, /// Premature exit PrematureExit, } @@ -178,6 +180,7 @@ impl fmt::Display for Error { current.as_millis() ), Self::AuthenticationFailed => write!(f, "authentication failed"), + Self::AuthMiddlewareNotSet => write!(f, "authentication middleware not set"), Self::PrematureExit => write!(f, "premature exit"), } } diff --git a/crates/nostr-relay-pool/src/relay/inner.rs b/crates/nostr-relay-pool/src/relay/inner.rs index bc5ce6a02..d3fc01d33 100644 --- a/crates/nostr-relay-pool/src/relay/inner.rs +++ b/crates/nostr-relay-pool/src/relay/inner.rs @@ -34,7 +34,7 @@ use super::{ Error, Reconciliation, RelayNotification, RelayStatus, SubscriptionActivity, SubscriptionAutoClosedReason, }; -use crate::policy::AdmitStatus; +use crate::policy::{AdmitStatus, AuthenticationMiddleware}; use crate::pool::RelayPoolNotification; use crate::relay::status::AtomicRelayStatus; use crate::shared::SharedState; @@ -988,6 +988,7 @@ impl InnerRelay { ); // Check if NIP42 auto authentication is enabled + // TODO: check also if middleware is set? if self.state.is_auto_authentication_enabled() { // Forward action to ingester let _ = ingester_tx.send(IngesterCommand::Authenticate { @@ -1217,13 +1218,18 @@ impl InnerRelay { } async fn auth(&self, challenge: String) -> Result<(), Error> { - // Get signer - let signer = self.state.signer().await?; - - // Construct event - let event: Event = EventBuilder::auth(challenge, self.url.clone()) - .sign(&signer) - .await?; + // Get middleware + let middleware: &Arc = self + .state + .auth_middleware + .as_ref() + .ok_or(Error::AuthMiddlewareNotSet)?; + + // Construct event builder + let builder: EventBuilder = EventBuilder::auth(challenge, self.url.clone()); + + // Create the authentication event + let event: Event = middleware.authenticate(&self.url, builder).await?; // Subscribe to notifications let mut notifications = self.internal_notification_sender.subscribe(); diff --git a/crates/nostr-relay-pool/src/relay/mod.rs b/crates/nostr-relay-pool/src/relay/mod.rs index 7288f0361..a2698f9e5 100644 --- a/crates/nostr-relay-pool/src/relay/mod.rs +++ b/crates/nostr-relay-pool/src/relay/mod.rs @@ -407,22 +407,29 @@ impl Relay { // If auth required, wait for authentication adn resend it if let Some(MachineReadablePrefix::AuthRequired) = MachineReadablePrefix::parse(&message) { - // Check if NIP42 auth is enabled and signer is set - let has_signer: bool = self.inner.state.has_signer().await; - if self.inner.state.is_auto_authentication_enabled() && has_signer { - // Wait that relay authenticate - self.wait_for_authentication(&mut notifications, WAIT_FOR_AUTHENTICATION_TIMEOUT) + // Check if NIP42 auth is enabled and middleware is set + if let Some(middleware) = &self.inner.state.auth_middleware { + let is_enabled: bool = self.inner.state.is_auto_authentication_enabled(); + let is_ready: bool = middleware.is_ready().await; + + if is_enabled && is_ready { + // Wait that relay authenticate + self.wait_for_authentication( + &mut notifications, + WAIT_FOR_AUTHENTICATION_TIMEOUT, + ) .await?; - // Try to resend event - let (status, message) = self._send_event(&mut notifications, event).await?; + // Try to resend event + let (status, message) = self._send_event(&mut notifications, event).await?; - // Check status - return if status { - Ok(event.id) - } else { - Err(Error::RelayMessage(message)) - }; + // Check status + return if status { + Ok(event.id) + } else { + Err(Error::RelayMessage(message)) + }; + } } } @@ -746,9 +753,10 @@ mod tests { use async_utility::time; use nostr_relay_builder::prelude::*; + use tokio::sync::RwLock; use super::{Error, *}; - use crate::policy::{AdmitPolicy, PolicyError}; + use crate::policy::{AdmitPolicy, AuthenticationMiddleware, PolicyError}; #[derive(Debug)] struct CustomTestPolicy { @@ -770,10 +778,55 @@ mod tests { } } + #[derive(Debug, Default)] + struct AuthenticationPolicy { + signer: RwLock>>, + } + + impl AuthenticationPolicy { + async fn set_signer(&self, signer: Option>) { + let mut s = self.signer.write().await; + *s = signer; + } + } + + impl AuthenticationMiddleware for AuthenticationPolicy { + fn is_ready(&self) -> BoxedFuture<'_, bool> { + Box::pin(async move { self.signer.read().await.is_some() }) + } + + fn authenticate<'a>( + &'a self, + _relay_url: &'a RelayUrl, + builder: EventBuilder, + ) -> BoxedFuture<'a, Result> { + Box::pin(async move { + let signer = self.signer.read().await; + + match signer.as_ref() { + Some(signer) => builder.sign(signer).await.map_err(PolicyError::backend), + None => { + return Err(PolicyError::backend(Error::AuthenticationFailed)); + } + } + }) + } + } + fn new_relay(url: RelayUrl, opts: RelayOptions) -> Relay { Relay::new(url, SharedState::default(), opts) } + fn new_relay_with_auth_middleware( + url: RelayUrl, + middleware: Arc, + opts: RelayOptions, + ) -> Relay { + let mut state: SharedState = SharedState::default(); + state.auth_middleware = Some(middleware); + Relay::new(url, state, opts) + } + /// Setup public (without NIP42 auth) relay with N events to test event fetching /// /// **Adds ONLY text notes** @@ -1161,7 +1214,10 @@ mod tests { let mock = LocalRelay::run(builder).await.unwrap(); let url = RelayUrl::parse(&mock.url()).unwrap(); - let relay: Relay = new_relay(url, RelayOptions::default()); + let middleware = Arc::new(AuthenticationPolicy::default()); + + let relay: Relay = + new_relay_with_auth_middleware(url, middleware.clone(), RelayOptions::default()); relay.inner.state.automatic_authentication(true); @@ -1185,7 +1241,7 @@ mod tests { } // Set a signer - relay.inner.state.set_signer(keys.clone()).await; + middleware.set_signer(Some(Arc::new(keys.clone()))).await; // Send as authenticated let event = EventBuilder::text_note("Test") @@ -1204,7 +1260,10 @@ mod tests { let mock = LocalRelay::run(builder).await.unwrap(); let url = RelayUrl::parse(&mock.url()).unwrap(); - let relay: Relay = new_relay(url, RelayOptions::default()); + let middleware = Arc::new(AuthenticationPolicy::default()); + + let relay: Relay = + new_relay_with_auth_middleware(url, middleware.clone(), RelayOptions::default()); relay.connect(); @@ -1256,7 +1315,7 @@ mod tests { assert!(matches!(err, Error::AuthenticationFailed)); // Set a signer - relay.inner.state.set_signer(keys).await; + middleware.set_signer(Some(Arc::new(keys))).await; // Authenticated fetch let res = relay diff --git a/crates/nostr-relay-pool/src/shared.rs b/crates/nostr-relay-pool/src/shared.rs index 272f9825b..349d2e3d7 100644 --- a/crates/nostr-relay-pool/src/shared.rs +++ b/crates/nostr-relay-pool/src/shared.rs @@ -10,13 +10,11 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use lru::LruCache; -use nostr::prelude::IntoNostrSigner; -use nostr::{EventId, NostrSigner}; +use nostr::EventId; use nostr_database::{IntoNostrDatabase, MemoryDatabase, NostrDatabase}; -use tokio::sync::RwLock; use crate::monitor::Monitor; -use crate::policy::AdmitPolicy; +use crate::policy::{AdmitPolicy, AuthenticationMiddleware}; use crate::transport::websocket::{DefaultWebsocketTransport, WebSocketTransport}; // LruCache pre-allocate, so keep this at a reasonable value. @@ -25,7 +23,6 @@ const MAX_VERIFICATION_CACHE_SIZE: usize = 128_000; #[derive(Debug)] pub enum SharedStateError { - SignerNotConfigured, MutexPoisoned, } @@ -34,7 +31,6 @@ impl std::error::Error for SharedStateError {} impl fmt::Display for SharedStateError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::SignerNotConfigured => write!(f, "signer not configured"), Self::MutexPoisoned => write!(f, "mutex poisoned"), } } @@ -44,10 +40,10 @@ impl fmt::Display for SharedStateError { pub struct SharedState { pub(crate) database: Arc, pub(crate) transport: Arc, - signer: Arc>>>, nip42_auto_authentication: Arc, verification_cache: Arc>>, pub(crate) admit_policy: Option>, + pub(crate) auth_middleware: Option>, pub(crate) monitor: Option, } @@ -68,8 +64,8 @@ impl SharedState { pub fn new( database: Arc, transport: Arc, - signer: Option>, admit_policy: Option>, + auth_middleware: Option>, nip42_auto_authentication: bool, monitor: Option, ) -> Self { @@ -80,10 +76,10 @@ impl SharedState { Self { database, transport, - signer: Arc::new(RwLock::new(signer)), nip42_auto_authentication: Arc::new(AtomicBool::new(nip42_auto_authentication)), verification_cache: Arc::new(Mutex::new(LruCache::new(max_verification_cache_size))), admit_policy, + auth_middleware, monitor, } } @@ -119,35 +115,6 @@ impl SharedState { &self.database } - /// Check if signer is configured - pub async fn has_signer(&self) -> bool { - let signer = self.signer.read().await; - signer.is_some() - } - - /// Get current nostr signer - /// - /// Rise error if it not set. - pub async fn signer(&self) -> Result, SharedStateError> { - let signer = self.signer.read().await; - signer.clone().ok_or(SharedStateError::SignerNotConfigured) - } - - /// Set nostr signer - pub async fn set_signer(&self, signer: T) - where - T: IntoNostrSigner, - { - let mut s = self.signer.write().await; - *s = Some(signer.into_nostr_signer()); - } - - /// Unset nostr signer - pub async fn unset_signer(&self) { - let mut s = self.signer.write().await; - *s = None; - } - pub(crate) fn verified(&self, id: &EventId) -> Result { let mut cache = self .verification_cache diff --git a/crates/nostr-sdk/src/client/error.rs b/crates/nostr-sdk/src/client/error.rs index 10e70d3d4..d43324927 100644 --- a/crates/nostr-sdk/src/client/error.rs +++ b/crates/nostr-sdk/src/client/error.rs @@ -36,6 +36,8 @@ pub enum Error { ImpossibleToZap(String), /// Broken down filters for gossip are empty GossipFiltersEmpty, + /// Signer not configured + SignerNotConfigured, /// Private message (NIP17) relays not found PrivateMsgRelaysNotFound, } @@ -63,6 +65,7 @@ impl fmt::Display for Error { Self::GossipFiltersEmpty => { write!(f, "gossip broken down filters are empty") } + Self::SignerNotConfigured => write!(f, "signer not configured"), Self::PrivateMsgRelaysNotFound => write!(f, "Private message relays not found. The user is not ready to receive private messages."), } } diff --git a/crates/nostr-sdk/src/client/middleware.rs b/crates/nostr-sdk/src/client/middleware.rs new file mode 100644 index 000000000..993a0d3da --- /dev/null +++ b/crates/nostr-sdk/src/client/middleware.rs @@ -0,0 +1,33 @@ +use std::sync::Arc; + +use nostr::prelude::BoxedFuture; +use nostr::{Event, EventBuilder, NostrSigner, RelayUrl}; +use nostr_relay_pool::policy::{AuthenticationMiddleware, PolicyError}; +use tokio::sync::RwLock; + +use super::error::Error; + +#[derive(Debug)] +pub(crate) struct DefaultAuthMiddleware { + pub(crate) signer: Arc>>>, +} + +impl AuthenticationMiddleware for DefaultAuthMiddleware { + fn is_ready(&self) -> BoxedFuture<'_, bool> { + Box::pin(async move { self.signer.read().await.is_some() }) + } + + fn authenticate<'a>( + &'a self, + _relay_url: &'a RelayUrl, + builder: EventBuilder, + ) -> BoxedFuture<'a, Result> { + Box::pin(async move { + let signer = self.signer.read().await; + let signer: &Arc = signer + .as_ref() + .ok_or(PolicyError::backend(Error::SignerNotConfigured))?; + builder.sign(signer).await.map_err(PolicyError::backend) + }) + } +} diff --git a/crates/nostr-sdk/src/client/mod.rs b/crates/nostr-sdk/src/client/mod.rs index d416a8e63..8c36bf6d6 100644 --- a/crates/nostr-sdk/src/client/mod.rs +++ b/crates/nostr-sdk/src/client/mod.rs @@ -13,14 +13,16 @@ use std::time::Duration; use nostr::prelude::*; use nostr_database::prelude::*; use nostr_relay_pool::prelude::*; -use tokio::sync::broadcast; +use tokio::sync::{broadcast, RwLock}; pub mod builder; mod error; +mod middleware; pub mod options; pub use self::builder::ClientBuilder; pub use self::error::Error; +use self::middleware::DefaultAuthMiddleware; pub use self::options::Options; #[cfg(not(target_arch = "wasm32"))] pub use self::options::{Connection, ConnectionTarget}; @@ -31,6 +33,7 @@ use crate::gossip::{BrokenDownFilters, Gossip}; pub struct Client { pool: RelayPool, gossip: Gossip, + signer: Arc>>>, opts: Options, } @@ -79,20 +82,35 @@ impl Client { } fn from_builder(builder: ClientBuilder) -> Self { + // Wrap signer + let signer: Arc>>> = + Arc::new(RwLock::new(builder.signer)); + + // Construct authentication middleware + let auth_middleware = Arc::new(DefaultAuthMiddleware { + // Clone the signer Arc! + // + // All changes done in the `Arc>` + // in the client will be reflected also in the middleware, + // since it's wrapped in an `Arc`. + signer: signer.clone(), + }); + // Construct relay pool builder let pool_builder: RelayPoolBuilder = RelayPoolBuilder { websocket_transport: builder.websocket_transport, admit_policy: builder.admit_policy, + auth_middleware: Some(auth_middleware), monitor: builder.monitor, opts: builder.opts.pool, __database: builder.database, - __signer: builder.signer, }; // Construct client Self { pool: pool_builder.build(), gossip: Gossip::new(), + signer, opts: builder.opts, } } @@ -117,7 +135,8 @@ impl Client { /// Check if signer is configured #[inline] pub async fn has_signer(&self) -> bool { - self.pool.state().has_signer().await + let signer = self.signer.read().await; + signer.is_some() } /// Get current nostr signer @@ -127,7 +146,8 @@ impl Client { /// Returns an error if the signer isn't set. #[inline] pub async fn signer(&self) -> Result, Error> { - Ok(self.pool.state().signer().await?) + let signer = self.signer.read().await; + signer.clone().ok_or(Error::SignerNotConfigured) } /// Set nostr signer @@ -136,13 +156,15 @@ impl Client { where T: IntoNostrSigner, { - self.pool.state().set_signer(signer).await; + let mut s = self.signer.write().await; + *s = Some(signer.into_nostr_signer()); } /// Unset nostr signer #[inline] pub async fn unset_signer(&self) { - self.pool.state().unset_signer().await; + let mut s = self.signer.write().await; + *s = None; } /// Get [`RelayPool`]