fix(Auth): Fixing keychain migration issue (#2978)

* fix(Auth): Fixing keychain migration issue

* adding integration tests

* worked on review comments

Author: harsh62

AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/Authorization/AuthorizationState+Resolver.swift

@@ -247,6 +247,10 @@ extension AuthorizationState {
                 if case .sessionEstablished(let credentials) = event.isAuthorizationEvent {
                     return .init(newState: .sessionEstablished(credentials))
                 }
+                
+                if case .throwError(let error) = event.isAuthorizationEvent {
+                    return .init(newState: .error(error))
+                }
                 return .from(oldState)
 
             case .error(let error):

AmplifyPlugins/Auth/Tests/AuthHostApp/AuthHostApp.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

@@ -235,4 +235,36 @@ class SignedInAuthSessionTests: AWSAuthBaseTest {
         }
         await waitForExpectations(timeout: networkTimeout)
     }
+
+    /// Test if successful session is retrieved before and after a user sign in
+    ///
+    /// - Given: A signed out Amplify Auth Category
+    /// - When:
+    ///    - I call fetchAuthSession
+    /// - Then:
+    ///    - I get a signed out session
+    /// - After that When:
+    ///    - I sign in to the Auth Category
+    /// - Then:
+    ///    - I should receive a valid session in signed in state
+    /// - After that When:
+    ///    - I call fetchAuthSession
+    /// - Then:
+    ///    - I should receive a valid session in signed in state
+    func testSuccessfulSessionFetchAndSignIn() async throws {
+        let username = "integTest\(UUID().uuidString)"
+        let password = "P123@\(UUID().uuidString)"
+
+        var session = try await Amplify.Auth.fetchAuthSession()
+        XCTAssertFalse(session.isSignedIn, "Session state should be signed out")
+
+        let didSucceed = try await AuthSignInHelper.registerAndSignInUser(
+            username: username,
+            password: password,
+            email: defaultTestEmail)
+        XCTAssertTrue(didSucceed, "SignIn operation failed")
+
+        session = try await Amplify.Auth.fetchAuthSession()
+        XCTAssertTrue(session.isSignedIn, "Session state should be signed In")
+    }
 }

AmplifyPlugins/Auth/Tests/AuthHostApp/AuthIntegrationTests/SessionTests/SignedInAuthSessionTests.swift

