BCH-1315: AuthZ Phase 3 — AuthZen HTTP driver + batch Evaluations + /me/permissions#428
BCH-1315: AuthZ Phase 3 — AuthZen HTTP driver + batch Evaluations + /me/permissions#428ccf-lisa[bot] wants to merge 3 commits into
Conversation
gusfcarvalho
left a comment
There was a problem hiding this comment.
Findings inline. Headline: one blocker — CI lint is red from errcheck on unchecked resp.Body.Close() in authzen.go (lines 179 & 207), a one-line fix each; plus a body-drain perf note in the same function and two Low items (builtin repeats admin DB lookups under /me/permissions; deriveEvaluationsURL silently mis-derives on trailing-slash/query endpoints). Solid design and strong tests otherwise — build/vet/gofmt and -race tests all green locally.
gusfcarvalho
left a comment
There was a problem hiding this comment.
All review feedback addressed in 555f147 and verified:
- errcheck blocker fixed (unchecked
resp.Body.Close()) — CIlintnow green. - Response body now drained before close, restoring keep-alive reuse on the PEP hot path.
deriveEvaluationsURLparses the URL (trailing slash / query no longer defeat the batch-URL derivation), with tests for both near-misses.- builtin
Evaluationsmemoizes admin facts per batch, so/me/permissionsno longer repeats user+SSO-link lookups per admin.* action, with a query-counting test.
Verified locally: build, vet, gofmt, golangci-lint (0 issues), and -race tests all green. CI: lint, unit-tests, integration-tests, check-diff all passing. LGTM.
|
PR approved. Marking ready for e2e. |
Implements BCH-1315 (AuthZ Phase 3) from the CCF Pluggable Authorization — Design Plan (§3.3, §6, open question 4). Builds on Phase 1 (#427): reuses the
internal/authzPDPinterface, driver registry, embedded manifest, and the singlemiddleware.PEP.Outcome: any AuthZen-compliant PDP (Topaz, Axiomatics, PlainID, …) becomes CCF's decision engine by changing one config key.
What's in here
authzenHTTP driver (internal/authz/authzen.go)PDP.Evaluate/Evaluationsagainst the OpenID AuthZen Authorization API.authz.endpointis the single-evaluation URL; the batch URL is derived by AuthZen convention (/evaluation→/evaluations), so it stays one config key.ErrUnavailable(PEP appliesauthz.fail_mode); other 4xx → 500 (a compliant PDP returns its verdict as a 200 body).Batch
Evaluations+/me/permissions(internal/api/handler/permissions.go)GET /api/me/permissions(JWT-auth, matches the BCH-1318 UI contract) returns the subject's allowedresource → [actions]map via a single batch call over the manifest vocabulary, so the UI can hide actions the user can't perform.SubjectFromContextso subject derivation is single-sourced.Optional short-TTL decision cache (
internal/authz/cache.go)authz.Openwhenauthz.cache_ttl > 0; absorbs the remote hop.Evaluationsserves hits locally and batches only misses into one inner call; bounded by a soft cap with opportunistic expiry sweep. Default off.Fail-closed + health (
internal/config/authz.go,internal/api/handler/health.go)authz.endpoint/authz.cache_ttlconfig (alongside existingdriver/fail_mode)./health/readygains an optional PDP dependency check via the newauthz.Healtherinterface (authzen probes/.well-known/authzen-configuration; the in-process builtin is always healthy).Tests
authzen_test.go: AuthZen request-shape conformance, single + batch decision mapping, canonical allow/deny cases, error→ErrUnavailableclassification, context-deadline handling, batch length mismatch, endpoint derivation, health probe, driver registration.cache_test.go: hit/miss, key variance, expiry, errors-not-cached, batch partial-miss single-inner-call, soft-cap eviction, health forwarding.permissions_test.go: allowed-subset mapping (single batch), fail-open/closed, internal-error → 500, subject derivation.go build ./...,go vet ./..., and the affected package tests are green.Scope held deliberately tight (other tickets)
/me/permissions.cedar/permit/openfga, noccf authz export/sync, no audit logging (later phases).🤖 Generated with Claude Code