Skip to content

feat: support Pubky signup - #1224

Open
ben-kaufman wants to merge 28 commits into
masterfrom
codex/pubky-ring-signup
Open

feat: support Pubky signup#1224
ben-kaufman wants to merge 28 commits into
masterfrom
codex/pubky-ring-signup

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Description

  • Accept app-authorized pubkyring://signup and auth-bearing pubkyauth://signup requests, plus direct pubkyauth://direct_signup and parameter-only legacy pubkyauth://signup, through the normal scanner and deep-link flow.
  • Register the wallet-derived Pubky identity with the requested Homeserver and optional signup token.
  • Require explicit approval and the existing PIN/biometric check for every signup format; show the requested homeserver before registration.
  • Restore locally owned identities by signing in on their existing homeserver, keeping credentials retryable after failures instead of registering at Homegate.
  • Activate the returned Paykit session and continue into the existing profile setup flow.
  • Reuse the approval sheet’s loading state while signup completes, report Already signed in when a local identity exists, and keep payment-only scanner state intact when rejecting Pubky requests.

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

  1. With no Pubky identity, scan a signup QR from staging Pubky App; approve it, confirm the website continues, and complete profile setup.
  2. With no Pubky identity, scan either direct signup format; confirm the homeserver is visible, Cancel performs no registration, and Authorize requires local authentication before profile setup opens.
  3. Scan a signup request while signed in and confirm the Already signed in state.
  4. Confirm payment-only scanners reject Pubky requests without clearing the current payment flow.
  5. With the wallet locked, open a signup deep link; confirm nothing registers or opens until unlocking, then the approval sheet appears.
  6. After signup on a non-default homeserver, make session recovery fail, restore connectivity, and retry Create/Restore Profile. Confirm it keeps the same homeserver. Repeat with a failed profile save and confirm retry never calls signup.

Validation:

  • Full testDevDebugUnitTest: 2,346 tests passed on the restacked branch.
  • compileDevDebugKotlin and detekt passed.
  • Regression coverage includes locked signup deep links, consent/local-auth ordering, real SDK activation and partial-persistence cleanup, stale pending state, and one-time profile navigation.
  • 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.

@ben-kaufman
ben-kaufman force-pushed the codex/pubky-ring-signup branch from d02c926 to 09dff52 Compare September 2, 2026 22:23
@greptile-apps

greptile-apps Bot commented Sep 2, 2026

Copy link
Copy Markdown

Greptile Summary

This 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.

  • Adds parsing and validation for pubkyring://signup requests.
  • Adds registration, requester authorization, session activation, and profile-setup continuation.
  • Preserves payment scanner state when Pubky requests are rejected and reports existing local identities.

Confidence Score: 3/5

This 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

Important Files Changed

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
Loading

Reviews (1): Last reviewed commit: d02c926 | Re-trigger Greptile

Comment thread app/src/main/java/to/bitkit/services/PaykitSdkService.kt
Comment thread app/src/main/java/to/bitkit/ui/ContentView.kt
@ben-kaufman ben-kaufman changed the title feat: support Pubky Ring signup feat: support Pubky signup Sep 2, 2026
@piotr-iohk

piotr-iohk commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Regtest device QA, home Scan, QR from staging.pubky.app.

Staging e2e doesn’t finish — is that expected?
Signup QR does create a Bitkit identity and Create Profile (ring2 / pubky9m…). Staging stays on the Ring / QR step after Authorize. Sign-in QR from the same flow is rejected (only Pubky grant auth URLs are supported). So this isn’t e2e with the Pubky staging app — Bitkit gets a local profile, the website never continues. If approveRingAuth is supposed to complete that session, it’s a bug. If the site only listens for Ring, say so; Ben’s “QR → sheet → profile” case is then only the Bitkit half.

No spinner after scan.
Scanner closes, home looks unchanged for ~18s while register/activate runs. Feels like the scan did nothing, worse when staging also doesn’t move. “Deriving your keys…” on Create Profile is too late. Need a loader from scan until the profile screen.

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

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

@piotr-iohk Thanks for the device QA and logs. You found a real signup interop bug: Bitkit treated every pubkyauth://signup request as direct signup, even when Pubky App included relay, secret, and caps. I pushed a fix so auth-bearing signup requests now register the identity and then complete the relay authorization; parameter-only/direct signup still skips app authorization. Direct signup also shows progress while registration and activation run.

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?

@piotr-iohk

piotr-iohk commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

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.

Screenshot 2026-09-03 at 16 50 53

bitkit_logs_2026-09-03_14-48-43.zip

Screen_Recording_20260903_164721_Bitkit.Regtest.mp4

piotr-iohk
piotr-iohk previously approved these changes Sep 3, 2026
@ovitrif

