From 09f05833499f83c024e057e0c44d09c3ffe99f54 Mon Sep 17 00:00:00 2001 From: Yaro Luchko Date: Thu, 25 Jul 2024 18:32:27 -0700 Subject: [PATCH 1/7] feat(Auth) Keychain Sharing (App Reload Required) * Remove migrateKeychainItemsOfUserSession bool from SecureStoragePreferences --- .../Categories/Auth/Models/AccessGroup.swift | 30 ++ .../AWSCognitoAuthPlugin+Configure.swift | 3 +- .../AWSCognitoAuthPlugin.swift | 16 +- .../AWSCognitoAuthCredentialStore.swift | 103 +++++- .../AWSCognitoSecureStoragePreferences.swift | 19 ++ .../MockCredentialStoreBehavior.swift | 7 + ...oAuthPluginAmplifyOutputsConfigTests.swift | 80 +++++ .../AWSCognitoAuthPluginConfigTests.swift | 96 ++++++ .../Support/DefaultConfig.swift | 4 + .../AuthHostApp.xcodeproj/project.pbxproj | 8 + .../AuthHostApp/AuthHostApp.entitlements | 11 + .../AWSAuthBaseTest.swift | 4 + .../CredentialStoreConfigurationTests.swift | 311 ++++++++++++++++++ .../Helpers/AuthEnvironmentHelper.swift | 8 + .../AuthHostApp/AuthWatchApp.entitlements | 11 + .../Keychain/KeychainStore.swift | 59 +++- .../Keychain/KeychainStoreAttributes.swift | 2 +- .../KeychainStoreAttributesTests.swift | 32 +- .../Mocks/MockKeychainStore.swift | 14 + 19 files changed, 794 insertions(+), 24 deletions(-) create mode 100644 Amplify/Categories/Auth/Models/AccessGroup.swift create mode 100644 AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/AWSCognitoSecureStoragePreferences.swift create mode 100644 AmplifyPlugins/Auth/Tests/AuthHostApp/AuthHostApp/AuthHostApp.entitlements create mode 100644 AmplifyPlugins/Auth/Tests/AuthHostApp/AuthWatchApp.entitlements diff --git a/Amplify/Categories/Auth/Models/AccessGroup.swift b/Amplify/Categories/Auth/Models/AccessGroup.swift new file mode 100644 index 0000000000..7fc1ebb3c6 --- /dev/null +++ b/Amplify/Categories/Auth/Models/AccessGroup.swift @@ -0,0 +1,30 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import Foundation + +public struct AccessGroup { + public let name: String? + public let migrateKeychainItems: Bool + + public init(name: String, migrateKeychainItemsOfUserSession: Bool = false) { + self.init(name: name, migrateKeychainItems: migrateKeychainItemsOfUserSession) + } + + public static func none(migrateKeychainItemsOfUserSession: Bool) -> AccessGroup { + return .init(name: nil, migrateKeychainItems: migrateKeychainItemsOfUserSession) + } + + public static var none: AccessGroup { + return .none(migrateKeychainItemsOfUserSession: false) + } + + private init(name: String?, migrateKeychainItems: Bool) { + self.name = name + self.migrateKeychainItems = migrateKeychainItems + } +} diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin+Configure.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin+Configure.swift index fdcfbf9385..ee8bcaf58f 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin+Configure.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin+Configure.swift @@ -177,7 +177,8 @@ extension AWSCognitoAuthPlugin { } private func makeCredentialStore() -> AmplifyAuthCredentialStoreBehavior { - AWSCognitoAuthCredentialStore(authConfiguration: authConfiguration) + return AWSCognitoAuthCredentialStore(authConfiguration: authConfiguration, accessGroup: secureStoragePreferences?.accessGroup?.name, + migrateKeychainItemsOfUserSession: secureStoragePreferences?.accessGroup?.migrateKeychainItems ?? false) } private func makeLegacyKeychainStore(service: String) -> KeychainStoreBehavior { diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin.swift index f32209ba9c..857651ffc3 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin.swift @@ -35,6 +35,9 @@ public final class AWSCognitoAuthPlugin: AWSCognitoAuthPluginBehavior { /// The user network preferences for timeout and retry let networkPreferences: AWSCognitoNetworkPreferences? + /// The user secure storage preferences for access group + let secureStoragePreferences: AWSCognitoSecureStoragePreferences? + @_spi(InternalAmplifyConfiguration) internal(set) public var jsonConfiguration: JSONValue? @@ -43,15 +46,14 @@ public final class AWSCognitoAuthPlugin: AWSCognitoAuthPluginBehavior { return "awsCognitoAuthPlugin" } - /// Instantiates an instance of the AWSCognitoAuthPlugin. - public init() { - self.networkPreferences = nil - } - - /// Instantiates an instance of the AWSCognitoAuthPlugin with custom network preferences + /// Instantiates an instance of the AWSCognitoAuthPlugin with optionally custom network + /// preferences and custom secure storage preferences /// - Parameters: /// - networkPreferences: network preferences - public init(networkPreferences: AWSCognitoNetworkPreferences) { + /// - secureStoragePreferences: secure storage preferences + public init(networkPreferences: AWSCognitoNetworkPreferences? = nil, + secureStoragePreferences: AWSCognitoSecureStoragePreferences = AWSCognitoSecureStoragePreferences()) { self.networkPreferences = networkPreferences + self.secureStoragePreferences = secureStoragePreferences } } diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift index 3bb2a2e1bb..a5666de8d7 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift @@ -13,6 +13,7 @@ struct AWSCognitoAuthCredentialStore { // Credential store constants private let service = "com.amplify.awsCognitoAuthPlugin" + private let sharedService = "com.amplify.awsCognitoAuthPluginShared" private let sessionKey = "session" private let deviceMetadataKey = "deviceMetadata" private let deviceASFKey = "deviceASF" @@ -25,14 +26,29 @@ struct AWSCognitoAuthCredentialStore { private var isKeychainConfiguredKey: String { "\(userDefaultsNameSpace).isKeychainConfigured" } + private var accessGroupKey: String { + "\(userDefaultsNameSpace).accessGroup" + } private let authConfiguration: AuthConfiguration private let keychain: KeychainStoreBehavior private let userDefaults = UserDefaults.standard + private let accessGroup: String? - init(authConfiguration: AuthConfiguration, accessGroup: String? = nil) { + init(authConfiguration: AuthConfiguration, accessGroup: String? = nil, migrateKeychainItemsOfUserSession: Bool = false) { self.authConfiguration = authConfiguration - self.keychain = KeychainStore(service: service, accessGroup: accessGroup) + self.accessGroup = accessGroup + if let accessGroup { + self.keychain = KeychainStore(service: sharedService, accessGroup: accessGroup) + } else { + self.keychain = KeychainStore(service: service) + } + + if migrateKeychainItemsOfUserSession { + try? migrateKeychainItemsToAccessGroup() + } + + try? saveStoredAccessGroup() if !userDefaults.bool(forKey: isKeychainConfiguredKey) { try? clearAllCredentials() @@ -182,6 +198,81 @@ extension AWSCognitoAuthCredentialStore: AmplifyAuthCredentialStoreBehavior { private func clearAllCredentials() throws { try keychain._removeAll() } + + private func retrieveStoredAccessGroup() throws -> String? { + return userDefaults.string(forKey: accessGroupKey) + } + + private func saveStoredAccessGroup() throws { + if let accessGroup { + userDefaults.set(accessGroup, forKey: accessGroupKey) + } else { + userDefaults.removeObject(forKey: accessGroupKey) + } + } + + private func migrateKeychainItemsToAccessGroup() throws { + let oldAccessGroup = try? retrieveStoredAccessGroup() + let oldKeychain: KeychainStoreBehavior + + if oldAccessGroup == accessGroup { + log.verbose("[AWSCognitoAuthCredentialStore] Stored access group is the same as current access group, aborting migration") + return + } + + if let oldAccessGroup { + oldKeychain = KeychainStore(service: sharedService, accessGroup: oldAccessGroup) + } else { + oldKeychain = KeychainStore(service: service) + } + + let authCredentialStoreKey = generateSessionKey(for: authConfiguration) + let authCredentialData: Data + let awsCredential: AmplifyCredentials + do { + authCredentialData = try oldKeychain._getData(authCredentialStoreKey) + awsCredential = try decode(data: authCredentialData) + } catch { + log.verbose("[AWSCognitoAuthCredentialStore] Could not retrieve previous credentials in keychain under old access group, nothing to migrate") + return + } + + guard awsCredential.areValid() else { + log.verbose("[AWSCognitoAuthCredentialStore] Credentials found are not valid (expired) in old access group keychain, aborting migration") + return + } + + let oldItems: [(key: String, value: Data)] + do { + oldItems = try oldKeychain._getAll() + } catch { + log.error("[AWSCognitoAuthCredentialStore] Error getting all items from keychain under old access group, aborting migration") + return + } + + if oldItems.isEmpty { + log.verbose("[AWSCognitoAuthCredentialStore] No items in keychain under old access group, clearing keychain items under new access group") + return + } + + for item in oldItems { + do { + try keychain._set(item.value, key: item.key) + } catch { + log.error("[AWSCognitoAuthCredentialStore] Error migrating one of the items, aborting migration: \(error)") + try? clearAllCredentials() + return + } + } + + do { + try oldKeychain._removeAll() + } catch { + log.error("[AWSCognitoAuthCredentialStore] Error deleting all items from keychain under old access group after migration") + } + + log.verbose("[AWSCognitoAuthCredentialStore] Migration of keychain items from old access group to new access group successful") + } } @@ -205,3 +296,11 @@ private extension AWSCognitoAuthCredentialStore { } } + +extension AWSCognitoAuthCredentialStore: DefaultLogger { + public static var log: Logger { + Amplify.Logging.logger(forNamespace: String(describing: self)) + } + + public nonisolated var log: Logger { Self.log } +} diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/AWSCognitoSecureStoragePreferences.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/AWSCognitoSecureStoragePreferences.swift new file mode 100644 index 0000000000..c7dcefe42b --- /dev/null +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/AWSCognitoSecureStoragePreferences.swift @@ -0,0 +1,19 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import Foundation +import Amplify + +public struct AWSCognitoSecureStoragePreferences { + + /// The access group that the keychain will use for auth items + public let accessGroup: AccessGroup? + + public init(accessGroup: AccessGroup? = nil) { + self.accessGroup = accessGroup + } +} diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/CredentialStore/MockCredentialStoreBehavior.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/CredentialStore/MockCredentialStoreBehavior.swift index 97b9201818..1f00f81df3 100644 --- a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/CredentialStore/MockCredentialStoreBehavior.swift +++ b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/CredentialStore/MockCredentialStoreBehavior.swift @@ -15,12 +15,15 @@ class MockKeychainStoreBehavior: KeychainStoreBehavior { typealias VoidHandler = () -> Void let data: String + let allData: [(key: String, value: Data)] let removeAllHandler: VoidHandler? + let mockKey: String = "mockKey" init(data: String, removeAllHandler: VoidHandler? = nil) { self.data = data self.removeAllHandler = removeAllHandler + self.allData = [(key: mockKey, value: Data(data.utf8))] } func _getString(_ key: String) throws -> String { @@ -41,4 +44,8 @@ class MockKeychainStoreBehavior: KeychainStoreBehavior { func _removeAll() throws { removeAllHandler?() } + + func _getAll() throws -> [(key: String, value: Data)] { + return allData + } } diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginAmplifyOutputsConfigTests.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginAmplifyOutputsConfigTests.swift index 85eba053d7..23ecfae901 100644 --- a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginAmplifyOutputsConfigTests.swift +++ b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginAmplifyOutputsConfigTests.swift @@ -123,4 +123,84 @@ class AWSCognitoAuthPluginAmplifyOutputsConfigTests: XCTestCase { XCTFail("Should not throw error. \(error)") } } + + /// Test Auth configuration with valid config for user pool and identity pool, with secure storage preferences + /// + /// - Given: Given valid config for user pool and identity pool with secure storage preferences + /// - When: + /// - I configure auth with the given configuration and secure storage preferences + /// - Then: + /// - I should not get any error while configuring auth + /// + func testConfigWithUserPoolAndIdentityPoolWithSecureStoragePreferences() throws { + let plugin = AWSCognitoAuthPlugin( + secureStoragePreferences: .init( + accessGroup: AccessGroup(name: "xx") + ) + ) + try Amplify.add(plugin: plugin) + + let amplifyConfig = AmplifyOutputsData(auth: .init( + awsRegion: "us-east-1", + userPoolId: "xx", + userPoolClientId: "xx", + identityPoolId: "xx")) + + do { + try Amplify.configure(amplifyConfig) + + let escapeHatch = plugin.getEscapeHatch() + guard case .userPoolAndIdentityPool(let userPoolClient, let identityPoolClient) = escapeHatch else { + XCTFail("Expected .userPool, got \(escapeHatch)") + return + } + XCTAssertNotNil(userPoolClient) + XCTAssertNotNil(identityPoolClient) + + } catch { + XCTFail("Should not throw error. \(error)") + } + } + + /// Test Auth configuration with valid config for user pool and identity pool, with network preferences and secure storage preferences + /// + /// - Given: Given valid config for user pool and identity pool, network preferences, and secure storage preferences + /// - When: + /// - I configure auth with the given configuration, network preferences, and secure storage preferences + /// - Then: + /// - I should not get any error while configuring auth + /// + func testConfigWithUserPoolAndIdentityPoolWithNetworkPreferencesAndSecureStoragePreferences() throws { + let plugin = AWSCognitoAuthPlugin( + networkPreferences: .init( + maxRetryCount: 2, + timeoutIntervalForRequest: 60, + timeoutIntervalForResource: 60), + secureStoragePreferences: .init( + accessGroup: AccessGroup(name: "xx") + ) + ) + try Amplify.add(plugin: plugin) + + let amplifyConfig = AmplifyOutputsData(auth: .init( + awsRegion: "us-east-1", + userPoolId: "xx", + userPoolClientId: "xx", + identityPoolId: "xx")) + + do { + try Amplify.configure(amplifyConfig) + + let escapeHatch = plugin.getEscapeHatch() + guard case .userPoolAndIdentityPool(let userPoolClient, let identityPoolClient) = escapeHatch else { + XCTFail("Expected .userPool, got \(escapeHatch)") + return + } + XCTAssertNotNil(userPoolClient) + XCTAssertNotNil(identityPoolClient) + + } catch { + XCTFail("Should not throw error. \(error)") + } + } } diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginConfigTests.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginConfigTests.swift index 8ba574028e..04cf1c78f9 100644 --- a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginConfigTests.swift +++ b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ConfigurationTests/AWSCognitoAuthPluginConfigTests.swift @@ -235,5 +235,101 @@ class AWSCognitoAuthPluginConfigTests: XCTestCase { XCTFail("Should not throw error. \(error)") } } + + /// Test Auth configuration with valid config for user pool and identity pool, with secure storage preferences + /// + /// - Given: Given valid config for user pool and identity pool, and secure storage preferences + /// - When: + /// - I configure auth with the given configuration and secure storage preferences + /// - Then: + /// - I should not get any error while configuring auth + /// + func testConfigWithUserPoolAndIdentityPoolWithSecureStoragePreferences() throws { + let plugin = AWSCognitoAuthPlugin( + secureStoragePreferences: .init( + accessGroup: AccessGroup(name: "xx") + ) + ) + try Amplify.add(plugin: plugin) + + let categoryConfig = AuthCategoryConfiguration(plugins: [ + "awsCognitoAuthPlugin": [ + "CredentialsProvider": ["CognitoIdentity": ["Default": + ["PoolId": "xx", + "Region": "us-east-1"] + ]], + "CognitoUserPool": ["Default": [ + "PoolId": "xx", + "Region": "us-east-1", + "AppClientId": "xx", + "AppClientSecret": "xx"]] + ] + ]) + let amplifyConfig = AmplifyConfiguration(auth: categoryConfig) + do { + try Amplify.configure(amplifyConfig) + + let escapeHatch = plugin.getEscapeHatch() + guard case .userPoolAndIdentityPool(let userPoolClient, let identityPoolClient) = escapeHatch else { + XCTFail("Expected .userPool, got \(escapeHatch)") + return + } + XCTAssertNotNil(userPoolClient) + XCTAssertNotNil(identityPoolClient) + + } catch { + XCTFail("Should not throw error. \(error)") + } + } + + /// Test Auth configuration with valid config for user pool and identity pool, with network preferences and secure storage preferences + /// + /// - Given: Given valid config for user pool and identity pool, network preferences, and secure storage preferences + /// - When: + /// - I configure auth with the given configuration, network preferences, and secure storage preferences + /// - Then: + /// - I should not get any error while configuring auth + /// + func testConfigWithUserPoolAndIdentityPoolWithNetworkPreferencesAndSecureStoragePreferences() throws { + let plugin = AWSCognitoAuthPlugin( + networkPreferences: .init( + maxRetryCount: 2, + timeoutIntervalForRequest: 60, + timeoutIntervalForResource: 60), + secureStoragePreferences: .init( + accessGroup: AccessGroup(name: "xx") + ) + ) + try Amplify.add(plugin: plugin) + + let categoryConfig = AuthCategoryConfiguration(plugins: [ + "awsCognitoAuthPlugin": [ + "CredentialsProvider": ["CognitoIdentity": ["Default": + ["PoolId": "xx", + "Region": "us-east-1"] + ]], + "CognitoUserPool": ["Default": [ + "PoolId": "xx", + "Region": "us-east-1", + "AppClientId": "xx", + "AppClientSecret": "xx"]] + ] + ]) + let amplifyConfig = AmplifyConfiguration(auth: categoryConfig) + do { + try Amplify.configure(amplifyConfig) + + let escapeHatch = plugin.getEscapeHatch() + guard case .userPoolAndIdentityPool(let userPoolClient, let identityPoolClient) = escapeHatch else { + XCTFail("Expected .userPool, got \(escapeHatch)") + return + } + XCTAssertNotNil(userPoolClient) + XCTAssertNotNil(identityPoolClient) + + } catch { + XCTFail("Should not throw error. \(error)") + } + } } diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Support/DefaultConfig.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Support/DefaultConfig.swift index 13e5a8781e..ef876129ae 100644 --- a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Support/DefaultConfig.swift +++ b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Support/DefaultConfig.swift @@ -366,6 +366,10 @@ struct MockLegacyStore: KeychainStoreBehavior { func _removeAll() throws { } + + func _getAll() throws -> [(key: String, value: Data)] { + return [] + } } diff --git a/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthHostApp.xcodeproj/project.pbxproj b/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthHostApp.xcodeproj/project.pbxproj index efe5f198a7..3801340b4e 100644 --- a/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthHostApp.xcodeproj/project.pbxproj +++ b/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthHostApp.xcodeproj/project.pbxproj @@ -216,6 +216,8 @@ B43C26C827BC9D54003F3BF7 /* AuthConfirmSignUpTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AuthConfirmSignUpTests.swift; sourceTree = ""; }; B43C26C927BC9D54003F3BF7 /* AuthResendSignUpCodeTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AuthResendSignUpCodeTests.swift; sourceTree = ""; }; B4B9F45628F47B7B004F346F /* amplify-ios */ = {isa = PBXFileReference; lastKnownFileType = wrapper; name = "amplify-ios"; path = ../../../..; sourceTree = ""; }; + E2A7D1732C5D76CB00B06999 /* AuthHostApp.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = AuthHostApp.entitlements; sourceTree = ""; }; + E2A7D1742C5D774200B06999 /* AuthWatchApp.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = AuthWatchApp.entitlements; sourceTree = ""; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -303,6 +305,7 @@ 485CB53127B614CE006CCEC7 = { isa = PBXGroup; children = ( + E2A7D1742C5D774200B06999 /* AuthWatchApp.entitlements */, 485CB5C627B62C5C006CCEC7 /* Packages */, 485CB53C27B614CE006CCEC7 /* AuthHostApp */, 485CB5A027B61E04006CCEC7 /* AuthIntegrationTests */, @@ -328,6 +331,7 @@ 485CB53C27B614CE006CCEC7 /* AuthHostApp */ = { isa = PBXGroup; children = ( + E2A7D1732C5D76CB00B06999 /* AuthHostApp.entitlements */, 681DFEA728E747B80000C36A /* AsyncTesting */, 485CB53D27B614CE006CCEC7 /* AuthHostAppApp.swift */, 485CB53F27B614CE006CCEC7 /* ContentView.swift */, @@ -1135,6 +1139,7 @@ buildSettings = { ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon; ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor; + CODE_SIGN_ENTITLEMENTS = AuthHostApp/AuthHostApp.entitlements; CODE_SIGN_STYLE = Automatic; CURRENT_PROJECT_VERSION = 1; DEVELOPMENT_ASSET_PATHS = "\"AuthHostApp/Preview Content\""; @@ -1168,6 +1173,7 @@ buildSettings = { ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon; ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor; + CODE_SIGN_ENTITLEMENTS = AuthHostApp/AuthHostApp.entitlements; CODE_SIGN_STYLE = Automatic; CURRENT_PROJECT_VERSION = 1; DEVELOPMENT_ASSET_PATHS = "\"AuthHostApp/Preview Content\""; @@ -1245,6 +1251,7 @@ buildSettings = { ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor; CLANG_CXX_LANGUAGE_STANDARD = "gnu++20"; + CODE_SIGN_ENTITLEMENTS = AuthWatchApp.entitlements; CODE_SIGN_STYLE = Automatic; CURRENT_PROJECT_VERSION = 1; DEVELOPMENT_ASSET_PATHS = ""; @@ -1275,6 +1282,7 @@ buildSettings = { ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor; CLANG_CXX_LANGUAGE_STANDARD = "gnu++20"; + CODE_SIGN_ENTITLEMENTS = AuthWatchApp.entitlements; CODE_SIGN_STYLE = Automatic; CURRENT_PROJECT_VERSION = 1; DEVELOPMENT_ASSET_PATHS = ""; diff --git a/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthHostApp/AuthHostApp.entitlements b/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthHostApp/AuthHostApp.entitlements new file mode 100644 index 0000000000..795ed76cfc --- /dev/null +++ b/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthHostApp/AuthHostApp.entitlements @@ -0,0 +1,11 @@ + + + + + keychain-access-groups + + $(AppIdentifierPrefix)com.aws.amplify.auth.AuthHostAppShared + $(AppIdentifierPrefix)com.aws.amplify.auth.AuthHostAppShared2 + + + diff --git a/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthIntegrationTests/AWSAuthBaseTest.swift b/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthIntegrationTests/AWSAuthBaseTest.swift index 69668eee0d..7c8ca70cf8 100644 --- a/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthIntegrationTests/AWSAuthBaseTest.swift +++ b/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthIntegrationTests/AWSAuthBaseTest.swift @@ -30,6 +30,10 @@ class AWSAuthBaseTest: XCTestCase { var amplifyOutputsFile = "testconfiguration/AWSCognitoAuthPluginIntegrationTests-amplify_outputs" let credentialsFile = "testconfiguration/AWSCognitoAuthPluginIntegrationTests-credentials" + let keychainAccessGroup = "94KV3E626L.com.aws.amplify.auth.AuthHostAppShared" + let keychainAccessGroup2 = "94KV3E626L.com.aws.amplify.auth.AuthHostAppShared2" + let keychainAccessGroupWatch = "W3DRXD72QU.com.amazon.aws.amplify.swift.AuthWatchAppShared" + let keychainAccessGroupWatch2 = "W3DRXD72QU.com.amazon.aws.amplify.swift.AuthWatchAppShared2" var amplifyConfiguration: AmplifyConfiguration! var amplifyOutputs: AmplifyOutputsData! diff --git a/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthIntegrationTests/CredentialStore/CredentialStoreConfigurationTests.swift b/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthIntegrationTests/CredentialStore/CredentialStoreConfigurationTests.swift index 0dd8d34e9f..5eaa8a6a9c 100644 --- a/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthIntegrationTests/CredentialStore/CredentialStoreConfigurationTests.swift +++ b/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthIntegrationTests/CredentialStore/CredentialStoreConfigurationTests.swift @@ -210,4 +210,315 @@ class CredentialStoreConfigurationTests: AWSAuthBaseTest { let credentials = try? newCredentialStore.retrieveCredential() XCTAssertNil(credentials) } + + /// Test migrating to a shared access group keeps credentials + /// + /// - Given: A user registered is configured + /// - When: + /// - The credential store is re-initialized with shared access group and migration set to true + /// - Then: + /// - The old credentials should still persist + /// + func testCredentialsRemainOnMigrationToSharedAccessGroup() { + // Given + let identityId = "identityId" + // Migration only happens if credentials are not expired, hence + // the need for nonimmediate expiration test data + let awsCredentials = AuthAWSCognitoCredentials.nonimmediateExpiryTestData + let initialCognitoCredentials = AmplifyCredentials.userPoolAndIdentityPool( + signedInData: .testData, + identityID: identityId, + credentials: awsCredentials) + let initialAuthConfig = AuthConfiguration.userPoolsAndIdentityPools( + Defaults.makeDefaultUserPoolConfigData(), + Defaults.makeIdentityConfigData()) + let credentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig) + do { + try credentialStore.saveCredential(initialCognitoCredentials) + } catch { + XCTFail("Unable to save credentials") + } + + // When migrating to shared access group with same configuration + #if os(watchOS) + let newCredentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroupWatch, migrateKeychainItemsOfUserSession: true) + #else + let newCredentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroup, migrateKeychainItemsOfUserSession: true) + #endif + + // Then + guard let credentials = try? newCredentialStore.retrieveCredential(), + case .userPoolAndIdentityPool(let retrievedTokens, + let retrievedIdentityID, + let retrievedCredentials) = credentials else { + XCTFail("Unable to retrieve Credentials") + return + } + XCTAssertNotNil(credentials) + XCTAssertNotNil(retrievedTokens) + XCTAssertNotNil(retrievedIdentityID) + XCTAssertNotNil(retrievedCredentials) + XCTAssertEqual(retrievedIdentityID, identityId) + XCTAssertEqual(retrievedCredentials, awsCredentials) + } + + /// Test migrating from a shared access group to an unshared access group keeps credentials + /// + /// - Given: A user registered is configured + /// - When: + /// - The credential store is re-initialized with unshared access group and migration set to true + /// - Then: + /// - The old credentials should still persist + /// + func testCredentialsRemainOnMigrationFromSharedAccessGroup() { + // Given + let identityId = "identityId" + let awsCredentials = AuthAWSCognitoCredentials.nonimmediateExpiryTestData + // Migration only happens if credentials are not expired, hence + // the need for nonimmediate expiration test data + let initialCognitoCredentials = AmplifyCredentials.userPoolAndIdentityPool( + signedInData: .testData, + identityID: identityId, + credentials: awsCredentials) + let initialAuthConfig = AuthConfiguration.userPoolsAndIdentityPools( + Defaults.makeDefaultUserPoolConfigData(), + Defaults.makeIdentityConfigData()) + #if os(watchOS) + let credentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroupWatch) + #else + let credentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroup) + #endif + do { + try credentialStore.saveCredential(initialCognitoCredentials) + } catch { + XCTFail("Unable to save credentials") + } + + // When migrating to unshared access group with same configuration + let newCredentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, migrateKeychainItemsOfUserSession: true) + + // Then + guard let credentials = try? newCredentialStore.retrieveCredential(), + case .userPoolAndIdentityPool(let retrievedTokens, + let retrievedIdentityID, + let retrievedCredentials) = credentials else { + XCTFail("Unable to retrieve Credentials") + return + } + XCTAssertNotNil(credentials) + XCTAssertNotNil(retrievedTokens) + XCTAssertNotNil(retrievedIdentityID) + XCTAssertNotNil(retrievedCredentials) + XCTAssertEqual(retrievedIdentityID, identityId) + XCTAssertEqual(retrievedCredentials, awsCredentials) + } + + /// Test migrating from a shared access group to another shared access group keeps credentials + /// + /// - Given: A user registered is configured + /// - When: + /// - The credential store is re-initialized with another shared access group and migration set to true + /// - Then: + /// - The old credentials should still persist + /// + func testCredentialsRemainOnMigrationFromSharedAccessGroupToAnotherSharedAccessGroup() { + // Given + let identityId = "identityId" + let awsCredentials = AuthAWSCognitoCredentials.nonimmediateExpiryTestData + // Migration only happens if credentials are not expired, hence + // the need for nonimmediate expiration test data + let initialCognitoCredentials = AmplifyCredentials.userPoolAndIdentityPool( + signedInData: .testData, + identityID: identityId, + credentials: awsCredentials) + let initialAuthConfig = AuthConfiguration.userPoolsAndIdentityPools( + Defaults.makeDefaultUserPoolConfigData(), + Defaults.makeIdentityConfigData()) + #if os(watchOS) + let credentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroupWatch) + #else + let credentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroup) + #endif + do { + try credentialStore.saveCredential(initialCognitoCredentials) + } catch { + XCTFail("Unable to save credentials") + } + + // When migrating to another shared access group with same configuration + #if os(watchOS) + let newCredentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroupWatch2, migrateKeychainItemsOfUserSession: true) + #else + let newCredentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroup2, migrateKeychainItemsOfUserSession: true) + #endif + + // Then + guard let credentials = try? newCredentialStore.retrieveCredential(), + case .userPoolAndIdentityPool(let retrievedTokens, + let retrievedIdentityID, + let retrievedCredentials) = credentials else { + XCTFail("Unable to retrieve Credentials") + return + } + XCTAssertNotNil(credentials) + XCTAssertNotNil(retrievedTokens) + XCTAssertNotNil(retrievedIdentityID) + XCTAssertNotNil(retrievedCredentials) + XCTAssertEqual(retrievedIdentityID, identityId) + XCTAssertEqual(retrievedCredentials, awsCredentials) + } + + /// Test moving to a shared access group without migration should not keep credentials + /// + /// - Given: A user registered is configured + /// - When: + /// - The credential store is re-initialized with shared access group and migration set to false + /// - Then: + /// - The old credentials should not persist + /// + func testCredentialsDoNotRemainOnNonMigrationToSharedAccessGroup() { + // Given + let identityId = "identityId" + let awsCredentials = AuthAWSCognitoCredentials.nonimmediateExpiryTestData + let initialCognitoCredentials = AmplifyCredentials.userPoolAndIdentityPool( + signedInData: .testData, + identityID: identityId, + credentials: awsCredentials) + let initialAuthConfig = AuthConfiguration.userPoolsAndIdentityPools( + Defaults.makeDefaultUserPoolConfigData(), + Defaults.makeIdentityConfigData()) + let credentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig) + do { + try credentialStore.saveCredential(initialCognitoCredentials) + } catch { + XCTFail("Unable to save credentials") + } + + // When moving to shared access group with same configuration but without migration + #if os(watchOS) + let newCredentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroupWatch, migrateKeychainItemsOfUserSession: false) + #else + let newCredentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroup, migrateKeychainItemsOfUserSession: false) + #endif + + // Then + guard let credentials = try? newCredentialStore.retrieveCredential(), + case .userPoolAndIdentityPool(let retrievedTokens, + let retrievedIdentityID, + let retrievedCredentials) = credentials else { + // Expected + return + } + + // If credentials are present, they should not be the same as those that were not migrated + XCTAssertNotNil(credentials) + XCTAssertNotNil(retrievedTokens) + XCTAssertNotNil(retrievedIdentityID) + XCTAssertNotNil(retrievedCredentials) + XCTAssertNotEqual(retrievedCredentials, awsCredentials) + } + + /// Test moving from a shared access group to an unshared access group without migration should not keep credentials + /// + /// - Given: A user registered is configured + /// - When: + /// - The credential store is re-initialized with unshared access group and migration set to false + /// - Then: + /// - The old credentials should not persist + /// + func testCredentialsDoNotRemainOnNonMigrationFromSharedAccessGroup() { + // Given + let identityId = "identityId" + let awsCredentials = AuthAWSCognitoCredentials.nonimmediateExpiryTestData + let initialCognitoCredentials = AmplifyCredentials.userPoolAndIdentityPool( + signedInData: .testData, + identityID: identityId, + credentials: awsCredentials) + let initialAuthConfig = AuthConfiguration.userPoolsAndIdentityPools( + Defaults.makeDefaultUserPoolConfigData(), + Defaults.makeIdentityConfigData()) + #if os(watchOS) + let credentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroupWatch) + #else + let credentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroup) + #endif + do { + try credentialStore.saveCredential(initialCognitoCredentials) + } catch { + XCTFail("Unable to save credentials") + } + + // When moving to unshared access group with same configuration but without migration + let newCredentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, migrateKeychainItemsOfUserSession: false) + + // Then + guard let credentials = try? newCredentialStore.retrieveCredential(), + case .userPoolAndIdentityPool(let retrievedTokens, + let retrievedIdentityID, + let retrievedCredentials) = credentials else { + // Expected + return + } + + // If credentials are present, they should not be the same as those that were not migrated + XCTAssertNotNil(credentials) + XCTAssertNotNil(retrievedTokens) + XCTAssertNotNil(retrievedIdentityID) + XCTAssertNotNil(retrievedCredentials) + XCTAssertNotEqual(retrievedCredentials, awsCredentials) + } + + /// Test moving from a shared access group to another shared access group without migration should not keep credentials + /// + /// - Given: A user registered is configured + /// - When: + /// - The credential store is re-initialized with another shared access group and migration set to false + /// - Then: + /// - The old credentials should not persist + /// + func testCredentialsDoNotRemainOnNonMigrationFromSharedAccessGroupToAnotherSharedAccessGroup() { + // Given + let identityId = "identityId" + let awsCredentials = AuthAWSCognitoCredentials.nonimmediateExpiryTestData + let initialCognitoCredentials = AmplifyCredentials.userPoolAndIdentityPool( + signedInData: .testData, + identityID: identityId, + credentials: awsCredentials) + let initialAuthConfig = AuthConfiguration.userPoolsAndIdentityPools( + Defaults.makeDefaultUserPoolConfigData(), + Defaults.makeIdentityConfigData()) + #if os(watchOS) + let credentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroupWatch) + #else + let credentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroup) + #endif + do { + try credentialStore.saveCredential(initialCognitoCredentials) + } catch { + XCTFail("Unable to save credentials") + } + + // When moving to another shared access group with same configuration but without migration + #if os(watchOS) + let newCredentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroupWatch2, migrateKeychainItemsOfUserSession: false) + #else + let newCredentialStore = AWSCognitoAuthCredentialStore(authConfiguration: initialAuthConfig, accessGroup: keychainAccessGroup2, migrateKeychainItemsOfUserSession: false) + #endif + + // Then + guard let credentials = try? newCredentialStore.retrieveCredential(), + case .userPoolAndIdentityPool(let retrievedTokens, + let retrievedIdentityID, + let retrievedCredentials) = credentials else { + // Expected + return + } + + // If credentials are present, they should not be the same as those that were not migrated + XCTAssertNotNil(credentials) + XCTAssertNotNil(retrievedTokens) + XCTAssertNotNil(retrievedIdentityID) + XCTAssertNotNil(retrievedCredentials) + XCTAssertNotEqual(retrievedCredentials, awsCredentials) + } } diff --git a/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthIntegrationTests/Helpers/AuthEnvironmentHelper.swift b/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthIntegrationTests/Helpers/AuthEnvironmentHelper.swift index cf495aa7fb..dbe55e8d23 100644 --- a/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthIntegrationTests/Helpers/AuthEnvironmentHelper.swift +++ b/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthIntegrationTests/Helpers/AuthEnvironmentHelper.swift @@ -40,6 +40,14 @@ extension AuthAWSCognitoCredentials { sessionToken: "xx", expiration: Date()) } + + static var nonimmediateExpiryTestData: AuthAWSCognitoCredentials { + return AuthAWSCognitoCredentials( + accessKeyId: "xx", + secretAccessKey: "xx", + sessionToken: "xx", + expiration: Date() + TimeInterval(200)) + } } extension AWSCognitoUserPoolTokens { diff --git a/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthWatchApp.entitlements b/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthWatchApp.entitlements new file mode 100644 index 0000000000..5658c2919e --- /dev/null +++ b/AmplifyPlugins/Auth/Tests/AuthHostApp/AuthWatchApp.entitlements @@ -0,0 +1,11 @@ + + + + + keychain-access-groups + + $(AppIdentifierPrefix)com.amazon.aws.amplify.swift.AuthWatchAppShared + $(AppIdentifierPrefix)com.amazon.aws.amplify.swift.AuthWatchAppShared2 + + + diff --git a/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift b/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift index 6327676c83..de534d7b67 100644 --- a/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift +++ b/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift @@ -52,6 +52,11 @@ public protocol KeychainStoreBehavior { /// Removes all key-value pair in the Keychain. /// This System Programming Interface (SPI) may have breaking changes in future updates. func _removeAll() throws + + @_spi(KeychainStore) + /// Retrieves all key-value pairs in the keychain + /// This System Programming Interface (SPI) may have breaking changes in future updates. + func _getAll() throws -> [(key: String, value: Data)] } public struct KeychainStore: KeychainStoreBehavior { @@ -70,14 +75,13 @@ public struct KeychainStore: KeychainStoreBehavior { } public init(service: String) { - self.init(service: service, accessGroup: nil) + attributes = KeychainStoreAttributes(service: service) + log.verbose("[KeychainStore] Initialized keychain with service=\(service), attributes=\(attributes), accessGroup=") } public init(service: String, accessGroup: String? = nil) { - var attributes = KeychainStoreAttributes(service: service) - attributes.accessGroup = accessGroup - self.init(attributes: attributes) - log.verbose("[KeychainStore] Initialized keychain with service=\(service), attributes=\(attributes), accessGroup=\(accessGroup ?? "")") + attributes = KeychainStoreAttributes(service: service, accessGroup: accessGroup) + log.verbose("[KeychainStore] Initialized keychain with service=\(service), attributes=\(attributes), accessGroup=\(attributes.accessGroup ?? "")") } @_spi(KeychainStore) @@ -206,7 +210,7 @@ public struct KeychainStore: KeychainStoreBehavior { let status = SecItemDelete(query as CFDictionary) if status != errSecSuccess && status != errSecItemNotFound { - log.error("[KeychainStore] Error removing itms from keychain with status=\(status)") + log.error("[KeychainStore] Error removing items from keychain with status=\(status)") throw KeychainStoreError.securityError(status) } log.verbose("[KeychainStore] Successfully removed item from keychain") @@ -229,6 +233,48 @@ public struct KeychainStore: KeychainStoreBehavior { } log.verbose("[KeychainStore] Successfully removed all items from keychain") } + + @_spi(KeychainStore) + /// Retrieves all key-value pairs in the keychain + /// This System Programming Interface (SPI) may have breaking changes in future updates. + public func _getAll() throws -> [(key: String, value: Data)] { + log.verbose("[KeychainStore] Starting to retrieve all items from keychain") + var query = attributes.defaultGetQuery() + query[Constants.MatchLimit] = Constants.MatchLimitAll + query[Constants.ReturnData] = kCFBooleanTrue + query[Constants.ReturnAttributes] = kCFBooleanTrue + query[Constants.ReturnRef] = kCFBooleanTrue + + var result: AnyObject? + let status = SecItemCopyMatching(query as CFDictionary, &result) + + switch status { + case errSecSuccess: + guard let items = result as? [[String: Any]] else { + log.error("[KeychainStore] The keychain items retrieved are not the correct type") + throw KeychainStoreError.unknown("The keychain items retrieved are not the correct type") + } + + var keyValuePairs = [(key: String, value: Data)]() + for item in items { + guard let key = item[Constants.AttributeAccount] as? String, + let value = item[Constants.ValueData] as? Data else { + log.error("[KeychainStore] Unable to retrieve key or value from keychain item") + continue + } + keyValuePairs.append((key: key, value: value)) + } + + log.verbose("[KeychainStore] Successfully retrieved \(keyValuePairs.count) items from keychain") + return keyValuePairs + case errSecItemNotFound: + log.verbose("[KeychainStore] No items found in keychain") + return [] + default: + log.error("[KeychainStore] Error of status=\(status) occurred when attempting to retrieve all items from keychain") + throw KeychainStoreError.securityError(status) + } + } } @@ -258,6 +304,7 @@ extension KeychainStore { /** Return Type Key Constants */ static let ReturnData = String(kSecReturnData) static let ReturnAttributes = String(kSecReturnAttributes) + static let ReturnRef = String(kSecReturnRef) /** Value Type Key Constants */ static let ValueData = String(kSecValueData) diff --git a/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreAttributes.swift b/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreAttributes.swift index a638b2879b..bbbab1d275 100644 --- a/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreAttributes.swift +++ b/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreAttributes.swift @@ -24,7 +24,7 @@ extension KeychainStoreAttributes { KeychainStore.Constants.UseDataProtectionKeyChain: kCFBooleanTrue ] - if let accessGroup = accessGroup { + if let accessGroup { query[KeychainStore.Constants.AttributeAccessGroup] = accessGroup } return query diff --git a/AmplifyPlugins/Core/AWSPluginsCoreTests/Keychain/KeychainStoreAttributesTests.swift b/AmplifyPlugins/Core/AWSPluginsCoreTests/Keychain/KeychainStoreAttributesTests.swift index 9c474d3a4f..501d8c8c37 100644 --- a/AmplifyPlugins/Core/AWSPluginsCoreTests/Keychain/KeychainStoreAttributesTests.swift +++ b/AmplifyPlugins/Core/AWSPluginsCoreTests/Keychain/KeychainStoreAttributesTests.swift @@ -45,10 +45,28 @@ class KeychainStoreAttributesTests: XCTestCase { XCTAssertNil(defaultGetAttributes[KeychainStore.Constants.AttributeAccessible] as? String) XCTAssertNil(defaultGetAttributes[KeychainStore.Constants.UseDataProtectionKeyChain] as? String) } + + /// Given: an instance of `KeychainStoreAttributes` + /// When: `keychainStoreAttribute.defaultSetQuery()` is invoked with a required service param + /// Then: Validate if the attributes contain the correct set query params + /// - AttributeService + /// - Class + /// - AttributeAccessible + /// - UseDataProtectionKeyChain + func testDefaultSetQuery() { + keychainStoreAttribute = KeychainStoreAttributes(service: "someService") + + let defaultSetAttributes = keychainStoreAttribute.defaultSetQuery() + XCTAssertEqual(defaultSetAttributes[KeychainStore.Constants.AttributeService] as? String, "someService") + XCTAssertEqual(defaultSetAttributes[KeychainStore.Constants.Class] as? String, KeychainStore.Constants.ClassGenericPassword) + XCTAssertNil(defaultSetAttributes[KeychainStore.Constants.AttributeAccessGroup] as? String) + XCTAssertEqual(defaultSetAttributes[KeychainStore.Constants.AttributeAccessible] as? String, KeychainStore.Constants.AttributeAccessibleAfterFirstUnlockThisDeviceOnly) + XCTAssertEqual(defaultSetAttributes[KeychainStore.Constants.UseDataProtectionKeyChain] as? Bool, true) + } /// Given: an instance of `KeychainStoreAttributes` /// When: `keychainStoreAttribute.defaultSetQuery()` is invoked with a required service param and access group - /// Then: Validate if the attributes contain the correct get query params + /// Then: Validate if the attributes contain the correct set query params /// - AttributeService /// - Class /// - AttributeAccessGroup @@ -57,12 +75,12 @@ class KeychainStoreAttributesTests: XCTestCase { func testDefaultSetQueryWithAccessGroup() { keychainStoreAttribute = KeychainStoreAttributes(service: "someService", accessGroup: "someAccessGroup") - let defaultGetAttributes = keychainStoreAttribute.defaultSetQuery() - XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.AttributeService] as? String, "someService") - XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.Class] as? String, KeychainStore.Constants.ClassGenericPassword) - XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.AttributeAccessGroup] as? String, "someAccessGroup") - XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.AttributeAccessible] as? String, KeychainStore.Constants.AttributeAccessibleAfterFirstUnlockThisDeviceOnly) - XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.UseDataProtectionKeyChain] as? Bool, true) + let defaultSetAttributes = keychainStoreAttribute.defaultSetQuery() + XCTAssertEqual(defaultSetAttributes[KeychainStore.Constants.AttributeService] as? String, "someService") + XCTAssertEqual(defaultSetAttributes[KeychainStore.Constants.Class] as? String, KeychainStore.Constants.ClassGenericPassword) + XCTAssertEqual(defaultSetAttributes[KeychainStore.Constants.AttributeAccessGroup] as? String, "someAccessGroup") + XCTAssertEqual(defaultSetAttributes[KeychainStore.Constants.AttributeAccessible] as? String, KeychainStore.Constants.AttributeAccessibleAfterFirstUnlockThisDeviceOnly) + XCTAssertEqual(defaultSetAttributes[KeychainStore.Constants.UseDataProtectionKeyChain] as? Bool, true) } override func tearDown() { diff --git a/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockKeychainStore.swift b/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockKeychainStore.swift index e2c127588e..3596ce8241 100644 --- a/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockKeychainStore.swift +++ b/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockKeychainStore.swift @@ -61,6 +61,20 @@ class MockKeychainStore: KeychainStoreBehavior { stringValues.removeAll() dataValues.removeAll() } + + func _getAll() throws -> [(key: String, value: Data)] { + var allValues: [(key: String, value: Data)] = [] + + for (key, value) in dataValues { + allValues.append((key: key, value: value)) + } + + for (key, value) in stringValues { + allValues.append((key: key, value: value.data(using: .utf8)!)) + } + + return allValues + } func resetCounters() { dataForKeyCount = 0 From 8dd74ed1be74ad0f8a7f15d36007f310aa140eb8 Mon Sep 17 00:00:00 2001 From: Yaro Luchko Date: Mon, 12 Aug 2024 17:02:53 -0700 Subject: [PATCH 2/7] Reconfigure when fetching auth session if sharing keychain --- .../AWSCognitoAuthPlugin+ClientBehavior.swift | 6 +++++- .../Task/AWSAuthFetchSessionTask.swift | 11 ++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/ClientBehavior/AWSCognitoAuthPlugin+ClientBehavior.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/ClientBehavior/AWSCognitoAuthPlugin+ClientBehavior.swift index 75bed80e89..70530e76e0 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/ClientBehavior/AWSCognitoAuthPlugin+ClientBehavior.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/ClientBehavior/AWSCognitoAuthPlugin+ClientBehavior.swift @@ -114,7 +114,11 @@ extension AWSCognitoAuthPlugin: AuthCategoryBehavior { public func fetchAuthSession(options: AuthFetchSessionRequest.Options?) async throws -> AuthSession { let options = options ?? AuthFetchSessionRequest.Options() let request = AuthFetchSessionRequest(options: options) - let task = AWSAuthFetchSessionTask(request, authStateMachine: authStateMachine) + let forceReconfigure = secureStoragePreferences?.accessGroup?.name != nil + let task = AWSAuthFetchSessionTask(request, + authStateMachine: authStateMachine, + configuration: authConfiguration, + forceReconfigure: forceReconfigure) return try await taskQueue.sync { return try await task.value } as! AuthSession diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Task/AWSAuthFetchSessionTask.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Task/AWSAuthFetchSessionTask.swift index 7a00941ad9..0350f08e5a 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Task/AWSAuthFetchSessionTask.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Task/AWSAuthFetchSessionTask.swift @@ -13,20 +13,29 @@ class AWSAuthFetchSessionTask: AuthFetchSessionTask, DefaultLogger { private let authStateMachine: AuthStateMachine private let fetchAuthSessionHelper: FetchAuthSessionOperationHelper private let taskHelper: AWSAuthTaskHelper + private let configuration: AuthConfiguration + private let forceReconfigure: Bool var eventName: HubPayloadEventName { HubPayload.EventName.Auth.fetchSessionAPI } - init(_ request: AuthFetchSessionRequest, authStateMachine: AuthStateMachine) { + init(_ request: AuthFetchSessionRequest, authStateMachine: AuthStateMachine, configuration: AuthConfiguration, forceReconfigure: Bool = false) { self.request = request self.authStateMachine = authStateMachine self.fetchAuthSessionHelper = FetchAuthSessionOperationHelper() self.taskHelper = AWSAuthTaskHelper(authStateMachine: authStateMachine) + self.configuration = configuration + self.forceReconfigure = forceReconfigure } func execute() async throws -> AuthSession { log.verbose("Starting execution") + if forceReconfigure { + log.verbose("Reconfiguring auth state machine for keychain sharing") + let event = AuthEvent(eventType: .reconfigure(configuration)) + await authStateMachine.send(event) + } await taskHelper.didStateMachineConfigured() let doesNeedForceRefresh = request.options.forceRefresh return try await fetchAuthSessionHelper.fetch(authStateMachine, From 3dfa43b29722bdf2734608067e03df86bf1c7537 Mon Sep 17 00:00:00 2001 From: aws-amplify-ops Date: Tue, 13 Aug 2024 00:08:33 +0000 Subject: [PATCH 3/7] Update API dumps for new version --- api-dump/AWSDataStorePlugin.json | 2 +- api-dump/AWSPluginsCore.json | 97 +++++++++++- api-dump/Amplify.json | 216 +++++++++++++++++++++++++- api-dump/CoreMLPredictionsPlugin.json | 2 +- 4 files changed, 313 insertions(+), 4 deletions(-) diff --git a/api-dump/AWSDataStorePlugin.json b/api-dump/AWSDataStorePlugin.json index 47c0e792de..8b0d52d144 100644 --- a/api-dump/AWSDataStorePlugin.json +++ b/api-dump/AWSDataStorePlugin.json @@ -8205,7 +8205,7 @@ "-module", "AWSDataStorePlugin", "-o", - "\/var\/folders\/hn\/5bx1f4_d4ds5vhwhkxc7vdcr0000gn\/T\/tmp.nAGRifhwH6\/AWSDataStorePlugin.json", + "\/var\/folders\/hw\/1f0gcr8d6kn9ms0_wn0_57qc0000gn\/T\/tmp.Op48snuvLw\/AWSDataStorePlugin.json", "-I", ".build\/debug", "-sdk-version", diff --git a/api-dump/AWSPluginsCore.json b/api-dump/AWSPluginsCore.json index 19b8e2fb57..74a86c25c0 100644 --- a/api-dump/AWSPluginsCore.json +++ b/api-dump/AWSPluginsCore.json @@ -5187,6 +5187,55 @@ "throwing": true, "reqNewWitnessTableEntry": true, "funcSelfKind": "NonMutating" + }, + { + "kind": "Function", + "name": "_getAll", + "printedName": "_getAll()", + "children": [ + { + "kind": "TypeNominal", + "name": "Array", + "printedName": "[(key: Swift.String, value: Foundation.Data)]", + "children": [ + { + "kind": "TypeNominal", + "name": "Tuple", + "printedName": "(key: Swift.String, value: Foundation.Data)", + "children": [ + { + "kind": "TypeNominal", + "name": "String", + "printedName": "Swift.String", + "usr": "s:SS" + }, + { + "kind": "TypeNominal", + "name": "Data", + "printedName": "Foundation.Data", + "usr": "s:10Foundation4DataV" + } + ] + } + ], + "usr": "s:Sa" + } + ], + "declKind": "Func", + "usr": "s:14AWSPluginsCore21KeychainStoreBehaviorP7_getAllSaySS3key_10Foundation4DataV5valuetGyKF", + "mangledName": "$s14AWSPluginsCore21KeychainStoreBehaviorP7_getAllSaySS3key_10Foundation4DataV5valuetGyKF", + "moduleName": "AWSPluginsCore", + "genericSig": "", + "protocolReq": true, + "declAttributes": [ + "SPIAccessControl" + ], + "spi_group_names": [ + "KeychainStore" + ], + "throwing": true, + "reqNewWitnessTableEntry": true, + "funcSelfKind": "NonMutating" } ], "declKind": "Protocol", @@ -5468,6 +5517,52 @@ "throwing": true, "funcSelfKind": "NonMutating" }, + { + "kind": "Function", + "name": "_getAll", + "printedName": "_getAll()", + "children": [ + { + "kind": "TypeNominal", + "name": "Array", + "printedName": "[(key: Swift.String, value: Foundation.Data)]", + "children": [ + { + "kind": "TypeNominal", + "name": "Tuple", + "printedName": "(key: Swift.String, value: Foundation.Data)", + "children": [ + { + "kind": "TypeNominal", + "name": "String", + "printedName": "Swift.String", + "usr": "s:SS" + }, + { + "kind": "TypeNominal", + "name": "Data", + "printedName": "Foundation.Data", + "usr": "s:10Foundation4DataV" + } + ] + } + ], + "usr": "s:Sa" + } + ], + "declKind": "Func", + "usr": "s:14AWSPluginsCore13KeychainStoreV7_getAllSaySS3key_10Foundation4DataV5valuetGyKF", + "mangledName": "$s14AWSPluginsCore13KeychainStoreV7_getAllSaySS3key_10Foundation4DataV5valuetGyKF", + "moduleName": "AWSPluginsCore", + "declAttributes": [ + "SPIAccessControl" + ], + "spi_group_names": [ + "KeychainStore" + ], + "throwing": true, + "funcSelfKind": "NonMutating" + }, { "kind": "Var", "name": "log", @@ -24273,7 +24368,7 @@ "-module", "AWSPluginsCore", "-o", - "\/var\/folders\/hn\/5bx1f4_d4ds5vhwhkxc7vdcr0000gn\/T\/tmp.nAGRifhwH6\/AWSPluginsCore.json", + "\/var\/folders\/hw\/1f0gcr8d6kn9ms0_wn0_57qc0000gn\/T\/tmp.Op48snuvLw\/AWSPluginsCore.json", "-I", ".build\/debug", "-sdk-version", diff --git a/api-dump/Amplify.json b/api-dump/Amplify.json index 1bb4d36d2c..a4fee7f5ad 100644 --- a/api-dump/Amplify.json +++ b/api-dump/Amplify.json @@ -18636,6 +18636,220 @@ } ] }, + { + "kind": "TypeDecl", + "name": "AccessGroup", + "printedName": "AccessGroup", + "children": [ + { + "kind": "Var", + "name": "name", + "printedName": "name", + "children": [ + { + "kind": "TypeNominal", + "name": "Optional", + "printedName": "Swift.String?", + "children": [ + { + "kind": "TypeNominal", + "name": "String", + "printedName": "Swift.String", + "usr": "s:SS" + } + ], + "usr": "s:Sq" + } + ], + "declKind": "Var", + "usr": "s:7Amplify11AccessGroupV4nameSSSgvp", + "mangledName": "$s7Amplify11AccessGroupV4nameSSSgvp", + "moduleName": "Amplify", + "declAttributes": [ + "HasStorage" + ], + "isLet": true, + "hasStorage": true, + "accessors": [ + { + "kind": "Accessor", + "name": "Get", + "printedName": "Get()", + "children": [ + { + "kind": "TypeNominal", + "name": "Optional", + "printedName": "Swift.String?", + "children": [ + { + "kind": "TypeNominal", + "name": "String", + "printedName": "Swift.String", + "usr": "s:SS" + } + ], + "usr": "s:Sq" + } + ], + "declKind": "Accessor", + "usr": "s:7Amplify11AccessGroupV4nameSSSgvg", + "mangledName": "$s7Amplify11AccessGroupV4nameSSSgvg", + "moduleName": "Amplify", + "implicit": true, + "declAttributes": [ + "Transparent" + ], + "accessorKind": "get" + } + ] + }, + { + "kind": "Var", + "name": "migrateKeychainItems", + "printedName": "migrateKeychainItems", + "children": [ + { + "kind": "TypeNominal", + "name": "Bool", + "printedName": "Swift.Bool", + "usr": "s:Sb" + } + ], + "declKind": "Var", + "usr": "s:7Amplify11AccessGroupV20migrateKeychainItemsSbvp", + "mangledName": "$s7Amplify11AccessGroupV20migrateKeychainItemsSbvp", + "moduleName": "Amplify", + "declAttributes": [ + "HasStorage" + ], + "isLet": true, + "hasStorage": true, + "accessors": [ + { + "kind": "Accessor", + "name": "Get", + "printedName": "Get()", + "children": [ + { + "kind": "TypeNominal", + "name": "Bool", + "printedName": "Swift.Bool", + "usr": "s:Sb" + } + ], + "declKind": "Accessor", + "usr": "s:7Amplify11AccessGroupV20migrateKeychainItemsSbvg", + "mangledName": "$s7Amplify11AccessGroupV20migrateKeychainItemsSbvg", + "moduleName": "Amplify", + "implicit": true, + "declAttributes": [ + "Transparent" + ], + "accessorKind": "get" + } + ] + }, + { + "kind": "Constructor", + "name": "init", + "printedName": "init(name:migrateKeychainItemsOfUserSession:)", + "children": [ + { + "kind": "TypeNominal", + "name": "AccessGroup", + "printedName": "Amplify.AccessGroup", + "usr": "s:7Amplify11AccessGroupV" + }, + { + "kind": "TypeNominal", + "name": "String", + "printedName": "Swift.String", + "usr": "s:SS" + }, + { + "kind": "TypeNominal", + "name": "Bool", + "printedName": "Swift.Bool", + "hasDefaultArg": true, + "usr": "s:Sb" + } + ], + "declKind": "Constructor", + "usr": "s:7Amplify11AccessGroupV4name33migrateKeychainItemsOfUserSessionACSS_Sbtcfc", + "mangledName": "$s7Amplify11AccessGroupV4name33migrateKeychainItemsOfUserSessionACSS_Sbtcfc", + "moduleName": "Amplify", + "init_kind": "Designated" + }, + { + "kind": "Function", + "name": "none", + "printedName": "none(migrateKeychainItemsOfUserSession:)", + "children": [ + { + "kind": "TypeNominal", + "name": "AccessGroup", + "printedName": "Amplify.AccessGroup", + "usr": "s:7Amplify11AccessGroupV" + }, + { + "kind": "TypeNominal", + "name": "Bool", + "printedName": "Swift.Bool", + "usr": "s:Sb" + } + ], + "declKind": "Func", + "usr": "s:7Amplify11AccessGroupV4none33migrateKeychainItemsOfUserSessionACSb_tFZ", + "mangledName": "$s7Amplify11AccessGroupV4none33migrateKeychainItemsOfUserSessionACSb_tFZ", + "moduleName": "Amplify", + "static": true, + "funcSelfKind": "NonMutating" + }, + { + "kind": "Var", + "name": "none", + "printedName": "none", + "children": [ + { + "kind": "TypeNominal", + "name": "AccessGroup", + "printedName": "Amplify.AccessGroup", + "usr": "s:7Amplify11AccessGroupV" + } + ], + "declKind": "Var", + "usr": "s:7Amplify11AccessGroupV4noneACvpZ", + "mangledName": "$s7Amplify11AccessGroupV4noneACvpZ", + "moduleName": "Amplify", + "static": true, + "accessors": [ + { + "kind": "Accessor", + "name": "Get", + "printedName": "Get()", + "children": [ + { + "kind": "TypeNominal", + "name": "AccessGroup", + "printedName": "Amplify.AccessGroup", + "usr": "s:7Amplify11AccessGroupV" + } + ], + "declKind": "Accessor", + "usr": "s:7Amplify11AccessGroupV4noneACvgZ", + "mangledName": "$s7Amplify11AccessGroupV4noneACvgZ", + "moduleName": "Amplify", + "static": true, + "accessorKind": "get" + } + ] + } + ], + "declKind": "Struct", + "usr": "s:7Amplify11AccessGroupV", + "mangledName": "$s7Amplify11AccessGroupV", + "moduleName": "Amplify" + }, { "kind": "TypeAlias", "name": "AdditionalInfo", @@ -179735,7 +179949,7 @@ "-module", "Amplify", "-o", - "\/var\/folders\/hn\/5bx1f4_d4ds5vhwhkxc7vdcr0000gn\/T\/tmp.nAGRifhwH6\/Amplify.json", + "\/var\/folders\/hw\/1f0gcr8d6kn9ms0_wn0_57qc0000gn\/T\/tmp.Op48snuvLw\/Amplify.json", "-I", ".build\/debug", "-sdk-version", diff --git a/api-dump/CoreMLPredictionsPlugin.json b/api-dump/CoreMLPredictionsPlugin.json index 1a9879407f..1d04459209 100644 --- a/api-dump/CoreMLPredictionsPlugin.json +++ b/api-dump/CoreMLPredictionsPlugin.json @@ -430,7 +430,7 @@ "-module", "CoreMLPredictionsPlugin", "-o", - "\/var\/folders\/hn\/5bx1f4_d4ds5vhwhkxc7vdcr0000gn\/T\/tmp.nAGRifhwH6\/CoreMLPredictionsPlugin.json", + "\/var\/folders\/hw\/1f0gcr8d6kn9ms0_wn0_57qc0000gn\/T\/tmp.Op48snuvLw\/CoreMLPredictionsPlugin.json", "-I", ".build\/debug", "-sdk-version", From f5c01b1a62e0b5fed5a309977bb5bef4dfd071a8 Mon Sep 17 00:00:00 2001 From: Yaro Luchko Date: Thu, 15 Aug 2024 22:58:53 -0700 Subject: [PATCH 4/7] Indentation, clean up, and batch migration to avoid inconsistent state --- .../Categories/Auth/Models/AccessGroup.swift | 4 +- .../AWSCognitoAuthPlugin+Configure.swift | 7 ++- .../AWSCognitoAuthPlugin.swift | 4 +- .../AWSCognitoAuthCredentialStore.swift | 35 ++++-------- .../Task/AWSAuthFetchSessionTask.swift | 7 ++- .../Keychain/KeychainStore.swift | 9 ++- .../Keychain/KeychainStoreMigrator.swift | 56 +++++++++++++++++++ 7 files changed, 87 insertions(+), 35 deletions(-) create mode 100644 AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreMigrator.swift diff --git a/Amplify/Categories/Auth/Models/AccessGroup.swift b/Amplify/Categories/Auth/Models/AccessGroup.swift index 7fc1ebb3c6..27fa47d561 100644 --- a/Amplify/Categories/Auth/Models/AccessGroup.swift +++ b/Amplify/Categories/Auth/Models/AccessGroup.swift @@ -16,14 +16,14 @@ public struct AccessGroup { } public static func none(migrateKeychainItemsOfUserSession: Bool) -> AccessGroup { - return .init(name: nil, migrateKeychainItems: migrateKeychainItemsOfUserSession) + return .init(migrateKeychainItems: migrateKeychainItemsOfUserSession) } public static var none: AccessGroup { return .none(migrateKeychainItemsOfUserSession: false) } - private init(name: String?, migrateKeychainItems: Bool) { + private init(name: String? = nil, migrateKeychainItems: Bool) { self.name = name self.migrateKeychainItems = migrateKeychainItems } diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin+Configure.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin+Configure.swift index ee8bcaf58f..21d5675a2e 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin+Configure.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin+Configure.swift @@ -177,8 +177,11 @@ extension AWSCognitoAuthPlugin { } private func makeCredentialStore() -> AmplifyAuthCredentialStoreBehavior { - return AWSCognitoAuthCredentialStore(authConfiguration: authConfiguration, accessGroup: secureStoragePreferences?.accessGroup?.name, - migrateKeychainItemsOfUserSession: secureStoragePreferences?.accessGroup?.migrateKeychainItems ?? false) + return AWSCognitoAuthCredentialStore( + authConfiguration: authConfiguration, + accessGroup: secureStoragePreferences?.accessGroup?.name, + migrateKeychainItemsOfUserSession: secureStoragePreferences?.accessGroup?.migrateKeychainItems ?? false + ) } private func makeLegacyKeychainStore(service: String) -> KeychainStoreBehavior { diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin.swift index 857651ffc3..59f224dc26 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/AWSCognitoAuthPlugin.swift @@ -46,8 +46,8 @@ public final class AWSCognitoAuthPlugin: AWSCognitoAuthPluginBehavior { return "awsCognitoAuthPlugin" } - /// Instantiates an instance of the AWSCognitoAuthPlugin with optionally custom network - /// preferences and custom secure storage preferences + /// Instantiates an instance of the AWSCognitoAuthPlugin with optional custom network + /// preferences and optional custom secure storage preferences /// - Parameters: /// - networkPreferences: network preferences /// - secureStoragePreferences: secure storage preferences diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift index a5666de8d7..c6d31de198 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift @@ -35,7 +35,11 @@ struct AWSCognitoAuthCredentialStore { private let userDefaults = UserDefaults.standard private let accessGroup: String? - init(authConfiguration: AuthConfiguration, accessGroup: String? = nil, migrateKeychainItemsOfUserSession: Bool = false) { + init( + authConfiguration: AuthConfiguration, + accessGroup: String? = nil, + migrateKeychainItemsOfUserSession: Bool = false + ) { self.authConfiguration = authConfiguration self.accessGroup = accessGroup if let accessGroup { @@ -242,33 +246,14 @@ extension AWSCognitoAuthCredentialStore: AmplifyAuthCredentialStoreBehavior { return } - let oldItems: [(key: String, value: Data)] - do { - oldItems = try oldKeychain._getAll() - } catch { - log.error("[AWSCognitoAuthCredentialStore] Error getting all items from keychain under old access group, aborting migration") - return - } - - if oldItems.isEmpty { - log.verbose("[AWSCognitoAuthCredentialStore] No items in keychain under old access group, clearing keychain items under new access group") - return - } - - for item in oldItems { - do { - try keychain._set(item.value, key: item.key) - } catch { - log.error("[AWSCognitoAuthCredentialStore] Error migrating one of the items, aborting migration: \(error)") - try? clearAllCredentials() - return - } - } + let oldService = oldAccessGroup != nil ? sharedService : service + let newService = accessGroup != nil ? sharedService : service do { - try oldKeychain._removeAll() + try KeychainStoreMigrator(oldService: oldService, newService: newService, oldAccessGroup: oldAccessGroup, newAccessGroup: accessGroup).migrate() } catch { - log.error("[AWSCognitoAuthCredentialStore] Error deleting all items from keychain under old access group after migration") + log.error("[AWSCognitoAuthCredentialStore] Migration has failed") + return } log.verbose("[AWSCognitoAuthCredentialStore] Migration of keychain items from old access group to new access group successful") diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Task/AWSAuthFetchSessionTask.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Task/AWSAuthFetchSessionTask.swift index 0350f08e5a..808363e6ee 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Task/AWSAuthFetchSessionTask.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Task/AWSAuthFetchSessionTask.swift @@ -20,7 +20,12 @@ class AWSAuthFetchSessionTask: AuthFetchSessionTask, DefaultLogger { HubPayload.EventName.Auth.fetchSessionAPI } - init(_ request: AuthFetchSessionRequest, authStateMachine: AuthStateMachine, configuration: AuthConfiguration, forceReconfigure: Bool = false) { + init( + _ request: AuthFetchSessionRequest, + authStateMachine: AuthStateMachine, + configuration: AuthConfiguration, + forceReconfigure: Bool = false + ) { self.request = request self.authStateMachine = authStateMachine self.fetchAuthSessionHelper = FetchAuthSessionOperationHelper() diff --git a/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift b/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift index de534d7b67..92a91116fc 100644 --- a/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift +++ b/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift @@ -75,13 +75,16 @@ public struct KeychainStore: KeychainStoreBehavior { } public init(service: String) { - attributes = KeychainStoreAttributes(service: service) - log.verbose("[KeychainStore] Initialized keychain with service=\(service), attributes=\(attributes), accessGroup=") + self.init(service: service, accessGroup: nil) } public init(service: String, accessGroup: String? = nil) { attributes = KeychainStoreAttributes(service: service, accessGroup: accessGroup) - log.verbose("[KeychainStore] Initialized keychain with service=\(service), attributes=\(attributes), accessGroup=\(attributes.accessGroup ?? "")") + log.verbose( + "[KeychainStore] Initialized keychain with service=\(service), " + + "attributes=\(attributes), " + + "accessGroup=\(attributes.accessGroup ?? "No access group specified")" + ) } @_spi(KeychainStore) diff --git a/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreMigrator.swift b/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreMigrator.swift new file mode 100644 index 0000000000..8eb78b77d6 --- /dev/null +++ b/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreMigrator.swift @@ -0,0 +1,56 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import Foundation +import Amplify + +public struct KeychainStoreMigrator { + let oldAttributes: KeychainStoreAttributes + let newAttributes: KeychainStoreAttributes + + public init(oldService: String, newService: String, oldAccessGroup: String?, newAccessGroup: String?) { + self.oldAttributes = KeychainStoreAttributes(service: oldService, accessGroup: oldAccessGroup) + self.newAttributes = KeychainStoreAttributes(service: newService, accessGroup: newAccessGroup) + } + + public func migrate() throws { + log.verbose("[KeychainStoreMigrator] Starting to migrate items") + + var updateQuery = oldAttributes.defaultGetQuery() + + var updateAttributes = [String: Any]() + updateAttributes[KeychainStore.Constants.AttributeService] = newAttributes.service + updateAttributes[KeychainStore.Constants.AttributeAccessGroup] = newAttributes.accessGroup + + // Remove any current items to avoid duplicate item error + try? KeychainStore(service: newAttributes.service, accessGroup: newAttributes.accessGroup)._removeAll() + + let updateStatus = SecItemUpdate(updateQuery as CFDictionary, updateAttributes as CFDictionary) + switch updateStatus { + case errSecSuccess: + break + case errSecItemNotFound: + log.verbose("[KeychainStoreMigrator] No items to migrate, keychain under new access group is cleared") + case errSecDuplicateItem: + log.verbose("[KeychainStoreMigrator] Duplicate items found, could not migrate") + return + default: + log.error("[KeychainStoreMigrator] Error of status=\(updateStatus) occurred when attempting to migrate items in keychain") + throw KeychainStoreError.securityError(updateStatus) + } + + log.verbose("[KeychainStoreMigrator] Successfully migrated items to new service and access group") + } +} + +extension KeychainStoreMigrator: DefaultLogger { + public static var log: Logger { + Amplify.Logging.logger(forNamespace: String(describing: self)) + } + + public nonisolated var log: Logger { Self.log } +} From a999a544063c9c64b18405c454daf5d12394c47b Mon Sep 17 00:00:00 2001 From: aws-amplify-ops Date: Fri, 16 Aug 2024 06:05:17 +0000 Subject: [PATCH 5/7] Update API dumps for new version --- api-dump/AWSDataStorePlugin.json | 2 +- api-dump/AWSPluginsCore.json | 181 +++++++++++++++++++++++++- api-dump/Amplify.json | 2 +- api-dump/CoreMLPredictionsPlugin.json | 2 +- 4 files changed, 183 insertions(+), 4 deletions(-) diff --git a/api-dump/AWSDataStorePlugin.json b/api-dump/AWSDataStorePlugin.json index 8b0d52d144..ff0535af91 100644 --- a/api-dump/AWSDataStorePlugin.json +++ b/api-dump/AWSDataStorePlugin.json @@ -8205,7 +8205,7 @@ "-module", "AWSDataStorePlugin", "-o", - "\/var\/folders\/hw\/1f0gcr8d6kn9ms0_wn0_57qc0000gn\/T\/tmp.Op48snuvLw\/AWSDataStorePlugin.json", + "\/var\/folders\/hw\/1f0gcr8d6kn9ms0_wn0_57qc0000gn\/T\/tmp.7FvtSuhXB9\/AWSDataStorePlugin.json", "-I", ".build\/debug", "-sdk-version", diff --git a/api-dump/AWSPluginsCore.json b/api-dump/AWSPluginsCore.json index 74a86c25c0..4aade42a2d 100644 --- a/api-dump/AWSPluginsCore.json +++ b/api-dump/AWSPluginsCore.json @@ -6398,6 +6398,185 @@ } ] }, + { + "kind": "TypeDecl", + "name": "KeychainStoreMigrator", + "printedName": "KeychainStoreMigrator", + "children": [ + { + "kind": "Constructor", + "name": "init", + "printedName": "init(oldService:newService:oldAccessGroup:newAccessGroup:)", + "children": [ + { + "kind": "TypeNominal", + "name": "KeychainStoreMigrator", + "printedName": "AWSPluginsCore.KeychainStoreMigrator", + "usr": "s:14AWSPluginsCore21KeychainStoreMigratorV" + }, + { + "kind": "TypeNominal", + "name": "String", + "printedName": "Swift.String", + "usr": "s:SS" + }, + { + "kind": "TypeNominal", + "name": "String", + "printedName": "Swift.String", + "usr": "s:SS" + }, + { + "kind": "TypeNominal", + "name": "Optional", + "printedName": "Swift.String?", + "children": [ + { + "kind": "TypeNominal", + "name": "String", + "printedName": "Swift.String", + "usr": "s:SS" + } + ], + "usr": "s:Sq" + }, + { + "kind": "TypeNominal", + "name": "Optional", + "printedName": "Swift.String?", + "children": [ + { + "kind": "TypeNominal", + "name": "String", + "printedName": "Swift.String", + "usr": "s:SS" + } + ], + "usr": "s:Sq" + } + ], + "declKind": "Constructor", + "usr": "s:14AWSPluginsCore21KeychainStoreMigratorV10oldService03newG00F11AccessGroup0hiJ0ACSS_S2SSgAHtcfc", + "mangledName": "$s14AWSPluginsCore21KeychainStoreMigratorV10oldService03newG00F11AccessGroup0hiJ0ACSS_S2SSgAHtcfc", + "moduleName": "AWSPluginsCore", + "init_kind": "Designated" + }, + { + "kind": "Function", + "name": "migrate", + "printedName": "migrate()", + "children": [ + { + "kind": "TypeNominal", + "name": "Void", + "printedName": "()" + } + ], + "declKind": "Func", + "usr": "s:14AWSPluginsCore21KeychainStoreMigratorV7migrateyyKF", + "mangledName": "$s14AWSPluginsCore21KeychainStoreMigratorV7migrateyyKF", + "moduleName": "AWSPluginsCore", + "throwing": true, + "funcSelfKind": "NonMutating" + }, + { + "kind": "Var", + "name": "log", + "printedName": "log", + "children": [ + { + "kind": "TypeNominal", + "name": "Logger", + "printedName": "any Amplify.Logger", + "usr": "s:7Amplify6LoggerP" + } + ], + "declKind": "Var", + "usr": "s:14AWSPluginsCore21KeychainStoreMigratorV3log7Amplify6Logger_pvpZ", + "mangledName": "$s14AWSPluginsCore21KeychainStoreMigratorV3log7Amplify6Logger_pvpZ", + "moduleName": "AWSPluginsCore", + "static": true, + "isFromExtension": true, + "accessors": [ + { + "kind": "Accessor", + "name": "Get", + "printedName": "Get()", + "children": [ + { + "kind": "TypeNominal", + "name": "Logger", + "printedName": "any Amplify.Logger", + "usr": "s:7Amplify6LoggerP" + } + ], + "declKind": "Accessor", + "usr": "s:14AWSPluginsCore21KeychainStoreMigratorV3log7Amplify6Logger_pvgZ", + "mangledName": "$s14AWSPluginsCore21KeychainStoreMigratorV3log7Amplify6Logger_pvgZ", + "moduleName": "AWSPluginsCore", + "static": true, + "isFromExtension": true, + "accessorKind": "get" + } + ] + }, + { + "kind": "Var", + "name": "log", + "printedName": "log", + "children": [ + { + "kind": "TypeNominal", + "name": "Logger", + "printedName": "any Amplify.Logger", + "usr": "s:7Amplify6LoggerP" + } + ], + "declKind": "Var", + "usr": "s:14AWSPluginsCore21KeychainStoreMigratorV3log7Amplify6Logger_pvp", + "mangledName": "$s14AWSPluginsCore21KeychainStoreMigratorV3log7Amplify6Logger_pvp", + "moduleName": "AWSPluginsCore", + "declAttributes": [ + "Nonisolated" + ], + "isFromExtension": true, + "accessors": [ + { + "kind": "Accessor", + "name": "Get", + "printedName": "Get()", + "children": [ + { + "kind": "TypeNominal", + "name": "Logger", + "printedName": "any Amplify.Logger", + "usr": "s:7Amplify6LoggerP" + } + ], + "declKind": "Accessor", + "usr": "s:14AWSPluginsCore21KeychainStoreMigratorV3log7Amplify6Logger_pvg", + "mangledName": "$s14AWSPluginsCore21KeychainStoreMigratorV3log7Amplify6Logger_pvg", + "moduleName": "AWSPluginsCore", + "isFromExtension": true, + "accessorKind": "get" + } + ] + } + ], + "declKind": "Struct", + "usr": "s:14AWSPluginsCore21KeychainStoreMigratorV", + "mangledName": "$s14AWSPluginsCore21KeychainStoreMigratorV", + "moduleName": "AWSPluginsCore", + "conformances": [ + { + "kind": "Conformance", + "name": "DefaultLogger", + "printedName": "DefaultLogger", + "usr": "s:7Amplify13DefaultLoggerP", + "mangledName": "$s7Amplify13DefaultLoggerP" + } + ] + }, { "kind": "TypeDecl", "name": "AnyModel", @@ -24368,7 +24547,7 @@ "-module", "AWSPluginsCore", "-o", - "\/var\/folders\/hw\/1f0gcr8d6kn9ms0_wn0_57qc0000gn\/T\/tmp.Op48snuvLw\/AWSPluginsCore.json", + "\/var\/folders\/hw\/1f0gcr8d6kn9ms0_wn0_57qc0000gn\/T\/tmp.7FvtSuhXB9\/AWSPluginsCore.json", "-I", ".build\/debug", "-sdk-version", diff --git a/api-dump/Amplify.json b/api-dump/Amplify.json index a4fee7f5ad..c2ea2e6a85 100644 --- a/api-dump/Amplify.json +++ b/api-dump/Amplify.json @@ -179949,7 +179949,7 @@ "-module", "Amplify", "-o", - "\/var\/folders\/hw\/1f0gcr8d6kn9ms0_wn0_57qc0000gn\/T\/tmp.Op48snuvLw\/Amplify.json", + "\/var\/folders\/hw\/1f0gcr8d6kn9ms0_wn0_57qc0000gn\/T\/tmp.7FvtSuhXB9\/Amplify.json", "-I", ".build\/debug", "-sdk-version", diff --git a/api-dump/CoreMLPredictionsPlugin.json b/api-dump/CoreMLPredictionsPlugin.json index 1d04459209..739ae56260 100644 --- a/api-dump/CoreMLPredictionsPlugin.json +++ b/api-dump/CoreMLPredictionsPlugin.json @@ -430,7 +430,7 @@ "-module", "CoreMLPredictionsPlugin", "-o", - "\/var\/folders\/hw\/1f0gcr8d6kn9ms0_wn0_57qc0000gn\/T\/tmp.Op48snuvLw\/CoreMLPredictionsPlugin.json", + "\/var\/folders\/hw\/1f0gcr8d6kn9ms0_wn0_57qc0000gn\/T\/tmp.7FvtSuhXB9\/CoreMLPredictionsPlugin.json", "-I", ".build\/debug", "-sdk-version", From 26fb82cae8b559fa968b5eaae7be6866d8481358 Mon Sep 17 00:00:00 2001 From: Yaro Luchko Date: Wed, 21 Aug 2024 19:46:18 -0700 Subject: [PATCH 6/7] Addressing review comments: documentation, no more credentials valid check, only delete items if absolutely necessary --- .../Categories/Auth/Models/AccessGroup.swift | 23 ++++++++- .../AWSCognitoAuthCredentialStore.swift | 48 +++++-------------- .../AWSCognitoSecureStoragePreferences.swift | 4 ++ .../MockCredentialStoreBehavior.swift | 5 -- .../Support/DefaultConfig.swift | 4 -- .../Keychain/KeychainStore.swift | 46 ------------------ .../Keychain/KeychainStoreMigrator.swift | 20 ++++---- .../Mocks/MockKeychainStore.swift | 14 ------ 8 files changed, 49 insertions(+), 115 deletions(-) diff --git a/Amplify/Categories/Auth/Models/AccessGroup.swift b/Amplify/Categories/Auth/Models/AccessGroup.swift index 27fa47d561..5842598910 100644 --- a/Amplify/Categories/Auth/Models/AccessGroup.swift +++ b/Amplify/Categories/Auth/Models/AccessGroup.swift @@ -7,22 +7,43 @@ import Foundation +/// A structure representing an access group for managing keychain items. public struct AccessGroup { + /// The name of the access group. public let name: String? + + /// A flag indicating whether to migrate keychain items. public let migrateKeychainItems: Bool + /** + Initializes an `AccessGroup` with the specified name and migration option. + + - Parameter name: The name of the access group. + - Parameter migrateKeychainItemsOfUserSession: A flag indicating whether to migrate keychain items. Defaults to `false`. + */ public init(name: String, migrateKeychainItemsOfUserSession: Bool = false) { self.init(name: name, migrateKeychainItems: migrateKeychainItemsOfUserSession) } + /** + Creates an `AccessGroup` instance with no specified name. + + - Parameter migrateKeychainItemsOfUserSession: A flag indicating whether to migrate keychain items. + - Returns: An `AccessGroup` instance with the migration option set. + */ public static func none(migrateKeychainItemsOfUserSession: Bool) -> AccessGroup { return .init(migrateKeychainItems: migrateKeychainItemsOfUserSession) } + /** + A static property representing an `AccessGroup` with no name and no migration. + + - Returns: An `AccessGroup` instance with no name and the migration option set to `false`. + */ public static var none: AccessGroup { return .none(migrateKeychainItemsOfUserSession: false) } - + private init(name: String? = nil, migrateKeychainItems: Bool) { self.name = name self.migrateKeychainItems = migrateKeychainItems diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift index c6d31de198..49ffbbc81e 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift @@ -26,6 +26,10 @@ struct AWSCognitoAuthCredentialStore { private var isKeychainConfiguredKey: String { "\(userDefaultsNameSpace).isKeychainConfigured" } + /// This UserDefaults Key is use to retrieve the stored access group to determine + /// which access group the migration should happen from + /// If none is found, the unshared service is used for migration and all items + /// under that service are queried private var accessGroupKey: String { "\(userDefaultsNameSpace).accessGroup" } @@ -48,11 +52,14 @@ struct AWSCognitoAuthCredentialStore { self.keychain = KeychainStore(service: service) } + let oldAccessGroup = retrieveStoredAccessGroup() if migrateKeychainItemsOfUserSession { try? migrateKeychainItemsToAccessGroup() + } else if oldAccessGroup == nil && oldAccessGroup != accessGroup{ + try? KeychainStore(service: service)._removeAll() } - try? saveStoredAccessGroup() + saveStoredAccessGroup() if !userDefaults.bool(forKey: isKeychainConfiguredKey) { try? clearAllCredentials() @@ -203,11 +210,11 @@ extension AWSCognitoAuthCredentialStore: AmplifyAuthCredentialStoreBehavior { try keychain._removeAll() } - private func retrieveStoredAccessGroup() throws -> String? { + private func retrieveStoredAccessGroup() -> String? { return userDefaults.string(forKey: accessGroupKey) } - private func saveStoredAccessGroup() throws { + private func saveStoredAccessGroup() { if let accessGroup { userDefaults.set(accessGroup, forKey: accessGroupKey) } else { @@ -216,33 +223,10 @@ extension AWSCognitoAuthCredentialStore: AmplifyAuthCredentialStoreBehavior { } private func migrateKeychainItemsToAccessGroup() throws { - let oldAccessGroup = try? retrieveStoredAccessGroup() - let oldKeychain: KeychainStoreBehavior + let oldAccessGroup = retrieveStoredAccessGroup() if oldAccessGroup == accessGroup { - log.verbose("[AWSCognitoAuthCredentialStore] Stored access group is the same as current access group, aborting migration") - return - } - - if let oldAccessGroup { - oldKeychain = KeychainStore(service: sharedService, accessGroup: oldAccessGroup) - } else { - oldKeychain = KeychainStore(service: service) - } - - let authCredentialStoreKey = generateSessionKey(for: authConfiguration) - let authCredentialData: Data - let awsCredential: AmplifyCredentials - do { - authCredentialData = try oldKeychain._getData(authCredentialStoreKey) - awsCredential = try decode(data: authCredentialData) - } catch { - log.verbose("[AWSCognitoAuthCredentialStore] Could not retrieve previous credentials in keychain under old access group, nothing to migrate") - return - } - - guard awsCredential.areValid() else { - log.verbose("[AWSCognitoAuthCredentialStore] Credentials found are not valid (expired) in old access group keychain, aborting migration") + log.info("[AWSCognitoAuthCredentialStore] Stored access group is the same as current access group, aborting migration") return } @@ -282,10 +266,4 @@ private extension AWSCognitoAuthCredentialStore { } -extension AWSCognitoAuthCredentialStore: DefaultLogger { - public static var log: Logger { - Amplify.Logging.logger(forNamespace: String(describing: self)) - } - - public nonisolated var log: Logger { Self.log } -} +extension AWSCognitoAuthCredentialStore: DefaultLogger { } diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/AWSCognitoSecureStoragePreferences.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/AWSCognitoSecureStoragePreferences.swift index c7dcefe42b..3ee17a804c 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/AWSCognitoSecureStoragePreferences.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/AWSCognitoSecureStoragePreferences.swift @@ -8,11 +8,15 @@ import Foundation import Amplify +/// A struct to store preferences for how the plugin uses storage public struct AWSCognitoSecureStoragePreferences { /// The access group that the keychain will use for auth items public let accessGroup: AccessGroup? + /// Creates an intstance of AWSCognitoSecureStoragePreferences + /// - Parameters: + /// - accessGroup: access group to be used public init(accessGroup: AccessGroup? = nil) { self.accessGroup = accessGroup } diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/CredentialStore/MockCredentialStoreBehavior.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/CredentialStore/MockCredentialStoreBehavior.swift index 1f00f81df3..d0ca03f00b 100644 --- a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/CredentialStore/MockCredentialStoreBehavior.swift +++ b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/CredentialStore/MockCredentialStoreBehavior.swift @@ -15,7 +15,6 @@ class MockKeychainStoreBehavior: KeychainStoreBehavior { typealias VoidHandler = () -> Void let data: String - let allData: [(key: String, value: Data)] let removeAllHandler: VoidHandler? let mockKey: String = "mockKey" @@ -23,7 +22,6 @@ class MockKeychainStoreBehavior: KeychainStoreBehavior { removeAllHandler: VoidHandler? = nil) { self.data = data self.removeAllHandler = removeAllHandler - self.allData = [(key: mockKey, value: Data(data.utf8))] } func _getString(_ key: String) throws -> String { @@ -45,7 +43,4 @@ class MockKeychainStoreBehavior: KeychainStoreBehavior { removeAllHandler?() } - func _getAll() throws -> [(key: String, value: Data)] { - return allData - } } diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Support/DefaultConfig.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Support/DefaultConfig.swift index ef876129ae..13e5a8781e 100644 --- a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Support/DefaultConfig.swift +++ b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Support/DefaultConfig.swift @@ -366,10 +366,6 @@ struct MockLegacyStore: KeychainStoreBehavior { func _removeAll() throws { } - - func _getAll() throws -> [(key: String, value: Data)] { - return [] - } } diff --git a/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift b/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift index 92a91116fc..e14e0a856e 100644 --- a/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift +++ b/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift @@ -53,10 +53,6 @@ public protocol KeychainStoreBehavior { /// This System Programming Interface (SPI) may have breaking changes in future updates. func _removeAll() throws - @_spi(KeychainStore) - /// Retrieves all key-value pairs in the keychain - /// This System Programming Interface (SPI) may have breaking changes in future updates. - func _getAll() throws -> [(key: String, value: Data)] } public struct KeychainStore: KeychainStoreBehavior { @@ -236,48 +232,6 @@ public struct KeychainStore: KeychainStoreBehavior { } log.verbose("[KeychainStore] Successfully removed all items from keychain") } - - @_spi(KeychainStore) - /// Retrieves all key-value pairs in the keychain - /// This System Programming Interface (SPI) may have breaking changes in future updates. - public func _getAll() throws -> [(key: String, value: Data)] { - log.verbose("[KeychainStore] Starting to retrieve all items from keychain") - var query = attributes.defaultGetQuery() - query[Constants.MatchLimit] = Constants.MatchLimitAll - query[Constants.ReturnData] = kCFBooleanTrue - query[Constants.ReturnAttributes] = kCFBooleanTrue - query[Constants.ReturnRef] = kCFBooleanTrue - - var result: AnyObject? - let status = SecItemCopyMatching(query as CFDictionary, &result) - - switch status { - case errSecSuccess: - guard let items = result as? [[String: Any]] else { - log.error("[KeychainStore] The keychain items retrieved are not the correct type") - throw KeychainStoreError.unknown("The keychain items retrieved are not the correct type") - } - - var keyValuePairs = [(key: String, value: Data)]() - for item in items { - guard let key = item[Constants.AttributeAccount] as? String, - let value = item[Constants.ValueData] as? Data else { - log.error("[KeychainStore] Unable to retrieve key or value from keychain item") - continue - } - keyValuePairs.append((key: key, value: value)) - } - - log.verbose("[KeychainStore] Successfully retrieved \(keyValuePairs.count) items from keychain") - return keyValuePairs - case errSecItemNotFound: - log.verbose("[KeychainStore] No items found in keychain") - return [] - default: - log.error("[KeychainStore] Error of status=\(status) occurred when attempting to retrieve all items from keychain") - throw KeychainStoreError.securityError(status) - } - } } diff --git a/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreMigrator.swift b/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreMigrator.swift index 8eb78b77d6..580366c519 100644 --- a/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreMigrator.swift +++ b/AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreMigrator.swift @@ -20,15 +20,21 @@ public struct KeychainStoreMigrator { public func migrate() throws { log.verbose("[KeychainStoreMigrator] Starting to migrate items") + // Check if there are any existing items under the new service and access group + let existingItemsQuery = newAttributes.defaultGetQuery() + let existingItemsStatus = SecItemCopyMatching(existingItemsQuery as CFDictionary, nil) + + if existingItemsStatus == errSecSuccess { + // Remove existing items to avoid duplicate item error + try? KeychainStore(service: newAttributes.service, accessGroup: newAttributes.accessGroup)._removeAll() + } + var updateQuery = oldAttributes.defaultGetQuery() var updateAttributes = [String: Any]() updateAttributes[KeychainStore.Constants.AttributeService] = newAttributes.service updateAttributes[KeychainStore.Constants.AttributeAccessGroup] = newAttributes.accessGroup - // Remove any current items to avoid duplicate item error - try? KeychainStore(service: newAttributes.service, accessGroup: newAttributes.accessGroup)._removeAll() - let updateStatus = SecItemUpdate(updateQuery as CFDictionary, updateAttributes as CFDictionary) switch updateStatus { case errSecSuccess: @@ -47,10 +53,4 @@ public struct KeychainStoreMigrator { } } -extension KeychainStoreMigrator: DefaultLogger { - public static var log: Logger { - Amplify.Logging.logger(forNamespace: String(describing: self)) - } - - public nonisolated var log: Logger { Self.log } -} +extension KeychainStoreMigrator: DefaultLogger { } diff --git a/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockKeychainStore.swift b/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockKeychainStore.swift index 3596ce8241..e2c127588e 100644 --- a/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockKeychainStore.swift +++ b/AmplifyPlugins/Internal/Tests/InternalAWSPinpointUnitTests/Mocks/MockKeychainStore.swift @@ -61,20 +61,6 @@ class MockKeychainStore: KeychainStoreBehavior { stringValues.removeAll() dataValues.removeAll() } - - func _getAll() throws -> [(key: String, value: Data)] { - var allValues: [(key: String, value: Data)] = [] - - for (key, value) in dataValues { - allValues.append((key: key, value: value)) - } - - for (key, value) in stringValues { - allValues.append((key: key, value: value.data(using: .utf8)!)) - } - - return allValues - } func resetCounters() { dataForKeyCount = 0 From 01e22685a69366b5547961985fa1e31e1b5d6044 Mon Sep 17 00:00:00 2001 From: Yaro Luchko Date: Wed, 21 Aug 2024 19:51:45 -0700 Subject: [PATCH 7/7] Style fixes --- .../CredentialStorage/AWSCognitoAuthCredentialStore.swift | 2 +- .../CredentialStore/MockCredentialStoreBehavior.swift | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift index 49ffbbc81e..795f1ce38e 100644 --- a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift +++ b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AWSCognitoAuthCredentialStore.swift @@ -55,7 +55,7 @@ struct AWSCognitoAuthCredentialStore { let oldAccessGroup = retrieveStoredAccessGroup() if migrateKeychainItemsOfUserSession { try? migrateKeychainItemsToAccessGroup() - } else if oldAccessGroup == nil && oldAccessGroup != accessGroup{ + } else if oldAccessGroup == nil && oldAccessGroup != accessGroup { try? KeychainStore(service: service)._removeAll() } diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/CredentialStore/MockCredentialStoreBehavior.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/CredentialStore/MockCredentialStoreBehavior.swift index d0ca03f00b..97b9201818 100644 --- a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/CredentialStore/MockCredentialStoreBehavior.swift +++ b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/CredentialStore/MockCredentialStoreBehavior.swift @@ -16,7 +16,6 @@ class MockKeychainStoreBehavior: KeychainStoreBehavior { let data: String let removeAllHandler: VoidHandler? - let mockKey: String = "mockKey" init(data: String, removeAllHandler: VoidHandler? = nil) { @@ -42,5 +41,4 @@ class MockKeychainStoreBehavior: KeychainStoreBehavior { func _removeAll() throws { removeAllHandler?() } - }