feat(extension-auth): accept Firefox identity redirect hosts for extension auth#1999
feat(extension-auth): accept Firefox identity redirect hosts for extension auth#1999ManthanNimodiya wants to merge 1 commit into
Conversation
| ); | ||
| const configuredExtensionId = serverEnv().CAP_CHROME_EXTENSION_ID; | ||
| const extensionId = url.hostname.slice(0, -identityHost.suffix.length); | ||
| const configuredExtensionId = identityHost.getConfiguredExtensionId(); |
There was a problem hiding this comment.
Mixed-Case Firefox Id Rejection
URL.hostname is normalized to lowercase before extensionId is sliced, but the new Firefox id comes straight from CAP_FIREFOX_EXTENSION_ID. If that env var is copied with uppercase characters, the configured-id check rejects the real Firefox redirect host and the extension auth flow always returns a bad request.
| const configuredExtensionId = identityHost.getConfiguredExtensionId(); | |
| const configuredExtensionId = identityHost | |
| .getConfiguredExtensionId() | |
| ?.toLowerCase(); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/web-backend/src/Extension/Http.ts
Line: 51
Comment:
**Mixed-Case Firefox Id Rejection**
`URL.hostname` is normalized to lowercase before `extensionId` is sliced, but the new Firefox id comes straight from `CAP_FIREFOX_EXTENSION_ID`. If that env var is copied with uppercase characters, the configured-id check rejects the real Firefox redirect host and the extension auth flow always returns a bad request.
```suggestion
const configuredExtensionId = identityHost
.getConfiguredExtensionId()
?.toLowerCase();
```
How can I resolve this? If you propose a fix, please make it concise.| const identityHost = IDENTITY_REDIRECT_HOSTS.find(({ suffix }) => | ||
| url.hostname.endsWith(suffix), | ||
| ); | ||
| if (url.protocol !== "https:" || !identityHost) { |
There was a problem hiding this comment.
Might be worth rejecting non-default ports / credentials here to keep the redirect URI shape tight (and avoid surprises if browser behavior changes).
| if (url.protocol !== "https:" || !identityHost) { | |
| if ( | |
| url.protocol !== "https:" || | |
| url.port !== "" || | |
| url.username || | |
| url.password || | |
| !identityHost | |
| ) { | |
| return yield* new HttpApiError.BadRequest(); | |
| } |
| const extensionId = url.hostname.slice(0, -identityHost.suffix.length); | ||
| const configuredExtensionId = identityHost.getConfiguredExtensionId(); |
There was a problem hiding this comment.
Since the comment says the leading label identifies the extension installation, it could be good to explicitly reject multi-label hosts (e.g. a.b.chromiumapp.org).
| const extensionId = url.hostname.slice(0, -identityHost.suffix.length); | |
| const configuredExtensionId = identityHost.getConfiguredExtensionId(); | |
| const extensionId = url.hostname.slice(0, -identityHost.suffix.length); | |
| if (extensionId.includes(".")) { | |
| return yield* new HttpApiError.BadRequest(); | |
| } | |
| const configuredExtensionId = identityHost.getConfiguredExtensionId(); |
Firefox's identity.launchWebAuthFlow redirects to https://.extensions.allizom.org/.
Generalizes validateExtensionRedirectUri to a host-suffix table and adds optional CAP_FIREFOX_EXTENSION_ID (packages/env).
Chrome pinning and the localhost-dev escape hatch unchanged. Independent of the other PRs.
(For production later: the hash for gecko id extension@cap.so is 324e9362f26688ae2eb9a75d1beae53ef6b8d0d2.)
Greptile Summary
This PR adds Firefox identity redirect support to extension auth. The main changes are:
CAP_FIREFOX_EXTENSION_IDto server env validation.Confidence Score: 5/5
This looks safe to merge after a small config-normalization cleanup.
packages/web-backend/src/Extension/Http.ts
Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(extension-auth): accept Firefox ide..." | Re-trigger Greptile
Context used: