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

feat: Allow granting the mobile-upload-file permission to guests #34191

Merged
merged 12 commits into from
Jan 20, 2025
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/shiny-singers-cheer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@rocket.chat/meteor": minor
---

Allows granting the `mobile-upload-file` permission to guests
8 changes: 7 additions & 1 deletion apps/meteor/app/authorization/client/restrictedRoles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ Meteor.startup(async () => {
const result = await sdk.call('license:isEnterprise');
if (result) {
// #ToDo: Load this from the server with an API call instead of having a duplicate list
AuthorizationUtils.addRolePermissionWhiteList('guest', ['view-d-room', 'view-joined-room', 'view-p-room', 'start-discussion']);
AuthorizationUtils.addRolePermissionWhiteList('guest', [
'view-d-room',
'view-joined-room',
'view-p-room',
'start-discussion',
'mobile-upload-file',
]);
}
});
8 changes: 6 additions & 2 deletions apps/meteor/app/authorization/lib/AuthorizationUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const restrictedRolePermissions = new Map();
const restrictedRolePermissions = new Map<string, Set<string>>();

export const AuthorizationUtils = class {
static addRolePermissionWhiteList(roleId: string, list: string[]): void {
Expand All @@ -17,7 +17,7 @@ export const AuthorizationUtils = class {
const rules = restrictedRolePermissions.get(roleId);

for (const permissionId of list) {
rules.add(permissionId);
rules?.add(permissionId);
}
}

Expand Down Expand Up @@ -51,4 +51,8 @@ export const AuthorizationUtils = class {

return false;
}

static hasRestrictionsToRole(roleId: string): boolean {
return restrictedRolePermissions.has(roleId);
}
};
2 changes: 2 additions & 0 deletions apps/meteor/app/authorization/server/constant/permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ export const permissions = [
{ _id: 'set-owner', roles: ['admin', 'owner'] },
{ _id: 'send-many-messages', roles: ['admin', 'bot', 'app'] },
{ _id: 'set-leader', roles: ['admin', 'owner'] },
{ _id: 'start-discussion', roles: ['admin', 'user', 'guest', 'app'] },
{ _id: 'start-discussion-other-user', roles: ['admin', 'user', 'owner', 'app'] },
scuciatto marked this conversation as resolved.
Show resolved Hide resolved
{ _id: 'unarchive-room', roles: ['admin'] },
{ _id: 'view-c-room', roles: ['admin', 'user', 'bot', 'app', 'anonymous'] },
{ _id: 'user-generate-access-token', roles: ['admin'] },
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { License } from '@rocket.chat/core-services';
import type { ServerMethods } from '@rocket.chat/ddp-client';
import { Permissions } from '@rocket.chat/models';
import { Meteor } from 'meteor/meteor';
Expand All @@ -15,6 +16,10 @@ declare module '@rocket.chat/ddp-client' {

Meteor.methods<ServerMethods>({
async 'authorization:addPermissionToRole'(permissionId, role) {
if (role === 'guest' && !AuthorizationUtils.hasRestrictionsToRole(role) && (await License.hasValidLicense())) {
AuthorizationUtils.addRolePermissionWhiteList(role, await License.getGuestPermissions());
}
scuciatto marked this conversation as resolved.
Show resolved Hide resolved

if (AuthorizationUtils.isPermissionRestrictedForRole(permissionId, role)) {
throw new Meteor.Error('error-action-not-allowed', 'Permission is restricted', {
method: 'authorization:addPermissionToRole',
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/ee/app/authorization/lib/guestPermissions.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
// This list is currently duplicated on the client code as there's no available API to load it from the server
export const guestPermissions = ['view-d-room', 'view-joined-room', 'view-p-room', 'start-discussion'];
export const guestPermissions = ['view-d-room', 'view-joined-room', 'view-p-room', 'start-discussion', 'mobile-upload-file'];
118 changes: 118 additions & 0 deletions apps/meteor/tests/end-to-end/api/guest-permissions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { expect } from 'chai';
import { before, describe, it, after } from 'mocha';

import { getCredentials, api, request, credentials, methodCall } from '../../data/api-data';
import { restorePermissionToRoles } from '../../data/permissions.helper';
import { IS_EE } from '../../e2e/config/constants';

(IS_EE ? describe : describe.skip)('[Guest Permissions]', () => {
const guestPermissions = ['view-d-room', 'view-joined-room', 'view-p-room', 'start-discussion', 'mobile-upload-file'];

before((done) => getCredentials(done));

after(() => Promise.all(guestPermissions.map((permissionName) => restorePermissionToRoles(permissionName))));

function succeedRemoveGuestPermission(permissionName: string) {
it(`should allow removing the whitelisted permission ${permissionName} from the guest role`, async () => {
const res = await request
.post(methodCall('authorization:removeRoleFromPermission'))
.set(credentials)
.send({
message: JSON.stringify({
method: 'authorization:removeRoleFromPermission',
params: [permissionName, 'guest'],
id: 'id',
msg: 'method',
}),
})
.expect('Content-Type', 'application/json')
.expect(200);

expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('message');
const message = JSON.parse(res.body.message);
expect(message).to.not.have.property('error');

const permissionsListRes = await request
.get(api('permissions.listAll'))
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200);
expect(permissionsListRes.body).to.have.property('success', true);
expect(permissionsListRes.body).to.have.property('update').and.to.be.an('array');
expect(permissionsListRes.body).to.have.property('remove').and.to.be.an('array');

const updatedPermission = permissionsListRes.body.update.find((permission: any) => permission._id === permissionName);
expect(updatedPermission).to.have.property('_id', permissionName);
expect(updatedPermission).to.have.property('roles').and.to.be.an('array').that.does.not.include('guest');
});
}

function succeedAddGuestPermission(permissionName: string) {
it(`should allow granting the whitelisted permission ${permissionName} to the guest role`, async () => {
const res = await request
.post(methodCall('authorization:addPermissionToRole'))
.set(credentials)
.send({
message: JSON.stringify({
method: 'authorization:addPermissionToRole',
params: [permissionName, 'guest'],
id: 'id',
msg: 'method',
}),
})
.expect('Content-Type', 'application/json')
.expect(200);

expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('message');
const message = JSON.parse(res.body.message);
expect(message).to.not.have.property('error');

const permissionsListRes = await request
.get(api('permissions.listAll'))
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200);
expect(permissionsListRes.body).to.have.property('success', true);
expect(permissionsListRes.body).to.have.property('update').and.to.be.an('array');
expect(permissionsListRes.body).to.have.property('remove').and.to.be.an('array');

const updatedPermission = permissionsListRes.body.update.find((permission: any) => permission._id === permissionName);
expect(updatedPermission).to.have.property('_id', permissionName);
expect(updatedPermission).to.have.property('roles').and.to.be.an('array').that.includes('guest');
});
}

describe('Default guest roles', () => {
guestPermissions.forEach((permissionName) => {
succeedRemoveGuestPermission(permissionName);
});

guestPermissions.forEach((permissionName) => {
succeedAddGuestPermission(permissionName);
});

it('should not allow adding a non whitelisted permission to the guest role', async () => {
const res = await request
.post(methodCall('authorization:addPermissionToRole'))
.set(credentials)
.send({
message: JSON.stringify({
method: 'authorization:addPermissionToRole',
params: ['create-c', 'guest'],
id: 'id',
msg: 'method',
}),
})
.expect('Content-Type', 'application/json')
.expect(200);

expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('message');
const message = JSON.parse(res.body.message);
expect(message).to.have.property('error');
expect(message.error).to.have.property('reason', 'Permission is restricted');
});
});
});
Loading