Introduce RBAC #1335
Introduce RBAC #1335
Conversation
googlebot
added
the
cla: yes
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
sethvargo
force-pushed
the
sethvargo/rbac
branch
2 times, most recently
from
d1dcee0
to
287bc1f
Compare
| func RequireAuth(cacher cache.Cacher, authProvider auth.Provider, db *database.Database, h *render.Renderer, sessionIdleTTL, expiryCheckTTL time.Duration) mux.MiddlewareFunc { | ||
| cacheTTL := 15 * time.Minute | ||
| cacheTTL := 30 * time.Minute |
There was a problem hiding this comment.
just wondering any particular reason for the cacheTTL doubling?
There was a problem hiding this comment.
I set it to 0 during testing, then changed it back. I apparently changed it back to the wrong value, but we could realistically cache this for 10000 days and it wouldn't matter because the gorm callbacks clear the cache when the key is changed or revoked.
This PR has a great set of nits fixed - that said it might have been easier to review if we split into 'functional' and 'nits' PRs. LGTM... but very possible I've missed something. /hold for when you're ready. |
Yea, sorry 😦. The HTML and trailing spaces is my editor and I didn't see that in the diff, sorry. I'm gonna wait for @mikehelmick to give a thumbs up before unholding. |
This introduces Role-Based Access Controls (RBAC) into the system. Common operations in the system are split into Read (view-only) and Write (create, update, delete). Users have 0 or more permissions on a realm through Memberships. Memberships replace the existing `user_realms` table (and obviates the `admin_realms` table). Since realm roles are no longer binary (previously user or admin), many of our existing UI elements no longer made sense. For example, we frequently displayed a "Realm Admin" pill in lists, but those have been removed since permissions are now multi-dimensional. There are two meta-permissions - LegacyRealmUser and LegacyRealmAdmin - which closely correspond to the existing primitive roles. The RBAC system also improves the event log, since individual permissions are now diffed. The RBAC system has security properties that prevent privilege escalation. The system forbids creating users with permissions greater than your own, and it forbids changing your own permissions entirely. The only exception is system administrators, who are granted full realm permissions when joining a realm and have all permissions revoked when leaving a realm (via the system admin console). The UI and templates can conditionally assert a membership's priviledges and updates accordingly. One of the biggest changes is conceptual - "Membership" is a first-class entity that must be handled and inspected in the system. The "current membership" defines the currently logged-in user and currently selected realm (if one exists). Additionally, RBAC assertion is now at the controller-level instead of the routing layer. I don't love it, but injecting it into the routing layer proved quite challenging and brittle. Finally, the system was designed to support more than just "users". If we ever wanted more granular permissions on, say API keys, we could easily add that. Other miscellaneous changes include: - Removing some totally dead code - Changing the response code on controller.NotFound to be 404 instead of 401 - Rendering a real 401 page instead of always forcing a sign-out - Better redirects after editing some resources List of future enhancements (I'll file issues after we reach consensus on this PR): - Display human descriptions of each permission in the UI
| <a href="/realm/apikeys/new" class="float-right mr-n1 text-secondary" data-toggle="tooltip" title="New API key"> | ||
| <span class="oi oi-plus small" aria-hidden="true"></span> | ||
| </a> | ||
| {{end}} |
There was a problem hiding this comment.
do we want to say that the person doesn't have ability to create API keys to avoid confusion - w/a. tooltip maybe?
There was a problem hiding this comment.
I actually had something like this originally, but then I looked across a bunch of systems and read a few blog posts that recommend against telling people they lack permissions.
I did update the realm guide to specifically say that "if you don't see this button, you don't have permission".
sethvargo
force-pushed
the
sethvargo/rbac
branch
from
287bc1f
to
54a75ee
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mikehelmick, sethvargo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This introduces Role-Based Access Controls (RBAC) into the system. Common operations in the system are split into Read (view-only) and Write (create, update, delete). Users have 0 or more permissions on a realm through Memberships. Memberships replace the existing
user_realmstable (and obviates theadmin_realmstable). Since realm roles are no longer binary (previously user or admin), many of our existing UI elements no longer made sense. For example, we frequently displayed a "Realm Admin" pill in lists, but those have been removed since permissions are now multi-dimensional. There are two meta-permissions - LegacyRealmUser and LegacyRealmAdmin - which closely correspond to the existing primitive roles. The RBAC system also improves the event log, since individual permissions are now diffed.The RBAC system has security properties that prevent privilege escalation. The system forbids creating users with permissions greater than your own, and it forbids changing your own permissions entirely. The only exception is system administrators, who are granted full realm permissions when joining a realm and have all permissions revoked when leaving a realm (via the system admin console). The UI and templates can conditionally assert a membership's priviledges and updates accordingly.
One of the biggest changes is conceptual - "Membership" is a first-class entity that must be handled and inspected in the system. The "current membership" defines the currently logged-in user and currently selected realm (if one exists). Additionally, RBAC assertion is now at the controller-level instead of the routing layer. I don't love it, but injecting it into the routing layer proved quite challenging and brittle.
Finally, the system was designed to support more than just "users". If we ever wanted more granular permissions on, say API keys, we could easily add that.
Other miscellaneous changes include:
List of future enhancements (I'll file issues after we reach consensus on this PR):
Fixes #1322
Release Note