argo-cd icon indicating copy to clipboard operation
argo-cd copied to clipboard

ArgoCD user interface does not (consistently) hide menu items based on RBAC

Open jshin47 opened this issue 4 years ago • 5 comments

ArgoCD Version: v1.8.0+fdb5ada

Describe the bug

In general, actions that are disallowed via RBAC are not hidden from the user interface.

To Reproduce

Specifically, I have the following policy.csv, and left policy.default undefined (no default policy):

p, DatabaseTeam, applications, get, database-team/*, allow
p, DatabaseTeam, applications, sync, database-team/*, allow
p, DatabaseTeam, applications, delete, database-team/*, deny
g, DatabaseTeam, role:DatabaseTeam

If I log in as a user who is only a member of DatabaseTeam, I only see the project "database-team" in the UI. That's good!

But if I drill into the database-team project, and select an application in the project, at the top of the screen, I still see the "Delete" button.

I know that if I attempt to actually delete the application, the API Server will produce an RBAC-related error that pops up in the bottom-right of the screen.

But it would be nice if the Delete button were just not visible at all for those who do not have the ability to Delete an application.

Expected behavior

The user interface should only show the menu items that the logged-in user is allowed to do.

Screenshots

This is what my logged-in user sees (note that the Delete button is visible):

image

But RBAC is actually enforced, thankfully, when I try to delete.

image

Version

argocd-server: v1.8.0+fdb5ada
  BuildDate: 2020-12-09T01:12:22Z
  GitCommit: fdb5ada06d0a38f3d7aae548f77e98cc08188fbe
  GitTreeState: clean
  GoVersion: go1.14.12
  Compiler: gc
  Platform: linux/amd64
  Ksonnet Version: v0.13.1
  Kustomize Version: v3.8.1 2020-07-16T00:58:46Z
  Helm Version: v3.4.1+gc4e7485
  Kubectl Version: v1.17.8

Logs

N/A

jshin47 avatar Dec 09 '20 18:12 jshin47

similar request #3477

ArgTang avatar Dec 17 '20 21:12 ArgTang

Apart from being confusing/annoying, this would also be good for creating a TV/Monitor UI (ie, hide all unnecessary buttons)

Jellyfrog avatar Mar 18 '22 15:03 Jellyfrog

It would be better for usability if the Delete button were disabled / "greyed-out" rather than hidden.

sengi avatar May 12 '23 10:05 sengi

Also for things under settings?

Devs might get confused when they login, see settings and can't access anything but the 'Appearance' option due to their RBAC, with everything else just giving Access Denied errors.

It would be better for usability if the Delete button were disabled / "greyed-out" rather than hidden.

I don't necessarily disagree but, as an example, our CI pipeline handles the creation/deletion of any apps within Argo using Terraform (devs only have read/sync/log access to their apps) so devs might think there's a problem/configuration issue if it's greyed, whereas the question wouldn't come up otherwise.

Not sure how unique that would be but even in other setups, a greyed button can always beg the question "why not working?" even if the user doesn't need the option, they'll still instinctively want to know why it's greyed, whereas no button wouldn't have that issue.

Just a thought, I guess, but it's sort of like what I was saying above - I'd like those menu options to disappear, not be greyed or throw an error.

Either way, I don't understand how it's better for usability. If you can't use the button, what good is having it show as greyed? You can't argue that seeing it greyed implies no access as not existing does the same. From an operator standpoint I also think it's more noticeable if a button is there or not versus greyed if checking/reviewing permissions is what you mean by usability..?

Consistency might be better for usability and, as an example, 'Web Terminal' is one thing that is gone unless enabled+RBAC'd. And that's definitely NOT something I'd want appearing and greyed out to anyone without permissions lest they think they can have/beg for terminal access!

ForbiddenEra avatar Dec 08 '23 13:12 ForbiddenEra

+1 users should not be allowed to interact with UI elements that execute operations that are not allowed

gioppoluca avatar Apr 29 '24 10:04 gioppoluca