Non-admin users should not have access to monitoring data

User feedback:

When I invited the first users, it was noticed that everyone gets access to the dashboard (which is logical), and thus to the link to our monitoring (Grafana) link. I'm not sure if there's an idea behind making this visible for everyone and not only people with admin status, but I think it would be a good idea to make it possible to hide it, as the first comment from the first person we onboarded was "should I have access to this, that's weird". Or am I totally wrong here?

Todo

• Don’t show the link in the dashboard for non-admin users
• When visiting the Grafana URL and logging in:
  • (Grafana) admin users should have access to everything they have access to now (no change)
  • Non-admin users should not
    • see most of the dashobards, manybe one hand-picked one which we verify it doesn't have any sensible data
    • have access to Loki because logs might contain passwords etc (we have luck that Loki is broken btw stackspin#1340 (closed) )

assigned to @arie

moved from stackspin#1395 (moved)

added weight::2 label

added Prio::High Security User feedback labels

I disagree with the admin user non-admin user distinction. IMO Grafana (maybe named "Monitoring Dashboard") should be an app access right configurable like any other app.

TBH I would be surprised if any of those users that saw the button on the dashboard could actually log into Grafana. If it can't be configured through the dashboard I think right now only admin users actually have the right to log into it.

On a side note: what's the difference between Todo point 1 and 2? Both seem to be about visibility in the dashboard, I think one of them should be about authentication access rights instead.

I disagree with the admin user non-admin user distinction. IMO Grafana (maybe named "Monitoring Dashboard") should be an app access right configurable like any other app.

Right, I agree.

On a side note: what's the difference between Todo point 1 and 2? Both seem to be about visibility in the dashboard, I think one of them should be about authentication access rights instead.

I tried to distinguish them better

Thanks.

I think the admin/non-admin user distinction for Grafana is not super relevant right now. Most people will either need full access to Grafana, or no access at all (which should be the default right now, which is also why I don't understand why my test user had access).

So denying Grafana access and removing it from the dashboard of normal users should be top priority when addressing this issue IMO.

By the way I just created a non-admin normal user and I can confirm it had access to the Grafana dashboards. I really don't understand why TBH

changed the description

added 1 deleted label

removed 1 deleted label

We leave the Prio::High in stackspin and Security because it will be a security issue when we fix loki by merging stackspin!1426 (merged)

Now that we're removing loki, I guess I can remove those labels again :p.

removed Security label

removed Prio::High label

changed milestone to %2.1

assigned to @arie

added Doing label

First:

• check if this is still an issue after the recent dashboard changes, which make grafana a more first-class app.

I ran into another issue when testing that: #98 (closed)

Specifically, I just now tested:

• logging in to grafana with a previously existing (test) user on stackspin.net that did not have access to "Monitoring" according to the dashboard. Even so, logging in succeeded, and the monitoring tile was shown on the user's dashboard.
• In the database, there was no entry for "Monitoring" for this user in the app_role table. I added one explicitly with the "no access" role, then logged out from the dashboard and from grafana, then logged in again. I could still log in to grafana though.
• Doing the same thing for nextcloud (changing app access, log out, try to log back in) "fails successfully": you cannot log in to nextcloud if you don't have that app access. So this is indeed somehow specific to grafana.

[2022-10-26 09:56:32,933] INFO in login: Info: Found kratos_id e9a69e58-0d23-4261-8dd5-95189c42d2c3
[2022-10-26 09:56:32,933] INFO in login: Info: Found app_id kube-prometheus-stack
[2022-10-26 09:56:32,933] INFO in login: Info: Getting user from admin e9a69e58-0d23-4261-8dd5-95189c42d2c3
[2022-10-26 09:56:33,010] INFO in login: Using '[]' when applying consent for e9a69e58-0d23-4261-8dd5-95189c42d2c3
[2022-10-26 09:56:33,011] INFO in login: Providing consent to kube-prometheus-stack for e9a69e58-0d23-4261-8dd5-95189c42d2c3
[2022-10-26 09:56:33,011] INFO in login: e9a69e58-0d23-4261-8dd5-95189c42d2c3 was granted access to kube-prometheus-stack

The app_id that comes in is kube-prometheus-stack, that probably doesn't match the app id that we use in the database which I think is monitoring.

Also the Using '[]' when applying consent sounds off, but that could be due to that same problem maybe, I'll have to check the source.

We could either

1. rename the oauth client ID from kube-prometheus-stack to monitoring. We're not sure how hydra will respond when we do that on existing clusters though; or
2. rename the internal app ID from monitoring to kube-prometheus-stack. This would require a database migration; or
3. keep both names the same, but introduce a mapping to specify which oauth client IDs are allowed for a given app ID.

An additional reason to do 3. is that we later may want to have several oauth clients (grafana but also prometheus dashboard, alertmanager via some http basic auth <-> SSO bridge) that are switched together as a single access permission "monitoring".

I noticed another weird thing about Grafana SSO behaviour. If you're SSO-logged in to Grafana, and then log out from Grafana, then log in to Grafana again using SSO, you have to log in again to Stackspin. This is strange because we have not implemented single-sign off yet. For other apps, if you log out of them and back in again, the existing SSO session is reused and you don't need to re-authenticate.

This is strange because we have not implemented single-sign off yet

We haven't implemented single sign off in the sense that logging out from Grafana (or the dashboard) will not log you out from Nextcloud for example. But we did set a post logout URL for all applications that support it that ends your Hydra and Kratos session when you log out from the application. Otherwise you wouldn't be able to log in as a different user at all.

for all applications that support it

I guess that's what tripped me up. I compared the Grafana behaviour to Zulip, which then apparently doesn't support this.

Otherwise you wouldn't be able to log in as a different user at all.

Well you can but it's more work: you first log out of the app, then end your SSO session by logging out from the dashboard. That's what you still have to do for Zulip then I imagine, if you want to switch user there.

yeah, Zulip's SSO support is rather simple. They also don't support role sync.

created branch 1395-non-admin-users-should-not-have-access-to-monitoring-data to address this issue

created branch 105-non-admin-users-should-not-have-access-to-monitoring-data to address this issue

mentioned in merge request !78 (merged)

mentioned in commit f7246126

closed with merge request !78 (merged)