foreman
foreman copied to clipboard
Fixes #37665 - Context-based frontend permission management
Redmine issue Community-forum post this implementation is based on
This PR implements a faster way to check user permissions in Foreman core/plugin frontend code. This speed advantage is gained by leveraging the ForemanContext to store the current user's permissions.
Changes made:
- Extended:
ForemanContext
Added permissions field to context. Contains the current user's permissions - Implmenented as a set of permission names. - Added:
Permitted
A component that abstracts the conditional rendering scheme of "Render if <permission> is granted"- Developers may require a single or multiple permissions
- Supports rendering a different component if the permissions are not granted
- Added:
usePermission/s
Two custom hooks that allow checking whether a is granted a single or multiple permissions - Added:
useForemanPermissions
Context hook that provides a reference to the actual permissions set - Added:
useRefreshedContext
A custom hook that refreshes the ForemanContext by requesting the up-to-date application state from the backend- automatically sets the ForemanContext when called
- Supports partial context updates
- Added:
ContextController
atapi/v2/context
Controller that interfaces with the application context on the backend- Only supports reading of the context at the moment
- Requires
view_context
permission
- Added: Foreman-core permissions as JS constants
- Added: Rake task to generate JS permission constants
- Added: DeveloperDocs page on permission handling
Performance
Five samples taken on a decently fast system using FireFox developer edition with cache disabled and no background tasks running. Different approaches tested using this component.
API-based approach
Sample | Total page load time (s) | API request duration (ms) |
---|---|---|
1 | 2.14 | 197 |
2 | 2.07 | 156 |
3 | 2.05 | 176 |
4 | 2.09 | 197 |
5 | 2.16 | 184 |
Mean | 2.10 | 182 |
Permitted
component
Sample | Total page load time (s) |
---|---|
1 | 1.98 |
2 | 1.96 |
3 | 1.91 |
4 | 1.99 |
5 | 1.99 |
Mean | 1.97 |
Permitted
component in conjunction with useRefreshedContext()
Sample | Total page load time (s) | API request duration (ms) |
---|---|---|
1 | 1.91 | 158 |
2 | 2.00 | 146 |
3 | 1.92 | 167 |
4 | 1.92 | 149 |
5 | 2.00 | 154 |
Mean | 1.95 | 155 |
Discussion
Although it may seem like the difference between Permitted
and Permitted
+ useRefreshedContext
is negligible, this is not the case, as the actual time to mount the component differs by at least the API request duration when using useRefreshedContext
. Unfortunately, I don't have any values there because the React profiler is ...not great.
Even with a full page-reload, a speed difference of ~7% may be observed. This will be amplified on weaker / busier systems than mine. Furthermore, navigation between frontend-rendered pages will benefit a lot more, as comparatively little data needs to be requested from the backend.
Unfortunately, I nuked my dev-environment shortly after collecting the data above, so I will have to amend the results for client-rendered -> client-rendered later :)
The case for permission constants in JS
Although not strictly in the scope of this issue, I believe that this presents a good opportunity to introduce permission constants for JS, similar to API status constants. These constants mainly offer the following benefits:
- Should it ever become necessary to rename a permission, having them constantized makes this very easy
- Modern IDEs can use these constants to perform analyses such as usage finding.
To aid developers in creating these constants, I created a rake task, export_permissions
, that automatically generates these JS constants from the permission seed file.
TODO
- ContextController test:
I tried a lot of things, but I didn't manage to mock the
app_metadata
function, so the test is useless at the moment - useRefreshedContext test:
Since this hook interfaces pretty deeply with the context and the API, testing it is rather difficult. I got pretty far with
renderHook
and mocks but am running into issues with the setForemanContext function.