aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authortv <tv@krebsco.de>2026-11-09 14:34:52 +0000
committertv <tv@krebsco.de>2026-11-09 16:10:00 +0100
commitcf2bfaf4f3d54eac5444ae74ecd1407b3eecedd5 (patch)
tree63acd212d4f41cd145805db79c545ca0f6bd032a
parent4dbd83e1c38bb8c2e17884ba3fd5145251b3cc0f (diff)
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).
-rw-r--r--src/api/access.rs81
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()