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

Exclude emails using app auth with GithubOrgReaderProcessor

Browse Source 
Co-authored-by: Ben Lambert <ben@blam.sh>
Signed-off-by: Johan Haals <johan.haals@gmail.com>
This commit is contained in:
Johan Haals
2021-05-21 14:21:58 +02:00
parent f2cb8c8b61
commit 0fd4ea4431
8 changed files with 167 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/giant-rivers-protect.md
+10
View File
@@ -0,0 +1,10 @@
---
'@backstage/integration': patch
'@backstage/plugin-catalog-backend': minor
---
Updates the `GithubCredentialsProvider` to return the token type, it can either be `token` or `app` depending on the authentication method.
Update the `GithubOrgReaderProcessor` NOT to query for email addresses if GitHub Apps is used for authentication, this is due to inconsistencies in the GitHub API when using server to server communications and installation tokens. https://github.community/t/api-v4-unable-to-retrieve-email-resource-not-accessible-by-integration/13831/4 for more info.
**Removes** deprecated GithubOrgReaderProcessor provider configuration(`catalog.processors.githubOrg`). If you're using the deprecated config section make sure to migrate to [integrations](https://backstage.io/docs/integrations/github/locations) instead.
packages/integration/src/github/GithubCredentialsProvider.test.ts
+4 -3
View File
@@ -81,13 +81,13 @@ describe('GithubCredentialsProvider tests', () => {
},
} as RestEndpointMethodTypes['apps']['createInstallationAccessToken']['response']);
const { token, headers } = await github.getCredentials({
const { token, headers, type } = await github.getCredentials({
url: 'https://github.com/backstage/foobar',
});
const { token: accessToken2 } = await github.getCredentials({
url: 'https://github.com/backstage/foobar',
});
expect(type).toEqual('app');
expect(token).toEqual('secret_token');
expect(token).toEqual(accessToken2);
expect(headers).toEqual({ Authorization: 'Bearer secret_token' });
@@ -102,6 +102,7 @@ describe('GithubCredentialsProvider tests', () => {
Authorization: 'Bearer hardcoded_token',
},
token: 'hardcoded_token',
type: 'token',
});
});
@@ -260,6 +261,6 @@ describe('GithubCredentialsProvider tests', () => {
githubProvider.getCredentials({
url: 'https://github.com/backstage',
}),
).resolves.toEqual({ headers: undefined, token: undefined });
).resolves.toEqual({ headers: undefined, token: undefined, type: 'token' });
});
});
packages/integration/src/github/GithubCredentialsProvider.ts
+6
View File
@@ -192,9 +192,12 @@ export class GithubAppCredentialsMux {
}
}
export type GithubCredentialType = 'app' | 'token';
export type GithubCredentials = {
headers?: { [name: string]: string };
token?: string;
type: GithubCredentialType;
};
// TODO: Possibly move this to a backend only package so that it's not used in the frontend by mistake
@@ -226,8 +229,10 @@ export class GithubCredentialsProvider {
const owner = parsed.owner || parsed.name;
const repo = parsed.owner ? parsed.name : undefined;
let type: 'app' | 'token' = 'app';
let token = await this.githubAppCredentialsMux.getAppToken(owner, repo);
if (!token) {
type = 'token';
token = this.token;
}
@@ -238,6 +243,7 @@ export class GithubCredentialsProvider {
}
: undefined,
token,
type,
};
}
}
packages/integration/src/github/index.ts
+1
View File
@@ -21,4 +21,5 @@ export {
export type { GitHubIntegrationConfig } from './config';
export { getGitHubFileFetchUrl, getGitHubRequestOptions } from './core';
export { GithubCredentialsProvider } from './GithubCredentialsProvider';
export type { GithubCredentialType } from './GithubCredentialsProvider';
export { GitHubIntegration } from './GitHubIntegration';
plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.test.ts
+114 -57
View File
@@ -13,15 +13,16 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
jest.mock('@octokit/graphql');
import { getVoidLogger } from '@backstage/backend-common';
import { LocationSpec } from '@backstage/catalog-model';
import {
GitHubIntegration,
ScmIntegrations,
ScmIntegrationsGroup,
GithubCredentialsProvider,
} from '@backstage/integration';
import { GithubOrgReaderProcessor, parseUrl } from './GithubOrgReaderProcessor';
import { graphql } from '@octokit/graphql';
import { ConfigReader } from '@backstage/config';
describe('GithubOrgReaderProcessor', () => {
describe('parseUrl', () => {
@@ -34,67 +35,27 @@ describe('GithubOrgReaderProcessor', () => {
});
describe('implementation', () => {
let integrations: ScmIntegrations;
let github: jest.Mocked<ScmIntegrationsGroup<GitHubIntegration>>;
const logger = getVoidLogger();
const integrations = ScmIntegrations.fromConfig(
new ConfigReader({
integrations: {
github: [
{
host: 'github.com',
},
],
},
}),
);
beforeEach(() => {
github = {
byHost: jest.fn(),
byUrl: jest.fn(),
list: jest.fn(),
};
integrations = ({
github,
} as Partial<ScmIntegrations>) as ScmIntegrations;
});
it('rejects unknown types', async () => {
const processor = new GithubOrgReaderProcessor({
providers: [
{
target: 'https://github.com',
apiBaseUrl: 'https://api.github.com',
},
],
integrations,
logger: getVoidLogger(),
});
const location: LocationSpec = {
type: 'not-github-org',
target: 'https://github.com',
};
await expect(
processor.readLocation(location, false, () => {}),
).resolves.toBeFalsy();
});
it('rejects unknown targets from providers', async () => {
const processor = new GithubOrgReaderProcessor({
providers: [
{
target: 'https://github.com',
apiBaseUrl: 'https://api.github.com',
},
],
integrations,
logger: getVoidLogger(),
});
const location: LocationSpec = {
type: 'github-org',
target: 'https://not.github.com/apa',
};
await expect(
processor.readLocation(location, false, () => {}),
).rejects.toThrow(
/There is no GitHub Org provider that matches https:\/\/not.github.com\/apa/,
);
jest.resetAllMocks();
});
it('rejects unknown targets from integrations', async () => {
const processor = new GithubOrgReaderProcessor({
providers: [],
integrations,
logger: getVoidLogger(),
logger,
});
const location: LocationSpec = {
type: 'github-org',
@@ -106,5 +67,101 @@ describe('GithubOrgReaderProcessor', () => {
/There is no GitHub Org provider that matches https:\/\/not.github.com\/apa/,
);
});
it('should not query for email addresses when GitHub Apps is used for authentication', async () => {
const mockGetCredentials = jest.fn().mockReturnValue({
headers: { token: 'blah' },
type: 'app',
});
const mockClient = jest.fn();
mockClient
.mockResolvedValueOnce({
organization: {
membersWithRole: { pageInfo: { hasNextPage: false }, nodes: [{}] },
},
})
.mockResolvedValueOnce({
organization: {
teams: {
pageInfo: { hasNextPage: false },
nodes: [
{ members: { pageInfo: { hasNextPage: false }, nodes: [{}] } },
],
},
},
});
(graphql.defaults as jest.Mock).mockReturnValue(mockClient);
jest.spyOn(GithubCredentialsProvider, 'create').mockReturnValue({
getCredentials: mockGetCredentials,
} as any);
const processor = new GithubOrgReaderProcessor({
integrations,
logger,
});
const location: LocationSpec = {
type: 'github-org',
target: 'https://github.com/backstage',
};
await processor.readLocation(location, false, () => {});
expect(mockClient).toHaveBeenCalledWith(
expect.stringContaining('@include(if: $email)'),
expect.objectContaining({ email: false }),
);
});
it('should query for email addresses when token is used for authentication', async () => {
const mockGetCredentials = jest.fn().mockReturnValue({
headers: { token: 'blah' },
type: 'token',
});
const mockClient = jest.fn();
mockClient
.mockResolvedValueOnce({
organization: {
membersWithRole: { pageInfo: { hasNextPage: false }, nodes: [{}] },
},
})
.mockResolvedValueOnce({
organization: {
teams: {
pageInfo: { hasNextPage: false },
nodes: [
{ members: { pageInfo: { hasNextPage: false }, nodes: [{}] } },
],
},
},
});
(graphql.defaults as jest.Mock).mockReturnValue(mockClient);
jest.spyOn(GithubCredentialsProvider, 'create').mockReturnValue({
getCredentials: mockGetCredentials,
} as any);
const processor = new GithubOrgReaderProcessor({
integrations,
logger,
});
const location: LocationSpec = {
type: 'github-org',
target: 'https://github.com/backstage',
};
await processor.readLocation(location, false, () => {});
expect(mockClient).toHaveBeenCalledWith(
expect.stringContaining('@include(if: $email)'),
expect.objectContaining({ email: true }),
);
});
});
});
plugins/catalog-backend/src/ingestion/processors/GithubOrgReaderProcessor.ts
+18 -63
View File
@@ -18,16 +18,12 @@ import { LocationSpec } from '@backstage/catalog-model';
import { Config } from '@backstage/config';
import {
GithubCredentialsProvider,
GithubCredentialType,
ScmIntegrations,
} from '@backstage/integration';
import { graphql } from '@octokit/graphql';
import { Logger } from 'winston';
import {
getOrganizationTeams,
getOrganizationUsers,
ProviderConfig,
readGithubConfig,
} from './github';
import { getOrganizationTeams, getOrganizationUsers } from './github';
import * as results from './results';
import { CatalogProcessor, CatalogProcessorEmit } from './types';
import { buildOrgHierarchy } from './util/org';
@@ -38,7 +34,6 @@ type GraphQL = typeof graphql;
* Extracts teams and users out of a GitHub org.
*/
export class GithubOrgReaderProcessor implements CatalogProcessor {
private readonly providers: ProviderConfig[];
private readonly integrations: ScmIntegrations;
private readonly logger: Logger;
@@ -47,17 +42,11 @@ export class GithubOrgReaderProcessor implements CatalogProcessor {
return new GithubOrgReaderProcessor({
...options,
providers: readGithubConfig(config),
integrations,
});
}
constructor(options: {
providers: ProviderConfig[];
integrations: ScmIntegrations;
logger: Logger;
}) {
this.providers = options.providers;
constructor(options: { integrations: ScmIntegrations; logger: Logger }) {
this.integrations = options.integrations;
this.logger = options.logger;
}
@@ -71,14 +60,14 @@ export class GithubOrgReaderProcessor implements CatalogProcessor {
return false;
}
const client = await this.createClient(location.target);
const { client, tokenType } = await this.createClient(location.target);
const { org } = parseUrl(location.target);
// Read out all of the raw data
const startTimestamp = Date.now();
this.logger.info('Reading GitHub users and groups');
const { users } = await getOrganizationUsers(client, org);
const { users } = await getOrganizationUsers(client, org, tokenType);
const { groups, groupMemberUsers } = await getOrganizationTeams(
client,
org,
@@ -112,65 +101,31 @@ export class GithubOrgReaderProcessor implements CatalogProcessor {
return true;
}
private async createClient(orgUrl: string): Promise<GraphQL> {
let client = await this.createClientFromIntegrations(orgUrl);
private async createClient(
orgUrl: string,
): Promise<{ client: GraphQL; tokenType: GithubCredentialType }> {
const gitHubConfig = this.integrations.github.byUrl(orgUrl)?.config;
if (!client) {
client = await this.createClientFromProvider(orgUrl);
}
if (!client) {
if (!gitHubConfig) {
throw new Error(
`There is no GitHub Org provider that matches ${orgUrl}. Please add a configuration for an integration or add an entry for it under catalog.processors.githubOrg.providers.`,
`There is no GitHub Org provider that matches ${orgUrl}. Please add a configuration for an integration.`,
);
}
return client;
}
private async createClientFromProvider(
orgUrl: string,
): Promise<GraphQL | undefined> {
const provider = this.providers.find(p =>
orgUrl.startsWith(`${p.target}/`),
);
if (!provider) {
return undefined;
}
this.logger.warn(
'GithubOrgReaderProcessor uses provider defined in catalog.processors.githubOrg.providers, migrate to integrations instead. See https://backstage.io/docs/integrations/github/locations',
);
return !provider.token
? graphql
: graphql.defaults({
baseUrl: provider.apiBaseUrl,
headers: {
authorization: `token ${provider.token}`,
},
});
}
private async createClientFromIntegrations(
orgUrl: string,
): Promise<GraphQL | undefined> {
const gitHubConfig = this.integrations.github.byUrl(orgUrl)?.config;
if (!gitHubConfig) {
return undefined;
}
const credentialsProvider = GithubCredentialsProvider.create(gitHubConfig);
const { headers } = await credentialsProvider.getCredentials({
const {
headers,
type: tokenType,
} = await credentialsProvider.getCredentials({
url: orgUrl,
});
return graphql.defaults({
const client = graphql.defaults({
baseUrl: gitHubConfig.apiBaseUrl,
headers,
});
return { client, tokenType };
}
}
plugins/catalog-backend/src/ingestion/processors/github/github.test.ts
+3 -1
View File
@@ -65,7 +65,9 @@ describe('github', () => {
graphqlMsw.query('users', (_req, res, ctx) => res(ctx.data(input))),
);
await expect(getOrganizationUsers(graphql, 'a')).resolves.toEqual(output);
await expect(
getOrganizationUsers(graphql, 'a', 'token'),
).resolves.toEqual(output);
});
});
plugins/catalog-backend/src/ingestion/processors/github/github.ts
+11 -3
View File
@@ -15,6 +15,7 @@
*/
import { GroupEntity, UserEntity } from '@backstage/catalog-model';
import { GithubCredentialType } from '@backstage/integration';
import { graphql } from '@octokit/graphql';
// Graphql types
@@ -77,13 +78,20 @@ export type Connection<T> = {
export async function getOrganizationUsers(
client: typeof graphql,
org: string,
tokenType: GithubCredentialType,
): Promise<{ users: UserEntity[] }> {
const query = `
query users($org: String!, $cursor: String) {
query users($org: String!, $email: Boolean!, $cursor: String) {
organization(login: $org) {
membersWithRole(first: 100, after: $cursor) {
pageInfo { hasNextPage, endCursor }
nodes { avatarUrl, bio, email, login, name }
nodes {
avatarUrl,
bio,
email @include(if: $email),
login,
name
}
}
}
}`;
@@ -119,7 +127,7 @@ export async function getOrganizationUsers(
query,
r => r.organization?.membersWithRole,
mapper,
{ org },
{ org, email: tokenType === 'token' },
);
return { users };