From e16067b3457bb8e996e323ca9b7fa60386724199 Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Sat, 11 Apr 2026 11:53:42 -0700 Subject: [PATCH] Fix hidden root navigation and browser fill matching --- browser/extension/content.js | 32 +++++++++--- browser/extension/manifest.firefox.json | 1 + internal/api/server.go | 26 ++++++++-- internal/api/server_test.go | 34 +++++++++++++ internal/appui/app.go | 4 +- internal/appui/entry_editor.go | 3 +- internal/appui/frame.go | 68 +++++++++++++++++++++++-- internal/appui/lifecycle_actions.go | 12 ++--- internal/appui/main_test.go | 31 +++++++++++ 9 files changed, 186 insertions(+), 25 deletions(-) diff --git a/browser/extension/content.js b/browser/extension/content.js index 3680808..abc3e53 100644 --- a/browser/extension/content.js +++ b/browser/extension/content.js @@ -13,10 +13,24 @@ function isVisibleInput(input) { } function dispatchFillEvents(input) { - input.dispatchEvent(new Event("input", { bubbles: true })); + if (typeof InputEvent === "function") { + input.dispatchEvent(new InputEvent("input", { bubbles: true, data: input.value, inputType: "insertText" })); + } else { + input.dispatchEvent(new Event("input", { bubbles: true })); + } input.dispatchEvent(new Event("change", { bubbles: true })); } +function setInputValue(input, value) { + const prototype = Object.getPrototypeOf(input); + const descriptor = prototype ? Object.getOwnPropertyDescriptor(prototype, "value") : null; + if (descriptor?.set) { + descriptor.set.call(input, value); + return; + } + input.value = value; +} + function findPasswordInput() { return Array.from(document.querySelectorAll('input[type="password"]')).find(isVisibleInput) || null; } @@ -24,12 +38,16 @@ function findPasswordInput() { function findUsernameInput(passwordInput) { const form = passwordInput?.form || null; const scope = form || document; - const candidates = Array.from(scope.querySelectorAll('input[type="text"], input[type="email"], input:not([type])')) + const candidates = Array.from(scope.querySelectorAll('input[type="text"], input[type="email"], input:not([type]), input[autocomplete="username"], input[autocomplete="email"]')) .filter(isVisibleInput); if (passwordInput) { - const sameForm = candidates.find((input) => input.form === passwordInput.form); - if (sameForm) { - return sameForm; + const sameForm = candidates.filter((input) => input.form === passwordInput.form); + if (sameForm.length !== 0) { + const priorSibling = sameForm.find((input) => + typeof input.compareDocumentPosition === "function" && + Boolean(input.compareDocumentPosition(passwordInput) & Node.DOCUMENT_POSITION_FOLLOWING) + ); + return priorSibling || sameForm[0]; } } return candidates[0] || null; @@ -41,12 +59,12 @@ function fillCredential(credential) { if (usernameInput && credential.username) { usernameInput.focus(); - usernameInput.value = credential.username; + setInputValue(usernameInput, credential.username); dispatchFillEvents(usernameInput); } if (passwordInput && credential.password) { passwordInput.focus(); - passwordInput.value = credential.password; + setInputValue(passwordInput, credential.password); dispatchFillEvents(passwordInput); } diff --git a/browser/extension/manifest.firefox.json b/browser/extension/manifest.firefox.json index 2632fb5..5612408 100644 --- a/browser/extension/manifest.firefox.json +++ b/browser/extension/manifest.firefox.json @@ -6,6 +6,7 @@ "permissions": ["activeTab", "nativeMessaging", "storage", "tabs"], "host_permissions": ["http://*/*", "https://*/*"], "background": { + "scripts": ["background.js"], "service_worker": "background.js" }, "action": { diff --git a/internal/api/server.go b/internal/api/server.go index 2a73fe2..a62e761 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -1027,12 +1027,28 @@ func normalizedBrowserHost(raw string) (string, error) { return host, nil } -func classifyBrowserEntryMatch(pageHost, rawEntryURL string) (string, int) { - parsed, err := url.Parse(strings.TrimSpace(rawEntryURL)) - if err != nil { - return "", 0 +func normalizedBrowserEntryHost(raw string) string { + raw = strings.TrimSpace(raw) + if raw == "" { + return "" } - entryHost := strings.ToLower(parsed.Hostname()) + parsed, err := url.Parse(raw) + if err == nil { + if host := strings.ToLower(parsed.Hostname()); host != "" { + return host + } + } + if !strings.Contains(raw, "://") { + parsed, err = url.Parse("https://" + raw) + if err == nil { + return strings.ToLower(parsed.Hostname()) + } + } + return "" +} + +func classifyBrowserEntryMatch(pageHost, rawEntryURL string) (string, int) { + entryHost := normalizedBrowserEntryHost(rawEntryURL) if entryHost == "" { return "", 0 } diff --git a/internal/api/server_test.go b/internal/api/server_test.go index 695e18a..d4f52c0 100644 --- a/internal/api/server_test.go +++ b/internal/api/server_test.go @@ -184,6 +184,40 @@ func TestVaultServiceFindsBrowserLoginsForAuthorizedClients(t *testing.T) { } } +func TestVaultServiceFindsBrowserLoginsForSchemeLessEntryURLs(t *testing.T) { + t.Parallel() + + client, _, cleanup := newTestClientForModel(t, vault.Model{ + Entries: []vault.Entry{ + { + ID: "gitlab", + Title: "GitLab", + Username: "jjulian", + Password: "secret", + URL: "gitlab.com", + Path: []string{"Root", "Internet"}, + }, + testAPITokenEntry(t, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"Root"}}}, + ), + }, + }) + defer cleanup() + + resp, err := client.FindBrowserLogins(tokenContext(defaultTestTokenSecret), &keepassgov1.FindBrowserLoginsRequest{ + PageUrl: "https://gitlab.com/users/sign_in", + }) + if err != nil { + t.Fatalf("FindBrowserLogins() error = %v", err) + } + if len(resp.Matches) != 1 { + t.Fatalf("len(FindBrowserLogins().Matches) = %d, want 1", len(resp.Matches)) + } + if resp.Matches[0].Id != "gitlab" { + t.Fatalf("FindBrowserLogins().Matches[0].Id = %q, want gitlab", resp.Matches[0].Id) + } +} + func TestVaultServiceFindsBrowserLoginsWithinAuthorizedGroupScope(t *testing.T) { t.Parallel() diff --git a/internal/appui/app.go b/internal/appui/app.go index 1b5bbb6..f6fd130 100644 --- a/internal/appui/app.go +++ b/internal/appui/app.go @@ -2871,7 +2871,7 @@ func (u *ui) groupBar(gtx layout.Context) layout.Dimensions { return layout.Inset{Bottom: unit.Dp(6)}.Layout(gtx, func(gtx layout.Context) layout.Dimensions { for u.groupClicks[idx].Clicked(gtx) { u.state.EnterGroup(name) - u.currentPath = append([]string(nil), u.state.CurrentPath...) + u.adoptStateCurrentPath() u.filter() } return tonedButton(gtx, u.theme, &u.groupClicks[idx], name) @@ -2902,7 +2902,7 @@ func (u *ui) groupBar(gtx layout.Context) layout.Dimensions { return layout.Inset{Bottom: unit.Dp(6)}.Layout(gtx, func(gtx layout.Context) layout.Dimensions { for u.groupClicks[idx].Clicked(gtx) { u.state.EnterGroup(name) - u.currentPath = append([]string(nil), u.state.CurrentPath...) + u.adoptStateCurrentPath() u.filter() } return tonedButton(gtx, u.theme, &u.groupClicks[idx], name) diff --git a/internal/appui/entry_editor.go b/internal/appui/entry_editor.go index 7542092..9e84b31 100644 --- a/internal/appui/entry_editor.go +++ b/internal/appui/entry_editor.go @@ -275,8 +275,7 @@ func (u *ui) deleteCurrentGroupAction() error { return err } u.clearDeleteGroupConfirmation() - u.currentPath = append([]string(nil), u.state.CurrentPath...) - u.syncedPath = append([]string(nil), u.state.CurrentPath...) + u.adoptStateCurrentPath() u.filter() return nil } diff --git a/internal/appui/frame.go b/internal/appui/frame.go index c70eb5e..78d4b0d 100644 --- a/internal/appui/frame.go +++ b/internal/appui/frame.go @@ -23,6 +23,7 @@ import ( detaillayout "git.julianfamily.org/keepassgo/internal/appui/detail/layout" "git.julianfamily.org/keepassgo/internal/clipboard" "git.julianfamily.org/keepassgo/internal/session" + "git.julianfamily.org/keepassgo/internal/vault" ) func (u *ui) bannerSurface() uiBanner { @@ -552,10 +553,72 @@ func (u *ui) setCurrentPath(path []string) { u.clearDeleteGroupConfirmation() } +func copyPath(path []string) []string { + return append([]string(nil), path...) +} + +func pathExistsInModel(model vault.Model, path []string) bool { + return len(model.EntriesInPath(path)) > 0 || len(model.ChildGroups(path)) > 0 || hasExactGroup(model, path) +} + +func normalizeEntriesPathWithoutModel(path []string, root string) []string { + if root == "" { + return copyPath(path) + } + if len(path) == 0 { + return []string{root} + } + if path[0] == "Root" { + return append([]string{root}, path[1:]...) + } + return copyPath(path) +} + +func (u *ui) normalizedEntriesPath(path []string) []string { + if u.state.Section != appstate.SectionEntries { + return copyPath(path) + } + root := u.hiddenVaultRoot() + model, err := u.state.Session.Current() + if err != nil { + return normalizeEntriesPathWithoutModel(path, root) + } + if len(path) == 0 { + if root == "" { + return nil + } + return []string{root} + } + if path[0] == "Root" && root != "" { + candidate := append([]string{root}, path[1:]...) + if pathExistsInModel(model, candidate) { + return candidate + } + } + if (len(path) == 1 && root != "" && path[0] == root) || pathExistsInModel(model, path) { + return copyPath(path) + } + if root == "" { + return copyPath(path) + } + return []string{root} +} + +func (u *ui) adoptStateCurrentPath() { + path := u.normalizedEntriesPath(u.state.CurrentPath) + u.currentPath = append([]string(nil), path...) + u.state.CurrentPath = append([]string(nil), path...) + u.syncedPath = append([]string(nil), path...) + u.syncPhoneGroupBrowser(path) + if len(u.deleteGroupPath) > 0 && !slices.Equal(u.deleteGroupPath, u.currentPath) { + u.clearDeleteGroupConfirmation() + } +} + func (u *ui) syncCurrentPath() { switch { case slices.Equal(u.currentPath, u.syncedPath) && !slices.Equal(u.state.CurrentPath, u.syncedPath): - u.currentPath = append([]string(nil), u.state.CurrentPath...) + u.adoptStateCurrentPath() case !slices.Equal(u.currentPath, u.syncedPath) && slices.Equal(u.state.CurrentPath, u.syncedPath): u.state.CurrentPath = append([]string(nil), u.currentPath...) case !slices.Equal(u.currentPath, u.syncedPath) && !slices.Equal(u.state.CurrentPath, u.syncedPath): @@ -1217,8 +1280,7 @@ func (u *ui) handleGroupClicks(gtx layout.Context) { for u.moveGroup.Clicked(gtx) { u.clearDeleteGroupConfirmation() u.runAction("move group", u.moveCurrentGroupAction) - u.currentPath = append([]string(nil), u.state.CurrentPath...) - u.syncedPath = append([]string(nil), u.state.CurrentPath...) + u.adoptStateCurrentPath() u.filter() } for u.toggleGroupControls.Clicked(gtx) { diff --git a/internal/appui/lifecycle_actions.go b/internal/appui/lifecycle_actions.go index 98d45e6..9fc4617 100644 --- a/internal/appui/lifecycle_actions.go +++ b/internal/appui/lifecycle_actions.go @@ -47,7 +47,7 @@ func (u *ui) createVaultAction() error { u.noteRecentVault(u.saveAsTargetPath()) } u.resetPasswordPeek() - u.currentPath = append([]string(nil), u.state.CurrentPath...) + u.adoptStateCurrentPath() u.loadSecuritySettingsFromSession() u.editingEntry = false u.filter() @@ -69,7 +69,7 @@ func (u *ui) openVaultAction() error { } u.noteRecentVault(path) u.resetPasswordPeek() - u.currentPath = append([]string(nil), u.state.CurrentPath...) + u.adoptStateCurrentPath() u.restoreRecentVaultGroup(path) u.syncSavedRemoteBindingSelection() if err := u.synchronizeSelectedRemoteBindingOnOpen(); err != nil { @@ -111,7 +111,7 @@ func (u *ui) startOpenVaultAction() { manager.ApplyPreparedLocalOpen(prepared) u.noteRecentVault(path) u.resetPasswordPeek() - u.currentPath = append([]string(nil), u.state.CurrentPath...) + u.adoptStateCurrentPath() u.restoreRecentVaultGroup(path) u.syncSavedRemoteBindingSelection() if err := u.synchronizeSelectedRemoteBindingOnOpen(); err != nil { @@ -329,7 +329,7 @@ func (u *ui) lockAction() error { return err } u.requestMasterPassFocus = true - u.currentPath = append([]string(nil), u.state.CurrentPath...) + u.adoptStateCurrentPath() u.resetPasswordPeek() u.editingEntry = false u.filter() @@ -346,7 +346,7 @@ func (u *ui) unlockAction() error { return err } u.resetPasswordPeek() - u.currentPath = append([]string(nil), u.state.CurrentPath...) + u.adoptStateCurrentPath() u.loadSecuritySettingsFromSession() u.editingEntry = false u.filter() @@ -375,7 +375,7 @@ func (u *ui) startUnlockAction() { return func() error { manager.ApplyPreparedUnlock(prepared) u.resetPasswordPeek() - u.currentPath = append([]string(nil), u.state.CurrentPath...) + u.adoptStateCurrentPath() u.loadSecuritySettingsFromSession() u.editingEntry = false u.filter() diff --git a/internal/appui/main_test.go b/internal/appui/main_test.go index 93e1a7c..7b4579f 100644 --- a/internal/appui/main_test.go +++ b/internal/appui/main_test.go @@ -5192,6 +5192,37 @@ func TestUIShowEntriesSectionRestoresHiddenRootAfterLeavingEntries(t *testing.T) } } +func TestUISyncCurrentPathNormalizesHiddenRootAfterSectionSwitch(t *testing.T) { + t.Parallel() + + u := newUIWithModel("desktop", vault.Model{ + Entries: []vault.Entry{ + {ID: "1", Title: "Vault Console", Path: []string{"keepass", "Crew", "Internet"}}, + }, + Groups: [][]string{ + {"keepass"}, + {"keepass", "Crew"}, + {"Recycle Bin"}, + }, + }) + + u.showEntriesSection() + u.showAPITokensSection() + u.state.Section = appstate.SectionEntries + u.state.CurrentPath = []string{"Root"} + u.currentPath = nil + u.syncedPath = nil + + u.syncCurrentPath() + + if got := u.currentPath; !slices.Equal(got, []string{"keepass"}) { + t.Fatalf("currentPath after syncCurrentPath() = %v, want [keepass]", got) + } + if got := u.displayPath(); len(got) != 0 { + t.Fatalf("displayPath() after syncCurrentPath() = %v, want root slash path", got) + } +} + func TestUIShowEntriesSectionRestoresEntriesViewState(t *testing.T) { t.Parallel()