ovitrif commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

needs conflict resolution @ben-kaufman

@ben-kaufman
ben-kaufman force-pushed the codex/pubky-ring-signup branch from 504ae04 to 94bc950 Compare September 3, 2026 19:03
ovitrif

This comment was marked as resolved.

@ben-kaufman
ben-kaufman force-pushed the codex/pubky-ring-signup branch from ea18746 to f331ad8 Compare September 3, 2026 22:16
@ben-kaufman
ben-kaufman requested a review from ovitrif September 3, 2026 22:18
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-rc50-auth branch from 21d5a06 to a681dfc Compare September 4, 2026 12:24
@ben-kaufman
ben-kaufman force-pushed the codex/pubky-ring-signup branch from f331ad8 to ef49aa1 Compare September 4, 2026 13:19

Copy link
Copy Markdown
Contributor Author

I checked the failing pubky_paykit CI shard. It repeatedly fails the profile/contact-edit scenario with concatenated old and new text (ALICE WALLET AALICE WALLET A, Wallet A original notesEdited from wallet B), rather than failing Ring signup. The shared E2E helper calls clearValue() then setValue() without checking the clear took effect; the app's edit handlers replace the name/bio state directly and do not append it. This points to the Android input-clearing/automation path, but I have not reproduced the cause locally yet, so I am not calling this CI failure resolved or a proven harmless flake. The previously discussed signup-session preservation fix and its regression coverage are present; I resolved that answered review thread.

@ben-kaufman
ben-kaufman force-pushed the codex/pubky-ring-signup branch from ef49aa1 to ff24be9 Compare September 4, 2026 22:15

Copy link
Copy Markdown
Contributor Author

Restacked onto the updated #1200 head (580b042fd) and pushed as ff24be924. All five signup commits are signed; range-diff confirms their patches are unchanged. The only resulting tree changes are the parent PR's SDK-version/changelog update. The previously reported profile-edit E2E issue is not being marked fixed by this restack; the new head still needs CI verification.

ovitrif

This comment was marked as resolved.

ovitrif

This comment was marked as resolved.

@ben-kaufman
ben-kaufman force-pushed the codex/pubky-ring-signup branch from ff24be9 to f54f1f3 Compare September 6, 2026 15:04

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread app/src/main/java/to/bitkit/repositories/PubkyRepo.kt

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:538deriveLocalSecretKeyFromWalletSeed: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:62wipeLocalState, 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:

  1. iOS is safer on the relay hang. PubkyProfileManager.approveSignupWithTimeout races the relay POST against a 30 s Task.sleep and explicitly refuses to activate a late response. Android awaits approveRingAuth unbounded while holding initializeMutex — that's ovi-reviewer's open thread, and the iOS implementation is a ready-made shape for the fix.
  2. 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* in Info.plist, and handleScannedData strips those prefixes at AppViewModel.swift:480 before testing PubkyAuthRequest.isProtocolURL at :536 — so lightning:pubkyauth://signup?hs=… opened from a web page reaches the same approval sheet. It's gated at :540 and 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.
  3. 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 performAuthorization runs in a view-owned task, so a programmatic sheet replacement could cancel it between approveAuth and activateIdentity.

Gating: dev/QA-facing today.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 CancellationExceptionServiceQueue.kt:31 rewraps → escapes withContext(ioDispatcher) at PubkyRepo.kt:1008runSuspendCatching at :1007 returns failureinitializeMutex.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.

Comment thread app/src/main/java/to/bitkit/ui/utils/PubkyAuthErrorMessage.kt Outdated

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

jvsena42

This comment was marked as low quality.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.14pubky 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

  1. Fall back to the legacy parser for non-grant pubkyauth:// URLs. In PubkyRepo.parseAuthUrl, when paykit.parsePubkyAuthUrl rejects a URL as non-grant, retry through parseLegacyPubkyAuthUrl rather than surfacing a failure. Approval then has to go through bitkitcore.approvePubkyAuth (as approveRingAuth already does), not PubkyRepo.approveAuth — that one lands in approvalBootstrap (PaykitSdkService.kt:1000-1006), which re-parses with the same grant-only parser and validates a clientId these URLs don't carry.

  2. Don't advertise handling for URLs Bitkit rejects. MainActivityPubkyAuth claims the entire pubkyauth scheme with no host (AndroidManifest.xml:172-177, pre-existing). On my device Pubky Ring is not installed, and pubkyauth://signin resolved straight to to.bitkit.dev/to.bitkit.ui.MainActivityPubkyAuth — Loopky's "is a Pubky authenticator present?" probe is literally pubkyauth://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.

  3. The toast has no description. localizedPubkyAuthMessage maps only PubkyAuthRequestError, WatchOnlyAccountError and the Ring timeout, so a paykit protocol error falls through to null and the user gets a bare "authorization failed" title with no reason. Worth a generic fallback string.

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

