From e8a48fb7aaee0ba5313376c23a66c234f456b2b0 Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Mon, 13 Apr 2026 06:58:11 -0700 Subject: [PATCH 1/9] Track vault root view bugfix --- AGENTS.md | 1 + TODO.md | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 189c080..4925d1d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -135,6 +135,7 @@ These features are product requirements, not “nice to have” ideas. ## Delivery Discipline +- Treat bug fixes as the highest-priority items in `TODO.md`. - Do not treat this product as complete until the stated requirements in this file are actually satisfied. - Do not stop at a “good checkpoint” or “meaningful tranche” when required product capabilities are still missing. - Continue iterating in test-first slices: diff --git a/TODO.md b/TODO.md index bd73621..cbae43c 100644 --- a/TODO.md +++ b/TODO.md @@ -6,6 +6,13 @@ These segments are intended to be independently executable wherever possible. Each segment has its own local exit criteria. The product is not complete until the global exit criteria at the end of this file are also met. +## Priority Bugs + +- Vault root view bug: + stop leaking the physical `keepass` storage root into user-facing group lists + and API-facing logical paths by introducing explicit `Vault`, `VaultRoot`, + and `VaultRecycleBin` datastore views. + ## UI Review Follow-Ups These items came from a hands-on emulator and desktop walkthrough. -- 2.52.0 From 32e6fc6c90899093c0322999fd092029df8e645f Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Mon, 13 Apr 2026 06:59:55 -0700 Subject: [PATCH 2/9] Break vault root bug into commit-sized todo items --- TODO.md | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/TODO.md b/TODO.md index cbae43c..164fb45 100644 --- a/TODO.md +++ b/TODO.md @@ -9,9 +9,25 @@ The product is not complete until the global exit criteria at the end of this fi ## Priority Bugs - Vault root view bug: - stop leaking the physical `keepass` storage root into user-facing group lists - and API-facing logical paths by introducing explicit `Vault`, `VaultRoot`, - and `VaultRecycleBin` datastore views. + add failing behavior tests for explicit `Vault`, `VaultRoot`, and + `VaultRecycleBin` datastore views, including `keepass` path translation and + recycle-bin projection. +- Vault root view bug: + introduce explicit `Vault`, `VaultRoot`, and `VaultRecycleBin` view factories + in `internal/vaultview` and move hidden-root behavior out of UI heuristics. +- Vault root view bug: + update `internal/appstate` and entries/recycle-bin UI plumbing to use + `VaultRoot` and `VaultRecycleBin` instead of raw datastore paths. +- Vault root view bug: + update gRPC/API-facing datastore reads and writes to use logical `VaultRoot` + paths while keeping authorization on canonical `Vault` paths. +- Vault root view bug: + add API-token path translation helpers so token editing, approval prompts, and + audit/resource display stop leaking the physical `keepass` root. +- Vault root view bug: + ensure vault create/open maintains the physical `keepass` storage root, + lazily creates recycle-bin structure when needed, and warns when opening a + legacy vault that had to be normalized. ## UI Review Follow-Ups -- 2.52.0 From 0ce25a971202efa0a994e7eebe3468ff19477a09 Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Mon, 13 Apr 2026 07:00:51 -0700 Subject: [PATCH 3/9] Add failing vault view behavior tests --- TODO.md | 4 -- internal/vaultview/view_test.go | 101 ++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 internal/vaultview/view_test.go diff --git a/TODO.md b/TODO.md index 164fb45..2519600 100644 --- a/TODO.md +++ b/TODO.md @@ -8,10 +8,6 @@ The product is not complete until the global exit criteria at the end of this fi ## Priority Bugs -- Vault root view bug: - add failing behavior tests for explicit `Vault`, `VaultRoot`, and - `VaultRecycleBin` datastore views, including `keepass` path translation and - recycle-bin projection. - Vault root view bug: introduce explicit `Vault`, `VaultRoot`, and `VaultRecycleBin` view factories in `internal/vaultview` and move hidden-root behavior out of UI heuristics. diff --git a/internal/vaultview/view_test.go b/internal/vaultview/view_test.go new file mode 100644 index 0000000..ef0904f --- /dev/null +++ b/internal/vaultview/view_test.go @@ -0,0 +1,101 @@ +package vaultview + +import ( + "slices" + "testing" + + "git.julianfamily.org/keepassgo/internal/vault" +) + +func TestVaultRootProjectsKeepassStorageRoot(t *testing.T) { + t.Parallel() + + model := vault.Model{ + Entries: []vault.Entry{ + {ID: "bellagio-ledger", Title: "Bellagio Ledger", Path: []string{"keepass", "Crew", "Internet"}}, + {ID: "fountain-cameras", Title: "Fountain Cameras", Path: []string{"keepass", "Crew", "Security"}}, + }, + Groups: [][]string{ + {"keepass"}, + {"keepass", "Crew"}, + {"keepass", "Crew", "Internet"}, + {"keepass", "Crew", "Security"}, + {"Recycle Bin"}, + }, + } + + view := VaultRoot(model) + + if got := view.ChildGroups(nil); !slices.Equal(got, []string{"Crew"}) { + t.Fatalf("VaultRoot(model).ChildGroups(nil) = %v, want [Crew]", got) + } + if got := view.ChildGroups([]string{"Crew"}); !slices.Equal(got, []string{"Internet", "Security"}) { + t.Fatalf("VaultRoot(model).ChildGroups([Crew]) = %v, want [Internet Security]", got) + } + + gotEntries := view.EntriesInPath([]string{"Crew", "Internet"}) + if len(gotEntries) != 1 || !slices.Equal(gotEntries[0].Path, []string{"Crew", "Internet"}) { + t.Fatalf("VaultRoot(model).EntriesInPath([Crew Internet]) = %#v, want logical path [Crew Internet]", gotEntries) + } + + if got := view.ToPhysicalPath(nil); !slices.Equal(got, []string{"keepass"}) { + t.Fatalf("VaultRoot(model).ToPhysicalPath(nil) = %v, want [keepass]", got) + } + if got := view.ToPhysicalPath([]string{"Crew", "Internet"}); !slices.Equal(got, []string{"keepass", "Crew", "Internet"}) { + t.Fatalf("VaultRoot(model).ToPhysicalPath([Crew Internet]) = %v, want [keepass Crew Internet]", got) + } + if got := view.FromPhysicalPath([]string{"keepass", "Crew", "Internet"}); !slices.Equal(got, []string{"Crew", "Internet"}) { + t.Fatalf("VaultRoot(model).FromPhysicalPath([keepass Crew Internet]) = %v, want [Crew Internet]", got) + } +} + +func TestVaultRecycleBinProjectsRecycleTree(t *testing.T) { + t.Parallel() + + model := vault.Model{ + RecycleBin: []vault.Entry{ + {ID: "bellagio-ledger", Title: "Bellagio Ledger", Path: []string{"Crew", "Internet"}}, + {ID: "fountain-cameras", Title: "Fountain Cameras", Path: []string{"Crew", "Security"}}, + }, + } + + view := VaultRecycleBin(model) + + if got := view.ChildGroups(nil); !slices.Equal(got, []string{"Crew"}) { + t.Fatalf("VaultRecycleBin(model).ChildGroups(nil) = %v, want [Crew]", got) + } + if got := view.ChildGroups([]string{"Crew"}); !slices.Equal(got, []string{"Internet", "Security"}) { + t.Fatalf("VaultRecycleBin(model).ChildGroups([Crew]) = %v, want [Internet Security]", got) + } + + gotEntries := view.EntriesInPath([]string{"Crew", "Internet"}) + if len(gotEntries) != 1 || !slices.Equal(gotEntries[0].Path, []string{"Crew", "Internet"}) { + t.Fatalf("VaultRecycleBin(model).EntriesInPath([Crew Internet]) = %#v, want logical recycle-bin path [Crew Internet]", gotEntries) + } + + if got := view.ToPhysicalPath([]string{"Crew", "Internet"}); !slices.Equal(got, []string{"Crew", "Internet"}) { + t.Fatalf("VaultRecycleBin(model).ToPhysicalPath([Crew Internet]) = %v, want [Crew Internet]", got) + } +} + +func TestVaultReturnsPhysicalPathsUnchanged(t *testing.T) { + t.Parallel() + + model := vault.Model{ + Entries: []vault.Entry{ + {ID: "bellagio-ledger", Title: "Bellagio Ledger", Path: []string{"keepass", "Crew", "Internet"}}, + }, + } + + view := Vault(model) + + if got := view.ChildGroups(nil); !slices.Equal(got, []string{"keepass"}) { + t.Fatalf("Vault(model).ChildGroups(nil) = %v, want [keepass]", got) + } + if got := view.ToPhysicalPath([]string{"keepass", "Crew"}); !slices.Equal(got, []string{"keepass", "Crew"}) { + t.Fatalf("Vault(model).ToPhysicalPath([keepass Crew]) = %v, want [keepass Crew]", got) + } + if got := view.FromPhysicalEntry(model.Entries[0]); !slices.Equal(got.Path, []string{"keepass", "Crew", "Internet"}) { + t.Fatalf("Vault(model).FromPhysicalEntry(entry).Path = %v, want [keepass Crew Internet]", got.Path) + } +} -- 2.52.0 From ea30775eb7bb334cdf2b95e26646ce96db1c0eb3 Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Mon, 13 Apr 2026 07:02:44 -0700 Subject: [PATCH 4/9] Add explicit vault view factories --- TODO.md | 6 +- internal/vaultview/root.go | 26 ++-- internal/vaultview/view.go | 269 +++++++++++++++++++++++++++++++++++++ 3 files changed, 288 insertions(+), 13 deletions(-) create mode 100644 internal/vaultview/view.go diff --git a/TODO.md b/TODO.md index 2519600..c43ae90 100644 --- a/TODO.md +++ b/TODO.md @@ -8,12 +8,10 @@ The product is not complete until the global exit criteria at the end of this fi ## Priority Bugs -- Vault root view bug: - introduce explicit `Vault`, `VaultRoot`, and `VaultRecycleBin` view factories - in `internal/vaultview` and move hidden-root behavior out of UI heuristics. - Vault root view bug: update `internal/appstate` and entries/recycle-bin UI plumbing to use - `VaultRoot` and `VaultRecycleBin` instead of raw datastore paths. + `VaultRoot` and `VaultRecycleBin` instead of raw datastore paths, removing + the hidden-root heuristic from entries browsing. - Vault root view bug: update gRPC/API-facing datastore reads and writes to use logical `VaultRoot` paths while keeping authorization on canonical `Vault` paths. diff --git a/internal/vaultview/root.go b/internal/vaultview/root.go index 7b398ba..bd5a09b 100644 --- a/internal/vaultview/root.go +++ b/internal/vaultview/root.go @@ -5,19 +5,27 @@ 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 len(model.EntriesInPath(nil)) != 0 { + if !hasGroup(model.Groups, []string{KeepassRoot}) { return "" } - groups := model.ChildGroups(nil) - roots := make([]string, 0, len(groups)) + return KeepassRoot +} + +func hasGroup(groups [][]string, path []string) bool { for _, group := range groups { - if group == "Recycle Bin" { + if len(group) != len(path) { continue } - roots = append(roots, group) + match := true + for i := range group { + if group[i] != path[i] { + match = false + break + } + } + if match { + return true + } } - if len(roots) != 1 { - return "" - } - return roots[0] + return false } diff --git a/internal/vaultview/view.go b/internal/vaultview/view.go new file mode 100644 index 0000000..23e82b3 --- /dev/null +++ b/internal/vaultview/view.go @@ -0,0 +1,269 @@ +package vaultview + +import ( + "slices" + + "git.julianfamily.org/keepassgo/internal/vault" +) + +const KeepassRoot = "keepass" + +// View projects the physical vault model into a logical tree for a specific +// product surface. +type View interface { + ChildGroups(path []string) []string + EntriesInPath(path []string) []vault.Entry + EntriesUnderPath(path []string) []vault.Entry + ToPhysicalPath(path []string) []string + FromPhysicalPath(path []string) []string + ToPhysicalEntry(entry vault.Entry) vault.Entry + FromPhysicalEntry(entry vault.Entry) vault.Entry +} + +// Vault returns the physical datastore view. +func Vault(model vault.Model) View { + return physicalView{model: model} +} + +// VaultRoot returns the logical main-vault view rooted at the physical +// keepass storage group. +func VaultRoot(model vault.Model) View { + return rootView{model: model} +} + +// VaultRecycleBin returns the logical recycle-bin view. +func VaultRecycleBin(model vault.Model) View { + return recycleBinView{model: model} +} + +type physicalView struct { + model vault.Model +} + +func (v physicalView) ChildGroups(path []string) []string { + return v.model.ChildGroups(path) +} + +func (v physicalView) EntriesInPath(path []string) []vault.Entry { + return cloneEntries(v.model.EntriesInPath(path)) +} + +func (v physicalView) EntriesUnderPath(path []string) []vault.Entry { + return cloneEntries(v.model.EntriesUnderPath(path)) +} + +func (v physicalView) ToPhysicalPath(path []string) []string { + return clonePath(path) +} + +func (v physicalView) FromPhysicalPath(path []string) []string { + return clonePath(path) +} + +func (v physicalView) ToPhysicalEntry(entry vault.Entry) vault.Entry { + return cloneEntry(entry) +} + +func (v physicalView) FromPhysicalEntry(entry vault.Entry) vault.Entry { + return cloneEntry(entry) +} + +type rootView struct { + model vault.Model +} + +func (v rootView) ChildGroups(path []string) []string { + return v.model.ChildGroups(v.ToPhysicalPath(path)) +} + +func (v rootView) EntriesInPath(path []string) []vault.Entry { + return v.mapEntries(v.model.EntriesInPath(v.ToPhysicalPath(path))) +} + +func (v rootView) EntriesUnderPath(path []string) []vault.Entry { + return v.mapEntries(v.model.EntriesUnderPath(v.ToPhysicalPath(path))) +} + +func (v rootView) ToPhysicalPath(path []string) []string { + if len(path) == 0 { + return []string{KeepassRoot} + } + return append([]string{KeepassRoot}, clonePath(path)...) +} + +func (v rootView) FromPhysicalPath(path []string) []string { + if len(path) == 0 { + return nil + } + if path[0] != KeepassRoot { + return clonePath(path) + } + return clonePath(path[1:]) +} + +func (v rootView) ToPhysicalEntry(entry vault.Entry) vault.Entry { + entry = cloneEntry(entry) + entry.Path = v.ToPhysicalPath(entry.Path) + for i := range entry.History { + entry.History[i].Path = v.ToPhysicalPath(entry.History[i].Path) + } + return entry +} + +func (v rootView) FromPhysicalEntry(entry vault.Entry) vault.Entry { + entry = cloneEntry(entry) + entry.Path = v.FromPhysicalPath(entry.Path) + for i := range entry.History { + entry.History[i].Path = v.FromPhysicalPath(entry.History[i].Path) + } + return entry +} + +func (v rootView) mapEntries(entries []vault.Entry) []vault.Entry { + out := make([]vault.Entry, 0, len(entries)) + for _, entry := range entries { + out = append(out, v.FromPhysicalEntry(entry)) + } + return out +} + +type recycleBinView struct { + model vault.Model +} + +func (v recycleBinView) ChildGroups(path []string) []string { + return childGroups(v.model.RecycleBin, path) +} + +func (v recycleBinView) EntriesInPath(path []string) []vault.Entry { + return entriesInPath(v.model.RecycleBin, path) +} + +func (v recycleBinView) EntriesUnderPath(path []string) []vault.Entry { + var out []vault.Entry + for _, entry := range v.model.RecycleBin { + if len(path) > len(entry.Path) { + continue + } + if !slices.Equal(entry.Path[:len(path)], path) { + continue + } + out = append(out, cloneEntry(entry)) + } + slices.SortFunc(out, func(a, b vault.Entry) int { + switch { + case a.Title < b.Title: + return -1 + case a.Title > b.Title: + return 1 + default: + return 0 + } + }) + return out +} + +func (v recycleBinView) ToPhysicalPath(path []string) []string { + return clonePath(path) +} + +func (v recycleBinView) FromPhysicalPath(path []string) []string { + return clonePath(path) +} + +func (v recycleBinView) ToPhysicalEntry(entry vault.Entry) vault.Entry { + return cloneEntry(entry) +} + +func (v recycleBinView) FromPhysicalEntry(entry vault.Entry) vault.Entry { + return cloneEntry(entry) +} + +func childGroups(entries []vault.Entry, path []string) []string { + seen := map[string]bool{} + var groups []string + for _, entry := range entries { + if len(path) > len(entry.Path) { + continue + } + if !slices.Equal(entry.Path[:len(path)], path) { + continue + } + if len(entry.Path) == len(path) { + continue + } + group := entry.Path[len(path)] + if seen[group] { + continue + } + seen[group] = true + groups = append(groups, group) + } + slices.Sort(groups) + return groups +} + +func entriesInPath(entries []vault.Entry, path []string) []vault.Entry { + var out []vault.Entry + for _, entry := range entries { + if slices.Equal(entry.Path, path) { + out = append(out, cloneEntry(entry)) + } + } + slices.SortFunc(out, func(a, b vault.Entry) int { + switch { + case a.Title < b.Title: + return -1 + case a.Title > b.Title: + return 1 + default: + return 0 + } + }) + return out +} + +func cloneEntries(entries []vault.Entry) []vault.Entry { + if len(entries) == 0 { + return nil + } + out := make([]vault.Entry, len(entries)) + for i := range entries { + out[i] = cloneEntry(entries[i]) + } + return out +} + +func cloneEntry(entry vault.Entry) vault.Entry { + entry.Path = clonePath(entry.Path) + entry.Tags = slices.Clone(entry.Tags) + if entry.Fields != nil { + fields := make(map[string]string, len(entry.Fields)) + for key, value := range entry.Fields { + fields[key] = value + } + entry.Fields = fields + } + if entry.Attachments != nil { + attachments := make(map[string][]byte, len(entry.Attachments)) + for key, value := range entry.Attachments { + attachments[key] = slices.Clone(value) + } + entry.Attachments = attachments + } + if len(entry.History) != 0 { + history := make([]vault.Entry, len(entry.History)) + for i := range entry.History { + history[i] = cloneEntry(entry.History[i]) + } + entry.History = history + } + return entry +} + +func clonePath(path []string) []string { + if len(path) == 0 { + return nil + } + return slices.Clone(path) +} -- 2.52.0 From 59cd01f8e76166b06b103f259464414ad750757e Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Mon, 13 Apr 2026 07:12:32 -0700 Subject: [PATCH 5/9] Use vault views for entry and recycle-bin state --- TODO.md | 4 - internal/appstate/state.go | 149 ++++++++++++++++++++++++++++---- internal/appstate/state_test.go | 79 +++++++++++++++-- internal/appui/frame.go | 13 ++- internal/appui/main_test.go | 42 ++++----- internal/appui/recent_state.go | 81 ++++++++++++++--- internal/vaultview/view.go | 33 ++++++- 7 files changed, 338 insertions(+), 63 deletions(-) diff --git a/TODO.md b/TODO.md index c43ae90..3aa7188 100644 --- a/TODO.md +++ b/TODO.md @@ -8,10 +8,6 @@ The product is not complete until the global exit criteria at the end of this fi ## Priority Bugs -- Vault root view bug: - update `internal/appstate` and entries/recycle-bin UI plumbing to use - `VaultRoot` and `VaultRecycleBin` instead of raw datastore paths, removing - the hidden-root heuristic from entries browsing. - Vault root view bug: update gRPC/API-facing datastore reads and writes to use logical `VaultRoot` paths while keeping authorization on canonical `Vault` paths. diff --git a/internal/appstate/state.go b/internal/appstate/state.go index 1b7d52f..78319bf 100644 --- a/internal/appstate/state.go +++ b/internal/appstate/state.go @@ -11,6 +11,7 @@ import ( "git.julianfamily.org/keepassgo/internal/apiaudit" "git.julianfamily.org/keepassgo/internal/apitokens" "git.julianfamily.org/keepassgo/internal/vault" + "git.julianfamily.org/keepassgo/internal/vaultview" "git.julianfamily.org/keepassgo/internal/webdav" ) @@ -30,6 +31,8 @@ const ( SectionAbout Section = "about" ) +const entriesRootLabel = "Root" + type CurrentSession interface { Current() (vault.Model, error) } @@ -375,7 +378,7 @@ func (s *State) VisibleEntries() ([]vault.Entry, error) { } if s.Section == SectionEntries { - return entriesInPath(model.Entries, s.CurrentPath), nil + return entriesInPath(entries, logicalEntriesPathForModel(model, s.CurrentPath)), nil } if s.Section == SectionRecycleBin || len(s.CurrentPath) == 0 { return entries, nil @@ -401,7 +404,7 @@ func (s *State) ChildGroups() ([]string, error) { return childGroups(s.entriesForSection(model), s.CurrentPath), nil } - return model.ChildGroups(s.CurrentPath), nil + return vaultview.VaultRoot(model).ChildGroups(entriesViewPathForModel(model, s.CurrentPath)), nil } func (s *State) SelectVisibleIndex(index int) error { @@ -447,11 +450,11 @@ func (s *State) entriesForSection(model vault.Model) []vault.Entry { case SectionTemplates: return slices.Clone(model.Templates) case SectionRecycleBin: - return slices.Clone(model.RecycleBin) + return logicalEntries(vaultview.VaultRecycleBin(model).EntriesUnderPath(nil)) case SectionAPITokens, SectionAPIAudit, SectionAbout: return nil default: - return slices.Clone(model.Entries) + return logicalEntries(vaultview.VaultRoot(model).EntriesUnderPath(nil)) } } @@ -463,7 +466,9 @@ func (s State) SearchPathContext(entry vault.Entry) string { path = append([]string{"Templates"}, path...) } case SectionRecycleBin: - path = append([]string{"Recycle Bin"}, path...) + path = append([]string{"Recycle Bin"}, logicalEntriesPath(path)...) + case SectionEntries: + path = logicalEntriesPath(path) } return strings.Join(path, " / ") } @@ -520,6 +525,116 @@ func filterEntries(entries []vault.Entry, query string) []vault.Entry { return out } +func logicalEntriesPathForModel(model vault.Model, path []string) []string { + if len(path) == 0 { + return []string{entriesRootLabel} + } + if path[0] == entriesRootLabel { + return append([]string(nil), path...) + } + if usesPhysicalEntriesRoot(model) && path[0] == vaultview.KeepassRoot { + path = path[1:] + } + return append([]string{entriesRootLabel}, append([]string(nil), path...)...) +} + +func logicalEntriesPath(path []string) []string { + if len(path) == 0 { + return []string{entriesRootLabel} + } + if path[0] == entriesRootLabel { + return append([]string(nil), path...) + } + if path[0] == vaultview.KeepassRoot { + path = path[1:] + } + return append([]string{entriesRootLabel}, append([]string(nil), path...)...) +} + +func entriesViewPathForModel(model vault.Model, path []string) []string { + if len(path) == 0 { + return nil + } + switch { + case usesPhysicalEntriesRoot(model) && path[0] == entriesRootLabel: + return append([]string(nil), path[1:]...) + case usesLogicalEntriesRoot(model): + return append([]string(nil), path...) + case path[0] == entriesRootLabel: + return append([]string(nil), path[1:]...) + default: + return append([]string(nil), path...) + } +} + +func logicalEntry(entry vault.Entry) vault.Entry { + entry.Path = logicalEntriesPath(entry.Path) + for i := range entry.History { + entry.History[i] = logicalEntry(entry.History[i]) + } + return entry +} + +func logicalEntries(entries []vault.Entry) []vault.Entry { + if len(entries) == 0 { + return nil + } + out := make([]vault.Entry, len(entries)) + for i := range entries { + out[i] = logicalEntry(entries[i]) + } + return out +} + +func entryForModel(model vault.Model, entry vault.Entry) vault.Entry { + entry.Path = entriesViewPathForModel(model, entry.Path) + for i := range entry.History { + entry.History[i] = entryForModel(model, entry.History[i]) + } + return entry +} + +func usesPhysicalEntriesRoot(model vault.Model) bool { + if len(model.Entries) == 0 && len(model.Groups) == 0 && len(model.RecycleBin) == 0 { + return true + } + for _, group := range model.Groups { + if len(group) > 0 && group[0] == vaultview.KeepassRoot { + return true + } + } + for _, entry := range model.Entries { + if len(entry.Path) > 0 && entry.Path[0] == vaultview.KeepassRoot { + return true + } + } + for _, entry := range model.RecycleBin { + if len(entry.Path) > 0 && entry.Path[0] == vaultview.KeepassRoot { + return true + } + } + return false +} + +func usesLogicalEntriesRoot(model vault.Model) bool { + for _, group := range model.Groups { + if len(group) > 0 && group[0] == entriesRootLabel { + return true + } + } + for _, entry := range model.Entries { + if len(entry.Path) > 0 && entry.Path[0] == entriesRootLabel { + return true + } + } + for _, entry := range model.RecycleBin { + if len(entry.Path) > 0 && entry.Path[0] == entriesRootLabel { + return true + } + } + return false +} + func childGroups(entries []vault.Entry, path []string) []string { seen := map[string]bool{} var groups []string @@ -594,7 +709,7 @@ func (s *State) UpsertEntry(entry vault.Entry) error { return err } - model.UpsertEntry(entry) + model.UpsertEntry(vaultview.VaultRoot(model).ToPhysicalEntry(entryForModel(model, entry))) session.Replace(model) s.SelectedEntryID = entry.ID return s.markDirtyAndAutoSave() @@ -628,7 +743,7 @@ func (s *State) InstantiateTemplate(templateID string, overrides vault.Entry) (v return vault.Entry{}, err } - entry, err := model.InstantiateTemplate(templateID, overrides) + entry, err := model.InstantiateTemplate(templateID, vaultview.VaultRoot(model).ToPhysicalEntry(entryForModel(model, overrides))) if err != nil { return vault.Entry{}, err } @@ -638,7 +753,7 @@ func (s *State) InstantiateTemplate(templateID string, overrides vault.Entry) (v if err := s.markDirtyAndAutoSave(); err != nil { return vault.Entry{}, err } - return entry, nil + return logicalEntry(entry), nil } func (s *State) DeleteTemplate(id string) error { @@ -993,7 +1108,7 @@ func (s *State) CreateGroup(name string) error { return err } - model.CreateGroup(s.CurrentPath, name) + model.CreateGroup(vaultview.VaultRoot(model).ToPhysicalPath(entriesViewPathForModel(model, s.CurrentPath)), name) session.Replace(model) return s.markDirtyAndAutoSave() } @@ -1007,13 +1122,15 @@ func (s *State) MoveCurrentGroup(parent []string) error { if err != nil { return err } - current := append([]string(nil), s.CurrentPath...) - if err := model.MoveGroup(current, parent); err != nil { + current := logicalEntriesPathForModel(model, s.CurrentPath) + currentViewPath := entriesViewPathForModel(model, current) + parentViewPath := entriesViewPathForModel(model, parent) + if err := model.MoveGroup(vaultview.VaultRoot(model).ToPhysicalPath(currentViewPath), vaultview.VaultRoot(model).ToPhysicalPath(parentViewPath)); err != nil { return err } session.Replace(model) - if len(current) > 0 { - s.CurrentPath = append(append([]string(nil), parent...), current[len(current)-1]) + if len(currentViewPath) > 0 { + s.CurrentPath = logicalEntriesPathForModel(model, append(append([]string(nil), parentViewPath...), currentViewPath[len(currentViewPath)-1])) } return s.markDirtyAndAutoSave() } @@ -1029,7 +1146,7 @@ func (s *State) RenameCurrentGroup(newName string) error { return err } - if err := model.RenameGroup(s.CurrentPath, newName); err != nil { + if err := model.RenameGroup(vaultview.VaultRoot(model).ToPhysicalPath(entriesViewPathForModel(model, s.CurrentPath)), newName); err != nil { return err } @@ -1051,7 +1168,7 @@ func (s *State) MoveSelectedEntry(path []string) error { return err } - if err := model.MoveEntry(s.SelectedEntryID, path); err != nil { + if err := model.MoveEntry(s.SelectedEntryID, vaultview.VaultRoot(model).ToPhysicalPath(entriesViewPathForModel(model, path))); err != nil { return err } @@ -1070,7 +1187,7 @@ func (s *State) DeleteCurrentGroup() error { return err } - if err := model.DeleteGroup(s.CurrentPath); err != nil { + if err := model.DeleteGroup(vaultview.VaultRoot(model).ToPhysicalPath(entriesViewPathForModel(model, s.CurrentPath))); err != nil { return err } diff --git a/internal/appstate/state_test.go b/internal/appstate/state_test.go index 5888209..18279e2 100644 --- a/internal/appstate/state_test.go +++ b/internal/appstate/state_test.go @@ -27,7 +27,7 @@ func TestVisibleEntriesFollowsCurrentPathWithoutSearch(t *testing.T) { }, }, }, - CurrentPath: []string{"Crew", "Internet"}, + CurrentPath: []string{"Root", "Crew", "Internet"}, } got, err := state.VisibleEntries() @@ -583,6 +583,75 @@ func TestSearchPathContextIncludesSectionRoots(t *testing.T) { } } +func TestVisibleEntriesUseLogicalVaultRootForPhysicalKeepassModel(t *testing.T) { + t.Parallel() + + state := State{ + Session: stubSession{ + model: vault.Model{ + Entries: []vault.Entry{ + {ID: "bellagio", Title: "Bellagio", Path: []string{"keepass", "Crew", "Internet"}}, + {ID: "vault-console", Title: "Vault Console", Path: []string{"keepass", "Crew", "Internet"}}, + {ID: "surveillance-console", Title: "Surveillance Console", Path: []string{"keepass", "Crew", "Security Office"}}, + }, + Groups: [][]string{ + {"keepass"}, + {"keepass", "Crew"}, + {"keepass", "Crew", "Internet"}, + {"keepass", "Crew", "Security Office"}, + }, + }, + }, + CurrentPath: []string{"Crew", "Internet"}, + } + + got, err := state.VisibleEntries() + if err != nil { + t.Fatalf("VisibleEntries() error = %v", err) + } + + titles := make([]string, 0, len(got)) + for _, entry := range got { + titles = append(titles, entry.Title) + } + if !slices.Equal(titles, []string{"Bellagio", "Vault Console"}) { + t.Fatalf("VisibleEntries() titles = %v, want [Bellagio Vault Console]", titles) + } + if !slices.Equal(got[0].Path, []string{"Root", "Crew", "Internet"}) { + t.Fatalf("VisibleEntries()[0].Path = %v, want [Root Crew Internet]", got[0].Path) + } +} + +func TestChildGroupsUseLogicalVaultRootForPhysicalKeepassModel(t *testing.T) { + t.Parallel() + + state := State{ + Session: stubSession{ + model: vault.Model{ + Entries: []vault.Entry{ + {ID: "bellagio", Title: "Bellagio", Path: []string{"keepass", "Crew", "Internet"}}, + {ID: "surveillance-console", Title: "Surveillance Console", Path: []string{"keepass", "Crew", "Security Office"}}, + }, + Groups: [][]string{ + {"keepass"}, + {"keepass", "Crew"}, + {"keepass", "Crew", "Internet"}, + {"keepass", "Crew", "Security Office"}, + }, + }, + }, + } + + got, err := state.ChildGroups() + if err != nil { + t.Fatalf("ChildGroups() error = %v", err) + } + + if !slices.Equal(got, []string{"Crew"}) { + t.Fatalf("ChildGroups() = %v, want [Crew]", got) + } +} + func TestChildGroupsUsesCurrentModelAndCurrentPath(t *testing.T) { t.Parallel() @@ -1634,11 +1703,11 @@ func TestCreateGroupSupportsNestedGroupPath(t *testing.T) { t.Fatalf("CreateGroup() error = %v", err) } - if got := session.model.ChildGroups([]string{"Root"}); !slices.Equal(got, []string{"Infrastructure"}) { - t.Fatalf("ChildGroups(Root) = %v, want [Infrastructure]", got) + if got := session.model.ChildGroups([]string{"keepass"}); !slices.Equal(got, []string{"Infrastructure"}) { + t.Fatalf("ChildGroups(keepass) = %v, want [Infrastructure]", got) } - if got := session.model.ChildGroups([]string{"Root", "Infrastructure"}); !slices.Equal(got, []string{"Prod"}) { - t.Fatalf("ChildGroups(Root/Infrastructure) = %v, want [Prod]", got) + if got := session.model.ChildGroups([]string{"keepass", "Infrastructure"}); !slices.Equal(got, []string{"Prod"}) { + t.Fatalf("ChildGroups(keepass/Infrastructure) = %v, want [Prod]", got) } } diff --git a/internal/appui/frame.go b/internal/appui/frame.go index 78d4b0d..c8b04fe 100644 --- a/internal/appui/frame.go +++ b/internal/appui/frame.go @@ -24,6 +24,7 @@ import ( "git.julianfamily.org/keepassgo/internal/clipboard" "git.julianfamily.org/keepassgo/internal/session" "git.julianfamily.org/keepassgo/internal/vault" + "git.julianfamily.org/keepassgo/internal/vaultview" ) func (u *ui) bannerSurface() uiBanner { @@ -558,6 +559,11 @@ func copyPath(path []string) []string { } func pathExistsInModel(model vault.Model, path []string) bool { + if len(path) > 0 && path[0] == "Root" { + view := vaultview.VaultRoot(model) + viewPath := entriesViewPathForModel(model, path) + return len(view.EntriesInPath(viewPath)) > 0 || len(view.ChildGroups(viewPath)) > 0 || hasExactGroup(model, view.ToPhysicalPath(viewPath)) + } return len(model.EntriesInPath(path)) > 0 || len(model.ChildGroups(path)) > 0 || hasExactGroup(model, path) } @@ -569,9 +575,12 @@ func normalizeEntriesPathWithoutModel(path []string, root string) []string { return []string{root} } if path[0] == "Root" { + return copyPath(path) + } + if path[0] == vaultview.KeepassRoot { return append([]string{root}, path[1:]...) } - return copyPath(path) + return append([]string{root}, copyPath(path)...) } func (u *ui) normalizedEntriesPath(path []string) []string { @@ -590,7 +599,7 @@ func (u *ui) normalizedEntriesPath(path []string) []string { return []string{root} } if path[0] == "Root" && root != "" { - candidate := append([]string{root}, path[1:]...) + candidate := copyPath(path) if pathExistsInModel(model, candidate) { return candidate } diff --git a/internal/appui/main_test.go b/internal/appui/main_test.go index 7b4579f..62bada1 100644 --- a/internal/appui/main_test.go +++ b/internal/appui/main_test.go @@ -3606,11 +3606,11 @@ func TestUICreateGroupActionSupportsNestedSubgroups(t *testing.T) { t.Fatalf("createGroupAction() error = %v", err) } - if got := u.state.Session.(*uiSession).model.ChildGroups([]string{"Root"}); !slices.Equal(got, []string{"Infrastructure"}) { - t.Fatalf("ChildGroups(Root) = %v, want [Infrastructure]", got) + if got := u.state.Session.(*uiSession).model.ChildGroups([]string{"keepass"}); !slices.Equal(got, []string{"Infrastructure"}) { + t.Fatalf("ChildGroups(keepass) = %v, want [Infrastructure]", got) } - if got := u.state.Session.(*uiSession).model.ChildGroups([]string{"Root", "Infrastructure"}); !slices.Equal(got, []string{"Prod"}) { - t.Fatalf("ChildGroups(Root/Infrastructure) = %v, want [Prod]", got) + if got := u.state.Session.(*uiSession).model.ChildGroups([]string{"keepass", "Infrastructure"}); !slices.Equal(got, []string{"Prod"}) { + t.Fatalf("ChildGroups(keepass/Infrastructure) = %v, want [Prod]", got) } } @@ -5125,8 +5125,8 @@ func TestUIAutoEntersSingleVaultRootGroupAndDisplaysSlashRoot(t *testing.T) { t.Fatalf("openVaultAction() error = %v", err) } - if got := u.currentPath; !slices.Equal(got, []string{"keepass"}) { - t.Fatalf("currentPath = %v, want [keepass]", got) + if got := u.currentPath; !slices.Equal(got, []string{"Root"}) { + t.Fatalf("currentPath = %v, want [Root]", got) } if got := u.displayPath(); len(got) != 0 { t.Fatalf("displayPath() = %v, want root slash path", got) @@ -5152,8 +5152,8 @@ func TestUIAutoEntersSingleVaultRootWhenRecycleBinAlsoExists(t *testing.T) { u.showEntriesSection() - if got := u.currentPath; !slices.Equal(got, []string{"keepass"}) { - t.Fatalf("currentPath = %v, want [keepass]", got) + if got := u.currentPath; !slices.Equal(got, []string{"Root"}) { + t.Fatalf("currentPath = %v, want [Root]", got) } if got := u.displayPath(); len(got) != 0 { t.Fatalf("displayPath() = %v, want root slash path", got) @@ -5174,15 +5174,15 @@ func TestUIShowEntriesSectionRestoresHiddenRootAfterLeavingEntries(t *testing.T) }) u.showEntriesSection() - if got := u.currentPath; !slices.Equal(got, []string{"keepass"}) { - t.Fatalf("currentPath after initial entries section = %v, want [keepass]", got) + if got := u.currentPath; !slices.Equal(got, []string{"Root"}) { + t.Fatalf("currentPath after initial entries section = %v, want [Root]", got) } u.showAPITokensSection() u.showEntriesSection() - if got := u.currentPath; !slices.Equal(got, []string{"keepass"}) { - t.Fatalf("currentPath after returning to entries = %v, want [keepass]", got) + if got := u.currentPath; !slices.Equal(got, []string{"Root"}) { + t.Fatalf("currentPath after returning to entries = %v, want [Root]", got) } if got := u.displayPath(); len(got) != 0 { t.Fatalf("displayPath() after returning to entries = %v, want root slash path", got) @@ -5215,8 +5215,8 @@ func TestUISyncCurrentPathNormalizesHiddenRootAfterSectionSwitch(t *testing.T) { u.syncCurrentPath() - if got := u.currentPath; !slices.Equal(got, []string{"keepass"}) { - t.Fatalf("currentPath after syncCurrentPath() = %v, want [keepass]", got) + if got := u.currentPath; !slices.Equal(got, []string{"Root"}) { + t.Fatalf("currentPath after syncCurrentPath() = %v, want [Root]", got) } if got := u.displayPath(); len(got) != 0 { t.Fatalf("displayPath() after syncCurrentPath() = %v, want root slash path", got) @@ -5235,7 +5235,7 @@ func TestUIShowEntriesSectionRestoresEntriesViewState(t *testing.T) { }) u.showEntriesSection() - u.setCurrentPath([]string{"keepass", "Crew", "Internet"}) + u.setCurrentPath([]string{"Root", "Crew", "Internet"}) u.search.SetText("amazon") u.filter() u.state.SelectedEntryID = "amazon" @@ -5245,8 +5245,8 @@ func TestUIShowEntriesSectionRestoresEntriesViewState(t *testing.T) { u.showAPITokensSection() u.showEntriesSection() - if got := u.currentPath; !slices.Equal(got, []string{"keepass", "Crew", "Internet"}) { - t.Fatalf("currentPath after returning to entries = %v, want [keepass Crew Internet]", got) + if got := u.currentPath; !slices.Equal(got, []string{"Root", "Crew", "Internet"}) { + t.Fatalf("currentPath after returning to entries = %v, want [Root Crew Internet]", got) } if got := u.search.Text(); got != "amazon" { t.Fatalf("search text after returning to entries = %q, want amazon", got) @@ -8073,7 +8073,7 @@ func TestUISelectedRemoteCardUsesLocalCacheSummaryForBoundRemote(t *testing.T) { wantDetails := []string{ "/vaults/cache", "Sync target: home.kdbx · dav.example.invalid", - "Last group: Root / Internet", + "Last group: Internet", } if !slices.Equal(gotDetails, wantDetails) { t.Fatalf("selectedRemoteCardDetailLines() = %v, want %v", gotDetails, wantDetails) @@ -8105,7 +8105,7 @@ func TestUISelectedRemoteCardUsesConnectionSummaryWithoutLocalCache(t *testing.T wantDetails := []string{ "Path: vaults/home.kdbx", "Server: https://dav.example.invalid", - "Last group: Root / Internet", + "Last group: Internet", } if !slices.Equal(gotDetails, wantDetails) { t.Fatalf("selectedRemoteCardDetailLines() = %v, want %v", gotDetails, wantDetails) @@ -9327,8 +9327,8 @@ func TestUIAPIPolicyTargetActionsUseCurrentContext(t *testing.T) { if err := u.useCurrentGroupForPolicyAction(); err != nil { t.Fatalf("useCurrentGroupForPolicyAction() error = %v", err) } - if got := u.apiPolicyPath.Text(); got != "bashertarr" { - t.Fatalf("apiPolicyPath.Text() = %q, want %q", got, "bashertarr") + if got := u.apiPolicyPath.Text(); got != "Crew / bashertarr" { + t.Fatalf("apiPolicyPath.Text() = %q, want %q", got, "Crew / bashertarr") } if !u.apiPolicyGroupScopeW.Value { t.Fatal("apiPolicyGroupScopeW.Value = false, want true") diff --git a/internal/appui/recent_state.go b/internal/appui/recent_state.go index 8cb49b4..2936f36 100644 --- a/internal/appui/recent_state.go +++ b/internal/appui/recent_state.go @@ -1260,14 +1260,10 @@ func (u *ui) recentVaultGroup(path string) []string { } func (u *ui) hiddenVaultRoot() string { - if u.state.Section != appstate.SectionEntries { - return "" + if u.state.Section == appstate.SectionEntries { + return "Root" } - model, err := u.state.Session.Current() - if err != nil { - return "" - } - return vaultview.HiddenRoot(model) + return "" } func (u *ui) enterHiddenVaultRoot() { @@ -1294,7 +1290,7 @@ func (u *ui) restoreRecentVaultGroup(path string) { u.setCurrentPath(saved) return } - if len(model.EntriesInPath(saved)) > 0 || len(model.ChildGroups(saved)) > 0 || hasExactGroup(model, saved) { + if pathExistsInModel(model, saved) { u.setCurrentPath(saved) return } @@ -1317,7 +1313,7 @@ func (u *ui) restoreRecentRemoteGroup(baseURL, path string) { u.setCurrentPath(saved) return } - if len(model.EntriesInPath(saved)) > 0 || len(model.ChildGroups(saved)) > 0 || hasExactGroup(model, saved) { + if pathExistsInModel(model, saved) { u.setCurrentPath(saved) return } @@ -1339,7 +1335,7 @@ func (u *ui) restoreEntriesPath(path []string) { u.setCurrentPath(path) return } - if len(model.EntriesInPath(path)) > 0 || len(model.ChildGroups(path)) > 0 || hasExactGroup(model, path) { + if pathExistsInModel(model, path) { u.setCurrentPath(path) return } @@ -1415,6 +1411,22 @@ func pathHasPrefix(path, prefix []string) bool { return slices.Equal(path[:len(prefix)], prefix) } +func entriesViewPathForModel(model vault.Model, path []string) []string { + if len(path) == 0 { + return nil + } + switch { + case usesPhysicalEntriesRoot(model) && path[0] == "Root": + return append([]string(nil), path[1:]...) + case usesLogicalEntriesRoot(model): + return append([]string(nil), path...) + case path[0] == "Root": + return append([]string(nil), path[1:]...) + default: + return append([]string(nil), path...) + } +} + func hasExactGroup(model vault.Model, path []string) bool { for _, group := range model.Groups { if slices.Equal(group, path) { @@ -1433,12 +1445,14 @@ func (u *ui) currentGroupDeletionState() (bool, string) { if err != nil { return false, "" } - path := append([]string(nil), u.currentPath...) - if len(model.ChildGroups(path)) > 0 { + view := vaultview.VaultRoot(model) + path := entriesViewPathForModel(model, u.currentPath) + physicalPath := view.ToPhysicalPath(path) + if len(model.ChildGroups(physicalPath)) > 0 { return false, "This group contains child groups. Move or delete them before removing the group." } for _, item := range model.Entries { - if slices.Equal(item.Path, path) || pathHasPrefix(item.Path, path) { + if slices.Equal(item.Path, physicalPath) || pathHasPrefix(item.Path, physicalPath) { return false, "This group contains entries. Move or delete them before removing the group." } } @@ -1450,6 +1464,47 @@ func (u *ui) currentGroupDeletionState() (bool, string) { return true, "Deleting this empty group will not remove any entries." } +func usesPhysicalEntriesRoot(model vault.Model) bool { + if len(model.Entries) == 0 && len(model.Groups) == 0 && len(model.RecycleBin) == 0 { + return true + } + for _, group := range model.Groups { + if len(group) > 0 && group[0] == vaultview.KeepassRoot { + return true + } + } + for _, entry := range model.Entries { + if len(entry.Path) > 0 && entry.Path[0] == vaultview.KeepassRoot { + return true + } + } + for _, entry := range model.RecycleBin { + if len(entry.Path) > 0 && entry.Path[0] == vaultview.KeepassRoot { + return true + } + } + return false +} + +func usesLogicalEntriesRoot(model vault.Model) bool { + for _, group := range model.Groups { + if len(group) > 0 && group[0] == "Root" { + return true + } + } + for _, entry := range model.Entries { + if len(entry.Path) > 0 && entry.Path[0] == "Root" { + return true + } + } + for _, entry := range model.RecycleBin { + if len(entry.Path) > 0 && entry.Path[0] == "Root" { + return true + } + } + return false +} + func (u *ui) deleteGroupPendingConfirmation() bool { return len(u.deleteGroupPath) > 0 && slices.Equal(u.deleteGroupPath, u.currentPath) } diff --git a/internal/vaultview/view.go b/internal/vaultview/view.go index 23e82b3..b9500d1 100644 --- a/internal/vaultview/view.go +++ b/internal/vaultview/view.go @@ -28,7 +28,7 @@ func Vault(model vault.Model) View { // VaultRoot returns the logical main-vault view rooted at the physical // keepass storage group. func VaultRoot(model vault.Model) View { - return rootView{model: model} + return rootView{model: model, rooted: usesKeepassRoot(model)} } // VaultRecycleBin returns the logical recycle-bin view. @@ -69,7 +69,8 @@ func (v physicalView) FromPhysicalEntry(entry vault.Entry) vault.Entry { } type rootView struct { - model vault.Model + model vault.Model + rooted bool } func (v rootView) ChildGroups(path []string) []string { @@ -85,6 +86,9 @@ func (v rootView) EntriesUnderPath(path []string) []vault.Entry { } func (v rootView) ToPhysicalPath(path []string) []string { + if !v.rooted { + return clonePath(path) + } if len(path) == 0 { return []string{KeepassRoot} } @@ -92,6 +96,9 @@ func (v rootView) ToPhysicalPath(path []string) []string { } func (v rootView) FromPhysicalPath(path []string) []string { + if !v.rooted { + return clonePath(path) + } if len(path) == 0 { return nil } @@ -267,3 +274,25 @@ func clonePath(path []string) []string { } return slices.Clone(path) } + +func usesKeepassRoot(model vault.Model) bool { + if len(model.Entries) == 0 && len(model.Groups) == 0 && len(model.RecycleBin) == 0 { + return true + } + for _, group := range model.Groups { + if len(group) > 0 && group[0] == KeepassRoot { + return true + } + } + for _, entry := range model.Entries { + if len(entry.Path) > 0 && entry.Path[0] == KeepassRoot { + return true + } + } + for _, entry := range model.RecycleBin { + if len(entry.Path) > 0 && entry.Path[0] == KeepassRoot { + return true + } + } + return false +} -- 2.52.0 From 9882d3fc04aab52e55606916a1545e9a9aa4b5ae Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Mon, 13 Apr 2026 07:15:16 -0700 Subject: [PATCH 6/9] Authorize logical root API paths against vault storage --- TODO.md | 3 --- internal/api/server.go | 41 +++++++++++++++++++++++++++++++++++-- internal/api/server_test.go | 14 ++++++------- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/TODO.md b/TODO.md index 3aa7188..7fb2ba3 100644 --- a/TODO.md +++ b/TODO.md @@ -8,9 +8,6 @@ The product is not complete until the global exit criteria at the end of this fi ## Priority Bugs -- Vault root view bug: - update gRPC/API-facing datastore reads and writes to use logical `VaultRoot` - paths while keeping authorization on canonical `Vault` paths. - Vault root view bug: add API-token path translation helpers so token editing, approval prompts, and audit/resource display stop leaking the physical `keepass` root. diff --git a/internal/api/server.go b/internal/api/server.go index 2d5fc7f..5aba80b 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -291,7 +291,7 @@ func (s *Server) FindBrowserLogins(ctx context.Context, req *keepassgov1.FindBro }, score: score, resource: resource, - decision: apitokens.Evaluate(token, apitokens.OperationListEntries, resource), + decision: evaluateAuthorization(model, token, apitokens.OperationListEntries, resource), }) } slices.SortFunc(matches, func(a, b rankedBrowserMatch) int { @@ -1220,7 +1220,8 @@ func (s *Server) authorizeTemplateRequest(ctx context.Context, op apitokens.Oper } func (s *Server) authorizeResourceRequest(ctx context.Context, token apitokens.Token, op apitokens.Operation, resource apitokens.Resource) (apitokens.Token, error) { - switch apitokens.Evaluate(token, op, resource) { + model, _ := s.snapshotModel() + switch evaluateAuthorization(model, token, op, resource) { case apitokens.DecisionAllow: return token, nil case apitokens.DecisionDeny: @@ -1337,6 +1338,42 @@ func hasPolicyRule(rules []apitokens.PolicyRule, target apitokens.PolicyRule) bo return false } +func evaluateAuthorization(model vault.Model, token apitokens.Token, op apitokens.Operation, resource apitokens.Resource) apitokens.Decision { + return apitokens.Evaluate(canonicalizeTokenForAuthorization(model, token), op, canonicalizeAuthorizationResource(model, resource)) +} + +func canonicalizeTokenForAuthorization(model vault.Model, token apitokens.Token) apitokens.Token { + token.Policies = append([]apitokens.PolicyRule(nil), token.Policies...) + for i := range token.Policies { + token.Policies[i].Resource = canonicalizeAuthorizationResource(model, token.Policies[i].Resource) + } + return token +} + +func canonicalizeAuthorizationResource(model vault.Model, resource apitokens.Resource) apitokens.Resource { + resource.Path = canonicalAuthorizationPath(model, resource.Path) + return resource +} + +func canonicalAuthorizationPath(model vault.Model, path []string) []string { + if len(path) == 0 { + return nil + } + if path[0] == vaultview.KeepassRoot { + return append([]string(nil), path...) + } + if path[0] == "Root" { + if len(path) > 1 && (path[1] == "Templates" || path[1] == "API Tokens") { + return append([]string(nil), path...) + } + return vaultview.VaultRoot(model).ToPhysicalPath(path[1:]) + } + if path[0] == "Templates" || path[0] == "API Tokens" { + return append([]string(nil), path...) + } + return vaultview.VaultRoot(model).ToPhysicalPath(path) +} + func copyOperation(target string) apitokens.Operation { switch clipboard.Target(target) { case clipboard.TargetUsername: diff --git a/internal/api/server_test.go b/internal/api/server_test.go index 4d55fce..08b9cfb 100644 --- a/internal/api/server_test.go +++ b/internal/api/server_test.go @@ -316,7 +316,7 @@ func TestVaultServiceFindsBrowserLoginsWithinAuthorizedGroupScope(t *testing.T) Path: []string{"keepass", "Joe", "Internet"}, }, testAPITokenEntry(t, - apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"keepass", "Joe", "codex"}}}, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"Root", "Joe", "codex"}}}, ), }, }) @@ -452,7 +452,7 @@ func TestVaultServiceListEntriesHidesSingleInternalVaultRoot(t *testing.T) { Path: []string{"keepass", "Joe", "codex"}, }, testAPITokenEntry(t, - apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"keepass", "Joe", "codex"}}}, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"Root", "Joe", "codex"}}}, ), }, Groups: [][]string{ @@ -491,7 +491,7 @@ func TestVaultServiceListEntriesHidesSingleInternalVaultRootWhenRecycleBinExists Path: []string{"keepass", "Joe", "codex"}, }, testAPITokenEntry(t, - apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"keepass", "Joe", "codex"}}}, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"Root", "Joe", "codex"}}}, ), }, Groups: [][]string{ @@ -523,7 +523,7 @@ func TestVaultServiceListGroupsHidesSingleInternalVaultRoot(t *testing.T) { client, _, cleanup := newTestClientForModel(t, vault.Model{ Entries: []vault.Entry{ testAPITokenEntry(t, - apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListGroups, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"keepass"}}}, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListGroups, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"Root"}}}, ), }, Groups: [][]string{ @@ -549,7 +549,7 @@ func TestVaultServiceListGroupsHidesSingleInternalVaultRootWhenRecycleBinExists( client, _, cleanup := newTestClientForModel(t, vault.Model{ Entries: []vault.Entry{ testAPITokenEntry(t, - apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListGroups, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"keepass"}}}, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListGroups, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"Root"}}}, ), }, Groups: [][]string{ @@ -1387,8 +1387,8 @@ func TestVaultServiceUpsertsNewEntryWithinAuthorizedGroupScope(t *testing.T) { client, _, cleanup := newTestClientForModel(t, vault.Model{ Entries: []vault.Entry{ testAPITokenEntry(t, - apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationMutateEntry, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"keepass", "Joe", "codex"}}}, - apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"keepass", "Joe", "codex"}}}, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationMutateEntry, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"Root", "Joe", "codex"}}}, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"Root", "Joe", "codex"}}}, ), }, Groups: [][]string{ -- 2.52.0 From 6790399e2407b6d1efb319d02527f80503bc278f Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Mon, 13 Apr 2026 07:18:33 -0700 Subject: [PATCH 7/9] Hide physical keepass paths in token and approval UX --- TODO.md | 3 --- internal/api/server.go | 31 +++++++++++++++++----- internal/api/server_test.go | 52 +++++++++++++++++++++++++++++++++++++ internal/appui/api/model.go | 16 +++++++++++- internal/appui/api_views.go | 6 ++--- internal/appui/app.go | 3 ++- internal/appui/main_test.go | 43 ++++++++++++++++++++++++++++++ 7 files changed, 140 insertions(+), 14 deletions(-) diff --git a/TODO.md b/TODO.md index 7fb2ba3..d5bd714 100644 --- a/TODO.md +++ b/TODO.md @@ -8,9 +8,6 @@ The product is not complete until the global exit criteria at the end of this fi ## Priority Bugs -- Vault root view bug: - add API-token path translation helpers so token editing, approval prompts, and - audit/resource display stop leaking the physical `keepass` root. - Vault root view bug: ensure vault create/open maintains the physical `keepass` storage root, lazily creates recycle-bin structure when needed, and warns when opening a diff --git a/internal/api/server.go b/internal/api/server.go index 5aba80b..469ee4b 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -1221,6 +1221,7 @@ func (s *Server) authorizeTemplateRequest(ctx context.Context, op apitokens.Oper func (s *Server) authorizeResourceRequest(ctx context.Context, token apitokens.Token, op apitokens.Operation, resource apitokens.Resource) (apitokens.Token, error) { model, _ := s.snapshotModel() + displayResource := displayAuthorizationResource(resource) switch evaluateAuthorization(model, token, op, resource) { case apitokens.DecisionAllow: return token, nil @@ -1233,9 +1234,9 @@ func (s *Server) authorizeResourceRequest(ctx context.Context, token apitokens.T TokenName: token.Name, ClientName: token.ClientName, Operation: op, - Resource: resource, + Resource: displayResource, }) - result, err := s.approvals.Request(ctx, token, op, resource) + result, err := s.approvals.Request(ctx, token, op, displayResource) if result.Rule != nil { if persistErr := s.persistApprovalRule(token.ID, *result.Rule); persistErr != nil { return apitokens.Token{}, status.Errorf(codes.Internal, "persist approval decision: %v", persistErr) @@ -1249,7 +1250,7 @@ func (s *Server) authorizeResourceRequest(ctx context.Context, token apitokens.T TokenName: token.Name, ClientName: token.ClientName, Operation: op, - Resource: resource, + Resource: displayResource, }) return token, nil case errors.Is(err, apiapproval.ErrRequestDenied): @@ -1259,7 +1260,7 @@ func (s *Server) authorizeResourceRequest(ctx context.Context, token apitokens.T TokenName: token.Name, ClientName: token.ClientName, Operation: op, - Resource: resource, + Resource: displayResource, }) return apitokens.Token{}, status.Error(codes.PermissionDenied, "access denied by user approval") case errors.Is(err, apiapproval.ErrRequestCanceled): @@ -1269,7 +1270,7 @@ func (s *Server) authorizeResourceRequest(ctx context.Context, token apitokens.T TokenName: token.Name, ClientName: token.ClientName, Operation: op, - Resource: resource, + Resource: displayResource, }) return apitokens.Token{}, status.Error(codes.Unauthenticated, "authorization request canceled") case errors.Is(err, apiapproval.ErrRequestTimedOut): @@ -1279,7 +1280,7 @@ func (s *Server) authorizeResourceRequest(ctx context.Context, token apitokens.T TokenName: token.Name, ClientName: token.ClientName, Operation: op, - Resource: resource, + Resource: displayResource, }) return apitokens.Token{}, status.Error(codes.DeadlineExceeded, "authorization request timed out") case errors.Is(err, context.Canceled): @@ -1355,6 +1356,11 @@ func canonicalizeAuthorizationResource(model vault.Model, resource apitokens.Res return resource } +func displayAuthorizationResource(resource apitokens.Resource) apitokens.Resource { + resource.Path = displayAuthorizationPath(resource.Path) + return resource +} + func canonicalAuthorizationPath(model vault.Model, path []string) []string { if len(path) == 0 { return nil @@ -1374,6 +1380,19 @@ func canonicalAuthorizationPath(model vault.Model, path []string) []string { return vaultview.VaultRoot(model).ToPhysicalPath(path) } +func displayAuthorizationPath(path []string) []string { + if len(path) == 0 { + return nil + } + if path[0] == vaultview.KeepassRoot { + return append([]string{"Root"}, append([]string(nil), path[1:]...)...) + } + if path[0] == "Root" { + return append([]string(nil), path...) + } + return append([]string(nil), path...) +} + func copyOperation(target string) apitokens.Operation { switch clipboard.Target(target) { case clipboard.TargetUsername: diff --git a/internal/api/server_test.go b/internal/api/server_test.go index 08b9cfb..85fb51b 100644 --- a/internal/api/server_test.go +++ b/internal/api/server_test.go @@ -396,6 +396,58 @@ func TestVaultServiceFindsBrowserLoginsRechecksChildPoliciesAfterPrompt(t *testi } } +func TestVaultServiceApprovalRequestsUseLogicalRootPathForPhysicalVault(t *testing.T) { + t.Parallel() + + model := vault.Model{ + Entries: []vault.Entry{ + { + ID: "codex-nextcloud", + Title: "Nextcloud (codex)", + Username: "jjulian", + Password: "secret-1", + URL: "https://nextcloud.example.invalid", + Path: []string{"keepass", "Joe", "codex"}, + }, + testAPITokenEntry(t), + }, + Groups: [][]string{ + {"keepass"}, + {"keepass", "Joe"}, + {"keepass", "Joe", "codex"}, + }, + } + client, _, service, cleanup := newTestHarnessForModel(t, model) + defer cleanup() + service.approvals = apiapproval.NewBroker(time.Minute) + + respCh := make(chan *keepassgov1.ListEntriesResponse, 1) + errCh := make(chan error, 1) + go func() { + resp, err := client.ListEntries(tokenContext(defaultTestTokenSecret), &keepassgov1.ListEntriesRequest{ + Path: []string{"Joe", "codex"}, + }) + respCh <- resp + errCh <- err + }() + + pending := waitForServerPendingApproval(t, service, 1)[0] + if got := pending.Resource.Path; !slices.Equal(got, []string{"Root", "Joe", "codex"}) { + t.Fatalf("pending.Resource.Path = %v, want [Root Joe codex]", got) + } + if _, _, err := service.ResolveApproval(pending.ID, apiapproval.OutcomeAllowOnce); err != nil { + t.Fatalf("ResolveApproval(allow once) error = %v", err) + } + + if err := <-errCh; err != nil { + t.Fatalf("ListEntries() error = %v", err) + } + resp := <-respCh + if len(resp.GetEntries()) != 1 || resp.GetEntries()[0].GetId() != "codex-nextcloud" { + t.Fatalf("ListEntries().Entries = %#v, want codex-nextcloud after approval", resp.GetEntries()) + } +} + func TestVaultServiceDoesNotMatchSpecificBrowserEntryToParentDomain(t *testing.T) { t.Parallel() diff --git a/internal/appui/api/model.go b/internal/appui/api/model.go index 1ec92bc..74dc47f 100644 --- a/internal/appui/api/model.go +++ b/internal/appui/api/model.go @@ -71,7 +71,7 @@ func AuditEventSearchTerms(event apiaudit.Event) string { event.ClientName, string(event.Operation), AuditOperationLabel(event.Operation), - strings.Join(event.Resource.Path, " / "), + FormatResourcePath(event.Resource.Path), event.Resource.EntryID, event.Message, } @@ -91,3 +91,17 @@ func AuditEventSearchTerms(event apiaudit.Event) string { } return strings.ToLower(strings.Join(parts, " ")) } + +func DisplayResourcePath(path []string) []string { + if len(path) == 0 { + return nil + } + if path[0] == "keepass" { + return append([]string{"Root"}, append([]string(nil), path[1:]...)...) + } + return append([]string(nil), path...) +} + +func FormatResourcePath(path []string) string { + return strings.Join(DisplayResourcePath(path), " / ") +} diff --git a/internal/appui/api_views.go b/internal/appui/api_views.go index 4a53aa7..2a9d391 100644 --- a/internal/appui/api_views.go +++ b/internal/appui/api_views.go @@ -336,7 +336,7 @@ func (u *ui) editAPIPolicyRuleAction(index int) error { } u.apiPolicyGroupScope = true u.apiPolicyGroupScopeW.Value = true - u.apiPolicyPath.SetText(strings.Join(rule.Resource.Path, " / ")) + u.apiPolicyPath.SetText(apiui.FormatResourcePath(rule.Resource.Path)) u.apiPolicyEntryID.SetText("") return nil } @@ -476,7 +476,7 @@ func policyRuleParts(rule apitokens.PolicyRule) (string, string, string) { if rule.Resource.Kind == apitokens.ResourceEntry { resource = "Entry: " + rule.Resource.EntryID } else if len(rule.Resource.Path) > 0 { - resource = strings.Join(rule.Resource.Path, " / ") + resource = apiui.FormatResourcePath(rule.Resource.Path) } return effect, operation, resource } @@ -1211,5 +1211,5 @@ func formatAuditResource(resource apitokens.Resource) string { if len(resource.Path) == 0 { return "/" } - return strings.Join(resource.Path, " / ") + return apiui.FormatResourcePath(resource.Path) } diff --git a/internal/appui/app.go b/internal/appui/app.go index f6fd130..170ace8 100644 --- a/internal/appui/app.go +++ b/internal/appui/app.go @@ -27,6 +27,7 @@ import ( "git.julianfamily.org/keepassgo/internal/apiaudit" "git.julianfamily.org/keepassgo/internal/apitokens" "git.julianfamily.org/keepassgo/internal/appstate" + apiui "git.julianfamily.org/keepassgo/internal/appui/api" detailmodel "git.julianfamily.org/keepassgo/internal/appui/detail" detaillayout "git.julianfamily.org/keepassgo/internal/appui/detail/layout" lifecyclemodel "git.julianfamily.org/keepassgo/internal/appui/lifecycle" @@ -1511,7 +1512,7 @@ func approvalResourceText(request apiapproval.Request) string { } case apitokens.ResourceGroup: if len(request.Resource.Path) > 0 { - return strings.Join(request.Resource.Path, " / ") + return apiui.FormatResourcePath(request.Resource.Path) } } return "Vault root" diff --git a/internal/appui/main_test.go b/internal/appui/main_test.go index 62bada1..59e5212 100644 --- a/internal/appui/main_test.go +++ b/internal/appui/main_test.go @@ -9355,6 +9355,49 @@ func TestUIAPIPolicyTargetActionsUseCurrentContext(t *testing.T) { } } +func TestUIEditAPIPolicyRuleHidesPhysicalKeepassRoot(t *testing.T) { + t.Parallel() + + token := apitokens.Token{ + ID: "token-1", + Name: "Crew Browser", + Policies: []apitokens.PolicyRule{{ + Effect: apitokens.EffectAllow, + Operation: apitokens.OperationListEntries, + Resource: apitokens.Resource{ + Kind: apitokens.ResourceGroup, + Path: []string{"keepass", "Crew", "bashertarr"}, + }, + }}, + } + u := newUIWithModel("desktop", vault.Model{ + Entries: []vault.Entry{ + token.Entry(apitokens.EntryPath), + }, + }) + u.showAPITokensSection() + u.state.SelectedEntryID = "token-1" + + if err := u.editAPIPolicyRuleAction(0); err != nil { + t.Fatalf("editAPIPolicyRuleAction() error = %v", err) + } + if got := u.apiPolicyPath.Text(); got != "Root / Crew / bashertarr" { + t.Fatalf("apiPolicyPath.Text() = %q, want %q", got, "Root / Crew / bashertarr") + } +} + +func TestUIAuditAndApprovalFormattingHidePhysicalKeepassRoot(t *testing.T) { + t.Parallel() + + resource := apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"keepass", "Crew", "bashertarr"}} + if got := formatAuditResource(resource); got != "Root / Crew / bashertarr" { + t.Fatalf("formatAuditResource() = %q, want %q", got, "Root / Crew / bashertarr") + } + if got := approvalResourceText(apiapproval.Request{Resource: resource}); got != "Root / Crew / bashertarr" { + t.Fatalf("approvalResourceText() = %q, want %q", got, "Root / Crew / bashertarr") + } +} + func TestUIVisibleBreadcrumbsCompressesAggressivelyOnPhone(t *testing.T) { t.Parallel() -- 2.52.0 From eccfb886ee65826e32ce3f7b2ee9bc193fc2bc70 Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Mon, 13 Apr 2026 07:29:51 -0700 Subject: [PATCH 8/9] Normalize vault storage root on open and create --- TODO.md | 7 --- internal/appstate/state.go | 18 +++++- internal/appui/main_test.go | 55 ++++++++++++++---- internal/session/session.go | 97 ++++++++++++++++++++++++-------- internal/session/session_test.go | 75 ++++++++++++++++++++---- internal/vault/kdbx.go | 5 ++ internal/vault/kdbx_test.go | 51 +++++++++++++++++ 7 files changed, 252 insertions(+), 56 deletions(-) diff --git a/TODO.md b/TODO.md index d5bd714..bd73621 100644 --- a/TODO.md +++ b/TODO.md @@ -6,13 +6,6 @@ These segments are intended to be independently executable wherever possible. Each segment has its own local exit criteria. The product is not complete until the global exit criteria at the end of this file are also met. -## Priority Bugs - -- Vault root view bug: - ensure vault create/open maintains the physical `keepass` storage root, - lazily creates recycle-bin structure when needed, and warns when opening a - legacy vault that had to be normalized. - ## UI Review Follow-Ups These items came from a hands-on emulator and desktop walkthrough. diff --git a/internal/appstate/state.go b/internal/appstate/state.go index 78319bf..9c886ad 100644 --- a/internal/appstate/state.go +++ b/internal/appstate/state.go @@ -101,6 +101,10 @@ type RemoteOpenableSession interface { OpenRemote(webdav.Client, string, vault.MasterKey) error } +type WarningSession interface { + ConsumeWarning() string +} + type SecurityConfigurableSession interface { ConfigureSecurity(vault.SecuritySettings) error SecuritySettings() vault.SecuritySettings @@ -841,7 +845,13 @@ func (s *State) Unlock(key vault.MasterKey) error { return fmt.Errorf("session is not lockable") } - return session.Unlock(key) + if err := session.Unlock(key); err != nil { + return err + } + if warningSession, ok := s.Session.(WarningSession); ok { + s.StatusMessage = warningSession.ConsumeWarning() + } + return nil } func (s *State) ChangeMasterKey(key vault.MasterKey) error { @@ -1003,6 +1013,9 @@ func (s *State) OpenVault(path string, key vault.MasterKey) error { s.CurrentPath = nil s.SelectedEntryID = "" s.Dirty = false + if warningSession, ok := s.Session.(WarningSession); ok { + s.StatusMessage = warningSession.ConsumeWarning() + } return nil } @@ -1033,6 +1046,9 @@ func (s *State) OpenRemoteVault(client webdav.Client, path string, key vault.Mas s.CurrentPath = nil s.SelectedEntryID = "" s.Dirty = false + if warningSession, ok := s.Session.(WarningSession); ok { + s.StatusMessage = warningSession.ConsumeWarning() + } return nil } diff --git a/internal/appui/main_test.go b/internal/appui/main_test.go index 59e5212..66ce854 100644 --- a/internal/appui/main_test.go +++ b/internal/appui/main_test.go @@ -2441,8 +2441,8 @@ func TestUIOpenRemoteActionBootstrapsFromLocalVaultBinding(t *testing.T) { if err != nil { t.Fatalf("Session.Current() error = %v", err) } - if got := current.EntriesInPath([]string{"Root", "Internet"}); len(got) != 1 || got[0].Title != "Vault Console" { - t.Fatalf("EntriesInPath(Root/Internet) = %#v, want Vault Console", got) + if got := current.EntriesInPath([]string{"keepass", "Internet"}); len(got) != 1 || got[0].Title != "Vault Console" { + t.Fatalf("EntriesInPath(keepass/Internet) = %#v, want Vault Console", got) } } @@ -2675,8 +2675,8 @@ func TestUIStartOpenRemoteActionBootstrapsFromLocalVaultBinding(t *testing.T) { if err != nil { t.Fatalf("Session.Current() error = %v", err) } - if got := current.EntriesInPath([]string{"Root", "Internet"}); len(got) != 1 || got[0].Title != "Vault Console" { - t.Fatalf("EntriesInPath(Root/Internet) = %#v, want Vault Console", got) + if got := current.EntriesInPath([]string{"keepass", "Internet"}); len(got) != 1 || got[0].Title != "Vault Console" { + t.Fatalf("EntriesInPath(keepass/Internet) = %#v, want Vault Console", got) } } @@ -3180,8 +3180,8 @@ func TestUIAdvancedSynchronizeFromLocalMergesIntoCurrentVault(t *testing.T) { if err != nil { t.Fatalf("reopened Current() error = %v", err) } - if got := len(model.EntriesInPath([]string{"Root", "Internet"})); got != 2 { - t.Fatalf("len(EntriesInPath(Root/Internet)) = %d, want 2", got) + if got := len(model.EntriesInPath([]string{"keepass", "Internet"})); got != 2 { + t.Fatalf("len(EntriesInPath(keepass/Internet)) = %d, want 2", got) } } @@ -3241,8 +3241,8 @@ func TestUIAdvancedSynchronizeFromImportedLocalVaultMergesIntoCurrentVault(t *te if err != nil { t.Fatalf("reopened Current() error = %v", err) } - if got := len(model.EntriesInPath([]string{"Root", "Internet"})); got != 2 { - t.Fatalf("len(EntriesInPath(Root/Internet)) = %d, want 2", got) + if got := len(model.EntriesInPath([]string{"keepass", "Internet"})); got != 2 { + t.Fatalf("len(EntriesInPath(keepass/Internet)) = %d, want 2", got) } } @@ -3406,8 +3406,8 @@ func TestUIAdvancedSynchronizeToRemoteWritesMergedVaultToTarget(t *testing.T) { if err != nil { t.Fatalf("reopened Current() error = %v", err) } - if got := len(model.EntriesInPath([]string{"Root", "Internet"})); got != 2 { - t.Fatalf("len(EntriesInPath(Root/Internet)) = %d, want 2", got) + if got := len(model.EntriesInPath([]string{"keepass", "Internet"})); got != 2 { + t.Fatalf("len(EntriesInPath(keepass/Internet)) = %d, want 2", got) } } @@ -5136,6 +5136,39 @@ func TestUIAutoEntersSingleVaultRootGroupAndDisplaysSlashRoot(t *testing.T) { } } +func TestUIOpenVaultShowsLegacyRootNormalizationWarning(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "legacy-root.kdbx") + var encoded bytes.Buffer + if err := vault.SaveKDBX(&encoded, vault.Model{ + Entries: []vault.Entry{ + {ID: "vault-console", Title: "Vault Console", Path: []string{"Root", "Crew", "Internet"}}, + }, + Groups: [][]string{ + {"Root"}, + {"Root", "Crew"}, + {"Root", "Crew", "Internet"}, + }, + }, "correct horse battery staple"); err != nil { + t.Fatalf("SaveKDBX() error = %v", err) + } + if err := os.WriteFile(path, encoded.Bytes(), 0o600); err != nil { + t.Fatalf("WriteFile(legacy-root.kdbx) error = %v", err) + } + + u := newUIWithSession("desktop", &session.Manager{}) + u.masterPassword.SetText("correct horse battery staple") + u.vaultPath.SetText(path) + if err := u.openVaultAction(); err != nil { + t.Fatalf("openVaultAction() error = %v", err) + } + + if got := u.state.StatusMessage; !strings.Contains(got, "legacy vault root") { + t.Fatalf("StatusMessage = %q, want legacy vault root normalization warning", got) + } +} + func TestUIAutoEntersSingleVaultRootWhenRecycleBinAlsoExists(t *testing.T) { t.Parallel() @@ -8480,7 +8513,7 @@ func TestUIConsumesPendingSharedVaultImportOnStartup(t *testing.T) { if err := reopened.openVaultAction(); err != nil { t.Fatalf("openVaultAction(imported) error = %v", err) } - reopened.state.NavigateToPath([]string{"Crew", "Internet"}) + reopened.state.NavigateToPath([]string{"Root", "Crew", "Internet"}) reopened.filter() if got := reopened.filteredTitles(); !slices.Equal(got, []string{"Bellagio"}) { t.Fatalf("filteredTitles() = %v, want [Bellagio]", got) diff --git a/internal/session/session.go b/internal/session/session.go index b443cc3..95fc9a5 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -12,6 +12,7 @@ import ( "strings" "git.julianfamily.org/keepassgo/internal/vault" + "git.julianfamily.org/keepassgo/internal/vaultview" "git.julianfamily.org/keepassgo/internal/webdav" ) @@ -31,6 +32,7 @@ type Manager struct { remoteClient *webdav.Client remotePath string remoteVersion webdav.Version + warning string } type PreparedLocalOpen struct { @@ -40,6 +42,7 @@ type PreparedLocalOpen struct { Key vault.MasterKey Encoded []byte VaultRoot string + Warning string } type PreparedRemoteOpen struct { @@ -51,6 +54,7 @@ type PreparedRemoteOpen struct { Encoded []byte VaultRoot string RemoteVersion webdav.Version + Warning string } type PreparedUnlock struct { @@ -58,6 +62,7 @@ type PreparedUnlock struct { Config *vault.KDBXConfig Key vault.MasterKey VaultRoot string + Warning string } func (m *Manager) SecuritySettings() vault.SecuritySettings { @@ -74,7 +79,7 @@ func (m *Manager) ConfigureSecurity(settings vault.SecuritySettings) error { } func (m *Manager) Create(model vault.Model, key vault.MasterKey) error { - root := detectSingleVaultRoot(model) + root := vaultview.KeepassRoot model = normalizeUnderRoot(model, root) var encoded bytes.Buffer if err := vault.SaveKDBXWithConfigAndKey(&encoded, model, key, m.config); err != nil { @@ -86,6 +91,7 @@ func (m *Manager) Create(model vault.Model, key vault.MasterKey) error { m.vaultRoot = root m.encoded = encoded.Bytes() m.locked = false + m.warning = "" return nil } @@ -118,6 +124,12 @@ func (m *Manager) Open(path string, key vault.MasterKey) error { return nil } +func (m *Manager) ConsumeWarning() string { + warning := strings.TrimSpace(m.warning) + m.warning = "" + return warning +} + func (m *Manager) Save() error { if m.remoteClient != nil && m.remotePath != "" { return m.SaveRemote() @@ -254,7 +266,7 @@ func (m *Manager) SaveAs(path string) error { func (m *Manager) Replace(model vault.Model) { root := m.vaultRoot if root == "" { - root = detectSingleVaultRoot(model) + root = vaultview.KeepassRoot } m.model = normalizeUnderRoot(model, root) m.vaultRoot = root @@ -305,12 +317,13 @@ func PrepareLocalOpen(path string, key vault.MasterKey) (PreparedLocalOpen, erro return PreparedLocalOpen{}, fmt.Errorf("open %s: %w", path, err) } return PreparedLocalOpen{ - Model: model, + Model: normalizeUnderRoot(model, vaultview.KeepassRoot), Config: config, Path: path, Key: key, Encoded: content, - VaultRoot: detectSingleVaultRoot(model), + VaultRoot: vaultview.KeepassRoot, + Warning: normalizationWarning(model), }, nil } @@ -324,14 +337,15 @@ func PrepareRemoteOpen(client webdav.Client, path string, key vault.MasterKey) ( return PreparedRemoteOpen{}, fmt.Errorf("decode remote %s: %w", path, err) } return PreparedRemoteOpen{ - Model: model, + Model: normalizeUnderRoot(model, vaultview.KeepassRoot), Config: config, Client: client, Path: path, Key: key, Encoded: content, - VaultRoot: detectSingleVaultRoot(model), + VaultRoot: vaultview.KeepassRoot, RemoteVersion: version, + Warning: normalizationWarning(model), }, nil } @@ -341,10 +355,11 @@ func PrepareUnlock(encoded []byte, key vault.MasterKey) (PreparedUnlock, error) return PreparedUnlock{}, fmt.Errorf("unlock vault: %w", err) } return PreparedUnlock{ - Model: model, + Model: normalizeUnderRoot(model, vaultview.KeepassRoot), Config: config, Key: key, - VaultRoot: detectSingleVaultRoot(model), + VaultRoot: vaultview.KeepassRoot, + Warning: normalizationWarning(model), }, nil } @@ -359,6 +374,7 @@ func (m *Manager) ApplyPreparedLocalOpen(prepared PreparedLocalOpen) { m.remoteClient = nil m.remotePath = "" m.remoteVersion = webdav.Version{} + m.warning = prepared.Warning } func (m *Manager) ApplyPreparedRemoteOpen(prepared PreparedRemoteOpen) { @@ -372,6 +388,7 @@ func (m *Manager) ApplyPreparedRemoteOpen(prepared PreparedRemoteOpen) { m.remotePath = prepared.Path m.remoteVersion = prepared.RemoteVersion m.path = "" + m.warning = prepared.Warning } func (m *Manager) ApplyPreparedUnlock(prepared PreparedUnlock) { @@ -380,6 +397,7 @@ func (m *Manager) ApplyPreparedUnlock(prepared PreparedUnlock) { m.key = prepared.Key m.vaultRoot = prepared.VaultRoot m.locked = false + m.warning = prepared.Warning } func (m *Manager) ChangeMasterKey(key vault.MasterKey) error { @@ -584,9 +602,7 @@ func (m *Manager) reloadCurrentLocal(merged vault.Model) error { return err } m.model = merged - if root := detectSingleVaultRoot(merged); root != "" { - m.vaultRoot = root - } + m.vaultRoot = vaultview.KeepassRoot m.encoded = encoded m.locked = false return nil @@ -603,9 +619,7 @@ func (m *Manager) reloadCurrentRemote(merged vault.Model) error { return fmt.Errorf("reopen remote %s after synchronize: %w", m.remotePath, err) } m.model = merged - if root := detectSingleVaultRoot(merged); root != "" { - m.vaultRoot = root - } + m.vaultRoot = vaultview.KeepassRoot m.encoded = encoded m.remoteVersion = version m.locked = false @@ -867,17 +881,6 @@ func mergePeerGroups(primary, secondary [][]string) [][]string { return out } -func detectSingleVaultRoot(model vault.Model) string { - if len(model.EntriesInPath(nil)) != 0 { - return "" - } - groups := model.ChildGroups(nil) - if len(groups) != 1 { - return "" - } - return groups[0] -} - func normalizeUnderRoot(model vault.Model, root string) vault.Model { if root == "" { return model @@ -888,8 +891,15 @@ func normalizeUnderRoot(model vault.Model, root string) vault.Model { switch { case len(path) == 0: return []string{root} + case path[0] == "Root": + if len(path) == 1 { + return []string{root} + } + return append([]string{root}, path[1:]...) case path[0] == root: return path + case path[0] == "Templates": + return path default: return append([]string{root}, path...) } @@ -907,12 +917,49 @@ func normalizeUnderRoot(model vault.Model, root string) vault.Model { out.RecycleBin[i].History[j].Path = normalizePath(out.RecycleBin[i].History[j].Path) } } + for i := range out.Templates { + out.Templates[i].Path = normalizePath(out.Templates[i].Path) + for j := range out.Templates[i].History { + out.Templates[i].History[j].Path = normalizePath(out.Templates[i].History[j].Path) + } + } for i := range out.Groups { out.Groups[i] = normalizePath(out.Groups[i]) } return out } +func normalizationWarning(model vault.Model) string { + if len(model.Entries) == 0 && len(model.Groups) == 0 && len(model.RecycleBin) == 0 { + return "" + } + if usesKeepassStorageRoot(model) { + return "" + } + return "Opened legacy vault root layout and normalized it under keepass." +} + +func usesKeepassStorageRoot(model vault.Model) bool { + if len(model.Entries) != 0 || len(model.RecycleBin) != 0 { + for _, entry := range model.Entries { + if len(entry.Path) > 0 && entry.Path[0] == vaultview.KeepassRoot { + return true + } + } + for _, entry := range model.RecycleBin { + if len(entry.Path) > 0 && entry.Path[0] == vaultview.KeepassRoot { + return true + } + } + } + for _, group := range model.Groups { + if len(group) > 0 && group[0] == vaultview.KeepassRoot { + return true + } + } + return false +} + func loadLocalSource(path string, key vault.MasterKey) (vault.Model, *vault.KDBXConfig, error) { content, err := os.ReadFile(path) if err != nil { diff --git a/internal/session/session_test.go b/internal/session/session_test.go index d2f38ab..7d2a23d 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -64,7 +64,7 @@ func TestCreateSaveAsLockAndUnlockRoundTripsVault(t *testing.T) { t.Fatalf("Current() after Unlock() error = %v", err) } - got := current.EntriesInPath([]string{"Root", "Internet"}) + got := current.EntriesInPath([]string{"keepass", "Internet"}) if len(got) != 1 || got[0].Title != "Vault Console" || got[0].Password != "token-1" { t.Fatalf("Current() entries = %#v, want persisted Vault Console entry", got) } @@ -110,12 +110,63 @@ func TestOpenLoadsExistingKDBXFromDisk(t *testing.T) { t.Fatalf("Current() error = %v", err) } - got := current.EntriesInPath([]string{"Root", "Home Assistant"}) + got := current.EntriesInPath([]string{"keepass", "Home Assistant"}) if len(got) != 1 || got[0].Password != "token-2" { t.Fatalf("Current() entries = %#v, want Home Assistant entry", got) } } +func TestOpenNormalizesLegacyVaultRootToKeepassAndReportsWarning(t *testing.T) { + t.Parallel() + + key := vault.MasterKey{Password: "correct horse battery staple"} + model := vault.Model{ + Entries: []vault.Entry{ + { + ID: "entry-1", + Title: "Surveillance Console", + Username: "codex", + Password: "token-2", + URL: "https://surveillance.crew.example.invalid", + Path: []string{"Root", "Home Assistant"}, + }, + }, + Groups: [][]string{ + {"Root"}, + {"Root", "Home Assistant"}, + }, + } + + path := filepath.Join(t.TempDir(), "legacy-root.kdbx") + file, err := os.Create(path) + if err != nil { + t.Fatalf("Create(legacy path) error = %v", err) + } + if err := vault.SaveKDBXWithKey(file, model, key); err != nil { + file.Close() + t.Fatalf("SaveKDBXWithKey() error = %v", err) + } + if err := file.Close(); err != nil { + t.Fatalf("Close(legacy path) error = %v", err) + } + + var sess Manager + if err := sess.Open(path, key); err != nil { + t.Fatalf("Open() error = %v", err) + } + + current, err := sess.Current() + if err != nil { + t.Fatalf("Current() error = %v", err) + } + if got := current.EntriesInPath([]string{"keepass", "Home Assistant"}); len(got) != 1 || got[0].ID != "entry-1" { + t.Fatalf("Current().EntriesInPath([keepass Home Assistant]) = %#v, want normalized legacy entry", got) + } + if got := sess.ConsumeWarning(); got == "" { + t.Fatal("ConsumeWarning() = empty, want legacy root normalization warning") + } +} + func TestSavePersistsEditsBackToCurrentPath(t *testing.T) { t.Parallel() @@ -169,7 +220,7 @@ func TestSavePersistsEditsBackToCurrentPath(t *testing.T) { t.Fatalf("LoadKDBXWithKey() error = %v", err) } - got := loaded.EntriesInPath([]string{"Root", "Internet"}) + got := loaded.EntriesInPath([]string{"keepass", "Internet"}) if len(got) != 1 || got[0].Password != "token-2" { t.Fatalf("loaded entries = %#v, want updated password token-2", got) } @@ -307,7 +358,7 @@ func TestOpenRemoteLoadsExistingKDBXFromWebDAV(t *testing.T) { t.Fatalf("Current() error = %v", err) } - got := current.EntriesInPath([]string{"Root", "Internet"}) + got := current.EntriesInPath([]string{"keepass", "Internet"}) if len(got) != 1 || got[0].Password != "token-1" { t.Fatalf("Current() entries = %#v, want Vault Console entry from remote vault", got) } @@ -392,7 +443,7 @@ func TestSaveRemotePersistsEditsBackToWebDAV(t *testing.T) { t.Fatalf("LoadKDBXWithKey(savedBytes) error = %v", err) } - got := loaded.EntriesInPath([]string{"Root", "Home Assistant"}) + got := loaded.EntriesInPath([]string{"keepass", "Home Assistant"}) if len(got) != 1 || got[0].Password != "token-2" { t.Fatalf("loaded remote entries = %#v, want updated token-2 entry", got) } @@ -513,7 +564,7 @@ func TestChangeMasterKeyReencryptsSavedAndLockedVault(t *testing.T) { if err != nil { t.Fatalf("Current() error = %v", err) } - got := current.EntriesInPath([]string{"Root", "Internet"}) + got := current.EntriesInPath([]string{"keepass", "Internet"}) if len(got) != 1 || got[0].Title != "Vault Console" { t.Fatalf("Current() entries = %#v, want Vault Console entry after ChangeMasterKey", got) } @@ -720,7 +771,7 @@ func TestRemoteSaveAndReopenPreservesCrossFeatureState(t *testing.T) { t.Fatalf("Current() after reopen error = %v", err) } - got := current.EntriesInPath([]string{"Root", "Internet"}) + got := current.EntriesInPath([]string{"keepass", "Internet"}) if len(got) != 1 { t.Fatalf("len(EntriesInPath(Root/Internet)) after reopen = %d, want 1", len(got)) } @@ -879,7 +930,7 @@ func TestSynchronizeRemotePreservesOverwrittenRemoteVariantInHistory(t *testing. t.Fatalf("reopened Current() error = %v", err) } - got := current.EntriesInPath([]string{"Root", "Internet"}) + got := current.EntriesInPath([]string{"keepass", "Internet"}) if len(got) != 1 { t.Fatalf("len(EntriesInPath(Root/Internet)) = %d, want 1", len(got)) } @@ -947,7 +998,7 @@ func TestSynchronizeFromLocalMergesOtherVaultIntoCurrentSource(t *testing.T) { t.Fatalf("reopened Current() error = %v", err) } - got := current.EntriesInPath([]string{"Root", "Internet"}) + got := current.EntriesInPath([]string{"keepass", "Internet"}) if len(got) != 2 { t.Fatalf("len(EntriesInPath(Root/Internet)) = %d, want 2", len(got)) } @@ -1004,7 +1055,7 @@ func TestSynchronizeFromLocalBytesMergesOtherVaultIntoCurrentSource(t *testing.T t.Fatalf("reopened Current() error = %v", err) } - got := current.EntriesInPath([]string{"Root", "Internet"}) + got := current.EntriesInPath([]string{"keepass", "Internet"}) if len(got) != 2 { t.Fatalf("len(EntriesInPath(Root/Internet)) = %d, want 2", len(got)) } @@ -1063,7 +1114,7 @@ func TestSynchronizeToLocalWritesMergedVaultToTarget(t *testing.T) { t.Fatalf("reopened Current() error = %v", err) } - got := current.EntriesInPath([]string{"Root", "Internet"}) + got := current.EntriesInPath([]string{"keepass", "Internet"}) if len(got) != 2 { t.Fatalf("len(EntriesInPath(Root/Internet)) = %d, want 2", len(got)) } @@ -1148,7 +1199,7 @@ func TestSynchronizeToRemoteWritesMergedVaultToTarget(t *testing.T) { t.Fatalf("reopened Current() error = %v", err) } - got := current.EntriesInPath([]string{"Root", "Internet"}) + got := current.EntriesInPath([]string{"keepass", "Internet"}) if len(got) != 2 { t.Fatalf("len(EntriesInPath(Root/Internet)) = %d, want 2", len(got)) } diff --git a/internal/vault/kdbx.go b/internal/vault/kdbx.go index b4e5850..962f258 100644 --- a/internal/vault/kdbx.go +++ b/internal/vault/kdbx.go @@ -26,6 +26,7 @@ var ErrInvalidMasterKey = errors.New("invalid master key") const ( templatesRoot = "Templates" recycleBinRoot = "Recycle Bin" + keepassRoot = "keepass" keepassGOIDField = "KeePassGO-ID" remoteProfilesKey = "keepassgo.remoteProfiles" ) @@ -502,6 +503,10 @@ func compareGroupNames(a, b string) int { return -1 case b == "Root": return 1 + case a == keepassRoot: + return -1 + case b == keepassRoot: + return 1 case a == templatesRoot: return -1 case b == templatesRoot: diff --git a/internal/vault/kdbx_test.go b/internal/vault/kdbx_test.go index 5a41a89..0077df2 100644 --- a/internal/vault/kdbx_test.go +++ b/internal/vault/kdbx_test.go @@ -755,6 +755,57 @@ func TestKDBXReopenCyclesPreserveStableIDsAndCrossFeatureState(t *testing.T) { } } +func TestKDBXKeepassRootEntriesPreserveAttachmentsWithTemplates(t *testing.T) { + t.Parallel() + + model := Model{ + Entries: []Entry{ + { + ID: "entry-1", + Title: "Vault Console", + Username: "dannyocean", + Password: "bellagio-pass-2", + URL: "https://vault.crew.example.invalid", + Path: []string{"keepass", "Internet"}, + Attachments: map[string][]byte{ + "token.txt": []byte("secret attachment contents"), + }, + }, + }, + Templates: []Entry{ + { + ID: "tpl-1", + Title: "Website Login", + Username: "template-user", + Password: "template-password", + Path: []string{"Templates", "Web"}, + }, + }, + Groups: [][]string{ + {"keepass", "Internet"}, + {"Templates", "Web"}, + }, + } + + var encoded bytes.Buffer + if err := SaveKDBX(&encoded, model, "correct horse battery staple"); err != nil { + t.Fatalf("SaveKDBX() error = %v", err) + } + + loaded, err := LoadKDBX(bytes.NewReader(encoded.Bytes()), "correct horse battery staple") + if err != nil { + t.Fatalf("LoadKDBX() error = %v", err) + } + + got := loaded.EntriesInPath([]string{"keepass", "Internet"}) + if len(got) != 1 { + t.Fatalf("len(EntriesInPath()) = %d, want 1", len(got)) + } + if string(got[0].Attachments["token.txt"]) != "secret attachment contents" { + t.Fatalf("attachment contents = %q, want %q", string(got[0].Attachments["token.txt"]), "secret attachment contents") + } +} + func mustGroup(name string, children ...any) gokeepasslib.Group { group := gokeepasslib.NewGroup() group.Name = name -- 2.52.0 From a88b8a824b89d90b630d9eb1603e2c8adfca6b3e Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Mon, 13 Apr 2026 08:50:33 -0700 Subject: [PATCH 9/9] Add explicit templates vault view --- internal/appstate/state.go | 107 ++++++++++++++++--- internal/vaultview/view.go | 179 +++++++++++++++++++++++++++----- internal/vaultview/view_test.go | 38 +++++++ 3 files changed, 284 insertions(+), 40 deletions(-) diff --git a/internal/appstate/state.go b/internal/appstate/state.go index 9c886ad..9f31153 100644 --- a/internal/appstate/state.go +++ b/internal/appstate/state.go @@ -32,6 +32,7 @@ const ( ) const entriesRootLabel = "Root" +const templatesRootLabel = "Templates" type CurrentSession interface { Current() (vault.Model, error) @@ -402,8 +403,8 @@ func (s *State) ChildGroups() ([]string, error) { } if s.Section != SectionEntries { - if s.Section == SectionTemplates && len(s.CurrentPath) == 0 { - return childGroups(s.entriesForSection(model), []string{"Templates"}), nil + if s.Section == SectionTemplates { + return vaultview.VaultTemplates(model).ChildGroups(templatesViewPath(s.CurrentPath)), nil } return childGroups(s.entriesForSection(model), s.CurrentPath), nil } @@ -452,7 +453,7 @@ func (s *State) currentModel() (vault.Model, error) { func (s *State) entriesForSection(model vault.Model) []vault.Entry { switch s.Section { case SectionTemplates: - return slices.Clone(model.Templates) + return logicalTemplateEntries(vaultview.VaultTemplates(model).EntriesUnderPath(nil)) case SectionRecycleBin: return logicalEntries(vaultview.VaultRecycleBin(model).EntriesUnderPath(nil)) case SectionAPITokens, SectionAPIAudit, SectionAbout: @@ -466,9 +467,7 @@ func (s State) SearchPathContext(entry vault.Entry) string { path := slices.Clone(entry.Path) switch s.Section { case SectionTemplates: - if len(path) == 0 || path[0] != "Templates" { - path = append([]string{"Templates"}, path...) - } + path = logicalTemplatePath(path) case SectionRecycleBin: path = append([]string{"Recycle Bin"}, logicalEntriesPath(path)...) case SectionEntries: @@ -555,6 +554,26 @@ func logicalEntriesPath(path []string) []string { return append([]string{entriesRootLabel}, append([]string(nil), path...)...) } +func logicalTemplatePath(path []string) []string { + if len(path) == 0 { + return []string{templatesRootLabel} + } + if path[0] == templatesRootLabel { + return append([]string(nil), path...) + } + return append([]string{templatesRootLabel}, append([]string(nil), path...)...) +} + +func templatesViewPath(path []string) []string { + if len(path) == 0 { + return nil + } + if path[0] == templatesRootLabel { + return append([]string(nil), path[1:]...) + } + return append([]string(nil), path...) +} + func entriesViewPathForModel(model vault.Model, path []string) []string { if len(path) == 0 { return nil @@ -590,6 +609,25 @@ func logicalEntries(entries []vault.Entry) []vault.Entry { return out } +func logicalTemplateEntry(entry vault.Entry) vault.Entry { + entry.Path = logicalTemplatePath(entry.Path) + for i := range entry.History { + entry.History[i] = logicalTemplateEntry(entry.History[i]) + } + return entry +} + +func logicalTemplateEntries(entries []vault.Entry) []vault.Entry { + if len(entries) == 0 { + return nil + } + out := make([]vault.Entry, len(entries)) + for i := range entries { + out[i] = logicalTemplateEntry(entries[i]) + } + return out +} + func entryForModel(model vault.Model, entry vault.Entry) vault.Entry { entry.Path = entriesViewPathForModel(model, entry.Path) for i := range entry.History { @@ -598,6 +636,14 @@ func entryForModel(model vault.Model, entry vault.Entry) vault.Entry { return entry } +func templateEntryForModel(entry vault.Entry) vault.Entry { + entry.Path = templatesViewPath(entry.Path) + for i := range entry.History { + entry.History[i] = templateEntryForModel(entry.History[i]) + } + return entry +} + func usesPhysicalEntriesRoot(model vault.Model) bool { if len(model.Entries) == 0 && len(model.Groups) == 0 && len(model.RecycleBin) == 0 { return true @@ -663,6 +709,33 @@ func childGroups(entries []vault.Entry, path []string) []string { return groups } +func sectionGroupView(model vault.Model, section Section) vaultview.View { + switch section { + case SectionTemplates: + return vaultview.VaultTemplates(model) + default: + return vaultview.VaultRoot(model) + } +} + +func sectionGroupViewPath(model vault.Model, section Section, path []string) []string { + switch section { + case SectionTemplates: + return templatesViewPath(path) + default: + return entriesViewPathForModel(model, path) + } +} + +func sectionGroupLogicalPath(model vault.Model, section Section, path []string) []string { + switch section { + case SectionTemplates: + return logicalTemplatePath(path) + default: + return logicalEntriesPathForModel(model, path) + } +} + func (s *State) DeleteSelectedEntry() error { session, ok := s.Session.(MutableSession) if !ok { @@ -730,7 +803,7 @@ func (s *State) UpsertTemplate(entry vault.Entry) error { return err } - model.UpsertTemplate(entry) + model.UpsertTemplate(vaultview.VaultTemplates(model).ToPhysicalEntry(templateEntryForModel(entry))) session.Replace(model) s.SelectedEntryID = entry.ID return s.markDirtyAndAutoSave() @@ -1124,7 +1197,8 @@ func (s *State) CreateGroup(name string) error { return err } - model.CreateGroup(vaultview.VaultRoot(model).ToPhysicalPath(entriesViewPathForModel(model, s.CurrentPath)), name) + view := sectionGroupView(model, s.Section) + model.CreateGroup(view.ToPhysicalPath(sectionGroupViewPath(model, s.Section, s.CurrentPath)), name) session.Replace(model) return s.markDirtyAndAutoSave() } @@ -1138,15 +1212,16 @@ func (s *State) MoveCurrentGroup(parent []string) error { if err != nil { return err } - current := logicalEntriesPathForModel(model, s.CurrentPath) - currentViewPath := entriesViewPathForModel(model, current) - parentViewPath := entriesViewPathForModel(model, parent) - if err := model.MoveGroup(vaultview.VaultRoot(model).ToPhysicalPath(currentViewPath), vaultview.VaultRoot(model).ToPhysicalPath(parentViewPath)); err != nil { + view := sectionGroupView(model, s.Section) + current := sectionGroupLogicalPath(model, s.Section, s.CurrentPath) + currentViewPath := sectionGroupViewPath(model, s.Section, current) + parentViewPath := sectionGroupViewPath(model, s.Section, parent) + if err := model.MoveGroup(view.ToPhysicalPath(currentViewPath), view.ToPhysicalPath(parentViewPath)); err != nil { return err } session.Replace(model) if len(currentViewPath) > 0 { - s.CurrentPath = logicalEntriesPathForModel(model, append(append([]string(nil), parentViewPath...), currentViewPath[len(currentViewPath)-1])) + s.CurrentPath = sectionGroupLogicalPath(model, s.Section, append(append([]string(nil), parentViewPath...), currentViewPath[len(currentViewPath)-1])) } return s.markDirtyAndAutoSave() } @@ -1162,7 +1237,8 @@ func (s *State) RenameCurrentGroup(newName string) error { return err } - if err := model.RenameGroup(vaultview.VaultRoot(model).ToPhysicalPath(entriesViewPathForModel(model, s.CurrentPath)), newName); err != nil { + view := sectionGroupView(model, s.Section) + if err := model.RenameGroup(view.ToPhysicalPath(sectionGroupViewPath(model, s.Section, s.CurrentPath)), newName); err != nil { return err } @@ -1203,7 +1279,8 @@ func (s *State) DeleteCurrentGroup() error { return err } - if err := model.DeleteGroup(vaultview.VaultRoot(model).ToPhysicalPath(entriesViewPathForModel(model, s.CurrentPath))); err != nil { + view := sectionGroupView(model, s.Section) + if err := model.DeleteGroup(view.ToPhysicalPath(sectionGroupViewPath(model, s.Section, s.CurrentPath))); err != nil { return err } diff --git a/internal/vaultview/view.go b/internal/vaultview/view.go index b9500d1..260ff1e 100644 --- a/internal/vaultview/view.go +++ b/internal/vaultview/view.go @@ -7,6 +7,7 @@ import ( ) const KeepassRoot = "keepass" +const TemplatesRoot = "Templates" // View projects the physical vault model into a logical tree for a specific // product surface. @@ -28,7 +29,13 @@ func Vault(model vault.Model) View { // VaultRoot returns the logical main-vault view rooted at the physical // keepass storage group. func VaultRoot(model vault.Model) View { - return rootView{model: model, rooted: usesKeepassRoot(model)} + return prefixedView{model: model, root: KeepassRoot, rooted: usesTopLevelRoot(model, KeepassRoot)} +} + +// VaultTemplates returns the logical templates view rooted at the physical +// Templates storage group. +func VaultTemplates(model vault.Model) View { + return templatesView{model: model} } // VaultRecycleBin returns the logical recycle-bin view. @@ -68,47 +75,48 @@ func (v physicalView) FromPhysicalEntry(entry vault.Entry) vault.Entry { return cloneEntry(entry) } -type rootView struct { +type prefixedView struct { model vault.Model + root string rooted bool } -func (v rootView) ChildGroups(path []string) []string { +func (v prefixedView) ChildGroups(path []string) []string { return v.model.ChildGroups(v.ToPhysicalPath(path)) } -func (v rootView) EntriesInPath(path []string) []vault.Entry { +func (v prefixedView) EntriesInPath(path []string) []vault.Entry { return v.mapEntries(v.model.EntriesInPath(v.ToPhysicalPath(path))) } -func (v rootView) EntriesUnderPath(path []string) []vault.Entry { +func (v prefixedView) EntriesUnderPath(path []string) []vault.Entry { return v.mapEntries(v.model.EntriesUnderPath(v.ToPhysicalPath(path))) } -func (v rootView) ToPhysicalPath(path []string) []string { +func (v prefixedView) ToPhysicalPath(path []string) []string { if !v.rooted { return clonePath(path) } if len(path) == 0 { - return []string{KeepassRoot} + return []string{v.root} } - return append([]string{KeepassRoot}, clonePath(path)...) + return append([]string{v.root}, clonePath(path)...) } -func (v rootView) FromPhysicalPath(path []string) []string { +func (v prefixedView) FromPhysicalPath(path []string) []string { if !v.rooted { return clonePath(path) } if len(path) == 0 { return nil } - if path[0] != KeepassRoot { + if path[0] != v.root { return clonePath(path) } return clonePath(path[1:]) } -func (v rootView) ToPhysicalEntry(entry vault.Entry) vault.Entry { +func (v prefixedView) ToPhysicalEntry(entry vault.Entry) vault.Entry { entry = cloneEntry(entry) entry.Path = v.ToPhysicalPath(entry.Path) for i := range entry.History { @@ -117,7 +125,7 @@ func (v rootView) ToPhysicalEntry(entry vault.Entry) vault.Entry { return entry } -func (v rootView) FromPhysicalEntry(entry vault.Entry) vault.Entry { +func (v prefixedView) FromPhysicalEntry(entry vault.Entry) vault.Entry { entry = cloneEntry(entry) entry.Path = v.FromPhysicalPath(entry.Path) for i := range entry.History { @@ -126,7 +134,7 @@ func (v rootView) FromPhysicalEntry(entry vault.Entry) vault.Entry { return entry } -func (v rootView) mapEntries(entries []vault.Entry) []vault.Entry { +func (v prefixedView) mapEntries(entries []vault.Entry) []vault.Entry { out := make([]vault.Entry, 0, len(entries)) for _, entry := range entries { out = append(out, v.FromPhysicalEntry(entry)) @@ -138,6 +146,89 @@ type recycleBinView struct { model vault.Model } +type templatesView struct { + model vault.Model +} + +func (v templatesView) ChildGroups(path []string) []string { + return groupChildren(templateGroupPaths(v.model), v.EntriesUnderPath(nil), path) +} + +func (v templatesView) EntriesInPath(path []string) []vault.Entry { + return entriesInPath(v.EntriesUnderPath(nil), path) +} + +func (v templatesView) EntriesUnderPath(path []string) []vault.Entry { + var out []vault.Entry + for _, entry := range v.model.Templates { + if len(path) > len(entry.Path) { + continue + } + physical := entry.Path + if len(physical) > 0 && physical[0] == TemplatesRoot { + physical = physical[1:] + } + if len(path) > len(physical) { + continue + } + if !slices.Equal(physical[:len(path)], path) { + continue + } + item := cloneEntry(entry) + item.Path = clonePath(physical) + for i := range item.History { + item.History[i].Path = v.FromPhysicalPath(item.History[i].Path) + } + out = append(out, item) + } + slices.SortFunc(out, func(a, b vault.Entry) int { + switch { + case a.Title < b.Title: + return -1 + case a.Title > b.Title: + return 1 + default: + return 0 + } + }) + return out +} + +func (v templatesView) ToPhysicalPath(path []string) []string { + if len(path) == 0 { + return []string{TemplatesRoot} + } + return append([]string{TemplatesRoot}, clonePath(path)...) +} + +func (v templatesView) FromPhysicalPath(path []string) []string { + if len(path) == 0 { + return nil + } + if path[0] != TemplatesRoot { + return clonePath(path) + } + return clonePath(path[1:]) +} + +func (v templatesView) ToPhysicalEntry(entry vault.Entry) vault.Entry { + entry = cloneEntry(entry) + entry.Path = v.ToPhysicalPath(entry.Path) + for i := range entry.History { + entry.History[i].Path = v.ToPhysicalPath(entry.History[i].Path) + } + return entry +} + +func (v templatesView) FromPhysicalEntry(entry vault.Entry) vault.Entry { + entry = cloneEntry(entry) + entry.Path = v.FromPhysicalPath(entry.Path) + for i := range entry.History { + entry.History[i].Path = v.FromPhysicalPath(entry.History[i].Path) + } + return entry +} + func (v recycleBinView) ChildGroups(path []string) []string { return childGroups(v.model.RecycleBin, path) } @@ -187,6 +278,10 @@ func (v recycleBinView) FromPhysicalEntry(entry vault.Entry) vault.Entry { } func childGroups(entries []vault.Entry, path []string) []string { + return groupChildren(nil, entries, path) +} + +func groupChildren(groupPaths [][]string, entries []vault.Entry, path []string) []string { seen := map[string]bool{} var groups []string for _, entry := range entries { @@ -206,6 +301,23 @@ func childGroups(entries []vault.Entry, path []string) []string { seen[group] = true groups = append(groups, group) } + for _, groupPath := range groupPaths { + if len(path) > len(groupPath) { + continue + } + if !slices.Equal(groupPath[:len(path)], path) { + continue + } + if len(groupPath) == len(path) { + continue + } + group := groupPath[len(path)] + if seen[group] { + continue + } + seen[group] = true + groups = append(groups, group) + } slices.Sort(groups) return groups } @@ -275,22 +387,39 @@ func clonePath(path []string) []string { return slices.Clone(path) } -func usesKeepassRoot(model vault.Model) bool { - if len(model.Entries) == 0 && len(model.Groups) == 0 && len(model.RecycleBin) == 0 { - return true - } +func templateGroupPaths(model vault.Model) [][]string { + var out [][]string for _, group := range model.Groups { - if len(group) > 0 && group[0] == KeepassRoot { - return true + if len(group) == 0 || group[0] != TemplatesRoot { + continue } + out = append(out, clonePath(group[1:])) } - for _, entry := range model.Entries { - if len(entry.Path) > 0 && entry.Path[0] == KeepassRoot { - return true - } + return out +} + +func usesTopLevelRoot(model vault.Model, root string) bool { + if len(model.Entries) == 0 && len(model.Groups) == 0 && len(model.RecycleBin) == 0 { + return root == KeepassRoot } - for _, entry := range model.RecycleBin { - if len(entry.Path) > 0 && entry.Path[0] == KeepassRoot { + return groupsUseRoot(model.Groups, root) || + entriesUseRoot(model.Entries, root) || + entriesUseRoot(model.Templates, root) || + entriesUseRoot(model.RecycleBin, root) +} + +func groupsUseRoot(groups [][]string, root string) bool { + for _, group := range groups { + if len(group) > 0 && group[0] == root { + return true + } + } + return false +} + +func entriesUseRoot(entries []vault.Entry, root string) bool { + for _, entry := range entries { + if len(entry.Path) > 0 && entry.Path[0] == root { return true } } diff --git a/internal/vaultview/view_test.go b/internal/vaultview/view_test.go index ef0904f..66a6b14 100644 --- a/internal/vaultview/view_test.go +++ b/internal/vaultview/view_test.go @@ -78,6 +78,44 @@ func TestVaultRecycleBinProjectsRecycleTree(t *testing.T) { } } +func TestVaultTemplatesProjectsTemplatesStorageRoot(t *testing.T) { + t.Parallel() + + model := vault.Model{ + Templates: []vault.Entry{ + {ID: "website-login", Title: "Website Login", Path: []string{"Templates", "Web"}}, + {ID: "ssh-login", Title: "SSH Login", Path: []string{"Templates", "Infra"}}, + }, + Groups: [][]string{ + {"Templates"}, + {"Templates", "Infra"}, + {"Templates", "Web"}, + {"keepass"}, + }, + } + + view := VaultTemplates(model) + + if got := view.ChildGroups(nil); !slices.Equal(got, []string{"Infra", "Web"}) { + t.Fatalf("VaultTemplates(model).ChildGroups(nil) = %v, want [Infra Web]", got) + } + + gotEntries := view.EntriesInPath([]string{"Web"}) + if len(gotEntries) != 1 || !slices.Equal(gotEntries[0].Path, []string{"Web"}) { + t.Fatalf("VaultTemplates(model).EntriesInPath([Web]) = %#v, want logical path [Web]", gotEntries) + } + + if got := view.ToPhysicalPath(nil); !slices.Equal(got, []string{"Templates"}) { + t.Fatalf("VaultTemplates(model).ToPhysicalPath(nil) = %v, want [Templates]", got) + } + if got := view.ToPhysicalPath([]string{"Web"}); !slices.Equal(got, []string{"Templates", "Web"}) { + t.Fatalf("VaultTemplates(model).ToPhysicalPath([Web]) = %v, want [Templates Web]", got) + } + if got := view.FromPhysicalPath([]string{"Templates", "Web"}); !slices.Equal(got, []string{"Web"}) { + t.Fatalf("VaultTemplates(model).FromPhysicalPath([Templates Web]) = %v, want [Web]", got) + } +} + func TestVaultReturnsPhysicalPathsUnchanged(t *testing.T) { t.Parallel() -- 2.52.0