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..d129ab76b 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 (#806; 120 before) | 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 @@ -83,7 +83,13 @@ ignores any ambient session bearer, which would otherwise resolve a tenant and d 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. + what an MCP client needs. **Decided in #806:** on `/mcp`, a missing, revoked or + wrong-audience token gets the MCP scheme's challenge naming the protected-resource + metadata, and a token with neither MCP scope gets `403` with `error="insufficient_scope"`. + The shared middleware's own 401s (disabled user, suspended farm, superseded credentials, + connected apps off) still carry no header; an MCP client then falls back to the + well-known metadata URL, which the MCP authorization spec requires it to try. A `429` on + `/mcp` is a JSON-RPC error. - **Audit attribution** of the acting client (#788 plans first-class columns). ## How it is enforced diff --git a/docs/decisions/843-adapter-tiers.md b/docs/decisions/843-adapter-tiers.md index ad4e137f8..fa49f046d 100644 --- a/docs/decisions/843-adapter-tiers.md +++ b/docs/decisions/843-adapter-tiers.md @@ -161,3 +161,11 @@ When #806 lands `/mcp` and its tool classes under `Cluckwork.Api.Mcp`: add each tool class's repository reach as an `adapters` row exactly like an endpoint's, and drop `AdapterTierRealTreeTests.McpTierRow_IsDormantToday` — its own failure is the signal that the row it pins is no longer dormant. + +**Amended by #806.** `/mcp` is mapped, so the row is active and +`McpTierRow_IsDormantToday` is gone. The privilege is renamed from +`DirectRepository` to `ContractsOnly`, because since #1123 CW1004 and the reach +guard hold a tool to module contracts like any endpoint: no scanner ever read +`DirectRepository` as an exemption, and a tool cannot inject a repository. The +row's reason now says so, and its `reviewBy` is the epic, #789. #806 ships no +tool class, so no `adapters` row is added yet; the first tools (#807) add theirs. diff --git a/docs/plans/770-mcp-server/01-design.md b/docs/plans/770-mcp-server/01-design.md index c43b97cd6..38e07830f 100644 --- a/docs/plans/770-mcp-server/01-design.md +++ b/docs/plans/770-mcp-server/01-design.md @@ -46,18 +46,21 @@ harnesses; #684/#146 packaging. ### Connecting -No new process and nothing to install. The server is the API already running. +No new process and nothing to install. The server is the API already running, and the +client needs only its URL. ```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 client connects through OAuth, not with a session token. (Corrected in #806: this section +first said a token from `POST /api/v1/auth/login` connects, and `/mcp` refuses every session +JWT.) An unauthenticated request gets a 401 whose `WWW-Authenticate` names +`/.well-known/oauth-protected-resource/mcp`. That document names the farm's authorization +server (#788), where the client registers, the user approves it, and the client receives an +access token for the resource `https://farm.example/mcp`. Farm, role and flock assignments come +from the approving user. The resource URL is derived from `OAuth:Issuer`, so there is **no new +server config key**. ### 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..20a61e6b5 100644 --- a/docs/plans/770-mcp-server/02-guards.md +++ b/docs/plans/770-mcp-server/02-guards.md @@ -23,6 +23,16 @@ matters more than count: prefer "walk everything, exclude deliberately". ## Transport and blast radius +#806 implements rows 8 to 12, 16 to 18, 29 and 30 in `tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs`, +under different names, behind the real pipeline and with the SDK's own client. Row 10's premise is false: +`MapMcp` maps a `RequestDelegate`, which carries no `MethodInfo`, so `BodyReadingEndpointTests` skips `/mcp` +and removing the marker leaves it green (checked by mutation). `/mcp` keeps `ReadsRequestBodyAttribute` +because it does read its body, but no guard holds it. Row 11 is `OAuthServerProductionTests`, which reaches +`/mcp` under Production configuration with no added key. Row 18's exemption is the `McpEndpoint` endpoint metadata rather than a separate +`HandlesOwnIdempotencyAttribute`. Rows 13 to 15 wait for the first tool (#807): with no tool in the tree, +a parity walk has nothing to check. #806 adds the rule rows 13 to 15 build on: every tool names a scope +policy and a role policy (`McpToolSurfaceTests`). + | # | Guard | Invariant | Red mutation | |---|---|---|---| | 8 | `SessionMode_IsStateless` | the actual load-bearing setting | set `SessionMode = Stateful` or `StatefulForInitializeClients`. **Note what is deliberately NOT guarded:** `ScopeRequests`. The SDK forces it under Stateless, so a guard on it reddens on a mutation that leaves the product safe — a false alarm, which all three candidates proposed. | diff --git a/src/Cluckwork.Api/AuthPolicies.cs b/src/Cluckwork.Api/AuthPolicies.cs index 875c6d316..eec0e4e29 100644 --- a/src/Cluckwork.Api/AuthPolicies.cs +++ b/src/Cluckwork.Api/AuthPolicies.cs @@ -1,4 +1,6 @@ +using Cluckwork.Application.Modules.Access.Contracts; using Cluckwork.Domain.Modules.Farm.Contracts; +using OpenIddict.Abstractions; namespace Cluckwork.Api; @@ -56,6 +58,12 @@ 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)); + + // #806 — one policy per OAuth scope, named for it. An MCP tool carries a scope + // policy beside its role policy; stacked [Authorize] attributes AND together, so + // the tool admits role ∩ scope and tools/list hides it from everyone else. + foreach (var scope in OAuthScopes.All) + opts.AddPolicy(scope, p => p.RequireAuthenticatedUser().RequireAssertion(ctx => ctx.User.HasScope(scope))); } private const string WorkerRole = "Worker"; diff --git a/src/Cluckwork.Api/Cluckwork.Api.csproj b/src/Cluckwork.Api/Cluckwork.Api.csproj index d0a821a4a..ce9e595a0 100644 --- a/src/Cluckwork.Api/Cluckwork.Api.csproj +++ b/src/Cluckwork.Api/Cluckwork.Api.csproj @@ -25,6 +25,7 @@ graph transitively, so removing this line leaves the suite green. --> + diff --git a/src/Cluckwork.Api/Hosting/CluckworkIdentityServiceCollectionExtensions.cs b/src/Cluckwork.Api/Hosting/CluckworkIdentityServiceCollectionExtensions.cs index 968d36449..ecaa0c451 100644 --- a/src/Cluckwork.Api/Hosting/CluckworkIdentityServiceCollectionExtensions.cs +++ b/src/Cluckwork.Api/Hosting/CluckworkIdentityServiceCollectionExtensions.cs @@ -19,6 +19,7 @@ using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.Identity; using Microsoft.IdentityModel.Tokens; +using ModelContextProtocol.AspNetCore.Authentication; using OpenIddict.Validation.AspNetCore; namespace Cluckwork.Api.Hosting; @@ -188,13 +189,20 @@ 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. + // Neither token is ever handed to the other's handler. #806 — /mcp goes through + // the MCP scheme, which forwards authentication to OpenIddict and adds the + // protected-resource metadata to its challenge. services.AddAuthentication(options => options.DefaultScheme = BearerSelectorScheme) .AddPolicyScheme(BearerSelectorScheme, displayName: null, options => - options.ForwardDefaultSelector = context => - OAuthEndpoints.AcceptsOAuthTokens(context.GetEndpoint()) - ? OpenIddictValidationAspNetCoreDefaults.AuthenticationScheme - : JwtBearerDefaults.AuthenticationScheme); + options.ForwardDefaultSelector = context => context.GetEndpoint() switch + { + { } endpoint when endpoint.Metadata.GetMetadata() is not null => + McpAuthenticationDefaults.AuthenticationScheme, + { } endpoint when OAuthEndpoints.AcceptsOAuthTokens(endpoint) => + OpenIddictValidationAspNetCoreDefaults.AuthenticationScheme, + _ => JwtBearerDefaults.AuthenticationScheme, + }) + .AddCluckworkMcp(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..d2ef347ea --- /dev/null +++ b/src/Cluckwork.Api/Hosting/CluckworkMcp.cs @@ -0,0 +1,72 @@ +using Cluckwork.Api.Mcp; +using Cluckwork.Api.Modules.Access.OAuth; +using Cluckwork.Application.Modules.Access.Contracts; +using Cluckwork.Infrastructure.Modules.Access.OAuth; +using Microsoft.AspNetCore.Authentication; +using ModelContextProtocol.AspNetCore; +using ModelContextProtocol.Authentication; +using OpenIddict.Validation.AspNetCore; + +namespace Cluckwork.Api.Hosting; + +// #806 — endpoint metadata marking /mcp. The bearer selector sends its challenge through +// the MCP scheme, IdempotencyMiddleware lets its POSTs through without an Idempotency-Key, +// and its 403 and 429 responses take the shape an MCP client reads. +public sealed record McpEndpoint(Uri ResourceMetadata) +{ + public string InsufficientScopeChallenge => + $"Bearer error=\"insufficient_scope\", scope=\"{OAuthScopes.ReadFarm}\", resource_metadata=\"{ResourceMetadata}\""; + + public static McpEndpoint? Of(HttpContext context) => context.GetEndpoint()?.Metadata.GetMetadata(); +} + +internal static class CluckworkMcp +{ + private const string Path = "/mcp"; + + // Every MCP message is one small JSON-RPC object; the largest planned is a daily entry. + private const long MaxRequestBodyBytes = 64 * 1024; + + public static void AddCluckworkMcp(this AuthenticationBuilder authentication, Uri issuer) + { + var resource = OAuthServerRegistration.McpResource(issuer); + // RFC 9728 §3.1: the well-known segment goes between the host and the resource's path. + var resourceMetadata = new Uri( + resource.GetLeftPart(UriPartial.Authority) + "/.well-known/oauth-protected-resource" + resource.AbsolutePath); + + // Stateless is the SDK default; pinned here because the SessionMode is what keeps a + // tool in the HTTP request's DI scope, which McpCallContext checks (#805). + authentication.Services.AddMcpServer() + .WithHttpTransport(options => options.SessionMode = HttpServerSessionMode.Stateless) + .AddAuthorizationFilters() + .WithToolsFromAssembly(typeof(McpCallContext).Assembly); + + // Serves the protected-resource metadata and writes /mcp's 401 challenge. The + // token itself is validated by OpenIddict, as on every AcceptOAuthTokens endpoint. + authentication.AddMcp(options => + { + options.ForwardAuthenticate = OpenIddictValidationAspNetCoreDefaults.AuthenticationScheme; + options.ResourceMetadataUri = resourceMetadata; + options.ResourceMetadata = new ProtectedResourceMetadata + { + Resource = resource.AbsoluteUri, + AuthorizationServers = [issuer.AbsoluteUri], + ScopesSupported = [.. OAuthScopes.All], + ResourceName = "Cluckwork", + }; + }); + + authentication.Services.AddSingleton(new McpEndpoint(resourceMetadata)); + } + + // A tool narrows this with its own role and scope policies; tools/list hides a tool + // whose policies the caller fails. The SDK reads the body itself, so the marker reports + // a failed read as a body error (#398). No guard holds it: the SDK maps a + // RequestDelegate, which BodyReadingEndpointTests cannot see. + public static void MapCluckworkMcp(this WebApplication app) => + app.MapMcp(Path) + .WithMetadata(app.Services.GetRequiredService(), new ReadsRequestBodyAttribute()) + .WithMaxRequestBodyBytes(MaxRequestBodyBytes) + .AcceptOAuthTokens(OAuthScopes.ReadFarm, OAuthScopes.WriteDailyEntries) + .RequireAuthorization(); +} diff --git a/src/Cluckwork.Api/Hosting/CluckworkRateLimitingServiceCollectionExtensions.cs b/src/Cluckwork.Api/Hosting/CluckworkRateLimitingServiceCollectionExtensions.cs index 8d57dcd5f..cbb78b9e3 100644 --- a/src/Cluckwork.Api/Hosting/CluckworkRateLimitingServiceCollectionExtensions.cs +++ b/src/Cluckwork.Api/Hosting/CluckworkRateLimitingServiceCollectionExtensions.cs @@ -94,10 +94,25 @@ public static CluckworkRateLimitingRegistration AddCluckworkRateLimiting( context.HttpContext.Request.Path.Value); } + // #806 — an MCP client reads JSON-RPC, and oauth-api counts per token, not per + // address. The request's id is unknown here, so the error carries none. + if (McpEndpoint.Of(context.HttpContext) is not null) + { + await Results.Json(new + { + jsonrpc = "2.0", + id = (object?)null, + error = new { code = -32000, message = "Too many requests on this connection. Try again later." }, + }, statusCode: StatusCodes.Status429TooManyRequests) + .ExecuteAsync(context.HttpContext); + return; + } + await Results.Problem( title: "Too many requests", - detail: - "Too many requests from this address. Try again later.", + detail: policyName == RateLimitingOptions.OAuthApiPolicyName + ? "Too many requests with this access token. Try again later." + : "Too many requests from this address. Try again later.", statusCode: StatusCodes.Status429TooManyRequests) .ExecuteAsync(context.HttpContext); }; diff --git a/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs b/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs index a5d792370..c16314468 100644 --- a/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs +++ b/src/Cluckwork.Api/Middleware/ForbiddenProblemResultHandler.cs @@ -1,6 +1,9 @@ +using Cluckwork.Api.Hosting; +using Cluckwork.Application.Modules.Access.Contracts; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Authorization.Policy; using Microsoft.AspNetCore.Mvc; +using OpenIddict.Abstractions; namespace Cluckwork.Api.Middleware; @@ -16,11 +19,20 @@ public async Task HandleAsync( { if (authorizeResult.Forbidden) { + // #806 — RFC 6750 §3.1: a token holding none of /mcp's scopes is told which to + // ask for, so an MCP client can reauthorize instead of giving up. + var mcp = McpEndpoint.Of(context) is { } endpoint && !OAuthScopes.All.Any(context.User.HasScope) + ? endpoint + : null; + if (mcp is not null) + context.Response.Headers.WWWAuthenticate = mcp.InsufficientScopeChallenge; context.Response.StatusCode = StatusCodes.Status403Forbidden; await context.Response.WriteAsJsonAsync(new ProblemDetails { Title = "Forbidden", - Detail = "This action requires the Admin role.", + Detail = mcp is null + ? "This action requires the Admin role." + : "This connection was not granted access to farm data.", Status = StatusCodes.Status403Forbidden }); return; diff --git a/src/Cluckwork.Api/Middleware/IdempotencyMiddleware.cs b/src/Cluckwork.Api/Middleware/IdempotencyMiddleware.cs index 8289af69d..b0b554f67 100644 --- a/src/Cluckwork.Api/Middleware/IdempotencyMiddleware.cs +++ b/src/Cluckwork.Api/Middleware/IdempotencyMiddleware.cs @@ -1,5 +1,6 @@ using System.Security.Cryptography; using System.Text; +using Cluckwork.Api.Hosting; using Cluckwork.Infrastructure.Persistence; using Microsoft.AspNetCore.Diagnostics; using Microsoft.EntityFrameworkCore; @@ -134,6 +135,14 @@ public async Task InvokeAsync( return; } + // #806 — MCP sends every message as a POST, initialize and tools/list included, + // and no MCP client sends an Idempotency-Key. A write tool keys its own retries. + if (McpEndpoint.Of(context) is not null) + { + await next(context); + return; + } + if (ResponseNotCacheable.Any(p => context.Request.Path.StartsWithSegments(p, StringComparison.OrdinalIgnoreCase))) { 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..eda9c2dd7 100644 --- a/src/Cluckwork.Api/RateLimiting/RateLimitingOptions.cs +++ b/src/Cluckwork.Api/RateLimiting/RateLimitingOptions.cs @@ -34,7 +34,8 @@ 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 its only endpoint, and every MCP message is one request. + 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..ce8fce87f 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", + "ContractsOnly", "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)", - "#806")] + "MCP tool classes are adapters like the endpoints: CW1004 and the reach guard hold them to module contracts (#1123, #806)", + "#789")] 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..728e02aee 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,17 @@ 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 these tokens are for (RFC 8707), derived from the issuer + // and never from a request's Host. + public static Uri McpResource(Uri issuer) => new(PublicBase(issuer), "mcp"); + + private static Uri PublicBase(Uri issuer) => + issuer.AbsoluteUri.EndsWith('/') ? issuer : new Uri(issuer.AbsoluteUri + "/"); + public static IServiceCollection AddAccessOAuthServer( this IServiceCollection services, Uri issuer, bool allowPlainHttp, ClientMetadataOptions clientMetadata) { + var mcpResource = McpResource(issuer); services.AddOpenIddict() .AddCore(core => core .UseEntityFrameworkCore() @@ -37,10 +46,13 @@ public static IServiceCollection AddAccessOAuthServer( .UseReferenceAccessTokens() .SetAccessTokenLifetime(null) .RegisterScopes([.. OAuthScopes.All]) + // #806 — any other resource is refused with invalid_target. + .RegisterResources(mcpResource) // 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). .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 +117,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 +134,20 @@ public static IServiceCollection AddAccessOAuthServer( server.AddEventHandler(ClientMetadataDocuments.Descriptor); + // #806 — every code, and so every access token, names its resource. A client + // that sent none gets /mcp, the only resource, as the scope default above + // gets farm:read. + server.AddEventHandler(handler => handler + .UseInlineHandler(context => + { + if (context.EndpointType is OpenIddictServerEndpointType.Authorization) + context.Principal!.SetResources(context.Request.GetResources() is { IsEmpty: false } asked + ? asked + : [mcpResource.AbsoluteUri]); + 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() @@ -133,6 +159,8 @@ public static IServiceCollection AddAccessOAuthServer( .AddValidation(validation => { validation.UseLocalServer(); + // #806 — a token issued for another resource, or for none, is refused. + validation.AddAudiences(mcpResource.AbsoluteUri); validation.UseDataProtection(); // Disconnect revokes the authorization, so every request checks it (#796). validation.EnableAuthorizationEntryValidation(); diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs new file mode 100644 index 000000000..b4036d110 --- /dev/null +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpEndpointTests.cs @@ -0,0 +1,439 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Text; +using System.Text.Json; +using Cluckwork.Api.Hosting; +using Cluckwork.Api.IntegrationTests.Infrastructure; +using Cluckwork.Api.Mcp; +using Cluckwork.Application.Modules.Access.Contracts; +using Cluckwork.Domain.Modules.Farm.Contracts; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.TestHost; +using Microsoft.AspNetCore.WebUtilities; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Options; +using ModelContextProtocol; +using ModelContextProtocol.AspNetCore; +using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; +using OpenIddict.Abstractions; +using OpenIddict.Server; +using Xunit.Abstractions; +using static OpenIddict.Abstractions.OpenIddictConstants; + +namespace Cluckwork.Api.IntegrationTests.Mcp; + +// #806 — /mcp behind the real pipeline, driven by the SDK's own client where a session +// matters. Production has no tools yet, so the test host adds ProbeTools: one read tool and +// one write tool, authorized the way a real tool is (a scope policy, and a role policy). +[Collection(IntegrationCollection.Name)] +public sealed class McpEndpointTests(CluckworkWebApplicationFactory factory, ITestOutputHelper output) +{ + private const string Resource = "https://localhost/mcp"; + private const string MetadataUrl = "https://localhost/.well-known/oauth-protected-resource/mcp"; + private const string OtherResource = "https://other.example/api"; + private const string OtherScope = "cw806.other"; + private const string Initialize = """ + {"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"cw806","version":"1"}}} + """; + + [Fact] + public async Task NoToken_GetsAChallengeNamingTheResourceMetadata() + { + using var host = Host(); + + using var response = await PostAsync(host, token: null); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + Assert.Equal($"Bearer resource_metadata=\"{MetadataUrl}\"", Assert.Single(response.Headers.WwwAuthenticate).ToString()); + } + + // The client follows the challenge to the metadata, and the metadata to the issuer's + // discovery document, whose issuer must match it byte for byte. + [Fact] + public async Task Metadata_LeadsToTheAuthorizationServer() + { + using var host = Host(); + using var client = OAuthServerTests.HttpsClient(host, bearer: null); + + var metadata = await client.GetFromJsonAsync(MetadataUrl); + var issuer = Assert.Single(metadata.GetProperty("authorization_servers").EnumerateArray()).GetString(); + var discovery = await client.GetFromJsonAsync( + new Uri(new Uri(issuer!), ".well-known/oauth-authorization-server")); + + Assert.Equal(Resource, metadata.GetProperty("resource").GetString()); + Assert.Equal(OAuthScopes.All, metadata.GetProperty("scopes_supported").EnumerateArray().Select(s => s.GetString())); + Assert.Equal(issuer, discovery.GetProperty("issuer").GetString()); + } + + [Fact] + public async Task SessionJwt_IsRefused() + { + using var host = Host(); + var user = await SeedAsync(Roles.Manager); + + using var response = await PostAsync(host, user.Jwt); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + // One endpoint serves both revisions #789 targets. The SDK's client sends no + // Idempotency-Key, and the tool body resolves McpCallContext in the request's scope. + [Theory] + [InlineData(null)] + [InlineData("2025-11-25")] + public async Task BothRevisions_ListAndCallATool(string? protocolVersion) + { + using var host = Host(); + var user = await SeedAsync(Roles.Manager); + var token = await ConnectAsync(host, user.Jwt, OAuthScopes.ReadFarm); + var counter = new RequestCounter(); + + await using (var client = await McpAsync(host, token, protocolVersion, counter)) + { + var tools = await client.ListToolsAsync(); + var result = await client.CallToolAsync(ProbeTools.Read); + + Assert.Contains(tools, tool => tool.Name == ProbeTools.Read); + Assert.Equal($"{user.Id} {user.AccountId}", Assert.IsType(Assert.Single(result.Content)).Text); + } + + output.WriteLine($"{protocolVersion ?? "latest"}: {counter.Count} HTTP requests to connect, list and call one tool"); + } + + // Stacked [Authorize] attributes: a tool is listed only for role ∩ scope. + [Theory] + [InlineData(Roles.Manager, OAuthScopes.ReadFarm, new[] { ProbeTools.Read })] + [InlineData(Roles.Manager, OAuthScopes.WriteDailyEntries, new[] { ProbeTools.Write })] + [InlineData(Roles.Manager, OAuthScopes.ReadFarm + " " + OAuthScopes.WriteDailyEntries, new[] { ProbeTools.Read, ProbeTools.Write })] + [InlineData(Roles.ReadOnly, OAuthScopes.ReadFarm + " " + OAuthScopes.WriteDailyEntries, new[] { ProbeTools.Read })] + public async Task ToolsList_IsFilteredByRoleAndScope(string role, string scope, string[] expected) + { + using var host = Host(); + var user = await SeedAsync(role); + var token = await ConnectAsync(host, user.Jwt, scope); + + await using var client = await McpAsync(host, token); + var tools = await client.ListToolsAsync(); + + Assert.Equal(expected, tools.Select(tool => tool.Name).Order()); + } + + [Fact] + public async Task UnlistedTool_IsRefusedWhenCalled() + { + using var host = Host(); + var user = await SeedAsync(Roles.Manager); + var token = await ConnectAsync(host, user.Jwt, OAuthScopes.ReadFarm); + + await using var client = await McpAsync(host, token); + var refusal = await Assert.ThrowsAsync(() => client.CallToolAsync(ProbeTools.Write).AsTask()); + + Assert.Equal(McpErrorCode.InvalidRequest, refusal.ErrorCode); + } + + // RFC 8707: a client that names no resource still gets one, the only one there is. + [Fact] + public async Task TokenRequestedWithoutAResource_IsBoundToMcp() + { + using var host = Host(); + var user = await SeedAsync(Roles.Manager); + var token = await ConnectAsync(host, user.Jwt, OAuthScopes.ReadFarm, resource: null); + + await using var client = await McpAsync(host, token); + Assert.Contains(await client.ListToolsAsync(), tool => tool.Name == ProbeTools.Read); + } + + [Fact] + public async Task AuthorizationRequest_ForAnotherResource_IsRefused() + { + using var host = Host(); + var user = await SeedAsync(Roles.Manager); + var clientId = await OAuthServerTests.RegisterClientAsync(host.Services); + var query = OAuthServerTests.AuthorizeQuery(clientId, OAuthServerTests.NewCodeVerifier()); + query["resource"] = OtherResource; + + using var response = await OAuthServerTests.SendAuthorizeAsync(host, user.Jwt, query); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + Assert.Contains($"error:{Errors.InvalidTarget}", await response.Content.ReadAsStringAsync()); + } + + [Fact] + public async Task TokenForAnotherResource_IsRefused() + { + using var host = Host(server => server.Resources.Add(new Uri(OtherResource))); + var user = await SeedAsync(Roles.Manager); + var token = await ConnectAsync(host, user.Jwt, OAuthScopes.ReadFarm, OtherResource); + + using var response = await PostAsync(host, token); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + [Fact] + public async Task TokenWithNoAudience_IsRefused() + { + using var host = Host(server => server.Handlers.Add(AfterResourcesAreSet(principal => principal.SetResources([])))); + var user = await SeedAsync(Roles.Manager); + var token = await ConnectAsync(host, user.Jwt, OAuthScopes.ReadFarm); + + using var response = await PostAsync(host, token); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + // The route's own gate. Real tokens always hold farm:read or daily-entries:write, + // so the test host registers a third scope. + [Fact] + public async Task TokenWithNoMcpScope_GetsInsufficientScope() + { + using var host = Host(server => server.Scopes.Add(OtherScope)); + var user = await SeedAsync(Roles.Manager); + var token = await ConnectAsync(host, user.Jwt, OtherScope); + + using var response = await PostAsync(host, token); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + Assert.Equal( + $"Bearer error=\"insufficient_scope\", scope=\"{OAuthScopes.ReadFarm}\", resource_metadata=\"{MetadataUrl}\"", + Assert.Single(response.Headers.WwwAuthenticate).ToString()); + } + + // Disconnect: the next request is challenged, so the client knows to reconnect. + [Fact] + public async Task RevokedToken_IsChallenged() + { + using var host = Host(); + var user = await SeedAsync(Roles.Manager); + var token = await ConnectAsync(host, user.Jwt, OAuthScopes.ReadFarm); + + 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); + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + Assert.Contains(MetadataUrl, response.Headers.WwwAuthenticate.ToString()); + } + + [Fact] + public async Task ConnectedAppsOff_IsRefused() + { + using var host = Host(); + var user = await SeedAsync(Roles.Manager); + var token = await ConnectAsync(host, user.Jwt, OAuthScopes.ReadFarm); + + await factory.WithTenantScopeAsync(user.AccountId, db => db.Accounts.Where(a => a.Id == user.AccountId) + .ExecuteUpdateAsync(setters => setters.SetProperty(a => a.AllowConnectedApps, false))); + + using var response = await PostAsync(host, token); + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + Assert.Equal("Auth.ConnectedAppsOff", await TitleOf(response)); + } + + // Consent refuses a user with a pending password change, so no real token carries the + // claim; the test host adds it to prove the gate covers /mcp too. + [Fact] + public async Task MustChangePassword_IsRefused() + { + using var host = Host(server => server.Handlers.Add(AfterResourcesAreSet(principal => + { + principal.SetClaim("must_change_password", "true"); + principal.SetDestinations(static _ => [Destinations.AccessToken]); + }))); + var user = await SeedAsync(Roles.Manager); + var token = await ConnectAsync(host, user.Jwt, OAuthScopes.ReadFarm); + + using var response = await PostAsync(host, token); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + Assert.Equal("Auth.MustChangePassword", await TitleOf(response)); + } + + [Fact] + public async Task RateLimitRefusal_IsAJsonRpcError() + { + using var host = factory.WithWebHostBuilder(builder => + { + builder.UseSetting("RateLimiting:OAuthApi:PermitLimit", "1"); + builder.UseSetting("RateLimiting:OAuthApi:WindowSeconds", "86400"); + }); + var user = await SeedAsync(Roles.Manager); + var token = await ConnectAsync(host, user.Jwt, OAuthScopes.ReadFarm); + + using var first = await PostAsync(host, token); + using var second = await PostAsync(host, token); + + Assert.Equal(HttpStatusCode.OK, first.StatusCode); + Assert.Equal(HttpStatusCode.TooManyRequests, second.StatusCode); + Assert.NotNull(second.Headers.RetryAfter); + var body = await second.Content.ReadFromJsonAsync(); + Assert.Equal("2.0", body.GetProperty("jsonrpc").GetString()); + Assert.Equal(JsonValueKind.Null, body.GetProperty("id").ValueKind); + Assert.Contains("this connection", body.GetProperty("error").GetProperty("message").GetString()); + } + + [Fact] + public async Task OversizedBody_IsRefused() + { + using var host = Host(); + var user = await SeedAsync(Roles.Manager); + var token = await ConnectAsync(host, user.Jwt, OAuthScopes.ReadFarm); + var padding = new string(' ', 64 * 1024); + + using var response = await PostAsync(host, token, Initialize + padding); + + Assert.Equal(HttpStatusCode.RequestEntityTooLarge, response.StatusCode); + } + + // Design row 17: the idempotency exemption is metadata, and only /mcp carries it. + [Fact] + public void OnlyTheMcpPost_CarriesTheMcpMarker() + { + var marked = factory.Services.GetRequiredService().Endpoints.OfType() + .Where(endpoint => endpoint.Metadata.GetMetadata() is not null) + .Select(endpoint => $"{string.Join(",", endpoint.Metadata.GetMetadata()?.HttpMethods ?? [])} {endpoint.RoutePattern.RawText}"); + + Assert.Equal(["POST /mcp/"], marked); + } + + // Design rows 8, 9 and 12. Stateless keeps a tool in the request's scope (#805); + // ConfigureSessionOptions is the one hook that runs after the SDK's stateless setup. + // The idle-session service is registered regardless and must never start its timer. + [Fact] + public void Transport_IsStatelessAndKeepsNoSessions() + { + var transport = factory.Services.GetRequiredService>().Value; + var idleTracking = Assert.Single(factory.Services.GetServices(), + service => service.GetType().Name == "IdleTrackingBackgroundService"); + + Assert.Equal(HttpServerSessionMode.Stateless, transport.SessionMode); + Assert.Null(transport.ConfigureSessionOptions); + Assert.Null(Assert.IsAssignableFrom(idleTracking).ExecuteTask); + } + + private sealed record SeededUser(Guid Id, Guid AccountId, string Jwt); + + private async Task SeedAsync(string? role) + { + var email = $"mcp-{Guid.NewGuid():N}@test.local"; + var accountId = await factory.SeedAccountWithUserAsync($"mcp-owner-{Guid.NewGuid():N}@test.local"); + 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)); + } + + // A connection the way an MCP client makes one: authorize, then redeem, both naming + // the resource. + private static async Task ConnectAsync( + WebApplicationFactory host, string jwt, string scope, string? resource = Resource) + { + var clientId = await OAuthServerTests.RegisterClientAsync(host.Services); + var verifier = OAuthServerTests.NewCodeVerifier(); + var query = OAuthServerTests.AuthorizeQuery(clientId, verifier); + query["scope"] = scope; + var redeem = new Dictionary + { + ["grant_type"] = GrantTypes.AuthorizationCode, + ["client_id"] = clientId, + ["redirect_uri"] = OAuthServerTests.RedirectUri, + ["code_verifier"] = verifier, + }; + if (resource is not null) + query["resource"] = redeem["resource"] = resource; + + var location = await OAuthServerTests.ApproveAsync(host, jwt, query); + redeem["code"] = QueryHelpers.ParseQuery(location.Query)["code"].ToString(); + using var response = await OAuthServerTests.HttpsClient(host, bearer: null) + .PostAsync("/api/v1/oauth/token", new FormUrlEncodedContent(redeem)); + response.EnsureSuccessStatusCode(); + return (await response.Content.ReadFromJsonAsync()).GetProperty("access_token").GetString()!; + } + + private static Task McpAsync( + WebApplicationFactory host, string token, string? protocolVersion = null, RequestCounter? counter = null) + { + var http = host.CreateDefaultClient(new Uri("https://localhost"), counter ?? new RequestCounter()); + http.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token); + return McpClient.CreateAsync( + new HttpClientTransport(new HttpClientTransportOptions { Endpoint = new Uri(Resource) }, http, ownsHttpClient: true), + new McpClientOptions { ProtocolVersion = protocolVersion }); + } + + private static Task PostAsync( + WebApplicationFactory host, string? token, string body = Initialize) + { + var request = new HttpRequestMessage(HttpMethod.Post, "/mcp") + { + Content = new StringContent(body, Encoding.UTF8, "application/json"), + }; + request.Headers.Accept.ParseAdd("application/json"); + request.Headers.Accept.ParseAdd("text/event-stream"); + request.Headers.Add("MCP-Protocol-Version", "2025-11-25"); + return OAuthServerTests.HttpsClient(host, token).SendAsync(request); + } + + private static async Task TitleOf(HttpResponseMessage response) => + (await response.Content.ReadFromJsonAsync()).GetProperty("title").GetString(); + + // Runs after the server stamps a code's resources, before the code is written. + private static OpenIddictServerHandlerDescriptor AfterResourcesAreSet(Action change) => + OpenIddictServerHandlerDescriptor.CreateBuilder() + .UseInlineHandler(context => + { + if (context.EndpointType is OpenIddictServerEndpointType.Authorization) + change(context.Principal!); + return default; + }) + .SetOrder(OpenIddictServerHandlers.InferResources.Descriptor.Order + 600) + .SetType(OpenIddictServerHandlerType.Custom) + .Build(); + + private WebApplicationFactory Host(Action? server = null) => + factory.WithWebHostBuilder(builder => builder.ConfigureTestServices(services => + { + services.AddMcpServer().WithTools(); + if (server is not null) + services.Configure(server); + })); + + private sealed class RequestCounter : DelegatingHandler + { + private int _count; + + public int Count => _count; + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + Interlocked.Increment(ref _count); + return base.SendAsync(request, cancellationToken); + } + } + + [McpServerToolType] + internal sealed class ProbeTools(McpCallContext call) + { + public const string Read = "probe_read"; + public const string Write = "probe_write"; + + [McpServerTool(Name = Read, ReadOnly = true)] + [Authorize(Policy = OAuthScopes.ReadFarm)] + public string ReadCaller() => $"{call.UserId} {call.AccountId}"; + + [McpServerTool(Name = Write)] + [Authorize(Policy = OAuthScopes.WriteDailyEntries)] + [Authorize(Policy = AuthPolicies.ProductionWrite)] + public string WriteNothing() => call.ConnectedApp.ClientId; + } +} diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.cs index 3954263c8..fedce944d 100644 --- a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.cs +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurface.cs @@ -1,6 +1,8 @@ using System.Reflection; using Cluckwork.Api.Mcp; +using Cluckwork.Application.Modules.Access.Contracts; using Cluckwork.Infrastructure.Persistence; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; @@ -8,8 +10,8 @@ namespace Cluckwork.Api.IntegrationTests.Mcp; // #805 rows 6 and 7 — what a tool may inject. A tool is a type carrying an attribute // NAMED McpServerToolTypeAttribute, and a tool method one carrying McpServerToolAttribute. -// Matched by name because the SDK arrives with #806; AdapterTierScanner matches the -// same name, and the fixtures here declare look-alikes to prove the walk goes red. +// Matched by name, as AdapterTierScanner matches it, so the fixtures here can declare +// look-alikes to prove the walk goes red. internal static class McpToolSurface { // A supertype is banned too: ICurrentUser is CurrentUserContext's port and DbContext @@ -58,6 +60,22 @@ public static IReadOnlyList Findings(IEnumerable types) return findings; } + // #806 — every tool method names a scope policy and a role policy, on itself or its + // type. The SDK reads [Authorize] from both and ANDs them, so the tool admits role ∩ + // scope; a bare [Authorize] is the default policy, which is a role policy. + public static IReadOnlyList AuthorizationFindings(IEnumerable types) => + ToolTypes(types).SelectMany(tool => ToolMethods(tool).SelectMany(method => + { + var policies = tool.GetCustomAttributes(inherit: true).Concat(method.GetCustomAttributes(inherit: true)) + .OfType().Select(a => a.Policy).ToList(); + var findings = new List(); + if (!policies.Any(p => p is not null && OAuthScopes.All.Contains(p))) + findings.Add($"{tool.FullName}.{method.Name} names no scope policy"); + if (!policies.Any(p => p is null || !OAuthScopes.All.Contains(p))) + findings.Add($"{tool.FullName}.{method.Name} names no role policy"); + return findings; + })).ToList(); + // #805 row 7b, the namespace half: no type under the MCP namespace declares a scope // opener, as a constructor or method parameter, return type, field or property. A // lambda taking IServiceProvider compiles to a method on a nested type, so it counts. diff --git a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurfaceTests.cs b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurfaceTests.cs index 46aa3e076..94ea52b4d 100644 --- a/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurfaceTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/Mcp/McpToolSurfaceTests.cs @@ -1,15 +1,17 @@ using System.Reflection; using Cluckwork.Api.Mcp; using Cluckwork.Application.Common; +using Cluckwork.Application.Modules.Access.Contracts; using Cluckwork.Infrastructure.Persistence; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; namespace Cluckwork.Api.IntegrationTests.Mcp; -// #805 rows 6, 7 and the namespace half of 7b. The real tree holds no tool until #806, -// so the fixtures below carry the red mutations. +// #805 rows 6, 7 and the namespace half of 7b, and #806's scope-and-role rule. The real +// tree holds no tool until #807, so the fixtures below carry the red mutations. public sealed class McpToolSurfaceTests { private static readonly Type[] ApiTypes = typeof(McpCallContext).Assembly.GetTypes(); @@ -27,6 +29,21 @@ public void RealTree_NoTypeUnderTheMcpNamespace_DeclaresAScopeOpener() AssertNone(McpToolSurface.ScopeOpenerDeclarations(mcpTypes)); } + [Fact] + public void RealTree_EveryToolNamesAScopeAndARolePolicy() => + AssertNone(McpToolSurface.AuthorizationFindings(ApiTypes)); + + [Fact] + public void ToolsNamingAScopeAndARole_HaveNoAuthorizationFindings() => + Assert.Empty(McpToolSurface.AuthorizationFindings([typeof(ScopedTool), typeof(TypeScopedTool)])); + + [Theory] + [InlineData(typeof(ConstructorTool), "no scope policy", "no role policy")] + [InlineData(typeof(RoleOnlyTool), "no scope policy")] + [InlineData(typeof(ScopeOnlyTool), "no role policy")] + public void ToolMissingAScopeOrARole_IsAnAuthorizationFinding(Type tool, params string[] expected) => + Assert.Equal(expected, McpToolSurface.AuthorizationFindings([tool]).Select(f => f[(f.IndexOf(" names ") + 7)..])); + [Fact] public void ToolsTakingTheContext_HaveNoFindings() => Assert.Empty(McpToolSurface.Findings([typeof(ConstructorTool), typeof(StaticTool)])); @@ -104,6 +121,40 @@ private static class StaticTool public static string Read(McpCallContext call) => call.Email; } + [McpServerToolType] + private sealed class ScopedTool(McpCallContext call) + { + [McpServerTool] + [Authorize(Policy = OAuthScopes.ReadFarm)] + [Authorize] + public string Read() => call.Email; + } + + [McpServerToolType] + [Authorize(Policy = AuthPolicies.ProductionWrite)] + private sealed class TypeScopedTool(McpCallContext call) + { + [McpServerTool] + [Authorize(Policy = OAuthScopes.WriteDailyEntries)] + public string Write() => call.Email; + } + + [McpServerToolType] + private sealed class RoleOnlyTool(McpCallContext call) + { + [McpServerTool] + [Authorize(Policy = AuthPolicies.SalesAccess)] + public string Read() => call.Email; + } + + [McpServerToolType] + private sealed class ScopeOnlyTool(McpCallContext call) + { + [McpServerTool] + [Authorize(Policy = OAuthScopes.ReadFarm)] + public string Read() => call.Email; + } + [McpServerToolType] private sealed class BareTool { diff --git a/tests/Cluckwork.Api.IntegrationTests/OAuthServerProductionTests.cs b/tests/Cluckwork.Api.IntegrationTests/OAuthServerProductionTests.cs index 5e88da143..e6919439a 100644 --- a/tests/Cluckwork.Api.IntegrationTests/OAuthServerProductionTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/OAuthServerProductionTests.cs @@ -58,8 +58,20 @@ 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 initialize = new HttpRequestMessage(HttpMethod.Post, "/mcp") + { + Content = new StringContent( + """{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"cw806","version":"1"}}}""", + System.Text.Encoding.UTF8, "application/json"), + }; + initialize.Headers.Accept.ParseAdd("application/json"); + initialize.Headers.Accept.ParseAdd("text/event-stream"); + using var mcp = await OAuthServerTests.HttpsClient(factory, accessToken).SendAsync(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..a5ff912a0 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, "ContractsOnly", "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..c20ae2ccf 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 = "ContractsOnly", 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(\"\", \"ContractsOnly\", " + "\"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("ContractsOnly", StringComparison.Ordinal)); } [Fact] diff --git a/tests/Cluckwork.Application.Tests/Architecture/ModuleLedger.cs b/tests/Cluckwork.Application.Tests/Architecture/ModuleLedger.cs index c2113cfb4..a42e32209 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 ContractsOnlyPrivilege = "ContractsOnly"; // 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"] = ContractsOnlyPrivilege, }; } diff --git a/tests/Cluckwork.Application.Tests/TenantBypass/BypassAllowList.cs b/tests/Cluckwork.Application.Tests/TenantBypass/BypassAllowList.cs index 067c64103..78530ec6c 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 = "3892826c", 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()