@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 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 keepPubkyAuthManifestTest 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 nullreturn 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.

Comment thread app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt Outdated
@jvsena42

Copy link
Copy Markdown
Member

Interop test: a third-party app signing in through Bitkit (Loopky)

I tested Bitkit codex/pubky-ring-signup @ 40b218228 (dev, 2.4.1) as the signer for Loopky's grant sign-in on an Android emulator against staging. The request was pubkyauth://signin_grant, client_id=loopky.app, caps /pub/loopky/:rw,/pub/pubky.app/:rw. Loopky side: jvsena42/loopky#296 + jvsena42/pubky-core-ffi-fork#7.

✅ Grant approval works end to end. Bitkit parses the request, and the sheet shows "Requester ID: loopky.app" with both scopes as READ, WRITE. After Authorize it logs Approving grant signin … client_id=loopky.appAuth payload delivered successfully. Loopky receives the payload over the relay and exchanges the grant at the homeserver. The session it gets back ({capabilities, grant_secret, pubky}) revalidates, and reads work.

Three things came up, most disruptive first:

1. A pubkyauth:// deep link that arrives during a cold start is dropped silently. The intent reaches MainActivityPubkyAuth, but PubkyAuthHandlerRegistrar disables that alias about a second later because pubkyRepo.publicKey hasn't loaded yet. handlePubkyAuth then returns on publicKey == null with only a toast:

21:27:05.489  START … cmp=to.bitkit.dev/to.bitkit.ui.MainActivityPubkyAuth
21:27:06.455  Updated Pubky handler 'to.bitkit.ui.MainActivityPubkyAuth' to 'disabled'
21:27:06.848  Received deeplink 'pubkyauth://signin_grant'
21:27:12.695  Updated Pubky handler 'to.bitkit.ui.MainActivityPubkyAuth' to 'enabled'

From the requesting app nothing happens at all: no sheet, no callback. It happened both times Bitkit was cold-started; with Bitkit already running, the same request works. Holding the request until the identity has loaded, as a signup deep link already waits for unlock, would fix it.

2. Bitkit doesn't return to the requesting app. Loopky appends x-success / x-cancel / x-error (→ loopky://login-callback) and x-source, as Pubky Ring expects. Bitkit only emits those for its own Ring auth (PubkyRingAuthCallback) and never reads them from an incoming request. After "Authorization Successful" the user has to switch back by hand, where Pubky Ring opens the x-success URL itself.

3. The approved identity can't write /pub/pubky.app/tags/ (probably not this PR's code, but maybe its signup). Loopky's first write after sign-in is a tag on the user's own profile at /pub/pubky.app/tags/<id>. Staging answered 403 Forbidden - Write to this path is not allowed. That message is the homeserver's WritePathForbidden, raised by the per-user allowed_write_paths quota check (persistence/files/write_path_layer.rs), not by grant capability enforcement. A grant with the same client_id and caps, signed for a different staging account, writes that same path without error. So the identity held by this Bitkit (j8zch…) looks like it was registered with a write-path allowlist. If identities created through this PR's signup get such an allowlist, they won't be able to use apps that write outside it. Is that intended? (Not tested yet: writes under /pub/loopky/.)

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

@jvsena42 Fixed the identity-loading race. Grant sign-in links now wait for Pubky initialization within the existing scan flow, and the auth alias keeps its saved state during startup. Newer scans still take precedence, and wallet unlock and approval are still required.

Incoming x-success, x-cancel, and x-error callbacks are still unsupported, so returning to Loopky still requires switching apps manually.

Bitkit does not add a write-path allowlist to the signup request. It passes the homeserver and signup token to the SDK, and the homeserver can apply the token's quota, including allowed_write_paths, to the new identity. Was this account created through a signup link or Bitkit's Create Profile flow? That determines which signup token policy needs checking.

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

@jvsena42 We'll handle app-return callback support separately later. It's out of scope for this PR, so users will still need to switch back to the requesting app manually.

@jvsena42

jvsena42 commented Sep 11, 2026

Copy link
Copy Markdown
Member

@ben-kaufman Thanks for the quick fix on the startup race. I'll retest it on the next build.

The account was created through Bitkit's Create Profile flow, not a signup link. So the policy to check is the one on the signup token Create Profile registers with on staging. The identity it produced (j8zch…) gets 403 Write to this path is not allowed on /pub/pubky.app/tags/. I haven't tried /pub/loopky/ yet.

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

