diff --git a/.changeset/eight-plums-hide.md b/.changeset/eight-plums-hide.md new file mode 100644 index 0000000000..d1f8bcecfe --- /dev/null +++ b/.changeset/eight-plums-hide.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Forward user token to scaffolder task for subsequent api requests diff --git a/plugins/scaffolder-backend/migrations/20210409225200_secrets.js b/plugins/scaffolder-backend/migrations/20210409225200_secrets.js new file mode 100644 index 0000000000..e96f406782 --- /dev/null +++ b/plugins/scaffolder-backend/migrations/20210409225200_secrets.js @@ -0,0 +1,38 @@ +/* + * 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. + */ + +// @ts-check + +/** + * @param {import('knex').Knex} knex + */ +exports.up = async function up(knex) { + await knex.schema.alterTable('tasks', table => { + table + .text('secrets') + .nullable() + .comment('JSON encoded secrets to authenticate tasks with'); + }); +}; + +/** + * @param {import('knex').Knex} knex + */ +exports.down = async function down(knex) { + await knex.schema.alterTable('tasks', table => { + table.dropColumn('secrets'); + }); +}; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.test.ts index 070ad15ab7..16a918e207 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.test.ts @@ -84,10 +84,13 @@ describe('catalog:register', () => { catalogInfoUrl: 'http://foo/var', }, }); - expect(addLocation).toBeCalledWith({ - type: 'url', - target: 'http://foo/var', - }); + expect(addLocation).toBeCalledWith( + { + type: 'url', + target: 'http://foo/var', + }, + {}, + ); expect(mockContext.output).toBeCalledWith( 'entityRef', diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts index 8ee98f5d20..e9710f7da4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts @@ -95,10 +95,13 @@ export function createCatalogRegisterAction(options: { ctx.logger.info(`Registering ${catalogInfoUrl} in the catalog`); - const result = await catalogClient.addLocation({ - type: 'url', - target: catalogInfoUrl, - }); + const result = await catalogClient.addLocation( + { + type: 'url', + target: catalogInfoUrl, + }, + ctx.token ? { token: ctx.token } : {}, + ); if (result.entities.length >= 1) { const { kind, name, namespace } = getEntityName(result.entities[0]); ctx.output('entityRef', `${kind}:${namespace}/${name}`); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/types.ts b/plugins/scaffolder-backend/src/scaffolder/actions/types.ts index 955cb72de9..fbe18b848e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/types.ts @@ -32,6 +32,10 @@ export type ActionContext = { logger: Logger; logStream: Writable; + /** + * User token forwarded from initial request, for use in subsequent api requests + */ + token?: string | undefined; workspacePath: string; input: Input; output(name: string, value: JsonValue): void; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index 91264bd3e2..1f1228baf8 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -24,6 +24,7 @@ import { DbTaskRow, Status, TaskEventType, + TaskSecrets, TaskSpec, TaskStore, TaskStoreEmitOptions, @@ -42,6 +43,7 @@ export type RawDbTaskRow = { status: Status; last_heartbeat_at?: string; created_at: string; + secrets?: string; }; export type RawDbTaskEventRow = { @@ -71,23 +73,29 @@ export class DatabaseTaskStore implements TaskStore { } try { const spec = JSON.parse(result.spec); + const secrets = result.secrets ? JSON.parse(result.secrets) : undefined; return { id: result.id, spec, status: result.status, lastHeartbeatAt: result.last_heartbeat_at, createdAt: result.created_at, + secrets, }; } catch (error) { throw new Error(`Failed to parse spec of task '${taskId}', ${error}`); } } - async createTask(spec: TaskSpec): Promise<{ taskId: string }> { + async createTask( + spec: TaskSpec, + secrets?: TaskSecrets, + ): Promise<{ taskId: string }> { const taskId = uuid(); await this.db('tasks').insert({ id: taskId, spec: JSON.stringify(spec), + secrets: secrets ? JSON.stringify(secrets) : undefined, status: 'open', }); return { taskId }; @@ -119,12 +127,14 @@ export class DatabaseTaskStore implements TaskStore { try { const spec = JSON.parse(task.spec); + const secrets = task.secrets ? JSON.parse(task.secrets) : undefined; return { id: task.id, spec, status: 'processing', lastHeartbeatAt: task.last_heartbeat_at, createdAt: task.created_at, + secrets, }; } catch (error) { throw new Error(`Failed to parse spec of task '${task.id}', ${error}`); @@ -209,6 +219,7 @@ export class DatabaseTaskStore implements TaskStore { }) .update({ status, + secrets: null as any, }); if (updateCount !== 1) { throw new ConflictError( diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts index fd2de30332..686dfc052f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts @@ -21,7 +21,7 @@ import { import { ConfigReader } from '@backstage/config'; import { DatabaseTaskStore } from './DatabaseTaskStore'; import { StorageTaskBroker, TaskAgent } from './StorageTaskBroker'; -import { TaskSpec, DbTaskEventRow } from './types'; +import { TaskSecrets, TaskSpec, DbTaskEventRow } from './types'; async function createStore(): Promise { const manager = SingleConnectionDatabaseManager.fromConfig( @@ -39,6 +39,7 @@ async function createStore(): Promise { describe('StorageTaskBroker', () => { let storage: DatabaseTaskStore; + const fakeSecrets = { token: 'secret' } as TaskSecrets; beforeAll(async () => { storage = await createStore(); @@ -78,6 +79,13 @@ describe('StorageTaskBroker', () => { await expect(taskC.spec.steps[0].id).toBe('c'); }); + it('should store secrets', async () => { + const broker = new StorageTaskBroker(storage, logger); + await broker.dispatch({} as TaskSpec, fakeSecrets); + const task = await broker.claim(); + expect(task.secrets).toEqual(fakeSecrets); + }, 10000); + it('should complete a task', async () => { const broker = new StorageTaskBroker(storage, logger); const dispatchResult = await broker.dispatch({} as TaskSpec); @@ -87,6 +95,16 @@ describe('StorageTaskBroker', () => { expect(taskRow.status).toBe('completed'); }, 10000); + it('should remove secrets after completing a task', async () => { + const broker = new StorageTaskBroker(storage, logger); + const dispatchResult = await broker.dispatch({} as TaskSpec, fakeSecrets); + const task = await broker.claim(); + await task.complete('completed'); + const taskRow = await storage.getTask(dispatchResult.taskId); + expect(taskRow.status).toBe('completed'); + expect(taskRow.secrets).toBeUndefined(); + }, 10000); + it('should fail a task', async () => { const broker = new StorageTaskBroker(storage, logger); const dispatchResult = await broker.dispatch({} as TaskSpec); @@ -96,6 +114,16 @@ describe('StorageTaskBroker', () => { expect(taskRow.status).toBe('failed'); }); + it('should remove secrets after failing a task', async () => { + const broker = new StorageTaskBroker(storage, logger); + const dispatchResult = await broker.dispatch({} as TaskSpec, fakeSecrets); + const task = await broker.claim(); + await task.complete('failed'); + const taskRow = await storage.getTask(dispatchResult.taskId); + expect(taskRow.status).toBe('failed'); + expect(taskRow.secrets).toBeUndefined(); + }); + it('multiple brokers should be able to observe a single task', async () => { const broker1 = new StorageTaskBroker(storage, logger); const broker2 = new StorageTaskBroker(storage, logger); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index fb1f4ab422..6a82fa53ca 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -18,6 +18,7 @@ import { Logger } from 'winston'; import { CompletedTaskState, Task, + TaskSecrets, TaskSpec, TaskStore, TaskBroker, @@ -48,6 +49,10 @@ export class TaskAgent implements Task { return this.state.spec; } + get secrets() { + return this.state.secrets; + } + async getWorkspaceName() { return this.state.taskId; } @@ -101,6 +106,7 @@ export class TaskAgent implements Task { interface TaskState { spec: TaskSpec; taskId: string; + secrets?: TaskSecrets; } function defer() { @@ -126,6 +132,7 @@ export class StorageTaskBroker implements TaskBroker { { taskId: pendingTask.id, spec: pendingTask.spec, + secrets: pendingTask.secrets, }, this.storage, this.logger, @@ -136,8 +143,11 @@ export class StorageTaskBroker implements TaskBroker { } } - async dispatch(spec: TaskSpec): Promise { - const taskRow = await this.storage.createTask(spec); + async dispatch( + spec: TaskSpec, + secrets?: TaskSecrets, + ): Promise { + const taskRow = await this.storage.createTask(spec, secrets); this.signalDispatch(); return { taskId: taskRow.taskId, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 14752e1988..9e4deab31a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -164,6 +164,7 @@ export class TaskWorker { logger: taskLogger, logStream: stream, input, + token: task.secrets?.token, workspacePath, async createTemporaryDirectory() { const tmpDir = await fs.mkdtemp( diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 6963817a1e..d7d57a55ff 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -31,6 +31,7 @@ export type DbTaskRow = { status: Status; createdAt: string; lastHeartbeatAt?: string; + secrets?: TaskSecrets; }; export type TaskEventType = 'completion' | 'log'; @@ -54,12 +55,17 @@ export type TaskSpec = { output: { [name: string]: string }; }; +export type TaskSecrets = { + token: string | undefined; +}; + export type DispatchResult = { taskId: string; }; export interface Task { spec: TaskSpec; + secrets?: TaskSecrets; done: boolean; emitLog(message: string, metadata?: JsonValue): Promise; complete(result: CompletedTaskState, metadata?: JsonValue): Promise; @@ -68,7 +74,7 @@ export interface Task { export interface TaskBroker { claim(): Promise; - dispatch(spec: TaskSpec): Promise; + dispatch(spec: TaskSpec, secrets?: TaskSecrets): Promise; vacuumTasks(timeoutS: { timeoutS: number }): Promise; observe( options: { @@ -93,7 +99,10 @@ export type TaskStoreGetEventsOptions = { }; export interface TaskStore { - createTask(task: TaskSpec): Promise<{ taskId: string }>; + createTask( + task: TaskSpec, + secrets?: TaskSecrets, + ): Promise<{ taskId: string }>; getTask(taskId: string): Promise; claimTask(): Promise; completeTask(options: { diff --git a/plugins/scaffolder-backend/src/service/router.test.ts b/plugins/scaffolder-backend/src/service/router.test.ts index f01b4f8c95..bb52edaaea 100644 --- a/plugins/scaffolder-backend/src/service/router.test.ts +++ b/plugins/scaffolder-backend/src/service/router.test.ts @@ -301,4 +301,27 @@ describe('createRouter', () => { expect(response.status).toEqual(201); }); }); + + describe('GET /v2/tasks/:taskId', () => { + it('does not divulge secrets', async () => { + const postResponse = await request(app) + .post('/v2/tasks') + .set('Authorization', 'Bearer secret') + .send({ + templateName: 'create-react-app-template', + values: { + storePath: 'https://github.com/backstage/backstage', + component_id: '123', + name: 'test', + use_typescript: false, + }, + }); + + const response = await request(app) + .get(`/v2/tasks/${postResponse.body.id}`) + .send(); + expect(response.status).toEqual(200); + expect(response.body.secrets).toBeUndefined(); + }); + }); }); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 1e2891cfb8..35a57da6a2 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -358,8 +358,9 @@ export async function createRouter( .post('/v2/tasks', async (req, res) => { const templateName: string = req.body.templateName; const values: TemplaterValues = req.body.values; + const token = getBearerToken(req.headers.authorization); const template = await entityClient.findTemplate(templateName, { - token: getBearerToken(req.headers.authorization), + token, }); let taskSpec; @@ -402,7 +403,9 @@ export async function createRouter( ); } - const result = await taskBroker.dispatch(taskSpec); + const result = await taskBroker.dispatch(taskSpec, { + token: token, + }); res.status(201).json({ id: result.taskId }); }) @@ -412,6 +415,8 @@ export async function createRouter( if (!task) { throw new NotFoundError(`Task with id ${taskId} does not exist`); } + // Do not disclose secrets + delete task.secrets; res.status(200).json(task); }) .get('/v2/tasks/:taskId/eventstream', async (req, res) => {