Skip to content

Commit 04bcb01

Browse files
Move 'incorrect account' handling into core (microsoft#224872)
So that all auth providers can take advantage of this logic. This basically will do a light enforcement that the account you signed in to matches the account that was requested (if it was specified). This is needed for finalization.
1 parent 02b638a commit 04bcb01

File tree

2 files changed

+43
-36
lines changed

2 files changed

+43
-36
lines changed

extensions/github-authentication/src/github.ts

Lines changed: 5 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { PromiseAdapter, arrayEquals, promiseFromEvent } from './common/utils';
1111
import { ExperimentationTelemetry } from './common/experimentationService';
1212
import { Log } from './common/logger';
1313
import { crypto } from './node/crypto';
14-
import { CANCELLATION_ERROR, TIMED_OUT_ERROR, USER_CANCELLATION_ERROR } from './common/errors';
14+
import { TIMED_OUT_ERROR, USER_CANCELLATION_ERROR } from './common/errors';
1515

1616
interface SessionData {
1717
id: string;
@@ -316,39 +316,13 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid
316316

317317
const sessions = await this._sessionsPromise;
318318

319-
const forcedLogin = options?.account?.label;
320-
const backupLogin = sessions[0]?.account.label;
321-
this._logger.info(`Logging in with '${forcedLogin ? forcedLogin : 'any'}' account...`);
319+
// First we use the account specified in the options, otherwise we use the first account we have to seed auth.
320+
const loginWith = options?.account?.label ?? sessions[0]?.account.label;
321+
this._logger.info(`Logging in with '${loginWith ? loginWith : 'any'}' account...`);
322322

323323
const scopeString = sortedScopes.join(' ');
324-
const token = await this._githubServer.login(scopeString, forcedLogin ?? backupLogin);
324+
const token = await this._githubServer.login(scopeString, loginWith);
325325
const session = await this.tokenToSession(token, scopes);
326-
327-
// If an account was specified, we should ensure that the token we got back is for that account.
328-
if (forcedLogin) {
329-
if (session.account.label !== forcedLogin) {
330-
const keepNewAccount = vscode.l10n.t('Keep {0}', session.account.label);
331-
const tryAgain = vscode.l10n.t('Login with {0}', forcedLogin);
332-
const result = await vscode.window.showWarningMessage(
333-
vscode.l10n.t('Incorrect account detected'),
334-
{ modal: true, detail: vscode.l10n.t('The chosen account, {0}, does not match the requested account, {1}.', session.account.label, forcedLogin) },
335-
keepNewAccount,
336-
tryAgain
337-
);
338-
if (result === tryAgain) {
339-
return await this.createSession(scopes, {
340-
...options,
341-
// The id doesn't matter here, we just need to pass the label through
342-
account: { id: forcedLogin, label: forcedLogin }
343-
});
344-
}
345-
// Cancelled result
346-
if (!result) {
347-
throw new Error(CANCELLATION_ERROR);
348-
}
349-
// Keep result continues on
350-
}
351-
}
352326
this.afterSessionLoad(session);
353327

354328
const sessionIndex = sessions.findIndex(

src/vs/workbench/api/browser/mainThreadAuthentication.ts

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { IAuthenticationUsageService } from 'vs/workbench/services/authenticatio
1919
import { getAuthenticationProviderActivationEvent } from 'vs/workbench/services/authentication/browser/authenticationService';
2020
import { URI, UriComponents } from 'vs/base/common/uri';
2121
import { IOpenerService } from 'vs/platform/opener/common/opener';
22+
import { CancellationError } from 'vs/base/common/errors';
2223

2324
interface AuthenticationForceNewSessionOptions {
2425
detail?: string;
@@ -159,6 +160,31 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu
159160
return result ?? false;
160161
}
161162

163+
private async continueWithIncorrectAccountPrompt(chosenAccountLabel: string, requestedAccountLabel: string): Promise<boolean> {
164+
const result = await this.dialogService.prompt({
165+
message: nls.localize('incorrectAccount', "Incorrect account detected"),
166+
detail: nls.localize('incorrectAccountDetail', "The chosen account, {0}, does not match the requested account, {1}.", chosenAccountLabel, requestedAccountLabel),
167+
type: Severity.Warning,
168+
cancelButton: true,
169+
buttons: [
170+
{
171+
label: nls.localize('keep', 'Keep {0}', chosenAccountLabel),
172+
run: () => chosenAccountLabel
173+
},
174+
{
175+
label: nls.localize('loginWith', 'Login with {0}', requestedAccountLabel),
176+
run: () => requestedAccountLabel
177+
}
178+
],
179+
});
180+
181+
if (!result.result) {
182+
throw new CancellationError();
183+
}
184+
185+
return result.result === chosenAccountLabel;
186+
}
187+
162188
private async doGetSession(providerId: string, scopes: string[], extensionId: string, extensionName: string, options: AuthenticationGetSessionOptions): Promise<AuthenticationSession | undefined> {
163189
const sessions = await this.authenticationService.getSessions(providerId, scopes, options.account, true);
164190
const provider = this.authenticationService.getProvider(providerId);
@@ -212,18 +238,25 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu
212238
throw new Error('User did not consent to login.');
213239
}
214240

215-
let session;
241+
let session: AuthenticationSession;
216242
if (sessions?.length && !options.forceNewSession) {
217243
session = provider.supportsMultipleAccounts && !options.account
218244
? await this.authenticationExtensionsService.selectSession(providerId, extensionId, extensionName, scopes, sessions)
219245
: sessions[0];
220246
} else {
221-
let account: AuthenticationSessionAccount | undefined = options.account;
222-
if (!account) {
247+
let accountToCreate: AuthenticationSessionAccount | undefined = options.account;
248+
if (!accountToCreate) {
223249
const sessionIdToRecreate = this.authenticationExtensionsService.getSessionPreference(providerId, extensionId, scopes);
224-
account = sessionIdToRecreate ? sessions.find(session => session.id === sessionIdToRecreate)?.account : undefined;
250+
accountToCreate = sessionIdToRecreate ? sessions.find(session => session.id === sessionIdToRecreate)?.account : undefined;
225251
}
226-
session = await this.authenticationService.createSession(providerId, scopes, { activateImmediate: true, account });
252+
253+
do {
254+
session = await this.authenticationService.createSession(providerId, scopes, { activateImmediate: true, account: accountToCreate });
255+
} while (
256+
accountToCreate
257+
&& accountToCreate.label !== session.account.label
258+
&& !await this.continueWithIncorrectAccountPrompt(session.account.label, accountToCreate.label)
259+
);
227260
}
228261

229262
this.authenticationAccessService.updateAllowedExtensions(providerId, session.account.label, [{ id: extensionId, name: extensionName, allowed: true }]);

0 commit comments

Comments
 (0)