Skip to content

feat(W-23159740): per-user persistent feature flags (iOS) - #4086

Merged
wmathurin merged 23 commits into
forcedotcom:devfrom
wmathurin:W-21167151-feature-flags-per-user
Jun 29, 2026
Merged

feat(W-23159740): per-user persistent feature flags (iOS)#4086
wmathurin merged 23 commits into
forcedotcom:devfrom
wmathurin:W-21167151-feature-flags-per-user

Conversation

@wmathurin

Copy link
Copy Markdown
Contributor

Summary

  • Adds per-user feature flag storage to SFUserAccount via persistedFeatureFlags (NSSet, serialized with NSKeyedArchiver into UserAccount.plist)
  • SFSDKAppFeatureMarkers gains per-user register/unregister/query methods; +appFeaturesForUser: returns global ∪ per-user union; +loadPersistedFeatures:forUser: hydrates in-memory state without writeback
  • SalesforceSDKManager.userAgentString:qualifier:forUser: resolves per-user flags for UA construction; hydratePerUserFeatureFlags populates in-memory state at SDK init
  • SFUserAccountManager.finalizeAuthCompletion: promotes BW/WD/QR transient global flags to per-user after authentication completes
  • SmartStore (SU) and MobileSync (MS) flags now registered per-user
  • 6 new unit tests in SFSDKAppFeatureMarkersTests

GUS Stories

  • W-23159740 (iOS per-user feature flags)
  • W-21167151 (spike)

Test plan

  • SFSDKAppFeatureMarkersTests — 9 tests, 0 failures
  • SalesforceSDKManagerTests — all existing tests pass
  • Build succeeds: xcodebuild build -scheme SalesforceSDKCore -sdk iphonesimulator
  • Integration tests (require sandbox credentials — run in CI)

Implements per-user feature flag storage and hydration so each
user account's active features are accurately reflected in the
User-Agent string across app restarts.

- SFSDKAppFeatureMarkers: +registerAppFeature:forUser:, +unregisterAppFeature:forUser:,
  +appFeaturesForUser: (union of global + per-user), +loadPersistedFeatures:forUser:
- SFUserAccount: persistedFeatureFlags property (NSSet<NSString*>),
  serialized via NSKeyedArchiver into UserAccount.plist
- SalesforceSDKManager: userAgentString:qualifier:forUser: unions global + per-user flags;
  hydratePerUserFeatureFlags called from sharedManager dispatch_once
- SFUserAccountManager: finalizeAuthCompletion promotes BW/WD/QR transient
  global flags to per-user after login completes
- SFSmartStore: SU flag now registered per-user
- SFMobileSyncSyncManager: MS flag now registered per-user
- SFSDKAppFeatureMarkersTests: 6 new tests covering isolation, union,
  nil-user fallback, per-user unregister, hydration, and persistence
@github-actions

github-actions Bot commented Jun 25, 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
SFUserAccount Nullability Memory error nil assigned to a pointer which is expected to have non-null value 266 22
SFUserAccount Nullability Memory error nil passed to a callee that requires a non-null 1st parameter 450 18
SFUserAccount Nullability Memory error nil passed to a callee that requires a non-null 1st parameter 452 18
SFUserAccountManager Nullability Memory error Null passed to a callee that requires a non-null 2nd parameter 1618 15
SFUserAccountManager Nullability Memory error Null passed to a callee that requires a non-null 2nd parameter 1633 15
SFUserAccountManager Nullability Memory error nil passed to a callee that requires a non-null 2nd parameter 2306 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) 145 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) 157 5
SFSmartStore Nullability Memory error nil passed to a callee that requires a non-null 2nd parameter 583 28
SFSmartStore Nullability Memory error nil passed to a callee that requires a non-null 2nd parameter 596 27
SFSmartStore NSError** null dereference Coding conventions (Apple) Potential null dereference. According to coding standards in 'Creating and Returning NSError Objects' the parameter may be null 1349 16
SFSmartStore Nullability Memory error nil passed to a callee that requires a non-null 1st parameter 1425 28

Generated by 🚫 Danger

@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.33962% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.65%. Comparing base (10c1396) to head (af2df34).
⚠️ Report is 27 commits behind head on dev.

