mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(mcp): name the host an authorization goes to on the consent page
Dynamic client registration is open by specification and the client names itself, so anyone could register under a well-known assistant's name with a callback on a host of their own and send a signed-in person the authorize link. The consent page showed the borrowed name and "Unverified application", never where the code would be sent. Every consent page now says where the access goes: the callback's host, or an application on this device for a loopback callback, and the Allow button names the host. For a self-registered client whose callback leaves the device, a warning that names the host leads the page, above the self-reported name. Registration stays open: the hosted assistants register this way with their own https callback.
This commit is contained in:
1 parent
77ace106b3
commit
d1fcbca63b
2 files changed
+109
-1
No files matched your search
@@ -1,6 +1,7 @@
|
||||
/**
|
||||
* Authorize endpoint — consent gate + audience + PKCE + redirect-URI binding.
|
||||
*/
|
||||
import { NextRequest } from "next/server";
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
|
||||
process.env.API_TOKEN_HMAC_KEY = "x".repeat(48);
|
||||
@@ -496,3 +497,58 @@ describe("authorize CSP — the consent form may lead only to the validated redi
|
||||
expect(csp(unavailable)).toContain("default-src 'none'");
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Dynamic registration lets anyone register "Claude" with a callback on a
|
||||
* host of their own and send a signed-in person the authorize link. The
|
||||
* consent page used to show the borrowed name and "Unverified application",
|
||||
* never where the code would go.
|
||||
*/
|
||||
describe("GET /authorize — where the access goes (M3)", () => {
|
||||
it("names the redirect host for a self-registered client, on the page and on Allow", async () => {
|
||||
const phish = registerDcrClient({
|
||||
clientName: "Claude",
|
||||
redirectUris: ["https://collector.example.net/cb"],
|
||||
});
|
||||
signedIn();
|
||||
const body = await (
|
||||
await GET(
|
||||
new NextRequest(
|
||||
authorizeUrl({
|
||||
client_id: phish.clientId,
|
||||
redirect_uri: "https://collector.example.net/cb",
|
||||
}),
|
||||
),
|
||||
)
|
||||
).text();
|
||||
expect(body).toMatch(/Check where this access goes/);
|
||||
expect(body).toMatch(
|
||||
/Access goes to:<\/strong> <code>collector\.example\.net<\/code>/,
|
||||
);
|
||||
expect(body).toMatch(
|
||||
/value="allow">Allow access for collector\.example\.net</,
|
||||
);
|
||||
// The warning comes before the borrowed name.
|
||||
expect(body.indexOf("Check where")).toBeLessThan(body.indexOf("Claude"));
|
||||
});
|
||||
|
||||
it("says a loopback redirect stays on this device", async () => {
|
||||
const desktop = registerDcrClient({
|
||||
clientName: "Desktop client",
|
||||
redirectUris: ["http://127.0.0.1:4000/callback"],
|
||||
});
|
||||
signedIn();
|
||||
const body = await (
|
||||
await GET(
|
||||
new NextRequest(
|
||||
authorizeUrl({
|
||||
client_id: desktop.clientId,
|
||||
redirect_uri: "http://127.0.0.1:4000/callback",
|
||||
}),
|
||||
),
|
||||
)
|
||||
).text();
|
||||
expect(body).toMatch(/an application running on this device/);
|
||||
expect(body).not.toMatch(/Check where this access goes/);
|
||||
});
|
||||
});
|
||||
@@ -420,6 +420,7 @@ async function authorizeGet(request: NextRequest): Promise<Response> {
|
||||
v.clientSource === "cimd"
|
||||
? `<p>Verified origin: <code>${htmlEscape(v.clientOrigin)}</code></p>`
|
||||
: `<p><strong>Unverified application</strong> (dynamically registered, the name above is self-reported and not verified).</p>`;
|
||||
const destination = consentDestination(v.redirectUri, v.clientSource);
|
||||
// Plain-language access summary. When write is granted, say so explicitly —
|
||||
// the app will be able to READ and LOG/WRITE health data, not just read.
|
||||
const writeGranted = v.scope.split(/\s+/).includes(SCOPE_HEALTH_WRITE);
|
||||
@@ -436,8 +437,10 @@ async function authorizeGet(request: NextRequest): Promise<Response> {
|
||||
html(
|
||||
`<main>
|
||||
<h1>Authorize access</h1>
|
||||
${destination.warning}
|
||||
<p><strong>${htmlEscape(v.clientName)}</strong> is requesting access to your HealthLog data.</p>
|
||||
${provenance}
|
||||
${destination.line}
|
||||
<p>Scope: <code>${htmlEscape(v.scope)}</code></p>
|
||||
${accessSummary}
|
||||
${readSummary}
|
||||
@@ -450,7 +453,7 @@ async function authorizeGet(request: NextRequest): Promise<Response> {
|
||||
${hidden("scope", v.scope)}
|
||||
${hidden("resource", v.resource)}
|
||||
${v.state !== undefined ? hidden("state", v.state) : ""}
|
||||
<button type="submit" name="decision" value="allow">Allow</button>
|
||||
<button type="submit" name="decision" value="allow">${destination.allowLabel}</button>
|
||||
<button type="submit" name="decision" value="deny">Deny</button>
|
||||
</form>
|
||||
</main>`,
|
||||
@@ -460,6 +463,55 @@ async function authorizeGet(request: NextRequest): Promise<Response> {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Where an Allow sends the access, said on the consent page in words a person
|
||||
* checks: the redirect URI's host, or "an application on this device" for a
|
||||
* loopback redirect.
|
||||
*
|
||||
* Dynamic registration is open by specification and the client names itself,
|
||||
* so anyone can register "Claude" with a redirect to a host of their own and
|
||||
* send a signed-in person the authorize link; the page used to show the
|
||||
* borrowed name and never the host the code would go to. The host is now on
|
||||
* every consent page and on the Allow button. For an unverified client whose
|
||||
* redirect leaves this device, a warning leads the page.
|
||||
*
|
||||
* Registration itself stays open: the hosted assistants register this way
|
||||
* with their own https callback, so closing it or allowlisting hosts would
|
||||
* break them on every instance, and a switch left at its default would
|
||||
* protect nobody. Naming where the access goes is what a phished person can
|
||||
* act on.
|
||||
*/
|
||||
function consentDestination(
|
||||
redirectUri: string,
|
||||
clientSource: string,
|
||||
): { warning: string; line: string; allowLabel: string } {
|
||||
let url: URL;
|
||||
try {
|
||||
url = new URL(redirectUri);
|
||||
} catch {
|
||||
return { warning: "", line: "", allowLabel: "Allow" };
|
||||
}
|
||||
const loopback =
|
||||
url.protocol === "http:" &&
|
||||
["localhost", "127.0.0.1", "[::1]", "::1"].includes(url.hostname);
|
||||
if (loopback) {
|
||||
return {
|
||||
warning: "",
|
||||
line: `<p><strong>Access goes to:</strong> an application running on this device (<code>${htmlEscape(url.host)}</code>).</p>`,
|
||||
allowLabel: "Allow",
|
||||
};
|
||||
}
|
||||
const host = htmlEscape(url.host);
|
||||
return {
|
||||
warning:
|
||||
clientSource === "cimd"
|
||||
? ""
|
||||
: `<p><strong>Check where this access goes before you allow it.</strong> Whoever runs <code>${host}</code> will be able to use it. Allow only if you started this connection from <code>${host}</code> yourself; the application's name below is its own claim.</p>`,
|
||||
line: `<p><strong>Access goes to:</strong> <code>${host}</code></p>`,
|
||||
allowLabel: `Allow access for ${host}`,
|
||||
};
|
||||
}
|
||||
|
||||
async function authorizePost(request: NextRequest): Promise<Response> {
|
||||
return withBackgroundEvent("mcp.oauth.authorize", async () => {
|
||||
if (!(await surfaceAvailable())) {
|
||||
|
||||
Reference in new issue
Block a user