Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Uses the wrong account when signed in with multiple GitHub.com accounts #5699

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -975,6 +975,11 @@
"title": "%command.pr.signinAndRefreshList.title%",
"category": "%command.pull.request.category%"
},
{
"command": "pr.signinAlternateAndRefreshList",
"title": "%command.pr.signinAndRefreshList.title%",
"category": "%command.pull.request.category%"
},
{
"command": "pr.configureRemotes",
"title": "%command.pr.configureRemotes.title%",
Expand Down Expand Up @@ -1664,6 +1669,10 @@
"command": "pr.signinAndRefreshList",
"when": "false"
},
{
"command": "pr.signinAlternateAndRefreshList",
"when": "false"
},
{
"command": "pr.copyCommitHash",
"when": "false"
Expand Down
14 changes: 11 additions & 3 deletions src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { NotificationProvider } from './github/notifications';
import { GHPRComment, GHPRCommentThread, TemporaryComment } from './github/prComment';
import { PullRequestModel } from './github/pullRequestModel';
import { PullRequestOverviewPanel } from './github/pullRequestOverview';
import { RepositoriesManager } from './github/repositoriesManager';
import { AuthenticationType, RepositoriesManager } from './github/repositoriesManager';
import { getIssuesUrl, getPullsUrl, isInCodespaces, vscodeDevPrLink } from './github/utils';
import { PullRequestsTreeDataProvider } from './view/prsTreeDataProvider';
import { ReviewCommentController } from './view/reviewCommentController';
Expand Down Expand Up @@ -864,13 +864,13 @@ export function registerCommands(

context.subscriptions.push(
vscode.commands.registerCommand('pr.signinNoEnterprise', async () => {
await reposManager.authenticate(false);
await reposManager.authenticate(AuthenticationType.GitHub);
}),
);

context.subscriptions.push(
vscode.commands.registerCommand('pr.signinenterprise', async () => {
await reposManager.authenticate(true);
await reposManager.authenticate(AuthenticationType.Enterprise);
}),
);

Expand All @@ -890,6 +890,14 @@ export function registerCommands(
}),
);

context.subscriptions.push(
vscode.commands.registerCommand('pr.signinAlternateAndRefreshList', async () => {
if (await reposManager.authenticate(AuthenticationType.AlternateGitHub)) {
vscode.commands.executeCommand('pr.refreshList');
}
}),
);

