Skip to content

Fix nil-sceneId crash on advanced-auth browser callback for pre-scene logins - #4098

Merged
brandonpage merged 2 commits into
forcedotcom:devfrom
brandonpage:fix-nil-sceneid-advanced-auth-callback
Jul 9, 2026
Merged

Fix nil-sceneId crash on advanced-auth browser callback for pre-scene logins#4098
brandonpage merged 2 commits into
forcedotcom:devfrom
brandonpage:fix-nil-sceneid-advanced-auth-callback

Conversation

@brandonpage

@brandonpage brandonpage commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What

Fixes a crash on the advanced-authentication browser callback for logins that start before any UIScene has connected — i.e. apps that begin login from AppDelegate.didFinishLaunchingWithOptions (hybrid and React Native).

Root cause

When login starts pre-scene, request.scene.session.persistentIdentifier is nil, so SFSDKAuthSession.sceneId was nil. On the ASWebAuthenticationSession completion handler, the SDK built an options dictionary keyed by the scene id (kSFIDPSceneIdKey) and looked the session back up in the authSessions store by that id. A nil key both crashed the dictionary insert and dropped the session from the store.

Fix

  • SFSDKAuthSession.m — synthesize a unique per-session scene id (com.salesforce.mobilesdk.unscopedAuthSession-<UUID>) when no scene is connected, so each scene-less session gets its own authSessions[] key and the browser callback can key back to it.
  • SFOAuthCoordinator.m — add a nil guard when building the callback options dictionary.
  • SFOAuthCoordinatorTests.m — unit tests covering the scene-less login case.

Testing

  • New unit tests for the scene-less path.
  • End-to-end proven on a generated hybrid app with forceAdvancedAuthentication enabled: browser login → token exchange → account + photo fetch all succeed with no crash (previously crashed on the browser callback).

… logins

Apps that trigger login before any UIScene connects (hybrid and React Native
start login from AppDelegate.didFinishLaunchingWithOptions) had a nil sceneId,
which crashed the advanced-auth browser callback when building the
kSFIDPSceneIdKey options dictionary and dropped the session from the
authSessions store.

Synthesize a unique per-session scene id when no scene is connected so each
session gets its own authSessions[] key, and add a nil guard when building the
callback options. Adds unit tests for the scene-less case.
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
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
SFOAuthCoordinator Nullability Memory error nil assigned to a pointer which is expected to have non-null value 121 19
SFOAuthCoordinator Nullability Memory error nil assigned to a pointer which is expected to have non-null value 249 15

Generated by 🚫 Danger

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
TestsPassed ✅SkippedFailed
SalesforceSDKCore iOS ^18 Test Results763 ran763 ✅
TestResult
No test annotations available

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.33%. Comparing base (8c672d1) to head (0ae2950).
⚠️ Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
...lesforceSDKCore/Classes/OAuth/SFOAuthCoordinator.m 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #4098      +/-   ##
==========================================
+ Coverage   67.24%   67.33%   +0.09%     
==========================================
  Files         247      247              
  Lines       21835    21837       +2     
==========================================
+ Hits        14683    14705      +22     
+ Misses       7152     7132      -20     
Components Coverage Δ
Analytics 70.78% <ø> (ø)
Common 70.60% <ø> (-0.10%) ⬇️
Core 60.50% <75.00%> (+0.13%) ⬆️
SmartStore 73.60% <ø> (+0.15%) ⬆️
MobileSync 88.73% <ø> (-0.06%) ⬇️
Files with missing lines Coverage Δ
...SalesforceSDKCore/Classes/OAuth/SFSDKAuthSession.m 90.32% <100.00%> (ø)
...lesforceSDKCore/Classes/OAuth/SFOAuthCoordinator.m 52.87% <66.66%> (+0.13%) ⬆️

... and 12 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The nil-guard lines lived inside the ASWebAuthenticationSession completion
block, which only runs on a live browser callback, so they were unreachable by
unit tests and showed as uncovered in the PR patch coverage.

Extract the option-building into -browserCallbackOptionsForSceneId: (declared in
SFOAuthCoordinator+Internal.h) and add tests for both branches: a non-nil
sceneId is carried under kSFIDPSceneIdKey, and a nil sceneId yields an empty
dictionary instead of crashing on a nil insert. Behavior is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
TestsPassedSkippedFailed ❌️
AuthFlowTester UI Test Results all1 ran1 ❌
TestResult
AuthFlowTester UI Test Results all
AuthFlowTesterUITests.xctest
LegacyLoginTests.testCAOpaque_DefaultScopes_WebServerFlow()❌ failure

@sfdctaka sfdctaka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@brandonpage
brandonpage merged commit e4e8388 into forcedotcom:dev Jul 9, 2026
20 of 23 checks passed
JohnsonEricAtSalesforce added a commit to JohnsonEricAtSalesforce/SalesforceMobileSDK-iOS that referenced this pull request Jul 20, 2026
… callback (unit 43/49)

Semantic re-implementation FROM forcedotcom/dev e4e8388 INTO the ObjC->Swift
migration branch. OAuth advanced-auth (ASWebAuthenticationSession) scene fix.

When advanced-auth login starts before any UIScene has connected (cold launch),
or the weak authSession deallocates before the browser callback fires, sceneId
was nil and got inserted into the URL-handler options dictionary, crashing on
the nil insert and dropping the session from the authSessions store.

Ported into the compiled Swift twins:
- SFSDKAuthSession.swift: synthesize a unique per-session scene id
  (com.salesforce.mobilesdk.unscopedAuthSession-<UUID>) when no scene is
  connected. This also fixes a latent collision in the migration, where the
  non-optional sceneId defaulted to "" and all scene-less sessions shared one
  authSessions[] key.
- SFOAuthCoordinator.swift: browserCallbackOptions(forSceneId:) omits the key
  when the id is nil (empty dict) instead of inserting nil; the URL handler then
  falls back to the default scene. Helper is internal (visible to @testable),
  no new public API.

De-referenced .m files (SFSDKAuthSession.m, SFOAuthCoordinator.m,
SFOAuthCoordinatorTests.m) ref-synced byte-faithful to upstream for clean future
merges; SFOAuthCoordinator+Internal.h is a migration tombstone (skip). The 4 new
tests were also ported to the compiled Swift twin SFOAuthCoordinatorTests.swift.

SDKCore/SmartStore/MobileSync TEST BUILD SUCCEEDED (0 errors, 0 new warnings);
6 SFOAuthCoordinatorTests pass. Marker advanced 42->43 (e4e8388).

Escalation: OAuth advanced-auth/scene callback behavior -- flag in PR.
brandonpage added a commit that referenced this pull request Aug 5, 2026
… logins (#4098) (#4122)

Fix nil-sceneId crash on advanced-auth browser callback for pre-scene logins
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants