Skip to content

Commit d0a136b

Browse files
elgorroclaude
andauthored
refactor(mcp): replace hand-rolled JWT with jose library (#86)
Replace custom HMAC-SHA256 signJwt/verifyJwt helpers in provider.ts with jose (SignJWT/jwtVerify), delegating iss/aud validation to the library. Also includes redirect_uri validation in loginHandler and a fail-fast guard requiring MCP_AUTH_ENABLED or MCP_ALLOW_UNAUTHENTICATED for HTTP transport. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> ## Description Brief description of the changes. ## Type of change - [ ] Bug fix - [ ] New feature - [ ] Documentation update - [ ] Refactoring - [ ] Other (please describe) ## Component - [ ] MCP Server - [ ] Nextcloud App - [ ] Documentation - [ ] CI/CD ## Checklist - [ ] I have tested my changes locally - [ ] I have added tests for new functionality - [ ] I have updated the documentation if needed - [ ] My code follows the project's style guidelines ## Related Issues Fixes # ## Screenshots (if applicable) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 3382af3 commit d0a136b

9 files changed

Lines changed: 165 additions & 48 deletions

File tree

mcp-server/package-lock.json

Lines changed: 6 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

mcp-server/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "aiquila-mcp",
3-
"version": "0.1.41",
3+
"version": "0.1.42",
44
"description": "AIquila - MCP server for Nextcloud integration with Claude AI",
55
"type": "module",
66
"main": "dist/index.js",
@@ -33,6 +33,7 @@
3333
],
3434
"dependencies": {
3535
"@modelcontextprotocol/sdk": "^1.27.1",
36+
"jose": "^6.2.0",
3637
"pino": "^9.0.0",
3738
"typescript": "^5.8.3",
3839
"webdav": "^5.9.0",

mcp-server/server.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,13 @@
77
"url": "https://github.com/elgorro/aiquila.git",
88
"source": "github"
99
},
10-
"version": "0.1.41",
10+
"version": "0.1.42",
1111
"websiteUrl": "https://github.com/elgorro/aiquila",
1212
"packages": [
1313
{
1414
"registryType": "npm",
1515
"identifier": "aiquila-mcp",
16-
"version": "0.1.41",
16+
"version": "0.1.42",
1717
"runtimeHint": "npx",
1818
"transport": { "type": "stdio" },
1919
"environmentVariables": [
@@ -39,7 +39,7 @@
3939
},
4040
{
4141
"registryType": "oci",
42-
"identifier": "ghcr.io/elgorro/aiquila-mcp:0.1.41",
42+
"identifier": "ghcr.io/elgorro/aiquila-mcp:0.1.42",
4343
"runtimeHint": "docker",
4444
"transport": { "type": "stdio" },
4545
"environmentVariables": [

mcp-server/src/__tests__/auth/login.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ describe('loginHandler', () => {
3333
...savedEnv,
3434
NEXTCLOUD_URL: 'https://cloud.example.com',
3535
MCP_AUTH_SECRET: 'test-secret-at-least-32-chars-long!!',
36+
MCP_AUTH_ISSUER: 'https://test.example.com',
37+
MCP_CLIENT_ID: 'c1',
38+
MCP_CLIENT_REDIRECT_URIS: 'https://example.com/callback',
3639
};
3740
provider = new NextcloudOAuthProvider();
3841
vi.clearAllMocks();

mcp-server/src/__tests__/auth/provider.test.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,11 @@ describe('NextcloudOAuthProvider', () => {
8888
const savedEnv = process.env;
8989

9090
beforeEach(() => {
91-
process.env = { ...savedEnv, MCP_AUTH_SECRET: TEST_SECRET };
91+
process.env = {
92+
...savedEnv,
93+
MCP_AUTH_SECRET: TEST_SECRET,
94+
MCP_AUTH_ISSUER: 'https://test.example.com',
95+
};
9296
provider = new NextcloudOAuthProvider();
9397
});
9498

@@ -300,6 +304,27 @@ describe('NextcloudOAuthProvider', () => {
300304
process.env.MCP_AUTH_SECRET = 'completely-different-secret-value!!';
301305
await expect(provider.verifyAccessToken(token)).rejects.toThrow();
302306
});
307+
308+
it('embeds iss and aud claims in the issued token', async () => {
309+
const token = await issueToken();
310+
const payload = JSON.parse(Buffer.from(token.split('.')[1], 'base64url').toString());
311+
expect(payload.iss).toBe('https://test.example.com');
312+
expect(payload.aud).toBe('https://test.example.com/mcp');
313+
});
314+
315+
it('throws when verified against a different issuer', async () => {
316+
const token = await issueToken();
317+
process.env.MCP_AUTH_ISSUER = 'https://other.example.com';
318+
await expect(provider.verifyAccessToken(token)).rejects.toThrow(
319+
'Invalid or expired access token'
320+
);
321+
});
322+
323+
it('embeds correct aud claim (resource server URL)', async () => {
324+
const token = await issueToken();
325+
const payload = JSON.parse(Buffer.from(token.split('.')[1], 'base64url').toString());
326+
expect(payload.aud).toBe('https://test.example.com/mcp');
327+
});
303328
});
304329

305330
// ---- exchangeRefreshToken ----

mcp-server/src/__tests__/transports.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ describe('http transport', () => {
9595

9696
beforeEach(() => {
9797
vi.clearAllMocks();
98-
process.env = { ...savedEnv };
98+
process.env = { ...savedEnv, MCP_ALLOW_UNAUTHENTICATED: 'true' };
9999
delete process.env.MCP_AUTH_ENABLED;
100100
});
101101

@@ -203,7 +203,7 @@ describe('http transport with auth enabled', () => {
203203
it('registers POST /auth/login handler', async () => {
204204
const { startHttp } = await import('../transports/http.js');
205205
await startHttp();
206-
expect(mockPost).toHaveBeenCalledWith('/auth/login', mockLoginHandlerFn);
206+
expect(mockPost).toHaveBeenCalledWith('/auth/login', expect.any(Function), mockLoginHandlerFn);
207207
});
208208

209209
it('mounts /mcp with bearer middleware and handler', async () => {

mcp-server/src/auth/login.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,40 @@ export function loginHandler(provider: NextcloudOAuthProvider) {
2929
return;
3030
}
3131

32+
const client = await provider.clientsStore.getClient(client_id);
33+
if (!client) {
34+
res
35+
.status(400)
36+
.type('html')
37+
.send(
38+
renderLoginForm({
39+
clientId: client_id,
40+
redirectUri: redirect_uri,
41+
codeChallenge: code_challenge,
42+
state,
43+
scope,
44+
error: 'Unknown client',
45+
})
46+
);
47+
return;
48+
}
49+
if (!client.redirect_uris.map(String).includes(redirect_uri)) {
50+
res
51+
.status(400)
52+
.type('html')
53+
.send(
54+
renderLoginForm({
55+
clientId: client_id,
56+
redirectUri: '',
57+
codeChallenge: code_challenge,
58+
state,
59+
scope,
60+
error: 'Invalid redirect URI',
61+
})
62+
);
63+
return;
64+
}
65+
3266
logger.info({ user: username, client: client_id, nc: ncUrl }, '[auth] Login attempt');
3367

3468
try {

mcp-server/src/auth/provider.ts

Lines changed: 57 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { createHmac, timingSafeEqual } from 'node:crypto';
1+
import { SignJWT, jwtVerify, type JWTPayload } from 'jose';
22
import type {
33
OAuthServerProvider,
44
AuthorizationParams,
@@ -14,40 +14,35 @@ import { InvalidGrantError } from '@modelcontextprotocol/sdk/server/auth/errors.
1414
import { ClientsStore, CodeStore, RefreshStore } from './store.js';
1515
import { logger } from '../logger.js';
1616

17-
// --- JWT helpers (HMAC-SHA256 via node:crypto — no extra deps) ---
17+
// --- JWT helpers (HMAC-SHA256 via jose) ---
1818

19-
function base64url(buf: Buffer): string {
20-
return buf.toString('base64').replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '');
21-
}
22-
23-
function base64urlStr(s: string): string {
24-
return base64url(Buffer.from(s, 'utf8'));
25-
}
26-
27-
function signJwt(payload: Record<string, unknown>, secret: string, expiresInSecs: number): string {
19+
async function signJwt(
20+
payload: Record<string, unknown>,
21+
secret: string,
22+
expiresInSecs: number
23+
): Promise<string> {
24+
const key = new TextEncoder().encode(secret);
2825
const now = Math.floor(Date.now() / 1000);
29-
const claims = { ...payload, iat: now, exp: now + expiresInSecs };
30-
const header = base64urlStr(JSON.stringify({ alg: 'HS256', typ: 'JWT' }));
31-
const body = base64urlStr(JSON.stringify(claims));
32-
const signing = `${header}.${body}`;
33-
const sig = base64url(createHmac('sha256', secret).update(signing).digest());
34-
return `${signing}.${sig}`;
26+
return new SignJWT(payload)
27+
.setProtectedHeader({ alg: 'HS256' })
28+
.setIssuedAt(now)
29+
.setExpirationTime(now + expiresInSecs)
30+
.sign(key);
3531
}
3632

37-
function verifyJwt(token: string, secret: string): Record<string, unknown> | null {
38-
const parts = token.split('.');
39-
if (parts.length !== 3) return null;
40-
const [header, body, sig] = parts;
41-
const expected = base64url(createHmac('sha256', secret).update(`${header}.${body}`).digest());
42-
const sigBuf = Buffer.from(sig);
43-
const expBuf = Buffer.from(expected);
44-
// HMAC-SHA256 base64url is always 43 chars — same length guaranteed
45-
if (sigBuf.length !== expBuf.length) return null;
46-
if (!timingSafeEqual(sigBuf, expBuf)) return null;
33+
async function verifyJwt(
34+
token: string,
35+
secret: string,
36+
opts?: { issuer?: string; audience?: string }
37+
): Promise<JWTPayload | null> {
4738
try {
48-
const claims = JSON.parse(Buffer.from(body, 'base64url').toString('utf8'));
49-
if (typeof claims.exp === 'number' && Date.now() / 1000 > claims.exp) return null;
50-
return claims as Record<string, unknown>;
39+
const key = new TextEncoder().encode(secret);
40+
const { payload } = await jwtVerify(token, key, {
41+
algorithms: ['HS256'],
42+
...(opts?.issuer && { issuer: opts.issuer }),
43+
...(opts?.audience && { audience: opts.audience }),
44+
});
45+
return payload;
5146
} catch {
5247
return null;
5348
}
@@ -195,8 +190,14 @@ export class NextcloudOAuthProvider implements OAuthServerProvider {
195190
}
196191
this.codeStore.delete(authorizationCode);
197192

198-
const accessToken = signJwt(
199-
{ sub: entry.userId, client_id: entry.clientId, scopes: entry.scopes },
193+
const accessToken = await signJwt(
194+
{
195+
sub: entry.userId,
196+
client_id: entry.clientId,
197+
scopes: entry.scopes,
198+
iss: this.getIssuer(),
199+
aud: this.getResourceUrl(),
200+
},
200201
this.getSecret(),
201202
ACCESS_TOKEN_TTL_SECS
202203
);
@@ -236,8 +237,14 @@ export class NextcloudOAuthProvider implements OAuthServerProvider {
236237
const effectiveScopes = scopes ?? entry.scopes;
237238
this.refreshStore.delete(refreshToken);
238239

239-
const accessToken = signJwt(
240-
{ sub: entry.userId, client_id: entry.clientId, scopes: effectiveScopes },
240+
const accessToken = await signJwt(
241+
{
242+
sub: entry.userId,
243+
client_id: entry.clientId,
244+
scopes: effectiveScopes,
245+
iss: this.getIssuer(),
246+
aud: this.getResourceUrl(),
247+
},
241248
this.getSecret(),
242249
ACCESS_TOKEN_TTL_SECS
243250
);
@@ -258,7 +265,12 @@ export class NextcloudOAuthProvider implements OAuthServerProvider {
258265
}
259266

260267
async verifyAccessToken(token: string): Promise<AuthInfo> {
261-
const claims = verifyJwt(token, this.getSecret());
268+
const issuer = process.env.MCP_AUTH_ISSUER;
269+
const claims = await verifyJwt(
270+
token,
271+
this.getSecret(),
272+
issuer ? { issuer, audience: new URL('/mcp', issuer).toString() } : undefined
273+
);
262274
if (!claims) {
263275
logger.warn('[auth] Access token verification failed: invalid or expired token');
264276
throw new Error('Invalid or expired access token');
@@ -295,4 +307,14 @@ export class NextcloudOAuthProvider implements OAuthServerProvider {
295307
if (!secret) throw new Error('MCP_AUTH_SECRET is not set');
296308
return secret;
297309
}
310+
311+
private getIssuer(): string {
312+
const issuer = process.env.MCP_AUTH_ISSUER;
313+
if (!issuer) throw new Error('MCP_AUTH_ISSUER is not set');
314+
return issuer;
315+
}
316+
317+
private getResourceUrl(): string {
318+
return new URL('/mcp', this.getIssuer()).toString();
319+
}
298320
}

mcp-server/src/transports/http.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,16 @@ export async function startHttp(): Promise<void> {
9999
const host = process.env.MCP_HOST || '0.0.0.0';
100100
const authEnabled = process.env.MCP_AUTH_ENABLED === 'true';
101101

102+
if (!authEnabled) {
103+
if (process.env.MCP_ALLOW_UNAUTHENTICATED !== 'true') {
104+
throw new Error(
105+
'HTTP transport requires MCP_AUTH_ENABLED=true. ' +
106+
'Set MCP_ALLOW_UNAUTHENTICATED=true to override (development only).'
107+
);
108+
}
109+
logger.warn('[startup] Running in UNAUTHENTICATED mode — do not use in production');
110+
}
111+
102112
// When auth is enabled, derive allowedHosts from the public issuer URL.
103113
// This suppresses the MCP SDK's "binding to 0.0.0.0 without DNS rebinding
104114
// protection" console.warn (which breaks structured-log parsers like jq) and
@@ -207,7 +217,28 @@ export async function startHttp(): Promise<void> {
207217
app.use(express.urlencoded({ extended: false }));
208218

209219
// Nextcloud credential validation → auth code issuance
210-
app.post('/auth/login', loginHandler(provider));
220+
const loginRateLimit = (() => {
221+
const attempts = new Map<string, { count: number; resetAt: number }>();
222+
const MAX = 10,
223+
WINDOW_MS = 15 * 60 * 1000;
224+
return (req: any, res: any, next: any) => {
225+
const ip = req.ip ?? 'unknown';
226+
const now = Date.now();
227+
const entry = attempts.get(ip);
228+
if (!entry || now > entry.resetAt) {
229+
attempts.set(ip, { count: 1, resetAt: now + WINDOW_MS });
230+
return next();
231+
}
232+
entry.count++;
233+
if (entry.count > MAX) {
234+
logger.warn({ ip }, '[auth] Login rate limit exceeded');
235+
res.status(429).json({ error: 'too_many_requests' });
236+
return;
237+
}
238+
next();
239+
};
240+
})();
241+
app.post('/auth/login', loginRateLimit, loginHandler(provider));
211242

212243
// Protect /mcp with Bearer token auth
213244
app.all(

0 commit comments

Comments
 (0)