Files with missing lines Patch % Lines
...SDKCore/Classes/UserAccount/SFUserAccountManager.m 73.91% 6 Missing ⚠️

❌ Your patch status has failed because the patch coverage (77.88%) 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    #4086      +/-   ##
==========================================
+ Coverage   66.71%   70.65%   +3.93%     
==========================================
  Files         246      246              
  Lines       21541    21620      +79     
==========================================
+ Hits        14372    15275     +903     
+ Misses       7169     6345     -824     
Components Coverage Δ
Analytics 70.78% <ø> (ø)
Common 70.97% <ø> (+0.18%) ⬆️
Core 65.50% <94.23%> (+6.07%) ⬆️
SmartStore 73.44% <100.00%> (-0.16%) ⬇️
MobileSync 88.79% <100.00%> (+0.23%) ⬆️
Files with missing lines Coverage Δ
...bileSync/Classes/Manager/SFMobileSyncSyncManager.m 76.06% <100.00%> (ø)
...rceSDKCore/Classes/Common/SFSDKAppFeatureMarkers.m 100.00% <100.00%> (ø)
...forceSDKCore/Classes/Common/SalesforceSDKManager.m 73.63% <100.00%> (+2.06%) ⬆️
...esforceSDKCore/Classes/UserAccount/SFUserAccount.m 80.76% <100.00%> (+2.47%) ⬆️
libs/SmartStore/SmartStore/Classes/SFSmartStore.m 80.47% <100.00%> (-0.36%) ⬇️
...SDKCore/Classes/UserAccount/SFUserAccountManager.m 61.26% <73.91%> (+16.10%) ⬆️

... and 39 files with indirect coverage changes

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

@github-actions

Copy link
Copy Markdown
TestsPassed ✅SkippedFailed
SmartStore iOS ^26 Test Results177 ran177 ✅
TestResult
No test annotations available

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
TestsPassed ✅SkippedFailed
SalesforceSDKCore iOS ^26 Test Results693 ran693 ✅
TestResult
No test annotations available

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
TestsPassed ☑️SkippedFailed ❌️
MobileSync iOS ^18 Test Results234 ran233 ✅1 ❌
TestResult
MobileSync iOS ^18 Test Results
SyncManagerTests.testCleanResyncGhostsForMRUTarget❌ failure

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
TestsPassed ✅SkippedFailed
MobileSync iOS ^26 Test Results234 ran234 ✅
TestResult
No test annotations available

@github-actions

github-actions Bot commented Jun 25, 2026

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

Comment thread libs/SalesforceSDKCore/SalesforceSDKCore/Classes/Common/SFSDKAppFeatureMarkers.m Outdated
Comment thread libs/SalesforceSDKCore/SalesforceSDKCore/Classes/Common/SFSDKAppFeatureMarkers.m Outdated
- Replace custom SFSDKUserKey() with SFKeyForUserAndScope(user, SFUserAccountScopeUser)
  to reuse the existing codebase convention for user-scoped keys
- Capture per-user flag snapshot inside dispatch_sync block before
  assigning to user.persistedFeatureFlags, eliminating the map read
  outside the queue's protection
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
TestsPassed ☑️Skipped ⚠️Failed ❌️
AuthFlowTester UI Test Results all84 ran65 ✅2 ⚠️17 ❌
TestResult
AuthFlowTester UI Test Results all
AuthFlowTesterUITests.xctest
LoginWithRestartTests.testAdvancedAuth_WithRestart()❌ failure
LoginWithRestartTests.testMultiUserRestart()❌ failure
RTRLoginTests.testECAJwtRtr_NoHybrid()❌ failure
RefreshTokenMigrationTests.testMigrateCAToECA()❌ failure
RefreshTokenMigrationTests.testMigrateOneUserOnly()❌ failure
RefreshTokenMigrationTests.testMigrateBeaconToCA()❌ failure
MultiUserLoginTests.testLogoutUserWithDynamicConfig_OtherUserUnaffected()❌ failure
MultiUserLoginTests.testAdvancedAuthUser_HasBWFlag_RegularAuthUser_DoesNot()❌ failure
MultiUserLoginTests.testBothStatic_SameApp_DifferentScopes()❌ failure
MultiUserLoginTests.testBothStatic_DifferentApps()❌ failure
MultiUserLoginTests.testFirstStatic_SecondDynamic_DifferentApps()❌ failure
MultiUserLoginTests.testRevokeAccessForUserWithDynamicConfig_OtherUserUnaffected()❌ failure
MultiUserLoginTests.testDifferentAppTypes_LogoutCaUser_EcaUserUnaffected()❌ failure
MultiUserLoginTests.testBothStatic_SameApp_SameScopes()❌ failure
MultiUserLoginTests.testBothDynamic_DifferentApps()❌ failure
RefreshTokenMigrationWithRestartTests.testMigrateMultipleUsers_WithRestart()❌ failure
RefreshTokenMigrationWithRestartTests.testMigrateBeaconScopeAddition_WithRestart()❌ failure

