From ef9117f50a7f1c9f5151aa642a0d097c3bd3dfa6 Mon Sep 17 00:00:00 2001 From: Iain Billett Date: Tue, 15 Sep 2020 18:06:44 +0100 Subject: [PATCH 1/3] Validate template values on calls to the scaffolder API Calls to the scaffolder API should use the schema from the template entity to validate input values. --- plugins/scaffolder-backend/package.json | 1 + .../src/service/router.test.ts | 94 +++++++++++++++++++ .../scaffolder-backend/src/service/router.ts | 7 ++ yarn.lock | 5 + 4 files changed, 107 insertions(+) create mode 100644 plugins/scaffolder-backend/src/service/router.test.ts diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 2283dd5d05..b6f927bc61 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -36,6 +36,7 @@ "git-url-parse": "^11.1.2", "globby": "^11.0.0", "helmet": "^4.0.0", + "jsonschema": "^1.2.6", "morgan": "^1.10.0", "nodegit": "0.26.5", "uuid": "^8.2.0", diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts new file mode 100644 index 0000000000..0349530e45 --- /dev/null +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -0,0 +1,94 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { getVoidLogger } from '@backstage/backend-common'; +import express from 'express'; +import request from 'supertest'; +import { createRouter } from './router'; +import { Templaters, Preparers, PublisherBase } from '../scaffolder'; +import Docker from 'dockerode'; + +jest.mock('dockerode'); + +describe('createRouter', () => { + let app: express.Express; + const publisher: jest.Mocked = { publish: jest.fn() }; + + beforeAll(async () => { + const router = await createRouter({ + logger: getVoidLogger(), + preparers: new Preparers(), + templaters: new Templaters(), + publisher: publisher, + dockerClient: new Docker(), + }); + app = express().use(router); + }); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + describe('POST /v1/jobs', () => { + const template = { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Template', + metadata: { + description: 'Create a new CRA website project', + name: 'create-react-app-template', + tags: ['experimental', 'react', 'cra'], + title: 'Create React App Template', + }, + spec: { + owner: 'web@example.com', + path: '.', + schema: { + type: 'string', + properties: { + component_id: { + description: 'Unique name of the component', + title: 'Name', + type: 'string', + }, + description: { + description: 'Description of the component', + title: 'Description', + type: 'string', + }, + use_typescript: { + default: true, + description: 'Include typescript', + title: 'Use Typescript', + type: 'boolean', + }, + }, + required: ['component_id', 'use_typescript'], + }, + templater: 'cra', + type: 'website', + }, + }; + + it('rejects template values which do not match the template schema definition', async () => { + const response = await request(app).post('/v1/jobs').send({ + template, + values: {}, + }); + + expect(response.status).toEqual(400); + }); + }); +}); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 1b96158fa4..8a778f31b4 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -28,6 +28,7 @@ import { TemplaterBuilder, PublisherBase, } from '../scaffolder'; +import {validate, ValidatorResult} from 'jsonschema'; export interface RouterOptions { preparers: PreparerBuilder; @@ -84,6 +85,12 @@ export async function createRouter( const values: RequiredTemplateValues & Record = req.body.values; + const validationResult: ValidatorResult = validate(values, template.spec.schema); + if (!validationResult.valid) { + res.status(400).json({ errors: validationResult.errors }); + return; + } + const job = jobProcessor.create({ entity: template, values, diff --git a/yarn.lock b/yarn.lock index 38f5d6c799..3da4f288c5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -13288,6 +13288,11 @@ jsonpointer@^4.0.1: resolved "https://registry.npmjs.org/jsonpointer/-/jsonpointer-4.0.1.tgz#4fd92cb34e0e9db3c89c8622ecf51f9b978c6cb9" integrity sha1-T9kss04OnbPInIYi7PUfm5eMbLk= +jsonschema@^1.2.6: + version "1.2.6" + resolved "https://registry.npmjs.org/jsonschema/-/jsonschema-1.2.6.tgz#52b0a8e9dc06bbae7295249d03e4b9faee8a0c0b" + integrity sha512-SqhURKZG07JyKKeo/ir24QnS4/BV7a6gQy93bUSe4lUdNp0QNpIz2c9elWJQ9dpc5cQYY6cvCzgRwy0MQCLyqA== + jspdf-autotable@3.5.3: version "3.5.3" resolved "https://registry.npmjs.org/jspdf-autotable/-/jspdf-autotable-3.5.3.tgz#2f73adb07f340e7dbf22950e3e6c8bf853991479" From cf96d90e6285aabc54089840bbb1d983dbabbb9c Mon Sep 17 00:00:00 2001 From: Iain Billett Date: Tue, 15 Sep 2020 20:14:47 +0100 Subject: [PATCH 2/3] Fix formatting --- plugins/scaffolder-backend/src/service/router.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 8a778f31b4..5c67612176 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -28,7 +28,7 @@ import { TemplaterBuilder, PublisherBase, } from '../scaffolder'; -import {validate, ValidatorResult} from 'jsonschema'; +import { validate, ValidatorResult } from 'jsonschema'; export interface RouterOptions { preparers: PreparerBuilder; @@ -85,7 +85,10 @@ export async function createRouter( const values: RequiredTemplateValues & Record = req.body.values; - const validationResult: ValidatorResult = validate(values, template.spec.schema); + const validationResult: ValidatorResult = validate( + values, + template.spec.schema, + ); if (!validationResult.valid) { res.status(400).json({ errors: validationResult.errors }); return; From f0143360b7700f5da69ef07a71df66585834a824 Mon Sep 17 00:00:00 2001 From: Iain Billett Date: Wed, 16 Sep 2020 09:02:42 +0100 Subject: [PATCH 3/3] Fix test schema --- plugins/scaffolder-backend/src/service/router.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index 0349530e45..8fd9bca02a 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -56,7 +56,6 @@ describe('createRouter', () => { owner: 'web@example.com', path: '.', schema: { - type: 'string', properties: { component_id: { description: 'Unique name of the component',