Repository navigation
feat(mobile): organization sign-in for NIP-FI relays - #8700
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 262d3e4405
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
## Summary Saved accounts awaiting screening can report `account_status: ACCOUNT_STATUS_PENDING` while the deprecated `account_standing` still says details are missing. Treat only `ACCOUNT_STATUS_NEEDS_DETAILS` as requiring browser setup, so resuming those accounts does not reopen `/auth/native`. Product access remains enforced by the backend. Stacked on #8712 at `36d2bccc3ade84beb9f61723fa343ef0c6b8d45d`; land #8700 and #8712 first, then retarget/rebase this PR onto main. Do not merge into the parent feature branch. Companion pending-screening callback fix: squareup/cash-server#144323. ## Validation Extended the existing saved-account invite journey to cover pending screening, retained consent across restart, and no browser launch. Existing fresh and saved-incomplete journeys use the current account-status wire contract. All 21 hosted journeys passed in an isolated control; restoring the old production predicate fails the pending-account journey (one browser launch instead of zero). The account client test file also passed (11 tests). Mandatory pre-push checks passed at `ac6920635`: mobile analysis, 4,005 default Flutter tests (29 skipped), four unconfigured-push tests and 27 secp256k1 tests. Draft pending review, CI, and real-device/human acceptance. Local synthetic journeys do not prove live screening or native browser behavior. Verify a saved pending account retains its invite without reopening setup, and an account genuinely missing details still enters setup. Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
) ## Summary Public mobile sign-in currently returns a native credential before new accounts finish web setup. Route the public account origin through `/auth/native`, preserving the existing state, S256 challenge, callback and retained invite. Older saved accounts marked incomplete enter the same setup flow, retaining their subject so a different browser account cannot replace them. Organization sign-in keeps the existing direct CLI route. ### Related issue Stacked on #8700 (`262d3e4405`), targeting `mobile-organization-sign-in` for a narrow review. Land #8700 first, then rebase/retarget this PR to main; do not merge this into studio's branch. Existing open-PR search found no duplicate public setup caller change. Companions: squareup/cash-server#144146 and squareup/ext-builderbot-ui#347. ### Deployment and acceptance gates This affects public sign-in in **all builds, including App Store builds**, not only dogfood. Backend then web must be deployed in the target environment before this mobile change merges into a release branch or ships. A missing route/POST would otherwise break public sign-in. Confirm public authentication-time persistence and the configured login route. Staging builds require the corresponding staging deployments. Keep draft until real-device and human acceptance required by AGENTS.md is recorded. After deployment, use a staging-configured build and a fresh invite: create a new account, verify email, complete DOB/setup, return to Buzz, complete identity/backup, and reach the invited community. Also retry an older incomplete saved account, confirm cancel/retry retains the invite, and verify organization sign-in still works. The browser attempt retains its existing ten-minute lifetime; an expired attempt requires retry. ### Testing Extended the existing browser cold-start/PKCE test and invite widget journey for fresh and saved-incomplete accounts. The journey uses real flow, account, identity, admission and persistence with synthetic browser/network/storage boundaries. Verified invite retention before callback and successful admission through starter-recovery retry. Existing organization route and shared-callback tests pass. The old URL and missing saved-session recovery each produced a failing regression before their fixes. The browser test also checks a temporary backend error surfaces retry while retaining the invite. Mandatory pre-push checks passed at `aafd876b6`: mobile format/analyze, the default Flutter suite (3,950 passed, 29 skipped), four unconfigured-push tests and 27 secp256k1 tests. The separately enabled hosted invite journey file passed all 21 tests. Carl reviewed the production delta with no blockers. Local widget tests do not prove live Auth0, email verification or native browser return. Device and human acceptance remain outstanding. ### iOS bridge follow-up The iOS browser bridge now allows `/auth/native` alongside the existing login path. Trusted hosts, HTTPS, URL credentials/port/fragment rejection and callback handling remain unchanged. The focused BrowserAuthBridgeTests journey passed on a separate simulator, the staging simulator successfully opened web signup, and mandatory mobile push checks passed at `6d7b010ed`. Full live signup remains blocked pending backend/web follow-ups: squareup/cash-server#144323 and squareup/ext-builderbot-ui#354. Saved-account status recovery is stacked in #8716. --------- Signed-off-by: Luke Tornquist <tornquist@squareup.com> Signed-off-by: Tom Brow <tomb@block.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com> Signed-off-by: seiler <seiler@squareup.com> Signed-off-by: Brad Seiler <seiler@squareup.com> Signed-off-by: Tom Brow <tomb@squareup.com> Co-authored-by: Luke Tornquist <tornquist@squareup.com> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Tom Brow <tomb@block.xyz> Co-authored-by: george <973be878edd6d4f79692a3827f20a6f7fe0ed0362f269ff6b6ba8867033d7d8e@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Co-authored-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz> Co-authored-by: Brad Seiler <seiler@squareup.com> Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Codex <codex@openai.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
Self-hosted invite claims retain the generated identity in secure storage before the HTTP request and reuse it after a lost response or restart. Saving or reopening an existing destination retires the matching journal. Sign-out/removal deletes all matching pending key copies before removing the saved community, preserving the community for retry if cleanup fails. Pairing verifies that relay authentication completed before saving/activating a destination or reporting success. On iOS 16, hosted entry permits recovery and reuse of backed-up identities, including binding a recovered identity with a confirmed backup receipt. Paths requiring a new secure backup explain the iOS 17 requirement before generating or binding a new key. Self-hosted entry remains available. ### Validation - Initial focused entry/recovery files: 83 passed, 1 skipped. Review-fix focused storage, community, invite and hosted files: 82 passed, 1 skipped; extended invite journey: 17 passed, 1 skipped. - Final hosted journey extension: 20 passed, including both bound and unbound recovery with native sealing unavailable. - Isolated mutations removing invite retention, pairing validation, journal removal/re-entry cleanup or restoring the blanket platform gate fail their intended regressions. Restored controls pass. - iOS simulator build passed on the original integration base; final native/device acceptance remains outstanding. - Carl and three specialist reviewers found no source-review blockers at `323cec6ea2`. `a55a246857` adds analyzer fixes, copy cleanup and the recovered-unbound journey coverage. Reviewers did not run tests or the app. - Fixed a push-hook timeout in `4564b3562a`: idle serial queues now release completed futures, avoiding scheduler retention across widget tests while preserving in-flight ordering and failure isolation. The existing dispatcher journey fails promptly before the fix and passes afterward; Carl reviewed this delta with no blockers. Focused dispatcher/invite/storage checks passed, and all 20 hosted journeys passed with hosted auth enabled. - Mandatory pre-push checks passed at `4564b3562a` (scope, branch skew, file size, mobile analysis/tests and crypto package checks); GitHub head verified at that commit. ### Remaining acceptance Draft pending agent/device and human acceptance. Exercise interrupted one-use invite claim and retry across restart, sign-out then reopen without restoring the removed identity, rejected self-hosted pairing, iOS 16 backed-up identity recovery (including binding), and iOS 16 new setup blocked before key creation. Synthetic network/storage boundaries do not prove native durability or live server behavior. Hosted CI results are not claimed here. Optional follow-ups remain outside this patch: pruning provably unadmitted journals, malformed-record recovery, avoiding an iOS 16 dead-end device-key selection, and pre-existing removal/push-revocation ordering. This PR contains three existing-path fixes. The two organization-login fixes depend on #8700 and remain separate: its stale parent conflicts with main in unrelated relay code, and the required branch-skew hook prevents publishing that stack unchanged. This PR does not refresh that parent or change deployment policy. --------- Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
) ## Summary Public mobile sign-in currently returns a native credential before new accounts finish web setup. Route the public account origin through `/auth/native`, preserving the existing state, S256 challenge, callback and retained invite. Older saved accounts marked incomplete enter the same setup flow, retaining their subject so a different browser account cannot replace them. Organization sign-in keeps the existing direct CLI route. ### Related issue Stacked on #8700 (`262d3e4405`), targeting `mobile-organization-sign-in` for a narrow review. Land #8700 first, then rebase/retarget this PR to main; do not merge this into studio's branch. Existing open-PR search found no duplicate public setup caller change. Companions: squareup/cash-server#144146 and squareup/ext-builderbot-ui#347. ### Deployment and acceptance gates This affects public sign-in in **all builds, including App Store builds**, not only dogfood. Backend then web must be deployed in the target environment before this mobile change merges into a release branch or ships. A missing route/POST would otherwise break public sign-in. Confirm public authentication-time persistence and the configured login route. Staging builds require the corresponding staging deployments. Keep draft until real-device and human acceptance required by AGENTS.md is recorded. After deployment, use a staging-configured build and a fresh invite: create a new account, verify email, complete DOB/setup, return to Buzz, complete identity/backup, and reach the invited community. Also retry an older incomplete saved account, confirm cancel/retry retains the invite, and verify organization sign-in still works. The browser attempt retains its existing ten-minute lifetime; an expired attempt requires retry. ### Testing Extended the existing browser cold-start/PKCE test and invite widget journey for fresh and saved-incomplete accounts. The journey uses real flow, account, identity, admission and persistence with synthetic browser/network/storage boundaries. Verified invite retention before callback and successful admission through starter-recovery retry. Existing organization route and shared-callback tests pass. The old URL and missing saved-session recovery each produced a failing regression before their fixes. The browser test also checks a temporary backend error surfaces retry while retaining the invite. Mandatory pre-push checks passed at `aafd876b6`: mobile format/analyze, the default Flutter suite (3,950 passed, 29 skipped), four unconfigured-push tests and 27 secp256k1 tests. The separately enabled hosted invite journey file passed all 21 tests. Carl reviewed the production delta with no blockers. Local widget tests do not prove live Auth0, email verification or native browser return. Device and human acceptance remain outstanding. ### iOS bridge follow-up The iOS browser bridge now allows `/auth/native` alongside the existing login path. Trusted hosts, HTTPS, URL credentials/port/fragment rejection and callback handling remain unchanged. The focused BrowserAuthBridgeTests journey passed on a separate simulator, the staging simulator successfully opened web signup, and mandatory mobile push checks passed at `6d7b010ed`. Full live signup remains blocked pending backend/web follow-ups: squareup/cash-server#144323 and squareup/ext-builderbot-ui#354. Saved-account status recovery is stacked in #8716. --------- Signed-off-by: Luke Tornquist <tornquist@squareup.com> Signed-off-by: Tom Brow <tomb@block.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com> Signed-off-by: seiler <seiler@squareup.com> Signed-off-by: Brad Seiler <seiler@squareup.com> Signed-off-by: Tom Brow <tomb@squareup.com> Co-authored-by: Luke Tornquist <tornquist@squareup.com> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Tom Brow <tomb@block.xyz> Co-authored-by: george <973be878edd6d4f79692a3827f20a6f7fe0ed0362f269ff6b6ba8867033d7d8e@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Co-authored-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz> Co-authored-by: Brad Seiler <seiler@squareup.com> Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Codex <codex@openai.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
0ff0749 to
e44c45a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e44c45a088
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
) Public mobile sign-in currently returns a native credential before new accounts finish web setup. Route the public account origin through `/auth/native`, preserving the existing state, S256 challenge, callback and retained invite. Older saved accounts marked incomplete enter the same setup flow, retaining their subject so a different browser account cannot replace them. Organization sign-in keeps the existing direct CLI route. Stacked on #8700 (`262d3e4405`), targeting `mobile-organization-sign-in` for a narrow review. Land #8700 first, then rebase/retarget this PR to main; do not merge this into studio's branch. Existing open-PR search found no duplicate public setup caller change. Companions: squareup/cash-server#144146 and squareup/ext-builderbot-ui#347. This affects public sign-in in **all builds, including App Store builds**, not only dogfood. Backend then web must be deployed in the target environment before this mobile change merges into a release branch or ships. A missing route/POST would otherwise break public sign-in. Confirm public authentication-time persistence and the configured login route. Staging builds require the corresponding staging deployments. Keep draft until real-device and human acceptance required by AGENTS.md is recorded. After deployment, use a staging-configured build and a fresh invite: create a new account, verify email, complete DOB/setup, return to Buzz, complete identity/backup, and reach the invited community. Also retry an older incomplete saved account, confirm cancel/retry retains the invite, and verify organization sign-in still works. The browser attempt retains its existing ten-minute lifetime; an expired attempt requires retry. Extended the existing browser cold-start/PKCE test and invite widget journey for fresh and saved-incomplete accounts. The journey uses real flow, account, identity, admission and persistence with synthetic browser/network/storage boundaries. Verified invite retention before callback and successful admission through starter-recovery retry. Existing organization route and shared-callback tests pass. The old URL and missing saved-session recovery each produced a failing regression before their fixes. The browser test also checks a temporary backend error surfaces retry while retaining the invite. Mandatory pre-push checks passed at `aafd876b6`: mobile format/analyze, the default Flutter suite (3,950 passed, 29 skipped), four unconfigured-push tests and 27 secp256k1 tests. The separately enabled hosted invite journey file passed all 21 tests. Carl reviewed the production delta with no blockers. Local widget tests do not prove live Auth0, email verification or native browser return. Device and human acceptance remain outstanding. The iOS browser bridge now allows `/auth/native` alongside the existing login path. Trusted hosts, HTTPS, URL credentials/port/fragment rejection and callback handling remain unchanged. The focused BrowserAuthBridgeTests journey passed on a separate simulator, the staging simulator successfully opened web signup, and mandatory mobile push checks passed at `6d7b010ed`. Full live signup remains blocked pending backend/web follow-ups: squareup/cash-server#144323 and squareup/ext-builderbot-ui#354. Saved-account status recovery is stacked in #8716. --------- Signed-off-by: Luke Tornquist <tornquist@squareup.com> Signed-off-by: Tom Brow <tomb@block.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com> Signed-off-by: seiler <seiler@squareup.com> Signed-off-by: Brad Seiler <seiler@squareup.com> Signed-off-by: Tom Brow <tomb@squareup.com> Co-authored-by: Luke Tornquist <tornquist@squareup.com> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Tom Brow <tomb@block.xyz> Co-authored-by: george <973be878edd6d4f79692a3827f20a6f7fe0ed0362f269ff6b6ba8867033d7d8e@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Co-authored-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz> Co-authored-by: Brad Seiler <seiler@squareup.com> Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Codex <codex@openai.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
580e0c6 to
e0a60ba
Compare
🔐 Codex Security Review
Review SummaryOverall Risk: HIGH
Findings[HIGH]
|
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Verdict: two blocking findings at 8f33cde, both inline: the organization denial state has no account-recovery path, and cachedHeaders never matches organization assertions. Two optional notes are inline too: first binding, and the iOS allowlist.
Held up under review: credentials and assertions stay inside their realm (org session only to accountOrigin, assertions only to exact BUZZ_ORGANIZATION_RELAYS hosts, remote agents stay hosted). Callback routing between BrowserAuth owners, the shared _pending/_cached serialization, and the signed-out no-assertion connect also held. Hosted and organization banners are mutually exclusive, and #8712's needsAccountSetup branch keeps the target and pairing intent. Both earlier Codex findings are addressed.
Validation gap: the organization tests call signIn()/receive() and a counting reconnect directly. None mounts OrganizationSignInBanner or reaches the real relay-session reconnect. Extending one journey through denial → account change → reconnect would cover the first blocker.
Exit criteria: an organization account-change path that clears only the organization session, the cachedHeaders fix, and that journey step.
Reviewed from source with CI green on mobile, Android and Swift; the Codex security review was still pending. No local runs.
| actions = [ | ||
| TextButton( | ||
| onPressed: () => | ||
| unawaited(ref.read(relaySessionProvider.notifier).reconnect()), |
There was a problem hiding this comment.
Blocking (P2): a denied organization account has no way out. When the signed-in organization account is refused for this key (bound to a different key, or no access), the only action is Retry. Retry reconnects with the same session, so it fails the same way every time. HostedReconnectBanner deliberately hides its account-change action on organization relays (hosted_reconnect_banner.dart:25). Nothing in mobile/lib ever clears or switches the organization session (organizationAccountProvider is only read by organization_sign_in.dart and hosted_relay_auth.dart).
Scenario: open an organization community with key K and sign in as an organization account bound to another key. The banner shows "cannot use this community with this key" with Retry as the only option, and the user is stuck until they reinstall. Reopening login wouldn't help either: exchange refuses a different subject while the old record exists.
Fix: for denied, add a change-account action that calls clearLocalSession() on the organization client only, then signIn(). Cover it as a step in a widget journey that mounts the banner and goes through the real relay reconnect.
There was a problem hiding this comment.
🤖 Agreed. Addressed in c097f9e. A denied organization account now gets Use a different account next to Retry. It clears only the organization session (clearLocalSession on the organization client) and then signs in again. The new journey in hosted_journeys_test.dart mounts the real banner and goes through denied, Retry (still denied), change account, and a real relay-session reconnect that carries the new account's assertion. It checks that the hosted session credential is unchanged, and it fails without the fix.
| enabled && | ||
| HostedEnvironment.requiresAuth(Uri.parse(config.baseUrl).host) && | ||
| HostedEnvironment.supportsRelay(Uri.parse(config.baseUrl)); | ||
| bool protects(RelayConfig config) => _accountFor(config) != null; |
There was a problem hiding this comment.
Blocking (small): cachedHeaders (line 105) still compares cached.account with the hosted account.account. protects now covers organization relays, but _issue caches the organization session there, and the organization and hosted sessions are never the same object. So cachedHeaders(orgConfig) always returns null, and saved_identity_profile.dart:50 skips the profile read. A saved identity whose profile lives on an organization relay loses its name and avatar in identity selection. It fails safe, so nothing leaks.
Fix: compare with _accountFor(config)?.account, as _headers does at line 140.
| final identity = current['identity']; | ||
| if (current['error'] != null) throw AccountFailure.invalidResponse; | ||
| if (identity == null) { | ||
| if (identity == null && identical(account, organization)) { |
There was a problem hiding this comment.
Optional question, not blocking. I'm leaving first binding without a backup check alone, since the PR states it as the intended behavior. One edge to confirm on the issuer side: when the organization session expires, restore → _clear deletes the stored subject, so the next sign-in can finish silently as a different tenant account B (Safari SSO cookie reuse). If B has no bound key, this branch issues for K and the tenant binds K to B. Does the tenant refuse to bind a key that's already bound to another subject? If yes, nothing to do here. If not, that's a tenant-side fix rather than a mobile one. The account-change path above at least makes a wrong binding recoverable in the app.
There was a problem hiding this comment.
🤖 I haven't verified that on the issuer side. Whether the tenant refuses to bind a key that's already bound to another subject is a Builderlab question, so I'm leaving mobile as is. The change-account path in c097f9e now makes a wrong binding recoverable in the app.
| url.user == nil, url.password == nil, url.port == nil, url.fragment == nil | ||
| else { return false } | ||
| let builderlab = ["builderlab.xyz", "builderlabstaging.xyz"].contains { | ||
| host.hasSuffix(".\($0)") |
There was a problem hiding this comment.
Nit (optional, defense-in-depth). This accepts either path on any *.builderlab.xyz / *.builderlabstaging.xyz host at any depth, including /auth/native on tenants and CLI login on app.. The URL comes from compile-time Dart config, so this isn't exploitable as written. Tighter options: pair each path with its host (/auth/native only on app.*), or pass the configured organization host in through Info.plist.
There was a problem hiding this comment.
🤖 Taken. Addressed in 248118c: the public app.* hosts now open only /auth/native, and single-label tenant hosts open only the CLI login. The XCTest covers the crossed and nested cases.
| failure = null; | ||
| _set(OrganizationSessionStatus.signedIn); | ||
| await reconnect(); | ||
| } on AccountFailure catch (error) { |
There was a problem hiding this comment.
🤖 P2 · Cancelled exchanges overwrite newer logins
Start login A, let its callback enter a slow account.exchange(), then cancel and start B. Cancellation only advances BrowserAuth's generation, so A's already-dispatched _exchange() still runs. If A subsequently fails, this catch resets B to signedOut while B's browser is active. A successful late completion can instead clear B's retained intent and replace its status.
User/system effect: B loses its “Cancel sign-in” action, and its later browser failure is ignored because _onBrowserFailure requires signingIn. Exchange completion and failure need to remain owned by the attempt that started them.
Test: hold A's exchange, cancel A, start B, then release A with an error and verify B remains signing in and cancellable.
There was a problem hiding this comment.
🤖 Addressed in b7742ec. Each start and cancel now advances an attempt token, and an exchange applies its result only if its attempt is still current, so a late success or failure from a cancelled attempt leaves the newer sign-in alone. Test: a cancelled exchange never changes a newer sign-in holds A's exchange, cancels, starts B, fails A, and checks that B stays signing in and can still be cancelled.
Builds can name an organization identity provider (a Builderlab tenant) and the exact relays that trust it with BUZZ_ORGANIZATION_URL and BUZZ_ORGANIZATION_RELAYS. Those relays get NIP-FI assertions from a separate organization session, signed in through the tenant's browser login, while hosted communities keep their own account. Signed out, the app connects without an assertion and offers sign-in. This is the transport half of per-origin accounts: account clients and browser owners are per origin, the shared callback is routed by state, and relay auth selects the account for each relay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
A relay retry that restored the organization session after an offline start left the sign-in banner up: subject listeners ran before the account was assigned and still read it as signed out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
…logins The bridge accepted only the public hosts' CLI login, so the public account's /auth/native page and an organization tenant's login on its own host failed as unavailable before a browser opened. Accept either login path on any Builderlab subdomain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
After a restart, a peer owner checking who owns a shared callback could read the saved attempt just after its owner deleted it, then fail its own flow with restartLogin. Mark the state consumed before deleting the attempt, and recheck it after the peer read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
cachedHeaders compared the cached account with the hosted session, so organization relays never matched and saved identities lost their profile in identity selection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
A denied organization account could only retry the same session. Offer Use a different account, which clears only the organization session and signs in again; the hosted account is untouched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
Public hosts open only /auth/native and single-label tenant hosts only the CLI login. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
The organization sign-in banner, the hosted reconnect banner and the sign-in recovery row were pinned to the safe-area bottom, so Home's floating Search/New bar covered their buttons. These overlays only show over Home, so lift them above the bar using a shared mobileTabBarTop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
At cold start, organization sign-in and the first relay request both called restore() before an account was loaded. The second restore adopted a new account object while the first caller's assertion request was still waiting, so that request failed the session identity check and showed "cannot use this community with this key". Overlapping restores now share one future; a later restore still re-verifies so retry detects expiry. Signed-off-by: Tom Brow <tomb@squareup.com>
A stalled organization issuer no longer delays assertion issuance for a hosted community, and vice versa. Signed-off-by: Tom Brow <tomb@squareup.com>
clearIntent and cancel now raise BrowserAuthFailure.storage instead of a raw platform exception, matching the other secure-storage operations. Signed-off-by: Tom Brow <tomb@squareup.com>
An exchange from a cancelled attempt no longer changes a newer sign-in. Storage failures after a successful exchange or during cancel leave the busy state. An unreadable saved organization session is reported in the banner and replaced by the next sign-in, without touching the hosted account or community keys. Signed-off-by: Tom Brow <tomb@squareup.com>
The hosted reconnect banner now nests its own Stack, so the layer that lifts banners above Home's tab bar must expand; otherwise its bottom-anchored buttons fall outside hit-testable bounds. Signed-off-by: Tom Brow <tomb@squareup.com>
Check the sign-in attempt inside the serialized exchange, just before the session is written, so a cancelled exchange that succeeds late cannot replace a newer sign-in. Mark sign-in busy before clearing a session so a second tap cannot start another attempt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
Organization and hosted issuance run in parallel. Order failures so one that started before another relay's unresolved failure was recorded cannot replace it and hide that relay's recovery actions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
The test skips without BUZZ_AUTH_DOMAINS, so add it to the configured domains step. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
ca0667a to
b18807e
Compare
salman1993
left a comment
There was a problem hiding this comment.
🤖 Stamped by Salman's agent at his request.
Stacked on #8700. Review only the last commit until #8700 merges. NIP-FI relays can publish a relay-signed verified name for each member who signs in with an organization assertion (kind 30382, specified in #8745 and implemented in the relay in #8774). Desktop shows these names in block/buzz-app#2005. Mobile showed only self-declared kind 0 names, so a verified Block name looked no different from one anyone could set. This adds verified names to mobile, following the spec and the desktop client's validation rules. **What people see** - A verified member's name is the issuer-verified display name wherever mobile resolves names through identity names (message headers, threads, DM titles, member lists). - Message headers and the profile sheet show a verification mark. The profile sheet also shows the full name when present, "Verified by {issuer}", "Last verified {time ago}", and marks the record as cached when it hasn't been reconciled with the relay on this connection. Verification is described as historical, never as current access. - Nothing changes on relays that don't advertise `federated_identity.public_profile`, and marks never come from kind 0 or NIP-05. **How it works** - NIP-11 discovery decides whether marks may show: a successful response without `public_profile` hides them (cached records are kept), and a failed or malformed response keeps the last state. Discovery refreshes on connect and every 5 minutes while names are tracked. - Subjects are tracked as the profile cache resolves them, and fetched in batches of up to 128 with `{kinds:[30382], authors:[self], #d:[...]}`. Live updates and retractions apply on the same subscription. A full batch that omits subjects is not treated as proof they have no record. - Each event's id and signature are verified (in an isolate) before use, and it must be authored by the community's NIP-11 `self` key with exactly the specified tags (plus the optional `full_name` the relay emits). Names follow the spec's NFC and character rules; `unorm_dart` (MIT, pure Dart) is added for the NFC check because Dart has no built-in normalizer. - The cache is per community and is cleared when the relay's `self` key changes. **Not yet covered** - Some surfaces still read the kind 0 name directly: the members sheet initial, and search and inbox rows. The mark isn't shown in search rows, inbox rows, or member tiles. (The mention picker now matches, ranks and inserts by the verified name and shows the mark, and it also offers cached verified people outside the channel whose kind 0 name differs.) - NIP-FI says clients MUST show `issuer_label` beside every verified name. Here it appears only in the mark's tooltip and accessibility label and in the profile sheet, and verified names reaching DM titles and member lists show no mark or issuer. Follow-up: show the issuer inline wherever a verified name is shown. - Live updates are coalesced to the newest per subject before signature proof, ordered by unverified `created_at`/`id`. Only a misbehaving relay could exploit this (the subscription is `authors:[self]`), and it could already withhold events. Follow-up: prove first, then coalesce among valid events. - The verified `username` isn't shown beside the name in message headers (the NIP-05 handle still is). - Collision qualification relies on the existing name-disambiguation policy rather than the spec's exact self-declared-name rule. ### Validation Added tests: - Validation: discovery enabled, disabled and malformed; wrong author; mismatched or duplicate `d`/`p`; extra or duplicate tags; invalid usernames; non-NFC and whitespace-padded names; `verified_at` after `created_at`; bad signatures; retractions. - Directory: discovery gating, batching, truncation, live retraction, reconnect reconciliation, and disable hiding cached records. - Directory failures: a pre-EOSE flood, failed proof retried on refresh, oversized events (by total size, id or signature) dropped before proof, live events coalesced, a retry before or after the first EOSE never proving absence, a retry during proof not promoting records to checked, and a newer signed malformed projection clearing the older one. - Mention picker: a verified name finds, ranks and inserts the verified person; the self-declared name still finds them; a verified person outside the channel is found by verified name even when an impostor copies that name into kind 0. - Identity names: a verified name outranks the kind 0 name. - UI: the mark in message headers and the profile sheet details, no mark for a cached record, and a single accessible label. Full `mobile-check` and `mobile-test` pass. **Live relay (checked 2026-11-01):** not yet run end to end. - Staging now advertises the feature: the bb-block staging relay's NIP-11 (`buzz.block.builderlabstaging.xyz`) has `federated_identity.public_profile` with `kind: 30382` and `multiple_issuers: false`, bound to `self` `f7f4df94…dcf1`. So #8774 is deployed and enabled there. The other staging relays (`communities.public.builderlabstaging.xyz` and its `team.` host) advertise no `federated_identity`, and the client shows nothing on them. - I couldn't confirm that real 30382 events are served or render them. The relay requires NIP-42 auth plus a client-attached NIP-FI assertion, which needs an interactive Builderlab/Okta sign-in on a physical device. SSO doesn't work in the iOS simulator. The remaining step is a device run signed in to the staging Block community, with screenshots of a verified name, its mark and its details. **Shipping behind discovery gating:** this is safe. The client reads and shows verified names only when a successful NIP-11 discovery advertises `public_profile` bound to `self`. It accepts only kind 30382 events signed by that key that pass id and signature verification and strict shape checks. On every relay without the feature, which is every relay but bb-block staging today, it shows nothing new and changes no names. A failed or malformed discovery never turns the feature on, and it marks cached records as unchecked, which hides their mark. The subscription and reconciliation paths are covered by the tests above, and Carl's review reran retry probes through the real `RelaySessionNotifier`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Tom Brow <tomb@squareup.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
## Summary Saved accounts awaiting screening can report `account_status: ACCOUNT_STATUS_PENDING` while the deprecated `account_standing` still says details are missing. Treat only `ACCOUNT_STATUS_NEEDS_DETAILS` as requiring browser setup, so resuming those accounts does not reopen `/auth/native`. Product access remains enforced by the backend. Stacked on #8712 at `36d2bccc3ade84beb9f61723fa343ef0c6b8d45d`; land #8700 and #8712 first, then retarget/rebase this PR onto main. Do not merge into the parent feature branch. Companion pending-screening callback fix: squareup/cash-server#144323. ## Validation Extended the existing saved-account invite journey to cover pending screening, retained consent across restart, and no browser launch. Existing fresh and saved-incomplete journeys use the current account-status wire contract. All 21 hosted journeys passed in an isolated control; restoring the old production predicate fails the pending-account journey (one browser launch instead of zero). The account client test file also passed (11 tests). Mandatory pre-push checks passed at `ac6920635`: mobile analysis, 4,005 default Flutter tests (29 skipped), four unconfigured-push tests and 27 secp256k1 tests. Draft pending review, CI, and real-device/human acceptance. Local synthetic journeys do not prove live screening or native browser behavior. Verify a saved pending account retains its invite without reopening setup, and an account genuinely missing details still enters setup. Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz> (cherry picked from commit bfb2aa5) Signed-off-by: Tom Brow <tomb@squareup.com>
…8914) On main, "or sign in with email" on iOS fails immediately with "We could not finish connecting. You can try again." No Builderlab page opens. [#8700](#8700) changed the iOS browser bridge so the public account hosts (`app.builderlab.xyz`, `app.builderlabstaging.xyz`) may only open `/auth/native`. The Dart side still opens `/api/goose/v1/auth/login` for them, so the bridge rejects every public sign-in before presenting a browser. Every iOS build since #8700 is affected; organization sign-in on tenant hosts is not. The matching Dart change was Wes's [#8712](#8712). It was merged into the #8700 branch, but main's squash of #8700 kept only the bridge half. This PR restores #8712 on top of main: - The public account opens `/auth/native` (state, S256 challenge, callback and retained invite unchanged, without the CLI-only `type`/`product` parameters). Organization tenants keep `/api/goose/v1/auth/login`. - A saved session whose account still needs web setup goes through the same browser handoff, keeping its subject and destination. Android has no path allowlist in its browser plugin, so it needs no change. ### Validation - Reproduced on an iPhone 17 Pro (iOS 27) simulator with a main-based debug build: "or sign in with email" fails instantly. With this change applied, the same build shows the system "Buzz wants to use app.builderlab.xyz to sign in" prompt, then the login.builderlab.xyz "Welcome back to Buzz" page, which confirms production serves `/auth/native`. - Restored #8712's tests on top of main's current tests: the browser test checks the public login URL and its exact parameters, and the hosted invite journey runs for both a fresh account and a saved account that still needs setup, asserting `/auth/native` and that the invite is retained. Without the Dart fix, these fail. - Mobile format and analyze pass; the hosted onboarding journeys and `shared/hosted` tests pass with the CI auth-domain configuration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Tom Brow <tomb@squareup.com> Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
The Block tenant links a member's existing key to their account the first time it issues them a relay assertion ([#8700](#8700)). Mobile never backed that key up. Desktop FI builds back up to the tenant, but on mobile a Block key that lives on just one phone can't be recovered once Block turns backups on. Mobile now backs up the organization relay's key to the organization account: - After every organization assertion, it reads `/v1/buzz/nsec-backups/status`. If the key is already listed, there's nothing to do. - If the bound key isn't listed, it enrolls, seals with HPKE and uploads, using the existing `HostedBackupService`. That service now uses its account client's own origin, so organization and hosted backups never share a pending envelope. Hosted storage keys and receipts don't change. - If the upload's outcome is unknown, the saved envelope is replayed exactly at the next assertion. - A tenant without backups answers 404 (Block production today). That, and other definitive refusals, stop attempts for that account and key until the next launch. Any other failure is retried at a later assertion, after a delay that doubles from 1 minute to at most 1 hour. Assertions renew while the app runs, so retries continue until the tenant lists the key. - Concurrent assertions share one attempt. - iOS 16 can't seal, so it sends nothing. Backup failures never affect the relay connection. There's no UI. As in mobile's hosted onboarding, the backup happens automatically. Desktop asks for an explicit backup choice first, so whether mobile should too is a product call. ### Validation Added tests (`organization_key_backup_test.dart`), which run through real `HostedRelayAuth` assertion issuance with only HTTP faked: - An uncertain upload is replayed with the same envelope at the next assertion, then nothing more is sent in that launch. - A key that's already listed sends no upload and drops a leftover envelope. - A tenant without backups is asked once per launch. - A device that can't seal sends nothing. - A failed status read is retried after the delay, not at the immediate re-assertion. A key the tenant bound to someone else is never uploaded, and its checks keep going at growing intervals. - Two concurrent assertions make one attempt. - A failing seal-capability check sends nothing. Removing the assertion hook fails 4 of the 6 tests. Removing the shared attempt or the retry delay fails its test. Full `mobile-check` and `mobile-test` pass. Device (iPhone 12 mini, release build) against bb-block staging, where backups are configured and the relay enforces NIP-FI, with an existing Block SSO session: - On launch the app asserts, reads backup status and finds this key listed, so it uploads nothing. The relay connection and channel list are unaffected. - A test-only build that ignored the listed shortcut enrolled, sealed with the staging tenant's real KMS-backed key and uploaded, with a stored response in about 360 ms. That re-stored my own test key. - The final build at this head was then reinstalled and relaunched with no backup failure logged. The device's staging community was added in an earlier run, because adding an enforcing organization relay by link is still blocked ([#8791](#8791)). An independent agent review ran; its should-fix items are addressed in the second commit. Codex's retry finding is addressed in the third, which changes only failure-path timing; the device-tested success path is unchanged. Wes's optional review notes are addressed in the last three: an exact-timing retry test, backup queues per account origin (a stalled organization backup no longer delays hosted onboarding's backup, with a test), and a subject check on each enrollment and upload request. Full `mobile-check` and `mobile-test` pass at the head. Human testing is waived under the mobile automated-testing policy; the device run above covers the at-risk flows (organization assertion, relay connection, backup status, enrollment, sealing and upload). buzz-review-completed 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Tom Brow <tomb@squareup.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Block is turning on NIP-FI enforcement on the Block relay. Desktop gets organization sign-in in block/buzz-app#1424. Mobile can already attach NIP-FI assertions, but only for Builderlab-hosted communities, through the build's single Builderlab account. The dogfood build needs that account for public hosted communities, so it can't simply be pointed at the Block tenant.
This adds a separate organization realm, configured at build time:
BUZZ_ORGANIZATION_URL: the tenant's HTTPS origin (for examplehttps://block.builderlab.xyz).BUZZ_ORGANIZATION_RELAYS: the exact relay hosts that trust that tenant.The two must be set together. The app refuses to start if they're malformed, overlap a hosted community, or reuse the hosted origin. Without them, nothing changes.
For a configured relay:
/api/goose/v1/auth/login). Block SSO happens there./v1/buzz/identity/assertionsroute that hosted communities use, and attaches them to the relay socket, HTTP, media, huddle and remote-agent requests.The tenant accepts only the registered
xyz.block.buzz.mobile:/auth/callbackreturn URL, so both realms share it. EachBrowserAuthowner leaves callbacks whosestatebelongs to another owner's attempt to that owner.Before merge
Validation
Added tests (
organization_relay_auth_test.dart):Full
mobile-checkandmobile-testpass. RunnerTests pass locally excepttestNativeNavigationTitleExpandsAgainAtTop, which this PR doesn't touch.Added tests (
organization_sign_in_test.dart,relay_auth_isolation_test.dart,builderlab_account_test.dart), each failing without its fix:BUZZ_AUTH_DOMAINSand skips without it. CI's configured-auth-domains step doesn't list it yet, because that workflow change needs a token withworkflowscope.Added tests also cover a restore after an offline start (the banner clears once a relay retry restores the session) and the iOS bridge allowlist (
/auth/nativeand tenant login on Builderlab subdomains; other hosts, paths, ports, userinfo and fragments refused).Simulator (iPhone 17, iOS 27) with the dogfood defines against the production Block relay
buzz.block.builderlab.xyz, on a WARP Mac:The app starts with the organization configuration and connects to the relay while signed out, showing the organization Sign in banner.
Sign in opens the system sign-in session for
block.builderlab.xyz, and the banner shows Finishing organization sign-in…. Cancelling returns to Sign in with no error. Before the bridge fix, no browser opened and the banner reported that sign-in did not finish.The bottom banners (organization sign-in, hosted reconnect, and the sign-in recovery row) now sit above Home's floating Search/New bar, which used to cover Sign in. A journey test checks the banner clears the bar, and fails without the fix.
The simulator run stopped at the system consent prompt (SSO does not work in the simulator). The app calls were driven through the VM service (the same
signIn()/cancel()the banner buttons call) because the simulator could not be tapped.Device (iPhone 12 mini) against bb-block staging in enforce mode, where every probe without an assertion gets 401:
Known limitation: adding an enforcing organization relay by community link is tracked in #8791.
The dogfood build enables this in squareup/buzz-releases (
tomb/dogfood-organization-sign-in). App Store builds stay unchanged.🤖 Generated with Claude Code