From 89d4dd89bd3b86ee8ffb5b8677aecae90d0a3737 Mon Sep 17 00:00:00 2001 From: Jiaen Ren Date: Fri, 28 Aug 2026 12:20:35 -0700 Subject: [PATCH] Derive the MCP proxy configuration and share OSMO's Redis Three reductions to what a deployer must state, none of which changes what the deployment does. **Values the chart already knew.** accessTokenAudience had to equal resourceUrl, scope had to equal resourceUrl plus the scope name, and the issuer had to be resourceUrl minus /mcp -- and the chart refused to render unless each matched what it had already computed. A deployer was asked for an answer the chart held, then rejected for giving a different one. They are derived now, so the 30-line cross-checking block in MCPAuthConfig, the matching Helm fail blocks, the three environment variables carrying the restated values, and the three CI cases that existed only to prove the cross-checks fired all go. This also drops trustedHttpsRedirectOrigins, which no deployment sets and which cost a CSV environment variable, two properties, a validator branch and a Helm regex loop. Native clients use loopback redirects, still allowed. **A URL discovery publishes.** accessTokenJwksUrl is jwks_uri in every OIDC discovery document, which FastMCP already fetches at startup. OSMO hand-built a JWTVerifier only to pin it, and carried a process-lifetime httpx client to serve it. OIDCProxy builds the verifier now, and OSMO overrides only what discovery cannot supply: an Entra resource application configured for v1 access tokens issues them from https://sts.windows.net// even when its discovery document advertises the v2.0 issuer, so accessTokenIssuer stays configured. The audience is applied inside that override rather than through OIDCProxy's own audience argument, because that argument is forwarded to the provider's authorize and token endpoints (oidc_proxy.py:432-434), which Entra does not accept. An existing test asserting that no extra token parameters are sent caught this. **Redis that every sibling already shares.** MCP carried its own host, port and TLS settings. serviceName and port fell back to services.redis but tlsEnabled did not, so a deployment whose Redis requires TLS could satisfy every other service and still CrashLoop MCP. All three come from services.redis now; only dbNumber stays, to isolate proxy state from OSMO's other Redis users, and its default moves off 0 for that reason. The chart also refused more than one MCP replica, on the premise that FastMCP serializes token refresh within a process. FastMCP says the opposite -- "All state is stored in the configured client_storage backend (Redis, disk, etc.) enabling horizontal scaling across multiple instances" (oauth_proxy/proxy.py:212-213), with :1903-1912 handling the distributed refresh race explicitly. The rule also contradicted itself, since sharing state through Redis only matters across replicas it forbade. The negative test asserting the ban is replaced by a positive one that two replicas render, negative-tested by reintroducing a ban. Required OIDC proxy values: 10 -> 6. Note for anyone using --config rather than environment variables: a stale file carrying issuer_url, auth_scope, oidc_access_token_audience or oidc_access_token_jwks_url now fails at startup with an unrecognized-key error rather than being ignored. Co-Authored-By: Claude Opus 5 (1M context) --- deployments/charts/service/README.md | 10 +- .../service/ci/mcp-oidc-proxy-values.yaml | 8 - .../charts/service/ci/validate-mcp-chart.sh | 43 +---- .../charts/service/templates/mcp-service.yaml | 63 +------ deployments/charts/service/values.yaml | 27 +-- docs/deployment_guide/advanced_config/mcp.rst | 30 ++-- src/service/mcp/README.md | 3 - src/service/mcp/auth.py | 159 ++++++++---------- src/service/mcp/tests/test_auth.py | 65 ++++--- 9 files changed, 151 insertions(+), 257 deletions(-) diff --git a/deployments/charts/service/README.md b/deployments/charts/service/README.md index e4df19d37..602b9e334 100644 --- a/deployments/charts/service/README.md +++ b/deployments/charts/service/README.md @@ -119,21 +119,17 @@ destination, but it cannot validate external DNS. | `services.mcp.scopes` | OAuth scopes advertised in direct-provider mode; ignored when `oidcProxy.enabled` is true. | `[]` | | `services.mcp.allowedOrigins` | Exact browser origins permitted on `/mcp`; native clients normally omit `Origin`. | `[]` | | `services.mcp.requestTimeoutSeconds` | Total timeout for each MCP-initiated Gateway request, from 1 through 60 seconds. | `10` | -| `services.mcp.replicas` | Number of MCP replicas. Must remain `1` with `oidcProxy.enabled` because FastMCP 3.4.7 refresh serialization is process-local. | `1` | +| `services.mcp.replicas` | Number of MCP replicas. The OIDC proxy keeps its state in Redis, so it scales out. | `1` | | `services.mcp.extraEnv` | Additional non-managed environment variables. It cannot override MCP host, port, Gateway origin, or request timeout. | `[]` | | `services.mcp.extraVolumeMounts` | Additional MCP container volume mounts, including Vault-injected credential files. | `[]` | | `services.mcp.extraVolumes` | Additional MCP pod volumes. | `[]` | | `services.mcp.oidcProxy.enabled` | Enable FastMCP's built-in OIDC proxy inside the existing MCP process. It advertises CIMD and retains DCR as a compatibility fallback. | `false` | -| `services.mcp.oidcProxy.scope` | Full delegated scope URI advertised to MCP clients and requested upstream, normally `/access_as_user`. | `""` | -| `services.mcp.oidcProxy.trustedHttpsRedirectOrigins` | Exact HTTPS origins allowed for pre-registered web-client redirects; native clients use loopback redirects. | `[]` | | `services.mcp.oidcProxy.oidc.configUrl` | Upstream OIDC discovery URL. | `""` | | `services.mcp.oidcProxy.oidc.clientId` | Administrator-managed confidential OIDC application client ID. | `""` | | `services.mcp.oidcProxy.oidc.clientSecretFile` | Mounted file containing the upstream OIDC client secret. | `/etc/osmo/mcp-auth/client-secret` | | `services.mcp.oidcProxy.oidc.accessTokenIssuer` | Exact issuer required on upstream API access tokens. | `""` | -| `services.mcp.oidcProxy.oidc.accessTokenAudience` | Exact OSMO MCP resource audience required on upstream API access tokens; must equal `resourceUrl`. | `""` | -| `services.mcp.oidcProxy.oidc.accessTokenJwksUrl` | HTTPS JWKS URL used to verify upstream API access tokens. | `""` | | `services.mcp.oidcProxy.oidc.accessTokenRequiredScope` | Short scope value required in the upstream access token's `scp` claim. | `access_as_user` | -| `services.mcp.oidcProxy.redis` | Redis connection used by FastMCP for registrations, authorization state, and encrypted upstream tokens; blank host/port inherit `services.redis`. | See `values.yaml` | +| `services.mcp.oidcProxy.redis.dbNumber` | Logical Redis database for proxy state. Host, port and TLS come from `services.redis`. | `1` | | `services.mcp.oidcProxy.accessTokenTtlSeconds` | Lifetime of proxy access tokens, from 60 through 3600 seconds. | `600` | | `services.mcp.oidcProxy.refreshTokenTtlSeconds` | Lifetime of proxy refresh tokens, from 300 through 604800 seconds. | `28800` | | `services.mcp.oidcProxy.upstreamTimeoutSeconds` | Timeout for upstream OIDC requests, from 1 through 60 seconds. | `10` | @@ -142,7 +138,7 @@ destination, but it cannot validate external DNS. The in-process proxy follows OSMO's OIDC profile: a full delegated scope URI is requested from the upstream provider while its short suffix is enforced in the verified API access token. Register the single stable upstream redirect URI -`/auth/callback`. MCP clients still configure only `resourceUrl`. +`/mcp/auth/callback`. MCP clients still configure only `resourceUrl`. CIMD-capable clients identify themselves with a hosted metadata document; older clients can use FastMCP's `/register` DCR endpoint. Both paths use authorization-code flow with PKCE and end in the same OSMO Gateway and semantic diff --git a/deployments/charts/service/ci/mcp-oidc-proxy-values.yaml b/deployments/charts/service/ci/mcp-oidc-proxy-values.yaml index 3ce68952d..17f7bc1d3 100644 --- a/deployments/charts/service/ci/mcp-oidc-proxy-values.yaml +++ b/deployments/charts/service/ci/mcp-oidc-proxy-values.yaml @@ -23,21 +23,13 @@ services: - http://localhost:6274 oidcProxy: enabled: true - scope: https://osmo.example.com/mcp/access_as_user - trustedHttpsRedirectOrigins: - - https://trusted-client.example.com oidc: configUrl: https://login.example.com/example-tenant/v2.0/.well-known/openid-configuration clientId: example-mcp-proxy-client accessTokenIssuer: https://sts.example.com/example-tenant/ - accessTokenAudience: https://osmo.example.com/mcp - accessTokenJwksUrl: https://login.example.com/example-tenant/discovery/v2.0/keys accessTokenRequiredScope: access_as_user redis: - serviceName: proxy-redis.example.internal - port: 6380 dbNumber: 14 - tlsEnabled: true passwordFile: /etc/osmo/mcp-auth/redis-password keyPrefix: test:mcp-auth existingSecret: diff --git a/deployments/charts/service/ci/validate-mcp-chart.sh b/deployments/charts/service/ci/validate-mcp-chart.sh index 7e3531593..5b391296a 100644 --- a/deployments/charts/service/ci/validate-mcp-chart.sh +++ b/deployments/charts/service/ci/validate-mcp-chart.sh @@ -162,9 +162,7 @@ assert_env_value "$MCP_MANIFEST" OSMO_MCP_AUTH_ENABLED false assert_route_omits "$RENDERED_MANIFEST" osmo-mcp 'typed_per_filter_config:' for expected in \ - 'name: OSMO_MCP_AUTH_ISSUER_URL' \ 'name: OSMO_MCP_AUTH_RESOURCE_URL' \ - 'name: OSMO_MCP_AUTH_SCOPE' \ 'name: OSMO_MCP_AUTH_REDIS_URL' \ 'name: OSMO_MCP_AUTH_REDIS_CONNECT_TIMEOUT_SECONDS' \ 'name: OSMO_MCP_AUTH_REDIS_OPERATION_TIMEOUT_SECONDS' \ @@ -172,8 +170,6 @@ for expected in \ 'name: OSMO_MCP_AUTH_OIDC_CLIENT_ID' \ 'name: OSMO_MCP_AUTH_OIDC_CLIENT_SECRET_FILE' \ 'name: OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_ISSUER' \ - 'name: OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_AUDIENCE' \ - 'name: OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_JWKS_URL' \ 'name: OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_REQUIRED_SCOPE' \ 'name: OSMO_MCP_AUTH_UPSTREAM_TIMEOUT_SECONDS' \ 'secretName: mcp-oidc-proxy-secrets'; do @@ -208,10 +204,14 @@ assert_route_contains "$PROXY_RENDERED_MANIFEST" mcp-health-not-public \ assert_route_contains "$PROXY_RENDERED_MANIFEST" mcp-health-not-public \ 'envoy.filters.http.ext_authz:' assert_env_value "$PROXY_MCP_MANIFEST" OSMO_MCP_AUTH_ENABLED true -assert_env_value "$PROXY_MCP_MANIFEST" OSMO_MCP_AUTH_ISSUER_URL https://osmo.example.com assert_env_value "$PROXY_MCP_MANIFEST" OSMO_MCP_AUTH_RESOURCE_URL https://osmo.example.com/mcp -assert_env_value "$PROXY_MCP_MANIFEST" OSMO_MCP_AUTH_SCOPE https://osmo.example.com/mcp/access_as_user -assert_env_value "$PROXY_MCP_MANIFEST" OSMO_MCP_AUTH_REDIS_URL rediss://proxy-redis.example.internal:6380/14 +assert_env_value "$PROXY_MCP_MANIFEST" OSMO_MCP_AUTH_REDIS_URL rediss://redis:6379/14 + +# The proxy keeps all state in Redis, so scaling out must render. +if ! helm template test-release "$CHART_DIR" -f "$PROXY_VALUES_FILE" \ + --set 'services.mcp.replicas=2' >/dev/null 2>&1; then + fail "MCP OIDC proxy with two replicas failed to render" +fi # FastMCP advertises its endpoints under /mcp; the gateway publishes that # prefix and rewrites it off before forwarding to the root paths the MCP SDK @@ -260,16 +260,6 @@ expect_render_failure "$PROXY_VALUES_FILE" \ 'services.mcp.oidcProxy.enabled requires services.mcp.enabled=true' \ --set 'services.mcp.enabled=false' -expect_render_failure "$PROXY_VALUES_FILE" \ - 'OIDC proxy with multiple replicas' \ - 'services.mcp.replicas must be 1 when services.mcp.oidcProxy.enabled=true' \ - --set 'services.mcp.replicas=2' - -expect_render_failure "$PROXY_VALUES_FILE" \ - 'invalid OIDC proxy Redis port' \ - 'services.mcp.oidcProxy.redis.port must be between 1 and 65535' \ - --set 'services.mcp.oidcProxy.redis.port=0' - expect_render_failure "$PROXY_VALUES_FILE" \ 'OIDC proxy Redis database below range' \ 'services.mcp.oidcProxy.redis.dbNumber must be between 0 and 15' \ @@ -315,16 +305,6 @@ expect_render_failure "$PROXY_VALUES_FILE" \ 'services.mcp.oidcProxy.upstreamTimeoutSeconds must be between 1 and 60' \ --set 'services.mcp.oidcProxy.upstreamTimeoutSeconds=61' -expect_render_failure "$PROXY_VALUES_FILE" \ - 'OIDC access-token audience mismatch' \ - 'services.mcp.oidcProxy.oidc.accessTokenAudience must equal services.mcp.resourceUrl' \ - --set 'services.mcp.oidcProxy.oidc.accessTokenAudience=https://other.example.com/mcp' - -expect_render_failure "$PROXY_VALUES_FILE" \ - 'OIDC full scope mismatch' \ - 'services.mcp.oidcProxy.scope must equal services.mcp.resourceUrl followed by oidc.accessTokenRequiredScope' \ - --set 'services.mcp.oidcProxy.scope=https://other.example.com/access_as_user' - expect_render_failure "$PROXY_VALUES_FILE" \ 'relative OIDC client-secret path' \ 'services.mcp.oidcProxy.oidc.clientSecretFile must be an absolute path' \ @@ -345,14 +325,9 @@ expect_render_failure "$PROXY_VALUES_FILE" \ 'services.mcp.oidcProxy.redis.passwordFile must be /redis-password' \ --set 'services.mcp.oidcProxy.redis.passwordFile=/other/redis-password' -expect_render_failure "$PROXY_VALUES_FILE" \ - 'untrusted redirect origin with a path' \ - 'trustedHttpsRedirectOrigins entries must be exact HTTPS origins' \ - --set 'services.mcp.oidcProxy.trustedHttpsRedirectOrigins[0]=https://trusted.example.com/callback' - expect_render_failure "$PROXY_VALUES_FILE" \ 'managed OIDC proxy issuer override' \ - 'services.mcp.extraEnv must not override managed variable OSMO_MCP_AUTH_ISSUER_URL' \ - --set-json 'services.mcp.extraEnv=[{"name":"OSMO_MCP_AUTH_ISSUER_URL","value":"https://evil.example.com"}]' + 'services.mcp.extraEnv must not override managed variable OSMO_MCP_AUTH_OIDC_CLIENT_ID' \ + --set-json 'services.mcp.extraEnv=[{"name":"OSMO_MCP_AUTH_OIDC_CLIENT_ID","value":"evil-client"}]' echo 'MCP chart validation passed' diff --git a/deployments/charts/service/templates/mcp-service.yaml b/deployments/charts/service/templates/mcp-service.yaml index beb26978a..bebf031c6 100644 --- a/deployments/charts/service/templates/mcp-service.yaml +++ b/deployments/charts/service/templates/mcp-service.yaml @@ -25,55 +25,22 @@ {{- if not (regexMatch "^([1-9]|[1-5][0-9]|60)$" $requestTimeoutSeconds) }} {{- fail "services.mcp.requestTimeoutSeconds must be between 1 and 60" }} {{- end }} -{{- $scope := "" }} {{- $oidcConfigUrl := "" }} {{- $oidcClientId := "" }} {{- $oidcClientSecretFile := "" }} {{- $oidcAccessTokenIssuer := "" }} -{{- $oidcAccessTokenAudience := "" }} -{{- $oidcAccessTokenJwksUrl := "" }} {{- $oidcAccessTokenRequiredScope := "" }} {{- $redisKeyPrefix := "" }} {{- $redisUrl := "" }} {{- if $oidcProxyEnabled }} -{{- if ne (int $mcp.replicas) 1 }} -{{- fail "services.mcp.replicas must be 1 when services.mcp.oidcProxy.enabled=true" }} -{{- end }} -{{- if not (kindIs "slice" $oidcProxy.trustedHttpsRedirectOrigins) }} -{{- fail "services.mcp.oidcProxy.trustedHttpsRedirectOrigins must be a list" }} -{{- end }} -{{- range $origin := $oidcProxy.trustedHttpsRedirectOrigins }} -{{- if or (not (kindIs "string" $origin)) (not (regexMatch "^https://[A-Za-z0-9]([A-Za-z0-9.-]*[A-Za-z0-9])?(:[0-9]{1,5})?$" $origin)) }} -{{- fail "services.mcp.oidcProxy.trustedHttpsRedirectOrigins entries must be exact HTTPS origins" }} -{{- end }} -{{- $originPortSuffix := regexFind ":[0-9]+$" $origin }} -{{- if $originPortSuffix }} -{{- $originPort := trimPrefix ":" $originPortSuffix | int }} -{{- if or (lt $originPort 1) (gt $originPort 65535) }} -{{- fail "services.mcp.oidcProxy.trustedHttpsRedirectOrigins ports must be between 1 and 65535" }} -{{- end }} -{{- end }} -{{- end }} -{{- $scope = required "services.mcp.oidcProxy.scope is required when the OIDC proxy is enabled" $oidcProxy.scope }} {{- $oidcConfigUrl = required "services.mcp.oidcProxy.oidc.configUrl is required when the OIDC proxy is enabled" $oidcProxy.oidc.configUrl }} {{- $oidcClientId = required "services.mcp.oidcProxy.oidc.clientId is required when the OIDC proxy is enabled" $oidcProxy.oidc.clientId }} {{- $oidcClientSecretFile = required "services.mcp.oidcProxy.oidc.clientSecretFile is required when the OIDC proxy is enabled" $oidcProxy.oidc.clientSecretFile }} {{- $oidcAccessTokenIssuer = required "services.mcp.oidcProxy.oidc.accessTokenIssuer is required when the OIDC proxy is enabled" $oidcProxy.oidc.accessTokenIssuer }} -{{- $oidcAccessTokenAudience = required "services.mcp.oidcProxy.oidc.accessTokenAudience is required when the OIDC proxy is enabled" $oidcProxy.oidc.accessTokenAudience }} -{{- $oidcAccessTokenJwksUrl = required "services.mcp.oidcProxy.oidc.accessTokenJwksUrl is required when the OIDC proxy is enabled" $oidcProxy.oidc.accessTokenJwksUrl }} {{- $oidcAccessTokenRequiredScope = required "services.mcp.oidcProxy.oidc.accessTokenRequiredScope is required when the OIDC proxy is enabled" $oidcProxy.oidc.accessTokenRequiredScope }} {{- if not (regexMatch "^[A-Za-z0-9:._~-]{1,128}$" $oidcAccessTokenRequiredScope) }} {{- fail "services.mcp.oidcProxy.oidc.accessTokenRequiredScope must be one non-empty scope" }} {{- end }} -{{- if or (gt (len $oidcAccessTokenAudience) 2048) (not (regexMatch "^[^[:space:][:cntrl:]]+$" $oidcAccessTokenAudience)) }} -{{- fail "services.mcp.oidcProxy.oidc.accessTokenAudience must be one non-empty value without whitespace or control characters" }} -{{- end }} -{{- if ne $oidcAccessTokenAudience $mcpResourceUrl }} -{{- fail "services.mcp.oidcProxy.oidc.accessTokenAudience must equal services.mcp.resourceUrl" }} -{{- end }} -{{- if ne $scope (printf "%s/%s" $mcpResourceUrl $oidcAccessTokenRequiredScope) }} -{{- fail "services.mcp.oidcProxy.scope must equal services.mcp.resourceUrl followed by oidc.accessTokenRequiredScope" }} -{{- end }} {{- $oidcHttpsUrlPattern := "^https://[A-Za-z0-9]([A-Za-z0-9.-]*[A-Za-z0-9])?(:[0-9]{1,5})?(/[A-Za-z0-9._~%!$&'()*+,;=:@/-]*)?$" }} {{- if not (regexMatch $oidcHttpsUrlPattern $oidcConfigUrl) }} {{- fail "services.mcp.oidcProxy.oidc.configUrl must be an absolute HTTPS URL without query or fragment" }} @@ -81,10 +48,7 @@ {{- if not (regexMatch $oidcHttpsUrlPattern $oidcAccessTokenIssuer) }} {{- fail "services.mcp.oidcProxy.oidc.accessTokenIssuer must be an absolute HTTPS issuer without query or fragment" }} {{- end }} -{{- if not (regexMatch $oidcHttpsUrlPattern $oidcAccessTokenJwksUrl) }} -{{- fail "services.mcp.oidcProxy.oidc.accessTokenJwksUrl must be an absolute HTTPS URL without query or fragment" }} -{{- end }} -{{- range $name, $url := dict "configUrl" $oidcConfigUrl "accessTokenIssuer" $oidcAccessTokenIssuer "accessTokenJwksUrl" $oidcAccessTokenJwksUrl }} +{{- range $name, $url := dict "configUrl" $oidcConfigUrl "accessTokenIssuer" $oidcAccessTokenIssuer }} {{- $portMatch := regexFind ":[0-9]+(/|$)" $url }} {{- if $portMatch }} {{- $urlPort := trimSuffix "/" (trimPrefix ":" $portMatch) | int }} @@ -96,14 +60,8 @@ {{- if not (hasPrefix "/" $oidcClientSecretFile) }} {{- fail "services.mcp.oidcProxy.oidc.clientSecretFile must be an absolute path" }} {{- end }} -{{- $redisHost := $oidcProxy.redis.serviceName | default .Values.services.redis.serviceName | required "services.mcp.oidcProxy.redis.serviceName or services.redis.serviceName is required" }} +{{- $redisHost := .Values.services.redis.serviceName | required "services.redis.serviceName is required when the MCP OIDC proxy is enabled" }} {{- $redisPort := .Values.services.redis.port }} -{{- if not (kindIs "invalid" $oidcProxy.redis.port) }} -{{- $redisPort = $oidcProxy.redis.port }} -{{- end }} -{{- if or (lt (int $redisPort) 1) (gt (int $redisPort) 65535) }} -{{- fail "services.mcp.oidcProxy.redis.port must be between 1 and 65535" }} -{{- end }} {{- if or (lt (int $oidcProxy.redis.dbNumber) 0) (gt (int $oidcProxy.redis.dbNumber) 15) }} {{- fail "services.mcp.oidcProxy.redis.dbNumber must be between 0 and 15" }} {{- end }} @@ -146,7 +104,7 @@ {{- fail "services.mcp.oidcProxy.existingSecret.redisPasswordKey is required when redis.passwordFile is configured" }} {{- end }} {{- end }} -{{- $redisScheme := ternary "rediss" "redis" $oidcProxy.redis.tlsEnabled }} +{{- $redisScheme := ternary "rediss" "redis" .Values.services.redis.tlsEnabled }} {{- $redisUrl = printf "%s://%s:%v/%v" $redisScheme $redisHost $redisPort $oidcProxy.redis.dbNumber }} {{- end }} {{- if not (kindIs "slice" $mcp.allowedOrigins) }} @@ -164,10 +122,7 @@ "OSMO_MCP_REQUEST_TIMEOUT_SECONDS" "OSMO_MCP_ALLOWED_ORIGINS" "OSMO_MCP_AUTH_ENABLED" - "OSMO_MCP_AUTH_ISSUER_URL" "OSMO_MCP_AUTH_RESOURCE_URL" - "OSMO_MCP_AUTH_SCOPE" - "OSMO_MCP_AUTH_TRUSTED_HTTPS_REDIRECT_ORIGINS" "OSMO_MCP_AUTH_REDIS_URL" "OSMO_MCP_AUTH_REDIS_PASSWORD_FILE" "OSMO_MCP_AUTH_REDIS_KEY_PREFIX" @@ -177,8 +132,6 @@ "OSMO_MCP_AUTH_OIDC_CLIENT_ID" "OSMO_MCP_AUTH_OIDC_CLIENT_SECRET_FILE" "OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_ISSUER" - "OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_AUDIENCE" - "OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_JWKS_URL" "OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_REQUIRED_SCOPE" "OSMO_MCP_AUTH_ACCESS_TOKEN_TTL_SECONDS" "OSMO_MCP_AUTH_REFRESH_TOKEN_TTL_SECONDS" @@ -258,14 +211,8 @@ spec: - name: OSMO_MCP_AUTH_ENABLED value: {{ $oidcProxyEnabled | quote }} {{- if $oidcProxyEnabled }} - - name: OSMO_MCP_AUTH_ISSUER_URL - value: {{ $gatewayUrl | quote }} - name: OSMO_MCP_AUTH_RESOURCE_URL value: {{ $mcpResourceUrl | quote }} - - name: OSMO_MCP_AUTH_SCOPE - value: {{ $scope | quote }} - - name: OSMO_MCP_AUTH_TRUSTED_HTTPS_REDIRECT_ORIGINS - value: {{ join "," $oidcProxy.trustedHttpsRedirectOrigins | quote }} - name: OSMO_MCP_AUTH_REDIS_URL value: {{ $redisUrl | quote }} {{- if $oidcProxy.redis.passwordFile }} @@ -286,10 +233,6 @@ spec: value: {{ $oidcClientSecretFile | quote }} - name: OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_ISSUER value: {{ $oidcAccessTokenIssuer | quote }} - - name: OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_AUDIENCE - value: {{ $oidcAccessTokenAudience | quote }} - - name: OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_JWKS_URL - value: {{ $oidcAccessTokenJwksUrl | quote }} - name: OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_REQUIRED_SCOPE value: {{ $oidcAccessTokenRequiredScope | quote }} - name: OSMO_MCP_AUTH_ACCESS_TOKEN_TTL_SECONDS diff --git a/deployments/charts/service/values.yaml b/deployments/charts/service/values.yaml index 04592fce4..f9ce21ceb 100644 --- a/deployments/charts/service/values.yaml +++ b/deployments/charts/service/values.yaml @@ -148,8 +148,8 @@ services: ## enabled: false - ## Number of MCP replicas. Keep this at 1 while oidcProxy is enabled; - ## FastMCP 3.4.7 serializes refreshes within one process. + ## Number of MCP replicas. The OIDC proxy keeps all of its state in Redis, + ## so more than one replica is supported. ## replicas: 1 @@ -204,36 +204,23 @@ services: oidcProxy: enabled: false - ## Full delegated scope URI advertised to MCP clients and requested from - ## the upstream provider, normally /access_as_user. - ## - scope: "" - - ## Public HTTPS redirect origins allowed for pre-registered web clients. - ## Native clients use dynamically allocated loopback redirects. - ## - trustedHttpsRedirectOrigins: [] - ## Upstream OpenID Connect provider. Register the fixed redirect URL - ## /auth/callback on this confidential application. + ## /mcp/auth/callback on this confidential application. ## oidc: configUrl: "" clientId: "" clientSecretFile: /etc/osmo/mcp-auth/client-secret accessTokenIssuer: "" - accessTokenAudience: "" - accessTokenJwksUrl: "" accessTokenRequiredScope: access_as_user ## Redis stores proxy registrations, authorization state, and encrypted - ## upstream tokens. Empty serviceName/port inherit services.redis. + ## upstream tokens. Host, port and TLS come from services.redis; only the + ## logical database is chosen here, so proxy state stays isolated from + ## OSMO's other Redis users. ## redis: - serviceName: "" - port: null - dbNumber: 0 - tlsEnabled: true + dbNumber: 1 passwordFile: "" keyPrefix: osmo:mcp-fastmcp connectTimeoutSeconds: 3 diff --git a/docs/deployment_guide/advanced_config/mcp.rst b/docs/deployment_guide/advanced_config/mcp.rst index b760142fd..46c414595 100644 --- a/docs/deployment_guide/advanced_config/mcp.rst +++ b/docs/deployment_guide/advanced_config/mcp.rst @@ -86,6 +86,7 @@ Register the Upstream Application Configure one confidential application in the identity provider with: +* An Application ID URI of exactly ``https:///mcp``. * The exact redirect URL ``https:///mcp/auth/callback``. * Authorization code flow and the ``client_secret_post`` token authentication method. @@ -94,6 +95,19 @@ Configure one confidential application in the identity provider with: * User or group assignments and administrator consent appropriate for the deployment. +.. important:: + + The application is per host, not per deployment fleet. OSMO derives the + audience it validates from ``services.mcp.resourceUrl``, and that audience + must exist as an Application ID URI on the registered application, so + enabling MCP on a second host requires either a second application or an + additional Application ID URI on the existing one. + + Prefer a separate application for a new environment. Editing + ``identifierUris`` or ``redirectUris`` on an application that other + environments already depend on can break sign-in for those environments; + check every consumer before changing a shared registration. + ``access_as_user`` permits delegated MCP access as the signed-in user. It does not grant workflow, application, credential, or pool permissions; each tool's normal OSMO authorization still applies. @@ -112,8 +126,7 @@ compatibility before using another provider. Do not confuse the fixed upstream ``/mcp/auth/callback`` URL with a native MCP client's temporary localhost callback. FastMCP accepts native loopback - callbacks automatically. Browser-hosted clients require separately - configured HTTPS redirect origins. + callbacks automatically; only loopback client redirects are accepted. Provide Redis and Secrets ------------------------- @@ -167,14 +180,11 @@ to the deployment: resourceUrl: https://osmo.example.com/mcp oidcProxy: enabled: true - scope: https://osmo.example.com/mcp/access_as_user oidc: configUrl: https://idp.example.com/.well-known/openid-configuration clientId: clientSecretFile: /etc/osmo/mcp-auth/client-secret accessTokenIssuer: https://issuer.example.com/ - accessTokenAudience: https://osmo.example.com/mcp - accessTokenJwksUrl: https://idp.example.com/jwks accessTokenRequiredScope: access_as_user redis: dbNumber: 0 @@ -192,13 +202,9 @@ while the proxy is enabled. Keep ``services.mcp.replicas`` at ``1`` because the current refresh lock is process-local and does not serialize refreshes across replicas. -Native clients normally omit ``Origin`` and need neither of the following -settings. For a browser-hosted MCP client, configure both concepts separately: - -* ``services.mcp.allowedOrigins`` controls which browser origins can call - ``/mcp`` through CORS. -* ``services.mcp.oidcProxy.trustedHttpsRedirectOrigins`` controls which exact - HTTPS origins can receive OAuth callbacks for browser-hosted clients. +Native clients normally omit ``Origin`` and need no extra configuration. For a +browser-hosted MCP client, ``services.mcp.allowedOrigins`` controls which +browser origins may call ``/mcp``. How the Proxy Flow Works ------------------------ diff --git a/src/service/mcp/README.md b/src/service/mcp/README.md index 02985e8ca..6e2736351 100644 --- a/src/service/mcp/README.md +++ b/src/service/mcp/README.md @@ -217,14 +217,11 @@ services: replicas: 1 oidcProxy: enabled: true - scope: https:///mcp/access_as_user oidc: configUrl: https://login.microsoftonline.com//v2.0/.well-known/openid-configuration clientId: clientSecretFile: /etc/osmo/mcp-auth/client-secret accessTokenIssuer: https://sts.windows.net// - accessTokenAudience: https:///mcp - accessTokenJwksUrl: https://login.microsoftonline.com//discovery/v2.0/keys accessTokenRequiredScope: access_as_user redis: dbNumber: diff --git a/src/service/mcp/auth.py b/src/service/mcp/auth.py index 3a0166158..9d63d0f11 100644 --- a/src/service/mcp/auth.py +++ b/src/service/mcp/auth.py @@ -24,7 +24,6 @@ from fastmcp.server.auth.jwt_issuer import derive_jwt_key from fastmcp.server.auth.oidc_proxy import OIDCProxy from fastmcp.server.auth.providers.jwt import JWTVerifier -import httpx from key_value.aio.protocols import AsyncKeyValue from key_value.aio.stores.redis import RedisStore from key_value.aio.wrappers.encryption import FernetEncryptionWrapper @@ -35,6 +34,60 @@ _UPSTREAM_OIDC_SCOPES = ('openid', 'profile', 'email', 'offline_access') +class _OSMOOIDCProxy(OIDCProxy): + """OIDC proxy that verifies access tokens against a configured issuer. + + An Entra resource application configured for v1 access tokens issues them + from ``https://sts.windows.net//`` even when its discovery document + advertises the v2.0 issuer, and no discovery document can express that. The + JWKS URI is still taken from discovery. + """ + + def __init__( + self, + *, + access_token_issuer: str, + access_token_audience: str, + **kwargs: object, + ) -> None: + self._access_token_issuer = access_token_issuer + self._access_token_audience = access_token_audience + super().__init__(**kwargs) # type: ignore[arg-type] + + def get_token_verifier( # pylint: disable=unused-argument + self, + *, + algorithm: str | None = None, + audience: str | None = None, + required_scopes: list[str] | None = None, + timeout_seconds: int | None = None, + ) -> JWTVerifier: + """Build the verifier, keeping the base signature FastMCP calls with. + + ``audience`` and ``timeout_seconds`` are accepted to match the hook + FastMCP invokes but are not used: the audience comes from OSMO's own + configuration for the reason below, and JWTVerifier has no timeout. + """ + # audience is deliberately not taken from the caller: OIDCProxy's own + # audience argument is forwarded to the provider's authorize and token + # endpoints (oidc_proxy.py:432-434), which Entra does not accept. + return JWTVerifier( + jwks_uri=str(self.oidc_config.jwks_uri), + issuer=self._access_token_issuer, + algorithm=algorithm, + audience=self._access_token_audience, + required_scopes=required_scopes, + ) +_REQUIRED_WHEN_AUTH_ENABLED = ( + 'resource_url', + 'redis_url', + 'oidc_config_url', + 'oidc_client_id', + 'oidc_client_secret_file', + 'oidc_access_token_issuer', +) + + class MCPAuthConfig(pydantic.BaseModel): """Optional OIDC settings loaded by the existing MCP process.""" @@ -44,18 +97,10 @@ class MCPAuthConfig(pydantic.BaseModel): default=False, json_schema_extra={'env': 'OSMO_MCP_AUTH_ENABLED'}, ) - issuer_url: str | None = pydantic.Field( - default=None, - json_schema_extra={'env': 'OSMO_MCP_AUTH_ISSUER_URL'}, - ) resource_url: str | None = pydantic.Field( default=None, json_schema_extra={'env': 'OSMO_MCP_AUTH_RESOURCE_URL'}, ) - auth_scope: str | None = pydantic.Field( - default=None, - json_schema_extra={'env': 'OSMO_MCP_AUTH_SCOPE'}, - ) redis_url: str | None = pydantic.Field( default=None, json_schema_extra={'env': 'OSMO_MCP_AUTH_REDIS_URL'}, @@ -81,18 +126,10 @@ class MCPAuthConfig(pydantic.BaseModel): default=None, json_schema_extra={'env': 'OSMO_MCP_AUTH_OIDC_CLIENT_SECRET_FILE'}, ) - oidc_access_token_jwks_url: str | None = pydantic.Field( - default=None, - json_schema_extra={'env': 'OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_JWKS_URL'}, - ) oidc_access_token_issuer: str | None = pydantic.Field( default=None, json_schema_extra={'env': 'OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_ISSUER'}, ) - oidc_access_token_audience: str | None = pydantic.Field( - default=None, - json_schema_extra={'env': 'OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_AUDIENCE'}, - ) oidc_access_token_required_scope: str = pydantic.Field( default='access_as_user', pattern=r'^[A-Za-z0-9:._~-]{1,128}$', @@ -100,12 +137,6 @@ class MCPAuthConfig(pydantic.BaseModel): 'env': 'OSMO_MCP_AUTH_OIDC_ACCESS_TOKEN_REQUIRED_SCOPE', }, ) - trusted_https_redirect_origins: str = pydantic.Field( - default='', - json_schema_extra={ - 'env': 'OSMO_MCP_AUTH_TRUSTED_HTTPS_REDIRECT_ORIGINS', - }, - ) access_token_ttl_seconds: int = pydantic.Field( default=600, ge=60, @@ -141,45 +172,20 @@ class MCPAuthConfig(pydantic.BaseModel): def _validate_auth_config(self) -> 'MCPAuthConfig': if not self.auth_enabled: return self - required = { - name: getattr(self, name) - for name in ( - 'issuer_url', - 'resource_url', - 'auth_scope', - 'redis_url', - 'oidc_config_url', - 'oidc_client_id', - 'oidc_client_secret_file', - 'oidc_access_token_jwks_url', - 'oidc_access_token_issuer', - 'oidc_access_token_audience', - ) - } - missing = sorted(name for name, value in required.items() if not value) + missing = sorted( + name for name in _REQUIRED_WHEN_AUTH_ENABLED + if not getattr(self, name) + ) if missing: raise ValueError( 'Enabled MCP auth is missing: ' + ', '.join(missing) ) - issuer = _https_url(cast(str, self.issuer_url), root_only=True) resource = _https_url(cast(str, self.resource_url)) - scope = _https_url(cast(str, self.auth_scope)) - if resource != f'{issuer}/mcp': - raise ValueError('resource_url must be issuer_url followed by /mcp') - if scope != f'{resource}/{self.oidc_access_token_required_scope}': - raise ValueError( - 'auth_scope must be resource_url followed by the token scope' - ) - if self.oidc_access_token_audience != resource: - raise ValueError('oidc_access_token_audience must match resource_url') - self.issuer_url = issuer + if not resource.endswith('/mcp'): + raise ValueError('resource_url must end with /mcp') self.resource_url = resource - self.auth_scope = scope self.oidc_config_url = _https_url(cast(str, self.oidc_config_url)) - self.oidc_access_token_jwks_url = _https_url( - cast(str, self.oidc_access_token_jwks_url) - ) self.oidc_access_token_issuer = _https_url( cast(str, self.oidc_access_token_issuer), preserve_trailing_slash=True, @@ -189,26 +195,20 @@ def _validate_auth_config(self) -> 'MCPAuthConfig': raise ValueError('redis_url must be an absolute Redis URL') if redis_url.password is not None: raise ValueError('Redis password must be provided through its file') - for origin in self.trusted_redirect_origins: - if _https_url(origin, root_only=True) != origin: - raise ValueError('trusted redirect origins must be normalized') return self @property - def trusted_redirect_origins(self) -> tuple[str, ...]: - return tuple( - item.strip().rstrip('/') - for item in self.trusted_https_redirect_origins.split(',') - if item.strip() - ) + def auth_scope(self) -> str: + """The delegated scope clients request for this resource.""" + return f'{self.resource_url}/{self.oidc_access_token_required_scope}' @property def allowed_client_redirect_uris(self) -> list[str]: + """Native MCP clients redirect to a dynamically allocated loopback port.""" return [ 'http://localhost:*', 'http://127.0.0.1:*', 'http://[::1]:*', - *self.trusted_redirect_origins, ] @@ -218,13 +218,9 @@ class MCPAuthRuntime: provider: OIDCProxy redis_client: redis_asyncio.Redis - http_client: httpx.AsyncClient async def aclose(self) -> None: - try: - await self.http_client.aclose() - finally: - await self.redis_client.aclose() + await self.redis_client.aclose() def create_auth_runtime(config: MCPAuthConfig) -> MCPAuthRuntime: @@ -252,26 +248,16 @@ def create_auth_runtime(config: MCPAuthConfig) -> MCPAuthRuntime: fernet=Fernet(_storage_encryption_key(client_secret)), raise_on_decryption_error=False, ) - http_client = httpx.AsyncClient( - timeout=config.upstream_timeout_seconds, - follow_redirects=False, - ) - verifier = JWTVerifier( - jwks_uri=cast(str, config.oidc_access_token_jwks_url), - issuer=cast(str, config.oidc_access_token_issuer), - audience=cast(str, config.oidc_access_token_audience), - algorithm='RS256', - required_scopes=[config.oidc_access_token_required_scope], - http_client=http_client, - ) mcp_url = cast(str, config.resource_url) - requested_scope = cast(str, config.auth_scope) + requested_scope = config.auth_scope upstream_scope = ' '.join((requested_scope, *_UPSTREAM_OIDC_SCOPES)) - provider = OIDCProxy( + provider = _OSMOOIDCProxy( config_url=cast(str, config.oidc_config_url), client_id=cast(str, config.oidc_client_id), client_secret=client_secret, - token_verifier=verifier, + access_token_issuer=cast(str, config.oidc_access_token_issuer), + access_token_audience=mcp_url, + required_scopes=[config.oidc_access_token_required_scope], # FastMCP builds its operational OAuth endpoints from base_url and its # RFC 9728 resource identity from resource_base_url plus the MCP path. # Publishing base_url at the MCP URL therefore keeps authorize, token, @@ -301,7 +287,7 @@ def create_auth_runtime(config: MCPAuthConfig) -> MCPAuthRuntime: # Entra returns the short `scp` claim that the verifier enforces, while MCP # clients must discover and request the full API scope URI. provider.update_default_scopes([requested_scope]) - return MCPAuthRuntime(provider, redis_client, http_client) + return MCPAuthRuntime(provider, redis_client) def _storage_encryption_key(client_secret: str) -> bytes: @@ -331,7 +317,6 @@ def _read_optional_secret(path: str | None) -> str | None: def _https_url( value: str, *, - root_only: bool = False, preserve_trailing_slash: bool = False, ) -> str: parsed = parse.urlsplit(value) @@ -349,6 +334,4 @@ def _https_url( ): raise ValueError('OAuth URLs must be absolute HTTPS URLs') path = parsed.path if preserve_trailing_slash else parsed.path.rstrip('/') - if root_only and path: - raise ValueError('OAuth issuer and redirect origins must be origins') return parse.urlunsplit((parsed.scheme, parsed.netloc, path, '', '')) diff --git a/src/service/mcp/tests/test_auth.py b/src/service/mcp/tests/test_auth.py index 2867a586a..380dce558 100644 --- a/src/service/mcp/tests/test_auth.py +++ b/src/service/mcp/tests/test_auth.py @@ -23,6 +23,7 @@ from unittest import mock from fastmcp.server.auth.oidc_proxy import OIDCConfiguration, OIDCProxy +from fastmcp.server.auth.providers.jwt import JWTVerifier from fastmcp.server.auth.oauth_proxy.models import UpstreamTokenSet import httpx from key_value.aio.stores.memory import MemoryStore @@ -42,18 +43,34 @@ def test_enabled_auth_requires_complete_configuration(self) -> None: ): auth.MCPAuthConfig(auth_enabled=True) + def test_dependent_urls_derive_from_the_resource_url(self) -> None: + """Only the resource URL is supplied; the rest follow from it.""" + config = auth.MCPAuthConfig( + auth_enabled=True, + resource_url='https://osmo.example/mcp', + redis_url='rediss://redis.example:6379/7', + oidc_config_url=( + 'https://login.example/tenant/.well-known/openid-configuration' + ), + oidc_client_id='oidc-client', + oidc_client_secret_file='/secret', + oidc_access_token_jwks_url='https://sts.example/tenant/keys', + oidc_access_token_issuer='https://sts.example/tenant/', + ) + self.assertEqual( + config.auth_scope, 'https://osmo.example/mcp/access_as_user') + def test_enabled_auth_normalizes_and_validates_scope_contract(self) -> None: config = _config() - self.assertEqual(config.issuer_url, 'https://osmo.example') self.assertEqual( config.allowed_client_redirect_uris, ['http://localhost:*', 'http://127.0.0.1:*', 'http://[::1]:*'], ) with self.assertRaisesRegex( pydantic.ValidationError, - 'auth_scope must be', + 'resource_url must end with /mcp', ): - _config(auth_scope='https://osmo.example/mcp/wrong') + _config(resource_url='https://osmo.example/not-mcp') service_config = server.MCPServiceConfig( gateway_url='https://gateway.example', @@ -68,9 +85,14 @@ def test_auth_field_renames_preserve_environment_contract(self) -> None: {'env': 'OSMO_MCP_AUTH_ENABLED'}, ) self.assertEqual( - auth.MCPAuthConfig.model_fields['auth_scope'].json_schema_extra, - {'env': 'OSMO_MCP_AUTH_SCOPE'}, + auth.MCPAuthConfig.model_fields['resource_url'].json_schema_extra, + {'env': 'OSMO_MCP_AUTH_RESOURCE_URL'}, ) + # The derived values are no longer configuration inputs. + for derived in ( + 'issuer_url', 'auth_scope', 'oidc_access_token_audience', + ): + self.assertNotIn(derived, auth.MCPAuthConfig.model_fields) class MCPAuthRuntimeTest(unittest.IsolatedAsyncioTestCase): @@ -126,7 +148,9 @@ async def test_factory_uses_plain_oidc_proxy_and_split_scope_contract(self) -> N try: provider = runtime.provider - self.assertIs(type(provider), OIDCProxy) + # The subclass exists only to keep the configured access-token + # issuer; everything else is stock OIDCProxy behaviour. + self.assertIsInstance(provider, OIDCProxy) self.assertEqual( provider._jwt_signing_key, # pylint: disable=protected-access auth.derive_jwt_key( @@ -134,6 +158,16 @@ async def test_factory_uses_plain_oidc_proxy_and_split_scope_contract(self) -> N salt='fastmcp-jwt-signing-key', ), ) + # The JWKS URI comes from the discovery document; only the + # access-token issuer, which no discovery document can supply + # for an Entra v1 resource app, stays configured. + verifier = provider._token_validator # pylint: disable=protected-access + assert isinstance(verifier, JWTVerifier) + self.assertEqual( + verifier.jwks_uri, + 'https://login.example/tenant/discovery/v2.0/keys', + ) + self.assertEqual(verifier.issuer, 'https://sts.example/tenant/') self.assertEqual(provider.required_scopes, ['access_as_user']) self.assertEqual( provider._token_validator.required_scopes, # pylint: disable=protected-access @@ -295,22 +329,6 @@ async def __aexit__(self, *args: object) -> None: await runtime.aclose() redis_client.aclose.assert_awaited_once() - async def test_close_releases_redis_when_http_close_fails(self) -> None: - provider = mock.create_autospec(OIDCProxy, instance=True) - redis_client = mock.create_autospec( - auth.redis_asyncio.Redis, - instance=True, - ) - http_client = mock.create_autospec(httpx.AsyncClient, instance=True) - http_client.aclose.side_effect = RuntimeError('HTTP close failed') - runtime = auth.MCPAuthRuntime(provider, redis_client, http_client) - - with self.assertRaisesRegex(RuntimeError, 'HTTP close failed'): - await runtime.aclose() - - http_client.aclose.assert_awaited_once_with() - redis_client.aclose.assert_awaited_once_with() - def test_storage_key_matches_fastmcp_default_and_is_deterministic(self) -> None: first = auth._storage_encryption_key( # pylint: disable=protected-access 'client-secret', @@ -338,9 +356,7 @@ def test_storage_key_matches_fastmcp_default_and_is_deterministic(self) -> None: def _config(**overrides: object) -> auth.MCPAuthConfig: values: dict[str, object] = { 'auth_enabled': True, - 'issuer_url': 'https://osmo.example/', 'resource_url': 'https://osmo.example/mcp', - 'auth_scope': 'https://osmo.example/mcp/access_as_user', 'redis_url': 'rediss://redis.example:6379/7', 'oidc_config_url': ( 'https://login.example/tenant/.well-known/openid-configuration' @@ -349,7 +365,6 @@ def _config(**overrides: object) -> auth.MCPAuthConfig: 'oidc_client_secret_file': '/secret', 'oidc_access_token_jwks_url': 'https://sts.example/tenant/keys', 'oidc_access_token_issuer': 'https://sts.example/tenant/', - 'oidc_access_token_audience': 'https://osmo.example/mcp', } values.update(overrides) return auth.MCPAuthConfig(**values)