From a5cdddbb2b7bdd3b9a89edcb2d789c4902bc5753 Mon Sep 17 00:00:00 2001 From: kernelkind Date: Tue, 26 Nov 2024 20:49:02 -0500 Subject: [PATCH 1/2] user can upgrade their npub -> nsec closes https://github.com/damus-io/notedeck/issues/476 Signed-off-by: kernelkind --- src/accounts/mod.rs | 63 +++++++++++++++++++++++++++++++++------------ src/app.rs | 1 + src/test_data.rs | 8 +++--- 3 files changed, 52 insertions(+), 20 deletions(-) diff --git a/src/accounts/mod.rs b/src/accounts/mod.rs index d0e81f2a..584b3593 100644 --- a/src/accounts/mod.rs +++ b/src/accounts/mod.rs @@ -138,27 +138,45 @@ impl Accounts { } } - pub fn has_account_pubkey(&self, pubkey: &[u8; 32]) -> bool { - for account in &self.accounts { - if account.pubkey.bytes() == pubkey { - return true; + fn contains_account(&self, pubkey: &[u8; 32]) -> Option { + for (index, account) in self.accounts.iter().enumerate() { + let has_pubkey = account.pubkey.bytes() == pubkey; + let has_nsec = account.secret_key.is_some(); + if has_pubkey { + return Some(ContainsAccount { has_nsec, index }); } } - false + None } #[must_use = "UnknownIdAction's must be handled. Use .process_unknown_id_action()"] - pub fn add_account(&mut self, account: Keypair) -> SingleUnkIdAction { - if self.has_account_pubkey(account.pubkey.bytes()) { - info!("already have account, not adding {}", account.pubkey); - return SingleUnkIdAction::pubkey(account.pubkey); - } + pub fn add_account(&mut self, account: Keypair) -> LoginAction { + let pubkey = account.pubkey; + let switch_to_index = if let Some(contains_acc) = self.contains_account(pubkey.bytes()) { + if account.secret_key.is_some() && !contains_acc.has_nsec { + info!( + "user provided nsec, but we already have npub {}. Upgrading to nsec", + pubkey + ); + let _ = self.key_store.add_key(&account); - let _ = self.key_store.add_key(&account); - let pk = account.pubkey; - self.accounts.push(account); - SingleUnkIdAction::pubkey(pk) + self.accounts[contains_acc.index] = account; + } else { + info!("already have account, not adding {}", pubkey); + } + contains_acc.index + } else { + info!("adding new account {}", pubkey); + let _ = self.key_store.add_key(&account); + self.accounts.push(account); + self.accounts.len() - 1 + }; + + LoginAction { + unk: SingleUnkIdAction::pubkey(pubkey), + switch_to_index, + } } pub fn num_accounts(&self) -> usize { @@ -217,12 +235,23 @@ pub fn process_login_view_response( manager: &mut Accounts, response: AccountLoginResponse, ) -> SingleUnkIdAction { - let r = match response { + let login_action = match response { AccountLoginResponse::CreateNew => { manager.add_account(FullKeypair::generate().to_keypair()) } AccountLoginResponse::LoginWith(keypair) => manager.add_account(keypair), }; - manager.select_account(manager.num_accounts() - 1); - r + manager.select_account(login_action.switch_to_index); + login_action.unk +} + +pub struct LoginAction { + pub unk: SingleUnkIdAction, + pub switch_to_index: usize, +} + +#[derive(Default)] +struct ContainsAccount { + pub has_nsec: bool, + pub index: usize, } diff --git a/src/app.rs b/src/app.rs index 4e7ee14a..85c364bd 100644 --- a/src/app.rs +++ b/src/app.rs @@ -429,6 +429,7 @@ impl Damus { info!("adding account: {}", key.pubkey); accounts .add_account(key) + .unk .process_action(&mut unknown_ids, &ndb, &txn); } } diff --git a/src/test_data.rs b/src/test_data.rs index 8e95cf94..7b4a8b13 100644 --- a/src/test_data.rs +++ b/src/test_data.rs @@ -102,9 +102,11 @@ pub fn test_app() -> Damus { let accounts = get_test_accounts(); let txn = Transaction::new(&app.ndb).expect("txn"); for account in accounts { - app.accounts_mut() - .add_account(account) - .process_action(&mut app.unknown_ids, &app.ndb, &txn) + app.accounts_mut().add_account(account).unk.process_action( + &mut app.unknown_ids, + &app.ndb, + &txn, + ) } app From e69a7f83ae4fccd6b9d124d5daa083761ead6417 Mon Sep 17 00:00:00 2001 From: William Casarin Date: Wed, 27 Nov 2024 09:17:45 -0800 Subject: [PATCH 2/2] refactor: make LoginAction a bit safer We make LoginAction a simple wrapper around processing the unknown action to expose too much internal logic. This allows us to have a must_use on our LoginAction type, otherwise the SingleUnkIdAction's must_use will be lost when returned in the login action. Fixes: a5cdddbb2b7b ("user can upgrade their npub -> nsec") Signed-off-by: William Casarin --- src/accounts/mod.rs | 16 ++++++++++++++-- src/app.rs | 1 - src/test_data.rs | 8 +++----- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/accounts/mod.rs b/src/accounts/mod.rs index 584b3593..1c25c108 100644 --- a/src/accounts/mod.rs +++ b/src/accounts/mod.rs @@ -1,7 +1,7 @@ use std::cmp::Ordering; use enostr::{FilledKeypair, FullKeypair, Keypair}; -use nostrdb::Ndb; +use nostrdb::{Ndb, Transaction}; use crate::{ column::Columns, @@ -14,6 +14,7 @@ use crate::{ accounts::{AccountsView, AccountsViewResponse}, }, unknowns::SingleUnkIdAction, + unknowns::UnknownIds, user_account::UserAccount, }; use tracing::{error, info}; @@ -245,11 +246,22 @@ pub fn process_login_view_response( login_action.unk } +#[must_use = "You must call process_login_action on this to handle unknown ids"] pub struct LoginAction { - pub unk: SingleUnkIdAction, + unk: SingleUnkIdAction, pub switch_to_index: usize, } +impl LoginAction { + // Simple wrapper around processing the unknown action to expose too + // much internal logic. This allows us to have a must_use on our + // LoginAction type, otherwise the SingleUnkIdAction's must_use will + // be lost when returned in the login action + pub fn process_action(&mut self, ids: &mut UnknownIds, ndb: &Ndb, txn: &Transaction) { + self.unk.process_action(ids, ndb, txn); + } +} + #[derive(Default)] struct ContainsAccount { pub has_nsec: bool, diff --git a/src/app.rs b/src/app.rs index 85c364bd..4e7ee14a 100644 --- a/src/app.rs +++ b/src/app.rs @@ -429,7 +429,6 @@ impl Damus { info!("adding account: {}", key.pubkey); accounts .add_account(key) - .unk .process_action(&mut unknown_ids, &ndb, &txn); } } diff --git a/src/test_data.rs b/src/test_data.rs index 7b4a8b13..8e95cf94 100644 --- a/src/test_data.rs +++ b/src/test_data.rs @@ -102,11 +102,9 @@ pub fn test_app() -> Damus { let accounts = get_test_accounts(); let txn = Transaction::new(&app.ndb).expect("txn"); for account in accounts { - app.accounts_mut().add_account(account).unk.process_action( - &mut app.unknown_ids, - &app.ndb, - &txn, - ) + app.accounts_mut() + .add_account(account) + .process_action(&mut app.unknown_ids, &app.ndb, &txn) } app