paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 24 Packages Projects Releases Wiki Activity

Merge pull request #5202 from erikxiv/fix/scaffolder-forward-token

Browse Source 
fix: Forward user token to scaffolder taskworker for subsequent api r…
This commit is contained in:
Fredrik Adelöw
2021-04-17 20:14:47 +02:00
committed by GitHub
parent 6b0761dbbc 3f4c4a00dd
commit 2f423757d2
12 changed files with 156 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/eight-plums-hide.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-scaffolder-backend': patch
---
Forward user token to scaffolder task for subsequent api requests
plugins/scaffolder-backend/migrations/20210409225200_secrets.js
+38
View File
@@ -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');
});
};
plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.test.ts
+7 -4
View File
@@ -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',
plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts
+7 -4
View File
@@ -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}`);
plugins/scaffolder-backend/src/scaffolder/actions/types.ts
+4
View File
@@ -32,6 +32,10 @@ export type ActionContext<Input extends InputBase> = {
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;
plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts
+12 -1
View File
@@ -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<RawDbTaskRow>('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(
plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts
+29 -1
View File
@@ -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<DatabaseTaskStore> {
const manager = SingleConnectionDatabaseManager.fromConfig(
@@ -39,6 +39,7 @@ async function createStore(): Promise<DatabaseTaskStore> {
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);
plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts
+12 -2
View File
@@ -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<DispatchResult> {
const taskRow = await this.storage.createTask(spec);
async dispatch(
spec: TaskSpec,
secrets?: TaskSecrets,
): Promise<DispatchResult> {
const taskRow = await this.storage.createTask(spec, secrets);
this.signalDispatch();
return {
taskId: taskRow.taskId,
plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts
+1
View File
@@ -164,6 +164,7 @@ export class TaskWorker {
logger: taskLogger,
logStream: stream,
input,
token: task.secrets?.token,
workspacePath,
async createTemporaryDirectory() {
const tmpDir = await fs.mkdtemp(
plugins/scaffolder-backend/src/scaffolder/tasks/types.ts
+11 -2
View File
@@ -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<void>;
complete(result: CompletedTaskState, metadata?: JsonValue): Promise<void>;
@@ -68,7 +74,7 @@ export interface Task {
export interface TaskBroker {
claim(): Promise<Task>;
dispatch(spec: TaskSpec): Promise<DispatchResult>;
dispatch(spec: TaskSpec, secrets?: TaskSecrets): Promise<DispatchResult>;
vacuumTasks(timeoutS: { timeoutS: number }): Promise<void>;
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<DbTaskRow>;
claimTask(): Promise<DbTaskRow | undefined>;
completeTask(options: {
plugins/scaffolder-backend/src/service/router.test.ts
+23
View File
@@ -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();
});
});
});
plugins/scaffolder-backend/src/service/router.ts
+7 -2
View File
@@ -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) => {