diff --git a/chess-server/Sources/App/Auth/AuthController.swift b/chess-server/Sources/App/Auth/AuthController.swift index 17a1f23..197ec00 100644 --- a/chess-server/Sources/App/Auth/AuthController.swift +++ b/chess-server/Sources/App/Auth/AuthController.swift @@ -57,18 +57,27 @@ struct AuthController: RouteCollection { let body = try req.content.decode(RefreshRequest.self, as: .json) let digest = RefreshToken.hash(body.refreshToken) - guard let stored = try await RefreshToken.query(on: req.db) - .filter(\.$tokenHash == digest) - .first() - else { - throw Abort(.unauthorized, reason: "invalid refresh token") + // Consume the token atomically: one DELETE … RETURNING, so two + // requests presenting the same token (a real client and a replayed + // stolen token) can never both mint a fresh pair — only the racer + // whose DELETE hits the row proceeds; the other gets 401. A + // SELECT-then-delete here was a replay window (#147). Expired tokens + // don't match, so they can't be exchanged either; cleanup reaps them. + // Mirrors `consumeNonce`; portable across Postgres and SQLite 3.35+. + guard let sql = req.db as? SQLDatabase else { + throw Abort(.internalServerError, reason: "refresh token store requires an SQL database") } + struct Consumed: Decodable { let user_id: String } + let consumed = try await sql.raw(""" + DELETE FROM refresh_tokens + WHERE token_hash = \(bind: digest) AND expires_at > \(bind: Date()) + RETURNING CAST(user_id AS text) AS user_id + """).first(decoding: Consumed.self) - try await stored.delete(on: req.db) - guard stored.expiresAt > Date() else { - throw Abort(.unauthorized, reason: "refresh token expired") + guard let consumed, let userID = UUID(uuidString: consumed.user_id) else { + throw Abort(.unauthorized, reason: "invalid or expired refresh token") } - guard let user = try await User.find(stored.$user.id, on: req.db) else { + guard let user = try await User.find(userID, on: req.db) else { throw Abort(.unauthorized, reason: "unknown user") } return try await issueTokens(for: user, on: req) diff --git a/chess-server/Tests/AppTests/AuthTests.swift b/chess-server/Tests/AppTests/AuthTests.swift index 800e324..f42ecba 100644 --- a/chess-server/Tests/AppTests/AuthTests.swift +++ b/chess-server/Tests/AppTests/AuthTests.swift @@ -80,6 +80,42 @@ final class AuthTests: XCTestCase { }) } + func testConcurrentRefreshRedeemsTheTokenExactlyOnce() async throws { + let auth = try await register() + // Application isn't Sendable; a box lets it cross into the concurrent + // task closures. Safe here — Vapor routes each test request + // independently, which is exactly the concurrency under test. + let box = SendableBox(value: app!) + let token = auth.refreshToken + + // Fire several simultaneous refreshes of the SAME token. Rotation is + // atomic (DELETE … RETURNING), so exactly one may redeem it — a + // replayed stolen token and the real client cannot both mint a session + // (#147). This is the concurrent race testRefreshRotatesToken can't + // reach: the old SELECT-then-delete let multiple racers both through. + let codes = try await withThrowingTaskGroup(of: UInt.self) { group in + for _ in 0..<8 { + group.addTask { + var code: UInt = 0 + try await box.value.test(.POST, "auth/refresh", beforeRequest: { req in + try req.content.encode(RefreshRequest(refreshToken: token), as: .json) + }, afterResponse: { res async in + code = res.status.code + }) + return code + } + } + var all: [UInt] = [] + for try await code in group { all.append(code) } + return all + } + + XCTAssertEqual(codes.filter { $0 == 200 }.count, 1, + "exactly one concurrent refresh may redeem the token") + XCTAssertEqual(codes.filter { $0 == 401 }.count, 7, + "every other concurrent refresh is rejected") + } + func testProtectedRoutesRejectAnonymous() async throws { try await app.test(.GET, "me", afterResponse: { res async in XCTAssertEqual(res.status, .unauthorized) @@ -401,3 +437,9 @@ final class AuthTests: XCTestCase { XCTAssertEqual(holders, 1) } } + +/// Carries a non-Sendable value into concurrent task closures for the +/// refresh-race test. +private struct SendableBox: @unchecked Sendable { + let value: T +}