Skip to content

feat: [JWT-6] Identity Verification for the request pipeline - #1711

Open
nan-li wants to merge 6 commits into
nan/jwt-pr5-delta-ownershipfrom
nan/jwt-pr6-request-pipeline
Open

feat: [JWT-6] Identity Verification for the request pipeline#1711
nan-li wants to merge 6 commits into
nan/jwt-pr5-delta-ownershipfrom
nan/jwt-pr6-request-pipeline

Conversation

@nan-li

@nan-li nan-li commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Description

One Line Summary

Gate user-scoped request addressing and signing on Identity Verification — path alias + JWT from one user, hold the queue while the requirement is unknown, and drop anonymous work when IV is on — with Update Subscription intentionally exempt.

Details

Motivation

Everything that sends a user-scoped call needs one place to decide how to address and sign it. Under Identity Verification a user is named by external_id (app-chosen, so path segments are percent-encoded), and the alias and token must never come from different users. Sending unsigned while the requirement is still unknown would be rejected; sending signed too early is not possible — so queued work must wait, then flush when the answer arrives.

Scope

  • OSRequestAuth / OSAliasPair — for a given user, which alias names them in the path and which token signs it, read together
  • OSUrlPath — percent-encode Identity Verification path segments
  • Requests carry the identity model that owns them rather than reading whoever is current at send time
  • OSOperationRepo — hold while JWT requirement is unknown; flush when hydrated; drop work with no external ID when IV is on (anonymous users are never created on the server)
  • Executors + user/subscription/custom-events request builders wired through that auth path
  • Update Subscription deliberately exempt: a push subscription belongs to the device, not the signed-in user
  • Logout while IV is on internally disables the push subscription (replacement anonymous user is never created); login / requirement hydrating to off clears that
  • Test-only: MockNewRecordsState.holdWhilePresent so purge tests stay deterministic under zero OP_REPO_POST_CREATE_DELAY_SECONDS
  • Unit / executor / lifecycle coverage for auth, path encoding, queue hold/flush, anonymous purge

Stacked on #1710 (nan/jwt-pr5-delta-ownership).

Testing

Unit testing

  • OSRequestAuthTests, RequestPathEncodingTests
  • OSOperationRepoIdentityVerificationTests, expanded flush tests / shared support
  • ExecutorAnonymousPurgeTests, expanded UserExecutorTests / DeltaOwnershipTests
  • UserJwtLifecycleTests
  • Existing custom-events / concurrency / race / switch-user tests updated for auth ownership

Manual testing

Built in sequence as part of the local JWT stack against an iOS Simulator; this PR’s commits compiled on top of PR5.

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

Made with Cursor

@nan-li nan-li changed the title feat: [PR6] Identity Verification for the request pipeline feat: [JWT-6] Identity Verification for the request pipeline Aug 12, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Multi-model adversarial review (interrogate)

Intent

Gate user-scoped request addressing and signing on Identity Verification. For a given user, OSRequestAuth / OSAliasPair choose the path alias and JWT bearer together so they never come from different users; percent-encode IV path segments via OSUrlPath; requests carry their owning identity model; OSOperationRepo holds while the requirement is unknown, flushes on hydrate, and drops anonymous work when IV is on; Update Subscription is exempt; logout under IV disables push; test-only holdWhilePresent for purge determinism.

Reviewers

  • A: claude-fable-5-thinking-xhigh — 9 findings
  • B: gpt-5.6-sol-xhigh — 6 findings
  • C: cursor-grok-4.5-high-fast — 6 findings
  • D: claude-opus-5-thinking-high — 15 findings

Act On

  1. Cold-start Identify User is dropped under IV instead of reshaped to Create User (C; related A) — uncacheUserRequests branch 3 treats prepare == false as permanent discard, but Identify's prepare intentionally fails under IV so reshape can promote it. When identityModelToUpdate is still the current user, the login is stranded with no onesignal_id / no Create User. The existing test locks in drop-without-Create and misses the “ToUpdate still current” path.
  2. addsNewRecords decodes false for pre-upgrade Create User caches (A, C, D) — decodeBool on a missing key skips newRecordsState, weakening RYW and risking the 404 → logout path the cool-down exists to prevent.
  3. Bearer JWTs land in VERBOSE request logs (B) — setBearer writes Authorization into additionalHeaders; OneSignalClient logs those headers in full.
  4. awaitsToken false-positive stalls non-auth prepare failures (A) — cool-down / other prepare failures are treated as “waiting for JWT,” so the wait-only pass schedules no delayed retry and never asks for a token.
  5. OneSignalUserManagerImpl.swift crosses 1k lines (987 → 1065) (A, B, C, D) — IV logout silencing, hydrate restore, and storeJwt wake belong in OneSignalUserManagerImpl+Jwt.swift (or a focused helper).

