fix(settings): confirm before forgetting a password other profiles share

"Forget Password" was a bare menu item: one click deleted the keychain
entry, with no confirmation and nothing said about who else it took
down. The entry is keyed by `user@host:port`, so two profiles that reach
the same endpoint — one direct, one through a jump host — share exactly
one secret, and forgetting from either row signed both of them out. The
notification even worded itself by endpoint while the action hung off a
single profile's menu.

It now asks first, the way deleting a profile does, and when the
endpoint is shared the dialog names the blast radius instead of leaving
it to turn up at the next connect on a host nobody touched.

Deleting a profile stays conservative on purpose — the menu that could
remove the secret is about to disappear — so the two paths keep their
different policies. What they no longer keep is two copies of the
"is this endpoint shared" question: `profiles_sharing_endpoint` is now
the one place that answers it, and it has the test.
This commit is contained in:
l0ng-ai
2026-08-11 19:47:07 +08:00
parent 0f5e63701e
commit f4f9b2bac4
5 changed files with 156 additions and 12 deletions
+13
View File
@@ -200,6 +200,13 @@ pub fn translate_en(key: L10nKey) -> &'static str {
L10nKey::SettingsCopyAddress => "Copy Address",
L10nKey::SettingsDuplicate => "Duplicate",
L10nKey::SettingsForgetPassword => "Forget Password",
L10nKey::SettingsForgetPasswordTitle => "Forget the saved password for {endpoint}?",
L10nKey::SettingsForgetPasswordBody => {
"The next connection to it asks for the password again. Nothing else about this host changes."
}
L10nKey::SettingsForgetPasswordSharedBody => {
"{count} other host profiles use {endpoint} as well, so those connections will have to enter the password again too."
}
L10nKey::SettingsForgotPasswordFor => "Forgot saved password for {endpoint}",
L10nKey::SettingsDeleteProfileBody => {
"The password saved for it goes too, unless another connection still uses the same address."
@@ -1599,6 +1606,12 @@ pub fn translate_variant_en(key: L10nKey, branch: &'static str) -> Option<&'stat
(L10nKey::SettingsOfflineMachines, "other") => {
"{count} more saved machines are not connected — open a workspace on one to install its hooks there."
}
(L10nKey::SettingsForgetPasswordSharedBody, "one") => {
"1 other host profile uses {endpoint} as well, so that connection will have to enter the password again too."
}
(L10nKey::SettingsForgetPasswordSharedBody, "other") => {
"{count} other host profiles use {endpoint} as well, so those connections will have to enter the password again too."
}
(L10nKey::SftpReplaceBody, "one") => {
"{names} already exists in this folder. Uploading overwrites it."
}
+13
View File
@@ -201,6 +201,13 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> {
L10nKey::SettingsCopyAddress => "アドレスをコピー",
L10nKey::SettingsDuplicate => "複製",
L10nKey::SettingsForgetPassword => "パスワードを消去",
L10nKey::SettingsForgetPasswordTitle => "{endpoint} の保存されたパスワードを消去しますか?",
L10nKey::SettingsForgetPasswordBody => {
"次に接続するときに、もう一度パスワードを尋ねられます。このホストの他の設定は変わりません"
}
L10nKey::SettingsForgetPasswordSharedBody => {
"他にも {count} 件のホストプロファイルが {endpoint} を使っているため、それらの接続でもパスワードの再入力が必要になります"
}
L10nKey::SettingsForgotPasswordFor => "{endpoint} の保存されたパスワードを消去しました",
L10nKey::SettingsDeleteProfileBody => {
"保存されたパスワードも一緒に削除されます。同じアドレスを使う接続が他にある場合は残ります。"
@@ -1644,6 +1651,12 @@ pub fn translate_variant_ja(key: L10nKey, branch: &'static str) -> Option<&'stat
(L10nKey::SettingsOfflineMachines, "other") => {
"未接続の保存済みマシンがさらに {count} 台あります — いずれかでワークスペースを開くと、そこにフックをインストールできます"
}
(L10nKey::SettingsForgetPasswordSharedBody, "one") => {
"他にも 1 件のホストプロファイルが {endpoint} を使っているため、その接続でもパスワードの再入力が必要になります"
}
(L10nKey::SettingsForgetPasswordSharedBody, "other") => {
"他にも {count} 件のホストプロファイルが {endpoint} を使っているため、それらの接続でもパスワードの再入力が必要になります"
}
(L10nKey::SftpReplaceBody, "one") => {
"{names} はこのフォルダに既に存在します。アップロードすると上書きされます。"
}
+4
View File
@@ -227,6 +227,9 @@ l10n_keys! {
SettingsCopyAddress,
SettingsDuplicate,
SettingsForgetPassword,
SettingsForgetPasswordTitle,
SettingsForgetPasswordBody,
SettingsForgetPasswordSharedBody,
SettingsForgotPasswordFor,
SettingsDeleteProfileBody,
SettingsCouldntForgetPassword,
@@ -1994,6 +1997,7 @@ mod tests {
L10nKey::SettingsAliasesLinked,
L10nKey::SettingsRulesOpenedWithConnection,
L10nKey::SettingsOfflineMachines,
L10nKey::SettingsForgetPasswordSharedBody,
L10nKey::PanelMoreChangedFiles,
L10nKey::ScmFilesChanged,
L10nKey::ScmStagedFileCount,
+13
View File
@@ -181,6 +181,13 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> {
L10nKey::SettingsCopyAddress => "复制地址",
L10nKey::SettingsDuplicate => "复制",
L10nKey::SettingsForgetPassword => "清除已保存的密码",
L10nKey::SettingsForgetPasswordTitle => "要清除 {endpoint} 的已保存密码吗?",
L10nKey::SettingsForgetPasswordBody => {
"下次连接它时会重新询问密码。这台主机的其他设置不受影响。"
}
L10nKey::SettingsForgetPasswordSharedBody => {
"还有 {count} 个主机配置同样使用 {endpoint},那些连接也需要重新输入密码。"
}
L10nKey::SettingsForgotPasswordFor => "已清除 {endpoint} 的已保存密码",
L10nKey::SettingsDeleteProfileBody => {
"为它保存的密码也会一并删除,除非还有别的连接用着同一个地址。"
@@ -1517,6 +1524,12 @@ pub fn translate_variant_zh(key: L10nKey, branch: &'static str) -> Option<&'stat
(L10nKey::SettingsOfflineMachines, "other") => {
"还有 {count} 台已保存的机器未连接——在其中一台上打开工作区,即可在那台机器上安装 hook。"
}
(L10nKey::SettingsForgetPasswordSharedBody, "one") => {
"还有 1 个主机配置同样使用 {endpoint},那个连接也需要重新输入密码。"
}
(L10nKey::SettingsForgetPasswordSharedBody, "other") => {
"还有 {count} 个主机配置同样使用 {endpoint},那些连接也需要重新输入密码。"
}
(L10nKey::SftpReplaceBody, "one") => "{names} 在这个文件夹里已经存在,上传会覆盖它。",
(L10nKey::SftpReplaceBody, "other") => "{names} 在这个文件夹里已经存在,上传会覆盖它们。",
(L10nKey::AppTabsNotRestored, "one") => "上次的 1 个标签页没能重新打开",
+113 -12
View File
@@ -797,6 +797,27 @@ fn ssh_row_matches(p: &SshProfile, query: &str) -> bool {
hit(&p.name) || hit(&p.host) || hit(&p.user) || hit(&p.port.to_string())
}
/// How many *other* profiles reach the same `user@host:port` as this one.
///
/// The keychain is keyed by the endpoint, not by the profile, so two hosts that
/// differ only in how they get there — one direct, one through a jump host —
/// hand the same saved password back and forth. Every path that is about to
/// remove that password has to know this first: deleting a profile keeps the
/// secret while someone else still needs it, and forgetting one says out loud
/// who else it takes down. Both used to work the answer out on their own, which
/// is exactly how the two policies would have drifted apart.
fn profiles_sharing_endpoint(cfg: &Config, id: Uuid) -> usize {
let Some(profile) = cfg.ssh_profiles.iter().find(|p| p.id == id) else {
return 0;
};
cfg.ssh_profiles
.iter()
.filter(|p| {
p.id != id && (&p.user, &p.host, p.port) == (&profile.user, &profile.host, profile.port)
})
.count()
}
pub(crate) struct SshProfileForm {
editing: Uuid,
carry_group: Option<String>,
@@ -2834,13 +2855,8 @@ impl Tty7App {
PopupMenuItem::new(t(L10nKey::SettingsForgetPassword)).on_click({
let app = app.clone();
move |_, window, cx| {
if let Some(msg) = app
.update(cx, |this, cx| this.forget_profile_password(id, cx))
.ok()
.flatten()
{
window.push_notification(msg, cx);
}
let _ =
app.update(cx, |this, cx| this.forget_profile_password(id, window, cx));
}
}),
)
@@ -3224,11 +3240,7 @@ impl Tty7App {
.iter()
.find(|p| p.id == id)
.map(|p| (p.user.clone(), p.host.clone(), p.port));
let shared = endpoint.as_ref().is_some_and(|(user, host, port)| {
cfg.ssh_profiles
.iter()
.any(|p| p.id != id && (&p.user, &p.host, p.port) == (user, host, *port))
});
let shared = profiles_sharing_endpoint(cfg, id) > 0;
if let Some((user, host, port)) = endpoint.filter(|_| !shared) {
use crate::core::keychain::{CredentialStore, OsCredentialStore};
let _ = OsCredentialStore.delete_password(&user, &host, port);
@@ -3270,6 +3282,60 @@ impl Tty7App {
}
pub(crate) fn forget_profile_password(
&mut self,
id: Uuid,
window: &mut Window,
cx: &mut Context<Self>,
) {
let cfg = cx.global::<Config>();
let Some(endpoint) = cfg
.ssh_profiles
.iter()
.find(|p| p.id == id)
.map(|p| format!("{}@{}:{}", p.user, p.host, p.port))
else {
return;
};
// One click used to be the whole gesture, and the thing it removed does
// not come back. Worse, the entry is the endpoint's rather than this
// row's, so a menu opened on one host can sign several of them out —
// name that count here instead of letting it turn up at the next
// connect on a host nobody touched.
let others = profiles_sharing_endpoint(cfg, id);
let body = if others == 0 {
t(L10nKey::SettingsForgetPasswordBody).to_string()
} else {
t_plural(
L10nKey::SettingsForgetPasswordSharedBody,
others,
&[("endpoint", &endpoint)],
)
};
let answer = window.prompt(
gpui::PromptLevel::Warning,
&t_fmt(
L10nKey::SettingsForgetPasswordTitle,
&[("endpoint", &endpoint)],
),
Some(&body),
&crate::ui::confirm_answers(t(L10nKey::SettingsForgetPassword), t(L10nKey::Cancel)),
cx,
);
cx.spawn_in(window, async move |this, cx| {
let Ok(0) = answer.await else { return };
// The notification is the only sign the keychain was touched, and
// by now the click that asked for it is long gone — so it has to be
// raised from in here, on the window the prompt belonged to.
let _ = this.update_in(cx, |this, window, cx| {
if let Some(msg) = this.forget_profile_password_confirmed(id, cx) {
window.push_notification(msg, cx);
}
});
})
.detach();
}
fn forget_profile_password_confirmed(
&mut self,
id: Uuid,
cx: &mut Context<Self>,
@@ -6578,6 +6644,41 @@ mod tests {
assert_eq!(hp.port, 2222);
assert_eq!(parse_host_port("host").unwrap().port, 0);
}
fn profile_at(name: &str, user: &str, host: &str, port: u16) -> SshProfile {
let mut p = SshProfile::new(name);
p.user = user.to_string();
p.host = host.to_string();
p.port = port;
p
}
/// The saved password belongs to `user@host:port`, so what counts as
/// "shared" is exactly that triple — a different name or a jump host in
/// front of it changes nothing, and a different port makes it a different
/// secret entirely.
#[test]
fn the_same_endpoint_under_two_names_counts_as_shared() {
let direct = profile_at("direct", "ana", "build.example.com", 22);
let mut via_jump = profile_at("via bastion", "ana", "build.example.com", 22);
via_jump.jump_host = Some(direct.id);
let staging = profile_at("staging", "ana", "build.example.com", 2222);
let other_user = profile_at("root", "root", "build.example.com", 22);
let mut cfg = Config::default();
let (direct_id, jump_id, staging_id) = (direct.id, via_jump.id, staging.id);
cfg.ssh_profiles = vec![direct, via_jump, staging, other_user];
// The two that reach the same endpoint see each other, and neither
// counts itself.
assert_eq!(profiles_sharing_endpoint(&cfg, direct_id), 1);
assert_eq!(profiles_sharing_endpoint(&cfg, jump_id), 1);
// A port apart is a keychain entry apart, so this one is alone even
// though the user and host match two of the others.
assert_eq!(profiles_sharing_endpoint(&cfg, staging_id), 0);
// A profile that is no longer on the list shares with nobody.
assert_eq!(profiles_sharing_endpoint(&cfg, Uuid::new_v4()), 0);
}
}
#[cfg(test)]