diff --git a/.changeset/rare-pugs-exercise.md b/.changeset/rare-pugs-exercise.md new file mode 100644 index 0000000000..e4071ee59c --- /dev/null +++ b/.changeset/rare-pugs-exercise.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Pass on credentials to the integrations package, so that it can properly pick the API route when using GitHub apps based auth diff --git a/.changeset/silent-dragons-fold.md b/.changeset/silent-dragons-fold.md new file mode 100644 index 0000000000..df5c271fb8 --- /dev/null +++ b/.changeset/silent-dragons-fold.md @@ -0,0 +1,7 @@ +--- +'@backstage/integration': minor +--- + +`getGitHubFileFetchUrl` and `getGitHubRequestOptions` now require a `credentials` argument. This is needed to address an issue where the raw route was chosen by the `UrlReader` when using GitHub Apps based auth. + +Deprecated the `getGitHubRequestOptions` function, which is no longer used internally. diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index 930c544287..40d36fb2a0 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -91,15 +91,20 @@ export class GithubUrlReader implements UrlReader { url: string, options?: ReadUrlOptions, ): Promise { - const ghUrl = getGitHubFileFetchUrl(url, this.integration.config); - const { headers } = await this.deps.credentialsProvider.getCredentials({ + const credentials = await this.deps.credentialsProvider.getCredentials({ url, }); + const ghUrl = getGitHubFileFetchUrl( + url, + this.integration.config, + credentials, + ); + let response: Response; try { - response = await fetch(ghUrl.toString(), { + response = await fetch(ghUrl, { headers: { - ...headers, + ...credentials?.headers, ...(options?.etag && { 'If-None-Match': options.etag }), Accept: 'application/vnd.github.v3.raw', }, diff --git a/packages/integration/api-report.md b/packages/integration/api-report.md index 160a0a12ca..52663f55fa 100644 --- a/packages/integration/api-report.md +++ b/packages/integration/api-report.md @@ -165,20 +165,23 @@ export function getBitbucketRequestOptions( // Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen +// Warning: (ae-forgotten-export) The symbol "GithubCredentials" needs to be exported by the entry point index.d.ts // Warning: (ae-missing-release-tag) "getGitHubFileFetchUrl" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public export function getGitHubFileFetchUrl( url: string, config: GitHubIntegrationConfig, + credentials: GithubCredentials, ): string; // Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // Warning: (ae-missing-release-tag) "getGitHubRequestOptions" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // -// @public +// @public @deprecated export function getGitHubRequestOptions( config: GitHubIntegrationConfig, + credentials: GithubCredentials, ): RequestInit; // Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen @@ -228,7 +231,6 @@ export class GithubCredentialsProvider { // Warning: (tsdoc-escape-right-brace) The "}" character should be escaped using a backslash to avoid confusion with a TSDoc inline tag // Warning: (tsdoc-malformed-inline-tag) Expecting a TSDoc tag starting with "{@" // Warning: (tsdoc-malformed-inline-tag) Expecting a TSDoc tag starting with "{@" - // Warning: (ae-forgotten-export) The symbol "GithubCredentials" needs to be exported by the entry point index.d.ts getCredentials(opts: { url: string }): Promise; } diff --git a/packages/integration/src/github/GithubCredentialsProvider.ts b/packages/integration/src/github/GithubCredentialsProvider.ts index a3938b7ba3..c7ba6408d9 100644 --- a/packages/integration/src/github/GithubCredentialsProvider.ts +++ b/packages/integration/src/github/GithubCredentialsProvider.ts @@ -250,11 +250,7 @@ export class GithubCredentialsProvider { } return { - headers: token - ? { - Authorization: `Bearer ${token}`, - } - : undefined, + headers: token ? { Authorization: `Bearer ${token}` } : undefined, token, type, }; diff --git a/packages/integration/src/github/core.test.ts b/packages/integration/src/github/core.test.ts index 2dab4ab4d5..53733b40d0 100644 --- a/packages/integration/src/github/core.test.ts +++ b/packages/integration/src/github/core.test.ts @@ -16,8 +16,25 @@ import { GitHubIntegrationConfig } from './config'; import { getGitHubFileFetchUrl, getGitHubRequestOptions } from './core'; +import { GithubCredentials } from './GithubCredentialsProvider'; describe('github core', () => { + const appCredentials: GithubCredentials = { + type: 'app', + token: 'A', + headers: {}, + }; + + const tokenCredentials: GithubCredentials = { + type: 'token', + token: 'A', + headers: {}, + }; + + const noCredentials: GithubCredentials = { + type: 'token', + }; + describe('getGitHubRequestOptions', () => { it('inserts a token when needed', () => { const withToken: GitHubIntegrationConfig = { @@ -30,10 +47,12 @@ describe('github core', () => { rawBaseUrl: '', }; expect( - (getGitHubRequestOptions(withToken).headers as any).Authorization, + (getGitHubRequestOptions(withToken, appCredentials).headers as any) + .Authorization, ).toEqual('token A'); expect( - (getGitHubRequestOptions(withoutToken).headers as any).Authorization, + (getGitHubRequestOptions(withoutToken, noCredentials).headers as any) + .Authorization, ).toBeUndefined(); }); }); @@ -41,7 +60,7 @@ describe('github core', () => { describe('getGitHubFileFetchUrl', () => { it('rejects targets that do not look like URLs', () => { const config: GitHubIntegrationConfig = { host: '', apiBaseUrl: '' }; - expect(() => getGitHubFileFetchUrl('a/b', config)).toThrow( + expect(() => getGitHubFileFetchUrl('a/b', config, noCredentials)).toThrow( /Incorrect URL: a\/b/, ); }); @@ -55,14 +74,16 @@ describe('github core', () => { getGitHubFileFetchUrl( 'https://github.com/a/b/blob/branchname/path/to/c.yaml', config, + appCredentials, ), ).toEqual( 'https://api.github.com/repos/a/b/contents/path/to/c.yaml?ref=branchname', ); expect( getGitHubFileFetchUrl( - 'https://ghe.mycompany.net/a/b/blob/branchname/path/to/c.yaml', + 'https://github.com/a/b/blob/branchname/path/to/c.yaml', config, + tokenCredentials, ), ).toEqual( 'https://api.github.com/repos/a/b/contents/path/to/c.yaml?ref=branchname', @@ -78,6 +99,16 @@ describe('github core', () => { getGitHubFileFetchUrl( 'https://ghe.mycompany.net/a/b/blob/branchname/path/to/c.yaml', config, + appCredentials, + ), + ).toEqual( + 'https://ghe.mycompany.net/api/v3/repos/a/b/contents/path/to/c.yaml?ref=branchname', + ); + expect( + getGitHubFileFetchUrl( + 'https://ghe.mycompany.net/a/b/blob/branchname/path/to/c.yaml', + config, + tokenCredentials, ), ).toEqual( 'https://ghe.mycompany.net/api/v3/repos/a/b/contents/path/to/c.yaml?ref=branchname', @@ -93,6 +124,7 @@ describe('github core', () => { getGitHubFileFetchUrl( 'https://github.com/a/b/tree/branchname/path/to/c.yaml', config, + tokenCredentials, ), ).toEqual( 'https://api.github.com/repos/a/b/contents/path/to/c.yaml?ref=branchname', @@ -108,6 +140,7 @@ describe('github core', () => { getGitHubFileFetchUrl( 'https://ghe.mycompany.net/a/b/tree/branchname/path/to/c.yaml', config, + tokenCredentials, ), ).toEqual( 'https://ghe.mycompany.net/api/v3/repos/a/b/contents/path/to/c.yaml?ref=branchname', @@ -123,6 +156,7 @@ describe('github core', () => { getGitHubFileFetchUrl( 'https://github.com/a/b/blob/branchname/path/to/c.yaml', config, + tokenCredentials, ), ).toEqual( 'https://raw.githubusercontent.com/a/b/branchname/path/to/c.yaml', @@ -138,6 +172,7 @@ describe('github core', () => { getGitHubFileFetchUrl( 'https://ghe.mycompany.net/a/b/blob/branchname/path/to/c.yaml', config, + tokenCredentials, ), ).toEqual('https://ghe.mycompany.net/raw/a/b/branchname/path/to/c.yaml'); }); diff --git a/packages/integration/src/github/core.ts b/packages/integration/src/github/core.ts index 4a069bcba4..49695ec5ac 100644 --- a/packages/integration/src/github/core.ts +++ b/packages/integration/src/github/core.ts @@ -16,6 +16,7 @@ import parseGitUrl from 'git-url-parse'; import { GitHubIntegrationConfig } from './config'; +import { GithubCredentials } from './GithubCredentialsProvider'; /** * Given a URL pointing to a file on a provider, returns a URL that is suitable @@ -32,6 +33,7 @@ import { GitHubIntegrationConfig } from './config'; export function getGitHubFileFetchUrl( url: string, config: GitHubIntegrationConfig, + credentials: GithubCredentials, ): string { try { const { owner, name, ref, filepathtype, filepath } = parseGitUrl(url); @@ -49,7 +51,7 @@ export function getGitHubFileFetchUrl( } const pathWithoutSlash = filepath.replace(/^\//, ''); - if (chooseEndpoint(config) === 'api') { + if (chooseEndpoint(config, credentials) === 'api') { return `${config.apiBaseUrl}/repos/${owner}/${name}/contents/${pathWithoutSlash}?ref=${ref}`; } return `${config.rawBaseUrl}/${owner}/${name}/${ref}/${pathWithoutSlash}`; @@ -61,25 +63,31 @@ export function getGitHubFileFetchUrl( /** * Gets the request options necessary to make requests to a given provider. * + * @deprecated This function is no longer used internally * @param config The relevant provider config */ export function getGitHubRequestOptions( config: GitHubIntegrationConfig, + credentials: GithubCredentials, ): RequestInit { const headers: HeadersInit = {}; - if (chooseEndpoint(config) === 'api') { + if (chooseEndpoint(config, credentials) === 'api') { headers.Accept = 'application/vnd.github.v3.raw'; } - if (config.token) { - headers.Authorization = `token ${config.token}`; + + if (credentials.token) { + headers.Authorization = `token ${credentials.token}`; } return { headers }; } -export function chooseEndpoint(config: GitHubIntegrationConfig): 'api' | 'raw' { - if (config.apiBaseUrl && (config.token || !config.rawBaseUrl)) { +export function chooseEndpoint( + config: GitHubIntegrationConfig, + credentials: GithubCredentials, +): 'api' | 'raw' { + if (config.apiBaseUrl && (credentials.token || !config.rawBaseUrl)) { return 'api'; } return 'raw';