refactor(handler): migrate authorizeAction + plan-review preflight to single bypass
authorizeAction (file API) and executePlanReview both used to make their own IsAdmin / IsSubtreeAdmin / CanEditZddc calls before falling through to the decider. After this commit every admin/elevation branch is in policy.InternalDecider.Allow — the handlers just call AllowActionFromChainP with the principal and let the decider decide. fileapi.go authorizeAction: - ~60 lines → ~20 lines. - Three early-outs (IsAdmin / IsSubtreeAdmin / CanEditZddc) removed. - .zddc strict-ancestor rule preserved: AllowActionFromChainP detects action == ActionAdmin (serveFilePut tags .zddc writes that way) and applies excludeLeaf=true to IsAdminForChain. planreview.go executePlanReview: - Two preflight checks now flow through AllowActionFromChainP. - The "is admin OR is subtree admin? else fall through to decider" braid collapses to one decider call per target. - Behavior preserved: subtree-admin authority required for the reviewing/staging workflow roots (strict-ancestor via ActionAdmin), WORM-cr authority required for received/<tracking>/ creation. Plan Review and Accept Transmittal tests still pass, lock-in invariants still hold (un-elevated admin denied, elevated admin bypasses, subtree scope, strict-ancestor, etc.). Next: remove the now-dead IsAdmin / IsSubtreeAdmin / CanEditZddc helpers (still referenced by profilehandler and authcheck), or keep them — they're not on a hot path and the migration there is its own commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
465d2f605c
commit
6c818648ca
2 changed files with 26 additions and 43 deletions
|
|
@ -118,16 +118,12 @@ func resolveTargetPath(cfg config.Config, urlPath string) (absPath, cleanURL str
|
|||
// that create a brand-new file inherit the parent directory's chain).
|
||||
// Returns allowed=false with the response status already written on deny.
|
||||
//
|
||||
// Admin escape hatches: root admins (IsAdmin) and subtree admins
|
||||
// (IsSubtreeAdmin) get unconditional access — the cascade evaluator
|
||||
// and the WORM mask do not see their requests at all. This is the
|
||||
// only way to mutate filed documents in Issued/Received.
|
||||
//
|
||||
// .zddc writes use the stricter CanEditZddc rule (strict-ancestor
|
||||
// admin authority) regardless of the action verb, since the file
|
||||
// being written is itself the source of the authority decision and
|
||||
// the strict-ancestor rule is the existing defense against
|
||||
// self-elevation.
|
||||
// All admin / WORM / ACL logic lives downstream in the decider's single
|
||||
// bypass site (policy.InternalDecider.Allow). AllowActionFromChainP
|
||||
// computes IsActiveAdmin from the chain and Principal.Elevated, with
|
||||
// the strict-ancestor rule applied when action == ActionAdmin (the
|
||||
// caller tags .zddc writes that way). The handler does NOT make
|
||||
// admin/elevation decisions of its own — one bypass site, one helper.
|
||||
func authorizeAction(cfg config.Config, w http.ResponseWriter, r *http.Request, absPath, urlPath, action string) bool {
|
||||
probe := filepath.Dir(absPath)
|
||||
for {
|
||||
|
|
@ -142,40 +138,14 @@ func authorizeAction(cfg config.Config, w http.ResponseWriter, r *http.Request,
|
|||
probe = filepath.Dir(probe)
|
||||
}
|
||||
|
||||
email := EmailFromContext(r)
|
||||
p := PrincipalFromContext(r)
|
||||
|
||||
// Admin bypass — root and subtree. Un-elevated admins fall through
|
||||
// to the regular decider; the Principal's Elevated flag short-
|
||||
// circuits both IsAdmin and IsSubtreeAdmin so we don't accidentally
|
||||
// hand out WORM bypass to a user who hasn't asked for it.
|
||||
if zddc.IsAdmin(cfg.Root, p) {
|
||||
return true
|
||||
}
|
||||
if zddc.IsSubtreeAdmin(cfg.Root, probe, p) {
|
||||
return true
|
||||
}
|
||||
|
||||
// .zddc writes: CanEditZddc enforces the strict-ancestor rule that
|
||||
// prevents a subtree admin from elevating themselves by editing the
|
||||
// .zddc that grants their authority. Non-admins (or un-elevated
|
||||
// admins) fall through to the regular decider — they will be denied
|
||||
// unless an explicit `a` verb is granted to a non-admin role at
|
||||
// this path, which is unusual.
|
||||
if filepath.Base(absPath) == ".zddc" {
|
||||
zddcDir := filepath.Dir(absPath)
|
||||
if zddc.CanEditZddc(cfg.Root, zddcDir, p) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
chain, err := zddc.EffectivePolicy(cfg.Root, probe)
|
||||
if err != nil {
|
||||
slog.Warn("file API ACL chain error", "path", absPath, "err", err)
|
||||
}
|
||||
|
||||
decider := DeciderFromContext(r)
|
||||
allowed, _ := policy.AllowActionFromChain(r.Context(), decider, chain, email, urlPath, action)
|
||||
allowed, _ := policy.AllowActionFromChainP(r.Context(), decider, chain, p, urlPath, action)
|
||||
if !allowed {
|
||||
http.Error(w, "Forbidden", http.StatusForbidden)
|
||||
return false
|
||||
|
|
|
|||
|
|
@ -157,21 +157,34 @@ func executePlanReview(cfg config.Config, r *http.Request, project, party, track
|
|||
if email == "" {
|
||||
return nil, http.StatusForbidden, "Forbidden — no authenticated principal"
|
||||
}
|
||||
// All three pre-flight checks go through the consolidated decider.
|
||||
// AllowActionFromChainP applies the strict-ancestor rule for
|
||||
// .zddc-targeted actions (ActionAdmin) and the single admin-bypass
|
||||
// branch for elevated admins. No manual IsAdmin / IsSubtreeAdmin /
|
||||
// CanEditZddc branching here.
|
||||
decider := DeciderFromContext(r)
|
||||
for _, root := range []string{reviewingRoot, stagingRoot} {
|
||||
if !zddc.CanEditZddc(cfg.Root, root, p) {
|
||||
chain, perr := zddc.EffectivePolicy(cfg.Root, root)
|
||||
if perr != nil {
|
||||
return nil, http.StatusInternalServerError, "Internal Server Error — cascade lookup: " + perr.Error()
|
||||
}
|
||||
rel, _ := filepath.Rel(cfg.Root, root)
|
||||
rootURL := "/" + filepath.ToSlash(rel) + "/.zddc"
|
||||
allowed, _ := policy.AllowActionFromChainP(r.Context(), decider, chain, p, rootURL, policy.ActionAdmin)
|
||||
if !allowed {
|
||||
return nil, http.StatusForbidden, fmt.Sprintf("Forbidden — %s lacks subtree-admin authority for %s",
|
||||
email, strings.TrimPrefix(root, cfg.Root+string(filepath.Separator)))
|
||||
}
|
||||
}
|
||||
// (b) — verify `c` authority on received/<tracking>/. Admins bypass
|
||||
// the policy and would pass anyway; non-admin doc_controllers come
|
||||
// through the WORM-list grant.
|
||||
if !zddc.IsAdmin(cfg.Root, p) && !zddc.IsSubtreeAdmin(cfg.Root, receivedAbs, p) {
|
||||
// Verify `c` (create) authority on received/<tracking>/. Elevated
|
||||
// admins short-circuit inside the decider; non-admin doc_controllers
|
||||
// come through the WORM-list grant. One code path either way.
|
||||
{
|
||||
chain, perr := zddc.EffectivePolicy(cfg.Root, receivedAbs)
|
||||
if perr != nil {
|
||||
return nil, http.StatusInternalServerError, "Internal Server Error — cascade lookup: " + perr.Error()
|
||||
}
|
||||
allowed, _ := policy.AllowActionFromChain(r.Context(), DeciderFromContext(r), chain, email, cleanURL, policy.ActionCreate)
|
||||
allowed, _ := policy.AllowActionFromChainP(r.Context(), decider, chain, p, cleanURL, policy.ActionCreate)
|
||||
if !allowed {
|
||||
return nil, http.StatusForbidden, fmt.Sprintf("Forbidden — %s lacks create authority on %s (filing this submittal requires the doc_controller WORM grant)",
|
||||
email, strings.TrimPrefix(receivedAbs, cfg.Root+string(filepath.Separator)))
|
||||
|
|
|
|||
Loading…
Reference in a new issue