From b5583f8ee087a0fb06fbdcc8518770cadde9a45b Mon Sep 17 00:00:00 2001 From: Marco Crivellaro Date: Fri, 15 Jul 2022 14:38:26 +0100 Subject: [PATCH] feat: simplified the scaffolding audit log Signed-off-by: Marco Crivellaro --- app-config.yaml | 6 - .../src/service/router.test.ts | 338 +++--------------- .../scaffolder-backend/src/service/router.ts | 30 +- 3 files changed, 49 insertions(+), 325 deletions(-) diff --git a/app-config.yaml b/app-config.yaml index 706137f991..a56a3df9d1 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -286,12 +286,6 @@ scaffolder: # email: scaffolder@backstage.io # Use to customize the default commit message when new components are created # defaultCommitMessage: 'Initial commit' - # Use to customize the audit log emitted when a template task is created - auditlog: - enabled: true # emit an audit log when a new scaffolder task is created, by default it logs the name of the authenticated user (if available) - # Optional authenticated user annotations to log (i.e. userEntity.metadata.annotations['name/of-annotation']) - # identityAnnotations: - # - name/of-annotation auth: ### Add auth.keyStore.provider to more granularly control how to store JWK data when running diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index b3bef8bb9a..f545e57f85 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -75,6 +75,7 @@ const mockUrlReader = UrlReaders.default({ describe('createRouter', () => { let app: express.Express; + let loggerSpy: jest.SpyInstance; let taskBroker: TaskBroker; const catalogClient = { getEntityByRef: jest.fn() } as unknown as CatalogApi; @@ -134,9 +135,10 @@ describe('createRouter', () => { jest.spyOn(taskBroker, 'get'); jest.spyOn(taskBroker, 'list'); jest.spyOn(taskBroker, 'event$'); + loggerSpy = jest.spyOn(logger, 'info'); const router = await createRouter({ - logger: getVoidLogger(), + logger: logger, config: new ConfigReader({}), database: createDatabase(), catalogClient, @@ -289,6 +291,48 @@ describe('createRouter', () => { }), ); }); + + it('should emit auditlog containing without user identifier when no backstage auth is passed', async () => { + await request(app) + .post('/v2/tasks') + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + + expect(loggerSpy).toHaveBeenCalledTimes(1); + expect(loggerSpy).toHaveBeenCalledWith( + 'Scaffolding task for template:default/create-react-app-template', + ); + }); + + it('should emit auditlog containing user identifier when backstage auth is passed', async () => { + const mockToken = + 'blob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvZ3Vlc3QiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; + + await request(app) + .post('/v2/tasks') + .set('Authorization', `Bearer ${mockToken}`) + .send({ + templateRef: stringifyEntityRef({ + kind: 'template', + name: 'create-react-app-template', + }), + values: { + required: 'required-value', + }, + }); + + expect(loggerSpy).toHaveBeenCalledTimes(1); + expect(loggerSpy).toHaveBeenCalledWith( + 'Scaffolding task for template:default/create-react-app-template created by user:default/guest', + ); + }); }); describe('GET /v2/tasks', () => { @@ -596,295 +640,3 @@ data: {"id":1,"taskId":"a-random-id","type":"completion","createdAt":"","body":{ }); }); }); - -describe('createRouter logging', () => { - let app: express.Express; - let taskBroker: TaskBroker; - const catalogClient = { getEntityByRef: jest.fn() } as unknown as CatalogApi; - - const mockTemplate: TemplateEntityV1beta3 = { - apiVersion: 'scaffolder.backstage.io/v1beta3', - kind: 'Template', - metadata: { - description: 'Create a new CRA website project', - name: 'create-react-app-template', - tags: ['experimental', 'react', 'cra'], - title: 'Create React App Template', - annotations: { - 'backstage.io/managed-by-location': 'url:https://dev.azure.com', - }, - }, - spec: { - owner: 'web@example.com', - type: 'website', - steps: [], - parameters: { - type: 'object', - required: ['required'], - properties: { - required: { - type: 'string', - description: 'Required parameter', - }, - }, - }, - }, - }; - - const mockUser: UserEntity = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'User', - metadata: { - name: 'guest', - annotations: { - 'google.com/email': 'bobby@tables.com', - 'example.com/uuid': 'bc48e920-f1c9-4094-bb5c-99d120e87ab8', - }, - }, - spec: { - profile: { - displayName: 'Robert Tables of the North', - }, - }, - }; - - beforeEach(async () => { - const logger = getVoidLogger(); - - const databaseTaskStore = await DatabaseTaskStore.create({ - database: await createDatabase().getClient(), - }); - taskBroker = new StorageTaskBroker(databaseTaskStore, logger); - - jest.spyOn(taskBroker, 'dispatch'); - jest.spyOn(taskBroker, 'get'); - jest.spyOn(taskBroker, 'list'); - jest.spyOn(taskBroker, 'event$'); - - jest - .spyOn(catalogClient, 'getEntityByRef') - .mockImplementation(async ref => { - const { kind } = parseEntityRef(ref); - - if (kind === 'template') { - return mockTemplate; - } - - if (kind === 'user') { - return mockUser; - } - throw new Error(`no mock found for kind: ${kind}`); - }); - }); - - afterEach(() => { - jest.resetAllMocks(); - }); - - describe('POST /v2/tasks', () => { - it('does not log auditlog by default', async () => { - const config = new ConfigReader({}); - - const logger = getVoidLogger(); - const loggerSpy = jest.spyOn(logger, 'info'); - - const router = await createRouter({ - logger: logger, - config: config, - database: createDatabase(), - catalogClient, - reader: mockUrlReader, - taskBroker, - }); - app = express().use(router); - - const response = await request(app) - .post('/v2/tasks') - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - required: 'required-value', - }, - }); - - expect(loggerSpy).toHaveBeenCalledTimes(0); - expect(response.status).toEqual(201); - }); - - it('does not log auditlog when not enabled', async () => { - const config = new ConfigReader({ - scaffolder: { - auditlog: { - enabled: false, - }, - }, - }); - - const logger = getVoidLogger(); - const loggerSpy = jest.spyOn(logger, 'info'); - - const router = await createRouter({ - logger: logger, - config: config, - database: createDatabase(), - catalogClient, - reader: mockUrlReader, - taskBroker, - }); - app = express().use(router); - - const response = await request(app) - .post('/v2/tasks') - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - required: 'required-value', - }, - }); - - expect(loggerSpy).toHaveBeenCalledTimes(0); - expect(response.status).toEqual(201); - }); - - it('emits auditlog when enabled without identity', async () => { - const config = new ConfigReader({ - scaffolder: { - auditlog: { - enabled: true, - }, - }, - }); - - const logger = getVoidLogger(); - const loggerSpy = jest.spyOn(logger, 'info'); - - const router = await createRouter({ - logger: logger, - config: config, - database: createDatabase(), - catalogClient, - reader: mockUrlReader, - taskBroker, - }); - app = express().use(router); - - const response = await request(app) - .post('/v2/tasks') - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - required: 'required-value', - }, - }); - - expect(loggerSpy).toHaveBeenCalledTimes(1); - expect(loggerSpy).toHaveBeenCalledWith( - "Scaffolding task for 'template:default/create-react-app-template'", - ); - - expect(response.status).toEqual(201); - }); - - it('emits auditlog when enabled with identity', async () => { - const config = new ConfigReader({ - scaffolder: { - auditlog: { - enabled: true, - }, - }, - }); - - const logger = getVoidLogger(); - const loggerSpy = jest.spyOn(logger, 'info'); - - const router = await createRouter({ - logger: logger, - config: config, - database: createDatabase(), - catalogClient, - reader: mockUrlReader, - taskBroker, - }); - app = express().use(router); - - const mockToken = - 'blob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvZ3Vlc3QiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; - - const response = await request(app) - .post('/v2/tasks') - .set('Authorization', `Bearer ${mockToken}`) - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - required: 'required-value', - }, - }); - - expect(loggerSpy).toHaveBeenCalledTimes(1); - expect(loggerSpy).toHaveBeenCalledWith( - "Scaffolding task for 'template:default/create-react-app-template' created by guest", - ); - - expect(response.status).toEqual(201); - }); - - it('emits auditlog when enabled with identity and annotations', async () => { - const config = new ConfigReader({ - scaffolder: { - auditlog: { - enabled: true, - identityAnnotations: ['google.com/email', 'example.com/uuid'], - }, - }, - }); - - const logger = getVoidLogger(); - const loggerSpy = jest.spyOn(logger, 'info'); - - const router = await createRouter({ - logger: logger, - config: config, - database: createDatabase(), - catalogClient, - reader: mockUrlReader, - taskBroker, - }); - app = express().use(router); - - const mockToken = - 'blob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvZ3Vlc3QiLCJuYW1lIjoiSm9obiBEb2UifQ.blob'; - - const response = await request(app) - .post('/v2/tasks') - .set('Authorization', `Bearer ${mockToken}`) - .send({ - templateRef: stringifyEntityRef({ - kind: 'template', - name: 'create-react-app-template', - }), - values: { - required: 'required-value', - }, - }); - - expect(loggerSpy).toHaveBeenCalledTimes(1); - expect(loggerSpy).toHaveBeenCalledWith( - 'Scaffolding task for \'template:default/create-react-app-template\' created by guest [{"google.com/email":"bobby@tables.com"},{"example.com/uuid":"bc48e920-f1c9-4094-bb5c-99d120e87ab8"}]', - ); - - expect(response.status).toEqual(201); - }); - }); -}); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 7d2c1848ab..b682ebdf25 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -193,33 +193,11 @@ export async function createRouter( ? await catalogClient.getEntityByRef(userEntityRef, { token }) : undefined; - const auditlogEnabled = config.getOptionalBoolean( - 'scaffolder.auditlog.enabled', - ); - - if (auditlogEnabled) { - const identityAnnotations = config.getOptionalStringArray( - 'scaffolder.auditlog.identityAnnotations', - ); - - const logMe: { [key: string]: string }[] = []; - - identityAnnotations?.forEach((prop: string) => - logMe.push({ - [prop]: userEntity?.metadata.annotations?.[prop] as string, - }), - ); - - let userLog = ''; - if (userEntity) { - userLog = ` created by ${userEntity.metadata.name}`; - if (logMe.length > 0) { - userLog += ` ${JSON.stringify(logMe)}`; - } - } - - logger.info(`Scaffolding task for '${templateRef}'${userLog}`); + let auditLog = `Scaffolding task for ${templateRef}`; + if (userEntityRef) { + auditLog += ` created by ${userEntityRef}`; } + logger.info(auditLog); const values = req.body.values;