From 87b0c54586a5f0d39fb04d5dad9fbfe96f7d677c Mon Sep 17 00:00:00 2001 From: Rachel Macfarlane Date: Fri, 15 May 2020 14:33:45 -0700 Subject: [PATCH] Make AuthenticationSession a class and remove getAccessToken method, fixes #91554 --- .../github-authentication/src/github.ts | 41 ++--- .../microsoft-authentication/src/AADHelper.ts | 149 +++++++++--------- .../microsoft-authentication/src/extension.ts | 5 +- src/vs/editor/common/modes.ts | 2 +- src/vs/vscode.proposed.d.ts | 41 ++++- .../api/browser/mainThreadAuthentication.ts | 21 +-- .../workbench/api/common/extHost.api.impl.ts | 5 +- .../api/common/extHostAuthentication.ts | 8 +- src/vs/workbench/api/common/extHostTypes.ts | 8 + .../browser/userDataSyncAccount.ts | 4 +- 10 files changed, 147 insertions(+), 137 deletions(-) diff --git a/extensions/github-authentication/src/github.ts b/extensions/github-authentication/src/github.ts index c583b691f6b..7ad1b238499 100644 --- a/extensions/github-authentication/src/github.ts +++ b/extensions/github-authentication/src/github.ts @@ -34,7 +34,7 @@ function isOldSessionData(x: any): x is OldSessionData { } export class GitHubAuthenticationProvider { - private _sessions: vscode.AuthenticationSession[] = []; + private _sessions: vscode.AuthenticationSession2[] = []; private _githubServer = new GitHubServer(); public async initialize(): Promise { @@ -51,7 +51,7 @@ export class GitHubAuthenticationProvider { private pollForChange() { setTimeout(async () => { - let storedSessions: vscode.AuthenticationSession[]; + let storedSessions: vscode.AuthenticationSession2[]; try { storedSessions = await this.readSessions(); } catch (e) { @@ -94,12 +94,12 @@ export class GitHubAuthenticationProvider { }, 1000 * 30); } - private async readSessions(): Promise { + private async readSessions(): Promise { const storedSessions = await keychain.getToken(); if (storedSessions) { try { const sessionData: (SessionData | OldSessionData)[] = JSON.parse(storedSessions); - const sessionPromises = sessionData.map(async (session: SessionData | OldSessionData): Promise => { + const sessionPromises = sessionData.map(async (session: SessionData | OldSessionData): Promise => { const needsUserInfo = isOldSessionData(session) || !session.account; let userInfo: { id: string, accountName: string }; if (needsUserInfo) { @@ -117,7 +117,7 @@ export class GitHubAuthenticationProvider { : session.account?.id ?? userInfo!.id }, scopes: session.scopes, - getAccessToken: () => Promise.resolve(session.accessToken) + accessToken: session.accessToken }; }); @@ -136,24 +136,14 @@ export class GitHubAuthenticationProvider { } private async storeSessions(): Promise { - const sessionData: SessionData[] = await Promise.all(this._sessions.map(async session => { - const resolvedAccessToken = await session.getAccessToken(); - return { - id: session.id, - account: session.account, - scopes: session.scopes, - accessToken: resolvedAccessToken - }; - })); - - await keychain.setToken(JSON.stringify(sessionData)); + await keychain.setToken(JSON.stringify(this._sessions)); } - get sessions(): vscode.AuthenticationSession[] { + get sessions(): vscode.AuthenticationSession2[] { return this._sessions; } - public async login(scopes: string): Promise { + public async login(scopes: string): Promise { const token = scopes === 'vso' ? await this.loginAndInstallApp(scopes) : await this._githubServer.login(scopes); const session = await this.tokenToSession(token, scopes.split(' ')); await this.setToken(session); @@ -174,19 +164,12 @@ export class GitHubAuthenticationProvider { this._githubServer.manuallyProvideToken(); } - private async tokenToSession(token: string, scopes: string[]): Promise { + private async tokenToSession(token: string, scopes: string[]): Promise { const userInfo = await this._githubServer.getUserInfo(token); - return { - id: uuid(), - getAccessToken: () => Promise.resolve(token), - account: { - displayName: userInfo.accountName, - id: userInfo.id - }, - scopes: scopes - }; + return new vscode.AuthenticationSession2(uuid(), token, { displayName: userInfo.accountName, id: userInfo.id }, scopes); } - private async setToken(session: vscode.AuthenticationSession): Promise { + + private async setToken(session: vscode.AuthenticationSession2): Promise { const sessionIndex = this._sessions.findIndex(s => s.id === session.id); if (sessionIndex > -1) { this._sessions.splice(sessionIndex, 1, session); diff --git a/extensions/microsoft-authentication/src/AADHelper.ts b/extensions/microsoft-authentication/src/AADHelper.ts index c8bd5cd2ffa..fd826ec1e2e 100644 --- a/extensions/microsoft-authentication/src/AADHelper.ts +++ b/extensions/microsoft-authentication/src/AADHelper.ts @@ -204,13 +204,9 @@ export class AzureActiveDirectoryService { }, 1000 * 30); } - private convertToSession(token: IToken): vscode.AuthenticationSession { - return { - id: token.sessionId, - getAccessToken: () => this.resolveAccessToken(token), - account: token.account, - scopes: token.scope.split(' ') - }; + private async convertToSession(token: IToken): Promise { + const resolvedToken = await this.resolveAccessToken(token); + return new vscode.AuthenticationSession2(token.sessionId, resolvedToken, token.account, token.scope.split(' ')); } private async resolveAccessToken(token: IToken): Promise { @@ -243,77 +239,81 @@ export class AzureActiveDirectoryService { } } - get sessions(): vscode.AuthenticationSession[] { - return this._tokens.map(token => this.convertToSession(token)); + get sessions(): Promise { + return Promise.all(this._tokens.map(token => this.convertToSession(token))); } - public async login(scope: string): Promise { + public async login(scope: string): Promise { Logger.info('Logging in...'); - - if (vscode.env.uiKind === vscode.UIKind.Web) { - await this.loginWithoutLocalServer(scope); - return; - } - - const nonce = crypto.randomBytes(16).toString('base64'); - const { server, redirectPromise, codePromise } = createServer(nonce); - - let token: IToken | undefined; - try { - const port = await startServer(server); - vscode.env.openExternal(vscode.Uri.parse(`http://localhost:${port}/signin?nonce=${encodeURIComponent(nonce)}`)); - - const redirectReq = await redirectPromise; - if ('err' in redirectReq) { - const { err, res } = redirectReq; - res.writeHead(302, { Location: `/?error=${encodeURIComponent(err && err.message || 'Unknown error')}` }); - res.end(); - throw err; + return new Promise(async (resolve, reject) => { + if (vscode.env.uiKind === vscode.UIKind.Web) { + resolve(this.loginWithoutLocalServer(scope)); + return; } - const host = redirectReq.req.headers.host || ''; - const updatedPortStr = (/^[^:]+:(\d+)$/.exec(Array.isArray(host) ? host[0] : host) || [])[1]; - const updatedPort = updatedPortStr ? parseInt(updatedPortStr, 10) : port; - - const state = `${updatedPort},${encodeURIComponent(nonce)}`; - - const codeVerifier = toBase64UrlEncoding(crypto.randomBytes(32).toString('base64')); - const codeChallenge = toBase64UrlEncoding(crypto.createHash('sha256').update(codeVerifier).digest('base64')); - const loginUrl = `${loginEndpointUrl}${tenant}/oauth2/v2.0/authorize?response_type=code&response_mode=query&client_id=${encodeURIComponent(clientId)}&redirect_uri=${encodeURIComponent(redirectUrl)}&state=${state}&scope=${encodeURIComponent(scope)}&prompt=select_account&code_challenge_method=S256&code_challenge=${codeChallenge}`; - - await redirectReq.res.writeHead(302, { Location: loginUrl }); - redirectReq.res.end(); - - const codeRes = await codePromise; - const res = codeRes.res; + const nonce = crypto.randomBytes(16).toString('base64'); + const { server, redirectPromise, codePromise } = createServer(nonce); + let token: IToken | undefined; try { - if ('err' in codeRes) { - throw codeRes.err; - } - token = await this.exchangeCodeForToken(codeRes.code, codeVerifier, scope); - this.setToken(token, scope); - Logger.info('Login successful'); - res.writeHead(302, { Location: '/' }); - res.end(); - } catch (err) { - res.writeHead(302, { Location: `/?error=${encodeURIComponent(err && err.message || 'Unknown error')}` }); - res.end(); - throw new Error(err.message); - } - } catch (e) { - Logger.error(e.message); + const port = await startServer(server); + vscode.env.openExternal(vscode.Uri.parse(`http://localhost:${port}/signin?nonce=${encodeURIComponent(nonce)}`)); - // If the error was about starting the server, try directly hitting the login endpoint instead - if (e.message === 'Error listening to server' || e.message === 'Closed' || e.message === 'Timeout waiting for port') { - await this.loginWithoutLocalServer(scope); + const redirectReq = await redirectPromise; + if ('err' in redirectReq) { + const { err, res } = redirectReq; + res.writeHead(302, { Location: `/?error=${encodeURIComponent(err && err.message || 'Unknown error')}` }); + res.end(); + throw err; + } + + const host = redirectReq.req.headers.host || ''; + const updatedPortStr = (/^[^:]+:(\d+)$/.exec(Array.isArray(host) ? host[0] : host) || [])[1]; + const updatedPort = updatedPortStr ? parseInt(updatedPortStr, 10) : port; + + const state = `${updatedPort},${encodeURIComponent(nonce)}`; + + const codeVerifier = toBase64UrlEncoding(crypto.randomBytes(32).toString('base64')); + const codeChallenge = toBase64UrlEncoding(crypto.createHash('sha256').update(codeVerifier).digest('base64')); + const loginUrl = `${loginEndpointUrl}${tenant}/oauth2/v2.0/authorize?response_type=code&response_mode=query&client_id=${encodeURIComponent(clientId)}&redirect_uri=${encodeURIComponent(redirectUrl)}&state=${state}&scope=${encodeURIComponent(scope)}&prompt=select_account&code_challenge_method=S256&code_challenge=${codeChallenge}`; + + await redirectReq.res.writeHead(302, { Location: loginUrl }); + redirectReq.res.end(); + + const codeRes = await codePromise; + const res = codeRes.res; + + try { + if ('err' in codeRes) { + throw codeRes.err; + } + token = await this.exchangeCodeForToken(codeRes.code, codeVerifier, scope); + this.setToken(token, scope); + Logger.info('Login successful'); + res.writeHead(302, { Location: '/' }); + const session = await this.convertToSession(token); + resolve(session); + res.end(); + } catch (err) { + res.writeHead(302, { Location: `/?error=${encodeURIComponent(err && err.message || 'Unknown error')}` }); + res.end(); + reject(err.message); + } + } catch (e) { + Logger.error(e.message); + + // If the error was about starting the server, try directly hitting the login endpoint instead + if (e.message === 'Error listening to server' || e.message === 'Closed' || e.message === 'Timeout waiting for port') { + await this.loginWithoutLocalServer(scope); + } + + reject(e.message); + } finally { + setTimeout(() => { + server.close(); + }, 5000); } - throw new Error(e.message); - } finally { - setTimeout(() => { - server.close(); - }, 5000); - } + }); } private getCallbackEnvironment(callbackUri: vscode.Uri): string { @@ -333,7 +333,7 @@ export class AzureActiveDirectoryService { } } - private async loginWithoutLocalServer(scope: string): Promise { + private async loginWithoutLocalServer(scope: string): Promise { const callbackUri = await vscode.env.asExternalUri(vscode.Uri.parse(`${vscode.env.uriScheme}://vscode.microsoft-authentication`)); const nonce = crypto.randomBytes(16).toString('base64'); const port = (callbackUri.authority.match(/:([0-9]*)$/) || [])[1] || (callbackUri.scheme === 'https' ? 443 : 80); @@ -348,7 +348,7 @@ export class AzureActiveDirectoryService { }); vscode.env.openExternal(uri); - const timeoutPromise = new Promise((_: (value: IToken) => void, reject) => { + const timeoutPromise = new Promise((_: (value: vscode.AuthenticationSession2) => void, reject) => { const wait = setTimeout(() => { clearTimeout(wait); reject('Login timed out.'); @@ -358,9 +358,9 @@ export class AzureActiveDirectoryService { return Promise.race([this.handleCodeResponse(state, codeVerifier, scope), timeoutPromise]); } - private async handleCodeResponse(state: string, codeVerifier: string, scope: string) { + private async handleCodeResponse(state: string, codeVerifier: string, scope: string): Promise { let uriEventListener: vscode.Disposable; - return new Promise((resolve: (value: IToken) => void, reject) => { + return new Promise((resolve: (value: vscode.AuthenticationSession2) => void, reject) => { uriEventListener = this._uriHandler.event(async (uri: vscode.Uri) => { try { const query = parseQuery(uri); @@ -374,7 +374,8 @@ export class AzureActiveDirectoryService { const token = await this.exchangeCodeForToken(code, codeVerifier, scope); this.setToken(token, scope); - resolve(token); + const session = await this.convertToSession(token); + resolve(session); } catch (err) { reject(err); } diff --git a/extensions/microsoft-authentication/src/extension.ts b/extensions/microsoft-authentication/src/extension.ts index cc2954e05a6..ed751d48fb3 100644 --- a/extensions/microsoft-authentication/src/extension.ts +++ b/extensions/microsoft-authentication/src/extension.ts @@ -26,10 +26,9 @@ export async function activate(context: vscode.ExtensionContext) { login: async (scopes: string[]) => { try { telemetryReporter.sendTelemetryEvent('login'); - await loginService.login(scopes.sort().join(' ')); - const session = loginService.sessions[loginService.sessions.length - 1]; + const session = await loginService.login(scopes.sort().join(' ')); onDidChangeSessions.fire({ added: [session.id], removed: [], changed: [] }); - return loginService.sessions[0]!; + return session; } catch (e) { telemetryReporter.sendTelemetryEvent('loginFailed'); throw e; diff --git a/src/vs/editor/common/modes.ts b/src/vs/editor/common/modes.ts index 77e0df59de7..5d9d1a6f983 100644 --- a/src/vs/editor/common/modes.ts +++ b/src/vs/editor/common/modes.ts @@ -1411,7 +1411,7 @@ export interface RenameProvider { */ export interface AuthenticationSession { id: string; - getAccessToken(): Thenable; + accessToken: string; account: { displayName: string; id: string; diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 53e50a94eba..5ec78c64b0f 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -28,6 +28,41 @@ declare module 'vscode' { scopes: string[]; } + export class AuthenticationSession2 { + /** + * The identifier of the authentication session. + */ + readonly id: string; + + /** + * The access token. + */ + readonly accessToken: string; + + /** + * The account associated with the session. + */ + readonly account: { + /** + * The human-readable name of the account. + */ + readonly displayName: string; + + /** + * The unique identifier of the account. + */ + readonly id: string; + }; + + /** + * The permissions granted by the session's access token. Available scopes + * are defined by the authentication provider. + */ + readonly scopes: string[]; + + constructor(id: string, accessToken: string, account: { displayName: string, id: string }, scopes: string[]); + } + /** * An [event](#Event) which fires when an [AuthenticationProvider](#AuthenticationProvider) is added or removed. */ @@ -112,12 +147,12 @@ declare module 'vscode' { /** * Returns an array of current sessions. */ - getSessions(): Thenable>; + getSessions(): Thenable>; /** * Prompts a user to login. */ - login(scopes: string[]): Thenable; + login(scopes: string[]): Thenable; /** * Removes the session corresponding to session id. @@ -170,7 +205,7 @@ declare module 'vscode' { * @returns A thenable that resolves to an authentication session if available, or undefined if there are no sessions and * `createIfNone` was not specified. */ - export function getSession(providerId: string, scopes: string[], options: AuthenticationGetSessionOptions): Thenable; + export function getSession(providerId: string, scopes: string[], options: AuthenticationGetSessionOptions): Thenable; /** * @deprecated diff --git a/src/vs/workbench/api/browser/mainThreadAuthentication.ts b/src/vs/workbench/api/browser/mainThreadAuthentication.ts index fb31869d00e..0e089dbb4a7 100644 --- a/src/vs/workbench/api/browser/mainThreadAuthentication.ts +++ b/src/vs/workbench/api/browser/mainThreadAuthentication.ts @@ -206,17 +206,7 @@ export class MainThreadAuthenticationProvider extends Disposable { } async getSessions(): Promise> { - return (await this._proxy.$getSessions(this.id)).map(session => { - return { - id: session.id, - account: session.account, - scopes: session.scopes, - getAccessToken: () => { - addAccountUsage(this.storageService, this.id, session.account.displayName, 'preferencessync', nls.localize('sync', "Preferences Sync")); - return this._proxy.$getSessionAccessToken(this.id, session.id); - } - }; - }); + return this._proxy.$getSessions(this.id); } async updateSessionItems(event: modes.AuthenticationSessionsChangeEvent): Promise { @@ -247,14 +237,7 @@ export class MainThreadAuthenticationProvider extends Disposable { } login(scopes: string[]): Promise { - return this._proxy.$login(this.id, scopes).then(session => { - return { - id: session.id, - account: session.account, - scopes: session.scopes, - getAccessToken: () => this._proxy.$getSessionAccessToken(this.id, session.id) - }; - }); + return this._proxy.$login(this.id, scopes); } async logout(sessionId: string): Promise { diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 448f568c195..e5e4797bd54 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -199,7 +199,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I hasSessions(providerId: string, scopes: string[]): Thenable { return extHostAuthentication.hasSessions(providerId, scopes); }, - getSession(providerId: string, scopes: string[], options: vscode.AuthenticationGetSessionOptions): Thenable { + getSession(providerId: string, scopes: string[], options: vscode.AuthenticationGetSessionOptions): Thenable { return extHostAuthentication.getSession(extension, providerId, scopes, options); }, getSessions(providerId: string, scopes: string[]): Thenable { @@ -1076,7 +1076,8 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I TimelineItem: extHostTypes.TimelineItem, CellKind: extHostTypes.CellKind, CellOutputKind: extHostTypes.CellOutputKind, - NotebookCellRunState: extHostTypes.NotebookCellRunState + NotebookCellRunState: extHostTypes.NotebookCellRunState, + AuthenticationSession2: extHostTypes.AuthenticationSession }; }; } diff --git a/src/vs/workbench/api/common/extHostAuthentication.ts b/src/vs/workbench/api/common/extHostAuthentication.ts index 59dfdb260cf..0ffdaff9d14 100644 --- a/src/vs/workbench/api/common/extHostAuthentication.ts +++ b/src/vs/workbench/api/common/extHostAuthentication.ts @@ -43,7 +43,7 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { return !!(await provider.getSessions()).filter(session => session.scopes.sort().join(' ') === orderedScopes).length; } - async getSession(requestingExtension: IExtensionDescription, providerId: string, scopes: string[], options: vscode.AuthenticationGetSessionOptions): Promise { + async getSession(requestingExtension: IExtensionDescription, providerId: string, scopes: string[], options: vscode.AuthenticationGetSessionOptions): Promise { const provider = this._authenticationProviders.get(providerId); if (!provider) { throw new Error(`No authentication provider with id '${providerId}' is currently registered.`); @@ -113,7 +113,7 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { throw new Error('User did not consent to token access.'); } - return session.getAccessToken(); + return session.accessToken; } }; }); @@ -149,7 +149,7 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { throw new Error('User did not consent to token access.'); } - return session.getAccessToken(); + return session.accessToken; } }; } @@ -219,7 +219,7 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { const sessions = await authProvider.getSessions(); const session = sessions.find(session => session.id === sessionId); if (session) { - return session.getAccessToken(); + return session.accessToken; } throw new Error(`Unable to find session with id: ${sessionId}`); diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index 4894d276e46..fb460278bf5 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -2766,3 +2766,11 @@ export enum ExtensionMode { } //#endregion ExtensionContext + + +//#region Authentication +export class AuthenticationSession implements vscode.AuthenticationSession2 { + constructor(public id: string, public accessToken: string, public account: { displayName: string, id: string }, public scopes: string[]) { } +} + +//#endregion Authentication diff --git a/src/vs/workbench/contrib/userDataSync/browser/userDataSyncAccount.ts b/src/vs/workbench/contrib/userDataSync/browser/userDataSyncAccount.ts index a8c4cbdd708..9894869cf65 100644 --- a/src/vs/workbench/contrib/userDataSync/browser/userDataSyncAccount.ts +++ b/src/vs/workbench/contrib/userDataSync/browser/userDataSyncAccount.ts @@ -38,7 +38,7 @@ export class UserDataSyncAccount { get sessionId(): string { return this.session.id; } get accountName(): string { return this.session.account.displayName; } get accountId(): string { return this.session.account.id; } - getToken(): Thenable { return this.session.getAccessToken(); } + get token(): string { return this.session.accessToken; } } export class UserDataSyncAccounts extends Disposable { @@ -159,7 +159,7 @@ export class UserDataSyncAccounts extends Disposable { if (current) { try { this.logService.trace('Preferences Sync: Updating the token for the account', current.accountName); - const token = await current.getToken(); + const token = current.token; this.logService.trace('Preferences Sync: Token updated for the account', current.accountName); value = { token, authenticationProviderId: current.authenticationProviderId }; } catch (e) {