From 1cf5d7359462c4fe7be76b85d2d216a796042160 Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Wed, 30 Sep 2026 03:08:00 +0200 Subject: [PATCH 1/4] fix: clean up an interrupted ring pubky pick Roll back the session and Ring reference when the pick is cancelled after sign in, and clear both when activation fails after the session is saved. Refs #1363 --- .../java/to/bitkit/repositories/PubkyRepo.kt | 22 ++++++- .../to/bitkit/services/PaykitSdkService.kt | 58 ++++++++++------- .../to/bitkit/repositories/PubkyRepoTest.kt | 63 +++++++++++++++--- .../bitkit/services/PaykitSdkServiceTest.kt | 64 +++++++++++++++++++ changelog.d/next/1363.fixed.md | 1 + 5 files changed, 172 insertions(+), 36 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 9eac4b0a2b..d96bcab809 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -344,7 +344,8 @@ class PubkyRepo @Inject constructor( suspend fun adoptRingIdentity(pubky: String): Result = withContext(ioDispatcher) { val reference = "${SharedPubkyContract.RING_SOURCE_PREFIX}$pubky" - var identityInstalled = false + var sessionInstalled = false + var completed = false try { runSuspendCatching { val publicKey = initializeMutex.withLock { @@ -356,11 +357,11 @@ class PubkyRepo @Inject constructor( } keychain.upsertString(Keychain.Key.SHARED_PUBKY_SOURCE.name, reference) signInOrSignUpAdoptedIdentity(secretKeyHex, rawPublicKey) + sessionInstalled = true val prefixedPublicKey = rawPublicKey.ensurePubkyPrefix() clearProfileIfIdentityChanged(prefixedPublicKey) _publicKey.update { prefixedPublicKey } - identityInstalled = true notifyBackupStateChanged() Logger.info("Adopted ring identity for '${redacted(rawPublicKey)}'", context = TAG) prefixedPublicKey @@ -374,15 +375,30 @@ class PubkyRepo @Inject constructor( val hasProfile = _profile.value?.publicKey == publicKey runSuspendCatching { settingsStore.setPubkyProfileSetupPending(!hasProfile) } .onFailure { Logger.warn("Failed to save pending profile setup", it, context = TAG) } + completed = true hasProfile } }.onFailure { clearAdoptedSourceIfMatches(reference) } } catch (error: CancellationException) { - if (!identityInstalled) clearAdoptedSourceIfMatches(reference) + if (!completed) rollBackInterruptedAdoption(reference, sessionInstalled) throw error } } + /** An interrupted pick ends signed out, with no session and no Ring reference left behind. */ + private suspend fun rollBackInterruptedAdoption(reference: String, sessionInstalled: Boolean) { + withContext(NonCancellable + ioDispatcher) { + initializeMutex.withLock { + if (keychain.loadString(Keychain.Key.SHARED_PUBKY_SOURCE.name) != reference) return@withLock + if (sessionInstalled) { + discardAbandonedSession() + clearLocalState() + } + clearAdoptedSourceIfMatches(reference) + } + } + } + private suspend fun clearAdoptedSourceIfMatches(reference: String) { runSuspendCatching { if (keychain.loadString(Keychain.Key.SHARED_PUBKY_SOURCE.name) == reference) { diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt index c2a5fde8fa..45e4f83a76 100644 --- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt +++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt @@ -360,13 +360,7 @@ class PaykitSdkService @Inject constructor( signupCode = signupCode, requiredCapabilities = requiredCapabilities(), ) - operationMutex.withLock { - activateBootstrapResult( - result = result, - previousPublicKey = previousPublicKey, - ) - } - notifyBackupStateChanged() + activateSignedIn(result, previousPublicKey) return result } @@ -389,16 +383,14 @@ class PaykitSdkService @Inject constructor( isSetup.await() val previousPublicKey = operationMutex.withLock { currentSdkStatePublicKeyLocked() } operationMutex.withLock { - var activated = false - try { - activateBootstrapResult( - result = result, - previousPublicKey = previousPublicKey, - ) - activated = true - } finally { - if (!activated) clearRegisteredIdentityActivationLocked() - } + activateOrClearLocked(result, previousPublicKey, onlyAdoptedRingIdentity = false) + } + notifyBackupStateChanged() + } + + internal suspend fun activateSignedIn(result: PubkySessionBootstrapResult, previousPublicKey: String?) { + operationMutex.withLock { + activateOrClearLocked(result, previousPublicKey, onlyAdoptedRingIdentity = true) } notifyBackupStateChanged() } @@ -411,13 +403,7 @@ class PaykitSdkService @Inject constructor( receiverNoiseSecretKey = sessionProvider.loadOrDeriveReceiverNoiseSecretKey(), requiredCapabilities = requiredCapabilities(), ) - operationMutex.withLock { - activateBootstrapResult( - result = result, - previousPublicKey = previousPublicKey, - ) - } - notifyBackupStateChanged() + activateSignedIn(result, previousPublicKey) return result } @@ -995,6 +981,30 @@ class PaykitSdkService @Inject constructor( republishIdentityIfNeeded(publicKey = result.publicKey) } + /** + * Activation saves the session before it initializes the SDK, so a failure leaves a session behind. + * An adopted Ring identity has no local secret key to sign in again with, so its session is cleared + * together with the Ring reference. Local identities keep their credentials for the sign-in retry. + */ + private suspend fun activateOrClearLocked( + result: PubkySessionBootstrapResult, + previousPublicKey: String?, + onlyAdoptedRingIdentity: Boolean, + ) { + var activated = false + try { + activateBootstrapResult( + result = result, + previousPublicKey = previousPublicKey, + ) + activated = true + } finally { + if (!activated && (!onlyAdoptedRingIdentity || sessionProvider.adoptedPubky() != null)) { + clearRegisteredIdentityActivationLocked() + } + } + } + private suspend fun clearRegisteredIdentityActivationLocked() = withContext(NonCancellable) { runSuspendCatching { sessionProvider.clearSessionAccess() } .onFailure { Logger.warn("Failed to clear incomplete Pubky signup session", it, context = TAG) } diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index 7e5d5e9810..cd70be632e 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -1209,17 +1209,21 @@ class PubkyRepoTest : BaseUnitTest() { } @Test - fun `canceling adopted identity profile loading keeps the committed identity`() = test { + fun `canceling an adopted identity after sign in signs out and clears the ring reference`() = test { sut.awaitInitialization() val ringPubky = stubRingCredential() val reference = SharedPubkyContract.RING_SOURCE_PREFIX + ringPubky - var source: String? = null - whenever(keychain.loadString(Keychain.Key.SHARED_PUBKY_SOURCE.name)).thenAnswer { source } - whenever(keychain.upsertString(Keychain.Key.SHARED_PUBKY_SOURCE.name, reference)).thenAnswer { - source = reference + var session: String? = null + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenAnswer { session } + whenever(pubkyService.signIn("ring_secret")).thenAnswer { + session = "installed_session" + Unit + } + whenever(pubkyService.signOut()).thenAnswer { + session = null + adoptedSource = null Unit } - whenever(pubkyService.signIn("ring_secret")).thenReturn(Unit) val profileLoadStarted = CompletableDeferred() whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).doSuspendableAnswer { profileLoadStarted.complete(Unit) @@ -1227,12 +1231,53 @@ class PubkyRepoTest : BaseUnitTest() { } val adoption = async { sut.adoptRingIdentity(ringPubky) } profileLoadStarted.await() + assertEquals(reference, adoptedSource) + assertEquals(VALID_SELF_KEY, sut.publicKey.value) adoption.cancelAndJoin() - assertEquals(VALID_SELF_KEY, sut.publicKey.value) - assertEquals(reference, source) - verifyBlocking(pubkyService, never()) { signOut() } + assertNull(session) + assertNull(adoptedSource) + assertNull(sut.publicKey.value) + assertNull(sut.profile.value) + assertFalse(profileSetupPending.value) + verifyBlocking(pubkyService) { signOut() } + } + + @Test + fun `canceling an adopted identity clears the session even when the revocation fails`() = test { + sut.awaitInitialization() + val ringPubky = stubRingCredential() + var session: String? = null + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenAnswer { session } + whenever(keychain.delete(Keychain.Key.PAYKIT_SESSION.name)).thenAnswer { + session = null + Unit + } + whenever(pubkyService.signIn("ring_secret")).thenAnswer { + session = "installed_session" + Unit + } + whenever(pubkyService.signOut()).thenAnswer { throw TestAppError("Offline") } + whenever(pubkyService.forgetSessionAccess()).thenAnswer { + session = null + adoptedSource = null + Unit + } + val profileLoadStarted = CompletableDeferred() + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).doSuspendableAnswer { + profileLoadStarted.complete(Unit) + awaitCancellation() + } + val adoption = async { sut.adoptRingIdentity(ringPubky) } + profileLoadStarted.await() + + adoption.cancelAndJoin() + + assertNull(session) + assertNull(adoptedSource) + assertNull(sut.publicKey.value) + verifyBlocking(pubkyService) { forgetSessionAccess() } } @Test diff --git a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt index 9ddf826083..138a2b6069 100644 --- a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt +++ b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt @@ -106,6 +106,70 @@ class PaykitSdkServiceTest { } } + @Test + fun `sign in for an adopted ring identity clears its session and ring reference when activation fails`() = runTest { + val keychain = mock() + val blocking = mock() + whenever(keychain.accessBlocking(any())).doAnswer { + it.getArgument Any?>(0).invoke(blocking) + } + whenever(blocking.load(Keychain.Key.PAYKIT_RECEIVER_NOISE_SECRET_KEY.name)).thenReturn(ByteArray(32) { 1 }) + whenever(keychain.loadString(Keychain.Key.SHARED_PUBKY_SOURCE.name)).thenReturn("app.pubkyring:$RING_PUBKY") + val sdk = mock() + val activationError = IllegalStateException("activation failed") + whenever(sdk.initialize()).thenThrow(activationError) + val access = mock() + val noise = mock() + whenever(noise.exportBytes()).thenReturn(ByteArray(32) { 1 }) + whenever(access.exportSessionSecret()).thenReturn("new-session") + whenever(access.exportLocalSecretKey()).thenReturn(mock()) + whenever(access.exportReceiverNoiseSecretKey()).thenReturn(noise) + val store = mock() + whenever(store.data).thenReturn(flowOf(PubkyStoreData())) + val service = PaykitSdkService(mock(), keychain, store) { sdk } + + val thrown = assertFailsWith { + service.activateSignedIn(PubkySessionBootstrapResult(access, "pubky_test"), previousPublicKey = null) + } + + assertEquals(activationError, thrown) + verify(keychain).upsertString(Keychain.Key.PAYKIT_SESSION.name, "new-session") + verify(blocking).delete(Keychain.Key.PAYKIT_SESSION.name) + verify(blocking).delete(Keychain.Key.SHARED_PUBKY_SOURCE.name) + } + + @Test + fun `sign in for a local identity keeps its credentials when activation fails`() = runTest { + val keychain = mock() + val blocking = mock() + whenever(keychain.accessBlocking(any())).doAnswer { + it.getArgument Any?>(0).invoke(blocking) + } + whenever(blocking.load(Keychain.Key.PAYKIT_RECEIVER_NOISE_SECRET_KEY.name)).thenReturn(ByteArray(32) { 1 }) + val sdk = mock() + val activationError = IllegalStateException("activation failed") + whenever(sdk.initialize()).thenThrow(activationError) + val access = mock() + val noise = mock() + whenever(noise.exportBytes()).thenReturn(ByteArray(32) { 1 }) + whenever(access.exportSessionSecret()).thenReturn("new-session") + val secret = mock() + whenever(secret.exportBytes()).thenReturn(ByteArray(32) { 1 }) + whenever(access.exportLocalSecretKey()).thenReturn(secret) + whenever(access.exportReceiverNoiseSecretKey()).thenReturn(noise) + val store = mock() + whenever(store.data).thenReturn(flowOf(PubkyStoreData())) + val service = PaykitSdkService(mock(), keychain, store) { sdk } + + val thrown = assertFailsWith { + service.activateSignedIn(PubkySessionBootstrapResult(access, "pubky_test"), previousPublicKey = null) + } + + assertEquals(activationError, thrown) + verify(keychain).upsertString(Keychain.Key.PAYKIT_SESSION.name, "new-session") + verify(blocking, never()).delete(any()) + } + @Test fun `identity lookup failure preserves stored state and stops activation`() = runTest { for (error in listOf( diff --git a/changelog.d/next/1363.fixed.md b/changelog.d/next/1363.fixed.md new file mode 100644 index 0000000000..4ebee7bfac --- /dev/null +++ b/changelog.d/next/1363.fixed.md @@ -0,0 +1 @@ +Backing out of Pubky Choice while picking a Ring pubky, or a failed sign-in, no longer leaves a hidden signed-in session behind. From dab5dbc9b376e169d3a103197c6a5b89a2ae7c95 Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Wed, 30 Sep 2026 17:08:54 +0200 Subject: [PATCH 2/4] fix: keep ring pick cleanup inside the pick --- .../java/to/bitkit/repositories/PubkyRepo.kt | 38 +++++----- .../to/bitkit/services/PaykitSdkService.kt | 58 +++++++-------- .../to/bitkit/repositories/PubkyRepoTest.kt | 71 +++++++++++++++++++ .../bitkit/services/PaykitSdkServiceTest.kt | 64 ----------------- 4 files changed, 116 insertions(+), 115 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index d96bcab809..ad08c711fe 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -95,6 +95,7 @@ class PubkyRepo @Inject constructor( private val loadProfileMutex = Mutex() private val loadContactsMutex = Mutex() private val adoptedSourceCheckMutex = Mutex() + private var adoptionGeneration = 0L private var isServiceInitialized = false private val _profile = MutableStateFlow(null) @@ -344,11 +345,13 @@ class PubkyRepo @Inject constructor( suspend fun adoptRingIdentity(pubky: String): Result = withContext(ioDispatcher) { val reference = "${SharedPubkyContract.RING_SOURCE_PREFIX}$pubky" + var generation = 0L var sessionInstalled = false var completed = false try { runSuspendCatching { val publicKey = initializeMutex.withLock { + generation = ++adoptionGeneration ensureServiceInitialized() val secretKeyHex = sharedPubkyClient.ringCredential(pubky).getOrThrow() val rawPublicKey = pubkyService.publicKeyFromSecret(secretKeyHex) @@ -380,20 +383,17 @@ class PubkyRepo @Inject constructor( } }.onFailure { clearAdoptedSourceIfMatches(reference) } } catch (error: CancellationException) { - if (!completed) rollBackInterruptedAdoption(reference, sessionInstalled) + if (!completed) rollBackInterruptedAdoption(reference, generation, sessionInstalled) throw error } } - /** An interrupted pick ends signed out, with no session and no Ring reference left behind. */ - private suspend fun rollBackInterruptedAdoption(reference: String, sessionInstalled: Boolean) { + private suspend fun rollBackInterruptedAdoption(reference: String, generation: Long, sessionInstalled: Boolean) { withContext(NonCancellable + ioDispatcher) { initializeMutex.withLock { + if (generation != adoptionGeneration) return@withLock if (keychain.loadString(Keychain.Key.SHARED_PUBKY_SOURCE.name) != reference) return@withLock - if (sessionInstalled) { - discardAbandonedSession() - clearLocalState() - } + if (sessionInstalled && !discardAbandonedSession()) clearLocalState() clearAdoptedSourceIfMatches(reference) } } @@ -415,7 +415,7 @@ class PubkyRepo @Inject constructor( val hasIdentityRecord = runSuspendCatching { pubkyService.hasIdentityRecord(publicKey) } .onFailure { Logger.warn("Failed to check ring identity record", it, context = TAG) } .getOrNull() - if (hasIdentityRecord != false) throw it + if (hasIdentityRecord != false || hasNewSession(previousSession)) 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) @@ -424,18 +424,19 @@ class PubkyRepo @Inject constructor( } finally { if (!completed) { withContext(NonCancellable + ioDispatcher) { - val installedSession = runSuspendCatching { - val currentSession = keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) - currentSession != null && currentSession != previousSession - }.onFailure { - Logger.warn("Failed to identify incomplete adopted Pubky session", it, context = TAG) - }.getOrDefault(false) - if (installedSession) discardAbandonedSession() + if (hasNewSession(previousSession)) discardAbandonedSession() } } } } + private suspend fun hasNewSession(previousSession: String?): Boolean = runSuspendCatching { + val currentSession = keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) + currentSession != null && currentSession != previousSession + }.onFailure { + Logger.warn("Failed to identify incomplete adopted Pubky session", it, context = TAG) + }.getOrDefault(false) + private suspend fun clearProfileIfIdentityChanged(publicKey: String) { if (_publicKey.value == publicKey) return _contactsLoadVersion.update { 0L } @@ -630,14 +631,15 @@ class PubkyRepo @Inject constructor( discardAbandonedSession() } - private suspend fun discardAbandonedSession() { + private suspend fun discardAbandonedSession(): Boolean { val revocationError = runSuspendCatching { withContext(NonCancellable + ioDispatcher) { pubkyService.signOut() } - }.exceptionOrNull() ?: return + }.exceptionOrNull() ?: return false Logger.warn("Failed to revoke abandoned Pubky session", revocationError, context = TAG) + var clearedLocalState = false runSuspendCatching { withContext(NonCancellable + ioDispatcher) { pubkyService.forgetSessionAccess() @@ -647,7 +649,9 @@ class PubkyRepo @Inject constructor( withContext(NonCancellable + ioDispatcher) { clearLocalState(publicPaykitCleanupPending = true) } + clearedLocalState = true } + return clearedLocalState } suspend fun uploadAvatar(imageBytes: ByteArray): Result = runSuspendCatching { diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt index 45e4f83a76..c2a5fde8fa 100644 --- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt +++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt @@ -360,7 +360,13 @@ class PaykitSdkService @Inject constructor( signupCode = signupCode, requiredCapabilities = requiredCapabilities(), ) - activateSignedIn(result, previousPublicKey) + operationMutex.withLock { + activateBootstrapResult( + result = result, + previousPublicKey = previousPublicKey, + ) + } + notifyBackupStateChanged() return result } @@ -383,14 +389,16 @@ class PaykitSdkService @Inject constructor( isSetup.await() val previousPublicKey = operationMutex.withLock { currentSdkStatePublicKeyLocked() } operationMutex.withLock { - activateOrClearLocked(result, previousPublicKey, onlyAdoptedRingIdentity = false) - } - notifyBackupStateChanged() - } - - internal suspend fun activateSignedIn(result: PubkySessionBootstrapResult, previousPublicKey: String?) { - operationMutex.withLock { - activateOrClearLocked(result, previousPublicKey, onlyAdoptedRingIdentity = true) + var activated = false + try { + activateBootstrapResult( + result = result, + previousPublicKey = previousPublicKey, + ) + activated = true + } finally { + if (!activated) clearRegisteredIdentityActivationLocked() + } } notifyBackupStateChanged() } @@ -403,7 +411,13 @@ class PaykitSdkService @Inject constructor( receiverNoiseSecretKey = sessionProvider.loadOrDeriveReceiverNoiseSecretKey(), requiredCapabilities = requiredCapabilities(), ) - activateSignedIn(result, previousPublicKey) + operationMutex.withLock { + activateBootstrapResult( + result = result, + previousPublicKey = previousPublicKey, + ) + } + notifyBackupStateChanged() return result } @@ -981,30 +995,6 @@ class PaykitSdkService @Inject constructor( republishIdentityIfNeeded(publicKey = result.publicKey) } - /** - * Activation saves the session before it initializes the SDK, so a failure leaves a session behind. - * An adopted Ring identity has no local secret key to sign in again with, so its session is cleared - * together with the Ring reference. Local identities keep their credentials for the sign-in retry. - */ - private suspend fun activateOrClearLocked( - result: PubkySessionBootstrapResult, - previousPublicKey: String?, - onlyAdoptedRingIdentity: Boolean, - ) { - var activated = false - try { - activateBootstrapResult( - result = result, - previousPublicKey = previousPublicKey, - ) - activated = true - } finally { - if (!activated && (!onlyAdoptedRingIdentity || sessionProvider.adoptedPubky() != null)) { - clearRegisteredIdentityActivationLocked() - } - } - } - private suspend fun clearRegisteredIdentityActivationLocked() = withContext(NonCancellable) { runSuspendCatching { sessionProvider.clearSessionAccess() } .onFailure { Logger.warn("Failed to clear incomplete Pubky signup session", it, context = TAG) } diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index cd70be632e..9fa04c4c92 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -1176,6 +1176,39 @@ class PubkyRepoTest : BaseUnitTest() { verifyBlocking(pubkyService, never()) { signUp(any(), any(), any()) } } + @Test + fun `adoptRingIdentity should not sign up when sign in saved a session before failing`() = test { + val httpClient = identityHttpClient() + sut = createSut(httpClient) + val ringPubky = stubRingCredential() + val reference = SharedPubkyContract.RING_SOURCE_PREFIX + ringPubky + var session: String? = null + var sourceAtSignOut: String? = null + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenAnswer { session } + whenever(pubkyService.signIn("ring_secret")).thenAnswer { + session = "installed_session" + throw TestAppError("Activation failed") + } + whenever(pubkyService.hasIdentityRecord(ringPubky)).thenReturn(false) + whenever(pubkyService.signUp("ring_secret", "test-homeserver", "test-code")).thenReturn(Unit) + whenever(pubkyService.signOut()).thenAnswer { + sourceAtSignOut = adoptedSource + session = null + Unit + } + + val result = sut.adoptRingIdentity(ringPubky) + httpClient.close() + + assertTrue(result.isFailure) + assertNull(session) + assertNull(adoptedSource) + assertEquals(reference, sourceAtSignOut) + assertNull(sut.publicKey.value) + verifyBlocking(pubkyService) { signOut() } + verifyBlocking(pubkyService, never()) { signUp(any(), any(), any()) } + } + @Test fun `wipe completes while adopted identity profile loading remains in flight`() = test { sut.awaitInitialization() @@ -1280,6 +1313,44 @@ class PubkyRepoTest : BaseUnitTest() { verifyBlocking(pubkyService) { forgetSessionAccess() } } + @Test + fun `canceling an older pick keeps a newer pick of the same pubky`() = test { + sut.awaitInitialization() + val ringPubky = stubRingCredential() + val reference = SharedPubkyContract.RING_SOURCE_PREFIX + ringPubky + var session: String? = null + var signIns = 0 + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenAnswer { session } + val secondSignedIn = CompletableDeferred() + whenever(pubkyService.signIn("ring_secret")).thenAnswer { + session = "session_${++signIns}" + if (signIns == 2) secondSignedIn.complete(Unit) + Unit + } + val firstProfileLoadStarted = CompletableDeferred() + var profileLoads = 0 + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).doSuspendableAnswer { + if (++profileLoads == 1) { + firstProfileLoadStarted.complete(Unit) + awaitCancellation() + } + createResolution(VALID_SELF_KEY, pubkyProfile = createPubkyProfile()) + } + val firstPick = async { sut.adoptRingIdentity(ringPubky) } + firstProfileLoadStarted.await() + + val secondPick = async { sut.adoptRingIdentity(ringPubky) } + secondSignedIn.await() + + firstPick.cancelAndJoin() + + assertTrue(secondPick.await().isSuccess) + assertEquals("session_2", session) + assertEquals(reference, adoptedSource) + assertEquals(VALID_SELF_KEY, sut.publicKey.value) + verifyBlocking(pubkyService, never()) { signOut() } + } + @Test fun `adopted identity loads after previous identity loads finish`() = test { val oldPublicKey = VALID_SELF_KEY diff --git a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt index 138a2b6069..9ddf826083 100644 --- a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt +++ b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt @@ -106,70 +106,6 @@ class PaykitSdkServiceTest { } } - @Test - fun `sign in for an adopted ring identity clears its session and ring reference when activation fails`() = runTest { - val keychain = mock() - val blocking = mock() - whenever(keychain.accessBlocking(any())).doAnswer { - it.getArgument Any?>(0).invoke(blocking) - } - whenever(blocking.load(Keychain.Key.PAYKIT_RECEIVER_NOISE_SECRET_KEY.name)).thenReturn(ByteArray(32) { 1 }) - whenever(keychain.loadString(Keychain.Key.SHARED_PUBKY_SOURCE.name)).thenReturn("app.pubkyring:$RING_PUBKY") - val sdk = mock() - val activationError = IllegalStateException("activation failed") - whenever(sdk.initialize()).thenThrow(activationError) - val access = mock() - val noise = mock() - whenever(noise.exportBytes()).thenReturn(ByteArray(32) { 1 }) - whenever(access.exportSessionSecret()).thenReturn("new-session") - whenever(access.exportLocalSecretKey()).thenReturn(mock()) - whenever(access.exportReceiverNoiseSecretKey()).thenReturn(noise) - val store = mock() - whenever(store.data).thenReturn(flowOf(PubkyStoreData())) - val service = PaykitSdkService(mock(), keychain, store) { sdk } - - val thrown = assertFailsWith { - service.activateSignedIn(PubkySessionBootstrapResult(access, "pubky_test"), previousPublicKey = null) - } - - assertEquals(activationError, thrown) - verify(keychain).upsertString(Keychain.Key.PAYKIT_SESSION.name, "new-session") - verify(blocking).delete(Keychain.Key.PAYKIT_SESSION.name) - verify(blocking).delete(Keychain.Key.SHARED_PUBKY_SOURCE.name) - } - - @Test - fun `sign in for a local identity keeps its credentials when activation fails`() = runTest { - val keychain = mock() - val blocking = mock() - whenever(keychain.accessBlocking(any())).doAnswer { - it.getArgument Any?>(0).invoke(blocking) - } - whenever(blocking.load(Keychain.Key.PAYKIT_RECEIVER_NOISE_SECRET_KEY.name)).thenReturn(ByteArray(32) { 1 }) - val sdk = mock() - val activationError = IllegalStateException("activation failed") - whenever(sdk.initialize()).thenThrow(activationError) - val access = mock() - val noise = mock() - whenever(noise.exportBytes()).thenReturn(ByteArray(32) { 1 }) - whenever(access.exportSessionSecret()).thenReturn("new-session") - val secret = mock() - whenever(secret.exportBytes()).thenReturn(ByteArray(32) { 1 }) - whenever(access.exportLocalSecretKey()).thenReturn(secret) - whenever(access.exportReceiverNoiseSecretKey()).thenReturn(noise) - val store = mock() - whenever(store.data).thenReturn(flowOf(PubkyStoreData())) - val service = PaykitSdkService(mock(), keychain, store) { sdk } - - val thrown = assertFailsWith { - service.activateSignedIn(PubkySessionBootstrapResult(access, "pubky_test"), previousPublicKey = null) - } - - assertEquals(activationError, thrown) - verify(keychain).upsertString(Keychain.Key.PAYKIT_SESSION.name, "new-session") - verify(blocking, never()).delete(any()) - } - @Test fun `identity lookup failure preserves stored state and stops activation`() = runTest { for (error in listOf( From e48e27772240f5e0b7f4247a524908dd3ab9e98d Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Wed, 30 Sep 2026 18:13:05 +0200 Subject: [PATCH 3/4] fix: roll back a ring pick after another pick fails early --- .../java/to/bitkit/repositories/PubkyRepo.kt | 2 +- .../to/bitkit/repositories/PubkyRepoTest.kt | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index ad08c711fe..9546fa949c 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -351,13 +351,13 @@ class PubkyRepo @Inject constructor( try { runSuspendCatching { val publicKey = initializeMutex.withLock { - generation = ++adoptionGeneration ensureServiceInitialized() val secretKeyHex = sharedPubkyClient.ringCredential(pubky).getOrThrow() val rawPublicKey = pubkyService.publicKeyFromSecret(secretKeyHex) require(PubkyPublicKeyFormat.matches(rawPublicKey, pubky)) { "Ring credential does not match '${redacted(pubky)}'" } + generation = ++adoptionGeneration keychain.upsertString(Keychain.Key.SHARED_PUBKY_SOURCE.name, reference) signInOrSignUpAdoptedIdentity(secretKeyHex, rawPublicKey) sessionInstalled = true diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index 9fa04c4c92..026f8ff22a 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -1351,6 +1351,40 @@ class PubkyRepoTest : BaseUnitTest() { verifyBlocking(pubkyService, never()) { signOut() } } + @Test + fun `canceling a pick after a failed pick of another pubky signs out the installed session`() = test { + sut.awaitInitialization() + val ringPubky = stubRingCredential() + val otherPubky = VALID_CONTACT_KEY_A.removePrefix("pubky") + whenever(sharedPubkyClient.ringCredential(otherPubky)).thenReturn(Result.failure(TestAppError("Denied"))) + var session: String? = null + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenAnswer { session } + whenever(pubkyService.signIn("ring_secret")).thenAnswer { + session = "installed_session" + Unit + } + whenever(pubkyService.signOut()).thenAnswer { + session = null + adoptedSource = null + Unit + } + val profileLoadStarted = CompletableDeferred() + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).doSuspendableAnswer { + profileLoadStarted.complete(Unit) + awaitCancellation() + } + val firstPick = async { sut.adoptRingIdentity(ringPubky) } + profileLoadStarted.await() + + assertTrue(sut.adoptRingIdentity(otherPubky).isFailure) + firstPick.cancelAndJoin() + + assertNull(session) + assertNull(adoptedSource) + assertNull(sut.publicKey.value) + verifyBlocking(pubkyService) { signOut() } + } + @Test fun `adopted identity loads after previous identity loads finish`() = test { val oldPublicKey = VALID_SELF_KEY From effae808686b32597ca6a5f0265c88c6271625f6 Mon Sep 17 00:00:00 2001 From: Ovi Trif Date: Wed, 30 Sep 2026 18:31:14 +0200 Subject: [PATCH 4/4] fix: run ring picks one at a time --- .../java/to/bitkit/repositories/PubkyRepo.kt | 79 +++++++++---------- .../to/bitkit/repositories/PubkyRepoTest.kt | 53 +++++++++++-- 2 files changed, 86 insertions(+), 46 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index 9546fa949c..3eb24104d9 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -95,7 +95,7 @@ class PubkyRepo @Inject constructor( private val loadProfileMutex = Mutex() private val loadContactsMutex = Mutex() private val adoptedSourceCheckMutex = Mutex() - private var adoptionGeneration = 0L + private val adoptionMutex = Mutex() private var isServiceInitialized = false private val _profile = MutableStateFlow(null) @@ -345,53 +345,52 @@ class PubkyRepo @Inject constructor( suspend fun adoptRingIdentity(pubky: String): Result = withContext(ioDispatcher) { val reference = "${SharedPubkyContract.RING_SOURCE_PREFIX}$pubky" - var generation = 0L - var sessionInstalled = false - var completed = false - try { - runSuspendCatching { - val publicKey = initializeMutex.withLock { - ensureServiceInitialized() - val secretKeyHex = sharedPubkyClient.ringCredential(pubky).getOrThrow() - val rawPublicKey = pubkyService.publicKeyFromSecret(secretKeyHex) - require(PubkyPublicKeyFormat.matches(rawPublicKey, pubky)) { - "Ring credential does not match '${redacted(pubky)}'" + adoptionMutex.withLock { + var sessionInstalled = false + var completed = false + try { + runSuspendCatching { + val publicKey = initializeMutex.withLock { + ensureServiceInitialized() + val secretKeyHex = sharedPubkyClient.ringCredential(pubky).getOrThrow() + val rawPublicKey = pubkyService.publicKeyFromSecret(secretKeyHex) + require(PubkyPublicKeyFormat.matches(rawPublicKey, pubky)) { + "Ring credential does not match '${redacted(pubky)}'" + } + keychain.upsertString(Keychain.Key.SHARED_PUBKY_SOURCE.name, reference) + signInOrSignUpAdoptedIdentity(secretKeyHex, rawPublicKey) + sessionInstalled = true + + val prefixedPublicKey = rawPublicKey.ensurePubkyPrefix() + clearProfileIfIdentityChanged(prefixedPublicKey) + _publicKey.update { prefixedPublicKey } + notifyBackupStateChanged() + Logger.info("Adopted ring identity for '${redacted(rawPublicKey)}'", context = TAG) + prefixedPublicKey } - generation = ++adoptionGeneration - keychain.upsertString(Keychain.Key.SHARED_PUBKY_SOURCE.name, reference) - signInOrSignUpAdoptedIdentity(secretKeyHex, rawPublicKey) - sessionInstalled = true - - val prefixedPublicKey = rawPublicKey.ensurePubkyPrefix() - clearProfileIfIdentityChanged(prefixedPublicKey) - _publicKey.update { prefixedPublicKey } - notifyBackupStateChanged() - Logger.info("Adopted ring identity for '${redacted(rawPublicKey)}'", context = TAG) - prefixedPublicKey - } - loadProfile() - loadContacts() + loadProfile() + loadContacts() - initializeMutex.withLock { - check(_publicKey.value == publicKey) { "Adopted Pubky identity changed before setup completed" } - val hasProfile = _profile.value?.publicKey == publicKey - runSuspendCatching { settingsStore.setPubkyProfileSetupPending(!hasProfile) } - .onFailure { Logger.warn("Failed to save pending profile setup", it, context = TAG) } - completed = true - hasProfile - } - }.onFailure { clearAdoptedSourceIfMatches(reference) } - } catch (error: CancellationException) { - if (!completed) rollBackInterruptedAdoption(reference, generation, sessionInstalled) - throw error + initializeMutex.withLock { + check(_publicKey.value == publicKey) { "Adopted Pubky identity changed before setup completed" } + val hasProfile = _profile.value?.publicKey == publicKey + runSuspendCatching { settingsStore.setPubkyProfileSetupPending(!hasProfile) } + .onFailure { Logger.warn("Failed to save pending profile setup", it, context = TAG) } + completed = true + hasProfile + } + }.onFailure { clearAdoptedSourceIfMatches(reference) } + } catch (error: CancellationException) { + if (!completed) rollBackInterruptedAdoption(reference, sessionInstalled) + throw error + } } } - private suspend fun rollBackInterruptedAdoption(reference: String, generation: Long, sessionInstalled: Boolean) { + private suspend fun rollBackInterruptedAdoption(reference: String, sessionInstalled: Boolean) { withContext(NonCancellable + ioDispatcher) { initializeMutex.withLock { - if (generation != adoptionGeneration) return@withLock if (keychain.loadString(Keychain.Key.SHARED_PUBKY_SOURCE.name) != reference) return@withLock if (sessionInstalled && !discardAbandonedSession()) clearLocalState() clearAdoptedSourceIfMatches(reference) diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index 026f8ff22a..6943536484 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -1321,10 +1321,13 @@ class PubkyRepoTest : BaseUnitTest() { var session: String? = null var signIns = 0 whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenAnswer { session } - val secondSignedIn = CompletableDeferred() whenever(pubkyService.signIn("ring_secret")).thenAnswer { session = "session_${++signIns}" - if (signIns == 2) secondSignedIn.complete(Unit) + Unit + } + whenever(pubkyService.signOut()).thenAnswer { + session = null + adoptedSource = null Unit } val firstProfileLoadStarted = CompletableDeferred() @@ -1338,17 +1341,17 @@ class PubkyRepoTest : BaseUnitTest() { } val firstPick = async { sut.adoptRingIdentity(ringPubky) } firstProfileLoadStarted.await() - val secondPick = async { sut.adoptRingIdentity(ringPubky) } - secondSignedIn.await() + assertEquals(1, signIns) firstPick.cancelAndJoin() assertTrue(secondPick.await().isSuccess) + assertEquals(2, signIns) assertEquals("session_2", session) assertEquals(reference, adoptedSource) assertEquals(VALID_SELF_KEY, sut.publicKey.value) - verifyBlocking(pubkyService, never()) { signOut() } + verifyBlocking(pubkyService) { signOut() } } @Test @@ -1375,10 +1378,48 @@ class PubkyRepoTest : BaseUnitTest() { } val firstPick = async { sut.adoptRingIdentity(ringPubky) } profileLoadStarted.await() + val secondPick = async { sut.adoptRingIdentity(otherPubky) } + + firstPick.cancelAndJoin() + + assertTrue(secondPick.await().isFailure) + assertNull(session) + assertNull(adoptedSource) + assertNull(sut.publicKey.value) + verifyBlocking(pubkyService) { signOut() } + } + + @Test + fun `canceling a pick while another pubky fails to sign in leaves no session or ring reference`() = test { + sut.awaitInitialization() + val ringPubky = stubRingCredential() + val otherPubky = VALID_CONTACT_KEY_A.removePrefix("pubky") + whenever(sharedPubkyClient.ringCredential(otherPubky)).thenReturn(Result.success("other_secret")) + whenever(pubkyService.publicKeyFromSecret("other_secret")).thenReturn(otherPubky) + whenever(pubkyService.signIn("other_secret")).thenAnswer { throw TestAppError("Relay unavailable") } + whenever(pubkyService.hasIdentityRecord(otherPubky)).thenReturn(true) + var session: String? = null + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenAnswer { session } + whenever(pubkyService.signIn("ring_secret")).thenAnswer { + session = "installed_session" + Unit + } + whenever(pubkyService.signOut()).thenAnswer { + session = null + Unit + } + val profileLoadStarted = CompletableDeferred() + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).doSuspendableAnswer { + profileLoadStarted.complete(Unit) + awaitCancellation() + } + val firstPick = async { sut.adoptRingIdentity(ringPubky) } + profileLoadStarted.await() + val secondPick = async { sut.adoptRingIdentity(otherPubky) } - assertTrue(sut.adoptRingIdentity(otherPubky).isFailure) firstPick.cancelAndJoin() + assertTrue(secondPick.await().isFailure) assertNull(session) assertNull(adoptedSource) assertNull(sut.publicKey.value)