max / makenotwork
19 files changed,
+305 insertions,
-148 deletions
| @@ -0,0 +1,15 @@ | |||
| 1 | + | { | |
| 2 | + | "db_name": "PostgreSQL", | |
| 3 | + | "query": "DELETE FROM user_sessions WHERE id = $1 AND user_id = $2 AND kind = 'pending_2fa'", | |
| 4 | + | "describe": { | |
| 5 | + | "columns": [], | |
| 6 | + | "parameters": { | |
| 7 | + | "Left": [ | |
| 8 | + | "Uuid", | |
| 9 | + | "Uuid" | |
| 10 | + | ] | |
| 11 | + | }, | |
| 12 | + | "nullable": [] | |
| 13 | + | }, | |
| 14 | + | "hash": "37fe22d8f1b67526c177e01df340d821303d4380caa18b31bd5bc6d37bff0166" | |
| 15 | + | } |
| @@ -1,14 +0,0 @@ | |||
| 1 | - | { | |
| 2 | - | "db_name": "PostgreSQL", | |
| 3 | - | "query": "DELETE FROM user_sessions WHERE id = $1 AND kind = 'pending_2fa'", | |
| 4 | - | "describe": { | |
| 5 | - | "columns": [], | |
| 6 | - | "parameters": { | |
| 7 | - | "Left": [ | |
| 8 | - | "Uuid" | |
| 9 | - | ] | |
| 10 | - | }, | |
| 11 | - | "nullable": [] | |
| 12 | - | }, | |
| 13 | - | "hash": "4ae1e1059770f170c60e6268fae20313ed3f0dddc4e02df751f4f0268ce40ec9" | |
| 14 | - | } |
| @@ -0,0 +1,15 @@ | |||
| 1 | + | { | |
| 2 | + | "db_name": "PostgreSQL", | |
| 3 | + | "query": "\n DELETE FROM user_sessions\n WHERE user_id = $1 AND kind = 'active'\n AND id NOT IN (\n SELECT id FROM user_sessions\n WHERE user_id = $1 AND kind = 'active'\n ORDER BY last_active_at DESC, id DESC\n LIMIT $2\n )\n ", | |
| 4 | + | "describe": { | |
| 5 | + | "columns": [], | |
| 6 | + | "parameters": { | |
| 7 | + | "Left": [ | |
| 8 | + | "Uuid", | |
| 9 | + | "Int8" | |
| 10 | + | ] | |
| 11 | + | }, | |
| 12 | + | "nullable": [] | |
| 13 | + | }, | |
| 14 | + | "hash": "610c5a3c538abcb94268bcec240364cb36b5e690a82a7e8811e9ff40d45c06de" | |
| 15 | + | } |
| @@ -1,15 +0,0 @@ | |||
| 1 | - | { | |
| 2 | - | "db_name": "PostgreSQL", | |
| 3 | - | "query": "\n DELETE FROM user_sessions\n WHERE user_id = $1 AND kind = 'active'\n AND id NOT IN (\n SELECT id FROM user_sessions\n WHERE user_id = $1 AND kind = 'active'\n ORDER BY last_active_at DESC\n LIMIT $2\n )\n ", | |
| 4 | - | "describe": { | |
| 5 | - | "columns": [], | |
| 6 | - | "parameters": { | |
| 7 | - | "Left": [ | |
| 8 | - | "Uuid", | |
| 9 | - | "Int8" | |
| 10 | - | ] | |
| 11 | - | }, | |
| 12 | - | "nullable": [] | |
| 13 | - | }, | |
| 14 | - | "hash": "e628881689b873891be57b389ec3e7cebb8c5a518053a1aeea2664f9e86a4feb" | |
| 15 | - | } |
| @@ -343,12 +343,43 @@ impl FromRequestParts<crate::AppState> for MaybeUserVerified { | |||
| 343 | 343 | } | |
| 344 | 344 | } | |
| 345 | 345 | ||
| 346 | + | /// Proof that an admin identity was established by the [`AdminUser`] extractor. | |
| 347 | + | /// | |
| 348 | + | /// The inner `UserId` is private to this module and the only public constructor | |
| 349 | + | /// is [`AdminUser::admin_id`], so an `AdminId` cannot exist without having passed | |
| 350 | + | /// the `require_admin` gate. DB writers that stamp an actor (`moderation_actions`, | |
| 351 | + | /// `report.resolved_by`) take `AdminId` instead of a bare `UserId`, making a | |
| 352 | + | /// forged or caller-supplied admin id un-constructible at the type level rather | |
| 353 | + | /// than relying on every route to remember the guard (ultra-fuzz Run 11 Sec M2). | |
| 354 | + | #[derive(Clone, Copy, Debug)] | |
| 355 | + | pub struct AdminId(UserId); | |
| 356 | + | ||
| 357 | + | impl AdminId { | |
| 358 | + | /// The witnessed admin user id, for binding into a query. | |
| 359 | + | pub fn get(self) -> UserId { | |
| 360 | + | self.0 | |
| 361 | + | } | |
| 362 | + | } | |
| 363 | + | ||
| 346 | 364 | /// Extractor for admin users - returns NotFound (hides admin routes) if not admin. | |
| 347 | 365 | /// | |
| 348 | 366 | /// Combines `AuthUser` session check with `require_admin` config check into a | |
| 349 | 367 | /// single type-safe extractor, eliminating per-handler `require_admin()` calls. | |
| 350 | 368 | pub struct AdminUser(pub SessionUser); | |
| 351 | 369 | ||
| 370 | + | impl AdminUser { | |
| 371 | + | /// Mint the [`AdminId`] witness for this verified admin. The only way to | |
| 372 | + | /// obtain an `AdminId` — its private field can't be constructed elsewhere. | |
| 373 | + | pub fn admin_id(&self) -> AdminId { | |
| 374 | + | AdminId(self.0.id) | |
| 375 | + | } | |
| 376 | + | ||
| 377 | + | /// The admin's plain `UserId`, for tracing/display (not a write witness). | |
| 378 | + | pub fn id(&self) -> UserId { | |
| 379 | + | self.0.id | |
| 380 | + | } | |
| 381 | + | } | |
| 382 | + | ||
| 352 | 383 | impl FromRequestParts<crate::AppState> for AdminUser { | |
| 353 | 384 | type Rejection = AppError; | |
| 354 | 385 | ||
| @@ -489,6 +520,57 @@ pub async fn verify_password_async(password: String, hash: String) -> Result<boo | |||
| 489 | 520 | .map_err(|e| AppError::Internal(anyhow::anyhow!("argon2 verify task join: {e}")))? | |
| 490 | 521 | } | |
| 491 | 522 | ||
| 523 | + | /// Outcome of [`relying_party_login_gate`]. | |
| 524 | + | pub enum LoginGate { | |
| 525 | + | /// Password correct and the account may complete a relying-party login. | |
| 526 | + | Allow, | |
| 527 | + | /// Login refused. `just_locked` is true when *this* attempt tripped the | |
| 528 | + | /// lockout (so an interactive caller can show a one-time lockout notice); | |
| 529 | + | /// every other refusal reason is indistinguishable by design. | |
| 530 | + | Deny { just_locked: bool }, | |
| 531 | + | } | |
| 532 | + | ||
| 533 | + | /// Uniform password + account-status gate for relying-party logins (OAuth | |
| 534 | + | /// authorize, SyncKit auth) — the flows that reject 2FA accounts outright. | |
| 535 | + | /// | |
| 536 | + | /// Runs Argon2 first, then folds *every* refusal reason (wrong password, | |
| 537 | + | /// suspended, deactivated, locked, 2FA-enabled) into a single accounted | |
| 538 | + | /// decision: a denial always increments the failed-login counter, a success | |
| 539 | + | /// always resets it. Collapsing the blocked-account cases into the wrong-password | |
| 540 | + | /// path is what stops the counter from becoming a confirmed-password oracle — a | |
| 541 | + | /// correct guess against a 2FA/suspended account must be indistinguishable from a | |
| 542 | + | /// wrong one (ultra-fuzz Run 3 / Run 11 Sec M1). Both relying parties call this | |
| 543 | + | /// instead of open-coding the ordering, so the invariant lives in one place. | |
| 544 | + | pub async fn relying_party_login_gate( | |
| 545 | + | pool: &sqlx::PgPool, | |
| 546 | + | user: &db::DbUser, | |
| 547 | + | password: &str, | |
| 548 | + | ) -> Result<LoginGate, AppError> { | |
| 549 | + | let valid = verify_password_async(password.to_string(), user.password_hash.clone()).await?; | |
| 550 | + | ||
| 551 | + | let locked = user | |
| 552 | + | .locked_until | |
| 553 | + | .is_some_and(|locked_until| locked_until > chrono::Utc::now()); | |
| 554 | + | let denied = | |
| 555 | + | !valid || user.is_suspended() || user.is_deactivated() || locked || user.totp_enabled; | |
| 556 | + | ||
| 557 | + | if denied { | |
| 558 | + | let result = db::auth::increment_failed_login( | |
| 559 | + | pool, | |
| 560 | + | user.id, | |
| 561 | + | constants::MAX_LOGIN_ATTEMPTS, | |
| 562 | + | constants::LOCKOUT_MINUTES, | |
| 563 | + | ) | |
| 564 | + | .await?; | |
| 565 | + | return Ok(LoginGate::Deny { | |
| 566 | + | just_locked: result.just_locked, | |
| 567 | + | }); | |
| 568 | + | } | |
| 569 | + | ||
| 570 | + | db::auth::reset_failed_login(pool, user.id).await?; | |
| 571 | + | Ok(LoginGate::Allow) | |
| 572 | + | } | |
| 573 | + | ||
| 492 | 574 | /// Store user in session with session regeneration to prevent fixation attacks | |
| 493 | 575 | #[tracing::instrument(skip_all, fields(user_id = %user.id))] | |
| 494 | 576 | pub async fn login_user(session: &Session, user: SessionUser) -> Result<(), AppError> { |
| @@ -25,7 +25,7 @@ pub struct DbModerationAction { | |||
| 25 | 25 | pub async fn create_action( | |
| 26 | 26 | pool: &PgPool, | |
| 27 | 27 | user_id: UserId, | |
| 28 | - | admin_id: UserId, | |
| 28 | + | admin_id: crate::auth::AdminId, | |
| 29 | 29 | action_type: ModerationActionType, | |
| 30 | 30 | reason: &str, | |
| 31 | 31 | content_ref: Option<&str>, | |
| @@ -38,7 +38,7 @@ pub async fn create_action( | |||
| 38 | 38 | "#, | |
| 39 | 39 | ) | |
| 40 | 40 | .bind(user_id) | |
| 41 | - | .bind(admin_id) | |
| 41 | + | .bind(admin_id.get()) | |
| 42 | 42 | .bind(action_type) | |
| 43 | 43 | .bind(reason) | |
| 44 | 44 | .bind(content_ref) |
| @@ -98,7 +98,7 @@ pub async fn resolve_report( | |||
| 98 | 98 | id: ReportId, | |
| 99 | 99 | status: ReportStatus, | |
| 100 | 100 | admin_notes: &str, | |
| 101 | - | resolved_by: UserId, | |
| 101 | + | resolved_by: crate::auth::AdminId, | |
| 102 | 102 | ) -> Result<()> { | |
| 103 | 103 | let result = sqlx::query( | |
| 104 | 104 | r#" | |
| @@ -110,7 +110,7 @@ pub async fn resolve_report( | |||
| 110 | 110 | .bind(id) | |
| 111 | 111 | .bind(status) | |
| 112 | 112 | .bind(admin_notes) | |
| 113 | - | .bind(resolved_by) | |
| 113 | + | .bind(resolved_by.get()) | |
| 114 | 114 | .execute(pool) | |
| 115 | 115 | .await?; | |
| 116 | 116 |
| @@ -32,6 +32,10 @@ pub async fn create_user_session( | |||
| 32 | 32 | /// intermediates are never touched. Ordered by `last_active_at DESC`, so the | |
| 33 | 33 | /// freshly-created current session (newest) is always kept and the stalest are | |
| 34 | 34 | /// evicted. Returns the number of rows pruned. | |
| 35 | + | /// | |
| 36 | + | /// The keep-set is ordered `last_active_at DESC, id DESC` — the `id` tie-break | |
| 37 | + | /// makes the eviction deterministic when several sessions share a timestamp, | |
| 38 | + | /// instead of leaving the choice to Postgres' undefined row order (Run 11 Sec LOW). | |
| 35 | 39 | #[tracing::instrument(skip_all)] | |
| 36 | 40 | pub async fn prune_user_sessions_over_cap( | |
| 37 | 41 | pool: &PgPool, | |
| @@ -45,7 +49,7 @@ pub async fn prune_user_sessions_over_cap( | |||
| 45 | 49 | AND id NOT IN ( | |
| 46 | 50 | SELECT id FROM user_sessions | |
| 47 | 51 | WHERE user_id = $1 AND kind = 'active' | |
| 48 | - | ORDER BY last_active_at DESC | |
| 52 | + | ORDER BY last_active_at DESC, id DESC | |
| 49 | 53 | LIMIT $2 | |
| 50 | 54 | ) | |
| 51 | 55 | "#, | |
| @@ -103,11 +107,19 @@ pub async fn pending_2fa_session_exists( | |||
| 103 | 107 | /// Delete a pending_2fa tracking row. Called when 2FA succeeds (the caller | |
| 104 | 108 | /// then `track_session`s a fresh 'active' row) or when the pending state is | |
| 105 | 109 | /// cleared (expiry, account lockout, navigation away). | |
| 110 | + | /// | |
| 111 | + | /// Scoped by `user_id` (not id alone) so a guessed/enumerated session id can't | |
| 112 | + | /// delete another user's pending_2fa row (ultra-fuzz Run 11 Sec LOW). | |
| 106 | 113 | #[tracing::instrument(skip_all)] | |
| 107 | - | pub async fn delete_pending_2fa_session(pool: &PgPool, id: UserSessionId) -> Result<()> { | |
| 114 | + | pub async fn delete_pending_2fa_session( | |
| 115 | + | pool: &PgPool, | |
| 116 | + | id: UserSessionId, | |
| 117 | + | user_id: UserId, | |
| 118 | + | ) -> Result<()> { | |
| 108 | 119 | sqlx::query!( | |
| 109 | - | "DELETE FROM user_sessions WHERE id = $1 AND kind = 'pending_2fa'", | |
| 120 | + | "DELETE FROM user_sessions WHERE id = $1 AND user_id = $2 AND kind = 'pending_2fa'", | |
| 110 | 121 | id as UserSessionId, | |
| 122 | + | user_id as UserId, | |
| 111 | 123 | ) | |
| 112 | 124 | .execute(pool) | |
| 113 | 125 | .await?; |
| @@ -167,7 +167,7 @@ pub(super) struct ReportDecisionForm { | |||
| 167 | 167 | #[tracing::instrument(skip_all, name = "admin::admin_resolve_report")] | |
| 168 | 168 | pub(super) async fn admin_resolve_report( | |
| 169 | 169 | State(state): State<AppState>, | |
| 170 | - | AdminUser(admin): AdminUser, | |
| 170 | + | admin_user: AdminUser, | |
| 171 | 171 | Path(id): Path<ReportId>, | |
| 172 | 172 | Form(form): Form<ReportDecisionForm>, | |
| 173 | 173 | ) -> Result<impl IntoResponse> { | |
| @@ -182,7 +182,7 @@ pub(super) async fn admin_resolve_report( | |||
| 182 | 182 | id, | |
| 183 | 183 | status, | |
| 184 | 184 | form.admin_notes.trim(), | |
| 185 | - | admin.id, | |
| 185 | + | admin_user.admin_id(), | |
| 186 | 186 | ).await?; | |
| 187 | 187 | ||
| 188 | 188 | tracing::info!(report_id = %id, decision = %form.decision, "admin resolved report"); | |
| @@ -207,7 +207,7 @@ pub(super) struct ItemRemovalForm { | |||
| 207 | 207 | #[tracing::instrument(skip_all, name = "admin::admin_remove_item")] | |
| 208 | 208 | pub(super) async fn admin_remove_item( | |
| 209 | 209 | State(state): State<AppState>, | |
| 210 | - | AdminUser(admin): AdminUser, | |
| 210 | + | admin_user: AdminUser, | |
| 211 | 211 | Path(item_id): Path<ItemId>, | |
| 212 | 212 | Form(form): Form<ItemRemovalForm>, | |
| 213 | 213 | ) -> Result<impl IntoResponse> { | |
| @@ -235,12 +235,12 @@ pub(super) async fn admin_remove_item( | |||
| 235 | 235 | ||
| 236 | 236 | // Record moderation action against the item owner | |
| 237 | 237 | db::moderation::create_action( | |
| 238 | - | &state.db, owner_id, admin.id, ModerationActionType::ContentRemoval, reason, Some(&item_id.to_string()), | |
| 238 | + | &state.db, owner_id, admin_user.admin_id(), ModerationActionType::ContentRemoval, reason, Some(&item_id.to_string()), | |
| 239 | 239 | ).await?; | |
| 240 | 240 | ||
| 241 | 241 | tracing::info!( | |
| 242 | 242 | item_id = %item_id, | |
| 243 | - | admin_id = %admin.id, | |
| 243 | + | admin_id = %admin_user.id(), | |
| 244 | 244 | reason = %reason, | |
| 245 | 245 | "admin removed item" | |
| 246 | 246 | ); | |
| @@ -252,7 +252,7 @@ pub(super) async fn admin_remove_item( | |||
| 252 | 252 | #[tracing::instrument(skip_all, name = "admin::admin_restore_item")] | |
| 253 | 253 | pub(super) async fn admin_restore_item( | |
| 254 | 254 | State(state): State<AppState>, | |
| 255 | - | AdminUser(admin): AdminUser, | |
| 255 | + | admin_user: AdminUser, | |
| 256 | 256 | Path(item_id): Path<ItemId>, | |
| 257 | 257 | ) -> Result<impl IntoResponse> { | |
| 258 | 258 | let item = db::items::admin_restore_item(&state.db, item_id).await?; | |
| @@ -276,7 +276,7 @@ pub(super) async fn admin_restore_item( | |||
| 276 | 276 | ||
| 277 | 277 | tracing::info!( | |
| 278 | 278 | item_id = %item_id, | |
| 279 | - | admin_id = %admin.id, | |
| 279 | + | admin_id = %admin_user.id(), | |
| 280 | 280 | "admin restored item" | |
| 281 | 281 | ); | |
| 282 | 282 |
| @@ -110,7 +110,7 @@ pub(super) struct SuspendForm { | |||
| 110 | 110 | #[tracing::instrument(skip_all, name = "admin::admin_warn_user")] | |
| 111 | 111 | pub(super) async fn admin_warn_user( | |
| 112 | 112 | State(state): State<AppState>, | |
| 113 | - | AdminUser(admin): AdminUser, | |
| 113 | + | admin_user: AdminUser, | |
| 114 | 114 | Path(id): Path<UserId>, | |
| 115 | 115 | Form(form): Form<SuspendForm>, | |
| 116 | 116 | ) -> Result<impl IntoResponse> { | |
| @@ -124,7 +124,7 @@ pub(super) async fn admin_warn_user( | |||
| 124 | 124 | .ok_or(AppError::NotFound)?; | |
| 125 | 125 | ||
| 126 | 126 | // Record warning in moderation history | |
| 127 | - | db::moderation::create_action(&state.db, id, admin.id, ModerationActionType::Warning, reason, None).await?; | |
| 127 | + | db::moderation::create_action(&state.db, id, admin_user.admin_id(), ModerationActionType::Warning, reason, None).await?; | |
| 128 | 128 | ||
| 129 | 129 | // Send warning email | |
| 130 | 130 | if let Err(e) = state.email | |
| @@ -134,7 +134,7 @@ pub(super) async fn admin_warn_user( | |||
| 134 | 134 | tracing::error!(error = ?e, user_id = %id, "failed to send warning email"); | |
| 135 | 135 | } | |
| 136 | 136 | ||
| 137 | - | tracing::info!(user_id = %id, admin_id = %admin.id, reason = %reason, "admin sent policy warning"); | |
| 137 | + | tracing::info!(user_id = %id, admin_id = %admin_user.id(), reason = %reason, "admin sent policy warning"); | |
| 138 | 138 | ||
| 139 | 139 | refresh_user_entries_partial(&state).await | |
| 140 | 140 | } | |
| @@ -143,7 +143,7 @@ pub(super) async fn admin_warn_user( | |||
| 143 | 143 | #[tracing::instrument(skip_all, name = "admin::admin_suspend_user")] | |
| 144 | 144 | pub(super) async fn admin_suspend_user( | |
| 145 | 145 | State(state): State<AppState>, | |
| 146 | - | AdminUser(admin): AdminUser, | |
| 146 | + | admin_user: AdminUser, | |
| 147 | 147 | Path(id): Path<UserId>, | |
| 148 | 148 | Form(form): Form<SuspendForm>, | |
| 149 | 149 | ) -> Result<impl IntoResponse> { | |
| @@ -170,7 +170,7 @@ pub(super) async fn admin_suspend_user( | |||
| 170 | 170 | } | |
| 171 | 171 | ||
| 172 | 172 | // Record moderation action for audit trail | |
| 173 | - | db::moderation::create_action(&state.db, id, admin.id, ModerationActionType::Suspension, reason, None).await?; | |
| 173 | + | db::moderation::create_action(&state.db, id, admin_user.admin_id(), ModerationActionType::Suspension, reason, None).await?; | |
| 174 | 174 | ||
| 175 | 175 | // Pause fan subscriptions to this creator's projects. The DB pause runs | |
| 176 | 176 | // inline (fast, single statement); the per-sub Stripe calls are fanned out | |
| @@ -253,7 +253,7 @@ pub(super) async fn admin_unsuspend_user( | |||
| 253 | 253 | #[tracing::instrument(skip_all, name = "admin::admin_terminate_user")] | |
| 254 | 254 | pub(super) async fn admin_terminate_user( | |
| 255 | 255 | State(state): State<AppState>, | |
| 256 | - | AdminUser(admin): AdminUser, | |
| 256 | + | admin_user: AdminUser, | |
| 257 | 257 | Path(id): Path<UserId>, | |
| 258 | 258 | ) -> Result<impl IntoResponse> { | |
| 259 | 259 | let db_user = db::users::get_user_by_id(&state.db, id) | |
| @@ -276,7 +276,7 @@ pub(super) async fn admin_terminate_user( | |||
| 276 | 276 | ||
| 277 | 277 | // Record moderation action | |
| 278 | 278 | db::moderation::create_action( | |
| 279 | - | &state.db, id, admin.id, ModerationActionType::Termination, | |
| 279 | + | &state.db, id, admin_user.admin_id(), ModerationActionType::Termination, | |
| 280 | 280 | db_user.suspension_reason.as_deref().unwrap_or("Account terminated"), | |
| 281 | 281 | None, | |
| 282 | 282 | ).await?; | |
| @@ -310,7 +310,7 @@ pub(super) async fn admin_terminate_user( | |||
| 310 | 310 | ||
| 311 | 311 | tracing::info!( | |
| 312 | 312 | user_id = %id, | |
| 313 | - | admin_id = %admin.id, | |
| 313 | + | admin_id = %admin_user.id(), | |
| 314 | 314 | "admin terminated user account (30-day export window started)" | |
| 315 | 315 | ); | |
| 316 | 316 | ||
| @@ -349,7 +349,7 @@ pub(super) async fn admin_untrust_user( | |||
| 349 | 349 | #[tracing::instrument(skip_all, name = "admin::admin_lock_custom_pages")] | |
| 350 | 350 | pub(super) async fn admin_lock_custom_pages( | |
| 351 | 351 | State(state): State<AppState>, | |
| 352 | - | AdminUser(admin): AdminUser, | |
| 352 | + | admin_user: AdminUser, | |
| 353 | 353 | Path(id): Path<UserId>, | |
| 354 | 354 | headers: axum::http::HeaderMap, | |
| 355 | 355 | ) -> Result<Response> { | |
| @@ -357,7 +357,7 @@ pub(super) async fn admin_lock_custom_pages( | |||
| 357 | 357 | db::moderation::create_action( | |
| 358 | 358 | &state.db, | |
| 359 | 359 | id, | |
| 360 | - | admin.id, | |
| 360 | + | admin_user.admin_id(), | |
| 361 | 361 | db::ModerationActionType::ContentRemoval, | |
| 362 | 362 | "custom pages locked", | |
| 363 | 363 | Some("custom-page"), |
| @@ -20,7 +20,7 @@ use tower_sessions::Session; | |||
| 20 | 20 | ||
| 21 | 21 | use crate::{ | |
| 22 | 22 | auth::{verify_password_async, MaybeUserVerified}, | |
| 23 | - | constants::{self, LOCKOUT_MINUTES, MAX_LOGIN_ATTEMPTS}, | |
| 23 | + | constants::{self, LOCKOUT_MINUTES}, | |
| 24 | 24 | csrf, | |
| 25 | 25 | db::{self, CreatorTier, SyncAppId, UserId, Username}, | |
| 26 | 26 | error::{AppError, Result}, | |
| @@ -506,57 +506,33 @@ async fn authorize_post( | |||
| 506 | 506 | )); | |
| 507 | 507 | } | |
| 508 | 508 | ||
| 509 | - | // Verify password | |
| 510 | - | if !verify_password_async(password.to_string(), user.password_hash.clone()).await? { | |
| 511 | - | let result = db::auth::increment_failed_login( | |
| 512 | - | &state.db, user.id, MAX_LOGIN_ATTEMPTS, LOCKOUT_MINUTES, | |
| 513 | - | ).await?; | |
| 514 | - | ||
| 515 | - | if result.just_locked { | |
| 509 | + | // Verify the password and account status through the shared relying-party | |
| 510 | + | // gate. It folds wrong-password / suspended / deactivated / locked / 2FA | |
| 511 | + | // into one accounted decision (always increment on denial, reset on | |
| 512 | + | // success) so a correct guess against a blocked account is NOT | |
| 513 | + | // distinguishable from a wrong one — closing the confirmed-password oracle | |
| 514 | + | // that arose from resetting before the status gates (Run 11 Sec M1). The | |
| 515 | + | // friendly "already locked" message above still short-circuits before | |
| 516 | + | // Argon2; here, only a freshly-tripped lockout earns a distinct notice. | |
| 517 | + | match crate::auth::relying_party_login_gate(&state.db, &user, password).await? { | |
| 518 | + | crate::auth::LoginGate::Deny { just_locked } => { | |
| 519 | + | let message = if just_locked { | |
| 520 | + | format!( | |
| 521 | + | "Too many failed attempts. Account locked for {} minutes.", | |
| 522 | + | LOCKOUT_MINUTES | |
| 523 | + | ) | |
| 524 | + | } else { | |
| 525 | + | "Invalid username/email or password".to_string() | |
| 526 | + | }; | |
| 516 | 527 | return Ok(render_authorize_error( | |
| 517 | 528 | Some(csrf_token), | |
| 518 | 529 | session_user, | |
| 519 | 530 | &app.name, | |
| 520 | 531 | &form, | |
| 521 | - | &format!( | |
| 522 | - | "Too many failed attempts. Account locked for {} minutes.", | |
| 523 | - | LOCKOUT_MINUTES | |
| 524 | - | ), | |
| 532 | + | &message, | |
| 525 | 533 | )); | |
| 526 | 534 | } | |
| 527 | - | ||
| 528 | - | return Ok(render_authorize_error( | |
| 529 | - | Some(csrf_token), | |
| 530 | - | session_user, | |
| 531 | - | &app.name, | |
| 532 | - | &form, | |
| 533 | - | "Invalid username/email or password", | |
| 534 | - | )); | |
| 535 | - | } | |
| 536 | - | ||
| 537 | - | // Successful auth — reset failed attempts | |
| 538 | - | db::auth::reset_failed_login(&state.db, user.id).await?; | |
| 539 | - | ||
| 540 | - | // Block suspended or deactivated users | |
| 541 | - | if user.is_suspended() || user.is_deactivated() { | |
| 542 | - | return Ok(render_authorize_error( | |
| 543 | - | Some(csrf_token), | |
| 544 | - | session_user, | |
| 545 | - | &app.name, | |
| 546 | - | &form, | |
| 547 | - | "This account is not active.", | |
| 548 | - | )); | |
| 549 | - | } | |
| 550 | - | ||
| 551 | - | // If user has TOTP 2FA enabled, reject — they must log in via the main site first | |
| 552 | - | if user.totp_enabled { | |
| 553 | - | return Ok(render_authorize_error( | |
| 554 | - | Some(csrf_token), | |
| 555 | - | session_user, | |
| 556 | - | &app.name, | |
| 557 | - | &form, | |
| 558 | - | "This account has two-factor authentication enabled. Please log in at makenot.work first, then return here to authorize the app.", | |
| 559 | - | )); | |
| 535 | + | crate::auth::LoginGate::Allow => {} | |
| 560 | 536 | } | |
| 561 | 537 | ||
| 562 | 538 | user.id |
| @@ -34,10 +34,15 @@ const PENDING_2FA_TRACKING_KEY: &str = "pending_2fa_tracking_id"; | |||
| 34 | 34 | /// expires, so a stale unattended browser can't sit "one TOTP away from | |
| 35 | 35 | /// logged in" past `PENDING_2FA_TTL_SECS`. | |
| 36 | 36 | async fn clear_pending_2fa(session: &Session, pool: &PgPool) { | |
| 37 | - | if let Ok(Some(tracking_id)) = session | |
| 37 | + | let tracking_id = session | |
| 38 | 38 | .get::<crate::db::UserSessionId>(PENDING_2FA_TRACKING_KEY) | |
| 39 | 39 | .await | |
| 40 | - | && let Err(e) = db::sessions::delete_pending_2fa_session(pool, tracking_id).await | |
| 40 | + | .ok() | |
| 41 | + | .flatten(); | |
| 42 | + | let pending_user_id = session.get::<UserId>(PENDING_2FA_KEY).await.ok().flatten(); | |
| 43 | + | if let (Some(tracking_id), Some(pending_user_id)) = (tracking_id, pending_user_id) | |
| 44 | + | && let Err(e) = | |
| 45 | + | db::sessions::delete_pending_2fa_session(pool, tracking_id, pending_user_id).await | |
| 41 | 46 | { | |
| 42 | 47 | tracing::warn!(error = ?e, "failed to delete pending_2fa tracking row"); | |
| 43 | 48 | } |
| @@ -83,52 +83,40 @@ pub(super) async fn sync_auth( | |||
| 83 | 83 | } | |
| 84 | 84 | }; | |
| 85 | 85 | ||
| 86 | - | let valid = crate::auth::verify_password_async(req.password.clone(), user.password_hash.clone()).await?; | |
| 87 | - | ||
| 88 | 86 | // Account-status checks run after verify_password to avoid timing oracles. | |
| 89 | 87 | // A correct password that still can't complete login here — suspended, | |
| 90 | - | // locked, or 2FA-gated (2FA users must use the OAuth flow) — is accounted | |
| 91 | - | // and answered EXACTLY like a wrong password: increment the failed-login | |
| 92 | - | // counter and return 401. Otherwise the counter is an oracle that confirms | |
| 93 | - | // the password of a 2FA/locked/suspended account (wrong guesses increment, | |
| 94 | - | // a correct-but-blocked guess would not). Returning 401 (not 400) also | |
| 95 | - | // avoids leaking 2FA status. (ultra-fuzz Run 3 SECURITY #4.) | |
| 96 | - | let denied = !valid | |
| 97 | - | || user.is_suspended() | |
| 98 | - | || user | |
| 99 | - | .locked_until | |
| 100 | - | .is_some_and(|locked_until| locked_until > chrono::Utc::now()) | |
| 101 | - | || user.totp_enabled; | |
| 102 | - | ||
| 103 | - | if denied { | |
| 104 | - | db::auth::increment_failed_login( | |
| 105 | - | &state.db, user.id, | |
| 106 | - | crate::constants::MAX_LOGIN_ATTEMPTS, | |
| 107 | - | crate::constants::LOCKOUT_MINUTES, | |
| 108 | - | ).await?; | |
| 109 | - | // Audit the failed sync auth (best-effort). Recorded only for a real | |
| 110 | - | // account; the email-enumeration equalization paths above (unknown user, | |
| 111 | - | // malformed input) deliberately don't log, both to avoid noise and | |
| 112 | - | // because they carry no user_id. | |
| 113 | - | let ip = crate::helpers::extract_client_ip(&headers); | |
| 114 | - | if let Err(e) = db::synckit::record_security_event( | |
| 115 | - | &state.db, | |
| 116 | - | app.id, | |
| 117 | - | Some(user.id), | |
| 118 | - | db::synckit::sync_security_event::AUTH_FAILURE, | |
| 119 | - | None, | |
| 120 | - | ip.as_deref(), | |
| 121 | - | ) | |
| 122 | - | .await | |
| 123 | - | { | |
| 124 | - | tracing::error!(error = ?e, "failed to record auth_failure security event"); | |
| 88 | + | // deactivated, locked, or 2FA-gated (2FA users must use the OAuth flow) — is | |
| 89 | + | // accounted and answered EXACTLY like a wrong password: increment the | |
| 90 | + | // failed-login counter and return 401. Otherwise the counter is an oracle | |
| 91 | + | // that confirms the password of a 2FA/locked/suspended account (wrong guesses | |
| 92 | + | // increment, a correct-but-blocked guess would not). Returning 401 (not 400) | |
| 93 | + | // also avoids leaking 2FA status. (ultra-fuzz Run 3 SECURITY #4.) The folding | |
| 94 | + | // and counter ordering live in the shared relying-party gate so this flow and | |
| 95 | + | // OAuth can't drift apart (Run 11 Sec M1). | |
| 96 | + | match crate::auth::relying_party_login_gate(&state.db, &user, &req.password).await? { | |
| 97 | + | crate::auth::LoginGate::Deny { .. } => { | |
| 98 | + | // Audit the failed sync auth (best-effort). Recorded only for a real | |
| 99 | + | // account; the email-enumeration equalization paths above (unknown | |
| 100 | + | // user, malformed input) deliberately don't log, both to avoid noise | |
| 101 | + | // and because they carry no user_id. | |
| 102 | + | let ip = crate::helpers::extract_client_ip(&headers); | |
| 103 | + | if let Err(e) = db::synckit::record_security_event( | |
| 104 | + | &state.db, | |
| 105 | + | app.id, | |
| 106 | + | Some(user.id), | |
| 107 | + | db::synckit::sync_security_event::AUTH_FAILURE, | |
| 108 | + | None, | |
| 109 | + | ip.as_deref(), | |
| 110 | + | ) | |
| 111 | + | .await | |
| 112 | + | { | |
| 113 | + | tracing::error!(error = ?e, "failed to record auth_failure security event"); | |
| 114 | + | } | |
| 115 | + | return Err(AppError::Unauthorized); | |
| 125 | 116 | } | |
| 126 | - | return Err(AppError::Unauthorized); | |
| 117 | + | crate::auth::LoginGate::Allow => {} | |
| 127 | 118 | } | |
| 128 | 119 | ||
| 129 | - | // Successful auth — reset failed login counter | |
| 130 | - | db::auth::reset_failed_login(&state.db, user.id).await?; | |
| 131 | - | ||
| 132 | 120 | // Register the session's billing key under the per-key cap AT MINT TIME, so a | |
| 133 | 121 | // token can never be issued for a key beyond the developer's paid allowance. | |
| 134 | 122 | // Previously the key was claimed lazily on first write and the JWT `key` |
| @@ -415,7 +415,14 @@ fn walk_zip<R: std::io::Read + std::io::Seek>( | |||
| 415 | 415 | } else { | |
| 416 | 416 | total_compressed += entry_compressed; | |
| 417 | 417 | total_uncompressed += actual_size; | |
| 418 | - | if entry_compressed > 0 && actual_size >= 1024 * 1024 { | |
| 418 | + | // Per-entry ratio is a fast-path signal; the accumulation vector | |
| 419 | + | // (many small ultra-compressed entries) is the total-ratio guard's | |
| 420 | + | // job below. The size floor keeps tiny, naturally-compressible | |
| 421 | + | // files (text/JSON/SVG) from tripping the 100x ratio — lowered | |
| 422 | + | // from 1 MiB to 64 KiB so mid-size bombs are caught at the entry | |
| 423 | + | // level too, where a >100x ratio is already anomalous (Run 11 Sec | |
| 424 | + | // LOW). | |
| 425 | + | if entry_compressed > 0 && actual_size >= 64 * 1024 { | |
| 419 | 426 | let entry_ratio = actual_size as f64 / entry_compressed as f64; | |
| 420 | 427 | if entry_ratio > constants::SCAN_ZIP_MAX_RATIO { | |
| 421 | 428 | archive_res = Some(LayerResult { |
| @@ -26,7 +26,9 @@ pub const ERROR_POLICY: ErrorPolicy = ErrorPolicy::FailOpen; | |||
| 26 | 26 | /// the modern format and what every current toolchain emits. | |
| 27 | 27 | const APPIMAGE_MARKER: [u8; 3] = *b"AI\x02"; | |
| 28 | 28 | ||
| 29 | - | /// Path-based entry. Mmaps the spooled file and delegates. | |
| 29 | + | /// Path-based entry. Mmaps the spooled file and delegates. Test-only — the live | |
| 30 | + | /// path verifies already-resident bytes (ultra-fuzz Run 11 Sec L1). | |
| 31 | + | #[cfg(test)] | |
| 30 | 32 | pub fn verify_appimage_signature_path(path: &std::path::Path, file_type: FileType) -> LayerResult { | |
| 31 | 33 | if !matches!(file_type, FileType::Download) { | |
| 32 | 34 | return skip("Not a download file type"); |
| @@ -45,7 +45,9 @@ pub const ERROR_POLICY: ErrorPolicy = ErrorPolicy::FailOpen; | |||
| 45 | 45 | ||
| 46 | 46 | /// Path-based entry. Mmaps the spooled file (DMG signature lookup walks | |
| 47 | 47 | /// the trailer; MachFile parses headers + load commands — both touch | |
| 48 | - | /// specific offsets, demand-paged through the mmap) and delegates. | |
| 48 | + | /// specific offsets, demand-paged through the mmap) and delegates. Test-only — | |
| 49 | + | /// the live path verifies already-resident bytes (ultra-fuzz Run 11 Sec L1). | |
| 50 | + | #[cfg(test)] | |
| 49 | 51 | pub fn verify_apple_signature_path(path: &std::path::Path, file_type: FileType) -> LayerResult { | |
| 50 | 52 | if !matches!(file_type, FileType::Download | FileType::Insertion) { | |
| 51 | 53 | return skip("Not a download file type"); |
| @@ -35,7 +35,9 @@ pub const ERROR_POLICY: ErrorPolicy = ErrorPolicy::FailOpen; | |||
| 35 | 35 | ||
| 36 | 36 | /// Path-based entry. Mmaps the spooled file and delegates. PE parsing | |
| 37 | 37 | /// walks the NT headers + attribute-cert directory at fixed offsets, so | |
| 38 | - | /// demand-paging covers the whole inspection without buffering. | |
| 38 | + | /// demand-paging covers the whole inspection without buffering. Test-only — the | |
| 39 | + | /// live path verifies already-resident bytes (ultra-fuzz Run 11 Sec L1). | |
| 40 | + | #[cfg(test)] | |
| 39 | 41 | pub fn verify_authenticode_path(path: &std::path::Path, file_type: FileType) -> LayerResult { | |
| 40 | 42 | if !matches!(file_type, FileType::Download) { | |
| 41 | 43 | return skip("Not a download file type"); |
| @@ -139,9 +139,12 @@ pub fn scan_with_yara(rules: &yara_x::Rules, data: &[u8]) -> LayerResult { | |||
| 139 | 139 | } | |
| 140 | 140 | ||
| 141 | 141 | /// Scan a spooled file against YARA rules. Reads the file into memory | |
| 142 | - | /// (yara-x's `Scanner::scan` takes a byte slice). Path-based entry exists | |
| 143 | - | /// so the streaming code path has a clean call site even though it does | |
| 144 | - | /// not yet save on memory; the win comes when yara-x exposes mmap input. | |
| 142 | + | /// (yara-x's `Scanner::scan` takes a byte slice). | |
| 143 | + | /// | |
| 144 | + | /// Test-only: the live scan path uses [`scan_with_yara`] on already-resident | |
| 145 | + | /// bytes. Gated `#[cfg(test)]` so this uncapped `fs::read` cannot be reached off | |
| 146 | + | /// the request path (ultra-fuzz Run 11 Sec L1). | |
| 147 | + | #[cfg(test)] | |
| 145 | 148 | pub fn scan_with_yara_path(rules: &yara_x::Rules, path: &std::path::Path) -> LayerResult { | |
| 146 | 149 | match std::fs::read(path) { | |
| 147 | 150 | Ok(data) => scan_with_yara(rules, &data), |
| @@ -299,6 +299,83 @@ async fn oauth_invalid_credentials() { | |||
| 299 | 299 | ); | |
| 300 | 300 | } | |
| 301 | 301 | ||
| 302 | + | /// Regression (ultra-fuzz Run 11 Sec M1): the OAuth authorize password path must | |
| 303 | + | /// not be a confirmed-password oracle for 2FA accounts. A CORRECT password | |
| 304 | + | /// against a TOTP-enabled account must be indistinguishable from a wrong one — | |
| 305 | + | /// the same generic error AND an incremented failed-login counter — not a | |
| 306 | + | /// distinct "two-factor enabled" message with the counter reset. | |
| 307 | + | #[tokio::test] | |
| 308 | + | async fn oauth_2fa_account_password_not_an_oracle() { | |
| 309 | + | let mut h = TestHarness::new().await; | |
| 310 | + | let user_id = h | |
| 311 | + | .signup("oauth2fa", "oauth2fa@test.com", "Password1!") | |
| 312 | + | .await; | |
| 313 | + | h.client.post_form("/logout", "").await; | |
| 314 | + | ||
| 315 | + | // Enable 2FA directly — the OAuth flow rejects 2FA accounts outright. | |
| 316 | + | sqlx::query("UPDATE users SET totp_enabled = true WHERE id = $1") | |
| 317 | + | .bind(user_id) | |
| 318 | + | .execute(&h.db) | |
| 319 | + | .await | |
| 320 | + | .expect("enable totp"); | |
| 321 | + | ||
| 322 | + | let (_app_id, client_id) = create_sync_app(&h.db, user_id).await; | |
| 323 | + | let (_verifier, challenge) = generate_pkce(); | |
| 324 | + | let state_param = "test-state-12345"; | |
| 325 | + | let redirect_uri = "http://127.0.0.1:9999/callback"; | |
| 326 | + | ||
| 327 | + | let resp = h | |
| 328 | + | .client | |
| 329 | + | .get(&format!( | |
| 330 | + | "/oauth/authorize?response_type=code&client_id={}&redirect_uri={}&state={}&code_challenge={}&code_challenge_method=S256", | |
| 331 | + | urlencoding::encode(&client_id), | |
| 332 | + | urlencoding::encode(redirect_uri), | |
| 333 | + | state_param, | |
| 334 | + | challenge, | |
| 335 | + | )) | |
| 336 | + | .await; | |
| 337 | + | assert_eq!(resp.status.as_u16(), 200); | |
| 338 | + | let csrf = h.client.csrf_token().expect("No CSRF token").to_string(); | |
| 339 | + | ||
| 340 | + | // POST with the CORRECT password. | |
| 341 | + | let body = format!( | |
| 342 | + | "client_id={}&redirect_uri={}&state={}&code_challenge={}&code_challenge_method=S256&login={}&password={}&_csrf={}", | |
| 343 | + | urlencoding::encode(&client_id), | |
| 344 | + | urlencoding::encode(redirect_uri), | |
| 345 | + | state_param, | |
| 346 | + | challenge, | |
| 347 | + | "oauth2fa", | |
| 348 | + | "Password1%21", | |
| 349 | + | urlencoding::encode(&csrf), | |
| 350 | + | ); | |
| 351 | + | let resp = h.client.post_form("/oauth/authorize", &body).await; | |
| 352 | + | ||
| 353 | + | // Re-renders the form with the SAME generic error as a wrong password — | |
| 354 | + | // never the distinct "two-factor" hint that leaked the password's validity. | |
| 355 | + | assert_eq!(resp.status.as_u16(), 200, "should re-render, not authorize"); | |
| 356 | + | assert!( | |
| 357 | + | !resp.text.to_lowercase().contains("two-factor") | |
| 358 | + | && !resp.text.to_lowercase().contains("two factor"), | |
| 359 | + | "must not reveal 2FA status on a correct password: {}", | |
| 360 | + | resp.text | |
| 361 | + | ); | |
| 362 | + | assert!( | |
| 363 | + | resp.text.contains("Invalid") || resp.text.contains("invalid"), | |
| 364 | + | "should show the generic invalid-credentials error: {}", | |
| 365 | + | resp.text | |
| 366 | + | ); | |
| 367 | + | ||
| 368 | + | // The denial was accounted: a correct-but-blocked guess increments the | |
| 369 | + | // counter exactly like a wrong password (no oracle via the counter either). | |
| 370 | + | let attempts: i32 = | |
| 371 | + | sqlx::query_scalar("SELECT failed_login_attempts FROM users WHERE id = $1") | |
| 372 | + | .bind(user_id) | |
| 373 | + | .fetch_one(&h.db) | |
| 374 | + | .await | |
| 375 | + | .expect("read failed_login_attempts"); | |
| 376 | + | assert_eq!(attempts, 1, "correct password on a 2FA account must increment"); | |
| 377 | + | } | |
| 378 | + | ||
| 302 | 379 | // ── Userinfo (`/oauth/userinfo`) ── | |
| 303 | 380 | // | |
| 304 | 381 | // `userinfo` is the canonical entitlement endpoint for external "Log in with MNW" |