mirror of
https://github.com/HeyPuter/puter.git
synced 2026-10-13 07:10:05 +00:00
refactor(auth): one confirmEmail for the code and link confirmation routes
Both routes now re-run the email checks, refuse an address another account confirmed, strip it from other rows, promote the group and push user.email_confirmed to the account's sockets (with the originating socket id on the code route). The link route's full-table UPDATE clearing other accounts' pending change-email requests is dropped: /change_email/confirm already refuses an address a confirmed account owns.
This commit is contained in:
1 parent
7b0ea3d5a6
commit
a526ee526f
5 files changed
+176
-135
No files matched your search
@@ -1086,62 +1086,10 @@ export class AuthController extends PuterController {
|
||||
return;
|
||||
}
|
||||
|
||||
// Re-validate the email at confirmation time — the address may
|
||||
// have been added to the blocklist (or flagged by an extension)
|
||||
// after signup but before confirmation.
|
||||
await this.services.signup.validateEmail(user.email!);
|
||||
|
||||
// An account that already confirmed this address proved access to the
|
||||
// inbox, and revoking it below would hand the address to whoever
|
||||
// confirmed second. Refuse instead — a duplicate this old is data to
|
||||
// repair, not a race to resolve.
|
||||
const canonical = cleanEmail(user.email!);
|
||||
const confirmedRival = await this.stores.user.findConfirmedOtherByEmail(
|
||||
user.id,
|
||||
user.email!,
|
||||
canonical,
|
||||
);
|
||||
if (confirmedRival) {
|
||||
throw new HttpError(
|
||||
400,
|
||||
'This email was confirmed on a different account.',
|
||||
{ legacyCode: 'email_already_in_use' as never },
|
||||
);
|
||||
}
|
||||
|
||||
// Revoke the address from every remaining (unconfirmed) account holding
|
||||
// it, THEN confirm this one. Only one row may own an address, so
|
||||
// confirming first would momentarily create a second owner — which the
|
||||
// unique index rejects, turning a legitimate confirmation into a 500.
|
||||
await this.stores.user.unconfirmOthersByEmail(
|
||||
user.id,
|
||||
user.email!,
|
||||
canonical,
|
||||
);
|
||||
|
||||
await this.stores.user.update(user.id, {
|
||||
email_confirmed: 1,
|
||||
requires_email_confirmation: 0,
|
||||
email_confirm_code: null,
|
||||
email_confirm_token: null,
|
||||
await this.services.signup.confirmEmail(user, {
|
||||
originalClientSocketId: original_client_socket_id,
|
||||
});
|
||||
|
||||
await promoteToVerifiedGroup(this.stores.group, this.config, user);
|
||||
|
||||
try {
|
||||
this.clients.event?.emit(
|
||||
'user.email-confirmed' as never,
|
||||
{
|
||||
user_id: user.id,
|
||||
user_uid: user.uuid,
|
||||
email: user.email,
|
||||
} as never,
|
||||
{},
|
||||
);
|
||||
} catch {
|
||||
// ignore — event is a side-channel signal, not load-bearing
|
||||
}
|
||||
|
||||
res.json({ email_confirmed: true, original_client_socket_id });
|
||||
}
|
||||
|
||||
|
||||
@@ -375,6 +375,38 @@ describe('StaticPagesController GET /confirm-email-by-token', () => {
|
||||
expect(Boolean(refreshed?.requires_email_confirmation)).toBe(false);
|
||||
});
|
||||
|
||||
it('re-runs the email checks, as the code route does', async () => {
|
||||
const user = await makeUser({ email_confirmed: 0 });
|
||||
const deny = (_k: string, data: unknown) => {
|
||||
Object.assign(data as object, {
|
||||
allow: false,
|
||||
message: 'Address no longer accepted.',
|
||||
});
|
||||
};
|
||||
server.clients.event.on('email.validate', deny);
|
||||
const { res, captured } = makeRes();
|
||||
try {
|
||||
await callRoute(
|
||||
'get',
|
||||
'/confirm-email-by-token',
|
||||
makeReq({
|
||||
query: {
|
||||
user_uuid: user.uuid,
|
||||
token: user.email_confirm_token,
|
||||
},
|
||||
}),
|
||||
res,
|
||||
);
|
||||
} finally {
|
||||
server.clients.event.off('email.validate', deny);
|
||||
}
|
||||
expect(String(captured.body)).toContain('Address no longer accepted.');
|
||||
const refreshed = await server.stores.user.getById(user.id, {
|
||||
force: true,
|
||||
});
|
||||
expect(Boolean(refreshed?.email_confirmed)).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects when the email is already confirmed on a different account', async () => {
|
||||
// Duplicate-email gate fires only when the existing row is
|
||||
// (a) email_confirmed=1 and (b) has a non-null password — the
|
||||
|
||||
@@ -19,7 +19,7 @@
|
||||
|
||||
import { PuterController } from '../types.js';
|
||||
import type { PuterRouter } from '../../core/http/PuterRouter';
|
||||
import { promoteToVerifiedGroup } from '../../util/userProvisioning.js';
|
||||
import { HttpError } from '../../core/http/HttpError.js';
|
||||
|
||||
/**
|
||||
* One-off user-facing pages.
|
||||
@@ -274,85 +274,14 @@ export class StaticPagesController extends PuterController {
|
||||
return;
|
||||
}
|
||||
|
||||
// v2 writes `clean_email` at signup (lowercased email). Older rows
|
||||
// that predate that may be null — fall back to email.lower().
|
||||
const cleanEmail =
|
||||
(user.clean_email as string | null | undefined) ??
|
||||
String(user.email ?? '').toLowerCase();
|
||||
|
||||
// An account that already confirmed this address proved access
|
||||
// to the inbox. The strip below would take it away from them,
|
||||
// so refuse here instead. Password-less accounts count: an
|
||||
// identity provider verified the address for those, and they
|
||||
// are exactly what the old `password IS NOT NULL` clause let
|
||||
// through.
|
||||
const confirmedRival =
|
||||
await this.stores.user.findConfirmedOtherByEmail(
|
||||
user.id as number,
|
||||
user.email as string,
|
||||
cleanEmail,
|
||||
);
|
||||
if (confirmedRival) {
|
||||
res.send(
|
||||
err('This email was confirmed on a different account.'),
|
||||
);
|
||||
try {
|
||||
await this.services.signup.confirmEmail(user);
|
||||
} catch (e) {
|
||||
if (!(e instanceof HttpError)) throw e;
|
||||
res.send(err(e.message));
|
||||
return;
|
||||
}
|
||||
|
||||
// Revoke any other accounts' pending change-email slots targeting
|
||||
// this address — they're no longer valid once someone confirms it.
|
||||
await this.clients.db.write(
|
||||
'UPDATE `user` SET `unconfirmed_change_email` = NULL, `change_email_confirm_token` = NULL WHERE `unconfirmed_change_email` = ?',
|
||||
[user.email],
|
||||
);
|
||||
|
||||
// Take the address off every remaining row before confirming
|
||||
// this one. The check above leaves only unconfirmed rows, and
|
||||
// only one row may own an address once this one is confirmed.
|
||||
await this.stores.user.unconfirmOthersByEmail(
|
||||
user.id,
|
||||
user.email as string,
|
||||
cleanEmail,
|
||||
);
|
||||
|
||||
await this.stores.user.update(user.id, {
|
||||
email_confirmed: 1,
|
||||
requires_email_confirmation: 0,
|
||||
email_confirm_code: null,
|
||||
email_confirm_token: null,
|
||||
});
|
||||
|
||||
await promoteToVerifiedGroup(
|
||||
this.stores.group,
|
||||
this.config,
|
||||
user,
|
||||
);
|
||||
|
||||
// Best-effort side-channels — don't fail the user-visible response
|
||||
// if sockets or the event bus are unavailable.
|
||||
try {
|
||||
await this.services.socket.send(
|
||||
{ room: user.id },
|
||||
'user.email_confirmed',
|
||||
{},
|
||||
);
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
try {
|
||||
this.clients.event?.emit(
|
||||
'user.email-confirmed',
|
||||
{
|
||||
user_id: user.id,
|
||||
user_uid: user.uuid,
|
||||
email: user.email,
|
||||
},
|
||||
{},
|
||||
);
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
|
||||
res.send(ok('Your email has been successfully confirmed.'));
|
||||
},
|
||||
);
|
||||
|
||||
@@ -18,7 +18,7 @@
|
||||
*/
|
||||
|
||||
import type { Request } from 'express';
|
||||
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
|
||||
import { PuterServer } from '../../server.js';
|
||||
import { setupTestServer } from '../../testUtil.js';
|
||||
|
||||
@@ -201,3 +201,75 @@ describe('SignupService.announceSignup', () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('SignupService.confirmEmail', () => {
|
||||
const makeUnconfirmed = async () => {
|
||||
const username = `ce_${slug()}`;
|
||||
const created = await server.stores.user.create({
|
||||
username,
|
||||
uuid: crypto.randomUUID(),
|
||||
password: null,
|
||||
email: `${username}@example.com`,
|
||||
requires_email_confirmation: true,
|
||||
});
|
||||
return (await server.stores.user.getById(created.id, {
|
||||
force: true,
|
||||
}))!;
|
||||
};
|
||||
|
||||
it("tells the account's other tabs, naming the one that confirmed", async () => {
|
||||
const user = await makeUnconfirmed();
|
||||
const send = vi.spyOn(server.services.socket, 'send');
|
||||
try {
|
||||
await server.services.signup.confirmEmail(user, {
|
||||
originalClientSocketId: 'sock-1',
|
||||
});
|
||||
expect(send).toHaveBeenCalledWith(
|
||||
{ room: user.id },
|
||||
'user.email_confirmed',
|
||||
{ original_client_socket_id: 'sock-1' },
|
||||
);
|
||||
} finally {
|
||||
send.mockRestore();
|
||||
}
|
||||
const after = await server.stores.user.getById(user.id, {
|
||||
force: true,
|
||||
});
|
||||
expect(after!.email_confirmed).toBeTruthy();
|
||||
expect(after!.email_confirm_code).toBeNull();
|
||||
});
|
||||
|
||||
it('leaves another account no way to take the address through a pending change', async () => {
|
||||
const owner = await makeUnconfirmed();
|
||||
const other = await makeUnconfirmed();
|
||||
await server.stores.user.update(other.id, {
|
||||
unconfirmed_change_email: owner.email,
|
||||
change_email_confirm_token: 'pending-tok',
|
||||
});
|
||||
|
||||
await server.services.signup.confirmEmail(owner);
|
||||
|
||||
const token = server.services.token.sign(
|
||||
'otp',
|
||||
{
|
||||
token: 'pending-tok',
|
||||
user_id: other.id,
|
||||
purpose: 'change-email',
|
||||
},
|
||||
{ expiresIn: '1h' },
|
||||
);
|
||||
await expect(
|
||||
server.controllers.auth.handleChangeEmailConfirm(
|
||||
{ query: { token } } as unknown as Request,
|
||||
{ send: vi.fn() } as never,
|
||||
),
|
||||
).rejects.toMatchObject({
|
||||
statusCode: 400,
|
||||
legacyCode: 'email_already_in_use',
|
||||
});
|
||||
const after = await server.stores.user.getById(other.id, {
|
||||
force: true,
|
||||
});
|
||||
expect(after!.email).toBe(other.email);
|
||||
});
|
||||
});
|
||||
@@ -86,9 +86,9 @@ export const signupClientIp = (req: Request): string | null =>
|
||||
req.ip || req.socket?.remoteAddress || null;
|
||||
|
||||
/**
|
||||
* Account creation for every signup route (form, temp, OIDC). Callers parse
|
||||
* requests and set cookies; the steps and the events listeners rely on live
|
||||
* here.
|
||||
* Account creation for every signup route (form, temp, OIDC) and email
|
||||
* confirmation for every confirmation route. Callers parse requests and set
|
||||
* cookies; the steps and the events listeners rely on live here.
|
||||
*/
|
||||
export class SignupService extends PuterService {
|
||||
/**
|
||||
@@ -614,6 +614,66 @@ export class SignupService extends PuterService {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Confirm the account's current address, for the code and the link routes
|
||||
* alike. Re-runs the email checks (the address may have been blocked since
|
||||
* signup), refuses an address another account already confirmed, and takes
|
||||
* it off every other row. Another account's pending change to this address
|
||||
* needs no cleanup: `/change_email/confirm` refuses an address a confirmed
|
||||
* account owns.
|
||||
*/
|
||||
async confirmEmail(
|
||||
user: UserRow,
|
||||
{ originalClientSocketId }: { originalClientSocketId?: unknown } = {},
|
||||
): Promise<void> {
|
||||
const email = user.email as string;
|
||||
await this.validateEmail(email);
|
||||
|
||||
const canonical = cleanEmail(email);
|
||||
const confirmedRival = await this.stores.user.findConfirmedOtherByEmail(
|
||||
user.id,
|
||||
email,
|
||||
canonical,
|
||||
);
|
||||
if (confirmedRival) {
|
||||
throw new HttpError(
|
||||
400,
|
||||
'This email was confirmed on a different account.',
|
||||
{ legacyCode: 'email_already_in_use' as never },
|
||||
);
|
||||
}
|
||||
|
||||
// Only one row may own an address once this one is confirmed, so the
|
||||
// others lose it first.
|
||||
await this.stores.user.unconfirmOthersByEmail(
|
||||
user.id,
|
||||
email,
|
||||
canonical,
|
||||
);
|
||||
await this.stores.user.update(user.id, {
|
||||
email_confirmed: 1,
|
||||
requires_email_confirmation: 0,
|
||||
email_confirm_code: null,
|
||||
email_confirm_token: null,
|
||||
});
|
||||
await promoteToVerifiedGroup(this.stores.group, this.config, user);
|
||||
|
||||
// Best-effort side channels. Other tabs refresh; the one that sent
|
||||
// the code (named by its socket id) already knows.
|
||||
try {
|
||||
await this.services.socket.send(
|
||||
{ room: user.id },
|
||||
'user.email_confirmed',
|
||||
originalClientSocketId === undefined
|
||||
? {}
|
||||
: { original_client_socket_id: originalClientSocketId },
|
||||
);
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
this.#emitEmailConfirmed(user);
|
||||
}
|
||||
|
||||
/**
|
||||
* The account holding `email` that a signup has to contend with. A seat
|
||||
* whose address its team typed and nobody confirmed doesn't count; with
|
||||
|
||||
Reference in new issue
Block a user