Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix useAuthProvider may return undefined when no authProvider is available #9861

Merged
merged 2 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion docs/Upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ React-admin v5 mostly focuses on removing deprecated features and upgrading depe
- [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)
- [TypeScript](#typescript)
- [Fields Components Requires The source Prop](#fields-components-requires-the-source-prop)
- [`useRecordContext` Returns undefined When No Record Is Available](#userecordcontext-returns-undefined-when-no-record-is-available)
- [`useRecordContext` Returns `undefined` When No Record Is Available](#userecordcontext-returns-undefined-when-no-record-is-available)
- [`useAuthProvider` Returns `undefined` When No `authProvider` Is Available](#useauthprovider-returns-undefined-when-no-authprovider-is-available)
- [Page Contexts Are Now Types Instead of Interfaces](#page-contexts-are-now-types-instead-of-interfaces)
- [Stronger Types For Page Contexts](#stronger-types-for-page-contexts)
- [EditProps and CreateProps now expect a children prop](#editprops-and-createprops-now-expect-a-children-prop)
Expand Down Expand Up @@ -1086,6 +1087,30 @@ const MyComponent = () => {
};
```

### `useAuthProvider` Returns `undefined` When No `authProvider` Is Available

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.

As a consequence, the TypeScript compilation of your project may fail if you don't check the existence of the `authProvider` before reading it.

To fix this error, your code should handle the case where `useAuthProvider` returns `undefined`:

```diff
const useGetPermissions = (): GetPermissions => {
const authProvider = useAuthProvider();
const getPermissions = useCallback(
(params: any = {}) =>
+ authProvider ?
authProvider
.getPermissions(params)
.then(result => result ?? null)
+ : Promise.resolve([]),
[authProvider]
);
return getPermissions;
};
```

### Page Contexts Are Now Types Instead of Interfaces

The return type of page controllers is now a type. If you were using an interface extending one of:
Expand Down
15 changes: 2 additions & 13 deletions packages/ra-core/src/auth/AuthContext.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,7 @@
import { createContext } from 'react';

import { AuthProvider, UserIdentity } from '../types';
import { AuthProvider } from '../types';

const defaultIdentity: UserIdentity = { id: '' };

export const defaultAuthProvider: AuthProvider = {
login: () => Promise.resolve(),
logout: () => Promise.resolve(),
checkAuth: () => Promise.resolve(),
checkError: () => Promise.resolve(),
getPermissions: () => Promise.resolve(),
getIdentity: () => Promise.resolve(defaultIdentity),
};

export const AuthContext = createContext<AuthProvider>(defaultAuthProvider);
export const AuthContext = createContext<AuthProvider | undefined>(undefined);

AuthContext.displayName = 'AuthContext';
3 changes: 2 additions & 1 deletion packages/ra-core/src/auth/useAuthProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const defaultAuthParams = {
*/
const useAuthProvider = <
AuthProviderType extends AuthProvider = AuthProvider
>(): AuthProviderType => useContext(AuthContext) as AuthProviderType;
>(): AuthProviderType | undefined =>
useContext(AuthContext) as AuthProviderType | undefined;

export default useAuthProvider;
42 changes: 24 additions & 18 deletions packages/ra-core/src/auth/useCheckAuth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,27 +53,33 @@ export const useCheckAuth = (): CheckAuth => {

const checkAuth = useCallback(
(params: any = {}, logoutOnFailure = true, redirectTo = loginUrl) =>
authProvider.checkAuth(params).catch(error => {
if (logoutOnFailure) {
logout(
{},
error && error.redirectTo != null
? error.redirectTo
: redirectTo
);
const shouldSkipNotify = error && error.message === false;
!shouldSkipNotify &&
notify(
getErrorMessage(error, 'ra.auth.auth_check_error'),
{ type: 'error' }
);
}
throw error;
}),
authProvider
? authProvider.checkAuth(params).catch(error => {
if (logoutOnFailure) {
logout(
{},
error && error.redirectTo != null
? error.redirectTo
: redirectTo
);
const shouldSkipNotify =
error && error.message === false;
!shouldSkipNotify &&
notify(
getErrorMessage(
error,
'ra.auth.auth_check_error'
),
{ type: 'error' }
);
}
throw error;
})
: checkAuthWithoutAuthProvider(),
[authProvider, logout, notify, loginUrl]
);

return authProvider ? checkAuth : checkAuthWithoutAuthProvider;
return checkAuth;
};

const checkAuthWithoutAuthProvider = () => Promise.resolve();
Expand Down
8 changes: 6 additions & 2 deletions packages/ra-core/src/auth/useGetPermissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,15 @@ const useGetPermissions = (): GetPermissions => {
const getPermissions = useCallback(
(params: any = {}) =>
// react-query requires the query to return something
authProvider.getPermissions(params).then(result => result ?? null),
authProvider
? authProvider
.getPermissions(params)
.then(result => result ?? null)
: getPermissionsWithoutProvider(),
[authProvider]
);

return authProvider ? getPermissions : getPermissionsWithoutProvider;
return getPermissions;
};

/**
Expand Down
46 changes: 22 additions & 24 deletions packages/ra-core/src/auth/useLogin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,28 @@ const useLogin = (): Login => {
);

const login = useCallback(
(params: any = {}, pathName) =>
authProvider.login(params).then(ret => {
resetNotifications();
if (ret && ret.hasOwnProperty('redirectTo')) {
if (ret) {
navigate(ret.redirectTo);
(params: any = {}, pathName) => {
if (authProvider) {
return authProvider.login(params).then(ret => {
resetNotifications();
if (ret && ret.hasOwnProperty('redirectTo')) {
if (ret) {
navigate(ret.redirectTo);
}
} else {
const redirectUrl = pathName
? pathName
: nextPathName + nextSearch || afterLoginUrl;
navigate(redirectUrl);
}
} else {
const redirectUrl = pathName
? pathName
: nextPathName + nextSearch || afterLoginUrl;
navigate(redirectUrl);
}
return ret;
}),
return ret;
});
} else {
resetNotifications();
navigate(afterLoginUrl);
return Promise.resolve();
}
},
[
authProvider,
navigate,
Expand All @@ -68,16 +75,7 @@ const useLogin = (): Login => {
]
);

const loginWithoutProvider = useCallback(
(_, __) => {
resetNotifications();
navigate(afterLoginUrl);
return Promise.resolve();
},
[navigate, resetNotifications, afterLoginUrl]
);

return authProvider ? login : loginWithoutProvider;
return login;
};

/**
Expand Down
126 changes: 66 additions & 60 deletions packages/ra-core/src/auth/useLogout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,77 +62,83 @@ const useLogout = (): Logout => {
params = {},
redirectTo = loginUrl,
redirectToCurrentLocationAfterLogin = true
) =>
authProvider.logout(params).then(redirectToFromProvider => {
if (redirectToFromProvider === false || redirectTo === false) {
resetStore();
queryClient.clear();
// do not redirect
return;
}
) => {
if (authProvider) {
return authProvider
.logout(params)
.then(redirectToFromProvider => {
if (
redirectToFromProvider === false ||
redirectTo === false
) {
resetStore();
queryClient.clear();
// do not redirect
return;
}

const finalRedirectTo = redirectToFromProvider || redirectTo;
const finalRedirectTo =
redirectToFromProvider || redirectTo;

if (finalRedirectTo?.startsWith('http')) {
// absolute link (e.g. https://my.oidc.server/login)
resetStore();
queryClient.clear();
window.location.href = finalRedirectTo;
return finalRedirectTo;
}
if (finalRedirectTo?.startsWith('http')) {
// absolute link (e.g. https://my.oidc.server/login)
resetStore();
queryClient.clear();
window.location.href = finalRedirectTo;
return finalRedirectTo;
}

// redirectTo is an internal location that may contain a query string, e.g. '/login?foo=bar'
// we must split it to pass a structured location to navigate()
const redirectToParts = finalRedirectTo.split('?');
const newLocation: Partial<Path> = {
pathname: redirectToParts[0],
};
let newLocationOptions = {};
// redirectTo is an internal location that may contain a query string, e.g. '/login?foo=bar'
// we must split it to pass a structured location to navigate()
const redirectToParts = finalRedirectTo.split('?');
const newLocation: Partial<Path> = {
pathname: redirectToParts[0],
};
let newLocationOptions = {};

if (
redirectToCurrentLocationAfterLogin &&
locationRef.current &&
locationRef.current.pathname
) {
newLocationOptions = {
if (
redirectToCurrentLocationAfterLogin &&
locationRef.current &&
locationRef.current.pathname
) {
newLocationOptions = {
state: {
nextPathname: locationRef.current.pathname,
nextSearch: locationRef.current.search,
},
};
}
if (redirectToParts[1]) {
newLocation.search = redirectToParts[1];
}
navigateRef.current(newLocation, newLocationOptions);
resetStore();
queryClient.clear();

return redirectToFromProvider;
});
} else {
navigateRef.current(
{
pathname: loginUrl,
},
{
state: {
nextPathname: locationRef.current.pathname,
nextSearch: locationRef.current.search,
nextPathname:
locationRef.current &&
locationRef.current.pathname,
},
};
}
if (redirectToParts[1]) {
newLocation.search = redirectToParts[1];
}
navigateRef.current(newLocation, newLocationOptions);
}
);
resetStore();
queryClient.clear();

return redirectToFromProvider;
}),
[authProvider, resetStore, loginUrl, queryClient]
);

const logoutWithoutProvider = useCallback(
_ => {
navigate(
{
pathname: loginUrl,
},
{
state: {
nextPathname: location && location.pathname,
},
}
);
resetStore();
queryClient.clear();
return Promise.resolve();
return Promise.resolve();
}
},
[resetStore, location, navigate, loginUrl, queryClient]
[authProvider, resetStore, loginUrl, queryClient]
);

return authProvider ? logout : logoutWithoutProvider;
return logout;
};

/**
Expand Down
14 changes: 8 additions & 6 deletions packages/ra-core/src/auth/useLogoutIfAccessDenied.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,11 @@ const useLogoutIfAccessDenied = (): LogoutIfAccessDenied => {
const notify = useNotify();
const navigate = useNavigate();
const logoutIfAccessDenied = useCallback(
(error?: any) =>
authProvider
(error?: any) => {
if (!authProvider) {
return logoutIfAccessDeniedWithoutProvider();
}
return authProvider
.checkError(error)
.then(() => false)
.catch(async e => {
Expand Down Expand Up @@ -110,12 +113,11 @@ const useLogoutIfAccessDenied = (): LogoutIfAccessDenied => {
}

return true;
}),
});
},
[authProvider, logout, notify, navigate]
);
return authProvider
? logoutIfAccessDenied
: logoutIfAccessDeniedWithoutProvider;
return logoutIfAccessDenied;
};

const logoutIfAccessDeniedWithoutProvider = () => Promise.resolve(false);
Expand Down
Loading
Loading