@W-19461869: [M1][MSDK13.1] Enable Flexible Server Matching for QR Code Login (Android) #2762

JohnsonEricAtSalesforce · 2025-09-10T02:45:47Z

🎸 Ready For Final Review 🥁

This is a counterpart to the same set of fixes and tests for MSDK iOS. The only significant difference is that this is all AI generated. The tools did a really nice job taking a prompt from the iOS update and reflecting that in the Android logic, though I did massage a few naming conventions to better fit the Android module's naming. This also brings the structure of QR Code Login much more in-line with the code review conversation we've had on the iOS side, which has yielded a lot of benefit compared to the earlier structure that was still in use on the Android side.

I'll add some detail highlights in the diff.

This has all been tested with this code cherry picked into the latest Salesforce app build and MSDK branch against two Salesforce app test orgs. One of them has advanced authentication enabled and the other does not. Everything works now in those tests, especially where advanced authentication had some troubles. The new validation alerts and the server matching are also working just like iOS does.

The existing tests have been updated and pass. Code coverage is likely still low while I await generated tests on the new classes.

…de Login (Android)

github-actions · 2025-09-10T02:51:23Z

	1 Warning
⚠️	libs/SalesforceSDK/src/com/salesforce/androidsdk/config/LoginServerManager.java#L348 - Use of this function is discouraged because resource reflection makes it harder to perform build optimizations and compile-time verification of code. It is much more efficient to retrieve resources by identifier (e.g. `R.foo.bar`) than by name (e.g. `getIdentifier("bar", "foo", null)`).

Generated by 🚫 Danger

…de Login (Android) (Correct `LoginActivityTests`)

…de Login (Android) (`FrontdoorBridgeLoginOverrideTest` And `FrontdoorBridgeLoginOverride`)

…de Login (Android) (Correct String Resources)

github-actions · 2025-09-10T03:43:01Z

	1 Warning
⚠️	Big PR, try to keep changes smaller if you can.

Generated by 🚫 Danger

JohnsonEricAtSalesforce · 2025-09-10T03:44:05Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/config/LoginServerManager.java

 * @author bhariharan
 */
