From 6ed2b47d60dbe303edb71d6ee8b186005eb2d25e Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Tue, 5 Jan 2021 11:15:13 +0100 Subject: [PATCH 01/34] Add identity token to api requests --- .changeset/bright-rules-know.md | 12 ++ .../catalog-client/src/CatalogClient.test.ts | 18 ++- packages/catalog-client/src/CatalogClient.ts | 28 ++++- packages/catalog-client/src/types.ts | 14 +++ .../lib/catalog/CatalogIdentityClient.test.ts | 114 +++++++++++++++--- .../src/lib/catalog/CatalogIdentityClient.ts | 55 +++++++-- .../src/providers/google/provider.ts | 27 ++++- plugins/auth-backend/src/providers/types.ts | 2 - plugins/auth-backend/src/service/router.ts | 3 - .../src/api/CatalogImportClient.ts | 14 ++- plugins/catalog-import/src/plugin.ts | 11 +- plugins/catalog/src/plugin.ts | 6 +- plugins/fossa/src/api/FossaClient.ts | 14 ++- plugins/fossa/src/plugin.ts | 10 +- .../src/api/KubernetesBackendClient.ts | 11 +- plugins/kubernetes/src/plugin.ts | 7 +- plugins/rollbar/src/api/RollbarClient.ts | 16 ++- plugins/rollbar/src/plugin.ts | 6 +- .../src/lib/catalog/CatalogEntityClient.ts | 10 +- .../scaffolder-backend/src/service/router.ts | 7 +- 20 files changed, 317 insertions(+), 68 deletions(-) create mode 100644 .changeset/bright-rules-know.md diff --git a/.changeset/bright-rules-know.md b/.changeset/bright-rules-know.md new file mode 100644 index 0000000000..bc837a0bae --- /dev/null +++ b/.changeset/bright-rules-know.md @@ -0,0 +1,12 @@ +--- +'@backstage/catalog-client': minor +'@backstage/plugin-auth-backend': minor +'@backstage/plugin-catalog': minor +'@backstage/plugin-catalog-import': minor +'@backstage/plugin-fossa': minor +'@backstage/plugin-kubernetes': minor +'@backstage/plugin-rollbar': minor +'@backstage/plugin-scaffolder-backend': minor +--- + +Add identity token to api requests diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index 6369f95b76..9ee4fe3165 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -18,7 +18,7 @@ import { Entity } from '@backstage/catalog-model'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import { CatalogClient } from './CatalogClient'; -import { CatalogListResponse, DiscoveryApi } from './types'; +import { CatalogListResponse, DiscoveryApi, IdentityApi } from './types'; const server = setupServer(); const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; @@ -27,6 +27,20 @@ const discoveryApi: DiscoveryApi = { return mockBaseUrl; }, }; +const identityApi: IdentityApi = { + getUserId() { + return 'jane-fonda'; + }, + getProfile() { + return { email: 'jane-fonda@spotify.com' }; + }, + async getIdToken() { + return Promise.resolve('fake-id-token'); + }, + async signOut() { + return Promise.resolve(); + }, +}; describe('CatalogClient', () => { let client: CatalogClient; @@ -36,7 +50,7 @@ describe('CatalogClient', () => { afterEach(() => server.resetHandlers()); beforeEach(() => { - client = new CatalogClient({ discoveryApi }); + client = new CatalogClient({ discoveryApi, identityApi }); }); describe('getEntities', () => { diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index 362b1e71da..c559cb1210 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -28,13 +28,19 @@ import { CatalogEntitiesRequest, CatalogListResponse, DiscoveryApi, + IdentityApi, } from './types'; export class CatalogClient implements CatalogApi { private readonly discoveryApi: DiscoveryApi; + private readonly identityApi: IdentityApi; - constructor(options: { discoveryApi: DiscoveryApi }) { + constructor(options: { + discoveryApi: DiscoveryApi; + identityApi: IdentityApi; + }) { this.discoveryApi = options.discoveryApi; + this.identityApi = options.identityApi; } async getLocationById(id: String): Promise { @@ -76,12 +82,14 @@ export class CatalogClient implements CatalogApi { target, dryRun, }: AddLocationRequest): Promise { + const idToken = await this.identityApi.getIdToken(); const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/locations${ dryRun ? '?dryRun=true' : '' }`, { headers: { + authorization: `Bearer ${idToken}`, 'Content-Type': 'application/json', }, method: 'POST', @@ -119,9 +127,13 @@ export class CatalogClient implements CatalogApi { } async removeEntityByUid(uid: string): Promise { + const idToken = await this.identityApi.getIdToken(); const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/entities/by-uid/${uid}`, { + headers: { + authorization: `Bearer ${idToken}`, + }, method: 'DELETE', }, ); @@ -140,7 +152,12 @@ export class CatalogClient implements CatalogApi { private async getRequired(path: string): Promise { const url = `${await this.discoveryApi.getBaseUrl('catalog')}${path}`; - const response = await fetch(url); + const idToken = await this.identityApi.getIdToken(); + const response = await fetch(url, { + headers: { + authorization: `Bearer ${idToken}`, + }, + }); if (!response.ok) { const payload = await response.text(); @@ -153,7 +170,12 @@ export class CatalogClient implements CatalogApi { private async getOptional(path: string): Promise { const url = `${await this.discoveryApi.getBaseUrl('catalog')}${path}`; - const response = await fetch(url); + const idToken = await this.identityApi.getIdToken(); + const response = await fetch(url, { + headers: { + authorization: `Bearer ${idToken}`, + }, + }); if (!response.ok) { if (response.status === 404) { diff --git a/packages/catalog-client/src/types.ts b/packages/catalog-client/src/types.ts index e72317d444..0742dfed53 100644 --- a/packages/catalog-client/src/types.ts +++ b/packages/catalog-client/src/types.ts @@ -53,3 +53,17 @@ export type AddLocationResponse = { export type DiscoveryApi = { getBaseUrl(pluginId: string): Promise; }; +/** + * This is a copy of the core IdentityApi, to avoid importing core. + */ +export type ProfileInfo = { + email?: string; + displayName?: string; + picture?: string; +}; +export type IdentityApi = { + getUserId(): string; + getProfile(): ProfileInfo; + getIdToken(): Promise; + signOut(): Promise; +}; diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts index ec6aa1b6ba..0594785760 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts @@ -14,35 +14,109 @@ * limitations under the License. */ -import { CatalogApi } from '@backstage/catalog-client'; import { UserEntity } from '@backstage/catalog-model'; +import { rest } from 'msw'; +import { setupServer } from 'msw/node'; import { CatalogIdentityClient } from './CatalogIdentityClient'; +import { PluginEndpointDiscovery } from '@backstage/backend-common'; + +const server = setupServer(); +const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; +const discovery: PluginEndpointDiscovery = { + async getBaseUrl(_pluginId) { + return mockBaseUrl; + }, + async getExternalBaseUrl(_pluginId) { + return mockBaseUrl; + }, +}; describe('CatalogIdentityClient', () => { - const catalogApi: jest.Mocked = { - getLocationById: jest.fn(), - getEntityByName: jest.fn(), - getEntities: jest.fn(), - addLocation: jest.fn(), - getLocationByEntity: jest.fn(), - removeEntityByUid: jest.fn(), - }; + let client: CatalogIdentityClient; - afterEach(() => jest.resetAllMocks()); + beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); + afterAll(() => server.close()); + afterEach(() => server.resetHandlers()); - it('passes through the correct search params', async () => { - catalogApi.getEntities.mockResolvedValueOnce({ items: [{} as UserEntity] }); - const client = new CatalogIdentityClient({ - catalogApi: catalogApi as CatalogApi, + beforeEach(() => { + client = new CatalogIdentityClient({ discovery }); + }); + + describe('findUser', () => { + const defaultServiceResponse: UserEntity[] = [ + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + name: 'Test1', + namespace: 'test1', + annotations: { + key: 'value', + }, + }, + spec: { + memberOf: ['group1'], + }, + }, + ]; + + beforeEach(() => { + server.use( + rest.get(`${mockBaseUrl}/entities`, (_, res, ctx) => { + return res(ctx.json(defaultServiceResponse)); + }), + ); }); - client.findUser({ annotations: { key: 'value' } }); + it('should entities from correct endpoint', async () => { + const response = await client.findUser({ annotations: { key: 'value' } }); + expect(response).toEqual(defaultServiceResponse[0]); + }); - expect(catalogApi.getEntities).toBeCalledWith({ - filter: { - kind: 'user', - 'metadata.annotations.key': 'value', - }, + it('builds entity search filters properly', async () => { + expect.assertions(2); + + server.use( + rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { + expect(req.url.search).toBe( + '?filter=kind=user,metadata.annotations.key=value', + ); + return res(ctx.json(defaultServiceResponse)); + }), + ); + + const response = await client.findUser({ annotations: { key: 'value' } }); + + expect(response).toEqual(defaultServiceResponse[0]); + }); + + it('omits authorization header if not available', async () => { + expect.assertions(1); + + server.use( + rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { + expect(req.headers.has('authorization')).toBe(false); + return res(ctx.json([])); + }), + ); + + client.findUser({ annotations: { key: 'value' } }); + }); + + it('adds authorization header if available', async () => { + expect.assertions(1); + + server.use( + rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { + expect(req.headers.get('authorization')).toEqual('hello'); + return res(ctx.json([])); + }), + ); + + client.findUser( + { annotations: { key: 'value' } }, + { headers: { authorization: 'hello' } }, + ); }); }); }); diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts index 5bc4e5de21..b746d59cad 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts @@ -14,10 +14,13 @@ * limitations under the License. */ -import { ConflictError, NotFoundError } from '@backstage/backend-common'; -import { CatalogApi } from '@backstage/catalog-client'; +import fetch from 'cross-fetch'; +import { + ConflictError, + NotFoundError, + PluginEndpointDiscovery, +} from '@backstage/backend-common'; import { UserEntity } from '@backstage/catalog-model'; - type UserQuery = { annotations: Record; }; @@ -26,10 +29,10 @@ type UserQuery = { * A catalog client tailored for reading out identity data from the catalog. */ export class CatalogIdentityClient { - private readonly catalogApi: CatalogApi; + private readonly discovery: PluginEndpointDiscovery; - constructor(options: { catalogApi: CatalogApi }) { - this.catalogApi = options.catalogApi; + constructor(options: { discovery: PluginEndpointDiscovery }) { + this.discovery = options.discovery; } /** @@ -37,24 +40,54 @@ export class CatalogIdentityClient { * * Throws a NotFoundError or ConflictError if 0 or multiple users are found. */ - async findUser(query: UserQuery): Promise { + async findUser( + query: UserQuery, + options?: { headers?: Record }, + ): Promise { const filter: Record = { kind: 'user', }; for (const [key, value] of Object.entries(query.annotations)) { filter[`metadata.annotations.${key}`] = value; } + const params: string[] = []; - const { items } = await this.catalogApi.getEntities({ filter }); + const filterParts: string[] = []; + for (const [key, value] of Object.entries(filter)) { + for (const v of [value].flat()) { + filterParts.push(`${encodeURIComponent(key)}=${encodeURIComponent(v)}`); + } + } + if (filterParts.length) { + params.push(`filter=${filterParts.join(',')}`); + } + const queryPart = params.length ? `?${params.join('&')}` : ''; - if (items.length !== 1) { - if (items.length > 1) { + const url = `${await this.discovery.getBaseUrl( + 'catalog', + )}/entities${queryPart}`; + const response = await fetch(url, { + headers: { + ...options?.headers, + }, + }); + + if (!response.ok) { + const payload = await response.text(); + const message = `Request failed with ${response.status} ${response.statusText}, ${payload}`; + throw new Error(message); + } + + const entities: UserEntity[] = await response.json(); + + if (entities.length !== 1) { + if (entities.length > 1) { throw new ConflictError('User lookup resulted in multiple matches'); } else { throw new NotFoundError('User not found'); } } - return items[0] as UserEntity; + return entities[0] as UserEntity; } } diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index ae4f5fabd0..10f3ae96f2 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -38,6 +38,7 @@ import { PassportDoneCallback, } from '../../lib/passport'; import { AuthProviderFactory, RedirectInfo } from '../types'; +import { TokenIssuer } from '../../identity'; type PrivateInfo = { refreshToken: string; @@ -46,16 +47,19 @@ type PrivateInfo = { export type GoogleAuthProviderOptions = OAuthProviderOptions & { logger: Logger; identityClient: CatalogIdentityClient; + tokenIssuer: TokenIssuer; }; export class GoogleAuthProvider implements OAuthHandlers { private readonly _strategy: GoogleStrategy; private readonly logger: Logger; private readonly identityClient: CatalogIdentityClient; + private readonly tokenIssuer: TokenIssuer; constructor(options: GoogleAuthProviderOptions) { this.logger = options.logger; this.identityClient = options.identityClient; + this.tokenIssuer = options.tokenIssuer; // TODO: throw error if env variables not set? this._strategy = new GoogleStrategy( { @@ -150,11 +154,21 @@ export class GoogleAuthProvider implements OAuthHandlers { } try { - const user = await this.identityClient.findUser({ - annotations: { - 'google.com/email': profile.email, - }, + const token = await this.tokenIssuer.issueToken({ + claims: { sub: 'backstage.io/auth-backend' }, }); + const user = await this.identityClient.findUser( + { + annotations: { + 'google.com/email': profile.email, + }, + }, + { + headers: { + authorization: `Bearer ${token}`, + }, + }, + ); return { ...response, @@ -180,7 +194,7 @@ export const createGoogleProvider: AuthProviderFactory = ({ config, logger, tokenIssuer, - catalogApi, + discovery, }) => OAuthEnvironmentHandler.mapConfig(config, envConfig => { const clientId = envConfig.getString('clientId'); @@ -192,7 +206,8 @@ export const createGoogleProvider: AuthProviderFactory = ({ clientSecret, callbackUrl, logger, - identityClient: new CatalogIdentityClient({ catalogApi }), + tokenIssuer, + identityClient: new CatalogIdentityClient({ discovery }), }); return OAuthAdapter.fromConfig(globalConfig, provider, { diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 1a4e7b118a..8339258654 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -15,7 +15,6 @@ */ import { PluginEndpointDiscovery } from '@backstage/backend-common'; -import { CatalogApi } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; import express from 'express'; import { Logger } from 'winston'; @@ -132,7 +131,6 @@ export type AuthProviderFactoryOptions = { logger: Logger; tokenIssuer: TokenIssuer; discovery: PluginEndpointDiscovery; - catalogApi: CatalogApi; identityResolver?: ExperimentalIdentityResolver; }; diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 85d60661e9..742a269b73 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -27,7 +27,6 @@ import { PluginDatabaseManager, PluginEndpointDiscovery, } from '@backstage/backend-common'; -import { CatalogClient } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; import { createOidcRouter, DatabaseKeyStore, TokenFactory } from '../identity'; import session from 'express-session'; @@ -66,7 +65,6 @@ export async function createRouter({ keyDurationSeconds, logger: logger.child({ component: 'token-factory' }), }); - const catalogApi = new CatalogClient({ discoveryApi: discovery }); const secret = config.getOptionalString('auth.session.secret'); if (secret) { @@ -103,7 +101,6 @@ export async function createRouter({ logger, tokenIssuer, discovery, - catalogApi, }); const r = Router(); diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index d1509c2105..9b52de0ba4 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -15,23 +15,31 @@ */ import { Octokit } from '@octokit/rest'; -import { DiscoveryApi, OAuthApi, ConfigApi } from '@backstage/core'; +import { + DiscoveryApi, + IdentityApi, + OAuthApi, + ConfigApi, +} from '@backstage/core'; import { CatalogImportApi } from './CatalogImportApi'; import { PartialEntity } from '../util/types'; import { GitHubIntegrationConfig } from '@backstage/integration'; export class CatalogImportClient implements CatalogImportApi { private readonly discoveryApi: DiscoveryApi; + private readonly identityApi: IdentityApi; private readonly githubAuthApi: OAuthApi; private readonly configApi: ConfigApi; constructor(options: { discoveryApi: DiscoveryApi; githubAuthApi: OAuthApi; + identityApi: IdentityApi; configApi: ConfigApi; }) { this.discoveryApi = options.discoveryApi; this.githubAuthApi = options.githubAuthApi; + this.identityApi = options.identityApi; this.configApi = options.configApi; } @@ -40,10 +48,12 @@ export class CatalogImportClient implements CatalogImportApi { }: { repo: string; }): Promise { + const idToken = await this.identityApi.getIdToken(); const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/analyze-location`, { headers: { + authorization: `Bearer ${idToken}`, 'Content-Type': 'application/json', }, method: 'POST', @@ -69,10 +79,12 @@ export class CatalogImportClient implements CatalogImportApi { }: { location: string; }): Promise { + const idToken = await this.identityApi.getIdToken(); const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/locations`, { headers: { + authorization: `Bearer ${idToken}`, 'Content-Type': 'application/json', }, method: 'POST', diff --git a/plugins/catalog-import/src/plugin.ts b/plugins/catalog-import/src/plugin.ts index 42210d0ed1..60e5128df0 100644 --- a/plugins/catalog-import/src/plugin.ts +++ b/plugins/catalog-import/src/plugin.ts @@ -20,6 +20,7 @@ import { createRouteRef, discoveryApiRef, githubAuthApiRef, + identityApiRef, configApiRef, createRoutableExtension, } from '@backstage/core'; @@ -39,10 +40,16 @@ export const catalogImportPlugin = createPlugin({ deps: { discoveryApi: discoveryApiRef, githubAuthApi: githubAuthApiRef, + identityApi: identityApiRef, configApi: configApiRef, }, - factory: ({ discoveryApi, githubAuthApi, configApi }) => - new CatalogImportClient({ discoveryApi, githubAuthApi, configApi }), + factory: ({ discoveryApi, githubAuthApi, identityApi, configApi }) => + new CatalogImportClient({ + discoveryApi, + githubAuthApi, + identityApi, + configApi, + }), }), ], routes: { diff --git a/plugins/catalog/src/plugin.ts b/plugins/catalog/src/plugin.ts index c19925775d..557762ee46 100644 --- a/plugins/catalog/src/plugin.ts +++ b/plugins/catalog/src/plugin.ts @@ -20,6 +20,7 @@ import { createPlugin, discoveryApiRef, createRoutableExtension, + identityApiRef, } from '@backstage/core'; import { catalogApiRef, @@ -32,8 +33,9 @@ export const catalogPlugin = createPlugin({ apis: [ createApiFactory({ api: catalogApiRef, - deps: { discoveryApi: discoveryApiRef }, - factory: ({ discoveryApi }) => new CatalogClient({ discoveryApi }), + deps: { discoveryApi: discoveryApiRef, identityApi: identityApiRef }, + factory: ({ discoveryApi, identityApi }) => + new CatalogClient({ discoveryApi, identityApi }), }), ], routes: { diff --git a/plugins/fossa/src/api/FossaClient.ts b/plugins/fossa/src/api/FossaClient.ts index 05a889177a..6730f7ddb3 100644 --- a/plugins/fossa/src/api/FossaClient.ts +++ b/plugins/fossa/src/api/FossaClient.ts @@ -14,28 +14,38 @@ * limitations under the License. */ -import { DiscoveryApi } from '@backstage/core'; +import { DiscoveryApi, IdentityApi } from '@backstage/core'; import fetch from 'cross-fetch'; import { FindingSummary, FossaApi } from './FossaApi'; export class FossaClient implements FossaApi { discoveryApi: DiscoveryApi; + identityApi?: IdentityApi; organizationId?: string; constructor({ discoveryApi, + identityApi, organizationId, }: { discoveryApi: DiscoveryApi; + identityApi?: IdentityApi; organizationId?: string; }) { this.discoveryApi = discoveryApi; + this.identityApi = identityApi; this.organizationId = organizationId; } private async callApi(path: string): Promise { const apiUrl = `${await this.discoveryApi.getBaseUrl('proxy')}/fossa`; - const response = await fetch(`${apiUrl}/${path}`); + const headers: Record = {}; + if (this.identityApi) { + headers.authorization = `Bearer ${this.identityApi.getIdToken()}`; + } + const response = await fetch(`${apiUrl}/${path}`, { + headers, + }); if (response.status === 200) { return await response.json(); } diff --git a/plugins/fossa/src/plugin.ts b/plugins/fossa/src/plugin.ts index cb1de3911f..638be4a0bf 100644 --- a/plugins/fossa/src/plugin.ts +++ b/plugins/fossa/src/plugin.ts @@ -19,6 +19,7 @@ import { createApiFactory, createPlugin, discoveryApiRef, + identityApiRef, } from '@backstage/core'; import { fossaApiRef, FossaClient } from './api'; @@ -27,10 +28,15 @@ export const fossaPlugin = createPlugin({ apis: [ createApiFactory({ api: fossaApiRef, - deps: { configApi: configApiRef, discoveryApi: discoveryApiRef }, - factory: ({ configApi, discoveryApi }) => + deps: { + configApi: configApiRef, + discoveryApi: discoveryApiRef, + identityApi: identityApiRef, + }, + factory: ({ configApi, discoveryApi, identityApi }) => new FossaClient({ discoveryApi, + identityApi, organizationId: configApi.getOptionalString('fossa.organizationId'), }), }), diff --git a/plugins/kubernetes/src/api/KubernetesBackendClient.ts b/plugins/kubernetes/src/api/KubernetesBackendClient.ts index 287b411259..e005b29517 100644 --- a/plugins/kubernetes/src/api/KubernetesBackendClient.ts +++ b/plugins/kubernetes/src/api/KubernetesBackendClient.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { DiscoveryApi } from '@backstage/core'; +import { DiscoveryApi, IdentityApi } from '@backstage/core'; import { KubernetesApi } from './types'; import { KubernetesRequestBody, @@ -23,9 +23,14 @@ import { export class KubernetesBackendClient implements KubernetesApi { private readonly discoveryApi: DiscoveryApi; + private readonly identityApi: IdentityApi; - constructor(options: { discoveryApi: DiscoveryApi }) { + constructor(options: { + discoveryApi: DiscoveryApi; + identityApi: IdentityApi; + }) { this.discoveryApi = options.discoveryApi; + this.identityApi = options.identityApi; } private async getRequired( @@ -33,9 +38,11 @@ export class KubernetesBackendClient implements KubernetesApi { requestBody: KubernetesRequestBody, ): Promise { const url = `${await this.discoveryApi.getBaseUrl('kubernetes')}${path}`; + const idToken = await this.identityApi.getIdToken(); const response = await fetch(url, { method: 'POST', headers: { + authorization: `Bearer ${idToken}`, 'Content-Type': 'application/json', }, body: JSON.stringify(requestBody), diff --git a/plugins/kubernetes/src/plugin.ts b/plugins/kubernetes/src/plugin.ts index cff9e1468b..04ab7a086f 100644 --- a/plugins/kubernetes/src/plugin.ts +++ b/plugins/kubernetes/src/plugin.ts @@ -18,6 +18,7 @@ import { createPlugin, createRouteRef, discoveryApiRef, + identityApiRef, googleAuthApiRef, } from '@backstage/core'; import { KubernetesBackendClient } from './api/KubernetesBackendClient'; @@ -35,9 +36,9 @@ export const plugin = createPlugin({ apis: [ createApiFactory({ api: kubernetesApiRef, - deps: { discoveryApi: discoveryApiRef }, - factory: ({ discoveryApi }) => - new KubernetesBackendClient({ discoveryApi }), + deps: { discoveryApi: discoveryApiRef, identityApi: identityApiRef }, + factory: ({ discoveryApi, identityApi }) => + new KubernetesBackendClient({ discoveryApi, identityApi }), }), createApiFactory({ api: kubernetesAuthProvidersApiRef, diff --git a/plugins/rollbar/src/api/RollbarClient.ts b/plugins/rollbar/src/api/RollbarClient.ts index e612890655..1041accf19 100644 --- a/plugins/rollbar/src/api/RollbarClient.ts +++ b/plugins/rollbar/src/api/RollbarClient.ts @@ -20,13 +20,18 @@ import { RollbarProject, RollbarTopActiveItem, } from './types'; -import { DiscoveryApi } from '@backstage/core'; +import { DiscoveryApi, IdentityApi } from '@backstage/core'; export class RollbarClient implements RollbarApi { private readonly discoveryApi: DiscoveryApi; + private readonly identityApi: IdentityApi; - constructor(options: { discoveryApi: DiscoveryApi }) { + constructor(options: { + discoveryApi: DiscoveryApi; + identityApi: IdentityApi; + }) { this.discoveryApi = options.discoveryApi; + this.identityApi = options.identityApi; } async getAllProjects(): Promise { @@ -53,7 +58,12 @@ export class RollbarClient implements RollbarApi { private async get(path: string): Promise { const url = `${await this.discoveryApi.getBaseUrl('rollbar')}${path}`; - const response = await fetch(url); + const idToken = await this.identityApi.getIdToken(); + const response = await fetch(url, { + headers: { + authorization: `Bearer ${idToken}`, + }, + }); if (!response.ok) { const payload = await response.text(); diff --git a/plugins/rollbar/src/plugin.ts b/plugins/rollbar/src/plugin.ts index c3ca6173ff..9456c10c6b 100644 --- a/plugins/rollbar/src/plugin.ts +++ b/plugins/rollbar/src/plugin.ts @@ -18,6 +18,7 @@ import { createPlugin, createApiFactory, discoveryApiRef, + identityApiRef, } from '@backstage/core'; import { rootRouteRef, entityRouteRef } from './routes'; import { RollbarHome } from './components/RollbarHome/RollbarHome'; @@ -30,8 +31,9 @@ export const plugin = createPlugin({ apis: [ createApiFactory({ api: rollbarApiRef, - deps: { discoveryApi: discoveryApiRef }, - factory: ({ discoveryApi }) => new RollbarClient({ discoveryApi }), + deps: { discoveryApi: discoveryApiRef, identityApi: identityApiRef }, + factory: ({ discoveryApi, identityApi }) => + new RollbarClient({ discoveryApi, identityApi }), }), ], register({ router }) { diff --git a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts index 4541f03a4a..c5e20202c5 100644 --- a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts +++ b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts @@ -37,7 +37,10 @@ export class CatalogEntityClient { * * Throws a NotFoundError or ConflictError if 0 or multiple templates are found. */ - async findTemplate(templateName: string): Promise { + async findTemplate( + templateName: string, + options?: { headers?: Record }, + ): Promise { const conditions = [ 'kind=template', `metadata.name=${encodeURIComponent(templateName)}`, @@ -46,6 +49,11 @@ export class CatalogEntityClient { const baseUrl = await this.discovery.getBaseUrl('catalog'); const response = await fetch( `${baseUrl}/entities?filter=${conditions.join(',')}`, + { + headers: { + ...options?.headers, + }, + }, ); if (!response.ok) { diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 9098e5d05e..070b7cffc7 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -97,7 +97,12 @@ export async function createRouter( }, }; - const template = await entityClient.findTemplate(templateName); + // Forward authorization header from client + const template = await entityClient.findTemplate(templateName, { + headers: req.headers.authorization + ? { authorization: req.headers.authorization } + : {}, + }); const validationResult: ValidatorResult = validate( values, From 570884ece6afd2843090422ad1d452e0df1dbdac Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Tue, 5 Jan 2021 11:22:50 +0100 Subject: [PATCH 02/34] Fix linting --- .../src/lib/catalog/CatalogIdentityClient.test.ts | 4 ++-- plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts index 0594785760..9d29e68277 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts @@ -23,10 +23,10 @@ import { PluginEndpointDiscovery } from '@backstage/backend-common'; const server = setupServer(); const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; const discovery: PluginEndpointDiscovery = { - async getBaseUrl(_pluginId) { + async getBaseUrl() { return mockBaseUrl; }, - async getExternalBaseUrl(_pluginId) { + async getExternalBaseUrl() { return mockBaseUrl; }, }; diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts index b746d59cad..f4aa722b2d 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts @@ -21,6 +21,7 @@ import { PluginEndpointDiscovery, } from '@backstage/backend-common'; import { UserEntity } from '@backstage/catalog-model'; + type UserQuery = { annotations: Record; }; From b52baa518b4868a3c344a752cfa29fab1c24515a Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Tue, 19 Jan 2021 18:34:21 +0100 Subject: [PATCH 03/34] Make identityApi required --- plugins/fossa/src/api/FossaClient.test.ts | 25 ++++++++++++++++++++--- plugins/fossa/src/api/FossaClient.ts | 11 +++++----- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/plugins/fossa/src/api/FossaClient.test.ts b/plugins/fossa/src/api/FossaClient.test.ts index dd0fdeff5e..820210e2b6 100644 --- a/plugins/fossa/src/api/FossaClient.test.ts +++ b/plugins/fossa/src/api/FossaClient.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { UrlPatternDiscovery } from '@backstage/core'; +import { UrlPatternDiscovery, IdentityApi } from '@backstage/core'; import { msw } from '@backstage/test-utils'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; @@ -22,6 +22,21 @@ import { FindingSummary, FossaApi, FossaClient } from './index'; const server = setupServer(); +const identityApi: IdentityApi = { + getUserId() { + return 'jane-fonda'; + }, + getProfile() { + return { email: 'jane-fonda@spotify.com' }; + }, + async getIdToken() { + return Promise.resolve('fake-id-token'); + }, + async signOut() { + return Promise.resolve(); + }, +}; + describe('FossaClient', () => { msw.setupDefaultHandlers(server); @@ -30,7 +45,11 @@ describe('FossaClient', () => { let client: FossaApi; beforeEach(() => { - client = new FossaClient({ discoveryApi, organizationId: '8736' }); + client = new FossaClient({ + discoveryApi, + identityApi, + organizationId: '8736', + }); }); it('should report finding summary', async () => { @@ -137,7 +156,7 @@ describe('FossaClient', () => { }); it('should skip organizationId', async () => { - client = new FossaClient({ discoveryApi }); + client = new FossaClient({ discoveryApi, identityApi }); server.use( rest.get(`${mockBaseUrl}/fossa/projects`, (req, res, ctx) => { diff --git a/plugins/fossa/src/api/FossaClient.ts b/plugins/fossa/src/api/FossaClient.ts index 6730f7ddb3..33c8c65c5f 100644 --- a/plugins/fossa/src/api/FossaClient.ts +++ b/plugins/fossa/src/api/FossaClient.ts @@ -20,7 +20,7 @@ import { FindingSummary, FossaApi } from './FossaApi'; export class FossaClient implements FossaApi { discoveryApi: DiscoveryApi; - identityApi?: IdentityApi; + identityApi: IdentityApi; organizationId?: string; constructor({ @@ -29,7 +29,7 @@ export class FossaClient implements FossaApi { organizationId, }: { discoveryApi: DiscoveryApi; - identityApi?: IdentityApi; + identityApi: IdentityApi; organizationId?: string; }) { this.discoveryApi = discoveryApi; @@ -39,10 +39,9 @@ export class FossaClient implements FossaApi { private async callApi(path: string): Promise { const apiUrl = `${await this.discoveryApi.getBaseUrl('proxy')}/fossa`; - const headers: Record = {}; - if (this.identityApi) { - headers.authorization = `Bearer ${this.identityApi.getIdToken()}`; - } + const headers: Record = { + authorization: await `Bearer ${this.identityApi.getIdToken()}`, + }; const response = await fetch(`${apiUrl}/${path}`, { headers, }); From 3647d0577f7e18201e98f2de24c6762e165b9e47 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Tue, 19 Jan 2021 18:38:33 +0100 Subject: [PATCH 04/34] Add token to CatalogClient and add wrapper class for frontend --- .../catalog-client/src/CatalogClient.test.ts | 25 ++---- packages/catalog-client/src/CatalogClient.ts | 84 +++++++++++-------- packages/catalog-client/src/index.ts | 8 +- packages/catalog-client/src/types.ts | 14 ---- .../catalog/src/CatalogClientWrapper.test.ts | 65 ++++++++++++++ plugins/catalog/src/CatalogClientWrapper.ts | 68 +++++++++++++++ plugins/catalog/src/plugin.ts | 7 +- 7 files changed, 200 insertions(+), 71 deletions(-) create mode 100644 plugins/catalog/src/CatalogClientWrapper.test.ts create mode 100644 plugins/catalog/src/CatalogClientWrapper.ts diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index 9ee4fe3165..20cc35f423 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -18,29 +18,16 @@ import { Entity } from '@backstage/catalog-model'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import { CatalogClient } from './CatalogClient'; -import { CatalogListResponse, DiscoveryApi, IdentityApi } from './types'; +import { CatalogListResponse, DiscoveryApi } from './types'; const server = setupServer(); +const token = 'fake-token'; const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; const discoveryApi: DiscoveryApi = { async getBaseUrl(_pluginId) { return mockBaseUrl; }, }; -const identityApi: IdentityApi = { - getUserId() { - return 'jane-fonda'; - }, - getProfile() { - return { email: 'jane-fonda@spotify.com' }; - }, - async getIdToken() { - return Promise.resolve('fake-id-token'); - }, - async signOut() { - return Promise.resolve(); - }, -}; describe('CatalogClient', () => { let client: CatalogClient; @@ -50,7 +37,7 @@ describe('CatalogClient', () => { afterEach(() => server.resetHandlers()); beforeEach(() => { - client = new CatalogClient({ discoveryApi, identityApi }); + client = new CatalogClient({ discoveryApi }); }); describe('getEntities', () => { @@ -85,7 +72,7 @@ describe('CatalogClient', () => { }); it('should entities from correct endpoint', async () => { - const response = await client.getEntities(); + const response = await client.getEntities(token); expect(response).toEqual(defaultResponse); }); @@ -99,7 +86,7 @@ describe('CatalogClient', () => { }), ); - const response = await client.getEntities({ + const response = await client.getEntities(token, { filter: { a: '1', b: ['2', '3'], @@ -120,7 +107,7 @@ describe('CatalogClient', () => { }), ); - const response = await client.getEntities({ + const response = await client.getEntities(token, { fields: ['a.b', 'รถ'], }); diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index c559cb1210..c4f8c3d2b0 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -24,30 +24,27 @@ import fetch from 'cross-fetch'; import { AddLocationRequest, AddLocationResponse, - CatalogApi, CatalogEntitiesRequest, CatalogListResponse, DiscoveryApi, - IdentityApi, } from './types'; -export class CatalogClient implements CatalogApi { +export class CatalogClient { private readonly discoveryApi: DiscoveryApi; - private readonly identityApi: IdentityApi; - constructor(options: { - discoveryApi: DiscoveryApi; - identityApi: IdentityApi; - }) { + constructor(options: { discoveryApi: DiscoveryApi }) { this.discoveryApi = options.discoveryApi; - this.identityApi = options.identityApi; } - async getLocationById(id: String): Promise { - return await this.getOptional(`/locations/${id}`); + async getLocationById( + token: string | undefined, + id: String, + ): Promise { + return await this.getOptional(token, `/locations/${id}`); } async getEntities( + token: string | undefined, request?: CatalogEntitiesRequest, ): Promise> { const { filter = {}, fields = [] } = request ?? {}; @@ -68,28 +65,35 @@ export class CatalogClient implements CatalogApi { } const query = params.length ? `?${params.join('&')}` : ''; - const entities: Entity[] = await this.getRequired(`/entities${query}`); + const entities: Entity[] = await this.getRequired( + token, + `/entities${query}`, + ); return { items: entities }; } - async getEntityByName(compoundName: EntityName): Promise { + async getEntityByName( + token: string | undefined, + compoundName: EntityName, + ): Promise { const { kind, namespace = 'default', name } = compoundName; - return this.getOptional(`/entities/by-name/${kind}/${namespace}/${name}`); + return this.getOptional( + token, + `/entities/by-name/${kind}/${namespace}/${name}`, + ); } - async addLocation({ - type = 'url', - target, - dryRun, - }: AddLocationRequest): Promise { - const idToken = await this.identityApi.getIdToken(); + async addLocation( + token: string | undefined, + { type = 'url', target, dryRun }: AddLocationRequest, + ): Promise { const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/locations${ dryRun ? '?dryRun=true' : '' }`, { headers: { - authorization: `Bearer ${idToken}`, + authorization: `Bearer ${token}`, 'Content-Type': 'application/json', }, method: 'POST', @@ -118,21 +122,29 @@ export class CatalogClient implements CatalogApi { }; } - async getLocationByEntity(entity: Entity): Promise { + async getLocationByEntity( + token: string | undefined, + entity: Entity, + ): Promise { const locationCompound = entity.metadata.annotations?.[LOCATION_ANNOTATION]; - const all: { data: Location }[] = await this.getRequired('/locations'); + const all: { data: Location }[] = await this.getRequired( + token, + '/locations', + ); return all .map(r => r.data) .find(l => locationCompound === `${l.type}:${l.target}`); } - async removeEntityByUid(uid: string): Promise { - const idToken = await this.identityApi.getIdToken(); + async removeEntityByUid( + token: string | undefined, + uid: string, + ): Promise { const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/entities/by-uid/${uid}`, { headers: { - authorization: `Bearer ${idToken}`, + authorization: `Bearer ${token}`, }, method: 'DELETE', }, @@ -150,13 +162,13 @@ export class CatalogClient implements CatalogApi { // Private methods // - private async getRequired(path: string): Promise { + private async getRequired( + token: string | undefined, + path: string, + ): Promise { const url = `${await this.discoveryApi.getBaseUrl('catalog')}${path}`; - const idToken = await this.identityApi.getIdToken(); const response = await fetch(url, { - headers: { - authorization: `Bearer ${idToken}`, - }, + headers: token ? { authorization: `Bearer ${token}` } : {}, }); if (!response.ok) { @@ -168,13 +180,13 @@ export class CatalogClient implements CatalogApi { return await response.json(); } - private async getOptional(path: string): Promise { + private async getOptional( + token: string | undefined, + path: string, + ): Promise { const url = `${await this.discoveryApi.getBaseUrl('catalog')}${path}`; - const idToken = await this.identityApi.getIdToken(); const response = await fetch(url, { - headers: { - authorization: `Bearer ${idToken}`, - }, + headers: token ? { authorization: `Bearer ${token}` } : {}, }); if (!response.ok) { diff --git a/packages/catalog-client/src/index.ts b/packages/catalog-client/src/index.ts index 9bd13f9b7c..c5a626e25b 100644 --- a/packages/catalog-client/src/index.ts +++ b/packages/catalog-client/src/index.ts @@ -15,4 +15,10 @@ */ export { CatalogClient } from './CatalogClient'; -export type { CatalogApi } from './types'; +export type { + AddLocationRequest, + AddLocationResponse, + CatalogApi, + CatalogEntitiesRequest, + CatalogListResponse, +} from './types'; diff --git a/packages/catalog-client/src/types.ts b/packages/catalog-client/src/types.ts index 0742dfed53..e72317d444 100644 --- a/packages/catalog-client/src/types.ts +++ b/packages/catalog-client/src/types.ts @@ -53,17 +53,3 @@ export type AddLocationResponse = { export type DiscoveryApi = { getBaseUrl(pluginId: string): Promise; }; -/** - * This is a copy of the core IdentityApi, to avoid importing core. - */ -export type ProfileInfo = { - email?: string; - displayName?: string; - picture?: string; -}; -export type IdentityApi = { - getUserId(): string; - getProfile(): ProfileInfo; - getIdToken(): Promise; - signOut(): Promise; -}; diff --git a/plugins/catalog/src/CatalogClientWrapper.test.ts b/plugins/catalog/src/CatalogClientWrapper.test.ts new file mode 100644 index 0000000000..8a5c58194a --- /dev/null +++ b/plugins/catalog/src/CatalogClientWrapper.test.ts @@ -0,0 +1,65 @@ +/* + * 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 { CatalogClient } from '@backstage/catalog-client'; +import { DiscoveryApi, IdentityApi } from '@backstage/core'; +import { CatalogClientWrapper } from './CatalogClientWrapper'; + +jest.mock('./CatalogClient'); +const MockedCatalogClient = CatalogClient as jest.Mock; + +const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; +const discoveryApi: DiscoveryApi = { + async getBaseUrl(_pluginId) { + return mockBaseUrl; + }, +}; +const identityApi: IdentityApi = { + getUserId() { + return 'jane-fonda'; + }, + getProfile() { + return { email: 'jane-fonda@spotify.com' }; + }, + async getIdToken() { + return Promise.resolve('fake-id-token'); + }, + async signOut() { + return Promise.resolve(); + }, +}; + +describe('CatalogClientWrapper', () => { + let client: CatalogClientWrapper; + + beforeEach(() => { + MockedCatalogClient.mockClear(); + client = new CatalogClientWrapper({ + client: new MockedCatalogClient({ discoveryApi }), + identityApi, + }); + }); + + describe('getEntities', () => { + it('injects authorization token', async () => { + expect.assertions(2); + await client.getEntities(); + const getEntities = MockedCatalogClient.mock.instances[0].getEntities; + expect(getEntities).toHaveBeenCalledWith('fake-id-token', undefined); + expect(getEntities).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/plugins/catalog/src/CatalogClientWrapper.ts b/plugins/catalog/src/CatalogClientWrapper.ts new file mode 100644 index 0000000000..fab551de48 --- /dev/null +++ b/plugins/catalog/src/CatalogClientWrapper.ts @@ -0,0 +1,68 @@ +/* + * 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 { Entity, EntityName, Location } from '@backstage/catalog-model'; +import { + AddLocationRequest, + AddLocationResponse, + CatalogApi, + CatalogEntitiesRequest, + CatalogListResponse, + CatalogClient, +} from '@backstage/catalog-client'; +import { IdentityApi } from '@backstage/core'; + +export class CatalogClientWrapper implements CatalogApi { + private readonly identityApi: IdentityApi; + private readonly client: CatalogClient; + + constructor(options: { client: CatalogClient; identityApi: IdentityApi }) { + this.client = options.client; + this.identityApi = options.identityApi; + } + + async getLocationById(id: String): Promise { + const token = await this.identityApi.getIdToken(); + return await this.client.getLocationById(token, id); + } + + async getEntities( + request?: CatalogEntitiesRequest, + ): Promise> { + const token = await this.identityApi.getIdToken(); + return await this.client.getEntities(token, request); + } + + async getEntityByName(compoundName: EntityName): Promise { + const token = await this.identityApi.getIdToken(); + return await this.client.getEntityByName(token, compoundName); + } + + async addLocation(request: AddLocationRequest): Promise { + const token = await this.identityApi.getIdToken(); + return await this.client.addLocation(token, request); + } + + async getLocationByEntity(entity: Entity): Promise { + const token = await this.identityApi.getIdToken(); + return await this.client.getLocationByEntity(token, entity); + } + + async removeEntityByUid(uid: string): Promise { + const token = await this.identityApi.getIdToken(); + return await this.client.removeEntityByUid(token, uid); + } +} diff --git a/plugins/catalog/src/plugin.ts b/plugins/catalog/src/plugin.ts index 557762ee46..55a9f29372 100644 --- a/plugins/catalog/src/plugin.ts +++ b/plugins/catalog/src/plugin.ts @@ -27,6 +27,8 @@ import { catalogRouteRef, entityRouteRef, } from '@backstage/plugin-catalog-react'; +import { catalogRouteRef, entityRouteRef } from './routes'; +import { CatalogClientWrapper } from './CatalogClientWrapper'; export const catalogPlugin = createPlugin({ id: 'catalog', @@ -35,7 +37,10 @@ export const catalogPlugin = createPlugin({ api: catalogApiRef, deps: { discoveryApi: discoveryApiRef, identityApi: identityApiRef }, factory: ({ discoveryApi, identityApi }) => - new CatalogClient({ discoveryApi, identityApi }), + new CatalogClientWrapper({ + client: new CatalogClient({ discoveryApi }), + identityApi, + }), }), ], routes: { From be2958d731f08614df02bd9213a7776e2eec7485 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Tue, 19 Jan 2021 19:17:27 +0100 Subject: [PATCH 05/34] CatalogIdentityClient uses CatalogClient --- .../lib/catalog/CatalogIdentityClient.test.ts | 112 +++--------------- .../src/lib/catalog/CatalogIdentityClient.ts | 51 ++------ .../src/providers/google/provider.ts | 19 +-- plugins/auth-backend/src/providers/types.ts | 2 + plugins/auth-backend/src/service/router.ts | 3 + 5 files changed, 38 insertions(+), 149 deletions(-) diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts index 9d29e68277..ffd118884e 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts @@ -14,109 +14,31 @@ * limitations under the License. */ -import { UserEntity } from '@backstage/catalog-model'; -import { rest } from 'msw'; -import { setupServer } from 'msw/node'; +import { CatalogClient } from '@backstage/catalog-client'; import { CatalogIdentityClient } from './CatalogIdentityClient'; -import { PluginEndpointDiscovery } from '@backstage/backend-common'; -const server = setupServer(); -const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; -const discovery: PluginEndpointDiscovery = { - async getBaseUrl() { - return mockBaseUrl; - }, - async getExternalBaseUrl() { - return mockBaseUrl; - }, -}; +jest.mock('./CatalogClient'); +const MockedCatalogClient = CatalogClient as jest.Mock; describe('CatalogIdentityClient', () => { - let client: CatalogIdentityClient; + const token = 'fake-id-token'; - beforeAll(() => server.listen({ onUnhandledRequest: 'error' })); - afterAll(() => server.close()); - afterEach(() => server.resetHandlers()); + afterEach(() => jest.resetAllMocks()); - beforeEach(() => { - client = new CatalogIdentityClient({ discovery }); - }); + it('passes through the correct search params', async () => { + const client = new CatalogIdentityClient({ + catalogClient: new MockedCatalogClient(), + }); - describe('findUser', () => { - const defaultServiceResponse: UserEntity[] = [ - { - apiVersion: 'backstage.io/v1alpha1', - kind: 'User', - metadata: { - name: 'Test1', - namespace: 'test1', - annotations: { - key: 'value', - }, - }, - spec: { - memberOf: ['group1'], - }, + client.findUser(token, { annotations: { key: 'value' } }); + + const getEntities = MockedCatalogClient.mock.instances[0].getEntities; + expect(getEntities).toHaveBeenCalledWith(token, { + filter: { + kind: 'user', + 'metadata.annotations.key': 'value', }, - ]; - - beforeEach(() => { - server.use( - rest.get(`${mockBaseUrl}/entities`, (_, res, ctx) => { - return res(ctx.json(defaultServiceResponse)); - }), - ); - }); - - it('should entities from correct endpoint', async () => { - const response = await client.findUser({ annotations: { key: 'value' } }); - expect(response).toEqual(defaultServiceResponse[0]); - }); - - it('builds entity search filters properly', async () => { - expect.assertions(2); - - server.use( - rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - expect(req.url.search).toBe( - '?filter=kind=user,metadata.annotations.key=value', - ); - return res(ctx.json(defaultServiceResponse)); - }), - ); - - const response = await client.findUser({ annotations: { key: 'value' } }); - - expect(response).toEqual(defaultServiceResponse[0]); - }); - - it('omits authorization header if not available', async () => { - expect.assertions(1); - - server.use( - rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - expect(req.headers.has('authorization')).toBe(false); - return res(ctx.json([])); - }), - ); - - client.findUser({ annotations: { key: 'value' } }); - }); - - it('adds authorization header if available', async () => { - expect.assertions(1); - - server.use( - rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - expect(req.headers.get('authorization')).toEqual('hello'); - return res(ctx.json([])); - }), - ); - - client.findUser( - { annotations: { key: 'value' } }, - { headers: { authorization: 'hello' } }, - ); }); + expect(getEntities).toHaveBeenCalledTimes(1); }); }); diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts index f4aa722b2d..0fad8122f4 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts @@ -14,12 +14,8 @@ * limitations under the License. */ -import fetch from 'cross-fetch'; -import { - ConflictError, - NotFoundError, - PluginEndpointDiscovery, -} from '@backstage/backend-common'; +import { ConflictError, NotFoundError } from '@backstage/backend-common'; +import { CatalogClient } from '@backstage/catalog-client'; import { UserEntity } from '@backstage/catalog-model'; type UserQuery = { @@ -30,10 +26,10 @@ type UserQuery = { * A catalog client tailored for reading out identity data from the catalog. */ export class CatalogIdentityClient { - private readonly discovery: PluginEndpointDiscovery; + private readonly catalogClient: CatalogClient; - constructor(options: { discovery: PluginEndpointDiscovery }) { - this.discovery = options.discovery; + constructor(options: { catalogClient: CatalogClient }) { + this.catalogClient = options.catalogClient; } /** @@ -42,8 +38,8 @@ export class CatalogIdentityClient { * Throws a NotFoundError or ConflictError if 0 or multiple users are found. */ async findUser( + token: string | undefined, query: UserQuery, - options?: { headers?: Record }, ): Promise { const filter: Record = { kind: 'user', @@ -51,44 +47,17 @@ export class CatalogIdentityClient { for (const [key, value] of Object.entries(query.annotations)) { filter[`metadata.annotations.${key}`] = value; } - const params: string[] = []; - const filterParts: string[] = []; - for (const [key, value] of Object.entries(filter)) { - for (const v of [value].flat()) { - filterParts.push(`${encodeURIComponent(key)}=${encodeURIComponent(v)}`); - } - } - if (filterParts.length) { - params.push(`filter=${filterParts.join(',')}`); - } - const queryPart = params.length ? `?${params.join('&')}` : ''; + const { items } = await this.catalogClient.getEntities(token, { filter }); - const url = `${await this.discovery.getBaseUrl( - 'catalog', - )}/entities${queryPart}`; - const response = await fetch(url, { - headers: { - ...options?.headers, - }, - }); - - if (!response.ok) { - const payload = await response.text(); - const message = `Request failed with ${response.status} ${response.statusText}, ${payload}`; - throw new Error(message); - } - - const entities: UserEntity[] = await response.json(); - - if (entities.length !== 1) { - if (entities.length > 1) { + if (items.length !== 1) { + if (items.length > 1) { throw new ConflictError('User lookup resulted in multiple matches'); } else { throw new NotFoundError('User not found'); } } - return entities[0] as UserEntity; + return items[0] as UserEntity; } } diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 10f3ae96f2..340ec46529 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -157,18 +157,11 @@ export class GoogleAuthProvider implements OAuthHandlers { const token = await this.tokenIssuer.issueToken({ claims: { sub: 'backstage.io/auth-backend' }, }); - const user = await this.identityClient.findUser( - { - annotations: { - 'google.com/email': profile.email, - }, + const user = await this.identityClient.findUser(token, { + annotations: { + 'google.com/email': profile.email, }, - { - headers: { - authorization: `Bearer ${token}`, - }, - }, - ); + }); return { ...response, @@ -194,7 +187,7 @@ export const createGoogleProvider: AuthProviderFactory = ({ config, logger, tokenIssuer, - discovery, + catalogClient, }) => OAuthEnvironmentHandler.mapConfig(config, envConfig => { const clientId = envConfig.getString('clientId'); @@ -207,7 +200,7 @@ export const createGoogleProvider: AuthProviderFactory = ({ callbackUrl, logger, tokenIssuer, - identityClient: new CatalogIdentityClient({ discovery }), + identityClient: new CatalogIdentityClient({ catalogClient }), }); return OAuthAdapter.fromConfig(globalConfig, provider, { diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 8339258654..1ab90ac522 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -15,6 +15,7 @@ */ import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { CatalogClient } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; import express from 'express'; import { Logger } from 'winston'; @@ -132,6 +133,7 @@ export type AuthProviderFactoryOptions = { tokenIssuer: TokenIssuer; discovery: PluginEndpointDiscovery; identityResolver?: ExperimentalIdentityResolver; + catalogClient: CatalogClient; }; export type AuthProviderFactory = ( diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 742a269b73..859c2a4846 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -27,6 +27,7 @@ import { PluginDatabaseManager, PluginEndpointDiscovery, } from '@backstage/backend-common'; +import { CatalogClient } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; import { createOidcRouter, DatabaseKeyStore, TokenFactory } from '../identity'; import session from 'express-session'; @@ -65,6 +66,7 @@ export async function createRouter({ keyDurationSeconds, logger: logger.child({ component: 'token-factory' }), }); + const catalogClient = new CatalogClient({ discoveryApi: discovery }); const secret = config.getOptionalString('auth.session.secret'); if (secret) { @@ -101,6 +103,7 @@ export async function createRouter({ logger, tokenIssuer, discovery, + catalogClient, }); const r = Router(); From 214ca40d07a0b4cc2c842a7cef5bb4d371aa6487 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Tue, 19 Jan 2021 19:46:36 +0100 Subject: [PATCH 06/34] CatalogEntityClient uses CatalogClient --- packages/backend/package.json | 1 + packages/backend/src/plugins/scaffolder.ts | 4 +- plugins/scaffolder-backend/package.json | 1 + .../src/lib/catalog/CatalogEntityClient.ts | 43 +++++-------------- .../scaffolder-backend/src/service/router.ts | 13 +++--- 5 files changed, 23 insertions(+), 39 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index aa00407952..8d215d5137 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -28,6 +28,7 @@ }, "dependencies": { "@backstage/backend-common": "^0.5.1", + "@backstage/catalog-client": "^0.3.5", "@backstage/catalog-model": "^0.7.0", "@backstage/config": "^0.1.2", "@backstage/plugin-app-backend": "^0.3.5", diff --git a/packages/backend/src/plugins/scaffolder.ts b/packages/backend/src/plugins/scaffolder.ts index 4e2257a46c..bd3616116b 100644 --- a/packages/backend/src/plugins/scaffolder.ts +++ b/packages/backend/src/plugins/scaffolder.ts @@ -24,6 +24,7 @@ import { CatalogEntityClient, } from '@backstage/plugin-scaffolder-backend'; import { SingleHostDiscovery } from '@backstage/backend-common'; +import { CatalogClient } from '@backstage/catalog-client'; import type { PluginEnvironment } from '../types'; import Docker from 'dockerode'; @@ -44,7 +45,8 @@ export default async function createPlugin({ const dockerClient = new Docker(); const discovery = SingleHostDiscovery.fromConfig(config); - const entityClient = new CatalogEntityClient({ discovery }); + const catalogClient = new CatalogClient({ discoveryApi: discovery }); + const entityClient = new CatalogEntityClient({ catalogClient }); return await createRouter({ preparers, diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 7af9f567e3..c9a439bea3 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -30,6 +30,7 @@ }, "dependencies": { "@backstage/backend-common": "^0.5.1", + "@backstage/catalog-client": "^0.3.4", "@backstage/catalog-model": "^0.7.0", "@backstage/config": "^0.1.2", "@backstage/integration": "^0.3.1", diff --git a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts index c5e20202c5..6c2909d360 100644 --- a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts +++ b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts @@ -14,22 +14,18 @@ * limitations under the License. */ -import fetch from 'cross-fetch'; import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { - ConflictError, - NotFoundError, - PluginEndpointDiscovery, -} from '@backstage/backend-common'; +import { CatalogClient } from '@backstage/catalog-client'; +import { ConflictError, NotFoundError } from '@backstage/backend-common'; /** * A catalog client tailored for reading out entity data from the catalog. */ export class CatalogEntityClient { - private readonly discovery: PluginEndpointDiscovery; + private readonly catalogClient: CatalogClient; - constructor(options: { discovery: PluginEndpointDiscovery }) { - this.discovery = options.discovery; + constructor(options: { catalogClient: CatalogClient }) { + this.catalogClient = options.catalogClient; } /** @@ -38,32 +34,15 @@ export class CatalogEntityClient { * Throws a NotFoundError or ConflictError if 0 or multiple templates are found. */ async findTemplate( + token: string | undefined, templateName: string, - options?: { headers?: Record }, ): Promise { - const conditions = [ - 'kind=template', - `metadata.name=${encodeURIComponent(templateName)}`, - ]; - - const baseUrl = await this.discovery.getBaseUrl('catalog'); - const response = await fetch( - `${baseUrl}/entities?filter=${conditions.join(',')}`, - { - headers: { - ...options?.headers, - }, + const { items: templates } = (await this.catalogClient.getEntities(token, { + filter: { + kind: 'template', + 'metadata.name': templateName, }, - ); - - if (!response.ok) { - const text = await response.text(); - throw new Error( - `Request failed with ${response.status} ${response.statusText}, ${text}`, - ); - } - - const templates: TemplateEntityV1alpha1[] = await response.json(); + })) as { items: TemplateEntityV1alpha1[] }; if (templates.length !== 1) { if (templates.length > 1) { diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 070b7cffc7..606aa05bc1 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -15,6 +15,7 @@ */ import { Config } from '@backstage/config'; +import { BackstageIdentity } from '@backstage/core'; import Docker from 'dockerode'; import express from 'express'; import { resolve as resolvePath } from 'path'; @@ -97,12 +98,12 @@ export async function createRouter( }, }; - // Forward authorization header from client - const template = await entityClient.findTemplate(templateName, { - headers: req.headers.authorization - ? { authorization: req.headers.authorization } - : {}, - }); + // Forward authorization from client + const user = req.user as BackstageIdentity; + const template = await entityClient.findTemplate( + user?.idToken, + templateName, + ); const validationResult: ValidatorResult = validate( values, From 82cac24d19e384d7795e31e9c3324d31f468956d Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Tue, 19 Jan 2021 20:51:54 +0100 Subject: [PATCH 07/34] Fix client reference --- plugins/catalog/src/CatalogClientWrapper.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/catalog/src/CatalogClientWrapper.test.ts b/plugins/catalog/src/CatalogClientWrapper.test.ts index 8a5c58194a..e2aea16526 100644 --- a/plugins/catalog/src/CatalogClientWrapper.test.ts +++ b/plugins/catalog/src/CatalogClientWrapper.test.ts @@ -18,7 +18,7 @@ import { CatalogClient } from '@backstage/catalog-client'; import { DiscoveryApi, IdentityApi } from '@backstage/core'; import { CatalogClientWrapper } from './CatalogClientWrapper'; -jest.mock('./CatalogClient'); +jest.mock('@backstage/catalog-client'); const MockedCatalogClient = CatalogClient as jest.Mock; const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base'; From 0e40a5915d828dd73294f80aa5ef3e3d5c51c361 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Tue, 19 Jan 2021 22:56:43 +0100 Subject: [PATCH 08/34] Fix client reference --- .../auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts index ffd118884e..e1a45a4465 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts @@ -17,7 +17,7 @@ import { CatalogClient } from '@backstage/catalog-client'; import { CatalogIdentityClient } from './CatalogIdentityClient'; -jest.mock('./CatalogClient'); +jest.mock('@backstage/catalog-client'); const MockedCatalogClient = CatalogClient as jest.Mock; describe('CatalogIdentityClient', () => { From ca51082b3ff0e0744d3092e94bdeffa1c2177b3d Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 20 Jan 2021 01:11:12 +0100 Subject: [PATCH 09/34] Fix CatalogClient reference in create-app too --- .../default-app/packages/backend/src/plugins/scaffolder.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts index c8bd3e5012..72f0f5099a 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts @@ -8,6 +8,7 @@ import { CatalogEntityClient, } from '@backstage/plugin-scaffolder-backend'; import { SingleHostDiscovery } from '@backstage/backend-common'; +import { CatalogClient } from '@backstage/catalog-client'; import type { PluginEnvironment } from '../types'; import Docker from 'dockerode'; @@ -28,7 +29,8 @@ export default async function createPlugin({ const dockerClient = new Docker(); const discovery = SingleHostDiscovery.fromConfig(config); - const entityClient = new CatalogEntityClient({ discovery }); + const catalogClient = new CatalogClient({ discoveryApi: discovery }); + const entityClient = new CatalogEntityClient({ catalogClient }); return await createRouter({ preparers, From fdbea3ae20935a50c5075ef5222894cd01262ca7 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 20 Jan 2021 07:50:30 +0100 Subject: [PATCH 10/34] Move catalog-client dep to create-app --- packages/backend/package.json | 1 - .../templates/default-app/packages/backend/package.json.hbs | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index 8d215d5137..aa00407952 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -28,7 +28,6 @@ }, "dependencies": { "@backstage/backend-common": "^0.5.1", - "@backstage/catalog-client": "^0.3.5", "@backstage/catalog-model": "^0.7.0", "@backstage/config": "^0.1.2", "@backstage/plugin-app-backend": "^0.3.5", diff --git a/packages/create-app/templates/default-app/packages/backend/package.json.hbs b/packages/create-app/templates/default-app/packages/backend/package.json.hbs index 3fed72f07b..81f73768ca 100644 --- a/packages/create-app/templates/default-app/packages/backend/package.json.hbs +++ b/packages/create-app/templates/default-app/packages/backend/package.json.hbs @@ -19,6 +19,7 @@ "dependencies": { "app": "0.0.0", "@backstage/backend-common": "^{{version '@backstage/backend-common'}}", + "@backstage/catalog-client": "^{{version '@backstage/catalog-client'}}", "@backstage/catalog-model": "^{{version '@backstage/catalog-model'}}", "@backstage/config": "^{{version '@backstage/config'}}", "@backstage/plugin-app-backend": "^{{version '@backstage/plugin-app-backend'}}", From 09b97cbe84931183423e3cd6abbb1e03ef17894f Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 20 Jan 2021 08:03:38 +0100 Subject: [PATCH 11/34] Add catalog-client dep again.. --- packages/backend/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/backend/package.json b/packages/backend/package.json index aa00407952..8d215d5137 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -28,6 +28,7 @@ }, "dependencies": { "@backstage/backend-common": "^0.5.1", + "@backstage/catalog-client": "^0.3.5", "@backstage/catalog-model": "^0.7.0", "@backstage/config": "^0.1.2", "@backstage/plugin-app-backend": "^0.3.5", From 05a6c4c39481949d36d0a7a1a22f4973a1208755 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Fri, 22 Jan 2021 22:16:54 +0100 Subject: [PATCH 12/34] Use RequestOptions --- packages/catalog-client/src/CatalogClient.ts | 50 ++++++++++++-------- packages/catalog-client/src/types.ts | 4 ++ 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index c4f8c3d2b0..657841c000 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -26,6 +26,7 @@ import { AddLocationResponse, CatalogEntitiesRequest, CatalogListResponse, + CatalogRequestOptions, DiscoveryApi, } from './types'; @@ -37,15 +38,15 @@ export class CatalogClient { } async getLocationById( - token: string | undefined, id: String, + options?: CatalogRequestOptions, ): Promise { - return await this.getOptional(token, `/locations/${id}`); + return await this.getOptional(`/locations/${id}`, options); } async getEntities( - token: string | undefined, request?: CatalogEntitiesRequest, + options?: CatalogRequestOptions, ): Promise> { const { filter = {}, fields = [] } = request ?? {}; const params: string[] = []; @@ -66,36 +67,39 @@ export class CatalogClient { const query = params.length ? `?${params.join('&')}` : ''; const entities: Entity[] = await this.getRequired( - token, `/entities${query}`, + options, ); return { items: entities }; } async getEntityByName( - token: string | undefined, compoundName: EntityName, + options?: CatalogRequestOptions, ): Promise { const { kind, namespace = 'default', name } = compoundName; return this.getOptional( - token, `/entities/by-name/${kind}/${namespace}/${name}`, + options, ); } async addLocation( - token: string | undefined, { type = 'url', target, dryRun }: AddLocationRequest, + options?: CatalogRequestOptions, ): Promise { + const headers = { + 'Content-Type': 'application/json', + } as { [header: string]: string }; + if (options?.token) { + headers.authorization = `Bearer ${options.token}`; + } const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/locations${ dryRun ? '?dryRun=true' : '' }`, { - headers: { - authorization: `Bearer ${token}`, - 'Content-Type': 'application/json', - }, + headers, method: 'POST', body: JSON.stringify({ type, target }), }, @@ -123,13 +127,13 @@ export class CatalogClient { } async getLocationByEntity( - token: string | undefined, entity: Entity, + options?: CatalogRequestOptions, ): Promise { const locationCompound = entity.metadata.annotations?.[LOCATION_ANNOTATION]; const all: { data: Location }[] = await this.getRequired( - token, '/locations', + options, ); return all .map(r => r.data) @@ -137,15 +141,15 @@ export class CatalogClient { } async removeEntityByUid( - token: string | undefined, uid: string, + options?: CatalogRequestOptions, ): Promise { const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/entities/by-uid/${uid}`, { - headers: { - authorization: `Bearer ${token}`, - }, + headers: options?.token + ? { authorization: `Bearer ${options.token}` } + : {}, method: 'DELETE', }, ); @@ -163,12 +167,14 @@ export class CatalogClient { // private async getRequired( - token: string | undefined, path: string, + options?: CatalogRequestOptions, ): Promise { const url = `${await this.discoveryApi.getBaseUrl('catalog')}${path}`; const response = await fetch(url, { - headers: token ? { authorization: `Bearer ${token}` } : {}, + headers: options?.token + ? { authorization: `Bearer ${options.token}` } + : {}, }); if (!response.ok) { @@ -181,12 +187,14 @@ export class CatalogClient { } private async getOptional( - token: string | undefined, path: string, + options?: CatalogRequestOptions, ): Promise { const url = `${await this.discoveryApi.getBaseUrl('catalog')}${path}`; const response = await fetch(url, { - headers: token ? { authorization: `Bearer ${token}` } : {}, + headers: options?.token + ? { authorization: `Bearer ${options.token}` } + : {}, }); if (!response.ok) { diff --git a/packages/catalog-client/src/types.ts b/packages/catalog-client/src/types.ts index e72317d444..66fbedd38e 100644 --- a/packages/catalog-client/src/types.ts +++ b/packages/catalog-client/src/types.ts @@ -25,6 +25,10 @@ export type CatalogListResponse = { items: T[]; }; +export type CatalogRequestOptions = { + token: string | undefined; +}; + export interface CatalogApi { getLocationById(id: String): Promise; getEntityByName(name: EntityName): Promise; From b3e9b08fd7c248ec2f2593e5a74033ed2fc052d0 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Fri, 22 Jan 2021 23:44:26 +0100 Subject: [PATCH 13/34] tsc --- .../catalog-client/src/CatalogClient.test.ts | 26 ++++++----- .../src/lib/catalog/CatalogIdentityClient.ts | 5 ++- .../src/providers/aws-alb/provider.test.ts | 24 +++++----- .../src/providers/aws-alb/provider.ts | 10 ++--- plugins/auth-backend/src/providers/types.ts | 2 +- .../src/api/CatalogImportClient.test.ts | 15 +++++++ .../components/ImportComponentForm.test.tsx | 16 +++++++ .../components/ImportComponentPage.test.tsx | 16 +++++++ plugins/catalog/src/CatalogClientWrapper.ts | 12 ++--- .../src/lib/catalog/CatalogEntityClient.ts | 13 +++--- yarn.lock | 45 +++++++++++++++++++ 11 files changed, 142 insertions(+), 42 deletions(-) diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index 20cc35f423..d9bfb1eebd 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -72,7 +72,7 @@ describe('CatalogClient', () => { }); it('should entities from correct endpoint', async () => { - const response = await client.getEntities(token); + const response = await client.getEntities({}, { token }); expect(response).toEqual(defaultResponse); }); @@ -86,13 +86,16 @@ describe('CatalogClient', () => { }), ); - const response = await client.getEntities(token, { - filter: { - a: '1', - b: ['2', '3'], - รถ: '=', + const response = await client.getEntities( + { + filter: { + a: '1', + b: ['2', '3'], + รถ: '=', + }, }, - }); + { token }, + ); expect(response.items).toEqual([]); }); @@ -107,9 +110,12 @@ describe('CatalogClient', () => { }), ); - const response = await client.getEntities(token, { - fields: ['a.b', 'รถ'], - }); + const response = await client.getEntities( + { + fields: ['a.b', 'รถ'], + }, + { token }, + ); expect(response.items).toEqual([]); }); diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts index 0fad8122f4..9967a7be17 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts @@ -48,7 +48,10 @@ export class CatalogIdentityClient { filter[`metadata.annotations.${key}`] = value; } - const { items } = await this.catalogClient.getEntities(token, { filter }); + const { items } = await this.catalogClient.getEntities( + { filter }, + { token }, + ); if (items.length !== 1) { if (items.length > 1) { diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts index d05b6bbfaf..536ba71f6b 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts @@ -14,6 +14,7 @@ * limitations under the License. */ import { getVoidLogger } from '@backstage/backend-common'; +import { CatalogClient } from '@backstage/catalog-client'; import express from 'express'; import { JWT } from 'jose'; @@ -43,6 +44,9 @@ jest.mock('cross-fetch', () => ({ }, })); +jest.mock('@backstage/catalog-client'); +const MockedCatalogClient = CatalogClient as jest.Mock; + const identityResolutionCallbackMock = async (): Promise> => { return { backstageIdentity: { @@ -67,15 +71,7 @@ beforeEach(() => { }); describe('AwsALBAuthProvider', () => { - const catalogApi = { - /* eslint-disable-next-line @typescript-eslint/no-unused-vars */ - addLocation: jest.fn(), - getEntities: jest.fn(), - getLocationByEntity: jest.fn(), - getLocationById: jest.fn(), - removeEntityByUid: jest.fn(), - getEntityByName: jest.fn(), - }; + const catalogClient = new MockedCatalogClient(); const mockResponseSend = jest.fn(); const mockRequest = ({ @@ -95,7 +91,7 @@ describe('AwsALBAuthProvider', () => { describe('should transform to type OAuthResponse', () => { it('when JWT is valid and identity is resolved successfully', async () => { - const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogClient, { region: 'us-west-2', identityResolutionCallback: identityResolutionCallbackMock, issuer: 'foo', @@ -121,7 +117,7 @@ describe('AwsALBAuthProvider', () => { }); describe('should fail when', () => { it('JWT is missing', async () => { - const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogClient, { region: 'us-west-2', identityResolutionCallback: identityResolutionCallbackMock, issuer: 'foo', @@ -133,7 +129,7 @@ describe('AwsALBAuthProvider', () => { }); it('JWT is invalid', async () => { - const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogClient, { region: 'us-west-2', identityResolutionCallback: identityResolutionCallbackMock, issuer: 'foo', @@ -149,7 +145,7 @@ describe('AwsALBAuthProvider', () => { }); it('issuer is invalid', async () => { - const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogClient, { region: 'us-west-2', identityResolutionCallback: identityResolutionCallbackMock, issuer: 'foobar', @@ -163,7 +159,7 @@ describe('AwsALBAuthProvider', () => { }); it('identity resolution callback rejects', async () => { - const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogClient, { region: 'us-west-2', identityResolutionCallback: identityResolutionCallbackRejectedMock, issuer: 'foo', diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.ts b/plugins/auth-backend/src/providers/aws-alb/provider.ts index 61ea10947e..f8b61c15fd 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.ts @@ -25,7 +25,7 @@ import { KeyObject } from 'crypto'; import { Logger } from 'winston'; import NodeCache from 'node-cache'; import { JWT } from 'jose'; -import { CatalogApi } from '@backstage/catalog-client'; +import { CatalogClient } from '@backstage/catalog-client'; const ALB_JWT_HEADER = 'x-amzn-oidc-data'; /** @@ -44,13 +44,13 @@ export const getJWTHeaders = (input: string) => { export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { private logger: Logger; - private readonly catalogClient: CatalogApi; + private readonly catalogClient: CatalogClient; private options: AwsAlbAuthProviderOptions; private readonly keyCache: NodeCache; constructor( logger: Logger, - catalogClient: CatalogApi, + catalogClient: CatalogClient, options: AwsAlbAuthProviderOptions, ) { this.logger = logger; @@ -108,14 +108,14 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { export const createAwsAlbProvider = ({ logger, - catalogApi, + catalogClient, config, identityResolver, }: AuthProviderFactoryOptions) => { const region = config.getString('region'); const issuer = config.getOptionalString('iss'); if (identityResolver !== undefined) { - return new AwsAlbAuthProvider(logger, catalogApi, { + return new AwsAlbAuthProvider(logger, catalogClient, { region, issuer, identityResolutionCallback: identityResolver, diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 1ab90ac522..b2f488546b 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -122,7 +122,7 @@ export type ExperimentalIdentityResolver = ( * An object containing information specific to the auth provider. */ payload: object, - catalogApi: CatalogApi, + catalogClient: CatalogClient, ) => Promise>; export type AuthProviderFactoryOptions = { diff --git a/plugins/catalog-import/src/api/CatalogImportClient.test.ts b/plugins/catalog-import/src/api/CatalogImportClient.test.ts index 0e0cf4b323..321e4ba108 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.test.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.test.ts @@ -45,10 +45,25 @@ jest.mock('@octokit/rest', () => ({ })); describe('CatalogImportClient', () => { + const identityApi = { + getUserId: () => { + return 'user'; + }, + getProfile: () => { + return {}; + }, + getIdToken: () => { + return Promise.resolve('token'); + }, + signOut: () => { + return Promise.resolve(); + }, + }; describe('checkForExistingCatalogInfo', () => { const cic = new CatalogImportClient({ discoveryApi: { getBaseUrl: () => Promise.resolve('base') }, githubAuthApi: { getAccessToken: (_, __) => Promise.resolve('token') }, + identityApi, configApi: {} as any, }); it('should return the closest-to-root catalog-info from multiple responses', async () => { diff --git a/plugins/catalog-import/src/components/ImportComponentForm.test.tsx b/plugins/catalog-import/src/components/ImportComponentForm.test.tsx index 38b1065325..9aef548856 100644 --- a/plugins/catalog-import/src/components/ImportComponentForm.test.tsx +++ b/plugins/catalog-import/src/components/ImportComponentForm.test.tsx @@ -35,6 +35,21 @@ describe('', () => { error$: jest.fn(), }; + const identityApi = { + getUserId: () => { + return 'user'; + }, + getProfile: () => { + return {}; + }, + getIdToken: () => { + return Promise.resolve('token'); + }, + signOut: () => { + return Promise.resolve(); + }, + }; + beforeEach(() => { apis = ApiRegistry.from([ [catalogApiRef, new CatalogClient({ discoveryApi: {} as DiscoveryApi })], @@ -45,6 +60,7 @@ describe('', () => { githubAuthApi: { getAccessToken: (_, __) => Promise.resolve('token'), }, + identityApi, configApi: {} as any, }), ], diff --git a/plugins/catalog-import/src/components/ImportComponentPage.test.tsx b/plugins/catalog-import/src/components/ImportComponentPage.test.tsx index f6ef07e1b2..90974e671d 100644 --- a/plugins/catalog-import/src/components/ImportComponentPage.test.tsx +++ b/plugins/catalog-import/src/components/ImportComponentPage.test.tsx @@ -70,6 +70,21 @@ describe('', () => { const server = setupServer(); msw.setupDefaultHandlers(server); + const identityApi = { + getUserId: () => { + return 'user'; + }, + getProfile: () => { + return {}; + }, + getIdToken: () => { + return Promise.resolve('token'); + }, + signOut: () => { + return Promise.resolve(); + }, + }; + beforeEach(() => { server.use( rest.post('https://backend.localhost/locations', (_, res, ctx) => { @@ -112,6 +127,7 @@ describe('', () => { githubAuthApi: { getAccessToken: (_, __) => Promise.resolve('token'), }, + identityApi, configApi: {} as any, }), ], diff --git a/plugins/catalog/src/CatalogClientWrapper.ts b/plugins/catalog/src/CatalogClientWrapper.ts index fab551de48..1f06cd380b 100644 --- a/plugins/catalog/src/CatalogClientWrapper.ts +++ b/plugins/catalog/src/CatalogClientWrapper.ts @@ -36,33 +36,33 @@ export class CatalogClientWrapper implements CatalogApi { async getLocationById(id: String): Promise { const token = await this.identityApi.getIdToken(); - return await this.client.getLocationById(token, id); + return await this.client.getLocationById(id, { token }); } async getEntities( request?: CatalogEntitiesRequest, ): Promise> { const token = await this.identityApi.getIdToken(); - return await this.client.getEntities(token, request); + return await this.client.getEntities(request, { token }); } async getEntityByName(compoundName: EntityName): Promise { const token = await this.identityApi.getIdToken(); - return await this.client.getEntityByName(token, compoundName); + return await this.client.getEntityByName(compoundName, { token }); } async addLocation(request: AddLocationRequest): Promise { const token = await this.identityApi.getIdToken(); - return await this.client.addLocation(token, request); + return await this.client.addLocation(request, { token }); } async getLocationByEntity(entity: Entity): Promise { const token = await this.identityApi.getIdToken(); - return await this.client.getLocationByEntity(token, entity); + return await this.client.getLocationByEntity(entity, { token }); } async removeEntityByUid(uid: string): Promise { const token = await this.identityApi.getIdToken(); - return await this.client.removeEntityByUid(token, uid); + return await this.client.removeEntityByUid(uid, { token }); } } diff --git a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts index 6c2909d360..0440a45f66 100644 --- a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts +++ b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts @@ -37,12 +37,15 @@ export class CatalogEntityClient { token: string | undefined, templateName: string, ): Promise { - const { items: templates } = (await this.catalogClient.getEntities(token, { - filter: { - kind: 'template', - 'metadata.name': templateName, + const { items: templates } = (await this.catalogClient.getEntities( + { + filter: { + kind: 'template', + 'metadata.name': templateName, + }, }, - })) as { items: TemplateEntityV1alpha1[] }; + { token }, + )) as { items: TemplateEntityV1alpha1[] }; if (templates.length !== 1) { if (templates.length > 1) { diff --git a/yarn.lock b/yarn.lock index aac0d3735b..1e17345f48 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2645,6 +2645,46 @@ remark-gfm "^1.0.0" zen-observable "^0.8.15" +"@backstage/core@^0.4.3": + version "0.4.4" + resolved "https://registry.npmjs.org/@backstage/core/-/core-0.4.4.tgz#83d5ce5c8dd8a9803badc1dde389b5bde963e959" + integrity sha512-UEZhPBC7DAZEOCRAYQ9XcXch5JDuPn+wYNp65Ik60SSz/RFjqhq8D3P7V074cMQqWLAddVDj5dUrNeN8N1gAUQ== + dependencies: + "@backstage/config" "^0.1.2" + "@backstage/core-api" "^0.2.8" + "@backstage/theme" "^0.2.2" + "@material-ui/core" "^4.11.0" + "@material-ui/icons" "^4.9.1" + "@material-ui/lab" "4.0.0-alpha.45" + "@types/dagre" "^0.7.44" + "@types/prop-types" "^15.7.3" + "@types/react" "^16.9" + "@types/react-sparklines" "^1.7.0" + classnames "^2.2.6" + clsx "^1.1.0" + d3-selection "^2.0.0" + d3-shape "^2.0.0" + d3-zoom "^2.0.0" + dagre "^0.8.5" + immer "^7.0.9" + lodash "^4.17.15" + material-table "^1.69.1" + prop-types "^15.7.2" + qs "^6.9.4" + rc-progress "^3.0.0" + react "^16.12.0" + react-dom "^16.12.0" + react-helmet "6.1.0" + react-hook-form "^6.6.0" + react-markdown "^5.0.2" + react-router "6.0.0-beta.0" + react-router-dom "6.0.0-beta.0" + react-sparklines "^1.7.0" + react-syntax-highlighter "^13.5.1" + react-use "^15.3.3" + remark-gfm "^1.0.0" + zen-observable "^0.8.15" + "@bcoe/v8-coverage@^0.2.3": version "0.2.3" resolved "https://registry.npmjs.org/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39" @@ -15469,6 +15509,11 @@ immer@1.10.0: resolved "https://registry.npmjs.org/immer/-/immer-1.10.0.tgz#bad67605ba9c810275d91e1c2a47d4582e98286d" integrity sha512-O3sR1/opvCDGLEVcvrGTMtLac8GJ5IwZC4puPrLuRj3l7ICKvkmA0vGuU9OW8mV9WIBRnaxp5GJh9IEAaNOoYg== +immer@^7.0.9: + version "7.0.15" + resolved "https://registry.npmjs.org/immer/-/immer-7.0.15.tgz#dc3bc6db87401659d2e737c67a21b227c484a4ad" + integrity sha512-yM7jo9+hvYgvdCQdqvhCNRRio0SCXc8xDPzA25SvKWa7b1WVPjLwQs1VYU5JPXjcJPTqAa5NP5dqpORGYBQ2AA== + immer@^8.0.1: version "8.0.1" resolved "https://registry.npmjs.org/immer/-/immer-8.0.1.tgz#9c73db683e2b3975c424fb0572af5889877ae656" From 3a3cdfb1ca41be3410ccb5f5c5273a3d39e5e75c Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Sun, 24 Jan 2021 01:55:36 +0100 Subject: [PATCH 14/34] Backwards compatible scaffolder --- packages/backend/package.json | 1 - packages/backend/src/plugins/scaffolder.ts | 4 +--- .../default-app/packages/backend/package.json.hbs | 1 - .../default-app/packages/backend/src/plugins/scaffolder.ts | 4 +--- plugins/catalog/src/CatalogClientWrapper.test.ts | 4 +++- .../src/lib/catalog/CatalogEntityClient.ts | 7 +++++-- 6 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index 8d215d5137..aa00407952 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -28,7 +28,6 @@ }, "dependencies": { "@backstage/backend-common": "^0.5.1", - "@backstage/catalog-client": "^0.3.5", "@backstage/catalog-model": "^0.7.0", "@backstage/config": "^0.1.2", "@backstage/plugin-app-backend": "^0.3.5", diff --git a/packages/backend/src/plugins/scaffolder.ts b/packages/backend/src/plugins/scaffolder.ts index bd3616116b..4e2257a46c 100644 --- a/packages/backend/src/plugins/scaffolder.ts +++ b/packages/backend/src/plugins/scaffolder.ts @@ -24,7 +24,6 @@ import { CatalogEntityClient, } from '@backstage/plugin-scaffolder-backend'; import { SingleHostDiscovery } from '@backstage/backend-common'; -import { CatalogClient } from '@backstage/catalog-client'; import type { PluginEnvironment } from '../types'; import Docker from 'dockerode'; @@ -45,8 +44,7 @@ export default async function createPlugin({ const dockerClient = new Docker(); const discovery = SingleHostDiscovery.fromConfig(config); - const catalogClient = new CatalogClient({ discoveryApi: discovery }); - const entityClient = new CatalogEntityClient({ catalogClient }); + const entityClient = new CatalogEntityClient({ discovery }); return await createRouter({ preparers, diff --git a/packages/create-app/templates/default-app/packages/backend/package.json.hbs b/packages/create-app/templates/default-app/packages/backend/package.json.hbs index 81f73768ca..3fed72f07b 100644 --- a/packages/create-app/templates/default-app/packages/backend/package.json.hbs +++ b/packages/create-app/templates/default-app/packages/backend/package.json.hbs @@ -19,7 +19,6 @@ "dependencies": { "app": "0.0.0", "@backstage/backend-common": "^{{version '@backstage/backend-common'}}", - "@backstage/catalog-client": "^{{version '@backstage/catalog-client'}}", "@backstage/catalog-model": "^{{version '@backstage/catalog-model'}}", "@backstage/config": "^{{version '@backstage/config'}}", "@backstage/plugin-app-backend": "^{{version '@backstage/plugin-app-backend'}}", diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts index 72f0f5099a..c8bd3e5012 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/scaffolder.ts @@ -8,7 +8,6 @@ import { CatalogEntityClient, } from '@backstage/plugin-scaffolder-backend'; import { SingleHostDiscovery } from '@backstage/backend-common'; -import { CatalogClient } from '@backstage/catalog-client'; import type { PluginEnvironment } from '../types'; import Docker from 'dockerode'; @@ -29,8 +28,7 @@ export default async function createPlugin({ const dockerClient = new Docker(); const discovery = SingleHostDiscovery.fromConfig(config); - const catalogClient = new CatalogClient({ discoveryApi: discovery }); - const entityClient = new CatalogEntityClient({ catalogClient }); + const entityClient = new CatalogEntityClient({ discovery }); return await createRouter({ preparers, diff --git a/plugins/catalog/src/CatalogClientWrapper.test.ts b/plugins/catalog/src/CatalogClientWrapper.test.ts index e2aea16526..8ed53bddb0 100644 --- a/plugins/catalog/src/CatalogClientWrapper.test.ts +++ b/plugins/catalog/src/CatalogClientWrapper.test.ts @@ -58,7 +58,9 @@ describe('CatalogClientWrapper', () => { expect.assertions(2); await client.getEntities(); const getEntities = MockedCatalogClient.mock.instances[0].getEntities; - expect(getEntities).toHaveBeenCalledWith('fake-id-token', undefined); + expect(getEntities).toHaveBeenCalledWith(undefined, { + token: 'fake-id-token', + }); expect(getEntities).toHaveBeenCalledTimes(1); }); }); diff --git a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts index 0440a45f66..7afbdd3fb9 100644 --- a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts +++ b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts @@ -16,6 +16,7 @@ import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { CatalogClient } from '@backstage/catalog-client'; +import { DiscoveryApi } from '@backstage/core'; import { ConflictError, NotFoundError } from '@backstage/backend-common'; /** @@ -24,8 +25,10 @@ import { ConflictError, NotFoundError } from '@backstage/backend-common'; export class CatalogEntityClient { private readonly catalogClient: CatalogClient; - constructor(options: { catalogClient: CatalogClient }) { - this.catalogClient = options.catalogClient; + constructor(options: { discovery: DiscoveryApi }) { + this.catalogClient = new CatalogClient({ + discoveryApi: options.discovery, + }); } /** From 3af90e2f467c232c8b04d6e50ac4e77128a6f5b9 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Sun, 24 Jan 2021 02:12:37 +0100 Subject: [PATCH 15/34] Fix test --- .../src/lib/catalog/CatalogIdentityClient.test.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts index e1a45a4465..9137af5e83 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts @@ -33,12 +33,15 @@ describe('CatalogIdentityClient', () => { client.findUser(token, { annotations: { key: 'value' } }); const getEntities = MockedCatalogClient.mock.instances[0].getEntities; - expect(getEntities).toHaveBeenCalledWith(token, { - filter: { - kind: 'user', - 'metadata.annotations.key': 'value', + expect(getEntities).toHaveBeenCalledWith( + { + filter: { + kind: 'user', + 'metadata.annotations.key': 'value', + }, }, - }); + { token }, + ); expect(getEntities).toHaveBeenCalledTimes(1); }); }); From 7d7fe3d7881ebda31ccd354069afc362e9c39996 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 27 Jan 2021 00:10:19 +0100 Subject: [PATCH 16/34] Avoid importing core --- .../src/lib/catalog/CatalogEntityClient.ts | 9 ++++++--- plugins/scaffolder-backend/src/service/router.ts | 6 +++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts index 7afbdd3fb9..56b82760bd 100644 --- a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts +++ b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts @@ -16,8 +16,11 @@ import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; import { CatalogClient } from '@backstage/catalog-client'; -import { DiscoveryApi } from '@backstage/core'; -import { ConflictError, NotFoundError } from '@backstage/backend-common'; +import { + ConflictError, + NotFoundError, + PluginEndpointDiscovery, +} from '@backstage/backend-common'; /** * A catalog client tailored for reading out entity data from the catalog. @@ -25,7 +28,7 @@ import { ConflictError, NotFoundError } from '@backstage/backend-common'; export class CatalogEntityClient { private readonly catalogClient: CatalogClient; - constructor(options: { discovery: DiscoveryApi }) { + constructor(options: { discovery: PluginEndpointDiscovery }) { this.catalogClient = new CatalogClient({ discoveryApi: options.discovery, }); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 606aa05bc1..25b8c6e6f9 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -15,7 +15,6 @@ */ import { Config } from '@backstage/config'; -import { BackstageIdentity } from '@backstage/core'; import Docker from 'dockerode'; import express from 'express'; import { resolve as resolvePath } from 'path'; @@ -46,6 +45,11 @@ export interface RouterOptions { entityClient: CatalogEntityClient; } +// Avoid import of @backstage/core +type BackstageIdentity = { + idToken: string; +}; + export async function createRouter( options: RouterOptions, ): Promise { From 97d13a88896e63e831303a954f688fe512685ed0 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 27 Jan 2021 02:30:33 +0100 Subject: [PATCH 17/34] Use request options --- .../src/lib/catalog/CatalogIdentityClient.test.ts | 2 +- .../src/lib/catalog/CatalogIdentityClient.ts | 4 +++- plugins/auth-backend/src/providers/google/provider.ts | 11 +++++++---- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts index 9137af5e83..7eb50c92bf 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts @@ -30,7 +30,7 @@ describe('CatalogIdentityClient', () => { catalogClient: new MockedCatalogClient(), }); - client.findUser(token, { annotations: { key: 'value' } }); + client.findUser({ annotations: { key: 'value' } }, { token }); const getEntities = MockedCatalogClient.mock.instances[0].getEntities; expect(getEntities).toHaveBeenCalledWith( diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts index 9967a7be17..6c052321a1 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts @@ -17,6 +17,7 @@ import { ConflictError, NotFoundError } from '@backstage/backend-common'; import { CatalogClient } from '@backstage/catalog-client'; import { UserEntity } from '@backstage/catalog-model'; +import { CatalogIdentityRequestOptions } from './types'; type UserQuery = { annotations: Record; @@ -38,9 +39,10 @@ export class CatalogIdentityClient { * Throws a NotFoundError or ConflictError if 0 or multiple users are found. */ async findUser( - token: string | undefined, query: UserQuery, + options?: CatalogIdentityRequestOptions, ): Promise { + const token = options?.token; const filter: Record = { kind: 'user', }; diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 340ec46529..15c2172e6e 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -157,11 +157,14 @@ export class GoogleAuthProvider implements OAuthHandlers { const token = await this.tokenIssuer.issueToken({ claims: { sub: 'backstage.io/auth-backend' }, }); - const user = await this.identityClient.findUser(token, { - annotations: { - 'google.com/email': profile.email, + const user = await this.identityClient.findUser( + { + annotations: { + 'google.com/email': profile.email, + }, }, - }); + { token }, + ); return { ...response, From 1476950a127ce7bdde25e4baa560f3287c02c5f6 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 27 Jan 2021 02:30:59 +0100 Subject: [PATCH 18/34] Increase code coverage --- .../catalog-client/src/CatalogClient.test.ts | 47 +++++++++ plugins/auth-backend/src/lib/catalog/types.ts | 19 ++++ .../catalog/src/CatalogClientWrapper.test.ts | 97 ++++++++++++++++++- 3 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 plugins/auth-backend/src/lib/catalog/types.ts diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index d9bfb1eebd..19da45e4ec 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -120,4 +120,51 @@ describe('CatalogClient', () => { expect(response.items).toEqual([]); }); }); + + describe('getLocationById', () => { + const defaultResponse = { + data: { + id: '42', + }, + }; + + beforeEach(() => { + server.use( + rest.get(`${mockBaseUrl}/locations/42`, (_, res, ctx) => { + return res(ctx.json(defaultResponse)); + }), + ); + }); + + it('should locations from correct endpoint', async () => { + const response = await client.getLocationById('42', { token }); + expect(response).toEqual(defaultResponse); + }); + + it('forwards authorization token', async () => { + expect.assertions(1); + + server.use( + rest.get(`${mockBaseUrl}/locations/42`, (req, res, ctx) => { + expect(req.headers.get('authorization')).toBe(`Bearer ${token}`); + return res(ctx.json(defaultResponse)); + }), + ); + + await client.getLocationById('42', { token }); + }); + + it('skips authorization header if token is omitted', async () => { + expect.assertions(1); + + server.use( + rest.get(`${mockBaseUrl}/locations/42`, (req, res, ctx) => { + expect(req.headers.get('authorization')).toBeNull(); + return res(ctx.json(defaultResponse)); + }), + ); + + await client.getLocationById('42'); + }); + }); }); diff --git a/plugins/auth-backend/src/lib/catalog/types.ts b/plugins/auth-backend/src/lib/catalog/types.ts new file mode 100644 index 0000000000..389cc14bc5 --- /dev/null +++ b/plugins/auth-backend/src/lib/catalog/types.ts @@ -0,0 +1,19 @@ +/* + * 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. + */ + +export type CatalogIdentityRequestOptions = { + token: string | undefined; +}; diff --git a/plugins/catalog/src/CatalogClientWrapper.test.ts b/plugins/catalog/src/CatalogClientWrapper.test.ts index 8ed53bddb0..9d6f8b5c57 100644 --- a/plugins/catalog/src/CatalogClientWrapper.test.ts +++ b/plugins/catalog/src/CatalogClientWrapper.test.ts @@ -41,9 +41,24 @@ const identityApi: IdentityApi = { return Promise.resolve(); }, }; +const guestIdentityApi: IdentityApi = { + getUserId() { + return 'guest'; + }, + getProfile() { + return {}; + }, + async getIdToken() { + return Promise.resolve(undefined); + }, + async signOut() { + return Promise.resolve(); + }, +}; describe('CatalogClientWrapper', () => { - let client: CatalogClientWrapper; + let client; + let guestClient; beforeEach(() => { MockedCatalogClient.mockClear(); @@ -51,6 +66,10 @@ describe('CatalogClientWrapper', () => { client: new MockedCatalogClient({ discoveryApi }), identityApi, }); + guestClient = new CatalogClientWrapper({ + client: new MockedCatalogClient({ discoveryApi }), + identityApi: guestIdentityApi, + }); }); describe('getEntities', () => { @@ -64,4 +83,80 @@ describe('CatalogClientWrapper', () => { expect(getEntities).toHaveBeenCalledTimes(1); }); }); + + describe('getLocationById', () => { + it('omits authorization token when guest', async () => { + expect.assertions(2); + await guestClient.getLocationById('42'); + const getLocationById = + MockedCatalogClient.mock.instances[0].getLocationById; + expect(getLocationById).toHaveBeenCalledWith('42', {}); + expect(getLocationById).toHaveBeenCalledTimes(1); + }); + }); + + describe('getEntityByName', () => { + const name = { + kind: 'kind', + namespace: 'namespace', + name: 'name', + }; + it('injects authorization token', async () => { + expect.assertions(2); + await client.getEntityByName(name); + const getEntityByName = + MockedCatalogClient.mock.instances[0].getEntityByName; + expect(getEntityByName).toHaveBeenCalledWith(name, { + token: 'fake-id-token', + }); + expect(getEntityByName).toHaveBeenCalledTimes(1); + }); + }); + + describe('addLocation', () => { + const location = { target: 'target' }; + it('injects authorization token', async () => { + expect.assertions(2); + await client.addLocation(location); + const addLocation = MockedCatalogClient.mock.instances[0].addLocation; + expect(addLocation).toHaveBeenCalledWith(location, { + token: 'fake-id-token', + }); + expect(addLocation).toHaveBeenCalledTimes(1); + }); + }); + + describe('getLocationByEntity', () => { + const entity = { + apiVersion: 'apiVersion', + kind: 'kind', + metadata: { + name: 'name', + }, + }; + it('injects authorization token', async () => { + expect.assertions(2); + await client.getLocationByEntity(entity); + const getLocationByEntity = + MockedCatalogClient.mock.instances[0].getLocationByEntity; + expect(getLocationByEntity).toHaveBeenCalledWith(entity, { + token: 'fake-id-token', + }); + expect(getLocationByEntity).toHaveBeenCalledTimes(1); + }); + }); + + describe('removeEntityByUid', () => { + it('injects authorization token', async () => { + const uid = 'uid'; + expect.assertions(2); + await client.removeEntityByUid(uid); + const removeEntityByUid = + MockedCatalogClient.mock.instances[0].removeEntityByUid; + expect(removeEntityByUid).toHaveBeenCalledWith(uid, { + token: 'fake-id-token', + }); + expect(removeEntityByUid).toHaveBeenCalledTimes(1); + }); + }); }); From cf074558429f7b36f4df319f0af47401e92850ec Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 27 Jan 2021 02:48:04 +0100 Subject: [PATCH 19/34] Type those variables --- plugins/catalog/src/CatalogClientWrapper.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/catalog/src/CatalogClientWrapper.test.ts b/plugins/catalog/src/CatalogClientWrapper.test.ts index 9d6f8b5c57..de60ca8dd2 100644 --- a/plugins/catalog/src/CatalogClientWrapper.test.ts +++ b/plugins/catalog/src/CatalogClientWrapper.test.ts @@ -57,8 +57,8 @@ const guestIdentityApi: IdentityApi = { }; describe('CatalogClientWrapper', () => { - let client; - let guestClient; + let client: CatalogClientWrapper; + let guestClient: CatalogClientWrapper; beforeEach(() => { MockedCatalogClient.mockClear(); From 2688210621507d50dfb1f4ed33251dc112a50e3c Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 27 Jan 2021 09:52:26 +0100 Subject: [PATCH 20/34] use the second mock --- plugins/catalog/src/CatalogClientWrapper.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/catalog/src/CatalogClientWrapper.test.ts b/plugins/catalog/src/CatalogClientWrapper.test.ts index de60ca8dd2..0306d10b5d 100644 --- a/plugins/catalog/src/CatalogClientWrapper.test.ts +++ b/plugins/catalog/src/CatalogClientWrapper.test.ts @@ -89,7 +89,7 @@ describe('CatalogClientWrapper', () => { expect.assertions(2); await guestClient.getLocationById('42'); const getLocationById = - MockedCatalogClient.mock.instances[0].getLocationById; + MockedCatalogClient.mock.instances[1].getLocationById; expect(getLocationById).toHaveBeenCalledWith('42', {}); expect(getLocationById).toHaveBeenCalledTimes(1); }); From cee1933d77eb28ad87b2a3417ffee24d60eb103a Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 27 Jan 2021 13:21:19 +0100 Subject: [PATCH 21/34] Trying to make tests work on Github too --- .../catalog/src/CatalogClientWrapper.test.ts | 39 ++++++++++++------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/plugins/catalog/src/CatalogClientWrapper.test.ts b/plugins/catalog/src/CatalogClientWrapper.test.ts index 0306d10b5d..d95d274ee9 100644 --- a/plugins/catalog/src/CatalogClientWrapper.test.ts +++ b/plugins/catalog/src/CatalogClientWrapper.test.ts @@ -57,24 +57,17 @@ const guestIdentityApi: IdentityApi = { }; describe('CatalogClientWrapper', () => { - let client: CatalogClientWrapper; - let guestClient: CatalogClientWrapper; - beforeEach(() => { MockedCatalogClient.mockClear(); - client = new CatalogClientWrapper({ - client: new MockedCatalogClient({ discoveryApi }), - identityApi, - }); - guestClient = new CatalogClientWrapper({ - client: new MockedCatalogClient({ discoveryApi }), - identityApi: guestIdentityApi, - }); }); describe('getEntities', () => { it('injects authorization token', async () => { expect.assertions(2); + const client = new CatalogClientWrapper({ + client: new MockedCatalogClient({ discoveryApi }), + identityApi, + }); await client.getEntities(); const getEntities = MockedCatalogClient.mock.instances[0].getEntities; expect(getEntities).toHaveBeenCalledWith(undefined, { @@ -87,9 +80,13 @@ describe('CatalogClientWrapper', () => { describe('getLocationById', () => { it('omits authorization token when guest', async () => { expect.assertions(2); - await guestClient.getLocationById('42'); + const client = new CatalogClientWrapper({ + client: new MockedCatalogClient({ discoveryApi }), + identityApi: guestIdentityApi, + }); + await client.getLocationById('42'); const getLocationById = - MockedCatalogClient.mock.instances[1].getLocationById; + MockedCatalogClient.mock.instances[0].getLocationById; expect(getLocationById).toHaveBeenCalledWith('42', {}); expect(getLocationById).toHaveBeenCalledTimes(1); }); @@ -103,6 +100,10 @@ describe('CatalogClientWrapper', () => { }; it('injects authorization token', async () => { expect.assertions(2); + const client = new CatalogClientWrapper({ + client: new MockedCatalogClient({ discoveryApi }), + identityApi, + }); await client.getEntityByName(name); const getEntityByName = MockedCatalogClient.mock.instances[0].getEntityByName; @@ -117,6 +118,10 @@ describe('CatalogClientWrapper', () => { const location = { target: 'target' }; it('injects authorization token', async () => { expect.assertions(2); + const client = new CatalogClientWrapper({ + client: new MockedCatalogClient({ discoveryApi }), + identityApi, + }); await client.addLocation(location); const addLocation = MockedCatalogClient.mock.instances[0].addLocation; expect(addLocation).toHaveBeenCalledWith(location, { @@ -136,6 +141,10 @@ describe('CatalogClientWrapper', () => { }; it('injects authorization token', async () => { expect.assertions(2); + const client = new CatalogClientWrapper({ + client: new MockedCatalogClient({ discoveryApi }), + identityApi, + }); await client.getLocationByEntity(entity); const getLocationByEntity = MockedCatalogClient.mock.instances[0].getLocationByEntity; @@ -150,6 +159,10 @@ describe('CatalogClientWrapper', () => { it('injects authorization token', async () => { const uid = 'uid'; expect.assertions(2); + const client = new CatalogClientWrapper({ + client: new MockedCatalogClient({ discoveryApi }), + identityApi, + }); await client.removeEntityByUid(uid); const removeEntityByUid = MockedCatalogClient.mock.instances[0].removeEntityByUid; From 5ba0804189667cf4804446d7a850b0ca803ec1d5 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Fri, 29 Jan 2021 16:38:53 +0100 Subject: [PATCH 22/34] Get clean yarn from master --- yarn.lock | 45 --------------------------------------------- 1 file changed, 45 deletions(-) diff --git a/yarn.lock b/yarn.lock index 1e17345f48..aac0d3735b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2645,46 +2645,6 @@ remark-gfm "^1.0.0" zen-observable "^0.8.15" -"@backstage/core@^0.4.3": - version "0.4.4" - resolved "https://registry.npmjs.org/@backstage/core/-/core-0.4.4.tgz#83d5ce5c8dd8a9803badc1dde389b5bde963e959" - integrity sha512-UEZhPBC7DAZEOCRAYQ9XcXch5JDuPn+wYNp65Ik60SSz/RFjqhq8D3P7V074cMQqWLAddVDj5dUrNeN8N1gAUQ== - dependencies: - "@backstage/config" "^0.1.2" - "@backstage/core-api" "^0.2.8" - "@backstage/theme" "^0.2.2" - "@material-ui/core" "^4.11.0" - "@material-ui/icons" "^4.9.1" - "@material-ui/lab" "4.0.0-alpha.45" - "@types/dagre" "^0.7.44" - "@types/prop-types" "^15.7.3" - "@types/react" "^16.9" - "@types/react-sparklines" "^1.7.0" - classnames "^2.2.6" - clsx "^1.1.0" - d3-selection "^2.0.0" - d3-shape "^2.0.0" - d3-zoom "^2.0.0" - dagre "^0.8.5" - immer "^7.0.9" - lodash "^4.17.15" - material-table "^1.69.1" - prop-types "^15.7.2" - qs "^6.9.4" - rc-progress "^3.0.0" - react "^16.12.0" - react-dom "^16.12.0" - react-helmet "6.1.0" - react-hook-form "^6.6.0" - react-markdown "^5.0.2" - react-router "6.0.0-beta.0" - react-router-dom "6.0.0-beta.0" - react-sparklines "^1.7.0" - react-syntax-highlighter "^13.5.1" - react-use "^15.3.3" - remark-gfm "^1.0.0" - zen-observable "^0.8.15" - "@bcoe/v8-coverage@^0.2.3": version "0.2.3" resolved "https://registry.npmjs.org/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39" @@ -15509,11 +15469,6 @@ immer@1.10.0: resolved "https://registry.npmjs.org/immer/-/immer-1.10.0.tgz#bad67605ba9c810275d91e1c2a47d4582e98286d" integrity sha512-O3sR1/opvCDGLEVcvrGTMtLac8GJ5IwZC4puPrLuRj3l7ICKvkmA0vGuU9OW8mV9WIBRnaxp5GJh9IEAaNOoYg== -immer@^7.0.9: - version "7.0.15" - resolved "https://registry.npmjs.org/immer/-/immer-7.0.15.tgz#dc3bc6db87401659d2e737c67a21b227c484a4ad" - integrity sha512-yM7jo9+hvYgvdCQdqvhCNRRio0SCXc8xDPzA25SvKWa7b1WVPjLwQs1VYU5JPXjcJPTqAa5NP5dqpORGYBQ2AA== - immer@^8.0.1: version "8.0.1" resolved "https://registry.npmjs.org/immer/-/immer-8.0.1.tgz#9c73db683e2b3975c424fb0572af5889877ae656" From 320708e17175ee2e3dd8eec2c4b37f83d928c24d Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Sat, 30 Jan 2021 04:03:43 +0100 Subject: [PATCH 23/34] Only add authorization header if token exists --- .../src/api/CatalogImportClient.ts | 22 ++++++++++++------- plugins/fossa/src/api/FossaClient.ts | 6 ++--- .../src/api/KubernetesBackendClient.ts | 11 ++++++---- plugins/rollbar/src/api/RollbarClient.ts | 4 +--- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index 9b52de0ba4..b812e0dda9 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -49,13 +49,16 @@ export class CatalogImportClient implements CatalogImportApi { repo: string; }): Promise { const idToken = await this.identityApi.getIdToken(); + const headers = { + 'Content-Type': 'application/json', + } as { [header: string]: string }; + if (idToken) { + headers.authorization = `Bearer ${idToken}`; + } const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/analyze-location`, { - headers: { - authorization: `Bearer ${idToken}`, - 'Content-Type': 'application/json', - }, + headers, method: 'POST', body: JSON.stringify({ location: { type: 'url', target: repo }, @@ -80,13 +83,16 @@ export class CatalogImportClient implements CatalogImportApi { location: string; }): Promise { const idToken = await this.identityApi.getIdToken(); + const headers = { + 'Content-Type': 'application/json', + } as { [header: string]: string }; + if (idToken) { + headers.authorization = `Bearer ${idToken}`; + } const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/locations`, { - headers: { - authorization: `Bearer ${idToken}`, - 'Content-Type': 'application/json', - }, + headers, method: 'POST', body: JSON.stringify({ type: 'url', diff --git a/plugins/fossa/src/api/FossaClient.ts b/plugins/fossa/src/api/FossaClient.ts index 33c8c65c5f..4428ec7dbb 100644 --- a/plugins/fossa/src/api/FossaClient.ts +++ b/plugins/fossa/src/api/FossaClient.ts @@ -39,11 +39,9 @@ export class FossaClient implements FossaApi { private async callApi(path: string): Promise { const apiUrl = `${await this.discoveryApi.getBaseUrl('proxy')}/fossa`; - const headers: Record = { - authorization: await `Bearer ${this.identityApi.getIdToken()}`, - }; + const idToken = await this.identityApi.getIdToken(); const response = await fetch(`${apiUrl}/${path}`, { - headers, + headers: idToken ? { authorization: `Bearer ${idToken}` } : {}, }); if (response.status === 200) { return await response.json(); diff --git a/plugins/kubernetes/src/api/KubernetesBackendClient.ts b/plugins/kubernetes/src/api/KubernetesBackendClient.ts index e005b29517..96bdb9420f 100644 --- a/plugins/kubernetes/src/api/KubernetesBackendClient.ts +++ b/plugins/kubernetes/src/api/KubernetesBackendClient.ts @@ -39,12 +39,15 @@ export class KubernetesBackendClient implements KubernetesApi { ): Promise { const url = `${await this.discoveryApi.getBaseUrl('kubernetes')}${path}`; const idToken = await this.identityApi.getIdToken(); + const headers = { + 'Content-Type': 'application/json', + } as { [header: string]: string }; + if (idToken) { + headers.authorization = `Bearer ${idToken}`; + } const response = await fetch(url, { method: 'POST', - headers: { - authorization: `Bearer ${idToken}`, - 'Content-Type': 'application/json', - }, + headers, body: JSON.stringify(requestBody), }); diff --git a/plugins/rollbar/src/api/RollbarClient.ts b/plugins/rollbar/src/api/RollbarClient.ts index 1041accf19..dfc82b09e5 100644 --- a/plugins/rollbar/src/api/RollbarClient.ts +++ b/plugins/rollbar/src/api/RollbarClient.ts @@ -60,9 +60,7 @@ export class RollbarClient implements RollbarApi { const url = `${await this.discoveryApi.getBaseUrl('rollbar')}${path}`; const idToken = await this.identityApi.getIdToken(); const response = await fetch(url, { - headers: { - authorization: `Bearer ${idToken}`, - }, + headers: idToken ? { authorization: `Bearer ${idToken}` } : {}, }); if (!response.ok) { From 9729d9e572a589c0b2da33a179b8c0547a5faadb Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Sat, 30 Jan 2021 04:14:17 +0100 Subject: [PATCH 24/34] Use request options --- .../src/lib/catalog/CatalogEntityClient.ts | 4 ++- .../src/lib/catalog/types.ts | 32 +++++++++++++++++++ .../scaffolder-backend/src/service/router.ts | 7 ++-- 3 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 plugins/scaffolder-backend/src/lib/catalog/types.ts diff --git a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts index 56b82760bd..ed8b28552d 100644 --- a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts +++ b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts @@ -21,6 +21,7 @@ import { NotFoundError, PluginEndpointDiscovery, } from '@backstage/backend-common'; +import { CatalogEntityRequestOptions } from './types'; /** * A catalog client tailored for reading out entity data from the catalog. @@ -40,9 +41,10 @@ export class CatalogEntityClient { * Throws a NotFoundError or ConflictError if 0 or multiple templates are found. */ async findTemplate( - token: string | undefined, templateName: string, + options?: CatalogEntityRequestOptions, ): Promise { + const token = options?.token; const { items: templates } = (await this.catalogClient.getEntities( { filter: { diff --git a/plugins/scaffolder-backend/src/lib/catalog/types.ts b/plugins/scaffolder-backend/src/lib/catalog/types.ts new file mode 100644 index 0000000000..188efbd9d0 --- /dev/null +++ b/plugins/scaffolder-backend/src/lib/catalog/types.ts @@ -0,0 +1,32 @@ +/* + * Copyright 2021 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. + */ +/* + * 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. + */ + +export type CatalogEntityRequestOptions = { + token: string | undefined; +}; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 25b8c6e6f9..cf10643de3 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -104,10 +104,9 @@ export async function createRouter( // Forward authorization from client const user = req.user as BackstageIdentity; - const template = await entityClient.findTemplate( - user?.idToken, - templateName, - ); + const template = await entityClient.findTemplate(templateName, { + token: user?.idToken, + }); const validationResult: ValidatorResult = validate( values, From 987fa8b24fce3d32820be7d0dcf16988775a0afa Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Sun, 31 Jan 2021 01:17:11 +0100 Subject: [PATCH 25/34] Forward authorization header token --- plugins/scaffolder-backend/package.json | 1 + plugins/scaffolder-backend/src/service/router.ts | 9 ++------- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index c9a439bea3..ecf4978ff2 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -34,6 +34,7 @@ "@backstage/catalog-model": "^0.7.0", "@backstage/config": "^0.1.2", "@backstage/integration": "^0.3.1", + "@backstage/plugin-auth-backend": "^0.2.12", "@gitbeaker/core": "^28.0.2", "@gitbeaker/node": "^28.0.2", "@octokit/rest": "^18.0.12", diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index cf10643de3..e522417945 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -15,6 +15,7 @@ */ import { Config } from '@backstage/config'; +import { IdentityClient } from '@backstage/plugin-auth-backend'; import Docker from 'dockerode'; import express from 'express'; import { resolve as resolvePath } from 'path'; @@ -45,11 +46,6 @@ export interface RouterOptions { entityClient: CatalogEntityClient; } -// Avoid import of @backstage/core -type BackstageIdentity = { - idToken: string; -}; - export async function createRouter( options: RouterOptions, ): Promise { @@ -103,9 +99,8 @@ export async function createRouter( }; // Forward authorization from client - const user = req.user as BackstageIdentity; const template = await entityClient.findTemplate(templateName, { - token: user?.idToken, + token: IdentityClient.getBearerToken(req.headers.authorization), }); const validationResult: ValidatorResult = validate( From 124d21c20afc20c89495ca5076f60d597189ba9d Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Wed, 3 Feb 2021 02:00:32 +0100 Subject: [PATCH 26/34] Refactor to use context argument --- packages/catalog-client/src/CatalogClient.ts | 51 ++++++------- packages/catalog-client/src/index.ts | 1 + packages/catalog-client/src/types.ts | 27 +++++-- .../lib/catalog/CatalogIdentityClient.test.ts | 26 ++++--- .../src/lib/catalog/CatalogIdentityClient.ts | 20 ++---- plugins/auth-backend/src/lib/catalog/types.ts | 19 ----- .../src/providers/aws-alb/provider.test.ts | 24 ++++--- .../src/providers/aws-alb/provider.ts | 10 +-- .../src/providers/google/provider.ts | 4 +- plugins/auth-backend/src/providers/types.ts | 6 +- plugins/auth-backend/src/service/router.ts | 4 +- plugins/catalog/src/CatalogClientWrapper.ts | 72 ++++++++++++++----- plugins/catalog/src/plugin.ts | 1 - .../src/lib/catalog/CatalogEntityClient.ts | 8 +-- .../src/lib/catalog/types.ts | 32 --------- 15 files changed, 149 insertions(+), 156 deletions(-) delete mode 100644 plugins/auth-backend/src/lib/catalog/types.ts delete mode 100644 plugins/scaffolder-backend/src/lib/catalog/types.ts diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index 657841c000..b0c28bc284 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -24,13 +24,14 @@ import fetch from 'cross-fetch'; import { AddLocationRequest, AddLocationResponse, + ApiContext, + CatalogApi, CatalogEntitiesRequest, CatalogListResponse, - CatalogRequestOptions, DiscoveryApi, } from './types'; -export class CatalogClient { +export class CatalogClient implements CatalogApi { private readonly discoveryApi: DiscoveryApi; constructor(options: { discoveryApi: DiscoveryApi }) { @@ -39,14 +40,14 @@ export class CatalogClient { async getLocationById( id: String, - options?: CatalogRequestOptions, + context?: ApiContext, ): Promise { - return await this.getOptional(`/locations/${id}`, options); + return await this.getOptional(`/locations/${id}`, context); } async getEntities( request?: CatalogEntitiesRequest, - options?: CatalogRequestOptions, + context?: ApiContext, ): Promise> { const { filter = {}, fields = [] } = request ?? {}; const params: string[] = []; @@ -68,31 +69,31 @@ export class CatalogClient { const query = params.length ? `?${params.join('&')}` : ''; const entities: Entity[] = await this.getRequired( `/entities${query}`, - options, + context, ); return { items: entities }; } async getEntityByName( compoundName: EntityName, - options?: CatalogRequestOptions, + context?: ApiContext, ): Promise { const { kind, namespace = 'default', name } = compoundName; return this.getOptional( `/entities/by-name/${kind}/${namespace}/${name}`, - options, + context, ); } async addLocation( { type = 'url', target, dryRun }: AddLocationRequest, - options?: CatalogRequestOptions, + context?: ApiContext, ): Promise { const headers = { 'Content-Type': 'application/json', } as { [header: string]: string }; - if (options?.token) { - headers.authorization = `Bearer ${options.token}`; + if (context?.token) { + headers.authorization = `Bearer ${context.token}`; } const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/locations${ @@ -128,27 +129,24 @@ export class CatalogClient { async getLocationByEntity( entity: Entity, - options?: CatalogRequestOptions, + context?: ApiContext, ): Promise { const locationCompound = entity.metadata.annotations?.[LOCATION_ANNOTATION]; const all: { data: Location }[] = await this.getRequired( '/locations', - options, + context, ); return all .map(r => r.data) .find(l => locationCompound === `${l.type}:${l.target}`); } - async removeEntityByUid( - uid: string, - options?: CatalogRequestOptions, - ): Promise { + async removeEntityByUid(uid: string, context?: ApiContext): Promise { const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/entities/by-uid/${uid}`, { - headers: options?.token - ? { authorization: `Bearer ${options.token}` } + headers: context?.token + ? { authorization: `Bearer ${context.token}` } : {}, method: 'DELETE', }, @@ -166,14 +164,11 @@ export class CatalogClient { // Private methods // - private async getRequired( - path: string, - options?: CatalogRequestOptions, - ): Promise { + private async getRequired(path: string, context?: ApiContext): Promise { const url = `${await this.discoveryApi.getBaseUrl('catalog')}${path}`; const response = await fetch(url, { - headers: options?.token - ? { authorization: `Bearer ${options.token}` } + headers: context?.token + ? { authorization: `Bearer ${context.token}` } : {}, }); @@ -188,12 +183,12 @@ export class CatalogClient { private async getOptional( path: string, - options?: CatalogRequestOptions, + context?: ApiContext, ): Promise { const url = `${await this.discoveryApi.getBaseUrl('catalog')}${path}`; const response = await fetch(url, { - headers: options?.token - ? { authorization: `Bearer ${options.token}` } + headers: context?.token + ? { authorization: `Bearer ${context.token}` } : {}, }); diff --git a/packages/catalog-client/src/index.ts b/packages/catalog-client/src/index.ts index c5a626e25b..11003580a0 100644 --- a/packages/catalog-client/src/index.ts +++ b/packages/catalog-client/src/index.ts @@ -18,6 +18,7 @@ export { CatalogClient } from './CatalogClient'; export type { AddLocationRequest, AddLocationResponse, + ApiContext, CatalogApi, CatalogEntitiesRequest, CatalogListResponse, diff --git a/packages/catalog-client/src/types.ts b/packages/catalog-client/src/types.ts index 66fbedd38e..9a0ec90c16 100644 --- a/packages/catalog-client/src/types.ts +++ b/packages/catalog-client/src/types.ts @@ -25,19 +25,32 @@ export type CatalogListResponse = { items: T[]; }; -export type CatalogRequestOptions = { - token: string | undefined; +export type ApiContext = { + token?: string; }; export interface CatalogApi { - getLocationById(id: String): Promise; - getEntityByName(name: EntityName): Promise; + getLocationById( + id: String, + context?: ApiContext, + ): Promise; + getEntityByName( + name: EntityName, + context?: ApiContext, + ): Promise; getEntities( request?: CatalogEntitiesRequest, + context?: ApiContext, ): Promise>; - addLocation(location: AddLocationRequest): Promise; - getLocationByEntity(entity: Entity): Promise; - removeEntityByUid(uid: string): Promise; + addLocation( + location: AddLocationRequest, + context?: ApiContext, + ): Promise; + getLocationByEntity( + entity: Entity, + context?: ApiContext, + ): Promise; + removeEntityByUid(uid: string, context?: ApiContext): Promise; } export type AddLocationRequest = { diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts index 7eb50c92bf..4d4bcaf2bf 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.test.ts @@ -14,34 +14,38 @@ * limitations under the License. */ -import { CatalogClient } from '@backstage/catalog-client'; +import { CatalogApi } from '@backstage/catalog-client'; +import { UserEntity } from '@backstage/catalog-model'; import { CatalogIdentityClient } from './CatalogIdentityClient'; -jest.mock('@backstage/catalog-client'); -const MockedCatalogClient = CatalogClient as jest.Mock; - describe('CatalogIdentityClient', () => { - const token = 'fake-id-token'; + const catalogApi: jest.Mocked = { + getLocationById: jest.fn(), + getEntityByName: jest.fn(), + getEntities: jest.fn(), + addLocation: jest.fn(), + getLocationByEntity: jest.fn(), + removeEntityByUid: jest.fn(), + }; afterEach(() => jest.resetAllMocks()); it('passes through the correct search params', async () => { + catalogApi.getEntities.mockResolvedValueOnce({ items: [{} as UserEntity] }); const client = new CatalogIdentityClient({ - catalogClient: new MockedCatalogClient(), + catalogApi: catalogApi as CatalogApi, }); - client.findUser({ annotations: { key: 'value' } }, { token }); + client.findUser({ annotations: { key: 'value' } }); - const getEntities = MockedCatalogClient.mock.instances[0].getEntities; - expect(getEntities).toHaveBeenCalledWith( + expect(catalogApi.getEntities).toBeCalledWith( { filter: { kind: 'user', 'metadata.annotations.key': 'value', }, }, - { token }, + undefined, ); - expect(getEntities).toHaveBeenCalledTimes(1); }); }); diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts index 6c052321a1..3892b83da0 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts @@ -15,9 +15,8 @@ */ import { ConflictError, NotFoundError } from '@backstage/backend-common'; -import { CatalogClient } from '@backstage/catalog-client'; +import { ApiContext, CatalogApi } from '@backstage/catalog-client'; import { UserEntity } from '@backstage/catalog-model'; -import { CatalogIdentityRequestOptions } from './types'; type UserQuery = { annotations: Record; @@ -27,10 +26,10 @@ type UserQuery = { * A catalog client tailored for reading out identity data from the catalog. */ export class CatalogIdentityClient { - private readonly catalogClient: CatalogClient; + private readonly catalogApi: CatalogApi; - constructor(options: { catalogClient: CatalogClient }) { - this.catalogClient = options.catalogClient; + constructor(options: { catalogApi: CatalogApi }) { + this.catalogApi = options.catalogApi; } /** @@ -38,11 +37,7 @@ export class CatalogIdentityClient { * * Throws a NotFoundError or ConflictError if 0 or multiple users are found. */ - async findUser( - query: UserQuery, - options?: CatalogIdentityRequestOptions, - ): Promise { - const token = options?.token; + async findUser(query: UserQuery, context?: ApiContext): Promise { const filter: Record = { kind: 'user', }; @@ -50,10 +45,7 @@ export class CatalogIdentityClient { filter[`metadata.annotations.${key}`] = value; } - const { items } = await this.catalogClient.getEntities( - { filter }, - { token }, - ); + const { items } = await this.catalogApi.getEntities({ filter }, context); if (items.length !== 1) { if (items.length > 1) { diff --git a/plugins/auth-backend/src/lib/catalog/types.ts b/plugins/auth-backend/src/lib/catalog/types.ts deleted file mode 100644 index 389cc14bc5..0000000000 --- a/plugins/auth-backend/src/lib/catalog/types.ts +++ /dev/null @@ -1,19 +0,0 @@ -/* - * 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. - */ - -export type CatalogIdentityRequestOptions = { - token: string | undefined; -}; diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts index 536ba71f6b..d05b6bbfaf 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts @@ -14,7 +14,6 @@ * limitations under the License. */ import { getVoidLogger } from '@backstage/backend-common'; -import { CatalogClient } from '@backstage/catalog-client'; import express from 'express'; import { JWT } from 'jose'; @@ -44,9 +43,6 @@ jest.mock('cross-fetch', () => ({ }, })); -jest.mock('@backstage/catalog-client'); -const MockedCatalogClient = CatalogClient as jest.Mock; - const identityResolutionCallbackMock = async (): Promise> => { return { backstageIdentity: { @@ -71,7 +67,15 @@ beforeEach(() => { }); describe('AwsALBAuthProvider', () => { - const catalogClient = new MockedCatalogClient(); + const catalogApi = { + /* eslint-disable-next-line @typescript-eslint/no-unused-vars */ + addLocation: jest.fn(), + getEntities: jest.fn(), + getLocationByEntity: jest.fn(), + getLocationById: jest.fn(), + removeEntityByUid: jest.fn(), + getEntityByName: jest.fn(), + }; const mockResponseSend = jest.fn(); const mockRequest = ({ @@ -91,7 +95,7 @@ describe('AwsALBAuthProvider', () => { describe('should transform to type OAuthResponse', () => { it('when JWT is valid and identity is resolved successfully', async () => { - const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogClient, { + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { region: 'us-west-2', identityResolutionCallback: identityResolutionCallbackMock, issuer: 'foo', @@ -117,7 +121,7 @@ describe('AwsALBAuthProvider', () => { }); describe('should fail when', () => { it('JWT is missing', async () => { - const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogClient, { + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { region: 'us-west-2', identityResolutionCallback: identityResolutionCallbackMock, issuer: 'foo', @@ -129,7 +133,7 @@ describe('AwsALBAuthProvider', () => { }); it('JWT is invalid', async () => { - const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogClient, { + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { region: 'us-west-2', identityResolutionCallback: identityResolutionCallbackMock, issuer: 'foo', @@ -145,7 +149,7 @@ describe('AwsALBAuthProvider', () => { }); it('issuer is invalid', async () => { - const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogClient, { + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { region: 'us-west-2', identityResolutionCallback: identityResolutionCallbackMock, issuer: 'foobar', @@ -159,7 +163,7 @@ describe('AwsALBAuthProvider', () => { }); it('identity resolution callback rejects', async () => { - const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogClient, { + const provider = new AwsAlbAuthProvider(getVoidLogger(), catalogApi, { region: 'us-west-2', identityResolutionCallback: identityResolutionCallbackRejectedMock, issuer: 'foo', diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.ts b/plugins/auth-backend/src/providers/aws-alb/provider.ts index f8b61c15fd..61ea10947e 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.ts @@ -25,7 +25,7 @@ import { KeyObject } from 'crypto'; import { Logger } from 'winston'; import NodeCache from 'node-cache'; import { JWT } from 'jose'; -import { CatalogClient } from '@backstage/catalog-client'; +import { CatalogApi } from '@backstage/catalog-client'; const ALB_JWT_HEADER = 'x-amzn-oidc-data'; /** @@ -44,13 +44,13 @@ export const getJWTHeaders = (input: string) => { export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { private logger: Logger; - private readonly catalogClient: CatalogClient; + private readonly catalogClient: CatalogApi; private options: AwsAlbAuthProviderOptions; private readonly keyCache: NodeCache; constructor( logger: Logger, - catalogClient: CatalogClient, + catalogClient: CatalogApi, options: AwsAlbAuthProviderOptions, ) { this.logger = logger; @@ -108,14 +108,14 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { export const createAwsAlbProvider = ({ logger, - catalogClient, + catalogApi, config, identityResolver, }: AuthProviderFactoryOptions) => { const region = config.getString('region'); const issuer = config.getOptionalString('iss'); if (identityResolver !== undefined) { - return new AwsAlbAuthProvider(logger, catalogClient, { + return new AwsAlbAuthProvider(logger, catalogApi, { region, issuer, identityResolutionCallback: identityResolver, diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 15c2172e6e..848a02bad6 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -190,7 +190,7 @@ export const createGoogleProvider: AuthProviderFactory = ({ config, logger, tokenIssuer, - catalogClient, + catalogApi, }) => OAuthEnvironmentHandler.mapConfig(config, envConfig => { const clientId = envConfig.getString('clientId'); @@ -203,7 +203,7 @@ export const createGoogleProvider: AuthProviderFactory = ({ callbackUrl, logger, tokenIssuer, - identityClient: new CatalogIdentityClient({ catalogClient }), + identityClient: new CatalogIdentityClient({ catalogApi }), }); return OAuthAdapter.fromConfig(globalConfig, provider, { diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index b2f488546b..1a4e7b118a 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -15,7 +15,7 @@ */ import { PluginEndpointDiscovery } from '@backstage/backend-common'; -import { CatalogClient } from '@backstage/catalog-client'; +import { CatalogApi } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; import express from 'express'; import { Logger } from 'winston'; @@ -122,7 +122,7 @@ export type ExperimentalIdentityResolver = ( * An object containing information specific to the auth provider. */ payload: object, - catalogClient: CatalogClient, + catalogApi: CatalogApi, ) => Promise>; export type AuthProviderFactoryOptions = { @@ -132,8 +132,8 @@ export type AuthProviderFactoryOptions = { logger: Logger; tokenIssuer: TokenIssuer; discovery: PluginEndpointDiscovery; + catalogApi: CatalogApi; identityResolver?: ExperimentalIdentityResolver; - catalogClient: CatalogClient; }; export type AuthProviderFactory = ( diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 859c2a4846..85d60661e9 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -66,7 +66,7 @@ export async function createRouter({ keyDurationSeconds, logger: logger.child({ component: 'token-factory' }), }); - const catalogClient = new CatalogClient({ discoveryApi: discovery }); + const catalogApi = new CatalogClient({ discoveryApi: discovery }); const secret = config.getOptionalString('auth.session.secret'); if (secret) { @@ -103,7 +103,7 @@ export async function createRouter({ logger, tokenIssuer, discovery, - catalogClient, + catalogApi, }); const r = Router(); diff --git a/plugins/catalog/src/CatalogClientWrapper.ts b/plugins/catalog/src/CatalogClientWrapper.ts index 1f06cd380b..19bf77c91d 100644 --- a/plugins/catalog/src/CatalogClientWrapper.ts +++ b/plugins/catalog/src/CatalogClientWrapper.ts @@ -18,6 +18,7 @@ import { Entity, EntityName, Location } from '@backstage/catalog-model'; import { AddLocationRequest, AddLocationResponse, + ApiContext, CatalogApi, CatalogEntitiesRequest, CatalogListResponse, @@ -25,6 +26,9 @@ import { } from '@backstage/catalog-client'; import { IdentityApi } from '@backstage/core'; +/** + * CatalogClient wrapper that injects identity token for all requests + */ export class CatalogClientWrapper implements CatalogApi { private readonly identityApi: IdentityApi; private readonly client: CatalogClient; @@ -34,35 +38,69 @@ export class CatalogClientWrapper implements CatalogApi { this.identityApi = options.identityApi; } - async getLocationById(id: String): Promise { - const token = await this.identityApi.getIdToken(); - return await this.client.getLocationById(id, { token }); + private async injectToken(context?: ApiContext) { + const result: ApiContext = context ?? {}; + // Inject identity token if not provided + if (!result.token) { + result.token = await this.identityApi.getIdToken(); + } + return result; + } + + async getLocationById( + id: String, + context?: ApiContext, + ): Promise { + return await this.client.getLocationById( + id, + await this.injectToken(context), + ); } async getEntities( request?: CatalogEntitiesRequest, + context?: ApiContext, ): Promise> { - const token = await this.identityApi.getIdToken(); - return await this.client.getEntities(request, { token }); + return await this.client.getEntities( + request, + await this.injectToken(context), + ); } - async getEntityByName(compoundName: EntityName): Promise { - const token = await this.identityApi.getIdToken(); - return await this.client.getEntityByName(compoundName, { token }); + async getEntityByName( + compoundName: EntityName, + context?: ApiContext, + ): Promise { + return await this.client.getEntityByName( + compoundName, + await this.injectToken(context), + ); } - async addLocation(request: AddLocationRequest): Promise { - const token = await this.identityApi.getIdToken(); - return await this.client.addLocation(request, { token }); + async addLocation( + request: AddLocationRequest, + context?: ApiContext, + ): Promise { + return await this.client.addLocation( + request, + await this.injectToken(context), + ); } - async getLocationByEntity(entity: Entity): Promise { - const token = await this.identityApi.getIdToken(); - return await this.client.getLocationByEntity(entity, { token }); + async getLocationByEntity( + entity: Entity, + context?: ApiContext, + ): Promise { + return await this.client.getLocationByEntity( + entity, + await this.injectToken(context), + ); } - async removeEntityByUid(uid: string): Promise { - const token = await this.identityApi.getIdToken(); - return await this.client.removeEntityByUid(uid, { token }); + async removeEntityByUid(uid: string, context?: ApiContext): Promise { + return await this.client.removeEntityByUid( + uid, + await this.injectToken(context), + ); } } diff --git a/plugins/catalog/src/plugin.ts b/plugins/catalog/src/plugin.ts index 55a9f29372..f4f121f0e6 100644 --- a/plugins/catalog/src/plugin.ts +++ b/plugins/catalog/src/plugin.ts @@ -27,7 +27,6 @@ import { catalogRouteRef, entityRouteRef, } from '@backstage/plugin-catalog-react'; -import { catalogRouteRef, entityRouteRef } from './routes'; import { CatalogClientWrapper } from './CatalogClientWrapper'; export const catalogPlugin = createPlugin({ diff --git a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts index ed8b28552d..9ab9b17483 100644 --- a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts +++ b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts @@ -15,13 +15,12 @@ */ import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { CatalogClient } from '@backstage/catalog-client'; +import { ApiContext, CatalogClient } from '@backstage/catalog-client'; import { ConflictError, NotFoundError, PluginEndpointDiscovery, } from '@backstage/backend-common'; -import { CatalogEntityRequestOptions } from './types'; /** * A catalog client tailored for reading out entity data from the catalog. @@ -42,9 +41,8 @@ export class CatalogEntityClient { */ async findTemplate( templateName: string, - options?: CatalogEntityRequestOptions, + context?: ApiContext, ): Promise { - const token = options?.token; const { items: templates } = (await this.catalogClient.getEntities( { filter: { @@ -52,7 +50,7 @@ export class CatalogEntityClient { 'metadata.name': templateName, }, }, - { token }, + context, )) as { items: TemplateEntityV1alpha1[] }; if (templates.length !== 1) { diff --git a/plugins/scaffolder-backend/src/lib/catalog/types.ts b/plugins/scaffolder-backend/src/lib/catalog/types.ts deleted file mode 100644 index 188efbd9d0..0000000000 --- a/plugins/scaffolder-backend/src/lib/catalog/types.ts +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright 2021 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. - */ -/* - * 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. - */ - -export type CatalogEntityRequestOptions = { - token: string | undefined; -}; From d7f0203094e11a823aa80a57fc6ec2b8d0b3d98c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 10 Feb 2021 11:59:30 +0100 Subject: [PATCH 27/34] scaffolder-backend: remove auth-backend dependency --- plugins/scaffolder-backend/package.json | 1 - plugins/scaffolder-backend/src/service/router.ts | 7 +++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 50f383e9ca..b8b61e4649 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -34,7 +34,6 @@ "@backstage/catalog-model": "^0.7.1", "@backstage/config": "^0.1.2", "@backstage/integration": "^0.3.2", - "@backstage/plugin-auth-backend": "^0.2.12", "@gitbeaker/core": "^28.0.2", "@gitbeaker/node": "^28.0.2", "@octokit/rest": "^18.0.12", diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 0237386a4b..51425f75e8 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -15,7 +15,6 @@ */ import { Config } from '@backstage/config'; -import { IdentityClient } from '@backstage/plugin-auth-backend'; import Docker from 'dockerode'; import express from 'express'; import { resolve as resolvePath } from 'path'; @@ -136,7 +135,7 @@ export async function createRouter( // Forward authorization from client const template = await entityClient.findTemplate(templateName, { - token: IdentityClient.getBearerToken(req.headers.authorization), + token: getBearerToken(req.headers.authorization), }); const validationResult: ValidatorResult = validate( @@ -299,3 +298,7 @@ export async function createRouter( return app; } + +function getBearerToken(header?: string): string | undefined { + return header?.match(/Bearer\s+(\S+)/i)?.[1]; +} From 56aa06e47ca2c339d3648f795ccd6dbdeb86faf8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 10 Feb 2021 12:00:51 +0100 Subject: [PATCH 28/34] catalog-client: rename ApiContext to CatalogRequestOptions + avoid export --- packages/catalog-client/src/CatalogClient.ts | 48 +++++++++++--------- packages/catalog-client/src/index.ts | 1 - packages/catalog-client/src/types.ts | 17 ++++--- 3 files changed, 37 insertions(+), 29 deletions(-) diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index b0c28bc284..070a8999db 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -24,7 +24,7 @@ import fetch from 'cross-fetch'; import { AddLocationRequest, AddLocationResponse, - ApiContext, + CatalogRequestOptions, CatalogApi, CatalogEntitiesRequest, CatalogListResponse, @@ -40,14 +40,14 @@ export class CatalogClient implements CatalogApi { async getLocationById( id: String, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise { - return await this.getOptional(`/locations/${id}`, context); + return await this.getOptional(`/locations/${id}`, options); } async getEntities( request?: CatalogEntitiesRequest, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise> { const { filter = {}, fields = [] } = request ?? {}; const params: string[] = []; @@ -69,31 +69,31 @@ export class CatalogClient implements CatalogApi { const query = params.length ? `?${params.join('&')}` : ''; const entities: Entity[] = await this.getRequired( `/entities${query}`, - context, + options, ); return { items: entities }; } async getEntityByName( compoundName: EntityName, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise { const { kind, namespace = 'default', name } = compoundName; return this.getOptional( `/entities/by-name/${kind}/${namespace}/${name}`, - context, + options, ); } async addLocation( { type = 'url', target, dryRun }: AddLocationRequest, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise { const headers = { 'Content-Type': 'application/json', } as { [header: string]: string }; - if (context?.token) { - headers.authorization = `Bearer ${context.token}`; + if (options?.token) { + headers.authorization = `Bearer ${options.token}`; } const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/locations${ @@ -129,24 +129,27 @@ export class CatalogClient implements CatalogApi { async getLocationByEntity( entity: Entity, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise { const locationCompound = entity.metadata.annotations?.[LOCATION_ANNOTATION]; const all: { data: Location }[] = await this.getRequired( '/locations', - context, + options, ); return all .map(r => r.data) .find(l => locationCompound === `${l.type}:${l.target}`); } - async removeEntityByUid(uid: string, context?: ApiContext): Promise { + async removeEntityByUid( + uid: string, + options?: CatalogRequestOptions, + ): Promise { const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/entities/by-uid/${uid}`, { - headers: context?.token - ? { authorization: `Bearer ${context.token}` } + headers: options?.token + ? { authorization: `Bearer ${options.token}` } : {}, method: 'DELETE', }, @@ -164,11 +167,14 @@ export class CatalogClient implements CatalogApi { // Private methods // - private async getRequired(path: string, context?: ApiContext): Promise { + private async getRequired( + path: string, + options?: CatalogRequestOptions, + ): Promise { const url = `${await this.discoveryApi.getBaseUrl('catalog')}${path}`; const response = await fetch(url, { - headers: context?.token - ? { authorization: `Bearer ${context.token}` } + headers: options?.token + ? { authorization: `Bearer ${options.token}` } : {}, }); @@ -183,12 +189,12 @@ export class CatalogClient implements CatalogApi { private async getOptional( path: string, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise { const url = `${await this.discoveryApi.getBaseUrl('catalog')}${path}`; const response = await fetch(url, { - headers: context?.token - ? { authorization: `Bearer ${context.token}` } + headers: options?.token + ? { authorization: `Bearer ${options.token}` } : {}, }); diff --git a/packages/catalog-client/src/index.ts b/packages/catalog-client/src/index.ts index 11003580a0..c5a626e25b 100644 --- a/packages/catalog-client/src/index.ts +++ b/packages/catalog-client/src/index.ts @@ -18,7 +18,6 @@ export { CatalogClient } from './CatalogClient'; export type { AddLocationRequest, AddLocationResponse, - ApiContext, CatalogApi, CatalogEntitiesRequest, CatalogListResponse, diff --git a/packages/catalog-client/src/types.ts b/packages/catalog-client/src/types.ts index 9a0ec90c16..a36bad9c55 100644 --- a/packages/catalog-client/src/types.ts +++ b/packages/catalog-client/src/types.ts @@ -25,32 +25,35 @@ export type CatalogListResponse = { items: T[]; }; -export type ApiContext = { +export type CatalogRequestOptions = { token?: string; }; export interface CatalogApi { getLocationById( id: String, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise; getEntityByName( name: EntityName, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise; getEntities( request?: CatalogEntitiesRequest, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise>; addLocation( location: AddLocationRequest, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise; getLocationByEntity( entity: Entity, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise; - removeEntityByUid(uid: string, context?: ApiContext): Promise; + removeEntityByUid( + uid: string, + options?: CatalogRequestOptions, + ): Promise; } export type AddLocationRequest = { From 227a59688a558d2bda8a236b22b65547ccd1ae3d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 10 Feb 2021 12:06:22 +0100 Subject: [PATCH 29/34] refactor existing usage of ApiContext --- .../src/lib/catalog/CatalogIdentityClient.ts | 9 ++- plugins/catalog/src/CatalogClientWrapper.ts | 71 ++++++++----------- .../src/lib/catalog/CatalogEntityClient.ts | 6 +- 3 files changed, 40 insertions(+), 46 deletions(-) diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts index 3892b83da0..e1a3553394 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts @@ -15,7 +15,7 @@ */ import { ConflictError, NotFoundError } from '@backstage/backend-common'; -import { ApiContext, CatalogApi } from '@backstage/catalog-client'; +import { CatalogApi } from '@backstage/catalog-client'; import { UserEntity } from '@backstage/catalog-model'; type UserQuery = { @@ -37,7 +37,10 @@ export class CatalogIdentityClient { * * Throws a NotFoundError or ConflictError if 0 or multiple users are found. */ - async findUser(query: UserQuery, context?: ApiContext): Promise { + async findUser( + query: UserQuery, + options?: { token?: string }, + ): Promise { const filter: Record = { kind: 'user', }; @@ -45,7 +48,7 @@ export class CatalogIdentityClient { filter[`metadata.annotations.${key}`] = value; } - const { items } = await this.catalogApi.getEntities({ filter }, context); + const { items } = await this.catalogApi.getEntities({ filter }, options); if (items.length !== 1) { if (items.length > 1) { diff --git a/plugins/catalog/src/CatalogClientWrapper.ts b/plugins/catalog/src/CatalogClientWrapper.ts index 19bf77c91d..42e9d872d4 100644 --- a/plugins/catalog/src/CatalogClientWrapper.ts +++ b/plugins/catalog/src/CatalogClientWrapper.ts @@ -18,7 +18,6 @@ import { Entity, EntityName, Location } from '@backstage/catalog-model'; import { AddLocationRequest, AddLocationResponse, - ApiContext, CatalogApi, CatalogEntitiesRequest, CatalogListResponse, @@ -26,6 +25,10 @@ import { } from '@backstage/catalog-client'; import { IdentityApi } from '@backstage/core'; +type CatalogRequestOptions = { + token?: string; +}; + /** * CatalogClient wrapper that injects identity token for all requests */ @@ -38,69 +41,57 @@ export class CatalogClientWrapper implements CatalogApi { this.identityApi = options.identityApi; } - private async injectToken(context?: ApiContext) { - const result: ApiContext = context ?? {}; - // Inject identity token if not provided - if (!result.token) { - result.token = await this.identityApi.getIdToken(); - } - return result; - } - async getLocationById( id: String, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise { - return await this.client.getLocationById( - id, - await this.injectToken(context), - ); + return await this.client.getLocationById(id, { + token: options?.token ?? (await this.identityApi.getIdToken()), + }); } async getEntities( request?: CatalogEntitiesRequest, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise> { - return await this.client.getEntities( - request, - await this.injectToken(context), - ); + return await this.client.getEntities(request, { + token: options?.token ?? (await this.identityApi.getIdToken()), + }); } async getEntityByName( compoundName: EntityName, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise { - return await this.client.getEntityByName( - compoundName, - await this.injectToken(context), - ); + return await this.client.getEntityByName(compoundName, { + token: options?.token ?? (await this.identityApi.getIdToken()), + }); } async addLocation( request: AddLocationRequest, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise { - return await this.client.addLocation( - request, - await this.injectToken(context), - ); + return await this.client.addLocation(request, { + token: options?.token ?? (await this.identityApi.getIdToken()), + }); } async getLocationByEntity( entity: Entity, - context?: ApiContext, + options?: CatalogRequestOptions, ): Promise { - return await this.client.getLocationByEntity( - entity, - await this.injectToken(context), - ); + return await this.client.getLocationByEntity(entity, { + token: options?.token ?? (await this.identityApi.getIdToken()), + }); } - async removeEntityByUid(uid: string, context?: ApiContext): Promise { - return await this.client.removeEntityByUid( - uid, - await this.injectToken(context), - ); + async removeEntityByUid( + uid: string, + options?: CatalogRequestOptions, + ): Promise { + return await this.client.removeEntityByUid(uid, { + token: options?.token ?? (await this.identityApi.getIdToken()), + }); } } diff --git a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts index 9ab9b17483..827ed0559a 100644 --- a/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts +++ b/plugins/scaffolder-backend/src/lib/catalog/CatalogEntityClient.ts @@ -15,7 +15,7 @@ */ import { TemplateEntityV1alpha1 } from '@backstage/catalog-model'; -import { ApiContext, CatalogClient } from '@backstage/catalog-client'; +import { CatalogClient } from '@backstage/catalog-client'; import { ConflictError, NotFoundError, @@ -41,7 +41,7 @@ export class CatalogEntityClient { */ async findTemplate( templateName: string, - context?: ApiContext, + options?: { token?: string }, ): Promise { const { items: templates } = (await this.catalogClient.getEntities( { @@ -50,7 +50,7 @@ export class CatalogEntityClient { 'metadata.name': templateName, }, }, - context, + options, )) as { items: TemplateEntityV1alpha1[] }; if (templates.length !== 1) { From 2392cbf8eebb51aadb5480408fb69a0180551cac Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 10 Feb 2021 12:58:19 +0100 Subject: [PATCH 30/34] catalog-import: removed bonus code --- .../src/api/CatalogImportClient.ts | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index 6809404e62..16ad849d3c 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -161,37 +161,6 @@ export class CatalogImportClient implements CatalogImportApi { return payload.generateEntities.map((x: any) => x.entity); } - async createRepositoryLocation({ - location, - }: { - location: string; - }): Promise { - const idToken = await this.identityApi.getIdToken(); - const headers = { - 'Content-Type': 'application/json', - } as { [header: string]: string }; - if (idToken) { - headers.authorization = `Bearer ${idToken}`; - } - const response = await fetch( - `${await this.discoveryApi.getBaseUrl('catalog')}/locations`, - { - headers, - method: 'POST', - body: JSON.stringify({ - type: 'url', - target: location, - presence: 'optional', - }), - }, - ); - if (!response.ok) { - throw new Error( - `Received http response ${response.status}: ${response.statusText}`, - ); - } - } - // TODO: this response should better be part of the analyze-locations response and scm-independent / implemented per scm private async checkGitHubForExistingCatalogInfo({ url, From eacf540bbd1a20097c1632f3be9524c4ae3598c6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 10 Feb 2021 13:58:12 +0100 Subject: [PATCH 31/34] inline optional id token auth headers --- packages/catalog-client/src/CatalogClient.ts | 11 ++++------- plugins/catalog-import/src/api/CatalogImportClient.ts | 11 ++++------- plugins/kubernetes/src/api/KubernetesBackendClient.ts | 11 ++++------- 3 files changed, 12 insertions(+), 21 deletions(-) diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index 2cee8cd4ff..92432e7088 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -89,18 +89,15 @@ export class CatalogClient implements CatalogApi { { type = 'url', target, dryRun, presence }: AddLocationRequest, options?: CatalogRequestOptions, ): Promise { - const headers = { - 'Content-Type': 'application/json', - } as { [header: string]: string }; - if (options?.token) { - headers.authorization = `Bearer ${options.token}`; - } const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/locations${ dryRun ? '?dryRun=true' : '' }`, { - headers, + headers: { + 'Content-Type': 'application/json', + ...(options?.token && { Authorization: `Bearer ${options?.token}` }), + }, method: 'POST', body: JSON.stringify({ type, target, presence }), }, diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index 16ad849d3c..b4b1ebe845 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -133,16 +133,13 @@ export class CatalogImportClient implements CatalogImportApi { repo: string; }): Promise { const idToken = await this.identityApi.getIdToken(); - const headers = { - 'Content-Type': 'application/json', - } as { [header: string]: string }; - if (idToken) { - headers.authorization = `Bearer ${idToken}`; - } const response = await fetch( `${await this.discoveryApi.getBaseUrl('catalog')}/analyze-location`, { - headers, + headers: { + 'Content-Type': 'application/json', + ...(idToken && { Authorization: `Bearer ${idToken}` }), + }, method: 'POST', body: JSON.stringify({ location: { type: 'url', target: repo }, diff --git a/plugins/kubernetes/src/api/KubernetesBackendClient.ts b/plugins/kubernetes/src/api/KubernetesBackendClient.ts index 96bdb9420f..dcb2cca142 100644 --- a/plugins/kubernetes/src/api/KubernetesBackendClient.ts +++ b/plugins/kubernetes/src/api/KubernetesBackendClient.ts @@ -39,15 +39,12 @@ export class KubernetesBackendClient implements KubernetesApi { ): Promise { const url = `${await this.discoveryApi.getBaseUrl('kubernetes')}${path}`; const idToken = await this.identityApi.getIdToken(); - const headers = { - 'Content-Type': 'application/json', - } as { [header: string]: string }; - if (idToken) { - headers.authorization = `Bearer ${idToken}`; - } const response = await fetch(url, { method: 'POST', - headers, + headers: { + 'Content-Type': 'application/json', + ...(idToken && { Authorization: `Bearer ${idToken}` }), + }, body: JSON.stringify(requestBody), }); From 2dee1bc9a40d34584a0b1143320f4e32706b0428 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 10 Feb 2021 14:41:49 +0100 Subject: [PATCH 32/34] update changeset bump levels --- .changeset/bright-rules-know.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.changeset/bright-rules-know.md b/.changeset/bright-rules-know.md index bc837a0bae..48765a61dd 100644 --- a/.changeset/bright-rules-know.md +++ b/.changeset/bright-rules-know.md @@ -1,12 +1,12 @@ --- -'@backstage/catalog-client': minor -'@backstage/plugin-auth-backend': minor -'@backstage/plugin-catalog': minor +'@backstage/catalog-client': patch +'@backstage/plugin-auth-backend': patch +'@backstage/plugin-catalog': patch '@backstage/plugin-catalog-import': minor -'@backstage/plugin-fossa': minor -'@backstage/plugin-kubernetes': minor +'@backstage/plugin-fossa': patch +'@backstage/plugin-kubernetes': patch '@backstage/plugin-rollbar': minor -'@backstage/plugin-scaffolder-backend': minor +'@backstage/plugin-scaffolder-backend': patch --- -Add identity token to api requests +Include Backstage identity token in requests to backend plugins. From 8d2089a432c4ad80bd92b277f26b53d51d83e023 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 10 Feb 2021 15:20:54 +0100 Subject: [PATCH 33/34] scaffolder: include backstage identity token in requests --- .changeset/bright-rules-know.md | 1 + plugins/scaffolder/dev/index.tsx | 7 ++++--- plugins/scaffolder/src/api.ts | 16 +++++++++++++--- plugins/scaffolder/src/plugin.ts | 6 ++++-- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/.changeset/bright-rules-know.md b/.changeset/bright-rules-know.md index 48765a61dd..d9607e319f 100644 --- a/.changeset/bright-rules-know.md +++ b/.changeset/bright-rules-know.md @@ -6,6 +6,7 @@ '@backstage/plugin-fossa': patch '@backstage/plugin-kubernetes': patch '@backstage/plugin-rollbar': minor +'@backstage/plugin-scaffolder': minor '@backstage/plugin-scaffolder-backend': patch --- diff --git a/plugins/scaffolder/dev/index.tsx b/plugins/scaffolder/dev/index.tsx index eff16ab0f5..e75aadb03f 100644 --- a/plugins/scaffolder/dev/index.tsx +++ b/plugins/scaffolder/dev/index.tsx @@ -16,7 +16,7 @@ import React from 'react'; import { createDevApp } from '@backstage/dev-utils'; -import { discoveryApiRef } from '@backstage/core'; +import { discoveryApiRef, identityApiRef } from '@backstage/core'; import { CatalogClient } from '@backstage/catalog-client'; import { catalogApiRef } from '@backstage/plugin-catalog-react'; import { TemplateIndexPage, TemplatePage } from '../src/plugin'; @@ -30,8 +30,9 @@ createDevApp() }) .registerApi({ api: scaffolderApiRef, - deps: { discoveryApi: discoveryApiRef }, - factory: ({ discoveryApi }) => new ScaffolderApi({ discoveryApi }), + deps: { discoveryApi: discoveryApiRef, identityApi: identityApiRef }, + factory: ({ discoveryApi, identityApi }) => + new ScaffolderApi({ discoveryApi, identityApi }), }) .addPage({ path: '/create', diff --git a/plugins/scaffolder/src/api.ts b/plugins/scaffolder/src/api.ts index ca52418e92..453b1b6a7e 100644 --- a/plugins/scaffolder/src/api.ts +++ b/plugins/scaffolder/src/api.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { createApiRef, DiscoveryApi } from '@backstage/core'; +import { createApiRef, DiscoveryApi, IdentityApi } from '@backstage/core'; export const scaffolderApiRef = createApiRef({ id: 'plugin.scaffolder.service', @@ -23,9 +23,14 @@ export const scaffolderApiRef = createApiRef({ export class ScaffolderApi { private readonly discoveryApi: DiscoveryApi; + private readonly identityApi: IdentityApi; - constructor(options: { discoveryApi: DiscoveryApi }) { + constructor(options: { + discoveryApi: DiscoveryApi; + identityApi: IdentityApi; + }) { this.discoveryApi = options.discoveryApi; + this.identityApi = options.identityApi; } /** @@ -36,11 +41,13 @@ export class ScaffolderApi { * @param values Parameters for the template, e.g. name, description */ async scaffold(templateName: string, values: Record) { + const token = await this.identityApi.getIdToken(); const url = `${await this.discoveryApi.getBaseUrl('scaffolder')}/v1/jobs`; const response = await fetch(url, { method: 'POST', headers: { 'Content-Type': 'application/json', + ...(token && { Authorization: `Bearer ${token}` }), }, body: JSON.stringify({ templateName, values: { ...values } }), }); @@ -56,8 +63,11 @@ export class ScaffolderApi { } async getJob(jobId: string) { + const token = await this.identityApi.getIdToken(); const baseUrl = await this.discoveryApi.getBaseUrl('scaffolder'); const url = `${baseUrl}/v1/job/${encodeURIComponent(jobId)}`; - return fetch(url).then(x => x.json()); + return fetch(url, { + headers: token ? { Authorization: `Bearer ${token}` } : {}, + }).then(x => x.json()); } } diff --git a/plugins/scaffolder/src/plugin.ts b/plugins/scaffolder/src/plugin.ts index ed20705185..3e884d7683 100644 --- a/plugins/scaffolder/src/plugin.ts +++ b/plugins/scaffolder/src/plugin.ts @@ -18,6 +18,7 @@ import { createPlugin, createApiFactory, discoveryApiRef, + identityApiRef, createRoutableExtension, } from '@backstage/core'; import { ScaffolderPage as ScaffolderPageComponent } from './components/ScaffolderPage'; @@ -30,8 +31,9 @@ export const scaffolderPlugin = createPlugin({ apis: [ createApiFactory({ api: scaffolderApiRef, - deps: { discoveryApi: discoveryApiRef }, - factory: ({ discoveryApi }) => new ScaffolderApi({ discoveryApi }), + deps: { discoveryApi: discoveryApiRef, identityApi: identityApiRef }, + factory: ({ discoveryApi, identityApi }) => + new ScaffolderApi({ discoveryApi, identityApi }), }), ], register({ router }) { From 6cd39362e76e5cfb8635d1590b9a2392443311bd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 10 Feb 2021 17:23:28 +0100 Subject: [PATCH 34/34] review feedback tweaks --- packages/catalog-client/src/CatalogClient.ts | 6 +++--- .../src/components/ImportComponentPage.test.tsx | 2 +- plugins/catalog/src/CatalogClientWrapper.test.ts | 6 ------ plugins/fossa/src/api/FossaClient.ts | 2 +- plugins/rollbar/src/api/RollbarClient.ts | 2 +- 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index 92432e7088..04206edcdb 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -146,7 +146,7 @@ export class CatalogClient implements CatalogApi { `${await this.discoveryApi.getBaseUrl('catalog')}/entities/by-uid/${uid}`, { headers: options?.token - ? { authorization: `Bearer ${options.token}` } + ? { Authorization: `Bearer ${options.token}` } : {}, method: 'DELETE', }, @@ -171,7 +171,7 @@ export class CatalogClient implements CatalogApi { const url = `${await this.discoveryApi.getBaseUrl('catalog')}${path}`; const response = await fetch(url, { headers: options?.token - ? { authorization: `Bearer ${options.token}` } + ? { Authorization: `Bearer ${options.token}` } : {}, }); @@ -191,7 +191,7 @@ export class CatalogClient implements CatalogApi { const url = `${await this.discoveryApi.getBaseUrl('catalog')}${path}`; const response = await fetch(url, { headers: options?.token - ? { authorization: `Bearer ${options.token}` } + ? { Authorization: `Bearer ${options.token}` } : {}, }); diff --git a/plugins/catalog-import/src/components/ImportComponentPage.test.tsx b/plugins/catalog-import/src/components/ImportComponentPage.test.tsx index ed73d1c435..4cadea85c0 100644 --- a/plugins/catalog-import/src/components/ImportComponentPage.test.tsx +++ b/plugins/catalog-import/src/components/ImportComponentPage.test.tsx @@ -57,7 +57,7 @@ describe('', () => { new CatalogImportClient({ discoveryApi: {} as any, githubAuthApi: { - getAccessToken: (_, __) => Promise.resolve('token'), + getAccessToken: async () => 'token', }, identityApi, configApi: {} as any, diff --git a/plugins/catalog/src/CatalogClientWrapper.test.ts b/plugins/catalog/src/CatalogClientWrapper.test.ts index d95d274ee9..87dcd8f74b 100644 --- a/plugins/catalog/src/CatalogClientWrapper.test.ts +++ b/plugins/catalog/src/CatalogClientWrapper.test.ts @@ -63,7 +63,6 @@ describe('CatalogClientWrapper', () => { describe('getEntities', () => { it('injects authorization token', async () => { - expect.assertions(2); const client = new CatalogClientWrapper({ client: new MockedCatalogClient({ discoveryApi }), identityApi, @@ -79,7 +78,6 @@ describe('CatalogClientWrapper', () => { describe('getLocationById', () => { it('omits authorization token when guest', async () => { - expect.assertions(2); const client = new CatalogClientWrapper({ client: new MockedCatalogClient({ discoveryApi }), identityApi: guestIdentityApi, @@ -99,7 +97,6 @@ describe('CatalogClientWrapper', () => { name: 'name', }; it('injects authorization token', async () => { - expect.assertions(2); const client = new CatalogClientWrapper({ client: new MockedCatalogClient({ discoveryApi }), identityApi, @@ -117,7 +114,6 @@ describe('CatalogClientWrapper', () => { describe('addLocation', () => { const location = { target: 'target' }; it('injects authorization token', async () => { - expect.assertions(2); const client = new CatalogClientWrapper({ client: new MockedCatalogClient({ discoveryApi }), identityApi, @@ -140,7 +136,6 @@ describe('CatalogClientWrapper', () => { }, }; it('injects authorization token', async () => { - expect.assertions(2); const client = new CatalogClientWrapper({ client: new MockedCatalogClient({ discoveryApi }), identityApi, @@ -158,7 +153,6 @@ describe('CatalogClientWrapper', () => { describe('removeEntityByUid', () => { it('injects authorization token', async () => { const uid = 'uid'; - expect.assertions(2); const client = new CatalogClientWrapper({ client: new MockedCatalogClient({ discoveryApi }), identityApi, diff --git a/plugins/fossa/src/api/FossaClient.ts b/plugins/fossa/src/api/FossaClient.ts index 4428ec7dbb..5693895901 100644 --- a/plugins/fossa/src/api/FossaClient.ts +++ b/plugins/fossa/src/api/FossaClient.ts @@ -41,7 +41,7 @@ export class FossaClient implements FossaApi { const apiUrl = `${await this.discoveryApi.getBaseUrl('proxy')}/fossa`; const idToken = await this.identityApi.getIdToken(); const response = await fetch(`${apiUrl}/${path}`, { - headers: idToken ? { authorization: `Bearer ${idToken}` } : {}, + headers: idToken ? { Authorization: `Bearer ${idToken}` } : {}, }); if (response.status === 200) { return await response.json(); diff --git a/plugins/rollbar/src/api/RollbarClient.ts b/plugins/rollbar/src/api/RollbarClient.ts index dfc82b09e5..d3ad3d63db 100644 --- a/plugins/rollbar/src/api/RollbarClient.ts +++ b/plugins/rollbar/src/api/RollbarClient.ts @@ -60,7 +60,7 @@ export class RollbarClient implements RollbarApi { const url = `${await this.discoveryApi.getBaseUrl('rollbar')}${path}`; const idToken = await this.identityApi.getIdToken(); const response = await fetch(url, { - headers: idToken ? { authorization: `Bearer ${idToken}` } : {}, + headers: idToken ? { Authorization: `Bearer ${idToken}` } : {}, }); if (!response.ok) {