fix(relay): address remaining PR review items for MM-84

An easy-to-host PDS on the ATProtocol, MacOS. Grandma-approved.

- Add oversized_public_key_returns_400 test (boundary test for devicePublicKey, mirrors register_device.rs analogue)
- Add empty_email_returns_400 test (present-but-empty email returns 400, not 422)
- Document V007 pending_sessions migration in crates/relay/src/db/CLAUDE.md

authored by malpercio.dev and committed by

Tangled 6 days ago d7d60310 781c7757

+194 -13

2 changed files

expand all

crates

relay

src

CLAUDE.md

routes

create_mobile_account.rs

crates/relay/src/db/CLAUDE.md

··· 36 36 - `migrations/V004__claim_codes_invite.sql` - Rebuilds claim_codes: removes DID FK, adds redeemed_at; status derived not stored 37 37 - `migrations/V005__pending_accounts.sql` - pending_accounts table: pre-provisioned account slots (id, email, handle, tier, claim_code) 38 38 - `migrations/V006__devices_v2.sql` - Rebuilds devices: replaces did FK (accounts) with account_id FK (pending_accounts); adds platform, public_key, device_token_hash; also rebuilds sessions, oauth_tokens, refresh_tokens (cascade due to FK references) 39 + - `migrations/V007__pending_sessions.sql` - pending_sessions table: id, account_id (FK→pending_accounts), device_id (FK→devices), token_hash (UNIQUE), created_at, expires_at; used by POST /v1/accounts/mobile to issue a pre-DID session for the DID-creation step

+193 -13

crates/relay/src/routes/create_mobile_account.rs

