Skip to content

Commit

Permalink
[BC Break] Do not pass location to AUTH_GET_PERMISSIONS call
Browse files Browse the repository at this point in the history
Also, hooks that return a callback no longer accept a parameter. Pass the parameter to the callback instead.
  • Loading branch information
fzaninotto committed Sep 9, 2019
1 parent 94dfbe4 commit 9730e6c
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 86 deletions.
25 changes: 25 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -965,3 +965,28 @@ const ExportButton = ({ sort, filter, maxResults = 1000, resource }) => {
);
};
```

## The `authProvider` no longer receives the Location pathname in AUTH_GET_PERMISSIONS

When calling the `authProvider` for permissions (with the `AUTH_GET_PERMISSIONS` verb), react-admin used to include the pathname as second parameter. That allowed you to return different permissions based on the page.

We believe that permissions should not vary depending on where you are in the application ; it's up to components to decide to do something or not depending on permissions. So we've removed the pathname parameter from the calls - the `authProvider` doesn't receive it anymore.

If you want to keep location-dependent permissions logic, red the current location from the `window` object direclty in your `authProvider`:

```diff
// in myauthProvider.js
import { AUTH_LOGIN, AUTH_LOGOUT, AUTH_ERROR, AUTH_GET_PERMISSIONS } from 'react-admin';
import decodeJwt from 'jwt-decode';

