Skip to content

Commit fab197b

Browse files
Copilotalexr00
andauthored
Respect preferred GitHub account across authentication scopes (#8940)
* Initial plan * Respect preferred account across auth scopes Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * Complete the account change cleanup * Fix test * CCR --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent 89a2698 commit fab197b

21 files changed

Lines changed: 540 additions & 71 deletions

src/extension.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ import { CopilotRemoteAgentManager } from './github/copilotRemoteAgent';
2727
import { CredentialStore } from './github/credentials';
2828
import { FolderRepositoryManager } from './github/folderRepositoryManager';
2929
import { FolderRepositoryManagerResolver } from './github/folderRepositoryManagerResolver';
30+
import { IssueOverviewPanel } from './github/issueOverview';
3031
import { OverviewRestorer } from './github/overviewRestorer';
32+
import { PullRequestOverviewPanel } from './github/pullRequestOverview';
3133
import { RepositoriesManager } from './github/repositoriesManager';
3234
import { registerBuiltinGitProvider, registerLiveShareGitProvider } from './gitProviders/api';
3335
import { GitHubContactServiceProvider } from './gitProviders/GitHubContactServiceProvider';
@@ -276,10 +278,21 @@ async function init(
276278
if (e.provider.id !== AuthProvider.github && e.provider.id !== AuthProvider.githubEnterprise) {
277279
return;
278280
}
281+
if (e.accountChanged) {
282+
IssueOverviewPanel.clearAll();
283+
PullRequestOverviewPanel.clearAll();
284+
activePrViewCoordinator.clearForAuthChange();
285+
createPrHelper.clearForAuthChange();
286+
const reviewsCleanup = reviewsManager.clearForAuthChange();
287+
issueStateManager.clearForAuthChange();
288+
notificationsManager.clear();
289+
reposManager.clearForAuthChange();
290+
await reviewsCleanup;
291+
}
279292
await reposManager.refreshRepositories();
280293
await Promise.all(reviewsManager.reviewManagers.map(reviewManager => reviewManager.updateState(true)));
281-
tree.refreshAll(true);
282-
await issueStateManager.refreshForAuthChange();
294+
reviewsManager.refreshPullRequestsTree(!e.accountChanged);
295+
await issueStateManager.refreshAfterAuthChange();
283296
notificationsManager.refresh();
284297
}));
285298

src/github/credentials.ts

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,28 @@ interface ExistingSession {
4949
scopes: string[];
5050
}
5151

