From ab7a5ec3ade55070225a70837e5c6c30322fd98c Mon Sep 17 00:00:00 2001 From: Rachel Macfarlane Date: Tue, 30 Jun 2020 17:59:42 -0700 Subject: [PATCH] Address part of feedback for #100993 --- .../github-authentication/src/extension.ts | 2 +- .../microsoft-authentication/src/extension.ts | 2 +- src/vs/editor/common/modes.ts | 8 ++-- src/vs/vscode.proposed.d.ts | 45 ++++++++++--------- .../api/browser/mainThreadAuthentication.ts | 16 +++---- .../workbench/api/common/extHost.protocol.ts | 2 +- .../api/common/extHostAuthentication.ts | 12 ++--- .../parts/activitybar/activitybarActions.ts | 2 +- .../userDataSync/browser/userDataSync.ts | 6 +-- .../browser/authenticationService.ts | 8 ++-- .../browser/userDataSyncWorkbenchService.ts | 4 +- 11 files changed, 56 insertions(+), 51 deletions(-) diff --git a/extensions/github-authentication/src/extension.ts b/extensions/github-authentication/src/extension.ts index 3980ba7c7fd..f9fa5210aae 100644 --- a/extensions/github-authentication/src/extension.ts +++ b/extensions/github-authentication/src/extension.ts @@ -24,7 +24,7 @@ export async function activate(context: vscode.ExtensionContext) { vscode.authentication.registerAuthenticationProvider({ id: 'github', - displayName: 'GitHub', + label: 'GitHub', supportsMultipleAccounts: false, onDidChangeSessions: onDidChangeSessions.event, getSessions: () => Promise.resolve(loginService.sessions), diff --git a/extensions/microsoft-authentication/src/extension.ts b/extensions/microsoft-authentication/src/extension.ts index ed751d48fb3..08aaae0f0f6 100644 --- a/extensions/microsoft-authentication/src/extension.ts +++ b/extensions/microsoft-authentication/src/extension.ts @@ -19,7 +19,7 @@ export async function activate(context: vscode.ExtensionContext) { context.subscriptions.push(vscode.authentication.registerAuthenticationProvider({ id: 'microsoft', - displayName: 'Microsoft', + label: 'Microsoft', supportsMultipleAccounts: true, onDidChangeSessions: onDidChangeSessions.event, getSessions: () => Promise.resolve(loginService.sessions), diff --git a/src/vs/editor/common/modes.ts b/src/vs/editor/common/modes.ts index 89a77eed92e..88c9caf5fe3 100644 --- a/src/vs/editor/common/modes.ts +++ b/src/vs/editor/common/modes.ts @@ -1416,16 +1416,16 @@ export interface AuthenticationSession { displayName: string; id: string; } - scopes: string[]; + scopes: ReadonlyArray; } /** * @internal */ export interface AuthenticationSessionsChangeEvent { - added: string[]; - removed: string[]; - changed: string[]; + added: ReadonlyArray; + removed: ReadonlyArray; + changed: ReadonlyArray; } export interface Command { diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 0cc285c6071..84ba37ce730 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -34,25 +34,30 @@ declare module 'vscode' { /** * 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; - }; + readonly account: AuthenticationSessionAccountInformation; /** * The permissions granted by the session's access token. Available scopes * are defined by the authentication provider. */ - readonly scopes: string[]; + readonly scopes: ReadonlyArray; - constructor(id: string, accessToken: string, account: { displayName: string, id: string }, scopes: string[]); + constructor(id: string, accessToken: string, account: AuthenticationSessionAccountInformation, scopes: string[]); + } + + /** + * The information of an account associated with an authentication session. + */ + export interface AuthenticationSessionAccountInformation { + /** + * The human-readable name of the account. + */ + readonly displayName: string; + + /** + * The unique identifier of the account. + */ + readonly id: string; } /** @@ -62,12 +67,12 @@ declare module 'vscode' { /** * The ids of the [authenticationProvider](#AuthenticationProvider)s that have been added. */ - readonly added: string[]; + readonly added: ReadonlyArray; /** * The ids of the [authenticationProvider](#AuthenticationProvider)s that have been removed. */ - readonly removed: string[]; + readonly removed: ReadonlyArray; } /** @@ -93,17 +98,17 @@ declare module 'vscode' { /** * The ids of the [AuthenticationSession](#AuthenticationSession)s that have been added. */ - readonly added: string[]; + readonly added: ReadonlyArray; /** * The ids of the [AuthenticationSession](#AuthenticationSession)s that have been removed. */ - readonly removed: string[]; + readonly removed: ReadonlyArray; /** * The ids of the [AuthenticationSession](#AuthenticationSession)s that have been changed. */ - readonly changed: string[]; + readonly changed: ReadonlyArray; } /** @@ -122,7 +127,7 @@ declare module 'vscode' { /** * The human-readable name of the provider. */ - readonly displayName: string; + readonly label: string; /** * Whether it is possible to be signed into multiple accounts at once with this provider @@ -179,7 +184,7 @@ declare module 'vscode' { * @deprecated * An array of the ids of authentication providers that are currently registered. */ - export const providerIds: string[]; + export const providerIds: ReadonlyArray; /** * Returns whether a provider has any sessions matching the requested scopes. This request diff --git a/src/vs/workbench/api/browser/mainThreadAuthentication.ts b/src/vs/workbench/api/browser/mainThreadAuthentication.ts index f1df047dc5a..f7d90a25e0d 100644 --- a/src/vs/workbench/api/browser/mainThreadAuthentication.ts +++ b/src/vs/workbench/api/browser/mainThreadAuthentication.ts @@ -75,7 +75,7 @@ export class MainThreadAuthenticationProvider extends Disposable { constructor( private readonly _proxy: ExtHostAuthenticationShape, public readonly id: string, - public readonly displayName: string, + public readonly label: string, public readonly supportsMultipleAccounts: boolean, private readonly notificationService: INotificationService, private readonly storageKeysSyncRegistryService: IStorageKeysSyncRegistryService, @@ -235,8 +235,8 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu return Promise.resolve(this.authenticationService.getProviderIds()); } - async $registerAuthenticationProvider(id: string, displayName: string, supportsMultipleAccounts: boolean): Promise { - const provider = new MainThreadAuthenticationProvider(this._proxy, id, displayName, supportsMultipleAccounts, this.notificationService, this.storageKeysSyncRegistryService, this.storageService, this.quickInputService, this.dialogService); + async $registerAuthenticationProvider(id: string, label: string, supportsMultipleAccounts: boolean): Promise { + const provider = new MainThreadAuthenticationProvider(this._proxy, id, label, supportsMultipleAccounts, this.notificationService, this.storageKeysSyncRegistryService, this.storageService, this.quickInputService, this.dialogService); await provider.initialize(); this.authenticationService.registerAuthenticationProvider(id, provider); } @@ -267,13 +267,13 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu async $getSession(providerId: string, scopes: string[], extensionId: string, extensionName: string, options: { createIfNone: boolean, clearSessionPreference: boolean }): Promise { const orderedScopes = scopes.sort().join(' '); - const sessions = (await this.$getSessions(providerId)).filter(session => session.scopes.sort().join(' ') === orderedScopes); - const displayName = this.authenticationService.getDisplayName(providerId); + const sessions = (await this.$getSessions(providerId)).filter(session => session.scopes.slice().sort().join(' ') === orderedScopes); + const label = this.authenticationService.getLabel(providerId); if (sessions.length) { if (!this.authenticationService.supportsMultipleAccounts(providerId)) { const session = sessions[0]; - const allowed = await this.$getSessionsPrompt(providerId, session.account.displayName, displayName, extensionId, extensionName); + const allowed = await this.$getSessionsPrompt(providerId, session.account.displayName, label, extensionId, extensionName); if (allowed) { return session; } else { @@ -282,11 +282,11 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu } // On renderer side, confirm consent, ask user to choose between accounts if multiple sessions are valid - const selected = await this.$selectSession(providerId, displayName, extensionId, extensionName, sessions, scopes, !!options.clearSessionPreference); + const selected = await this.$selectSession(providerId, label, extensionId, extensionName, sessions, scopes, !!options.clearSessionPreference); return sessions.find(session => session.id === selected.id); } else { if (options.createIfNone) { - const isAllowed = await this.$loginPrompt(displayName, extensionName); + const isAllowed = await this.$loginPrompt(label, extensionName); if (!isAllowed) { throw new Error('User did not consent to login.'); } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 0490360f285..edd6bbd616c 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 MainThreadCommentsShape extends IDisposable { } export interface MainThreadAuthenticationShape extends IDisposable { - $registerAuthenticationProvider(id: string, displayName: string, supportsMultipleAccounts: boolean): void; + $registerAuthenticationProvider(id: string, label: string, supportsMultipleAccounts: boolean): void; $unregisterAuthenticationProvider(id: string): void; $getProviderIds(): Promise; $sendDidChangeSessions(providerId: string, event: modes.AuthenticationSessionsChangeEvent): void; diff --git a/src/vs/workbench/api/common/extHostAuthentication.ts b/src/vs/workbench/api/common/extHostAuthentication.ts index 107a5614834..ae19b1d0a5d 100644 --- a/src/vs/workbench/api/common/extHostAuthentication.ts +++ b/src/vs/workbench/api/common/extHostAuthentication.ts @@ -53,7 +53,7 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { async hasSessions(providerId: string, scopes: string[]): Promise { const orderedScopes = scopes.sort().join(' '); const sessions = await this.resolveSessions(providerId); - return !!(sessions.filter(session => session.scopes.sort().join(' ') === orderedScopes).length); + return !!(sessions.filter(session => session.scopes.slice().sort().join(' ') === orderedScopes).length); } async getSession(requestingExtension: IExtensionDescription, providerId: string, scopes: string[], options: vscode.AuthenticationGetSessionOptions & { createIfNone: true }): Promise; @@ -67,12 +67,12 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { } const orderedScopes = scopes.sort().join(' '); - const sessions = (await provider.getSessions()).filter(session => session.scopes.sort().join(' ') === orderedScopes); + const sessions = (await provider.getSessions()).filter(session => session.scopes.slice().sort().join(' ') === orderedScopes); if (sessions.length) { if (!provider.supportsMultipleAccounts) { const session = sessions[0]; - const allowed = await this._proxy.$getSessionsPrompt(providerId, session.account.displayName, provider.displayName, extensionId, extensionName); + const allowed = await this._proxy.$getSessionsPrompt(providerId, session.account.displayName, provider.label, extensionId, extensionName); if (allowed) { return session; } else { @@ -81,11 +81,11 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { } // On renderer side, confirm consent, ask user to choose between accounts if multiple sessions are valid - const selected = await this._proxy.$selectSession(providerId, provider.displayName, extensionId, extensionName, sessions, scopes, !!options.clearSessionPreference); + const selected = await this._proxy.$selectSession(providerId, provider.label, extensionId, extensionName, sessions, scopes, !!options.clearSessionPreference); return sessions.find(session => session.id === selected.id); } else { if (options.createIfNone) { - const isAllowed = await this._proxy.$loginPrompt(provider.displayName, extensionName); + const isAllowed = await this._proxy.$loginPrompt(provider.label, extensionName); if (!isAllowed) { throw new Error('User did not consent to login.'); } @@ -120,7 +120,7 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { this._proxy.$sendDidChangeSessions(provider.id, e); }); - this._proxy.$registerAuthenticationProvider(provider.id, provider.displayName, provider.supportsMultipleAccounts); + this._proxy.$registerAuthenticationProvider(provider.id, provider.label, provider.supportsMultipleAccounts); return new Disposable(() => { listener.dispose(); diff --git a/src/vs/workbench/browser/parts/activitybar/activitybarActions.ts b/src/vs/workbench/browser/parts/activitybar/activitybarActions.ts index dd706f66468..e81e4527ec3 100644 --- a/src/vs/workbench/browser/parts/activitybar/activitybarActions.ts +++ b/src/vs/workbench/browser/parts/activitybar/activitybarActions.ts @@ -151,7 +151,7 @@ export class AccountsActionViewItem extends ActivityActionViewItem { const result = await Promise.all(allSessions); let menus: (IAction | ContextSubMenu)[] = []; result.forEach(sessionInfo => { - const providerDisplayName = this.authenticationService.getDisplayName(sessionInfo.providerId); + const providerDisplayName = this.authenticationService.getLabel(sessionInfo.providerId); sessionInfo.sessions.forEach(session => { const accountName = session.account.displayName; const menu = new ContextSubMenu(`${accountName} (${providerDisplayName})`, [ diff --git a/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts b/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts index 436487eab2f..586b7f4f541 100644 --- a/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts +++ b/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts @@ -483,8 +483,8 @@ export class UserDataSyncWorkbenchContribution extends Disposable implements IWo } else { const orTerm = localize({ key: 'or', comment: ['Here is the context where it is used - Sign in with your A or B or C account to synchronize your data across devices.'] }, "or"); const displayName = this.userDataSyncWorkbenchService.authenticationProviders.length === 1 - ? this.authenticationService.getDisplayName(this.userDataSyncWorkbenchService.authenticationProviders[0].id) - : this.userDataSyncWorkbenchService.authenticationProviders.map(({ id }) => this.authenticationService.getDisplayName(id)).join(` ${orTerm} `); + ? this.authenticationService.getLabel(this.userDataSyncWorkbenchService.authenticationProviders[0].id) + : this.userDataSyncWorkbenchService.authenticationProviders.map(({ id }) => this.authenticationService.getLabel(id)).join(` ${orTerm} `); quickPick.description = localize('sign in and turn on sync detail', "Sign in with your {0} account to synchronize your data across devices.", displayName); quickPick.customLabel = localize('sign in and turn on sync', "Sign in & Turn on"); } @@ -900,7 +900,7 @@ export class UserDataSyncWorkbenchContribution extends Disposable implements IWo items.push({ id: syncNowCommand.id, label: syncNowCommand.title, description: syncNowCommand.description(that.userDataSyncService) }); if (that.userDataAutoSyncService.canToggleEnablement()) { const account = that.userDataSyncWorkbenchService.current; - items.push({ id: turnOffSyncCommand.id, label: turnOffSyncCommand.title, description: account ? `${account.accountName} (${that.authenticationService.getDisplayName(account.authenticationProviderId)})` : undefined }); + items.push({ id: turnOffSyncCommand.id, label: turnOffSyncCommand.title, description: account ? `${account.accountName} (${that.authenticationService.getLabel(account.authenticationProviderId)})` : undefined }); } quickPick.items = items; disposables.add(quickPick.onDidAccept(() => { diff --git a/src/vs/workbench/services/authentication/browser/authenticationService.ts b/src/vs/workbench/services/authentication/browser/authenticationService.ts index 6a2e8562974..22247c9904c 100644 --- a/src/vs/workbench/services/authentication/browser/authenticationService.ts +++ b/src/vs/workbench/services/authentication/browser/authenticationService.ts @@ -33,7 +33,7 @@ export interface IAuthenticationService { readonly onDidChangeSessions: Event<{ providerId: string, event: AuthenticationSessionsChangeEvent }>; getSessions(providerId: string): Promise>; - getDisplayName(providerId: string): string; + getLabel(providerId: string): string; supportsMultipleAccounts(providerId: string): boolean; login(providerId: string, scopes: string[]): Promise; logout(providerId: string, sessionId: string): Promise; @@ -187,7 +187,7 @@ export class AuthenticationService extends Disposable implements IAuthentication let changed = false; Object.keys(existingRequestsForProvider).forEach(requestedScopes => { - if (sessions.some(session => session.scopes.sort().join('') === requestedScopes)) { + if (sessions.some(session => session.scopes.slice().sort().join('') === requestedScopes)) { // Request has been completed changed = true; const sessionRequest = existingRequestsForProvider[requestedScopes]; @@ -295,10 +295,10 @@ export class AuthenticationService extends Disposable implements IAuthentication this._badgeDisposable = this.activityService.showAccountsActivity({ badge }); } } - getDisplayName(id: string): string { + getLabel(id: string): string { const authProvider = this._authenticationProviders.get(id); if (authProvider) { - return authProvider.displayName; + return authProvider.label; } else { throw new Error(`No authentication provider '${id}' is currently registered.`); } diff --git a/src/vs/workbench/services/userDataSync/browser/userDataSyncWorkbenchService.ts b/src/vs/workbench/services/userDataSync/browser/userDataSyncWorkbenchService.ts index 578c545aa3c..9d479364190 100644 --- a/src/vs/workbench/services/userDataSync/browser/userDataSyncWorkbenchService.ts +++ b/src/vs/workbench/services/userDataSync/browser/userDataSyncWorkbenchService.ts @@ -361,7 +361,7 @@ export class UserDataSyncWorkbenchService extends Disposable implements IUserDat quickPickItems.push({ type: 'separator', label: localize('signed in', "Signed in") }); for (const authenticationProvider of authenticationProviders) { const accounts = (this._all.get(authenticationProvider.id) || []).sort(({ sessionId }) => sessionId === this.current?.sessionId ? -1 : 1); - const providerName = this.authenticationService.getDisplayName(authenticationProvider.id); + const providerName = this.authenticationService.getLabel(authenticationProvider.id); for (const account of accounts) { quickPickItems.push({ label: `${account.accountName} (${providerName})`, @@ -378,7 +378,7 @@ export class UserDataSyncWorkbenchService extends Disposable implements IUserDat for (const authenticationProvider of this.authenticationProviders) { const signedInForProvider = this.all.some(account => account.authenticationProviderId === authenticationProvider.id); if (!signedInForProvider || this.authenticationService.supportsMultipleAccounts(authenticationProvider.id)) { - const providerName = this.authenticationService.getDisplayName(authenticationProvider.id); + const providerName = this.authenticationService.getLabel(authenticationProvider.id); quickPickItems.push({ label: localize('sign in using account', "Sign in with {0}", providerName), authenticationProvider }); } } -- GitLab