diff --git a/browser/extension/background.js b/browser/extension/background.js index 3cfb627..fcaf63b 100644 --- a/browser/extension/background.js +++ b/browser/extension/background.js @@ -697,6 +697,21 @@ if (isNodeTestEnv) { await refreshActivePage({ force: true }).catch(() => null); sendResponse({ success: true }); return; + case "keepassgo-search-logins": { + const settings = await loadSettings(); + const response = await connectNative({ + action: "search-logins", + bearerToken: settings.bearerToken, + query: String(message?.query || "").trim() + }); + sendResponse({ + success: Boolean(response?.success), + error: response?.error || "", + results: Array.isArray(response?.searchResults) ? response.searchResults : [], + status: response?.status ?? null + }); + return; + } case "keepassgo-page-ready": if (Number.isInteger(sender?.tab?.id)) { sendResponse(await refreshPageState(sender.tab.id, sender.tab.url, { diff --git a/browser/extension/popup.html b/browser/extension/popup.html index 8df2a02..ab28393 100644 --- a/browser/extension/popup.html +++ b/browser/extension/popup.html @@ -24,6 +24,14 @@

Matches

+
+

Search Vault

+
+ + +
+
+
diff --git a/browser/extension/popup.js b/browser/extension/popup.js index 71c01b8..4c56f4b 100644 --- a/browser/extension/popup.js +++ b/browser/extension/popup.js @@ -43,21 +43,19 @@ function matchSubtitle(match) { return parts.join(" · ") || "No username"; } -function renderMatches(state) { - const root = document.getElementById("matches"); +function renderMatchList(root, matches, options = {}) { const targetTabID = popupTabID(); + const emptyMessage = options.emptyMessage || "No matching entries."; root.textContent = ""; - if (!Array.isArray(state.matches) || state.matches.length === 0) { + if (!Array.isArray(matches) || matches.length === 0) { const empty = document.createElement("p"); empty.className = "subtle"; - empty.textContent = state.pageHasLoginForm - ? "No matching entries for this page." - : "No login fields detected on this page."; + empty.textContent = emptyMessage; root.appendChild(empty); return; } - for (const match of state.matches) { + for (const match of matches) { const row = document.createElement("button"); row.type = "button"; row.className = "match-row"; @@ -98,6 +96,28 @@ function renderMatches(state) { } } +function renderMatches(state) { + const emptyMessage = state.pageHasLoginForm + ? "No matching entries for this page." + : "No login fields detected on this page."; + renderMatchList(document.getElementById("matches"), state.matches, { emptyMessage }); +} + +function renderSearchResults(results, query) { + const root = document.getElementById("search-results"); + if (!query) { + root.textContent = ""; + const hint = document.createElement("p"); + hint.className = "subtle"; + hint.textContent = "Search all entries you can access with this token."; + root.appendChild(hint); + return; + } + renderMatchList(root, results, { + emptyMessage: `No entries matched "${query}".` + }); +} + function renderPageHint(state) { const hint = document.getElementById("page-hint"); if (state.pendingFill) { @@ -124,8 +144,38 @@ function popupTabID() { return Number.isInteger(parsed) ? parsed : null; } +async function searchVault(event) { + event.preventDefault(); + const query = document.getElementById("search-query").value.trim(); + const resultsRoot = document.getElementById("search-results"); + if (!query) { + renderSearchResults([], ""); + return; + } + resultsRoot.textContent = ""; + const loading = document.createElement("p"); + loading.className = "subtle"; + loading.textContent = "Searching KeePassGO…"; + resultsRoot.appendChild(loading); + try { + const response = await runtimeSend({ + type: "keepassgo-search-logins", + query + }); + if (!response?.success) { + throw new Error(response?.error || "Search failed."); + } + renderSearchResults(Array.isArray(response.results) ? response.results : [], query); + } catch (error) { + renderSearchResults([], query); + setStatus("Search failed", error instanceof Error ? error.message : String(error), "error"); + } +} + async function main() { try { + document.getElementById("search-form").addEventListener("submit", searchVault); + renderSearchResults([], ""); const state = await runtimeSend({ type: "keepassgo-popup-state", force: true, diff --git a/browser/extension/style.css b/browser/extension/style.css index 63228d5..5c3cc25 100644 --- a/browser/extension/style.css +++ b/browser/extension/style.css @@ -96,6 +96,17 @@ h2 { gap: 8px; } +.search-section { + margin-top: 16px; +} + +.search-form { + display: grid; + grid-template-columns: minmax(0, 1fr) auto; + gap: 8px; + margin-bottom: 10px; +} + .match-row, button, .link-button { diff --git a/docs/browser-extension.md b/docs/browser-extension.md index 17abca2..87413e7 100644 --- a/docs/browser-extension.md +++ b/docs/browser-extension.md @@ -93,6 +93,28 @@ Chromium / Chrome: - Username and password fields get an inline KeePassGO affordance that opens a candidate chooser anchored to the focused field and keeps fills scoped to that field's form when possible. - If a fill request needs user approval, the extension keeps the pending state visible in both the page affordance and the popup until KeePassGO resolves it, using the token-scoped pending-approval count from the local gRPC API. +## Search And Matching + +User story: + +- When a page has no obvious match, the popup still lets the user search the + vault without leaving the browser. +- Search results must stay scoped to what the current API token can actually + access. +- Browser matching must treat common KeePass data conventions as real browser + targets, not just the primary `URL` field. + +Expected behavior: + +- The popup exposes a `Search Vault` field that queries KeePassGO directly. +- Search results use the same fill path as page matches. +- Search never leaks entries outside the token's authorized group scope. +- A browser match can come from: + - the primary `URL` field + - scheme-less host values such as `gitlab.com` + - custom URL fields such as `URL1`, `URL2`, and similar KeePass-style URL + slots + For extension-side regression checks, run: ```bash diff --git a/internal/api/server.go b/internal/api/server.go index 469ee4b..9001593 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -275,7 +275,7 @@ func (s *Server) FindBrowserLogins(ctx context.Context, req *keepassgov1.FindBro var matches []rankedBrowserMatch for _, entry := range displayModel.Entries { - quality, score := classifyBrowserEntryMatch(pageHost, entry.URL) + quality, score := classifyBrowserEntry(pageHost, entry) if score == 0 { continue } @@ -390,7 +390,7 @@ func (s *Server) GetBrowserCredential(ctx context.Context, req *keepassgov1.GetB if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } - if _, score := classifyBrowserEntryMatch(pageHost, entry.URL); score == 0 { + if _, score := classifyBrowserEntry(pageHost, entry); score == 0 { return nil, status.Error(codes.InvalidArgument, "entry url does not match requested page") } } @@ -446,19 +446,22 @@ func (s *Server) ListEntries(ctx context.Context, req *keepassgov1.ListEntriesRe } displayModel := visibleModel(model) internalPath := expandClientPath(displayModel, req.GetPath()) - if _, err := s.authorizePathRequest(ctx, apitokens.OperationListEntries, internalPath); err != nil { - return nil, err - } - model = displayModel var entries []vault.Entry if strings.TrimSpace(req.GetQuery()) != "" { + token, err := s.authenticateRequest(ctx) + if err != nil { + return nil, err + } results := model.Search(req.GetQuery()) - entries = make([]vault.Entry, 0, len(results)) - for _, result := range results { - entries = append(entries, result.Entry) + entries, err = s.authorizedSearchEntries(ctx, model, token, internalPath, results) + if err != nil { + return nil, err } } else { + if _, err := s.authorizePathRequest(ctx, apitokens.OperationListEntries, internalPath); err != nil { + return nil, err + } entries = model.EntriesInPath(internalPath) } @@ -472,6 +475,49 @@ func (s *Server) ListEntries(ctx context.Context, req *keepassgov1.ListEntriesRe return resp, nil } +func (s *Server) authorizedSearchEntries(ctx context.Context, model vault.Model, token apitokens.Token, path []string, results []vault.SearchResult) ([]vault.Entry, error) { + entries := make([]vault.Entry, 0, len(results)) + var promptResource *apitokens.Resource + for _, result := range results { + entry := result.Entry + if !hasPathPrefix(path, entry.Path) { + continue + } + resource := apitokens.Resource{Kind: apitokens.ResourceGroup, Path: entry.Path} + switch evaluateAuthorization(model, token, apitokens.OperationListEntries, resource) { + case apitokens.DecisionAllow: + entries = append(entries, entry) + case apitokens.DecisionPrompt: + if promptResource == nil { + candidate := resource + promptResource = &candidate + } + } + } + if len(entries) != 0 || promptResource == nil { + return entries, nil + } + if _, err := s.authorizeResourceRequest(ctx, token, apitokens.OperationListEntries, *promptResource); err != nil { + return nil, err + } + return authorizedSearchEntriesWithinPath(path, promptResource.Path, results), nil +} + +func authorizedSearchEntriesWithinPath(requestPath, approvedPath []string, results []vault.SearchResult) []vault.Entry { + entries := make([]vault.Entry, 0, len(results)) + for _, result := range results { + entry := result.Entry + if !hasPathPrefix(requestPath, entry.Path) { + continue + } + if !hasPathPrefix(approvedPath, entry.Path) { + continue + } + entries = append(entries, entry) + } + return entries +} + func (s *Server) ListGroups(ctx context.Context, req *keepassgov1.ListGroupsRequest) (*keepassgov1.ListGroupsResponse, error) { model, locked := s.snapshotModel() if locked { @@ -1063,6 +1109,52 @@ func normalizedBrowserEntryHost(raw string) string { return "" } +func browserURLFieldKey(key string) bool { + if len(key) <= len("URL") || !strings.EqualFold(key[:len("URL")], "URL") { + return false + } + for _, r := range key[len("URL"):] { + if r < '0' || r > '9' { + return false + } + } + return true +} + +func browserEntryURLs(entry vault.Entry) []string { + urls := make([]string, 0, 1+len(entry.Fields)) + if raw := strings.TrimSpace(entry.URL); raw != "" { + urls = append(urls, raw) + } + if len(entry.Fields) == 0 { + return urls + } + keys := slices.Collect(maps.Keys(entry.Fields)) + slices.Sort(keys) + for _, key := range keys { + if !browserURLFieldKey(key) { + continue + } + if raw := strings.TrimSpace(entry.Fields[key]); raw != "" { + urls = append(urls, raw) + } + } + return urls +} + +func classifyBrowserEntry(pageHost string, entry vault.Entry) (string, int) { + bestQuality := "" + bestScore := 0 + for _, rawURL := range browserEntryURLs(entry) { + quality, score := classifyBrowserEntryMatch(pageHost, rawURL) + if score > bestScore { + bestQuality = quality + bestScore = score + } + } + return bestQuality, bestScore +} + func classifyBrowserEntryMatch(pageHost, rawEntryURL string) (string, int) { entryHost := normalizedBrowserEntryHost(rawEntryURL) if entryHost == "" { @@ -1078,6 +1170,13 @@ func classifyBrowserEntryMatch(pageHost, rawEntryURL string) (string, int) { } } +func hasPathPrefix(prefix, path []string) bool { + if len(prefix) > len(path) { + return false + } + return slices.Equal(prefix, path[:len(prefix)]) +} + func visibleModel(model vault.Model) vault.Model { out := model out.Entries = nil diff --git a/internal/api/server_test.go b/internal/api/server_test.go index 85fb51b..3668952 100644 --- a/internal/api/server_test.go +++ b/internal/api/server_test.go @@ -294,6 +294,55 @@ func TestVaultServiceFindsBrowserLoginsForSchemeLessEntryURLs(t *testing.T) { } } +func TestVaultServiceFindsBrowserLoginsForCustomURLFields(t *testing.T) { + t.Parallel() + + client, _, cleanup := newTestClientForModel(t, vault.Model{ + Entries: []vault.Entry{ + { + ID: "night-fox-gitlab", + Title: "Night Fox GitLab", + Username: "nightfox", + Password: "vault-code", + Path: []string{"Root", "Internet"}, + Fields: map[string]string{ + "URL1": "gitlab.com", + }, + }, + testAPITokenEntry(t, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"Root"}}}, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationCopyUsername, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"Root"}}}, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationCopyPassword, 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 != "night-fox-gitlab" { + t.Fatalf("FindBrowserLogins().Matches[0].Id = %q, want night-fox-gitlab", resp.Matches[0].Id) + } + + credential, err := client.GetBrowserCredential(tokenContext(defaultTestTokenSecret), &keepassgov1.GetBrowserCredentialRequest{ + Id: "night-fox-gitlab", + PageUrl: "https://gitlab.com/users/sign_in", + }) + if err != nil { + t.Fatalf("GetBrowserCredential() error = %v", err) + } + if credential.GetId() != "night-fox-gitlab" { + t.Fatalf("GetBrowserCredential().Id = %q, want night-fox-gitlab", credential.GetId()) + } +} + func TestVaultServiceFindsBrowserLoginsWithinAuthorizedGroupScope(t *testing.T) { t.Parallel() @@ -1203,6 +1252,51 @@ func TestVaultServiceListsEntriesForAuthorizedClients(t *testing.T) { } } +func TestVaultServiceSearchesEntriesWithinAuthorizedScope(t *testing.T) { + t.Parallel() + + client, _, cleanup := newTestClientForModel(t, vault.Model{ + Entries: []vault.Entry{ + { + ID: "turk-codex", + Title: "Turk Codex GitLab", + Username: "basher", + Password: "chip-stack", + URL: "https://gitlab.com", + Path: []string{"keepass", "Joe", "codex"}, + }, + { + ID: "rusty-internet", + Title: "Rusty Internet GitLab", + Username: "rusty", + Password: "bellagio-stack", + URL: "https://gitlab.com", + Path: []string{"keepass", "Joe", "Internet"}, + }, + testAPITokenEntry(t, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"Root", "Joe", "codex"}}}, + ), + }, + }) + defer cleanup() + + resp, err := client.ListEntries(tokenContext(defaultTestTokenSecret), &keepassgov1.ListEntriesRequest{ + Query: "GitLab", + }) + if err != nil { + t.Fatalf("ListEntries() error = %v", err) + } + if len(resp.Entries) != 1 { + t.Fatalf("len(ListEntries().Entries) = %d, want 1", len(resp.Entries)) + } + if got := resp.Entries[0].Id; got != "turk-codex" { + t.Fatalf("ListEntries().Entries[0].Id = %q, want turk-codex", got) + } + if got := resp.Entries[0].Path; !slices.Equal(got, []string{"Joe", "codex"}) { + t.Fatalf("ListEntries().Entries[0].Path = %v, want [Joe codex]", got) + } +} + func TestVaultServiceListsCreatesAndRenamesGroupsForAuthorizedClients(t *testing.T) { t.Parallel() diff --git a/internal/browserbridge/bridge.go b/internal/browserbridge/bridge.go index 987134c..410743a 100644 --- a/internal/browserbridge/bridge.go +++ b/internal/browserbridge/bridge.go @@ -32,15 +32,17 @@ type Request struct { BearerToken string `json:"bearerToken,omitempty"` URL string `json:"url,omitempty"` EntryID string `json:"entryId,omitempty"` + Query string `json:"query,omitempty"` } type Response struct { - Success bool `json:"success"` - Error string `json:"error,omitempty"` - Status *Status `json:"status,omitempty"` - Matches []Match `json:"matches,omitempty"` - Credential *Credential `json:"credential,omitempty"` - Version string `json:"version,omitempty"` + Success bool `json:"success"` + Error string `json:"error,omitempty"` + Status *Status `json:"status,omitempty"` + Matches []Match `json:"matches,omitempty"` + SearchResults []Match `json:"searchResults,omitempty"` + Credential *Credential `json:"credential,omitempty"` + Version string `json:"version,omitempty"` } type Status struct { @@ -77,6 +79,7 @@ type Connection struct { type Client interface { Status(context.Context) (*keepassgov1.GetSessionStatusResponse, error) FindBrowserLogins(context.Context, string) ([]*keepassgov1.BrowserLoginMatch, error) + ListEntries(context.Context, []string, string) ([]*keepassgov1.Entry, error) GetBrowserCredential(context.Context, string, string) (*keepassgov1.GetBrowserCredentialResponse, error) } @@ -179,6 +182,15 @@ func HandleRequest(ctx context.Context, req Request, grpcAddr string, client Cli return Response{Success: false, Error: err.Error(), Status: disconnectedStatus(conn.GRPCAddress)} } return Response{Success: true, Status: availableStatus(conn.GRPCAddress), Matches: matches, Version: responseVersion} + case "search-logins": + results, err := searchEntries(ctx, client, req.Query) + if err != nil { + if status := inferredActionStatus(conn.GRPCAddress, err); status != nil { + return Response{Success: true, Status: status, SearchResults: nil, Version: responseVersion} + } + return Response{Success: false, Error: err.Error(), Status: disconnectedStatus(conn.GRPCAddress)} + } + return Response{Success: true, Status: availableStatus(conn.GRPCAddress), SearchResults: results, Version: responseVersion} case "get-login": credential, err := loadCredential(ctx, client, req.EntryID, req.URL) if err != nil { @@ -264,6 +276,24 @@ func loadCredential(ctx context.Context, client Client, entryID, rawURL string) }, nil } +func searchEntries(ctx context.Context, client Client, query string) ([]Match, error) { + resp, err := client.ListEntries(ctx, nil, strings.TrimSpace(query)) + if err != nil { + return nil, err + } + out := make([]Match, 0, len(resp)) + for _, entry := range resp { + out = append(out, Match{ + ID: entry.GetId(), + Title: entry.GetTitle(), + Username: entry.GetUsername(), + URL: entry.GetUrl(), + Path: append([]string(nil), entry.GetPath()...), + }) + } + return out, nil +} + func Manifest(browser Browser, binaryPath, extensionID string) (NativeHostManifest, error) { path := strings.TrimSpace(binaryPath) if path == "" { diff --git a/internal/browserbridge/bridge_test.go b/internal/browserbridge/bridge_test.go index 59fd806..e69019d 100644 --- a/internal/browserbridge/bridge_test.go +++ b/internal/browserbridge/bridge_test.go @@ -149,6 +149,27 @@ func TestHandleRequestGetLogin(t *testing.T) { } } +func TestHandleRequestSearchLogins(t *testing.T) { + t.Parallel() + + client := &fakeClient{ + entries: []*keepassgov1.Entry{ + {Id: "rusty-gitlab", Title: "Rusty GitLab", Username: "rustyryan", Url: "gitlab.com", Path: []string{"Joe", "Internet"}}, + }, + } + resp := HandleRequest(context.Background(), Request{ + Action: "search-logins", + BearerToken: "secret", + Query: "GitLab", + }, "", client) + if !resp.Success { + t.Fatalf("HandleRequest(search-logins) success = false, error = %q", resp.Error) + } + if len(resp.SearchResults) != 1 || resp.SearchResults[0].ID != "rusty-gitlab" { + t.Fatalf("HandleRequest(search-logins).SearchResults = %#v, want rusty-gitlab", resp.SearchResults) + } +} + func TestHandleRequestFindLoginsInfersLockedStatusFromRPC(t *testing.T) { t.Parallel() @@ -309,9 +330,11 @@ func TestEnsureNativeHostManifestsInstallsFirefoxAndDiscoveredChromium(t *testin type fakeClient struct { status *keepassgov1.GetSessionStatusResponse matches []*keepassgov1.BrowserLoginMatch + entries []*keepassgov1.Entry credential *keepassgov1.GetBrowserCredentialResponse err error matchesErr error + entriesErr error credentialErr error statusCalls int } @@ -382,6 +405,16 @@ func (f *fakeClient) FindBrowserLogins(context.Context, string) ([]*keepassgov1. return f.matches, nil } +func (f *fakeClient) ListEntries(context.Context, []string, string) ([]*keepassgov1.Entry, error) { + if f.entriesErr != nil { + return nil, f.entriesErr + } + if f.err != nil { + return nil, f.err + } + return f.entries, nil +} + func (f *fakeClient) GetBrowserCredential(context.Context, string, string) (*keepassgov1.GetBrowserCredentialResponse, error) { if f.credentialErr != nil { return nil, f.credentialErr diff --git a/internal/browserbridge/client.go b/internal/browserbridge/client.go index 8d0f01f..e80ba19 100644 --- a/internal/browserbridge/client.go +++ b/internal/browserbridge/client.go @@ -65,6 +65,17 @@ func (c *GRPCClient) FindBrowserLogins(ctx context.Context, pageURL string) ([]* return resp.GetMatches(), nil } +func (c *GRPCClient) ListEntries(ctx context.Context, path []string, query string) ([]*keepassgov1.Entry, error) { + resp, err := c.client.ListEntries(ctx, &keepassgov1.ListEntriesRequest{ + Path: append([]string(nil), path...), + Query: strings.TrimSpace(query), + }) + if err != nil { + return nil, err + } + return resp.GetEntries(), nil +} + func (c *GRPCClient) GetBrowserCredential(ctx context.Context, entryID, pageURL string) (*keepassgov1.GetBrowserCredentialResponse, error) { return c.client.GetBrowserCredential(ctx, &keepassgov1.GetBrowserCredentialRequest{ Id: strings.TrimSpace(entryID), diff --git a/internal/vaultview/root.go b/internal/vaultview/root.go index bd5a09b..8631b39 100644 --- a/internal/vaultview/root.go +++ b/internal/vaultview/root.go @@ -5,10 +5,13 @@ import "git.julianfamily.org/keepassgo/internal/vault" // HiddenRoot returns the single synthetic top-level vault group that should be // treated as an internal storage root rather than as a user-visible group. func HiddenRoot(model vault.Model) string { - if !hasGroup(model.Groups, []string{KeepassRoot}) { - return "" + if hasGroup(model.Groups, []string{KeepassRoot}) { + return KeepassRoot } - return KeepassRoot + if usesTopLevelRoot(model, KeepassRoot) { + return KeepassRoot + } + return "" } func hasGroup(groups [][]string, path []string) bool { diff --git a/internal/vaultview/root_test.go b/internal/vaultview/root_test.go index fee444e..926dc3f 100644 --- a/internal/vaultview/root_test.go +++ b/internal/vaultview/root_test.go @@ -24,3 +24,20 @@ func TestHiddenRootIgnoresRecycleBin(t *testing.T) { t.Fatalf("HiddenRoot() = %q, want %q", got, "keepass") } } + +func TestHiddenRootFallsBackToEntryPathsWhenGroupsAreSparse(t *testing.T) { + t.Parallel() + + model := vault.Model{ + Entries: []vault.Entry{ + {ID: "rusty", Title: "Rusty GitLab", Path: []string{"keepass", "Joe", "Internet"}}, + }, + Groups: [][]string{ + {"Recycle Bin"}, + }, + } + + if got := HiddenRoot(model); got != "keepass" { + t.Fatalf("HiddenRoot() = %q, want %q", got, "keepass") + } +}