Skip to content
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
7 changes: 7 additions & 0 deletions .changeset/pink-moons-cheer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@rocket.chat/meteor": minor
"@rocket.chat/apps-engine": minor
"@rocket.chat/apps": minor
---

Allows apps with the right permission to read room's ABAC attributes.
17 changes: 17 additions & 0 deletions apps/meteor/app/apps/server/converters/rooms.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { secureFieldsMapper } from '@rocket.chat/apps/dist/lib/SecureFields';
import { RoomType } from '@rocket.chat/apps-engine/definition/rooms';
import { LivechatVisitors, Rooms, LivechatDepartment, Users, LivechatContacts } from '@rocket.chat/models';

Expand Down Expand Up @@ -435,6 +436,22 @@ export class AppRoomsConverter {

return this.orch.getConverters().get('rooms').convertById(prid);
},
...secureFieldsMapper((room) => {
if (!room.abacAttributes) {
return undefined;
}

const value = [
{
permission: 'abac.read',
name: 'abacAttributes',
value: room.abacAttributes,
},
];

delete room.abacAttributes;
return value;
}),
};

return transformMappedData(originalRoom, map);
Expand Down
12 changes: 12 additions & 0 deletions packages/apps-engine/src/definition/abac/AbacAttributes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export interface IAbacAttributeDefinition {
/**
* Validation expectation (NOT enforced here, must be enforced by caller):
* /^[A-Za-z0-9_-]+$/
*/
key: string;

/**
* List of string values for this attribute key.
*/
values: string[];
}
3 changes: 3 additions & 0 deletions packages/apps-engine/src/definition/rooms/IRoom.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { IAbacAttributeDefinition } from '../abac/AbacAttributes';
import type { IUser } from '../users';
import type { RoomType } from './RoomType';

Expand Down Expand Up @@ -25,4 +26,6 @@ export interface IRoom {
customFields?: { [key: string]: any };
parentRoom?: IRoom;
livechatData?: { [key: string]: any };

abacAttributes?: IAbacAttributeDefinition[];
}
18 changes: 15 additions & 3 deletions packages/apps/deno-runtime/lib/codec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import { Buffer } from 'node:buffer';
import { Decoder, Encoder, ExtensionCodec } from '@msgpack/msgpack';

import { decode, Decoder, Encoder, ExtensionCodec } from '@msgpack/msgpack';
import type { App as _App } from '@rocket.chat/apps-engine/definition/App';

import { require } from './require.ts';
import { applySecureFields, type WithSecureFields } from './secureFields.ts';

const FUNCTION_DISABLER_EXT = 0;
const BUFFER_HANDLER_EXT = 1;
const SECURE_FIELDS_HANDLER_EXT = 2;

