Skip to content

Commit

Permalink
Merge branch 'develop' into VLN-82
Browse files Browse the repository at this point in the history
  • Loading branch information
@debdutdeb
debdutdeb authored Dec 24, 2024
2 parents 83d4059 + b31e7f9 commit b4e4d36
Show file tree
Hide file tree
Showing 108 changed files with 445 additions and 354 deletions.
6 changes: 6 additions & 0 deletions .changeset/popular-cameras-grin.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@rocket.chat/meteor": patch
"@rocket.chat/model-typings": patch
---

Fixes livechat conversations not being assigned to the contact manager even when the "Assign new conversations to the contact manager" setting is enabled
10 changes: 10 additions & 0 deletions .changeset/quick-rivers-ring.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"@rocket.chat/meteor": patch
---

Fixes dates being incorrect when fetching omnichannel reports with the following time periods:

last week
last month
last 6 months
last year
188 changes: 114 additions & 74 deletions apps/meteor/app/api/server/api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ import type { RateLimiterOptionsToCheck } from 'meteor/rate-limit';
import { RateLimiter } from 'meteor/rate-limit';
import type { Request, Response } from 'meteor/rocketchat:restivus';
import { Restivus } from 'meteor/rocketchat:restivus';
import semver from 'semver';
import _ from 'underscore';

import type { PermissionsPayload } from './api.helpers';
import { checkPermissionsForInvocation, checkPermissions, parseDeprecation } from './api.helpers';
import type {
FailureResult,
ForbiddenResult,
InternalError,
NotFoundResult,
Operations,
Expand All @@ -36,10 +38,16 @@ import { hasPermissionAsync } from '../../authorization/server/functions/hasPerm
import { notifyOnUserChangeAsync } from '../../lib/server/lib/notifyListener';
import { metrics } from '../../metrics/server';
import { settings } from '../../settings/server';
import { Info } from '../../utils/rocketchat.info';
import { getDefaultUserFields } from '../../utils/server/functions/getDefaultUserFields';

const logger = new Logger('API');

// We have some breaking changes planned to the API.
// To avoid conflicts or missing something during the period we are adopting a 'feature flag approach'
// TODO: MAJOR check if this is still needed
const applyBreakingChanges = semver.gte(Info.version, '8.0.0');

interface IAPIProperties {
useDefaultAuth: boolean;
prettyJson: boolean;
Expand Down Expand Up @@ -298,21 +306,34 @@ export class APIClass<TBasePath extends string = ''> extends Restivus {
statusCode: 500,
body: {
success: false,
error: msg || 'Internal error occured',
error: msg || 'Internal server error',
},
};
}

public unauthorized<T>(msg?: T): UnauthorizedResult<T> {
return {
statusCode: 403,
statusCode: 401,
body: {
success: false,
error: msg || 'unauthorized',
},
};
}

public forbidden<T>(msg?: T): ForbiddenResult<T> {
return {
statusCode: 403,
body: {
success: false,
// TODO: MAJOR remove 'unauthorized' in favor of 'forbidden'
// because of reasons beyond my control we were used to send `unauthorized` to 403 cases, to avoid a breaking change we just adapted here
// but thanks to the semver check tests should break as soon we bump to a new version
error: msg || (applyBreakingChanges ? 'forbidden' : 'unauthorized'),
},
};
}

public tooManyRequests(msg?: string): { statusCode: number; body: Record<string, any> & { success?: boolean } } {
return {
statusCode: 429,
Expand Down Expand Up @@ -577,13 +598,16 @@ export class APIClass<TBasePath extends string = ''> extends Restivus {
}

if (!this.user && !settings.get('Accounts_AllowAnonymousRead')) {
return {
statusCode: 401,
body: {
const result = api.unauthorized('You must be logged in to do this.');
// compatibility with the old API
// TODO: MAJOR
if (!applyBreakingChanges) {
Object.assign(result.body, {
status: 'error',
message: 'You must be logged in to do this.',
},
};
});
}
return result;
}
}

Expand Down Expand Up @@ -612,18 +636,29 @@ export class APIClass<TBasePath extends string = ''> extends Restivus {
throw new Meteor.Error('invalid-params', validatorFunc.errors?.map((error: any) => error.message).join('\n '));
}
}
if (
shouldVerifyPermissions &&
(!this.userId ||
if (shouldVerifyPermissions) {
if (!this.userId) {
if (applyBreakingChanges) {
throw new Meteor.Error('error-unauthorized', 'You must be logged in to do this');
}
throw new Meteor.Error('error-unauthorized', 'User does not have the permissions required for this action');
}
if (
!(await checkPermissionsForInvocation(
this.userId,
_options.permissionsRequired as PermissionsPayload,
this.request.method,
)))
) {
throw new Meteor.Error('error-unauthorized', 'User does not have the permissions required for this action', {
permissions: _options.permissionsRequired,
});
))
) {
if (applyBreakingChanges) {
throw new Meteor.Error('error-forbidden', 'User does not have the permissions required for this action', {
permissions: _options.permissionsRequired,
});
}
throw new Meteor.Error('error-unauthorized', 'User does not have the permissions required for this action', {
permissions: _options.permissionsRequired,
});
}
}

