Propagate admin access to all apps for dashboard admins
For the context see also #61 (closed)
We need to handle dashboard admins specially so they have admin access to all apps by default, also when adding more apps. So for dashboard admins it should not be configurable which access they have to apps because they automatically grant admin access to all apps (the access menues shold be greyed out and should show "Admin".
Activity
marked this issue as related to #61 (closed)
marked this issue as related to #63 (closed)
If this is implemented, we need to take out the quick fix from !36 (merged)
mentioned in merge request stackspin!1219 (merged)
changed milestone to %0.8.3
added Prio::High label
@davor @luka Could anyone of you please review this issue and estimate how much time it would take ? I put it into %0.8.3 and raised the priority because it is blocking the %0.8.3 release. This showed up in our upgrade MR pipeline, see my note regarding this.
The problem boils down to: When a admin user is created with the v0.8 Stackspin branch, and Stackspin is upgraded to the current main branch, the admin user won't have access to any app:
@varac I've created this MR: !42 (merged)
Please test and check if everything is ok.
I didn't want to create a migration script to change current app roles for current users since it would be strange for them IMHO.
@davor Thanks for the MR. But see my comment above, the migration script for existing admin users is the most important part which is blocking the current %0.8.3 release, because of the reasons explained above. Could you please add it ?
Sorry I'm a bit late to this party.
Just a couple of questions
- If we create a new user in dashboard with admin rights - new user should have admin rights for all apps by default?
No not really. Well the way I thought this should be fixed, is that in the logic that checks if a user has access to an app, admin users always have access, and no access right specific to the app is checked at all. That way, admin always have access to all apps, also if you add an app later for example -- without needing any migration.
- If we upgrade existing user to dashboard admin - we should update all apps rights to admin?
Same answer as above basically: if this is implemented the way I imagined, you wouldn't need to do anything special in this upgrade case, it would be handled by the same logic that checks app access.
I get your point. So basically if the user is admin, every app role will be resolved to admin so we could remove app role editor from the FE because it doesn't affect anything if the user is admin and e.g. Zulip role is set to user.
So if the user has user dashboard role, then app roles could be set individually (e.g. Zulip can be admin, Wordpress can be user and all others can be No access).
And if I understand you correctly I think that app roles editor should be hidden or disabled if the user dashboard role is set to 'admin' because it would be strange if it would be 'user' Zulip role, but actually in backend it would resolve admin role.
-
I get your point. So basically if the user is admin, every app role will be resolved to admin so we could remove app role editor from the FE because it doesn't affect anything if the user is admin and e.g. Zulip role is set to user.
Exactly!
Only removing the app role editor is maybe slightly confusing. Maybe we could disable it (grayed out), or remove it but in its place display a short message "Admin users automatically have admin-level access to all apps."?
-
I think so, let me just check with @varac so we are speaking with one voice :).
Thanks @davor and sorry for the misunderstanding
- MR for FE: !42 (merged)
- MR for BE: dashboard-backend!81 (merged)
It would be great if it could be double checked and tested again :)
Yes,, it should work: dashboard-backend!81 (diffs)
If the user is dashboard admin it doesn't matter what is app access for that user, we're returning 'admin' rights. I've tested backend locally with wordpress and nextcloud where I used a user with dashboard admin and both wordpress and nextcloud no access rights and I logged in normally to both those services.
A quick summary of me and @davor chatting in PMs (correct my if I'm wrong):
- Unclear why this user wasn't granted admin access to the dashboard
- @davor Manually gave the user admin rights
- Afterwards it worked as expected
However, this is excatly the problem which we try to solve. This admin user was created a while ago using and older Stackspin version, and after trying with both related MRs, it doesn't have admin access anymore, without manual changes. I guess we need to try the upgrade pipeline with these MRs to be sure.
I agree with @varac. I'm not familiar with whole setup from scratch and what is inserted in the DB. Migration scripts should be checked if the upgrades and downgrades are working as expected, I've created only the last migration script so I'm not sure how others work. Maybe it would be good to have dev and testing DB's where testing DB should not be changed manually (without write permissions) so that we can have one DB with data that reflects production?
@davor: The admin user gets added by the job-initialize-user helm post-install/post-upgrade job. So i.e. with Stackspin %0.8.2, the dashboard chart 0.2.3 was used which installed the 0.1.8 dashboard image (87007c95). The helm chart v0.2.3 job-initialize-user job doesn't look much different though, basically they both call
flask cli user create $SETUP_EMAIL; ... flask cli user setrole $SETUP_EMAIL dashboard admin;
assigned to @davor
mentioned in issue stackspin#1313 (closed)
mentioned in merge request !42 (merged)
added Doing label
mentioned in merge request dashboard-backend!81 (merged)
@davor: Could you please try with the admin user from the screenshot above and confirm this ?
closed with merge request dashboard-backend!81 (merged)
mentioned in commit ff82530f
removed Doing label
