Skip to content

[iOS] AuthFlowTester: display mainSid/uiSid and assert them in tests - #4157

Merged
wmathurin merged 2 commits into
forcedotcom:devfrom
wmathurin:ui-sid
Sep 1, 2026
Merged

[iOS] AuthFlowTester: display mainSid/uiSid and assert them in tests#4157
wmathurin merged 2 commits into
forcedotcom:devfrom
wmathurin:ui-sid

Conversation

@wmathurin

@wmathurin wmathurin commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • UserCredentialsView: Adds `mainSid` and `uiSid` rows in the Domains and SIDs section and includes them in the JSON export (used by UI tests).
  • AuthFlowTesterMainPageObject: Adds `mainSid` and `uiSid` to `UserCredentialsData` struct and parses them from the exported JSON.
  • BaseAuthFlowTester.assertSIDs: Adds `isDPoP` parameter and new assertions:
    • `mainSid` is always non-empty
    • `uiSid` is non-empty when DPoP is active
    • `mainSid == uiSid` (DPoP), `mainSid == parentSid` (JWT hybrid), or `mainSid == accessToken` (opaque Bearer)

Bug fix: uiSid was never captured in DPoP sessions

`SFOAuthCredentials.updateCredentials:` compared the server-returned token type against the lowercase literal `@"dpop"`, but the server returns `@"DPoP"` (mixed case). The case-sensitive `isEqualToString:` comparison always failed, so `uiSid` was set to `nil` on every DPoP login.

Fix: added `kSFOAuthDPoPTokenType = @"DPoP"` to `SFSDKOAuthConstants.h` and replaced all bare string literals in production code with this constant. Updated `SFOAuthCredentialsTests.m` to pass the correct mixed-case token type the server actually sends.

Note: `DPoPRequestDecorator.isDPoPTokenType(_:)` already performs a case-insensitive comparison (RFC-compliant); the existing test that passes `"dpop"` to that method is intentional and unchanged.

Dependencies

Builds on top of merged PRs:

  • #4154 — feat: capture ui_sid from DPoP token responses
  • #4155 — fix: clear stale uiSid on DPoP-to-Bearer downgrade

Related Android PR: forcedotcom/SalesforceMobileSDK-Android#3020

Test plan

  • DPoP test: `test_givenDPoPHybrid_whenLogin_thenTokenTypeIsDPoPAndRefreshWorks` — verifies uiSid non-empty and mainSid == uiSid
  • JWT + hybrid test: `testECAJwt_DefaultScopes` — verifies mainSid == parentSid
  • Non-DPoP non-JWT test: `testCAOpaque_DefaultScopes_WebServerFlow` — verifies mainSid == accessToken

