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

[NEW] Granular permissions for settings #8942

Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
eed869a
Allow maintenance of per-setting permissions - first draft, not beaut…
mrsimpson Nov 17, 2017
28b769b
Implicitly assign and revoke setting group permissions
mrsimpson Nov 19, 2017
8523456
Improve Display of setting permissions
mrsimpson Nov 19, 2017
c87a30d
Add path to permission title
mrsimpson Nov 19, 2017
48b1076
Permission to access setting permissions
mrsimpson Nov 20, 2017
daccad8
Adapt wording
mrsimpson Nov 20, 2017
00e4bb5
UI-adaptation: Allow users with permission 'manage-selected-permissio…
mrsimpson Nov 21, 2017
9b71b62
Don't adapt sorting on the client side
mrsimpson Nov 24, 2017
293ad73
Fix: Apply changed setting permissions reactively
mrsimpson Nov 26, 2017
8f59f1c
Move setting-based permissions to own collection
mrsimpson Nov 27, 2017
8d923c2
Unify collections for setting and other permissions again into one
mrsimpson Nov 27, 2017
a7fdc87
Get rid of frontend exceptions on changing selected settings
mrsimpson Nov 29, 2017
f007231
- Sort permissions by group
mrsimpson Nov 29, 2017
47d8749
Merge branch 'develop' into core/#8771-fine-granular-settings-permiss…
mrsimpson Dec 6, 2017
c706dd4
Allow administration even if user has got only `edit-privileged-setti…
mrsimpson Dec 7, 2017
f26246b
fix css-selectors for permission inputs
mrsimpson Jan 26, 2018
cd336fe
Merge branch 'core/#8771-fine-granular-settings-permissions' of https…
mrsimpson Jan 26, 2018
c7d7145
Merge remote-tracking branch 'upstream/master' into core/#8771-fine-g…
mrsimpson Jan 26, 2018
4108b7a
Merge tag '0.68.4' into core/#8771-fine-granular-settings-permissions
mrsimpson Aug 24, 2018
02c0160
Merge remote-tracking branch 'upstream/develop' into core/#8771-fine-…
mrsimpson Aug 24, 2018
71beed0
satisfy linter
mrsimpson Aug 25, 2018
642e3b3
Fix settings based permissions test when running as part of the full …
mrsimpson Aug 25, 2018
d302fb5
Update 13a-setting-permissions.js
mrsimpson Aug 25, 2018
2a0627b
Fix availability of admin menu and permission visibility
mrsimpson Sep 4, 2018
d75b360
Merge remote-tracking branch 'upstream/develop' into core/#8771-fine-…
mrsimpson Sep 25, 2018
200902d
stylint
mrsimpson Sep 25, 2018
b138793
Reduce footprint of PR - remove everything not related to actual feature
mrsimpson Sep 25, 2018
b2e901a
[WIP] - merge latest develop
mrsimpson Aug 7, 2019
b94cb37
Fix linting issues in test
mrsimpson Aug 7, 2019
5c73a9e
merge latest develop
mrsimpson Aug 7, 2019
a1a0153
Merge remote-tracking branch 'upstream/develop' into core/#8771-fine-…
mrsimpson Aug 7, 2019
e8e8704
re-add accidentally removed permissions
mrsimpson Aug 7, 2019
a39f732
Merge remote-tracking branch 'upstream/develop' into core/#8771-fine-…
mrsimpson Aug 8, 2019
5397e75
Add UI-texts for settings
mrsimpson Aug 8, 2019
cdbf318
Adapt selector in tests
mrsimpson Aug 8, 2019
57d8f3e
Remove unneccessarily changed files
mrsimpson Aug 8, 2019
d59d490
Move changes to publications to moved files
mrsimpson Aug 8, 2019
d5a931c
Fix error handling
mrsimpson Aug 8, 2019
6e778c3
Change order in which client is notified
mrsimpson Aug 8, 2019
ddd5606
Merge remote-tracking branch 'upstream/develop' into core/#8771-fine-…
mrsimpson Aug 8, 2019
b80e1ec
Merge branch 'develop' into core/#8771-fine-granular-settings-permiss…
mrsimpson Aug 8, 2019
34d51a4
Merge branch 'develop' into core/#8771-fine-granular-settings-permiss…
mrsimpson Aug 9, 2019
9a18fcc
Adapt automated test for setting based permissions
mrsimpson Aug 12, 2019
a55afb4
Merge branch 'core/#8771-fine-granular-settings-permissions' of githu…
mrsimpson Aug 12, 2019
93a0464
[WIP] reviewing
ggazzo Aug 12, 2019
59e2aab
Permission table
ggazzo Aug 12, 2019
8419341
Adapt setting permissions test to tabbed layout
mrsimpson Aug 13, 2019
7870cd6
fix settings
ggazzo Aug 15, 2019
d238b7a
settings level constant
ggazzo Aug 15, 2019
f2e8ada
Merge branch 'core/#8771-fine-granular-settings-permissions' of githu…
ggazzo Aug 15, 2019
0858217
emiiter
ggazzo Aug 15, 2019
b09fd79
emiiter
ggazzo Aug 15, 2019
bb2af49
Merge branch 'core/#8771-fine-granular-settings-permissions' of githu…
ggazzo Aug 15, 2019
3808a30
remove useless comments
ggazzo Aug 20, 2019
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
12 changes: 6 additions & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@
"chat"
],
"scripts": {
"start": "meteor npm i && meteor",
"start": "meteor npm i && meteor run",
"debug": "meteor run --inspect",
mrsimpson marked this conversation as resolved.
Show resolved Hide resolved
"debug-brk": "meteor run --inspect-brk",
"lint": "eslint .",
"lint-fix": "eslint . --fix",
"stylelint": "stylelint packages/**/*.css",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
font-weight: bold !important;
}

& .section:not(.section-collapsed) {
inline-size: fit-content;
mrsimpson marked this conversation as resolved.
Show resolved Hide resolved
}

& .permission-grid {
& th {
position: relative;
Expand Down
108 changes: 74 additions & 34 deletions packages/rocketchat-authorization/client/views/permissions.html
Original file line number Diff line number Diff line change
@@ -1,36 +1,76 @@
<template name="permissionsTable">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Created a template for the table as both permission tables follow the same scheme.

<table border="1" class="permission-grid secondary-background-color">
<thead class="content-background-color">
<tr>
<th class="border-component-color">&nbsp;</th>
{{#each role in allRoles}}
<th class="border-component-color" title="{{role.description}}">
<a href="{{pathFor "admin-permissions-edit" name=role._id}}">
{{role._id}}
<i class="icon-edit"></i>
</a>
</th>
{{/each}}
</tr>
</thead>
<tbody>
{{#each permission in permissions}}
<tr class="admin-table-row">
<td class="permission-name border-component-color"
title="{{permissionDescription permission}}">{{permissionName permission}}<br>[{{permission._id}}]
</td>
{{#each role in allRoles}}
<td class="border-component-color">
<input type="checkbox" name="perm[{{_id}}][{{../_id}}]" class="role-permission"
value="1" checked="{{granted permission.roles role}}" data-role="{{role._id}}"
data-permission="{{permission._id}}">
</td>
{{/each}}
</tr>
{{/each}}
</tbody>
</table>
</template>
<template name="permissions">
<div class="permissions-manager">
{{#if hasPermission}}
<a href="{{pathFor "admin-permissions-new"}}" class="button primary new-role">{{_ "New_role"}}</a>
<table border="1" class="permission-grid secondary-background-color">
<thead class="content-background-color">
<tr>
<th class="border-component-color">&nbsp;</th>
{{#each role}}
<th class="border-component-color" title="{{description}}">
<a href="{{pathFor "admin-permissions-edit" name=_id}}">
{{_id}}
<i class="icon-edit"></i>
</a>
</th>
{{/each}}
</tr>
</thead>
<tbody>
{{#each permission}}
<tr class="admin-table-row">
<td class="permission-name border-component-color" title="{{_ permissionDescription}}">{{_ permissionName}}<br>[{{_id}}]</td>
{{#each role}}
<td class="border-component-color">
<input type="checkbox" name="perm[{{_id}}][{{../_id}}]" class="role-permission" value="1" checked="{{granted ../roles}}" data-role="{{_id}}" data-permission="{{../_id}}">
</td>
{{/each}}
</tr>
{{/each}}
</tbody>
</table>
{{else}}
{{_ "Not_authorized"}}
{{/if}}
</div>
<section class="page-settings">
<div class="permissions-manager">
{{#if hasPermission}}
<a href="{{pathFor "admin-permissions-new"}}" class="button primary new-role">{{_ "New_role"}}</a>
<div class="rocket-form">
<div class="section">
{{> permissionsTable permissions=permissions allRoles=roles collection='Chat'}}
</div>
</div>
{{#if hasSettingPermission}}
<div class="rocket-form">
<div class="section {{#unless settingPermissionExpanded}}section-collapsed{{/unless}}">
<div class="section-title">
<div class="section-title-text">
{{_ "Setting_permissions"}}</div>
<div class="section-title-right">
<button class="button primary js-toggle-setting-permissions"><span>
{{#if settingPermissionExpanded }}
{{_ "Collapse"}}
{{else}}
{{_ "Expand"}}
{{/if}}
</span>
</button>
</div>
</div>
<div class="section-content border-component-color">
{{#if settingPermissionExpanded }}
{{> permissionsTable permissions=settingPermissions allRoles=roles collection='Setting'}}
{{else}}
{{_ "Not_authorized"}}
{{/if}}
</div>
</div>
</div>
{{/if}}
{{else}}
{{_ "Not_authorized"}}
{{/if}}
</div>
</section>
</template>
115 changes: 89 additions & 26 deletions packages/rocketchat-authorization/client/views/permissions.js
Original file line number Diff line number Diff line change
@@ -1,66 +1,121 @@
/* globals ChatPermissions */
import {permissionLevel} from '../../lib/rocketchat';

const whereNotSetting = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

$ne is not supported by Minimongo. Thus, had to fallback to $where. Still, this does not seem to work, see the .filter-comment some lines below

$where: function() {
return this.level !== permissionLevel.SETTING;
}.toString()
};

Template.permissions.helpers({
role() {
roles() {
return Template.instance().roles.get();
},

permission() {
return ChatPermissions.find({}, {
sort: {
_id: 1
permissions() {
return ChatPermissions.find(whereNotSetting, //the $where seems to have no effect - filtered as workaround after fetch()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

any idea why the $where has no effect?

{
sort: {
_id: 1
}
}).fetch()
.filter((setting) => !setting.level);
},

settingPermissions() {
return ChatPermissions.find({
level: permissionLevel.SETTING
},
{
sort: { //sorting seems not to be copied from the publication, we need to request it explicitly in find()
group: 1,
section: 1
}
});
}).fetch()
.filter((setting) => setting.group); //group permissions are assigned implicitly, we can hide them. $exists: {group:false} not supported by Minimongo
},

granted(roles) {
hasPermission() {
return RocketChat.authz.hasAllPermission('access-permissions');
},

hasSettingPermission() {
return RocketChat.authz.hasAllPermission('access-setting-permissions');
},

settingPermissionExpanded() {
return Template.instance().settingPermissionsExpanded.get();
}
});

Template.permissions.events({
'click .js-toggle-setting-permissions'(event, instance) {
instance.settingPermissionsExpanded.set(!instance.settingPermissionsExpanded.get());
}
});

Template.permissions.onCreated(function() {
this.settingPermissionsExpanded = new ReactiveVar(false);
this.roles = new ReactiveVar([]);

Tracker.autorun(() => {
this.roles.set(RocketChat.models.Roles.find().fetch());
});
});

Template.permissionsTable.helpers({
granted(roles, role) {
if (roles) {
if (roles.indexOf(this._id) !== -1) {
if (roles.indexOf(role._id) !== -1) {
return 'checked';
}
}
},

permissionName() {
return `${ this._id }`;
},

permissionDescription() {
return `${ this._id }_description`;
permissionName(permission) {
if (permission.level === permissionLevel.SETTING) {
let path = '';
if (permission.group) {
path = `${ t(permission.group) } > `;
}
if (permission.section) {
path = `${ path }${ t(permission.section) } > `;
}
path = `${ path }${ t(permission.settingId) }`;
return path;
} else {
return t(permission._id);
}
},

hasPermission() {
return RocketChat.authz.hasAllPermission('access-permissions');
permissionDescription(permission) {
return t(`${ permission._id }_description`);
}
});

Template.permissions.events({
Template.permissionsTable.events({
'click .role-permission'(e, instance) {
const permission = e.currentTarget.getAttribute('data-permission');
const role = e.currentTarget.getAttribute('data-role');

if (instance.permissionByRole[permission].indexOf(role) === -1) {
if (!instance.permissionByRole[permission] // the permissino has this role not assigned at all (undefined)
|| instance.permissionByRole[permission].indexOf(role) === -1) {
return Meteor.call('authorization:addPermissionToRole', permission, role);
} else {
return Meteor.call('authorization:removeRoleFromPermission', permission, role);
}
}
});

Template.permissions.onCreated(function() {
this.roles = new ReactiveVar([]);
Template.permissionsTable.onCreated(function() {
this.permissionByRole = {};
this.actions = {
added: {},
removed: {}
};

Tracker.autorun(() => {
this.roles.set(RocketChat.models.Roles.find().fetch());
});

Tracker.autorun(() => {
ChatPermissions.find().observeChanges({
const observer = {
added: (id, fields) => {
this.permissionByRole[id] = fields.roles;
},
Expand All @@ -70,6 +125,14 @@ Template.permissions.onCreated(function() {
removed: (id) => {
delete this.permissionByRole[id];
}
});
};
if (this.data.collection === 'Chat') {
ChatPermissions.find(whereNotSetting).observeChanges(observer);
}

if (this.data.collection === 'Setting') {
ChatPermissions.find({level: permissionLevel.SETTING}).observeChanges(observer);
}
});
});

4 changes: 4 additions & 0 deletions packages/rocketchat-authorization/lib/rocketchat.js
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
RocketChat.authz = {};

export const permissionLevel = {
SETTING: 'setting'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can imagine that there are other levels as well, such as the permission to change dedicated roles, thus making it a map

};
Original file line number Diff line number Diff line change
@@ -1,12 +1,28 @@
import {permissionLevel} from '../../lib/rocketchat';

Meteor.methods({
'authorization:addPermissionToRole'(permission, role) {
if (!Meteor.userId() || !RocketChat.authz.hasPermission(Meteor.userId(), 'access-permissions')) {
if (!Meteor.userId() || !RocketChat.authz.hasPermission(Meteor.userId(), 'access-permissions')
|| (permission.level === permissionLevel.SETTING && !RocketChat.authz.hasPermission(Meteor.userId(), 'access-setting-permissions'))
) {
throw new Meteor.Error('error-action-not-allowed', 'Adding permission is not allowed', {
method: 'authorization:addPermissionToRole',
action: 'Adding_permission'
});
}

// for setting-based-permissions, authorize the group access as well
const addParentPermissions = function(permissionId, role) {
const permission = RocketChat.models.Permissions.findOneById(permissionId);
if (permission.groupPermissionId) {
const groupPermission = RocketChat.models.Permissions.findOneById(permission.groupPermissionId);
if (groupPermission.roles.indexOf(role) === -1) {
RocketChat.models.Permissions.addRole(permission.groupPermissionId, role);
}
}
};

addParentPermissions(permission, role);
return RocketChat.models.Permissions.addRole(permission, role);
}
});
Loading