mirror of
https://github.com/HeyPuter/puter.git
synced 2026-08-26 07:57:10 +00:00
Ns/bug fixes 060426 (#3214)
* Harden backend auth * Puter.js auth hardening
This commit is contained in:
@@ -37,6 +37,7 @@ import type { EventClient } from '../../clients/event/EventClient.js';
|
||||
import type { Actor } from '../../core/actor.js';
|
||||
import { runWithContext } from '../../core/context.js';
|
||||
import { HttpError } from '../../core/http/HttpError.js';
|
||||
import { requireUserActorGate } from '../../core/http/middleware/gates.js';
|
||||
import { PuterServer } from '../../server.js';
|
||||
import { setupTestServer } from '../../testUtil.js';
|
||||
|
||||
@@ -794,6 +795,65 @@ describe('AuthController.handleLogout', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('AuthController account-lifecycle route gating', () => {
|
||||
const routeOptions = (method: string, path: string) => {
|
||||
const proto = Object.getPrototypeOf(controller) as {
|
||||
__puterRoutes?: Array<{
|
||||
method: string;
|
||||
path: string;
|
||||
options?: Record<string, unknown>;
|
||||
}>;
|
||||
};
|
||||
const route = (proto.__puterRoutes ?? []).find(
|
||||
(r) =>
|
||||
r.method.toLowerCase() === method.toLowerCase() &&
|
||||
r.path === path,
|
||||
);
|
||||
expect(route, `route ${method} ${path} not found`).toBeDefined();
|
||||
return route!.options ?? {};
|
||||
};
|
||||
|
||||
it('POST /logout requires a human user actor', () => {
|
||||
const opts = routeOptions('post', '/logout');
|
||||
expect(opts.requireUserActor).toBe(true);
|
||||
expect(opts.antiCsrf).toBe(true);
|
||||
});
|
||||
|
||||
it('GET /get-anticsrf-token requires a human user actor', () => {
|
||||
const opts = routeOptions('get', '/get-anticsrf-token');
|
||||
expect(opts.requireUserActor).toBe(true);
|
||||
});
|
||||
|
||||
it('requireUserActorGate rejects app-under-user and access-token actors', () => {
|
||||
const gate = requireUserActorGate();
|
||||
const run = (actor: Partial<Actor>) =>
|
||||
new Promise<unknown>((resolve) => {
|
||||
gate(
|
||||
{ actor } as never,
|
||||
{} as never,
|
||||
(err?: unknown) => resolve(err),
|
||||
);
|
||||
});
|
||||
|
||||
return (async () => {
|
||||
const appActor = await run({
|
||||
user: { uuid: 'u1' },
|
||||
app: { uid: 'app-1' },
|
||||
} as Partial<Actor>);
|
||||
expect(appActor).toMatchObject({ statusCode: 403 });
|
||||
|
||||
const tokenActor = await run({
|
||||
user: { uuid: 'u1' },
|
||||
accessToken: { uid: 'tok-1' },
|
||||
} as Partial<Actor>);
|
||||
expect(tokenActor).toMatchObject({ statusCode: 403 });
|
||||
|
||||
const human = await run({ user: { uuid: 'u1' } } as Partial<Actor>);
|
||||
expect(human).toBeUndefined();
|
||||
})();
|
||||
});
|
||||
});
|
||||
|
||||
// ── Token grants: user → user / app / group ─────────────────────────
|
||||
|
||||
describe('AuthController grant flows', () => {
|
||||
|
||||
@@ -764,7 +764,7 @@ export class AuthController extends PuterController {
|
||||
// -- Logout ------------------------------------------------------
|
||||
|
||||
@Post('/logout', {
|
||||
requireAuth: true,
|
||||
requireUserActor: true,
|
||||
allowUnconfirmed: true,
|
||||
antiCsrf: true,
|
||||
})
|
||||
@@ -1634,7 +1634,9 @@ export class AuthController extends PuterController {
|
||||
// -- Anti-CSRF token generation ----------------------------------
|
||||
|
||||
@Get('/get-anticsrf-token', {
|
||||
requireAuth: true,
|
||||
// Anti-CSRF tokens are only consumed by `requireUserActor` routes,
|
||||
// so issuance is scoped to the same actor kind for consistency.
|
||||
requireUserActor: true,
|
||||
allowUnconfirmed: true,
|
||||
})
|
||||
async handleGetAntiCsrfToken(req: Request, res: Response): Promise<void> {
|
||||
|
||||
@@ -1010,6 +1010,76 @@ describe('LegacyFSController.sign', () => {
|
||||
),
|
||||
).rejects.toMatchObject({ statusCode: 404 });
|
||||
});
|
||||
|
||||
it('refuses an app actor signing for a different app (403)', async () => {
|
||||
// An app-under-user actor may only mint a token for its own app;
|
||||
// requesting a different app's UID is rejected.
|
||||
const { actor: userActor } = await makeUser();
|
||||
const targetApp = await (
|
||||
server.stores.app.create as unknown as (
|
||||
fields: Record<string, unknown>,
|
||||
opts: { ownerUserId: number },
|
||||
) => Promise<{ uid: string; id: number }>
|
||||
)(
|
||||
{
|
||||
name: `victim-${uuidv4()}`,
|
||||
title: 'Victim app',
|
||||
index_url: 'https://example.test/victim.html',
|
||||
},
|
||||
{ ownerUserId: userActor.user!.id! },
|
||||
);
|
||||
const attackerActor: Actor = {
|
||||
...userActor,
|
||||
app: { uid: `attacker-${uuidv4()}` },
|
||||
};
|
||||
|
||||
const { res } = makeRes();
|
||||
await expect(
|
||||
withActor(attackerActor, () =>
|
||||
controller.sign(
|
||||
makeReq({
|
||||
body: {
|
||||
items: [{}],
|
||||
app_uid: targetApp.uid,
|
||||
},
|
||||
actor: attackerActor,
|
||||
}),
|
||||
res,
|
||||
),
|
||||
),
|
||||
).rejects.toMatchObject({ statusCode: 403, legacyCode: 'forbidden' });
|
||||
});
|
||||
|
||||
it('lets an app actor sign for its own app', async () => {
|
||||
const { actor: userActor } = await makeUser();
|
||||
const ownApp = await (
|
||||
server.stores.app.create as unknown as (
|
||||
fields: Record<string, unknown>,
|
||||
opts: { ownerUserId: number },
|
||||
) => Promise<{ uid: string; id: number }>
|
||||
)(
|
||||
{
|
||||
name: `self-${uuidv4()}`,
|
||||
title: 'Self app',
|
||||
index_url: 'https://example.test/self.html',
|
||||
},
|
||||
{ ownerUserId: userActor.user!.id! },
|
||||
);
|
||||
const appActor: Actor = { ...userActor, app: { uid: ownApp.uid } };
|
||||
|
||||
const { res, captured } = makeRes();
|
||||
await withActor(appActor, () =>
|
||||
controller.sign(
|
||||
makeReq({
|
||||
body: { items: [{}], app_uid: ownApp.uid },
|
||||
actor: appActor,
|
||||
}),
|
||||
res,
|
||||
),
|
||||
);
|
||||
const body = captured.body as { token?: string };
|
||||
expect(typeof body.token).toBe('string');
|
||||
});
|
||||
});
|
||||
|
||||
// ── writeFile (validation paths) ────────────────────────────────────
|
||||
|
||||
@@ -1182,6 +1182,24 @@ describe('AuthService (integration)', () => {
|
||||
).rejects.toMatchObject({ statusCode: 400 });
|
||||
});
|
||||
|
||||
it('createWorkerAppToken refuses an app actor targeting a different app (403)', async () => {
|
||||
const user = await makeUser();
|
||||
const actor = {
|
||||
user: { id: user.id, uuid: user.uuid, username: user.username },
|
||||
app: { uid: `app-${uuidv4()}` },
|
||||
} as Actor;
|
||||
await expect(
|
||||
authService.createWorkerAppToken(
|
||||
actor,
|
||||
`app-${uuidv4()}`,
|
||||
'wk-x',
|
||||
),
|
||||
).rejects.toMatchObject({
|
||||
statusCode: 403,
|
||||
legacyCode: 'forbidden',
|
||||
});
|
||||
});
|
||||
|
||||
// ── Revocation flow ────────────────────────────────────────
|
||||
|
||||
it('revokeSession on a worker session — authenticate returns reauth.session_revoked', async () => {
|
||||
@@ -1362,6 +1380,67 @@ describe('AuthService (integration)', () => {
|
||||
// same app session row.
|
||||
expect(decodedFirst.session_uid).toBe(decodedSecond.session_uid);
|
||||
});
|
||||
|
||||
// Delegation scope: a scoped actor (app-under-user or access-token)
|
||||
// may only mint a token for its own app; only a root user session
|
||||
// may request a token for an arbitrary app.
|
||||
it('lets an app actor mint a token for its own app', async () => {
|
||||
const user = await makeUser();
|
||||
const ownApp = `app-${uuidv4()}`;
|
||||
const actor = {
|
||||
user: { id: user.id, uuid: user.uuid, username: user.username },
|
||||
app: { uid: ownApp },
|
||||
} as Actor;
|
||||
const token = await authService.getUserAppToken(actor, ownApp);
|
||||
const decoded = server.services.token.verify('auth', token) as {
|
||||
app_uid: string;
|
||||
};
|
||||
expect(decoded.app_uid).toBe(ownApp);
|
||||
});
|
||||
|
||||
it('refuses an app actor minting a token for a different app (403)', async () => {
|
||||
const user = await makeUser();
|
||||
const actor = {
|
||||
user: { id: user.id, uuid: user.uuid, username: user.username },
|
||||
app: { uid: `app-${uuidv4()}` },
|
||||
} as Actor;
|
||||
await expect(
|
||||
authService.getUserAppToken(actor, `app-${uuidv4()}`),
|
||||
).rejects.toMatchObject({
|
||||
statusCode: 403,
|
||||
legacyCode: 'forbidden',
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses an access-token actor minting an app token (403)', async () => {
|
||||
const user = await makeUser();
|
||||
const issuer = {
|
||||
user: { id: user.id, uuid: user.uuid, username: user.username },
|
||||
} as Actor;
|
||||
const actor = {
|
||||
user: { id: user.id, uuid: user.uuid, username: user.username },
|
||||
accessToken: { uid: `tok-${uuidv4()}`, issuer, authorized: null },
|
||||
} as Actor;
|
||||
await expect(
|
||||
authService.getUserAppToken(actor, `app-${uuidv4()}`),
|
||||
).rejects.toMatchObject({
|
||||
statusCode: 403,
|
||||
legacyCode: 'forbidden',
|
||||
});
|
||||
});
|
||||
|
||||
it('lets a root user session mint a token for any app', async () => {
|
||||
const user = await makeUser();
|
||||
const actor = {
|
||||
user: { id: user.id, uuid: user.uuid, username: user.username },
|
||||
} as Actor;
|
||||
const anyApp = `app-${uuidv4()}`;
|
||||
const token = await authService.getUserAppToken(actor, anyApp);
|
||||
const decoded = server.services.token.verify('auth', token) as {
|
||||
app_uid: string;
|
||||
};
|
||||
expect(decoded.app_uid).toBe(anyApp);
|
||||
});
|
||||
});
|
||||
|
||||
describe('createAccessToken / revokeAccessToken', () => {
|
||||
|
||||
@@ -357,6 +357,7 @@ export class AuthService extends PuterService {
|
||||
legacyCode: 'bad_request',
|
||||
});
|
||||
}
|
||||
this.#assertAppDelegationAllowed(actor, appUid);
|
||||
const auth_id = this.#authIdFor(actor.user as UserRow);
|
||||
const session = await this.stores.session.getOrCreateWorker(
|
||||
actor.user.id,
|
||||
@@ -384,6 +385,22 @@ export class AuthService extends PuterService {
|
||||
return user.uuid;
|
||||
}
|
||||
|
||||
/**
|
||||
* Scope app token delegation by actor kind. An app-under-user or
|
||||
* access-token actor is bound to a single app and may only mint a token
|
||||
* for that same app; only a root user session may request a token for an
|
||||
* arbitrary app (the GUI's app-launch delegation).
|
||||
*/
|
||||
#assertAppDelegationAllowed(actor: Actor, appUid: string): void {
|
||||
if ((actor.app || actor.accessToken) && actor.app?.uid !== appUid) {
|
||||
throw new HttpError(
|
||||
403,
|
||||
'Actor cannot mint a token for another app',
|
||||
{ legacyCode: 'forbidden' },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Convert a jsonwebtoken-style `expiresIn` (seconds, or `'1h'`/`'30d'`)
|
||||
* into an absolute unix-seconds timestamp for the session row. Returns
|
||||
@@ -1057,6 +1074,7 @@ export class AuthService extends PuterService {
|
||||
throw new HttpError(403, 'Actor must be a user', {
|
||||
legacyCode: 'forbidden',
|
||||
});
|
||||
this.#assertAppDelegationAllowed(actor, appUid);
|
||||
|
||||
// Request-context (IP / UA) isn't available on the Actor shape —
|
||||
// the app row's `last_ip` / `last_user_agent` start NULL and get
|
||||
|
||||
@@ -56,6 +56,9 @@ class Auth {
|
||||
let settled = false;
|
||||
// Interval id for polling whether the user closed the popup.
|
||||
let checkClosed = null;
|
||||
// The auth popup window we opened. Pinned as the expected
|
||||
// `event.source` when validating the token message.
|
||||
let popupWindow = null;
|
||||
|
||||
const cleanup = () => {
|
||||
if ( checkClosed ) {
|
||||
@@ -66,6 +69,20 @@ class Auth {
|
||||
};
|
||||
|
||||
function messageHandler (e) {
|
||||
// Only accept the token from the Puter GUI origin AND from the
|
||||
// popup window we opened. Origin alone is insufficient (any
|
||||
// frame on the GUI domain could post), so also pin
|
||||
// event.source. Mirrors the validated handler in index.js.
|
||||
// msg_id binds the message to this attempt.
|
||||
if ( e.origin !== puter.defaultGUIOrigin ) {
|
||||
return;
|
||||
}
|
||||
if ( popupWindow && e.source !== popupWindow ) {
|
||||
return;
|
||||
}
|
||||
if ( e.data?.msg !== 'puter.token' ) {
|
||||
return;
|
||||
}
|
||||
if ( e.data?.msg_id != msg_id ) {
|
||||
return;
|
||||
}
|
||||
@@ -101,6 +118,8 @@ class Auth {
|
||||
reject({ error: 'popup_blocked', msg: 'The sign-in popup was blocked by the browser.' });
|
||||
return;
|
||||
}
|
||||
// Record the popup so messageHandler can pin event.source.
|
||||
popupWindow = popup;
|
||||
checkClosed = setInterval(() => {
|
||||
if ( ! popup.closed ) {
|
||||
return;
|
||||
|
||||
@@ -440,6 +440,16 @@ class PuterDialog extends (globalThis.HTMLElement || Object) { // It will fall b
|
||||
|
||||
// Event listener for the 'message' event
|
||||
this.messageListener = async (event) => {
|
||||
// Only accept the token from the Puter GUI origin AND from the
|
||||
// popup we opened. Origin alone is insufficient (any frame on the
|
||||
// GUI domain could post), so also pin event.source. Mirrors the
|
||||
// validated handler in index.js.
|
||||
if ( event.origin !== puter.defaultGUIOrigin ) {
|
||||
return;
|
||||
}
|
||||
if ( this.authPopup && event.source !== this.authPopup ) {
|
||||
return;
|
||||
}
|
||||
if ( event.data.msg === 'puter.token' ) {
|
||||
this.close();
|
||||
// Set the authToken property
|
||||
@@ -519,6 +529,8 @@ class PuterDialog extends (globalThis.HTMLElement || Object) { // It will fall b
|
||||
// safe from being popup-blocked because it happens inside a click.
|
||||
this.shadowRoot.querySelector('#launch-auth-popup')?.addEventListener('click', () => {
|
||||
const popup = openAuthPopup(this.#popupURL());
|
||||
// Pinned as the expected event.source in messageListener.
|
||||
this.authPopup = popup;
|
||||
|
||||
// Launcher mode: hand the popup back to the caller and close the
|
||||
// consent dialog — its only job was to provide the user gesture.
|
||||
@@ -544,6 +556,8 @@ class PuterDialog extends (globalThis.HTMLElement || Object) { // It will fall b
|
||||
open () {
|
||||
if ( hasUserActivation() ) {
|
||||
const popup = openAuthPopup(this.#popupURL());
|
||||
// Pinned as the expected event.source in messageListener.
|
||||
this.authPopup = popup;
|
||||
if ( this.options.popupURL && typeof this.options.onLaunch === 'function' ) {
|
||||
this.options.onLaunch(popup);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user