From 5636d21e83ec959b667a86cceab0747843074506 Mon Sep 17 00:00:00 2001 From: Mark DeLaVernge Date: Fri, 24 Feb 2023 11:43:36 -0500 Subject: [PATCH] fix: Refresh auth token on 401 http errors --- src/hooks/useAuth.test.tsx | 60 ++++++++++++++++++++++++++++++++ src/hooks/useAuth.tsx | 34 ++++++++++++------ src/hooks/useHttpClient.test.tsx | 26 ++++++++++++++ src/hooks/useHttpClient.tsx | 37 ++++++++++++++++---- src/navigators/RootStack.tsx | 2 +- 5 files changed, 141 insertions(+), 18 deletions(-) diff --git a/src/hooks/useAuth.test.tsx b/src/hooks/useAuth.test.tsx index cfadbd33..38219fe4 100644 --- a/src/hooks/useAuth.test.tsx +++ b/src/hooks/useAuth.test.tsx @@ -56,6 +56,7 @@ beforeEach(() => { useCurrentAppStateMock.mockReturnValue({ currentAppState: 'active', }); + jest.useRealTimers(); }); test('without provider, methods fail', async () => { @@ -67,6 +68,9 @@ test('without provider, methods fail', async () => { result.current.storeAuthResult(exampleAuthResult), ).rejects.toBeUndefined(); await expect(result.current.clearAuthResult()).rejects.toBeUndefined(); + await expect( + result.current.refreshForAuthFailure(new Error()), + ).rejects.toBeUndefined(); }); test('initial state test', async () => { @@ -291,3 +295,59 @@ test('app state change refreshes auth token if needed', async () => { test('shouldAttemptTokenRefresh handles edge case of accessTokenExpirationDate not being set', () => { expect(shouldAttemptTokenRefresh(undefined)).toBe(false); }); + +test('refreshForAuthFailure refreshes auth token if not already loading', async () => { + // 1. Setup being initialized and authorized + const authResult = { + ...exampleAuthResult, + accessTokenExpirationDate: new Date( + Date.now() + 60 * 60 * 1000, + ).toISOString(), + }; + mockAuthResult(authResult); + const { result } = await renderHookInContext(); + await act(async () => { + await result.current.initialize(refreshHandler); + }); + expect(result.current.loading).toBe(false); + expect(result.current.authResult).toEqual(authResult); + expect(result.current.isLoggedIn).toBe(true); + expect(refreshHandler).not.toHaveBeenCalled(); + + // 2. Simulate reporting a 401 error + const refreshedAuthResult = { + ...exampleAuthResult, + accessToken: 'REFRESHED_accessToken', + idToken: 'REFRESHED_idToken', + refreshToken: 'REFRESHED_refreshToken', + accessTokenExpirationDate: new Date( + Date.now() + 60 * 60 * 1000, + ).toISOString(), // Expires in 1 hour + }; + refreshHandler.mockResolvedValue( + new Promise((resolve) => + process.nextTick(() => resolve(refreshedAuthResult)), + ), + ); + jest.useFakeTimers(); + act(() => { + result.current.refreshForAuthFailure(new Error()); + }); + expect(refreshHandler).toHaveBeenCalledTimes(1); + expect(result.current.loading).toBe(true); + + // 3. 401 reports have no effect while still loading + await act(async () => { + await result.current.refreshForAuthFailure(new Error()); + }); + expect(refreshHandler).toHaveBeenCalledTimes(1); + + // 4. Resolve refreshHandler inside act so hook can "react" + await act(async () => { + jest.runAllTimers(); + }); + + expect(result.current.loading).toBe(false); + expect(result.current.authResult).toEqual(refreshedAuthResult); + expect(result.current.isLoggedIn).toBe(true); +}); diff --git a/src/hooks/useAuth.tsx b/src/hooks/useAuth.tsx index 92e8e9af..54518df1 100644 --- a/src/hooks/useAuth.tsx +++ b/src/hooks/useAuth.tsx @@ -16,6 +16,7 @@ export interface AuthStatus { storeAuthResult: (authResult: AuthResult) => Promise; clearAuthResult: () => Promise; initialize: (refreshHandler: RefreshHandler) => Promise; + refreshForAuthFailure: (error: Error) => Promise; } export type AuthResult = Omit; @@ -27,6 +28,7 @@ const AuthContext = createContext({ storeAuthResult: (_) => Promise.reject(), clearAuthResult: () => Promise.reject(), initialize: (_) => Promise.reject(), + refreshForAuthFailure: (_) => Promise.reject(), }); const secureStorage = new SecureStore('auth-hook'); @@ -78,6 +80,9 @@ export const AuthContextProvider = ({ const refreshAuthResult = useCallback( async (_refreshHandler: RefreshHandler, _authResult: AuthResult) => { + if (__DEV__) { + console.warn('Attempting access token refresh'); + } try { setLoading(true); const refreshResult = await _refreshHandler(_authResult); @@ -118,16 +123,20 @@ export const AuthContextProvider = ({ [refreshAuthResult], ); - const refreshIfNeeded = useCallback(async () => { - if ( - !loading && - refreshHandler && - authResult && - shouldAttemptTokenRefresh(authResult.accessTokenExpirationDate) - ) { - refreshAuthResult(refreshHandler, authResult); - } - }, [loading, refreshHandler, authResult, refreshAuthResult]); + const refreshIfNeeded = useCallback( + async (skipExpirationCheck: boolean = false) => { + if ( + !loading && + refreshHandler && + authResult && + (skipExpirationCheck || + shouldAttemptTokenRefresh(authResult.accessTokenExpirationDate)) + ) { + return refreshAuthResult(refreshHandler, authResult); + } + }, + [loading, refreshHandler, authResult, refreshAuthResult], + ); useEffect(() => { if (currentAppState === 'active') { @@ -135,6 +144,10 @@ export const AuthContextProvider = ({ } }, [currentAppState, refreshIfNeeded]); + const refreshForAuthFailure = useCallback(async () => { + return refreshIfNeeded(true); + }, [refreshIfNeeded]); + const context = { loading, isLoggedIn, @@ -142,6 +155,7 @@ export const AuthContextProvider = ({ storeAuthResult, clearAuthResult, initialize, + refreshForAuthFailure, }; return ( diff --git a/src/hooks/useHttpClient.test.tsx b/src/hooks/useHttpClient.test.tsx index 28725ef7..dfd971bc 100644 --- a/src/hooks/useHttpClient.test.tsx +++ b/src/hooks/useHttpClient.test.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import axios from 'axios'; import { act, renderHook } from '@testing-library/react-native'; import MockAdapter from 'axios-mock-adapter'; import { AuthResult, useAuth } from './useAuth'; @@ -37,6 +38,20 @@ test('initial state test', async () => { expect(result.current.httpClient).toBeDefined(); }); +test('provides default baseURL if one is not provided', async () => { + const axiosInstance = axios.create(); + const { result } = renderHook(() => useHttpClient(), { + wrapper: ({ children }) => ( + + {children} + + ), + }); + expect(result.current.httpClient.defaults.baseURL).toEqual( + 'https://api.us.lifeomic.com', + ); +}); + test('if authResult is not present, has no Authorization header', async () => { const { result } = await renderHookInContext(); const axiosMock = new MockAdapter(result.current.httpClient); @@ -64,3 +79,14 @@ test('once authResult is set, adds bearer token', async () => { expect(getHeaders?.Authorization).toBe(`Bearer ${authResult.accessToken}`); expect(getHeaders?.['Content-Type']).toBe('application/json'); }); + +test('reports 401 errors to useAuth and throws', async () => { + const refreshForAuthFailure = jest.fn().mockResolvedValue({}); + useAuthMock.mockReturnValue({ authResult, refreshForAuthFailure }); + const { result } = await renderHookInContext(); + + const axiosMock = new MockAdapter(result.current.httpClient); + axiosMock.onGet('/v1/accounts').reply(401); + await expect(result.current.httpClient.get('/v1/accounts')).rejects.toThrow(); + expect(refreshForAuthFailure).toHaveBeenCalled(); +}); diff --git a/src/hooks/useHttpClient.tsx b/src/hooks/useHttpClient.tsx index dda7b198..6da5efab 100644 --- a/src/hooks/useHttpClient.tsx +++ b/src/hooks/useHttpClient.tsx @@ -20,12 +20,13 @@ const HttpClientContext = createContext({ httpClient: defaultAxiosInstance, }); -let interceptorId: number; +let requestInterceptorId: number; +let responseInterceptorId: number; /** * The HttpClientContextProvider's job is to provide an HTTP client that - * takes care of things like managing the HTTP Authorization header and - * other default behavior. + * takes care of things like managing the HTTP Authorization header, error + * response handling, and other default behavior. */ export const HttpClientContextProvider = ({ injectedAxiosInstance, @@ -36,7 +37,7 @@ export const HttpClientContextProvider = ({ baseURL?: string; children?: React.ReactNode; }) => { - const { authResult } = useAuth(); + const { authResult, refreshForAuthFailure } = useAuth(); const axiosInstance = injectedAxiosInstance || defaultAxiosInstance; @@ -45,16 +46,38 @@ export const HttpClientContextProvider = ({ } const httpClient = useMemo(() => { - axiosInstance.interceptors.request.eject(interceptorId); + axiosInstance.interceptors.request.eject(requestInterceptorId); + axiosInstance.interceptors.response.eject(responseInterceptorId); if (!authResult?.accessToken) { return axiosInstance; } - interceptorId = axiosInstance.interceptors.request.use((config) => { + + // Add current access token as auth header + requestInterceptorId = axiosInstance.interceptors.request.use((config) => { config.headers.Authorization = `Bearer ${authResult.accessToken}`; return config; }); + + // Detect 401s and ask for refresh + responseInterceptorId = axiosInstance.interceptors.response.use( + undefined, + async function (error: Error) { + if (axios.isAxiosError(error)) { + if (__DEV__) { + console.warn('Request Failed: ', error.toJSON()); + } + + if (error.response?.status === 401) { + await refreshForAuthFailure(error); + } + } + + return Promise.reject(error); + }, + ); + return axiosInstance; - }, [authResult?.accessToken, axiosInstance]); + }, [authResult?.accessToken, axiosInstance, refreshForAuthFailure]); const context: HttpClient = { httpClient }; diff --git a/src/navigators/RootStack.tsx b/src/navigators/RootStack.tsx index 1a6e513f..aabfd18c 100644 --- a/src/navigators/RootStack.tsx +++ b/src/navigators/RootStack.tsx @@ -19,7 +19,7 @@ export type NotLoggedInRootParamList = { export function RootStack() { const { isLoggedIn, loading } = useAuth(); - if (loading) { + if (!isLoggedIn && loading) { return (