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
google · Dec 12, 2020 · 287bc1f · 287bc1f
1 parent 2d2753f
commit 287bc1f
Show file tree

Hide file tree

Showing 133 changed files with 2,530 additions and 1,858 deletions.
diff --git 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
@@ -0,0 +1,21 @@
+{{define "401"}}
+<!doctype html>
+<html lang="en">
+<head>
+  {{template "head" .}}
+</head>
+
+<body>
+  <main role="main" class="container mt-5">
+    <h1>Unauthorized</h1>
+    <p>
+      You are not authorized to perform that action!
+    </p>
+    <div class="d-flex justify-content-between">
+      <a href="#" onclick="history.go(-1); return false;">&larr; Go back</a>
+      <a href="/signout">Sign out</a>
+    </div>
+  </main>
+</body>
+</html>
+{{end}}
diff --git a/cmd/server/assets/404.html b/cmd/server/assets/404.html
@@ -0,0 +1,20 @@
+{{define "404"}}
+<!doctype html>
+<html lang="en">
+<head>
+  {{template "head" .}}
+</head>
+
+<body>
+  <main role="main" class="container mt-5">
+    <h1>Not Found</h1>
+    <p>
+      The resource you attempted to access does not exist.
+    </p>
+    <p>
+      <a href="#" onclick="history.go(-1); return false;">&larr; Go back</a>
+    </p>
+  </main>
+</body>
+</html>
+{{end}}
diff --git a/cmd/server/assets/admin/_nav.html b/cmd/server/assets/admin/_nav.html
@@ -1,4 +1,7 @@
 {{define "admin/navbar"}}
+
+{{$currentMemberships := .currentMemberships}}
+
 <header class="mb-3">
   <div href="/" class="d-block px-3 py-2 text-center text-bold text-white admin-header">
     System admin
@@ -7,14 +10,22 @@
   <nav class="nav nav-tabs navbar-expand-md navbar-light bg-light">
     <div class="container">
       {{template "navtoggle" .}}
+
       <div class="collapse navbar-collapse mt-2" id="navbar">
-        {{if .currentRealm}}
         <ul class="navbar-nav mr-auto">
           <li class="nav-item">
-            <a class="nav-link" href="/codes/issue">&larr; Back to {{.currentRealm.Name}} </a>
+            {{if eq (len $currentMemberships) 1}}
+              <a class="nav-link" href="/codes/issue">
+                &larr; Back to {{(index $currentMemberships 0).Realm.Name}}
+              </a>
+            {{else if gt (len $currentMemberships) 1}}
+              <a class="nav-link" href="/login/select-realm">
+                &larr; Back to realm selection
+              </a>
+            {{end}}
           </li>
         </ul>
-        {{end}}
+
         {{template "navdropdown" .}}
       </div>
     </div>

diff --git a/cmd/server/assets/admin/info.html b/cmd/server/assets/admin/info.html
@@ -12,9 +12,9 @@
   <main role="main" class="container">
     {{template "flash" .}}
 
-    <div class="card mb-3 shadow-sm">
+    <div class="card shadow-sm mb-3">
       <div class="card-header">Info</div>
-      <div class="card-body">
+      <div class="card-body mb-n3">
         <dl>
           <dt>Build ID</dt>
           <dd>{{.buildID}}</dd>

diff --git a/cmd/server/assets/admin/realms/edit.html b/cmd/server/assets/admin/realms/edit.html
@@ -2,6 +2,7 @@
 
 {{$currentUser := .currentUser}}
 {{$realm := .realm}}
+{{$membership := .membership}}
 {{$systemSMSConfig := .systemSMSConfig}}
 {{$systemEmailConfig := .systemEmailConfig}}
 
@@ -23,20 +24,7 @@ <h1>Edit {{$realm.Name}}</h1>
     </p>
 
     <div class="card mb-3 shadow-sm">