…tion to UI tests

- UserCredentialsView: add "SDK" section with User Agent row and JSON export
- AuthFlowTesterMainPageObject: add getUserAgent() via JSON export
- BaseAuthFlowTester: add validateUserAgent(), loginOtherUser(), isMultiUser param
- MultiUserLoginTests: add testAdvancedAuthUser_HasBWFlag_RegularAuthUser_DoesNot
- SFUserAccountManager: fix BW/WD global flag bleed by clearing global flags after
  promoting to per-user in finalizeAuthCompletion (matches existing QR pattern)
@github-actions

Copy link
Copy Markdown
TestsPassed ✅SkippedFailed
SmartStore iOS ^18 Test Results177 ran177 ✅
TestResult
No test annotations available

wmathurin added 12 commits June 25, 2026 14:05
…Agent into public/private overloads

validateUserAgent(loginHost:...) now fetches the UA once and delegates to a
private overload that takes ua: String. validate() calls the private overload
using credentials already loaded, eliminating a second export-button round-trip.
…ode paths

- SFSDKAppFeatureMarkersTests: appFeaturesForUser:nil equivalence, unregister updates persistedFeatureFlags
- SFUserAccountManagerTests: persistedFeatureFlags NSKeyedArchiver encode/decode roundtrip
- SalesforceSDKManagerTests: userAgentString:forUser: with per-user flag, nil-user fallback, hydratePerUserFeatureFlags
- Expose hydratePerUserFeatureFlags in SalesforceSDKManager+Internal.h for testability
…e-deriving from credentials.domain

By auth completion, handleCustomDomainUpdateWithLoginHint:myDomain: has replaced
credentials.domain with the resolved org domain, which no longer contains '/discovery'.
Re-evaluating isDiscoveryDomain at that point always returns false, so the WD flag
was never promoted to per-user. Use the transient global flag (already set by
SFOAuthCoordinator at auth start) as the source of truth, matching the QR flag pattern.
…r BW, WD, QR

Covers the six promotion call patterns from SFUserAccountManager auth-completion
block (lines 2082-2107): BW promoted/cleared on advancedBrowser vs non-advanced
authType, WD promoted/cleared based on global flag presence, QR promoted/cleared
based on global flag presence.
…inForAdmin

LoginForAdminTests use a regular login host but browser-based auth, which
sets the BW flag. validateUserAgent was deciding BW solely from loginHost,
causing LoginForAdminTests to fail. Now callers pass expectAdvancedAuth
(true when loginForAdmin || loginHost == .advancedAuth) and the UA check
uses that instead of re-deriving from the host.
Server returns invalid_grant for RTR + JWT tokens in hybrid flow.
Tests will be re-enabled once the server fix lands.
…e UA before revoke/refresh

- Add userAgent to UserCredentialsData and parse it from the sdk section
  in getUserCredentials(), eliminating the separate getUserAgent() UI round trip
- Remove standalone getUserAgent() from page object and BaseAuthFlowTester
- validateUserAgent() now takes userCredentials instead of calling getUserAgent()
- In validate(), move validateUserAgent before assertRevokeAndRefreshWorks so
  the UA is checked on the just-authenticated state, not after token rotation
…nsistency with Android

