From d7c8cbdc34e8910ef80ea2da6c2620e0c4cfc3e9 Mon Sep 17 00:00:00 2001
From: greenart7c3 <greenart7c3@proton.me>
Date: Wed, 18 Dec 2024 08:37:28 -0300
Subject: [PATCH] Fix some racing conditions when using bunker

---
 .idea/inspectionProfiles/Project_Default.xml  |  1 +
 .../nostrsigner/service/AmberUtils.kt         | 17 +++--
 .../service/EventNotificationConsumer.kt      | 70 +++++++++++++++--
 .../nostrsigner/service/IntentUtils.kt        |  7 +-
 .../nostrsigner/service/NotificationUtils.kt  |  2 +-
 .../nostrsigner/ui/AccountScreen.kt           | 12 ++-
 .../nostrsigner/ui/IncomingRequestScreen.kt   |  2 +-
 .../greenart7c3/nostrsigner/ui/MainScreen.kt  | 75 ++++++++++++-------
 .../ui/components/MultiEventHomeScreen.kt     | 15 +++-
 .../ui/components/SingleEventHomeScreen.kt    | 11 +--
 10 files changed, 158 insertions(+), 54 deletions(-)

diff --git a/.idea/inspectionProfiles/Project_Default.xml b/.idea/inspectionProfiles/Project_Default.xml
index e0da3ee5..afb1d81f 100644
--- a/.idea/inspectionProfiles/Project_Default.xml
+++ b/.idea/inspectionProfiles/Project_Default.xml
@@ -1,6 +1,7 @@
 <component name="InspectionProjectProfileManager">
   <profile version="1.0">
     <option name="myName" value="Project Default" />
+    <inspection_tool class="AndroidLintUnsafeImplicitIntentLaunch" enabled="false" level="ERROR" enabled_by_default="false" />
     <inspection_tool class="ComposePreviewDimensionRespectsLimit" enabled="true" level="WARNING" enabled_by_default="true">
       <option name="composableFile" value="true" />
       <option name="previewFile" value="true" />
diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/AmberUtils.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/AmberUtils.kt
index 671641b3..93532117 100644
--- a/app/src/main/java/com/greenart7c3/nostrsigner/service/AmberUtils.kt
+++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/AmberUtils.kt
@@ -11,6 +11,7 @@ import com.greenart7c3.nostrsigner.models.BunkerResponse
 import com.greenart7c3.nostrsigner.models.IntentData
 import com.greenart7c3.nostrsigner.models.SignerType
 import com.greenart7c3.nostrsigner.models.kindToNip
+import com.greenart7c3.nostrsigner.ui.IntentResultType
 import com.vitorpamplona.ammolite.relays.RelaySetupInfo
 import com.vitorpamplona.quartz.crypto.CryptoUtils
 import com.vitorpamplona.quartz.encoders.HexKey
