📧 fix: Cancel Signup if Email Issuance Fails (#3010)

* fix: user.id assignment in jwtStrategy.js

* refactor(sendEmail): pass params as object, await email sending to propogate errors and restrict registration flow

* fix(Conversations): handle missing updatedAt field

* refactor: use `processDeleteRequest` when deleting user account for user file deletion

* refactor: delete orphaned files when deleting user account

* fix: remove unnecessary 404 status code in server/index.js
This commit is contained in:
Danny Avila 2024-06-08 06:51:29 -04:00 committed by GitHub
parent 084cf266a2
commit 92232afaca
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 104 additions and 57 deletions

View file

@ -56,7 +56,7 @@ const updateUser = async function (userId, updateData) {
* Creates a new user, optionally with a TTL of 1 week. * Creates a new user, optionally with a TTL of 1 week.
* @param {MongoUser} data - The user data to be created, must contain user_id. * @param {MongoUser} data - The user data to be created, must contain user_id.
* @param {boolean} [disableTTL=true] - Whether to disable the TTL. Defaults to `true`. * @param {boolean} [disableTTL=true] - Whether to disable the TTL. Defaults to `true`.
* @returns {Promise<string>} A promise that resolves to the created user document ID. * @returns {Promise<ObjectId>} A promise that resolves to the created user document ID.
* @throws {Error} If a user with the same user_id already exists. * @throws {Error} If a user with the same user_id already exists.
*/ */
const createUser = async (data, disableTTL = true) => { const createUser = async (data, disableTTL = true) => {

View file

@ -1,6 +1,7 @@
const { const {
Session, Session,
Balance, Balance,
getFiles,
deleteFiles, deleteFiles,
deleteConvos, deleteConvos,
deletePresets, deletePresets,
@ -10,6 +11,7 @@ const {
const { updateUserPluginAuth, deleteUserPluginAuth } = require('~/server/services/PluginService'); const { updateUserPluginAuth, deleteUserPluginAuth } = require('~/server/services/PluginService');
const { updateUserPluginsService, deleteUserKey } = require('~/server/services/UserService'); const { updateUserPluginsService, deleteUserKey } = require('~/server/services/UserService');
const { verifyEmail, resendVerificationEmail } = require('~/server/services/AuthService'); const { verifyEmail, resendVerificationEmail } = require('~/server/services/AuthService');
const { processDeleteRequest } = require('~/server/services/Files/process');
const { deleteAllSharedLinks } = require('~/models/Share'); const { deleteAllSharedLinks } = require('~/models/Share');
const { Transaction } = require('~/models/Transaction'); const { Transaction } = require('~/models/Transaction');
const { logger } = require('~/config'); const { logger } = require('~/config');
@ -18,6 +20,18 @@ const getUserController = async (req, res) => {
res.status(200).send(req.user); res.status(200).send(req.user);
}; };
const deleteUserFiles = async (req) => {
try {
const userFiles = await getFiles({ user: req.user.id });
await processDeleteRequest({
req,
files: userFiles,
});
} catch (error) {
logger.error('[deleteUserFiles]', error);
}
};
const updateUserPluginsController = async (req, res) => { const updateUserPluginsController = async (req, res) => {
const { user } = req; const { user } = req;
const { pluginKey, action, auth, isAssistantTool } = req.body; const { pluginKey, action, auth, isAssistantTool } = req.body;
@ -75,11 +89,13 @@ const deleteUserController = async (req, res) => {
await deleteUserKey({ userId: user.id, all: true }); // delete user keys await deleteUserKey({ userId: user.id, all: true }); // delete user keys
await Balance.deleteMany({ user: user._id }); // delete user balances await Balance.deleteMany({ user: user._id }); // delete user balances
await deletePresets(user.id); // delete user presets await deletePresets(user.id); // delete user presets
/* TODO: Delete Assistant Threads */
await deleteConvos(user.id); // delete user convos await deleteConvos(user.id); // delete user convos
await deleteUserPluginAuth(user.id, null, true); // delete user plugin auth await deleteUserPluginAuth(user.id, null, true); // delete user plugin auth
await deleteUserById(user.id); // delete user await deleteUserById(user.id); // delete user
await deleteFiles(null, user.id); // delete user files
await deleteAllSharedLinks(user.id); // delete user shared links await deleteAllSharedLinks(user.id); // delete user shared links
await deleteUserFiles(req); // delete user files
await deleteFiles(null, user.id); // delete database files in case of orphaned files from previous steps
/* TODO: queue job for cleaning actions and assistants of non-existant users */ /* TODO: queue job for cleaning actions and assistants of non-existant users */
logger.info(`User deleted account. Email: ${user.email} ID: ${user.id}`); logger.info(`User deleted account. Email: ${user.email} ID: ${user.id}`);
res.status(200).send({ message: 'User deleted' }); res.status(200).send({ message: 'User deleted' });

View file

@ -93,7 +93,7 @@ const startServer = async () => {
app.use('/api/share', routes.share); app.use('/api/share', routes.share);
app.use((req, res) => { app.use((req, res) => {
res.status(404).sendFile(path.join(app.locals.paths.dist, 'index.html')); res.sendFile(path.join(app.locals.paths.dist, 'index.html'));
}); });
app.listen(port, host, () => { app.listen(port, host, () => {

View file

@ -2,12 +2,13 @@ const crypto = require('crypto');
const bcrypt = require('bcryptjs'); const bcrypt = require('bcryptjs');
const { errorsToString } = require('librechat-data-provider'); const { errorsToString } = require('librechat-data-provider');
const { const {
findUser,
countUsers, countUsers,
createUser, createUser,
findUser,
updateUser, updateUser,
generateToken,
getUserById, getUserById,
generateToken,
deleteUserById,
} = require('~/models/userMethods'); } = require('~/models/userMethods');
const { sendEmail, checkEmailConfig } = require('~/server/utils'); const { sendEmail, checkEmailConfig } = require('~/server/utils');
const { registerSchema } = require('~/strategies/validators'); const { registerSchema } = require('~/strategies/validators');
@ -61,6 +62,19 @@ const sendVerificationEmail = async (user) => {
let verifyToken = crypto.randomBytes(32).toString('hex'); let verifyToken = crypto.randomBytes(32).toString('hex');
const hash = bcrypt.hashSync(verifyToken, 10); const hash = bcrypt.hashSync(verifyToken, 10);
const verificationLink = `${domains.client}/verify?token=${verifyToken}&email=${user.email}`;
await sendEmail({
email: user.email,
subject: 'Verify your email',
payload: {
appName: process.env.APP_TITLE || 'LibreChat',
name: user.name,
verificationLink: verificationLink,
year: new Date().getFullYear(),
},
template: 'verifyEmail.handlebars',
});
await new Token({ await new Token({
userId: user._id, userId: user._id,
email: user.email, email: user.email,
@ -68,21 +82,7 @@ const sendVerificationEmail = async (user) => {
createdAt: Date.now(), createdAt: Date.now(),
}).save(); }).save();
const verificationLink = `${domains.client}/verify?token=${verifyToken}&email=${user.email}`;
logger.info(`[sendVerificationEmail] Verification link issued. [Email: ${user.email}]`); logger.info(`[sendVerificationEmail] Verification link issued. [Email: ${user.email}]`);
sendEmail(
user.email,
'Verify your email',
{
appName: process.env.APP_TITLE || 'LibreChat',
name: user.name,
verificationLink: verificationLink,
year: new Date().getFullYear(),
},
'verifyEmail.handlebars',
);
return;
}; };
/** /**
@ -136,6 +136,7 @@ const registerUser = async (user) => {
const { email, password, name, username } = user; const { email, password, name, username } = user;
let newUserId;
try { try {
const existingUser = await findUser({ email }, 'email _id'); const existingUser = await findUser({ email }, 'email _id');
@ -173,7 +174,7 @@ const registerUser = async (user) => {
}; };
const emailEnabled = checkEmailConfig(); const emailEnabled = checkEmailConfig();
const newUserId = await createUser(newUserData, false); newUserId = await createUser(newUserData, false);
if (emailEnabled) { if (emailEnabled) {
await sendVerificationEmail({ await sendVerificationEmail({
_id: newUserId, _id: newUserId,
@ -187,6 +188,12 @@ const registerUser = async (user) => {
return { status: 200, message: genericVerificationMessage }; return { status: 200, message: genericVerificationMessage };
} catch (err) { } catch (err) {
logger.error('[registerUser] Error in registering user:', err); logger.error('[registerUser] Error in registering user:', err);
if (newUserId) {
const result = await deleteUserById(newUserId);
logger.warn(
`[registerUser] [Email: ${email}] [Temporary User deleted: ${JSON.stringify(result)}]`,
);
}
return { status: 500, message: 'Something went wrong' }; return { status: 500, message: 'Something went wrong' };
} }
}; };
@ -226,17 +233,17 @@ const requestPasswordReset = async (req) => {
const link = `${domains.client}/reset-password?token=${resetToken}&userId=${user._id}`; const link = `${domains.client}/reset-password?token=${resetToken}&userId=${user._id}`;
if (emailEnabled) { if (emailEnabled) {
sendEmail( await sendEmail({
user.email, email: user.email,
'Password Reset Request', subject: 'Password Reset Request',
{ payload: {
appName: process.env.APP_TITLE || 'LibreChat', appName: process.env.APP_TITLE || 'LibreChat',
name: user.name, name: user.name,
link: link, link: link,
year: new Date().getFullYear(), year: new Date().getFullYear(),
}, },
'requestPasswordReset.handlebars', template: 'requestPasswordReset.handlebars',
); });
logger.info( logger.info(
`[requestPasswordReset] Link emailed. [Email: ${email}] [ID: ${user._id}] [IP: ${req.ip}]`, `[requestPasswordReset] Link emailed. [Email: ${email}] [ID: ${user._id}] [IP: ${req.ip}]`,
); );
@ -277,16 +284,16 @@ const resetPassword = async (userId, token, password) => {
const user = await updateUser(userId, { password: hash }); const user = await updateUser(userId, { password: hash });
if (checkEmailConfig()) { if (checkEmailConfig()) {
sendEmail( await sendEmail({
user.email, email: user.email,
'Password Reset Successfully', subject: 'Password Reset Successfully',
{ payload: {
appName: process.env.APP_TITLE || 'LibreChat', appName: process.env.APP_TITLE || 'LibreChat',
name: user.name, name: user.name,
year: new Date().getFullYear(), year: new Date().getFullYear(),
}, },
'passwordReset.handlebars', template: 'passwordReset.handlebars',
); });
} }
await passwordResetToken.deleteOne(); await passwordResetToken.deleteOne();
@ -356,6 +363,20 @@ const resendVerificationEmail = async (req) => {
let verifyToken = crypto.randomBytes(32).toString('hex'); let verifyToken = crypto.randomBytes(32).toString('hex');
const hash = bcrypt.hashSync(verifyToken, 10); const hash = bcrypt.hashSync(verifyToken, 10);
const verificationLink = `${domains.client}/verify?token=${verifyToken}&email=${user.email}`;
await sendEmail({
email: user.email,
subject: 'Verify your email',
payload: {
appName: process.env.APP_TITLE || 'LibreChat',
name: user.name,
verificationLink: verificationLink,
year: new Date().getFullYear(),
},
template: 'verifyEmail.handlebars',
});
await new Token({ await new Token({
userId: user._id, userId: user._id,
email: user.email, email: user.email,
@ -363,21 +384,8 @@ const resendVerificationEmail = async (req) => {
createdAt: Date.now(), createdAt: Date.now(),
}).save(); }).save();
const verificationLink = `${domains.client}/verify?token=${verifyToken}&email=${user.email}`;
logger.info(`[resendVerificationEmail] Verification link issued. [Email: ${user.email}]`); logger.info(`[resendVerificationEmail] Verification link issued. [Email: ${user.email}]`);
sendEmail(
user.email,
'Verify your email',
{
appName: process.env.APP_TITLE || 'LibreChat',
name: user.name,
verificationLink: verificationLink,
year: new Date().getFullYear(),
},
'verifyEmail.handlebars',
);
return { return {
status: 200, status: 200,
message: genericVerificationMessage, message: genericVerificationMessage,

View file

@ -5,7 +5,34 @@ const handlebars = require('handlebars');
const { isEnabled } = require('~/server/utils/handleText'); const { isEnabled } = require('~/server/utils/handleText');
const logger = require('~/config/winston'); const logger = require('~/config/winston');
const sendEmail = async (email, subject, payload, template) => { /**
* Sends an email using the specified template, subject, and payload.
*
* @async
* @function sendEmail
* @param {Object} params - The parameters for sending the email.
* @param {string} params.email - The recipient's email address.
* @param {string} params.subject - The subject of the email.
* @param {Record<string, string>} params.payload - The data to be used in the email template.
* @param {string} params.template - The filename of the email template.
* @param {boolean} [throwError=true] - Whether to throw an error if the email sending process fails.
* @returns {Promise<Object>} - A promise that resolves to the info object of the sent email or the error if sending the email fails.
*
* @example
* const emailData = {
* email: 'recipient@example.com',
* subject: 'Welcome!',
* payload: { name: 'Recipient' },
* template: 'welcome.html'
* };
*
* sendEmail(emailData)
* .then(info => console.log('Email sent:', info))
* .catch(error => console.error('Error sending email:', error));
*
* @throws Will throw an error if the email sending process fails and throwError is `true`.
*/
const sendEmail = async ({ email, subject, payload, template, throwError = true }) => {
try { try {
const transporterOptions = { const transporterOptions = {
// Use STARTTLS by default instead of obligatory TLS // Use STARTTLS by default instead of obligatory TLS
@ -58,16 +85,11 @@ const sendEmail = async (email, subject, payload, template) => {
}; };
// Send email // Send email
transporter.sendMail(options(), (error, info) => { return await transporter.sendMail(options());
if (error) {
logger.error('[sendEmail]', error);
return error;
} else {
logger.debug('[sendEmail]', info);
return info;
}
});
} catch (error) { } catch (error) {
if (throwError) {
throw error;
}
logger.error('[sendEmail]', error); logger.error('[sendEmail]', error);
return error; return error;
} }

View file

@ -12,8 +12,8 @@ const jwtLogin = async () =>
async (payload, done) => { async (payload, done) => {
try { try {
const user = await getUserById(payload?.id, '-password -__v'); const user = await getUserById(payload?.id, '-password -__v');
user.id = user._id.toString();
if (user) { if (user) {
user.id = user._id.toString();
done(null, user); done(null, user);
} else { } else {
logger.warn('[jwtLogin] JwtStrategy => no user found: ' + payload?.id); logger.warn('[jwtLogin] JwtStrategy => no user found: ' + payload?.id);

View file

@ -21,7 +21,8 @@ const Conversations = ({
); );
const firstTodayConvoId = useMemo( const firstTodayConvoId = useMemo(
() => () =>
conversations.find((convo) => convo && isToday(parseISO(convo.updatedAt)))?.conversationId, conversations.find((convo) => convo && convo.updatedAt && isToday(parseISO(convo.updatedAt)))
?.conversationId,
[conversations], [conversations],
); );