validateUser() now validates feature flags inline (matching Android's validateUser()),
so callers never need a separate validateUserAgent step. The expectAdvancedAuth,
usesWelcomeDiscovery, and isMultiUser flags are forwarded through validate(),
restartAndValidateUser, and switchToUserAndValidateUser.
…/switchToUserAndValidateUser

expectAdvancedAuth was derived from loginHost == .advancedAuth alone, missing the
loginForAdmin case where browser-based auth is used regardless of loginHost.
…estart

Adds two tests to LoginWithRestartTests:
- testAdvancedAuth_WithRestart: login via advanced auth (BW flag), restart, assert BW still set
- testWelcomeDiscovery_WithRestart: login via welcome discovery (WD flag), restart, assert WD still set

Also adds usesWelcomeDiscovery param to restartAndValidateUser so the WD flag
can be asserted post-restart.
…tAndValidateUser call

loginHost: .advancedAuth already implies expectAdvancedAuth = true internally.
…wise doing two welcome discovery tests in a row with the second one selecting the advanced auth org would fail - the second test would attempt to change login server while the advanced auth login was already shown
@wmathurin

Copy link
Copy Markdown
Contributor Author

All UI tests are passing locally.

@wmathurin
wmathurin merged commit 99a173b into forcedotcom:dev Jun 29, 2026
26 of 32 checks passed
wmathurin added a commit to wmathurin/SalesforceMobileSDK-iOS-Hybrid that referenced this pull request Jun 29, 2026
Resolve submodule conflict in external/SalesforceMobileSDK-iOS by keeping
our SHA (99a173b) which includes the per-user feature flag changes
(forcedotcom/SalesforceMobileSDK-iOS#4086). Upstream had 2692236.
JohnsonEricAtSalesforce added a commit to JohnsonEricAtSalesforce/SalesforceMobileSDK-iOS that referenced this pull request Jul 20, 2026
…ore/MobileSync (upstream unit 35)

Semantic port of forcedotcom/dev 99a173b into the migration branch.

Adds per-user feature flags alongside the existing global set:
- SFSDKAppFeatureMarkers (Swift twin): registerAppFeature(_:forUser:),
  unregisterAppFeature(_:forUser:), appFeatures(forUser:),
  loadPersistedFeatures(_:forUser:); per-user map keyed by
  SFKeyForUserAndScope(user, .user); persist via
  UserAccountManager.shared.upsert(_:).
- SFUserAccount (Swift twin): new persistedFeatureFlags: Set<String>?
  property + NSSecureCoding encode/decode (kUser_FEATURE_FLAGS).
- SalesforceSDKManager (Swift twin): new public
  userAgent(qualifier:for:) (@objc userAgentString:forUser:) whose ftr_
  segment unions global + per-user flags; defaultUserAgentString
  delegates to it; hydratePerUserFeatureFlags() on startup.
- SFUserAccountManager (Swift twin): finalizeAuthCompletion promotes
  transient global auth-method flags (BW/WD/QR) to the now-known user
  and clears the global flag; bio-auth/screen-lock recorded per-user.
- SFSmartStore / SFMobileSyncSyncManager (Swift twins): per-user
  registration one-liners.

De-ref .m ref-synced where a matching region exists (SFUserAccount.m,
SFMobileSyncSyncManager.m verbatim; SFSmartStore.m one-liner). Migration
stubs (SFSDKAppFeatureMarkers.h/.m, SalesforceSDKManager.h/+Internal.h,
SFUserAccountManager.m) have no matching region.

SalesforceSDKCore.podspec intentionally NOT modified: upstream's hunk was
a pure reordering of the auto-generated public_header_files list (header
set byte-identical), so there is no material change to port.

Tests: SFSDKAppFeatureMarkersTests.m ref-synced (216 lines) + 13 new
tests in the .swift twin; +1 encode/decode roundtrip in
SFUserAccountManagerTests and +3 per-user user-agent tests in
SalesforceSDKManagerTests (.m ref-synced, .swift twin extended, adapted
to upsert/delete). Sample AuthFlowTester (9 files) ref-synced to
post-image, preserving the migration's idData? optional-chaining.

All 3 lib schemes (SDKCore, SmartStore, MobileSync) build green with 0
new warnings; 17 AppFeatureMarkers + 4 manager/UA tests pass.

Escalation (feature-flags + OAuth/login + new public API) flagged in the
PR escalation digest.

Marker advanced 6a8a477 -> 99a173b (units 1-35 done).
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