From d35c6e1be0d88e1a46e12678e3854179ff615fad Mon Sep 17 00:00:00 2001 From: Richard Topchii Date: Fri, 14 Mar 2025 20:00:14 +0200 Subject: [PATCH 1/2] Remove "internal" access modifiers from code #339 --- Sources/Valet/Internal/Configuration.swift | 8 +++--- Sources/Valet/Internal/Keychain.swift | 26 +++++++++---------- Sources/Valet/Internal/SecItem.swift | 14 +++++----- Sources/Valet/Internal/Service.swift | 14 +++++----- Sources/Valet/Internal/WeakStorage.swift | 4 +-- Sources/Valet/MigratableKeyValuePair.swift | 4 +-- Sources/Valet/SecureEnclave.swift | 16 ++++++------ .../Valet/SecureEnclaveAccessControl.swift | 4 +-- Sources/Valet/SecureEnclaveValet.swift | 2 +- Sources/Valet/SharedGroupIdentifier.swift | 6 ++--- .../SinglePromptSecureEnclaveValet.swift | 2 +- Sources/Valet/Valet.swift | 8 +++--- .../ValetIntegrationTests.swift | 2 +- 13 files changed, 55 insertions(+), 55 deletions(-) diff --git a/Sources/Valet/Internal/Configuration.swift b/Sources/Valet/Internal/Configuration.swift index 3355f4c6..395de24b 100644 --- a/Sources/Valet/Internal/Configuration.swift +++ b/Sources/Valet/Internal/Configuration.swift @@ -17,7 +17,7 @@ import Foundation -internal enum Configuration: CustomStringConvertible, Sendable { +enum Configuration: CustomStringConvertible, Sendable { case valet(Accessibility) case iCloud(CloudAccessibility) case secureEnclave(SecureEnclaveAccessControl) @@ -25,7 +25,7 @@ internal enum Configuration: CustomStringConvertible, Sendable { // MARK: CustomStringConvertible - internal var description: String { + var description: String { switch self { case .valet: return "VALValet" @@ -40,7 +40,7 @@ internal enum Configuration: CustomStringConvertible, Sendable { // MARK: Internal Properties - internal var accessibility: Accessibility { + var accessibility: Accessibility { switch self { case let .valet(accessibility): return accessibility @@ -51,7 +51,7 @@ internal enum Configuration: CustomStringConvertible, Sendable { } } - internal var prettyDescription: String { + var prettyDescription: String { let configurationDescription: String = { switch self { case .valet: diff --git a/Sources/Valet/Internal/Keychain.swift b/Sources/Valet/Internal/Keychain.swift index f35b24d2..b6011d68 100644 --- a/Sources/Valet/Internal/Keychain.swift +++ b/Sources/Valet/Internal/Keychain.swift @@ -17,7 +17,7 @@ import Foundation -internal final class Keychain { +final class Keychain { // MARK: Private Static Properties @@ -26,7 +26,7 @@ internal final class Keychain { // MARK: Keychain Accessibility - internal static func canAccess(attributes: [String : AnyHashable]) -> Bool { + static func canAccess(attributes: [String : AnyHashable]) -> Bool { func isCanaryValueInKeychain() -> Bool { do { let retrievedCanaryValue = try string(forKey: canaryKey, options: attributes) @@ -52,7 +52,7 @@ internal final class Keychain { // MARK: Getters - internal static func string(forKey key: String, options: [String : AnyHashable]) throws(KeychainError) -> String { + static func string(forKey key: String, options: [String : AnyHashable]) throws(KeychainError) -> String { let data = try object(forKey: key, options: options) if let string = String(data: data, encoding: .utf8) { return string @@ -61,7 +61,7 @@ internal final class Keychain { } } - internal static func object(forKey key: String, options: [String : AnyHashable]) throws(KeychainError) -> Data { + static func object(forKey key: String, options: [String : AnyHashable]) throws(KeychainError) -> Data { guard !key.isEmpty else { throw KeychainError.emptyKey } @@ -76,12 +76,12 @@ internal final class Keychain { // MARK: Setters - internal static func setString(_ string: String, forKey key: String, options: [String: AnyHashable]) throws(KeychainError) { + static func setString(_ string: String, forKey key: String, options: [String: AnyHashable]) throws(KeychainError) { let data = Data(string.utf8) try setObject(data, forKey: key, options: options) } - internal static func setObject(_ object: Data, forKey key: String, options: [String: AnyHashable]) throws(KeychainError) { + static func setObject(_ object: Data, forKey key: String, options: [String: AnyHashable]) throws(KeychainError) { guard !key.isEmpty else { throw KeychainError.emptyKey } @@ -110,7 +110,7 @@ internal final class Keychain { // MARK: Removal - internal static func removeObject(forKey key: String, options: [String : AnyHashable]) throws(KeychainError) { + static func removeObject(forKey key: String, options: [String : AnyHashable]) throws(KeychainError) { guard !key.isEmpty else { throw KeychainError.emptyKey } @@ -121,13 +121,13 @@ internal final class Keychain { try SecItem.deleteItems(matching: secItemQuery) } - internal static func removeAllObjects(matching options: [String : AnyHashable]) throws(KeychainError) { + static func removeAllObjects(matching options: [String : AnyHashable]) throws(KeychainError) { try SecItem.deleteItems(matching: options) } // MARK: Contains - internal static func performCopy(forKey key: String, options: [String : AnyHashable]) -> OSStatus { + static func performCopy(forKey key: String, options: [String : AnyHashable]) -> OSStatus { guard !key.isEmpty else { return errSecParam } @@ -140,7 +140,7 @@ internal final class Keychain { // MARK: AllObjects - internal static func allKeys(options: [String: AnyHashable]) throws(KeychainError) -> Set { + static func allKeys(options: [String: AnyHashable]) throws(KeychainError) -> Set { var secItemQuery = options secItemQuery[kSecMatchLimit as String] = kSecMatchLimitAll secItemQuery[kSecReturnAttributes as String] = true @@ -171,7 +171,7 @@ internal final class Keychain { // MARK: Migration - internal static func migrateObjects(matching query: [String : AnyHashable], into destinationAttributes: [String : AnyHashable], compactMap: (MigratableKeyValuePair) throws -> MigratableKeyValuePair?) throws { + static func migrateObjects(matching query: [String : AnyHashable], into destinationAttributes: [String : AnyHashable], compactMap: (MigratableKeyValuePair) throws -> MigratableKeyValuePair?) throws { guard !query.isEmpty else { // Migration requires secItemQuery to contain values. throw MigrationError.invalidQuery @@ -317,7 +317,7 @@ internal final class Keychain { } } - internal static func migrateObjects(matching query: [String : AnyHashable], into destinationAttributes: [String : AnyHashable], removeOnCompletion: Bool) throws { + static func migrateObjects(matching query: [String : AnyHashable], into destinationAttributes: [String : AnyHashable], removeOnCompletion: Bool) throws { // Capture the keys in the destination prior to migration beginning. let keysInKeychainPreMigration = Set(try Keychain.allKeys(options: destinationAttributes)) @@ -343,7 +343,7 @@ internal final class Keychain { } } - internal static func revertMigration(into destinationAttributes: [String : AnyHashable], keysInKeychainPreMigration: Set) { + static func revertMigration(into destinationAttributes: [String : AnyHashable], keysInKeychainPreMigration: Set) { if let allKeysPostPotentiallyPartialMigration = try? Keychain.allKeys(options: destinationAttributes) { let migratedKeys = allKeysPostPotentiallyPartialMigration.subtracting(keysInKeychainPreMigration) migratedKeys.forEach { migratedKey in diff --git a/Sources/Valet/Internal/SecItem.swift b/Sources/Valet/Internal/SecItem.swift index 3156bd3e..b3493bec 100644 --- a/Sources/Valet/Internal/SecItem.swift +++ b/Sources/Valet/Internal/SecItem.swift @@ -17,7 +17,7 @@ import Foundation -internal func execute(in lock: NSLock, block: () throws -> ReturnType) rethrows -> ReturnType { +func execute(in lock: NSLock, block: () throws -> ReturnType) rethrows -> ReturnType { lock.lock() defer { lock.unlock() @@ -26,11 +26,11 @@ internal func execute(in lock: NSLock, block: () throws -> ReturnTyp } -internal final class SecItem { +final class SecItem { // MARK: Internal Class Methods - internal static func copy(matching query: [String : AnyHashable]) throws(KeychainError) -> DesiredType { + static func copy(matching query: [String : AnyHashable]) throws(KeychainError) -> DesiredType { if query.isEmpty { assertionFailure("Must provide a query with at least one item") } @@ -56,7 +56,7 @@ internal final class SecItem { } } - internal static func performCopy(matching query: [String : AnyHashable]) -> OSStatus { + static func performCopy(matching query: [String : AnyHashable]) -> OSStatus { guard !query.isEmpty else { // Must provide a query with at least one item return errSecParam @@ -70,7 +70,7 @@ internal final class SecItem { return status } - internal static func add(attributes: [String : AnyHashable]) throws(KeychainError) { + static func add(attributes: [String : AnyHashable]) throws(KeychainError) { if attributes.isEmpty { assertionFailure("Must provide attributes with at least one item") } @@ -90,7 +90,7 @@ internal final class SecItem { } } - internal static func update(attributes: [String : AnyHashable], forItemsMatching query: [String : AnyHashable]) throws(KeychainError) { + static func update(attributes: [String : AnyHashable], forItemsMatching query: [String : AnyHashable]) throws(KeychainError) { if attributes.isEmpty { assertionFailure("Must provide attributes with at least one item") } @@ -113,7 +113,7 @@ internal final class SecItem { } } - internal static func deleteItems(matching query: [String : AnyHashable]) throws(KeychainError) { + static func deleteItems(matching query: [String : AnyHashable]) throws(KeychainError) { if query.isEmpty { assertionFailure("Must provide a query with at least one item") } diff --git a/Sources/Valet/Internal/Service.swift b/Sources/Valet/Internal/Service.swift index ae3bc70b..8498a78a 100644 --- a/Sources/Valet/Internal/Service.swift +++ b/Sources/Valet/Internal/Service.swift @@ -17,7 +17,7 @@ import Foundation -internal enum Service: CustomStringConvertible, Equatable, Sendable { +enum Service: CustomStringConvertible, Equatable, Sendable { case standard(Identifier, Configuration) case sharedGroup(SharedGroupIdentifier, Identifier?, Configuration) @@ -28,23 +28,23 @@ internal enum Service: CustomStringConvertible, Equatable, Sendable { // MARK: Equatable - internal static func ==(lhs: Service, rhs: Service) -> Bool { + static func ==(lhs: Service, rhs: Service) -> Bool { lhs.description == rhs.description } // MARK: CustomStringConvertible - internal var description: String { + var description: String { secService } // MARK: Internal Static Methods - internal static func standard(with configuration: Configuration, identifier: Identifier, accessibilityDescription: String) -> String { + static func standard(with configuration: Configuration, identifier: Identifier, accessibilityDescription: String) -> String { "VAL_\(configuration.description)_initWithIdentifier:accessibility:_\(identifier)_\(accessibilityDescription)" } - internal static func sharedGroup(with configuration: Configuration, groupIdentifier: SharedGroupIdentifier, identifier: Identifier?, accessibilityDescription: String) -> String { + static func sharedGroup(with configuration: Configuration, groupIdentifier: SharedGroupIdentifier, identifier: Identifier?, accessibilityDescription: String) -> String { if let identifier = identifier { return "VAL_\(configuration.description)_initWithSharedAccessGroupIdentifier:accessibility:_\(groupIdentifier.groupIdentifier)_\(identifier)_\(accessibilityDescription)" } else { @@ -52,13 +52,13 @@ internal enum Service: CustomStringConvertible, Equatable, Sendable { } } - internal static func sharedGroup(with configuration: Configuration, explicitlySetIdentifier identifier: Identifier, accessibilityDescription: String) -> String { + static func sharedGroup(with configuration: Configuration, explicitlySetIdentifier identifier: Identifier, accessibilityDescription: String) -> String { "VAL_\(configuration.description)_initWithSharedAccessGroupIdentifier:accessibility:_\(identifier)_\(accessibilityDescription)" } // MARK: Internal Methods - internal func generateBaseQuery() -> [String : AnyHashable] { + func generateBaseQuery() -> [String : AnyHashable] { var baseQuery: [String : AnyHashable] = [ kSecClass as String : kSecClassGenericPassword as String, kSecAttrService as String : secService, diff --git a/Sources/Valet/Internal/WeakStorage.swift b/Sources/Valet/Internal/WeakStorage.swift index a6bf3d0a..7a07fdc3 100644 --- a/Sources/Valet/Internal/WeakStorage.swift +++ b/Sources/Valet/Internal/WeakStorage.swift @@ -17,8 +17,8 @@ import Foundation -internal final class WeakStorage: @unchecked Sendable { - internal subscript(_ key: String) -> T? { +final class WeakStorage: @unchecked Sendable { + subscript(_ key: String) -> T? { get { lock.withLock { identifierToValetMap.object(forKey: key as NSString) diff --git a/Sources/Valet/MigratableKeyValuePair.swift b/Sources/Valet/MigratableKeyValuePair.swift index eebfba51..1c4cd6de 100644 --- a/Sources/Valet/MigratableKeyValuePair.swift +++ b/Sources/Valet/MigratableKeyValuePair.swift @@ -54,7 +54,7 @@ public final class ObjectiveCCompatibilityMigratableKeyValuePairInput: NSObject // MARK: Initialization - internal init(key: Any, value: Data) { + init(key: Any, value: Data) { self.key = key self.value = value } @@ -116,7 +116,7 @@ public class ObjectiveCCompatibilityMigratableKeyValuePairOutput: NSObject { // MARK: Internal - internal fileprivate(set) var preventMigration: Bool + fileprivate(set) var preventMigration: Bool } diff --git a/Sources/Valet/SecureEnclave.swift b/Sources/Valet/SecureEnclave.swift index e0f36e02..dd6f9064 100644 --- a/Sources/Valet/SecureEnclave.swift +++ b/Sources/Valet/SecureEnclave.swift @@ -27,7 +27,7 @@ public final class SecureEnclave: Sendable { /// - Parameter service: The service of the keychain slice we want to check if we can access. /// - Returns: `true` if the keychain is accessible for reading and writing, `false` otherwise. /// - Note: Determined by writing a value to the keychain and then reading it back out. - internal static func canAccessKeychain(with service: Service) -> Bool { + static func canAccessKeychain(with service: Service) -> Bool { // To avoid prompting the user for Touch ID or passcode, create a Valet with our identifier and accessibility and ask it if it can access the keychain. let noPromptValet: Valet switch service { @@ -53,7 +53,7 @@ public final class SecureEnclave: Sendable { /// - key: A key that can be used to retrieve the `object` from the keychain. /// - options: A base query used to scope the calls in the keychain. /// - Throws: An error of type `KeychainError`. - internal static func setObject(_ object: Data, forKey key: String, options: [String : AnyHashable]) throws(KeychainError) { + static func setObject(_ object: Data, forKey key: String, options: [String : AnyHashable]) throws(KeychainError) { // Remove the key before trying to set it. This will prevent us from calling SecItemUpdate on an item stored on the Secure Enclave, which would cause iOS to prompt the user for authentication. try Keychain.removeObject(forKey: key, options: options) @@ -68,7 +68,7 @@ public final class SecureEnclave: Sendable { /// - options: A base query used to scope the calls in the keychain. /// - Returns: The data currently stored in the keychain for the provided key. /// - Throws: An error of type `KeychainError`. - internal static func object( + static func object( forKey key: String, withPrompt userPrompt: String, context: LAContext?, @@ -88,7 +88,7 @@ public final class SecureEnclave: Sendable { /// - options: A base query used to scope the calls in the keychain. /// - Returns: The data currently stored in the keychain for the provided key. /// - Throws: An error of type `KeychainError`. - internal static func object( + static func object( forKey key: String, options: [String : AnyHashable] ) throws(KeychainError) -> Data { @@ -102,7 +102,7 @@ public final class SecureEnclave: Sendable { /// - options: A base query used to scope the calls in the keychain. /// - Returns: `true` if a value has been set for the given key, `false` otherwise. /// - Throws: An error of type `KeychainError`. - internal static func containsObject(forKey key: String, options: [String : AnyHashable]) throws(KeychainError) -> Bool { + static func containsObject(forKey key: String, options: [String : AnyHashable]) throws(KeychainError) -> Bool { var secItemQuery = options let context = LAContext() context.interactionNotAllowed = true @@ -127,7 +127,7 @@ public final class SecureEnclave: Sendable { /// - key: A key that can be used to retrieve the `string` from the keychain. /// - options: A base query used to scope the calls in the keychain. /// - Throws: An error of type `KeychainError`. - internal static func setString(_ string: String, forKey key: String, options: [String : AnyHashable]) throws(KeychainError) { + static func setString(_ string: String, forKey key: String, options: [String : AnyHashable]) throws(KeychainError) { // Remove the key before trying to set it. This will prevent us from calling SecItemUpdate on an item stored on the Secure Enclave, which would cause iOS to prompt the user for authentication. try Keychain.removeObject(forKey: key, options: options) @@ -142,7 +142,7 @@ public final class SecureEnclave: Sendable { /// - options: A base query used to scope the calls in the keychain. /// - Returns: The string currently stored in the keychain for the provided key. /// - Throws: An error of type `KeychainError`. - internal static func string( + static func string( forKey key: String, withPrompt userPrompt: String, context: LAContext?, @@ -162,7 +162,7 @@ public final class SecureEnclave: Sendable { /// - options: A base query used to scope the calls in the keychain. /// - Returns: The string currently stored in the keychain for the provided key. /// - Throws: An error of type `KeychainError`. - internal static func string( + static func string( forKey key: String, options: [String : AnyHashable] ) throws(KeychainError) -> String { diff --git a/Sources/Valet/SecureEnclaveAccessControl.swift b/Sources/Valet/SecureEnclaveAccessControl.swift index 7c0883ad..f4379f1c 100644 --- a/Sources/Valet/SecureEnclaveAccessControl.swift +++ b/Sources/Valet/SecureEnclaveAccessControl.swift @@ -52,7 +52,7 @@ public enum SecureEnclaveAccessControl: Int, CustomStringConvertible, Equatable, // MARK: Internal Properties - internal var secAccessControl: SecAccessControlCreateFlags { + var secAccessControl: SecAccessControlCreateFlags { switch self { case .userPresence: .userPresence @@ -73,7 +73,7 @@ public enum SecureEnclaveAccessControl: Int, CustomStringConvertible, Equatable, } } - internal static func allValues() -> [SecureEnclaveAccessControl] { + static func allValues() -> [SecureEnclaveAccessControl] { [ .userPresence, .devicePasscode, diff --git a/Sources/Valet/SecureEnclaveValet.swift b/Sources/Valet/SecureEnclaveValet.swift index 14c95bc4..9ce77397 100644 --- a/Sources/Valet/SecureEnclaveValet.swift +++ b/Sources/Valet/SecureEnclaveValet.swift @@ -283,7 +283,7 @@ public final class SecureEnclaveValet: NSObject, Sendable { // MARK: Internal Properties - internal let service: Service + let service: Service // MARK: Private Properties diff --git a/Sources/Valet/SharedGroupIdentifier.swift b/Sources/Valet/SharedGroupIdentifier.swift index d73aa804..f87ad698 100644 --- a/Sources/Valet/SharedGroupIdentifier.swift +++ b/Sources/Valet/SharedGroupIdentifier.swift @@ -63,10 +63,10 @@ public struct SharedGroupIdentifier: CustomStringConvertible, Sendable { // MARK: Internal Properties - internal let prefix: String - internal let groupIdentifier: String + let prefix: String + let groupIdentifier: String - internal var asIdentifier: Identifier { + var asIdentifier: Identifier { // It is safe to force unwrap because we've already validated that our description is non-empty. Identifier(nonEmpty: description)! } diff --git a/Sources/Valet/SinglePromptSecureEnclaveValet.swift b/Sources/Valet/SinglePromptSecureEnclaveValet.swift index e702b6fe..8c0a4469 100644 --- a/Sources/Valet/SinglePromptSecureEnclaveValet.swift +++ b/Sources/Valet/SinglePromptSecureEnclaveValet.swift @@ -248,7 +248,7 @@ public final class SinglePromptSecureEnclaveValet: NSObject, @unchecked Sendable // MARK: Internal Properties - internal let service: Service + let service: Service // MARK: Private Properties diff --git a/Sources/Valet/Valet.swift b/Sources/Valet/Valet.swift index 93fbaf15..cba46c27 100644 --- a/Sources/Valet/Valet.swift +++ b/Sources/Valet/Valet.swift @@ -487,9 +487,9 @@ public final class Valet: NSObject, Sendable { // MARK: Internal Properties - internal let configuration: Configuration - internal let service: Service - internal var baseKeychainQuery: [String : AnyHashable] { + let configuration: Configuration + let service: Service + var baseKeychainQuery: [String : AnyHashable] { return service.generateBaseQuery() } @@ -723,7 +723,7 @@ extension Valet { // MARK: - Testing -internal extension Valet { +extension Valet { // MARK: Permutations diff --git a/Tests/ValetIntegrationTests/ValetIntegrationTests.swift b/Tests/ValetIntegrationTests/ValetIntegrationTests.swift index c784af1f..3c04616c 100644 --- a/Tests/ValetIntegrationTests/ValetIntegrationTests.swift +++ b/Tests/ValetIntegrationTests/ValetIntegrationTests.swift @@ -66,7 +66,7 @@ func testEnvironmentSupportsWhenPasscodeSet() -> Bool { } -internal extension Valet { +extension Valet { // MARK: Shared Access Group From b96f91f3ac501e54afbbd80398c5abffa116dc52 Mon Sep 17 00:00:00 2001 From: Richard Topchii Date: Fri, 14 Mar 2025 20:01:41 +0200 Subject: [PATCH 2/2] Code cleanup: remove unnecessary return statements --- Sources/Valet/Identifier.swift | 2 +- Sources/Valet/Valet.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Valet/Identifier.swift b/Sources/Valet/Identifier.swift index 9a588f7f..2791758b 100644 --- a/Sources/Valet/Identifier.swift +++ b/Sources/Valet/Identifier.swift @@ -32,7 +32,7 @@ public struct Identifier: CustomStringConvertible, Sendable { // MARK: CustomStringConvertible public var description: String { - return backingString + backingString } // MARK: Private Properties diff --git a/Sources/Valet/Valet.swift b/Sources/Valet/Valet.swift index cba46c27..e5768982 100644 --- a/Sources/Valet/Valet.swift +++ b/Sources/Valet/Valet.swift @@ -490,7 +490,7 @@ public final class Valet: NSObject, Sendable { let configuration: Configuration let service: Service var baseKeychainQuery: [String : AnyHashable] { - return service.generateBaseQuery() + service.generateBaseQuery() } // MARK: Private Properties