@@ -99,7 +99,7 @@ public struct KeychainStore: KeychainStoreBehavior {
     /// - Parameter key: A String key use to look up the value in the Keychain
     /// - Returns: A data value
     public func _getData(_ key: String) throws -> Data {
-        var query = attributes.query()
+        var query = attributes.defaultGetQuery()
 
         query[Constants.MatchLimit] = Constants.MatchLimitOne
         query[Constants.ReturnData] = kCFBooleanTrue
@@ -142,24 +142,26 @@ public struct KeychainStore: KeychainStoreBehavior {
     ///   - value: A data value to store in Keychain
     ///   - key: A String key for the value to store in the Keychain
     public func _set(_ value: Data, key: String) throws {
-        var query = attributes.query()
-        query[Constants.AttributeAccount] = key
+        var getQuery = attributes.defaultGetQuery()
+        getQuery[Constants.AttributeAccount] = key
 
-        let fetchStatus = SecItemCopyMatching(query as CFDictionary, nil)
+        let fetchStatus = SecItemCopyMatching(getQuery as CFDictionary, nil)
         switch fetchStatus {
         case errSecSuccess:
 
             var attributesToUpdate = [String: Any]()
             attributesToUpdate[Constants.ValueData] = value
 
-            let updateStatus = SecItemUpdate(query as CFDictionary, attributesToUpdate as CFDictionary)
+            let updateStatus = SecItemUpdate(getQuery as CFDictionary, attributesToUpdate as CFDictionary)
             if updateStatus != errSecSuccess {
                 throw KeychainStoreError.securityError(updateStatus)
             }
         case errSecItemNotFound:
-            let attributes = attributes.fetchAll(for: key, and: value)
+            var attributesToSet = attributes.defaultSetQuery()
+            attributesToSet[Constants.AttributeAccount] = key
+            attributesToSet[Constants.ValueData] = value
 
-            let addStatus = SecItemAdd(attributes as CFDictionary, nil)
+            let addStatus = SecItemAdd(attributesToSet as CFDictionary, nil)
             if addStatus != errSecSuccess {
                 throw KeychainStoreError.securityError(addStatus)
             }
@@ -173,7 +175,7 @@ public struct KeychainStore: KeychainStoreBehavior {
     /// This System Programming Interface (SPI) may have breaking changes in future updates.
     /// - Parameter key: A String key to delete the key-value pair
     public func _remove(_ key: String) throws {
-        var query = attributes.query()
+        var query = attributes.defaultGetQuery()
         query[Constants.AttributeAccount] = key
 
         let status = SecItemDelete(query as CFDictionary)
@@ -186,7 +188,7 @@ public struct KeychainStore: KeychainStoreBehavior {
     /// Removes all key-value pair in the Keychain.
     /// This System Programming Interface (SPI) may have breaking changes in future updates.
     public func _removeAll() throws {
-        var query = attributes.query()
+        var query = attributes.defaultGetQuery()
 #if !os(iOS) && !os(watchOS) && !os(tvOS)
         query[Constants.MatchLimit] = Constants.MatchLimitAll
 #endif

AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift

@@ -12,47 +12,26 @@ struct KeychainStoreAttributes {
     var itemClass: String = KeychainStore.Constants.ClassGenericPassword
     var service: String
     var accessGroup: String?
-
-    var label: String?
-    var comment: String?
+    
 }
 
 extension KeychainStoreAttributes {
 
-    func query() -> [String: Any] {
+    func defaultGetQuery() -> [String: Any] {
         var query: [String: Any] = [
             KeychainStore.Constants.Class: itemClass,
             KeychainStore.Constants.AttributeService: service
         ]
         if let accessGroup = accessGroup {
             query[KeychainStore.Constants.AttributeAccessGroup] = accessGroup
         }
-        query[KeychainStore.Constants.AttributeAccessible] = KeychainStore.Constants.AttributeAccessibleAfterFirstUnlockThisDeviceOnly
-        query[KeychainStore.Constants.UseDataProtectionKeyChain] = kCFBooleanTrue
         return query
     }
 
-    func fetchAll(for key: String?, and value: Data) -> [String: Any] {
-
-        var attributes: [String: Any]
-
-        if key != nil {
-            attributes = query()
-            attributes[KeychainStore.Constants.AttributeAccount] = key
-        } else {
-            attributes = [String: Any]()
-        }
-
-        attributes[KeychainStore.Constants.ValueData] = value
-
-        if label != nil {
-            attributes[KeychainStore.Constants.AttributeLabel] = label
-        }
-        if comment != nil {
-            attributes[KeychainStore.Constants.AttributeComment] = comment
-        }
-
-        return attributes
-
+    func defaultSetQuery() -> [String: Any] {
+        var query: [String: Any] = defaultGetQuery()
+        query[KeychainStore.Constants.AttributeAccessible] = KeychainStore.Constants.AttributeAccessibleAfterFirstUnlockThisDeviceOnly
+        query[KeychainStore.Constants.UseDataProtectionKeyChain] = kCFBooleanTrue
+        return query
     }
 }

AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreAttributes.swift

@@ -0,0 +1,71 @@
+//
+// Copyright Amazon.com Inc. or its affiliates.
+// All Rights Reserved.
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+import XCTest
+@testable import Amplify
+@testable import AWSPluginsCore
+
+class KeychainStoreAttributesTests: XCTestCase {
+
+    var keychainStoreAttribute: KeychainStoreAttributes!
+
+    /// Given: an instance of `KeychainStoreAttributes`
+    /// When: `keychainStoreAttribute.defaultGetQuery()` is invoked with a required service param
+    /// Then: Validate if the attributes contain the correct get query params
+    ///     - AttributeService
+    ///     - Class
+    func testDefaultGetQuery() {
+        keychainStoreAttribute = KeychainStoreAttributes(service: "someService")
+
+        let defaultGetAttributes = keychainStoreAttribute.defaultGetQuery()
+        XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.AttributeService] as? String, "someService")
+        XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.Class] as? String, KeychainStore.Constants.ClassGenericPassword)
+        XCTAssertNil(defaultGetAttributes[KeychainStore.Constants.AttributeAccessGroup] as? String)
+        XCTAssertNil(defaultGetAttributes[KeychainStore.Constants.AttributeAccessible] as? String)
+        XCTAssertNil(defaultGetAttributes[KeychainStore.Constants.UseDataProtectionKeyChain] as? String)
+    }
+
+    /// Given: an instance of `KeychainStoreAttributes`
+    /// When: `keychainStoreAttribute.defaultGetQuery()` is invoked with a required service param and access group
+    /// Then: Validate if the attributes contain the correct get query params
+    ///     - AttributeService
+    ///     - Class
+    ///     - AttributeAccessGroup
+    func testDefaultGetQueryWithAccessGroup() {
+        keychainStoreAttribute = KeychainStoreAttributes(service: "someService", accessGroup: "someAccessGroup")
+
+        let defaultGetAttributes = keychainStoreAttribute.defaultGetQuery()
+        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")
+        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 and access group
+    /// Then: Validate if the attributes contain the correct get query params
+    ///     - AttributeService
+    ///     - Class
+    ///     - AttributeAccessGroup
+    ///     - AttributeAccessible
+    ///     - UseDataProtectionKeyChain
+    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)
+    }
+
+    override func tearDown() {
+        keychainStoreAttribute = nil
+    }
+}