Skip to content

Fixes keychain access#314

Merged
koke merged 5 commits intorelease/3.8.4from
feature/credentials-fix
Nov 6, 2013
Merged

Fixes keychain access#314
koke merged 5 commits intorelease/3.8.4from
feature/credentials-fix

Conversation

@koke
Copy link
Copy Markdown
Member

@koke koke commented Nov 4, 2013

This should fix #306:

  • Drops every SFHFKeychainUtils dupes and it only keeps our modified version that uses kSecAttrAccessibleAfterFirstUnlock
  • Run a migration on launch that updates kSecAttrAccessibleWhenUnlocked items to kSecAttrAccessibleAfterFirstUnlock

koke added 5 commits November 4, 2013 19:42
Cocoapods keeps trying to update it when changing other stuff, so update
it on a separate commit in preparation for some other pod changes
Mae sure any keychain item with the kSecAttrAccessibleWhenUnlocked is
updated to kSecAttrAccessibleAfterFirstUnlock so we don't break when
waking up from a notification

fixes #306
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this fix required to finish before doing stuff like doing the push notification registration?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. What is required is that a user manually launches the app after update.

There could still be the case that the app auto updates, and it gets launched from a push notification before user interaction.

In that case, I believe the fixKeychainAccess method would just get an errSecInteractionNotAllowed error and return.

So it doesn't fix 100% of the cases on update, but eventually will. To handle that other case, there's https://gist.github.com/koke/7238985 but I haven't tested it that much

@sendhil
Copy link
Copy Markdown
Contributor

sendhil commented Nov 5, 2013

👍

@astralbodies
Copy link
Copy Markdown
Contributor

@koke I think this looks good. Merge time?

koke added a commit that referenced this pull request Nov 6, 2013
@koke koke merged commit 28ee32a into release/3.8.4 Nov 6, 2013
@koke koke deleted the feature/credentials-fix branch November 6, 2013 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants