Skip to content

Commit ae54d9d

Browse files
David Ruzickaclaude
andcommitted
fix(oauth): env vars override profile config; fix envelope restart-recovery gaps
- MCP4_ALLOW_UNREGISTERED_CLIENTS/MCP4_ALLOWED_UNREGISTERED_REDIRECT_URIS/ MCP4_ALLOWED_ORIGINS now take full precedence over profile JSON when set; previously ?? meant allow_unregistered_clients=false in profile blocked env var - Envelope restart-recovery: populate inboundAuthTokenStore after createSession so enterprise enforcement works for restored sessions; move oauthTokensByAccessToken population post-createSession to prevent map leaks on init failure - Stale envelope (>30 days) now returns HTTP 401 instead of creating scopeless session - Fix entry guard: !tokenData instead of !refreshToken to skip recovery when oauthTokensByAccessToken already populated (non-rotating IdPs with no refresh token) Co-Authored-By: Agent <noreply@anthropic.com>
1 parent 08bf9fc commit ae54d9d

5 files changed

Lines changed: 355 additions & 22 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515
- `ClientAuthGate` orchestrator wired into HTTP transport session init: validates inbound client API key before session establishment; resolves `AuthorizedPrincipal` (authType=`token`) and attaches it as `session.clientPrincipal`; mode-aware (`required` rejects with HTTP 401 when no identity is resolved, `optional` allows anonymous sessions); when configured, the gate becomes the inbound auth authority and bypasses the legacy `authConfigs` token-required guard so `mode='optional'` can permit anonymous initialization (AUTH-02; partial AUTH-03). JWT/OIDC gate added in Phase 4.
1616

1717
### Fixed
18+
- `MCP4_ALLOW_UNREGISTERED_CLIENTS`, `MCP4_ALLOWED_UNREGISTERED_REDIRECT_URIS`, and `MCP4_ALLOWED_ORIGINS` env vars now act as operator overrides: when set, they take full precedence over profile JSON values (previously `??` meant `allow_unregistered_clients: false` in profile silently blocked the env var).
19+
- Encrypted token envelope restart-recovery: `inboundAuthTokenStore` now populated after session creation (fixes enterprise enforcement on restored sessions); stale envelopes (>30 days) now return HTTP 401 instead of silently creating a scopeless session; map population moved post-`createSession` to prevent map leaks on init failure; entry guard changed from `!refreshToken` to `!tokenData` to correctly skip recovery when OAuth map already populated for non-rotating IdPs.
1820
- Invalid supplied tokens during HTTP session initialization now return HTTP 403 instead of 401, preventing VS Code from misclassifying bearer-token failures as OAuth discovery and showing an irrelevant dynamic client registration prompt.
1921
- Tenant OAuth degradation: auth gate now checks `isOAuthConfigOperational` on the effective OAuth config (which may be tenant-specific) so an inoperational tenant OAuth config no longer sends an uncompletable 401 OAuth challenge.
2022
- Server-side env token validation at session init: when a profile auth config has both `value_from_env` and `validation_endpoint`, the resolved env token is validated via the endpoint before the session is established, failing fast with HTTP 401 instead of accepting the connection and returning 401 on every tool call.

src/mcp/mcp-server.test.ts

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,119 @@ paths:
867867
expect(context.oauthConfig?.allowed_unregistered_redirect_uris).toEqual(['http://localhost', 'cursor://']);
868868
});
869869

