Skip to content

Commit bad4d78

Browse files
committed
refactor(api): implement permissions middleware for enhanced access control
1 parent b8b8611 commit bad4d78

File tree

2 files changed

+51
-29
lines changed

2 files changed

+51
-29
lines changed

apps/meteor/app/api/server/ApiClass.ts

Lines changed: 3 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ import type { RateLimiterOptionsToCheck } from 'meteor/rate-limit';
1818
import { RateLimiter } from 'meteor/rate-limit';
1919
import _ from 'underscore';
2020

21-
import type { PermissionsPayload } from './api.helpers';
22-
import { checkPermissionsForInvocation, checkPermissions, parseDeprecation } from './api.helpers';
21+
import { parseDeprecation } from './api.helpers';
2322
import type {
2423
FailureResult,
2524
ForbiddenResult,
@@ -42,6 +41,7 @@ import type {
4241
import { getUserInfo } from './helpers/getUserInfo';
4342
import { parseJsonQuery } from './helpers/parseJsonQuery';
4443
import { authenticationMiddlewareForHono } from './middlewares/authenticationHono';
44+
import { permissionsMiddleware } from './middlewares/permissions';
4545
import type { APIActionContext } from './router';
4646
import { RocketChatAPIRouter } from './router';
4747
import { license } from '../../../ee/app/api-enterprise/server/middlewares/license';
@@ -781,8 +781,6 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
781781

782782
const operations = endpoints;
783783

784-
const shouldVerifyPermissions = checkPermissions(options);
785-
786784
// Allow for more than one route using the same option and endpoints
787785
if (!Array.isArray(subpaths)) {
788786
subpaths = [subpaths];
@@ -856,31 +854,6 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
856854
throw new Meteor.Error('invalid-params', validatorFunc.errors?.map((error: any) => error.message).join('\n '));
857855
}
858856
}
859-
if (shouldVerifyPermissions) {
860-
if (!this.userId) {
861-
if (applyBreakingChanges) {
862-
throw new Meteor.Error('error-unauthorized', 'You must be logged in to do this');
863-
}
864-
throw new Meteor.Error('error-unauthorized', 'User does not have the permissions required for this action');
865-
}
866-
if (
867-
!(await checkPermissionsForInvocation(
868-
this.userId,
869-
_options.permissionsRequired as PermissionsPayload,
870-
this.request.method as Method,
871-
))
872-
) {
873-
if (applyBreakingChanges) {
874-
throw new Meteor.Error('error-forbidden', 'User does not have the permissions required for this action', {
875-
permissions: _options.permissionsRequired,
876-
});
877-
}
878-
throw new Meteor.Error('error-unauthorized', 'User does not have the permissions required for this action', {
879-
permissions: _options.permissionsRequired,
880-
});
881-
}
882-
}
883-
884857
if (
885858
this.userId &&
886859
(await api.processTwoFactor({
@@ -941,6 +914,7 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
941914
userWithoutUsername: options.userWithoutUsername,
942915
logger,
943916
}),
917+
permissionsMiddleware(_options as TypedOptions),
944918
license(_options as TypedOptions, License),
945919
(operations[method as keyof Operations<TPathPattern, TOptions>] as Record<string, any>).action,
946920
);
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import type { Method } from '@rocket.chat/rest-typings';
2+
import type { MiddlewareHandler } from 'hono';
3+
4+
import { applyBreakingChanges } from '../ApiClass';
5+
import { API } from '../api';
6+
import { type PermissionsPayload, checkPermissionsForInvocation } from '../api.helpers';
7+
import type { TypedOptions } from '../definition';
8+
import type { HonoContext } from '../router';
9+
10+
export const permissionsMiddleware =
11+
(options: TypedOptions): MiddlewareHandler =>
12+
async (c: HonoContext, next) => {
13+
if (!options.permissionsRequired) {
14+
return next();
15+
}
16+
17+
const user = c.get('user');
18+
19+
if (!user) {
20+
const message = applyBreakingChanges
21+
? 'You must be logged in to do this'
22+
: 'User does not have the permissions required for this action';
23+
24+
const failure = API.v1.failure({ error: message, message });
25+
26+
return c.json(failure.body, failure.statusCode);
27+
}
28+
29+
const hasPermission = await checkPermissionsForInvocation(
30+
user._id,
31+
options.permissionsRequired as PermissionsPayload,
32+
c.req.method as Method,
33+
);
34+
35+
if (!hasPermission) {
36+
const message = 'User does not have the permissions required for this action';
37+
38+
if (applyBreakingChanges) {
39+
const failure = API.v1.failure({ error: message, message });
40+
return c.json(failure.body, failure.statusCode);
41+
}
42+
43+
const failure = API.v1.failure({ error: message, message });
44+
return c.json(failure.body, failure.statusCode);
45+
}
46+
47+
return next();
48+
};

0 commit comments

Comments
 (0)