Hide physical keepass paths in token and approval UX
This commit is contained in:
@@ -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
|
||||
|
||||
+25
-6
@@ -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:
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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), " / ")
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user