diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index 9eac4b0a2b..3eb24104d9 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 val adoptionMutex = Mutex() private var isServiceInitialized = false private val _profile = MutableStateFlow(null) @@ -344,42 +345,56 @@ class PubkyRepo @Inject constructor( suspend fun adoptRingIdentity(pubky: String): Result = withContext(ioDispatcher) { val reference = "${SharedPubkyContract.RING_SOURCE_PREFIX}$pubky" - var identityInstalled = 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 } - keychain.upsertString(Keychain.Key.SHARED_PUBKY_SOURCE.name, reference) - signInOrSignUpAdoptedIdentity(secretKeyHex, rawPublicKey) - - val prefixedPublicKey = rawPublicKey.ensurePubkyPrefix() - clearProfileIfIdentityChanged(prefixedPublicKey) - _publicKey.update { prefixedPublicKey } - identityInstalled = true - 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) } - hasProfile - } - }.onFailure { clearAdoptedSourceIfMatches(reference) } - } catch (error: CancellationException) { - if (!identityInstalled) clearAdoptedSourceIfMatches(reference) - 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, 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) + } } } @@ -399,7 +414,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) @@ -408,18 +423,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 } @@ -614,14 +630,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() @@ -631,7 +648,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/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index 7e5d5e9810..6943536484 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() @@ -1209,17 +1242,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 +1264,166 @@ class PubkyRepoTest : BaseUnitTest() { } val adoption = async { sut.adoptRingIdentity(ringPubky) } profileLoadStarted.await() + assertEquals(reference, adoptedSource) + assertEquals(VALID_SELF_KEY, sut.publicKey.value) adoption.cancelAndJoin() + 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 + 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 } + whenever(pubkyService.signIn("ring_secret")).thenAnswer { + session = "session_${++signIns}" + Unit + } + whenever(pubkyService.signOut()).thenAnswer { + session = null + adoptedSource = null + 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) } + 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) - assertEquals(reference, source) - verifyBlocking(pubkyService, never()) { signOut() } + verifyBlocking(pubkyService) { 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() + 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) } + + firstPick.cancelAndJoin() + + assertTrue(secondPick.await().isFailure) + assertNull(session) + assertNull(adoptedSource) + assertNull(sut.publicKey.value) + verifyBlocking(pubkyService) { signOut() } } @Test 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.