fix bugs and add new a test case

OneSignal · Mar 28, 2024 · ee6ff69 · ee6ff69
1 parent 0ea7689
commit ee6ff69
Show file tree

Hide file tree

Showing 12 changed files with 98 additions and 22 deletions.
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/IOneSignal.kt
@@ -22,7 +22,7 @@ interface IOneSignal {
     /**
      * Whether the security feature to authenticate your external user ids is enabled
      */
-    val isIdentityVerificationEnabled: Boolean
+    val useIdentityVerification: Boolean
 
     /**
      * The user manager for accessing user-scoped

diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignal.kt
@@ -33,8 +33,8 @@ object OneSignal {
      * Whether the security feature to authenticate your external user ids is enabled
      */
     @JvmStatic
-    val isIdentityVerificationEnabled: Boolean
-        get() = oneSignal.isIdentityVerificationEnabled
+    val useIdentityVerification: Boolean
+        get() = oneSignal.useIdentityVerification
 
     /**
      * The current SDK version as a string.

diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt
@@ -58,7 +58,8 @@ import org.json.JSONObject
 internal class OneSignalImp : IOneSignal, IServiceProvider {
     override val sdkVersion: String = OneSignalUtils.SDK_VERSION
     override var isInitialized: Boolean = false
-    override val isIdentityVerificationEnabled: Boolean = false
+    override val useIdentityVerification: Boolean
+        get() = configModel?.useIdentityVerification?: true
 
     override var consentRequired: Boolean
         get() = configModel?.consentRequired ?: (_consentRequired == true)
@@ -136,6 +137,7 @@ internal class OneSignalImp : IOneSignal, IServiceProvider {
     private var sessionModel: SessionModel? = null
     private var _consentRequired: Boolean? = null
     private var _consentGiven: Boolean? = null
+    private var _useIdentityVerification: Boolean? = false
     private var _disableGMSMissingPrompt: Boolean? = null
     private val initLock: Any = Any()
     private val loginLock: Any = Any()

diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserManager.kt
@@ -279,7 +279,7 @@ internal open class UserManager(
                 // Fire the event when the JWT has been invalidated.
                 val oldJwt = args.oldValue.toString()
                 val newJwt = args.newValue.toString()
-                if (OneSignal.isIdentityVerificationEnabled && oldJwt == newJwt && newJwt.isEmpty()) {
+                if (OneSignal.useIdentityVerification && oldJwt != newJwt && newJwt.isEmpty()) {
                     jwtInvalidatedCallback.fire {
                         it.onUserJwtInvalidated(UserJwtInvalidatedEvent((externalId)))
                     }

diff --git a/...K/onesignal/core/src/main/java/com/onesignal/user/internal/identity/IdentityModelStore.kt b/...K/onesignal/core/src/main/java/com/onesignal/user/internal/identity/IdentityModelStore.kt
@@ -3,7 +3,15 @@ package com.onesignal.user.internal.identity
 import com.onesignal.common.modeling.SimpleModelStore
 import com.onesignal.common.modeling.SingletonModelStore
 import com.onesignal.core.internal.preferences.IPreferencesService
+import com.onesignal.user.internal.backend.IdentityConstants
 
 open class IdentityModelStore(prefs: IPreferencesService) : SingletonModelStore<IdentityModel>(
     SimpleModelStore({ IdentityModel() }, "identity", prefs),
-)
+) {
+    fun invalidateJwt() {
+        model.setStringProperty(
+                IdentityConstants.JWT_TOKEN,
+                "",
+        )
+    }
+}
diff --git a/...n/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt b/...n/java/com/onesignal/user/internal/operations/impl/executors/IdentityOperationExecutor.kt
@@ -55,8 +55,10 @@ internal class IdentityOperationExecutor(
                         ExecutionResponse(ExecutionResult.FAIL_NORETRY)
                     NetworkUtils.ResponseStatusType.CONFLICT ->
                         ExecutionResponse(ExecutionResult.FAIL_CONFLICT)
-                    NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
-                        ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
+                    NetworkUtils.ResponseStatusType.UNAUTHORIZED -> {
+                        _identityModelStore.invalidateJwt()
+                        return ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
+                    }
                     NetworkUtils.ResponseStatusType.MISSING -> {
                         val operations = _buildUserService.getRebuildOperationsIfCurrentUser(lastOperation.appId, lastOperation.onesignalId)
                         if (operations == null) {
@@ -92,8 +94,10 @@ internal class IdentityOperationExecutor(
                         ExecutionResponse(ExecutionResult.SUCCESS)
                     NetworkUtils.ResponseStatusType.INVALID ->
                         ExecutionResponse(ExecutionResult.FAIL_NORETRY)
-                    NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
-                        ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
+                    NetworkUtils.ResponseStatusType.UNAUTHORIZED -> {
+                        _identityModelStore.invalidateJwt()
+                        return ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
+                    }
                     NetworkUtils.ResponseStatusType.MISSING -> {
                         val operations = _buildUserService.getRebuildOperationsIfCurrentUser(lastOperation.appId, lastOperation.onesignalId)
                         if (operations == null) {

diff --git a/...nal/user/internal/operations/impl/executors/LoginUserFromSubscriptionOperationExecutor.kt b/...nal/user/internal/operations/impl/executors/LoginUserFromSubscriptionOperationExecutor.kt
@@ -79,8 +79,10 @@ internal class LoginUserFromSubscriptionOperationExecutor(
             return when (responseType) {
                 NetworkUtils.ResponseStatusType.RETRYABLE ->
                     ExecutionResponse(ExecutionResult.FAIL_RETRY)
-                NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
+                NetworkUtils.ResponseStatusType.UNAUTHORIZED -> {
+                    _identityModelStore.invalidateJwt()
                     ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
+                }
                 else ->
                     ExecutionResponse(ExecutionResult.FAIL_NORETRY)
             }

diff --git a/.../java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt b/.../java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt
@@ -126,10 +126,7 @@ internal class LoginUserOperationExecutor(
                     createUser(loginUserOp, operations)
                 }
                 ExecutionResult.FAIL_UNAUTHORIZED -> {
-                    _identityModelStore.model.setStringProperty(
-                            IdentityConstants.JWT_TOKEN,
-                            "",
-                    )
+                    _identityModelStore.invalidateJwt()
                     ExecutionResponse(result.result)
                 }
                 else -> ExecutionResponse(result.result)
@@ -209,8 +206,10 @@ internal class LoginUserOperationExecutor(
             return when (responseType) {
                 NetworkUtils.ResponseStatusType.RETRYABLE ->
                     ExecutionResponse(ExecutionResult.FAIL_RETRY)
-                NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
+                NetworkUtils.ResponseStatusType.UNAUTHORIZED -> {
+                    _identityModelStore.invalidateJwt()
                     ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
+                }
                 else ->
                     ExecutionResponse(ExecutionResult.FAIL_PAUSE_OPREPO)
             }

diff --git a/...ava/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt b/...ava/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt
@@ -130,10 +130,7 @@ internal class RefreshUserOperationExecutor(
                 NetworkUtils.ResponseStatusType.RETRYABLE ->
                     ExecutionResponse(ExecutionResult.FAIL_RETRY)
                 NetworkUtils.ResponseStatusType.UNAUTHORIZED -> {
-                    _identityModelStore.model.setStringProperty(
-                            IdentityConstants.JWT_TOKEN,
-                            "",
-                    )
+                    _identityModelStore.invalidateJwt()
                     ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
                 }
                 NetworkUtils.ResponseStatusType.MISSING -> {

diff --git a/...va/com/onesignal/user/internal/operations/impl/executors/SubscriptionOperationExecutor.kt b/...va/com/onesignal/user/internal/operations/impl/executors/SubscriptionOperationExecutor.kt
@@ -129,8 +129,10 @@ internal class SubscriptionOperationExecutor(
                 NetworkUtils.ResponseStatusType.INVALID,
                 ->
                     ExecutionResponse(ExecutionResult.FAIL_NORETRY)
-                NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
+                NetworkUtils.ResponseStatusType.UNAUTHORIZED -> {
+                    _identityModelStore.invalidateJwt()
                     ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
+                }
                 NetworkUtils.ResponseStatusType.MISSING -> {
                     val operations = _buildUserService.getRebuildOperationsIfCurrentUser(createOperation.appId, createOperation.onesignalId)
                     if (operations == null) {

diff --git a/...java/com/onesignal/user/internal/operations/impl/executors/UpdateUserOperationExecutor.kt b/...java/com/onesignal/user/internal/operations/impl/executors/UpdateUserOperationExecutor.kt
@@ -160,8 +160,10 @@ internal class UpdateUserOperationExecutor(
                 return when (responseType) {
                     NetworkUtils.ResponseStatusType.RETRYABLE ->
                         ExecutionResponse(ExecutionResult.FAIL_RETRY)
-                    NetworkUtils.ResponseStatusType.UNAUTHORIZED ->
+                    NetworkUtils.ResponseStatusType.UNAUTHORIZED -> {
+                        _identityModelStore.invalidateJwt()
                         ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
+                    }
                     NetworkUtils.ResponseStatusType.MISSING -> {
                         val operations = _buildUserService.getRebuildOperationsIfCurrentUser(appId, onesignalId)
                         if (operations == null) {

diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserManagerTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserManagerTests.kt
@@ -1,17 +1,33 @@
 package com.onesignal.user.internal
 
+import com.onesignal.IUserJwtInvalidatedListener
+import com.onesignal.common.modeling.ModelChangeTags
+import com.onesignal.common.modeling.ModelChangedArgs
 import com.onesignal.core.internal.language.ILanguageContext
+import com.onesignal.core.internal.operations.ExecutionResponse
+import com.onesignal.core.internal.operations.ExecutionResult
+import com.onesignal.core.internal.operations.Operation
 import com.onesignal.mocks.MockHelper
+import com.onesignal.user.internal.backend.CreateUserResponse
+import com.onesignal.user.internal.backend.IUserBackendService
+import com.onesignal.user.internal.backend.IdentityConstants
+import com.onesignal.user.internal.backend.PropertiesObject
+import com.onesignal.user.internal.operations.LoginUserOperation
+import com.onesignal.user.internal.operations.impl.executors.IdentityOperationExecutor
+import com.onesignal.user.internal.operations.impl.executors.LoginUserOperationExecutor
 import com.onesignal.user.internal.subscriptions.ISubscriptionManager
 import com.onesignal.user.internal.subscriptions.SubscriptionList
+import com.onesignal.user.internal.subscriptions.SubscriptionModelStore
 import io.kotest.core.spec.style.FunSpec
 import io.kotest.matchers.shouldBe
 import io.kotest.matchers.shouldNotBe
+import io.mockk.coEvery
 import io.mockk.every
 import io.mockk.just
 import io.mockk.mockk
 import io.mockk.runs
 import io.mockk.slot
+import io.mockk.spyk
 import io.mockk.verify
 
 class UserManagerTests : FunSpec({
@@ -191,4 +207,48 @@ class UserManagerTests : FunSpec({
         verify(exactly = 1) { mockSubscriptionManager.addSmsSubscription("+15558675309") }
         verify(exactly = 1) { mockSubscriptionManager.removeSmsSubscription("+15558675309") }
     }
+
+    test("login user with jwt calls onUserJwtInvalidated() when the jwt is unauthorized") {
+        // Given
+        val appId = "appId"
+        val localOneSignalId = "local-onesignalId"
+        val remoteOneSignalId = "remote-onesignalId"
+
+        // mock components
+        val mockSubscriptionManager = mockk<ISubscriptionManager>()
+        val mockIdentityModelStore = MockHelper.identityModelStore()
+        val mockPropertiesModelStore = MockHelper.propertiesModelStore()
+        val mockSubscriptionsModelStore = mockk<SubscriptionModelStore>()
+        val mockLanguageContext = MockHelper.languageContext()
+
+        // mock backend service
+        val mockUserBackendService = mockk<IUserBackendService>()
+        coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns
+                CreateUserResponse(mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId), PropertiesObject(), listOf())
+
+        // mock operation for login user
+        val mockIdentityOperationExecutor = mockk<IdentityOperationExecutor>()
+        coEvery { mockIdentityOperationExecutor.execute(any()) } returns
+                ExecutionResponse(ExecutionResult.FAIL_UNAUTHORIZED)
+        val loginUserOperationExecutor =
+                LoginUserOperationExecutor(mockIdentityOperationExecutor, MockHelper.applicationService(), MockHelper.deviceService(), mockUserBackendService, mockIdentityModelStore, mockPropertiesModelStore, mockSubscriptionsModelStore, MockHelper.configModelStore(), mockLanguageContext)
+        val operations = listOf<Operation>(LoginUserOperation(appId, localOneSignalId, "externalId", "existingOneSignalId"))
+
+        // mock user manager with jwtInvalidatedListener added
+        val userManager =
+                UserManager(mockSubscriptionManager, mockIdentityModelStore, mockPropertiesModelStore, mockLanguageContext)
+        mockIdentityModelStore.subscribe(userManager)
+        val spyJwtInvalidatedListener = spyk<IUserJwtInvalidatedListener>()
+        userManager.addUserJwtInvalidatedListner(spyJwtInvalidatedListener)
+
+        // When
+        val response = loginUserOperationExecutor.execute(operations)
+
+        // Then
+        userManager.jwtInvalidatedCallback.hasSubscribers shouldBe true
+        response.result shouldBe ExecutionResult.FAIL_UNAUTHORIZED
+        verify(exactly = 1) { mockIdentityModelStore.invalidateJwt() }
+        // Note: set the default value of useIdentityVerification in OneSignalImp.kt to pass the test
+        verify(exactly = 1) { spyJwtInvalidatedListener.onUserJwtInvalidated(any()) }
+    }
 })