Skip to content

fix: detect when Identity verification becomes turned off#1674

Open
nan-li wants to merge 4 commits into
fix/identity_verification_crashesfrom
fix/detect_iv_turned_off
Open

fix: detect when Identity verification becomes turned off#1674
nan-li wants to merge 4 commits into
fix/identity_verification_crashesfrom
fix/detect_iv_turned_off

Conversation

@nan-li

@nan-li nan-li commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Description

One Line Summary

Detect when Identity Verification is turned off and release any requests that were waiting on a JWT.

Details

Motivation

The SDK detected Identity Verification (IV) turning on but never handled it turning off. Two gaps:

  1. A missing jwt_required field in remote params only set IV off when the value was previously unknown, so an app that had IV on stayed on after it was disabled remotely — the on→off transition was never even produced.
  2. No OSUserJwtConfigListener acted on the off transition, so requests parked per-external-id awaiting a JWT (each executor's pendingAuthRequests) were stranded forever — once IV is off, no JWT will ever arrive to release them.

Scope

  • A missing jwt_required remote param now always resolves IV to off (including the previously-on case).
  • On the off transition, each executor (User, Identity, Subscription, Property, CustomEvents) re-queues its auth-pended requests and flushes them without a JWT; the IAM controller re-fires a deferred fetch.
  • Not changed: the on/unknown transitions, the deferral of requests while IV status is unknown, and OSOperationRepo (deltas still flush on the normal poll cadence).
  • Also fixes a pre-existing bug where removeInvalidDeltasAndRequests persisted updateRequestQueue under the add/remove subscription queue keys.

Testing

Unit testing

Added per-executor tests asserting parked requests are released and sent without auth on the off transition, plus OSUserJwtConfig tests verifying a missing jwt_required resolves to off (including previously-on) and that an unchanged value does not re-fire listeners. All pass in OneSignalUserTests.

Manual testing

Not tested on a physical device; behavior is covered by the unit tests above.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

@nan-li

nan-li commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

@claude review

@nan-li nan-li changed the title Fix/detect iv turned off fix: detect when Identity verification becomes turned off Jun 11, 2026
@nan-li nan-li force-pushed the fix/detect_iv_turned_off branch from b000fdc to aef1699 Compare June 11, 2026 01:35
@nan-li

nan-li commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@claude review

Comment on lines 1322 to +1328
- (void)onRequiresUserAuthChangedFrom:(enum OSRequiresUserAuth)from to:(enum OSRequiresUserAuth)to {
// This callback is unused, the controller will fetch when subscription ID changes
// Identity Verification was turned off: a fetch deferred waiting for a JWT may never be
// retried via onJwtUpdated once auth is off, so release it here
if (to == OSRequiresUserAuthOff && shouldRetryGetInAppMessagesOnJwtUpdated) {
shouldRetryGetInAppMessagesOnJwtUpdated = false;
[self getInAppMessagesFromServer];
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 The new IV-off handler in onRequiresUserAuthChangedFrom:to: only retries the IAM fetch when shouldRetryGetInAppMessagesOnJwtUpdated is set, but getInAppMessagesFromServer also defers via shouldRetryGetInAppMessagesOnUserChange when getAliasForCurrentUser returns nil — e.g. an anon user with no external_id while IV is on, or the unknown-IV boot window. Once IV flips off the alias becomes resolvable from onesignal_id, but no user-state observer fires (it is guarded by ID equality), so the deferred fetch is stranded until the next session. Consider draining shouldRetryGetInAppMessagesOnUserChange in the same handler for symmetry.

Extended reasoning...

The gap

OSMessagingController.getInAppMessagesFromServer has two distinct deferral flags:

  • shouldRetryGetInAppMessagesOnJwtUpdated — set when the user header is missing (no JWT) or on a 401 response.
  • shouldRetryGetInAppMessagesOnUserChange — set when getAliasForCurrentUser returns nil, or when onesignalId is nil.

The PR's new handler at OSMessagingController.m:1322-1328 only consumes the JWT flag. The user-change flag is silently ignored on the IV-off transition, even though that transition itself can be the event that makes the previously-blocked alias resolvable.

How the user-change flag gets set under IV-on

OSUserUtils.getAlias is the source of the alias used by the IAM fetch. Its logic:

guard let jwtRequired = jwtConfig.isRequired else { return nil }
if jwtRequired, let externalId = identityModel.externalId {
    return OSAliasPair(OS_EXTERNAL_ID, externalId)
} else if !jwtRequired, let onesignalId = identityModel.onesignalId {
    return OSAliasPair(OS_ONESIGNAL_ID, onesignalId)
}
return nil

So under IV-on, an anon user (no external_id) yields a nil alias — the fetch is blocked at the alias check and the user-change flag is set. Likewise during the unknown-IV boot window (isRequired == nil), getAlias returns nil regardless of which IDs are populated, so any IAM fetch that runs before remote params resolve also lands in the user-change branch.

Why the user observer won't recover this

fireUserStateChanged in OSUserUtils.swift is guarded by guard prevOnesignalId != newOnesignalId || prevExternalId != newExternalId. It is only invoked from OSIdentityModel.hydrateModel (id changes) and from _logout under IV-on. The IV-off transition by itself does not change onesignal_id or external_id for a cached user, so the snapshot-equality guard short-circuits and the user observer never re-fires. Result: the deferred fetch stays stranded until the next cold start (or some unrelated identity change).

Step-by-step proof

  1. App launches with a cached anon user (has onesignal_id, no external_id) and IV currently on.
  2. Push subscription becomes valid → onPushSubscriptionDidChangeWithState calls getInAppMessagesFromServer.
  3. getAliasForCurrentUser returns nil under IV-on (anon, no external_id) → line 295 sets shouldRetryGetInAppMessagesOnUserChange = true and returns.
  4. Remote params come back without jwt_requiredremoteParamsReturnedUnknownRequiresUserAuth (post-PR) sets jwtConfig.isRequired = false unconditionally → fires onRequiresUserAuthChanged(on, off) to all subscribers including OSMessagingController.
  5. The new handler checks shouldRetryGetInAppMessagesOnJwtUpdated — it is false — and returns without retrying.
  6. No user-state observer fires (onesignal_id/external_id unchanged). IAMs remain unfetched until the next cold start.

If step 5 had also released the user-change flag, the very next call to getInAppMessagesFromServer would succeed: the !jwtRequired branch of getAlias now resolves OS_ONESIGNAL_ID, the fetch executes and IAMs arrive in the current session.

Impact

In-app messages don't display for the current session for the affected users (anon-with-cached-onesignalId, or any user where the IAM fetch fired during the unknown-IV boot window). The PR's explicit motivation is IV-off recovery, and the handler's own comment — "a fetch deferred waiting for a JWT may never be retried via onJwtUpdated once auth is off, so release it here" — applies symmetrically to a fetch deferred waiting for a user-state change.

Fix

Add a parallel check in the same handler:

if (to == OSRequiresUserAuthOff) {
    if (shouldRetryGetInAppMessagesOnJwtUpdated || shouldRetryGetInAppMessagesOnUserChange) {
        shouldRetryGetInAppMessagesOnJwtUpdated = false;
        shouldRetryGetInAppMessagesOnUserChange = false;
        [self getInAppMessagesFromServer];
    }
}

@nan-li nan-li force-pushed the fix/identity_verification_crashes branch from 4d9f72f to 7699bbd Compare June 18, 2026 06:07
nan-li and others added 4 commits June 17, 2026 23:08
A missing jwt_required field previously set the flag off only when it was still unknown, so an app that had Identity Verification on stayed on after it was disabled remotely. Treat a missing field as off unconditionally so the on->off transition is actually detected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d off

While auth is required, requests without a valid JWT are parked per external ID awaiting a token. Once Identity Verification is turned off no token will arrive, so each JWT listener (User, Identity, Subscription, Property, and CustomEvents executors, plus the IAM controller) now releases the parked work and flushes it on the off transition.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
removeInvalidDeltasAndRequests saved updateRequestQueue under both the add and remove request queue keys, corrupting those caches when Identity Verification is enabled. Persist the matching addRequestQueue and removeRequestQueue instead.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add per-executor tests asserting parked requests are released and sent without auth once Identity Verification is turned off, plus OSUserJwtConfig tests verifying a missing jwt_required remote param resolves to off (including the previously-on case) and that an unchanged value does not re-fire listeners.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@nan-li nan-li force-pushed the fix/detect_iv_turned_off branch from aef1699 to d4d113e Compare June 18, 2026 17:21
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.

1 participant