Skip to content

Commit 6ad204e

Browse files
committed
Address Claude review bot feedback on IV PR
- Redact JWT from login/loginSuspend logs (show last 8 chars) - Fix error message in updateUserJwtSuspend - Add FAIL_UNAUTHORIZED to deleteSubscription and CustomEvent executors - Drop TransferSubscriptionOperation when IV=ON - Skip IAM fetch when JWT is invalidated (prevent rate-limit poisoning) - Clear existingOnesignalId unconditionally in removeOperationsWithoutExternalId
1 parent 6050a4b commit 6ad204e

File tree

5 files changed

+28
-19
lines changed

5 files changed

+28
-19
lines changed

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package com.onesignal.core.internal.operations.impl
22

3-
import com.onesignal.common.IDManager
43
import com.onesignal.common.threading.WaiterWithValue
54
import com.onesignal.core.internal.config.ConfigModelStore
65
import com.onesignal.core.internal.operations.ExecutionResult
@@ -546,18 +545,13 @@ internal class OperationRepo(
546545
Logging.debug("OperationRepo: removed ${toRemove.size} anonymous operations (no externalId)")
547546
}
548547

549-
// Any LoginUserOperation whose existingOnesignalId is a local ID was
550-
// waiting on a now-purged anonymous CreateUserOperation to translate it.
551-
// Clear it so canStartExecute unblocks and the executor takes the
552-
// createUser() path instead.
548+
// IV=ON never transfers anonymous state; clear existingOnesignalId so
549+
// the executor takes the createUser (upsert) path.
553550
queue.forEach {
554551
val op = it.operation
555-
if (op is LoginUserOperation) {
556-
val existing = op.existingOnesignalId
557-
if (existing != null && IDManager.isLocalId(existing)) {
558-
op.existingOnesignalId = null
559-
Logging.debug("OperationRepo: cleared local existingOnesignalId on LoginUserOperation (was $existing)")
560-
}
552+
if (op is LoginUserOperation && op.existingOnesignalId != null) {
553+
Logging.debug("OperationRepo: cleared existingOnesignalId on LoginUserOperation (was ${op.existingOnesignalId})")
554+
op.existingOnesignalId = null
561555
}
562556
}
563557
}

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ internal class OneSignalImp(
380380
externalId: String,
381381
jwtBearerToken: String?,
382382
) {
383-
Logging.log(LogLevel.DEBUG, "Calling deprecated login(externalId: $externalId, jwtBearerToken: $jwtBearerToken)")
383+
Logging.log(LogLevel.DEBUG, "Calling deprecated login(externalId: $externalId, jwtBearerToken: ...${jwtBearerToken?.takeLast(8)})")
384384

385385
if (isBackgroundThreadingEnabled) {
386386
waitForInit(operationName = "login")
@@ -425,7 +425,7 @@ internal class OneSignalImp(
425425
externalId: String,
426426
token: String,
427427
) {
428-
Logging.log(LogLevel.DEBUG, "updateUserJwt(externalId: $externalId, token: <redacted>)")
428+
Logging.log(LogLevel.DEBUG, "updateUserJwt(externalId: $externalId, token: ...${token.takeLast(8)})")
429429

430430
if (isBackgroundThreadingEnabled) {
431431
waitForInit(operationName = "updateUserJwt")
@@ -444,12 +444,12 @@ internal class OneSignalImp(
444444
externalId: String,
445445
token: String,
446446
) = withContext(runtimeIoDispatcher) {
447-
Logging.log(LogLevel.DEBUG, "updateUserJwtSuspend(externalId: $externalId, token: <redacted>)")
447+
Logging.log(LogLevel.DEBUG, "updateUserJwtSuspend(externalId: $externalId, token: ...${token.takeLast(8)})")
448448

449449
suspendUntilInit(operationName = "updateUserJwt")
450450

451451
if (!isInitialized) {
452-
throw IllegalStateException("'initWithContext failed' before 'updateUserJwt'")
452+
throw IllegalStateException("Must call 'initWithContext' before 'updateUserJwt'")
453453
}
454454

455455
jwtTokenStore.putJwt(externalId, token)
@@ -693,7 +693,7 @@ internal class OneSignalImp(
693693
externalId: String,
694694
jwtBearerToken: String?,
695695
) = withContext(runtimeIoDispatcher) {
696-
Logging.log(LogLevel.DEBUG, "login(externalId: $externalId, jwtBearerToken: $jwtBearerToken)")
696+
Logging.log(LogLevel.DEBUG, "login(externalId: $externalId, jwtBearerToken: ...${jwtBearerToken?.takeLast(8)})")
697697

698698
suspendUntilInit(operationName = "login")
699699

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/CustomEventOperationExecutor.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ internal class CustomEventOperationExecutor(
6161
return when (responseType) {
6262
NetworkUtils.ResponseStatusType.RETRYABLE ->
6363
ExecutionResponse(ExecutionResult.FAIL_RETRY, retryAfterSeconds = ex.retryAfterSeconds)
64+
NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
65+
ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED, retryAfterSeconds = ex.retryAfterSeconds)
6466
else ->
6567
ExecutionResponse(ExecutionResult.FAIL_NORETRY)
6668
}

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/SubscriptionOperationExecutor.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,11 @@ internal class SubscriptionOperationExecutor(
252252

253253
// TODO: whenever the end-user changes users, we need to add the read-your-write token here, currently no code to handle the re-fetch IAMs
254254
private suspend fun transferSubscription(startingOperation: TransferSubscriptionOperation): ExecutionResponse {
255+
if (_configModelStore.model.useIdentityVerification == true) {
256+
Logging.warn("TransferSubscriptionOperation is not supported when identity verification is enabled. Dropping.")
257+
return ExecutionResponse(ExecutionResult.FAIL_NORETRY)
258+
}
259+
255260
val (aliasLabel, aliasValue) =
256261
IdentityConstants.resolveAlias(
257262
_configModelStore.model.useIdentityVerification,
@@ -323,6 +328,8 @@ internal class SubscriptionOperationExecutor(
323328
}
324329
NetworkUtils.ResponseStatusType.RETRYABLE ->
325330
ExecutionResponse(ExecutionResult.FAIL_RETRY, retryAfterSeconds = ex.retryAfterSeconds)
331+
NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
332+
ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED, retryAfterSeconds = ex.retryAfterSeconds)
326333
else ->
327334
ExecutionResponse(ExecutionResult.FAIL_NORETRY)
328335
}

OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -302,9 +302,15 @@ internal class InAppMessagesManager(
302302
}
303303

304304
val externalId = _identityModelStore.model.externalId
305-
if (_configModelStore.model.useIdentityVerification == true && externalId == null) {
306-
Logging.debug("InAppMessagesManager.fetchMessages: Skipping IAM fetch for anonymous user while identity verification is enabled.")
307-
return
305+
if (_configModelStore.model.useIdentityVerification == true) {
306+
if (externalId == null) {
307+
Logging.debug("InAppMessagesManager.fetchMessages: Skipping IAM fetch for anonymous user while identity verification is enabled.")
308+
return
309+
}
310+
if (_jwtTokenStore.getJwt(externalId) == null) {
311+
Logging.debug("InAppMessagesManager.fetchMessages: Skipping IAM fetch while JWT is invalidated for user: $externalId")
312+
return
313+
}
308314
}
309315

310316
fetchIAMMutex.withLock {

0 commit comments

Comments
 (0)