Consider

  • ownerExternalId != nil || prepare on uncache is not gated on IV, so IV-off apps can retain forever-unsendable owned requests (D).
  • Update Subscription now archives a full OSIdentityModel (including bearer) and can revive it into the token pool (D).
  • Cached-requirement hydrate clears _isDisabledInternally before subscription listeners start, so IV-off restore can skip the server update (B).
  • authorization(onesignalId:externalId:) / OSUserRequestAuthorization are unused in production; the doc claims IAM already consumes them (A, B, D) — demote or land with the caller; IAM wiring itself can stay stacked.
  • Transient clearUserData anonymity can drop in-flight identified work under IV (A).
  • Legacy FetchIdentityBySubscription migration is hard-dropped under IV (A, C).
  • Unbounded 401 ↔ ask-for-token if the app keeps returning the same rejected JWT (D).

Noted

  • Anonymous purge duplicated across executors; purge-on-every-flush overhead; OSUrlPath doc mentions Live Activities it does not encode; unlabeled OSAliasPair init.

Dismissed

  • Must wire IAM fetches in this PR (B critical) — out of scope for the queued user-request pipeline; the unused public API / false “currently IAM” claim is still Act/Consider above.
  • Logout silencing while requirement is .unknown (D) — intentional and covered by UserJwtLifecycleTests.
  • Trusting a persisted non-unknown requirement across sessions (B) — established JWT-config cache contract from earlier stack PRs; not introduced uniquely here.

Agreement Map

Strong consensus on the 1k-line crossing and addsNewRecords upgrade decode. High-confidence solo/pair hits after lead verification: Identify cold-start drop (C), JWT logging (B), awaitsToken stall (A). Largest disagreement: B treated missing IAM wiring as a ship-blocker; lead scopes that to a later consumer PR while still rejecting the dead public surface claim.

Open in Web View Automation 

Sent by Cursor Automation: Untitled

Comment thread iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift Outdated
Comment thread iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestCreateUser.swift Outdated
Comment thread iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift
Comment thread iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSRequestAuth.swift
@nan-li
nan-li force-pushed the nan/jwt-pr5-delta-ownership branch from 96a17a3 to f0dc805 Compare August 12, 2026 16:57
@nan-li
nan-li force-pushed the nan/jwt-pr6-request-pipeline branch from 8dabb1a to 1d08c49 Compare August 12, 2026 16:57
@nan-li
nan-li force-pushed the nan/jwt-pr5-delta-ownership branch from f0dc805 to 2c267bf Compare August 12, 2026 17:35
@nan-li
nan-li force-pushed the nan/jwt-pr6-request-pipeline branch from 1d08c49 to b0aec18 Compare August 12, 2026 17:35
nan-li and others added 3 commits August 12, 2026 14:22
Everything that sends a user-scoped call now decides how to address and
sign it in one place. OSRequestAuth answers, for a given user, which alias
names them in the path and which token signs it, read together so the
alias and the token can never come from different users.

Under Identity Verification a user is addressed by external_id, which the
app chooses, so those path segments are percent-encoded through OSUrlPath.
Requests carry the identity model that owns them rather than reading
whoever is current at send time.

The operation repo holds queued work while the requirement is still
unknown, since sending unsigned would be rejected and sending signed too
early is not possible. Once the answer arrives the queue flushes. When
Identity Verification is on, work belonging to no external ID is dropped
rather than sent: an anonymous user is never created on the server, so
that work has no user to belong to.

Update Subscription is deliberately exempt from all of this. A push
subscription belongs to the device, not the signed-in user, so it always
goes out whether or not anyone is logged in and whether or not a token is
valid; nothing about it is gated on auth.

Logging out while Identity Verification is on internally disables the push
subscription, since the replacement anonymous user is never created and
the subscription would otherwise keep reporting under the logged-out user.
Logging back in clears that, as does the requirement hydrating to off.

Co-authored-by: Cursor <cursoragent@cursor.com>
Under TEST, OP_REPO_POST_CREATE_DELAY_SECONDS is 0, so canAccess released
an ID the instant it was added and the Requests left the executor queues
before removeOperationsWithoutExternalId could see them.

MockNewRecordsState.holdWhilePresent keeps an ID inaccessible for as long
as it is present. Purge tests opt in; every other consumer keeps the
production timer behavior.

Co-authored-by: Cursor <cursoragent@cursor.com>
No behavior change — move code into extensions / a top-level
OSPushSubscriptionImpl so file_length and type_body_length stay under
error thresholds.

Co-authored-by: Cursor <cursoragent@cursor.com>
@nan-li
nan-li force-pushed the nan/jwt-pr6-request-pipeline branch from b0aec18 to a642d1c Compare August 12, 2026 21:22
nan-li and others added 3 commits August 12, 2026 17:09
prepareForExecution is false when Identity Verification is on, which uncache
treated as a drop. A cold-start Identify whose ToUpdate is still current never
reached reshape, so the login had no Create User. Keep it when IV is on and
let reshape convert or drop.

Co-authored-by: Cursor <cursoragent@cursor.com>
decodeBool is false when the key is absent, so pre-upgrade Create User
caches skipped newRecordsState and follow-ups could 404. A missing key
cools down; recovery creates write the field explicitly.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ion is on

Without it, an owned Request whose identity model is gone can never become sendable, so uncache must drop it instead of keeping it forever.

Co-authored-by: Cursor <cursoragent@cursor.com>
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