@@ -127,7 +128,7 @@ object AmberUtils {
         relays: List<RelaySetupInfo>,
         context: Context,
         closeApplication: Boolean,
-        onRemoveIntentData: (IntentData) -> Unit,
+        onRemoveIntentData: (IntentData, IntentResultType) -> Unit,
         onLoading: (Boolean) -> Unit,
     ) {
         IntentUtils.sendBunkerResponse(
@@ -137,11 +138,15 @@ object AmberUtils {
             BunkerResponse(bunkerRequest.id, "", "user rejected"),
             relays,
             onLoading = onLoading,
-            onDone = {
-                onRemoveIntentData(intentData)
-                context.getAppCompatActivity()?.intent = null
-                if (closeApplication) {
-                    context.getAppCompatActivity()?.finish()
+            onDone = { result ->
+                if (!result) {
+                    onLoading(false)
+                } else {
+                    onRemoveIntentData(intentData, IntentResultType.REMOVE)
+                    context.getAppCompatActivity()?.intent = null
+                    if (closeApplication) {
+                        context.getAppCompatActivity()?.finish()
+                    }
                 }
             },
         )
diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumer.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumer.kt
index 83a187e8..69a8e60f 100644
--- a/app/src/main/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumer.kt
+++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumer.kt
@@ -135,7 +135,19 @@ class EventNotificationConsumer(private val applicationContext: Context) {
                 BunkerResponse(bunkerRequest.id, bunkerRequest.nostrConnectSecret.ifBlank { "ack" }, null),
                 permission.application.relays,
                 onLoading = {},
-                onDone = {},
+                onDone = {
+                    if (!it) {
+                        IntentUtils.sendBunkerResponse(
+                            applicationContext,
+                            acc,
+                            bunkerRequest,
+                            BunkerResponse(bunkerRequest.id, bunkerRequest.nostrConnectSecret.ifBlank { "ack" }, null),
+                            permission.application.relays,
+                            onLoading = {},
+                            onDone = {},
+                        )
+                    }
+                },
             )
             return
         }
@@ -169,7 +181,19 @@ class EventNotificationConsumer(private val applicationContext: Context) {
                         BunkerResponse(bunkerRequest.id, "", message),
                         applicationWithSecret?.application?.relays ?: NostrSigner.getInstance().getSavedRelays().toList(),
                         onLoading = { },
-                        onDone = { },
+                        onDone = {
+                            if (!it) {
+                                IntentUtils.sendBunkerResponse(
+                                    applicationContext,
+                                    acc,
+                                    bunkerRequest,
+                                    BunkerResponse(bunkerRequest.id, "", message),
+                                    applicationWithSecret?.application?.relays ?: NostrSigner.getInstance().getSavedRelays().toList(),
+                                    onLoading = { },
+                                    onDone = { },
+                                )
+                            }
+                        },
                     )
                     return
                 }
@@ -218,7 +242,19 @@ class EventNotificationConsumer(private val applicationContext: Context) {
                 BunkerResponse(bunkerRequest.id, "", "no permission"),
                 relays,
                 onLoading = { },
-                onDone = {},
+                onDone = {
+                    if (!it) {
+                        IntentUtils.sendBunkerResponse(
+                            applicationContext,
+                            acc,
+                            bunkerRequest,
+                            BunkerResponse(bunkerRequest.id, "", "no permission"),
+                            relays,
+                            onLoading = { },
+                            onDone = {},
+                        )
+                    }
+                },
             )
             return
         }
@@ -245,7 +281,19 @@ class EventNotificationConsumer(private val applicationContext: Context) {
                             BunkerResponse(bunkerRequest.id, "", "user rejected"),
                             relays,
                             onLoading = { },
-                            onDone = { },
+                            onDone = {
+                                if (!it) {
+                                    IntentUtils.sendBunkerResponse(
+                                        applicationContext,
+                                        acc,
+                                        bunkerRequest,
+                                        BunkerResponse(bunkerRequest.id, "", "user rejected"),
+                                        relays,
+                                        onLoading = { },
+                                        onDone = { },
+                                    )
+                                }
+                            },
                         )
                     } else {
                         var index = localCursor.getColumnIndex("event")
@@ -261,7 +309,19 @@ class EventNotificationConsumer(private val applicationContext: Context) {
                             BunkerResponse(bunkerRequest.id, result, null),
                             relays,
                             onLoading = { },
-                            onDone = { },
+                            onDone = {
+                                if (!it) {
+                                    IntentUtils.sendBunkerResponse(
+                                        applicationContext,
+                                        acc,
+                                        bunkerRequest,
+                                        BunkerResponse(bunkerRequest.id, result, null),
+                                        relays,
+                                        onLoading = { },
+                                        onDone = { },
+                                    )
+                                }
+                            },
                         )
                     }
                 } else {
diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/IntentUtils.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/IntentUtils.kt
index cb789e56..40fc69f9 100644
--- a/app/src/main/java/com/greenart7c3/nostrsigner/service/IntentUtils.kt
+++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/IntentUtils.kt
@@ -322,7 +322,7 @@ object IntentUtils {
         bunkerResponse: BunkerResponse,
         relays: List<RelaySetupInfo>,
         onLoading: (Boolean) -> Unit,
-        onDone: () -> Unit,
+        onDone: (Boolean) -> Unit,
     ) {
         AmberListenerSingleton.getListener()?.let {
             Client.unsubscribe(it)
@@ -379,7 +379,7 @@ object IntentUtils {
         encryptedContent: String,
         relays: List<RelaySetupInfo>,
         onLoading: (Boolean) -> Unit,
-        onDone: () -> Unit,
+        onDone: (Boolean) -> Unit,
     ) {
         account.signer.sign<Event>(
             TimeUtils.now(),
@@ -399,8 +399,9 @@ object IntentUtils {
                 val success = Client.sendAndWaitForResponse(it, relayList = relays)
                 if (success) {
                     Log.d("IntentUtils", "Success response to relays ${relays.map { it.url }} type ${bunkerRequest?.method}")
-                    onDone()
+                    onDone(true)
                 } else {
+                    onDone(false)
                     Log.d("IntentUtils", "Failed response to relays ${relays.map { it.url }} type ${bunkerRequest?.method}")
                 }
                 AmberListenerSingleton.getListener()?.let {
diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/NotificationUtils.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/NotificationUtils.kt
index b3309120..586bb41f 100644
--- a/app/src/main/java/com/greenart7c3/nostrsigner/service/NotificationUtils.kt
+++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/NotificationUtils.kt
@@ -92,7 +92,7 @@ object NotificationUtils {
         bunkerRequest: BunkerRequest,
     ) {
         val notId = id.hashCode()
-        val notifications: Array<StatusBarNotification> = getActiveNotifications()
+        val notifications: Array<StatusBarNotification> = activeNotifications
         for (notification in notifications) {
             if (notification.id == notId) {
                 return
diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/ui/AccountScreen.kt b/app/src/main/java/com/greenart7c3/nostrsigner/ui/AccountScreen.kt
index 85bb7da3..cd457ec8 100644
--- a/app/src/main/java/com/greenart7c3/nostrsigner/ui/AccountScreen.kt
+++ b/app/src/main/java/com/greenart7c3/nostrsigner/ui/AccountScreen.kt
@@ -140,9 +140,17 @@ fun AccountScreen(
                         database,
                         navController,
                         storageHelper,
-                        onRemoveIntentData = {
+                        onRemoveIntentData = { intentData, type ->
                             val oldIntents = intents.toMutableList()
-                            oldIntents.remove(it)
+                            when (type) {
+                                IntentResultType.ADD -> {
+                                    oldIntents.add(intentData)
+                                }
+                                IntentResultType.REMOVE -> {
+                                    oldIntents.remove(intentData)
+                                }
+                            }
+
                             flow.value = oldIntents
                         },
                     )
diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/ui/IncomingRequestScreen.kt b/app/src/main/java/com/greenart7c3/nostrsigner/ui/IncomingRequestScreen.kt
index 3702742c..cebf069e 100644
--- a/app/src/main/java/com/greenart7c3/nostrsigner/ui/IncomingRequestScreen.kt
+++ b/app/src/main/java/com/greenart7c3/nostrsigner/ui/IncomingRequestScreen.kt
@@ -45,7 +45,7 @@ fun IncomingRequestScreen(
     account: Account,
     database: AppDatabase,
     navController: NavController,
-    onRemoveIntentData: (IntentData) -> Unit,
+    onRemoveIntentData: (IntentData, IntentResultType) -> Unit,
 ) {
     var loading by remember { mutableStateOf(false) }
     val context = LocalContext.current
diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/ui/MainScreen.kt b/app/src/main/java/com/greenart7c3/nostrsigner/ui/MainScreen.kt
index 2b005b98..cee4eb53 100644
--- a/app/src/main/java/com/greenart7c3/nostrsigner/ui/MainScreen.kt
+++ b/app/src/main/java/com/greenart7c3/nostrsigner/ui/MainScreen.kt
@@ -140,7 +140,7 @@ fun sendResult(
     permissions: List<Permission>? = null,
     appName: String? = null,
     signPolicy: Int? = null,
-    onRemoveIntentData: (IntentData) -> Unit,
+    onRemoveIntentData: (IntentData, IntentResultType) -> Unit,
 ) {
     onLoading(true)
     GlobalScope.launch(Dispatchers.IO) {
@@ -254,7 +254,10 @@ fun sendResult(
             )
         }
 
-        if (intentData.bunkerRequest == null && intentData.type == SignerType.GET_PUBLIC_KEY) {
+        if (intentData.type == SignerType.CONNECT) {
+            database.applicationDao().deletePermissions(key)
+        }
+        if ((intentData.bunkerRequest == null && intentData.type == SignerType.GET_PUBLIC_KEY) || (intentData.bunkerRequest != null && intentData.type == SignerType.CONNECT)) {
             if (!application.permissions.any { it.type == SignerType.GET_PUBLIC_KEY.toString() }) {
                 application.permissions.add(
                     ApplicationPermissionsEntity(
@@ -269,6 +272,25 @@ fun sendResult(
         }
 
         if (intentData.bunkerRequest != null) {
+            val localIntentData = intentData.copy()
+
+            // assume that everything worked and try to revert it if it fails
+            EventNotificationConsumer(context).notificationManager().cancelAll()
+            database.applicationDao().insertApplicationWithPermissions(application)
+            database.applicationDao().addHistory(
+                HistoryEntity(
+                    0,
+                    key,
+                    intentData.type.toString(),
+                    kind,
+                    TimeUtils.now(),
+                    true,
+                ),
+            )
+
+            EventNotificationConsumer(context).notificationManager().cancelAll()
+            onRemoveIntentData(intentData, IntentResultType.REMOVE)
+
             IntentUtils.sendBunkerResponse(
                 context,
                 account,
@@ -277,27 +299,21 @@ fun sendResult(
                 application.application.relays.ifEmpty { relays },
                 onLoading,
                 onDone = {
-                    EventNotificationConsumer(context).notificationManager().cancelAll()
-                    if (intentData.type == SignerType.CONNECT) {
-                        database.applicationDao().deletePermissions(key)
-                    }
-                    database.applicationDao().insertApplicationWithPermissions(application)
-                    database.applicationDao().addHistory(
-                        HistoryEntity(
-                            0,
-                            key,
-                            intentData.type.toString(),
-                            kind,
-                            TimeUtils.now(),
-                            true,
-                        ),
-                    )
-
-                    EventNotificationConsumer(context).notificationManager().cancelAll()
-                    onRemoveIntentData(intentData)
-                    activity?.intent = null
-                    if (application.application.closeApplication || intentData.bunkerRequest.closeApplication) {
-                        activity?.finish()
+                    if (!it) {
+                        if (intentData.type == SignerType.SIGN_EVENT) {
+                            kind?.let {
+                                database.applicationDao().deletePermissions(key, intentData.type.toString(), kind)
+                            }
+                        } else {
+                            database.applicationDao().deletePermissions(key, intentData.type.toString())
+                        }
+                        onLoading(false)
+                        onRemoveIntentData(localIntentData, IntentResultType.ADD)
+                    } else {
+                        activity?.intent = null
+                        if (application.application.closeApplication || intentData.bunkerRequest.closeApplication) {
+                            activity?.finish()
+                        }
                     }
                 },
             )
@@ -323,7 +339,7 @@ fun sendResult(
                 intent.putExtra("package", BuildConfig.APPLICATION_ID)
             }
             activity?.setResult(RESULT_OK, intent)
-            onRemoveIntentData(intentData)
+            onRemoveIntentData(intentData, IntentResultType.REMOVE)
             activity?.intent = null
             activity?.finish()
         } else if (!intentData.callBackUrl.isNullOrBlank()) {
@@ -351,7 +367,7 @@ fun sendResult(
                     context.startActivity(intent)
                 }
             }
-            onRemoveIntentData(intentData)
+            onRemoveIntentData(intentData, IntentResultType.REMOVE)
             activity?.intent = null
             activity?.finish()
         } else {
@@ -377,7 +393,7 @@ fun sendResult(
                     Toast.LENGTH_SHORT,
                 ).show()
             }
-            onRemoveIntentData(intentData)
+            onRemoveIntentData(intentData, IntentResultType.REMOVE)
             activity?.intent = null
             activity?.finish()
         }
@@ -445,6 +461,11 @@ fun Color.Companion.fromHex(colorString: String) = try {
     Unspecified
 }
 
+enum class IntentResultType {
+    REMOVE,
+    ADD,
+}
+
 @OptIn(ExperimentalMaterial3Api::class)
 @Composable
 fun MainScreen(
@@ -457,7 +478,7 @@ fun MainScreen(
     database: AppDatabase,
     navController: NavHostController,
     storageHelper: SimpleStorageHelper,
-    onRemoveIntentData: (IntentData) -> Unit,
+    onRemoveIntentData: (IntentData, IntentResultType) -> Unit,
 ) {
     val navBackStackEntry by navController.currentBackStackEntryAsState()
     val destinationRoute = navBackStackEntry?.destination?.route ?: ""
diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/ui/components/MultiEventHomeScreen.kt b/app/src/main/java/com/greenart7c3/nostrsigner/ui/components/MultiEventHomeScreen.kt
index 4f00780d..57a9dd93 100644
--- a/app/src/main/java/com/greenart7c3/nostrsigner/ui/components/MultiEventHomeScreen.kt
+++ b/app/src/main/java/com/greenart7c3/nostrsigner/ui/components/MultiEventHomeScreen.kt
@@ -67,6 +67,7 @@ import com.greenart7c3.nostrsigner.service.IntentUtils
 import com.greenart7c3.nostrsigner.service.MultiEventScreenIntents
 import com.greenart7c3.nostrsigner.service.getAppCompatActivity
 import com.greenart7c3.nostrsigner.service.toShortenHex
+import com.greenart7c3.nostrsigner.ui.IntentResultType
 import com.greenart7c3.nostrsigner.ui.Result
 import com.greenart7c3.nostrsigner.ui.navigation.Route
 import com.greenart7c3.nostrsigner.ui.theme.orange
@@ -84,7 +85,7 @@ fun MultiEventHomeScreen(
     packageName: String?,
     accountParam: Account,
     navController: NavController,
-    onRemoveIntentData: (IntentData) -> Unit,
+    onRemoveIntentData: (IntentData, IntentResultType) -> Unit,
     onLoading: (Boolean) -> Unit,
 ) {
     val coroutineScope = rememberCoroutineScope()
@@ -254,7 +255,9 @@ fun MultiEventHomeScreen(
                                             application.application.relays,
                                             onLoading = {},
                                             onDone = {
-                                                onRemoveIntentData(intentData)
+                                                if (it) {
+                                                    onRemoveIntentData(intentData, IntentResultType.REMOVE)
+                                                }
                                             },
                                         )
                                     } else {
@@ -333,7 +336,9 @@ fun MultiEventHomeScreen(
                                             application.application.relays,
                                             onLoading = {},
                                             onDone = {
-                                                onRemoveIntentData(intentData)
+                                                if (it) {
+                                                    onRemoveIntentData(intentData, IntentResultType.REMOVE)
+                                                }
                                             },
                                         )
                                     } else {
@@ -397,7 +402,9 @@ fun MultiEventHomeScreen(
                                             application.application.relays,
                                             onLoading = {},
                                             onDone = {
-                                                onRemoveIntentData(intentData)
+                                                if (it) {
+                                                    onRemoveIntentData(intentData, IntentResultType.REMOVE)
+                                                }
                                             },
                                         )
                                     } else {
diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/ui/components/SingleEventHomeScreen.kt b/app/src/main/java/com/greenart7c3/nostrsigner/ui/components/SingleEventHomeScreen.kt
index 2445cd29..e264ee99 100644
--- a/app/src/main/java/com/greenart7c3/nostrsigner/ui/components/SingleEventHomeScreen.kt
+++ b/app/src/main/java/com/greenart7c3/nostrsigner/ui/components/SingleEventHomeScreen.kt
@@ -37,6 +37,7 @@ import com.greenart7c3.nostrsigner.models.kindToNip
 import com.greenart7c3.nostrsigner.service.AmberUtils
 import com.greenart7c3.nostrsigner.service.getAppCompatActivity
 import com.greenart7c3.nostrsigner.service.toShortenHex
+import com.greenart7c3.nostrsigner.ui.IntentResultType
 import com.greenart7c3.nostrsigner.ui.sendResult
 import com.vitorpamplona.quartz.crypto.CryptoUtils
 import com.vitorpamplona.quartz.encoders.bechToBytes
@@ -55,7 +56,7 @@ fun SingleEventHomeScreen(
     intentData: IntentData,
     account: Account,
     database: AppDatabase,
-    onRemoveIntentData: (IntentData) -> Unit,
+    onRemoveIntentData: (IntentData, IntentResultType) -> Unit,
     onLoading: (Boolean) -> Unit,
 ) {
     var applicationEntity by remember {
@@ -158,7 +159,7 @@ fun SingleEventHomeScreen(
                             )
                         }
                     } else {
-                        onRemoveIntentData(intentData)
+                        onRemoveIntentData(intentData, IntentResultType.REMOVE)
                         context.getAppCompatActivity()?.intent = null
                         context.getAppCompatActivity()?.finish()
                     }
@@ -284,7 +285,7 @@ fun SingleEventHomeScreen(
                                 onRemoveIntentData = onRemoveIntentData,
                             )
                         } else {
-                            onRemoveIntentData(intentData)
+                            onRemoveIntentData(intentData, IntentResultType.REMOVE)
                             context.getAppCompatActivity()?.intent = null
                             if (application.application.closeApplication) {
                                 context.getAppCompatActivity()?.finish()
@@ -428,7 +429,7 @@ fun SingleEventHomeScreen(
                                 onLoading,
                             )
                         } else {
-                            onRemoveIntentData(intentData)
+                            onRemoveIntentData(intentData, IntentResultType.REMOVE)
                             context.getAppCompatActivity()?.intent = null
                             if (application.application.closeApplication) {
                                 context.getAppCompatActivity()?.finish()
@@ -597,7 +598,7 @@ fun SingleEventHomeScreen(
                                     onRemoveIntentData = onRemoveIntentData,
                                 )
                             } else {
-                                onRemoveIntentData(intentData)
+                                onRemoveIntentData(intentData, IntentResultType.REMOVE)
                                 context.getAppCompatActivity()?.intent = null
                                 if (application.application.closeApplication) {
                                     context.getAppCompatActivity()?.finish()