okta / okta-mobile-swift

okta-mobile-swift
https://github.com/okta/okta-mobile-swift
Apache License 2.0
44 stars 19 forks source link

Keychain Token Broken After Update #138

Closed aetherealtech closed 1 year ago

aetherealtech commented 1 year ago

Describe the bug?

We recently updated to version 1.4.0, and found that our app was making us log in on every session, and was never receiving a notification that the token updated after saving it.

After considerable investigation, we determined that the JSON format of the token changed with this new version, which causes Credential.default to fail silently when attempting to decode what's stored in the keychain. Since it returns nil, it's impossible for us to remove the old token from the keychain. After logging in, and saving the token, we followed the code down into where it tries to delete the existing token and add the new one. The SecItemDelete call return an OSStatus of errSecItemNotFound, but the Okta library code that calls this doesn't check the status of delete. It only checks the status of the SecItemAdd call, which returns no error despite not adding the new token, presumably because it already exists in the keychain. Despite reporting no error, the new token is not added. The old token is unchanged.

Since the old token is already there, the notification that the default changed doesn't fire. If we close and re-open our app, the old token is still in the keychain, it can't be decoded, and therefore can't be deleted, and we're stuck. The only way out of this we could find was to reset the simulator. Deleting the app isn't enough because the keychain survives this. I'm not sure what a user on a real device would have to do.

What is expected to happen?

On an upgrade, the SDK should gracefully handle that the token JSON format has changed, and at worst delete the old token so we can cleanly log in and save a new token.

What is the actual behavior?

The old token in the keychain is undecodable, cannot be deleted manually (Credential.default returns nil, so Credential.default?.remove() does nothing), is not deleted when the SDK tries to save a new token, and can never be updated. It becomes impossible to save a new token without deleting the keychain.

Reproduction Steps?

Additional Information?

No response

SDK Version(s)

1.4.0

Build Information

No response

farshadtx commented 1 year ago

+1

mikenachbaur-okta commented 1 year ago

Thank you for bringing this to our attention. Pull request #141 should fix this issue, and I'll have a release ready for this shortly.

mikenachbaur-okta commented 1 year ago

@aetherealtech @farshadtx This has been fixed in version 1.4.1.