diff --git a/extensions/github-authentication/package.json b/extensions/github-authentication/package.json index ed024ee59b8..3306b4b9042 100644 --- a/extensions/github-authentication/package.json +++ b/extensions/github-authentication/package.json @@ -41,7 +41,7 @@ "id": "github-enterprise" } ], - "configuration": { + "configuration": [{ "title": "GitHub Enterprise Server Authentication Provider", "properties": { "github-enterprise.uri": { @@ -49,7 +49,17 @@ "description": "GitHub Enterprise Server URI" } } + }, + { + "title": "GitHub Authentication", + "properties": { + "github.experimental.multipleAccounts": { + "type": "boolean", + "description": "Experimental support for multiple GitHub accounts" + } + } } + ] }, "aiKey": "0c6ae279ed8443289764825290e4f9e2-1a736e7c-1324-4338-be46-fc2a58ae4d14-7255", "main": "./out/extension.js", diff --git a/extensions/github-authentication/src/github.ts b/extensions/github-authentication/src/github.ts index 08fb16730e6..949ab1154b1 100644 --- a/extensions/github-authentication/src/github.ts +++ b/extensions/github-authentication/src/github.ts @@ -97,6 +97,7 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid private readonly _keychain: Keychain; private readonly _accountsSeen = new Set(); private readonly _disposable: vscode.Disposable | undefined; + private _supportsMultipleAccounts = false; private _sessionsPromise: Promise; @@ -133,10 +134,20 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid return sessions; }); + this._supportsMultipleAccounts = vscode.workspace.getConfiguration('github.experimental').get('multipleAccounts', false); + this._disposable = vscode.Disposable.from( this._telemetryReporter, - vscode.authentication.registerAuthenticationProvider(type, this._githubServer.friendlyName, this, { supportsMultipleAccounts: false }), - this.context.secrets.onDidChange(() => this.checkForUpdates()) + vscode.authentication.registerAuthenticationProvider(type, this._githubServer.friendlyName, this, { supportsMultipleAccounts: this._supportsMultipleAccounts }), + this.context.secrets.onDidChange(() => this.checkForUpdates()), + vscode.workspace.onDidChangeConfiguration(async e => { + if (e.affectsConfiguration('github.experimental.multipleAccounts')) { + const result = await vscode.window.showInformationMessage(vscode.l10n.t('Please reload the window to apply the new setting.'), { modal: true }, vscode.l10n.t('Reload Window')); + if (result) { + vscode.commands.executeCommand('workbench.action.reloadWindow'); + } + } + }) ); } @@ -229,7 +240,7 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid const sessionPromises = sessionData.map(async (session: SessionData) => { // For GitHub scope list, order doesn't matter so we immediately sort the scopes const scopesStr = [...session.scopes].sort().join(' '); - if (scopesSeen.has(scopesStr)) { + if (!this._supportsMultipleAccounts && scopesSeen.has(scopesStr)) { return undefined; } let userInfo: { id: string; accountName: string } | undefined; @@ -301,28 +312,8 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid const sessions = await this._sessionsPromise; - let forcedLogin = options?.account?.label; - let backupLogin: string | undefined; - if (!forcedLogin) { - const accounts = new Set(sessions.map(session => session.account.label)); - this._logger.info(`Found ${accounts.size} accounts.`); - // This helps us tell GitHub that we're already logged in to an account/accounts - // and should probably use it. The user _can_ sign in to a different account - // if they want to, in the browser, but this is a good default for the happy path. - if (accounts.size > 1) { - // If there are multiple accounts, we should prompt the user to choose one. - const newAccount = vscode.l10n.t('New account...'); - const accountChoiceResult = await vscode.window.showQuickPick( - [...accounts, newAccount], - { placeHolder: vscode.l10n.t('Choose an account that you would like to log in to') } - ); - forcedLogin = accountChoiceResult === newAccount ? undefined : accountChoiceResult; - } else { - // If there is only one account, we can use that to seed the login, but - // we don't want to force the user to use it. - backupLogin = sessions[0]?.account.label; - } - } + const forcedLogin = options?.account?.label; + const backupLogin = sessions[0]?.account.label; this._logger.info(`Logging in with '${forcedLogin ? forcedLogin : 'any'}' account...`); const scopeString = sortedScopes.join(' '); @@ -356,7 +347,11 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid } this.afterSessionLoad(session); - const sessionIndex = sessions.findIndex(s => s.id === session.id || arrayEquals([...s.scopes].sort(), sortedScopes)); + const sessionIndex = sessions.findIndex( + this._supportsMultipleAccounts + ? s => s.account.id === session.account.id && arrayEquals([...s.scopes].sort(), sortedScopes) + : s => s.id === session.id || arrayEquals([...s.scopes].sort(), sortedScopes) + ); const removed = new Array(); if (sessionIndex > -1) { removed.push(...sessions.splice(sessionIndex, 1, session)); diff --git a/src/vs/workbench/api/browser/mainThreadAuthentication.ts b/src/vs/workbench/api/browser/mainThreadAuthentication.ts index d833eab2860..28ef23588c6 100644 --- a/src/vs/workbench/api/browser/mainThreadAuthentication.ts +++ b/src/vs/workbench/api/browser/mainThreadAuthentication.ts @@ -261,15 +261,7 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu } async $getAccounts(providerId: string): Promise> { - const sessions = await this.authenticationService.getSessions(providerId); - const accounts = new Array(); - const seenAccounts = new Set(); - for (const session of sessions) { - if (!seenAccounts.has(session.account.label)) { - seenAccounts.add(session.account.label); - accounts.push(session.account); - } - } + const accounts = await this.authenticationService.getAccounts(providerId); return accounts; } diff --git a/src/vs/workbench/services/authentication/browser/authenticationExtensionsService.ts b/src/vs/workbench/services/authentication/browser/authenticationExtensionsService.ts index 1cdb2d5a5ae..2b0a0e98872 100644 --- a/src/vs/workbench/services/authentication/browser/authenticationExtensionsService.ts +++ b/src/vs/workbench/services/authentication/browser/authenticationExtensionsService.ts @@ -15,7 +15,7 @@ import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storag import { IActivityService, NumberBadge } from 'vs/workbench/services/activity/common/activity'; import { IAuthenticationAccessService } from 'vs/workbench/services/authentication/browser/authenticationAccessService'; import { IAuthenticationUsageService } from 'vs/workbench/services/authentication/browser/authenticationUsageService'; -import { AuthenticationSession, IAuthenticationProvider, IAuthenticationService, IAuthenticationExtensionsService } from 'vs/workbench/services/authentication/common/authentication'; +import { AuthenticationSession, IAuthenticationProvider, IAuthenticationService, IAuthenticationExtensionsService, AuthenticationSessionAccount } from 'vs/workbench/services/authentication/common/authentication'; // OAuth2 spec prohibits space in a scope, so use that to join them. const SCOPESLIST_SEPARATOR = ' '; @@ -212,45 +212,52 @@ export class AuthenticationExtensionsService extends Disposable implements IAuth return result === SessionPromptChoice.Allow; } + /** + * This function should be used only when there are sessions to disambiguate. + */ async selectSession(providerId: string, extensionId: string, extensionName: string, scopes: string[], availableSessions: AuthenticationSession[]): Promise { - return new Promise((resolve, reject) => { - // This function should be used only when there are sessions to disambiguate. - if (!availableSessions.length) { - reject('No available sessions'); - return; + const allAccounts = await this._authenticationService.getAccounts(providerId); + if (!allAccounts.length) { + throw new Error('No accounts available'); + } + const quickPick = this.quickInputService.createQuickPick<{ label: string; session?: AuthenticationSession; account?: AuthenticationSessionAccount }>(); + quickPick.ignoreFocusOut = true; + const items: { label: string; session?: AuthenticationSession; account?: AuthenticationSessionAccount }[] = availableSessions.map(session => { + return { + label: session.account.label, + session: session + }; + }); + + // Add the additional accounts that have been logged into the provider but are + // don't have a session yet. + const accountsWithSessions = new Set(availableSessions.map(session => session.account.label)); + allAccounts.forEach(account => { + if (!accountsWithSessions.has(account.label)) { + items.push({ label: account.label, account }); } + }); + items.push({ label: nls.localize('useOtherAccount', "Sign in to another account") }); + quickPick.items = items; + quickPick.title = nls.localize( + { + key: 'selectAccount', + comment: ['The placeholder {0} is the name of an extension. {1} is the name of the type of account, such as Microsoft or GitHub.'] + }, + "The extension '{0}' wants to access a {1} account", + extensionName, + this._authenticationService.getProvider(providerId).label + ); + quickPick.placeholder = nls.localize('getSessionPlateholder', "Select an account for '{0}' to use or Esc to cancel", extensionName); - const quickPick = this.quickInputService.createQuickPick<{ label: string; session?: AuthenticationSession }>(); - quickPick.ignoreFocusOut = true; - const items: { label: string; session?: AuthenticationSession }[] = availableSessions.map(session => { - return { - label: session.account.label, - session: session - }; - }); - - items.push({ - label: nls.localize('useOtherAccount', "Sign in to another account") - }); - - quickPick.items = items; - - quickPick.title = nls.localize( - { - key: 'selectAccount', - comment: ['The placeholder {0} is the name of an extension. {1} is the name of the type of account, such as Microsoft or GitHub.'] - }, - "The extension '{0}' wants to access a {1} account", - extensionName, - this._authenticationService.getProvider(providerId).label); - quickPick.placeholder = nls.localize('getSessionPlateholder', "Select an account for '{0}' to use or Esc to cancel", extensionName); - + return await new Promise((resolve, reject) => { quickPick.onDidAccept(async _ => { quickPick.dispose(); let session = quickPick.selectedItems[0].session; if (!session) { + const account = quickPick.selectedItems[0].account; try { - session = await this._authenticationService.createSession(providerId, scopes); + session = await this._authenticationService.createSession(providerId, scopes, { account }); } catch (e) { reject(e); return; diff --git a/src/vs/workbench/services/authentication/browser/authenticationService.ts b/src/vs/workbench/services/authentication/browser/authenticationService.ts index d154337b9a5..c9ab3050421 100644 --- a/src/vs/workbench/services/authentication/browser/authenticationService.ts +++ b/src/vs/workbench/services/authentication/browser/authenticationService.ts @@ -164,6 +164,20 @@ export class AuthenticationService extends Disposable implements IAuthentication throw new Error(`No authentication provider '${id}' is currently registered.`); } + async getAccounts(id: string): Promise> { + // TODO: Cache this + const sessions = await this.getSessions(id); + const accounts = new Array(); + const seenAccounts = new Set(); + for (const session of sessions) { + if (!seenAccounts.has(session.account.label)) { + seenAccounts.add(session.account.label); + accounts.push(session.account); + } + } + return accounts; + } + async getSessions(id: string, scopes?: string[], account?: AuthenticationSessionAccount, activateImmediate: boolean = false): Promise> { const authProvider = this._authenticationProviders.get(id) || await this.tryActivateProvider(id, activateImmediate); if (authProvider) { diff --git a/src/vs/workbench/services/authentication/common/authentication.ts b/src/vs/workbench/services/authentication/common/authentication.ts index 1d99ffc059a..605de7f8466 100644 --- a/src/vs/workbench/services/authentication/common/authentication.ts +++ b/src/vs/workbench/services/authentication/common/authentication.ts @@ -129,6 +129,13 @@ export interface IAuthenticationService { */ getProvider(id: string): IAuthenticationProvider; + /** + * Gets all accounts that are currently logged in across all sessions + * @param id The id of the provider to ask for accounts + * @returns A promise that resolves to an array of accounts + */ + getAccounts(id: string): Promise>; + /** * Gets all sessions that satisfy the given scopes from the provider with the given id * @param id The id of the provider to ask for a session