From 6663e57fa7116ff0803fa5f524017facc3af52c3 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Fri, 3 Feb 2023 10:44:10 +0000 Subject: [PATCH] Added tests to check authorized template on get and put endpoint Signed-off-by: Harry Hogg Co-authored-by: Vincenzo Scamporlino --- .../src/service/router.test.ts | 188 +++++++++++++++--- .../scaffolder-backend/src/service/router.ts | 1 + 2 files changed, 157 insertions(+), 32 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index a7d1862f7a..2bf9203056 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -89,8 +89,12 @@ describe('createRouter', () => { let loggerSpy: jest.SpyInstance; let taskBroker: TaskBroker; const catalogClient = { getEntityByRef: jest.fn() } as unknown as CatalogApi; + const permissionApi = { + authorize: jest.fn(), + authorizeConditional: jest.fn(), + } as unknown as PermissionEvaluator; - const mockTemplate: TemplateEntityV1beta3 = { + const getMockTemplate = (): TemplateEntityV1beta3 => ({ apiVersion: 'scaffolder.backstage.io/v1beta3', kind: 'Template', metadata: { @@ -117,7 +121,7 @@ describe('createRouter', () => { }, }, }, - }; + }); const mockUser: UserEntity = { apiVersion: 'backstage.io/v1alpha1', @@ -143,18 +147,6 @@ describe('createRouter', () => { }); taskBroker = new StorageTaskBroker(databaseTaskStore, logger); - const permissionApi: PermissionEvaluator = { - authorize: jest.fn(), - authorizeConditional: jest.fn().mockResolvedValue([ - { - result: AuthorizeResult.ALLOW, - }, - { - result: AuthorizeResult.ALLOW, - }, - ]), - }; - jest.spyOn(taskBroker, 'dispatch'); jest.spyOn(taskBroker, 'get'); jest.spyOn(taskBroker, 'list'); @@ -177,15 +169,27 @@ describe('createRouter', () => { .mockImplementation(async ref => { const { kind } = parseEntityRef(ref); - if (kind === 'template') { - return mockTemplate; + if (kind.toLocaleLowerCase() === 'template') { + return getMockTemplate(); } - if (kind === 'user') { + if (kind.toLocaleLowerCase() === 'user') { return mockUser; } + throw new Error(`no mock found for kind: ${kind}`); }); + + jest + .spyOn(permissionApi, 'authorizeConditional') + .mockImplementation(async () => [ + { + result: AuthorizeResult.ALLOW, + }, + { + result: AuthorizeResult.ALLOW, + }, + ]); }); afterEach(() => { @@ -246,6 +250,7 @@ describe('createRouter', () => { taskBroker.dispatch as jest.Mocked['dispatch']; const mockToken = 'blob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvZ3Vlc3QiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; + const mockTemplate = getMockTemplate(); await request(app) .post('/v2/tasks') @@ -301,6 +306,7 @@ describe('createRouter', () => { const broker = taskBroker.dispatch as jest.Mocked['dispatch']; const mockToken = 'blob.eyJzdWIiOiIiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; + const mockTemplate = getMockTemplate(); await request(app) .post('/v2/tasks') @@ -730,6 +736,7 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }); }); }); + describe('providing an identity api', () => { beforeEach(async () => { const logger = getVoidLogger(); @@ -761,18 +768,6 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }, ); - const permissionApi: PermissionEvaluator = { - authorize: jest.fn(), - authorizeConditional: jest.fn().mockResolvedValue([ - { - result: AuthorizeResult.ALLOW, - }, - { - result: AuthorizeResult.ALLOW, - }, - ]), - }; - const router = await createRouter({ logger: logger, config: new ConfigReader({}), @@ -790,15 +785,26 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ .mockImplementation(async ref => { const { kind } = parseEntityRef(ref); - if (kind === 'template') { - return mockTemplate; + if (kind.toLocaleLowerCase() === 'template') { + return getMockTemplate(); } - if (kind === 'user') { + if (kind.toLocaleLowerCase() === 'user') { return mockUser; } throw new Error(`no mock found for kind: ${kind}`); }); + + jest + .spyOn(permissionApi, 'authorizeConditional') + .mockImplementation(async () => [ + { + result: AuthorizeResult.ALLOW, + }, + { + result: AuthorizeResult.ALLOW, + }, + ]); }); afterEach(() => { @@ -814,6 +820,62 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }); }); + describe('GET /v2/templates/:namespace/:kind/:name/parameter-schema', () => { + it('returns the parameter schema', async () => { + const response = await request(app) + .get( + '/v2/templates/default/Template/create-react-app-template/parameter-schema', + ) + .send(); + + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + title: 'Create React App Template', + description: 'Create a new CRA website project', + steps: [ + { + title: 'Please enter the following information', + schema: { + required: ['required'], + type: 'object', + properties: { + required: { + description: 'Required parameter', + type: 'string', + }, + }, + }, + }, + ], + }); + }); + + it('filters parameters that the user is not authorized to see', async () => { + jest + .spyOn(permissionApi, 'authorizeConditional') + .mockImplementationOnce(async () => [ + { + result: AuthorizeResult.DENY, + }, + { + result: AuthorizeResult.ALLOW, + }, + ]); + const response = await request(app) + .get( + '/v2/templates/default/Template/create-react-app-template/parameter-schema', + ) + .send(); + + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + title: 'Create React App Template', + description: 'Create a new CRA website project', + steps: [], + }); + }); + }); + describe('POST /v2/tasks', () => { it('rejects template values which do not match the template schema definition', async () => { const response = await request(app) @@ -831,6 +893,67 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ expect(response.status).toEqual(400); }); + it('filters steps that the user is not authorized to see', async () => { + jest + .spyOn(permissionApi, 'authorizeConditional') + .mockImplementation(async () => [ + { + result: AuthorizeResult.ALLOW, + }, + { + result: AuthorizeResult.DENY, + }, + ]); + + const broker = + taskBroker.dispatch as jest.Mocked['dispatch']; + const mockTemplate = getMockTemplate(); + + await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + expect(broker).toHaveBeenCalledWith( + expect.objectContaining({ + createdBy: 'user:default/guest', + secrets: { + backstageToken: 'token', + }, + + spec: { + apiVersion: mockTemplate.apiVersion, + steps: [], + output: mockTemplate.spec.output ?? {}, + parameters: { + required: 'required-value', + }, + user: { + entity: mockUser, + ref: 'user:default/guest', + }, + templateInfo: { + entityRef: stringifyEntityRef({ + kind: 'Template', + namespace: 'Default', + name: mockTemplate.metadata?.name, + }), + baseUrl: 'https://dev.azure.com', + entity: { + metadata: mockTemplate.metadata, + }, + }, + }, + }), + ); + }); + it('return the template id', async () => { const broker = taskBroker.dispatch as jest.Mocked['dispatch']; @@ -857,6 +980,7 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ it('should call the broker with a correct spec', async () => { const broker = taskBroker.dispatch as jest.Mocked['dispatch']; + const mockTemplate = getMockTemplate(); await request(app) .post('/v2/tasks') diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index fc410ed634..6bd0b93cc9 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -325,6 +325,7 @@ export async function createRouter( for (const parameters of [template.spec.parameters ?? []].flat()) { const result = validate(values, parameters); + if (!result.valid) { res.status(400).json({ errors: result.errors }); return;