52+
export function hasAccountChanged(currentAccountId: string | undefined, newSession: vscode.AuthenticationSession | undefined): boolean {
53+
return currentAccountId !== newSession?.account.id;
54+
}
55+
5256
export async function findExistingSession(
5357
authProviderId: AuthProvider,
5458
getSession: AuthenticationSessionGetter = (providerId, scopes, options) => vscode.authentication.getSession(providerId, scopes, options),
5559
): Promise<ExistingSession | undefined> {
56-
// Establish the preferred account with the normal scopes before looking for broader sessions.
57-
// Otherwise, a single broader session from another account can override the workspace preference.
60+
// Establish the preferred account across all scopes before looking for its best session.
61+
// A scope-specific lookup can otherwise fall back to the only account with those scopes.
62+
const preferredSession = await getSession(authProviderId, [], { silent: true });
63+
if (preferredSession) {
64+
const scopesInPreferenceOrder = [SCOPES_WITH_ADDITIONAL, SCOPES_OLD, SCOPES_OLDEST];
65+
for (const scopes of scopesInPreferenceOrder) {
66+
const session = await getSession(authProviderId, scopes, { silent: true, account: preferredSession.account });
67+
if (session) {
68+
return { session, scopes };
69+
}
70+
}
71+
return { session: preferredSession, scopes: [...preferredSession.scopes] };
72+
}
73+
5874
const scopePreferences = [
5975
{ scopes: SCOPES_OLD, broaderScopes: [SCOPES_WITH_ADDITIONAL] },
6076
{ scopes: SCOPES_OLDEST, broaderScopes: [SCOPES_WITH_ADDITIONAL, SCOPES_OLD] },
@@ -88,12 +104,18 @@ interface AuthResult {
88104
canceled: boolean;
89105
}
90106

107+
export interface CredentialStoreSessionsChangeEvent extends vscode.AuthenticationSessionsChangeEvent {
108+
accountChanged: boolean;
109+
}
110+
91111
export class CredentialStore extends Disposable {
92112
private static readonly ID = 'Authentication';
93113
private _githubAPI: GitHub | undefined;
94114
private _sessionId: string | undefined;
115+
private _accountId: string | undefined;
95116
private _githubEnterpriseAPI: GitHub | undefined;
96117
private _enterpriseSessionId: string | undefined;
118+
private _enterpriseAccountId: string | undefined;
97119
private _isInitialized: boolean = false;
98120
private _onDidInitialize: vscode.EventEmitter<void> = new vscode.EventEmitter();
99121
public readonly onDidInitialize: vscode.Event<void> = this._onDidInitialize.event;
@@ -108,7 +130,7 @@ export class CredentialStore extends Disposable {
108130
// is invalidated again soon after re-auth will still trigger another prompt.
109131
private static readonly AUTH_ERROR_COOLDOWN_MS = 60_000;
110132

111-
private _onDidChangeSessions: vscode.EventEmitter<vscode.AuthenticationSessionsChangeEvent> = new vscode.EventEmitter();
133+
private _onDidChangeSessions: vscode.EventEmitter<CredentialStoreSessionsChangeEvent> = new vscode.EventEmitter();
112134
public readonly onDidChangeSessions = this._onDidChangeSessions.event;
113135

114136
private _onDidGetSession: vscode.EventEmitter<void> = new vscode.EventEmitter();
@@ -130,6 +152,7 @@ export class CredentialStore extends Disposable {
130152
return;
131153
}
132154
let sessionChanged = false;
155+
let accountChanged = false;
133156
if (currentProvider) {
134157
const newSession = await this.getSession(currentProvider, { silent: true }, currentProvider === AuthProvider.github ? this._scopes : this._scopesEnterprise, false);
135158
const currentSessionId = currentProvider === AuthProvider.github ? this._sessionId : this._enterpriseSessionId;
@@ -138,11 +161,15 @@ export class CredentialStore extends Disposable {
138161
}
139162
sessionChanged = true;
140163
if (currentProvider === AuthProvider.github) {
164+
accountChanged = hasAccountChanged(this._accountId, newSession.session);
141165
this._githubAPI = undefined;
142166
this._sessionId = undefined;
167+
this._accountId = undefined;
143168
} else {
169+
accountChanged = hasAccountChanged(this._enterpriseAccountId, newSession.session);
144170
this._githubEnterpriseAPI = undefined;
145171
this._enterpriseSessionId = undefined;
172+
this._enterpriseAccountId = undefined;
146173
}
147174
}
148175
const promises: Promise<any>[] = [];
@@ -158,10 +185,10 @@ export class CredentialStore extends Disposable {
158185
if (this.isAnyAuthenticated()) {
159186
this._onDidGetSession.fire();
160187
if (sessionChanged && !this._isSamling) {
161-
this._onDidChangeSessions.fire(e);
188+
this._onDidChangeSessions.fire({ ...e, accountChanged });
162189
}
163190
} else if (!this._isSamling) {
164-
this._onDidChangeSessions.fire(e);
191+
this._onDidChangeSessions.fire({ ...e, accountChanged });
165192
}
166193
}
167194

@@ -196,6 +223,7 @@ export class CredentialStore extends Disposable {
196223
const github = await this.createHub(token, authProviderId);
197224
this._githubAPI = github;
198225
this._sessionId = 'environment-token';
226+
this._accountId = undefined;
199227
if (!this._isInitialized) {
200228
this._isInitialized = true;
201229
this._onDidInitialize.fire();
@@ -261,8 +289,10 @@ export class CredentialStore extends Disposable {
261289
if (session) {
262290
if (!isEnterprise(authProviderId)) {
263291
this._sessionId = session.id;
292+
this._accountId = session.account.id;
264293
} else {
265294
this._enterpriseSessionId = session.id;
295+
this._enterpriseAccountId = session.account.id;
266296
}
267297
let github: GitHub | undefined;
268298
try {
@@ -413,6 +443,10 @@ export class CredentialStore extends Disposable {
413443
return this._githubEnterpriseAPI;
414444
}
415445

446+
public getAccountId(authProviderId: AuthProvider): string | undefined {
447+
return isEnterprise(authProviderId) ? this._enterpriseAccountId : this._accountId;
448+
}
449+
416450
public areScopesOld(authProviderId: AuthProvider): boolean {
417451
if (!isEnterprise(authProviderId)) {
418452
return !this.allScopesIncluded(this._scopes, SCOPES_OLD);

0 commit comments

Comments
 (0)