Skip to content

Commit 9a46596

Browse files
djhidjhi
authored
Merge pull request #10691 from marmelab/fix-disableAuthentication-canAccess
Fix `canAccess` is called even when `disableAuthentication` is `true`
2 parents 863f7d8 + 0c017e9 commit 9a46596

16 files changed

+407
-78
lines changed

‎packages/ra-core/src/controller/create/useCreateController.security.stories.tsx‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
CreateControllerProps,
1414
useCreateController,
1515
} from './useCreateController';
16+
import { useAuthState } from '../../auth';
1617

1718
export default {
1819
title: 'ra-core/controller/useCreateController',
@@ -39,6 +40,16 @@ const defaultDataProvider = fakeDataProvider(
3940
process.env.NODE_ENV === 'development'
4041
);
4142

43+
const PostList = () => {
44+
useAuthState();
45+
return (
46+
<div style={styles.mainContainer}>
47+
<div>List view</div>
48+
<Link to="/posts/create">Create</Link>
49+
</div>
50+
);
51+
};
52+
4253
const CreatePost = (props: Partial<CreateControllerProps>) => {
4354
const params = useCreateController({
4455
resource: 'posts',
@@ -47,6 +58,7 @@ const CreatePost = (props: Partial<CreateControllerProps>) => {
4758
return (
4859
<div style={styles.mainContainer}>
4960
{params.isPending ? <p>Loading...</p> : <div>Create view</div>}
61+
<Link to="/posts">List</Link>
5062
</div>
5163
);
5264
};
@@ -93,16 +105,33 @@ export const DisableAuthentication = ({
93105
dataProvider={dataProvider}
94106
authProvider={authProvider}
95107
>
96-
<CoreAdminUI>
108+
<CoreAdminUI accessDenied={AccessDenied}>
97109
<Resource
98110
name="posts"
111+
list={<PostList />}
99112
create={<CreatePost disableAuthentication />}
100113
/>
101114
</CoreAdminUI>
102115
</CoreAdminContext>
103116
</TestMemoryRouter>
104117
);
105118
};
119+
DisableAuthentication.args = {
120+
authProvider: undefined,
121+
};
122+
DisableAuthentication.argTypes = {
123+
authProvider: {
124+
options: ['default', 'canAccess'],
125+
mapping: {
126+
default: undefined,
127+
canAccess: {
128+
...defaultAuthProvider,
129+
canAccess: () => Promise.resolve(false),
130+
},
131+
},
132+
control: { type: 'inline-radio' },
133+
},
134+
};
106135

107136
export const CanAccess = ({
108137
authProviderDelay = 300,

‎packages/ra-core/src/controller/create/useCreateController.spec.tsx‎

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import React from 'react';
1010
import { Route, Routes } from 'react-router-dom';
1111

1212
import {
13+
AuthProvider,
1314
CreateContextProvider,
1415
DataProvider,
1516
Form,
@@ -28,7 +29,10 @@ import {
2829
import { CreateController } from './CreateController';
2930

3031
import { TestMemoryRouter } from '../../routing';
31-
import { CanAccess } from './useCreateController.security.stories';
32+
import {
33+
CanAccess,
34+
DisableAuthentication,
35+
} from './useCreateController.security.stories';
3236

3337
describe('useCreateController', () => {
3438
const defaultProps = {
@@ -70,8 +74,8 @@ describe('useCreateController', () => {
7074
let saveCallback;
7175
const dataProvider = testDataProvider({
7276
getOne: () => Promise.resolve({ data: { id: 12 } } as any),
73-
// @ts-ignore
7477
create: (_, { data }) =>
78+
// @ts-ignore
7579
Promise.resolve({ data: { id: 123, ...data } }),
7680
});
7781

@@ -224,8 +228,8 @@ describe('useCreateController', () => {
224228
let saveCallback;
225229
const dataProvider = testDataProvider({
226230
getOne: () => Promise.resolve({ data: { id: 12 } } as any),
227-
// @ts-ignore
228231
create: (_, { data }) =>
232+
// @ts-ignore
229233
Promise.resolve({ data: { id: 123, ...data } }),
230234
});
231235
const onSuccess = jest.fn();
@@ -262,8 +266,8 @@ describe('useCreateController', () => {
262266
let saveCallback;
263267
const dataProvider = testDataProvider({
264268
getOne: () => Promise.resolve({ data: { id: 12 } } as any),
265-
// @ts-ignore
266269
create: (_, { data }) =>
270+
// @ts-ignore
267271
Promise.resolve({ data: { id: 123, ...data } }),
268272
});
269273
const onSuccess = jest.fn();
@@ -692,5 +696,39 @@ describe('useCreateController', () => {
692696
await screen.findByText('Loading...');
693697
await screen.findByText('Create view');
694698
});
699+
700+
it('should not call checkAuth nor canAccess when disableAuthentication is true', async () => {
701+
const authProvider: AuthProvider = {
702+
checkAuth: jest.fn().mockResolvedValue(true),
703+
login: () => Promise.resolve(),
704+
logout: () => Promise.resolve(),
705+
checkError: () => Promise.resolve(),
706+
getPermissions: () => Promise.resolve(),
707+
canAccess: jest.fn().mockResolvedValue(false),
708+
};
709+
render(<DisableAuthentication authProvider={authProvider} />);
710+
await screen.findByText('Create view');
711+
expect(authProvider.checkAuth).not.toHaveBeenCalled();
712+
expect(authProvider.canAccess).not.toHaveBeenCalled();
713+
});
714+
715+
it('should not call checkAuth nor canAccess when disableAuthentication is true even if useAuthState was called before', async () => {
716+
const authProvider: AuthProvider = {
717+
checkAuth: jest.fn().mockResolvedValue(true),
718+
login: () => Promise.resolve(),
719+
logout: () => Promise.resolve(),
720+
checkError: () => Promise.resolve(),
721+
getPermissions: () => Promise.resolve(),
722+
canAccess: jest.fn().mockResolvedValue(false),
723+
};
724+
render(<DisableAuthentication authProvider={authProvider} />);
725+
await screen.findByText('Create view');
726+
fireEvent.click(await screen.findByText('List'));
727+
await screen.findByText('List view');
728+
fireEvent.click(await screen.findByText('Create'));
729+
await screen.findByText('Create view');
730+
expect(authProvider.checkAuth).toHaveBeenCalledTimes(1);
731+
expect(authProvider.canAccess).not.toHaveBeenCalled();
732+
});
695733
});
696734
});

‎packages/ra-core/src/controller/create/useCreateController.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,7 @@ export const useCreateController = <
7171
const { isPending: isPendingCanAccess } = useRequireAccess<RecordType>({
7272
action: 'create',
7373
resource,
74-
// If disableAuthentication is true then isPendingAuthenticated will always be true so this hook is disabled
75-
enabled: !isPendingAuthenticated,
74+
enabled: !disableAuthentication && !isPendingAuthenticated,
7675
});
7776
const { hasEdit, hasShow } = useResourceDefinition(props);
7877
const finalRedirectTo =

‎packages/ra-core/src/controller/edit/useEditController.security.stories.tsx‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { Resource } from '../../core/Resource';
1010
import { AuthProvider, DataProvider } from '../../types';
1111
import { TestMemoryRouter } from '../../routing/TestMemoryRouter';
1212
import { EditControllerProps, useEditController } from './useEditController';
13+
import { useAuthState } from '../..';
1314

1415
export default {
1516
title: 'ra-core/controller/useEditController',
@@ -36,6 +37,16 @@ const defaultDataProvider = fakeDataProvider(
3637
process.env.NODE_ENV === 'development'
3738
);
3839

40+
const PostList = () => {
41+
useAuthState();
42+
return (
43+
<div style={styles.mainContainer}>
44+
<div>List view</div>
45+
<Link to="/posts/1">Edit</Link>
46+
</div>
47+
);
48+
};
49+
3950
const Post = (props: Partial<EditControllerProps>) => {
4051
const params = useEditController({
4152
id: 1,
@@ -51,6 +62,7 @@ const Post = (props: Partial<EditControllerProps>) => {
5162
{params.record.title} - {params.record.votes} votes
5263
</div>
5364
)}
65+
<Link to="/posts">List</Link>
5466
</div>
5567
);
5668
};
@@ -97,16 +109,33 @@ export const DisableAuthentication = ({
97109
dataProvider={dataProvider}
98110
authProvider={authProvider}
99111
>
100-
<CoreAdminUI>
112+
<CoreAdminUI accessDenied={AccessDenied}>
101113
<Resource
102114
name="posts"
115+
list={<PostList />}
103116
edit={<Post disableAuthentication />}
104117
/>
105118
</CoreAdminUI>
106119
</CoreAdminContext>
107120
</TestMemoryRouter>
108121
);
109122
};
123+
DisableAuthentication.args = {
124+
authProvider: undefined,
125+
};
126+
DisableAuthentication.argTypes = {
127+
authProvider: {
128+
options: ['default', 'canAccess'],
129+
mapping: {
130+
default: undefined,
131+
canAccess: {
132+
...defaultAuthProvider,
133+
canAccess: () => Promise.resolve(false),
134+
},
135+
},
136+
control: { type: 'inline-radio' },
137+
},
138+
};
110139

111140
export const CanAccess = ({
112141
authProviderDelay = 300,

‎packages/ra-core/src/controller/edit/useEditController.spec.tsx‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,5 +1295,39 @@ describe('useEditController', () => {
12951295
expect(dataProvider.getOne).toHaveBeenCalled();
12961296
expect(authProvider.checkAuth).not.toHaveBeenCalled();
12971297
});
1298+
1299+
it('should not call checkAuth nor canAccess when disableAuthentication is true', async () => {
1300+
const authProvider: AuthProvider = {
1301+
checkAuth: jest.fn().mockResolvedValue(true),
1302+
login: () => Promise.resolve(),
1303+
logout: () => Promise.resolve(),
1304+
checkError: () => Promise.resolve(),
1305+
getPermissions: () => Promise.resolve(),
1306+
canAccess: jest.fn().mockResolvedValue(false),
1307+
};
1308+
render(<DisableAuthentication authProvider={authProvider} />);
1309+
await screen.findByText('Post #1 - 90 votes');
1310+
expect(authProvider.checkAuth).not.toHaveBeenCalled();
1311+
expect(authProvider.canAccess).not.toHaveBeenCalled();
1312+
});
1313+
1314+
it('should not call checkAuth nor canAccess when disableAuthentication is true even if useAuthState was called before', async () => {
1315+
const authProvider: AuthProvider = {
1316+
checkAuth: jest.fn().mockResolvedValue(true),
1317+
login: () => Promise.resolve(),
1318+
logout: () => Promise.resolve(),
1319+
checkError: () => Promise.resolve(),
1320+
getPermissions: () => Promise.resolve(),
1321+
canAccess: jest.fn().mockResolvedValue(false),
1322+
};
1323+
render(<DisableAuthentication authProvider={authProvider} />);
1324+
await screen.findByText('Post #1 - 90 votes');
1325+
fireEvent.click(await screen.findByText('List'));
1326+
await screen.findByText('List view');
1327+
fireEvent.click(await screen.findByText('Edit'));
1328+
await screen.findByText('Post #1 - 90 votes');
1329+
expect(authProvider.checkAuth).toHaveBeenCalledTimes(1);
1330+
expect(authProvider.canAccess).not.toHaveBeenCalled();
1331+
});
12981332
});
12991333
});

‎packages/ra-core/src/controller/edit/useEditController.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,7 @@ export const useEditController = <
7676
const { isPending: isPendingCanAccess } = useRequireAccess<RecordType>({
7777
action: 'edit',
7878
resource,
79-
// If disableAuthentication is true then isPendingAuthenticated will always be true so this hook is disabled
80-
enabled: !isPendingAuthenticated,
79+
enabled: !disableAuthentication && !isPendingAuthenticated,
8180
});
8281

8382
const getRecordRepresentation = useGetRecordRepresentation(resource);

‎packages/ra-core/src/controller/list/useInfiniteListController.spec.tsx‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -701,8 +701,43 @@ describe('useInfiniteListController', () => {
701701
);
702702
await screen.findByText('A post - 0 votes');
703703
expect(dataProvider.getList).toHaveBeenCalled();
704-
// Only called once by NavigationToFirstResource
705-
expect(authProvider.checkAuth).toHaveBeenCalledTimes(1);
704+
expect(authProvider.checkAuth).not.toHaveBeenCalled();
705+
});
706+
707+
it('should not call checkAuth nor canAccess when disableAuthentication is true', async () => {
708+
const authProvider: AuthProvider = {
709+
checkAuth: jest.fn().mockResolvedValue(true),
710+
login: () => Promise.resolve(),
711+
logout: () => Promise.resolve(),
712+
checkError: () => Promise.resolve(),
713+
getPermissions: () => Promise.resolve(),
714+
canAccess: jest.fn().mockResolvedValue(false),
715+
};
716+
render(<DisableAuthentication authProvider={authProvider} />);
717+
await screen.findByText('Post #1 - 90 votes');
718+
expect(authProvider.checkAuth).not.toHaveBeenCalled();
719+
expect(authProvider.canAccess).not.toHaveBeenCalled();
720+
});
721+
722+
it('should not call checkAuth nor canAccess when disableAuthentication is true even if useAuthState was called before', async () => {
723+
const authProvider: AuthProvider = {
724+
checkAuth: jest.fn().mockResolvedValue(true),
725+
login: () => Promise.resolve(),
726+
logout: () => Promise.resolve(),
727+
checkError: () => Promise.resolve(),
728+
getPermissions: () => Promise.resolve(),
729+
canAccess: jest.fn().mockResolvedValue(false),
730+
};
731+
render(<DisableAuthentication authProvider={authProvider} />);
732+
await screen.findByText('Post #1 - 90 votes');
733+
fireEvent.click(await screen.findByText('Dashboard'));
734+
await screen.findByText('Dashboard view');
735+
fireEvent.click(await screen.findByText('List'));
736+
await screen.findByText('Post #1 - 90 votes');
737+
// checkAuth is called twice: once by RA (with different params)
738+
// and once by our custom Dashboard component
739+
expect(authProvider.checkAuth).toHaveBeenCalledTimes(2);
740+
expect(authProvider.canAccess).not.toHaveBeenCalled();
706741
});
707742
});
708743
});

0 commit comments

Comments
 (0)