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{