Hello,
While contemplating how to check permissions for the wizard that creates new job invocations, I was looking for some unified/established way that we have to approach the issue, but unfortunately, I couldn’t find one. It seems like Katello has its own method?
Regarding the new job wizard I mentioned earlier, I came up with a workaround solution (PR link) using a new API to fetch current user permissions (PR link).
I had some great conversations about this issue with some of my teammates (you can see some of these discussions in the first PR), which brought up several observations:
Permissions lack dependencies between each other: users can have the create_job_invocations permission without the view_job_templates one, which is needed to create a job. Should we automatically bundle necessary permissions together? Currently, users can end up with not fully compatible permissions.
Single permission checks per action: We only check for one permission per action, even though sometimes we need multiple. For example, action categories can be in the engine.rb permission block just once. If listed in multiple ones, having only one of those permissions is enough (not all of them).
Roles: Should we enforce using the predefined roles that have all the needed permissions?
React path issue and lack of react “access control” methods: some react#index paths lack the correct authorizations (e.g. display_link_if_authorized) unless the user is admin (e.g. match 'new/hosts/' => 'react#index', :via => :get, :as => :new_hosts_index_page). Should we add/assign the needed permissions for every page?
I’d love to hear your thoughts / solutions / ideas / issues. Does it even make sense? Your feedback is appreciated!
Here’s an example of how it’s implemented in Katello:
The basic flow is
The API request sends include_permissions=true
The API response includes a list of permissions the current user has
On the React page, we look at multiple permissions and only show actions if the user has all of the required permissions.
Good point about the React path issue; there probably should be a Redmine for that.
I feel like Katello’s approach above works fine for handling multiple permissions in the UI. Permission dependencies can also be handled, simply by adding them to the list of required permissions. On the backend, our permission system with the roles/filters/permissions structure is already pretty mature and mostly works well.
It will be good to agree on a standard way that can be consistently implemented across Foreman and plugins.
I like the approach of enhancing API requests to include the correct information so you don’t have permission checks in 2 places.
Our current API doesn’t do this well, but in a REST API you can include the URLs to related collections/objects/actions. This has the massive benefit that the UI code isn’t sprawled with hardcoded URLs. You can also enhance this. From the plugin example, you could include
{
"import_html_url": "/path/to/import/view"
}
But if there’s no permission to do so:
{
"import_html_url": null
}
Note I distinguish between frontend (HTML) and API URLs, just like GitHub’s API does.
Just a quick reminder and I think, everyone is aware of this:
if permissions are verified on a page and it hides a action button its nice to hide it, if the user doesn’t have the permission to run the action. The check for the permission must be done on the backend rails API, too because JS code can be easily adapted in the browser.
In ReactApp, we can see that every time the root React-app is mounted, a context, ForemanContext is given along.
This context is therefore available to every child component.
The context contains, among others, information about the UI, the organization and pagination settings.
My proposal would be to pass the current user’s permissions to that context.
Since the permissions are unique, it makes sense to implement this using a data-structure with a fast access time. In this case, I chose Set, as, ideally, the .has method has a time complexity of O(1).
(ES6 mandates that this operation must complete in sublinear time, but any JS-engine worth it’s salt will do it in O(1))
I implemented a rough proof-of-concept and used it in the rewritten-using-React Ansible roles page I implemented recently:
Proof of concept implementation in Foreman:
Along with the contextified permissions, I would like to propose three ways of consuming them in frontend code:
the usePermission hook
This custom hook takes a permission name and returns whether the current user has the given permission
the Permitted component
This component aims to extract the pattern of “render if permission”. It wraps the component that is supposed to be conditionally rendered and a permission’s name. Only if the user has the given permission, the component will be rendered.
using the useForemanPermissions and useForemanContext hooks.
Intended for advanced use cases, the useForemanPermissions hook gives access to the permissions set.
Reactified Ansible roles page
This is the original implementation of the React roles page. As with the old one, the button to import roles from a Smart Proxy is hidden if a user does not have the import_ansible_roles permission.
This is done by wrapping the button component, making an API request to query the user’s permission and rendering the button if the permission is granted.
Reactified Ansible roles page using the proof-of-concept
Here, I replaced the aforementioned API-based permission checking with the context-based solution.
As a result of this a lot of API-handling boilerplate can be omitted, as there is no more API-request. Furthermore, the time complexity of the permission-check was reduced from linear to constant time (in V8 at least).
On a side note: I think it would make sense in general to provide the permissions as constants as this makes refactoring a lot easier should it ever become necessary.
A few words about performance:
Since this approach uses a hash based data-structure instead of an array, the time-complexity of a single permission check has been reduced from O(n) to O(1). Consequentially, the time-complexity for multiple permission checks has been reduced from O(n²) to O(n) with n being the number of permission checks. Depending on how loaded a Foreman server is with plugins, it can make a difference. Furthermore, this approximation still leaves out the API-request.
The old, API based approach completes the request to fetch the current user’s permissions in around ~200ms (on a decently fast system).
I would try to profile the custom hook, but profiling React is just a pain. I am confident however that the hook completes much faster.
The loading time of the whole page is reduced by 100ms-150ms on average.
The mounting time of the root app is increased as is the metadata payload, but not by much as the front-end only requires the name of the permissions and is not concerned about id or ressource_type.
The data to fill the context is queried by the root-app every time it is mounted, which still happens quite often at the moment, but this will obviously decrease once more and more pages use client-side routing.
I can deep-dive into the performance implications if desired, but I think this already paints a pretty clear picture.
One downside:
At the moment, I can only think of one downside to this approach and that is stale context.
As mentioned above, the context is refreshed every time the React app is mounted. Usually, this happens when the page reloads.
In a full-fledged SPA scenario, it would be possible for a user to acquire a permission, which would not be acknowledged by the UI until it is reloaded and therefore, the context is refreshed.
Though, this will be a problem regardless of permissions and by the time we get there, the context would have to be refreshed or the page reloaded explicitly. I can’t think of an impromptu example, but I don’t think big-corp SPAs like MS Teams are above this…
As of right now though, this can not happen through the UI, since every management UI that can grant permissions to users is completed by pressing “Submit”, which reloads the page, as it is rendered server-side.
Yeah, a bit of a long one, so thanks a lot for reading and I’m very much looking forward to your opinions on this!
Can’t edit the post…
I have to correct myself there: The time-complexity for multiple API-based permission checks is O(mn) with m being the number of permissions and n being the number of checks…
If we get there and it does indeed become a problem, then I’m sure we can send a notification to the front end. I think we should replace the polling on notifications with a websockets implementation and if we design it in a more generic way then you could also push those changes. Even if the implementation is just a “your permissions have changed, reload the page” banner in the initial stage.
It makes a lot of sense to me. Not having to retrieve it from an API request is much better and probably a much bigger impact than the exact implementation of a set or a list. Also in reliability, because any network request can (and at some point will) fail.
I just want to remind folks that our permission system allows to define conditions for many resources, e.g. view_hosts can be limited by scoped_search query like name ~ a*. This is always evaluated on demand on the backend side. I’m not sure if we can handle that somehow on the frontend side. I can’t tell how many people use such capabilities, I know about some doing things like owner = current_user, which basically give host permissions to hosts the current user owns. Maybe frontend does not have to go that far, not sure.
Especially for hosts and compute resources such filters are common. Hosts are filtered at nearly all customers were multiple teams use Foreman, compute resources at the larger ones typically using VMware to prevent misplacement in the virtual environment.
I also took that into account, but I’d think it’s incredibly hard to take that into account in our current infrastructure.
That is why I suggested providing the links in the REST API, but even that is limited because an endpoint may have multiple actions (GET/POST/PUT/DELETE for view, create, update, delete).
I really don’t see a good way to efficiently in the frontend.
I’m glad you like it! Yes, the datastructure used is basically irrelevant, as the biggest difference will be the ~200ms needed for the API request. Law of triviality…
Regarding Marek’s point about fine-grained permissions: I do not think it is easily possible to handle those in the front-end. For permissions like those, I think it is perfectly valid to handle them in the back-end.
(How) Should I proceed with my proposal?
