Closed jkroepke closed 3 months ago
I am not quite sure if I understand this PR.
Imagine a situation where I have two dashboards: dash A and dash B.
In this case, what would be the minimum permission setting to the plugin? If I set Viewer
as minimum permission, there is nothing preventing from "regular" users to generate reports for dash B (as it will be the service token that makes API requests to Grafana and it has permissions to do so). If I set Editor
as minimum permission, "regular" users will not be able to generate reports for dash A. Do you see what I mean or Am I missing something here?
This PR has the focus on who is able to generate reports. For example Viewers should not have the permissions to generate reports overall. That all.
The case with Dashboard A and Dashboard B is a different use case and not coverer by this PR
Ahhh ok! Now I see what you mean. I think the PR title is very misleading though. What you are proposing here is to extend RBAC of Grafana to the plugin. I see some value in this feature.
However, regarding #75 all is not lost yet. Looking at app-with-rbac plugin example, it is very well possible to check for the permissions of the user on a given dashboard using authzClient
of Grafana. We can use the signed JWT token in this client and I have made some rapid tests with Grafana OSS image and it works like charm. No side effects. Operators need to enable accessControlOnCall
and idForwarding
feature flags.
I will put up a PR soon and we can discuss over there. What do you think?
While Grafana RBAC is an open-source feature, assign user to RBAC roles is an enterprise only feature.
Ref: https://github.com/grafana/grafana-plugin-examples/tree/main/examples/app-with-rbac#assign-the-role
Thats, why I implement the current approach.
However, your case to check the dashboard permissions seems valid, but it's distinct to this PR.
I'm aware that the plugin can define own RBAC permissions, but not sure if they are usable. In any case, such permissions are not provisionable.
While Grafana RBAC is an open-source feature, assign user to RBAC roles is an enterprise only feature.
I did not mean to create new role. I was talking about using RBAC to verify if the user have access to the resources.
The plugin was always meant to be a user facing one. When users have view permission on a given dashboard, it is natural that these users should be able to create a report out of it, if they want.
If we want to provide this sort of access control to the plugin, we need to be consistent with other Grafana components like using same style as dashboard permissions.
I did not mean to create new role. I was talking about using RBAC to verify if the user have access to the resources.
We should stop to discuss about this in this PR, since that is not the scope of that PR
The plugin was always meant to be a user facing one. When users have view permission on a given dashboard, it is natural that these users should be able to create a report out of it, if they want.
This PR change this. Only users view Editor permissions should able to do this. We have to abuse the default roles here, since Grafana RBAC is not availible in OSS.
If we want to provide this sort of access control to the plugin, we need to be consistent with other Grafana components like using same style as dashboard permissions.
This fine-granted access control requires Grafana Enterprise.
In our case, end-user show able to view dashboard, but they should not able generate PDF reports.
We have to abuse the default roles here, since Grafana RBAC is not availible in OSS.
You answered your question. It is a hack and not a pretty one. I never had any request on this sort of feature until now. And I feel that this will only confuse the users more. As a maintainer, I need to take care of the support issues, so I prefer to keep things as simple as I can. I would like to keep the plugin more "reporting" centric hiding all the Grafana's internals.
In our case, end-user show able to view dashboard, but they should not able generate PDF reports.
Well, you can separate the users in a different Org. and not enable plugin in that Org.
I will wait until more users ask for this sort of access control.
It is a hack and not a pretty one.
Sorry that I have to say this. The whole plugin can be considers as a hack. It's re-implement a enterprise only feature.
Well, you can separate the users in a different Org. and not enable plugin in that Org.
This is not possible, because Grafana does support multi-org scenarios together with OIDC. You can't tell Grafana that one user should be 2 orgs. You have to manually assign users to second org after initial login.
In general, Grafana does not really care about multi-org scenarios, since the "multi-tenant" strategy of Grafana is Grafana Teams (Enterprise Feature). They had already the plan to deprecate orgs (https://github.com/grafana/grafana/issues/24588), but they are keeped for now, but don't care anymore.
The Grafana Helm Charts which can sync dashboard from Kubernetes config maps to Grafana does only support 1 single org.
The multi org scenario is the worst possible option in Grafana.
As a maintainer, I need to take care of the support issues, so I prefer to keep things as simple as I can
Okay, so if this is the strategy, I had drop future efforts and start maintain a fork here.
As a maintainer, I need to take care of the support issues, so I prefer to keep things as simple as I can
As someone who was involved in this plugin right from the beginning, I did not understand what you are trying to achieve here. I would expect it will only make things more complicated for a regular sysadmin who wishes to use the plugin. It seems something you need for your specific use case.
Okay, so if this is the strategy, I had drop future efforts and start maintain a fork here.
Feel free. If you have specific needs, that would be a better solution.
@jkroepke Cheers for all the contributions tho!! Really appreciate them!
This PR adds the possibility to define a minimum permission set which is required to generate PDFs