Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions architecture.d/mcp-readonly-oauth-1103.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"reason": "signedFileRequestForRequirements is the one requirements-owned rule for which signed file-action subsets an application may request; both authorization request routes and the planner tests use it. The reconciliation predicate replaces an exact-match check and the now-internal fileCapabilityForRequirements export, so net growth is one declaration.",
"growth": {
"typeScriptExportDeclarations": 1
}
}
3 changes: 3 additions & 0 deletions changelog.d/mcp-readonly-oauth-1103.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## Fixed

- Read-only (`mdbase:read`) MCP authorization no longer fails with HTTP 500 before consent. Connect now accepts a signed application request that selects a subset of its declared optional file actions (every required action, declared order, same scope), and keeps exactly that signed subset as the ceiling for consent, approval and the provider grant, so an approval cannot add file actions the application did not sign. Periodic application reconciliation no longer retires grants whose approved file actions are a permitted subset of the declaration (#1103).
231 changes: 231 additions & 0 deletions services/mcp/src/readonly-authorization.real-server.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,231 @@
// Read-only MCP authorization against the real Connect control plane (#1103).
// The MCP gateway signs only the file actions paired with its requested
// operations; the existing gateway tests use a fake upstream that accepts any
// proof, so they never compared that signature with the server's expectation.
// Only the hosted storage provider (a separate Rust service) is a test double.
import { afterEach, expect, it, vi } from "vitest";
import type { FileCapability } from "@mdbase-dev/connect-protocol";
import { buildApp as buildConnectServer } from "../../server/src/app.js";
import { createDatabase as createConnectDatabase } from "../../server/src/db.js";
import type { HostedProviderClient, HostedReplicaEnrollment } from "../../server/src/hosted-provider.js";
import { reconcileApplicationGrants } from "../../server/src/features/grants/service.js";
import type { RelayHub } from "../../server/src/relay.js";
import { buildApp } from "./app.js";
import { createDatabase } from "./db.js";
import type { McpRuntimeConfig } from "./config.js";
import { READ_OPERATIONS } from "./oauth.js";
import { SecretBox, pkceChallenge } from "./security.js";

const CONNECT = "http://connect.test";
const resources: Array<() => Promise<unknown>> = [];
afterEach(async () => {
vi.unstubAllGlobals();
while (resources.length) await resources.pop()!();
});

it("completes read-only (mdbase:read) MCP authorization and consent with exactly the signed file actions", async () => {
const enrolled: HostedReplicaEnrollment[] = [];
const hostedProvider = {
url: "https://sync.example",
ready: vi.fn(),
assertFreshV2AuthorizationSupport: vi.fn(),
upsertAccount: vi.fn().mockResolvedValue({}),
createCollection: vi.fn(),
collectionContracts: vi.fn().mockResolvedValue([]),
provisionApplicationSetup: vi.fn(),
registerReplica: vi.fn(async (_collectionId: string, replica: HostedReplicaEnrollment) => {
// The provider admits a replica only when its file capability is within
// the signed request (connect-hosted-provider verified_setup_evidence).
const signed = replica.applicationAuthorization?.binding.requested_files;
const files = replica.fileCapability as FileCapability | undefined;
if (files && !files.actions.every((action) => signed?.actions.includes(action))) {
throw new Error("application_declaration_mismatch");
}
enrolled.push(replica);
}),
updateApplicationReplica: vi.fn(),
rotateReplicaToken: vi.fn(),
revokeReplica: vi.fn(),
upsertNotificationGrant: vi.fn(),
revokeNotificationGrant: vi.fn(),
compactThrough: vi.fn()
} as unknown as HostedProviderClient;
const connectDb = await createConnectDatabase("memory");
resources.push(() => connectDb.end());
const { app: connect } = await buildConnectServer({
db: connectDb,
devAuth: true,
hostedCollections: true,
hostedProvider,
publicUrl: CONNECT
});
resources.push(() => connect.close());

const realFetch = globalThis.fetch;
vi.stubGlobal("fetch", async (input: string | URL | Request, init?: RequestInit) => {
const url = new URL(typeof input === "string" || input instanceof URL ? input : input.url);
if (url.origin !== CONNECT) return realFetch(input, init);
const response = await connect.inject({
method: (init?.method ?? "GET") as "GET" | "POST",
url: `${url.pathname}${url.search}`,
headers: Object.fromEntries(new Headers(init?.headers)),
...(init?.body === undefined ? {} : { payload: String(init.body) })
});
return new Response(response.body, {
status: response.statusCode,
headers: { "content-type": String(response.headers["content-type"] ?? "application/json") }
});
});

const mcpDb = await createDatabase("memory");
resources.push(() => mcpDb.end());
const { app: mcp } = await buildApp({ db: mcpDb, config: mcpConfig() });
resources.push(() => mcp.close());

const session = await connect.inject({
method: "POST",
url: "/v1/dev/session",
payload: { name: "Owner", email: "mcp-readonly-owner@example.test" }
});
const setCookie = session.headers["set-cookie"]!;
const cookie = (Array.isArray(setCookie) ? setCookie[0]! : setCookie).split(";")[0]!;
const created = await connect.inject({
method: "POST",
url: "/v1/hosted/collections",
headers: { cookie },
payload: { display_name: "Read-only MCP notes", template: "mdbase", timezone: "UTC" }
});
expect(created.statusCode, created.body).toBe(201);
const collectionId = created.json().collection.id as string;

const registration = await mcp.inject({
method: "POST",
url: "/oauth/register",
payload: {
client_name: "Read-only MCP host",
redirect_uris: ["https://client.example/callback"],
token_endpoint_auth_method: "none"
}
});
expect(registration.statusCode).toBe(201);
const clientId = registration.json().client_id as string;
const verifier = "read-only-host-pkce-verifier-long-enough-for-s256-01";
const authorize = await mcp.inject({
method: "GET",
url: `/oauth/authorize?${new URLSearchParams({
response_type: "code",
client_id: clientId,
redirect_uri: "https://client.example/callback",
code_challenge: pkceChallenge(verifier),
code_challenge_method: "S256",
state: "read-only-host-state",
scope: "mdbase:read",
resource: "https://mcp.example/mcp"
})}`
});
// Before #1103 the real server refused the read-only proof here with
// durable_mutation_unsupported, surfacing as an HTTP 500 from MCP.
expect(authorize.statusCode, authorize.body).toBe(302);
const upstream = new URL(authorize.headers.location!);
expect(upstream.origin).toBe(CONNECT);
expect(upstream.pathname).toBe("/oauth/authorize");
const requestId = upstream.searchParams.get("request_id")!;

const pending = await connectDb.query<{ application_authorization: { binding: Record<string, any> } }>(
"SELECT application_authorization FROM authorization_requests WHERE id = $1",
[requestId]
);
const binding = pending.rows[0]!.application_authorization.binding;
expect(binding.requested_operations).toEqual([...READ_OPERATIONS]);
expect(binding.requested_files).toEqual({ actions: ["list", "read"], scope: { kind: "collection" } });
expect(binding.contracts).not.toHaveProperty("durable_mutation");

const claimed = await connect.inject({ method: "GET", url: `${upstream.pathname}${upstream.search}`, headers: { cookie } });
expect(claimed.statusCode, claimed.body).toBe(302);
const consent = await connect.inject({ method: "GET", url: `/v1/authorization-requests/${requestId}`, headers: { cookie } });
expect(consent.statusCode, consent.body).toBe(200);
const offered = consent.json().collections.find((candidate: { id: string }) => candidate.id === collectionId);
// Consent offers the signed subset, not the declaration's optional writes.
expect(offered.authorization).toEqual({
available: true,
operations: [...READ_OPERATIONS],
file_actions: ["list", "read"]
});

// Approve as staging extended acceptance does: the requested operations,
// no explicit file choice, no contract setup.
const approved = await connect.inject({
method: "POST",
url: `/v1/authorization-requests/${requestId}/approve`,
headers: { cookie },
payload: {
collection_id: collectionId,
operations: consent.json().authorization.requested_operations,
contract_setups: []
}
});
expect(approved.statusCode, approved.body).toBe(200);
expect(enrolled).toHaveLength(1);
expect(enrolled[0]).toMatchObject({
mode: "read_only",
fileCapability: { actions: ["list", "read"], scope: { kind: "collection" } }
});
// A write file action cannot be approved beyond what the application signed.
const grants = await connectDb.query<{ id: string; file_capability: FileCapability }>(
"SELECT id, file_capability FROM grants WHERE hosted_collection_id = $1 AND revoked_at IS NULL",
[collectionId]
);
expect(grants.rows).toHaveLength(1);
expect(grants.rows[0]!.file_capability.actions).toEqual(["list", "read"]);

const status = await connect.inject({ method: "GET", url: `/v1/authorization-requests/${requestId}/status`, headers: { cookie } });
expect(status.json().status).toBe("approved");
const callback = new URL(status.json().redirect_uri);
expect(`${callback.origin}${callback.pathname}`).toBe("https://mcp.example/oauth/connect/callback");
const completed = await mcp.inject({ method: "GET", url: `${callback.pathname}${callback.search}` });
expect(completed.statusCode, completed.body).toBe(302);
const hostCallback = new URL(completed.headers.location!);
expect(hostCallback.searchParams.get("state")).toBe("read-only-host-state");
const token = await mcp.inject({
method: "POST",
url: "/oauth/token",
headers: { "content-type": "application/x-www-form-urlencoded" },
payload: new URLSearchParams({
grant_type: "authorization_code",
code: hostCallback.searchParams.get("code")!,
client_id: clientId,
redirect_uri: "https://client.example/callback",
code_verifier: verifier,
resource: "https://mcp.example/mcp"
}).toString()
});
expect(token.statusCode, token.body).toBe(200);
expect(token.json().scope).toBe("mdbase:read");

// Periodic application reconciliation keeps the approved subset; it is a
// permitted selection, not a manifest change.
const application = await connectDb.query(
"SELECT id, family_identity, manifest_digest, requirements, notifications FROM applications WHERE id = $1",
[binding.application_id]
);
await reconcileApplicationGrants(
connectDb,
{ pushPolicy: vi.fn() } as unknown as RelayHub,
hostedProvider,
application.rows[0]
);
const retained = await connectDb.query("SELECT revoked_at FROM grants WHERE id = $1", [grants.rows[0]!.id]);
expect(retained.rows[0].revoked_at).toBeNull();
});

function mcpConfig(): McpRuntimeConfig {
return {
host: "127.0.0.1",
port: 8790,
publicUrl: "https://mcp.example",
connectUrl: CONNECT,
resource: "https://mcp.example/mcp",
masterKey: new SecretBox(Buffer.alloc(32, 7)),
trustProxy: false
};
}
20 changes: 20 additions & 0 deletions services/server/src/application-requirements.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,23 @@ export function fileRequestForRequirements(requirements: ApplicationRequirements
if ("actions" in files) throw new Error("Semantic v2 requires required/optional file actions.");
return applicationFileRequest(files);
}

/**
* The file request an application may sign: its declaration in declared order,
* narrowed to the optional actions it actually requested. Semantic v1 has no
* optional actions, so only the exact declaration is permitted. When `signed`
* omits a required action, the full declaration is returned and the exact proof
* comparison refuses it; this never widens beyond the declaration.
*/
export function signedFileRequestForRequirements(
requirements: ApplicationRequirements,
signed: ApplicationFileRequest | undefined
): ApplicationFileRequest | undefined {
const declared = fileRequestForRequirements(requirements);
const files = requirements.files;
if (!declared || !signed || !files || "actions" in files) return declared;
const actions = declared.actions.filter((action) => signed.actions.includes(action));
return files.required.every((action) => actions.includes(action))
? { actions, scope: declared.scope }
: declared;
}
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ export async function approvePortalAuthorization(
requestedOperations: input.operations,
applicationOperationCeiling:
pending.requested_operations as CollectionOperation[],
applicationFileCeiling: pending.application_authorization.binding.requested_files?.actions ?? [],
requestedFileActions: input.fileActions,
requestedPeoplePermissions: input.peoplePermissions,
requirements: pending.requirements,
Expand Down Expand Up @@ -384,6 +385,7 @@ export async function approvePortalAuthorization(
const finalScope = planCollectionGrant({
requestedOperations: grant!.operations,
applicationOperationCeiling: grant!.operations,
applicationFileCeiling: grant!.file_capability?.actions ?? [],
requestedFileActions: grant!.file_capability?.actions,
requirements,
access: grantAccess!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ export async function approveHostedAuthorization(
requestedOperations: input.operations,
applicationOperationCeiling:
pending.requested_operations as CollectionOperation[],
applicationFileCeiling: pending.application_authorization.binding.requested_files?.actions ?? [],
requestedFileActions: input.fileActions,
requestedPeoplePermissions: input.peoplePermissions,
requirements: pending.requirements,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ async function approveConnectorAuthorization(
planCollectionGrant({
requestedOperations: input.operations,
applicationOperationCeiling: binding.requested_operations,
applicationFileCeiling: binding.requested_files?.actions ?? [],
requestedFileActions: input.fileActions,
requestedPeoplePermissions: input.peoplePermissions,
requirements: selected.requirements,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export async function approveNativeAuthorizationInTransaction(connection: Databa
const { plan, legacy } = await planFreshNextGrant(connection, input.collectionId, {
requestedOperations: input.operations,
applicationOperationCeiling: pending.requested_operations as CollectionOperation[],
applicationFileCeiling: pending.application_authorization.binding.requested_files?.actions ?? [],
requestedFileActions: input.fileActions,
requestedPeoplePermissions: input.peoplePermissions,
requirements: pending.requirements,
Expand Down
23 changes: 18 additions & 5 deletions services/server/src/features/authorizations/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import type {
GrantEncryption,
GrantScope
} from "@mdbase-dev/connect-protocol";
import { assertFreshApplicationAuthorization, fileRequestForRequirements, requirementContractVersion, type ApplicationRequirements } from "../../application-requirements.js";
import { assertFreshApplicationAuthorization, requirementContractVersion, signedFileRequestForRequirements, type ApplicationRequirements } from "../../application-requirements.js";
import type { FastifyInstance } from "fastify";
import { z } from "zod";
import { verifyApplicationAuthorization } from "../../application-authorization.js";
import { parseApplicationAuthorization, verifyApplicationAuthorization } from "../../application-authorization.js";
import { isCanonicalCollectionGrantScope } from "../../application-grant-scope.js";
import {
accessView,
Expand Down Expand Up @@ -183,7 +183,12 @@ export function registerAuthorizationRoutes(
codeChallenge: input.code_challenge,
requestedOperations,
semanticCapabilityContractVersion: requirementContractVersion(application.rows[0].requirements),
requestedFiles: fileRequestForRequirements(application.rows[0].requirements),
// The application signs the declared file actions it is requesting;
// consent and approval keep exactly that subset as their ceiling.
requestedFiles: signedFileRequestForRequirements(
application.rows[0].requirements,
parseApplicationAuthorization(input.application_authorization).binding.requested_files
),
collectionId: input.collection_id
}
);
Expand Down Expand Up @@ -524,7 +529,12 @@ export function registerAuthorizationRoutes(
codeChallenge: input.code_challenge,
requestedOperations,
semanticCapabilityContractVersion: requirementContractVersion(application.rows[0].requirements),
requestedFiles: fileRequestForRequirements(application.rows[0].requirements),
// The application signs the declared file actions it is requesting;
// consent and approval keep exactly that subset as their ceiling.
requestedFiles: signedFileRequestForRequirements(
application.rows[0].requirements,
parseApplicationAuthorization(input.application_authorization).binding.requested_files
),
collectionId: input.collection_id
}
);
Expand Down Expand Up @@ -661,6 +671,7 @@ export function registerAuthorizationRoutes(
a.homepage, a.project_url, a.icon,
a.application_declaration->>'platform' AS platform,
a.requirements, a.provisions, a.notifications,
ar.application_authorization->'binding'->'requested_files'->'actions' AS requested_file_actions,
${options.nextClientKeys ? "ck.client_pk" : "NULL::bytea"} AS client_pk
FROM authorization_requests ar JOIN applications a ON a.id = ar.application_id
JOIN users consenting ON consenting.id = ar.user_id
Expand All @@ -670,7 +681,7 @@ export function registerAuthorizationRoutes(
[requestId, user.id]
);
if (!authorization.rows[0]) return reply.code(404).send(apiError("authorization_not_found", "Authorization request expired or was not found."));
const {redirect_uri, device_origin, ...pending} = authorization.rows[0];
const {redirect_uri, device_origin, requested_file_actions: applicationFileCeiling, ...pending} = authorization.rows[0];
const applicationOrigin = pending.flow === "device_code" ? device_origin ?? "null" : applicationOriginForRedirect(redirect_uri, pending.homepage);
const activeGrants = await options.db.query<{
collection_id: string;
Expand Down Expand Up @@ -739,6 +750,7 @@ export function registerAuthorizationRoutes(
access: accessView(access),
authorization: previewCollectionGrant({
applicationOperationCeiling: authorization.rows[0].requested_operations,
applicationFileCeiling: applicationFileCeiling ?? undefined,
requirements: authorization.rows[0].requirements,
access
})
Expand All @@ -763,6 +775,7 @@ export function registerAuthorizationRoutes(
access: accessView(access),
authorization: previewCollectionGrant({
applicationOperationCeiling: authorization.rows[0].requested_operations,
applicationFileCeiling: applicationFileCeiling ?? undefined,
requirements: authorization.rows[0].requirements,
access
})
Expand Down
1 change: 0 additions & 1 deletion services/server/src/features/grants/connector-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import type { FastifyInstance } from "fastify";
import { z } from "zod";
import { COLLECTION_OPERATIONS } from "../../collection-access.js";
import type { DatabasePool } from "../../db.js";
import { fileCapabilityForRequirements } from "../../grant-planner.js";
import { collectionContractDescriptorSchema } from "../../protocol-schemas.js";
import type { RelayHub } from "../../relay.js";
import { localGrantRevocationStatus, queueLocalGrantRevocations } from "../../local-grant-revocation.js";
Expand Down
Loading
Loading