Cannot stay logged in after token expires about okta-oidc-ios

[jmelberg-okta]

Thanks for the report @WillChilcutt, @jplourde13, and @samleegray!

This issue seems closely related to #63, where we're seeing consistent issues around unarchiving the object from a saved state.

I'm going to prioritize this issue with our team so it gets addressed quickly.

[jmelberg-okta]

Hi all,

Apologies again for the issues this is causing. We're looking into the root cause of this issue this week. Once our analysis is complete, we'll update this issue with a status report.

[jmelberg-okta]

Thanks again for your ongoing patience. I was able to identify the root cause of this issue, and plan to release a new version with this fix as soon as I can.

I'm moving tagging this issue as "In Progress" - and will update this issue as work is completed.

[jplourde13]

This also happens to me when using the Android version of this sdk:

https://github.com/okta/okta-sdk-appauth-android

[samleegray]

I'm getting the same issue on iOS.

[WillChilcutt]

@jmelberg-okta Is there any ETA on when this fix will be implemented? We are blocked right now as users cannot stay logged in for more than an hour.

Could we get an update on this issue? It's impacting us here at Nike as well.

[jdberry]

What's the status on this issue? This is killing us. It's incredible to me that this has seemingly gotten no attention.

[jdberry]

@jmaldonado-okta: Are you able to give any summary on the scope of the change and what your ETA is for fixing it? We have a deadline coming up very soon for which we need this fix, and have to evaluate whether to come up with our own solution.

[jmelberg-okta]

@jdberry - The fix is pretty straightforward, and you can see the changes in #68.

I was able to validate the changes given @WillChilcutt's reproduction steps above, however I'd still like to spend more time writing tests to cover more flows before this hits master.

If you'd like, please pull down the changes and test locally. If anything doesn't act as you'd expect it to, please feel free to tag me here.

Again, thank you all for your patience.

[jdberry]

@jmaldonado-okta : that seems to change the behavior a bit, but it still doesn't seem to be working correctly.

We get into this state, for instance, if the app restarts after the access token has timed out. What we do then is the following:

On startup:

1. If the OktaAuth.isAuthenticated(), then (we're good)
2. Otherwise, try to OktaAuth.refresh() before doing anything else (i.e., before falling back to calling login())

In this case, in step 2, the refresh appears to succeed, but at that point OktaAuth.isAuthenticated() still says that it's not authenticated. The refresh hasn't stored new tokens in the TokenManager?

Is this the thing we should be doing, or is something else expected?

[jdberry]

@jmaldonado-okta : I made a change that seems (on very, very quick inspection) to have fixed the issue I describe above.

I altered the code of OktaRefresh.swift to call storeAuthState(tokens!) just before the final resolve(token) call. I don't know if that's the best fix, but it seems to solve the issue I'm seeing (after applying your patch).

if error != nil {
    return reject(OktaError.ErrorFetchingFreshTokens(error!.localizedDescription))
}
guard let token = accessToken else {
    return reject(OktaError.ErrorFetchingFreshTokens("Access Token could not be refreshed."))
}

// I added the following line...
OktaAuthorization().storeAuthState(tokens!)

return resolve(token)

I'll note that the reason the storeAuthState is needed is that the first thing isAuthenticated does is try to unarchive the token manager from the keychain. By adding the call to storeAuthState, we put a valid token manager and tokens there for it to unarchive when its next called.

[jdberry]

Incidentally, there's another fix we also had to make. We call Okta using CocoaPods, and it's linked as a framework. Due to this, there's inconsistency in how OktaTokenManager is named, and due to swift's addition of the module name to the class name, the OktaTokenManager cannot sometimes be successfully unarchived from the keychain.

We fixed this problem by doing the following at our app init time:

        NSKeyedArchiver.setClassName("OktaTokenManager", for: OktaTokenManager.self)
        NSKeyedUnarchiver.setClass(OktaTokenManager.self, forClassName: "OktaTokenManager")

Ideally, OktaTokenManager would have an @objc(OktaTokenManager) attribute, in order to pin down the name of the class, but note that this could cause existing code/data to not be able to restore at all, so some nuance is needed.

[WillChilcutt]

Pulled in both @jmelberg-okta and @jdberry 's changes, which ended up working for us! Thanks!

A mix of the two worked for us as well. Thanks guys! 🙇

[bynelus]

Also pulled in both changes, but still won't work. When I login at start it, it succeeds. But when I close the app and remove it from background, it seems the the tokens are not stored resulting in a login every time the app starts.

Is there anyone who can help out to fix this?

[jmelberg-okta]

Hi all,

I want to apologize again for the unacceptably long delay for not getting this resolved sooner. Our team is working on getting a dedicated engineer to maintain this library - something this repository has never had.

This bug has now been merged into master is is available on Cocoapods as version 1.0.1.

[WillChilcutt]

@jmelberg-okta Is there anything we need to do different on our side as users of this framework with the new 1.0.1 version?

[jmelberg-okta]

Hey @WillChilcutt - No changes should be required on your end.

This library was neglecting to re-save the authentication state when refreshing tokens, so if you had any workaround logic for this, it should no longer be required. Therefore, you should be safe to upgrade :)