From 35a35ee881d08fc190b9e0b5a2d9edc99b8e59bb Mon Sep 17 00:00:00 2001 From: Federico Maccaroni Date: Fri, 27 Jun 2025 17:32:46 -0300 Subject: [PATCH] [PM-22811] fix: Implement invalid_grant on 400 response behavior while refreshing token (#1694) Co-authored-by: Matt Czech --- .../CrashlyticsErrorReporter.swift | 3 +- .../CrashlyticsErrorReporter.swift | 3 +- .../Extensions/Error+Networking.swift | 9 ++- .../Extensions/ErrorNetworkingTests.swift | 22 ++++++-- .../API/Errors/NonLoggableError.swift | 2 + .../Services/API/Errors/ServerError.swift | 2 +- .../Handlers/ResponseValidationHandler.swift | 2 +- .../Utilities/OSLogErrorReporter.swift | 3 +- .../API/Auth/Fixtures/APITestData+Auth.swift | 4 ++ ...IdentityTokenRefreshInvalidGrantError.json | 3 + .../IdentityTokenRefreshStubError.json | 3 + .../IdentityTokenRefreshRequest.swift | 27 +++++++++ .../IdentityTokenRefreshRequestTests.swift | 35 ++++++++++++ .../Platform/Services/API/APIService.swift | 15 ++++- .../Services/API/APIServiceTests.swift | 12 +++- .../Services/API/AccountTokenProvider.swift | 52 ++++++++++++++---- .../API/AccountTokenProviderTests.swift | 34 +++++++++++- .../API/TestHelpers/APIService+Mocks.swift | 2 + .../MockAccountTokenProvider.swift | 36 ++++++++++++ .../Platform/Application/AppProcessor.swift | 30 +++++++++- .../Application/AppProcessorTests.swift | 55 +++++++++++++++++++ 21 files changed, 316 insertions(+), 38 deletions(-) create mode 100644 BitwardenKit/Core/Platform/Services/API/Errors/NonLoggableError.swift create mode 100644 BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/IdentityTokenRefreshInvalidGrantError.json create mode 100644 BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/IdentityTokenRefreshStubError.json create mode 100644 BitwardenShared/Core/Platform/Services/API/TestHelpers/MockAccountTokenProvider.swift diff --git a/Authenticator/Application/Services/ErrorReporter/CrashlyticsErrorReporter.swift b/Authenticator/Application/Services/ErrorReporter/CrashlyticsErrorReporter.swift index 111cd7fa9..404e53e7c 100644 --- a/Authenticator/Application/Services/ErrorReporter/CrashlyticsErrorReporter.swift +++ b/Authenticator/Application/Services/ErrorReporter/CrashlyticsErrorReporter.swift @@ -40,8 +40,7 @@ final class CrashlyticsErrorReporter: ErrorReporter { logger.log("Error: \(error)\n\(callStack)") } - // Don't log networking related errors to Crashlytics. - guard !error.isNetworkingError else { return } + guard !error.isNonLoggableError else { return } Crashlytics.crashlytics().record(error: error) } diff --git a/Bitwarden/Application/Services/ErrorReporter/CrashlyticsErrorReporter.swift b/Bitwarden/Application/Services/ErrorReporter/CrashlyticsErrorReporter.swift index dbd4fdf7b..1fbe11fba 100644 --- a/Bitwarden/Application/Services/ErrorReporter/CrashlyticsErrorReporter.swift +++ b/Bitwarden/Application/Services/ErrorReporter/CrashlyticsErrorReporter.swift @@ -45,8 +45,7 @@ final class CrashlyticsErrorReporter: ErrorReporter { logger.log("Error: \(error)\n\(callStack)") } - // Don't log networking related errors to Crashlytics. - guard !error.isNetworkingError else { return } + guard !error.isNonLoggableError else { return } Crashlytics.crashlytics().record(error: error) } diff --git a/BitwardenKit/Core/Platform/Extensions/Error+Networking.swift b/BitwardenKit/Core/Platform/Extensions/Error+Networking.swift index 7414cd543..6dc304715 100644 --- a/BitwardenKit/Core/Platform/Extensions/Error+Networking.swift +++ b/BitwardenKit/Core/Platform/Extensions/Error+Networking.swift @@ -1,14 +1,13 @@ import Foundation public extension Error { - /// Attempts to determine if the error is a networking related error. This can be useful for - /// deciding if the error should be logged to an external error reporting service where + /// Attempts to determine if the error should not be logged, like a networking related error. + /// This can be useful for deciding if the error should be logged to an external error reporting service where /// networking or server errors may add noise instead of being actionable errors that need to /// be fixed in the app. - var isNetworkingError: Bool { + var isNonLoggableError: Bool { switch self { - case is ResponseValidationError, // Bitwarden Server specific errors. - is ServerError, // Any other non-2XX HTTP errors. + case is NonLoggableError, // Any error marked as `NetworkingError` is URLError: // URLSession errors. true default: diff --git a/BitwardenKit/Core/Platform/Extensions/ErrorNetworkingTests.swift b/BitwardenKit/Core/Platform/Extensions/ErrorNetworkingTests.swift index 093dd8fcd..1d2320f66 100644 --- a/BitwardenKit/Core/Platform/Extensions/ErrorNetworkingTests.swift +++ b/BitwardenKit/Core/Platform/Extensions/ErrorNetworkingTests.swift @@ -12,7 +12,7 @@ class ErrorNetworkingTests: BitwardenTestCase { func test_isNetworkingError_other() { struct NonNetworkingError: Error {} - XCTAssertFalse(NonNetworkingError().isNetworkingError) + XCTAssertFalse(NonNetworkingError().isNonLoggableError) } /// `isNetworkingError` returns `true` for `ResponseValidationError`s. @@ -20,7 +20,7 @@ class ErrorNetworkingTests: BitwardenTestCase { let response = HTTPResponse.failure(statusCode: 500) let error = ResponseValidationError(response: response) - XCTAssertTrue(error.isNetworkingError) + XCTAssertTrue(error.isNonLoggableError) } /// `isNetworkingError` returns `true` for `ServerError`s. @@ -28,13 +28,23 @@ class ErrorNetworkingTests: BitwardenTestCase { let response = HTTPResponse.failure(statusCode: 400, body: APITestData.bitwardenErrorMessage.data) let error = try ServerError.error(errorResponse: ErrorResponseModel(response: response)) - XCTAssertTrue(error.isNetworkingError) + XCTAssertTrue(error.isNonLoggableError) } /// `isNetworkingError` returns `true` for `URLError`s. func test_isNetworkingError_urlError() throws { - XCTAssertTrue(URLError(.cancelled).isNetworkingError) - XCTAssertTrue(URLError(.networkConnectionLost).isNetworkingError) - XCTAssertTrue(URLError(.timedOut).isNetworkingError) + XCTAssertTrue(URLError(.cancelled).isNonLoggableError) + XCTAssertTrue(URLError(.networkConnectionLost).isNonLoggableError) + XCTAssertTrue(URLError(.timedOut).isNonLoggableError) + } + + /// `isNetworkingError` returns `true` for custom `NetworkingError`. + func test_isNetworkingError_networkingError() throws { + XCTAssertTrue(TestNetworkingError.test.isNonLoggableError) } } + +/// Error to be used as test for `NetworkingError`. +enum TestNetworkingError: NonLoggableError { + case test +} diff --git a/BitwardenKit/Core/Platform/Services/API/Errors/NonLoggableError.swift b/BitwardenKit/Core/Platform/Services/API/Errors/NonLoggableError.swift new file mode 100644 index 000000000..7994cabc0 --- /dev/null +++ b/BitwardenKit/Core/Platform/Services/API/Errors/NonLoggableError.swift @@ -0,0 +1,2 @@ +/// A protocol used to mark errors that shouldn't be logged by log reporters. +public protocol NonLoggableError: Error {} diff --git a/BitwardenKit/Core/Platform/Services/API/Errors/ServerError.swift b/BitwardenKit/Core/Platform/Services/API/Errors/ServerError.swift index 3cb08fd12..1a8f618ed 100644 --- a/BitwardenKit/Core/Platform/Services/API/Errors/ServerError.swift +++ b/BitwardenKit/Core/Platform/Services/API/Errors/ServerError.swift @@ -4,7 +4,7 @@ import Foundation /// An enumeration of server errors. /// -public enum ServerError: Error, Equatable, CustomNSError { +public enum ServerError: NonLoggableError, Equatable, CustomNSError { /// A generic error. /// /// - Parameter errorResponse: The error response returned from the API request. diff --git a/BitwardenKit/Core/Platform/Services/API/Handlers/ResponseValidationHandler.swift b/BitwardenKit/Core/Platform/Services/API/Handlers/ResponseValidationHandler.swift index d75781a9e..18e2c6144 100644 --- a/BitwardenKit/Core/Platform/Services/API/Handlers/ResponseValidationHandler.swift +++ b/BitwardenKit/Core/Platform/Services/API/Handlers/ResponseValidationHandler.swift @@ -4,7 +4,7 @@ import Networking /// An error indicating that the response was invalid and didn't contain a successful HTTP status code. /// -public struct ResponseValidationError: Error, Equatable { +public struct ResponseValidationError: NonLoggableError, Equatable { // MARK: Properties /// The received HTTP response. diff --git a/BitwardenKit/Core/Platform/Utilities/OSLogErrorReporter.swift b/BitwardenKit/Core/Platform/Utilities/OSLogErrorReporter.swift index a4b574ed4..0920b7825 100644 --- a/BitwardenKit/Core/Platform/Utilities/OSLogErrorReporter.swift +++ b/BitwardenKit/Core/Platform/Utilities/OSLogErrorReporter.swift @@ -35,8 +35,7 @@ public final class OSLogErrorReporter: ErrorReporter { logger.log("Error: \(error as NSError)\n\(callStack)") } - // Don't crash for networking related errors. - guard !error.isNetworkingError else { return } + guard !error.isNonLoggableError else { return } // Crash in debug builds to make the error more visible during development. assertionFailure("Unexpected error: \(error)") diff --git a/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/APITestData+Auth.swift b/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/APITestData+Auth.swift index cabddd43e..8acf4c41a 100644 --- a/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/APITestData+Auth.swift +++ b/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/APITestData+Auth.swift @@ -19,6 +19,10 @@ extension APITestData { ) static let identityTokenNoMasterPassword = loadFromJsonBundle(resource: "IdentityTokenNoMasterPassword") static let identityTokenRefresh = loadFromJsonBundle(resource: "identityTokenRefresh") + static let identityTokenRefreshInvalidGrantError = loadFromJsonBundle( + resource: "IdentityTokenRefreshInvalidGrantError" + ) + static let identityTokenRefreshStubError = loadFromJsonBundle(resource: "IdentityTokenRefreshStubError") static let identityTokenTrustedDevice = loadFromJsonBundle(resource: "IdentityTokenTrustedDevice") static let identityTokenTwoFactorError = loadFromJsonBundle(resource: "IdentityTokenTwoFactorFailure") static let preValidateSingleSignOn = loadFromJsonBundle(resource: "preValidateSingleSignOn") diff --git a/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/IdentityTokenRefreshInvalidGrantError.json b/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/IdentityTokenRefreshInvalidGrantError.json new file mode 100644 index 000000000..e4ec9ec4c --- /dev/null +++ b/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/IdentityTokenRefreshInvalidGrantError.json @@ -0,0 +1,3 @@ +{ + "error": "invalid_grant" +} diff --git a/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/IdentityTokenRefreshStubError.json b/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/IdentityTokenRefreshStubError.json new file mode 100644 index 000000000..d59d456da --- /dev/null +++ b/BitwardenShared/Core/Auth/Services/API/Auth/Fixtures/IdentityTokenRefreshStubError.json @@ -0,0 +1,3 @@ +{ + "error": "stub" +} diff --git a/BitwardenShared/Core/Auth/Services/API/Auth/Requests/IdentityTokenRefreshRequest.swift b/BitwardenShared/Core/Auth/Services/API/Auth/Requests/IdentityTokenRefreshRequest.swift index bc0d48364..3c09542e2 100644 --- a/BitwardenShared/Core/Auth/Services/API/Auth/Requests/IdentityTokenRefreshRequest.swift +++ b/BitwardenShared/Core/Auth/Services/API/Auth/Requests/IdentityTokenRefreshRequest.swift @@ -1,5 +1,16 @@ +import BitwardenKit import Networking +// MARK: - IdentityTokenRefreshRequestError + +/// Errors that can occur when sending an `IdentityTokenRefreshRequest`. +enum IdentityTokenRefreshRequestError: NonLoggableError, Equatable { + /// Not allowed because of invalid grant. + case invalidGrant +} + +// MARK: - IdentityTokenRefreshRequest + /// Data model for performing a identity token refresh request. /// struct IdentityTokenRefreshRequest: Request { @@ -30,4 +41,20 @@ struct IdentityTokenRefreshRequest: Request { init(refreshToken: String) { requestModel = IdentityTokenRefreshRequestModel(refreshToken: refreshToken) } + + // MARK: Methods + + func validate(_ response: HTTPResponse) throws { + if response.statusCode == 400 { + guard let errorModel = try? IdentityTokenErrorModel.decoder.decode( + IdentityTokenErrorModel.self, + from: response.body + ) else { return } + + if let error = errorModel.error, + error == IdentityTokenError.invalidGrant { + throw IdentityTokenRefreshRequestError.invalidGrant + } + } + } } diff --git a/BitwardenShared/Core/Auth/Services/API/Auth/Requests/IdentityTokenRefreshRequestTests.swift b/BitwardenShared/Core/Auth/Services/API/Auth/Requests/IdentityTokenRefreshRequestTests.swift index af468d7a4..bf8841fdd 100644 --- a/BitwardenShared/Core/Auth/Services/API/Auth/Requests/IdentityTokenRefreshRequestTests.swift +++ b/BitwardenShared/Core/Auth/Services/API/Auth/Requests/IdentityTokenRefreshRequestTests.swift @@ -1,3 +1,5 @@ +import Networking +import TestHelpers import XCTest @testable import BitwardenShared @@ -47,6 +49,39 @@ class IdentityTokenRefreshRequestTests: BitwardenTestCase { XCTAssertEqual(subject.path, "/connect/token") } + /// `validate(_:)` with a valid response does not throw a validation error. + func test_validate_with200() { + let response = HTTPResponse.success( + body: APITestData.identityTokenRefresh.data + ) + + XCTAssertNoThrow(try subject.validate(response)) + } + + /// `validate(_:)` with a `400` status code and non invalid grant in the response body + /// doesn't throw an error. + func test_validate_with400DoesntThrow() { + let response = HTTPResponse.failure( + statusCode: 400, + body: APITestData.identityTokenRefreshStubError.data + ) + + XCTAssertNoThrow(try subject.validate(response)) + } + + /// `validate(_:)` with a `400` status code and invalid grant in the response body throws a + /// `.invalidGrant` error. + func test_validate_with400InvalidGrantError() { + let response = HTTPResponse.failure( + statusCode: 400, + body: APITestData.identityTokenRefreshInvalidGrantError.data + ) + + XCTAssertThrowsError(try subject.validate(response)) { error in + XCTAssertEqual(error as? IdentityTokenRefreshRequestError, .invalidGrant) + } + } + /// `query` returns no query parameters. func test_query() { XCTAssertTrue(subject.query.isEmpty) diff --git a/BitwardenShared/Core/Platform/Services/API/APIService.swift b/BitwardenShared/Core/Platform/Services/API/APIService.swift index 4ad9f9086..490ac3456 100644 --- a/BitwardenShared/Core/Platform/Services/API/APIService.swift +++ b/BitwardenShared/Core/Platform/Services/API/APIService.swift @@ -39,6 +39,8 @@ class APIService { /// Initialize an `APIService` used to make API requests. /// /// - Parameters: + /// - accountTokenProvider: The `AccountTokenProvider` to use. This is helpful for testing. + /// The default will be built by default. /// - 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. @@ -48,6 +50,7 @@ class APIService { /// account's tokens. /// init( + accountTokenProvider: AccountTokenProvider? = nil, client: HTTPClient = URLSession.shared, environmentService: EnvironmentService, flightRecorder: FlightRecorder, @@ -70,21 +73,21 @@ class APIService { ] ) - accountTokenProvider = AccountTokenProvider( + self.accountTokenProvider = accountTokenProvider ?? DefaultAccountTokenProvider( httpService: httpServiceBuilder.makeService(baseURLGetter: { environmentService.identityURL }), tokenService: tokenService ) apiService = httpServiceBuilder.makeService( baseURLGetter: { environmentService.apiURL }, - tokenProvider: accountTokenProvider + tokenProvider: self.accountTokenProvider ) apiUnauthenticatedService = httpServiceBuilder.makeService( baseURLGetter: { environmentService.apiURL } ) eventsService = httpServiceBuilder.makeService( baseURLGetter: { environmentService.eventsURL }, - tokenProvider: accountTokenProvider + tokenProvider: self.accountTokenProvider ) hibpService = httpServiceBuilder.makeService( baseURLGetter: { URL(string: "https://api.pwnedpasswords.com")! } @@ -107,4 +110,10 @@ class APIService { tokenProvider: accountTokenProvider ) } + + /// Sets the account token provider delegate. + /// - Parameter delegate: The delegate to use. + func setAccountTokenProviderDelegate(delegate: AccountTokenProviderDelegate) async { + await accountTokenProvider.setDelegate(delegate: delegate) + } } diff --git a/BitwardenShared/Core/Platform/Services/API/APIServiceTests.swift b/BitwardenShared/Core/Platform/Services/API/APIServiceTests.swift index d79dc5d6b..a68adc3d9 100644 --- a/BitwardenShared/Core/Platform/Services/API/APIServiceTests.swift +++ b/BitwardenShared/Core/Platform/Services/API/APIServiceTests.swift @@ -6,17 +6,20 @@ import XCTest @testable import Networking class APIServiceTests: BitwardenTestCase { + var accountTokenProvider: MockAccountTokenProvider! var subject: APIService! override func setUp() { super.setUp() - subject = APIService(client: MockHTTPClient()) + accountTokenProvider = MockAccountTokenProvider() + subject = APIService(accountTokenProvider: accountTokenProvider, client: MockHTTPClient()) } override func tearDown() { super.tearDown() + accountTokenProvider = nil subject = nil } @@ -68,4 +71,11 @@ class APIServiceTests: BitwardenTestCase { ) XCTAssertNil(subject.identityService.tokenProvider) } + + /// `setupAccountTokenProviderDelegate(:)` sets up the delegate in the account token provider. + func test_setupAccountTokenProviderDelegate() async { + await subject.setAccountTokenProviderDelegate(delegate: MockAccountTokenProviderDelegate()) + let delegate = await accountTokenProvider.delegate + XCTAssertNotNil(delegate) + } } diff --git a/BitwardenShared/Core/Platform/Services/API/AccountTokenProvider.swift b/BitwardenShared/Core/Platform/Services/API/AccountTokenProvider.swift index 476bb78ab..1162d09cf 100644 --- a/BitwardenShared/Core/Platform/Services/API/AccountTokenProvider.swift +++ b/BitwardenShared/Core/Platform/Services/API/AccountTokenProvider.swift @@ -1,11 +1,25 @@ import Networking +// MARK: - AccountTokenProvider + +/// A more specific `TokenProvider` protocol to use and ease testing. +protocol AccountTokenProvider: TokenProvider { + /// Sets the delegate to use in this token provider. + /// - Parameter delegate: The delegate to use. + func setDelegate(delegate: AccountTokenProviderDelegate) async +} + +// MARK: - DefaultAccountTokenProvider + /// A `TokenProvider` that gets the access token for the current account and can refresh it when /// necessary. /// -actor AccountTokenProvider: TokenProvider { +actor DefaultAccountTokenProvider: AccountTokenProvider { // MARK: Properties + /// The delegate to use for specific operations on the token provider. + private weak var accountTokenProviderDelegate: AccountTokenProviderDelegate? + /// The `HTTPService` used to make the API call to refresh the access token. let httpService: HTTPService @@ -54,19 +68,37 @@ actor AccountTokenProvider: TokenProvider { let refreshTask = Task { defer { self.refreshTask = nil } - let refreshToken = try await tokenService.getRefreshToken() - let response = try await httpService.send( - IdentityTokenRefreshRequest(refreshToken: refreshToken) - ) - try await tokenService.setTokens( - accessToken: response.accessToken, - refreshToken: response.refreshToken - ) + do { + let refreshToken = try await tokenService.getRefreshToken() + let response = try await httpService.send( + IdentityTokenRefreshRequest(refreshToken: refreshToken) + ) + try await tokenService.setTokens( + accessToken: response.accessToken, + refreshToken: response.refreshToken + ) - return response.accessToken + return response.accessToken + } catch { + if let accountTokenProviderDelegate { + try await accountTokenProviderDelegate.onRefreshTokenError(error: error) + } + throw error + } } self.refreshTask = refreshTask _ = try await refreshTask.value } + + func setDelegate(delegate: AccountTokenProviderDelegate) async { + accountTokenProviderDelegate = delegate + } +} + +/// Delegate to be used by the `AccountTokenProvider`. +protocol AccountTokenProviderDelegate: AnyObject { + /// Callbac to be used when an error is thrown when refreshing the access token. + /// - Parameter error: `Error` thrown. + func onRefreshTokenError(error: Error) async throws } diff --git a/BitwardenShared/Core/Platform/Services/API/AccountTokenProviderTests.swift b/BitwardenShared/Core/Platform/Services/API/AccountTokenProviderTests.swift index e27f6b347..f7aed03ee 100644 --- a/BitwardenShared/Core/Platform/Services/API/AccountTokenProviderTests.swift +++ b/BitwardenShared/Core/Platform/Services/API/AccountTokenProviderTests.swift @@ -8,7 +8,7 @@ class AccountTokenProviderTests: BitwardenTestCase { // MARK: Properties var client: MockHTTPClient! - var subject: AccountTokenProvider! + var subject: DefaultAccountTokenProvider! var tokenService: MockTokenService! // MARK: Setup & Teardown @@ -19,7 +19,7 @@ class AccountTokenProviderTests: BitwardenTestCase { client = MockHTTPClient() tokenService = MockTokenService() - subject = AccountTokenProvider( + subject = DefaultAccountTokenProvider( httpService: HTTPService(baseURL: URL(string: "https://example.com")!, client: client), tokenService: tokenService ) @@ -88,4 +88,34 @@ class AccountTokenProviderTests: BitwardenTestCase { let refreshTask = await subject.refreshTask XCTAssertNil(refreshTask) } + + /// `refreshToken()` throws trying to refresh the access token + /// and gets handled by the delegate before throwing it again. + func test_refreshToken_handlesErrorInDelegateAndThrows() async throws { + let delegate = MockAccountTokenProviderDelegate() + await subject.setDelegate(delegate: delegate) + + tokenService.accessToken = "🔑" + tokenService.refreshToken = "🔒" + + client.result = .failure(BitwardenTestError.example) + + await assertAsyncThrows(error: BitwardenTestError.example) { + try await subject.refreshToken() + } + XCTAssertTrue(delegate.onRefreshTokenErrorCalled) + } + + /// `refreshToken()` throws trying to refresh the access token + /// and gets handled by the delegate before throwing it again. + func test_refreshToken_throwsWithNoDelegate() async throws { + tokenService.accessToken = "🔑" + tokenService.refreshToken = "🔒" + + client.result = .failure(BitwardenTestError.example) + + await assertAsyncThrows(error: BitwardenTestError.example) { + try await subject.refreshToken() + } + } } diff --git a/BitwardenShared/Core/Platform/Services/API/TestHelpers/APIService+Mocks.swift b/BitwardenShared/Core/Platform/Services/API/TestHelpers/APIService+Mocks.swift index ff570213f..3f6a69c3d 100644 --- a/BitwardenShared/Core/Platform/Services/API/TestHelpers/APIService+Mocks.swift +++ b/BitwardenShared/Core/Platform/Services/API/TestHelpers/APIService+Mocks.swift @@ -6,12 +6,14 @@ import Networking extension APIService { convenience init( + accountTokenProvider: AccountTokenProvider? = nil, client: HTTPClient, environmentService: EnvironmentService = MockEnvironmentService(), flightRecorder: FlightRecorder = MockFlightRecorder(), stateService: StateService = MockStateService() ) { self.init( + accountTokenProvider: accountTokenProvider, client: client, environmentService: environmentService, flightRecorder: flightRecorder, diff --git a/BitwardenShared/Core/Platform/Services/API/TestHelpers/MockAccountTokenProvider.swift b/BitwardenShared/Core/Platform/Services/API/TestHelpers/MockAccountTokenProvider.swift new file mode 100644 index 000000000..4462e7a4b --- /dev/null +++ b/BitwardenShared/Core/Platform/Services/API/TestHelpers/MockAccountTokenProvider.swift @@ -0,0 +1,36 @@ +import BitwardenKit + +@testable import BitwardenShared + +// MARK: - MockAccountTokenProvider + +@MainActor +class MockAccountTokenProvider: AccountTokenProvider { + var delegate: AccountTokenProviderDelegate? + var getTokenResult: Result = .success("ACCESS_TOKEN") + var refreshTokenResult: Result = .success(()) + + func getToken() async throws -> String { + try getTokenResult.get() + } + + func refreshToken() async throws { + try refreshTokenResult.get() + } + + func setDelegate(delegate: AccountTokenProviderDelegate) async { + self.delegate = delegate + } +} + +// MARK: - MockAccountTokenProviderDelegate + +class MockAccountTokenProviderDelegate: AccountTokenProviderDelegate { + var onRefreshTokenErrorCalled = false + var onRefreshTokenErrorResult: Result = .success(()) + + func onRefreshTokenError(error: any Error) async throws { + onRefreshTokenErrorCalled = true + try onRefreshTokenErrorResult.get() + } +} diff --git a/BitwardenShared/UI/Platform/Application/AppProcessor.swift b/BitwardenShared/UI/Platform/Application/AppProcessor.swift index 06ec77ce3..fcef92de2 100644 --- a/BitwardenShared/UI/Platform/Application/AppProcessor.swift +++ b/BitwardenShared/UI/Platform/Application/AppProcessor.swift @@ -81,6 +81,10 @@ public class AppProcessor { self.services.pendingAppIntentActionMediator.setDelegate(self) self.services.syncService.delegate = self + Task { + await services.apiService.setAccountTokenProviderDelegate(delegate: self) + } + startEventTimer() UI.initialLanguageCode = services.appSettingsStore.appLocale ?? Bundle.main.preferredLocalizations.first @@ -495,6 +499,18 @@ extension AppProcessor { return AppRoute.tab(.vault(.vaultItemSelection(totpKeyModel))) } + /// Logs out the user automatically, if `nil` is passed as `userId` then it will act on the current user. + /// - Parameter userId: The ID of the user to logout, current if `nil`. + private func logOutAutomatically(userId: String? = nil) async { + coordinator?.hideLoadingOverlay() + do { + try await services.authRepository.logout(userId: userId, userInitiated: false) + } catch { + services.errorReporter.log(error: error) + } + await coordinator?.handleEvent(.didLogout(userId: userId, userInitiated: false)) + } + /// Starts timer to send organization events regularly private func startEventTimer() { sendEventTimer = Timer.scheduledTimer(withTimeInterval: 5 * 60, repeats: true) { _ in @@ -621,9 +637,7 @@ extension AppProcessor: SyncServiceDelegate { func securityStampChanged(userId: String) async { // Log the user out if their security stamp changes. - coordinator?.hideLoadingOverlay() - try? await services.authRepository.logout(userId: userId, userInitiated: false) - await coordinator?.handleEvent(.didLogout(userId: userId, userInitiated: false)) + await logOutAutomatically(userId: userId) } func setMasterPassword(orgIdentifier: String) async { @@ -652,6 +666,16 @@ public extension AppProcessor { } } +// MARK: - AccountTokenProviderDelegate + +extension AppProcessor: AccountTokenProviderDelegate { + func onRefreshTokenError(error: any Error) async throws { + if case IdentityTokenRefreshRequestError.invalidGrant = error { + await logOutAutomatically() + } + } +} + // MARK: - AutofillCredentialServiceDelegate extension AppProcessor: AutofillCredentialServiceDelegate { diff --git a/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift b/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift index c413f40d4..21513a95f 100644 --- a/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift +++ b/BitwardenShared/UI/Platform/Application/AppProcessorTests.swift @@ -1372,4 +1372,59 @@ class AppProcessorTests: BitwardenTestCase { // swiftlint:disable:this type_body XCTAssertFalse(coordinator.isLoadingOverlayShowing) XCTAssertEqual(coordinator.events, [.didLogout(userId: "1", userInitiated: false)]) } + + /// `securityStampChanged(userId:)` throws logging the user out which is logged and notifies the coordinator. + @MainActor + func test_securityStampChanged_throwsLogging() async { + coordinator.isLoadingOverlayShowing = true + authRepository.logoutResult = .failure(BitwardenTestError.example) + + await subject.securityStampChanged(userId: "1") + + XCTAssertTrue(authRepository.logoutCalled) + XCTAssertEqual(errorReporter.errors as? [BitwardenTestError], [.example]) + XCTAssertFalse(coordinator.isLoadingOverlayShowing) + XCTAssertEqual(coordinator.events, [.didLogout(userId: "1", userInitiated: false)]) + } + + /// `onRefreshTokenError(error:)` logs the user out and notifies the coordinator when error is `.invalidGrant`. + @MainActor + func test_onRefreshTokenError_logOutInvalidGrant() async throws { + coordinator.isLoadingOverlayShowing = true + + try await subject.onRefreshTokenError(error: IdentityTokenRefreshRequestError.invalidGrant) + + XCTAssertTrue(authRepository.logoutCalled) + XCTAssertEqual(authRepository.logoutUserId, nil) + XCTAssertFalse(authRepository.logoutUserInitiated) + XCTAssertFalse(coordinator.isLoadingOverlayShowing) + XCTAssertEqual(coordinator.events, [.didLogout(userId: nil, userInitiated: false)]) + } + + /// `onRefreshTokenError(error:)` throws logging the user out which is logged and notifies the coordinator + /// when error is `.invalidGrant`. + @MainActor + func test_onRefreshTokenError_logOutInvalidGrantThrowsLogging() async throws { + coordinator.isLoadingOverlayShowing = true + authRepository.logoutResult = .failure(BitwardenTestError.example) + + try await subject.onRefreshTokenError(error: IdentityTokenRefreshRequestError.invalidGrant) + + XCTAssertTrue(authRepository.logoutCalled) + XCTAssertEqual(errorReporter.errors as? [BitwardenTestError], [.example]) + XCTAssertFalse(coordinator.isLoadingOverlayShowing) + XCTAssertEqual(coordinator.events, [.didLogout(userId: nil, userInitiated: false)]) + } + + /// `onRefreshTokenError(error:)` doesn't perform log out when error is not `.invalidGrant`. + @MainActor + func test_onRefreshTokenError_notInvalidGrant() async throws { + coordinator.isLoadingOverlayShowing = true + + try await subject.onRefreshTokenError(error: BitwardenTestError.example) + + XCTAssertFalse(authRepository.logoutCalled) + XCTAssertTrue(coordinator.isLoadingOverlayShowing) + XCTAssertTrue(coordinator.events.isEmpty) + } }