From 06479695d1fd4eccb05a912e5fa1c20385fd1cfa Mon Sep 17 00:00:00 2001 From: Splurge Budget <124757361+splurgebudget@users.noreply.github.com> Date: Wed, 26 Jul 2023 10:56:09 -0600 Subject: [PATCH] Use async/await to allow for async storage. (#61) Redo the way requests queue in the interceptor to remove a race condition. Bump the major version --- package.json | 2 +- src/BrowserStorageService.ts | 6 +- src/StorageType.ts | 6 +- src/authTokenInterceptor.ts | 95 +++++++++--------------------- src/setAuthTokens.ts | 4 +- src/tokensUtils.ts | 25 ++++---- tests/authTokenInterceptor.test.ts | 6 +- tests/clearAuthTokens.test.ts | 9 ++- tests/getAccessToken.test.ts | 26 ++++---- tests/getRefreshToken.test.ts | 12 ++-- tests/isLoggedIn.test.ts | 12 ++-- tests/setAccessToken.test.ts | 25 ++++---- tests/setAuthTokens.test.ts | 8 +-- 13 files changed, 101 insertions(+), 135 deletions(-) diff --git a/package.json b/package.json index d732e37..ae7cfd9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "axios-jwt", - "version": "3.0.2", + "version": "4.0.0", "description": "Axios interceptor to store, use, and refresh tokens for authentication.", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/src/BrowserStorageService.ts b/src/BrowserStorageService.ts index 655bc3e..db1d285 100644 --- a/src/BrowserStorageService.ts +++ b/src/BrowserStorageService.ts @@ -5,15 +5,15 @@ export class BrowserStorageService { this._storage = storage } - remove(key: string) { + async remove(key: string) { this._storage.removeItem(key) } - get(key: string) { + async get(key: string) { return this._storage.getItem(key) } - set(key: string, value: string) { + async set(key: string, value: string) { this._storage.setItem(key, value) } } diff --git a/src/StorageType.ts b/src/StorageType.ts index caaf435..b7711e2 100644 --- a/src/StorageType.ts +++ b/src/StorageType.ts @@ -1,5 +1,5 @@ export type StorageType = { - remove(key: string): void - set(key: string, value: string): void - get(value: string): string | null + remove(key: string): Promise + set(key: string, value: string): Promise + get(value: string): Promise } diff --git a/src/authTokenInterceptor.ts b/src/authTokenInterceptor.ts index d271eec..f29f809 100644 --- a/src/authTokenInterceptor.ts +++ b/src/authTokenInterceptor.ts @@ -15,37 +15,7 @@ import ms from 'ms' // A little time before expiration to try refresh (seconds) let expireFudge = 10 -type RequestsQueue = { - resolve: (value?: unknown) => void - reject: (reason?: unknown) => void -}[] - -let isRefreshing = false -let queue: RequestsQueue = [] - -/** - * Function that resolves all items in the queue with the provided token - * @param token New access token - */ -const resolveQueue = (token?: Token) => { - queue.forEach((p) => { - p.resolve(token) - }) - - queue = [] -} - -/** - * Function that declines all items in the queue with the provided error - * @param error Error - */ -const declineQueue = (error: Error) => { - queue.forEach((p) => { - p.reject(error) - }) - - queue = [] -} +let currentlyRequestingPromise: Promise | undefined = undefined /** * Gets the unix timestamp from an access token @@ -87,17 +57,16 @@ const isTokenExpired = (token: Token): boolean => { /** * Refreshes the access token using the provided function + * Note: NOT to be called externally. Only accessible through an interceptor * * @param {requestRefresh} requestRefresh - Function that is used to get a new access token * @returns {string} - Fresh access token */ const refreshToken = async (requestRefresh: TokenRefreshRequest): Promise => { - const refreshToken = getRefreshToken() + const refreshToken = await getRefreshToken() if (!refreshToken) throw new Error('No refresh token available') try { - isRefreshing = true - // Refresh and store access token using the supplied refresh function const newTokens = await requestRefresh(refreshToken) if (typeof newTokens === 'object' && newTokens?.accessToken) { @@ -115,7 +84,7 @@ const refreshToken = async (requestRefresh: TokenRefreshRequest): Promise const status = error.response?.status if (status === 401 || status === 422) { // The refresh token is invalid so remove the stored tokens - StorageProxy.Storage?.remove(STORAGE_KEY) + await StorageProxy.Storage?.remove(STORAGE_KEY) throw new Error(`Got ${status} on token refresh; clearing both auth tokens`) } } @@ -126,8 +95,6 @@ const refreshToken = async (requestRefresh: TokenRefreshRequest): Promise } else { throw new Error('Failed to refresh auth token and failed to parse error') } - } finally { - isRefreshing = false } } @@ -146,12 +113,11 @@ export const refreshTokenIfNeeded = async ( requestRefresh: TokenRefreshRequest ): Promise => { // use access token (if we have it) - let accessToken = getAccessToken() + let accessToken = await getAccessToken() // check if access token is expired if (!accessToken || isTokenExpired(accessToken)) { // do refresh - accessToken = await refreshToken(requestRefresh) } @@ -182,33 +148,30 @@ export const authTokenInterceptor = ({ return async (requestConfig: any): Promise => { // Waiting for a fix in axios types // We need refresh token to do any authenticated requests - if (!getRefreshToken()) return requestConfig - - // Queue the request if another refresh request is currently happening - if (isRefreshing) { - return new Promise((resolve, reject) => { - queue.push({ resolve, reject }) - }) - .then((token) => { - if (requestConfig.headers) { - requestConfig.headers[header] = `${headerPrefix}${token}` - } - return requestConfig - }) - .catch(Promise.reject) - } - - // Do refresh if needed - let accessToken - try { - accessToken = await refreshTokenIfNeeded(requestRefresh) - resolveQueue(accessToken) - } catch (error: unknown) { - if (error instanceof Error) { - declineQueue(error) - throw new Error( - `Unable to refresh access token for request due to token refresh error: ${error.message}` - ) + if (!(await getRefreshToken())) return requestConfig + + let accessToken = undefined + + // Try to await a current request + if (currentlyRequestingPromise) accessToken = await currentlyRequestingPromise + + if (!accessToken) { + try { + // Sets the promise so everyone else will wait - then get the value + currentlyRequestingPromise = refreshTokenIfNeeded(requestRefresh) + accessToken = await currentlyRequestingPromise + + // Reset the promise + currentlyRequestingPromise = undefined + } catch (error: unknown) { + // Reset the promise + currentlyRequestingPromise = undefined + + if (error instanceof Error) { + throw new Error( + `Unable to refresh access token for request due to token refresh error: ${error.message}` + ) + } } } diff --git a/src/setAuthTokens.ts b/src/setAuthTokens.ts index 09e96e3..b10cf14 100644 --- a/src/setAuthTokens.ts +++ b/src/setAuthTokens.ts @@ -6,5 +6,5 @@ import { IAuthTokens } from './IAuthTokens' * Sets the access and refresh tokens * @param {IAuthTokens} tokens - Access and Refresh tokens */ -export const setAuthTokens = (tokens: IAuthTokens): void => - StorageProxy.Storage?.set(STORAGE_KEY, JSON.stringify(tokens)) +export const setAuthTokens = async (tokens: IAuthTokens): Promise => + await StorageProxy.Storage?.set(STORAGE_KEY, JSON.stringify(tokens)) diff --git a/src/tokensUtils.ts b/src/tokensUtils.ts index a629bec..be069a8 100644 --- a/src/tokensUtils.ts +++ b/src/tokensUtils.ts @@ -10,8 +10,8 @@ import { IAuthTokens } from './IAuthTokens' * Returns the refresh and access tokens * @returns {IAuthTokens} Object containing refresh and access tokens */ -const getAuthTokens = (): IAuthTokens | undefined => { - const rawTokens = StorageProxy.Storage?.get(STORAGE_KEY) +const getAuthTokens = async (): Promise => { + const rawTokens = await StorageProxy.Storage?.get(STORAGE_KEY) if (!rawTokens) return try { @@ -29,22 +29,22 @@ const getAuthTokens = (): IAuthTokens | undefined => { * Sets the access token * @param {string} token - Access token */ -export const setAccessToken = (token: Token): void => { - const tokens = getAuthTokens() +export const setAccessToken = async (token: Token): Promise => { + const tokens = await getAuthTokens() if (!tokens) { throw new Error('Unable to update access token since there are not tokens currently stored') } tokens.accessToken = token - setAuthTokens(tokens) + await setAuthTokens(tokens) } /** * Returns the stored refresh token * @returns {string} Refresh token */ -export const getRefreshToken = (): Token | undefined => { - const tokens = getAuthTokens() +export const getRefreshToken = async (): Promise => { + const tokens = await getAuthTokens() return tokens ? tokens.refreshToken : undefined } @@ -52,21 +52,22 @@ export const getRefreshToken = (): Token | undefined => { * Returns the stored access token * @returns {string} Access token */ -export const getAccessToken = (): Token | undefined => { - const tokens = getAuthTokens() +export const getAccessToken = async (): Promise => { + const tokens = await getAuthTokens() return tokens ? tokens.accessToken : undefined } /** * Clears both tokens */ -export const clearAuthTokens = (): void => StorageProxy.Storage?.remove(STORAGE_KEY) +export const clearAuthTokens = async (): Promise => + await StorageProxy.Storage?.remove(STORAGE_KEY) /** * Checks if refresh tokens are stored * @returns Whether the user is logged in or not */ -export const isLoggedIn = (): boolean => { - const token = getRefreshToken() +export const isLoggedIn = async (): Promise => { + const token = await getRefreshToken() return !!token } diff --git a/tests/authTokenInterceptor.test.ts b/tests/authTokenInterceptor.test.ts index 0d96787..0b2d2cb 100644 --- a/tests/authTokenInterceptor.test.ts +++ b/tests/authTokenInterceptor.test.ts @@ -1,6 +1,6 @@ -import { AxiosRequestConfig } from 'axios'; -import jwt from 'jsonwebtoken'; -import { authTokenInterceptor } from '../index'; +import { AxiosRequestConfig } from 'axios' +import jwt from 'jsonwebtoken' +import { authTokenInterceptor } from '../index' describe('authTokenInterceptor', () => { it('returns the original request config if refresh token is not set', async () => { diff --git a/tests/clearAuthTokens.test.ts b/tests/clearAuthTokens.test.ts index a4b2a04..47ab015 100644 --- a/tests/clearAuthTokens.test.ts +++ b/tests/clearAuthTokens.test.ts @@ -1,9 +1,8 @@ -import { STORAGE_KEY } from '../src/StorageKey'; -import { clearAuthTokens } from '../index'; - +import { STORAGE_KEY } from '../src/StorageKey' +import { clearAuthTokens } from '../index' describe('clearAuthTokens', () => { - it('removes the tokens from localstorage', () => { + it('removes the tokens from localstorage', async () => { // GIVEN // Tokens are stored in localStorage const tokens = { accessToken: 'accesstoken', refreshToken: 'refreshtoken' } @@ -11,7 +10,7 @@ describe('clearAuthTokens', () => { // WHEN // I call clearAuthTokens - clearAuthTokens() + await clearAuthTokens() // THEN // I expect the localstorage to be empty diff --git a/tests/getAccessToken.test.ts b/tests/getAccessToken.test.ts index e40e5ed..a9fc22c 100644 --- a/tests/getAccessToken.test.ts +++ b/tests/getAccessToken.test.ts @@ -1,5 +1,5 @@ -import { getAccessToken, authTokenInterceptor, getBrowserSessionStorage } from '../index'; -import { STORAGE_KEY } from '../src/StorageKey'; +import { getAccessToken, authTokenInterceptor, getBrowserSessionStorage } from '../index' +import { STORAGE_KEY } from '../src/StorageKey' describe('getAccessToken', () => { beforeEach(function () { @@ -8,21 +8,21 @@ describe('getAccessToken', () => { }) describe('for localStorage', function () { - it('returns undefined if tokens are not set', () => { + it('returns undefined if tokens are not set', async () => { // GIVEN // localStorage is empty localStorage.removeItem(STORAGE_KEY) // WHEN // I call getAccessToken - const result = getAccessToken() + const result = await getAccessToken() // THEN // I expect the result to be undefined expect(result).toEqual(undefined) }) - it('returns the access token is it is set', () => { + it('returns the access token is it is set', async () => { // GIVEN // Both tokens are stored in localstorage const tokens = { accessToken: 'accesstoken', refreshToken: 'refreshtoken' } @@ -30,37 +30,37 @@ describe('getAccessToken', () => { // WHEN // I call getAccessToken - const result = getAccessToken() + const result = await getAccessToken() // THEN // I expect the result to be the supplied access token expect(result).toEqual('accesstoken') }) - }); + }) describe('for sessionStorage', function () { - beforeEach( () => { + beforeEach(() => { const getStorage = getBrowserSessionStorage const requestRefresh = jest.fn() - authTokenInterceptor({getStorage, requestRefresh }) + authTokenInterceptor({ getStorage, requestRefresh }) }) - it('returns undefined if tokens are not set', () => { + it('returns undefined if tokens are not set', async () => { // GIVEN // localStorage is empty sessionStorage.removeItem(STORAGE_KEY) // WHEN // I call getAccessToken - const result = getAccessToken() + const result = await getAccessToken() // THEN // I expect the result to be undefined expect(result).toEqual(undefined) }) - it('returns the access token is it is set', () => { + it('returns the access token is it is set', async () => { // GIVEN // Both tokens are stored in localstorage const tokens = { accessToken: 'accesstoken_session', refreshToken: 'refreshtoken_session' } @@ -68,7 +68,7 @@ describe('getAccessToken', () => { // WHEN // I call getAccessToken - const result = getAccessToken() + const result = await getAccessToken() // THEN // I expect the result to be the supplied access token diff --git a/tests/getRefreshToken.test.ts b/tests/getRefreshToken.test.ts index d27c843..32c3ae1 100644 --- a/tests/getRefreshToken.test.ts +++ b/tests/getRefreshToken.test.ts @@ -1,22 +1,22 @@ -import { getRefreshToken } from '../index'; -import { STORAGE_KEY } from '../src/StorageKey'; +import { getRefreshToken } from '../index' +import { STORAGE_KEY } from '../src/StorageKey' describe('getRefreshToken', () => { - it('returns undefined if tokens are not set', () => { + it('returns undefined if tokens are not set', async () => { // GIVEN // localStorage is empty localStorage.removeItem(STORAGE_KEY) // WHEN // I call getRefreshToken - const result = getRefreshToken() + const result = await getRefreshToken() // THEN // I expect the result to be undefined expect(result).toEqual(undefined) }) - it('returns the access token is it is set', () => { + it('returns the access token is it is set', async () => { // GIVEN // Both tokens are stored in localstorage const tokens = { accessToken: 'accesstoken', refreshToken: 'refreshtoken' } @@ -24,7 +24,7 @@ describe('getRefreshToken', () => { // WHEN // I call getRefreshToken - const result = getRefreshToken() + const result = await getRefreshToken() // THEN // I expect the result to be the supplied refresh token diff --git a/tests/isLoggedIn.test.ts b/tests/isLoggedIn.test.ts index ff07f51..97a646f 100644 --- a/tests/isLoggedIn.test.ts +++ b/tests/isLoggedIn.test.ts @@ -1,22 +1,22 @@ -import { STORAGE_KEY } from '../src/StorageKey'; -import { isLoggedIn } from '../index'; +import { STORAGE_KEY } from '../src/StorageKey' +import { isLoggedIn } from '../index' describe('isLoggedIn', () => { - it('returns false if tokens are not set', () => { + it('returns false if tokens are not set', async () => { // GIVEN // localStorage is empty localStorage.removeItem(STORAGE_KEY) // WHEN // I call isLoggedIn - const result = isLoggedIn() + const result = await isLoggedIn() // THEN // I expect the result to be false expect(result).toEqual(false) }) - it('returns true if refresh token is set', () => { + it('returns true if refresh token is set', async () => { // GIVEN // Both tokens are stored in localstorage const tokens = { accessToken: 'accesstoken', refreshToken: 'refreshtoken' } @@ -24,7 +24,7 @@ describe('isLoggedIn', () => { // WHEN // I call isLoggedIn - const result = isLoggedIn() + const result = await isLoggedIn() // THEN // I expect the result to be true diff --git a/tests/setAccessToken.test.ts b/tests/setAccessToken.test.ts index e74ddea..e4f8f11 100644 --- a/tests/setAccessToken.test.ts +++ b/tests/setAccessToken.test.ts @@ -1,5 +1,5 @@ -import { setAccessToken } from '../index'; -import { STORAGE_KEY } from '../src/StorageKey'; +import { setAccessToken } from '../index' +import { STORAGE_KEY } from '../src/StorageKey' describe('setAccessToken', () => { it('throws an error if there are no tokens stored', () => { @@ -11,9 +11,9 @@ describe('setAccessToken', () => { // I call setAccessToken // THEN // I expect an error to have been thrown - expect(() => { - setAccessToken('accesstoken') - }).toThrow('Unable to update access token since there are not tokens currently stored') + expect(async () => { + await setAccessToken('accesstoken') + }).rejects.toThrow('Unable to update access token since there are not tokens currently stored') }) it('throws an error if the stored tokens cannot be parsed', () => { @@ -25,12 +25,12 @@ describe('setAccessToken', () => { // I call setAuthTokens // THEN // I expect an error to be thrown - expect(() => { - setAccessToken('accesstoken') - }).toThrow('Failed to parse auth tokens: totallynotjson') + expect(async () => { + await setAccessToken('accesstoken') + }).rejects.toThrow('Failed to parse auth tokens: totallynotjson') }) - it('stores the tokens in localstorage', () => { + it('stores the tokens in localstorage', async () => { // GIVEN // localStorage is empty const tokens = { accessToken: 'accesstoken', refreshToken: 'refreshtoken' } @@ -38,11 +38,14 @@ describe('setAccessToken', () => { // WHEN // I call setAccessToken - setAccessToken('newaccesstoken') + await setAccessToken('newaccesstoken') // THEN // I expect the stored access token to have been updated const storedTokens = localStorage.getItem(STORAGE_KEY) as string - expect(JSON.parse(storedTokens)).toEqual({ accessToken: 'newaccesstoken', refreshToken: 'refreshtoken' }) + expect(JSON.parse(storedTokens)).toEqual({ + accessToken: 'newaccesstoken', + refreshToken: 'refreshtoken', + }) }) }) diff --git a/tests/setAuthTokens.test.ts b/tests/setAuthTokens.test.ts index 610737c..7a45faf 100644 --- a/tests/setAuthTokens.test.ts +++ b/tests/setAuthTokens.test.ts @@ -1,8 +1,8 @@ -import { setAuthTokens } from '../index'; -import { STORAGE_KEY } from '../src/StorageKey'; +import { setAuthTokens } from '../index' +import { STORAGE_KEY } from '../src/StorageKey' describe('setAuthTokens', () => { - it('stores the tokens in localstorage', () => { + it('stores the tokens in localstorage', async () => { // GIVEN // localStorage is empty localStorage.removeItem(STORAGE_KEY) @@ -10,7 +10,7 @@ describe('setAuthTokens', () => { // WHEN // I call setAuthTokens const tokens = { accessToken: 'accesstoken', refreshToken: 'refreshtoken' } - setAuthTokens(tokens) + await setAuthTokens(tokens) // THEN // I expect them to have been stored in localstorage