Skip to content

Commit ac538e3

Browse files
coding-boboclaude
andcommitted
fix: address review feedback on invalidation scopes and example AS policy
invalidateCredentials('discovery') now drops the cached issuer and resource URLs so a callback cannot mint against a stale audience. The example AS binds the issued identity and scopes to the verified workload instead of copying them from the request, and the projected token file is created with O_EXCL so a pre-planted symlink fails closed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 535fadb commit ac538e3

3 files changed

Lines changed: 66 additions & 12 deletions

File tree

examples/oauth-workload-identity/server.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
* API socket instead of a temp file this process wrote.
2626
*/
2727
import { randomUUID } from 'node:crypto';
28-
import { writeFileSync } from 'node:fs';
28+
import { rmSync, writeFileSync } from 'node:fs';
2929
import { tmpdir } from 'node:os';
3030
import path from 'node:path';
3131

@@ -78,7 +78,10 @@ const workloadJwt = await new jose.SignJWT({})
7878
// out-of-band handshake. In a pod this path is the projected-volume mount point
7979
// (`/var/run/secrets/tokens/...`); `WIF_WORKLOAD_TOKEN_PATH` overrides it.
8080
const tokenPath = process.env.WIF_WORKLOAD_TOKEN_PATH ?? path.join(tmpdir(), `mcp-wif-workload-token-${port}.jwt`);
81-
writeFileSync(tokenPath, workloadJwt, { mode: 0o600 });
81+
// `wx` (O_CREAT | O_EXCL) refuses to follow a pre-planted symlink at this
82+
// well-known path and fails closed if anything reappears between rm and open.
83+
rmSync(tokenPath, { force: true });
84+
writeFileSync(tokenPath, workloadJwt, { mode: 0o600, flag: 'wx' });
8285

8386
// ---- Authorization Server (jwt-bearer only) ----
8487
const metadata: OAuthMetadata = {
@@ -133,12 +136,25 @@ asApp.post('/token', async (req, res) => {
133136
res.status(400).json({ error: 'invalid_grant' });
134137
return;
135138
}
139+
// Bind the issued identity to the verified workload, not to request
140+
// parameters: federation policy decides what this subject may act as. A
141+
// request may omit client_id entirely (the assertion is the credential),
142+
// but it must not claim a different one.
143+
if (body.client_id !== undefined && body.client_id !== DEMO_CLIENT_ID) {
144+
console.error(`[auth-server] client_id ${body.client_id} is not federated to ${WORKLOAD_SUBJECT}`);
145+
res.status(400).json({ error: 'invalid_grant' });
146+
return;
147+
}
136148
const scopes = (body.scope ?? '').split(' ').filter(Boolean);
149+
if (!scopes.every(scope => metadata.scopes_supported!.includes(scope))) {
150+
res.status(400).json({ error: 'invalid_scope' });
151+
return;
152+
}
137153
const accessToken = randomUUID();
138154
const expiresIn = 300;
139155
issuedTokens.set(accessToken, {
140156
token: accessToken,
141-
clientId: body.client_id ?? DEMO_CLIENT_ID,
157+
clientId: DEMO_CLIENT_ID,
142158
scopes,
143159
expiresAt: Math.floor(Date.now() / 1000) + expiresIn,
144160
extra: { workloadSubject: WORKLOAD_SUBJECT }

packages/client/src/client/authExtensions.ts

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -904,17 +904,35 @@ export class WorkloadIdentityProvider implements OAuthClientProvider {
904904
* the verdict may name the most recently handed-out assertion rather than the
905905
* exact one the failing flow sent; that is conservative and fails closed.
906906
* `'all'` is a host-driven reset rather than a rejection, so it clears the
907-
* memory instead of recording one. The remaining scopes name state this
908-
* provider does not keep.
907+
* memory instead of recording one. `'discovery'` drops the cached
908+
* authorization server and resource URLs so a later flow cannot mint an
909+
* assertion against a stale issuer; `auth()` repopulates them on its next
910+
* discovery pass. `'client'` and `'verifier'` name state this provider does
911+
* not keep.
909912
*/
910913
invalidateCredentials(scope: 'all' | 'client' | 'tokens' | 'verifier' | 'discovery'): void {
911-
if (scope === 'tokens') {
912-
this._tokens = undefined;
913-
this._rejectedAssertion = this._lastAssertion;
914-
} else if (scope === 'all') {
915-
this._tokens = undefined;
916-
this._lastAssertion = undefined;
917-
this._rejectedAssertion = undefined;
914+
switch (scope) {
915+
case 'tokens': {
916+
this._tokens = undefined;
917+
this._rejectedAssertion = this._lastAssertion;
918+
break;
919+
}
920+
case 'discovery': {
921+
this._authorizationServerUrl = undefined;
922+
this._resourceUrl = undefined;
923+
break;
924+
}
925+
case 'all': {
926+
this._tokens = undefined;
927+
this._lastAssertion = undefined;
928+
this._rejectedAssertion = undefined;
929+
this._authorizationServerUrl = undefined;
930+
this._resourceUrl = undefined;
931+
break;
932+
}
933+
default: {
934+
break;
935+
}
918936
}
919937
}
920938

packages/client/test/client/workloadIdentity.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,26 @@ describe('WorkloadIdentityProvider rejection memory', () => {
331331

332332
expect(provider.tokens()?.access_token).toBe('stored-token');
333333
});
334+
335+
it("drops cached discovery URLs on invalidateCredentials('discovery') but keeps tokens", () => {
336+
const provider = makeProvider();
337+
provider.saveTokens({ access_token: 'stored-token', token_type: 'Bearer' });
338+
339+
provider.invalidateCredentials('discovery');
340+
341+
expect(provider.authorizationServerUrl()).toBeUndefined();
342+
expect(provider.resourceUrl?.()).toBeUndefined();
343+
expect(provider.tokens()?.access_token).toBe('stored-token');
344+
});
345+
346+
it("drops cached discovery URLs on invalidateCredentials('all')", () => {
347+
const provider = makeProvider();
348+
349+
provider.invalidateCredentials('all');
350+
351+
expect(provider.authorizationServerUrl()).toBeUndefined();
352+
expect(provider.resourceUrl?.()).toBeUndefined();
353+
});
334354
});
335355

336356
describe('WorkloadIdentityProvider (end-to-end with auth())', () => {

0 commit comments

Comments
 (0)