From 38acf39ccc54ab47765134d4e0a45a3324e131b4 Mon Sep 17 00:00:00 2001 From: Jason van den Berg Date: Tue, 29 Sep 2026 13:48:02 +0200 Subject: [PATCH 1/5] fix: finish or roll back an interrupted ring pubky pick --- .../java/to/bitkit/repositories/PubkyRepo.kt | 121 ++++- .../screens/profile/PubkyChoiceViewModel.kt | 8 +- .../to/bitkit/repositories/PubkyRepoTest.kt | 457 +++++++++++++++++- .../profile/PubkyChoiceViewModelTest.kt | 45 ++ changelog.d/next/1363.fixed.md | 1 + 5 files changed, 604 insertions(+), 28 deletions(-) create mode 100644 changelog.d/next/1363.fixed.md diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index 9711ff9577..c65cd951fd 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -304,7 +304,8 @@ class PubkyRepo @Inject constructor( } if (ringPubkys.any { "${SharedPubkyContract.RING_SOURCE_PREFIX}$it" == reference }) return - Logger.warn("Adopted ring identity '${redacted(reference)}' is gone, clearing session", context = TAG) + val adoptedPubky = reference.removePrefix(SharedPubkyContract.RING_SOURCE_PREFIX) + Logger.warn("Adopted ring identity '${redacted(adoptedPubky)}' is gone, clearing session", context = TAG) runSuspendCatching { pubkyService.clearSessionAccess() } .onFailure { Logger.warn("Failed to clear adopted session access", it, context = TAG) } clearLocalState() @@ -317,38 +318,108 @@ class PubkyRepo @Inject constructor( suspend fun adoptRingIdentity(pubky: String): Result = withContext(ioDispatcher) { runSuspendCatching { ensureServiceInitialized() - val secretKeyHex = sharedPubkyClient.ringCredential(pubky).getOrThrow() - val publicKey = pubkyService.publicKeyFromSecret(secretKeyHex) - require(PubkyPublicKeyFormat.matches(publicKey, pubky)) { - "Ring credential does not match '${redacted(pubky)}'" + val hasProfile = initializeMutex.withLock { + if (_publicKey.value != null) throw PubkyAlreadySignedInError + val secretKeyHex = sharedPubkyClient.ringCredential(pubky).getOrThrow() + val publicKey = pubkyService.publicKeyFromSecret(secretKeyHex) + require(PubkyPublicKeyFormat.matches(publicKey, pubky)) { + "Ring credential does not match '${redacted(pubky)}'" + } + withContext(NonCancellable) { commitRingIdentity(pubky, publicKey, secretKeyHex) } } + loadContacts() + hasProfile + } + } + + private suspend fun commitRingIdentity(pubky: String, publicKey: String, secretKeyHex: String): Boolean { + val previousReference = runSuspendCatching { keychain.loadString(Keychain.Key.SHARED_PUBKY_SOURCE.name) } + val previousSession = runSuspendCatching { keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) } + var committed = false + val profile = try { keychain.upsertString( Keychain.Key.SHARED_PUBKY_SOURCE.name, "${SharedPubkyContract.RING_SOURCE_PREFIX}$pubky", ) + signInRingIdentity(publicKey, secretKeyHex, previousSession) + val remoteProfile = fetchRemoteProfile(publicKey) + .onFailure { Logger.warn("Failed to look up adopted ring profile", it, context = TAG) } + .getOrNull() + settingsStore.setPubkyProfileSetupPending(remoteProfile == null) + committed = true + remoteProfile + } finally { + if (!committed) rollBackRingIdentity(previousReference, previousSession) + } - runSuspendCatching { pubkyService.signIn(secretKeyHex) }.getOrElse { - val hasIdentityRecord = runSuspendCatching { pubkyService.hasIdentityRecord(publicKey) } - .onFailure { Logger.warn("Failed to check ring identity record", it, context = TAG) } - .getOrNull() - if (hasIdentityRecord != false) throw it - Logger.warn("Signing up ring identity without a published record", it, context = TAG) - val homegate = fetchHomegateSignupCode() - pubkyService.signUp(secretKeyHex, homegate.homeserverPubky, homegate.signupCode) - } + profile?.let { adoptedProfile -> + _profile.update { adoptedProfile } + runSuspendCatching { cacheMetadata(adoptedProfile) } + .onFailure { Logger.warn("Failed to cache adopted ring profile", it, context = TAG) } + } + _publicKey.update { publicKey.ensurePubkyPrefix() } + notifyBackupStateChanged() + Logger.info("Adopted ring identity for '${redacted(publicKey)}'", context = TAG) + return profile != null + } - _publicKey.update { publicKey.ensurePubkyPrefix() } - notifyBackupStateChanged() - Logger.info("Adopted ring identity for '${redacted(publicKey)}'", context = TAG) - loadProfile() - loadContacts() - val hasProfile = _profile.value != null - runSuspendCatching { settingsStore.setPubkyProfileSetupPending(!hasProfile) } - .onFailure { Logger.warn("Failed to save pending profile setup", it, context = TAG) } - hasProfile - }.onFailure { - runCatching { keychain.delete(Keychain.Key.SHARED_PUBKY_SOURCE.name) } + private suspend fun signInRingIdentity( + publicKey: String, + secretKeyHex: String, + previousSession: Result, + ) { + runSuspendCatching { pubkyService.signIn(secretKeyHex) }.getOrElse { + if (hasSessionChangedSince(previousSession)) throw it + val hasIdentityRecord = runSuspendCatching { pubkyService.hasIdentityRecord(publicKey) } + .onFailure { Logger.warn("Failed to check ring identity record", it, context = TAG) } + .getOrNull() + if (hasIdentityRecord != false) throw it + Logger.warn("Signing up ring identity without a published record", it, context = TAG) + val homegate = fetchHomegateSignupCode() + pubkyService.signUp(secretKeyHex, homegate.homeserverPubky, homegate.signupCode) + } + } + + private suspend fun hasSessionChangedSince(previousSession: Result): Boolean { + val currentSession = runSuspendCatching { keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) } + if (previousSession.isFailure || currentSession.isFailure) return true + return previousSession.getOrNull() != currentSession.getOrNull() + } + + private suspend fun rollBackRingIdentity( + previousReference: Result, + previousSession: Result, + ) { + if (hasSessionChangedSince(previousSession)) { + Logger.info("Discarding session saved by a failed ring identity adoption", context = TAG) + discardRingSession() + restoreRingReference(null) + return } + val hadSession = !previousSession.getOrNull().isNullOrEmpty() + restoreRingReference(previousReference.getOrNull()?.takeIf { hadSession }) + } + + private suspend fun discardRingSession() { + val revocationError = runSuspendCatching { pubkyService.signOut() }.exceptionOrNull() ?: return + Logger.warn("Failed to revoke abandoned ring session", revocationError, context = TAG) + val forgetError = runSuspendCatching { pubkyService.forgetSessionAccess() }.exceptionOrNull() ?: return + Logger.warn("Failed to forget abandoned ring session access", forgetError, context = TAG) + runSuspendCatching { pubkyService.clearSessionAccess() } + .onFailure { Logger.warn("Failed to clear abandoned ring session access", it, context = TAG) } + runSuspendCatching { keychain.delete(Keychain.Key.PAYKIT_SESSION.name) } + .onFailure { Logger.warn("Failed to delete abandoned ring session", it, context = TAG) } + } + + private suspend fun restoreRingReference(reference: String?) { + runSuspendCatching { + if (reference == null) { + keychain.delete(Keychain.Key.SHARED_PUBKY_SOURCE.name) + } else { + keychain.upsertString(Keychain.Key.SHARED_PUBKY_SOURCE.name, reference) + } + }.onFailure { Logger.warn("Failed to restore ring identity reference", it, context = TAG) } + notifyBackupStateChanged() } // endregion diff --git a/app/src/main/java/to/bitkit/ui/screens/profile/PubkyChoiceViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/profile/PubkyChoiceViewModel.kt index 17eef049ff..df3a3f2abf 100644 --- a/app/src/main/java/to/bitkit/ui/screens/profile/PubkyChoiceViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/profile/PubkyChoiceViewModel.kt @@ -19,6 +19,7 @@ import kotlinx.coroutines.launch import to.bitkit.R import to.bitkit.models.PubkyPublicKeyFormat import to.bitkit.models.Toast +import to.bitkit.repositories.PubkyAlreadySignedInError import to.bitkit.repositories.PubkyRepo import to.bitkit.ui.shared.toast.ToastEventBus import to.bitkit.utils.Logger @@ -49,8 +50,9 @@ class PubkyChoiceViewModel @Inject constructor( } fun onIdentityClick(pubky: String) { + if (_uiState.value.adoptingPubky != null) return + _uiState.update { it.copy(adoptingPubky = pubky) } viewModelScope.launch { - _uiState.update { it.copy(adoptingPubky = pubky) } pubkyRepo.adoptRingIdentity(pubky) .onSuccess { hasProfile -> if (hasProfile) { @@ -72,6 +74,10 @@ class PubkyChoiceViewModel @Inject constructor( _effects.emit(effect) } .onFailure { + if (it is PubkyAlreadySignedInError) { + _uiState.update { state -> state.copy(adoptingPubky = null, navigateToProfile = true) } + return@onFailure + } Logger.error("Failed to adopt ring identity", it, context = TAG) _uiState.update { state -> state.copy(adoptingPubky = null) } 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 51d8f2cb69..68007ca7ec 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -24,26 +24,34 @@ import kotlinx.collections.immutable.persistentListOf import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.async import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeoutOrNull import org.junit.Before import org.junit.Test import org.mockito.Mockito.clearInvocations +import org.mockito.invocation.InvocationOnMock import org.mockito.kotlin.any import org.mockito.kotlin.atLeastOnce import org.mockito.kotlin.doSuspendableAnswer +import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyBlocking import org.mockito.kotlin.whenever +import to.bitkit.async.ServiceQueue import to.bitkit.data.PubkyStore import to.bitkit.data.PubkyStoreData import to.bitkit.data.SettingsData @@ -61,12 +69,16 @@ import to.bitkit.services.PubkyRingAuthTimeoutError import to.bitkit.services.PubkyService import to.bitkit.test.BaseUnitTest import to.bitkit.utils.AppError +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.CopyOnWriteArraySet import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.seconds import com.synonym.paykit.PubkyProfile as SdkPubkyProfile @Suppress("LargeClass") @@ -77,6 +89,12 @@ class PubkyRepoTest : BaseUnitTest() { private const val NON_CANONICAL_CONTACT_KEY_A = "pubky3rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xg" private const val VALID_CONTACT_KEY_B = "pubky1rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xy" private const val VALID_SELF_KEY = "pubky5rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xy" + + /** Ring source reference that an adoption of [VALID_SELF_KEY] saves. */ + private val RING_REFERENCE = SharedPubkyContract.RING_SOURCE_PREFIX + VALID_SELF_KEY.removePrefix("pubky") + + /** Real-time bound on waiting for a stubbed step that runs on a service queue thread. */ + private val STEP_TIMEOUT = 5.seconds } private lateinit var sut: PubkyRepo @@ -89,6 +107,9 @@ class PubkyRepoTest : BaseUnitTest() { private val settingsStore = mock() private val settingsFlow = MutableStateFlow(SettingsData()) private val profileSetupPending = MutableStateFlow(false) + private val ringKeychain = ConcurrentHashMap() + private val ringPickEvents = CopyOnWriteArrayList() + private val failingRingTeardowns = CopyOnWriteArraySet() @Before fun setUp() = runBlocking { @@ -956,7 +977,7 @@ class PubkyRepoTest : BaseUnitTest() { } @Test - fun `adoptRingIdentity should reject a mismatching credential and clear the reference`() = test { + fun `adoptRingIdentity should reject a mismatching credential without touching the reference`() = test { val ringPubky = VALID_SELF_KEY.removePrefix("pubky") whenever(sharedPubkyClient.ringCredential(ringPubky)).thenReturn(Result.success("ring_secret")) whenever(pubkyService.publicKeyFromSecret("ring_secret")) @@ -967,7 +988,8 @@ class PubkyRepoTest : BaseUnitTest() { assertTrue(result.isFailure) assertNull(sut.publicKey.value) verifyBlocking(pubkyService, never()) { signIn(any()) } - verifyBlocking(keychain) { delete(Keychain.Key.SHARED_PUBKY_SOURCE.name) } + verifyBlocking(keychain, never()) { upsertString(eq(Keychain.Key.SHARED_PUBKY_SOURCE.name), any()) } + verifyBlocking(keychain, never()) { delete(Keychain.Key.SHARED_PUBKY_SOURCE.name) } } @Test @@ -1041,6 +1063,367 @@ class PubkyRepoTest : BaseUnitTest() { assertFalse(profileSetupPending.value) } + @Test + fun `adoptRingIdentity should finish the pick when cancelled during sign in`() = test { + stubRingPickKeychain() + val ringPubky = stubRingCredential() + val signInStarted = CompletableDeferred() + val finishSignIn = CompletableDeferred() + whenever(pubkyService.signIn("ring_secret")).doSuspendableAnswer { + ServiceQueue.CORE.background { + signInStarted.complete(Unit) + finishSignIn.await() + saveRingSession() + } + } + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).thenReturn(null) + + val pick = async { sut.adoptRingIdentity(ringPubky) } + val signInStartedInTime = signInStarted.awaitStarted() + pick.cancel() + finishSignIn.complete(Unit) + pick.join() + + assertTrue(signInStartedInTime, "Timed out waiting for sign in to start") + assertTrue(pick.isCancelled) + assertRingPickCommitted() + } + + @Test + fun `adoptRingIdentity should finish the pick when cancelled during the profile lookup`() = test { + stubRingPickKeychain() + val ringPubky = stubRingCredential() + stubRingSignInSavesSession() + val lookupStarted = CompletableDeferred() + val finishLookup = CompletableDeferred() + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).doSuspendableAnswer { + ServiceQueue.CORE.background { + lookupStarted.complete(Unit) + finishLookup.await() + null + } + } + + val pick = async { sut.adoptRingIdentity(ringPubky) } + val lookupStartedInTime = lookupStarted.awaitStarted() + pick.cancel() + finishLookup.complete(Unit) + pick.join() + + assertTrue(lookupStartedInTime, "Timed out waiting for the profile lookup to start") + assertTrue(pick.isCancelled) + assertRingPickCommitted() + } + + @Test + fun `adoptRingIdentity should keep the finished pick when cancelled during the contact load`() = test { + stubRingPickKeychain() + val ringPubky = stubRingCredential() + stubRingSignInSavesSession() + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).thenReturn(null) + val contactsStarted = CompletableDeferred() + val finishContacts = CompletableDeferred() + whenever(pubkyService.contactRecords()).doSuspendableAnswer { + ServiceQueue.CORE.background { + contactsStarted.complete(Unit) + finishContacts.await() + emptyList() + } + } + + val pick = async { sut.adoptRingIdentity(ringPubky) } + val contactsStartedInTime = contactsStarted.awaitStarted() + pick.cancel() + finishContacts.complete(Unit) + pick.join() + + assertTrue(contactsStartedInTime, "Timed out waiting for the contact load to start") + assertTrue(pick.isCancelled) + assertRingPickCommitted() + } + + @Test + fun `adoptRingIdentity should write nothing when cancelled before the commit`() = test { + stubRingPickKeychain() + val ringPubky = stubRingCredential() + val derivationStarted = CompletableDeferred() + whenever(pubkyService.publicKeyFromSecret("ring_secret")).doSuspendableAnswer { + derivationStarted.complete(Unit) + awaitCancellation() + } + + val pick = async { sut.adoptRingIdentity(ringPubky) } + val derivationStartedFirst = derivationStarted.isCompleted + pick.cancelAndJoin() + + assertTrue(derivationStartedFirst) + assertTrue(pick.isCancelled) + assertTrue(ringPickEvents.isEmpty()) + assertTrue(ringKeychain.isEmpty()) + verifyBlocking(pubkyService, never()) { signIn(any()) } + } + + @Test + fun `adoptRingIdentity should reject a retried pick once an abandoned pick signs in`() = test { + stubRingPickKeychain() + val ringPubky = stubRingCredential() + val firstStarted = CompletableDeferred() + val finishFirst = CompletableDeferred() + whenever(pubkyService.signIn("ring_secret")).doSuspendableAnswer { + if (!firstStarted.isCompleted) { + withContext(NonCancellable) { + firstStarted.complete(Unit) + finishFirst.await() + } + } + saveRingSession() + currentCoroutineContext().ensureActive() + } + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).thenReturn(null) + + val abandoned = async { sut.adoptRingIdentity(ringPubky) } + val firstSignInStarted = firstStarted.isCompleted + abandoned.cancel() + val retried = async { sut.adoptRingIdentity(ringPubky) } + val eventsWhileFirstSignsIn = ringPickEvents.toList() + finishFirst.complete(Unit) + abandoned.join() + + assertTrue(firstSignInStarted) + assertEquals(listOf("save reference"), eventsWhileFirstSignsIn) + assertTrue(abandoned.isCancelled) + assertEquals(PubkyAlreadySignedInError, retried.await().exceptionOrNull()) + assertRingPickCommitted() + verifyBlocking(pubkyService, times(1)) { signIn("ring_secret") } + } + + @Test + fun `adoptRingIdentity should refuse to run over a signed-in identity`() = test { + authenticateForTesting() + val ringPubky = stubRingCredential() + + val result = sut.adoptRingIdentity(ringPubky) + + assertEquals(PubkyAlreadySignedInError, result.exceptionOrNull()) + assertEquals("pubkytest_pk_12345", sut.publicKey.value) + verifyBlocking(sharedPubkyClient, never()) { ringCredential(any()) } + verifyBlocking(pubkyService, never()) { signIn(any()) } + verifyBlocking(keychain, never()) { upsertString(eq(Keychain.Key.SHARED_PUBKY_SOURCE.name), any()) } + } + + @Test + fun `adoptRingIdentity should discard the saved session before the reference when activation fails`() = test { + stubRingPickKeychain() + val ringPubky = stubRingCredential() + stubRingActivationFailure() + + val result = sut.adoptRingIdentity(ringPubky) + + assertEquals("Initialize failed", result.exceptionOrNull()?.message) + assertEquals(listOf("save reference", "save session", "sign out", "delete reference"), ringPickEvents) + assertRingPickSignedOut() + verifyBlocking(pubkyService, never()) { hasIdentityRecord(any()) } + verifyBlocking(pubkyService, never()) { signUp(any(), any(), any()) } + } + + @Test + fun `adoptRingIdentity should not sign up after sign in saved a session`() = test { + val httpClient = identityHttpClient() + sut = createSut(httpClient) + stubRingPickKeychain() + val ringPubky = stubRingCredential() + stubRingActivationFailure() + whenever(pubkyService.hasIdentityRecord(ringPubky)).thenReturn(false) + whenever(pubkyService.signUp("ring_secret", "test-homeserver", "test-code")).thenReturn(Unit) + + val result = sut.adoptRingIdentity(ringPubky) + httpClient.close() + + assertEquals("Initialize failed", result.exceptionOrNull()?.message) + assertRingPickSignedOut() + verifyBlocking(pubkyService, never()) { hasIdentityRecord(any()) } + verifyBlocking(pubkyService, never()) { signUp(any(), any(), any()) } + } + + @Test + fun `adoptRingIdentity should discard the session when the saved session cannot be read`() = test { + val httpClient = identityHttpClient() + sut = createSut(httpClient) + stubRingPickKeychain() + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)) + .thenAnswer { throw TestAppError("Locked") } + .thenAnswer { ringKeychain[Keychain.Key.PAYKIT_SESSION.name] } + val ringPubky = stubRingCredential() + whenever(pubkyService.signIn("ring_secret")).thenAnswer { throw TestAppError("Relay unavailable") } + whenever(pubkyService.hasIdentityRecord(ringPubky)).thenReturn(false) + whenever(pubkyService.signUp("ring_secret", "test-homeserver", "test-code")).thenReturn(Unit) + + val result = sut.adoptRingIdentity(ringPubky) + httpClient.close() + + assertEquals("Relay unavailable", result.exceptionOrNull()?.message) + assertEquals(listOf("save reference", "sign out", "delete reference"), ringPickEvents) + assertRingPickSignedOut() + verifyBlocking(pubkyService, never()) { hasIdentityRecord(any()) } + verifyBlocking(pubkyService, never()) { signUp(any(), any(), any()) } + } + + @Test + fun `adoptRingIdentity should roll back the pick when pending profile setup cannot be saved`() = test { + stubRingPickKeychain() + val ringPubky = stubRingCredential() + stubRingSignInSavesSession() + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).thenReturn(null) + whenever(settingsStore.setPubkyProfileSetupPending(true)).thenAnswer { throw TestAppError("Disk full") } + profileSetupPending.value = false + val publicKeys = CopyOnWriteArrayList() + backgroundScope.launch { sut.publicKey.collect { publicKeys += it } } + + val result = sut.adoptRingIdentity(ringPubky) + + assertEquals("Disk full", result.exceptionOrNull()?.message) + assertEquals(listOf("save reference", "save session", "sign out", "delete reference"), ringPickEvents) + assertRingPickSignedOut() + assertEquals(listOf(null), publicKeys) + } + + @Test + fun `adoptRingIdentity should save pending profile setup before publishing the identity`() = test { + val ringPubky = stubRingCredential() + whenever(pubkyService.signIn("ring_secret")).thenReturn(Unit) + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).thenReturn(null) + val publicKeysWhenPending = CopyOnWriteArrayList() + whenever(settingsStore.setPubkyProfileSetupPending(true)).thenAnswer { + publicKeysWhenPending += sut.publicKey.value + profileSetupPending.value = true + Unit + } + + val result = sut.adoptRingIdentity(ringPubky) + + assertEquals(false, result.getOrNull()) + assertEquals(listOf(null), publicKeysWhenPending) + assertEquals(VALID_SELF_KEY, sut.publicKey.value) + } + + @Test + fun `adoptRingIdentity should forget the saved session when revocation fails`() = test { + stubRingPickKeychain() + val ringPubky = stubRingCredential() + stubRingActivationFailure() + failingRingTeardowns += "sign out" + + val result = sut.adoptRingIdentity(ringPubky) + + assertEquals("Initialize failed", result.exceptionOrNull()?.message) + assertEquals( + listOf("save reference", "save session", "sign out", "forget session", "delete reference"), + ringPickEvents, + ) + assertRingPickSignedOut() + verifyBlocking(pubkyService, never()) { clearSessionAccess() } + } + + @Test + fun `adoptRingIdentity should clear the saved session locally when the sdk cannot`() = test { + stubRingPickKeychain() + val ringPubky = stubRingCredential() + stubRingActivationFailure() + failingRingTeardowns += listOf("sign out", "forget session", "clear session access") + + val result = sut.adoptRingIdentity(ringPubky) + + assertEquals("Initialize failed", result.exceptionOrNull()?.message) + assertEquals( + listOf( + "save reference", + "save session", + "sign out", + "forget session", + "clear session access", + "delete session", + "delete reference", + ), + ringPickEvents, + ) + assertRingPickSignedOut() + assertFalse(settingsFlow.value.publicPaykitCleanupPending) + } + + @Test + fun `adoptRingIdentity should roll back a failed pick after it was cancelled`() = test { + stubRingPickKeychain() + val ringPubky = stubRingCredential() + val signInStarted = CompletableDeferred() + val finishSignIn = CompletableDeferred() + whenever(pubkyService.signIn("ring_secret")).doSuspendableAnswer { + ServiceQueue.CORE.background { + signInStarted.complete(Unit) + finishSignIn.await() + saveRingSession() + throw TestAppError("Initialize failed") + } + } + failingRingTeardowns += "sign out" + + val pick = async { sut.adoptRingIdentity(ringPubky) } + val signInStartedInTime = signInStarted.awaitStarted() + pick.cancel() + finishSignIn.complete(Unit) + pick.join() + + assertTrue(signInStartedInTime, "Timed out waiting for sign in to start") + assertTrue(pick.isCancelled) + assertEquals( + listOf("save reference", "save session", "sign out", "forget session", "delete reference"), + ringPickEvents, + ) + assertRingPickSignedOut() + } + + @Test + fun `adoptRingIdentity should keep a saved session and its reference when sign in fails before saving`() = + test { + val keptPubky = VALID_CONTACT_KEY_A.removePrefix("pubky") + val keptReference = SharedPubkyContract.RING_SOURCE_PREFIX + keptPubky + stubRingPickKeychain( + Keychain.Key.PAYKIT_SESSION to "kept_session", + Keychain.Key.SHARED_PUBKY_SOURCE to keptReference, + ) + whenever(pubkyService.importSession("kept_session")).thenAnswer { throw TestAppError("Expired") } + whenever(sharedPubkyClient.ringCredential(keptPubky)) + .thenReturn(Result.failure(TestAppError("Unavailable"))) + whenever(sharedPubkyClient.listRingIdentities()).thenReturn(Result.failure(TestAppError("Unavailable"))) + sut.initialize() + assertTrue(sut.sessionRestorationFailed.value) + val ringPubky = stubRingCredential() + whenever(pubkyService.signIn("ring_secret")).thenAnswer { throw TestAppError("Relay unavailable") } + whenever(pubkyService.hasIdentityRecord(ringPubky)).thenReturn(true) + + val result = sut.adoptRingIdentity(ringPubky) + + assertEquals("Relay unavailable", result.exceptionOrNull()?.message) + assertEquals("kept_session", ringKeychain[Keychain.Key.PAYKIT_SESSION.name]) + assertEquals(keptReference, ringKeychain[Keychain.Key.SHARED_PUBKY_SOURCE.name]) + assertEquals(listOf("save reference", "save reference"), ringPickEvents) + assertNull(sut.publicKey.value) + } + + @Test + fun `adoptRingIdentity should delete a leftover reference without a saved session when sign in fails`() = test { + val leftoverReference = SharedPubkyContract.RING_SOURCE_PREFIX + VALID_CONTACT_KEY_A.removePrefix("pubky") + stubRingPickKeychain(Keychain.Key.SHARED_PUBKY_SOURCE to leftoverReference) + val ringPubky = stubRingCredential() + whenever(pubkyService.signIn("ring_secret")).thenAnswer { throw TestAppError("Relay unavailable") } + whenever(pubkyService.hasIdentityRecord(ringPubky)).thenReturn(true) + + val result = sut.adoptRingIdentity(ringPubky) + + assertEquals("Relay unavailable", result.exceptionOrNull()?.message) + assertEquals(listOf("save reference", "delete reference"), ringPickEvents) + assertRingPickSignedOut() + } + @Test fun `initialize should clear an adopted identity that is gone from pubky ring`() = test { stubAdoptedRingSource() @@ -1867,6 +2250,76 @@ class PubkyRepoTest : BaseUnitTest() { status = status, ) + private suspend fun stubRingPickKeychain(vararg saved: Pair) { + saved.forEach { (key, value) -> ringKeychain[key.name] = value } + mapOf( + Keychain.Key.PAYKIT_SESSION.name to "session", + Keychain.Key.SHARED_PUBKY_SOURCE.name to "reference", + ).forEach { (key, label) -> + whenever(keychain.loadString(key)).thenAnswer { ringKeychain[key] } + whenever(keychain.exists(key)).thenAnswer { ringKeychain.containsKey(key) } + whenever(keychain.upsertString(eq(key), any())).doSuspendableAnswer { + currentCoroutineContext().ensureActive() + ringKeychain[key] = it.getArgument(1) + ringPickEvents += "save $label" + Unit + } + whenever(keychain.delete(key)).doSuspendableAnswer { + currentCoroutineContext().ensureActive() + ringKeychain.remove(key) + ringPickEvents += "delete $label" + Unit + } + } + whenever(pubkyService.signOut()).doSuspendableAnswer(ringTeardown("sign out")) + whenever(pubkyService.forgetSessionAccess()).doSuspendableAnswer(ringTeardown("forget session")) + whenever(pubkyService.clearSessionAccess()).doSuspendableAnswer(ringTeardown("clear session access")) + } + + private fun ringTeardown(event: String): suspend (InvocationOnMock) -> Unit = { + currentCoroutineContext().ensureActive() + ServiceQueue.CORE.background { + ringPickEvents += event + if (event in failingRingTeardowns) throw TestAppError("Offline") + ringKeychain.remove(Keychain.Key.PAYKIT_SESSION.name) + ringKeychain.remove(Keychain.Key.SHARED_PUBKY_SOURCE.name) + Unit + } + } + + private fun saveRingSession() { + ringKeychain[Keychain.Key.PAYKIT_SESSION.name] = "ring_session" + ringPickEvents += "save session" + } + + private suspend fun stubRingSignInSavesSession() { + whenever(pubkyService.signIn("ring_secret")).thenAnswer { saveRingSession() } + } + + private suspend fun stubRingActivationFailure() { + whenever(pubkyService.signIn("ring_secret")).thenAnswer { + saveRingSession() + throw TestAppError("Initialize failed") + } + } + + private suspend fun CompletableDeferred.awaitStarted(): Boolean = + withContext(Dispatchers.Default) { withTimeoutOrNull(STEP_TIMEOUT) { await() } } != null + + private fun assertRingPickCommitted() { + assertEquals(VALID_SELF_KEY, sut.publicKey.value) + assertEquals("ring_session", ringKeychain[Keychain.Key.PAYKIT_SESSION.name]) + assertEquals(RING_REFERENCE, ringKeychain[Keychain.Key.SHARED_PUBKY_SOURCE.name]) + assertTrue(profileSetupPending.value) + assertEquals(listOf("save reference", "save session"), ringPickEvents) + } + + private fun assertRingPickSignedOut() { + assertNull(sut.publicKey.value) + assertTrue(ringKeychain.isEmpty(), "Expected no saved session or reference, found '$ringKeychain'") + assertFalse(profileSetupPending.value) + } + private suspend fun stubRingCredential(): String { val ringPubky = VALID_SELF_KEY.removePrefix("pubky") whenever(sharedPubkyClient.ringCredential(ringPubky)).thenReturn(Result.success("ring_secret")) diff --git a/app/src/test/java/to/bitkit/ui/screens/profile/PubkyChoiceViewModelTest.kt b/app/src/test/java/to/bitkit/ui/screens/profile/PubkyChoiceViewModelTest.kt index fece646fec..c45e12a39a 100644 --- a/app/src/test/java/to/bitkit/ui/screens/profile/PubkyChoiceViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/screens/profile/PubkyChoiceViewModelTest.kt @@ -2,17 +2,22 @@ package to.bitkit.ui.screens.profile import android.content.Context import kotlinx.collections.immutable.persistentListOf +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch import kotlinx.coroutines.test.advanceUntilIdle import org.junit.Before import org.junit.Test +import org.mockito.kotlin.doSuspendableAnswer import org.mockito.kotlin.mock +import org.mockito.kotlin.times +import org.mockito.kotlin.verifyBlocking import org.mockito.kotlin.whenever import to.bitkit.R import to.bitkit.models.PubkyProfile import to.bitkit.models.Toast +import to.bitkit.repositories.PubkyAlreadySignedInError import to.bitkit.repositories.PubkyRepo import to.bitkit.test.BaseUnitTest import to.bitkit.ui.shared.toast.ToastEventBus @@ -177,6 +182,46 @@ class PubkyChoiceViewModelTest : BaseUnitTest() { toastJob.cancel() } + @Test + fun `onIdentityClick opens the profile when another pick already signed in`() = test { + whenever(pubkyRepo.adoptRingIdentity(RING_PUBKY)).thenReturn(Result.failure(PubkyAlreadySignedInError)) + createSut() + val effects = mutableListOf() + val toasts = mutableListOf() + val effectsJob = launch { sut.effects.collect { effects.add(it) } } + val toastJob = launch { ToastEventBus.events.collect { toasts.add(it) } } + + sut.onIdentityClick(RING_PUBKY) + advanceUntilIdle() + + assertTrue(sut.uiState.value.navigateToProfile) + assertNull(sut.uiState.value.adoptingPubky) + assertTrue(effects.isEmpty()) + assertTrue(toasts.isEmpty()) + effectsJob.cancel() + toastJob.cancel() + } + + @Test + fun `onIdentityClick ignores another click while a pick is in progress`() = test { + val finishPick = CompletableDeferred>() + whenever(pubkyRepo.adoptRingIdentity(RING_PUBKY)).doSuspendableAnswer { finishPick.await() } + createSut() + val effects = mutableListOf() + val effectsJob = launch { sut.effects.collect { effects.add(it) } } + + sut.onIdentityClick(RING_PUBKY) + sut.onIdentityClick(RING_PUBKY) + finishPick.complete(Result.success(false)) + advanceUntilIdle() + + verifyBlocking(pubkyRepo, times(1)) { adoptRingIdentity(RING_PUBKY) } + assertEquals(PubkyChoiceEffect.NavigateToCreateProfile, effects.single()) + assertFalse(sut.uiState.value.navigateToProfile) + assertNull(sut.uiState.value.adoptingPubky) + effectsJob.cancel() + } + @Test fun `session restoration redirects to profile when already authenticated`() = test { isAuthenticated.value = true diff --git a/changelog.d/next/1363.fixed.md b/changelog.d/next/1363.fixed.md new file mode 100644 index 0000000000..3979f40cf4 --- /dev/null +++ b/changelog.d/next/1363.fixed.md @@ -0,0 +1 @@ +Going back while Bitkit is signing in with a Pubky Ring pubky now completes the sign-in so profile setup can resume, and a failed sign-in no longer leaves a hidden session behind. From 7e744e5c4c0b3a37ad0d51eee012f0f73528e9cb Mon Sep 17 00:00:00 2001 From: Jason van den Berg Date: Tue, 29 Sep 2026 13:48:02 +0200 Subject: [PATCH 2/5] fix: clear the pubky session before its ring reference --- .../to/bitkit/services/PaykitSdkService.kt | 2 +- .../bitkit/services/PaykitSdkServiceTest.kt | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt index ecbfdffb84..68dc19bdf6 100644 --- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt +++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt @@ -1223,8 +1223,8 @@ internal class PaykitSdkSessionProvider( override fun clearSessionAccess() { clearLiveSessionAccess() keychain.accessBlocking { - delete(Keychain.Key.SHARED_PUBKY_SOURCE.name) clearPubkySessionCredentials(::delete) + delete(Keychain.Key.SHARED_PUBKY_SOURCE.name) } } diff --git a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt index e61d30889c..dd9d4351d7 100644 --- a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt +++ b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt @@ -290,6 +290,39 @@ class PaykitSdkServiceTest { ) } + @Test + fun `session provider clears the session before the ring reference`() { + val blocking = mock() + val provider = sessionProvider(blocking) + + provider.clearSessionAccess() + + inOrder(blocking) { + verify(blocking).delete(Keychain.Key.PAYKIT_SESSION.name) + verify(blocking).delete(Keychain.Key.PUBKY_SECRET_KEY.name) + verify(blocking).delete(Keychain.Key.SHARED_PUBKY_SOURCE.name) + } + } + + @Test + fun `session provider keeps the ring reference when the session cannot be cleared`() { + val blocking = mock() + val provider = sessionProvider(blocking) + whenever(blocking.delete(Keychain.Key.PAYKIT_SESSION.name)).doAnswer { throw AppError("Delete failed") } + + assertFailsWith { provider.clearSessionAccess() } + + verify(blocking, never()).delete(Keychain.Key.SHARED_PUBKY_SOURCE.name) + } + + private fun sessionProvider(blocking: Keychain.BlockingAccess): PaykitSdkSessionProvider { + val keychain = mock() + whenever(keychain.accessBlocking(any())).doAnswer { + it.getArgument Any?>(0).invoke(blocking) + } + return PaykitSdkSessionProvider(keychain, mock()) + } + private fun keyStore( loadBytes: () -> ByteArray?, upsertBytes: (ByteArray) -> Unit = {}, From 57ab5c56adef9ad2c1020c50cf65bb472d4d1764 Mon Sep 17 00:00:00 2001 From: Jason van den Berg Date: Tue, 29 Sep 2026 14:05:19 +0200 Subject: [PATCH 3/5] fix: recheck the pubky auth handler when ring returns --- app/src/main/AndroidManifest.xml | 2 +- .../java/to/bitkit/repositories/PubkyRepo.kt | 10 +++- .../services/PubkyAuthHandlerRegistrar.kt | 15 ++++-- .../to/bitkit/repositories/PubkyRepoTest.kt | 46 +++++++++++++++++++ .../services/PubkyAuthHandlerRegistrarTest.kt | 41 ++++++++++++++++- changelog.d/next/1363.fixed.md | 2 +- 6 files changed, 109 insertions(+), 7 deletions(-) diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index cd89e25350..12045520f8 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -165,7 +165,7 @@ android:resource="@xml/shortcuts" /> - + = _adoptedSourceLost.asStateFlow() + private val _adoptedSourceUnreachable = MutableStateFlow(false) + val adoptedSourceUnreachable: StateFlow = _adoptedSourceUnreachable.asStateFlow() + private val _pendingImportProfile = MutableStateFlow(null) val pendingImportProfile: StateFlow = _pendingImportProfile.asStateFlow() @@ -300,9 +303,13 @@ class PubkyRepo @Inject constructor( val reference = keychain.loadString(Keychain.Key.SHARED_PUBKY_SOURCE.name) ?: return val ringPubkys = sharedPubkyClient.listRingIdentities().getOrElse { Logger.warn("Failed to list ring identities", it, context = TAG) + _adoptedSourceUnreachable.update { true } + return + } + if (ringPubkys.any { "${SharedPubkyContract.RING_SOURCE_PREFIX}$it" == reference }) { + _adoptedSourceUnreachable.update { false } return } - if (ringPubkys.any { "${SharedPubkyContract.RING_SOURCE_PREFIX}$it" == reference }) return val adoptedPubky = reference.removePrefix(SharedPubkyContract.RING_SOURCE_PREFIX) Logger.warn("Adopted ring identity '${redacted(adoptedPubky)}' is gone, clearing session", context = TAG) @@ -1367,6 +1374,7 @@ class PubkyRepo @Inject constructor( _contactsLoadCompletionVersion.update { 0L } clearPendingImport() _sessionRestorationFailed.update { false } + _adoptedSourceUnreachable.update { false } } private fun markContactsLoaded() { diff --git a/app/src/main/java/to/bitkit/services/PubkyAuthHandlerRegistrar.kt b/app/src/main/java/to/bitkit/services/PubkyAuthHandlerRegistrar.kt index cd7a13870d..1696d15702 100644 --- a/app/src/main/java/to/bitkit/services/PubkyAuthHandlerRegistrar.kt +++ b/app/src/main/java/to/bitkit/services/PubkyAuthHandlerRegistrar.kt @@ -20,7 +20,12 @@ import java.util.concurrent.atomic.AtomicBoolean import javax.inject.Inject import javax.inject.Singleton -/** Advertises Pubky signup and authorization handlers when their required identity state is available. */ +/** + * Advertises Pubky signup and authorization handlers when their required identity state is available. + * + * The authorization handler is re-checked when the identity, the Paykit flag or the reachability of an adopted + * Pubky Ring pubky changes, so it follows Pubky Ring going away and coming back without a restart. + */ @Singleton internal class PubkyAuthHandlerRegistrar @Inject constructor( @ApplicationContext private val context: Context, @@ -48,8 +53,12 @@ internal class PubkyAuthHandlerRegistrar @Inject constructor( collectionScope.launch { pubkyRepo.awaitInitialization() - combine(settingsStore.isPaykitEnabled, pubkyRepo.publicKey) { localFlagEnabled, publicKey -> - localFlagEnabled to publicKey + combine( + settingsStore.isPaykitEnabled, + pubkyRepo.publicKey, + pubkyRepo.adoptedSourceUnreachable, + ) { localFlagEnabled, publicKey, adoptedSourceUnreachable -> + Triple(localFlagEnabled, publicKey, adoptedSourceUnreachable) } .distinctUntilChanged() .collectLatest { (localFlagEnabled, publicKey) -> diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index 68007ca7ec..aeee0cf7da 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -1444,9 +1444,55 @@ class PubkyRepoTest : BaseUnitTest() { sut.initialize() assertFalse(sut.adoptedSourceLost.value) + assertTrue(sut.adoptedSourceUnreachable.value) verifyBlocking(pubkyService, never()) { clearSessionAccess() } } + @Test + fun `checkAdoptedSource should mark pubky ring reachable again without changing the identity`() = test { + val session = "saved_session" + val ringPubky = VALID_SELF_KEY.removePrefix("pubky") + stubAdoptedRingSource() + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn(session) + whenever(pubkyService.importSession(session)).thenReturn(ringPubky) + whenever(sharedPubkyClient.listRingIdentities()).thenReturn(Result.failure(TestAppError("Unavailable"))) + sut.initialize() + assertTrue(sut.adoptedSourceUnreachable.value) + + whenever(sharedPubkyClient.listRingIdentities()).thenReturn(Result.success(persistentListOf(ringPubky))) + val result = sut.checkAdoptedSource() + + assertTrue(result.isSuccess) + assertFalse(sut.adoptedSourceUnreachable.value) + assertFalse(sut.adoptedSourceLost.value) + assertEquals(VALID_SELF_KEY, sut.publicKey.value) + verifyBlocking(sharedPubkyClient, never()) { ringCredential(any()) } + } + + @Test + fun `checkAdoptedSource should reset the unreachable flag when pubky ring drops the pubky`() = test { + stubAdoptedRingSource() + whenever(sharedPubkyClient.listRingIdentities()).thenReturn(Result.failure(TestAppError("Unavailable"))) + sut.checkAdoptedSource() + assertTrue(sut.adoptedSourceUnreachable.value) + + whenever(sharedPubkyClient.listRingIdentities()) + .thenReturn(Result.success(persistentListOf(VALID_CONTACT_KEY_A.removePrefix("pubky")))) + sut.checkAdoptedSource() + + assertTrue(sut.adoptedSourceLost.value) + assertFalse(sut.adoptedSourceUnreachable.value) + } + + @Test + fun `checkAdoptedSource should not query pubky ring without an adopted pubky`() = test { + val result = sut.checkAdoptedSource() + + assertTrue(result.isSuccess) + assertFalse(sut.adoptedSourceUnreachable.value) + verifyBlocking(sharedPubkyClient, never()) { listRingIdentities() } + } + @Test fun `checkAdoptedSource should clear an adopted identity removed from pubky ring after startup`() = test { stubAdoptedRingSource() diff --git a/app/src/test/java/to/bitkit/services/PubkyAuthHandlerRegistrarTest.kt b/app/src/test/java/to/bitkit/services/PubkyAuthHandlerRegistrarTest.kt index 0dada70974..ecd44cf668 100644 --- a/app/src/test/java/to/bitkit/services/PubkyAuthHandlerRegistrarTest.kt +++ b/app/src/test/java/to/bitkit/services/PubkyAuthHandlerRegistrarTest.kt @@ -18,6 +18,7 @@ import org.mockito.kotlin.doThrow import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.robolectric.RobolectricTestRunner @@ -42,6 +43,7 @@ class PubkyAuthHandlerRegistrarTest : BaseUnitTest() { private val settingsStore: SettingsStore = mock() private val isPaykitEnabled = MutableStateFlow(false) private val publicKey = MutableStateFlow(null) + private val adoptedSourceUnreachable = MutableStateFlow(false) @Before fun setUp() { @@ -49,6 +51,7 @@ class PubkyAuthHandlerRegistrarTest : BaseUnitTest() { whenever(context.packageManager).thenReturn(packageManager) whenever(settingsStore.isPaykitEnabled).thenReturn(isPaykitEnabled) whenever(pubkyRepo.publicKey).thenReturn(publicKey) + whenever(pubkyRepo.adoptedSourceUnreachable).thenReturn(adoptedSourceUnreachable) } @Test @@ -118,7 +121,7 @@ class PubkyAuthHandlerRegistrarTest : BaseUnitTest() { } @Test - fun `handler is disabled for a Ring managed identity`() = test { + fun `handler is disabled while the Ring pubky secret key is unavailable`() = test { isPaykitEnabled.value = true publicKey.value = "pubkyring" whenever(pubkyRepo.hasSecretKey()).thenReturn(false) @@ -129,6 +132,42 @@ class PubkyAuthHandlerRegistrarTest : BaseUnitTest() { verifyComponentStates(authEnabled = false, signupEnabled = false) } + @Test + fun `handler is enabled when Pubky Ring becomes reachable again for the same pubky`() = test { + isPaykitEnabled.value = true + publicKey.value = "pubkyring" + whenever(pubkyRepo.hasSecretKey()).thenReturn(false) + createSut().start(backgroundScope) + runCurrent() + adoptedSourceUnreachable.value = true + runCurrent() + clearInvocations(packageManager) + + whenever(pubkyRepo.hasSecretKey()).thenReturn(true) + adoptedSourceUnreachable.value = false + runCurrent() + + verifyComponentStates(authEnabled = true, signupEnabled = false) + verify(pubkyRepo, times(3)).hasSecretKey() + } + + @Test + fun `handler is disabled when Pubky Ring becomes unreachable for the same pubky`() = test { + isPaykitEnabled.value = true + publicKey.value = "pubkyring" + whenever(pubkyRepo.hasSecretKey()).thenReturn(true) + createSut().start(backgroundScope) + runCurrent() + clearInvocations(packageManager) + + whenever(pubkyRepo.hasSecretKey()).thenReturn(false) + adoptedSourceUnreachable.value = true + runCurrent() + + verifyComponentStates(authEnabled = false, signupEnabled = false) + verify(pubkyRepo, times(2)).hasSecretKey() + } + @Test fun `authorization handler switches to signup when the local identity is removed`() = test { isPaykitEnabled.value = true diff --git a/changelog.d/next/1363.fixed.md b/changelog.d/next/1363.fixed.md index 3979f40cf4..0506db1ada 100644 --- a/changelog.d/next/1363.fixed.md +++ b/changelog.d/next/1363.fixed.md @@ -1 +1 @@ -Going back while Bitkit is signing in with a Pubky Ring pubky now completes the sign-in so profile setup can resume, and a failed sign-in no longer leaves a hidden session behind. +Going back while signing in with a Pubky Ring pubky now finishes the sign-in so profile setup can resume, a failed sign-in no longer leaves a hidden session behind, and Pubky authorization links reach Bitkit again once Pubky Ring is available, without a restart. From 25902dc3cce332d1a597558440059359a3ec86fe Mon Sep 17 00:00:00 2001 From: Jason van den Berg Date: Tue, 29 Sep 2026 14:49:01 +0200 Subject: [PATCH 4/5] fix: load contacts and log failures of an interrupted ring pick --- .../java/to/bitkit/repositories/PubkyRepo.kt | 15 +++- .../screens/profile/PubkyChoiceViewModel.kt | 1 - .../to/bitkit/repositories/PubkyRepoTest.kt | 89 +++++++++++++++---- 3 files changed, 82 insertions(+), 23 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index 99a11eab50..95cdae9d4b 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -300,7 +300,10 @@ class PubkyRepo @Inject constructor( private suspend fun checkAdoptedSourcePresent() { if (!adoptedSourceCheckMutex.tryLock()) return try { - val reference = keychain.loadString(Keychain.Key.SHARED_PUBKY_SOURCE.name) ?: return + val reference = keychain.loadString(Keychain.Key.SHARED_PUBKY_SOURCE.name) ?: run { + _adoptedSourceUnreachable.update { false } + return + } val ringPubkys = sharedPubkyClient.listRingIdentities().getOrElse { Logger.warn("Failed to list ring identities", it, context = TAG) _adoptedSourceUnreachable.update { true } @@ -334,13 +337,16 @@ class PubkyRepo @Inject constructor( } withContext(NonCancellable) { commitRingIdentity(pubky, publicKey, secretKeyHex) } } - loadContacts() + withContext(NonCancellable) { loadContacts() } hasProfile + }.onFailure { + if (it !is PubkyAlreadySignedInError) Logger.error("Failed to adopt ring identity", it, context = TAG) } } private suspend fun commitRingIdentity(pubky: String, publicKey: String, secretKeyHex: String): Boolean { val previousReference = runSuspendCatching { keychain.loadString(Keychain.Key.SHARED_PUBKY_SOURCE.name) } + .getOrNull() val previousSession = runSuspendCatching { keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) } var committed = false val profile = try { @@ -364,6 +370,7 @@ class PubkyRepo @Inject constructor( runSuspendCatching { cacheMetadata(adoptedProfile) } .onFailure { Logger.warn("Failed to cache adopted ring profile", it, context = TAG) } } + _adoptedSourceUnreachable.update { false } _publicKey.update { publicKey.ensurePubkyPrefix() } notifyBackupStateChanged() Logger.info("Adopted ring identity for '${redacted(publicKey)}'", context = TAG) @@ -394,7 +401,7 @@ class PubkyRepo @Inject constructor( } private suspend fun rollBackRingIdentity( - previousReference: Result, + previousReference: String?, previousSession: Result, ) { if (hasSessionChangedSince(previousSession)) { @@ -404,7 +411,7 @@ class PubkyRepo @Inject constructor( return } val hadSession = !previousSession.getOrNull().isNullOrEmpty() - restoreRingReference(previousReference.getOrNull()?.takeIf { hadSession }) + restoreRingReference(previousReference?.takeIf { hadSession }) } private suspend fun discardRingSession() { diff --git a/app/src/main/java/to/bitkit/ui/screens/profile/PubkyChoiceViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/profile/PubkyChoiceViewModel.kt index df3a3f2abf..11ecd0f66e 100644 --- a/app/src/main/java/to/bitkit/ui/screens/profile/PubkyChoiceViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/profile/PubkyChoiceViewModel.kt @@ -78,7 +78,6 @@ class PubkyChoiceViewModel @Inject constructor( _uiState.update { state -> state.copy(adoptingPubky = null, navigateToProfile = true) } return@onFailure } - Logger.error("Failed to adopt ring identity", it, context = TAG) _uiState.update { state -> state.copy(adoptingPubky = null) } ToastEventBus.send( type = Toast.ToastType.ERROR, diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index aeee0cf7da..24d1a90108 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -1089,6 +1089,34 @@ class PubkyRepoTest : BaseUnitTest() { assertRingPickCommitted() } + @Test + fun `adoptRingIdentity should finish a pick with a profile when cancelled during sign in`() = test { + stubRingPickKeychain() + val ringPubky = stubRingCredential() + val signInStarted = CompletableDeferred() + val finishSignIn = CompletableDeferred() + whenever(pubkyService.signIn("ring_secret")).doSuspendableAnswer { + ServiceQueue.CORE.background { + signInStarted.complete(Unit) + finishSignIn.await() + saveRingSession() + } + } + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)) + .thenReturn(createResolution(VALID_SELF_KEY, pubkyProfile = createPubkyProfile(name = "Alice"))) + + val pick = async { sut.adoptRingIdentity(ringPubky) } + val signInStartedInTime = signInStarted.awaitStarted() + pick.cancel() + finishSignIn.complete(Unit) + pick.join() + + assertTrue(signInStartedInTime, "Timed out waiting for sign in to start") + assertTrue(pick.isCancelled) + assertRingPickCommitted(hasProfile = true) + assertEquals("Alice", sut.profile.value?.name) + } + @Test fun `adoptRingIdentity should finish the pick when cancelled during the profile lookup`() = test { stubRingPickKeychain() @@ -1116,7 +1144,7 @@ class PubkyRepoTest : BaseUnitTest() { } @Test - fun `adoptRingIdentity should keep the finished pick when cancelled during the contact load`() = test { + fun `adoptRingIdentity should finish the contact load when cancelled during it`() = test { stubRingPickKeychain() val ringPubky = stubRingCredential() stubRingSignInSavesSession() @@ -1147,16 +1175,22 @@ class PubkyRepoTest : BaseUnitTest() { stubRingPickKeychain() val ringPubky = stubRingCredential() val derivationStarted = CompletableDeferred() + val finishDerivation = CompletableDeferred() whenever(pubkyService.publicKeyFromSecret("ring_secret")).doSuspendableAnswer { - derivationStarted.complete(Unit) - awaitCancellation() + ServiceQueue.CORE.background { + derivationStarted.complete(Unit) + finishDerivation.await() + ringPubky + } } val pick = async { sut.adoptRingIdentity(ringPubky) } - val derivationStartedFirst = derivationStarted.isCompleted - pick.cancelAndJoin() + val derivationStartedInTime = derivationStarted.awaitStarted() + pick.cancel() + finishDerivation.complete(Unit) + pick.join() - assertTrue(derivationStartedFirst) + assertTrue(derivationStartedInTime, "Timed out waiting for key derivation to start") assertTrue(pick.isCancelled) assertTrue(ringPickEvents.isEmpty()) assertTrue(ringKeychain.isEmpty()) @@ -1424,6 +1458,23 @@ class PubkyRepoTest : BaseUnitTest() { assertRingPickSignedOut() } + @Test + fun `adoptRingIdentity should mark pubky ring reachable when a pick commits`() = test { + stubAdoptedRingSource() + whenever(sharedPubkyClient.listRingIdentities()).thenReturn(Result.failure(TestAppError("Unavailable"))) + sut.checkAdoptedSource() + assertTrue(sut.adoptedSourceUnreachable.value) + val ringPubky = stubRingCredential() + whenever(pubkyService.signIn("ring_secret")).thenReturn(Unit) + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).thenReturn(null) + + val result = sut.adoptRingIdentity(ringPubky) + + assertTrue(result.isSuccess) + assertEquals(VALID_SELF_KEY, sut.publicKey.value) + assertFalse(sut.adoptedSourceUnreachable.value) + } + @Test fun `initialize should clear an adopted identity that is gone from pubky ring`() = test { stubAdoptedRingSource() @@ -2296,7 +2347,7 @@ class PubkyRepoTest : BaseUnitTest() { status = status, ) - private suspend fun stubRingPickKeychain(vararg saved: Pair) { + private fun stubRingPickKeychain(vararg saved: Pair) { saved.forEach { (key, value) -> ringKeychain[key.name] = value } mapOf( Keychain.Key.PAYKIT_SESSION.name to "session", @@ -2304,22 +2355,22 @@ class PubkyRepoTest : BaseUnitTest() { ).forEach { (key, label) -> whenever(keychain.loadString(key)).thenAnswer { ringKeychain[key] } whenever(keychain.exists(key)).thenAnswer { ringKeychain.containsKey(key) } - whenever(keychain.upsertString(eq(key), any())).doSuspendableAnswer { + whenever { keychain.upsertString(eq(key), any()) }.doSuspendableAnswer { currentCoroutineContext().ensureActive() ringKeychain[key] = it.getArgument(1) ringPickEvents += "save $label" Unit } - whenever(keychain.delete(key)).doSuspendableAnswer { + whenever { keychain.delete(key) }.doSuspendableAnswer { currentCoroutineContext().ensureActive() ringKeychain.remove(key) ringPickEvents += "delete $label" Unit } } - whenever(pubkyService.signOut()).doSuspendableAnswer(ringTeardown("sign out")) - whenever(pubkyService.forgetSessionAccess()).doSuspendableAnswer(ringTeardown("forget session")) - whenever(pubkyService.clearSessionAccess()).doSuspendableAnswer(ringTeardown("clear session access")) + whenever { pubkyService.signOut() }.doSuspendableAnswer(ringTeardown("sign out")) + whenever { pubkyService.forgetSessionAccess() }.doSuspendableAnswer(ringTeardown("forget session")) + whenever { pubkyService.clearSessionAccess() }.doSuspendableAnswer(ringTeardown("clear session access")) } private fun ringTeardown(event: String): suspend (InvocationOnMock) -> Unit = { @@ -2338,12 +2389,12 @@ class PubkyRepoTest : BaseUnitTest() { ringPickEvents += "save session" } - private suspend fun stubRingSignInSavesSession() { - whenever(pubkyService.signIn("ring_secret")).thenAnswer { saveRingSession() } + private fun stubRingSignInSavesSession() { + whenever { pubkyService.signIn("ring_secret") }.thenAnswer { saveRingSession() } } - private suspend fun stubRingActivationFailure() { - whenever(pubkyService.signIn("ring_secret")).thenAnswer { + private fun stubRingActivationFailure() { + whenever { pubkyService.signIn("ring_secret") }.thenAnswer { saveRingSession() throw TestAppError("Initialize failed") } @@ -2352,12 +2403,14 @@ class PubkyRepoTest : BaseUnitTest() { private suspend fun CompletableDeferred.awaitStarted(): Boolean = withContext(Dispatchers.Default) { withTimeoutOrNull(STEP_TIMEOUT) { await() } } != null - private fun assertRingPickCommitted() { + private fun assertRingPickCommitted(hasProfile: Boolean = false) { assertEquals(VALID_SELF_KEY, sut.publicKey.value) assertEquals("ring_session", ringKeychain[Keychain.Key.PAYKIT_SESSION.name]) assertEquals(RING_REFERENCE, ringKeychain[Keychain.Key.SHARED_PUBKY_SOURCE.name]) - assertTrue(profileSetupPending.value) + assertEquals(!hasProfile, profileSetupPending.value) assertEquals(listOf("save reference", "save session"), ringPickEvents) + verifyBlocking(pubkyService) { contactRecords() } + assertTrue(sut.contactsLoadCompletionVersion.value > 0) } private fun assertRingPickSignedOut() { From 6f080b63673970993ec512666598b06b6f4d7faa Mon Sep 17 00:00:00 2001 From: Jason van den Berg Date: Wed, 30 Sep 2026 08:38:15 +0200 Subject: [PATCH 5/5] chore: rename changelog fragment --- changelog.d/next/{1363.fixed.md => 1393.fixed.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/next/{1363.fixed.md => 1393.fixed.md} (100%) diff --git a/changelog.d/next/1363.fixed.md b/changelog.d/next/1393.fixed.md similarity index 100% rename from changelog.d/next/1363.fixed.md rename to changelog.d/next/1393.fixed.md