fix(server): transactional sync push; single-use refresh rotation

Two concurrency fixes from the 2026-07-06 review (findings M1, M2):

sync push (M1): the load→merge→store cycle ran as three separate DB
operations. Two devices pushing concurrently both read the same stored
payload, merged independently, and the second store overwrote the first
merge — the server visibly regressed until the losing device pushed
again. The whole cycle (including the leaderboard update) now runs in
one transaction; SQLite serialises the writers. The leaderboard
helper's stale docstring (claiming a single conditional UPDATE that was
actually two statements) is corrected — the transaction now provides
the atomicity it described.

refresh rotation (M2): SELECT-then-DELETE let two concurrent refreshes
with the same token both pass the liveness check and both mint fresh
token pairs. The SELECT is gone; rotation now gates on the DELETE's
rows_affected — whoever removes the jti row wins, everyone else gets
401. Sequential reuse was already covered by
consumed_refresh_token_is_rejected, which still passes unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-07-06 18:00:00 -07:00
parent 15bb136c79
commit 8eb316751d
2 changed files with 44 additions and 31 deletions
+12 -15
View File
@@ -283,23 +283,20 @@ pub async fn refresh(
// Tokens without jti predate rotation — require re-login.
let jti = claims.jti.ok_or(AppError::Unauthorized)?;
// Verify this jti is still live (not yet consumed or from a deleted account).
// SQLite TEXT columns are always nullable in sqlx; flatten the double-Option.
let exists: Option<String> =
sqlx::query_scalar!("SELECT jti FROM refresh_tokens WHERE jti = ?", jti)
.fetch_optional(&state.pool)
.await?
.flatten();
if exists.is_none() {
return Err(AppError::Unauthorized);
}
// Consume the old token before issuing new ones. If the insert below
// fails, the user loses this session (must re-login) — safe by design.
sqlx::query!("DELETE FROM refresh_tokens WHERE jti = ?", jti)
// Consume the old token before issuing new ones, gating on the DELETE
// actually removing a row. rows_affected == 0 covers both "jti never
// existed / account deleted" and "a concurrent refresh already consumed
// it" — the previous SELECT-then-DELETE let two concurrent refreshes
// both pass the check and both mint fresh token pairs. The DELETE is
// the mutex: whoever removes the row wins; everyone else gets 401.
// If the insert below fails, the user loses this session (must
// re-login) — safe by design.
let deleted = sqlx::query!("DELETE FROM refresh_tokens WHERE jti = ?", jti)
.execute(&state.pool)
.await?;
if deleted.rows_affected() != 1 {
return Err(AppError::Unauthorized);
}
let new_access = make_access_token(&claims.sub, &state.jwt_secret)?;
let (new_refresh, new_jti) = make_refresh_token(&claims.sub, &state.jwt_secret)?;