Skip to content

Commit

Permalink
WIP: Uses the wrong account when signed in with multiple GitHub.com a…
Browse files Browse the repository at this point in the history
…ccounts

Fixes #5159
  • Loading branch information
alexr00 committed Jan 30, 2024
1 parent de466c8 commit 25aa738
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 13 deletions.
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

0 comments on commit 25aa738

Please sign in to comment.