diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index e9f94c2cb8..49c64e3810 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -1240,16 +1240,6 @@ class PubkyRepo @Inject constructor( } } - suspend fun deleteProfileWithSessionRetry(): Result = withContext(ioDispatcher) { - val initialResult = deleteProfile() - if (initialResult.isSuccess) return@withContext initialResult - - val refreshedSession = refreshSessionIfPossible().getOrDefault(false) - if (!refreshedSession) return@withContext initialResult - - deleteProfile() - } - suspend fun deleteProfile(): Result = runSuspendCatching { withContext(ioDispatcher) { requireNotNull(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)) { @@ -1283,10 +1273,10 @@ class PubkyRepo @Inject constructor( val publicKey = _publicKey.value ?: return val known = ownProfileRevision val revision = known ?: pubkyService.fetchPubkyProfileWithRevision(publicKey)?.revision ?: return - runSuspendCatching { withPubkyAppScope { pubkyService.deletePubkyProfile(revision, publicKey) } }.getOrElse { + runSuspendCatching { pubkyService.deletePubkyProfile(revision, publicKey) }.getOrElse { if (known == null || !it.isConcurrentUpdate()) throw it val current = pubkyService.fetchPubkyProfileWithRevision(publicKey)?.revision - if (current != null) withPubkyAppScope { pubkyService.deletePubkyProfile(current, publicKey) } + if (current != null) pubkyService.deletePubkyProfile(current, publicKey) } ownProfileRevision = null } @@ -2319,7 +2309,7 @@ class PubkyRepo @Inject constructor( } } - suspend fun refreshSessionIfPossible(): Result = runSuspendCatching { + private suspend fun refreshSessionIfPossible(): Result = runSuspendCatching { withContext(ioDispatcher) { val storedSecretKeyHex = activeSecretKeyHex() ?: return@withContext false diff --git a/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt index ad2302f4a4..b03f5e61e6 100644 --- a/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt @@ -294,7 +294,7 @@ class EditProfileViewModel @Inject constructor( try { privatePaykitRepo.beginProfileDeletion() privatePaykitRepo.removePublishedEndpointsForCleanup(TAG, deferAppSync = true) - val result = pubkyRepo.deleteProfileWithSessionRetry() + val result = pubkyRepo.deleteProfile() if (result.isSuccess) { privatePaykitRepo.closeAndClear() ToastEventBus.send( diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index 6625ed2540..131888168a 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -1414,6 +1414,26 @@ class PubkyRepoTest : BaseUnitTest() { verifyBlocking(pubkyService) { deletePubkyProfile("revision-Test", VALID_SELF_KEY) } } + @Test + fun `pubky app deletion does not renew the session or replay an identity error`() = test { + val secretKey = "local_secret" + authenticateForTesting(publicKey = VALID_SELF_KEY) + whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(secretKey) + whenever(pubkyService.publicKeyFromSecret(secretKey)).thenReturn(VALID_SELF_KEY.removePrefix("pubky")) + val identityError = PaykitException.Identity("identity_error", "Missing capabilities") + whenever(pubkyService.deletePubkyProfile("revision-Test", VALID_SELF_KEY)) + .thenAnswer { throw identityError } + .thenReturn(Unit) + + val result = sut.deleteProfile() + + assertSame(identityError, result.exceptionOrNull()) + verify(pubkyService).deletePubkyProfile("revision-Test", VALID_SELF_KEY) + verify(pubkyService, never()).signIn(any()) + verify(pubkyService, never()).deletePaykitProfile() + verify(pubkyService, never()).signOut() + } + @Test fun `deleteProfile is not followed by a profile copy write from a load under way`() = test { authenticateForTesting(publicKey = VALID_SELF_KEY) @@ -2121,36 +2141,46 @@ class PubkyRepoTest : BaseUnitTest() { } @Test - fun `deleteProfileWithSessionRetry should refresh session and retry delete`() = test { - val expiredSession = "expired_session" - val newSession = "new_session" + fun `profile deletion does not replay after an uncertain response`() = test { val secretKey = "local_secret" - authenticateForTesting(publicKey = VALID_SELF_KEY, secret = expiredSession) - whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn(expiredSession, newSession) + authenticateForTesting(publicKey = VALID_SELF_KEY, secret = "test_session") whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(secretKey) - whenever(pubkyService.deletePaykitProfile()).thenAnswer { throw TestAppError("Expired") }.thenReturn(Unit) - whenever(pubkyService.signIn(secretKey)).thenReturn(Unit) whenever(pubkyService.publicKeyFromSecret(secretKey)).thenReturn(VALID_SELF_KEY.removePrefix("pubky")) + val uncertainResponse = TestAppError("Connection closed after profile deletion") + whenever(pubkyService.deletePaykitProfile()).thenAnswer { throw uncertainResponse }.thenReturn(Unit) - val result = sut.deleteProfileWithSessionRetry() + val result = sut.deleteProfile() - assertTrue(result.isSuccess) - verifyBlocking(pubkyService, times(2)) { deletePaykitProfile() } + assertSame(uncertainResponse, result.exceptionOrNull()) + verify(pubkyService).deletePaykitProfile() + verify(pubkyService, never()).signIn(any()) + verify(pubkyService, never()).signOut() } @Test - fun `deleteProfileWithSessionRetry should return failure when session cannot refresh`() = test { - val expiredSession = "expired_session" - authenticateForTesting(publicKey = VALID_SELF_KEY, secret = expiredSession) - whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn(expiredSession) - whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(null) - whenever(pubkyService.deletePaykitProfile()).thenAnswer { throw TestAppError("Expired") } - - val result = sut.deleteProfileWithSessionRetry() + fun `failed profile deletion does not restore an identity after sign out`() = test { + val secretKey = "local_secret" + authenticateForTesting(publicKey = VALID_SELF_KEY, secret = "test_session") + whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(secretKey) + whenever(pubkyService.publicKeyFromSecret(secretKey)).thenReturn(VALID_SELF_KEY.removePrefix("pubky")) + val deletionStarted = CompletableDeferred() + val finishDeletion = CompletableDeferred() + val uncertainResponse = TestAppError("Connection closed after profile deletion") + whenever(pubkyService.deletePaykitProfile()).doSuspendableAnswer { + deletionStarted.complete(Unit) + withContext(NonCancellable) { finishDeletion.await() } + throw uncertainResponse + } + val deletion = async { sut.deleteProfile() } + deletionStarted.await() + sut.signOut().getOrThrow() + finishDeletion.complete(Unit) - assertTrue(result.isFailure) - verifyBlocking(pubkyService) { deletePaykitProfile() } - verifyBlocking(pubkyService, never()) { signIn(any()) } + assertSame(uncertainResponse, deletion.await().exceptionOrNull()) + assertNull(sut.publicKey.value) + assertFalse(sut.isAuthenticated.value) + verify(pubkyService, never()).signIn(any()) + verify(pubkyService).deletePaykitProfile() } @Test @@ -4420,31 +4450,6 @@ class PubkyRepoTest : BaseUnitTest() { assertFalse(profileSetupPending.value) } - @Test - fun `refreshSessionIfPossible should refresh session when local secret key exists`() = test { - val secretKey = "local_secret" - whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(secretKey) - whenever(pubkyService.signIn(secretKey)).thenReturn(Unit) - whenever(pubkyService.publicKeyFromSecret(secretKey)).thenReturn(VALID_SELF_KEY.removePrefix("pubky")) - - val result = sut.refreshSessionIfPossible() - - assertEquals(true, result.getOrNull()) - assertEquals(VALID_SELF_KEY, sut.publicKey.value) - assertTrue(sut.isAuthenticated.value) - } - - @Test - fun `refreshSessionIfPossible should return false when local secret key is missing`() = test { - whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(null) - - val result = sut.refreshSessionIfPossible() - - assertEquals(false, result.getOrNull()) - assertNull(sut.publicKey.value) - assertFalse(sut.isAuthenticated.value) - } - @Test fun `restoreSessionBackupState should derive local secret key for local seed backups`() = test { whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn("test mnemonic") @@ -5348,7 +5353,7 @@ class PubkyRepoTest : BaseUnitTest() { } @Test - fun `a contact edit started before the session of its identity is refreshed still saves`() = test { + fun `a contact edit started before the session of its identity is restored still saves`() = test { val store = stubGatedPubkyStore() authenticateForTesting(publicKey = VALID_SELF_KEY) whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn("local_secret") @@ -5356,7 +5361,7 @@ class PubkyRepoTest : BaseUnitTest() { whenever(pubkyService.publicKeyFromSecret("local_secret")).thenReturn(VALID_SELF_KEY.removePrefix("pubky")) val signIn = checkNotNull(sut.currentSignIn()) - assertEquals(true, sut.refreshSessionIfPossible().getOrNull()) + sut.initialize().getOrThrow() val result = sut.updateContact(signIn, VALID_CONTACT_KEY_A, "Alice", "", null, emptyList(), listOf("Friend")) assertTrue(result.isSuccess) diff --git a/app/src/test/java/to/bitkit/ui/screens/profile/EditProfileViewModelTest.kt b/app/src/test/java/to/bitkit/ui/screens/profile/EditProfileViewModelTest.kt index 83107c23de..61bad7ecd1 100644 --- a/app/src/test/java/to/bitkit/ui/screens/profile/EditProfileViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/screens/profile/EditProfileViewModelTest.kt @@ -62,7 +62,7 @@ class EditProfileViewModelTest : BaseUnitTest() { sut.showDeleteConfirmation() advanceUntilIdle() verify(pubkyRepo, times(1)).forgetUnrestoredIdentity() - verify(pubkyRepo, never()).deleteProfileWithSessionRetry() + verify(pubkyRepo, never()).deleteProfile() verify(pubkyRepo, never()).saveProfile(any(), any(), any(), any(), anyOrNull(), any()) assertFalse(sut.uiState.value.showDeleteDialog) @@ -115,7 +115,7 @@ class EditProfileViewModelTest : BaseUnitTest() { val sut = createSut() advanceUntilIdle() val result = CompletableDeferred>() - whenever(pubkyRepo.deleteProfileWithSessionRetry()).doSuspendableAnswer { result.await() } + whenever(pubkyRepo.deleteProfile()).doSuspendableAnswer { result.await() } sut.showDeleteConfirmation() sut.deleteProfile() @@ -131,7 +131,7 @@ class EditProfileViewModelTest : BaseUnitTest() { sut.disconnectProfile() sut.showDeleteConfirmation() advanceUntilIdle() - verify(pubkyRepo, times(1)).deleteProfileWithSessionRetry() + verify(pubkyRepo, times(1)).deleteProfile() verify(pubkyRepo, never()).saveProfile(any(), any(), any(), any(), anyOrNull(), any()) verify(pubkyRepo, never()).signOut() assertFalse(sut.uiState.value.showDeleteDialog) @@ -165,7 +165,7 @@ class EditProfileViewModelTest : BaseUnitTest() { sut.showDeleteConfirmation() advanceUntilIdle() verify(pubkyRepo, times(1)).saveProfile(any(), any(), any(), any(), anyOrNull(), any()) - verify(pubkyRepo, never()).deleteProfileWithSessionRetry() + verify(pubkyRepo, never()).deleteProfile() verify(pubkyRepo, never()).forgetUnrestoredIdentity() assertFalse(sut.uiState.value.showDeleteDialog) @@ -187,7 +187,7 @@ class EditProfileViewModelTest : BaseUnitTest() { @Test fun `deleteProfile should emit success when repository delete succeeds`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn(Result.success(Unit)) + whenever(pubkyRepo.deleteProfile()).thenReturn(Result.success(Unit)) val sut = createSut() advanceUntilIdle() @@ -201,14 +201,14 @@ class EditProfileViewModelTest : BaseUnitTest() { assertFalse(sut.uiState.value.showDeleteFailureDialog) inOrder(privatePaykitRepo, pubkyRepo).apply { verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) - verify(pubkyRepo).deleteProfileWithSessionRetry() + verify(pubkyRepo).deleteProfile() verify(privatePaykitRepo).closeAndClear() } } @Test fun `profile created in Bitkit keeps delete copy`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn(Result.success(Unit)) + whenever(pubkyRepo.deleteProfile()).thenReturn(Result.success(Unit)) val sut = createSut() advanceUntilIdle() @@ -222,7 +222,7 @@ class EditProfileViewModelTest : BaseUnitTest() { @Test fun `ring identity is unlinked with the same cleanup`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn(Result.success(Unit)) + whenever(pubkyRepo.deleteProfile()).thenReturn(Result.success(Unit)) val sut = createSut(isRingIdentity = true) advanceUntilIdle() @@ -238,7 +238,7 @@ class EditProfileViewModelTest : BaseUnitTest() { verify(context, never()).getString(R.string.profile__delete_success) inOrder(privatePaykitRepo, pubkyRepo).apply { verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) - verify(pubkyRepo).deleteProfileWithSessionRetry() + verify(pubkyRepo).deleteProfile() verify(privatePaykitRepo).closeAndClear() } } @@ -260,7 +260,7 @@ class EditProfileViewModelTest : BaseUnitTest() { @Test fun `retryDeleteProfile should emit success when repository delete succeeds`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn(Result.success(Unit)) + whenever(pubkyRepo.deleteProfile()).thenReturn(Result.success(Unit)) val sut = createSut() advanceUntilIdle() @@ -274,14 +274,14 @@ class EditProfileViewModelTest : BaseUnitTest() { assertFalse(sut.uiState.value.showDeleteFailureDialog) inOrder(privatePaykitRepo, pubkyRepo).apply { verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) - verify(pubkyRepo).deleteProfileWithSessionRetry() + verify(pubkyRepo).deleteProfile() verify(privatePaykitRepo).closeAndClear() } } @Test fun `deleteProfile should show retry dialog when delete still fails`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn( + whenever(pubkyRepo.deleteProfile()).thenReturn( Result.failure(TestAppError("expired session")), ) @@ -299,7 +299,7 @@ class EditProfileViewModelTest : BaseUnitTest() { @Test fun `disconnectProfile should emit disconnect success`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn( + whenever(pubkyRepo.deleteProfile()).thenReturn( Result.failure(TestAppError("expired session")), ) whenever(pubkyRepo.signOut()).thenReturn(Result.success(Unit)) @@ -358,7 +358,7 @@ class EditProfileViewModelTest : BaseUnitTest() { val sut = createSut() whenever { privatePaykitRepo.removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) } .thenReturn(Result.failure(TestAppError("cleanup failed"))) - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn(Result.success(Unit)) + whenever(pubkyRepo.deleteProfile()).thenReturn(Result.success(Unit)) advanceUntilIdle() sut.effects.test { @@ -371,14 +371,14 @@ class EditProfileViewModelTest : BaseUnitTest() { assertFalse(sut.uiState.value.isDeleting) inOrder(privatePaykitRepo, pubkyRepo).apply { verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) - verify(pubkyRepo).deleteProfileWithSessionRetry() + verify(pubkyRepo).deleteProfile() verify(privatePaykitRepo).closeAndClear() } } @Test fun `dismissDeleteFailureDialog should hide retry dialog`() = test { - whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn( + whenever(pubkyRepo.deleteProfile()).thenReturn( Result.failure(TestAppError("expired session")), ) diff --git a/changelog.d/next/1688.fixed.md b/changelog.d/next/1688.fixed.md new file mode 100644 index 0000000000..fb97b9ecd4 --- /dev/null +++ b/changelog.d/next/1688.fixed.md @@ -0,0 +1 @@ +Prevent uncertain profile deletion failures from automatically repeating contact cleanup and deletion. diff --git a/journeys/profile/delete-profile-uncertain-response.xml b/journeys/profile/delete-profile-uncertain-response.xml new file mode 100644 index 0000000000..fe825c36ff --- /dev/null +++ b/journeys/profile/delete-profile-uncertain-response.xml @@ -0,0 +1,17 @@ + + + Requires a disposable test identity and a controlled homeserver proxy; never use a personal profile. + Configure the proxy to accept one profile deletion but lose its acknowledgement, retaining a redacted + request counter. This is a fault-injection fixture requirement, not evidence that this journey has run. + Normal SDK recovery may retry an individual request only after a confirmed authentication rejection; + the app must not restart contact cleanup and profile deletion because a request has an uncertain outcome. + + + Open Edit Profile and confirm deletion + Drop the acknowledgement after the server accepts the profile deletion + Verify failure options are shown and no success notification or onboarding navigation occurs + Wait through the network timeout and verify the app starts no second contact cleanup, profile deletion or sign-in + Cancel the failure dialog and verify no mutation is retried + Restart the app without choosing Retry and verify it does not automatically repeat the deletion + +