Support arbitrary transforms of data during migration

[sergstav]

Thank you for this great library.

I try to do simple migration my old keychain data to Valet, but it gives me an error: keyInQueryResultInvalid

This is my source code:

 func migrateOldKeychainDataToValet(key: String) {
        
        let query = [
        kSecClass as String       : kSecClassGenericPassword,
        kSecAttrAccount as String : key,
        kSecAttrService as String : Bundle.main.bundleIdentifier ?? "SwiftKeychainWrapper",
        kSecMatchLimit as String  : kSecMatchLimitAll ] as [String : AnyHashable]
        
        let result = KeychainWrapper.standart.migrateObjects(matching: query, removeOnCompletion: false)
        print("result: \(result)")
    }

Where
KeychainWrapper.standart.migrateObjects(matching: query, removeOnCompletion: false)
is just a wrapper of Valet's method.

public func migrateObjects(matching query: [String : AnyHashable], removeOnCompletion: Bool) -> MigrationResult {
        return valet.migrateObjects(matching: query, removeOnCompletion: removeOnCompletion)
    }

As I understand issue appear on this line

So this line returns error.

guard !key.isEmpty else {
                return .keyInQueryResultInvalid
            }

I'm doing something wrong?

[dfed]

Thank you for the report @sergstav! Love all the context you gave. Can you:

1. Let us know what key you are passing into the key parameter in the migrateOldKeychainDataToValet(key:) method.
2. Print the keychainEntry that is causing this issue? Feel free to redact the values in the dictionary, but please do leave the type information so I can understand what this value is.
3. Optional: let us know how you stored data in the keychain prior to Valet.

I'm also curious why you're running migrations on a per-key basis. I'd expect that you could write a method without kSecAttrAccount as String : key, in your migration query to migrate all key:value pairs that belong to your kSecAttrService. But that said, if you already have an entry in the keychain with an empty kSecAttrAccount, you may have to delete that first. Assuming that you don't need the keychain entry that has no associated account, you could consider writing:

func migrateOldKeychainDataToValet() {
    let query = [
        kSecClass as String       : kSecClassGenericPassword,
        kSecAttrService as String : Bundle.main.bundleIdentifier ?? "SwiftKeychainWrapper",
        kSecMatchLimit as String  : kSecMatchLimitAll
    ] as [String : AnyHashable]
    
    // Delete malformed data from the keychain before migrating.    
    var deleteQuery = query
    deleteQuery[kSecAttrAccount as String] = ""
    SecItemDelete(deleteQuery as CFDictionary)
    
    let result = KeychainWrapper.standart.migrateObjects(matching: query, removeOnCompletion: false)
    print("result: \(result)")
}

If the above snippet works for you, let me know and I close out this issue. If the above snippet doesn't work for you or isn't a path you want to take, please do provide us the information requested at the top.

[sergstav]

@dfed
Thank you for your quick reply!

Try to reproduce migration with code above and after removing application from simulator getting error .keyInQueryResultInvalid

1. It was my mistake, removed the key from query.
2. Entire keychainEntry contains 15 elements, but I found line with error(see screenshot below).
3. Prior to Valet I stored data with SwiftKeychainWrapper, with simple set method:
   

This line gives error:

As you can see check
guard let key = keychainEntry[kSecAttrAccount as String] as? String fails at "as?", because type is Data.

Here is my updated source code:

internal static func migrateOldKeychainDataToValet() {
        let query = [
            kSecClass as String       : kSecClassGenericPassword,
            kSecAttrService as String : Bundle.main.bundleIdentifier ?? "SwiftKeychainWrapper",
            kSecMatchLimit as String  : kSecMatchLimitAll
            ] as [String : AnyHashable]
        
        let result = KeychainWrapper.standart.migrateObjects(matching: query, removeOnCompletion: false)
        print("result: \(result)")
    }

[sergstav]

@dfed

Here is full keychainEntry variable, I just edited app name to "myApp".

