From 6b03c558dc7182bb471e5bc063838f22ff1b1af2 Mon Sep 17 00:00:00 2001 From: Tim Leung Date: Fri, 10 May 2024 12:27:52 +0100 Subject: [PATCH] Fix legacy-client to handle fetch token errors (#291) --- .changeset/weak-squids-remember.md | 5 ++ .../ConfidentialClientFlow.test.ts | 33 ++++++++ .../src/oauth-client/OAuthToken.test.ts | 75 +++++++++++++++++++ .../src/oauth-client/OAuthToken.ts | 31 ++++++-- .../PublicClient/PublicClientAuth.ts | 17 +++-- .../oauth-client/utils/fetchFormEncoded.ts | 6 ++ 6 files changed, 155 insertions(+), 12 deletions(-) create mode 100644 .changeset/weak-squids-remember.md create mode 100644 packages/legacy-client/src/oauth-client/OAuthToken.test.ts diff --git a/.changeset/weak-squids-remember.md b/.changeset/weak-squids-remember.md new file mode 100644 index 000000000..34f5a1905 --- /dev/null +++ b/.changeset/weak-squids-remember.md @@ -0,0 +1,5 @@ +--- +"@osdk/legacy-client": minor +--- + +Fix legacy-client to handle fetch token errors diff --git a/packages/legacy-client/src/oauth-client/ConfidentialClient/ConfidentialClientFlow.test.ts b/packages/legacy-client/src/oauth-client/ConfidentialClient/ConfidentialClientFlow.test.ts index 6622e34b2..857b62584 100644 --- a/packages/legacy-client/src/oauth-client/ConfidentialClient/ConfidentialClientFlow.test.ts +++ b/packages/legacy-client/src/oauth-client/ConfidentialClient/ConfidentialClientFlow.test.ts @@ -71,6 +71,39 @@ describe("ConfidentialClientFlow", () => { expect(token.expiresIn).approximately(3600, 1); }); + it("getTokenWithClientSecret with fetch error", async () => { + const mockFetch: Mock = vi.fn(); + mockFetch.mockResolvedValue({ + ok: false, + status: 400, + statusText: "Bad Request", + }); + + await expect(() => + getTokenWithClientSecret( + clientId, + clientSecret, + url, + mockFetch, + undefined, + scopes, + ) + ).rejects.toThrow("400 Bad Request"); + + expect(mockFetch).toBeCalledWith( + `https://example.com/multipass/api/oauth2/token`, + { + body: + "grant_type=client_credentials&client_id=testClientId&client_secret=testClientSecret&scopes=offline_access+api%3Aread+api%3Awrite", + cache: "no-cache", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + method: "POST", + }, + ); + }); + it("revokeTokenWithClientSecret", async () => { const mockFetch: Mock = vi.fn(); mockFetch.mockResolvedValue({ diff --git a/packages/legacy-client/src/oauth-client/OAuthToken.test.ts b/packages/legacy-client/src/oauth-client/OAuthToken.test.ts new file mode 100644 index 000000000..48f97bc7c --- /dev/null +++ b/packages/legacy-client/src/oauth-client/OAuthToken.test.ts @@ -0,0 +1,75 @@ +/* + * Copyright 2024 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { describe, expect, it } from "vitest"; +import { OAuthToken } from "./OAuthToken"; + +describe("OAuthToken", () => { + it("constructs successfully", () => { + const tokenResponse = { + access_token: "testAccessToken", + token_type: "Bearer", + expires_in: 3600, + }; + + const token = new OAuthToken(tokenResponse); + expect(token).toBeInstanceOf(OAuthToken); + expect(token.accessToken).toEqual("testAccessToken"); + expect(token.tokenType === "Bearer"); + expect(token.expiresIn).approximately(3600, 1); + expect(token.refreshToken).toBeUndefined(); + }); + + it("constructs successfully with refresh token", () => { + const tokenResponse = { + access_token: "testAccessToken", + token_type: "Bearer", + expires_in: 3600, + refresh_token: "testRefreshToken", + }; + + const token = new OAuthToken(tokenResponse); + expect(token).toBeInstanceOf(OAuthToken); + expect(token.accessToken).toEqual("testAccessToken"); + expect(token.tokenType === "Bearer"); + expect(token.expiresIn).approximately(3600, 1); + expect(token.refreshToken).toEqual("testRefreshToken"); + }); + + it("fails when required token response fields are missing", () => { + expect( + () => + new OAuthToken({ + token_type: "Bearer", + expires_in: 3600, + } as any), + ).toThrow("missing access_token"); + expect( + () => + new OAuthToken({ + access_token: "testAccessToken", + expires_in: 3600, + } as any), + ).toThrow("missing token_type"); + expect( + () => + new OAuthToken({ + access_token: "testAccessToken", + token_type: "Bearer", + } as any), + ).toThrow("missing expires_in"); + }); +}); diff --git a/packages/legacy-client/src/oauth-client/OAuthToken.ts b/packages/legacy-client/src/oauth-client/OAuthToken.ts index 721d65ead..7d98aa714 100644 --- a/packages/legacy-client/src/oauth-client/OAuthToken.ts +++ b/packages/legacy-client/src/oauth-client/OAuthToken.ts @@ -15,19 +15,24 @@ */ import type { Token, TokenValue } from "./Token"; + +interface TokenResponse { + access_token: TokenValue; + token_type: string; + expires_in: number; + refresh_token?: string; +} + export class OAuthToken implements Token { accessToken: TokenValue; tokenType: string; - refreshToken: string; + refreshToken?: string; /** The epoch milliseconds when the access token will expire. */ expiresAt: number; - constructor(tokenResponse: { - access_token: TokenValue; - token_type: string; - refresh_token: string; - expires_in: number; - }) { + constructor(tokenResponse: TokenResponse) { + this.checkTokenResponse(tokenResponse); + this.accessToken = tokenResponse.access_token; this.tokenType = tokenResponse.token_type; this.refreshToken = tokenResponse.refresh_token; @@ -44,4 +49,16 @@ export class OAuthToken implements Token { get isExpired(): boolean { return Date.now() >= this.expiresAt; } + + private checkTokenResponse(tokenResponse: TokenResponse): void { + if (tokenResponse.access_token == null) { + throw new Error("missing access_token"); + } + if (tokenResponse.token_type == null) { + throw new Error("missing token_type"); + } + if (tokenResponse.expires_in == null) { + throw new Error("missing expires_in"); + } + } } diff --git a/packages/legacy-client/src/oauth-client/PublicClient/PublicClientAuth.ts b/packages/legacy-client/src/oauth-client/PublicClient/PublicClientAuth.ts index 89b5c4873..446ffffa0 100644 --- a/packages/legacy-client/src/oauth-client/PublicClient/PublicClientAuth.ts +++ b/packages/legacy-client/src/oauth-client/PublicClient/PublicClientAuth.ts @@ -132,10 +132,12 @@ export class PublicClientAuth implements Auth { this.options.fetchFn, this.options.multipassContextPath, ); - localStorage.setItem( - this.palantirRefreshToken, - this.token.refreshToken, - ); + if (this.token.refreshToken != null) { + localStorage.setItem( + this.palantirRefreshToken, + this.token.refreshToken, + ); + } shouldMakeAuthRequest = false; } catch (e) { // eslint-disable-next-line no-console @@ -310,7 +312,12 @@ export class PublicClientAuth implements Auth { this.options.fetchFn, this.options.multipassContextPath, ); - localStorage.setItem(this.palantirRefreshToken, this.token.refreshToken); + if (this.token.refreshToken != null) { + localStorage.setItem( + this.palantirRefreshToken, + this.token.refreshToken, + ); + } return true; } catch (e) { // eslint-disable-next-line no-console diff --git a/packages/legacy-client/src/oauth-client/utils/fetchFormEncoded.ts b/packages/legacy-client/src/oauth-client/utils/fetchFormEncoded.ts index 2003e223f..dab56dcb7 100644 --- a/packages/legacy-client/src/oauth-client/utils/fetchFormEncoded.ts +++ b/packages/legacy-client/src/oauth-client/utils/fetchFormEncoded.ts @@ -28,5 +28,11 @@ export async function fetchFormEncoded( cache: "no-cache", }); + if (!response.ok) { + throw new Error( + `Response was not ok: ${response.status} ${response.statusText}`, + ); + } + return response; }