-      <div class="card-header">
-        {{if $currentUser.CanAdminRealm $realm.ID}}
-        <a href="/admin/realms/{{$realm.ID}}/realmadmin">
-          <span class="oi oi-wrench text-dark" aria-hidden="true"
-          data-toggle="tooltip" title="Edit as realm admin"></span>
-        </a>
-        {{else}}
-        <span data-toggle="tooltip" title="Join realm to configure as realm admin">
-          <a class="disabled">
-            <span class="oi oi-wrench" aria-hidden="true"></span>
-          </a>
-        </span>
-        {{end}}
-        Details</div>
+      <div class="card-header">Details</div>
       <div class="card-body">
         <form method="POST" action="/admin/realms/{{$realm.ID}}">
           {{ .csrfField }}
@@ -133,7 +121,7 @@ <h6 class="mb-2">View</h6>
       </div>
     </div>
 
-    {{if $currentUser.CanAdminRealm $realm.ID}}
+    {{if and $membership ($membership.Can rbac.LegacyRealmAdmin)}}
       <div class="card mb-3 shadow-sm">
         <div class="card-header">Leave realm</div>
         <div class="card-body">

diff --git a/cmd/server/assets/admin/realms/index.html b/cmd/server/assets/admin/realms/index.html
@@ -1,6 +1,7 @@
 {{define "admin/realms/index"}}
 
 {{$realms := .realms}}
+{{$memberships := .memberships}}
 
 <!doctype html>
 <html lang="en">
@@ -53,7 +54,13 @@
             <tr>
               <td class="text-center">{{.ID}}</td>
               <td>
-                <a href="/admin/realms/{{.ID}}/edit">{{.Name}}</a>
+                <a href="/admin/realms/{{.ID}}/edit">
+                  {{.Name}}
+                </a>
+                {{if (index $memberships .ID)}}
+                  <span class="small oi oi-circle-check text-success ml-1" aria-hidden="true"
+                    data-toggle="tooltip" title="You have permissions on this realm"></span>
+                {{end}}
               </td>
               <td class="text-center">{{.RegionCode}}</td>
               <td class="text-center">

diff --git a/cmd/server/assets/admin/users/index.html b/cmd/server/assets/admin/users/index.html
@@ -30,7 +30,6 @@
             <span class="input-group-prepend">
               <select name="filter" id="filter" class="text-truncate custom-select dropdown-toggle border-right-0" style="border-radius:0.25rem 0 0 0.25rem;">
                 <option value="" selected>All users</option>
-                <option value="realmAdmins" {{selectedIf (eq .filter "realmAdmins")}}>Realm admins</option>
                 <option value="systemAdmins" {{selectedIf (eq .filter "systemAdmins")}}>System admins</option>
               </select>
             </span>
@@ -50,7 +49,6 @@
         <table class="table table-bordered table-striped table-fixed table-inner-border-only border-top mb-0" id="results-table">
           <thead>
             <tr>
-              <th scope="col" width="40"></th>
               <th scope="col">Name</th>
               <th scope="col">Email</th>
               <th scope="col" width="40"></th>
@@ -59,17 +57,11 @@
           <tbody>
           {{range $users}}
             <tr>
-              <td class="text-center">
-                {{if .SystemAdmin}}
-                <span class="oi oi-wrench" aria-hidden="true"
-                  data-toggle="tooltip" title="System admin"></span>
-                {{end}}
-              </td>
               <td class="text-truncate">
                 <a href="/admin/users/{{.ID}}">{{.Name}}</a>
-                {{if .IsRealmAdmin}}
-                <span class="oi oi-badge ml-2" aria-hidden="true"
-                  data-toggle="tooltip" title="Realm admin"></span>
+                {{if .SystemAdmin}}
+                  <span class="oi oi-wrench ml-1" aria-hidden="true"
+                    data-toggle="tooltip" title="System admin"></span>
                 {{end}}
               </td>
               <td class="text-truncate">{{.Email}}</td>

