DataStoreStorage replaces the deprecated SharedPreferencesStorage #259
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
library/src/main/java/io/fusionauth/mobilesdk/storage/DataStoreStorage.kt
Fixed
Show fixed
Hide fixed
There was a problem hiding this comment.
Pull request overview
This PR deprecates SharedPreferencesStorage in favor of the new DataStoreStorage implementation for token persistence. The change aligns with modern Android development practices by adopting Jetpack DataStore.
Key Changes:
- Introduced
DataStoreStorageclass using Jetpack DataStore for persistence - Deprecated
SharedPreferencesStoragewith a warning-level annotation - Updated sample app activities to use the new
DataStoreStorage
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| SharedPreferencesStorage.kt | Added deprecation annotation directing users to DataStoreStorage |
| DataStoreStorage.kt | New storage implementation using Jetpack DataStore with synchronous API wrapping coroutines |
| library/build.gradle.kts | Added DataStore preferences dependency (version 1.2.0) |
| TokenActivity.kt | Replaced SharedPreferencesStorage with DataStoreStorage in initialization |
| LoginActivity.kt | Replaced SharedPreferencesStorage with DataStoreStorage in initialization |
| README.md | Updated documentation to reference DataStoreStorage instead of SharedPreferencesStorage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return runBlocking { | ||
| context.dataStore.data.first()[prefKey] | ||
| } |
There was a problem hiding this comment.
Using runBlocking on the main thread for storage operations can cause UI freezes and ANR (Application Not Responding) errors. Consider making the Storage interface suspend-based or providing async alternatives, as DataStore is designed to be used with coroutines asynchronously.
| runBlocking { | ||
| context.dataStore.edit { settings -> | ||
| settings[prefKey] = content.toString() | ||
| } | ||
| } |
There was a problem hiding this comment.
Using runBlocking on the main thread for storage operations can cause UI freezes and ANR errors. Consider making the Storage interface suspend-based or providing async alternatives.
| runBlocking { | ||
| context.dataStore.edit { settings -> | ||
| settings.remove(prefKey) | ||
| } | ||
| } |
There was a problem hiding this comment.
Using runBlocking on the main thread for storage operations can cause UI freezes and ANR errors. Consider making the Storage interface suspend-based or providing async alternatives.
| */ | ||
| @Deprecated( | ||
| message = "Use DataStoreStorage instead", | ||
| replaceWith = ReplaceWith("DataStoreStorage"), |
There was a problem hiding this comment.
The ReplaceWith should include the full qualified name or import statement. Use ReplaceWith(\"DataStoreStorage\", \"io.fusionauth.mobilesdk.storage.DataStoreStorage\") to ensure IDEs can properly auto-import when applying the replacement.
| replaceWith = ReplaceWith("DataStoreStorage"), | |
| replaceWith = ReplaceWith("DataStoreStorage", "io.fusionauth.mobilesdk.storage.DataStoreStorage"), |
| return | ||
| lifecycleScope.launch { | ||
| if (AuthorizationManager.isAuthenticated()) { | ||
| Log.i(TAG, "User is already authenticated, proceeding to token activity") |
Check notice
Code scanning / mobsfscan
The App logs information. Sensitive information should never be logged. Note
| try { | ||
| val authState: FusionAuthState = AuthorizationManager.oAuth(this@TokenActivity) | ||
| .handleRedirect(intent) | ||
| Log.i(TAG, authState.toString()) |
Check notice
Code scanning / mobsfscan
The App logs information. Sensitive information should never be logged. Note
| Log.i(TAG, authState.toString()) | ||
| fetchUserInfoAndDisplayAuthorized() | ||
| } catch (ex: AuthorizationException) { | ||
| Log.e(TAG, "Failed to exchange authorization code", ex) |
Check notice
Code scanning / mobsfscan
The App logs information. Sensitive information should never be logged. Note
| try { | ||
| val authState: FusionAuthState = AuthorizationManager.oAuth(this@TokenActivity) | ||
| .handleRedirect(intent) | ||
| Log.i(TAG, authState.toString()) |
Check failure
Code scanning / CodeQL
Insertion of sensitive information into log files High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 15 hours ago
To fix this issue, we must ensure that sensitive data such as access tokens, ID tokens, and refresh tokens are not written to any log output. The problematic log statement is Log.i(TAG, authState.toString()) on line 87 of TokenActivity.kt. Rather than logging the entire object, log a generic message indicating success or relevant non-sensitive details (e.g., that the authorization state was received or exchanged), without including actual tokens or authentication credentials. No changes are needed elsewhere; tokens should not be logged anywhere.
- Change line 87 to something like:
Log.i(TAG, "Authorization state successfully obtained") - If you want to assist debugging, you can include non-sensitive fields (other than tokens), but based on the given code, it's safest to just log a status message.
| @@ -84,7 +84,7 @@ | ||
| try { | ||
| val authState: FusionAuthState = AuthorizationManager.oAuth(this@TokenActivity) | ||
| .handleRedirect(intent) | ||
| Log.i(TAG, authState.toString()) | ||
| Log.i(TAG, "Authorization state successfully obtained") | ||
| fetchUserInfoAndDisplayAuthorized() | ||
| } catch (ex: AuthorizationException) { | ||
| Log.e(TAG, "Failed to exchange authorization code", ex) |
DataStoreStorage replaces the deprecated SharedPreferencesStorage as a way to store tokens.