From 287bc1fc90a34d54e09eadda5c5c8337530e4c57 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Thu, 10 Dec 2020 13:43:01 -0500 Subject: [PATCH] Introduce RBAC 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 --- README.md | 2 +- cmd/server/assets/401.html | 21 ++ cmd/server/assets/404.html | 20 ++ cmd/server/assets/admin/_nav.html | 17 +- cmd/server/assets/admin/info.html | 4 +- cmd/server/assets/admin/realms/edit.html | 18 +- cmd/server/assets/admin/realms/index.html | 9 +- cmd/server/assets/admin/users/index.html | 14 +- cmd/server/assets/admin/users/show.html | 31 +- cmd/server/assets/apikeys/index.html | 60 ++-- cmd/server/assets/apikeys/show.html | 14 +- cmd/server/assets/codes/index.html | 4 + cmd/server/assets/codes/issue.html | 3 +- cmd/server/assets/codes/show.html | 47 +-- cmd/server/assets/header.html | 166 +++++++---- cmd/server/assets/login/account.html | 37 +-- cmd/server/assets/login/change-password.html | 5 +- cmd/server/assets/login/login.html | 30 +- cmd/server/assets/login/register-phone.html | 71 +++-- cmd/server/assets/login/select-realm.html | 72 +++-- cmd/server/assets/login/signout.html | 12 +- cmd/server/assets/mobileapps/index.html | 60 ++-- cmd/server/assets/mobileapps/show.html | 18 +- .../realmadmin/_form_abuse_prevention.html | 23 +- cmd/server/assets/realmadmin/_form_codes.html | 71 +++-- cmd/server/assets/realmadmin/_form_email.html | 4 +- cmd/server/assets/realmadmin/edit.html | 53 +++- cmd/server/assets/realmadmin/events.html | 1 - cmd/server/assets/realmadmin/keys.html | 23 ++ cmd/server/assets/realmadmin/stats.html | 16 +- cmd/server/assets/users/_form.html | 51 ++-- cmd/server/assets/users/edit.html | 4 - cmd/server/assets/users/import.html | 24 +- cmd/server/assets/users/index.html | 66 +++-- cmd/server/assets/users/new.html | 5 +- cmd/server/assets/users/show.html | 58 ++-- docs/images/admin/users02.png | Bin 110652 -> 137990 bytes .../alerts/ElevatedRateLimitedCount.md | 30 +- docs/production.md | 26 +- docs/realm-admin-guide.md | 18 +- go.mod | 3 +- go.sum | 6 +- internal/envstest/server.go | 32 +- internal/routes/server.go | 36 +-- internal/routes/server_test.go | 4 - pkg/controller/admin/caches.go | 5 +- pkg/controller/admin/caches_test.go | 13 +- pkg/controller/admin/email_test.go | 13 +- pkg/controller/admin/events_test.go | 12 +- pkg/controller/admin/mobile_apps_test.go | 12 +- pkg/controller/admin/realms.go | 111 +++---- pkg/controller/admin/realms_test.go | 13 +- pkg/controller/admin/sms_test.go | 13 +- pkg/controller/admin/users_list.go | 9 +- pkg/controller/admin/users_test.go | 13 +- pkg/controller/apikey/create.go | 18 +- pkg/controller/apikey/create_test.go | 31 +- pkg/controller/apikey/disable.go | 18 +- pkg/controller/apikey/enable.go | 18 +- pkg/controller/apikey/index.go | 19 +- pkg/controller/apikey/show.go | 18 +- pkg/controller/apikey/stats.go | 20 +- pkg/controller/apikey/update.go | 20 +- pkg/controller/codes/bulk_issue.go | 17 +- pkg/controller/codes/expire.go | 26 +- pkg/controller/codes/index.go | 18 +- pkg/controller/codes/issue.go | 21 +- pkg/controller/codes/issue_test.go | 29 +- pkg/controller/codes/logic.go | 50 ++-- pkg/controller/codes/show.go | 130 ++++---- pkg/controller/context.go | 50 ++++ pkg/controller/controller.go | 18 +- pkg/controller/issueapi/issue.go | 5 +- pkg/controller/issueapi/issue_batch.go | 8 +- pkg/controller/issueapi/logic.go | 40 ++- pkg/controller/login/controller.go | 4 + pkg/controller/login/register.go | 43 +-- pkg/controller/login/select.go | 33 +-- pkg/controller/middleware/auth.go | 23 +- pkg/controller/middleware/csrf.go | 2 - pkg/controller/middleware/emailverified.go | 21 +- pkg/controller/middleware/firewall.go | 13 +- .../middleware/{realm.go => membership.go} | 138 +++------ pkg/controller/middleware/mfa.go | 16 +- pkg/controller/mobileapps/create.go | 18 +- pkg/controller/mobileapps/create_test.go | 41 ++- pkg/controller/mobileapps/disable.go | 18 +- pkg/controller/mobileapps/enable.go | 18 +- pkg/controller/mobileapps/index.go | 15 +- pkg/controller/mobileapps/show.go | 15 +- pkg/controller/mobileapps/update.go | 18 +- pkg/controller/realmadmin/events.go | 17 +- pkg/controller/realmadmin/express.go | 67 ++--- pkg/controller/realmadmin/settings.go | 139 +++++---- pkg/controller/realmadmin/sms_test.go | 24 +- pkg/controller/realmadmin/smtp_test.go | 24 +- pkg/controller/realmadmin/stats.go | 27 +- pkg/controller/realmkeys/activate.go | 19 +- .../realmkeys/{new.go => create.go} | 16 +- pkg/controller/realmkeys/destroy.go | 16 +- pkg/controller/realmkeys/index.go | 14 +- pkg/controller/realmkeys/save.go | 28 +- pkg/controller/realmkeys/upgrade.go | 23 +- pkg/controller/user/controller.go | 21 +- pkg/controller/user/create.go | 53 ++-- pkg/controller/user/delete.go | 22 +- pkg/controller/user/export.go | 18 +- pkg/controller/user/import.go | 11 + pkg/controller/user/importbatch.go | 45 ++- pkg/controller/user/index.go | 21 +- pkg/controller/user/reset_password.go | 15 +- pkg/controller/user/search_test.go | 33 +-- pkg/controller/user/show.go | 64 ++-- pkg/controller/user/stats.go | 20 +- pkg/controller/user/update.go | 82 +++--- pkg/database/authorized_app_test.go | 8 +- pkg/database/database.go | 73 ++++- pkg/database/email_config_test.go | 15 +- pkg/database/membership.go | 55 ++++ pkg/database/migrations.go | 194 ++++++------ pkg/database/realm.go | 103 +++++-- pkg/database/realm_test.go | 8 +- pkg/database/scopes.go | 9 - pkg/database/sms_config_test.go | 15 +- pkg/database/user.go | 278 +++++++++--------- pkg/database/user_test.go | 47 ++- pkg/database/vercode.go | 2 - pkg/database/vercode_test.go | 19 +- pkg/rbac/rbac.go | 133 +++++++++ pkg/rbac/rbac_gen.go | 65 ++++ pkg/render/render.go | 11 + terraform/service_redirect.tf | 3 +- tools/seed/main.go | 18 +- 133 files changed, 2530 insertions(+), 1858 deletions(-) create mode 100644 cmd/server/assets/401.html create mode 100644 cmd/server/assets/404.html rename pkg/controller/middleware/{realm.go => membership.go} (51%) rename pkg/controller/realmkeys/{new.go => create.go} (74%) create mode 100644 pkg/database/membership.go create mode 100644 pkg/rbac/rbac.go create mode 100644 pkg/rbac/rbac_gen.go diff --git a/README.md b/README.md index 9d33c0815..f3813a511 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ for a verification system: - [API guide](docs/api.md) - [Exposure Notifications Express SMS / Deep Link Handling](docs/link_handling.md) -- [Realm admin guide](docs/realm-admin-guide.md) +- [Realm administration guide](docs/realm-admin-guide.md) - [Case worker guide](docs/case-worker-guide.md) - [System admin guide](docs/system-admin-guide.md) - [Development](docs/development.md) diff --git a/cmd/server/assets/401.html b/cmd/server/assets/401.html new file mode 100644 index 000000000..2531164d4 --- /dev/null +++ b/cmd/server/assets/401.html @@ -0,0 +1,21 @@ +{{define "401"}} + + + + {{template "head" .}} + + + +
+

Unauthorized

+

+ You are not authorized to perform that action! +

+ +
+ + +{{end}} diff --git a/cmd/server/assets/404.html b/cmd/server/assets/404.html new file mode 100644 index 000000000..15b16c18d --- /dev/null +++ b/cmd/server/assets/404.html @@ -0,0 +1,20 @@ +{{define "404"}} + + + + {{template "head" .}} + + + +
+

Not Found

+

+ The resource you attempted to access does not exist. +

+

+ ← Go back +

+
+ + +{{end}} diff --git a/cmd/server/assets/admin/_nav.html b/cmd/server/assets/admin/_nav.html index bede139cf..dea2af14b 100644 --- a/cmd/server/assets/admin/_nav.html +++ b/cmd/server/assets/admin/_nav.html @@ -1,4 +1,7 @@ {{define "admin/navbar"}} + +{{$currentMemberships := .currentMemberships}} +
System admin @@ -7,14 +10,22 @@