Skip to content

Commit 47fb139

Browse files
thonatosthonatos
committed
fix(auth): recover failed account email delivery
1 parent cd6dacb commit 47fb139

31 files changed

Lines changed: 1839 additions & 59 deletions

‎apps/api/Dockerfile‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,12 @@ COPY --from=deps /app/packages/db/node_modules /app/packages/db/node_modules
3737
COPY --from=deps /app/packages/shared/node_modules /app/packages/shared/node_modules
3838
COPY --from=build /app/package.json /app/pnpm-workspace.yaml /app/tsconfig.base.json ./
3939
COPY --from=build /app/apps/api/package.json /app/apps/api/package.json
40+
COPY --from=build /app/apps/api/tsconfig.json /app/apps/api/tsconfig.json
41+
COPY --from=build /app/apps/api/scripts /app/apps/api/scripts
4042
COPY --from=build /app/apps/api/src /app/apps/api/src
4143
COPY --from=build /app/packages/db /app/packages/db
4244
COPY --from=build /app/packages/shared /app/packages/shared
4345
COPY --from=build /app/scripts /app/scripts
46+
RUN CNODE_ENV=production pnpm --filter @cnode/api exec tsx scripts/mail-template-smoke.ts
4447
EXPOSE 3001
4548
CMD ["pnpm", "--filter", "@cnode/api", "exec", "tsx", "src/bootstrap.ts", "api"]

‎apps/api/scripts/mail-template-smoke.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import {
2+
buildActiveMail,
3+
buildAtNotifyMail,
4+
buildReplyNotifyMail,
5+
buildResetPassMail,
6+
} from "../src/lib/mail-template";
7+
8+
const templates = await Promise.all([
9+
buildActiveMail("smoke-key", "https://example.com"),
10+
buildResetPassMail("smoke-key", "https://example.com"),
11+
buildReplyNotifyMail("Smoke topic", "Smoke reply", "https://example.com/topic/1"),
12+
buildAtNotifyMail("Smoke topic", "Smoke mention", "https://example.com/topic/1"),
13+
]);
14+
15+
if (templates.some((template) => !template.subject || !template.html || !template.text)) {
16+
throw new Error("Mail template smoke failed");
17+
}

‎apps/api/src/lib/account-activation.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
export interface PendingAccount {
2+
id: number;
3+
email: string;
4+
pass: string | null;
5+
active: boolean | null;
6+
}
7+
8+
export interface AccountActivationDependencies {
9+
createKey(): string;
10+
now(): number;
11+
sendActivation(email: string, key: string): Promise<void>;
12+
}
13+
14+
export function activeAccount<T extends { active: boolean | null }>(user: T | null): T | null {
15+
return user?.active ? user : null;
16+
}
17+
18+
export async function createPendingAccount(
19+
input: { loginname: string; passhash: string; email: string },
20+
dependencies: AccountActivationDependencies & {
21+
createUser(input: {
22+
loginname: string;
23+
pass: string;
24+
email: string;
25+
active: false;
26+
retrieveKey: string;
27+
retrieveTime: number;
28+
}): Promise<unknown>;
29+
},
30+
) {
31+
const retrieveKey = dependencies.createKey();
32+
await dependencies.createUser({
33+
loginname: input.loginname,
34+
pass: input.passhash,
35+
email: input.email,
36+
active: false,
37+
retrieveKey,
38+
retrieveTime: dependencies.now(),
39+
});
40+
try {
41+
await dependencies.sendActivation(input.email, retrieveKey);
42+
return { status: "created" as const };
43+
} catch {
44+
return { status: "email_failed" as const };
45+
}
46+
}
47+
48+
export async function resendAccountActivation(
49+
input: { name: string; pass: string },
50+
dependencies: AccountActivationDependencies & {
51+
findUser(name: string): Promise<PendingAccount | null>;
52+
verifyPassword(password: string, hash: string): Promise<boolean>;
53+
updateKey(userId: number, key: string, time: number): Promise<void>;
54+
},
55+
) {
56+
const user = await dependencies.findUser(input.name);
57+
if (!user?.pass || user.active || !(await dependencies.verifyPassword(input.pass, user.pass))) {
58+
return { status: "ignored" as const };
59+
}
60+
61+
const retrieveKey = dependencies.createKey();
62+
await dependencies.updateKey(user.id, retrieveKey, dependencies.now());
63+
try {
64+
await dependencies.sendActivation(user.email, retrieveKey);
65+
return { status: "sent" as const };
66+
} catch {
67+
return { status: "email_failed" as const };
68+
}
69+
}

