fix: detect when Identity verification becomes turned off#1674
Merged
Conversation
Contributor
Author
|
@claude review |
nan-li
changed the title
nan-li
force-pushed
the
fix/detect_iv_turned_off
branch
from
b000fdc to
aef1699
Compare
Contributor
Author
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
fix unsynchronized reads of OSIdentityModel state (jwtBearerToken in particular)
Two follow-on JWT concurrency issues exposed while reviewing the prior fix. 1. OSIdentityModelRepo.updateJwtToken fired the model's change notifier synchronously (→ onModelUpdated → onJwtTokenChanged → executor listeners) while still holding the repo's NSLock. Today nothing re-enters the repo lock so it doesn't deadlock by luck, but it's a trap for any future listener. The fix collects matching models under the lock and mutates them outside, so the notifier fires lock-free. 2. invalidateJwtForExternalId had a TOCTOU between its "is it already invalid?" read and the "set to invalid" write. A concurrent valid-token write landing between them would be overwritten with INVALID and trigger a needless re-auth. The transition is now an atomic compare-and-set on the model (invalidateJwtBearerToken); only the thread that wins the transition fires fireJwtExpired. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
nan-li
force-pushed
the
fix/identity_verification_crashes
branch
from
4d9f72f to
7699bbd
Compare
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
force-pushed
the
fix/detect_iv_turned_off
branch
from
aef1699 to
d4d113e
Compare
|
|
||
| /// Identity Verification was turned off: move every auth-pended request, across all | ||
| /// external IDs, back into the request queue and flush. | ||
| private func reQueueAllPendingRequests() { |
Contributor
There was a problem hiding this comment.
so even though the calls originated with JWT and now JWT is off, we want to reQueue the requests?
Contributor
Author
There was a problem hiding this comment.
Good point, we could decide to drop but this would come from something like:
- login(userA) - addTag(userA)
- SDK tries to send it but the token for userA has since expired
- login(userB) - addTag(userB)
- SDK tries to send but the token for userB has expires
- All these requests go into a pending queue until an updated token is provided for one or both of them
- When IV resolves to false, we can send these requests without a token
abdulraqeeb33
approved these changes
Jun 24, 2026
Base automatically changed from
fix/identity_verification_crashes
to
identity_verification_beta
June 26, 2026 00:52
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
jwt_requiredfield 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.OSUserJwtConfigListeneracted on the off transition, so requests parked per-external-id awaiting a JWT (each executor'spendingAuthRequests) were stranded forever — once IV is off, no JWT will ever arrive to release them.Scope
jwt_requiredremote param now always resolves IV to off (including the previously-on case).OSOperationRepo(deltas still flush on the normal poll cadence).removeInvalidDeltasAndRequestspersistedupdateRequestQueueunder 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
OSUserJwtConfigtests verifying a missingjwt_requiredresolves to off (including previously-on) and that an unchanged value does not re-fire listeners. All pass inOneSignalUserTests.Manual testing
Not tested on a physical device; behavior is covered by the unit tests above.
Affected code checklist
Checklist
Overview
Testing
Final pass