Skip to content

Commit 4033b1e

Browse files
ben-kaufmanben-kaufman
committed
fix: harden pubky restore
1 parent 4fabede commit 4033b1e

3 files changed

Lines changed: 84 additions & 36 deletions

File tree

‎app/src/main/java/to/bitkit/repositories/PubkyRepo.kt‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,8 @@ class PubkyRepo @Inject constructor(
190190
if (storedSecretKeyHex.isNullOrEmpty()) {
191191
if (!savedSessionSecret.isNullOrEmpty()) {
192192
Logger.warn("Skipped re-sign-in recovery, no secret key available", context = TAG)
193+
runCatching { keychain.delete(Keychain.Key.PAYKIT_SESSION.name) }
194+
notifyBackupStateChanged()
193195
InitResult.RestorationFailed
194196
} else {
195197
InitResult.NoSession
@@ -755,27 +757,33 @@ class PubkyRepo @Inject constructor(
755757

756758
suspend fun restoreSessionBackupState(backup: PubkySessionBackupV1?): Result<Unit> = runCatching {
757759
withContext(ioDispatcher) {
758-
pubkyService.forceSignOut()
759-
clearAuthenticatedState()
760-
runCatching { keychain.delete(Keychain.Key.PAYKIT_SESSION.name) }
761-
runCatching { keychain.delete(Keychain.Key.PUBKY_SECRET_KEY.name) }
762-
763-
when (backup?.kind) {
764-
null -> Unit
765-
PubkySessionBackupKind.LocalSeed -> {
766-
val secretKeyHex = deriveLocalSecretKeyFromWalletSeed()
767-
keychain.upsertString(Keychain.Key.PUBKY_SECRET_KEY.name, secretKeyHex)
760+
initializeMutex.withLock {
761+
if (backup == null) {
762+
notifyBackupStateChanged()
763+
return@withLock
768764
}
769765

770-
PubkySessionBackupKind.ExternalSession -> {
771-
val sessionSecret = requireNotNull(backup.sessionSecret?.takeIf { it.isNotBlank() }) {
772-
"Missing session secret in backup"
766+
pubkyService.forceSignOut()
767+
clearAuthenticatedState()
768+
runCatching { keychain.delete(Keychain.Key.PAYKIT_SESSION.name) }
769+
runCatching { keychain.delete(Keychain.Key.PUBKY_SECRET_KEY.name) }
770+
771+
when (backup.kind) {
772+
PubkySessionBackupKind.LocalSeed -> {
773+
val secretKeyHex = deriveLocalSecretKeyFromWalletSeed()
774+
keychain.upsertString(Keychain.Key.PUBKY_SECRET_KEY.name, secretKeyHex)
775+
}
776+
777+
PubkySessionBackupKind.ExternalSession -> {
778+
val sessionSecret = requireNotNull(backup.sessionSecret?.takeIf { it.isNotBlank() }) {
779+
"Missing session secret in backup"
780+
}
781+
keychain.upsertString(Keychain.Key.PAYKIT_SESSION.name, sessionSecret)
773782
}
774-
keychain.upsertString(Keychain.Key.PAYKIT_SESSION.name, sessionSecret)
775783
}
776-
}
777784

778-
notifyBackupStateChanged()
785+
notifyBackupStateChanged()
786+
}
779787
}
780788
}
781789

‎app/src/main/java/to/bitkit/ui/screens/contacts/AddContactScreen.kt‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ private fun AddContactSheetContent(
163163
trailingIcon = {
164164
IconButton(
165165
onClick = onPaste,
166-
modifier = Modifier.testTag("AddContactPaste"),
166+
modifier = Modifier.testTag("AddContactPaste")
167167
) {
168168
Icon(
169169
painter = painterResource(R.drawable.ic_clipboard_text),
@@ -174,7 +174,7 @@ private fun AddContactSheetContent(
174174
},
175175
modifier = Modifier
176176
.fillMaxWidth()
177-
.testTag("AddContactPubkyField"),
177+
.testTag("AddContactPubkyField")
178178
)
179179
VerticalSpacer(16.dp)
180180

@@ -187,15 +187,15 @@ private fun AddContactSheetContent(
187187
onClick = onScanQr,
188188
modifier = Modifier
189189
.weight(1f)
190-
.testTag("AddContactScanQR"),
190+
.testTag("AddContactScanQR")
191191
)
192192
PrimaryButton(
193193
text = stringResource(R.string.contacts__add_button),
194194
onClick = onSubmit,
195195
enabled = isSubmitEnabled,
196196
modifier = Modifier
197197
.weight(1f)
198-
.testTag("AddContactAdd"),
198+
.testTag("AddContactAdd")
199199
)
200200
}
201201
VerticalSpacer(16.dp)
@@ -418,7 +418,7 @@ private fun ErrorContent(
418418
SecondaryButton(
419419
text = stringResource(R.string.common__retry),
420420
onClick = onRetry,
421-
modifier = Modifier.testTag("AddContactRetry"),
421+
modifier = Modifier.testTag("AddContactRetry")
422422
)
423423
}
424424
}
@@ -462,15 +462,15 @@ private fun LoadedContent(
462462
onClick = onDiscard,
463463
modifier = Modifier
464464
.weight(1f)
465-
.testTag("AddContactDiscard"),
465+
.testTag("AddContactDiscard")
466466
)
467467
PrimaryButton(
468468
text = stringResource(R.string.common__save),
469469
onClick = onSave,
470470
enabled = !isLoading,
471471
modifier = Modifier
472472
.weight(1f)
473-
.testTag("AddContactSave"),
473+
.testTag("AddContactSave")
474474
)
475475
}
476476
VerticalSpacer(16.dp)

‎app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt‎

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import to.bitkit.models.PubkySessionBackupKind
2626
import to.bitkit.models.PubkySessionBackupV1
2727
import to.bitkit.services.PubkyService
2828
import to.bitkit.test.BaseUnitTest
29+
import to.bitkit.utils.AppError
2930
import kotlin.test.assertEquals
3031
import kotlin.test.assertFalse
3132
import kotlin.test.assertNotNull
@@ -84,7 +85,7 @@ class PubkyRepoTest : BaseUnitTest() {
8485

8586
@Test
8687
fun `startAuthentication should reset state on failure`() = test {
87-
whenever(pubkyService.startAuth()).thenThrow(RuntimeException("Auth failed"))
88+
whenever(pubkyService.startAuth()).thenAnswer { throw TestAppError("Auth failed") }
8889

8990
val result = sut.startAuthentication()
9091

@@ -145,7 +146,7 @@ class PubkyRepoTest : BaseUnitTest() {
145146

146147
@Test
147148
fun `completeAuthentication should reset state on failure`() = test {
148-
whenever(pubkyService.completeAuth()).thenThrow(RuntimeException("Failed"))
149+
whenever(pubkyService.completeAuth()).thenAnswer { throw TestAppError("Failed") }
149150

150151
val result = sut.completeAuthentication()
151152

@@ -193,7 +194,7 @@ class PubkyRepoTest : BaseUnitTest() {
193194
assertNotNull(existingProfile)
194195

195196
val pk = checkNotNull(sut.publicKey.value) { "publicKey should be set after authentication" }
196-
whenever(pubkyService.getProfile(pk)).thenThrow(RuntimeException("Network error"))
197+
whenever(pubkyService.getProfile(pk)).thenAnswer { throw TestAppError("Network error") }
197198

198199
sut.loadProfile()
199200

@@ -266,8 +267,8 @@ class PubkyRepoTest : BaseUnitTest() {
266267
fun `deleteProfile should fail when signOut fails`() = test {
267268
authenticateForTesting()
268269
whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn("test_secret")
269-
whenever(pubkyService.signOut()).thenThrow(RuntimeException("Sign out failed"))
270-
whenever(pubkyService.forceSignOut()).thenThrow(RuntimeException("Force sign out failed"))
270+
whenever(pubkyService.signOut()).thenAnswer { throw TestAppError("Sign out failed") }
271+
whenever(pubkyService.forceSignOut()).thenAnswer { throw TestAppError("Force sign out failed") }
271272

272273
val result = sut.deleteProfile()
273274

@@ -284,7 +285,11 @@ class PubkyRepoTest : BaseUnitTest() {
284285
whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(secretKey)
285286
whenever(pubkyService.sessionList(expiredSession, Env.contactsBasePath)).thenReturn(emptyList())
286287
whenever(pubkyService.sessionList(newSession, Env.contactsBasePath)).thenReturn(emptyList())
287-
whenever(pubkyService.sessionDelete(expiredSession, Env.profilePath)).thenThrow(RuntimeException("Expired"))
288+
whenever(
289+
pubkyService.sessionDelete(expiredSession, Env.profilePath)
290+
).thenAnswer {
291+
throw TestAppError("Expired")
292+
}
288293
whenever(pubkyService.signIn(secretKey)).thenReturn(newSession)
289294
whenever(pubkyService.importSession(newSession)).thenReturn(VALID_SELF_KEY)
290295

@@ -303,7 +308,11 @@ class PubkyRepoTest : BaseUnitTest() {
303308
whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn(expiredSession)
304309
whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(null)
305310
whenever(pubkyService.sessionList(expiredSession, Env.contactsBasePath)).thenReturn(emptyList())
306-
whenever(pubkyService.sessionDelete(expiredSession, Env.profilePath)).thenThrow(RuntimeException("Expired"))
311+
whenever(
312+
pubkyService.sessionDelete(expiredSession, Env.profilePath)
313+
).thenAnswer {
314+
throw TestAppError("Expired")
315+
}
307316

308317
val result = sut.deleteProfileWithSessionRetry()
309318

@@ -315,7 +324,7 @@ class PubkyRepoTest : BaseUnitTest() {
315324
@Test
316325
fun `signOut should force sign out when server sign out fails`() = test {
317326
authenticateForTesting()
318-
whenever(pubkyService.signOut()).thenThrow(RuntimeException("Server error"))
327+
whenever(pubkyService.signOut()).thenAnswer { throw TestAppError("Server error") }
319328

320329
val result = sut.signOut()
321330

@@ -455,6 +464,20 @@ class PubkyRepoTest : BaseUnitTest() {
455464
verifyBlocking(keychain) { upsertString(Keychain.Key.PAYKIT_SESSION.name, session) }
456465
}
457466

467+
@Test
468+
fun `initialize should delete stale saved session when re-sign-in is unavailable`() = test {
469+
val session = "stale_session"
470+
whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn(session)
471+
whenever(keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)).thenReturn(null)
472+
whenever(pubkyService.importSession(session)).thenAnswer { throw TestAppError("Expired") }
473+
474+
sut.initialize()
475+
476+
assertTrue(sut.sessionRestorationFailed.value)
477+
assertFalse(sut.isAuthenticated.value)
478+
verifyBlocking(keychain) { delete(Keychain.Key.PAYKIT_SESSION.name) }
479+
}
480+
458481
@Test
459482
fun `refreshSessionIfPossible should refresh session when local secret key exists`() = test {
460483
val secretKey = "local_secret"
@@ -511,6 +534,21 @@ class PubkyRepoTest : BaseUnitTest() {
511534
verifyBlocking(keychain) { upsertString(Keychain.Key.PAYKIT_SESSION.name, "external_session") }
512535
}
513536

537+
@Test
538+
fun `restoreSessionBackupState should keep current session when backup has no pubky state`() = test {
539+
authenticateForTesting(publicKey = VALID_SELF_KEY)
540+
clearInvocations(pubkyService, keychain)
541+
542+
val result = sut.restoreSessionBackupState(null)
543+
544+
assertTrue(result.isSuccess)
545+
assertTrue(sut.isAuthenticated.value)
546+
assertEquals(VALID_SELF_KEY, sut.publicKey.value)
547+
verifyBlocking(pubkyService, never()) { forceSignOut() }
548+
verifyBlocking(keychain, never()) { delete(Keychain.Key.PAYKIT_SESSION.name) }
549+
verifyBlocking(keychain, never()) { delete(Keychain.Key.PUBKY_SECRET_KEY.name) }
550+
}
551+
514552
@Test
515553
fun `loadContacts should populate contacts on success`() = test {
516554
authenticateForTesting()
@@ -651,7 +689,7 @@ class PubkyRepoTest : BaseUnitTest() {
651689
val pk = checkNotNull(sut.publicKey.value)
652690
val strippedPk = pk.removePrefix("pubky")
653691
whenever(pubkyService.fetchFileString("pubky://$strippedPk${Env.contactsBasePath}$contactKey"))
654-
.thenThrow(RuntimeException("Network error"))
692+
.thenAnswer { throw TestAppError("Network error") }
655693

656694
sut.loadContacts()
657695

@@ -666,7 +704,7 @@ class PubkyRepoTest : BaseUnitTest() {
666704
authenticateForTesting()
667705
whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn("test_secret")
668706
whenever(pubkyService.sessionList("test_secret", Env.contactsBasePath))
669-
.thenThrow(RuntimeException("Directory Not Found (404)"))
707+
.thenAnswer { throw TestAppError("Directory Not Found (404)") }
670708

671709
sut.loadContacts()
672710

@@ -695,7 +733,7 @@ class PubkyRepoTest : BaseUnitTest() {
695733
val strippedKey = contactKey.removePrefix("pubky")
696734
val contactProfile = mock<CorePubkyProfile>()
697735
whenever(pubkyService.fetchFileString("pubky://$strippedKey${Env.profilePath}"))
698-
.thenThrow(RuntimeException("Missing bitkit profile"))
736+
.thenAnswer { throw TestAppError("Missing bitkit profile") }
699737
whenever(contactProfile.name).thenReturn("Bob")
700738
whenever(contactProfile.bio).thenReturn("Bio")
701739
whenever(pubkyService.getProfile(contactKey)).thenReturn(contactProfile)
@@ -711,8 +749,8 @@ class PubkyRepoTest : BaseUnitTest() {
711749
val contactKey = VALID_CONTACT_KEY_A
712750
val strippedKey = contactKey.removePrefix("pubky")
713751
whenever(pubkyService.fetchFileString("pubky://$strippedKey${Env.profilePath}"))
714-
.thenThrow(RuntimeException("Missing bitkit profile"))
715-
whenever(pubkyService.getProfile(contactKey)).thenThrow(RuntimeException("Profile not found"))
752+
.thenAnswer { throw TestAppError("Missing bitkit profile") }
753+
whenever(pubkyService.getProfile(contactKey)).thenAnswer { throw TestAppError("Profile not found") }
716754

717755
val result = sut.fetchContactProfile(contactKey)
718756

@@ -892,5 +930,7 @@ class PubkyRepoTest : BaseUnitTest() {
892930
}
893931
}
894932

933+
private class TestAppError(message: String) : AppError(message)
934+
895935
private fun String.ensurePubkyPrefixForTest(): String =
896936
if (startsWith("pubky")) this else "pubky$this"

0 commit comments

Comments
 (0)