870+
it('MCP4_ALLOW_UNREGISTERED_CLIENTS=true overrides profile allow_unregistered_clients=false', () => {
871+
const serverWithMock = new MCPServer({
872+
info: () => {},
873+
warn: () => {},
874+
error: () => {},
875+
debug: () => {},
876+
} as any);
877+
878+
(serverWithMock as any).parser = {
879+
getBaseUrl: () => 'https://api.test',
880+
getResourceMetadata: () => ({ name: 'Test', documentation: 'Docs' })
881+
};
882+
883+
(serverWithMock as any).profile = {
884+
profile_name: 'test',
885+
description: 'test profile',
886+
tools: [],
887+
interceptors: {
888+
auth: [{
889+
type: 'oauth',
890+
priority: 1,
891+
oauth_config: {
892+
issuer: 'https://issuer.test',
893+
client_id: 'client-id',
894+
redirect_uri: 'https://app.test/callback',
895+
allow_unregistered_clients: false,
896+
}
897+
}]
898+
}
899+
};
900+
901+
process.env.MCP4_ALLOW_UNREGISTERED_CLIENTS = 'true';
902+
process.env.MCP4_ALLOWED_UNREGISTERED_REDIRECT_URIS = 'http://localhost';
903+
904+
const context = serverWithMock.getHttpProfileContext();
905+
expect(context.oauthConfig?.allow_unregistered_clients).toBe(true);
906+
});
907+
908+
it('MCP4_ALLOWED_UNREGISTERED_REDIRECT_URIS overrides profile allowed_unregistered_redirect_uris', () => {
909+
const serverWithMock = new MCPServer({
910+
info: () => {},
911+
warn: () => {},
912+
error: () => {},
913+
debug: () => {},
914+
} as any);
915+
916+
(serverWithMock as any).parser = {
917+
getBaseUrl: () => 'https://api.test',
918+
getResourceMetadata: () => ({ name: 'Test', documentation: 'Docs' })
919+
};
920+
921+
(serverWithMock as any).profile = {
922+
profile_name: 'test',
923+
description: 'test profile',
924+
tools: [],
925+
interceptors: {
926+
auth: [{
927+
type: 'oauth',
928+
priority: 1,
929+
oauth_config: {
930+
issuer: 'https://issuer.test',
931+
client_id: 'client-id',
932+
redirect_uri: 'https://app.test/callback',
933+
allow_unregistered_clients: true,
934+
allowed_unregistered_redirect_uris: ['http://profile-only.example.com'],
935+
}
936+
}]
937+
}
938+
};
939+
940+
process.env.MCP4_ALLOWED_UNREGISTERED_REDIRECT_URIS = 'http://localhost,http://127.0.0.1';
941+
942+
const context = serverWithMock.getHttpProfileContext();
943+
expect(context.oauthConfig?.allowed_unregistered_redirect_uris).toEqual(['http://localhost', 'http://127.0.0.1']);
944+
});
945+
946+
it('MCP4_ALLOWED_ORIGINS overrides profile allowed_redirect_hosts', () => {
947+
const serverWithMock = new MCPServer({
948+
info: () => {},
949+
warn: () => {},
950+
error: () => {},
951+
debug: () => {},
952+
} as any);
953+
954+
(serverWithMock as any).parser = {
955+
getBaseUrl: () => 'https://api.test',
956+
getResourceMetadata: () => ({ name: 'Test', documentation: 'Docs' })
957+
};
958+
959+
(serverWithMock as any).profile = {
960+
profile_name: 'test',
961+
description: 'test profile',
962+
tools: [],
963+
interceptors: {
964+
auth: [{
965+
type: 'oauth',
966+
priority: 1,
967+
oauth_config: {
968+
issuer: 'https://issuer.test',
969+
client_id: 'client-id',
970+
redirect_uri: 'https://app.test/callback',
971+
allowed_redirect_hosts: ['profile-host.example.com'],
972+
}
973+
}]
974+
}
975+
};
976+
977+
process.env.MCP4_ALLOWED_ORIGINS = 'https://env-host.example.com';
978+
979+
const context = serverWithMock.getHttpProfileContext();
980+
expect(context.oauthConfig?.allowed_redirect_hosts).toEqual(['env-host.example.com']);
981+
});
982+
870983
it('should derive OAuth redirect hosts and token limits from environment', async () => {
871984
const capturedConfigs: any[] = [];
872985

src/mcp/mcp-server.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -683,16 +683,15 @@ export class MCPServer {
683683

684684
return {
685685
...oauthConfig,
686-
allowed_redirect_hosts: oauthConfig.allowed_redirect_hosts
687-
|| (process.env.MCP4_ALLOWED_ORIGINS
688-
? this.extractHostsFromOrigins(process.env.MCP4_ALLOWED_ORIGINS)
689-
: undefined),
690-
allow_unregistered_clients: oauthConfig.allow_unregistered_clients
691-
?? (process.env.MCP4_ALLOW_UNREGISTERED_CLIENTS === 'true'),
692-
allowed_unregistered_redirect_uris: oauthConfig.allowed_unregistered_redirect_uris
693-
|| (allowedUnregisteredRedirectUris && allowedUnregisteredRedirectUris.length > 0
694-
? allowedUnregisteredRedirectUris
695-
: undefined),
686+
allowed_redirect_hosts: process.env.MCP4_ALLOWED_ORIGINS
687+
? this.extractHostsFromOrigins(process.env.MCP4_ALLOWED_ORIGINS)
688+
: oauthConfig.allowed_redirect_hosts,
689+
allow_unregistered_clients: process.env.MCP4_ALLOW_UNREGISTERED_CLIENTS !== undefined
690+
? process.env.MCP4_ALLOW_UNREGISTERED_CLIENTS === 'true'
691+
: (oauthConfig.allow_unregistered_clients ?? false),
692+
allowed_unregistered_redirect_uris: allowedUnregisteredRedirectUris && allowedUnregisteredRedirectUris.length > 0
693+
? allowedUnregisteredRedirectUris
694+
: oauthConfig.allowed_unregistered_redirect_uris,
696695
};
697696
}
698697

src/transport/http-transport.test.ts

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2783,8 +2783,11 @@ describeIfListen('HttpTransport', () => {
27832783
});
27842784

27852785
it('does not warn about MCP4_TOKEN_KEY when tokenKey is set', async () => {
2786+
const savedKey = process.env.MCP4_TOKEN_KEY;
2787+
delete process.env.MCP4_TOKEN_KEY;
27862788
const mockLogger = buildMockLogger();
27872789
const t = new HttpTransport({ ...baseConfig, tokenKey: Buffer.alloc(32) }, mockLogger);
2790+
if (savedKey !== undefined) process.env.MCP4_TOKEN_KEY = savedKey;
27882791
const warnCalls = mockLogger.warn.mock.calls;
27892792
const matchingCall = warnCalls.find((args: unknown[]) =>
27902793
typeof args[0] === 'string' && (args[0] as string).includes('MCP4_TOKEN_KEY'),
@@ -2793,6 +2796,21 @@ describeIfListen('HttpTransport', () => {
27932796
await t.stop();
27942797
});
27952798

2799+
it('warns when MCP4_TOKEN_KEY is a passphrase (non-hex key)', async () => {
2800+
const savedKey = process.env.MCP4_TOKEN_KEY;
2801+
process.env.MCP4_TOKEN_KEY = 'my-weak-passphrase';
2802+
const mockLogger = buildMockLogger();
2803+
const t = new HttpTransport({ ...baseConfig, tokenKey: Buffer.alloc(32) }, mockLogger);
2804+
if (savedKey !== undefined) process.env.MCP4_TOKEN_KEY = savedKey;
2805+
else delete process.env.MCP4_TOKEN_KEY;
2806+
const warnCalls = mockLogger.warn.mock.calls;
2807+
const matchingCall = warnCalls.find((args: unknown[]) =>
2808+
typeof args[0] === 'string' && (args[0] as string).includes('MCP4_TOKEN_KEY is a passphrase'),
2809+
);
2810+
expect(matchingCall).toBeDefined();
2811+
await t.stop();
2812+
});
2813+
27962814
it('accepts a 4096-char token at the new DEFAULT_MAX_TOKEN_LENGTH boundary', async () => {
27972815
const t = new HttpTransport({ ...baseConfig }, logger);
27982816
const tApp = (t as any).app;
@@ -3881,6 +3899,8 @@ describeIfListen('HttpTransport', () => {
38813899
expect(returned.startsWith('mcp4.v1.')).toBe(true);
38823900
expect(profileState.oauthTokensByAccessToken.has(returned)).toBe(true);
38833901
expect(profileState.oauthTokensByAccessToken.has(tokens.access_token)).toBe(false);
3902+
const mapEntry = profileState.oauthTokensByAccessToken.get(returned);
3903+
expect(mapEntry?.rawAccessToken).toBe('idp-access-d');
38843904
await t.stop();
38853905
});
38863906

@@ -4481,6 +4501,165 @@ describeIfListen('HttpTransport', () => {
44814501
expect(restoredCall).toBeUndefined();
44824502
await t.stop();
44834503
});
4504+
4505+
it('Test S: stale envelope (>30 days old) returns 401 with warn log', async () => {
4506+
const { encryptTokenPayload } = await import('../auth/token-envelope.js');
4507+
const mockLogger = mkLogger();
4508+
const key = buildKey();
4509+
const t = buildTransport({ tokenKey: key, testLogger: mockLogger });
4510+
const tApp = (t as any).app;
4511+
t.setMessageHandler(async () => ({ protocolVersion: '2025-03-26', serverInfo: { name: 'test' } }));
4512+
4513+
const staleIat = Date.now() - (31 * 24 * 60 * 60 * 1000);
4514+
const envelope = encryptTokenPayload(
4515+
{ v: 1, at: 'idp-access-s', rt: 'idp-refresh-s', pid: 'default', iat: staleIat },
4516+
key,
4517+
);
4518+
4519+
const response = await request(tApp)
4520+
.post('/mcp')
4521+
.set('Accept', 'application/json, text/event-stream')
4522+
.set('Authorization', `Bearer ${envelope}`)
4523+
.send({ jsonrpc: '2.0', id: 1, method: 'initialize', params: {} });
4524+
4525+
expect(response.status).toBe(401);
4526+
expect(mockLogger.warn).toHaveBeenCalledWith(
4527+
'Encrypted token envelope expired (iat too old)',
4528+
expect.objectContaining({ profileId: 'default' }),
4529+
);
4530+
// No session created
4531+
const profileState = (t as any).__test_profileState;
4532+
expect(profileState.sessions.size).toBe(0);
4533+
await t.stop();
4534+
});
4535+
4536+
it('Test T: envelope with creg but client already exists - registerClient NOT called, restoredClientReg=true', async () => {
4537+
const { encryptTokenPayload } = await import('../auth/token-envelope.js');
4538+
const key = buildKey();
4539+
const mockLogger = mkLogger();
4540+
const t = buildTransport({ tokenKey: key, testLogger: mockLogger });
4541+
const tApp = (t as any).app;
4542+
t.setMessageHandler(async () => ({ protocolVersion: '2025-03-26', serverInfo: { name: 'test' } }));
4543+
4544+
// Override getClient to return an existing client
4545+
const profileState = (t as any).__test_profileState;
4546+
const registerClientSpy = (t as any).__test_registerClientSpy;
4547+
profileState.oauthProvider.clientsStore.getClient = vi.fn(async () => ({ client_id: 'existing-t' }));
4548+
4549+
const envelope = encryptTokenPayload(
4550+
{
4551+
v: 1,
4552+
at: 'at-t',
4553+
rt: 'rt-t',
4554+
pid: 'default',
4555+
iat: Date.now(),
4556+
creg: { id: 'existing-t', ru: ['https://x/cb'] },
4557+
},
4558+
key,
4559+
);
4560+
4561+
await request(tApp)
4562+
.post('/mcp')
4563+
.set('Accept', 'application/json, text/event-stream')
4564+
.set('Authorization', `Bearer ${envelope}`)
4565+
.send({ jsonrpc: '2.0', id: 1, method: 'initialize', params: {} });
4566+
4567+
expect(registerClientSpy).not.toHaveBeenCalled();
4568+
expect(mockLogger.info).toHaveBeenCalledWith(
4569+
'Session restored from encrypted token envelope after restart',
4570+
expect.objectContaining({ restoredClientReg: true }),
4571+
);
4572+
await t.stop();
4573+
});
4574+
4575+
it('Test U: registerClient throws during recovery - warn logged but session still created', async () => {
4576+
const { encryptTokenPayload } = await import('../auth/token-envelope.js');
4577+
const key = buildKey();
4578+
const mockLogger = mkLogger();
4579+
const t = buildTransport({ tokenKey: key, testLogger: mockLogger });
4580+
const tApp = (t as any).app;
4581+
t.setMessageHandler(async () => ({ protocolVersion: '2025-03-26', serverInfo: { name: 'test' } }));
4582+
4583+
const profileState = (t as any).__test_profileState;
4584+
class OAuthClientStoreCapacityError extends Error {
4585+
constructor() { super('capacity exceeded'); this.name = 'OAuthClientStoreCapacityError'; }
4586+
}
4587+
profileState.oauthProvider.clientsStore.registerClient = vi.fn(async () => { throw new OAuthClientStoreCapacityError(); });
4588+
4589+
const envelope = encryptTokenPayload(
4590+
{
4591+
v: 1,
4592+
at: 'at-u',
4593+
rt: 'rt-u',
4594+
pid: 'default',
4595+
iat: Date.now(),
4596+
creg: { id: 'client-u' },
4597+
},
4598+
key,
4599+
);
4600+
4601+
const response = await request(tApp)
4602+
.post('/mcp')
4603+
.set('Accept', 'application/json, text/event-stream')
4604+
.set('Authorization', `Bearer ${envelope}`)
4605+
.send({ jsonrpc: '2.0', id: 1, method: 'initialize', params: {} });
4606+
4607+
expect(response.status).toBe(200);
4608+
expect(mockLogger.warn).toHaveBeenCalledWith(
4609+
'Failed to re-register OAuth client from envelope during restart recovery',
4610+
expect.objectContaining({ clientId: 'client-u', errorType: 'OAuthClientStoreCapacityError' }),
4611+
);
4612+
// Session still created with refresh token
4613+
const session = findOnlySession(t);
4614+
expect(session).toBeDefined();
4615+
expect(session.refreshToken).toBe('rt-u');
4616+
await t.stop();
4617+
});
4618+
4619+
it('Test V: recovered envelope populates both oauthTokensByAccessToken and inboundAuthTokenStore', async () => {
4620+
const { encryptTokenPayload } = await import('../auth/token-envelope.js');
4621+
const key = buildKey();
4622+
const t = buildTransport({ tokenKey: key });
4623+
const tApp = (t as any).app;
4624+
t.setMessageHandler(async () => ({ protocolVersion: '2025-03-26', serverInfo: { name: 'test' } }));
4625+
4626+
const envelope = encryptTokenPayload(
4627+
{
4628+
v: 1,
4629+
at: 'raw-access-v',
4630+
rt: 'refresh-v',
4631+
exp: Date.now() + 60_000,
4632+
cid: 'client-v',
4633+
sc: ['read'],
4634+
pid: 'default',
4635+
iat: Date.now(),
4636+
},
4637+
key,
4638+
);
4639+
4640+
const response = await request(tApp)
4641+
.post('/mcp')
4642+
.set('Accept', 'application/json, text/event-stream')
4643+
.set('Authorization', `Bearer ${envelope}`)
4644+
.send({ jsonrpc: '2.0', id: 1, method: 'initialize', params: {} });
4645+
4646+
expect(response.status).toBe(200);
4647+
4648+
const profileState = (t as any).__test_profileState;
4649+
// oauthTokensByAccessToken must hold rawAccessToken for upstream calls
4650+
const mapEntry = profileState.oauthTokensByAccessToken.get(envelope);
4651+
expect(mapEntry).toBeDefined();
4652+
expect(mapEntry.rawAccessToken).toBe('raw-access-v');
4653+
4654+
// inboundAuthTokenStore must hold entry so enterprise/session checks resolve
4655+
const inboundStore = (t as any).inboundAuthTokenStore;
4656+
const record = inboundStore.get(envelope);
4657+
expect(record).toBeDefined();
4658+
expect(record.principal.authType).toBe('oauth');
4659+
expect(record.principal.clientId).toBe('client-v');
4660+
4661+
await t.stop();
4662+
});
44844663
});
44854664

44864665
describe('getSessionToken', () => {
@@ -4490,6 +4669,22 @@ describeIfListen('HttpTransport', () => {
44904669
const token = transport.getSessionToken('default', sessionId);
44914670
expect(token).toBe('my-auth-token');
44924671
});
4672+
4673+
it('returns rawAccessToken when session.authToken is an encrypted envelope', () => {
4674+
const profileState = createProfileState(transport as any);
4675+
const envelopeToken = 'mcp4.v1.fake-envelope-token';
4676+
const rawAccessToken = 'real-idp-access-token';
4677+
const sessionId = (transport as any).createSession(profileState, envelopeToken);
4678+
profileState.oauthTokensByAccessToken.set(envelopeToken, {
4679+
refreshToken: 'refresh-x',
4680+
expiresAt: Date.now() + 60_000,
4681+
clientId: 'client-x',
4682+
scopes: ['read'],
4683+
rawAccessToken,
4684+
});
4685+
const token = transport.getSessionToken('default', sessionId);
4686+
expect(token).toBe(rawAccessToken);
4687+
});
44934688
});
44944689

44954690
describe('profile routing', () => {

0 commit comments

Comments
 (0)