Merge pull request #9786 from marmelab/fix-anonymous-access-custom-ro…

…utes Fix `<Admin requireAuth>` forbids access to custom routes with no layout
marmelab · Apr 22, 2024 · a2746b4 · a2746b4
2 parents 6b78433 + 1816aa1
commit a2746b4
Show file tree

Hide file tree

Showing 4 changed files with 136 additions and 19 deletions.
diff --git a/packages/ra-core/src/auth/LogoutOnMount.ts b/packages/ra-core/src/auth/LogoutOnMount.ts
@@ -0,0 +1,17 @@
+import { useEffect } from 'react';
+import useLogout from './useLogout';
+
+/**
+ * Log the user out and redirect them to login.
+ *
+ * To be used as a catch-all route for anonymous users in a secure app.
+ *
+ * @see CoreAdminRoutes
+ */
+export const LogoutOnMount = () => {
+    const logout = useLogout();
+    useEffect(() => {
+        logout();
+    }, [logout]);
+    return null;
+};
diff --git a/packages/ra-core/src/auth/index.ts b/packages/ra-core/src/auth/index.ts
@@ -11,6 +11,7 @@ import useLogoutIfAccessDenied from './useLogoutIfAccessDenied';
 import convertLegacyAuthProvider from './convertLegacyAuthProvider';
 
 export * from './Authenticated';
+export * from './LogoutOnMount';
 export * from './types';
 export * from './useAuthenticated';
 export * from './useCheckAuth';

diff --git a/packages/ra-core/src/core/CoreAdminRoutes.spec.tsx b/packages/ra-core/src/core/CoreAdminRoutes.spec.tsx
@@ -205,8 +205,7 @@ describe('<CoreAdminRoutes>', () => {
             jest.useRealTimers();
         });
     });
