Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: afterCreateUser callback being called before setting user's roles #30309

Merged
merged 17 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tame-ducks-turn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@rocket.chat/meteor": patch
---

Fixed a problem that caused `afterCreateUser` callback to be called without new user's roles inside. This caused Omnichannel Business Hour manager to ignore these users from assigning open business hours until the manager restarted or the business hour restarted.
15 changes: 10 additions & 5 deletions apps/meteor/app/authentication/server/startup/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,10 +265,14 @@ Accounts.onCreateUser(function (...args) {

const { insertUserDoc } = Accounts;
const insertUserDocAsync = async function (options, user) {
const globalRoles = [];
const globalRoles = new Set();

if (Match.test(options.globalRoles, [String]) && options.globalRoles.length > 0) {
options.globalRoles.map((role) => globalRoles.add(role));
}

if (Match.test(user.globalRoles, [String]) && user.globalRoles.length > 0) {
globalRoles.push(...user.globalRoles);
user.globalRoles.map((role) => globalRoles.add(role));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user.globalRoles.forEach((role) => globalRoles.add(role));

}

delete user.globalRoles;
Expand All @@ -277,11 +281,12 @@ const insertUserDocAsync = async function (options, user) {
const defaultAuthServiceRoles = parseCSV(settings.get('Accounts_Registration_AuthenticationServices_Default_Roles') || '');

if (defaultAuthServiceRoles.length > 0) {
globalRoles.push(...defaultAuthServiceRoles);
defaultAuthServiceRoles.map((role) => globalRoles.add(role));
}
}

const roles = getNewUserRoles(globalRoles);
const arrayGlobalRoles = [...globalRoles];
const roles = options.skipNewUserRolesSetting ? arrayGlobalRoles : getNewUserRoles(arrayGlobalRoles);

if (!user.type) {
user.type = 'user';
Expand Down Expand Up @@ -326,7 +331,7 @@ const insertUserDocAsync = async function (options, user) {
await addUserRolesAsync(_id, roles);

// Make user's roles to be present on callback
user = await Users.findOneById(_id, { projection: { username: 1, type: 1 } });
user = await Users.findOneById(_id, { projection: { username: 1, type: 1, roles: 1 } });

if (user.username) {
if (options.joinDefaultChannels !== false) {
Expand Down
3 changes: 2 additions & 1 deletion apps/meteor/app/lib/server/functions/saveUser.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,8 @@ const saveNewUser = async function (userData, sendPassword) {
password: userData.password,
joinDefaultChannels: userData.joinDefaultChannels,
isGuest,
globalRoles: roles,
skipNewUserRolesSetting: true,
};
if (userData.email) {
createUser.email = userData.email;
Expand All @@ -285,7 +287,6 @@ const saveNewUser = async function (userData, sendPassword) {

const updateUser = {
$set: {
roles,
...(typeof userData.name !== 'undefined' && { name: userData.name }),
settings: userData.settings || {},
},
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/lib/callbacks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ type ChainedCallbackSignatures = {

'livechat.onLoadConfigApi': (config: { room: IOmnichannelRoom }) => Record<string, unknown>;

'afterCreateUser': (user: IUser) => IUser;
'afterCreateUser': (user: AtLeast<IUser, '_id' | 'username' | 'roles'>) => IUser;
'afterDeleteRoom': (rid: IRoom['_id']) => IRoom['_id'];
'livechat:afterOnHold': (room: Pick<IOmnichannelRoom, '_id'>) => Pick<IOmnichannelRoom, '_id'>;
'livechat:afterOnHoldChatResumed': (room: Pick<IOmnichannelRoom, '_id'>) => Pick<IOmnichannelRoom, '_id'>;
Expand Down
18 changes: 18 additions & 0 deletions apps/meteor/tests/end-to-end/api/livechat/19-business-hours.ts
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,24 @@ describe('LIVECHAT - business hours', function () {
expect(latestAgent.statusLivechat).to.be.undefined;
});

describe('Special Case - Agent created, BH already enabled', () => {
let newAgent: ILivechatAgent;
let newAgentCredentials: IUserCredentialsHeader;
before(async () => {
newAgent = await createUser({ roles: ['user', 'livechat-agent'] });
newAgentCredentials = await login(newAgent.username, password);
});
after(async () => {
await deleteUser(newAgent);
});
it('should verify a newly created agent to be assigned to the default business hour', async () => {
const latestAgent: ILivechatAgent = await getMe(newAgentCredentials as any);
expect(latestAgent).to.be.an('object');
expect(latestAgent.openBusinessHours).to.be.an('array').of.length(1);
expect(latestAgent?.openBusinessHours?.[0]).to.be.equal(defaultBH._id);
});
});

after(async () => {
await deleteUser(agent._id);
});
Expand Down
Loading