server: improve security of password reset flow #213
2 changed files with 41 additions and 32 deletions
|
@ -4,7 +4,7 @@ import config from '@/config/index.js';
|
||||||
import { Users, UserProfiles, PasswordResetRequests } from '@/models/index.js';
|
import { Users, UserProfiles, PasswordResetRequests } from '@/models/index.js';
|
||||||
import { sendEmail } from '@/services/send-email.js';
|
import { sendEmail } from '@/services/send-email.js';
|
||||||
import { genId } from '@/misc/gen-id.js';
|
import { genId } from '@/misc/gen-id.js';
|
||||||
import { HOUR } from '@/const.js';
|
import { DAY } from '@/const.js';
|
||||||
import define from '../define.js';
|
import define from '../define.js';
|
||||||
|
|
||||||
export const meta = {
|
export const meta = {
|
||||||
|
@ -15,12 +15,8 @@ export const meta = {
|
||||||
description: 'Request a users password to be reset.',
|
description: 'Request a users password to be reset.',
|
||||||
|
|
||||||
limit: {
|
limit: {
|
||||||
duration: HOUR,
|
duration: DAY,
|
||||||
max: 3,
|
max: 1,
|
||||||
},
|
|
||||||
|
|
||||||
errors: {
|
|
||||||
|
|
||||||
},
|
},
|
||||||
} as const;
|
} as const;
|
||||||
|
|
||||||
|
@ -35,25 +31,26 @@ export const paramDef = {
|
||||||
|
|
||||||
// eslint-disable-next-line import/no-default-export
|
// eslint-disable-next-line import/no-default-export
|
||||||
export default define(meta, paramDef, async (ps) => {
|
export default define(meta, paramDef, async (ps) => {
|
||||||
const user = await Users.findOneBy({
|
const [user, profile] = await Promise.all([
|
||||||
usernameLower: ps.username.toLowerCase(),
|
Users.findOneBy({
|
||||||
host: IsNull(),
|
usernameLower: ps.username.toLowerCase(),
|
||||||
|
host: IsNull(),
|
||||||
|
}),
|
||||||
|
UserProfiles.findOneByOrFail({ userId: user.id }),
|
||||||
|
]);
|
||||||
|
|
||||||
|
const resetRequestExists = await PasswordResetRequests.countBy({
|
||||||
|
userId: user.id,
|
||||||
});
|
});
|
||||||
|
|
||||||
// 合致するユーザーが登録されていなかったら無視
|
// This is an "arithmetical or" to try to prevent timing attacks which might reveal
|
||||||
if (user == null) {
|
// a users email address.
|
||||||
return;
|
if (
|
||||||
}
|
user == null
|
||||||
|
| profile.email !== ps.email
|
||||||
const profile = await UserProfiles.findOneByOrFail({ userId: user.id });
|
| !profile.emailVerified
|
||||||
|
| resetRequestExists > 0
|
||||||
// 合致するメアドが登録されていなかったら無視
|
) {
|
||||||
if (profile.email !== ps.email) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
// メアドが認証されていなかったら無視
|
|
||||||
if (!profile.emailVerified) {
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -69,6 +66,6 @@ export default define(meta, paramDef, async (ps) => {
|
||||||
const link = `${config.url}/reset-password/${token}`;
|
const link = `${config.url}/reset-password/${token}`;
|
||||||
|
|
||||||
sendEmail(ps.email, 'Password reset requested',
|
sendEmail(ps.email, 'Password reset requested',
|
||||||
`To reset password, please click this link:<br><a href="${link}">${link}</a>`,
|
`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>`,
|
||||||
`To reset password, please click this link: ${link}`);
|
`Hello ${user.username}!\nYou requested to reset your password. To do so, please follow this link: ${link}\nThe link is valid for 30 minutes.`);
|
||||||
});
|
});
|
||||||
|
|
|
@ -1,5 +1,6 @@
|
||||||
import bcrypt from 'bcryptjs';
|
import bcrypt from 'bcryptjs';
|
||||||
import { UserProfiles, PasswordResetRequests } from '@/models/index.js';
|
import { UserProfiles, PasswordResetRequests } from '@/models/index.js';
|
||||||
|
import { MINUTE } from '@/const.js';
|
||||||
import define from '../define.js';
|
import define from '../define.js';
|
||||||
|
|
||||||
export const meta = {
|
export const meta = {
|
||||||
|
@ -10,7 +11,11 @@ export const meta = {
|
||||||
description: 'Complete the password reset that was previously requested.',
|
description: 'Complete the password reset that was previously requested.',
|
||||||
|
|
||||||
errors: {
|
errors: {
|
||||||
|
noSuchResetRequest: {
|
||||||
|
message: 'No such password reset request.',
|
||||||
|
code: 'NO_SUCH_RESET_REQUEST',
|
||||||
|
id: '2c455231-675b-41be-b313-1181646a3ad3',
|
||||||
|
},
|
||||||
},
|
},
|
||||||
} as const;
|
} as const;
|
||||||
|
|
||||||
|
@ -25,13 +30,20 @@ export const paramDef = {
|
||||||
|
|
||||||
// eslint-disable-next-line import/no-default-export
|
// eslint-disable-next-line import/no-default-export
|
||||||
export default define(meta, paramDef, async (ps, user) => {
|
export default define(meta, paramDef, async (ps, user) => {
|
||||||
const req = await PasswordResetRequests.findOneByOrFail({
|
const req = await PasswordResetRequests.findOneBy({
|
||||||
token: ps.token,
|
token: ps.token,
|
||||||
});
|
});
|
||||||
|
|
||||||
// 発行してから30分以上経過していたら無効
|
if (req == null) throw new ApiError(meta.errors.noSuchResetRequest);
|
||||||
if (Date.now() - req.createdAt.getTime() > 1000 * 60 * 30) {
|
|
||||||
throw new Error(); // TODO
|
// Check that the has not expired yet after 30 minutes.
|
||||||
Johann150 marked this conversation as resolved
|
|||||||
|
//
|
||||||
|
// 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
|
// Generate hash of password
|
||||||
|
@ -42,5 +54,5 @@ export default define(meta, paramDef, async (ps, user) => {
|
||||||
password: hash,
|
password: hash,
|
||||||
});
|
});
|
||||||
|
|
||||||
PasswordResetRequests.delete(req.id);
|
await PasswordResetRequests.delete(req.id);
|
||||||
});
|
});
|
||||||
|
|
Loading…
Reference in a new issue
Guessing that the comment meant to say