From 88a0a4ee6a29de2b40961db916d1c1ea57e68c47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 5 Jan 2022 21:19:07 +0100 Subject: [PATCH 1/6] Make the techdocs APIs use the FetchApi MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- plugins/techdocs/api-report.md | 54 +++++++++++++----------------- plugins/techdocs/src/api.ts | 31 +++++++++++++++++ plugins/techdocs/src/client.ts | 49 ++++++++++++++++----------- plugins/techdocs/src/plugin.ts | 9 +++-- plugins/techdocs/src/setupTests.ts | 1 + 5 files changed, 92 insertions(+), 52 deletions(-) diff --git a/plugins/techdocs/api-report.md b/plugins/techdocs/api-report.md index fc7b515167..7aed7139db 100644 --- a/plugins/techdocs/api-report.md +++ b/plugins/techdocs/api-report.md @@ -13,6 +13,7 @@ import { CSSProperties } from '@material-ui/styles'; import { DiscoveryApi } from '@backstage/core-plugin-api'; import { Entity } from '@backstage/catalog-model'; import { EntityName } from '@backstage/catalog-model'; +import { FetchApi } from '@backstage/core-plugin-api'; import { IdentityApi } from '@backstage/core-plugin-api'; import { LocationSpec } from '@backstage/catalog-model'; import { default as React_2 } from 'react'; @@ -197,16 +198,11 @@ export const Reader: ({ // @public (undocumented) export const Router: () => JSX.Element; -// Warning: (ae-missing-release-tag) "SyncResult" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public (undocumented) +// @public export type SyncResult = 'cached' | 'updated'; -// Warning: (ae-missing-release-tag) "TechDocsApi" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public (undocumented) +// @public export interface TechDocsApi { - // (undocumented) getApiOrigin(): Promise; // Warning: (ae-forgotten-export) The symbol "TechDocsEntityMetadata" needs to be exported by the entry point index.d.ts // @@ -218,23 +214,16 @@ export interface TechDocsApi { getTechDocsMetadata(entityId: EntityName): Promise; } -// Warning: (ae-missing-release-tag) "techdocsApiRef" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public (undocumented) +// @public export const techdocsApiRef: ApiRef; -// Warning: (ae-missing-release-tag) "TechDocsClient" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// // @public export class TechDocsClient implements TechDocsApi { - constructor({ - configApi, - discoveryApi, - identityApi, - }: { + constructor(options: { configApi: Config; discoveryApi: DiscoveryApi; identityApi: IdentityApi; + fetchApi?: FetchApi; }); // (undocumented) configApi: Config; @@ -347,11 +336,8 @@ export const TechDocsReaderPage: ({ children, }: TechDocsPageProps) => JSX.Element; -// Warning: (ae-missing-release-tag) "TechDocsStorageApi" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public (undocumented) +// @public export interface TechDocsStorageApi { - // (undocumented) getApiOrigin(): Promise; // (undocumented) getBaseUrl( @@ -372,23 +358,31 @@ export interface TechDocsStorageApi { ): Promise; } -// Warning: (ae-missing-release-tag) "techdocsStorageApiRef" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public (undocumented) +// @public export const techdocsStorageApiRef: ApiRef; -// Warning: (ae-missing-release-tag) "TechDocsStorageClient" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// Warning: (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name +// Warning: (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name +// Warning: (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name +// Warning: (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name +// Warning: (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name +// Warning: (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name +// Warning: (tsdoc-malformed-html-name) Invalid HTML element: A space is not allowed here +// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag +// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag +// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag +// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag +// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag +// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag +// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag // // @public export class TechDocsStorageClient implements TechDocsStorageApi { - constructor({ - configApi, - discoveryApi, - identityApi, - }: { + constructor(options: { configApi: Config; discoveryApi: DiscoveryApi; identityApi: IdentityApi; + fetchApi?: FetchApi; }); // (undocumented) configApi: Config; diff --git a/plugins/techdocs/src/api.ts b/plugins/techdocs/src/api.ts index cd130f083d..4566bbf028 100644 --- a/plugins/techdocs/src/api.ts +++ b/plugins/techdocs/src/api.ts @@ -18,17 +18,40 @@ import { EntityName } from '@backstage/catalog-model'; import { TechDocsEntityMetadata, TechDocsMetadata } from './types'; import { createApiRef } from '@backstage/core-plugin-api'; +/** + * Utility API reference for the {@link TechDocsStorageApi}. + * + * @public + */ export const techdocsStorageApiRef = createApiRef({ id: 'plugin.techdocs.storageservice', }); +/** + * Utility API reference for the {@link TechDocsApi}. + * + * @public + */ export const techdocsApiRef = createApiRef({ id: 'plugin.techdocs.service', }); +/** + * The outcome of a docs sync operation. + * + * @public + */ export type SyncResult = 'cached' | 'updated'; +/** + * API which talks to TechDocs storage to fetch files to render. + * + * @public + */ export interface TechDocsStorageApi { + /** + * Set to techdocs.requestUrl as the URL for techdocs-backend API. + */ getApiOrigin(): Promise; getStorageUrl(): Promise; getBuilder(): Promise; @@ -44,7 +67,15 @@ export interface TechDocsStorageApi { ): Promise; } +/** + * API to talk to techdocs-backend. + * + * @public + */ export interface TechDocsApi { + /** + * Set to techdocs.requestUrl as the URL for techdocs-backend API. + */ getApiOrigin(): Promise; getTechDocsMetadata(entityId: EntityName): Promise; getEntityMetadata(entityId: EntityName): Promise; diff --git a/plugins/techdocs/src/client.ts b/plugins/techdocs/src/client.ts index 5cf33ac8d5..6f381a5bd9 100644 --- a/plugins/techdocs/src/client.ts +++ b/plugins/techdocs/src/client.ts @@ -16,7 +16,11 @@ import { EntityName } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { DiscoveryApi, IdentityApi } from '@backstage/core-plugin-api'; +import { + DiscoveryApi, + FetchApi, + IdentityApi, +} from '@backstage/core-plugin-api'; import { NotFoundError, ResponseError } from '@backstage/errors'; import { EventSourcePolyfill } from 'event-source-polyfill'; import { SyncResult, TechDocsApi, TechDocsStorageApi } from './api'; @@ -24,24 +28,25 @@ import { TechDocsEntityMetadata, TechDocsMetadata } from './types'; /** * API to talk to `techdocs-backend`. + * + * @public */ export class TechDocsClient implements TechDocsApi { public configApi: Config; public discoveryApi: DiscoveryApi; public identityApi: IdentityApi; + private fetchApi: FetchApi; - constructor({ - configApi, - discoveryApi, - identityApi, - }: { + constructor(options: { configApi: Config; discoveryApi: DiscoveryApi; identityApi: IdentityApi; + fetchApi?: FetchApi; }) { - this.configApi = configApi; - this.discoveryApi = discoveryApi; - this.identityApi = identityApi; + this.configApi = options.configApi; + this.discoveryApi = options.discoveryApi; + this.identityApi = options.identityApi; + this.fetchApi = options.fetchApi ?? { fetch }; } async getApiOrigin(): Promise { @@ -67,7 +72,7 @@ export class TechDocsClient implements TechDocsApi { const requestUrl = `${apiOrigin}/metadata/techdocs/${namespace}/${kind}/${name}`; const { token } = await this.identityApi.getCredentials(); - const request = await fetch(`${requestUrl}`, { + const request = await this.fetchApi.fetch(`${requestUrl}`, { headers: token ? { Authorization: `Bearer ${token}` } : {}, }); @@ -95,7 +100,7 @@ export class TechDocsClient implements TechDocsApi { const requestUrl = `${apiOrigin}/metadata/entity/${namespace}/${kind}/${name}`; const { token } = await this.identityApi.getCredentials(); - const request = await fetch(`${requestUrl}`, { + const request = await this.fetchApi.fetch(`${requestUrl}`, { headers: token ? { Authorization: `Bearer ${token}` } : {}, }); @@ -109,24 +114,28 @@ export class TechDocsClient implements TechDocsApi { /** * API which talks to TechDocs storage to fetch files to render. +<<<<<<< HEAD +======= + * + * @public +>>>>>>> 31c54b8ea2 (Make the techdocs APIs use the FetchApi) */ export class TechDocsStorageClient implements TechDocsStorageApi { public configApi: Config; public discoveryApi: DiscoveryApi; public identityApi: IdentityApi; + private fetchApi: FetchApi; - constructor({ - configApi, - discoveryApi, - identityApi, - }: { + constructor(options: { configApi: Config; discoveryApi: DiscoveryApi; identityApi: IdentityApi; + fetchApi?: FetchApi; }) { - this.configApi = configApi; - this.discoveryApi = discoveryApi; - this.identityApi = identityApi; + this.configApi = options.configApi; + this.discoveryApi = options.discoveryApi; + this.identityApi = options.identityApi; + this.fetchApi = options.fetchApi ?? { fetch }; } async getApiOrigin(): Promise { @@ -162,7 +171,7 @@ export class TechDocsStorageClient implements TechDocsStorageApi { const url = `${storageUrl}/${namespace}/${kind}/${name}/${path}`; const { token } = await this.identityApi.getCredentials(); - const request = await fetch( + const request = await this.fetchApi.fetch( `${url.endsWith('/') ? url : `${url}/`}index.html`, { headers: token ? { Authorization: `Bearer ${token}` } : {}, diff --git a/plugins/techdocs/src/plugin.ts b/plugins/techdocs/src/plugin.ts index 844003987a..274d1904d8 100644 --- a/plugins/techdocs/src/plugin.ts +++ b/plugins/techdocs/src/plugin.ts @@ -28,6 +28,7 @@ import { createPlugin, createRoutableExtension, discoveryApiRef, + fetchApiRef, identityApiRef, } from '@backstage/core-plugin-api'; @@ -40,12 +41,14 @@ export const techdocsPlugin = createPlugin({ configApi: configApiRef, discoveryApi: discoveryApiRef, identityApi: identityApiRef, + fetchApi: fetchApiRef, }, - factory: ({ configApi, discoveryApi, identityApi }) => + factory: ({ configApi, discoveryApi, identityApi, fetchApi }) => new TechDocsStorageClient({ configApi, discoveryApi, identityApi, + fetchApi, }), }), createApiFactory({ @@ -54,12 +57,14 @@ export const techdocsPlugin = createPlugin({ configApi: configApiRef, discoveryApi: discoveryApiRef, identityApi: identityApiRef, + fetchApi: fetchApiRef, }, - factory: ({ configApi, discoveryApi, identityApi }) => + factory: ({ configApi, discoveryApi, identityApi, fetchApi }) => new TechDocsClient({ configApi, discoveryApi, identityApi, + fetchApi, }), }), ], diff --git a/plugins/techdocs/src/setupTests.ts b/plugins/techdocs/src/setupTests.ts index 963c0f188b..c1d649f2ad 100644 --- a/plugins/techdocs/src/setupTests.ts +++ b/plugins/techdocs/src/setupTests.ts @@ -15,3 +15,4 @@ */ import '@testing-library/jest-dom'; +import 'cross-fetch/polyfill'; From aecfe4f403d72dd5e4c60893b659d459ff11acdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 5 Jan 2022 21:35:51 +0100 Subject: [PATCH 2/6] same for the scaffolder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/modern-knives-roll.md | 5 ++++ .changeset/sweet-candles-greet.md | 5 ++++ plugins/scaffolder/api-report.md | 24 +++++----------- plugins/scaffolder/src/api.ts | 46 +++++++++++++++++++++---------- plugins/scaffolder/src/plugin.ts | 11 ++++++-- 5 files changed, 57 insertions(+), 34 deletions(-) create mode 100644 .changeset/modern-knives-roll.md create mode 100644 .changeset/sweet-candles-greet.md diff --git a/.changeset/modern-knives-roll.md b/.changeset/modern-knives-roll.md new file mode 100644 index 0000000000..4d69f285b2 --- /dev/null +++ b/.changeset/modern-knives-roll.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder': patch +--- + +Make `ScaffolderClient` use the `FetchApi`. diff --git a/.changeset/sweet-candles-greet.md b/.changeset/sweet-candles-greet.md new file mode 100644 index 0000000000..1a83f7828a --- /dev/null +++ b/.changeset/sweet-candles-greet.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs': patch +--- + +Make `TechDocsClient` and `TechDocsStorageClient` use the `FetchApi`. diff --git a/plugins/scaffolder/api-report.md b/plugins/scaffolder/api-report.md index d2bbb2a5e1..835712651d 100644 --- a/plugins/scaffolder/api-report.md +++ b/plugins/scaffolder/api-report.md @@ -15,6 +15,7 @@ import { Entity } from '@backstage/catalog-model'; import { EntityName } from '@backstage/catalog-model'; import { Extension } from '@backstage/core-plugin-api'; import { ExternalRouteRef } from '@backstage/core-plugin-api'; +import { FetchApi } from '@backstage/core-plugin-api'; import { FieldProps } from '@rjsf/core'; import { FieldValidation } from '@rjsf/core'; import { IconButton } from '@material-ui/core'; @@ -166,9 +167,7 @@ export interface RepoUrlPickerUiOptions { allowedOwners?: string[]; } -// Warning: (ae-missing-release-tag) "ScaffolderApi" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public (undocumented) +// @public export interface ScaffolderApi { // (undocumented) getIntegrationsList(options: { allowedHosts: string[] }): Promise< @@ -200,27 +199,18 @@ export interface ScaffolderApi { // Warning: (ae-forgotten-export) The symbol "LogEvent" needs to be exported by the entry point index.d.ts // // (undocumented) - streamLogs({ - taskId, - after, - }: { - taskId: string; - after?: number; - }): Observable; + streamLogs(options: { taskId: string; after?: number }): Observable; } -// Warning: (ae-missing-release-tag) "scaffolderApiRef" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public (undocumented) +// @public export const scaffolderApiRef: ApiRef; -// Warning: (ae-missing-release-tag) "ScaffolderClient" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public (undocumented) +// @public export class ScaffolderClient implements ScaffolderApi { constructor(options: { discoveryApi: DiscoveryApi; identityApi: IdentityApi; + fetchApi?: FetchApi; scmIntegrationsApi: ScmIntegrationRegistry; useLongPollingLogs?: boolean; }); @@ -246,7 +236,7 @@ export class ScaffolderClient implements ScaffolderApi { secrets?: Record, ): Promise; // (undocumented) - streamLogs(opts: { taskId: string; after?: number }): Observable; + streamLogs(options: { taskId: string; after?: number }): Observable; } // Warning: (ae-missing-release-tag) "ScaffolderFieldExtensions" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) diff --git a/plugins/scaffolder/src/api.ts b/plugins/scaffolder/src/api.ts index 7f096a02f7..77af1460c3 100644 --- a/plugins/scaffolder/src/api.ts +++ b/plugins/scaffolder/src/api.ts @@ -18,6 +18,7 @@ import { EntityName } from '@backstage/catalog-model'; import { createApiRef, DiscoveryApi, + FetchApi, IdentityApi, } from '@backstage/core-plugin-api'; import { ResponseError } from '@backstage/errors'; @@ -28,6 +29,11 @@ import qs from 'qs'; import ObservableImpl from 'zen-observable'; import { ListActionsResponse, ScaffolderTask, Status } from './types'; +/** + * Utility API reference for the {@link ScaffolderApi}. + * + * @public + */ export const scaffolderApiRef = createApiRef({ id: 'plugin.scaffolder.service', }); @@ -58,6 +64,11 @@ export type CustomField = { validation: (data: JsonValue, field: FieldValidation) => void; }; +/** + * An API to interact with the scaffolder backend. + * + * @public + */ export interface ScaffolderApi { getTemplateParameterSchema( templateName: EntityName, @@ -86,29 +97,31 @@ export interface ScaffolderApi { // Returns a list of all installed actions. listActions(): Promise; - streamLogs({ - taskId, - after, - }: { - taskId: string; - after?: number; - }): Observable; + streamLogs(options: { taskId: string; after?: number }): Observable; } +/** + * An API to interact with the scaffolder backend. + * + * @public + */ export class ScaffolderClient implements ScaffolderApi { private readonly discoveryApi: DiscoveryApi; private readonly identityApi: IdentityApi; private readonly scmIntegrationsApi: ScmIntegrationRegistry; + private readonly fetchApi: FetchApi; private readonly useLongPollingLogs: boolean; constructor(options: { discoveryApi: DiscoveryApi; identityApi: IdentityApi; + fetchApi?: FetchApi; scmIntegrationsApi: ScmIntegrationRegistry; useLongPollingLogs?: boolean; }) { this.discoveryApi = options.discoveryApi; this.identityApi = options.identityApi; + this.fetchApi = options.fetchApi ?? { fetch }; this.scmIntegrationsApi = options.scmIntegrationsApi; this.useLongPollingLogs = options.useLongPollingLogs ?? false; } @@ -136,7 +149,7 @@ export class ScaffolderClient implements ScaffolderApi { .join('/'); const url = `${baseUrl}/v2/templates/${templatePath}/parameter-schema`; - const response = await fetch(url, { + const response = await this.fetchApi.fetch(url, { headers: { ...(token && { Authorization: `Bearer ${token}` }), }, @@ -165,7 +178,7 @@ export class ScaffolderClient implements ScaffolderApi { ): Promise { const { token } = await this.identityApi.getCredentials(); const url = `${await this.discoveryApi.getBaseUrl('scaffolder')}/v2/tasks`; - const response = await fetch(url, { + const response = await this.fetchApi.fetch(url, { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -192,7 +205,7 @@ export class ScaffolderClient implements ScaffolderApi { const { token } = await this.identityApi.getCredentials(); const baseUrl = await this.discoveryApi.getBaseUrl('scaffolder'); const url = `${baseUrl}/v2/tasks/${encodeURIComponent(taskId)}`; - const response = await fetch(url, { + const response = await this.fetchApi.fetch(url, { headers: token ? { Authorization: `Bearer ${token}` } : {}, }); @@ -203,12 +216,15 @@ export class ScaffolderClient implements ScaffolderApi { return await response.json(); } - streamLogs(opts: { taskId: string; after?: number }): Observable { + streamLogs(options: { + taskId: string; + after?: number; + }): Observable { if (this.useLongPollingLogs) { - return this.streamLogsPolling(opts); + return this.streamLogsPolling(options); } - return this.streamLogsEventStream(opts); + return this.streamLogsEventStream(options); } private streamLogsEventStream({ @@ -276,7 +292,7 @@ export class ScaffolderClient implements ScaffolderApi { const url = `${baseUrl}/v2/tasks/${encodeURIComponent( taskId, )}/events?${qs.stringify({ after })}`; - const response = await fetch(url); + const response = await this.fetchApi.fetch(url); if (!response.ok) { // wait for one second to not run into an @@ -307,7 +323,7 @@ export class ScaffolderClient implements ScaffolderApi { async listActions(): Promise { const baseUrl = await this.discoveryApi.getBaseUrl('scaffolder'); const { token } = await this.identityApi.getCredentials(); - const response = await fetch(`${baseUrl}/v2/actions`, { + const response = await this.fetchApi.fetch(`${baseUrl}/v2/actions`, { headers: token ? { Authorization: `Bearer ${token}` } : {}, }); diff --git a/plugins/scaffolder/src/plugin.ts b/plugins/scaffolder/src/plugin.ts index 02b36c4bb6..9221fc1bf3 100644 --- a/plugins/scaffolder/src/plugin.ts +++ b/plugins/scaffolder/src/plugin.ts @@ -33,6 +33,7 @@ import { createPlugin, createRoutableExtension, discoveryApiRef, + fetchApiRef, identityApiRef, } from '@backstage/core-plugin-api'; import { OwnedEntityPicker } from './components/fields/OwnedEntityPicker'; @@ -47,9 +48,15 @@ export const scaffolderPlugin = createPlugin({ discoveryApi: discoveryApiRef, identityApi: identityApiRef, scmIntegrationsApi: scmIntegrationsApiRef, + fetchApi: fetchApiRef, }, - factory: ({ discoveryApi, identityApi, scmIntegrationsApi }) => - new ScaffolderClient({ discoveryApi, identityApi, scmIntegrationsApi }), + factory: ({ discoveryApi, identityApi, scmIntegrationsApi, fetchApi }) => + new ScaffolderClient({ + discoveryApi, + identityApi, + scmIntegrationsApi, + fetchApi, + }), }), ], routes: { From 6bf7826258ee4e1f475b8728db207e882d5a7ea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 6 Jan 2022 19:46:33 +0100 Subject: [PATCH 3/6] add and use MockFetchApi MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/modern-knives-roll.md | 2 +- .changeset/shiny-garlics-cough.md | 5 ++ .changeset/sweet-candles-greet.md | 2 +- .../src/apis/definitions/FetchApi.ts | 3 + packages/test-utils/api-report.md | 14 ++++ packages/test-utils/package.json | 4 +- packages/test-utils/src/setupTests.ts | 1 + .../apis/FetchApi/MockFetchApi.test.ts | 78 +++++++++++++++++ .../testUtils/apis/FetchApi/MockFetchApi.ts | 84 +++++++++++++++++++ .../src/testUtils/apis/FetchApi/index.ts | 17 ++++ .../test-utils/src/testUtils/apis/index.ts | 1 + .../test-utils/src/testUtils/msw/index.ts | 15 +--- .../testUtils/msw/setupRequestMockHandlers.ts | 30 +++++++ plugins/scaffolder/api-report.md | 6 +- plugins/scaffolder/dev/index.tsx | 10 +-- plugins/scaffolder/src/api.test.ts | 8 +- plugins/scaffolder/src/api.ts | 34 ++------ plugins/scaffolder/src/plugin.ts | 5 +- plugins/techdocs/api-report.md | 7 +- plugins/techdocs/src/client.test.ts | 28 +++++-- plugins/techdocs/src/client.ts | 31 ++----- plugins/techdocs/src/plugin.ts | 4 +- plugins/techdocs/src/setupTests.ts | 1 - 23 files changed, 284 insertions(+), 106 deletions(-) create mode 100644 .changeset/shiny-garlics-cough.md create mode 100644 packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.test.ts create mode 100644 packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.ts create mode 100644 packages/test-utils/src/testUtils/apis/FetchApi/index.ts create mode 100644 packages/test-utils/src/testUtils/msw/setupRequestMockHandlers.ts diff --git a/.changeset/modern-knives-roll.md b/.changeset/modern-knives-roll.md index 4d69f285b2..337fc62ed7 100644 --- a/.changeset/modern-knives-roll.md +++ b/.changeset/modern-knives-roll.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-scaffolder': patch +'@backstage/plugin-scaffolder': minor --- Make `ScaffolderClient` use the `FetchApi`. diff --git a/.changeset/shiny-garlics-cough.md b/.changeset/shiny-garlics-cough.md new file mode 100644 index 0000000000..ea6634506f --- /dev/null +++ b/.changeset/shiny-garlics-cough.md @@ -0,0 +1,5 @@ +--- +'@backstage/test-utils': patch +--- + +Added a `MockFetchApi` diff --git a/.changeset/sweet-candles-greet.md b/.changeset/sweet-candles-greet.md index 1a83f7828a..9fd08c65d6 100644 --- a/.changeset/sweet-candles-greet.md +++ b/.changeset/sweet-candles-greet.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-techdocs': patch +'@backstage/plugin-techdocs': minor --- Make `TechDocsClient` and `TechDocsStorageClient` use the `FetchApi`. diff --git a/packages/core-plugin-api/src/apis/definitions/FetchApi.ts b/packages/core-plugin-api/src/apis/definitions/FetchApi.ts index 67513b113a..b1a4dbbd0b 100644 --- a/packages/core-plugin-api/src/apis/definitions/FetchApi.ts +++ b/packages/core-plugin-api/src/apis/definitions/FetchApi.ts @@ -23,6 +23,9 @@ import { ApiRef, createApiRef } from '../system'; * @public */ export type FetchApi = { + /** + * The `fetch` implementation. + */ fetch: typeof fetch; }; diff --git a/packages/test-utils/api-report.md b/packages/test-utils/api-report.md index 2039bc60b5..143c88a4c2 100644 --- a/packages/test-utils/api-report.md +++ b/packages/test-utils/api-report.md @@ -13,10 +13,13 @@ import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { ComponentType } from 'react'; import { Config } from '@backstage/config'; import { ConfigApi } from '@backstage/core-plugin-api'; +import crossFetch from 'cross-fetch'; import { ErrorApi } from '@backstage/core-plugin-api'; import { ErrorApiError } from '@backstage/core-plugin-api'; import { ErrorApiErrorContext } from '@backstage/core-plugin-api'; import { ExternalRouteRef } from '@backstage/core-plugin-api'; +import { FetchApi } from '@backstage/core-plugin-api'; +import { IdentityApi } from '@backstage/core-plugin-api'; import { JsonObject } from '@backstage/types'; import { JsonValue } from '@backstage/types'; import { Observable } from '@backstage/types'; @@ -117,6 +120,17 @@ export type MockErrorApiOptions = { collect?: boolean; }; +// @public +export class MockFetchApi implements FetchApi { + constructor(implementation?: typeof crossFetch); + // (undocumented) + get fetch(): typeof crossFetch; + setAuthorization(options?: { + identityApi?: Pick; + token?: string; + }): this; +} + // @public export class MockPermissionApi implements PermissionApi { constructor( diff --git a/packages/test-utils/package.json b/packages/test-utils/package.json index 32d7010f53..b3f2b65aa5 100644 --- a/packages/test-utils/package.json +++ b/packages/test-utils/package.json @@ -41,6 +41,7 @@ "@testing-library/jest-dom": "^5.10.1", "@testing-library/react": "^11.2.5", "@testing-library/user-event": "^13.1.8", + "cross-fetch": "^3.0.6", "react-router": "6.0.0-beta.0", "react-router-dom": "6.0.0-beta.0", "zen-observable": "^0.8.15" @@ -52,7 +53,8 @@ "devDependencies": { "@backstage/cli": "^0.12.0-next.0", "@types/jest": "^26.0.7", - "@types/node": "^14.14.32" + "@types/node": "^14.14.32", + "msw": "^0.35.0" }, "files": [ "dist" diff --git a/packages/test-utils/src/setupTests.ts b/packages/test-utils/src/setupTests.ts index 963c0f188b..c1d649f2ad 100644 --- a/packages/test-utils/src/setupTests.ts +++ b/packages/test-utils/src/setupTests.ts @@ -15,3 +15,4 @@ */ import '@testing-library/jest-dom'; +import 'cross-fetch/polyfill'; diff --git a/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.test.ts b/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.test.ts new file mode 100644 index 0000000000..8ae98b83df --- /dev/null +++ b/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.test.ts @@ -0,0 +1,78 @@ +/* + * Copyright 2022 The Backstage Authors + * + * 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 { rest } from 'msw'; +import { setupServer } from 'msw/node'; +import { setupRequestMockHandlers } from '../../msw'; +import { MockFetchApi } from './MockFetchApi'; + +describe('MockFetchApi', () => { + const worker = setupServer(); + setupRequestMockHandlers(worker); + + it('works with default constructor', async () => { + worker.use( + rest.get('http://example.com/data.json', (_, res, ctx) => + res(ctx.status(200), ctx.json({ a: 'foo' })), + ), + ); + const m = new MockFetchApi(); + const response = await m.fetch('http://example.com/data.json'); + await expect(response.json()).resolves.toEqual({ a: 'foo' }); + }); + + it('works with a mock implementation', async () => { + const inner = jest.fn(); + const m = new MockFetchApi(inner); + await m.fetch('http://example.com/data.json'); + expect(inner).lastCalledWith('http://example.com/data.json'); + }); + + describe('setAuthorization', () => { + it('works with the default', async () => { + const inner = jest.fn(); + const m = new MockFetchApi(inner).setAuthorization(); + await m.fetch('http://example.com/data.json'); + expect(inner.mock.calls[0][0].headers.get('authorization')).toBe( + 'Bearer mocked', + ); + }); + + it('works with a static token', async () => { + const inner = jest.fn(); + const m = new MockFetchApi(inner).setAuthorization({ token: 'hello' }); + await m.fetch('http://example.com/data.json'); + expect(inner.mock.calls[0][0].headers.get('authorization')).toBe( + 'Bearer hello', + ); + }); + + it('works with an identity api', async () => { + const inner = jest.fn(); + const m = new MockFetchApi(inner).setAuthorization({ + identityApi: { + async getCredentials() { + return { token: 'hello2' }; + }, + }, + }); + await m.fetch('http://example.com/data.json'); + expect(inner.mock.calls[0][0].headers.get('authorization')).toBe( + 'Bearer hello2', + ); + }); + }); +}); diff --git a/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.ts b/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.ts new file mode 100644 index 0000000000..7df9b62e0a --- /dev/null +++ b/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.ts @@ -0,0 +1,84 @@ +/* + * Copyright 2022 The Backstage Authors + * + * 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 { FetchApi, IdentityApi } from '@backstage/core-plugin-api'; +import crossFetch, { Request } from 'cross-fetch'; + +/** + * A test helper implementation of {@link @backstage/core-plugin-api#FetchApi}. + * + * @public + */ +export class MockFetchApi implements FetchApi { + #implementation: typeof crossFetch; + + /** + * Creates a {@link MockFetchApi}. + * + * @param mockImplementation - Here you can pass in a `jest.fn()` for example, + * if you want to track the calls being made through the + * {@link @backstage/core-plugin-api#MockFetchApi}. If you pass in no + * mock implementation, the created + * {@link @backstage/core-plugin-api#MockFetchApi} will make actual + * requests using the global `fetch`. + */ + constructor(implementation?: typeof crossFetch) { + this.#implementation = implementation ?? crossFetch; + } + + /** {@inheritdoc @backstage/core-plugin-api#FetchApi.fetch} */ + get fetch(): typeof crossFetch { + return this.#implementation; + } + + /** + * Adds token based Authorization headers to requests, basically simulating + * what {@link @backstage/core-app-api#FetchMiddlewares.injectIdentityAuth} + * does. + * + * @remarks + * + * You can supply either a static mock token or a mock identity API. If + * neither is given, the static token string "mocked" is used. + */ + setAuthorization(options?: { + identityApi?: Pick; + token?: string; + }): this { + const next = this.#implementation; + + const getToken = async () => { + if (options?.token) { + return options.token; + } else if (options?.identityApi) { + const { token } = await options.identityApi.getCredentials(); + return token; + } + return 'mocked'; + }; + + this.#implementation = async (input, init) => { + const request = new Request(input, init); + const token = await getToken(); + if (token && !request.headers.get('authorization')) { + request.headers.set('authorization', `Bearer ${token}`); + } + return next(request); + }; + + return this; + } +} diff --git a/packages/test-utils/src/testUtils/apis/FetchApi/index.ts b/packages/test-utils/src/testUtils/apis/FetchApi/index.ts new file mode 100644 index 0000000000..058d4c4b27 --- /dev/null +++ b/packages/test-utils/src/testUtils/apis/FetchApi/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2022 The Backstage Authors + * + * 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 { MockFetchApi } from './MockFetchApi'; diff --git a/packages/test-utils/src/testUtils/apis/index.ts b/packages/test-utils/src/testUtils/apis/index.ts index e59608d2b0..e122a4b432 100644 --- a/packages/test-utils/src/testUtils/apis/index.ts +++ b/packages/test-utils/src/testUtils/apis/index.ts @@ -17,5 +17,6 @@ export * from './AnalyticsApi'; export * from './ConfigApi'; export * from './ErrorApi'; +export * from './FetchApi'; export * from './PermissionApi'; export * from './StorageApi'; diff --git a/packages/test-utils/src/testUtils/msw/index.ts b/packages/test-utils/src/testUtils/msw/index.ts index 625ef0e0fc..29cbdf854a 100644 --- a/packages/test-utils/src/testUtils/msw/index.ts +++ b/packages/test-utils/src/testUtils/msw/index.ts @@ -14,17 +14,4 @@ * limitations under the License. */ -/** - * Sets up handlers for request mocking - * @public - * @param worker - service worker - */ -export function setupRequestMockHandlers(worker: { - listen: (t: any) => void; - close: () => void; - resetHandlers: () => void; -}) { - beforeAll(() => worker.listen({ onUnhandledRequest: 'error' })); - afterAll(() => worker.close()); - afterEach(() => worker.resetHandlers()); -} +export { setupRequestMockHandlers } from './setupRequestMockHandlers'; diff --git a/packages/test-utils/src/testUtils/msw/setupRequestMockHandlers.ts b/packages/test-utils/src/testUtils/msw/setupRequestMockHandlers.ts new file mode 100644 index 0000000000..625ef0e0fc --- /dev/null +++ b/packages/test-utils/src/testUtils/msw/setupRequestMockHandlers.ts @@ -0,0 +1,30 @@ +/* + * Copyright 2020 The Backstage Authors + * + * 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. + */ + +/** + * Sets up handlers for request mocking + * @public + * @param worker - service worker + */ +export function setupRequestMockHandlers(worker: { + listen: (t: any) => void; + close: () => void; + resetHandlers: () => void; +}) { + beforeAll(() => worker.listen({ onUnhandledRequest: 'error' })); + afterAll(() => worker.close()); + afterEach(() => worker.resetHandlers()); +} diff --git a/plugins/scaffolder/api-report.md b/plugins/scaffolder/api-report.md index 835712651d..b77fb5f939 100644 --- a/plugins/scaffolder/api-report.md +++ b/plugins/scaffolder/api-report.md @@ -19,7 +19,6 @@ import { FetchApi } from '@backstage/core-plugin-api'; import { FieldProps } from '@rjsf/core'; import { FieldValidation } from '@rjsf/core'; import { IconButton } from '@material-ui/core'; -import { IdentityApi } from '@backstage/core-plugin-api'; import { JsonObject } from '@backstage/types'; import { JSONSchema } from '@backstage/catalog-model'; import { Observable } from '@backstage/types'; @@ -188,8 +187,6 @@ export interface ScaffolderApi { templateName: EntityName, ): Promise; // Warning: (ae-forgotten-export) The symbol "ListActionsResponse" needs to be exported by the entry point index.d.ts - // - // (undocumented) listActions(): Promise; scaffold( templateName: string, @@ -209,8 +206,7 @@ export const scaffolderApiRef: ApiRef; export class ScaffolderClient implements ScaffolderApi { constructor(options: { discoveryApi: DiscoveryApi; - identityApi: IdentityApi; - fetchApi?: FetchApi; + fetchApi: FetchApi; scmIntegrationsApi: ScmIntegrationRegistry; useLongPollingLogs?: boolean; }); diff --git a/plugins/scaffolder/dev/index.tsx b/plugins/scaffolder/dev/index.tsx index f7c6aa62fc..78e6254e09 100644 --- a/plugins/scaffolder/dev/index.tsx +++ b/plugins/scaffolder/dev/index.tsx @@ -26,9 +26,8 @@ import React from 'react'; import { scaffolderApiRef, ScaffolderClient } from '../src'; import { ScaffolderPage } from '../src/plugin'; import { - configApiRef, discoveryApiRef, - identityApiRef, + fetchApiRef, storageApiRef, } from '@backstage/core-plugin-api'; import { CatalogEntityPage } from '@backstage/plugin-catalog'; @@ -52,12 +51,11 @@ createDevApp() api: scaffolderApiRef, deps: { discoveryApi: discoveryApiRef, - identityApi: identityApiRef, - configApi: configApiRef, + fetchApi: fetchApiRef, scmIntegrationsApi: scmIntegrationsApiRef, }, - factory: ({ discoveryApi, identityApi, scmIntegrationsApi }) => - new ScaffolderClient({ discoveryApi, identityApi, scmIntegrationsApi }), + factory: ({ discoveryApi, fetchApi, scmIntegrationsApi }) => + new ScaffolderClient({ discoveryApi, fetchApi, scmIntegrationsApi }), }) .addPage({ path: '/create', diff --git a/plugins/scaffolder/src/api.test.ts b/plugins/scaffolder/src/api.test.ts index 01463a6e1c..369722ec8f 100644 --- a/plugins/scaffolder/src/api.test.ts +++ b/plugins/scaffolder/src/api.test.ts @@ -16,7 +16,7 @@ import { ConfigReader } from '@backstage/core-app-api'; import { ScmIntegrations } from '@backstage/integration'; -import { setupRequestMockHandlers } from '@backstage/test-utils'; +import { MockFetchApi, setupRequestMockHandlers } from '@backstage/test-utils'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import { ScaffolderClient } from './api'; @@ -32,7 +32,7 @@ describe('api', () => { const mockBaseUrl = 'http://backstage/api'; const discoveryApi = { getBaseUrl: async () => mockBaseUrl }; - const identityApi = {} as any; + const fetchApi = new MockFetchApi(); const scmIntegrationsApi = ScmIntegrations.fromConfig( new ConfigReader({ integrations: { @@ -50,7 +50,7 @@ describe('api', () => { apiClient = new ScaffolderClient({ scmIntegrationsApi, discoveryApi, - identityApi, + fetchApi, }); }); @@ -126,7 +126,7 @@ describe('api', () => { apiClient = new ScaffolderClient({ scmIntegrationsApi, discoveryApi, - identityApi, + fetchApi, useLongPollingLogs: true, }); }); diff --git a/plugins/scaffolder/src/api.ts b/plugins/scaffolder/src/api.ts index 77af1460c3..62fc06a6db 100644 --- a/plugins/scaffolder/src/api.ts +++ b/plugins/scaffolder/src/api.ts @@ -19,7 +19,6 @@ import { createApiRef, DiscoveryApi, FetchApi, - IdentityApi, } from '@backstage/core-plugin-api'; import { ResponseError } from '@backstage/errors'; import { ScmIntegrationRegistry } from '@backstage/integration'; @@ -94,7 +93,9 @@ export interface ScaffolderApi { allowedHosts: string[]; }): Promise<{ type: string; title: string; host: string }[]>; - // Returns a list of all installed actions. + /** + * Returns a list of all installed actions. + */ listActions(): Promise; streamLogs(options: { taskId: string; after?: number }): Observable; @@ -107,20 +108,17 @@ export interface ScaffolderApi { */ export class ScaffolderClient implements ScaffolderApi { private readonly discoveryApi: DiscoveryApi; - private readonly identityApi: IdentityApi; private readonly scmIntegrationsApi: ScmIntegrationRegistry; private readonly fetchApi: FetchApi; private readonly useLongPollingLogs: boolean; constructor(options: { discoveryApi: DiscoveryApi; - identityApi: IdentityApi; - fetchApi?: FetchApi; + fetchApi: FetchApi; scmIntegrationsApi: ScmIntegrationRegistry; useLongPollingLogs?: boolean; }) { this.discoveryApi = options.discoveryApi; - this.identityApi = options.identityApi; this.fetchApi = options.fetchApi ?? { fetch }; this.scmIntegrationsApi = options.scmIntegrationsApi; this.useLongPollingLogs = options.useLongPollingLogs ?? false; @@ -142,19 +140,13 @@ export class ScaffolderClient implements ScaffolderApi { ): Promise { const { namespace, kind, name } = templateName; - const { token } = await this.identityApi.getCredentials(); const baseUrl = await this.discoveryApi.getBaseUrl('scaffolder'); const templatePath = [namespace, kind, name] .map(s => encodeURIComponent(s)) .join('/'); const url = `${baseUrl}/v2/templates/${templatePath}/parameter-schema`; - const response = await this.fetchApi.fetch(url, { - headers: { - ...(token && { Authorization: `Bearer ${token}` }), - }, - }); - + const response = await this.fetchApi.fetch(url); if (!response.ok) { throw await ResponseError.fromResponse(response); } @@ -176,13 +168,11 @@ export class ScaffolderClient implements ScaffolderApi { values: Record, secrets: Record = {}, ): Promise { - const { token } = await this.identityApi.getCredentials(); const url = `${await this.discoveryApi.getBaseUrl('scaffolder')}/v2/tasks`; const response = await this.fetchApi.fetch(url, { method: 'POST', headers: { 'Content-Type': 'application/json', - ...(token && { Authorization: `Bearer ${token}` }), }, body: JSON.stringify({ templateName, @@ -202,13 +192,10 @@ export class ScaffolderClient implements ScaffolderApi { } async getTask(taskId: string) { - const { token } = await this.identityApi.getCredentials(); const baseUrl = await this.discoveryApi.getBaseUrl('scaffolder'); const url = `${baseUrl}/v2/tasks/${encodeURIComponent(taskId)}`; - const response = await this.fetchApi.fetch(url, { - headers: token ? { Authorization: `Bearer ${token}` } : {}, - }); + const response = await this.fetchApi.fetch(url); if (!response.ok) { throw await ResponseError.fromResponse(response); } @@ -317,16 +304,9 @@ export class ScaffolderClient implements ScaffolderApi { }); } - /** - * @returns ListActionsResponse containing all registered actions. - */ async listActions(): Promise { const baseUrl = await this.discoveryApi.getBaseUrl('scaffolder'); - const { token } = await this.identityApi.getCredentials(); - const response = await this.fetchApi.fetch(`${baseUrl}/v2/actions`, { - headers: token ? { Authorization: `Bearer ${token}` } : {}, - }); - + const response = await this.fetchApi.fetch(`${baseUrl}/v2/actions`); if (!response.ok) { throw await ResponseError.fromResponse(response); } diff --git a/plugins/scaffolder/src/plugin.ts b/plugins/scaffolder/src/plugin.ts index 9221fc1bf3..ccf8d2ee34 100644 --- a/plugins/scaffolder/src/plugin.ts +++ b/plugins/scaffolder/src/plugin.ts @@ -34,7 +34,6 @@ import { createRoutableExtension, discoveryApiRef, fetchApiRef, - identityApiRef, } from '@backstage/core-plugin-api'; import { OwnedEntityPicker } from './components/fields/OwnedEntityPicker'; import { EntityTagsPicker } from './components/fields/EntityTagsPicker'; @@ -46,14 +45,12 @@ export const scaffolderPlugin = createPlugin({ api: scaffolderApiRef, deps: { discoveryApi: discoveryApiRef, - identityApi: identityApiRef, scmIntegrationsApi: scmIntegrationsApiRef, fetchApi: fetchApiRef, }, - factory: ({ discoveryApi, identityApi, scmIntegrationsApi, fetchApi }) => + factory: ({ discoveryApi, scmIntegrationsApi, fetchApi }) => new ScaffolderClient({ discoveryApi, - identityApi, scmIntegrationsApi, fetchApi, }), diff --git a/plugins/techdocs/api-report.md b/plugins/techdocs/api-report.md index 7aed7139db..691df74656 100644 --- a/plugins/techdocs/api-report.md +++ b/plugins/techdocs/api-report.md @@ -222,8 +222,7 @@ export class TechDocsClient implements TechDocsApi { constructor(options: { configApi: Config; discoveryApi: DiscoveryApi; - identityApi: IdentityApi; - fetchApi?: FetchApi; + fetchApi: FetchApi; }); // (undocumented) configApi: Config; @@ -233,8 +232,6 @@ export class TechDocsClient implements TechDocsApi { getApiOrigin(): Promise; getEntityMetadata(entityId: EntityName): Promise; getTechDocsMetadata(entityId: EntityName): Promise; - // (undocumented) - identityApi: IdentityApi; } // Warning: (ae-missing-release-tag) "TechDocsCustomHome" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) @@ -382,7 +379,7 @@ export class TechDocsStorageClient implements TechDocsStorageApi { configApi: Config; discoveryApi: DiscoveryApi; identityApi: IdentityApi; - fetchApi?: FetchApi; + fetchApi: FetchApi; }); // (undocumented) configApi: Config; diff --git a/plugins/techdocs/src/client.test.ts b/plugins/techdocs/src/client.test.ts index 076d90cdad..8ea2b772b2 100644 --- a/plugins/techdocs/src/client.test.ts +++ b/plugins/techdocs/src/client.test.ts @@ -14,11 +14,11 @@ * limitations under the License. */ -import { MockConfigApi } from '@backstage/test-utils'; import { UrlPatternDiscovery } from '@backstage/core-app-api'; import { IdentityApi } from '@backstage/core-plugin-api'; import { NotFoundError } from '@backstage/errors'; import { EventSourcePolyfill } from 'event-source-polyfill'; +import { MockConfigApi, MockFetchApi } from '@backstage/test-utils'; import { TechDocsStorageClient } from './client'; const MockedEventSource = EventSourcePolyfill as jest.MockedClass< @@ -36,17 +36,13 @@ const mockEntity = { describe('TechDocsStorageClient', () => { const mockBaseUrl = 'http://backstage:9191/api/techdocs'; const configApi = new MockConfigApi({ - techdocs: { - requestUrl: 'http://backstage:9191/api/techdocs', - }, + techdocs: { requestUrl: 'http://backstage:9191/api/techdocs' }, }); const discoveryApi = UrlPatternDiscovery.compile(mockBaseUrl); const identityApi: jest.Mocked = { - signOut: jest.fn(), - getProfileInfo: jest.fn(), - getBackstageIdentity: jest.fn(), getCredentials: jest.fn(), - }; + } as unknown as jest.Mocked; + const fetchApi = new MockFetchApi().setAuthorization({ identityApi }); beforeEach(() => { jest.resetAllMocks(); @@ -58,6 +54,7 @@ describe('TechDocsStorageClient', () => { configApi, discoveryApi, identityApi, + fetchApi, }); await expect( @@ -78,6 +75,7 @@ describe('TechDocsStorageClient', () => { configApi, discoveryApi, identityApi, + fetchApi, }); await expect( @@ -93,6 +91,7 @@ describe('TechDocsStorageClient', () => { configApi, discoveryApi, identityApi, + fetchApi, }); MockedEventSource.prototype.addEventListener.mockImplementation( @@ -103,6 +102,7 @@ describe('TechDocsStorageClient', () => { }, ); + identityApi.getCredentials.mockResolvedValue({}); await storageApi.syncEntityDocs(mockEntity); expect(MockedEventSource).toBeCalledWith( @@ -116,6 +116,7 @@ describe('TechDocsStorageClient', () => { configApi, discoveryApi, identityApi, + fetchApi, }); MockedEventSource.prototype.addEventListener.mockImplementation( @@ -127,7 +128,6 @@ describe('TechDocsStorageClient', () => { ); identityApi.getCredentials.mockResolvedValue({ token: 'token' }); - await storageApi.syncEntityDocs(mockEntity); expect(MockedEventSource).toBeCalledWith( @@ -141,6 +141,7 @@ describe('TechDocsStorageClient', () => { configApi, discoveryApi, identityApi, + fetchApi, }); MockedEventSource.prototype.addEventListener.mockImplementation( @@ -151,6 +152,7 @@ describe('TechDocsStorageClient', () => { }, ); + identityApi.getCredentials.mockResolvedValue({}); await expect(storageApi.syncEntityDocs(mockEntity)).resolves.toEqual( 'cached', ); @@ -161,6 +163,7 @@ describe('TechDocsStorageClient', () => { configApi, discoveryApi, identityApi, + fetchApi, }); MockedEventSource.prototype.addEventListener.mockImplementation( @@ -171,6 +174,7 @@ describe('TechDocsStorageClient', () => { }, ); + identityApi.getCredentials.mockResolvedValue({}); await expect(storageApi.syncEntityDocs(mockEntity)).resolves.toEqual( 'updated', ); @@ -181,6 +185,7 @@ describe('TechDocsStorageClient', () => { configApi, discoveryApi, identityApi, + fetchApi, }); MockedEventSource.prototype.addEventListener.mockImplementation( @@ -195,6 +200,7 @@ describe('TechDocsStorageClient', () => { }, ); + identityApi.getCredentials.mockResolvedValue({}); const logHandler = jest.fn(); await expect( storageApi.syncEntityDocs(mockEntity, logHandler), @@ -209,9 +215,11 @@ describe('TechDocsStorageClient', () => { configApi, discoveryApi, identityApi, + fetchApi, }); // we await later after we emitted the error + identityApi.getCredentials.mockResolvedValue({}); const promise = storageApi.syncEntityDocs(mockEntity).then(); // flush the event loop @@ -234,9 +242,11 @@ describe('TechDocsStorageClient', () => { configApi, discoveryApi, identityApi, + fetchApi, }); // we await later after we emitted the error + identityApi.getCredentials.mockResolvedValue({}); const promise = storageApi.syncEntityDocs(mockEntity).then(); // flush the event loop diff --git a/plugins/techdocs/src/client.ts b/plugins/techdocs/src/client.ts index 6f381a5bd9..eac5da0b6b 100644 --- a/plugins/techdocs/src/client.ts +++ b/plugins/techdocs/src/client.ts @@ -34,19 +34,16 @@ import { TechDocsEntityMetadata, TechDocsMetadata } from './types'; export class TechDocsClient implements TechDocsApi { public configApi: Config; public discoveryApi: DiscoveryApi; - public identityApi: IdentityApi; private fetchApi: FetchApi; constructor(options: { configApi: Config; discoveryApi: DiscoveryApi; - identityApi: IdentityApi; - fetchApi?: FetchApi; + fetchApi: FetchApi; }) { this.configApi = options.configApi; this.discoveryApi = options.discoveryApi; - this.identityApi = options.identityApi; - this.fetchApi = options.fetchApi ?? { fetch }; + this.fetchApi = options.fetchApi; } async getApiOrigin(): Promise { @@ -70,12 +67,7 @@ export class TechDocsClient implements TechDocsApi { const apiOrigin = await this.getApiOrigin(); const requestUrl = `${apiOrigin}/metadata/techdocs/${namespace}/${kind}/${name}`; - const { token } = await this.identityApi.getCredentials(); - - const request = await this.fetchApi.fetch(`${requestUrl}`, { - headers: token ? { Authorization: `Bearer ${token}` } : {}, - }); - + const request = await this.fetchApi.fetch(`${requestUrl}`); if (!request.ok) { throw await ResponseError.fromResponse(request); } @@ -98,12 +90,8 @@ export class TechDocsClient implements TechDocsApi { const apiOrigin = await this.getApiOrigin(); const requestUrl = `${apiOrigin}/metadata/entity/${namespace}/${kind}/${name}`; - const { token } = await this.identityApi.getCredentials(); - - const request = await this.fetchApi.fetch(`${requestUrl}`, { - headers: token ? { Authorization: `Bearer ${token}` } : {}, - }); + const request = await this.fetchApi.fetch(`${requestUrl}`); if (!request.ok) { throw await ResponseError.fromResponse(request); } @@ -114,11 +102,8 @@ export class TechDocsClient implements TechDocsApi { /** * API which talks to TechDocs storage to fetch files to render. -<<<<<<< HEAD -======= * * @public ->>>>>>> 31c54b8ea2 (Make the techdocs APIs use the FetchApi) */ export class TechDocsStorageClient implements TechDocsStorageApi { public configApi: Config; @@ -130,12 +115,12 @@ export class TechDocsStorageClient implements TechDocsStorageApi { configApi: Config; discoveryApi: DiscoveryApi; identityApi: IdentityApi; - fetchApi?: FetchApi; + fetchApi: FetchApi; }) { this.configApi = options.configApi; this.discoveryApi = options.discoveryApi; this.identityApi = options.identityApi; - this.fetchApi = options.fetchApi ?? { fetch }; + this.fetchApi = options.fetchApi; } async getApiOrigin(): Promise { @@ -169,13 +154,9 @@ export class TechDocsStorageClient implements TechDocsStorageApi { const storageUrl = await this.getStorageUrl(); const url = `${storageUrl}/${namespace}/${kind}/${name}/${path}`; - const { token } = await this.identityApi.getCredentials(); const request = await this.fetchApi.fetch( `${url.endsWith('/') ? url : `${url}/`}index.html`, - { - headers: token ? { Authorization: `Bearer ${token}` } : {}, - }, ); let errorMessage = ''; diff --git a/plugins/techdocs/src/plugin.ts b/plugins/techdocs/src/plugin.ts index 274d1904d8..a37ad61f80 100644 --- a/plugins/techdocs/src/plugin.ts +++ b/plugins/techdocs/src/plugin.ts @@ -56,14 +56,12 @@ export const techdocsPlugin = createPlugin({ deps: { configApi: configApiRef, discoveryApi: discoveryApiRef, - identityApi: identityApiRef, fetchApi: fetchApiRef, }, - factory: ({ configApi, discoveryApi, identityApi, fetchApi }) => + factory: ({ configApi, discoveryApi, fetchApi }) => new TechDocsClient({ configApi, discoveryApi, - identityApi, fetchApi, }), }), diff --git a/plugins/techdocs/src/setupTests.ts b/plugins/techdocs/src/setupTests.ts index c1d649f2ad..963c0f188b 100644 --- a/plugins/techdocs/src/setupTests.ts +++ b/plugins/techdocs/src/setupTests.ts @@ -15,4 +15,3 @@ */ import '@testing-library/jest-dom'; -import 'cross-fetch/polyfill'; From 19ea047412a3a5b209a03d457c1531a0b7a9e360 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 7 Jan 2022 17:28:48 +0100 Subject: [PATCH 4/6] use a one-argument constructor instead of setters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/test-utils/api-report.md | 19 ++- .../apis/FetchApi/MockFetchApi.test.ts | 27 ++-- .../testUtils/apis/FetchApi/MockFetchApi.ts | 149 ++++++++++++------ .../src/testUtils/apis/FetchApi/index.ts | 1 + plugins/techdocs/src/client.test.ts | 2 +- 5 files changed, 126 insertions(+), 72 deletions(-) diff --git a/packages/test-utils/api-report.md b/packages/test-utils/api-report.md index 143c88a4c2..fade844712 100644 --- a/packages/test-utils/api-report.md +++ b/packages/test-utils/api-report.md @@ -122,13 +122,22 @@ export type MockErrorApiOptions = { // @public export class MockFetchApi implements FetchApi { - constructor(implementation?: typeof crossFetch); + constructor(options?: MockFetchApiOptions); // (undocumented) get fetch(): typeof crossFetch; - setAuthorization(options?: { - identityApi?: Pick; - token?: string; - }): this; +} + +// @public +export interface MockFetchApiOptions { + authorization?: + | { + token: string; + } + | { + identityApi: Pick; + } + | undefined; + baseImplementation?: 'fetch' | 'none' | typeof crossFetch | undefined; } // @public diff --git a/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.test.ts b/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.test.ts index 8ae98b83df..bf81e0ace0 100644 --- a/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.test.ts +++ b/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.test.ts @@ -36,24 +36,18 @@ describe('MockFetchApi', () => { it('works with a mock implementation', async () => { const inner = jest.fn(); - const m = new MockFetchApi(inner); + const m = new MockFetchApi({ baseImplementation: inner }); await m.fetch('http://example.com/data.json'); expect(inner).lastCalledWith('http://example.com/data.json'); }); describe('setAuthorization', () => { - it('works with the default', async () => { - const inner = jest.fn(); - const m = new MockFetchApi(inner).setAuthorization(); - await m.fetch('http://example.com/data.json'); - expect(inner.mock.calls[0][0].headers.get('authorization')).toBe( - 'Bearer mocked', - ); - }); - it('works with a static token', async () => { const inner = jest.fn(); - const m = new MockFetchApi(inner).setAuthorization({ token: 'hello' }); + const m = new MockFetchApi({ + baseImplementation: inner, + authorization: { token: 'hello' }, + }); await m.fetch('http://example.com/data.json'); expect(inner.mock.calls[0][0].headers.get('authorization')).toBe( 'Bearer hello', @@ -62,10 +56,13 @@ describe('MockFetchApi', () => { it('works with an identity api', async () => { const inner = jest.fn(); - const m = new MockFetchApi(inner).setAuthorization({ - identityApi: { - async getCredentials() { - return { token: 'hello2' }; + const m = new MockFetchApi({ + baseImplementation: inner, + authorization: { + identityApi: { + async getCredentials() { + return { token: 'hello2' }; + }, }, }, }); diff --git a/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.ts b/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.ts index 7df9b62e0a..7afa5a38b1 100644 --- a/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.ts +++ b/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.ts @@ -15,7 +15,46 @@ */ import { FetchApi, IdentityApi } from '@backstage/core-plugin-api'; -import crossFetch, { Request } from 'cross-fetch'; +import crossFetch, { Request, Response } from 'cross-fetch'; + +/** + * The options given when constructing a {@link MockFetchApi}. + * + * @public + */ +export interface MockFetchApiOptions { + /** + * Define the underlying base `fetch` implementation. + * + * @defaultValue 'fetch' + * @remarks + * + * `'fetch'` uses the global `fetch` implementation to make real network + * requests. This is the default. + * + * `'none'` swallows all calls and makes no requests at all. + * + * You can also pass in any `fetch` compatible callback, such as a + * `jest.fn()`, if you want to use a custom implementation or to just track + * and assert on calls. + */ + baseImplementation?: 'fetch' | 'none' | typeof crossFetch | undefined; + + /** + * If defined, adds token based Authorization headers to requests, basically + * simulating what + * {@link @backstage/core-app-api#FetchMiddlewares.injectIdentityAuth} does. + * + * @defaultValue undefined + * @remarks + * + * You can supply either a static token or an identity API. + */ + authorization?: + | { token: string } + | { identityApi: Pick } + | undefined; +} /** * A test helper implementation of {@link @backstage/core-plugin-api#FetchApi}. @@ -23,62 +62,70 @@ import crossFetch, { Request } from 'cross-fetch'; * @public */ export class MockFetchApi implements FetchApi { - #implementation: typeof crossFetch; + private readonly implementation: typeof crossFetch; /** - * Creates a {@link MockFetchApi}. - * - * @param mockImplementation - Here you can pass in a `jest.fn()` for example, - * if you want to track the calls being made through the - * {@link @backstage/core-plugin-api#MockFetchApi}. If you pass in no - * mock implementation, the created - * {@link @backstage/core-plugin-api#MockFetchApi} will make actual - * requests using the global `fetch`. + * Creates a mock {@link @backstage/core-plugin-api#FetchApi}. */ - constructor(implementation?: typeof crossFetch) { - this.#implementation = implementation ?? crossFetch; + constructor(options?: MockFetchApiOptions) { + this.implementation = build(options); } /** {@inheritdoc @backstage/core-plugin-api#FetchApi.fetch} */ get fetch(): typeof crossFetch { - return this.#implementation; - } - - /** - * Adds token based Authorization headers to requests, basically simulating - * what {@link @backstage/core-app-api#FetchMiddlewares.injectIdentityAuth} - * does. - * - * @remarks - * - * You can supply either a static mock token or a mock identity API. If - * neither is given, the static token string "mocked" is used. - */ - setAuthorization(options?: { - identityApi?: Pick; - token?: string; - }): this { - const next = this.#implementation; - - const getToken = async () => { - if (options?.token) { - return options.token; - } else if (options?.identityApi) { - const { token } = await options.identityApi.getCredentials(); - return token; - } - return 'mocked'; - }; - - this.#implementation = async (input, init) => { - const request = new Request(input, init); - const token = await getToken(); - if (token && !request.headers.get('authorization')) { - request.headers.set('authorization', `Bearer ${token}`); - } - return next(request); - }; - - return this; + return this.implementation; } } + +// +// Helpers +// + +const dummyFetch: typeof crossFetch = () => Promise.resolve(new Response(null)); + +function build(options?: MockFetchApiOptions): typeof crossFetch { + let implementation = baseImplementation(options); + implementation = authorization(options, implementation); + return implementation; +} + +function baseImplementation( + options: MockFetchApiOptions | undefined, +): typeof crossFetch { + const implementation = options?.baseImplementation ?? 'fetch'; + if (implementation === 'fetch') { + return crossFetch; + } else if (implementation === 'none') { + return dummyFetch; + } + return implementation; +} + +function authorization( + options: MockFetchApiOptions | undefined, + next: typeof crossFetch, +): typeof crossFetch { + const auth = options?.authorization; + if (!auth) { + return next; + } + + const getToken = async () => { + if ('token' in auth) { + return auth.token; + } + const { token } = await auth.identityApi.getCredentials(); + return token; + }; + + return async (input, init) => { + const request = new Request(input, init); + if (!request.headers.get('authorization')) { + const token = await getToken(); + if (token) { + request.headers.set('authorization', `Bearer ${token}`); + } + } + return next(request); + }; +} diff --git a/packages/test-utils/src/testUtils/apis/FetchApi/index.ts b/packages/test-utils/src/testUtils/apis/FetchApi/index.ts index 058d4c4b27..5ac0c08ff1 100644 --- a/packages/test-utils/src/testUtils/apis/FetchApi/index.ts +++ b/packages/test-utils/src/testUtils/apis/FetchApi/index.ts @@ -15,3 +15,4 @@ */ export { MockFetchApi } from './MockFetchApi'; +export type { MockFetchApiOptions } from './MockFetchApi'; diff --git a/plugins/techdocs/src/client.test.ts b/plugins/techdocs/src/client.test.ts index 8ea2b772b2..460b104c33 100644 --- a/plugins/techdocs/src/client.test.ts +++ b/plugins/techdocs/src/client.test.ts @@ -42,7 +42,7 @@ describe('TechDocsStorageClient', () => { const identityApi: jest.Mocked = { getCredentials: jest.fn(), } as unknown as jest.Mocked; - const fetchApi = new MockFetchApi().setAuthorization({ identityApi }); + const fetchApi = new MockFetchApi({ authorization: { identityApi } }); beforeEach(() => { jest.resetAllMocks(); From fb565073ec95afa5871e761d08c3ae215f31e447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 7 Jan 2022 20:46:52 +0100 Subject: [PATCH 5/6] implement injectIdentityAuth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/hip-grapes-count.md | 5 + packages/core-app-api/api-report.md | 1 + .../FetchApi/FetchMiddlewares.ts | 8 +- ...dentityAuthInjectorFetchMiddleware.test.ts | 50 ++----- .../IdentityAuthInjectorFetchMiddleware.ts | 43 ++++-- packages/test-utils/api-report.md | 14 +- .../apis/FetchApi/MockFetchApi.test.ts | 44 ++++-- .../testUtils/apis/FetchApi/MockFetchApi.ts | 132 +++++++++++------- plugins/techdocs/api-report.md | 15 -- plugins/techdocs/src/client.test.ts | 2 +- 10 files changed, 177 insertions(+), 137 deletions(-) create mode 100644 .changeset/hip-grapes-count.md diff --git a/.changeset/hip-grapes-count.md b/.changeset/hip-grapes-count.md new file mode 100644 index 0000000000..7fb218e5d6 --- /dev/null +++ b/.changeset/hip-grapes-count.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': patch +--- + +Add an `allowUrl` callback option to `FetchMiddlewares.injectIdentityAuth` diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index b813d1a9a5..ba2ed8cb62 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -359,6 +359,7 @@ export class FetchMiddlewares { identityApi: IdentityApi; config?: Config; urlPrefixAllowlist?: string[]; + allowUrl?: (url: string) => boolean; header?: { name: string; value: (backstageToken: string) => string; diff --git a/packages/core-app-api/src/apis/implementations/FetchApi/FetchMiddlewares.ts b/packages/core-app-api/src/apis/implementations/FetchApi/FetchMiddlewares.ts index 1cf09ff20d..8073571a04 100644 --- a/packages/core-app-api/src/apis/implementations/FetchApi/FetchMiddlewares.ts +++ b/packages/core-app-api/src/apis/implementations/FetchApi/FetchMiddlewares.ts @@ -56,14 +56,16 @@ export class FetchMiddlewares { * * The header injection only happens on allowlisted URLs. Per default, if the * `config` option is passed in, the `backend.baseUrl` is allowlisted, unless - * the `urlPrefixAllowlist` option is passed in, in which case it takes - * precedence. If you pass in neither config nor an allowlist, the middleware - * will have no effect. + * the `urlPrefixAllowlist` or `allowUrl` options are passed in, in which case + * they take precedence. If you pass in neither config nor an + * allowlist/callback, the middleware will have no effect since effectively no + * request will match the (nonexistent) rules. */ static injectIdentityAuth(options: { identityApi: IdentityApi; config?: Config; urlPrefixAllowlist?: string[]; + allowUrl?: (url: string) => boolean; header?: { name: string; value: (backstageToken: string) => string; diff --git a/packages/core-app-api/src/apis/implementations/FetchApi/IdentityAuthInjectorFetchMiddleware.test.ts b/packages/core-app-api/src/apis/implementations/FetchApi/IdentityAuthInjectorFetchMiddleware.test.ts index b69a4b9f7a..1e29f2548a 100644 --- a/packages/core-app-api/src/apis/implementations/FetchApi/IdentityAuthInjectorFetchMiddleware.test.ts +++ b/packages/core-app-api/src/apis/implementations/FetchApi/IdentityAuthInjectorFetchMiddleware.test.ts @@ -23,7 +23,7 @@ describe('IdentityAuthInjectorFetchMiddleware', () => { const middleware = IdentityAuthInjectorFetchMiddleware.create({ identityApi: undefined as any, }); - expect(middleware.urlPrefixAllowlist).toEqual([]); + expect(middleware.allowUrl('anything')).toEqual(false); expect(middleware.headerName).toEqual('authorization'); expect(middleware.headerValue('t')).toEqual('Bearer t'); }); @@ -36,7 +36,9 @@ describe('IdentityAuthInjectorFetchMiddleware', () => { }), header: { name: 'auth', value: t => `${t}!` }, }); - expect(middleware.urlPrefixAllowlist).toEqual(['https://example.com/api']); + expect(middleware.allowUrl('https://example.com/api')).toEqual(true); + expect(middleware.allowUrl('https://example.com/api/sss')).toEqual(true); + expect(middleware.allowUrl('https://evil.com/api')).toEqual(false); expect(middleware.headerName).toEqual('auth'); expect(middleware.headerValue('t')).toEqual('t!'); }); @@ -49,10 +51,10 @@ describe('IdentityAuthInjectorFetchMiddleware', () => { }), urlPrefixAllowlist: ['https://a.com', 'http://b.com:8080/'], }); - expect(middleware.urlPrefixAllowlist).toEqual([ - 'https://a.com', - 'http://b.com:8080', - ]); + expect(middleware.allowUrl('https://a.com')).toEqual(true); + expect(middleware.allowUrl('https://a.com:8080')).toEqual(false); + expect(middleware.allowUrl('https://a.com/sss')).toEqual(true); + expect(middleware.allowUrl('http://b.com:8080')).toEqual(true); }); it('injects the header only when a token is available', async () => { @@ -63,7 +65,7 @@ describe('IdentityAuthInjectorFetchMiddleware', () => { const middleware = new IdentityAuthInjectorFetchMiddleware( identityApi, - ['https://example.com'], + () => true, 'Authorization', token => `Bearer ${token}`, ); @@ -95,7 +97,7 @@ describe('IdentityAuthInjectorFetchMiddleware', () => { const middleware = new IdentityAuthInjectorFetchMiddleware( identityApi, - ['https://example.com'], + () => true, 'Authorization', token => `Bearer ${token}`, ); @@ -118,36 +120,4 @@ describe('IdentityAuthInjectorFetchMiddleware', () => { ['authorization', 'do-not-clobber'], ]); }); - - it('does not affect requests outside the allowlist', async () => { - const identityApi = { - getCredentials: () => ({ token: 'token' }), - } as unknown as IdentityApi; - - const middleware = new IdentityAuthInjectorFetchMiddleware( - identityApi, - ['https://example.com:8080/root'], - 'Authorization', - token => `Bearer ${token}`, - ); - - const inner = jest.fn(); - const outer = middleware.apply(inner); - - await outer(new Request('https://example.com:8080/root')); - await outer(new Request('https://example.com:8080/root/sub')); - await outer(new Request('https://example.com:8080/root2')); - await outer(new Request('https://example.com/root')); - await outer(new Request('http://example.com:8080/root')); - await outer(new Request('https://example.com/root')); - - const no: string[][] = []; - const yes: string[][] = [['authorization', 'Bearer token']]; - expect([...inner.mock.calls[0][0].headers.entries()]).toEqual(yes); - expect([...inner.mock.calls[1][0].headers.entries()]).toEqual(yes); - expect([...inner.mock.calls[2][0].headers.entries()]).toEqual(no); - expect([...inner.mock.calls[3][0].headers.entries()]).toEqual(no); - expect([...inner.mock.calls[4][0].headers.entries()]).toEqual(no); - expect([...inner.mock.calls[5][0].headers.entries()]).toEqual(no); - }); }); diff --git a/packages/core-app-api/src/apis/implementations/FetchApi/IdentityAuthInjectorFetchMiddleware.ts b/packages/core-app-api/src/apis/implementations/FetchApi/IdentityAuthInjectorFetchMiddleware.ts index 46ada0a505..810d9e76c1 100644 --- a/packages/core-app-api/src/apis/implementations/FetchApi/IdentityAuthInjectorFetchMiddleware.ts +++ b/packages/core-app-api/src/apis/implementations/FetchApi/IdentityAuthInjectorFetchMiddleware.ts @@ -27,24 +27,19 @@ export class IdentityAuthInjectorFetchMiddleware implements FetchMiddleware { identityApi: IdentityApi; config?: Config; urlPrefixAllowlist?: string[]; + allowUrl?: (url: string) => boolean; header?: { name: string; value: (backstageToken: string) => string; }; }): IdentityAuthInjectorFetchMiddleware { - const allowlist: string[] = []; - if (options.urlPrefixAllowlist) { - allowlist.push(...options.urlPrefixAllowlist); - } else if (options.config) { - allowlist.push(options.config.getString('backend.baseUrl')); - } - + const matcher = buildMatcher(options); const headerName = options.header?.name || 'authorization'; const headerValue = options.header?.value || (token => `Bearer ${token}`); return new IdentityAuthInjectorFetchMiddleware( options.identityApi, - allowlist.map(prefix => prefix.replace(/\/$/, '')), + matcher, headerName, headerValue, ); @@ -52,7 +47,7 @@ export class IdentityAuthInjectorFetchMiddleware implements FetchMiddleware { constructor( public readonly identityApi: IdentityApi, - public readonly urlPrefixAllowlist: string[], + public readonly allowUrl: (url: string) => boolean, public readonly headerName: string, public readonly headerValue: (pluginId: string) => string, ) {} @@ -65,12 +60,9 @@ export class IdentityAuthInjectorFetchMiddleware implements FetchMiddleware { const { token } = await this.identityApi.getCredentials(); if ( request.headers.get(this.headerName) || - !this.urlPrefixAllowlist.some( - prefix => - request.url === prefix || request.url.startsWith(`${prefix}/`), - ) || typeof token !== 'string' || - !token + !token || + !this.allowUrl(request.url) ) { return next(input, init); } @@ -80,3 +72,26 @@ export class IdentityAuthInjectorFetchMiddleware implements FetchMiddleware { }; } } + +function buildMatcher(options: { + config?: Config; + urlPrefixAllowlist?: string[]; + allowUrl?: (url: string) => boolean; +}): (url: string) => boolean { + if (options.allowUrl) { + return options.allowUrl; + } else if (options.urlPrefixAllowlist) { + return buildPrefixMatcher(options.urlPrefixAllowlist); + } else if (options.config) { + return buildPrefixMatcher([options.config.getString('backend.baseUrl')]); + } + return () => false; +} + +function buildPrefixMatcher(prefixes: string[]): (url: string) => boolean { + const trimmedPrefixes = prefixes.map(prefix => prefix.replace(/\/$/, '')); + return url => + trimmedPrefixes.some( + prefix => url === prefix || url.startsWith(`${prefix}/`), + ); +} diff --git a/packages/test-utils/api-report.md b/packages/test-utils/api-report.md index fade844712..4e5361ff80 100644 --- a/packages/test-utils/api-report.md +++ b/packages/test-utils/api-report.md @@ -14,6 +14,7 @@ import { ComponentType } from 'react'; import { Config } from '@backstage/config'; import { ConfigApi } from '@backstage/core-plugin-api'; import crossFetch from 'cross-fetch'; +import { DiscoveryApi } from '@backstage/core-plugin-api'; import { ErrorApi } from '@backstage/core-plugin-api'; import { ErrorApiError } from '@backstage/core-plugin-api'; import { ErrorApiErrorContext } from '@backstage/core-plugin-api'; @@ -129,15 +130,20 @@ export class MockFetchApi implements FetchApi { // @public export interface MockFetchApiOptions { - authorization?: + baseImplementation?: undefined | 'none' | typeof crossFetch; + injectIdentityAuth?: + | undefined | { token: string; } | { identityApi: Pick; - } - | undefined; - baseImplementation?: 'fetch' | 'none' | typeof crossFetch | undefined; + }; + resolvePluginProtocol?: + | undefined + | { + discoveryApi: Pick; + }; } // @public diff --git a/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.test.ts b/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.test.ts index bf81e0ace0..78f12a10f1 100644 --- a/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.test.ts +++ b/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.test.ts @@ -34,31 +34,51 @@ describe('MockFetchApi', () => { await expect(response.json()).resolves.toEqual({ a: 'foo' }); }); - it('works with a mock implementation', async () => { - const inner = jest.fn(); - const m = new MockFetchApi({ baseImplementation: inner }); - await m.fetch('http://example.com/data.json'); - expect(inner).lastCalledWith('http://example.com/data.json'); + describe('baseImplementation', () => { + it('works with a mock implementation', async () => { + const inner = jest.fn(); + const m = new MockFetchApi({ baseImplementation: inner }); + await m.fetch('http://example.com/data.json'); + expect(inner).lastCalledWith('http://example.com/data.json'); + }); }); - describe('setAuthorization', () => { - it('works with a static token', async () => { + describe('resolvePluginProtocol', () => { + it('works', async () => { const inner = jest.fn(); const m = new MockFetchApi({ baseImplementation: inner, - authorization: { token: 'hello' }, + resolvePluginProtocol: { + discoveryApi: { + getBaseUrl: async id => `https://blah.com/api/${id}`, + }, + }, + }); + await m.fetch('plugin://the-plugin/a/data.json'); + expect(inner.mock.calls[0][0]).toBe( + 'https://blah.com/api/the-plugin/a/data.json', + ); + }); + }); + + describe('injectIdentityAuth', () => { + it('works with token', async () => { + const inner = jest.fn(); + const m = new MockFetchApi({ + baseImplementation: inner, + injectIdentityAuth: { token: 'hello' }, }); await m.fetch('http://example.com/data.json'); - expect(inner.mock.calls[0][0].headers.get('authorization')).toBe( + expect(inner.mock.calls[0][0].headers?.get('authorization')).toBe( 'Bearer hello', ); }); - it('works with an identity api', async () => { + it('works with identityApi', async () => { const inner = jest.fn(); const m = new MockFetchApi({ baseImplementation: inner, - authorization: { + injectIdentityAuth: { identityApi: { async getCredentials() { return { token: 'hello2' }; @@ -67,7 +87,7 @@ describe('MockFetchApi', () => { }, }); await m.fetch('http://example.com/data.json'); - expect(inner.mock.calls[0][0].headers.get('authorization')).toBe( + expect(inner.mock.calls[0][0].headers?.get('authorization')).toBe( 'Bearer hello2', ); }); diff --git a/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.ts b/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.ts index 7afa5a38b1..3d02db0124 100644 --- a/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.ts +++ b/packages/test-utils/src/testUtils/apis/FetchApi/MockFetchApi.ts @@ -14,8 +14,17 @@ * limitations under the License. */ -import { FetchApi, IdentityApi } from '@backstage/core-plugin-api'; -import crossFetch, { Request, Response } from 'cross-fetch'; +import { + createFetchApi, + FetchMiddleware, + FetchMiddlewares, +} from '@backstage/core-app-api'; +import { + DiscoveryApi, + FetchApi, + IdentityApi, +} from '@backstage/core-plugin-api'; +import crossFetch, { Response } from 'cross-fetch'; /** * The options given when constructing a {@link MockFetchApi}. @@ -26,11 +35,11 @@ export interface MockFetchApiOptions { /** * Define the underlying base `fetch` implementation. * - * @defaultValue 'fetch' + * @defaultValue undefined * @remarks * - * `'fetch'` uses the global `fetch` implementation to make real network - * requests. This is the default. + * Leaving out this parameter or passing `undefined`, makes the API use the + * global `fetch` implementation to make real network requests. * * `'none'` swallows all calls and makes no requests at all. * @@ -38,22 +47,44 @@ export interface MockFetchApiOptions { * `jest.fn()`, if you want to use a custom implementation or to just track * and assert on calls. */ - baseImplementation?: 'fetch' | 'none' | typeof crossFetch | undefined; + baseImplementation?: undefined | 'none' | typeof crossFetch; /** - * If defined, adds token based Authorization headers to requests, basically + * Add translation from `plugin://` URLs to concrete http(s) URLs, basically * simulating what - * {@link @backstage/core-app-api#FetchMiddlewares.injectIdentityAuth} does. + * {@link @backstage/core-app-api#FetchMiddlewares.resolvePluginProtocol} + * does. * * @defaultValue undefined * @remarks * - * You can supply either a static token or an identity API. + * Leaving out this parameter or passing `undefined`, disables plugin protocol + * translation. + * + * To enable the feature, pass in a discovery API which is then used to + * resolve the URLs. */ - authorization?: + resolvePluginProtocol?: + | undefined + | { discoveryApi: Pick }; + + /** + * Add token based Authorization headers to requests, basically simulating + * what {@link @backstage/core-app-api#FetchMiddlewares.injectIdentityAuth} + * does. + * + * @defaultValue undefined + * @remarks + * + * Leaving out this parameter or passing `undefined`, disables auth injection. + * + * To enable the feature, pass in either a static token or an identity API + * which is queried on each request for a token. + */ + injectIdentityAuth?: + | undefined | { token: string } - | { identityApi: Pick } - | undefined; + | { identityApi: Pick }; } /** @@ -62,7 +93,7 @@ export interface MockFetchApiOptions { * @public */ export class MockFetchApi implements FetchApi { - private readonly implementation: typeof crossFetch; + private readonly implementation: FetchApi; /** * Creates a mock {@link @backstage/core-plugin-api#FetchApi}. @@ -73,7 +104,7 @@ export class MockFetchApi implements FetchApi { /** {@inheritdoc @backstage/core-plugin-api#FetchApi.fetch} */ get fetch(): typeof crossFetch { - return this.implementation; + return this.implementation.fetch; } } @@ -81,51 +112,56 @@ export class MockFetchApi implements FetchApi { // Helpers // -const dummyFetch: typeof crossFetch = () => Promise.resolve(new Response(null)); - -function build(options?: MockFetchApiOptions): typeof crossFetch { - let implementation = baseImplementation(options); - implementation = authorization(options, implementation); - return implementation; +function build(options?: MockFetchApiOptions): FetchApi { + return createFetchApi({ + baseImplementation: baseImplementation(options), + middleware: [ + resolvePluginProtocol(options), + injectIdentityAuth(options), + ].filter((x): x is FetchMiddleware => Boolean(x)), + }); } function baseImplementation( options: MockFetchApiOptions | undefined, ): typeof crossFetch { - const implementation = options?.baseImplementation ?? 'fetch'; - if (implementation === 'fetch') { + const implementation = options?.baseImplementation; + if (!implementation) { return crossFetch; } else if (implementation === 'none') { - return dummyFetch; + return () => Promise.resolve(new Response()); } return implementation; } -function authorization( - options: MockFetchApiOptions | undefined, - next: typeof crossFetch, -): typeof crossFetch { - const auth = options?.authorization; - if (!auth) { - return next; +function resolvePluginProtocol( + allOptions: MockFetchApiOptions | undefined, +): FetchMiddleware | undefined { + const options = allOptions?.resolvePluginProtocol; + if (!options) { + return undefined; } - const getToken = async () => { - if ('token' in auth) { - return auth.token; - } - const { token } = await auth.identityApi.getCredentials(); - return token; - }; - - return async (input, init) => { - const request = new Request(input, init); - if (!request.headers.get('authorization')) { - const token = await getToken(); - if (token) { - request.headers.set('authorization', `Bearer ${token}`); - } - } - return next(request); - }; + return FetchMiddlewares.resolvePluginProtocol({ + discoveryApi: options.discoveryApi, + }); +} + +function injectIdentityAuth( + allOptions: MockFetchApiOptions | undefined, +): FetchMiddleware | undefined { + const options = allOptions?.injectIdentityAuth; + if (!options) { + return undefined; + } + + const identityApi: Pick = + 'token' in options + ? { getCredentials: async () => ({ token: options.token }) } + : options.identityApi; + + return FetchMiddlewares.injectIdentityAuth({ + identityApi: identityApi as IdentityApi, + allowUrl: () => true, + }); } diff --git a/plugins/techdocs/api-report.md b/plugins/techdocs/api-report.md index 691df74656..9a25cb6375 100644 --- a/plugins/techdocs/api-report.md +++ b/plugins/techdocs/api-report.md @@ -358,21 +358,6 @@ export interface TechDocsStorageApi { // @public export const techdocsStorageApiRef: ApiRef; -// Warning: (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name -// Warning: (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name -// Warning: (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name -// Warning: (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name -// Warning: (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name -// Warning: (tsdoc-malformed-html-name) Invalid HTML element: Expecting an HTML name -// Warning: (tsdoc-malformed-html-name) Invalid HTML element: A space is not allowed here -// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag -// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag -// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag -// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag -// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag -// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag -// Warning: (tsdoc-escape-greater-than) The ">" character should be escaped using a backslash to avoid confusion with an HTML tag -// // @public export class TechDocsStorageClient implements TechDocsStorageApi { constructor(options: { diff --git a/plugins/techdocs/src/client.test.ts b/plugins/techdocs/src/client.test.ts index 460b104c33..a88c724cc8 100644 --- a/plugins/techdocs/src/client.test.ts +++ b/plugins/techdocs/src/client.test.ts @@ -42,7 +42,7 @@ describe('TechDocsStorageClient', () => { const identityApi: jest.Mocked = { getCredentials: jest.fn(), } as unknown as jest.Mocked; - const fetchApi = new MockFetchApi({ authorization: { identityApi } }); + const fetchApi = new MockFetchApi({ injectIdentityAuth: { identityApi } }); beforeEach(() => { jest.resetAllMocks(); From 3aa2ae1f91bb5ca21d5edc4de3f2c649fee18d9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 11 Jan 2022 16:29:48 +0100 Subject: [PATCH 6/6] expand on the changesets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/modern-knives-roll.md | 46 ++++++++++++++++++- .changeset/sweet-candles-greet.md | 73 ++++++++++++++++++++++++++++++- 2 files changed, 117 insertions(+), 2 deletions(-) diff --git a/.changeset/modern-knives-roll.md b/.changeset/modern-knives-roll.md index 337fc62ed7..9af062838c 100644 --- a/.changeset/modern-knives-roll.md +++ b/.changeset/modern-knives-roll.md @@ -2,4 +2,48 @@ '@backstage/plugin-scaffolder': minor --- -Make `ScaffolderClient` use the `FetchApi`. +Make `ScaffolderClient` use the `FetchApi`. You now need to pass in an instance +of that API when constructing the client, if you create a custom instance in +your app. + +If you are replacing the factory: + +```diff ++import { fetchApiRef } from '@backstage/core-plugin-api'; + + createApiFactory({ + api: scaffolderApiRef, + deps: { + discoveryApi: discoveryApiRef, + scmIntegrationsApi: scmIntegrationsApiRef, +- identityApi: identityApiRef, ++ fetchApi: fetchApiRef, + }, + factory: ({ + discoveryApi, + scmIntegrationsApi, +- identityApi, ++ fetchApi, + }) => + new ScaffolderClient({ + discoveryApi, + scmIntegrationsApi, +- identityApi, ++ fetchApi, + }), + }), +``` + +If instantiating directly: + +```diff ++import { fetchApiRef } from '@backstage/core-plugin-api'; + ++const fetchApi = useApi(fetchApiRef); + const client = new ScaffolderClient({ + discoveryApi, + scmIntegrationsApi, +- identityApi, ++ fetchApi, + }), +``` diff --git a/.changeset/sweet-candles-greet.md b/.changeset/sweet-candles-greet.md index 9fd08c65d6..8ab38ca0da 100644 --- a/.changeset/sweet-candles-greet.md +++ b/.changeset/sweet-candles-greet.md @@ -2,4 +2,75 @@ '@backstage/plugin-techdocs': minor --- -Make `TechDocsClient` and `TechDocsStorageClient` use the `FetchApi`. +Make `TechDocsClient` and `TechDocsStorageClient` use the `FetchApi`. You now +need to pass in an instance of that API when constructing the client, if you +create a custom instance in your app. + +If you are replacing the factory: + +```diff ++import { fetchApiRef } from '@backstage/core-plugin-api'; + + createApiFactory({ + api: techdocsStorageApiRef, + deps: { + configApi: configApiRef, + discoveryApi: discoveryApiRef, + identityApi: identityApiRef, ++ fetchApi: fetchApiRef, + }, + factory: ({ + configApi, + discoveryApi, + identityApi, ++ fetchApi, + }) => + new TechDocsStorageClient({ + configApi, + discoveryApi, + identityApi, ++ fetchApi, + }), + }), + createApiFactory({ + api: techdocsApiRef, + deps: { + configApi: configApiRef, + discoveryApi: discoveryApiRef, +- identityApi: identityApiRef, ++ fetchApi: fetchApiRef, + }, + factory: ({ + configApi, + discoveryApi, +- identityApi, ++ fetchApi, + }) => + new TechDocsClient({ + configApi, + discoveryApi, +- identityApi, ++ fetchApi, + }), + }), +``` + +If instantiating directly: + +```diff ++import { fetchApiRef } from '@backstage/core-plugin-api'; + ++const fetchApi = useApi(fetchApiRef); + const storageClient = new TechDocsStorageClient({ + configApi, + discoveryApi, + identityApi, ++ fetchApi, + }); + const techdocsClient = new TechDocsClient({ + configApi, + discoveryApi, +- identityApi, ++ fetchApi, + }), +```