diff --git a/.changeset/gold-flies-attend.md b/.changeset/gold-flies-attend.md new file mode 100644 index 00000000000..3ac68cbd501 --- /dev/null +++ b/.changeset/gold-flies-attend.md @@ -0,0 +1,6 @@ +--- +'@clerk/clerk-js': patch +'@clerk/types': patch +--- + +Update token refresh mechanism to handle network failures without raising an error diff --git a/packages/clerk-js/src/core/resources/Session.test.ts b/packages/clerk-js/src/core/resources/Session.test.ts index f705632e763..5969dbffc91 100644 --- a/packages/clerk-js/src/core/resources/Session.test.ts +++ b/packages/clerk-js/src/core/resources/Session.test.ts @@ -1,14 +1,12 @@ import type { SessionJSON } from '@clerk/types'; import { eventBus } from '../events'; -import { clerkMock, createUser } from '../test/fixtures'; +import createFapiClient from '../fapiClient'; +import { clerkMock, createUser, mockDevClerkInstance, mockJwt, mockNetworkFailedFetch } from '../test/fixtures'; import { BaseResource, Session } from './internal'; -export const mockJwt = - 'eyJhbGciOiJSUzI1NiIsImtpZCI6Imluc18yR0lvUWhiVXB5MGhYN0IyY1ZrdVRNaW5Yb0QiLCJ0eXAiOiJKV1QifQ.eyJhenAiOiJodHRwczovL2FjY291bnRzLmluc3BpcmVkLnB1bWEtNzQubGNsLmRldiIsImV4cCI6MTY2NjY0ODMxMCwiaWF0IjoxNjY2NjQ4MjUwLCJpc3MiOiJodHRwczovL2NsZXJrLmluc3BpcmVkLnB1bWEtNzQubGNsLmRldiIsIm5iZiI6MTY2NjY0ODI0MCwic2lkIjoic2Vzc18yR2JEQjRlbk5kQ2E1dlMxenBDM1h6Zzl0SzkiLCJzdWIiOiJ1c2VyXzJHSXBYT0VwVnlKdzUxcmtabjlLbW5jNlN4ciJ9.n1Usc-DLDftqA0Xb-_2w8IGs4yjCmwc5RngwbSRvwevuZOIuRoeHmE2sgCdEvjfJEa7ewL6EVGVcM557TWPW--g_J1XQPwBy8tXfz7-S73CEuyRFiR97L2AHRdvRtvGtwR-o6l8aHaFxtlmfWbQXfg4kFJz2UGe9afmh3U9-f_4JOZ5fa3mI98UMy1-bo20vjXeWQ9aGrqaxHQxjnzzC-1Kpi5LdPvhQ16H0dPB8MHRTSM5TAuLKTpPV7wqixmbtcc2-0k6b9FKYZNqRVTaIyV-lifZloBvdzlfOF8nW1VVH_fx-iW5Q3hovHFcJIULHEC1kcAYTubbxzpgeVQepGg'; - describe('Session', () => { - describe('getToken()', () => { + describe('creating new session', () => { let dispatchSpy; beforeEach(() => { @@ -19,42 +17,98 @@ describe('Session', () => { afterEach(() => { dispatchSpy?.mockRestore(); BaseResource.clerk = null as any; + // @ts-ignore + global.fetch?.mockClear(); }); - it('dispatches token:update event on getToken', async () => { - const session = new Session({ + it('dispatches token:update event on initilization with lastActiveToken', () => { + new Session({ status: 'active', id: 'session_1', object: 'session', user: createUser({}), last_active_organization_id: 'activeOrganization', + last_active_token: { object: 'token', jwt: mockJwt }, actor: null, created_at: new Date().getTime(), updated_at: new Date().getTime(), } as SessionJSON); - await session.getToken(); - expect(dispatchSpy).toBeCalledTimes(1); expect(dispatchSpy.mock.calls[0]).toMatchSnapshot(); }); + }); - it('dispatches token:update event on initilization with lastActiveToken', () => { - new Session({ + describe('getToken()', () => { + let dispatchSpy; + + beforeEach(() => { + dispatchSpy = jest.spyOn(eventBus, 'dispatch'); + BaseResource.clerk = clerkMock() as any; + }); + + afterEach(() => { + dispatchSpy?.mockRestore(); + BaseResource.clerk = null as any; + }); + + it('dispatches token:update event on getToken', async () => { + const session = new Session({ status: 'active', id: 'session_1', object: 'session', user: createUser({}), last_active_organization_id: 'activeOrganization', - last_active_token: { object: 'token', jwt: mockJwt }, actor: null, created_at: new Date().getTime(), updated_at: new Date().getTime(), } as SessionJSON); + await session.getToken(); + expect(dispatchSpy).toBeCalledTimes(1); expect(dispatchSpy.mock.calls[0]).toMatchSnapshot(); }); + + describe('with offline browser and network failure', () => { + let warnSpy; + beforeEach(() => { + Object.defineProperty(window.navigator, 'onLine', { + writable: true, + value: false, + }); + warnSpy = jest.spyOn(console, 'warn').mockReturnValue(); + }); + + afterEach(() => { + Object.defineProperty(window.navigator, 'onLine', { + writable: true, + value: true, + }); + warnSpy.mockRestore(); + }); + + it('returns null', async () => { + const session = new Session({ + status: 'active', + id: 'session_1', + object: 'session', + user: createUser({}), + last_active_organization_id: 'activeOrganization', + actor: null, + created_at: new Date().getTime(), + updated_at: new Date().getTime(), + } as SessionJSON); + + mockNetworkFailedFetch(); + BaseResource.clerk = { getFapiClient: () => createFapiClient(mockDevClerkInstance) } as any; + + const token = await session.getToken(); + + expect(dispatchSpy).toBeCalledTimes(1); + expect(token).toEqual(null); + }); + }); }); describe('isAuthorized()', () => { diff --git a/packages/clerk-js/src/core/resources/Session.ts b/packages/clerk-js/src/core/resources/Session.ts index e496715f8be..c35c1fcbd6d 100644 --- a/packages/clerk-js/src/core/resources/Session.ts +++ b/packages/clerk-js/src/core/resources/Session.ts @@ -130,11 +130,11 @@ export class Session extends BaseResource implements SessionResource { }; // Can be removed once `integration_firebase` are no longer supported - #handleLegacyIntegrationToken = async (options: GetTokenOptions): Promise => { + #handleLegacyIntegrationToken = async (options: GetTokenOptions): Promise => { const { template, leewayInSeconds } = options; const cachedEntry = SessionTokenCache.get({ tokenId: this.user!.id, audience: template }, leewayInSeconds); if (cachedEntry) { - return cachedEntry.tokenResolver.then(res => res.getRawString()); + return cachedEntry.tokenResolver.then(token => this.#getRawToken(token)); } const tokenResolver = Token.create(this.user!.pathRoot + '/tokens', { service: this.#removeLegacyIntegrationPrefix(template), @@ -144,11 +144,16 @@ export class Session extends BaseResource implements SessionResource { audience: template, tokenResolver, }); - return tokenResolver.then(token => { - return token.getRawString(); - }); + return tokenResolver.then(token => this.#getRawToken(token)); }; + /** + * Return null when raw string is empty to indicate that it's signed-out + */ + #getRawToken(tokenResolver: TokenResource): string | null { + return tokenResolver.getRawString() || null; + } + protected fromJSON(data: SessionJSON | null): this { if (!data) { return this; @@ -192,11 +197,11 @@ export class Session extends BaseResource implements SessionResource { const cachedEntry = skipCache ? undefined : SessionTokenCache.get({ tokenId }, leewayInSeconds); if (cachedEntry) { - const cachedToken = await cachedEntry.tokenResolver.then(res => res); + const cachedToken = await cachedEntry.tokenResolver; if (!template) { eventBus.dispatch(events.TokenUpdate, { token: cachedToken }); } - return cachedToken.getRawString(); + return this.#getRawToken(cachedToken); } const path = template ? `${this.path()}/tokens/${template}` : `${this.path()}/tokens`; const tokenResolver = Token.create(path); @@ -206,7 +211,7 @@ export class Session extends BaseResource implements SessionResource { if (!template) { eventBus.dispatch(events.TokenUpdate, { token }); } - return token.getRawString(); + return this.#getRawToken(token); }); } } diff --git a/packages/clerk-js/src/core/resources/Token.test.ts b/packages/clerk-js/src/core/resources/Token.test.ts new file mode 100644 index 00000000000..f283800895a --- /dev/null +++ b/packages/clerk-js/src/core/resources/Token.test.ts @@ -0,0 +1,98 @@ +import createFapiClient from '../fapiClient'; +import { mockDevClerkInstance, mockFetch, mockNetworkFailedFetch } from '../test/fixtures'; +import { BaseResource } from './internal'; +import { Token } from './Token'; + +describe('Token', () => { + describe('create', () => { + afterEach(() => { + // @ts-ignore + global.fetch?.mockClear(); + BaseResource.clerk = null as any; + }); + + it('with http 500 throws error', async () => { + mockFetch(false, 500); + BaseResource.clerk = { getFapiClient: () => createFapiClient(mockDevClerkInstance) } as any; + + await expect(Token.create('/path/to/tokens')).rejects.toMatchObject({ + message: '500', + }); + + expect(global.fetch).toHaveBeenCalledWith( + 'https://clerk.example.com/v1/path/to/tokens?_clerk_js_version=test-0.0.0', + // TODO(dimkl): omit extra params from fetch request (eg path, url) - remove expect.objectContaining + expect.objectContaining({ + method: 'POST', + body: '', + credentials: 'include', + headers: new Headers(), + }), + ); + }); + + describe('with offline browser and network failure', () => { + let warnSpy; + + beforeEach(() => { + Object.defineProperty(window.navigator, 'onLine', { + writable: true, + value: false, + }); + warnSpy = jest.spyOn(console, 'warn').mockReturnValue(); + }); + + afterEach(() => { + Object.defineProperty(window.navigator, 'onLine', { + writable: true, + value: true, + }); + warnSpy.mockRestore(); + }); + + it('create returns empty raw string', async () => { + mockNetworkFailedFetch(); + BaseResource.clerk = { getFapiClient: () => createFapiClient(mockDevClerkInstance) } as any; + + const token = await Token.create('/path/to/tokens'); + + expect(global.fetch).toHaveBeenCalledWith( + 'https://clerk.example.com/v1/path/to/tokens?_clerk_js_version=test-0.0.0', + // TODO(dimkl): omit extra params from fetch request (eg path, url) - remove expect.objectContaining + expect.objectContaining({ + method: 'POST', + body: '', + credentials: 'include', + headers: new Headers(), + }), + ); + + expect(token.getRawString()).toEqual(''); + expect(warnSpy).toBeCalled(); + }); + }); + + describe('with online browser and network failure', () => { + it('throws error', async () => { + mockNetworkFailedFetch(); + BaseResource.clerk = { getFapiClient: () => createFapiClient(mockDevClerkInstance) } as any; + + await expect(Token.create('/path/to/tokens')).rejects.toMatchObject({ + message: + 'ClerkJS: Network error at "https://clerk.example.com/v1/path/to/tokens?_clerk_js_version=test-0.0.0" - TypeError: Failed to fetch. Please try again.', + }); + + expect(global.fetch).toHaveBeenCalledWith( + 'https://clerk.example.com/v1/path/to/tokens?_clerk_js_version=test-0.0.0', + // TODO(dimkl): omit extra params from fetch request (eg path, url) - remove expect.objectContaining + expect.objectContaining({ + method: 'POST', + body: '', + credentials: 'include', + headers: new Headers(), + }), + ); + }); + }); + }); +}); diff --git a/packages/clerk-js/src/core/resources/Token.ts b/packages/clerk-js/src/core/resources/Token.ts index 9471cb468ca..0dbf3fc4846 100644 --- a/packages/clerk-js/src/core/resources/Token.ts +++ b/packages/clerk-js/src/core/resources/Token.ts @@ -6,7 +6,7 @@ import { BaseResource } from './internal'; export class Token extends BaseResource implements TokenResource { pathRoot = 'tokens'; - jwt: JWT; + jwt?: JWT; static async create(path: string, body: any = {}): Promise { const json = (await BaseResource._fetch({ @@ -18,18 +18,20 @@ export class Token extends BaseResource implements TokenResource { return new Token(json, path); } - constructor(data: TokenJSON, pathRoot?: string) { + constructor(data: TokenJSON | null, pathRoot?: string) { super(); if (pathRoot) { this.pathRoot = pathRoot; } - this.jwt = decode(data.jwt); + if (data?.jwt) { + this.jwt = decode(data.jwt); + } } getRawString = (): string => { - return this.jwt?.claims.__raw; + return this.jwt?.claims.__raw || ''; }; protected fromJSON(data: TokenJSON | null): this { diff --git a/packages/clerk-js/src/core/resources/__snapshots__/Session.test.ts.snap b/packages/clerk-js/src/core/resources/__snapshots__/Session.test.ts.snap index 040c50d4a7f..7308764b6b8 100644 --- a/packages/clerk-js/src/core/resources/__snapshots__/Session.test.ts.snap +++ b/packages/clerk-js/src/core/resources/__snapshots__/Session.test.ts.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Session getToken() dispatches token:update event on getToken 1`] = ` +exports[`Session creating new session dispatches token:update event on initilization with lastActiveToken 1`] = ` [ "token:update", { @@ -28,13 +28,13 @@ exports[`Session getToken() dispatches token:update event on getToken 1`] = ` "typ": "JWT", }, }, - "pathRoot": "/client/sessions/session_1/tokens", + "pathRoot": "tokens", }, }, ] `; -exports[`Session getToken() dispatches token:update event on initilization with lastActiveToken 1`] = ` +exports[`Session getToken() dispatches token:update event on getToken 1`] = ` [ "token:update", { @@ -62,7 +62,7 @@ exports[`Session getToken() dispatches token:update event on initilization with "typ": "JWT", }, }, - "pathRoot": "tokens", + "pathRoot": "/client/sessions/session_1/tokens", }, }, ] diff --git a/packages/clerk-js/src/core/services/authentication/SessionCookieService.ts b/packages/clerk-js/src/core/services/authentication/SessionCookieService.ts index ad75670e4b9..58b7afab8ec 100644 --- a/packages/clerk-js/src/core/services/authentication/SessionCookieService.ts +++ b/packages/clerk-js/src/core/services/authentication/SessionCookieService.ts @@ -1,5 +1,5 @@ import { is4xxError, isClerkAPIResponseError, isNetworkError } from '@clerk/shared/error'; -import type { Clerk, EnvironmentResource, SessionResource, TokenResource } from '@clerk/types'; +import type { Clerk, EnvironmentResource, SessionResource } from '@clerk/types'; import type { CookieHandler } from '../../../utils'; import { createCookieHandler, inBrowser } from '../../../utils'; @@ -71,12 +71,16 @@ export class SessionCookieService { return this.clerk.session?.getToken(); } - private setSessionCookie(token: TokenResource | string) { - this.cookies.setSessionCookie(typeof token === 'string' ? token : token.getRawString()); + private setSessionCookie(token: string) { + this.cookies.setSessionCookie(token); } - private updateSessionCookie(token: TokenResource | string | undefined | null) { - return token ? this.setSessionCookie(token) : this.removeSessionCookie(); + private updateSessionCookie(token: string | undefined | null) { + if (token) { + return this.setSessionCookie(token); + } + + return this.removeSessionCookie(); } private removeSessionCookie() { diff --git a/packages/clerk-js/src/core/test/fixtures.ts b/packages/clerk-js/src/core/test/fixtures.ts index ad335354ca5..e7020940d6d 100644 --- a/packages/clerk-js/src/core/test/fixtures.ts +++ b/packages/clerk-js/src/core/test/fixtures.ts @@ -251,3 +251,34 @@ export const clerkMock = () => { }), }; }; + +type RecursivePartial = { + [P in keyof T]?: RecursivePartial; +}; + +export const mockFetch = (ok = true, status = 200, responsePayload = {}) => { + // @ts-ignore + global.fetch = jest.fn(() => { + return Promise.resolve>({ + status, + statusText: status.toString(), + ok, + json: () => Promise.resolve(responsePayload), + }); + }); +}; + +export const mockNetworkFailedFetch = () => { + // @ts-ignore + global.fetch = jest.fn(() => { + return Promise.reject(new TypeError('Failed to fetch')); + }); +}; + +export const mockDevClerkInstance = { + frontendApi: 'clerk.example.com', + instanceType: 'development', + isSatellite: false, + version: 'test-0.0.0', + domain: '', +}; diff --git a/packages/clerk-js/src/core/tokenCache.ts b/packages/clerk-js/src/core/tokenCache.ts index b2f4a3f2fbd..343130adb54 100644 --- a/packages/clerk-js/src/core/tokenCache.ts +++ b/packages/clerk-js/src/core/tokenCache.ts @@ -80,6 +80,10 @@ const MemoryTokenCache = (prefix = KEY_PREFIX): TokenCache => { entry.tokenResolver .then(newToken => { + if (!newToken.jwt) { + return deleteKey(); + } + const expiresAt = newToken.jwt.claims.exp; const issuedAt = newToken.jwt.claims.iat; const expiresIn: Seconds = expiresAt - issuedAt; diff --git a/packages/types/src/token.ts b/packages/types/src/token.ts index 133271c3f9d..863b1c406b5 100644 --- a/packages/types/src/token.ts +++ b/packages/types/src/token.ts @@ -2,6 +2,6 @@ import type { JWT } from './jwt'; import type { ClerkResource } from './resource'; export interface TokenResource extends ClerkResource { - jwt: JWT; + jwt?: JWT; getRawString: () => string; }