secret: Handle per-session errors and migrations

Per-session errors should never happen in practice, it should be fine
to just log them.
Migrations already swallow any errors.
This commit is contained in:
Kévin Commaille 2023-11-18 13:36:52 +01:00 committed by Kévin Commaille
parent d26c313542
commit 5e03a39f41
6 changed files with 105 additions and 253 deletions

View file

@ -14,7 +14,6 @@ src/components/loading_row.ui
src/components/location_viewer.rs
src/components/media_content_viewer.rs
src/contrib/qr_code_scanner/mod.ui
src/error_page.rs
src/error_page.ui
src/greeter.rs
src/greeter.ui

View file

@ -1,27 +1,19 @@
use adw::subclass::prelude::BinImpl;
use gettextrs::gettext;
use gtk::{self, glib, glib::clone, prelude::*, subclass::prelude::*, CompositeTemplate};
use tracing::error;
use crate::{spawn, toast, Window};
use adw::subclass::prelude::*;
use gtk::{self, glib, CompositeTemplate};
pub enum ErrorSubpage {
SecretErrorSession,
SecretErrorOther,
SecretError,
}
impl AsRef<str> for ErrorSubpage {
fn as_ref(&self) -> &str {
impl ErrorSubpage {
fn as_str(&self) -> &str {
match self {
Self::SecretErrorSession => "secret-error-session",
Self::SecretErrorOther => "secret-error-other",
Self::SecretError => "secret-error",
}
}
}
mod imp {
use std::cell::RefCell;
use glib::subclass::InitializingObject;
use super::*;
@ -30,10 +22,9 @@ mod imp {
#[template(resource = "/org/gnome/Fractal/ui/error_page.ui")]
pub struct ErrorPage {
#[template_child]
pub page: TemplateChild<adw::StatusPage>,
pub secret_error_page: TemplateChild<adw::StatusPage>,
#[template_child]
pub stack: TemplateChild<gtk::Stack>,
pub secret_item: RefCell<Option<oo7::Item>>,
}
#[glib::object_subclass]
@ -45,15 +36,6 @@ mod imp {
fn class_init(klass: &mut Self::Class) {
Self::bind_template(klass);
klass.set_accessible_role(gtk::AccessibleRole::Group);
klass.install_action(
"error-page.remove-secret-error-session",
None,
|obj, _, _| {
spawn!(clone!(@weak obj => async move {
obj.remove_secret_error_session().await;
}));
},
);
}
fn instance_init(obj: &InitializingObject<Self>) {
@ -61,19 +43,13 @@ mod imp {
}
}
impl ObjectImpl for ErrorPage {
fn constructed(&self) {
self.obj()
.action_set_enabled("error-page.remove-secret-error-session", false);
}
}
impl ObjectImpl for ErrorPage {}
impl WidgetImpl for ErrorPage {}
impl BinImpl for ErrorPage {}
}
glib::wrapper! {
/// A view displaying an error.
pub struct ErrorPage(ObjectSubclass<imp::ErrorPage>)
@extends gtk::Widget, adw::Bin, @implements gtk::Accessible;
}
@ -83,41 +59,10 @@ impl ErrorPage {
glib::Object::new()
}
pub fn display_secret_error(&self, message: &str, item: Option<oo7::Item>) {
pub fn display_secret_error(&self, message: &str) {
let imp = self.imp();
self.action_set_enabled("error-page.remove-secret-error-session", item.is_some());
imp.page.set_description(Some(message));
let error_subpage = if item.is_some() {
ErrorSubpage::SecretErrorSession
} else {
ErrorSubpage::SecretErrorOther
};
imp.stack.set_visible_child_name(error_subpage.as_ref());
imp.secret_item.replace(item);
}
async fn remove_secret_error_session(&self) {
let Some(item) = self.imp().secret_item.take() else {
return;
};
match item.delete().await {
Ok(_) => {
self.action_set_enabled("error-page.remove-secret-error-session", false);
if let Some(window) = self.root().and_downcast_ref::<Window>() {
toast!(self, gettext("Session removed successfully."));
window.restore_sessions().await;
}
}
Err(error) => {
error!("Could not remove session from secret storage: {error}");
toast!(
self,
gettext("Could not remove session from secret storage")
);
}
}
imp.secret_error_page.set_description(Some(message));
imp.stack
.set_visible_child_name(ErrorSubpage::SecretError.as_str());
}
}

View file

@ -13,88 +13,18 @@
</object>
</child>
<property name="content">
<object class="AdwStatusPage" id="page">
<property name="title" translatable="yes">Secret Service Error</property>
<property name="icon-name">key-symbolic</property>
<property name="vexpand">true</property>
<object class="GtkStack" id="stack">
<child>
<object class="AdwClamp">
<child>
<object class="GtkStack" id="stack">
<object class="GtkStackPage">
<property name="name">secret-error</property>
<property name="child">
<object class="AdwStatusPage" id="secret_error_page">
<property name="title" translatable="yes">Secret Service Error</property>
<property name="icon-name">key-symbolic</property>
<property name="vexpand">true</property>
<child>
<object class="GtkStackPage">
<property name="name">secret-error-session</property>
<property name="child">
<object class="GtkBox">
<property name="orientation">vertical</property>
<property name="spacing">24</property>
<child>
<object class="GtkLabel">
<style>
<class name="large-line-height"/>
</style>
<property name="wrap">true</property>
<property name="wrap-mode">word-char</property>
<property name="xalign">0.0</property>
<property name="label" translatable="yes">It seems like one of the Fractal sessions stored in the Secret Service is corrupted. If you know how to fix it you should do so.</property>
</object>
</child>
<child>
<object class="GtkLabel">
<style>
<class name="large-line-height"/>
</style>
<property name="wrap">true</property>
<property name="wrap-mode">word-char</property>
<property name="xalign">0.0</property>
<property name="label" translatable="yes">Alternatively, we could disconnect this session for you. This means you will have to login again and you will lose access to your encrypted messages, unless you have a session open in another client or you have already backed up your encryption keys.</property>
</object>
</child>
<child>
<object class="GtkBox">
<style>
<class name="warning"/>
</style>
<property name="halign">center</property>
<property name="spacing">6</property>
<child>
<object class="GtkImage">
<property name="icon-name">warning-symbolic</property>
</object>
</child>
<child>
<object class="GtkLabel">
<style>
<class name="large-line-height"/>
</style>
<property name="wrap">true</property>
<property name="wrap-mode">word-char</property>
<property name="xalign">0.0</property>
<property name="label" translatable="yes">Clicking this button might disconnect more than one session!</property>
</object>
</child>
</object>
</child>
<child>
<object class="GtkButton">
<style>
<class name="destructive-action"/>
<class name="pill"/>
</style>
<property name="halign">center</property>
<property name="can-shrink">true</property>
<property name="label" translatable="yes">Disconnect Session</property>
<property name="action-name">error-page.remove-secret-error-session</property>
</object>
</child>
</object>
</property>
</object>
</child>
<child>
<object class="GtkStackPage">
<property name="name">secret-error-other</property>
<property name="child">
<object class="AdwClamp">
<child>
<object class="GtkBox">
<property name="orientation">vertical</property>
<property name="spacing">24</property>
@ -188,11 +118,11 @@
</object>
</child>
</object>
</property>
</child>
</object>
</child>
</object>
</child>
</property>
</object>
</child>
</object>

View file

@ -507,14 +507,8 @@ impl Login {
let handle = spawn_tokio!(async move { session_info.store().await });
if let Err(error) = handle.await.unwrap() {
error!("Couldn't store session: {error}");
let (message, item) = error.into_parts();
self.parent_window().switch_to_error_page(
&format!("{}\n\n{}", gettext("Unable to store session"), message),
item,
);
return;
error!("Could not store session: {error}");
toast!(self, gettext("Unable to store session"));
}
session.connect_ready(clone!(@weak self as obj => move |_| {

View file

@ -47,9 +47,12 @@ pub enum SecretError {
#[error("Session found with old version")]
OldVersion { item: Item, session: StoredSession },
/// A corrupted session was found.
#[error("{error}")]
CorruptSession { error: String, item: Item },
/// An invalid session was found.
///
/// This should only happen if for some reason we get an item from a
/// different application.
#[error("Invalid session: {0}")]
Invalid(String),
/// An error occurred interacting with the secret service.
#[error(transparent)]
@ -60,22 +63,18 @@ pub enum SecretError {
WrongProfile,
}
impl SecretError {
/// Split `self` between its message and its optional `Item`.
pub fn into_parts(self) -> (String, Option<Item>) {
impl UserFacingError for SecretError {
fn to_user_facing(self) -> String {
match self {
SecretError::UnsupportedVersion { version, item, .. } => (
gettext_f(
// Translators: Do NOT translate the content between '{' and '}', this is a
// variable name.
"Found stored session with unsupported version {version_nb}",
&[("version_nb", &version.to_string())],
),
Some(item),
SecretError::UnsupportedVersion { version, .. } => gettext_f(
// Translators: Do NOT translate the content between '{' and '}', this is a
// variable name.
"Found stored session with unsupported version {version_nb}",
&[("version_nb", &version.to_string())],
),
SecretError::CorruptSession { error, item } => (error, Some(item)),
SecretError::Oo7(error) => (error.to_user_facing(), None),
error => (error.to_string(), None),
SecretError::Invalid(error) => error,
SecretError::Oo7(error) => error.to_user_facing(),
error => error.to_string(),
}
}
}
@ -195,10 +194,9 @@ impl StoredSession {
Ok(version) => version,
Err(error) => {
error!("Could not parse 'version' attribute in stored session: {error}");
return Err(SecretError::CorruptSession {
error: gettext("Malformed version in stored session"),
item,
});
return Err(SecretError::Invalid(gettext(
"Malformed version in stored session",
)));
}
},
None => 0,
@ -219,10 +217,9 @@ impl StoredSession {
// It's an error if the version is at least 2 but there is no profile.
// Versions older than 2 will be migrated.
None if version >= 2 => {
return Err(SecretError::CorruptSession {
error: gettext("Could not find profile in stored session"),
item,
});
return Err(SecretError::Invalid(gettext(
"Could not find profile in stored session",
)));
}
// No issue for other cases.
_ => {}
@ -233,17 +230,15 @@ impl StoredSession {
Ok(homeserver) => homeserver,
Err(error) => {
error!("Could not parse 'homeserver' attribute in stored session: {error}");
return Err(SecretError::CorruptSession {
error: gettext("Malformed homeserver in stored session"),
item,
});
return Err(SecretError::Invalid(gettext(
"Malformed homeserver in stored session",
)));
}
},
None => {
return Err(SecretError::CorruptSession {
error: gettext("Could not find homeserver in stored session"),
item,
});
return Err(SecretError::Invalid(gettext(
"Could not find homeserver in stored session",
)));
}
};
let user_id = match attr.get("user") {
@ -251,35 +246,31 @@ impl StoredSession {
Ok(user_id) => user_id,
Err(error) => {
error!("Could not parse 'user' attribute in stored session: {error}");
return Err(SecretError::CorruptSession {
error: gettext("Malformed user ID in stored session"),
item,
});
return Err(SecretError::Invalid(gettext(
"Malformed user ID in stored session",
)));
}
},
None => {
return Err(SecretError::CorruptSession {
error: gettext("Could not find user ID in stored session"),
item,
});
return Err(SecretError::Invalid(gettext(
"Could not find user ID in stored session",
)));
}
};
let device_id = match attr.get("device-id") {
Some(string) => <&DeviceId>::from(string.as_str()).to_owned(),
None => {
return Err(SecretError::CorruptSession {
error: gettext("Could not find device ID in stored session"),
item,
});
return Err(SecretError::Invalid(gettext(
"Could not find device ID in stored session",
)));
}
};
let path = match attr.get("db-path") {
Some(string) => PathBuf::from(string),
None => {
return Err(SecretError::CorruptSession {
error: gettext("Could not find database path in stored session"),
item,
});
return Err(SecretError::Invalid(gettext(
"Could not find database path in stored session",
)));
}
};
let secret = match item.secret().await {
@ -289,10 +280,9 @@ impl StoredSession {
Ok(secret) => secret,
Err(error) => {
error!("Could not parse secret in stored session: {error:?}");
return Err(SecretError::CorruptSession {
error: gettext("Malformed secret in stored session"),
item,
});
return Err(SecretError::Invalid(gettext(
"Malformed secret in stored session",
)));
}
}
} else {
@ -300,20 +290,18 @@ impl StoredSession {
Ok(secret) => secret,
Err(error) => {
error!("Could not parse secret in stored session: {error}");
return Err(SecretError::CorruptSession {
error: gettext("Malformed secret in stored session"),
item,
});
return Err(SecretError::Invalid(gettext(
"Malformed secret in stored session",
)));
}
}
}
}
Err(error) => {
error!("Could not get secret in stored session: {error}");
return Err(SecretError::CorruptSession {
error: gettext("Could not get secret in stored session"),
item,
});
return Err(SecretError::Invalid(gettext(
"Could not get secret in stored session",
)));
}
};
@ -493,7 +481,7 @@ impl StoredSession {
/// Migrate this session to version 4.
///
/// This implies moving the database under Fractal's directory.
pub async fn migrate_to_v4(mut self, item: Item) {
pub async fn migrate_to_v4(&mut self, item: Item) {
warn!(
"Session {} with version {} found for user {}, migrating to version 4…",
self.id(),
@ -519,12 +507,13 @@ impl StoredSession {
self.version = 4;
let clone = self.clone();
spawn_tokio!(async move {
if let Err(error) = item.delete().await {
error!("Failed to remove outdated session: {error}");
}
if let Err(error) = self.store().await {
if let Err(error) = clone.store().await {
error!("Failed to store updated session: {error}");
}
})
@ -584,8 +573,22 @@ pub async fn restore_sessions() -> Result<Vec<StoredSession>, SecretError> {
match StoredSession::try_from_secret_item(item).await {
Ok(session) => sessions.push(session),
Err(SecretError::OldVersion { item, mut session }) => {
if session.version == 0 {
warn!(
"Found old session for {} with sled store, removing…",
session.user_id
);
session.delete(Some(item), true).await;
} else if session.version < 4 {
session.migrate_to_v4(item).await;
sessions.push(session);
}
}
Err(SecretError::WrongProfile) => {}
Err(error) => return Err(error),
Err(error) => {
error!("Failed to restore previous session: {error}");
}
}
}

View file

@ -13,7 +13,7 @@ use crate::{
greeter::Greeter,
login::Login,
prelude::*,
secret::{self, SecretError, StoredSession},
secret::{self, StoredSession},
session::{
model::{Session, SessionState},
view::{AccountSettings, SessionView},
@ -349,34 +349,15 @@ impl Window {
}
}
}
Err(error) => match error {
SecretError::OldVersion { item, session } => {
if session.version == 0 {
warn!("Found old session with sled store, removing…");
session.delete(Some(item), true).await
} else if session.version < 4 {
session.migrate_to_v4(item).await
}
Err(error) => {
error!("Failed to restore previous sessions: {error}");
// Restart.
spawn!(clone!(@weak self as obj => async move {
obj.restore_sessions().await;
}));
}
_ => {
error!("Failed to restore previous sessions: {error}");
let (message, item) = error.into_parts();
self.switch_to_error_page(
&format!(
"{}\n\n{}",
gettext("Failed to restore previous sessions"),
message,
),
item,
);
}
},
self.switch_to_error_page(&format!(
"{}\n\n{}",
gettext("Failed to restore previous sessions"),
error.to_user_facing(),
));
}
}
}
@ -481,9 +462,9 @@ impl Window {
imp.main_stack.set_visible_child(&*imp.greeter);
}
pub fn switch_to_error_page(&self, message: &str, item: Option<oo7::Item>) {
pub fn switch_to_error_page(&self, message: &str) {
let imp = self.imp();
imp.error_page.display_secret_error(message, item);
imp.error_page.display_secret_error(message);
imp.main_stack.set_visible_child(&*imp.error_page);
}