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

Prefer the API route in GithubUrlReader when using GitHub Apps based auth

Browse Source 
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
This commit is contained in:
Fredrik Adelöw
2021-08-11 11:49:53 +02:00
parent 0c58dd73d0
commit ce19580212
7 changed files with 79 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/rare-pugs-exercise.md
+5
View File
@@ -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
.changeset/silent-dragons-fold.md
+7
View File
@@ -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.
packages/backend-common/src/reading/GithubUrlReader.ts
+9 -4
View File
@@ -91,15 +91,20 @@ export class GithubUrlReader implements UrlReader {
url: string,
options?: ReadUrlOptions,
): Promise<ReadUrlResponse> {
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',
},
packages/integration/api-report.md
+4 -2
View File
@@ -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<GithubCredentials>;
}
packages/integration/src/github/GithubCredentialsProvider.ts
+1 -5
View File
@@ -250,11 +250,7 @@ export class GithubCredentialsProvider {
}
return {
headers: token
? {
Authorization: `Bearer ${token}`,
}
: undefined,
headers: token ? { Authorization: `Bearer ${token}` } : undefined,
token,
type,
};
packages/integration/src/github/core.test.ts
+39 -4
View File
@@ -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');
});
packages/integration/src/github/core.ts
+14 -6
View File
@@ -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';