Skip to content

Commit 63536b4

Browse files
authored
Merge pull request #9861 from marmelab/fix-authProviderContext-type
Fix `useAuthProvider` may return `undefined` when no `authProvider` is available
2 parents 67dea51 + 91477ac commit 63536b4

File tree

9 files changed

+160
-135
lines changed

9 files changed

+160
-135
lines changed

docs/Upgrade.md

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ React-admin v5 mostly focuses on removing deprecated features and upgrading depe
4747
- [Global Server Side Validation Error Message Must Be Passed Via The `root.serverError` Key](#global-server-side-validation-error-message-must-be-passed-via-the-rootservererror-key)
4848
- [TypeScript](#typescript)
4949
- [Fields Components Requires The source Prop](#fields-components-requires-the-source-prop)
50-
- [`useRecordContext` Returns undefined When No Record Is Available](#userecordcontext-returns-undefined-when-no-record-is-available)
50+
- [`useRecordContext` Returns `undefined` When No Record Is Available](#userecordcontext-returns-undefined-when-no-record-is-available)
51+
- [`useAuthProvider` Returns `undefined` When No `authProvider` Is Available](#useauthprovider-returns-undefined-when-no-authprovider-is-available)
5152
- [Page Contexts Are Now Types Instead of Interfaces](#page-contexts-are-now-types-instead-of-interfaces)
5253
- [Stronger Types For Page Contexts](#stronger-types-for-page-contexts)
5354
- [EditProps and CreateProps now expect a children prop](#editprops-and-createprops-now-expect-a-children-prop)
@@ -1086,6 +1087,30 @@ const MyComponent = () => {
10861087
};
10871088
```
10881089

1090+
### `useAuthProvider` Returns `undefined` When No `authProvider` Is Available
1091+
1092+
The `useAuthProvider` hook returns the current `authProvider`. Since the `authProvider` is optional, this context may be empty. Thus, the return type for `useAuthProvider` has been modified to `AuthProvider | undefined` instead of `AuthProvider` to denote this possibility.
1093+
1094+
As a consequence, the TypeScript compilation of your project may fail if you don't check the existence of the `authProvider` before reading it.
1095+
1096+
To fix this error, your code should handle the case where `useAuthProvider` returns `undefined`:
1097+
1098+
```diff
1099+
const useGetPermissions = (): GetPermissions => {
1100+
const authProvider = useAuthProvider();
1101+
const getPermissions = useCallback(
1102+
(params: any = {}) =>
1103+
+ authProvider ?
1104+
authProvider
1105+
.getPermissions(params)
1106+
.then(result => result ?? null)
1107+
+ : Promise.resolve([]),
1108+
[authProvider]
1109+
);
1110+
return getPermissions;
1111+
};
1112+
```
1113+
10891114
### Page Contexts Are Now Types Instead of Interfaces
10901115

10911116
The return type of page controllers is now a type. If you were using an interface extending one of:
Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,7 @@
11
import { createContext } from 'react';
22

3-
import { AuthProvider, UserIdentity } from '../types';
3+
import { AuthProvider } from '../types';
44

5-
const defaultIdentity: UserIdentity = { id: '' };
6-
7-
export const defaultAuthProvider: AuthProvider = {
8-
login: () => Promise.resolve(),
9-
logout: () => Promise.resolve(),
10-
checkAuth: () => Promise.resolve(),
11-
checkError: () => Promise.resolve(),
12-
getPermissions: () => Promise.resolve(),
13-
getIdentity: () => Promise.resolve(defaultIdentity),
14-
};
15-
16-
export const AuthContext = createContext<AuthProvider>(defaultAuthProvider);
5+
export const AuthContext = createContext<AuthProvider | undefined>(undefined);
176

187
AuthContext.displayName = 'AuthContext';

packages/ra-core/src/auth/useAuthProvider.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export const defaultAuthParams = {
1313
*/
1414
const useAuthProvider = <
1515
AuthProviderType extends AuthProvider = AuthProvider
16-
>(): AuthProviderType => useContext(AuthContext) as AuthProviderType;
16+
>(): AuthProviderType | undefined =>
17+
useContext(AuthContext) as AuthProviderType | undefined;
1718

1819
export default useAuthProvider;

packages/ra-core/src/auth/useCheckAuth.ts

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,27 +53,33 @@ export const useCheckAuth = (): CheckAuth => {
5353

5454
const checkAuth = useCallback(
5555
(params: any = {}, logoutOnFailure = true, redirectTo = loginUrl) =>
56-
authProvider.checkAuth(params).catch(error => {
57-
if (logoutOnFailure) {
58-
logout(
59-
{},
60-
error && error.redirectTo != null
61-
? error.redirectTo
62-
: redirectTo
63-
);
64-
const shouldSkipNotify = error && error.message === false;
65-
!shouldSkipNotify &&
66-
notify(
67-
getErrorMessage(error, 'ra.auth.auth_check_error'),
68-
{ type: 'error' }
69-
);
70-
}
71-
throw error;
72-
}),
56+
authProvider
57+
? authProvider.checkAuth(params).catch(error => {
58+
if (logoutOnFailure) {
59+
logout(
60+
{},
61+
error && error.redirectTo != null
62+
? error.redirectTo
63+
: redirectTo
64+
);
65+
const shouldSkipNotify =
66+
error && error.message === false;
67+
!shouldSkipNotify &&
68+
notify(
69+
getErrorMessage(
70+
error,
71+
'ra.auth.auth_check_error'
72+
),
73+
{ type: 'error' }
74+
);
75+
}
76+
throw error;
77+
})
78+
: checkAuthWithoutAuthProvider(),
7379
[authProvider, logout, notify, loginUrl]
7480
);
7581

76-
return authProvider ? checkAuth : checkAuthWithoutAuthProvider;
82+
return checkAuth;
7783
};
7884

7985
const checkAuthWithoutAuthProvider = () => Promise.resolve();

packages/ra-core/src/auth/useGetPermissions.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,15 @@ const useGetPermissions = (): GetPermissions => {
4040
const getPermissions = useCallback(
4141
(params: any = {}) =>
4242
// react-query requires the query to return something
43-
authProvider.getPermissions(params).then(result => result ?? null),
43+
authProvider
44+
? authProvider
45+
.getPermissions(params)
46+
.then(result => result ?? null)
47+
: getPermissionsWithoutProvider(),
4448
[authProvider]
4549
);
4650

47-
return authProvider ? getPermissions : getPermissionsWithoutProvider;
51+
return getPermissions;
4852
};
4953

5054
/**

packages/ra-core/src/auth/useLogin.ts

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -43,21 +43,28 @@ const useLogin = (): Login => {
4343
);
4444

4545
const login = useCallback(
46-
(params: any = {}, pathName) =>
47-
authProvider.login(params).then(ret => {
48-
resetNotifications();
49-
if (ret && ret.hasOwnProperty('redirectTo')) {
50-
if (ret) {
51-
navigate(ret.redirectTo);
46+
(params: any = {}, pathName) => {
47+
if (authProvider) {
48+
return authProvider.login(params).then(ret => {
49+
resetNotifications();
50+
if (ret && ret.hasOwnProperty('redirectTo')) {
51+
if (ret) {
52+
navigate(ret.redirectTo);
53+
}
54+
} else {
55+
const redirectUrl = pathName
56+
? pathName
57+
: nextPathName + nextSearch || afterLoginUrl;
58+
navigate(redirectUrl);
5259
}
53-
} else {
54-
const redirectUrl = pathName
55-
? pathName
56-
: nextPathName + nextSearch || afterLoginUrl;
57-
navigate(redirectUrl);
58-
}
59-
return ret;
60-
}),
60+
return ret;
61+
});
62+
} else {
63+
resetNotifications();
64+
navigate(afterLoginUrl);
65+
return Promise.resolve();
66+
}
67+
},
6168
[
6269
authProvider,
6370
navigate,
@@ -68,16 +75,7 @@ const useLogin = (): Login => {
6875
]
6976
);
7077

71-
const loginWithoutProvider = useCallback(
72-
(_, __) => {
73-
resetNotifications();
74-
navigate(afterLoginUrl);
75-
return Promise.resolve();
76-
},
77-
[navigate, resetNotifications, afterLoginUrl]
78-
);
79-
80-
return authProvider ? login : loginWithoutProvider;
78+
return login;
8179
};
8280

8381
/**

packages/ra-core/src/auth/useLogout.ts

Lines changed: 66 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -62,77 +62,83 @@ const useLogout = (): Logout => {
6262
params = {},
6363
redirectTo = loginUrl,
6464
redirectToCurrentLocationAfterLogin = true
65-
) =>
66-
authProvider.logout(params).then(redirectToFromProvider => {
67-
if (redirectToFromProvider === false || redirectTo === false) {
68-
resetStore();
69-
queryClient.clear();
70-
// do not redirect
71-
return;
72-
}
65+
) => {
66+
if (authProvider) {
67+
return authProvider
68+
.logout(params)
69+
.then(redirectToFromProvider => {
70+
if (
71+
redirectToFromProvider === false ||
72+
redirectTo === false
73+
) {
74+
resetStore();
75+
queryClient.clear();
76+
// do not redirect
77+
return;
78+
}
7379

74-
const finalRedirectTo = redirectToFromProvider || redirectTo;
80+
const finalRedirectTo =
81+
redirectToFromProvider || redirectTo;
7582

76-
if (finalRedirectTo?.startsWith('http')) {
77-
// absolute link (e.g. https://my.oidc.server/login)
78-
resetStore();
79-
queryClient.clear();
80-
window.location.href = finalRedirectTo;
81-
return finalRedirectTo;
82-
}
83+
if (finalRedirectTo?.startsWith('http')) {
84+
// absolute link (e.g. https://my.oidc.server/login)
85+
resetStore();
86+
queryClient.clear();
87+
window.location.href = finalRedirectTo;
88+
return finalRedirectTo;
89+
}
8390

84-
// redirectTo is an internal location that may contain a query string, e.g. '/login?foo=bar'
85-
// we must split it to pass a structured location to navigate()
86-
const redirectToParts = finalRedirectTo.split('?');
87-
const newLocation: Partial<Path> = {
88-
pathname: redirectToParts[0],
89-
};
90-
let newLocationOptions = {};
91+
// redirectTo is an internal location that may contain a query string, e.g. '/login?foo=bar'
92+
// we must split it to pass a structured location to navigate()
93+
const redirectToParts = finalRedirectTo.split('?');
94+
const newLocation: Partial<Path> = {
95+
pathname: redirectToParts[0],
96+
};
97+
let newLocationOptions = {};
9198

92-
if (
93-
redirectToCurrentLocationAfterLogin &&
94-
locationRef.current &&
95-
locationRef.current.pathname
96-
) {
97-
newLocationOptions = {
99+
if (
100+
redirectToCurrentLocationAfterLogin &&
101+
locationRef.current &&
102+
locationRef.current.pathname
103+
) {
104+
newLocationOptions = {
105+
state: {
106+
nextPathname: locationRef.current.pathname,
107+
nextSearch: locationRef.current.search,
108+
},
109+
};
110+
}
111+
if (redirectToParts[1]) {
112+
newLocation.search = redirectToParts[1];
113+
}
114+
navigateRef.current(newLocation, newLocationOptions);
115+
resetStore();
116+
queryClient.clear();
117+
118+
return redirectToFromProvider;
119+
});
120+
} else {
121+
navigateRef.current(
122+
{
123+
pathname: loginUrl,
124+
},
125+
{
98126
state: {
99-
nextPathname: locationRef.current.pathname,
100-
nextSearch: locationRef.current.search,
127+
nextPathname:
128+
locationRef.current &&
129+
locationRef.current.pathname,
101130
},
102-
};
103-
}
104-
if (redirectToParts[1]) {
105-
newLocation.search = redirectToParts[1];
106-
}
107-
navigateRef.current(newLocation, newLocationOptions);
131+
}
132+
);
108133
resetStore();
109134
queryClient.clear();
110-
111-
return redirectToFromProvider;
112-
}),
113-
[authProvider, resetStore, loginUrl, queryClient]
114-
);
115-
116-
const logoutWithoutProvider = useCallback(
117-
_ => {
118-
navigate(
119-
{
120-
pathname: loginUrl,
121-
},
122-
{
123-
state: {
124-
nextPathname: location && location.pathname,
125-
},
126-
}
127-
);
128-
resetStore();
129-
queryClient.clear();
130-
return Promise.resolve();
135+
return Promise.resolve();
136+
}
131137
},
132-
[resetStore, location, navigate, loginUrl, queryClient]
138+
[authProvider, resetStore, loginUrl, queryClient]
133139
);
134140

135-
return authProvider ? logout : logoutWithoutProvider;
141+
return logout;
136142
};
137143

138144
/**

packages/ra-core/src/auth/useLogoutIfAccessDenied.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,11 @@ const useLogoutIfAccessDenied = (): LogoutIfAccessDenied => {
4444
const notify = useNotify();
4545
const navigate = useNavigate();
4646
const logoutIfAccessDenied = useCallback(
47-
(error?: any) =>
48-
authProvider
47+
(error?: any) => {
48+
if (!authProvider) {
49+
return logoutIfAccessDeniedWithoutProvider();
50+
}
51+
return authProvider
4952
.checkError(error)
5053
.then(() => false)
5154
.catch(async e => {
@@ -110,12 +113,11 @@ const useLogoutIfAccessDenied = (): LogoutIfAccessDenied => {
110113
}
111114

112115
return true;
113-
}),
116+
});
117+
},
114118
[authProvider, logout, notify, navigate]
115119
);
116-
return authProvider
117-
? logoutIfAccessDenied
118-
: logoutIfAccessDeniedWithoutProvider;
120+
return logoutIfAccessDenied;
119121
};
120122

121123
const logoutIfAccessDeniedWithoutProvider = () => Promise.resolve(false);

0 commit comments

Comments
 (0)