Repository navigation
Native Login in SDK debug Login Options + AuthFlowTester Native Login UI tests - #4192
Conversation
…Options (@W-24600763)
…mode and native settings
…tomization hook Superseded by SDK-level debug Native Login options in Login Options.
Debug apps (isDevSupportEnabled) can switch Login Options between Browser and Native authentication. Native mode stores its own client ID, callback URL, community URL, DPoP and hybrid choices (NativeLoginDebugSettings), apart from the browser configuration, and presents the SDK's debug Native Login form. A native login registered by the app with useNativeLogin always wins. - NativeLoginDebugSettings / NativeLoginDebugConfig / AuthenticationMode, exposed as SalesforceManager.debugLoginSettings, with apply(json:) and reset(); reset is part of resetForUITesting - AuthenticationModeView in Login Options, with JSON import - DebugNativeLoginView(Controller) and NativeLoginDiagnostics (redacted per-attempt summary of the native login network calls) - Routing applied at SDK init, on authenticate and when Login Options close; a mode or configuration change restarts authentication - New Localizable strings for the mode picker, native fields and form
…in Options and form Covers persistence apart from the browser config, JSON import, Login Options visibility, diagnostics redaction, the debug form state machine, routing precedence (app-registered native login wins) and restart on mode or configuration change.
…DK debug settings --nativeLoginConfig=<json> and --authenticationMode=native|browser are applied through debugLoginSettings. NativeLoginTests drive the SDK debug form and Login Options import; the DPoP-required rejection now asserts HTTP 400 invalid_request (missing dpop_jkt).
docs/auth token lifecycle section, AuthFlowTester README and screenshots, and a RestAPIExplorer README explaining how to try Native Login from Login Options.
Generated by 🚫 Danger |
Clang Static Analysis Issues
Generated by 🚫 Danger |
|
||||||||||||||
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (61.01%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## dev #4192 +/- ##
==========================================
- Coverage 72.92% 72.60% -0.33%
==========================================
Files 254 258 +4
Lines 23531 24054 +523
==========================================
+ Hits 17160 17464 +304
- Misses 6371 6590 +219
🚀 New features to boost your workflow:
|
…x tests, update AuthFlowTester README
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
… link and Android L-marker note
|
iPhone 17 simulator: Full AuthFlowTester UI suite run locally: all tests pass against the test1 org (new NativeLoginTests plus the existing browser, Community, DPoP, RTR and restart suites). |
JohnsonEricAtSalesforce
left a comment
JohnsonEricAtSalesforce
left a comment
There was a problem hiding this comment.
Request changes on f82a52db01b432508c8ad6058db8f929f9fc7518.
Open Questions
- Should Native Login follow
isDevSupportEnabledin Release, like the existing developer menu, or additionally require DEBUG? The PR says debug-only, but the API documents the runtime flag as its gate; that flag also works in Release. - What explains the remaining CI failures? Core’s host-list restart test expects
test.salesforce.combut getsl, reproduced locally with native registration left by test initialization. Two native UI tests fail to return from Login Options; that transition passed offline locally, so their CI cause remains unclear. - Is there coverage for scene disconnect/replacement and multi-account isolation through the built-in form? The new UI tests exercise idle-form backgrounding/cancellation; prerequisite manager tests do not cover replacement of the debug form’s default registration.
Verification: focused Core 116/117 passed; two offline UI checks passed. The cancellation probe reproduced late account creation for Bearer and DPoP. All 12 live-org native tests skipped locally; the live matrix, full Core/browser suites and manual RestAPIExplorer login remain unverified. Core patch coverage reports 59.16% against an 80% target.
This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.
| /// account when there is one. | ||
| func cancel() { | ||
| stopLogin() | ||
| if canDismiss { |
There was a problem hiding this comment.
[P1] Cancel the SDK attempt even without an existing account
Without an existing account, Cancel skips cancelAction and only cancels the Swift task. Cancellation does not propagate through the Native Login/REST bridge. Holding a successful token response until after Cancel still created and selected an account for both Bearer and DPoP. Invalidate the owning SDK attempt, keep retry working, and add a regression that releases token success after Cancel.
This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.
There was a problem hiding this comment.
Fixed in a30fcff. Cancel now also invalidates the SDK attempt when there is no account (new cancelNativeAuthentications, which resets the in-flight native sessions without dismissing the login screen), so a token response released after Cancel is no longer the current attempt and creates no account; Log In starts a fresh attempt. Regression: test_givenDebugFormWithoutAccount_whenCancelledBeforeTokenSuccess_thenNoAccountAndRetryWorks holds the token response, cancels, releases it (Bearer and DPoP), asserts no account/identity request, then retries successfully. Plus unit tests for the new method (existing account and its DPoP key untouched, browser session kept).
| /// The first problem with this configuration, or nil when Native Login can use it. | ||
| public var validationError: String? { | ||
| if consumerKey.trimmingCharacters(in: .whitespaces).isEmpty { | ||
| return "Client ID is required" |
There was a problem hiding this comment.
[P2] Localize configuration errors
AuthenticationModeView displays these English validation messages and NativeLoginDebugSettingsError.errorDescription directly. Add localization keys for validation/import errors and resolve them through SFSDKResourceUtils.localizedString, including formatted messages.
This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.
There was a problem hiding this comment.
Fixed in a30fcff. Validation and import errors now resolve through SFSDKResourceUtils.localizedString (NATIVE_LOGIN_DEBUG_ERROR_* keys, formatted messages use String(format:)). New strings are called out in the PR description.
| func record(_ event: NativeLoginDiagnosticEvent) { | ||
| guard isEnabled() else { return } | ||
| lock.withLock { recorded.append(event) } | ||
| Self.logger.debug("\(event.summary, privacy: .public)") |
There was a problem hiding this comment.
[P2] Use the SDK logger for diagnostics
os.Logger bypasses SDK log-level controls and the configurable SFLogger receiver. Send the redacted summary through SFSDKCoreLogger so apps receive and control these diagnostics through their existing logging setup.
This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.
There was a problem hiding this comment.
Fixed in a30fcff. Diagnostics now go through SFSDKCoreLogger at debug level; os.Logger is removed.
…e debug errors; log diagnostics through SFSDKCoreLogger; fix host-list test isolation; gating docs
|
Release gating: the feature follows the runtime |
|
Re the two native UI tests that fail to return from Login Options in CI: they pass locally, and the full AuthFlowTester UI suite passes on the iPhone 17 simulator against the test1 org, so we are not investigating them further here. We will look at them if they keep failing on the next CI run. Scene disconnect/replacement is not covered: the test app has a single |
JohnsonEricAtSalesforce
left a comment
JohnsonEricAtSalesforce
left a comment
There was a problem hiding this comment.
Follow-up at a30fcff6198a68d9f5f064f664d9e7076bb895d7: the original late-token cancellation case, localized errors and SDK logging are verified fixed. All 121 focused Core tests pass, as does the original cancellation reproduction. Runtime developer-support gating is clarified. Request changes for the remaining cancellation-isolation issue below.
Open Questions
- Current CI still fails native retry, logout/reauthentication and browser fallback. Retry/fallback snapshots have no login controls; logout retains session details. Offline restart/fallback pass locally. What explains these transitions? An unrelated CI UI timeout does not establish the same cause.
- Core patch coverage remains 60.32% against 80%. The check filters to
SalesforceSDKCore, while UI uploads useAuthFlowTesterUI; the 93.42% combined headline does not clear that gate. Should the gate include UI coverage, or should unit coverage meet it separately? See Codecov flag filtering.
This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.
| dispatch_sync(dispatch_get_main_queue(), ^{ [self cancelNativeAuthentications]; }); | ||
| return; | ||
| } | ||
| for (SFSDKAuthSession *session in self.authSessions.allValues) { |
There was a problem hiding this comment.
[P2] Cancel only the form’s owning native attempt
DebugNativeLoginModel.stopLogin() calls this helper when one form is cancelled, but it resets every active native session. In a deterministic probe, cancelling the first form also removed a separately owned native session and deleted its DPoP key, preventing that login from completing. Scope invalidation to the cancelling form’s captured attempt/scene and add a two-session isolation regression.
This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.
There was a problem hiding this comment.
Fixed. Cancel now invalidates only the attempt the form owns: cancelNativeAuthenticationsForConfiguration: matches an authenticating native session by scene, client ID, callback URL and login site, and resets just that session (its DPoP key and nonce are cleaned up with it). Other native sessions and browser sessions are left alone. Added a regression that runs a browser session and a second native session (different client and login URL) alongside the form's attempt: only the form's attempt is invalidated, and the other native session stays authenticating with its DPoP key. Focused NativeLoginManagerTests, DebugNativeLoginModelTests and LoginForAdminTests pass.
There was a problem hiding this comment.
Verified at 5d0bd60c: late token responses after Cancel no longer create accounts, and cancelling one form preserves other sessions in the regression tests. All 121 affected Core tests pass. This addresses my cancellation finding.
This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.
…add isolation regression
|
On the Core patch coverage question (60.32% vs 80%): that gate is filtered to the |
|
On the CI UI test failures (native retry, logout/reauth and browser fallback): CI has been unreliable for the UI suites, and all of the AuthFlowTester UI tests, including these, pass locally. We'll rely on the CI re-run for the rest. |
JohnsonEricAtSalesforce
left a comment
JohnsonEricAtSalesforce
left a comment
There was a problem hiding this comment.
Verified at 5d0bd60c: late token responses after Cancel no longer create accounts, and cancelling one form preserves other sessions in the regression tests. All 121 affected Core tests pass. This addresses my cancellation finding.
This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.
Adds a developer-support Native Login mode to the SDK's Login Options, so any app with developer support enabled (the default in debug builds, for example RestAPIExplorer) can use Native Login without source changes, plus Native Login UI tests in AuthFlowTester. GUS: W-24600763. Android counterpart: forcedotcom/SalesforceMobileSDK-Android#3065. Builds on the Native Login DPoP work in #4187.
Screenshots
SDK (when developer support is enabled)
useNativeLoginalways takes precedence.SalesforceManager.shared.debugLoginSettings: apply JSON / reset, so automation can configure the mode deterministically.NativeLoginDiagnostics, recorded when developer support is enabled and written to the SDK log throughSFSDKCoreLogger): stage, HTTP status, OAuth error, whether a DPoP proof and nonce were present. No tokens, codes, proofs, nonces or passwords.isDevSupportEnabledflag, which defaults to on in debug builds and off in release builds unless the app enables it. Apps that don't enable it, and Browser mode, are unchanged.AuthFlowTester
NativeLoginTestsandNativeLoginPageObject: the four-case matrix (eca_jwt_headlessandeca_jwt_dpop_headless, client DPoP off/on, with explicit Bearer / DPoP / rejected expectations), plus cancellation, browser fallback, same-user DPoP-to-Bearer reauthentication, scene lifecycle and diagnostics redaction. Successful sessions check feature markers, a REST call, revoke/refresh and a cold restart.--nativeLoginConfig,--authenticationMode) map onto the SDK debug settings.ui_test_config.json.sampleand the AuthFlowTester / RestAPIExplorer READMEs document setup, commands and missing-fixture behavior. docs/auth is updated.For reviewers
SalesforceManager.shared.debugLoginSettings.Localizable.strings: Login Options, the native login form, and the configuration/import error messages (NATIVE_LOGIN_DEBUG_ERROR_*).Testing
NativeLoginDebugSettingsTests(settings persistence and separation, routing and precedence, restart on change, Login Options visibility per mode, form state machine).NativeLoginTestson iPhone 17 simulator against the test1 org: 12/12 pass (four-case matrix, retry after rejection, cancellation, no Cancel without an account, browser fallback, same-user DPoP-to-Bearer, backgrounding, redacted diagnostics, fixture validation).eca_jwt_headless,eca_jwt_dpop_headless,community_authhost); the suite skips when neither ECA is present and fails when only one is.NativeLoginTestsplus the existing browser, Community and DPoP suites).