mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-22 08:02:24 +00:00
fix(settings): confirm before forgetting a password other profiles share (#510)
"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. Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>
This commit is contained in:
@@ -212,6 +212,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."
|
||||
@@ -1643,6 +1650,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."
|
||||
}
|
||||
|
||||
@@ -215,6 +215,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 => {
|
||||
"保存されたパスワードも一緒に削除されます。同じアドレスを使う接続が他にある場合は残ります。"
|
||||
@@ -1695,6 +1702,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} はこのフォルダに既に存在します。アップロードすると上書きされます。"
|
||||
}
|
||||
|
||||
@@ -233,6 +233,9 @@ l10n_keys! {
|
||||
SettingsCopyAddress,
|
||||
SettingsDuplicate,
|
||||
SettingsForgetPassword,
|
||||
SettingsForgetPasswordTitle,
|
||||
SettingsForgetPasswordBody,
|
||||
SettingsForgetPasswordSharedBody,
|
||||
SettingsForgotPasswordFor,
|
||||
SettingsDeleteProfileBody,
|
||||
SettingsCouldntForgetPassword,
|
||||
@@ -2013,6 +2016,7 @@ mod tests {
|
||||
L10nKey::SettingsImportIgnored,
|
||||
L10nKey::SettingsRulesOpenedWithConnection,
|
||||
L10nKey::SettingsOfflineMachines,
|
||||
L10nKey::SettingsForgetPasswordSharedBody,
|
||||
L10nKey::PanelMoreChangedFiles,
|
||||
L10nKey::ScmFilesChanged,
|
||||
L10nKey::ScmStagedFileCount,
|
||||
|
||||
@@ -191,6 +191,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 => {
|
||||
"为它保存的密码也会一并删除,除非还有别的连接用着同一个地址。"
|
||||
@@ -1556,6 +1563,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
@@ -798,6 +798,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>,
|
||||
@@ -3075,13 +3096,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));
|
||||
}
|
||||
}),
|
||||
)
|
||||
@@ -3470,11 +3486,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);
|
||||
@@ -3608,6 +3620,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>,
|
||||
@@ -7146,6 +7212,41 @@ mod tests {
|
||||
assert!(!errors.is_empty());
|
||||
assert_eq!(profile.name, "half typed");
|
||||
}
|
||||
|
||||
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)]
|
||||
|
||||
Reference in New Issue
Block a user