-
-    describe('requireAuth', () => {
+    describe('anonymous access', () => {
         it('should not wait for the authProvider.checkAuth to return before rendering by default', () => {
             const authProvider = {
                 login: jest.fn().mockResolvedValue(''),
@@ -239,6 +238,45 @@ describe('<CoreAdminRoutes>', () => {
             expect(screen.queryByText('PostList')).not.toBeNull();
             expect(screen.queryByText('Loading')).toBeNull();
         });
+        it('should render custom routes with no layout when the user is not authenticated ', async () => {
+            const authProvider = {
+                login: jest.fn().mockResolvedValue(''),
+                logout: jest.fn().mockResolvedValue(''),
+                checkAuth: () => Promise.reject('Not authenticated'),
+                checkError: jest.fn().mockResolvedValue(''),
+                getPermissions: jest.fn().mockResolvedValue(''),
+            };
+
+            const history = createMemoryHistory();
+            render(
+                <CoreAdminContext
+                    authProvider={authProvider}
+                    dataProvider={testDataProvider()}
+                    history={history}
+                >
+                    <CoreAdminRoutes
+                        layout={Layout}
+                        loading={Loading}
+                        catchAll={CatchAll}
+                    >
+                        <CustomRoutes noLayout>
+                            <Route path="/custom" element={<i>Custom</i>} />
+                            <Route path="/login" element={<i>Login</i>} />
+                        </CustomRoutes>
+                        <Resource name="posts" list={() => <i>PostList</i>} />
+                    </CoreAdminRoutes>
+                </CoreAdminContext>
+            );
+            expect(screen.queryByText('PostList')).not.toBeNull();
+            expect(screen.queryByText('Loading')).toBeNull();
+            history.push('/custom');
+            await new Promise(resolve => setTimeout(resolve, 1100));
+            await waitFor(() =>
+                expect(screen.queryByText('Custom')).not.toBeNull()
+            );
+        });
+    });
+    describe('requireAuth', () => {
         it('should wait for the authProvider.checkAuth to return before rendering when requireAuth is true', async () => {
             let resolve;
             const authProvider = {
@@ -277,7 +315,7 @@ describe('<CoreAdminRoutes>', () => {
                 expect(screen.queryByText('PostList')).not.toBeNull()
             );
         });
-        it('should redirect to login when requireAuth is true and authProvider.checkAuth() rejects', async () => {
+        it('should redirect anonymous users to login when requireAuth is true and user accesses a resource page', async () => {
             let reject;
             const authProvider = {
                 login: jest.fn().mockResolvedValue(''),
@@ -315,16 +353,20 @@ describe('<CoreAdminRoutes>', () => {
                 expect(screen.queryByText('Login')).not.toBeNull()
             );
         });
-        it('should render custom routes when the user is not authenticated and requireAuth is false', async () => {
+        it('should redirect anonymous users to login when requireAuth is true and user accesses a custom route', async () => {
+            let reject;
             const authProvider = {
                 login: jest.fn().mockResolvedValue(''),
                 logout: jest.fn().mockResolvedValue(''),
-                checkAuth: () => Promise.reject('Not authenticated'),
+                checkAuth: (): Promise<void> =>
+                    new Promise((res, rej) => (reject = rej)),
                 checkError: jest.fn().mockResolvedValue(''),
                 getPermissions: jest.fn().mockResolvedValue(''),
             };
 
-            const history = createMemoryHistory();
+            const history = createMemoryHistory({
+                initialEntries: ['/custom'],
+            });
             render(
                 <CoreAdminContext
                     authProvider={authProvider}
@@ -335,22 +377,62 @@ describe('<CoreAdminRoutes>', () => {
                         layout={Layout}
                         loading={Loading}
                         catchAll={CatchAll}
+                        requireAuth
                     >
-                        <CustomRoutes noLayout>
+                        <CustomRoutes>
                             <Route path="/custom" element={<i>Custom</i>} />
+                        </CustomRoutes>
+                        <CustomRoutes noLayout>
                             <Route path="/login" element={<i>Login</i>} />
                         </CustomRoutes>
-                        <Resource name="posts" list={() => <i>PostList</i>} />
                     </CoreAdminRoutes>
                 </CoreAdminContext>
             );
-            expect(screen.queryByText('PostList')).not.toBeNull();
-            expect(screen.queryByText('Loading')).toBeNull();
-            history.push('/custom');
-            await new Promise(resolve => setTimeout(resolve, 1100));
+            expect(screen.queryByText('Custom')).toBeNull();
+            expect(screen.queryByText('Loading')).not.toBeNull();
+            reject();
             await waitFor(() =>
-                expect(screen.queryByText('Custom')).not.toBeNull()
+                expect(screen.queryByText('Login')).not.toBeNull()
             );
         });
+        it('should render custom routes with no layout even for anonymous users', async () => {
+            let reject;
+            const authProvider = {
+                login: jest.fn().mockResolvedValue(''),
+                logout: jest.fn().mockResolvedValue(''),
+                checkAuth: (): Promise<void> =>
+                    new Promise((res, rej) => (reject = rej)),
+                checkError: jest.fn().mockResolvedValue(''),
+                getPermissions: jest.fn().mockResolvedValue(''),
+            };
+
+            const history = createMemoryHistory({
+                initialEntries: ['/custom'],
+            });
+            render(
+                <CoreAdminContext
+                    authProvider={authProvider}
+                    dataProvider={testDataProvider()}
+                    history={history}
+                >
+                    <CoreAdminRoutes
+                        layout={Layout}
+                        loading={Loading}
+                        catchAll={CatchAll}
+                        requireAuth
+                    >
+                        <CustomRoutes noLayout>
+                            <Route path="/custom" element={<i>Custom</i>} />
+                            <Route path="/login" element={<i>Login</i>} />
+                        </CustomRoutes>
+                    </CoreAdminRoutes>
+                </CoreAdminContext>
+            );
+            // the custom page should show during loading and after the checkAuth promise is rejected
+            expect(screen.queryByText('Custom')).not.toBeNull();
+            expect(screen.queryByText('Loading')).toBeNull();
+            reject();
+            expect(screen.queryByText('Custom')).not.toBeNull();
+        });
     });
 });
diff --git a/packages/ra-core/src/core/CoreAdminRoutes.tsx b/packages/ra-core/src/core/CoreAdminRoutes.tsx
@@ -2,7 +2,7 @@ import * as React from 'react';
 import { useState, useEffect, Children, ComponentType } from 'react';
 import { Navigate, Route, Routes } from 'react-router-dom';
 
-import { WithPermissions, useCheckAuth } from '../auth';
+import { WithPermissions, useCheckAuth, LogoutOnMount } from '../auth';
 import { useScrollToTop, useCreatePath } from '../routing';
 import {
     AdminChildren,
@@ -35,24 +35,32 @@ export const CoreAdminRoutes = (props: CoreAdminRoutesProps) => {
         title,
     } = props;
 
-    const [canRender, setCanRender] = useState(!requireAuth);
+    const [onlyAnonymousRoutes, setOnlyAnonymousRoutes] = useState(requireAuth);
+    const [checkAuthLoading, setCheckAuthLoading] = useState(requireAuth);
     const checkAuth = useCheckAuth();
 
     useEffect(() => {
         if (requireAuth) {
-            checkAuth()
+            // do not log the user out on failure to allow access to custom routes with no layout
+            // for other routes, the LogoutOnMount component will log the user out
+            checkAuth(undefined, false)
                 .then(() => {
-                    setCanRender(true);
+                    setOnlyAnonymousRoutes(false);
                 })
-                .catch(() => {});
+                .catch(() => {})
+                .finally(() => {
+                    setCheckAuthLoading(false);
+                });
         }
     }, [checkAuth, requireAuth]);
 
     if (status === 'empty') {
         return <Ready />;
     }
 
-    if (status === 'loading' || !canRender) {
+    // Note: custom routes with no layout are always rendered, regardless of the auth status
+
+    if (status === 'loading' || checkAuthLoading) {
         return (
             <Routes>
                 {customRoutesWithoutLayout}
@@ -68,6 +76,15 @@ export const CoreAdminRoutes = (props: CoreAdminRoutesProps) => {
         );
     }
 
+    if (onlyAnonymousRoutes) {
+        return (
+            <Routes>
+                {customRoutesWithoutLayout}
+                <Route path="*" element={<LogoutOnMount />} />
+            </Routes>
+        );
+    }
+
     return (
         <Routes>
             {/*