[lharrison13]

✨ Feature / Enhancement PR

🔗 Epic / Issue

Link to the epic or parent issue:
Closes #7236

---

🚀 Summary (1-2 sentences)

Platform admins can now create and edit token-exchange gateways in the Admin UI. A token-exchange gateway can also have an optional discovery client-credentials credential. Registration, "🔧 Fetch Tools", refresh, and health checks use it, and tool calls keep the per-user RFC 8693 exchange.

---

📏 Reviewability

• This PR has one clear purpose
• The linked issue is not labeled triage
• Unrelated bugs or improvements are tracked in separate issues/PRs
• Tests are included with the code they validate
• If AI-assisted, I understand and can explain the generated changes

---

🧪 Checks

List exact commands, screenshots, videos, logs, validation steps, or explain why evidence is not feasible.

• make lint passes
• make test passes
• CHANGELOG updated (if user-facing)

---

📓 Notes (optional)

User Story 1: token-exchange in the Admin UI

• The add and edit forms (mcpgateway/templates/admin.html) offer token-exchange as a grant type. The option renders only when is_admin is true.
• A token-exchange field group shows target_audience, subject_token_type, requested_token_type, subject_token_source, and token_endpoint_auth_method. private_key_jwt stays API-only.
• _assemble_oauth_config_from_fields() in mcpgateway/admin.py reads the new fields through _TOKEN_EXCHANGE_FORM_FIELDS.
• _merge_token_exchange_edit() lays an edit submission over the stored oauth_config:
  • Keys the form does not render (for example private_key) keep their stored values.
  • A rendered field that is submitted blank clears its key.
  • The masked secret placeholder keeps the stored secret.
  • A description-only save leaves oauth_config unchanged.
• _require_token_exchange_client_secret() rejects a UI submission without a usable client_secret, unless the auth method is private_key_jwt or none. This check applies to the Admin UI only. The REST API does not change.
• A non-admin who opens a token-exchange gateway sees a read-only notice (lockTokenExchangeEditForNonAdmin()). The server-side platform-admin gate stays authoritative and returns 403.
• UI submissions go through the same token_url SSRF validation as the REST API.

User Story 2: separate discovery credential

• New optional oauth_config.discovery object. discovery.grant_type accepts client_credentials only. discovery is rejected on all other grants.
• build_discovery_oauth_config() derives the effective client-credentials config:
  • It inherits token_url, client_id, client_secret, token_endpoint_auth_method, and the signing-key keys from the parent.
  • discovery can override token_url, client_id, client_secret, and token_endpoint_auth_method.
  • The result never carries token-exchange keys, so it cannot drive a per-user exchange.
• discovery.token_url goes through the same egress validation as the parent token_url.
• GatewayService._resolve_discovery_header() gets the discovery token. These paths call it: _initialize_gateway(), health checks, reactivation, the lifecycle worker, update, and manual refresh (POST /oauth/fetch-tools/{id} and POST /gateways/{id}/tools/refresh).
• Invocation paths (tool_service, resource_service) never call the discovery resolver. test_invocation_services_never_reference_discovery_resolver pins this.
• Discovery tokens live in the new GatewayServiceTokenCache (mcpgateway/services/gateway_service_token_cache.py), separate from TokenExchangeCache:
  • The key is purpose:sha256(gateway_id):fingerprint.
  • The fingerprint covers the stored secret ciphertext, so a secret rotation misses the old entry on every worker.
  • A gateway update, deactivation, or delete invalidates the gateway's entries.
• TokenExchangeCache and GatewayServiceTokenCache now share a _BearerTokenStore base. TokenExchangeCache keys do not change.
• audit_token_exchange() takes a purpose field (invocation or discovery) and allows user_email=None for service tokens. Each discovery request emits an audit event with a correlation_id.
• OAuthManager.get_client_credentials_token_response() returns the full token response, so the cache can use expires_in.

Behavior without a discovery credential

Behavior stays as it is today. Registration persists an empty catalog, and health checks skip the gateway. _discovery_result_is_unusable() now protects token-exchange gateways during update, reactivation, and the lifecycle worker. An empty result from a skipped discovery no longer deletes stored tools.

Behavior with a discovery credential

• A discovery token failure at registration raises GatewayConnectionError.
• A discovery token failure during a health check marks the gateway unhealthy.
• A discovery failure does not affect the per-user exchange on tool calls.

Breaking or behavior changes

• Resource reads on token-exchange gateways now fail closed. resource_service raises ResourceError when no user JWT is present or the exchange fails. Before, the request went upstream without an Authorization header.
• Token-exchange audit events carry a new purpose key. The change is additive.
• No database migration. All new settings live in the existing oauth_config JSON. Nested discovery.client_secret values are encrypted at rest and masked in reads by the existing recursive handlers.

Deny-path tests