const invocation = new DDPCommon.MethodInvocation({
Expand Down Expand Up @@ -678,18 +713,26 @@ export class APIClass<TBasePath extends string = ''> extends Restivus {
responseTime: Date.now() - startTime,
});
} catch (e: any) {
const apiMethod: string =
{
'error-too-many-requests': 'tooManyRequests',
'error-unauthorized': 'unauthorized',
}[e.error as string] || 'failure';

result = (API.v1 as Record<string, any>)[apiMethod](
typeof e === 'string' ? e : e.message,
e.error,
process.env.TEST_MODE ? e.stack : undefined,
e,
);
result = ((e: any) => {
switch (e.error) {
case 'error-too-many-requests':
return API.v1.tooManyRequests(typeof e === 'string' ? e : e.message);
case 'unauthorized':
case 'error-unauthorized':
if (applyBreakingChanges) {
return API.v1.unauthorized(typeof e === 'string' ? e : e.message);
}
return API.v1.forbidden(typeof e === 'string' ? e : e.message);
case 'forbidden':
case 'error-forbidden':
if (applyBreakingChanges) {
return API.v1.forbidden(typeof e === 'string' ? e : e.message);
}
return API.v1.failure(typeof e === 'string' ? e : e.message, e.error, process.env.TEST_MODE ? e.stack : undefined, e);
default:
return API.v1.failure(typeof e === 'string' ? e : e.message, e.error, process.env.TEST_MODE ? e.stack : undefined, e);
}
})(e);

log.http({
err: e,
Expand Down Expand Up @@ -791,8 +834,8 @@ export class APIClass<TBasePath extends string = ''> extends Restivus {
// eslint-disable-next-line @typescript-eslint/no-this-alias
const self = this;

(this as APIClass<'/v1'>).addRoute<'/v1/login', { authRequired: false }>(
'login' as any,
(this as APIClass<'/v1'>).addRoute(
'login',
{ authRequired: false },
{
async post() {
Expand All @@ -803,58 +846,55 @@ export class APIClass<TBasePath extends string = ''> extends Restivus {
connection: generateConnection(getRequestIP(request) || '', this.request.headers),
});

let auth;
try {
auth = await DDP._CurrentInvocation.withValue(invocation as any, async () => Meteor.callAsync('login', args));
} catch (error: any) {
let e = error;
if (error.reason === 'User not found') {
e = {
error: 'Unauthorized',
reason: 'Unauthorized',
};
}

return {
statusCode: 401,
body: {
status: 'error',
error: e.error,
details: e.details,
message: e.reason || e.message,
const auth = await DDP._CurrentInvocation.withValue(invocation as any, async () => Meteor.callAsync('login', args));
this.user = await Users.findOne(
{
_id: auth.id,
},
} as unknown as SuccessResult<Record<string, any>>;
}
{
projection: getDefaultUserFields(),
},
);

this.user = await Users.findOne(
{
_id: auth.id,
},
{
projection: getDefaultUserFields(),
},
);

this.userId = (this.user as unknown as IUser)?._id;

const response = {
status: 'success',
data: {
userId: this.userId,
authToken: auth.token,
me: await getUserInfo(this.user || ({} as IUser)),
},
};
if (!this.user) {
return self.unauthorized();
}

const extraData = self._config.onLoggedIn?.call(this);
this.userId = this.user._id;

if (extraData != null) {
_.extend(response.data, {
extra: extraData,
const extraData = self._config.onLoggedIn?.call(this);

return self.success({
status: 'success',
data: {
userId: this.userId,
authToken: auth.token,
me: await getUserInfo(this.user || ({} as IUser)),
...(extraData && { extra: extraData }),
},
});
}
} catch (error) {
if (!(error instanceof Meteor.Error)) {
return self.internalError();
}

return response as unknown as SuccessResult<Record<string, any>>;
const result = self.unauthorized();

if (!applyBreakingChanges) {
Object.assign(result.body, {
status: 'error',
error: error.error,
details: error.details,
message: error.reason || error.message,
...(error.reason === 'User not found' && {
error: 'Unauthorized',
message: 'Unauthorized',
}),
});
}
return result;
}
},
},
);
Expand Down
19 changes: 17 additions & 2 deletions apps/meteor/app/api/server/definition.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,29 @@ export type FailureResult<T, TStack = undefined, TErrorType = undefined, TErrorD
};

export type UnauthorizedResult<T> = {
statusCode: 403;
statusCode: 401;
body: {
success: false;
error: T | 'unauthorized';
};
};

export type InternalError<T> = { statusCode: 500; body: { error: T | 'Internal error occured'; success: false } };
export type ForbiddenResult<T> = {
statusCode: 403;
body: {
success: false;
// TODO: MAJOR remove 'unauthorized'
error: T | 'forbidden' | 'unauthorized';
};
};

export type InternalError<T> = {
statusCode: 500;
body: {
error: T | 'Internal server error';
success: false;
};
};

export type NotFoundResult = {
statusCode: 404;
Expand Down
14 changes: 7 additions & 7 deletions apps/meteor/app/api/server/v1/channels.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ API.v1.addRoute(
});

if (!result) {
return API.v1.unauthorized();
return API.v1.forbidden();
}

return API.v1.success(result);
Expand Down Expand Up @@ -304,7 +304,7 @@ API.v1.addRoute(
(await hasPermissionAsync(this.userId, 'view-joined-room')) &&
!(await Subscriptions.findOneByRoomIdAndUserId(findResult._id, this.userId, { projection: { _id: 1 } }))
) {
return API.v1.unauthorized();
return API.v1.forbidden();
}

const { cursor, totalCount } = Messages.findPaginated(ourQuery, {
Expand Down Expand Up @@ -499,7 +499,7 @@ API.v1.addRoute(
}

if (channelId && !(await hasPermissionAsync(this.userId, 'edit-room', channelId))) {
return API.v1.unauthorized();
return API.v1.forbidden();
}

const room = await findChannelByIdOrName({
Expand Down Expand Up @@ -613,7 +613,7 @@ API.v1.addRoute(

if (userId) {
if (!access) {
return API.v1.unauthorized();
return API.v1.forbidden();
}
user = userId;
}
Expand Down Expand Up @@ -751,7 +751,7 @@ API.v1.addRoute(
});
} catch (e: any) {
if (e.message === 'unauthorized') {
error = API.v1.unauthorized();
error = API.v1.forbidden();
} else {
error = API.v1.failure(e.message);
}
Expand Down Expand Up @@ -801,7 +801,7 @@ API.v1.addRoute(
});

if (!(await canAccessRoomAsync(findResult, { _id: this.userId }))) {
return API.v1.unauthorized();
return API.v1.forbidden();
}

const { offset, count } = await getPaginationItems(this.queryParams);
Expand Down Expand Up @@ -1056,7 +1056,7 @@ API.v1.addRoute(
});

if (findResult.broadcast && !(await hasPermissionAsync(this.userId, 'view-broadcast-member-list', findResult._id))) {
return API.v1.unauthorized();
return API.v1.forbidden();
}

const { offset: skip, count: limit } = await getPaginationItems(this.queryParams);
Expand Down
Loading

0 comments on commit b4e4d36

Please sign in to comment.