fix: close the review findings on the seat experience

From the adversarial review of this PR.

The forced password-change gate could deadlock: it POSTed to the
cookie-only route with a bare fetch, and both initgui call sites open it
before update_auth_data mints the session cookie — a fresh browser with a
token URL 401'd every submit inside a non-dismissible loop. It uses the
session-cookie retry wrapper now, taking the caller's token because
window.auth_token does not exist yet on that path. It also gains the
logout footer its sibling gates have; a lost temporary password was a
hard lock with devtools as the only exit.

Password recovery refused for seats: the address is admin-supplied and
never verified, so whoever holds that inbox could take the seat over at
any later time. A seat's recovery channel is its admin's reset.

change-email gets the same seat guard as change-username and deletion —
the address is where admin-issued credentials go.

The login response now carries `team` alongside requires_password_change:
no-reload logins store that payload as window.user verbatim, and every
seat restriction keys on it.

Smaller: the team-badge tooltip no longer double-encodes; the create-token
hint for an emailless account stops pointing at a verification it can
never perform; the quotas doc records the halved org_seat_free allowance;
the config template tells upgrading operators how to keep the old flat
cap; the SDK suite covers emailless provisioning and the owner-only uuid.
This commit is contained in:
Juan Castro
2026-09-14 17:00:18 -04:00
parent 15527eac4c
commit c3e2717e8e
12 changed files with 215 additions and 14 deletions
+3 -1
View File
@@ -289,7 +289,9 @@
// name from the global username pool, so this is what bounds a team's blast
// radius. The owner's plan decides which of the two applies; defaults are 4
// and 40. Lowering either below a team's current seat count blocks new
// provisioning and disables nobody.
// provisioning and disables nobody. Deployments upgrading from the old flat
// default of 50: without a billing extension every owner reads as free, so
// set `max_seats_per_team` to keep the old behavior.
"max_seats_per_team_free": 4,
"max_seats_per_team_paid": 40,
//
@@ -5006,6 +5006,32 @@ describe('AuthController password recovery', () => {
expect(after!.pass_recovery_token).toBeTruthy();
});
it('send-pass-recovery-email: refuses a team seat, and writes no token', async () => {
// The seat's address is admin-supplied and unverified; recovery there
// would be a takeover channel. Its recovery is the admin's reset.
const { user: owner } = await makeUserAndActor();
const { user: seat } = await makeUserAndActor();
const team = await server.stores.team.create({
ownerUserId: owner.id,
name: 'Acme',
});
await server.stores.user.update(seat.id, { password: null });
await server.stores.team.addMember(team.uid, seat.id, {
orgOwned: true,
});
const res = makeRes();
await controller.handleSendPassRecoveryEmail(
makeReq({ username: seat.username }),
res,
);
expect((res.body as { message: string }).message).toMatch(
/If that account exists/i,
);
const after = await server.stores.user.getById(seat.id, { force: true });
expect(after!.pass_recovery_token).toBeFalsy();
});
it('verify-pass-recovery-token: 400 on missing token', async () => {
await expect(
controller.handleVerifyPassRecoveryToken(makeReq({}), makeRes()),
@@ -5253,6 +5279,26 @@ describe('AuthController user-protected mutations (validation paths)', () => {
).rejects.toMatchObject({ statusCode: 400 });
});
it('change-email: 403 for an account its team provisioned', async () => {
const { user: owner } = await makeUserAndActor();
const { user: seat, actor } = await makeUserAndActor();
const team = await server.stores.team.create({
ownerUserId: owner.id,
name: 'Acme',
});
await server.stores.user.update(seat.id, { password: null });
await server.stores.team.addMember(team.uid, seat.id, {
orgOwned: true,
});
await expect(
controller.handleChangeEmail(
makeReq({ new_email: `moved_${uniq()}@example.com` }, { actor }),
makeRes(),
),
).rejects.toMatchObject({ statusCode: 403 });
});
it('change-username: 403 for an account its team provisioned', async () => {
// The console lists members by username and the audit log records them
// by it; a self-service rename would desync both.
@@ -2293,6 +2293,14 @@ export class AuthController extends PuterController {
return;
}
// A seat's address is admin-supplied and never verified, so whoever
// holds that inbox could take the seat over. Its recovery channel is
// the team admin's password reset, not this one.
if (await this.stores.team.getOrgSeat(user.id)) {
res.json({ message: genericMessage });
return;
}
const pass_recovery_token = uuidv4();
await this.stores.user.update(user.id, { pass_recovery_token });
@@ -2628,6 +2636,16 @@ export class AuthController extends PuterController {
}
async handleChangeEmail(req: Request, res: Response): Promise<void> {
// The address is where admin-issued credentials and team notices go;
// same reasoning as the username and deletion guards above.
if (await this.stores.team.getOrgSeat(req.actor!.user.id!)) {
throw new HttpError(
403,
'Your team set this address. Ask a team admin to change it.',
{ legacyCode: 'forbidden' },
);
}
const { new_email } = req.body ?? {};
if (!new_email || typeof new_email !== 'string') {
throw new HttpError(400, '`new_email` is required', {
@@ -4875,6 +4893,20 @@ export class AuthController extends PuterController {
console.warn('[auth] taskbar_items resolution failed:', e);
}
// Same shape as whoami: no-reload logins store this payload as
// window.user verbatim, and every seat restriction keys on `team`.
let team: { uid: string; name: string | null } | undefined;
if (this.config.teams_enabled === true) {
try {
const seat = await this.stores.team.getOrgSeat(user.id);
if (seat) {
team = { uid: seat.team_uid, name: seat.team_name ?? null };
}
} catch (e) {
console.warn('[auth] team lookup failed:', e);
}
}
// Response body gets the GUI token (client never sees session token)
res.json({
proceed: true,
@@ -4891,6 +4923,7 @@ export class AuthController extends PuterController {
requires_card_verification: user.requires_card_verification,
requires_password_change: user.requires_password_change,
is_temp: user.password === null && user.email === null,
...(team ? { team } : {}),
taskbar_items,
},
});
+3 -1
View File
@@ -195,7 +195,9 @@ Available only where a deployment has turned teams on. Every team route is bound
| Seats one team may provision, paying owner | 40 |
| Member password resets per day | 20 |
A seat is a real Puter account on the ordinary tier, created by the team and paid for by its owner, so the seat limit is what bounds a team's size. Over it, provisioning fails with `seat_limit_reached`; over the team limit, creation fails with `team_limit_reached`. Both carry the limit in `fields.limit`.
A seat is a real Puter account created by the team and paid for by its owner, so the seat limit is what bounds a team's size. Over it, provisioning fails with `seat_limit_reached`; over the team limit, creation fails with `team_limit_reached`. Both carry the limit in `fields.limit`.
A seat whose team pays for no tier is on the `org_seat_free` plan: **half** the ordinary free allowance, usage and rate caps alike (a free account's `bySubscription` caps apply to every free plan). Without this, provisioning seats would mint full free tiers nobody pays for. A seat on a paid team tier gets that tier's allowance.
A reset returns a temporary password once and never again. It stops working 24 hours after it is issued, so an unused reset expires rather than becoming a standing credential; after that the administrator has to issue a new one. Until the member replaces it, every authenticated request from that account fails with `password_change_required` — signing in works, but nothing else does until they choose their own password.
+5 -1
View File
@@ -127,9 +127,13 @@ const TabAccount = {
if ( window.user?.email_confirmed ) {
h += `<button class="button copy-auth-token">${i18n('create_token')}</button>`;
} else {
// "Verify your email" is a dead end for an account with none.
const tokenHint = window.user?.email
? i18n('verify_email_to_create_token')
: i18n('email_needed_to_create_token');
// Disabled buttons have `pointer-events: none`, so the tooltip
// lives on a wrapping span that still receives hover.
h += `<span title="${html_encode(i18n('verify_email_to_create_token'))}" style="cursor: not-allowed;">`;
h += `<span title="${html_encode(tokenHint)}" style="cursor: not-allowed;">`;
h += `<button class="button copy-auth-token" disabled>${i18n('create_token')}</button>`;
h += '</span>';
}
+2 -1
View File
@@ -28,7 +28,8 @@ export const teamBadgeHtml = (user) => {
const name = user?.team?.name;
if ( typeof name !== 'string' || name.trim() === '' ) return '';
const label = window.html_encode(name);
const title = window.html_encode(i18n('teams_account_of', [name]));
// i18n encodes by default; encoding twice shows entities in the tooltip.
const title = window.html_encode(i18n('teams_account_of', [name], false));
return `<div class="dashboard-sidebar-team" title="${title}">${label}</div>`;
};
+12 -2
View File
@@ -1,7 +1,11 @@
import { describe, expect, it } from 'vitest';
globalThis.i18n = (key, args) => `${key}:${(args ?? []).join(',')}`;
globalThis.window = { html_encode: (v) => String(v).replace(/</g, '&lt;') };
// Mirrors the real i18n: encodes unless the third argument is false.
globalThis.i18n = (key, args, encode = true) => {
const raw = `${key}:${(args ?? []).join(',')}`;
return encode ? raw.replace(/&/g, '&amp;') : raw;
};
globalThis.window = { html_encode: (v) => String(v).replace(/&/g, '&amp;').replace(/</g, '&lt;') };
const { teamBadgeHtml } = await import('./teamBadge.js');
@@ -34,3 +38,9 @@ describe('the sidebar team badge', () => {
expect(h).toContain('teams_account_of:Acme');
});
});
it('does not double-encode the tooltip', () => {
const h = teamBadgeHtml({ team: { uid: 't-1', name: 'R&D' } });
expect(h).toContain('title="teams_account_of:R&amp;D"');
expect(h).not.toContain('&amp;amp;');
});
@@ -18,6 +18,7 @@
*/
import check_password_strength from '../helpers/checkPasswordStrength.js';
import { fetchWithSessionCookieRetry } from '../util/sessionAuth.js';
import UIWindow from './UIWindow.js';
/** Resolves true once a seat replaces the password its administrator chose. */
@@ -43,6 +44,12 @@ function UIWindowPasswordChangeRequired (options) {
h += '<input id="pcr-confirm" class="pcr-confirm" type="password" autocomplete="new-password" />';
h += `<button type="submit" class="button button-block button-primary pcr-btn" style="margin-top:16px;">${i18n('change_password')}</button>`;
h += '</form>';
// The gate loops; a lost temporary password needs a way out.
if ( options.logout_in_footer ) {
h += '<div style="text-align:center; padding:10px; font-size:14px; margin-top:10px;">';
h += `<span class="pcr-log-out" style="cursor:pointer; text-decoration:underline;">${i18n('log_out')}</span>`;
h += '</div>';
}
h += '</div>';
const el_window = await UIWindow({
@@ -87,6 +94,12 @@ function UIWindowPasswordChangeRequired (options) {
$(el_window).find('.pcr-current, .pcr-new, .pcr-confirm').attr('disabled', false);
};
$(el_window).find('.pcr-log-out').on('click', function () {
window.logout();
$(el_window).close();
resolve(false);
});
$(el_window).find('form').on('submit', async function (e) {
e.preventDefault();
const current_password = $(el_window).find('.pcr-current').val();
@@ -112,16 +125,22 @@ function UIWindowPasswordChangeRequired (options) {
$(el_window).find('.pcr-btn').addClass('disabled');
$(el_window).find('.pcr-current, .pcr-new, .pcr-confirm').attr('disabled', true);
// The route is cookie-gated and this gate can open before the
// session cookie exists; the wrapper mints it and retries once.
const send = () => fetch(`${origin}/user-protected/change-password`, {
method: 'POST',
credentials: 'include',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
password: current_password,
new_pass: new_password,
}),
});
let res;
try {
res = await fetch(`${origin}/user-protected/change-password`, {
method: 'POST',
credentials: 'include',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
password: current_password,
new_pass: new_password,
}),
res = await fetchWithSessionCookieRetry(send, {
origin,
authToken: options.auth_token ?? window.auth_token,
});
} catch (err) {
return fail(err?.message || 'Request failed');
@@ -58,6 +58,62 @@ describe('the forced password-change gate', () => {
fields['.pcr-confirm'] = 'ChosenPass1!';
});
it('mints the session cookie and retries when the first submit 401s', async () => {
globalThis.window.auth_token = 'tok-1';
const rejected = {
ok: false,
status: 401,
clone: () => ({ json: async () => ({ code: 'session_required' }) }),
json: async () => ({ code: 'session_required' }),
};
globalThis.fetch
.mockResolvedValueOnce(rejected) // first submit
.mockResolvedValueOnce({ ok: true, json: async () => ({}) }) // sync-cookie
.mockResolvedValueOnce({ ok: true }); // retried submit
UIWindowPasswordChangeRequired({});
await new Promise((r) => setTimeout(r));
await submit();
const urls = globalThis.fetch.mock.calls.map(([u]) => String(u));
expect(urls.some((u) => u.includes('/session/sync-cookie'))).toBe(true);
expect(state.closed).toBe(1);
});
it('offers a way out: the footer logout closes the gate', async () => {
let clickHandler = null;
const prevFind = globalThis.$;
globalThis.$ = () => ({
find: (sel) => ({
val: () => fields[sel],
on: (evt, fn) => {
if (evt === 'submit') submitHandler = fn;
if (evt === 'click' && sel === '.pcr-log-out') clickHandler = fn;
},
html: () => ({ fadeIn: () => {} }),
hide: () => {},
fadeIn: () => {},
addClass: () => {},
removeClass: () => {},
attr: () => {},
get: () => [undefined],
}),
close: () => { state.closed++; },
});
globalThis.window.logout = vi.fn();
const resolved = UIWindowPasswordChangeRequired({ logout_in_footer: true });
await new Promise((r) => setTimeout(r));
expect(state.el.body).toContain('pcr-log-out');
clickHandler();
expect(globalThis.window.logout).toHaveBeenCalled();
await expect(resolved).resolves.toBe(false);
globalThis.$ = prevFind;
});
it('renders no logout link unless the caller asks for one', async () => {
UIWindowPasswordChangeRequired({});
await new Promise((r) => setTimeout(r));
expect(state.el.body).not.toContain('pcr-log-out');
});
it('posts to the one route the gate lets through, with credentials', async () => {
globalThis.fetch.mockResolvedValue({ ok: true });
const gate = UIWindowPasswordChangeRequired({ show_close_button: false });
+1
View File
@@ -527,6 +527,7 @@ const en = {
teams_create_team_hint:
'A team pays for the accounts you create in it. You stay its only administrator.',
teams_create_team_prompt: 'What should the team be called?',
email_needed_to_create_token: 'This account has no email address, which tokens require.',
teams_audit_range: '{{from}}{{to}} of {{total}}',
previous: 'Previous',
next: 'Next',
+6
View File
@@ -1806,10 +1806,14 @@ window.initgui = async function (options) {
show_close_button: false,
stay_on_top: true,
has_head: false,
logout_in_footer: true,
auth_token: query_param_auth_token,
window_options: {
is_draggable: false,
},
});
// false = logged out; stop looping on a dead session.
if (changed === false && !window.auth_token) return;
} while (!changed);
}
// if user is logging in using an auth token that means it's not their first ever visit to Puter.com
@@ -2086,11 +2090,13 @@ window.initgui = async function (options) {
show_close_button: false,
stay_on_top: true,
has_head: false,
logout_in_footer: true,
window_options: {
is_draggable: false,
cover_page: window.is_embedded,
},
});
if (changed === false && !window.auth_token) return;
} while (!changed);
}
await window.update_auth_data(
@@ -122,6 +122,27 @@ export default suite('teams', {
t.assert.equal(member!.orgOwned, true);
},
'createMember needs no email, and the owner sees each seat uuid': async (t) => {
const team = await makeTeam(t, 'no-email');
const username = `tsn${tag()}`;
const created = (await t.puter.teams.createMember(team.uid, {
username,
})) as { username: string; temporaryPassword: string };
t.assert.equal(created.username, username);
t.assert.ok(created.temporaryPassword.length > 0, 'credential still issued');
const members = (await t.puter.teams.listMembers(team.uid)) as Array<{
username: string; orgOwned: boolean; uuid?: string;
}>;
const member = members.find((m) => m.username === username);
t.assert.ok(!!member, 'the emailless seat should be a member');
// The uuid is what billing keys a seat's plan on; owner-only.
t.assert.ok(
typeof member!.uuid === 'string' && member!.uuid.length > 0,
'the owner should see the seat uuid',
);
},
'createMember with a taken username rejects': async (t) => {
const team = await makeTeam(t, 'taken');
await t.assert.rejects(