Skip to content

Commit 4f761b7

Browse files
harsh62harsh62
authored
fix(Auth): Use correct queries when getting and setting keychain items (#2965)
1 parent 4241439 commit 4f761b7

File tree

3 files changed

+82
-6
lines changed

3 files changed

+82
-6
lines changed

‎AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStore.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ public struct KeychainStore: KeychainStoreBehavior {
9999
/// - Parameter key: A String key use to look up the value in the Keychain
100100
/// - Returns: A data value
101101
public func _getData(_ key: String) throws -> Data {
102-
var query = attributes.query()
102+
var query = attributes.defaultGetQuery()
103103

104104
query[Constants.MatchLimit] = Constants.MatchLimitOne
105105
query[Constants.ReturnData] = kCFBooleanTrue
@@ -142,7 +142,7 @@ public struct KeychainStore: KeychainStoreBehavior {
142142
/// - value: A data value to store in Keychain
143143
/// - key: A String key for the value to store in the Keychain
144144
public func _set(_ value: Data, key: String) throws {
145-
var query = attributes.query()
145+
var query = attributes.defaultSetQuery()
146146
query[Constants.AttributeAccount] = key
147147

148148
let fetchStatus = SecItemCopyMatching(query as CFDictionary, nil)
@@ -173,7 +173,7 @@ public struct KeychainStore: KeychainStoreBehavior {
173173
/// This System Programming Interface (SPI) may have breaking changes in future updates.
174174
/// - Parameter key: A String key to delete the key-value pair
175175
public func _remove(_ key: String) throws {
176-
var query = attributes.query()
176+
var query = attributes.defaultGetQuery()
177177
query[Constants.AttributeAccount] = key
178178

179179
let status = SecItemDelete(query as CFDictionary)
@@ -186,7 +186,7 @@ public struct KeychainStore: KeychainStoreBehavior {
186186
/// Removes all key-value pair in the Keychain.
187187
/// This System Programming Interface (SPI) may have breaking changes in future updates.
188188
public func _removeAll() throws {
189-
var query = attributes.query()
189+
var query = attributes.defaultGetQuery()
190190
#if !os(iOS) && !os(watchOS) && !os(tvOS)
191191
query[Constants.MatchLimit] = Constants.MatchLimitAll
192192
#endif

‎AmplifyPlugins/Core/AWSPluginsCore/Keychain/KeychainStoreAttributes.swift‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,19 @@ struct KeychainStoreAttributes {
1919

2020
extension KeychainStoreAttributes {
2121

22-
func query() -> [String: Any] {
22+
func defaultGetQuery() -> [String: Any] {
2323
var query: [String: Any] = [
2424
KeychainStore.Constants.Class: itemClass,
2525
KeychainStore.Constants.AttributeService: service
2626
]
2727
if let accessGroup = accessGroup {
2828
query[KeychainStore.Constants.AttributeAccessGroup] = accessGroup
2929
}
30+
return query
31+
}
32+
33+
func defaultSetQuery() -> [String: Any] {
34+
var query: [String: Any] = defaultGetQuery()
3035
query[KeychainStore.Constants.AttributeAccessible] = KeychainStore.Constants.AttributeAccessibleAfterFirstUnlockThisDeviceOnly
3136
query[KeychainStore.Constants.UseDataProtectionKeyChain] = kCFBooleanTrue
3237
return query
@@ -37,7 +42,7 @@ extension KeychainStoreAttributes {
3742
var attributes: [String: Any]
3843

3944
if key != nil {
40-
attributes = query()
45+
attributes = defaultGetQuery()
4146
attributes[KeychainStore.Constants.AttributeAccount] = key
4247
} else {
4348
attributes = [String: Any]()

‎AmplifyPlugins/Core/AWSPluginsCoreTests/Keychain/KeychainStoreAttributesTests.swift‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
//
2+
// Copyright Amazon.com Inc. or its affiliates.
3+
// All Rights Reserved.
4+
//
5+
// SPDX-License-Identifier: Apache-2.0
6+
//
7+
8+
import XCTest
9+
@testable import Amplify
10+
@testable import AWSPluginsCore
11+
12+
class KeychainStoreAttributesTests: XCTestCase {
13+
14+
var keychainStoreAttribute: KeychainStoreAttributes!
15+
16+
/// Given: an instance of `KeychainStoreAttributes`
17+
/// When: `keychainStoreAttribute.defaultGetQuery()` is invoked with a required service param
18+
/// Then: Validate if the attributes contain the correct get query params
19+
/// - AttributeService
20+
/// - Class
21+
func testDefaultGetQuery() {
22+
keychainStoreAttribute = KeychainStoreAttributes(service: "someService")
23+
24+
let defaultGetAttributes = keychainStoreAttribute.defaultGetQuery()
25+
XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.AttributeService] as? String, "someService")
26+
XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.Class] as? String, KeychainStore.Constants.ClassGenericPassword)
27+
XCTAssertNil(defaultGetAttributes[KeychainStore.Constants.AttributeAccessGroup] as? String)
28+
XCTAssertNil(defaultGetAttributes[KeychainStore.Constants.AttributeAccessible] as? String)
29+
XCTAssertNil(defaultGetAttributes[KeychainStore.Constants.UseDataProtectionKeyChain] as? String)
30+
}
31+
32+
/// Given: an instance of `KeychainStoreAttributes`
33+
/// When: `keychainStoreAttribute.defaultGetQuery()` is invoked with a required service param and access group
34+
/// Then: Validate if the attributes contain the correct get query params
35+
/// - AttributeService
36+
/// - Class
37+
/// - AttributeAccessGroup
38+
func testDefaultGetQueryWithAccessGroup() {
39+
keychainStoreAttribute = KeychainStoreAttributes(service: "someService", accessGroup: "someAccessGroup")
40+
41+
let defaultGetAttributes = keychainStoreAttribute.defaultGetQuery()
42+
XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.AttributeService] as? String, "someService")
43+
XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.Class] as? String, KeychainStore.Constants.ClassGenericPassword)
44+
XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.AttributeAccessGroup] as? String, "someAccessGroup")
45+
XCTAssertNil(defaultGetAttributes[KeychainStore.Constants.AttributeAccessible] as? String)
46+
XCTAssertNil(defaultGetAttributes[KeychainStore.Constants.UseDataProtectionKeyChain] as? String)
47+
}
48+
49+
/// Given: an instance of `KeychainStoreAttributes`
50+
/// When: `keychainStoreAttribute.defaultSetQuery()` is invoked with a required service param and access group
51+
/// Then: Validate if the attributes contain the correct get query params
52+
/// - AttributeService
53+
/// - Class
54+
/// - AttributeAccessGroup
55+
/// - AttributeAccessible
56+
/// - UseDataProtectionKeyChain
57+
func testDefaultSetQueryWithAccessGroup() {
58+
keychainStoreAttribute = KeychainStoreAttributes(service: "someService", accessGroup: "someAccessGroup")
59+
60+
let defaultGetAttributes = keychainStoreAttribute.defaultSetQuery()
61+
XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.AttributeService] as? String, "someService")
62+
XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.Class] as? String, KeychainStore.Constants.ClassGenericPassword)
63+
XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.AttributeAccessGroup] as? String, "someAccessGroup")
64+
XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.AttributeAccessible] as? String, KeychainStore.Constants.AttributeAccessibleAfterFirstUnlockThisDeviceOnly)
65+
XCTAssertEqual(defaultGetAttributes[KeychainStore.Constants.UseDataProtectionKeyChain] as? Bool, true)
66+
}
67+
68+
override func tearDown() {
69+
keychainStoreAttribute = nil
70+
}
71+
}

0 commit comments

Comments
 (0)