Conversation
Add chrome-extension://, moz-extension://, safari-extension://, safari-web-extension://, and extension:// to the protocol allow-list so browser extensions can obtain app tokens via /auth/get-user-app-token. Extract WEB_AND_EXTENSION_PROTOCOLS constant in validation.js so the allow-list is defined once and shared by AuthService, AppStore, and AppDriver.
Member
|
looks sane, just reviewing now |
Review follow-ups on the extension-origin change: - Require a host in `validateUrl`. Only "special" schemes need an authority, so `chrome-extension:` parsed with an empty hostname and slipped past the reserved-system-host guard in AppDriver. - Lowercase the host in `AuthService#normalizedOrigin`. `new URL()` lowercases http(s) hosts but leaves opaque ones alone, so one extension in two spellings resolved to two app uids — two AppData trees, two permission sets — and missed the origin blocklist. - Drop `extension:`. No browser emits it, and it accepted `extension://evil.com` as an app origin. - Freeze the allow-list and derive `validateUrl`'s http(s) default from `WEB_PROTOCOLS` so the two spellings can't drift apart. - Pin the tests to the real uid derivation, use unique extension ids so a row left by another test can't mask the bootstrap path, and cover the host-less and near-miss schemes. - Fix the prettier/eslint failure in AppStore.js.
This branch has not been deployed
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.
feat: allow browser extension origins in auth requests
Problem
Browser extensions sending requests to
/auth/get-user-app-tokenwith their native origin (e.g.chrome-extension://cafneielldmiliebnkhaeaaibinihgpb) receive a 400:{ "error": "Invalid origin URL", "message": "Invalid origin URL", "code": "bad_request" }This happens because
AuthService.#originFromUrlonly acceptedhttp:andhttps:protocols. Browser extensions are legitimate OAuth-style clients and should be able to obtain app tokens the same way web origins do.Changes
Extend the protocol allow-list from
['http:', 'https:']to include browser extension schemes:chrome-extension:moz-extension:safari-extension:safari-web-extension:extension:The allow-list is used in three places - all three are updated consistently:
AuthService.#originFromUrl- gatekeepsappUidFromOrigin(), which is the entry point that returned the errorAppStore.createFromOrigin- validatesoriginbefore persisting it asindex_urlon bootstrap app rowsAppDriver.#validateInput- validatesindex_urlon developer-created/updated app rowsDRY extraction
The protocol array was duplicated across all three files. This PR extracts it into a single exported constant:
All three consumers now import and reference this constant.
Security
javascript:,data:,file:,vbscript:) remain rejected - the existing negative tests confirm this.hostnameguard.AuthService.#originFromUrlchecksparsed.hostnameis non-empty after the protocol check. All extension schemes produce a hostname (the extension ID), so this passes correctly while still blocking edge-case schemes that parse without a host.How existing callers are unaffected
http:andhttps:origins work identically - the allow-list is a strict superset.validateUrlutility's defaultprotocolsparameter (['http:', 'https:']) is unchanged; only callers that explicitly pass the extended list are affected.Tests
AuthService.test.tsappUidFromOriginresolves 5 extension origins to deterministic app UIDsAppStore.test.jscreateFromOriginaccepts achrome-extension://origin and persists itAuthController.test.tshandleGetUserAppTokenend-to-end with an extension origin -> token + bootstrap row