- UserCredentialsView: add mainSid and uiSid rows in Domains and SIDs section
- AuthFlowTesterMainPageObject: add mainSid/uiSid to UserCredentialsData struct and JSON parsing
- BaseAuthFlowTester.assertSIDs: add isDPoP param; assert mainSid always non-empty, uiSid non-empty on DPoP, and mainSid == uiSid (DPoP) / parentSid (JWT hybrid) / accessToken (opaque) as appropriate
@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown
TestsPassed ☑️SkippedFailed ❌️
AuthFlowTester UI Test Results all111 ran59 ✅52 ❌
TestResult
AuthFlowTester UI Test Results all
AuthFlowTesterUITests.xctest
RefreshTokenMigrationWithRestartTests.testMigrateMultipleUsers_WithRestart()❌ failure
RefreshTokenMigrationWithRestartTests.testMigrateCAToBeacon_WithRestart()❌ failure
RefreshTokenMigrationWithRestartTests.testMigrateCAToECA_WithRestart()❌ failure
RefreshTokenMigrationWithRestartTests.testMigrateBeaconScopeAddition_WithRestart()❌ failure
RefreshTokenMigrationWithRestartTests.testMigrateScopeAddition_WithRestart()❌ failure
WelcomeLoginTests.testWelcomeDiscovery_RegularAuthLoginHost()❌ failure
WelcomeLoginTests.testWelcomeDiscovery_AdvancedAuthLoginHost()❌ failure
WelcomeLoginTests.testWelcomeDiscovery_RegularAuthLoginHost_DynamicConfig()❌ failure
WelcomeLoginTests.testWelcomeDiscovery_AdvancedAuthLoginHost_DynamicConfig()❌ failure
ECALoginTests.testECAJwt_AllScopes()❌ failure
ECALoginTests.testDynamicConfigurationWithInvalidScope()❌ failure
ECALoginTests.testECAOpaque_DefaultScopes()❌ failure
ECALoginTests.testDynamicConfigurationWithInvalidClientId()❌ failure
LoginForAdminTests.testLoginForAdmin_WebServerFlowDisabled()❌ failure
LoginForAdminTests.testLoginForAdmin_WebServerFlowEnabled()❌ failure
DPoPLoginTests.test_givenDPoPECA_whenAdminLogin_thenDPoPBindingWorksThroughBrowser()❌ failure
DPoPLoginTests.test_givenDPoPUser_whenAppRestart_thenSessionAndKeypairSurvive()❌ failure
DPoPLoginTests.test_givenTwoDPoPUsers_whenSwitchAndRefresh_thenTokensAndNoncesAreIsolated()❌ failure
DPoPLoginTests.test_givenDPoP_whenLoginViaPoolServer_thenTokenTypeIsDPoP()❌ failure
DPoPLoginTests.test_givenDPoPNoHybrid_whenLogin_thenTokenTypeIsDPoPAndRefreshWorks()❌ failure
DPoPLoginTests.test_givenDPoPRtr_whenLoginViaPoolServer_thenRefreshTokenSurvivesIdentityFetch()❌ failure
DPoPLoginTests.test_givenDPoPRtrHybrid_whenLogin_thenRefreshTokenRotatesAndDPoPBindingHolds()❌ failure
DPoPLoginTests.test_givenDPoPUser_whenMigrateToDPoPRtr_thenRefreshTokenRotationEnabled()❌ failure
DPoPLoginTests.test_givenDPoPSession_whenDowngradeFromDPoP_thenBearerUnbound()❌ failure
DPoPLoginTests.test_givenDPoPRtrNoHybrid_whenLogin_thenRefreshTokenRotatesAndDPoPBindingHolds()❌ failure
RTRLoginTests.testECAJwtRtr_NoHybrid_WithRestart()❌ failure
RTRLoginTests.testECAJwtRtr_NoHybrid()❌ failure
RTRLoginTests.testECAOpaqueRtr_NoHybrid_WithRestart()❌ failure
ForceAdvancedAuthTests.testForceAdvancedAuth_Disabled_BackAndGearStillPresent()❌ failure
ForceAdvancedAuthTests.testForceAdvancedAuth_Disabled_StandardServer_UsesInAppWebView()❌ failure
ForceAdvancedAuthTests.testForceAdvancedAuth_AddAdditionalUser_BackButtonAccessible()❌ failure
LoginWithRestartTests.testWelcomeDiscovery_WithRestart()❌ failure
LoginWithRestartTests.testECAOpaque_DefaultScopes_WithRestart()❌ failure
LoginWithRestartTests.testECAJwt_SubsetScopes_DynamicConfiguration_WithRestart()❌ failure
LoginWithRestartTests.testBeaconJwt_SubsetScopes_DynamicConfiguration_WithRestart()❌ failure
LegacyLoginTests.testCAOpaque_AllScopes_WebServerFlow_InAppWebView()❌ failure
LegacyLoginTests.testCAOpaque_DefaultScopes_WebServerFlow_InAppWebView()❌ failure
LegacyLoginTests.testCAOpaque_DefaultScopes_WebServerFlow()❌ failure
LegacyLoginTests.testCAOpaque_SubsetScopes_WebServerFlow_InAppWebView()❌ failure
LegacyLoginTests.testCAOpaque_SubsetScopes_WebServerFlow()❌ failure
RefreshTokenMigrationTests.testFlagDiversity_MigratedBeaconJwtVsNonHybridOpaque()❌ failure
RefreshTokenMigrationTests.testMigrateBeacon_AddMoreScopes()❌ failure
RefreshTokenMigrationTests.testMigrateCAToBeaconAndBack()❌ failure
RefreshTokenMigrationTests.testMigrateCAToECA()❌ failure
RefreshTokenMigrationTests.testMigrateOneUserOnly()❌ failure
RefreshTokenMigrationTests.testMigrateCAUserAgentToECAWebServer()❌ failure
MultiUserLoginTests.testFlagDiversity_BeaconNonHybridJwtVsHybridOpaque()❌ failure
MultiUserLoginTests.testBothStatic_SameApp_DifferentScopes()❌ failure
MultiUserLoginTests.testDifferentAppTypes_LogoutCaUser_EcaUserUnaffected()❌ failure
MultiUserLoginTests.testBeaconAndNonBeacon_MultiUser()❌ failure
BeaconLoginTests.testBeaconJwt_SubsetScopes()❌ failure
LegacyLoginTestsNotHybrid.testCAOpaque_AllScopes_WebServerFlow()❌ failure