• Non-admin add, edit, and enabling discovery all return 403. A team admin editing a team gateway also gets 403 (tests/unit/mcpgateway/test_admin.py, test_main.py).
• The token-exchange option is hidden for non-admins (test_admin_token_exchange_template.py).
• A disallowed discovery.token_url, an unsupported discovery grant, and an unknown auth method are rejected (test_schemas_auth_validation.py, test_token_exchange_discovery.py).
• A tool call without a user JWT is rejected and sends no upstream request.
• Resource reads without a user JWT, or with a failed exchange, fail closed.
• Live gateway: tests/live_gateway/mcp/test_token_exchange_discovery_credential.py. Mock upstream: tests/integration/test_token_exchange_mock_upstream.py. Playwright: tests/playwright/test_gateways.py.

Docs

• docs/docs/architecture/oauth-design.md: discovery with and without a discovery credential, invocation isolation, caching, audit.
• docs/docs/manage/identity-propagation.md: "Configuring token exchange in the Admin UI" and "Adding a discovery credential".

sequenceDiagram
    participant Admin as Admin UI / scheduler
    participant GW as ContextForge
    participant AS as Authorization server
    participant MCP as Upstream MCP server
    participant User as MCP client (user)

    Admin->>GW: Register / Fetch Tools / health check
    GW->>AS: client_credentials (discovery)
    AS-->>GW: Service token (GatewayServiceTokenCache)
    GW->>MCP: initialize + tools/list (service token)
    User->>GW: tools/call (user JWT)
    GW->>AS: RFC 8693 exchange (subject_token = user JWT)
    AS-->>GW: User-scoped token (TokenExchangeCache)
    GW->>MCP: tools/call (user-scoped token)

Loading

[msureshkumar88]

Thank you for the thorough work on #7236. The design follows the issue closely. The admin-only gate, the token_url validation, the separate discovery cache, and the invocation isolation are all sound. The PR is not ready to merge. Four items block it.

Verdict: request changes. Verified at head 6d75f0720 against base 86af292ee.

severity | path:line | problem | fix
blocking | mcpgateway/utils/token_exchange_audit.py:28, mcpgateway/services/gateway_service.py:1606,1632 | The PR does not change `token_exchange_audit.py`. `_request_discovery_token()` passes `purpose=` and `user_email=None`. `audit_token_exchange()` has no `purpose` parameter and types `user_email` as `str`. Every discovery token request raises `TypeError`. Registration, health checks, refresh, and "Fetch Tools" all fail for discovery gateways. | Add `purpose` (default `invocation`) and an optional `user_email`. Write `purpose` into the event.
blocking | tests/unit/... (22 failures) | The new tests assume the missing audit change. Failing files: `test_token_exchange_discovery.py` (11), `test_gateway_service_health_oauth.py` (2), `test_gateway_service.py` (4), `test_oauth_router.py` (1), `test_token_exchange_integration.py` (4). `make test` cannot pass. | Fix the audit module. Run `make test`. Tick the checklist items with evidence.
blocking | CHANGELOG.md | The PR body declares a breaking change: resource reads on token-exchange gateways now fail closed (`resource_service.py`). Before, the request went upstream without an `Authorization` header. `CHANGELOG.md` has no entry. | Add an entry under `[Unreleased]` > `Breaking Changes`. Name the resource-read change and the affected callers.
blocking | commits 5f6404f76, 6d75f0720 | Both commits carry a `Co-authored-by: Claude <noreply@anthropic.com>` trailer. Project rules keep AI attribution out of PRs and diffs. | @lharrison13: reword both commits, remove the `Co-authored-by` trailer, and keep `Signed-off-by`. Run `git rebase -i 86af292ee`, then `git push --force-with-lease`.
functionally-impacting | mcpgateway/admin.py:1099 | The edit form fills the secret field with the mask for token-exchange gateways. If an admin changes the grant to another type, the mask stays in the field. `_assemble_oauth_config_from_fields()` then encrypts the literal mask string and stores it as the client secret. I reproduced this: the stored secret decrypts to `*****`. The base code cleared the field, so the PR introduces this. | Keep the mask only when the stored and submitted grants are both `token-exchange`. Otherwise drop it. Add a regression test for the grant switch.
suggestion | mcpgateway/services/gateway_service.py:440-446 | `build_discovery_oauth_config()` inherits the parent `client_secret` when `discovery.token_url` or `discovery.client_id` is overridden. It also inherits the parent `scopes`. The docs state this, and an admin must set it. | Require an explicit `discovery.client_secret` when `token_url` is overridden. Consider not inheriting `scopes`.
suggestion | mcpgateway/services/gateway_service.py:5633 | A discovery token failure has no negative cache. An IdP outage retries per gateway on every health cycle. The user exchange already has `set_failure` and `is_failed`. | Add a short negative cache.
suggestion | mcpgateway/services/gateway_service_token_cache.py, token_exchange_cache.py | The PR adds a 152-line cache and moves `TokenExchangeCache` onto a shared base (about 230 changed lines). A discovery token needs one entry per gateway. | A small in-memory `{gateway_id: (token, expiry)}` with a lock avoids the refactor of the user-token cache. Add Redis only if workers must share the token.
suggestion | the PR | One PR carries User Story 1 (Admin UI grant) and User Story 2 (discovery credential). It adds 4642 lines. | Split into two PRs if the review load is high.
minor | tests/unit/mcpgateway/services/test_tool_service_coverage.py:11519 | `test_invocation_services_never_reference_discovery_resolver` checks source text with `inspect.getsource()`. A rename passes it. | Replace it with a behavioral test.
minor | mcpgateway/services/gateway_service.py | A cached discovery token is served until its TTL even after the upstream returns 401. Deactivation does not invalidate it. | Invalidate on a 401 and on deactivation.

