Skip to content

Commit

Permalink
Fix 401 token request in WordPressTemplate (#1724)
Browse files Browse the repository at this point in the history
* Add `skip` config option to `useAuth` to skip invocation of hook

* Check for existing tokens in `ensureAuthorization` before requesting new ones

* Add changesets

* Reset the access token after each test

* Update packages/faustwp-core/tests/hooks/useAuth.test.ts

Co-authored-by: Joe Fusco <[email protected]>

---------

Co-authored-by: Joe Fusco <[email protected]>
  • Loading branch information
blakewilson and josephfusco authored Jan 12, 2024
1 parent 5bad238 commit 085c30d
Show file tree
Hide file tree
Showing 7 changed files with 212 additions and 21 deletions.
5 changes: 5 additions & 0 deletions .changeset/shaggy-cooks-dream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@faustwp/core': minor
---

Added a new `skip` config option to `useAuth` to conditionally invoke the hook
5 changes: 5 additions & 0 deletions .changeset/tame-ears-flash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@faustwp/core': patch
---

Fixed a bug that made a request to the token endpoint on every page in `@faustwp/[email protected]`
138 changes: 125 additions & 13 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion packages/faustwp-core/src/auth/authorize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import 'isomorphic-fetch';
import isString from 'lodash/isString.js';
import { getWpUrl } from '../lib/getWpUrl.js';
import { getQueryParam, removeURLParam } from '../utils/index.js';
import { fetchAccessToken } from './client/accessToken.js';
import { fetchAccessToken, getAccessToken } from './client/accessToken.js';

export interface EnsureAuthorizationOptions {
redirectUri?: string;
Expand All @@ -23,6 +23,14 @@ export async function ensureAuthorization(
): Promise<
true | { redirect?: string | undefined; login?: string | undefined }
> {
/**
* If there is already an access token saved in memory, we can assume they
* are already authorized.
*/
if (getAccessToken()) {
return true;
}

const wpUrl = getWpUrl();
const { redirectUri, loginPageUri } = options || {};

Expand Down
9 changes: 7 additions & 2 deletions packages/faustwp-core/src/components/WordPressTemplate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function WordPressTemplateInternal(
props: WordPressTemplateProps & {
seedNode: SeedNode;
isPreview: boolean;
isAuthenticated: boolean;
isAuthenticated: boolean | null;
loading: boolean;
setLoading: (loading: boolean) => void;
},
Expand Down Expand Up @@ -193,6 +193,7 @@ export function WordPressTemplate(props: WordPressTemplateProps) {
const { isAuthenticated, loginUrl } = useAuth({
strategy: 'redirect',
shouldRedirect: false,
skip: !isPreview,
});

/**
Expand Down Expand Up @@ -287,7 +288,11 @@ export function WordPressTemplate(props: WordPressTemplateProps) {
})();
}, [seedNode, isPreview, isAuthenticated, basePath]);

if (seedNode === null || isPreview === null || isAuthenticated === null) {
if (
seedNode === null ||
isPreview === null ||
(isPreview && isAuthenticated === null)
) {
return null;
}

Expand Down
24 changes: 19 additions & 5 deletions packages/faustwp-core/src/hooks/useAuth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ import {
type RedirectStrategyConfig = {
strategy: 'redirect';
shouldRedirect?: boolean;
skip?: boolean;
};

type LocalStrategyConfig = {
strategy: 'local';
loginPageUrl: string;
shouldRedirect?: boolean;
skip?: boolean;
};

export type UseAuthConfig = RedirectStrategyConfig | LocalStrategyConfig;
Expand All @@ -23,6 +25,7 @@ export function useAuth(_config?: UseAuthConfig) {
const config = defaults(_config, {
strategy: 'redirect',
shouldRedirect: false,
skip: false,
}) as UseAuthConfig;

if (config.strategy === 'local' && !config.loginPageUrl) {
Expand All @@ -34,8 +37,19 @@ export function useAuth(_config?: UseAuthConfig) {
const [isAuthenticated, setIsAuthenticated] = useState<boolean | null>(null);
const [isReady, setIsReady] = useState<boolean>(false);
const [loginUrl, setLoginUrl] = useState<string | null>(null);
const [called, setCalled] = useState<boolean>(false);

useEffect(() => {
if (config.skip === true) {
return;
}

if (called) {
return;
}

setCalled(true);

const ensureAuthorizationConfig: EnsureAuthorizationOptions = {
redirectUri: window.location.href,
};
Expand Down Expand Up @@ -71,17 +85,17 @@ export function useAuth(_config?: UseAuthConfig) {

setIsReady(true);
})();

// NOTE: This effect should only be ran once on mount, so we are not
// providing the exhaustive deps to useEffect.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
}, [called, config]);

/**
* Automatically redirect the user to the login page if the
* shouldRedirect option is set to true.
*/
useEffect(() => {
if (config.skip === true) {
return;
}

if (
!config.shouldRedirect ||
!isReady ||
Expand Down
42 changes: 42 additions & 0 deletions packages/faustwp-core/tests/hooks/useAuth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import { renderHook } from '@testing-library/react-hooks';
import fetchMock from 'fetch-mock';
import { useAuth } from '../../src/hooks/useAuth';
import { setAccessToken } from '../../src/auth';

describe('useAuth hook', () => {
const envBackup = process.env;
Expand All @@ -17,6 +18,47 @@ describe('useAuth hook', () => {
process.env = envBackup;
});

afterEach(() => {
setAccessToken(undefined, undefined);
});

it('skips authentication check when skip is true', async () => {
// Set up the fetch mock but expect it not to be called
fetchMock.get(`/api/faust/auth/token`, {
status: 200,
body: JSON.stringify({ accessToken: 'at', refreshToken: 'rt' }),
});

const { result } = renderHook(() => useAuth({ skip: true }));

// The hook should skip the authentication check
expect(result.current.isAuthenticated).toStrictEqual(null);
expect(result.current.isReady).toStrictEqual(false);

// Ensure the token endpoint was not called
expect(fetchMock.called(`/api/faust/auth/token`)).toBe(false);

fetchMock.restore();
});

it('performs authentication check by default (skip not provided)', async () => {
// Mock a valid response
fetchMock.get(`/api/faust/auth/token`, {
status: 200,
body: JSON.stringify({ accessToken: 'at', refreshToken: 'rt' }),
});

const { result, waitForNextUpdate } = renderHook(() => useAuth());

await waitForNextUpdate();

// Default behavior should perform authentication check
expect(result.current.isAuthenticated).toStrictEqual(true);
expect(result.current.isReady).toStrictEqual(true);

fetchMock.restore();
});

it('Provides the proper login url with redirect strategy', async () => {
const { result, waitForNextUpdate } = renderHook(() => useAuth());
await waitForNextUpdate();
Expand Down

0 comments on commit 085c30d

Please sign in to comment.