Merge pull request #10383 from marmelab/fix-access-control-basename-handling

fzaninotto · web-flow · commit bf2c1451732e · 2024-11-26T15:46:36.000+01:00
Fix access control basename handling
diff --git a/packages/ra-core/src/auth/CanAccess.spec.tsx b/packages/ra-core/src/auth/CanAccess.spec.tsx
@@ -1,5 +1,6 @@
 import * as React from 'react';
-import { render, screen } from '@testing-library/react';
+import { render, screen, waitFor } from '@testing-library/react';
+import { Location } from 'react-router';
 import {
     Basic,
     CustomLoading,
@@ -89,6 +90,63 @@ describe('<CanAccess>', () => {
         resolveCheckAuth(false);
         await screen.findByText('Not allowed');
     });
+    it('redirects to the /authentication-error route by default in case of error', async () => {
+        let rejectCheckAuth;
+        let location: Location;
+        const authProvider: AuthProvider = {
+            login: () => Promise.reject('bad method'),
+            logout: () => Promise.reject('bad method'),
+            checkAuth: () => Promise.reject('bad method'),
+            checkError: () => Promise.reject('bad method'),
+            getPermissions: () => Promise.reject('bad method'),
+            canAccess: () =>
+                new Promise((_, reject) => {
+                    rejectCheckAuth = reject;
+                }),
+        };
+        const { container } = render(
+            <Basic
+                authProvider={authProvider}
+                locationCallback={l => {
+                    location = l;
+                }}
+            />
+        );
+        expect(container.textContent).toEqual('');
+        rejectCheckAuth(new Error('failed'));
+        await waitFor(() =>
+            expect(location.pathname).toEqual('/authentication-error')
+        );
+    });
+    it('redirects to the /authentication-error route by default in case of error in an Admin with a basename', async () => {
+        let rejectCheckAuth;
+        let location: Location;
+        const authProvider: AuthProvider = {
+            login: () => Promise.reject('bad method'),
+            logout: () => Promise.reject('bad method'),
+            checkAuth: () => Promise.reject('bad method'),
+            checkError: () => Promise.reject('bad method'),
+            getPermissions: () => Promise.reject('bad method'),
+            canAccess: () =>
+                new Promise((_, reject) => {
+                    rejectCheckAuth = reject;
+                }),
+        };
+        const { container } = render(
+            <Basic
+                authProvider={authProvider}
+                basename="/admin"
+                locationCallback={l => {
+                    location = l;
+                }}
+            />
+        );
+        expect(container.textContent).toEqual('');
+        rejectCheckAuth(new Error('failed'));
+        await waitFor(() =>
+            expect(location.pathname).toEqual('/admin/authentication-error')
+        );
+    });
     it('shows the protected content when users are authorized', async () => {
         let resolveCheckAuth;
         const authProvider: AuthProvider = {
diff --git a/packages/ra-core/src/auth/CanAccess.stories.tsx b/packages/ra-core/src/auth/CanAccess.stories.tsx
@@ -1,7 +1,10 @@
 import * as React from 'react';
+import { Location } from 'react-router';
+import { QueryClient } from '@tanstack/react-query';
 import { AuthProvider } from '../types';
 import { CoreAdminContext } from '../core';
 import { CanAccess } from './CanAccess';
+import { TestMemoryRouter } from '..';
 
 export default {
     title: 'ra-core/auth/CanAccess',
@@ -19,64 +22,90 @@ const defaultAuthProvider: AuthProvider = {
 
 export const Basic = ({
     authProvider = defaultAuthProvider,
+    basename,
+    locationCallback,
 }: {
     authProvider?: AuthProvider;
+    basename?: string;
+    locationCallback?: (location: Location) => void;
 }) => (
-    <CoreAdminContext authProvider={authProvider}>
-        <CanAccess action="read" resource="test">
-            protected content
-        </CanAccess>
-    </CoreAdminContext>
+    <TestMemoryRouter locationCallback={locationCallback}>
+        <CoreAdminContext
+            authProvider={authProvider}
+            basename={basename}
+            queryClient={
+                new QueryClient({
+                    defaultOptions: {
+                        queries: {
+                            retry: false,
+                        },
+                    },
+                })
+            }
+        >
+            <CanAccess action="read" resource="test">
+                protected content
+            </CanAccess>
+        </CoreAdminContext>
+    </TestMemoryRouter>
 );
 
 export const AccessDenied = ({
     authProvider = defaultAuthProvider,
 }: {
     authProvider?: AuthProvider;
 }) => (
-    <CoreAdminContext authProvider={authProvider}>
-        <CanAccess action="show" resource="test">
-            protected content
-        </CanAccess>
-    </CoreAdminContext>
+    <TestMemoryRouter>
+        <CoreAdminContext authProvider={authProvider}>
+            <CanAccess action="show" resource="test">
+                protected content
+            </CanAccess>
+        </CoreAdminContext>
+    </TestMemoryRouter>
 );
 
 export const CustomLoading = ({
     authProvider = defaultAuthProvider,
 }: {
     authProvider?: AuthProvider;
 }) => (
-    <CoreAdminContext authProvider={authProvider}>
-        <CanAccess
-            action="read"
-            resource="test"
-            loading={<div>Please wait...</div>}
-        >
-            protected content
-        </CanAccess>
-    </CoreAdminContext>
+    <TestMemoryRouter>
+        <CoreAdminContext authProvider={authProvider}>
+            <CanAccess
+                action="read"
+                resource="test"
+                loading={<div>Please wait...</div>}
+            >
+                protected content
+            </CanAccess>
+        </CoreAdminContext>
+    </TestMemoryRouter>
 );
 
 export const CustomAccessDenied = ({
     authProvider = defaultAuthProvider,
 }: {
     authProvider?: AuthProvider;
 }) => (
-    <CoreAdminContext authProvider={authProvider}>
-        <CanAccess
-            action="show"
-            resource="test"
-            accessDenied={<div>Not allowed</div>}
-        >
-            protected content
-        </CanAccess>
-    </CoreAdminContext>
+    <TestMemoryRouter>
+        <CoreAdminContext authProvider={authProvider}>
+            <CanAccess
+                action="show"
+                resource="test"
+                accessDenied={<div>Not allowed</div>}
+            >
+                protected content
+            </CanAccess>
+        </CoreAdminContext>
+    </TestMemoryRouter>
 );
 
 export const NoAuthProvider = () => (
-    <CoreAdminContext authProvider={undefined}>
-        <CanAccess action="read" resource="test">
-            protected content
-        </CanAccess>
-    </CoreAdminContext>
+    <TestMemoryRouter>
+        <CoreAdminContext authProvider={undefined}>
+            <CanAccess action="read" resource="test">
+                protected content
+            </CanAccess>
+        </CoreAdminContext>
+    </TestMemoryRouter>
 );
diff --git a/packages/ra-core/src/auth/CanAccess.tsx b/packages/ra-core/src/auth/CanAccess.tsx
@@ -1,7 +1,8 @@
 import * as React from 'react';
+import { Navigate } from 'react-router';
 import { useCanAccess, UseCanAccessOptions } from './useCanAccess';
 import { RaRecord } from '../types';
-import { Navigate } from 'react-router';
+import { useBasename } from '../routing';
 
 /**
  * A component that only displays its children after checking whether users are authorized to access the provided resource and action.
@@ -50,4 +51,9 @@ export interface CanAccessProps<
     error?: React.ReactNode;
 }
 
-const DEFAULT_ERROR = <Navigate to="/authentication-error" />;
+const CanAccessDefaultError = () => {
+    const basename = useBasename();
+    return <Navigate to={`${basename}/authentication-error`} />;
+};
+
+const DEFAULT_ERROR = <CanAccessDefaultError />;
diff --git a/packages/ra-core/src/auth/useRequireAccess.spec.tsx b/packages/ra-core/src/auth/useRequireAccess.spec.tsx
@@ -1,8 +1,8 @@
 import * as React from 'react';
 import expect from 'expect';
 import { waitFor, render, screen } from '@testing-library/react';
-
 import { QueryClient } from '@tanstack/react-query';
+import { Location } from 'react-router';
 import { Basic } from './useRequireAccess.stories';
 
 describe('useRequireAccess', () => {
@@ -77,6 +77,55 @@ describe('useRequireAccess', () => {
         await screen.findByText('Authentication Error');
     });
 
+    it('should redirect to /access-denied when users do not have access in an Admin with basename', async () => {
+        let location: Location;
+        const authProvider = {
+            login: () => Promise.reject('bad method'),
+            logout: () => Promise.reject('bad method'),
+            checkAuth: () => Promise.reject('bad method'),
+            checkError: () => Promise.reject('bad method'),
+            getPermissions: () => Promise.reject('bad method'),
+            canAccess: () => Promise.resolve(false),
+        };
+        render(
+            <Basic
+                authProvider={authProvider}
+                basename="/admin"
+                locationCallback={l => {
+                    location = l;
+                }}
+            />
+        );
+
+        await waitFor(() => {
+            expect(location.pathname).toEqual('/admin/access-denied');
+        });
+    });
+
+    it('should redirect to /authentication-error when auth.canAccess call fails in an Admin with basename', async () => {
+        let location: Location;
+        const authProvider = {
+            login: () => Promise.reject('bad method'),
+            logout: () => Promise.reject('bad method'),
+            checkAuth: () => Promise.reject('bad method'),
+            getPermissions: () => Promise.reject('bad method'),
+            checkError: () => Promise.reject('bad method'),
+            canAccess: () => Promise.reject('not good'),
+        };
+        render(
+            <Basic
+                authProvider={authProvider}
+                basename="/admin"
+                locationCallback={l => {
+                    location = l;
+                }}
+            />
+        );
+        await waitFor(() => {
+            expect(location.pathname).toEqual('/admin/authentication-error');
+        });
+    });
+
     it('should abort the request if the query is canceled', async () => {
         const abort = jest.fn();
         const authProvider = {
diff --git a/packages/ra-core/src/auth/useRequireAccess.stories.tsx b/packages/ra-core/src/auth/useRequireAccess.stories.tsx
@@ -1,6 +1,6 @@
 import * as React from 'react';
 import { QueryClient } from '@tanstack/react-query';
-import { Route, Routes } from 'react-router';
+import { Location, Route, Routes } from 'react-router';
 import { AuthProvider } from '../types';
 import { CoreAdminContext } from '../core';
 import { useRequireAccess, UseRequireAccessResult } from './useRequireAccess';
@@ -50,15 +50,20 @@ const defaultAuthProvider: AuthProvider = {
 
 export const Basic = ({
     authProvider = defaultAuthProvider,
+    basename,
+    locationCallback,
     queryClient,
 }: {
     authProvider?: AuthProvider | null;
+    basename?: string;
+    locationCallback?: (l: Location) => void;
     queryClient?: QueryClient;
 }) => (
-    <TestMemoryRouter>
+    <TestMemoryRouter locationCallback={locationCallback}>
         <CoreAdminContext
             authProvider={authProvider != null ? authProvider : undefined}
             queryClient={queryClient}
+            basename={basename}
         >
             <Routes>
                 <Route
diff --git a/packages/ra-core/src/auth/useRequireAccess.tsx b/packages/ra-core/src/auth/useRequireAccess.tsx
@@ -1,11 +1,12 @@
 import { useEffect } from 'react';
+import { useNavigate } from 'react-router';
 import { RaRecord } from '../types';
 import {
     useCanAccess,
     UseCanAccessOptions,
     UseCanAccessResult,
 } from './useCanAccess';
-import { useNavigate } from 'react-router';
+import { useBasename } from '../routing';
 
 /**
  * A hook that calls the authProvider.canAccess() method for a provided resource and action (and optionally a record).
@@ -50,20 +51,21 @@ export const useRequireAccess = <
 ) => {
     const { canAccess, data, error, ...rest } = useCanAccess(params);
     const navigate = useNavigate();
+    const basename = useBasename();
 
     useEffect(() => {
         if (rest.isPending) return;
 
         if (canAccess === false) {
-            navigate('/access-denied');
+            navigate(`${basename}/access-denied`);
         }
-    }, [canAccess, navigate, rest.isPending]);
+    }, [basename, canAccess, navigate, rest.isPending]);
 
     useEffect(() => {
         if (error) {
-            navigate('/authentication-error');
+            navigate(`${basename}/authentication-error`);
         }
-    }, [navigate, error]);
+    }, [basename, navigate, error]);
 
     return rest;
 };
diff --git a/packages/ra-ui-materialui/src/layout/ResourceMenuItem.spec.tsx b/packages/ra-ui-materialui/src/layout/ResourceMenuItem.spec.tsx
@@ -19,6 +19,25 @@ describe('ResourceMenuItem', () => {
         await screen.findByText('resources.posts.name');
         expect(screen.queryByText('resources.users.name')).toBeNull();
     });
+    it('should not render when authProvider.canAccess throws', async () => {
+        render(
+            <AccessControl
+                authProvider={
+                    {
+                        checkAuth: () => Promise.resolve(),
+                        canAccess: ({ resource }) =>
+                            resource === 'posts'
+                                ? Promise.resolve(true)
+                                : Promise.reject(
+                                      new Error('access control error')
+                                  ),
+                    } as any
+                }
+            />
+        );
+        await screen.findByText('resources.posts.name');
+        expect(screen.queryByText('resources.users.name')).toBeNull();
+    });
     it('should not render when authProvider.canAccess returns false with a Function as <Admin> child', async () => {
         render(<AccessControlInsideAdminChildFunction />);
         await screen.findByText('resources.posts.name');
diff --git a/packages/ra-ui-materialui/src/layout/ResourceMenuItem.stories.tsx b/packages/ra-ui-materialui/src/layout/ResourceMenuItem.stories.tsx
diff --git a/packages/ra-ui-materialui/src/layout/ResourceMenuItem.tsx b/packages/ra-ui-materialui/src/layout/ResourceMenuItem.tsx
diff --git a/packages/react-admin/src/Admin.stories.tsx b/packages/react-admin/src/Admin.stories.tsx
