An easy-to-host PDS on the ATProtocol, iPhone and MacOS. Maintain control of your keys and data, always.
fix(relay): address PR review issues for MM-83
Critical fixes:
- Distinguish unique constraint violations by inspecting db_err.message():
pending_accounts.email → AccountExists (409), pending_accounts.handle
→ HandleTaken (409), claim_codes.code → retry. Prevents TOCTOU races
from being silently swallowed and retried as claim-code collisions.
- Add AccountExists, HandleTaken, InvalidHandle to status_code_mapping test
Important fixes:
- Add duplicate_handle_in_handles_returns_409 test covering the
handle_in_handles SELECT path (was untested)
- Assert json["error"]["code"] in all four 409 conflict tests so
AccountExists vs HandleTaken body swaps are caught
- Add inspect_err logging to both execute calls inside
insert_pending_account for per-operation failure attribution
Suggestion:
- Replace retry-exhaustion message "failed to generate unique claim code
after retries" with generic "failed to create account"; move the
detail to a tracing::error! before the return
+111
-23
+3
crates/common/src/error.rs
+3
crates/common/src/error.rs
···
212
212
(ErrorCode::ServiceUnavailable, 503),
213
213
(ErrorCode::InternalError, 500),
214
214
(ErrorCode::MethodNotImplemented, 501),
215
+
(ErrorCode::AccountExists, 409),
216
+
(ErrorCode::HandleTaken, 409),
217
+
(ErrorCode::InvalidHandle, 400),
215
218
];
216
219
for (code, expected) in cases {
217
220
assert_eq!(code.status_code(), expected, "wrong status for {code:?}");
+108
-23
crates/relay/src/routes/create_account.rs
+108
-23
crates/relay/src/routes/create_account.rs
···
184
184
}),
185
185
))
186
186
}
187
-
Err(e) if is_unique_violation(&e) => {
188
-
tracing::warn!(attempt, "pending account insert conflict; retrying");
189
-
continue;
190
-
}
191
-
Err(e) => {
192
-
tracing::error!(error = %e, "failed to insert pending account");
193
-
return Err(ApiError::new(
194
-
ErrorCode::InternalError,
195
-
"failed to create account",
196
-
));
197
-
}
187
+
Err(e) => match unique_violation_source(&e) {
188
+
Some(UniqueConflict::ClaimCode) => {
189
+
tracing::warn!(attempt, "claim code collision; retrying");
190
+
continue;
191
+
}
192
+
Some(UniqueConflict::Email) => {
193
+
return Err(ApiError::new(
194
+
ErrorCode::AccountExists,
195
+
"an account with this email already exists",
196
+
));
197
+
}
198
+
Some(UniqueConflict::Handle) => {
199
+
return Err(ApiError::new(
200
+
ErrorCode::HandleTaken,
201
+
"this handle is already claimed",
202
+
));
203
+
}
204
+
None => {
205
+
tracing::error!(error = %e, "failed to insert pending account");
206
+
return Err(ApiError::new(
207
+
ErrorCode::InternalError,
208
+
"failed to create account",
209
+
));
210
+
}
211
+
},
198
212
}
199
213
}
200
214
201
-
Err(ApiError::new(
202
-
ErrorCode::InternalError,
203
-
"failed to generate unique claim code after retries",
204
-
))
215
+
tracing::error!("exhausted all claim code generation attempts");
216
+
Err(ApiError::new(ErrorCode::InternalError, "failed to create account"))
205
217
}
206
218
207
219
/// Validate that a handle string passes basic format checks.
···
258
270
.bind(claim_code)
259
271
.bind(expires_offset)
260
272
.execute(&mut *tx)
261
-
.await?;
273
+
.await
274
+
.inspect_err(|e| {
275
+
tracing::error!(error = %e, "failed to insert claim_codes row in pending account transaction");
276
+
})?;
262
277
263
278
sqlx::query(
264
279
"INSERT INTO pending_accounts (id, email, handle, tier, claim_code, created_at) \
···
270
285
.bind(tier)
271
286
.bind(claim_code)
272
287
.execute(&mut *tx)
273
-
.await?;
288
+
.await
289
+
.inspect_err(|e| {
290
+
tracing::error!(error = %e, "failed to insert pending_accounts row in pending account transaction");
291
+
})?;
274
292
275
293
tx.commit().await.inspect_err(|e| {
276
294
tracing::error!(error = %e, "failed to commit pending_account transaction");
···
279
297
Ok(())
280
298
}
281
299
282
-
fn is_unique_violation(e: &sqlx::Error) -> bool {
283
-
matches!(
284
-
e,
285
-
sqlx::Error::Database(db_err)
286
-
if db_err.kind() == sqlx::error::ErrorKind::UniqueViolation
287
-
)
300
+
/// Classification of a unique constraint violation by which column fired.
301
+
enum UniqueConflict {
302
+
/// `claim_codes.code` — safe to retry with a freshly generated code.
303
+
ClaimCode,
304
+
/// `pending_accounts.email` — return `AccountExists` immediately.
305
+
Email,
306
+
/// `pending_accounts.handle` — return `HandleTaken` immediately.
307
+
Handle,
308
+
}
309
+
310
+
/// Inspect a unique constraint violation to determine which column caused it.
311
+
/// Returns `None` for non-unique-violation errors.
312
+
///
313
+
/// SQLite unique violation messages take the stable form
314
+
/// "UNIQUE constraint failed: <table>.<column>", which is not locale-dependent.
315
+
fn unique_violation_source(e: &sqlx::Error) -> Option<UniqueConflict> {
316
+
if let sqlx::Error::Database(db_err) = e {
317
+
if db_err.kind() == sqlx::error::ErrorKind::UniqueViolation {
318
+
let msg = db_err.message();
319
+
if msg.contains("pending_accounts.email") {
320
+
return Some(UniqueConflict::Email);
321
+
}
322
+
if msg.contains("pending_accounts.handle") {
323
+
return Some(UniqueConflict::Handle);
324
+
}
325
+
return Some(UniqueConflict::ClaimCode);
326
+
}
327
+
}
328
+
None
288
329
}
289
330
290
331
#[cfg(test)]
···
443
484
.await
444
485
.unwrap();
445
486
assert_eq!(second.status(), StatusCode::CONFLICT);
487
+
let body = axum::body::to_bytes(second.into_body(), 4096).await.unwrap();
488
+
let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
489
+
assert_eq!(json["error"]["code"], "ACCOUNT_EXISTS");
446
490
}
447
491
448
492
#[tokio::test]
···
468
512
.unwrap();
469
513
470
514
assert_eq!(response.status(), StatusCode::CONFLICT);
515
+
let body = axum::body::to_bytes(response.into_body(), 4096).await.unwrap();
516
+
let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
517
+
assert_eq!(json["error"]["code"], "ACCOUNT_EXISTS");
471
518
}
472
519
473
520
// ── Duplicate handle ──────────────────────────────────────────────────────
···
495
542
.await
496
543
.unwrap();
497
544
assert_eq!(second.status(), StatusCode::CONFLICT);
545
+
let body = axum::body::to_bytes(second.into_body(), 4096).await.unwrap();
546
+
let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
547
+
assert_eq!(json["error"]["code"], "HANDLE_TAKEN");
548
+
}
549
+
550
+
#[tokio::test]
551
+
async fn duplicate_handle_in_handles_returns_409() {
552
+
// handle_in_handles query (line ~129) coverage
553
+
let state = test_state_with_admin_token().await;
554
+
555
+
// Seed a fully-provisioned account with an active handle.
556
+
sqlx::query(
557
+
"INSERT INTO accounts (did, email, password_hash, created_at, updated_at) \
558
+
VALUES ('did:plc:active1', 'active1@example.com', 'hash', datetime('now'), datetime('now'))",
559
+
)
560
+
.execute(&state.db)
561
+
.await
562
+
.unwrap();
563
+
sqlx::query(
564
+
"INSERT INTO handles (handle, did, created_at) \
565
+
VALUES ('active.example.com', 'did:plc:active1', datetime('now'))",
566
+
)
567
+
.execute(&state.db)
568
+
.await
569
+
.unwrap();
570
+
571
+
let response = app(state)
572
+
.oneshot(post_create_account(
573
+
r#"{"email":"new@example.com","handle":"active.example.com","tier":"free"}"#,
574
+
Some("test-admin-token"),
575
+
))
576
+
.await
577
+
.unwrap();
578
+
579
+
assert_eq!(response.status(), StatusCode::CONFLICT);
580
+
let body = axum::body::to_bytes(response.into_body(), 4096).await.unwrap();
581
+
let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
582
+
assert_eq!(json["error"]["code"], "HANDLE_TAKEN");
498
583
}
499
584
500
585
// ── Handle validation ─────────────────────────────────────────────────────