foreman icon indicating copy to clipboard operation
foreman copied to clipboard

Fixes #37665 - Context-based frontend permission management

Open Thorben-D opened this issue 5 months ago • 5 comments

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 at api/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.

Thorben-D avatar Oct 01 '24 05:10 Thorben-D