[PM-24195] fix: Pass userId to refreshToken and setTokens

BitwardenShared/Core/Platform/Services/API/APIService.swift

@@ -44,6 +44,7 @@ class APIService {
     ///   - client: The underlying `HTTPClient` that performs the network request. Defaults
     ///     to `URLSession.shared`.
     ///   - environmentService: The service used by the application to retrieve the environment settings.
+    ///   - errorReporter: The service used by the application to report non-fatal errors.
     ///   - flightRecorder: The service used by the application for recording temporary debug logs.
     ///   - serverCommunicationConfigClientSingleton: The service to get the server communication client
     ///   used to break circular dependency.
@@ -55,6 +56,7 @@ class APIService {
         accountTokenProvider: AccountTokenProvider? = nil,
         client: HTTPClient = URLSession.shared,
         environmentService: EnvironmentService,
+        errorReporter: ErrorReporter,
         flightRecorder: FlightRecorder,
         serverCommunicationConfigClientSingleton: @escaping () -> ServerCommunicationConfigClientSingleton?,
         stateService: StateService,
@@ -85,6 +87,8 @@ class APIService {
         self.accountTokenProvider = accountTokenProvider ?? DefaultAccountTokenProvider(
             httpService: httpServiceBuilder.makeService(baseURLGetter: { environmentService.identityURL }),
             tokenService: tokenService,
+            errorReporter: errorReporter,
+            stateService: stateService
         )
 
         apiService = httpServiceBuilder.makeService(

BitwardenShared/Core/Platform/Services/API/AccountTokenProvider.swift

@@ -22,12 +22,18 @@ actor DefaultAccountTokenProvider: AccountTokenProvider {
     /// The delegate to use for specific operations on the token provider.
     private weak var accountTokenProviderDelegate: AccountTokenProviderDelegate?
 
+    /// The service used to report non-fatal errors.
+    private let errorReporter: ErrorReporter
+
     /// The `HTTPService` used to make the API call to refresh the access token.
     private let httpService: HTTPService
 
     /// The task associated with refreshing the token, if one is in progress.
     private(set) var refreshTask: Task<String, Error>?
 
+    /// The service used to manage account state.
+    private let stateService: StateService
+
     /// The service used to get the present time.
     private let timeProvider: TimeProvider
 
@@ -42,15 +48,21 @@ actor DefaultAccountTokenProvider: AccountTokenProvider {
     ///   - httpService: The service used to make the API call to refresh the access token.
     ///   - timeProvider: The service used to get the present time.
     ///   - tokenService: The service used to get the current tokens from.
+    ///   - errorReporter: The service used to report non-fatal errors.
+    ///   - stateService: The service used to manage account state.
     ///
     init(
         httpService: HTTPService,
         timeProvider: TimeProvider = CurrentTime(),
         tokenService: TokenService,
+        errorReporter: ErrorReporter,
+        stateService: StateService,
     ) {
         self.httpService = httpService
         self.timeProvider = timeProvider
         self.tokenService = tokenService
+        self.errorReporter = errorReporter
+        self.stateService = stateService
     }
 
     // MARK: Methods
@@ -81,6 +93,9 @@ actor DefaultAccountTokenProvider: AccountTokenProvider {
             defer { self.refreshTask = nil }
 
             do {
+                // TODO: PM-33074 Remove logs after confirmation that the error doesn't happen anymore.
+                let userIdBefore = try await stateService.getActiveAccountId()
+
                 let refreshToken = try await tokenService.getRefreshToken()
                 let response = try await httpService.send(
                     IdentityTokenRefreshRequest(refreshToken: refreshToken),
@@ -93,6 +108,19 @@ actor DefaultAccountTokenProvider: AccountTokenProvider {
                     expirationDate: expirationDate,
                 )
 
+                do {
+                    let userIdAfter = try await stateService.getActiveAccountId()
+                    if userIdBefore != userIdAfter {
+                        let error = AccountTokenProviderError(
+                            userIdBefore: userIdBefore,
+                            userIdAfter: userIdAfter,
+                        )
+                        errorReporter.log(error: error)
+                    }
+                } catch {
+                    errorReporter.log(error: error)
+                }
+
                 return response.accessToken
             } catch {
                 if let accountTokenProviderDelegate {

BitwardenShared/Core/Platform/Services/API/AccountTokenProviderError.swift

@@ -0,0 +1,24 @@
+import Foundation
+
+// MARK: - AccountTokenProviderError
+
+/// Error logged when the active account changes during a token refresh operation.
+///
+struct AccountTokenProviderError: Error, CustomStringConvertible {
+    // MARK: Properties
+
+    /// The active user ID before the token refresh operation.
+    let userIdBefore: String
+
+    /// The active user ID after the token refresh operation.
+    let userIdAfter: String
+
+    // MARK: CustomStringConvertible
+
+    var description: String {
+        """
+        Token refresh race condition detected: Active account changed from '\(userIdBefore)' to '\(userIdAfter)' \
+        during token refresh operation. Tokens may have been stored under wrong account.
+        """
+    }
+}

BitwardenShared/Core/Platform/Services/API/AccountTokenProviderTests.swift

@@ -11,6 +11,8 @@ class AccountTokenProviderTests: BitwardenTestCase {
     // MARK: Properties
 
     var client: MockHTTPClient!
+    var errorReporter: MockErrorReporter!
+    var stateService: MockStateService!
     var subject: DefaultAccountTokenProvider!
     var timeProvider: MockTimeProvider!
     var tokenService: MockTokenService!
@@ -25,20 +27,27 @@ class AccountTokenProviderTests: BitwardenTestCase {
         super.setUp()
 
         client = MockHTTPClient()
+        errorReporter = MockErrorReporter()
+        stateService = MockStateService()
+        stateService.activeAccount = .fixture()
         timeProvider = MockTimeProvider(.mockTime(Date(year: 2025, month: 10, day: 2)))
         tokenService = MockTokenService()
 
         subject = DefaultAccountTokenProvider(
             httpService: HTTPService(baseURL: URL(string: "https://example.com")!, client: client),
             timeProvider: timeProvider,
             tokenService: tokenService,
+            errorReporter: errorReporter,
+            stateService: stateService,
         )
     }
 
     override func tearDown() async throws {
         try await super.tearDown()
 
         client = nil
+        errorReporter = nil
+        stateService = nil
         subject = nil
         timeProvider = nil
         tokenService = nil
@@ -171,4 +180,63 @@ class AccountTokenProviderTests: BitwardenTestCase {
             _ = try await subject.refreshToken()
         }
     }
+
+    /// `refreshToken()` logs an error when the active account changes during the token refresh operation.
+    func test_refreshToken_logsRaceCondition_whenUserIdChanges() async throws {
+        stateService.activeAccount = .fixture(profile: .fixture(userId: "user-1"))
+        tokenService.accessToken = "🔑"
+        tokenService.refreshToken = "🔒"
+
+        client.result = .httpSuccess(testData: .identityTokenRefresh)
+
+        // Simulate account switch during HTTP request
+        client.onRequest = { _ in
+            self.stateService.activeAccount = .fixture(profile: .fixture(userId: "user-2"))
+        }
+
+        _ = try await subject.refreshToken()
+
+        // Verify error was logged
+        XCTAssertEqual(errorReporter.errors.count, 1)
+        let error = errorReporter.errors[0] as? AccountTokenProviderError
+        XCTAssertNotNil(error)
+        XCTAssertEqual(error?.userIdBefore, "user-1")
+        XCTAssertEqual(error?.userIdAfter, "user-2")
+    }
+
+    /// `refreshToken()` does not log an error when the active account remains the same.
+    func test_refreshToken_doesNotLogError_whenUserIdStaysSame() async throws {
+        stateService.activeAccount = .fixture(profile: .fixture(userId: "user-1"))
+        tokenService.accessToken = "🔑"
+        tokenService.refreshToken = "🔒"
+
+        client.result = .httpSuccess(testData: .identityTokenRefresh)
+
+        _ = try await subject.refreshToken()
+
+        // Verify no error was logged
+        XCTAssertEqual(errorReporter.errors.count, 0)
+    }
+
+    /// `refreshToken()` logs an error when `getActiveAccountId` throws after tokens are stored,
+    /// but still returns the access token successfully.
+    func test_refreshToken_logsError_whenGetUserIdAfterThrows() async throws {
+        tokenService.accessToken = "🔑"
+        tokenService.refreshToken = "🔒"
+        client.result = .httpSuccess(testData: .identityTokenRefresh)
+
+        // Remove the active account during the HTTP request so the second
+        // getActiveAccountId() call (after setTokens) throws noActiveAccount.
+        client.onRequest = { _ in
+            self.stateService.activeAccount = nil
+        }
+
+        let token = try await subject.refreshToken()
+
+        // Token is still returned successfully despite the diagnostic check throwing
+        XCTAssertEqual(token, "ACCESS_TOKEN")
+        // The thrown error was logged via errorReporter
+        XCTAssertEqual(errorReporter.errors.count, 1)
+        XCTAssertEqual(errorReporter.errors[0] as? StateServiceError, StateServiceError.noActiveAccount)
+    }
 }

BitwardenShared/Core/Platform/Services/API/RefreshableAPIServiceTests.swift

@@ -21,6 +21,7 @@ class RefreshableAPIServiceTests: BitwardenTestCase {
         subject = APIService(
             accountTokenProvider: accountTokenProvider,
             environmentService: MockEnvironmentService(),
+            errorReporter: MockErrorReporter(),
             flightRecorder: MockFlightRecorder(),
             serverCommunicationConfigClientSingleton: { MockServerCommunicationConfigClientSingleton() },
             stateService: MockStateService(),

BitwardenShared/Core/Platform/Services/API/TestHelpers/APIService+Mocks.swift

@@ -9,6 +9,7 @@ extension APIService {
         accountTokenProvider: AccountTokenProvider? = nil,
         client: HTTPClient,
         environmentService: EnvironmentService = MockEnvironmentService(),
+        errorReporter: ErrorReporter = MockErrorReporter(),
         flightRecorder: FlightRecorder = MockFlightRecorder(),
         // swiftlint:disable:next line_length
         serverCommunicationConfigClientSingleton: ServerCommunicationConfigClientSingleton = MockServerCommunicationConfigClientSingleton(),
@@ -18,6 +19,7 @@ extension APIService {
             accountTokenProvider: accountTokenProvider,
             client: client,
             environmentService: environmentService,
+            errorReporter: errorReporter,
             flightRecorder: flightRecorder,
             serverCommunicationConfigClientSingleton: { serverCommunicationConfigClientSingleton },
             stateService: stateService,

BitwardenShared/Core/Platform/Services/ServiceContainer.swift

@@ -510,6 +510,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le
         let apiService = APIService(
             client: noRedirectSession,
             environmentService: environmentService,
+            errorReporter: errorReporter,
             flightRecorder: flightRecorder,
             serverCommunicationConfigClientSingleton: { serverCommConfigClientSingletonHolder },
             stateService: stateService,

BitwardenShared/UI/Vault/VaultItem/AddEditItem/AddEditItemProcessor.swift

@@ -698,7 +698,16 @@ final class AddEditItemProcessor: StateProcessor<// swiftlint:disable:this type_
             return
         } catch {
             await coordinator.showErrorAlert(error: error)
-            services.errorReporter.log(error: error)
+            if case let ServerError.error(errorResponse: errorResponse) = error,
+               errorResponse.message.contains("Cipher was not encrypted for the current user") {
+                services.errorReporter.log(error: BitwardenError.generalError(
+                    type: "Save item failed",
+                    message: errorResponse.message,
+                    error: error,
+                ))
+            } else {
+                services.errorReporter.log(error: error)
+            }
         }
     }
 

BitwardenShared/UI/Vault/VaultItem/AddEditItem/AddEditItemProcessorTests.swift

@@ -1391,6 +1391,39 @@ class AddEditItemProcessorTests: BitwardenTestCase {
         XCTAssertEqual(coordinator.errorAlertsShown as? [ServerError], [error])
     }
 
+    /// `perform(_:)` with `.savePressed` logs a `BitwardenError.generalError` when saving
+    /// fails with a `ServerError.error` carrying the targeted cipher-encryption message,
+    /// allowing it to reach Crashlytics despite `ServerError` being a `NonLoggableError`.
+    @MainActor
+    func test_perform_savePressed_serverError_logsGeneralError() async throws {
+        let body = #"{"Message":"Cipher was not encrypted for the current user","Object":"error"}"#
+        let response = HTTPResponse.failure(statusCode: 400, body: Data(body.utf8))
+        let error = try ServerError.error(errorResponse: ErrorResponseModel(response: response))
+        vaultRepository.addCipherResult = .failure(error)
+        subject.state.name = "vault item"
+
+        await subject.perform(.savePressed)
+
+        XCTAssertEqual(errorReporter.errors.count, 1)
+        let generalError = try XCTUnwrap(errorReporter.errors.last as? NSError)
+        XCTAssertEqual(generalError.domain, "General Error: Save item failed")
+    }
+
+    /// `perform(_:)` with `.savePressed` does not wrap the error in a `BitwardenError.generalError`
+    /// when a `ServerError.error` carries a message other than the targeted cipher-encryption
+    /// message. The raw `ServerError` is still sent to the reporter via the generic catch-all log.
+    @MainActor
+    func test_perform_savePressed_serverError_otherMessage_doesNotLog() async throws {
+        let response = HTTPResponse.failure(statusCode: 400, body: APITestData.bitwardenErrorMessage.data)
+        let error = try ServerError.error(errorResponse: ErrorResponseModel(response: response))
+        vaultRepository.addCipherResult = .failure(error)
+        subject.state.name = "vault item"
+
+        await subject.perform(.savePressed)
+
+        XCTAssertEqual(errorReporter.errors.count, 1)
+    }
+
     /// `perform(_:)` with `.savePressed` saves the item.
     @MainActor
     func test_perform_savePressed_secureNote() async {

TestHelpers/API/MockHTTPClient.swift

@@ -33,6 +33,10 @@ public final class MockHTTPClient: HTTPClient {
     /// A list of results that will be returned in order for future requests.
     public var results: [Result<HTTPResponse, Error>] = []
 
+    /// An optional callback that is invoked when a request is sent, useful for simulating
+    /// state changes during async operations in tests.
+    public var onRequest: ((HTTPRequest) -> Void)?
+
     // MARK: Initializer
 
     /// Initializes a `MockHTTPClient`.
@@ -62,6 +66,9 @@ public final class MockHTTPClient: HTTPClient {
     public func send(_ request: HTTPRequest) async throws -> HTTPResponse {
         requests.append(request)
 
+        // Invoke callback if provided (useful for simulating state changes during async operations)
+        onRequest?(request)
+
         guard !results.isEmpty else { throw MockHTTPClientError.noResultForRequest }
 
         let result = results.removeFirst()