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] Uncessary updates on Settings, Roles and Permissions on startup #17160

Merged
merged 14 commits into from
May 4, 2020
Merged
2 changes: 1 addition & 1 deletion app/api/server/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ export class APIClass extends Restivus {
'error-unauthorized': 'unauthorized',
}[e.error] || 'failure';

result = API.v1[apiMethod](typeof e === 'string' ? e : e.message, e.error, undefined, e);
result = API.v1[apiMethod](typeof e === 'string' ? e : e.message, e.error, process.env.TEST_MODE ? e.stack : undefined, e);
} finally {
delete Accounts._accountData[connection.id];
}
Expand Down
2 changes: 1 addition & 1 deletion app/api/server/v1/chat.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Match, check } from 'meteor/check';
import { Messages } from '../../../models';
import { canAccessRoom, hasPermission } from '../../../authorization';
import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser';
import { processWebhookMessage } from '../../../lib';
import { processWebhookMessage } from '../../../lib/server';
import { API } from '../api';
import Rooms from '../../../models/server/models/Rooms';
import Users from '../../../models/server/models/Users';
Expand Down
2 changes: 1 addition & 1 deletion app/api/server/v1/permissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Meteor } from 'meteor/meteor';
import { Match, check } from 'meteor/check';

import { hasPermission } from '../../../authorization';
import { Permissions, Roles } from '../../../models';
import { Permissions, Roles } from '../../../models/server';
import { API } from '../api';

/**
Expand Down
2 changes: 1 addition & 1 deletion app/api/server/v1/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Match, check } from 'meteor/check';
import { ServiceConfiguration } from 'meteor/service-configuration';
import _ from 'underscore';

import { Settings } from '../../../models';
import { Settings } from '../../../models/server';
import { hasPermission } from '../../../authorization';
import { API } from '../api';

Expand Down
2 changes: 1 addition & 1 deletion app/apps/server/orchestrator.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class AppServerOrchestrator {

initialize() {
this._rocketchatLogger = new Logger('Rocket.Chat Apps');
Permissions.createOrUpdate('manage-apps', ['admin']);
Permissions.create('manage-apps', ['admin']);

this._marketplaceUrl = 'https://marketplace.rocket.chat';

Expand Down
23 changes: 14 additions & 9 deletions app/authorization/server/startup.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint no-multi-spaces: 0 */
import { Meteor } from 'meteor/meteor';

