Fix fill-password: wrong host key, and silent failure
Two bugs made 'Fill saved password' do nothing: 1. HOST MISMATCH. Saving used location.hostname at submit time (e.g. accounts.google.com); filling derived the host from the app's configured URL (mail.google.com). The Keychain lookup always found nothing for any service with a separate auth domain. Fixed by storing the save-time hostname in App.saved_host and reading it back at fill time, falling back to default_scope for entries saved before this. 2. SILENT FAILURE. Every error path in the fillpw sentinel called only eprintln!, which is invisible to the user. A feature that fails silently is indistinguishable from one that is not wired up. Fixed by emitting a fill-error event; the shell shows it as a toast in the nav column, which is left of the stage and therefore always above native views.
This commit is contained in:
@@ -333,6 +333,7 @@ pub fn add_app(
|
|||||||
hidden: Vec::new(),
|
hidden: Vec::new(),
|
||||||
icon: None,
|
icon: None,
|
||||||
saved_account: None,
|
saved_account: None,
|
||||||
|
saved_host: None,
|
||||||
zoom: 1.0,
|
zoom: 1.0,
|
||||||
order,
|
order,
|
||||||
};
|
};
|
||||||
@@ -947,6 +948,7 @@ pub fn save_password(state: State<'_, AppState>) -> Result<String, String> {
|
|||||||
let mut cfg = state.config.lock().unwrap();
|
let mut cfg = state.config.lock().unwrap();
|
||||||
if let Some(a) = cfg.apps.iter_mut().find(|a| a.id == app_id) {
|
if let Some(a) = cfg.apps.iter_mut().find(|a| a.id == app_id) {
|
||||||
a.saved_account = Some(account);
|
a.saved_account = Some(account);
|
||||||
|
a.saved_host = Some(host.clone());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
state.persist()?;
|
state.persist()?;
|
||||||
@@ -977,7 +979,17 @@ pub fn fill_password_for(
|
|||||||
.saved_account
|
.saved_account
|
||||||
.clone()
|
.clone()
|
||||||
.ok_or("no password saved for this app")?;
|
.ok_or("no password saved for this app")?;
|
||||||
let host = config::default_scope(&target.url).ok_or("this app has no host")?;
|
// Prefer the hostname that was live when the credential was saved
|
||||||
|
// (`location.hostname` at submit time). For services with a separate auth
|
||||||
|
// domain (Google, Microsoft, Okta…) that differs from the app's configured
|
||||||
|
// URL, so looking up by the configured URL's host always finds nothing.
|
||||||
|
// Fall back to deriving from the URL for entries saved before this field
|
||||||
|
// was added.
|
||||||
|
let host = target
|
||||||
|
.saved_host
|
||||||
|
.clone()
|
||||||
|
.or_else(|| config::default_scope(&target.url))
|
||||||
|
.ok_or("this app has no host")?;
|
||||||
|
|
||||||
#[cfg(target_os = "macos")]
|
#[cfg(target_os = "macos")]
|
||||||
let password = {
|
let password = {
|
||||||
|
|||||||
@@ -41,6 +41,12 @@ pub struct App {
|
|||||||
/// the Keychain — but without it the Keychain cannot be asked for the item.
|
/// the Keychain — but without it the Keychain cannot be asked for the item.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub saved_account: Option<String>,
|
pub saved_account: Option<String>,
|
||||||
|
/// The hostname the credential was saved under — `location.hostname` at
|
||||||
|
/// submit time, which is often different from the app's configured URL
|
||||||
|
/// (e.g. `accounts.google.com` vs `mail.google.com`). Stored so the fill
|
||||||
|
/// path can look up the same key the save path wrote.
|
||||||
|
#[serde(default)]
|
||||||
|
pub saved_host: Option<String>,
|
||||||
/// Page zoom, remembered per app: a dense ERP and a mail client do not
|
/// Page zoom, remembered per app: a dense ERP and a mail client do not
|
||||||
/// want the same size.
|
/// want the same size.
|
||||||
#[serde(default = "default_zoom")]
|
#[serde(default = "default_zoom")]
|
||||||
@@ -221,6 +227,7 @@ pub fn seed() -> Config {
|
|||||||
hidden: Vec::new(),
|
hidden: Vec::new(),
|
||||||
icon: None,
|
icon: None,
|
||||||
saved_account: None,
|
saved_account: None,
|
||||||
|
saved_host: None,
|
||||||
zoom: 1.0,
|
zoom: 1.0,
|
||||||
order,
|
order,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -295,7 +295,7 @@ fn handle_sentinel(
|
|||||||
"fillpw" => {
|
"fillpw" => {
|
||||||
let state = handle.state::<crate::commands::AppState>();
|
let state = handle.state::<crate::commands::AppState>();
|
||||||
if let Err(e) = crate::commands::fill_password_for(&handle, &from, &state) {
|
if let Err(e) = crate::commands::fill_password_for(&handle, &from, &state) {
|
||||||
eprintln!("could not fill a password: {e}");
|
let _ = handle.emit("fill-error", e);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1084,6 +1084,7 @@ mod tests {
|
|||||||
hidden: vec![".ad".into()],
|
hidden: vec![".ad".into()],
|
||||||
icon: None,
|
icon: None,
|
||||||
saved_account: None,
|
saved_account: None,
|
||||||
|
saved_host: None,
|
||||||
zoom: 1.0,
|
zoom: 1.0,
|
||||||
order: 0,
|
order: 0,
|
||||||
};
|
};
|
||||||
|
|||||||
+21
@@ -32,6 +32,7 @@ export default function App() {
|
|||||||
const [downloads, setDownloads] = useState<Download[]>([]);
|
const [downloads, setDownloads] = useState<Download[]>([]);
|
||||||
const [offer, setOffer] = useState<PasswordOffer | null>(null);
|
const [offer, setOffer] = useState<PasswordOffer | null>(null);
|
||||||
const [saved, setSaved] = useState<string | null>(null);
|
const [saved, setSaved] = useState<string | null>(null);
|
||||||
|
const [fillError, setFillError] = useState<string | null>(null);
|
||||||
|
|
||||||
const stageRef = useRef<HTMLDivElement>(null);
|
const stageRef = useRef<HTMLDivElement>(null);
|
||||||
const booted = useRef(false);
|
const booted = useRef(false);
|
||||||
@@ -147,6 +148,11 @@ export default function App() {
|
|||||||
}),
|
}),
|
||||||
// A login was submitted. The password is held in Rust; this only asks.
|
// A login was submitted. The password is held in Rust; this only asks.
|
||||||
listen<PasswordOffer>("password-offer", (e) => setOffer(e.payload)),
|
listen<PasswordOffer>("password-offer", (e) => setOffer(e.payload)),
|
||||||
|
// Fill failed — show the reason rather than doing nothing silently.
|
||||||
|
listen<string>("fill-error", (e) => {
|
||||||
|
setFillError(e.payload);
|
||||||
|
setTimeout(() => setFillError(null), 5000);
|
||||||
|
}),
|
||||||
|
|
||||||
listen<{ id: number; name: string; path: string }>("download-started", (e) =>
|
listen<{ id: number; name: string; path: string }>("download-started", (e) =>
|
||||||
setDownloads((d) => [
|
setDownloads((d) => [
|
||||||
@@ -322,6 +328,21 @@ export default function App() {
|
|||||||
</Dialog>
|
</Dialog>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
|
{fillError && (
|
||||||
|
/* Positioned within the nav column (left of the stage), so it is
|
||||||
|
always above native views. The stage starts at x = collapsed ? rail
|
||||||
|
: PANEL; this toast stays well within that. */
|
||||||
|
<div
|
||||||
|
role="alert"
|
||||||
|
style={{ width: collapsed ? rail : 240 }}
|
||||||
|
className="fixed bottom-4 left-0 z-[80] px-3"
|
||||||
|
>
|
||||||
|
<div className="rounded-lg border border-red-200 bg-red-50 px-3 py-2 text-[12px] leading-snug text-red-700 shadow-md dark:border-red-900 dark:bg-red-950/60 dark:text-red-300">
|
||||||
|
{fillError}
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
|
|
||||||
{settingsOpen && (
|
{settingsOpen && (
|
||||||
<Settings
|
<Settings
|
||||||
config={config}
|
config={config}
|
||||||
|
|||||||
Reference in New Issue
Block a user