packages/backend/src/server/api/endpoints/request-reset-password.ts

@@ -4,7 +4,7 @@ import config from '@/config/index.js';
 import { Users, UserProfiles, PasswordResetRequests } from '@/models/index.js';
 import { sendEmail } from '@/services/send-email.js';
 import { genId } from '@/misc/gen-id.js';
-import { HOUR } from '@/const.js';
+import { DAY } from '@/const.js';
 import define from '../define.js';
 
 export const meta = {
@@ -15,12 +15,8 @@ export const meta = {
 	description: 'Request a users password to be reset.',
 
 	limit: {
-		duration: HOUR,
-		max: 3,
-	},
-
-	errors: {
-
+		duration: DAY,
+		max: 1,
 	},
 } as const;
 
@@ -35,25 +31,26 @@ export const paramDef = {
 
 // eslint-disable-next-line import/no-default-export
 export default define(meta, paramDef, async (ps) => {
-	const user = await Users.findOneBy({
+	const [user, profile] = await Promise.all([
+		Users.findOneBy({
 			usernameLower: ps.username.toLowerCase(),
 			host: IsNull(),
+		}),
+		UserProfiles.findOneByOrFail({ userId: user.id }),
+	]);
+
+	const resetRequestExists = await PasswordResetRequests.countBy({
+		userId: user.id,
 	});
 
-	// 合致するユーザーが登録されていなかったら無視
-	if (user == null) {
-		return;
-	}
-
-	const profile = await UserProfiles.findOneByOrFail({ userId: user.id });
-
-	// 合致するメアドが登録されていなかったら無視
-	if (profile.email !== ps.email) {
-		return;
-	}
-
-	// メアドが認証されていなかったら無視
-	if (!profile.emailVerified) {
+	// This is an "arithmetical or" to try to prevent timing attacks which might reveal
+	// a users email address.
+	if (
+		user == null
+		| profile.email !== ps.email
+		| !profile.emailVerified
+		| resetRequestExists > 0
+	) {
 		return;
 	}
 
@@ -69,6 +66,6 @@ export default define(meta, paramDef, async (ps) => {
 	const link = `${config.url}/reset-password/${token}`;
 
 	sendEmail(ps.email, 'Password reset requested',
-		`To reset password, please click this link:<br><a href="${link}">${link}</a>`,
-		`To reset password, please click this link: ${link}`);
+		`Hello ${user.username}!<p>You requested to reset your password. To do so, please follow this link:</p><a href="${link}">${link}</a><p>The link is valid for 30 minutes.</p>`,
+		`Hello ${user.username}!\nYou requested to reset your password. To do so, please follow this link: ${link}\nThe link is valid for 30 minutes.`);
 });

packages/backend/src/server/api/endpoints/reset-password.ts

@@ -1,5 +1,6 @@
 import bcrypt from 'bcryptjs';
 import { UserProfiles, PasswordResetRequests } from '@/models/index.js';
+import { MINUTE } from '@/const.js';
 import define from '../define.js';
 
 export const meta = {
@@ -10,7 +11,11 @@ export const meta = {
 	description: 'Complete the password reset that was previously requested.',
 
 	errors: {
-
+		noSuchResetRequest: {
+			message: 'No such password reset request.',
+			code: 'NO_SUCH_RESET_REQUEST',
+			id: '2c455231-675b-41be-b313-1181646a3ad3',
+		},
 	},
 } as const;
 
@@ -25,13 +30,20 @@ export const paramDef = {
 
 // eslint-disable-next-line import/no-default-export
 export default define(meta, paramDef, async (ps, user) => {
-	const req = await PasswordResetRequests.findOneByOrFail({
+	const req = await PasswordResetRequests.findOneBy({
 		token: ps.token,
 	});
 
-	// 発行してから30分以上経過していたら無効
-	if (Date.now() - req.createdAt.getTime() > 1000 * 60 * 30) {
-		throw new Error(); // TODO
+	if (req == null) throw new ApiError(meta.errors.noSuchResetRequest);
+
+	// Check that the has not expired yet after 30 minutes.
+	//
+	// This is a secondary check just in case the expiry task is broken,
+	// the expiry task schedule is not aligned to the timing, or something
+	// else strange is going on. After all this is security critical.
+	if (Date.now() - req.createdAt.getTime() > 30 * MINUTE) {
+		await PasswordResetRequests.delete(req.id);
+		throw new ApiError(meta.errors.noSuchResetRequest);
 	}
 
 	// Generate hash of password
@@ -42,5 +54,5 @@ export default define(meta, paramDef, async (ps, user) => {
 		password: hash,
 	});
 
-	PasswordResetRequests.delete(req.id);
+	await PasswordResetRequests.delete(req.id);
 });