max / makenotwork
- Co-Authored-By
- Claude Opus 4.8 <noreply@anthropic.com>
14 files changed,
+306 insertions,
-52 deletions
| @@ -149,12 +149,26 @@ | |||
| 149 | 149 | (plaintext, hash) | |
| 150 | 150 | } | |
| 151 | 151 | ||
| 152 | + | /// Lowercase SHA-256 hex digest of a string. The shared primitive behind the | |
| 153 | + | /// "store a hash, never the plaintext" credential pattern so a DB read can't | |
| 154 | + | /// recover a usable secret. | |
| 155 | + | pub fn sha256_hex(input: &str) -> String { | |
| 156 | + | use sha2::{Digest, Sha256}; | |
| 157 | + | Sha256::digest(input.as_bytes()).iter().map(|b| format!("{b:02x}")).collect() | |
| 158 | + | } | |
| 159 | + | ||
| 152 | 160 | /// SHA-256 hex digest of a git token's plaintext. Used both when minting a | |
| 153 | 161 | /// token and when verifying one on a request, so the stored hash is never the | |
| 154 | 162 | /// plaintext and a DB read can't recover a usable credential. | |
| 155 | 163 | pub fn git_token_hash(token: &str) -> String { | |
| 156 | - | use sha2::{Digest, Sha256}; | |
| 157 | - | Sha256::digest(token.as_bytes()).iter().map(|b| format!("{b:02x}")).collect() | |
| 164 | + | sha256_hex(token) | |
| 165 | + | } | |
| 166 | + | ||
| 167 | + | /// SHA-256 hex digest of an invite code. Stored in place of the plaintext so a | |
| 168 | + | /// DB read yields no directly-usable invite (matches the reset/login/git-PAT | |
| 169 | + | /// posture). Callers normalize the code (uppercase, dashes stripped) first. | |
| 170 | + | pub fn invite_code_hash(code: &str) -> String { | |
| 171 | + | sha256_hex(code) | |
| 158 | 172 | } | |
| 159 | 173 | ||
| 160 | 174 | /// Compute the hex HMAC-SHA256 over `feed:{user_id}:{version}` with `secret`. |
| @@ -80,6 +80,16 @@ | |||
| 80 | 80 | s == other | |
| 81 | 81 | } | |
| 82 | 82 | } | |
| 83 | + | ||
| 84 | + | impl $enum_name { | |
| 85 | + | /// Every wire/DB string this enum maps to. The single source of truth | |
| 86 | + | /// for the variant set; the enum-drift test asserts it equals the | |
| 87 | + | /// Postgres `CHECK (... IN (...))` list for each backing column, so a | |
| 88 | + | /// variant added here without widening the DB constraint (or vice | |
| 89 | + | /// versa) fails at test time instead of at the first read of a | |
| 90 | + | /// poisoned row. | |
| 91 | + | pub const VARIANTS: &'static [&'static str] = &[$($str),+]; | |
| 92 | + | } | |
| 83 | 93 | }; | |
| 84 | 94 | } | |
| 85 | 95 | ||
| @@ -157,6 +167,12 @@ | |||
| 157 | 167 | /// against double-submit on shared-cart PaymentIntents (Pay-S1, Run 9). | |
| 158 | 168 | Refunding, | |
| 159 | 169 | Refunded, | |
| 170 | + | /// Present in the DB `CHECK` since the initial schema but never written by | |
| 171 | + | /// the app (stale pending transactions are deleted, not failed). Kept as a | |
| 172 | + | /// variant so the enum can decode any legacy/manual `'failed'` row instead of | |
| 173 | + | /// fail-closed-poisoning the whole query — and so the enum-drift test's | |
| 174 | + | /// variant set matches the column constraint. | |
| 175 | + | Failed, | |
| 160 | 176 | } | |
| 161 | 177 | ||
| 162 | 178 | impl_str_enum!(TransactionStatus { | |
| @@ -164,6 +180,7 @@ | |||
| 164 | 180 | Completed => "completed", | |
| 165 | 181 | Refunding => "refunding", | |
| 166 | 182 | Refunded => "refunded", | |
| 183 | + | Failed => "failed", | |
| 167 | 184 | }); | |
| 168 | 185 | ||
| 169 | 186 | // ── Follows ── |
| @@ -42,6 +42,8 @@ | |||
| 42 | 42 | creator_id: UserId, | |
| 43 | 43 | code: &str, | |
| 44 | 44 | ) -> Result<DbInviteCode> { | |
| 45 | + | // Store the hash, never the plaintext: a DB read must not yield a usable | |
| 46 | + | // invite. The caller keeps the raw `code` to show/share with the creator. | |
| 45 | 47 | let invite = sqlx::query_as::<_, DbInviteCode>( | |
| 46 | 48 | r#" | |
| 47 | 49 | INSERT INTO invite_codes (creator_id, code) | |
| @@ -50,7 +52,7 @@ | |||
| 50 | 52 | "#, | |
| 51 | 53 | ) | |
| 52 | 54 | .bind(creator_id) | |
| 53 | - | .bind(code) | |
| 55 | + | .bind(crate::crypto::invite_code_hash(code)) | |
| 54 | 56 | .fetch_one(pool) | |
| 55 | 57 | .await?; | |
| 56 | 58 | ||
| @@ -83,35 +85,44 @@ | |||
| 83 | 85 | Ok(invites) | |
| 84 | 86 | } | |
| 85 | 87 | ||
| 86 | - | /// Look up an unredeemed invite code by its raw code string. | |
| 88 | + | /// Look up an unredeemed invite code by its raw code string. The raw code is | |
| 89 | + | /// hashed before lookup (codes are stored hashed); the caller normalizes it | |
| 90 | + | /// (uppercase, dashes stripped) first. | |
| 87 | 91 | #[tracing::instrument(skip_all)] | |
| 88 | 92 | pub async fn get_valid_invite_code(pool: &PgPool, code: &str) -> Result<Option<DbInviteCode>> { | |
| 89 | 93 | let invite = sqlx::query_as::<_, DbInviteCode>( | |
| 90 | 94 | "SELECT * FROM invite_codes WHERE code = $1 AND redeemed_by_id IS NULL", | |
| 91 | 95 | ) | |
| 92 | - | .bind(code) | |
| 96 | + | .bind(crate::crypto::invite_code_hash(code)) | |
| 93 | 97 | .fetch_optional(pool) | |
| 94 | 98 | .await?; | |
| 95 | 99 | ||
| 96 | 100 | Ok(invite) | |
| 97 | 101 | } | |
| 98 | 102 | ||
| 99 | - | /// Mark an invite code as redeemed by a specific user. | |
| 103 | + | /// Atomically claim an invite code for a user. Returns `true` if this call won | |
| 104 | + | /// the claim, `false` if the code was already redeemed (lost race / reused). | |
| 105 | + | /// | |
| 106 | + | /// The `AND redeemed_by_id IS NULL` guard makes the read-then-write in the | |
| 107 | + | /// caller race-free: two concurrent redeemers both pass `get_valid_invite_code` | |
| 108 | + | /// but only one satisfies the guarded UPDATE, so a single-use invite can never | |
| 109 | + | /// onboard two users. | |
| 100 | 110 | #[tracing::instrument(skip_all)] | |
| 101 | 111 | pub async fn redeem_invite_code( | |
| 102 | 112 | pool: &PgPool, | |
| 103 | 113 | code_id: InviteCodeId, | |
| 104 | 114 | user_id: UserId, | |
| 105 | - | ) -> Result<()> { | |
| 106 | - | sqlx::query( | |
| 107 | - | "UPDATE invite_codes SET redeemed_by_id = $2, redeemed_at = NOW() WHERE id = $1", | |
| 115 | + | ) -> Result<bool> { | |
| 116 | + | let claimed = sqlx::query_scalar::<_, InviteCodeId>( | |
| 117 | + | "UPDATE invite_codes SET redeemed_by_id = $2, redeemed_at = NOW() \ | |
| 118 | + | WHERE id = $1 AND redeemed_by_id IS NULL RETURNING id", | |
| 108 | 119 | ) | |
| 109 | 120 | .bind(code_id) | |
| 110 | 121 | .bind(user_id) | |
| 111 | - | .execute(pool) | |
| 122 | + | .fetch_optional(pool) | |
| 112 | 123 | .await?; | |
| 113 | 124 | ||
| 114 | - | Ok(()) | |
| 125 | + | Ok(claimed.is_some()) | |
| 115 | 126 | } | |
| 116 | 127 | ||
| 117 | 128 | #[cfg(test)] |
| @@ -41,7 +41,7 @@ | |||
| 41 | 41 | pub(crate) mod scan_jobs; | |
| 42 | 42 | pub(crate) mod scan_admin_actions; | |
| 43 | 43 | pub(crate) mod content_insertions; | |
| 44 | - | pub(crate) mod invites; | |
| 44 | + | pub mod invites; // pub so the integration test crate can exercise redemption races directly | |
| 45 | 45 | pub(crate) mod analytics; | |
| 46 | 46 | pub(crate) mod email_suppressions; | |
| 47 | 47 | pub mod git_repos; |
| @@ -82,28 +82,27 @@ | |||
| 82 | 82 | Ok(()) | |
| 83 | 83 | } | |
| 84 | 84 | ||
| 85 | - | /// Get the last accepted TOTP time step for a user (for replay prevention). | |
| 85 | + | /// Atomically advance the last accepted TOTP time step. Returns `true` if this | |
| 86 | + | /// call recorded the step (it was strictly newer than the stored one), `false` | |
| 87 | + | /// if a concurrent verification already consumed this step or a later one. | |
| 88 | + | /// | |
| 89 | + | /// The `AND $2 > totp_last_used_step` guard makes the read-then-write in the | |
| 90 | + | /// caller race-free: two concurrent submissions of the same code both read the | |
| 91 | + | /// old step and both match, but only the winner's UPDATE affects a row — so a | |
| 92 | + | /// code cannot be accepted twice within its 30s window. Callers must gate | |
| 93 | + | /// acceptance on the returned bool, not on their own prior step read. | |
| 86 | 94 | #[tracing::instrument(skip_all)] | |
| 87 | - | pub async fn get_totp_last_used_step(pool: &PgPool, user_id: UserId) -> Result<Option<i64>> { | |
| 88 | - | let step: Option<i64> = | |
| 89 | - | sqlx::query_scalar("SELECT totp_last_used_step FROM users WHERE id = $1") | |
| 90 | - | .bind(user_id) | |
| 91 | - | .fetch_one(pool) | |
| 92 | - | .await?; | |
| 95 | + | pub async fn set_totp_last_used_step(pool: &PgPool, user_id: UserId, step: i64) -> Result<bool> { | |
| 96 | + | let updated = sqlx::query_scalar::<_, UserId>( | |
| 97 | + | "UPDATE users SET totp_last_used_step = $2 \ | |
| 98 | + | WHERE id = $1 AND $2 > COALESCE(totp_last_used_step, 0) RETURNING id", | |
| 99 | + | ) | |
| 100 | + | .bind(user_id) | |
| 101 | + | .bind(step) | |
| 102 | + | .fetch_optional(pool) | |
| 103 | + | .await?; | |
| 93 | 104 | ||
| 94 | - | Ok(step) | |
| 95 | - | } | |
| 96 | - | ||
| 97 | - | /// Update the last accepted TOTP time step (set after successful verification). | |
| 98 | - | #[tracing::instrument(skip_all)] | |
| 99 | - | pub async fn set_totp_last_used_step(pool: &PgPool, user_id: UserId, step: i64) -> Result<()> { | |
| 100 | - | sqlx::query("UPDATE users SET totp_last_used_step = $2 WHERE id = $1") | |
| 101 | - | .bind(user_id) | |
| 102 | - | .bind(step) | |
| 103 | - | .execute(pool) | |
| 104 | - | .await?; | |
| 105 | - | ||
| 106 | - | Ok(()) | |
| 105 | + | Ok(updated.is_some()) | |
| 107 | 106 | } | |
| 108 | 107 | ||
| 109 | 108 | /// Check if a user has TOTP 2FA enabled. |
| @@ -71,6 +71,78 @@ | |||
| 71 | 71 | assert!(redeemed.is_some(), "Invite code should have been redeemed"); | |
| 72 | 72 | } | |
| 73 | 73 | ||
| 74 | + | /// Invite codes are stored hashed, never in plaintext: a DB read of the `code` | |
| 75 | + | /// column must yield a SHA-256 hex digest, not the shareable code. | |
| 76 | + | #[tokio::test] | |
| 77 | + | async fn invite_code_stored_hashed() { | |
| 78 | + | let mut h = TestHarness::new().await; | |
| 79 | + | let creator_id = h.signup("invhash", "invhash@test.com", "password123").await; | |
| 80 | + | h.grant_creator(creator_id).await; | |
| 81 | + | ||
| 82 | + | let raw = "TESTCODE1234"; | |
| 83 | + | makenotwork::db::invites::create_invite_code(&h.db, creator_id, raw) | |
| 84 | + | .await | |
| 85 | + | .unwrap(); | |
| 86 | + | ||
| 87 | + | let stored: String = | |
| 88 | + | sqlx::query_scalar("SELECT code FROM invite_codes WHERE creator_id = $1") | |
| 89 | + | .bind(creator_id) | |
| 90 | + | .fetch_one(&h.db) | |
| 91 | + | .await | |
| 92 | + | .unwrap(); | |
| 93 | + | ||
| 94 | + | assert_ne!(stored, raw, "plaintext invite code must not be stored"); | |
| 95 | + | assert_eq!(stored, makenotwork::crypto::invite_code_hash(raw), "stored value must be the code hash"); | |
| 96 | + | assert_eq!(stored.len(), 64, "SHA-256 hex digest is 64 chars"); | |
| 97 | + | ||
| 98 | + | // The hashed lookup still resolves the raw code. | |
| 99 | + | let found = makenotwork::db::invites::get_valid_invite_code(&h.db, raw) | |
| 100 | + | .await | |
| 101 | + | .unwrap(); | |
| 102 | + | assert!(found.is_some(), "hashed lookup should resolve the raw code"); | |
| 103 | + | } | |
| 104 | + | ||
| 105 | + | /// A single-use invite can never onboard two users: concurrent redemptions of | |
| 106 | + | /// the same code — both of which pass the validity read — resolve to exactly one | |
| 107 | + | /// winner via the guarded atomic UPDATE. | |
| 108 | + | #[tokio::test] | |
| 109 | + | async fn invite_redeemed_once_under_race() { | |
| 110 | + | let mut h = TestHarness::new().await; | |
| 111 | + | let creator_id = h.signup("invrace", "invrace@test.com", "password123").await; | |
| 112 | + | h.grant_creator(creator_id).await; | |
| 113 | + | ||
| 114 | + | let raw = "RACECODE5678"; | |
| 115 | + | makenotwork::db::invites::create_invite_code(&h.db, creator_id, raw) | |
| 116 | + | .await | |
| 117 | + | .unwrap(); | |
| 118 | + | let invite = makenotwork::db::invites::get_valid_invite_code(&h.db, raw) | |
| 119 | + | .await | |
| 120 | + | .unwrap() | |
| 121 | + | .expect("invite should be valid"); | |
| 122 | + | ||
| 123 | + | let user_a = h.signup("racea", "racea@test.com", "password123").await; | |
| 124 | + | let user_b = h.signup("raceb", "raceb@test.com", "password123").await; | |
| 125 | + | ||
| 126 | + | // Both readers saw the invite as unredeemed; only one write may win. | |
| 127 | + | let first = makenotwork::db::invites::redeem_invite_code(&h.db, invite.id, user_a) | |
| 128 | + | .await | |
| 129 | + | .unwrap(); | |
| 130 | + | let second = makenotwork::db::invites::redeem_invite_code(&h.db, invite.id, user_b) | |
| 131 | + | .await | |
| 132 | + | .unwrap(); | |
| 133 | + | ||
| 134 | + | assert!(first, "first redemption should win the claim"); | |
| 135 | + | assert!(!second, "second redemption of the same code must lose"); | |
| 136 | + | ||
| 137 | + | let redeemer: uuid::Uuid = | |
| 138 | + | sqlx::query_scalar("SELECT redeemed_by_id FROM invite_codes WHERE id = $1") | |
| 139 | + | .bind(invite.id) | |
| 140 | + | .fetch_one(&h.db) | |
| 141 | + | .await | |
| 142 | + | .unwrap(); | |
| 143 | + | assert_eq!(redeemer, *user_a.as_uuid(), "the winner must be the only redeemer"); | |
| 144 | + | } | |
| 145 | + | ||
| 74 | 146 | #[tokio::test] | |
| 75 | 147 | async fn invite_limit_enforced() { | |
| 76 | 148 | let mut h = TestHarness::new().await; |
| @@ -5,6 +5,7 @@ | |||
| 5 | 5 | mod sso; | |
| 6 | 6 | mod discover; | |
| 7 | 7 | mod embeds; | |
| 8 | + | mod enum_drift; | |
| 8 | 9 | mod streaming; | |
| 9 | 10 | mod creator; | |
| 10 | 11 | mod purchase; |
| @@ -814,11 +814,13 @@ | |||
| 814 | 814 | // Create a creator who can issue invites | |
| 815 | 815 | let creator_id = h.create_creator("jinviter").await; | |
| 816 | 816 | ||
| 817 | - | // Create an invite code directly in DB | |
| 817 | + | // Create an invite code directly in DB. Codes are stored hashed, so insert | |
| 818 | + | // the hash of the raw code the wizard will submit. | |
| 818 | 819 | sqlx::query( | |
| 819 | - | "INSERT INTO invite_codes (creator_id, code) VALUES ($1, 'TESTCODE')", | |
| 820 | + | "INSERT INTO invite_codes (creator_id, code) VALUES ($1, $2)", | |
| 820 | 821 | ) | |
| 821 | 822 | .bind(creator_id) | |
| 823 | + | .bind(makenotwork::crypto::invite_code_hash("TESTCODE")) | |
| 822 | 824 | .execute(&h.db) | |
| 823 | 825 | .await | |
| 824 | 826 | .unwrap(); | |
| @@ -842,10 +844,11 @@ | |||
| 842 | 844 | .await; | |
| 843 | 845 | assert!(resp.status.is_success(), "Signup with invite failed: {}", resp.text); | |
| 844 | 846 | ||
| 845 | - | // Verify invite redeemed | |
| 847 | + | // Verify invite redeemed (query by creator; the code column holds a hash now) | |
| 846 | 848 | let redeemed: bool = sqlx::query_scalar( | |
| 847 | - | "SELECT redeemed_by_id IS NOT NULL FROM invite_codes WHERE code = 'TESTCODE'", | |
| 849 | + | "SELECT redeemed_by_id IS NOT NULL FROM invite_codes WHERE creator_id = $1", | |
| 848 | 850 | ) | |
| 851 | + | .bind(creator_id) | |
| 849 | 852 | .fetch_one(&h.db) | |
| 850 | 853 | .await | |
| 851 | 854 | .unwrap(); |
| @@ -101,20 +101,25 @@ | |||
| 101 | 101 | // accepted step, and that step is then recorded. `setup` cleared the step to | |
| 102 | 102 | // NULL (defaults to 0), so a genuine first code (step ~= now/30) always wins. | |
| 103 | 103 | let now = chrono::Utc::now().timestamp() as u64; | |
| 104 | - | let matched_step = find_matching_step(&totp, &form.code, now); | |
| 105 | - | let last_step = db::totp::get_totp_last_used_step(&state.db, user.id) | |
| 106 | - | .await? | |
| 107 | - | .unwrap_or(0); | |
| 108 | - | let Some(step) = matched_step.filter(|&step| step > last_step) else { | |
| 109 | - | return Ok(( | |
| 104 | + | let invalid = || { | |
| 105 | + | ( | |
| 110 | 106 | [("HX-Retarget", "#totp-confirm-status"), ("HX-Reswap", "innerHTML")], | |
| 111 | 107 | Html("<span class=\"save-error\">Invalid code. Please try again.</span>"), | |
| 112 | 108 | ) | |
| 113 | - | .into_response()); | |
| 109 | + | .into_response() | |
| 114 | 110 | }; | |
| 115 | 111 | ||
| 116 | - | // Record the matched step to prevent replay on the very first login. | |
| 117 | - | db::totp::set_totp_last_used_step(&state.db, user.id, step).await?; | |
| 112 | + | let Some(step) = find_matching_step(&totp, &form.code, now) else { | |
| 113 | + | return Ok(invalid()); | |
| 114 | + | }; | |
| 115 | + | ||
| 116 | + | // Record the matched step atomically; the guarded write is the authoritative | |
| 117 | + | // replay gate. A concurrent submission of the same code that already advanced | |
| 118 | + | // the step loses here and is rejected — the step monotonicity is enforced by | |
| 119 | + | // the DB, not by a separate read-then-write. | |
| 120 | + | if !db::totp::set_totp_last_used_step(&state.db, user.id, step).await? { | |
| 121 | + | return Ok(invalid()); | |
| 122 | + | } | |
| 118 | 123 | ||
| 119 | 124 | db::totp::enable_totp(&state.db, user.id).await?; | |
| 120 | 125 |