diff --git a/cmd/server/assets/admin/users/show.html b/cmd/server/assets/admin/users/show.html
@@ -1,6 +1,7 @@
 {{define "admin/users/show"}}
+
 {{$user := .user}}
-{{$stats := .stats}}
+{{$memberships := .memberships}}
 
 <!doctype html>
 <html lang="en">
@@ -49,24 +50,20 @@ <h6 class="mb-2">System admin</h6>
 
     <div class="card mb-3 shadow-sm">
       <div class="card-header">Member of realms</div>
-      {{if $user.Realms}}
+      {{if $memberships}}
         <ul class="list-group list-group-flush">
-          {{range $realm := $user.Realms}}
-          <li class="list-group-item d-flex justify-content-between">
-            <div>
-              {{$realm.Name}}
-              {{if $user.CanAdminRealm $realm.ID}}
-                <span class="badge badge-primary ml-2">Admin</span>
-              {{end}}
-            </div>
+          {{range $membership := $memberships}}
+            {{$realm := $membership.Realm}}
+            {{$user := $membership.User}}
+            <li class="list-group-item d-flex justify-content-between">
+              <span>{{$realm.Name}}</span>
 
-            <a href="/admin/realms/{{$realm.ID}}/remove/{{$user.ID}}" class="text-danger"
-              data-toggle="tooltip" title="Remove from realm"
-              data-method="PATCH" data-confirm="Are you sure you want to remove {{$user.Name}} from {{$realm.Name}}?">
-              <span class="sr-only">Remove from realm</span>
-              <span class="oi oi-x" aria-hidden="true"></span>
-            </a>
-          </li>
+              <a href="/admin/realms/{{$realm.ID}}/remove/{{$user.ID}}" class="text-danger"
+                data-toggle="tooltip" title="Remove from realm"
+                data-method="PATCH" data-confirm="Are you sure you want to remove {{$membership.User.Name}} from {{$realm.Name}}?">
+                <span class="oi oi-x" aria-hidden="true"></span>
+              </a>
+            </li>
           {{end}}
         </ul>
       {{else}}

diff --git a/cmd/server/assets/apikeys/index.html b/cmd/server/assets/apikeys/index.html
@@ -2,6 +2,12 @@
 
 {{$authApp := .authApp}}
 
+{{$currentMembership := .currentMembership}}
+{{$canWrite := false}}
+{{if $currentMembership.Can rbac.APIKeyWrite}}
+  {{$canWrite = true}}
+{{end}}
+
 <!doctype html>
 <html lang="en">
 <head>
@@ -14,13 +20,15 @@
   <main role="main" class="container">
     {{template "flash" .}}
 
-    <div class="card mt-4 mb-3">
+    <div class="card shadow-sm mt-4 mb-3">
       <div class="card-header">
         <span class="oi oi-key mr-2 ml-n1" aria-hidden="true"></span>
         API keys
-        <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>
+        {{if $canWrite}}
+          <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}}
       </div>
 
       <div class="card-body">
@@ -46,7 +54,9 @@
               <th scope="col">App</th>
               <th scope="col" width="90">Key</th>
               <th scope="col" width="90">Type</th>
-              <th scope="col" width="40"></th>
+              {{if $canWrite}}
+                <th scope="col" width="40"></th>
+              {{end}}
             </tr>
           </thead>
           <tbody>
@@ -71,25 +81,27 @@
                 {{if .IsAdminType}}<span class="badge badge-pill badge-primary" data-toggle="tooltip" title="Can be used to issue verification codes">Admin</span>{{end}}
                 {{if .IsDeviceType}}<span class="badge badge-pill badge-secondary" data-toggle="tooltip" title="For use in mobile apps to verify codes and get certificates">Device</span>{{end}}
               </td>
