okta / okta-mobile-kotlin

Okta's Android Authentication SDK
https://okta.github.io/okta-mobile-kotlin/
Apache License 2.0
35 stars 12 forks source link

Cannot store a token response without an idToken (SSO use case) #309

Closed mcrawshaw closed 2 months ago

mcrawshaw commented 3 months ago

Describe the bug?

We use InterationCodeFlow and exchangeInteractionCodeForTokens to retrieve our primary token, then TokenExchangeFlow to retrieve a second token using the idToken and deviceSecret from the first. Unfortunately, when we go to store the second token via Credential.store, it cannot be later retrieved via getAccessTokenIfValid. This occurs because the second response from TokenExchangeFlow.start does not contain an idToken. While the storage code doesn't generally need the idToken, it does when retrieving the access token as idToken.issuedAt is used.

What is expected to happen?

An access token is retrieved regardless of the presence of an id token.

What is the actual behavior?

No access token is ever retrieved from the credentials store.

Reproduction Steps?

Install 2.0.1, add a token response without an id token, then try to get the access token.

Additional Information?

The issue is on this line of code in master: https://github.com/okta/okta-mobile-kotlin/blob/29063c7b8b0ac7e90d23dec79b351e7bf7af71be/auth-foundation/src/main/java/com/okta/authfoundation/credential/Credential.kt#L559 Please find another way to calculate/derive the time when the access token was issued.

SDK Version and Artifact(s) used.

2.0.1

Build Information

No response

rajdeepnanua-okta commented 3 months ago

Hi @mcrawshaw, I will look into this shortly. I will look into alternative ways to verify token validity rather than relying on idToken. For now, you could still get the accessToken by using Credential.token.accessToken.

mcrawshaw commented 3 months ago

Thanks, we are aware of the accessToken availability in the response. We are looking to place both in the same Credential store so we can secure behind biometrics.

rajdeepnanua-okta commented 2 months ago

@mcrawshaw, just wanted to give a status update on this issue. I am working on a solution where we add an issuedAt field using the device clock if idToken isn't present in a Token. This requires db migration, so I'm adding room migration tests, and checking for any edge cases during migration. I will try to release the fix for this next week, but if any unexpected issues come up, I'll let you know.

mcrawshaw commented 2 months ago

Appreciate the update!

mcrawshaw commented 2 months ago

@rajdeepnanua-okta How is this progressing?

rajdeepnanua-okta commented 2 months ago

Hi @mcrawshaw, I have a solution ready and will have a fixed release available by EOD Monday.

rajdeepnanua-okta commented 2 months ago

Hi @mcrawshaw, I discovered two more issues that I'm going to be releasing a fix for as well. One for disabling autobackup of some encrypted files in the SDK, and one fix for SDK migration from 1.x -> 2.x. I'll keep you posted on the progress. I'm aiming to get fixes out early this week, and then do a release. Thanks for your patience 🙏

mcrawshaw commented 2 months ago

Is this close to release?

rajdeepnanua-okta commented 2 months ago

It's close to release now. The fix for this issue has been merged in https://github.com/okta/okta-mobile-kotlin/pull/315 I'm waiting on approvals on two other PRs, and I will cut a release as soon as that happens.

rajdeepnanua-okta commented 2 months ago

@mcrawshaw This issue should be fixed in the newly released version 2.0.2