From 31be3a0b3e48a83ff385ec6d624921c4bf2ce338 Mon Sep 17 00:00:00 2001 From: Daniel Choudhury Date: Thu, 7 Jan 2021 13:57:56 +0000 Subject: [PATCH 1/8] Configure ApolloProvider to always use fresh tokens | AuthContext.authToken to be deparacated --- packages/auth/src/AuthProvider.tsx | 24 +++++++++++---- packages/testing/src/MockProviders.tsx | 1 + .../src/components/RedwoodApolloProvider.tsx | 30 +++++++++++++++++-- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/packages/auth/src/AuthProvider.tsx b/packages/auth/src/AuthProvider.tsx index 2b6447a5af09..e7ce78d1c33e 100644 --- a/packages/auth/src/AuthProvider.tsx +++ b/packages/auth/src/AuthProvider.tsx @@ -16,7 +16,7 @@ export interface AuthContextInterface { /* Determining your current authentication state */ loading: boolean isAuthenticated: boolean - authToken: string | null + authToken: string | null // @WARN! to be depracated /* The current user's data from the `getCurrentUser` function on the api side */ currentUser: null | CurrentUser /* The user's metadata from the auth provider */ @@ -25,6 +25,8 @@ export interface AuthContextInterface { logOut(options?: unknown): Promise signUp(options?: unknown): Promise getToken(): Promise + // To be removed once authToken is removed + getFreshToken(): Promise /** * Fetches the "currentUser" from the api side, * but does not update the current user state. @@ -55,7 +57,7 @@ export interface AuthContextInterface { export const AuthContext = React.createContext({ loading: true, isAuthenticated: false, - authToken: null, + authToken: null, // @WARN! to be depracated userMetadata: null, currentUser: null, }) @@ -69,7 +71,7 @@ type AuthProviderProps = { type AuthProviderState = { loading: boolean isAuthenticated: boolean - authToken: string | null + authToken: string | null // @WARN! to be depracated userMetadata: null | Record currentUser: null | CurrentUser hasError: boolean @@ -96,7 +98,7 @@ export class AuthProvider extends React.Component< state: AuthProviderState = { loading: true, isAuthenticated: false, - authToken: null, + authToken: null, // @WARN! to be depracated userMetadata: null, currentUser: null, hasError: false, @@ -115,6 +117,8 @@ export class AuthProvider extends React.Component< } getCurrentUser = async (): Promise> => { + // Always get a fresh token, rather than use the one in state + const token = await this.getToken() const response = await window.fetch( `${window.__REDWOOD__API_PROXY_PATH}/graphql`, { @@ -122,7 +126,7 @@ export class AuthProvider extends React.Component< headers: { 'content-type': 'application/json', 'auth-provider': this.rwClient.type, - authorization: `Bearer ${this.state.authToken}`, + authorization: `Bearer ${token}`, }, body: JSON.stringify({ query: @@ -178,10 +182,17 @@ export class AuthProvider extends React.Component< return authToken } + // Note: Used by apollo middleware to get a fresh token + // State isn't changed, to prevent infinite loops + // Remove this, and call getToken when AuthProviderState.authToken is removed + getFreshToken = async () => { + return this.rwClient.getToken() + } + reauthenticate = async () => { const notAuthenticatedState: AuthProviderState = { isAuthenticated: false, - authToken: null, + authToken: null, // @WARN! to be depracated currentUser: null, userMetadata: null, loading: false, @@ -253,6 +264,7 @@ export class AuthProvider extends React.Component< getCurrentUser: this.getCurrentUser, hasRole: this.hasRole, reauthenticate: this.reauthenticate, + getFreshToken: this.getFreshToken, client, type, }} diff --git a/packages/testing/src/MockProviders.tsx b/packages/testing/src/MockProviders.tsx index 1a2783ff8b99..93fcf5ef29d4 100644 --- a/packages/testing/src/MockProviders.tsx +++ b/packages/testing/src/MockProviders.tsx @@ -24,6 +24,7 @@ const fakeUseAuth = (): AuthContextInterface => ({ logOut: async () => undefined, signUp: async () => undefined, getToken: async () => null, + getFreshToken: async () => null, getCurrentUser: async () => null, hasRole: () => false, reauthenticate: async () => undefined, diff --git a/packages/web/src/components/RedwoodApolloProvider.tsx b/packages/web/src/components/RedwoodApolloProvider.tsx index 83f678c2b984..93f10070fbf4 100644 --- a/packages/web/src/components/RedwoodApolloProvider.tsx +++ b/packages/web/src/components/RedwoodApolloProvider.tsx @@ -2,12 +2,15 @@ import { ApolloProvider, ApolloClientOptions, ApolloClient, + ApolloLink, InMemoryCache, useQuery, useMutation, + createHttpLink, } from '@apollo/client' +import { setContext } from '@apollo/client/link/context' -import type { AuthContextInterface } from '@redwoodjs/auth' +import { AuthContextInterface, useAuth } from '@redwoodjs/auth' import { FetchConfigProvider, @@ -20,12 +23,33 @@ const ApolloProviderWithFetchConfig: React.FunctionComponent<{ config?: Omit, 'cache'> }> = ({ config = {}, children }) => { const { uri, headers } = useFetchConfig() + const { getFreshToken, type: authProviderType } = useAuth() + + const withToken = setContext(async () => { + const token = await getFreshToken() + return { token } + }) + + const authMiddleware = new ApolloLink((operation, forward) => { + const { token } = operation.getContext() + + operation.setContext(() => ({ + headers: { + ...headers, + // Duped, because we may remove FetchContext at a later date + 'auth-provider': authProviderType, + authorization: token ? `Bearer ${token}` : null, + }, + })) + return forward(operation) + }) + + const httpLink = createHttpLink({ uri }) const client = new ApolloClient({ cache: new InMemoryCache(), - uri, - headers, ...config, + link: ApolloLink.from([withToken, authMiddleware.concat(httpLink)]), }) return {children} From a388d617fc0541e3c67aca035b050bc1f8787029 Mon Sep 17 00:00:00 2001 From: Daniel Choudhury Date: Thu, 7 Jan 2021 14:10:20 +0000 Subject: [PATCH 2/8] Update packages/auth/src/AuthProvider.tsx Co-authored-by: Peter Pistorius --- packages/auth/src/AuthProvider.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/auth/src/AuthProvider.tsx b/packages/auth/src/AuthProvider.tsx index e7ce78d1c33e..bf5e9be6a2be 100644 --- a/packages/auth/src/AuthProvider.tsx +++ b/packages/auth/src/AuthProvider.tsx @@ -16,6 +16,9 @@ export interface AuthContextInterface { /* Determining your current authentication state */ loading: boolean isAuthenticated: boolean + /** + * @deprecated auth tokens are now refreshed when they expire, and this value will be removed from the context in the next few releases: https://github.com/redwoodjs/redwood/pull/1609 + */ authToken: string | null // @WARN! to be depracated /* The current user's data from the `getCurrentUser` function on the api side */ currentUser: null | CurrentUser From 839a60855086de7a7475409f978dabbd819e8cae Mon Sep 17 00:00:00 2001 From: Daniel Choudhury Date: Thu, 7 Jan 2021 16:03:44 +0000 Subject: [PATCH 3/8] Implement requested changes on PR --- packages/auth/src/AuthProvider.tsx | 28 ++++++------------- packages/testing/src/MockProviders.tsx | 1 - .../src/components/RedwoodApolloProvider.tsx | 6 ++-- 3 files changed, 11 insertions(+), 24 deletions(-) diff --git a/packages/auth/src/AuthProvider.tsx b/packages/auth/src/AuthProvider.tsx index bf5e9be6a2be..2cffda95b2c6 100644 --- a/packages/auth/src/AuthProvider.tsx +++ b/packages/auth/src/AuthProvider.tsx @@ -16,10 +16,10 @@ export interface AuthContextInterface { /* Determining your current authentication state */ loading: boolean isAuthenticated: boolean - /** - * @deprecated auth tokens are now refreshed when they expire, and this value will be removed from the context in the next few releases: https://github.com/redwoodjs/redwood/pull/1609 - */ - authToken: string | null // @WARN! to be depracated + /** + * @deprecated auth tokens are now refreshed when they expire, use getToken() instead. authToken will be removed from this context in future releases + */ + authToken: string | null // @WARN! deprecated, will always be null /* The current user's data from the `getCurrentUser` function on the api side */ currentUser: null | CurrentUser /* The user's metadata from the auth provider */ @@ -28,8 +28,6 @@ export interface AuthContextInterface { logOut(options?: unknown): Promise signUp(options?: unknown): Promise getToken(): Promise - // To be removed once authToken is removed - getFreshToken(): Promise /** * Fetches the "currentUser" from the api side, * but does not update the current user state. @@ -60,7 +58,7 @@ export interface AuthContextInterface { export const AuthContext = React.createContext({ loading: true, isAuthenticated: false, - authToken: null, // @WARN! to be depracated + authToken: null, // @WARN! deprecated, will always be null userMetadata: null, currentUser: null, }) @@ -74,7 +72,7 @@ type AuthProviderProps = { type AuthProviderState = { loading: boolean isAuthenticated: boolean - authToken: string | null // @WARN! to be depracated + authToken: string | null // @WARN! deprecated, will always be null userMetadata: null | Record currentUser: null | CurrentUser hasError: boolean @@ -101,7 +99,7 @@ export class AuthProvider extends React.Component< state: AuthProviderState = { loading: true, isAuthenticated: false, - authToken: null, // @WARN! to be depracated + authToken: null, // @WARN! deprecated, will always be null userMetadata: null, currentUser: null, hasError: false, @@ -180,22 +178,13 @@ export class AuthProvider extends React.Component< } getToken = async () => { - const authToken = await this.rwClient.getToken() - this.setState({ ...this.state, authToken }) - return authToken - } - - // Note: Used by apollo middleware to get a fresh token - // State isn't changed, to prevent infinite loops - // Remove this, and call getToken when AuthProviderState.authToken is removed - getFreshToken = async () => { return this.rwClient.getToken() } reauthenticate = async () => { const notAuthenticatedState: AuthProviderState = { isAuthenticated: false, - authToken: null, // @WARN! to be depracated + authToken: null, // @WARN! deprecated, will always be null currentUser: null, userMetadata: null, loading: false, @@ -267,7 +256,6 @@ export class AuthProvider extends React.Component< getCurrentUser: this.getCurrentUser, hasRole: this.hasRole, reauthenticate: this.reauthenticate, - getFreshToken: this.getFreshToken, client, type, }} diff --git a/packages/testing/src/MockProviders.tsx b/packages/testing/src/MockProviders.tsx index 93fcf5ef29d4..1a2783ff8b99 100644 --- a/packages/testing/src/MockProviders.tsx +++ b/packages/testing/src/MockProviders.tsx @@ -24,7 +24,6 @@ const fakeUseAuth = (): AuthContextInterface => ({ logOut: async () => undefined, signUp: async () => undefined, getToken: async () => null, - getFreshToken: async () => null, getCurrentUser: async () => null, hasRole: () => false, reauthenticate: async () => undefined, diff --git a/packages/web/src/components/RedwoodApolloProvider.tsx b/packages/web/src/components/RedwoodApolloProvider.tsx index 93f10070fbf4..6ade5050ead7 100644 --- a/packages/web/src/components/RedwoodApolloProvider.tsx +++ b/packages/web/src/components/RedwoodApolloProvider.tsx @@ -23,10 +23,10 @@ const ApolloProviderWithFetchConfig: React.FunctionComponent<{ config?: Omit, 'cache'> }> = ({ config = {}, children }) => { const { uri, headers } = useFetchConfig() - const { getFreshToken, type: authProviderType } = useAuth() + const { getToken, type: authProviderType } = useAuth() const withToken = setContext(async () => { - const token = await getFreshToken() + const token = await getToken() return { token } }) @@ -36,7 +36,7 @@ const ApolloProviderWithFetchConfig: React.FunctionComponent<{ operation.setContext(() => ({ headers: { ...headers, - // Duped, because we may remove FetchContext at a later date + // Duped auth headers, because we may remove FetchContext at a later date 'auth-provider': authProviderType, authorization: token ? `Bearer ${token}` : null, }, From 206f109a4c92b461d6abba8473fb414e3d08d18a Mon Sep 17 00:00:00 2001 From: Daniel Choudhury Date: Thu, 7 Jan 2021 16:12:26 +0000 Subject: [PATCH 4/8] Update tests to reflect authToken changes --- packages/auth/src/__tests__/AuthProvider.test.tsx | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/auth/src/__tests__/AuthProvider.test.tsx b/packages/auth/src/__tests__/AuthProvider.test.tsx index 8ea14eb37eaf..3656e383bd52 100644 --- a/packages/auth/src/__tests__/AuthProvider.test.tsx +++ b/packages/auth/src/__tests__/AuthProvider.test.tsx @@ -9,6 +9,8 @@ import type { AuthClient } from '../authClients' import { AuthProvider } from '../AuthProvider' import { useAuth } from '../useAuth' +import { useEffect, useState } from 'react' + let CURRENT_USER_DATA: { name: string; email: string; roles?: string[] } = { name: 'Peter Pistorius', email: 'nospam@example.net', @@ -41,9 +43,9 @@ const AuthConsumer = () => { const { loading, isAuthenticated, - authToken, logOut, logIn, + getToken, userMetadata, currentUser, reauthenticate, @@ -52,6 +54,17 @@ const AuthConsumer = () => { error, } = useAuth() + const [authToken, setAuthToken] = useState(null) + + const retrieveToken = async () => { + const token = await getToken() + setAuthToken(token) + } + + useEffect(() => { + retrieveToken() + }) + if (loading) { return <>Loading... } From 421613b289f2e0b601b77639ab251f34cb6935ff Mon Sep 17 00:00:00 2001 From: Daniel Choudhury Date: Thu, 7 Jan 2021 16:16:32 +0000 Subject: [PATCH 5/8] Import order for linting --- packages/auth/src/__tests__/AuthProvider.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/auth/src/__tests__/AuthProvider.test.tsx b/packages/auth/src/__tests__/AuthProvider.test.tsx index 3656e383bd52..e1ee2dab1885 100644 --- a/packages/auth/src/__tests__/AuthProvider.test.tsx +++ b/packages/auth/src/__tests__/AuthProvider.test.tsx @@ -1,5 +1,7 @@ require('whatwg-fetch') +import { useEffect, useState } from 'react' + import { render, screen, fireEvent, waitFor } from '@testing-library/react' import '@testing-library/jest-dom/extend-expect' import { graphql } from 'msw' @@ -9,8 +11,6 @@ import type { AuthClient } from '../authClients' import { AuthProvider } from '../AuthProvider' import { useAuth } from '../useAuth' -import { useEffect, useState } from 'react' - let CURRENT_USER_DATA: { name: string; email: string; roles?: string[] } = { name: 'Peter Pistorius', email: 'nospam@example.net', From 485bcddbc2fd7fd0b4a3b118510b2670213a179b Mon Sep 17 00:00:00 2001 From: Daniel Choudhury Date: Tue, 12 Jan 2021 13:48:02 +0000 Subject: [PATCH 6/8] getToken only when isAuthentication | Remove always-null authorization header --- .../src/components/FetchConfigProvider.tsx | 7 +++--- .../src/components/RedwoodApolloProvider.tsx | 23 +++++++++++++++---- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/packages/web/src/components/FetchConfigProvider.tsx b/packages/web/src/components/FetchConfigProvider.tsx index 5fcb11cefd2b..b9280d274540 100644 --- a/packages/web/src/components/FetchConfigProvider.tsx +++ b/packages/web/src/components/FetchConfigProvider.tsx @@ -2,7 +2,7 @@ import type { AuthContextInterface, SupportedAuthTypes } from '@redwoodjs/auth' export interface FetchConfig { uri: string - headers?: { 'auth-provider': SupportedAuthTypes; authorization: string } + headers?: { 'auth-provider': SupportedAuthTypes; authorization?: string } } export const FetchConfigContext = React.createContext({ uri: `${window.__REDWOOD__API_PROXY_PATH}/graphql`, @@ -19,11 +19,11 @@ export const FetchConfigProvider: React.FunctionComponent<{ (() => ({ loading: false, isAuthenticated: false })), ...rest }) => { - const { isAuthenticated, authToken, type } = useAuth() + const { isAuthenticated, type } = useAuth() // Even though the user may be authenticated and we may require `authToken` to continue // This should be handled by the `Private` route. - if (!isAuthenticated || !authToken) { + if (!isAuthenticated) { return ( , 'cache'> }> = ({ config = {}, children }) => { const { uri, headers } = useFetchConfig() - const { getToken, type: authProviderType } = useAuth() + const { getToken, type: authProviderType, isAuthenticated } = useAuth() const withToken = setContext(async () => { - const token = await getToken() - return { token } + if (isAuthenticated && getToken) { + const token = await getToken() + + return { token } + } + + return { token: null } }) const authMiddleware = new ApolloLink((operation, forward) => { const { token } = operation.getContext() + // Only add auth headers when token is present + // Token is null, when !isAuthenticated + const authHeaders = token + ? { + 'auth-provider': authProviderType, + authorization: token ? `Bearer ${token}` : null, + } + : {} + operation.setContext(() => ({ headers: { ...headers, // Duped auth headers, because we may remove FetchContext at a later date - 'auth-provider': authProviderType, - authorization: token ? `Bearer ${token}` : null, + ...authHeaders, }, })) return forward(operation) From 3224ed478daf555e1ed0ecc8006879ec3990184a Mon Sep 17 00:00:00 2001 From: Daniel Choudhury Date: Thu, 21 Jan 2021 11:17:33 +0000 Subject: [PATCH 7/8] Update packages/web/src/components/RedwoodApolloProvider.tsx Co-authored-by: Peter Pistorius --- packages/web/src/components/RedwoodApolloProvider.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web/src/components/RedwoodApolloProvider.tsx b/packages/web/src/components/RedwoodApolloProvider.tsx index 674c98690623..d1e1cca1676d 100644 --- a/packages/web/src/components/RedwoodApolloProvider.tsx +++ b/packages/web/src/components/RedwoodApolloProvider.tsx @@ -43,7 +43,7 @@ const ApolloProviderWithFetchConfig: React.FunctionComponent<{ const authHeaders = token ? { 'auth-provider': authProviderType, - authorization: token ? `Bearer ${token}` : null, + authorization: `Bearer ${token}` } : {} From 3a11c79ff7b4d1876796d30930626535fb69e1bb Mon Sep 17 00:00:00 2001 From: Peter Pistorius Date: Thu, 21 Jan 2021 17:46:02 +0200 Subject: [PATCH 8/8] Update packages/auth/src/__tests__/AuthProvider.test.tsx --- packages/auth/src/__tests__/AuthProvider.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/auth/src/__tests__/AuthProvider.test.tsx b/packages/auth/src/__tests__/AuthProvider.test.tsx index e1ee2dab1885..6c1d9241d405 100644 --- a/packages/auth/src/__tests__/AuthProvider.test.tsx +++ b/packages/auth/src/__tests__/AuthProvider.test.tsx @@ -63,7 +63,7 @@ const AuthConsumer = () => { useEffect(() => { retrieveToken() - }) + }, []) if (loading) { return <>Loading...