Remove generic key which over-constrains query #79
Conversation
| @@ -191,7 +189,6 @@ + (BOOL) removeItemForKey: (NSString *) key | |||
| NSMutableDictionary *itemToDelete = [[NSMutableDictionary alloc] init]; | |||
| [itemToDelete setObject:(__bridge id)kSecClassGenericPassword forKey:(__bridge id)kSecClass]; | |||
| [itemToDelete setObject:service forKey:(__bridge id)kSecAttrService]; | |||
| [itemToDelete setObject:key forKey:(__bridge id)kSecAttrGeneric]; | |||
| [itemToDelete setObject:key forKey:(__bridge id)kSecAttrAccount]; | |||
| #if !TARGET_IPHONE_SIMULATOR && defined(__IPHONE_OS_VERSION_MIN_REQUIRED) | |||
There was a problem hiding this comment.
Can this dictionary generation code be consolidated into one place?
There was a problem hiding this comment.
The objects set on the dictionary/query are not consistent between methods, and removeAllItemsForService uses the dictionary in a loop. The only consistent line is
[query setObject:(__bridge id)kSecClassGenericPassword forKey:(__bridge id)kSecClass];
There was a problem hiding this comment.
Would this work?
static NSMutableDictionary* NSMutableDictionary createQuery(id class, id service, id account) {
...
}
There was a problem hiding this comment.
We could do that, providing we cover all the parameters for all the methods, such as kSecReturnData, kSecReturnAttributes, kSecMatchLimit, kSecAttrAccessGroup...
@insha any comment to add?
|
@insha @chrisinsfo Can you address this? |
|
While the code is working, consolidating this code will affect all our apps and therefore will take a while with testing. Is it possible that we create an issue on GitHub to track this? |
|
OK, @insha Can you create a issue from @chrisinsfo 's comment? |
Remove generic key which over-constrains query
|
I'm concerned that setting nil as a query parameter via a class method to create the query, may actually constrain the query, which is what I'm trying to avoid. Will need testing or advice - @YuanZhu-apple ? |
|
@YuanZhu-apple Will do. Thank you. |
|
@chrisinsfo How about we discuss it when the PR for #80 is submitted? |
ghost
deleted the
Unit test fixes and added hook for onboarding
Aims: Fix bug where passcode is sometimes not accepted.
Test:
Expected:
Passcode is accepted and previously viewed tab is shown.
Actual without fix:
Passcode is denied