Scope and checks:

• The change is both UI and backend. No Alembic migration is needed. oauth_config is an existing JSON column, and no model changed.
• No unrelated changes found. .secrets.baseline churn is ignored.
• Prompt gets on OAuth gateways send no Authorization header (build_gateway_auth_headers). This predates the PR and is out of scope.
• The PR checklist is unchecked. Only the DCO check has run.
• Please also run make coverage diff-cover and make test-e2e, and attach the results.

I am happy to re-review after the audit fix and the green test run.

[lharrison13]

Validation results for make coverage diff-cover and make test-e2e at head 7932bd887, base main at 85dabf0be.

make coverage diff-cover

diff-cover passes: 100% of changed lines are covered (gate is 90%).

-------------
Diff Coverage
Diff: main...HEAD, staged and unstaged changes
-------------
mcpgateway/admin.py (100%)
mcpgateway/schemas.py (100%)
mcpgateway/services/gateway_service.py (100%)
mcpgateway/services/gateway_service_token_cache.py (100%)
mcpgateway/services/oauth_manager.py (100%)
mcpgateway/services/resource_service.py (100%)
mcpgateway/services/token_exchange_cache.py (100%)
-------------
Total:   298 lines
Missing: 0 lines
Coverage: 100%
-------------

Total project coverage is 98% (79433 statements, 1544 missed).

The coverage pytest run reported 11 failed, 24639 passed, 996 skipped. All 11 failures are in tests/security/test_regex_reachability.py. This PR does not change that module or the code it tests. The same file passes in isolation (pytest tests/security/test_regex_reachability.py: 34 passed). The failures come from test interaction under pytest-xdist in the full run, not from this change.

make test-e2e

=========== 129 passed, 14 skipped, 12 warnings in 93.98s (0:01:33) ============
✅ E2E suite passed!

Stack: production Rust image (container-build RUST_MCP_BUILD=1), make testing-up RUST_MCP_MODE=, 3 gateway replicas behind nginx, fresh volumes. The run uses podman with a local compose override for podman networking and healthchecks. The override does not change gateway configuration.

The 14 skips are feature-gated suites that this stack does not enable:

5  Modern MCP E2E requires MCP_INBOUND_PROTOCOL_MODE=auto and RUST_MCP_MODE=off
2  MCP Apps E2E requires MCPGATEWAY_MCP_APPS_ENABLED=true
1  Dedicated PostgreSQL expansion run requires MCP_RESOURCE_NAME_EXPANSION=true
1  No prompts registered on gateway
1  No resources registered on gateway
1  Requires direct proxy and Redis in both gateway and test process
1  Rust MCP public transport not active at http://127.0.0.1:8080
1  Set MCP_CATALOG_REPLICA_URLS to at least two stateful gateway replicas

[msureshkumar88]

Approved at head 7932bd887. The four blocking items and the mask finding from the earlier review are fixed.

• Audit: audit_token_exchange() takes purpose and an optional user_email.
• Tests: CI passes. The affected unit test files pass.
• CHANGELOG: the fail-closed resource read has a Breaking Changes entry.
• Commits: the Co-authored-by trailers are gone. Signed-off-by stays.
• Mask: the masked secret is dropped on a grant switch, with regression tests.

One new finding. It does not block this approval.

severity | path:line | problem | fix
functionally-impacting | mcpgateway/admin.py:1241-1245 | _merge_discovery_edit() keeps the stored discovery client_secret when an admin clears the discovery client_id override. The discovery token request then pairs the parent client_id with the discovery secret and fails. | Drop the stored discovery client_secret when the client_id override is cleared or changed. Add a regression test.
suggestion | mcpgateway/services/gateway_service.py:7630 | _refresh_gateway_tools_resources_prompts guards empty results for authorization_code gateways only. Main has the same gap. | Reuse _discovery_result_is_unusable() here, or track it in a follow-up issue.

The earlier suggestions stay open as follow-ups: discovery inherits the parent scopes and client_secret, no negative cache, no invalidation on an upstream 401.