From 04d329cd49a8850ccf30c9ac9f5ac7048d23e802 Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Sun, 26 Jul 2026 20:36:44 +0700 Subject: [PATCH] fix(mcp): send a valid Keycloak token exchange request --- .../mcp/McpDownstreamOAuthConfiguration.java | 36 ++++++---- .../mcp/McpApiAuthorizationTests.java | 21 ++++++ docs/runbooks/mcp-asset-delivery.md | 21 ++++-- .../scripts/configure-keycloak-mcp.sh | 67 +++++++++++++++++++ .../scripts/test-keycloak-mcp-onboarding.sh | 61 +++++++++++++++++ .../keycloak/mcp-gateway-client.json | 6 ++ 6 files changed, 195 insertions(+), 17 deletions(-) create mode 100644 infrastructure/keycloak/mcp-gateway-client.json diff --git a/apps/mcp/src/main/java/com/orgmemory/mcp/McpDownstreamOAuthConfiguration.java b/apps/mcp/src/main/java/com/orgmemory/mcp/McpDownstreamOAuthConfiguration.java index a4f21c02..a3c3b56a 100644 --- a/apps/mcp/src/main/java/com/orgmemory/mcp/McpDownstreamOAuthConfiguration.java +++ b/apps/mcp/src/main/java/com/orgmemory/mcp/McpDownstreamOAuthConfiguration.java @@ -15,28 +15,24 @@ import org.springframework.security.oauth2.client.web.OAuth2AuthorizedClientRepository; import org.springframework.security.oauth2.core.OAuth2Token; import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; -import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; @Configuration(proxyBeanMethods = false) class McpDownstreamOAuthConfiguration { + private static final String ACCESS_TOKEN_TYPE = + "urn:ietf:params:oauth:token-type:access_token"; + @Bean OAuth2AuthorizedClientManager mcpAuthorizedClientManager( ClientRegistrationRepository registrations, McpGatewayProperties properties) { var responseClient = new RestClientTokenExchangeTokenResponseClient(); - responseClient.addParametersConverter(request -> { - var parameters = - new LinkedMultiValueMap(); - parameters.set( - OAuth2ParameterNames.SUBJECT_TOKEN_TYPE, - "urn:ietf:params:oauth:token-type:access_token"); - parameters.set( - OAuth2ParameterNames.AUDIENCE, - properties.apiAudience()); - return parameters; - }); + responseClient.setParametersCustomizer(parameters -> + customizeTokenExchangeParameters( + parameters, + properties.apiAudience())); var tokenExchange = new TokenExchangeOAuth2AuthorizedClientProvider(); @@ -55,6 +51,22 @@ OAuth2AuthorizedClientManager mcpAuthorizedClientManager( return manager; } + static void customizeTokenExchangeParameters( + MultiValueMap parameters, + String audience) { + /* + * Spring identifies Jwt as the RFC 8693 "jwt" token type by default, + * while Keycloak's standard exchange accepts its access-token JWT as + * an "access_token". set() also replaces that default value. Using an + * additional parameters converter would append a second + * subject_token_type and Keycloak rejects the ambiguous request. + */ + parameters.set( + OAuth2ParameterNames.SUBJECT_TOKEN_TYPE, + ACCESS_TOKEN_TYPE); + parameters.set(OAuth2ParameterNames.AUDIENCE, audience); + } + /** * Exchanged tokens are bound to the exact inbound subject token. Reusing * one by principal name could carry stale grants into a later MCP request. diff --git a/apps/mcp/src/test/java/com/orgmemory/mcp/McpApiAuthorizationTests.java b/apps/mcp/src/test/java/com/orgmemory/mcp/McpApiAuthorizationTests.java index 4ca075b8..fb3a9bcf 100644 --- a/apps/mcp/src/test/java/com/orgmemory/mcp/McpApiAuthorizationTests.java +++ b/apps/mcp/src/test/java/com/orgmemory/mcp/McpApiAuthorizationTests.java @@ -19,8 +19,10 @@ import org.springframework.security.oauth2.core.OAuth2AccessToken; import org.springframework.security.oauth2.core.OAuth2AuthorizationException; import org.springframework.security.oauth2.core.OAuth2Error; +import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; import org.springframework.security.oauth2.jwt.Jwt; import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken; +import org.springframework.util.LinkedMultiValueMap; class McpApiAuthorizationTests { @@ -97,6 +99,25 @@ void exchangedAuthorizedClientsAreNeverPersistedByPrincipalName() { new MockHttpServletRequest())); } + @Test + void keycloakTokenExchangeUsesOneAccessTokenTypeAndTheApiAudience() { + var parameters = new LinkedMultiValueMap(); + parameters.add( + OAuth2ParameterNames.SUBJECT_TOKEN_TYPE, + "urn:ietf:params:oauth:token-type:jwt"); + + McpDownstreamOAuthConfiguration.customizeTokenExchangeParameters( + parameters, + "orgmemory-web"); + + assertEquals( + List.of("urn:ietf:params:oauth:token-type:access_token"), + parameters.get(OAuth2ParameterNames.SUBJECT_TOKEN_TYPE)); + assertEquals( + List.of("orgmemory-web"), + parameters.get(OAuth2ParameterNames.AUDIENCE)); + } + private static McpTransportContext authenticatedContext() { Instant now = Instant.parse("2026-07-26T00:00:00Z"); var incoming = Jwt.withTokenValue("mcp-audience-token") diff --git a/docs/runbooks/mcp-asset-delivery.md b/docs/runbooks/mcp-asset-delivery.md index e0510317..d0a399a8 100644 --- a/docs/runbooks/mcp-asset-delivery.md +++ b/docs/runbooks/mcp-asset-delivery.md @@ -76,11 +76,14 @@ is rejected by MCP. The checked-in realm files are a baseline for a new Keycloak realm. Keycloak imports with `IGNORE_EXISTING`, so deploying a new image does not mutate an already-created realm. `configure-keycloak-mcp.sh` therefore updates only the -named MCP client policies and anonymous registration-policy components after -Keycloak is healthy; it preserves unrelated client policies, users, -credentials, federation, and clients. The `assets:read` scope/mappers and -confidential `orgmemory-mcp` exchange client must already exist from the realm -baseline or the PR4 migration. Verify: +named MCP client policies, the required token-exchange attribute on the +existing `orgmemory-mcp` client, and anonymous registration-policy components +after Keycloak is healthy. The client update starts from Keycloak's complete +current representation and merges only the required attribute, preserving its +secret and unrelated settings. The migration preserves unrelated client +policies, users, credentials, federation, and clients. The `assets:read` +scope/mappers and confidential `orgmemory-mcp` exchange client must already +exist from the realm baseline or the initial MCP migration. Verify: - authorization-server metadata advertises DCR but not CIMD; - a Claude-shaped DCR request uses the documented callback, creates a public @@ -90,6 +93,14 @@ baseline or the PR4 migration. Verify: - the exchanged token has only `orgmemory-web`, has `azp=orgmemory-mcp`, and retains `assets:read`. +Spring Security models a Keycloak access token as `Jwt` and therefore defaults +RFC 8693 `subject_token_type` to `urn:ietf:params:oauth:token-type:jwt`. +OrgMemory replaces that single parameter with +`urn:ietf:params:oauth:token-type:access_token`, which is the token type +Keycloak standard exchange accepts for its access tokens. Do not append a +second `subject_token_type`: Keycloak rejects an ambiguous multi-value request +with `invalid_request`. + Rotate `ORGMEMORY_MCP_OIDC_CLIENT_SECRET` independently from the web client secret. Never put either value in a document, issue, or log. diff --git a/infrastructure/deployment/scripts/configure-keycloak-mcp.sh b/infrastructure/deployment/scripts/configure-keycloak-mcp.sh index 1dde886b..d915fa6e 100755 --- a/infrastructure/deployment/scripts/configure-keycloak-mcp.sh +++ b/infrastructure/deployment/scripts/configure-keycloak-mcp.sh @@ -10,6 +10,7 @@ profiles_source="$repo_root/infrastructure/keycloak/mcp-client-profiles.json" policies_source="$repo_root/infrastructure/keycloak/mcp-client-policies.json" registration_policy_source="$repo_root/infrastructure/keycloak/mcp-dcr-registration-policy.json" basic_scope_source="$repo_root/infrastructure/keycloak/mcp-basic-client-scope.json" +gateway_client_source="$repo_root/infrastructure/keycloak/mcp-gateway-client.json" kcadm_config="/tmp/orgmemory-mcp-kcadm.config" tmp_root="${TMPDIR:-/tmp}" tmp_dir="$(mktemp -d "${tmp_root%/}/orgmemory-keycloak-mcp.XXXXXX")" @@ -58,6 +59,72 @@ keycloak_exec bash -ec \ kcadm get "realms/$realm" >/dev/null +clients_path="$tmp_dir/clients.json" +kcadm get clients -r "$realm" >"$clients_path" +gateway_client_id="$( + python3 - "$clients_path" "$gateway_client_source" <<'PY' +import json +import sys + +clients_path, desired_path = sys.argv[1:] +with open(clients_path, encoding="utf-8") as stream: + clients = json.load(stream) +with open(desired_path, encoding="utf-8") as stream: + client_id = json.load(stream)["clientId"] +matches = [client for client in clients if client.get("clientId") == client_id] +if len(matches) != 1: + raise SystemExit( + f"Expected exactly one Keycloak client {client_id!r}, found {len(matches)}" + ) +print(matches[0]["id"]) +PY +)" +gateway_client_current="$tmp_dir/gateway-client-current.json" +gateway_client_synced="$tmp_dir/gateway-client-synced.json" +kcadm get "clients/$gateway_client_id" \ + -r "$realm" >"$gateway_client_current" +python3 \ + - "$gateway_client_current" "$gateway_client_source" \ + >"$gateway_client_synced" <<'PY' +import json +import sys + +current_path, desired_path = sys.argv[1:] +with open(current_path, encoding="utf-8") as stream: + current = json.load(stream) +with open(desired_path, encoding="utf-8") as stream: + desired = json.load(stream) +if current.get("clientId") != desired["clientId"]: + raise SystemExit("Refusing to update a different Keycloak client") +current["attributes"] = { + **current.get("attributes", {}), + **desired.get("attributes", {}), +} +json.dump(current, sys.stdout) +PY +kcadm update "clients/$gateway_client_id" \ + -r "$realm" \ + -f - <"$gateway_client_synced" +kcadm get "clients/$gateway_client_id" \ + -r "$realm" >"$gateway_client_current" +python3 \ + - "$gateway_client_current" "$gateway_client_source" <<'PY' +import json +import sys + +current_path, desired_path = sys.argv[1:] +with open(current_path, encoding="utf-8") as stream: + current = json.load(stream) +with open(desired_path, encoding="utf-8") as stream: + desired = json.load(stream) +actual_attributes = current.get("attributes", {}) +for key, value in desired.get("attributes", {}).items(): + if actual_attributes.get(key) != value: + raise SystemExit( + f"Keycloak gateway client attribute verification failed for {key}" + ) +PY + client_scopes_csv="$tmp_dir/client-scopes.csv" kcadm get client-scopes \ -r "$realm" \ diff --git a/infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh b/infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh index bc0c3fed..fbe45f97 100755 --- a/infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh +++ b/infrastructure/deployment/scripts/test-keycloak-mcp-onboarding.sh @@ -97,6 +97,48 @@ MSYS_NO_PATHCONV=1 docker exec "$container" \ -r orgmemory \ -s description=drifted \ --config "$container_kcadm_config" >/dev/null +MSYS_NO_PATHCONV=1 docker exec "$container" \ + /opt/keycloak/bin/kcadm.sh get clients \ + -r orgmemory \ + --config "$container_kcadm_config" >"$tmp_dir/clients.json" +gateway_client_id="$( + python3 - "$tmp_dir/clients.json" <<'PY' +import json +import sys + +with open(sys.argv[1], encoding="utf-8") as stream: + clients = json.load(stream) +print(next( + client["id"] + for client in clients + if client["clientId"] == "orgmemory-mcp" +)) +PY +)" +MSYS_NO_PATHCONV=1 docker exec "$container" \ + /opt/keycloak/bin/kcadm.sh get "clients/$gateway_client_id" \ + -r orgmemory \ + --config "$container_kcadm_config" >"$tmp_dir/gateway-client.json" +python3 \ + - "$tmp_dir/gateway-client.json" \ + >"$tmp_dir/gateway-client-drifted.json" <<'PY' +import json +import sys + +with open(sys.argv[1], encoding="utf-8") as stream: + client = json.load(stream) +client.setdefault("attributes", {}).pop( + "standard.token.exchange.enabled", + None, +) +json.dump(client, sys.stdout) +PY +MSYS_NO_PATHCONV=1 docker exec -i "$container" \ + /opt/keycloak/bin/kcadm.sh update "clients/$gateway_client_id" \ + -r orgmemory \ + -f - \ + --config "$container_kcadm_config" \ + <"$tmp_dir/gateway-client-drifted.json" ORGMEMORY_KEYCLOAK_CONTAINER="$container" \ ORGMEMORY_KEYCLOAK_REALM=orgmemory \ "$repo_root/infrastructure/deployment/scripts/configure-keycloak-mcp.sh" @@ -116,6 +158,25 @@ with open(sys.argv[2], encoding="utf-8") as stream: expected = json.load(stream) assert scope["description"] == expected["description"], scope PY +MSYS_NO_PATHCONV=1 docker exec "$container" \ + /opt/keycloak/bin/kcadm.sh get "clients/$gateway_client_id" \ + -r orgmemory \ + --config "$container_kcadm_config" >"$tmp_dir/gateway-client.json" +python3 \ + - "$tmp_dir/gateway-client.json" \ + "$repo_root/infrastructure/keycloak/mcp-gateway-client.json" <<'PY' +import json +import sys + +with open(sys.argv[1], encoding="utf-8") as stream: + client = json.load(stream) +with open(sys.argv[2], encoding="utf-8") as stream: + expected = json.load(stream) +for key, value in expected["attributes"].items(): + assert client["attributes"].get(key) == value, client["attributes"] +assert client["clientAuthenticatorType"] == "client-secret", client +assert client["publicClient"] is False, client +PY curl --fail --silent --show-error "$metadata_url" >"$tmp_dir/metadata.json" python3 - "$tmp_dir/metadata.json" <<'PY' diff --git a/infrastructure/keycloak/mcp-gateway-client.json b/infrastructure/keycloak/mcp-gateway-client.json new file mode 100644 index 00000000..9a2b0bc6 --- /dev/null +++ b/infrastructure/keycloak/mcp-gateway-client.json @@ -0,0 +1,6 @@ +{ + "clientId": "orgmemory-mcp", + "attributes": { + "standard.token.exchange.enabled": "true" + } +}