From 1d0923f4b28bb9141e5b229e5d76f2cbd80e0f20 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 17:43:24 +0000 Subject: [PATCH 1/3] Initial plan From b1afa3d33705121147ca11f74b4cc018982ce583 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 17:55:25 +0000 Subject: [PATCH 2/3] Fix stale tokens persisting after refresh failure and add token cleanup tests - In logout(), try individual token deletions when clearAll() throws to prevent partial failures from leaving stale tokens behind - In checkExistingAuth(), clear tokens in the refresh failure catch block so subsequent calls don't retry a failed refresh (breaking the loop) - In checkExistingAuth(), start refresh timer after successful token refresh (was missing, unlike the valid-access-token path) - Add comprehensive tests for token cleanup after refresh 400 failure Co-authored-by: sirily11 <32106111+sirily11@users.noreply.github.com> --- Sources/RxAuthSwift/OAuthManager.swift | 8 + .../OAuthManagerTokenCleanupTests.swift | 303 ++++++++++++++++++ 2 files changed, 311 insertions(+) create mode 100644 Tests/RxAuthSwiftTests/OAuthManagerTokenCleanupTests.swift diff --git a/Sources/RxAuthSwift/OAuthManager.swift b/Sources/RxAuthSwift/OAuthManager.swift index 4fcc6ae..840f8c2 100644 --- a/Sources/RxAuthSwift/OAuthManager.swift +++ b/Sources/RxAuthSwift/OAuthManager.swift @@ -47,9 +47,13 @@ public final class OAuthManager: Sendable { } else if tokenStorage.getRefreshToken() != nil { do { try await refreshTokenIfNeeded() + startTokenRefreshTimer() logger.info("Refreshed token from existing session") } catch { logger.warning("Token refresh failed: \(error.localizedDescription)") + // Ensure stale tokens are fully cleared so that subsequent + // calls to checkExistingAuth() don't retry a failed refresh. + try? tokenStorage.clearAll() authState = .unauthenticated } } else { @@ -86,6 +90,10 @@ public final class OAuthManager: Sendable { try tokenStorage.clearAll() } catch { logger.error("Failed to clear token storage: \(error.localizedDescription)") + // Attempt to clear individual tokens even if clearAll() fails, + // so that a partial failure doesn't leave stale tokens behind. + try? tokenStorage.deleteAccessToken() + try? tokenStorage.deleteRefreshToken() } currentUser = nil diff --git a/Tests/RxAuthSwiftTests/OAuthManagerTokenCleanupTests.swift b/Tests/RxAuthSwiftTests/OAuthManagerTokenCleanupTests.swift new file mode 100644 index 0000000..9059b23 --- /dev/null +++ b/Tests/RxAuthSwiftTests/OAuthManagerTokenCleanupTests.swift @@ -0,0 +1,303 @@ +import Foundation +import Testing +@testable import RxAuthSwift + +// MARK: - Mock Token Storage (clearAll fails but individual deletes work) + +private final class PartiallyFailingTokenStorage: TokenStorageProtocol, @unchecked Sendable { + private let lock = NSLock() + private var accessToken: String? + private var refreshToken: String? + private var expiresAt: Date? + + func saveAccessToken(_ token: String) throws { + lock.lock() + defer { lock.unlock() } + accessToken = token + } + + func getAccessToken() -> String? { + lock.lock() + defer { lock.unlock() } + return accessToken + } + + func deleteAccessToken() throws { + lock.lock() + defer { lock.unlock() } + accessToken = nil + } + + func saveRefreshToken(_ token: String) throws { + lock.lock() + defer { lock.unlock() } + refreshToken = token + } + + func getRefreshToken() -> String? { + lock.lock() + defer { lock.unlock() } + return refreshToken + } + + func deleteRefreshToken() throws { + lock.lock() + defer { lock.unlock() } + refreshToken = nil + } + + func saveExpiresAt(_ date: Date) throws { + lock.lock() + defer { lock.unlock() } + expiresAt = date + } + + func getExpiresAt() -> Date? { + lock.lock() + defer { lock.unlock() } + return expiresAt + } + + func isTokenExpired() -> Bool { + lock.lock() + defer { lock.unlock() } + guard let expiresAt else { return true } + return expiresAt.timeIntervalSinceNow < 600 + } + + /// Always throws to simulate a storage failure during clearAll(). + func clearAll() throws { + throw ClearAllError.simulatedFailure + } + + enum ClearAllError: Error { + case simulatedFailure + } +} + +// MARK: - Mock URL Protocol + +private final class MockURLProtocol: URLProtocol, @unchecked Sendable { + nonisolated(unsafe) static var requestHandler: (@Sendable (URLRequest) -> (HTTPURLResponse, Data))? + + override class func canInit(with request: URLRequest) -> Bool { true } + override class func canonicalRequest(for request: URLRequest) -> URLRequest { request } + + override func startLoading() { + guard let handler = Self.requestHandler else { + client?.urlProtocolDidFinishLoading(self) + return + } + let (response, data) = handler(request) + client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed) + client?.urlProtocol(self, didLoad: data) + client?.urlProtocolDidFinishLoading(self) + } + + override func stopLoading() {} +} + +// MARK: - Tests + +@Suite("OAuthManager Token Cleanup") +struct OAuthManagerTokenCleanupTests { + private func makeConfig() -> RxAuthConfiguration { + RxAuthConfiguration( + issuer: "https://auth.example.com", + clientID: "test-client", + redirectURI: "testapp://callback" + ) + } + + // MARK: - logout() Tests + + @Test @MainActor func logoutClearsAllTokens() async throws { + let storage = InMemoryTokenStorage() + try storage.saveAccessToken("access-123") + try storage.saveRefreshToken("refresh-456") + try storage.saveExpiresAt(Date().addingTimeInterval(3600)) + + let manager = OAuthManager( + configuration: makeConfig(), + tokenStorage: storage + ) + + await manager.logout() + + #expect(storage.getAccessToken() == nil) + #expect(storage.getRefreshToken() == nil) + #expect(storage.getExpiresAt() == nil) + #expect(manager.authState == .unauthenticated) + #expect(manager.currentUser == nil) + } + + @Test @MainActor func logoutClearsTokensWhenClearAllFails() async throws { + let storage = PartiallyFailingTokenStorage() + try storage.saveAccessToken("access-123") + try storage.saveRefreshToken("refresh-456") + try storage.saveExpiresAt(Date().addingTimeInterval(3600)) + + let manager = OAuthManager( + configuration: makeConfig(), + tokenStorage: storage + ) + + await manager.logout() + + // Even though clearAll() throws, individual deletes should remove tokens + #expect(storage.getAccessToken() == nil) + #expect(storage.getRefreshToken() == nil) + #expect(manager.authState == .unauthenticated) + #expect(manager.currentUser == nil) + } + + // MARK: - checkExistingAuth() Tests + + @Test @MainActor func checkExistingAuthUnauthenticatedWithNoTokens() async { + let storage = InMemoryTokenStorage() + let manager = OAuthManager( + configuration: makeConfig(), + tokenStorage: storage + ) + + await manager.checkExistingAuth() + + #expect(manager.authState == .unauthenticated) + } + + @Test @MainActor func checkExistingAuthClearsStaleTokensAfterRefreshFailure() async throws { + // Register mock URL protocol to return 400 for token refresh + URLProtocol.registerClass(MockURLProtocol.self) + defer { URLProtocol.unregisterClass(MockURLProtocol.self) } + + MockURLProtocol.requestHandler = { request in + let response = HTTPURLResponse( + url: request.url!, + statusCode: 400, + httpVersion: nil, + headerFields: nil + )! + return (response, Data()) + } + + let storage = InMemoryTokenStorage() + // Set up stale tokens: expired access token + refresh token + try storage.saveAccessToken("expired-access") + try storage.saveRefreshToken("stale-refresh") + try storage.saveExpiresAt(Date().addingTimeInterval(-3600)) // expired 1 hour ago + + let manager = OAuthManager( + configuration: makeConfig(), + tokenStorage: storage + ) + + // First call finds stale refresh token, tries refresh → 400 → clears tokens + await manager.checkExistingAuth() + + #expect(manager.authState == .unauthenticated) + // Tokens must be cleared so the next checkExistingAuth() won't retry + #expect(storage.getAccessToken() == nil) + #expect(storage.getRefreshToken() == nil) + } + + @Test @MainActor func checkExistingAuthDoesNotRetryAfterTokensCleared() async throws { + // Register mock URL protocol to track requests + URLProtocol.registerClass(MockURLProtocol.self) + defer { URLProtocol.unregisterClass(MockURLProtocol.self) } + + var requestCount = 0 + let lock = NSLock() + MockURLProtocol.requestHandler = { request in + lock.lock() + requestCount += 1 + lock.unlock() + let response = HTTPURLResponse( + url: request.url!, + statusCode: 400, + httpVersion: nil, + headerFields: nil + )! + return (response, Data()) + } + + let storage = InMemoryTokenStorage() + try storage.saveAccessToken("expired-access") + try storage.saveRefreshToken("stale-refresh") + try storage.saveExpiresAt(Date().addingTimeInterval(-3600)) + + let manager = OAuthManager( + configuration: makeConfig(), + tokenStorage: storage + ) + + // First call: finds stale refresh token, tries refresh, fails, clears tokens + await manager.checkExistingAuth() + #expect(manager.authState == .unauthenticated) + + let firstCallCount = requestCount + + // Second call: no tokens should remain, so no refresh attempt + await manager.checkExistingAuth() + #expect(manager.authState == .unauthenticated) + #expect(requestCount == firstCallCount, "Should not make additional refresh requests after tokens are cleared") + } + + @Test @MainActor func fullCycleRefreshFailureThenRecheck() async throws { + // Simulate the full bug scenario: + // 1. User has tokens + // 2. Refresh fails with 400 + // 3. Tokens should be fully cleared + // 4. Subsequent checkExistingAuth() should NOT attempt to refresh + + URLProtocol.registerClass(MockURLProtocol.self) + defer { URLProtocol.unregisterClass(MockURLProtocol.self) } + + MockURLProtocol.requestHandler = { request in + let response = HTTPURLResponse( + url: request.url!, + statusCode: 400, + httpVersion: nil, + headerFields: nil + )! + return (response, Data()) + } + + let storage = InMemoryTokenStorage() + try storage.saveAccessToken("old-access") + try storage.saveRefreshToken("old-refresh") + try storage.saveExpiresAt(Date().addingTimeInterval(-100)) // expired + + let manager = OAuthManager( + configuration: makeConfig(), + tokenStorage: storage + ) + + // Step 1: Check existing auth → refresh fails → signs out + await manager.checkExistingAuth() + #expect(manager.authState == .unauthenticated) + #expect(storage.getAccessToken() == nil) + #expect(storage.getRefreshToken() == nil) + + // Step 2: Simulate user signing in again by saving new tokens + try storage.saveAccessToken("new-access") + try storage.saveRefreshToken("new-refresh") + try storage.saveExpiresAt(Date().addingTimeInterval(3600)) // valid + + // Step 3: Now update the mock to return valid responses for the new session + MockURLProtocol.requestHandler = { request in + let userInfoJSON = #"{"id":"user-1","name":"Test User","email":"test@example.com"}"# + let response = HTTPURLResponse( + url: request.url!, + statusCode: 200, + httpVersion: nil, + headerFields: ["Content-Type": "application/json"] + )! + return (response, userInfoJSON.data(using: .utf8)!) + } + + // Step 4: Check existing auth with new valid tokens + await manager.checkExistingAuth() + #expect(manager.authState == .authenticated) + #expect(manager.currentUser?.id == "user-1") + } +} From d3474830b317afc4a8ce3bada91f54fecd491d8c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 17:56:41 +0000 Subject: [PATCH 3/3] Address code review: rename test class, fix handler assignment order Co-authored-by: sirily11 <32106111+sirily11@users.noreply.github.com> --- .../OAuthManagerTokenCleanupTests.swift | 57 ++++++++++++------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/Tests/RxAuthSwiftTests/OAuthManagerTokenCleanupTests.swift b/Tests/RxAuthSwiftTests/OAuthManagerTokenCleanupTests.swift index 9059b23..17d7668 100644 --- a/Tests/RxAuthSwiftTests/OAuthManagerTokenCleanupTests.swift +++ b/Tests/RxAuthSwiftTests/OAuthManagerTokenCleanupTests.swift @@ -4,7 +4,7 @@ import Testing // MARK: - Mock Token Storage (clearAll fails but individual deletes work) -private final class PartiallyFailingTokenStorage: TokenStorageProtocol, @unchecked Sendable { +private final class ClearAllFailingTokenStorage: TokenStorageProtocol, @unchecked Sendable { private let lock = NSLock() private var accessToken: String? private var refreshToken: String? @@ -97,9 +97,28 @@ private final class MockURLProtocol: URLProtocol, @unchecked Sendable { override func stopLoading() {} } +// MARK: - Thread-safe Counter + +private final class LockedCounter: @unchecked Sendable { + private let lock = NSLock() + private var _value = 0 + + var value: Int { + lock.lock() + defer { lock.unlock() } + return _value + } + + func increment() { + lock.lock() + defer { lock.unlock() } + _value += 1 + } +} + // MARK: - Tests -@Suite("OAuthManager Token Cleanup") +@Suite("OAuthManager Token Cleanup", .serialized) struct OAuthManagerTokenCleanupTests { private func makeConfig() -> RxAuthConfiguration { RxAuthConfiguration( @@ -132,7 +151,7 @@ struct OAuthManagerTokenCleanupTests { } @Test @MainActor func logoutClearsTokensWhenClearAllFails() async throws { - let storage = PartiallyFailingTokenStorage() + let storage = ClearAllFailingTokenStorage() try storage.saveAccessToken("access-123") try storage.saveRefreshToken("refresh-456") try storage.saveExpiresAt(Date().addingTimeInterval(3600)) @@ -166,10 +185,7 @@ struct OAuthManagerTokenCleanupTests { } @Test @MainActor func checkExistingAuthClearsStaleTokensAfterRefreshFailure() async throws { - // Register mock URL protocol to return 400 for token refresh - URLProtocol.registerClass(MockURLProtocol.self) - defer { URLProtocol.unregisterClass(MockURLProtocol.self) } - + // Set up mock handler before registering protocol to avoid race conditions MockURLProtocol.requestHandler = { request in let response = HTTPURLResponse( url: request.url!, @@ -180,6 +196,9 @@ struct OAuthManagerTokenCleanupTests { return (response, Data()) } + URLProtocol.registerClass(MockURLProtocol.self) + defer { URLProtocol.unregisterClass(MockURLProtocol.self) } + let storage = InMemoryTokenStorage() // Set up stale tokens: expired access token + refresh token try storage.saveAccessToken("expired-access") @@ -201,16 +220,9 @@ struct OAuthManagerTokenCleanupTests { } @Test @MainActor func checkExistingAuthDoesNotRetryAfterTokensCleared() async throws { - // Register mock URL protocol to track requests - URLProtocol.registerClass(MockURLProtocol.self) - defer { URLProtocol.unregisterClass(MockURLProtocol.self) } - - var requestCount = 0 - let lock = NSLock() + let counter = LockedCounter() MockURLProtocol.requestHandler = { request in - lock.lock() - requestCount += 1 - lock.unlock() + counter.increment() let response = HTTPURLResponse( url: request.url!, statusCode: 400, @@ -220,6 +232,9 @@ struct OAuthManagerTokenCleanupTests { return (response, Data()) } + URLProtocol.registerClass(MockURLProtocol.self) + defer { URLProtocol.unregisterClass(MockURLProtocol.self) } + let storage = InMemoryTokenStorage() try storage.saveAccessToken("expired-access") try storage.saveRefreshToken("stale-refresh") @@ -234,12 +249,12 @@ struct OAuthManagerTokenCleanupTests { await manager.checkExistingAuth() #expect(manager.authState == .unauthenticated) - let firstCallCount = requestCount + let firstCallCount = counter.value // Second call: no tokens should remain, so no refresh attempt await manager.checkExistingAuth() #expect(manager.authState == .unauthenticated) - #expect(requestCount == firstCallCount, "Should not make additional refresh requests after tokens are cleared") + #expect(counter.value == firstCallCount, "Should not make additional refresh requests after tokens are cleared") } @Test @MainActor func fullCycleRefreshFailureThenRecheck() async throws { @@ -249,9 +264,6 @@ struct OAuthManagerTokenCleanupTests { // 3. Tokens should be fully cleared // 4. Subsequent checkExistingAuth() should NOT attempt to refresh - URLProtocol.registerClass(MockURLProtocol.self) - defer { URLProtocol.unregisterClass(MockURLProtocol.self) } - MockURLProtocol.requestHandler = { request in let response = HTTPURLResponse( url: request.url!, @@ -262,6 +274,9 @@ struct OAuthManagerTokenCleanupTests { return (response, Data()) } + URLProtocol.registerClass(MockURLProtocol.self) + defer { URLProtocol.unregisterClass(MockURLProtocol.self) } + let storage = InMemoryTokenStorage() try storage.saveAccessToken("old-access") try storage.saveRefreshToken("old-refresh")