context.subscriptions.push(
vscode.commands.registerCommand('pr.configureRemotes', async () => {
return vscode.commands.executeCommand('workbench.action.openSettings', `@ext:${EXTENSION_ID} remotes`);
Expand Down
7 changes: 6 additions & 1 deletion src/github/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ export class CredentialStore implements vscode.Disposable {
}
}

public async login(authProviderId: AuthProvider): Promise<GitHub | undefined> {
public async login(authProviderId: AuthProvider, useAlternateAccount: boolean = false): Promise<GitHub | undefined> {
/* __GDPR__
"auth.start" : {}
*/
Expand All @@ -299,6 +299,11 @@ export class CredentialStore implements vscode.Disposable {
let retry: boolean = true;
let octokit: GitHub | undefined = undefined;
const sessionOptions: vscode.AuthenticationGetSessionOptions = { createIfNone: true };
if (useAlternateAccount) {
sessionOptions.clearSessionPreference = true;
sessionOptions.forceNewSession = true;
sessionOptions.createIfNone = undefined;
}
let isCanceled: boolean = false;
while (retry) {
try {
Expand Down
6 changes: 5 additions & 1 deletion src/github/folderRepositoryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { git } from '../gitProviders/gitCommands';
import { OctokitCommon } from './common';
import { ConflictModel } from './conflictGuide';
import { CredentialStore } from './credentials';
import { GitHubRepository, ItemsData, PullRequestData, TeamReviewerRefreshKind, ViewerPermission } from './githubRepository';
import { GitHubRepository, ItemsData, ItemsResponseError, PullRequestData, TeamReviewerRefreshKind, ViewerPermission } from './githubRepository';
import { PullRequestState, UserResponse } from './graphql';
import { IAccount, ILabel, IMilestone, IProject, IPullRequestsPagingOptions, Issue, ITeam, MergeMethod, PRType, PullRequestMergeability, RepoAccessAndMergeMethods, User } from './interface';
import { IssueModel } from './issueModel';
Expand All @@ -60,6 +60,7 @@ export interface ItemsResponseResult<T> {
items: T[];
hasMorePages: boolean;
hasUnsearchedRepositories: boolean;
error?: ItemsResponseError;
}

export class NoGitHubReposError extends Error {
Expand Down Expand Up @@ -969,6 +970,7 @@ export class FolderRepositoryManager implements vscode.Disposable {
if (page) {
itemData.items = itemData.items.concat(page.items);
itemData.hasMorePages = page.hasMorePages;
itemData.error = page.error ?? itemData.error;
}
};

Expand Down Expand Up @@ -1048,6 +1050,7 @@ export class FolderRepositoryManager implements vscode.Disposable {
items: itemData.items,
hasMorePages: pageInformation.hasMorePages,
hasUnsearchedRepositories: i < githubRepositories.length - 1,
error: itemData.error
};
}
}
Expand All @@ -1056,6 +1059,7 @@ export class FolderRepositoryManager implements vscode.Disposable {
items: itemData.items,
hasMorePages: false,
hasUnsearchedRepositories: false,
error: itemData.error
};
}

Expand Down
9 changes: 7 additions & 2 deletions src/github/githubRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,14 @@ export const PULL_REQUEST_PAGE_SIZE = 20;

const GRAPHQL_COMPONENT_ID = 'GraphQL';

export enum ItemsResponseError {
NotFound = 1
}

export interface ItemsData {
items: any[];
hasMorePages: boolean;
error?: ItemsResponseError;
}

export interface IssueData extends ItemsData {
Expand Down Expand Up @@ -529,15 +534,15 @@ export class GitHubRepository implements vscode.Disposable {
} catch (e) {
Logger.error(`Fetching all pull requests failed: ${e}`, GitHubRepository.ID);
if (e.code === 404) {
// not found
// not found, can also indicate that this is a private repo that the current user doesn't have access to.
vscode.window.showWarningMessage(
`Fetching pull requests for remote '${this.remote.remoteName}' failed, please check if the url ${this.remote.url} is valid.`,
);
return { items: [], hasMorePages: false, error: ItemsResponseError.NotFound };
} else {
throw e;
}
}
return undefined;
}

async getPullRequestForBranch(branch: string, headOwner: string): Promise<PullRequestModel | undefined> {
Expand Down
14 changes: 10 additions & 4 deletions src/github/repositoriesManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ export interface PullRequestDefaults {
base: string;
}

export enum AuthenticationType {
GitHub = 1,
Enterprise = 2,
AlternateGitHub = 3
}

export class RepositoriesManager implements vscode.Disposable {
static ID = 'RepositoriesManager';

Expand Down Expand Up @@ -181,14 +187,14 @@ export class RepositoriesManager implements vscode.Disposable {
this.state = ReposManagerState.Initializing;
}

async authenticate(enterprise?: boolean): Promise<boolean> {
if (enterprise === false) {
return !!this._credentialStore.login(AuthProvider.github);
async authenticate(authenticationType: AuthenticationType = AuthenticationType.GitHub): Promise<boolean> {
if (authenticationType !== AuthenticationType.Enterprise) {
return !!(await this._credentialStore.login(AuthProvider.github, authenticationType === AuthenticationType.AlternateGitHub));
}
const { dotComRemotes, enterpriseRemotes, unknownRemotes } = await findDotComAndEnterpriseRemotes(this.folderManagers);
const yes = vscode.l10n.t('Yes');

if (enterprise) {
if (authenticationType === AuthenticationType.Enterprise) {
const remoteToUse = getEnterpriseUri()?.toString() ?? (enterpriseRemotes.length ? enterpriseRemotes[0].normalizedHost : (unknownRemotes.length ? unknownRemotes[0].normalizedHost : undefined));
if (enterpriseRemotes.length === 0 && unknownRemotes.length === 0) {
Logger.appendLine(`Enterprise login selected, but no possible enterprise remotes discovered (${dotComRemotes.length} .com)`);
Expand Down
17 changes: 15 additions & 2 deletions src/view/treeNodes/categoryNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { PR_SETTINGS_NAMESPACE, QUERIES } from '../../common/settingKeys';
import { ITelemetry } from '../../common/telemetry';
import { formatError } from '../../common/utils';
import { FolderRepositoryManager, ItemsResponseResult } from '../../github/folderRepositoryManager';
import { ItemsResponseError } from '../../github/githubRepository';
import { PRType } from '../../github/interface';
import { NotificationProvider } from '../../github/notifications';
import { PullRequestModel } from '../../github/pullRequestModel';
Expand All @@ -25,6 +26,7 @@ export enum PRCategoryActionType {
NoRemotes,
NoMatchingRemotes,
ConfigureRemotes,
RepositoryNotFound
}

interface QueryInspect {
Expand Down Expand Up @@ -101,6 +103,14 @@ export class PRCategoryActionNode extends TreeNode implements vscode.TreeItem {
arguments: [],
};
break;
case PRCategoryActionType.RepositoryNotFound:
this.label = vscode.l10n.t('Repository not found. Try another account...');
this.command = {
title: vscode.l10n.t('Sign in'),
command: 'pr.signinAlternateAndRefreshList',
arguments: [],
};
break;
default:
break;
}
Expand Down Expand Up @@ -316,6 +326,7 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
let hasMorePages = false;
let hasUnsearchedRepositories = false;
let needLogin = false;
let repositoryNotFound = false;
if (this.type === PRType.LocalPullRequest) {
try {
this.prs = (await this._prsTreeModel.getLocalPullRequests(this._folderRepoManager)).items;
Expand All @@ -334,7 +345,9 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
response = await this._prsTreeModel.getPullRequestsForQuery(this._folderRepoManager, this.fetchNextPage, this._categoryQuery!);
break;
}
if (!this.fetchNextPage) {
if (response.error === ItemsResponseError.NotFound) {
repositoryNotFound = true;
} else if (!this.fetchNextPage) {
this.prs = response.items;
} else {
this.prs = this.prs.concat(response.items);
Expand Down Expand Up @@ -362,7 +375,7 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
this.children = nodes;
return nodes;
} else {
const category = needLogin ? PRCategoryActionType.Login : PRCategoryActionType.Empty;
const category = needLogin ? PRCategoryActionType.Login : (repositoryNotFound ? PRCategoryActionType.RepositoryNotFound : PRCategoryActionType.Empty);
const result = [new PRCategoryActionNode(this, category)];

this.children = result;
Expand Down