refactor(signer): Signer trait now returns SigningError instead of AppError
Completes the approved Step 3 API (the enum + error types landed in
e6e4922; this closes the trait half):
- Signer trait: get_public_key, sign_event, pubkey_for (default),
disconnect, revoke all return Result<_, SigningError>
- Signing enum wrapper (pubkey/sign) returns SigningError; local sign
failures map to InvalidSignature, external identity/verify failures to
IdentityMismatch/InvalidSignature
- EmbeddedSigner::resolve_keys + respond_to_approval speak SigningError;
AppError from vault/profile ops converts via SigningError::from_app
- Nip46ClientSigner: inherent methods (connect/disconnect/revoke/
parse_connect_uri/permissions/nip44) keep AppError; the trait impl
wraps them with from_app
- No mode branching introduced; IPC boundary conversion
(From<SigningError> for AppError) already exists
Verification: cargo test 196 passed, clippy clean, fmt clean
This commit is contained in:
parent
a2307ac475
commit
b2755d8343
3 changed files with 81 additions and 52 deletions
|
|
@ -8,8 +8,8 @@ use nostr_sdk::prelude::*;
|
||||||
use tokio::sync::{oneshot, Mutex};
|
use tokio::sync::{oneshot, Mutex};
|
||||||
|
|
||||||
use crate::app::App;
|
use crate::app::App;
|
||||||
use crate::errors::AppError;
|
|
||||||
use crate::profiles;
|
use crate::profiles;
|
||||||
|
use crate::signer::backend::SigningError;
|
||||||
use crate::signer::types::{ApprovalDetails, ApprovalResult, SignerType};
|
use crate::signer::types::{ApprovalDetails, ApprovalResult, SignerType};
|
||||||
use crate::signer::Signer;
|
use crate::signer::Signer;
|
||||||
|
|
||||||
|
|
@ -55,20 +55,22 @@ impl EmbeddedSigner {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Resolve the active profile's Keys, checking vault lock state.
|
/// Resolve the active profile's Keys, checking vault lock state.
|
||||||
async fn resolve_keys(&self) -> Result<Keys, AppError> {
|
async fn resolve_keys(&self) -> Result<Keys, SigningError> {
|
||||||
let app = self.app.lock().await;
|
let app = self.app.lock().await;
|
||||||
let npub_guard = self.active_npub.lock().await;
|
let npub_guard = self.active_npub.lock().await;
|
||||||
let npub = npub_guard.as_ref().ok_or_else(|| {
|
let npub = npub_guard.as_ref().ok_or(SigningError::NoActiveProfile)?;
|
||||||
AppError::config("No active profile selected. Choose a profile first.")
|
|
||||||
})?;
|
|
||||||
|
|
||||||
if app.is_locked() {
|
if app.is_locked() {
|
||||||
return Err(AppError::vault_locked());
|
return Err(SigningError::InternalKeyUnavailable {
|
||||||
|
detail: "vault locked".to_string(),
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
let vault_key = app.vault_key().copied();
|
let vault_key = app.vault_key().copied();
|
||||||
let secret_hex = profiles::resolve_secret_key(&app.vault, npub, vault_key.as_ref())?;
|
let secret_hex = profiles::resolve_secret_key(&app.vault, npub, vault_key.as_ref())
|
||||||
let secret_key = profiles::parse_secret_key(&secret_hex)?;
|
.map_err(|e| SigningError::from_app(&e))?;
|
||||||
|
let secret_key =
|
||||||
|
profiles::parse_secret_key(&secret_hex).map_err(|e| SigningError::from_app(&e))?;
|
||||||
Ok(Keys::new(secret_key))
|
Ok(Keys::new(secret_key))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -120,10 +122,16 @@ impl EmbeddedSigner {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Approve or reject a pending request by index.
|
/// Approve or reject a pending request by index.
|
||||||
pub async fn respond_to_approval(&self, index: usize, approved: bool) -> Result<(), AppError> {
|
pub async fn respond_to_approval(
|
||||||
|
&self,
|
||||||
|
index: usize,
|
||||||
|
approved: bool,
|
||||||
|
) -> Result<(), SigningError> {
|
||||||
let mut pending = self.pending.lock().await;
|
let mut pending = self.pending.lock().await;
|
||||||
if index >= pending.len() {
|
if index >= pending.len() {
|
||||||
return Err(AppError::config("No pending request at that index"));
|
return Err(SigningError::Internal {
|
||||||
|
detail: "No pending request at that index".to_string(),
|
||||||
|
});
|
||||||
}
|
}
|
||||||
let entry = pending.remove(index);
|
let entry = pending.remove(index);
|
||||||
let _ = entry.sender.send(if approved {
|
let _ = entry.sender.send(if approved {
|
||||||
|
|
@ -175,12 +183,12 @@ impl EmbeddedSigner {
|
||||||
|
|
||||||
#[async_trait]
|
#[async_trait]
|
||||||
impl Signer for EmbeddedSigner {
|
impl Signer for EmbeddedSigner {
|
||||||
async fn get_public_key(&self) -> Result<PublicKey, AppError> {
|
async fn get_public_key(&self) -> Result<PublicKey, SigningError> {
|
||||||
let keys = self.resolve_keys().await?;
|
let keys = self.resolve_keys().await?;
|
||||||
Ok(keys.public_key())
|
Ok(keys.public_key())
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn sign_event(&self, event: UnsignedEvent) -> Result<Event, AppError> {
|
async fn sign_event(&self, event: UnsignedEvent) -> Result<Event, SigningError> {
|
||||||
let keys = self.resolve_keys().await?;
|
let keys = self.resolve_keys().await?;
|
||||||
|
|
||||||
// Request approval for sensitive operations
|
// Request approval for sensitive operations
|
||||||
|
|
@ -188,11 +196,13 @@ impl Signer for EmbeddedSigner {
|
||||||
let approval = self.request_approval(details).await;
|
let approval = self.request_approval(details).await;
|
||||||
|
|
||||||
match approval {
|
match approval {
|
||||||
ApprovalResult::Approved => keys
|
ApprovalResult::Approved => {
|
||||||
.sign_event(event)
|
keys.sign_event(event).map_err(|e| SigningError::Internal {
|
||||||
.map_err(|e| AppError::internal(format!("Failed to sign event: {e}"))),
|
detail: format!("Failed to sign event: {e}"),
|
||||||
ApprovalResult::Rejected => Err(AppError::config("Signing request rejected by user")),
|
})
|
||||||
ApprovalResult::Timeout => Err(AppError::config("Signing request timed out")),
|
}
|
||||||
|
ApprovalResult::Rejected => Err(SigningError::Rejected),
|
||||||
|
ApprovalResult::Timeout => Err(SigningError::Timeout),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -210,14 +220,14 @@ impl Signer for EmbeddedSigner {
|
||||||
self.await_approval(details).await
|
self.await_approval(details).await
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn disconnect(&self) -> Result<(), AppError> {
|
async fn disconnect(&self) -> Result<(), SigningError> {
|
||||||
let mut npub_guard = self.active_npub.lock().await;
|
let mut npub_guard = self.active_npub.lock().await;
|
||||||
*npub_guard = None;
|
*npub_guard = None;
|
||||||
self.pending.lock().await.clear();
|
self.pending.lock().await.clear();
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn revoke(&self) -> Result<(), AppError> {
|
async fn revoke(&self) -> Result<(), SigningError> {
|
||||||
let npub = {
|
let npub = {
|
||||||
let mut npub_guard = self.active_npub.lock().await;
|
let mut npub_guard = self.active_npub.lock().await;
|
||||||
npub_guard.take()
|
npub_guard.take()
|
||||||
|
|
@ -225,7 +235,9 @@ impl Signer for EmbeddedSigner {
|
||||||
if let Some(npub) = npub {
|
if let Some(npub) = npub {
|
||||||
let mut app = self.app.lock().await;
|
let mut app = self.app.lock().await;
|
||||||
let _ = profiles::delete_profile(&mut app.vault, &npub);
|
let _ = profiles::delete_profile(&mut app.vault, &npub);
|
||||||
app.save_vault()?;
|
app.save_vault().map_err(|e| SigningError::Internal {
|
||||||
|
detail: format!("Could not persist vault: {e}"),
|
||||||
|
})?;
|
||||||
}
|
}
|
||||||
self.pending.lock().await.clear();
|
self.pending.lock().await.clear();
|
||||||
Ok(())
|
Ok(())
|
||||||
|
|
|
||||||
|
|
@ -12,34 +12,39 @@ use async_trait::async_trait;
|
||||||
use nostr_sdk::prelude::*;
|
use nostr_sdk::prelude::*;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
|
||||||
use crate::errors::AppError;
|
|
||||||
use crate::signer::types::{ApprovalDetails, ApprovalResult, SignerType};
|
use crate::signer::types::{ApprovalDetails, ApprovalResult, SignerType};
|
||||||
|
|
||||||
/// Common interface for all signer implementations.
|
/// Common interface for all signer implementations.
|
||||||
|
///
|
||||||
|
/// Every method that can fail a *signing* operation returns
|
||||||
|
/// [`Result<_, SigningError>`], so the whole signing subsystem speaks one
|
||||||
|
/// closed error type. The IPC layer converts [`SigningError`] to the app-wide
|
||||||
|
/// [`crate::errors::AppError`] at the boundary (via [`From`]) — no string
|
||||||
|
/// matching, no mode branching.
|
||||||
#[async_trait]
|
#[async_trait]
|
||||||
pub trait Signer: Send + Sync {
|
pub trait Signer: Send + Sync {
|
||||||
/// Get the public key of the active signing identity.
|
/// Get the public key of the active signing identity.
|
||||||
async fn get_public_key(&self) -> Result<PublicKey, AppError>;
|
async fn get_public_key(&self) -> Result<PublicKey, SigningError>;
|
||||||
|
|
||||||
/// Resolve the public key this signer will sign user content with,
|
/// Resolve the public key this signer will sign user content with,
|
||||||
/// enforcing that it matches the active profile's canonical identity.
|
/// enforcing that it matches the active profile's canonical identity.
|
||||||
///
|
///
|
||||||
/// The default implementation compares `get_public_key()` against
|
/// The default implementation compares `get_public_key()` against
|
||||||
/// `profile_pubkey` using canonical hex, returning
|
/// `profile_pubkey` using canonical hex, returning
|
||||||
/// [`AppError::external_signer_identity_mismatch`] on any difference.
|
/// [`SigningError::IdentityMismatch`] on any difference. External signers
|
||||||
/// External signers may override this to consult the remote signer's
|
/// may override this to consult the remote signer's identity. Callers must
|
||||||
/// identity. Callers must use the returned key as the event's `pubkey`
|
/// use the returned key as the event's `pubkey` and must never sign user
|
||||||
/// and must never sign user content when this errors.
|
/// content when this errors.
|
||||||
async fn pubkey_for(&self, profile_pubkey: &PublicKey) -> Result<PublicKey, AppError> {
|
async fn pubkey_for(&self, profile_pubkey: &PublicKey) -> Result<PublicKey, SigningError> {
|
||||||
let signer_pubkey = self.get_public_key().await?;
|
let signer_pubkey = self.get_public_key().await?;
|
||||||
if signer_pubkey.to_hex() != profile_pubkey.to_hex() {
|
if signer_pubkey.to_hex() != profile_pubkey.to_hex() {
|
||||||
return Err(AppError::external_signer_identity_mismatch());
|
return Err(SigningError::IdentityMismatch);
|
||||||
}
|
}
|
||||||
Ok(signer_pubkey)
|
Ok(signer_pubkey)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Sign an event with the active key.
|
/// Sign an event with the active key.
|
||||||
async fn sign_event(&self, event: UnsignedEvent) -> Result<Event, AppError>;
|
async fn sign_event(&self, event: UnsignedEvent) -> Result<Event, SigningError>;
|
||||||
|
|
||||||
/// Get the type of this signer.
|
/// Get the type of this signer.
|
||||||
fn get_signer_type(&self) -> SignerType;
|
fn get_signer_type(&self) -> SignerType;
|
||||||
|
|
@ -52,10 +57,10 @@ pub trait Signer: Send + Sync {
|
||||||
async fn request_approval(&self, details: ApprovalDetails) -> ApprovalResult;
|
async fn request_approval(&self, details: ApprovalDetails) -> ApprovalResult;
|
||||||
|
|
||||||
/// Disconnect/stop the signer (for NIP-46, closes connection).
|
/// Disconnect/stop the signer (for NIP-46, closes connection).
|
||||||
async fn disconnect(&self) -> Result<(), AppError>;
|
async fn disconnect(&self) -> Result<(), SigningError>;
|
||||||
|
|
||||||
/// Revoke the signer authorization (for NIP-46, revokes the connection).
|
/// Revoke the signer authorization (for NIP-46, revokes the connection).
|
||||||
async fn revoke(&self) -> Result<(), AppError>;
|
async fn revoke(&self) -> Result<(), SigningError>;
|
||||||
|
|
||||||
/// Get a human-readable status string for UI display.
|
/// Get a human-readable status string for UI display.
|
||||||
async fn status_string(&self) -> String;
|
async fn status_string(&self) -> String;
|
||||||
|
|
@ -155,10 +160,10 @@ impl Signing {
|
||||||
/// The public key user content will be signed with.
|
/// The public key user content will be signed with.
|
||||||
///
|
///
|
||||||
/// For [`Signing::External`] this enforces identity validation and returns
|
/// For [`Signing::External`] this enforces identity validation and returns
|
||||||
/// the signer's key; it returns [`AppError::external_signer_identity_mismatch`]
|
/// the signer's key; it returns [`SigningError::IdentityMismatch`] when the
|
||||||
/// when the signer does not control the active profile. Callers MUST use the
|
/// signer does not control the active profile. Callers MUST use the
|
||||||
/// returned key as the event's `pubkey`.
|
/// returned key as the event's `pubkey`.
|
||||||
pub async fn pubkey(&self) -> Result<PublicKey, AppError> {
|
pub async fn pubkey(&self) -> Result<PublicKey, SigningError> {
|
||||||
match self {
|
match self {
|
||||||
Signing::Local(keys) => Ok(keys.public_key()),
|
Signing::Local(keys) => Ok(keys.public_key()),
|
||||||
Signing::External {
|
Signing::External {
|
||||||
|
|
@ -174,18 +179,20 @@ impl Signing {
|
||||||
/// For [`Signing::External`] the returned event is re-checked against the
|
/// For [`Signing::External`] the returned event is re-checked against the
|
||||||
/// validated identity and verified as a well-formed signature before it is
|
/// validated identity and verified as a well-formed signature before it is
|
||||||
/// returned, so a misbehaving signer cannot substitute a different key.
|
/// returned, so a misbehaving signer cannot substitute a different key.
|
||||||
pub async fn sign(&self, unsigned: UnsignedEvent) -> Result<Event, AppError> {
|
pub async fn sign(&self, unsigned: UnsignedEvent) -> Result<Event, SigningError> {
|
||||||
match self {
|
match self {
|
||||||
Signing::Local(keys) => keys.sign_event(unsigned).map_err(AppError::sign_failed),
|
Signing::Local(keys) => keys
|
||||||
|
.sign_event(unsigned)
|
||||||
|
.map_err(|_e| SigningError::InvalidSignature),
|
||||||
Signing::External {
|
Signing::External {
|
||||||
signer,
|
signer,
|
||||||
profile_pubkey,
|
profile_pubkey,
|
||||||
} => {
|
} => {
|
||||||
let event = signer.sign_event(unsigned).await?;
|
let event = signer.sign_event(unsigned).await?;
|
||||||
if event.pubkey != *profile_pubkey {
|
if event.pubkey != *profile_pubkey {
|
||||||
return Err(AppError::external_signer_identity_mismatch());
|
return Err(SigningError::IdentityMismatch);
|
||||||
}
|
}
|
||||||
event.verify().map_err(AppError::sign_failed)?;
|
event.verify().map_err(|_| SigningError::InvalidSignature)?;
|
||||||
Ok(event)
|
Ok(event)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -18,6 +18,7 @@ use tokio::sync::{oneshot, Mutex};
|
||||||
use crate::app::App;
|
use crate::app::App;
|
||||||
use crate::errors::AppError;
|
use crate::errors::AppError;
|
||||||
use crate::profiles;
|
use crate::profiles;
|
||||||
|
use crate::signer::backend::SigningError;
|
||||||
use crate::signer::permissions::Nip46Permissions;
|
use crate::signer::permissions::Nip46Permissions;
|
||||||
use crate::signer::types::{
|
use crate::signer::types::{
|
||||||
ApprovalDetails, ApprovalResult, Nip46Connection, Nip46Status, PendingApproval, SignerType,
|
ApprovalDetails, ApprovalResult, Nip46Connection, Nip46Status, PendingApproval, SignerType,
|
||||||
|
|
@ -826,22 +827,27 @@ impl Clone for Nip46ClientSigner {
|
||||||
|
|
||||||
#[async_trait]
|
#[async_trait]
|
||||||
impl Signer for Nip46ClientSigner {
|
impl Signer for Nip46ClientSigner {
|
||||||
async fn get_public_key(&self) -> Result<PublicKey, AppError> {
|
async fn get_public_key(&self) -> Result<PublicKey, SigningError> {
|
||||||
let inner = self.inner.lock().await;
|
let inner = self.inner.lock().await;
|
||||||
let connection = inner
|
let connection = inner
|
||||||
.connection
|
.connection
|
||||||
.as_ref()
|
.as_ref()
|
||||||
.ok_or_else(|| AppError::config("Not connected to a signer"))?;
|
.ok_or(SigningError::NotConnected)?;
|
||||||
PublicKey::from_hex(&connection.signer_pubkey)
|
PublicKey::from_hex(&connection.signer_pubkey).map_err(|e| SigningError::Internal {
|
||||||
.map_err(|_| AppError::config("Invalid signer public key"))
|
detail: format!("Invalid signer public key: {e}"),
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn sign_event(&self, _event: UnsignedEvent) -> Result<Event, AppError> {
|
async fn sign_event(&self, _event: UnsignedEvent) -> Result<Event, SigningError> {
|
||||||
// For NIP-46 client, signing happens via the NIP-46 channel with user approval
|
// For NIP-46 client, signing happens via the NIP-46 channel with user
|
||||||
// The actual flow uses request_approval + respond_to_approval
|
// approval. The actual flow uses request_approval +
|
||||||
Err(AppError::config(
|
// respond_to_approval; this method exists to satisfy the object-safe
|
||||||
"NIP-46 signing uses async approval flow. Use request_approval.",
|
// trait and fails closed if a caller tries to bypass it.
|
||||||
))
|
Err(SigningError::Internal {
|
||||||
|
detail:
|
||||||
|
"NIP-46 signing uses the async approval flow; direct sign_event is not supported"
|
||||||
|
.to_string(),
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
fn get_signer_type(&self) -> SignerType {
|
fn get_signer_type(&self) -> SignerType {
|
||||||
|
|
@ -857,12 +863,16 @@ impl Signer for Nip46ClientSigner {
|
||||||
self.await_approval(details).await
|
self.await_approval(details).await
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn disconnect(&self) -> Result<(), AppError> {
|
async fn disconnect(&self) -> Result<(), SigningError> {
|
||||||
Nip46ClientSigner::disconnect(self).await
|
Nip46ClientSigner::disconnect(self)
|
||||||
|
.await
|
||||||
|
.map_err(|e| SigningError::from_app(&e))
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn revoke(&self) -> Result<(), AppError> {
|
async fn revoke(&self) -> Result<(), SigningError> {
|
||||||
Nip46ClientSigner::revoke(self).await
|
Nip46ClientSigner::revoke(self)
|
||||||
|
.await
|
||||||
|
.map_err(|e| SigningError::from_app(&e))
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn status_string(&self) -> String {
|
async fn status_string(&self) -> String {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue