Allow consumer key to be defined at runtime by application by wmathurin · Pull Request #3946 · forcedotcom/SalesforceMobileSDK-iOS

wmathurin · 2025-10-25T01:17:19Z

(Apologies for the large PR)

Motivations:

Allow a seamless move to Beacon apps for SApp and other internal apps.
Enable 3rd party apps that need to connect to different orgs with different consumer keys (more important now with the recent changes to Connected App and with the push towards External Client Apps).

Overview:

Application can optionally provide a lambda/block bootConfigRuntimeSelector to SalesforceManager
This lambda/block will be invoked during login with the selected login host passed in and should return the desired configuration via a callback
The default configuration (bootconfig) is used when this lambda/block is not defined or if it returns nil
Tests were added in SalesforceSDKManagerTests.m and SFUserAccountManagerTests.m
A new sample AuthFlowTester was added which allows manual testing of the new functionality - it will become our go to sample app for login related changes (e.g. the upcoming refresh token migration support)
CI scripts were updated: the new app should be built during PR and nightly builds

Screenshots from new sample app:

…e dynamic config (WIP)

…g or some dynamic config (WIP)" This reverts commit 7356c32.

Made sections more readable / collapsable etc

…given the login host and will return the desired app config.

Also showing scopes in config screen and allowing user to enter scopes for dynamic config

NB: using "instant login" for authenticated tests

github-actions · 2025-10-25T01:18:57Z

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

Generated by 🚫 Danger

github-actions · 2025-10-25T01:23:00Z

	1 Warning
⚠️	Static Analysis found an issue with one or more files you modified. Please fix the issue(s).

Clang Static Analysis Issues

File	Type	Category	Description	Line	Col
SFUserAccountManager	Nullability	Memory error	Null passed to a callee that requires a non-null 2nd parameter	1482	15
SFUserAccountManager	Nullability	Memory error	Null passed to a callee that requires a non-null 2nd parameter	1497	15
SFUserAccountManager	Nullability	Memory error	nil passed to a callee that requires a non-null 2nd parameter	2129	13
SalesforceSDKManager	Nil value used as mutex for @synchronized() (no synchronization will occur)	Logic error	Nil value used as mutex for @synchronized() (no synchronization will occur)	144	5
SalesforceSDKManager	Nil value used as mutex for @synchronized() (no synchronization will occur)	Logic error	Nil value used as mutex for @synchronized() (no synchronization will occur)	156	5

Generated by 🚫 Danger

wmathurin · 2025-10-25T01:23:09Z

libs/SalesforceSDKCore/SalesforceSDKCore/Classes/UserAccount/SFUserAccountManager.m

-    request.oauthClientId = self.oauthClientId;
-    request.oauthCompletionUrl = self.oauthCompletionUrl;
-    request.scopes = self.scopes;
+    request.oauthClientId = appConfig != nil ? appConfig.remoteAccessConsumerKey : self.oauthClientId;


That's the heart of the change

I don't see it in the MDM docs, but I noticed that we have the consumer key and callback URL as managed preferences that get set on the manager here. Do we still support that and should the runtime one take precedence if we do?

