fix(auth): extension CoinPay sign-in never picked up the session - #49
Merged
Conversation
Clicking "Sign in with CoinPay" in the extension's options page signed the
WEBSITE in and left the extension signed out.
coinpay-auth.js asked the backend to redirect back to
chrome.identity.getRedirectURL() -> https://<ext-id>.chromiumapp.org/.
That target is off-origin, so safeRedirect() rejected it (correctly — the
session token rides in the redirect fragment) and /api/auth/coinpay/callback
fell through to `c.redirect(`${APP_URL}/?signedin=1`)`. The OAuth dance
completed, tronbrowser.dev got its tb_session cookie, and the extension got
nothing. (Ungoogled Chromium also rewrites chromiumapp.org to a
non-resolving .qjz9zk host, so launchWebAuthFlow could not have worked
either.) bittorrented.js already worked around this with ext-callback;
CoinPay never got the same treatment.
- Add GET /api/auth/ext-login: validates an on-origin callback, and when the
user is ALREADY signed in on the website hands a session straight back
instead of running CoinPay a second time. Otherwise forwards to the
CoinPay dance preserving the callback.
- coinpaySignIn() opens that URL in a tab and waits for the token, matching
the bittorrented flow. Listener is registered before the tab is created so
a fast redirect can't race it.
- ext-callback.js now serves both flows, keyed on `?src=tb`, and background
handles the generalized 'ext-token' message.
- Drop the now-unused "identity" permission from the manifest.
Redirect targets stay same-origin only, so a drive-by navigation to
/api/auth/ext-login lands the token in a fragment the navigating site
cannot read.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vu1nz Security Review0 finding(s) in PR #? No security issues found. |
ralyodio
marked this pull request as ready for review
July 31, 2026 03:24
…ured parts CI has been red on main since #48: noUncheckedIndexedAccess types the destructured `label`/`tld` as `string | undefined`, because TS can't narrow an array to a 2-tuple from a `parts.length !== 2` check. Four TS2345/TS2322 errors failed `apps/desktop` and took the whole `pnpm -r build` with it. Guard explicitly. Behavior is unchanged: length 2 already guarantees both values, and an empty label would fail LABEL.test() on the next line anyway. Unrelated to this branch's auth work, but it blocks the PR from going green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merged
ralyodio
added a commit
that referenced
this pull request
Jul 31, 2026
Ships the extension CoinPay sign-in fix (#49): the extension asked the backend to redirect to an off-origin chromiumapp.org callback, safeRedirect() rejected it, and only the website ended up signed in. Bumps every workspace package, the Expo app, and the AI sidebar extension manifest to 3.8.4 via scripts/set-version.mjs. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Clicking Sign in with CoinPay in the extension's options page (
chrome-extension://…/options.html) signs the website in and leaves the extension signed out — exactly the reported "tronbrowser.dev is authed but not the web extension".Root cause
coinpay-auth.jsasked the backend to redirect back tochrome.identity.getRedirectURL()→https://<ext-id>.chromiumapp.org/.That target is off-origin, so
safeRedirect()rejected it — correctly, since the session token rides in the redirect fragment. With the redirect dropped,/api/auth/coinpay/callbackfell through toc.redirect(${APP_URL}/?signedin=1). The OAuth dance completed,tronbrowser.devgot itstb_sessioncookie, and the extension got nothing.Ungoogled Chromium independently rewrites
chromiumapp.orgto a non-resolving.qjz9zkhost, solaunchWebAuthFlowcould not have worked either.bittorrented.jsalready worked around this viaext-callback; CoinPay never got the same treatment.The fix
GET /api/auth/ext-login— validates an on-origin callback, and when the user is already signed in on the website hands a session straight back rather than running CoinPay a second time. Otherwise forwards to the CoinPay dance, preserving the callback.coinpaySignIn()opens that URL in a normal tab and waits for the token, matching the bittorrented flow. The storage listener is registered before the tab is created so a fast redirect can't race it.ext-callback.jsnow serves both flows, keyed on?src=tb(tbAuthTokenvsbtrToken);background.jshandles the generalizedext-tokenmessage.identitypermission from the manifest.Decision logic lives in
services/api/src/ext-login.tsso it's unit-testable —index.tscallsserve()at import and can't be imported from a test.Security
Redirect targets remain same-origin only. A drive-by top-level navigation to
/api/auth/ext-loginmints a token that lands in the fragment of a URL on our origin, which the navigating site cannot read. Covered by tests, including an explicit case rejecting thechromiumapp.orgURL that caused this.Verification
vitest runinservices/api— 57 passed (9 files), including 5 newext-loginteststsc --noEmit— cleaneslinton all changed files — 0 errors (8 pre-existinganywarnings on untouched lines)Not verified: a live browser round-trip through CoinPay — that needs real OAuth credentials.
Note for the release
This is unreleased, so
tronbrowser updatecorrectly reports "already up to date (v3.8.3)" — there is nothing newer to pull yet. This needs to ship in v3.8.4 before it reaches anyone.Separately worth a look:
COINPAY_REDIRECT_URIdefaults tohttps://tronbrowser**.com**/api/auth/coinpay/callback(services/api/src/index.ts:20) whileAPP_URLdefaults totronbrowser.dev. Left alone here since production presumably sets the env var, but if it doesn't, that's a second broken hop.🤖 Generated with Claude Code
Second commit: unbreaking CI (unrelated to the auth fix)
buildwas already failing on main before this branch existed — CI went red at5bad141(#48, Moshpit resolve); #47 before it was green.noUncheckedIndexedAccesstypes the destructuredlabel/tldinparseRegistryNameasstring | undefined, because TS can't narrow an array to a 2-tuple from aparts.length !== 2check. Four TS2345/TS2322 errors failedapps/desktopand took the wholepnpm -r builddown with it.Fixed with an explicit guard. Behavior is unchanged: length 2 already guarantees both values exist, and an empty label would fail
LABEL.test()on the very next line.It's scope bleed, but the PR could not go green without it — and neither could anything else landing on main.
Full workspace verification after both commits:
pnpm -r build— exit 0pnpm -r test— exit 0, 22 packagespnpm lint— exit 0 (warnings only, all pre-existing)buildpass (first green build since feat(resolve): choose whether Moshpit overrides clearnet for a contested name #48),packagepass, security review pass, Socket pass