import { Roles, Permissions, Settings } from '../../models';
import { Roles, Permissions, Settings } from '../../models/server';
import { settings } from '../../settings/server';
import { getSettingPermissionId, CONSTANTS } from '../lib';
import { clearCache } from './functions/hasPermission';
Expand Down Expand Up @@ -114,9 +114,7 @@ Meteor.startup(function() {
];

for (const permission of permissions) {
if (!Permissions.findOneById(permission._id)) {
Permissions.upsert(permission._id, { $set: permission });
}
Permissions.create(permission._id, permission.roles);
}

const defaultRoles = [
Expand All @@ -134,7 +132,7 @@ Meteor.startup(function() {
];

for (const role of defaultRoles) {
Roles.upsert({ _id: role.name }, { $setOnInsert: { scope: role.scope, description: role.description || '', protected: true, mandatory2fa: false } });
Roles.createOrUpdate(role.name, role.scope, role.description, true, false);
sampaiodiego marked this conversation as resolved.
Show resolved Hide resolved
}

const getPreviousPermissions = function(settingId) {
Expand All @@ -155,27 +153,34 @@ Meteor.startup(function() {
const createSettingPermission = function(setting, previousSettingPermissions) {
const permissionId = getSettingPermissionId(setting._id);
const permission = {
_id: permissionId,
level: CONSTANTS.SETTINGS_LEVEL,
// copy those setting-properties which are needed to properly publish the setting-based permissions
settingId: setting._id,
group: setting.group,
section: setting.section,
sorter: setting.sorter,
roles: [],
};
// copy previously assigned roles if available
if (previousSettingPermissions[permissionId] && previousSettingPermissions[permissionId].roles) {
permission.roles = previousSettingPermissions[permissionId].roles;
} else {
permission.roles = [];
}
if (setting.group) {
permission.groupPermissionId = getSettingPermissionId(setting.group);
}
if (setting.section) {
permission.sectionPermissionId = getSettingPermissionId(setting.section);
}
Permissions.upsert(permission._id, { $set: permission });

const existent = Permissions.findOne({
_id: permissionId,
...permission,
}, { fields: { _id: 1 } });

if (!existent) {
Permissions.upsert({ _id: permissionId }, { $set: permission });
Copy link
Member

Choose a reason for hiding this comment

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

insert?

Copy link
Member Author

Choose a reason for hiding this comment

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

The permission may exist with different properties

}

delete previousSettingPermissions[permissionId];
};

Expand Down
4 changes: 1 addition & 3 deletions app/channel-settings-mail-messages/server/lib/startup.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,5 @@ Meteor.startup(function() {
_id: 'mail-messages',
roles: ['admin'],
};
return Permissions.upsert(permission._id, {
$setOnInsert: permission,
});
return Permissions.create(permission._id, permission.roles);
});
6 changes: 3 additions & 3 deletions app/channel-settings/server/startup.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Meteor } from 'meteor/meteor';
import { Permissions } from '../../models';

Meteor.startup(function() {
Permissions.upsert('post-readonly', { $setOnInsert: { roles: ['admin', 'owner', 'moderator'] } });
Permissions.upsert('set-readonly', { $setOnInsert: { roles: ['admin', 'owner'] } });
Permissions.upsert('set-react-when-readonly', { $setOnInsert: { roles: ['admin', 'owner'] } });
Permissions.create('post-readonly', ['admin', 'owner', 'moderator']);
Permissions.create('set-readonly', ['admin', 'owner']);
Permissions.create('set-react-when-readonly', ['admin', 'owner']);
});
2 changes: 1 addition & 1 deletion app/cloud/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { Permissions } from '../../models';
import { settings } from '../../settings/server';

if (Permissions) {
Permissions.createOrUpdate('manage-cloud', ['admin']);
Permissions.create('manage-cloud', ['admin']);
}

const licenseCronName = 'Cloud Workspace Sync';
Expand Down
2 changes: 1 addition & 1 deletion app/custom-sounds/server/startup/permissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ import { Permissions } from '../../../models';

Meteor.startup(() => {
if (Permissions) {
Permissions.createOrUpdate('manage-sounds', ['admin']);
Permissions.create('manage-sounds', ['admin']);
}
});
4 changes: 1 addition & 3 deletions app/discussion/server/permissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ Meteor.startup(() => {
];

for (const permission of permissions) {
if (!Permissions.findOneById(permission._id)) {
Permissions.upsert(permission._id, { $set: permission });
}
Permissions.create(permission._id, permission.roles);
}
});
2 changes: 1 addition & 1 deletion app/lib/server/functions/createDirectRoom.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Rooms, Subscriptions } from '../../../models/server';
import { settings } from '../../../settings/lib/settings';
import { settings } from '../../../settings/server';
import { getDefaultSubscriptionPref } from '../../../utils/server';
import { callbacks } from '../../../callbacks/server';

Expand Down
2 changes: 1 addition & 1 deletion app/lib/server/startup/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -2766,7 +2766,7 @@ settings.addGroup('Setup_Wizard', function() {
secret: true,
});

this.add('Cloud_Workspace_Access_Token_Expires_At', new Date(), {
this.add('Cloud_Workspace_Access_Token_Expires_At', new Date(0), {
type: 'date',
hidden: true,
readonly: true,
Expand Down
7 changes: 1 addition & 6 deletions app/mail-messages/server/startup.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,5 @@ import { Meteor } from 'meteor/meteor';
import { Permissions } from '../../models';

Meteor.startup(function() {
return Permissions.upsert('access-mailer', {
$setOnInsert: {
_id: 'access-mailer',
roles: ['admin'],
},
});
return Permissions.create('access-mailer', ['admin']);
});
6 changes: 1 addition & 5 deletions app/message-pin/server/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,5 @@ Meteor.startup(function() {
group: 'Message',
public: true,
});
return Permissions.upsert('pin-message', {
$setOnInsert: {
roles: ['owner', 'moderator', 'admin'],
},
});
return Permissions.create('pin-message', ['owner', 'moderator', 'admin']);
});
6 changes: 1 addition & 5 deletions app/message-snippet/server/startup/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,5 @@ Meteor.startup(function() {
public: true,
group: 'Message',
});
Permissions.upsert('snippet-message', {
$setOnInsert: {
roles: ['owner', 'moderator', 'admin'],
},
});
Permissions.create('snippet-message', ['owner', 'moderator', 'admin']);
});
23 changes: 21 additions & 2 deletions app/models/server/models/Permissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,34 @@ export class Permissions extends Base {
}

createOrUpdate(name, roles) {
const exists = this.findOne({
_id: name,
roles,
}, { fields: { _id: 1 } });

if (exists) {
return exists._id;
}

this.upsert({ _id: name }, { $set: { roles } });
}

create(name, roles) {
const exists = this.findOneById(name, { fields: { _id: 1 } });

if (exists) {
return exists._id;
}

this.upsert({ _id: name }, { $set: { roles } });
}

addRole(permission, role) {
this.update({ _id: permission }, { $addToSet: { roles: role } });
this.update({ _id: permission, roles: { $ne: role } }, { $addToSet: { roles: role } });
}

removeRole(permission, role) {
this.update({ _id: permission }, { $pull: { roles: role } });
this.update({ _id: permission, roles: role }, { $pull: { roles: role } });
}
}

Expand Down
27 changes: 14 additions & 13 deletions app/models/server/models/Roles.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,22 @@ export class Roles extends Base {
});
}

