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();