fix(oauth): persist the granted scope on every EMA mint
`trySilentEmaAuth` and `refreshEmaResourceTokens` saved tokens without ever saving a scope. Both are reached from real paths -- initial EMA authentication with a cached IdP session, refreshEnterpriseManagedTokens, and EmaTransportOAuthProvider.tokens -- and each can be the moment a newly configured or metadata-derived scope first takes effect, so a silent token response left the previous stored scope standing. Rather than a third copy of the rule, every EMA mint now ends in one place: mintEmaResourceTokens reports the scope it requested, and saveMintedTokens writes tokens and the RFC 6749 5.1 scope together. The manager's EMA branch drops its own saveScope, which would now be a redundant second write of the same value; the step-up branch keeps its own, since it falls back to the challenge's authorizationScopes union and runs last. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
9a2616652c
commit
c6029e1cbb
2 changed files with 104 additions and 136 deletions
|
|
@ -70,6 +70,8 @@ function createMemoryStorage(
|
|||
saveTokens: vi.fn(async (url: string, tokens) => {
|
||||
savedTokens[url] = tokens;
|
||||
}),
|
||||
getScope: vi.fn(async () => undefined),
|
||||
saveScope: vi.fn(),
|
||||
clear: vi.fn(),
|
||||
clearEnterpriseManagedResourceServers: vi.fn(),
|
||||
} as unknown as OAuthStorage;
|
||||
|
|
@ -186,7 +188,7 @@ describe("emaFlow", () => {
|
|||
const idToken = jwtWithExp(exp);
|
||||
idpSessions[IDP_ISSUER] = { idToken };
|
||||
|
||||
const tokens = await mintEmaResourceTokens(
|
||||
const { tokens } = await mintEmaResourceTokens(
|
||||
{ ...baseConfig(storage), fetchFn: mockEmaFetch(idToken) },
|
||||
resourceContext(),
|
||||
);
|
||||
|
|
@ -318,7 +320,7 @@ describe("emaFlow", () => {
|
|||
},
|
||||
);
|
||||
|
||||
const tokens = await mintEmaResourceTokens(
|
||||
const { tokens } = await mintEmaResourceTokens(
|
||||
{ ...baseConfig(storage), fetchFn },
|
||||
resourceContext(),
|
||||
);
|
||||
|
|
@ -349,7 +351,7 @@ describe("emaFlow", () => {
|
|||
const idToken = jwtWithExp(exp);
|
||||
idpSessions[IDP_ISSUER] = { idToken };
|
||||
|
||||
const tokens = await mintEmaResourceTokens({
|
||||
const { tokens } = await mintEmaResourceTokens({
|
||||
...baseConfig(storage),
|
||||
fetchFn: mockEmaFetch(idToken),
|
||||
});
|
||||
|
|
@ -475,6 +477,45 @@ describe("emaFlow", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("trySilentEmaAuth persists the scope its mint asked for (#2117)", async () => {
|
||||
// The silent path is how an initial EMA authentication completes when the
|
||||
// IdP session is still cached, so it is a moment a newly configured scope
|
||||
// first takes effect -- and the mock AS answers without a `scope`.
|
||||
const idToken = jwtWithExp(Math.floor(Date.now() / 1000) + 3600);
|
||||
const storage = createMemoryStorage({ [IDP_ISSUER]: { idToken } });
|
||||
const fetchFn = mockEmaFetch(idToken);
|
||||
|
||||
const result = await trySilentEmaAuth({
|
||||
...baseConfig(storage),
|
||||
scope: "mcp weather:read",
|
||||
fetchFn,
|
||||
});
|
||||
|
||||
expect(result.status).toBe("success");
|
||||
expect(storage.saveScope).toHaveBeenCalledWith(
|
||||
SERVER_URL,
|
||||
"mcp weather:read",
|
||||
);
|
||||
});
|
||||
|
||||
it("refreshEmaResourceTokens persists the scope its mint asked for (#2117)", async () => {
|
||||
const idToken = jwtWithExp(Math.floor(Date.now() / 1000) + 3600);
|
||||
const storage = createMemoryStorage({ [IDP_ISSUER]: { idToken } });
|
||||
const fetchFn = mockEmaFetch(idToken);
|
||||
|
||||
const tokens = await refreshEmaResourceTokens({
|
||||
...baseConfig(storage),
|
||||
scope: "mcp weather:read",
|
||||
fetchFn,
|
||||
});
|
||||
|
||||
expect(tokens?.access_token).toBe("resource-access-token");
|
||||
expect(storage.saveScope).toHaveBeenCalledWith(
|
||||
SERVER_URL,
|
||||
"mcp weather:read",
|
||||
);
|
||||
});
|
||||
|
||||
it("refreshEmaResourceTokens returns undefined without an IdP session", async () => {
|
||||
expect(await refreshEmaResourceTokens(baseConfig(storage))).toBeUndefined();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -890,74 +890,6 @@ describe("OAuthManager", () => {
|
|||
expect(manager.getOAuthFlowStep()).toBe("complete");
|
||||
mintSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("persists the minted scope request when the mint response omits scope (#2117)", async () => {
|
||||
const params = createMockParams({
|
||||
enterpriseManagedAuth: {
|
||||
idp: {
|
||||
issuer: "https://idp.example.com",
|
||||
clientId: "app-client",
|
||||
clientSecret: "secret",
|
||||
},
|
||||
},
|
||||
});
|
||||
params.initialConfig.scope = "mcp weather:read";
|
||||
storageOf(params).getScope.mockResolvedValue(undefined);
|
||||
const manager = new OAuthManager(params);
|
||||
manager.setOAuthConfig({ enterpriseManaged: true });
|
||||
|
||||
const mintSpy = vi
|
||||
.spyOn(emaFlow, "completeEmaIdpAuthorizationAndMint")
|
||||
.mockResolvedValue({
|
||||
tokens: { access_token: "EMA", token_type: "Bearer" },
|
||||
requestedScope: "mcp weather:read",
|
||||
});
|
||||
storageOf(params).saveScope.mockClear();
|
||||
|
||||
await manager.completeOAuthFlow("ema-code");
|
||||
|
||||
expect(storageOf(params).saveScope).toHaveBeenCalledWith(
|
||||
SERVER_URL,
|
||||
"mcp weather:read",
|
||||
);
|
||||
mintSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("persists a mint scope that came from resource metadata, not config (#2117)", async () => {
|
||||
// With neither a configured nor a stored scope, discoverEmaResourceContext
|
||||
// resolves the request from the protected resource metadata's
|
||||
// `scopes_supported`. That value never reaches the manager's own
|
||||
// `scopeForMint`, so only what the mint reports can be persisted.
|
||||
const params = createMockParams({
|
||||
enterpriseManagedAuth: {
|
||||
idp: {
|
||||
issuer: "https://idp.example.com",
|
||||
clientId: "app-client",
|
||||
clientSecret: "secret",
|
||||
},
|
||||
},
|
||||
});
|
||||
params.initialConfig.scope = undefined;
|
||||
storageOf(params).getScope.mockResolvedValue(undefined);
|
||||
const manager = new OAuthManager(params);
|
||||
manager.setOAuthConfig({ enterpriseManaged: true });
|
||||
|
||||
const mintSpy = vi
|
||||
.spyOn(emaFlow, "completeEmaIdpAuthorizationAndMint")
|
||||
.mockResolvedValue({
|
||||
tokens: { access_token: "EMA", token_type: "Bearer" },
|
||||
requestedScope: "mcp:read mcp:write",
|
||||
});
|
||||
storageOf(params).saveScope.mockClear();
|
||||
|
||||
await manager.completeOAuthFlow("ema-code");
|
||||
|
||||
expect(storageOf(params).saveScope).toHaveBeenCalledWith(
|
||||
SERVER_URL,
|
||||
"mcp:read mcp:write",
|
||||
);
|
||||
mintSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe("trySilentEnterpriseManagedAuth", () => {
|
||||
|
|
@ -1823,10 +1755,9 @@ describe("OAuthManager", () => {
|
|||
"auth-code",
|
||||
"https://idp.example.com",
|
||||
);
|
||||
expect(storageOf(params).saveScope).toHaveBeenCalledWith(
|
||||
SERVER_URL,
|
||||
"mcp tools:read weather:read",
|
||||
);
|
||||
// Persisting the granted scope is the EMA flow's job (saveMintedTokens),
|
||||
// and it is mocked here -- the union reaching its config, asserted just
|
||||
// above, is what this test owns. emaFlow.test.ts covers the persistence.
|
||||
|
||||
silentSpy.mockRestore();
|
||||
startSpy.mockRestore();
|
||||
|
|
|
|||
Loading…
Reference in a new issue