const { App } = require('@rocket.chat/apps-engine/definition/App.js') as {
App: typeof _App;
Expand All @@ -11,7 +17,7 @@ const { App } = require('@rocket.chat/apps-engine/definition/App.js') as {
const extensionCodec = new ExtensionCodec();

extensionCodec.register({
type: 0,
type: FUNCTION_DISABLER_EXT,
encode: (object: unknown) => {
// We don't care about functions, but also don't want to throw an error
if (typeof object === 'function' || object instanceof App) {
Expand All @@ -25,7 +31,7 @@ extensionCodec.register({

// Since Deno doesn't have Buffer by default, we need to use Uint8Array
extensionCodec.register({
type: 1,
type: BUFFER_HANDLER_EXT,
encode: (object: unknown) => {
if (object instanceof Buffer) {
return new Uint8Array(object.buffer, object.byteOffset, object.byteLength);
Expand All @@ -39,5 +45,11 @@ extensionCodec.register({
},
});

extensionCodec.register({
type: SECURE_FIELDS_HANDLER_EXT,
encode: (_object: unknown) => null,
decode: (data: Uint8Array) => applySecureFields(decode(data, { extensionCodec }) as WithSecureFields<Record<string, unknown>>),
});

export const encoder = new Encoder({ extensionCodec });
export const decoder = new Decoder({ extensionCodec });
4 changes: 4 additions & 0 deletions packages/apps/deno-runtime/lib/room.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { IAbacAttributeDefinition } from '@rocket.chat/apps-engine/definition/abac/AbacAttributes';
import type { IRoom } from '@rocket.chat/apps-engine/definition/rooms/IRoom';
import type { RoomType } from '@rocket.chat/apps-engine/definition/rooms/RoomType';
import type { IUser } from '@rocket.chat/apps-engine/definition/users/IUser';
Expand Down Expand Up @@ -42,6 +43,8 @@ export class Room {

public userIds?: Array<string>;

public abacAttributes?: IAbacAttributeDefinition[];

private _USERNAMES: Promise<Array<string>> | undefined;

private [PrivateManager]: IRoomManager | undefined;
Expand Down Expand Up @@ -88,6 +91,7 @@ export class Room {
lastModifiedAt: this.lastModifiedAt,
customFields: this.customFields,
userIds: this.userIds,
abacAttributes: this.abacAttributes,
};
}

Expand Down
26 changes: 26 additions & 0 deletions packages/apps/deno-runtime/lib/secureFields.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { kSecureFields, WithSecureFields } from '@rocket.chat/apps/dist/lib/SecureFields';
import type { App } from '@rocket.chat/apps-engine/definition/App';

import { AppObjectRegistry } from '../AppObjectRegistry.ts';

export type { WithSecureFields } from '@rocket.chat/apps/dist/lib/SecureFields';

export function applySecureFields(object: WithSecureFields<Record<string, unknown>>) {
const { [kSecureFields]: secureFields, ...rest } = object;

const app = AppObjectRegistry.get<App>('app');

if (!app) {
throw new Error("App unavailable, can't parse object with secure fields");
}

secureFields.forEach(({ permission, name, value }) => {
if (!app.getInfo().permissions?.find((p) => p.name === permission)) {
return;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

rest[name] = value;
});

return rest;
}
59 changes: 59 additions & 0 deletions packages/apps/deno-runtime/lib/tests/secureFields.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { assertEquals, assertThrows } from 'https://deno.land/std@0.203.0/assert/mod.ts';
import { beforeEach, describe, it } from 'https://deno.land/std@0.203.0/testing/bdd.ts';

import { AppObjectRegistry } from '../../AppObjectRegistry.ts';
import { applySecureFields } from '../secureFields.ts';

const SECURE_FIELDS_KEY = '@@SecureFields';

describe('applySecureFields', () => {
beforeEach(() => {
AppObjectRegistry.clear();
});

it('throws when app is unavailable', () => {
assertThrows(
() => applySecureFields({ foo: 'bar', [SECURE_FIELDS_KEY]: [] } as any),
Error,
"App unavailable, can't parse object with secure fields",
);
});

it('applies only secure fields with matching permissions', () => {
AppObjectRegistry.set('app', {
getInfo: () => ({
permissions: [{ name: 'abac.read' }],
}),
});

const parsed = applySecureFields({
foo: 'bar',
[SECURE_FIELDS_KEY]: [
{ permission: 'abac.read', name: 'abacAttributes', value: { department: 'support' } },
{ permission: 'api.read', name: 'apiToken', value: 'secret' },
],
} as any);

assertEquals(parsed, {
foo: 'bar',
abacAttributes: { department: 'support' },
});
});

it('overwrites an existing field when permission is granted', () => {
AppObjectRegistry.set('app', {
getInfo: () => ({
permissions: [{ name: 'abac.read' }],
}),
});

const parsed = applySecureFields({
abacAttributes: null,
[SECURE_FIELDS_KEY]: [{ permission: 'abac.read', name: 'abacAttributes', value: { tenant: 'alpha' } }],
} as any);

assertEquals(parsed, {
abacAttributes: { tenant: 'alpha' },
});
});
});
15 changes: 9 additions & 6 deletions packages/apps/deno-runtime/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,20 @@ async function requestRouter({ type, payload }: Messenger.JsonRpcRequest): Promi
const logger = new Logger(method);

const context: RequestContext = Object.assign(payload, {
context: { logger }
})
context: { logger },
});

const [methodPrefix] = method.split(':') as [keyof Handlers];
const handler = methodHandlers[methodPrefix];

if (!handler) {
return Messenger.errorResponse({
error: { message: 'Method not found', code: -32601 },
id,
}, context);
return Messenger.errorResponse(
{
error: { message: 'Method not found', code: -32601 },
id,
},
context,
);
}

const result = await handler(context);
Expand Down
19 changes: 19 additions & 0 deletions packages/apps/src/lib/SecureFields.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
export type SecureFieldDescriptor<T extends Record<string, unknown>, K extends keyof T> = {
permission: string;
name: K;
value: T[K];
};

export const kSecureFields = '@@SecureFields';

export type WithSecureFields<T extends Record<string, unknown>> = T & { [kSecureFields]: SecureFieldDescriptor<T, keyof T>[] };

export function secureFieldsMapper<T extends Record<string, unknown>>(
mapper: (object: T) => SecureFieldDescriptor<T, keyof T>[] | undefined,
) {
return { [kSecureFields]: mapper };
}

export function hasSecureFields(object: unknown): boolean {
return !!object?.[kSecureFields];
}
Comment thread
d-gubert marked this conversation as resolved.
21 changes: 18 additions & 3 deletions packages/apps/src/server/bridges/RoomBridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,25 @@ import { AppPermissions } from '../permissions/AppPermissions';
export { GetMessagesSortableFields };
export type { GetMessagesOptions, GetRoomsFilters, GetRoomsOptions };

const READ_ONLY_ROOM_FIELDS = ['abacAttributes'] as const;

const stripReadOnlyRoomFields = (room: IRoom): IRoom => {
for (const field of READ_ONLY_ROOM_FIELDS) {
delete room[field];

// This prevents the field being added when a room gets created
// since on room creation `isPartial=false` and so these props are spread on the final room object
// If we need to allow abac room creation from apps, we would need to remove this.
delete (room as any)._unmappedProperties_?.[field];
}

return room;
};

export abstract class RoomBridge extends BaseBridge {
public async doCreate(room: IRoom, members: Array<string>, appId: string): Promise<string> {
if (this.hasWritePermission(appId)) {
return this.create(room, members, appId);
return this.create(stripReadOnlyRoomFields(room), members, appId);
}
}

Expand Down Expand Up @@ -67,7 +82,7 @@ export abstract class RoomBridge extends BaseBridge {

public async doUpdate(room: IRoom, members: Array<string>, appId: string): Promise<void> {
if (this.hasWritePermission(appId)) {
return this.update(room, members, appId);
return this.update(stripReadOnlyRoomFields(room), members, appId);
}
}

Expand All @@ -79,7 +94,7 @@ export abstract class RoomBridge extends BaseBridge {
appId: string,
): Promise<string> {
if (this.hasWritePermission(appId)) {
return this.createDiscussion(room, parentMessage, reply, members, appId);
return this.createDiscussion(stripReadOnlyRoomFields(room), parentMessage, reply, members, appId);
}
}

Expand Down
3 changes: 3 additions & 0 deletions packages/apps/src/server/permissions/AppPermissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ export const AppPermissions = {
'experimental': {
default: { name: 'experimental.default' },
},
'abac': {
read: { name: 'abac.read' },
},
};

/**
Expand Down
39 changes: 36 additions & 3 deletions packages/apps/src/server/runtime/deno/codec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import { Decoder as _Decoder, Encoder as _Encoder, ExtensionCodec } from '@msgpack/msgpack';
import { Decoder as _Decoder, Encoder as _Encoder, encode, ExtensionCodec } from '@msgpack/msgpack';

import { hasSecureFields } from '../../../lib/SecureFields';

const extensionCodec = new ExtensionCodec();

const FUNCTION_DISABLER_EXT = 0;
const BUFFER_HANDLER_EXT = 1;
const SECURE_FIELDS_HANDLER_EXT = 2;

extensionCodec.register({
type: 0,
type: FUNCTION_DISABLER_EXT,
encode: (object: unknown) => {
// We don't care about functions, but also don't want to throw an error
if (typeof object === 'function') {
Expand All @@ -16,7 +22,7 @@ extensionCodec.register({

// We need to handle Buffers because Deno needs its own decoding
extensionCodec.register({
type: 1,
type: BUFFER_HANDLER_EXT,
encode: (object: unknown) => {
if (object instanceof Buffer) {
return new Uint8Array(object.buffer, object.byteOffset, object.byteLength);
Expand All @@ -27,6 +33,33 @@ extensionCodec.register({
decode: (data: Uint8Array) => Buffer.from(data),
});

extensionCodec.register({
type: SECURE_FIELDS_HANDLER_EXT,
/**
* This extension doesn't really change the encoding process, but by
* not returning null or undefined, msgpack attributes the decoding of this
* object to this extension, allowing us to handle secure field logic on the
* subprocess side, without having to iterate through all objects in search
* of the field.
*/
encode: (object: unknown, context: { ignoreRoot?: boolean } = {}) => {
// Ignoring the root object allows msgpack to take care of encoding the object's properties,
// while we mark the root object itself as an extension type.
if (context?.ignoreRoot) {
context.ignoreRoot = false;

return null;
}

if (hasSecureFields(object)) {
return encode(object, { extensionCodec, context: { ignoreRoot: true } });
}
},

// We don't really need to handle decoding here, as the subprocess will never send a message with secure fields
decode: (_data: Uint8Array) => undefined,
});

/**
* The Encoder and Decoder classes perform "stateful" operations, i.e. they read from a
* stream, store the data locally and decode it from its buffer.
Expand Down
Loading
Loading