diff --git a/package.json b/package.json index 76c018e69..985854bd3 100644 --- a/package.json +++ b/package.json @@ -77,7 +77,7 @@ }, { "path": "./build/releases/OneSignalSDK.page.es6.js", - "limit": "44.49 kB", + "limit": "44.58 kB", "gzip": true }, { diff --git a/src/core/executors/SubscriptionOperationExecutor.ts b/src/core/executors/SubscriptionOperationExecutor.ts index cf3e6f3a9..33404af81 100644 --- a/src/core/executors/SubscriptionOperationExecutor.ts +++ b/src/core/executors/SubscriptionOperationExecutor.ts @@ -248,6 +248,8 @@ export class SubscriptionOperationExecutor implements IOperationExecutor { }), ], }; + case ResponseStatusType._Unauthorized: + return this._unsignedRouteRejected('update'); default: return { _result: ExecutionResult._FailNoretry }; } @@ -315,9 +317,18 @@ export class SubscriptionOperationExecutor implements IOperationExecutor { _result: ExecutionResult._FailRetry, _retryAfterSeconds: retryAfterSeconds, }; + case ResponseStatusType._Unauthorized: + return this._unsignedRouteRejected('delete'); default: return { _result: ExecutionResult._FailNoretry }; } } + + // The SDK never signs PATCH or DELETE subscriptions/{id}, so a 401 here means the + // server contract changed. The operation is dropped; make the loss visible. + private _unsignedRouteRejected(route: 'update' | 'delete'): ExecutionResponse { + Log._error(`SubOpExec: 401 on unsigned ${route}`); + return { _result: ExecutionResult._FailNoretry }; + } } diff --git a/src/core/executors/executorsIv.test.ts b/src/core/executors/executorsIv.test.ts index f2eeb2d66..d8ac50756 100644 --- a/src/core/executors/executorsIv.test.ts +++ b/src/core/executors/executorsIv.test.ts @@ -269,6 +269,36 @@ describe('executors under Identity Verification', () => { expect(Log._error).toHaveBeenCalledWith(expect.stringContaining('no externalId')); }); + describe('routes the server does not sign', () => { + // PATCH subscriptions/{id} rejects a bearer; DELETE subscriptions/{id} ignores it. + beforeEach(() => { + setGates(true, JwtRequirement._Required); + tokens._putJwt(EXTERNAL_ID, JWT); + }); + + test('update subscription sends no Authorization header', async () => { + const response = await subscription._execute([ + new UpdateSubscriptionOperation({ ...owner, ...sub }), + ]); + + expect(response._result).toBe(ExecutionResult._Success); + const { headers, url } = lastRequest(); + expect(headers).not.toHaveProperty('authorization'); + expect(url.endsWith(`/apps/${APP_ID}/subscriptions/${SUB_ID}`)).toBe(true); + }); + + test('delete subscription sends no Authorization header', async () => { + const response = await subscription._execute([ + new DeleteSubscriptionOperation({ ...owner, subscriptionId: SUB_ID }), + ]); + + expect(response._result).toBe(ExecutionResult._Success); + const { headers, url } = lastRequest(); + expect(headers).not.toHaveProperty('authorization'); + expect(url.endsWith(`/apps/${APP_ID}/subscriptions/${SUB_ID}`)).toBe(true); + }); + }); + describe('401 under IV', () => { const unauthorized = (method: 'post' | 'patch' | 'delete') => getHandler({ uri: '*', method, status: 401, retryAfter: 15 }); @@ -308,22 +338,24 @@ describe('executors under Identity Verification', () => { // These routes carry no token, so a 401 says nothing about the stored JWT // and must not reach the unauthorized handler that invalidates it. - test('unsigned update subscription stays _FailNoretry', async () => { + test('unsigned update subscription stays _FailNoretry and logs an error', async () => { unauthorized('patch'); const response = await subscription._execute([ new UpdateSubscriptionOperation({ ...owner, ...sub }), ]); expect(lastRequest().headers).not.toHaveProperty('authorization'); expect(response).toEqual({ _result: ExecutionResult._FailNoretry }); + expect(Log._error).toHaveBeenCalledWith(expect.stringContaining('401 on unsigned update')); }); - test('unsigned delete subscription stays _FailNoretry', async () => { + test('unsigned delete subscription stays _FailNoretry and logs an error', async () => { unauthorized('delete'); const response = await subscription._execute([ new DeleteSubscriptionOperation({ ...owner, subscriptionId: SUB_ID }), ]); expect(lastRequest().headers).not.toHaveProperty('authorization'); expect(response).toEqual({ _result: ExecutionResult._FailNoretry }); + expect(Log._error).toHaveBeenCalledWith(expect.stringContaining('401 on unsigned delete')); }); }); }); diff --git a/src/core/operationRepo/OperationRepo.test.ts b/src/core/operationRepo/OperationRepo.test.ts index 1087c300b..40d4567d9 100644 --- a/src/core/operationRepo/OperationRepo.test.ts +++ b/src/core/operationRepo/OperationRepo.test.ts @@ -13,6 +13,7 @@ import { afterEach, beforeEach, describe, expect, test, vi, type Mock } from 'vi import { JwtTokenStore } from '../JwtTokenStore'; import { OperationModelStore } from '../modelRepo/OperationModelStore'; import { CreateSubscriptionOperation } from '../operations/CreateSubscriptionOperation'; +import { DeleteSubscriptionOperation } from '../operations/DeleteSubscriptionOperation'; import { LoginUserOperation } from '../operations/LoginUserOperation'; import { GroupComparisonType, @@ -246,6 +247,21 @@ describe('OperationRepo', () => { expect(mockOperationModelStore._list()).toEqual([identified]); }); + test('IV active: keeps an anonymous operation that needs no JWT', async () => { + setJwtRequirement(JwtRequirement._Required); + const remove = new DeleteSubscriptionOperation({ + appId: APP_ID, + onesignalId: ONESIGNAL_ID, + subscriptionId: SUB_ID, + }); + seedSaved(new Operation('anon'), remove); + + await opRepo._start(); + + expect(queued()).toEqual([remove]); + expect(mockOperationModelStore._list()).toEqual([remove]); + }); + test('IV active: clears existingOnesignalId on a surviving LoginUserOperation', async () => { setJwtRequirement(JwtRequirement._Required); const localId = IDManager._createLocalId(); @@ -445,6 +461,19 @@ describe('OperationRepo', () => { expect(warn).not.toHaveBeenCalled(); }); + test('an anonymous operation that needs no JWT is queued and dispatches', () => { + const op = new DeleteSubscriptionOperation({ + appId: APP_ID, + onesignalId: ONESIGNAL_ID, + subscriptionId: SUB_ID, + }); + opRepo._enqueue(op); + + expect(opRepo._queue).toEqual([{ operation: op, bucket: 0, retries: 0 }]); + expect(warn).not.toHaveBeenCalled(); + expect(opRepo._getNextOps(0)).toEqual([{ operation: op, bucket: 0, retries: 0 }]); + }); + test('an identified operation is queued', () => { const op = identifiedOp(); opRepo._enqueue(op); @@ -555,6 +584,27 @@ describe('OperationRepo', () => { expect(mockOperationModelStore._list()).toEqual([]); }); + test('IV active: an operation that needs no JWT is dropped and keeps the token', async () => { + setJwtRequirement(JwtRequirement._Required); + jwtTokenStore._putJwt(EXTERNAL_ID, 'kept'); + failUnauthorized(); + + class NoJwtOperation extends Operation { + override get _requiresJwt() { + return false; + } + } + const op = ownedBy(new NoJwtOperation('no-jwt'), EXTERNAL_ID); + const waiter = rejectionOf(opRepo._enqueueAndWait(op)); + await executeOps(opRepo); + + expect((await waiter)._result).toBe(ExecutionResult._FailUnauthorized); + expect(invalidated).not.toHaveBeenCalled(); + expect(jwtTokenStore._getJwt(EXTERNAL_ID)).toBe('kept'); + expect(opRepo._queue).toEqual([]); + expect(mockOperationModelStore._list()).toEqual([]); + }); + test('IV inactive with the new code path on: drops the operation and fires no event', async () => { setJwtRequirement(JwtRequirement._NotRequired); localStorage.setItem('os_feature_overrides', 'sdk_identity_verification'); diff --git a/src/core/operationRepo/OperationRepo.ts b/src/core/operationRepo/OperationRepo.ts index ba8245053..e09b77117 100644 --- a/src/core/operationRepo/OperationRepo.ts +++ b/src/core/operationRepo/OperationRepo.ts @@ -141,13 +141,14 @@ export class OperationRepo implements IOperationRepo, IStartableService { * An anonymous operation can never dispatch while IV behavior is active: the gate * needs a token and an anonymous user has none. Drop it at enqueue instead of * holding it forever. LoginUserOperation is exempt; login and the push grant - * enqueue it on purpose, and the load-time purge removes a stale one. + * enqueue it on purpose, and the load-time purge removes a stale one. An + * operation that needs no JWT is exempt too; the gate lets it through. * Outer gate isIvCodePathEnabled keeps the legacy enqueue path unchanged when * the flag is off. */ private _shouldSuppressAnonymousOp(op: Operation): boolean { if (!isIvCodePathEnabled()) return false; - if (op instanceof LoginUserOperation) return false; + if (op instanceof LoginUserOperation || !op._requiresJwt) return false; if (!isIvBehaviorActive() || op._externalId) return false; // Bypasses Log so the developer sees this in production builds. @@ -158,16 +159,18 @@ export class OperationRepo implements IOperationRepo, IStartableService { } /** - * Removes every queued operation with no externalId. These were persisted while - * the requirement was off or unknown, and an anonymous user has no JWT, so they - * can never pass the dispatch gate. Models are untouched; only operations go. + * Removes every queued operation with no externalId that needs a JWT. These were + * persisted while the requirement was off or unknown, and an anonymous user has + * no JWT, so they can never pass the dispatch gate. An operation that needs no + * JWT stays; the gate lets it through. Models are untouched; only operations go. * Surviving LoginUserOperations lose existingOnesignalId because the anonymous * login that would have resolved a local id is gone. */ private _purgeAnonymousOperations(): void { const total = this._queue.length; - const removed = this._queue.filter((item) => !item.operation._externalId); - this._queue = this._queue.filter((item) => item.operation._externalId); + const isPurged = (op: Operation) => !op._externalId && op._requiresJwt; + const removed = this._queue.filter((item) => isPurged(item.operation)); + this._queue = this._queue.filter((item) => !isPurged(item.operation)); for (const item of removed) { this._operationModelStore._remove(item.operation._modelId); @@ -363,8 +366,9 @@ export class OperationRepo implements IOperationRepo, IStartableService { * wakes the waiters, and re-queues the operations at the head with no resolver. * The dispatch gate then holds them until a new token is stored, so there is no * retry loop. If a newer token is already stored, it is kept and the re-queued - * operations retry with it. Returns false when IV is inactive or the operation - * is anonymous; the caller then drops the operations. + * operations retry with it. Returns false when IV is inactive, the operation + * is anonymous, or the operation sent no token (a 401 on an unsigned request + * says nothing about the stored token); the caller then drops the operations. */ private _handleFailUnauthorized( ops: OperationQueueItem[], @@ -372,8 +376,8 @@ export class OperationRepo implements IOperationRepo, IStartableService { jwtAtDispatch: string | undefined, ): boolean { if (!ivBehaviorActive) return false; - const externalId = ops[0].operation._externalId; - if (!externalId) return false; + const { _externalId: externalId, _requiresJwt: requiresJwt } = ops[0].operation; + if (!externalId || !requiresJwt) return false; if (this._jwtTokenStore._getJwt(externalId) === jwtAtDispatch) { this._jwtTokenStore._invalidateJwt(externalId); diff --git a/src/core/operations/DeleteSubscriptionOperation.ts b/src/core/operations/DeleteSubscriptionOperation.ts index 95dfac567..1bc56eacb 100644 --- a/src/core/operations/DeleteSubscriptionOperation.ts +++ b/src/core/operations/DeleteSubscriptionOperation.ts @@ -13,4 +13,10 @@ export class DeleteSubscriptionOperation extends BaseSubscriptionOperation { override get _groupComparisonType(): GroupComparisonValue { return GroupComparisonType._None; } + + // The server ignores a bearer on DELETE subscriptions/{id}, so a pending + // remove must not wait for a token, for example after logout. + override get _requiresJwt(): boolean { + return false; + } } diff --git a/src/core/operations/Operation.test.ts b/src/core/operations/Operation.test.ts index 11f3d813b..9cd551654 100644 --- a/src/core/operations/Operation.test.ts +++ b/src/core/operations/Operation.test.ts @@ -91,6 +91,12 @@ const cases: [string, Operation, Operation][] = [ ], ]; +// PATCH and DELETE subscriptions/{id} take no user JWT on the server. +const unsignedRoutes = new Set([ + OPERATION_NAME._UpdateSubscription, + OPERATION_NAME._DeleteSubscription, +]); + describe('Operation owner', () => { describe.each(cases)('%s', (name, identified, anonymous) => { test('carries the externalId it was built with', () => { @@ -104,9 +110,10 @@ describe('Operation owner', () => { expect(anonymous.toJSON()).not.toHaveProperty('externalId'); }); - test('requires a JWT by default', () => { - expect(identified._requiresJwt).toBe(true); - expect(anonymous._requiresJwt).toBe(true); + test('requires a JWT unless the server does not check one on its route', () => { + const expected = !unsignedRoutes.has(name); + expect(identified._requiresJwt).toBe(expected); + expect(anonymous._requiresJwt).toBe(expected); }); test('round-trips the externalId through the operation store', () => { diff --git a/src/core/operations/UpdateSubscriptionOperation.ts b/src/core/operations/UpdateSubscriptionOperation.ts index bb38bc682..e1aa7d22b 100644 --- a/src/core/operations/UpdateSubscriptionOperation.ts +++ b/src/core/operations/UpdateSubscriptionOperation.ts @@ -11,4 +11,10 @@ export class UpdateSubscriptionOperation extends BaseFullSubscriptionOperation { constructor(subscription?: SubscriptionWithAppId) { super(OPERATION_NAME._UpdateSubscription, subscription); } + + // The server rejects a bearer on PATCH subscriptions/{id} and accepts the + // request without one, so the SDK never signs it and must not wait for a token. + override get _requiresJwt(): boolean { + return false; + } } diff --git a/src/core/requests/api.ts b/src/core/requests/api.ts index 5be1030ae..cc97f5c78 100644 --- a/src/core/requests/api.ts +++ b/src/core/requests/api.ts @@ -179,12 +179,14 @@ export async function createSubscriptionByAlias( /** * Updates an existing Subscription’s properties. + * Never signed: the server rejects a bearer on this route with 401 and + * accepts the request without one. * @param requestMetadata - { appId } * @param subscriptionId - subscription id * @param subscription - subscription object */ export async function updateSubscriptionById( - requestMetadata: RequestMetadata, + requestMetadata: Omit, subscriptionId: string, subscription: ICreateUserSubscription, ) { @@ -197,11 +199,12 @@ export async function updateSubscriptionById( /** * Deletes the subscription. * Creates an "orphan" user record if the user has no other subscriptions. + * Never signed: the server ignores a bearer on this route. * @param requestMetadata - { appId } * @param subscriptionId - subscription id */ export async function deleteSubscriptionById( - requestMetadata: RequestMetadata, + requestMetadata: Omit, subscriptionId: string, ) { const { appId } = requestMetadata;