-public class LoginServerManager {
+public class LoginServerManager implements LoginServerManaging {


@wmathurin - Here's a comparable interface for the protocol we added on iOS. It does really help the tests and logic objects remain consistent.

JohnsonEricAtSalesforce · 2025-09-10T03:44:43Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/FrontdoorBridgeLoginOverride.kt

+ * For Salesforce Identity UI Bridge API support, an overriding front door
+ * bridge URL to use in place of the default initial URL.
+ */
+internal class FrontdoorBridgeLoginOverride(


@wmathurin - This is nearly identical to the iOS version we created.

This class encapsulates the otherwise separate front-door bridge properties that were attached to unrelated classes along with all the logic and tests that has accumulated around them.

I really don't love the very overly documented generated code :/

JohnsonEricAtSalesforce · 2025-09-10T03:46:38Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/FrontdoorBridgeUrlAppLoginServerMatch.kt

+import com.salesforce.androidsdk.config.LoginServerManaging
+import java.net.URL
+
+internal data class FrontdoorBridgeUrlAppLoginServerMatch(


@wmathurin - This is another bit of logic we can use as-is from the iOS code along with all its tests.

Why is this a class and not a function that returns a single boolean? It is instantiated just so appLoginServerMatch can be checked once.

Seems reasonable. @wmathurin - Thinking of the way FrontdoorBridgeLoginOverride grew over time from a single property on an existing class to an entire class of its own in your reviews from iOS, now we have this new FrontdoorBridgeUrlAppLoginServerMatch type which we broke out in turn. It has humble enough beginnings (for now) to make a stand-alone function as Brandon suggested. Do you see this type getting more complex over time? If not, we can do as Brandon suggested easily enough.

JohnsonEricAtSalesforce · 2025-09-10T03:48:47Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/LoginActivity.kt

                    // Fetch well known config and load in custom tab if required.
                    fetchAuthenticationConfiguration {
-                        if (isBrowserLoginEnabled) {
+                        if (isBrowserLoginEnabled && viewModel.frontdoorBridgeLoginOverride == null /* Browser-based authentication is applicable when not authenticating with a front-door bridge URL */) {


For Android and just like iOS, this is the key fix to enable a front-door bridge URL generated by an advanced authentication app to bypass showing the unneeded native browser. The user does get logged in successfully when the front door bridge URL is loaded in the default web view.

So if the app has QR Code login setup it disables Advanced Authentication?

Also, please move the comment from inside the if to a separate line.

I'll get that comment moved. 👍🏻

frontdoorBridgeLoginOverride is only set during the lifetime of a front door bridge login event. At the end of the event, resetFrontDoorBridgeUrl is called and all properties related to front door use are cleared (conveniently this is a single object now based on Wolf's iOS reviews). After attempting the QR Code, the advanced authentication logic is as it was before.

For additional context, users/apps with advanced authentication can authenticate via a front door bridge URL without MSDK needing to show the browser. This new condition prevents MSDK from displaying the unneeded browser while behind it the user would already be authenticated successfully from the front door bridge URL.

github-actions · 2025-09-10T03:48:52Z

Job Summary for Gradle

Pull Request :: test-android

Gradle Root Project	Requested Tasks	Gradle Version	Build Outcome
SalesforceMobileSDK-Android	libs:SalesforceSDK:lint	8.14.2	✅
SalesforceMobileSDK-Android	libs:SalesforceSDK:assembleAndroidTest	8.14.2	❌
SalesforceMobileSDK-Android	libs:SalesforceSDK:convertCodeCoverage	8.14.2	✅

wmathurin

LGTM!

…de Login (Android) (Correct Assertions For `FrontdoorBridgeLoginOverrideTest` And `FrontdoorBridgeLoginOverride`)

JohnsonEricAtSalesforce · 2025-09-11T00:00:10Z

Today's commit updates the newly generated tests to resolve issues the generator had when using "Mockk" plus resolves one string comparison issue with the My Domain soft-matching that the tests revealed 🥁 There's almost no functional change since yesterday's approval by @wmathurin, so this should be merge-ready.

wmathurin · 2025-09-11T00:15:06Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/FrontdoorBridgeUrlAppLoginServerMatch.kt

+                val url = URL(selectedAppLoginHost)
+                results.add(url.host)
+            } catch (_: Exception) {
+                // If parsing fails, try to use as-is (might already be just a host)


When do you think it would not parse but should still be used?

Of all the interesting things the AI tooling did that worked and didn't, this was the oddest choice - Or so I thought! 🥴. Nothing in my prompt or the existing logic suggested this, from what I saw. I was already planning on removing this before merging.

wmathurin · 2025-09-11T00:19:52Z

.../test/SalesforceSDKTest/src/com/salesforce/androidsdk/ui/FrontdoorBridgeLoginOverrideTest.kt

+    @Test
+    fun testEdgeCase_VeryLongCodeVerifier() {
+        // Arrange
+        val longCodeVerifier = "a".repeat(1000) // Very long code verifier


Isn't the code verifier a base64-encoded 128-byte randomly generated key?
It should always be the same length. That test probably does not make sense?

This test doesn't make sense, agreed. MSDK only passes this value through and I don't see there's any length restriction that should have prompted the tool to create this test. I'll remove it.

wmathurin

LGTM - I went through the tests and found only one to be maybe unnecessary?

brandonpage · 2025-09-11T00:51:37Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/config/RuntimeConfig.java

    private static RuntimeConfig INSTANCE = null;

-    RuntimeConfig(Context ctx) {
+    @VisibleForTesting(otherwise = PACKAGE_PRIVATE)


I did not know you could could use the Kotlin style named parameter syntax in a Java file. Must be because it is an annotation?

I recall using named parameters with Java annotations all the way back to Java 5 and Hibernate ORM (2005), which used it a lot. That's an interesting place where named parameters snuck in early and I'd never thought about it 🙂

brandonpage · 2025-09-11T01:03:28Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/FrontdoorBridgeLoginOverride.kt

+        val frontdoorBridgeUrlComponents = frontdoorBridgeUrl.toString()
+        val frontdoorBridgeUri = frontdoorBridgeUrlComponents.toUri()


We take in frontdoorBridgeUrl as a Uri to make a frontdoorBridgeUrlComponents String... and then only use that to make frontdoorBridgeUri which is a Uri.

Am I missing something or is this completely pointless?

That is indeed completely pointless and I hadn't gotten that far in reviewing the generated output. Thanks for finding it.

brandonpage · 2025-09-11T01:22:44Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/FrontdoorBridgeLoginOverride.kt

+     * allowing the addition and switching of app login servers.  This is
+     * intended for test automation only
+     */
+    addingAndSwitchingLoginServersPerMdm: Boolean = true,


NIT: Something like mdmControlledServers might be better?

brandonpage · 2025-09-11T01:27:05Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/FrontdoorBridgeLoginOverride.kt

+        }
+
+        // Check if the front door URL host matches the app's selected login server
+        val addingAndSwitchingLoginServersAllowedResolved = if (!addingAndSwitchingLoginServersPerMdm) {


NIT: Unnecessary !. Just flip the if and else blocks so it is easier for a human to read.

brandonpage · 2025-09-11T01:28:32Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/FrontdoorBridgeLoginOverride.kt

+    /**
+     * The preference for allowing the addition and switching of app login
+     * servers when the MDM preference is ignored.  This is intended for test
+     * automation only
+     */
+    addingAndSwitchingLoginServerOverride: Boolean = false


Why (even for testing) would we ever want to ignore a hard requirement from an MDM?

If this is never set outside of tests why does it exists. Why test both true and false in tests if it can only ever be false in production?

It is just there for testing, true. It's use is paired with addingAndSwitchingLoginServerOverride which lets the test choose the test value. An alternate approach could be to pop and interface on RuntimeConfig and mock it much as we did for LoginServerManager.

brandonpage · 2025-09-11T02:14:52Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/FrontdoorBridgeLoginOverride.kt

+            } catch (_: Exception) {
+                null
+            }
+            return !onlyShowAuthorizedServers && (mdmLoginServers?.isEmpty() != false)


Seems like neither of these values tell us if adding is allowed. Just if switching is allowed/possible.

OnlyShowAuthorizedHosts: If true, prevents users from modifying the list of hosts that the Salesforce mobile app can connect to. See doc.

Hmmm, OnlyShowAuthorizedHosts seems poorly named but fair enough. Maybe we should add a code comment stating that?

brandonpage · 2025-09-11T02:17:06Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/FrontdoorBridgeLoginOverride.kt

+        }
+    }
+
+    private val addingAndSwitchingLoginServersAllowed: Boolean


NIT: Why are these private vals at the bottom of the file?

brandonpage · 2025-09-11T03:15:03Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/LoginActivity.kt

    @Suppress("MemberVisibilityCanBePrivate")
    fun loginWithFrontdoorBridgeUrl(
-        frontdoorBridgeUrl: String,
+        frontdoorBridgeUrl: Uri,


This breaks semantic versioning.

brandonpage · 2025-09-11T03:19:21Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/LoginActivity.kt

                    // Fetch well known config and load in custom tab if required.
                    fetchAuthenticationConfiguration {
-                        if (isBrowserLoginEnabled) {
+                        if (isBrowserLoginEnabled && viewModel.frontdoorBridgeLoginOverride == null /* Browser-based authentication is applicable when not authenticating with a front-door bridge URL */) {


So if the app has QR Code login setup it disables Advanced Authentication?

Also, please move the comment from inside the if to a separate line.

brandonpage · 2025-09-11T03:23:03Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/LoginActivity.kt

-            val frontdoorBridgeUrl: String,
+            val frontdoorBridgeUrl: Uri,


Again, semver.

brandonpage · 2025-09-11T03:25:37Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/LoginViewModel.kt

-        if (!isUsingFrontDoorBridge) {
-            // The Web Server Flow code challenge makes the authorization url unique each time,
-            // which triggers recomposition.  For User Agent Flow, change it to blank.
-            if (!SalesforceSDKManager.getInstance().useWebServerAuthentication) {
-                loginUrl.value = ABOUT_BLANK
-            }


Revert this! This is a bug.

That one was a hallucination in the tool 🤖 and I'll revert it.

brandonpage · 2025-09-11T03:28:15Z

libs/SalesforceSDK/src/com/salesforce/androidsdk/ui/LoginViewModel.kt

-        frontdoorBridgeServer = with(URI(frontdoorBridgeUrl)) { "${scheme}://${host}" }
-        frontdoorBridgeCodeVerifier = pkceCodeVerifier
-        loginUrl.value = frontdoorBridgeUrl
+        val frontdoorBridgeLoginOverride = FrontdoorBridgeLoginOverride(


Shadowed name. And why are there two? We set the class level one from the Activity. Why are we making one local to this function?

This was also work-in-progress output of the tool that hadn't yet been reviewed 🤖 I'll optimize this ✅

wmathurin

For the tests we we have a front door url and a bunch of login servers, and we want to see which one match, should we use a parametrized test class (like this one.
It would be a lot easier to read, and we could see right away if we are testing all the permutations we wish to test etc.

wmathurin · 2025-09-11T19:56:57Z

...esforceSDKTest/src/com/salesforce/androidsdk/ui/FrontdoorBridgeUrlAppLoginServerMatchTest.kt

+            selectedAppLoginServer = "different.salesforce.com"
+        )
+
+        // Assert - Should match the first server ending with "my.salesforce.com"


Wait. if front door url is: https://company.my.salesforce.com
And login servers are login.salesforce.com, test.my.salesforce.com, custom.my.salesforce.com, then it should soft-match with login.salesforce.com.

@wmathurin - This specific rule is where our notion of "matching" the front-door URL's host to another host needs work. The users I was given by the app cannot log in using the "soft-matched" login server. Only the exact My Domain host that's encoded in the front-door URL by the Identity API will work.

This may be the reason only the logic that creates the QR Code and obtains the UI Bridge front-door URL from the API might be able to know what the appropriate login server. The client code, like we attempted here in MSDK, may not be able to guess the answer. Even if it could, the server should likely own that decision for additional future-proofing in the code and avoiding redundant client implementation.

We should ask the identity team for more information on this.

…de Login (Android) (W.I.P.: Temporarily Disable Flapping Test)

JohnsonEricAtSalesforce · 2025-09-12T01:54:35Z

I'll be doing additional review on this one this evening and tomorrow, in particular since we'll need to vet the soft-matching logic again. I'd recommend holding any additional detailed review until I certify that. Thanks!

…de Login (Android) (W.I.P.: Code Review Updates w/Remaining To-Dos And Disabling Test That Only Fail In CI)

…de Login (Android) (W.I.P.: Correct Tests In `FrontdoorBridgeUrlAppLoginServerMatchTest.kt` With URL Cleanup As Well)

JohnsonEricAtSalesforce · 2025-09-15T16:07:16Z

This pull request was superseded by #2764 until the requirements that were being attempted here are fully defined and testable in a future release. The iOS version of this change was also reverted in forcedotcom/SalesforceMobileSDK-iOS#3917.

JohnsonEricAtSalesforce · 2025-09-15T16:15:23Z

This change has been shelved indefinitely pending systems requirements analysis.

JohnsonEricAtSalesforce self-assigned this Sep 10, 2025

@W-19461869: [M1][MSDK13.1] Enable Flexible Server Matching for QR Co…

a92e929

…de Login (Android)

JohnsonEricAtSalesforce force-pushed the feature/w-19461869_m1-msdk13.1-enable-flexible-server-matching-for-qr-code-login-android branch from 4af6091 to a92e929 Compare September 10, 2025 02:46

JohnsonEricAtSalesforce added 2 commits September 9, 2025 21:00

@W-19461869: [M1][MSDK13.1] Enable Flexible Server Matching for QR Co…

250fe94

…de Login (Android) (Correct `LoginActivityTests`)

@W-19461869: [M1][MSDK13.1] Enable Flexible Server Matching for QR Co…

f4ba7a9

…de Login (Android) (`FrontdoorBridgeLoginOverrideTest` And `FrontdoorBridgeLoginOverride`)

JohnsonEricAtSalesforce requested a review from wmathurin September 10, 2025 03:38

JohnsonEricAtSalesforce marked this pull request as ready for review September 10, 2025 03:39

@W-19461869: [M1][MSDK13.1] Enable Flexible Server Matching for QR Co…

de898b2

…de Login (Android) (Correct String Resources)

JohnsonEricAtSalesforce commented Sep 10, 2025

View reviewed changes

wmathurin approved these changes Sep 10, 2025

View reviewed changes

@W-19461869: [M1][MSDK13.1] Enable Flexible Server Matching for QR Co…

aa5ea4d

…de Login (Android) (Correct Assertions For `FrontdoorBridgeLoginOverrideTest` And `FrontdoorBridgeLoginOverride`)

wmathurin reviewed Sep 11, 2025

View reviewed changes

brandonpage reviewed Sep 11, 2025

View reviewed changes

brandonpage requested changes Sep 11, 2025

View reviewed changes

wmathurin requested changes Sep 11, 2025

View reviewed changes

@W-19461869: [M1][MSDK13.1] Enable Flexible Server Matching for QR Co…

6717f3a

…de Login (Android) (W.I.P.: Temporarily Disable Flapping Test)

JohnsonEricAtSalesforce added 2 commits September 11, 2025 20:12

@W-19461869: [M1][MSDK13.1] Enable Flexible Server Matching for QR Co…

2577239

…de Login (Android) (W.I.P.: Code Review Updates w/Remaining To-Dos And Disabling Test That Only Fail In CI)

@W-19461869: [M1][MSDK13.1] Enable Flexible Server Matching for QR Co…

d9a6f19

…de Login (Android) (W.I.P.: Correct Tests In `FrontdoorBridgeUrlAppLoginServerMatchTest.kt` With URL Cleanup As Well)

JohnsonEricAtSalesforce mentioned this pull request Sep 12, 2025

@W-19628153: [Android][QRCode] When Auth Config "Use the native browser for user authentication on Android" is enabled users logging in via QR code still challenged with browser login #2764

Merged

JohnsonEricAtSalesforce closed this Sep 15, 2025

JohnsonEricAtSalesforce deleted the feature/w-19461869_m1-msdk13.1-enable-flexible-server-matching-for-qr-code-login-android branch September 15, 2025 16:15

		val frontdoorBridgeUrlComponents = frontdoorBridgeUrl.toString()
		val frontdoorBridgeUri = frontdoorBridgeUrlComponents.toUri()

@W-19461869: [M1][MSDK13.1] Enable Flexible Server Matching for QR Code Login (Android) #2762

@W-19461869: [M1][MSDK13.1] Enable Flexible Server Matching for QR Code Login (Android) #2762

Uh oh!

Conversation

JohnsonEricAtSalesforce commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 10, 2025

Job Summary for Gradle

Pull Request :: test-android

Uh oh!

wmathurin left a comment

Choose a reason for hiding this comment

Uh oh!

JohnsonEricAtSalesforce commented Sep 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wmathurin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohnsonEricAtSalesforce Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brandonpage Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohnsonEricAtSalesforce commented Sep 10, 2025 •

edited

Loading

github-actions bot commented Sep 10, 2025 •

edited

Loading

JohnsonEricAtSalesforce Sep 11, 2025 •

edited

Loading

brandonpage Sep 11, 2025 •

edited

Loading