From 42d45598ffcf41ceef66570415e8d0fa531f1aed Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Thu, 23 May 2019 08:30:56 +0200 Subject: [PATCH 1/8] Delegate the redirection after logout to authProvider --- packages/ra-core/src/sideEffect/auth.ts | 168 ++++++++++++------------ 1 file changed, 82 insertions(+), 86 deletions(-) diff --git a/packages/ra-core/src/sideEffect/auth.ts b/packages/ra-core/src/sideEffect/auth.ts index 452caac01bc..db65e4bb35d 100644 --- a/packages/ra-core/src/sideEffect/auth.ts +++ b/packages/ra-core/src/sideEffect/auth.ts @@ -23,12 +23,14 @@ import { } from '../actions/authActions'; import { FETCH_ERROR } from '../actions/fetchActions'; import { AUTH_LOGIN, AUTH_CHECK, AUTH_ERROR, AUTH_LOGOUT } from '../auth'; + const nextPathnameSelector = state => { const locationState = state.router.location.state; return locationState && locationState.nextPathname; }; const currentPathnameSelector = state => state.router.location; + const getErrorMessage = (error, defaultMessage) => typeof error === 'string' ? error @@ -40,94 +42,88 @@ export default (authProvider?: AuthProvider) => { if (!authProvider) { return () => null; } - function* handleAuth(action) { - const { type, payload, error, meta } = action; - switch (type) { - case USER_LOGIN: { - try { - yield put({ type: USER_LOGIN_LOADING }); - const authPayload = yield call( - authProvider, - AUTH_LOGIN, - payload - ); - yield put({ - type: USER_LOGIN_SUCCESS, - payload: authPayload, - }); - const redirectTo = yield meta.pathName || - select(nextPathnameSelector); - yield put(push(redirectTo || '/')); - } catch (e) { - yield put({ - type: USER_LOGIN_FAILURE, - error: e, - meta: { auth: true }, - }); - const errorMessage = getErrorMessage( - e, - 'ra.auth.sign_in_error' - ); - yield put(showNotification(errorMessage, 'warning')); - } - break; - } - case USER_CHECK: { - try { - yield call(authProvider, AUTH_CHECK, payload); - } catch (error) { - yield call(authProvider, AUTH_LOGOUT); - yield put( - replace({ - pathname: (error && error.redirectTo) || '/login', - state: { nextPathname: meta.pathName }, - }) - ); - const errorMessage = getErrorMessage( - error, - 'ra.auth.auth_check_error' - ); - yield put(showNotification(errorMessage, 'warning')); - } - break; - } - case USER_LOGOUT: { - yield put( - push( - (action.payload && action.payload.redirectTo) || - '/login' - ) - ); - yield call(authProvider, AUTH_LOGOUT); - break; - } - case FETCH_ERROR: - try { - yield call(authProvider, AUTH_ERROR, error); - } catch (e) { - const nextPathname = yield select(currentPathnameSelector); - yield call(authProvider, AUTH_LOGOUT); - yield put( - push({ - pathname: '/login', - state: { nextPathname }, - }) - ); - yield put(hideNotification()); - yield put( - showNotification( - 'ra.notification.logged_out', - 'warning' - ) - ); - } - break; - } - } return function* watchAuthActions() { yield all([ - takeEvery(action => action.meta && action.meta.auth, handleAuth), - takeLatest(FETCH_ERROR, handleAuth), + takeEvery(USER_LOGIN, handleLogin(authProvider)), + takeEvery(USER_CHECK, handleCheck(authProvider)), + takeEvery(USER_LOGOUT, handleLogout(authProvider)), + takeLatest(FETCH_ERROR, handleFetchError(authProvider)), ]); }; }; + +export const handleLogin = (authProvider: AuthProvider) => + function*(action) { + const { payload, meta } = action; + try { + yield put({ type: USER_LOGIN_LOADING }); + const authPayload = yield call(authProvider, AUTH_LOGIN, payload); + yield put({ + type: USER_LOGIN_SUCCESS, + payload: authPayload, + }); + const redirectTo = yield meta.pathName || + select(nextPathnameSelector); + yield put(push(redirectTo || '/')); + } catch (e) { + yield put({ + type: USER_LOGIN_FAILURE, + error: e, + meta: { auth: true }, + }); + const errorMessage = getErrorMessage(e, 'ra.auth.sign_in_error'); + yield put(showNotification(errorMessage, 'warning')); + } + }; + +export const handleCheck = (authProvider: AuthProvider) => + function*(action) { + const { payload, meta } = action; + try { + yield call(authProvider, AUTH_CHECK, payload); + } catch (error) { + yield call(authProvider, AUTH_LOGOUT); + yield put( + replace({ + pathname: (error && error.redirectTo) || '/login', + state: { nextPathname: meta.pathName }, + }) + ); + const errorMessage = getErrorMessage( + error, + 'ra.auth.auth_check_error' + ); + yield put(showNotification(errorMessage, 'warning')); + } + }; + +export const handleLogout = (authProvider: AuthProvider) => + function*(action) { + const { payload } = action; + + const redirectTo = yield call(authProvider, AUTH_LOGOUT); + yield put( + push((payload && payload.redirectTo) || redirectTo || '/login') + ); + }; + +export const handleFetchError = (authProvider: AuthProvider) => + function*(action) { + const { error } = action; + try { + yield call(authProvider, AUTH_ERROR, error); + } catch (e) { + const nextPathname = yield select(currentPathnameSelector); + const redirectTo = yield call(authProvider, AUTH_LOGOUT); + yield put( + push({ + pathname: redirectTo || '/login', + state: { nextPathname }, + }) + ); + yield put(hideNotification()); + yield put( + showNotification('ra.notification.logged_out', 'warning') + ); + } + }; From fd2bf6a233cf626a2d1fb23435cbe10034ddfc58 Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Thu, 23 May 2019 17:10:10 +0200 Subject: [PATCH 2/8] Fix login --- packages/ra-core/src/sideEffect/auth.ts | 28 ++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/ra-core/src/sideEffect/auth.ts b/packages/ra-core/src/sideEffect/auth.ts index db65e4bb35d..42dd2055847 100644 --- a/packages/ra-core/src/sideEffect/auth.ts +++ b/packages/ra-core/src/sideEffect/auth.ts @@ -24,20 +24,6 @@ import { import { FETCH_ERROR } from '../actions/fetchActions'; import { AUTH_LOGIN, AUTH_CHECK, AUTH_ERROR, AUTH_LOGOUT } from '../auth'; -const nextPathnameSelector = state => { - const locationState = state.router.location.state; - return locationState && locationState.nextPathname; -}; - -const currentPathnameSelector = state => state.router.location; - -const getErrorMessage = (error, defaultMessage) => - typeof error === 'string' - ? error - : typeof error === 'undefined' || !error.message - ? defaultMessage - : error.message; - export default (authProvider?: AuthProvider) => { if (!authProvider) { return () => null; @@ -52,6 +38,20 @@ export default (authProvider?: AuthProvider) => { }; }; +const nextPathnameSelector = state => { + const locationState = state.router.location.state; + return locationState && locationState.nextPathname; +}; + +const currentPathnameSelector = state => state.router.location; + +const getErrorMessage = (error, defaultMessage) => + typeof error === 'string' + ? error + : typeof error === 'undefined' || !error.message + ? defaultMessage + : error.message; + export const handleLogin = (authProvider: AuthProvider) => function*(action) { const { payload, meta } = action; From 9f736423bd9a00562d59b8b388521490db1a766d Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Fri, 24 May 2019 10:58:34 +0200 Subject: [PATCH 3/8] Introduce clear storage action --- packages/ra-core/src/actions/clearActions.ts | 8 ++++++++ packages/ra-core/src/actions/index.ts | 1 + packages/ra-core/src/createAdminStore.ts | 4 ++-- packages/ra-core/src/sideEffect/auth.ts | 5 ++++- 4 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 packages/ra-core/src/actions/clearActions.ts diff --git a/packages/ra-core/src/actions/clearActions.ts b/packages/ra-core/src/actions/clearActions.ts new file mode 100644 index 00000000000..4928f59b9c5 --- /dev/null +++ b/packages/ra-core/src/actions/clearActions.ts @@ -0,0 +1,8 @@ +export const CLEAR_STATE = 'RA/CLEAR_STATE'; + +// The CLEAR_STATE action will completely reset the react-admin redux state to its initial value. +// This should only be called once the user has been redirected to a page which do not use the +// state such as the login page. +export const clearState = () => ({ + type: CLEAR_STATE, +}); diff --git a/packages/ra-core/src/actions/index.ts b/packages/ra-core/src/actions/index.ts index 7a13e36544e..deaf7db1873 100644 --- a/packages/ra-core/src/actions/index.ts +++ b/packages/ra-core/src/actions/index.ts @@ -1,5 +1,6 @@ export * from './accumulateActions'; export * from './authActions'; +export * from './clearActions'; export * from './dataActions'; export * from './fetchActions'; export * from './filterActions'; diff --git a/packages/ra-core/src/createAdminStore.ts b/packages/ra-core/src/createAdminStore.ts index 2403e200834..35a50f69339 100644 --- a/packages/ra-core/src/createAdminStore.ts +++ b/packages/ra-core/src/createAdminStore.ts @@ -5,11 +5,11 @@ import { all, fork } from 'redux-saga/effects'; import { History } from 'history'; import { AuthProvider, DataProvider, I18nProvider } from './types'; -import { USER_LOGOUT } from './actions/authActions'; import createAppReducer from './reducer'; import { adminSaga } from './sideEffect'; import { defaultI18nProvider } from './i18n'; import formMiddleware from './form/formMiddleware'; +import { CLEAR_STATE } from './actions/clearActions'; interface Window { __REDUX_DEVTOOLS_EXTENSION__?: () => () => void; @@ -45,7 +45,7 @@ export default ({ ); const resettableAppReducer = (state, action) => - appReducer(action.type !== USER_LOGOUT ? state : undefined, action); + appReducer(action.type !== CLEAR_STATE ? state : undefined, action); const saga = function* rootSaga() { yield all( [ diff --git a/packages/ra-core/src/sideEffect/auth.ts b/packages/ra-core/src/sideEffect/auth.ts index 42dd2055847..5c4a89a64c2 100644 --- a/packages/ra-core/src/sideEffect/auth.ts +++ b/packages/ra-core/src/sideEffect/auth.ts @@ -23,6 +23,7 @@ import { } from '../actions/authActions'; import { FETCH_ERROR } from '../actions/fetchActions'; import { AUTH_LOGIN, AUTH_CHECK, AUTH_ERROR, AUTH_LOGOUT } from '../auth'; +import { clearState } from '../actions/clearActions'; export default (authProvider?: AuthProvider) => { if (!authProvider) { @@ -94,17 +95,18 @@ export const handleCheck = (authProvider: AuthProvider) => 'ra.auth.auth_check_error' ); yield put(showNotification(errorMessage, 'warning')); + yield put(clearState()); } }; export const handleLogout = (authProvider: AuthProvider) => function*(action) { const { payload } = action; - const redirectTo = yield call(authProvider, AUTH_LOGOUT); yield put( push((payload && payload.redirectTo) || redirectTo || '/login') ); + yield put(clearState()); }; export const handleFetchError = (authProvider: AuthProvider) => @@ -121,6 +123,7 @@ export const handleFetchError = (authProvider: AuthProvider) => state: { nextPathname }, }) ); + yield put(clearState()); yield put(hideNotification()); yield put( showNotification('ra.notification.logged_out', 'warning') From 6174be050eed816fa9f7aaecc2ed9278cd5b7829 Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Mon, 22 Jul 2019 09:42:41 +0200 Subject: [PATCH 4/8] Use redirection url from auth_logout if provided --- packages/ra-core/src/sideEffect/auth.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/ra-core/src/sideEffect/auth.ts b/packages/ra-core/src/sideEffect/auth.ts index 5c4a89a64c2..9909bf295c9 100644 --- a/packages/ra-core/src/sideEffect/auth.ts +++ b/packages/ra-core/src/sideEffect/auth.ts @@ -83,10 +83,11 @@ export const handleCheck = (authProvider: AuthProvider) => try { yield call(authProvider, AUTH_CHECK, payload); } catch (error) { - yield call(authProvider, AUTH_LOGOUT); + const redirectTo = yield call(authProvider, AUTH_LOGOUT); yield put( replace({ - pathname: (error && error.redirectTo) || '/login', + pathname: + (error && error.redirectTo) || redirectTo || '/login', state: { nextPathname: meta.pathName }, }) ); From ce805d1b8a161a3a7d792c69796846bd1557b652 Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Mon, 22 Jul 2019 10:04:45 +0200 Subject: [PATCH 5/8] Fix clear state order and add comments --- packages/ra-core/src/sideEffect/auth.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/ra-core/src/sideEffect/auth.ts b/packages/ra-core/src/sideEffect/auth.ts index 9909bf295c9..2967f9c1390 100644 --- a/packages/ra-core/src/sideEffect/auth.ts +++ b/packages/ra-core/src/sideEffect/auth.ts @@ -91,12 +91,14 @@ export const handleCheck = (authProvider: AuthProvider) => state: { nextPathname: meta.pathName }, }) ); + // Clear the state before showing a notification as it would be dismissed immediately otherwise + yield put(clearState()); + const errorMessage = getErrorMessage( error, 'ra.auth.auth_check_error' ); yield put(showNotification(errorMessage, 'warning')); - yield put(clearState()); } }; @@ -124,7 +126,9 @@ export const handleFetchError = (authProvider: AuthProvider) => state: { nextPathname }, }) ); + // Clear the state before showing a notification as it would be dismissed immediately otherwise yield put(clearState()); + yield put(hideNotification()); yield put( showNotification('ra.notification.logged_out', 'warning') From 38fda171f2d1e6f4df9f271d546d1c406756d386 Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Mon, 22 Jul 2019 11:46:34 +0200 Subject: [PATCH 6/8] Added tests --- packages/ra-core/src/sideEffect/auth.spec.ts | 192 +++++++++++++++++++ 1 file changed, 192 insertions(+) create mode 100644 packages/ra-core/src/sideEffect/auth.spec.ts diff --git a/packages/ra-core/src/sideEffect/auth.spec.ts b/packages/ra-core/src/sideEffect/auth.spec.ts new file mode 100644 index 00000000000..396fdda2f09 --- /dev/null +++ b/packages/ra-core/src/sideEffect/auth.spec.ts @@ -0,0 +1,192 @@ +import { runSaga } from 'redux-saga'; +import { + handleLogin, + handleCheck, + handleLogout, + handleFetchError, +} from './auth'; +import { AUTH_LOGIN, AUTH_CHECK, AUTH_LOGOUT, AUTH_ERROR } from '../auth'; +import { + USER_LOGIN_LOADING, + USER_LOGIN_SUCCESS, + USER_LOGIN_FAILURE, +} from '../actions/authActions'; +import { push, replace } from 'connected-react-router'; +import { + showNotification, + hideNotification, +} from '../actions/notificationActions'; +import { clearState } from '../actions/clearActions'; + +const wait = (timeout = 100) => + new Promise(resolve => setTimeout(resolve, timeout)); + +describe('Auth saga', () => { + describe('Login saga', () => { + test('Handle successful login', async () => { + const dispatch = jest.fn(); + const authProvider = jest.fn().mockResolvedValue({ role: 'admin' }); + const action = { + payload: { + login: 'user', + password: 'password123', + }, + meta: { + pathName: '/posts', + }, + }; + + await runSaga({ dispatch }, handleLogin(authProvider), action); + expect(authProvider).toHaveBeenCalledWith(AUTH_LOGIN, { + login: 'user', + password: 'password123', + }); + expect(dispatch).toHaveBeenCalledWith({ type: USER_LOGIN_LOADING }); + expect(dispatch).toHaveBeenCalledWith({ + type: USER_LOGIN_SUCCESS, + payload: { role: 'admin' }, + }); + expect(dispatch).toHaveBeenCalledWith(push('/posts')); + }); + + test('Handle failed login', async () => { + const dispatch = jest.fn(); + const error = { message: 'Bazinga!' }; + const authProvider = jest.fn().mockRejectedValue(error); + const action = { + payload: { + login: 'user', + password: 'password123', + }, + meta: { + pathName: '/posts', + }, + }; + + await runSaga({ dispatch }, handleLogin(authProvider), action); + expect(authProvider).toHaveBeenCalledWith(AUTH_LOGIN, { + login: 'user', + password: 'password123', + }); + expect(dispatch).toHaveBeenCalledWith({ type: USER_LOGIN_LOADING }); + expect(dispatch).toHaveBeenCalledWith({ + type: USER_LOGIN_FAILURE, + error, + meta: { auth: true }, + }); + expect(dispatch).toHaveBeenCalledWith( + showNotification('Bazinga!', 'warning') + ); + }); + }); + describe('Check saga', () => { + test('Handle successful check', async () => { + const dispatch = jest.fn(); + const authProvider = jest.fn().mockResolvedValue({ role: 'admin' }); + const action = { + payload: { + resource: 'posts', + }, + meta: { + pathName: '/posts', + }, + }; + + await runSaga({ dispatch }, handleCheck(authProvider), action); + expect(authProvider).toHaveBeenCalledWith(AUTH_CHECK, { + resource: 'posts', + }); + expect(dispatch).not.toHaveBeenCalled(); + }); + + test('Handle failed check', async () => { + const dispatch = jest.fn(); + const error = { message: 'Bazinga!' }; + const authProvider = jest + .fn() + .mockRejectedValueOnce(error) + .mockResolvedValueOnce('/custom'); + + const action = { + payload: { + resource: 'posts', + }, + meta: { + pathName: '/posts', + }, + }; + + await runSaga({ dispatch }, handleCheck(authProvider), action); + expect(authProvider).toHaveBeenCalledWith(AUTH_CHECK, { + resource: 'posts', + }); + expect(authProvider).toHaveBeenCalledWith(AUTH_LOGOUT); + await wait(); + expect(dispatch).toHaveBeenCalledWith( + replace({ + pathname: '/custom', + state: { nextPathname: '/posts' }, + }) + ); + expect(dispatch).toHaveBeenCalledWith(clearState()); + expect(dispatch).toHaveBeenCalledWith( + showNotification('Bazinga!', 'warning') + ); + }); + }); + describe('Logout saga', () => { + test('Handle logout', async () => { + const dispatch = jest.fn(); + const authProvider = jest.fn().mockResolvedValue('/custom'); + const action = { + payload: { + resource: 'posts', + }, + meta: { + pathName: '/posts', + }, + }; + + await runSaga({ dispatch }, handleLogout(authProvider), action); + expect(authProvider).toHaveBeenCalledWith(AUTH_LOGOUT); + expect(dispatch).toHaveBeenCalledWith(push('/custom')); + expect(dispatch).toHaveBeenCalledWith(clearState()); + }); + }); + describe('Fetch error saga', () => { + test('Handle errors when authProvider throws', async () => { + const dispatch = jest.fn(); + const error = { message: 'Bazinga!' }; + const authProvider = jest + .fn() + .mockRejectedValueOnce(undefined) + .mockResolvedValueOnce('/custom'); + const action = { + error, + }; + + await runSaga( + { + dispatch, + getState: () => ({ router: { location: '/posts' } }), + }, + handleFetchError(authProvider), + action + ); + expect(authProvider).toHaveBeenCalledWith(AUTH_ERROR, error); + expect(authProvider).toHaveBeenCalledWith(AUTH_LOGOUT); + await wait(); + expect(dispatch).toHaveBeenCalledWith( + push({ + pathname: '/custom', + state: { nextPathname: '/posts' }, + }) + ); + expect(dispatch).toHaveBeenCalledWith(hideNotification()); + expect(dispatch).toHaveBeenCalledWith( + showNotification('ra.notification.logged_out', 'warning') + ); + expect(dispatch).toHaveBeenCalledWith(clearState()); + }); + }); +}); From 99465e0f5d0fcec5f49d1930a3f6863880b051cc Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Mon, 22 Jul 2019 11:54:06 +0200 Subject: [PATCH 7/8] Update documentation --- docs/Authentication.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/Authentication.md b/docs/Authentication.md index 5523486babc..ef020f55318 100644 --- a/docs/Authentication.md +++ b/docs/Authentication.md @@ -136,6 +136,8 @@ export default (type, params) => { The `authProvider` is also a good place to notify the authentication API that the user credentials are no longer valid after logout. +Note that the `authProvider` can return the url to which the user will be redirected once logged out. By default, this is the `/login` route. + ## Catching Authentication Errors On The API If the API requires authentication, and the user credentials are missing in the request or invalid, the API usually answers with an HTTP error code 401 or 403. @@ -167,6 +169,8 @@ export default (type, params) => { }; ``` +Note that react-admin will call the `authProvider` with the `AUTH_LOGOUT` type before redirecting when you reject the promise and will use the url which may have been return by the call to `AUTH_LOGOUT`. + ## Checking Credentials During Navigation Redirecting to the login page whenever a REST response uses a 401 status code is usually not enough, because react-admin keeps data on the client side, and could display stale data while contacting the server - even after the credentials are no longer valid. @@ -221,6 +225,8 @@ export default (type, params) => { }; ``` +Note that react-admin will call the `authProvider` with the `AUTH_LOGOUT` type before redirecting. If you specify the `redirectTo` here, it will override the url which may have been return by the call to `AUTH_LOGOUT`. + **Tip**: For the `AUTH_CHECK` call, the `params` argument contains the `resource` name, so you can implement different checks for different resources: ```jsx From 52ea7e7720e821a42457b26b54c70b1d58a256c7 Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Mon, 22 Jul 2019 16:37:25 +0200 Subject: [PATCH 8/8] Added a test for redirection to previous location --- packages/ra-core/src/sideEffect/auth.spec.ts | 36 ++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/packages/ra-core/src/sideEffect/auth.spec.ts b/packages/ra-core/src/sideEffect/auth.spec.ts index 396fdda2f09..01be3bcef01 100644 --- a/packages/ra-core/src/sideEffect/auth.spec.ts +++ b/packages/ra-core/src/sideEffect/auth.spec.ts @@ -49,6 +49,42 @@ describe('Auth saga', () => { expect(dispatch).toHaveBeenCalledWith(push('/posts')); }); + test('Handle successful login with redirection from previous state', async () => { + const dispatch = jest.fn(); + const authProvider = jest.fn().mockResolvedValue({ role: 'admin' }); + const action = { + payload: { + login: 'user', + password: 'password123', + }, + meta: {}, + }; + + await runSaga( + { + dispatch, + getState: () => ({ + router: { + location: { state: { nextPathname: '/posts/1' } }, + }, + }), + }, + handleLogin(authProvider), + action + ); + + expect(authProvider).toHaveBeenCalledWith(AUTH_LOGIN, { + login: 'user', + password: 'password123', + }); + expect(dispatch).toHaveBeenCalledWith({ type: USER_LOGIN_LOADING }); + expect(dispatch).toHaveBeenCalledWith({ + type: USER_LOGIN_SUCCESS, + payload: { role: 'admin' }, + }); + expect(dispatch).toHaveBeenCalledWith(push('/posts/1')); + }); + test('Handle failed login', async () => { const dispatch = jest.fn(); const error = { message: 'Bazinga!' };