-
Notifications
You must be signed in to change notification settings - Fork 392
@W-23201591: [Android] Surface RTR state in developer info screen #2974
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,7 +53,9 @@ import android.text.TextUtils.join | |
| import android.view.WindowInsetsController.APPEARANCE_LIGHT_STATUS_BARS | ||
| import android.webkit.CookieManager | ||
| import android.webkit.URLUtil.isHttpsUrl | ||
| import android.widget.Toast | ||
| import androidx.annotation.VisibleForTesting | ||
| import androidx.annotation.VisibleForTesting.Companion.PRIVATE | ||
| import androidx.annotation.VisibleForTesting.Companion.PROTECTED | ||
| import androidx.compose.material3.ColorScheme | ||
| import androidx.compose.runtime.Composable | ||
|
|
@@ -147,6 +149,7 @@ import kotlinx.coroutines.CoroutineScope | |
| import kotlinx.coroutines.Dispatchers.Default | ||
| import kotlinx.coroutines.Dispatchers.Main | ||
| import kotlinx.coroutines.launch | ||
| import kotlinx.coroutines.withContext | ||
| import kotlinx.coroutines.withTimeoutOrNull | ||
| import okhttp3.HttpUrl.Companion.toHttpUrlOrNull | ||
| import java.lang.String.CASE_INSENSITIVE_ORDER | ||
|
|
@@ -1385,6 +1388,21 @@ open class SalesforceSDKManager protected constructor( | |
| */ | ||
| fun isGlobalFeatureRegistered(appFeatureCode: String) = features.contains(appFeatureCode) | ||
|
|
||
| /** | ||
| * Returns true if the feature code is registered for the given user | ||
| * (falling back to the current user when [user] is null). Reads the | ||
| * per-user feature set that backs the user agent's ftr_ token, so this | ||
| * reflects features such as RTR that are registered per account. | ||
| * | ||
| * @param appFeatureCode The app feature code | ||
| * @param user The user account, or null to use the current user | ||
| */ | ||
| internal fun isUserFeatureRegistered(appFeatureCode: String, user: UserAccount? = null): Boolean { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New read accessor for per-user feature flags. RTR-active state is stored as the per-user feature flag Rather than widen This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce. |
||
| val resolvedUser = user ?: userAccountManager.currentUser ?: return false | ||
| val key = "${resolvedUser.orgId}/${resolvedUser.userId}" | ||
| return perUserFeatures[key]?.contains(appFeatureCode) == true | ||
| } | ||
|
|
||
| /** | ||
| * Adds a per-user app feature code for reporting in the user agent header. | ||
| * Falls back to the global set when user is null. | ||
|
|
@@ -1568,11 +1586,61 @@ open class SalesforceSDKManager protected constructor( | |
| }) | ||
| } | ||
| } | ||
|
|
||
| /* | ||
| * Debug-only helper: proactively drive the SDK's standard | ||
| * token-refresh path so developers can observe Refresh Token | ||
| * Rotation (RTR) state update in the dev info screen without | ||
| * waiting for the access token to expire naturally. This whole | ||
| * menu is only shown when isDevSupportEnabled() is true (debug | ||
| * builds by default). | ||
| */ | ||
| actions["Force Token Refresh"] = object : DevActionHandler { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great idea.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sfdctaka Should we add this to iOS as well?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @brandonpage Ditto. Let me file a ticket for it and we can take care of it next week.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good — thanks for filing the iOS ticket, @sfdctaka. Happy to help with the parity work when it's up. Likewise the AuthFlowTester RTR UI is a nice follow-up for another day. This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce. |
||
| override fun onSelected() { | ||
| val user = userAccountManager.currentUser ?: return | ||
| CoroutineScope(Default).launch { | ||
| val message = forceTokenRefresh(user) | ||
| withContext(Main) { | ||
| Toast.makeText(appContext, message, Toast.LENGTH_LONG).show() | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return actions | ||
| } | ||
|
|
||
| /** | ||
| * Drives the SDK's standard token-refresh path for [user] so developers | ||
| * can observe Refresh Token Rotation (RTR) state update in the dev info | ||
| * screen without waiting for the access token to expire naturally. Backs | ||
| * the debug-only "Force Token Refresh" dev action. | ||
| * | ||
| * @param user The user whose access token should be refreshed. | ||
| * @param restClient The REST client to refresh, or null to resolve the | ||
| * user's client via [ClientManager.peekRestClient]. Tests can supply a | ||
| * mock to avoid a network call. | ||
| * @return A human-readable result message suitable for a Toast. Never | ||
| * throws — resolving the client and refreshing the token both happen | ||
| * inside the catch, so any failure (including the | ||
| * AccountInfoNotFoundException that [ClientManager.peekRestClient] raises | ||
| * when the account is missing or logging out) is caught, logged, and | ||
| * returned as a message (with a null-message fallback to the exception's | ||
| * simple class name). | ||
| */ | ||
| @VisibleForTesting(otherwise = PRIVATE) | ||
| internal fun forceTokenRefresh( | ||
| user: UserAccount, | ||
| restClient: RestClient? = null | ||
| ): String = try { | ||
| (restClient ?: clientManager.peekRestClient(user)).refreshAccessToken() | ||
| "Token refresh complete — check RTR section in dev info" | ||
| } catch (ex: Exception) { | ||
| e(TAG, "Force Token Refresh failed", ex) | ||
| "Token refresh failed: ${ex.message ?: ex.javaClass.simpleName}" | ||
| } | ||
|
|
||
| /** Information to display in the developer support dialog */ | ||
| @Deprecated( | ||
| "Will be removed in Mobile SDK 14.0, please use the new data class representation.", | ||
|
|
@@ -1626,6 +1694,8 @@ open class SalesforceSDKManager protected constructor( | |
| // "Identity Provider" to "$isIdentityProvider", | ||
| // ) | ||
| // | ||
| // // NOTE: carry over the RTR additionalSections.add(...) from the | ||
| // // live getter below, or RTR drops off the dev info screen. | ||
| // return DevSupportInfo( | ||
| // basicInfo, | ||
| // authConfig, | ||
|
|
@@ -1635,9 +1705,29 @@ open class SalesforceSDKManager protected constructor( | |
| // ) | ||
| // } | ||
| // | ||
| // TODO: Replace devSupportInfo with the above implementation when devSupportInfos is removed in 14.0. | ||
| /* | ||
| * TODO: Replace devSupportInfo with the above implementation when | ||
| * devSupportInfos is removed in 14.0. When doing so, preserve the RTR | ||
| * section appended in the live getter below — the commented-out | ||
| * implementation above builds DevSupportInfo via its structured | ||
| * constructor and does not add it, so RTR would otherwise silently drop | ||
| * from the dev info screen. | ||
| */ | ||
| open val devSupportInfo: DevSupportInfo | ||
| get() = DevSupportInfo.createFromLegacyDevInfos(devSupportInfos) | ||
| get() = DevSupportInfo.createFromLegacyDevInfos(devSupportInfos).apply { | ||
| /* | ||
| * Surface Refresh Token Rotation (RTR) state so developers can | ||
| * verify whether RTR is active for the current user's session and | ||
| * when the token last rotated. | ||
| */ | ||
| val currentUser = userAccountManager.cachedCurrentUser | ||
| additionalSections.add( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the RTR section is appended via Deliberately not done: (a) editing
This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @JohnsonEricAtSalesforce If you cannot add RTR to the new impl comment above, please add a comment noting that RTR needs to be added so it does not get lost.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done — added a note in the commented-out 14.0 This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce. |
||
| DevSupportInfo.parseRtrSection( | ||
| currentUser = currentUser, | ||
| rtrActive = currentUser != null && isUserFeatureRegistered(Features.FEATURE_RTR, currentUser), | ||
| ) | ||
| ) | ||
| } | ||
|
|
||
| /** Sends the logout completed intent */ | ||
| private fun sendLogoutCompleteIntent(logoutReason: LogoutReason, userAccount: UserAccount?) = | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be nice when everything is Kotlin and we can make things like this internal. Not necessary for this PR, but curious if you have an opinion on RestrictTo? It does not actually prevent someone from using the API, but adds a stern lint warning/error indicating that we do not want them to.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that's the right direction for library-internal API that has to stay
publicfrom Java. I'd rather not add it to just these two accessors here, since the siblingUserAccountaccessors (e.g.tokenType) aren't annotated and a partial application would be inconsistent — better as a uniform sweep across the account accessors. Filed W-23667824 to track that. Out of scope for this PR.This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.