-              <td class="text-center">
-                {{if .DeletedAt}}
-                <a href="/realm/apikeys/{{.ID}}/enable" class="d-block text-danger"
-                  data-method="patch"
-                  data-confirm="Are you sure you want to restore '{{.Name}}'?"
-                  data-toggle="tooltip"
-                  title="Restore this API key">
-                  <span class="oi oi-loop-circular" aria-hidden="true"></span>
-                </a>
-                {{else}}
-                <a href="/realm/apikeys/{{.ID}}/disable" class="d-block text-danger"
-                  data-method="patch"
-                  data-confirm="Are you sure you want to disable '{{.Name}}'?"
-                  data-toggle="tooltip"
-                  title="Disable this API key">
-                  <span class="oi oi-trash" aria-hidden="true"></span>
-                </a>
-                {{end}}
-              </td>
+              {{if $canWrite}}
+                <td class="text-center">
+                  {{if .DeletedAt}}
+                  <a href="/realm/apikeys/{{.ID}}/enable" class="d-block text-danger"
+                    data-method="patch"
+                    data-confirm="Are you sure you want to restore '{{.Name}}'?"
+                    data-toggle="tooltip"
+                    title="Restore this API key">
+                    <span class="oi oi-loop-circular" aria-hidden="true"></span>
+                  </a>
+                  {{else}}
+                  <a href="/realm/apikeys/{{.ID}}/disable" class="d-block text-danger"
+                    data-method="patch"
+                    data-confirm="Are you sure you want to disable '{{.Name}}'?"
+                    data-toggle="tooltip"
+                    title="Disable this API key">
+                    <span class="oi oi-trash" aria-hidden="true"></span>
+                  </a>
+                  {{end}}
+                </td>
+              {{end}}
             </tr>
           {{end}}
           </tbody>

diff --git a/cmd/server/assets/apikeys/show.html b/cmd/server/assets/apikeys/show.html
@@ -4,6 +4,12 @@
 {{$authApp := .authApp}}
 {{$stats := .stats}}
 
+{{$currentMembership := .currentMembership}}
+{{$canWrite := false}}
+{{if $currentMembership.Can rbac.APIKeyWrite}}
+  {{$canWrite = true}}
+{{end}}
+
 <!doctype html>
 <html lang="en">
 
@@ -40,9 +46,11 @@ <h1>{{$authApp.Name}} API key</h1>
       <div class="card-header">
         <span class="oi oi-key mr-2 ml-n1" aria-hidden="true"></span>
         Details about {{$authApp.Name}}
-        <a href="/realm/apikeys/{{$authApp.ID}}/edit" class="float-right mr-n1 text-body" id="edit" data-toggle="tooltip" title="Edit this API key">
-          <span class="oi oi-pencil" aria-hidden="true"></span>
-        </a>
+        {{if $canWrite}}
+          <a href="/realm/apikeys/{{$authApp.ID}}/edit" class="float-right mr-n1 text-body" id="edit" data-toggle="tooltip" title="Edit this API key">
+            <span class="oi oi-pencil" aria-hidden="true"></span>
+          </a>
+        {{end}}
       </div>
       <div class="card-body">
         <strong>App name</strong>

diff --git a/cmd/server/assets/codes/index.html b/cmd/server/assets/codes/index.html
@@ -53,6 +53,10 @@ <h1>Verification code status</h1>
               {{$code.CreatedAt.Format "2006-02-01 15:04"}}
             </small>
           </a>
+        {{else}}
+          <div class="card-body text-center">
+            <em>You have not issued any codes recently.</em>
+          </div>
         {{end}}
       </div>
     </div>

diff --git a/cmd/server/assets/codes/issue.html b/cmd/server/assets/codes/issue.html
@@ -1,6 +1,7 @@
 {{define "codes/issue"}}
 
-{{$currentRealm := .currentRealm}}
+{{$currentMembership := .currentMembership}}
+{{$currentRealm := $currentMembership.Realm}}
 {{$hasSMSConfig := .hasSMSConfig}}
 
 <!doctype html>