From 3ceb4fb7fa8b7644cef737d48714046618b1a4fe Mon Sep 17 00:00:00 2001 From: Robin van Boven <497556+Beanow@users.noreply.github.com> Date: Fri, 29 Nov 2019 11:53:07 +0100 Subject: [PATCH] GitHub: update token validation function (#1471) Previously an inline check was used for this. It only accepted the personal access token format. This adds installation tokens as requested in #1461. With more complex logic, we'd benefit from tests. Therefore it's a separate function with a test suite. --- src/plugins/github/fetchGithubRepo.js | 12 ++- src/plugins/github/token.js | 47 ++++++++++++ src/plugins/github/token.test.js | 101 ++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 src/plugins/github/token.js create mode 100644 src/plugins/github/token.test.js diff --git a/src/plugins/github/fetchGithubRepo.js b/src/plugins/github/fetchGithubRepo.js index c901946..0f5f62a 100644 --- a/src/plugins/github/fetchGithubRepo.js +++ b/src/plugins/github/fetchGithubRepo.js @@ -17,6 +17,7 @@ import * as Schema from "../../graphql/schema"; import {BLACKLISTED_IDS} from "./blacklistedObjectIds"; import type {Repository} from "./graphqlTypes"; import schema from "./schema"; +import {validateToken} from "./token"; /** * Scrape data from a GitHub repo using the GitHub API. @@ -37,10 +38,15 @@ export default async function fetchGithubRepo( ): Promise { const {token, cacheDirectory} = options; - const validToken = /^[A-Fa-f0-9]{40}$/; - if (!validToken.test(token)) { - throw new Error(`Invalid token: ${token}`); + // Right now, only warn on likely to be bad tokens (see #1461). + // This lets us proceed to the GitHub API validating the token, + // while giving users instructions to remedy if it was their mistake. + try { + validateToken(token); + } catch (e) { + console.warn(`Warning: ${e}`); } + const postQueryWithToken = (payload) => postQuery(payload, token); const resolvedId: Schema.ObjectId = await resolveRepositoryGraphqlId( diff --git a/src/plugins/github/token.js b/src/plugins/github/token.js new file mode 100644 index 0000000..b533553 --- /dev/null +++ b/src/plugins/github/token.js @@ -0,0 +1,47 @@ +// @flow + +/** + * Validates a token against know formatting. + * Throws an error if it appears invalid. + * + * Personal access token + * https://help.github.com/en/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line + * + * Installation access token + * https://developer.github.com/v3/apps/#create-a-new-installation-token + */ +export function validateToken(token: string) { + const personalAccessTokenRE = /^[A-Fa-f0-9]{40}$/; + if (personalAccessTokenRE.test(token)) { + return; + } + + // We're currently being lenient with installation tokens, since we're not completely + // sure on the exact format. We're only warning on unexpected values but leave it up + // to the GitHub API to reject the token if it's actually invalid. + const installationAccessTokenRE = /^(v\d+)\.([A-Fa-f0-9]+)$/; + const matches = installationAccessTokenRE.exec(token); + if (matches != null) { + const [_, version, hexCode] = matches; + + if (version != "v1") { + console.warn( + `Warning: GitHub installation access token has an unexpected version "${version}".` + ); + } + + if (hexCode.length != 16) { + console.warn( + `Warning: GitHub installation access token has an unexpected hexadecimal component ` + + `length of ${hexCode.length}.` + ); + } + + return; + } + + throw new Error( + `The token supplied to $SOURCECRED_GITHUB_TOKEN doesn't match any format known to work.\n` + + `Please verify the token "${token}" is correct, or report a bug if you think it should work.` + ); +} diff --git a/src/plugins/github/token.test.js b/src/plugins/github/token.test.js new file mode 100644 index 0000000..786d77e --- /dev/null +++ b/src/plugins/github/token.test.js @@ -0,0 +1,101 @@ +// @flow + +import {validateToken} from "./token"; + +describe("plugins/github/token", () => { + describe("validateToken", () => { + function spyWarn(): JestMockFn<[string], void> { + return ((console.warn: any): JestMockFn); + } + beforeEach(() => { + jest.spyOn(console, "warn").mockImplementation(() => {}); + }); + afterEach(() => { + try { + expect(console.warn).not.toHaveBeenCalled(); + } finally { + spyWarn().mockRestore(); + } + }); + + it("should throw on empty tokens", () => { + // Given + const token = ""; + + // When + const fn = () => validateToken(token); + + // Then + expect(fn).toThrow( + "The token supplied to $SOURCECRED_GITHUB_TOKEN doesn't match any format known to work.\n" + + 'Please verify the token "" is correct, or report a bug if you think it should work.' + ); + }); + + it("should throw on an unknown format token", () => { + // Given + const token = "EXAMPLE-INVALID-TOKEN-1082369"; + + // When + const fn = () => validateToken(token); + + // Then + expect(fn).toThrow( + "The token supplied to $SOURCECRED_GITHUB_TOKEN doesn't match any format known to work.\n" + + 'Please verify the token "EXAMPLE-INVALID-TOKEN-1082369" is correct, or report a bug if you think it should work.' + ); + }); + + it("should accept a personal access token format", () => { + // Given + const token = "1bfb713d900c4962586ec615260b3902438b1d3c"; + + // When + validateToken(token); + + // Then + // Shouldn't throw. + }); + + it("should accept an installation access token format", () => { + // Given + const token = "v1.1bfb713d900c4962"; + + // When + validateToken(token); + + // Then + // Shouldn't throw. + }); + + it("should warn when installation access token has an unexpected version", () => { + // Given + const token = "v5.1bfb713d900c4962"; + + // When + validateToken(token); + + // Then + expect(console.warn).toHaveBeenCalledWith( + 'Warning: GitHub installation access token has an unexpected version "v5".' + ); + expect(console.warn).toHaveBeenCalledTimes(1); + spyWarn().mockReset(); + }); + + it("should warn when installation access token has an unexpected length", () => { + // Given + const token = "v1.1bfb713d900c49621bfb713d900c4962"; + + // When + validateToken(token); + + // Then + expect(console.warn).toHaveBeenCalledWith( + "Warning: GitHub installation access token has an unexpected hexadecimal component length of 32." + ); + expect(console.warn).toHaveBeenCalledTimes(1); + spyWarn().mockReset(); + }); + }); +});