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
4 changes: 4 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ jobs:
with:
node-version: "22"
- run: npm ci
- run: npm ci
working-directory: mcp-server
- run: npm test
working-directory: mcp-server
- run: node scripts/check-schema-drift.mjs
- run: npm test
- run: npx tsc --noEmit
Expand Down
3 changes: 3 additions & 0 deletions docs/0-requirements.ja.md
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,9 @@ Requirements:
- MCP client user を認証する
- installation 済み repository に access する
- GitHub API read に token を使う
- cached Dynamic Client Registration は、登録済み redirect URI 集合が今回の
OAuth flow で要求する localhost callback URI を全て含む場合だけ再利用し、
含まない場合は authorization 前に client registration を置き換える

## Storage Rules

Expand Down
3 changes: 3 additions & 0 deletions docs/0-requirements.md
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,9 @@ Requirements:
- authenticate the MCP client user
- access repositories through the installed app
- use GitHub tokens for API reads
- reuse a cached Dynamic Client Registration only when its registered redirect
URI set covers every localhost callback URI requested for the current OAuth
flow; otherwise register a replacement client before authorization

## Storage Rules

Expand Down
11 changes: 10 additions & 1 deletion mcp-server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ OAuth client registration and tokens are stored in:
- `~/.github-rag-mcp/oauth-client.json` (dynamic client registration)
- `~/.github-rag-mcp/oauth-tokens.json` (access + refresh tokens)

The proxy reuses a cached client registration only when it includes every
redirect URI required by the current localhost callback listener. Because the
listener uses a random port, a registration from a previous authorization may
be replaced automatically before the browser opens.

Delete these files to force a fresh authorization flow.

## Tools exposed
Expand All @@ -101,7 +106,8 @@ The `type` filter accepts: `issue`, `pull_request`, `release`, `doc`, `wiki_doc`
## Authentication flow

1. On first tool call, the proxy discovers OAuth metadata at `${RAG_WORKER_URL}/.well-known/oauth-authorization-server`.
2. It performs Dynamic Client Registration (RFC 7591) if no client is cached.
2. It performs Dynamic Client Registration (RFC 7591) if no compatible client
is cached for the current localhost callback URIs.
3. It starts a one-shot localhost HTTP listener on a random port and opens the browser to the Worker's authorization endpoint.
4. After you approve, the Worker redirects to `http://127.0.0.1:<port>/callback` with an authorization code.
5. The proxy exchanges the code for tokens (PKCE S256) and saves them.
Expand All @@ -112,6 +118,9 @@ The browser callback never leaves your machine; the authorization code is delive
## Troubleshooting

- **Browser does not open.** The proxy logs the authorization URL to stderr; copy it into a browser manually.
- **`redirect_uri is not associated with this application`.** Upgrade the
proxy. Current versions replace cached client registrations whose redirect
URI set does not cover the callback port selected for this authorization.
- **`OAuth callback timed out after 5 minutes`.** Re-invoke any tool to restart the flow.
- **`Failed to reach worker`.** Check that `RAG_WORKER_URL` is correct and reachable from your machine.
- **Stale credentials.** Remove `~/.github-rag-mcp/oauth-tokens.json` (and optionally `oauth-client.json`) and retry.
Expand Down
2 changes: 1 addition & 1 deletion mcp-server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
],
"scripts": {
"start": "node server/index.js",
"test": "node --check server/index.js",
"test": "node --check server/index.js && node --test test/oauth-client-registration.test.js",
"pack:mcpb": "mcpb pack"
},
"dependencies": {
Expand Down
3 changes: 2 additions & 1 deletion mcp-server/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { homedir } from "node:os";
import { join } from "node:path";
import { exec } from "node:child_process";
import { createRequire } from "node:module";
import { supportsRedirectUris } from "./oauth-client-registration.js";

const require = createRequire(import.meta.url);
const { version: PACKAGE_VERSION } = require("../package.json");
Expand Down Expand Up @@ -89,7 +90,7 @@ async function saveClientRegistration(reg) {

async function ensureClientRegistration(metadata, redirectUris) {
const existing = await loadClientRegistration();
if (existing) return existing;
if (supportsRedirectUris(existing, redirectUris)) return existing;

if (!metadata.registration_endpoint) {
throw new Error("OAuth server does not support dynamic client registration");
Expand Down
21 changes: 21 additions & 0 deletions mcp-server/server/oauth-client-registration.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Return true only when a cached Dynamic Client Registration can be reused for
* every redirect URI requested by the current OAuth callback listener.
*/
export function supportsRedirectUris(registration, redirectUris) {
if (
registration === null ||
typeof registration !== "object" ||
typeof registration.client_id !== "string" ||
registration.client_id.length === 0 ||
!Array.isArray(registration.redirect_uris)
) {
return false;
}

const registeredUris = new Set(
registration.redirect_uris.filter((uri) => typeof uri === "string"),
);

return redirectUris.every((uri) => registeredUris.has(uri));
}
80 changes: 80 additions & 0 deletions mcp-server/test/oauth-client-registration.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import assert from "node:assert/strict";
import test from "node:test";

import { supportsRedirectUris } from "../server/oauth-client-registration.js";

const REQUIRED_URIS = [
"http://127.0.0.1:43123/callback",
"http://localhost:43123/callback",
];

test("reuses a registration that covers every requested redirect URI", () => {
assert.equal(
supportsRedirectUris(
{
client_id: "client-id",
redirect_uris: [...REQUIRED_URIS].reverse(),
},
REQUIRED_URIS,
),
true,
);
});

test("reuses a registration whose redirect URI set is a superset", () => {
assert.equal(
supportsRedirectUris(
{
client_id: "client-id",
redirect_uris: [
...REQUIRED_URIS,
"http://127.0.0.1:49999/callback",
],
},
REQUIRED_URIS,
),
true,
);
});

test("rejects a stale registration from a different callback port", () => {
assert.equal(
supportsRedirectUris(
{
client_id: "client-id",
redirect_uris: [
"http://127.0.0.1:40000/callback",
"http://localhost:40000/callback",
],
},
REQUIRED_URIS,
),
false,
);
});

test("rejects a registration missing one requested redirect URI", () => {
assert.equal(
supportsRedirectUris(
{
client_id: "client-id",
redirect_uris: [REQUIRED_URIS[0]],
},
REQUIRED_URIS,
),
false,
);
});

test("rejects malformed cached registrations", () => {
assert.equal(supportsRedirectUris(null, REQUIRED_URIS), false);
assert.equal(supportsRedirectUris({}, REQUIRED_URIS), false);
assert.equal(
supportsRedirectUris({ client_id: "", redirect_uris: REQUIRED_URIS }, REQUIRED_URIS),
false,
);
assert.equal(
supportsRedirectUris({ client_id: "client-id", redirect_uris: "not-an-array" }, REQUIRED_URIS),
false,
);
});
Loading