fix: prevent infinite recursion when server throws 401 after successful authentication (#945)
Co-authored-by: Vinicius Costa <viniciuscsouza@yahoo.com.br> Co-authored-by: Tyler James Leonhardt <2644648+TylerLeonhardt@users.noreply.github.com> Co-authored-by: Blust.AI <159488814+blustAI@users.noreply.github.com> Co-authored-by: Eugene <eugene@blust.ai>
This commit is contained in:
parent
058b87c163
commit
7146eede0b
2 changed files with 75 additions and 0 deletions
|
|
@ -1001,4 +1001,69 @@ describe("StreamableHTTPClientTransport", () => {
|
|||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("prevent infinite recursion when server returns 401 after successful auth", () => {
|
||||
it("should throw error when server returns 401 after successful auth", async () => {
|
||||
const message: JSONRPCMessage = {
|
||||
jsonrpc: "2.0",
|
||||
method: "test",
|
||||
params: {},
|
||||
id: "test-id"
|
||||
};
|
||||
|
||||
// Mock provider with refresh token to enable token refresh flow
|
||||
mockAuthProvider.tokens.mockResolvedValue({
|
||||
access_token: "test-token",
|
||||
token_type: "Bearer",
|
||||
refresh_token: "refresh-token",
|
||||
});
|
||||
|
||||
const unauthedResponse = {
|
||||
ok: false,
|
||||
status: 401,
|
||||
statusText: "Unauthorized",
|
||||
headers: new Headers()
|
||||
};
|
||||
|
||||
(global.fetch as jest.Mock)
|
||||
// First request - 401, triggers auth flow
|
||||
.mockResolvedValueOnce(unauthedResponse)
|
||||
// Resource discovery, path aware
|
||||
.mockResolvedValueOnce(unauthedResponse)
|
||||
// Resource discovery, root
|
||||
.mockResolvedValueOnce(unauthedResponse)
|
||||
// OAuth metadata discovery
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({
|
||||
issuer: "http://localhost:1234",
|
||||
authorization_endpoint: "http://localhost:1234/authorize",
|
||||
token_endpoint: "http://localhost:1234/token",
|
||||
response_types_supported: ["code"],
|
||||
code_challenge_methods_supported: ["S256"],
|
||||
}),
|
||||
})
|
||||
// Token refresh succeeds
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({
|
||||
access_token: "new-access-token",
|
||||
token_type: "Bearer",
|
||||
expires_in: 3600,
|
||||
}),
|
||||
})
|
||||
// Retry the original request - still 401 (broken server)
|
||||
.mockResolvedValueOnce(unauthedResponse);
|
||||
|
||||
await expect(transport.send(message)).rejects.toThrow("Server returned 401 after successful authentication");
|
||||
expect(mockAuthProvider.saveTokens).toHaveBeenCalledWith({
|
||||
access_token: "new-access-token",
|
||||
token_type: "Bearer",
|
||||
expires_in: 3600,
|
||||
refresh_token: "refresh-token", // Refresh token is preserved
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -131,6 +131,7 @@ export class StreamableHTTPClientTransport implements Transport {
|
|||
private _sessionId?: string;
|
||||
private _reconnectionOptions: StreamableHTTPReconnectionOptions;
|
||||
private _protocolVersion?: string;
|
||||
private _hasCompletedAuthFlow = false; // Circuit breaker: detect auth success followed by immediate 401
|
||||
|
||||
onclose?: () => void;
|
||||
onerror?: (error: Error) => void;
|
||||
|
|
@ -437,6 +438,10 @@ export class StreamableHTTPClientTransport implements Transport {
|
|||
|
||||
if (!response.ok) {
|
||||
if (response.status === 401 && this._authProvider) {
|
||||
// Prevent infinite recursion when server returns 401 after successful auth
|
||||
if (this._hasCompletedAuthFlow) {
|
||||
throw new StreamableHTTPError(401, "Server returned 401 after successful authentication");
|
||||
}
|
||||
|
||||
this._resourceMetadataUrl = extractResourceMetadataUrl(response);
|
||||
|
||||
|
|
@ -445,6 +450,8 @@ export class StreamableHTTPClientTransport implements Transport {
|
|||
throw new UnauthorizedError();
|
||||
}
|
||||
|
||||
// Mark that we completed auth flow
|
||||
this._hasCompletedAuthFlow = true;
|
||||
// Purposely _not_ awaited, so we don't call onerror twice
|
||||
return this.send(message);
|
||||
}
|
||||
|
|
@ -455,6 +462,9 @@ export class StreamableHTTPClientTransport implements Transport {
|
|||
);
|
||||
}
|
||||
|
||||
// Reset auth loop flag on successful response
|
||||
this._hasCompletedAuthFlow = false;
|
||||
|
||||
// If the response is 202 Accepted, there's no body to process
|
||||
if (response.status === 202) {
|
||||
// if the accepted notification is initialized, we start the SSE stream
|
||||
|
|
|
|||
Loading…
Reference in a new issue