‎apps/api/src/lib/db.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ function getDb(): DB {
3333
return dbInstance;
3434
}
3535

36+
export function buildUserByTokenQuery(db: Database, token: string) {
37+
return db
38+
.select()
39+
.from(users)
40+
.where(and(eq(users.accessToken, token), boolEq(users.active, true)))
41+
.limit(1);
42+
}
43+
3644
export const userQueries = {
3745
async getByLoginName(loginname: string) {
3846
const db = getDb();
@@ -60,7 +68,7 @@ export const userQueries = {
6068

6169
async getByToken(token: string) {
6270
const db = getDb();
63-
const result = await db.select().from(users).where(eq(users.accessToken, token)).limit(1);
71+
const result = await buildUserByTokenQuery(db, token);
6472
return result[0] || null;
6573
},
6674

@@ -89,6 +97,8 @@ export const userQueries = {
8997
githubId?: string;
9098
githubUsername?: string;
9199
githubAccessToken?: string;
100+
retrieveKey?: string;
101+
retrieveTime?: number;
92102
}) {
93103
const db = getDb();
94104
const [user] = await db
@@ -103,6 +113,8 @@ export const userQueries = {
103113
githubId: params.githubId,
104114
githubUsername: params.githubUsername,
105115
githubAccessToken: params.githubAccessToken,
116+
retrieveKey: params.retrieveKey,
117+
retrieveTime: params.retrieveTime,
106118
})
107119
.returning();
108120
return user;

‎apps/api/src/lib/mail.ts‎

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,30 @@ interface MailData {
4545
text?: string;
4646
}
4747

48+
export type MailFailureStage = "template" | "smtp";
49+
50+
export class MailDeliveryError extends Error {
51+
constructor(
52+
readonly stage: MailFailureStage,
53+
options?: ErrorOptions,
54+
) {
55+
super(`Mail ${stage} failed`, options);
56+
this.name = "MailDeliveryError";
57+
}
58+
}
59+
4860
export async function sendMail(data: MailData) {
49-
const t = getTransporter();
61+
let t: Transporter | null;
62+
try {
63+
t = getTransporter();
64+
} catch (error) {
65+
appLog("mail.send.failed", "ERROR", {
66+
outcome: "failed",
67+
"mail.stage": "smtp",
68+
"error.type": errorType(error),
69+
});
70+
throw new MailDeliveryError("smtp", { cause: error });
71+
}
5072
if (!t) return;
5173

5274
const from = process.env.SMTP_FROM || data.from;
@@ -62,27 +84,35 @@ export async function sendMail(data: MailData) {
6284
appLog("mail.send.failed", "ERROR", {
6385
outcome: "failed",
6486
attempt: i,
87+
"mail.stage": "smtp",
6588
"error.type": errorType(err),
6689
});
67-
if (i === 5) throw err;
90+
if (i === 5) throw new MailDeliveryError("smtp", { cause: err });
6891
}
6992
}
7093
}
7194

95+
async function buildAndSend(to: string, build: () => Promise<Omit<MailData, "from" | "to">>) {
96+
let content: Omit<MailData, "from" | "to">;
97+
try {
98+
content = await build();
99+
} catch (error) {
100+
appLog("mail.template.failed", "ERROR", {
101+
outcome: "failed",
102+
"mail.stage": "template",
103+
"error.type": errorType(error),
104+
});
105+
throw new MailDeliveryError("template", { cause: error });
106+
}
107+
await sendMail({ from: "cnode@localhost", to, ...content });
108+
}
109+
72110
export async function sendActiveMail(email: string, key: string) {
73-
await sendMail({
74-
from: "cnode@localhost",
75-
to: email,
76-
...(await buildActiveMail(key)),
77-
});
111+
await buildAndSend(email, () => buildActiveMail(key));
78112
}
79113

80114
export async function sendResetPassMail(email: string, key: string) {
81-
await sendMail({
82-
from: "cnode@localhost",
83-
to: email,
84-
...(await buildResetPassMail(key)),
85-
});
115+
await buildAndSend(email, () => buildResetPassMail(key));
86116
}
87117

88118
export async function sendReplyNotifyMail(
@@ -91,11 +121,7 @@ export async function sendReplyNotifyMail(
91121
replyContent: string,
92122
topicUrl: string,
93123
) {
94-
await sendMail({
95-
from: "cnode@localhost",
96-
to: email,
97-
...(await buildReplyNotifyMail(topicTitle, replyContent, topicUrl)),
98-
});
124+
await buildAndSend(email, () => buildReplyNotifyMail(topicTitle, replyContent, topicUrl));
99125
}
100126

101127
export async function sendAtNotifyMail(
@@ -104,9 +130,5 @@ export async function sendAtNotifyMail(
104130
replyContent: string,
105131
topicUrl: string,
106132
) {
107-
await sendMail({
108-
from: "cnode@localhost",
109-
to: email,
110-
...(await buildAtNotifyMail(topicTitle, replyContent, topicUrl)),
111-
});
133+
await buildAndSend(email, () => buildAtNotifyMail(topicTitle, replyContent, topicUrl));
112134
}

‎apps/api/src/lib/user-registration.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
export type UserUniqueConflict = "loginname" | "email" | null;
2+
3+
export function userUniqueConflict(error: unknown): UserUniqueConflict {
4+
if (!error || typeof error !== "object") return null;
5+
const value = error as {
6+
code?: string;
7+
constraint?: string;
8+
message?: string;
9+
detail?: string;
10+
cause?: unknown;
11+
};
12+
const description = `${value.constraint || ""} ${value.message || ""} ${value.detail || ""}`;
13+
if (value.code === "23505" || /unique/i.test(description)) {
14+
if (/loginname/i.test(description)) return "loginname";
15+
if (/email/i.test(description)) return "email";
16+
}
17+
return value.cause ? userUniqueConflict(value.cause) : null;
18+
}

‎apps/api/src/middleware/auth.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { createMiddleware } from "hono/factory";
22
import { getCookie, setCookie, deleteCookie } from "hono/cookie";
33
import { roleQueries, userQueries } from "../lib/db";
44
import { resolveUserAccess } from "../lib/user-access";
5+
import { activeAccount } from "../lib/account-activation";
56

67
export interface AuthVars {
78
user: Awaited<ReturnType<typeof userQueries.getById>> | null;
@@ -49,7 +50,7 @@ export const authMiddleware = () =>
4950
if (userId) {
5051
const id = Number(userId);
5152
if (id > 0 && !Number.isNaN(id)) {
52-
user = await userQueries.getById(id);
53+
user = activeAccount(await userQueries.getById(id));
5354
}
5455
}
5556
}

‎apps/api/src/middleware/rate-limit.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createMiddleware } from "hono/factory";
2+
import { createHash } from "node:crypto";
23
import { getCache } from "../lib/cache";
34
import { settingQueries, userQueries } from "../lib/db";
45

@@ -97,6 +98,21 @@ export function perIpPerDay(name: string, limitCount: number, showJson = true) {
9798
});
9899
}
99100

101+
export function perAccountPerDay(name: string, limitCount: number, showJson = true) {
102+
return makePerDayLimiter({
103+
identityName: "peraccountperday",
104+
name,
105+
identityFn: (c) => {
106+
const identity = String(c.req.valid?.("json")?.name || "")
107+
.trim()
108+
.toLowerCase();
109+
return createHash("sha256").update(identity).digest("hex");
110+
},
111+
limitCount,
112+
showJson,
113+
});
114+
}
115+
100116
export function perUserPerDaySetting(
101117
name: string,
102118
settingKey: string,

0 commit comments

Comments
 (0)