An easy-to-host PDS on the ATProtocol, MacOS. Grandma-approved.
fix(relay): address PR review issues for MM-87
Critical: correct V006 migration comment — SQLite does not auto-update FK
references in child tables on RENAME; the migration is safe because all
tables are empty (no DML-time FK checks fire).
Important:
- Add UNIQUE INDEX idx_devices_token_hash on devices.device_token_hash
- Add max-length check (512 chars) on devicePublicKey input
- Add #[tracing::instrument] + claim_code field to redeem_and_register;
distinguish RowNotFound from other errors in log messages
- Fix seed_pending_account helper to generate unique codes/email/handle
per call so it is safe to invoke multiple times on the same pool
- Add orphaned_claim_code_returns_500_and_does_not_redeem_code test
(verifies atomicity: transaction rolls back if pending_accounts lookup
fails, leaving claim code unredeemed)
- Extend closed_db_pool_returns_500 and platform_is_case_sensitive tests
to assert error code in response body
- Add oversized_public_key_returns_400 test
authored by
Tangled
5c5fefef
e92d67c6
+18
-8
crates/relay/src/db/migrations/V006__devices_v2.sql
+18
-8
crates/relay/src/db/migrations/V006__devices_v2.sql
···
5
5
-- and adds platform, public_key, and device_token_hash for challenge-response auth.
6
6
--
7
7
-- Cascade: sessions and oauth_tokens FK to devices; refresh_tokens FKs to sessions.
8
-
-- SQLite 3.26+ auto-updates FK references in child tables when a parent is renamed,
9
-
-- so renaming devices → devices_v1 also updates sessions and oauth_tokens to reference
10
-
-- devices_v1, and renaming sessions → sessions_v1 updates refresh_tokens similarly.
11
-
-- All four tables are therefore recreated here. All are empty at this migration (pre-launch).
8
+
-- SQLite does NOT auto-update FK references in child tables when a parent is renamed
9
+
-- (ALTER TABLE RENAME only rewrites references inside trigger and view bodies — not
10
+
-- foreign key definitions in other tables). All FK columns still reference the original
11
+
-- table names after the rename, which is exactly what we want: after the _v1 tables are
12
+
-- dropped and the new tables are created under the same original names, the unchanged FK
13
+
-- definitions automatically point to the correct new tables.
14
+
-- All four tables are empty at this migration (pre-launch), so no DML-time FK checks fire.
12
15
--
13
16
-- IMPORTANT index naming: SQLite indexes follow the table when it is renamed — they
14
17
-- retain their original names on the renamed table. Dropping the old tables (which
15
18
-- also drops their indexes) must happen BEFORE creating the new tables, otherwise
16
19
-- CREATE INDEX fails with "already exists". Drop order: children before parents.
17
20
18
-
-- Step 1: Rename all affected tables (most-derived first so FK auto-updates cascade
19
-
-- in the right direction as parent tables are renamed after their children).
21
+
-- Step 1: Rename all affected tables (children first so that at each rename the parent
22
+
-- table being renamed still exists; FK references in child tables are unchanged by the
23
+
-- rename, so their outbound FKs continue pointing to the original name, not the _v1 name).
20
24
ALTER TABLE refresh_tokens RENAME TO refresh_tokens_v1;
21
25
ALTER TABLE oauth_tokens RENAME TO oauth_tokens_v1;
22
26
ALTER TABLE sessions RENAME TO sessions_v1;
···
25
29
-- Step 2: Drop old tables in children-before-parents order. Each DROP also removes
26
30
-- the table's indexes (idx_refresh_tokens_did, idx_oauth_tokens_did, idx_sessions_did,
27
31
-- idx_devices_did), clearing the way for the new tables to use the same index names.
28
-
-- FK enforcement: at DROP time SQLite only checks for child rows in the table being
29
-
-- dropped, not the table's own outbound FKs. All tables are empty (pre-launch).
32
+
-- FK enforcement at DROP time: SQLite checks only whether child rows reference the table
33
+
-- being dropped. Since no FK was updated to reference _v1 names (see above), nothing
34
+
-- references the _v1 tables — all FKs still point to the original names. Drop succeeds.
30
35
DROP TABLE refresh_tokens_v1;
31
36
DROP TABLE oauth_tokens_v1;
32
37
DROP TABLE sessions_v1;
···
37
42
-- public_key is stored as provided by the device (used for future challenge-response auth).
38
43
-- device_token_hash is SHA-256(device_token); the plaintext token is returned once at
39
44
-- registration and never stored.
45
+
-- device_token_hash is UNIQUE: every registered device receives a distinct token;
46
+
-- the uniqueness constraint provides defense-in-depth and documents the invariant.
40
47
CREATE TABLE devices (
41
48
id TEXT NOT NULL,
42
49
account_id TEXT NOT NULL REFERENCES pending_accounts (id),
···
51
58
52
59
-- Device listing by account (e.g., show all devices for a user).
53
60
CREATE INDEX idx_devices_account_id ON devices (account_id);
61
+
62
+
-- Each registered device must have a distinct token hash (defense-in-depth).
63
+
CREATE UNIQUE INDEX idx_devices_token_hash ON devices (device_token_hash);
54
64
55
65
-- Step 4: Recreate sessions, oauth_tokens, and refresh_tokens with FKs pointing to the
56
66
-- new devices/sessions tables. Schemas are identical to V002 except for the FK targets.
+113
-12
crates/relay/src/routes/register_device.rs
+113
-12
crates/relay/src/routes/register_device.rs
···
1
1
// pattern: Imperative Shell
2
2
//
3
3
// Gathers: JSON request body (claim_code, device_public_key, platform), DB pool
4
-
// Processes: platform validation → public key non-empty check → atomic claim-code
5
-
// redemption + device registration (single transaction):
4
+
// Processes: platform validation → public key non-empty/length check → atomic claim-code
5
+
// redemption + device registration (single transaction, rolls back on any step
6
+
// failure):
6
7
// UPDATE claim_codes WHERE code = ? AND unredeemed AND unexpired
7
8
// SELECT pending_accounts.id WHERE claim_code = ?
8
9
// INSERT INTO devices (...)
···
19
20
20
21
use crate::app::AppState;
21
22
23
+
/// Maximum allowed length for a device public key string.
24
+
/// A P-256 uncompressed public key in base64 is ~88 chars; 512 is generous
25
+
/// enough to accommodate any standard encoding without accepting unbounded input.
26
+
const MAX_PUBLIC_KEY_LEN: usize = 512;
27
+
22
28
#[derive(Deserialize)]
23
29
#[serde(rename_all = "camelCase")]
24
30
pub struct RegisterDeviceRequest {
···
47
53
));
48
54
}
49
55
50
-
// --- Validate device_public_key is non-empty ---
56
+
// --- Validate device_public_key ---
51
57
if payload.device_public_key.is_empty() {
52
58
return Err(ApiError::new(
53
59
ErrorCode::InvalidClaim,
54
60
"devicePublicKey must not be empty",
55
61
));
56
62
}
63
+
if payload.device_public_key.len() > MAX_PUBLIC_KEY_LEN {
64
+
return Err(ApiError::new(
65
+
ErrorCode::InvalidClaim,
66
+
format!("devicePublicKey must be at most {MAX_PUBLIC_KEY_LEN} characters"),
67
+
));
68
+
}
57
69
58
70
// --- Generate device credentials ---
59
71
// 32 random bytes → base64url (no padding) for the wire; SHA-256 hex for the DB.
···
99
111
/// redeemed — no race window, and no second SELECT is needed for the guard.
100
112
///
101
113
/// Returns the `account_id` (pending_accounts.id) on success.
114
+
/// On any failure after the transaction has begun, the transaction is dropped and
115
+
/// SQLite rolls back all changes — the claim code remains unredeemed.
116
+
#[tracing::instrument(skip(db), err, fields(claim_code = %claim_code))]
102
117
async fn redeem_and_register(
103
118
db: &sqlx::SqlitePool,
104
119
claim_code: &str,
···
122
137
.execute(&mut *tx)
123
138
.await
124
139
.inspect_err(|e| {
125
-
tracing::error!(error = %e, "failed to redeem claim code");
140
+
tracing::error!(error = %e, "failed to execute claim code redemption UPDATE");
126
141
})
127
142
.map_err(|_| ApiError::new(ErrorCode::InternalError, "failed to register device"))?;
128
143
···
141
156
.fetch_one(&mut *tx)
142
157
.await
143
158
.inspect_err(|e| {
144
-
tracing::error!(error = %e, "failed to fetch pending account for claim code");
159
+
if matches!(e, sqlx::Error::RowNotFound) {
160
+
tracing::error!("no pending_account row found for claim code — orphaned code");
161
+
} else {
162
+
tracing::error!(error = %e, "failed to fetch pending account for claim code");
163
+
}
145
164
})
146
165
.map_err(|_| ApiError::new(ErrorCode::InternalError, "failed to register device"))?;
147
166
···
192
211
193
212
/// Seed a pending account with a valid (unredeemed, unexpired) claim code.
194
213
/// Returns (account_id, claim_code).
214
+
///
215
+
/// Each call generates a unique claim code and unique email/handle so the helper
216
+
/// is safe to call multiple times on the same pool without UNIQUE constraint conflicts.
195
217
async fn seed_pending_account(db: &sqlx::SqlitePool) -> (String, String) {
196
218
let account_id = uuid::Uuid::new_v4().to_string();
197
-
let claim_code = "SEED01";
219
+
let claim_code: String = uuid::Uuid::new_v4()
220
+
.simple()
221
+
.to_string()
222
+
.chars()
223
+
.take(8)
224
+
.map(|c| c.to_ascii_uppercase())
225
+
.collect();
226
+
let email = format!("test-{}@example.com", &account_id[..8]);
227
+
let handle = format!("test-{}.example.com", &account_id[..8]);
198
228
199
229
sqlx::query(
200
230
"INSERT INTO claim_codes (code, expires_at, created_at) \
201
231
VALUES (?, datetime('now', '+24 hours'), datetime('now'))",
202
232
)
203
-
.bind(claim_code)
233
+
.bind(&claim_code)
204
234
.execute(db)
205
235
.await
206
236
.unwrap();
207
237
208
238
sqlx::query(
209
239
"INSERT INTO pending_accounts (id, email, handle, tier, claim_code, created_at) \
210
-
VALUES (?, 'alice@example.com', 'alice.example.com', 'free', ?, datetime('now'))",
240
+
VALUES (?, ?, ?, 'free', ?, datetime('now'))",
211
241
)
212
242
.bind(&account_id)
213
-
.bind(claim_code)
243
+
.bind(&email)
244
+
.bind(&handle)
245
+
.bind(&claim_code)
214
246
.execute(db)
215
247
.await
216
248
.unwrap();
217
249
218
-
(account_id, claim_code.to_string())
250
+
(account_id, claim_code)
219
251
}
220
252
221
253
// ── Happy path ────────────────────────────────────────────────────────────
···
411
443
assert!(redeemed_at.is_some(), "claim code must have redeemed_at set");
412
444
}
413
445
446
+
#[tokio::test]
447
+
async fn orphaned_claim_code_returns_500_and_does_not_redeem_code() {
448
+
// Atomicity: if the pending_accounts lookup fails (orphaned code — code exists in
449
+
// claim_codes but no matching pending_accounts row), the transaction must roll back
450
+
// so the claim code remains unredeemed. Verifies the UPDATE is not committed without
451
+
// the subsequent INSERT also succeeding.
452
+
let state = test_state().await;
453
+
let db = state.db.clone();
454
+
let claim_code = "ORPHAN1";
455
+
456
+
sqlx::query(
457
+
"INSERT INTO claim_codes (code, expires_at, created_at) \
458
+
VALUES (?, datetime('now', '+24 hours'), datetime('now'))",
459
+
)
460
+
.bind(claim_code)
461
+
.execute(&state.db)
462
+
.await
463
+
.unwrap();
464
+
// Deliberately omit the matching pending_accounts insert.
465
+
466
+
let body = format!(
467
+
r#"{{"claimCode":"{claim_code}","devicePublicKey":"dGVzdC1rZXk=","platform":"ios"}}"#
468
+
);
469
+
let response = app(state)
470
+
.oneshot(post_register_device(&body))
471
+
.await
472
+
.unwrap();
473
+
474
+
assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR);
475
+
let body = axum::body::to_bytes(response.into_body(), 4096).await.unwrap();
476
+
let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
477
+
assert_eq!(json["error"]["code"], "INTERNAL_ERROR");
478
+
479
+
// Transaction must have rolled back: claim code must remain unredeemed.
480
+
let redeemed_at: Option<String> =
481
+
sqlx::query_scalar("SELECT redeemed_at FROM claim_codes WHERE code = ?")
482
+
.bind(claim_code)
483
+
.fetch_one(&db)
484
+
.await
485
+
.unwrap();
486
+
assert!(
487
+
redeemed_at.is_none(),
488
+
"claim code must remain unredeemed after failed registration (transaction rollback)"
489
+
);
490
+
}
491
+
414
492
// ── Invalid / expired / redeemed claim codes ──────────────────────────────
415
493
416
494
#[tokio::test]
···
434
512
// MM-87.AC2: expired code returns error
435
513
let state = test_state().await;
436
514
let account_id = uuid::Uuid::new_v4().to_string();
437
-
let claim_code = "EXPIRD";
515
+
let claim_code = "EXPIRD1";
438
516
439
517
sqlx::query(
440
518
"INSERT INTO claim_codes (code, expires_at, created_at) \
···
549
627
.unwrap();
550
628
551
629
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
630
+
let body = axum::body::to_bytes(response.into_body(), 4096).await.unwrap();
631
+
let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
632
+
assert_eq!(json["error"]["code"], "INVALID_CLAIM");
552
633
}
553
634
554
-
// ── Empty public key ──────────────────────────────────────────────────────
635
+
// ── Public key validation ─────────────────────────────────────────────────
555
636
556
637
#[tokio::test]
557
638
async fn empty_public_key_returns_400() {
···
568
649
assert_eq!(json["error"]["code"], "INVALID_CLAIM");
569
650
}
570
651
652
+
#[tokio::test]
653
+
async fn oversized_public_key_returns_400() {
654
+
let oversized_key = "A".repeat(super::MAX_PUBLIC_KEY_LEN + 1);
655
+
let body = format!(
656
+
r#"{{"claimCode":"ABC123","devicePublicKey":"{oversized_key}","platform":"ios"}}"#
657
+
);
658
+
let response = app(test_state().await)
659
+
.oneshot(post_register_device(&body))
660
+
.await
661
+
.unwrap();
662
+
663
+
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
664
+
let body = axum::body::to_bytes(response.into_body(), 4096).await.unwrap();
665
+
let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
666
+
assert_eq!(json["error"]["code"], "INVALID_CLAIM");
667
+
}
668
+
571
669
// ── Missing required fields ───────────────────────────────────────────────
572
670
573
671
#[tokio::test]
···
621
719
.unwrap();
622
720
623
721
assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR);
722
+
let body = axum::body::to_bytes(response.into_body(), 4096).await.unwrap();
723
+
let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
724
+
assert_eq!(json["error"]["code"], "INTERNAL_ERROR");
624
725
}
625
726
626
727
// ── Pure unit tests ───────────────────────────────────────────────────────