feat: support Pubky signup - #1224
Conversation
d02c926 to
09dff52
Compare
Greptile SummaryThis PR adds Pubky Ring signup URL parsing and scanner routing, registers the wallet-derived identity with the requested Homeserver, activates Paykit, and resumes profile setup through durable local state.
Confidence Score: 3/5This PR should not merge until signup activation can recover from the second network operation failing and pending profile setup can be exited without an immediate navigation loop. The new signup sequence can complete remote registration and authorization while leaving Bitkit without a local session, and the successful path's durable pending marker makes the CreateProfile back action ineffective. Files Needing Attention: app/src/main/java/to/bitkit/services/PaykitSdkService.kt, app/src/main/java/to/bitkit/repositories/PubkyRepo.kt, app/src/main/java/to/bitkit/ui/ContentView.kt
|
| Filename | Overview |
|---|---|
| app/src/main/java/to/bitkit/models/PubkyAuthRequest.kt | Adds strict Ring signup parsing, query validation, and conversion into the existing Pubky authorization request model. |
| app/src/main/java/to/bitkit/repositories/PubkyRepo.kt | Coordinates signup registration, authorization, activation, and pending profile state, but the multi-step flow can strand remotely completed signup without a local session. |
| app/src/main/java/to/bitkit/services/PaykitSdkService.kt | Adds registration without activation, discarding an activatable signup result and requiring a failure-prone second sign-in. |
| app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt | Routes Ring signup through the normal scanner while explicitly rejecting Pubky requests in payment-only contexts. |
| app/src/main/java/to/bitkit/ui/ContentView.kt | Resumes pending profile setup automatically, but conflicts with the unchanged dismissible CreateProfile back action. |
| app/src/main/java/to/bitkit/ui/screens/profile/PubkyAuthApprovalViewModel.kt | Dispatches Ring signup approval, handles existing identities, and dismisses the approval sheet before profile setup. |
Sequence Diagram
sequenceDiagram
participant Scanner
participant Approval as Approval UI
participant Repo as PubkyRepo
participant Server as Homeserver
participant App as Requesting app
participant Paykit
Scanner->>Approval: Pubky Ring signup request
Approval->>Repo: Approve signup
Repo->>Server: Register derived identity
Repo->>App: Approve authorization
Repo->>Paykit: Sign in and activate session
Paykit-->>Repo: Active local session
Repo-->>Approval: Dismiss approval
Repo->>Repo: Mark profile setup pending
Repo-->>Scanner: Navigate to profile setup
Reviews (1): Last reviewed commit: d02c926 | Re-trigger Greptile
|
Regtest device QA, home Scan, QR from staging.pubky.app. Staging e2e doesn’t finish — is that expected? No spinner after scan. Already signed in toast works and the scanner closes (iOS leaves it up — noted on #724). bitkit_logs_2026-09-03_09-44-43-android.zip Screen_Recording_20260903_113712_Bitkit.Regtest-android.mp4 |
|
@piotr-iohk Thanks for the device QA and logs. You found a real signup interop bug: Bitkit treated every The ordinary sign-in QR rejection is separate. These PRs use Paykit rc50’s app-scoped grant auth model, while staging Pubky App is still generating the older auth request format. Pubky App needs to update its sign-in flow to the new grant model for ordinary sign-in to work with Bitkit. Could you please recheck the staging signup path on this head? |
447e781 to
36bfd02
Compare
174e4c8 to
504ae04
Compare
|
Retested the rebased head on a physical Samsung S22 using the regtest build and a signup QR from staging.pubky.app. Signup now completes end to end: authorization progress is visible, Bitkit creates the Pubky identity and opens profile setup, and the staging website continues successfully. The logs confirm the authorization completed and the local identity/session were created. Ordinary sign-in still returns Authorization failed because staging currently generates the older non-grant authorization request. As clarified, that is outside the scope of this signup PR. Non-blocking UI parity note: the approval screen differs between platforms. iOS (left) always shows the placeholder profile card, while Android (right) omits it because no profile exists yet. It would be good to align the intended design, but I don’t think this should block the signup fix.
bitkit_logs_2026-09-03_14-48-43.zip Screen_Recording_20260903_164721_Bitkit.Regtest.mp4 |
|
needs conflict resolution @ben-kaufman |
504ae04 to
94bc950
Compare
ea18746 to
f331ad8
Compare
21d5a06 to
a681dfc
Compare
f331ad8 to
ef49aa1
Compare
|
I checked the failing |
ef49aa1 to
ff24be9
Compare
|
Restacked onto the updated #1200 head ( |
ff24be9 to
f54f1f3
Compare
jvsena42
left a comment
There was a problem hiding this comment.
15aca5075 closes it. You took the second option — scoping the Authorizing refusal to a live inFlightAuthorization rather than settling the state on the terminal paths — and it works for both branches at once, since the guard keys on the in-flight record instead of on which branch was taken.
Trace: after either terminal path returns, the finally at :161 CASes inFlightAuthorization back to null. Re-opening the same URL then hits load:55 (in-flight restore skipped) → resetForLoad:63 → state is Authorizing but inFlightAuthorization.get()?.authUrl == authUrl is null == authUrl, so isAuthorizing is false → CAS to {authUrl, Loading} at :278 → parseAuthUrl → Authorize with real buttons. No SuccessContent flash either, since neither path touches _uiState.
No new door opened: resetForLoad(A) is unreachable while A is genuinely in flight because load:55-62 returns early on the same condition, and a second approveSignupAuth is independently blocked by the untouched inFlightAuthorization.compareAndSet(null, …) at :149 plus initializeMutex and the hasIdentity() re-check in PubkyRepo.approveSignupAuth:1006-1013. A UI-state reset can't bypass those.
terminal signup outcomes allow the same URL to reload for consent (PubkyAuthApprovalViewModelTest.kt:240-270) is a real regression test — revert the :270-271/:274 hunk and it fails on both the assertEquals(Authorize, …) at :261 and the parseAuthUrl verification at :262, and it loops both Result.success and PubkyAlreadySignedInError.
One sibling case inline.
| if ( | ||
| currentState.authUrl == authUrl && | ||
| currentState.state in setOf(ApprovalState.Authenticating, ApprovalState.Authorizing) | ||
| (currentState.state == ApprovalState.Authenticating || isAuthorizing) |
There was a problem hiding this comment.
Low, dev/QA-facing — the Authorizing half is fixed, but Authenticating can strand a URL the same way, via a dismissal the VM never hears about.
With PIN enabled: Authorize → requestAuthorize:104-111 sets Authenticating → AuthCheckView overlays inside the sheet's Box (PubkyAuthApprovalSheet.kt:80-98, :149-164). If the user then taps the scrim, swipes down, or presses system back, none of PubkyAuthApprovalSheet.kt, AuthCheckView.kt or BiometricsView.kt declares a BackHandler or DisposableEffect, so the event goes straight to SheetHost.kt:181-195 → onDismiss → ContentView.kt:499 appViewModel.hideSheet(), bypassing the VM entirely. cancelLocalAuth is only wired to AuthCheckView.onBack / BiometricsView.onFailure (:160, :175), neither of which fires on host-driven dismissal.
So the VM stays {authUrl, Authenticating} with no pending prompt, and unlike the Authorizing branch there is no in-flight record to invalidate it — this guard returns false on the flag alone. Re-scanning the same static direct-signup URL renders AuthorizingContent: spinner, no PIN pad, no buttons, for the life of the Activity. Same symptom you just fixed, reached from the sibling state. Biometrics is mostly immune, since the system prompt's cancel drives onFailure → cancelLocalAuth.
The new test at :217-222 pins the Authenticating refusal as intended but doesn't cover host-driven dismissal, so it passes either way.
Smallest fix keeps the guard and releases the state when the overlay goes away — in PubkyAuthorizationLocalAuth:
DisposableEffect(Unit) { onDispose { pendingAuthUrl?.let(viewModel::cancelLocalAuth) } }Alternatively collapse Authenticating → Authorize from the sheet host's onDismiss.
There was a problem hiding this comment.
Dismissing the approval sheet now clears its pending authentication state. Reopening the same URL returns to consent and requires local authentication again.
|
Moved toast initialization before the startup collectors, so an immediate session-recovery error can show its toast without crashing the app. |
c3d33f1 to
d828fc9
Compare
|
For QA 1, Bitkit waits for the relay approval call to finish before opening Create Profile. That does not confirm the companion received the response. The screenshot shows |
jvsena42
left a comment
There was a problem hiding this comment.
d828fc945 closes it, and the approach is better than either option I offered.
Rather than hooking PubkyAuthorizationLocalAuth or the sheet host's onDismiss, you put a DisposableEffect(viewModel, authUrl) on the sheet root (PubkyAuthApprovalSheet.kt:79-81) calling cancelLocalAuth(authUrl). That catches every dismissal route — back (SheetHost.kt:181-188), scrim (:190-195) and swipe (confirmValueChange:128 permits Hidden since dismissEnabled is true for PubkyAuth; ContentView.kt:472 only restricts Sheet.Send) — because they all converge on hideSheet() → _currentSheet = null → the composable leaves composition. It also covers two cases I hadn't raised: Activity recreation (rotating during the PIN pad used to strand Authenticating with no pad) and sheet replacement by a TimedSheet or incoming-payment sheet.
Nothing new opened. cancelLocalAuth guards on Authenticating only (:139), so it's a deliberate no-op during Authorizing — correct, that state is genuinely in flight, and its terminal cases were already handled by 15aca5075's inFlightAuthorization predicate. There's no window between local-auth success and Authorizing either: AuthCheckView.onSuccess:158-162 and BiometricsView.onSuccess:173-177 call confirmAuthorize synchronously in the same callback. Key-change ordering is safe since Compose dispatches forgets before remembers, and the cancel is URL-guarded anyway.
The toastManager move to :423 is right for the same commit's NPE: Kotlin runs initializers in textual order, and init:493 collects an already-true sessionRestorationFailed → ToastEventBus.send → toast() → toastManager.enqueue, all inline on Main.immediate inside the constructor, before the old :4508 declaration had run. AppViewModelSendFlowTest.kt:271-296 is a real guard for it — on revert enqueue is never reached and the verify fails.
One coverage note, not a defect: PubkyAuthApprovalViewModelTest.kt:226-228 passes on head~1 too, since after cancelLocalAuth the state is Authorize and load on Authorize already re-parsed. The Compose fix itself has nothing driving it — PubkyAuthApprovalSheetTest.kt only covers resolvePubkyApprovalLocalAuthMode.
There was a problem hiding this comment.
I found one blocker in the Ring signup failure path.
Replies
ben-kaufman: Moved toast initialization before the startup collectors, so an immediate session-recovery error can show its toast without crashing the app. (comment)
I verified the construction order and its focused regression on the current code; the startup failure path is covered and this is fixed.
ben-kaufman: For QA 1, Bitkit waits for the relay approval call to finish before opening Create Profile. That does not confirm the companion received the response. (comment)
The previous run did not record the companion commit, relay configuration, or browser diagnostics, so that result does not isolate a Bitkit defect. I have marked it for a fresh device run with those details captured.
Coverage
Total: 58% (whole PR, 23 files)
- Journeys: 83% - Five of six signup journeys have regression coverage; Ring signup lacks bounded recovery when relay delivery never returns.
- Unit tests: 92% - Focused tests cover signup parsing, routing, ordering, rollback, state, and navigation; none covers a non-returning Ring relay retry.
- QA: 0% - Manual Tests not run
Reviewed by Codex (gpt-5.6-sol-high) via gh-pr-review-loop skill
| homeserverZ32 = requireNotNull(request.homeserverPublicKey), | ||
| signupCode = request.signupToken, | ||
| ) | ||
| request.authorizationUrl?.let { pubkyService.approveRingAuth(it, secretKeyHex) } |
There was a problem hiding this comment.
I found that this awaits the Ring relay with no timeout, while the pinned bitkit-core/pubky client has no global HTTP request timeout. If the relay accepts a connection but stops responding, this coroutine stays in Authorizing, retains inFlightAuthorization, and holds initializeMutex, so the user cannot retry signup without restarting the app. Could we bound this call with a Kotlin Duration, restore the sheet to Authorize on timeout, and add a test that proves a retry can proceed?
There was a problem hiding this comment.
Bounded the Ring relay call to 30.seconds. A timeout returns the sheet to consent, and a manual retry requests local authentication again.
jvsena42
left a comment
There was a problem hiding this comment.
Clean — no new findings. Every real issue on this surface is already in the thread history, either fixed, accepted, or still open as ovi-reviewer's relay-timeout thread 3976005716, which I'm not re-raising.
Key material. Secret is seed-derived (PubkyRepo.deriveKeys:538 → deriveLocalSecretKeyFromWalletSeed:1370), passed in-process to bootstrap().signUp (PaykitSdkService.kt:292) and bitkitcore approvePubkyAuth (PubkyService.kt:143), and persisted only via persistSessionAccess (:909, :913) into the AndroidKeyStore-backed Keychain — identical to the pre-existing signIn. No new recovery blob: snapshotSessionBackupState emits LocalSeed kind with no secret. Logging is closed: scanLogId routes through sanitizedQrLogValue() (redacted#<sha256[0:8]>), deeplinks through sanitizedDeeplinkLogValue() (drops query and fragment), and every new parse error message in PubkyAuthRequest.kt is value-free (:249, :253).
Authorization / TOCTOU. requestAuthorize CASes on state.authUrl == authUrl && state == Authorize (PubkyAuthApprovalViewModel.kt:106), confirmAuthorize CASes inFlightAuthorization (:149), and authorize() re-parses the same immutable URL with pure functions (:167) — so the homeserverPublicKey rendered at PubkyAuthApprovalSheet.kt:411-418 is the value handed to registerIdentity (PubkyRepo.kt:1016). approveSignupAuth runs under initializeMutex (:1006) with hasIdentity() checked both before and after deriveKeys() (:1010, :1013), so a Ring-delegated session, a RestorationFailed identity, or a concurrent initialize() all block a second identity. One production caller, reachable only via Authorize → local auth → confirmAuthorize.
Deeplink surface. MainActivityPubkySignup is enabled="false" / exported="true" (AndroidManifest.xml:184-185), with no flavour manifest overriding it, flipped solely by PubkyAuthHandlerRegistrar.kt:67 on isPaykitUiEnabled && !hasIdentity. Any app can fire the intent while it's enabled, but the most it reaches is the consent sheet: processDeeplink requires Paykit on and a wallet (AppViewModel.kt:5172), launchScan defers while !_isAuthenticated (:2076), and flushDeferredScan refuses until authenticated (:2203).
Lifecycle / partial failure. registerIdentity persists nothing, so an approveRingAuth failure after it leaves only a remote account that rc51's 409 path recovers on retry. activateRegisteredIdentity failure runs clearRegisteredIdentityActivationLocked under NonCancellable (:314, :935), clearing both keychain entries plus SDK state; a failed pending-flag write runs forgetSessionAccess + clearLocalState. Wipe is complete: WipeWalletUseCase.kt:62 → wipeLocalState, then settingsStore.reset() at :69 clears the whole local_settings store, so the pending flag can't survive into a new wallet.
Cancellation. The VM is hiltViewModel() in the sheet host (ContentView.kt:593-597), a sibling of RootNavHost, so it's Activity-scoped and sheet dismissal cannot cancel approveSignupAuth. Every rollback is NonCancellable. No new runCatching around suspend calls. A dropped Dismiss effect is harmless because CreateProfile navigation is driven by the persisted flag.
Cross-identity. activateBootstrapResult deletes PAYKIT_SDK_STATE when the public key changes (:926-928); sharesPrivatePaykitEndpoints is reset before registration; the receiver noise key is seed-derived and so identical for the same wallet by construction.
Standing limitation, unchanged: PubkyAuthErrorMessage.kt:29 still falls back to message for unknown Rust errors. That's a pre-existing sink shared with ordinary auth, and my local bitkit-core checkout predates the pubky module, so I still can't read those strings — same limitation I stated before, not a new finding.
Parity with synonymdev/bitkit-ios#724, three things worth cross-feeding:
- iOS is safer on the relay hang.
PubkyProfileManager.approveSignupWithTimeoutraces the relay POST against a 30 sTask.sleepand explicitly refuses to activate a late response. Android awaitsapproveRingAuthunbounded while holdinginitializeMutex— that's ovi-reviewer's open thread, and the iOS implementation is a ready-made shape for the fix. - Correcting something I said on the iOS side: I'd previously called iOS free of any deeplink vector. That's wrong, and it cuts against the "Android's surface is strictly larger" framing. iOS registers
lightning/lnurl*inInfo.plist, andhandleScannedDatastrips those prefixes atAppViewModel.swift:480before testingPubkyAuthRequest.isProtocolURLat:536— solightning:pubkyauth://signup?hs=…opened from a web page reaches the same approval sheet. It's gated at:540and ends at a consent screen, so it's LOW there, but the two platforms have the same shape of exposure, not different ones. Detail is on that PR. - Android is safer on mid-flight teardown. Android's approval VM is Activity-scoped so activation completes even if the sheet is replaced; on iOS
performAuthorizationruns in a view-owned task, so a programmatic sheet replacement could cancel it betweenapproveAuthandactivateIdentity.
Gating: dev/QA-facing today.
jvsena42
left a comment
There was a problem hiding this comment.
Fix confirmed at e3670e99 — ovi-reviewer's thread 3976005716 can be closed. One new LOW inline, which is pre-existing and only surfaced because chasing this fix finally let me read the bitkit-core internals I'd twice flagged as unverified.
The bound works, and a late relay response cannot activate anything. PubkyService.kt:155-157 wraps the exact relay await:
withTimeoutOrNull(timeout) { approvePubkyAuth(authUrl, secretKeyHex) } ?: throw PubkyRingAuthTimeoutError()approvePubkyAuth has exactly one caller (PubkyRepo.kt:1021), so the non-signup approval paths are byte-identical.
The part the new tests don't prove — they mock the FFI with delay — is whether the cancel reaches the socket. It does: bitkitcore.android.kt:18102-18122 runs uniffiRustCallAsync in withContext(Dispatchers.IO), suspends in suspendCancellableCoroutine, and registers invokeOnCancellation { cancelFunc(rustFuture) }. On the Rust side (v0.5.14 src/modules/pubky/auth.rs:122-136) approve_pubky_auth awaits signer.approve_auth inline with no tokio::spawn and never touches the AUTH_FLOW mutex, so cancel/free drops the reqwest POST mid-flight rather than waiting on a hung socket.
Mutex release traced: PubkyRingAuthTimeoutError is an AppError, not a CancellationException → ServiceQueue.kt:31 rewraps → escapes withContext(ioDispatcher) at PubkyRepo.kt:1008 → runSuspendCatching at :1007 returns failure → initializeMutex.withLock at :1006 unlocks in its finally.
Late response: there is no callback path. activateRegisteredIdentity (:1024), _publicKey (:1034) and setPubkyProfileSetupPending(true) (:1038) are all sequenced after :1021 returns normally; after the throw, control never reaches them. A response arriving after the bound resumes an already-cancelled continuation, which kotlinx discards. So no keychain write, no in-memory identity, no pending flag — the iOS guarantee holds by construction here, not just by convention.
On the catching pattern: neither runCatching nor the guarded TimeoutCancellationException shape is used, and neither is needed. withTimeoutOrNull converts only its own timeout into null and rethrows a foreign CancellationException, so structured-concurrency cancellation is preserved and the surfaced error is a plain retriable AppError. That's cleaner than the BlocktankRepo.refreshCjitEntries precedent, and PubkyServiceTest's "relay cancellation propagates" case pins that a real cancellation isn't laundered into a timeout. No rollback is needed on this path because nothing local is written between registerIdentity (:1016, an in-memory bootstrap().signUp result) and the throw; the two NonCancellable rollbacks at :1028/:1042 are unchanged.
RING_AUTH_TIMEOUT = 30.seconds is a kotlin.time.Duration, KDoc'd, overridable, and matches iOS. approvePubkyAuth returns Unit, so the elvis can't misfire on a legitimate null. New string is alphabetical (strings.xml:626, between _missing_claim and _title).
Closing my standing limitation. I've said twice on this PR that I couldn't judge PubkyAuthErrorMessage.kt's raw-message fallback because my local bitkit-core checkout predated the pubky module. I fetched v0.5.14 and read it, so that's now settled — and the answer is that it does leak attacker-controlled text, though no key material. Detail inline. It's pre-existing and gated, so it doesn't affect this PR's verdict; I'm raising it as its own thread rather than on 3976005716 because the same fallback serves the pre-existing paykit approveAuth path too.
Observation, not a finding: registerIdentity (:1016) is still unbounded from Bitkit's side, but it goes through paykit's client, which has its own PubkyClientConfig.requestTimeoutSecs — a different client from the one ovi's thread was about, and the same shape as the pre-existing createIdentity → signUp. Not asking for a second bound.
| } | ||
| current = current.cause | ||
| } | ||
| return message |
There was a problem hiding this comment.
Low, gated, and pre-existing — this commit only moved the when into pubkyAuthMessageResource(), it didn't introduce the fallback. Raising it now because I can finally substantiate it: I'd previously said I couldn't read the bitkit-core error strings, so this was left unverified. I fetched v0.5.14 and read them.
else -> null at :33 means any PubkyException from bitkit-core falls through to return message here, and that message carries the relay's response body verbatim.
The chain: approve_pubky_auth maps every failure to PubkyError::AuthFailed { reason: e.to_string() } (v0.5.14 src/modules/pubky/auth.rs:133-135) → Kotlin PubkyException.AuthFailed.message = "reason=$reason" (bitkitcore.common.kt:4341-4346) → copied by AppError(cause) (Errors.kt:12) → lands here. bitkit-core v0.5.14 pins pubky = "0.6.0", whose signer/auth.rs:38-75 and errors.rs:123-139 produce, among others:
"Server responded with an error: <status> - <response body>"— the relay's body verbatim"HTTP transport error: error sending request for url (<relay>/<base64url(hash(secret))>)"— relay URL plus channel id
Never the secret, never the token (encrypted, request body only), never key material. So this is not a key leak.
What it is: attacker-controlled text rendered inside Bitkit's own chrome. Concretely — QR pubkyring://signup?hs=<valid z32>&relay=https://attacker/inbox/&secret=<32B>&caps=…, user consents (homeserver and trust warning are shown, which is the accepted limitation), relay answers 400 with body Your Bitkit wallet is compromised, move funds to bc1q…. The user sees an error toast titled "Authorization Failed" whose body is reason=Request failed: Server responded with an error: 400 Bad Request - Your Bitkit wallet is compromised…, subject to the toast's line cap. The same text lands in QA log bundles.
Minimal fix, in the function this commit already touched — add a catch-all so remote text never reaches the UI:
is PubkyException -> R.string.profile__auth_error_genericor return null at :18 and let the caller supply generic copy. Either way the timeout path you just added is unaffected: it resolves to a resource at :22 before reaching the fallback.
There was a problem hiding this comment.
Unknown authorization errors now show the localized error title instead of raw exception text. Specific messages, including relay timeout, remain unchanged.
jvsena42
left a comment
There was a problem hiding this comment.
Fix confirmed at 1c43a454. PubkyAuthErrorMessage.kt:18 is now return null instead of return message, so no bitkit-core error text reaches the UI or the logs.
I checked the callers rather than assuming null is safe there — all four pass the result straight into toast(description = …), and toast's signature is description: String? = null (AppViewModel.kt:4515), so a null renders the title alone ("Authorization Failed"). That's the generic copy the finding asked for, with no caller doing an ?: error.message fallback that would defeat it. Call sites: PubkyAuthApprovalViewModel.kt:71 and :313, WatchOnlyAccountsViewModel.kt:79, AppViewModel.kt:5227.
The two new tests are on point — unmapped errors do not expose remote text in toast descriptions and wrapped known errors retain their localized descriptions. The second is the one that matters for not over-correcting: mapped errors still resolve through pubkyAuthMessageResource() and keep their localized strings, including the PubkyRingAuthTimeoutError case added in the previous commit.
That closes the last thing I had open on this PR. Clean from my side.
There was a problem hiding this comment.
Requesting changes: signing in to a third-party Pubky app with a Bitkit-managed identity dead-ends on this branch.
Screen_recording_20260910_115851.mp4
Repro
Dev/regtest build of this branch on an emulator, signing in to Loopky with the identity Bitkit created via the new signup flow. Bitkit takes the deeplink, then dismisses the approval sheet with a title-only error toast. Reproduced twice in one session (deeplink at 14:57, QR scan of the same URL at 14:55).
14:57:23.922 DEBUG [AppViewModel.kt:5053] Received deeplink 'pubkyauth://signin' - AppViewModel
14:57:37.378 ERROR [PubkyAuthApprovalViewModel.kt:67] Failed to parse auth request
[AppError='code=protocol_error, context=only Pubky grant auth URLs are supported'] - PubkyAuthApprovalVM
The URL, from the sender's logcat (secret elided):
pubkyauth://signin?caps=%2Fpub%2Floopky%2F%3Arw%2C%2Fpub%2Fpubky.app%2F%3Arw
&relay=https%3A%2F%2Fhttprelay.pubky.app%2Finbox&secret=…
&x-success=loopky%3A%2F%2Flogin-callback&x-cancel=…&x-error=…&x-source=Loopky
Why it fails — Bitkit's side, and it already ships the fix
pubkyauth://signin?caps=&relay=&secret= is the cookie-flow auth URL that every released Pubky Ring understands (Ring bundles react-native-pubky 0.13.0 = pubky 0.9.x, whose parser knows signin/signup/direct_signup/session). Apps that want to work with shipped Ring have to mint that form; the pubky 0.10+ grant URL (pubkyauth://signin_grant?…) makes Ring answer "Unrecognized format". So the sender is not malformed — this is a pubky-generation mismatch, and Bitkit is on the far side of it.
PubkyRepo.parseAuthUrl (PubkyRepo.kt:996) routes everything that isn't a signup URL to paykit.parsePubkyAuthUrl. paykit 0.1.0-rc51 is built on pubky 0.11.0, whose parser accepts grant URLs only — strings on libpaykit.so has exactly the message we hit next to expected a Pubky grant auth URL.
But Bitkit also links a parser that does understand this URL: bitkitcore.parsePubkyAuthUrl (bitkit-core 0.5.14 → pubky 0.6.0; its PubkyAuthKind covers signin, signup, secret_export), imported at PubkyService.kt:18 as parseLegacyPubkyAuthUrl and used by this PR in validateSignupRequest — with bitkitcore.approvePubkyAuth already wired up in approveRingAuth. So this PR teaches Bitkit to accept the legacy signup intent, while its sibling signin still goes to the grant-only parser and dies.
Scope note, so this isn't misread: the sign-in fall-through is pre-existing on master (parseAuthUrl there has no signup branch and the same single-parser path). I'm raising it here because this PR is where the legacy-URL entry points are being defined and it already has both primitives in hand — happy for it to move to a follow-up if you'd rather keep this one tight.
Asks
-
Fall back to the legacy parser for non-grant
pubkyauth://URLs. InPubkyRepo.parseAuthUrl, whenpaykit.parsePubkyAuthUrlrejects a URL as non-grant, retry throughparseLegacyPubkyAuthUrlrather than surfacing a failure. Approval then has to go throughbitkitcore.approvePubkyAuth(asapproveRingAuthalready does), notPubkyRepo.approveAuth— that one lands inapprovalBootstrap(PaykitSdkService.kt:1000-1006), which re-parses with the same grant-only parser and validates aclientIdthese URLs don't carry. -
Don't advertise handling for URLs Bitkit rejects.
MainActivityPubkyAuthclaims the entirepubkyauthscheme with no host (AndroidManifest.xml:172-177, pre-existing). On my device Pubky Ring is not installed, andpubkyauth://signinresolved straight toto.bitkit.dev/to.bitkit.ui.MainActivityPubkyAuth— Loopky's "is a Pubky authenticator present?" probe is literallypubkyauth://signin, so Bitkit answers yes and then can't serve what it invited. Either land (1) or restrict the filter to the hosts Bitkit can actually process (signin_grant,signup_grant), the way the new signup alias is scoped. -
The toast has no description.
localizedPubkyAuthMessagemaps onlyPubkyAuthRequestError,WatchOnlyAccountErrorand the Ring timeout, so a paykit protocol error falls through tonulland the user gets a bare "authorization failed" title with no reason. Worth a generic fallback string.
|
@jvsena42 We decided not to support legacy Pubky sign-in, so we will not add the legacy parser or approval fallback. Bitkit now advertises only the supported grant and signup links. Unknown authorization errors also show a localized description, with raw exception text kept hidden. |
jvsena42
left a comment
There was a problem hiding this comment.
Manifest restriction reviewed at a9c5c5c9. The restriction itself is correct and complete at the OS level — one LOW inline about a route that goes around it, which is the Android twin of what you fixed on synonymdev/bitkit-ios#724.
One correction to my own framing before anything else: the two added lines are on the authorization alias .ui.MainActivityPubkyAuth, not the signup alias. The signup alias is byte-identical to 1c43a454. So the predicate governing the changed filter is isPaykitUiEnabled && hasIdentity && hasSecretKey, not !hasIdentity.
Matched-URI set, before vs after. Android merges all <data> attributes within one filter as a cross-product, and a filter carrying any android:host requires a non-null host — so:
| before | after | |
|---|---|---|
MainActivityPubkyAuth |
pubkyauth: × any host including absent — pubkyauth://signin, pubkyauth:///?…, pubkyauth://session, pubkyauth://secret_export, … |
pubkyauth × host ∈ {signin_grant, signup_grant} only |
That is a real narrowing, and I couldn't find an OS-level way around it: pubkyring:// isn't on this alias; absent-host forms (pubkyauth:///?…, opaque pubkyauth:signin_grant?…) are excluded rather than over-matched because AuthorityEntry.match returns NO_MATCH_DATA on a null host; scheme matching is case-sensitive so PUBKYAUTH:// fails closed; and pubkyauth://signin_grant.evil / pubkyauth://evil#@signin_grant don't resolve. Host matching is case-insensitive, so pubkyauth://SIGNIN_GRANT resolves — but paykit's parser then adjudicates, worst case an error toast.
Legitimate flow still works, which I checked rather than assumed, since an over-tight filter would be worse than the hole: paykit rc51's intent table carries signin_grant, signup_grant, direct_signup, signin, signup, so both advertised hosts parse. Bitkit's own outbound pubkyauth:///?relay=… is handed in-process to bitkitcore and never fired as an intent, so the narrower filter doesn't touch it, and the signup shapes live on the untouched signup alias.
Runtime gate holds: still enabled="false" / exported="true", and PubkyAuthHandlerRegistrar.kt:81 remains the only setComponentEnabledSetting writer in app/src/main. Nothing in this commit widens exported or touches the registrar.
The tests earn their keep — PubkyAuthManifestTest now asserts the negatives, not just the positives: pubkyauth://?caps=rw, pubkyauth://signin, pubkyauth://grant, pubkyauth://session, pubkyauth://secret_export must resolve to nothing. Every one of those resolved to the auth alias before this commit, so the test genuinely fails without the manifest change.
And as I said, I checked the error-copy line myself: return null → return context.getString(R.string.common__error_body) still keeps bitkit-core text out of the UI, it just gives the toast a body instead of a title alone. Fine.
| contactPaymentContext: ContactPaymentContext?, | ||
| allowPubkyAuth: Boolean, | ||
| ) = withContext(bgDispatcher) { | ||
| val input = result.removeLightningSchemes() |
There was a problem hiding this comment.
Low, gated — but it routes around the restriction you just added, so worth folding in here rather than later.
This line strips the lightning/lnurl prefix before the Pubky test at :2673, so a wrapped Pubky URL is delivered through MainActivity's always-on lightning filter and never consults either Pubky alias — meaning both the enabled="false" gate and the new signin_grant/signup_grant host restriction are bypassed at delivery time.
Trace for lightning:pubkyauth://signin?caps=…&relay=…&secret=… opened from any app or web page:
- Delivered to
MainActivity(manifest:120,lightningscheme, always enabled). processDeeplink:5171testsisProtocolUrl(uri.toString())on the raw string → scheme islightning→ false → falls through to:5184 launchScan(...), which takes the defaultallowPubkyAuth = isMainScanner(true outside the Send sheet).handleScan:2626strips →:2673 isProtocolUrl(input)now true →:2675flag check →handlePubkyAuth→ identity/secret-key checks →Sheet.PubkyAuth("pubkyauth://signin?…").
So the legacy signin shape you deliberately stopped advertising can still be presented for approval remotely. Same for lnurl:pubkyauth://… and LIGHTNING:… (the patterns at :5301-5302 are IGNORE_CASE).
Not a consent or lock bypass — the sheet still requires Authorize plus PIN/biometrics, and the in-app identity checks mirror the alias predicate. What it defeats is the delivery-time restriction, which is precisely what this commit set out to add.
Minimal fix, mirroring what landed on synonymdev/bitkit-ios#724 — decide Pubky-ness on the raw input rather than the stripped one. Either test result instead of input at :2673, or guard right here:
if (PubkyAuthRequest.isProtocolUrl(input) && input != result) {
// a Pubky URL never legitimately arrives wrapped in lightning:/lnurl:
return@withContext
}A wrapped URL then falls through to coreService.decode, which has no handleDecodedScan branch for Scanner.PubkyAuth, so it lands on the existing warning toast — no new copy needed.
Worth a case in AppViewModelSendFlowTest: handleDeeplinkIntent("lightning:pubkyauth://signin_grant?…") must not produce Sheet.PubkyAuth. The manifest test can't cover this one, since the vector lives on MainActivity's filter rather than the aliases.
There was a problem hiding this comment.
Wrapped Pubky URLs now use the existing scan error path before payment state is reset. They no longer reach authorization through lightning or LNURL links.

Description
pubkyring://signupand auth-bearingpubkyauth://signuprequests, plus directpubkyauth://direct_signupand parameter-only legacypubkyauth://signup, through the normal scanner and deep-link flow.This PR is stacked on #1200 and uses its Paykit rc51 authorization model. Ordinary Pubky App sign-in must use that grant-auth model; compatibility with the older sign-in request is intentionally outside this signup PR.
Preview
Not included; this reuses the existing scanner, authorization approval sheet, loading treatment, and profile setup UI.
QA Notes
Validation:
testDevDebugUnitTest: 2,346 tests passed on the restacked branch.compileDevDebugKotlinanddetektpassed.PubkyRepoTest.kt: 97 tests passed, including existing-key recovery, secure-storage errors, sign-in/profile failure retries, cancellation, and no-key signup. Detekt reports no changed-file findings.