@jvsena42 That confirms the 403 comes from the Homeserver/Homegate policy, not a decision made by this Android PR or its signup-link implementation. Create Profile uses Homegate's IP-verification signup token. The checked-in staging config limits those accounts to /pub/paykit/ and /pub/staging.bitkit.to/, so /pub/pubky.app/tags/ and /pub/loopky/ are outside the allowlist. This matches the behavior you saw, but it is based on the checked-in deployment config rather than a direct read of that account's live quota.

@jvsena42

jvsena42 commented Sep 11, 2026

Copy link
Copy Markdown
Member

@ben-kaufman Retested on 48c8b0418 (dev debug, arm64 emulator) with Loopky.

Warm start works: the sheet appears, and the approval is delivered.

Cold start still drops the link. It never reaches the new wait in launchScan:

06:35:25.258  process start
06:35:25.654  Received deeplink 'pubkyauth://signin_grant'
                (no "Starting scan from 'deeplink'", no sheet)
06:35:33.904  Updated Pubky handler 'to.bitkit.ui.MainActivityPubkyAuth' to 'enabled'

The early return in processDeeplink, if (!isPaykitEnabled.value || !walletRepo.walletExists()) return@launch, reads isPaykitEnabled, which is stateIn(viewModelScope, SharingStarted.Eagerly, false). It's still false 0.4 s into a cold start, so the link returns silently before launchScan. Sending the pubkyauth branch straight to launchScan would close it, since awaitPubkyDeeplinkInitialization already checks Paykit and the wallet after initialization. Awaiting the first settings value there instead of reading .value would also work.

Can be considered a LOW, since doesn't affect prod

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked hardest at c9356312d, since retaining a link across startup is exactly the shape that could bring back the PIN bypass this PR originally had. It doesn't — no consent or PIN bypass in either commit.

The "retained" link isn't persisted and isn't dispatched anywhere new. It's the scan job itself, parked in awaitPubkyDeeplinkInitialization while holding scanMutex (AppViewModel.kt:2105-2114). When it resumes after awaitInitialization() (:2147), deferLockedScan re-reads _isAuthenticated.value at :2159 — so if the app locked while it waited, it re-enqueues to deferredScan and returns without touching handleScan. The only replay of that slot is flushDeferredScan:2236, which refuses at :2237 unless authenticated. The terminal is still showSheet(Sheet.PubkyAuth(authUrl)) at :5255, with Authorize + local auth behind it unchanged. The hold releases on initializationJob.join() rather than on unlock, but release only ever leads back into deferLockedScan, so unlock is still required before the sheet.

The other gates are re-evaluated at replay too. For grant URLs :2148 re-checks isPaykitEnabled and walletExists() after the await (pinned by cold pubky auth deeplink stops when Paykit is disabled during initialization); for signup URLs handleScan:2709 re-checks the flag on every replay. A link surviving a wipe fails closed regardless, since WipeWalletUseCase.kt:69 resets isPaykitEnabled to false and the link lands on the :2712-2717 error toast. Nothing persists a link across process death. Single-use holds (scheduledScan cleared in invokeOnCompletion at :2125; flushDeferredScan nulls the slot before launching at :2243), and retention never reaches approveSignupAuth, so the inFlightAuthorization CAS and initializeMutex guards are unaffected.

The registrar change (PubkyAuthHandlerRegistrar.kt:50) doesn't widen anything — both alias predicates are byte-identical. Its effect is that the aliases keep their last-written state during startup instead of being recomputed from a null publicKey, which actually removes the ~1s cold-start window where identity-holding users briefly had the signup alias enabled.

40b218228 is complete for the reachable vectors and fails closed with feedback. rejectPubkyAuthScan runs on the raw input before the prefix strip, so any of the five wrapping prefixes around pubkyauth:// or pubkyring://signup gets an error toast with payment state preserved. I also tried double prefixes (they survive one fold as a non-protocol string → generic decode error), URL-encoded schemes (no scheme for URI()), leading whitespace (URI() throws), and BIP21 ?lightning=pubkyauth://… (goes through decode, and there's no Scanner.PubkyAuth branch in handleDecodedScan). None reach handlePubkyAuth.

One lifecycle note and one diagnostics nit inline. Both affect opted-in users on released builds (isPaykitEnabled defaults false); neither blocks.

Comment thread app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
Comment thread app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
@ben-kaufman

Copy link
Copy Markdown
Contributor Author

@jvsena42 Fixed this. Pubky auth links now enter the scan queue before the cached Paykit flag is ready. The scan reads the persisted setting before routing, so the cold-start path no longer drops the link while disabled Paykit and missing-wallet cases still stop.

@jvsena42

Copy link
Copy Markdown
Member

@ben-kaufman conflict

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants