Skip to content

Commit

Permalink
Fix missing name validation on object names at update (twentyhq#5434)
Browse files Browse the repository at this point in the history
## Context
as per title

## How was it tested? 
local (/metadata + in product)
  • Loading branch information
ijreilly authored May 16, 2024
1 parent d741f4a commit 1694e0c
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 44 deletions.
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import {
ForbiddenException,
Injectable,
UnauthorizedException,
} from '@nestjs/common';
import { Injectable, UnauthorizedException } from '@nestjs/common';

import {
BeforeCreateOneHook,
Expand All @@ -11,27 +7,6 @@ import {

import { CreateObjectInput } from 'src/engine/metadata-modules/object-metadata/dtos/create-object.input';

const coreObjectNames = [
'appToken',
'billingSubscription',
'billingSubscriptionItem',
'featureFlag',
'user',
'userWorkspace',
'workspace',
];

const reservedKeywords = [
...coreObjectNames,
'event',
'field',
'link',
'currency',
'fullName',
'address',
'links',
];

@Injectable()
export class BeforeCreateOneObject<T extends CreateObjectInput>
implements BeforeCreateOneHook<T, any>
Expand All @@ -46,14 +21,6 @@ export class BeforeCreateOneObject<T extends CreateObjectInput>
throw new UnauthorizedException();
}

if (
reservedKeywords.includes(instance.input.nameSingular) ||
reservedKeywords.includes(instance.input.namePlural)
) {
throw new ForbiddenException(
'You cannot create an object with this name.',
);
}
instance.input.workspaceId = workspaceId;

return instance;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ import { createWorkspaceMigrationsForCustomObjectRelations } from 'src/engine/me
import { createWorkspaceMigrationsForRemoteObjectRelations } from 'src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-remote-object-relations.util';
import { computeColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util';
import { DataSourceEntity } from 'src/engine/metadata-modules/data-source/data-source.entity';
import { validateObjectMetadataInput } from 'src/engine/metadata-modules/object-metadata/utils/validate-object-metadata-input.util';
import { validateObjectMetadataInputOrThrow } from 'src/engine/metadata-modules/object-metadata/utils/validate-object-metadata-input.util';
import { mapUdtNameToFieldType } from 'src/engine/metadata-modules/remote-server/remote-table/utils/udt-name-mapper.util';
import { WorkspaceCacheVersionService } from 'src/engine/metadata-modules/workspace-cache-version/workspace-cache-version.service';
import { UpdateOneObjectInput } from 'src/engine/metadata-modules/object-metadata/dtos/update-object.input';
Expand Down Expand Up @@ -253,7 +253,7 @@ export class ObjectMetadataService extends TypeOrmQueryService<ObjectMetadataEnt
objectMetadataInput.workspaceId,
);

validateObjectMetadataInput(objectMetadataInput);
validateObjectMetadataInputOrThrow(objectMetadataInput);

if (
objectMetadataInput.nameSingular.toLowerCase() ===
Expand Down Expand Up @@ -427,6 +427,8 @@ export class ObjectMetadataService extends TypeOrmQueryService<ObjectMetadataEnt
input: UpdateOneObjectInput,
workspaceId: string,
): Promise<ObjectMetadataEntity> {
validateObjectMetadataInputOrThrow(input.update);

const updatedObject = await super.updateOne(input.id, input.update);

await this.workspaceCacheVersionService.incrementVersion(workspaceId);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { UpdateObjectPayload } from 'src/engine/metadata-modules/object-metadata/dtos/update-object.input';
import { validateObjectMetadataInputOrThrow } from 'src/engine/metadata-modules/object-metadata/utils/validate-object-metadata-input.util';

const validObjectInput: UpdateObjectPayload = {
labelPlural: 'Car',
labelSingular: 'Cars',
nameSingular: 'car',
namePlural: 'cars',
};

const reservedKeyword = 'user';

describe('validateObjectName', () => {
it('should not throw if names are valid', () => {
expect(() =>
validateObjectMetadataInputOrThrow(validObjectInput),
).not.toThrow();
});

it('should throw is nameSingular has invalid characters', () => {
const invalidObjectInput = {
...validObjectInput,
nameSingular: 'μ',
};

expect(() =>
validateObjectMetadataInputOrThrow(invalidObjectInput),
).toThrow();
});

it('should throw is namePlural has invalid characters', () => {
const invalidObjectInput = {
...validObjectInput,
namePlural: 'μ',
};

expect(() =>
validateObjectMetadataInputOrThrow(invalidObjectInput),
).toThrow();
});

it('should throw if nameSingular is a reserved keyword', async () => {
const invalidObjectInput = {
...validObjectInput,
nameSingular: reservedKeyword,
};

expect(() =>
validateObjectMetadataInputOrThrow(invalidObjectInput),
).toThrow();
});

it('should throw if namePlural is a reserved keyword', async () => {
const invalidObjectInput = {
...validObjectInput,
namePlural: reservedKeyword,
};

expect(() =>
validateObjectMetadataInputOrThrow(invalidObjectInput),
).toThrow();
});
});
Original file line number Diff line number Diff line change
@@ -1,27 +1,70 @@
import { BadRequestException } from '@nestjs/common';
import { BadRequestException, ForbiddenException } from '@nestjs/common';

import { InvalidStringException } from 'src/engine/metadata-modules/errors/InvalidStringException';
import { CreateObjectInput } from 'src/engine/metadata-modules/object-metadata/dtos/create-object.input';
import { UpdateObjectPayload } from 'src/engine/metadata-modules/object-metadata/dtos/update-object.input';
import { validateMetadataName } from 'src/engine/metadata-modules/utils/validate-metadata-name.utils';

export const validateObjectMetadataInput = <
const coreObjectNames = [
'appToken',
'billingSubscription',
'billingSubscriptions',
'billingSubscriptionItem',
'billingSubscriptionItems',
'featureFlag',
'user',
'users',
'userWorkspace',
'userWorkspaces',
'workspace',
'workspaces',
];

const reservedKeywords = [
...coreObjectNames,
'event',
'events',
'field',
'fields',
'link',
'links',
'currency',
'currencies',
'fullName',
'fullNames',
'address',
'addresses',
];

export const validateObjectMetadataInputOrThrow = <
T extends UpdateObjectPayload | CreateObjectInput,
>(
objectMetadataInput: T,
): void => {
try {
if (objectMetadataInput.nameSingular) {
validateMetadataName(objectMetadataInput.nameSingular);
validateNameCharactersOrThrow(objectMetadataInput.nameSingular);
validateNameCharactersOrThrow(objectMetadataInput.namePlural);

validateNameIsNotReservedKeywordOrThrow(objectMetadataInput.nameSingular);
validateNameIsNotReservedKeywordOrThrow(objectMetadataInput.namePlural);
};

const validateNameIsNotReservedKeywordOrThrow = (name?: string) => {
if (name) {
if (reservedKeywords.includes(name)) {
throw new ForbiddenException(`The name "${name}" is not available`);
}
}
};

if (objectMetadataInput.namePlural) {
validateMetadataName(objectMetadataInput.namePlural);
const validateNameCharactersOrThrow = (name?: string) => {
try {
if (name) {
validateMetadataName(name);
}
} catch (error) {
if (error instanceof InvalidStringException) {
throw new BadRequestException(
`Characters used in name "${objectMetadataInput.nameSingular}" or "${objectMetadataInput.namePlural}" are not supported`,
`Characters used in name "${name}" are not supported`,
);
} else {
throw error;
Expand Down

0 comments on commit 1694e0c

Please sign in to comment.