mirror of
https://github.com/HeyPuter/puter.git
synced 2026-10-02 18:08:13 +00:00
feat: allow browser extension origins in auth requests (#3907)
* feat: allow browser extension origins in auth requests 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. * fix: harden the extension-origin allow-list 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. --------- Co-authored-by: Daniel Salazar <daniel.salazar@puter.com>
This commit is contained in:
co-authored by
Daniel Salazar
parent
606f5a27a3
commit
9f69483593
@@ -3119,6 +3119,27 @@ describe('AuthController.handleGetUserAppToken + handleCheckApp', () => {
|
||||
expect(bootstrapped).toBeTruthy();
|
||||
expect(bootstrapped?.owner_user_id).toBe(owner!.id);
|
||||
});
|
||||
|
||||
it('supports browser extension origins in handleGetUserAppToken', async () => {
|
||||
// Random id: a pre-existing row for this origin would resolve through
|
||||
// the canonical lookup and never exercise the bootstrap path.
|
||||
const origin = `chrome-extension://${uuidv4()}`;
|
||||
const res = makeRes();
|
||||
await inCtx(actor, () =>
|
||||
controller.handleGetUserAppToken(
|
||||
makeReq({ origin }, { actor }),
|
||||
res,
|
||||
),
|
||||
);
|
||||
const body = res.body as { token: string; app_uid: string };
|
||||
expect(body.app_uid).toBe(
|
||||
`app-${uuidv5(origin, APP_ORIGIN_UUID_NAMESPACE)}`,
|
||||
);
|
||||
const bootstrapped = await server.stores.app.getByUid(body.app_uid);
|
||||
expect(bootstrapped?.index_url).toBe(origin);
|
||||
// Proves the row came from the bootstrap path, not an earlier test.
|
||||
expect(bootstrapped?.description).toMatch(/^App created from origin /);
|
||||
});
|
||||
});
|
||||
|
||||
// ── Access tokens: create + revoke ─────────────────────────────────
|
||||
|
||||
@@ -48,6 +48,7 @@ import {
|
||||
validateJsonObject,
|
||||
validateString,
|
||||
validateUrl,
|
||||
WEB_AND_EXTENSION_PROTOCOLS,
|
||||
} from '../../util/validation.js';
|
||||
import { PuterDriver } from '../types.js';
|
||||
|
||||
@@ -632,6 +633,7 @@ export class AppDriver extends PuterDriver {
|
||||
key: 'index_url',
|
||||
maxLen: 3000,
|
||||
required: isCreate,
|
||||
protocols: WEB_AND_EXTENSION_PROTOCOLS,
|
||||
});
|
||||
// Only enforce on new/changed values so rows that already
|
||||
// carry a reserved host (migration-seeded builtins) can still
|
||||
|
||||
@@ -18,7 +18,7 @@
|
||||
*/
|
||||
|
||||
import jwt from 'jsonwebtoken';
|
||||
import { v4 as uuidv4 } from 'uuid';
|
||||
import { v4 as uuidv4, v5 as uuidv5 } from 'uuid';
|
||||
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
|
||||
import { makeActor, type Actor } from '../../core/actor.js';
|
||||
import { PuterServer } from '../../server.js';
|
||||
@@ -1631,14 +1631,81 @@ describe('AuthService (integration)', () => {
|
||||
'data:text/html,<script>alert(1)</script>',
|
||||
'file:///etc/passwd',
|
||||
'vbscript:msgbox(1)',
|
||||
])('throws 400 for non-http(s) scheme %s', async (origin) => {
|
||||
// These parse fine via `new URL()` but must never become a
|
||||
// bootstrap app `index_url` — that would be a stored XSS /
|
||||
// code-execution vector when launched as `iframe.src`.
|
||||
await expect(
|
||||
authService.appUidFromOrigin(origin),
|
||||
).rejects.toMatchObject({ statusCode: 400 });
|
||||
])(
|
||||
'throws 400 for unsafe non-web/non-extension scheme %s',
|
||||
async (origin) => {
|
||||
// These parse fine via `new URL()` but must never become a
|
||||
// bootstrap app `index_url` — that would be a stored XSS /
|
||||
// code-execution vector when launched as `iframe.src`.
|
||||
await expect(
|
||||
authService.appUidFromOrigin(origin),
|
||||
).rejects.toMatchObject({ statusCode: 400 });
|
||||
},
|
||||
);
|
||||
|
||||
// Mirrors APP_ORIGIN_UUID_NAMESPACE in AuthService: changing it
|
||||
// changes every origin-derived app uid in the fleet.
|
||||
const APP_ORIGIN_UUID_NAMESPACE =
|
||||
'33de3768-8ee0-43e9-9e73-db192b97a5d8';
|
||||
// Unique per run — a real `apps` row carrying the same origin as its
|
||||
// `index_url` resolves ahead of the uuidv5 fallback.
|
||||
const extensionId = uuidv4();
|
||||
|
||||
it.each([
|
||||
`chrome-extension://${extensionId}`,
|
||||
`moz-extension://${extensionId}`,
|
||||
`safari-extension://${extensionId}`,
|
||||
`safari-web-extension://${extensionId}`,
|
||||
])('derives the app uid for extension origin %s', async (origin) => {
|
||||
const uid = await authService.appUidFromOrigin(origin);
|
||||
expect(uid).toBe(
|
||||
`app-${uuidv5(origin, APP_ORIGIN_UUID_NAMESPACE)}`,
|
||||
);
|
||||
expect(await authService.appUidFromOrigin(origin)).toBe(uid);
|
||||
});
|
||||
|
||||
it('gives two extensions two different app uids', async () => {
|
||||
const a = await authService.appUidFromOrigin(
|
||||
`chrome-extension://${uuidv4()}`,
|
||||
);
|
||||
const b = await authService.appUidFromOrigin(
|
||||
`chrome-extension://${uuidv4()}`,
|
||||
);
|
||||
expect(a).not.toBe(b);
|
||||
});
|
||||
|
||||
it('resolves an extension id case-insensitively', async () => {
|
||||
// `new URL()` lowercases http(s) hosts but leaves opaque ones
|
||||
// alone, so without normalization one extension would get two
|
||||
// uids, two AppData trees and two permission sets.
|
||||
const id = uuidv4();
|
||||
expect(
|
||||
await authService.appUidFromOrigin(
|
||||
`chrome-extension://${id.toUpperCase()}`,
|
||||
),
|
||||
).toBe(
|
||||
await authService.appUidFromOrigin(`chrome-extension://${id}`),
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
// Extension schemes are not "special", so `new URL()` accepts them
|
||||
// with no authority — which would slip past every host-based guard.
|
||||
'chrome-extension:',
|
||||
'moz-extension:',
|
||||
// Near-misses: no browser emits any of these.
|
||||
'extension://my-extension-id',
|
||||
'web-extension://my-extension-id',
|
||||
'ms-browser-extension://my-extension-id',
|
||||
'chrome-extensions://my-extension-id',
|
||||
])(
|
||||
'throws 400 for host-less or non-browser scheme %s',
|
||||
async (origin) => {
|
||||
await expect(
|
||||
authService.appUidFromOrigin(origin),
|
||||
).rejects.toMatchObject({ statusCode: 400 });
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe('subdomainOwnerIdFromOrigin', () => {
|
||||
|
||||
@@ -26,6 +26,7 @@ import {
|
||||
type Actor,
|
||||
} from '../../core/actor';
|
||||
import { HttpError } from '../../core/http/HttpError.js';
|
||||
import { WEB_AND_EXTENSION_PROTOCOLS } from '../../util/validation.js';
|
||||
import {
|
||||
ASSET_WINDOW_SECONDS,
|
||||
WEB_WINDOW_SECONDS,
|
||||
@@ -915,10 +916,13 @@ export class AuthService extends PuterService {
|
||||
return this.#normalizedOrigin(parsed);
|
||||
}
|
||||
|
||||
/** Scheme + host + explicit port, with no trailing separator. */
|
||||
/** Scheme + lowercased host + explicit port, with no trailing separator. */
|
||||
#normalizedOrigin(parsed: URL): string {
|
||||
const port = parsed.port ? `:${parsed.port}` : '';
|
||||
return `${parsed.protocol}//${parsed.hostname}${port}`;
|
||||
// `new URL()` lowercases http(s) hosts but leaves opaque ones alone, so
|
||||
// without this an extension id in two spellings hashes to two app uids
|
||||
// (and misses the blocklist, which matches on a lowercased host).
|
||||
return `${parsed.protocol}//${parsed.hostname.toLowerCase()}${port}`;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1786,14 +1790,15 @@ export class AuthService extends PuterService {
|
||||
#originFromUrl(url: string): string | null {
|
||||
try {
|
||||
const parsed = new URL(url);
|
||||
// A real web origin is always http(s). `new URL()` happily parses
|
||||
// `javascript:`, `data:`, `file:`, `vbscript:`, etc.; if one of
|
||||
// those slips through it ends up persisted as an app `index_url`
|
||||
// (see AppStore.createFromOrigin) and later loaded as `iframe.src`
|
||||
// — an XSS/code-execution primitive. Reject anything that isn't
|
||||
// http(s) so the bootstrap path matches AppDriver's validateUrl
|
||||
// allow-list.
|
||||
if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
|
||||
// This gets persisted as an app `index_url` and later loaded as
|
||||
// `iframe.src` (see AppStore.createFromOrigin), so anything outside
|
||||
// the allow-list is a stored code-execution vector.
|
||||
if (!WEB_AND_EXTENSION_PROTOCOLS.includes(parsed.protocol)) {
|
||||
return null;
|
||||
}
|
||||
// Extension schemes aren't "special", so `new URL()` accepts them
|
||||
// with no authority at all (`chrome-extension:`).
|
||||
if (!parsed.hostname) {
|
||||
return null;
|
||||
}
|
||||
return this.#normalizedOrigin(parsed);
|
||||
|
||||
@@ -21,7 +21,10 @@ import { v4 as uuidv4 } from 'uuid';
|
||||
import { PuterStore } from '../types';
|
||||
import { HttpError } from '../../core/http/HttpError.js';
|
||||
import { isUniqueViolation } from '../../util/dbError.js';
|
||||
import { validateUrl } from '../../util/validation.js';
|
||||
import {
|
||||
validateUrl,
|
||||
WEB_AND_EXTENSION_PROTOCOLS,
|
||||
} from '../../util/validation.js';
|
||||
|
||||
/**
|
||||
* Persistence + cache for the `apps` table.
|
||||
@@ -470,11 +473,14 @@ export class AppStore extends PuterStore {
|
||||
{ ownerUserId = null } = {},
|
||||
) {
|
||||
// Bootstrap apps persist `origin` straight into `index_url`, which is
|
||||
// later loaded as `iframe.src`. Enforce the same http(s) scheme
|
||||
// allow-list as the AppDriver create/update path so a caller-supplied
|
||||
// later loaded as `iframe.src`. Enforce the same scheme allow-list
|
||||
// as the AppDriver create/update path so a caller-supplied
|
||||
// `javascript:`/`data:`/`file:` origin can never become a stored,
|
||||
// launchable code-execution vector.
|
||||
validateUrl(origin, { key: 'origin' });
|
||||
validateUrl(origin, {
|
||||
key: 'origin',
|
||||
protocols: WEB_AND_EXTENSION_PROTOCOLS,
|
||||
});
|
||||
|
||||
const fields = {
|
||||
name: uid,
|
||||
|
||||
@@ -776,6 +776,10 @@ describe('AppStore CRUD and cache invalidation', () => {
|
||||
'javascript:alert(1)',
|
||||
'data:text/html,<script>alert(1)</script>',
|
||||
'file:///etc/passwd',
|
||||
// Allow-listed scheme, but `new URL()` accepts it with no authority.
|
||||
'chrome-extension:',
|
||||
// Not a scheme any browser emits.
|
||||
'extension://my-extension-id',
|
||||
])('refuses to bootstrap an app from the %s origin', async (origin) => {
|
||||
await expect(
|
||||
appStore.createFromOrigin(
|
||||
@@ -785,6 +789,16 @@ describe('AppStore CRUD and cache invalidation', () => {
|
||||
).rejects.toMatchObject({ statusCode: 400 });
|
||||
});
|
||||
|
||||
it('creates an origin-bootstrap app for a browser extension origin', async () => {
|
||||
const uid = `app-ext-${Math.random().toString(36).slice(2, 10)}`;
|
||||
const origin = `chrome-extension://${uid}`;
|
||||
|
||||
const app = await appStore.createFromOrigin(uid, origin);
|
||||
|
||||
expect(app.uid).toBe(uid);
|
||||
expect(app.index_url).toBe(origin);
|
||||
});
|
||||
|
||||
it('returns the existing row when the deterministic uid was already inserted', async () => {
|
||||
const uid = `app-origin-${Math.random().toString(36).slice(2, 10)}`;
|
||||
const origin = `https://${uid}.example.com`;
|
||||
|
||||
@@ -24,6 +24,31 @@ import { HttpError } from '../core/http/HttpError.js';
|
||||
* ...) on failure. Returns the value on success.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Real web origins. Anything outside this list is an XSS/SSRF primitive once
|
||||
* the value is consumed as `iframe.src`, `window.location`, a server-side
|
||||
* fetch, etc. — `new URL()` alone happily parses `javascript:alert(1)`,
|
||||
* `data:text/html,…`, `file:///etc/passwd`, and `vbscript:`.
|
||||
*/
|
||||
export const WEB_PROTOCOLS = Object.freeze(['http:', 'https:']);
|
||||
|
||||
/**
|
||||
* Web origins plus the browser-extension schemes, for app origins and
|
||||
* `index_url` values. Shared by AuthService.#originFromUrl,
|
||||
* AppStore.createFromOrigin and AppDriver.#validateInput.
|
||||
*
|
||||
* Only Chrome derives the extension host from the signing key. Firefox and
|
||||
* Safari generate it per install, so one extension is a distinct origin — and
|
||||
* therefore a distinct app row — for each user who installs it.
|
||||
*/
|
||||
export const WEB_AND_EXTENSION_PROTOCOLS = Object.freeze([
|
||||
...WEB_PROTOCOLS,
|
||||
'chrome-extension:',
|
||||
'moz-extension:',
|
||||
'safari-extension:',
|
||||
'safari-web-extension:',
|
||||
]);
|
||||
|
||||
export function validateString(
|
||||
value,
|
||||
{ key, maxLen, regex, required = true, allowEmpty = false } = {},
|
||||
@@ -66,13 +91,9 @@ export function validateUrl(
|
||||
key,
|
||||
maxLen = 3000,
|
||||
required = true,
|
||||
// Default allowlist is http(s) only — anything else is an XSS/SSRF
|
||||
// primitive when the value is later consumed as `iframe.src`,
|
||||
// `window.location`, a server-side fetch, etc. `new URL()` alone
|
||||
// happily parses `javascript:alert(1)`, `data:text/html,…`,
|
||||
// `file:///etc/passwd`, and `vbscript:`; callers that need
|
||||
// something exotic must opt in explicitly.
|
||||
protocols = ['http:', 'https:'],
|
||||
// http(s) by default (see WEB_PROTOCOLS); callers that need something
|
||||
// exotic must opt in explicitly.
|
||||
protocols = WEB_PROTOCOLS,
|
||||
} = {},
|
||||
) {
|
||||
if (value === undefined || value === null) {
|
||||
@@ -98,6 +119,14 @@ export function validateUrl(
|
||||
{ legacyCode: 'bad_request' },
|
||||
);
|
||||
}
|
||||
// Only "special" schemes (http:, https:, ws:, …) require an authority.
|
||||
// `new URL()` accepts `chrome-extension:` and `extension:javascript:alert(1)`
|
||||
// with an empty host, which slips past every host-based guard downstream.
|
||||
if (!parsed.hostname) {
|
||||
throw new HttpError(400, `\`${key}\` must include a host`, {
|
||||
legacyCode: 'bad_request',
|
||||
});
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
|
||||
@@ -24,6 +24,7 @@ import {
|
||||
validateJsonObject,
|
||||
validateString,
|
||||
validateUrl,
|
||||
WEB_AND_EXTENSION_PROTOCOLS,
|
||||
} from './validation.js';
|
||||
|
||||
/**
|
||||
@@ -163,6 +164,56 @@ describe('validateUrl', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
'chrome-extension://cafneielldmiliebnkhaeaaibinihgpb',
|
||||
'moz-extension://f4b30177-3e5e-49b4-bb50-32df6ff09033',
|
||||
'safari-extension://f4b30177-3e5e-49b4-bb50-32df6ff09033',
|
||||
'safari-web-extension://f4b30177-3e5e-49b4-bb50-32df6ff09033',
|
||||
])('accepts the browser extension origin %s when opted in', (value) => {
|
||||
expect(
|
||||
validateUrl(value, {
|
||||
key: 'index_url',
|
||||
protocols: WEB_AND_EXTENSION_PROTOCOLS,
|
||||
}),
|
||||
).toBe(value);
|
||||
});
|
||||
|
||||
it.each([
|
||||
'chrome-extension:',
|
||||
'moz-extension:',
|
||||
'extension:javascript:alert(1)',
|
||||
])('rejects the host-less value %s', (value) => {
|
||||
// Only "special" schemes require an authority, so these parse with an
|
||||
// empty host and would slip past every host-based guard downstream.
|
||||
expectBadRequest(
|
||||
() =>
|
||||
validateUrl(value, {
|
||||
key: 'index_url',
|
||||
protocols: [...WEB_AND_EXTENSION_PROTOCOLS, 'extension:'],
|
||||
}),
|
||||
'`index_url` must include a host',
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
'extension://my-extension-id',
|
||||
'web-extension://my-extension-id',
|
||||
'ms-browser-extension://my-extension-id',
|
||||
])('keeps rejecting the non-browser scheme %s', (value) => {
|
||||
expectBadRequest(
|
||||
() =>
|
||||
validateUrl(value, {
|
||||
key: 'index_url',
|
||||
protocols: WEB_AND_EXTENSION_PROTOCOLS,
|
||||
}),
|
||||
'must use one of the following protocols',
|
||||
);
|
||||
});
|
||||
|
||||
it('exposes the extension allow-list as a frozen value', () => {
|
||||
expect(Object.isFrozen(WEB_AND_EXTENSION_PROTOCOLS)).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects a missing value when required, passes it through otherwise', () => {
|
||||
expectBadRequest(
|
||||
() => validateUrl(undefined, { key: 'url' }),
|
||||
|
||||
Reference in New Issue
Block a user