Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When is the default 'user' role applied or reapplied? #3721

Open
minrk opened this issue Dec 16, 2021 · 1 comment
Open

When is the default 'user' role applied or reapplied? #3721

minrk opened this issue Dec 16, 2021 · 1 comment

Comments

@minrk
Copy link
Member

minrk commented Dec 16, 2021

reposting remaining tasks from merged PR #3714

One of the benefits of the RBAC switch in 2.0 is that a user can have their permissions reduced to zero without needing to delete the user from the db. This is, in most cases, deleting their user role assignment (and all others).

However, when exactly the 'user' role is assigned, or especially re-assigned has led to some confusion and indeed a few bugs (#3720, #3714, #3708, #3703).

I think things aren't quite right in 2.0.0 for the various cases of a User record being created for the first time, which can occur in a few places:

  • explicit in allowed_users (assigns user role, this one is simple). Same for explicit user.users role membership.
  • on first authentication (assigned user if it doesn't already have a role assigned - we should probably always include 'user' here, rather than making it dependent on whether there are other roles - fixed by simplify default role assignment #3720)
  • user declared for the first time in a custom role - this prevents user role assignment on login, making an inconsistent state, depending on if this role assignment occurred before or after their first login (fixed by Grant role after user creation during config load #3714)
  • user declared for the first time in a Group (mostly the same as in a role here, also fixed by Grant role after user creation during config load #3714)

I think in all of these cases, it makes sense for them to get user assigned at creation time, and open PRs #3708, #3714, and #3720 fix this.

Then I think the main question is: what do we do with users already in the db who have no role assignments, when not using allowed_users?

Also related: should a successful authentication imply user role membership, if it has been revoked in the past? As it is now, they get their role assignment on first login if they were not in the db already. However, if they are in the db and do not have the user role for whatever reason (e.g. explicitly revoked), they will not get it back. This has the possibly confusing effect of deleting a user with ~no permissions from the db having the effect of granting them more permissions when they next log in. I'm not sure that's desirable.

@minrk
Copy link
Member Author

minrk commented Dec 16, 2021

Because we do not have separate "authentication" and "authorization", Authenticators really do both. Given that, I think it makes sense for every successful authentication to ensure a user has the user role. That is, if it has been revoked (for whatever reason), it will be reinstated on successful login. That will make behavior consistent with a user being removed from the db.

If we want authenticators to gain explicit role assignment (makes sense, follows group management in #3548 and #3307), they can exclude 'user', but I don't think it makes a lot of sense now to allow a user to authenticate, but then not be able to do what any normal user can do.

Then, I think the main question is what to do on startup, if allowed_users is not specified: should it reinstate user role assignment, or not? I'm inclined to say no, and wait for a login to trigger this. Because something must have caused the user to loses that assignment (it could have been a bug, but it should have been an explicit action. The main case affected by this is admin-triggered start of other users' servers, which may fail due to insufficient permission (the error seen in #3703).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant