access: a token's limit narrows what an endpoint takes

Review 4, F1: `authorize` decided whether a request's endpoint was allowed by the
user's full capabilities, and applied a token's limit only afterwards, to the `Access`
it left for handlers. The store and the users endpoints read that; nothing else does.
So a token saying less than its user may do (the dashboard's hand-over token, reading
the member list only, for five minutes) did whatever the user may among the other
endpoints, on every node: for an operator, forgetting members, proposing or aborting
remakes, exporting the document, the quarantine, the log filter.

The limit is now applied first, and the endpoint decided by what is left (`allowed`,
its own function now, with a test: the hand-over token of an operator may look, and
nothing else).

1 files changed, 62 insertions, 19 deletions

diff --git a/src/api/access.rs b/src/api/access.rs
index 8737ef8..11daf83 100644
--- a/src/api/access.rs
+++ b/src/api/access.rs
@@ -287,8 +287,21 @@ fn needed(method: &Method, path: &str, node: &str) -> Need {
     }
 }
 
-/// Finds what the request's user may do, refuses the request if its endpoint needs
-/// more (`needed`), and leaves `Access` on it for the store's handlers. Inside
+/// Whether `access` (a token's limit applied) covers what an endpoint needs.
+fn allowed(need: Need, access: &Access) -> bool {
+    match need {
+        Need::Store | Need::Auth => true,
+        Need::Observe => match access {
+            Access::Everything => true,
+            Access::Caps(caps) => !caps.is_empty(),
+        },
+        Need::Protocol(right, path) => access.allows(right, &path),
+    }
+}
+
+/// Finds what the request's user may do, a token's limit applied, refuses the request
+/// if its endpoint needs more (`needed`), and leaves `Access` on it for the store's
+/// handlers. Inside
 /// `auth::basic_auth`, which found the user. A user the registry doesn't have may do
 /// nothing: `403`.
 pub(crate) async fn authorize(
@@ -325,23 +338,7 @@ pub(crate) async fn authorize(
             }
         }
     };
-    let allowed = match needed(request.method(), request.uri().path(), state.local_id()) {
-        Need::Store | Need::Auth => true,
-        Need::Observe => match &access {
-            Access::Everything => true,
-            Access::Caps(caps) => !caps.is_empty(),
-        },
-        Need::Protocol(right, path) => access.allows(right, &path),
-    };
-    if !allowed {
-        debug!(user = %user, method = %request.method(), path = %request.uri().path(), "Refused a request the user's capabilities don't cover");
-        return error_response(
-            StatusCode::FORBIDDEN,
-            format!("{user} may not do that here (docs/access.md)"),
-            None,
-        );
-    }
-    // A token that says less: only what it allows too.
+    // A token that says less: only what it allows too, for deciding as for the handlers.
     let access = match request
         .extensions()
         .get::<super::login::Token>()
@@ -350,6 +347,17 @@ pub(crate) async fn authorize(
         None => access,
         Some(limit) => Access::Caps(access.caps(&state, &user).within(limit)),
     };
+    if !allowed(
+        needed(request.method(), request.uri().path(), state.local_id()),
+        &access,
+    ) {
+        debug!(user = %user, method = %request.method(), path = %request.uri().path(), "Refused a request the user's capabilities don't cover");
+        return error_response(
+            StatusCode::FORBIDDEN,
+            format!("{user} may not do that here (docs/access.md)"),
+            None,
+        );
+    }
     request.extensions_mut().insert(access);
     next.run(request).await
 }
@@ -367,6 +375,41 @@ mod tests {
             .collect()
     }
 
+    /// A token's limit narrows what an endpoint takes, not only what the store's handlers
+    /// read: the dashboard's hand-over token (the member list, read) of an operator may
+    /// look, and nothing else.
+    #[test]
+    fn a_limit_narrows_endpoints() {
+        let operator = Caps::new(vec![Capability {
+            path: path("_continuum"),
+            rights: ["read".to_owned(), "write".to_owned()].into(),
+            by: String::new(),
+            since: 0,
+        }]);
+        let members = vec![Capability {
+            path: path("_continuum/members"),
+            rights: ["read".to_owned()].into(),
+            by: String::new(),
+            since: 0,
+        }];
+        let handed = Access::Caps(operator.clone().within(members));
+        let need = |method: &Method, at: &str| needed(method, at, "n1");
+        assert!(allowed(need(&Method::GET, "/admin/stats"), &handed));
+        assert!(allowed(need(&Method::GET, "/peers"), &handed));
+        for (method, at) in [
+            (Method::DELETE, "/peers/continuum:00"),
+            (Method::POST, "/admin/remakes"),
+            (Method::GET, "/admin/document"),
+            (Method::PUT, "/admin/log"),
+        ] {
+            assert!(!allowed(need(&method, at), &handed), "{method} {at}");
+            assert!(
+                allowed(need(&method, at), &Access::Caps(operator.clone())),
+                "{method} {at}"
+            );
+        }
+    }
+
     fn caps(list: &[(&str, &[&str])]) -> Access {
         Access::Caps(Caps::new(
             list.iter()