Skip to content

Commit f581e70

Browse files
committed
refactor(api): implement permissions middleware for enhanced access control
1 parent 3a7f517 commit f581e70

File tree

2 files changed

+61
-35
lines changed

2 files changed

+61
-35
lines changed

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

Lines changed: 13 additions & 35 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 { checkPermissions, 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';
@@ -856,31 +856,6 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
856856
throw new Meteor.Error('invalid-params', validatorFunc.errors?.map((error: any) => error.message).join('\n '));
857857
}
858858
}
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-
884859
if (
885860
this.userId &&
886861
(await api.processTwoFactor({
@@ -935,14 +910,17 @@ export class APIClass<TBasePath extends string = '', TOperations extends Record<
935910
this.router[method.toLowerCase() as 'get' | 'post' | 'put' | 'delete'](
936911
`/${route}`.replaceAll('//', '/'),
937912
{ ..._options, tags } as TypedOptions,
938-
authenticationMiddlewareForHono(this, {
939-
authRequired: options.authRequired,
940-
authOrAnonRequired: options.authOrAnonRequired,
941-
userWithoutUsername: options.userWithoutUsername,
942-
logger,
943-
}),
944-
license(_options as TypedOptions, License),
945-
(operations[method as keyof Operations<TPathPattern, TOptions>] as Record<string, any>).action,
913+
[
914+
authenticationMiddlewareForHono(this, {
915+
authRequired: options.authRequired,
916+
authOrAnonRequired: options.authOrAnonRequired,
917+
userWithoutUsername: options.userWithoutUsername,
918+
logger,
919+
}),
920+
shouldVerifyPermissions && permissionsMiddleware(_options as TypedOptions),
921+
license(_options as TypedOptions, License),
922+
(operations[method as keyof Operations<TPathPattern, TOptions>] as Record<string, any>).action,
923+
].filter(Boolean),
946924
);
947925
this._routes.push({
948926
path: route,
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)