Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add state parameter to AuthorizationCodeFlow.start #278

Merged
merged 3 commits into from
Jan 31, 2024

Conversation

rajdeepnanua-okta
Copy link
Contributor

@rajdeepnanua-okta rajdeepnanua-okta commented Jan 29, 2024

This PR adds state parameter to AuthorizationCodeFlow.start as requested by #276.

This PR also fixes some issues related to DeviceTokenCookie initialization with the following:

  1. Lazily loading DT cookie in DeviceTokenCookieJar to avoid crashes when users create OidcConfiguration in a Jetpack startup initializer.
  2. Attempting to reinitialize DeviceTokenProvider's storage by deleting the storage during Exception, and then recreating the storage.

These should fix most of the issues reported by users. The only exception being if the encryption key is itself corrupted, which can happen and has been reported as an issue for EncryptedSharedPreferences. But, that case should be much rarer than the issues addressed by the above two fixes.

Comment on lines 72 to 74
/** The default [CookieJar]. By default, it adds a DT cookie for identifying the device.
* To use the default [CookieJar] in OkHttp, set this to [CookieJar.NO_COOKIES] */
var cookieJar: CookieJar by NoSetAfterGetWithLazyDefaultFactory { DeviceTokenCookieJar(clock) }
var cookieJar: CookieJar by NoSetAfterGetWithLazyDefaultFactory { CookieJar.NO_COOKIES }
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you propose handling DeviceToken support for IDX? The documentation for this property no longer matches up with the implementation, so I'm wondering what the plan is?

Furthermore, if the cookieJar is set to NO_COOKIES, is this property even needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to revert this, and instead fixed the issues with the following:

  1. Lazily loading DT cookie in DeviceTokenCookieJar to avoid crashes when users create OidcConfiguration in a Jetpack startup initializer.
  2. Attempting to reinitialize DeviceTokenProvider's storage by deleting the storage during Exception, and then recreating the storage.

These should fix most of the issues reported by users. The only exception being if the encryption key is itself corrupted, which can happen and has been reported as an issue for EncryptedSharedPreferences. But, that case should be much rarer than the issues addressed by the above two fixes.

@rajdeepnanua-okta rajdeepnanua-okta merged commit a0a1562 into master Jan 31, 2024
3 checks passed
@rajdeepnanua-okta rajdeepnanua-okta deleted the state_parameter branch January 31, 2024 16:42
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.

2 participants