Introduce updateWorkspaceMemberSettings and clarify product (#19441)
## Summary Introduces a dedicated **metadata** mutation to update **standard (non-custom)** workspace member settings, moves profile-related UI to use it, and aligns **workspace member** record permissions with the rest of the CRM so users cannot escalate visibility via RLS by editing their own member record. ## Product behaviour ### Profile and appearance (standard fields) - Users can still update **their own** standard workspace member fields that the product exposes in **Settings / Profile** (e.g. name, locale, color scheme, avatar flow) via the new **`updateWorkspaceMemberSettings`** mutation. - The mutation returns a **boolean**; the app **merges** the updated fields into local state so the UI stays in sync without refetching the full workspace member record. - **Locale** changes also keep **`userWorkspace`** in sync when a locale is present in the payload (including from the workspace `updateOne` path when applicable). ### Custom fields on workspace members - The dedicated metadata mutation **rejects** any **custom** workspace member field (and unknown keys). Those updates must go through the normal **object** `updateOne` pipeline, which is subject to **object- and field-level** permissions like other records. But since we don't have object- and field-level permission configuration for system objects yet, this permission is derived from Workspace member settings permission. - **Workspace member** is no longer exempt from ORM permission validation for updates merely because it is a **system** object. Users who **do not** have workspace member access (e.g. no **Workspace members** settings permission and no equivalent broad settings access on the role) **cannot** use `updateOne` on `workspaceMember` to change **custom** (or other) fields on their own row—even though that row is used for RLS predicates. - This closes a path where someone could widen what they can see by writing to fields that drive row-level rules. ### Who can change another member - Updating **another** user’s workspace member still requires **Workspace members** (or equivalent) settings permission, consistent with admin tooling.
This commit is contained in:
Marie
+302
@@ -0,0 +1,302 @@
|
||||
import { Test, type TestingModule } from '@nestjs/testing';
|
||||
import { getRepositoryToken } from '@nestjs/typeorm';
|
||||
|
||||
import { PermissionFlagType } from 'twenty-shared/constants';
|
||||
import { STANDARD_OBJECTS } from 'twenty-shared/metadata';
|
||||
import { type Repository } from 'typeorm';
|
||||
|
||||
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
|
||||
import { FieldPermissionEntity } from 'src/engine/metadata-modules/object-permission/field-permission/field-permission.entity';
|
||||
import { ObjectPermissionEntity } from 'src/engine/metadata-modules/object-permission/object-permission.entity';
|
||||
import { PermissionFlagEntity } from 'src/engine/metadata-modules/permission-flag/permission-flag.entity';
|
||||
import { RoleEntity } from 'src/engine/metadata-modules/role/role.entity';
|
||||
import { WorkspaceRolesPermissionsCacheService } from 'src/engine/metadata-modules/role/services/workspace-roles-permissions-cache.service';
|
||||
import { RowLevelPermissionPredicateGroupEntity } from 'src/engine/metadata-modules/row-level-permission-predicate/entities/row-level-permission-predicate-group.entity';
|
||||
import { RowLevelPermissionPredicateEntity } from 'src/engine/metadata-modules/row-level-permission-predicate/entities/row-level-permission-predicate.entity';
|
||||
|
||||
const WORKSPACE_ID = '20202020-0000-4000-8000-000000000000';
|
||||
const ROLE_ID = '11111111-1111-4111-8111-111111111111';
|
||||
const WORKSPACE_MEMBER_OBJECT_METADATA_ID =
|
||||
'22222222-2222-4222-8222-222222222222';
|
||||
const WORKFLOW_OBJECT_METADATA_ID = '33333333-3333-4333-8333-333333333333';
|
||||
const PERSON_OBJECT_METADATA_ID = '44444444-4444-4444-8444-444444444444';
|
||||
|
||||
const createBaseRole = (
|
||||
overrides: Partial<RoleEntity> &
|
||||
Pick<RoleEntity, 'permissionFlags' | 'objectPermissions'>,
|
||||
): RoleEntity =>
|
||||
({
|
||||
id: ROLE_ID,
|
||||
label: 'Test role',
|
||||
workspaceId: WORKSPACE_ID,
|
||||
canUpdateAllSettings: false,
|
||||
canAccessAllTools: false,
|
||||
canReadAllObjectRecords: false,
|
||||
canUpdateAllObjectRecords: false,
|
||||
canSoftDeleteAllObjectRecords: false,
|
||||
canDestroyAllObjectRecords: false,
|
||||
description: null,
|
||||
icon: null,
|
||||
isEditable: true,
|
||||
canBeAssignedToUsers: true,
|
||||
canBeAssignedToAgents: true,
|
||||
canBeAssignedToApiKeys: true,
|
||||
fieldPermissions: [],
|
||||
rowLevelPermissionPredicates: [],
|
||||
rowLevelPermissionPredicateGroups: [],
|
||||
...overrides,
|
||||
}) as RoleEntity;
|
||||
|
||||
describe('WorkspaceRolesPermissionsCacheService', () => {
|
||||
let service: WorkspaceRolesPermissionsCacheService;
|
||||
let roleRepository: jest.Mocked<Pick<Repository<RoleEntity>, 'find'>>;
|
||||
let objectMetadataRepository: jest.Mocked<
|
||||
Pick<Repository<ObjectMetadataEntity>, 'find'>
|
||||
>;
|
||||
let objectPermissionRepository: jest.Mocked<
|
||||
Pick<Repository<ObjectPermissionEntity>, 'find'>
|
||||
>;
|
||||
let permissionFlagRepository: jest.Mocked<
|
||||
Pick<Repository<PermissionFlagEntity>, 'find'>
|
||||
>;
|
||||
|
||||
const workspaceObjectMetadataFixture: ObjectMetadataEntity[] = [
|
||||
{
|
||||
id: WORKSPACE_MEMBER_OBJECT_METADATA_ID,
|
||||
isSystem: true,
|
||||
universalIdentifier: STANDARD_OBJECTS.workspaceMember.universalIdentifier,
|
||||
labelIdentifierFieldMetadataId: null,
|
||||
} as ObjectMetadataEntity,
|
||||
{
|
||||
id: WORKFLOW_OBJECT_METADATA_ID,
|
||||
isSystem: true,
|
||||
universalIdentifier: STANDARD_OBJECTS.workflow.universalIdentifier,
|
||||
labelIdentifierFieldMetadataId: null,
|
||||
} as ObjectMetadataEntity,
|
||||
{
|
||||
id: PERSON_OBJECT_METADATA_ID,
|
||||
isSystem: false,
|
||||
universalIdentifier: STANDARD_OBJECTS.person.universalIdentifier,
|
||||
labelIdentifierFieldMetadataId: null,
|
||||
} as ObjectMetadataEntity,
|
||||
];
|
||||
|
||||
beforeEach(async () => {
|
||||
roleRepository = {
|
||||
find: jest.fn(),
|
||||
};
|
||||
|
||||
objectMetadataRepository = {
|
||||
find: jest.fn().mockResolvedValue(workspaceObjectMetadataFixture),
|
||||
};
|
||||
|
||||
objectPermissionRepository = {
|
||||
find: jest.fn().mockResolvedValue([]),
|
||||
};
|
||||
|
||||
permissionFlagRepository = {
|
||||
find: jest.fn().mockResolvedValue([]),
|
||||
};
|
||||
const fieldPermissionRepository = {
|
||||
find: jest.fn().mockResolvedValue([]),
|
||||
};
|
||||
const rowLevelPermissionPredicateRepository = {
|
||||
find: jest.fn().mockResolvedValue([]),
|
||||
};
|
||||
const rowLevelPermissionPredicateGroupRepository = {
|
||||
find: jest.fn().mockResolvedValue([]),
|
||||
};
|
||||
|
||||
const module: TestingModule = await Test.createTestingModule({
|
||||
providers: [
|
||||
WorkspaceRolesPermissionsCacheService,
|
||||
{
|
||||
provide: getRepositoryToken(ObjectMetadataEntity),
|
||||
useValue: objectMetadataRepository,
|
||||
},
|
||||
{
|
||||
provide: getRepositoryToken(RoleEntity),
|
||||
useValue: roleRepository,
|
||||
},
|
||||
{
|
||||
provide: getRepositoryToken(ObjectPermissionEntity),
|
||||
useValue: objectPermissionRepository,
|
||||
},
|
||||
{
|
||||
provide: getRepositoryToken(PermissionFlagEntity),
|
||||
useValue: permissionFlagRepository,
|
||||
},
|
||||
{
|
||||
provide: getRepositoryToken(FieldPermissionEntity),
|
||||
useValue: fieldPermissionRepository,
|
||||
},
|
||||
{
|
||||
provide: getRepositoryToken(RowLevelPermissionPredicateEntity),
|
||||
useValue: rowLevelPermissionPredicateRepository,
|
||||
},
|
||||
{
|
||||
provide: getRepositoryToken(RowLevelPermissionPredicateGroupEntity),
|
||||
useValue: rowLevelPermissionPredicateGroupRepository,
|
||||
},
|
||||
],
|
||||
}).compile();
|
||||
|
||||
service = module.get(WorkspaceRolesPermissionsCacheService);
|
||||
});
|
||||
|
||||
describe('workspaceMember object', () => {
|
||||
it('should deny all record permissions when role has neither workspace members access nor update-all-settings', async () => {
|
||||
roleRepository.find.mockResolvedValue([
|
||||
createBaseRole({
|
||||
permissionFlags: [],
|
||||
objectPermissions: [],
|
||||
}),
|
||||
]);
|
||||
|
||||
const result = await service.computeForCache(WORKSPACE_ID);
|
||||
const workspaceMemberPermissions =
|
||||
result[ROLE_ID][WORKSPACE_MEMBER_OBJECT_METADATA_ID];
|
||||
|
||||
expect(workspaceMemberPermissions.canReadObjectRecords).toBe(true);
|
||||
expect(workspaceMemberPermissions.canUpdateObjectRecords).toBe(false);
|
||||
expect(workspaceMemberPermissions.canSoftDeleteObjectRecords).toBe(false);
|
||||
expect(workspaceMemberPermissions.canDestroyObjectRecords).toBe(false);
|
||||
});
|
||||
|
||||
it('should grant all record permissions when role has WORKSPACE_MEMBERS permission flag', async () => {
|
||||
permissionFlagRepository.find.mockResolvedValue([
|
||||
{
|
||||
roleId: ROLE_ID,
|
||||
flag: PermissionFlagType.WORKSPACE_MEMBERS,
|
||||
} as PermissionFlagEntity,
|
||||
]);
|
||||
|
||||
roleRepository.find.mockResolvedValue([
|
||||
createBaseRole({
|
||||
permissionFlags: [],
|
||||
objectPermissions: [],
|
||||
}),
|
||||
]);
|
||||
|
||||
const result = await service.computeForCache(WORKSPACE_ID);
|
||||
const workspaceMemberPermissions =
|
||||
result[ROLE_ID][WORKSPACE_MEMBER_OBJECT_METADATA_ID];
|
||||
|
||||
expect(workspaceMemberPermissions.canReadObjectRecords).toBe(true);
|
||||
expect(workspaceMemberPermissions.canUpdateObjectRecords).toBe(true);
|
||||
expect(workspaceMemberPermissions.canSoftDeleteObjectRecords).toBe(true);
|
||||
expect(workspaceMemberPermissions.canDestroyObjectRecords).toBe(true);
|
||||
});
|
||||
|
||||
it('should grant all record permissions when role has canUpdateAllSettings', async () => {
|
||||
roleRepository.find.mockResolvedValue([
|
||||
createBaseRole({
|
||||
canUpdateAllSettings: true,
|
||||
permissionFlags: [],
|
||||
objectPermissions: [],
|
||||
}),
|
||||
]);
|
||||
|
||||
const result = await service.computeForCache(WORKSPACE_ID);
|
||||
const workspaceMemberPermissions =
|
||||
result[ROLE_ID][WORKSPACE_MEMBER_OBJECT_METADATA_ID];
|
||||
|
||||
expect(workspaceMemberPermissions.canReadObjectRecords).toBe(true);
|
||||
expect(workspaceMemberPermissions.canUpdateObjectRecords).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('workflow object', () => {
|
||||
it('should deny all record permissions when role has neither workflows access nor update-all-settings', async () => {
|
||||
roleRepository.find.mockResolvedValue([
|
||||
createBaseRole({
|
||||
permissionFlags: [],
|
||||
objectPermissions: [],
|
||||
}),
|
||||
]);
|
||||
|
||||
const result = await service.computeForCache(WORKSPACE_ID);
|
||||
const workflowPermissions = result[ROLE_ID][WORKFLOW_OBJECT_METADATA_ID];
|
||||
|
||||
expect(workflowPermissions.canReadObjectRecords).toBe(false);
|
||||
expect(workflowPermissions.canUpdateObjectRecords).toBe(false);
|
||||
expect(workflowPermissions.canSoftDeleteObjectRecords).toBe(false);
|
||||
expect(workflowPermissions.canDestroyObjectRecords).toBe(false);
|
||||
});
|
||||
|
||||
it('should grant all record permissions when role has WORKFLOWS permission flag', async () => {
|
||||
permissionFlagRepository.find.mockResolvedValue([
|
||||
{
|
||||
roleId: ROLE_ID,
|
||||
flag: PermissionFlagType.WORKFLOWS,
|
||||
} as PermissionFlagEntity,
|
||||
]);
|
||||
|
||||
roleRepository.find.mockResolvedValue([
|
||||
createBaseRole({
|
||||
permissionFlags: [],
|
||||
objectPermissions: [],
|
||||
}),
|
||||
]);
|
||||
|
||||
const result = await service.computeForCache(WORKSPACE_ID);
|
||||
const workflowPermissions = result[ROLE_ID][WORKFLOW_OBJECT_METADATA_ID];
|
||||
|
||||
expect(workflowPermissions.canReadObjectRecords).toBe(true);
|
||||
expect(workflowPermissions.canUpdateObjectRecords).toBe(true);
|
||||
expect(workflowPermissions.canSoftDeleteObjectRecords).toBe(true);
|
||||
expect(workflowPermissions.canDestroyObjectRecords).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('regular object (person)', () => {
|
||||
it('should apply object permission overrides when object is not system', async () => {
|
||||
objectPermissionRepository.find.mockResolvedValue([
|
||||
{
|
||||
roleId: ROLE_ID,
|
||||
objectMetadataId: PERSON_OBJECT_METADATA_ID,
|
||||
canReadObjectRecords: true,
|
||||
canUpdateObjectRecords: false,
|
||||
canSoftDeleteObjectRecords: false,
|
||||
canDestroyObjectRecords: false,
|
||||
} as ObjectPermissionEntity,
|
||||
]);
|
||||
|
||||
roleRepository.find.mockResolvedValue([
|
||||
createBaseRole({
|
||||
permissionFlags: [],
|
||||
objectPermissions: [],
|
||||
}),
|
||||
]);
|
||||
|
||||
const result = await service.computeForCache(WORKSPACE_ID);
|
||||
const personPermissions = result[ROLE_ID][PERSON_OBJECT_METADATA_ID];
|
||||
|
||||
expect(personPermissions.canReadObjectRecords).toBe(true);
|
||||
expect(personPermissions.canUpdateObjectRecords).toBe(false);
|
||||
expect(personPermissions.canSoftDeleteObjectRecords).toBe(false);
|
||||
expect(personPermissions.canDestroyObjectRecords).toBe(false);
|
||||
});
|
||||
|
||||
it('should use role-wide CRUD defaults when no object permission row exists', async () => {
|
||||
roleRepository.find.mockResolvedValue([
|
||||
createBaseRole({
|
||||
canReadAllObjectRecords: true,
|
||||
canUpdateAllObjectRecords: true,
|
||||
canSoftDeleteAllObjectRecords: true,
|
||||
canDestroyAllObjectRecords: true,
|
||||
permissionFlags: [],
|
||||
objectPermissions: [],
|
||||
}),
|
||||
]);
|
||||
|
||||
const result = await service.computeForCache(WORKSPACE_ID);
|
||||
const personPermissions = result[ROLE_ID][PERSON_OBJECT_METADATA_ID];
|
||||
|
||||
expect(personPermissions.canReadObjectRecords).toBe(true);
|
||||
expect(personPermissions.canUpdateObjectRecords).toBe(true);
|
||||
expect(personPermissions.canSoftDeleteObjectRecords).toBe(true);
|
||||
expect(personPermissions.canDestroyObjectRecords).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
+57
-39
@@ -28,6 +28,8 @@ const WORKFLOW_STANDARD_OBJECT_UNIVERSAL_IDENTIFIERS = [
|
||||
STANDARD_OBJECTS.workflowRun.universalIdentifier,
|
||||
STANDARD_OBJECTS.workflowVersion.universalIdentifier,
|
||||
] as const;
|
||||
const WORKSPACE_MEMBER_OBJECT_UNIVERSAL_IDENTIFIER =
|
||||
STANDARD_OBJECTS.workspaceMember.universalIdentifier;
|
||||
|
||||
@Injectable()
|
||||
@WorkspaceCache('rolesPermissions')
|
||||
@@ -137,47 +139,66 @@ export class WorkspaceRolesPermissionsCacheService extends WorkspaceCacheProvide
|
||||
let canDestroy = role.canDestroyAllObjectRecords;
|
||||
const restrictedFields: RestrictedFieldsPermissions = {};
|
||||
|
||||
if (
|
||||
const isWorkspaceMemberObject =
|
||||
universalIdentifier === WORKSPACE_MEMBER_OBJECT_UNIVERSAL_IDENTIFIER;
|
||||
const isWorkflowRelatedObject =
|
||||
WORKFLOW_STANDARD_OBJECT_UNIVERSAL_IDENTIFIERS.includes(
|
||||
universalIdentifier as (typeof WORKFLOW_STANDARD_OBJECT_UNIVERSAL_IDENTIFIERS)[number],
|
||||
)
|
||||
) {
|
||||
const hasWorkflowsPermissions = this.hasWorkflowsPermissions(
|
||||
role,
|
||||
rolePermissionFlags,
|
||||
);
|
||||
|
||||
if (isWorkflowRelatedObject) {
|
||||
const hasWorkflowsPermissions =
|
||||
this.hasSettingsGatedObjectPermissions(
|
||||
role,
|
||||
rolePermissionFlags,
|
||||
PermissionFlagType.WORKFLOWS,
|
||||
);
|
||||
|
||||
canRead = hasWorkflowsPermissions;
|
||||
canUpdate = hasWorkflowsPermissions;
|
||||
canSoftDelete = hasWorkflowsPermissions;
|
||||
canDestroy = hasWorkflowsPermissions;
|
||||
} else {
|
||||
const objectRecordPermissionsOverride = roleObjectPermissions.find(
|
||||
(objectPermission) =>
|
||||
objectPermission.objectMetadataId === objectMetadataId,
|
||||
);
|
||||
if (isWorkspaceMemberObject) {
|
||||
const hasWorkspaceMembersPermissions =
|
||||
this.hasSettingsGatedObjectPermissions(
|
||||
role,
|
||||
rolePermissionFlags,
|
||||
PermissionFlagType.WORKSPACE_MEMBERS,
|
||||
);
|
||||
|
||||
const getPermissionValue = (
|
||||
overrideValue: boolean | undefined,
|
||||
defaultValue: boolean,
|
||||
) => (isSystem ? true : (overrideValue ?? defaultValue));
|
||||
canRead = true;
|
||||
canUpdate = hasWorkspaceMembersPermissions;
|
||||
canSoftDelete = hasWorkspaceMembersPermissions;
|
||||
canDestroy = hasWorkspaceMembersPermissions;
|
||||
} else {
|
||||
const objectRecordPermissionsOverride = roleObjectPermissions.find(
|
||||
(objectPermission) =>
|
||||
objectPermission.objectMetadataId === objectMetadataId,
|
||||
);
|
||||
|
||||
canRead = getPermissionValue(
|
||||
objectRecordPermissionsOverride?.canReadObjectRecords,
|
||||
canRead,
|
||||
);
|
||||
canUpdate = getPermissionValue(
|
||||
objectRecordPermissionsOverride?.canUpdateObjectRecords,
|
||||
canUpdate,
|
||||
);
|
||||
canSoftDelete = getPermissionValue(
|
||||
objectRecordPermissionsOverride?.canSoftDeleteObjectRecords,
|
||||
canSoftDelete,
|
||||
);
|
||||
canDestroy = getPermissionValue(
|
||||
objectRecordPermissionsOverride?.canDestroyObjectRecords,
|
||||
canDestroy,
|
||||
);
|
||||
const getPermissionValue = (
|
||||
overrideValue: boolean | undefined,
|
||||
defaultValue: boolean,
|
||||
) => (isSystem ? true : (overrideValue ?? defaultValue));
|
||||
|
||||
canRead = getPermissionValue(
|
||||
objectRecordPermissionsOverride?.canReadObjectRecords,
|
||||
canRead,
|
||||
);
|
||||
canUpdate = getPermissionValue(
|
||||
objectRecordPermissionsOverride?.canUpdateObjectRecords,
|
||||
canUpdate,
|
||||
);
|
||||
canSoftDelete = getPermissionValue(
|
||||
objectRecordPermissionsOverride?.canSoftDeleteObjectRecords,
|
||||
canSoftDelete,
|
||||
);
|
||||
canDestroy = getPermissionValue(
|
||||
objectRecordPermissionsOverride?.canDestroyObjectRecords,
|
||||
canDestroy,
|
||||
);
|
||||
}
|
||||
|
||||
const fieldPermissionsForObject = roleFieldPermissions.filter(
|
||||
(fieldPermission) =>
|
||||
@@ -246,21 +267,18 @@ export class WorkspaceRolesPermissionsCacheService extends WorkspaceCacheProvide
|
||||
return workspaceObjectMetadata;
|
||||
}
|
||||
|
||||
private hasWorkflowsPermissions(
|
||||
private hasSettingsGatedObjectPermissions(
|
||||
role: RoleEntity,
|
||||
permissionFlags: PermissionFlagEntity[],
|
||||
permissionFlagType: PermissionFlagType,
|
||||
): boolean {
|
||||
const hasWorkflowsPermissionFromRole = role.canUpdateAllSettings;
|
||||
const hasWorkflowsPermissionsFromSettingPermissions = isDefined(
|
||||
const hasPermissionFromRole = role.canUpdateAllSettings;
|
||||
const hasPermissionFromSettingPermissions = isDefined(
|
||||
permissionFlags.find(
|
||||
(permissionFlag) =>
|
||||
permissionFlag.flag === PermissionFlagType.WORKFLOWS,
|
||||
(permissionFlag) => permissionFlag.flag === permissionFlagType,
|
||||
),
|
||||
);
|
||||
|
||||
return (
|
||||
hasWorkflowsPermissionFromRole ||
|
||||
hasWorkflowsPermissionsFromSettingPermissions
|
||||
);
|
||||
return hasPermissionFromRole || hasPermissionFromSettingPermissions;
|
||||
}
|
||||
}
|
||||
|
||||
+1
@@ -177,6 +177,7 @@ export class ViewQueryParamsService {
|
||||
await this.globalWorkspaceOrmManager.getRepository<WorkspaceMemberWorkspaceEntity>(
|
||||
workspaceId,
|
||||
'workspaceMember',
|
||||
{ shouldBypassPermissionChecks: true },
|
||||
);
|
||||
|
||||
const workspaceMember = await workspaceMemberRepository.findOne({
|
||||
|
||||
Reference in New Issue
Block a user