From 85119afc7bacf48638f7ecdc554363e74696a209 Mon Sep 17 00:00:00 2001 From: Rachel Macfarlane Date: Mon, 30 Mar 2020 14:34:27 -0700 Subject: [PATCH] Issue distinct sessions per extension, remove session when extension is removed from trusted list --- .../github-authentication/src/github.ts | 3 +- extensions/vscode-account/package.json | 4 +- extensions/vscode-account/src/AADHelper.ts | 3 +- extensions/vscode-account/yarn.lock | 10 ++++ .../api/browser/mainThreadAuthentication.ts | 36 ++++++++++-- .../workbench/api/common/extHost.api.impl.ts | 2 +- .../workbench/api/common/extHost.protocol.ts | 2 +- .../api/common/extHostAuthentication.ts | 55 ++++++++++++++++--- 8 files changed, 97 insertions(+), 18 deletions(-) diff --git a/extensions/github-authentication/src/github.ts b/extensions/github-authentication/src/github.ts index e586b9659a5..6fbe82516ac 100644 --- a/extensions/github-authentication/src/github.ts +++ b/extensions/github-authentication/src/github.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as vscode from 'vscode'; +import * as uuid from 'uuid'; import { keychain } from './common/keychain'; import { GitHubServer } from './githubServer'; import Logger from './common/logger'; @@ -122,7 +123,7 @@ export class GitHubAuthenticationProvider { private async tokenToSession(token: string, scopes: string[]): Promise { const userInfo = await this._githubServer.getUserInfo(token); return { - id: userInfo.id, + id: uuid(), getAccessToken: () => Promise.resolve(token), accountName: userInfo.accountName, scopes: scopes diff --git a/extensions/vscode-account/package.json b/extensions/vscode-account/package.json index cd579aef85a..73f165c79ad 100644 --- a/extensions/vscode-account/package.json +++ b/extensions/vscode-account/package.json @@ -38,9 +38,11 @@ "typescript": "^3.7.4", "tslint": "^5.12.1", "@types/node": "^10.12.21", - "@types/keytar": "^4.0.1" + "@types/keytar": "^4.0.1", + "@types/uuid": "^3.4.6" }, "dependencies": { + "uuid": "^3.3.3", "vscode-nls": "^4.1.1" } } diff --git a/extensions/vscode-account/src/AADHelper.ts b/extensions/vscode-account/src/AADHelper.ts index 59f837d5579..eac21bf67e3 100644 --- a/extensions/vscode-account/src/AADHelper.ts +++ b/extensions/vscode-account/src/AADHelper.ts @@ -7,6 +7,7 @@ import * as crypto from 'crypto'; import * as https from 'https'; import * as querystring from 'querystring'; import * as vscode from 'vscode'; +import * as uuid from 'uuid'; import { createServer, startServer } from './authServer'; import { keychain } from './keychain'; import Logger from './logger'; @@ -407,7 +408,7 @@ export class AzureActiveDirectoryService { accessToken: json.access_token, refreshToken: json.refresh_token, scope, - sessionId: `${claims.tid}/${(claims.oid || (claims.altsecid || '' + claims.ipd || ''))}/${scope}`, + sessionId: `${claims.tid}/${(claims.oid || (claims.altsecid || '' + claims.ipd || ''))}/${uuid()}`, accountName: claims.email || claims.unique_name || 'user@example.com' }; } diff --git a/extensions/vscode-account/yarn.lock b/extensions/vscode-account/yarn.lock index 4a86ea6a2a2..1506f62c87d 100644 --- a/extensions/vscode-account/yarn.lock +++ b/extensions/vscode-account/yarn.lock @@ -30,6 +30,11 @@ resolved "https://registry.yarnpkg.com/@types/node/-/node-10.17.13.tgz#ccebcdb990bd6139cd16e84c39dc2fb1023ca90c" integrity sha512-pMCcqU2zT4TjqYFrWtYHKal7Sl30Ims6ulZ4UFXxI4xbtQqK/qqKwkDoBFCfooRqqmRu9vY3xaJRwxSh673aYg== +"@types/uuid@^3.4.6": + version "3.4.8" + resolved "https://registry.yarnpkg.com/@types/uuid/-/uuid-3.4.8.tgz#4ba887fcef88bd9a7515ca2de336d691e3e18318" + integrity sha512-zHWce3allXWSmRx6/AGXKCtSOA7JjeWd2L3t4aHfysNk8mouQnWCocveaT7a4IEIlPVHp81jzlnknqTgCjCLXA== + ansi-regex@^2.0.0: version "2.1.1" resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-2.1.1.tgz#c3b33ab5ee360d86e0e628f0468ae7ef27d654df" @@ -635,6 +640,11 @@ util-deprecate@^1.0.1, util-deprecate@~1.0.1: resolved "https://registry.yarnpkg.com/util-deprecate/-/util-deprecate-1.0.2.tgz#450d4dc9fa70de732762fbd2d4a28981419a0ccf" integrity sha1-RQ1Nyfpw3nMnYvvS1KKJgUGaDM8= +uuid@^3.3.3: + version "3.4.0" + resolved "https://registry.yarnpkg.com/uuid/-/uuid-3.4.0.tgz#b23e4358afa8a202fe7a100af1f5f883f02007ee" + integrity sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A== + vscode-nls@^4.1.1: version "4.1.1" resolved "https://registry.yarnpkg.com/vscode-nls/-/vscode-nls-4.1.1.tgz#f9916b64e4947b20322defb1e676a495861f133c" diff --git a/src/vs/workbench/api/browser/mainThreadAuthentication.ts b/src/vs/workbench/api/browser/mainThreadAuthentication.ts index 7c5957ad2dc..52b7f7ef026 100644 --- a/src/vs/workbench/api/browser/mainThreadAuthentication.ts +++ b/src/vs/workbench/api/browser/mainThreadAuthentication.ts @@ -35,6 +35,7 @@ const BUILT_IN_AUTH_DEPENDENTS: AuthDependent[] = [ interface AllowedExtension { id: string; name: string; + sessionIds?: string[]; } function readAllowedExtensions(storageService: IStorageService, providerId: string, accountName: string): AllowedExtension[] { @@ -85,6 +86,16 @@ export class MainThreadAuthenticationProvider extends Disposable { quickPick.onDidAccept(() => { const updatedAllowedList = quickPick.selectedItems.map(item => item.extension); storageService.store(`${this.id}-${accountName}`, JSON.stringify(updatedAllowedList), StorageScope.GLOBAL); + + // Remove sessions of untrusted extensions + const deselectedItems = items.filter(item => !quickPick.selectedItems.includes(item)); + deselectedItems.forEach(item => { + const extensionData = allowedExtensions.find(extension => item.extension.id === extension.id); + extensionData?.sessionIds?.forEach(sessionId => { + this.logout(sessionId); + }); + }); + quickPick.dispose(); }); @@ -275,9 +286,19 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu this.authenticationService.sessionsUpdate(id, event); } - async $getSessionsPrompt(providerId: string, accountName: string, providerName: string, extensionId: string, extensionName: string): Promise { - let allowList = readAllowedExtensions(this.storageService, providerId, accountName); - if (allowList.some(extension => extension.id === extensionId)) { + async $getSessionsPrompt(providerId: string, accountName: string, sessionId: string, providerName: string, extensionId: string, extensionName: string): Promise { + const allowList = readAllowedExtensions(this.storageService, providerId, accountName); + const extensionData = allowList.find(extension => extension.id === extensionId); + if (extensionData) { + if (!extensionData.sessionIds) { + extensionData.sessionIds = []; + } + + if (!extensionData.sessionIds.find(id => id === sessionId)) { + extensionData.sessionIds.push(sessionId); + this.storageService.store(`${providerId}-${accountName}`, JSON.stringify(allowList), StorageScope.GLOBAL); + } + return true; } @@ -292,7 +313,7 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu const allow = choice === 1; if (allow) { - allowList = allowList.concat({ id: extensionId, name: extensionName }); + allowList.push({ id: extensionId, name: extensionName, sessionIds: [sessionId] }); this.storageService.store(`${providerId}-${accountName}`, JSON.stringify(allowList), StorageScope.GLOBAL); } @@ -313,7 +334,10 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu } async $setTrustedExtension(providerId: string, accountName: string, extensionId: string, extensionName: string): Promise { - const allowList = readAllowedExtensions(this.storageService, providerId, accountName).concat({ id: extensionId, name: extensionName }); - this.storageService.store(`${providerId}-${accountName}`, JSON.stringify(allowList), StorageScope.GLOBAL); + const allowList = readAllowedExtensions(this.storageService, providerId, accountName); + if (!allowList.find(allowed => allowed.id === extensionId)) { + allowList.push({ id: extensionId, name: extensionName, sessionIds: [] }); + this.storageService.store(`${providerId}-${accountName}`, JSON.stringify(allowList), StorageScope.GLOBAL); + } } } diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 37c7b210fc2..0dbb81ce77f 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -133,7 +133,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I const extHostLabelService = rpcProtocol.set(ExtHostContext.ExtHosLabelService, new ExtHostLabelService(rpcProtocol)); const extHostNotebook = rpcProtocol.set(ExtHostContext.ExtHostNotebook, new ExtHostNotebookController(rpcProtocol, extHostCommands, extHostDocumentsAndEditors)); const extHostTheming = rpcProtocol.set(ExtHostContext.ExtHostTheming, new ExtHostTheming(rpcProtocol)); - const extHostAuthentication = rpcProtocol.set(ExtHostContext.ExtHostAuthentication, new ExtHostAuthentication(rpcProtocol)); + const extHostAuthentication = rpcProtocol.set(ExtHostContext.ExtHostAuthentication, new ExtHostAuthentication(rpcProtocol, extHostStorage)); const extHostTimeline = rpcProtocol.set(ExtHostContext.ExtHostTimeline, new ExtHostTimeline(rpcProtocol, extHostCommands)); const extHostWebviews = rpcProtocol.set(ExtHostContext.ExtHostWebviews, new ExtHostWebviews(rpcProtocol, initData.environment, extHostWorkspace, extHostLogService, extHostApiDeprecation, extHostDocuments)); diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index c64dbcef8d9..28db2c35372 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -158,7 +158,7 @@ export interface MainThreadAuthenticationShape extends IDisposable { $registerAuthenticationProvider(id: string, displayName: string): void; $unregisterAuthenticationProvider(id: string): void; $onDidChangeSessions(providerId: string, event: modes.AuthenticationSessionsChangeEvent): void; - $getSessionsPrompt(providerId: string, accountName: string, providerName: string, extensionId: string, extensionName: string): Promise; + $getSessionsPrompt(providerId: string, accountName: string, sessionId: string, providerName: string, extensionId: string, extensionName: string): Promise; $loginPrompt(providerName: string, extensionName: string): Promise; $setTrustedExtension(providerId: string, accountName: string, extensionId: string, extensionName: string): Promise; } diff --git a/src/vs/workbench/api/common/extHostAuthentication.ts b/src/vs/workbench/api/common/extHostAuthentication.ts index 11ff861e137..fe4613bf6ec 100644 --- a/src/vs/workbench/api/common/extHostAuthentication.ts +++ b/src/vs/workbench/api/common/extHostAuthentication.ts @@ -9,6 +9,7 @@ import { Emitter, Event } from 'vs/base/common/event'; import { IMainContext, MainContext, MainThreadAuthenticationShape, ExtHostAuthenticationShape } from 'vs/workbench/api/common/extHost.protocol'; import { Disposable } from 'vs/workbench/api/common/extHostTypes'; import { IExtensionDescription, ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; +import { IExtHostStorage } from 'vs/workbench/api/common/extHostStorage'; export class ExtHostAuthentication implements ExtHostAuthenticationShape { private _proxy: MainThreadAuthenticationShape; @@ -20,7 +21,8 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { private _onDidChangeSessions = new Emitter<{ [providerId: string]: vscode.AuthenticationSessionsChangeEvent }>(); readonly onDidChangeSessions: Event<{ [providerId: string]: vscode.AuthenticationSessionsChangeEvent }> = this._onDidChangeSessions.event; - constructor(mainContext: IMainContext) { + constructor(mainContext: IMainContext, + @IExtHostStorage private readonly storageService: IExtHostStorage) { this._proxy = mainContext.getProxy(MainContext.MainThreadAuthentication); } @@ -33,15 +35,34 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { return ids; } + private async hasNotBeenReadByOtherExtension(providerId: string, session: vscode.AuthenticationSession, extensionId: string): Promise { + const readerId = await this.storageService.getValue(true, `${providerId}-${session.accountName}-${session.id}`); + if (!readerId) { + await this.storageService.setValue(true, `${providerId}-${session.accountName}-${session.id}`, extensionId as any); + return true; + } + + return readerId === extensionId; + } + + private async isMatchingSession(session: vscode.AuthenticationSession, scopes: string, providerId: string, extensionId: string): Promise { + return session.scopes.sort().join(' ') === scopes && (await this.hasNotBeenReadByOtherExtension(providerId, session, extensionId)); + } + async getSessions(requestingExtension: IExtensionDescription, providerId: string, scopes: string[]): Promise { const provider = this._authenticationProviders.get(providerId); if (!provider) { throw new Error(`No authentication provider with id '${providerId}' is currently registered.`); } + const extensionId = ExtensionIdentifier.toKey(requestingExtension.identifier); const orderedScopes = scopes.sort().join(' '); - return (await provider.getSessions()) - .filter(session => session.scopes.sort().join(' ') === orderedScopes) + + const sessions = await provider.getSessions(); + const filteredSessions = await Promise.all(sessions.map(session => this.isMatchingSession(session, orderedScopes, providerId, extensionId))); + + return sessions + .filter((_, i) => { return filteredSessions[i]; }) .map(session => { return { id: session.id, @@ -51,8 +72,9 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { const isAllowed = await this._proxy.$getSessionsPrompt( provider.id, session.accountName, + session.id, provider.displayName, - ExtensionIdentifier.toKey(requestingExtension.identifier), + extensionId, requestingExtension.displayName || requestingExtension.name); if (!isAllowed) { @@ -77,9 +99,28 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { throw new Error('User did not consent to login.'); } - const newSession = await provider.login(scopes); - await this._proxy.$setTrustedExtension(provider.id, newSession.accountName, ExtensionIdentifier.toKey(requestingExtension.identifier), extensionName); - return newSession; + const session = await provider.login(scopes); + await this._proxy.$setTrustedExtension(provider.id, session.accountName, ExtensionIdentifier.toKey(requestingExtension.identifier), extensionName); + return { + id: session.id, + accountName: session.accountName, + scopes: session.scopes, + getAccessToken: async () => { + const isAllowed = await this._proxy.$getSessionsPrompt( + provider.id, + session.accountName, + session.id, + provider.displayName, + ExtensionIdentifier.toKey(requestingExtension.identifier), + requestingExtension.displayName || requestingExtension.name); + + if (!isAllowed) { + throw new Error('User did not consent to token access.'); + } + + return session.getAccessToken(); + } + }; } registerAuthenticationProvider(provider: vscode.AuthenticationProvider): vscode.Disposable { -- GitLab