From 852df3025dc237e614fedddce66c73affa203e52 Mon Sep 17 00:00:00 2001 From: Rachel Macfarlane Date: Mon, 30 Mar 2020 17:03:14 -0700 Subject: [PATCH] Revert "Issue distinct sessions per extension, remove session when extension is removed from trusted list" This reverts commit 85119afc7bacf48638f7ecdc554363e74696a209. --- .../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, 18 insertions(+), 97 deletions(-) diff --git a/extensions/github-authentication/src/github.ts b/extensions/github-authentication/src/github.ts index 6fbe82516ac..e586b9659a5 100644 --- a/extensions/github-authentication/src/github.ts +++ b/extensions/github-authentication/src/github.ts @@ -4,7 +4,6 @@ *--------------------------------------------------------------------------------------------*/ import * as vscode from 'vscode'; -import * as uuid from 'uuid'; import { keychain } from './common/keychain'; import { GitHubServer } from './githubServer'; import Logger from './common/logger'; @@ -123,7 +122,7 @@ export class GitHubAuthenticationProvider { private async tokenToSession(token: string, scopes: string[]): Promise { const userInfo = await this._githubServer.getUserInfo(token); return { - id: uuid(), + id: userInfo.id, getAccessToken: () => Promise.resolve(token), accountName: userInfo.accountName, scopes: scopes diff --git a/extensions/vscode-account/package.json b/extensions/vscode-account/package.json index 73f165c79ad..cd579aef85a 100644 --- a/extensions/vscode-account/package.json +++ b/extensions/vscode-account/package.json @@ -38,11 +38,9 @@ "typescript": "^3.7.4", "tslint": "^5.12.1", "@types/node": "^10.12.21", - "@types/keytar": "^4.0.1", - "@types/uuid": "^3.4.6" + "@types/keytar": "^4.0.1" }, "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 eac21bf67e3..59f837d5579 100644 --- a/extensions/vscode-account/src/AADHelper.ts +++ b/extensions/vscode-account/src/AADHelper.ts @@ -7,7 +7,6 @@ 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'; @@ -408,7 +407,7 @@ export class AzureActiveDirectoryService { accessToken: json.access_token, refreshToken: json.refresh_token, scope, - sessionId: `${claims.tid}/${(claims.oid || (claims.altsecid || '' + claims.ipd || ''))}/${uuid()}`, + sessionId: `${claims.tid}/${(claims.oid || (claims.altsecid || '' + claims.ipd || ''))}/${scope}`, accountName: claims.email || claims.unique_name || 'user@example.com' }; } diff --git a/extensions/vscode-account/yarn.lock b/extensions/vscode-account/yarn.lock index 1506f62c87d..4a86ea6a2a2 100644 --- a/extensions/vscode-account/yarn.lock +++ b/extensions/vscode-account/yarn.lock @@ -30,11 +30,6 @@ 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" @@ -640,11 +635,6 @@ 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 52b7f7ef026..7c5957ad2dc 100644 --- a/src/vs/workbench/api/browser/mainThreadAuthentication.ts +++ b/src/vs/workbench/api/browser/mainThreadAuthentication.ts @@ -35,7 +35,6 @@ const BUILT_IN_AUTH_DEPENDENTS: AuthDependent[] = [ interface AllowedExtension { id: string; name: string; - sessionIds?: string[]; } function readAllowedExtensions(storageService: IStorageService, providerId: string, accountName: string): AllowedExtension[] { @@ -86,16 +85,6 @@ 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(); }); @@ -286,19 +275,9 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu this.authenticationService.sessionsUpdate(id, event); } - 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); - } - + 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)) { return true; } @@ -313,7 +292,7 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu const allow = choice === 1; if (allow) { - allowList.push({ id: extensionId, name: extensionName, sessionIds: [sessionId] }); + allowList = allowList.concat({ id: extensionId, name: extensionName }); this.storageService.store(`${providerId}-${accountName}`, JSON.stringify(allowList), StorageScope.GLOBAL); } @@ -334,10 +313,7 @@ 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); - if (!allowList.find(allowed => allowed.id === extensionId)) { - allowList.push({ id: extensionId, name: extensionName, sessionIds: [] }); - this.storageService.store(`${providerId}-${accountName}`, JSON.stringify(allowList), StorageScope.GLOBAL); - } + const allowList = readAllowedExtensions(this.storageService, providerId, accountName).concat({ id: extensionId, name: extensionName }); + 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 0dbb81ce77f..37c7b210fc2 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, extHostStorage)); + const extHostAuthentication = rpcProtocol.set(ExtHostContext.ExtHostAuthentication, new ExtHostAuthentication(rpcProtocol)); 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 28db2c35372..c64dbcef8d9 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, sessionId: string, providerName: string, extensionId: string, extensionName: string): Promise; + $getSessionsPrompt(providerId: string, accountName: 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 fe4613bf6ec..11ff861e137 100644 --- a/src/vs/workbench/api/common/extHostAuthentication.ts +++ b/src/vs/workbench/api/common/extHostAuthentication.ts @@ -9,7 +9,6 @@ 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; @@ -21,8 +20,7 @@ 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, - @IExtHostStorage private readonly storageService: IExtHostStorage) { + constructor(mainContext: IMainContext) { this._proxy = mainContext.getProxy(MainContext.MainThreadAuthentication); } @@ -35,34 +33,15 @@ 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(' '); - - 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]; }) + return (await provider.getSessions()) + .filter(session => session.scopes.sort().join(' ') === orderedScopes) .map(session => { return { id: session.id, @@ -72,9 +51,8 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { const isAllowed = await this._proxy.$getSessionsPrompt( provider.id, session.accountName, - session.id, provider.displayName, - extensionId, + ExtensionIdentifier.toKey(requestingExtension.identifier), requestingExtension.displayName || requestingExtension.name); if (!isAllowed) { @@ -99,28 +77,9 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { throw new Error('User did not consent to login.'); } - 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(); - } - }; + const newSession = await provider.login(scopes); + await this._proxy.$setTrustedExtension(provider.id, newSession.accountName, ExtensionIdentifier.toKey(requestingExtension.identifier), extensionName); + return newSession; } registerAuthenticationProvider(provider: vscode.AuthenticationProvider): vscode.Disposable {