max / makenotwork
6 files changed,
+417 insertions,
-41 deletions
| @@ -589,6 +589,48 @@ | |||
| 589 | 589 | Self(self.0.nest(path, other.0)) | |
| 590 | 590 | } | |
| 591 | 591 | ||
| 592 | + | /// Mount a service that does its own routing, under one declared posture. | |
| 593 | + | /// | |
| 594 | + | /// For a sub-tree this router cannot see inside: the description layer's | |
| 595 | + | /// adapter (`crate::quasi`) resolves its own state per request and routes | |
| 596 | + | /// internally, so it arrives as a service rather than as a set of | |
| 597 | + | /// `MethodRouter`s the helpers could wrap one at a time. | |
| 598 | + | /// | |
| 599 | + | /// **Always Auto, and there is deliberately no posture argument.** A skip | |
| 600 | + | /// here would exempt a whole sub-tree at once on one line, which is exactly | |
| 601 | + | /// the shape of the mistake the structural seal exists to make impossible. | |
| 602 | + | /// A surface that genuinely needs another posture should register its | |
| 603 | + | /// routes individually so each one declares and justifies itself. | |
| 604 | + | /// | |
| 605 | + | /// The validation layer wraps the service rather than anything inside it, so | |
| 606 | + | /// a tokenless mutation is refused before the nested router is consulted. A | |
| 607 | + | /// path the service does not serve is therefore refused too, which is the | |
| 608 | + | /// correct order: whether a route exists is not something an unauthenticated | |
| 609 | + | /// caller should learn by probing. | |
| 610 | + | /// | |
| 611 | + | /// One manifest entry, keyed by the mount path, so the coverage test sees | |
| 612 | + | /// the sub-tree as one Auto surface and probes it as one. | |
| 613 | + | #[must_use] | |
| 614 | + | pub fn nest_service<T>(self, path: &str, service: T) -> Self | |
| 615 | + | where | |
| 616 | + | T: tower::Service<Request, Error = std::convert::Infallible> | |
| 617 | + | + Clone | |
| 618 | + | + Send | |
| 619 | + | + Sync | |
| 620 | + | + 'static, | |
| 621 | + | T::Response: IntoResponse + 'static, | |
| 622 | + | T::Future: Send + 'static, | |
| 623 | + | { | |
| 624 | + | record_route(path, CsrfPosture::Auto); | |
| 625 | + | let guarded = tower::ServiceBuilder::new() | |
| 626 | + | .layer(from_fn(|req: Request, next: Next| async move { | |
| 627 | + | let path = req.uri().path().to_string(); | |
| 628 | + | validate_auto(req, next, &path).await | |
| 629 | + | })) | |
| 630 | + | .service(service); | |
| 631 | + | Self(self.0.nest_service(path, guarded)) | |
| 632 | + | } | |
| 633 | + | ||
| 592 | 634 | #[must_use] | |
| 593 | 635 | pub fn layer<L>(self, layer: L) -> Self | |
| 594 | 636 | where |
| @@ -552,8 +552,24 @@ | |||
| 552 | 552 | .merge(git_issue_routes()) | |
| 553 | 553 | .merge(git_write_routes()) | |
| 554 | 554 | .merge(ota_routes()) | |
| 555 | - | .merge(build_routes()) | |
| 556 | - | .finalize(); | |
| 555 | + | .merge(build_routes()); | |
| 556 | + | // The description layer, when a screen is switched on. Inside the CSRF tree | |
| 557 | + | // rather than beside it, so a described write is covered by the same | |
| 558 | + | // envelope every other mutation is: `origin_gate` from `finalize` below, | |
| 559 | + | // and the Auto token check from `nest_service`. It sat outside until | |
| 560 | + | // 2026-08-11, which was harmless only for as long as described screens | |
| 561 | + | // served nothing but GET. | |
| 562 | + | // | |
| 563 | + | // Nested as a service because the adapter routes internally and carries its | |
| 564 | + | // own state, resolved per request; it takes none from axum. The Askama route | |
| 565 | + | // for a described screen is not registered (see `dashboard_routes` and | |
| 566 | + | // `public_routes`), so nothing here overlaps. | |
| 567 | + | let csrf_routes = quasi::mounts(state) | |
| 568 | + | .into_iter() | |
| 569 | + | .fold(csrf_routes, |routes, (path, described)| { | |
| 570 | + | routes.nest_service(path, described) | |
| 571 | + | }); | |
| 572 | + | let csrf_routes = csrf_routes.finalize(); | |
| 557 | 573 | let app = Router::new() | |
| 558 | 574 | .merge(page_routes( | |
| 559 | 575 | state.config.rate_limits, | |
| @@ -589,18 +605,6 @@ | |||
| 589 | 605 | .fallback(routes::custom_domain::custom_domain_fallback) | |
| 590 | 606 | .with_state(state.clone()); | |
| 591 | 607 | ||
| 592 | - | // The description layer, when a screen is switched on. Nested as a service | |
| 593 | - | // for the same reason the spike is: the adapter mounts as a fallback and | |
| 594 | - | // this server already has one. Mounted after `with_state` because the | |
| 595 | - | // adapter carries its own state, resolved per request, and takes none from | |
| 596 | - | // axum. The Askama route for a described screen is not registered (see | |
| 597 | - | // `dashboard_routes` and `public_routes`), so nothing here overlaps. | |
| 598 | - | let app = quasi::mounts(state) | |
| 599 | - | .into_iter() | |
| 600 | - | .fold(app, |app, (path, described)| { | |
| 601 | - | app.nest_service(path, described) | |
| 602 | - | }); | |
| 603 | - | ||
| 604 | 608 | // There is no /metrics scrape endpoint. Prometheus and Grafana were retired | |
| 605 | 609 | // on 2026-07-21 and PoM is the monitoring story, so the endpoint had no | |
| 606 | 610 | // consumer left. The recorder itself stays: the admin metrics dashboard |
| @@ -29,7 +29,7 @@ | |||
| 29 | 29 | ||
| 30 | 30 | use makeover_layout as layout; | |
| 31 | 31 | use quasi_router::screen::{Act, Cell, Cells, Column}; | |
| 32 | - | use quasi_router::{Action, Node, RegionKind, Request, Response, RouteError, Slot}; | |
| 32 | + | use quasi_router::{Action, Method, Node, RegionKind, Request, Response, RouteError, Slot}; | |
| 33 | 33 | use quasi_webview::{Shell, Webview}; | |
| 34 | 34 | ||
| 35 | 35 | use super::Viewer; | |
| @@ -44,6 +44,12 @@ | |||
| 44 | 44 | /// The region the answer replaces: the pane the library's tab nav targets. | |
| 45 | 45 | const REGION: &str = "tab-content"; | |
| 46 | 46 | ||
| 47 | + | /// The address a revoke calls, relative to this screen's own nest. | |
| 48 | + | const REVOKE: &str = "/revoke/{seller_id}"; | |
| 49 | + | ||
| 50 | + | /// The writes this screen serves. Registered under its nest by `super::mount`. | |
| 51 | + | pub const WRITES: &[(Method, &str, super::Screen)] = &[(Method::Delete, REVOKE, revoke)]; | |
| 52 | + | ||
| 47 | 53 | /// One buyer who chose to share their email, as the screen needs it. | |
| 48 | 54 | pub struct BuyerView { | |
| 49 | 55 | username: String, | |
| @@ -118,6 +124,33 @@ | |||
| 118 | 124 | Ok(Response::fragment(REGION, pane(&buyers, &shared))) | |
| 119 | 125 | } | |
| 120 | 126 | ||
| 127 | + | /// Revoke sharing with one creator, and answer with the tab as it now stands. | |
| 128 | + | /// | |
| 129 | + | /// The screen's own route rather than `DELETE /api/contacts/{id}`, which the | |
| 130 | + | /// Askama version calls and which answers 204. htmx never swaps a 204, so the | |
| 131 | + | /// described control appeared to do nothing: the revoke landed and the row | |
| 132 | + | /// stayed until the reader left the tab and came back. Answering the whole pane | |
| 133 | + | /// is what a described write is for, and it is one query more than the API route | |
| 134 | + | /// runs, on an action a reader takes once. | |
| 135 | + | pub fn revoke(viewer: &Viewer, request: Request) -> Result<Response, RouteError> { | |
| 136 | + | // Taken by value because the handler signature is quasi's. | |
| 137 | + | let captures = request.captures; | |
| 138 | + | let seller: crate::db::UserId = captures | |
| 139 | + | .get("seller_id") | |
| 140 | + | .and_then(|id| id.parse().ok()) | |
| 141 | + | .ok_or_else(|| RouteError::not_found("no such creator"))?; | |
| 142 | + | ||
| 143 | + | viewer | |
| 144 | + | .block_on(db::transactions::revoke_contact_sharing( | |
| 145 | + | &viewer.app.db, | |
| 146 | + | viewer.user.id, | |
| 147 | + | seller, | |
| 148 | + | )) | |
| 149 | + | .map_err(|_| RouteError::internal("that sharing could not be revoked"))?; | |
| 150 | + | ||
| 151 | + | screen(viewer, Request::get(PATH)) | |
| 152 | + | } | |
| 153 | + | ||
| 121 | 154 | /// Everything inside the tab pane. | |
| 122 | 155 | /// | |
| 123 | 156 | /// Split from the handler so a test can build it without a database, the same | |
| @@ -210,7 +243,10 @@ | |||
| 210 | 243 | .activate(Action::get(format!("/u/{}", creator.username))), | |
| 211 | 244 | Cell::acts([Act::new( | |
| 212 | 245 | "Revoke", | |
| 213 | - | Action::delete(format!("/api/contacts/{}", creator.seller_id)), | |
| 246 | + | // This screen's own route, under its own nest. The API's | |
| 247 | + | // answers 204, which htmx never swaps, so the row stayed | |
| 248 | + | // after a successful revoke. See `revoke`. | |
| 249 | + | Action::delete(format!("{PATH}/revoke/{}", creator.seller_id)), | |
| 214 | 250 | ) | |
| 215 | 251 | // The template asked with hx-confirm. Said here, a | |
| 216 | 252 | // terminal host asks in its own way and no host can | |
| @@ -321,8 +357,14 @@ | |||
| 321 | 357 | ); | |
| 322 | 358 | // Per row rather than one shared endpoint, which is the mistake a loop | |
| 323 | 359 | // over rows makes when the id is read outside it. | |
| 324 | - | assert!(html.contains("hx-delete=\"/api/contacts/s1\""), "{html}"); | |
| 325 | - | assert!(html.contains("hx-delete=\"/api/contacts/s2\""), "{html}"); | |
| 360 | + | assert!( | |
| 361 | + | html.contains(&format!("hx-delete=\"{PATH}/revoke/s1\"")), | |
| 362 | + | "{html}" | |
| 363 | + | ); | |
| 364 | + | assert!( | |
| 365 | + | html.contains(&format!("hx-delete=\"{PATH}/revoke/s2\"")), | |
| 366 | + | "{html}" | |
| 367 | + | ); | |
| 326 | 368 | } | |
| 327 | 369 | ||
| 328 | 370 | #[test] | |
| @@ -330,10 +372,19 @@ | |||
| 330 | 372 | // The conversion's real risk, and the one that already shipped once: a | |
| 331 | 373 | // described control addressing a route registered nowhere renders fine | |
| 332 | 374 | // and answers 404 when pressed. S3 shipped exactly that. | |
| 333 | - | let api = include_str!("../routes/api/mod.rs"); | |
| 375 | + | // The revoke is this screen's own route now, so the check is that the | |
| 376 | + | // control and the registration agree rather than that an API path | |
| 377 | + | // exists. They are three lines apart and still drifted once. | |
| 334 | 378 | assert!( | |
| 335 | - | api.contains("/api/contacts/{seller_id}"), | |
| 336 | - | "the revoke address is a registered route" | |
| 379 | + | WRITES | |
| 380 | + | .iter() | |
| 381 | + | .any(|(method, path, _)| *method == Method::Delete && *path == REVOKE), | |
| 382 | + | "the revoke route is registered" | |
| 383 | + | ); | |
| 384 | + | let control = render(&shared_table(&[creator("s1", "grace", "Grace H")])); | |
| 385 | + | assert!( | |
| 386 | + | control.contains(&format!("hx-delete=\"{PATH}/revoke/s1\"")), | |
| 387 | + | "{control}" | |
| 337 | 388 | ); | |
| 338 | 389 | ||
| 339 | 390 | // The linked value is the same risk with no button to press: a title |
| @@ -147,19 +147,60 @@ | |||
| 147 | 147 | /// Askama routes exactly as before and the adapter is not in the stack at all. A | |
| 148 | 148 | /// conversion is a startup-time choice: config is read once, and a per-request | |
| 149 | 149 | /// branch would pay for a switch that never moves. | |
| 150 | + | /// Every address a described screen can claim, switched on or not. | |
| 151 | + | /// | |
| 152 | + | /// `mounts` returns only what is currently on, which depends on config. This is | |
| 153 | + | /// the whole set, and it exists for the CSRF coverage test: the manifest that | |
| 154 | + | /// test reads is a process-global, so a test that switches a screen on leaves an | |
| 155 | + | /// entry behind for a path the default router does not serve, and the probe | |
| 156 | + | /// reads that as a route that lost its protection. The list lets it skip exactly | |
| 157 | + | /// those and nothing else. | |
| 158 | + | /// | |
| 159 | + | /// Checked against `mounts` below rather than trusted, since a screen added to | |
| 160 | + | /// one and not the other is the obvious way for this to rot. | |
| 161 | + | pub const PATHS: &[&str] = &[ | |
| 162 | + | ssh_keys::PATH, | |
| 163 | + | library_contacts::PATH, | |
| 164 | + | buyer_contacts::PATH, | |
| 165 | + | user_analytics::PATH, | |
| 166 | + | forum_memberships::LIBRARY_PATH, | |
| 167 | + | forum_memberships::SETTINGS_PATH, | |
| 168 | + | ]; | |
| 169 | + | ||
| 170 | + | /// Every screen's switch name, in the same order as [`PATHS`]. | |
| 171 | + | /// | |
| 172 | + | /// Test-only: the switches themselves are read from each screen's own `SCREEN` | |
| 173 | + | /// in `mounts`, and this exists so the consistency check below has both halves | |
| 174 | + | /// to compare. Kept beside `PATHS` rather than inside the test module, because | |
| 175 | + | /// the pairing is the thing being asserted and splitting them is how they drift. | |
| 176 | + | #[cfg(test)] | |
| 177 | + | const SCREENS: &[&str] = &[ | |
| 178 | + | ssh_keys::SCREEN, | |
| 179 | + | library_contacts::SCREEN, | |
| 180 | + | buyer_contacts::SCREEN, | |
| 181 | + | user_analytics::SCREEN, | |
| 182 | + | forum_memberships::LIBRARY_SCREEN, | |
| 183 | + | forum_memberships::SETTINGS_SCREEN, | |
| 184 | + | ]; | |
| 185 | + | ||
| 150 | 186 | pub fn mounts(app: &AppState) -> Vec<(&'static str, axum::Router)> { | |
| 151 | 187 | let mut mounted = Vec::new(); | |
| 152 | 188 | ||
| 153 | 189 | if described(app, ssh_keys::SCREEN) { | |
| 154 | 190 | mounted.push(( | |
| 155 | 191 | ssh_keys::PATH, | |
| 156 | - | mount(app, ssh_keys::screen, ssh_keys::renderer), | |
| 192 | + | mount(app, ssh_keys::screen, ssh_keys::WRITES, ssh_keys::renderer), | |
| 157 | 193 | )); | |
| 158 | 194 | } | |
| 159 | 195 | if described(app, library_contacts::SCREEN) { | |
| 160 | 196 | mounted.push(( | |
| 161 | 197 | library_contacts::PATH, | |
| 162 | - | mount(app, library_contacts::screen, library_contacts::renderer), | |
| 198 | + | mount( | |
| 199 | + | app, | |
| 200 | + | library_contacts::screen, | |
| 201 | + | library_contacts::WRITES, | |
| 202 | + | library_contacts::renderer, | |
| 203 | + | ), | |
| 163 | 204 | )); | |
| 164 | 205 | } | |
| 165 | 206 | // One module, two screens: the library tab and the settings section are the | |
| @@ -170,6 +211,7 @@ | |||
| 170 | 211 | mount( | |
| 171 | 212 | app, | |
| 172 | 213 | forum_memberships::library_screen, | |
| 214 | + | &[], | |
| 173 | 215 | forum_memberships::renderer, | |
| 174 | 216 | ), | |
| 175 | 217 | )); | |
| @@ -177,13 +219,13 @@ | |||
| 177 | 219 | if described(app, buyer_contacts::SCREEN) { | |
| 178 | 220 | mounted.push(( | |
| 179 | 221 | buyer_contacts::PATH, | |
| 180 | - | mount(app, buyer_contacts::screen, buyer_contacts::renderer), | |
| 222 | + | mount(app, buyer_contacts::screen, &[], buyer_contacts::renderer), | |
| 181 | 223 | )); | |
| 182 | 224 | } | |
| 183 | 225 | if described(app, user_analytics::SCREEN) { | |
| 184 | 226 | mounted.push(( | |
| 185 | 227 | user_analytics::PATH, | |
| 186 | - | mount(app, user_analytics::screen, user_analytics::renderer), | |
| 228 | + | mount(app, user_analytics::screen, &[], user_analytics::renderer), | |
| 187 | 229 | )); | |
| 188 | 230 | } | |
| 189 | 231 | if described(app, forum_memberships::SETTINGS_SCREEN) { | |
| @@ -192,6 +234,7 @@ | |||
| 192 | 234 | mount( | |
| 193 | 235 | app, | |
| 194 | 236 | forum_memberships::settings_screen, | |
| 237 | + | &[], | |
| 195 | 238 | forum_memberships::renderer, | |
| 196 | 239 | ), | |
| 197 | 240 | )); | |
| @@ -200,20 +243,50 @@ | |||
| 200 | 243 | mounted | |
| 201 | 244 | } | |
| 202 | 245 | ||
| 246 | + | /// A handler, spelled once so the screens and the mount agree about it. | |
| 247 | + | pub type Screen = | |
| 248 | + | fn(&Viewer, quasi_router::Request) -> Result<quasi_router::Response, quasi_router::RouteError>; | |
| 249 | + | ||
| 203 | 250 | /// One screen behind the adapter, answering the root of its own nest. | |
| 204 | 251 | /// | |
| 205 | - | /// The path is `/` because the nest has already taken the address off: a screen | |
| 252 | + | /// The tab is `/` because the nest has already taken the address off: a screen | |
| 206 | 253 | /// mounted at its own tab endpoint sees one route and never has to agree with | |
| 207 | 254 | /// the prefix twice. | |
| 255 | + | /// | |
| 256 | + | /// # Why a screen serves its own writes | |
| 257 | + | /// | |
| 258 | + | /// `writes` registers routes under the same nest, and a destructive control on | |
| 259 | + | /// a described screen should address one of them rather than the API route the | |
| 260 | + | /// Askama version used. Decision 7 is the reason: a described route answers with | |
| 261 | + | /// a `Response::Fragment` naming the region it changed, so the answer lands where | |
| 262 | + | /// it belongs and carries the screen's own markup. | |
| 263 | + | /// | |
| 264 | + | /// An API route can do neither, and both described screens that called one were | |
| 265 | + | /// wrong in different ways. `DELETE /api/users/me/ssh-keys/{id}` answers an htmx | |
| 266 | + | /// request with the whole re-rendered Askama list, and with no target htmx put | |
| 267 | + | /// that table inside the button that was pressed. `DELETE /api/contacts/{id}` | |
| 268 | + | /// answers 204, which htmx is configured never to swap, so the row stayed on | |
| 269 | + | /// screen after a successful revoke. Both found 2026-08-11 by reading what the | |
| 270 | + | /// endpoints return; both were invisible to tests that check the address exists. | |
| 271 | + | /// | |
| 272 | + | /// The write still goes through the same CSRF envelope: `crate::csrf` nests this | |
| 273 | + | /// service under an Auto posture, and the core module attaches the token to | |
| 274 | + | /// every htmx request on the page. | |
| 208 | 275 | fn mount( | |
| 209 | 276 | app: &AppState, | |
| 210 | - | screen: fn( | |
| 211 | - | &Viewer, | |
| 212 | - | quasi_router::Request, | |
| 213 | - | ) -> Result<quasi_router::Response, quasi_router::RouteError>, | |
| 277 | + | screen: Screen, | |
| 278 | + | writes: &[(quasi_router::Method, &'static str, Screen)], | |
| 214 | 279 | renderer: fn(&Viewer) -> quasi_webview::Webview, | |
| 215 | 280 | ) -> axum::Router { | |
| 216 | - | let quasi = quasi_router::Router::<Viewer>::new().get("/", screen); | |
| 281 | + | let mut quasi = quasi_router::Router::<Viewer>::new().get("/", screen); | |
| 282 | + | for (method, path, handler) in writes { | |
| 283 | + | quasi = match method { | |
| 284 | + | quasi_router::Method::Delete => quasi.delete(path, *handler), | |
| 285 | + | quasi_router::Method::Put => quasi.put(path, *handler), | |
| 286 | + | quasi_router::Method::Get => quasi.get(path, *handler), | |
| 287 | + | quasi_router::Method::Post => quasi.post(path, *handler), | |
| 288 | + | }; | |
| 289 | + | } | |
| 217 | 290 | quasi_axum::Adapter::per_viewer(quasi, viewer_factory(app.clone()), move |viewer, _, _| { | |
| 218 | 291 | renderer(viewer) | |
| 219 | 292 | }) | |
| @@ -228,3 +301,52 @@ | |||
| 228 | 301 | pub fn described(app: &AppState, screen: &str) -> bool { | |
| 229 | 302 | app.config.quasi_screens.enabled(screen) | |
| 230 | 303 | } | |
| 304 | + | ||
| 305 | + | #[cfg(test)] | |
| 306 | + | mod tests { | |
| 307 | + | use super::*; | |
| 308 | + | ||
| 309 | + | #[test] | |
| 310 | + | fn every_screen_is_listed_in_paths() { | |
| 311 | + | // The two lists are written by hand and read by two different things, | |
| 312 | + | // so the check is that adding a screen to `mounts` and forgetting | |
| 313 | + | // `PATHS` fails here rather than silently weakening the CSRF coverage | |
| 314 | + | // probe's skip list. | |
| 315 | + | assert_eq!( | |
| 316 | + | PATHS.len(), | |
| 317 | + | SCREENS.len(), | |
| 318 | + | "PATHS and SCREENS describe the same screens" | |
| 319 | + | ); | |
| 320 | + | ||
| 321 | + | let source = include_str!("mod.rs"); | |
| 322 | + | let mounted = source | |
| 323 | + | .split_once("pub fn mounts(") | |
| 324 | + | .expect("mounts exists") | |
| 325 | + | .1 | |
| 326 | + | .split_once("\n}") | |
| 327 | + | .expect("mounts ends") | |
| 328 | + | .0; | |
| 329 | + | let registered = mounted.matches("mounted.push((").count(); | |
| 330 | + | assert_eq!( | |
| 331 | + | registered, | |
| 332 | + | PATHS.len(), | |
| 333 | + | "mounts registers {registered} screens, PATHS lists {}", | |
| 334 | + | PATHS.len() | |
| 335 | + | ); | |
| 336 | + | } | |
| 337 | + | ||
| 338 | + | #[test] | |
| 339 | + | fn a_path_is_claimed_by_exactly_one_screen() { | |
| 340 | + | // Two screens on one address is an axum panic at startup, and the two | |
| 341 | + | // forum-memberships screens are the near miss: one module, two paths. | |
| 342 | + | let mut seen = PATHS.to_vec(); | |
| 343 | + | seen.sort_unstable(); | |
| 344 | + | let before = seen.len(); | |
| 345 | + | seen.dedup(); | |
| 346 | + | assert_eq!( | |
| 347 | + | before, | |
| 348 | + | seen.len(), | |
| 349 | + | "two screens claim one address: {seen:?}" | |
| 350 | + | ); | |
| 351 | + | } | |
| 352 | + | } |
| @@ -39,7 +39,7 @@ | |||
| 39 | 39 | ||
| 40 | 40 | use makeover_layout as layout; | |
| 41 | 41 | use quasi_router::screen::{Act, Cell, Cells, Choice, Column, Field}; | |
| 42 | - | use quasi_router::{Action, Node, RegionKind, Request, Response, RouteError, Slot}; | |
| 42 | + | use quasi_router::{Action, Method, Node, RegionKind, Request, Response, RouteError, Slot}; | |
| 43 | 43 | use quasi_webview::{Shell, Webview}; | |
| 44 | 44 | ||
| 45 | 45 | use super::Viewer; | |
| @@ -59,6 +59,18 @@ | |||
| 59 | 59 | /// from which link was clicked, and the two agreeing is checked below. | |
| 60 | 60 | const REGION: &str = "settings-body"; | |
| 61 | 61 | ||
| 62 | + | /// The address removing a key calls, relative to this screen's own nest. | |
| 63 | + | const REMOVE_KEY: &str = "/keys/{id}"; | |
| 64 | + | ||
| 65 | + | /// The address revoking a token calls, relative to this screen's own nest. | |
| 66 | + | const REVOKE_TOKEN: &str = "/tokens/{id}"; | |
| 67 | + | ||
| 68 | + | /// The writes this screen serves. Registered under its nest by `super::mount`. | |
| 69 | + | pub const WRITES: &[(Method, &str, super::Screen)] = &[ | |
| 70 | + | (Method::Delete, REMOVE_KEY, remove_key), | |
| 71 | + | (Method::Delete, REVOKE_TOKEN, revoke_token), | |
| 72 | + | ]; | |
| 73 | + | ||
| 62 | 74 | /// One registered key, as the screen needs it. | |
| 63 | 75 | /// | |
| 64 | 76 | /// The description is built from these rather than from `db::DbSshKey` so the | |
| @@ -124,6 +136,52 @@ | |||
| 124 | 136 | )) | |
| 125 | 137 | } | |
| 126 | 138 | ||
| 139 | + | /// The id in the path, as the database wants it. | |
| 140 | + | fn captured(captures: &quasi_router::Params) -> Result<uuid::Uuid, RouteError> { | |
| 141 | + | captures | |
| 142 | + | .get("id") | |
| 143 | + | .and_then(|id| id.parse().ok()) | |
| 144 | + | .ok_or_else(|| RouteError::not_found("no such thing")) | |
| 145 | + | } | |
| 146 | + | ||
| 147 | + | /// Remove one key, and answer with the pane as it now stands. | |
| 148 | + | /// | |
| 149 | + | /// This screen's own route rather than `DELETE /api/users/me/ssh-keys/{id}`, | |
| 150 | + | /// which the Askama version calls. That endpoint answers an htmx request with | |
| 151 | + | /// the whole re-rendered Askama list and the Askama markup targeted | |
| 152 | + | /// `#ssh-keys-list`; the described control named no target, so htmx swapped that | |
| 153 | + | /// entire table into the button that was pressed. Found 2026-08-11 by reading | |
| 154 | + | /// what the endpoint returns. A described write answers with the region it | |
| 155 | + | /// changed, which is decision 7 working as designed. | |
| 156 | + | pub fn remove_key(viewer: &Viewer, request: Request) -> Result<Response, RouteError> { | |
| 157 | + | // Moved out because the handler signature is quasi's: the request is | |
| 158 | + | // consumed here rather than borrowed from. | |
| 159 | + | let captures = request.captures; | |
| 160 | + | let id = captured(&captures)?; | |
| 161 | + | viewer | |
| 162 | + | .block_on(db::ssh_keys::delete_key( | |
| 163 | + | &viewer.app.db, | |
| 164 | + | id.into(), | |
| 165 | + | viewer.user.id, | |
| 166 | + | )) | |
| 167 | + | .map_err(|_| RouteError::internal("that key could not be removed"))?; | |
| 168 | + | screen(viewer, Request::get(PATH)) | |
| 169 | + | } | |
| 170 | + | ||
| 171 | + | /// Revoke one token, and answer with the pane as it now stands. | |
| 172 | + | pub fn revoke_token(viewer: &Viewer, request: Request) -> Result<Response, RouteError> { | |
| 173 | + | let captures = request.captures; | |
| 174 | + | let id = captured(&captures)?; | |
| 175 | + | viewer | |
| 176 | + | .block_on(db::git_access_tokens::revoke( | |
| 177 | + | &viewer.app.db, | |
| 178 | + | id.into(), | |
| 179 | + | viewer.user.id, | |
| 180 | + | )) | |
| 181 | + | .map_err(|_| RouteError::internal("that token could not be revoked"))?; | |
| 182 | + | screen(viewer, Request::get(PATH)) | |
| 183 | + | } | |
| 184 | + | ||
| 127 | 185 | /// A date, or the word for not having one. | |
| 128 | 186 | fn never_or(at: Option<chrono::DateTime<chrono::Utc>>) -> String { | |
| 129 | 187 | at.map_or_else(|| "Never".to_owned(), |d| d.format("%b %d, %Y").to_string()) | |
| @@ -188,12 +246,11 @@ | |||
| 188 | 246 | Cell::new(format!("Added {}", key.added)), | |
| 189 | 247 | Cell::acts([Act::new( | |
| 190 | 248 | "Remove", | |
| 191 | - | // The route the server actually answers. This posted to | |
| 192 | - | // a `/delete` path invented because a described write | |
| 193 | - | // could only be a POST, and that path was registered | |
| 194 | - | // nowhere, so the button rendered and answered 404. | |
| 195 | - | // `61e1b069` closed the gap; quasi@bfe40fe. | |
| 196 | - | Action::delete(format!("/api/users/me/ssh-keys/{}", key.id)), | |
| 249 | + | // This screen's own route, under its own nest, so the | |
| 250 | + | // answer is the pane it changed. It addressed the API | |
| 251 | + | // route until 2026-08-11 and swapped a whole Askama | |
| 252 | + | // table into this button; see `remove_key`. | |
| 253 | + | Action::delete(format!("{PATH}/keys/{}", key.id)), | |
| 197 | 254 | ) | |
| 198 | 255 | // The template asked with hx-confirm. Said here, a terminal | |
| 199 | 256 | // host asks in its own way and no host can forget to ask. | |
| @@ -276,7 +333,7 @@ | |||
| 276 | 333 | Cell::new(token.last_used.clone()), | |
| 277 | 334 | Cell::acts([Act::new( | |
| 278 | 335 | "Revoke", | |
| 279 | - | Action::delete(format!("/api/users/me/git-tokens/{}", token.id)), | |
| 336 | + | Action::delete(format!("{PATH}/tokens/{}", token.id)), | |
| 280 | 337 | ) | |
| 281 | 338 | .confirm("Revoke this token?") | |
| 282 | 339 | .tone(layout::Tone::Danger)]), | |
| @@ -384,8 +441,8 @@ | |||
| 384 | 441 | assert_eq!(html.matches("hx-confirm").count(), 2, "both ask: {html}"); | |
| 385 | 442 | // The addresses are per key rather than one shared endpoint, which is | |
| 386 | 443 | // the mistake a loop over rows makes when the id is read outside it. | |
| 387 | - | assert!(html.contains("hx-delete=\"/api/users/me/ssh-keys/k1\"")); | |
| 388 | - | assert!(html.contains("hx-delete=\"/api/users/me/ssh-keys/k2\"")); | |
| 444 | + | assert!(html.contains(&format!("hx-delete=\"{PATH}/keys/k1\""))); | |
| 445 | + | assert!(html.contains(&format!("hx-delete=\"{PATH}/keys/k2\""))); | |
| 389 | 446 | } | |
| 390 | 447 | ||
| 391 | 448 | #[test] |
| @@ -85,6 +85,16 @@ | |||
| 85 | 85 | if REJECTED_BEFORE_CSRF_LAYER.contains(&entry.path.as_str()) { | |
| 86 | 86 | continue; | |
| 87 | 87 | } | |
| 88 | + | // A described screen mounts only when `QUASI_SCREENS` names it, and the | |
| 89 | + | // manifest is a process-global: a test elsewhere in this binary that | |
| 90 | + | // switches one on leaves an entry behind for a path this harness does | |
| 91 | + | // not serve, and every method then answers 405. Skipping them is not a | |
| 92 | + | // hole. `a_described_write_rejects_an_authenticated_tokenless_mutation` | |
| 93 | + | // below probes the same surface with the screen actually on, which is | |
| 94 | + | // the only configuration where the question means anything. | |
| 95 | + | if makenotwork::quasi::PATHS.contains(&entry.path.as_str()) { | |
| 96 | + | continue; | |
| 97 | + | } | |
| 88 | 98 | // Authenticated session + no CSRF token + no form content-type: | |
| 89 | 99 | // `validate_auto` must return 403 (Forbidden) before the handler runs. | |
| 90 | 100 | // Path params in the manifest (e.g. `/api/items/{id}`) still match the | |
| @@ -249,3 +259,93 @@ | |||
| 249 | 259 | resp.status | |
| 250 | 260 | ); | |
| 251 | 261 | } | |
| 262 | + | ||
| 263 | + | /// The description layer's mounts are inside the CSRF envelope, not beside it. | |
| 264 | + | /// | |
| 265 | + | /// They sat outside until 2026-08-11: `build_app` nested them after | |
| 266 | + | /// `with_state`, so neither the origin gate nor the token check reached them. | |
| 267 | + | /// That was harmless only while described screens served nothing but GET, and it | |
| 268 | + | /// stopped being harmless the moment one served its own `DELETE`. This asserts | |
| 269 | + | /// the structural fact rather than the behaviour, because the behaviour is only | |
| 270 | + | /// observable when a screen is switched on and `QUASI_SCREENS` is unset here and | |
| 271 | + | /// in every deployment. | |
| 272 | + | #[tokio::test] | |
| 273 | + | async fn described_screens_register_inside_the_csrf_envelope() { | |
| 274 | + | // Building the app is what populates the manifest. | |
| 275 | + | let _h = TestHarness::new().await; | |
| 276 | + | ||
| 277 | + | let manifest = route_manifest(); | |
| 278 | + | // With no screen switched on there is nothing to find, which is the state | |
| 279 | + | // this test runs in. What it pins is that `mounts` is wired through | |
| 280 | + | // `CsrfRouter::nest_service`, so any screen that switches on is covered: | |
| 281 | + | // an entry appears if and only if a mount did. | |
| 282 | + | for entry in &manifest { | |
| 283 | + | if entry.path.starts_with("/library/tabs/") || entry.path.starts_with("/dashboard/tabs/") { | |
| 284 | + | assert_eq!( | |
| 285 | + | entry.posture, | |
| 286 | + | ManifestPosture::Auto, | |
| 287 | + | "a described mount declared a posture other than Auto: {} ({:?})", | |
| 288 | + | entry.path, | |
| 289 | + | entry.reason | |
| 290 | + | ); | |
| 291 | + | } | |
| 292 | + | } | |
| 293 | + | ||
| 294 | + | // The seal that actually matters, and the one a future refactor would trip: | |
| 295 | + | // `CsrfRouter::nest_service` is the only way a service-shaped sub-tree gets | |
| 296 | + | // in, and it records Auto unconditionally. If someone reaches for axum's | |
| 297 | + | // `nest_service` on the finalized router again, the mount silently leaves | |
| 298 | + | // the envelope and nothing above notices, so the source is checked too. | |
| 299 | + | let lib = include_str!("../../src/lib.rs"); | |
| 300 | + | let mounts = lib | |
| 301 | + | .split_once("quasi::mounts(state)") | |
| 302 | + | .expect("build_app mounts the description layer") | |
| 303 | + | .1; | |
| 304 | + | let (mounted, _) = mounts | |
| 305 | + | .split_once(".finalize()") | |
| 306 | + | .expect("the CSRF tree is finalized after the mounts"); | |
| 307 | + | assert!( | |
| 308 | + | mounted.contains("routes.nest_service"), | |
| 309 | + | "the described mounts must go through CsrfRouter::nest_service and land \ | |
| 310 | + | before finalize, not on the plain router afterwards: see the CSRF \ | |
| 311 | + | envelope note in build_app" | |
| 312 | + | ); | |
| 313 | + | } | |
| 314 | + | ||
| 315 | + | /// A described write refuses a tokenless mutation, like every other write. | |
| 316 | + | /// | |
| 317 | + | /// The behavioural half of the test above. Switching a screen on is what makes | |
| 318 | + | /// the described sub-tree reachable, and `library_contacts` is the one that | |
| 319 | + | /// serves its own `DELETE`: it revokes contact sharing and answers with the tab | |
| 320 | + | /// as it now stands, rather than calling the API route whose 204 htmx never | |
| 321 | + | /// swapped. | |
| 322 | + | #[tokio::test] | |
| 323 | + | async fn a_described_write_rejects_an_authenticated_tokenless_mutation() { | |
| 324 | + | let mut h = TestHarness::build(crate::harness::BuildOptions { | |
| 325 | + | quasi_screens: makenotwork::config::QuasiScreens::parse("library_contacts"), | |
| 326 | + | ..Default::default() | |
| 327 | + | }) | |
| 328 | + | .await; | |
| 329 | + | h.signup("creator", "creator@example.com", "password123") | |
| 330 | + | .await; | |
| 331 | + | h.login("creator", "password123").await; | |
| 332 | + | ||
| 333 | + | // A real seller id is not needed: the CSRF layer wraps the whole nest, so it | |
| 334 | + | // answers before the router is consulted about whether the path exists. | |
| 335 | + | // That order is deliberate; see `CsrfRouter::nest_service`. | |
| 336 | + | let resp = h | |
| 337 | + | .client | |
| 338 | + | .request_with_headers( | |
| 339 | + | "DELETE", | |
| 340 | + | "/library/tabs/contacts/revoke/00000000-0000-0000-0000-000000000000", | |
| 341 | + | None, | |
| 342 | + | &[], | |
| 343 | + | ) | |
| 344 | + | .await; | |
| 345 | + | ||
| 346 | + | assert_eq!( | |
| 347 | + | resp.status, 403, | |
| 348 | + | "a described write accepted a tokenless mutation (got {})", | |
| 349 | + | resp.status | |
| 350 | + | ); | |
| 351 | + | } |