From 2b615bb2c5cb4aca4dea9b48118244fbc37b2fc7 Mon Sep 17 00:00:00 2001 From: Vishal Kale Date: Sat, 15 Aug 2026 23:12:37 +0530 Subject: [PATCH 1/3] Add red contract test: concurrent middleware refreshes double-spend the one-time refresh token --- ...uth2MiddlewareConcurrentRefreshTests.swift | 166 ++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 Tests/ConduitTests/Auth/OAuth2MiddlewareConcurrentRefreshTests.swift diff --git a/Tests/ConduitTests/Auth/OAuth2MiddlewareConcurrentRefreshTests.swift b/Tests/ConduitTests/Auth/OAuth2MiddlewareConcurrentRefreshTests.swift new file mode 100644 index 0000000..000d8c5 --- /dev/null +++ b/Tests/ConduitTests/Auth/OAuth2MiddlewareConcurrentRefreshTests.swift @@ -0,0 +1,166 @@ +// +// OAuth2MiddlewareConcurrentRefreshTests.swift +// Conduit +// +// Copyright © 2026 MINDBODY. All rights reserved. +// + +import XCTest +@testable import Conduit + +/// Reproduces the concurrent-refresh double-spend: two middleware instances sharing one token store +/// both pass the refresh-lock check (check-then-act, no atomicity), and each issues a refresh_token +/// grant with the SAME one-time-use refresh token. Against IdentityServer, the second consume +/// succeeds only inside a 10s grace window; any later straggler gets invalid_grant — which the +/// middleware treats as terminal, deleting the token and force-logging the user out. +/// +/// CONTRACT (red until fixed): concurrent refreshes for the same client/authorization must coalesce +/// into a single grant. +final class OAuth2MiddlewareConcurrentRefreshTests: XCTestCase { + + /// Records every grant the middlewares issue and returns rotated tokens like a real token + /// endpoint. Shared by reference across both middleware instances. + private final class RecordingRefreshStrategyFactory: OAuth2RefreshStrategyFactory { + private let recordLock = NSLock() + private(set) var issuedGrantRefreshTokens: [String] = [] + + func make(refreshToken: String, clientConfiguration: OAuth2ClientConfiguration) -> OAuth2TokenGrantStrategy { + recordLock.lock() + issuedGrantRefreshTokens.append(refreshToken) + let rotation = issuedGrantRefreshTokens.count + recordLock.unlock() + let rotatedToken = BearerToken(accessToken: "ACCESS_\(rotation)", + refreshToken: "ROTATED_\(rotation)", + expiration: Date().addingTimeInterval(3_600)) + return StubbedGrantStrategy(token: rotatedToken) + } + } + + /// Completes asynchronously with a fixed token, mimicking token-endpoint latency. + private struct StubbedGrantStrategy: OAuth2TokenGrantStrategy { + let token: BearerToken + + func issueToken(completion: @escaping (Result) -> Void) { + DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(50)) { + completion(.value(token)) + } + } + + func issueToken() throws -> BearerToken { + return token + } + } + + /// Wraps the memory store and parks the FIRST lock-state reader until a second arrives — + /// deterministically holding both racers inside the check-then-act window that thread + /// scheduling opens in production. If refreshes are properly serialized (the fix), the lone + /// reader times out after 500ms and the test still terminates, green. + private final class RendezvousTokenStore: OAuth2TokenStore { + private let underlying = OAuth2TokenMemoryStore() + /// OAuth2TokenMemoryStore is not thread-safe (unlike the UserDefaults store used in + /// production) — serialize all underlying access so the racing refreshes corrupt tokens + /// logically, as in production, instead of segfaulting the process. + private let accessLock = NSLock() + private let stateLock = NSLock() + private var lockStateReads = 0 + private let secondReaderArrived = DispatchSemaphore(value: 0) + + @discardableResult + func store(token: Token?, for client: OAuth2ClientConfiguration, + with authorization: OAuth2Authorization) -> Bool { + accessLock.lock(); defer { accessLock.unlock() } + return underlying.store(token: token, for: client, with: authorization) + } + + func tokenFor(client: OAuth2ClientConfiguration, + authorization: OAuth2Authorization) -> Token? { + accessLock.lock(); defer { accessLock.unlock() } + return underlying.tokenFor(client: client, authorization: authorization) + } + + @discardableResult + func lockRefreshToken(timeout: TimeInterval, client: OAuth2ClientConfiguration, + authorization: OAuth2Authorization) -> Bool { + accessLock.lock(); defer { accessLock.unlock() } + return underlying.lockRefreshToken(timeout: timeout, client: client, authorization: authorization) + } + + @discardableResult + func unlockRefreshTokenFor(client: OAuth2ClientConfiguration, authorization: OAuth2Authorization) -> Bool { + accessLock.lock(); defer { accessLock.unlock() } + return underlying.unlockRefreshTokenFor(client: client, authorization: authorization) + } + + func refreshTokenLockExpirationFor(client: OAuth2ClientConfiguration, authorization: OAuth2Authorization) -> Date? { + // Read FIRST, rendezvous AFTER: both racers have then already observed "unlocked" before + // either writes the lock — the exact interleaving concurrent UserDefaults reads produce + // in production. The rendezvous (outside the access lock, or the parked reader would + // deadlock the racer) only makes the scheduler's worst case deterministic. + accessLock.lock() + let lockExpiration = underlying.refreshTokenLockExpirationFor(client: client, authorization: authorization) + accessLock.unlock() + stateLock.lock() + lockStateReads += 1 + let ordinal = lockStateReads + stateLock.unlock() + if ordinal == 1 { + _ = secondReaderArrived.wait(timeout: .now() + .milliseconds(500)) + } + else { + secondReaderArrived.signal() + } + return lockExpiration + } + } + + func testConcurrentRefreshesAcrossMiddlewareInstancesCoalesceToOneGrant() throws { + let authorization = OAuth2Authorization(type: .bearer, level: .user) + let serverEnvironment = OAuth2ServerEnvironment(scope: "urn:everything", + tokenGrantURL: try URL(absoluteString: "https://example.com/oauth2/token")) + let clientConfiguration = OAuth2ClientConfiguration(clientIdentifier: "test_client", clientSecret: "test_secret", + environment: serverEnvironment) + let tokenStorage = RendezvousTokenStore() + let expiredToken = BearerToken(accessToken: "EXPIRED_ACCESS", refreshToken: "RT_OLD", expiration: Date()) + tokenStorage.store(token: expiredToken, for: clientConfiguration, with: authorization) + + let grantRecorder = RecordingRefreshStrategyFactory() + // Two middleware instances over the same store — the app configures one per URLSessionClient. + var middlewareA = OAuth2RequestPipelineMiddleware(clientConfiguration: clientConfiguration, + authorization: authorization, + tokenStorage: tokenStorage) + middlewareA.refreshStrategyFactory = grantRecorder + var middlewareB = OAuth2RequestPipelineMiddleware(clientConfiguration: clientConfiguration, + authorization: authorization, + tokenStorage: tokenStorage) + middlewareB.refreshStrategyFactory = grantRecorder + + let requestBuilder = HTTPRequestBuilder(url: try URL(absoluteString: "https://example.com/api/resource")) + requestBuilder.method = .GET + let request = try requestBuilder.build() + + let bothRequestsPrepared = expectation(description: "both requests complete with a bearer header") + bothRequestsPrepared.expectedFulfillmentCount = 2 + + for middleware in [middlewareA, middlewareB] { + DispatchQueue.global().async { + middleware.prepareForTransport(request: request) { result in + XCTAssertNotNil(result.value, "both callers must eventually receive an authorized request") + bothRequestsPrepared.fulfill() + } + } + } + + waitForExpectations(timeout: 5) + + // Documentation of the spend: every grant that fired presented the same one-time-use token. + XCTAssertTrue(grantRecorder.issuedGrantRefreshTokens.allSatisfy { $0 == "RT_OLD" }, + "grants must only ever spend the token that was current when the refresh began") + + // THE CONTRACT (red today): one token, one spend. Every additional grant is a double-spend + // of a one-time refresh token — the invalid_grant force-logout seed observed in production. + XCTAssertEqual(grantRecorder.issuedGrantRefreshTokens.count, 1, + "concurrent refreshes must coalesce to a single grant; " + + "got \(grantRecorder.issuedGrantRefreshTokens.count) grants " + + "spending \(Set(grantRecorder.issuedGrantRefreshTokens))") + } +} From 7ab1df171b2037ababeb62ffaa9dca1e743dd73f Mon Sep 17 00:00:00 2001 From: Vishal Kale Date: Sat, 15 Aug 2026 23:12:37 +0530 Subject: [PATCH 2/3] Serialize in-process token refreshes with an atomic generation-scoped claim (static kill switch: refreshClaimCoordinationEnabled) --- .../OAuth2RequestPipelineMiddleware.swift | 114 +++++++++++- .../OAuth2RefreshClaimRegistry.swift | 53 ++++++ .../Auth/ConcurrentRefreshTestSupport.swift | 132 ++++++++++++++ ...uth2MiddlewareConcurrentRefreshTests.swift | 109 +++++------- ...Auth2MiddlewareRefreshClaimPathTests.swift | 166 ++++++++++++++++++ .../OAuth2RefreshClaimRegistryTests.swift | 81 +++++++++ 6 files changed, 591 insertions(+), 64 deletions(-) create mode 100644 Sources/Conduit/Auth/TokenStorage/Coordination/OAuth2RefreshClaimRegistry.swift create mode 100644 Tests/ConduitTests/Auth/ConcurrentRefreshTestSupport.swift create mode 100644 Tests/ConduitTests/Auth/OAuth2MiddlewareRefreshClaimPathTests.swift create mode 100644 Tests/ConduitTests/Auth/OAuth2RefreshClaimRegistryTests.swift diff --git a/Sources/Conduit/Auth/OAuth2RequestPipelineMiddleware.swift b/Sources/Conduit/Auth/OAuth2RequestPipelineMiddleware.swift index 57eb661..1fe6445 100644 --- a/Sources/Conduit/Auth/OAuth2RequestPipelineMiddleware.swift +++ b/Sources/Conduit/Auth/OAuth2RequestPipelineMiddleware.swift @@ -23,6 +23,8 @@ public struct OAuth2RequestPipelineMiddleware: RequestPipelineMiddleware { /// against lengthy session locks when terminating mid-flight. This will allow the host process to quickly pick /// up where the other process left off, if it needs to. Defaults to 30 seconds. public var tokenRefreshLockRelinquishInterval: TimeInterval = 30 + /// Kill switch for atomic in-process refresh serialization; static because instances are per-client value copies. Defaults to true. + public static var refreshClaimCoordinationEnabled = true let clientConfiguration: OAuth2ClientConfiguration let authorization: OAuth2Authorization let tokenStorage: OAuth2TokenStore @@ -34,6 +36,11 @@ public struct OAuth2RequestPipelineMiddleware: RequestPipelineMiddleware { return token } + /// One refresh claim per client + authorization, keyed by the store's canonical token identifier. + private var refreshClaimKey: String { + tokenStorage.tokenIdentifierFor(clientConfiguration: clientConfiguration, authorization: authorization) + ".refresh-claim" + } + /// Creates a new OAuth2RequestPipelineMiddleware /// - Parameters: /// - clientConfiguration: The configuration of the OAuth2 client @@ -47,6 +54,8 @@ public struct OAuth2RequestPipelineMiddleware: RequestPipelineMiddleware { self.tokenStorage = tokenStorage } + // Over the body-length limit only while the pre-coordination path stays inline; remove with it. + // swiftlint:disable:next function_body_length public func prepareForTransport(request: URLRequest, completion: @escaping (Result) -> Void) { let url = request.url?.absoluteString ?? "(Unknown URL)" let method = request.httpMethod ?? "(Unknown Method)" @@ -58,6 +67,10 @@ public struct OAuth2RequestPipelineMiddleware: RequestPipelineMiddleware { else if let token = token, token.refreshToken != nil, refreshStrategyFactory != nil { + if OAuth2RequestPipelineMiddleware.refreshClaimCoordinationEnabled { + refreshTokenAndResume(request: request, completion: completion) + return + } if tokenStorage.isRefreshTokenLockedFor(client: clientConfiguration, authorization: authorization), let tokenLockExpiration = tokenStorage.refreshTokenLockExpirationFor(client: clientConfiguration, authorization: authorization) { logger.info("Token refresh is active in an alternate session; retrying once lock is relinquished") @@ -72,7 +85,7 @@ public struct OAuth2RequestPipelineMiddleware: RequestPipelineMiddleware { tokenStorage.lockRefreshToken(timeout: tokenRefreshLockRelinquishInterval, client: clientConfiguration, authorization: authorization) logger.info("Token is expired, proceeding to refresh token") OAuth2TokenRefreshCoordinator.shared.beginTokenRefresh() - + refresh(token: token) { result in switch result { case .error(let error): @@ -103,6 +116,105 @@ public struct OAuth2RequestPipelineMiddleware: RequestPipelineMiddleware { } } + /// Refreshes the expired token and resumes the request; cross-process coordination via the storage lock, in-process via the claim registry. + private func refreshTokenAndResume(request: URLRequest, completion: @escaping (Result) -> Void) { + if tokenStorage.isRefreshTokenLockedFor(client: clientConfiguration, authorization: authorization), + let tokenLockExpiration = tokenStorage.refreshTokenLockExpirationFor(client: clientConfiguration, authorization: authorization) { + logger.info("Token refresh is active in an alternate session; retrying once lock is relinquished") + // Cap the wait: re-entry re-checks all state, so a missed unbuffered wake costs one short poll, not the lock lifetime. + let timeout = min(0.5, max(0, tokenLockExpiration.timeIntervalSinceNow)) + waitForRefreshThenResumeOnce(timeout: timeout, request: request, completion: completion) + return + } + let claimKey = refreshClaimKey + guard let claimGeneration = OAuth2RefreshClaimRegistry.shared.tryClaim(key: claimKey, + staleAfter: tokenRefreshLockRelinquishInterval) else { + // A refresh is already in flight in this process; a second grant would double-spend the one-time token. + if let storedToken: BearerToken = tokenStorage.tokenFor(client: clientConfiguration, authorization: authorization), + storedToken.isValid { + // The in-flight refresh already persisted its result (store precedes release). + makeRequestByApplyingAuthorizationHeader(to: request, with: storedToken, completion: completion) + return + } + logger.info("Token refresh already in flight in this process; waiting to reuse its result") + // The completion notification is unbuffered; a missed wake costs one short interval, not the full lock timeout. + waitForRefreshThenResumeOnce(timeout: min(0.5, tokenRefreshLockRelinquishInterval), + request: request, completion: completion) + return + } + // Re-read after acquiring the claim: refresh the current stored token, never a pre-claim snapshot. + guard let currentToken: BearerToken = tokenStorage.tokenFor(client: clientConfiguration, authorization: authorization) else { + logger.warn("Token disappeared while acquiring the refresh claim (e.g. logout); failing request") + OAuth2RefreshClaimRegistry.shared.release(key: claimKey, generation: claimGeneration) + completion(.error(OAuth2Error.internalFailure)) + return + } + if currentToken.isValid { + logger.info("Another refresh completed while acquiring the claim; reusing its token") + OAuth2RefreshClaimRegistry.shared.release(key: claimKey, generation: claimGeneration) + makeRequestByApplyingAuthorizationHeader(to: request, with: currentToken, completion: completion) + return + } + performRefresh(of: currentToken, claimKey: claimKey, claimGeneration: claimGeneration, + request: request, completion: completion) + } + + /// waitForRefresh can invoke its handler from both the timeout and the completion notification when + /// they race; collapse to a single re-entry so the request completion can never fire twice. + private func waitForRefreshThenResumeOnce(timeout: TimeInterval, request: URLRequest, + completion: @escaping (Result) -> Void) { + let resumeLock = NSLock() + var resumed = false + OAuth2TokenRefreshCoordinator.shared.waitForRefresh(timeout: timeout) { + resumeLock.lock() + let alreadyResumed = resumed + resumed = true + resumeLock.unlock() + if alreadyResumed { + return + } + self.prepareForTransport(request: request, completion: completion) + } + } + + /// Issues the refresh grant and resumes the request: persist, unlock, release claim, then wake waiters — in that order. + private func performRefresh(of token: BearerToken, claimKey: String, claimGeneration: UInt64, + request: URLRequest, completion: @escaping (Result) -> Void) { + tokenStorage.lockRefreshToken(timeout: tokenRefreshLockRelinquishInterval, client: clientConfiguration, authorization: authorization) + logger.info("Token is expired, proceeding to refresh token") + OAuth2TokenRefreshCoordinator.shared.beginTokenRefresh() + + refresh(token: token) { result in + switch result { + case .error(let error): + logger.warn("There was an error refreshing the token") + // A superseded holder must not delete the successor's token or unlock its lock. + if OAuth2RefreshClaimRegistry.shared.owns(key: claimKey, generation: claimGeneration) { + if case OAuth2Error.clientFailure = error { + self.tokenStorage.removeTokenFor(client: self.clientConfiguration, authorization: self.authorization) + } + tokenStorage.unlockRefreshTokenFor(client: clientConfiguration, authorization: authorization) + } + OAuth2RefreshClaimRegistry.shared.release(key: claimKey, generation: claimGeneration) + completion(.error(error)) + case .value(let newToken): + logger.info("Successfully refreshed token") + logger.debug("New token issued: \(newToken)") + // Persist before releasing the claim or waking waiters; a superseded holder must not + // overwrite the successor's newer token or unlock its lock. + if OAuth2RefreshClaimRegistry.shared.owns(key: claimKey, generation: claimGeneration) { + self.tokenStorage.store(token: newToken, for: self.clientConfiguration, with: self.authorization) + tokenStorage.unlockRefreshTokenFor(client: clientConfiguration, authorization: authorization) + } + OAuth2RefreshClaimRegistry.shared.release(key: claimKey, generation: claimGeneration) + + /// Resume original request using freshly obtained bearer token + self.makeRequestByApplyingAuthorizationHeader(to: request, with: newToken, completion: completion) + } + OAuth2TokenRefreshCoordinator.shared.endTokenRefresh() + } + } + private func prepareForTransportWithClientAuthorization(request: URLRequest, completion: @escaping (Result) -> Void) { logger.verbose("No bearer token found, but client authorization is requested.") if authorization.type == .bearer { diff --git a/Sources/Conduit/Auth/TokenStorage/Coordination/OAuth2RefreshClaimRegistry.swift b/Sources/Conduit/Auth/TokenStorage/Coordination/OAuth2RefreshClaimRegistry.swift new file mode 100644 index 0000000..e2383b2 --- /dev/null +++ b/Sources/Conduit/Auth/TokenStorage/Coordination/OAuth2RefreshClaimRegistry.swift @@ -0,0 +1,53 @@ +// +// OAuth2RefreshClaimRegistry.swift +// Conduit +// +// Copyright © 2026 MINDBODY. All rights reserved. +// + +import Foundation + +/// Process-wide atomic refresh claim per client + authorization — the in-process mutual exclusion the check-then-act persisted lock lacks. +final class OAuth2RefreshClaimRegistry { + + static let shared = OAuth2RefreshClaimRegistry() + + private struct Claim { + let generation: UInt64 + let expiresAt: Date + } + + private let lock = NSLock() + private var claims: [String: Claim] = [:] + private var nextGeneration: UInt64 = 0 + + /// Returns an ownership generation for `release(key:generation:)`, or nil if a live claim is held. + /// Claims expire after `staleAfter` and become stealable, so a hung refresh can never wedge refreshes permanently. + func tryClaim(key: String, staleAfter: TimeInterval) -> UInt64? { + lock.lock() + defer { lock.unlock() } + if let existing = claims[key], Date() < existing.expiresAt { + return nil + } + nextGeneration += 1 + claims[key] = Claim(generation: nextGeneration, expiresAt: Date().addingTimeInterval(staleAfter)) + return nextGeneration + } + + /// Returns whether `generation` still owns the claim; gates completion side effects of a possibly-superseded refresh. + func owns(key: String, generation: UInt64) -> Bool { + lock.lock() + defer { lock.unlock() } + return claims[key]?.generation == generation + } + + /// No-ops unless `generation` owns the current claim — a superseded holder cannot clear its successor's claim. + func release(key: String, generation: UInt64) { + lock.lock() + defer { lock.unlock() } + guard claims[key]?.generation == generation else { + return + } + claims[key] = nil + } +} diff --git a/Tests/ConduitTests/Auth/ConcurrentRefreshTestSupport.swift b/Tests/ConduitTests/Auth/ConcurrentRefreshTestSupport.swift new file mode 100644 index 0000000..817745a --- /dev/null +++ b/Tests/ConduitTests/Auth/ConcurrentRefreshTestSupport.swift @@ -0,0 +1,132 @@ +// +// ConcurrentRefreshTestSupport.swift +// Conduit +// +// Copyright © 2026 MINDBODY. All rights reserved. +// + +import Foundation +@testable import Conduit + +/// Records every grant issued and returns rotated tokens like a real token endpoint. +final class RecordingRefreshStrategyFactory: OAuth2RefreshStrategyFactory { + private let recordLock = NSLock() + private(set) var issuedGrantRefreshTokens: [String] = [] + + func make(refreshToken: String, clientConfiguration: OAuth2ClientConfiguration) -> OAuth2TokenGrantStrategy { + recordLock.lock() + issuedGrantRefreshTokens.append(refreshToken) + let rotation = issuedGrantRefreshTokens.count + recordLock.unlock() + let rotatedToken = BearerToken(accessToken: "ACCESS_\(rotation)", + refreshToken: "ROTATED_\(rotation)", + expiration: Date().addingTimeInterval(3_600)) + return StubbedGrantStrategy(token: rotatedToken) + } +} + +/// Completes asynchronously with a fixed token, mimicking token-endpoint latency. +struct StubbedGrantStrategy: OAuth2TokenGrantStrategy { + let token: BearerToken + + func issueToken(completion: @escaping (Result) -> Void) { + DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(50)) { + completion(.value(token)) + } + } + + func issueToken() throws -> BearerToken { + return token + } +} + +/// Fails with clientFailure only after the test signals, so the claim can go stale while the grant is in flight. +struct GatedFailureStrategy: OAuth2TokenGrantStrategy { + let proceed: DispatchSemaphore + + func issueToken(completion: @escaping (Result) -> Void) { + DispatchQueue.global().async { + self.proceed.wait() + completion(.error(OAuth2Error.clientFailure(nil, nil))) + } + } + + func issueToken() throws -> BearerToken { + throw OAuth2Error.clientFailure(nil, nil) + } +} + +final class GatedFailureStrategyFactory: OAuth2RefreshStrategyFactory { + let proceed = DispatchSemaphore(value: 0) + + func make(refreshToken: String, clientConfiguration: OAuth2ClientConfiguration) -> OAuth2TokenGrantStrategy { + GatedFailureStrategy(proceed: proceed) + } +} + +/// Succeeds with a rotated token only after the test signals, so the claim can go stale while the grant is in flight. +struct GatedSuccessStrategy: OAuth2TokenGrantStrategy { + let proceed: DispatchSemaphore + let token: BearerToken + + func issueToken(completion: @escaping (Result) -> Void) { + DispatchQueue.global().async { + self.proceed.wait() + completion(.value(token)) + } + } + + func issueToken() throws -> BearerToken { + return token + } +} + +final class GatedSuccessStrategyFactory: OAuth2RefreshStrategyFactory { + let proceed = DispatchSemaphore(value: 0) + + func make(refreshToken: String, clientConfiguration: OAuth2ClientConfiguration) -> OAuth2TokenGrantStrategy { + GatedSuccessStrategy(proceed: proceed, + token: BearerToken(accessToken: "LATE", refreshToken: "RT_LATE", + expiration: Date().addingTimeInterval(3_600))) + } +} + +/// Returns the expired token for the first read and the fresh token afterwards — scripting the +/// "another refresh persisted while we raced" interleavings deterministically. +final class ScriptedReadTokenStore: OAuth2TokenStore { + private let underlying = OAuth2TokenMemoryStore() + private let readLock = NSLock() + private var reads = 0 + let expiredToken = BearerToken(accessToken: "EXPIRED", refreshToken: "RT_OLD", expiration: Date()) + let freshToken = BearerToken(accessToken: "FRESH", refreshToken: "RT_NEW", + expiration: Date().addingTimeInterval(3_600)) + + @discardableResult + func store(token: Token?, for client: OAuth2ClientConfiguration, + with authorization: OAuth2Authorization) -> Bool { + underlying.store(token: token, for: client, with: authorization) + } + + func tokenFor(client: OAuth2ClientConfiguration, + authorization: OAuth2Authorization) -> Token? { + readLock.lock() + defer { readLock.unlock() } + reads += 1 + return (reads == 1 ? expiredToken : freshToken) as? Token + } + + @discardableResult + func lockRefreshToken(timeout: TimeInterval, client: OAuth2ClientConfiguration, + authorization: OAuth2Authorization) -> Bool { + underlying.lockRefreshToken(timeout: timeout, client: client, authorization: authorization) + } + + @discardableResult + func unlockRefreshTokenFor(client: OAuth2ClientConfiguration, authorization: OAuth2Authorization) -> Bool { + underlying.unlockRefreshTokenFor(client: client, authorization: authorization) + } + + func refreshTokenLockExpirationFor(client: OAuth2ClientConfiguration, authorization: OAuth2Authorization) -> Date? { + underlying.refreshTokenLockExpirationFor(client: client, authorization: authorization) + } +} diff --git a/Tests/ConduitTests/Auth/OAuth2MiddlewareConcurrentRefreshTests.swift b/Tests/ConduitTests/Auth/OAuth2MiddlewareConcurrentRefreshTests.swift index 000d8c5..f1b9398 100644 --- a/Tests/ConduitTests/Auth/OAuth2MiddlewareConcurrentRefreshTests.swift +++ b/Tests/ConduitTests/Auth/OAuth2MiddlewareConcurrentRefreshTests.swift @@ -8,58 +8,15 @@ import XCTest @testable import Conduit -/// Reproduces the concurrent-refresh double-spend: two middleware instances sharing one token store -/// both pass the refresh-lock check (check-then-act, no atomicity), and each issues a refresh_token -/// grant with the SAME one-time-use refresh token. Against IdentityServer, the second consume -/// succeeds only inside a 10s grace window; any later straggler gets invalid_grant — which the -/// middleware treats as terminal, deleting the token and force-logging the user out. -/// -/// CONTRACT (red until fixed): concurrent refreshes for the same client/authorization must coalesce -/// into a single grant. +/// Two middleware instances sharing one token store race the check-then-act refresh lock; the contract +/// is that concurrent refreshes for the same client/authorization coalesce into a single grant. final class OAuth2MiddlewareConcurrentRefreshTests: XCTestCase { - /// Records every grant the middlewares issue and returns rotated tokens like a real token - /// endpoint. Shared by reference across both middleware instances. - private final class RecordingRefreshStrategyFactory: OAuth2RefreshStrategyFactory { - private let recordLock = NSLock() - private(set) var issuedGrantRefreshTokens: [String] = [] - - func make(refreshToken: String, clientConfiguration: OAuth2ClientConfiguration) -> OAuth2TokenGrantStrategy { - recordLock.lock() - issuedGrantRefreshTokens.append(refreshToken) - let rotation = issuedGrantRefreshTokens.count - recordLock.unlock() - let rotatedToken = BearerToken(accessToken: "ACCESS_\(rotation)", - refreshToken: "ROTATED_\(rotation)", - expiration: Date().addingTimeInterval(3_600)) - return StubbedGrantStrategy(token: rotatedToken) - } - } - - /// Completes asynchronously with a fixed token, mimicking token-endpoint latency. - private struct StubbedGrantStrategy: OAuth2TokenGrantStrategy { - let token: BearerToken - - func issueToken(completion: @escaping (Result) -> Void) { - DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(50)) { - completion(.value(token)) - } - } - - func issueToken() throws -> BearerToken { - return token - } - } - - /// Wraps the memory store and parks the FIRST lock-state reader until a second arrives — - /// deterministically holding both racers inside the check-then-act window that thread - /// scheduling opens in production. If refreshes are properly serialized (the fix), the lone - /// reader times out after 500ms and the test still terminates, green. + /// Parks the FIRST lock-state reader until a second arrives, deterministically holding both racers + /// inside the check-then-act window; a lone reader times out after 3s so serialized refreshes still terminate. private final class RendezvousTokenStore: OAuth2TokenStore { private let underlying = OAuth2TokenMemoryStore() - /// OAuth2TokenMemoryStore is not thread-safe (unlike the UserDefaults store used in - /// production) — serialize all underlying access so the racing refreshes corrupt tokens - /// logically, as in production, instead of segfaulting the process. + /// OAuth2TokenMemoryStore is not thread-safe — serialize access so races corrupt tokens logically, not segfault. private let accessLock = NSLock() private let stateLock = NSLock() private var lockStateReads = 0 @@ -92,10 +49,8 @@ final class OAuth2MiddlewareConcurrentRefreshTests: XCTestCase { } func refreshTokenLockExpirationFor(client: OAuth2ClientConfiguration, authorization: OAuth2Authorization) -> Date? { - // Read FIRST, rendezvous AFTER: both racers have then already observed "unlocked" before - // either writes the lock — the exact interleaving concurrent UserDefaults reads produce - // in production. The rendezvous (outside the access lock, or the parked reader would - // deadlock the racer) only makes the scheduler's worst case deterministic. + // Read FIRST, rendezvous AFTER (outside the access lock, or the parked reader deadlocks the racer): + // both racers observe "unlocked" before either writes the lock. accessLock.lock() let lockExpiration = underlying.refreshTokenLockExpirationFor(client: client, authorization: authorization) accessLock.unlock() @@ -104,7 +59,7 @@ final class OAuth2MiddlewareConcurrentRefreshTests: XCTestCase { let ordinal = lockStateReads stateLock.unlock() if ordinal == 1 { - _ = secondReaderArrived.wait(timeout: .now() + .milliseconds(500)) + _ = secondReaderArrived.wait(timeout: .now() + .seconds(3)) } else { secondReaderArrived.signal() @@ -113,18 +68,19 @@ final class OAuth2MiddlewareConcurrentRefreshTests: XCTestCase { } } - func testConcurrentRefreshesAcrossMiddlewareInstancesCoalesceToOneGrant() throws { + /// Two middleware instances over one store race to refresh the same expired token through the rendezvous-gated window. + private func runConcurrentRefreshScenario() throws -> (grants: RecordingRefreshStrategyFactory, + servedAuthorizationHeaders: [String]) { let authorization = OAuth2Authorization(type: .bearer, level: .user) let serverEnvironment = OAuth2ServerEnvironment(scope: "urn:everything", tokenGrantURL: try URL(absoluteString: "https://example.com/oauth2/token")) - let clientConfiguration = OAuth2ClientConfiguration(clientIdentifier: "test_client", clientSecret: "test_secret", + let clientConfiguration = OAuth2ClientConfiguration(clientIdentifier: "coalesce_test_client", clientSecret: "test_secret", environment: serverEnvironment) let tokenStorage = RendezvousTokenStore() let expiredToken = BearerToken(accessToken: "EXPIRED_ACCESS", refreshToken: "RT_OLD", expiration: Date()) tokenStorage.store(token: expiredToken, for: clientConfiguration, with: authorization) let grantRecorder = RecordingRefreshStrategyFactory() - // Two middleware instances over the same store — the app configures one per URLSessionClient. var middlewareA = OAuth2RequestPipelineMiddleware(clientConfiguration: clientConfiguration, authorization: authorization, tokenStorage: tokenStorage) @@ -140,27 +96,54 @@ final class OAuth2MiddlewareConcurrentRefreshTests: XCTestCase { let bothRequestsPrepared = expectation(description: "both requests complete with a bearer header") bothRequestsPrepared.expectedFulfillmentCount = 2 + let headerLock = NSLock() + var servedAuthorizationHeaders: [String] = [] for middleware in [middlewareA, middlewareB] { DispatchQueue.global().async { middleware.prepareForTransport(request: request) { result in XCTAssertNotNil(result.value, "both callers must eventually receive an authorized request") + if let header = result.value?.value(forHTTPHeaderField: "Authorization") { + headerLock.lock() + servedAuthorizationHeaders.append(header) + headerLock.unlock() + } bothRequestsPrepared.fulfill() } } } waitForExpectations(timeout: 5) + return (grantRecorder, servedAuthorizationHeaders) + } - // Documentation of the spend: every grant that fired presented the same one-time-use token. - XCTAssertTrue(grantRecorder.issuedGrantRefreshTokens.allSatisfy { $0 == "RT_OLD" }, + func testConcurrentRefreshesAcrossMiddlewareInstancesCoalesceToOneGrant() throws { + let outcome = try runConcurrentRefreshScenario() + + XCTAssertTrue(outcome.grants.issuedGrantRefreshTokens.allSatisfy { $0 == "RT_OLD" }, "grants must only ever spend the token that was current when the refresh began") - // THE CONTRACT (red today): one token, one spend. Every additional grant is a double-spend - // of a one-time refresh token — the invalid_grant force-logout seed observed in production. - XCTAssertEqual(grantRecorder.issuedGrantRefreshTokens.count, 1, + // One token, one spend — every additional grant is a double-spend of a one-time refresh token. + XCTAssertEqual(outcome.grants.issuedGrantRefreshTokens.count, 1, "concurrent refreshes must coalesce to a single grant; " + - "got \(grantRecorder.issuedGrantRefreshTokens.count) grants " + - "spending \(Set(grantRecorder.issuedGrantRefreshTokens))") + "got \(outcome.grants.issuedGrantRefreshTokens.count) grants " + + "spending \(Set(outcome.grants.issuedGrantRefreshTokens))") + + // Both callers must be served the winner's fresh token — not the expired one they raced with. + XCTAssertEqual(outcome.servedAuthorizationHeaders, ["Bearer ACCESS_1", "Bearer ACCESS_1"], + "both coalesced callers must carry the single grant's rotated access token") + } + + func testDisabledCoordinationPreservesLegacyDoubleSpendHazard() throws { + OAuth2RequestPipelineMiddleware.refreshClaimCoordinationEnabled = false + defer { OAuth2RequestPipelineMiddleware.refreshClaimCoordinationEnabled = true } + + let outcome = try runConcurrentRefreshScenario() + + // The kill switch must revert to the pre-coordination path exactly, its double-spend hazard included. + XCTAssertEqual(outcome.grants.issuedGrantRefreshTokens.count, 2, + "flag off must run the legacy path byte-for-byte, double-spend included") + XCTAssertTrue(outcome.grants.issuedGrantRefreshTokens.allSatisfy { $0 == "RT_OLD" }, + "both legacy grants spend the same one-time token — the double-spend itself") } } diff --git a/Tests/ConduitTests/Auth/OAuth2MiddlewareRefreshClaimPathTests.swift b/Tests/ConduitTests/Auth/OAuth2MiddlewareRefreshClaimPathTests.swift new file mode 100644 index 0000000..c4323a2 --- /dev/null +++ b/Tests/ConduitTests/Auth/OAuth2MiddlewareRefreshClaimPathTests.swift @@ -0,0 +1,166 @@ +// +// OAuth2MiddlewareRefreshClaimPathTests.swift +// Conduit +// +// Copyright © 2026 MINDBODY. All rights reserved. +// + +import XCTest +@testable import Conduit + +/// Pins the coordinated refresh path's branches: superseded holders must not clobber successor state, +/// and both claim-loss branches must reuse a persisted fresh token instead of issuing a grant. +final class OAuth2MiddlewareRefreshClaimPathTests: XCTestCase { + + private struct SupersededScenarioOutcome { + let servedAuthorizationHeader: String? + let storedAccessToken: String? + let successorLockHeld: Bool + } + + private func makeClientConfiguration(clientIdentifier: String) throws -> OAuth2ClientConfiguration { + let serverEnvironment = OAuth2ServerEnvironment(scope: "urn:everything", + tokenGrantURL: try URL(absoluteString: "https://example.com/oauth2/token")) + return OAuth2ClientConfiguration(clientIdentifier: clientIdentifier, clientSecret: "test_secret", + environment: serverEnvironment) + } + + private func claimKeyFor(store: OAuth2TokenStore, clientIdentifier: String) throws -> String { + try store.tokenIdentifierFor(clientConfiguration: makeClientConfiguration(clientIdentifier: clientIdentifier), + authorization: OAuth2Authorization(type: .bearer, level: .user)) + ".refresh-claim" + } + + private func makeRequest() throws -> URLRequest { + let requestBuilder = HTTPRequestBuilder(url: try URL(absoluteString: "https://example.com/api/resource")) + requestBuilder.method = .GET + return try requestBuilder.build() + } + + /// Starts a refresh whose grant is parked on `proceed`, lets its claim go stale, plants successor + /// state (stolen claim + fresh token + re-locked storage), then releases the parked completion. + private func runSupersededRefreshScenario(clientIdentifier: String, + gatedFactory: OAuth2RefreshStrategyFactory, + proceed: DispatchSemaphore) throws -> SupersededScenarioOutcome { + addTeardownBlock { proceed.signal() } + let authorization = OAuth2Authorization(type: .bearer, level: .user) + let clientConfiguration = try makeClientConfiguration(clientIdentifier: clientIdentifier) + let tokenStorage = OAuth2TokenMemoryStore() + tokenStorage.store(token: BearerToken(accessToken: "EXPIRED", refreshToken: "RT_OLD", expiration: Date()), + for: clientConfiguration, with: authorization) + + var middleware = OAuth2RequestPipelineMiddleware(clientConfiguration: clientConfiguration, + authorization: authorization, + tokenStorage: tokenStorage) + middleware.refreshStrategyFactory = gatedFactory + middleware.tokenRefreshLockRelinquishInterval = 0.05 + + let headerLock = NSLock() + var servedAuthorizationHeader: String? + let refreshCompleted = expectation(description: "the superseded refresh completes for its own caller") + middleware.prepareForTransport(request: try makeRequest()) { result in + headerLock.lock() + servedAuthorizationHeader = result.value?.value(forHTTPHeaderField: "Authorization") + headerLock.unlock() + refreshCompleted.fulfill() + } + + Thread.sleep(forTimeInterval: 0.1) + let claimKey = try claimKeyFor(store: tokenStorage, clientIdentifier: clientIdentifier) + let successorGeneration = try XCTUnwrap(OAuth2RefreshClaimRegistry.shared.tryClaim(key: claimKey, staleAfter: 30), + "the stale claim must be stealable") + addTeardownBlock { + OAuth2RefreshClaimRegistry.shared.release(key: claimKey, generation: successorGeneration) + tokenStorage.unlockRefreshTokenFor(client: clientConfiguration, authorization: authorization) + } + tokenStorage.store(token: BearerToken(accessToken: "SUCCESSOR", refreshToken: "RT_NEW", + expiration: Date().addingTimeInterval(3_600)), + for: clientConfiguration, with: authorization) + tokenStorage.lockRefreshToken(timeout: 30, client: clientConfiguration, authorization: authorization) + + proceed.signal() + waitForExpectations(timeout: 5) + + let stored: BearerToken? = tokenStorage.tokenFor(client: clientConfiguration, authorization: authorization) + headerLock.lock() + let header = servedAuthorizationHeader + headerLock.unlock() + return SupersededScenarioOutcome(servedAuthorizationHeader: header, + storedAccessToken: stored?.accessToken, + successorLockHeld: tokenStorage.isRefreshTokenLockedFor(client: clientConfiguration, + authorization: authorization)) + } + + func testSupersededRefreshFailureDoesNotClobberSuccessorState() throws { + let gatedFactory = GatedFailureStrategyFactory() + let outcome = try runSupersededRefreshScenario(clientIdentifier: "superseded_failure_client", + gatedFactory: gatedFactory, proceed: gatedFactory.proceed) + + XCTAssertNil(outcome.servedAuthorizationHeader, "the superseded caller still receives its own clientFailure") + XCTAssertEqual(outcome.storedAccessToken, "SUCCESSOR", "a superseded clientFailure must not delete the successor's token") + XCTAssertTrue(outcome.successorLockHeld, "a superseded completion must not unlock the successor's lock") + } + + func testSupersededRefreshSuccessDoesNotClobberSuccessorState() throws { + let gatedFactory = GatedSuccessStrategyFactory() + let outcome = try runSupersededRefreshScenario(clientIdentifier: "superseded_success_client", + gatedFactory: gatedFactory, proceed: gatedFactory.proceed) + + XCTAssertEqual(outcome.servedAuthorizationHeader, "Bearer LATE", "the superseded caller is still served its own late token") + XCTAssertEqual(outcome.storedAccessToken, "SUCCESSOR", "a superseded success must not overwrite the successor's newer token") + XCTAssertTrue(outcome.successorLockHeld, "a superseded completion must not unlock the successor's lock") + } + + private func runScriptedReadScenario(clientIdentifier: String, + tokenStorage: ScriptedReadTokenStore) throws -> (grants: RecordingRefreshStrategyFactory, + servedAuthorizationHeader: String?) { + let authorization = OAuth2Authorization(type: .bearer, level: .user) + let clientConfiguration = try makeClientConfiguration(clientIdentifier: clientIdentifier) + let grantRecorder = RecordingRefreshStrategyFactory() + var middleware = OAuth2RequestPipelineMiddleware(clientConfiguration: clientConfiguration, + authorization: authorization, + tokenStorage: tokenStorage) + middleware.refreshStrategyFactory = grantRecorder + + var servedHeader: String? + let requestPrepared = expectation(description: "the caller is served without issuing a grant") + middleware.prepareForTransport(request: try makeRequest()) { result in + servedHeader = result.value?.value(forHTTPHeaderField: "Authorization") + requestPrepared.fulfill() + } + waitForExpectations(timeout: 5) + return (grantRecorder, servedHeader) + } + + func testWinnerReusesTokenRefreshedWhileAcquiringClaim() throws { + // Branch-entry read is expired; the post-claim re-read is fresh → serve it, release the claim, no grant. + let tokenStorage = ScriptedReadTokenStore() + let outcome = try runScriptedReadScenario(clientIdentifier: "winner_coalesce_client", tokenStorage: tokenStorage) + + XCTAssertEqual(outcome.servedAuthorizationHeader, "Bearer FRESH") + XCTAssertTrue(outcome.grants.issuedGrantRefreshTokens.isEmpty, + "a token refreshed while acquiring the claim must be reused, not refreshed again") + + let claimKey = try claimKeyFor(store: tokenStorage, clientIdentifier: "winner_coalesce_client") + let reclaimedGeneration = try XCTUnwrap(OAuth2RefreshClaimRegistry.shared.tryClaim(key: claimKey, staleAfter: 30), + "the coalescing winner must release its claim before serving") + OAuth2RefreshClaimRegistry.shared.release(key: claimKey, generation: reclaimedGeneration) + } + + func testLoserServesTokenPersistedByInFlightRefresh() throws { + // A simulated in-flight winner holds the claim; the loser's re-check reads the fresh token → serve, no grant, no wait. + let tokenStorage = ScriptedReadTokenStore() + let claimKey = try claimKeyFor(store: tokenStorage, clientIdentifier: "loser_fastpath_client") + let winnerGeneration = try XCTUnwrap(OAuth2RefreshClaimRegistry.shared.tryClaim(key: claimKey, staleAfter: 30)) + addTeardownBlock { + OAuth2RefreshClaimRegistry.shared.release(key: claimKey, generation: winnerGeneration) + } + + let outcome = try runScriptedReadScenario(clientIdentifier: "loser_fastpath_client", tokenStorage: tokenStorage) + + XCTAssertEqual(outcome.servedAuthorizationHeader, "Bearer FRESH") + XCTAssertTrue(outcome.grants.issuedGrantRefreshTokens.isEmpty, + "the loser must reuse the winner's persisted token, never issue its own grant") + XCTAssertNil(OAuth2RefreshClaimRegistry.shared.tryClaim(key: claimKey, staleAfter: 30), + "the loser must not release the in-flight winner's claim") + } +} diff --git a/Tests/ConduitTests/Auth/OAuth2RefreshClaimRegistryTests.swift b/Tests/ConduitTests/Auth/OAuth2RefreshClaimRegistryTests.swift new file mode 100644 index 0000000..a74f7c9 --- /dev/null +++ b/Tests/ConduitTests/Auth/OAuth2RefreshClaimRegistryTests.swift @@ -0,0 +1,81 @@ +// +// OAuth2RefreshClaimRegistryTests.swift +// Conduit +// +// Copyright © 2026 MINDBODY. All rights reserved. +// + +import XCTest +@testable import Conduit + +final class OAuth2RefreshClaimRegistryTests: XCTestCase { + + func testFirstClaimWinsAndSecondFailsUntilRelease() throws { + let registry = OAuth2RefreshClaimRegistry() + + let generation = try XCTUnwrap(registry.tryClaim(key: "clientA:user", staleAfter: 30)) + XCTAssertNil(registry.tryClaim(key: "clientA:user", staleAfter: 30)) + XCTAssertNotNil(registry.tryClaim(key: "clientB:user", staleAfter: 30), "claims are independent per key") + + registry.release(key: "clientA:user", generation: generation) + XCTAssertNotNil(registry.tryClaim(key: "clientA:user", staleAfter: 30), "released claim is reclaimable") + } + + func testStaleClaimIsStolen() throws { + let registry = OAuth2RefreshClaimRegistry() + + XCTAssertNotNil(registry.tryClaim(key: "clientA:user", staleAfter: 0.05)) + Thread.sleep(forTimeInterval: 0.1) + XCTAssertNotNil(registry.tryClaim(key: "clientA:user", staleAfter: 0.05), + "a claim older than staleAfter must be stealable so a hung refresh cannot wedge refreshes") + } + + func testSupersededHolderCannotReleaseSuccessorsClaim() throws { + let registry = OAuth2RefreshClaimRegistry() + + let staleGeneration = try XCTUnwrap(registry.tryClaim(key: "clientA:user", staleAfter: 0.05)) + Thread.sleep(forTimeInterval: 0.1) + let liveGeneration = try XCTUnwrap(registry.tryClaim(key: "clientA:user", staleAfter: 30), + "stale claim must be stolen") + + registry.release(key: "clientA:user", generation: staleGeneration) + XCTAssertNil(registry.tryClaim(key: "clientA:user", staleAfter: 30), + "a superseded holder's release must not clear the live successor's claim") + + registry.release(key: "clientA:user", generation: liveGeneration) + XCTAssertNotNil(registry.tryClaim(key: "clientA:user", staleAfter: 30), + "the live holder's release clears the claim") + } + + func testOwnershipTracksClaimLifecycle() throws { + let registry = OAuth2RefreshClaimRegistry() + + let staleGeneration = try XCTUnwrap(registry.tryClaim(key: "clientA:user", staleAfter: 0.05)) + XCTAssertTrue(registry.owns(key: "clientA:user", generation: staleGeneration)) + + Thread.sleep(forTimeInterval: 0.1) + let liveGeneration = try XCTUnwrap(registry.tryClaim(key: "clientA:user", staleAfter: 30)) + XCTAssertFalse(registry.owns(key: "clientA:user", generation: staleGeneration), + "a holder whose stale claim was stolen no longer owns it") + XCTAssertTrue(registry.owns(key: "clientA:user", generation: liveGeneration)) + + registry.release(key: "clientA:user", generation: liveGeneration) + XCTAssertFalse(registry.owns(key: "clientA:user", generation: liveGeneration), "released claims are unowned") + } + + func testExactlyOneWinnerUnderContention() { + let registry = OAuth2RefreshClaimRegistry() + let winnerCount = NSLock() + var winners = 0 + + DispatchQueue.concurrentPerform(iterations: 32) { _ in + if registry.tryClaim(key: "contended", staleAfter: 30) != nil { + winnerCount.lock() + winners += 1 + winnerCount.unlock() + } + } + + XCTAssertEqual(winners, 1, "tryClaim must be atomic: exactly one of 32 concurrent callers wins") + } +} From a10843b3a60e9d7d46ba40036d38783226df047d Mon Sep 17 00:00:00 2001 From: Vishal Kale Date: Mon, 31 Aug 2026 12:38:53 +0530 Subject: [PATCH 3/3] Test client failure waiter behavior --- .../Auth/ConcurrentRefreshTestSupport.swift | 13 ++++- ...uth2MiddlewareConcurrentRefreshTests.swift | 56 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/Tests/ConduitTests/Auth/ConcurrentRefreshTestSupport.swift b/Tests/ConduitTests/Auth/ConcurrentRefreshTestSupport.swift index 817745a..05c4937 100644 --- a/Tests/ConduitTests/Auth/ConcurrentRefreshTestSupport.swift +++ b/Tests/ConduitTests/Auth/ConcurrentRefreshTestSupport.swift @@ -58,9 +58,20 @@ struct GatedFailureStrategy: OAuth2TokenGrantStrategy { final class GatedFailureStrategyFactory: OAuth2RefreshStrategyFactory { let proceed = DispatchSemaphore(value: 0) + private let recordLock = NSLock() + private var recordedRefreshTokens: [String] = [] + + var issuedGrantRefreshTokens: [String] { + recordLock.lock() + defer { recordLock.unlock() } + return recordedRefreshTokens + } func make(refreshToken: String, clientConfiguration: OAuth2ClientConfiguration) -> OAuth2TokenGrantStrategy { - GatedFailureStrategy(proceed: proceed) + recordLock.lock() + recordedRefreshTokens.append(refreshToken) + recordLock.unlock() + return GatedFailureStrategy(proceed: proceed) } } diff --git a/Tests/ConduitTests/Auth/OAuth2MiddlewareConcurrentRefreshTests.swift b/Tests/ConduitTests/Auth/OAuth2MiddlewareConcurrentRefreshTests.swift index f1b9398..d8cb55d 100644 --- a/Tests/ConduitTests/Auth/OAuth2MiddlewareConcurrentRefreshTests.swift +++ b/Tests/ConduitTests/Auth/OAuth2MiddlewareConcurrentRefreshTests.swift @@ -134,6 +134,62 @@ final class OAuth2MiddlewareConcurrentRefreshTests: XCTestCase { "both coalesced callers must carry the single grant's rotated access token") } + func testClientFailureDeletesTokenBeforeWaiterResumesWithoutRetryingGrant() throws { + let authorization = OAuth2Authorization(type: .bearer, level: .user) + let serverEnvironment = OAuth2ServerEnvironment(scope: "urn:everything", + tokenGrantURL: try URL(absoluteString: "https://example.com/oauth2/token")) + let clientConfiguration = OAuth2ClientConfiguration(clientIdentifier: "client_failure_test_client", + clientSecret: "test_secret", + environment: serverEnvironment) + let tokenStorage = OAuth2TokenMemoryStore() + tokenStorage.store(token: BearerToken(accessToken: "EXPIRED_ACCESS", refreshToken: "RT_OLD", expiration: Date()), + for: clientConfiguration, with: authorization) + + let gatedFailure = GatedFailureStrategyFactory() + addTeardownBlock { gatedFailure.proceed.signal() } + var winner = OAuth2RequestPipelineMiddleware(clientConfiguration: clientConfiguration, + authorization: authorization, + tokenStorage: tokenStorage) + winner.refreshStrategyFactory = gatedFailure + var waiter = OAuth2RequestPipelineMiddleware(clientConfiguration: clientConfiguration, + authorization: authorization, + tokenStorage: tokenStorage) + waiter.refreshStrategyFactory = gatedFailure + + let requestBuilder = HTTPRequestBuilder(url: try URL(absoluteString: "https://example.com/api/resource")) + requestBuilder.method = .GET + let request = try requestBuilder.build() + let bothRequestsFailed = expectation(description: "the winner and waiter both fail") + bothRequestsFailed.expectedFulfillmentCount = 2 + + winner.prepareForTransport(request: request) { result in + guard let error = result.error, case OAuth2Error.clientFailure = error else { + XCTFail("the refresh winner must receive clientFailure") + return + } + bothRequestsFailed.fulfill() + } + XCTAssertEqual(gatedFailure.issuedGrantRefreshTokens, ["RT_OLD"], + "the first request must hold the only in-flight grant") + + // prepareForTransport returns after the waiter has registered for the winner's completion. + waiter.prepareForTransport(request: request) { result in + guard let error = result.error, case OAuth2Error.clientFailure = error else { + XCTFail("the waiter must fail fast after the token is deleted") + return + } + bothRequestsFailed.fulfill() + } + + gatedFailure.proceed.signal() + waitForExpectations(timeout: 5) + + let storedToken: BearerToken? = tokenStorage.tokenFor(client: clientConfiguration, authorization: authorization) + XCTAssertNil(storedToken, "clientFailure must delete the token before waking the waiter") + XCTAssertEqual(gatedFailure.issuedGrantRefreshTokens, ["RT_OLD"], + "a waiter must not retry a refresh token rejected with clientFailure") + } + func testDisabledCoordinationPreservesLegacyDoubleSpendHazard() throws { OAuth2RequestPipelineMiddleware.refreshClaimCoordinationEnabled = false defer { OAuth2RequestPipelineMiddleware.refreshClaimCoordinationEnabled = true }