From ede32e2b64a5ea6cc438757b96d954353a8cde1f Mon Sep 17 00:00:00 2001 From: mforce Date: Sat, 10 Oct 2026 20:09:10 +0000 Subject: [PATCH 1/6] feat(mcp): map /mcp for OAuth tokens with role and scope on every tool Adds ModelContextProtocol.AspNetCore 2.2.0 and maps /mcp beside the authorization server, stateless, serving MCP 2026-07-28 and 2025-11-25. - Tokens are bound to /mcp (RFC 8707): the server registers it as its only resource, consent stamps it, validation requires it. - The SDK's McpAuthenticationHandler serves the protected-resource metadata and answers a challenge with its URL (RFC 9728). - OAuthScopeRequirement backs AcceptOAuthTokens and two scope policies. A tool carries a role policy and a scope policy, so tools/list and tools/call follow role and scope together. A scope-only denial answers 403 insufficient_scope. - IdempotencyMiddleware skips endpoints marked HandlesOwnIdempotency (only /mcp). /mcp has a 32 KiB body cap. - oauth-api rises to 300 per minute, and its 429 is a JSON-RPC error. - The Cluckwork.Api.Mcp tier privilege is renamed ContractOnly. --- Directory.Packages.props | 1 + docs/decisions/796-oauth-fail-closed.md | 7 +- docs/decisions/806-mcp-endpoint.md | 97 ++++++ docs/decisions/843-adapter-tiers.md | 12 +- docs/plans/770-mcp-server/01-design.md | 17 +- docs/plans/770-mcp-server/02-guards.md | 19 ++ src/Cluckwork.Api/AuthPolicies.cs | 10 + src/Cluckwork.Api/Cluckwork.Api.csproj | 1 + ...workIdentityServiceCollectionExtensions.cs | 10 +- src/Cluckwork.Api/Hosting/CluckworkMcp.cs | 58 ++++ ...RateLimitingServiceCollectionExtensions.cs | 14 + .../ForbiddenProblemResultHandler.cs | 22 +- .../Middleware/IdempotencyMiddleware.cs | 15 + .../Modules/Access/OAuth/OAuthEndpoints.cs | 3 +- .../Access/OAuth/OAuthScopeRequirement.cs | 19 ++ src/Cluckwork.Api/Program.cs | 4 + .../RateLimiting/RateLimitingOptions.cs | 4 +- src/Cluckwork.Api/packages.lock.json | 30 ++ .../Common/Architecture/Modules/Platform.cs | 4 +- .../Access/OAuth/OAuthServerRegistration.cs | 36 ++- .../Mcp/McpAuthorizationTests.cs | 289 ++++++++++++++++++ .../Mcp/McpConnection.cs | 168 ++++++++++ .../Mcp/McpEndpointTests.cs | 113 +++++++ .../Mcp/McpToolSurface.cs | 30 +- .../Mcp/McpToolSurfaceTests.cs | 51 ++++ .../packages.lock.json | 31 ++ .../Architecture/AdapterReachTests.cs | 2 +- .../Architecture/AdapterTierRealTreeTests.cs | 11 - .../Architecture/AdapterTierTests.cs | 6 +- .../Architecture/ModuleLedger.cs | 4 +- .../TenantBypass/BypassAllowList.cs | 2 +- tools/oauth/mutation-check.sh | 39 ++- 32 files changed, 1073 insertions(+), 56 deletions(-) create mode 100644 docs/decisions/806-mcp-endpoint.md create mode 100644 src/Cluckwork.Api/Hosting/CluckworkMcp.cs create mode 100644 src/Cluckwork.Api/Modules/Access/OAuth/OAuthScopeRequirement.cs create mode 100644 tests/Cluckwork.Api.IntegrationTests/Mcp/McpAuthorizationTests.cs create mode 100644 tests/Cluckwork.Api.IntegrationTests/Mcp/McpConnection.cs create mode 100644 tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 40d7a4932..7a6b55290 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -41,6 +41,7 @@ requires Microsoft.OpenApi [2.7.5, 3.0.0). Reason also noted at the reference in Cluckwork.Api.csproj. --> + diff --git a/docs/decisions/796-oauth-fail-closed.md b/docs/decisions/796-oauth-fail-closed.md index f2d369e63..140a91995 100644 --- a/docs/decisions/796-oauth-fail-closed.md +++ b/docs/decisions/796-oauth-fail-closed.md @@ -64,7 +64,7 @@ logs) or a form body (they would dodge the per-token rate-limit key). |---|---|---|---| | `oauth-token` | `POST /api/v1/oauth/token` | client IP | 20 / 60 s | | `oauth-authorize` | `GET /api/v1/oauth/authorize` | client IP | 20 / 60 s | -| `oauth-api` | every `AcceptOAuthTokens` endpoint | SHA-256 of the bearer | 120 / 60 s | +| `oauth-api` | every `AcceptOAuthTokens` endpoint | SHA-256 of the bearer | 300 / 60 s (120 until #806) | OpenIddict also accepts a POSTed authorization request, which would match no endpoint and so no policy or body cap. A server handler refuses any non-GET authorization request @@ -82,8 +82,9 @@ ignores any ambient session bearer, which would otherwise resolve a tenant and d - **Junk bearers.** Each invalid token gets its own `oauth-api` bucket and costs one token lookup. A per-IP ceiling beside the per-token key is not built. - **Response shape for OAuth callers.** The chain's 401s carry no `WWW-Authenticate: - Bearer error="invalid_token"`, and a scope denial uses the role-denial body. #806 decides - what an MCP client needs. + Bearer error="invalid_token"`. #806 decided what an MCP client needs: a challenge naming + the protected-resource metadata, and `insufficient_scope` on a scope denial + ([`806-mcp-endpoint.md`](806-mcp-endpoint.md)). - **Audit attribution** of the acting client (#788 plans first-class columns). ## How it is enforced diff --git a/docs/decisions/806-mcp-endpoint.md b/docs/decisions/806-mcp-endpoint.md new file mode 100644 index 000000000..6beff0f7f --- /dev/null +++ b/docs/decisions/806-mcp-endpoint.md @@ -0,0 +1,97 @@ +# The MCP endpoint: OAuth only, bound to /mcp, stateless (#806) + +> The design is [`docs/plans/770-mcp-server/`](../plans/770-mcp-server/01-design.md); its +> guard table is [`02-guards.md`](../plans/770-mcp-server/02-guards.md). This file records +> what #806 decided while mapping the endpoint. + +**Status:** accepted +**Date:** 2026-10-10 + +## What happened + +No incident. Milestone 10 (#788) built the authorization server and left four MCP-shaped +questions to this slice: which tokens `/mcp` accepts, how a client discovers where to +authenticate, how scopes reach individual tools, and what the per-token rate limit should +be. The decisions on #789 (2026-10-10) settled the protocol version, audience binding and +the rate figure. The rest is recorded here. + +## The rule + +**`/mcp` exists only beside the authorization server.** `Program.cs` maps it inside +`if (identity.OAuthServer)`. Without an issuer the OAuth scheme is not registered, and +`/mcp` accepts no other token. The SDK is `ModelContextProtocol.AspNetCore` 2.2.0, and one +endpoint serves MCP 2026-07-28 and 2025-11-25 clients. + +**Stateless, pinned.** `SessionMode = Stateless` makes the SDK run each tool in the HTTP +request's own services, so `McpCallContext` (#805) sees the request's tenant, actor and flock +scope, and each tool call is one authenticated request that `CredentialEpochMiddleware` +re-checks (#364). The SDK's idle-session sweep is registered but never starts. + +**Tokens are bound to `/mcp` (RFC 8707).** The resource is `/mcp`, never the +request's Host. The authorization server registers it as its only resource, so an authorize +or token request naming another resource gets `invalid_target`. Consent stamps the named +resource on the code, and so on the token; a client that names none gets `/mcp`, the only +resource there is. Validation adds `/mcp` as the required audience, so a token minted before +this change, which carries none, is refused. + +**Discovery is the SDK's `McpAuthenticationHandler`.** The bearer selector's OAuth branch is +now the MCP scheme. That scheme authenticates through OpenIddict validation, answers a +challenge with `WWW-Authenticate: Bearer resource_metadata="/.well-known/oauth-protected-resource/mcp"`, +and serves that document (RFC 9728), naming the issuer as the authorization server and the +two scopes. Both URLs come from `OAuth:Issuer`. The handler only answers a metadata request +whose Host matches the issuer's. + +**Scopes reach tools as policies.** `OAuthScopeRequirement` replaces the inline assertion in +`AcceptOAuthTokens` and backs two named policies, `AuthPolicies.FarmReadScope` and +`AuthPolicies.DailyEntriesWriteScope`. Each tool carries a role `[Authorize]` and a scope +`[Authorize]`; the SDK's `AddAuthorizationFilters()` ANDs them, so `tools/list` shows a tool +only for role ∩ scope and `tools/call` refuses the rest. `McpToolSurfaceTests` fails a tool +missing either gate, and one carrying `[AllowAnonymous]`, because the SDK runs such a tool +for anyone. `AcceptOAuthTokens(ReadFarm, WriteDailyEntries)` stays on the mapping as the +outer gate. + +**A missing scope says which one.** When only scope requirements failed, +`ForbiddenProblemResultHandler` answers 403 with +`WWW-Authenticate: Bearer error="insufficient_scope", scope="...", resource_metadata="..."` +(RFC 6750 §3.1). When the role failed too, a new token would not help, and the role body +answers as before. + +**Idempotency is exempt by endpoint metadata.** Every MCP message is a POST and no MCP +client sends an `Idempotency-Key`, so `IdempotencyMiddleware` skips an endpoint carrying +`HandlesOwnIdempotencyAttribute`. Only `/mcp` carries it, pinned by +`McpEndpointTests.IdempotencyExemption_IsCarriedByTheMcpPostAlone`. The write tool keys its +own claim (#809). + +**Budget: 300 messages per minute per token.** `oauth-api` rose from 120. Measured with the +SDK's own client against this endpoint, a session spends 3 POSTs to its first tool result on +2026-07-28 (`server/discover`, `tools/list`, `tools/call`) and 4 on 2025-11-25 (`initialize` +and its notification replace discovery); each later call is one POST. No account-wide +ceiling: each token costs a step-up password entry (#798), which the password limiter +already bounds. A 429 on `oauth-api` is a JSON-RPC error with a null id, because the limiter +runs before the body is read, and its text names the connection, not an address. + +**The body cap is 32 KiB** (`WithMaxRequestBodyBytes`), beside `ReadsRequestBodyAttribute`, +which only classifies binding errors. + +**The adapter tier row grants nothing.** `Cluckwork.Api.Mcp`'s privilege is now +`ContractOnly`: CW1004 holds tool classes to module contracts as it holds endpoints. The old +`DirectRepository` name and its reason ("inject repositories by design") were wrong. + +## What this does NOT cover + +- **401s from the shared request checks carry no challenge header.** A disabled user, a + suspended farm, a superseded credential or the farm switch writes its 401 directly + (#796). An MCP client then probes `/.well-known/oauth-protected-resource/mcp` itself, as + the MCP authorization spec requires, and that document is served. +- **A tool-level scope denial is a JSON-RPC error, not an HTTP 403.** The SDK refuses a + `tools/call` for a tool the caller cannot see with `InvalidRequest` inside a 200. Only the + endpoint gate answers `insufficient_scope`. A client that follows `tools/list` never calls + a hidden tool. +- **Route parity for tools** (guard rows 13 to 15) arrives with the first tool (#807). + +## How it is enforced + +`tests/Cluckwork.Api.IntegrationTests/Mcp/`: `McpEndpointTests` reads the endpoint table and +services (rows 8 to 12, 17, 18, 29, 30), and `McpAuthorizationTests` drives the real OAuth +flow and the SDK's client against `/mcp` with probe tools that exist only in the test host. +`tools/oauth/mutation-check.sh` proves each test can fail; the #806 mutants are listed there. diff --git a/docs/decisions/843-adapter-tiers.md b/docs/decisions/843-adapter-tiers.md index ad4e137f8..94e041d6c 100644 --- a/docs/decisions/843-adapter-tiers.md +++ b/docs/decisions/843-adapter-tiers.md @@ -27,7 +27,7 @@ namespace, tied to the surface that will map it and a review issue that gives the exemption an end date. `RealModuleLedger.AdapterTiers` in `tests/Cluckwork.Application.Tests/Architecture/RealModuleLedger.Adapters.cs` is an array of `AdapterTier(Namespace, Privilege, Surface, Reason, ReviewBy)` rows; today it -holds one, `Cluckwork.Api.Mcp` → `DirectRepository`, surfaced by `MapMcp`, +holds one, `Cluckwork.Api.Mcp` → `ContractOnly`, surfaced by `MapMcp`, reviewed by `#806`. `AdapterTierScanner` walks every `.cs` under `src/` for a type carrying `[McpServerToolType]` (simple, qualified, or `Attribute`-suffixed, or reached through a `using X = ...;` alias — a file-level or namespace-block @@ -38,9 +38,9 @@ is red, and the failure prints the C# row to add. A tier row whose surface is never invoked and whose namespace holds no type is `Dormant` — informational, never red, because the row is meant to be committed before the code exists. `privilege` and `surface` are each a closed set, both read from -`AdapterTier.KnownSurfaces` beside `AdapterTier.DirectRepositoryPrivilege` — a +`AdapterTier.KnownSurfaces` beside `AdapterTier.ContractOnlyPrivilege` — a map from each supported surface to the privilege it grants, today just -`MapMcp` → `DirectRepository`. `AdapterTierScanner` walks invocations of the +`MapMcp` → `ContractOnly`. `AdapterTierScanner` walks invocations of the same map's keys, so a row naming a surface the scanner does not walk is a registry error rather than a silently inert exemption. `reviewBy` must match `^#[0-9]+$`, so an exemption with no end date is a registry error, alongside a @@ -49,8 +49,10 @@ blank or missing field and a duplicated `namespace` or `surface`. A tier is also an adapter root with a privilege. `AdapterReachScanner` unions every `adapterTiers[].namespace` into its own namespace roots — so a tool class's repository parameters are walked for module reach exactly like an -endpoint's — and into its persistence-forbidden set, because `DirectRepository` -authorizes a repository, not a direct `AppDbContext`. A tool class reaching an +endpoint's — and into its persistence-forbidden set, so a tool class never takes a +direct `AppDbContext`. **Amended by #806:** the privilege was named `DirectRepository` +until #806 mapped `/mcp`. Since #1123 CW1004 holds tool classes to module contracts, so the +row grants no repository reach, and the name now says so. A tool class reaching an undeclared module still needs an `adapters` row; one that takes `AppDbContext` still fails. diff --git a/docs/plans/770-mcp-server/01-design.md b/docs/plans/770-mcp-server/01-design.md index c43b97cd6..ef847bdc3 100644 --- a/docs/plans/770-mcp-server/01-design.md +++ b/docs/plans/770-mcp-server/01-design.md @@ -49,15 +49,18 @@ harnesses; #684/#146 packaging. No new process and nothing to install. The server is the API already running. ```jsonc -{ "mcpServers": { "cluckwork": { - "type": "http", - "url": "https://farm.example/mcp", - "headers": { "Authorization": "Bearer ${CLUCKWORK_TOKEN}" } } } } +{ "mcpServers": { "cluckwork": { "type": "http", "url": "https://farm.example/mcp" } } } ``` -The bearer is an ordinary Cluckwork access token from `POST /api/v1/auth/login` with a farm -code — the same one the SPA carries. Farm, role and flock assignments all come from it. There -is no MCP-specific identity, no API key, and **no new server config key**. +The assistant connects with OAuth, not a pasted token. **Corrected by #806:** the original text +said a session token from `POST /api/v1/auth/login` connects, and `/mcp` refuses session JWTs. +An unauthenticated request gets a 401 whose `WWW-Authenticate` names +`/.well-known/oauth-protected-resource/mcp` (RFC 9728). That document names the authorization +server (`OAuth:Issuer`, #795), where the assistant registers and a person approves it on the +consent screen (#797, #798). The resulting token is bound to the `/mcp` resource (RFC 8707) and +carries the approver's farm, role and flock assignments, so the assistant acts as that person. +There is no API key and **no new server config key**. See +[`docs/decisions/806-mcp-endpoint.md`](../../decisions/806-mcp-endpoint.md). ### Discovery is role-filtered, not merely enforced diff --git a/docs/plans/770-mcp-server/02-guards.md b/docs/plans/770-mcp-server/02-guards.md index 0bb84bbe2..6261c7e66 100644 --- a/docs/plans/770-mcp-server/02-guards.md +++ b/docs/plans/770-mcp-server/02-guards.md @@ -31,6 +31,11 @@ matters more than count: prefer "walk everything, exclude deliberately". | 11 | `McpBoots_UnderProductionShapedConfig` | #370/#565 stay untouched — no new required config key | add any required config key without teaching `bootstrap.sh`, `docker-compose.sim.yml`, `verify-harness.sh` and the AppHost | | 12 | `Mcp_AddsNoLiveHostedServiceUnderStateless` | #271's blocker list is not extended | set `SessionMode = Stateful`, which makes `IdleTrackingBackgroundService.StartAsync` run its prune timer instead of returning early. **Asserts inertness, not absence** — the service IS registered unconditionally by `WithHttpTransport`; a guard asserting zero `IHostedService` descriptors would be red on the correct configuration. Candidate 1 proposed exactly that wrong guard. | +#806 implements rows 8 to 12 in `tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs`. +Row 10 is the existing `BodyReadingEndpointTests` plus `McpPost_CarriesTheBodyCap_TheBudget_AndTheOAuthGate`. +Row 11 has no test of its own: `/mcp` adds no config key, and `ProcessRoleGuardTests` already +boots a Production-shaped serving process, which maps `/mcp`. + ## RBAC | # | Guard | Invariant | Red mutation | @@ -40,6 +45,12 @@ matters more than count: prefer "walk everything, exclude deliberately". | 15 | `MirrorsRoute_NamesResolve` | the registry stays honest | rename a route's `WithName()` and leave the attribute stale | | 16 | `ToolsList_IsFilteredByRole` (integration, real request) | unauthorized tools are invisible, not merely refused | remove `AddAuthorizationFilters()`. Single point of proof for SDK list-filtering behaviour — flagged as a risk in [`01-design.md`](01-design.md). | +#806 implements row 16 as `McpAuthorizationTests.ToolsList_ShowsOnlyToolsTheRoleAndScopesAllow`, +over probe tools that exist only in its test host, for role and for scope. Rows 13 to 15 wait +for the first tool (#807): with no tool there is no route to mirror. #806 adds a walk the +table did not have, `McpToolSurfaceTests.RealTree_EveryToolCarriesARolePolicyAndAScopePolicy`: +the SDK runs a tool with no `[Authorize]`, or with `[AllowAnonymous]`, for any caller. + ## Idempotency | # | Guard | Invariant | Red mutation | @@ -53,6 +64,9 @@ matters more than count: prefer "walk everything, exclude deliberately". | 23 | `Replay_WritesNoSecondAuditRow` | replay is a true no-op | drop the publish guard. **Asserts the absence of a second `DailyEntryUpdate` audit row, NOT the entry count** — the handler upserts on `(account, farm, house, flock, date)`, so a count assertion passes with idempotency entirely removed. Graft from candidate 1. | | 24 | `AtomicIdempotencyProtocolTests` + the other #307 suites | the extraction changed no behaviour | **these must pass UNEDITED.** An edit to a #307 test in the implementing PR is a stop-and-review, not a fix. This is the mitigation for the design's largest risk. | +#806 implements rows 17 and 18 as `McpEndpointTests.IdempotencyExemption_IsCarriedByTheMcpPostAlone` +and `McpMessages_WithoutAnIdempotencyKey_AreServed`. + ## Domain surface | # | Guard | Invariant | Red mutation | @@ -68,6 +82,11 @@ matters more than count: prefer "walk everything, exclude deliberately". | 31b | `BalanceReads_AreBoundedInTheDATABASE` (row-consumption assertion) | the **query** is bounded, not just the reply | keep the reply's `Take(cap)` and remove the bound from the query. Row 31 stays GREEN — every response is correctly capped with correct continuation metadata — while `ListCustomerBalancesAsync` materializes the entire farm's balance book first. `PaymentRepository` takes no paging arguments and materializes both grouped queries before building its list, so **reusing it at all is the defect**; this needs a paged balance query, and the test must assert rows consumed from the database, not items returned. A signature walk cannot establish this. **Measure rows CONSUMED, not SQL shape.** `ReportQueryBoundingTests`' `SqlCaptureInterceptor` is the right harness shape and the right precedent (#311 was the identical "loaded the whole ledger, walked it in memory" defect), but it records `(Sql, Parameters)` only — so an implementation that fetches every page in a loop and accumulates the book emits perfectly paged SQL and passes. Keep SQL inspection as *supporting* evidence and count consumption across the whole operation: wrap the returned `DbDataReader` and count successful `Read`/`ReadAsync` calls, covering both aggregate queries and any page-fetch loop. (Round-3 finding 3 — the round-2 fix named a mechanism that cannot measure what the row requires.) (Round-2 finding 2 — row 31 alone claimed an invariant its mutation did not test.) | | 32 | `NameResolution_IsExactlyOneAccessibleMatch` | a name-addressed write cannot hit the wrong row | make resolution take the first match. **Duplicate flock names are legal** (`FlockRepository.cs` says so, and its `ThenBy(f => f.Id)` tie-break exists because of it), so first-match records production against an arbitrary flock and `Single` throws on valid farm data. Zero matches → not-found; two or more → an ambiguity error listing candidate ids and distinguishing fields, and the follow-up write must pass an id. Test two same-name flocks in one accessible scope. (#770 adversarial review, finding 3.) | +#806 implements rows 29 and 30 as `McpEndpointTests.McpPost_CarriesTheBodyCap_TheBudget_AndTheOAuthGate` +and `OversizedBody_IsRefused`. `/mcp` takes its budget from `AcceptOAuthTokens`, the +`oauth-api` policy, which `OAuthFailClosedTests.OAuthLimits_AreDecidedByTheSharedCounter` +proves is decided by the shared counter. + ## Where these guards stop, stated plainly Row 7b **bounds** the secondary-scope hazard; it does not eliminate it. A closed registration model diff --git a/src/Cluckwork.Api/AuthPolicies.cs b/src/Cluckwork.Api/AuthPolicies.cs index 875c6d316..75fa80303 100644 --- a/src/Cluckwork.Api/AuthPolicies.cs +++ b/src/Cluckwork.Api/AuthPolicies.cs @@ -1,3 +1,5 @@ +using Cluckwork.Api.Modules.Access.OAuth; +using Cluckwork.Application.Modules.Access.Contracts; using Cluckwork.Domain.Modules.Farm.Contracts; namespace Cluckwork.Api; @@ -22,6 +24,11 @@ public static class AuthPolicies public const string SalesFlow = "SalesFlow"; // everyone but ReadOnly (workers sell — #73 principle) public const string ProductionWrite = "ProductionWrite"; // Owner/Manager/Worker(no elevated role) + // #806 — one policy per OAuth scope. An MCP tool carries one of these beside its role + // policy; stacked [Authorize] attributes AND together, so a tool runs for role ∩ scope. + public const string FarmReadScope = "scope:" + OAuthScopes.ReadFarm; + public const string DailyEntriesWriteScope = "scope:" + OAuthScopes.WriteDailyEntries; + // Kept for existing references; prefer Roles.* (#84). public const string AdminRole = Roles.Owner; @@ -56,6 +63,9 @@ public static void AddCluckworkPolicies(this Microsoft.AspNetCore.Authorization. // ReadOnly exist precisely to fence their holders off production. opts.AddPolicy(ProductionWrite, p => p.RequireAuthenticatedUser().RequireAssertion(ctx => EffectiveRole(ctx.User) is Roles.Owner or Roles.Manager or WorkerRole)); + + opts.AddPolicy(FarmReadScope, p => p.AddRequirements(new OAuthScopeRequirement(OAuthScopes.ReadFarm))); + opts.AddPolicy(DailyEntriesWriteScope, p => p.AddRequirements(new OAuthScopeRequirement(OAuthScopes.WriteDailyEntries))); } private const string WorkerRole = "Worker"; diff --git a/src/Cluckwork.Api/Cluckwork.Api.csproj b/src/Cluckwork.Api/Cluckwork.Api.csproj index d0a821a4a..bd0572d6a 100644 --- a/src/Cluckwork.Api/Cluckwork.Api.csproj +++ b/src/Cluckwork.Api/Cluckwork.Api.csproj @@ -11,6 +11,7 @@ + all runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/src/Cluckwork.Api/Hosting/CluckworkIdentityServiceCollectionExtensions.cs b/src/Cluckwork.Api/Hosting/CluckworkIdentityServiceCollectionExtensions.cs index 968d36449..bab5e9262 100644 --- a/src/Cluckwork.Api/Hosting/CluckworkIdentityServiceCollectionExtensions.cs +++ b/src/Cluckwork.Api/Hosting/CluckworkIdentityServiceCollectionExtensions.cs @@ -19,7 +19,7 @@ using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.Identity; using Microsoft.IdentityModel.Tokens; -using OpenIddict.Validation.AspNetCore; +using ModelContextProtocol.AspNetCore.Authentication; namespace Cluckwork.Api.Hosting; @@ -188,13 +188,15 @@ public static CluckworkIdentityRegistration AddCluckworkIdentity( configuration.GetSection("OAuth:ClientMetadata:PrivateHosts").Get()?.ToHashSet(StringComparer.OrdinalIgnoreCase))); // #796 — one handler per endpoint: OpenIddict validation where the endpoint // opted in through AcceptOAuthTokens, the session JWT scheme everywhere else. - // Neither token is ever handed to the other's handler. - services.AddAuthentication(options => options.DefaultScheme = BearerSelectorScheme) + // Neither token is ever handed to the other's handler. #806 — the OAuth side + // is the MCP scheme, which authenticates through OpenIddict validation. + var authentication = services.AddAuthentication(options => options.DefaultScheme = BearerSelectorScheme) .AddPolicyScheme(BearerSelectorScheme, displayName: null, options => options.ForwardDefaultSelector = context => OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint()) - ? OpenIddictValidationAspNetCoreDefaults.AuthenticationScheme + ? McpAuthenticationDefaults.AuthenticationScheme : JwtBearerDefaults.AuthenticationScheme); + services.AddCluckworkMcp(authentication, oauthIssuer); } return new(OAuthServer: oauthIssuer is not null); diff --git a/src/Cluckwork.Api/Hosting/CluckworkMcp.cs b/src/Cluckwork.Api/Hosting/CluckworkMcp.cs new file mode 100644 index 000000000..af287ec68 --- /dev/null +++ b/src/Cluckwork.Api/Hosting/CluckworkMcp.cs @@ -0,0 +1,58 @@ +using Cluckwork.Api.Mcp; +using Cluckwork.Api.Middleware; +using Cluckwork.Api.Modules.Access.OAuth; +using Cluckwork.Application.Modules.Access.Contracts; +using Cluckwork.Infrastructure.Modules.Access.OAuth; +using Microsoft.AspNetCore.Authentication; +using ModelContextProtocol.AspNetCore; +using ModelContextProtocol.Authentication; +using OpenIddict.Validation.AspNetCore; + +namespace Cluckwork.Api.Hosting; + +// #806 — the MCP endpoint: the official SDK serving Streamable HTTP at /mcp, to OAuth +// tokens only. It exists only where the authorization server does. +internal static class CluckworkMcp +{ + public const string Path = "/mcp"; + + // Generous for a JSON-RPC message: the largest planned one is the daily-entry write + // tool's arguments, well under a kilobyte. + public const long MaxRequestBodyBytes = 32 * 1024; + + public static void AddCluckworkMcp(this IServiceCollection services, AuthenticationBuilder authentication, Uri issuer) + { + // Stateless is also the SDK's default. It is set here because it is what runs a + // tool in the HTTP request's own scope, which McpCallContext then checks. + services.AddMcpServer() + .WithHttpTransport(options => options.SessionMode = HttpServerSessionMode.Stateless) + .AddAuthorizationFilters() + .WithToolsFromAssembly(typeof(McpCallContext).Assembly); + + // The scheme the bearer selector picks for every AcceptOAuthTokens endpoint. It + // authenticates through OpenIddict validation, answers a challenge with the + // protected-resource metadata URL (RFC 9728), and serves that document. Both URLs + // come from the configured issuer, never the request's Host (#538). + var resource = OAuthServerRegistration.ProtectedResource(issuer); + authentication.AddMcp(options => + { + options.ForwardAuthenticate = OpenIddictValidationAspNetCoreDefaults.AuthenticationScheme; + options.ResourceMetadataUri = new Uri(resource, "/.well-known/oauth-protected-resource" + resource.AbsolutePath); + options.ResourceMetadata = new ProtectedResourceMetadata + { + Resource = resource.AbsoluteUri, + AuthorizationServers = [issuer.AbsoluteUri], + ScopesSupported = [.. OAuthScopes.All], + ResourceName = "Cluckwork", + }; + }); + } + + // Either scope admits the request; each tool then requires its own (AuthPolicies). + public static IEndpointConventionBuilder MapCluckworkMcp(this IEndpointRouteBuilder app) => + app.MapMcp(Path) + .WithMetadata(new ReadsRequestBodyAttribute(), new HandlesOwnIdempotencyAttribute()) + .WithMaxRequestBodyBytes(MaxRequestBodyBytes) + .AcceptOAuthTokens(OAuthScopes.ReadFarm, OAuthScopes.WriteDailyEntries) + .RequireAuthorization(); +} diff --git a/src/Cluckwork.Api/Hosting/CluckworkRateLimitingServiceCollectionExtensions.cs b/src/Cluckwork.Api/Hosting/CluckworkRateLimitingServiceCollectionExtensions.cs index 8d57dcd5f..32e3b8321 100644 --- a/src/Cluckwork.Api/Hosting/CluckworkRateLimitingServiceCollectionExtensions.cs +++ b/src/Cluckwork.Api/Hosting/CluckworkRateLimitingServiceCollectionExtensions.cs @@ -94,6 +94,20 @@ public static CluckworkRateLimitingRegistration AddCluckworkRateLimiting( context.HttpContext.Request.Path.Value); } + // #806 — oauth-api counts per token and serves /mcp, whose client reads a + // JSON-RPC error. The body is unread here, so the request id is unknown. + if (policyName == RateLimitingOptions.OAuthApiPolicyName) + { + context.HttpContext.Response.StatusCode = StatusCodes.Status429TooManyRequests; + await context.HttpContext.Response.WriteAsJsonAsync(new + { + jsonrpc = "2.0", + id = (string?)null, + error = new { code = -32000, message = "Too many requests with this connection. Try again later." }, + }); + return; + } + await Results.Problem( title: "Too many requests", detail: diff --git a/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs b/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs index a5d792370..2d701d254 100644 --- a/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs +++ b/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs @@ -1,6 +1,9 @@ +using Cluckwork.Api.Modules.Access.OAuth; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Authorization.Policy; using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Options; +using ModelContextProtocol.AspNetCore.Authentication; namespace Cluckwork.Api.Middleware; @@ -16,11 +19,28 @@ public async Task HandleAsync( { if (authorizeResult.Forbidden) { + var failed = authorizeResult.AuthorizationFailure?.FailedRequirements.ToList() ?? []; + var onlyScopesFailed = failed.Count > 0 && failed.All(r => r is OAuthScopeRequirement); + + // #806 — RFC 6750 §3.1 and the MCP scope challenge: a token that lacks a scope + // tells its client which scopes to ask the user for. When the role failed too, + // no new token would help, so the role body below answers instead. + if (onlyScopesFailed) + { + var scopes = failed.Cast().SelectMany(r => r.AnyOf).Distinct(); + var metadata = context.RequestServices.GetRequiredService>() + .Get(McpAuthenticationDefaults.AuthenticationScheme).ResourceMetadataUri; + context.Response.Headers.WWWAuthenticate = + $"Bearer error=\"insufficient_scope\", scope=\"{string.Join(' ', scopes)}\", resource_metadata=\"{metadata}\""; + } + context.Response.StatusCode = StatusCodes.Status403Forbidden; await context.Response.WriteAsJsonAsync(new ProblemDetails { Title = "Forbidden", - Detail = "This action requires the Admin role.", + Detail = onlyScopesFailed + ? "This connection was not given the permission this action needs. Reconnect the app and allow it." + : "This action requires the Admin role.", Status = StatusCodes.Status403Forbidden }); return; diff --git a/src/Cluckwork.Api/Middleware/IdempotencyMiddleware.cs b/src/Cluckwork.Api/Middleware/IdempotencyMiddleware.cs index 8289af69d..8800d7ee6 100644 --- a/src/Cluckwork.Api/Middleware/IdempotencyMiddleware.cs +++ b/src/Cluckwork.Api/Middleware/IdempotencyMiddleware.cs @@ -134,6 +134,16 @@ public async Task InvokeAsync( return; } + // #806 — /mcp carries every MCP message as a POST, initialize and tools/list + // included, and no MCP client sends an Idempotency-Key. A write tool keys its own + // claim on the tool and its arguments (#809). Matched on endpoint metadata, never + // a path, so the bearer set is what the endpoint table says it is. + if (context.GetEndpoint()?.Metadata.GetMetadata() is not null) + { + await next(context); + return; + } + if (ResponseNotCacheable.Any(p => context.Request.Path.StartsWithSegments(p, StringComparison.OrdinalIgnoreCase))) { @@ -610,3 +620,8 @@ private static async Task ComputeRequestHashAsync(HttpRequest request, C private static string Sha256(string value) => Convert.ToHexString( SHA256.HashData(Encoding.UTF8.GetBytes(value))).ToLowerInvariant(); } + +// #806 — marks an endpoint IdempotencyMiddleware skips because the endpoint keys its own +// writes. Only /mcp carries it; McpEndpointTests pins that set. +[AttributeUsage(AttributeTargets.Method | AttributeTargets.Class)] +public sealed class HandlesOwnIdempotencyAttribute : Attribute; diff --git a/src/Cluckwork.Api/Modules/Access/OAuth/OAuthEndpoints.cs b/src/Cluckwork.Api/Modules/Access/OAuth/OAuthEndpoints.cs index 479267e36..fbe9ff26d 100644 --- a/src/Cluckwork.Api/Modules/Access/OAuth/OAuthEndpoints.cs +++ b/src/Cluckwork.Api/Modules/Access/OAuth/OAuthEndpoints.cs @@ -76,8 +76,7 @@ public static TBuilder AcceptOAuthTokens(this TBuilder builder, params return builder .WithMetadata(new AcceptsOAuthTokensMarker()) .RequireRateLimiting(RateLimitingOptions.OAuthApiPolicyName) - .RequireAuthorization(policy => policy.RequireAssertion(context => - scopes.Any(context.User.HasScope))); + .RequireAuthorization(policy => policy.AddRequirements(new OAuthScopeRequirement(scopes))); } public static bool AcceptsOAuthTokens(Endpoint? endpoint) => diff --git a/src/Cluckwork.Api/Modules/Access/OAuth/OAuthScopeRequirement.cs b/src/Cluckwork.Api/Modules/Access/OAuth/OAuthScopeRequirement.cs new file mode 100644 index 000000000..5c71d7080 --- /dev/null +++ b/src/Cluckwork.Api/Modules/Access/OAuth/OAuthScopeRequirement.cs @@ -0,0 +1,19 @@ +using Microsoft.AspNetCore.Authorization; +using OpenIddict.Abstractions; + +namespace Cluckwork.Api.Modules.Access.OAuth; + +// #806 — the caller's token must hold one of these scopes. A requirement type, not an +// assertion, so a denial can name the scopes that would have passed (insufficient_scope, +// ForbiddenProblemResultHandler). It is its own handler, as AssertionRequirement is. +public sealed class OAuthScopeRequirement(params string[] anyOf) : IAuthorizationRequirement, IAuthorizationHandler +{ + public IReadOnlyList AnyOf { get; } = anyOf; + + public Task HandleAsync(AuthorizationHandlerContext context) + { + if (AnyOf.Any(context.User.HasScope)) + context.Succeed(this); + return Task.CompletedTask; + } +} diff --git a/src/Cluckwork.Api/Program.cs b/src/Cluckwork.Api/Program.cs index 02894640f..97b936451 100644 --- a/src/Cluckwork.Api/Program.cs +++ b/src/Cluckwork.Api/Program.cs @@ -473,10 +473,14 @@ .MapExportEndpoints(); // #795 — OpenIddict answers the token endpoint itself; only authorization reaches an endpoint. +// #806 — /mcp accepts only OAuth tokens, so it exists only beside the server that issues them. if (identity.OAuthServer) +{ app.MapGroup("/api/v1/oauth") .WithTags("OAuth") .MapOAuthEndpoints(); + app.MapCluckworkMcp(); +} // #217 — browser error reports. Anonymous (the login screen can crash too); // the endpoint carries its own per-IP rate limit and size cap inside. diff --git a/src/Cluckwork.Api/RateLimiting/RateLimitingOptions.cs b/src/Cluckwork.Api/RateLimiting/RateLimitingOptions.cs index c64a4cc7c..d199c960e 100644 --- a/src/Cluckwork.Api/RateLimiting/RateLimitingOptions.cs +++ b/src/Cluckwork.Api/RateLimiting/RateLimitingOptions.cs @@ -34,7 +34,9 @@ public sealed class RateLimitingOptions public FixedWindow OAuthToken { get; init; } = new() { PermitLimit = 20, WindowSeconds = 60 }; public FixedWindow OAuthAuthorize { get; init; } = new() { PermitLimit = 20, WindowSeconds = 60 }; // An assistant calls far faster than a person clicks (#796), so this is per minute. - public FixedWindow OAuthApi { get; init; } = new() { PermitLimit = 120, WindowSeconds = 60 }; + // #806 — /mcp is the only endpoint on it, and each MCP message is one POST: a session + // spends some before its first tool call (decision 806-mcp-endpoint.md). + public FixedWindow OAuthApi { get; init; } = new() { PermitLimit = 300, WindowSeconds = 60 }; public FixedWindow OAuthRegister { get; init; } = new() { PermitLimit = 10, WindowSeconds = 3600 }; // Small on purpose: a report query is a bounded-range aggregate (#311), not // a hot path — a genuine user rarely has more than one or two in flight at diff --git a/src/Cluckwork.Api/packages.lock.json b/src/Cluckwork.Api/packages.lock.json index 05b4e196a..448e587ab 100644 --- a/src/Cluckwork.Api/packages.lock.json +++ b/src/Cluckwork.Api/packages.lock.json @@ -49,6 +49,15 @@ "resolved": "2.12.2", "contentHash": "ZkT7qiJe0Kj0+DTZOZo+xrDy1kNmdsWFQVPIQLNx9ZJ0T9cB2LG7r+Bi60so8/+yg1nkS7QCPadrzsTG4VPKrA==" }, + "ModelContextProtocol.AspNetCore": { + "type": "Direct", + "requested": "[2.2.0, )", + "resolved": "2.2.0", + "contentHash": "3JelDMuFIwFzXybsh6K30G6wXu5gmqKnuRnieqgBMuR0FkqqXRv4B+NYZuPgthSyNq5UwRyvv2U2VJBE3RD3PQ==", + "dependencies": { + "ModelContextProtocol": "[2.2.0]" + } + }, "OpenTelemetry.Exporter.OpenTelemetryProtocol": { "type": "Direct", "requested": "[1.*, )", @@ -189,6 +198,11 @@ "Microsoft.EntityFrameworkCore": "10.0.12" } }, + "Microsoft.Extensions.AI.Abstractions": { + "type": "Transitive", + "resolved": "10.8.3", + "contentHash": "K0B05oApxmviWalNHPMBBcRC7erKiDATz3ENNR/jqTR9JwIwLRefgDhj2jCRwL1aca99pXUe0qyQC73/xIuZig==" + }, "Microsoft.Extensions.DependencyModel": { "type": "Transitive", "resolved": "10.0.12", @@ -246,6 +260,22 @@ "resolved": "1.0.52", "contentHash": "oNv2JtYXhpdJrX63nibx1JT3uCESOBQ1LAk7Dtz/sr0+laW0KRM6eKp4CZ3MHDR2siIkKsY8MmUkeP5DKkQQ5w==" }, + "ModelContextProtocol": { + "type": "Transitive", + "resolved": "2.2.0", + "contentHash": "4Pb9u02Nwsp0poueDsqNdyGRojFxOYpljB7zDBsq+aHL+Afou3OgxlBc3GWFVnsRMRJrUtWqDh3s6k2JgPzmrQ==", + "dependencies": { + "ModelContextProtocol.Core": "[2.2.0]" + } + }, + "ModelContextProtocol.Core": { + "type": "Transitive", + "resolved": "2.2.0", + "contentHash": "FeBfXU6T8k+jw4afg4sfxdEX2rL/e5oKOk9ROOGztu9k47+7Bz08sdaToYt2XvMY1opNbwxYQOFMj6wH9TInhA==", + "dependencies": { + "Microsoft.Extensions.AI.Abstractions": "10.8.3" + } + }, "Mono.TextTemplating": { "type": "Transitive", "resolved": "3.0.0", diff --git a/src/Cluckwork.Domain/Common/Architecture/Modules/Platform.cs b/src/Cluckwork.Domain/Common/Architecture/Modules/Platform.cs index 041bd727a..88ca13499 100644 --- a/src/Cluckwork.Domain/Common/Architecture/Modules/Platform.cs +++ b/src/Cluckwork.Domain/Common/Architecture/Modules/Platform.cs @@ -29,8 +29,8 @@ namespace Cluckwork.Domain.Common.Architecture; PersistenceForbiddenNamespaces = ["Cluckwork.Api.Endpoints", "Cluckwork.Api.Modules"])] [AdapterTier( "Cluckwork.Api.Mcp", - "DirectRepository", + "ContractOnly", "MapMcp", - "MCP tool classes inject repositories by design (docs/plans/770-mcp-server/01-design.md:112); the contract-first shape is Track C (#514)", + "MCP tool classes are adapters behind /mcp; CW1004 holds them to module contracts, as it holds endpoints (docs/decisions/806-mcp-endpoint.md)", "#806")] internal static class PlatformModuleRules { } diff --git a/src/Cluckwork.Infrastructure/Modules/Access/OAuth/OAuthServerRegistration.cs b/src/Cluckwork.Infrastructure/Modules/Access/OAuth/OAuthServerRegistration.cs index 6f4da790b..fc0f28f2c 100644 --- a/src/Cluckwork.Infrastructure/Modules/Access/OAuth/OAuthServerRegistration.cs +++ b/src/Cluckwork.Infrastructure/Modules/Access/OAuth/OAuthServerRegistration.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Primitives; +using OpenIddict.Abstractions; using OpenIddict.Server; using OpenIddict.Validation; using static OpenIddict.Abstractions.OpenIddictConstants; @@ -18,9 +19,15 @@ namespace Cluckwork.Infrastructure.Modules.Access.OAuth; // authorization code with PKCE, reference access tokens that live until revoked (#788). public static class OAuthServerRegistration { + // #806 — the one resource this server issues tokens for: the API's /mcp endpoint, + // under the configured issuer and never the request's Host (#538). RFC 8707 binds + // each token to it, so a token cannot be replayed at any other resource. + public static Uri ProtectedResource(Uri issuer) => new(PublicBase(issuer), "mcp"); + public static IServiceCollection AddAccessOAuthServer( this IServiceCollection services, Uri issuer, bool allowPlainHttp, ClientMetadataOptions clientMetadata) { + var resource = ProtectedResource(issuer).AbsoluteUri; services.AddOpenIddict() .AddCore(core => core .UseEntityFrameworkCore() @@ -37,10 +44,13 @@ public static IServiceCollection AddAccessOAuthServer( .UseReferenceAccessTokens() .SetAccessTokenLifetime(null) .RegisterScopes([.. OAuthScopes.All]) + // A request naming any other resource is refused (invalid_target). + .RegisterResources(resource) // Every client registers itself anonymously (#797), so a per-client - // scope permission would be granted to all of them anyway. The user's - // consent is what limits a connection's scopes (#798). + // scope or resource permission would be granted to all of them anyway. + // The user's consent is what limits a connection's scopes (#798). .IgnoreScopePermissions() + .IgnoreResourcePermissions() // OpenIddict refuses to start without both keys, but with the Data // Protection format they only ever sign identity tokens, which this // server never issues. Codes and access tokens are protected by the @@ -105,7 +115,7 @@ public static IServiceCollection AddAccessOAuthServer( // Host the request arrived with: behind a proxy that Host can be an internal // name. #797 — OpenIddict has no registration endpoint; the API maps one // beside authorize, and discovery points clients at it. - var publicBase = issuer.AbsoluteUri.EndsWith('/') ? issuer : new Uri(issuer.AbsoluteUri + "/"); + var publicBase = PublicBase(issuer); server.AddEventHandler(handler => handler .UseInlineHandler(context => { @@ -122,6 +132,20 @@ public static IServiceCollection AddAccessOAuthServer( server.AddEventHandler(ClientMetadataDocuments.Descriptor); + // #806 — RFC 8707: the code, and the token it becomes, carry the resource the + // client named, which OpenIddict has already checked is /mcp. A client that + // names none gets /mcp, the only resource there is. + server.AddEventHandler(handler => handler + .UseInlineHandler(context => + { + if (context.EndpointType is OpenIddictServerEndpointType.Authorization) + context.Principal!.SetResources(context.Request!.GetResources() is { IsEmpty: false } asked + ? asked + : [resource]); + return default; + }) + .SetOrder(OpenIddictServerHandlers.InferResources.Descriptor.Order - 500)); + // Token passthrough maps the endpoint, which is what lets it opt into a // rate-limit policy and a body cap; OpenIddict still validates first. var aspNetCore = server.UseAspNetCore() @@ -134,6 +158,9 @@ public static IServiceCollection AddAccessOAuthServer( { validation.UseLocalServer(); validation.UseDataProtection(); + // #806 — a token without this audience, or issued for another resource, + // is refused. + validation.AddAudiences(resource); // Disconnect revokes the authorization, so every request checks it (#796). validation.EnableAuthorizationEntryValidation(); // OpenIddict skips that check for a token that names no authorization, so @@ -164,6 +191,9 @@ public static IServiceCollection AddAccessOAuthServer( return services; } + private static Uri PublicBase(Uri issuer) => + issuer.AbsoluteUri.EndsWith('/') ? issuer : new Uri(issuer.AbsoluteUri + "/"); + private static bool HasBearer(HttpRequest request) => request.Headers.Authorization.ToString().StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase); } diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpAuthorizationTests.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpAuthorizationTests.cs new file mode 100644 index 000000000..995b46edc --- /dev/null +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpAuthorizationTests.cs @@ -0,0 +1,289 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text.Json; +using System.Text.Json.Nodes; +using Cluckwork.Api.IntegrationTests.Infrastructure; +using Cluckwork.Application.Modules.Access.Contracts; +using Cluckwork.Domain.Modules.Farm.Contracts; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol; +using ModelContextProtocol.Protocol; +using OpenIddict.Abstractions; +using static Cluckwork.Api.IntegrationTests.Mcp.McpConnection; + +namespace Cluckwork.Api.IntegrationTests.Mcp; + +// #806 — who reaches /mcp and what they see: the challenge and discovery a client needs +// to connect, the token audience, role ∩ scope on tools/list and tools/call, and the +// shared request checks driven through the real route rather than a probe. +[Collection(IntegrationCollection.Name)] +public sealed class McpAuthorizationTests(CluckworkWebApplicationFactory factory) +{ + private static readonly string[] BothScopes = [OAuthScopes.ReadFarm, OAuthScopes.WriteDailyEntries]; + + [Theory] + [InlineData("2026-07-28")] + [InlineData("2025-11-25")] + public async Task McpClient_ConnectsAndCallsATool_OnBothProtocolRevisions(string protocolVersion) + { + using var host = Host(factory); + var user = await SeedAsync(factory, Roles.Manager); + var token = await IssueAsync(host, user, [OAuthScopes.ReadFarm]); + + await using var client = await ConnectAsync(host, token, protocolVersion); + var result = await client.CallToolAsync(ProbeTools.Read); + + Assert.Equal(protocolVersion, client.NegotiatedProtocolVersion); + var caller = Assert.IsType(Assert.Single(result.Content)).Text; + Assert.StartsWith($"{user.AccountId}/{user.Id}/", caller); + } + + // Role ∩ scope, by composition: a tool is listed only when both policies pass. + [Theory] + [InlineData(Roles.Manager, new[] { OAuthScopes.ReadFarm, OAuthScopes.WriteDailyEntries }, new[] { ProbeTools.Read, ProbeTools.Write })] + [InlineData(Roles.Manager, new[] { OAuthScopes.ReadFarm }, new[] { ProbeTools.Read })] + [InlineData(Roles.Manager, new[] { OAuthScopes.WriteDailyEntries }, new[] { ProbeTools.Write })] + [InlineData(Roles.ReadOnly, new[] { OAuthScopes.ReadFarm, OAuthScopes.WriteDailyEntries }, new[] { ProbeTools.Read })] + [InlineData(Roles.Sales, new[] { OAuthScopes.ReadFarm, OAuthScopes.WriteDailyEntries }, new[] { ProbeTools.Read })] + public async Task ToolsList_ShowsOnlyToolsTheRoleAndScopesAllow(string role, string[] scopes, string[] expected) + { + using var host = Host(factory); + var token = await IssueAsync(host, await SeedAsync(factory, role), scopes); + + await using var client = await ConnectAsync(host, token); + var tools = await client.ListToolsAsync(); + + Assert.Equal(expected.Order(), tools.Select(t => t.Name).Order()); + } + + [Fact] + public async Task HiddenTool_IsRefused_WhenCalledAnyway() + { + using var host = Host(factory); + var token = await IssueAsync(host, await SeedAsync(factory, Roles.Manager), [OAuthScopes.ReadFarm]); + + await using var client = await ConnectAsync(host, token); + var refused = await Assert.ThrowsAsync(() => client.CallToolAsync(ProbeTools.Write).AsTask()); + + Assert.Equal(McpErrorCode.InvalidRequest, refused.ErrorCode); + } + + // RFC 9728 §5.1: a client with no token learns where to authenticate from the challenge. + [Fact] + public async Task NoToken_Gets401_NamingTheProtectedResourceMetadata() + { + using var host = Host(factory); + + using var response = await PostAsync(host, bearer: null, Initialize); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + Assert.Equal($"Bearer resource_metadata=\"{ResourceMetadata}\"", response.Headers.WwwAuthenticate.ToString()); + } + + [Fact] + public async Task ProtectedResourceMetadata_NamesTheIssuerAndTheScopes() + { + using var host = Host(factory); + + var metadata = await OAuthServerTests.HttpsClient(host, bearer: null).GetFromJsonAsync(ResourceMetadata); + + Assert.Equal(McpUrl, metadata.GetProperty("resource").GetString()); + Assert.Equal(["https://localhost/"], metadata.GetProperty("authorization_servers").EnumerateArray().Select(s => s.GetString())); + Assert.Equal(BothScopes, metadata.GetProperty("scopes_supported").EnumerateArray().Select(s => s.GetString())); + } + + // The metadata URL comes from OAuth:Issuer, so a request arriving under another Host + // is still pointed at the issuer's (#538). + [Fact] + public async Task Challenge_NamesTheIssuer_NotTheRequestHost() + { + using var host = Host(factory).WithWebHostBuilder(builder => builder.UseSetting("AllowedHosts", "*")); + var request = new HttpRequestMessage(HttpMethod.Post, "https://internal.example/mcp") + { + Content = new StringContent(Initialize, System.Text.Encoding.UTF8, "application/json"), + }; + + using var response = await host.CreateClient().SendAsync(request); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + Assert.Contains(ResourceMetadata, response.Headers.WwwAuthenticate.ToString()); + } + + [Fact] + public async Task TokenWithoutTheMcpAudience_IsRefused() + { + // A token minted before #806 carries no audience at all. + using var host = Host(factory, alterIssued: principal => principal.SetResources(Array.Empty())); + var token = await IssueAsync(host, await SeedAsync(factory, Roles.Manager), [OAuthScopes.ReadFarm]); + + using var response = await PostAsync(host, token, Initialize); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + [Fact] + public async Task TokenIssuedForAnotherResource_IsRefused() + { + using var host = Host(factory); + var token = await IssueAsync(host, await SeedAsync(factory, Roles.Manager), [OAuthScopes.ReadFarm], OtherResource); + + using var response = await PostAsync(host, token, Initialize); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + // The positive half of the two refusals above: naming /mcp, or no resource at all, + // yields a token /mcp accepts. + [Theory] + [InlineData(McpUrl)] + [InlineData(null)] + public async Task TokenForMcp_IsAccepted(string? resource) + { + using var host = Host(factory); + var token = await IssueAsync(host, await SeedAsync(factory, Roles.Manager), [OAuthScopes.ReadFarm], resource); + + using var response = await PostAsync(host, token, Initialize); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + [Fact] + public async Task UnregisteredResource_IsRefused_AtAuthorizeAndAtTheTokenEndpoint() + { + using var host = Host(factory); + var user = await SeedAsync(factory, Roles.Manager); + var clientId = await OAuthServerTests.RegisterClientAsync(host.Services); + const string elsewhere = "https://elsewhere.example/api"; + + using var denied = await OAuthServerTests.SendAuthorizeAsync( + host, user.Jwt, AuthorizeQuery(clientId, Guid.NewGuid().ToString("N"), [OAuthScopes.ReadFarm], elsewhere)); + using var redeemed = await RedeemAsync(host, clientId, "cw806-code", "cw806-verifier", "https://client.example/callback", elsewhere); + + // Refused before the redirect URI is trusted, so OpenIddict answers directly. + Assert.Equal(HttpStatusCode.BadRequest, denied.StatusCode); + Assert.Contains($"error:{OpenIddictConstants.Errors.InvalidTarget}", await denied.Content.ReadAsStringAsync()); + Assert.Equal(HttpStatusCode.BadRequest, redeemed.StatusCode); + Assert.Equal(OpenIddictConstants.Errors.InvalidTarget, + (await redeemed.Content.ReadFromJsonAsync()).GetProperty("error").GetString()); + } + + // RFC 6750 §3.1: the token holds neither MCP scope, so the client is told which to ask for. + [Fact] + public async Task TokenWithoutAnMcpScope_Gets403InsufficientScope() + { + using var host = Host(factory); + var token = await IssueAsync(host, await SeedAsync(factory, Roles.Manager), [OtherScope]); + + using var response = await PostAsync(host, token, Initialize); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + Assert.Equal( + $"Bearer error=\"insufficient_scope\", scope=\"farm:read daily-entries:write\", resource_metadata=\"{ResourceMetadata}\"", + response.Headers.WwwAuthenticate.ToString()); + } + + [Fact] + public async Task SessionJwt_IsRefused() + { + using var host = Host(factory); + var user = await SeedAsync(factory, Roles.Manager); + + using var response = await PostAsync(host, user.Jwt, Initialize); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + // The shared request checks (#796, #1146) through /mcp itself. + [Fact] + public async Task FarmSwitchOff_RefusesTheConnection_OnTheNextRequest() + { + using var host = Host(factory); + var user = await SeedAsync(factory, Roles.Manager); + var token = await IssueAsync(host, user, [OAuthScopes.ReadFarm]); + await AssertAcceptedAsync(host, token); + + var owner = factory.CreateAuthedClient(await factory.LoginForAccessTokenAsync(user.OwnerEmail)); + var version = (await owner.GetFromJsonAsync("/api/v1/account"))!["version"]!.GetValue(); + using var off = new HttpRequestMessage(HttpMethod.Put, "/api/v1/account/connected-apps") + { + Content = JsonContent.Create(new { allow = false, version }), + }; + off.Headers.Add("Idempotency-Key", Guid.NewGuid().ToString()); + Assert.Equal(HttpStatusCode.NoContent, (await owner.SendAsync(off)).StatusCode); + + await AssertRefusedAsync(host, token, "Auth.ConnectedAppsOff"); + } + + [Fact] + public async Task Disconnect_RefusesTheConnection_OnTheNextRequest() + { + using var host = Host(factory); + var user = await SeedAsync(factory, Roles.Manager); + var token = await IssueAsync(host, user, [OAuthScopes.ReadFarm]); + await AssertAcceptedAsync(host, token); + + await using (var scope = host.Services.CreateAsyncScope()) + { + var authorizations = scope.ServiceProvider.GetRequiredService(); + await foreach (var authorization in authorizations.FindBySubjectAsync(user.Id.ToString())) + Assert.True(await authorizations.TryRevokeAsync(authorization), "the authorization was not revoked"); + } + + using var response = await PostAsync(host, token, Initialize); + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + Assert.Contains(ResourceMetadata, response.Headers.WwwAuthenticate.ToString()); + } + + // Consent refuses a user with a pending password change (OAuthFailClosedTests), so no + // real token carries one; this one stands in to prove /mcp is not on the allowed list. + [Fact] + public async Task PendingPasswordChange_IsRefused() + { + using var host = Host(factory, alterIssued: principal => + { + principal.SetClaim("must_change_password", "true"); + principal.SetDestinations(static _ => [OpenIddictConstants.Destinations.AccessToken]); + }); + var token = await IssueAsync(host, await SeedAsync(factory, Roles.Manager), [OAuthScopes.ReadFarm]); + + using var response = await PostAsync(host, token, Initialize); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + Assert.Equal("Auth.MustChangePassword", + (await response.Content.ReadFromJsonAsync()).GetProperty("title").GetString()); + } + + // #806 — an MCP client reads the 429 as a JSON-RPC error, and the text describes the + // per-token bucket rather than an address. + [Fact] + public async Task OverTheBudget_Gets429_AsAJsonRpcError() + { + using var host = Host(factory, limits: ("OAuthApi", 1)); + var token = await IssueAsync(host, await SeedAsync(factory, Roles.Manager), [OAuthScopes.ReadFarm]); + await AssertAcceptedAsync(host, token); + + using var response = await PostAsync(host, token, Initialize); + + Assert.Equal(HttpStatusCode.TooManyRequests, response.StatusCode); + var body = await response.Content.ReadFromJsonAsync(); + var isRpcError = body.TryGetProperty("jsonrpc", out var version) && version.GetString() == "2.0" + && body.TryGetProperty("id", out var id) && id.ValueKind == JsonValueKind.Null + && body.TryGetProperty("error", out var error) && error.GetProperty("code").GetInt32() == -32000 + && error.GetProperty("message").GetString()!.Contains("this connection"); + Assert.True(isRpcError, $"the 429 body is not a JSON-RPC error about this connection: {body}"); + } + + private static async Task AssertAcceptedAsync(WebApplicationFactory host, string token) + { + using var response = await PostAsync(host, token, Initialize); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + private static async Task AssertRefusedAsync(WebApplicationFactory host, string token, string title) + { + using var response = await PostAsync(host, token, Initialize); + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + Assert.Equal(title, (await response.Content.ReadFromJsonAsync()).GetProperty("title").GetString()); + } +} diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpConnection.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpConnection.cs new file mode 100644 index 000000000..90e9a5adb --- /dev/null +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpConnection.cs @@ -0,0 +1,168 @@ +using System.ComponentModel; +using System.Net.Http.Headers; +using System.Net.Http.Json; +using System.Security.Claims; +using System.Security.Cryptography; +using System.Text; +using System.Text.Json; +using Cluckwork.Api.IntegrationTests.Infrastructure; +using Cluckwork.Api.Mcp; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.TestHost; +using Microsoft.AspNetCore.WebUtilities; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol.Client; +using ModelContextProtocol.Server; +using OpenIddict.Abstractions; +using OpenIddict.Server; +using static OpenIddict.Abstractions.OpenIddictConstants; + +namespace Cluckwork.Api.IntegrationTests.Mcp; + +// #806 — a connected app reaching /mcp through the real OAuth flow and the real MCP +// client. The probe tools exist only in this test host; the real tree has none yet. +internal static class McpConnection +{ + // The endpoint a client connects to is also the resource its token names (RFC 8707). + public const string McpUrl = "https://localhost/mcp"; + public const string ResourceMetadata = "https://localhost/.well-known/oauth-protected-resource/mcp"; + // Registered only in this test host, so a token can lack both MCP scopes or name + // another resource. + public const string OtherScope = "cw806.other"; + public const string OtherResource = "https://other.example/api"; + + public sealed record User(Guid Id, Guid AccountId, string Jwt, string OwnerEmail); + + // alterIssued changes every principal the authorization server signs, so a test can + // hold a token the real flow no longer mints. + public static WebApplicationFactory Host( + CluckworkWebApplicationFactory factory, Action? alterIssued = null, + params (string Policy, int PermitLimit)[] limits) => + factory.WithWebHostBuilder(builder => + { + foreach (var (policy, permitLimit) in limits) + { + builder.UseSetting($"RateLimiting:{policy}:PermitLimit", permitLimit.ToString()); + builder.UseSetting($"RateLimiting:{policy}:WindowSeconds", "86400"); + } + builder.ConfigureTestServices(services => + { + services.AddMcpServer().WithTools(); + services.Configure(options => + { + options.Scopes.Add(OtherScope); + options.Resources.Add(new Uri(OtherResource)); + }); + if (alterIssued is not null) + services.AddOpenIddict().AddServer(server => + server.AddEventHandler(handler => handler + .UseInlineHandler(context => + { + alterIssued(context.Principal!); + return default; + }) + .SetOrder(OpenIddictServerHandlers.InferResources.Descriptor.Order - 250))); + }); + }); + + public static async Task SeedAsync(CluckworkWebApplicationFactory factory, string? role) + { + var ownerEmail = $"mcp-owner-{Guid.NewGuid():N}@test.local"; + var email = $"mcp-user-{Guid.NewGuid():N}@test.local"; + var accountId = await factory.SeedAccountWithUserAsync(ownerEmail); + await factory.SeedUserAsync(accountId, email, role); + var id = await factory.WithTenantScopeAsync(accountId, db => + db.Users.Where(u => u.Email == email).Select(u => u.Id).SingleAsync()); + return new(id, accountId, await factory.LoginForAccessTokenAsync(email), ownerEmail); + } + + public static async Task IssueAsync( + WebApplicationFactory host, User user, string[] scopes, string? resource = null) + { + var clientId = await OAuthServerTests.RegisterClientAsync(host.Services); + var verifier = Base64Url(RandomNumberGenerator.GetBytes(32)); + var query = AuthorizeQuery(clientId, verifier, scopes, resource); + var location = await OAuthServerTests.ApproveAsync(host, user.Jwt, query); + var code = QueryHelpers.ParseQuery(location.Query)["code"].ToString(); + using var response = await RedeemAsync(host, clientId, code, verifier, query["redirect_uri"]!, resource); + response.EnsureSuccessStatusCode(); + return (await response.Content.ReadFromJsonAsync()).GetProperty("access_token").GetString()!; + } + + public static Dictionary AuthorizeQuery( + string clientId, string verifier, string[] scopes, string? resource) + { + var query = OAuthServerTests.AuthorizeQuery(clientId, verifier); + query["scope"] = string.Join(' ', scopes); + if (resource is not null) + query["resource"] = resource; + return query; + } + + public static Task RedeemAsync( + WebApplicationFactory host, string clientId, string code, string verifier, + string redirectUri, string? resource) + { + var form = new Dictionary + { + ["grant_type"] = GrantTypes.AuthorizationCode, + ["client_id"] = clientId, + ["code"] = code, + ["redirect_uri"] = redirectUri, + ["code_verifier"] = verifier, + }; + if (resource is not null) + form["resource"] = resource; + return OAuthServerTests.HttpsClient(host, bearer: null).PostAsync("/api/v1/oauth/token", new FormUrlEncodedContent(form)); + } + + public static Task ConnectAsync(WebApplicationFactory host, string token, string? protocolVersion = null) + { + var http = host.CreateDefaultClient(new Uri("https://localhost")); + http.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token); + var transport = new HttpClientTransport( + new HttpClientTransportOptions { Endpoint = new Uri(McpUrl), TransportMode = HttpTransportMode.StreamableHttp }, + http, ownsHttpClient: true); + return McpClient.CreateAsync(transport, new McpClientOptions { ProtocolVersion = protocolVersion }); + } + + // One raw JSON-RPC message, as a client sends it, so a refusal before the SDK is visible. + public static Task PostAsync(WebApplicationFactory host, string? bearer, string json) + { + var request = new HttpRequestMessage(HttpMethod.Post, "/mcp") + { + Content = new StringContent(json, Encoding.UTF8, "application/json"), + }; + request.Headers.Accept.ParseAdd("application/json"); + request.Headers.Accept.ParseAdd("text/event-stream"); + return OAuthServerTests.HttpsClient(host, bearer).SendAsync(request); + } + + public const string Initialize = + """{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"cw806","version":"1"}}}"""; + + private static string Base64Url(byte[] bytes) => + Convert.ToBase64String(bytes).TrimEnd('=').Replace('+', '-').Replace('/', '_'); + + // Shaped like a real tool: it learns its caller from McpCallContext and carries a role + // policy beside a scope policy. + [McpServerToolType] + public sealed class ProbeTools(McpCallContext call) + { + public const string Read = "cw806_read"; + public const string Write = "cw806_write"; + + [McpServerTool(Name = Read, ReadOnly = true), Description("Who is calling.")] + [Authorize] + [Authorize(Policy = AuthPolicies.FarmReadScope)] + public string WhoIsCalling() => $"{call.AccountId}/{call.UserId}/{call.ConnectedApp.ClientId}"; + + [McpServerTool(Name = Write), Description("A write a Worker may make.")] + [Authorize(Policy = AuthPolicies.ProductionWrite)] + [Authorize(Policy = AuthPolicies.DailyEntriesWriteScope)] + public string Record() => call.UserId.ToString(); + } +} diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs new file mode 100644 index 000000000..c835ac814 --- /dev/null +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs @@ -0,0 +1,113 @@ +using System.Net; +using Cluckwork.Api.Hosting; +using Cluckwork.Api.IntegrationTests.Infrastructure; +using Cluckwork.Api.Middleware; +using Cluckwork.Api.Modules.Access.OAuth; +using Cluckwork.Api.RateLimiting; +using Cluckwork.Application.Modules.Access.Contracts; +using Cluckwork.Domain.Modules.Farm.Contracts; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.RateLimiting; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Options; +using ModelContextProtocol.AspNetCore; +using static Cluckwork.Api.IntegrationTests.Mcp.McpConnection; + +namespace Cluckwork.Api.IntegrationTests.Mcp; + +// #806 guards rows 8-12, 17, 18, 29 and 30 (docs/plans/770-mcp-server/02-guards.md), +// read from the running application's endpoint table and services. +[Collection(IntegrationCollection.Name)] +public sealed class McpEndpointTests(CluckworkWebApplicationFactory factory) +{ + // Rows 10, 29, 30: the body marker BodyReadingEndpointTests demands, the #309 cap that + // marker does not set, and the per-token budget and scope gate AcceptOAuthTokens adds. + [Fact] + public void McpPost_CarriesTheBodyCap_TheBudget_AndTheOAuthGate() + { + var mcp = McpPost(factory); + + Assert.Equal(CluckworkMcp.MaxRequestBodyBytes, mcp.Metadata.GetMetadata()?.Bytes); + Assert.Equal(RateLimitingOptions.OAuthApiPolicyName, mcp.Metadata.GetMetadata()?.PolicyName); + Assert.True(OAuthEndpoints.AcceptsOAuthTokens(mcp), "/mcp does not accept OAuth tokens"); + Assert.NotNull(mcp.Metadata.GetMetadata()); + } + + // Row 17: the exemption is endpoint metadata, and exactly one endpoint carries it. + [Fact] + public void IdempotencyExemption_IsCarriedByTheMcpPostAlone() + { + var endpoints = factory.Services.GetRequiredService().Endpoints.OfType().ToList(); + + Assert.True(endpoints.Count > 100, $"walked only {endpoints.Count} endpoints"); + var exempt = Assert.Single(endpoints, e => e.Metadata.GetMetadata() is not null); + Assert.Same(McpPost(factory), exempt); + } + + // Row 18: no MCP client sends an Idempotency-Key, and every MCP message is a POST. + [Fact] + public async Task McpMessages_WithoutAnIdempotencyKey_AreServed() + { + using var host = Host(factory); + var token = await IssueAsync(host, await SeedAsync(factory, Roles.Manager), [OAuthScopes.ReadFarm]); + + using var initialize = await PostAsync(host, token, Initialize); + using var list = await PostAsync(host, token, """{"jsonrpc":"2.0","id":2,"method":"tools/list"}"""); + + Assert.Equal(HttpStatusCode.OK, initialize.StatusCode); + Assert.Equal(HttpStatusCode.OK, list.StatusCode); + } + + // Rows 8 and 9. Under Stateless the SDK runs a tool in the request's own services; + // ScopeRequests is deliberately not pinned, because the SDK forces it there. + [Fact] + public void Transport_IsStateless_WithNoSessionHook() + { + var transport = factory.Services.GetRequiredService>().Value; + + Assert.Equal(HttpServerSessionMode.Stateless, transport.SessionMode); + Assert.Null(transport.ConfigureSessionOptions); + } + + // Row 12: WithHttpTransport registers the idle-session sweep unconditionally. Under + // Stateless it never starts, so #271's blocker list does not grow. Inertness, not absence. + [Fact] + public void IdleSessionSweep_IsRegistered_ButNeverStarts() + { + _ = factory.Server; + var sweep = Assert.Single(factory.Services.GetServices().OfType(), + s => s.GetType().Name == "IdleTrackingBackgroundService"); + + Assert.Null(sweep.ExecuteTask); + } + + // Row 29: the cap refuses a declared-oversize body before authentication. + [Fact] + public async Task OversizedBody_IsRefused() + { + using var response = await PostAsync(factory, bearer: null, new string(' ', (int)CluckworkMcp.MaxRequestBodyBytes + 1)); + + Assert.Equal(HttpStatusCode.RequestEntityTooLarge, response.StatusCode); + } + + // OAuth tokens are the only way in, so without the server that issues them there is no /mcp. + [Fact] + public void WithoutAnIssuer_McpIsNotMapped() + { + using var host = factory.WithWebHostBuilder(builder => builder.UseSetting("OAuth:Issuer", "")); + + var routes = host.Services.GetRequiredService().Endpoints.OfType() + .Select(e => e.RoutePattern.RawText).ToList(); + + Assert.Contains(routes, r => r?.StartsWith("/api/v1/flocks", StringComparison.Ordinal) == true); + Assert.DoesNotContain(routes, r => r?.TrimEnd('/') == CluckworkMcp.Path); + } + + private static RouteEndpoint McpPost(WebApplicationFactory host) => + Assert.Single(host.Services.GetRequiredService().Endpoints.OfType(), + e => e.RoutePattern.RawText?.TrimEnd('/') == CluckworkMcp.Path + && e.Metadata.GetMetadata()?.HttpMethods.Contains(HttpMethods.Post) == true); +} diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.cs index 3954263c8..314f9cc09 100644 --- a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.cs +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.cs @@ -1,6 +1,7 @@ using System.Reflection; using Cluckwork.Api.Mcp; using Cluckwork.Infrastructure.Persistence; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; @@ -8,8 +9,8 @@ namespace Cluckwork.Api.IntegrationTests.Mcp; // #805 rows 6 and 7 — what a tool may inject. A tool is a type carrying an attribute // NAMED McpServerToolTypeAttribute, and a tool method one carrying McpServerToolAttribute. -// Matched by name because the SDK arrives with #806; AdapterTierScanner matches the -// same name, and the fixtures here declare look-alikes to prove the walk goes red. +// Matched by name, as AdapterTierScanner matches it, so the fixtures here can declare +// look-alikes that prove the walk goes red. internal static class McpToolSurface { // A supertype is banned too: ICurrentUser is CurrentUserContext's port and DbContext @@ -58,6 +59,31 @@ public static IReadOnlyList Findings(IEnumerable types) return findings; } + // #806 — the SDK authorizes a tool only from attributes on it or its type, and runs + // one that carries none, or carries [AllowAnonymous], for any caller. So every tool + // carries a role policy and a scope policy, which AND together into role ∩ scope. + public static readonly string[] ScopePolicies = [AuthPolicies.FarmReadScope, AuthPolicies.DailyEntriesWriteScope]; + + public static IReadOnlyList GateFindings(IEnumerable types) + { + var findings = new List(); + foreach (var tool in ToolTypes(types)) + foreach (var method in ToolMethods(tool)) + { + var attributes = method.GetCustomAttributes(inherit: true).Concat(tool.GetCustomAttributes(inherit: true)).ToList(); + var site = $"{tool.FullName}.{method.Name}"; + if (attributes.OfType().Any()) + findings.Add($"{site} allows anonymous callers, so the SDK skips every policy on it"); + var gates = attributes.OfType().ToList(); + if (!gates.Any(g => ScopePolicies.Contains(g.Policy))) + findings.Add($"{site} has no scope policy; add [Authorize(Policy = AuthPolicies.Scope)]"); + if (!gates.Any(g => !ScopePolicies.Contains(g.Policy))) + findings.Add($"{site} has no role policy; add [Authorize] or [Authorize(Policy = AuthPolicies.)]"); + } + + return findings; + } + // #805 row 7b, the namespace half: no type under the MCP namespace declares a scope // opener, as a constructor or method parameter, return type, field or property. A // lambda taking IServiceProvider compiles to a method on a nested type, so it counts. diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurfaceTests.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurfaceTests.cs index 46aa3e076..e46aa72c8 100644 --- a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurfaceTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurfaceTests.cs @@ -2,6 +2,7 @@ using Cluckwork.Api.Mcp; using Cluckwork.Application.Common; using Cluckwork.Infrastructure.Persistence; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; @@ -18,6 +19,10 @@ public sealed class McpToolSurfaceTests public void RealTree_EveryToolTakesMcpCallContext_AndInjectsNoAmbientIdentity() => AssertNone(McpToolSurface.Findings(ApiTypes)); + [Fact] + public void RealTree_EveryToolCarriesARolePolicyAndAScopePolicy() => + AssertNone(McpToolSurface.GateFindings(ApiTypes)); + [Fact] public void RealTree_NoTypeUnderTheMcpNamespace_DeclaresAScopeOpener() { @@ -63,6 +68,17 @@ public void ToolInjectingAmbientIdentityOrAServiceLocator_IsAFinding(Type inject } } + [Fact] + public void GatedTools_HaveNoGateFindings() => + Assert.Empty(McpToolSurface.GateFindings([typeof(GatedTool), typeof(TypeGatedTool)])); + + [Theory] + [InlineData(typeof(RoleOnlyTool), "has no scope policy")] + [InlineData(typeof(ScopeOnlyTool), "has no role policy")] + [InlineData(typeof(AnonymousTool), "allows anonymous callers")] + public void ToolMissingAGate_IsAGateFinding(Type tool, string finding) => + Assert.Contains(McpToolSurface.GateFindings([tool]), f => f.Contains(finding)); + [Fact] public void ScopeFactoryHelper_IsADeclarationFinding() { @@ -135,6 +151,41 @@ private sealed class MethodInjects(McpCallContext call) public string Read(T injected) => $"{call.UserId} {injected}"; } + [McpServerToolType] + private sealed class GatedTool(McpCallContext call) + { + [McpServerTool, Authorize, Authorize(Policy = AuthPolicies.FarmReadScope)] + public string Read() => call.Email; + } + + [McpServerToolType, Authorize(Policy = AuthPolicies.ProductionWrite), Authorize(Policy = AuthPolicies.DailyEntriesWriteScope)] + private sealed class TypeGatedTool(McpCallContext call) + { + [McpServerTool] + public string Write() => call.Email; + } + + [McpServerToolType] + private sealed class RoleOnlyTool(McpCallContext call) + { + [McpServerTool, Authorize(Policy = AuthPolicies.AdminOnly)] + public string Read() => call.Email; + } + + [McpServerToolType] + private sealed class ScopeOnlyTool(McpCallContext call) + { + [McpServerTool, Authorize(Policy = AuthPolicies.FarmReadScope)] + public string Read() => call.Email; + } + + [McpServerToolType, Authorize, Authorize(Policy = AuthPolicies.FarmReadScope)] + private sealed class AnonymousTool(McpCallContext call) + { + [McpServerTool, AllowAnonymous] + public string Read() => call.Email; + } + private sealed class ScopeFactoryHelper(IServiceScopeFactory scopes) { public IServiceScope Open() => scopes.CreateScope(); diff --git a/tests/Cluckwork.Api.IntegrationTests/packages.lock.json b/tests/Cluckwork.Api.IntegrationTests/packages.lock.json index c7998e509..36ca70a39 100644 --- a/tests/Cluckwork.Api.IntegrationTests/packages.lock.json +++ b/tests/Cluckwork.Api.IntegrationTests/packages.lock.json @@ -187,6 +187,11 @@ "Microsoft.EntityFrameworkCore": "10.0.12" } }, + "Microsoft.Extensions.AI.Abstractions": { + "type": "Transitive", + "resolved": "10.8.3", + "contentHash": "K0B05oApxmviWalNHPMBBcRC7erKiDATz3ENNR/jqTR9JwIwLRefgDhj2jCRwL1aca99pXUe0qyQC73/xIuZig==" + }, "Microsoft.Extensions.DependencyModel": { "type": "Transitive", "resolved": "10.0.12", @@ -252,6 +257,22 @@ "Microsoft.TestPlatform.ObjectModel": "18.10.1" } }, + "ModelContextProtocol": { + "type": "Transitive", + "resolved": "2.2.0", + "contentHash": "4Pb9u02Nwsp0poueDsqNdyGRojFxOYpljB7zDBsq+aHL+Afou3OgxlBc3GWFVnsRMRJrUtWqDh3s6k2JgPzmrQ==", + "dependencies": { + "ModelContextProtocol.Core": "[2.2.0]" + } + }, + "ModelContextProtocol.Core": { + "type": "Transitive", + "resolved": "2.2.0", + "contentHash": "FeBfXU6T8k+jw4afg4sfxdEX2rL/e5oKOk9ROOGztu9k47+7Bz08sdaToYt2XvMY1opNbwxYQOFMj6wH9TInhA==", + "dependencies": { + "Microsoft.Extensions.AI.Abstractions": "10.8.3" + } + }, "Npgsql": { "type": "Transitive", "resolved": "10.0.3", @@ -449,6 +470,7 @@ "Microsoft.AspNetCore.Authentication.JwtBearer": "[10.*, )", "Microsoft.AspNetCore.OpenApi": "[10.*, )", "Microsoft.OpenApi": "[2.12.2, )", + "ModelContextProtocol.AspNetCore": "[2.2.0, )", "OpenTelemetry.Exporter.OpenTelemetryProtocol": "[1.*, )", "OpenTelemetry.Extensions.Hosting": "[1.*, )", "OpenTelemetry.Instrumentation.AspNetCore": "[1.*, )", @@ -548,6 +570,15 @@ "resolved": "2.12.2", "contentHash": "ZkT7qiJe0Kj0+DTZOZo+xrDy1kNmdsWFQVPIQLNx9ZJ0T9cB2LG7r+Bi60so8/+yg1nkS7QCPadrzsTG4VPKrA==" }, + "ModelContextProtocol.AspNetCore": { + "type": "CentralTransitive", + "requested": "[2.2.0, )", + "resolved": "2.2.0", + "contentHash": "3JelDMuFIwFzXybsh6K30G6wXu5gmqKnuRnieqgBMuR0FkqqXRv4B+NYZuPgthSyNq5UwRyvv2U2VJBE3RD3PQ==", + "dependencies": { + "ModelContextProtocol": "[2.2.0]" + } + }, "Npgsql.EntityFrameworkCore.PostgreSQL": { "type": "CentralTransitive", "requested": "[10.*, )", diff --git a/tests/Cluckwork.Application.Tests/Architecture/AdapterReachTests.cs b/tests/Cluckwork.Application.Tests/Architecture/AdapterReachTests.cs index 341420ba8..063a662a9 100644 --- a/tests/Cluckwork.Application.Tests/Architecture/AdapterReachTests.cs +++ b/tests/Cluckwork.Application.Tests/Architecture/AdapterReachTests.cs @@ -39,7 +39,7 @@ private AdapterReachReport Scan(IReadOnlyList? adapters = null, AdapterReachScanner.Scan(Path.Combine(_tempRoot, "src"), Ledger(adapters, adapterTiers, farmContract)); private static AdapterTier Tier(string ns = "Cluckwork.Temp.Mcp") => - new(ns, "DirectRepository", "MapMcp", "test", "#1"); + new(ns, "ContractOnly", "MapMcp", "test", "#1"); private static AdapterClaim Row(string symbol = Symbol, IReadOnlyList? reaches = null) => new(symbol, reaches ?? ["Farm"]); diff --git a/tests/Cluckwork.Application.Tests/Architecture/AdapterTierRealTreeTests.cs b/tests/Cluckwork.Application.Tests/Architecture/AdapterTierRealTreeTests.cs index cdfad64b9..64cd9b7e8 100644 --- a/tests/Cluckwork.Application.Tests/Architecture/AdapterTierRealTreeTests.cs +++ b/tests/Cluckwork.Application.Tests/Architecture/AdapterTierRealTreeTests.cs @@ -22,15 +22,4 @@ public void McpTierRow_IsDeclared() { Assert.Contains(RealModuleLedger.Value.AdapterTiers, t => t.Namespace == "Cluckwork.Api.Mcp" && t.Surface == "MapMcp"); } - - // /mcp is not mapped yet (#806 has not landed). This pins today's green: the - // row is committed but its namespace holds no tool type and MapMcp is not - // invoked. It goes stale the day #806 maps /mcp under this namespace — that - // author drops this assertion, not works around it. - [Fact] - public void McpTierRow_IsDormantToday() - { - var report = Scan(); - Assert.Contains(report.Dormant, t => t.Namespace == "Cluckwork.Api.Mcp"); - } } diff --git a/tests/Cluckwork.Application.Tests/Architecture/AdapterTierTests.cs b/tests/Cluckwork.Application.Tests/Architecture/AdapterTierTests.cs index 7d2d8faf9..69b8c45e6 100644 --- a/tests/Cluckwork.Application.Tests/Architecture/AdapterTierTests.cs +++ b/tests/Cluckwork.Application.Tests/Architecture/AdapterTierTests.cs @@ -20,7 +20,7 @@ private AdapterTierReport Scan(params AdapterTier[] tiers) new ModuleLedger([new("Hub", "platform", ["Cluckwork.Temp"], [])], [], []) { AdapterTiers = tiers })); } - private static AdapterTier Tier(string ns = "Cluckwork.Temp.Mcp", string privilege = "DirectRepository", + private static AdapterTier Tier(string ns = "Cluckwork.Temp.Mcp", string privilege = "ContractOnly", string surface = "MapMcp", string reason = "test reason", string reviewBy = "#1") => new(ns, privilege, surface, reason, reviewBy); @@ -144,7 +144,7 @@ public void SurfaceCallWithNoTierRow_IsSurfaceWithoutTier() Assert.Contains("MapMcp", failure); Assert.Contains("src/Program.cs:1", failure); Assert.EndsWith("add the row to PlatformModuleRules in src/Cluckwork.Domain/Common/Architecture/Modules/Platform.cs:\n" + - "[AdapterTier(\"\", \"DirectRepository\", " + + "[AdapterTier(\"\", \"ContractOnly\", " + "\"MapMcp\", \"\", \"<#issue>\")]", failure); } @@ -308,7 +308,7 @@ public void PrivilegeDoesNotMatchSurfaceMapping_IsRegistryError() Assert.Contains(Scan(row).RegistryErrors, e => e.Contains("privilege", StringComparison.Ordinal) && e.Contains("MapMcp", StringComparison.Ordinal) - && e.Contains("DirectRepository", StringComparison.Ordinal)); + && e.Contains("ContractOnly", StringComparison.Ordinal)); } [Fact] diff --git a/tests/Cluckwork.Application.Tests/Architecture/ModuleLedger.cs b/tests/Cluckwork.Application.Tests/Architecture/ModuleLedger.cs index c2113cfb4..c68dd985c 100644 --- a/tests/Cluckwork.Application.Tests/Architecture/ModuleLedger.cs +++ b/tests/Cluckwork.Application.Tests/Architecture/ModuleLedger.cs @@ -36,7 +36,7 @@ public sealed record AdapterClaim(string Symbol, IReadOnlyList Reaches); public sealed record AdapterTier(string Namespace, string Privilege, string Surface, string Reason, string ReviewBy) { - public const string DirectRepositoryPrivilege = "DirectRepository"; + public const string ContractOnlyPrivilege = "ContractOnly"; // The closed set of surfaces a tier row may name, each mapped to the // privilege it grants. AdapterTierScanner reads this same map, so a surface @@ -44,7 +44,7 @@ public sealed record AdapterTier(string Namespace, string Privilege, string Surf public static readonly IReadOnlyDictionary KnownSurfaces = new Dictionary(StringComparer.Ordinal) { - ["MapMcp"] = DirectRepositoryPrivilege, + ["MapMcp"] = ContractOnlyPrivilege, }; } diff --git a/tests/Cluckwork.Application.Tests/TenantBypass/BypassAllowList.cs b/tests/Cluckwork.Application.Tests/TenantBypass/BypassAllowList.cs index 067c64103..1c2485dbe 100644 --- a/tests/Cluckwork.Application.Tests/TenantBypass/BypassAllowList.cs +++ b/tests/Cluckwork.Application.Tests/TenantBypass/BypassAllowList.cs @@ -266,7 +266,7 @@ internal static class BypassAllowList { Symbol = "Cluckwork.Api.Middleware.IdempotencyMiddleware.InvokeAsync(HttpContext context, AppDbContext db, TenantContext tenant, CurrentUserContext user)", File = "src/Cluckwork.Api/Middleware/IdempotencyMiddleware.cs", - Hash = "81a2888e", + Hash = "4c659505", Justification = "Raw-SQL UPDATE of idempotency_records on successful completion; the statement carries WHERE \"AccountId\" = {accountId} (the claim was scoped to this tenant on insert). Not a row lock, so the predicate walk does not flag it — allow-listed as a raw-SQL occurrence.", }, new() diff --git a/tools/oauth/mutation-check.sh b/tools/oauth/mutation-check.sh index 3baac795e..3aba17934 100755 --- a/tools/oauth/mutation-check.sh +++ b/tools/oauth/mutation-check.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # -# tools/oauth/mutation-check.sh — proves each #795 to #799 and #1148 OAuth claim has a test that -# fails when the claim breaks. +# tools/oauth/mutation-check.sh — proves each #795 to #799, #806 and #1148 OAuth claim has a +# test that fails when the claim breaks. # # Baseline green, then per mutant: apply one exact-string edit (it must match once), # rebuild, run the ONE named test with a TRX logger, and classify from the TRX, never @@ -49,10 +49,13 @@ ACCOUNT=src/Cluckwork.Domain/Modules/Farm/Accounts/Account.cs ACCOUNT_ENDPOINTS=src/Cluckwork.Api/Modules/Farm/Accounts/AccountEndpoints.cs SWITCH_HANDLER=src/Cluckwork.Application/Modules/Farm/Accounts/SetConnectedApps/SetConnectedAppsHandler.cs SWITCH_MIGRATION=src/Cluckwork.Infrastructure/Persistence/Migrations/20261009193717_AddAccountAllowConnectedApps.cs +MCP=src/Cluckwork.Api/Hosting/CluckworkMcp.cs +FORBIDDEN=src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs +POLICIES=src/Cluckwork.Api/AuthPolicies.cs TESTS=tests/Cluckwork.Api.IntegrationTests TEST_NS=Cluckwork.Api.IntegrationTests -SUITE='FullyQualifiedName~OAuth|FullyQualifiedName~ClientMetadata' -SUITE_MIN=272 +SUITE='FullyQualifiedName~OAuth|FullyQualifiedName~ClientMetadata|FullyQualifiedName~Cluckwork.Api.IntegrationTests.Mcp.' +SUITE_MIN=340 # name # expect # file # find # replace # test # declared failure text # ('#' because C# anchors contain '|'; '\n' in a find or replace is a newline) @@ -92,15 +95,15 @@ verifier-logged#kill#TELEMETRY# .Filter.ByExcluding(OpenIddic parent-override-only#kill#TELEMETRY# .Filter.ByExcluding(OpenIddictBelowWarning),# .MinimumLevel.Override("OpenIddict", LogEventLevel.Warning),#OAuthServerTests.ProtocolSecrets_NeverReachTheLog#protocol secrets reached the log oauth-on-default-scheme#kill#IDENTITY# OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint())\n# OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint()) || context.Request.Headers.Authorization.ToString().Count(ch => ch == '.') != 2\n#OAuthServerTests.OAuthToken_IsRejectedByBusinessEndpoints_AtAuthentication#Expected: Unauthorized jwt-by-token-shape#kill#IDENTITY# OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint())\n# OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint()) && context.Request.Headers.Authorization.ToString().Count(ch => ch == '.') != 2\n#OAuthFailClosedTests.SessionJwt_IsRefused_WhereOAuthTokensAreAccepted#Expected: Unauthorized -authenticate-at-authorization#kill#ENDPOINT# .WithMetadata(new AcceptsOAuthTokensMarker())\n .RequireRateLimiting(RateLimitingOptions.OAuthApiPolicyName)\n .RequireAuthorization(policy => policy.RequireAssertion(# .RequireRateLimiting(RateLimitingOptions.OAuthApiPolicyName)\n .RequireAuthorization(policy => policy.AddAuthenticationSchemes("OpenIddict.Validation.AspNetCore").RequireAssertion(#OAuthFailClosedTests.DisabledUser_IsRefused_OnTheNextRequest#Expected: Unauthorized -authenticate-at-authorization-flocks#kill#ENDPOINT# .WithMetadata(new AcceptsOAuthTokensMarker())\n .RequireRateLimiting(RateLimitingOptions.OAuthApiPolicyName)\n .RequireAuthorization(policy => policy.RequireAssertion(# .RequireRateLimiting(RateLimitingOptions.OAuthApiPolicyName)\n .RequireAuthorization(policy => policy.AddAuthenticationSchemes("OpenIddict.Validation.AspNetCore").RequireAssertion(#OAuthFailClosedTests.Worker_IsFlockScoped#the worker's OAuth caller is unrestricted +authenticate-at-authorization#kill#ENDPOINT# .WithMetadata(new AcceptsOAuthTokensMarker())\n .RequireRateLimiting(RateLimitingOptions.OAuthApiPolicyName)\n .RequireAuthorization(policy => policy.AddRequirements(# .RequireRateLimiting(RateLimitingOptions.OAuthApiPolicyName)\n .RequireAuthorization(policy => policy.AddAuthenticationSchemes("OpenIddict.Validation.AspNetCore").AddRequirements(#OAuthFailClosedTests.DisabledUser_IsRefused_OnTheNextRequest#Expected: Unauthorized +authenticate-at-authorization-flocks#kill#ENDPOINT# .WithMetadata(new AcceptsOAuthTokensMarker())\n .RequireRateLimiting(RateLimitingOptions.OAuthApiPolicyName)\n .RequireAuthorization(policy => policy.AddRequirements(# .RequireRateLimiting(RateLimitingOptions.OAuthApiPolicyName)\n .RequireAuthorization(policy => policy.AddAuthenticationSchemes("OpenIddict.Validation.AspNetCore").AddRequirements(#OAuthFailClosedTests.Worker_IsFlockScoped#the worker's OAuth caller is unrestricted role-claim-dropped#kill#ENDPOINT#"credential_epoch", Claims.Role, #"credential_epoch", #OAuthFailClosedTests.OAuthToken_CarriesTheSessionPrincipal_ThroughTheWholeChain#Collections differ disabled-unchecked#kill#VERIFIER# if (credentialState.DisabledAt is not null)\n return CredentialVerdict.Disabled;\n##OAuthFailClosedTests.DisabledUser_IsRefused_OnTheNextRequest#Expected: Unauthorized suspended-unchecked#kill#VERIFIER# if (credentialState.AccountIsActive != true)\n return CredentialVerdict.FarmSuspended;\n##OAuthFailClosedTests.SuspendedFarm_IsRefused_OnTheNextRequest#Expected: Unauthorized epoch-exempts-oauth#kill#EPOCH# && !IsLogoutPath(context.Request.Path))# && !IsLogoutPath(context.Request.Path) && context.User.FindFirst("oi_au_id") is null)#OAuthFailClosedTests.RoleChange_RevokesTheToken#Expected: Unauthorized epoch-exempts-oauth-suspended#kill#EPOCH# && !IsLogoutPath(context.Request.Path))# && !IsLogoutPath(context.Request.Path) && context.User.FindFirst("oi_au_id") is null)#OAuthFailClosedTests.SuspendedFarm_IsRefused_OnTheNextRequest#Expected: Unauthorized must-change-may-authorize#kill#MUST_CHANGE# "/api/v1/auth/logout",\n# "/api/v1/auth/logout",\n "/api/v1/oauth/authorize",\n#OAuthFailClosedTests.MustChangePassword_BlocksIssuance#Expected: Forbidden -scope-gate-removed#kill#ENDPOINT#\n .RequireAuthorization(policy => policy.RequireAssertion(context =>\n scopes.Any(context.User.HasScope)));#;#OAuthFailClosedTests.ScopeAndRole_AreBothRequired#Expected: Forbidden +scope-gate-removed#kill#ENDPOINT#\n .RequireAuthorization(policy => policy.AddRequirements(new OAuthScopeRequirement(scopes)));#;#OAuthFailClosedTests.ScopeAndRole_AreBothRequired#Expected: Forbidden authorization-unchecked#kill#SERVER# validation.EnableAuthorizationEntryValidation();\n##OAuthFailClosedTests.Disconnect_RefusesTheAccessToken_OnTheNextRequest#Expected: Unauthorized unbound-token-trusted#kill#SERVER# if (string.IsNullOrEmpty(context.AuthorizationId))\n context.Reject(Errors.InvalidToken, "The token is not bound to an authorization.");\n##OAuthFailClosedTests.TokenWithoutAnAuthorization_IsRefused#Expected: Unauthorized refresh-grant-allowed#kill#SERVER# .AllowAuthorizationCodeFlow()# .AllowAuthorizationCodeFlow().AllowRefreshTokenFlow()#OAuthFailClosedTests.Disconnect_LeavesNoWayToANewToken#unsupported_grant_type @@ -221,6 +224,26 @@ cimd-response-type-fetched#kill#DOCUMENTS#if (context.Request.ResponseType != Re cimd-pkce-fetched#kill#DOCUMENTS#if (string.IsNullOrEmpty(context.Request.CodeChallenge) || context.Request.CodeChallengeMethod != CodeChallengeMethods.Sha256)#if (context.Request.CodeChallengeMethod == "unchecked")#OAuthFailClosedTests.MalformedRequest_IsRefused_BeforeAnyFetch#Assert.Empty() Failure cimd-approved-shares-strangers-budget#kill#DOCUMENTS#approved: await IsApprovedAsync(scope, applications, row, ct)#approved: false#OAuthFailClosedTests.ApprovedApp_IsRefreshed_PastADrainedGlobalBudget#an approved app was refused cimd-everyone-gets-approved-budget#kill#DOCUMENTS#approved: await IsApprovedAsync(scope, applications, row, ct)#approved: true#OAuthFailClosedTests.ApprovedApp_IsRefreshed_PastADrainedGlobalBudget#an approved app was refused +mcp-stateful#kill#MCP#options.SessionMode = HttpServerSessionMode.Stateless#options.SessionMode = HttpServerSessionMode.Stateful#Mcp.McpEndpointTests.Transport_IsStateless_WithNoSessionHook#Stateful +mcp-stateful-sweep-runs#kill#MCP#options.SessionMode = HttpServerSessionMode.Stateless#options.SessionMode = HttpServerSessionMode.Stateful#Mcp.McpEndpointTests.IdleSessionSweep_IsRegistered_ButNeverStarts#Assert.Null() Failure +mcp-session-hook#kill#MCP#options => options.SessionMode = HttpServerSessionMode.Stateless)#options => { options.SessionMode = HttpServerSessionMode.Stateless; options.ConfigureSessionOptions = (_, _, _) => Task.CompletedTask; })#Mcp.McpEndpointTests.Transport_IsStateless_WithNoSessionHook#Assert.Null() Failure +mcp-body-unclassified#kill#MCP#new ReadsRequestBodyAttribute(), ##BodyReadingEndpointTests.Every_endpoint_that_can_read_the_body_is_classified#POST /mcp +mcp-body-uncapped#kill#MCP#\n .WithMaxRequestBodyBytes(MaxRequestBodyBytes)##Mcp.McpEndpointTests.OversizedBody_IsRefused#Expected: RequestEntityTooLarge +mcp-idempotency-unmarked#kill#MCP#, new HandlesOwnIdempotencyAttribute()##Mcp.McpEndpointTests.McpMessages_WithoutAnIdempotencyKey_AreServed#Expected: OK +mcp-idempotency-second-bearer#kill#ENDPOINT#.WithMetadata(new IgnoresAmbientPrincipalAttribute(), new ReadsRequestBodyAttribute())#.WithMetadata(new IgnoresAmbientPrincipalAttribute(), new ReadsRequestBodyAttribute(), new HandlesOwnIdempotencyAttribute())#Mcp.McpEndpointTests.IdempotencyExemption_IsCarriedByTheMcpPostAlone#Assert.Single() Failure +mcp-idempotency-exemption-ignored#kill#IDEMPOTENCY#GetMetadata() is not null)#GetMetadata() is null && false)#Mcp.McpEndpointTests.McpMessages_WithoutAnIdempotencyKey_AreServed#Expected: OK +mcp-tools-list-unfiltered#kill#MCP#\n .AddAuthorizationFilters()##Mcp.McpAuthorizationTests.ToolsList_ShowsOnlyToolsTheRoleAndScopesAllow# +mcp-read-scope-policy-wrong#kill#POLICIES#new OAuthScopeRequirement(OAuthScopes.ReadFarm)#new OAuthScopeRequirement(OAuthScopes.WriteDailyEntries)#Mcp.McpAuthorizationTests.ToolsList_ShowsOnlyToolsTheRoleAndScopesAllow#Collections differ +mcp-mapped-without-issuer#kill#PROGRAM# .MapOAuthEndpoints();\n app.MapCluckworkMcp();\n}# .MapOAuthEndpoints();\n}\napp.MapCluckworkMcp();#Mcp.McpEndpointTests.WithoutAnIssuer_McpIsNotMapped#WithHttpTransport +audience-unchecked#kill#SERVER#\n validation.AddAudiences(resource);##Mcp.McpAuthorizationTests.TokenIssuedForAnotherResource_IsRefused#Expected: Unauthorized +audience-unchecked-unbound#kill#SERVER#\n validation.AddAudiences(resource);##Mcp.McpAuthorizationTests.TokenWithoutTheMcpAudience_IsRefused#Expected: Unauthorized +resource-not-stamped#kill#SERVER#context.Principal!.SetResources(context.Request!.GetResources() is { IsEmpty: false } asked\n ? asked\n : [resource]);#_ = resource;#Mcp.McpAuthorizationTests.TokenForMcp_IsAccepted#Expected: OK +default-resource-dropped#kill#SERVER# : [resource]);# : []);#Mcp.McpAuthorizationTests.TokenForMcp_IsAccepted#Expected: OK +resource-unregistered#kill#SERVER#\n .RegisterResources(resource)##Mcp.McpAuthorizationTests.TokenForMcp_IsAccepted#Expected: OK +challenge-without-metadata#kill#IDENTITY#? McpAuthenticationDefaults.AuthenticationScheme#? "OpenIddict.Validation.AspNetCore"#Mcp.McpAuthorizationTests.NoToken_Gets401_NamingTheProtectedResourceMetadata#Strings differ +metadata-from-request-host#kill#MCP#\n options.ResourceMetadataUri = new Uri(resource, "/.well-known/oauth-protected-resource" + resource.AbsolutePath);##Mcp.McpAuthorizationTests.Challenge_NamesTheIssuer_NotTheRequestHost#Sub-string not found +insufficient-scope-unannounced#kill#FORBIDDEN#var onlyScopesFailed = failed.Count > 0 && failed.All(r => r is OAuthScopeRequirement);#var onlyScopesFailed = false;#Mcp.McpAuthorizationTests.TokenWithoutAnMcpScope_Gets403InsufficientScope#Strings differ +rpc-429-as-problem#kill#LIMITS#if (policyName == RateLimitingOptions.OAuthApiPolicyName)\n {\n context.HttpContext.Response.StatusCode#if (policyName == "none")\n {\n context.HttpContext.Response.StatusCode#Mcp.McpAuthorizationTests.OverTheBudget_Gets429_AsAJsonRpcError#not a JSON-RPC error EOF ) @@ -315,7 +338,7 @@ sys.exit(1 if problems else 0) PY } -FILES=("$IDENTITY" "$TELEMETRY" "$SERVER" "$ENDPOINT" "$REGISTRATION" "$FETCHER" "$DOCUMENTS" "$IDEMPOTENCY" "$LIMITS" "$PURGE" "$SWEEP" "$WORKER" "$STAMP_MIGRATION" "$STAMP_CONFIG" "$VERIFIER" "$EPOCH" "$MUST_CHANGE" "$CONNECTED" "$LAST_USED" "$ME" "$PROGRAM" "$ACCOUNT" "$ACCOUNT_ENDPOINTS" "$SWITCH_HANDLER" "$SWITCH_MIGRATION") +FILES=("$MCP" "$FORBIDDEN" "$POLICIES" "$IDENTITY" "$TELEMETRY" "$SERVER" "$ENDPOINT" "$REGISTRATION" "$FETCHER" "$DOCUMENTS" "$IDEMPOTENCY" "$LIMITS" "$PURGE" "$SWEEP" "$WORKER" "$STAMP_MIGRATION" "$STAMP_CONFIG" "$VERIFIER" "$EPOCH" "$MUST_CHANGE" "$CONNECTED" "$LAST_USED" "$ME" "$PROGRAM" "$ACCOUNT" "$ACCOUNT_ENDPOINTS" "$SWITCH_HANDLER" "$SWITCH_MIGRATION") restore() { git checkout -- "${FILES[@]}"; } if ! git diff --quiet -- "${FILES[@]}"; then From 1c35b868bd11f85912c084f17e5c983e67913353 Mon Sep 17 00:00:00 2001 From: mforce Date: Sat, 10 Oct 2026 20:27:55 +0000 Subject: [PATCH 2/6] test(mcp): pin the /mcp body marker directly, since the body guard cannot see it MapMcp maps a RequestDelegate, which carries no MethodInfo metadata, so BodyReadingEndpointTests never walks /mcp. A mutant removing ReadsRequestBodyAttribute left it green. The mutant now targets McpEndpointTests, and the guard table says why. --- docs/plans/770-mcp-server/02-guards.md | 4 +++- tools/oauth/mutation-check.sh | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/plans/770-mcp-server/02-guards.md b/docs/plans/770-mcp-server/02-guards.md index 6261c7e66..8d829c2a9 100644 --- a/docs/plans/770-mcp-server/02-guards.md +++ b/docs/plans/770-mcp-server/02-guards.md @@ -32,7 +32,9 @@ matters more than count: prefer "walk everything, exclude deliberately". | 12 | `Mcp_AddsNoLiveHostedServiceUnderStateless` | #271's blocker list is not extended | set `SessionMode = Stateful`, which makes `IdleTrackingBackgroundService.StartAsync` run its prune timer instead of returning early. **Asserts inertness, not absence** — the service IS registered unconditionally by `WithHttpTransport`; a guard asserting zero `IHostedService` descriptors would be red on the correct configuration. Candidate 1 proposed exactly that wrong guard. | #806 implements rows 8 to 12 in `tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs`. -Row 10 is the existing `BodyReadingEndpointTests` plus `McpPost_CarriesTheBodyCap_TheBudget_AndTheOAuthGate`. +Row 10's premise does not hold on 2.2.0: `MapMcp` maps a `RequestDelegate`, which carries no +`MethodInfo` metadata, so `BodyReadingEndpointTests` never walks `/mcp` (a mutant removing the +marker left it green). `McpPost_CarriesTheBodyCap_TheBudget_AndTheOAuthGate` pins the marker instead. Row 11 has no test of its own: `/mcp` adds no config key, and `ProcessRoleGuardTests` already boots a Production-shaped serving process, which maps `/mcp`. diff --git a/tools/oauth/mutation-check.sh b/tools/oauth/mutation-check.sh index 3aba17934..005fcf59f 100755 --- a/tools/oauth/mutation-check.sh +++ b/tools/oauth/mutation-check.sh @@ -227,12 +227,12 @@ cimd-everyone-gets-approved-budget#kill#DOCUMENTS#approved: await IsApprovedAsyn mcp-stateful#kill#MCP#options.SessionMode = HttpServerSessionMode.Stateless#options.SessionMode = HttpServerSessionMode.Stateful#Mcp.McpEndpointTests.Transport_IsStateless_WithNoSessionHook#Stateful mcp-stateful-sweep-runs#kill#MCP#options.SessionMode = HttpServerSessionMode.Stateless#options.SessionMode = HttpServerSessionMode.Stateful#Mcp.McpEndpointTests.IdleSessionSweep_IsRegistered_ButNeverStarts#Assert.Null() Failure mcp-session-hook#kill#MCP#options => options.SessionMode = HttpServerSessionMode.Stateless)#options => { options.SessionMode = HttpServerSessionMode.Stateless; options.ConfigureSessionOptions = (_, _, _) => Task.CompletedTask; })#Mcp.McpEndpointTests.Transport_IsStateless_WithNoSessionHook#Assert.Null() Failure -mcp-body-unclassified#kill#MCP#new ReadsRequestBodyAttribute(), ##BodyReadingEndpointTests.Every_endpoint_that_can_read_the_body_is_classified#POST /mcp +mcp-body-unclassified#kill#MCP#new ReadsRequestBodyAttribute(), ##Mcp.McpEndpointTests.McpPost_CarriesTheBodyCap_TheBudget_AndTheOAuthGate#Assert.NotNull() Failure mcp-body-uncapped#kill#MCP#\n .WithMaxRequestBodyBytes(MaxRequestBodyBytes)##Mcp.McpEndpointTests.OversizedBody_IsRefused#Expected: RequestEntityTooLarge mcp-idempotency-unmarked#kill#MCP#, new HandlesOwnIdempotencyAttribute()##Mcp.McpEndpointTests.McpMessages_WithoutAnIdempotencyKey_AreServed#Expected: OK mcp-idempotency-second-bearer#kill#ENDPOINT#.WithMetadata(new IgnoresAmbientPrincipalAttribute(), new ReadsRequestBodyAttribute())#.WithMetadata(new IgnoresAmbientPrincipalAttribute(), new ReadsRequestBodyAttribute(), new HandlesOwnIdempotencyAttribute())#Mcp.McpEndpointTests.IdempotencyExemption_IsCarriedByTheMcpPostAlone#Assert.Single() Failure mcp-idempotency-exemption-ignored#kill#IDEMPOTENCY#GetMetadata() is not null)#GetMetadata() is null && false)#Mcp.McpEndpointTests.McpMessages_WithoutAnIdempotencyKey_AreServed#Expected: OK -mcp-tools-list-unfiltered#kill#MCP#\n .AddAuthorizationFilters()##Mcp.McpAuthorizationTests.ToolsList_ShowsOnlyToolsTheRoleAndScopesAllow# +mcp-tools-list-unfiltered#kill#MCP#\n .AddAuthorizationFilters()##Mcp.McpAuthorizationTests.ToolsList_ShowsOnlyToolsTheRoleAndScopesAllow#Request failed (remote): An error occurred. mcp-read-scope-policy-wrong#kill#POLICIES#new OAuthScopeRequirement(OAuthScopes.ReadFarm)#new OAuthScopeRequirement(OAuthScopes.WriteDailyEntries)#Mcp.McpAuthorizationTests.ToolsList_ShowsOnlyToolsTheRoleAndScopesAllow#Collections differ mcp-mapped-without-issuer#kill#PROGRAM# .MapOAuthEndpoints();\n app.MapCluckworkMcp();\n}# .MapOAuthEndpoints();\n}\napp.MapCluckworkMcp();#Mcp.McpEndpointTests.WithoutAnIssuer_McpIsNotMapped#WithHttpTransport audience-unchecked#kill#SERVER#\n validation.AddAudiences(resource);##Mcp.McpAuthorizationTests.TokenIssuedForAnotherResource_IsRefused#Expected: Unauthorized From f6f61eb4cb758c626042de59484dc9fd88cadca6 Mon Sep 17 00:00:00 2001 From: mforce Date: Sat, 10 Oct 2026 20:38:20 +0000 Subject: [PATCH 3/6] fix(mcp): answer GET and HEAD /mcp with 405 instead of the SPA shell A stateless MCP server serves no GET stream, and the MCP spec then asks for 405. In Production the SPA fallback takes every GET and HEAD, so a client probing for the stream got the app's HTML with 200. --- docs/decisions/806-mcp-endpoint.md | 4 +++- src/Cluckwork.Api/Hosting/CluckworkMcp.cs | 13 ++++++++++++- .../Mcp/McpEndpointTests.cs | 14 ++++++++++++++ tools/oauth/mutation-check.sh | 1 + 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/docs/decisions/806-mcp-endpoint.md b/docs/decisions/806-mcp-endpoint.md index 6beff0f7f..a69ddbd87 100644 --- a/docs/decisions/806-mcp-endpoint.md +++ b/docs/decisions/806-mcp-endpoint.md @@ -25,7 +25,9 @@ endpoint serves MCP 2026-07-28 and 2025-11-25 clients. **Stateless, pinned.** `SessionMode = Stateless` makes the SDK run each tool in the HTTP request's own services, so `McpCallContext` (#805) sees the request's tenant, actor and flock scope, and each tool call is one authenticated request that `CredentialEpochMiddleware` -re-checks (#364). The SDK's idle-session sweep is registered but never starts. +re-checks (#364). The SDK's idle-session sweep is registered but never starts. Stateless +serves no GET stream, so `GET` and `HEAD /mcp` answer 405, as the MCP spec asks; otherwise +the SPA fallback would answer them with the app's HTML. **Tokens are bound to `/mcp` (RFC 8707).** The resource is `/mcp`, never the request's Host. The authorization server registers it as its only resource, so an authorize diff --git a/src/Cluckwork.Api/Hosting/CluckworkMcp.cs b/src/Cluckwork.Api/Hosting/CluckworkMcp.cs index af287ec68..aade8a76c 100644 --- a/src/Cluckwork.Api/Hosting/CluckworkMcp.cs +++ b/src/Cluckwork.Api/Hosting/CluckworkMcp.cs @@ -49,10 +49,21 @@ public static void AddCluckworkMcp(this IServiceCollection services, Authenticat } // Either scope admits the request; each tool then requires its own (AuthPolicies). - public static IEndpointConventionBuilder MapCluckworkMcp(this IEndpointRouteBuilder app) => + public static void MapCluckworkMcp(this IEndpointRouteBuilder app) + { app.MapMcp(Path) .WithMetadata(new ReadsRequestBodyAttribute(), new HandlesOwnIdempotencyAttribute()) .WithMaxRequestBodyBytes(MaxRequestBodyBytes) .AcceptOAuthTokens(OAuthScopes.ReadFarm, OAuthScopes.WriteDailyEntries) .RequireAuthorization(); + + // Stateless serves no GET stream, and the MCP spec then wants 405. Without this the + // SPA fallback, which takes every GET and HEAD, would answer with the app's HTML. + app.MapMethods(Path, [HttpMethods.Get, HttpMethods.Head], (HttpResponse response) => + { + response.Headers.Allow = HttpMethods.Post; + return Results.StatusCode(StatusCodes.Status405MethodNotAllowed); + }) + .ExcludeFromDescription(); + } } diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs index c835ac814..04621987d 100644 --- a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs @@ -93,6 +93,20 @@ public async Task OversizedBody_IsRefused() Assert.Equal(HttpStatusCode.RequestEntityTooLarge, response.StatusCode); } + // Stateless serves no GET stream, so the MCP spec wants 405. In Production the SPA + // fallback would otherwise answer GET and HEAD with the app's HTML. + [Theory] + [InlineData("GET")] + [InlineData("HEAD")] + public async Task GetAndHead_Get405(string method) + { + using var response = await OAuthServerTests.HttpsClient(factory, bearer: null) + .SendAsync(new HttpRequestMessage(new HttpMethod(method), CluckworkMcp.Path)); + + Assert.Equal(HttpStatusCode.MethodNotAllowed, response.StatusCode); + Assert.Equal(["POST"], response.Content.Headers.Allow); + } + // OAuth tokens are the only way in, so without the server that issues them there is no /mcp. [Fact] public void WithoutAnIssuer_McpIsNotMapped() diff --git a/tools/oauth/mutation-check.sh b/tools/oauth/mutation-check.sh index 005fcf59f..6877eae12 100755 --- a/tools/oauth/mutation-check.sh +++ b/tools/oauth/mutation-check.sh @@ -234,6 +234,7 @@ mcp-idempotency-second-bearer#kill#ENDPOINT#.WithMetadata(new IgnoresAmbientPrin mcp-idempotency-exemption-ignored#kill#IDEMPOTENCY#GetMetadata() is not null)#GetMetadata() is null && false)#Mcp.McpEndpointTests.McpMessages_WithoutAnIdempotencyKey_AreServed#Expected: OK mcp-tools-list-unfiltered#kill#MCP#\n .AddAuthorizationFilters()##Mcp.McpAuthorizationTests.ToolsList_ShowsOnlyToolsTheRoleAndScopesAllow#Request failed (remote): An error occurred. mcp-read-scope-policy-wrong#kill#POLICIES#new OAuthScopeRequirement(OAuthScopes.ReadFarm)#new OAuthScopeRequirement(OAuthScopes.WriteDailyEntries)#Mcp.McpAuthorizationTests.ToolsList_ShowsOnlyToolsTheRoleAndScopesAllow#Collections differ +mcp-get-falls-to-spa#kill#MCP# app.MapMethods(Path, [HttpMethods.Get, HttpMethods.Head], (HttpResponse response) =># app.MapMethods(Path + "/unused", [HttpMethods.Get, HttpMethods.Head], (HttpResponse response) =>#Mcp.McpEndpointTests.GetAndHead_Get405#Expected: MethodNotAllowed mcp-mapped-without-issuer#kill#PROGRAM# .MapOAuthEndpoints();\n app.MapCluckworkMcp();\n}# .MapOAuthEndpoints();\n}\napp.MapCluckworkMcp();#Mcp.McpEndpointTests.WithoutAnIssuer_McpIsNotMapped#WithHttpTransport audience-unchecked#kill#SERVER#\n validation.AddAudiences(resource);##Mcp.McpAuthorizationTests.TokenIssuedForAnotherResource_IsRefused#Expected: Unauthorized audience-unchecked-unbound#kill#SERVER#\n validation.AddAudiences(resource);##Mcp.McpAuthorizationTests.TokenWithoutTheMcpAudience_IsRefused#Expected: Unauthorized From 68063d5d28b26820221b5d8d5aa9af5700bdb668 Mon Sep 17 00:00:00 2001 From: mforce Date: Sat, 10 Oct 2026 21:18:40 +0000 Subject: [PATCH 4/6] fix(mcp): challenge every refused OAuth token, and walk inherited tool methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 1 on #1195. - A disabled user, a suspended farm, a superseded credential or the farm switch now answers /mcp with WWW-Authenticate: Bearer error="invalid_token", resource_metadata="..." beside its problem body (RFC 6750 §3). CluckworkMcp.AddChallenge builds it for these 401s and for the insufficient_scope 403. - McpToolSurface finds tool types and methods the way SDK 2.2.0 does, including a public method a tool type inherits, gated by its declaring type's attributes. McpEndpointTests checks the walk reaches every tool the SDK registers, which is non-zero with the probe tools. - The 429 test asserts JSON-RPC field types and Retry-After instead of an English phrase. - A Production host calls /mcp with only the issuer set, a client follows the challenge to the authorization server's discovery document, and a flock-scoped Worker's tool call sees exactly its assigned flock. - 02-guards.md names the Production proof for row 11; mutation-check.sh gains five mutants for the new guards. --- docs/decisions/806-mcp-endpoint.md | 21 +++-- docs/plans/770-mcp-server/02-guards.md | 12 +-- src/Cluckwork.Api/Hosting/CluckworkMcp.cs | 13 +++ .../Middleware/CredentialEpochMiddleware.cs | 4 + .../ForbiddenProblemResultHandler.cs | 8 +- .../Mcp/McpAuthorizationTests.cs | 86 +++++++++++++++++-- .../Mcp/McpConnection.cs | 3 +- .../Mcp/McpEndpointTests.cs | 19 ++++ .../Mcp/McpToolSurface.cs | 16 +++- .../Mcp/McpToolSurfaceTests.cs | 40 +++++++++ .../OAuthServerProductionTests.cs | 8 +- tools/oauth/mutation-check.sh | 10 ++- 12 files changed, 207 insertions(+), 33 deletions(-) diff --git a/docs/decisions/806-mcp-endpoint.md b/docs/decisions/806-mcp-endpoint.md index a69ddbd87..84b56abba 100644 --- a/docs/decisions/806-mcp-endpoint.md +++ b/docs/decisions/806-mcp-endpoint.md @@ -52,6 +52,15 @@ missing either gate, and one carrying `[AllowAnonymous]`, because the SDK runs s for anyone. `AcceptOAuthTokens(ReadFarm, WriteDailyEntries)` stays on the mapping as the outer gate. +**Every refusal of an OAuth token carries a challenge.** RFC 6750 §3 asks for a bearer +challenge whenever the credentials do not grant access, and an MCP client re-authorizes from +it. `CluckworkMcp.AddChallenge` writes `Bearer , resource_metadata="..."`, the same +header the SDK's handler emits. `CredentialEpochMiddleware` adds it with +`error="invalid_token"` beside its problem body when a disabled user, a suspended farm, a +superseded credential or the farm switch refuses a token on an `AcceptOAuthTokens` endpoint. +`TenantResolutionMiddleware`'s bodiless 401s are left alone: they refuse a principal with no +`account_id` or `sub`, and consent copies both from the session into every token. + **A missing scope says which one.** When only scope requirements failed, `ForbiddenProblemResultHandler` answers 403 with `WWW-Authenticate: Bearer error="insufficient_scope", scope="...", resource_metadata="..."` @@ -64,10 +73,12 @@ client sends an `Idempotency-Key`, so `IdempotencyMiddleware` skips an endpoint `McpEndpointTests.IdempotencyExemption_IsCarriedByTheMcpPostAlone`. The write tool keys its own claim (#809). -**Budget: 300 messages per minute per token.** `oauth-api` rose from 120. Measured with the +**Budget: 300 messages per minute per token.** `oauth-api` rose from 120. Traced with the SDK's own client against this endpoint, a session spends 3 POSTs to its first tool result on -2026-07-28 (`server/discover`, `tools/list`, `tools/call`) and 4 on 2025-11-25 (`initialize` -and its notification replace discovery); each later call is one POST. No account-wide +2026-07-28 (`server/discover`, `tools/list`, `tools/call`) and 4 on 2025-11-25 (`initialize`, +`notifications/initialized`, `tools/list`, `tools/call`, plus a `GET` the limiter does not +count); each later call is one POST. In process a call took 4 to 6 ms, so only a runaway loop +reaches the limit; a model spends seconds between calls. No account-wide ceiling: each token costs a step-up password entry (#798), which the password limiter already bounds. A 429 on `oauth-api` is a JSON-RPC error with a null id, because the limiter runs before the body is read, and its text names the connection, not an address. @@ -81,10 +92,6 @@ which only classifies binding errors. ## What this does NOT cover -- **401s from the shared request checks carry no challenge header.** A disabled user, a - suspended farm, a superseded credential or the farm switch writes its 401 directly - (#796). An MCP client then probes `/.well-known/oauth-protected-resource/mcp` itself, as - the MCP authorization spec requires, and that document is served. - **A tool-level scope denial is a JSON-RPC error, not an HTTP 403.** The SDK refuses a `tools/call` for a tool the caller cannot see with `InvalidRequest` inside a 200. Only the endpoint gate answers `insufficient_scope`. A client that follows `tools/list` never calls diff --git a/docs/plans/770-mcp-server/02-guards.md b/docs/plans/770-mcp-server/02-guards.md index 8d829c2a9..50bb9a35d 100644 --- a/docs/plans/770-mcp-server/02-guards.md +++ b/docs/plans/770-mcp-server/02-guards.md @@ -35,8 +35,9 @@ matters more than count: prefer "walk everything, exclude deliberately". Row 10's premise does not hold on 2.2.0: `MapMcp` maps a `RequestDelegate`, which carries no `MethodInfo` metadata, so `BodyReadingEndpointTests` never walks `/mcp` (a mutant removing the marker left it green). `McpPost_CarriesTheBodyCap_TheBudget_AndTheOAuthGate` pins the marker instead. -Row 11 has no test of its own: `/mcp` adds no config key, and `ProcessRoleGuardTests` already -boots a Production-shaped serving process, which maps `/mcp`. +Row 11 is `OAuthServerProductionTests.Production_ServesTheConnectFlow`: a Production host with +only `OAuth:Issuer` set connects an app and calls `/mcp` with its token. (`ProcessRoleGuardTests` +only proves that each serving guard fails a boot, not that `/mcp` works.) ## RBAC @@ -106,9 +107,10 @@ of the residue, which is why slice 6 is blocked on it — but a secondary-scope - A `ReviewedFactory` row approves a service type and the type that registers it. If the delegate's body changes later, the row still approves it. A second factory for the same service in another type, and a row the walk no longer reaches, both fail. -- `McpToolSurface` finds tool methods with `BindingFlags.DeclaredOnly`. A tool method inherited from - a base class is not walked. Whether the SDK serves inherited attributed methods is unproven until - #806 picks the SDK version. +- Since #806, `McpToolSurface` finds tool types and methods the way SDK 2.2.0's `WithTools` does, + including a public method a tool type inherits, whose policies come from the class declaring it. + `McpEndpointTests.ToolWalk_ReachesEveryToolTheSdkRegisters` fails if the walk and the SDK's + registrations ever count differently, for example after an SDK upgrade. - A `ReviewedRequestReader` row is keyed by its consumer only. A changed body, an added reader parameter, or a member returning a provider on a reviewed reader is not detected, the same kind of drift as a `ReviewedFactory` row. diff --git a/src/Cluckwork.Api/Hosting/CluckworkMcp.cs b/src/Cluckwork.Api/Hosting/CluckworkMcp.cs index aade8a76c..89af29c1a 100644 --- a/src/Cluckwork.Api/Hosting/CluckworkMcp.cs +++ b/src/Cluckwork.Api/Hosting/CluckworkMcp.cs @@ -4,7 +4,9 @@ using Cluckwork.Application.Modules.Access.Contracts; using Cluckwork.Infrastructure.Modules.Access.OAuth; using Microsoft.AspNetCore.Authentication; +using Microsoft.Extensions.Options; using ModelContextProtocol.AspNetCore; +using ModelContextProtocol.AspNetCore.Authentication; using ModelContextProtocol.Authentication; using OpenIddict.Validation.AspNetCore; @@ -48,6 +50,17 @@ public static void AddCluckworkMcp(this IServiceCollection services, Authenticat }); } + // RFC 6750 §3 and RFC 9728 §5.1: a refused OAuth token is answered with a bearer + // challenge naming the protected-resource metadata, which is how an MCP client + // re-authorizes. The SDK's handler adds it to a challenge; the shared request checks + // and the scope gate write their own responses and call this. + public static void AddChallenge(HttpContext context, string parameters) + { + var metadata = context.RequestServices.GetRequiredService>() + .Get(McpAuthenticationDefaults.AuthenticationScheme).ResourceMetadataUri; + context.Response.Headers.WWWAuthenticate = $"Bearer {parameters}, resource_metadata=\"{metadata}\""; + } + // Either scope admits the request; each tool then requires its own (AuthPolicies). public static void MapCluckworkMcp(this IEndpointRouteBuilder app) { diff --git a/src/Cluckwork.Api/Middleware/CredentialEpochMiddleware.cs b/src/Cluckwork.Api/Middleware/CredentialEpochMiddleware.cs index 18a324b73..2a0937fdd 100644 --- a/src/Cluckwork.Api/Middleware/CredentialEpochMiddleware.cs +++ b/src/Cluckwork.Api/Middleware/CredentialEpochMiddleware.cs @@ -1,3 +1,4 @@ +using Cluckwork.Api.Hosting; using Cluckwork.Api.Modules.Access.OAuth; using Cluckwork.Application.Modules.Access.Contracts; using System.Globalization; @@ -55,6 +56,9 @@ public async Task InvokeAsync(HttpContext context, ICredentialEpochVerifier veri _ => ("Auth.CredentialsSuperseded", "Your credentials have been superseded. Sign in again."), }; context.Response.StatusCode = StatusCodes.Status401Unauthorized; + // #806 — the token no longer grants access, so an MCP client must re-authorize. + if (OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint())) + CluckworkMcp.AddChallenge(context, "error=\"invalid_token\""); context.Response.ContentType = "application/problem+json"; await context.Response.WriteAsJsonAsync(new ProblemDetails { diff --git a/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs b/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs index 2d701d254..d61f91401 100644 --- a/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs +++ b/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs @@ -1,9 +1,8 @@ +using Cluckwork.Api.Hosting; using Cluckwork.Api.Modules.Access.OAuth; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Authorization.Policy; using Microsoft.AspNetCore.Mvc; -using Microsoft.Extensions.Options; -using ModelContextProtocol.AspNetCore.Authentication; namespace Cluckwork.Api.Middleware; @@ -28,10 +27,7 @@ public async Task HandleAsync( if (onlyScopesFailed) { var scopes = failed.Cast().SelectMany(r => r.AnyOf).Distinct(); - var metadata = context.RequestServices.GetRequiredService>() - .Get(McpAuthenticationDefaults.AuthenticationScheme).ResourceMetadataUri; - context.Response.Headers.WWWAuthenticate = - $"Bearer error=\"insufficient_scope\", scope=\"{string.Join(' ', scopes)}\", resource_metadata=\"{metadata}\""; + CluckworkMcp.AddChallenge(context, $"error=\"insufficient_scope\", scope=\"{string.Join(' ', scopes)}\""); } context.Response.StatusCode = StatusCodes.Status403Forbidden; diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpAuthorizationTests.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpAuthorizationTests.cs index 995b46edc..fcb7cec7d 100644 --- a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpAuthorizationTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpAuthorizationTests.cs @@ -2,10 +2,12 @@ using System.Net.Http.Json; using System.Text.Json; using System.Text.Json.Nodes; +using Cluckwork.Domain.Modules.Farm.Accounts; using Cluckwork.Api.IntegrationTests.Infrastructure; using Cluckwork.Application.Modules.Access.Contracts; using Cluckwork.Domain.Modules.Farm.Contracts; using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; using ModelContextProtocol; using ModelContextProtocol.Protocol; @@ -215,6 +217,72 @@ public async Task FarmSwitchOff_RefusesTheConnection_OnTheNextRequest() await AssertRefusedAsync(host, token, "Auth.ConnectedAppsOff"); } + // Each shared check, tripped after the token was issued, refuses the next request. + [Theory] + [InlineData("Auth.AccountDisabled")] + [InlineData("Auth.FarmSuspended")] + [InlineData("Auth.CredentialsSuperseded")] + public async Task SharedCheck_RefusesTheConnection_WithAChallenge(string title) + { + using var host = Host(factory); + var user = await SeedAsync(factory, Roles.Manager); + var token = await IssueAsync(host, user, [OAuthScopes.ReadFarm]); + await AssertAcceptedAsync(host, token); + + await factory.WithTenantScopeAsync(user.AccountId, db => title switch + { + "Auth.AccountDisabled" => db.Users.Where(u => u.Id == user.Id) + .ExecuteUpdateAsync(s => s.SetProperty(u => u.DisabledAt, DateTimeOffset.UtcNow)), + "Auth.FarmSuspended" => db.Accounts.Where(a => a.Id == user.AccountId) + .ExecuteUpdateAsync(s => s.SetProperty(a => a.IsActive, false)), + _ => db.Users.Where(u => u.Id == user.Id) + .ExecuteUpdateAsync(s => s.SetProperty(u => u.CredentialEpoch, u => u.CredentialEpoch + 1)), + }); + + await AssertRefusedAsync(host, token, title); + } + + // #388 through /mcp: a Worker's tool call sees exactly the flocks assigned to them. + [Fact] + public async Task AssignedWorker_ToolSeesOnlyTheirFlocks() + { + using var host = Host(factory); + var user = await SeedAsync(factory, role: null); + var assigned = await factory.SeedFlockAsync(user.AccountId, Guid.NewGuid()); + await factory.SeedFlockAsync(user.AccountId, Guid.NewGuid()); + await factory.WithTenantScopeAsync(user.AccountId, async db => + { + db.UserRoleAssignments.Add(UserRoleAssignment.Create(Guid.NewGuid(), user.AccountId, user.Id, null, null, assigned)); + await db.SaveChangesAsync(); + }); + var token = await IssueAsync(host, user, [OAuthScopes.ReadFarm]); + + await using var client = await ConnectAsync(host, token); + var caller = Assert.IsType(Assert.Single((await client.CallToolAsync(ProbeTools.Read)).Content)).Text; + + Assert.EndsWith($"/True/{assigned}", caller); + } + + // What a client does with no configuration: follow the challenge to the resource + // metadata, and that to the authorization server's own discovery document. + [Fact] + public async Task Challenge_LeadsToTheAuthorizationServer() + { + using var host = Host(factory); + using var http = OAuthServerTests.HttpsClient(host, bearer: null); + + using var challenge = await PostAsync(host, bearer: null, Initialize); + var metadataUrl = challenge.Headers.WwwAuthenticate.Single().Parameter!.Split('"')[1]; + var metadata = await http.GetFromJsonAsync(metadataUrl); + var issuer = Assert.Single(metadata.GetProperty("authorization_servers").EnumerateArray()).GetString()!; + var discovery = await http.GetFromJsonAsync(new Uri(new Uri(issuer), ".well-known/oauth-authorization-server")); + + Assert.Equal(ResourceMetadata, metadataUrl); + Assert.Equal("https://localhost/", issuer); + Assert.Equal(issuer, discovery.GetProperty("issuer").GetString()); + Assert.Equal("https://localhost/api/v1/oauth/authorize", discovery.GetProperty("authorization_endpoint").GetString()); + } + [Fact] public async Task Disconnect_RefusesTheConnection_OnTheNextRequest() { @@ -254,8 +322,8 @@ public async Task PendingPasswordChange_IsRefused() (await response.Content.ReadFromJsonAsync()).GetProperty("title").GetString()); } - // #806 — an MCP client reads the 429 as a JSON-RPC error, and the text describes the - // per-token bucket rather than an address. + // #806 — an MCP client reads the 429 as a JSON-RPC error. The limiter runs before the + // body is read, so the request id is unknown and null. [Fact] public async Task OverTheBudget_Gets429_AsAJsonRpcError() { @@ -266,12 +334,15 @@ public async Task OverTheBudget_Gets429_AsAJsonRpcError() using var response = await PostAsync(host, token, Initialize); Assert.Equal(HttpStatusCode.TooManyRequests, response.StatusCode); + Assert.True(response.Headers.RetryAfter?.Delta > TimeSpan.Zero, "the 429 carries no Retry-After"); var body = await response.Content.ReadFromJsonAsync(); - var isRpcError = body.TryGetProperty("jsonrpc", out var version) && version.GetString() == "2.0" + var isRpcError = body.TryGetProperty("jsonrpc", out var version) && version.ValueKind == JsonValueKind.String && version.GetString() == "2.0" && body.TryGetProperty("id", out var id) && id.ValueKind == JsonValueKind.Null - && body.TryGetProperty("error", out var error) && error.GetProperty("code").GetInt32() == -32000 - && error.GetProperty("message").GetString()!.Contains("this connection"); - Assert.True(isRpcError, $"the 429 body is not a JSON-RPC error about this connection: {body}"); + && body.TryGetProperty("error", out var error) + && error.TryGetProperty("code", out var code) && code.ValueKind == JsonValueKind.Number && code.TryGetInt32(out _) + && error.TryGetProperty("message", out var message) && message.ValueKind == JsonValueKind.String + && !string.IsNullOrWhiteSpace(message.GetString()); + Assert.True(isRpcError, $"the 429 body is not a JSON-RPC error: {body}"); } private static async Task AssertAcceptedAsync(WebApplicationFactory host, string token) @@ -280,10 +351,13 @@ private static async Task AssertAcceptedAsync(WebApplicationFactory hos Assert.Equal(HttpStatusCode.OK, response.StatusCode); } + // The problem body names the check; the challenge tells the client to re-authorize. private static async Task AssertRefusedAsync(WebApplicationFactory host, string token, string title) { using var response = await PostAsync(host, token, Initialize); Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); Assert.Equal(title, (await response.Content.ReadFromJsonAsync()).GetProperty("title").GetString()); + Assert.Equal($"Bearer error=\"invalid_token\", resource_metadata=\"{ResourceMetadata}\"", + response.Headers.WwwAuthenticate.ToString()); } } diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpConnection.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpConnection.cs index 90e9a5adb..a03d8bbd8 100644 --- a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpConnection.cs +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpConnection.cs @@ -158,7 +158,8 @@ public sealed class ProbeTools(McpCallContext call) [McpServerTool(Name = Read, ReadOnly = true), Description("Who is calling.")] [Authorize] [Authorize(Policy = AuthPolicies.FarmReadScope)] - public string WhoIsCalling() => $"{call.AccountId}/{call.UserId}/{call.ConnectedApp.ClientId}"; + public string WhoIsCalling() => + $"{call.AccountId}/{call.UserId}/{call.ConnectedApp.ClientId}/{call.IsFlockRestricted}/{string.Join(',', call.AssignedFlockIds.Order())}"; [McpServerTool(Name = Write), Description("A write a Worker may make.")] [Authorize(Policy = AuthPolicies.ProductionWrite)] diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs index 04621987d..544034a95 100644 --- a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs @@ -14,6 +14,8 @@ using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Options; using ModelContextProtocol.AspNetCore; +using ModelContextProtocol.Server; +using Cluckwork.Api.Mcp; using static Cluckwork.Api.IntegrationTests.Mcp.McpConnection; namespace Cluckwork.Api.IntegrationTests.Mcp; @@ -107,6 +109,20 @@ public async Task GetAndHead_Get405(string method) Assert.Equal(["POST"], response.Content.Headers.Allow); } + // The tool walks in McpToolSurfaceTests must reach every tool the SDK serves. The + // probe host adds two tools, so a walk that finds nothing fails here today. + [Fact] + public void ToolWalk_ReachesEveryToolTheSdkRegisters() + { + var apiTypes = typeof(McpCallContext).Assembly.GetTypes(); + using var probed = Host(factory); + var registered = probed.Services.GetServices().Count(); + + Assert.Equal(factory.Services.GetServices().Count(), Walked(apiTypes)); + Assert.True(registered >= 2, $"the probe host registered {registered} tools"); + Assert.Equal(registered, Walked(apiTypes.Append(typeof(ProbeTools)))); + } + // OAuth tokens are the only way in, so without the server that issues them there is no /mcp. [Fact] public void WithoutAnIssuer_McpIsNotMapped() @@ -120,6 +136,9 @@ public void WithoutAnIssuer_McpIsNotMapped() Assert.DoesNotContain(routes, r => r?.TrimEnd('/') == CluckworkMcp.Path); } + private static int Walked(IEnumerable types) => + McpToolSurface.ToolTypes(types).SelectMany(McpToolSurface.ToolMethods).Count(); + private static RouteEndpoint McpPost(WebApplicationFactory host) => Assert.Single(host.Services.GetRequiredService().Endpoints.OfType(), e => e.RoutePattern.RawText?.TrimEnd('/') == CluckworkMcp.Path diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.cs index 314f9cc09..e89b3e046 100644 --- a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.cs +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.cs @@ -10,7 +10,9 @@ namespace Cluckwork.Api.IntegrationTests.Mcp; // #805 rows 6 and 7 — what a tool may inject. A tool is a type carrying an attribute // NAMED McpServerToolTypeAttribute, and a tool method one carrying McpServerToolAttribute. // Matched by name, as AdapterTierScanner matches it, so the fixtures here can declare -// look-alikes that prove the walk goes red. +// look-alikes that prove the walk goes red. Types and methods are found the way the SDK's +// WithTools finds them (McpServerBuilderExtensions, 2.2.0): attributes read with +// inheritance, and every public instance method a tool type inherits. internal static class McpToolSurface { // A supertype is banned too: ICurrentUser is CurrentUserContext's port and DbContext @@ -24,11 +26,14 @@ internal static class McpToolSurface private const BindingFlags Declared = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly; + private const BindingFlags AsTheSdkFindsTools = + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance; + public static IEnumerable ToolTypes(IEnumerable types) => types.Where(t => HasAttribute(t, "McpServerToolTypeAttribute")); public static IEnumerable ToolMethods(Type tool) => - tool.GetMethods(Declared).Where(m => HasAttribute(m, "McpServerToolAttribute")); + tool.GetMethods(AsTheSdkFindsTools).Where(m => HasAttribute(m, "McpServerToolAttribute")); // Everything DI hands a tool: constructor parameters and tool-method parameters. public static IEnumerable<(string Site, Type Type)> Injections(Type tool) => @@ -70,7 +75,10 @@ public static IReadOnlyList GateFindings(IEnumerable types) foreach (var tool in ToolTypes(types)) foreach (var method in ToolMethods(tool)) { - var attributes = method.GetCustomAttributes(inherit: true).Concat(tool.GetCustomAttributes(inherit: true)).ToList(); + // The SDK reads a tool's policies from the method and its DECLARING type, so an + // inherited method carries its base class's attributes, not the tool type's. + var attributes = method.GetCustomAttributes(inherit: true) + .Concat(method.DeclaringType!.GetCustomAttributes(inherit: true)).ToList(); var site = $"{tool.FullName}.{method.Name}"; if (attributes.OfType().Any()) findings.Add($"{site} allows anonymous callers, so the SDK skips every policy on it"); @@ -109,5 +117,5 @@ private static bool IsBanned(Type type) => || (type.HasElementType && IsBanned(type.GetElementType()!)); private static bool HasAttribute(MemberInfo member, string name) => - member.CustomAttributes.Any(a => a.AttributeType.Name == name); + member.GetCustomAttributes(inherit: true).Any(a => a.GetType().Name == name); } diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurfaceTests.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurfaceTests.cs index e46aa72c8..835802b4e 100644 --- a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurfaceTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurfaceTests.cs @@ -72,6 +72,33 @@ public void ToolInjectingAmbientIdentityOrAServiceLocator_IsAFinding(Type inject public void GatedTools_HaveNoGateFindings() => Assert.Empty(McpToolSurface.GateFindings([typeof(GatedTool), typeof(TypeGatedTool)])); + // The floor for the gate fixtures: the walk reaches every tool method in them, + // including one a tool type inherits. McpEndpointTests holds the real tree to the + // tools the SDK actually registers. + [Fact] + public void GateWalk_ReachesEveryFixtureToolMethod() + { + Type[] fixtures = [typeof(GatedTool), typeof(TypeGatedTool), typeof(RoleOnlyTool), + typeof(ScopeOnlyTool), typeof(AnonymousTool), typeof(InheritingTool)]; + + var methods = McpToolSurface.ToolTypes(fixtures).SelectMany(McpToolSurface.ToolMethods) + .Select(m => $"{m.DeclaringType!.Name}.{m.Name}").Order(); + + Assert.Equal(["AnonymousTool.Read", "GatedTool.Read", "InheritingTool.Read", "RoleOnlyTool.Read", + "ScopeOnlyTool.Read", "TypeGatedTool.Write", "UngatedBase.Leak"], methods); + } + + // The SDK serves a public tool method a tool type inherits, under the attributes of + // the class that declares it, so the gates on InheritingTool do not cover Leak. + [Fact] + public void InheritedToolMethod_IsGatedByItsDeclaringType() + { + var findings = McpToolSurface.GateFindings([typeof(InheritingTool)]); + + Assert.Contains(findings, f => f.EndsWith(".Leak has no scope policy; add [Authorize(Policy = AuthPolicies.Scope)]", StringComparison.Ordinal)); + Assert.DoesNotContain(findings, f => f.Contains(".Read ", StringComparison.Ordinal)); + } + [Theory] [InlineData(typeof(RoleOnlyTool), "has no scope policy")] [InlineData(typeof(ScopeOnlyTool), "has no role policy")] @@ -186,6 +213,19 @@ private sealed class AnonymousTool(McpCallContext call) public string Read() => call.Email; } + private class UngatedBase + { + [McpServerTool] + public string Leak() => ""; + } + + [McpServerToolType, Authorize, Authorize(Policy = AuthPolicies.FarmReadScope)] + private sealed class InheritingTool(McpCallContext call) : UngatedBase + { + [McpServerTool] + public string Read() => call.Email; + } + private sealed class ScopeFactoryHelper(IServiceScopeFactory scopes) { public IServiceScope Open() => scopes.CreateScope(); diff --git a/tests/Cluckwork.Api.IntegrationTests/OAuthServerProductionTests.cs b/tests/Cluckwork.Api.IntegrationTests/OAuthServerProductionTests.cs index 5e88da143..327308939 100644 --- a/tests/Cluckwork.Api.IntegrationTests/OAuthServerProductionTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/OAuthServerProductionTests.cs @@ -58,8 +58,12 @@ public async Task Production_ServesTheConnectFlow() Assert.Equal("Claude Desktop", (await consent.Content.ReadFromJsonAsync()).GetProperty("clientName").GetString()); Assert.Equal(HttpStatusCode.OK, token.StatusCode); - Assert.False(string.IsNullOrEmpty( - (await token.Content.ReadFromJsonAsync()).GetProperty("access_token").GetString())); + var accessToken = (await token.Content.ReadFromJsonAsync()).GetProperty("access_token").GetString(); + Assert.False(string.IsNullOrEmpty(accessToken)); + + // #806 — /mcp needs no configuration beyond the issuer. + using var mcp = await Mcp.McpConnection.PostAsync(factory, accessToken, Mcp.McpConnection.Initialize); + Assert.Equal(HttpStatusCode.OK, mcp.StatusCode); } [Theory] diff --git a/tools/oauth/mutation-check.sh b/tools/oauth/mutation-check.sh index 6877eae12..f0f801d31 100755 --- a/tools/oauth/mutation-check.sh +++ b/tools/oauth/mutation-check.sh @@ -52,10 +52,11 @@ SWITCH_MIGRATION=src/Cluckwork.Infrastructure/Persistence/Migrations/20261009193 MCP=src/Cluckwork.Api/Hosting/CluckworkMcp.cs FORBIDDEN=src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs POLICIES=src/Cluckwork.Api/AuthPolicies.cs +TOOL_SURFACE=tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.cs TESTS=tests/Cluckwork.Api.IntegrationTests TEST_NS=Cluckwork.Api.IntegrationTests SUITE='FullyQualifiedName~OAuth|FullyQualifiedName~ClientMetadata|FullyQualifiedName~Cluckwork.Api.IntegrationTests.Mcp.' -SUITE_MIN=340 +SUITE_MIN=355 # name # expect # file # find # replace # test # declared failure text # ('#' because C# anchors contain '|'; '\n' in a find or replace is a newline) @@ -235,6 +236,11 @@ mcp-idempotency-exemption-ignored#kill#IDEMPOTENCY#GetMetadata# app.MapMethods(Path + "/unused", [HttpMethods.Get, HttpMethods.Head], (HttpResponse response) =>#Mcp.McpEndpointTests.GetAndHead_Get405#Expected: MethodNotAllowed +shared-check-unchallenged#kill#EPOCH# if (OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint()))# if (OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint()) && false)#Mcp.McpAuthorizationTests.SharedCheck_RefusesTheConnection_WithAChallenge#Strings differ +rpc-429-no-retry-after#kill#LIMITS#context.HttpContext.Response.Headers.RetryAfter =#_ =#Mcp.McpAuthorizationTests.OverTheBudget_Gets429_AsAJsonRpcError#no Retry-After +tool-walk-declared-only#kill#TOOL_SURFACE# BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance;# BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance | BindingFlags.DeclaredOnly;#Mcp.McpToolSurfaceTests.InheritedToolMethod_IsGatedByItsDeclaringType#Assert.Contains() Failure +tool-walk-reflected-type-gates#kill#TOOL_SURFACE#.Concat(method.DeclaringType!.GetCustomAttributes(inherit: true))#.Concat(method.ReflectedType!.GetCustomAttributes(inherit: true))#Mcp.McpToolSurfaceTests.InheritedToolMethod_IsGatedByItsDeclaringType#Assert.Contains() Failure +tool-walk-finds-nothing#kill#TOOL_SURFACE#Where(m => HasAttribute(m, "McpServerToolAttribute"))#Where(m => HasAttribute(m, "NoSuchAttribute"))#Mcp.McpEndpointTests.ToolWalk_ReachesEveryToolTheSdkRegisters#Values differ mcp-mapped-without-issuer#kill#PROGRAM# .MapOAuthEndpoints();\n app.MapCluckworkMcp();\n}# .MapOAuthEndpoints();\n}\napp.MapCluckworkMcp();#Mcp.McpEndpointTests.WithoutAnIssuer_McpIsNotMapped#WithHttpTransport audience-unchecked#kill#SERVER#\n validation.AddAudiences(resource);##Mcp.McpAuthorizationTests.TokenIssuedForAnotherResource_IsRefused#Expected: Unauthorized audience-unchecked-unbound#kill#SERVER#\n validation.AddAudiences(resource);##Mcp.McpAuthorizationTests.TokenWithoutTheMcpAudience_IsRefused#Expected: Unauthorized @@ -339,7 +345,7 @@ sys.exit(1 if problems else 0) PY } -FILES=("$MCP" "$FORBIDDEN" "$POLICIES" "$IDENTITY" "$TELEMETRY" "$SERVER" "$ENDPOINT" "$REGISTRATION" "$FETCHER" "$DOCUMENTS" "$IDEMPOTENCY" "$LIMITS" "$PURGE" "$SWEEP" "$WORKER" "$STAMP_MIGRATION" "$STAMP_CONFIG" "$VERIFIER" "$EPOCH" "$MUST_CHANGE" "$CONNECTED" "$LAST_USED" "$ME" "$PROGRAM" "$ACCOUNT" "$ACCOUNT_ENDPOINTS" "$SWITCH_HANDLER" "$SWITCH_MIGRATION") +FILES=("$MCP" "$FORBIDDEN" "$POLICIES" "$TOOL_SURFACE" "$IDENTITY" "$TELEMETRY" "$SERVER" "$ENDPOINT" "$REGISTRATION" "$FETCHER" "$DOCUMENTS" "$IDEMPOTENCY" "$LIMITS" "$PURGE" "$SWEEP" "$WORKER" "$STAMP_MIGRATION" "$STAMP_CONFIG" "$VERIFIER" "$EPOCH" "$MUST_CHANGE" "$CONNECTED" "$LAST_USED" "$ME" "$PROGRAM" "$ACCOUNT" "$ACCOUNT_ENDPOINTS" "$SWITCH_HANDLER" "$SWITCH_MIGRATION") restore() { git checkout -- "${FILES[@]}"; } if ! git diff --quiet -- "${FILES[@]}"; then From 6f71cfc439946c473be69e68a92ce910ca87867d Mon Sep 17 00:00:00 2001 From: mforce Date: Sat, 10 Oct 2026 21:34:01 +0000 Subject: [PATCH 5/6] test(mcp): make the Retry-After mutant compile The rate limiter's OnRejected lambda names its CancellationToken parameter _, so assigning to _ was not a discard. --- tools/oauth/mutation-check.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/oauth/mutation-check.sh b/tools/oauth/mutation-check.sh index f0f801d31..028e07a81 100755 --- a/tools/oauth/mutation-check.sh +++ b/tools/oauth/mutation-check.sh @@ -237,7 +237,7 @@ mcp-tools-list-unfiltered#kill#MCP#\n .AddAuthorizationFilters()##Mcp mcp-read-scope-policy-wrong#kill#POLICIES#new OAuthScopeRequirement(OAuthScopes.ReadFarm)#new OAuthScopeRequirement(OAuthScopes.WriteDailyEntries)#Mcp.McpAuthorizationTests.ToolsList_ShowsOnlyToolsTheRoleAndScopesAllow#Collections differ mcp-get-falls-to-spa#kill#MCP# app.MapMethods(Path, [HttpMethods.Get, HttpMethods.Head], (HttpResponse response) =># app.MapMethods(Path + "/unused", [HttpMethods.Get, HttpMethods.Head], (HttpResponse response) =>#Mcp.McpEndpointTests.GetAndHead_Get405#Expected: MethodNotAllowed shared-check-unchallenged#kill#EPOCH# if (OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint()))# if (OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint()) && false)#Mcp.McpAuthorizationTests.SharedCheck_RefusesTheConnection_WithAChallenge#Strings differ -rpc-429-no-retry-after#kill#LIMITS#context.HttpContext.Response.Headers.RetryAfter =#_ =#Mcp.McpAuthorizationTests.OverTheBudget_Gets429_AsAJsonRpcError#no Retry-After +rpc-429-no-retry-after#kill#LIMITS#context.HttpContext.Response.Headers.RetryAfter =#var unusedRetryAfter =#Mcp.McpAuthorizationTests.OverTheBudget_Gets429_AsAJsonRpcError#no Retry-After tool-walk-declared-only#kill#TOOL_SURFACE# BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance;# BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance | BindingFlags.DeclaredOnly;#Mcp.McpToolSurfaceTests.InheritedToolMethod_IsGatedByItsDeclaringType#Assert.Contains() Failure tool-walk-reflected-type-gates#kill#TOOL_SURFACE#.Concat(method.DeclaringType!.GetCustomAttributes(inherit: true))#.Concat(method.ReflectedType!.GetCustomAttributes(inherit: true))#Mcp.McpToolSurfaceTests.InheritedToolMethod_IsGatedByItsDeclaringType#Assert.Contains() Failure tool-walk-finds-nothing#kill#TOOL_SURFACE#Where(m => HasAttribute(m, "McpServerToolAttribute"))#Where(m => HasAttribute(m, "NoSuchAttribute"))#Mcp.McpEndpointTests.ToolWalk_ReachesEveryToolTheSdkRegisters#Values differ From 38801b35798e7f949682a33cd40cb6ca50606d92 Mon Sep 17 00:00:00 2001 From: mforce Date: Sat, 10 Oct 2026 21:48:37 +0000 Subject: [PATCH 6/6] refactor(mcp): check the endpoint inside AddChallenge CodeQL's cs/user-controlled-bypass read the AcceptsOAuthTokens call in CredentialEpochMiddleware as a sensitive action under the existing logout-path exemption. The check moves into the helper, which every caller wants anyway. --- src/Cluckwork.Api/Hosting/CluckworkMcp.cs | 5 ++++- src/Cluckwork.Api/Middleware/CredentialEpochMiddleware.cs | 3 +-- tools/oauth/mutation-check.sh | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Cluckwork.Api/Hosting/CluckworkMcp.cs b/src/Cluckwork.Api/Hosting/CluckworkMcp.cs index 89af29c1a..5f1f569e0 100644 --- a/src/Cluckwork.Api/Hosting/CluckworkMcp.cs +++ b/src/Cluckwork.Api/Hosting/CluckworkMcp.cs @@ -53,9 +53,12 @@ public static void AddCluckworkMcp(this IServiceCollection services, Authenticat // RFC 6750 §3 and RFC 9728 §5.1: a refused OAuth token is answered with a bearer // challenge naming the protected-resource metadata, which is how an MCP client // re-authorizes. The SDK's handler adds it to a challenge; the shared request checks - // and the scope gate write their own responses and call this. + // and the scope gate write their own responses and call this. Other endpoints take + // session JWTs, which have no protected-resource metadata. public static void AddChallenge(HttpContext context, string parameters) { + if (!OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint())) + return; var metadata = context.RequestServices.GetRequiredService>() .Get(McpAuthenticationDefaults.AuthenticationScheme).ResourceMetadataUri; context.Response.Headers.WWWAuthenticate = $"Bearer {parameters}, resource_metadata=\"{metadata}\""; diff --git a/src/Cluckwork.Api/Middleware/CredentialEpochMiddleware.cs b/src/Cluckwork.Api/Middleware/CredentialEpochMiddleware.cs index 2a0937fdd..e32fe9314 100644 --- a/src/Cluckwork.Api/Middleware/CredentialEpochMiddleware.cs +++ b/src/Cluckwork.Api/Middleware/CredentialEpochMiddleware.cs @@ -57,8 +57,7 @@ public async Task InvokeAsync(HttpContext context, ICredentialEpochVerifier veri }; context.Response.StatusCode = StatusCodes.Status401Unauthorized; // #806 — the token no longer grants access, so an MCP client must re-authorize. - if (OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint())) - CluckworkMcp.AddChallenge(context, "error=\"invalid_token\""); + CluckworkMcp.AddChallenge(context, "error=\"invalid_token\""); context.Response.ContentType = "application/problem+json"; await context.Response.WriteAsJsonAsync(new ProblemDetails { diff --git a/tools/oauth/mutation-check.sh b/tools/oauth/mutation-check.sh index 028e07a81..64d8949c3 100755 --- a/tools/oauth/mutation-check.sh +++ b/tools/oauth/mutation-check.sh @@ -236,7 +236,7 @@ mcp-idempotency-exemption-ignored#kill#IDEMPOTENCY#GetMetadata# app.MapMethods(Path + "/unused", [HttpMethods.Get, HttpMethods.Head], (HttpResponse response) =>#Mcp.McpEndpointTests.GetAndHead_Get405#Expected: MethodNotAllowed -shared-check-unchallenged#kill#EPOCH# if (OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint()))# if (OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint()) && false)#Mcp.McpAuthorizationTests.SharedCheck_RefusesTheConnection_WithAChallenge#Strings differ +shared-check-unchallenged#kill#EPOCH# CluckworkMcp.AddChallenge(context, "error=\"invalid_token\"");\n##Mcp.McpAuthorizationTests.SharedCheck_RefusesTheConnection_WithAChallenge#Strings differ rpc-429-no-retry-after#kill#LIMITS#context.HttpContext.Response.Headers.RetryAfter =#var unusedRetryAfter =#Mcp.McpAuthorizationTests.OverTheBudget_Gets429_AsAJsonRpcError#no Retry-After tool-walk-declared-only#kill#TOOL_SURFACE# BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance;# BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance | BindingFlags.DeclaredOnly;#Mcp.McpToolSurfaceTests.InheritedToolMethod_IsGatedByItsDeclaringType#Assert.Contains() Failure tool-walk-reflected-type-gates#kill#TOOL_SURFACE#.Concat(method.DeclaringType!.GetCustomAttributes(inherit: true))#.Concat(method.ReflectedType!.GetCustomAttributes(inherit: true))#Mcp.McpToolSurfaceTests.InheritedToolMethod_IsGatedByItsDeclaringType#Assert.Contains() Failure