[fix] Allow undefined url in permission check#1298
Conversation
srosset81
left a comment
There was a problem hiding this comment.
It means that the user could potentially see the page (potentially in a "flash") even if he's not connected. That doesn't seem like a good practice. useCheckPermissions is supposed to act like a "blocker".
There are many ways to ensure the URL is present before calling the hook. Where exactly do you have the problem ? React-Admin notably has a useGetRecordId that always returns the URI, even if the resource is not loaded yet.
|
Thanks for your comment ! I had the problem on Archipelago when I wanted to convert to TS these files:
In that case, the resourceId exists in context, but it is loaded only after resource fetch. So at the first render of the page, it is undefined. If the resource is not authorized, it returns a 403 which made the page to redirect. There is no content flash has react-admin seems to handle that internally until the resource is loaded. So finally, this call to useCheckPermissions is useless, as it is handled directly by react-admin when fetching the resource.
In that case, resourceId doesn't exist as it is a "container" page. We need to ask dataProvider to recreate the correct middleware url to check permissions. That line is here useful. In the same logic, at the first render, the url is not provided, so it is also undefined (until the dataProvider has fetched the void endpoint to retrieve middleware containers urls). I didn't detect any content flash here, but we can eventually wait for container url defined to display the page. I also found that, contrary to what I thought, even if the url is undefined when calling useCheckPermissions, react-admin transforms it to a empty object in usePermissions hook (https://github.com/marmelab/react-admin/blob/v4.16.20/packages/ra-core/src/auth/usePermissions.ts#L39) !! So my updates in the authProvider.js file here is useless too 😅 Sorry to have missed that point! I amend my commit to just update the useCheckPermissions types to allow undefined urls. |
e08f99f to
723e7d6
Compare
Hello,
Little change here to allow undefined urls to be passed to useCheckPermissions hook and getPermissions method.
React-admin can first render view component without context url fully provided, and so an error was thrown.