User create / registeration queue refactor (#13905)

* feat: add options parameter to User.create

add emailVerification: ('send'|'verify'|'skip') param to User.create to control email verification

add a new method User.createOrQueue(). store options that will be passed to User.create() when registration is accepted in _opts

If there is no password passed to registration queue(SSO register) don't store hashedPassword

removed the isFirstUser hack in user.create, when creating the admin user in install.js passing `emailVerification: 'verify'` to immediately verify the email, same with all the hacks in tests

auth: if an SSO plugin sends back an info object, redirect to root and display the message

* refactor: make function private

* refactor: destruct return

* test: fix flag test

* test: group tests

* feat: show ssoIcon if available in register queue

* add icon/title
This commit is contained in:
Barış Uşaklı
2026-01-19 18:40:48 -05:00
committed by GitHub
parent 6383bb58e9
commit e2e1744824
17 changed files with 176 additions and 112 deletions

View File

@@ -41,13 +41,11 @@ async function registerAndLoginUser(req, res, userData) {
return;
}
const queue = await user.shouldQueueUser(req.ip);
const result = await plugins.hooks.fire('filter:register.shouldQueue', { req, res, userData, queue });
if (result.queue) {
return await addToApprovalQueue(req, userData);
const { queued, uid, message } = await user.createOrQueue(req, userData);
if (queued) {
return { message };
}
const uid = await user.create(userData);
if (res.locals.processLogin) {
const hasLoginPrivilege = await privileges.global.can('local:login', uid);
if (hasLoginPrivilege) {
@@ -125,22 +123,6 @@ authenticationController.register = async function (req, res) {
}
};
async function addToApprovalQueue(req, userData) {
userData.ip = req.ip;
await user.addToApprovalQueue(userData);
let message = '[[register:registration-added-to-queue]]';
if (meta.config.showAverageApprovalTime) {
const average_time = await db.getObjectField('registration:queue:approval:times', 'average');
if (average_time > 0) {
message += ` [[register:registration-queue-average-time, ${Math.floor(average_time / 60)}, ${Math.floor(average_time % 60)}]]`;
}
}
if (meta.config.autoApproveTime > 0) {
message += ` [[register:registration-queue-auto-approve-time, ${meta.config.autoApproveTime}]]`;
}
return { message: message };
}
authenticationController.registerComplete = async function (req, res) {
try {
// For the interstitials that respond, execute the callback with the form body

View File

@@ -1,6 +1,7 @@
'use strict';
const nconf = require('nconf');
const winston = require('winston');
const validator = require('validator');
const querystring = require('querystring');
const _ = require('lodash');
@@ -23,8 +24,10 @@ const url = nconf.get('url');
helpers.noScriptErrors = async function (req, res, error, httpStatus) {
if (req.body.noscript !== 'true') {
if (typeof error === 'string') {
winston.error(`${new Error(error).stack}`);
return res.status(httpStatus).send(error);
}
winston.error(`${new Error(JSON.stringify(error)).stack}`);
return res.status(httpStatus).json(error);
}
const middleware = require('../middleware');

View File

@@ -366,6 +366,8 @@ async function createAdmin() {
username: results.username,
password: results.password,
email: results.email,
}, {
emailVerification: 'verify',
});
await Groups.join('administrators', adminUid);
await Groups.show('administrators');

View File

@@ -121,7 +121,7 @@ Auth.reloadRoutes = async function (params) {
// passport seems to remove `req.session.returnTo` after it redirects
req.session.registration.returnTo = req.session.next || req.session.returnTo;
passport.authenticate(strategy.name, (err, user) => {
passport.authenticate(strategy.name, (err, user, info) => {
if (err) {
if (req.session && req.session.registration) {
delete req.session.registration;
@@ -133,7 +133,10 @@ Auth.reloadRoutes = async function (params) {
if (req.session && req.session.registration) {
delete req.session.registration;
}
return helpers.redirect(res, strategy.failureUrl !== undefined ? strategy.failureUrl : '/login');
if (info && info.message) {
return helpers.redirect(res, `/?register=${encodeURIComponent(info.message)}`);
}
return helpers.redirect(res, strategy.failureUrl || '/login');
}
res.locals.user = user;
@@ -149,7 +152,7 @@ Auth.reloadRoutes = async function (params) {
return next(err);
}
helpers.redirect(res, strategy.successUrl !== undefined ? strategy.successUrl : '/');
helpers.redirect(res, strategy.successUrl || '/');
});
});
});

View File

@@ -22,6 +22,18 @@ module.exports = function (User) {
}
}), null, true);
User.createOrQueue = async function (req, userData, opts = {}) {
const queue = await User.shouldQueueUser(req.ip);
const result = await plugins.hooks.fire('filter:register.shouldQueue', { req, userData, queue });
if (result.queue) {
await User.addToApprovalQueue({ ...userData, ip: req.ip, _opts: JSON.stringify(opts) });
return { queued: true, message: await getRegistrationQueuedMessage() };
}
const uid = await User.create(userData, opts);
return { queued: false, uid };
};
User.addToApprovalQueue = async function (userData) {
userData.username = userData.username.trim();
userData.userslug = slugify(userData.username);
@@ -31,9 +43,12 @@ module.exports = function (User) {
username: userData.username,
email: userData.email,
ip: userData.ip,
hashedPassword: hashedPassword,
_opts: userData._opts || '{}',
};
const results = await plugins.hooks.fire('filter:user.addToApprovalQueue', { data: data, userData: userData });
if (hashedPassword) {
data.hashedPassword = hashedPassword;
}
const results = await plugins.hooks.fire('filter:user.addToApprovalQueue', { data, userData });
await db.setObject(`registration:queue:name:${userData.username}`, results.data);
await db.sortedSetAdd('registration:queue', Date.now(), userData.username);
await sendNotificationToAdmins(userData.username);
@@ -69,12 +84,15 @@ module.exports = function (User) {
if (!userData) {
throw new Error('[[error:invalid-data]]');
}
const opts = parseCreateOptions(userData);
const creation_time = await db.sortedSetScore('registration:queue', username);
const uid = await User.create(userData);
await User.setUserFields(uid, {
password: userData.hashedPassword,
'password:shaWrapped': 1,
});
const uid = await User.create(userData, opts);
if (userData.hashedPassword) {
await User.setUserFields(uid, {
password: userData.hashedPassword,
'password:shaWrapped': 1,
});
}
await removeFromQueue(username);
await markNotificationRead(username);
await plugins.hooks.fire('filter:register.complete', { uid: uid });
@@ -90,6 +108,18 @@ module.exports = function (User) {
return uid;
};
function parseCreateOptions(userData) {
let opts = {};
try {
opts = JSON.parse(userData._opts || '{}');
delete userData._opts;
return opts;
} catch (err) {
winston.error(`[user.acceptRegistration] Failed to parse create options for queued user ${userData.username}: ${err.stack}`);
return {};
}
}
async function markNotificationRead(username) {
const nid = `new-register:${username}`;
const uids = await groups.getMembers('administrators', 0, -1);
@@ -120,6 +150,20 @@ module.exports = function (User) {
return false;
};
async function getRegistrationQueuedMessage() {
let message = '[[register:registration-added-to-queue]]';
if (meta.config.showAverageApprovalTime) {
const average_time = await db.getObjectField('registration:queue:approval:times', 'average');
if (average_time > 0) {
message += ` [[register:registration-queue-average-time, ${Math.floor(average_time / 60)}, ${Math.floor(average_time % 60)}]]`;
}
}
if (meta.config.autoApproveTime > 0) {
message += ` [[register:registration-queue-auto-approve-time, ${meta.config.autoApproveTime}]]`;
}
return message;
};
User.getRegistrationQueue = async function (start, stop) {
const data = await db.getSortedSetRevRangeWithScores('registration:queue', start, stop);
const keys = data.filter(Boolean).map(user => `registration:queue:name:${user.value}`);

View File

@@ -12,7 +12,8 @@ const meta = require('../meta');
const analytics = require('../analytics');
module.exports = function (User) {
User.create = async function (data) {
User.create = async function (data, opts = {}) {
opts = { emailVerification: 'send', ...opts };
data.username = data.username.trim();
data.userslug = slugify(data.username);
if (data.email !== undefined) {
@@ -27,7 +28,7 @@ module.exports = function (User) {
}
try {
return await create(data);
return await create(data, opts);
} finally {
await db.deleteObjectFields('locks', [data.username, data.email]);
}
@@ -40,7 +41,7 @@ module.exports = function (User) {
}
}
async function create(data) {
async function create(data, opts) {
const timestamp = data.timestamp || Date.now();
let userData = {
@@ -73,7 +74,6 @@ module.exports = function (User) {
userData = results.user;
const uid = await db.incrObjectField('global', 'nextUid');
const isFirstUser = uid === 1;
userData.uid = uid;
await db.setObject(`user:${uid}`, userData);
@@ -103,20 +103,8 @@ module.exports = function (User) {
User.updateDigestSetting(userData.uid, meta.config.dailyDigestFreq),
]);
if (data.email && isFirstUser) {
await User.setUserField(uid, 'email', data.email);
await User.email.confirmByUid(userData.uid);
}
await handleEmailVerification(uid, data.email, data.token, opts.emailVerification);
if (data.email && userData.uid > 1) {
if (!data.token || !await User.isInviteTokenValid(data.token, data.email)) {
await User.email.sendValidationEmail(userData.uid, {
email: data.email,
template: 'welcome',
subject: `[[email:welcome-to, ${meta.config.title || meta.config.browserTitle || 'NodeBB'}]]`,
}).catch(err => winston.error(`[user.create] Validation email failed to send\n[emailer.send] ${err.stack}`));
}
}
if (userNameChanged) {
await User.notifications.sendNameChangeNotification(userData.uid, userData.username);
}
@@ -138,6 +126,31 @@ module.exports = function (User) {
]);
}
async function handleEmailVerification(uid, email, token, method) {
if (email) {
switch (method) {
case 'verify':
await User.setUserField(uid, 'email', email);
await User.email.confirmByUid(uid);
break;
case 'send':
if (!token || !await User.isInviteTokenValid(token, email)) {
await User.email.sendValidationEmail(uid, {
email: email,
template: 'welcome',
subject: `[[email:welcome-to, ${meta.config.title || meta.config.browserTitle || 'NodeBB'}]]`,
}).catch(err => winston.error(`[user.create] Validation email failed to send\n[emailer.send] ${err.stack}`));
}
break;
case 'skip':
// explicitly do nothing
break;
default:
throw new Error(`Invalid email verification mode: ${method}`);
}
}
}
User.isDataValid = async function (userData) {
if (userData.email && !utils.isEmailValid(userData.email)) {
throw new Error('[[error:invalid-email]]');

View File

@@ -35,6 +35,9 @@
{{{ end }}}
{{{ end }}}
{./username}
{{{ if ./sso }}}
<i class="{./sso.icon}" title="{./sso.name}"></i>
{{{ end }}}
</td>
<td>
{{{ if ./emailSpam }}}

View File

@@ -185,13 +185,10 @@ describe('API', async () => {
}
// Create sample users
const adminUid = await user.create({ username: 'admin', password: '123456' });
const unprivUid = await user.create({ username: 'unpriv', password: '123456' });
const adminUid = await user.create({ username: 'admin', password: '123456', email: 'test@example.org' }, { emailVerification: 'verify' });
const unprivUid = await user.create({ username: 'unpriv', password: '123456', email: 'unpriv@example.org' }, { emailVerification: 'verify' });
const emailConfirmationUid = await user.create({ username: 'emailConf', email: 'emailConf@example.org' });
await user.setUserField(adminUid, 'email', 'test@example.org');
await user.setUserField(unprivUid, 'email', 'unpriv@example.org');
await user.email.confirmByUid(adminUid);
await user.email.confirmByUid(unprivUid);
mocks.get['/api/confirm/{code}'][0].example = await db.get(`confirm:byUid:${emailConfirmationUid}`);
for (let x = 0; x < 4; x++) {

View File

@@ -20,19 +20,19 @@ describe('authentication', () => {
let regularUid;
const dummyEmailerHook = async (data) => {};
before((done) => {
before(async () => {
// Attach an emailer hook so related requests do not error
plugins.hooks.register('authentication-test', {
hook: 'static:email.send',
method: dummyEmailerHook,
});
user.create({ username: 'regular', password: 'regularpwd', email: 'regular@nodebb.org' }, (err, uid) => {
assert.ifError(err);
regularUid = uid;
assert.strictEqual(uid, 1);
done();
regularUid = await user.create({
username: 'regular', password: 'regularpwd', email: 'regular@nodebb.org',
}, {
emailVerification: 'verify',
});
assert.strictEqual(regularUid, 1);
});
after(() => {
@@ -393,9 +393,9 @@ describe('authentication', () => {
it('should be able to login with email', async () => {
const email = 'ginger@nodebb.org';
const uid = await user.create({ username: 'ginger', password: '123456', email });
await user.setUserField(uid, 'email', email);
await user.email.confirmByUid(uid);
const uid = await user.create({ username: 'ginger', password: '123456', email }, {
emailVerification: 'verify',
});
const { response } = await helpers.loginUser('ginger@nodebb.org', '123456');
assert.equal(response.statusCode, 200);
});

View File

@@ -4,8 +4,8 @@ const async = require('async');
const assert = require('assert');
const nconf = require('nconf');
const request = require('../src/request');
const db = require('./mocks/databasemock');
const request = require('../src/request');
const categories = require('../src/categories');
const topics = require('../src/topics');
const user = require('../src/user');

View File

@@ -39,9 +39,9 @@ describe('Controllers', () => {
});
cid = category.cid;
fooUid = await user.create({ username: 'foo', password: 'barbar', gdpr_consent: true });
await user.setUserField(fooUid, 'email', 'foo@test.com');
await user.email.confirmByUid(fooUid);
fooUid = await user.create({ username: 'foo', password: 'barbar', gdpr_consent: true, email: 'foo@test.com' }, {
emailVerification: 'verify',
});
adminUid = await user.create({ username: 'admin', password: 'barbar', gdpr_consent: true });
await groups.join('administrators', adminUid);
@@ -395,9 +395,9 @@ describe('Controllers', () => {
it('should remove current email (only allowed if email not required)', async () => {
meta.config.requireEmailAddress = 0;
const uid = await user.create({ username: 'interstiuser5' });
await user.setUserField(uid, 'email', 'interstiuser5@nodebb.org');
await user.email.confirmByUid(uid);
const uid = await user.create({
username: 'interstiuser5', email: 'interstiuser5@nodebb.org',
}, { emailVerification: 'verify' });
const result = await user.interstitials.email({
userData: { uid: uid, updateEmail: true },
@@ -418,9 +418,11 @@ describe('Controllers', () => {
it('should require a password (if one is set) for email change', async () => {
try {
const [username, password] = [utils.generateUUID().slice(0, 10), utils.generateUUID()];
const uid = await user.create({ username, password });
await user.setUserField(uid, 'email', `${username}@nodebb.org`);
await user.email.confirmByUid(uid);
const uid = await user.create({
username, password, email: `${username}@nodebb.org`,
}, {
emailVerification: 'verify',
});
const result = await user.interstitials.email({
userData: { uid: uid, updateEmail: true },
@@ -441,9 +443,11 @@ describe('Controllers', () => {
try {
const [username, password] = [utils.generateUUID().slice(0, 10), utils.generateUUID()];
const uid = await user.create({ username, password });
await user.setUserField(uid, 'email', `${username}@nodebb.org`);
await user.email.confirmByUid(uid);
const uid = await user.create({
username, password, email: `${username}@nodebb.org`,
}, {
emailVerification: 'verify',
});
const result = await user.interstitials.email({
userData: { uid: uid, updateEmail: true },
@@ -463,9 +467,11 @@ describe('Controllers', () => {
it('should successfully issue validation request if the correct password is passed in', async () => {
const [username, password] = [utils.generateUUID().slice(0, 10), utils.generateUUID()];
const uid = await user.create({ username, password });
await user.setUserField(uid, 'email', `${username}@nodebb.org`);
await user.email.confirmByUid(uid);
const uid = await user.create({
username, password, email: `${username}@nodebb.org`,
}, {
emailVerification: 'verify',
});
const result = await user.interstitials.email({
userData: { uid: uid, updateEmail: true },

View File

@@ -148,8 +148,9 @@ describe('emailer', () => {
let recipientUid;
before(async () => {
recipientUid = await user.create({ username: 'recipient', email: 'test@example.org' });
await user.email.confirmByUid(recipientUid);
recipientUid = await user.create({ username: 'recipient', email: 'test@example.org' }, {
emailVerification: 'verify',
});
});
it('should not send email to a banned user', async () => {

View File

@@ -38,7 +38,9 @@ describe('Flags', () => {
});
// Create some stuff to flag
uid1 = await User.create({ username: 'testUser', password: 'abcdef', email: 'b@c.com' });
uid1 = await User.create({ username: 'testUser', password: 'abcdef', email: 'b@c.com' }, {
emailVerification: 'verify',
});
adminUid = await User.create({ username: 'testUser2', password: 'abcdef', email: 'c@d.com' });
await Groups.join('administrators', adminUid);

View File

@@ -73,12 +73,16 @@ describe('Groups', () => {
testUid = await User.create({
username: 'testuser',
email: 'b@c.com',
}, {
emailVerification: 'verify',
});
adminUid = await User.create({
username: 'admin',
email: 'admin@admin.com',
password: '123456',
}, {
emailVerification: 'verify',
});
await Groups.join('administrators', adminUid);
});

View File

@@ -32,18 +32,16 @@ describe('socket.io', () => {
before(async () => {
const data = await Promise.all([
user.create({ username: 'admin', password: 'adminpwd' }),
user.create({ username: 'regular', password: 'regularpwd' }),
user.create({ username: 'regular', password: 'regularpwd', email: 'regular@test.com' }, { emailVerification: 'verify' }),
categories.create({
name: 'Test Category',
description: 'Test category created by testing script',
}),
]);
adminUid = data[0];
await groups.join('administrators', data[0]);
await groups.join('administrators', adminUid);
regularUid = data[1];
await user.setUserField(regularUid, 'email', 'regular@test.com');
await user.email.confirmByUid(regularUid);
cid = data[2].cid;
await topics.post({

View File

@@ -72,11 +72,12 @@ describe('User', () => {
describe('.create(), when created', () => {
it('should be created properly', async () => {
testUid = await User.create({ username: userData.username, password: userData.password });
testUid = await User.create({
username: userData.username, password: userData.password, email: userData.email,
}, {
emailVerification: 'verify',
});
assert.ok(testUid);
await User.setUserField(testUid, 'email', userData.email);
await User.email.confirmByUid(testUid);
});
it('should be created properly', async () => {
@@ -694,12 +695,12 @@ describe('User', () => {
let csrf_token;
before(async () => {
const newUid = await User.create({ username: 'updateprofile', email: 'update@me.com', password: '123456' });
const newUid = await User.create({
username: 'updateprofile', email: 'update@me.com', password: '123456',
}, {
emailVerification: 'verify',
});
uid = newUid;
await User.setUserField(uid, 'email', 'update@me.com');
await User.email.confirmByUid(uid);
({ jar, csrf_token } = await helpers.loginUser('updateprofile', '123456'));
});
@@ -734,9 +735,11 @@ describe('User', () => {
let uid;
it('should update a user\'s profile', async () => {
uid = await User.create({ username: 'justforupdate', email: 'just@for.updated', password: '123456' });
await User.setUserField(uid, 'email', 'just@for.updated');
await User.email.confirmByUid(uid);
uid = await User.create({
username: 'justforupdate', email: 'just@for.updated', password: '123456',
}, {
emailVerification: 'verify',
});
const data = {
uid: uid,
@@ -772,9 +775,9 @@ describe('User', () => {
it('should not change the username to escaped version', async () => {
const uid = await User.create({
username: 'ex\'ample_user', email: '13475@test.com', password: '123456',
}, {
emailVerification: 'verify',
});
await User.setUserField(uid, 'email', '13475@test.com');
await User.email.confirmByUid(uid);
const data = {
uid: uid,
@@ -1418,9 +1421,12 @@ describe('User', () => {
it('should send digests', async () => {
const oldValue = meta.config.includeUnverifiedEmails;
meta.config.includeUnverifiedEmails = true;
const uid = await User.create({ username: 'digest' });
await User.setUserField(uid, 'email', 'email@test.com');
await User.email.confirmByUid(uid);
const uid = await User.create({
username: 'digest', email: 'email@test.com',
}, {
emailVerification: 'verify',
});
await User.digest.execute({
interval: 'day',
subscribers: [uid],
@@ -1893,7 +1899,7 @@ describe('User', () => {
privateGroup: groups.create({ name: PRIVATE_GROUP, private: 1 }),
hiddenGroup: groups.create({ name: HIDDEN_GROUP, hidden: 1 }),
notAnInviter: User.create({ username: 'notAnInviter', password: COMMON_PW }),
inviter: User.create({ username: 'inviter', password: COMMON_PW }),
inviter: User.create({ username: 'inviter', password: COMMON_PW, email: 'invited@nodebb.org' }, { emailVerification: 'verify' }),
admin: User.create({ username: 'adminInvite', password: COMMON_PW }),
});
@@ -1901,12 +1907,10 @@ describe('User', () => {
inviterUid = results.inviter;
adminUid = results.admin;
await User.setUserField(inviterUid, 'email', 'inviter@nodebb.org');
await Promise.all([
groups.create({ name: OWN_PRIVATE_GROUP, ownerUid: inviterUid, private: 1 }),
groups.join('administrators', adminUid),
groups.join('cid:0:privileges:invite', inviterUid),
User.email.confirmByUid(inviterUid),
]);
});

View File

@@ -16,9 +16,11 @@ describe('Password reset (library methods)', () => {
let uid;
let code;
before(async () => {
uid = await user.create({ username: 'resetuser', password: '123456' });
await user.setUserField(uid, 'email', 'reset@me.com');
await user.email.confirmByUid(uid);
uid = await user.create({
username: 'resetuser', password: '123456', email: 'reset@me.com',
}, {
emailVerification: 'verify',
});
});
it('.generate() should generate a new reset code', (done) => {
@@ -117,10 +119,10 @@ describe('locks', () => {
let email;
beforeEach(async () => {
const [username, password] = [utils.generateUUID().slice(0, 10), utils.generateUUID()];
uid = await user.create({ username, password });
email = `${username}@nodebb.org`;
await user.setUserField(uid, 'email', email);
await user.email.confirmByUid(uid);
uid = await user.create({ username, password, email }, {
emailVerification: 'verify',
});
});
it('should disallow reset request if one was made within the minute', async () => {