export default (type, params) => {
// ...
if (type === AUTH_GET_PERMISSIONS) {
- const { pathname } = params;
+ const pathname = window.location.pathname;
// pathname-dependent logic follows
// ...
}
return Promise.reject('Unknown method');
};
```
10 changes: 5 additions & 5 deletions packages/ra-core/src/auth/useAuthState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const emptyParams = {};
*
* @see useAuthenticated()
*
* @param {Object} authParams Any params you want to pass to the authProvider
* @param {Object} params Any params you want to pass to the authProvider
*
* @returns The current auth check state. Destructure as { authenticated, error, loading, loaded }.
*
Expand All @@ -48,22 +48,22 @@ const emptyParams = {};
* </Admin>
* );
*/
const useAuthState = (authParams: any = emptyParams): State => {
const useAuthState = (params: any = emptyParams): State => {
const [state, setState] = useSafeSetState({
loading: true,
loaded: false,
authenticated: true, // optimistic
});
const checkAuth = useCheckAuth(authParams);
const checkAuth = useCheckAuth();
useEffect(() => {
checkAuth(false)
checkAuth(params, false)
.then(() =>
setState({ loading: false, loaded: true, authenticated: true })
)
.catch(() =>
setState({ loading: false, loaded: true, authenticated: false })
);
}, [checkAuth, setState]);
}, [checkAuth, params, setState]);
return state;
};

Expand Down
3 changes: 0 additions & 3 deletions packages/ra-core/src/auth/useAuthenticated.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ describe('useAuthenticated', () => {
);
expect(authProvider).toBeCalledTimes(1);
expect(authProvider.mock.calls[0][0]).toBe('AUTH_CHECK');
const payload = authProvider.mock.calls[0][1] as any;
expect(payload.afterLoginUrl).toBe('/');
expect(payload.loginUrl).toBe('/login');
expect(dispatch).toHaveBeenCalledTimes(0);
});

Expand Down
10 changes: 6 additions & 4 deletions packages/ra-core/src/auth/useAuthenticated.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { useEffect } from 'react';
import useCheckAuth from './useCheckAuth';

const emptyParams = {};

/**
* Restrict access to authenticated users.
* Redirect anonymous users to the login page.
Expand All @@ -26,9 +28,9 @@ import useCheckAuth from './useCheckAuth';
* </Admin>
* );
*/
export default authParams => {
const checkAuth = useCheckAuth(authParams);
export default (params: any = emptyParams) => {
const checkAuth = useCheckAuth();
useEffect(() => {
checkAuth().catch(() => {});
}, [checkAuth]);
checkAuth(params).catch(() => {});
}, [checkAuth, params]);
};
24 changes: 14 additions & 10 deletions packages/ra-core/src/auth/useCheckAuth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import useNotify from '../sideEffect/useNotify';
* @see useAuthenticated
* @see useAuthState
*
* @param {Object} authParams Any params you want to pass to the authProvider
*
* @returns {Function} checkAuth callback
*
* @example
Expand All @@ -33,24 +31,28 @@ import useNotify from '../sideEffect/useNotify';
* } // tip: use useAuthenticated() hook instead
*
* const MyPage = () => {
* const checkAuth = usecheckAuth();
* const checkAuth = useCheckAuth();
* const [authenticated, setAuthenticated] = useState(true); // optimistic auth
* useEffect(() => {
* checkAuth(false)
* checkAuth({}, false)
* .then() => setAuthenticated(true))
* .catch(() => setAuthenticated(false));
* }, []);
* return authenticated ? <Bar /> : <BarNotAuthenticated />;
* } // tip: use useAuthState() hook instead
*/
const useCheckAuth = (authParams: any = defaultAuthParams): CheckAuth => {
const useCheckAuth = (): CheckAuth => {
const authProvider = useAuthProvider();
const notify = useNotify();
const logout = useLogout(authParams);
const logout = useLogout();

const checkAuth = useCallback(
(logoutOnFailure = true, redirectTo = authParams.loginUrl) =>
authProvider(AUTH_CHECK, authParams).catch(error => {
(
params: any = {},
logoutOnFailure = true,
redirectTo = defaultAuthParams.loginUrl
) =>
authProvider(AUTH_CHECK, params).catch(error => {
if (logoutOnFailure) {
logout(redirectTo);
notify(
Expand All @@ -60,24 +62,26 @@ const useCheckAuth = (authParams: any = defaultAuthParams): CheckAuth => {
}
throw error;
}),
[authParams, authProvider, logout, notify]
[authProvider, logout, notify]
);

return authProvider ? checkAuth : checkAuthWithoutAuthProvider;
};

const checkAuthWithoutAuthProvider = (_, __) => Promise.resolve();
const checkAuthWithoutAuthProvider = () => Promise.resolve();

/**
* Check if the current user is authenticated by calling the authProvider AUTH_CHECK verb.
* Logs the user out on failure.
*
* @param {Object} params The parameters to pass to the authProvider
* @param {boolean} logoutOnFailure Whether the user should be logged out if the authProvider fails to authenticatde them. True by default.
* @param {string} redirectTo The login form url. Defaults to '/login'
*
* @return {Promise} Resolved to the authProvider response if the user passes the check, or rejected with an error otherwise
*/
type CheckAuth = (
params?: any,
logoutOnFailure?: boolean,
redirectTo?: string
) => Promise<any>;
Expand Down
35 changes: 6 additions & 29 deletions packages/ra-core/src/auth/useGetPermissions.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import { useCallback } from 'react';
import { useStore } from 'react-redux';
import { Location } from 'history';

import useAuthProvider, { defaultAuthParams } from './useAuthProvider';
import useAuthProvider from './useAuthProvider';
import { AUTH_GET_PERMISSIONS } from './types';

/**
* Get a callback for calling the authProvider with the AUTH_GET_PERMISSIONS verb.
*
* @see useAuthProvider
* @param {Object} authParams Any params you want to pass to the authProvider
*
* @returns {Function} getPermissions callback
*
Expand Down Expand Up @@ -37,31 +34,11 @@ import { AUTH_GET_PERMISSIONS } from './types';
* );
* }
*/
const useGetPermissions = (
authParams: any = defaultAuthParams
): GetPermissions => {
const useGetPermissions = (): GetPermissions => {
const authProvider = useAuthProvider();
/**
* We need the current location to pass to the authProvider for GET_PERMISSIONS.
*
* But if we used useSelector to read it from the store, the getPermissions function
* would be rebuilt each time the user changes location. Consequently, that
* would force a rerender of the enclosing component upon navigation.
*
* To avoid that, we don't subscribe to the store using useSelector;
* instead, we get a pointer to the store, and determine the location only
* after the getPermissions function was called.
*/

const store = useStore();

const getPermissions = useCallback(
(location?: Location) =>
authProvider(AUTH_GET_PERMISSIONS, {
location: location || store.getState().router.location,
...authParams,
}),
[authParams, authProvider, store]
(params: any = {}) => authProvider(AUTH_GET_PERMISSIONS, params),
[authProvider]
);

return authProvider ? getPermissions : getPermissionsWithoutProvider;
Expand All @@ -72,10 +49,10 @@ const getPermissionsWithoutProvider = () => Promise.resolve([]);
/**
* Ask the permissions to the authProvider using the AUTH_GET_PERMISSIONS verb
*
* @param {Location} location the current location from history (optional)
* @param {Object} params The parameters to pass to the authProvider
*
* @return {Promise} The authProvider response
*/
type GetPermissions = (location?: Location) => Promise<any>;
type GetPermissions = (params?: any) => Promise<any>;

export default useGetPermissions;
13 changes: 6 additions & 7 deletions packages/ra-core/src/auth/useLogin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { ReduxState } from '../types';
* and redirect to the previous authenticated page (or the home page) on success.
*
* @see useAuthProvider
* @param {Object} authParams Any params you want to pass to the authProvider
*
* @returns {Function} login callback
*
Expand All @@ -30,7 +29,7 @@ import { ReduxState } from '../types';
* return <button onClick={handleClick}>Login</button>;
* }
*/
const useLogin = (authParams: any = defaultAuthParams): Login => {
const useLogin = (): Login => {
const authProvider = useAuthProvider();
const currentLocation = useSelector(
(state: ReduxState) => state.router.location
Expand All @@ -40,20 +39,20 @@ const useLogin = (authParams: any = defaultAuthParams): Login => {
const dispatch = useDispatch();

const login = useCallback(
(params, pathName = authParams.afterLoginUrl) =>
authProvider(AUTH_LOGIN, { ...params, ...authParams }).then(ret => {
(params: any = {}, pathName = defaultAuthParams.afterLoginUrl) =>
authProvider(AUTH_LOGIN, params).then(ret => {
dispatch(push(nextPathName || pathName));
return ret;
}),
[authParams, authProvider, dispatch, nextPathName]
[authProvider, dispatch, nextPathName]
);

const loginWithoutProvider = useCallback(
(_, __) => {
dispatch(push(authParams.afterLoginUrl));
dispatch(push(defaultAuthParams.afterLoginUrl));
return Promise.resolve();
},
[authParams.afterLoginUrl, dispatch]
[dispatch]
);

return authProvider ? login : loginWithoutProvider;
Expand Down
44 changes: 21 additions & 23 deletions packages/ra-core/src/auth/useLogout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { clearState } from '../actions/clearActions';
* redirect to the login page, and clear the Redux state.
*
* @see useAuthProvider
* @param {Object} authParams Any params you want to pass to the authProvider
*
* @returns {Function} logout callback
*
Expand All @@ -25,7 +24,7 @@ import { clearState } from '../actions/clearActions';
* return <button onClick={handleClick}>Logout</button>;
* }
*/
const useLogout = (authParams: any = defaultAuthParams): Logout => {
const useLogout = (): Logout => {
const authProvider = useAuthProvider();
/**
* We need the current location to pass in the router state
Expand All @@ -44,32 +43,30 @@ const useLogout = (authParams: any = defaultAuthParams): Logout => {
const dispatch = useDispatch();

const logout = useCallback(
(redirectTo = authParams.loginUrl) =>
authProvider(AUTH_LOGOUT, authParams).then(
redirectToFromProvider => {
dispatch(clearState());
const currentLocation = store.getState().router.location;
dispatch(
push({
pathname: redirectToFromProvider || redirectTo,
state: {
nextPathname:
currentLocation && currentLocation.pathname,
},
})
);
return redirectToFromProvider;
}
),
[authParams, authProvider, store, dispatch]
(params = {}, redirectTo = defaultAuthParams.loginUrl) =>
authProvider(AUTH_LOGOUT, params).then(redirectToFromProvider => {
dispatch(clearState());
const currentLocation = store.getState().router.location;
dispatch(
push({
pathname: redirectToFromProvider || redirectTo,
state: {
nextPathname:
currentLocation && currentLocation.pathname,
},
})
);
return redirectToFromProvider;
}),
[authProvider, store, dispatch]
);

const logoutWithoutProvider = useCallback(
_ => {
const currentLocation = store.getState().router.location;
dispatch(
push({
pathname: authParams.loginUrl,
pathname: defaultAuthParams.loginUrl,
state: {
nextPathname:
currentLocation && currentLocation.pathname,
Expand All @@ -79,7 +76,7 @@ const useLogout = (authParams: any = defaultAuthParams): Logout => {
dispatch(clearState());
return Promise.resolve();
},
[authParams.loginUrl, store, dispatch]
[store, dispatch]
);

return authProvider ? logout : logoutWithoutProvider;
Expand All @@ -89,10 +86,11 @@ const useLogout = (authParams: any = defaultAuthParams): Logout => {
* Log the current user out by calling the authProvider AUTH_LOGOUT verb,
* and redirect them to the login screen.
*
* @param {Object} params The parameters to pass to the authProvider
* @param {string} redirectTo The path name to redirect the user to (optional, defaults to login)
*
* @return {Promise} The authProvider response
*/
type Logout = (redirectTo?: string) => Promise<any>;
type Logout = (params?: any, redirectTo?: string) => Promise<any>;

export default useLogout;
Loading

0 comments on commit 9730e6c

Please sign in to comment.