From 79c5176487fbbd6b8a84ea70409e1f643b0c43b4 Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Mon, 6 Apr 2026 11:53:16 -0700 Subject: [PATCH] Stop storing remote credentials in recent state --- main.go | 40 +++---------- main_test.go | 159 ++++++++++++++++++++------------------------------- ui_forms.go | 36 ------------ 3 files changed, 68 insertions(+), 167 deletions(-) diff --git a/main.go b/main.go index 6c47aa0..6d4b89c 100644 --- a/main.go +++ b/main.go @@ -152,8 +152,6 @@ type recentVaultRecord struct { type recentRemoteRecord struct { BaseURL string `json:"baseUrl"` Path string `json:"path"` - Username string `json:"username,omitempty"` - Password string `json:"password,omitempty"` LastGroup []string `json:"lastGroup,omitempty"` UsedAt string `json:"usedAt,omitempty"` } @@ -359,7 +357,6 @@ type ui struct { cancelLifecycleProgress widget.Clickable retryLifecycleOpen widget.Clickable approvalPermanent widget.Bool - rememberRemoteAuth widget.Bool apiPolicyAllow widget.Bool apiPolicyGroupScopeW widget.Bool apiTokenDisabled widget.Bool @@ -1083,9 +1080,6 @@ func (u *ui) openRemoteAction() error { u.noteRecentRemote( strings.TrimSpace(u.remoteBaseURL.Text()), strings.TrimSpace(u.remotePath.Text()), - strings.TrimSpace(u.remoteUsername.Text()), - u.remotePassword.Text(), - u.rememberRemoteAuth.Value, ) u.resetPasswordPeek() u.restoreRecentRemoteGroup(strings.TrimSpace(u.remoteBaseURL.Text()), strings.TrimSpace(u.remotePath.Text())) @@ -1125,9 +1119,6 @@ func (u *ui) startOpenRemoteAction() { u.noteRecentRemote( strings.TrimSpace(u.remoteBaseURL.Text()), remotePath, - strings.TrimSpace(u.remoteUsername.Text()), - u.remotePassword.Text(), - u.rememberRemoteAuth.Value, ) u.resetPasswordPeek() u.restoreRecentRemoteGroup(strings.TrimSpace(u.remoteBaseURL.Text()), remotePath) @@ -1709,7 +1700,7 @@ func (u *ui) setAutofillNoticePreference(value autofillNoticeMode) { u.saveUIPreferences() } -func (u *ui) noteRecentRemote(baseURL, path, username, password string, rememberAuth bool) { +func (u *ui) noteRecentRemote(baseURL, path string) { baseURL = strings.TrimSpace(baseURL) path = strings.TrimSpace(path) if baseURL == "" || path == "" { @@ -1724,10 +1715,6 @@ func (u *ui) noteRecentRemote(baseURL, path, username, password string, remember if len(record.LastGroup) == 0 { record.LastGroup = u.recentRemoteGroup(baseURL, path) } - if rememberAuth { - record.Username = strings.TrimSpace(username) - record.Password = password - } next := []recentRemoteRecord{record} for _, existing := range u.recentRemotes { if existing.BaseURL == baseURL && existing.Path == path { @@ -1830,7 +1817,6 @@ func (u *ui) switchToLifecycleSelection(mode string) { u.remotePath.SetText("") u.remoteUsername.SetText("") u.remotePassword.SetText("") - u.rememberRemoteAuth.Value = false u.selectedRemoteConnection = false default: u.vaultPath.SetText("") @@ -1838,7 +1824,6 @@ func (u *ui) switchToLifecycleSelection(mode string) { u.remotePath.SetText("") u.remoteUsername.SetText("") u.remotePassword.SetText("") - u.rememberRemoteAuth.Value = false u.selectedRemoteConnection = false } u.requestMasterPassFocus = u.hasSelectedLifecycleTarget() @@ -1861,10 +1846,8 @@ func (u *ui) latestRecentRemote() (recentRemoteRecord, bool, time.Time) { func (u *ui) currentRemoteRecord() recentRemoteRecord { return recentRemoteRecord{ - BaseURL: strings.TrimSpace(u.remoteBaseURL.Text()), - Path: strings.TrimSpace(u.remotePath.Text()), - Username: strings.TrimSpace(u.remoteUsername.Text()), - Password: u.remotePassword.Text(), + BaseURL: strings.TrimSpace(u.remoteBaseURL.Text()), + Path: strings.TrimSpace(u.remotePath.Text()), } } @@ -1884,33 +1867,25 @@ func (u *ui) selectedRecentRemoteRecord() (recentRemoteRecord, bool) { func (u *ui) applyRecentRemoteRecord(record recentRemoteRecord) { u.remoteBaseURL.SetText(record.BaseURL) u.remotePath.SetText(record.Path) - u.remoteUsername.SetText(record.Username) - u.remotePassword.SetText(record.Password) u.remotePassword.Mask = '•' - u.rememberRemoteAuth.Value = strings.TrimSpace(record.Username) != "" || record.Password != "" u.selectedRemoteConnection = true } func (u *ui) remotePreferencesCurrentSummary() string { - selected, hasSelected := u.selectedRecentRemoteRecord() switch { - case !u.rememberRemoteAuth.Value: - return "Current choice: KeePassGO will remember only the WebDAV location for this connection." - case hasSelected && (strings.TrimSpace(selected.Username) != "" || selected.Password != ""): - return "Current choice: a successful open will update the saved sign-in for this connection on this device." case strings.TrimSpace(u.remoteUsername.Text()) != "" || u.remotePassword.Text() != "": - return "Current choice: a successful open will save the entered sign-in for this connection on this device." + return "Current choice: the entered WebDAV sign-in is used for this open. To persist it, store it in the vault and bind this vault to the remote profile." default: - return "Current choice: sign-in retention is enabled, but no username or password is entered yet." + return "Current choice: KeePassGO remembers this connection's location only. Remote credentials belong in the vault, not device state." } } func (u *ui) remotePreferencesAlwaysSavedSummary() string { - return "Recent Connections always stores the WebDAV base URL, remote path, and the last group you opened for that connection." + return "Recent Connections stores only the WebDAV base URL, remote path, and the last group you opened for that connection." } func (u *ui) remotePreferencesRetentionSummary() string { - return "KeePassGO keeps up to six recent connections. Turning off Remember sign-in and reopening rewrites that connection without the saved username or password." + return "KeePassGO keeps up to six recent connections. Store remote credentials in the vault if this connection should persist across devices or reinstalls." } func (u *ui) noteCurrentRemotePath() { @@ -3257,7 +3232,6 @@ func (u *ui) layout(gtx layout.Context) layout.Dimensions { u.remotePath.SetText("") u.remoteUsername.SetText("") u.remotePassword.SetText("") - u.rememberRemoteAuth.Value = false u.state.ErrorMessage = "" u.state.StatusMessage = "" u.requestMasterPassFocus = true diff --git a/main_test.go b/main_test.go index 3165c59..eaf9c7b 100644 --- a/main_test.go +++ b/main_test.go @@ -3853,11 +3853,11 @@ func TestUIRecentRemoteConnectionsPersistAndReload(t *testing.T) { first.recentRemotesPath = configPath first.recentRemotes = nil first.currentPath = []string{"Root", "Internet"} - first.noteRecentRemote("https://dav.example.com", "vaults/home.kdbx", "alice", "secret-1", true) + first.noteRecentRemote("https://dav.example.com", "vaults/home.kdbx") first.currentPath = []string{"Root", "Home"} - first.noteRecentRemote("https://dav.example.com", "vaults/team.kdbx", "bob", "secret-2", false) + first.noteRecentRemote("https://dav.example.com", "vaults/team.kdbx") first.currentPath = []string{"Root", "Finance"} - first.noteRecentRemote("https://dav.example.com", "vaults/home.kdbx", "alice", "secret-3", true) + first.noteRecentRemote("https://dav.example.com", "vaults/home.kdbx") second := newUIWithSession("desktop", &session.Manager{}) second.recentRemotesPath = configPath @@ -3867,20 +3867,47 @@ func TestUIRecentRemoteConnectionsPersistAndReload(t *testing.T) { if got := len(second.recentRemotes); got != 2 { t.Fatalf("len(recentRemotes) = %d, want 2", got) } - if got := second.recentRemotes[0]; got.BaseURL != "https://dav.example.com" || got.Path != "vaults/home.kdbx" || got.Username != "alice" || got.Password != "secret-3" { - t.Fatalf("recentRemotes[0] = %#v, want updated remembered credentials", got) + if got := second.recentRemotes[0]; got.BaseURL != "https://dav.example.com" || got.Path != "vaults/home.kdbx" { + t.Fatalf("recentRemotes[0] = %#v, want updated location-only record", got) } if got := second.recentRemotes[0].LastGroup; !slices.Equal(got, []string{"Root", "Finance"}) { t.Fatalf("recentRemotes[0].LastGroup = %v, want [Root Finance]", got) } - if got := second.recentRemotes[1]; got.Username != "" || got.Password != "" { - t.Fatalf("recentRemotes[1] = %#v, want credentials omitted when remember disabled", got) - } if got := second.recentRemotes[1].LastGroup; !slices.Equal(got, []string{"Root", "Home"}) { t.Fatalf("recentRemotes[1].LastGroup = %v, want [Root Home]", got) } } +func TestUILoadRecentRemotesIgnoresLegacySavedCredentials(t *testing.T) { + t.Parallel() + + configPath := filepath.Join(t.TempDir(), "recent-remotes.json") + content := `[ + { + "baseUrl": "https://dav.example.com", + "path": "vaults/home.kdbx", + "username": "alice", + "password": "secret-1", + "lastGroup": ["Root", "Internet"] + } +]` + if err := os.WriteFile(configPath, []byte(content), 0o600); err != nil { + t.Fatalf("WriteFile(recent-remotes.json) error = %v", err) + } + + u := newUIWithSession("desktop", &session.Manager{}) + u.recentRemotesPath = configPath + u.recentRemotes = nil + u.loadRecentRemotes() + + if got := len(u.recentRemotes); got != 1 { + t.Fatalf("len(recentRemotes) = %d, want 1", got) + } + if got := u.recentRemotes[0]; got.BaseURL != "https://dav.example.com" || got.Path != "vaults/home.kdbx" { + t.Fatalf("recentRemotes[0] = %#v, want location-only record", got) + } +} + func TestUIStartupPreselectsNewestTargetAcrossLocalAndRemote(t *testing.T) { t.Parallel() @@ -3898,7 +3925,7 @@ func TestUIStartupPreselectsNewestTargetAcrossLocalAndRemote(t *testing.T) { first.now = func() time.Time { return time.Date(2026, 3, 30, 12, 0, 0, 0, time.UTC) } first.noteRecentVault("/tmp/local.kdbx") first.now = func() time.Time { return time.Date(2026, 3, 30, 13, 0, 0, 0, time.UTC) } - first.noteRecentRemote("https://dav.example.com", "vaults/home.kdbx", "alice", "secret-1", true) + first.noteRecentRemote("https://dav.example.com", "vaults/home.kdbx") second := newUIWithSession("desktop", &session.Manager{}, paths) @@ -3911,11 +3938,11 @@ func TestUIStartupPreselectsNewestTargetAcrossLocalAndRemote(t *testing.T) { if got := second.remotePath.Text(); got != "vaults/home.kdbx" { t.Fatalf("remotePath = %q, want vaults/home.kdbx", got) } - if got := second.remoteUsername.Text(); got != "alice" { - t.Fatalf("remoteUsername = %q, want alice", got) + if got := second.remoteUsername.Text(); got != "" { + t.Fatalf("remoteUsername = %q, want empty for location-only recent remote", got) } - if got := second.remotePassword.Text(); got != "secret-1" { - t.Fatalf("remotePassword = %q, want secret-1", got) + if got := second.remotePassword.Text(); got != "" { + t.Fatalf("remotePassword = %q, want empty for location-only recent remote", got) } } @@ -4311,13 +4338,13 @@ func TestSelectingRecentRemoteConnectionKeepsPasswordMasked(t *testing.T) { u := newUIWithSession("desktop", &session.Manager{}) u.recentRemotes = []recentRemoteRecord{{ - BaseURL: "https://dav.example.com", - Path: "vaults/home.kdbx", - Username: "alice", - Password: "secret-1", + BaseURL: "https://dav.example.com", + Path: "vaults/home.kdbx", }} u.recentRemoteClicks = make([]widget.Clickable, 1) + u.remoteUsername.SetText("alice") + u.remotePassword.SetText("secret-1") u.remotePassword.Mask = 0 u.recentRemoteClicks[0].Click() @@ -4326,15 +4353,18 @@ func TestSelectingRecentRemoteConnectionKeepsPasswordMasked(t *testing.T) { record := u.recentRemotes[0] u.remoteBaseURL.SetText(record.BaseURL) u.remotePath.SetText(record.Path) - u.remoteUsername.SetText(record.Username) - u.remotePassword.SetText(record.Password) u.remotePassword.Mask = '•' - u.rememberRemoteAuth.Value = true } if got := u.remotePassword.Mask; got != '•' { t.Fatalf("remotePassword.Mask = %q, want bullet mask", got) } + if got := u.remoteUsername.Text(); got != "alice" { + t.Fatalf("remoteUsername = %q, want preserved manual username", got) + } + if got := u.remotePassword.Text(); got != "secret-1" { + t.Fatalf("remotePassword = %q, want preserved manual password", got) + } } func TestSelectingRecentVaultSwitchesToLocalMode(t *testing.T) { @@ -4460,10 +4490,8 @@ func TestApplyingRecentRemoteRecordMarksSelectedRemoteConnection(t *testing.T) { } u.applyRecentRemoteRecord(recentRemoteRecord{ - BaseURL: "https://dav.crew.example.invalid", - Path: "vaults/bellagio.kdbx", - Username: "dannyocean", - Password: "topsecret", + BaseURL: "https://dav.crew.example.invalid", + Path: "vaults/bellagio.kdbx", }) if !u.hasSelectedRemoteTarget() { @@ -4481,7 +4509,6 @@ func TestSwitchToLifecycleSelectionResetsLockedLocalSession(t *testing.T) { u.remotePath.SetText("vaults/remote.kdbx") u.remoteUsername.SetText("dannyocean") u.remotePassword.SetText("topsecret") - u.rememberRemoteAuth.Value = true u.masterPassword.SetText("correct horse battery staple") u.keyFilePath.SetText("/vaults/keyfile.keyx") u.search.SetText("crew") @@ -4513,9 +4540,6 @@ func TestSwitchToLifecycleSelectionResetsLockedLocalSession(t *testing.T) { if got := u.remotePassword.Text(); got != "" { t.Fatalf("remotePassword = %q, want empty", got) } - if u.rememberRemoteAuth.Value { - t.Fatal("rememberRemoteAuth = true, want false") - } if got := u.masterPassword.Text(); got != "" { t.Fatalf("masterPassword = %q, want empty", got) } @@ -4549,7 +4573,6 @@ func TestSwitchToLifecycleSelectionResetsLockedRemoteSession(t *testing.T) { u.remotePath.SetText("vaults/remote.kdbx") u.remoteUsername.SetText("rustyryan") u.remotePassword.SetText("topsecret") - u.rememberRemoteAuth.Value = true u.switchToLifecycleSelection("remote") @@ -4574,9 +4597,6 @@ func TestSwitchToLifecycleSelectionResetsLockedRemoteSession(t *testing.T) { if got := u.remotePassword.Text(); got != "" { t.Fatalf("remotePassword = %q, want empty", got) } - if u.rememberRemoteAuth.Value { - t.Fatal("rememberRemoteAuth = true, want false") - } } func TestSelectingRecentRemoteSwitchesToRemoteMode(t *testing.T) { @@ -4656,90 +4676,33 @@ func TestFriendlyRecentRemoteLabelUsesVaultNameBeforeHost(t *testing.T) { } } -func TestRecentRemoteStoredAuthSummaryDescribesSavedCredentialState(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - record recentRemoteRecord - want string - }{ - { - name: "location_only", - record: recentRemoteRecord{}, - want: "location only", - }, - { - name: "username_only", - record: recentRemoteRecord{Username: "alice"}, - want: "saved username", - }, - { - name: "password_only", - record: recentRemoteRecord{Password: "token-1"}, - want: "saved password", - }, - { - name: "full_sign_in", - record: recentRemoteRecord{Username: "alice", Password: "token-1"}, - want: "saved username and password", - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - if got := recentRemoteStoredAuthSummary(tt.record); got != tt.want { - t.Fatalf("recentRemoteStoredAuthSummary(%+v) = %q, want %q", tt.record, got, tt.want) - } - }) - } -} - -func TestUIRemotePreferencesCurrentSummaryExplainsWhatWillBeRemembered(t *testing.T) { +func TestUIRemotePreferencesCurrentSummaryExplainsVaultBackedCredentialFlow(t *testing.T) { t.Parallel() u := newUIWithSession("desktop", &session.Manager{}) u.remoteBaseURL.SetText("https://dav.example.com") u.remotePath.SetText("vaults/home.kdbx") - if got := u.remotePreferencesCurrentSummary(); got != "Current choice: KeePassGO will remember only the WebDAV location for this connection." { - t.Fatalf("remotePreferencesCurrentSummary() = %q, want location-only guidance", got) - } - - u.rememberRemoteAuth.Value = true - if got := u.remotePreferencesCurrentSummary(); got != "Current choice: sign-in retention is enabled, but no username or password is entered yet." { - t.Fatalf("remotePreferencesCurrentSummary() = %q, want empty-sign-in guidance", got) + if got := u.remotePreferencesCurrentSummary(); got != "Current choice: KeePassGO remembers this connection's location only. Remote credentials belong in the vault, not device state." { + t.Fatalf("remotePreferencesCurrentSummary() = %q, want location-only vault guidance", got) } u.remoteUsername.SetText("alice") - if got := u.remotePreferencesCurrentSummary(); got != "Current choice: a successful open will save the entered sign-in for this connection on this device." { - t.Fatalf("remotePreferencesCurrentSummary() = %q, want pending-save guidance", got) - } - - u.recentRemotes = []recentRemoteRecord{{ - BaseURL: "https://dav.example.com", - Path: "vaults/home.kdbx", - Username: "alice", - Password: "secret-1", - }} - if got := u.remotePreferencesCurrentSummary(); got != "Current choice: a successful open will update the saved sign-in for this connection on this device." { - t.Fatalf("remotePreferencesCurrentSummary() = %q, want saved-sign-in guidance", got) + if got := u.remotePreferencesCurrentSummary(); got != "Current choice: the entered WebDAV sign-in is used for this open. To persist it, store it in the vault and bind this vault to the remote profile." { + t.Fatalf("remotePreferencesCurrentSummary() = %q, want vault-storage guidance", got) } } -func TestUIRemotePreferencesHelpExplainsSavedFieldsAndRetention(t *testing.T) { +func TestUIRemotePreferencesHelpExplainsLocationOnlyRetention(t *testing.T) { t.Parallel() u := newUIWithSession("desktop", &session.Manager{}) - if got := u.remotePreferencesAlwaysSavedSummary(); got != "Recent Connections always stores the WebDAV base URL, remote path, and the last group you opened for that connection." { + if got := u.remotePreferencesAlwaysSavedSummary(); got != "Recent Connections stores only the WebDAV base URL, remote path, and the last group you opened for that connection." { t.Fatalf("remotePreferencesAlwaysSavedSummary() = %q, want saved-fields guidance", got) } - if got := u.remotePreferencesRetentionSummary(); got != "KeePassGO keeps up to six recent connections. Turning off Remember sign-in and reopening rewrites that connection without the saved username or password." { - t.Fatalf("remotePreferencesRetentionSummary() = %q, want retention guidance", got) + if got := u.remotePreferencesRetentionSummary(); got != "KeePassGO keeps up to six recent connections. Store remote credentials in the vault if this connection should persist across devices or reinstalls." { + t.Fatalf("remotePreferencesRetentionSummary() = %q, want vault retention guidance", got) } } diff --git a/ui_forms.go b/ui_forms.go index a31f9f1..c22633e 100644 --- a/ui_forms.go +++ b/ui_forms.go @@ -155,14 +155,6 @@ func (u *ui) lifecycleControls(gtx layout.Context) layout.Dimensions { } return layout.Spacer{Height: unit.Dp(6)}.Layout(gtx) }), - layout.Rigid(func(gtx layout.Context) layout.Dimensions { - if !showRemoteChooser { - return layout.Dimensions{} - } - box := material.CheckBox(u.theme, &u.rememberRemoteAuth, "Remember sign-in on this device") - box.Color = accentColor - return box.Layout(gtx) - }), layout.Rigid(func(gtx layout.Context) layout.Dimensions { if !showRemoteChooser { return layout.Dimensions{} @@ -388,14 +380,6 @@ func (u *ui) selectedRemoteConnectionCard(gtx layout.Context) layout.Dimensions lbl.Color = mutedColor return lbl.Layout(gtx) }), - layout.Rigid(func(gtx layout.Context) layout.Dimensions { - lbl := material.Label(u.theme, unit.Sp(11), "Auth: "+recentRemoteStoredAuthSummary(recentRemoteRecord{ - Username: strings.TrimSpace(u.remoteUsername.Text()), - Password: u.remotePassword.Text(), - })) - lbl.Color = mutedColor - return lbl.Layout(gtx) - }), layout.Rigid(func(gtx layout.Context) layout.Dimensions { if len(lastGroup) == 0 { return layout.Dimensions{} @@ -617,11 +601,6 @@ func (u *ui) recentRemoteList(gtx layout.Context) layout.Dimensions { lbl.Color = mutedColor return lbl.Layout(gtx) }), - layout.Rigid(func(gtx layout.Context) layout.Dimensions { - lbl := material.Label(u.theme, unit.Sp(11), "Auth: "+recentRemoteStoredAuthSummary(record)) - lbl.Color = mutedColor - return lbl.Layout(gtx) - }), layout.Rigid(func(gtx layout.Context) layout.Dimensions { if len(record.LastGroup) == 0 { return layout.Dimensions{} @@ -750,21 +729,6 @@ func normalizedRemoteHost(baseURL string) string { return strings.TrimSuffix(host, "/") } -func recentRemoteStoredAuthSummary(record recentRemoteRecord) string { - username := strings.TrimSpace(record.Username) - hasPassword := record.Password != "" - switch { - case username != "" && hasPassword: - return "saved username and password" - case username != "": - return "saved username" - case hasPassword: - return "saved password" - default: - return "location only" - } -} - func (u *ui) attachmentList(gtx layout.Context) layout.Dimensions { items := u.selectedAttachmentItems() if len(items) == 0 {