@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.21%. Comparing base (81bfec4) to head (a10735d).
⚠️ Report is 2 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #4157      +/-   ##
==========================================
+ Coverage   67.44%   71.21%   +3.76%     
==========================================
  Files         254      254              
  Lines       22868    22868              
==========================================
+ Hits        15424    16286     +862     
+ Misses       7444     6582     -862     
Components Coverage Δ
Analytics 70.78% <ø> (ø)
Common 70.88% <ø> (ø)
Core 66.76% <100.00%> (+5.65%) ⬆️
SmartStore 73.45% <ø> (ø)
MobileSync 88.84% <ø> (ø)
Files with missing lines Coverage Δ
...forceSDKCore/Classes/Common/SalesforceSDKManager.m 77.10% <100.00%> (+1.32%) ⬆️
...lesforceSDKCore/Classes/OAuth/SFOAuthCredentials.m 93.11% <100.00%> (+0.98%) ⬆️

... and 31 files with indirect coverage changes

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

@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!

…e case

The token type comparison in updateCredentials: used @"dpop" (lowercase)
but the server returns @"DPoP" (mixed case), so uiSid was never captured
after login even when the server provided it.

Fix: add kSFOAuthDPoPTokenType = @"DPoP" to SFSDKOAuthConstants.h and
use it in SFOAuthCredentials.m and SalesforceSDKManager.m. Update
SFOAuthCredentialsTests.m to pass the correct mixed-case token type that
the server actually sends.

Note: DPoPRequestDecorator.isDPoPTokenType(_:) already performs a
case-insensitive comparison (RFC-compliant), which is why the test at
line 292 of SFSDKDPoPTests that passes "dpop" is intentional and stays.
@github-actions

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
SFOAuthCredentials Dereference of null pointer Logic error Access to instance variable '_credentialsChangeSet' results in a dereference of a null pointer (loaded from variable 'self') 200 27
SFOAuthCredentials Nullability Memory error nil passed to a callee that requires a non-null 1st parameter 306 20
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) 171 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) 183 5

Generated by 🚫 Danger

@github-actions

Copy link
Copy Markdown
TestsPassed ☑️SkippedFailed ❌️
SalesforceSDKCore iOS ^18 Test Results1016 ran1015 ✅1 ❌
TestResult
SalesforceSDKCore iOS ^18 Test Results
SalesforceRestAPITests.testSOQLWithNewLine❌ failure

@wmathurin
wmathurin merged commit eeb6b62 into forcedotcom:dev Sep 1, 2026
21 of 24 checks passed
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.

2 participants