It's a bit messy. SFUserAccountManager has clientId/callback URL etc on the shared instance / which is backed by prefs and initially populated from the static bootconfig.
For login, I now allow the app to inject alternate values in which case the shared instance values / prefs are not used but left alone (but they are not overridden either). The client id etc used during login is saved in the user account creds (that's not a new behavior) so will be used during refresh.

Also I'm thinking of having the new block take a callback to allow the app to do an async operation (e.g. network call) to figure out what client id to use. I believe that's how SApp wants to use it.

Now it's async: see 5ff8ac4

wmathurin · 2025-10-25T01:23:22Z

native/SampleApps/AuthFlowTester/AuthFlowTesterUITests/TestHelper.swift

+
+        // Configure launch arguments
+        app.launchArguments = [
+            "-creds", credentialsString


Using instant login !

wmathurin · 2025-10-25T01:24:34Z

native/SampleApps/AuthFlowTester/AuthFlowTesterUITests/TestHelper.swift

+import XCTest
+
+/// Helper functions for AuthFlowTester UI Tests
+class TestHelper {


If we do more UI tests in the main repo, we should move that class out to SalesforceSDKCore (it complements TestSetupUtils).

codecov · 2025-10-25T01:28:17Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.05%. Comparing base (1649d0d) to head (90df65a).
⚠️ Report is 35 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #3946      +/-   ##
==========================================
- Coverage   63.32%   62.05%   -1.27%     
==========================================
  Files         250      249       -1     
  Lines       22548    22472      -76     
==========================================
- Hits        14278    13946     -332     
- Misses       8270     8526     +256

Components	Coverage Δ
Analytics	`70.78% <ø> (ø)`
Common	`71.09% <ø> (+0.51%)`	⬆️
Core	`51.58% <68.36%> (-2.14%)`	⬇️
SmartStore	`73.66% <ø> (ø)`
MobileSync	`87.76% <ø> (+0.10%)`	⬆️

Files with missing lines	Coverage Δ
...forceSDKCore/Classes/Common/SalesforceSDKManager.m	`59.55% <100.00%> (+1.73%)`	⬆️
...lesforceSDKCore/Classes/OAuth/JwtAccessToken.swift	`72.22% <100.00%> (-8.34%)`	⬇️
...SDKCore/Classes/UserAccount/SFUserAccountManager.m	`36.07% <100.00%> (-3.14%)`	⬇️

... and 42 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Static configuration can be edited Took out broken new tests (will bring them back if they make sense)

- changed SFSDKAppConfigRuntimeSelectorBlock to take a callback parameter - renamed runtimeSelectedAppConfig to appConfigForLoginHost and it now takes a callback parameter - moved the call to appConfigForLoginHost in SFUserAccountManager to the async part of authenticateWithRequest (leaving the building of the auth request unchanged from before this PR) - updated tests / sample app

It's not high value and we will be back in that code when we will add support for refresh token migration

bbirman · 2025-10-30T17:05:04Z

native/SampleApps/AuthFlowTester/AuthFlowTester/Supporting Files/AuthFlowTester.entitlements

+<dict>
+	<key>keychain-access-groups</key>
+	<array>
+		<string>$(AppIdentifierPrefix)com.salesforce.mobilesdk.sample.authflowtester</string>


Are we going to use the app for shared keychain testing? Asking because a lot of our apps have a shared keychain set by default but if we don't need it, it's easier to do on-device testing if we don't specify one

Are you saying it would be better to remove this?

👍 (unless you need it)

Have the default bootconfig be the "old" one (opaque token) and the dynamic one be the "new" one (jwt based token)

brandonpage · 2025-11-03T19:10:56Z

native/SampleApps/AuthFlowTester/AuthFlowTesterUITests/AuthFlowTesterAuthenticatedUITests.swift

+//    func testSessionDetailScreenIsVisible() throws {
+//        // Verify we're on the session detail screen (not config picker)
+//        XCTAssertTrue(app.navigationBars["AuthFlowTester"].waitForExistence(timeout: 10))
+//        
+//        // Should see authenticated UI elements
+//        XCTAssertTrue(app.buttons["Revoke Access Token"].waitForExistence(timeout: 5))
+//        XCTAssertTrue(app.buttons["Make REST API Request"].waitForExistence(timeout: 5))
+//
+//    }
+
+    //
+    // TODO write more tests
+    //


Should this be in the PR?

I'm not sure these tests on the test app are very useful overall.
We probably want to build robust UI tests (most likely in the UI-Tests repo) that use the test app to test the various login flows.
I can leave the tests in for that PR but might remove them entirely in some future PR (before we tag for the next release).

Or I can remove them now?

Either way is fine by me if we have stories test this in the other repro. Do you want me to create them?

I think I will remove them from here. The code still lives in my fork / branch so could be used for inspiration for the UI-Tests work. Please go ahead and create the new stories.

Removed the UI tests with 90df65a

Done. W-20117061 and W-20117067.

brandonpage

LGTM! Test failures are concerning though.

We are planning to build robust UI tests that will leverage AuthFlowTester to test various login flows etc

wmathurin · 2025-11-04T22:04:08Z

There are a couple of test failures in SalesforceSDKCore on CI. I don't think they are related to the change.
I'll merge this PR in - I have another one lined-up after. I'll have another look at the tests then.

wmathurin added 14 commits October 20, 2025 18:16

Basic methods to override consumer key / revert to bootconfig

ff26d73

Showing buttons before login to pick between static bootconfig or som…

7356c32

…e dynamic config (WIP)

Revert "Showing buttons before login to pick between static bootconfi…

825f2e8

…g or some dynamic config (WIP)" This reverts commit 7356c32.

New sample app AuthFlowTester

c595468

AuthFlowTester now showing session details

4aa7a09

Enriching AuthFlowTester application

cf2033d

Removing duplicate in RestAPIExplorer project

d8971ab

Added revoke access token button

414d432

Made sections more readable / collapsable etc

Merge from dev

abb0366

Instead of passing a consumer key, apps pass in a block that will be …

5fae385

…given the login host and will return the desired app config.

Using SalesforceManager.shared.bootConfigRuntimeSelector

3fa16e5

Also showing scopes in config screen and allowing user to enter scopes for dynamic config

New unit tests

2565980

(Incomplete) Tests for AuthFlowTester

9ec1996

NB: using "instant login" for authenticated tests

Building new sample app and running new UI tests in CI

ed39a21

wmathurin commented Oct 25, 2025

View reviewed changes

wmathurin added 3 commits October 24, 2025 18:52

Masking client id in JWT payload details

d22f67f

Added explaining comment in SalesforceSDKManager

0fc755c

Static configuration can be edited Took out broken new tests (will bring them back if they make sense)

wmathurin marked this pull request as ready for review October 27, 2025 23:01

Taking out one of the new tests - for now

2cdd53a

It's not high value and we will be back in that code when we will add support for refresh token migration

wmathurin requested review from JohnsonEricAtSalesforce and brandonpage October 29, 2025 22:36

bbirman reviewed Oct 30, 2025

View reviewed changes

Removing keychain group

485997a

Have the default bootconfig be the "old" one (opaque token) and the dynamic one be the "new" one (jwt based token)

wmathurin requested a review from bbirman November 3, 2025 17:45

brandonpage reviewed Nov 3, 2025

View reviewed changes

brandonpage approved these changes Nov 3, 2025

View reviewed changes

Removing UI tests for the new test app AuthFlowTester

90df65a

We are planning to build robust UI tests that will leverage AuthFlowTester to test various login flows etc

wmathurin merged commit ecb52c4 into forcedotcom:dev Nov 4, 2025
26 of 32 checks passed

Conversation

wmathurin commented Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivations:

Overview:

Screenshots from new sample app:

Uh oh!

github-actions bot commented Oct 25, 2025

Uh oh!

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

Uh oh!

Clang Static Analysis Issues

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bbirman Oct 27, 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!

codecov bot commented Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

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!

brandonpage left a comment

Choose a reason for hiding this comment

Uh oh!

wmathurin commented Nov 4, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wmathurin commented Oct 25, 2025 •

edited

Loading

github-actions bot commented Oct 25, 2025 •

edited

Loading

bbirman Oct 27, 2025 •

edited

Loading

codecov bot commented Oct 25, 2025 •

edited

Loading