-
Notifications
You must be signed in to change notification settings - Fork 154
[PM-40519] feat: Add passkey cipher storage and identity foundation to TestHarness #2977
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import BitwardenSdk | ||
| import Foundation | ||
|
|
||
| // MARK: - CipherStorageService | ||
|
|
||
| // sourcery: AutoMockable | ||
| /// A service that locally persists the SDK-encrypted `Cipher`s created by the SDK-backed passkey | ||
| /// scenarios, so they survive app relaunches. | ||
| /// | ||
| protocol CipherStorageService: AnyObject { | ||
| /// Loads the persisted ciphers, if any. | ||
| /// | ||
| /// - Returns: The persisted ciphers, in the order they were saved. Empty if none are stored. | ||
| /// | ||
| func loadCiphers() -> [Cipher] | ||
|
|
||
| /// Persists the given ciphers, replacing whatever was previously stored. | ||
| /// | ||
| /// - Parameter ciphers: The full list of ciphers to persist. | ||
| /// | ||
| func save(ciphers: [Cipher]) | ||
| } | ||
|
|
||
| // MARK: - DefaultCipherStorageService | ||
|
|
||
| /// The default `CipherStorageService` implementation, backed by `UserDefaults`. This is safe | ||
| /// to store outside the keychain because every sensitive field is already SDK ciphertext β | ||
| /// useless without the synthetic identity's key, which is kept in the keychain (see | ||
| /// `SyntheticIdentity`). | ||
| /// | ||
| final class DefaultCipherStorageService: CipherStorageService { | ||
| // MARK: Private Properties | ||
|
|
||
| /// The `UserDefaults` key under which the persisted ciphers are stored. | ||
| private static let storageKey = "PasskeyStoredCiphers" | ||
|
Comment on lines
+34
to
+35
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π€ I wonder if we should use some key format alike what we use now in |
||
|
|
||
| /// The `UserDefaults` instance used for persistence. | ||
| private let userDefaults: UserDefaults | ||
|
|
||
| // MARK: Initialization | ||
|
|
||
| /// Initializes a `DefaultCipherStorageService`. | ||
| /// | ||
| /// - Parameter userDefaults: The `UserDefaults` instance used for persistence. | ||
| /// | ||
| init(userDefaults: UserDefaults = .standard) { | ||
| self.userDefaults = userDefaults | ||
| } | ||
|
|
||
| // MARK: Methods | ||
|
|
||
| func loadCiphers() -> [Cipher] { | ||
| guard let data = userDefaults.data(forKey: Self.storageKey), | ||
| let storedCiphers = try? JSONDecoder().decode([StoredCipher].self, from: data) else { | ||
| return [] | ||
| } | ||
| return storedCiphers.map(\.cipher) | ||
| } | ||
|
|
||
| func save(ciphers: [Cipher]) { | ||
| let storedCiphers = ciphers.compactMap(StoredCipher.init(cipher:)) | ||
| guard let data = try? JSONEncoder().encode(storedCiphers) else { return } | ||
| userDefaults.set(data, forKey: Self.storageKey) | ||
| } | ||
|
Comment on lines
+52
to
+64
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π€ I usually like to avoid |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import BitwardenSdk | ||
|
|
||
| // MARK: - ClientManagedTokensProvider | ||
|
|
||
| /// A `ClientManagedTokens` implementation for the SDK-backed passkey scenarios, which never | ||
| /// make network requests and so never have an access token to provide. | ||
| /// | ||
| final class ClientManagedTokensProvider: ClientManagedTokens { | ||
| func getAccessToken() async -> String? { | ||
| nil | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import BitwardenKitMocks | ||
| import BitwardenSdk | ||
| import TestHelpers | ||
| import XCTest | ||
|
|
||
| @testable import TestHarnessShared | ||
|
|
||
| // MARK: - DefaultCipherStorageServiceTests | ||
|
|
||
| /// Tests for `DefaultCipherStorageService`. | ||
| /// | ||
| class DefaultCipherStorageServiceTests: BitwardenTestCase { | ||
| // MARK: Properties | ||
|
|
||
| var subject: DefaultCipherStorageService! | ||
| var userDefaults: UserDefaults! | ||
|
|
||
| // MARK: Setup & Teardown | ||
|
|
||
| override func setUp() { | ||
| super.setUp() | ||
| userDefaults = UserDefaults(suiteName: "DefaultCipherStorageServiceTests") | ||
| userDefaults.removePersistentDomain(forName: "DefaultCipherStorageServiceTests") | ||
| subject = DefaultCipherStorageService(userDefaults: userDefaults) | ||
| } | ||
|
|
||
| override func tearDown() { | ||
| super.tearDown() | ||
| userDefaults.removePersistentDomain(forName: "DefaultCipherStorageServiceTests") | ||
| subject = nil | ||
| userDefaults = nil | ||
| } | ||
|
|
||
| // MARK: Tests | ||
|
|
||
| /// `loadCiphers()` returns an empty list when nothing has been saved. | ||
| func test_loadCiphers_empty() { | ||
| XCTAssertTrue(subject.loadCiphers().isEmpty) | ||
| } | ||
|
|
||
| /// `save(ciphers:)` followed by `loadCiphers()` returns the persisted ciphers. | ||
| func test_save_thenLoadCiphers_returnsPersistedCiphers() { | ||
| let cipher = Cipher(cipherView: .fixture( | ||
| login: .fixture(fido2Credentials: [Fido2Credential(fido2CredentialView: .fixture())]), | ||
| )) | ||
|
|
||
| subject.save(ciphers: [cipher]) | ||
|
|
||
| XCTAssertEqual(subject.loadCiphers(), [cipher]) | ||
| } | ||
|
|
||
| /// `save(ciphers:)` silently skips ciphers with no Fido2 credential, since this storage | ||
| /// service only knows how to persist the login+Fido2 shape these scenarios create. | ||
| func test_save_ciphersWithoutFido2Credential_areSkipped() { | ||
| let cipher = Cipher(cipherView: .fixture(login: .fixture(fido2Credentials: nil))) | ||
|
|
||
| subject.save(ciphers: [cipher]) | ||
|
|
||
| XCTAssertTrue(subject.loadCiphers().isEmpty) | ||
| } | ||
|
|
||
| /// Persisted ciphers remain available to a new `DefaultCipherStorageService` instance | ||
| /// backed by the same `UserDefaults`, simulating an app relaunch. | ||
| func test_save_persistsAcrossInstances() { | ||
| let cipher = Cipher(cipherView: .fixture( | ||
| login: .fixture(fido2Credentials: [Fido2Credential(fido2CredentialView: .fixture())]), | ||
| )) | ||
| subject.save(ciphers: [cipher]) | ||
|
|
||
| let relaunchedSubject = DefaultCipherStorageService(userDefaults: userDefaults) | ||
|
|
||
| XCTAssertEqual(relaunchedSubject.loadCiphers(), [cipher]) | ||
| } | ||
| } |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π¨ I think we should think about moving our current
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that idea |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| import BitwardenSdk | ||
| import Foundation | ||
|
|
||
| extension CipherView { | ||
| static func fixture( | ||
| id: CipherId? = nil, | ||
| login: LoginView? = .fixture(), | ||
| name: String = "Example", | ||
| ) -> CipherView { | ||
| CipherView( | ||
| id: id, | ||
| organizationId: nil, | ||
| folderId: nil, | ||
| collectionIds: [], | ||
| key: nil, | ||
| name: name, | ||
| notes: nil, | ||
| type: .login, | ||
| login: login, | ||
| identity: nil, | ||
| card: nil, | ||
| secureNote: nil, | ||
| sshKey: nil, | ||
| bankAccount: nil, | ||
| driversLicense: nil, | ||
| passport: nil, | ||
| favorite: false, | ||
| reprompt: .none, | ||
| organizationUseTotp: false, | ||
| edit: true, | ||
| permissions: nil, | ||
| viewPassword: true, | ||
| localData: nil, | ||
| attachments: nil, | ||
| attachmentDecryptionFailures: nil, | ||
| fields: nil, | ||
| passwordHistory: nil, | ||
| creationDate: Date(timeIntervalSince1970: 0), | ||
| deletedDate: nil, | ||
| revisionDate: Date(timeIntervalSince1970: 0), | ||
| archivedDate: nil, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| extension LoginView { | ||
| static func fixture( | ||
| fido2Credentials: [Fido2Credential]? = nil, | ||
| username: String? = "user@example.com", | ||
| ) -> LoginView { | ||
| LoginView( | ||
| username: username, | ||
| password: nil, | ||
| passwordRevisionDate: nil, | ||
| uris: nil, | ||
| totp: nil, | ||
| autofillOnPageLoad: nil, | ||
| fido2Credentials: fido2Credentials, | ||
| ) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import BitwardenSdk | ||
| import Foundation | ||
|
|
||
| extension Fido2CredentialAutofillView { | ||
| static func fixture( | ||
| credentialId: Data = Data([0x01]), | ||
| cipherId: String = "cipher-id", | ||
| rpId: String = "bitwarden.com", | ||
| userNameForUi: String? = "user@example.com", | ||
| userHandle: Data = Data([0x02]), | ||
| hasCounter: Bool = false, | ||
| ) -> Fido2CredentialAutofillView { | ||
| Fido2CredentialAutofillView( | ||
| credentialId: credentialId, | ||
| cipherId: cipherId, | ||
| rpId: rpId, | ||
| userNameForUi: userNameForUi, | ||
| userHandle: userHandle, | ||
| hasCounter: hasCounter, | ||
| ) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import BitwardenSdk | ||
| import Foundation | ||
|
|
||
| extension Fido2CredentialView { | ||
| static func fixture( | ||
| credentialId: String = "credential-id", | ||
| rpId: String = "bitwarden.com", | ||
| userName: String? = "user@example.com", | ||
| ) -> Fido2CredentialView { | ||
| Fido2CredentialView( | ||
| credentialId: credentialId, | ||
| keyType: "public-key", | ||
| keyAlgorithm: "ECDSA", | ||
| keyCurve: "P-256", | ||
| keyValue: "keyValue", | ||
| rpId: rpId, | ||
| userHandle: nil, | ||
| userName: userName, | ||
| counter: "0", | ||
| rpName: nil, | ||
| userDisplayName: nil, | ||
| discoverable: "true", | ||
| creationDate: Date(timeIntervalSince1970: 0), | ||
| ) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import Foundation | ||
|
|
||
| // MARK: - PasskeyError | ||
|
|
||
| /// Errors produced by the SDK-backed passkey scenarios. | ||
| /// | ||
| enum PasskeyError: Equatable, Error, LocalizedError { | ||
| /// More than one stored credential matched the requested relying party; picking between | ||
| /// them isn't supported by this scenario yet. | ||
| case ambiguousCredential | ||
|
|
||
| /// No stored credential matched the requested relying party. | ||
| case noMatchingCredential | ||
|
|
||
| var errorDescription: String? { | ||
| switch self { | ||
| case .ambiguousCredential: | ||
| Localizations.ambiguousCredentialReceived | ||
| case .noMatchingCredential: | ||
| Localizations.noMatchingCredentialReceived | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import BitwardenKit | ||
| import Foundation | ||
|
|
||
| // MARK: - PasskeyKeychainItem | ||
|
|
||
| /// The keychain items used by the SDK-backed passkey scenarios. | ||
| /// | ||
| enum PasskeyKeychainItem: Equatable, KeychainItem { | ||
| /// The keychain item for the synthetic identity used to bootstrap the SDK client, so the | ||
| /// same identity β and therefore the same crypto keys β can be reconstructed across app | ||
| /// launches. | ||
| case syntheticIdentity | ||
|
|
||
| /// The `SecAccessControlCreateFlags` protection level for this keychain item. No extra | ||
| /// protection is needed since this is throwaway, synthetic identity material with no real | ||
| /// security value. | ||
| /// | ||
| var accessControlFlags: SecAccessControlCreateFlags? { nil } | ||
|
|
||
| /// The protection level for this keychain item. | ||
| var protection: CFTypeRef { kSecAttrAccessibleWhenUnlockedThisDeviceOnly } | ||
|
|
||
| /// The storage key for this keychain item. | ||
| /// | ||
| var unformattedKey: String { | ||
| switch self { | ||
| case .syntheticIdentity: | ||
| "sdkPasskeySyntheticIdentity" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
βοΈ We usually put sourcery inline with the protocol definition. I'm almost certain that you should get a warning on how it's done there above the DocC.