diff --git a/BitwardenShared/Core/Platform/Services/ClientCertificateService.swift b/BitwardenShared/Core/Platform/Services/ClientCertificateService.swift index 498435ac7..700d98b59 100644 --- a/BitwardenShared/Core/Platform/Services/ClientCertificateService.swift +++ b/BitwardenShared/Core/Platform/Services/ClientCertificateService.swift @@ -9,6 +9,15 @@ import Security /// A service for managing client certificates used for mTLS authentication. /// protocol ClientCertificateService: AnyObject { // sourcery: AutoMockable + /// Gets the client certificate identity for mTLS authentication from the current environment. + /// + /// The environment service determines which URLs are active (pre-auth or logged-in account), + /// so this returns the correct certificate for the current context. + /// + /// - Returns: A `SecIdentity` for the certificate, or `nil` if none is configured. + /// + func getClientCertificateIdentity() async -> SecIdentity? + /// Import a client certificate from PKCS#12 data. /// /// Parses the certificate, stores the identity in the Keychain, and returns the SHA-256 @@ -45,15 +54,6 @@ protocol ClientCertificateService: AnyObject { // sourcery: AutoMockable /// /// sourcery: useSelectorName func removeCertificate(userId: String) async throws - - /// Gets the client certificate identity for mTLS authentication from the current environment. - /// - /// The environment service determines which URLs are active (pre-auth or logged-in account), - /// so this returns the correct certificate for the current context. - /// - /// - Returns: A `SecIdentity` for the certificate, or `nil` if none is configured. - /// - func getClientCertificateIdentity() async -> SecIdentity? } // MARK: - DefaultClientCertificateService @@ -93,6 +93,14 @@ final class DefaultClientCertificateService: ClientCertificateService { // MARK: Methods + func getClientCertificateIdentity() async -> SecIdentity? { + guard let fingerprint = environmentService.clientCertificateFingerprint, + !fingerprint.isEmpty else { + return nil + } + return try? await keychainRepository.getClientCertificateIdentity(fingerprint: fingerprint) + } + func importCertificate( data: Data, password: String, @@ -152,14 +160,6 @@ final class DefaultClientCertificateService: ClientCertificateService { } } - func getClientCertificateIdentity() async -> SecIdentity? { - guard let fingerprint = environmentService.clientCertificateFingerprint, - !fingerprint.isEmpty else { - return nil - } - return try? await keychainRepository.getClientCertificateIdentity(fingerprint: fingerprint) - } - // MARK: Private /// Computes the SHA-256 fingerprint of the certificate within a SecIdentity. diff --git a/BitwardenShared/Core/Platform/Services/ClientCertificateServiceTests.swift b/BitwardenShared/Core/Platform/Services/ClientCertificateServiceTests.swift index 2ca623b7c..404f415f2 100644 --- a/BitwardenShared/Core/Platform/Services/ClientCertificateServiceTests.swift +++ b/BitwardenShared/Core/Platform/Services/ClientCertificateServiceTests.swift @@ -38,98 +38,25 @@ final class ClientCertificateServiceTests: BitwardenTestCase { subject = nil } - // MARK: Tests - removeCertificate(userId:) + // MARK: Tests - getClientCertificateIdentity() - /// `removeCertificate(userId:)` keeps the keychain identity when another account references - /// the same certificate fingerprint in its environment URLs. - func test_removeCertificate_sharedFingerprintAcrossAccounts_doesNotDeleteKeychainIdentity() async throws { - let user1 = "1" - let user2 = "2" - let fingerprint = "shared-fingerprint" + /// `getClientCertificateIdentity()` returns nil when the fingerprint is set but the keychain + /// identity is missing. + func test_getClientCertificateIdentity_fingerprintSetButKeychainMissing_returnsNil() async { + environmentService.clientCertificateFingerprint = "missing-from-keychain" - stateService.accounts = [ - .fixture(profile: .fixture(userId: user1)), - .fixture(profile: .fixture(userId: user2)), - ] - stateService.activeAccount = .fixture(profile: .fixture(userId: user1)) - stateService.environmentURLs[user1] = EnvironmentURLData( - base: URL(string: "https://example.com"), - clientCertificateAlias: "Cert A", - clientCertificateFingerprint: fingerprint, - ) - stateService.environmentURLs[user2] = EnvironmentURLData( - base: URL(string: "https://example.com"), - clientCertificateAlias: "Cert B", - clientCertificateFingerprint: fingerprint, - ) + let result = await subject.getClientCertificateIdentity() - try await subject.removeCertificate(userId: user1) - - XCTAssertEqual(keychainRepository.deleteClientCertIdentityFingerprints, []) + XCTAssertNil(result) } - /// `removeCertificate(userId:)` deletes the keychain identity when the removed user is the - /// last reference to the certificate fingerprint. - func test_removeCertificate_lastFingerprintReference_deletesKeychainIdentity() async throws { - let user1 = "1" - let fingerprint = "only-fingerprint" + /// `getClientCertificateIdentity()` returns nil when no fingerprint is in the environment. + func test_getClientCertificateIdentity_noFingerprint_returnsNil() async { + environmentService.clientCertificateFingerprint = nil - stateService.accounts = [ - .fixture(profile: .fixture(userId: user1)), - ] - stateService.activeAccount = .fixture(profile: .fixture(userId: user1)) - stateService.environmentURLs[user1] = EnvironmentURLData( - base: URL(string: "https://example.com"), - clientCertificateAlias: "Cert A", - clientCertificateFingerprint: fingerprint, - ) + let result = await subject.getClientCertificateIdentity() - try await subject.removeCertificate(userId: user1) - - XCTAssertEqual(keychainRepository.deleteClientCertIdentityFingerprints, [fingerprint]) - } - - /// `removeCertificate(userId:)` keeps the keychain identity when the pre-auth environment URLs - /// still reference the same certificate fingerprint. - func test_removeCertificate_sharedWithPreAuth_doesNotDeleteKeychainIdentity() async throws { - let user1 = "1" - let fingerprint = "shared-with-preauth" - - stateService.accounts = [ - .fixture(profile: .fixture(userId: user1)), - ] - stateService.activeAccount = .fixture(profile: .fixture(userId: user1)) - stateService.environmentURLs[user1] = EnvironmentURLData( - base: URL(string: "https://example.com"), - clientCertificateAlias: "Cert A", - clientCertificateFingerprint: fingerprint, - ) - stateService.preAuthEnvironmentURLs = EnvironmentURLData( - base: URL(string: "https://example.com"), - clientCertificateAlias: "PreAuth Cert", - clientCertificateFingerprint: fingerprint, - ) - - try await subject.removeCertificate(userId: user1) - - XCTAssertEqual(keychainRepository.deleteClientCertIdentityFingerprints, []) - } - - /// `removeCertificate(userId:)` succeeds gracefully when no certificate is configured. - func test_removeCertificate_noCertConfigured_succeeds() async throws { - let user1 = "1" - - stateService.accounts = [ - .fixture(profile: .fixture(userId: user1)), - ] - stateService.activeAccount = .fixture(profile: .fixture(userId: user1)) - stateService.environmentURLs[user1] = EnvironmentURLData( - base: URL(string: "https://example.com"), - ) - - try await subject.removeCertificate(userId: user1) - - XCTAssertEqual(keychainRepository.deleteClientCertIdentityFingerprints, []) + XCTAssertNil(result) } // MARK: Tests - removeCertificate(fingerprint:) @@ -164,24 +91,97 @@ final class ClientCertificateServiceTests: BitwardenTestCase { XCTAssertEqual(keychainRepository.deleteClientCertIdentityFingerprints, []) } - // MARK: Tests - getClientCertificateIdentity() + // MARK: Tests - removeCertificate(userId:) - /// `getClientCertificateIdentity()` returns nil when no fingerprint is in the environment. - func test_getClientCertificateIdentity_noFingerprint_returnsNil() async { - environmentService.clientCertificateFingerprint = nil + /// `removeCertificate(userId:)` deletes the keychain identity when the removed user is the + /// last reference to the certificate fingerprint. + func test_removeCertificate_lastFingerprintReference_deletesKeychainIdentity() async throws { + let user1 = "1" + let fingerprint = "only-fingerprint" - let result = await subject.getClientCertificateIdentity() + stateService.accounts = [ + .fixture(profile: .fixture(userId: user1)), + ] + stateService.activeAccount = .fixture(profile: .fixture(userId: user1)) + stateService.environmentURLs[user1] = EnvironmentURLData( + base: URL(string: "https://example.com"), + clientCertificateAlias: "Cert A", + clientCertificateFingerprint: fingerprint, + ) - XCTAssertNil(result) + try await subject.removeCertificate(userId: user1) + + XCTAssertEqual(keychainRepository.deleteClientCertIdentityFingerprints, [fingerprint]) } - /// `getClientCertificateIdentity()` returns nil when the fingerprint is set but the keychain - /// identity is missing. - func test_getClientCertificateIdentity_fingerprintSetButKeychainMissing_returnsNil() async { - environmentService.clientCertificateFingerprint = "missing-from-keychain" + /// `removeCertificate(userId:)` succeeds gracefully when no certificate is configured. + func test_removeCertificate_noCertConfigured_succeeds() async throws { + let user1 = "1" - let result = await subject.getClientCertificateIdentity() + stateService.accounts = [ + .fixture(profile: .fixture(userId: user1)), + ] + stateService.activeAccount = .fixture(profile: .fixture(userId: user1)) + stateService.environmentURLs[user1] = EnvironmentURLData( + base: URL(string: "https://example.com"), + ) - XCTAssertNil(result) + try await subject.removeCertificate(userId: user1) + + XCTAssertEqual(keychainRepository.deleteClientCertIdentityFingerprints, []) + } + + /// `removeCertificate(userId:)` keeps the keychain identity when another account references + /// the same certificate fingerprint in its environment URLs. + func test_removeCertificate_sharedFingerprintAcrossAccounts_doesNotDeleteKeychainIdentity() async throws { + let user1 = "1" + let user2 = "2" + let fingerprint = "shared-fingerprint" + + stateService.accounts = [ + .fixture(profile: .fixture(userId: user1)), + .fixture(profile: .fixture(userId: user2)), + ] + stateService.activeAccount = .fixture(profile: .fixture(userId: user1)) + stateService.environmentURLs[user1] = EnvironmentURLData( + base: URL(string: "https://example.com"), + clientCertificateAlias: "Cert A", + clientCertificateFingerprint: fingerprint, + ) + stateService.environmentURLs[user2] = EnvironmentURLData( + base: URL(string: "https://example.com"), + clientCertificateAlias: "Cert B", + clientCertificateFingerprint: fingerprint, + ) + + try await subject.removeCertificate(userId: user1) + + XCTAssertEqual(keychainRepository.deleteClientCertIdentityFingerprints, []) + } + + /// `removeCertificate(userId:)` keeps the keychain identity when the pre-auth environment URLs + /// still reference the same certificate fingerprint. + func test_removeCertificate_sharedWithPreAuth_doesNotDeleteKeychainIdentity() async throws { + let user1 = "1" + let fingerprint = "shared-with-preauth" + + stateService.accounts = [ + .fixture(profile: .fixture(userId: user1)), + ] + stateService.activeAccount = .fixture(profile: .fixture(userId: user1)) + stateService.environmentURLs[user1] = EnvironmentURLData( + base: URL(string: "https://example.com"), + clientCertificateAlias: "Cert A", + clientCertificateFingerprint: fingerprint, + ) + stateService.preAuthEnvironmentURLs = EnvironmentURLData( + base: URL(string: "https://example.com"), + clientCertificateAlias: "PreAuth Cert", + clientCertificateFingerprint: fingerprint, + ) + + try await subject.removeCertificate(userId: user1) + + XCTAssertEqual(keychainRepository.deleteClientCertIdentityFingerprints, []) } }