silently getting session should return a session (if any) regardless of preference. fixes #137819

This commit is contained in:
Tyler Leonhardt 2021-11-24 16:40:39 -08:00
parent 26fe37ca3d
commit 9ec1ae1d92
No known key found for this signature in database
GPG key ID: 1BC2B6244363E77E
3 changed files with 132 additions and 77 deletions

View file

@ -207,6 +207,7 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu
private async doGetSession(providerId: string, scopes: string[], extensionId: string, extensionName: string, options: AuthenticationGetSessionOptions): Promise<modes.AuthenticationSession | undefined> {
const sessions = await this.authenticationService.getSessions(providerId, scopes, true);
const supportsMultipleAccounts = this.authenticationService.supportsMultipleAccounts(providerId);
// Error cases
if (options.forceNewSession && !sessions.length) {
@ -224,7 +225,7 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu
// Check if the sessions we have are valid
if (!options.forceNewSession && sessions.length) {
if (this.authenticationService.supportsMultipleAccounts(providerId)) {
if (supportsMultipleAccounts) {
if (options.clearSessionPreference) {
this.storageService.remove(`${extensionName}-${providerId}`, StorageScope.GLOBAL);
} else {
@ -251,18 +252,20 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu
throw new Error('User did not consent to login.');
}
const session = sessions?.length && !options.forceNewSession
const session = sessions?.length && !options.forceNewSession && supportsMultipleAccounts
? await this.authenticationService.selectSession(providerId, extensionId, extensionName, scopes, sessions)
: await this.authenticationService.createSession(providerId, scopes, true);
await this.setTrustedExtensionAndAccountPreference(providerId, session.account.label, extensionId, extensionName, session.id);
return session;
}
// passive flows
if (!options.silent) {
// passive flows (silent or default)
const validSession = sessions.find(s => this.authenticationService.isAccessAllowed(providerId, s.account.label, extensionId));
if (!options.silent && !validSession) {
await this.authenticationService.requestNewSession(providerId, scopes, extensionId, extensionName);
}
return undefined;
return validSession;
}
async $getSession(providerId: string, scopes: string[], extensionId: string, extensionName: string, options: AuthenticationGetSessionOptions): Promise<modes.AuthenticationSession | undefined> {

View file

@ -614,68 +614,70 @@ export class AuthenticationService extends Disposable implements IAuthentication
});
}
if (provider) {
const providerRequests = this._signInRequestItems.get(providerId);
const scopesList = scopes.join(SCOPESLIST_SEPARATOR);
const extensionHasExistingRequest = providerRequests
&& providerRequests[scopesList]
&& providerRequests[scopesList].requestingExtensionIds.includes(extensionId);
if (extensionHasExistingRequest) {
return;
}
// Construct a commandId that won't clash with others generated here, nor likely with an extension's command
const commandId = `${providerId}:${extensionId}:signIn${Object.keys(providerRequests || []).length}`;
const menuItem = MenuRegistry.appendMenuItem(MenuId.AccountsContext, {
group: '2_signInRequests',
command: {
id: commandId,
title: nls.localize({
key: 'signInRequest',
comment: [`The placeholder {0} will be replaced with an authentication provider's label. {1} will be replaced with an extension name. (1) is to indicate that this menu item contributes to a badge count.`]
},
"Sign in with {0} to use {1} (1)",
provider.label,
extensionName)
}
});
const signInCommand = CommandsRegistry.registerCommand({
id: commandId,
handler: async (accessor) => {
const authenticationService = accessor.get(IAuthenticationService);
const storageService = accessor.get(IStorageService);
const session = await authenticationService.createSession(providerId, scopes);
// Add extension to allow list since user explicitly signed in on behalf of it
this.updatedAllowedExtension(providerId, session.account.label, extensionId, extensionName, true);
// And also set it as the preferred account for the extension
storageService.store(`${extensionName}-${providerId}`, session.id, StorageScope.GLOBAL, StorageTarget.MACHINE);
}
});
if (providerRequests) {
const existingRequest = providerRequests[scopesList] || { disposables: [], requestingExtensionIds: [] };
providerRequests[scopesList] = {
disposables: [...existingRequest.disposables, menuItem, signInCommand],
requestingExtensionIds: [...existingRequest.requestingExtensionIds, extensionId]
};
this._signInRequestItems.set(providerId, providerRequests);
} else {
this._signInRequestItems.set(providerId, {
[scopesList]: {
disposables: [menuItem, signInCommand],
requestingExtensionIds: [extensionId]
}
});
}
this.updateBadgeCount();
if (!provider) {
return;
}
const providerRequests = this._signInRequestItems.get(providerId);
const scopesList = scopes.join(SCOPESLIST_SEPARATOR);
const extensionHasExistingRequest = providerRequests
&& providerRequests[scopesList]
&& providerRequests[scopesList].requestingExtensionIds.includes(extensionId);
if (extensionHasExistingRequest) {
return;
}
// Construct a commandId that won't clash with others generated here, nor likely with an extension's command
const commandId = `${providerId}:${extensionId}:signIn${Object.keys(providerRequests || []).length}`;
const menuItem = MenuRegistry.appendMenuItem(MenuId.AccountsContext, {
group: '2_signInRequests',
command: {
id: commandId,
title: nls.localize({
key: 'signInRequest',
comment: [`The placeholder {0} will be replaced with an authentication provider's label. {1} will be replaced with an extension name. (1) is to indicate that this menu item contributes to a badge count.`]
},
"Sign in with {0} to use {1} (1)",
provider.label,
extensionName)
}
});
const signInCommand = CommandsRegistry.registerCommand({
id: commandId,
handler: async (accessor) => {
const authenticationService = accessor.get(IAuthenticationService);
const storageService = accessor.get(IStorageService);
const session = await authenticationService.createSession(providerId, scopes);
// Add extension to allow list since user explicitly signed in on behalf of it
this.updatedAllowedExtension(providerId, session.account.label, extensionId, extensionName, true);
// And also set it as the preferred account for the extension
storageService.store(`${extensionName}-${providerId}`, session.id, StorageScope.GLOBAL, StorageTarget.MACHINE);
}
});
if (providerRequests) {
const existingRequest = providerRequests[scopesList] || { disposables: [], requestingExtensionIds: [] };
providerRequests[scopesList] = {
disposables: [...existingRequest.disposables, menuItem, signInCommand],
requestingExtensionIds: [...existingRequest.requestingExtensionIds, extensionId]
};
this._signInRequestItems.set(providerId, providerRequests);
} else {
this._signInRequestItems.set(providerId, {
[scopesList]: {
disposables: [menuItem, signInCommand],
requestingExtensionIds: [extensionId]
}
});
}
this.updateBadgeCount();
}
getLabel(id: string): string {
const authProvider = this._authenticationProviders.get(id);

View file

@ -56,6 +56,7 @@ class AuthTestQuickInputService extends TestQuickInputService {
}
class TestAuthProvider implements AuthenticationProvider {
private id = 1;
private sessions = new Map<string, AuthenticationSession>();
onDidChangeSessions = () => { return { dispose() { } }; };
async getSessions(scopes?: readonly string[]): Promise<AuthenticationSession[]> {
@ -73,14 +74,15 @@ class TestAuthProvider implements AuthenticationProvider {
const scopesStr = scopes.join(' ');
const session = {
scopes,
id: 'test' + scopesStr,
id: `${this.id}`,
account: {
label: scopesStr,
id: scopesStr,
label: `${this.id}`,
id: `${this.id}`,
},
accessToken: Math.random() + '',
};
this.sessions.set(scopesStr, session);
this.id++;
return session;
}
async removeSession(sessionId: string): Promise<void> {
@ -137,7 +139,7 @@ suite('ExtHostAuthentication', () => {
{
createIfNone: true
});
assert.strictEqual(session?.id, 'test' + scopes.join(' '));
assert.strictEqual(session?.id, '1');
assert.strictEqual(session?.scopes[0], 'foo');
});
@ -159,7 +161,7 @@ suite('ExtHostAuthentication', () => {
createIfNone: true
});
assert.strictEqual(session?.id, 'test' + scopes.join(' '));
assert.strictEqual(session?.id, '1');
assert.strictEqual(session?.scopes[0], 'foo');
const session2 = await extHostAuthentication.getSession(
@ -194,7 +196,7 @@ suite('ExtHostAuthentication', () => {
createIfNone: true
});
assert.strictEqual(session?.id, 'test' + scopes.join(' '));
assert.strictEqual(session?.id, '1');
assert.strictEqual(session?.scopes[0], 'foo');
const session2 = await extHostAuthentication.getSession(
@ -228,7 +230,7 @@ suite('ExtHostAuthentication', () => {
forceNewSession: true
});
assert.strictEqual(session2?.id, 'test' + scopes.join(' '));
assert.strictEqual(session2?.id, '2');
assert.strictEqual(session2?.scopes[0], 'foo');
assert.notStrictEqual(session1.accessToken, session2?.accessToken);
});
@ -252,11 +254,13 @@ suite('ExtHostAuthentication', () => {
forceNewSession: { detail: 'bar' }
});
assert.strictEqual(session2?.id, 'test' + scopes.join(' '));
assert.strictEqual(session2?.id, '2');
assert.strictEqual(session2?.scopes[0], 'foo');
assert.notStrictEqual(session1.accessToken, session2?.accessToken);
});
//#region Multi-Account AuthProvider
test('clearSessionPreference - true', async () => {
const scopes = ['foo'];
// Now create the session
@ -268,7 +272,7 @@ suite('ExtHostAuthentication', () => {
createIfNone: true
});
assert.strictEqual(session?.id, 'test' + scopes.join(' '));
assert.strictEqual(session?.id, '1');
assert.strictEqual(session?.scopes[0], scopes[0]);
const scopes2 = ['bar'];
@ -279,7 +283,7 @@ suite('ExtHostAuthentication', () => {
{
createIfNone: true
});
assert.strictEqual(session2?.id, 'test' + scopes2.join(' '));
assert.strictEqual(session2?.id, '2');
assert.strictEqual(session2?.scopes[0], scopes2[0]);
const session3 = await extHostAuthentication.getSession(
@ -298,6 +302,52 @@ suite('ExtHostAuthentication', () => {
assert.strictEqual(session.accessToken, session3?.accessToken);
});
test('silently getting session should return a session (if any) regardless of preference - fixes #137819', async () => {
const scopes = ['foo'];
// Now create the session
const session = await extHostAuthentication.getSession(
extensionDescription,
'test-multiple',
scopes,
{
createIfNone: true
});
assert.strictEqual(session?.id, '1');
assert.strictEqual(session?.scopes[0], scopes[0]);
const scopes2 = ['bar'];
const session2 = await extHostAuthentication.getSession(
extensionDescription,
'test-multiple',
scopes2,
{
createIfNone: true
});
assert.strictEqual(session2?.id, '2');
assert.strictEqual(session2?.scopes[0], scopes2[0]);
const shouldBeSession1 = await extHostAuthentication.getSession(
extensionDescription,
'test-multiple',
scopes,
{});
assert.strictEqual(session.id, shouldBeSession1?.id);
assert.strictEqual(session.scopes[0], shouldBeSession1?.scopes[0]);
assert.strictEqual(session.accessToken, shouldBeSession1?.accessToken);
const shouldBeSession2 = await extHostAuthentication.getSession(
extensionDescription,
'test-multiple',
scopes2,
{});
assert.strictEqual(session2.id, shouldBeSession2?.id);
assert.strictEqual(session2.scopes[0], shouldBeSession2?.scopes[0]);
assert.strictEqual(session2.accessToken, shouldBeSession2?.accessToken);
});
//#endregion
//#region error cases
test('forceNewSession with no sessions', async () => {