Merge pull request #4645 from marmelab/avoid-multiple-notifications-o…

…n-expired-auth Avoid Multiple Notifications on Expired Session
marmelab · Apr 7, 2020 · 6ef44eb · 6ef44eb
2 parents 487938c + 32a725e
commit 6ef44eb
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 18 deletions.
diff --git a/packages/ra-core/src/auth/useLogoutIfAccessDenied.spec.tsx b/packages/ra-core/src/auth/useLogoutIfAccessDenied.spec.tsx
@@ -25,10 +25,16 @@ const TestComponent = ({ error }: { error?: any }) => {
     return <div>{loggedOut ? '' : 'logged in'}</div>;
 };
 
+let loggedIn = true;
+
 const authProvider: AuthProvider = {
     login: () => Promise.reject('bad method'),
-    logout: () => Promise.reject('bad method'),
-    checkAuth: () => Promise.reject('bad method'),
+    logout: () => {
+        loggedIn = false;
+        return Promise.resolve();
+    },
+    checkAuth: () =>
+        loggedIn ? Promise.resolve() : Promise.reject('bad method'),
     checkError: params => {
         if (params instanceof Error && params.message === 'denied') {
             return Promise.reject(new Error('logout'));
@@ -39,7 +45,11 @@ const authProvider: AuthProvider = {
 };
 
 describe('useLogoutIfAccessDenied', () => {
-    afterEach(cleanup);
+    afterEach(() => {
+        logout.mockClear();
+        notify.mockClear();
+        cleanup();
+    });
 
     it('should not logout if passed no error', async () => {
         const { queryByText } = render(
@@ -76,4 +86,17 @@ describe('useLogoutIfAccessDenied', () => {
         expect(notify).toHaveBeenCalledTimes(1);
         expect(queryByText('logged in')).toBeNull();
     });
+
+    it('should not send multiple notifications if already logged out', async () => {
+        const { queryByText } = render(
+            <AuthContext.Provider value={authProvider}>
+                <TestComponent error={new Error('denied')} />
+                <TestComponent error={new Error('denied')} />
+            </AuthContext.Provider>
+        );
+        await wait();
+        expect(logout).toHaveBeenCalledTimes(1);
+        expect(notify).toHaveBeenCalledTimes(1);
+        expect(queryByText('logged in')).toBeNull();
+    });
 });
diff --git a/packages/ra-core/src/auth/useLogoutIfAccessDenied.ts b/packages/ra-core/src/auth/useLogoutIfAccessDenied.ts
@@ -4,6 +4,8 @@ import useAuthProvider from './useAuthProvider';
 import useLogout from './useLogout';
 import { useNotify } from '../sideEffect';
 
+let authCheckPromise;
+
 /**
  * Returns a callback used to call the authProvider.checkError() method
  * and an error from the dataProvider. If the authProvider rejects the call,
@@ -40,21 +42,32 @@ const useLogoutIfAccessDenied = (): LogoutIfAccessDenied => {
     const logout = useLogout();
     const notify = useNotify();
     const logoutIfAccessDenied = useCallback(
-        (error?: any) =>
-            authProvider
-                .checkError(error)
-                .then(() => false)
-                .catch(e => {
-                    const redirectTo =
-                        e && e.redirectTo
-                            ? e.redirectTo
-                            : error && error.redirectTo
-                            ? error.redirectto
-                            : undefined;
-                    logout({}, redirectTo);
-                    notify('ra.notification.logged_out', 'warning');
-                    return true;
-                }),
+        (error?: any) => {
+            // Sometimes, a component might trigger multiple simultaneous
+            // dataProvider calls which all fail and call this function.
+            // To avoid having multiple notifications, we first verify if
+            // a checkError promise is already ongoing
+            if (!authCheckPromise) {
+                authCheckPromise = authProvider
+                    .checkError(error)
+                    .then(() => false)
+                    .catch(async e => {
+                        const redirectTo =
+                            e && e.redirectTo
+                                ? e.redirectTo
+                                : error && error.redirectTo
+                                ? error.redirectto
+                                : undefined;
+                        logout({}, redirectTo);
+                        notify('ra.notification.logged_out', 'warning');
+                        return true;
+                    })
+                    .finally(() => {
+                        authCheckPromise = undefined;
+                    });
+            }
+            return authCheckPromise;
+        },
         [authProvider, logout, notify]
     );
     return authProvider