Skip to content

BCH-1315: AuthZ Phase 3 — AuthZen HTTP driver + batch Evaluations + /me/permissions#428

Open
ccf-lisa[bot] wants to merge 3 commits into
mainfrom
lisa/feat/bch-1315
Open

BCH-1315: AuthZ Phase 3 — AuthZen HTTP driver + batch Evaluations + /me/permissions#428
ccf-lisa[bot] wants to merge 3 commits into
mainfrom
lisa/feat/bch-1315

Conversation

@ccf-lisa

@ccf-lisa ccf-lisa Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

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/authz PDP interface, driver registry, embedded manifest, and the single middleware.PEP.

Outcome: any AuthZen-compliant PDP (Topaz, Axiomatics, PlainID, …) becomes CCF's decision engine by changing one config key.

What's in here

authzen HTTP driver (internal/authz/authzen.go)

  • Implements PDP.Evaluate / Evaluations against the OpenID AuthZen Authorization API.
  • Single Access Evaluation + batch Access Evaluations (one HTTP call per batch, not N).
  • authz.endpoint is the single-evaluation URL; the batch URL is derived by AuthZen convention (/evaluation/evaluations), so it stays one config key.
  • Fail-mode contract: transport failure / context timeout / 5xx / 429 → ErrUnavailable (PEP applies authz.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 allowed resource → [actions] map via a single batch call over the manifest vocabulary, so the UI can hide actions the user can't perform.
  • Honors fail mode (closed → empty, open → full vocabulary). Reuses the PEP's SubjectFromContext so subject derivation is single-sourced.

Optional short-TTL decision cache (internal/authz/cache.go)

  • Generic decorator applied in authz.Open when authz.cache_ttl > 0; absorbs the remote hop.
  • Caches only successful decisions; key = sha256 of the full tuple; Evaluations serves 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)

  • New authz.endpoint / authz.cache_ttl config (alongside existing driver / fail_mode).
  • /health/ready gains an optional PDP dependency check via the new authz.Healther interface (authzen probes /.well-known/authzen-configuration; the in-process builtin is always healthy).
authz:
  driver: authzen
  endpoint: https://topaz.internal/access/v1/evaluation
  fail_mode: closed
  cache_ttl: 5s

Tests

  • authzen_test.go: AuthZen request-shape conformance, single + batch decision mapping, canonical allow/deny cases, error→ErrUnavailable classification, 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)

  • Existing list endpoints are not rewritten to batch-filter (premature before the BCH-1319 attribute surface); the concrete batch consumer shipped here is /me/permissions.
  • No new Subject/Resource attributes (BCH-1319 owns that). No cedar/permit/openfga, no ccf authz export/sync, no audit logging (later phases).

🤖 Generated with Claude Code

@gusfcarvalho gusfcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread internal/authz/authzen.go Outdated
Comment thread internal/api/handler/permissions.go
Comment thread internal/authz/authzen.go

@gusfcarvalho gusfcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All review feedback addressed in 555f147 and verified:

  • errcheck blocker fixed (unchecked resp.Body.Close()) — CI lint now green.
  • Response body now drained before close, restoring keep-alive reuse on the PEP hot path.
  • deriveEvaluationsURL parses the URL (trailing slash / query no longer defeat the batch-URL derivation), with tests for both near-misses.
  • builtin Evaluations memoizes admin facts per batch, so /me/permissions no 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.

@ccf-lisa

ccf-lisa Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

PR approved. Marking ready for e2e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant