Fix Login for Admin and Welcome Discovery incompatibility #2933
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -90,6 +90,7 @@ import androidx.compose.runtime.mutableStateOf | |
| import androidx.compose.runtime.remember | ||
| import androidx.compose.runtime.setValue | ||
| import androidx.compose.ui.Alignment | ||
| import androidx.core.net.toUri | ||
| import androidx.compose.ui.Modifier | ||
| import androidx.compose.ui.draw.shadow | ||
| import androidx.compose.ui.graphics.Color | ||
|
|
@@ -147,10 +148,16 @@ fun LoginView() { | |
| val viewModel: LoginViewModel = | ||
| viewModel(factory = SalesforceSDKManager.getInstance().loginViewModelFactory) | ||
| val frontDoorBridgeUrl = viewModel.frontDoorBridgeUrl.observeAsState() | ||
| // Observe selectedServer and loginUrl AS COMPOSE STATE so that recomposition triggers when | ||
| // either changes — and so titleText is read inside Compose's snapshot system, not via the | ||
| // non-reactive `defaultTitleText` getter chain (which reads LiveData.getValue() directly and | ||
| // is invisible to Compose). | ||
| val selectedServer = viewModel.selectedServer.observeAsState() | ||
| val loginUrl = viewModel.loginUrl.observeAsState() | ||
| val titleText = if (frontDoorBridgeUrl.value != null) { | ||
| viewModel.frontdoorBridgeServer ?: "" | ||
| } else { | ||
| viewModel.titleText ?: if (loginUrl.value == LoginActivity.ABOUT_BLANK) "" else selectedServer.value ?: "" | ||
| } | ||
| val showDevSupport = with(SalesforceSDKManager.getInstance()) { | ||
| return@with if (isDebugBuild && isDevSupportEnabled()) { | ||
|
|
@@ -160,6 +167,8 @@ fun LoginView() { | |
| } | ||
| } | ||
|
|
||
| val isWelcomeDiscoveryServer = selectedServer.value | ||
| ?.let { LoginActivity.isSalesforceWelcomeDiscoveryUrlPath(it.toUri()) } == true | ||
|
Contributor
There was a problem hiding this comment.
|
||
| val topAppBar = viewModel.topAppBar ?: { | ||
| DefaultTopAppBar( | ||
| backgroundColor = viewModel.topBarColor ?: viewModel.dynamicBackgroundColor.value, | ||
|
|
@@ -172,7 +181,9 @@ fun LoginView() { | |
| shouldShowBackButton = viewModel.shouldShowBackButton, | ||
| showDevSupport = showDevSupport, | ||
| finish = { activity.handleBackBehavior() }, | ||
| onLoginForAdmins = if (isWelcomeDiscoveryServer) null else { | ||
| { activity.launchLoginForAdminsAction() } | ||
| }, | ||
| ) | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -131,9 +131,11 @@ import com.salesforce.androidsdk.R.string.sf__server_url_save | |
| import com.salesforce.androidsdk.accounts.UserAccount | ||
| import com.salesforce.androidsdk.accounts.UserAccountManager | ||
| import com.salesforce.androidsdk.app.SalesforceSDKManager | ||
| import androidx.core.net.toUri | ||
| import com.salesforce.androidsdk.config.LoginServerManager | ||
| import com.salesforce.androidsdk.config.LoginServerManager.LoginServer | ||
| import com.salesforce.androidsdk.ui.CORNER_RADIUS | ||
| import com.salesforce.androidsdk.ui.LoginActivity | ||
| import com.salesforce.androidsdk.ui.LoginViewModel | ||
| import com.salesforce.androidsdk.ui.PADDING_SIZE | ||
| import com.salesforce.androidsdk.ui.theme.hintTextColor | ||
|
|
@@ -177,6 +179,12 @@ internal fun TestablePickerBottomSheet( | |
| if (newSelectedServer != SalesforceSDKManager.getInstance().loginServerManager.selectedLoginServer) { | ||
| viewModel.loading.value = true | ||
| SalesforceSDKManager.getInstance().loginServerManager.selectedLoginServer = newSelectedServer | ||
| } else if (LoginActivity.isSalesforceWelcomeDiscoveryUrlPath(newSelectedServer.url.toUri())) { | ||
| // Re-selecting Welcome Discovery while it is already the selected server should | ||
| // return the WebView to Phase 1 (welcome.salesforce.com/discovery), even though | ||
| // LoginServerManager's selection didn't change. This mirrors the reload-button | ||
| // behavior the user expects when stuck on a discovered My Domain in Phase 2. | ||
| viewModel.reloadWebView() | ||
| } | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ import com.salesforce.androidsdk.config.LoginServerManager.LoginServer | |
| import com.salesforce.androidsdk.config.LoginServerManager.WELCOME_LOGIN_URL | ||
| import com.salesforce.androidsdk.config.OAuthConfig | ||
| import com.salesforce.androidsdk.security.SalesforceKeyGenerator.getSHA256Hash | ||
| import com.salesforce.androidsdk.ui.LoginActivity | ||
| import com.salesforce.androidsdk.ui.LoginActivity.Companion.ABOUT_BLANK | ||
| import com.salesforce.androidsdk.ui.LoginViewModel | ||
| import io.mockk.coEvery | ||
|
|
@@ -69,6 +70,7 @@ import org.junit.Rule | |
| import org.junit.Test | ||
| import org.junit.runner.RunWith | ||
| import java.net.URI | ||
| import java.net.URLEncoder | ||
|
|
||
| private const val FAKE_SERVER_URL = "shouldMatchNothing.salesforce.com" | ||
| private const val FAKE_JWT = "1234" | ||
|
|
@@ -778,6 +780,12 @@ class LoginViewModelTest { | |
| assertTrue("isUsingFrontDoorBridge should be true", viewModel.isUsingFrontDoorBridge) | ||
| assertEquals("frontDoorBridgeUrl should be front door URL", frontDoorUrl, viewModel.frontDoorBridgeUrl.value) | ||
|
|
||
| // Precondition: invariant ("frontdoor wins over reload") only holds when the | ||
| // LoginServerManager-selected server is NOT a Welcome Discovery URL. The Welcome Discovery | ||
| // branch in reloadWebView intentionally runs before the frontdoor short-circuit. | ||
| assertFalse(LoginActivity.isSalesforceWelcomeDiscoveryUrlPath( | ||
| (SalesforceSDKManager.getInstance().loginServerManager.selectedLoginServer?.url ?: "").toUri())) | ||
|
|
||
| // Call reloadWebView | ||
| viewModel.reloadWebView() | ||
| testDispatcher.scheduler.advanceUntilIdle() | ||
|
|
@@ -786,6 +794,107 @@ class LoginViewModelTest { | |
| assertEquals("frontDoorBridgeUrl should still be front door URL", frontDoorUrl, viewModel.frontDoorBridgeUrl.value) | ||
| } | ||
|
|
||
| @Test | ||
| fun test_givenLoginServerManagerSelectedServerIsWelcomeDiscovery_whenReloadWebView_thenLoginUrlIsDiscoveryMobileUrl() { | ||
| val loginServerManager = SalesforceSDKManager.getInstance().loginServerManager | ||
| loginServerManager.addCustomLoginServer("Welcome", WELCOME_LOGIN_URL) | ||
| // Simulate Phase 2: VM's selectedServer is the discovered My Domain even though | ||
| // LoginServerManager still has Welcome Discovery selected. | ||
| viewModel.selectedServer.value = "https://acme.my.salesforce.com" | ||
| testDispatcher.scheduler.advanceUntilIdle() | ||
|
|
||
| viewModel.reloadWebView() | ||
| testDispatcher.scheduler.advanceUntilIdle() | ||
|
|
||
| val resultUri = viewModel.loginUrl.value?.toUri() | ||
| assertNotNull(resultUri) | ||
| assertTrue(LoginActivity.isSalesforceWelcomeDiscoveryMobileUrl(resultUri!!)) | ||
| assertEquals(viewModel.oAuthConfig.consumerKey, | ||
| resultUri.getQueryParameter(LoginActivity.SALESFORCE_WELCOME_DISCOVERY_MOBILE_URL_QUERY_PARAMETER_KEY_CLIENT_ID)) | ||
| assertFalse(viewModel.loginUrl.value!!.contains("/services/oauth2/authorize")) | ||
| } | ||
|
|
||
| @Test | ||
| fun test_givenLoginServerManagerSelectedServerIsNotWelcomeDiscovery_whenReloadWebView_thenLoginUrlIsAuthUrl() { | ||
| // LoginServerManager defaults to Production (login.salesforce.com). | ||
| viewModel.selectedServer.value = "https://login.salesforce.com" | ||
| testDispatcher.scheduler.advanceUntilIdle() | ||
|
|
||
| viewModel.reloadWebView() | ||
| testDispatcher.scheduler.advanceUntilIdle() | ||
|
|
||
| val url = viewModel.loginUrl.value | ||
| assertNotNull(url) | ||
| assertTrue(url!!.contains("/services/oauth2/authorize")) | ||
| assertFalse(LoginActivity.isSalesforceWelcomeDiscoveryUrlPath(url.toUri())) | ||
| } | ||
|
|
||
| @Test | ||
| fun test_givenLoginServerManagerSelectedServerIsWelcomeDiscovery_whenIsUsingFrontDoorBridge_andReloadWebView_thenLoginUrlIsDiscoveryMobileUrl() { | ||
| val loginServerManager = SalesforceSDKManager.getInstance().loginServerManager | ||
| loginServerManager.addCustomLoginServer("Welcome", WELCOME_LOGIN_URL) | ||
| val frontdoorUrl = "https://example.my.salesforce.com/secur/frontdoor.jsp?sid=fake" | ||
| viewModel.loginWithFrontDoorBridgeUrl(frontdoorUrl, pkceCodeVerifier = null) | ||
| assertTrue(viewModel.isUsingFrontDoorBridge) | ||
| testDispatcher.scheduler.advanceUntilIdle() | ||
|
|
||
| viewModel.reloadWebView() | ||
| testDispatcher.scheduler.advanceUntilIdle() | ||
|
|
||
| val resultUri = viewModel.loginUrl.value?.toUri() | ||
| assertNotNull(resultUri) | ||
| assertTrue(LoginActivity.isSalesforceWelcomeDiscoveryMobileUrl(resultUri!!)) | ||
| assertNotEquals(frontdoorUrl, viewModel.loginUrl.value) | ||
| } | ||
|
|
||
| /** | ||
| * Regression guard for the Phase-2 reload bug: when the user is in Phase 2 of the Welcome | ||
| * Discovery flow, viewModel.selectedServer is the discovered My Domain (NOT the Welcome URL), | ||
| * but LoginServerManager retains Welcome as the user's actual server selection. Reload must | ||
| * return the WebView to Phase 1. | ||
| */ | ||
| @Test | ||
| fun test_givenInPhase2OfWelcomeDiscovery_whenReloadWebView_thenLoginUrlReturnsToPhase1() { | ||
| val loginServerManager = SalesforceSDKManager.getInstance().loginServerManager | ||
| loginServerManager.addCustomLoginServer("Welcome", WELCOME_LOGIN_URL) | ||
| // Phase 2 state: VM mirrors the My Domain from applySalesforceWelcomeLoginHintAndHost. | ||
| val myDomainUrl = "https://acme.my.salesforce.com" | ||
| viewModel.selectedServer.value = myDomainUrl | ||
| testDispatcher.scheduler.advanceUntilIdle() | ||
|
|
||
| viewModel.reloadWebView() | ||
| testDispatcher.scheduler.advanceUntilIdle() | ||
|
|
||
| val resultUri = viewModel.loginUrl.value?.toUri() | ||
| assertNotNull(resultUri) | ||
| assertEquals("welcome.salesforce.com", resultUri!!.host) | ||
| assertEquals(LoginActivity.SALESFORCE_WELCOME_DISCOVERY_URL_PATH, resultUri.path) | ||
| assertFalse(viewModel.loginUrl.value!!.contains(myDomainUrl)) | ||
| assertFalse(viewModel.loginUrl.value!!.contains("/services/oauth2/authorize")) | ||
|
|
||
| // Reload must also realign VM-level selectedServer back to the Welcome URL so the top | ||
| // app bar title (defaultTitleText reads selectedServer.value) and the Compose menu | ||
| // gating ("Login for Admin" hidden when selectedServer is the discovery URL) follow. | ||
| assertEquals(WELCOME_LOGIN_URL, viewModel.selectedServer.value) | ||
| } | ||
|
|
||
| @Test | ||
| fun test_givenConsumerKeyAndAppVersion_whenGenerateSalesforceWelcomeDiscoveryMobileUrl_thenReturnsExpectedUrl() { | ||
| val input = "https://welcome.salesforce.com/discovery".toUri() | ||
| val result = viewModel.generateSalesforceWelcomeDiscoveryMobileUrl(input) | ||
|
|
||
| assertEquals("welcome.salesforce.com", result.host) | ||
| assertEquals("/discovery", result.path) | ||
| assertEquals(viewModel.oAuthConfig.consumerKey, | ||
| result.getQueryParameter(LoginActivity.SALESFORCE_WELCOME_DISCOVERY_MOBILE_URL_QUERY_PARAMETER_KEY_CLIENT_ID)) | ||
| assertEquals(URLEncoder.encode(SalesforceSDKManager.getInstance().appVersion, "utf8"), | ||
| result.getQueryParameter(LoginActivity.SALESFORCE_WELCOME_DISCOVERY_MOBILE_URL_QUERY_PARAMETER_KEY_CLIENT_VERSION)) | ||
| assertEquals("sfdc://discocallback", | ||
| result.getQueryParameter(LoginActivity.SALESFORCE_WELCOME_DISCOVERY_MOBILE_URL_QUERY_PARAMETER_KEY_CALLBACK_URL)) | ||
| // Companion validator round-trip: | ||
| assertTrue(LoginActivity.isSalesforceWelcomeDiscoveryMobileUrl(result)) | ||
| } | ||
|
|
||
| @Test | ||
| fun reloadWebView_WithUserAgentFlow_SetsAboutBlankFirst() { | ||
| try { | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The Compose-layer visibility gate (
isWelcomeDiscoveryServerinLoginView.kt) is keyed offviewModel.selectedServer, while this programmatic guard is keyed offLoginServerManager.selectedLoginServer. These are intentionally different state sources (documented well inreloadWebView), but they create a subtle window: in phase 2 the Compose layer correctly shows the LFA item (VM'sselectedServeris the My Domain), yet if this guard fires anyway (e.g. in a race or from a direct external call), the action silently no-ops while the user sees the item as active. A brief inline comment here noting the dual-source-of-truth intent would help future readers understand why the two guards differ rather than assuming one is stale.