feat(auth): drop arkavo_ prefix from OIDC discovery extensions#3538
Closed
arkavo-com wants to merge 8 commits into
Closed
feat(auth): drop arkavo_ prefix from OIDC discovery extensions#3538arkavo-com wants to merge 8 commits into
arkavo-com wants to merge 8 commits into
Conversation
* feat(authorization): pluggable file-backed policy provider Adds a read-only, in-memory policy provider (service/policy/filestore) that loads namespaces, attributes, subject mappings, registered resources, and obligations from a YAML or JSON snapshot. When configured via services.authorization.policy_file, both v1 and v2 authorization endpoints serve every lookup from memory. Running with mode "core,-policy" omits the policy CRUD service, so no database client is initialized — the production runtime no longer requires Postgres. Authoring continues on a separate admin server (otdfctl talks to that one) which exports the snapshot consumed here. * service/policy/filestore: provider implementing EntitlementPolicyStore and a new SubjectMappingMatcher; safe for concurrent reads, returns proto clones so callers can freely mutate without closing cycles. * service/authorization (v1): loadEntitlementPolicy and retrieveAttributeDefinitions branch on the store; SDK calls are preserved as the fallback path. * service/authorization/v2: PolicyFile takes precedence over the cache; Service.cache widened to access.EntitlementPolicyStore so the file store satisfies the same contract. * internal/access/v2: JustInTimePDP uses an optional SubjectMappingMatcher to filter subject mappings locally, avoiding an SDK round-trip to the policy service for stores that support it. * examples/config/policy.example.yaml: snapshot schema reference. * opentdf-core-no-db.yaml: turnkey config for the Postgres-free runtime. This commit also re-bases the branch onto opentdf/platform's main; the prior fork history is preserved locally in the arkavo-fork-legacy branch. * feat(filestore): wire obligation triggers and registered resource AAVs Extends the file-backed policy schema so the v2 PDP and obligations PDP can actually decision against snapshot data — previously these objects loaded but had no edges. * registered_resources[].values[].action_attribute_values: binds an action taken on a registered resource value to the attribute value that gates it. Resolves to the live *policy.Value from the snapshot, so the v2 PDP's entitleable-attributes graph includes registered resource edges. * obligations[].values[].triggers: pairs an attribute value FQN with an action and (optionally) PEP client_id contexts. The obligations PDP iterates these at construction to build its trigger lookup map. ObligationValue back-reference on the trigger is intentionally omitted to keep proto.Clone safe (no cycles). * Registered resource and obligation value FQNs are computed in the builder using the same format expected by the PDP — namespaced https://<ns>/reg_res/<name>/value/<v> and obl/... respectively, with legacy https://reg_res/... when namespace is absent. * Builder errors out on unknown attribute_value_fqn references in AAVs or triggers; tests cover the rejection path. * New integration test feeds a full snapshot through access.NewPolicyDecisionPoint and obligations.NewObligationsPolicyDecisionPoint to prove the produced graphs are shape-compatible with the real decisioning code. * fix(authz): satisfy golangci-lint on filestore + slog calls - Replace direct proto.Clone type assertions in filestore.Store with a generic cloneAs helper so errcheck/forcetypeassert no longer flag the call sites (8 occurrences). - Rename inner 'ok' in buildSubjectMappings to refOK to silence the govet shadow warning. - Wrap deprecated policy.Action.Value assignment with nolint:staticcheck and a note pointing at the action-name migration (DECRYPT->read, TRANSMIT->create) that the rest of the codebase still owes. - Run gofumpt over the new filestore files (struct tag alignment). - Split slog.Error / slog.Info argument lists onto separate lines per sloglint in authorization/authorization.go and v2/authorization.go. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(authz): explain nolint directive for deprecated proto field nolintlint requires a comment after the //nolint directive; add the explanation that the rest of the codebase still owes the action-name migration before the deprecated field can be removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
* feat(authorization): RFC 8693 + RFC 9396 token-exchange POC
Adds a token-decoration endpoint that takes a verified upstream JWT and
mints a new platform-signed access token carrying RFC 9396
authorization_details. Disabled by default; opt-in via
services.authorization.rar.enabled.
Flow:
1. Client presents an IdP-issued subject_token to POST
/v2/authorization/token (RFC 8693, form-encoded).
2. Handler verifies the subject_token using the existing platform
AccessTokenVerifier.
3. For every requested authorization_details entry, fans (action ×
location) pairs out to the existing v2 PDP. Only permitted
combinations make it into the response.
4. Issues a JWT signed by an ephemeral Ed25519 key; public JWKS is
served at GET /v2/authorization/jwks.json for resource servers.
Scope is deliberately narrow — this is "token decoration", not a full
OAuth Authorization Server:
* Only grant_type=urn:ietf:params:oauth:grant-type:token-exchange.
* Only requested_token_type=urn:ietf:params:oauth:token-type:jwt.
* Only detail type "opentdf_attribute" with actions + locations.
* No /authorize, no refresh tokens, no PKCE, no client registration.
* Signing key is in-memory; restart rotates and invalidates issued
tokens. A persistent KMS-backed signer is a follow-up.
Implementation:
* rar.go — HTTP handlers (token + JWKS), parsing, validation, PDP
fan-out. Sets up gRPC metadata + audit transaction on the request
context so the PDP's existing audit + client-id machinery works
when called outside an interceptor chain.
* rar_signer.go — Ed25519 keypair, JWT minting, JWKS export.
* authorization.go — single buildRARHandler helper returns the
HandlerServer for every RegisterFunc exit; serviceregistry mounts
routes on the platform HTTP mux.
* config.go — RARConfig (enabled, issuer, token_ttl) added under
services.authorization.rar.
* rar_test.go — signer round-trip, parser, validator, and an
end-to-end test that loads a file-store policy, stubs ERS +
verifier, posts to the live httptest endpoint, parses the issued
JWT under the JWKS, and asserts the granted authorization_details
match what the PDP permits.
* fix(authz): satisfy golangci-lint on RAR endpoint
- Annotate RFC 8693 IANA URI constants (grant_type, token-type) with
nolint:gosec G101; they are public identifiers, not credentials.
- Rename test stub error type assertErr -> assertError to match errname
xxxError convention.
- Run gofumpt over rar.go, rar_signer.go, rar_test.go.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
Reshapes the work into the two distinct PDP layers from the discussion:
- Entitlement PDP (service/authorization/v2 RAR endpoint):
materialize the subject's full grant set and embed it in a signed JWT.
- Access PDP (service/access/local, new package): pure boolean
evaluation over the materialized grants — local, deterministic, no
remote calls. Importable from any Go resource server.
The RAR endpoint now defaults to full materialization (no
authorization_details body returns the subject's entire entitlement set).
Projection mode is preserved as opt-in narrowing.
Rebased onto main after #8 and #11 (originally posted as #10 atop #9).
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Updated project name and removed vulnerability badge.
…#13) * feat(authz): accept CWT subject tokens at /v2/authorization/token Adds a new subject_token_type to the RFC 8693 token-exchange endpoint: urn:ietf:params:oauth:token-type:cwt When enabled, the endpoint accepts base64url-encoded RFC 8392 CWTs (COSE_Sign1 / ES256) as subject tokens. The verifier fetches the IdP's COSE Key Set from the configured /.well-known/cose-keys endpoint (authnz-rs publishes one), verifies the signature, validates iss / aud / exp / nbf per RFC 8392 §3, and decodes CWT claims (integer labels per §4 plus arkavo_* text-label customs). Bridge to the existing claims-mode ERS uses an unsigned (alg=none) JWT synthesized from the verified CWT claims — the ERS calls jwt.Parse with WithVerify(false), so the unsigned representation is safe (trust was already enforced upstream in the RAR endpoint) and cheap (no second signing key needed). No proto change. Opt-in via the new RARConfig.CWTVerifier block: server: authorization: rar: enabled: true cwt_verifier: enabled: true cose_keys_url: https://authnz.example/.well-known/cose-keys issuer: https://authnz.example audience: opentdf-platform algorithm: ES256 cache_ttl: 10m Disabled by default. JWT-family subject tokens continue to work unchanged; a request with subject_token_type=cwt when the verifier is not configured returns 400 invalid_request. New files: - service/internal/auth/cwt_verifier.go (verifier + caches) - service/internal/auth/cwt_verifier_test.go (9 unit tests) - service/authorization/v2/rar_cwt_test.go (4 integration tests via httptest) Modified: - service/authorization/v2/rar.go (URN switch + dispatch) - service/authorization/v2/authorization.go (constructor wiring) - service/authorization/v2/config.go (CWTVerifierConfig) - opentdf-example.yaml (commented example block) Dependencies added: github.com/veraison/go-cose v1.3.0, github.com/fxamacker/cbor/v2 v2.9.2 (both MIT, mature COSE/CBOR implementations). Tests: all new tests pass, existing service tests still green, clippy on changed files clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(authz): emit CWT access tokens by default at /v2/authorization/token Completes the "CWT throughout" story started in #13: the platform now emits RFC 8392 CWTs (COSE_Sign1 / ES256) as the default response from the RFC 8693 token-exchange endpoint, alongside the existing JWT path which becomes opt-in. ## Wire shape CWT response body — raw CBOR, Content-Type: application/cwt+cbor: CWT payload (CBOR map): 1 (iss): configured issuer 2 (sub): entity subject 3 (aud): caller-supplied audience (omitted if absent) 4 (exp): unix seconds 5 (nbf): unix seconds 6 (iat): unix seconds 7 (cti): 16-byte UUID "authorization_details": [ {type, actions[], locations[], obligations[]}, ... ] Standard claims use RFC 8392 integer labels; authorization_details uses a text label with the JSON shape so existing `local.Grant` consumers read the same fields. The signer is an ephemeral ES256 (P-256) keypair, regenerated each process restart. The matching public key is published as a one-entry CBOR COSE Key Set (RFC 9052 §7) at GET /v2/authorization/cose-keys — the same shape authnz-rs serves at /.well-known/cose-keys, so PEPs can use one COSE verifier code path for both directions. ## API change - Default `requested_token_type` flips from JWT to CWT. - Existing JSON-envelope behavior is preserved when clients explicitly send `requested_token_type=urn:ietf:params:oauth:token-type:jwt`. - New endpoint: GET /v2/authorization/cose-keys (CBOR Key Set). This is a breaking change for clients that depend on the JSON envelope shape without setting requested_token_type. Mitigation: add the JWT URN to existing client request forms. ## What's new - service/authorization/v2/rar_cwt_signer.go: RARCWTSigner (ephemeral ES256, Issue, COSEKeySet). - service/authorization/v2/rar.go: format dispatch on requested_token_type; /v2/authorization/cose-keys handler. - service/authorization/v2/authorization.go: parallel CWT signer construction in buildRARHandler. - service/authorization/v2/rar_cwt_response_test.go: 6 integration tests covering default-is-CWT, signature verifies against published keys, signer-absent rejection, COSE keys endpoint, JWT opt-in, unknown URN. ## Test coverage - All existing rar_test.go and rar_cwt_test.go tests updated to opt into JWT explicitly so they continue asserting on the JSON envelope. - 6 new tests covering the CWT response path. - `go test ./authorization/v2/ ./internal/auth/` green. - golangci-lint clean on changed files. ## Out of scope - Configurable signing-key persistence (still ephemeral per process). - Multi-key rotation in the published COSE Key Set. - Removing the JWT path entirely (kept for compatibility). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rename two custom OIDC-discovery extension fields to their unprefixed form on the wire: arkavo_cose_keys_uri -> cose_keys_uri arkavo_access_token_format -> access_token_format Also surface access_token_format in the well-known passthrough (the field was previously silently dropped by the OIDCConfiguration struct's unmarshal/remarshal round-trip), and remove the fabricated require_request_uri_registration: false entry (a zero-value bool the struct was emitting even though the IdP doesn't advertise it). This is the consumer side. Pair with the matching publisher change in authnz-rs (arkavo-org/authnz-rs#tbd). JWT bearer tokens remain rejected on inbound paths -- only the discovery field names change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Paul Flynn <paul@arkavo.com>
github-actions
Bot
added
comp:policy
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant architectural changes to support decentralized authorization and reduce platform dependencies. By implementing a file-backed policy store and an RFC 8693-compliant token exchange endpoint, the platform can now operate without a persistent database for policy lookups. Additionally, the transition to CWT-based bearer tokens and the inclusion of a local Access PDP significantly improve performance and security for resource servers. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. The policy file is read with care, No database to find it there. With CWTs the tokens fly, And authorization passes by. Footnotes
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (34)
📝 WalkthroughWalkthroughThis PR implements RFC 8693 token exchange (RAR) with RFC 9396 authorization details, file-backed policy snapshots, CWT subject-token verification, and a local authorization PDP. It replaces JWT bearer verification with CWT/COSE, adds file-loaded policy storage, introduces a deterministic grant-based access control system, and wires the complete token-exchange flow into the authorization service. ChangesCore Authorization & Policy Components
RFC 8693 Token Exchange with Authorization Details
Service Integration & Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Author
|
Wrong target repo — re-opening against arkavo-org/opentdf-platform |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a file-backed, read-only policy provider (filestore) to allow the authorization service to run without a Postgres dependency. It also implements an RFC 8693 token-exchange endpoint (RAR) supporting RFC 9396 authorization_details claims, with support for both JWT and CBOR-encoded CWT (RFC 8392) subject and response tokens. The review feedback highlights several critical robustness improvements: ensuring a fallback to the client ID when the subject claim is missing, configuring a timeout on the HTTP client used for fetching COSE keys, limiting the read size of the key set response body to prevent OOM issues, and adding a defensive bounds check when iterating over resource decisions to avoid potential out-of-bounds panics.
Comment on lines
+242
to
+247
| subject, _ := verified.Get("sub") | ||
| subjectStr, _ := subject.(string) | ||
| if subjectStr == "" { | ||
| subjectStr = verified.Subject() | ||
| } | ||
| clientID := clientIDFromToken(verified) |
Contributor
There was a problem hiding this comment.
For client-credentials tokens, the sub claim may be absent, resulting in an empty subjectStr. This leads to issuing tokens with an empty subject, which can cause failures in downstream resource servers. Falling back to the clientID when subjectStr is empty ensures robust identification of the calling principal.
Suggested change
| subject, _ := verified.Get("sub") | |
| subjectStr, _ := subject.(string) | |
| if subjectStr == "" { | |
| subjectStr = verified.Subject() | |
| } | |
| clientID := clientIDFromToken(verified) | |
| clientID := clientIDFromToken(verified) | |
| subject, _ := verified.Get("sub") | |
| subjectStr, _ := subject.(string) | |
| if subjectStr == "" { | |
| subjectStr = verified.Subject() | |
| } | |
| if subjectStr == "" { | |
| subjectStr = clientID | |
| } | |
| if subjectStr == "" { | |
| writeError(w, http.StatusUnauthorized, "invalid_token", "subject token has no subject or client ID") | |
| return | |
| } |
Comment on lines
+132
to
+134
| if cfg.HTTPClient == nil { | ||
| cfg.HTTPClient = http.DefaultClient | ||
| } |
Contributor
There was a problem hiding this comment.
Using http.DefaultClient without a timeout can cause the application to hang indefinitely if the external OIDC provider or COSE Key Set endpoint becomes unresponsive. Constructing a dedicated client with a sensible timeout prevents resource exhaustion and goroutine leaks.
Suggested change
| if cfg.HTTPClient == nil { | |
| cfg.HTTPClient = http.DefaultClient | |
| } | |
| if cfg.HTTPClient == nil { | |
| cfg.HTTPClient = &http.Client{ | |
| Timeout: 15 * time.Second, | |
| } | |
| } |
| if resp.StatusCode != http.StatusOK { | ||
| return nil, fmt.Errorf("cose key set fetch returned %d", resp.StatusCode) | ||
| } | ||
| body, err := io.ReadAll(resp.Body) |
Contributor
There was a problem hiding this comment.
Reading the entire response body using io.ReadAll without a limit can lead to Out-Of-Memory (OOM) issues if the server returns an unexpectedly large payload. Limiting the reader to a reasonable size (e.g., 1MB) mitigates this risk.
Suggested change
| body, err := io.ReadAll(resp.Body) | |
| body, err := io.ReadAll(io.LimitReader(resp.Body, 1024*1024)) |
| if err != nil { | ||
| return nil, fmt.Errorf("decide action %q: %w", action, err) | ||
| } | ||
| for i, rd := range resp.Msg.GetResourceDecisions() { |
Contributor
There was a problem hiding this comment.
Accessing locations[i] directly assumes that the number of decisions returned by the PDP matches the number of requested locations. If there is a mismatch, this will panic with an index out of bounds. Adding a defensive bounds check prevents potential runtime panics.
Suggested change
| for i, rd := range resp.Msg.GetResourceDecisions() { | |
| decisions := resp.Msg.GetResourceDecisions() | |
| if len(decisions) != len(locations) { | |
| return nil, fmt.Errorf("decide action %q: decision count mismatch (got %d, want %d)", action, len(decisions), len(locations)) | |
| } | |
| for i, rd := range decisions { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
arkavo_cose_keys_uri→cose_keys_uriinOIDCConfiguration(service/internal/auth/discovery.go).AccessTokenFormat string \json:"access_token_format,omitempty"`so the IdP'saccess_token_format: application/cwtsurvives the struct round-trip and surfaces in/.well-known/opentdf-configuration` instead of being silently dropped.RequireRequestURIRegistrationfield — its zero-valuefalsewas being fabricated into the output even though the IdP doesn't actually advertise it.Coordination
This is the consumer side. Pair with the matching publisher change in arkavo-org/authnz-rs#33. Deploy order must be:
identity.arkavo.netstarts publishingcose_keys_uri.token_verifier.go:51-57:idp ... does not advertise cose_keys_uri).No protocol change beyond the field names; JWT bearer tokens remain rejected on inbound paths.
Test plan
go build ./service/internal/auth/... ./service/pkg/server/...— cleango test ./service/internal/auth/... ./service/pkg/server/...— passgolangci-lint run ./service/internal/auth/... ./service/pkg/server/...— 10 pre-existing issues, none in changed filescurl http://127.0.0.1:8181/.well-known/opentdf-configurationreturnscose_keys_uri+access_token_formatand norequire_request_uri_registration🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Configuration
policy_fileconfiguration option for loading entitlement policies from YAML/JSON snapshots.