compose/pgp: rewrite key selection logic

Rewrite key selection logic when opening the key selector widget for
signing or encryption keys:

- Fix encrypt_for_self incorrectly using the To: header email instead of
  From: (ugh!)
- Include all recipient addresses in selection (To:, Cc: Bcc:), also
  From:
- Use all addresses in a header value instead of just the first address
- Show shortcuts to sign/encrypt fields in Compose form

Signed-off-by: Manos Pitsidianakis <manos@pitsidianak.is>
This commit is contained in:
Manos Pitsidianakis 2024-11-30 16:18:42 +02:00
parent 6b3636013c
commit 3433c5c3d5
No known key found for this signature in database
GPG key ID: 7729C7707F7E09D0
2 changed files with 386 additions and 181 deletions

View file

@ -725,6 +725,12 @@ To: {}
let attachments_no = self.draft.attachments().len();
let theme_default = crate::conf::value(context, "theme_default");
grid.clear_area(area, theme_default);
let our_map: ShortcutMap =
account_settings!(context[self.account_hash].shortcuts.composing).key_values();
let mut shortcuts: ShortcutMaps = Default::default();
shortcuts.insert(Shortcuts::COMPOSING, our_map);
let toggle_shortcut = Key::Char('\n');
let edit_shortcut = &shortcuts[Shortcuts::COMPOSING]["edit"];
#[cfg(feature = "gpgme")]
if self
.gpg_state
@ -742,12 +748,14 @@ To: {}
grid.write_string(
&format!(
"☑ sign with {}",
"☑ sign with [toggle: {}, edit: {}] {}",
toggle_shortcut,
edit_shortcut,
if self.gpg_state.sign_keys.is_empty() {
"default key"
} else {
key_list.as_str()
}
},
),
theme_default.fg,
if self.cursor == Cursor::Sign {
@ -762,7 +770,10 @@ To: {}
);
} else {
grid.write_string(
"☐ don't sign",
&format!(
"☐ don't sign [toggle: {}, edit: {}]",
toggle_shortcut, edit_shortcut,
),
theme_default.fg,
if self.cursor == Cursor::Sign {
crate::conf::value(context, "highlight").bg
@ -792,12 +803,22 @@ To: {}
grid.write_string(
&format!(
"{}{}",
"{}{}{}",
if self.gpg_state.encrypt_keys.is_empty() {
"☐ no keys to encrypt with!"
"☐ no keys selected to encrypt with"
} else {
"☑ encrypt with "
"☑ encrypt with"
},
&format!(
" [toggle: {}, edit: {}]{}",
toggle_shortcut,
edit_shortcut,
if self.gpg_state.encrypt_keys.is_empty() {
""
} else {
" "
}
),
if self.gpg_state.encrypt_keys.is_empty() {
""
} else {
@ -817,7 +838,10 @@ To: {}
);
} else {
grid.write_string(
"☐ don't encrypt",
&format!(
"☐ don't encrypt [toggle: {}, edit: {}]",
toggle_shortcut, edit_shortcut,
),
theme_default.fg,
if self.cursor == Cursor::Encrypt {
crate::conf::value(context, "highlight").bg
@ -832,7 +856,7 @@ To: {}
}
if attachments_no == 0 {
grid.write_string(
"no attachments",
&format!("no attachments [edit: {}]", edit_shortcut),
theme_default.fg,
if self.cursor == Cursor::Attachments {
crate::conf::value(context, "highlight").bg
@ -846,7 +870,7 @@ To: {}
);
} else {
grid.write_string(
&format!("{} attachments ", attachments_no),
&format!("{} attachments [edit: {}]", attachments_no, edit_shortcut),
theme_default.fg,
if self.cursor == Cursor::Attachments {
crate::conf::value(context, "highlight").bg
@ -912,6 +936,36 @@ To: {}
}
}
}
#[cfg(feature = "gpgme")]
fn create_key_selection_widget(
&self,
secret: bool,
header: &HeaderName,
context: &Context,
) -> Result<gpg::KeySelectionLoading> {
let (_, mut list) = melib::email::parser::address::rfc2822address_list(
self.form.values()[header.as_str()].as_str().as_bytes(),
)
.map_err(|_err| -> Error { format!("No valid address in `{header}:`").into() })?;
if list.is_empty() {
return Err(format!("No valid address in `{header}:`").into());
}
let first = list.remove(0);
let patterns = (
first.get_email(),
list.into_iter()
.map(|addr| addr.get_email())
.collect::<Vec<String>>(),
);
gpg::KeySelectionLoading::new(
secret,
account_settings!(context[self.account_hash].pgp.allow_remote_lookup).is_true(),
patterns,
*account_settings!(context[self.account_hash].pgp.allow_remote_lookup),
context,
)
}
}
impl Component for Composer {
@ -978,12 +1032,27 @@ impl Component for Composer {
if self.dirty {
grid.clear_area(area.nth_row(0), crate::conf::value(context, "highlight"));
let our_map: ShortcutMap =
account_settings!(context[self.account_hash].shortcuts.composing).key_values();
let mut shortcuts: ShortcutMaps = Default::default();
shortcuts.insert(Shortcuts::COMPOSING, our_map);
let scroll_down_shortcut = &shortcuts[Shortcuts::COMPOSING]["scroll_down"];
let scroll_up_shortcut = &shortcuts[Shortcuts::COMPOSING]["scroll_up"];
let field_shortcut = Key::Char('\n');
let edit_shortcut = &shortcuts[Shortcuts::COMPOSING]["edit"];
grid.write_string(
if self.reply_context.is_some() {
"COMPOSING REPLY"
} else {
"COMPOSING MESSAGE"
},
&format!(
"COMPOSING {} [scroll down: {}, scroll up: {}, edit fields: {}, edit body: {}]",
if self.reply_context.is_some() {
"REPLY"
} else {
"MESSAGE"
},
scroll_down_shortcut,
scroll_up_shortcut,
field_shortcut,
edit_shortcut
),
crate::conf::value(context, "highlight").fg,
crate::conf::value(context, "highlight").bg,
crate::conf::value(context, "highlight").attrs,
@ -1049,11 +1118,11 @@ impl Component for Composer {
let stopped_message: String =
format!("Process with PID {} has stopped.", guard.child_pid);
let stopped_message_2: String = format!(
"-press '{}' (edit shortcut) to re-activate.",
"- re-activate '{}' (edit shortcut)",
shortcuts[Shortcuts::COMPOSING]["edit"]
);
const STOPPED_MESSAGE_3: &str =
"-press Ctrl-C to forcefully kill it and return to editor.";
"- press Ctrl-C to forcefully kill it and return to editor";
let max_len = std::cmp::max(
stopped_message.len(),
std::cmp::max(stopped_message_2.len(), STOPPED_MESSAGE_3.len()),
@ -1473,13 +1542,13 @@ impl Component for Composer {
ViewMode::SelectKey(is_encrypt, ref mut selector),
UIEvent::FinishedUIDialog(id, result),
) if *id == selector.id() => {
if let Some(Some(key)) = result.downcast_mut::<Option<melib::gpgme::Key>>() {
if let Some(Some(keys)) = result.downcast_mut::<Option<Vec<melib::gpgme::Key>>>() {
if *is_encrypt {
self.gpg_state.encrypt_keys.clear();
self.gpg_state.encrypt_keys.push(key.clone());
self.gpg_state.encrypt_keys = std::mem::take(keys);
} else {
self.gpg_state.sign_keys.clear();
self.gpg_state.sign_keys.push(key.clone());
self.gpg_state.sign_keys = std::mem::take(keys);
}
}
self.mode = ViewMode::Edit;
@ -1756,39 +1825,28 @@ impl Component for Composer {
&& shortcut!(key == shortcuts[Shortcuts::COMPOSING]["edit"]) =>
{
#[cfg(feature = "gpgme")]
match melib::email::parser::address::rfc2822address_list(
self.form.values()["From"].as_str().as_bytes(),
)
.map_err(|_err| -> Error { "No valid sender address in `From:`".into() })
.and_then(|(_, list)| {
list.first()
.cloned()
.ok_or_else(|| "No valid sender address in `From:`".into())
})
.and_then(|addr| {
gpg::KeySelection::new(
false,
account_settings!(context[self.account_hash].pgp.allow_remote_lookup)
.is_true(),
addr.get_email(),
*account_settings!(context[self.account_hash].pgp.allow_remote_lookup),
context,
)
}) {
Ok(widget) => {
self.gpg_state.sign_mail = Some(ActionFlag::from(true));
self.mode = ViewMode::SelectKey(false, widget);
}
Err(err) => {
context.replies.push_back(UIEvent::Notification {
title: Some("Could not list keys.".into()),
source: None,
body: format!("libgpgme error: {err}").into(),
kind: Some(NotificationType::Error(melib::error::ErrorKind::External)),
});
{
match self
.create_key_selection_widget(false, &HeaderName::FROM, context)
.map(Into::into)
{
Ok(widget) => {
self.gpg_state.sign_mail = Some(ActionFlag::from(true));
self.mode = ViewMode::SelectKey(false, widget);
}
Err(err) => {
context.replies.push_back(UIEvent::Notification {
title: Some("Could not list keys.".into()),
source: None,
body: format!("libgpgme error: {err}").into(),
kind: Some(NotificationType::Error(
melib::error::ErrorKind::External,
)),
});
}
}
self.set_dirty(true);
}
self.set_dirty(true);
return true;
}
UIEvent::Input(ref key)
@ -1797,39 +1855,63 @@ impl Component for Composer {
&& shortcut!(key == shortcuts[Shortcuts::COMPOSING]["edit"]) =>
{
#[cfg(feature = "gpgme")]
match melib::email::parser::address::rfc2822address_list(
self.form.values()["To"].as_str().as_bytes(),
)
.map_err(|_err| -> Error { "No valid recipient addresses in `To:`".into() })
.and_then(|(_, list)| {
list.first()
.cloned()
.ok_or_else(|| "No valid recipient addresses in `To:`".into())
})
.and_then(|addr| {
gpg::KeySelection::new(
false,
account_settings!(context[self.account_hash].pgp.allow_remote_lookup)
.is_true(),
addr.get_email(),
*account_settings!(context[self.account_hash].pgp.allow_remote_lookup),
context,
)
}) {
Ok(widget) => {
self.gpg_state.encrypt_mail = Some(ActionFlag::from(true));
self.mode = ViewMode::SelectKey(true, widget);
}
Err(err) => {
context.replies.push_back(UIEvent::Notification {
title: Some("Could not list keys.".into()),
source: None,
body: format!("libgpgme error: {err}").into(),
kind: Some(NotificationType::Error(melib::error::ErrorKind::External)),
{
let mut result =
self.create_key_selection_widget(false, &HeaderName::TO, context);
if !self.form.values()[HeaderName::CC.as_str()]
.as_str()
.is_empty()
{
result = result.and_then(|mut to_result| {
let cc_result =
self.create_key_selection_widget(false, &HeaderName::CC, context)?;
to_result.merge(cc_result);
Ok(to_result)
});
}
if !self.form.values()[HeaderName::BCC.as_str()]
.as_str()
.is_empty()
{
result = result.and_then(|mut to_result| {
let bcc_result =
self.create_key_selection_widget(false, &HeaderName::BCC, context)?;
to_result.merge(bcc_result);
Ok(to_result)
});
}
if !self.form.values()[HeaderName::FROM.as_str()]
.as_str()
.is_empty()
{
result = result.and_then(|mut to_result| {
let from_result = self.create_key_selection_widget(
false,
&HeaderName::FROM,
context,
)?;
to_result.merge(from_result);
Ok(to_result)
});
}
match result.map(Into::into) {
Ok(widget) => {
self.gpg_state.encrypt_mail = Some(ActionFlag::from(true));
self.mode = ViewMode::SelectKey(true, widget);
}
Err(err) => {
context.replies.push_back(UIEvent::Notification {
title: Some("Could not list keys.".into()),
source: None,
body: format!("libgpgme error: {err}").into(),
kind: Some(NotificationType::Error(
melib::error::ErrorKind::External,
)),
});
}
}
self.set_dirty(true);
}
self.set_dirty(true);
return true;
}
UIEvent::Input(ref key)
@ -2658,7 +2740,7 @@ pub fn send_draft_async(
gpg_state.encrypt_for_self.then_some(()).map_or_else(
|| Ok(None),
|()| {
draft.headers().get(HeaderName::TO).map_or_else(
draft.headers().get(HeaderName::FROM).map_or_else(
|| Ok(None),
|s| Some(melib::Address::try_from(s)).transpose(),
)

View file

@ -21,15 +21,94 @@
use super::*;
type KeylistJoinHandle = JoinHandle<Result<Vec<melib::gpgme::Key>>>;
#[derive(Debug)]
pub enum KeySelection {
LoadingKeys {
handle: JoinHandle<Result<Vec<melib::gpgme::Key>>>,
progress_spinner: ProgressSpinner,
pub struct KeySelectionLoading {
handles: (KeylistJoinHandle, Vec<KeylistJoinHandle>),
progress_spinner: ProgressSpinner,
secret: bool,
local: bool,
patterns: (String, Vec<String>),
allow_remote_lookup: ActionFlag,
}
impl KeySelectionLoading {
pub fn new(
secret: bool,
local: bool,
pattern: String,
patterns: (String, Vec<String>),
allow_remote_lookup: ActionFlag,
context: &Context,
) -> Result<Self> {
use melib::gpgme::{self, *};
let mut ctx = gpgme::Context::new()?;
if local {
ctx.set_auto_key_locate(LocateKey::LOCAL)?;
} else {
ctx.set_auto_key_locate(LocateKey::WKD | LocateKey::LOCAL)?;
}
let (pattern, other_patterns) = patterns;
let main_job = ctx.keylist(secret, Some(pattern.clone()))?;
let main_handle = context.main_loop_handler.job_executor.spawn(
"gpg::keylist".into(),
main_job,
IsAsync::Blocking,
);
let other_handles = other_patterns
.iter()
.map(|pattern| {
let job = ctx.keylist(secret, Some(pattern.clone()))?;
Ok(context.main_loop_handler.job_executor.spawn(
"gpg::keylist".into(),
job,
IsAsync::Blocking,
))
})
.collect::<Result<Vec<KeylistJoinHandle>>>()?;
let mut progress_spinner = ProgressSpinner::new(8, context);
progress_spinner.start();
Ok(Self {
handles: (main_handle, other_handles),
secret,
local,
patterns: (pattern, other_patterns),
allow_remote_lookup,
progress_spinner,
})
}
pub fn merge(&mut self, rhs: Self) {
let Self {
handles: (_, ref mut other_handles),
secret: _,
local: _,
patterns: (_, ref mut other_patterns),
allow_remote_lookup: _,
progress_spinner: _,
} = self;
let Self {
handles: (rhs_handle, rhs_other_handles),
patterns: (rhs_pattern, rhs_other_patterns),
secret: _,
local: _,
allow_remote_lookup: _,
progress_spinner: _,
} = rhs;
other_handles.push(rhs_handle);
other_handles.extend(rhs_other_handles);
other_patterns.push(rhs_pattern);
other_patterns.extend(rhs_other_patterns);
}
}
#[derive(Debug)]
pub enum KeySelection {
Loading {
inner: KeySelectionLoading,
/// Accumulate results from intermediate results (i.e. not the main
/// pattern)
keys_accumulator: Vec<melib::gpgme::Key>,
},
Error {
id: ComponentId,
@ -41,52 +120,31 @@ pub enum KeySelection {
},
}
impl From<KeySelectionLoading> for KeySelection {
fn from(inner: KeySelectionLoading) -> Self {
Self::Loading {
inner,
keys_accumulator: vec![],
}
}
}
impl std::fmt::Display for KeySelection {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(f, "select pgp keys")
}
}
impl KeySelection {
pub fn new(
secret: bool,
local: bool,
pattern: String,
allow_remote_lookup: ActionFlag,
context: &Context,
) -> Result<Self> {
use melib::gpgme::{self, *};
let mut ctx = gpgme::Context::new()?;
if local {
ctx.set_auto_key_locate(LocateKey::LOCAL)?;
} else {
ctx.set_auto_key_locate(LocateKey::WKD | LocateKey::LOCAL)?;
}
let job = ctx.keylist(secret, Some(pattern.clone()))?;
let handle = context.main_loop_handler.job_executor.spawn(
"gpg::keylist".into(),
job,
IsAsync::Blocking,
);
let mut progress_spinner = ProgressSpinner::new(8, context);
progress_spinner.start();
Ok(Self::LoadingKeys {
handle,
secret,
local,
pattern,
allow_remote_lookup,
progress_spinner,
})
}
}
impl Component for KeySelection {
fn draw(&mut self, grid: &mut CellBuffer, area: Area, context: &mut Context) {
match self {
Self::LoadingKeys {
ref mut progress_spinner,
..
Self::Loading {
inner:
KeySelectionLoading {
ref mut progress_spinner,
..
},
keys_accumulator: _,
} => progress_spinner.draw(grid, area.center_inside((2, 2)), context),
Self::Error { ref err, .. } => {
let theme_default = crate::conf::value(context, "theme_default");
@ -106,16 +164,30 @@ impl Component for KeySelection {
fn process_event(&mut self, event: &mut UIEvent, context: &mut Context) -> bool {
match self {
Self::LoadingKeys {
ref mut progress_spinner,
ref mut handle,
secret,
local,
ref mut pattern,
allow_remote_lookup,
..
Self::Loading {
inner:
KeySelectionLoading {
ref mut progress_spinner,
handles: (ref mut main_handle, ref mut other_handles),
secret,
local,
patterns: (ref mut pattern, ref mut other_patterns),
allow_remote_lookup,
..
},
ref mut keys_accumulator,
} => match event {
UIEvent::StatusEvent(StatusEvent::JobFinished(ref id)) if *id == handle.job_id => {
UIEvent::StatusEvent(StatusEvent::JobFinished(ref id))
if *id == main_handle.job_id
|| other_handles.iter().any(|h| h.job_id == *id) =>
{
let mut main_handle_ref = &mut (*main_handle);
let is_main = *id == main_handle_ref.job_id;
let handle = if is_main {
&mut main_handle_ref
} else {
&mut other_handles.iter_mut().find(|h| h.job_id == *id).unwrap()
};
match handle.chan.try_recv() {
Err(_) => { /* Job was canceled */ }
Ok(None) => { /* something happened, perhaps a worker thread panicked */ }
@ -123,15 +195,19 @@ impl Component for KeySelection {
if keys.is_empty() {
let id = progress_spinner.id();
if allow_remote_lookup.is_true() {
match Self::new(
match KeySelectionLoading::new(
*secret,
*local,
std::mem::take(pattern),
(std::mem::take(pattern), std::mem::take(other_patterns)),
*allow_remote_lookup,
context,
) {
Ok(w) => {
*self = w;
Ok(inner) => {
let keys_accumulator = std::mem::take(keys_accumulator);
*self = Self::Loading {
inner,
keys_accumulator,
};
}
Err(err) => *self = Self::Error { err, id },
}
@ -157,42 +233,74 @@ impl Component for KeySelection {
context.replies.push_back(UIEvent::StatusEvent(
StatusEvent::DisplayMessage(err.to_string()),
));
let res: Option<melib::gpgme::Key> = None;
// Even in case of error, we should send a FinishedUIDialog
// event so that the component parent knows we're done.
let res: Option<Vec<melib::gpgme::Key>> = None;
context
.replies
.push_back(UIEvent::FinishedUIDialog(id, Box::new(res)));
}
return false;
}
let mut widget = Box::new(UIDialog::new(
"select key",
keys.iter()
.map(|k| {
(
k.clone(),
if let Some(primary_uid) = k.primary_uid() {
format!("{} {}", k.fingerprint(), primary_uid)
} else {
k.fingerprint().to_string()
},
)
})
.collect::<Vec<(melib::gpgme::Key, String)>>(),
true,
Some(Box::new(
move |id: ComponentId, results: &[melib::gpgme::Key]| {
Some(UIEvent::FinishedUIDialog(
id,
Box::new(results.first().cloned()),
))
},
)),
context,
));
widget.set_dirty(true);
*self = Self::Loaded { widget, keys };
keys_accumulator.extend(keys);
if !is_main {
other_handles.retain(|h| h.job_id != *id);
return false;
}
if other_handles.is_empty() {
// We are done with all Futures, so finally transition into the
// "show the user the list of keys to select" state.
let mut widget = Box::new(UIDialog::new(
"select key",
keys_accumulator
.iter()
.map(|k| {
(
k.clone(),
if let Some(primary_uid) = k.primary_uid() {
format!("{} {}", k.fingerprint(), primary_uid)
} else {
k.fingerprint().to_string()
},
)
})
.collect::<Vec<(melib::gpgme::Key, String)>>(),
false,
Some(Box::new(
move |id: ComponentId, results: &[melib::gpgme::Key]| {
Some(UIEvent::FinishedUIDialog(
id,
Box::new(if results.is_empty() {
None
} else {
Some(results.to_vec())
}),
))
},
)),
context,
));
widget.set_dirty(true);
*self = Self::Loaded {
widget,
keys: std::mem::take(keys_accumulator),
};
} else {
// Main handle has finished, replace it with some other one from
// other_handles.
*main_handle = other_handles.remove(0);
}
}
Ok(Some(Err(err))) => {
context.replies.push_back(UIEvent::StatusEvent(
StatusEvent::DisplayMessage(err.to_string()),
));
// Even in case of error, we should send a FinishedUIDialog
// event so that the component parent knows we're done.
let res: Option<Vec<melib::gpgme::Key>> = None;
context
.replies
.push_back(UIEvent::FinishedUIDialog(self.id(), Box::new(res)));
*self = Self::Error {
err,
id: ComponentId::default(),
@ -210,9 +318,13 @@ impl Component for KeySelection {
fn is_dirty(&self) -> bool {
match self {
Self::LoadingKeys {
ref progress_spinner,
..
Self::Loading {
inner:
KeySelectionLoading {
ref progress_spinner,
..
},
keys_accumulator: _,
} => progress_spinner.is_dirty(),
Self::Error { .. } => true,
Self::Loaded { ref widget, .. } => widget.is_dirty(),
@ -221,9 +333,13 @@ impl Component for KeySelection {
fn set_dirty(&mut self, value: bool) {
match self {
Self::LoadingKeys {
ref mut progress_spinner,
..
Self::Loading {
inner:
KeySelectionLoading {
ref mut progress_spinner,
..
},
keys_accumulator: _,
} => progress_spinner.set_dirty(value),
Self::Error { .. } => {}
Self::Loaded { ref mut widget, .. } => widget.set_dirty(value),
@ -234,16 +350,20 @@ impl Component for KeySelection {
fn shortcuts(&self, context: &Context) -> ShortcutMaps {
match self {
Self::LoadingKeys { .. } | Self::Error { .. } => ShortcutMaps::default(),
Self::Loading { .. } | Self::Error { .. } => ShortcutMaps::default(),
Self::Loaded { ref widget, .. } => widget.shortcuts(context),
}
}
fn id(&self) -> ComponentId {
match self {
Self::LoadingKeys {
ref progress_spinner,
..
Self::Loading {
inner:
KeySelectionLoading {
ref progress_spinner,
..
},
keys_accumulator: _,
} => progress_spinner.id(),
Self::Error { ref id, .. } => *id,
Self::Loaded { ref widget, .. } => widget.id(),
@ -303,13 +423,16 @@ mod tests {
);
let mut progress_spinner = ProgressSpinner::new(8, context);
progress_spinner.start();
Ok(Self::LoadingKeys {
handle,
secret,
local,
pattern,
allow_remote_lookup,
progress_spinner,
Ok(Self::Loading {
inner: KeySelectionLoading {
handles: (handle, vec![]),
secret,
local,
patterns: (pattern, vec![]),
allow_remote_lookup,
progress_spinner,
},
keys_accumulator: vec![],
})
}
}