diff --git a/plugins/auth-react/api-report.md b/plugins/auth-react/api-report.md index 660154271c..01ec9b028a 100644 --- a/plugins/auth-react/api-report.md +++ b/plugins/auth-react/api-report.md @@ -21,12 +21,19 @@ export type CookieAuthRefreshProviderProps = { export function useCookieAuthRefresh(options: { pluginId: string; path?: string; -}): { - status: 'loading' | 'error' | 'success'; - error?: Error; - result?: { - expiresAt: string; - }; - retry: () => void; -}; +}): + | { + status: 'loading'; + } + | { + status: 'error'; + error: Error; + retry: () => void; + } + | { + status: 'success'; + data: { + expiresAt: string; + }; + }; ``` diff --git a/plugins/auth-react/src/components/CookieAuthRefreshProvider/CookieAuthRefreshProvider.tsx b/plugins/auth-react/src/components/CookieAuthRefreshProvider/CookieAuthRefreshProvider.tsx index 8f6f38e225..85846b28c9 100644 --- a/plugins/auth-react/src/components/CookieAuthRefreshProvider/CookieAuthRefreshProvider.tsx +++ b/plugins/auth-react/src/components/CookieAuthRefreshProvider/CookieAuthRefreshProvider.tsx @@ -44,16 +44,16 @@ export function CookieAuthRefreshProvider( const app = useApp(); const { Progress } = app.getComponents(); - const { status, error, retry } = useCookieAuthRefresh(options); + const result = useCookieAuthRefresh(options); - if (status === 'loading') { + if (result.status === 'loading') { return ; } - if (status === 'error' && error) { + if (result.status === 'error') { return ( - - diff --git a/plugins/auth-react/src/hooks/useCookieAuthRefresh/useCookieAuthRefresh.test.tsx b/plugins/auth-react/src/hooks/useCookieAuthRefresh/useCookieAuthRefresh.test.tsx index 2d5d738348..5c754b584c 100644 --- a/plugins/auth-react/src/hooks/useCookieAuthRefresh/useCookieAuthRefresh.test.tsx +++ b/plugins/auth-react/src/hooks/useCookieAuthRefresh/useCookieAuthRefresh.test.tsx @@ -80,8 +80,6 @@ describe('useCookieAuthRefresh', () => { ); expect(result.current.status).toBe('loading'); - expect(result.current.error).toBeUndefined(); - expect(result.current.result).toBeUndefined(); }); it('should return a loading status when retrying without previous success', async () => { @@ -112,19 +110,25 @@ describe('useCookieAuthRefresh', () => { }, ); - expect(result.current.status).toBe('loading'); - expect(result.current.error).toBeUndefined(); - expect(result.current.result).toBeUndefined(); + expect(result.current).toStrictEqual({ status: 'loading' }); - await waitFor(() => expect(result.current.status).toBe('error')); - expect(result.current.result).toBeUndefined(); - expect(result.current.error).toStrictEqual(error); + await waitFor(() => + expect(result.current).toStrictEqual({ + status: 'error', + error, + retry: expect.any(Function), + }), + ); - result.current.retry(); + if (result.current.status === 'error') { + result.current.retry(); + } - await waitFor(() => expect(result.current.status).toBe('loading')); - expect(result.current.result).toBeUndefined(); - expect(result.current.error).toStrictEqual(error); + await waitFor(() => + expect(result.current).toStrictEqual({ + status: 'loading', + }), + ); }); it('should return a loading status when retrying with previous success', async () => { @@ -159,25 +163,34 @@ describe('useCookieAuthRefresh', () => { }, ); - expect(result.current.status).toBe('loading'); - expect(result.current.error).toBeUndefined(); - expect(result.current.result).toBeUndefined(); + expect(result.current).toStrictEqual({ status: 'loading' }); - await waitFor(() => expect(result.current.status).toBe('success')); - expect(result.current.result).toMatchObject({ expiresAt }); - expect(result.current.error).toBeUndefined(); + await waitFor(() => + expect(result.current).toStrictEqual({ + status: 'success', + data: { expiresAt }, + }), + ); - result.current.retry(); + jest.advanceTimersByTime(tenMinutesInMilliseconds); - await waitFor(() => expect(result.current.status).toBe('error')); - expect(result.current.result).toMatchObject({ expiresAt }); - expect(result.current.error).toStrictEqual(error); + await waitFor(() => + expect(result.current).toStrictEqual({ + status: 'error', + error, + retry: expect.any(Function), + }), + ); - result.current.retry(); + if (result.current.status === 'error') { + result.current.retry(); + } - await waitFor(() => expect(result.current.status).toBe('loading')); - expect(result.current.result).toMatchObject({ expiresAt }); - expect(result.current.error).toStrictEqual(error); + await waitFor(() => + expect(result.current).toStrictEqual({ + status: 'loading', + }), + ); }); it('should return an error status when the refresh has failed', async () => { @@ -205,7 +218,13 @@ describe('useCookieAuthRefresh', () => { }, ); - await waitFor(() => expect(result.current.error).toStrictEqual(error)); + await waitFor(() => + expect(result.current).toStrictEqual({ + status: 'error', + error, + retry: expect.any(Function), + }), + ); }); it('should call the api to get the cookie and use it', async () => { @@ -233,7 +252,12 @@ describe('useCookieAuthRefresh', () => { ), ); - expect(result.current.result).toMatchObject({ expiresAt }); + await waitFor(() => + expect(result.current).toStrictEqual({ + status: 'success', + data: { expiresAt }, + }), + ); }); it('should cancel the refresh when a message is received from another tab', async () => { @@ -295,9 +319,14 @@ describe('useCookieAuthRefresh', () => { }, ); - await waitFor(() => expect(fetchApiMock.fetch).toHaveBeenCalledTimes(1)); + await waitFor(() => + expect(result.current).toStrictEqual({ + status: 'success', + data: { expiresAt }, + }), + ); - expect(result.current.result).toMatchObject({ expiresAt }); + await waitFor(() => expect(fetchApiMock.fetch).toHaveBeenCalledTimes(1)); unmount(); diff --git a/plugins/auth-react/src/hooks/useCookieAuthRefresh/useCookieAuthRefresh.tsx b/plugins/auth-react/src/hooks/useCookieAuthRefresh/useCookieAuthRefresh.tsx index c7b4984928..79e80c3954 100644 --- a/plugins/auth-react/src/hooks/useCookieAuthRefresh/useCookieAuthRefresh.tsx +++ b/plugins/auth-react/src/hooks/useCookieAuthRefresh/useCookieAuthRefresh.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import { useEffect, useCallback, useMemo } from 'react'; +import { useEffect, useCallback } from 'react'; import { discoveryApiRef, fetchApiRef, @@ -34,14 +34,10 @@ export function useCookieAuthRefresh(options: { pluginId: string; // The path to used for calling the refresh cookie endpoint, default to '/cookie' path?: string; -}): { - status: 'loading' | 'error' | 'success'; - error?: Error; - result?: { - expiresAt: string; - }; - retry: () => void; -} { +}): + | { status: 'loading' } + | { status: 'error'; error: Error; retry: () => void } + | { status: 'success'; data: { expiresAt: string } } { const { pluginId, path = '/cookie' } = options ?? {}; const fetchApi = useApi(fetchApiRef); const storageApi = useApi(storageApiRef); @@ -58,14 +54,19 @@ export function useCookieAuthRefresh(options: { if (!response.ok) { throw await ResponseError.fromResponse(response); } - return await response.json(); + const data = await response.json(); + if (!data.expiresAt) { + throw new Error('No expiration date found in response'); + } + return data; }); useMountEffect(actions.execute); const refresh = useCallback( (params: { expiresAt: string }) => { - // Randomize the refreshing margin to avoid all tabs refreshing at the same time + // Randomize the refreshing margin with a margin of 1-4 minutes to avoid all tabs refreshing at the same time + // It cannot be less than 5 minutes otherwise the backend will return the same expiration date const margin = (1 + 3 * Math.random()) * 60000; const delay = Date.parse(params.expiresAt) - Date.now() - margin; const timeout = setTimeout(actions.execute, delay); @@ -97,43 +98,35 @@ export function useCookieAuthRefresh(options: { }; }, [state, refresh, store]); - const status = useMemo(() => { - // Initialising - if (state.status === 'not-executed') { - return 'loading'; - } - - // First refresh or retrying without any success before - // Possible states transitions: - // e.g. not-executed -> loading (first-refresh) - // e.g. not-executed -> loading (first-refresh) -> error -> loading (manual-retry) - if (state.status === 'loading' && !state.result) { - return 'loading'; - } - - // Retrying after having succeeding at least once - // Current states is: { status: 'loading', result: {...}, error: undefined | Error } - // e.g. not-executed -> loading (first-refresh) -> success -> loading (scheduled-refresh) -> error -> loading (manual-retry) - if (state.status === 'loading' && state.error) { - return 'loading'; - } - - // Something went wrong during the any situation of a refresh - if (state.status === 'error' && state.error) { - return 'error'; - } - - return 'success'; - }, [state]); - const retry = useCallback(() => { actions.execute(); }, [actions]); - return { - retry, - status, - result: state.result, - error: state.error, - }; + // Initialising + if (state.status === 'not-executed') { + return { status: 'loading' }; + } + + // First refresh or retrying without any success before + // Possible state transitions: + // e.g. not-executed -> loading (first-refresh) + // e.g. not-executed -> loading (first-refresh) -> error -> loading (manual-retry) + if (state.status === 'loading' && !state.result) { + return { status: 'loading' }; + } + + // Retrying after having succeeding at least once + // Current state is: { status: 'loading', result: {...}, error: undefined | Error } + // e.g. not-executed -> loading (first-refresh) -> success -> loading (scheduled-refresh) -> error -> loading (manual-retry) + if (state.status === 'loading' && state.error) { + return { status: 'loading' }; + } + + // Something went wrong during any situation of a refresh + if (state.status === 'error' && state.error) { + return { status: 'error', error: state.error, retry }; + } + + // At this point it should be safe to assume that we have a successful refresh + return { status: 'success', data: state.result! }; }