max / makenotwork
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
10 files changed,
+49 insertions,
-33 deletions
| @@ -23,13 +23,29 @@ | |||
| 23 | 23 | } | |
| 24 | 24 | ||
| 25 | 25 | /// Get the existing CSRF token from the session, or create and store a new one. | |
| 26 | - | pub async fn get_or_create_token(session: &Session) -> String { | |
| 26 | + | /// | |
| 27 | + | /// Returns a 500 response if the new token could not be written to the session | |
| 28 | + | /// store. Rendering a page around a token the session does not hold produces a | |
| 29 | + | /// form whose next POST fails validation for no reason the user can see, which | |
| 30 | + | /// is worse than an error: the middleware compares against the stored token, so | |
| 31 | + | /// an unstored token is a guaranteed silent failure later. Failing here keeps | |
| 32 | + | /// the failure where it happened. | |
| 33 | + | pub async fn get_or_create_token(session: &Session) -> Result<String, Response> { | |
| 27 | 34 | if let Ok(Some(token)) = session.get::<String>(CSRF_SESSION_KEY).await { | |
| 28 | - | return token; | |
| 35 | + | return Ok(token); | |
| 29 | 36 | } | |
| 30 | 37 | let token = generate_token(); | |
| 31 | - | let _ = session.insert(CSRF_SESSION_KEY, &token).await; | |
| 32 | - | token | |
| 38 | + | match session.insert(CSRF_SESSION_KEY, &token).await { | |
| 39 | + | Ok(()) => Ok(token), | |
| 40 | + | Err(e) => { | |
| 41 | + | tracing::error!(error = ?e, "failed to store CSRF token in session"); | |
| 42 | + | Err(( | |
| 43 | + | StatusCode::INTERNAL_SERVER_ERROR, | |
| 44 | + | "Session store unavailable", | |
| 45 | + | ) | |
| 46 | + | .into_response()) | |
| 47 | + | } | |
| 48 | + | } | |
| 33 | 49 | } | |
| 34 | 50 | ||
| 35 | 51 | /// Constant-time comparison to prevent timing attacks. |
| @@ -29,7 +29,7 @@ | |||
| 29 | 29 | session: Session, | |
| 30 | 30 | RequireUser(user): RequireUser, | |
| 31 | 31 | ) -> Result<AccountSettingsTemplate, Response> { | |
| 32 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 32 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 33 | 33 | let (signature_markdown, signature_html) = | |
| 34 | 34 | mt_db::queries::get_user_signature(&state.db, user.user_id) | |
| 35 | 35 | .await |
| @@ -30,7 +30,7 @@ | |||
| 30 | 30 | PlatformAdmin(admin): PlatformAdmin, | |
| 31 | 31 | Query(query): Query<AdminSearchQuery>, | |
| 32 | 32 | ) -> Result<impl IntoResponse, Response> { | |
| 33 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 33 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 34 | 34 | ||
| 35 | 35 | // Cap the listing and surface overflow rather than silently truncating | |
| 36 | 36 | // (mirrors the moderation lists' CAP+1). Fetch CAP+1 so we can tell whether | |
| @@ -224,7 +224,7 @@ | |||
| 224 | 224 | PlatformAdmin(admin): PlatformAdmin, | |
| 225 | 225 | Path(slug): Path<String>, | |
| 226 | 226 | ) -> Result<AdminCommunityTemplate, Response> { | |
| 227 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 227 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 228 | 228 | let community = get_community(&state.db, &slug).await?; | |
| 229 | 229 | ||
| 230 | 230 | let thread_count = mt_db::queries::count_threads_in_community(&state.db, community.id) |
| @@ -20,7 +20,7 @@ | |||
| 20 | 20 | use axum::{ | |
| 21 | 21 | Json, Router, | |
| 22 | 22 | http::StatusCode, | |
| 23 | - | response::IntoResponse, | |
| 23 | + | response::{IntoResponse, Response}, | |
| 24 | 24 | routing::{get, post}, | |
| 25 | 25 | }; | |
| 26 | 26 | use serde::Deserialize; | |
| @@ -555,19 +555,19 @@ | |||
| 555 | 555 | axum::extract::State(state): axum::extract::State<AppState>, | |
| 556 | 556 | session: Session, | |
| 557 | 557 | MaybeUser(session_user): MaybeUser, | |
| 558 | - | ) -> impl IntoResponse { | |
| 559 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 558 | + | ) -> Result<impl IntoResponse, Response> { | |
| 559 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 560 | 560 | let session_user = session_user | |
| 561 | 561 | .as_ref() | |
| 562 | 562 | .map(|u| template_user(u, state.config.platform_admin_id)); | |
| 563 | - | ( | |
| 563 | + | Ok(( | |
| 564 | 564 | StatusCode::NOT_FOUND, | |
| 565 | 565 | Error404Template { | |
| 566 | 566 | csrf_token, | |
| 567 | 567 | session_user, | |
| 568 | 568 | mnw_base_url: state.config.mnw_base_url.clone(), | |
| 569 | 569 | }, | |
| 570 | - | ) | |
| 570 | + | )) | |
| 571 | 571 | } | |
| 572 | 572 | ||
| 573 | 573 | #[cfg(test)] |
| @@ -259,7 +259,7 @@ | |||
| 259 | 259 | session: Session, | |
| 260 | 260 | RequireUser(user): RequireUser, | |
| 261 | 261 | ) -> Result<impl IntoResponse, Response> { | |
| 262 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 262 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 263 | 263 | // `require_mod_or_owner` already 403s a suspended community. | |
| 264 | 264 | let (community, role) = require_mod_or_owner(&state, &slug, &user).await?; | |
| 265 | 265 | ||
| @@ -550,7 +550,7 @@ | |||
| 550 | 550 | session: Session, | |
| 551 | 551 | RequireUser(user): RequireUser, | |
| 552 | 552 | ) -> Result<impl IntoResponse, Response> { | |
| 553 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 553 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 554 | 554 | let (community, _role) = require_mod_or_owner(&state, &slug, &user).await?; | |
| 555 | 555 | ||
| 556 | 556 | // Same cap and truncation signal as the bans/flags reads on the moderation | |
| @@ -661,7 +661,7 @@ | |||
| 661 | 661 | session: Session, | |
| 662 | 662 | RequireUser(user): RequireUser, | |
| 663 | 663 | ) -> Result<impl IntoResponse, Response> { | |
| 664 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 664 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 665 | 665 | let (community, _role) = require_mod_or_owner(&state, &slug, &user).await?; | |
| 666 | 666 | ||
| 667 | 667 | let per_page: i64 = 50; |
| @@ -31,7 +31,7 @@ | |||
| 31 | 31 | session: Session, | |
| 32 | 32 | RequireUser(user): RequireUser, | |
| 33 | 33 | ) -> Result<impl IntoResponse, Response> { | |
| 34 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 34 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 35 | 35 | // `require_owner` already 403s a suspended community. | |
| 36 | 36 | let community = require_owner(&state, &slug, &user).await?; | |
| 37 | 37 | ||
| @@ -213,7 +213,7 @@ | |||
| 213 | 213 | session: Session, | |
| 214 | 214 | RequireUser(user): RequireUser, | |
| 215 | 215 | ) -> Result<impl IntoResponse, Response> { | |
| 216 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 216 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 217 | 217 | let community = require_owner(&state, &slug, &user).await?; | |
| 218 | 218 | ||
| 219 | 219 | let cat_id = parse_uuid(&cat_id_str)?; |
| @@ -80,16 +80,16 @@ | |||
| 80 | 80 | axum::extract::State(state): axum::extract::State<AppState>, | |
| 81 | 81 | session: Session, | |
| 82 | 82 | MaybeUser(session_user): MaybeUser, | |
| 83 | - | ) -> impl IntoResponse { | |
| 84 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 83 | + | ) -> Result<impl IntoResponse, Response> { | |
| 84 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 85 | 85 | let session_user = session_user | |
| 86 | 86 | .as_ref() | |
| 87 | 87 | .map(|u| template_user(u, state.config.platform_admin_id)); | |
| 88 | - | TrackingInfoTemplate { | |
| 88 | + | Ok(TrackingInfoTemplate { | |
| 89 | 89 | csrf_token, | |
| 90 | 90 | session_user, | |
| 91 | 91 | mnw_base_url: state.config.mnw_base_url.clone(), | |
| 92 | - | } | |
| 92 | + | }) | |
| 93 | 93 | } | |
| 94 | 94 | ||
| 95 | 95 | /// GET /tracked, tracked threads page | |
| @@ -100,7 +100,7 @@ | |||
| 100 | 100 | Query(query): Query<super::ForumDirectoryQuery>, | |
| 101 | 101 | RequireUser(user): RequireUser, | |
| 102 | 102 | ) -> Result<impl IntoResponse, Response> { | |
| 103 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 103 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 104 | 104 | const PER_PAGE: i64 = 50; | |
| 105 | 105 | let total = mt_db::queries::count_tracked_threads(&state.db, user.user_id) | |
| 106 | 106 | .await |
| @@ -406,7 +406,7 @@ | |||
| 406 | 406 | session: tower_sessions::Session, | |
| 407 | 407 | RequireUser(user): RequireUser, | |
| 408 | 408 | ) -> Result<impl IntoResponse, Response> { | |
| 409 | - | let csrf_token = Some(crate::csrf::get_or_create_token(&session).await); | |
| 409 | + | let csrf_token = Some(crate::csrf::get_or_create_token(&session).await?); | |
| 410 | 410 | let scope = | |
| 411 | 411 | CommunityScope::<ThreadWithBreadcrumb>::resolve(&state.db, &slug, &thread_id_str).await?; | |
| 412 | 412 | scope.require_write_access(&state.db, user.user_id).await?; |
| @@ -27,7 +27,7 @@ | |||
| 27 | 27 | session: Session, | |
| 28 | 28 | MaybeUser(session_user): MaybeUser, | |
| 29 | 29 | ) -> Result<impl IntoResponse, Response> { | |
| 30 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 30 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 31 | 31 | ||
| 32 | 32 | let scope = | |
| 33 | 33 | CommunityScope::<ThreadWithBreadcrumb>::resolve(&state.db, &slug, &thread_id).await?; |
| @@ -36,8 +36,8 @@ | |||
| 36 | 36 | Query(query): Query<ForumDirectoryQuery>, | |
| 37 | 37 | session: Session, | |
| 38 | 38 | MaybeUser(session_user): MaybeUser, | |
| 39 | - | ) -> impl IntoResponse { | |
| 40 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 39 | + | ) -> Result<impl IntoResponse, Response> { | |
| 40 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 41 | 41 | let viewing_archived = query.filter.as_deref() == Some("archived"); | |
| 42 | 42 | ||
| 43 | 43 | let per_page: i64 = 25; | |
| @@ -82,14 +82,14 @@ | |||
| 82 | 82 | .as_ref() | |
| 83 | 83 | .map(|u| template_user(u, state.config.platform_admin_id)); | |
| 84 | 84 | ||
| 85 | - | ForumDirectoryTemplate { | |
| 85 | + | Ok(ForumDirectoryTemplate { | |
| 86 | 86 | csrf_token, | |
| 87 | 87 | session_user, | |
| 88 | 88 | mnw_base_url: state.config.mnw_base_url.clone(), | |
| 89 | 89 | communities, | |
| 90 | 90 | pagination, | |
| 91 | 91 | viewing_archived, | |
| 92 | - | } | |
| 92 | + | }) | |
| 93 | 93 | } | |
| 94 | 94 | ||
| 95 | 95 | /// Project forum, categories within a project. | |
| @@ -100,7 +100,7 @@ | |||
| 100 | 100 | session: Session, | |
| 101 | 101 | MaybeUser(session_user): MaybeUser, | |
| 102 | 102 | ) -> Result<impl IntoResponse, Response> { | |
| 103 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 103 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 104 | 104 | let community = get_community(&state.db, &slug).await?; | |
| 105 | 105 | ||
| 106 | 106 | check_community_access( | |
| @@ -157,7 +157,7 @@ | |||
| 157 | 157 | session: Session, | |
| 158 | 158 | MaybeUser(session_user): MaybeUser, | |
| 159 | 159 | ) -> Result<impl IntoResponse, Response> { | |
| 160 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 160 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 161 | 161 | let community = get_community(&state.db, &slug).await?; | |
| 162 | 162 | ||
| 163 | 163 | check_community_access( | |
| @@ -212,7 +212,7 @@ | |||
| 212 | 212 | session: Session, | |
| 213 | 213 | MaybeUser(session_user): MaybeUser, | |
| 214 | 214 | ) -> Result<impl IntoResponse, Response> { | |
| 215 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 215 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 216 | 216 | let per_page: i64 = 25; | |
| 217 | 217 | ||
| 218 | 218 | // Parse sort column, only allow known values, default to "activity" | |
| @@ -382,7 +382,7 @@ | |||
| 382 | 382 | session: Session, | |
| 383 | 383 | MaybeUser(session_user): MaybeUser, | |
| 384 | 384 | ) -> Result<impl IntoResponse, Response> { | |
| 385 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 385 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 386 | 386 | let community = get_community(&state.db, &slug).await?; | |
| 387 | 387 | ||
| 388 | 388 | // Check suspension + ban for logged-in users (form display only; POST enforces fully) | |
| @@ -434,7 +434,7 @@ | |||
| 434 | 434 | session: Session, | |
| 435 | 435 | MaybeUser(session_user): MaybeUser, | |
| 436 | 436 | ) -> Result<impl IntoResponse, Response> { | |
| 437 | - | let csrf_token = Some(csrf::get_or_create_token(&session).await); | |
| 437 | + | let csrf_token = Some(csrf::get_or_create_token(&session).await?); | |
| 438 | 438 | let community = get_community(&state.db, &slug).await?; | |
| 439 | 439 | ||
| 440 | 440 | check_community_access( |