Skip to content

Commit

Permalink
Merge branch 'master' into fix/redirect-vulnerability
Browse files Browse the repository at this point in the history
  • Loading branch information
wRLSS authored Oct 4, 2024
2 parents 7bb93fb + 5e0b813 commit 794c77a
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 6 deletions.
2 changes: 2 additions & 0 deletions registry/server/templates/interfaces/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,5 @@ export type UpdateTemplatePayload = Omit<Template, 'name' | 'localizedVersions'>

export type CreateTemplatePayload = Omit<Template, 'localizedVersions'> &
Partial<Pick<TemplateWithLocalizedVersions, 'localizedVersions'>>;

export type PartialUpdateTemplatePayload = Pick<Template, 'content'>;
2 changes: 2 additions & 0 deletions registry/server/templates/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import createTemplate from './createTemplate';
import deleteTemplate from './deleteTemplate';
import upsertTemplateLocalizedVersion from './upsertTemplateLocalizedVersion';
import deleteTemplateLocalizedVersion from './deleteTemplateLocalizedVersion';
import partialUpdateTemplate from './partialUpdateTemplate';

export default (authMw: RequestHandler[]) => {
const templatesRouter = express.Router();
Expand All @@ -17,6 +18,7 @@ export default (authMw: RequestHandler[]) => {
templatesRouter.get('/:name/rendered', ...getRenderedTemplate);
templatesRouter.get('/:name', ...getTemplate);
templatesRouter.put('/:name', authMw, ...updateTemplate);
templatesRouter.patch('/:name', authMw, ...partialUpdateTemplate);
templatesRouter.delete('/:name', authMw, ...deleteTemplate);
templatesRouter.put('/:name/localized/:locale', authMw, ...upsertTemplateLocalizedVersion);
templatesRouter.delete('/:name/localized/:locale', authMw, ...deleteTemplateLocalizedVersion);
Expand Down
48 changes: 48 additions & 0 deletions registry/server/templates/routes/partialUpdateTemplate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { Request, Response } from 'express';
import Joi from 'joi';

import validateRequestFactory from '../../common/services/validateRequest';
import { validateLocalesMiddleware } from '../../middleware/validatelocales';
import { exhaustiveCheck } from '../../util/exhaustiveCheck';
import { templatesRepository } from '../services/templatesRepository';
import { commonTemplate, templateNameSchema } from './validation';

type UpdateTemplateRequestParams = {
name: string;
};

const validateRequestBeforeUpdateTemplate = validateRequestFactory([
{
schema: Joi.object({
name: templateNameSchema.required(),
}),
selector: 'params',
},
{
schema: Joi.object({
content: commonTemplate.content.required(),
}),
selector: 'body',
},
]);

const partialUpdateTemplate = async (req: Request<UpdateTemplateRequestParams>, res: Response): Promise<void> => {
const result = await templatesRepository.partialUpdateTemplate(req.params.name, req.body, req.user);

switch (result.type) {
case 'notFound': {
res.status(404).send('Not found');
return;
}
case 'ok': {
res.status(200).send(result.template);
return;
}
default: {
/* istanbul ignore next */
exhaustiveCheck(result);
}
}
};

export default [validateRequestBeforeUpdateTemplate, partialUpdateTemplate];
2 changes: 1 addition & 1 deletion registry/server/templates/routes/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import renderTemplate from '../services/renderTemplate';

export const templateNameSchema = Joi.string().min(1).max(50);

const commonTemplate = {
export const commonTemplate = {
content: Joi.string()
.min(1)
.external(async (value) => {
Expand Down
53 changes: 50 additions & 3 deletions registry/server/templates/services/templatesRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import Template, {
CreateTemplatePayload,
LocalizedTemplateRow,
LocalizedVersion,
PartialUpdateTemplatePayload,
TemplateWithLocalizedVersions,
UpdateTemplatePayload,
} from '../interfaces';
Expand Down Expand Up @@ -60,6 +61,17 @@ type DeleteTemplateLocalizedVersionResult =
| DeleteTemplateLocalizedVersionResultOk
| DeleteTemplateLocalizedVersionResultNotFound;

type PartialUpdateTemplateResultOk = {
type: 'ok';
template: VersionedRecord<Template>;
};

type PartialUpdateTemplateResultNotFound = {
type: 'notFound';
};

type PartialUpdateTemplateResult = PartialUpdateTemplateResultOk | PartialUpdateTemplateResultNotFound;

export class TemplatesRepository {
constructor(private db: VersionedKnex) {}

Expand Down Expand Up @@ -142,7 +154,7 @@ export class TemplatesRepository {
}
});

const savedTemplate = await this.mustReadTemplateWithAllVersions(template.name);
const savedTemplate = await this.readTemplateWithAllVersionsOrFail(template.name);

return savedTemplate;
}
Expand Down Expand Up @@ -177,7 +189,29 @@ export class TemplatesRepository {
},
);

const updatedTemplate = await this.mustReadTemplateWithAllVersions(templateName);
const updatedTemplate = await this.readTemplateWithAllVersionsOrFail(templateName);

return { type: 'ok', template: updatedTemplate };
}

async partialUpdateTemplate(
templateName: string,
payload: PartialUpdateTemplatePayload,
user: User | undefined,
): Promise<PartialUpdateTemplateResult> {
const templateExists = await this.db(Tables.Templates).where({ name: templateName }).select(1).first();
if (!templateExists) {
return { type: 'notFound' };
}

await db.versioning(user, { type: EntityTypes.templates, id: templateName }, async (trx) => {
await db(Tables.Templates)
.where({ name: templateName })
.update({ content: payload.content })
.transacting(trx);
});

const updatedTemplate = await this.readTemplateOrFail(templateName);

return { type: 'ok', template: updatedTemplate };
}
Expand Down Expand Up @@ -227,7 +261,20 @@ export class TemplatesRepository {
return { type: 'ok' };
}

private async mustReadTemplateWithAllVersions(
private async readTemplateOrFail(templateName: string): Promise<VersionedRecord<Template>> {
const [maybeTemplate] = await db
.selectVersionedRowsFrom<Template>(Tables.Templates, 'name', EntityTypes.templates, [
`${Tables.Templates}.*`,
])
.where('name', templateName);

if (!maybeTemplate) {
throw new errors.TemplateNotFoundError({ data: { templateName } });
}
return maybeTemplate;
}

private async readTemplateWithAllVersionsOrFail(
templateName: string,
): Promise<VersionedRecord<TemplateWithLocalizedVersions>> {
const maybeTemplate = await this.readTemplateWithAllVersions(templateName);
Expand Down
55 changes: 53 additions & 2 deletions registry/tests/templates.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import _ from 'lodash';
import nock from 'nock';
import supertest, { type Agent } from 'supertest';
import app from '../server/app';
import { expect, getServerAddress } from './common';
import { SettingKeys } from '../server/settings/interfaces';
import { withSetting } from './utils/withSetting';
import { expect, getServerAddress } from './common';
import { makeFilterQuery } from './utils/makeFilterQuery';
import { withSetting } from './utils/withSetting';

const example = {
url: '/api/v1/template/',
Expand Down Expand Up @@ -576,6 +576,57 @@ describe(`Tests ${example.url}`, () => {
});
});

describe('Partial Update', () => {
it("should not partially update any record if record doesn't exist", async () => {
const nonExisting = { name: 123 };
await req
.patch(example.url + nonExisting.name)
.send({ content: example.correct.content })
.expect(404, 'Not found');
});

it('should successfully partially update record', async () => {
await req.post(example.url).send(example.correct).expect(200);

const response = await req
.patch(example.url + example.correct.name)
.send({ content: example.updated.content })
.expect(200);
expect(response.body).deep.equal({
...example.updated,
versionId: response.body.versionId,
});
});

it('should not delete localized versions of the template on partial update', async () => {
await withSetting(
SettingKeys.I18nSupportedLocales,
Object.keys(example.correctLocalized.localizedVersions),
async () => {
await req.post(example.url).send(example.correctLocalized).expect(200);
},
);

await req
.patch(example.url + example.correctLocalized.name)
.send({ content: example.updated.content })
.expect(200);

const template = await req.get(example.url + example.correctLocalized.name).expect(200);
expect(template.body.localizedVersions).deep.equal(example.correctLocalized.localizedVersions);

await req.delete(example.url + example.correctLocalized.name).expect(204);
});

it('should not allow to pass localizedVersions to the PATCH request', async () => {
await req.post(example.url).send(example.correct).expect(200);
await req
.patch(example.url + example.correct.name)
.send(_.omit(example.correctLocalized, 'name'))
.expect(422, '"localizedVersions" is not allowed');
});
});

describe('Delete', () => {
it("should not delete any record if record doesn't exist", async () => {
const incorrect = { name: 123 };
Expand Down

0 comments on commit 794c77a

Please sign in to comment.