Skip to content

Commit

Permalink
Fix legacy-client to handle fetch token errors (#291)
Browse files Browse the repository at this point in the history
  • Loading branch information
tzyl authored May 10, 2024
1 parent efcffb7 commit 6b03c55
Show file tree
Hide file tree
Showing 6 changed files with 155 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .changeset/weak-squids-remember.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@osdk/legacy-client": minor
---

Fix legacy-client to handle fetch token errors
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
75 changes: 75 additions & 0 deletions packages/legacy-client/src/oauth-client/OAuthToken.test.ts
Original file line number Diff line number Diff line change
@@ -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");
});
});
31 changes: 24 additions & 7 deletions packages/legacy-client/src/oauth-client/OAuthToken.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit 6b03c55

Please sign in to comment.