Also use credentials in non-canary mode

We want users to be able to download databases from private/internal
repositories without using canary mode. This will change the prompt to
ask for credentials in non-canary mode as well.
This commit is contained in:
Koen Vlaswinkel
2023-11-14 14:20:13 +01:00
parent 636f8f1b5f
commit 5d42cbc1c8
3 changed files with 154 additions and 133 deletions

View File

@@ -1,21 +1,19 @@
import { window } from "vscode";
import { DisposableObject } from "../common/disposable-object";
import { App } from "../common/app";
import { findGitHubRepositoryForWorkspace } from "./github-repository-finder";
import { redactableError } from "../common/errors";
import { asError, getErrorMessage } from "../common/helpers-pure";
import {
askForGitHubDatabaseDownload,
CodeqlDatabase,
findGitHubDatabasesForRepository,
promptGitHubDatabaseDownload,
downloadDatabaseFromGitHub,
} from "./github-database-prompt";
import {
GitHubDatabaseConfig,
GitHubDatabaseConfigListener,
isCanary,
} from "../config";
import { AppOctokit } from "../common/octokit";
import { GitHubDatabaseConfig, GitHubDatabaseConfigListener } from "../config";
import { DatabaseManager } from "./local-databases";
import { CodeQLCliServer } from "../codeql-cli/cli";
import { showNeverAskAgainDialog } from "../common/vscode/dialog";
export class GithubDatabaseModule extends DisposableObject {
private readonly config: GitHubDatabaseConfig;
@@ -93,11 +91,31 @@ export class GithubDatabaseModule extends DisposableObject {
return;
}
const credentials = isCanary() ? this.app.credentials : undefined;
const credentials = this.app.credentials;
const octokit = credentials
? await credentials.getOctokit()
: new AppOctokit();
const hasAccessToken = !!(await credentials.getExistingAccessToken());
// If the user does not have an access token, ask whether they want to connect.
if (!hasAccessToken) {
const answer = await showNeverAskAgainDialog(
"This repository has an origin (GitHub) that may have one or more CodeQL databases. Connect to GitHub and download any existing databases?",
false,
"Connect",
"Not now",
"Never",
);
if (answer === "Not now" || answer === undefined) {
return;
}
if (answer === "Never") {
await this.config.setDownload("never");
return;
}
}
const octokit = await credentials.getOctokit();
let databases: CodeqlDatabase[];
try {
@@ -121,15 +139,29 @@ export class GithubDatabaseModule extends DisposableObject {
}
if (databases.length === 0) {
// If the user didn't have an access token, they have already been prompted,
// so we should give feedback.
if (!hasAccessToken) {
void window.showInformationMessage(
"The GitHub repository does not have any CodeQL databases.",
);
}
return;
}
void promptGitHubDatabaseDownload(
// If the user already had an access token, first ask if they even want to download the DB.
if (hasAccessToken) {
if (!(await askForGitHubDatabaseDownload(databases, this.config))) {
return;
}
}
await downloadDatabaseFromGitHub(
octokit,
githubRepository.owner,
githubRepository.name,
databases,
this.config,
this.databaseManager,
this.databaseStoragePath,
this.cliServer,

View File

@@ -30,20 +30,13 @@ export async function findGitHubDatabasesForRepository(
}
/**
* Prompt the user to download a database from GitHub. This is a blocking method, so this should
* almost never be called with `await`.
* Ask whether the user wants to download a database from GitHub.
* @return true if the user wants to download a database, false otherwise.
*/
export async function promptGitHubDatabaseDownload(
octokit: Octokit,
owner: string,
repo: string,
export async function askForGitHubDatabaseDownload(
databases: CodeqlDatabase[],
config: GitHubDatabaseConfig,
databaseManager: DatabaseManager,
storagePath: string,
cliServer: CodeQLCliServer,
commandManager: AppCommandManager,
): Promise<void> {
): Promise<boolean> {
const languages = databases.map((database) => database.language);
const databasesMessage =
@@ -57,26 +50,45 @@ export async function promptGitHubDatabaseDownload(
const connectMessage =
databases.length === 1
? `Connect to GitHub and download the existing database?`
: `Connect to GitHub and download any existing databases?`;
? `Download the existing database from GitHub?`
: `Download any existing databases from GitHub?`;
const answer = await showNeverAskAgainDialog(
`${databasesMessage} ${connectMessage}`,
false,
"Connect",
"Download",
"Not now",
"Never",
);
if (answer === "Not now" || answer === undefined) {
return;
return false;
}
if (answer === "Never") {
await config.setDownload("never");
return;
return false;
}
return true;
}
/**
* Download a database from GitHub by asking the user for a language and then
* downloading the database for that language.
*/
export async function downloadDatabaseFromGitHub(
octokit: Octokit,
owner: string,
repo: string,
databases: CodeqlDatabase[],
databaseManager: DatabaseManager,
storagePath: string,
cliServer: CodeQLCliServer,
commandManager: AppCommandManager,
): Promise<void> {
const languages = databases.map((database) => database.language);
const language = await promptForLanguage(languages, undefined);
if (!language) {
return;

View File

@@ -2,8 +2,9 @@ import { faker } from "@faker-js/faker";
import { Octokit } from "@octokit/rest";
import { mockedObject } from "../../utils/mocking.helpers";
import {
askForGitHubDatabaseDownload,
CodeqlDatabase,
promptGitHubDatabaseDownload,
downloadDatabaseFromGitHub,
} from "../../../../src/databases/github-database-prompt";
import { DatabaseManager } from "../../../../src/databases/local-databases";
import { GitHubDatabaseConfig } from "../../../../src/config";
@@ -12,13 +13,86 @@ import { createMockCommandManager } from "../../../__mocks__/commandsMock";
import * as databaseFetcher from "../../../../src/databases/database-fetcher";
import * as dialog from "../../../../src/common/vscode/dialog";
describe("promptGitHubDatabaseDownload", () => {
describe("askForGitHubDatabaseDownload", () => {
const setDownload = jest.fn();
let config: GitHubDatabaseConfig;
const databases = [
mockedObject<CodeqlDatabase>({
language: "swift",
url: faker.internet.url({
protocol: "https",
}),
}),
];
let showNeverAskAgainDialogSpy: jest.SpiedFunction<
typeof dialog.showNeverAskAgainDialog
>;
beforeEach(() => {
config = mockedObject<GitHubDatabaseConfig>({
setDownload,
});
showNeverAskAgainDialogSpy = jest
.spyOn(dialog, "showNeverAskAgainDialog")
.mockResolvedValue("Connect");
});
describe("when answering not now to prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue("Not now");
});
it("returns false", async () => {
expect(await askForGitHubDatabaseDownload(databases, config)).toEqual(
false,
);
expect(setDownload).not.toHaveBeenCalled();
});
});
describe("when cancelling prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue(undefined);
});
it("returns false", async () => {
expect(await askForGitHubDatabaseDownload(databases, config)).toEqual(
false,
);
expect(setDownload).not.toHaveBeenCalled();
});
});
describe("when answering never to prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue("Never");
});
it("returns false", async () => {
expect(await askForGitHubDatabaseDownload(databases, config)).toEqual(
false,
);
});
it("sets the config to never", async () => {
await askForGitHubDatabaseDownload(databases, config);
expect(setDownload).toHaveBeenCalledWith("never");
});
});
});
describe("downloadDatabaseFromGitHub", () => {
let octokit: Octokit;
const owner = "github";
const repo = "codeql";
let databaseManager: DatabaseManager;
const setDownload = jest.fn();
let config: GitHubDatabaseConfig;
const storagePath = "/a/b/c/d";
let cliServer: CodeQLCliServer;
const commandManager = createMockCommandManager();
@@ -35,9 +109,6 @@ describe("promptGitHubDatabaseDownload", () => {
}),
];
let showNeverAskAgainDialogSpy: jest.SpiedFunction<
typeof dialog.showNeverAskAgainDialog
>;
let promptForLanguageSpy: jest.SpiedFunction<
typeof databaseFetcher.promptForLanguage
>;
@@ -48,14 +119,8 @@ describe("promptGitHubDatabaseDownload", () => {
beforeEach(() => {
octokit = mockedObject<Octokit>({});
databaseManager = mockedObject<DatabaseManager>({});
config = mockedObject<GitHubDatabaseConfig>({
setDownload,
});
cliServer = mockedObject<CodeQLCliServer>({});
showNeverAskAgainDialogSpy = jest
.spyOn(dialog, "showNeverAskAgainDialog")
.mockResolvedValue("Connect");
promptForLanguageSpy = jest
.spyOn(databaseFetcher, "promptForLanguage")
.mockResolvedValue(databases[0].language);
@@ -65,12 +130,11 @@ describe("promptGitHubDatabaseDownload", () => {
});
it("downloads the database", async () => {
await promptGitHubDatabaseDownload(
await downloadDatabaseFromGitHub(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
@@ -94,90 +158,6 @@ describe("promptGitHubDatabaseDownload", () => {
false,
);
expect(promptForLanguageSpy).toHaveBeenCalledWith(["swift"], undefined);
expect(config.setDownload).not.toHaveBeenCalled();
});
describe("when answering not now to prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue("Not now");
});
it("does not download the database", async () => {
await promptGitHubDatabaseDownload(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
commandManager,
);
expect(downloadGitHubDatabaseFromUrlSpy).not.toHaveBeenCalled();
});
});
describe("when cancelling prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue(undefined);
});
it("does not download the database", async () => {
await promptGitHubDatabaseDownload(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
commandManager,
);
expect(downloadGitHubDatabaseFromUrlSpy).not.toHaveBeenCalled();
});
});
describe("when answering never to prompt", () => {
beforeEach(() => {
showNeverAskAgainDialogSpy.mockResolvedValue("Never");
});
it("does not download the database", async () => {
await promptGitHubDatabaseDownload(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
commandManager,
);
expect(downloadGitHubDatabaseFromUrlSpy).not.toHaveBeenCalled();
});
it('sets the config to "never"', async () => {
await promptGitHubDatabaseDownload(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
commandManager,
);
expect(config.setDownload).toHaveBeenCalledTimes(1);
expect(config.setDownload).toHaveBeenCalledWith("never");
});
});
describe("when not selecting language", () => {
@@ -186,12 +166,11 @@ describe("promptGitHubDatabaseDownload", () => {
});
it("does not download the database", async () => {
await promptGitHubDatabaseDownload(
await downloadDatabaseFromGitHub(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
@@ -229,12 +208,11 @@ describe("promptGitHubDatabaseDownload", () => {
});
it("downloads the correct database", async () => {
await promptGitHubDatabaseDownload(
await downloadDatabaseFromGitHub(
octokit,
owner,
repo,
databases,
config,
databaseManager,
storagePath,
cliServer,
@@ -261,7 +239,6 @@ describe("promptGitHubDatabaseDownload", () => {
["swift", "go"],
undefined,
);
expect(config.setDownload).not.toHaveBeenCalled();
});
});
});