(lldb) po keychainEntry
▿ 15 elements
▿ 0 : 2 elements
- key : "musr"
▿ value : AnyHashable(0 bytes)
▿ value : 0 bytes
- count : 0
▿ pointer : 0x00007ffeed348b40
- pointerValue : 140732878064448
- bytes : 0 elements
▿ 1 : 2 elements
- key : "agrp"
▿ value : AnyHashable("group.com.myApp.myApp")
- value : "group.com.myApp.myApp"
▿ 2 : 2 elements
- key : "svce"
▿ value : AnyHashable("com.myApp.app")
- value : "com.myApp.app"
▿ 3 : 2 elements
- key : "sync"
▿ value : AnyHashable(0)
- value : 0
▿ 4 : 2 elements
- key : "tomb"
▿ value : AnyHashable(0)
- value : 0
▿ 5 : 2 elements
- key : "gena"
▿ value : AnyHashable(13 bytes)
▿ value : 13 bytes
- count : 13
▿ pointer : 0x00007ffeed348b40
- pointerValue : 140732878064448
▿ bytes : 13 elements
- 0 : 117
- 1 : 115
- 2 : 101
- 3 : 114
- 4 : 68
- 5 : 111
- 6 : 109
- 7 : 97
- 8 : 105
- 9 : 110
- 10 : 75
- 11 : 101
- 12 : 121
▿ 6 : 2 elements
- key : "v_Data"
▿ value : AnyHashable(8 bytes)
▿ value : 8 bytes
- count : 8
▿ pointer : 0x00007ffeed348b40
- pointerValue : 140732878064448
▿ bytes : 8 elements
- 0 : 115
- 1 : 101
- 2 : 114
- 3 : 103
- 4 : 116
- 5 : 101
- 6 : 115
- 7 : 116
▿ 7 : 2 elements
- key : "mdat"
▿ value : AnyHashable(2020-05-18 19:13:14 +0000)
▿ value : 2020-05-18 19:13:14 +0000
- timeIntervalSinceReferenceDate : 611521994.136842
▿ 8 : 2 elements
- key : "persistref"
▿ value : AnyHashable(0 bytes)
▿ value : 0 bytes
- count : 0
▿ pointer : 0x00007ffeed348b40
- pointerValue : 140732878064448
- bytes : 0 elements
▿ 9 : 2 elements
- key : "cdat"
▿ value : AnyHashable(2020-05-08 11:50:58 +0000)
▿ value : 2020-05-08 11:50:58 +0000
- timeIntervalSinceReferenceDate : 610631458.494373
▿ 10 : 2 elements
- key : "pdmn"
▿ value : AnyHashable("ak")
- value : "ak"
▿ 11 : 2 elements
- key : "acct"
▿ value : AnyHashable(13 bytes)
▿ value : 13 bytes
- count : 13
▿ pointer : 0x00007ffeed348b40
- pointerValue : 140732878064448
▿ bytes : 13 elements
- 0 : 117
- 1 : 115
- 2 : 101
- 3 : 114
- 4 : 68
- 5 : 111
- 6 : 109
- 7 : 97
- 8 : 105
- 9 : 110
- 10 : 75
- 11 : 101
- 12 : 121
▿ 12 : 2 elements
- key : "accc"
▿ value : AnyHashable(<SecAccessControlRef: ak>)
- value : <SecAccessControlRef: ak>
▿ 13 : 2 elements
- key : "sha1"
▿ value : AnyHashable(20 bytes)
▿ value : 20 bytes
- count : 20
▿ pointer : 0x00007b1c00020d40
- pointerValue : 135360189435200
▿ bytes : 20 elements
- 0 : 40
- 1 : 189
- 2 : 138
- 3 : 173
- 4 : 83
- 5 : 191
- 6 : 179
- 7 : 57
- 8 : 218
- 9 : 201
- 10 : 254
- 11 : 99
- 12 : 157
- 13 : 109
- 14 : 32
- 15 : 197
- 16 : 84
- 17 : 11
- 18 : 184
- 19 : 55
▿ 14 : 2 elements
- key : "v_PersistentRef"
▿ value : AnyHashable(12 bytes)
▿ value : 12 bytes
- count : 12
▿ pointer : 0x00007ffeed348b40
- pointerValue : 140732878064448
▿ bytes : 12 elements
- 0 : 103
- 1 : 101
- 2 : 110
- 3 : 112
- 4 : 0
- 5 : 0
- 6 : 0
- 7 : 0
- 8 : 0
- 9 : 0
- 10 : 0
- 11 : 154

(lldb)

@dfed
here is the type
(lldb) po type(of: keychainEntry)
Swift.Dictionary<Swift.String, Swift.AnyHashable>

[dfed]

As you can see check
guard let key = keychainEntry[kSecAttrAccount as String] as? String fails at "as?", because type is Data

This is absolutely the crux of the issue. Good sleuthing @sergstav!

Judging from a quick read of SwiftKeychainWrapper, it does seem they expect to store SecAttrAccount keys as Data.

There isn't a quick answer here. I'm going to re-task this Issue to be a feature request to support arbitrarily transforming data as part of a migration. Until I land that, though, I think you may need to write your own custom migration code 😞

[dfed]

I'm imagining a world where we have a migrateObjects method that looks something like:

struct KeyValuePairToMigrate<KeyType> {
  let key: KeyType
  let value: Data
}

struct MigratedKeyValuePair {
  let pair: KeyValuePairToMigrate<String>?
  let removeOriginalOnCompletion: Bool
}

func migrateObjects(matching query: [String : AnyHashable], transform: (KeyValuePairToMigrate<Any>) throws -> MigratedKeyValuePair) throws

Where the transform block can be used to:

1. Enable the remapping of keys from a legacy value to a new/desired value (which could include changing the key's type from Data to String)
2. Enable the remapping of data from a legacy value to a new/desired value
3. Enable not migrating particular key/value pairs
4. Enable throwing an error during migration that would trigger reverting the migration.

Open to API suggestions, but the above summarizes the capability I think we should support. Our existing migrateObjects methods would be able to call through to this underlying method. Note that this feature won't land until Valet 4, which is hopefully landing this month.

[sergstav]

Judging from a quick read of SwiftKeychainWrapper, it does seem they expect to store SecAttrAccount keys as Data.

Yes, saw this line, but did not attach the necessary meaning, my bad, lack of experience working with keychain.

Until I land that, though, I think you may need to write your own custom migration code 😞

Ok, there is no problem. I think I can get my stored values from keychain using Apple API for Keychain, without any library, and then save it to Valet for further using.

[dfed]

All good! Glad you feel unblocked, and thank you for raising this issue. It'll finally get me to add a migration method that can do in-place transforms, which is something I've wanted for a long time 🙂