diff --git a/OneSignalSDK/detekt/detekt-baseline-core.xml b/OneSignalSDK/detekt/detekt-baseline-core.xml index 88b28052a4..98692b29ce 100644 --- a/OneSignalSDK/detekt/detekt-baseline-core.xml +++ b/OneSignalSDK/detekt/detekt-baseline-core.xml @@ -63,6 +63,8 @@ ConstructorParameterNaming:NewRecordsState.kt$NewRecordsState$private val _time: ITime ConstructorParameterNaming:OSDatabase.kt$OSDatabase$private val _outcomeTableProvider: OutcomeTableProvider ConstructorParameterNaming:OperationRepo.kt$OperationRepo$private val _configModelStore: ConfigModelStore + ConstructorParameterNaming:OperationRepo.kt$OperationRepo$private val _identityVerificationService: IdentityVerificationService + ConstructorParameterNaming:OperationRepo.kt$OperationRepo$private val _jwtTokenStore: JwtTokenStore ConstructorParameterNaming:OperationRepo.kt$OperationRepo$private val _newRecordState: NewRecordsState ConstructorParameterNaming:OperationRepo.kt$OperationRepo$private val _operationModelStore: OperationModelStore ConstructorParameterNaming:OperationRepo.kt$OperationRepo$private val _time: ITime @@ -82,6 +84,7 @@ ConstructorParameterNaming:PreferencesService.kt$PreferencesService$private val _applicationService: IApplicationService ConstructorParameterNaming:PreferencesService.kt$PreferencesService$private val _time: ITime ConstructorParameterNaming:PropertiesModelStoreListener.kt$PropertiesModelStoreListener$private val _configModelStore: ConfigModelStore + ConstructorParameterNaming:PropertiesModelStoreListener.kt$PropertiesModelStoreListener$private val _identityModelStore: IdentityModelStore ConstructorParameterNaming:RebuildUserService.kt$RebuildUserService$private val _configModelStore: ConfigModelStore ConstructorParameterNaming:RebuildUserService.kt$RebuildUserService$private val _identityModelStore: IdentityModelStore ConstructorParameterNaming:RebuildUserService.kt$RebuildUserService$private val _propertiesModelStore: PropertiesModelStore @@ -161,7 +164,6 @@ ForbiddenComment:IUserBackendService.kt$IUserBackendService$// TODO: Change to send only the push subscription, optimally ForbiddenComment:LoginHelper.kt$LoginHelper$// TODO: Set JWT Token for all future requests. ForbiddenComment:LogoutHelper.kt$LogoutHelper$// TODO: remove JWT Token for all future requests. - ForbiddenComment:OperationRepo.kt$OperationRepo$// TODO: Need to provide callback for app to reset JWT. For now, fail with no retry. ForbiddenComment:ParamsBackendService.kt$ParamsBackendService$// TODO: New ForbiddenComment:PermissionsActivity.kt$PermissionsActivity$// TODO after we remove IAM from being an activity window we may be able to remove this handler ForbiddenComment:PermissionsActivity.kt$PermissionsActivity$// TODO improve this method @@ -194,6 +196,7 @@ LongMethod:TrackGooglePurchase.kt$TrackGooglePurchase$private fun queryBoughtItems() LongMethod:TrackGooglePurchase.kt$TrackGooglePurchase$private fun sendPurchases( skusToAdd: ArrayList<String>, newPurchaseTokens: ArrayList<String>, ) LongMethod:UpdateUserOperationExecutor.kt$UpdateUserOperationExecutor$override suspend fun execute(operations: List<Operation>): ExecutionResponse + LongParameterList:CreateSubscriptionOperation.kt$CreateSubscriptionOperation$(appId: String, onesignalId: String, externalId: String?, subscriptionId: String, type: SubscriptionType, enabled: Boolean, address: String, status: SubscriptionStatus) LongParameterList:ICustomEventBackendService.kt$ICustomEventBackendService$( appId: String, onesignalId: String, externalId: String?, timestamp: Long, eventName: String, eventProperties: String?, metadata: CustomEventMetadata, ) LongParameterList:IDatabase.kt$IDatabase$( table: String, columns: Array<String>? = null, whereClause: String? = null, whereArgs: Array<String>? = null, groupBy: String? = null, having: String? = null, orderBy: String? = null, limit: String? = null, action: (ICursor) -> Unit, ) LongParameterList:IOutcomeEventsBackendService.kt$IOutcomeEventsBackendService$( appId: String, userId: String, subscriptionId: String, deviceType: String, direct: Boolean?, event: OutcomeEvent, ) @@ -203,6 +206,7 @@ LongParameterList:OutcomeEventsController.kt$OutcomeEventsController$( private val _session: ISessionService, private val _influenceManager: IInfluenceManager, private val _outcomeEventsCache: IOutcomeEventsRepository, private val _outcomeEventsPreferences: IOutcomeEventsPreferences, private val _outcomeEventsBackend: IOutcomeEventsBackendService, private val _configModelStore: ConfigModelStore, private val _identityModelStore: IdentityModelStore, private val _subscriptionManager: ISubscriptionManager, private val _deviceService: IDeviceService, private val _time: ITime, ) LongParameterList:SubscriptionObject.kt$SubscriptionObject$( val id: String? = null, val type: SubscriptionObjectType? = null, val token: String? = null, val enabled: Boolean? = null, val notificationTypes: Int? = null, val sdk: String? = null, val deviceModel: String? = null, val deviceOS: String? = null, val rooted: Boolean? = null, val netType: Int? = null, val carrier: String? = null, val appVersion: String? = null, ) LongParameterList:SubscriptionOperationExecutor.kt$SubscriptionOperationExecutor$( private val _subscriptionBackend: ISubscriptionBackendService, private val _deviceService: IDeviceService, private val _applicationService: IApplicationService, private val _subscriptionModelStore: SubscriptionModelStore, private val _configModelStore: ConfigModelStore, private val _buildUserService: IRebuildUserService, private val _newRecordState: NewRecordsState, private val _consistencyManager: IConsistencyManager, ) + LongParameterList:UpdateSubscriptionOperation.kt$UpdateSubscriptionOperation$(appId: String, onesignalId: String, externalId: String?, subscriptionId: String, type: SubscriptionType, enabled: Boolean, address: String, status: SubscriptionStatus) LongParameterList:UserSwitcher.kt$UserSwitcher$( private val preferencesService: IPreferencesService, private val operationRepo: IOperationRepo, private val services: ServiceProvider, private val idManager: IDManager = IDManager, private val identityModelStore: IdentityModelStore, private val propertiesModelStore: PropertiesModelStore, private val subscriptionModelStore: SubscriptionModelStore, private val configModel: ConfigModel, private val oneSignalUtils: OneSignalUtils = OneSignalUtils, private val carrierName: String? = null, private val deviceOS: String? = null, private val androidUtils: AndroidUtils = AndroidUtils, private val appContextProvider: () -> Context, ) LoopWithTooManyJumpStatements:ModelStore.kt$ModelStore$for (index in jsonArray.length() - 1 downTo 0) { val newModel = create(jsonArray.getJSONObject(index)) ?: continue /* * NOTE: Migration fix for bug introduced in 5.1.12 * The following check is intended for the operation model store. * When the call to this method moved out of the operation model store's initializer, * duplicate operations could be cached. * See https://github.com/OneSignal/OneSignal-Android-SDK/pull/2099 */ val hasExisting = models.any { it.id == newModel.id } if (hasExisting) { Logging.debug("ModelStore<$name>: load - operation.id: ${newModel.id} already exists in the store.") continue } models.add(0, newModel) // listen for changes to this model newModel.subscribe(this) } MagicNumber:ApplicationService.kt$ApplicationService$50 @@ -285,6 +289,7 @@ ReturnCount:BackgroundManager.kt$BackgroundManager$override fun cancelRunBackgroundServices(): Boolean ReturnCount:OneSignalImp.kt$OneSignalImp$private fun internalInit( context: Context, appId: String?, ): Boolean ReturnCount:ConfigModel.kt$ConfigModel$override fun createModelForProperty( property: String, jsonObject: JSONObject, ): Model? + ReturnCount:FeatureFlagsBackendService.kt$FeatureFlagsBackendService$override suspend fun fetchRemoteFeatureFlags(appId: String): RemoteFeatureFlagsFetchOutcome ReturnCount:HttpClient.kt$HttpClient$private suspend fun makeRequest( url: String, method: String?, jsonBody: JSONObject?, timeout: Int, headers: OptionalHeaders?, ): HttpResponse ReturnCount:IdentityOperationExecutor.kt$IdentityOperationExecutor$override suspend fun execute(operations: List<Operation>): ExecutionResponse ReturnCount:JSONUtils.kt$JSONUtils$fun compareJSONArrays( jsonArray1: JSONArray?, jsonArray2: JSONArray?, ): Boolean @@ -297,6 +302,8 @@ ReturnCount:Model.kt$Model$protected fun getOptLongProperty( name: String, create: (() -> Long?)? = null, ): Long? ReturnCount:Model.kt$Model$protected inline fun <reified T : Enum<T>> getOptEnumProperty(name: String): T? ReturnCount:OperationModelStore.kt$OperationModelStore$override fun create(jsonObject: JSONObject?): Operation? + ReturnCount:OperationRepoIvExtensions.kt$internal fun OperationRepo.handleFailUnauthorized( startingOp: OperationRepo.OperationQueueItem, ops: List<OperationRepo.OperationQueueItem>, jwtTokenStore: JwtTokenStore, ivBehaviorActive: Boolean, ): Boolean + ReturnCount:OperationRepoIvExtensions.kt$internal fun OperationRepo.hasValidJwtIfRequired( jwtTokenStore: JwtTokenStore, op: com.onesignal.core.internal.operations.Operation, ivBehaviorActive: Boolean, ): Boolean ReturnCount:OperationModelStore.kt$OperationModelStore$private fun isValidOperation(jsonObject: JSONObject): Boolean ReturnCount:OutcomeEventsController.kt$OutcomeEventsController$private suspend fun sendAndCreateOutcomeEvent( name: String, weight: Float, // Note: this is optional sessionTime: Long, influences: List<Influence>, ): OutcomeEvent? ReturnCount:OutcomeEventsController.kt$OutcomeEventsController$private suspend fun sendUniqueOutcomeEvent( name: String, sessionInfluences: List<Influence>, ): OutcomeEvent? diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt index 908f55a0d4..b66559e7db 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt @@ -13,6 +13,7 @@ import com.onesignal.core.internal.background.impl.BackgroundManager import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.core.internal.config.impl.ConfigModelStoreListener import com.onesignal.core.internal.config.impl.FeatureFlagsRefreshService +import com.onesignal.core.internal.config.impl.IdentityVerificationService import com.onesignal.core.internal.database.IDatabaseProvider import com.onesignal.core.internal.database.impl.DatabaseProvider import com.onesignal.core.internal.device.IDeviceService @@ -70,6 +71,9 @@ internal class CoreModule : IModule { builder.register().provides() builder.register().provides() + builder.register() + .provides() + .provides() // Operations builder.register().provides() diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt index dfaaa027dc..ff9f967fc4 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/backend/impl/ParamsBackendService.kt @@ -71,8 +71,7 @@ internal class ParamsBackendService( return ParamsObject( googleProjectNumber = responseJson.safeString("android_sender_id"), enterprise = responseJson.safeBool("enterp"), - // TODO: New - useIdentityVerification = responseJson.safeBool("require_ident_auth"), + useIdentityVerification = responseJson.safeBool("jwt_required"), notificationChannels = responseJson.optJSONArray("chnl_lst"), firebaseAnalytics = responseJson.safeBool("fba"), restoreTTLFilter = responseJson.safeBool("restore_ttl_filter"), diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt index 26d4e1b86e..25e069ebbd 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/ConfigModelStoreListener.kt @@ -83,12 +83,10 @@ internal class ConfigModelStoreListener( config.fcmParams.projectId = params.fcmParams.projectId config.fcmParams.appId = params.fcmParams.appId config.fcmParams.apiKey = params.fcmParams.apiKey + config.useIdentityVerification = JwtRequirement.fromBoolean(params.useIdentityVerification ?: false) // these are only copied from the backend params when the backend has set them. params.enterprise?.let { config.enterprise = it } - params.useIdentityVerification?.let { - config.useIdentityVerification = JwtRequirement.fromBoolean(it) - } params.firebaseAnalytics?.let { config.firebaseAnalytics = it } params.restoreTTLFilter?.let { config.restoreTTLFilter = it } params.clearGroupOnSummaryClick?.let { config.clearGroupOnSummaryClick = it } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/IdentityVerificationService.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/IdentityVerificationService.kt new file mode 100644 index 0000000000..dc2eb6d3ce --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/config/impl/IdentityVerificationService.kt @@ -0,0 +1,75 @@ +package com.onesignal.core.internal.config.impl + +import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler +import com.onesignal.common.modeling.ModelChangeTags +import com.onesignal.common.modeling.ModelChangedArgs +import com.onesignal.core.internal.config.ConfigModel +import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.core.internal.features.FeatureFlag +import com.onesignal.core.internal.features.IFeatureManager +import com.onesignal.core.internal.startup.IStartableService +import com.onesignal.user.internal.jwt.JwtRequirement + +/** + * Single source of truth for Identity Verification gating, and for forwarding HYDRATE events to + * the [com.onesignal.core.internal.operations.IOperationRepo] post-HYDRATE choreography. + * + * Gate state is derived on read from the injected [IFeatureManager] (rollout flag) and + * [ConfigModelStore] (customer `jwt_required`); nothing is duplicated here. UNKNOWN + * (pre-HYDRATE) reads as `false` for both gates, which is the safe default. + * + * Invariant `ivBehaviorActive == true ⇒ newCodePathsRun == true` holds because both are derived + * from the same `useIdentityVerification` field. + * + * Consumers (e.g. OperationRepo) wire post-HYDRATE behavior via [setOnJwtConfigHydratedHandler]; + * the handler fires once per HYDRATE with `ivRequired = useIdentityVerification == REQUIRED`. + */ +internal class IdentityVerificationService( + private val featureManager: IFeatureManager, + private val configModelStore: ConfigModelStore, +) : IStartableService, ISingletonModelStoreChangeHandler { + /** Whether IV-specific behavior (JWT attachment, auth error handling) applies. UNKNOWN reads as `false`. */ + val ivBehaviorActive: Boolean + get() = configModelStore.model.useIdentityVerification == JwtRequirement.REQUIRED + + /** Whether new IV-related code paths should run. `featureFlag_IV_ON || jwt_required == REQUIRED`. */ + val newCodePathsRun: Boolean + get() = featureManager.isEnabled(FeatureFlag.SDK_IDENTITY_VERIFICATION) || ivBehaviorActive + + private val handlerLock = Any() + private var onJwtConfigHydrated: ((ivRequired: Boolean) -> Unit)? = null + + /** + * Register a handler invoked once per HYDRATE of the config model. Used by OperationRepo to + * release pre-HYDRATE deferral and (when IV is required) purge anonymous queued ops. + * Pass `null` to clear. + */ + fun setOnJwtConfigHydratedHandler(handler: ((ivRequired: Boolean) -> Unit)?) { + synchronized(handlerLock) { + onJwtConfigHydrated = handler + } + } + + override fun start() { + configModelStore.subscribe(this) + } + + override fun onModelReplaced( + model: ConfigModel, + tag: String, + ) { + if (tag != ModelChangeTags.HYDRATE) return + // Snapshot the handler under the lock, then invoke outside — never hold the lock + // across user-supplied code. + val handler = synchronized(handlerLock) { onJwtConfigHydrated } + handler?.invoke(model.useIdentityVerification == JwtRequirement.REQUIRED) + } + + override fun onModelUpdated( + args: ModelChangedArgs, + tag: String, + ) { + // Remote params arrive as full-model replacements (HYDRATE); individual property + // updates are not expected for useIdentityVerification. + } +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt index 04918d90f7..c9e43bad68 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/features/FeatureManager.kt @@ -8,7 +8,6 @@ import com.onesignal.core.internal.backend.impl.FeatureFlagsJsonParser import com.onesignal.core.internal.config.ConfigModel import com.onesignal.core.internal.config.ConfigModelStore import com.onesignal.debug.internal.logging.Logging -import com.onesignal.user.internal.jwt.IdentityVerificationGates import kotlinx.serialization.json.JsonObject internal interface IFeatureManager { @@ -165,12 +164,9 @@ internal class FeatureManager( source = "FeatureManager:${feature.activationMode}" ) - FeatureFlag.SDK_IDENTITY_VERIFICATION -> - IdentityVerificationGates.update( - featureFlagOn = enabled, - jwtRequirement = configModelStore.model.useIdentityVerification, - source = "FeatureManager:${feature.activationMode}" - ) + // SDK_IDENTITY_VERIFICATION has no side effect: IdentityVerificationService + // reads featureStates directly via isEnabled() at gate-check time. + FeatureFlag.SDK_IDENTITY_VERIFICATION -> {} } } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt index 7237b1f164..c27527a05a 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt @@ -2,7 +2,9 @@ package com.onesignal.core.internal.operations.impl import com.onesignal.common.IDManager import com.onesignal.common.threading.WaiterWithValue +import com.onesignal.common.threading.suspendifyOnIO import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.core.internal.config.impl.IdentityVerificationService import com.onesignal.core.internal.operations.ExecutionResult import com.onesignal.core.internal.operations.GroupComparisonType import com.onesignal.core.internal.operations.IOperationExecutor @@ -12,6 +14,8 @@ import com.onesignal.core.internal.startup.IStartableService import com.onesignal.core.internal.time.ITime import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging +import com.onesignal.user.internal.jwt.JwtRequirement +import com.onesignal.user.internal.jwt.JwtTokenStore import com.onesignal.user.internal.operations.LoginUserOperation import com.onesignal.user.internal.operations.impl.states.NewRecordsState import kotlinx.coroutines.CompletableDeferred @@ -30,7 +34,10 @@ internal class OperationRepo( private val _configModelStore: ConfigModelStore, private val _time: ITime, private val _newRecordState: NewRecordsState, + private val _jwtTokenStore: JwtTokenStore, + private val _identityVerificationService: IdentityVerificationService, ) : IOperationRepo, IStartableService { + internal class OperationQueueItem( val operation: Operation, var waiter: WaiterWithValue? = null, // waiter may transfer during operation de-dupe @@ -104,6 +111,12 @@ internal class OperationRepo( override fun start() { paused = false + // Wire post-HYDRATE choreography. Constructor injection of IOperationRepo into + // IdentityVerificationService would create a cycle, so the service exposes a setter + // that we call here. + _identityVerificationService.setOnJwtConfigHydratedHandler { ivRequired -> + onJwtConfigHydrated(ivRequired) + } scope.launch { // load saved operations first then start processing the queue to ensure correct operation order loadSavedOperations() @@ -203,9 +216,10 @@ internal class OperationRepo( } else { queue.add(queueItem) } - } - if (addToStore) { - _operationModelStore.add(queueItem.operation) + // Inside the lock so queue.add + store.add are atomic vs. the IO-side purge. + if (addToStore) { + _operationModelStore.add(queueItem.operation) + } } waiter.wake(LoopWaiterMessage(flush, 0)) @@ -225,7 +239,10 @@ internal class OperationRepo( } val ops = getNextOps(executeBucket) - Logging.debug("processQueueForever:ops:\n$ops") + if (Logging.atLogLevel(LogLevel.DEBUG)) { + val queueSnapshotForLogging = synchronized(queue) { queue.toList() } + Logging.debug("processQueueForever:ops:\n$ops\nqueue(${queueSnapshotForLogging.size}):\n$queueSnapshotForLogging") + } if (ops != null) { executeOperations(ops) @@ -244,6 +261,40 @@ internal class OperationRepo( waiter.wake(LoopWaiterMessage(false)) } + /** + * Drops queued operations whose externalId is null. Called by the IV-aware HYDRATE + * choreography in [OperationRepoIvExtensions] when `jwt_required` becomes REQUIRED + * to evict anon ops that can no longer execute. + */ + internal fun removeOperationsWithoutExternalId() { + val removedIds: List = + synchronized(queue) { + val anonymous = queue.filter { it.operation.externalId == null } + anonymous.forEach { it.waiter?.wake(false) } + queue.removeAll(anonymous) + Logging.debug("OperationRepo: removeOperationsWithoutExternalId removed ${anonymous.size} of ${anonymous.size + queue.size} operations") + anonymous.map { it.operation.id } + } + // Persistent store removal outside the queue lock; ModelStore has its own locking. + removedIds.forEach { _operationModelStore.remove(it) } + } + + /** + * Post-HYDRATE maintenance: scheduled on IO so it runs *after* `loadSavedOperations` + * populates the queue (fix for an earlier race where the purge ran against an empty + * in-memory queue on cold start). Force-execute always fires to release the pre-HYDRATE + * deferral in [getNextOps]. Invoked via the handler registered in [start]. + */ + internal fun onJwtConfigHydrated(ivRequired: Boolean) { + suspendifyOnIO { + awaitInitialized() + if (ivRequired) { + removeOperationsWithoutExternalId() + } + forceExecuteOperations() + } + } + /** * Waits until a new operation is enqueued, then wait an additional * amount of time afterwards, so operations can be grouped/batched. @@ -305,14 +356,27 @@ internal class OperationRepo( ops.forEach { _operationModelStore.remove(it.operation.id) } ops.forEach { it.waiter?.wake(true) } } - ExecutionResult.FAIL_UNAUTHORIZED, // TODO: Need to provide callback for app to reset JWT. For now, fail with no retry. + ExecutionResult.FAIL_UNAUTHORIZED -> { + // Outer gate: dispatch to IV extension only on new code paths. + val handled = + _identityVerificationService.newCodePathsRun && + handleFailUnauthorized( + startingOp, + ops, + _jwtTokenStore, + _identityVerificationService.ivBehaviorActive, + ) + if (!handled) { + // IV inactive or anon op: drop and wake waiters, matching FAIL_NORETRY. + Logging.warn("Operation execution failed without retry: $operations") + dropAndWake(ops) + } + } ExecutionResult.FAIL_NORETRY, ExecutionResult.FAIL_CONFLICT, -> { Logging.warn("Operation execution failed without retry: $operations") - // on failure we remove the operation from the store and wake any waiters - ops.forEach { _operationModelStore.remove(it.operation.id) } - ops.forEach { it.waiter?.wake(false) } + dropAndWake(ops) } ExecutionResult.SUCCESS_STARTING_ONLY -> { // remove the starting operation from the store and wake any waiters, then @@ -372,13 +436,16 @@ internal class OperationRepo( } } catch (e: Throwable) { Logging.log(LogLevel.ERROR, "Error attempting to execute operation: $ops", e) - - // on failure we remove the operation from the store and wake any waiters - ops.forEach { _operationModelStore.remove(it.operation.id) } - ops.forEach { it.waiter?.wake(false) } + dropAndWake(ops) } } + /** Drop ops from the persistent store and wake any waiters with `false` (failure). */ + private fun dropAndWake(ops: List) { + ops.forEach { _operationModelStore.remove(it.operation.id) } + ops.forEach { it.waiter?.wake(false) } + } + /** * Wait which ever is longer, retryAfterSeconds returned by the server, * or based on the retry count. @@ -415,12 +482,28 @@ internal class OperationRepo( } internal fun getNextOps(bucketFilter: Int): List? { + // Pre-HYDRATE deferral: wait until we know whether IV is required before dispatching + // any op, otherwise we could send an unsigned request when the customer has IV enabled. + // `isInitializedWithRemote` would be wrong here: pre-IV SDKs persisted it as `true` + // without ever reading `jwt_required`, so cached config from an upgrade looks + // "initialized" while `useIdentityVerification` is still UNKNOWN. Gating on UNKNOWN + // directly is correct because the IV-aware backend ships alongside this SDK — every + // successful HYDRATE will populate the field with REQUIRED or NOT_REQUIRED. + if (_configModelStore.model.useIdentityVerification == JwtRequirement.UNKNOWN) { + return null + } + + // Snapshot gate state once per pass so all queue items see the same IV view. + val newCodePathsRun = _identityVerificationService.newCodePathsRun + val ivBehaviorActive = _identityVerificationService.ivBehaviorActive return synchronized(queue) { val startingOp = queue.firstOrNull { it.operation.canStartExecute && _newRecordState.canAccess(it.operation.applyToRecordId) && - it.bucket <= bucketFilter + it.bucket <= bucketFilter && + // Outer gate: skip IV JWT check entirely on old code path. + (!newCodePathsRun || hasValidJwtIfRequired(_jwtTokenStore, it.operation, ivBehaviorActive)) } if (startingOp != null) { diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepoIvExtensions.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepoIvExtensions.kt new file mode 100644 index 0000000000..a6a788b074 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepoIvExtensions.kt @@ -0,0 +1,67 @@ +package com.onesignal.core.internal.operations.impl + +import com.onesignal.debug.internal.logging.Logging +import com.onesignal.user.internal.jwt.JwtTokenStore + +/** + * IV-specific behavior layered onto [OperationRepo]. Base-class dispatch sites are gated on + * `IdentityVerificationService.newCodePathsRun`; the caller passes `ivBehaviorActive` through so + * these extensions can short-circuit and stay inert for Phase 3 users (new code path on, IV + * behavior off). + */ + +/** + * Returns `true` if [op] may execute given current IV state. + * + * - IV behavior inactive → always `true` (no gating; new paths run but behavior is same as old). + * - Op opts out via `requiresJwt = false` → `true`. + * - Anonymous op while IV active → `false` (can't authenticate without an externalId). + * - Otherwise → `true` iff a JWT is currently stored for the op's externalId. + */ +internal fun OperationRepo.hasValidJwtIfRequired( + jwtTokenStore: JwtTokenStore, + op: com.onesignal.core.internal.operations.Operation, + ivBehaviorActive: Boolean, +): Boolean { + if (!ivBehaviorActive || !op.requiresJwt) return true + val externalId = op.externalId ?: return false + return jwtTokenStore.getJwt(externalId) != null +} + +/** + * Handles a [com.onesignal.core.internal.operations.ExecutionResult.FAIL_UNAUTHORIZED] response + * when IV behavior is active. Invalidates the JWT for the failing op's externalId (which fires + * `IJwtUpdateListener.onJwtInvalidated` to subscribers, surfacing to the developer via the + * public-API layer), and re-queues the ops (waiter wake with `false` so `enqueueAndWait` + * callers don't hang). + * + * Returns `true` if IV-specific handling was applied (caller should stop processing this result), + * or `false` when IV behavior is inactive or the op is anonymous (caller falls back to default + * drop-on-fail handling). + */ +internal fun OperationRepo.handleFailUnauthorized( + startingOp: OperationRepo.OperationQueueItem, + ops: List, + jwtTokenStore: JwtTokenStore, + ivBehaviorActive: Boolean, +): Boolean { + if (!ivBehaviorActive) return false + val externalId = startingOp.operation.externalId ?: return false + + // Fires onJwtInvalidated to subscribers BEFORE we wake waiters — otherwise an + // `enqueueAndWait` caller could return before the developer-facing event propagates. + jwtTokenStore.invalidateJwt(externalId) + Logging.info( + "Operation execution failed with 401 Unauthorized, JWT invalidated for user: $externalId. " + + "Operations re-queued.", + ) + // Wake enqueueAndWait callers; re-queue with waiter = null because the original waiter + // is already woken. + ops.forEach { it.waiter?.wake(false) } + synchronized(queue) { + ops.reversed().forEach { + queue.add(0, OperationRepo.OperationQueueItem(it.operation, waiter = null, bucket = it.bucket, retries = it.retries)) + } + } + return true +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/jwt/IJwtUpdateListener.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/jwt/IJwtUpdateListener.kt index 63b6a4075d..4663523f09 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/jwt/IJwtUpdateListener.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/jwt/IJwtUpdateListener.kt @@ -1,10 +1,19 @@ package com.onesignal.user.internal.jwt /** - * Wake-up notification from [JwtTokenStore] when the JWT for [externalId] changes. - * Listeners must call [JwtTokenStore.getJwt] for the current value — event delivery + * Notifications from [JwtTokenStore] about JWT state changes for an [externalId]. + * Listeners should call [JwtTokenStore.getJwt] for the current value — event delivery * order is not guaranteed to match mutation order across concurrent writers. */ internal interface IJwtUpdateListener { - fun onJwtUpdated(externalId: String) + /** Fired when a JWT was added or refreshed (`putJwt`), or when stale entries are pruned. */ + fun onJwtUpdated(externalId: String) {} + + /** + * Fired when a JWT is explicitly invalidated (`invalidateJwt`), e.g. on a 401 response. + * Surfaced to the developer as "the JWT for this user is no longer valid; please refresh." + * Don't trigger from internal cleanup paths (logout, user switch) where notifying the + * app is undesirable. + */ + fun onJwtInvalidated(externalId: String) {} } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/jwt/IdentityVerificationGates.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/jwt/IdentityVerificationGates.kt deleted file mode 100644 index 685777dafd..0000000000 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/jwt/IdentityVerificationGates.kt +++ /dev/null @@ -1,52 +0,0 @@ -package com.onesignal.user.internal.jwt - -import com.onesignal.debug.internal.logging.Logging - -/** - * Current Identity Verification gate state, pushed by `FeatureManager.applySideEffects`. - * - * Stored state is the tri-state [jwtRequirement] — UNKNOWN until the first HYDRATE so consumers - * that need to distinguish "we don't know yet" from "customer opted out" can. The Boolean - * derivations [ivBehaviorActive] and [newCodePathsRun] cover the common case where consumers - * just want yes/no; UNKNOWN reads as `false` for both, which is the safe default before HYDRATE. - * - * The two gates differ on purpose: [newCodePathsRun] also flips on when our SDK feature flag is - * on, honoring rollout state even when the customer hasn't opted in. [ivBehaviorActive] tracks - * customer config alone. - * - * Invariant `ivBehaviorActive == true ⇒ newCodePathsRun == true` is preserved at every - * observation because both are derived on read from the stored inputs; a reader can't observe an - * inconsistent state. - */ -internal object IdentityVerificationGates { - @Volatile - private var _featureFlagOn: Boolean = false - - /** Customer config (`jwt_required`); [JwtRequirement.UNKNOWN] until the first HYDRATE. */ - @Volatile - var jwtRequirement: JwtRequirement = JwtRequirement.UNKNOWN - private set - - /** Whether IV-specific behavior (JWT attachment, auth error handling) applies. UNKNOWN reads as `false`. */ - val ivBehaviorActive: Boolean - get() = jwtRequirement == JwtRequirement.REQUIRED - - /** Whether new IV-related code paths should run. `featureFlag_IV_ON || jwt_required == REQUIRED`. */ - val newCodePathsRun: Boolean - get() = _featureFlagOn || ivBehaviorActive - - /** Idempotent. [source] is logged for traceability. */ - fun update( - featureFlagOn: Boolean, - jwtRequirement: JwtRequirement, - source: String, - ) { - _featureFlagOn = featureFlagOn - this.jwtRequirement = jwtRequirement - - Logging.debug( - "OneSignal: IdentityVerificationGates.update: newCodePathsRun=$newCodePathsRun, " + - "ivBehaviorActive=$ivBehaviorActive (source=$source)", - ) - } -} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/jwt/JwtTokenStore.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/jwt/JwtTokenStore.kt index 102db5e591..6fb0c55393 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/jwt/JwtTokenStore.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/jwt/JwtTokenStore.kt @@ -12,7 +12,7 @@ import org.json.JSONObject /** * Persistent store mapping externalId -> JWT. Multi-user so ops queued under a previous user * can still resolve their JWT at execution time. Storage is unconditional; *usage* of JWTs is - * gated on [IdentityVerificationGates.ivBehaviorActive]. + * gated on `IdentityVerificationService.ivBehaviorActive`. */ internal class JwtTokenStore( private val _prefs: IPreferencesService, @@ -55,7 +55,12 @@ internal class JwtTokenStore( } } - /** Removes the JWT for [externalId] and notifies subscribers. */ + /** + * Removes the JWT for [externalId] and notifies subscribers via + * [IJwtUpdateListener.onJwtInvalidated]. Surfaced to the developer as "your JWT is no + * longer valid; please refresh." Don't call from internal cleanup paths (logout, user + * switch) — use a different mechanism if you need to clear without notifying the app. + */ fun invalidateJwt(externalId: String) { val existed: Boolean synchronized(tokens) { @@ -66,7 +71,14 @@ internal class JwtTokenStore( } } if (existed) { - updates.fire { it.onJwtUpdated(externalId) } + // Per-subscriber try/catch so one throwing listener doesn't break others or + // propagate up into the operation queue (would otherwise drop the failing op). + updates.fire { listener -> + runCatching { listener.onJwtInvalidated(externalId) } + .onFailure { ex -> + Logging.warn("JwtTokenStore: subscriber threw on onJwtInvalidated for externalId=$externalId", ex) + } + } } } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/IdentityVerificationServiceTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/IdentityVerificationServiceTests.kt new file mode 100644 index 0000000000..0265fa0806 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/config/impl/IdentityVerificationServiceTests.kt @@ -0,0 +1,133 @@ +package com.onesignal.core.internal.config.impl + +import com.onesignal.common.modeling.ModelChangeTags +import com.onesignal.core.internal.config.ConfigModel +import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.core.internal.features.FeatureFlag +import com.onesignal.core.internal.features.IFeatureManager +import com.onesignal.debug.LogLevel +import com.onesignal.debug.internal.logging.Logging +import com.onesignal.user.internal.jwt.JwtRequirement +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.runs +import io.mockk.verify + +class IdentityVerificationServiceTests : FunSpec({ + beforeEach { Logging.logLevel = LogLevel.NONE } + + fun makeService( + featureFlagOn: Boolean = false, + requirement: JwtRequirement = JwtRequirement.UNKNOWN, + ): Triple { + val featureManager = mockk() + every { featureManager.isEnabled(FeatureFlag.SDK_IDENTITY_VERIFICATION) } returns featureFlagOn + + val configModel = mockk(relaxed = true) + every { configModel.useIdentityVerification } returns requirement + + val configModelStore = mockk() + every { configModelStore.model } returns configModel + every { configModelStore.subscribe(any()) } just runs + + val service = IdentityVerificationService(featureManager, configModelStore) + return Triple(service, configModel, configModelStore) + } + + test("start subscribes to ConfigModelStore") { + val (service, _, configModelStore) = makeService() + + service.start() + + verify(exactly = 1) { configModelStore.subscribe(service) } + } + + // --- Gate derivation ------------------------------------------------------------------- + + test("flag off + UNKNOWN: both gates false (safe pre-HYDRATE default)") { + val (service, _, _) = makeService(featureFlagOn = false, requirement = JwtRequirement.UNKNOWN) + service.newCodePathsRun shouldBe false + service.ivBehaviorActive shouldBe false + } + + test("flag off + NOT_REQUIRED: both gates false") { + val (service, _, _) = makeService(featureFlagOn = false, requirement = JwtRequirement.NOT_REQUIRED) + service.newCodePathsRun shouldBe false + service.ivBehaviorActive shouldBe false + } + + test("ERROR STATE — flag off + REQUIRED: both gates true (customer config wins)") { + val (service, _, _) = makeService(featureFlagOn = false, requirement = JwtRequirement.REQUIRED) + service.newCodePathsRun shouldBe true + service.ivBehaviorActive shouldBe true + } + + test("flag on + UNKNOWN: newCodePathsRun true, ivBehaviorActive false") { + val (service, _, _) = makeService(featureFlagOn = true, requirement = JwtRequirement.UNKNOWN) + service.newCodePathsRun shouldBe true + service.ivBehaviorActive shouldBe false + } + + test("flag on + NOT_REQUIRED: newCodePathsRun true, ivBehaviorActive false (Phase 3)") { + val (service, _, _) = makeService(featureFlagOn = true, requirement = JwtRequirement.NOT_REQUIRED) + service.newCodePathsRun shouldBe true + service.ivBehaviorActive shouldBe false + } + + test("flag on + REQUIRED: both gates true (full IV)") { + val (service, _, _) = makeService(featureFlagOn = true, requirement = JwtRequirement.REQUIRED) + service.newCodePathsRun shouldBe true + service.ivBehaviorActive shouldBe true + } + + test("gates are derived on read — config flip is reflected without explicit update") { + val (service, configModel, _) = makeService(featureFlagOn = false, requirement = JwtRequirement.UNKNOWN) + service.ivBehaviorActive shouldBe false + + every { configModel.useIdentityVerification } returns JwtRequirement.REQUIRED + + service.ivBehaviorActive shouldBe true + service.newCodePathsRun shouldBe true + } + + // --- HYDRATE forwarding ---------------------------------------------------------------- + + test("HYDRATE with REQUIRED invokes registered handler with ivRequired=true") { + val (service, model, _) = makeService(requirement = JwtRequirement.REQUIRED) + var fired: Boolean? = null + service.setOnJwtConfigHydratedHandler { fired = it } + + service.onModelReplaced(model, ModelChangeTags.HYDRATE) + + fired shouldBe true + } + + test("HYDRATE with NOT_REQUIRED invokes registered handler with ivRequired=false") { + val (service, model, _) = makeService(requirement = JwtRequirement.NOT_REQUIRED) + var fired: Boolean? = null + service.setOnJwtConfigHydratedHandler { fired = it } + + service.onModelReplaced(model, ModelChangeTags.HYDRATE) + + fired shouldBe false + } + + test("non-HYDRATE replacement is ignored") { + val (service, model, _) = makeService(requirement = JwtRequirement.REQUIRED) + var fired: Boolean? = null + service.setOnJwtConfigHydratedHandler { fired = it } + + service.onModelReplaced(model, ModelChangeTags.NORMAL) + + fired shouldBe null + } + + test("no handler registered: HYDRATE is a no-op (no NPE)") { + val (service, model, _) = makeService(requirement = JwtRequirement.REQUIRED) + // Don't register a handler. + service.onModelReplaced(model, ModelChangeTags.HYDRATE) + } +}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt index 9fc1d8301d..a02c1bbf76 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/features/FeatureManagerTests.kt @@ -4,7 +4,6 @@ import com.onesignal.common.modeling.ModelChangeTags import com.onesignal.common.threading.ThreadingMode import com.onesignal.core.internal.config.ConfigModel import com.onesignal.core.internal.config.ConfigModelStore -import com.onesignal.user.internal.jwt.IdentityVerificationGates import com.onesignal.user.internal.jwt.JwtRequirement import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe @@ -17,7 +16,6 @@ import kotlinx.serialization.json.jsonPrimitive class FeatureManagerTests : FunSpec({ beforeEach { ThreadingMode.useBackgroundThreading = false - IdentityVerificationGates.update(false, JwtRequirement.UNKNOWN, "test-reset") } fun stubConfigModel(model: ConfigModel) { @@ -193,109 +191,25 @@ class FeatureManagerTests : FunSpec({ manager.enabledFeatureKeys() shouldBe emptyList() } - test("initial state: IDENTITY_VERIFICATION flag off + jwt_required=null → gates both false") { + test("IDENTITY_VERIFICATION is IMMEDIATE: mid-session flag flip flows through isEnabled") { val initialModel = mockk() stubConfigModel(initialModel) val configModelStore = mockk() every { configModelStore.model } returns initialModel every { configModelStore.subscribe(any()) } just runs - val manager = FeatureManager(configModelStore) manager.isEnabled(FeatureFlag.SDK_IDENTITY_VERIFICATION) shouldBe false - IdentityVerificationGates.newCodePathsRun shouldBe false - IdentityVerificationGates.ivBehaviorActive shouldBe false - } - - test("initial state: IDENTITY_VERIFICATION flag on → newCodePathsRun=true, ivBehaviorActive=false") { - val initialModel = mockk() - stubConfigModel(initialModel) - every { initialModel.sdkRemoteFeatureFlags } returns listOf(FeatureFlag.SDK_IDENTITY_VERIFICATION.key) - val configModelStore = mockk() - every { configModelStore.model } returns initialModel - every { configModelStore.subscribe(any()) } just runs - - val manager = FeatureManager(configModelStore) - - manager.isEnabled(FeatureFlag.SDK_IDENTITY_VERIFICATION) shouldBe true - IdentityVerificationGates.newCodePathsRun shouldBe true - IdentityVerificationGates.ivBehaviorActive shouldBe false - } - - test("ERROR STATE: flag off + jwt_required=true → both gates true (customer config wins)") { - val initialModel = mockk() - stubConfigModel(initialModel) - every { initialModel.useIdentityVerification } returns JwtRequirement.REQUIRED - val configModelStore = mockk() - every { configModelStore.model } returns initialModel - every { configModelStore.subscribe(any()) } just runs - - val manager = FeatureManager(configModelStore) - - manager.isEnabled(FeatureFlag.SDK_IDENTITY_VERIFICATION) shouldBe false - IdentityVerificationGates.newCodePathsRun shouldBe true - IdentityVerificationGates.ivBehaviorActive shouldBe true - } - - test("initial state: flag on + jwt_required=true → full IV (both gates true)") { - val initialModel = mockk() - stubConfigModel(initialModel) - every { initialModel.sdkRemoteFeatureFlags } returns listOf(FeatureFlag.SDK_IDENTITY_VERIFICATION.key) - every { initialModel.useIdentityVerification } returns JwtRequirement.REQUIRED - val configModelStore = mockk() - every { configModelStore.model } returns initialModel - every { configModelStore.subscribe(any()) } just runs - - val manager = FeatureManager(configModelStore) - - manager.isEnabled(FeatureFlag.SDK_IDENTITY_VERIFICATION) shouldBe true - IdentityVerificationGates.newCodePathsRun shouldBe true - IdentityVerificationGates.ivBehaviorActive shouldBe true - } - - test("HYDRATE updates gates when useIdentityVerification arrives as true") { - val initialModel = mockk() - stubConfigModel(initialModel) - val configModelStore = mockk() - every { configModelStore.model } returns initialModel - every { configModelStore.subscribe(any()) } just runs - val manager = FeatureManager(configModelStore) - - IdentityVerificationGates.ivBehaviorActive shouldBe false - - val updatedModel = mockk() - stubConfigModel(updatedModel) - every { updatedModel.useIdentityVerification } returns JwtRequirement.REQUIRED - // Match production: store's model is swapped before onModelReplaced fires. - every { configModelStore.model } returns updatedModel - - manager.onModelReplaced(updatedModel, ModelChangeTags.HYDRATE) - - IdentityVerificationGates.newCodePathsRun shouldBe true - IdentityVerificationGates.ivBehaviorActive shouldBe true - } - - test("IDENTITY_VERIFICATION is IMMEDIATE: mid-session flag flip flows through to the gates") { - val initialModel = mockk() - stubConfigModel(initialModel) - val configModelStore = mockk() - every { configModelStore.model } returns initialModel - every { configModelStore.subscribe(any()) } just runs - val manager = FeatureManager(configModelStore) // Mid-session model replacement enables the flag remotely. val updatedModel = mockk() stubConfigModel(updatedModel) every { updatedModel.sdkRemoteFeatureFlags } returns listOf(FeatureFlag.SDK_IDENTITY_VERIFICATION.key) - every { updatedModel.useIdentityVerification } returns JwtRequirement.NOT_REQUIRED every { configModelStore.model } returns updatedModel manager.onModelReplaced(updatedModel, ModelChangeTags.HYDRATE) // Feature flag flips in-memory because IDENTITY_VERIFICATION is IMMEDIATE. manager.isEnabled(FeatureFlag.SDK_IDENTITY_VERIFICATION) shouldBe true - // newCodePathsRun reflects the flipped flag; ivBehaviorActive still false (jwt_required=false). - IdentityVerificationGates.newCodePathsRun shouldBe true - IdentityVerificationGates.ivBehaviorActive shouldBe false } }) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt index a9f7db4f57..5183bafe8f 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt @@ -10,8 +10,12 @@ import com.onesignal.core.internal.preferences.PreferenceStores import com.onesignal.core.internal.time.impl.Time import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging +import com.onesignal.mocks.CoreInternalMocks import com.onesignal.mocks.MockHelper import com.onesignal.mocks.MockPreferencesService +import com.onesignal.user.internal.jwt.IJwtUpdateListener +import com.onesignal.user.internal.jwt.JwtRequirement +import com.onesignal.user.internal.jwt.JwtTokenStore import com.onesignal.user.internal.operations.ExecutorMocks.Companion.getNewRecordState import com.onesignal.user.internal.operations.LoginUserOperation import io.kotest.core.spec.style.FunSpec @@ -40,7 +44,13 @@ import java.util.UUID // Mocks used by every test in this file private class Mocks { - val configModelStore = MockHelper.configModelStore() + val configModelStore = + MockHelper.configModelStore { + // Default to "post-HYDRATE" so the pre-HYDRATE deferral doesn't hold up + // tests that aren't exercising that path. + it.isInitializedWithRemote = true + it.useIdentityVerification = JwtRequirement.NOT_REQUIRED + } val operationModelStore: OperationModelStore = run { @@ -65,6 +75,10 @@ private class Mocks { mockExecutor } + val jwtTokenStore: JwtTokenStore = JwtTokenStore(MockPreferencesService()) + + var identityVerificationService = CoreInternalMocks.identityVerificationService() + val operationRepo: OperationRepo by lazy { spyk( OperationRepo( @@ -73,6 +87,8 @@ private class Mocks { configModelStore, Time(), getNewRecordState(configModelStore), + jwtTokenStore, + identityVerificationService, ), recordPrivateCalls = true, ) @@ -98,6 +114,8 @@ class OperationRepoTests : FunSpec({ mocks.configModelStore, Time(), getNewRecordState(mocks.configModelStore), + JwtTokenStore(MockPreferencesService()), + CoreInternalMocks.identityVerificationService(), ), ) @@ -982,6 +1000,204 @@ class OperationRepoTests : FunSpec({ // Verify that the grouped execution happened with both operations // We can't easily verify the exact list content with MockK, but we verified it in the execution order tracking } + + // + // ---- PR 3: IV queue-runtime tests ---- + // + + test("pre-HYDRATE deferral: getNextOps returns null when useIdentityVerification is UNKNOWN") { + val mocks = Mocks() + mocks.configModelStore.model.useIdentityVerification = JwtRequirement.UNKNOWN + + // enqueue an op and start processing + mocks.operationRepo.start() + mocks.operationRepo.enqueue(mockOperation()) + delay(200) + + // Op should have been enqueued but not yet executed. + coVerify(exactly = 0) { mocks.executor.execute(any()) } + // getNextOps returning null does NOT remove the op from the queue. + mocks.operationRepo.queue.size shouldBe 1 + } + + test("post-HYDRATE resume: useIdentityVerification flipping out of UNKNOWN unblocks the queue") { + val mocks = Mocks() + mocks.configModelStore.model.useIdentityVerification = JwtRequirement.UNKNOWN + + mocks.operationRepo.start() + mocks.operationRepo.enqueue(mockOperation()) + delay(100) + coVerify(exactly = 0) { mocks.executor.execute(any()) } + + // Simulate HYDRATE completing with jwt_required=false, then wake the queue. + mocks.configModelStore.model.useIdentityVerification = JwtRequirement.NOT_REQUIRED + mocks.operationRepo.forceExecuteOperations() + delay(500) + + coVerify(exactly = 1) { mocks.executor.execute(any()) } + } + + test("removeOperationsWithoutExternalId drops queued anon ops and persists removal") { + val mocks = Mocks() + val anonOp = mockOperation(externalId = null) + val identifiedOp = mockOperation(externalId = "alice") + val anonId = anonOp.id + val identifiedId = identifiedOp.id + + mocks.operationRepo.enqueue(anonOp) + mocks.operationRepo.enqueue(identifiedOp) + // Let the scope.launch-ed internalEnqueues land in the queue. + delay(50) + + mocks.operationRepo.removeOperationsWithoutExternalId() + + mocks.operationRepo.queue.size shouldBe 1 + mocks.operationRepo.queue.first().operation.externalId shouldBe "alice" + verify(exactly = 1) { mocks.operationModelStore.remove(anonId) } + verify(exactly = 0) { mocks.operationModelStore.remove(identifiedId) } + } + + test("onJwtConfigHydrated(true) awaits init, purges anon ops, then force-executes (in order)") { + val mocks = Mocks() + coEvery { mocks.operationRepo.awaitInitialized() } just runs + every { mocks.operationRepo.removeOperationsWithoutExternalId() } just runs + every { mocks.operationRepo.forceExecuteOperations() } just runs + + mocks.operationRepo.onJwtConfigHydrated(ivRequired = true) + // suspendifyOnIO launches on IO; allow the orchestration to land. + delay(100) + + coVerifyOrder { + mocks.operationRepo.awaitInitialized() + mocks.operationRepo.removeOperationsWithoutExternalId() + mocks.operationRepo.forceExecuteOperations() + } + } + + test("onJwtConfigHydrated(false) skips the purge but still force-executes") { + val mocks = Mocks() + coEvery { mocks.operationRepo.awaitInitialized() } just runs + every { mocks.operationRepo.removeOperationsWithoutExternalId() } just runs + every { mocks.operationRepo.forceExecuteOperations() } just runs + + mocks.operationRepo.onJwtConfigHydrated(ivRequired = false) + delay(100) + + coVerify(exactly = 1) { mocks.operationRepo.awaitInitialized() } + verify(exactly = 0) { mocks.operationRepo.removeOperationsWithoutExternalId() } + verify(exactly = 1) { mocks.operationRepo.forceExecuteOperations() } + } + + test("FAIL_UNAUTHORIZED with IV active invalidates JWT, re-queues ops, and fires onJwtInvalidated") { + val mocks = Mocks() + mocks.identityVerificationService = CoreInternalMocks.identityVerificationService( + newCodePathsRun = true, + ivBehaviorActive = true, + ) + mocks.configModelStore.model.useIdentityVerification = JwtRequirement.REQUIRED + + val op = mockOperation(externalId = "alice") + val opId = op.id + coEvery { mocks.executor.execute(any()) } returns ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED) + + // Pre-seed the JWT store so invalidation is observable. + mocks.jwtTokenStore.putJwt("alice", "stale-token") + + var invalidatedId: String? = null + mocks.jwtTokenStore.subscribe( + object : IJwtUpdateListener { + override fun onJwtInvalidated(externalId: String) { + invalidatedId = externalId + } + }, + ) + + mocks.operationRepo.start() + // enqueueAndWait with failure should wake waiter with false. + val waitResult = + runBlocking { + withTimeout(2_000) { + mocks.operationRepo.enqueueAndWait(op) + } + } + + waitResult shouldBe false + invalidatedId shouldBe "alice" + mocks.jwtTokenStore.getJwt("alice") shouldBe null + // Op was re-queued (not dropped from store). + verify(exactly = 0) { mocks.operationModelStore.remove(opId) } + } + + test("FAIL_UNAUTHORIZED with throwing onJwtInvalidated subscriber does not drop ops") { + // Regression: a misbehaving subscriber must not propagate an exception up into + // executeOperations' catch, which would route the op through dropAndWake (op lost). + val mocks = Mocks() + mocks.identityVerificationService = CoreInternalMocks.identityVerificationService( + newCodePathsRun = true, + ivBehaviorActive = true, + ) + mocks.configModelStore.model.useIdentityVerification = JwtRequirement.REQUIRED + + val op = mockOperation(externalId = "alice") + val opId = op.id + coEvery { mocks.executor.execute(any()) } returns ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED) + mocks.jwtTokenStore.putJwt("alice", "stale-token") + mocks.jwtTokenStore.subscribe( + object : IJwtUpdateListener { + override fun onJwtInvalidated(externalId: String) { + throw RuntimeException("boom from subscriber") + } + }, + ) + + mocks.operationRepo.start() + val waitResult = + runBlocking { + withTimeout(2_000) { + mocks.operationRepo.enqueueAndWait(op) + } + } + + waitResult shouldBe false + // JWT was still invalidated despite the subscriber throw. + mocks.jwtTokenStore.getJwt("alice") shouldBe null + // Op was re-queued (not dropped) — proving the throw didn't escape into executeOperations. + verify(exactly = 0) { mocks.operationModelStore.remove(opId) } + } + + test("FAIL_UNAUTHORIZED with IV inactive falls back to default drop-on-fail") { + val mocks = Mocks() + mocks.identityVerificationService = CoreInternalMocks.identityVerificationService( + newCodePathsRun = true, + ivBehaviorActive = false, + ) + mocks.configModelStore.model.useIdentityVerification = JwtRequirement.NOT_REQUIRED + val op = mockOperation(externalId = "alice") + val opId = op.id + coEvery { mocks.executor.execute(any()) } returns ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED) + + var invalidatedFired = false + mocks.jwtTokenStore.subscribe( + object : IJwtUpdateListener { + override fun onJwtInvalidated(externalId: String) { + invalidatedFired = true + } + }, + ) + + mocks.operationRepo.start() + val waitResult = + runBlocking { + withTimeout(2_000) { + mocks.operationRepo.enqueueAndWait(op) + } + } + + waitResult shouldBe false + invalidatedFired shouldBe false + // Default behavior: drop the op. + verify(exactly = 1) { mocks.operationModelStore.remove(opId) } + } }) { companion object { private fun mockOperation( @@ -993,6 +1209,8 @@ class OperationRepoTests : FunSpec({ modifyComparisonKey: String = "modify-key", operationIdSlot: CapturingSlot? = null, applyToRecordId: String = "", + externalId: String? = null, + requiresJwt: Boolean = true, ): Operation { val operation = mockk() val opIdSlot = operationIdSlot ?: slot() @@ -1006,6 +1224,8 @@ class OperationRepoTests : FunSpec({ every { operation.modifyComparisonKey } returns modifyComparisonKey every { operation.translateIds(any()) } just runs every { operation.applyToRecordId } returns applyToRecordId + every { operation.externalId } returns externalId + every { operation.requiresJwt } returns requiresJwt return operation } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/mocks/CoreInternalMocks.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/mocks/CoreInternalMocks.kt new file mode 100644 index 0000000000..cbb8b32d47 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/mocks/CoreInternalMocks.kt @@ -0,0 +1,21 @@ +package com.onesignal.mocks + +import com.onesignal.core.internal.config.impl.IdentityVerificationService +import io.mockk.every +import io.mockk.mockk + +/** + * Helpers for core-internal types that can't live in [MockHelper] (testhelpers module can't see + * `internal` declarations from core). + */ +internal object CoreInternalMocks { + fun identityVerificationService( + newCodePathsRun: Boolean = false, + ivBehaviorActive: Boolean = false, + ): IdentityVerificationService { + val mock = mockk(relaxed = true) + every { mock.newCodePathsRun } returns newCodePathsRun + every { mock.ivBehaviorActive } returns ivBehaviorActive + return mock + } +} diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/jwt/IdentityVerificationGatesTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/jwt/IdentityVerificationGatesTests.kt deleted file mode 100644 index d185346ac7..0000000000 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/jwt/IdentityVerificationGatesTests.kt +++ /dev/null @@ -1,97 +0,0 @@ -package com.onesignal.user.internal.jwt - -import io.kotest.core.spec.style.FunSpec -import io.kotest.matchers.shouldBe - -class IdentityVerificationGatesTests : FunSpec({ - // Singleton state leaks across tests; reset before each. - beforeEach { - IdentityVerificationGates.update(false, JwtRequirement.UNKNOWN, "test-reset") - } - - test("defaults to newCodePathsRun=false and ivBehaviorActive=false") { - IdentityVerificationGates.newCodePathsRun shouldBe false - IdentityVerificationGates.ivBehaviorActive shouldBe false - } - - test("featureFlagOn=false, jwtRequirement=UNKNOWN: both gates are false (safe default)") { - IdentityVerificationGates.update( - featureFlagOn = false, - jwtRequirement = JwtRequirement.UNKNOWN, - source = "test", - ) - IdentityVerificationGates.newCodePathsRun shouldBe false - IdentityVerificationGates.ivBehaviorActive shouldBe false - } - - test("featureFlagOn=false, jwtRequirement=NOT_REQUIRED: both gates are false") { - IdentityVerificationGates.update( - featureFlagOn = false, - jwtRequirement = JwtRequirement.NOT_REQUIRED, - source = "test", - ) - IdentityVerificationGates.newCodePathsRun shouldBe false - IdentityVerificationGates.ivBehaviorActive shouldBe false - } - - test("ERROR STATE — featureFlagOn=false, jwtRequirement=REQUIRED: both gates true (customer config wins)") { - IdentityVerificationGates.update( - featureFlagOn = false, - jwtRequirement = JwtRequirement.REQUIRED, - source = "test", - ) - IdentityVerificationGates.newCodePathsRun shouldBe true - IdentityVerificationGates.ivBehaviorActive shouldBe true - } - - test("featureFlagOn=true, jwtRequirement=UNKNOWN: newCodePathsRun true, ivBehaviorActive false") { - IdentityVerificationGates.update( - featureFlagOn = true, - jwtRequirement = JwtRequirement.UNKNOWN, - source = "test", - ) - IdentityVerificationGates.newCodePathsRun shouldBe true - IdentityVerificationGates.ivBehaviorActive shouldBe false - } - - test("featureFlagOn=true, jwtRequirement=NOT_REQUIRED: newCodePathsRun true, ivBehaviorActive false (Phase 3)") { - IdentityVerificationGates.update( - featureFlagOn = true, - jwtRequirement = JwtRequirement.NOT_REQUIRED, - source = "test", - ) - IdentityVerificationGates.newCodePathsRun shouldBe true - IdentityVerificationGates.ivBehaviorActive shouldBe false - } - - test("featureFlagOn=true, jwtRequirement=REQUIRED: both gates true (full IV)") { - IdentityVerificationGates.update( - featureFlagOn = true, - jwtRequirement = JwtRequirement.REQUIRED, - source = "test", - ) - IdentityVerificationGates.newCodePathsRun shouldBe true - IdentityVerificationGates.ivBehaviorActive shouldBe true - } - - test("updating to the same values is a no-op but still reflects in reads") { - IdentityVerificationGates.update(true, JwtRequirement.REQUIRED, "first") - IdentityVerificationGates.update(true, JwtRequirement.REQUIRED, "second") - IdentityVerificationGates.newCodePathsRun shouldBe true - IdentityVerificationGates.ivBehaviorActive shouldBe true - } - - test("transition: non-IV → IV-active → off") { - IdentityVerificationGates.update(false, JwtRequirement.NOT_REQUIRED, "phase-1-non-iv") - IdentityVerificationGates.newCodePathsRun shouldBe false - IdentityVerificationGates.ivBehaviorActive shouldBe false - - IdentityVerificationGates.update(true, JwtRequirement.REQUIRED, "phase-2-iv-on") - IdentityVerificationGates.newCodePathsRun shouldBe true - IdentityVerificationGates.ivBehaviorActive shouldBe true - - IdentityVerificationGates.update(false, JwtRequirement.NOT_REQUIRED, "kill-switch") - IdentityVerificationGates.newCodePathsRun shouldBe false - IdentityVerificationGates.ivBehaviorActive shouldBe false - } -}) diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/jwt/JwtTokenStoreTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/jwt/JwtTokenStoreTests.kt index 664e502349..ae933240b5 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/jwt/JwtTokenStoreTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/jwt/JwtTokenStoreTests.kt @@ -133,37 +133,70 @@ class JwtTokenStoreTests : FunSpec({ calls.isEmpty() shouldBe true } - test("subscribers are notified on invalidation") { + test("subscribers are notified on invalidation via onJwtInvalidated") { val store = JwtTokenStore(MockPreferencesService()) store.putJwt("alice", "token-a") - val calls = mutableListOf() + val invalidatedCalls = mutableListOf() + val updatedCalls = mutableListOf() store.subscribe( object : IJwtUpdateListener { override fun onJwtUpdated(externalId: String) { - calls.add(externalId) + updatedCalls.add(externalId) + } + override fun onJwtInvalidated(externalId: String) { + invalidatedCalls.add(externalId) } }, ) store.invalidateJwt("alice") - calls shouldBe listOf("alice") + invalidatedCalls shouldBe listOf("alice") + // putJwt fired onJwtUpdated above; invalidateJwt should not fire onJwtUpdated. + updatedCalls.isEmpty() shouldBe true } test("subscribers are NOT notified when invalidating a non-existent token") { val store = JwtTokenStore(MockPreferencesService()) - val calls = mutableListOf() + val invalidatedCalls = mutableListOf() store.subscribe( object : IJwtUpdateListener { - override fun onJwtUpdated(externalId: String) { - calls.add(externalId) + override fun onJwtInvalidated(externalId: String) { + invalidatedCalls.add(externalId) } }, ) store.invalidateJwt("alice") - calls.isEmpty() shouldBe true + invalidatedCalls.isEmpty() shouldBe true + } + + test("throwing subscriber on onJwtInvalidated is isolated: no escape, other subscribers still fire") { + val store = JwtTokenStore(MockPreferencesService()) + store.putJwt("alice", "token-a") + val laterCalls = mutableListOf() + // First subscriber throws; second must still fire; invalidateJwt must not propagate. + store.subscribe( + object : IJwtUpdateListener { + override fun onJwtInvalidated(externalId: String) { + throw RuntimeException("boom") + } + }, + ) + store.subscribe( + object : IJwtUpdateListener { + override fun onJwtInvalidated(externalId: String) { + laterCalls.add(externalId) + } + }, + ) + + // Should not throw out of invalidateJwt. + store.invalidateJwt("alice") + + laterCalls shouldBe listOf("alice") + store.getJwt("alice") shouldBe null } test("pruneToExternalIds fires for each removed externalId") { diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/migrations/RecoverFromDroppedLoginBugTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/migrations/RecoverFromDroppedLoginBugTests.kt index 554c09ac96..cb16466bc2 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/migrations/RecoverFromDroppedLoginBugTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/migrations/RecoverFromDroppedLoginBugTests.kt @@ -5,7 +5,11 @@ import com.onesignal.core.internal.operations.impl.OperationRepo import com.onesignal.core.internal.time.impl.Time import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging +import com.onesignal.mocks.CoreInternalMocks import com.onesignal.mocks.MockHelper +import com.onesignal.mocks.MockPreferencesService +import com.onesignal.user.internal.jwt.JwtRequirement +import com.onesignal.user.internal.jwt.JwtTokenStore import com.onesignal.user.internal.operations.ExecutorMocks import com.onesignal.user.internal.operations.LoginUserOperation import io.kotest.core.spec.style.FunSpec @@ -29,7 +33,11 @@ private class Mocks { every { mockOperationModelStore.remove(any()) } just runs mockOperationModelStore } - val configModelStore = MockHelper.configModelStore() + val configModelStore = + MockHelper.configModelStore { + it.isInitializedWithRemote = true + it.useIdentityVerification = JwtRequirement.NOT_REQUIRED + } val operationRepo = spyk( OperationRepo( @@ -38,6 +46,8 @@ private class Mocks { configModelStore, Time(), ExecutorMocks.getNewRecordState(configModelStore), + JwtTokenStore(MockPreferencesService()), + CoreInternalMocks.identityVerificationService(), ), )