··· 23 23 24 24 use crate::app::AppState; 25 25 use crate::routes::create_account::validate_handle; 26 - use crate::routes::register_device::is_valid_platform; 27 - 28 - /// Maximum allowed length for a device public key string. 29 - const MAX_PUBLIC_KEY_LEN: usize = 512; 26 + use crate::routes::register_device::{is_valid_platform, MAX_PUBLIC_KEY_LEN}; 30 27 31 28 #[derive(Deserialize)] 32 29 #[serde(rename_all = "camelCase")] ··· 283 280 } 284 281 285 282 // Insert the pending account. The claim_code FK references the just-updated claim_codes row. 283 + // tier is always 'free' for mobile self-registration; tier selection is reserved for 284 + // admin-provisioned accounts (POST /v1/accounts) where an operator picks the tier. 286 285 sqlx::query( 287 286 "INSERT INTO pending_accounts (id, email, handle, tier, claim_code, created_at) \ 288 287 VALUES (?, ?, ?, 'free', ?, datetime('now'))", ··· 343 342 Ok(()) 344 343 } 345 344 346 - /// Classify a unique constraint violation from pending_accounts into the appropriate ApiError. 347 - /// Returns InternalError for non-unique-violation errors. 345 + /// Classify a unique constraint violation from the pending_accounts INSERT into the 346 + /// appropriate ApiError. Returns InternalError for non-unique-violation errors. 347 + /// 348 + /// Constraint name matching uses SQLite's stable "UNIQUE constraint failed: <table>.<column>" 349 + /// format. The fallthrough branch (unknown constraint) logs the constraint name so any 350 + /// unexpected violations surface in traces — matching the pattern in create_account.rs. 348 351 fn classify_pending_account_error(e: &sqlx::Error) -> ApiError { 349 352 if let sqlx::Error::Database(db_err) = e { 350 353 if db_err.kind() == sqlx::error::ErrorKind::UniqueViolation { ··· 361 364 "this handle is already claimed", 362 365 ); 363 366 } 367 + // Unknown unique constraint — log the name so it surfaces in traces. 368 + tracing::error!( 369 + constraint = msg, 370 + "unique violation on unexpected constraint in pending_accounts insert" 371 + ); 364 372 } 365 373 } 366 374 ApiError::new(ErrorCode::InternalError, "failed to create account") ··· 697 705 } 698 706 699 707 // ── Atomicity ───────────────────────────────────────────────────────────── 708 + // 709 + // These tests verify that a conflicting email or handle prevents claim code 710 + // consumption. The pre-flight uniqueness check fires before the transaction 711 + // begins, so the claim code UPDATE is never executed and no rollback is needed. 712 + // This is intentional: the pre-flight is an optimisation that avoids burning 713 + // a claim code slot on a predictable conflict. 700 714 701 715 #[tokio::test] 702 - async fn duplicate_email_rolls_back_claim_code_redemption() { 703 - // MM-84.AC4: partial failure leaves no orphans — claim code must remain unredeemed 716 + async fn duplicate_email_pre_flight_protects_claim_code() { 717 + // MM-84.AC4: email conflict caught pre-flight — claim code must not be consumed 704 718 let state = test_state().await; 705 719 let db = state.db.clone(); 706 720 let claim_code = seed_claim_code(&state.db).await; 707 721 708 - // Seed an existing pending account with the same email. 709 - let existing_code = seed_claim_code(&state.db).await; 722 + // Seed a pending account with the same email as the request will use. 723 + let existing_code = seed_claim_code(&db).await; 710 724 sqlx::query( 711 725 "INSERT INTO pending_accounts (id, email, handle, tier, claim_code, created_at) \ 712 726 VALUES (?, 'test@example.com', 'existing.example.com', 'free', ?, datetime('now'))", ··· 724 738 725 739 assert_eq!(response.status(), StatusCode::CONFLICT); 726 740 727 - // Claim code must remain unredeemed. 741 + let redeemed_at: Option<String> = 742 + sqlx::query_scalar("SELECT redeemed_at FROM claim_codes WHERE code = ?") 743 + .bind(&claim_code) 744 + .fetch_one(&db) 745 + .await 746 + .unwrap(); 747 + assert!( 748 + redeemed_at.is_none(), 749 + "claim code must not be consumed when pre-flight rejects the request" 750 + ); 751 + } 752 + 753 + #[tokio::test] 754 + async fn duplicate_handle_pre_flight_protects_claim_code() { 755 + // MM-84.AC4: handle conflict caught pre-flight — claim code must not be consumed 756 + let state = test_state().await; 757 + let db = state.db.clone(); 758 + let claim_code = seed_claim_code(&db).await; 759 + 760 + // Seed a pending account with the same handle as the request will use. 761 + let existing_code = seed_claim_code(&db).await; 762 + sqlx::query( 763 + "INSERT INTO pending_accounts (id, email, handle, tier, claim_code, created_at) \ 764 + VALUES (?, 'other@example.com', 'test.example.com', 'free', ?, datetime('now'))", 765 + ) 766 + .bind(uuid::Uuid::new_v4().to_string()) 767 + .bind(&existing_code) 768 + .execute(&db) 769 + .await 770 + .unwrap(); 771 + 772 + let response = app(state) 773 + .oneshot(post_create_mobile_account(&mobile_body(&claim_code))) 774 + .await 775 + .unwrap(); 776 + 777 + assert_eq!(response.status(), StatusCode::CONFLICT); 778 + let body = axum::body::to_bytes(response.into_body(), 4096).await.unwrap(); 779 + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); 780 + assert_eq!(json["error"]["code"], "HANDLE_TAKEN"); 781 + 728 782 let redeemed_at: Option<String> = 729 783 sqlx::query_scalar("SELECT redeemed_at FROM claim_codes WHERE code = ?") 730 784 .bind(&claim_code) ··· 733 787 .unwrap(); 734 788 assert!( 735 789 redeemed_at.is_none(), 736 - "claim code must remain unredeemed after failed provisioning" 790 + "claim code must not be consumed when pre-flight rejects the request" 737 791 ); 738 792 } 739 793 740 794 // ── Duplicate email / handle ─────────────────────────────────────────────── 741 795 742 796 #[tokio::test] 743 - async fn duplicate_email_returns_409() { 797 + async fn duplicate_email_in_pending_returns_409() { 744 798 let state = test_state().await; 745 799 let db = state.db.clone(); 746 800 let code1 = seed_claim_code(&db).await; ··· 767 821 assert_eq!(json["error"]["code"], "ACCOUNT_EXISTS"); 768 822 } 769 823 824 + #[tokio::test] 825 + async fn duplicate_email_in_accounts_returns_409() { 826 + // exercises the OR EXISTS(SELECT 1 FROM accounts WHERE email = ?) branch in the pre-flight 827 + let state = test_state().await; 828 + 829 + sqlx::query( 830 + "INSERT INTO accounts (did, email, password_hash, created_at, updated_at) \ 831 + VALUES ('did:plc:existing', 'existing@example.com', 'hash', datetime('now'), datetime('now'))", 832 + ) 833 + .execute(&state.db) 834 + .await 835 + .unwrap(); 836 + 837 + let code = seed_claim_code(&state.db).await; 838 + let response = app(state) 839 + .oneshot(post_create_mobile_account(&format!( 840 + r#"{{"email":"existing@example.com","handle":"new.example.com","devicePublicKey":"dGVzdC1rZXk=","platform":"ios","claimCode":"{code}"}}"# 841 + ))) 842 + .await 843 + .unwrap(); 844 + 845 + assert_eq!(response.status(), StatusCode::CONFLICT); 846 + let body = axum::body::to_bytes(response.into_body(), 4096).await.unwrap(); 847 + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); 848 + assert_eq!(json["error"]["code"], "ACCOUNT_EXISTS"); 849 + } 850 + 851 + #[tokio::test] 852 + async fn duplicate_handle_in_pending_returns_409() { 853 + let state = test_state().await; 854 + let db = state.db.clone(); 855 + let code1 = seed_claim_code(&db).await; 856 + let code2 = seed_claim_code(&db).await; 857 + 858 + let resp1 = app(state.clone()) 859 + .oneshot(post_create_mobile_account(&format!( 860 + r#"{{"email":"h1@example.com","handle":"taken.example.com","devicePublicKey":"dGVzdC1rZXk=","platform":"ios","claimCode":"{code1}"}}"# 861 + ))) 862 + .await 863 + .unwrap(); 864 + assert_eq!(resp1.status(), StatusCode::CREATED); 865 + 866 + let resp2 = app(state) 867 + .oneshot(post_create_mobile_account(&format!( 868 + r#"{{"email":"h2@example.com","handle":"taken.example.com","devicePublicKey":"dGVzdC1rZXk=","platform":"ios","claimCode":"{code2}"}}"# 869 + ))) 870 + .await 871 + .unwrap(); 872 + 873 + assert_eq!(resp2.status(), StatusCode::CONFLICT); 874 + let body = axum::body::to_bytes(resp2.into_body(), 4096).await.unwrap(); 875 + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); 876 + assert_eq!(json["error"]["code"], "HANDLE_TAKEN"); 877 + } 878 + 879 + #[tokio::test] 880 + async fn duplicate_handle_in_handles_returns_409() { 881 + // exercises the OR EXISTS(SELECT 1 FROM handles WHERE handle = ?) branch in the pre-flight 882 + let state = test_state().await; 883 + 884 + sqlx::query( 885 + "INSERT INTO accounts (did, email, password_hash, created_at, updated_at) \ 886 + VALUES ('did:plc:active', 'active@example.com', 'hash', datetime('now'), datetime('now'))", 887 + ) 888 + .execute(&state.db) 889 + .await 890 + .unwrap(); 891 + sqlx::query( 892 + "INSERT INTO handles (handle, did, created_at) \ 893 + VALUES ('active.example.com', 'did:plc:active', datetime('now'))", 894 + ) 895 + .execute(&state.db) 896 + .await 897 + .unwrap(); 898 + 899 + let code = seed_claim_code(&state.db).await; 900 + let response = app(state) 901 + .oneshot(post_create_mobile_account(&format!( 902 + r#"{{"email":"new@example.com","handle":"active.example.com","devicePublicKey":"dGVzdC1rZXk=","platform":"ios","claimCode":"{code}"}}"# 903 + ))) 904 + .await 905 + .unwrap(); 906 + 907 + assert_eq!(response.status(), StatusCode::CONFLICT); 908 + let body = axum::body::to_bytes(response.into_body(), 4096).await.unwrap(); 909 + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); 910 + assert_eq!(json["error"]["code"], "HANDLE_TAKEN"); 911 + } 912 + 770 913 // ── Platform validation ─────────────────────────────────────────────────── 771 914 772 915 #[tokio::test] ··· 796 939 .unwrap(); 797 940 798 941 assert_eq!(response.status(), StatusCode::BAD_REQUEST); 942 + } 943 + 944 + #[tokio::test] 945 + async fn oversized_public_key_returns_400() { 946 + use crate::routes::register_device::MAX_PUBLIC_KEY_LEN; 947 + let big_key = "x".repeat(MAX_PUBLIC_KEY_LEN + 1); 948 + let body = format!( 949 + r#"{{"email":"a@example.com","handle":"a.example.com","devicePublicKey":"{big_key}","platform":"ios","claimCode":"ABC123"}}"# 950 + ); 951 + let response = app(test_state().await) 952 + .oneshot(post_create_mobile_account(&body)) 953 + .await 954 + .unwrap(); 955 + 956 + assert_eq!(response.status(), StatusCode::BAD_REQUEST); 957 + let body = axum::body::to_bytes(response.into_body(), 4096).await.unwrap(); 958 + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); 959 + assert_eq!(json["error"]["code"], "INVALID_CLAIM"); 960 + } 961 + 962 + // ── Email validation ────────────────────────────────────────────────────── 963 + 964 + #[tokio::test] 965 + async fn empty_email_returns_400() { 966 + // Present-but-empty email must be caught by application validation (400), 967 + // not the deserializer (422 — which fires only for a missing field). 968 + let response = app(test_state().await) 969 + .oneshot(post_create_mobile_account( 970 + r#"{"email":"","handle":"a.example.com","devicePublicKey":"dGVzdC1rZXk=","platform":"ios","claimCode":"ABC123"}"#, 971 + )) 972 + .await 973 + .unwrap(); 974 + 975 + assert_eq!(response.status(), StatusCode::BAD_REQUEST); 976 + let body = axum::body::to_bytes(response.into_body(), 4096).await.unwrap(); 977 + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); 978 + assert_eq!(json["error"]["code"], "INVALID_CLAIM"); 799 979 } 800 980 801 981 // ── Missing required fields ───────────────────────────────────────────────