createOrUpdate(name, scope = 'Users', description, protectedRole, mandatory2fa) {
const updateData = {};
updateData.name = name;
updateData.scope = scope;

if (description != null) {
updateData.description = description;
}
createOrUpdate(name, scope = 'Users', description = '', protectedRole = true, mandatory2fa = false) {
const updateData = {
name,
scope,
description,
protected: protectedRole,
mandatory2fa,
};

if (protectedRole) {
updateData.protected = protectedRole;
}
const exists = this.findOne({
_id: name,
...updateData,
}, { fields: { _id: 1 } });

if (mandatory2fa != null) {
updateData.mandatory2fa = mandatory2fa;
if (exists) {
return exists._id;
}

this.upsert({ _id: name }, { $set: updateData });
Expand Down
File renamed without changes.
46 changes: 0 additions & 46 deletions app/settings/client/lib/settings.js

This file was deleted.

50 changes: 50 additions & 0 deletions app/settings/client/lib/settings.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { Meteor } from 'meteor/meteor';
import { ReactiveDict } from 'meteor/reactive-dict';

import { CachedCollection } from '../../../ui-cached-collection';
import { SettingsBase, SettingValue } from '../../lib/settings';

const cachedCollection = new CachedCollection({
name: 'public-settings',
eventType: 'onAll',
userRelated: false,
listenChangesForLoggedUsersOnly: true,
});

class Settings extends SettingsBase {
cachedCollection = cachedCollection

collection = cachedCollection.collection;

dict = new ReactiveDict<any>('settings');

get(_id: string): any {
return this.dict.get(_id);
}

init(): void {
let initialLoad = true;
this.collection.find().observe({
added: (record: {_id: string; value: SettingValue}) => {
Meteor.settings[record._id] = record.value;
this.dict.set(record._id, record.value);
this.load(record._id, record.value, initialLoad);
},
changed: (record: {_id: string; value: SettingValue}) => {
Meteor.settings[record._id] = record.value;
this.dict.set(record._id, record.value);
this.load(record._id, record.value, initialLoad);
},
removed: (record: {_id: string}) => {
delete Meteor.settings[record._id];
this.dict.set(record._id, null);
this.load(record._id, undefined, initialLoad);
},
});
initialLoad = false;
}
}

export const settings = new Settings();

settings.init();
4 changes: 2 additions & 2 deletions app/settings/index.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Meteor } from 'meteor/meteor';

if (Meteor.isClient) {
module.exports = require('./client/index.js');
module.exports = require('./client/index.ts');
}
if (Meteor.isServer) {
module.exports = require('./server/index.js');
module.exports = require('./server/index.ts');
}
Loading