From a6de0eb5db48f30d71edc6b4b5ac38d9f8771f9b Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Sun, 20 Sep 2026 22:07:39 +0100 Subject: [PATCH 01/19] feat: enforce downstream JWT trust and API permissions Require configured issuer, audience, expiry, and signing algorithms. Bound JWKS refreshes, normalize Watson identity claims, and enforce MCPUser permissions before configuration lookup with reusable Admin guards. Add authentication and backend-denial regression tests, runtime configuration examples, and documentation of the remaining tenant-isolation and integration work. Signed-off-by: Pratik Gandhi --- README.md | 12 +- _context/wiki/architecture.md | 7 +- _context/wiki/config.md | 99 +++-- _context/wiki/deployment.md | 6 +- _context/wiki/failure-modes.md | 6 +- _context/wiki/getting-started.md | 5 +- _context/wiki/performance.md | 31 ++ _context/wiki/security.md | 35 +- .../src/authorization/jwks/jwks.rs | 227 +++++----- .../src/authorization/jwks/jwks/tests.rs | 165 ++++++++ .../authorization/jwks/jwks_authorization.rs | 361 ++-------------- .../src/authorization/mod.rs | 37 +- .../cel_principal_extractor.rs | 116 ++--- .../default_principal_extractor.rs | 139 +++++- .../authorization/principal_extractor/mod.rs | 193 ++++++++- .../src/common/cli_config.rs | 28 ++ .../src/common/config.rs | 28 +- .../src/common/mod.rs | 12 + .../contextforge-data-plane-lib/src/errors.rs | 4 +- .../src/layers/claims_id.rs | 26 +- .../src/layers/mcp_header_limits.rs | 4 +- .../src/layers/mod.rs | 2 + .../src/layers/permission.rs | 52 +++ .../src/layers/principal_extractor.rs | 4 +- crates/contextforge-data-plane-lib/src/lib.rs | 21 +- .../contextforge-data-plane-lib/src/tools.rs | 9 +- .../tests/gateway.rs | 3 + .../tests/gateway/downstream_auth.rs | 397 ++++++++++++++++++ .../tests/gateway/harness/auth.rs | 8 +- .../tests/gateway/harness/mod.rs | 10 +- docker/docker-compose.yml | 24 ++ 31 files changed, 1494 insertions(+), 577 deletions(-) create mode 100644 crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs create mode 100644 crates/contextforge-data-plane-lib/src/layers/permission.rs create mode 100644 crates/contextforge-data-plane-lib/tests/gateway/downstream_auth.rs diff --git a/README.md b/README.md index 2f4afbfb..f23c5598 100644 --- a/README.md +++ b/README.md @@ -15,12 +15,18 @@ test stack: ```bash export CONTEXTFORGE_DATA_PLANE_JWKS_URL=https://your-issuer.example/.well-known/jwks.json +export CONTEXTFORGE_DATA_PLANE_JWT_ISSUER=https://your-issuer.example +export CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES=your-gateway-audience make docker-prod make compose-up ``` -Replace the example JWKS URL with the HTTPS endpoint for the issuer of your -control-plane tokens. The production image includes the plugin factories and +Replace the example trust settings with the exact issuer, accepted audience, +and HTTPS JWKS endpoint for your tokens. Tokens must also contain the user, +tenant, and role claims described in [JWT configuration](_context/wiki/config.md#jwt-claims-validated-by-claims_layer). +Existing control-plane tokens with a different claim layout need an explicit +CEL mapping; a valid signature alone no longer grants MCP access. The production +image includes the plugin factories and `/contextforge-rs/health`; it does not include `with_tools`. That feature is for testing only and enables unauthenticated token, JWKS, and config helpers. @@ -79,7 +85,6 @@ cargo run --release \ --redis-port 6379 \ --redis-address 127.0.0.1 \ --jwks-url "$CONTEXTFORGE_DATA_PLANE_JWKS_URL" \ - --number-of-cpus 16 \ --redis-mode=plain-text \ --upstream-connection-mode=plain-text-or-tls \ --runtime-plugins-enabled true @@ -99,6 +104,7 @@ cargo run -p contextforge-data-plane --features with_tools \ --redis-mode plain-text \ --token-verification-private-key ./assets/jwt.key \ --jwks-url http://127.0.0.1:8080/contextforge-rs/admin/.well-known/jwks.json \ +--jwt-issuer local-dev --jwt-audiences local-gateway \ --upstream-connection-mode plain-text-or-tls \ --tls-address 127.0.0.1:8443 \ --server-private-key ./assets/tls_key.pem \ diff --git a/_context/wiki/architecture.md b/_context/wiki/architecture.md index 246745c4..9494a64d 100644 --- a/_context/wiki/architecture.md +++ b/_context/wiki/architecture.md @@ -15,8 +15,9 @@ TCP/TLS listener -> CORS layer -> mcp_header_limits_layer bounds MCP headers (431) -> virtual_host_id_layer inserts VirtualHostId from path (400) - -> claims_layer verifies JWT, inserts AuthorizationClaims (401) + -> claims_layer verifies JWT, inserts AuthorizationClaims (401; JWKS unavailable 503) -> PrincipalExtractorLayer inserts AuthorizedPrincipal (401) + -> require_permission requires MCPUser (403), before any config lookup -> user_config_store_layer loads UserConfig (400 missing, 500 decode/error) -> virtual_host_config_layer checks caller's virtual host (404) -> /servers/{virtual_host_name}/mcp RMCP service @@ -38,7 +39,7 @@ consume typed extensions; they do not parse Redis keys. ```text modern MCP request - -> header, JWT, and principal checks + -> header, JWT, principal, and MCPUser permission checks -> user config and virtual-host check -> published object/backend route -> recognized tool parameter-header validation @@ -84,7 +85,7 @@ into successful response hooks. See [Routing](routing.md) and | State | Owner | Lifetime | | --- | --- | --- | | Parsed config and shared upstream HTTP client | Gateway | Process. | -| JWKS keys | JWT authorization service | Five-minute cache; fetched when verification needs them. | +| JWKS keys | JWT authorization service | Five-minute cache; serialized refresh, five-second cooldown; no expired-key fallback. | | User config | Redis store and optional local LRU | Redis is authoritative; local capacity 50,000, default expiry 60 seconds. | | Principal, claims, virtual-host ID, config snapshot | HTTP request extensions | One request. | | Backend RMCP service | Routed operation | One request; explicitly closed after the call. | diff --git a/_context/wiki/config.md b/_context/wiki/config.md index 0b1bd36c..9fcc6257 100644 --- a/_context/wiki/config.md +++ b/_context/wiki/config.md @@ -3,7 +3,7 @@ ## Minimum Required Flags ```text ---redis-address --redis-port --redis-mode --jwks-url +--redis-address --redis-port --redis-mode --jwks-url --jwt-issuer --jwt-audiences ``` Plus at least one listener: `--address` or `--tls-address`. Development builds @@ -33,6 +33,14 @@ Origin and Host settings retain the explicitly configured | `--server-private-key ` | `CONTEXTFORGE_DATA_PLANE_TLS_SERVER_PRIVATE_KEY` | With `--tls-address` | PEM private key for downstream TLS. | | `--jwks-url ` | `CONTEXTFORGE_DATA_PLANE_JWKS_URL` | Required | Fetches RSA/EC JWT verification keys. HTTPS required except for loopback HTTP testing. | | `--jwks-ca-cert-path ` | `CONTEXTFORGE_DATA_PLANE_JWKS_CA_PATH` | Optional | PEM CA bundle trusted by the JWKS HTTP client. | +| `--jwt-issuer ` | `CONTEXTFORGE_DATA_PLANE_JWT_ISSUER` | Required | Exact trusted issuer. | +| `--jwt-audiences ` | `CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES` | Required | At least one token audience must match. | +| `--jwt-algorithms ` | `CONTEXTFORGE_DATA_PLANE_JWT_ALGORITHMS` | `RS256` | Explicit RSA/EC allowlist; HMAC is rejected at startup. | +| `--jwt-leeway-seconds ` | `CONTEXTFORGE_DATA_PLANE_JWT_LEEWAY_SECONDS` | `30` | Time tolerance, maximum 300 seconds. | +| `--jwt-user-claim ` | `CONTEXTFORGE_DATA_PLANE_JWT_USER_CLAIM` | `sub` | `sub` or explicit `wo-user-id` profile. | +| `--jwt-admin-scopes ` | `CONTEXTFORGE_DATA_PLANE_JWT_ADMIN_SCOPES` | None | Scope names granting Admin and MCPUser. Setting either mapping makes scopes restrict roles. | +| `--jwt-mcp-user-scopes ` | `CONTEXTFORGE_DATA_PLANE_JWT_MCP_USER_SCOPES` | None | Scope names granting MCPUser only. | +| `--jwt-scopes-only` | `CONTEXTFORGE_DATA_PLANE_JWT_SCOPES_ONLY` | `false` | Explicit scope-only authorization; requires a scope mapping. | | `--token-verification-private-key ` | None (CLI only) | Required when built with `with_tools` | Signs local test tokens and supplies the public key served by the local JWKS helper. | | `--cel-principal-extractor-path ` | None (CLI only) | Optional | CEL principal mapping for custom claim layouts; otherwise uses the default user/tenant claim mapping below. | @@ -108,31 +116,65 @@ the HTTP transport. ## JWT Claims (validated by `claims_layer`) -The JWT signature is checked against the configured JWKS. The default principal -extractor then requires user and tenant IDs at the top level of the claims: +JWT trust settings come from deployment configuration, never from token headers +or claims. The verifier requires an allowed signature algorithm and a nonempty +`kid` that matches a configured JWKS signing key, including its declared algorithm. -| Claim | Current behavior | +| Claim | Behavior | | --- | --- | -| `sub`, `user_id`, `UserId` | First present alias must be a string; supplies the user ID used for Redis config lookup. | -| `tenantId`, `tenant_id` | First present alias must be a string; supplies the principal's tenant ID. | -| `exp` | Checked when present; the local helper sets a one-hour expiry. | -| `nbf` | Checked when present; rejects tokens that are not yet valid, subject to verifier leeway. | -| `iss`, `aud` | No fixed issuer or audience is currently enforced by the JWKS verifier. | - -The default extractor does not infer the tenant from `teams`, email, or a nested -`user` object. Use `--cel-principal-extractor-path` for a custom mapping. -An earlier alias with a non-string value prevents fallback to a later alias. -The tenant ID is required by extraction but is not currently included in the -user-config Redis/cache key. JWT scopes and RBAC are not enforced here; object -visibility comes from the published routing maps. - -The local `GET /contextforge-rs/admin/tokens/{tenant_id}/{user_id}` helper sets -`tenant_id` and `sub` from the path. Its raw JWT response belongs in the -`Authorization: Bearer ...` header; it is not a JSON token object. - -There is no per-token revocation. Verification keys are cached for five minutes; -removing a key from JWKS is not immediate invalidation of cached keys. Restart -the dataplane after removing a key if that cache must be cleared immediately. +| `iss` | Required; exact configured issuer. Preserved in the request identity. | +| `aud` | Required string or string array; must include a configured audience. | +| `exp` | Required and validated, with configured clock leeway. | +| `nbf` | Validated when present; malformed or future values are rejected. | +| `sub` | Default required nonempty string user ID. No implicit fallback. | +| `woUserId` | Used instead of `sub` only with `--jwt-user-claim wo-user-id`. | +| `woTenantId`, `tenant_id`, `tenantId` | At least one nonempty string required. All present aliases must agree. | +| `role`, `roles` | String and string list respectively. `admin` grants Admin and MCPUser; `builder`/`user` grant MCPUser. Unknown or missing roles grant nothing. | +| `scope`, `permissions` | Space-separated string and string list respectively. Combined into normalized scopes; no scope grants access without explicit configuration. | + +Every MCP request requires MCPUser before configuration lookup. Missing/invalid +identity returns 401; valid identity without permission returns 403. A reusable +Admin guard is available for future management APIs; this branch adds no such APIs. + +By default roles decide permissions. Setting either scope mapping intersects +role permissions with scope permissions; absent/empty/unmapped scopes then deny. +For example, with `--jwt-mcp-user-scopes aipg.mcp`, even an admin needs that +scope to use MCP, and cannot gain Admin from it. `--jwt-scopes-only` explicitly +switches to scope-based grants, including for tokens without roles. Empty scope +names in configuration are invalid. Malformed role/scope claim types return 401. +These role defaults and scope names need confirmation with the Watson token owner +before real integration. + +For custom or nested claims, `--cel-principal-extractor-path` takes a trusted CEL +expression returning `user_id`, `tenant_id`, and optional `role`, `roles`, `scope`, +and `permissions`. The input variable is `claims`. Example: + +```cel +{"user_id": claims.woUserId, "tenant_id": claims.woTenantId, "roles": claims.user.roles} +``` + +The same permission policy applies to CEL results. It cannot override the issuer, +which always comes from the verified token. There is no implicit default tenant. +User IDs need not be emails. The issuer and tenant are carried in the request +identity, but **persistent/cache keys still contain only the user ID**. This +change does not establish isolation between tenants sharing a subject. A coordinated +publisher/key migration and CPEX policy integration remain separate work. + +The development token helper sets the configured issuer/audiences, path-derived +`sub`, `woUserId`, `tenant_id`, and `role: user`. It uses RS256 and key ID `test`. +Its token is a raw string for `Authorization: Bearer ...`. Custom scope profiles +need matching claims through the POST helper. All `with_tools` routes remain +unauthenticated, testing-only helpers; production builds must exclude them. + +Keys are cached for five minutes. One fetch runs at a time and refresh attempts +are separated by at least five seconds, including after failure. Unknown key IDs +can trigger a refresh after that cooldown; rotation may take up to the cooldown +plus fetch time to be observed. Known unexpired keys remain usable during a JWKS +outage; expired keys never authenticate. Fetches have a ten-second total timeout, +a 1 MiB response limit, and no redirects. Unavailable or unusable JWKS returns 503 +when no usable cached key is available; invalid tokens return 401. No per-token +revocation lookup exists. Removing a signing key is effective after refresh or +cache expiry; restart to clear the cache immediately if necessary. ## UserConfig Shape (from `contextforge-data-plane-apis`) @@ -306,6 +348,7 @@ cargo run -p contextforge-data-plane \ --redis-port 6379 \ --redis-mode plain-text \ --jwks-url http://127.0.0.1:8001/contextforge-rs/admin/.well-known/jwks.json \ + --jwt-issuer local-dev --jwt-audiences local-gateway \ --token-verification-private-key assets/jwt.key \ --upstream-connection-mode plain-text-or-tls \ --runtime-plugins-enabled true @@ -328,7 +371,8 @@ cargo nextest run --locked -p contextforge-data-plane-lib --test gateway -E 'tes | mTLS upstream without certificate/key | Upstream HTTP client construction. | | Invalid JWKS URL scheme or non-loopback plain HTTP URL | Authorization-service construction. | | Missing/invalid plugin document when enabled | CPEX initialization, before serving requests. | -| Unreachable JWKS endpoint | Token verification when keys must be fetched. | +| Missing/empty JWT issuer/audience, non-RSA/EC algorithm, or excessive leeway | Authorization-service construction. | +| Unreachable JWKS endpoint | 503 during token verification when keys must be fetched. | | Unreachable backend or HTTP URL with default HTTPS-only mode | When a request selects that backend. | Redis connection setup retries rather than failing immediately. The local @@ -370,7 +414,9 @@ is **15 seconds**; allow up to about 45–60 seconds after generating traffic. | Symptom | Where to look | | --- | --- | -| `401` | Bearer header, `validate: unable to refresh SaaS JWKS`, `validate_and_decode_claims`, and `Can't extract the principal` logs. | +| `401` | Bearer header, signature/issuer/audience/time checks, and principal claim types. | +| `403` | Required API permission missing. | +| `503` | `jwks_refresh` failure logs and JWKS reachability. | | `400` config error | `user_config_store_layer` and whether the publisher used the extracted user ID. A Redis GET failure also maps here. | | `404 Server not found` | `virtual_host_config_layer`; requested vhost versus caller's published configuration. | | MCP routing errors | `AuthorizedCallValidator::validate` log prefix (from `validate_stateless`), then `call_tool`, `read_resource`, or `get_prompt` diagnostics. | @@ -412,6 +458,7 @@ cargo run --release -p contextforge-data-plane --features with_tools,plugins \ --address 127.0.0.1:8001 \ --redis-port 6379 --redis-address 127.0.0.1 --redis-mode plain-text \ --jwks-url http://127.0.0.1:8001/contextforge-rs/admin/.well-known/jwks.json \ + --jwt-issuer local-dev --jwt-audiences local-gateway \ --token-verification-private-key assets/jwt.key \ --upstream-connection-mode plain-text-or-tls \ --runtime-plugins-enabled true \ diff --git a/_context/wiki/deployment.md b/_context/wiki/deployment.md index b0cef3b2..8418c030 100644 --- a/_context/wiki/deployment.md +++ b/_context/wiki/deployment.md @@ -8,8 +8,10 @@ operation. There is no sticky-session requirement. Follow 1. Route the configured `/contextforge-rs` prefix to the external dataplane and keep older clients and legacy SSE on Python routes. -2. Configure a reachable trusted `--jwks-url` and a principal mapping matching - the publisher's user IDs and tenant claims. +2. Configure `--jwks-url`, exact `--jwt-issuer`, and `--jwt-audiences`. Match + the principal and role/scope mapping to issued tokens and publisher IDs. + Tenant-aware persistent keys are still pending; this change alone does not + isolate tenants with colliding user IDs. 3. Provide Redis connectivity and control-plane publication (`DATAPLANE_PUBLISHER=true` in the control-plane deployment). Restrict writes to trusted publishers; use TLS/mTLS across trust zones. diff --git a/_context/wiki/failure-modes.md b/_context/wiki/failure-modes.md index b625db0c..1d5baaff 100644 --- a/_context/wiki/failure-modes.md +++ b/_context/wiki/failure-modes.md @@ -12,8 +12,10 @@ layers may return before the layer listed below is reached. | MCP standard-header count or byte budget exceeded | `431` | `mcp_header_limits_layer`. | | A request reaching virtual-host extraction does not match `/servers/{id}/mcp` | `400` | `virtual_host_id_layer`; unrelated router paths may instead be `404`. | | Missing Authorization or non-Bearer scheme | `401` | `claims_layer`. | -| Bad JWT, unsupported algorithm, no matching JWKS key, fetch failure, or invalid time claim | `401` `Invalid token` | `claims_layer`. | -| Missing/non-string mapped user or tenant | `401` `Invalid token. Unable to extract the principal from claims` | `PrincipalExtractorLayer`. | +| Bad JWT, unsupported algorithm, unmatched key, wrong issuer/audience, missing exp, or invalid time claim | `401` `Invalid bearer token` | `claims_layer`; Bearer challenge included. | +| JWKS fetch/unusable document and no usable cached key | `503` | `claims_layer`; expired keys are never used. | +| Valid identity without MCPUser permission | `403` | `require_permission`, before configuration lookup. | +| Missing/empty/non-string user or tenant, conflicting tenant aliases, or malformed permission claims | `401` `Invalid token. Unable to extract the principal from claims` | `PrincipalExtractorLayer`. | | Missing user configuration | `400` | `user_config_store_layer`, keyed by extracted user ID. | | Config cannot be decoded / key cannot be encoded | `500` | Config store / `user_config_store_layer`. | | Virtual host absent from caller's config | `404` `{"detail":"Server not found"}` | `virtual_host_config_layer`. | diff --git a/_context/wiki/getting-started.md b/_context/wiki/getting-started.md index 0dc69da4..a48ece1e 100644 --- a/_context/wiki/getting-started.md +++ b/_context/wiki/getting-started.md @@ -68,6 +68,7 @@ cargo run -p contextforge-data-plane --features with_tools,plugins \ --redis-port 6379 \ --redis-mode plain-text \ --jwks-url http://127.0.0.1:8001/contextforge-rs/admin/.well-known/jwks.json \ + --jwt-issuer local-dev --jwt-audiences local-gateway \ --token-verification-private-key assets/jwt.key \ --upstream-connection-mode plain-text-or-tls \ --runtime-plugins-enabled true \ @@ -107,7 +108,7 @@ Expect `{"status": "healthy"}` and a JWKS document containing a `keys` array. The token response is a raw JWT, stored in `TOKEN` without printing it. Tokens expire after one hour; repeat the token command to refresh. Both tenant and user path segments are required. The helper sets top-level `tenant_id` and `sub` -claims; the optional email does not select the user's Redis configuration. +claims, configured issuer/audiences, and `role: user`; the optional email does not select the user's Redis configuration. Keep the listener, JWKS, token, and MCP URLs on the same instance. If you use port `9090`, change all four together. Port `8080` belongs to the full Docker @@ -243,6 +244,8 @@ then build and start the stack: ```bash export CONTEXTFORGE_DATA_PLANE_JWKS_URL=https://your-issuer.example/.well-known/jwks.json +export CONTEXTFORGE_DATA_PLANE_JWT_ISSUER=https://your-issuer.example +export CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES=your-gateway-audience make docker-prod make compose-up ``` diff --git a/_context/wiki/performance.md b/_context/wiki/performance.md index ab500ec0..889c5777 100644 --- a/_context/wiki/performance.md +++ b/_context/wiki/performance.md @@ -85,3 +85,34 @@ standalone snapshot that is not being republished during the run. Record request rate, latency percentiles, failures, and resource usage after warmup. Do not infer correctness or protocol coverage from successful load; run [workspace and conformance checks](testing.md) separately. + +## Cached Authentication Probe (issue #753) + +The first downstream-authentication slice includes an explicitly invoked probe: + +```bash +cargo +1.96 test --locked -p contextforge-data-plane-lib --all-features \ + --test gateway cached_authentication_load -- --ignored --nocapture +``` + +This is an in-process router measurement in the debug profile: 100 warmup +requests, then 8,000 signed-RSA `server/discover` requests at concurrency 16 +on four Tokio workers. It uses a loopback JWKS server and an in-memory config +store, without Redis, plugins, or a routed backend. It is not a production or +Watson integration load test. `--all-features` here is for testing only. + +On 2026-09-20, three alternating runs compared main `9836fdc` with the first +implementation on `user/pratik-gandhi/downstream-authentication`, using the same +probe and claims (including both equivalent tenant aliases): + +| Measurement (median of three runs) | Baseline | Auth implementation | +| --- | ---: | ---: | +| Requests/second | 11,179 | 10,585 | +| Per-run p95 latency | 1,866 µs | 2,280 µs | +| JWKS fetches per run | 1 | 1 | +| Failed requests | 0 | 0 | + +The additional checks cost about 5.3% throughput in this short debug probe; +latency tails varied across runs. Repeat in release mode and with the real +Watson deployment before drawing capacity conclusions. Use separate Cargo target +directories for the two worktrees to avoid replacing each other's crate artifacts. diff --git a/_context/wiki/security.md b/_context/wiki/security.md index ac9ed7e2..e980f19c 100644 --- a/_context/wiki/security.md +++ b/_context/wiki/security.md @@ -18,30 +18,35 @@ user database. Configuration does not require a management API call per request, but verification can fetch keys from the trusted issuer's JWKS endpoint. The request path is Origin/header checks → JWT verification → principal -extraction → user configuration → virtual-host check → RMCP validation → +extraction → MCPUser permission → user configuration → virtual-host check → RMCP validation → published object route → backend call. -- JWT verification uses RSA/EC JWKS keys. HMAC secrets and the old public-key - CLI flag are not supported. `exp` and `nbf` are validated when present; no - fixed issuer/audience or mandatory expiration claim is enforced today. -- The default extractor requires a string user ID (`sub`, `user_id`, or - `UserId`) and tenant ID (`tenantId` or `tenant_id`). The first present alias - wins and must have the right type. User IDs need not be emails. CEL can - define a custom mapping; see [Configuration](config.md#jwt-claims-validated-by-claims_layer). +- JWT verification requires `exp`, the configured issuer and audience, and an + explicitly allowed RSA/EC algorithm (`RS256` by default). `nbf` is validated + when present. HMAC and token-directed key URLs are not supported. +- The default identity uses `sub`, or `woUserId` under an explicit profile. + Tenant aliases `woTenantId`, `tenant_id`, and `tenantId` must be nonempty + strings and agree. No tenant is inferred from teams or email. CEL handles + custom layouts through the same identity and permission checks. +- `admin` grants Admin and MCPUser; `builder`/`user` grant MCPUser. Every MCP + request, including discovery/list requests, needs MCPUser before config lookup. + Unknown/missing roles grant nothing. Explicit scope mapping can restrict roles; + scope-only authorization requires explicit opt-in. See [Configuration](config.md#jwt-claims-validated-by-claims_layer). - The Redis/cache key currently contains **only the extracted user ID**, not the tenant. Identical user IDs in different tenants resolve to the same stored configuration. Tenant extraction alone is not an isolation boundary. - The virtual host and each targeted tool, resource, or prompt must exist in that user's published routing maps. Publishing a backend alone does not expose all its objects. The dataplane does not derive routes by prefix. -- JWT scopes, teams, and compiled RBAC are not independently enforced on this - path. Stronger isolation and policy checks in the +- Object-level authorization still comes from published routing maps. CPEX + policy integration and tenant-aware keys in the [target authorization model](mcp-capability-allocation.md#target-authorization-invariants) - are proposed work, not current guarantees. -- There is no per-token blocklist/revocation lookup. Keys are cached for five - minutes; removing a JWKS key is not immediate invalidation until refresh or - restart. A token without `exp` has no expiration enforced by this verifier. - Removing a user's published configuration blocks access after cache expiry. + remain future work. +- There is no per-token revocation lookup. Keys are cached for five minutes; + refresh is serialized with a five-second cooldown. Expired cached keys never + authenticate during an outage. Known fresh keys remain usable. Removing a key + takes effect after refresh, expiry, or restart. Removing published user config + blocks access after its cache expires. ## What Compromise Means diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs index 5482d25b..ac92321c 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs @@ -2,129 +2,172 @@ use std::time::Duration; use futures::StreamExt as _; use jsonwebtoken::{ - Algorithm, AlgorithmFamily, DecodingKey, Header, Validation, decode, + AlgorithmFamily, DecodingKey, Header, Validation, decode, jwk::{Jwk, JwkSet, KeyOperations, PublicKeyUse}, }; -use lru_time_cache::LruCache; - use reqwest::Url; use serde_json::Value; -use tokio::sync::RwLock; -use tracing::debug; -use typed_builder::TypedBuilder; - -use crate::authorization::{AuthorizationClaims, AuthorizationError}; +use tokio::{ + sync::{Mutex, RwLock}, + time::Instant, +}; -pub const JWKS_CACHE_TTL: Duration = Duration::from_mins(5); -pub const JWKS_CACHE_KEY: &str = "jwks"; +use crate::{ + JwksConfig, + authorization::{AuthenticationError, AuthorizationClaims, AuthorizationError}, +}; +const JWKS_CACHE_TTL: Duration = Duration::from_mins(5); +const JWKS_REFRESH_COOLDOWN: Duration = Duration::from_secs(5); const JWKS_MAX_RESPONSE_BYTES: usize = 1024 * 1024; -#[derive(TypedBuilder)] +struct CachedKeys { + keys: Vec, + expires_at: Instant, +} + +#[derive(Default)] +struct RefreshState { + last_attempt: Option, + failed: bool, +} + pub(super) struct Jwks { client: reqwest::Client, url: Url, - #[builder(default = RwLock::new(LruCache::with_expiry_duration(JWKS_CACHE_TTL)))] - cache: RwLock>>, - #[builder(default = false)] - validate_audience: bool, - #[builder(default = true)] - validate_expiry: bool, - #[builder(default = true)] - validate_not_before: bool, + validation: Validation, + cache: RwLock>, + // One refresh in flight, including concurrent unknown-kid requests. + refresh: Mutex, } impl Jwks { - fn validation(&self, alg: Algorithm) -> Validation { - let mut validation = Validation::new(alg); - validation.required_spec_claims.clear(); - validation.validate_aud = self.validate_audience; - validation.validate_exp = self.validate_expiry; - validation.validate_nbf = self.validate_not_before; - validation + pub fn new(client: reqwest::Client, url: Url, validation: Validation) -> Self { + Self { client, url, validation, cache: RwLock::new(None), refresh: Mutex::new(RefreshState::default()) } } - pub async fn validate(&self, token: &str, header: &Header) -> Option { + pub fn validation(config: &JwksConfig) -> Result { + if config.issuer.trim().is_empty() + || config.audiences.is_empty() + || config.audiences.iter().any(|aud| aud.trim().is_empty()) + || config.algorithms.is_empty() + || config.algorithms.iter().any(|alg| !matches!(alg.family(), AlgorithmFamily::Rsa | AlgorithmFamily::Ec)) + || config.leeway_seconds > 300 { - let cache = self.cache.read().await; - - if let Some(keys) = cache.peek(JWKS_CACHE_KEY) - && keys.iter().any(|key| key.matches(header)) - { - return Self::validate_with_keys(keys, token, header, &self.validation(header.alg)); - } + return Err(AuthorizationError::InvalidTrustConfiguration); } + let mut validation = Validation::new(config.algorithms[0]); + validation.algorithms.clone_from(&config.algorithms); + validation.set_required_spec_claims(&["exp", "iss", "aud"]); + validation.set_issuer(&[&config.issuer]); + validation.set_audience(&config.audiences); + validation.validate_nbf = true; + validation.leeway = config.leeway_seconds; + Ok(validation) + } - match fetch_jwks(&self.client, &self.url).await { - Ok(keys) => { - let key_count = keys.len(); - let claims = Self::validate_with_keys(&keys, token, header, &self.validation(header.alg)); - self.cache.write().await.insert(JWKS_CACHE_KEY.to_owned(), keys); - tracing::info!("validate: SaaS JWKS cache refreshed {key_count}"); + pub async fn validate(&self, token: &str, header: &Header) -> Result { + if !self.validation.algorithms.contains(&header.alg) + || header.kid.as_ref().is_none_or(|kid| kid.trim().is_empty()) + { + return Err(AuthenticationError::InvalidToken); + } + if let Some(result) = self.validate_cached(token, header).await { + return result; + } - claims - }, - Err(error) => { - tracing::info!("validate: unable to refresh SaaS JWKS {error:?}"); - None - }, + let mut refresh = self.refresh.lock().await; + // Another request may have loaded or rotated the keys while we waited. + if let Some(result) = self.validate_cached(token, header).await { + return result; } + if refresh.last_attempt.is_some_and(|time| time.elapsed() < JWKS_REFRESH_COOLDOWN) { + return Err(if refresh.failed { + AuthenticationError::KeysUnavailable + } else { + AuthenticationError::InvalidToken + }); + } + // Record before awaiting I/O so cancellation cannot bypass the cooldown. + refresh.last_attempt = Some(Instant::now()); + refresh.failed = true; + let result = fetch_jwks(&self.client, &self.url).await; + refresh.last_attempt = Some(Instant::now()); + refresh.failed = result.is_err(); + let keys = result.map_err(|_| { + // Do not log response bodies, key material, tokens, or URLs with query credentials. + tracing::warn!("jwks_refresh - unable to retrieve usable verification keys"); + AuthenticationError::KeysUnavailable + })?; + let claims = self.validate_with_keys(&keys, token, header).unwrap_or(Err(AuthenticationError::InvalidToken)); + *self.cache.write().await = Some(CachedKeys { keys, expires_at: Instant::now() + JWKS_CACHE_TTL }); + tracing::info!("jwks_refresh - verification keys refreshed"); + claims } - fn validate_with_keys( - keys: &[VerificationKey], + async fn validate_cached( + &self, token: &str, header: &Header, - validation: &Validation, - ) -> Option { - keys.iter() - .filter(|key| key.matches(header)) - .find_map(|key| Self::validate_and_decode_claims(token, &key.decoding_key, validation)) + ) -> Option> { + let cache = self.cache.read().await; + let cache = cache.as_ref().filter(|cache| cache.expires_at > Instant::now())?; + self.validate_with_keys(&cache.keys, token, header) } - fn validate_and_decode_claims( + fn validate_with_keys( + &self, + keys: &[VerificationKey], token: &str, - key: &DecodingKey, - validation: &Validation, - ) -> Option { - let claims = decode::(token, key, validation) - .inspect_err(|e| { - debug!("validate_and_decode_claims: problem {e:?}"); - }) - .ok()? - .claims; - - Some(AuthorizationClaims::from(claims)) + header: &Header, + ) -> Option> { + let key = keys.iter().find(|key| key.matches(header))?; + Some( + decode::(token, &key.decoding_key, &self.validation) + .map(|token| AuthorizationClaims::from(token.claims)) + .map_err(|_| AuthenticationError::InvalidToken), + ) } } -pub struct VerificationKey { - pub(crate) key_id: Option, - pub(crate) decoding_key: DecodingKey, +struct VerificationKey { + key_id: String, + decoding_key: DecodingKey, + algorithm: Option, } impl VerificationKey { - fn from_jwk(jwk: Jwk) -> Result, AuthorizationError> { + fn from_jwk(jwk: &Jwk) -> Result, AuthorizationError> { if jwk.common.public_key_use.as_ref().is_some_and(|key_use| key_use != &PublicKeyUse::Signature) || jwk.common.key_operations.as_ref().is_some_and(|operations| !operations.contains(&KeyOperations::Verify)) { return Ok(None); } - - let decoding_key = DecodingKey::from_jwk(&jwk).map_err(AuthorizationError::InvalidKey)?; - if !matches!(decoding_key.family(), AlgorithmFamily::Rsa | AlgorithmFamily::Ec) { + let Some(key_id) = jwk.common.key_id.as_ref().filter(|kid| !kid.trim().is_empty()) else { + return Ok(None); + }; + // Ignore symmetric and unsupported key types before decoding. + if !matches!( + jwk.algorithm, + jsonwebtoken::jwk::AlgorithmParameters::RSA(_) | jsonwebtoken::jwk::AlgorithmParameters::EllipticCurve(_) + ) { return Ok(None); } - - Ok(Some(Self { key_id: jwk.common.key_id, decoding_key })) + let algorithm = match jwk.common.key_algorithm { + Some(alg) => match jsonwebtoken::Algorithm::try_from(alg) { + Ok(alg) => Some(alg), + Err(_) => return Ok(None), + }, + None => None, + }; + let decoding_key = DecodingKey::from_jwk(jwk).map_err(AuthorizationError::InvalidKey)?; + Ok(Some(Self { key_id: key_id.clone(), decoding_key, algorithm })) } - pub(super) fn matches(&self, header: &Header) -> bool { - self.decoding_key.family() == header.alg.family() - && header - .kid - .as_ref() - .is_none_or(|header_key_id| self.key_id.as_ref().is_none_or(|key_id| key_id == header_key_id)) + fn matches(&self, header: &Header) -> bool { + header.kid.as_ref() == Some(&self.key_id) + && self.decoding_key.family() == header.alg.family() + && self.algorithm.is_none_or(|alg| alg == header.alg) } } @@ -135,10 +178,7 @@ async fn fetch_jwks(client: &reqwest::Client, url: &Url) -> Result u64::try_from(JWKS_MAX_RESPONSE_BYTES).unwrap_or(u64::MAX)) - { + if response.content_length().is_some_and(|length| length > JWKS_MAX_RESPONSE_BYTES as u64) { return Err(AuthorizationError::JwksResponseTooLarge); } let mut body = Vec::new(); @@ -151,21 +191,20 @@ async fn fetch_jwks(client: &reqwest::Client, url: &Url) -> Result(&body).map_err(AuthorizationError::InvalidJson)?; - if jwks.keys.is_empty() { Ok(Vec::new()) } else { validated_json_web_keys(jwks.keys) } -} - -pub(super) fn validated_json_web_keys( - jwks: impl IntoIterator, -) -> Result, AuthorizationError> { - let mut keys = Vec::new(); - for jwk in jwks { - if let Some(key) = VerificationKey::from_jwk(jwk)? { + let mut keys: Vec = Vec::new(); + for jwk in jwks.keys { + if let Some(key) = VerificationKey::from_jwk(&jwk)? { + if keys.iter().any(|existing| existing.key_id == key.key_id) { + return Err(AuthorizationError::DuplicateKeyId); + } keys.push(key); } } - if keys.is_empty() { return Err(AuthorizationError::NoSupportedKeys); } Ok(keys) } + +#[cfg(test)] +mod tests; diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs new file mode 100644 index 00000000..474821ac --- /dev/null +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs @@ -0,0 +1,165 @@ +use super::*; +use axum::{Router, routing::get}; +use http::StatusCode; +use jsonwebtoken::{Algorithm, EncodingKey, encode}; +use serde_json::json; +use std::sync::{ + Arc, + atomic::{AtomicUsize, Ordering}, +}; + +struct Fixture { + verifier: Jwks, + body: Arc)>>, + requests: Arc, + delay_ms: Arc, + task: tokio::task::JoinHandle<()>, +} +impl Drop for Fixture { + fn drop(&mut self) { + self.task.abort(); + } +} +impl Fixture { + async fn new(document: Value) -> Self { + let body = Arc::new(RwLock::new((StatusCode::OK, serde_json::to_vec(&document).unwrap()))); + let response = Arc::clone(&body); + let requests = Arc::new(AtomicUsize::new(0)); + let counter = Arc::clone(&requests); + let delay_ms = Arc::new(AtomicUsize::new(0)); + let delay = Arc::clone(&delay_ms); + let app = Router::new().route( + "/", + get(move || { + counter.fetch_add(1, Ordering::SeqCst); + let response = Arc::clone(&response); + let delay = delay.load(Ordering::SeqCst); + async move { + tokio::time::sleep(Duration::from_millis(delay as u64)).await; + response.read().await.clone() + } + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = format!("http://{}/", listener.local_addr().unwrap()).parse().unwrap(); + let task = tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + let verifier = Jwks::new(reqwest::Client::new(), url, Jwks::validation(&JwksConfig::default()).unwrap()); + Self { verifier, body, requests, delay_ms, task } + } + async fn allow_refresh(&self) { + self.verifier.refresh.lock().await.last_attempt = None; + } + async fn expire(&self) { + self.verifier.cache.write().await.as_mut().unwrap().expires_at = Instant::now(); + self.allow_refresh().await; + } + async fn verify(&self, kid: &str) -> Result { + let (token, header) = token(kid); + self.verifier.validate(&token, &header).await + } +} +fn key() -> EncodingKey { + EncodingKey::from_rsa_pem(include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/../../assets/jwt.key"))).unwrap() +} +fn document(kid: &str) -> Value { + let mut key = Jwk::from_encoding_key(&key(), Algorithm::RS256).unwrap(); + key.common.key_id = Some(kid.into()); + json!({"keys":[key]}) +} +fn token(kid: &str) -> (String, Header) { + let mut header = Header::new(Algorithm::RS256); + header.kid = Some(kid.into()); + let claims = json!({"iss":"mcpgateway","aud":"mcpgateway-api","exp":jsonwebtoken::get_current_timestamp()+3600}); + (encode(&header, &claims, &key()).unwrap(), header) +} + +#[tokio::test] +async fn concurrent_cold_requests_and_unknown_keys_have_bounded_refreshes() { + let fixture = Fixture::new(document("one")).await; + let results = futures::future::join_all((0..32).map(|_| fixture.verify("one"))).await; + assert!(results.into_iter().all(|result| result.is_ok())); + assert_eq!(fixture.requests.load(Ordering::SeqCst), 1); + fixture.allow_refresh().await; + let results = futures::future::join_all((0..32).map(|_| fixture.verify("unknown"))).await; + assert!(results.into_iter().all(|result| result == Err(AuthenticationError::InvalidToken))); + assert_eq!(fixture.requests.load(Ordering::SeqCst), 2); + assert!(fixture.verify("one").await.is_ok()); +} + +#[tokio::test] +async fn rotation_replaces_keys_and_expired_keys_fail_closed_during_outage() { + let fixture = Fixture::new(document("one")).await; + assert!(fixture.verify("one").await.is_ok()); + *fixture.body.write().await = (StatusCode::OK, serde_json::to_vec(&document("two")).unwrap()); + fixture.allow_refresh().await; + assert!(fixture.verify("two").await.is_ok()); + assert_eq!(fixture.verify("one").await, Err(AuthenticationError::InvalidToken)); + *fixture.body.write().await = (StatusCode::SERVICE_UNAVAILABLE, Vec::new()); + fixture.allow_refresh().await; + assert_eq!(fixture.verify("unknown").await, Err(AuthenticationError::KeysUnavailable)); + // Known, unexpired keys remain usable during a failed refresh for a different kid. + assert!(fixture.verify("two").await.is_ok()); + fixture.expire().await; + assert_eq!(fixture.verify("two").await, Err(AuthenticationError::KeysUnavailable)); + *fixture.body.write().await = (StatusCode::OK, serde_json::to_vec(&document("two")).unwrap()); + fixture.allow_refresh().await; + assert!(fixture.verify("two").await.is_ok()); +} + +#[tokio::test] +async fn rejects_unusable_key_sets_and_declared_algorithm_mismatch() { + let document = document("one"); + let mut duplicate = document.clone(); + duplicate["keys"].as_array_mut().unwrap().push(document["keys"][0].clone()); + let mut encryption_key = document.clone(); + encryption_key["keys"][0]["use"] = "enc".into(); + let mut non_verify = document.clone(); + non_verify["keys"][0]["key_ops"] = json!(["sign"]); + let mut no_kid = document.clone(); + no_kid["keys"][0].as_object_mut().unwrap().remove("kid"); + for document in [json!({"keys":[]}), duplicate, encryption_key, non_verify, no_kid] { + let fixture = Fixture::new(document).await; + assert_eq!(fixture.verify("one").await, Err(AuthenticationError::KeysUnavailable)); + } + let mut mismatch = document.clone(); + mismatch["keys"][0]["alg"] = "RS384".into(); + let fixture = Fixture::new(mismatch).await; + assert_eq!(fixture.verify("one").await, Err(AuthenticationError::InvalidToken)); +} + +#[tokio::test] +async fn bounded_response_and_malformed_json_are_unavailable() { + for body in [b"not-json".to_vec(), vec![b' '; JWKS_MAX_RESPONSE_BYTES + 1]] { + let fixture = Fixture::new(document("one")).await; + *fixture.body.write().await = (StatusCode::OK, body); + assert_eq!(fixture.verify("one").await, Err(AuthenticationError::KeysUnavailable)); + assert_eq!(fixture.verify("one").await, Err(AuthenticationError::KeysUnavailable)); + assert_eq!(fixture.requests.load(Ordering::SeqCst), 1); + } +} + +#[test] +fn rejects_incomplete_or_unsafe_trust_configuration() { + let config = JwksConfig::default(); + for config in [ + JwksConfig { issuer: String::new(), ..config.clone() }, + JwksConfig { audiences: vec![], ..config.clone() }, + JwksConfig { audiences: vec![String::new()], ..config.clone() }, + JwksConfig { algorithms: vec![], ..config.clone() }, + JwksConfig { algorithms: vec![Algorithm::HS256], ..config.clone() }, + JwksConfig { leeway_seconds: 301, ..config }, + ] { + assert!(Jwks::validation(&config).is_err()); + } +} + +#[tokio::test] +async fn cancelled_fetch_cannot_bypass_refresh_cooldown() { + let fixture = Fixture::new(document("one")).await; + fixture.delay_ms.store(5000, Ordering::SeqCst); + assert!(tokio::time::timeout(Duration::from_millis(100), fixture.verify("one")).await.is_err()); + assert_eq!(fixture.verify("one").await, Err(AuthenticationError::KeysUnavailable)); + assert_eq!(fixture.requests.load(Ordering::SeqCst), 1); +} diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs index dea3e91a..1fcc0d44 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs @@ -1,10 +1,11 @@ +use crate::JwksConfig; use crate::authorization::jwks::jwks::Jwks; -use crate::authorization::{AuthorizationClaims, AuthorizationError, AuthorizationService}; +use crate::authorization::{AuthenticationError, AuthorizationClaims, AuthorizationError, AuthorizationService}; use async_trait::async_trait; use jsonwebtoken::decode_header; use std::fmt; use std::net::IpAddr; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::time::Duration; use tracing::instrument; use url::Url; @@ -18,8 +19,9 @@ pub struct JwtAuthorizationService { } impl JwtAuthorizationService { - pub fn from_jwks_url(jwks_url: Url, ca_cert_path: Option<&PathBuf>) -> Result { - let url = parse_jwks_url(jwks_url)?; + pub fn new(config: &JwksConfig) -> Result { + let validation = Jwks::validation(config)?; + let url = parse_jwks_url(config.url.clone())?; let mut client = reqwest::Client::builder() .tls_backend_rustls() .connect_timeout(JWKS_CONNECT_TIMEOUT) @@ -27,15 +29,15 @@ impl JwtAuthorizationService { .timeout(JWKS_REQUEST_TIMEOUT) .redirect(reqwest::redirect::Policy::none()) .user_agent(concat!("mcp-ops/", env!("CARGO_PKG_VERSION"))); - if let Some(ca_cert_path) = ca_cert_path { + if let Some(ca_cert_path) = &config.ca_cert_path { client = client.tls_certs_only(load_ca_certificates(ca_cert_path)?); } let client = client.build().map_err(AuthorizationError::JwksRequest)?; - Ok(Self { jwks: Jwks::builder().client(client).url(url).build() }) + Ok(Self { jwks: Jwks::new(client, url, validation) }) } - async fn authorize_token(&self, token: &str) -> Option { - let header = decode_header(token).ok()?; + async fn authorize_token(&self, token: &str) -> Result { + let header = decode_header(token).map_err(|_| AuthenticationError::InvalidToken)?; self.jwks.validate(token, &header).await } } @@ -52,16 +54,17 @@ impl fmt::Debug for JwtAuthorizationService { #[async_trait] impl AuthorizationService for JwtAuthorizationService { #[instrument(name = "jwt_authorization_service", level = "info", skip_all)] - async fn authorize(&self, authorization_token: &http::HeaderValue) -> Option { - let token = authorization_token.as_bytes().strip_prefix(b"Bearer ")?; - let token = str::from_utf8(token).ok()?; - let claims = self.authorize_token(token).await; - - if claims.is_none() { - tracing::debug!("validate_saas_jwt SaaS JWT was rejected"); + async fn authorize( + &self, + authorization_token: &http::HeaderValue, + ) -> Result { + let value = authorization_token.to_str().map_err(|_| AuthenticationError::InvalidToken)?; + let (scheme, token) = value.split_once(' ').ok_or(AuthenticationError::InvalidToken)?; + if !scheme.eq_ignore_ascii_case("Bearer") || token.is_empty() || token.bytes().any(|b| b.is_ascii_whitespace()) + { + return Err(AuthenticationError::InvalidToken); } - - claims + self.authorize_token(token).await } } @@ -69,9 +72,10 @@ fn parse_jwks_url(url: Url) -> Result { let secure = url.scheme() == "https"; let local_http = url.scheme() == "http" && url.host_str().is_some_and(|host| { - host.eq_ignore_ascii_case("localhost") || host.parse::().is_ok_and(|address| address.is_loopback()) + host.eq_ignore_ascii_case("localhost") + || host.trim_matches(['[', ']']).parse::().is_ok_and(|address| address.is_loopback()) }); - if !secure && !local_http { + if (!secure && !local_http) || !url.username().is_empty() || url.password().is_some() || url.fragment().is_some() { return Err(AuthorizationError::InsecureJwksUrl); } Ok(url) @@ -89,313 +93,22 @@ fn load_ca_certificates(path: &Path) -> Result, Author } #[cfg(test)] -mod test { - use crate::{ - Config, - authorization::{AuthorizationClaims, Scopes}, - common::ContextForgeDataPlaneAppState, - config_stores::ConfigStoreError, - layers::claims_id::claims_layer, - }; - use crate::{ - authorization::{ - AuthorizationError, - jwks::{ - JwtAuthorizationService, - jwks::{JWKS_CACHE_KEY, Jwks, VerificationKey}, - jwks_authorization::{JWKS_CONNECT_TIMEOUT, JWKS_READ_TIMEOUT, JWKS_REQUEST_TIMEOUT}, - }, - }, - config_stores::ConfigStore, - }; - use async_trait::async_trait; - use axum::{Router, body::Body, middleware, response::Response, routing::get}; - - use contextforge_data_plane_apis::{User, user_store::UserConfig}; - use http::{HeaderMap, Request, StatusCode}; - use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, Header, encode}; - use lru_time_cache::LruCache; - use serde_json::json; - - use std::sync::{Arc, Once}; - use std::{str::FromStr, time::Duration}; - use tokio::sync::RwLock; - use tower::ServiceExt; - - use url::Url; - use uuid::Uuid; - - const GATEWAY_AUDIENCE: &str = "audience"; - const GATEWAY_ISSUER: &str = "issuer"; - - impl VerificationKey { - pub fn new(id: Option, decoding_key: DecodingKey) -> Self { - Self { key_id: id, decoding_key } - } - } - - impl AuthorizationClaims { - fn clear(&mut self, name: &str) { - if let Some(value) = self.value.get_mut(name) { - *value = serde_json::Value::Null; - } - } - - fn set(&mut self, name: &str, new_value: serde_json::Value) { - if let Some(value) = self.value.get_mut(name) { - *value = new_value; - } - } - fn get(&mut self, name: &str) -> Option<&serde_json::Value> { - self.value.get(name) - } - } - - impl JwtAuthorizationService { - pub async fn from_keys(verification_keys: Vec) -> Result { - let url: Url = Url::from_str("http://127.0.0.1:0/").expect("this should work"); - let client = reqwest::Client::builder() - .tls_backend_rustls() - .connect_timeout(JWKS_CONNECT_TIMEOUT) - .read_timeout(JWKS_READ_TIMEOUT) - .timeout(JWKS_REQUEST_TIMEOUT) - .redirect(reqwest::redirect::Policy::none()) - .user_agent(concat!("mcp-ops/", env!("CARGO_PKG_VERSION"))); - - let client = client.build().map_err(AuthorizationError::JwksRequest)?; - - let cache = RwLock::new(LruCache::with_expiry_duration(Duration::from_hours(100))); - let mut guard = cache.write().await; - guard.insert(JWKS_CACHE_KEY.to_owned(), verification_keys); - drop(guard); - - Ok(Self { jwks: Jwks::builder().cache(cache).client(client).url(url).build() }) - } - } - - static CRYPTO: Once = Once::new(); - const HMAC_SECRET: &[u8] = b"my-test-key-but-now-longer-than-32-bytes"; - - fn now_epoch_seconds() -> u64 { - std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).expect("Time went backwards").as_secs() - } - - fn active_test_claims() -> AuthorizationClaims { - let now = now_epoch_seconds(); - let user_id = "11111111-1111-1111-1111-111111111111".to_owned(); - - let map = json!( { - "iss": GATEWAY_ISSUER.to_owned(), - "sub": user_id.clone(), - "aud": GATEWAY_AUDIENCE.to_owned(), - "exp": now + Duration::from_hours(1).as_secs(), - "nbf": now - Duration::from_mins(1).as_secs(), - "iat": now, - "jti": Uuid::new_v4().to_string(), - "token_use": Some("api".to_owned()), - "teams": vec!["team_awesome".to_owned()], - "user": crate::authorization::User::builder() - .tenant_id("team_awesome".to_owned()) - .user_id(user_id.clone()) - .build(), - "scopes": Scopes::builder() - .server_id(Some("my_id".to_owned())) - .ip_restrictions(vec!["192.169.1.0/24".to_owned()]) - .permissions(vec!["tools.read".to_owned(), "servers.use".to_owned()]) - .time_restrictions(None) - .build(), - "tenant_id": "tenant".to_owned(), - }); - AuthorizationClaims::from(map) - } - - fn get_hmac_token_for_claims(claims: &AuthorizationClaims) -> String { - let key = EncodingKey::from_secret(HMAC_SECRET); - let header = Header::new(Algorithm::HS256); - let claims = claims.value.clone(); - encode::(&header, &claims, &key).expect("Expecting this to work") - } - - #[derive(Debug, Clone)] - struct MockedUserConfigStore; - #[async_trait] - impl ConfigStore for MockedUserConfigStore { - async fn get_config<'a>(&self, _: &'a User) -> Result { - Err(ConfigStoreError::InvalidConnection) - } - - async fn set_config<'a>(&self, _: &'a User, _: &'a UserConfig) -> Result<(), ConfigStoreError> { - Err(ConfigStoreError::InvalidConnection) - } - } - +mod tests { + use super::*; #[test] - fn test_active_token() { - let mut claims = active_test_claims(); - assert_ne!(claims.get("exp").and_then(serde_json::Value::as_i64), Some(0_i64)); - claims.set("exp", 0.into()); - assert_eq!(claims.get("exp").and_then(serde_json::Value::as_i64), Some(0_i64)); - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - #[allow(clippy::items_after_statements)] - #[test_log::test] - async fn claim_test_valid_hmac() { - CRYPTO.call_once(|| { - _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); - }); - - async fn handle(_: HeaderMap) -> Response { - Response::builder().status(StatusCode::OK).body(Body::empty()).expect("Expecting this to work") + fn only_trusted_transport_urls_are_accepted() { + for url in + ["https://issuer.example/keys", "http://localhost/keys", "http://127.0.0.1/keys", "http://[::1]/keys"] + { + assert!(parse_jwks_url(url.parse().unwrap()).is_ok(), "{url}"); } - - let token = get_hmac_token_for_claims(&active_test_claims()); - - let decoding_key = DecodingKey::from_secret(HMAC_SECRET); - let verfication_key = VerificationKey::new(Some("HS256".to_owned()), decoding_key); - - let state = ContextForgeDataPlaneAppState { - authorization_service: Arc::new( - JwtAuthorizationService::from_keys(vec![verfication_key]).await.expect("this should work"), - ), - config_store: Arc::new(MockedUserConfigStore {}), - config: Config::default(), - }; - let http_requst = Request::builder() - .header("Authorization", format!("Bearer {token}")) - .method("GET") - .body(Body::empty()) - .expect("This should work"); - - let app = - Router::new().route("/", get(handle)).layer(middleware::from_fn_with_state(state.clone(), claims_layer)); - - let res = app.oneshot(http_requst).await.unwrap(); - assert_eq!(res.status(), StatusCode::OK); - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - #[allow(clippy::items_after_statements)] - async fn claim_test_missing_scopes_is_allowed() { - CRYPTO.call_once(|| { - _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); - }); - - async fn handle(_: HeaderMap) -> Response { - Response::builder().status(StatusCode::OK).body(Body::empty()).expect("Expecting this to work") + for url in [ + "http://issuer.example/keys", + "file:///keys", + "https://user:secret@issuer.example/keys", + "https://issuer.example/keys#fragment", + ] { + assert!(parse_jwks_url(url.parse().unwrap()).is_err(), "{url}"); } - - let mut claims = active_test_claims(); - claims.clear("scopes"); - let token = get_hmac_token_for_claims(&claims); - - let decoding_key = DecodingKey::from_secret(HMAC_SECRET); - let verfication_key = VerificationKey::new(Some("HS256".to_owned()), decoding_key); - let state = ContextForgeDataPlaneAppState { - authorization_service: Arc::new( - JwtAuthorizationService::from_keys(vec![verfication_key]).await.expect("this should work"), - ), - config_store: Arc::new(MockedUserConfigStore {}), - config: Config::default(), - }; - let http_requst = Request::builder() - .header("Authorization", format!("Bearer {token}")) - .method("GET") - .body(Body::empty()) - .expect("This should work"); - - let app = - Router::new().route("/", get(handle)).layer(middleware::from_fn_with_state(state.clone(), claims_layer)); - - let res = app.oneshot(http_requst).await.unwrap(); - assert_eq!(res.status(), StatusCode::OK); - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - #[allow(clippy::items_after_statements)] - async fn claim_test_missing_token_use_and_full_name_is_allowed() { - CRYPTO.call_once(|| { - _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); - }); - let user_id = "11111111-1111-1111-1111-111111111111".to_owned(); - - async fn handle(_: HeaderMap) -> Response { - Response::builder().status(StatusCode::OK).body(Body::empty()).expect("Expecting this to work") - } - - let mut claims = active_test_claims(); - claims.clear("token_use"); - claims.set( - "user", - serde_json::to_value( - crate::authorization::User::builder() - .tenant_id("team_awesome".to_owned()) - .user_id(user_id.clone()) - .build(), - ) - .expect("should work"), - ); - - let token = get_hmac_token_for_claims(&claims); - - let decoding_key = DecodingKey::from_secret(HMAC_SECRET); - let verfication_key = VerificationKey::new(Some("HS256".to_owned()), decoding_key); - - let state = ContextForgeDataPlaneAppState { - authorization_service: Arc::new( - JwtAuthorizationService::from_keys(vec![verfication_key]).await.expect("this should work"), - ), - config_store: Arc::new(MockedUserConfigStore {}), - config: Config::default(), - }; - let http_requst = Request::builder() - .header("Authorization", format!("Bearer {token}")) - .method("GET") - .body(Body::empty()) - .expect("This should work"); - - let app = - Router::new().route("/", get(handle)).layer(middleware::from_fn_with_state(state.clone(), claims_layer)); - - let res = app.oneshot(http_requst).await.unwrap(); - assert_eq!(res.status(), StatusCode::OK); - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - #[allow(clippy::items_after_statements)] - async fn claim_test_expired_token() { - CRYPTO.call_once(|| { - _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); - }); - - async fn handle(_: HeaderMap) -> Response { - Response::builder().status(StatusCode::OK).body(Body::empty()).expect("Expecting this to work") - } - - let mut claims = active_test_claims(); - claims.set("exp", 1000.into()); - let token = get_hmac_token_for_claims(&claims); - - let decoding_key = DecodingKey::from_secret(HMAC_SECRET); - let verfication_key = VerificationKey::new(Some("HS256".to_owned()), decoding_key); - - let state = ContextForgeDataPlaneAppState { - authorization_service: Arc::new( - JwtAuthorizationService::from_keys(vec![verfication_key]).await.expect("this should work"), - ), - config_store: Arc::new(MockedUserConfigStore {}), - config: Config::default(), - }; - let http_requst = Request::builder() - .header("Authorization", format!("Bearer {token}")) - .method("GET") - .body(Body::empty()) - .expect("This should work"); - - let app = - Router::new().route("/", get(handle)).layer(middleware::from_fn_with_state(state.clone(), claims_layer)); - - let res = app.oneshot(http_requst).await.unwrap(); - assert_eq!(res.status(), StatusCode::UNAUTHORIZED); } } diff --git a/crates/contextforge-data-plane-lib/src/authorization/mod.rs b/crates/contextforge-data-plane-lib/src/authorization/mod.rs index 49c95a67..61c478c8 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/mod.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/mod.rs @@ -12,24 +12,39 @@ mod jwks; mod principal_extractor; pub use principal_extractor::{ - AuthorizedPrincipal, CelPrincipalExtractor, DefaultPrincipalExtractor, PrincipalExtractor, + AuthorizedPrincipal, CelPrincipalExtractor, DefaultPrincipalExtractor, Permission, PrincipalConfig, + PrincipalExtractor, ScopeMapping, UserClaim, }; pub fn get_authorization_service( config: &JwksConfig, ) -> Result, AuthorizationError> { - let service = jwks::JwtAuthorizationService::from_jwks_url(config.url.clone(), config.ca_cert_path.as_ref())?; + let service = jwks::JwtAuthorizationService::new(config)?; Ok(Arc::new(service) as Arc) } #[async_trait] pub trait AuthorizationService: std::fmt::Debug { - async fn authorize(&self, authorization_token: &HeaderValue) -> Option; + async fn authorize(&self, authorization_token: &HeaderValue) -> Result; +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] +pub enum AuthenticationError { + #[error("invalid bearer token")] + InvalidToken, + #[error("verification keys unavailable")] + KeysUnavailable, } #[derive(Debug, thiserror::Error)] #[allow(dead_code)] pub enum AuthorizationError { + #[error( + "JWT trust configuration requires a nonempty issuer, audiences, and RSA/EC algorithms; leeway must be at most 300 seconds" + )] + InvalidTrustConfiguration, + #[error("JWKS contains duplicate signing key IDs")] + DuplicateKeyId, #[error("SaaS JWKS contains no supported signing keys")] NoSupportedKeys, #[error("SaaS JWKS is invalid")] @@ -37,7 +52,7 @@ pub enum AuthorizationError { #[error("SaaS JWKS is invalid")] InvalidKey(#[source] jsonwebtoken::errors::Error), - #[error("MCPOPS_JWKS_URL must use HTTPS (HTTP is allowed only for loopback testing)")] + #[error("JWKS URL must use HTTPS (HTTP is allowed only for loopback testing), without credentials or fragments")] InsecureJwksUrl, #[error("unable to retrieve SaaS JWKS")] JwksRequest(#[source] reqwest::Error), @@ -80,12 +95,24 @@ pub struct Idp { iss: String, } -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, TypedBuilder)] +#[derive(Clone, Serialize, Deserialize, PartialEq, TypedBuilder)] #[serde(rename_all = "camelCase")] pub struct AuthorizationClaims { value: serde_json::Value, } +impl AuthorizationClaims { + pub fn as_value(&self) -> &serde_json::Value { + &self.value + } +} + +impl std::fmt::Debug for AuthorizationClaims { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("AuthorizationClaims").finish_non_exhaustive() + } +} + impl From for AuthorizationClaims { fn from(value: serde_json::Value) -> Self { Self { value } diff --git a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/cel_principal_extractor.rs b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/cel_principal_extractor.rs index 6c56ea46..bea2a5f0 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/cel_principal_extractor.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/cel_principal_extractor.rs @@ -5,9 +5,9 @@ use std::sync::Arc; use cel::{Context, Program, objects::Key}; use serde_json::Value as JsonValue; use thiserror::Error; -use tracing::{debug, instrument}; +use tracing::instrument; -use crate::authorization::{AuthorizedPrincipal, PrincipalExtractor}; +use super::{AuthorizedPrincipal, PrincipalConfig, PrincipalExtractor, UserClaim}; #[derive(Error, Debug)] pub enum CelPrincipalExtractorError { @@ -22,41 +22,25 @@ pub enum CelPrincipalExtractorError { #[error("CEL expression did not return a map: {0:?}")] InvalidReturnType(JsonValue), - - #[error("Missing required field in CEL result: {0}")] - MissingRequiredField(String), - - #[error("Invalid field type in CEL result: field={0}, expected={1}")] - InvalidFieldType(String, String), } -/// A CEL-based principal extractor that evaluates a CEL expression to extract -/// principal information from authorization claims. -/// -/// The CEL expression should return a map with the following fields: -/// - `user_id` (string, required): The user identifier -/// - `tenant_id` (string, required): The tenant identifier -/// - `scopes` (list of strings, optional): The user's scopes/permissions -/// -/// The CEL expression has access to the following variables: -/// - `claims`: A map containing all the authorization claims -/// - `sub`: The subject claim (shorthand for claims.sub) -/// - `tenant_id`: The tenant_id claim (shorthand for claims.tenant_id) -/// -/// Example CEL expression: -/// ```cel -/// { -/// "user_id": claims.sub, -/// "tenant_id": claims.tenant_id, -/// "scopes": [] -/// } -/// ``` +/// Trusted CEL mapping for nonstandard claims. Return `user_id`, `tenant_id`, +/// and optional `role`, `roles`, `scope`, `permissions`. Permissions are always +/// computed by the same policy as the default extractor. The verified issuer +/// is taken from the original claims and cannot be overridden by CEL. #[derive(Clone, Debug)] pub struct CelPrincipalExtractor { program: Arc, + config: PrincipalConfig, } impl CelPrincipalExtractor { + pub fn with_config(mut self, mut config: PrincipalConfig) -> Self { + config.user_claim = UserClaim::Sub; + self.config = config; + self + } + pub fn from_file>(path: P) -> Result { let expression = fs::read_to_string(path)?; Self::from_expression(&expression) @@ -66,7 +50,7 @@ impl CelPrincipalExtractor { let program = Program::compile(expression).map_err(|e| CelPrincipalExtractorError::CompilationError(e.to_string()))?; - Ok(Self { program: Arc::new(program) }) + Ok(Self { program: Arc::new(program), config: PrincipalConfig::default() }) } } @@ -85,30 +69,33 @@ impl PrincipalExtractor for CelPrincipalExtractor { let result = self.program.execute(&context).map_err(|e| CelPrincipalExtractorError::EvaluationError(e.to_string()))?; - debug!("CEL expression evaluated to: {:?}", result); - - Ok(AuthorizedPrincipal::try_from(result)?) - } -} - -impl TryFrom for AuthorizedPrincipal { - type Error = CelPrincipalExtractorError; - - fn try_from(value: cel::Value) -> Result { - match value { - cel::Value::Map(map) => { - if let Some(cel::Value::String(user_id)) = map.get(&Key::from("user_id".to_owned())) - && let Some(cel::Value::String(tenant_id)) = map.get(&Key::from("tenant_id".to_owned())) - { - let user_id = (**user_id).clone(); - let tenant_id = (**tenant_id).clone(); - Ok(AuthorizedPrincipal::builder().user_id(user_id).tenant_id(tenant_id).scopes(vec![]).build()) - } else { - Err(CelPrincipalExtractorError::InvalidReturnType(serde_json::Value::Null)) - } - }, - _ => Err(CelPrincipalExtractorError::InvalidReturnType(serde_json::Value::Null)), + let cel::Value::Map(map) = result else { + return Err(CelPrincipalExtractorError::InvalidReturnType(JsonValue::Null).into()); + }; + let mut normalized = serde_json::Map::new(); + for name in ["user_id", "tenant_id", "role", "roles", "scope", "permissions"] { + if let Some(value) = map.get(&Key::from(name.to_owned())) { + let value = match value { + cel::Value::String(value) => JsonValue::String(value.to_string()), + cel::Value::List(values) => JsonValue::Array( + values + .iter() + .map(|value| { + if let cel::Value::String(value) = value { + JsonValue::String(value.to_string()) + } else { + JsonValue::Null + } + }) + .collect(), + ), + _ => JsonValue::Null, + }; + normalized.insert(if name == "user_id" { "sub" } else { name }.to_owned(), value); + } } + normalized.insert("iss".to_owned(), claims.get("iss").cloned().unwrap_or(JsonValue::Null)); + Ok(AuthorizedPrincipal::from_claims(&JsonValue::Object(normalized), &self.config)?) } } @@ -178,3 +165,26 @@ mod tests { assert!(result.is_err()); } } + +#[cfg(test)] +mod permission_tests { + use super::*; + use crate::Permission; + + #[test] + fn nested_roles_use_common_policy_and_cannot_override_issuer() { + let extractor = CelPrincipalExtractor::from_expression(r#"{"user_id": claims.sub, "tenant_id": claims.woTenantId, "roles": claims.user.roles, "iss": "untrusted"}"#).unwrap(); + let principal = extractor + .extract(&serde_json::json!({"sub":"user","woTenantId":"tenant","iss":"watson","user":{"roles":["admin"]}})) + .unwrap(); + assert_eq!(principal.issuer(), "watson"); + assert!(principal.has_permission(Permission::Admin)); + let extractor = + CelPrincipalExtractor::from_expression(r#"{"user_id": claims.sub, "tenant_id": claims.woTenantId}"#) + .unwrap(); + let principal = extractor + .extract(&serde_json::json!({"sub":"user","woTenantId":"tenant","iss":"watson","role":"admin"})) + .unwrap(); + assert!(!principal.has_permission(Permission::MCPUser)); + } +} diff --git a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/default_principal_extractor.rs b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/default_principal_extractor.rs index 7cae3c4d..8b7e0c4c 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/default_principal_extractor.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/default_principal_extractor.rs @@ -1,9 +1,16 @@ +use super::{AuthorizedPrincipal, PrincipalConfig, PrincipalExtractor}; use tracing::instrument; -use crate::authorization::{AuthorizedPrincipal, PrincipalExtractor}; +#[derive(Debug, Clone, Default)] +pub struct DefaultPrincipalExtractor { + config: PrincipalConfig, +} -#[derive(Debug, Clone)] -pub struct DefaultPrincipalExtractor {} +impl DefaultPrincipalExtractor { + pub fn new(config: PrincipalConfig) -> Self { + Self { config } + } +} impl PrincipalExtractor for DefaultPrincipalExtractor { #[instrument(name = "principal_extract", level = "info", skip_all)] @@ -11,17 +18,121 @@ impl PrincipalExtractor for DefaultPrincipalExtractor { &self, claims: &serde_json::Value, ) -> Result> { - let user_id = - ["sub", "user_id", "UserId"].into_iter().find_map(|claim| claims.get(claim)).and_then(|v| v.as_str()); - let tenant_id = - ["tenantId", "tenant_id"].into_iter().find_map(|claim| claims.get(claim)).and_then(|v| v.as_str()); - match (user_id, tenant_id) { - (Some(user_id), Some(tenant_id)) => Ok(AuthorizedPrincipal::builder() - .user_id(user_id.to_owned()) - .tenant_id(tenant_id.to_owned()) - .scopes(vec![]) - .build()), - _ => Err("Can't create principal".into()), + Ok(AuthorizedPrincipal::from_claims(claims, &self.config)?) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::authorization::{Permission, ScopeMapping, UserClaim}; + use serde_json::{Value, json}; + + fn claims() -> Value { + json!({"iss":"watson", "sub":"subject", "woUserId":"watson-user", "woTenantId":"tenant", "role":"user"}) + } + + #[test] + fn role_permissions_are_explicit_and_admin_includes_mcp() { + for (role, admin, mcp) in [ + ("admin", true, true), + ("builder", false, true), + ("user", false, true), + ("unknown", false, false), + ("Admin", false, false), + ("", false, false), + ] { + let mut claims = claims(); + claims["role"] = role.into(); + let principal = DefaultPrincipalExtractor::default().extract(&claims).unwrap(); + assert_eq!(principal.has_permission(Permission::Admin), admin, "{role}"); + assert_eq!(principal.has_permission(Permission::MCPUser), mcp, "{role}"); + assert_eq!( + (principal.issuer(), principal.user_id(), principal.tenant_id()), + ("watson", "subject", "tenant") + ); + } + let mut claims = claims(); + claims.as_object_mut().unwrap().remove("role"); + assert!(!DefaultPrincipalExtractor::default().extract(&claims).unwrap().has_permission(Permission::MCPUser)); + claims["roles"] = json!(["unknown", "admin"]); + assert!(DefaultPrincipalExtractor::default().extract(&claims).unwrap().has_permission(Permission::Admin)); + } + + #[test] + fn identity_profile_and_tenant_aliases_are_strict() { + let mut claims = claims(); + let extractor = + DefaultPrincipalExtractor::new(PrincipalConfig { user_claim: UserClaim::WoUserId, ..Default::default() }); + assert_eq!(extractor.extract(&claims).unwrap().user_id(), "watson-user"); + claims["tenant_id"] = "tenant".into(); + assert!(extractor.extract(&claims).is_ok()); + claims["tenantId"] = "different".into(); + assert!(extractor.extract(&claims).is_err()); + for invalid in [Value::Null, json!(42), json!(""), json!(" ")] { + for claim in ["sub", "woTenantId", "iss"] { + let mut claims = super::tests::claims(); + claims[claim] = invalid.clone(); + assert!(DefaultPrincipalExtractor::default().extract(&claims).is_err(), "{claim}"); + } + } + for tenant in ["woTenantId", "tenant_id", "tenantId"] { + let claims = json!({"iss":"watson", "sub":"subject", tenant:"tenant", "role":"user"}); + assert!(DefaultPrincipalExtractor::default().extract(&claims).is_ok()); + } + } + + #[test] + fn scopes_only_grant_permissions_when_configured() { + let mut claims = claims(); + claims["role"] = "unknown".into(); + claims["scope"] = "aipg.admin unrelated".into(); + assert!(!DefaultPrincipalExtractor::default().extract(&claims).unwrap().has_permission(Permission::Admin)); + let mapping = ScopeMapping { admin: vec!["aipg.admin".into()], mcp_user: vec!["aipg.mcp".into()] }; + let mut config = PrincipalConfig { scope_mapping: Some(mapping), ..Default::default() }; + assert!( + !DefaultPrincipalExtractor::new(config.clone()).extract(&claims).unwrap().has_permission(Permission::Admin) + ); + config.scopes_only = true; + assert!( + DefaultPrincipalExtractor::new(config.clone()).extract(&claims).unwrap().has_permission(Permission::Admin) + ); + claims["scope"] = "".into(); + claims["permissions"] = json!(["aipg.mcp"]); + let principal = DefaultPrincipalExtractor::new(config.clone()).extract(&claims).unwrap(); + assert!(!principal.has_permission(Permission::Admin)); + assert!(principal.has_permission(Permission::MCPUser)); + assert_eq!(principal.scopes(), &["aipg.mcp"]); + config.scopes_only = false; + claims["role"] = "admin".into(); + assert!( + !DefaultPrincipalExtractor::new(config.clone()).extract(&claims).unwrap().has_permission(Permission::Admin) + ); + claims["permissions"] = json!([]); + assert!(!DefaultPrincipalExtractor::new(config).extract(&claims).unwrap().has_permission(Permission::MCPUser)); + } + + #[test] + fn malformed_permission_claims_are_rejected() { + for (name, value) in [ + ("role", json!(["admin"])), + ("roles", json!("admin")), + ("roles", json!([42])), + ("scope", json!(["aipg.admin"])), + ("permissions", json!({"admin":true})), + ] { + let mut claims = claims(); + claims[name] = value; + assert!(DefaultPrincipalExtractor::default().extract(&claims).is_err(), "{name}"); + } + } + + #[test] + fn debug_does_not_disclose_identity_or_scopes() { + let principal = DefaultPrincipalExtractor::default().extract(&claims()).unwrap(); + let debug = format!("{principal:?}"); + for sensitive in ["watson", "subject", "tenant"] { + assert!(!debug.contains(sensitive)); } } } diff --git a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/mod.rs b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/mod.rs index 44742246..78b88d0d 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/mod.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/mod.rs @@ -1,28 +1,203 @@ mod cel_principal_extractor; mod default_principal_extractor; + +use std::collections::BTreeSet; + +use clap::ValueEnum; use contextforge_data_plane_apis::User; -use typed_builder::TypedBuilder; +use serde_json::Value; pub use cel_principal_extractor::CelPrincipalExtractor; pub use default_principal_extractor::DefaultPrincipalExtractor; -#[derive(Debug, Clone, TypedBuilder)] -#[allow(dead_code)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum Permission { + Admin, + MCPUser, +} + +#[derive(Debug, Clone, Copy, Default, ValueEnum)] +pub enum UserClaim { + #[default] + Sub, + WoUserId, +} + +impl UserClaim { + pub fn as_str(self) -> &'static str { + match self { + Self::Sub => "sub", + Self::WoUserId => "woUserId", + } + } +} + +/// Scope names are an explicit deployment contract, not inferred from arbitrary scopes. +#[derive(Debug, Clone, Default)] +pub struct ScopeMapping { + pub admin: Vec, + pub mcp_user: Vec, +} + +#[derive(Debug, Clone, Default)] +pub struct PrincipalConfig { + pub user_claim: UserClaim, + /// When configured, intersect role permissions with the mapped scope permissions. + pub scope_mapping: Option, + /// Explicit opt-in to granting permissions using scopes without roles. + pub scopes_only: bool, +} + +impl PrincipalConfig { + pub fn validate(&self) -> Result<(), PrincipalError> { + if self.scopes_only && self.scope_mapping.is_none() + || self.scope_mapping.as_ref().is_some_and(|mapping| { + mapping + .admin + .iter() + .chain(&mapping.mcp_user) + .any(|scope| scope.is_empty() || scope.chars().any(char::is_whitespace)) + }) + { + return Err(PrincipalError::InvalidConfiguration); + } + Ok(()) + } +} + +#[derive(Debug, thiserror::Error)] +pub enum PrincipalError { + #[error("missing or malformed identity claim")] + InvalidIdentity, + #[error("conflicting tenant claims")] + ConflictingTenant, + #[error("malformed role or scope claims")] + InvalidPermissions, + #[error("invalid principal mapping configuration")] + InvalidConfiguration, +} + +/// Request-local identity derived only from verified claims and trusted mapping configuration. +/// This is not yet a tenant-aware persistent configuration key. +#[derive(Clone)] pub struct AuthorizedPrincipal { + issuer: String, user_id: String, tenant_id: String, scopes: Vec, + permissions: BTreeSet, +} + +impl std::fmt::Debug for AuthorizedPrincipal { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("AuthorizedPrincipal").field("permissions", &self.permissions).finish_non_exhaustive() + } +} + +impl AuthorizedPrincipal { + pub fn issuer(&self) -> &str { + &self.issuer + } + pub fn user_id(&self) -> &str { + &self.user_id + } + pub fn tenant_id(&self) -> &str { + &self.tenant_id + } + pub fn scopes(&self) -> &[String] { + &self.scopes + } + pub fn has_permission(&self, permission: Permission) -> bool { + self.permissions.contains(&permission) + } + + fn from_claims(claims: &Value, config: &PrincipalConfig) -> Result { + let issuer = identity_string(claims.get("iss"))?; + let user_id = identity_string(claims.get(config.user_claim.as_str()))?; + let mut tenant_id = None; + for name in ["woTenantId", "tenant_id", "tenantId"] { + if let Some(value) = claims.get(name) { + let value = identity_string(Some(value))?; + if tenant_id.is_some_and(|tenant| tenant != value) { + return Err(PrincipalError::ConflictingTenant); + } + tenant_id = Some(value); + } + } + let tenant_id = tenant_id.ok_or(PrincipalError::InvalidIdentity)?; + let mut roles = string_list(claims.get("roles"))?; + if let Some(role) = claims.get("role") { + roles.push(role.as_str().ok_or(PrincipalError::InvalidPermissions)?.to_owned()); + } + let mut scopes = string_list(claims.get("permissions"))?; + if let Some(scope) = claims.get("scope") { + scopes.extend( + scope.as_str().ok_or(PrincipalError::InvalidPermissions)?.split_ascii_whitespace().map(str::to_owned), + ); + } + scopes.sort_unstable(); + scopes.dedup(); + let mut permissions = BTreeSet::new(); + for role in roles { + match role.as_str() { + "admin" => { + permissions.extend([Permission::Admin, Permission::MCPUser]); + }, + "builder" | "user" => { + permissions.insert(Permission::MCPUser); + }, + _ => {}, + } + } + if let Some(mapping) = &config.scope_mapping { + let mut scope_permissions = BTreeSet::new(); + if mapping.admin.iter().any(|scope| scopes.contains(scope)) { + scope_permissions.extend([Permission::Admin, Permission::MCPUser]); + } + if mapping.mcp_user.iter().any(|scope| scopes.contains(scope)) { + scope_permissions.insert(Permission::MCPUser); + } + if config.scopes_only { + permissions = scope_permissions; + } else { + permissions.retain(|permission| scope_permissions.contains(permission)); + } + } else if config.scopes_only { + return Err(PrincipalError::InvalidConfiguration); + } + Ok(Self { + issuer: issuer.to_owned(), + user_id: user_id.to_owned(), + tenant_id: tenant_id.to_owned(), + scopes, + permissions, + }) + } +} + +fn identity_string(value: Option<&Value>) -> Result<&str, PrincipalError> { + value.and_then(Value::as_str).filter(|value| !value.trim().is_empty()).ok_or(PrincipalError::InvalidIdentity) +} + +fn string_list(value: Option<&Value>) -> Result, PrincipalError> { + match value { + None => Ok(Vec::new()), + Some(Value::Array(values)) => values + .iter() + .map(|value| value.as_str().map(str::to_owned).ok_or(PrincipalError::InvalidPermissions)) + .collect(), + _ => Err(PrincipalError::InvalidPermissions), + } } -impl<'a> From<&'a AuthorizedPrincipal> for User { - fn from(value: &'a AuthorizedPrincipal) -> Self { +impl From<&AuthorizedPrincipal> for User { + fn from(value: &AuthorizedPrincipal) -> Self { + // The publisher and ConfigStore still use subject-only keys. Tenant isolation + // requires a coordinated schema/publisher migration in the next change. Self::new(&value.user_id) } } pub trait PrincipalExtractor { - fn extract( - &self, - claims: &serde_json::Value, - ) -> Result>; + fn extract(&self, claims: &Value) -> Result>; } diff --git a/crates/contextforge-data-plane-lib/src/common/cli_config.rs b/crates/contextforge-data-plane-lib/src/common/cli_config.rs index 477b20af..91bdaff9 100644 --- a/crates/contextforge-data-plane-lib/src/common/cli_config.rs +++ b/crates/contextforge-data-plane-lib/src/common/cli_config.rs @@ -17,6 +17,34 @@ pub struct CliConfig { #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWKS_CA_PATH")] pub jwks_ca_cert_path: Option, + /// Exact trusted token issuer; never discovered from the incoming token. + #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_ISSUER")] + pub jwt_issuer: String, + + /// Accepted audiences (comma-separated); at least one must match. + #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES", value_delimiter = ',', required = true)] + pub jwt_audiences: Vec, + + #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_ALGORITHMS", value_delimiter = ',', default_value = "RS256")] + pub jwt_algorithms: Vec, + + #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_LEEWAY_SECONDS", default_value_t = 30)] + pub jwt_leeway_seconds: u64, + + #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_USER_CLAIM", value_enum, default_value = "sub")] + pub jwt_user_claim: crate::authorization::UserClaim, + + /// When either scope mapping is set, scopes restrict role-derived permissions. + #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_ADMIN_SCOPES", value_delimiter = ',')] + pub jwt_admin_scopes: Option>, + + #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_MCP_USER_SCOPES", value_delimiter = ',')] + pub jwt_mcp_user_scopes: Option>, + + /// Explicitly authorize using configured scope mappings instead of roles. + #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_SCOPES_ONLY", default_value_t = false)] + pub jwt_scopes_only: bool, + #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_ENABLE_OPEN_TELEMETRY")] pub enable_open_telemetry: Option, diff --git a/crates/contextforge-data-plane-lib/src/common/config.rs b/crates/contextforge-data-plane-lib/src/common/config.rs index 18e9059b..1ed30023 100644 --- a/crates/contextforge-data-plane-lib/src/common/config.rs +++ b/crates/contextforge-data-plane-lib/src/common/config.rs @@ -146,11 +146,22 @@ impl From<&CliConfig> for UpstreamTransportConfig { pub struct JwksConfig { pub url: url::Url, pub ca_cert_path: Option, + pub issuer: String, + pub audiences: Vec, + pub algorithms: Vec, + pub leeway_seconds: u64, } impl From<&CliConfig> for JwksConfig { fn from(value: &CliConfig) -> Self { let CliConfig { jwks_url, jwks_ca_cert_path, .. } = value.clone(); - Self { url: jwks_url, ca_cert_path: jwks_ca_cert_path } + Self { + url: jwks_url, + ca_cert_path: jwks_ca_cert_path, + issuer: value.jwt_issuer.clone(), + audiences: value.jwt_audiences.clone(), + algorithms: value.jwt_algorithms.clone(), + leeway_seconds: value.jwt_leeway_seconds, + } } } @@ -160,6 +171,7 @@ pub struct Config { pub observability_config: ObservabilityConfig, pub jwks_config: JwksConfig, + pub principal_config: crate::authorization::PrincipalConfig, /// Expiry in seconds for the in-process user config cache in front of /// Redis. The control-plane dataplane publisher rewrites UserConfig keys @@ -354,6 +366,10 @@ mod tests { fn observability_config_is_derived_from_cli_config() { let args = vec![ "contextforge-data-plane", + "--jwt-issuer", + "mcpgateway", + "--jwt-audiences", + "mcpgateway-api", "--jwks-url", "http://127.0.0.1:8080/", "--redis-address", @@ -400,7 +416,14 @@ mod tests { impl Default for super::JwksConfig { fn default() -> Self { - Self { url: "http://127.0.0.1:8080/".parse().expect("should work"), ca_cert_path: None } + Self { + url: "http://127.0.0.1:8080/".parse().expect("should work"), + ca_cert_path: None, + issuer: "mcpgateway".to_owned(), + audiences: vec!["mcpgateway-api".to_owned()], + algorithms: vec![jsonwebtoken::Algorithm::RS256], + leeway_seconds: 30, + } } } @@ -409,6 +432,7 @@ mod tests { Self { address: None, jwks_config: super::JwksConfig::default(), + principal_config: crate::authorization::PrincipalConfig::default(), observability_config: super::ObservabilityConfig::default(), mcp_standard_header_max_count: 10, mcp_standard_header_max_value_bytes: 4096, diff --git a/crates/contextforge-data-plane-lib/src/common/mod.rs b/crates/contextforge-data-plane-lib/src/common/mod.rs index 9deb9265..f2110452 100644 --- a/crates/contextforge-data-plane-lib/src/common/mod.rs +++ b/crates/contextforge-data-plane-lib/src/common/mod.rs @@ -64,6 +64,17 @@ impl TryFrom for Config { let downstream_transport_config = DownstreamTransportConfig::from(&value); let upstream_transport_config = UpstreamTransportConfig::from(&value); let jwks_config = JwksConfig::from(&value); + let principal_config = crate::authorization::PrincipalConfig { + user_claim: value.jwt_user_claim, + scopes_only: value.jwt_scopes_only, + scope_mapping: (value.jwt_admin_scopes.is_some() || value.jwt_mcp_user_scopes.is_some()).then(|| { + crate::authorization::ScopeMapping { + admin: value.jwt_admin_scopes.clone().unwrap_or_default(), + mcp_user: value.jwt_mcp_user_scopes.clone().unwrap_or_default(), + } + }), + }; + principal_config.validate()?; let CliConfig { address, runtime_plugins_enabled, @@ -76,6 +87,7 @@ impl TryFrom for Config { Ok(Self { address, jwks_config, + principal_config, observability_config, downstream_transport_config, upstream_transport_config, diff --git a/crates/contextforge-data-plane-lib/src/errors.rs b/crates/contextforge-data-plane-lib/src/errors.rs index 2c3ec1c2..2f28be73 100644 --- a/crates/contextforge-data-plane-lib/src/errors.rs +++ b/crates/contextforge-data-plane-lib/src/errors.rs @@ -2,7 +2,9 @@ use axum::response::Response; use http::{StatusCode, header}; pub(crate) fn unauthorized_response(message: &str) -> Response { - custom_error(StatusCode::UNAUTHORIZED, message) + let mut response = custom_error(StatusCode::UNAUTHORIZED, message); + response.headers_mut().insert(header::WWW_AUTHENTICATE, http::HeaderValue::from_static("Bearer")); + response } pub(crate) fn bad_request(message: &str) -> Response { diff --git a/crates/contextforge-data-plane-lib/src/layers/claims_id.rs b/crates/contextforge-data-plane-lib/src/layers/claims_id.rs index 4f4cc9ed..a0f48eaa 100644 --- a/crates/contextforge-data-plane-lib/src/layers/claims_id.rs +++ b/crates/contextforge-data-plane-lib/src/layers/claims_id.rs @@ -4,7 +4,11 @@ use axum::{ response::Response, }; -use crate::{common::ContextForgeDataPlaneAppState, errors::unauthorized_response}; +use crate::{ + AuthenticationError, + common::ContextForgeDataPlaneAppState, + errors::{custom_error, unauthorized_response}, +}; pub async fn claims_layer( State(state): State, @@ -13,13 +17,21 @@ pub async fn claims_layer( ) -> Response { let (mut parts, body) = request.into_parts(); - let Some(authorization) = parts.headers.get("Authorization") else { return unauthorized_response("No header") }; - - let Some(claims) = state.authorization_service.authorize(authorization).await else { - return unauthorized_response("Invalid token"); + let mut authorizations = parts.headers.get_all(http::header::AUTHORIZATION).iter(); + let Some(authorization) = authorizations.next() else { + return unauthorized_response("Missing bearer token"); }; - - parts.extensions.insert(claims.clone()); + if authorizations.next().is_some() { + return unauthorized_response("Ambiguous bearer token"); + } + let claims = match state.authorization_service.authorize(authorization).await { + Ok(claims) => claims, + Err(AuthenticationError::InvalidToken) => return unauthorized_response("Invalid bearer token"), + Err(AuthenticationError::KeysUnavailable) => { + return custom_error(http::StatusCode::SERVICE_UNAVAILABLE, "Authentication temporarily unavailable"); + }, + }; + parts.extensions.insert(claims); let request = Request::from_parts(parts, body); next.run(request).await } diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_header_limits.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_header_limits.rs index 8292a19b..506a29bd 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mcp_header_limits.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_header_limits.rs @@ -208,8 +208,8 @@ mod tests { #[async_trait] impl AuthorizationService for Noop { - async fn authorize(&self, _: &HeaderValue) -> Option { - None + async fn authorize(&self, _: &HeaderValue) -> Result { + Err(crate::AuthenticationError::InvalidToken) } } diff --git a/crates/contextforge-data-plane-lib/src/layers/mod.rs b/crates/contextforge-data-plane-lib/src/layers/mod.rs index bb341459..1740d2bd 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mod.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mod.rs @@ -6,3 +6,5 @@ pub mod virtual_host_config; pub mod virtual_host_id; pub use principal_extractor::PrincipalExtractorLayer; + +pub mod permission; diff --git a/crates/contextforge-data-plane-lib/src/layers/permission.rs b/crates/contextforge-data-plane-lib/src/layers/permission.rs new file mode 100644 index 00000000..77e3a231 --- /dev/null +++ b/crates/contextforge-data-plane-lib/src/layers/permission.rs @@ -0,0 +1,52 @@ +use crate::{ + authorization::{AuthorizedPrincipal, Permission}, + errors::{custom_error, unauthorized_response}, +}; +use axum::{ + extract::{Request, State}, + middleware::Next, + response::Response, +}; +use http::StatusCode; + +/// Reusable API-level guard. Install after verified principal extraction and before +/// configuration or backend access. Use `Admin` for future management routes. +pub async fn require_permission(State(permission): State, request: Request, next: Next) -> Response { + let Some(principal) = request.extensions().get::() else { + return unauthorized_response("Missing verified identity"); + }; + if !principal.has_permission(permission) { + return custom_error(StatusCode::FORBIDDEN, "Insufficient permission"); + } + next.run(request).await +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::authorization::{DefaultPrincipalExtractor, PrincipalExtractor}; + use axum::{Router, body::Body, middleware, routing::get}; + use tower::ServiceExt; + + #[tokio::test] + async fn guards_require_a_principal_and_the_requested_permission() { + for permission in [Permission::Admin, Permission::MCPUser] { + let app = Router::new() + .route("/", get(|| async { StatusCode::NO_CONTENT })) + .layer(middleware::from_fn_with_state(permission, require_permission)); + let response = app.clone().oneshot(Request::new(Body::empty())).await.unwrap(); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + for role in ["admin", "builder", "user", "unknown"] { + let principal = DefaultPrincipalExtractor::default() + .extract(&serde_json::json!({"iss":"watson","sub":"user","tenant_id":"tenant","role":role})) + .unwrap(); + let mut request = Request::new(Body::empty()); + request.extensions_mut().insert(principal); + let response = app.clone().oneshot(request).await.unwrap(); + let allowed = + role == "admin" || permission == Permission::MCPUser && ["builder", "user"].contains(&role); + assert_eq!(response.status(), if allowed { StatusCode::NO_CONTENT } else { StatusCode::FORBIDDEN }); + } + } + } +} diff --git a/crates/contextforge-data-plane-lib/src/layers/principal_extractor.rs b/crates/contextforge-data-plane-lib/src/layers/principal_extractor.rs index a1c37a35..7b92b4d3 100644 --- a/crates/contextforge-data-plane-lib/src/layers/principal_extractor.rs +++ b/crates/contextforge-data-plane-lib/src/layers/principal_extractor.rs @@ -58,8 +58,8 @@ where let maybe_authorization_claims = request.extensions().get::(); let Some(Ok(authorized_principal)) = maybe_authorization_claims.map(|authorization_claims| { principal_extractor - .extract(&authorization_claims.into()) - .inspect_err(|e| debug!("Can't extract the principal {e:?}")) + .extract(authorization_claims.as_value()) + .inspect_err(|_| debug!("principal_extract - invalid identity or permission claims")) }) else { return Ok(unauthorized_response("Invalid token. Unable to extract the principal from claims")); }; diff --git a/crates/contextforge-data-plane-lib/src/lib.rs b/crates/contextforge-data-plane-lib/src/lib.rs index 1eec4bca..74e131c7 100644 --- a/crates/contextforge-data-plane-lib/src/lib.rs +++ b/crates/contextforge-data-plane-lib/src/lib.rs @@ -52,7 +52,11 @@ use crate::{ virtual_host_id::virtual_host_id_layer, }, }; -pub use authorization::{AuthorizationClaims, AuthorizationService, get_authorization_service}; +pub use authorization::{ + AuthenticationError, AuthorizationClaims, AuthorizationService, AuthorizedPrincipal, Permission, PrincipalConfig, + ScopeMapping, UserClaim, get_authorization_service, +}; +pub use layers::permission::require_permission; #[derive(Clone)] pub enum UserConfigStoreType { @@ -109,6 +113,7 @@ impl Gateway { /// callers bind listeners before starting the service. pub async fn into_router(self) -> Result { let Gateway { config, session_manager, user_config_store_type, plugin_runtime, authorization_service } = self; + config.principal_config.validate()?; let user_config_store = match user_config_store_type { UserConfigStoreType::Redis => Arc::new(get_config_store(&config).await?), UserConfigStoreType::Test(store) => store, @@ -143,14 +148,18 @@ impl Gateway { let app = axum::Router::new() .nest_service("/servers/{virtual_host_name}/mcp", mcp_service) .layer(middleware::from_fn(virtual_host_config_layer)) - .layer(middleware::from_fn_with_state(mcp_gateway_state.clone(), user_config_store_layer)); + .layer(middleware::from_fn_with_state(mcp_gateway_state.clone(), user_config_store_layer)) + .layer(middleware::from_fn_with_state(Permission::MCPUser, require_permission)); let app = if let Some(cel_principal_extractor_path) = config.cel_principal_extractor_path.as_ref() { - app.layer(layers::PrincipalExtractorLayer::new(CelPrincipalExtractor::from_file( - cel_principal_extractor_path, - )?)) + app.layer(layers::PrincipalExtractorLayer::new( + CelPrincipalExtractor::from_file(cel_principal_extractor_path)? + .with_config(config.principal_config.clone()), + )) } else { - app.layer(layers::PrincipalExtractorLayer::new(DefaultPrincipalExtractor {})) + app.layer(layers::PrincipalExtractorLayer::new(DefaultPrincipalExtractor::new( + config.principal_config.clone(), + ))) }; let app = app diff --git a/crates/contextforge-data-plane-lib/src/tools.rs b/crates/contextforge-data-plane-lib/src/tools.rs index e3a8a2f4..e82482ab 100644 --- a/crates/contextforge-data-plane-lib/src/tools.rs +++ b/crates/contextforge-data-plane-lib/src/tools.rs @@ -38,11 +38,12 @@ async fn get_jwks(State(state): State) -> Respons .into_response(); }; - let Ok(key) = Jwk::from_encoding_key(&key, jsonwebtoken::Algorithm::RS256) else { + let Ok(mut key) = Jwk::from_encoding_key(&key, jsonwebtoken::Algorithm::RS256) else { return (StatusCode::INTERNAL_SERVER_ERROR, "Can't find the encoding key or the format is wrong") .into_response(); }; + key.common.key_id = Some("test".to_owned()); let keys = vec![key]; (StatusCode::OK, [(CACHE_CONTROL, JWKS_CACHE_CONTROL)], Json(JwkSet { keys })).into_response() } @@ -95,9 +96,11 @@ pub async fn get_token( std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).expect("Time went backwards").as_secs(); let map = json!( { - "iss": "contexforge-dataplane", + "iss": state.config.jwks_config.issuer, "sub": user_id.clone(), - "aud": "contexforge-dataplane-audience", + "aud": state.config.jwks_config.audiences, + "role": "user", + "woUserId": user_id.clone(), "exp": now + Duration::from_hours(1).as_secs(), "nbf": now - Duration::from_mins(1).as_secs(), "iat": now, diff --git a/crates/contextforge-data-plane-lib/tests/gateway.rs b/crates/contextforge-data-plane-lib/tests/gateway.rs index 0abd8d15..8c9120df 100644 --- a/crates/contextforge-data-plane-lib/tests/gateway.rs +++ b/crates/contextforge-data-plane-lib/tests/gateway.rs @@ -17,3 +17,6 @@ mod resources; mod subscriptions; #[path = "gateway/tools.rs"] mod tools; + +#[path = "gateway/downstream_auth.rs"] +mod downstream_auth; diff --git a/crates/contextforge-data-plane-lib/tests/gateway/downstream_auth.rs b/crates/contextforge-data-plane-lib/tests/gateway/downstream_auth.rs new file mode 100644 index 00000000..165aa223 --- /dev/null +++ b/crates/contextforge-data-plane-lib/tests/gateway/downstream_auth.rs @@ -0,0 +1,397 @@ +use crate::harness::{TestServer, create_default_config}; +use async_trait::async_trait; +use axum::{Json, Router, body::Body, routing::get}; +use contextforge_data_plane_apis::{ + User, + user_store::{UserConfig, VirtualHost}, +}; +use contextforge_data_plane_lib::{ + Config, ConfigStore, ConfigStoreError, Gateway, UserConfigStoreType, get_authorization_service, +}; +use http::{Request, StatusCode}; +use jsonwebtoken::{Algorithm, EncodingKey, Header, encode, jwk::Jwk}; +use rmcp::transport::streamable_http_server::session::local::LocalSessionManager; +use serde_json::{Value, json}; +use std::{ + collections::HashMap, + sync::{ + Arc, + atomic::{AtomicUsize, Ordering}, + }, + time::{SystemTime, UNIX_EPOCH}, +}; +use tower::ServiceExt; + +fn key() -> EncodingKey { + EncodingKey::from_rsa_pem(include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/../../assets/jwt.key"))) + .expect("valid authentication test fixture") +} +fn claims() -> Value { + let now = SystemTime::now().duration_since(UNIX_EPOCH).expect("valid authentication test fixture").as_secs(); + json!({"iss":"mcpgateway", "aud":"mcpgateway-api", "sub":"user", "woTenantId":"tenant", "tenant_id":"tenant", "role":"user", "exp":now+3600, "nbf":now-60}) +} +fn token(claims: &Value, kid: Option<&str>, algorithm: Algorithm) -> String { + let mut header = Header::new(algorithm); + header.kid = kid.map(str::to_owned); + encode(&header, claims, &key()).expect("valid authentication test fixture") +} + +#[derive(Clone, Default)] +struct CountingStore { + reads: Arc, + virtual_host: Option, +} +#[async_trait] +impl ConfigStore for CountingStore { + async fn get_config<'a>(&self, user: &'a User) -> Result { + assert_eq!(user.key(), "user"); + self.reads.fetch_add(1, Ordering::SeqCst); + Ok(UserConfig { + virtual_hosts: HashMap::from([( + "test".into(), + self.virtual_host.clone().unwrap_or_else(|| VirtualHost { + backends: HashMap::new(), + tools: HashMap::new(), + resources: HashMap::new(), + resource_templates: HashMap::new(), + prompts: HashMap::new(), + }), + )]), + }) + } + async fn set_config<'a>(&self, _: &'a User, _: &'a UserConfig) -> Result<(), ConfigStoreError> { + unreachable!() + } +} + +async fn gateway(config: Config, store: CountingStore) -> Router { + Gateway::builder() + .with_authorization_service( + get_authorization_service(&config.jwks_config).expect("valid authentication test fixture"), + ) + .with_config(config) + .with_user_config_store_type(UserConfigStoreType::Test(Arc::new(store))) + .with_session_manager(Arc::new(LocalSessionManager::default())) + .build() + .into_router() + .await + .expect("valid authentication test fixture") +} + +fn request(token: Option<&str>, method: &str) -> Request { + let mut request = Request::builder() + .method("POST") + .uri("/contextforge-rs/servers/test/mcp") + .header("host", "localhost") + .header("content-type", "application/json") + .header("accept", "application/json, text/event-stream") + .header("MCP-Protocol-Version", "2026-07-28") + .header("Mcp-Method", method); + if method == "tools/call" { + request = request.header("Mcp-Name", "sum"); + } + if let Some(token) = token { + request = request.header("authorization", format!("Bearer {token}")); + } + let mut params = json!({ + "_meta": { + "io.modelcontextprotocol/protocolVersion": "2026-07-28", + "io.modelcontextprotocol/clientInfo": {"name": "auth-test", "version": "1"}, + "io.modelcontextprotocol/clientCapabilities": {} + } + }); + if method == "tools/call" { + params["name"] = "sum".into(); + params["arguments"] = json!({"a": 2, "b": 3}); + } + request + .body(Body::from(json!({"jsonrpc": "2.0", "id": 1, "method": method, "params": params}).to_string())) + .expect("valid authentication test fixture") +} + +async fn key_server() -> (TestServer, Arc) { + let mut jwk = Jwk::from_encoding_key(&key(), Algorithm::RS256).expect("valid authentication test fixture"); + jwk.common.key_id = Some("test".into()); + let document = json!({"keys":[jwk]}); + let requests = Arc::new(AtomicUsize::new(0)); + let counter = Arc::clone(&requests); + let server = TestServer::start_http(Router::new().route( + "/jwks", + get(move || { + counter.fetch_add(1, Ordering::SeqCst); + let document = document.clone(); + async move { Json(document) } + }), + )) + .await + .expect("valid authentication test fixture"); + (server, requests) +} + +#[tokio::test] +async fn authenticates_real_rsa_tokens_and_rejects_before_configuration() { + let (server, requests) = key_server().await; + let mut config = create_default_config(); + config.jwks_config.url = server.url("/jwks").parse().expect("valid authentication test fixture"); + let store = CountingStore::default(); + let app = gateway(config, store.clone()).await; + // Warm both authentication and the request path, then deny access on every subsequent request. + let valid = token(&claims(), Some("test"), Algorithm::RS256); + let response = + app.clone().oneshot(request(Some(&valid), "server/discover")).await.expect("valid authentication test fixture"); + let status = response.status(); + let body = axum::body::to_bytes(response.into_body(), 65_536).await.expect("valid authentication test fixture"); + assert_eq!(status, StatusCode::OK, "{}", String::from_utf8_lossy(&body)); + assert_eq!(store.reads.load(Ordering::SeqCst), 1); + for method in ["server/discover", "tools/list", "tools/call", "resources/list", "prompts/list"] { + let mut denied = claims(); + denied["role"] = "unknown".into(); + let denied = token(&denied, Some("test"), Algorithm::RS256); + assert_eq!( + app.clone() + .oneshot(request(Some(&denied), method)) + .await + .expect("valid authentication test fixture") + .status(), + StatusCode::FORBIDDEN, + "{method}" + ); + } + let mut invalid = vec![ + None, + Some("malformed".into()), + Some(token(&claims(), None, Algorithm::RS256)), + Some(token(&claims(), Some("unknown"), Algorithm::RS256)), + Some(token(&claims(), Some("test"), Algorithm::RS384)), + ]; + for (name, value) in [ + ("iss", json!("wrong")), + ("aud", json!("wrong")), + ("exp", json!(1)), + ("nbf", json!(9_999_999_999_u64)), + ("nbf", json!("tomorrow")), + ("sub", json!("")), + ("woTenantId", json!(null)), + ("tenant_id", json!("conflicting")), + ] { + let mut c = claims(); + c[name] = value; + invalid.push(Some(token(&c, Some("test"), Algorithm::RS256))); + } + for missing in ["iss", "aud", "exp", "sub", "woTenantId"] { + let mut c = claims(); + c.as_object_mut().expect("valid authentication test fixture").remove(missing); + if missing == "woTenantId" { + c.as_object_mut().expect("claims object").remove("tenant_id"); + } + invalid.push(Some(token(&c, Some("test"), Algorithm::RS256))); + } + let mut tampered = valid.clone().into_bytes(); + let position = tampered.iter().rposition(|b| *b == b'.').expect("valid authentication test fixture") + 1; + tampered[position] = if tampered[position] == b'A' { b'B' } else { b'A' }; + invalid.push(Some(String::from_utf8(tampered).expect("valid authentication test fixture"))); + for token in invalid { + let response = app + .clone() + .oneshot(request(token.as_deref(), "server/discover")) + .await + .expect("valid authentication test fixture"); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert_eq!(response.headers()["www-authenticate"], "Bearer"); + } + let mut ambiguous = request(Some(&valid), "server/discover"); + ambiguous + .headers_mut() + .append("authorization", format!("Bearer {valid}").parse().expect("valid authentication test fixture")); + assert_eq!( + app.clone().oneshot(ambiguous).await.expect("valid authentication test fixture").status(), + StatusCode::UNAUTHORIZED + ); + assert_eq!(store.reads.load(Ordering::SeqCst), 1, "denials must not access configuration"); + assert_eq!(requests.load(Ordering::SeqCst), 1, "valid, invalid and unknown kid traffic uses a bounded cache"); + for role in ["admin", "builder", "user"] { + let mut c = claims(); + c["role"] = role.into(); + c["aud"] = json!(["unrelated", "mcpgateway-api"]); + let token = token(&c, Some("test"), Algorithm::RS256); + assert_eq!( + app.clone() + .oneshot(request(Some(&token), "server/discover")) + .await + .expect("valid authentication test fixture") + .status(), + StatusCode::OK + ); + } + server.shutdown().await.expect("valid authentication test fixture"); +} + +#[tokio::test] +async fn jwks_outage_is_503_and_never_reaches_configuration() { + let requests = Arc::new(AtomicUsize::new(0)); + let count = Arc::clone(&requests); + let server = TestServer::start_http(Router::new().route( + "/jwks", + get(move || { + count.fetch_add(1, Ordering::SeqCst); + async { StatusCode::SERVICE_UNAVAILABLE } + }), + )) + .await + .expect("valid authentication test fixture"); + let mut config = create_default_config(); + config.jwks_config.url = server.url("/jwks").parse().expect("valid authentication test fixture"); + let store = CountingStore::default(); + let app = gateway(config, store.clone()).await; + let token = token(&claims(), Some("test"), Algorithm::RS256); + let results = + futures::future::join_all((0..20).map(|_| app.clone().oneshot(request(Some(&token), "server/discover")))).await; + for result in results { + assert_eq!(result.expect("valid authentication test fixture").status(), StatusCode::SERVICE_UNAVAILABLE); + } + assert_eq!(requests.load(Ordering::SeqCst), 1); + assert_eq!(store.reads.load(Ordering::SeqCst), 0); + server.shutdown().await.expect("valid authentication test fixture"); +} + +/// Small repeatable load probe of the complete cached-auth/discovery path. +/// Run explicitly with --ignored --nocapture; compare the same profile on both revisions. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +#[ignore = "manual cached-authentication load comparison"] +async fn cached_authentication_load() { + let (server, requests) = key_server().await; + let mut config = create_default_config(); + config.jwks_config.url = server.url("/jwks").parse().expect("valid authentication test fixture"); + let store = CountingStore::default(); + let app = gateway(config, store).await; + let token = token(&claims(), Some("test"), Algorithm::RS256); + for _ in 0..100 { + assert_eq!( + app.clone() + .oneshot(request(Some(&token), "server/discover")) + .await + .expect("valid authentication test fixture") + .status(), + StatusCode::OK + ); + } + let start = std::time::Instant::now(); + let tasks = (0..16) + .map(|_| { + let app = app.clone(); + let token = token.clone(); + tokio::spawn(async move { + let mut times = Vec::with_capacity(500); + for _ in 0..500 { + let start = std::time::Instant::now(); + let response = app + .clone() + .oneshot(request(Some(&token), "server/discover")) + .await + .expect("valid authentication test fixture"); + assert_eq!(response.status(), StatusCode::OK); + let _ = axum::body::to_bytes(response.into_body(), 65_536) + .await + .expect("valid authentication test fixture"); + times.push(start.elapsed().as_micros()); + } + times + }) + }) + .collect::>(); + let mut times = Vec::new(); + for task in tasks { + times.extend(task.await.expect("valid authentication test fixture")); + } + let elapsed = start.elapsed(); + times.sort_unstable(); + println!( + "cached auth/discovery: requests=8000 concurrency=16 elapsed_ms={} rps={:.0} p50_us={} p95_us={} jwks_fetches={}", + elapsed.as_millis(), + 8000.0 / elapsed.as_secs_f64(), + times[4000], + times[7600], + requests.load(Ordering::SeqCst) + ); + assert_eq!(requests.load(Ordering::SeqCst), 1); + server.shutdown().await.expect("valid authentication test fixture"); +} + +#[tokio::test] +async fn permission_denial_prevents_backend_calls_after_a_successful_request() { + use axum::{ + extract::State, + middleware::{self, Next}, + }; + use contextforge_data_plane_apis::user_store::{BackendMCPGateway, ServiceRoute}; + use rmcp::transport::{StreamableHttpServerConfig, StreamableHttpService}; + let hits = Arc::new(AtomicUsize::new(0)); + let service = StreamableHttpService::new( + || Ok(crate::harness::mock_counter::Counter::new()), + LocalSessionManager::default().into(), + StreamableHttpServerConfig::default(), + ); + let backend = + TestServer::start_http(Router::new().route_service("/mcp", service).layer(middleware::from_fn_with_state( + Arc::clone(&hits), + |State(hits): State>, request: axum::extract::Request, next: Next| async move { + if request.headers().get("Mcp-Method").is_some_and(|method| method == "tools/call") { + hits.fetch_add(1, Ordering::SeqCst); + } + next.run(request).await + }, + ))) + .await + .expect("valid authentication test fixture"); + let store = CountingStore { + virtual_host: Some(VirtualHost { + backends: HashMap::from([( + "counter".into(), + BackendMCPGateway { + name: "counter".into(), + url: backend.url("/mcp").parse().expect("valid authentication test fixture"), + mcp_protocol_version: rmcp::model::ProtocolVersion::V_2026_07_28, + passthrough_headers: vec![], + add_headers: HashMap::new(), + remove_headers: vec![], + tool_schemas: HashMap::new(), + completion: HashMap::new(), + }, + )]), + tools: HashMap::from([( + "sum".into(), + ServiceRoute { backend_name: "counter".into(), upstream_name: "sum".into() }, + )]), + resources: HashMap::new(), + resource_templates: HashMap::new(), + prompts: HashMap::new(), + }), + ..Default::default() + }; + let (server, _) = key_server().await; + let mut config = create_default_config(); + config.upstream_transport_config.upstream_connection_mode = + Some(contextforge_data_plane_lib::UpstreamConnectionMode::PlainTextOrTls); + config.jwks_config.url = server.url("/jwks").parse().expect("valid authentication test fixture"); + let app = gateway(config, store.clone()).await; + for (role, expected) in [("user", StatusCode::OK), ("unknown", StatusCode::FORBIDDEN)] { + let mut c = claims(); + c["role"] = role.into(); + let token = token(&c, Some("test"), Algorithm::RS256); + let request = request(Some(&token), "tools/call"); + let response = app.clone().oneshot(request).await.expect("valid authentication test fixture"); + let status = response.status(); + let body = axum::body::to_bytes(response.into_body(), 65_536).await.expect("valid authentication test fixture"); + assert_eq!(status, expected, "{}", String::from_utf8_lossy(&body)); + if role == "user" { + let text = String::from_utf8_lossy(&body); + let data = text.lines().find_map(|line| line.strip_prefix("data: ")).unwrap_or(&text); + let message: Value = serde_json::from_str(data).expect("MCP result JSON"); + assert_eq!(message["result"]["content"][0]["text"], "5", "{message}"); + } + assert_eq!(store.reads.load(Ordering::SeqCst), 1); + assert_eq!(hits.load(Ordering::SeqCst), 1, "denied request must never reach backend"); + } + server.shutdown().await.expect("valid authentication test fixture"); + backend.shutdown().await.expect("valid authentication test fixture"); +} diff --git a/crates/contextforge-data-plane-lib/tests/gateway/harness/auth.rs b/crates/contextforge-data-plane-lib/tests/gateway/harness/auth.rs index 0a1495fc..8a7f2672 100644 --- a/crates/contextforge-data-plane-lib/tests/gateway/harness/auth.rs +++ b/crates/contextforge-data-plane-lib/tests/gateway/harness/auth.rs @@ -19,6 +19,7 @@ fn default_claims(user_id: &str) -> serde_json::Value { "iss": "mcpgateway", "sub": user_id, "tenant_id": "test_tenant", + "role": "user", "aud": "mcpgateway-api", "exp": now + TEST_TOKEN_TTL_SECS, "iat": now, @@ -60,7 +61,10 @@ impl AlwaysAllowAuthorizatioService { #[async_trait] impl AuthorizationService for AlwaysAllowAuthorizatioService { - async fn authorize(&self, _: &HeaderValue) -> Option { - Some(AuthorizationClaims::from(default_claims(&self.user))) + async fn authorize( + &self, + _: &HeaderValue, + ) -> Result { + Ok(AuthorizationClaims::from(default_claims(&self.user))) } } diff --git a/crates/contextforge-data-plane-lib/tests/gateway/harness/mod.rs b/crates/contextforge-data-plane-lib/tests/gateway/harness/mod.rs index 2af3d501..bf14b9d8 100644 --- a/crates/contextforge-data-plane-lib/tests/gateway/harness/mod.rs +++ b/crates/contextforge-data-plane-lib/tests/gateway/harness/mod.rs @@ -48,7 +48,15 @@ pub(crate) use user_config_store::MemoryUserConfigStore; pub fn create_default_config() -> Config { Config { address: None, - jwks_config: JwksConfig { url: "http://127.0.0.1:8080/".parse().expect("should work"), ca_cert_path: None }, + jwks_config: JwksConfig { + url: "http://127.0.0.1:8080/".parse().expect("should work"), + ca_cert_path: None, + issuer: "mcpgateway".to_owned(), + audiences: vec!["mcpgateway-api".to_owned()], + algorithms: vec![jsonwebtoken::Algorithm::RS256], + leeway_seconds: 30, + }, + principal_config: contextforge_data_plane_lib::PrincipalConfig::default(), mcp_standard_header_max_count: 10, mcp_standard_header_max_value_bytes: 4096, diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index e8d95a31..0c14c98f 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -76,6 +76,14 @@ services: - CONTEXTFORGE_DATA_PLANE_UPSTREAM_CONNECTION_MODE=plain-text-or-tls # Set the HTTPS JWKS endpoint for the token issuer in the host environment. - CONTEXTFORGE_DATA_PLANE_JWKS_URL + - CONTEXTFORGE_DATA_PLANE_JWT_ISSUER + - CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES + - CONTEXTFORGE_DATA_PLANE_JWT_ALGORITHMS + - CONTEXTFORGE_DATA_PLANE_JWT_LEEWAY_SECONDS + - CONTEXTFORGE_DATA_PLANE_JWT_USER_CLAIM + - CONTEXTFORGE_DATA_PLANE_JWT_ADMIN_SCOPES + - CONTEXTFORGE_DATA_PLANE_JWT_MCP_USER_SCOPES + - CONTEXTFORGE_DATA_PLANE_JWT_SCOPES_ONLY - RUST_LOG=debug sysctls: @@ -136,6 +144,14 @@ services: - CONTEXTFORGE_DATA_PLANE_UPSTREAM_CONNECTION_MODE=plain-text-or-tls # Set the HTTPS JWKS endpoint for the token issuer in the host environment. - CONTEXTFORGE_DATA_PLANE_JWKS_URL + - CONTEXTFORGE_DATA_PLANE_JWT_ISSUER + - CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES + - CONTEXTFORGE_DATA_PLANE_JWT_ALGORITHMS + - CONTEXTFORGE_DATA_PLANE_JWT_LEEWAY_SECONDS + - CONTEXTFORGE_DATA_PLANE_JWT_USER_CLAIM + - CONTEXTFORGE_DATA_PLANE_JWT_ADMIN_SCOPES + - CONTEXTFORGE_DATA_PLANE_JWT_MCP_USER_SCOPES + - CONTEXTFORGE_DATA_PLANE_JWT_SCOPES_ONLY - RUST_LOG=debug sysctls: @@ -173,6 +189,14 @@ services: - CONTEXTFORGE_DATA_PLANE_UPSTREAM_CONNECTION_MODE=plain-text-or-tls # Set the HTTPS JWKS endpoint for the token issuer in the host environment. - CONTEXTFORGE_DATA_PLANE_JWKS_URL + - CONTEXTFORGE_DATA_PLANE_JWT_ISSUER + - CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES + - CONTEXTFORGE_DATA_PLANE_JWT_ALGORITHMS + - CONTEXTFORGE_DATA_PLANE_JWT_LEEWAY_SECONDS + - CONTEXTFORGE_DATA_PLANE_JWT_USER_CLAIM + - CONTEXTFORGE_DATA_PLANE_JWT_ADMIN_SCOPES + - CONTEXTFORGE_DATA_PLANE_JWT_MCP_USER_SCOPES + - CONTEXTFORGE_DATA_PLANE_JWT_SCOPES_ONLY - RUST_LOG=debug sysctls: From 33a4a89d8c3d3465c4da2c26035599e65ddce1f0 Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Wed, 23 Sep 2026 11:56:51 +0100 Subject: [PATCH 02/19] docs: separate documentation from downstream auth implementation Signed-off-by: Pratik Gandhi --- README.md | 12 +--- _context/wiki/architecture.md | 7 +-- _context/wiki/config.md | 99 +++++++++----------------------- _context/wiki/deployment.md | 6 +- _context/wiki/failure-modes.md | 6 +- _context/wiki/getting-started.md | 5 +- _context/wiki/performance.md | 31 ---------- _context/wiki/security.md | 35 +++++------ 8 files changed, 52 insertions(+), 149 deletions(-) diff --git a/README.md b/README.md index f23c5598..2f4afbfb 100644 --- a/README.md +++ b/README.md @@ -15,18 +15,12 @@ test stack: ```bash export CONTEXTFORGE_DATA_PLANE_JWKS_URL=https://your-issuer.example/.well-known/jwks.json -export CONTEXTFORGE_DATA_PLANE_JWT_ISSUER=https://your-issuer.example -export CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES=your-gateway-audience make docker-prod make compose-up ``` -Replace the example trust settings with the exact issuer, accepted audience, -and HTTPS JWKS endpoint for your tokens. Tokens must also contain the user, -tenant, and role claims described in [JWT configuration](_context/wiki/config.md#jwt-claims-validated-by-claims_layer). -Existing control-plane tokens with a different claim layout need an explicit -CEL mapping; a valid signature alone no longer grants MCP access. The production -image includes the plugin factories and +Replace the example JWKS URL with the HTTPS endpoint for the issuer of your +control-plane tokens. The production image includes the plugin factories and `/contextforge-rs/health`; it does not include `with_tools`. That feature is for testing only and enables unauthenticated token, JWKS, and config helpers. @@ -85,6 +79,7 @@ cargo run --release \ --redis-port 6379 \ --redis-address 127.0.0.1 \ --jwks-url "$CONTEXTFORGE_DATA_PLANE_JWKS_URL" \ + --number-of-cpus 16 \ --redis-mode=plain-text \ --upstream-connection-mode=plain-text-or-tls \ --runtime-plugins-enabled true @@ -104,7 +99,6 @@ cargo run -p contextforge-data-plane --features with_tools \ --redis-mode plain-text \ --token-verification-private-key ./assets/jwt.key \ --jwks-url http://127.0.0.1:8080/contextforge-rs/admin/.well-known/jwks.json \ ---jwt-issuer local-dev --jwt-audiences local-gateway \ --upstream-connection-mode plain-text-or-tls \ --tls-address 127.0.0.1:8443 \ --server-private-key ./assets/tls_key.pem \ diff --git a/_context/wiki/architecture.md b/_context/wiki/architecture.md index 9494a64d..246745c4 100644 --- a/_context/wiki/architecture.md +++ b/_context/wiki/architecture.md @@ -15,9 +15,8 @@ TCP/TLS listener -> CORS layer -> mcp_header_limits_layer bounds MCP headers (431) -> virtual_host_id_layer inserts VirtualHostId from path (400) - -> claims_layer verifies JWT, inserts AuthorizationClaims (401; JWKS unavailable 503) + -> claims_layer verifies JWT, inserts AuthorizationClaims (401) -> PrincipalExtractorLayer inserts AuthorizedPrincipal (401) - -> require_permission requires MCPUser (403), before any config lookup -> user_config_store_layer loads UserConfig (400 missing, 500 decode/error) -> virtual_host_config_layer checks caller's virtual host (404) -> /servers/{virtual_host_name}/mcp RMCP service @@ -39,7 +38,7 @@ consume typed extensions; they do not parse Redis keys. ```text modern MCP request - -> header, JWT, principal, and MCPUser permission checks + -> header, JWT, and principal checks -> user config and virtual-host check -> published object/backend route -> recognized tool parameter-header validation @@ -85,7 +84,7 @@ into successful response hooks. See [Routing](routing.md) and | State | Owner | Lifetime | | --- | --- | --- | | Parsed config and shared upstream HTTP client | Gateway | Process. | -| JWKS keys | JWT authorization service | Five-minute cache; serialized refresh, five-second cooldown; no expired-key fallback. | +| JWKS keys | JWT authorization service | Five-minute cache; fetched when verification needs them. | | User config | Redis store and optional local LRU | Redis is authoritative; local capacity 50,000, default expiry 60 seconds. | | Principal, claims, virtual-host ID, config snapshot | HTTP request extensions | One request. | | Backend RMCP service | Routed operation | One request; explicitly closed after the call. | diff --git a/_context/wiki/config.md b/_context/wiki/config.md index 9fcc6257..0b1bd36c 100644 --- a/_context/wiki/config.md +++ b/_context/wiki/config.md @@ -3,7 +3,7 @@ ## Minimum Required Flags ```text ---redis-address --redis-port --redis-mode --jwks-url --jwt-issuer --jwt-audiences +--redis-address --redis-port --redis-mode --jwks-url ``` Plus at least one listener: `--address` or `--tls-address`. Development builds @@ -33,14 +33,6 @@ Origin and Host settings retain the explicitly configured | `--server-private-key ` | `CONTEXTFORGE_DATA_PLANE_TLS_SERVER_PRIVATE_KEY` | With `--tls-address` | PEM private key for downstream TLS. | | `--jwks-url ` | `CONTEXTFORGE_DATA_PLANE_JWKS_URL` | Required | Fetches RSA/EC JWT verification keys. HTTPS required except for loopback HTTP testing. | | `--jwks-ca-cert-path ` | `CONTEXTFORGE_DATA_PLANE_JWKS_CA_PATH` | Optional | PEM CA bundle trusted by the JWKS HTTP client. | -| `--jwt-issuer ` | `CONTEXTFORGE_DATA_PLANE_JWT_ISSUER` | Required | Exact trusted issuer. | -| `--jwt-audiences ` | `CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES` | Required | At least one token audience must match. | -| `--jwt-algorithms ` | `CONTEXTFORGE_DATA_PLANE_JWT_ALGORITHMS` | `RS256` | Explicit RSA/EC allowlist; HMAC is rejected at startup. | -| `--jwt-leeway-seconds ` | `CONTEXTFORGE_DATA_PLANE_JWT_LEEWAY_SECONDS` | `30` | Time tolerance, maximum 300 seconds. | -| `--jwt-user-claim ` | `CONTEXTFORGE_DATA_PLANE_JWT_USER_CLAIM` | `sub` | `sub` or explicit `wo-user-id` profile. | -| `--jwt-admin-scopes ` | `CONTEXTFORGE_DATA_PLANE_JWT_ADMIN_SCOPES` | None | Scope names granting Admin and MCPUser. Setting either mapping makes scopes restrict roles. | -| `--jwt-mcp-user-scopes ` | `CONTEXTFORGE_DATA_PLANE_JWT_MCP_USER_SCOPES` | None | Scope names granting MCPUser only. | -| `--jwt-scopes-only` | `CONTEXTFORGE_DATA_PLANE_JWT_SCOPES_ONLY` | `false` | Explicit scope-only authorization; requires a scope mapping. | | `--token-verification-private-key ` | None (CLI only) | Required when built with `with_tools` | Signs local test tokens and supplies the public key served by the local JWKS helper. | | `--cel-principal-extractor-path ` | None (CLI only) | Optional | CEL principal mapping for custom claim layouts; otherwise uses the default user/tenant claim mapping below. | @@ -116,65 +108,31 @@ the HTTP transport. ## JWT Claims (validated by `claims_layer`) -JWT trust settings come from deployment configuration, never from token headers -or claims. The verifier requires an allowed signature algorithm and a nonempty -`kid` that matches a configured JWKS signing key, including its declared algorithm. +The JWT signature is checked against the configured JWKS. The default principal +extractor then requires user and tenant IDs at the top level of the claims: -| Claim | Behavior | +| Claim | Current behavior | | --- | --- | -| `iss` | Required; exact configured issuer. Preserved in the request identity. | -| `aud` | Required string or string array; must include a configured audience. | -| `exp` | Required and validated, with configured clock leeway. | -| `nbf` | Validated when present; malformed or future values are rejected. | -| `sub` | Default required nonempty string user ID. No implicit fallback. | -| `woUserId` | Used instead of `sub` only with `--jwt-user-claim wo-user-id`. | -| `woTenantId`, `tenant_id`, `tenantId` | At least one nonempty string required. All present aliases must agree. | -| `role`, `roles` | String and string list respectively. `admin` grants Admin and MCPUser; `builder`/`user` grant MCPUser. Unknown or missing roles grant nothing. | -| `scope`, `permissions` | Space-separated string and string list respectively. Combined into normalized scopes; no scope grants access without explicit configuration. | - -Every MCP request requires MCPUser before configuration lookup. Missing/invalid -identity returns 401; valid identity without permission returns 403. A reusable -Admin guard is available for future management APIs; this branch adds no such APIs. - -By default roles decide permissions. Setting either scope mapping intersects -role permissions with scope permissions; absent/empty/unmapped scopes then deny. -For example, with `--jwt-mcp-user-scopes aipg.mcp`, even an admin needs that -scope to use MCP, and cannot gain Admin from it. `--jwt-scopes-only` explicitly -switches to scope-based grants, including for tokens without roles. Empty scope -names in configuration are invalid. Malformed role/scope claim types return 401. -These role defaults and scope names need confirmation with the Watson token owner -before real integration. - -For custom or nested claims, `--cel-principal-extractor-path` takes a trusted CEL -expression returning `user_id`, `tenant_id`, and optional `role`, `roles`, `scope`, -and `permissions`. The input variable is `claims`. Example: - -```cel -{"user_id": claims.woUserId, "tenant_id": claims.woTenantId, "roles": claims.user.roles} -``` - -The same permission policy applies to CEL results. It cannot override the issuer, -which always comes from the verified token. There is no implicit default tenant. -User IDs need not be emails. The issuer and tenant are carried in the request -identity, but **persistent/cache keys still contain only the user ID**. This -change does not establish isolation between tenants sharing a subject. A coordinated -publisher/key migration and CPEX policy integration remain separate work. - -The development token helper sets the configured issuer/audiences, path-derived -`sub`, `woUserId`, `tenant_id`, and `role: user`. It uses RS256 and key ID `test`. -Its token is a raw string for `Authorization: Bearer ...`. Custom scope profiles -need matching claims through the POST helper. All `with_tools` routes remain -unauthenticated, testing-only helpers; production builds must exclude them. - -Keys are cached for five minutes. One fetch runs at a time and refresh attempts -are separated by at least five seconds, including after failure. Unknown key IDs -can trigger a refresh after that cooldown; rotation may take up to the cooldown -plus fetch time to be observed. Known unexpired keys remain usable during a JWKS -outage; expired keys never authenticate. Fetches have a ten-second total timeout, -a 1 MiB response limit, and no redirects. Unavailable or unusable JWKS returns 503 -when no usable cached key is available; invalid tokens return 401. No per-token -revocation lookup exists. Removing a signing key is effective after refresh or -cache expiry; restart to clear the cache immediately if necessary. +| `sub`, `user_id`, `UserId` | First present alias must be a string; supplies the user ID used for Redis config lookup. | +| `tenantId`, `tenant_id` | First present alias must be a string; supplies the principal's tenant ID. | +| `exp` | Checked when present; the local helper sets a one-hour expiry. | +| `nbf` | Checked when present; rejects tokens that are not yet valid, subject to verifier leeway. | +| `iss`, `aud` | No fixed issuer or audience is currently enforced by the JWKS verifier. | + +The default extractor does not infer the tenant from `teams`, email, or a nested +`user` object. Use `--cel-principal-extractor-path` for a custom mapping. +An earlier alias with a non-string value prevents fallback to a later alias. +The tenant ID is required by extraction but is not currently included in the +user-config Redis/cache key. JWT scopes and RBAC are not enforced here; object +visibility comes from the published routing maps. + +The local `GET /contextforge-rs/admin/tokens/{tenant_id}/{user_id}` helper sets +`tenant_id` and `sub` from the path. Its raw JWT response belongs in the +`Authorization: Bearer ...` header; it is not a JSON token object. + +There is no per-token revocation. Verification keys are cached for five minutes; +removing a key from JWKS is not immediate invalidation of cached keys. Restart +the dataplane after removing a key if that cache must be cleared immediately. ## UserConfig Shape (from `contextforge-data-plane-apis`) @@ -348,7 +306,6 @@ cargo run -p contextforge-data-plane \ --redis-port 6379 \ --redis-mode plain-text \ --jwks-url http://127.0.0.1:8001/contextforge-rs/admin/.well-known/jwks.json \ - --jwt-issuer local-dev --jwt-audiences local-gateway \ --token-verification-private-key assets/jwt.key \ --upstream-connection-mode plain-text-or-tls \ --runtime-plugins-enabled true @@ -371,8 +328,7 @@ cargo nextest run --locked -p contextforge-data-plane-lib --test gateway -E 'tes | mTLS upstream without certificate/key | Upstream HTTP client construction. | | Invalid JWKS URL scheme or non-loopback plain HTTP URL | Authorization-service construction. | | Missing/invalid plugin document when enabled | CPEX initialization, before serving requests. | -| Missing/empty JWT issuer/audience, non-RSA/EC algorithm, or excessive leeway | Authorization-service construction. | -| Unreachable JWKS endpoint | 503 during token verification when keys must be fetched. | +| Unreachable JWKS endpoint | Token verification when keys must be fetched. | | Unreachable backend or HTTP URL with default HTTPS-only mode | When a request selects that backend. | Redis connection setup retries rather than failing immediately. The local @@ -414,9 +370,7 @@ is **15 seconds**; allow up to about 45–60 seconds after generating traffic. | Symptom | Where to look | | --- | --- | -| `401` | Bearer header, signature/issuer/audience/time checks, and principal claim types. | -| `403` | Required API permission missing. | -| `503` | `jwks_refresh` failure logs and JWKS reachability. | +| `401` | Bearer header, `validate: unable to refresh SaaS JWKS`, `validate_and_decode_claims`, and `Can't extract the principal` logs. | | `400` config error | `user_config_store_layer` and whether the publisher used the extracted user ID. A Redis GET failure also maps here. | | `404 Server not found` | `virtual_host_config_layer`; requested vhost versus caller's published configuration. | | MCP routing errors | `AuthorizedCallValidator::validate` log prefix (from `validate_stateless`), then `call_tool`, `read_resource`, or `get_prompt` diagnostics. | @@ -458,7 +412,6 @@ cargo run --release -p contextforge-data-plane --features with_tools,plugins \ --address 127.0.0.1:8001 \ --redis-port 6379 --redis-address 127.0.0.1 --redis-mode plain-text \ --jwks-url http://127.0.0.1:8001/contextforge-rs/admin/.well-known/jwks.json \ - --jwt-issuer local-dev --jwt-audiences local-gateway \ --token-verification-private-key assets/jwt.key \ --upstream-connection-mode plain-text-or-tls \ --runtime-plugins-enabled true \ diff --git a/_context/wiki/deployment.md b/_context/wiki/deployment.md index 8418c030..b0cef3b2 100644 --- a/_context/wiki/deployment.md +++ b/_context/wiki/deployment.md @@ -8,10 +8,8 @@ operation. There is no sticky-session requirement. Follow 1. Route the configured `/contextforge-rs` prefix to the external dataplane and keep older clients and legacy SSE on Python routes. -2. Configure `--jwks-url`, exact `--jwt-issuer`, and `--jwt-audiences`. Match - the principal and role/scope mapping to issued tokens and publisher IDs. - Tenant-aware persistent keys are still pending; this change alone does not - isolate tenants with colliding user IDs. +2. Configure a reachable trusted `--jwks-url` and a principal mapping matching + the publisher's user IDs and tenant claims. 3. Provide Redis connectivity and control-plane publication (`DATAPLANE_PUBLISHER=true` in the control-plane deployment). Restrict writes to trusted publishers; use TLS/mTLS across trust zones. diff --git a/_context/wiki/failure-modes.md b/_context/wiki/failure-modes.md index 1d5baaff..b625db0c 100644 --- a/_context/wiki/failure-modes.md +++ b/_context/wiki/failure-modes.md @@ -12,10 +12,8 @@ layers may return before the layer listed below is reached. | MCP standard-header count or byte budget exceeded | `431` | `mcp_header_limits_layer`. | | A request reaching virtual-host extraction does not match `/servers/{id}/mcp` | `400` | `virtual_host_id_layer`; unrelated router paths may instead be `404`. | | Missing Authorization or non-Bearer scheme | `401` | `claims_layer`. | -| Bad JWT, unsupported algorithm, unmatched key, wrong issuer/audience, missing exp, or invalid time claim | `401` `Invalid bearer token` | `claims_layer`; Bearer challenge included. | -| JWKS fetch/unusable document and no usable cached key | `503` | `claims_layer`; expired keys are never used. | -| Valid identity without MCPUser permission | `403` | `require_permission`, before configuration lookup. | -| Missing/empty/non-string user or tenant, conflicting tenant aliases, or malformed permission claims | `401` `Invalid token. Unable to extract the principal from claims` | `PrincipalExtractorLayer`. | +| Bad JWT, unsupported algorithm, no matching JWKS key, fetch failure, or invalid time claim | `401` `Invalid token` | `claims_layer`. | +| Missing/non-string mapped user or tenant | `401` `Invalid token. Unable to extract the principal from claims` | `PrincipalExtractorLayer`. | | Missing user configuration | `400` | `user_config_store_layer`, keyed by extracted user ID. | | Config cannot be decoded / key cannot be encoded | `500` | Config store / `user_config_store_layer`. | | Virtual host absent from caller's config | `404` `{"detail":"Server not found"}` | `virtual_host_config_layer`. | diff --git a/_context/wiki/getting-started.md b/_context/wiki/getting-started.md index a48ece1e..0dc69da4 100644 --- a/_context/wiki/getting-started.md +++ b/_context/wiki/getting-started.md @@ -68,7 +68,6 @@ cargo run -p contextforge-data-plane --features with_tools,plugins \ --redis-port 6379 \ --redis-mode plain-text \ --jwks-url http://127.0.0.1:8001/contextforge-rs/admin/.well-known/jwks.json \ - --jwt-issuer local-dev --jwt-audiences local-gateway \ --token-verification-private-key assets/jwt.key \ --upstream-connection-mode plain-text-or-tls \ --runtime-plugins-enabled true \ @@ -108,7 +107,7 @@ Expect `{"status": "healthy"}` and a JWKS document containing a `keys` array. The token response is a raw JWT, stored in `TOKEN` without printing it. Tokens expire after one hour; repeat the token command to refresh. Both tenant and user path segments are required. The helper sets top-level `tenant_id` and `sub` -claims, configured issuer/audiences, and `role: user`; the optional email does not select the user's Redis configuration. +claims; the optional email does not select the user's Redis configuration. Keep the listener, JWKS, token, and MCP URLs on the same instance. If you use port `9090`, change all four together. Port `8080` belongs to the full Docker @@ -244,8 +243,6 @@ then build and start the stack: ```bash export CONTEXTFORGE_DATA_PLANE_JWKS_URL=https://your-issuer.example/.well-known/jwks.json -export CONTEXTFORGE_DATA_PLANE_JWT_ISSUER=https://your-issuer.example -export CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES=your-gateway-audience make docker-prod make compose-up ``` diff --git a/_context/wiki/performance.md b/_context/wiki/performance.md index 889c5777..ab500ec0 100644 --- a/_context/wiki/performance.md +++ b/_context/wiki/performance.md @@ -85,34 +85,3 @@ standalone snapshot that is not being republished during the run. Record request rate, latency percentiles, failures, and resource usage after warmup. Do not infer correctness or protocol coverage from successful load; run [workspace and conformance checks](testing.md) separately. - -## Cached Authentication Probe (issue #753) - -The first downstream-authentication slice includes an explicitly invoked probe: - -```bash -cargo +1.96 test --locked -p contextforge-data-plane-lib --all-features \ - --test gateway cached_authentication_load -- --ignored --nocapture -``` - -This is an in-process router measurement in the debug profile: 100 warmup -requests, then 8,000 signed-RSA `server/discover` requests at concurrency 16 -on four Tokio workers. It uses a loopback JWKS server and an in-memory config -store, without Redis, plugins, or a routed backend. It is not a production or -Watson integration load test. `--all-features` here is for testing only. - -On 2026-09-20, three alternating runs compared main `9836fdc` with the first -implementation on `user/pratik-gandhi/downstream-authentication`, using the same -probe and claims (including both equivalent tenant aliases): - -| Measurement (median of three runs) | Baseline | Auth implementation | -| --- | ---: | ---: | -| Requests/second | 11,179 | 10,585 | -| Per-run p95 latency | 1,866 µs | 2,280 µs | -| JWKS fetches per run | 1 | 1 | -| Failed requests | 0 | 0 | - -The additional checks cost about 5.3% throughput in this short debug probe; -latency tails varied across runs. Repeat in release mode and with the real -Watson deployment before drawing capacity conclusions. Use separate Cargo target -directories for the two worktrees to avoid replacing each other's crate artifacts. diff --git a/_context/wiki/security.md b/_context/wiki/security.md index e980f19c..ac9ed7e2 100644 --- a/_context/wiki/security.md +++ b/_context/wiki/security.md @@ -18,35 +18,30 @@ user database. Configuration does not require a management API call per request, but verification can fetch keys from the trusted issuer's JWKS endpoint. The request path is Origin/header checks → JWT verification → principal -extraction → MCPUser permission → user configuration → virtual-host check → RMCP validation → +extraction → user configuration → virtual-host check → RMCP validation → published object route → backend call. -- JWT verification requires `exp`, the configured issuer and audience, and an - explicitly allowed RSA/EC algorithm (`RS256` by default). `nbf` is validated - when present. HMAC and token-directed key URLs are not supported. -- The default identity uses `sub`, or `woUserId` under an explicit profile. - Tenant aliases `woTenantId`, `tenant_id`, and `tenantId` must be nonempty - strings and agree. No tenant is inferred from teams or email. CEL handles - custom layouts through the same identity and permission checks. -- `admin` grants Admin and MCPUser; `builder`/`user` grant MCPUser. Every MCP - request, including discovery/list requests, needs MCPUser before config lookup. - Unknown/missing roles grant nothing. Explicit scope mapping can restrict roles; - scope-only authorization requires explicit opt-in. See [Configuration](config.md#jwt-claims-validated-by-claims_layer). +- JWT verification uses RSA/EC JWKS keys. HMAC secrets and the old public-key + CLI flag are not supported. `exp` and `nbf` are validated when present; no + fixed issuer/audience or mandatory expiration claim is enforced today. +- The default extractor requires a string user ID (`sub`, `user_id`, or + `UserId`) and tenant ID (`tenantId` or `tenant_id`). The first present alias + wins and must have the right type. User IDs need not be emails. CEL can + define a custom mapping; see [Configuration](config.md#jwt-claims-validated-by-claims_layer). - The Redis/cache key currently contains **only the extracted user ID**, not the tenant. Identical user IDs in different tenants resolve to the same stored configuration. Tenant extraction alone is not an isolation boundary. - The virtual host and each targeted tool, resource, or prompt must exist in that user's published routing maps. Publishing a backend alone does not expose all its objects. The dataplane does not derive routes by prefix. -- Object-level authorization still comes from published routing maps. CPEX - policy integration and tenant-aware keys in the +- JWT scopes, teams, and compiled RBAC are not independently enforced on this + path. Stronger isolation and policy checks in the [target authorization model](mcp-capability-allocation.md#target-authorization-invariants) - remain future work. -- There is no per-token revocation lookup. Keys are cached for five minutes; - refresh is serialized with a five-second cooldown. Expired cached keys never - authenticate during an outage. Known fresh keys remain usable. Removing a key - takes effect after refresh, expiry, or restart. Removing published user config - blocks access after its cache expires. + are proposed work, not current guarantees. +- There is no per-token blocklist/revocation lookup. Keys are cached for five + minutes; removing a JWKS key is not immediate invalidation until refresh or + restart. A token without `exp` has no expiration enforced by this verifier. + Removing a user's published configuration blocks access after cache expiry. ## What Compromise Means From 85f6df5fd66cfeaf4dcc239c20ec79b9b6e61a21 Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Wed, 23 Sep 2026 15:05:44 +0100 Subject: [PATCH 03/19] refactor: narrow downstream authentication PoC Signed-off-by: Pratik Gandhi --- .../src/authorization/jwks/jwks.rs | 79 ++----- .../src/authorization/jwks/jwks/tests.rs | 202 +++++------------ .../src/authorization/mod.rs | 7 +- .../cel_principal_extractor.rs | 74 +++--- .../default_principal_extractor.rs | 66 +----- .../authorization/principal_extractor/mod.rs | 87 +------ .../src/common/cli_config.rs | 20 -- .../src/common/config.rs | 8 - .../src/common/mod.rs | 12 - crates/contextforge-data-plane-lib/src/lib.rs | 16 +- .../tests/gateway/downstream_auth.rs | 212 ++---------------- .../tests/gateway/harness/mod.rs | 3 - docker/docker-compose.yml | 18 -- 13 files changed, 141 insertions(+), 663 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs index ac92321c..8afbe95f 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs @@ -2,15 +2,13 @@ use std::time::Duration; use futures::StreamExt as _; use jsonwebtoken::{ - AlgorithmFamily, DecodingKey, Header, Validation, decode, + Algorithm, DecodingKey, Header, Validation, decode, jwk::{Jwk, JwkSet, KeyOperations, PublicKeyUse}, }; +use lru_time_cache::LruCache; use reqwest::Url; use serde_json::Value; -use tokio::{ - sync::{Mutex, RwLock}, - time::Instant, -}; +use tokio::sync::RwLock; use crate::{ JwksConfig, @@ -18,51 +16,34 @@ use crate::{ }; const JWKS_CACHE_TTL: Duration = Duration::from_mins(5); -const JWKS_REFRESH_COOLDOWN: Duration = Duration::from_secs(5); +const JWKS_CACHE_KEY: &str = "jwks"; const JWKS_MAX_RESPONSE_BYTES: usize = 1024 * 1024; -struct CachedKeys { - keys: Vec, - expires_at: Instant, -} - -#[derive(Default)] -struct RefreshState { - last_attempt: Option, - failed: bool, -} - pub(super) struct Jwks { client: reqwest::Client, url: Url, validation: Validation, - cache: RwLock>, - // One refresh in flight, including concurrent unknown-kid requests. - refresh: Mutex, + cache: RwLock>>, } impl Jwks { pub fn new(client: reqwest::Client, url: Url, validation: Validation) -> Self { - Self { client, url, validation, cache: RwLock::new(None), refresh: Mutex::new(RefreshState::default()) } + Self { client, url, validation, cache: RwLock::new(LruCache::with_expiry_duration(JWKS_CACHE_TTL)) } } pub fn validation(config: &JwksConfig) -> Result { if config.issuer.trim().is_empty() || config.audiences.is_empty() || config.audiences.iter().any(|aud| aud.trim().is_empty()) - || config.algorithms.is_empty() - || config.algorithms.iter().any(|alg| !matches!(alg.family(), AlgorithmFamily::Rsa | AlgorithmFamily::Ec)) - || config.leeway_seconds > 300 { return Err(AuthorizationError::InvalidTrustConfiguration); } - let mut validation = Validation::new(config.algorithms[0]); - validation.algorithms.clone_from(&config.algorithms); + let mut validation = Validation::new(Algorithm::RS256); validation.set_required_spec_claims(&["exp", "iss", "aud"]); validation.set_issuer(&[&config.issuer]); validation.set_audience(&config.audiences); validation.validate_nbf = true; - validation.leeway = config.leeway_seconds; + validation.leeway = 30; Ok(validation) } @@ -72,49 +53,23 @@ impl Jwks { { return Err(AuthenticationError::InvalidToken); } - if let Some(result) = self.validate_cached(token, header).await { - return result; - } - - let mut refresh = self.refresh.lock().await; - // Another request may have loaded or rotated the keys while we waited. - if let Some(result) = self.validate_cached(token, header).await { - return result; - } - if refresh.last_attempt.is_some_and(|time| time.elapsed() < JWKS_REFRESH_COOLDOWN) { - return Err(if refresh.failed { - AuthenticationError::KeysUnavailable - } else { - AuthenticationError::InvalidToken - }); + { + let cache = self.cache.read().await; + if let Some(keys) = cache.peek(JWKS_CACHE_KEY) + && let Some(result) = self.validate_with_keys(keys, token, header) + { + return result; + } } - // Record before awaiting I/O so cancellation cannot bypass the cooldown. - refresh.last_attempt = Some(Instant::now()); - refresh.failed = true; - let result = fetch_jwks(&self.client, &self.url).await; - refresh.last_attempt = Some(Instant::now()); - refresh.failed = result.is_err(); - let keys = result.map_err(|_| { - // Do not log response bodies, key material, tokens, or URLs with query credentials. + let keys = fetch_jwks(&self.client, &self.url).await.map_err(|_| { tracing::warn!("jwks_refresh - unable to retrieve usable verification keys"); AuthenticationError::KeysUnavailable })?; let claims = self.validate_with_keys(&keys, token, header).unwrap_or(Err(AuthenticationError::InvalidToken)); - *self.cache.write().await = Some(CachedKeys { keys, expires_at: Instant::now() + JWKS_CACHE_TTL }); - tracing::info!("jwks_refresh - verification keys refreshed"); + self.cache.write().await.insert(JWKS_CACHE_KEY.to_owned(), keys); claims } - async fn validate_cached( - &self, - token: &str, - header: &Header, - ) -> Option> { - let cache = self.cache.read().await; - let cache = cache.as_ref().filter(|cache| cache.expires_at > Instant::now())?; - self.validate_with_keys(&cache.keys, token, header) - } - fn validate_with_keys( &self, keys: &[VerificationKey], diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs index 474821ac..d0a559d1 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs @@ -1,165 +1,83 @@ use super::*; -use axum::{Router, routing::get}; -use http::StatusCode; -use jsonwebtoken::{Algorithm, EncodingKey, encode}; +use jsonwebtoken::{EncodingKey, encode}; use serde_json::json; -use std::sync::{ - Arc, - atomic::{AtomicUsize, Ordering}, -}; -struct Fixture { - verifier: Jwks, - body: Arc)>>, - requests: Arc, - delay_ms: Arc, - task: tokio::task::JoinHandle<()>, -} -impl Drop for Fixture { - fn drop(&mut self) { - self.task.abort(); - } -} -impl Fixture { - async fn new(document: Value) -> Self { - let body = Arc::new(RwLock::new((StatusCode::OK, serde_json::to_vec(&document).unwrap()))); - let response = Arc::clone(&body); - let requests = Arc::new(AtomicUsize::new(0)); - let counter = Arc::clone(&requests); - let delay_ms = Arc::new(AtomicUsize::new(0)); - let delay = Arc::clone(&delay_ms); - let app = Router::new().route( - "/", - get(move || { - counter.fetch_add(1, Ordering::SeqCst); - let response = Arc::clone(&response); - let delay = delay.load(Ordering::SeqCst); - async move { - tokio::time::sleep(Duration::from_millis(delay as u64)).await; - response.read().await.clone() - } - }), - ); - let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); - let url = format!("http://{}/", listener.local_addr().unwrap()).parse().unwrap(); - let task = tokio::spawn(async move { - axum::serve(listener, app).await.unwrap(); - }); - let verifier = Jwks::new(reqwest::Client::new(), url, Jwks::validation(&JwksConfig::default()).unwrap()); - Self { verifier, body, requests, delay_ms, task } - } - async fn allow_refresh(&self) { - self.verifier.refresh.lock().await.last_attempt = None; - } - async fn expire(&self) { - self.verifier.cache.write().await.as_mut().unwrap().expires_at = Instant::now(); - self.allow_refresh().await; - } - async fn verify(&self, kid: &str) -> Result { - let (token, header) = token(kid); - self.verifier.validate(&token, &header).await - } -} -fn key() -> EncodingKey { +fn signing_key() -> EncodingKey { EncodingKey::from_rsa_pem(include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/../../assets/jwt.key"))).unwrap() } -fn document(kid: &str) -> Value { - let mut key = Jwk::from_encoding_key(&key(), Algorithm::RS256).unwrap(); - key.common.key_id = Some(kid.into()); - json!({"keys":[key]}) -} -fn token(kid: &str) -> (String, Header) { - let mut header = Header::new(Algorithm::RS256); - header.kid = Some(kid.into()); - let claims = json!({"iss":"mcpgateway","aud":"mcpgateway-api","exp":jsonwebtoken::get_current_timestamp()+3600}); - (encode(&header, &claims, &key()).unwrap(), header) -} - -#[tokio::test] -async fn concurrent_cold_requests_and_unknown_keys_have_bounded_refreshes() { - let fixture = Fixture::new(document("one")).await; - let results = futures::future::join_all((0..32).map(|_| fixture.verify("one"))).await; - assert!(results.into_iter().all(|result| result.is_ok())); - assert_eq!(fixture.requests.load(Ordering::SeqCst), 1); - fixture.allow_refresh().await; - let results = futures::future::join_all((0..32).map(|_| fixture.verify("unknown"))).await; - assert!(results.into_iter().all(|result| result == Err(AuthenticationError::InvalidToken))); - assert_eq!(fixture.requests.load(Ordering::SeqCst), 2); - assert!(fixture.verify("one").await.is_ok()); -} -#[tokio::test] -async fn rotation_replaces_keys_and_expired_keys_fail_closed_during_outage() { - let fixture = Fixture::new(document("one")).await; - assert!(fixture.verify("one").await.is_ok()); - *fixture.body.write().await = (StatusCode::OK, serde_json::to_vec(&document("two")).unwrap()); - fixture.allow_refresh().await; - assert!(fixture.verify("two").await.is_ok()); - assert_eq!(fixture.verify("one").await, Err(AuthenticationError::InvalidToken)); - *fixture.body.write().await = (StatusCode::SERVICE_UNAVAILABLE, Vec::new()); - fixture.allow_refresh().await; - assert_eq!(fixture.verify("unknown").await, Err(AuthenticationError::KeysUnavailable)); - // Known, unexpired keys remain usable during a failed refresh for a different kid. - assert!(fixture.verify("two").await.is_ok()); - fixture.expire().await; - assert_eq!(fixture.verify("two").await, Err(AuthenticationError::KeysUnavailable)); - *fixture.body.write().await = (StatusCode::OK, serde_json::to_vec(&document("two")).unwrap()); - fixture.allow_refresh().await; - assert!(fixture.verify("two").await.is_ok()); +fn public_key() -> Jwk { + let mut jwk = Jwk::from_encoding_key(&signing_key(), Algorithm::RS256).unwrap(); + jwk.common.key_id = Some("test".into()); + jwk } #[tokio::test] -async fn rejects_unusable_key_sets_and_declared_algorithm_mismatch() { - let document = document("one"); - let mut duplicate = document.clone(); - duplicate["keys"].as_array_mut().unwrap().push(document["keys"][0].clone()); - let mut encryption_key = document.clone(); - encryption_key["keys"][0]["use"] = "enc".into(); - let mut non_verify = document.clone(); - non_verify["keys"][0]["key_ops"] = json!(["sign"]); - let mut no_kid = document.clone(); - no_kid["keys"][0].as_object_mut().unwrap().remove("kid"); - for document in [json!({"keys":[]}), duplicate, encryption_key, non_verify, no_kid] { - let fixture = Fixture::new(document).await; - assert_eq!(fixture.verify("one").await, Err(AuthenticationError::KeysUnavailable)); +async fn verifies_signed_tokens_and_requires_trusted_claims() { + let config = JwksConfig::default(); + let verifier = Jwks::new(reqwest::Client::new(), config.url.clone(), Jwks::validation(&config).unwrap()); + verifier + .cache + .write() + .await + .insert(JWKS_CACHE_KEY.to_owned(), vec![VerificationKey::from_jwk(&public_key()).unwrap().unwrap()]); + let mut header = Header::new(Algorithm::RS256); + header.kid = Some("test".into()); + let claims = json!({"iss":"mcpgateway", "aud":"mcpgateway-api", "exp":jsonwebtoken::get_current_timestamp()+3600}); + let signed = |claims: &Value, header: &Header| encode(header, claims, &signing_key()).unwrap(); + assert!(verifier.validate(&signed(&claims, &header), &header).await.is_ok()); + let mut invalid = Vec::new(); + for (name, value) in + [("iss", json!("other")), ("aud", json!("other")), ("exp", json!(1)), ("nbf", json!(9_999_999_999_u64))] + { + let mut modified = claims.clone(); + modified[name] = value; + invalid.push(modified); } - let mut mismatch = document.clone(); - mismatch["keys"][0]["alg"] = "RS384".into(); - let fixture = Fixture::new(mismatch).await; - assert_eq!(fixture.verify("one").await, Err(AuthenticationError::InvalidToken)); + for name in ["iss", "aud", "exp"] { + let mut modified = claims.clone(); + modified.as_object_mut().unwrap().remove(name); + invalid.push(modified); + } + for claims in invalid { + assert_eq!(verifier.validate(&signed(&claims, &header), &header).await, Err(AuthenticationError::InvalidToken)); + } + let mut wrong_algorithm = header.clone(); + wrong_algorithm.alg = Algorithm::RS384; + assert_eq!( + verifier.validate(&signed(&claims, &wrong_algorithm), &wrong_algorithm).await, + Err(AuthenticationError::InvalidToken) + ); + header.kid = None; + assert_eq!(verifier.validate(&signed(&claims, &header), &header).await, Err(AuthenticationError::InvalidToken)); } -#[tokio::test] -async fn bounded_response_and_malformed_json_are_unavailable() { - for body in [b"not-json".to_vec(), vec![b' '; JWKS_MAX_RESPONSE_BYTES + 1]] { - let fixture = Fixture::new(document("one")).await; - *fixture.body.write().await = (StatusCode::OK, body); - assert_eq!(fixture.verify("one").await, Err(AuthenticationError::KeysUnavailable)); - assert_eq!(fixture.verify("one").await, Err(AuthenticationError::KeysUnavailable)); - assert_eq!(fixture.requests.load(Ordering::SeqCst), 1); - } +#[test] +fn selects_only_the_matching_signing_key() { + let mut jwk = public_key(); + let mut header = Header::new(Algorithm::RS256); + header.kid = Some("test".into()); + assert!(VerificationKey::from_jwk(&jwk).unwrap().unwrap().matches(&header)); + header.kid = Some("other".into()); + assert!(!VerificationKey::from_jwk(&jwk).unwrap().unwrap().matches(&header)); + header.kid = Some("test".into()); + jwk.common.key_algorithm = Some(jsonwebtoken::jwk::KeyAlgorithm::RS384); + assert!(!VerificationKey::from_jwk(&jwk).unwrap().unwrap().matches(&header)); + jwk.common.public_key_use = Some(PublicKeyUse::Encryption); + assert!(VerificationKey::from_jwk(&jwk).unwrap().is_none()); + jwk = public_key(); + jwk.common.key_id = None; + assert!(VerificationKey::from_jwk(&jwk).unwrap().is_none()); } #[test] -fn rejects_incomplete_or_unsafe_trust_configuration() { +fn requires_explicit_issuer_and_audience() { let config = JwksConfig::default(); for config in [ JwksConfig { issuer: String::new(), ..config.clone() }, JwksConfig { audiences: vec![], ..config.clone() }, - JwksConfig { audiences: vec![String::new()], ..config.clone() }, - JwksConfig { algorithms: vec![], ..config.clone() }, - JwksConfig { algorithms: vec![Algorithm::HS256], ..config.clone() }, - JwksConfig { leeway_seconds: 301, ..config }, + JwksConfig { audiences: vec![String::new()], ..config }, ] { assert!(Jwks::validation(&config).is_err()); } } - -#[tokio::test] -async fn cancelled_fetch_cannot_bypass_refresh_cooldown() { - let fixture = Fixture::new(document("one")).await; - fixture.delay_ms.store(5000, Ordering::SeqCst); - assert!(tokio::time::timeout(Duration::from_millis(100), fixture.verify("one")).await.is_err()); - assert_eq!(fixture.verify("one").await, Err(AuthenticationError::KeysUnavailable)); - assert_eq!(fixture.requests.load(Ordering::SeqCst), 1); -} diff --git a/crates/contextforge-data-plane-lib/src/authorization/mod.rs b/crates/contextforge-data-plane-lib/src/authorization/mod.rs index 61c478c8..7f070373 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/mod.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/mod.rs @@ -12,8 +12,7 @@ mod jwks; mod principal_extractor; pub use principal_extractor::{ - AuthorizedPrincipal, CelPrincipalExtractor, DefaultPrincipalExtractor, Permission, PrincipalConfig, - PrincipalExtractor, ScopeMapping, UserClaim, + AuthorizedPrincipal, CelPrincipalExtractor, DefaultPrincipalExtractor, Permission, PrincipalExtractor, }; pub fn get_authorization_service( @@ -39,9 +38,7 @@ pub enum AuthenticationError { #[derive(Debug, thiserror::Error)] #[allow(dead_code)] pub enum AuthorizationError { - #[error( - "JWT trust configuration requires a nonempty issuer, audiences, and RSA/EC algorithms; leeway must be at most 300 seconds" - )] + #[error("JWT trust configuration requires a nonempty issuer and audience")] InvalidTrustConfiguration, #[error("JWKS contains duplicate signing key IDs")] DuplicateKeyId, diff --git a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/cel_principal_extractor.rs b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/cel_principal_extractor.rs index bea2a5f0..0bfa82e7 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/cel_principal_extractor.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/cel_principal_extractor.rs @@ -7,7 +7,7 @@ use serde_json::Value as JsonValue; use thiserror::Error; use tracing::instrument; -use super::{AuthorizedPrincipal, PrincipalConfig, PrincipalExtractor, UserClaim}; +use super::{AuthorizedPrincipal, PrincipalExtractor}; #[derive(Error, Debug)] pub enum CelPrincipalExtractorError { @@ -24,23 +24,14 @@ pub enum CelPrincipalExtractorError { InvalidReturnType(JsonValue), } -/// Trusted CEL mapping for nonstandard claims. Return `user_id`, `tenant_id`, -/// and optional `role`, `roles`, `scope`, `permissions`. Permissions are always -/// computed by the same policy as the default extractor. The verified issuer -/// is taken from the original claims and cannot be overridden by CEL. +/// CEL maps user_id and tenant_id. Roles and issuer always come from the +/// original verified claims and use the same policy as the default extractor. #[derive(Clone, Debug)] pub struct CelPrincipalExtractor { program: Arc, - config: PrincipalConfig, } impl CelPrincipalExtractor { - pub fn with_config(mut self, mut config: PrincipalConfig) -> Self { - config.user_claim = UserClaim::Sub; - self.config = config; - self - } - pub fn from_file>(path: P) -> Result { let expression = fs::read_to_string(path)?; Self::from_expression(&expression) @@ -50,7 +41,7 @@ impl CelPrincipalExtractor { let program = Program::compile(expression).map_err(|e| CelPrincipalExtractorError::CompilationError(e.to_string()))?; - Ok(Self { program: Arc::new(program), config: PrincipalConfig::default() }) + Ok(Self { program: Arc::new(program) }) } } @@ -73,29 +64,19 @@ impl PrincipalExtractor for CelPrincipalExtractor { return Err(CelPrincipalExtractorError::InvalidReturnType(JsonValue::Null).into()); }; let mut normalized = serde_json::Map::new(); - for name in ["user_id", "tenant_id", "role", "roles", "scope", "permissions"] { - if let Some(value) = map.get(&Key::from(name.to_owned())) { - let value = match value { - cel::Value::String(value) => JsonValue::String(value.to_string()), - cel::Value::List(values) => JsonValue::Array( - values - .iter() - .map(|value| { - if let cel::Value::String(value) = value { - JsonValue::String(value.to_string()) - } else { - JsonValue::Null - } - }) - .collect(), - ), - _ => JsonValue::Null, - }; - normalized.insert(if name == "user_id" { "sub" } else { name }.to_owned(), value); + for (name, claim) in [("user_id", "sub"), ("tenant_id", "tenant_id")] { + let value = match map.get(&Key::from(name.to_owned())) { + Some(cel::Value::String(value)) => JsonValue::String(value.to_string()), + _ => JsonValue::Null, + }; + normalized.insert(claim.to_owned(), value); + } + for name in ["iss", "role", "roles"] { + if let Some(value) = claims.get(name) { + normalized.insert(name.to_owned(), value.clone()); } } - normalized.insert("iss".to_owned(), claims.get("iss").cloned().unwrap_or(JsonValue::Null)); - Ok(AuthorizedPrincipal::from_claims(&JsonValue::Object(normalized), &self.config)?) + Ok(AuthorizedPrincipal::from_claims(&JsonValue::Object(normalized))?) } } @@ -172,19 +153,18 @@ mod permission_tests { use crate::Permission; #[test] - fn nested_roles_use_common_policy_and_cannot_override_issuer() { - let extractor = CelPrincipalExtractor::from_expression(r#"{"user_id": claims.sub, "tenant_id": claims.woTenantId, "roles": claims.user.roles, "iss": "untrusted"}"#).unwrap(); - let principal = extractor - .extract(&serde_json::json!({"sub":"user","woTenantId":"tenant","iss":"watson","user":{"roles":["admin"]}})) - .unwrap(); + fn mapping_cannot_override_verified_roles_or_issuer() { + let extractor = CelPrincipalExtractor::from_expression( + r#"{"user_id": claims.sub, "tenant_id": claims.woTenantId, "role": "admin", "iss": "other"}"#, + ) + .unwrap(); + let claims = serde_json::json!({"sub":"user","woTenantId":"tenant","iss":"watson","roles":["user"]}); + let principal = extractor.extract(&claims).unwrap(); assert_eq!(principal.issuer(), "watson"); - assert!(principal.has_permission(Permission::Admin)); - let extractor = - CelPrincipalExtractor::from_expression(r#"{"user_id": claims.sub, "tenant_id": claims.woTenantId}"#) - .unwrap(); - let principal = extractor - .extract(&serde_json::json!({"sub":"user","woTenantId":"tenant","iss":"watson","role":"admin"})) - .unwrap(); - assert!(!principal.has_permission(Permission::MCPUser)); + assert!(principal.has_permission(Permission::MCPUser)); + assert!(!principal.has_permission(Permission::Admin)); + let mut no_roles = claims; + no_roles.as_object_mut().unwrap().remove("roles"); + assert!(!extractor.extract(&no_roles).unwrap().has_permission(Permission::MCPUser)); } } diff --git a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/default_principal_extractor.rs b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/default_principal_extractor.rs index 8b7e0c4c..2d7b2516 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/default_principal_extractor.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/default_principal_extractor.rs @@ -1,16 +1,8 @@ -use super::{AuthorizedPrincipal, PrincipalConfig, PrincipalExtractor}; +use super::{AuthorizedPrincipal, PrincipalExtractor}; use tracing::instrument; #[derive(Debug, Clone, Default)] -pub struct DefaultPrincipalExtractor { - config: PrincipalConfig, -} - -impl DefaultPrincipalExtractor { - pub fn new(config: PrincipalConfig) -> Self { - Self { config } - } -} +pub struct DefaultPrincipalExtractor {} impl PrincipalExtractor for DefaultPrincipalExtractor { #[instrument(name = "principal_extract", level = "info", skip_all)] @@ -18,14 +10,14 @@ impl PrincipalExtractor for DefaultPrincipalExtractor { &self, claims: &serde_json::Value, ) -> Result> { - Ok(AuthorizedPrincipal::from_claims(claims, &self.config)?) + Ok(AuthorizedPrincipal::from_claims(claims)?) } } #[cfg(test)] mod tests { use super::*; - use crate::authorization::{Permission, ScopeMapping, UserClaim}; + use crate::authorization::Permission; use serde_json::{Value, json}; fn claims() -> Value { @@ -60,11 +52,13 @@ mod tests { } #[test] - fn identity_profile_and_tenant_aliases_are_strict() { + fn subject_and_tenant_aliases_are_required() { let mut claims = claims(); - let extractor = - DefaultPrincipalExtractor::new(PrincipalConfig { user_claim: UserClaim::WoUserId, ..Default::default() }); - assert_eq!(extractor.extract(&claims).unwrap().user_id(), "watson-user"); + let extractor = DefaultPrincipalExtractor {}; + assert_eq!(extractor.extract(&claims).unwrap().user_id(), "subject"); + let mut missing_sub = claims.clone(); + missing_sub.as_object_mut().unwrap().remove("sub"); + assert!(extractor.extract(&missing_sub).is_err()); claims["tenant_id"] = "tenant".into(); assert!(extractor.extract(&claims).is_ok()); claims["tenantId"] = "different".into(); @@ -82,45 +76,9 @@ mod tests { } } - #[test] - fn scopes_only_grant_permissions_when_configured() { - let mut claims = claims(); - claims["role"] = "unknown".into(); - claims["scope"] = "aipg.admin unrelated".into(); - assert!(!DefaultPrincipalExtractor::default().extract(&claims).unwrap().has_permission(Permission::Admin)); - let mapping = ScopeMapping { admin: vec!["aipg.admin".into()], mcp_user: vec!["aipg.mcp".into()] }; - let mut config = PrincipalConfig { scope_mapping: Some(mapping), ..Default::default() }; - assert!( - !DefaultPrincipalExtractor::new(config.clone()).extract(&claims).unwrap().has_permission(Permission::Admin) - ); - config.scopes_only = true; - assert!( - DefaultPrincipalExtractor::new(config.clone()).extract(&claims).unwrap().has_permission(Permission::Admin) - ); - claims["scope"] = "".into(); - claims["permissions"] = json!(["aipg.mcp"]); - let principal = DefaultPrincipalExtractor::new(config.clone()).extract(&claims).unwrap(); - assert!(!principal.has_permission(Permission::Admin)); - assert!(principal.has_permission(Permission::MCPUser)); - assert_eq!(principal.scopes(), &["aipg.mcp"]); - config.scopes_only = false; - claims["role"] = "admin".into(); - assert!( - !DefaultPrincipalExtractor::new(config.clone()).extract(&claims).unwrap().has_permission(Permission::Admin) - ); - claims["permissions"] = json!([]); - assert!(!DefaultPrincipalExtractor::new(config).extract(&claims).unwrap().has_permission(Permission::MCPUser)); - } - #[test] fn malformed_permission_claims_are_rejected() { - for (name, value) in [ - ("role", json!(["admin"])), - ("roles", json!("admin")), - ("roles", json!([42])), - ("scope", json!(["aipg.admin"])), - ("permissions", json!({"admin":true})), - ] { + for (name, value) in [("role", json!(["admin"])), ("roles", json!("admin")), ("roles", json!([42]))] { let mut claims = claims(); claims[name] = value; assert!(DefaultPrincipalExtractor::default().extract(&claims).is_err(), "{name}"); @@ -128,7 +86,7 @@ mod tests { } #[test] - fn debug_does_not_disclose_identity_or_scopes() { + fn debug_does_not_disclose_identity() { let principal = DefaultPrincipalExtractor::default().extract(&claims()).unwrap(); let debug = format!("{principal:?}"); for sensitive in ["watson", "subject", "tenant"] { diff --git a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/mod.rs b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/mod.rs index 78b88d0d..f16a5b8e 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/mod.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/mod.rs @@ -3,7 +3,6 @@ mod default_principal_extractor; use std::collections::BTreeSet; -use clap::ValueEnum; use contextforge_data_plane_apis::User; use serde_json::Value; @@ -16,65 +15,14 @@ pub enum Permission { MCPUser, } -#[derive(Debug, Clone, Copy, Default, ValueEnum)] -pub enum UserClaim { - #[default] - Sub, - WoUserId, -} - -impl UserClaim { - pub fn as_str(self) -> &'static str { - match self { - Self::Sub => "sub", - Self::WoUserId => "woUserId", - } - } -} - -/// Scope names are an explicit deployment contract, not inferred from arbitrary scopes. -#[derive(Debug, Clone, Default)] -pub struct ScopeMapping { - pub admin: Vec, - pub mcp_user: Vec, -} - -#[derive(Debug, Clone, Default)] -pub struct PrincipalConfig { - pub user_claim: UserClaim, - /// When configured, intersect role permissions with the mapped scope permissions. - pub scope_mapping: Option, - /// Explicit opt-in to granting permissions using scopes without roles. - pub scopes_only: bool, -} - -impl PrincipalConfig { - pub fn validate(&self) -> Result<(), PrincipalError> { - if self.scopes_only && self.scope_mapping.is_none() - || self.scope_mapping.as_ref().is_some_and(|mapping| { - mapping - .admin - .iter() - .chain(&mapping.mcp_user) - .any(|scope| scope.is_empty() || scope.chars().any(char::is_whitespace)) - }) - { - return Err(PrincipalError::InvalidConfiguration); - } - Ok(()) - } -} - #[derive(Debug, thiserror::Error)] pub enum PrincipalError { #[error("missing or malformed identity claim")] InvalidIdentity, #[error("conflicting tenant claims")] ConflictingTenant, - #[error("malformed role or scope claims")] + #[error("malformed role claims")] InvalidPermissions, - #[error("invalid principal mapping configuration")] - InvalidConfiguration, } /// Request-local identity derived only from verified claims and trusted mapping configuration. @@ -84,7 +32,6 @@ pub struct AuthorizedPrincipal { issuer: String, user_id: String, tenant_id: String, - scopes: Vec, permissions: BTreeSet, } @@ -104,16 +51,13 @@ impl AuthorizedPrincipal { pub fn tenant_id(&self) -> &str { &self.tenant_id } - pub fn scopes(&self) -> &[String] { - &self.scopes - } pub fn has_permission(&self, permission: Permission) -> bool { self.permissions.contains(&permission) } - fn from_claims(claims: &Value, config: &PrincipalConfig) -> Result { + fn from_claims(claims: &Value) -> Result { let issuer = identity_string(claims.get("iss"))?; - let user_id = identity_string(claims.get(config.user_claim.as_str()))?; + let user_id = identity_string(claims.get("sub"))?; let mut tenant_id = None; for name in ["woTenantId", "tenant_id", "tenantId"] { if let Some(value) = claims.get(name) { @@ -129,14 +73,6 @@ impl AuthorizedPrincipal { if let Some(role) = claims.get("role") { roles.push(role.as_str().ok_or(PrincipalError::InvalidPermissions)?.to_owned()); } - let mut scopes = string_list(claims.get("permissions"))?; - if let Some(scope) = claims.get("scope") { - scopes.extend( - scope.as_str().ok_or(PrincipalError::InvalidPermissions)?.split_ascii_whitespace().map(str::to_owned), - ); - } - scopes.sort_unstable(); - scopes.dedup(); let mut permissions = BTreeSet::new(); for role in roles { match role.as_str() { @@ -149,27 +85,10 @@ impl AuthorizedPrincipal { _ => {}, } } - if let Some(mapping) = &config.scope_mapping { - let mut scope_permissions = BTreeSet::new(); - if mapping.admin.iter().any(|scope| scopes.contains(scope)) { - scope_permissions.extend([Permission::Admin, Permission::MCPUser]); - } - if mapping.mcp_user.iter().any(|scope| scopes.contains(scope)) { - scope_permissions.insert(Permission::MCPUser); - } - if config.scopes_only { - permissions = scope_permissions; - } else { - permissions.retain(|permission| scope_permissions.contains(permission)); - } - } else if config.scopes_only { - return Err(PrincipalError::InvalidConfiguration); - } Ok(Self { issuer: issuer.to_owned(), user_id: user_id.to_owned(), tenant_id: tenant_id.to_owned(), - scopes, permissions, }) } diff --git a/crates/contextforge-data-plane-lib/src/common/cli_config.rs b/crates/contextforge-data-plane-lib/src/common/cli_config.rs index 91bdaff9..74921777 100644 --- a/crates/contextforge-data-plane-lib/src/common/cli_config.rs +++ b/crates/contextforge-data-plane-lib/src/common/cli_config.rs @@ -25,26 +25,6 @@ pub struct CliConfig { #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES", value_delimiter = ',', required = true)] pub jwt_audiences: Vec, - #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_ALGORITHMS", value_delimiter = ',', default_value = "RS256")] - pub jwt_algorithms: Vec, - - #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_LEEWAY_SECONDS", default_value_t = 30)] - pub jwt_leeway_seconds: u64, - - #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_USER_CLAIM", value_enum, default_value = "sub")] - pub jwt_user_claim: crate::authorization::UserClaim, - - /// When either scope mapping is set, scopes restrict role-derived permissions. - #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_ADMIN_SCOPES", value_delimiter = ',')] - pub jwt_admin_scopes: Option>, - - #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_MCP_USER_SCOPES", value_delimiter = ',')] - pub jwt_mcp_user_scopes: Option>, - - /// Explicitly authorize using configured scope mappings instead of roles. - #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_SCOPES_ONLY", default_value_t = false)] - pub jwt_scopes_only: bool, - #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_ENABLE_OPEN_TELEMETRY")] pub enable_open_telemetry: Option, diff --git a/crates/contextforge-data-plane-lib/src/common/config.rs b/crates/contextforge-data-plane-lib/src/common/config.rs index 1ed30023..55186540 100644 --- a/crates/contextforge-data-plane-lib/src/common/config.rs +++ b/crates/contextforge-data-plane-lib/src/common/config.rs @@ -148,8 +148,6 @@ pub struct JwksConfig { pub ca_cert_path: Option, pub issuer: String, pub audiences: Vec, - pub algorithms: Vec, - pub leeway_seconds: u64, } impl From<&CliConfig> for JwksConfig { fn from(value: &CliConfig) -> Self { @@ -159,8 +157,6 @@ impl From<&CliConfig> for JwksConfig { ca_cert_path: jwks_ca_cert_path, issuer: value.jwt_issuer.clone(), audiences: value.jwt_audiences.clone(), - algorithms: value.jwt_algorithms.clone(), - leeway_seconds: value.jwt_leeway_seconds, } } } @@ -171,7 +167,6 @@ pub struct Config { pub observability_config: ObservabilityConfig, pub jwks_config: JwksConfig, - pub principal_config: crate::authorization::PrincipalConfig, /// Expiry in seconds for the in-process user config cache in front of /// Redis. The control-plane dataplane publisher rewrites UserConfig keys @@ -421,8 +416,6 @@ mod tests { ca_cert_path: None, issuer: "mcpgateway".to_owned(), audiences: vec!["mcpgateway-api".to_owned()], - algorithms: vec![jsonwebtoken::Algorithm::RS256], - leeway_seconds: 30, } } } @@ -432,7 +425,6 @@ mod tests { Self { address: None, jwks_config: super::JwksConfig::default(), - principal_config: crate::authorization::PrincipalConfig::default(), observability_config: super::ObservabilityConfig::default(), mcp_standard_header_max_count: 10, mcp_standard_header_max_value_bytes: 4096, diff --git a/crates/contextforge-data-plane-lib/src/common/mod.rs b/crates/contextforge-data-plane-lib/src/common/mod.rs index f2110452..9deb9265 100644 --- a/crates/contextforge-data-plane-lib/src/common/mod.rs +++ b/crates/contextforge-data-plane-lib/src/common/mod.rs @@ -64,17 +64,6 @@ impl TryFrom for Config { let downstream_transport_config = DownstreamTransportConfig::from(&value); let upstream_transport_config = UpstreamTransportConfig::from(&value); let jwks_config = JwksConfig::from(&value); - let principal_config = crate::authorization::PrincipalConfig { - user_claim: value.jwt_user_claim, - scopes_only: value.jwt_scopes_only, - scope_mapping: (value.jwt_admin_scopes.is_some() || value.jwt_mcp_user_scopes.is_some()).then(|| { - crate::authorization::ScopeMapping { - admin: value.jwt_admin_scopes.clone().unwrap_or_default(), - mcp_user: value.jwt_mcp_user_scopes.clone().unwrap_or_default(), - } - }), - }; - principal_config.validate()?; let CliConfig { address, runtime_plugins_enabled, @@ -87,7 +76,6 @@ impl TryFrom for Config { Ok(Self { address, jwks_config, - principal_config, observability_config, downstream_transport_config, upstream_transport_config, diff --git a/crates/contextforge-data-plane-lib/src/lib.rs b/crates/contextforge-data-plane-lib/src/lib.rs index 74e131c7..22a92232 100644 --- a/crates/contextforge-data-plane-lib/src/lib.rs +++ b/crates/contextforge-data-plane-lib/src/lib.rs @@ -53,8 +53,8 @@ use crate::{ }, }; pub use authorization::{ - AuthenticationError, AuthorizationClaims, AuthorizationService, AuthorizedPrincipal, Permission, PrincipalConfig, - ScopeMapping, UserClaim, get_authorization_service, + AuthenticationError, AuthorizationClaims, AuthorizationService, AuthorizedPrincipal, Permission, + get_authorization_service, }; pub use layers::permission::require_permission; @@ -113,7 +113,6 @@ impl Gateway { /// callers bind listeners before starting the service. pub async fn into_router(self) -> Result { let Gateway { config, session_manager, user_config_store_type, plugin_runtime, authorization_service } = self; - config.principal_config.validate()?; let user_config_store = match user_config_store_type { UserConfigStoreType::Redis => Arc::new(get_config_store(&config).await?), UserConfigStoreType::Test(store) => store, @@ -152,14 +151,11 @@ impl Gateway { .layer(middleware::from_fn_with_state(Permission::MCPUser, require_permission)); let app = if let Some(cel_principal_extractor_path) = config.cel_principal_extractor_path.as_ref() { - app.layer(layers::PrincipalExtractorLayer::new( - CelPrincipalExtractor::from_file(cel_principal_extractor_path)? - .with_config(config.principal_config.clone()), - )) + app.layer(layers::PrincipalExtractorLayer::new(CelPrincipalExtractor::from_file( + cel_principal_extractor_path, + )?)) } else { - app.layer(layers::PrincipalExtractorLayer::new(DefaultPrincipalExtractor::new( - config.principal_config.clone(), - ))) + app.layer(layers::PrincipalExtractorLayer::new(DefaultPrincipalExtractor {})) }; let app = app diff --git a/crates/contextforge-data-plane-lib/tests/gateway/downstream_auth.rs b/crates/contextforge-data-plane-lib/tests/gateway/downstream_auth.rs index 165aa223..58904dc6 100644 --- a/crates/contextforge-data-plane-lib/tests/gateway/downstream_auth.rs +++ b/crates/contextforge-data-plane-lib/tests/gateway/downstream_auth.rs @@ -109,212 +109,19 @@ fn request(token: Option<&str>, method: &str) -> Request { .expect("valid authentication test fixture") } -async fn key_server() -> (TestServer, Arc) { +async fn key_server() -> TestServer { let mut jwk = Jwk::from_encoding_key(&key(), Algorithm::RS256).expect("valid authentication test fixture"); jwk.common.key_id = Some("test".into()); let document = json!({"keys":[jwk]}); - let requests = Arc::new(AtomicUsize::new(0)); - let counter = Arc::clone(&requests); - let server = TestServer::start_http(Router::new().route( + TestServer::start_http(Router::new().route( "/jwks", get(move || { - counter.fetch_add(1, Ordering::SeqCst); let document = document.clone(); async move { Json(document) } }), )) .await - .expect("valid authentication test fixture"); - (server, requests) -} - -#[tokio::test] -async fn authenticates_real_rsa_tokens_and_rejects_before_configuration() { - let (server, requests) = key_server().await; - let mut config = create_default_config(); - config.jwks_config.url = server.url("/jwks").parse().expect("valid authentication test fixture"); - let store = CountingStore::default(); - let app = gateway(config, store.clone()).await; - // Warm both authentication and the request path, then deny access on every subsequent request. - let valid = token(&claims(), Some("test"), Algorithm::RS256); - let response = - app.clone().oneshot(request(Some(&valid), "server/discover")).await.expect("valid authentication test fixture"); - let status = response.status(); - let body = axum::body::to_bytes(response.into_body(), 65_536).await.expect("valid authentication test fixture"); - assert_eq!(status, StatusCode::OK, "{}", String::from_utf8_lossy(&body)); - assert_eq!(store.reads.load(Ordering::SeqCst), 1); - for method in ["server/discover", "tools/list", "tools/call", "resources/list", "prompts/list"] { - let mut denied = claims(); - denied["role"] = "unknown".into(); - let denied = token(&denied, Some("test"), Algorithm::RS256); - assert_eq!( - app.clone() - .oneshot(request(Some(&denied), method)) - .await - .expect("valid authentication test fixture") - .status(), - StatusCode::FORBIDDEN, - "{method}" - ); - } - let mut invalid = vec![ - None, - Some("malformed".into()), - Some(token(&claims(), None, Algorithm::RS256)), - Some(token(&claims(), Some("unknown"), Algorithm::RS256)), - Some(token(&claims(), Some("test"), Algorithm::RS384)), - ]; - for (name, value) in [ - ("iss", json!("wrong")), - ("aud", json!("wrong")), - ("exp", json!(1)), - ("nbf", json!(9_999_999_999_u64)), - ("nbf", json!("tomorrow")), - ("sub", json!("")), - ("woTenantId", json!(null)), - ("tenant_id", json!("conflicting")), - ] { - let mut c = claims(); - c[name] = value; - invalid.push(Some(token(&c, Some("test"), Algorithm::RS256))); - } - for missing in ["iss", "aud", "exp", "sub", "woTenantId"] { - let mut c = claims(); - c.as_object_mut().expect("valid authentication test fixture").remove(missing); - if missing == "woTenantId" { - c.as_object_mut().expect("claims object").remove("tenant_id"); - } - invalid.push(Some(token(&c, Some("test"), Algorithm::RS256))); - } - let mut tampered = valid.clone().into_bytes(); - let position = tampered.iter().rposition(|b| *b == b'.').expect("valid authentication test fixture") + 1; - tampered[position] = if tampered[position] == b'A' { b'B' } else { b'A' }; - invalid.push(Some(String::from_utf8(tampered).expect("valid authentication test fixture"))); - for token in invalid { - let response = app - .clone() - .oneshot(request(token.as_deref(), "server/discover")) - .await - .expect("valid authentication test fixture"); - assert_eq!(response.status(), StatusCode::UNAUTHORIZED); - assert_eq!(response.headers()["www-authenticate"], "Bearer"); - } - let mut ambiguous = request(Some(&valid), "server/discover"); - ambiguous - .headers_mut() - .append("authorization", format!("Bearer {valid}").parse().expect("valid authentication test fixture")); - assert_eq!( - app.clone().oneshot(ambiguous).await.expect("valid authentication test fixture").status(), - StatusCode::UNAUTHORIZED - ); - assert_eq!(store.reads.load(Ordering::SeqCst), 1, "denials must not access configuration"); - assert_eq!(requests.load(Ordering::SeqCst), 1, "valid, invalid and unknown kid traffic uses a bounded cache"); - for role in ["admin", "builder", "user"] { - let mut c = claims(); - c["role"] = role.into(); - c["aud"] = json!(["unrelated", "mcpgateway-api"]); - let token = token(&c, Some("test"), Algorithm::RS256); - assert_eq!( - app.clone() - .oneshot(request(Some(&token), "server/discover")) - .await - .expect("valid authentication test fixture") - .status(), - StatusCode::OK - ); - } - server.shutdown().await.expect("valid authentication test fixture"); -} - -#[tokio::test] -async fn jwks_outage_is_503_and_never_reaches_configuration() { - let requests = Arc::new(AtomicUsize::new(0)); - let count = Arc::clone(&requests); - let server = TestServer::start_http(Router::new().route( - "/jwks", - get(move || { - count.fetch_add(1, Ordering::SeqCst); - async { StatusCode::SERVICE_UNAVAILABLE } - }), - )) - .await - .expect("valid authentication test fixture"); - let mut config = create_default_config(); - config.jwks_config.url = server.url("/jwks").parse().expect("valid authentication test fixture"); - let store = CountingStore::default(); - let app = gateway(config, store.clone()).await; - let token = token(&claims(), Some("test"), Algorithm::RS256); - let results = - futures::future::join_all((0..20).map(|_| app.clone().oneshot(request(Some(&token), "server/discover")))).await; - for result in results { - assert_eq!(result.expect("valid authentication test fixture").status(), StatusCode::SERVICE_UNAVAILABLE); - } - assert_eq!(requests.load(Ordering::SeqCst), 1); - assert_eq!(store.reads.load(Ordering::SeqCst), 0); - server.shutdown().await.expect("valid authentication test fixture"); -} - -/// Small repeatable load probe of the complete cached-auth/discovery path. -/// Run explicitly with --ignored --nocapture; compare the same profile on both revisions. -#[tokio::test(flavor = "multi_thread", worker_threads = 4)] -#[ignore = "manual cached-authentication load comparison"] -async fn cached_authentication_load() { - let (server, requests) = key_server().await; - let mut config = create_default_config(); - config.jwks_config.url = server.url("/jwks").parse().expect("valid authentication test fixture"); - let store = CountingStore::default(); - let app = gateway(config, store).await; - let token = token(&claims(), Some("test"), Algorithm::RS256); - for _ in 0..100 { - assert_eq!( - app.clone() - .oneshot(request(Some(&token), "server/discover")) - .await - .expect("valid authentication test fixture") - .status(), - StatusCode::OK - ); - } - let start = std::time::Instant::now(); - let tasks = (0..16) - .map(|_| { - let app = app.clone(); - let token = token.clone(); - tokio::spawn(async move { - let mut times = Vec::with_capacity(500); - for _ in 0..500 { - let start = std::time::Instant::now(); - let response = app - .clone() - .oneshot(request(Some(&token), "server/discover")) - .await - .expect("valid authentication test fixture"); - assert_eq!(response.status(), StatusCode::OK); - let _ = axum::body::to_bytes(response.into_body(), 65_536) - .await - .expect("valid authentication test fixture"); - times.push(start.elapsed().as_micros()); - } - times - }) - }) - .collect::>(); - let mut times = Vec::new(); - for task in tasks { - times.extend(task.await.expect("valid authentication test fixture")); - } - let elapsed = start.elapsed(); - times.sort_unstable(); - println!( - "cached auth/discovery: requests=8000 concurrency=16 elapsed_ms={} rps={:.0} p50_us={} p95_us={} jwks_fetches={}", - elapsed.as_millis(), - 8000.0 / elapsed.as_secs_f64(), - times[4000], - times[7600], - requests.load(Ordering::SeqCst) - ); - assert_eq!(requests.load(Ordering::SeqCst), 1); - server.shutdown().await.expect("valid authentication test fixture"); + .expect("valid authentication test fixture") } #[tokio::test] @@ -368,15 +175,20 @@ async fn permission_denial_prevents_backend_calls_after_a_successful_request() { }), ..Default::default() }; - let (server, _) = key_server().await; + let server = key_server().await; let mut config = create_default_config(); config.upstream_transport_config.upstream_connection_mode = Some(contextforge_data_plane_lib::UpstreamConnectionMode::PlainTextOrTls); config.jwks_config.url = server.url("/jwks").parse().expect("valid authentication test fixture"); let app = gateway(config, store.clone()).await; - for (role, expected) in [("user", StatusCode::OK), ("unknown", StatusCode::FORBIDDEN)] { + for (role, expected) in + [("user", StatusCode::OK), ("unknown", StatusCode::FORBIDDEN), ("expired", StatusCode::UNAUTHORIZED)] + { let mut c = claims(); c["role"] = role.into(); + if role == "expired" { + c["exp"] = json!(1); + } let token = token(&c, Some("test"), Algorithm::RS256); let request = request(Some(&token), "tools/call"); let response = app.clone().oneshot(request).await.expect("valid authentication test fixture"); @@ -392,6 +204,10 @@ async fn permission_denial_prevents_backend_calls_after_a_successful_request() { assert_eq!(store.reads.load(Ordering::SeqCst), 1); assert_eq!(hits.load(Ordering::SeqCst), 1, "denied request must never reach backend"); } + let response = app.oneshot(request(None, "tools/call")).await.expect("authentication fixture"); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert_eq!(store.reads.load(Ordering::SeqCst), 1); + assert_eq!(hits.load(Ordering::SeqCst), 1); server.shutdown().await.expect("valid authentication test fixture"); backend.shutdown().await.expect("valid authentication test fixture"); } diff --git a/crates/contextforge-data-plane-lib/tests/gateway/harness/mod.rs b/crates/contextforge-data-plane-lib/tests/gateway/harness/mod.rs index bf14b9d8..b0e9f238 100644 --- a/crates/contextforge-data-plane-lib/tests/gateway/harness/mod.rs +++ b/crates/contextforge-data-plane-lib/tests/gateway/harness/mod.rs @@ -53,10 +53,7 @@ pub fn create_default_config() -> Config { ca_cert_path: None, issuer: "mcpgateway".to_owned(), audiences: vec!["mcpgateway-api".to_owned()], - algorithms: vec![jsonwebtoken::Algorithm::RS256], - leeway_seconds: 30, }, - principal_config: contextforge_data_plane_lib::PrincipalConfig::default(), mcp_standard_header_max_count: 10, mcp_standard_header_max_value_bytes: 4096, diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index 0c14c98f..c497f36f 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -78,12 +78,6 @@ services: - CONTEXTFORGE_DATA_PLANE_JWKS_URL - CONTEXTFORGE_DATA_PLANE_JWT_ISSUER - CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES - - CONTEXTFORGE_DATA_PLANE_JWT_ALGORITHMS - - CONTEXTFORGE_DATA_PLANE_JWT_LEEWAY_SECONDS - - CONTEXTFORGE_DATA_PLANE_JWT_USER_CLAIM - - CONTEXTFORGE_DATA_PLANE_JWT_ADMIN_SCOPES - - CONTEXTFORGE_DATA_PLANE_JWT_MCP_USER_SCOPES - - CONTEXTFORGE_DATA_PLANE_JWT_SCOPES_ONLY - RUST_LOG=debug sysctls: @@ -146,12 +140,6 @@ services: - CONTEXTFORGE_DATA_PLANE_JWKS_URL - CONTEXTFORGE_DATA_PLANE_JWT_ISSUER - CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES - - CONTEXTFORGE_DATA_PLANE_JWT_ALGORITHMS - - CONTEXTFORGE_DATA_PLANE_JWT_LEEWAY_SECONDS - - CONTEXTFORGE_DATA_PLANE_JWT_USER_CLAIM - - CONTEXTFORGE_DATA_PLANE_JWT_ADMIN_SCOPES - - CONTEXTFORGE_DATA_PLANE_JWT_MCP_USER_SCOPES - - CONTEXTFORGE_DATA_PLANE_JWT_SCOPES_ONLY - RUST_LOG=debug sysctls: @@ -191,12 +179,6 @@ services: - CONTEXTFORGE_DATA_PLANE_JWKS_URL - CONTEXTFORGE_DATA_PLANE_JWT_ISSUER - CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES - - CONTEXTFORGE_DATA_PLANE_JWT_ALGORITHMS - - CONTEXTFORGE_DATA_PLANE_JWT_LEEWAY_SECONDS - - CONTEXTFORGE_DATA_PLANE_JWT_USER_CLAIM - - CONTEXTFORGE_DATA_PLANE_JWT_ADMIN_SCOPES - - CONTEXTFORGE_DATA_PLANE_JWT_MCP_USER_SCOPES - - CONTEXTFORGE_DATA_PLANE_JWT_SCOPES_ONLY - RUST_LOG=debug sysctls: From 17c6de15d32fc5b3f20221d9a3f033a633a9800c Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Wed, 23 Sep 2026 15:09:53 +0100 Subject: [PATCH 04/19] test: mark dummy JWKS credentials and refresh secret baseline Signed-off-by: Pratik Gandhi --- .secrets.baseline | 20 +++++++++---------- .../authorization/jwks/jwks_authorization.rs | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 6a0e6bc8..001dbd0f 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "(?x)(Cargo\\.lock$|\\.lock$)|^\\.secrets\\.baseline$|^.secrets.baseline$", "lines": null }, - "generated_at": "2026-09-16T09:14:49Z", + "generated_at": "2026-09-23T14:09:14Z", "plugins_used": [ { "name": "AWSKeyDetector" @@ -142,7 +142,7 @@ "hashed_secret": "bfc6000db1195a9522813fc405c666dd4ce669ad", "is_secret": false, "is_verified": false, - "line_number": 98, + "line_number": 106, "type": "Secret Keyword", "verified_result": null }, @@ -150,7 +150,7 @@ "hashed_secret": "4a4645604f0b9e29503be96a87f6f47a6e4a7890", "is_secret": false, "is_verified": false, - "line_number": 105, + "line_number": 113, "type": "Secret Keyword", "verified_result": null } @@ -168,7 +168,7 @@ "hashed_secret": "4a4645604f0b9e29503be96a87f6f47a6e4a7890", "is_secret": false, "is_verified": false, - "line_number": 176, + "line_number": 183, "type": "Secret Keyword", "verified_result": null } @@ -374,7 +374,7 @@ "hashed_secret": "fdda45b7f6d2ead95d9991fc4678640c3bab0d84", "is_secret": false, "is_verified": false, - "line_number": 342, + "line_number": 348, "type": "Secret Keyword", "verified_result": null }, @@ -382,7 +382,7 @@ "hashed_secret": "093d378410a5cfa4bd5088f3fef62fbdb8a95665", "is_secret": false, "is_verified": false, - "line_number": 348, + "line_number": 354, "type": "Secret Keyword", "verified_result": null }, @@ -390,7 +390,7 @@ "hashed_secret": "c3de40d5e3fc71ed62771c2127a8e42585026c97", "is_secret": false, "is_verified": false, - "line_number": 350, + "line_number": 356, "type": "Secret Keyword", "verified_result": null }, @@ -398,7 +398,7 @@ "hashed_secret": "4d4acd9b084d13f5fdb23807d857e1c48a1cfd0f", "is_secret": false, "is_verified": false, - "line_number": 439, + "line_number": 445, "type": "Secret Keyword", "verified_result": null }, @@ -406,7 +406,7 @@ "hashed_secret": "bd0160c2cf35d950843c88f3be2b9412ed71f485", "is_secret": false, "is_verified": false, - "line_number": 474, + "line_number": 480, "type": "Secret Keyword", "verified_result": null }, @@ -414,7 +414,7 @@ "hashed_secret": "293324f6824bb3a6db5c4dc42a60ddd4a9851c99", "is_secret": false, "is_verified": false, - "line_number": 637, + "line_number": 643, "type": "Hex High Entropy String", "verified_result": null } diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs index 1fcc0d44..154a7879 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs @@ -105,7 +105,7 @@ mod tests { for url in [ "http://issuer.example/keys", "file:///keys", - "https://user:secret@issuer.example/keys", + "https://user:secret@issuer.example/keys", // pragma: allowlist secret (synthetic URL-rejection fixture) "https://issuer.example/keys#fragment", ] { assert!(parse_jwks_url(url.parse().unwrap()).is_err(), "{url}"); From 2ad6c7a7e22609a6e1ab2fb480d0ba062d19f6bc Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Thu, 24 Sep 2026 11:14:47 +0100 Subject: [PATCH 05/19] auth: preserve identity extraction and isolate role checks Signed-off-by: Pratik Gandhi --- .../src/authorization/mod.rs | 8 +- .../cel_principal_extractor.rs | 94 ++++++++------ .../default_principal_extractor.rs | 108 ++++++---------- .../authorization/principal_extractor/mod.rs | 114 ++--------------- .../src/layers/permission.rs | 120 +++++++++++++++--- .../src/layers/principal_extractor.rs | 4 +- 6 files changed, 211 insertions(+), 237 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/authorization/mod.rs b/crates/contextforge-data-plane-lib/src/authorization/mod.rs index 7f070373..8d4ec8eb 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/mod.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/mod.rs @@ -12,9 +12,15 @@ mod jwks; mod principal_extractor; pub use principal_extractor::{ - AuthorizedPrincipal, CelPrincipalExtractor, DefaultPrincipalExtractor, Permission, PrincipalExtractor, + AuthorizedPrincipal, CelPrincipalExtractor, DefaultPrincipalExtractor, PrincipalExtractor, }; +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum Permission { + Admin, + MCPUser, +} + pub fn get_authorization_service( config: &JwksConfig, ) -> Result, AuthorizationError> { diff --git a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/cel_principal_extractor.rs b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/cel_principal_extractor.rs index 0bfa82e7..6c56ea46 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/cel_principal_extractor.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/cel_principal_extractor.rs @@ -5,9 +5,9 @@ use std::sync::Arc; use cel::{Context, Program, objects::Key}; use serde_json::Value as JsonValue; use thiserror::Error; -use tracing::instrument; +use tracing::{debug, instrument}; -use super::{AuthorizedPrincipal, PrincipalExtractor}; +use crate::authorization::{AuthorizedPrincipal, PrincipalExtractor}; #[derive(Error, Debug)] pub enum CelPrincipalExtractorError { @@ -22,10 +22,35 @@ pub enum CelPrincipalExtractorError { #[error("CEL expression did not return a map: {0:?}")] InvalidReturnType(JsonValue), + + #[error("Missing required field in CEL result: {0}")] + MissingRequiredField(String), + + #[error("Invalid field type in CEL result: field={0}, expected={1}")] + InvalidFieldType(String, String), } -/// CEL maps user_id and tenant_id. Roles and issuer always come from the -/// original verified claims and use the same policy as the default extractor. +/// A CEL-based principal extractor that evaluates a CEL expression to extract +/// principal information from authorization claims. +/// +/// The CEL expression should return a map with the following fields: +/// - `user_id` (string, required): The user identifier +/// - `tenant_id` (string, required): The tenant identifier +/// - `scopes` (list of strings, optional): The user's scopes/permissions +/// +/// The CEL expression has access to the following variables: +/// - `claims`: A map containing all the authorization claims +/// - `sub`: The subject claim (shorthand for claims.sub) +/// - `tenant_id`: The tenant_id claim (shorthand for claims.tenant_id) +/// +/// Example CEL expression: +/// ```cel +/// { +/// "user_id": claims.sub, +/// "tenant_id": claims.tenant_id, +/// "scopes": [] +/// } +/// ``` #[derive(Clone, Debug)] pub struct CelPrincipalExtractor { program: Arc, @@ -60,23 +85,30 @@ impl PrincipalExtractor for CelPrincipalExtractor { let result = self.program.execute(&context).map_err(|e| CelPrincipalExtractorError::EvaluationError(e.to_string()))?; - let cel::Value::Map(map) = result else { - return Err(CelPrincipalExtractorError::InvalidReturnType(JsonValue::Null).into()); - }; - let mut normalized = serde_json::Map::new(); - for (name, claim) in [("user_id", "sub"), ("tenant_id", "tenant_id")] { - let value = match map.get(&Key::from(name.to_owned())) { - Some(cel::Value::String(value)) => JsonValue::String(value.to_string()), - _ => JsonValue::Null, - }; - normalized.insert(claim.to_owned(), value); - } - for name in ["iss", "role", "roles"] { - if let Some(value) = claims.get(name) { - normalized.insert(name.to_owned(), value.clone()); - } + debug!("CEL expression evaluated to: {:?}", result); + + Ok(AuthorizedPrincipal::try_from(result)?) + } +} + +impl TryFrom for AuthorizedPrincipal { + type Error = CelPrincipalExtractorError; + + fn try_from(value: cel::Value) -> Result { + match value { + cel::Value::Map(map) => { + if let Some(cel::Value::String(user_id)) = map.get(&Key::from("user_id".to_owned())) + && let Some(cel::Value::String(tenant_id)) = map.get(&Key::from("tenant_id".to_owned())) + { + let user_id = (**user_id).clone(); + let tenant_id = (**tenant_id).clone(); + Ok(AuthorizedPrincipal::builder().user_id(user_id).tenant_id(tenant_id).scopes(vec![]).build()) + } else { + Err(CelPrincipalExtractorError::InvalidReturnType(serde_json::Value::Null)) + } + }, + _ => Err(CelPrincipalExtractorError::InvalidReturnType(serde_json::Value::Null)), } - Ok(AuthorizedPrincipal::from_claims(&JsonValue::Object(normalized))?) } } @@ -146,25 +178,3 @@ mod tests { assert!(result.is_err()); } } - -#[cfg(test)] -mod permission_tests { - use super::*; - use crate::Permission; - - #[test] - fn mapping_cannot_override_verified_roles_or_issuer() { - let extractor = CelPrincipalExtractor::from_expression( - r#"{"user_id": claims.sub, "tenant_id": claims.woTenantId, "role": "admin", "iss": "other"}"#, - ) - .unwrap(); - let claims = serde_json::json!({"sub":"user","woTenantId":"tenant","iss":"watson","roles":["user"]}); - let principal = extractor.extract(&claims).unwrap(); - assert_eq!(principal.issuer(), "watson"); - assert!(principal.has_permission(Permission::MCPUser)); - assert!(!principal.has_permission(Permission::Admin)); - let mut no_roles = claims; - no_roles.as_object_mut().unwrap().remove("roles"); - assert!(!extractor.extract(&no_roles).unwrap().has_permission(Permission::MCPUser)); - } -} diff --git a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/default_principal_extractor.rs b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/default_principal_extractor.rs index 2d7b2516..986aa3bb 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/default_principal_extractor.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/default_principal_extractor.rs @@ -1,7 +1,8 @@ -use super::{AuthorizedPrincipal, PrincipalExtractor}; use tracing::instrument; -#[derive(Debug, Clone, Default)] +use crate::authorization::{AuthorizedPrincipal, PrincipalExtractor}; + +#[derive(Debug, Clone)] pub struct DefaultPrincipalExtractor {} impl PrincipalExtractor for DefaultPrincipalExtractor { @@ -10,87 +11,60 @@ impl PrincipalExtractor for DefaultPrincipalExtractor { &self, claims: &serde_json::Value, ) -> Result> { - Ok(AuthorizedPrincipal::from_claims(claims)?) + let user_id = + ["sub", "user_id", "UserId"].into_iter().find_map(|claim| claims.get(claim)).and_then(|v| v.as_str()); + let tenant_id = ["tenantId", "tenant_id", "woTenantId"] + .into_iter() + .find_map(|claim| claims.get(claim)) + .and_then(|v| v.as_str()); + match (user_id, tenant_id) { + (Some(user_id), Some(tenant_id)) => Ok(AuthorizedPrincipal::builder() + .user_id(user_id.to_owned()) + .tenant_id(tenant_id.to_owned()) + .scopes(vec![]) + .build()), + _ => Err("Can't create principal".into()), + } } } #[cfg(test)] mod tests { use super::*; - use crate::authorization::Permission; - use serde_json::{Value, json}; - - fn claims() -> Value { - json!({"iss":"watson", "sub":"subject", "woUserId":"watson-user", "woTenantId":"tenant", "role":"user"}) - } - - #[test] - fn role_permissions_are_explicit_and_admin_includes_mcp() { - for (role, admin, mcp) in [ - ("admin", true, true), - ("builder", false, true), - ("user", false, true), - ("unknown", false, false), - ("Admin", false, false), - ("", false, false), - ] { - let mut claims = claims(); - claims["role"] = role.into(); - let principal = DefaultPrincipalExtractor::default().extract(&claims).unwrap(); - assert_eq!(principal.has_permission(Permission::Admin), admin, "{role}"); - assert_eq!(principal.has_permission(Permission::MCPUser), mcp, "{role}"); - assert_eq!( - (principal.issuer(), principal.user_id(), principal.tenant_id()), - ("watson", "subject", "tenant") - ); - } - let mut claims = claims(); - claims.as_object_mut().unwrap().remove("role"); - assert!(!DefaultPrincipalExtractor::default().extract(&claims).unwrap().has_permission(Permission::MCPUser)); - claims["roles"] = json!(["unknown", "admin"]); - assert!(DefaultPrincipalExtractor::default().extract(&claims).unwrap().has_permission(Permission::Admin)); - } + use serde_json::json; #[test] - fn subject_and_tenant_aliases_are_required() { - let mut claims = claims(); - let extractor = DefaultPrincipalExtractor {}; - assert_eq!(extractor.extract(&claims).unwrap().user_id(), "subject"); - let mut missing_sub = claims.clone(); - missing_sub.as_object_mut().unwrap().remove("sub"); - assert!(extractor.extract(&missing_sub).is_err()); - claims["tenant_id"] = "tenant".into(); - assert!(extractor.extract(&claims).is_ok()); - claims["tenantId"] = "different".into(); - assert!(extractor.extract(&claims).is_err()); - for invalid in [Value::Null, json!(42), json!(""), json!(" ")] { - for claim in ["sub", "woTenantId", "iss"] { - let mut claims = super::tests::claims(); - claims[claim] = invalid.clone(); - assert!(DefaultPrincipalExtractor::default().extract(&claims).is_err(), "{claim}"); + fn supported_identity_aliases_are_preserved() { + for user_claim in ["sub", "user_id", "UserId"] { + for tenant_claim in ["tenantId", "tenant_id", "woTenantId"] { + let claims = json!({user_claim: "user", tenant_claim: "tenant"}); + let principal = DefaultPrincipalExtractor {}.extract(&claims).unwrap(); + assert_eq!(principal.user_id, "user"); + assert_eq!(principal.tenant_id, "tenant"); + assert!(principal.scopes.is_empty()); } } - for tenant in ["woTenantId", "tenant_id", "tenantId"] { - let claims = json!({"iss":"watson", "sub":"subject", tenant:"tenant", "role":"user"}); - assert!(DefaultPrincipalExtractor::default().extract(&claims).is_ok()); - } } #[test] - fn malformed_permission_claims_are_rejected() { - for (name, value) in [("role", json!(["admin"])), ("roles", json!("admin")), ("roles", json!([42]))] { - let mut claims = claims(); - claims[name] = value; - assert!(DefaultPrincipalExtractor::default().extract(&claims).is_err(), "{name}"); - } + fn existing_alias_precedence_is_preserved() { + let claims = json!({ + "sub": "subject", "user_id": "alternate", "UserId": "other", + "tenantId": "tenant", "tenant_id": "alternate", "woTenantId": "other" + }); + let principal = DefaultPrincipalExtractor {}.extract(&claims).unwrap(); + assert_eq!(principal.user_id, "subject"); + assert_eq!(principal.tenant_id, "tenant"); } #[test] - fn debug_does_not_disclose_identity() { - let principal = DefaultPrincipalExtractor::default().extract(&claims()).unwrap(); - let debug = format!("{principal:?}"); - for sensitive in ["watson", "subject", "tenant"] { - assert!(!debug.contains(sensitive)); + fn missing_identity_is_rejected() { + for claims in [ + json!({"sub": "user"}), + json!({"tenant_id": "tenant"}), + json!({"woUserId": "user", "woTenantId": "tenant"}), + ] { + assert!(DefaultPrincipalExtractor {}.extract(&claims).is_err()); } } } diff --git a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/mod.rs b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/mod.rs index f16a5b8e..44742246 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/mod.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/principal_extractor/mod.rs @@ -1,122 +1,28 @@ mod cel_principal_extractor; mod default_principal_extractor; - -use std::collections::BTreeSet; - use contextforge_data_plane_apis::User; -use serde_json::Value; +use typed_builder::TypedBuilder; pub use cel_principal_extractor::CelPrincipalExtractor; pub use default_principal_extractor::DefaultPrincipalExtractor; -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] -pub enum Permission { - Admin, - MCPUser, -} - -#[derive(Debug, thiserror::Error)] -pub enum PrincipalError { - #[error("missing or malformed identity claim")] - InvalidIdentity, - #[error("conflicting tenant claims")] - ConflictingTenant, - #[error("malformed role claims")] - InvalidPermissions, -} - -/// Request-local identity derived only from verified claims and trusted mapping configuration. -/// This is not yet a tenant-aware persistent configuration key. -#[derive(Clone)] +#[derive(Debug, Clone, TypedBuilder)] +#[allow(dead_code)] pub struct AuthorizedPrincipal { - issuer: String, user_id: String, tenant_id: String, - permissions: BTreeSet, -} - -impl std::fmt::Debug for AuthorizedPrincipal { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("AuthorizedPrincipal").field("permissions", &self.permissions).finish_non_exhaustive() - } -} - -impl AuthorizedPrincipal { - pub fn issuer(&self) -> &str { - &self.issuer - } - pub fn user_id(&self) -> &str { - &self.user_id - } - pub fn tenant_id(&self) -> &str { - &self.tenant_id - } - pub fn has_permission(&self, permission: Permission) -> bool { - self.permissions.contains(&permission) - } - - fn from_claims(claims: &Value) -> Result { - let issuer = identity_string(claims.get("iss"))?; - let user_id = identity_string(claims.get("sub"))?; - let mut tenant_id = None; - for name in ["woTenantId", "tenant_id", "tenantId"] { - if let Some(value) = claims.get(name) { - let value = identity_string(Some(value))?; - if tenant_id.is_some_and(|tenant| tenant != value) { - return Err(PrincipalError::ConflictingTenant); - } - tenant_id = Some(value); - } - } - let tenant_id = tenant_id.ok_or(PrincipalError::InvalidIdentity)?; - let mut roles = string_list(claims.get("roles"))?; - if let Some(role) = claims.get("role") { - roles.push(role.as_str().ok_or(PrincipalError::InvalidPermissions)?.to_owned()); - } - let mut permissions = BTreeSet::new(); - for role in roles { - match role.as_str() { - "admin" => { - permissions.extend([Permission::Admin, Permission::MCPUser]); - }, - "builder" | "user" => { - permissions.insert(Permission::MCPUser); - }, - _ => {}, - } - } - Ok(Self { - issuer: issuer.to_owned(), - user_id: user_id.to_owned(), - tenant_id: tenant_id.to_owned(), - permissions, - }) - } -} - -fn identity_string(value: Option<&Value>) -> Result<&str, PrincipalError> { - value.and_then(Value::as_str).filter(|value| !value.trim().is_empty()).ok_or(PrincipalError::InvalidIdentity) -} - -fn string_list(value: Option<&Value>) -> Result, PrincipalError> { - match value { - None => Ok(Vec::new()), - Some(Value::Array(values)) => values - .iter() - .map(|value| value.as_str().map(str::to_owned).ok_or(PrincipalError::InvalidPermissions)) - .collect(), - _ => Err(PrincipalError::InvalidPermissions), - } + scopes: Vec, } -impl From<&AuthorizedPrincipal> for User { - fn from(value: &AuthorizedPrincipal) -> Self { - // The publisher and ConfigStore still use subject-only keys. Tenant isolation - // requires a coordinated schema/publisher migration in the next change. +impl<'a> From<&'a AuthorizedPrincipal> for User { + fn from(value: &'a AuthorizedPrincipal) -> Self { Self::new(&value.user_id) } } pub trait PrincipalExtractor { - fn extract(&self, claims: &Value) -> Result>; + fn extract( + &self, + claims: &serde_json::Value, + ) -> Result>; } diff --git a/crates/contextforge-data-plane-lib/src/layers/permission.rs b/crates/contextforge-data-plane-lib/src/layers/permission.rs index 77e3a231..c14f2435 100644 --- a/crates/contextforge-data-plane-lib/src/layers/permission.rs +++ b/crates/contextforge-data-plane-lib/src/layers/permission.rs @@ -1,5 +1,5 @@ use crate::{ - authorization::{AuthorizedPrincipal, Permission}, + authorization::{AuthenticationError, AuthorizationClaims, AuthorizedPrincipal, Permission}, errors::{custom_error, unauthorized_response}, }; use axum::{ @@ -12,40 +12,118 @@ use http::StatusCode; /// Reusable API-level guard. Install after verified principal extraction and before /// configuration or backend access. Use `Admin` for future management routes. pub async fn require_permission(State(permission): State, request: Request, next: Next) -> Response { - let Some(principal) = request.extensions().get::() else { + if request.extensions().get::().is_none() { return unauthorized_response("Missing verified identity"); + } + let Some(claims) = request.extensions().get::() else { + return unauthorized_response("Missing verified claims"); }; - if !principal.has_permission(permission) { - return custom_error(StatusCode::FORBIDDEN, "Insufficient permission"); + match has_permission(claims, permission) { + Ok(true) => next.run(request).await, + Ok(false) => custom_error(StatusCode::FORBIDDEN, "Insufficient permission"), + Err(_) => unauthorized_response("Invalid role claims"), } - next.run(request).await +} + +/// Provisional PoC role mapping, using only the original verified token claims. +fn has_permission(claims: &AuthorizationClaims, permission: Permission) -> Result { + let allows = |role: &str| match role { + "admin" => true, + "builder" | "user" => permission == Permission::MCPUser, + _ => false, + }; + let claims = claims.as_value(); + let mut granted = false; + if let Some(roles) = claims.get("roles") { + for role in roles.as_array().ok_or(AuthenticationError::InvalidToken)? { + granted |= allows(role.as_str().ok_or(AuthenticationError::InvalidToken)?); + } + } + if let Some(role) = claims.get("role") { + granted |= allows(role.as_str().ok_or(AuthenticationError::InvalidToken)?); + } + Ok(granted) } #[cfg(test)] mod tests { use super::*; - use crate::authorization::{DefaultPrincipalExtractor, PrincipalExtractor}; + use crate::authorization::{CelPrincipalExtractor, DefaultPrincipalExtractor, PrincipalExtractor}; use axum::{Router, body::Body, middleware, routing::get}; + use serde_json::json; use tower::ServiceExt; + fn app(permission: Permission) -> Router { + Router::new() + .route("/", get(|| async { StatusCode::NO_CONTENT })) + .layer(middleware::from_fn_with_state(permission, require_permission)) + } + #[tokio::test] - async fn guards_require_a_principal_and_the_requested_permission() { - for permission in [Permission::Admin, Permission::MCPUser] { - let app = Router::new() - .route("/", get(|| async { StatusCode::NO_CONTENT })) - .layer(middleware::from_fn_with_state(permission, require_permission)); - let response = app.clone().oneshot(Request::new(Body::empty())).await.unwrap(); + async fn guards_require_both_verified_claims_and_identity() { + let claims = json!({"sub":"user", "tenant_id":"tenant", "roles":["admin"]}); + let principal = DefaultPrincipalExtractor {}.extract(&claims).unwrap(); + for (include_claims, include_principal) in [(false, false), (true, false), (false, true)] { + let mut request = Request::new(Body::empty()); + if include_claims { + request.extensions_mut().insert(AuthorizationClaims::from(claims.clone())); + } + if include_principal { + request.extensions_mut().insert(principal.clone()); + } + let response = app(Permission::MCPUser).oneshot(request).await.unwrap(); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); - for role in ["admin", "builder", "user", "unknown"] { - let principal = DefaultPrincipalExtractor::default() - .extract(&serde_json::json!({"iss":"watson","sub":"user","tenant_id":"tenant","role":role})) - .unwrap(); + } + } + + #[tokio::test] + async fn guards_map_verified_roles_to_the_requested_permission() { + use StatusCode as S; + for (role_claims, admin_status, mcp_status) in [ + (json!({"roles":["admin"]}), S::NO_CONTENT, S::NO_CONTENT), + (json!({"roles":["builder"]}), S::FORBIDDEN, S::NO_CONTENT), + (json!({"roles":["user"]}), S::FORBIDDEN, S::NO_CONTENT), + (json!({"role":"admin"}), S::NO_CONTENT, S::NO_CONTENT), + (json!({"roles":["user"], "role":"admin"}), S::NO_CONTENT, S::NO_CONTENT), + (json!({"roles":["unknown", "user"]}), S::FORBIDDEN, S::NO_CONTENT), + (json!({"roles":["ServiceAdmin"]}), S::FORBIDDEN, S::FORBIDDEN), + (json!({"roles":[]}), S::FORBIDDEN, S::FORBIDDEN), + (json!({}), S::FORBIDDEN, S::FORBIDDEN), + (json!({"roles":"admin"}), S::UNAUTHORIZED, S::UNAUTHORIZED), + (json!({"role":["admin"]}), S::UNAUTHORIZED, S::UNAUTHORIZED), + (json!({"roles":["admin", 42]}), S::UNAUTHORIZED, S::UNAUTHORIZED), + (json!({"roles":["admin"], "role":42}), S::UNAUTHORIZED, S::UNAUTHORIZED), + ] { + let mut claims = json!({"sub":"user", "tenant_id":"tenant"}); + claims.as_object_mut().unwrap().extend(role_claims.as_object().unwrap().clone()); + let principal = DefaultPrincipalExtractor {}.extract(&claims).unwrap(); + for (permission, expected) in [(Permission::Admin, admin_status), (Permission::MCPUser, mcp_status)] { + let mut request = Request::new(Body::empty()); + request.extensions_mut().insert(principal.clone()); + request.extensions_mut().insert(AuthorizationClaims::from(claims.clone())); + let response = app(permission).oneshot(request).await.unwrap(); + assert_eq!(response.status(), expected, "{permission:?}: {role_claims}"); + } + } + } + + #[tokio::test] + async fn cel_identity_mapping_cannot_grant_roles_absent_from_the_token() { + let extractor = CelPrincipalExtractor::from_expression( + r#"{"user_id": claims.sub, "tenant_id": claims.woTenantId, "role": "admin", "scopes": ["Admin"]}"#, + ) + .unwrap(); + for (roles, mcp_status) in [(json!(["user"]), StatusCode::NO_CONTENT), (json!([]), StatusCode::FORBIDDEN)] { + let claims = json!({"sub":"user", "woTenantId":"tenant", "roles":roles}); + let principal = extractor.extract(&claims).unwrap(); + for (permission, expected) in + [(Permission::Admin, StatusCode::FORBIDDEN), (Permission::MCPUser, mcp_status)] + { let mut request = Request::new(Body::empty()); - request.extensions_mut().insert(principal); - let response = app.clone().oneshot(request).await.unwrap(); - let allowed = - role == "admin" || permission == Permission::MCPUser && ["builder", "user"].contains(&role); - assert_eq!(response.status(), if allowed { StatusCode::NO_CONTENT } else { StatusCode::FORBIDDEN }); + request.extensions_mut().insert(principal.clone()); + request.extensions_mut().insert(AuthorizationClaims::from(claims.clone())); + let response = app(permission).oneshot(request).await.unwrap(); + assert_eq!(response.status(), expected); } } } diff --git a/crates/contextforge-data-plane-lib/src/layers/principal_extractor.rs b/crates/contextforge-data-plane-lib/src/layers/principal_extractor.rs index 7b92b4d3..a1c37a35 100644 --- a/crates/contextforge-data-plane-lib/src/layers/principal_extractor.rs +++ b/crates/contextforge-data-plane-lib/src/layers/principal_extractor.rs @@ -58,8 +58,8 @@ where let maybe_authorization_claims = request.extensions().get::(); let Some(Ok(authorized_principal)) = maybe_authorization_claims.map(|authorization_claims| { principal_extractor - .extract(authorization_claims.as_value()) - .inspect_err(|_| debug!("principal_extract - invalid identity or permission claims")) + .extract(&authorization_claims.into()) + .inspect_err(|e| debug!("Can't extract the principal {e:?}")) }) else { return Ok(unauthorized_response("Invalid token. Unable to extract the principal from claims")); }; From 31a62a81ad0a304d9dc74c1c39fc18da11856657 Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Thu, 24 Sep 2026 14:25:46 +0100 Subject: [PATCH 06/19] auth: restore original claims and bearer handling Signed-off-by: Pratik Gandhi --- .../authorization/jwks/jwks_authorization.rs | 43 ++++++++----------- .../src/authorization/mod.rs | 4 +- .../src/layers/claims_id.rs | 26 +++-------- .../src/layers/mcp_header_limits.rs | 4 +- .../tests/gateway/harness/auth.rs | 7 +-- 5 files changed, 30 insertions(+), 54 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs index 154a7879..f1096e52 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs @@ -1,6 +1,6 @@ use crate::JwksConfig; use crate::authorization::jwks::jwks::Jwks; -use crate::authorization::{AuthenticationError, AuthorizationClaims, AuthorizationError, AuthorizationService}; +use crate::authorization::{AuthorizationClaims, AuthorizationError, AuthorizationService}; use async_trait::async_trait; use jsonwebtoken::decode_header; use std::fmt; @@ -36,9 +36,9 @@ impl JwtAuthorizationService { Ok(Self { jwks: Jwks::new(client, url, validation) }) } - async fn authorize_token(&self, token: &str) -> Result { - let header = decode_header(token).map_err(|_| AuthenticationError::InvalidToken)?; - self.jwks.validate(token, &header).await + async fn authorize_token(&self, token: &str) -> Option { + let header = decode_header(token).ok()?; + self.jwks.validate(token, &header).await.ok() } } @@ -54,17 +54,16 @@ impl fmt::Debug for JwtAuthorizationService { #[async_trait] impl AuthorizationService for JwtAuthorizationService { #[instrument(name = "jwt_authorization_service", level = "info", skip_all)] - async fn authorize( - &self, - authorization_token: &http::HeaderValue, - ) -> Result { - let value = authorization_token.to_str().map_err(|_| AuthenticationError::InvalidToken)?; - let (scheme, token) = value.split_once(' ').ok_or(AuthenticationError::InvalidToken)?; - if !scheme.eq_ignore_ascii_case("Bearer") || token.is_empty() || token.bytes().any(|b| b.is_ascii_whitespace()) - { - return Err(AuthenticationError::InvalidToken); + async fn authorize(&self, authorization_token: &http::HeaderValue) -> Option { + let token = authorization_token.as_bytes().strip_prefix(b"Bearer ")?; + let token = str::from_utf8(token).ok()?; + let claims = self.authorize_token(token).await; + + if claims.is_none() { + tracing::debug!("validate_saas_jwt SaaS JWT was rejected"); } - self.authorize_token(token).await + + claims } } @@ -72,10 +71,9 @@ fn parse_jwks_url(url: Url) -> Result { let secure = url.scheme() == "https"; let local_http = url.scheme() == "http" && url.host_str().is_some_and(|host| { - host.eq_ignore_ascii_case("localhost") - || host.trim_matches(['[', ']']).parse::().is_ok_and(|address| address.is_loopback()) + host.eq_ignore_ascii_case("localhost") || host.parse::().is_ok_and(|address| address.is_loopback()) }); - if (!secure && !local_http) || !url.username().is_empty() || url.password().is_some() || url.fragment().is_some() { + if !secure && !local_http { return Err(AuthorizationError::InsecureJwksUrl); } Ok(url) @@ -97,17 +95,10 @@ mod tests { use super::*; #[test] fn only_trusted_transport_urls_are_accepted() { - for url in - ["https://issuer.example/keys", "http://localhost/keys", "http://127.0.0.1/keys", "http://[::1]/keys"] - { + for url in ["https://issuer.example/keys", "http://localhost/keys", "http://127.0.0.1/keys"] { assert!(parse_jwks_url(url.parse().unwrap()).is_ok(), "{url}"); } - for url in [ - "http://issuer.example/keys", - "file:///keys", - "https://user:secret@issuer.example/keys", // pragma: allowlist secret (synthetic URL-rejection fixture) - "https://issuer.example/keys#fragment", - ] { + for url in ["http://issuer.example/keys", "file:///keys"] { assert!(parse_jwks_url(url.parse().unwrap()).is_err(), "{url}"); } } diff --git a/crates/contextforge-data-plane-lib/src/authorization/mod.rs b/crates/contextforge-data-plane-lib/src/authorization/mod.rs index 8d4ec8eb..175aa957 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/mod.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/mod.rs @@ -30,7 +30,7 @@ pub fn get_authorization_service( #[async_trait] pub trait AuthorizationService: std::fmt::Debug { - async fn authorize(&self, authorization_token: &HeaderValue) -> Result; + async fn authorize(&self, authorization_token: &HeaderValue) -> Option; } #[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] @@ -55,7 +55,7 @@ pub enum AuthorizationError { #[error("SaaS JWKS is invalid")] InvalidKey(#[source] jsonwebtoken::errors::Error), - #[error("JWKS URL must use HTTPS (HTTP is allowed only for loopback testing), without credentials or fragments")] + #[error("MCPOPS_JWKS_URL must use HTTPS (HTTP is allowed only for loopback testing)")] InsecureJwksUrl, #[error("unable to retrieve SaaS JWKS")] JwksRequest(#[source] reqwest::Error), diff --git a/crates/contextforge-data-plane-lib/src/layers/claims_id.rs b/crates/contextforge-data-plane-lib/src/layers/claims_id.rs index a0f48eaa..4f4cc9ed 100644 --- a/crates/contextforge-data-plane-lib/src/layers/claims_id.rs +++ b/crates/contextforge-data-plane-lib/src/layers/claims_id.rs @@ -4,11 +4,7 @@ use axum::{ response::Response, }; -use crate::{ - AuthenticationError, - common::ContextForgeDataPlaneAppState, - errors::{custom_error, unauthorized_response}, -}; +use crate::{common::ContextForgeDataPlaneAppState, errors::unauthorized_response}; pub async fn claims_layer( State(state): State, @@ -17,21 +13,13 @@ pub async fn claims_layer( ) -> Response { let (mut parts, body) = request.into_parts(); - let mut authorizations = parts.headers.get_all(http::header::AUTHORIZATION).iter(); - let Some(authorization) = authorizations.next() else { - return unauthorized_response("Missing bearer token"); - }; - if authorizations.next().is_some() { - return unauthorized_response("Ambiguous bearer token"); - } - let claims = match state.authorization_service.authorize(authorization).await { - Ok(claims) => claims, - Err(AuthenticationError::InvalidToken) => return unauthorized_response("Invalid bearer token"), - Err(AuthenticationError::KeysUnavailable) => { - return custom_error(http::StatusCode::SERVICE_UNAVAILABLE, "Authentication temporarily unavailable"); - }, + let Some(authorization) = parts.headers.get("Authorization") else { return unauthorized_response("No header") }; + + let Some(claims) = state.authorization_service.authorize(authorization).await else { + return unauthorized_response("Invalid token"); }; - parts.extensions.insert(claims); + + parts.extensions.insert(claims.clone()); let request = Request::from_parts(parts, body); next.run(request).await } diff --git a/crates/contextforge-data-plane-lib/src/layers/mcp_header_limits.rs b/crates/contextforge-data-plane-lib/src/layers/mcp_header_limits.rs index 506a29bd..8292a19b 100644 --- a/crates/contextforge-data-plane-lib/src/layers/mcp_header_limits.rs +++ b/crates/contextforge-data-plane-lib/src/layers/mcp_header_limits.rs @@ -208,8 +208,8 @@ mod tests { #[async_trait] impl AuthorizationService for Noop { - async fn authorize(&self, _: &HeaderValue) -> Result { - Err(crate::AuthenticationError::InvalidToken) + async fn authorize(&self, _: &HeaderValue) -> Option { + None } } diff --git a/crates/contextforge-data-plane-lib/tests/gateway/harness/auth.rs b/crates/contextforge-data-plane-lib/tests/gateway/harness/auth.rs index 8a7f2672..5389a747 100644 --- a/crates/contextforge-data-plane-lib/tests/gateway/harness/auth.rs +++ b/crates/contextforge-data-plane-lib/tests/gateway/harness/auth.rs @@ -61,10 +61,7 @@ impl AlwaysAllowAuthorizatioService { #[async_trait] impl AuthorizationService for AlwaysAllowAuthorizatioService { - async fn authorize( - &self, - _: &HeaderValue, - ) -> Result { - Ok(AuthorizationClaims::from(default_claims(&self.user))) + async fn authorize(&self, _: &HeaderValue) -> Option { + Some(AuthorizationClaims::from(default_claims(&self.user))) } } From 7d75d13fdfbcd9d03ecd6624142dc935d88e5ba1 Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Thu, 24 Sep 2026 15:11:59 +0100 Subject: [PATCH 07/19] auth: preserve original JWKS flow with token trust checks Signed-off-by: Pratik Gandhi --- .../src/authorization/jwks/jwks.rs | 173 +++++---- .../src/authorization/jwks/jwks/tests.rs | 41 +-- .../authorization/jwks/jwks_authorization.rs | 343 +++++++++++++++++- .../src/authorization/mod.rs | 11 +- 4 files changed, 439 insertions(+), 129 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs index 8afbe95f..663b889b 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs @@ -2,127 +2,133 @@ use std::time::Duration; use futures::StreamExt as _; use jsonwebtoken::{ - Algorithm, DecodingKey, Header, Validation, decode, + Algorithm, AlgorithmFamily, DecodingKey, Header, Validation, decode, jwk::{Jwk, JwkSet, KeyOperations, PublicKeyUse}, }; use lru_time_cache::LruCache; + use reqwest::Url; use serde_json::Value; use tokio::sync::RwLock; +use tracing::debug; +use typed_builder::TypedBuilder; -use crate::{ - JwksConfig, - authorization::{AuthenticationError, AuthorizationClaims, AuthorizationError}, -}; +use crate::authorization::{AuthorizationClaims, AuthorizationError}; + +pub const JWKS_CACHE_TTL: Duration = Duration::from_mins(5); +pub const JWKS_CACHE_KEY: &str = "jwks"; -const JWKS_CACHE_TTL: Duration = Duration::from_mins(5); -const JWKS_CACHE_KEY: &str = "jwks"; const JWKS_MAX_RESPONSE_BYTES: usize = 1024 * 1024; +#[derive(TypedBuilder)] pub(super) struct Jwks { client: reqwest::Client, url: Url, - validation: Validation, + issuer: String, + audiences: Vec, + #[builder(default = RwLock::new(LruCache::with_expiry_duration(JWKS_CACHE_TTL)))] cache: RwLock>>, + #[builder(default = true)] + validate_audience: bool, + #[builder(default = true)] + validate_expiry: bool, + #[builder(default = true)] + validate_not_before: bool, } impl Jwks { - pub fn new(client: reqwest::Client, url: Url, validation: Validation) -> Self { - Self { client, url, validation, cache: RwLock::new(LruCache::with_expiry_duration(JWKS_CACHE_TTL)) } - } - - pub fn validation(config: &JwksConfig) -> Result { - if config.issuer.trim().is_empty() - || config.audiences.is_empty() - || config.audiences.iter().any(|aud| aud.trim().is_empty()) - { - return Err(AuthorizationError::InvalidTrustConfiguration); - } + fn validation(&self) -> Validation { let mut validation = Validation::new(Algorithm::RS256); validation.set_required_spec_claims(&["exp", "iss", "aud"]); - validation.set_issuer(&[&config.issuer]); - validation.set_audience(&config.audiences); - validation.validate_nbf = true; - validation.leeway = 30; - Ok(validation) + validation.set_issuer(&[&self.issuer]); + validation.set_audience(&self.audiences); + validation.validate_aud = self.validate_audience; + validation.validate_exp = self.validate_expiry; + validation.validate_nbf = self.validate_not_before; + validation } - pub async fn validate(&self, token: &str, header: &Header) -> Result { - if !self.validation.algorithms.contains(&header.alg) - || header.kid.as_ref().is_none_or(|kid| kid.trim().is_empty()) - { - return Err(AuthenticationError::InvalidToken); - } + pub async fn validate(&self, token: &str, header: &Header) -> Option { { let cache = self.cache.read().await; + if let Some(keys) = cache.peek(JWKS_CACHE_KEY) - && let Some(result) = self.validate_with_keys(keys, token, header) + && keys.iter().any(|key| key.matches(header)) { - return result; + return Self::validate_with_keys(keys, token, header, &self.validation()); } } - let keys = fetch_jwks(&self.client, &self.url).await.map_err(|_| { - tracing::warn!("jwks_refresh - unable to retrieve usable verification keys"); - AuthenticationError::KeysUnavailable - })?; - let claims = self.validate_with_keys(&keys, token, header).unwrap_or(Err(AuthenticationError::InvalidToken)); - self.cache.write().await.insert(JWKS_CACHE_KEY.to_owned(), keys); - claims + + match fetch_jwks(&self.client, &self.url).await { + Ok(keys) => { + let key_count = keys.len(); + let claims = Self::validate_with_keys(&keys, token, header, &self.validation()); + self.cache.write().await.insert(JWKS_CACHE_KEY.to_owned(), keys); + tracing::info!("validate: SaaS JWKS cache refreshed {key_count}"); + + claims + }, + Err(error) => { + tracing::info!("validate: unable to refresh SaaS JWKS {error:?}"); + None + }, + } } fn validate_with_keys( - &self, keys: &[VerificationKey], token: &str, header: &Header, - ) -> Option> { - let key = keys.iter().find(|key| key.matches(header))?; - Some( - decode::(token, &key.decoding_key, &self.validation) - .map(|token| AuthorizationClaims::from(token.claims)) - .map_err(|_| AuthenticationError::InvalidToken), - ) + validation: &Validation, + ) -> Option { + keys.iter() + .filter(|key| key.matches(header)) + .find_map(|key| Self::validate_and_decode_claims(token, &key.decoding_key, validation)) + } + + fn validate_and_decode_claims( + token: &str, + key: &DecodingKey, + validation: &Validation, + ) -> Option { + let claims = decode::(token, key, validation) + .inspect_err(|e| { + debug!("validate_and_decode_claims: problem {e:?}"); + }) + .ok()? + .claims; + + Some(AuthorizationClaims::from(claims)) } } -struct VerificationKey { - key_id: String, - decoding_key: DecodingKey, - algorithm: Option, +pub struct VerificationKey { + pub(crate) key_id: Option, + pub(crate) decoding_key: DecodingKey, } impl VerificationKey { - fn from_jwk(jwk: &Jwk) -> Result, AuthorizationError> { + fn from_jwk(jwk: Jwk) -> Result, AuthorizationError> { if jwk.common.public_key_use.as_ref().is_some_and(|key_use| key_use != &PublicKeyUse::Signature) || jwk.common.key_operations.as_ref().is_some_and(|operations| !operations.contains(&KeyOperations::Verify)) { return Ok(None); } - let Some(key_id) = jwk.common.key_id.as_ref().filter(|kid| !kid.trim().is_empty()) else { - return Ok(None); - }; - // Ignore symmetric and unsupported key types before decoding. - if !matches!( - jwk.algorithm, - jsonwebtoken::jwk::AlgorithmParameters::RSA(_) | jsonwebtoken::jwk::AlgorithmParameters::EllipticCurve(_) - ) { + + let decoding_key = DecodingKey::from_jwk(&jwk).map_err(AuthorizationError::InvalidKey)?; + if !matches!(decoding_key.family(), AlgorithmFamily::Rsa | AlgorithmFamily::Ec) { return Ok(None); } - let algorithm = match jwk.common.key_algorithm { - Some(alg) => match jsonwebtoken::Algorithm::try_from(alg) { - Ok(alg) => Some(alg), - Err(_) => return Ok(None), - }, - None => None, - }; - let decoding_key = DecodingKey::from_jwk(jwk).map_err(AuthorizationError::InvalidKey)?; - Ok(Some(Self { key_id: key_id.clone(), decoding_key, algorithm })) + + Ok(Some(Self { key_id: jwk.common.key_id, decoding_key })) } - fn matches(&self, header: &Header) -> bool { - header.kid.as_ref() == Some(&self.key_id) - && self.decoding_key.family() == header.alg.family() - && self.algorithm.is_none_or(|alg| alg == header.alg) + pub(super) fn matches(&self, header: &Header) -> bool { + self.decoding_key.family() == header.alg.family() + && header + .kid + .as_ref() + .is_none_or(|header_key_id| self.key_id.as_ref().is_none_or(|key_id| key_id == header_key_id)) } } @@ -133,7 +139,10 @@ async fn fetch_jwks(client: &reqwest::Client, url: &Url) -> Result JWKS_MAX_RESPONSE_BYTES as u64) { + if response + .content_length() + .is_some_and(|length| length > u64::try_from(JWKS_MAX_RESPONSE_BYTES).unwrap_or(u64::MAX)) + { return Err(AuthorizationError::JwksResponseTooLarge); } let mut body = Vec::new(); @@ -146,15 +155,19 @@ async fn fetch_jwks(client: &reqwest::Client, url: &Url) -> Result(&body).map_err(AuthorizationError::InvalidJson)?; - let mut keys: Vec = Vec::new(); - for jwk in jwks.keys { - if let Some(key) = VerificationKey::from_jwk(&jwk)? { - if keys.iter().any(|existing| existing.key_id == key.key_id) { - return Err(AuthorizationError::DuplicateKeyId); - } + if jwks.keys.is_empty() { Ok(Vec::new()) } else { validated_json_web_keys(jwks.keys) } +} + +pub(super) fn validated_json_web_keys( + jwks: impl IntoIterator, +) -> Result, AuthorizationError> { + let mut keys = Vec::new(); + for jwk in jwks { + if let Some(key) = VerificationKey::from_jwk(jwk)? { keys.push(key); } } + if keys.is_empty() { return Err(AuthorizationError::NoSupportedKeys); } diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs index d0a559d1..b5b13178 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::{JwksConfig, get_authorization_service}; use jsonwebtoken::{EncodingKey, encode}; use serde_json::json; @@ -15,17 +16,22 @@ fn public_key() -> Jwk { #[tokio::test] async fn verifies_signed_tokens_and_requires_trusted_claims() { let config = JwksConfig::default(); - let verifier = Jwks::new(reqwest::Client::new(), config.url.clone(), Jwks::validation(&config).unwrap()); + let verifier = Jwks::builder() + .client(reqwest::Client::new()) + .url(config.url) + .issuer(config.issuer) + .audiences(config.audiences) + .build(); verifier .cache .write() .await - .insert(JWKS_CACHE_KEY.to_owned(), vec![VerificationKey::from_jwk(&public_key()).unwrap().unwrap()]); + .insert(JWKS_CACHE_KEY.to_owned(), vec![VerificationKey::from_jwk(public_key()).unwrap().unwrap()]); let mut header = Header::new(Algorithm::RS256); header.kid = Some("test".into()); let claims = json!({"iss":"mcpgateway", "aud":"mcpgateway-api", "exp":jsonwebtoken::get_current_timestamp()+3600}); let signed = |claims: &Value, header: &Header| encode(header, claims, &signing_key()).unwrap(); - assert!(verifier.validate(&signed(&claims, &header), &header).await.is_ok()); + assert!(verifier.validate(&signed(&claims, &header), &header).await.is_some()); let mut invalid = Vec::new(); for (name, value) in [("iss", json!("other")), ("aud", json!("other")), ("exp", json!(1)), ("nbf", json!(9_999_999_999_u64))] @@ -40,34 +46,11 @@ async fn verifies_signed_tokens_and_requires_trusted_claims() { invalid.push(modified); } for claims in invalid { - assert_eq!(verifier.validate(&signed(&claims, &header), &header).await, Err(AuthenticationError::InvalidToken)); + assert!(verifier.validate(&signed(&claims, &header), &header).await.is_none()); } let mut wrong_algorithm = header.clone(); wrong_algorithm.alg = Algorithm::RS384; - assert_eq!( - verifier.validate(&signed(&claims, &wrong_algorithm), &wrong_algorithm).await, - Err(AuthenticationError::InvalidToken) - ); - header.kid = None; - assert_eq!(verifier.validate(&signed(&claims, &header), &header).await, Err(AuthenticationError::InvalidToken)); -} - -#[test] -fn selects_only_the_matching_signing_key() { - let mut jwk = public_key(); - let mut header = Header::new(Algorithm::RS256); - header.kid = Some("test".into()); - assert!(VerificationKey::from_jwk(&jwk).unwrap().unwrap().matches(&header)); - header.kid = Some("other".into()); - assert!(!VerificationKey::from_jwk(&jwk).unwrap().unwrap().matches(&header)); - header.kid = Some("test".into()); - jwk.common.key_algorithm = Some(jsonwebtoken::jwk::KeyAlgorithm::RS384); - assert!(!VerificationKey::from_jwk(&jwk).unwrap().unwrap().matches(&header)); - jwk.common.public_key_use = Some(PublicKeyUse::Encryption); - assert!(VerificationKey::from_jwk(&jwk).unwrap().is_none()); - jwk = public_key(); - jwk.common.key_id = None; - assert!(VerificationKey::from_jwk(&jwk).unwrap().is_none()); + assert!(verifier.validate(&signed(&claims, &wrong_algorithm), &wrong_algorithm).await.is_none()); } #[test] @@ -78,6 +61,6 @@ fn requires_explicit_issuer_and_audience() { JwksConfig { audiences: vec![], ..config.clone() }, JwksConfig { audiences: vec![String::new()], ..config }, ] { - assert!(Jwks::validation(&config).is_err()); + assert!(get_authorization_service(&config).is_err()); } } diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs index f1096e52..de2dd2e9 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs @@ -1,11 +1,10 @@ -use crate::JwksConfig; use crate::authorization::jwks::jwks::Jwks; use crate::authorization::{AuthorizationClaims, AuthorizationError, AuthorizationService}; use async_trait::async_trait; use jsonwebtoken::decode_header; use std::fmt; use std::net::IpAddr; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::time::Duration; use tracing::instrument; use url::Url; @@ -19,9 +18,16 @@ pub struct JwtAuthorizationService { } impl JwtAuthorizationService { - pub fn new(config: &JwksConfig) -> Result { - let validation = Jwks::validation(config)?; - let url = parse_jwks_url(config.url.clone())?; + pub fn from_jwks_url( + jwks_url: Url, + ca_cert_path: Option<&PathBuf>, + issuer: String, + audiences: Vec, + ) -> Result { + if issuer.trim().is_empty() || audiences.is_empty() || audiences.iter().any(|aud| aud.trim().is_empty()) { + return Err(AuthorizationError::InvalidTrustConfiguration); + } + let url = parse_jwks_url(jwks_url)?; let mut client = reqwest::Client::builder() .tls_backend_rustls() .connect_timeout(JWKS_CONNECT_TIMEOUT) @@ -29,16 +35,16 @@ impl JwtAuthorizationService { .timeout(JWKS_REQUEST_TIMEOUT) .redirect(reqwest::redirect::Policy::none()) .user_agent(concat!("mcp-ops/", env!("CARGO_PKG_VERSION"))); - if let Some(ca_cert_path) = &config.ca_cert_path { + if let Some(ca_cert_path) = ca_cert_path { client = client.tls_certs_only(load_ca_certificates(ca_cert_path)?); } let client = client.build().map_err(AuthorizationError::JwksRequest)?; - Ok(Self { jwks: Jwks::new(client, url, validation) }) + Ok(Self { jwks: Jwks::builder().client(client).url(url).issuer(issuer).audiences(audiences).build() }) } async fn authorize_token(&self, token: &str) -> Option { let header = decode_header(token).ok()?; - self.jwks.validate(token, &header).await.ok() + self.jwks.validate(token, &header).await } } @@ -91,15 +97,322 @@ fn load_ca_certificates(path: &Path) -> Result, Author } #[cfg(test)] -mod tests { - use super::*; +mod test { + use crate::{ + Config, + authorization::{AuthorizationClaims, Scopes}, + common::ContextForgeDataPlaneAppState, + config_stores::ConfigStoreError, + layers::claims_id::claims_layer, + }; + use crate::{ + authorization::{ + AuthorizationError, + jwks::{ + JwtAuthorizationService, + jwks::{JWKS_CACHE_KEY, Jwks, VerificationKey}, + jwks_authorization::{JWKS_CONNECT_TIMEOUT, JWKS_READ_TIMEOUT, JWKS_REQUEST_TIMEOUT}, + }, + }, + config_stores::ConfigStore, + }; + use async_trait::async_trait; + use axum::{Router, body::Body, middleware, response::Response, routing::get}; + + use contextforge_data_plane_apis::{User, user_store::UserConfig}; + use http::{HeaderMap, Request, StatusCode}; + use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, Header, encode}; + use lru_time_cache::LruCache; + use serde_json::json; + + use std::sync::{Arc, Once}; + use std::{str::FromStr, time::Duration}; + use tokio::sync::RwLock; + use tower::ServiceExt; + + use url::Url; + use uuid::Uuid; + + const GATEWAY_AUDIENCE: &str = "audience"; + const GATEWAY_ISSUER: &str = "issuer"; + + impl VerificationKey { + pub fn new(id: Option, decoding_key: DecodingKey) -> Self { + Self { key_id: id, decoding_key } + } + } + + impl AuthorizationClaims { + fn clear(&mut self, name: &str) { + if let Some(value) = self.value.get_mut(name) { + *value = serde_json::Value::Null; + } + } + + fn set(&mut self, name: &str, new_value: serde_json::Value) { + if let Some(value) = self.value.get_mut(name) { + *value = new_value; + } + } + fn get(&mut self, name: &str) -> Option<&serde_json::Value> { + self.value.get(name) + } + } + + impl JwtAuthorizationService { + pub async fn from_keys(verification_keys: Vec) -> Result { + let url: Url = Url::from_str("http://127.0.0.1:0/").expect("this should work"); + let client = reqwest::Client::builder() + .tls_backend_rustls() + .connect_timeout(JWKS_CONNECT_TIMEOUT) + .read_timeout(JWKS_READ_TIMEOUT) + .timeout(JWKS_REQUEST_TIMEOUT) + .redirect(reqwest::redirect::Policy::none()) + .user_agent(concat!("mcp-ops/", env!("CARGO_PKG_VERSION"))); + + let client = client.build().map_err(AuthorizationError::JwksRequest)?; + + let cache = RwLock::new(LruCache::with_expiry_duration(Duration::from_hours(100))); + let mut guard = cache.write().await; + guard.insert(JWKS_CACHE_KEY.to_owned(), verification_keys); + drop(guard); + + Ok(Self { + jwks: Jwks::builder() + .cache(cache) + .client(client) + .url(url) + .issuer(GATEWAY_ISSUER.to_owned()) + .audiences(vec![GATEWAY_AUDIENCE.to_owned()]) + .build(), + }) + } + } + + static CRYPTO: Once = Once::new(); + const RSA_PRIVATE_KEY: &[u8] = include_bytes!("../../../../../assets/jwt.key"); + const RSA_PUBLIC_KEY: &[u8] = include_bytes!("../../../../../assets/jwt.key.pub"); + + fn now_epoch_seconds() -> u64 { + std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).expect("Time went backwards").as_secs() + } + + fn active_test_claims() -> AuthorizationClaims { + let now = now_epoch_seconds(); + let user_id = "11111111-1111-1111-1111-111111111111".to_owned(); + + let map = json!( { + "iss": GATEWAY_ISSUER.to_owned(), + "sub": user_id.clone(), + "aud": GATEWAY_AUDIENCE.to_owned(), + "exp": now + Duration::from_hours(1).as_secs(), + "nbf": now - Duration::from_mins(1).as_secs(), + "iat": now, + "jti": Uuid::new_v4().to_string(), + "token_use": Some("api".to_owned()), + "teams": vec!["team_awesome".to_owned()], + "user": crate::authorization::User::builder() + .tenant_id("team_awesome".to_owned()) + .user_id(user_id.clone()) + .build(), + "scopes": Scopes::builder() + .server_id(Some("my_id".to_owned())) + .ip_restrictions(vec!["192.169.1.0/24".to_owned()]) + .permissions(vec!["tools.read".to_owned(), "servers.use".to_owned()]) + .time_restrictions(None) + .build(), + "tenant_id": "tenant".to_owned(), + }); + AuthorizationClaims::from(map) + } + + fn get_rsa_token_for_claims(claims: &AuthorizationClaims) -> String { + let key = EncodingKey::from_rsa_pem(RSA_PRIVATE_KEY).expect("RSA test signing key"); + let header = Header::new(Algorithm::RS256); + let claims = claims.value.clone(); + encode::(&header, &claims, &key).expect("Expecting this to work") + } + + #[derive(Debug, Clone)] + struct MockedUserConfigStore; + #[async_trait] + impl ConfigStore for MockedUserConfigStore { + async fn get_config<'a>(&self, _: &'a User) -> Result { + Err(ConfigStoreError::InvalidConnection) + } + + async fn set_config<'a>(&self, _: &'a User, _: &'a UserConfig) -> Result<(), ConfigStoreError> { + Err(ConfigStoreError::InvalidConnection) + } + } + #[test] - fn only_trusted_transport_urls_are_accepted() { - for url in ["https://issuer.example/keys", "http://localhost/keys", "http://127.0.0.1/keys"] { - assert!(parse_jwks_url(url.parse().unwrap()).is_ok(), "{url}"); + fn test_active_token() { + let mut claims = active_test_claims(); + assert_ne!(claims.get("exp").and_then(serde_json::Value::as_i64), Some(0_i64)); + claims.set("exp", 0.into()); + assert_eq!(claims.get("exp").and_then(serde_json::Value::as_i64), Some(0_i64)); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + #[allow(clippy::items_after_statements)] + #[test_log::test] + async fn claim_test_valid_rsa() { + CRYPTO.call_once(|| { + _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + }); + + async fn handle(_: HeaderMap) -> Response { + Response::builder().status(StatusCode::OK).body(Body::empty()).expect("Expecting this to work") + } + + let token = get_rsa_token_for_claims(&active_test_claims()); + + let decoding_key = DecodingKey::from_rsa_pem(RSA_PUBLIC_KEY).expect("RSA test verification key"); + let verfication_key = VerificationKey::new(Some("RS256".to_owned()), decoding_key); + + let state = ContextForgeDataPlaneAppState { + authorization_service: Arc::new( + JwtAuthorizationService::from_keys(vec![verfication_key]).await.expect("this should work"), + ), + config_store: Arc::new(MockedUserConfigStore {}), + config: Config::default(), + }; + let http_requst = Request::builder() + .header("Authorization", format!("Bearer {token}")) + .method("GET") + .body(Body::empty()) + .expect("This should work"); + + let app = + Router::new().route("/", get(handle)).layer(middleware::from_fn_with_state(state.clone(), claims_layer)); + + let res = app.oneshot(http_requst).await.unwrap(); + assert_eq!(res.status(), StatusCode::OK); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + #[allow(clippy::items_after_statements)] + async fn claim_test_missing_scopes_is_allowed() { + CRYPTO.call_once(|| { + _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + }); + + async fn handle(_: HeaderMap) -> Response { + Response::builder().status(StatusCode::OK).body(Body::empty()).expect("Expecting this to work") + } + + let mut claims = active_test_claims(); + claims.clear("scopes"); + let token = get_rsa_token_for_claims(&claims); + + let decoding_key = DecodingKey::from_rsa_pem(RSA_PUBLIC_KEY).expect("RSA test verification key"); + let verfication_key = VerificationKey::new(Some("RS256".to_owned()), decoding_key); + let state = ContextForgeDataPlaneAppState { + authorization_service: Arc::new( + JwtAuthorizationService::from_keys(vec![verfication_key]).await.expect("this should work"), + ), + config_store: Arc::new(MockedUserConfigStore {}), + config: Config::default(), + }; + let http_requst = Request::builder() + .header("Authorization", format!("Bearer {token}")) + .method("GET") + .body(Body::empty()) + .expect("This should work"); + + let app = + Router::new().route("/", get(handle)).layer(middleware::from_fn_with_state(state.clone(), claims_layer)); + + let res = app.oneshot(http_requst).await.unwrap(); + assert_eq!(res.status(), StatusCode::OK); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + #[allow(clippy::items_after_statements)] + async fn claim_test_missing_token_use_and_full_name_is_allowed() { + CRYPTO.call_once(|| { + _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + }); + let user_id = "11111111-1111-1111-1111-111111111111".to_owned(); + + async fn handle(_: HeaderMap) -> Response { + Response::builder().status(StatusCode::OK).body(Body::empty()).expect("Expecting this to work") } - for url in ["http://issuer.example/keys", "file:///keys"] { - assert!(parse_jwks_url(url.parse().unwrap()).is_err(), "{url}"); + + let mut claims = active_test_claims(); + claims.clear("token_use"); + claims.set( + "user", + serde_json::to_value( + crate::authorization::User::builder() + .tenant_id("team_awesome".to_owned()) + .user_id(user_id.clone()) + .build(), + ) + .expect("should work"), + ); + + let token = get_rsa_token_for_claims(&claims); + + let decoding_key = DecodingKey::from_rsa_pem(RSA_PUBLIC_KEY).expect("RSA test verification key"); + let verfication_key = VerificationKey::new(Some("RS256".to_owned()), decoding_key); + + let state = ContextForgeDataPlaneAppState { + authorization_service: Arc::new( + JwtAuthorizationService::from_keys(vec![verfication_key]).await.expect("this should work"), + ), + config_store: Arc::new(MockedUserConfigStore {}), + config: Config::default(), + }; + let http_requst = Request::builder() + .header("Authorization", format!("Bearer {token}")) + .method("GET") + .body(Body::empty()) + .expect("This should work"); + + let app = + Router::new().route("/", get(handle)).layer(middleware::from_fn_with_state(state.clone(), claims_layer)); + + let res = app.oneshot(http_requst).await.unwrap(); + assert_eq!(res.status(), StatusCode::OK); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + #[allow(clippy::items_after_statements)] + async fn claim_test_expired_token() { + CRYPTO.call_once(|| { + _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + }); + + async fn handle(_: HeaderMap) -> Response { + Response::builder().status(StatusCode::OK).body(Body::empty()).expect("Expecting this to work") } + + let mut claims = active_test_claims(); + claims.set("exp", 1000.into()); + let token = get_rsa_token_for_claims(&claims); + + let decoding_key = DecodingKey::from_rsa_pem(RSA_PUBLIC_KEY).expect("RSA test verification key"); + let verfication_key = VerificationKey::new(Some("RS256".to_owned()), decoding_key); + + let state = ContextForgeDataPlaneAppState { + authorization_service: Arc::new( + JwtAuthorizationService::from_keys(vec![verfication_key]).await.expect("this should work"), + ), + config_store: Arc::new(MockedUserConfigStore {}), + config: Config::default(), + }; + let http_requst = Request::builder() + .header("Authorization", format!("Bearer {token}")) + .method("GET") + .body(Body::empty()) + .expect("This should work"); + + let app = + Router::new().route("/", get(handle)).layer(middleware::from_fn_with_state(state.clone(), claims_layer)); + + let res = app.oneshot(http_requst).await.unwrap(); + assert_eq!(res.status(), StatusCode::UNAUTHORIZED); } } diff --git a/crates/contextforge-data-plane-lib/src/authorization/mod.rs b/crates/contextforge-data-plane-lib/src/authorization/mod.rs index 175aa957..21a0b8f5 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/mod.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/mod.rs @@ -24,7 +24,12 @@ pub enum Permission { pub fn get_authorization_service( config: &JwksConfig, ) -> Result, AuthorizationError> { - let service = jwks::JwtAuthorizationService::new(config)?; + let service = jwks::JwtAuthorizationService::from_jwks_url( + config.url.clone(), + config.ca_cert_path.as_ref(), + config.issuer.clone(), + config.audiences.clone(), + )?; Ok(Arc::new(service) as Arc) } @@ -37,8 +42,6 @@ pub trait AuthorizationService: std::fmt::Debug { pub enum AuthenticationError { #[error("invalid bearer token")] InvalidToken, - #[error("verification keys unavailable")] - KeysUnavailable, } #[derive(Debug, thiserror::Error)] @@ -46,8 +49,6 @@ pub enum AuthenticationError { pub enum AuthorizationError { #[error("JWT trust configuration requires a nonempty issuer and audience")] InvalidTrustConfiguration, - #[error("JWKS contains duplicate signing key IDs")] - DuplicateKeyId, #[error("SaaS JWKS contains no supported signing keys")] NoSupportedKeys, #[error("SaaS JWKS is invalid")] From ffc93b32b78241764e67e1370c2ae81c680e3b8c Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Thu, 24 Sep 2026 15:16:03 +0100 Subject: [PATCH 08/19] test: keep JWKS unit tests inline Signed-off-by: Pratik Gandhi --- .../src/authorization/jwks/jwks.rs | 70 ++++++++++++++++++- .../src/authorization/jwks/jwks/tests.rs | 66 ----------------- 2 files changed, 69 insertions(+), 67 deletions(-) delete mode 100644 crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs index 663b889b..9c574bb2 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs @@ -175,4 +175,72 @@ pub(super) fn validated_json_web_keys( } #[cfg(test)] -mod tests; +mod tests { + use super::*; + use crate::{JwksConfig, get_authorization_service}; + use jsonwebtoken::{EncodingKey, encode}; + use serde_json::json; + + fn signing_key() -> EncodingKey { + EncodingKey::from_rsa_pem(include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/../../assets/jwt.key"))).unwrap() + } + + fn public_key() -> Jwk { + let mut jwk = Jwk::from_encoding_key(&signing_key(), Algorithm::RS256).unwrap(); + jwk.common.key_id = Some("test".into()); + jwk + } + + #[tokio::test] + async fn verifies_signed_tokens_and_requires_trusted_claims() { + let config = JwksConfig::default(); + let verifier = Jwks::builder() + .client(reqwest::Client::new()) + .url(config.url) + .issuer(config.issuer) + .audiences(config.audiences) + .build(); + verifier + .cache + .write() + .await + .insert(JWKS_CACHE_KEY.to_owned(), vec![VerificationKey::from_jwk(public_key()).unwrap().unwrap()]); + let mut header = Header::new(Algorithm::RS256); + header.kid = Some("test".into()); + let claims = + json!({"iss":"mcpgateway", "aud":"mcpgateway-api", "exp":jsonwebtoken::get_current_timestamp()+3600}); + let signed = |claims: &Value, header: &Header| encode(header, claims, &signing_key()).unwrap(); + assert!(verifier.validate(&signed(&claims, &header), &header).await.is_some()); + let mut invalid = Vec::new(); + for (name, value) in + [("iss", json!("other")), ("aud", json!("other")), ("exp", json!(1)), ("nbf", json!(9_999_999_999_u64))] + { + let mut modified = claims.clone(); + modified[name] = value; + invalid.push(modified); + } + for name in ["iss", "aud", "exp"] { + let mut modified = claims.clone(); + modified.as_object_mut().unwrap().remove(name); + invalid.push(modified); + } + for claims in invalid { + assert!(verifier.validate(&signed(&claims, &header), &header).await.is_none()); + } + let mut wrong_algorithm = header.clone(); + wrong_algorithm.alg = Algorithm::RS384; + assert!(verifier.validate(&signed(&claims, &wrong_algorithm), &wrong_algorithm).await.is_none()); + } + + #[test] + fn requires_explicit_issuer_and_audience() { + let config = JwksConfig::default(); + for config in [ + JwksConfig { issuer: String::new(), ..config.clone() }, + JwksConfig { audiences: vec![], ..config.clone() }, + JwksConfig { audiences: vec![String::new()], ..config }, + ] { + assert!(get_authorization_service(&config).is_err()); + } + } +} diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs deleted file mode 100644 index b5b13178..00000000 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks/tests.rs +++ /dev/null @@ -1,66 +0,0 @@ -use super::*; -use crate::{JwksConfig, get_authorization_service}; -use jsonwebtoken::{EncodingKey, encode}; -use serde_json::json; - -fn signing_key() -> EncodingKey { - EncodingKey::from_rsa_pem(include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/../../assets/jwt.key"))).unwrap() -} - -fn public_key() -> Jwk { - let mut jwk = Jwk::from_encoding_key(&signing_key(), Algorithm::RS256).unwrap(); - jwk.common.key_id = Some("test".into()); - jwk -} - -#[tokio::test] -async fn verifies_signed_tokens_and_requires_trusted_claims() { - let config = JwksConfig::default(); - let verifier = Jwks::builder() - .client(reqwest::Client::new()) - .url(config.url) - .issuer(config.issuer) - .audiences(config.audiences) - .build(); - verifier - .cache - .write() - .await - .insert(JWKS_CACHE_KEY.to_owned(), vec![VerificationKey::from_jwk(public_key()).unwrap().unwrap()]); - let mut header = Header::new(Algorithm::RS256); - header.kid = Some("test".into()); - let claims = json!({"iss":"mcpgateway", "aud":"mcpgateway-api", "exp":jsonwebtoken::get_current_timestamp()+3600}); - let signed = |claims: &Value, header: &Header| encode(header, claims, &signing_key()).unwrap(); - assert!(verifier.validate(&signed(&claims, &header), &header).await.is_some()); - let mut invalid = Vec::new(); - for (name, value) in - [("iss", json!("other")), ("aud", json!("other")), ("exp", json!(1)), ("nbf", json!(9_999_999_999_u64))] - { - let mut modified = claims.clone(); - modified[name] = value; - invalid.push(modified); - } - for name in ["iss", "aud", "exp"] { - let mut modified = claims.clone(); - modified.as_object_mut().unwrap().remove(name); - invalid.push(modified); - } - for claims in invalid { - assert!(verifier.validate(&signed(&claims, &header), &header).await.is_none()); - } - let mut wrong_algorithm = header.clone(); - wrong_algorithm.alg = Algorithm::RS384; - assert!(verifier.validate(&signed(&claims, &wrong_algorithm), &wrong_algorithm).await.is_none()); -} - -#[test] -fn requires_explicit_issuer_and_audience() { - let config = JwksConfig::default(); - for config in [ - JwksConfig { issuer: String::new(), ..config.clone() }, - JwksConfig { audiences: vec![], ..config.clone() }, - JwksConfig { audiences: vec![String::new()], ..config }, - ] { - assert!(get_authorization_service(&config).is_err()); - } -} From a677bd96786eaa29e4808a0d349c1364f29258ea Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Thu, 24 Sep 2026 15:39:13 +0100 Subject: [PATCH 09/19] auth: restore original JWT algorithm handling and test fixtures Signed-off-by: Pratik Gandhi --- .../src/authorization/jwks/jwks.rs | 14 ++++---- .../authorization/jwks/jwks_authorization.rs | 35 +++++++++---------- 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs index 9c574bb2..0935c0ca 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs @@ -37,8 +37,8 @@ pub(super) struct Jwks { } impl Jwks { - fn validation(&self) -> Validation { - let mut validation = Validation::new(Algorithm::RS256); + fn validation(&self, alg: Algorithm) -> Validation { + let mut validation = Validation::new(alg); validation.set_required_spec_claims(&["exp", "iss", "aud"]); validation.set_issuer(&[&self.issuer]); validation.set_audience(&self.audiences); @@ -55,14 +55,14 @@ impl Jwks { if let Some(keys) = cache.peek(JWKS_CACHE_KEY) && keys.iter().any(|key| key.matches(header)) { - return Self::validate_with_keys(keys, token, header, &self.validation()); + return Self::validate_with_keys(keys, token, header, &self.validation(header.alg)); } } match fetch_jwks(&self.client, &self.url).await { Ok(keys) => { let key_count = keys.len(); - let claims = Self::validate_with_keys(&keys, token, header, &self.validation()); + let claims = Self::validate_with_keys(&keys, token, header, &self.validation(header.alg)); self.cache.write().await.insert(JWKS_CACHE_KEY.to_owned(), keys); tracing::info!("validate: SaaS JWKS cache refreshed {key_count}"); @@ -227,9 +227,9 @@ mod tests { for claims in invalid { assert!(verifier.validate(&signed(&claims, &header), &header).await.is_none()); } - let mut wrong_algorithm = header.clone(); - wrong_algorithm.alg = Algorithm::RS384; - assert!(verifier.validate(&signed(&claims, &wrong_algorithm), &wrong_algorithm).await.is_none()); + let mut another_rsa_algorithm = header.clone(); + another_rsa_algorithm.alg = Algorithm::RS384; + assert!(verifier.validate(&signed(&claims, &another_rsa_algorithm), &another_rsa_algorithm).await.is_some()); } #[test] diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs index de2dd2e9..566d89fc 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs @@ -190,8 +190,7 @@ mod test { } static CRYPTO: Once = Once::new(); - const RSA_PRIVATE_KEY: &[u8] = include_bytes!("../../../../../assets/jwt.key"); - const RSA_PUBLIC_KEY: &[u8] = include_bytes!("../../../../../assets/jwt.key.pub"); + const HMAC_SECRET: &[u8] = b"my-test-key-but-now-longer-than-32-bytes"; fn now_epoch_seconds() -> u64 { std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).expect("Time went backwards").as_secs() @@ -226,9 +225,9 @@ mod test { AuthorizationClaims::from(map) } - fn get_rsa_token_for_claims(claims: &AuthorizationClaims) -> String { - let key = EncodingKey::from_rsa_pem(RSA_PRIVATE_KEY).expect("RSA test signing key"); - let header = Header::new(Algorithm::RS256); + fn get_hmac_token_for_claims(claims: &AuthorizationClaims) -> String { + let key = EncodingKey::from_secret(HMAC_SECRET); + let header = Header::new(Algorithm::HS256); let claims = claims.value.clone(); encode::(&header, &claims, &key).expect("Expecting this to work") } @@ -257,7 +256,7 @@ mod test { #[tokio::test(flavor = "multi_thread", worker_threads = 1)] #[allow(clippy::items_after_statements)] #[test_log::test] - async fn claim_test_valid_rsa() { + async fn claim_test_valid_hmac() { CRYPTO.call_once(|| { _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); }); @@ -266,10 +265,10 @@ mod test { Response::builder().status(StatusCode::OK).body(Body::empty()).expect("Expecting this to work") } - let token = get_rsa_token_for_claims(&active_test_claims()); + let token = get_hmac_token_for_claims(&active_test_claims()); - let decoding_key = DecodingKey::from_rsa_pem(RSA_PUBLIC_KEY).expect("RSA test verification key"); - let verfication_key = VerificationKey::new(Some("RS256".to_owned()), decoding_key); + let decoding_key = DecodingKey::from_secret(HMAC_SECRET); + let verfication_key = VerificationKey::new(Some("HS256".to_owned()), decoding_key); let state = ContextForgeDataPlaneAppState { authorization_service: Arc::new( @@ -304,10 +303,10 @@ mod test { let mut claims = active_test_claims(); claims.clear("scopes"); - let token = get_rsa_token_for_claims(&claims); + let token = get_hmac_token_for_claims(&claims); - let decoding_key = DecodingKey::from_rsa_pem(RSA_PUBLIC_KEY).expect("RSA test verification key"); - let verfication_key = VerificationKey::new(Some("RS256".to_owned()), decoding_key); + let decoding_key = DecodingKey::from_secret(HMAC_SECRET); + let verfication_key = VerificationKey::new(Some("HS256".to_owned()), decoding_key); let state = ContextForgeDataPlaneAppState { authorization_service: Arc::new( JwtAuthorizationService::from_keys(vec![verfication_key]).await.expect("this should work"), @@ -353,10 +352,10 @@ mod test { .expect("should work"), ); - let token = get_rsa_token_for_claims(&claims); + let token = get_hmac_token_for_claims(&claims); - let decoding_key = DecodingKey::from_rsa_pem(RSA_PUBLIC_KEY).expect("RSA test verification key"); - let verfication_key = VerificationKey::new(Some("RS256".to_owned()), decoding_key); + let decoding_key = DecodingKey::from_secret(HMAC_SECRET); + let verfication_key = VerificationKey::new(Some("HS256".to_owned()), decoding_key); let state = ContextForgeDataPlaneAppState { authorization_service: Arc::new( @@ -391,10 +390,10 @@ mod test { let mut claims = active_test_claims(); claims.set("exp", 1000.into()); - let token = get_rsa_token_for_claims(&claims); + let token = get_hmac_token_for_claims(&claims); - let decoding_key = DecodingKey::from_rsa_pem(RSA_PUBLIC_KEY).expect("RSA test verification key"); - let verfication_key = VerificationKey::new(Some("RS256".to_owned()), decoding_key); + let decoding_key = DecodingKey::from_secret(HMAC_SECRET); + let verfication_key = VerificationKey::new(Some("HS256".to_owned()), decoding_key); let state = ContextForgeDataPlaneAppState { authorization_service: Arc::new( From 18c52a871286f4365d4b30abbc2601f37c83641b Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Thu, 24 Sep 2026 16:01:49 +0100 Subject: [PATCH 10/19] auth: preserve optional JWT expiry Signed-off-by: Pratik Gandhi --- .../src/authorization/jwks/jwks.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs index 0935c0ca..2f882e1a 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs @@ -39,7 +39,7 @@ pub(super) struct Jwks { impl Jwks { fn validation(&self, alg: Algorithm) -> Validation { let mut validation = Validation::new(alg); - validation.set_required_spec_claims(&["exp", "iss", "aud"]); + validation.set_required_spec_claims(&["iss", "aud"]); validation.set_issuer(&[&self.issuer]); validation.set_audience(&self.audiences); validation.validate_aud = self.validate_audience; @@ -211,6 +211,9 @@ mod tests { json!({"iss":"mcpgateway", "aud":"mcpgateway-api", "exp":jsonwebtoken::get_current_timestamp()+3600}); let signed = |claims: &Value, header: &Header| encode(header, claims, &signing_key()).unwrap(); assert!(verifier.validate(&signed(&claims, &header), &header).await.is_some()); + let mut without_expiry = claims.clone(); + without_expiry.as_object_mut().unwrap().remove("exp"); + assert!(verifier.validate(&signed(&without_expiry, &header), &header).await.is_some()); let mut invalid = Vec::new(); for (name, value) in [("iss", json!("other")), ("aud", json!("other")), ("exp", json!(1)), ("nbf", json!(9_999_999_999_u64))] @@ -219,7 +222,7 @@ mod tests { modified[name] = value; invalid.push(modified); } - for name in ["iss", "aud", "exp"] { + for name in ["iss", "aud"] { let mut modified = claims.clone(); modified.as_object_mut().unwrap().remove(name); invalid.push(modified); From c06de20fcd699ce458e8019d57d0a1c19b00a880 Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Thu, 24 Sep 2026 16:19:27 +0100 Subject: [PATCH 11/19] auth: allow optional issuer and audience claims Signed-off-by: Pratik Gandhi --- .../src/authorization/jwks/jwks.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs index 2f882e1a..cebaf837 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs @@ -39,7 +39,7 @@ pub(super) struct Jwks { impl Jwks { fn validation(&self, alg: Algorithm) -> Validation { let mut validation = Validation::new(alg); - validation.set_required_spec_claims(&["iss", "aud"]); + validation.required_spec_claims.clear(); validation.set_issuer(&[&self.issuer]); validation.set_audience(&self.audiences); validation.validate_aud = self.validate_audience; @@ -225,7 +225,7 @@ mod tests { for name in ["iss", "aud"] { let mut modified = claims.clone(); modified.as_object_mut().unwrap().remove(name); - invalid.push(modified); + assert!(verifier.validate(&signed(&modified, &header), &header).await.is_some()); } for claims in invalid { assert!(verifier.validate(&signed(&claims, &header), &header).await.is_none()); From 9b08e4a1cedc43044eec7e99964dc4119694bec8 Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Thu, 24 Sep 2026 16:32:17 +0100 Subject: [PATCH 12/19] auth: restore original audience validation default Signed-off-by: Pratik Gandhi --- .../src/authorization/jwks/jwks.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs index cebaf837..ddaef0b8 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs @@ -28,7 +28,7 @@ pub(super) struct Jwks { audiences: Vec, #[builder(default = RwLock::new(LruCache::with_expiry_duration(JWKS_CACHE_TTL)))] cache: RwLock>>, - #[builder(default = true)] + #[builder(default = false)] validate_audience: bool, #[builder(default = true)] validate_expiry: bool, @@ -214,10 +214,11 @@ mod tests { let mut without_expiry = claims.clone(); without_expiry.as_object_mut().unwrap().remove("exp"); assert!(verifier.validate(&signed(&without_expiry, &header), &header).await.is_some()); + let mut other_audience = claims.clone(); + other_audience["aud"] = json!("other"); + assert!(verifier.validate(&signed(&other_audience, &header), &header).await.is_some()); let mut invalid = Vec::new(); - for (name, value) in - [("iss", json!("other")), ("aud", json!("other")), ("exp", json!(1)), ("nbf", json!(9_999_999_999_u64))] - { + for (name, value) in [("iss", json!("other")), ("exp", json!(1)), ("nbf", json!(9_999_999_999_u64))] { let mut modified = claims.clone(); modified[name] = value; invalid.push(modified); From 7c1d2416189ab8e3d24ed5cdac1616b0d9de20b8 Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Thu, 24 Sep 2026 16:49:00 +0100 Subject: [PATCH 13/19] auth: remove PoC issuer and audience configuration Signed-off-by: Pratik Gandhi --- .../src/authorization/jwks/jwks.rs | 37 +++++-------------- .../authorization/jwks/jwks_authorization.rs | 22 ++--------- .../src/authorization/mod.rs | 9 +---- .../src/common/cli_config.rs | 8 ---- .../src/common/config.rs | 20 +--------- .../contextforge-data-plane-lib/src/tools.rs | 4 +- .../tests/gateway/harness/mod.rs | 7 +--- docker/docker-compose.yml | 6 --- 8 files changed, 18 insertions(+), 95 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs index ddaef0b8..ea092d5c 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs @@ -24,8 +24,6 @@ const JWKS_MAX_RESPONSE_BYTES: usize = 1024 * 1024; pub(super) struct Jwks { client: reqwest::Client, url: Url, - issuer: String, - audiences: Vec, #[builder(default = RwLock::new(LruCache::with_expiry_duration(JWKS_CACHE_TTL)))] cache: RwLock>>, #[builder(default = false)] @@ -40,8 +38,6 @@ impl Jwks { fn validation(&self, alg: Algorithm) -> Validation { let mut validation = Validation::new(alg); validation.required_spec_claims.clear(); - validation.set_issuer(&[&self.issuer]); - validation.set_audience(&self.audiences); validation.validate_aud = self.validate_audience; validation.validate_exp = self.validate_expiry; validation.validate_nbf = self.validate_not_before; @@ -177,7 +173,7 @@ pub(super) fn validated_json_web_keys( #[cfg(test)] mod tests { use super::*; - use crate::{JwksConfig, get_authorization_service}; + use crate::JwksConfig; use jsonwebtoken::{EncodingKey, encode}; use serde_json::json; @@ -192,14 +188,9 @@ mod tests { } #[tokio::test] - async fn verifies_signed_tokens_and_requires_trusted_claims() { + async fn verifies_signed_tokens_with_optional_claims() { let config = JwksConfig::default(); - let verifier = Jwks::builder() - .client(reqwest::Client::new()) - .url(config.url) - .issuer(config.issuer) - .audiences(config.audiences) - .build(); + let verifier = Jwks::builder().client(reqwest::Client::new()).url(config.url).build(); verifier .cache .write() @@ -214,11 +205,13 @@ mod tests { let mut without_expiry = claims.clone(); without_expiry.as_object_mut().unwrap().remove("exp"); assert!(verifier.validate(&signed(&without_expiry, &header), &header).await.is_some()); - let mut other_audience = claims.clone(); - other_audience["aud"] = json!("other"); - assert!(verifier.validate(&signed(&other_audience, &header), &header).await.is_some()); + for name in ["iss", "aud"] { + let mut modified = claims.clone(); + modified[name] = json!("other"); + assert!(verifier.validate(&signed(&modified, &header), &header).await.is_some()); + } let mut invalid = Vec::new(); - for (name, value) in [("iss", json!("other")), ("exp", json!(1)), ("nbf", json!(9_999_999_999_u64))] { + for (name, value) in [("exp", json!(1)), ("nbf", json!(9_999_999_999_u64))] { let mut modified = claims.clone(); modified[name] = value; invalid.push(modified); @@ -235,16 +228,4 @@ mod tests { another_rsa_algorithm.alg = Algorithm::RS384; assert!(verifier.validate(&signed(&claims, &another_rsa_algorithm), &another_rsa_algorithm).await.is_some()); } - - #[test] - fn requires_explicit_issuer_and_audience() { - let config = JwksConfig::default(); - for config in [ - JwksConfig { issuer: String::new(), ..config.clone() }, - JwksConfig { audiences: vec![], ..config.clone() }, - JwksConfig { audiences: vec![String::new()], ..config }, - ] { - assert!(get_authorization_service(&config).is_err()); - } - } } diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs index 566d89fc..dea3e91a 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks_authorization.rs @@ -18,15 +18,7 @@ pub struct JwtAuthorizationService { } impl JwtAuthorizationService { - pub fn from_jwks_url( - jwks_url: Url, - ca_cert_path: Option<&PathBuf>, - issuer: String, - audiences: Vec, - ) -> Result { - if issuer.trim().is_empty() || audiences.is_empty() || audiences.iter().any(|aud| aud.trim().is_empty()) { - return Err(AuthorizationError::InvalidTrustConfiguration); - } + pub fn from_jwks_url(jwks_url: Url, ca_cert_path: Option<&PathBuf>) -> Result { let url = parse_jwks_url(jwks_url)?; let mut client = reqwest::Client::builder() .tls_backend_rustls() @@ -39,7 +31,7 @@ impl JwtAuthorizationService { client = client.tls_certs_only(load_ca_certificates(ca_cert_path)?); } let client = client.build().map_err(AuthorizationError::JwksRequest)?; - Ok(Self { jwks: Jwks::builder().client(client).url(url).issuer(issuer).audiences(audiences).build() }) + Ok(Self { jwks: Jwks::builder().client(client).url(url).build() }) } async fn authorize_token(&self, token: &str) -> Option { @@ -177,15 +169,7 @@ mod test { guard.insert(JWKS_CACHE_KEY.to_owned(), verification_keys); drop(guard); - Ok(Self { - jwks: Jwks::builder() - .cache(cache) - .client(client) - .url(url) - .issuer(GATEWAY_ISSUER.to_owned()) - .audiences(vec![GATEWAY_AUDIENCE.to_owned()]) - .build(), - }) + Ok(Self { jwks: Jwks::builder().cache(cache).client(client).url(url).build() }) } } diff --git a/crates/contextforge-data-plane-lib/src/authorization/mod.rs b/crates/contextforge-data-plane-lib/src/authorization/mod.rs index 21a0b8f5..eb42269f 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/mod.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/mod.rs @@ -24,12 +24,7 @@ pub enum Permission { pub fn get_authorization_service( config: &JwksConfig, ) -> Result, AuthorizationError> { - let service = jwks::JwtAuthorizationService::from_jwks_url( - config.url.clone(), - config.ca_cert_path.as_ref(), - config.issuer.clone(), - config.audiences.clone(), - )?; + let service = jwks::JwtAuthorizationService::from_jwks_url(config.url.clone(), config.ca_cert_path.as_ref())?; Ok(Arc::new(service) as Arc) } @@ -47,8 +42,6 @@ pub enum AuthenticationError { #[derive(Debug, thiserror::Error)] #[allow(dead_code)] pub enum AuthorizationError { - #[error("JWT trust configuration requires a nonempty issuer and audience")] - InvalidTrustConfiguration, #[error("SaaS JWKS contains no supported signing keys")] NoSupportedKeys, #[error("SaaS JWKS is invalid")] diff --git a/crates/contextforge-data-plane-lib/src/common/cli_config.rs b/crates/contextforge-data-plane-lib/src/common/cli_config.rs index 74921777..477b20af 100644 --- a/crates/contextforge-data-plane-lib/src/common/cli_config.rs +++ b/crates/contextforge-data-plane-lib/src/common/cli_config.rs @@ -17,14 +17,6 @@ pub struct CliConfig { #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWKS_CA_PATH")] pub jwks_ca_cert_path: Option, - /// Exact trusted token issuer; never discovered from the incoming token. - #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_ISSUER")] - pub jwt_issuer: String, - - /// Accepted audiences (comma-separated); at least one must match. - #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES", value_delimiter = ',', required = true)] - pub jwt_audiences: Vec, - #[arg(long, env = "CONTEXTFORGE_DATA_PLANE_ENABLE_OPEN_TELEMETRY")] pub enable_open_telemetry: Option, diff --git a/crates/contextforge-data-plane-lib/src/common/config.rs b/crates/contextforge-data-plane-lib/src/common/config.rs index 55186540..18e9059b 100644 --- a/crates/contextforge-data-plane-lib/src/common/config.rs +++ b/crates/contextforge-data-plane-lib/src/common/config.rs @@ -146,18 +146,11 @@ impl From<&CliConfig> for UpstreamTransportConfig { pub struct JwksConfig { pub url: url::Url, pub ca_cert_path: Option, - pub issuer: String, - pub audiences: Vec, } impl From<&CliConfig> for JwksConfig { fn from(value: &CliConfig) -> Self { let CliConfig { jwks_url, jwks_ca_cert_path, .. } = value.clone(); - Self { - url: jwks_url, - ca_cert_path: jwks_ca_cert_path, - issuer: value.jwt_issuer.clone(), - audiences: value.jwt_audiences.clone(), - } + Self { url: jwks_url, ca_cert_path: jwks_ca_cert_path } } } @@ -361,10 +354,6 @@ mod tests { fn observability_config_is_derived_from_cli_config() { let args = vec![ "contextforge-data-plane", - "--jwt-issuer", - "mcpgateway", - "--jwt-audiences", - "mcpgateway-api", "--jwks-url", "http://127.0.0.1:8080/", "--redis-address", @@ -411,12 +400,7 @@ mod tests { impl Default for super::JwksConfig { fn default() -> Self { - Self { - url: "http://127.0.0.1:8080/".parse().expect("should work"), - ca_cert_path: None, - issuer: "mcpgateway".to_owned(), - audiences: vec!["mcpgateway-api".to_owned()], - } + Self { url: "http://127.0.0.1:8080/".parse().expect("should work"), ca_cert_path: None } } } diff --git a/crates/contextforge-data-plane-lib/src/tools.rs b/crates/contextforge-data-plane-lib/src/tools.rs index e82482ab..e1cb5177 100644 --- a/crates/contextforge-data-plane-lib/src/tools.rs +++ b/crates/contextforge-data-plane-lib/src/tools.rs @@ -96,9 +96,9 @@ pub async fn get_token( std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).expect("Time went backwards").as_secs(); let map = json!( { - "iss": state.config.jwks_config.issuer, + "iss": "contexforge-dataplane", "sub": user_id.clone(), - "aud": state.config.jwks_config.audiences, + "aud": "contexforge-dataplane-audience", "role": "user", "woUserId": user_id.clone(), "exp": now + Duration::from_hours(1).as_secs(), diff --git a/crates/contextforge-data-plane-lib/tests/gateway/harness/mod.rs b/crates/contextforge-data-plane-lib/tests/gateway/harness/mod.rs index b0e9f238..2af3d501 100644 --- a/crates/contextforge-data-plane-lib/tests/gateway/harness/mod.rs +++ b/crates/contextforge-data-plane-lib/tests/gateway/harness/mod.rs @@ -48,12 +48,7 @@ pub(crate) use user_config_store::MemoryUserConfigStore; pub fn create_default_config() -> Config { Config { address: None, - jwks_config: JwksConfig { - url: "http://127.0.0.1:8080/".parse().expect("should work"), - ca_cert_path: None, - issuer: "mcpgateway".to_owned(), - audiences: vec!["mcpgateway-api".to_owned()], - }, + jwks_config: JwksConfig { url: "http://127.0.0.1:8080/".parse().expect("should work"), ca_cert_path: None }, mcp_standard_header_max_count: 10, mcp_standard_header_max_value_bytes: 4096, diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index c497f36f..e8d95a31 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -76,8 +76,6 @@ services: - CONTEXTFORGE_DATA_PLANE_UPSTREAM_CONNECTION_MODE=plain-text-or-tls # Set the HTTPS JWKS endpoint for the token issuer in the host environment. - CONTEXTFORGE_DATA_PLANE_JWKS_URL - - CONTEXTFORGE_DATA_PLANE_JWT_ISSUER - - CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES - RUST_LOG=debug sysctls: @@ -138,8 +136,6 @@ services: - CONTEXTFORGE_DATA_PLANE_UPSTREAM_CONNECTION_MODE=plain-text-or-tls # Set the HTTPS JWKS endpoint for the token issuer in the host environment. - CONTEXTFORGE_DATA_PLANE_JWKS_URL - - CONTEXTFORGE_DATA_PLANE_JWT_ISSUER - - CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES - RUST_LOG=debug sysctls: @@ -177,8 +173,6 @@ services: - CONTEXTFORGE_DATA_PLANE_UPSTREAM_CONNECTION_MODE=plain-text-or-tls # Set the HTTPS JWKS endpoint for the token issuer in the host environment. - CONTEXTFORGE_DATA_PLANE_JWKS_URL - - CONTEXTFORGE_DATA_PLANE_JWT_ISSUER - - CONTEXTFORGE_DATA_PLANE_JWT_AUDIENCES - RUST_LOG=debug sysctls: From 2413a87c1691f5678f5d9b8038867e0649e6a363 Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Thu, 24 Sep 2026 16:55:59 +0100 Subject: [PATCH 14/19] test: remove added coverage for unchanged JWKS behaviour Signed-off-by: Pratik Gandhi --- .../src/authorization/jwks/jwks.rs | 60 ------------------- 1 file changed, 60 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs index ea092d5c..5482d25b 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/jwks/jwks.rs @@ -169,63 +169,3 @@ pub(super) fn validated_json_web_keys( } Ok(keys) } - -#[cfg(test)] -mod tests { - use super::*; - use crate::JwksConfig; - use jsonwebtoken::{EncodingKey, encode}; - use serde_json::json; - - fn signing_key() -> EncodingKey { - EncodingKey::from_rsa_pem(include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/../../assets/jwt.key"))).unwrap() - } - - fn public_key() -> Jwk { - let mut jwk = Jwk::from_encoding_key(&signing_key(), Algorithm::RS256).unwrap(); - jwk.common.key_id = Some("test".into()); - jwk - } - - #[tokio::test] - async fn verifies_signed_tokens_with_optional_claims() { - let config = JwksConfig::default(); - let verifier = Jwks::builder().client(reqwest::Client::new()).url(config.url).build(); - verifier - .cache - .write() - .await - .insert(JWKS_CACHE_KEY.to_owned(), vec![VerificationKey::from_jwk(public_key()).unwrap().unwrap()]); - let mut header = Header::new(Algorithm::RS256); - header.kid = Some("test".into()); - let claims = - json!({"iss":"mcpgateway", "aud":"mcpgateway-api", "exp":jsonwebtoken::get_current_timestamp()+3600}); - let signed = |claims: &Value, header: &Header| encode(header, claims, &signing_key()).unwrap(); - assert!(verifier.validate(&signed(&claims, &header), &header).await.is_some()); - let mut without_expiry = claims.clone(); - without_expiry.as_object_mut().unwrap().remove("exp"); - assert!(verifier.validate(&signed(&without_expiry, &header), &header).await.is_some()); - for name in ["iss", "aud"] { - let mut modified = claims.clone(); - modified[name] = json!("other"); - assert!(verifier.validate(&signed(&modified, &header), &header).await.is_some()); - } - let mut invalid = Vec::new(); - for (name, value) in [("exp", json!(1)), ("nbf", json!(9_999_999_999_u64))] { - let mut modified = claims.clone(); - modified[name] = value; - invalid.push(modified); - } - for name in ["iss", "aud"] { - let mut modified = claims.clone(); - modified.as_object_mut().unwrap().remove(name); - assert!(verifier.validate(&signed(&modified, &header), &header).await.is_some()); - } - for claims in invalid { - assert!(verifier.validate(&signed(&claims, &header), &header).await.is_none()); - } - let mut another_rsa_algorithm = header.clone(); - another_rsa_algorithm.alg = Algorithm::RS384; - assert!(verifier.validate(&signed(&claims, &another_rsa_algorithm), &another_rsa_algorithm).await.is_some()); - } -} From 1beddfbb8aa8c5b90888dfa4aa3d329807cdabdf Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Fri, 25 Sep 2026 10:18:49 +0100 Subject: [PATCH 15/19] test: restore original test JWKS key metadata Signed-off-by: Pratik Gandhi --- crates/contextforge-data-plane-lib/src/tools.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/tools.rs b/crates/contextforge-data-plane-lib/src/tools.rs index e1cb5177..4c56d14c 100644 --- a/crates/contextforge-data-plane-lib/src/tools.rs +++ b/crates/contextforge-data-plane-lib/src/tools.rs @@ -38,12 +38,11 @@ async fn get_jwks(State(state): State) -> Respons .into_response(); }; - let Ok(mut key) = Jwk::from_encoding_key(&key, jsonwebtoken::Algorithm::RS256) else { + let Ok(key) = Jwk::from_encoding_key(&key, jsonwebtoken::Algorithm::RS256) else { return (StatusCode::INTERNAL_SERVER_ERROR, "Can't find the encoding key or the format is wrong") .into_response(); }; - key.common.key_id = Some("test".to_owned()); let keys = vec![key]; (StatusCode::OK, [(CACHE_CONTROL, JWKS_CACHE_CONTROL)], Json(JwkSet { keys })).into_response() } From 8494aa32b898a7c255b07819a087e7eb36991d76 Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Fri, 25 Sep 2026 10:29:04 +0100 Subject: [PATCH 16/19] refactor: remove unnecessary auth type exports Signed-off-by: Pratik Gandhi --- crates/contextforge-data-plane-lib/src/lib.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/lib.rs b/crates/contextforge-data-plane-lib/src/lib.rs index 22a92232..c5748ab1 100644 --- a/crates/contextforge-data-plane-lib/src/lib.rs +++ b/crates/contextforge-data-plane-lib/src/lib.rs @@ -42,7 +42,7 @@ pub type Error = Box; pub type Result = std::result::Result; use crate::{ - authorization::{CelPrincipalExtractor, DefaultPrincipalExtractor}, + authorization::{CelPrincipalExtractor, DefaultPrincipalExtractor, Permission}, config_stores::RedisStore, layers::{ claims_id::claims_layer, @@ -52,10 +52,7 @@ use crate::{ virtual_host_id::virtual_host_id_layer, }, }; -pub use authorization::{ - AuthenticationError, AuthorizationClaims, AuthorizationService, AuthorizedPrincipal, Permission, - get_authorization_service, -}; +pub use authorization::{AuthorizationClaims, AuthorizationService, get_authorization_service}; pub use layers::permission::require_permission; #[derive(Clone)] From 69911b0a8f639519fba59ad9cbec8b4788a1e115 Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Fri, 25 Sep 2026 10:36:24 +0100 Subject: [PATCH 17/19] refactor: restore original auth debug and error responses Signed-off-by: Pratik Gandhi --- .../contextforge-data-plane-lib/src/authorization/mod.rs | 8 +------- crates/contextforge-data-plane-lib/src/errors.rs | 4 +--- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/authorization/mod.rs b/crates/contextforge-data-plane-lib/src/authorization/mod.rs index eb42269f..98ad898f 100644 --- a/crates/contextforge-data-plane-lib/src/authorization/mod.rs +++ b/crates/contextforge-data-plane-lib/src/authorization/mod.rs @@ -92,7 +92,7 @@ pub struct Idp { iss: String, } -#[derive(Clone, Serialize, Deserialize, PartialEq, TypedBuilder)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, TypedBuilder)] #[serde(rename_all = "camelCase")] pub struct AuthorizationClaims { value: serde_json::Value, @@ -104,12 +104,6 @@ impl AuthorizationClaims { } } -impl std::fmt::Debug for AuthorizationClaims { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("AuthorizationClaims").finish_non_exhaustive() - } -} - impl From for AuthorizationClaims { fn from(value: serde_json::Value) -> Self { Self { value } diff --git a/crates/contextforge-data-plane-lib/src/errors.rs b/crates/contextforge-data-plane-lib/src/errors.rs index 2f28be73..2c3ec1c2 100644 --- a/crates/contextforge-data-plane-lib/src/errors.rs +++ b/crates/contextforge-data-plane-lib/src/errors.rs @@ -2,9 +2,7 @@ use axum::response::Response; use http::{StatusCode, header}; pub(crate) fn unauthorized_response(message: &str) -> Response { - let mut response = custom_error(StatusCode::UNAUTHORIZED, message); - response.headers_mut().insert(header::WWW_AUTHENTICATE, http::HeaderValue::from_static("Bearer")); - response + custom_error(StatusCode::UNAUTHORIZED, message) } pub(crate) fn bad_request(message: &str) -> Response { From 91f3c7702174cbe26f96326c5e603cdaabe3a83d Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Fri, 25 Sep 2026 14:28:13 +0100 Subject: [PATCH 18/19] test: simplify downstream auth integration coverage Signed-off-by: Pratik Gandhi --- .../tests/gateway/downstream_auth.rs | 70 ++++++------------- 1 file changed, 22 insertions(+), 48 deletions(-) diff --git a/crates/contextforge-data-plane-lib/tests/gateway/downstream_auth.rs b/crates/contextforge-data-plane-lib/tests/gateway/downstream_auth.rs index 58904dc6..ee4cdf9a 100644 --- a/crates/contextforge-data-plane-lib/tests/gateway/downstream_auth.rs +++ b/crates/contextforge-data-plane-lib/tests/gateway/downstream_auth.rs @@ -28,36 +28,25 @@ fn key() -> EncodingKey { } fn claims() -> Value { let now = SystemTime::now().duration_since(UNIX_EPOCH).expect("valid authentication test fixture").as_secs(); - json!({"iss":"mcpgateway", "aud":"mcpgateway-api", "sub":"user", "woTenantId":"tenant", "tenant_id":"tenant", "role":"user", "exp":now+3600, "nbf":now-60}) + json!({"iss":"mcpgateway", "aud":"mcpgateway-api", "sub":"user", "woTenantId":"tenant", "role":"user", "exp":now+3600, "nbf":now-60}) } -fn token(claims: &Value, kid: Option<&str>, algorithm: Algorithm) -> String { - let mut header = Header::new(algorithm); - header.kid = kid.map(str::to_owned); +fn token(claims: &Value) -> String { + let mut header = Header::new(Algorithm::RS256); + header.kid = Some("test".into()); encode(&header, claims, &key()).expect("valid authentication test fixture") } -#[derive(Clone, Default)] +#[derive(Clone)] struct CountingStore { reads: Arc, - virtual_host: Option, + virtual_host: VirtualHost, } #[async_trait] impl ConfigStore for CountingStore { async fn get_config<'a>(&self, user: &'a User) -> Result { assert_eq!(user.key(), "user"); self.reads.fetch_add(1, Ordering::SeqCst); - Ok(UserConfig { - virtual_hosts: HashMap::from([( - "test".into(), - self.virtual_host.clone().unwrap_or_else(|| VirtualHost { - backends: HashMap::new(), - tools: HashMap::new(), - resources: HashMap::new(), - resource_templates: HashMap::new(), - prompts: HashMap::new(), - }), - )]), - }) + Ok(UserConfig { virtual_hosts: HashMap::from([("test".into(), self.virtual_host.clone())]) }) } async fn set_config<'a>(&self, _: &'a User, _: &'a UserConfig) -> Result<(), ConfigStoreError> { unreachable!() @@ -78,34 +67,28 @@ async fn gateway(config: Config, store: CountingStore) -> Router { .expect("valid authentication test fixture") } -fn request(token: Option<&str>, method: &str) -> Request { - let mut request = Request::builder() +fn request(token: &str) -> Request { + let request = Request::builder() .method("POST") .uri("/contextforge-rs/servers/test/mcp") .header("host", "localhost") .header("content-type", "application/json") .header("accept", "application/json, text/event-stream") .header("MCP-Protocol-Version", "2026-07-28") - .header("Mcp-Method", method); - if method == "tools/call" { - request = request.header("Mcp-Name", "sum"); - } - if let Some(token) = token { - request = request.header("authorization", format!("Bearer {token}")); - } - let mut params = json!({ + .header("Mcp-Method", "tools/call") + .header("Mcp-Name", "sum") + .header("authorization", format!("Bearer {token}")); + let params = json!({ + "name": "sum", + "arguments": {"a": 2, "b": 3}, "_meta": { "io.modelcontextprotocol/protocolVersion": "2026-07-28", "io.modelcontextprotocol/clientInfo": {"name": "auth-test", "version": "1"}, "io.modelcontextprotocol/clientCapabilities": {} } }); - if method == "tools/call" { - params["name"] = "sum".into(); - params["arguments"] = json!({"a": 2, "b": 3}); - } request - .body(Body::from(json!({"jsonrpc": "2.0", "id": 1, "method": method, "params": params}).to_string())) + .body(Body::from(json!({"jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": params}).to_string())) .expect("valid authentication test fixture") } @@ -151,7 +134,8 @@ async fn permission_denial_prevents_backend_calls_after_a_successful_request() { .await .expect("valid authentication test fixture"); let store = CountingStore { - virtual_host: Some(VirtualHost { + reads: Arc::new(AtomicUsize::new(0)), + virtual_host: VirtualHost { backends: HashMap::from([( "counter".into(), BackendMCPGateway { @@ -172,8 +156,7 @@ async fn permission_denial_prevents_backend_calls_after_a_successful_request() { resources: HashMap::new(), resource_templates: HashMap::new(), prompts: HashMap::new(), - }), - ..Default::default() + }, }; let server = key_server().await; let mut config = create_default_config(); @@ -181,16 +164,11 @@ async fn permission_denial_prevents_backend_calls_after_a_successful_request() { Some(contextforge_data_plane_lib::UpstreamConnectionMode::PlainTextOrTls); config.jwks_config.url = server.url("/jwks").parse().expect("valid authentication test fixture"); let app = gateway(config, store.clone()).await; - for (role, expected) in - [("user", StatusCode::OK), ("unknown", StatusCode::FORBIDDEN), ("expired", StatusCode::UNAUTHORIZED)] - { + for (role, expected) in [("user", StatusCode::OK), ("unknown", StatusCode::FORBIDDEN)] { let mut c = claims(); c["role"] = role.into(); - if role == "expired" { - c["exp"] = json!(1); - } - let token = token(&c, Some("test"), Algorithm::RS256); - let request = request(Some(&token), "tools/call"); + let token = token(&c); + let request = request(&token); let response = app.clone().oneshot(request).await.expect("valid authentication test fixture"); let status = response.status(); let body = axum::body::to_bytes(response.into_body(), 65_536).await.expect("valid authentication test fixture"); @@ -204,10 +182,6 @@ async fn permission_denial_prevents_backend_calls_after_a_successful_request() { assert_eq!(store.reads.load(Ordering::SeqCst), 1); assert_eq!(hits.load(Ordering::SeqCst), 1, "denied request must never reach backend"); } - let response = app.oneshot(request(None, "tools/call")).await.expect("authentication fixture"); - assert_eq!(response.status(), StatusCode::UNAUTHORIZED); - assert_eq!(store.reads.load(Ordering::SeqCst), 1); - assert_eq!(hits.load(Ordering::SeqCst), 1); server.shutdown().await.expect("valid authentication test fixture"); backend.shutdown().await.expect("valid authentication test fixture"); } From d91f38b994ad143276d2da72579d4e7085aa8d9c Mon Sep 17 00:00:00 2001 From: Pratik Gandhi Date: Fri, 25 Sep 2026 15:11:02 +0100 Subject: [PATCH 19/19] docs: simplify permission layer comments Signed-off-by: Pratik Gandhi --- crates/contextforge-data-plane-lib/src/layers/permission.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/contextforge-data-plane-lib/src/layers/permission.rs b/crates/contextforge-data-plane-lib/src/layers/permission.rs index c14f2435..2e3902bd 100644 --- a/crates/contextforge-data-plane-lib/src/layers/permission.rs +++ b/crates/contextforge-data-plane-lib/src/layers/permission.rs @@ -9,8 +9,6 @@ use axum::{ }; use http::StatusCode; -/// Reusable API-level guard. Install after verified principal extraction and before -/// configuration or backend access. Use `Admin` for future management routes. pub async fn require_permission(State(permission): State, request: Request, next: Next) -> Response { if request.extensions().get::().is_none() { return unauthorized_response("Missing verified identity"); @@ -25,7 +23,7 @@ pub async fn require_permission(State(permission): State, request: R } } -/// Provisional PoC role mapping, using only the original verified token claims. +/// Test role mapping; awaiting confirmation from WxO. fn has_permission(claims: &AuthorizationClaims, permission: Permission) -> Result { let allows = |role: &str| match role { "admin" => true,