Skip to content

Commit

Permalink
Make sure the same GitHub account is used until we support multiple G…
Browse files Browse the repository at this point in the history
…H accounts (#206847)

Fixes #203850
  • Loading branch information
TylerLeonhardt authored Mar 4, 2024
1 parent 5abb308 commit 191be39
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 12 deletions.
4 changes: 4 additions & 0 deletions extensions/github-authentication/src/common/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,7 @@ export const TIMED_OUT_ERROR = 'Timed out';
// These error messages are internal and should not be shown to the user in any way.
export const USER_CANCELLATION_ERROR = 'User Cancelled';
export const NETWORK_ERROR = 'network error';

// This is the error message that we throw if the login was cancelled for any reason. Extensions
// calling `getSession` can handle this error to know that the user cancelled the login.
export const CANCELLATION_ERROR = 'Cancelled';
13 changes: 11 additions & 2 deletions extensions/github-authentication/src/flows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ interface IFlowTriggerOptions {
callbackUri: Uri;
uriHandler: UriEventHandler;
enterpriseUri?: Uri;
existingLogin?: string;
}

interface IFlow {
Expand Down Expand Up @@ -149,7 +150,8 @@ const allFlows: IFlow[] = [
nonce,
callbackUri,
uriHandler,
enterpriseUri
enterpriseUri,
existingLogin
}: IFlowTriggerOptions): Promise<string> {
logger.info(`Trying without local server... (${scopes})`);
return await window.withProgress<string>({
Expand All @@ -169,6 +171,9 @@ const allFlows: IFlow[] = [
['scope', scopes],
['state', encodeURIComponent(callbackUri.toString(true))]
]);
if (existingLogin) {
searchParams.append('login', existingLogin);
}

// The extra toString, parse is apparently needed for env.openExternal
// to open the correct URL.
Expand Down Expand Up @@ -215,7 +220,8 @@ const allFlows: IFlow[] = [
baseUri,
redirectUri,
logger,
enterpriseUri
enterpriseUri,
existingLogin
}: IFlowTriggerOptions): Promise<string> {
logger.info(`Trying with local server... (${scopes})`);
return await window.withProgress<string>({
Expand All @@ -232,6 +238,9 @@ const allFlows: IFlow[] = [
['redirect_uri', redirectUri.toString(true)],
['scope', scopes],
]);
if (existingLogin) {
searchParams.append('login', existingLogin);
}

const loginUrl = baseUri.with({
path: '/login/oauth/authorize',
Expand Down
37 changes: 34 additions & 3 deletions extensions/github-authentication/src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { PromiseAdapter, arrayEquals, promiseFromEvent } from './common/utils';
import { ExperimentationTelemetry } from './common/experimentationService';
import { Log } from './common/logger';
import { crypto } from './node/crypto';
import { TIMED_OUT_ERROR, USER_CANCELLATION_ERROR } from './common/errors';
import { CANCELLATION_ERROR, TIMED_OUT_ERROR, USER_CANCELLATION_ERROR } from './common/errors';

interface SessionData {
id: string;
Expand Down Expand Up @@ -296,13 +296,44 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
scopes: JSON.stringify(scopes),
});

const sessions = await this._sessionsPromise;

const scopeString = sortedScopes.join(' ');
const token = await this._githubServer.login(scopeString);
const existingLogin = sessions[0]?.account.label;
const token = await this._githubServer.login(scopeString, existingLogin);
const session = await this.tokenToSession(token, scopes);
this.afterSessionLoad(session);

const sessions = await this._sessionsPromise;
if (sessions.some(s => s.id !== session.id)) {
const otherAccountsIndexes = new Array<number>();
const otherAccountsLabels = new Set<string>();
for (let i = 0; i < sessions.length; i++) {
if (sessions[i].id !== session.id) {
otherAccountsIndexes.push(i);
otherAccountsLabels.add(sessions[i].account.label);
}
}
const proceed = vscode.l10n.t("Continue");
const labelstr = [...otherAccountsLabels].join(', ');
const result = await vscode.window.showInformationMessage(
vscode.l10n.t({
message: "You are logged into another account already ({0}).\n\nDo you want to log out of that account and log in to '{1}' instead?",
comment: ['{0} is a comma-separated list of account names. {1} is the account name to log into.'],
args: [labelstr, session.account.label]
}),
{ modal: true },
proceed
);
if (result !== proceed) {
throw new Error(CANCELLATION_ERROR);
}

// Remove other accounts
for (const i of otherAccountsIndexes) {
sessions.splice(i, 1);
}
}

const sessionIndex = sessions.findIndex(s => s.id === session.id || arrayEquals([...s.scopes].sort(), sortedScopes));
if (sessionIndex > -1) {
sessions.splice(sessionIndex, 1, session);
Expand Down
11 changes: 4 additions & 7 deletions extensions/github-authentication/src/githubServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,15 @@ import { isSupportedClient, isSupportedTarget } from './common/env';
import { crypto } from './node/crypto';
import { fetching } from './node/fetch';
import { ExtensionHost, GitHubTarget, getFlows } from './flows';
import { NETWORK_ERROR, USER_CANCELLATION_ERROR } from './common/errors';
import { CANCELLATION_ERROR, NETWORK_ERROR, USER_CANCELLATION_ERROR } from './common/errors';
import { Config } from './config';
import { base64Encode } from './node/buffer';

// This is the error message that we throw if the login was cancelled for any reason. Extensions
// calling `getSession` can handle this error to know that the user cancelled the login.
const CANCELLATION_ERROR = 'Cancelled';

const REDIRECT_URL_STABLE = 'https://vscode.dev/redirect';
const REDIRECT_URL_INSIDERS = 'https://insiders.vscode.dev/redirect';

export interface IGitHubServer {
login(scopes: string): Promise<string>;
login(scopes: string, existingLogin?: string): Promise<string>;
logout(session: vscode.AuthenticationSession): Promise<void>;
getUserInfo(token: string): Promise<{ id: string; accountName: string }>;
sendAdditionalTelemetryInfo(session: vscode.AuthenticationSession): Promise<void>;
Expand Down Expand Up @@ -91,7 +87,7 @@ export class GitHubServer implements IGitHubServer {
return this._isNoCorsEnvironment;
}

public async login(scopes: string): Promise<string> {
public async login(scopes: string, existingLogin?: string): Promise<string> {
this._logger.info(`Logging in for the following scopes: ${scopes}`);

// Used for showing a friendlier message to the user when the explicitly cancel a flow.
Expand Down Expand Up @@ -143,6 +139,7 @@ export class GitHubServer implements IGitHubServer {
uriHandler: this._uriHandler,
enterpriseUri: this._ghesUri,
redirectUri: vscode.Uri.parse(await this.getRedirectEndpoint()),
existingLogin
});
} catch (e) {
userCancelled = this.processLoginError(e);
Expand Down

0 comments on commit 191be39

Please sign in to comment.