From c9dc54748c26301f6204e91e0a5708dba24ff811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Mon, 23 Feb 2026 14:31:01 -0500 Subject: [PATCH 1/5] Fix P10 and security review findings Security: remove debug log leaking group pubkey, add timeout to BunkerService runBlocking, narrow catch(Throwable) to catch(Exception). P10 Rule 5: add require() preconditions to AndroidKeystoreStorage and BunkerService security-critical functions. P10 Rule 7: replace !! with safe local variable captures, fix delegated property smart cast issues. P10 Rule 4: decompose oversized functions in MainActivity, ExportShareScreen, BunkerScreen, AppPermissionsScreen, PermissionsManagementScreen, SigningHistoryScreen, ImportShareScreen, and Nip55ContentProvider. --- .../io/privkey/keep/ExportShareScreen.kt | 363 +++++++++-------- .../io/privkey/keep/ImportNsecScreen.kt | 5 +- .../io/privkey/keep/ImportShareScreen.kt | 224 ++++++----- .../kotlin/io/privkey/keep/KeepMobileApp.kt | 2 +- .../kotlin/io/privkey/keep/MainActivity.kt | 293 ++++++++------ .../io/privkey/keep/nip46/BunkerScreen.kt | 233 ++++++----- .../io/privkey/keep/nip46/BunkerService.kt | 6 +- .../keep/nip46/Nip46ApprovalActivity.kt | 4 +- .../keep/nip55/AppPermissionsScreen.kt | 372 ++++++++++-------- .../privkey/keep/nip55/ConnectedAppsScreen.kt | 3 +- .../keep/nip55/Nip55ContentProvider.kt | 67 +++- .../keep/nip55/PermissionsManagementScreen.kt | 229 ++++++----- .../keep/nip55/SigningHistoryScreen.kt | 189 +++++---- .../keep/storage/AndroidKeystoreStorage.kt | 5 + 14 files changed, 1177 insertions(+), 818 deletions(-) diff --git a/app/src/main/kotlin/io/privkey/keep/ExportShareScreen.kt b/app/src/main/kotlin/io/privkey/keep/ExportShareScreen.kt index d35ec3e4..2ff7a9e8 100644 --- a/app/src/main/kotlin/io/privkey/keep/ExportShareScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/ExportShareScreen.kt @@ -101,6 +101,129 @@ private fun calculatePassphraseStrength(passphrase: SecurePassphrase): Passphras } } +@Composable +private fun PassphraseStrengthIndicator(strength: PassphraseStrength) { + Row( + modifier = Modifier.fillMaxWidth(), + verticalAlignment = Alignment.CenterVertically + ) { + LinearProgressIndicator( + progress = { (strength.ordinal + 1) / 4f }, + modifier = Modifier.weight(1f).height(4.dp), + color = strength.color(), + trackColor = MaterialTheme.colorScheme.surfaceVariant + ) + Spacer(modifier = Modifier.width(8.dp)) + Text( + text = strength.label, + style = MaterialTheme.typography.labelSmall, + color = strength.color() + ) + } +} + +@Composable +private fun ExportInputForm( + errorMessage: String?, + cipherError: String?, + passphrase: SecurePassphrase, + confirmPassphrase: SecurePassphrase, + passphraseDisplay: String, + confirmPassphraseDisplay: String, + onPassphraseChange: (String) -> Unit, + onConfirmPassphraseChange: (String) -> Unit, + onExport: () -> Unit, + onCancel: () -> Unit +) { + Card( + modifier = Modifier.fillMaxWidth(), + colors = CardDefaults.cardColors(containerColor = MaterialTheme.colorScheme.tertiaryContainer) + ) { + Text( + text = "Store this export securely. Anyone with this backup and passphrase can access your signing key share.", + modifier = Modifier.padding(16.dp), + color = MaterialTheme.colorScheme.onTertiaryContainer, + style = MaterialTheme.typography.bodySmall + ) + } + + Spacer(modifier = Modifier.height(16.dp)) + + errorMessage?.let { + ErrorCard(it) + Spacer(modifier = Modifier.height(16.dp)) + } + + cipherError?.let { + ErrorCard(it) + Spacer(modifier = Modifier.height(16.dp)) + } + + OutlinedTextField( + value = passphraseDisplay, + onValueChange = onPassphraseChange, + label = { Text("Export Passphrase") }, + placeholder = { Text("Enter a passphrase to encrypt") }, + modifier = Modifier.fillMaxWidth(), + visualTransformation = PasswordVisualTransformation(), + keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Password), + singleLine = true + ) + + if (passphrase.length > 0) { + Spacer(modifier = Modifier.height(8.dp)) + PassphraseStrengthIndicator(calculatePassphraseStrength(passphrase)) + } + + Spacer(modifier = Modifier.height(16.dp)) + + OutlinedTextField( + value = confirmPassphraseDisplay, + onValueChange = onConfirmPassphraseChange, + label = { Text("Confirm Passphrase") }, + placeholder = { Text("Re-enter passphrase") }, + modifier = Modifier.fillMaxWidth(), + visualTransformation = PasswordVisualTransformation(), + keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Password), + singleLine = true, + isError = confirmPassphrase.length > 0 && !passphrase.contentEquals(confirmPassphrase), + supportingText = if (confirmPassphrase.length > 0 && !passphrase.contentEquals(confirmPassphrase)) { + { Text("Passphrases do not match") } + } else null + ) + + Spacer(modifier = Modifier.height(16.dp)) + + Text( + text = "This passphrase encrypts the exported share. You will need it to import the share on another device.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + + Spacer(modifier = Modifier.height(24.dp)) + + Row( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.spacedBy(16.dp) + ) { + OutlinedButton( + onClick = onCancel, + modifier = Modifier.weight(1f) + ) { + Text("Cancel") + } + Button( + onClick = onExport, + modifier = Modifier.weight(1f), + enabled = passphrase.length >= MIN_PASSPHRASE_LENGTH && + passphrase.contentEquals(confirmPassphrase) && + calculatePassphraseStrength(passphrase) != PassphraseStrength.WEAK + ) { + Text("Export") + } + } +} + @OptIn(ExperimentalMaterial3Api::class) @Composable fun ExportShareScreen( @@ -171,186 +294,94 @@ fun ExportShareScreen( when (val state = exportState) { is ExportState.Idle, is ExportState.Error -> { - Card( - modifier = Modifier.fillMaxWidth(), - colors = CardDefaults.cardColors(containerColor = MaterialTheme.colorScheme.tertiaryContainer) - ) { - Text( - text = "Store this export securely. Anyone with this backup and passphrase can access your signing key share.", - modifier = Modifier.padding(16.dp), - color = MaterialTheme.colorScheme.onTertiaryContainer, - style = MaterialTheme.typography.bodySmall - ) - } - - Spacer(modifier = Modifier.height(16.dp)) - - if (state is ExportState.Error) { - ErrorCard(state.message) - Spacer(modifier = Modifier.height(16.dp)) - } - - cipherError?.let { - ErrorCard(it) - Spacer(modifier = Modifier.height(16.dp)) - } - - OutlinedTextField( - value = passphraseDisplay, - onValueChange = { + ExportInputForm( + errorMessage = (state as? ExportState.Error)?.message, + cipherError = cipherError, + passphrase = passphrase, + confirmPassphrase = confirmPassphrase, + passphraseDisplay = passphraseDisplay, + confirmPassphraseDisplay = confirmPassphraseDisplay, + onPassphraseChange = { passphrase.update(it) passphraseDisplay = it }, - label = { Text("Export Passphrase") }, - placeholder = { Text("Enter a passphrase to encrypt") }, - modifier = Modifier.fillMaxWidth(), - visualTransformation = PasswordVisualTransformation(), - keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Password), - singleLine = true - ) - - if (passphrase.length > 0) { - Spacer(modifier = Modifier.height(8.dp)) - val strength = calculatePassphraseStrength(passphrase) - Row( - modifier = Modifier.fillMaxWidth(), - verticalAlignment = Alignment.CenterVertically - ) { - LinearProgressIndicator( - progress = { (strength.ordinal + 1) / 4f }, - modifier = Modifier.weight(1f).height(4.dp), - color = strength.color(), - trackColor = MaterialTheme.colorScheme.surfaceVariant - ) - Spacer(modifier = Modifier.width(8.dp)) - Text( - text = strength.label, - style = MaterialTheme.typography.labelSmall, - color = strength.color() - ) - } - } - - Spacer(modifier = Modifier.height(16.dp)) - - OutlinedTextField( - value = confirmPassphraseDisplay, - onValueChange = { + onConfirmPassphraseChange = { confirmPassphrase.update(it) confirmPassphraseDisplay = it }, - label = { Text("Confirm Passphrase") }, - placeholder = { Text("Re-enter passphrase") }, - modifier = Modifier.fillMaxWidth(), - visualTransformation = PasswordVisualTransformation(), - keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Password), - singleLine = true, - isError = confirmPassphrase.length > 0 && !passphrase.contentEquals(confirmPassphrase), - supportingText = if (confirmPassphrase.length > 0 && !passphrase.contentEquals(confirmPassphrase)) { - { Text("Passphrases do not match") } - } else null - ) - - Spacer(modifier = Modifier.height(16.dp)) - - Text( - text = "This passphrase encrypts the exported share. You will need it to import the share on another device.", - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant - ) - - Spacer(modifier = Modifier.height(24.dp)) - - Row( - modifier = Modifier.fillMaxWidth(), - horizontalArrangement = Arrangement.spacedBy(16.dp) - ) { - OutlinedButton( - onClick = { - passphrase.clear() - confirmPassphrase.clear() - passphraseDisplay = "" - confirmPassphraseDisplay = "" - onDismiss() - }, - modifier = Modifier.weight(1f) - ) { - Text("Cancel") - } - Button( - onClick = { - if (passphrase.length < MIN_PASSPHRASE_LENGTH) { - exportState = ExportState.Error("Passphrase must be at least $MIN_PASSPHRASE_LENGTH characters") - return@Button - } - if (!passphrase.contentEquals(confirmPassphrase)) { - exportState = ExportState.Error("Passphrases do not match") - return@Button - } - if (calculatePassphraseStrength(passphrase) == PassphraseStrength.WEAK) { - exportState = ExportState.Error("Passphrase is too weak. Add length, mixed case, numbers, or symbols.") - return@Button - } - cipherError = null - val cipher = onGetCipher() - if (cipher == null) { - cipherError = "No encryption key available" - return@Button - } - val passphraseChars = passphrase.toCharArray() - fun clearChars() = Arrays.fill(passphraseChars, '\u0000') - try { - onBiometricAuth(cipher) { authedCipher -> - if (authedCipher != null) { - val exportId = java.util.UUID.randomUUID().toString() - storage.setPendingCipher(exportId, authedCipher) - exportState = ExportState.Exporting - coroutineScope.launch { - currentCoroutineContext()[Job]?.invokeOnCompletion { cause -> - if (cause is CancellationException) { - clearChars() - storage.clearPendingCipher(exportId) - } - } - try { - val data = withContext(Dispatchers.IO) { - storage.setRequestIdContext(exportId) - try { - keepMobile.exportShare(String(passphraseChars)) - } finally { - storage.clearRequestIdContext() - } - } - (exportState as? ExportState.Success)?.clear() - exportState = ExportState.Success(data, generateFrames(data, MAX_SINGLE_QR_BYTES)) - } catch (e: Exception) { - if (BuildConfig.DEBUG) Log.e("ExportShare", "Export failed: ${e::class.simpleName}") - exportState = ExportState.Error("Export failed. Please try again.") - } finally { + onExport = { + if (passphrase.length < MIN_PASSPHRASE_LENGTH) { + exportState = ExportState.Error("Passphrase must be at least $MIN_PASSPHRASE_LENGTH characters") + return@ExportInputForm + } + if (!passphrase.contentEquals(confirmPassphrase)) { + exportState = ExportState.Error("Passphrases do not match") + return@ExportInputForm + } + if (calculatePassphraseStrength(passphrase) == PassphraseStrength.WEAK) { + exportState = ExportState.Error("Passphrase is too weak. Add length, mixed case, numbers, or symbols.") + return@ExportInputForm + } + cipherError = null + val cipher = onGetCipher() + if (cipher == null) { + cipherError = "No encryption key available" + return@ExportInputForm + } + val passphraseChars = passphrase.toCharArray() + fun clearChars() = Arrays.fill(passphraseChars, '\u0000') + try { + onBiometricAuth(cipher) { authedCipher -> + if (authedCipher != null) { + val exportId = java.util.UUID.randomUUID().toString() + storage.setPendingCipher(exportId, authedCipher) + exportState = ExportState.Exporting + coroutineScope.launch { + currentCoroutineContext()[Job]?.invokeOnCompletion { cause -> + if (cause is CancellationException) { clearChars() storage.clearPendingCipher(exportId) } } - } else { - clearChars() - exportState = ExportState.Error("Authentication cancelled") - Toast.makeText(context, "Authentication cancelled", Toast.LENGTH_SHORT).show() + try { + val data = withContext(Dispatchers.IO) { + storage.setRequestIdContext(exportId) + try { + keepMobile.exportShare(String(passphraseChars)) + } finally { + storage.clearRequestIdContext() + } + } + (exportState as? ExportState.Success)?.clear() + exportState = ExportState.Success(data, generateFrames(data, MAX_SINGLE_QR_BYTES)) + } catch (e: Exception) { + if (BuildConfig.DEBUG) Log.e("ExportShare", "Export failed: ${e::class.simpleName}") + exportState = ExportState.Error("Export failed. Please try again.") + } finally { + clearChars() + storage.clearPendingCipher(exportId) + } } + } else { + clearChars() + exportState = ExportState.Error("Authentication cancelled") + Toast.makeText(context, "Authentication cancelled", Toast.LENGTH_SHORT).show() } - } catch (e: Exception) { - clearChars() - if (BuildConfig.DEBUG) Log.e("ExportShare", "Failed to init cipher: ${e::class.simpleName}") - cipherError = "Failed to initialize encryption" } - }, - modifier = Modifier.weight(1f), - enabled = passphrase.length >= MIN_PASSPHRASE_LENGTH && - passphrase.contentEquals(confirmPassphrase) && - calculatePassphraseStrength(passphrase) != PassphraseStrength.WEAK - ) { - Text("Export") + } catch (e: Exception) { + clearChars() + if (BuildConfig.DEBUG) Log.e("ExportShare", "Failed to init cipher: ${e::class.simpleName}") + cipherError = "Failed to initialize encryption" + } + }, + onCancel = { + passphrase.clear() + confirmPassphrase.clear() + passphraseDisplay = "" + confirmPassphraseDisplay = "" + onDismiss() } - } + ) } is ExportState.Exporting -> { diff --git a/app/src/main/kotlin/io/privkey/keep/ImportNsecScreen.kt b/app/src/main/kotlin/io/privkey/keep/ImportNsecScreen.kt index b94854ac..28700ab7 100644 --- a/app/src/main/kotlin/io/privkey/keep/ImportNsecScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/ImportNsecScreen.kt @@ -145,9 +145,10 @@ fun ImportNsecScreen( Spacer(modifier = Modifier.height(24.dp)) - if (scanError != null) { + val currentScanError = scanError + if (currentScanError != null) { StatusCard( - text = scanError!!, + text = currentScanError, containerColor = MaterialTheme.colorScheme.errorContainer, contentColor = MaterialTheme.colorScheme.onErrorContainer ) diff --git a/app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt b/app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt index 744084b6..5334baf3 100644 --- a/app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt @@ -123,112 +123,150 @@ fun ImportShareScreen( Spacer(modifier = Modifier.height(24.dp)) - OutlinedTextField( - value = shareDataDisplay, - onValueChange = { - if (it.length <= MAX_SHARE_LENGTH) { - shareData.update(it) - shareDataDisplay = it + ImportShareInputFields( + shareDataDisplay = shareDataDisplay, + onShareDataChange = { value -> + if (value.length <= MAX_SHARE_LENGTH) { + shareData.update(value) + shareDataDisplay = value } }, - label = { Text("Share Data") }, - placeholder = { Text("kshare1q...") }, - modifier = Modifier.fillMaxWidth(), - minLines = 3, - maxLines = 5, - enabled = isInputEnabled + passphraseDisplay = passphraseDisplay, + onPassphraseChange = { value -> + passphrase.update(value) + passphraseDisplay = value + }, + shareName = shareName, + onShareNameChange = { if (it.length <= 64) shareName = it }, + onScanClick = { showScanner = true }, + isInputEnabled = isInputEnabled ) - Spacer(modifier = Modifier.height(8.dp)) + Spacer(modifier = Modifier.height(24.dp)) - OutlinedButton( - onClick = { showScanner = true }, - modifier = Modifier.fillMaxWidth(), - enabled = isInputEnabled - ) { - Text("Scan QR Code") - } + ImportStatusAndActions( + importState = importState, + canImport = shareData.isNotBlank() && passphrase.length > 0 && isInputEnabled, + onDismiss = onDismiss, + onImportClick = { onError -> + val passphraseChars = passphrase.toCharArray() + fun clearChars() = Arrays.fill(passphraseChars, '\u0000') + try { + val cipher = onGetCipher() + onBiometricAuth(cipher) { authedCipher -> + try { + if (authedCipher != null) { + onImport(shareData.valueUnsafe(), String(passphraseChars), shareName, authedCipher) + } + } finally { + clearChars() + } + } + } catch (e: KeyPermanentlyInvalidatedException) { + clearChars() + if (BuildConfig.DEBUG) Log.e("ImportShare", "Biometric key invalidated during cipher init", e) + onError("Biometric key invalidated. Please re-enroll biometrics.") + } catch (e: Exception) { + clearChars() + if (BuildConfig.DEBUG) Log.e("ImportShare", "Failed to initialize cipher for biometric auth", e) + onError("Failed to initialize encryption") + } + } + ) + } +} - Spacer(modifier = Modifier.height(16.dp)) +@Composable +private fun ImportShareInputFields( + shareDataDisplay: String, + onShareDataChange: (String) -> Unit, + passphraseDisplay: String, + onPassphraseChange: (String) -> Unit, + shareName: String, + onShareNameChange: (String) -> Unit, + onScanClick: () -> Unit, + isInputEnabled: Boolean +) { + OutlinedTextField( + value = shareDataDisplay, + onValueChange = onShareDataChange, + label = { Text("Share Data") }, + placeholder = { Text("kshare1q...") }, + modifier = Modifier.fillMaxWidth(), + minLines = 3, + maxLines = 5, + enabled = isInputEnabled + ) - OutlinedTextField( - value = passphraseDisplay, - onValueChange = { - passphrase.update(it) - passphraseDisplay = it - }, - label = { Text("Passphrase") }, - modifier = Modifier.fillMaxWidth(), - visualTransformation = PasswordVisualTransformation(), - keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Password), - singleLine = true, - enabled = isInputEnabled - ) + Spacer(modifier = Modifier.height(8.dp)) - Spacer(modifier = Modifier.height(16.dp)) + OutlinedButton( + onClick = onScanClick, + modifier = Modifier.fillMaxWidth(), + enabled = isInputEnabled + ) { + Text("Scan QR Code") + } - OutlinedTextField( - value = shareName, - onValueChange = { if (it.length <= 64) shareName = it }, - label = { Text("Share Name") }, - modifier = Modifier.fillMaxWidth(), - singleLine = true, - enabled = isInputEnabled - ) + Spacer(modifier = Modifier.height(16.dp)) - Spacer(modifier = Modifier.height(24.dp)) + OutlinedTextField( + value = passphraseDisplay, + onValueChange = onPassphraseChange, + label = { Text("Passphrase") }, + modifier = Modifier.fillMaxWidth(), + visualTransformation = PasswordVisualTransformation(), + keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Password), + singleLine = true, + enabled = isInputEnabled + ) + + Spacer(modifier = Modifier.height(16.dp)) + + OutlinedTextField( + value = shareName, + onValueChange = onShareNameChange, + label = { Text("Share Name") }, + modifier = Modifier.fillMaxWidth(), + singleLine = true, + enabled = isInputEnabled + ) +} - if (importState is ImportState.Error) { - StatusCard( - text = importState.message, - containerColor = MaterialTheme.colorScheme.errorContainer, - contentColor = MaterialTheme.colorScheme.onErrorContainer - ) - Spacer(modifier = Modifier.height(16.dp)) - } +@Composable +private fun ImportStatusAndActions( + importState: ImportState, + canImport: Boolean, + onDismiss: () -> Unit, + onImportClick: (onError: (String) -> Unit) -> Unit +) { + if (importState is ImportState.Error) { + StatusCard( + text = importState.message, + containerColor = MaterialTheme.colorScheme.errorContainer, + contentColor = MaterialTheme.colorScheme.onErrorContainer + ) + Spacer(modifier = Modifier.height(16.dp)) + } - if (importState is ImportState.Success) { - StatusCard( - text = "Share '${importState.name}' imported successfully", - containerColor = MaterialTheme.colorScheme.primaryContainer, - contentColor = MaterialTheme.colorScheme.onPrimaryContainer - ) - Spacer(modifier = Modifier.height(16.dp)) - } + if (importState is ImportState.Success) { + StatusCard( + text = "Share '${importState.name}' imported successfully", + containerColor = MaterialTheme.colorScheme.primaryContainer, + contentColor = MaterialTheme.colorScheme.onPrimaryContainer + ) + Spacer(modifier = Modifier.height(16.dp)) + } - if (importState is ImportState.Importing) { - CircularProgressIndicator() - } else { - ImportButtons( - importState = importState, - canImport = shareData.isNotBlank() && passphrase.length > 0 && isInputEnabled, - onDismiss = onDismiss, - onImportClick = { onError -> - val passphraseChars = passphrase.toCharArray() - fun clearChars() = Arrays.fill(passphraseChars, '\u0000') - try { - val cipher = onGetCipher() - onBiometricAuth(cipher) { authedCipher -> - try { - if (authedCipher != null) { - onImport(shareData.valueUnsafe(), String(passphraseChars), shareName, authedCipher) - } - } finally { - clearChars() - } - } - } catch (e: KeyPermanentlyInvalidatedException) { - clearChars() - if (BuildConfig.DEBUG) Log.e("ImportShare", "Biometric key invalidated during cipher init", e) - onError("Biometric key invalidated. Please re-enroll biometrics.") - } catch (e: Exception) { - clearChars() - if (BuildConfig.DEBUG) Log.e("ImportShare", "Failed to initialize cipher for biometric auth", e) - onError("Failed to initialize encryption") - } - } - ) - } + if (importState is ImportState.Importing) { + CircularProgressIndicator() + } else { + ImportButtons( + importState = importState, + canImport = canImport, + onDismiss = onDismiss, + onImportClick = onImportClick + ) } } diff --git a/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt b/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt index f60e28c6..cfeeae0f 100644 --- a/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt +++ b/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt @@ -262,7 +262,7 @@ class KeepMobileApp : Application() { val proxy = proxyConfigStore?.getProxyConfig() if (BuildConfig.DEBUG) { val shareInfo = mobile.getShareInfo() - Log.d(TAG, "Share: index=${shareInfo?.shareIndex}, group=${shareInfo?.groupPubkey?.take(16)}") + Log.d(TAG, "Share: index=${shareInfo?.shareIndex}, hasGroup=${shareInfo?.groupPubkey != null}") Log.d(TAG, "Initializing with ${relays.size} relay(s), proxy=${proxy != null}") } initializeWithProxy(mobile, relays, proxy) diff --git a/app/src/main/kotlin/io/privkey/keep/MainActivity.kt b/app/src/main/kotlin/io/privkey/keep/MainActivity.kt index 3bcb7018..85eb2137 100644 --- a/app/src/main/kotlin/io/privkey/keep/MainActivity.kt +++ b/app/src/main/kotlin/io/privkey/keep/MainActivity.kt @@ -416,26 +416,15 @@ fun MainScreen( } if (showKillSwitchConfirmDialog) { - AlertDialog( - onDismissRequest = { showKillSwitchConfirmDialog = false }, - title = { Text("Enable Kill Switch?") }, - text = { Text("This will block all signing requests until you disable it. You will need biometric authentication to re-enable signing.") }, - confirmButton = { - TextButton(onClick = { - coroutineScope.launch { - withContext(Dispatchers.IO) { killSwitchStore.setEnabled(true) } - killSwitchEnabled = true - showKillSwitchConfirmDialog = false - } - }) { - Text("Enable") + KillSwitchConfirmDialog( + onConfirm = { + coroutineScope.launch { + withContext(Dispatchers.IO) { killSwitchStore.setEnabled(true) } + killSwitchEnabled = true + showKillSwitchConfirmDialog = false } }, - dismissButton = { - TextButton(onClick = { showKillSwitchConfirmDialog = false }) { - Text("Cancel") - } - } + onDismiss = { showKillSwitchConfirmDialog = false } ) } @@ -623,28 +612,16 @@ fun MainScreen( val pinMismatch = connectionState.pinMismatch if (pinMismatch != null) { - AlertDialog( - onDismissRequest = onDismissPinMismatch, - title = { Text("Certificate Pin Mismatch") }, - text = { - Text("The certificate for ${pinMismatch.hostname} has changed. This could indicate a security issue or a legitimate certificate rotation.") - }, - confirmButton = { - TextButton(onClick = { - coroutineScope.launch { - withContext(Dispatchers.IO) { onClearCertificatePin(pinMismatch.hostname) } - refreshCertificatePins() - onReconnectRelays() - } - }) { - Text("Clear Pin & Retry") + PinMismatchDialog( + hostname = pinMismatch.hostname, + onClearAndRetry = { + coroutineScope.launch { + withContext(Dispatchers.IO) { onClearCertificatePin(pinMismatch.hostname) } + refreshCertificatePins() + onReconnectRelays() } }, - dismissButton = { - TextButton(onClick = onDismissPinMismatch) { - Text("Dismiss") - } - } + onDismiss = onDismissPinMismatch ) } @@ -1030,30 +1007,21 @@ private fun SettingsTab( Spacer(modifier = Modifier.height(16.dp)) if (hasShare) { - RelaysCard( + ShareSettingsSection( relays = relays, + profileRelays = profileRelays, + certificatePins = certificatePins, + proxyEnabled = proxyEnabled, + proxyPort = proxyPort, onAddRelay = onAddRelay, onRemoveRelay = onRemoveRelay, - profileRelays = profileRelays, onAddProfileRelay = onAddProfileRelay, - onRemoveProfileRelay = onRemoveProfileRelay - ) - Spacer(modifier = Modifier.height(16.dp)) - - CertificatePinsCard( - pins = certificatePins, + onRemoveProfileRelay = onRemoveProfileRelay, onClearPin = onClearPin, - onClearAllPins = onClearAllPins - ) - Spacer(modifier = Modifier.height(16.dp)) - - TorOrbotCard( - enabled = proxyEnabled, - port = proxyPort, - onActivate = onProxyActivate, - onDeactivate = onProxyDeactivate + onClearAllPins = onClearAllPins, + onProxyActivate = onProxyActivate, + onProxyDeactivate = onProxyDeactivate ) - Spacer(modifier = Modifier.height(16.dp)) } AutoStartCard( @@ -1074,70 +1042,17 @@ private fun SettingsTab( HorizontalDivider() Spacer(modifier = Modifier.height(16.dp)) - Text( - "Database: $databaseSizeMb MB", - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant - ) - Spacer(modifier = Modifier.height(8.dp)) - - OutlinedButton( - onClick = { + DatabaseManagementSection( + databaseSizeMb = databaseSizeMb, + onCleanup = { scope.launch { onClearLogsAndActivity() refreshDatabaseSize() } - }, - modifier = Modifier.fillMaxWidth() - ) { - Text("Clean up expired data") - } - - Spacer(modifier = Modifier.height(16.dp)) - - Text( - "v${BuildConfig.VERSION_NAME}", - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant - ) - Spacer(modifier = Modifier.height(8.dp)) - - val primaryColor = MaterialTheme.colorScheme.primary - Text( - buildAnnotatedString { - withLink( - LinkAnnotation.Url( - "https://github.com/privkeyio/keep-android", - styles = TextLinkStyles( - style = SpanStyle( - color = primaryColor, - textDecoration = TextDecoration.Underline - ) - ) - ) - ) { - append("Source code") - } - append(" | ") - withLink( - LinkAnnotation.Url( - "https://privkey.io", - styles = TextLinkStyles( - style = SpanStyle( - color = primaryColor, - textDecoration = TextDecoration.Underline - ) - ) - ) - ) { - append("Support development") - } - }, - style = MaterialTheme.typography.bodySmall, - textAlign = TextAlign.Center + } ) - Spacer(modifier = Modifier.height(24.dp)) + SettingsFooterLinks() } } @@ -1258,6 +1173,156 @@ private data class PollResult( val pendingCount: Int ) +@Composable +private fun KillSwitchConfirmDialog( + onConfirm: () -> Unit, + onDismiss: () -> Unit +) { + AlertDialog( + onDismissRequest = onDismiss, + title = { Text("Enable Kill Switch?") }, + text = { Text("This will block all signing requests until you disable it. You will need biometric authentication to re-enable signing.") }, + confirmButton = { + TextButton(onClick = onConfirm) { Text("Enable") } + }, + dismissButton = { + TextButton(onClick = onDismiss) { Text("Cancel") } + } + ) +} + +@Composable +private fun PinMismatchDialog( + hostname: String, + onClearAndRetry: () -> Unit, + onDismiss: () -> Unit +) { + AlertDialog( + onDismissRequest = onDismiss, + title = { Text("Certificate Pin Mismatch") }, + text = { + Text("The certificate for $hostname has changed. This could indicate a security issue or a legitimate certificate rotation.") + }, + confirmButton = { + TextButton(onClick = onClearAndRetry) { Text("Clear Pin & Retry") } + }, + dismissButton = { + TextButton(onClick = onDismiss) { Text("Dismiss") } + } + ) +} + +@Composable +private fun ShareSettingsSection( + relays: List, + profileRelays: List, + certificatePins: List, + proxyEnabled: Boolean, + proxyPort: Int, + onAddRelay: (String) -> Unit, + onRemoveRelay: (String) -> Unit, + onAddProfileRelay: (String) -> Unit, + onRemoveProfileRelay: (String) -> Unit, + onClearPin: (String) -> Unit, + onClearAllPins: () -> Unit, + onProxyActivate: (Int) -> Unit, + onProxyDeactivate: () -> Unit +) { + RelaysCard( + relays = relays, + onAddRelay = onAddRelay, + onRemoveRelay = onRemoveRelay, + profileRelays = profileRelays, + onAddProfileRelay = onAddProfileRelay, + onRemoveProfileRelay = onRemoveProfileRelay + ) + Spacer(modifier = Modifier.height(16.dp)) + + CertificatePinsCard( + pins = certificatePins, + onClearPin = onClearPin, + onClearAllPins = onClearAllPins + ) + Spacer(modifier = Modifier.height(16.dp)) + + TorOrbotCard( + enabled = proxyEnabled, + port = proxyPort, + onActivate = onProxyActivate, + onDeactivate = onProxyDeactivate + ) + Spacer(modifier = Modifier.height(16.dp)) +} + +@Composable +private fun DatabaseManagementSection( + databaseSizeMb: String, + onCleanup: () -> Unit +) { + Text( + "Database: $databaseSizeMb MB", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + Spacer(modifier = Modifier.height(8.dp)) + + OutlinedButton( + onClick = onCleanup, + modifier = Modifier.fillMaxWidth() + ) { + Text("Clean up expired data") + } + + Spacer(modifier = Modifier.height(16.dp)) + + Text( + "v${BuildConfig.VERSION_NAME}", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + Spacer(modifier = Modifier.height(8.dp)) +} + +@Composable +private fun SettingsFooterLinks() { + val primaryColor = MaterialTheme.colorScheme.primary + Text( + buildAnnotatedString { + withLink( + LinkAnnotation.Url( + "https://github.com/privkeyio/keep-android", + styles = TextLinkStyles( + style = SpanStyle( + color = primaryColor, + textDecoration = TextDecoration.Underline + ) + ) + ) + ) { + append("Source code") + } + append(" | ") + withLink( + LinkAnnotation.Url( + "https://privkey.io", + styles = TextLinkStyles( + style = SpanStyle( + color = primaryColor, + textDecoration = TextDecoration.Underline + ) + ) + ) + ) { + append("Support development") + } + }, + style = MaterialTheme.typography.bodySmall, + textAlign = TextAlign.Center + ) + + Spacer(modifier = Modifier.height(24.dp)) +} + private fun getShareAwareCipher(storage: AndroidKeystoreStorage): Cipher? = runCatching { val key = storage.getActiveShareKey() diff --git a/app/src/main/kotlin/io/privkey/keep/nip46/BunkerScreen.kt b/app/src/main/kotlin/io/privkey/keep/nip46/BunkerScreen.kt index 45dda528..49dbd993 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip46/BunkerScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip46/BunkerScreen.kt @@ -94,8 +94,6 @@ fun BunkerScreen( var authorizedClients by remember { mutableStateOf(bunkerConfigStore.getAuthorizedClients()) } var showAddDialog by remember { mutableStateOf(false) } var showRevokeAllDialog by remember { mutableStateOf(false) } - var newRelayUrl by remember { mutableStateOf("") } - var error by remember { mutableStateOf(null) } val isEnabled = bunkerStatus == BunkerStatus.RUNNING || bunkerStatus == BunkerStatus.STARTING DisposableEffect(Unit) { @@ -105,12 +103,6 @@ fun BunkerScreen( } } - fun dismissDialog() { - showAddDialog = false - newRelayUrl = "" - error = null - } - Column( modifier = Modifier .fillMaxSize() @@ -203,113 +195,148 @@ fun BunkerScreen( } if (showAddDialog) { - AlertDialog( - onDismissRequest = ::dismissDialog, - title = { Text("Add Bunker Relay") }, - text = { - Column { - OutlinedTextField( - value = newRelayUrl, - onValueChange = { - newRelayUrl = it - error = null - }, - label = { Text("Relay URL") }, - placeholder = { Text("wss://relay.example.com or bunker://...") }, - singleLine = true, - isError = error != null + AddBunkerRelayDialog( + relays = relays, + onRelaysUpdated = { updated -> + relays = updated + scope.launch { bunkerConfigStore.setRelays(updated) } + }, + onDismiss = { showAddDialog = false } + ) + } + + if (showRevokeAllDialog) { + RevokeAllClientsDialog( + onConfirm = { + bunkerConfigStore.revokeAllClients() + authorizedClients = emptySet() + Toast.makeText(context, "All clients revoked", Toast.LENGTH_SHORT).show() + }, + onDismiss = { showRevokeAllDialog = false } + ) + } +} + +@Composable +private fun AddBunkerRelayDialog( + relays: List, + onRelaysUpdated: (List) -> Unit, + onDismiss: () -> Unit +) { + val scope = rememberCoroutineScope() + var newRelayUrl by remember { mutableStateOf("") } + var error by remember { mutableStateOf(null) } + + fun dismissDialog() { + newRelayUrl = "" + error = null + onDismiss() + } + + AlertDialog( + onDismissRequest = ::dismissDialog, + title = { Text("Add Bunker Relay") }, + text = { + Column { + OutlinedTextField( + value = newRelayUrl, + onValueChange = { + newRelayUrl = it + error = null + }, + label = { Text("Relay URL") }, + placeholder = { Text("wss://relay.example.com or bunker://...") }, + singleLine = true, + isError = error != null + ) + error?.let { + Text( + it, + color = MaterialTheme.colorScheme.error, + style = MaterialTheme.typography.bodySmall ) - error?.let { - Text( - it, - color = MaterialTheme.colorScheme.error, - style = MaterialTheme.typography.bodySmall + } + } + }, + confirmButton = { + TextButton(onClick = { + val trimmed = newRelayUrl.trim() + val bunkerRelays = parseBunkerUrlRelays(trimmed) + + when { + bunkerRelays != null -> scope.launch { + addBunkerRelays(bunkerRelays, relays).fold( + onSuccess = { toAdd -> + onRelaysUpdated(relays + toAdd) + dismissDialog() + }, + onFailure = { error = it.message } ) } - } - }, - confirmButton = { - TextButton(onClick = { - val trimmed = newRelayUrl.trim() - val bunkerRelays = parseBunkerUrlRelays(trimmed) - - when { - bunkerRelays != null -> scope.launch { - addBunkerRelays(bunkerRelays, relays).fold( - onSuccess = { toAdd -> - val updated = relays + toAdd - relays = updated - bunkerConfigStore.setRelays(updated) + trimmed.startsWith("bunker://") -> + error = "No relay URLs found in bunker URL" + else -> { + val url = normalizeRelayUrl(trimmed) + val validationError = validateRelayUrl(url, relays) + if (validationError != null) { + error = validationError + } else { + scope.launch { + val isInternal = withContext(Dispatchers.IO) { + BunkerConfigStore.isInternalHost(url) + } + if (isInternal) { + error = "Internal/private hosts are not allowed" + } else { + onRelaysUpdated(relays + url) dismissDialog() - }, - onFailure = { error = it.message } - ) - } - trimmed.startsWith("bunker://") -> - error = "No relay URLs found in bunker URL" - else -> { - val url = normalizeRelayUrl(trimmed) - val validationError = validateRelayUrl(url, relays) - if (validationError != null) { - error = validationError - } else { - scope.launch { - val isInternal = withContext(Dispatchers.IO) { - BunkerConfigStore.isInternalHost(url) - } - if (isInternal) { - error = "Internal/private hosts are not allowed" - } else { - val updated = relays + url - relays = updated - bunkerConfigStore.setRelays(updated) - dismissDialog() - } } } } } - }) { - Text("Add") - } - }, - dismissButton = { - TextButton(onClick = ::dismissDialog) { - Text("Cancel") } + }) { + Text("Add") } - ) - } + }, + dismissButton = { + TextButton(onClick = ::dismissDialog) { + Text("Cancel") + } + } + ) +} - if (showRevokeAllDialog) { - AlertDialog( - onDismissRequest = { showRevokeAllDialog = false }, - title = { Text("Revoke All Clients") }, - text = { - Text("This will disconnect all authorized clients. They will need to reconnect and be approved again.") - }, - confirmButton = { - TextButton( - onClick = { - bunkerConfigStore.revokeAllClients() - authorizedClients = emptySet() - showRevokeAllDialog = false - Toast.makeText(context, "All clients revoked", Toast.LENGTH_SHORT).show() - }, - colors = ButtonDefaults.textButtonColors( - contentColor = MaterialTheme.colorScheme.error - ) - ) { - Text("Revoke All") - } - }, - dismissButton = { - TextButton(onClick = { showRevokeAllDialog = false }) { - Text("Cancel") - } +@Composable +private fun RevokeAllClientsDialog( + onConfirm: () -> Unit, + onDismiss: () -> Unit +) { + AlertDialog( + onDismissRequest = onDismiss, + title = { Text("Revoke All Clients") }, + text = { + Text("This will disconnect all authorized clients. They will need to reconnect and be approved again.") + }, + confirmButton = { + TextButton( + onClick = { + onConfirm() + onDismiss() + }, + colors = ButtonDefaults.textButtonColors( + contentColor = MaterialTheme.colorScheme.error + ) + ) { + Text("Revoke All") } - ) - } + }, + dismissButton = { + TextButton(onClick = onDismiss) { + Text("Cancel") + } + } + ) } @Composable diff --git a/app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt b/app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt index 0a8c4c96..1568db19 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt @@ -38,6 +38,7 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeoutOrNull import java.util.UUID import java.util.concurrent.ArrayBlockingQueue import java.util.concurrent.ConcurrentHashMap @@ -455,6 +456,7 @@ class BunkerService : Service() { } private fun checkStoredPermission(clientPubkey: String, request: BunkerApprovalRequest): PermissionDecision? { + require(clientPubkey.isNotBlank()) { "Client pubkey must not be blank" } val store = permissionStore ?: return null val callerPackage = "nip46:$clientPubkey" val requestType = mapMethodToRequestType(request.method) ?: return null @@ -462,7 +464,9 @@ class BunkerService : Service() { return runCatching { runBlocking(Dispatchers.IO) { - store.getPermissionDecision(callerPackage, requestType, eventKind) + withTimeoutOrNull(5_000L) { + store.getPermissionDecision(callerPackage, requestType, eventKind) + } } }.getOrNull() } diff --git a/app/src/main/kotlin/io/privkey/keep/nip46/Nip46ApprovalActivity.kt b/app/src/main/kotlin/io/privkey/keep/nip46/Nip46ApprovalActivity.kt index c4b9889d..70b9a9b8 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip46/Nip46ApprovalActivity.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip46/Nip46ApprovalActivity.kt @@ -180,8 +180,8 @@ class Nip46ApprovalActivity : FragmentActivity() { eventKind = eventKind, duration = duration ) - } catch (t: Throwable) { - if (BuildConfig.DEBUG) Log.e(TAG, "Failed to persist permission: ${t::class.simpleName}") + } catch (e: Exception) { + if (BuildConfig.DEBUG) Log.e(TAG, "Failed to persist permission: ${e::class.simpleName}") } } } diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt b/app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt index 69daa14b..843abf72 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt @@ -27,6 +27,7 @@ import io.privkey.keep.R import io.privkey.keep.nip46.Nip46ClientStore import io.privkey.keep.storage.BunkerConfigStore import io.privkey.keep.storage.SignPolicyStore +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import kotlinx.coroutines.withContext @@ -100,42 +101,13 @@ fun AppPermissionsScreen( } if (showRevokeAllDialog) { - val isNip46 = packageName.startsWith("nip46:") - val dialogTitle = if (isNip46) "Disconnect Client?" else "Disconnect App?" - val dialogText = if (isNip46) { - "This will remove all saved permissions and revoke authorization for ${appState.label ?: packageName}. The client will need to reconnect." - } else { - "This will remove all saved permissions for ${appState.label ?: packageName}. The app will need to request permission again." - } - AlertDialog( - onDismissRequest = { showRevokeAllDialog = false }, - title = { Text(dialogTitle) }, - text = { Text(dialogText) }, - confirmButton = { - TextButton( - onClick = { - coroutineScope.launch { - withContext(Dispatchers.IO) { - runCatching { permissionStore.revokePermission(packageName) } - if (isNip46) { - val pubkey = packageName.removePrefix("nip46:") - runCatching { revokeNip46Client(context, pubkey) } - } - } - showRevokeAllDialog = false - onDismiss() - } - }, - colors = ButtonDefaults.textButtonColors(contentColor = MaterialTheme.colorScheme.error) - ) { - Text("Disconnect") - } - }, - dismissButton = { - TextButton(onClick = { showRevokeAllDialog = false }) { - Text("Cancel") - } - } + RevokeAllPermissionsDialog( + packageName = packageName, + appLabel = appState.label, + permissionStore = permissionStore, + coroutineScope = coroutineScope, + onDismissDialog = { showRevokeAllDialog = false }, + onDismissScreen = onDismiss ) } @@ -151,149 +123,235 @@ fun AppPermissionsScreen( ) } ) { padding -> - LazyColumn( - modifier = Modifier.fillMaxSize().padding(padding), - contentPadding = PaddingValues(16.dp), - verticalArrangement = Arrangement.spacedBy(16.dp) - ) { - item { - AppHeaderCard( - packageName = packageName, - appLabel = appState.label, - appIcon = appState.icon, - isVerified = appState.isVerified, - isNip46Client = appState.isNip46Client - ) + AppPermissionsListContent( + padding = padding, + packageName = packageName, + appState = appState, + appSettings = appSettings, + expiryDropdownExpanded = expiryDropdownExpanded, + onExpiryDropdownExpandedChange = { expiryDropdownExpanded = it }, + signPolicyStore = signPolicyStore, + permissionStore = permissionStore, + coroutineScope = coroutineScope, + onAppStateChange = { appState = it }, + onAppSettingsChange = { appSettings = it }, + onShowRevokeAllDialog = { showRevokeAllDialog = true }, + onDismiss = onDismiss + ) + } +} + +@Composable +private fun RevokeAllPermissionsDialog( + packageName: String, + appLabel: String?, + permissionStore: PermissionStore, + coroutineScope: CoroutineScope, + onDismissDialog: () -> Unit, + onDismissScreen: () -> Unit +) { + val context = LocalContext.current + val isNip46 = packageName.startsWith("nip46:") + val dialogTitle = if (isNip46) "Disconnect Client?" else "Disconnect App?" + val dialogText = if (isNip46) { + "This will remove all saved permissions and revoke authorization for ${appLabel ?: packageName}. The client will need to reconnect." + } else { + "This will remove all saved permissions for ${appLabel ?: packageName}. The app will need to request permission again." + } + AlertDialog( + onDismissRequest = onDismissDialog, + title = { Text(dialogTitle) }, + text = { Text(dialogText) }, + confirmButton = { + TextButton( + onClick = { + coroutineScope.launch { + withContext(Dispatchers.IO) { + runCatching { permissionStore.revokePermission(packageName) } + if (isNip46) { + val pubkey = packageName.removePrefix("nip46:") + runCatching { revokeNip46Client(context, pubkey) } + } + } + onDismissDialog() + onDismissScreen() + } + }, + colors = ButtonDefaults.textButtonColors(contentColor = MaterialTheme.colorScheme.error) + ) { + Text("Disconnect") + } + }, + dismissButton = { + TextButton(onClick = onDismissDialog) { + Text("Cancel") } + } + ) +} - item { - AppExpirySelector( - currentExpiry = appSettings?.expiresAt, - expanded = expiryDropdownExpanded, - onExpandedChange = { expiryDropdownExpanded = it }, - onDurationSelected = { duration -> - coroutineScope.launch { +@Composable +private fun AppPermissionsListContent( + padding: PaddingValues, + packageName: String, + appState: AppState, + appSettings: Nip55AppSettings?, + expiryDropdownExpanded: Boolean, + onExpiryDropdownExpandedChange: (Boolean) -> Unit, + signPolicyStore: SignPolicyStore?, + permissionStore: PermissionStore, + coroutineScope: CoroutineScope, + onAppStateChange: (AppState) -> Unit, + onAppSettingsChange: (Nip55AppSettings?) -> Unit, + onShowRevokeAllDialog: () -> Unit, + onDismiss: () -> Unit +) { + val context = LocalContext.current + + LazyColumn( + modifier = Modifier.fillMaxSize().padding(padding), + contentPadding = PaddingValues(16.dp), + verticalArrangement = Arrangement.spacedBy(16.dp) + ) { + item { + AppHeaderCard( + packageName = packageName, + appLabel = appState.label, + appIcon = appState.icon, + isVerified = appState.isVerified, + isNip46Client = appState.isNip46Client + ) + } + + item { + AppExpirySelector( + currentExpiry = appSettings?.expiresAt, + expanded = expiryDropdownExpanded, + onExpandedChange = onExpiryDropdownExpandedChange, + onDurationSelected = { duration -> + coroutineScope.launch { + withContext(Dispatchers.IO) { + permissionStore.setAppExpiry(packageName, duration) + } + onAppSettingsChange( withContext(Dispatchers.IO) { - permissionStore.setAppExpiry(packageName, duration) - } - appSettings = withContext(Dispatchers.IO) { permissionStore.getAppSettings(packageName) } - } + ) } - ) - } + } + ) + } - if (signPolicyStore != null && !appState.isLoading) { - item { - Card(modifier = Modifier.fillMaxWidth()) { - Column(modifier = Modifier.padding(16.dp)) { - AppSignPolicySelector( - currentOverride = appState.signPolicyOverride, - globalPolicy = signPolicyStore.getGlobalPolicy(), - onOverrideChange = { newOverride -> - coroutineScope.launch { - try { - withContext(Dispatchers.IO) { - permissionStore.setAppSignPolicyOverride(packageName, newOverride) - } - appState = appState.copy(signPolicyOverride = newOverride) - } catch (e: Exception) { - if (BuildConfig.DEBUG) Log.e("AppPermissions", "Failed to update sign policy", e) - Toast.makeText(context, "Failed to update sign policy", Toast.LENGTH_SHORT).show() + if (signPolicyStore != null && !appState.isLoading) { + item { + Card(modifier = Modifier.fillMaxWidth()) { + Column(modifier = Modifier.padding(16.dp)) { + AppSignPolicySelector( + currentOverride = appState.signPolicyOverride, + globalPolicy = signPolicyStore.getGlobalPolicy(), + onOverrideChange = { newOverride -> + coroutineScope.launch { + try { + withContext(Dispatchers.IO) { + permissionStore.setAppSignPolicyOverride(packageName, newOverride) } + onAppStateChange(appState.copy(signPolicyOverride = newOverride)) + } catch (e: Exception) { + if (BuildConfig.DEBUG) Log.e("AppPermissions", "Failed to update sign policy", e) + Toast.makeText(context, "Failed to update sign policy", Toast.LENGTH_SHORT).show() } } - ) - } + } + ) } } } + } - if (appState.isLoading) { - item { - Box( - modifier = Modifier.fillMaxWidth().padding(32.dp), - contentAlignment = Alignment.Center - ) { - CircularProgressIndicator() - } - } - } else if (appState.permissions.isEmpty()) { - item { - Text( - "No active permissions", - color = MaterialTheme.colorScheme.onSurfaceVariant - ) - } - } else { - item { - Text( - "Permissions", - style = MaterialTheme.typography.titleMedium - ) + if (appState.isLoading) { + item { + Box( + modifier = Modifier.fillMaxWidth().padding(32.dp), + contentAlignment = Alignment.Center + ) { + CircularProgressIndicator() } - items(appState.permissions, key = { it.id }) { permission -> - var updateError by remember { mutableStateOf(null) } - val requestType = findRequestType(permission.requestType) - - PermissionItem( - permission = permission, - onDecisionChange = { newDecision -> - if (requestType == null) return@PermissionItem - coroutineScope.launch { - try { - withContext(Dispatchers.IO) { - permissionStore.updatePermissionDecision( - permission.id, - newDecision, - packageName, - requestType, - permission.eventKind - ) - } - val newPermissions = withContext(Dispatchers.IO) { - permissionStore.getPermissionsForCaller(packageName) - } - appState = appState.copy(permissions = newPermissions) - updateError = null - } catch (e: Exception) { - if (BuildConfig.DEBUG) Log.e("AppPermissions", "Failed to update permission", e) - updateError = "Failed to update permission" + } + } else if (appState.permissions.isEmpty()) { + item { + Text( + "No active permissions", + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + } + } else { + item { + Text( + "Permissions", + style = MaterialTheme.typography.titleMedium + ) + } + items(appState.permissions, key = { it.id }) { permission -> + var updateError by remember { mutableStateOf(null) } + val requestType = findRequestType(permission.requestType) + + PermissionItem( + permission = permission, + onDecisionChange = { newDecision -> + if (requestType == null) return@PermissionItem + coroutineScope.launch { + try { + withContext(Dispatchers.IO) { + permissionStore.updatePermissionDecision( + permission.id, + newDecision, + packageName, + requestType, + permission.eventKind + ) + } + val newPermissions = withContext(Dispatchers.IO) { + permissionStore.getPermissionsForCaller(packageName) } + onAppStateChange(appState.copy(permissions = newPermissions)) + updateError = null + } catch (e: Exception) { + if (BuildConfig.DEBUG) Log.e("AppPermissions", "Failed to update permission", e) + updateError = "Failed to update permission" } - }, - errorMessage = updateError, - onRevoke = { - coroutineScope.launch { - try { - withContext(Dispatchers.IO) { - permissionStore.deletePermission(permission.id) - } - val newPermissions = withContext(Dispatchers.IO) { - permissionStore.getPermissionsForCaller(packageName) - } - appState = appState.copy(permissions = newPermissions) - if (newPermissions.isEmpty()) onDismiss() - } catch (e: Exception) { - if (BuildConfig.DEBUG) Log.e("AppPermissions", "Failed to revoke permission", e) - Toast.makeText(context, "Failed to revoke permission", Toast.LENGTH_SHORT).show() + } + }, + errorMessage = updateError, + onRevoke = { + coroutineScope.launch { + try { + withContext(Dispatchers.IO) { + permissionStore.deletePermission(permission.id) } + val newPermissions = withContext(Dispatchers.IO) { + permissionStore.getPermissionsForCaller(packageName) + } + onAppStateChange(appState.copy(permissions = newPermissions)) + if (newPermissions.isEmpty()) onDismiss() + } catch (e: Exception) { + if (BuildConfig.DEBUG) Log.e("AppPermissions", "Failed to revoke permission", e) + Toast.makeText(context, "Failed to revoke permission", Toast.LENGTH_SHORT).show() } } - ) - } + } + ) } + } - item { - Spacer(modifier = Modifier.height(8.dp)) - Button( - onClick = { showRevokeAllDialog = true }, - modifier = Modifier.fillMaxWidth(), - colors = ButtonDefaults.buttonColors(containerColor = MaterialTheme.colorScheme.error) - ) { - Text(if (packageName.startsWith("nip46:")) "Disconnect Client" else "Disconnect App") - } + item { + Spacer(modifier = Modifier.height(8.dp)) + Button( + onClick = onShowRevokeAllDialog, + modifier = Modifier.fillMaxWidth(), + colors = ButtonDefaults.buttonColors(containerColor = MaterialTheme.colorScheme.error) + ) { + Text(if (packageName.startsWith("nip46:")) "Disconnect Client" else "Disconnect App") } } } diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/ConnectedAppsScreen.kt b/app/src/main/kotlin/io/privkey/keep/nip55/ConnectedAppsScreen.kt index 4ae1c2fa..1017bcbd 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/ConnectedAppsScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/ConnectedAppsScreen.kt @@ -95,6 +95,7 @@ fun ConnectedAppsScreen( CircularProgressIndicator() } } else if (loadError != null) { + val currentLoadError = loadError Box( modifier = Modifier.fillMaxSize().padding(padding), contentAlignment = Alignment.Center @@ -114,7 +115,7 @@ fun ConnectedAppsScreen( ) Spacer(modifier = Modifier.height(8.dp)) Text( - loadError!!, + currentLoadError ?: "", style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant ) diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt b/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt index 8b1d5054..b6e75089 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt @@ -140,6 +140,25 @@ class Nip55ContentProvider : ContentProvider() { if (store == null) return null + val velocityCursor = checkVelocityLimits(store, callerPackage, requestType, eventKind) + if (velocityCursor != null) return velocityCursor + + val policyCursor = evaluateAutoSignPolicy( + currentApp, store, h, callerPackage, requestType, rawContent, rawPubkey, eventKind, currentUser + ) + if (policyCursor != null) return policyCursor.cursorOrNull + + return checkPermissionWithRisk( + store, h, currentApp, callerPackage, requestType, rawContent, rawPubkey, eventKind, currentUser + ) + } + + private fun checkVelocityLimits( + store: PermissionStore, + callerPackage: String, + requestType: Nip55RequestType, + eventKind: Int? + ): Cursor? { val velocityResult = runWithTimeout { store.checkAndRecordVelocity(callerPackage, eventKind) } if (velocityResult == null) { if (BuildConfig.DEBUG) Log.w(TAG, "Velocity check failed (timeout or concurrency limit), denying request") @@ -151,7 +170,29 @@ class Nip55ContentProvider : ContentProvider() { runWithTimeout { store.logOperation(callerPackage, requestType, eventKind, "velocity_blocked", wasAutomatic = true) } return rejectedCursor(null) } + return null + } + + private sealed class PolicyResult { + val cursorOrNull: Cursor? get() = when (this) { + is Decided -> cursor + is FallToUi -> null + } + class Decided(val cursor: Cursor?) : PolicyResult() + class FallToUi : PolicyResult() + } + private fun evaluateAutoSignPolicy( + currentApp: KeepMobileApp, + store: PermissionStore, + h: Nip55Handler, + callerPackage: String, + requestType: Nip55RequestType, + rawContent: String, + rawPubkey: String?, + eventKind: Int?, + currentUser: String? + ): PolicyResult? { val effectivePolicy = runWithTimeout { store.getAppSignPolicyOverride(callerPackage) ?.let { SignPolicy.fromOrdinal(it) } @@ -159,29 +200,29 @@ class Nip55ContentProvider : ContentProvider() { ?: SignPolicy.MANUAL } ?: SignPolicy.MANUAL - if (effectivePolicy == SignPolicy.MANUAL) return null + if (effectivePolicy == SignPolicy.MANUAL) return PolicyResult.FallToUi() if (effectivePolicy == SignPolicy.AUTO) { if (eventKind != null && isSensitiveKind(eventKind)) { if (BuildConfig.DEBUG) Log.d(TAG, "AUTO mode blocked for sensitive event kind $eventKind from ${hashPackageName(callerPackage)}") - return null + return PolicyResult.FallToUi() } val safeguards = currentApp.getAutoSigningSafeguards() if (safeguards == null) { if (BuildConfig.DEBUG) Log.w(TAG, "AUTO signing denied: AutoSigningSafeguards unavailable for ${hashPackageName(callerPackage)}") runWithTimeout { store.logOperation(callerPackage, requestType, eventKind, "deny_safeguards_unavailable", wasAutomatic = true) } - return rejectedCursor(null) + return PolicyResult.Decided(rejectedCursor(null)) } if (!safeguards.isOptedIn(callerPackage)) { if (BuildConfig.DEBUG) Log.d(TAG, "AUTO signing not opted-in for ${hashPackageName(callerPackage)}") - return null + return PolicyResult.FallToUi() } val denyReason = when (val usageResult = safeguards.checkAndRecordUsage(callerPackage)) { is AutoSigningSafeguards.UsageCheckResult.Allowed -> - return executeBackgroundRequest(h, store, currentApp, callerPackage, requestType, rawContent, rawPubkey, null, eventKind, currentUser) + return PolicyResult.Decided(executeBackgroundRequest(h, store, currentApp, callerPackage, requestType, rawContent, rawPubkey, null, eventKind, currentUser)) is AutoSigningSafeguards.UsageCheckResult.HourlyLimitExceeded -> "deny_hourly_limit" is AutoSigningSafeguards.UsageCheckResult.DailyLimitExceeded -> "deny_daily_limit" is AutoSigningSafeguards.UsageCheckResult.UnusualActivity -> "deny_unusual_activity" @@ -189,9 +230,23 @@ class Nip55ContentProvider : ContentProvider() { } if (BuildConfig.DEBUG) Log.w(TAG, "Auto-signing denied for ${hashPackageName(callerPackage)}: $denyReason") runWithTimeout { store.logOperation(callerPackage, requestType, eventKind, denyReason, wasAutomatic = true) } - return rejectedCursor(null) + return PolicyResult.Decided(rejectedCursor(null)) } + return null + } + + private fun checkPermissionWithRisk( + store: PermissionStore, + h: Nip55Handler, + currentApp: KeepMobileApp, + callerPackage: String, + requestType: Nip55RequestType, + rawContent: String, + rawPubkey: String?, + eventKind: Int?, + currentUser: String? + ): Cursor? { val isAppExpired = runWithTimeout { store.isAppExpired(callerPackage) } if (isAppExpired == null) { if (BuildConfig.DEBUG) Log.w(TAG, "isAppExpired check timed out for ${hashPackageName(callerPackage)}, denying request") diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/PermissionsManagementScreen.kt b/app/src/main/kotlin/io/privkey/keep/nip55/PermissionsManagementScreen.kt index bde8ee29..251aa45e 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/PermissionsManagementScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/PermissionsManagementScreen.kt @@ -100,54 +100,30 @@ fun PermissionsManagementScreen( ) } } else { - val groupedPermissions = permissions.groupBy { it.callerPackage } - - LazyColumn( + PermissionsGroupedList( + permissions = permissions, + velocityUsage = velocityUsage, modifier = Modifier.weight(1f), - verticalArrangement = Arrangement.spacedBy(12.dp) - ) { - groupedPermissions.forEach { (packageName, appPermissions) -> - item(key = "header_$packageName") { - AppPermissionHeader( - packageName = packageName, - permissionCount = appPermissions.size, - velocityUsage = velocityUsage[packageName], - onRevokeAll = { showRevokeAllDialog = packageName } - ) - } - items( - items = appPermissions, - key = { it.id } - ) { permission -> - val requestType = findRequestType(permission.requestType) - - PermissionCard( - permission = permission, - onDecisionChange = { newDecision -> - if (requestType == null) return@PermissionCard - coroutineScope.launch { - try { - permissionStore.updatePermissionDecision( - permission.id, - newDecision, - permission.callerPackage, - requestType, - permission.eventKind - ) - refreshPermissions() - } catch (e: Exception) { - loadError = "Failed to update permission" - } - } - }, - onDelete = { showDeleteDialog = permission } - ) - } - item(key = "spacer_$packageName") { - Spacer(modifier = Modifier.height(8.dp)) + onRevokeAll = { showRevokeAllDialog = it }, + onDecisionChange = { permission, newDecision -> + val requestType = findRequestType(permission.requestType) ?: return@PermissionsGroupedList + coroutineScope.launch { + try { + permissionStore.updatePermissionDecision( + permission.id, + newDecision, + permission.callerPackage, + requestType, + permission.eventKind + ) + refreshPermissions() + } catch (e: Exception) { + loadError = "Failed to update permission" + } } - } - } + }, + onDelete = { showDeleteDialog = it } + ) } Spacer(modifier = Modifier.height(16.dp)) @@ -162,68 +138,133 @@ fun PermissionsManagementScreen( } showRevokeAllDialog?.let { packageName -> - AlertDialog( - onDismissRequest = { showRevokeAllDialog = null }, - title = { Text("Revoke All Permissions") }, - text = { Text("Revoke all permissions for $packageName? This app will need to request permissions again.") }, - confirmButton = { - TextButton( - onClick = { - coroutineScope.launch { - try { - permissionStore.revokeAllForApp(packageName) - refreshPermissions() - } catch (e: Exception) { - loadError = "Failed to revoke permissions" - } - } - showRevokeAllDialog = null + RevokeAllPermissionsDialog( + packageName = packageName, + onConfirm = { + coroutineScope.launch { + try { + permissionStore.revokeAllForApp(packageName) + refreshPermissions() + } catch (e: Exception) { + loadError = "Failed to revoke permissions" } - ) { - Text("Revoke All", color = MaterialTheme.colorScheme.error) } + showRevokeAllDialog = null }, - dismissButton = { - TextButton(onClick = { showRevokeAllDialog = null }) { - Text("Cancel") - } - } + onDismiss = { showRevokeAllDialog = null } ) } showDeleteDialog?.let { permission -> - AlertDialog( - onDismissRequest = { showDeleteDialog = null }, - title = { Text("Delete Permission") }, - text = { - Text("Delete this ${formatRequestType(permission.requestType)} permission for ${permission.callerPackage}?") - }, - confirmButton = { - TextButton( - onClick = { - coroutineScope.launch { - try { - permissionStore.deletePermission(permission.id) - refreshPermissions() - } catch (e: Exception) { - loadError = "Failed to delete permission" - } - } - showDeleteDialog = null + DeletePermissionDialog( + permission = permission, + onConfirm = { + coroutineScope.launch { + try { + permissionStore.deletePermission(permission.id) + refreshPermissions() + } catch (e: Exception) { + loadError = "Failed to delete permission" } - ) { - Text("Delete", color = MaterialTheme.colorScheme.error) } + showDeleteDialog = null }, - dismissButton = { - TextButton(onClick = { showDeleteDialog = null }) { - Text("Cancel") - } - } + onDismiss = { showDeleteDialog = null } ) } } +@Composable +private fun RevokeAllPermissionsDialog( + packageName: String, + onConfirm: () -> Unit, + onDismiss: () -> Unit +) { + AlertDialog( + onDismissRequest = onDismiss, + title = { Text("Revoke All Permissions") }, + text = { Text("Revoke all permissions for $packageName? This app will need to request permissions again.") }, + confirmButton = { + TextButton(onClick = onConfirm) { + Text("Revoke All", color = MaterialTheme.colorScheme.error) + } + }, + dismissButton = { + TextButton(onClick = onDismiss) { + Text("Cancel") + } + } + ) +} + +@Composable +private fun DeletePermissionDialog( + permission: Nip55Permission, + onConfirm: () -> Unit, + onDismiss: () -> Unit +) { + AlertDialog( + onDismissRequest = onDismiss, + title = { Text("Delete Permission") }, + text = { + Text("Delete this ${formatRequestType(permission.requestType)} permission for ${permission.callerPackage}?") + }, + confirmButton = { + TextButton(onClick = onConfirm) { + Text("Delete", color = MaterialTheme.colorScheme.error) + } + }, + dismissButton = { + TextButton(onClick = onDismiss) { + Text("Cancel") + } + } + ) +} + +@Composable +private fun PermissionsGroupedList( + permissions: List, + velocityUsage: Map>, + modifier: Modifier = Modifier, + onRevokeAll: (String) -> Unit, + onDecisionChange: (Nip55Permission, PermissionDecision) -> Unit, + onDelete: (Nip55Permission) -> Unit +) { + val groupedPermissions = permissions.groupBy { it.callerPackage } + + LazyColumn( + modifier = modifier, + verticalArrangement = Arrangement.spacedBy(12.dp) + ) { + groupedPermissions.forEach { (packageName, appPermissions) -> + item(key = "header_$packageName") { + AppPermissionHeader( + packageName = packageName, + permissionCount = appPermissions.size, + velocityUsage = velocityUsage[packageName], + onRevokeAll = { onRevokeAll(packageName) } + ) + } + items( + items = appPermissions, + key = { it.id } + ) { permission -> + PermissionCard( + permission = permission, + onDecisionChange = { newDecision -> + onDecisionChange(permission, newDecision) + }, + onDelete = { onDelete(permission) } + ) + } + item(key = "spacer_$packageName") { + Spacer(modifier = Modifier.height(8.dp)) + } + } + } +} + @Composable private fun AppPermissionHeader( packageName: String, diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/SigningHistoryScreen.kt b/app/src/main/kotlin/io/privkey/keep/nip55/SigningHistoryScreen.kt index 05f8b3fe..6737a989 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/SigningHistoryScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/SigningHistoryScreen.kt @@ -2,6 +2,7 @@ package io.privkey.keep.nip55 import androidx.compose.foundation.layout.* import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.material.icons.Icons @@ -118,90 +119,27 @@ fun SigningHistoryScreen( Spacer(modifier = Modifier.height(12.dp)) if (availableApps.isNotEmpty()) { - ExposedDropdownMenuBox( + AppFilterDropdown( + availableApps = availableApps, + selectedApp = selectedApp, expanded = appFilterExpanded, - onExpandedChange = { appFilterExpanded = it } - ) { - OutlinedTextField( - value = selectedApp ?: "All apps", - onValueChange = {}, - readOnly = true, - label = { Text("Filter by app") }, - trailingIcon = { - Icon( - Icons.Default.KeyboardArrowDown, - contentDescription = null - ) - }, - modifier = Modifier - .fillMaxWidth() - .menuAnchor(ExposedDropdownMenuAnchorType.PrimaryNotEditable) - ) - ExposedDropdownMenu( - expanded = appFilterExpanded, - onDismissRequest = { appFilterExpanded = false } - ) { - DropdownMenuItem( - text = { Text("All apps") }, - onClick = { - selectedApp = null - appFilterExpanded = false - } - ) - availableApps.forEach { app -> - DropdownMenuItem( - text = { Text(app, maxLines = 1, overflow = TextOverflow.Ellipsis) }, - onClick = { - selectedApp = app - appFilterExpanded = false - } - ) - } + onExpandedChange = { appFilterExpanded = it }, + onAppSelected = { app -> + selectedApp = app + appFilterExpanded = false } - } + ) Spacer(modifier = Modifier.height(16.dp)) } - if (isLoading) { - Box(modifier = Modifier.weight(1f), contentAlignment = Alignment.Center) { - CircularProgressIndicator() - } - } else if (logs.isEmpty()) { - Box(modifier = Modifier.weight(1f), contentAlignment = Alignment.Center) { - Text( - text = "No signing history", - style = MaterialTheme.typography.bodyLarge, - color = MaterialTheme.colorScheme.onSurfaceVariant - ) - } - } else { - LazyColumn( - modifier = Modifier.weight(1f), - state = listState, - verticalArrangement = Arrangement.spacedBy(8.dp) - ) { - items( - items = logs, - key = { it.id } - ) { log -> - AuditLogCard(log) - } - - if (isLoadingMore) { - item { - Box( - modifier = Modifier - .fillMaxWidth() - .padding(16.dp), - contentAlignment = Alignment.Center - ) { - CircularProgressIndicator(modifier = Modifier.size(24.dp)) - } - } - } - } - } + SigningHistoryLogsList( + logs = logs, + isLoading = isLoading, + isLoadingMore = isLoadingMore, + listState = listState, + modifier = Modifier.weight(1f) + ) Spacer(modifier = Modifier.height(16.dp)) @@ -214,6 +152,101 @@ fun SigningHistoryScreen( } } +@OptIn(ExperimentalMaterial3Api::class) +@Composable +private fun AppFilterDropdown( + availableApps: List, + selectedApp: String?, + expanded: Boolean, + onExpandedChange: (Boolean) -> Unit, + onAppSelected: (String?) -> Unit +) { + ExposedDropdownMenuBox( + expanded = expanded, + onExpandedChange = onExpandedChange + ) { + OutlinedTextField( + value = selectedApp ?: "All apps", + onValueChange = {}, + readOnly = true, + label = { Text("Filter by app") }, + trailingIcon = { + Icon( + Icons.Default.KeyboardArrowDown, + contentDescription = null + ) + }, + modifier = Modifier + .fillMaxWidth() + .menuAnchor(ExposedDropdownMenuAnchorType.PrimaryNotEditable) + ) + ExposedDropdownMenu( + expanded = expanded, + onDismissRequest = { onExpandedChange(false) } + ) { + DropdownMenuItem( + text = { Text("All apps") }, + onClick = { onAppSelected(null) } + ) + availableApps.forEach { app -> + DropdownMenuItem( + text = { Text(app, maxLines = 1, overflow = TextOverflow.Ellipsis) }, + onClick = { onAppSelected(app) } + ) + } + } + } +} + +@Composable +private fun SigningHistoryLogsList( + logs: List, + isLoading: Boolean, + isLoadingMore: Boolean, + listState: LazyListState, + modifier: Modifier = Modifier +) { + if (isLoading) { + Box(modifier = modifier, contentAlignment = Alignment.Center) { + CircularProgressIndicator() + } + } else if (logs.isEmpty()) { + Box(modifier = modifier, contentAlignment = Alignment.Center) { + Text( + text = "No signing history", + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + } + } else { + LazyColumn( + modifier = modifier, + state = listState, + verticalArrangement = Arrangement.spacedBy(8.dp) + ) { + items( + items = logs, + key = { it.id } + ) { log -> + AuditLogCard(log) + } + + if (isLoadingMore) { + item { + Box( + modifier = Modifier + .fillMaxWidth() + .padding(16.dp), + contentAlignment = Alignment.Center + ) { + CircularProgressIndicator(modifier = Modifier.size(24.dp)) + } + } + } + } + } +} + @Composable private fun AuditLogCard(log: Nip55AuditLog) { val dateFormat = remember { SimpleDateFormat("MMM d, yyyy HH:mm:ss", Locale.getDefault()) } diff --git a/app/src/main/kotlin/io/privkey/keep/storage/AndroidKeystoreStorage.kt b/app/src/main/kotlin/io/privkey/keep/storage/AndroidKeystoreStorage.kt index 372571b5..eedf87a8 100644 --- a/app/src/main/kotlin/io/privkey/keep/storage/AndroidKeystoreStorage.kt +++ b/app/src/main/kotlin/io/privkey/keep/storage/AndroidKeystoreStorage.kt @@ -425,6 +425,8 @@ class AndroidKeystoreStorage(private val context: Context) : SecureStorage { } override fun storeShareByKey(key: String, data: ByteArray, metadata: ShareMetadataInfo) { + require(key.isNotBlank()) { "Share key must not be blank" } + require(data.isNotEmpty()) { "Share data must not be empty" } if (isMetadataKey(key)) { storeMetadata(key, data, metadata) return @@ -437,6 +439,7 @@ class AndroidKeystoreStorage(private val context: Context) : SecureStorage { } override fun loadShareByKey(key: String): ByteArray { + require(key.isNotBlank()) { "Share key must not be blank" } if (isMetadataKey(key)) { return loadMetadata(key) } @@ -459,6 +462,7 @@ class AndroidKeystoreStorage(private val context: Context) : SecureStorage { } override fun deleteShareByKey(key: String) { + require(key.isNotBlank()) { "Share key must not be blank" } if (isMetadataKey(key)) { val cleared = getSharePrefs(key).edit().clear().commit() if (!cleared) { @@ -504,6 +508,7 @@ class AndroidKeystoreStorage(private val context: Context) : SecureStorage { } override fun setActiveShareKey(key: String?) { + if (key != null) require(key.isNotBlank()) { "Active share key must not be blank" } val editor = multiSharePrefs.edit() if (key != null) { editor.putString(KEY_ACTIVE_SHARE, key) From a5cb4b2fbe8ce97e2135df6eb82960ecdc2daa53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Mon, 23 Feb 2026 14:39:04 -0500 Subject: [PATCH 2/5] Fix security and code quality findings from PR review --- .../kotlin/io/privkey/keep/ImportShareScreen.kt | 4 ++-- .../main/kotlin/io/privkey/keep/KeepMobileApp.kt | 2 +- .../kotlin/io/privkey/keep/nip46/BunkerService.kt | 4 ++++ .../io/privkey/keep/nip55/Nip55ContentProvider.kt | 3 ++- .../privkey/keep/storage/AndroidKeystoreStorage.kt | 13 ++++++++++--- 5 files changed, 19 insertions(+), 7 deletions(-) diff --git a/app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt b/app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt index 5334baf3..8feddd41 100644 --- a/app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt @@ -164,11 +164,11 @@ fun ImportShareScreen( } } catch (e: KeyPermanentlyInvalidatedException) { clearChars() - if (BuildConfig.DEBUG) Log.e("ImportShare", "Biometric key invalidated during cipher init", e) + if (BuildConfig.DEBUG) Log.e("ImportShare", "Biometric key invalidated during cipher init: ${e::class.simpleName}") onError("Biometric key invalidated. Please re-enroll biometrics.") } catch (e: Exception) { clearChars() - if (BuildConfig.DEBUG) Log.e("ImportShare", "Failed to initialize cipher for biometric auth", e) + if (BuildConfig.DEBUG) Log.e("ImportShare", "Failed to initialize cipher for biometric auth: ${e::class.simpleName}") onError("Failed to initialize encryption") } } diff --git a/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt b/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt index cfeeae0f..407161dd 100644 --- a/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt +++ b/app/src/main/kotlin/io/privkey/keep/KeepMobileApp.kt @@ -101,7 +101,7 @@ class KeepMobileApp : Application() { keepMobile = newKeepMobile nip55Handler = Nip55Handler(newKeepMobile) }.onFailure { e -> - initError = "${e::class.simpleName}: ${e.message}" + initError = "Failed to initialize application" if (BuildConfig.DEBUG) Log.e(TAG, "Failed to initialize KeepMobile: ${e::class.simpleName}", e) } } diff --git a/app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt b/app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt index 1568db19..8b8ab06f 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt @@ -62,6 +62,7 @@ class BunkerService : Service() { private const val APPROVAL_TIMEOUT_MS = 60_000L private const val GLOBAL_RATE_LIMIT_WINDOW_MS = 60_000L private const val GLOBAL_MAX_REQUESTS_PER_WINDOW = 100 + private const val MAX_TRACKED_CLIENTS = 1000 private val HEX_PUBKEY_REGEX = Regex("^[a-fA-F0-9]{64}$") @@ -167,6 +168,9 @@ class BunkerService : Service() { return true } + if (clientRequestHistory.size >= MAX_TRACKED_CLIENTS && !clientRequestHistory.containsKey(clientPubkey)) { + clientRequestHistory.keys.firstOrNull()?.let { clientRequestHistory.remove(it) } + } val history = clientRequestHistory.computeIfAbsent(clientPubkey) { mutableListOf() } synchronized(history) { history.removeAll { it < now - RATE_LIMIT_WINDOW_MS } diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt b/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt index b6e75089..258a43fb 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt @@ -375,7 +375,8 @@ class Nip55ContentProvider : ContentProvider() { .setAutoCancel(true) .build() - NotificationManagerCompat.from(ctx).notify(backgroundNotificationId.getAndIncrement(), notification) + val notifId = 2000 + (backgroundNotificationId.getAndIncrement() % 1000) + NotificationManagerCompat.from(ctx).notify(notifId, notification) } private fun getVerifiedCaller(): String? { diff --git a/app/src/main/kotlin/io/privkey/keep/storage/AndroidKeystoreStorage.kt b/app/src/main/kotlin/io/privkey/keep/storage/AndroidKeystoreStorage.kt index eedf87a8..25831e69 100644 --- a/app/src/main/kotlin/io/privkey/keep/storage/AndroidKeystoreStorage.kt +++ b/app/src/main/kotlin/io/privkey/keep/storage/AndroidKeystoreStorage.kt @@ -302,11 +302,18 @@ class AndroidKeystoreStorage(private val context: Context) : SecureStorage { private fun readMetadataFromPrefs(sharePrefs: SharedPreferences): ShareMetadataInfo? = try { val groupPubkeyB64 = sharePrefs.getString(KEY_SHARE_GROUP_PUBKEY, "") ?: "" + val identifier = sharePrefs.getInt(KEY_SHARE_INDEX, 0) + val threshold = sharePrefs.getInt(KEY_SHARE_THRESHOLD, 0) + val totalShares = sharePrefs.getInt(KEY_SHARE_TOTAL, 0) + val ushortRange = UShort.MIN_VALUE.toInt()..UShort.MAX_VALUE.toInt() + require(identifier in ushortRange && threshold in ushortRange && totalShares in ushortRange) { + "Share metadata values out of UShort range" + } ShareMetadataInfo( name = sharePrefs.getString(KEY_SHARE_NAME, "") ?: "", - identifier = sharePrefs.getInt(KEY_SHARE_INDEX, 0).toUShort(), - threshold = sharePrefs.getInt(KEY_SHARE_THRESHOLD, 0).toUShort(), - totalShares = sharePrefs.getInt(KEY_SHARE_TOTAL, 0).toUShort(), + identifier = identifier.toUShort(), + threshold = threshold.toUShort(), + totalShares = totalShares.toUShort(), groupPubkey = Base64.decode(groupPubkeyB64, Base64.NO_WRAP) ) } catch (e: Exception) { From 995339038b2d9011f9d757ae66ebdc89288bcff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Mon, 23 Feb 2026 14:40:35 -0500 Subject: [PATCH 3/5] Fix negative modulo on notification ID overflow --- .../main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt b/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt index 258a43fb..6f698094 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt @@ -54,7 +54,7 @@ class Nip55ContentProvider : ContentProvider() { } private val rateLimiter = RateLimiter() - private val backgroundNotificationId = AtomicInteger(2000) + private val backgroundNotificationId = AtomicInteger(0) private val concurrentRequestSemaphore = Semaphore(4) private val app: KeepMobileApp? get() = context?.applicationContext as? KeepMobileApp @@ -375,7 +375,7 @@ class Nip55ContentProvider : ContentProvider() { .setAutoCancel(true) .build() - val notifId = 2000 + (backgroundNotificationId.getAndIncrement() % 1000) + val notifId = 2000 + Math.floorMod(backgroundNotificationId.getAndIncrement(), 1000) NotificationManagerCompat.from(ctx).notify(notifId, notification) } From 9a3bf919dc7bb1974384b574aa98b0c921bc9cc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Mon, 23 Feb 2026 14:42:29 -0500 Subject: [PATCH 4/5] Simplify PolicyResult: FallToUi as object, concise cursorOrNull --- .../io/privkey/keep/nip55/Nip55ContentProvider.kt | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt b/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt index 6f698094..2a72252c 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/Nip55ContentProvider.kt @@ -174,12 +174,9 @@ class Nip55ContentProvider : ContentProvider() { } private sealed class PolicyResult { - val cursorOrNull: Cursor? get() = when (this) { - is Decided -> cursor - is FallToUi -> null - } + val cursorOrNull: Cursor? get() = (this as? Decided)?.cursor class Decided(val cursor: Cursor?) : PolicyResult() - class FallToUi : PolicyResult() + object FallToUi : PolicyResult() } private fun evaluateAutoSignPolicy( @@ -200,12 +197,12 @@ class Nip55ContentProvider : ContentProvider() { ?: SignPolicy.MANUAL } ?: SignPolicy.MANUAL - if (effectivePolicy == SignPolicy.MANUAL) return PolicyResult.FallToUi() + if (effectivePolicy == SignPolicy.MANUAL) return PolicyResult.FallToUi if (effectivePolicy == SignPolicy.AUTO) { if (eventKind != null && isSensitiveKind(eventKind)) { if (BuildConfig.DEBUG) Log.d(TAG, "AUTO mode blocked for sensitive event kind $eventKind from ${hashPackageName(callerPackage)}") - return PolicyResult.FallToUi() + return PolicyResult.FallToUi } val safeguards = currentApp.getAutoSigningSafeguards() @@ -217,7 +214,7 @@ class Nip55ContentProvider : ContentProvider() { if (!safeguards.isOptedIn(callerPackage)) { if (BuildConfig.DEBUG) Log.d(TAG, "AUTO signing not opted-in for ${hashPackageName(callerPackage)}") - return PolicyResult.FallToUi() + return PolicyResult.FallToUi } val denyReason = when (val usageResult = safeguards.checkAndRecordUsage(callerPackage)) { From 851b044855b5491768c8ad85404a26d0d0d1150d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Mon, 23 Feb 2026 15:15:41 -0500 Subject: [PATCH 5/5] Fix PR review findings: error handling, loading state, structured concurrency --- .../io/privkey/keep/nip46/BunkerScreen.kt | 79 +++++++++++-------- .../io/privkey/keep/nip46/BunkerService.kt | 6 +- .../keep/nip46/Nip46ApprovalActivity.kt | 4 + .../keep/nip55/AppPermissionsScreen.kt | 20 +++-- .../keep/nip55/PermissionsManagementScreen.kt | 8 +- .../keep/nip55/SigningHistoryScreen.kt | 9 +++ 6 files changed, 84 insertions(+), 42 deletions(-) diff --git a/app/src/main/kotlin/io/privkey/keep/nip46/BunkerScreen.kt b/app/src/main/kotlin/io/privkey/keep/nip46/BunkerScreen.kt index 49dbd993..86ff9016 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip46/BunkerScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip46/BunkerScreen.kt @@ -226,6 +226,7 @@ private fun AddBunkerRelayDialog( val scope = rememberCoroutineScope() var newRelayUrl by remember { mutableStateOf("") } var error by remember { mutableStateOf(null) } + var isAdding by remember { mutableStateOf(false) } fun dismissDialog() { newRelayUrl = "" @@ -259,44 +260,58 @@ private fun AddBunkerRelayDialog( } }, confirmButton = { - TextButton(onClick = { - val trimmed = newRelayUrl.trim() - val bunkerRelays = parseBunkerUrlRelays(trimmed) - - when { - bunkerRelays != null -> scope.launch { - addBunkerRelays(bunkerRelays, relays).fold( - onSuccess = { toAdd -> - onRelaysUpdated(relays + toAdd) - dismissDialog() - }, - onFailure = { error = it.message } - ) - } - trimmed.startsWith("bunker://") -> - error = "No relay URLs found in bunker URL" - else -> { - val url = normalizeRelayUrl(trimmed) - val validationError = validateRelayUrl(url, relays) - if (validationError != null) { - error = validationError - } else { + TextButton( + onClick = { + if (isAdding) return@TextButton + val trimmed = newRelayUrl.trim() + val bunkerRelays = parseBunkerUrlRelays(trimmed) + + when { + bunkerRelays != null -> { + isAdding = true scope.launch { - val isInternal = withContext(Dispatchers.IO) { - BunkerConfigStore.isInternalHost(url) - } - if (isInternal) { - error = "Internal/private hosts are not allowed" - } else { - onRelaysUpdated(relays + url) - dismissDialog() + addBunkerRelays(bunkerRelays, relays).fold( + onSuccess = { toAdd -> + onRelaysUpdated(relays + toAdd) + dismissDialog() + }, + onFailure = { error = it.message } + ) + isAdding = false + } + } + trimmed.startsWith("bunker://") -> + error = "No relay URLs found in bunker URL" + else -> { + val url = normalizeRelayUrl(trimmed) + val validationError = validateRelayUrl(url, relays) + if (validationError != null) { + error = validationError + } else { + isAdding = true + scope.launch { + val isInternal = withContext(Dispatchers.IO) { + BunkerConfigStore.isInternalHost(url) + } + if (isInternal) { + error = "Internal/private hosts are not allowed" + } else { + onRelaysUpdated(relays + url) + dismissDialog() + } + isAdding = false } } } } + }, + enabled = !isAdding + ) { + if (isAdding) { + CircularProgressIndicator(modifier = Modifier.size(16.dp), strokeWidth = 2.dp) + } else { + Text("Add") } - }) { - Text("Add") } }, dismissButton = { diff --git a/app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt b/app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt index 8b8ab06f..90ab3849 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt @@ -169,7 +169,11 @@ class BunkerService : Service() { } if (clientRequestHistory.size >= MAX_TRACKED_CLIENTS && !clientRequestHistory.containsKey(clientPubkey)) { - clientRequestHistory.keys.firstOrNull()?.let { clientRequestHistory.remove(it) } + clientRequestHistory.keys.firstOrNull()?.let { evictKey -> + clientRequestHistory.remove(evictKey) + clientBackoffUntil.remove(evictKey) + clientConsecutiveRequests.remove(evictKey) + } } val history = clientRequestHistory.computeIfAbsent(clientPubkey) { mutableListOf() } synchronized(history) { diff --git a/app/src/main/kotlin/io/privkey/keep/nip46/Nip46ApprovalActivity.kt b/app/src/main/kotlin/io/privkey/keep/nip46/Nip46ApprovalActivity.kt index 70b9a9b8..8eca3df1 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip46/Nip46ApprovalActivity.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip46/Nip46ApprovalActivity.kt @@ -155,6 +155,8 @@ class Nip46ApprovalActivity : FragmentActivity() { keystoreStorage.clearPendingCipher(reqId) } respond(true, duration) + } catch (e: kotlin.coroutines.cancellation.CancellationException) { + throw e } catch (e: Exception) { keystoreStorage.clearPendingCipher(reqId) if (BuildConfig.DEBUG) Log.e(TAG, "Error during approval: ${e::class.simpleName}") @@ -180,6 +182,8 @@ class Nip46ApprovalActivity : FragmentActivity() { eventKind = eventKind, duration = duration ) + } catch (e: kotlin.coroutines.cancellation.CancellationException) { + throw e } catch (e: Exception) { if (BuildConfig.DEBUG) Log.e(TAG, "Failed to persist permission: ${e::class.simpleName}") } diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt b/app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt index 843abf72..66df45ad 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt @@ -166,15 +166,21 @@ private fun RevokeAllPermissionsDialog( TextButton( onClick = { coroutineScope.launch { - withContext(Dispatchers.IO) { - runCatching { permissionStore.revokePermission(packageName) } - if (isNip46) { - val pubkey = packageName.removePrefix("nip46:") - runCatching { revokeNip46Client(context, pubkey) } + try { + withContext(Dispatchers.IO) { + permissionStore.revokePermission(packageName) + if (isNip46) { + val pubkey = packageName.removePrefix("nip46:") + revokeNip46Client(context, pubkey) + } } + onDismissDialog() + onDismissScreen() + } catch (e: Exception) { + if (BuildConfig.DEBUG) Log.e("AppPermissions", "Revoke failed: ${e::class.simpleName}") + onDismissDialog() + Toast.makeText(context, "Failed to revoke permissions", Toast.LENGTH_SHORT).show() } - onDismissDialog() - onDismissScreen() } }, colors = ButtonDefaults.textButtonColors(contentColor = MaterialTheme.colorScheme.error) diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/PermissionsManagementScreen.kt b/app/src/main/kotlin/io/privkey/keep/nip55/PermissionsManagementScreen.kt index 251aa45e..c3276b7a 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/PermissionsManagementScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/PermissionsManagementScreen.kt @@ -106,7 +106,11 @@ fun PermissionsManagementScreen( modifier = Modifier.weight(1f), onRevokeAll = { showRevokeAllDialog = it }, onDecisionChange = { permission, newDecision -> - val requestType = findRequestType(permission.requestType) ?: return@PermissionsGroupedList + val requestType = findRequestType(permission.requestType) + if (requestType == null) { + loadError = "Unknown request type: ${permission.requestType}" + return@PermissionsGroupedList + } coroutineScope.launch { try { permissionStore.updatePermissionDecision( @@ -231,7 +235,7 @@ private fun PermissionsGroupedList( onDecisionChange: (Nip55Permission, PermissionDecision) -> Unit, onDelete: (Nip55Permission) -> Unit ) { - val groupedPermissions = permissions.groupBy { it.callerPackage } + val groupedPermissions = remember(permissions) { permissions.groupBy { it.callerPackage } } LazyColumn( modifier = modifier, diff --git a/app/src/main/kotlin/io/privkey/keep/nip55/SigningHistoryScreen.kt b/app/src/main/kotlin/io/privkey/keep/nip55/SigningHistoryScreen.kt index 6737a989..33675300 100644 --- a/app/src/main/kotlin/io/privkey/keep/nip55/SigningHistoryScreen.kt +++ b/app/src/main/kotlin/io/privkey/keep/nip55/SigningHistoryScreen.kt @@ -118,6 +118,15 @@ fun SigningHistoryScreen( Spacer(modifier = Modifier.height(12.dp)) + loadError?.let { error -> + Text( + text = error, + color = MaterialTheme.colorScheme.error, + style = MaterialTheme.typography.bodyMedium + ) + Spacer(modifier = Modifier.height(12.dp)) + } + if (availableApps.isNotEmpty()) { AppFilterDropdown( availableApps = availableApps,