Skip to content

Commit aada71b

Browse files
committed
lazy-auth: address review nits
- Build the well-known rewrite regex from LAZY_AUTH_SLUG so it can't drift from the mount path if the slug changes. - Clarify in the docstring that an explicit PUBLIC_URL must include the /lazy-auth mount path (the package advertises it verbatim). - Tests: clear/restore ambient PUBLIC_URL around the integration server (was a latent flake); add unit coverage for the PUBLIC_URL derivation (trailing-slash strip, no-trailing-slash, explicit-wins, unset cases); add a public tool-call success case and an invalid-token 401 case.
1 parent 2476540 commit aada71b

2 files changed

Lines changed: 73 additions & 12 deletions

File tree

src/modules/example-apps/lazy-auth.integration.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,15 @@ function callTool(base: string, name: string, accessToken?: string): Promise<Htt
5151
describe('Lazy Auth example mount', () => {
5252
let server: http.Server;
5353
let base: string;
54+
let savedPublicUrl: string | undefined;
5455

5556
beforeAll(async () => {
57+
// These tests rely on the package's per-request Host resolution (no baseUri
58+
// passed below), so an ambient PUBLIC_URL would pin advertised URLs and
59+
// break the assertions. Clear it for the duration and restore afterwards.
60+
savedPublicUrl = process.env.PUBLIC_URL;
61+
delete process.env.PUBLIC_URL;
62+
5663
const app = express();
5764
// Mirror src/index.ts: the example mounts before the host's middleware
5865
// and routes.
@@ -75,6 +82,8 @@ describe('Lazy Auth example mount', () => {
7582

7683
afterAll(async () => {
7784
await new Promise((resolve) => server.close(resolve));
85+
if (savedPublicUrl === undefined) delete process.env.PUBLIC_URL;
86+
else process.env.PUBLIC_URL = savedPublicUrl;
7887
});
7988

8089
it('serves public MCP requests without auth', async () => {
@@ -95,12 +104,24 @@ describe('Lazy Auth example mount', () => {
95104
expect(res.body).toContain('Lazy Auth');
96105
});
97106

107+
it('serves a public tool call without auth', async () => {
108+
const res = await callTool(base, 'show_auth_button');
109+
expect(res.status).toBe(200);
110+
expect(res.headers['www-authenticate']).toBeUndefined();
111+
});
112+
98113
it('answers 401 with resource_metadata under the mount path for protected tools', async () => {
99114
const res = await callTool(base, 'get_secret');
100115
expect(res.status).toBe(401);
101116
expect(res.headers['www-authenticate']).toContain(`resource_metadata="${base}/lazy-auth/auth/prm"`);
102117
});
103118

119+
it('answers 401 with invalid_token for a bad bearer token', async () => {
120+
const res = await callTool(base, 'get_secret', 'not-a-real-token');
121+
expect(res.status).toBe(401);
122+
expect(res.headers['www-authenticate']).toContain('invalid_token');
123+
});
124+
104125
it('advertises mount-prefixed URLs in PRM and AS metadata', async () => {
105126
const prm = JSON.parse((await request(`${base}/lazy-auth/auth/prm`)).body);
106127
expect(prm.resource).toBe(`${base}/lazy-auth/mcp`);
@@ -172,3 +193,38 @@ describe('Lazy Auth example mount', () => {
172193
expect(secretRes.body).toContain('the-answer-is-42');
173194
});
174195
});
196+
197+
describe('mountLazyAuthExample PUBLIC_URL derivation', () => {
198+
let savedPublicUrl: string | undefined;
199+
200+
beforeEach(() => {
201+
savedPublicUrl = process.env.PUBLIC_URL;
202+
delete process.env.PUBLIC_URL;
203+
});
204+
205+
afterEach(() => {
206+
if (savedPublicUrl === undefined) delete process.env.PUBLIC_URL;
207+
else process.env.PUBLIC_URL = savedPublicUrl;
208+
});
209+
210+
it('derives PUBLIC_URL from baseUri, stripping trailing slashes and appending the mount path', () => {
211+
mountLazyAuthExample(express(), 'https://example.test/');
212+
expect(process.env.PUBLIC_URL).toBe('https://example.test/lazy-auth');
213+
});
214+
215+
it('handles a baseUri with no trailing slash', () => {
216+
mountLazyAuthExample(express(), 'https://example.test');
217+
expect(process.env.PUBLIC_URL).toBe('https://example.test/lazy-auth');
218+
});
219+
220+
it('does not override an explicitly set PUBLIC_URL', () => {
221+
process.env.PUBLIC_URL = 'https://tunnel.example/lazy-auth';
222+
mountLazyAuthExample(express(), 'https://example.test/');
223+
expect(process.env.PUBLIC_URL).toBe('https://tunnel.example/lazy-auth');
224+
});
225+
226+
it('leaves PUBLIC_URL unset when no baseUri is provided', () => {
227+
mountLazyAuthExample(express());
228+
expect(process.env.PUBLIC_URL).toBeUndefined();
229+
});
230+
});

src/modules/example-apps/lazy-auth.ts

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,30 +17,35 @@
1717
* them from PUBLIC_URL, falling back to the request Host header for loopback
1818
* hosts only. We derive PUBLIC_URL from this server's own public base URI -
1919
* already the source of truth for the host's OAuth issuer - so deployments
20-
* need no separate env var. An explicit PUBLIC_URL still wins (e.g. a tunnel);
21-
* omitting baseUri (e.g. tests on an ephemeral port) keeps the package's
22-
* per-request Host resolution.
20+
* need no separate env var. An explicit PUBLIC_URL still wins (e.g. a tunnel) -
21+
* note it must include the /lazy-auth mount path, since the package advertises
22+
* it verbatim. Omitting baseUri (e.g. tests on an ephemeral port) keeps the
23+
* package's per-request Host resolution.
2324
*/
2425
import { Express, Request, Response, NextFunction } from 'express';
2526
import { createApp as createLazyAuthApp } from '@modelcontextprotocol/server-lazy-auth';
2627

2728
export const LAZY_AUTH_SLUG = 'lazy-auth';
2829

30+
// RFC 8414/9728 place well-known discovery documents at the origin root, with
31+
// the resource path inserted after the well-known prefix
32+
// (e.g. /.well-known/oauth-authorization-server/lazy-auth). Built from
33+
// LAZY_AUTH_SLUG so it can't drift from the mount path.
34+
const WELL_KNOWN_INSERTION = new RegExp(
35+
`^/\\.well-known/(oauth-authorization-server|oauth-protected-resource)/${LAZY_AUTH_SLUG}(/.*)?$`
36+
);
37+
2938
export function mountLazyAuthExample(app: Express, baseUri?: string): void {
3039
if (baseUri && !process.env.PUBLIC_URL) {
3140
process.env.PUBLIC_URL = `${baseUri.replace(/\/+$/, '')}/${LAZY_AUTH_SLUG}`;
3241
}
3342

34-
// RFC 8414/9728 place well-known discovery documents at the origin root,
35-
// with the resource path inserted after the well-known prefix
36-
// (e.g. /.well-known/oauth-authorization-server/lazy-auth), and MCP SDK
37-
// clients only try that insertion form. Rewrite those root paths into the
38-
// mount - rather than dispatching to the sub-app directly - so req.baseUrl
39-
// (and therefore every URL the example advertises) stays consistent.
43+
// MCP SDK clients only try the path-insertion form above for discovery.
44+
// Rewrite those root paths into the mount - rather than dispatching to the
45+
// sub-app directly - so req.baseUrl (and therefore every URL the example
46+
// advertises) stays consistent.
4047
app.use((req: Request, _res: Response, next: NextFunction) => {
41-
const wellKnown = req.url.match(
42-
/^\/\.well-known\/(oauth-authorization-server|oauth-protected-resource)\/lazy-auth(\/.*)?$/
43-
);
48+
const wellKnown = req.url.match(WELL_KNOWN_INSERTION);
4449
if (wellKnown) {
4550
req.url = `/${LAZY_AUTH_SLUG}/.well-known/${wellKnown[1]}${wellKnown[2] ?? ''}`;
4651
}

0 commit comments

Comments
 (0)