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..84b56abba --- /dev/null +++ b/docs/decisions/806-mcp-endpoint.md @@ -0,0 +1,106 @@ +# 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. 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 +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. + +**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="..."` +(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. 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`, +`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. + +**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 + +- **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..50bb9a35d 100644 --- a/docs/plans/770-mcp-server/02-guards.md +++ b/docs/plans/770-mcp-server/02-guards.md @@ -31,6 +31,14 @@ 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'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 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 | # | Guard | Invariant | Red mutation | @@ -40,6 +48,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 +67,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 +85,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 @@ -85,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/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..5f1f569e0 --- /dev/null +++ b/src/Cluckwork.Api/Hosting/CluckworkMcp.cs @@ -0,0 +1,85 @@ +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 Microsoft.Extensions.Options; +using ModelContextProtocol.AspNetCore; +using ModelContextProtocol.AspNetCore.Authentication; +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", + }; + }); + } + + // 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. 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}\""; + } + + // Either scope admits the request; each tool then requires its own (AuthPolicies). + 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/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/CredentialEpochMiddleware.cs b/src/Cluckwork.Api/Middleware/CredentialEpochMiddleware.cs index 18a324b73..e32fe9314 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,8 @@ 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. + 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 a5d792370..d61f91401 100644 --- a/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs +++ b/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs @@ -1,3 +1,5 @@ +using Cluckwork.Api.Hosting; +using Cluckwork.Api.Modules.Access.OAuth; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Authorization.Policy; using Microsoft.AspNetCore.Mvc; @@ -16,11 +18,25 @@ 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(); + CluckworkMcp.AddChallenge(context, $"error=\"insufficient_scope\", scope=\"{string.Join(' ', scopes)}\""); + } + 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..fcb7cec7d --- /dev/null +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpAuthorizationTests.cs @@ -0,0 +1,363 @@ +using System.Net; +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; +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"); + } + + // 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() + { + 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. The limiter runs before the + // body is read, so the request id is unknown and null. + [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); + 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.ValueKind == JsonValueKind.String && version.GetString() == "2.0" + && body.TryGetProperty("id", out var id) && id.ValueKind == JsonValueKind.Null + && 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) + { + using var response = await PostAsync(host, token, Initialize); + 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 new file mode 100644 index 000000000..a03d8bbd8 --- /dev/null +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpConnection.cs @@ -0,0 +1,169 @@ +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}/{call.IsFlockRestricted}/{string.Join(',', call.AssignedFlockIds.Order())}"; + + [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..544034a95 --- /dev/null +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs @@ -0,0 +1,146 @@ +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 ModelContextProtocol.Server; +using Cluckwork.Api.Mcp; +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); + } + + // 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); + } + + // 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() + { + 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 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 + && 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..e89b3e046 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,10 @@ 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. 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 @@ -23,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) => @@ -58,6 +64,34 @@ 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)) + { + // 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"); + 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. @@ -83,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 46aa3e076..835802b4e 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,44 @@ public void ToolInjectingAmbientIdentityOrAServiceLocator_IsAFinding(Type inject } } + [Fact] + 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")] + [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 +178,54 @@ 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 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/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..64d8949c3 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,14 @@ 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 +TOOL_SURFACE=tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.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=355 # name # expect # file # find # replace # test # declared failure text # ('#' because C# anchors contain '|'; '\n' in a find or replace is a newline) @@ -92,15 +96,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 +225,32 @@ 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(), ##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#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 +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 +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 +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 +345,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" "$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