diff --git a/extensions/github-authentication/package.json b/extensions/github-authentication/package.json index c6255115310..eb0e7f2b618 100644 --- a/extensions/github-authentication/package.json +++ b/extensions/github-authentication/package.json @@ -19,7 +19,8 @@ ], "enabledApiProposals": [ "authIssuers", - "authProviderSpecific" + "authProviderSpecific", + "authSessionAccountIcon" ], "activationEvents": [], "capabilities": { diff --git a/extensions/github-authentication/src/github.ts b/extensions/github-authentication/src/github.ts index e9f97282bf8..05bce7c2a9b 100644 --- a/extensions/github-authentication/src/github.ts +++ b/extensions/github-authentication/src/github.ts @@ -14,6 +14,17 @@ import { crypto } from './node/crypto'; import { TIMED_OUT_ERROR, USER_CANCELLATION_ERROR } from './common/errors'; import { GitHubSocialSignInProvider, isSocialSignInProvider } from './flows'; +/** + * The stored (JSON) form of a vscode.Uri pointing to the account's avatar. + */ +interface StoredAccountIcon { + scheme: string; + authority?: string; + path?: string; + query?: string; + fragment?: string; +} + interface SessionData { id: string; account?: { @@ -22,11 +33,31 @@ interface SessionData { // Unfortunately, for some time the id was a number, so we need to support both. // This can be removed once we are confident that all users have migrated to the new id. id: string | number; + // `undefined` means the avatar has not been looked up yet, `null` means a lookup + // completed and found no avatar, and a `StoredAccountIcon` is a resolved avatar. + icon?: StoredAccountIcon | null; }; scopes: string[]; accessToken: string; } +/** + * Whether a stored session's account icon still needs to be looked up. + */ +export function needsAccountIconLookup(session: SessionData): boolean { + return !session.account || session.account.icon === undefined; +} + +/** + * Serializes an account icon for storage, using `null` to mark a completed lookup that found no avatar. + */ +export function serializeAccountIcon(icon: vscode.Uri | undefined, hasNoAvatar: boolean): StoredAccountIcon | null | undefined { + if (icon) { + return { scheme: icon.scheme, authority: icon.authority, path: icon.path, query: icon.query, fragment: icon.fragment }; + } + return hasNoAvatar ? null : undefined; +} + export enum AuthProviderType { github = 'github', githubEnterprise = 'github-enterprise' @@ -134,6 +165,7 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid private readonly _telemetryReporter: ExperimentationTelemetry; private readonly _keychain: Keychain; private readonly _accountsSeen = new Set(); + private readonly _sessionsWithoutAvatars = new WeakSet(); private readonly _disposable: vscode.Disposable | undefined; private _sessionsPromise: Promise; @@ -278,19 +310,25 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid // the sessions to migrate away from the bad number usage. // TODO@TylerLeonhardt: Remove this after we are confident that all users have migrated to the new id. let seenNumberAccountId: boolean = false; + // Sessions that were stored before the account icon was introduced are re-stored + // once an icon has been fetched so that we don't refetch it on every read. + let seenIconUpdate: boolean = false; // TODO: eventually remove this Set because we should only have one session per set of scopes. const scopesSeen = new Set(); const sessionPromises = sessionData.map(async (session: SessionData): Promise => { // For GitHub scope list, order doesn't matter so we immediately sort the scopes const scopesStr = [...session.scopes].sort().join(' '); - let userInfo: { id: string; accountName: string } | undefined; - if (!session.account) { + let userInfo: { id: string; accountName: string; avatarUrl: string | undefined } | undefined; + if (needsAccountIconLookup(session)) { + const needsAccount = !session.account; try { userInfo = await this._githubServer.getUserInfo(session.accessToken); - this._logger.info(`Verified session with the following scopes: ${scopesStr}`); + seenIconUpdate = true; + if (needsAccount) { + this._logger.info(`Verified session with the following scopes: ${scopesStr}`); + } } catch (e) { - // Remove sessions that return unauthorized response - if (e.message === 'Unauthorized') { + if (e.message === 'Unauthorized' && needsAccount) { return undefined; } } @@ -308,19 +346,30 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid } else { accountId = userInfo?.id ?? ''; } - return { + let icon: vscode.Uri | undefined; + if (session.account?.icon?.scheme) { + icon = vscode.Uri.from(session.account.icon); + } else if (userInfo?.avatarUrl) { + icon = vscode.Uri.parse(userInfo.avatarUrl); + } + const resolvedSession: vscode.AuthenticationSession = { id: session.id, account: { label: session.account ? session.account.label ?? session.account.displayName ?? '' - : userInfo?.accountName ?? '', - id: accountId + : (userInfo?.accountName ?? ''), + id: accountId, + icon, }, // we set this to session.scopes to maintain the original order of the scopes requested // by the extension that called getSession() scopes: session.scopes, accessToken: session.accessToken }; + if (!icon && (session.account?.icon === null || userInfo)) { + this._sessionsWithoutAvatars.add(resolvedSession); + } + return resolvedSession; }); const verifiedSessions = (await Promise.allSettled(sessionPromises)) @@ -329,7 +378,7 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid .filter((p?: T): p is T => Boolean(p)); this._logger.info(`Got ${verifiedSessions.length} verified sessions.`); - if (seenNumberAccountId || verifiedSessions.length !== sessionData.length) { + if (seenNumberAccountId || seenIconUpdate || verifiedSessions.length !== sessionData.length) { await this.storeSessions(verifiedSessions); } @@ -339,7 +388,17 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid private async storeSessions(sessions: vscode.AuthenticationSession[]): Promise { this._logger.info(`Storing ${sessions.length} sessions...`); this._sessionsPromise = Promise.resolve(sessions); - await this._keychain.setToken(JSON.stringify(sessions)); + const storedSessions: SessionData[] = sessions.map(session => ({ + id: session.id, + account: { + label: session.account.label, + id: session.account.id, + icon: serializeAccountIcon(session.account.icon, this._sessionsWithoutAvatars.has(session)), + }, + scopes: [...session.scopes], + accessToken: session.accessToken + })); + await this._keychain.setToken(JSON.stringify(storedSessions)); this._logger.info(`Stored ${sessions.length} sessions!`); } @@ -409,12 +468,16 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid private async tokenToSession(token: string, scopes: string[]): Promise { const userInfo = await this._githubServer.getUserInfo(token); - return { + const session: vscode.AuthenticationSession = { id: crypto.getRandomValues(new Uint32Array(2)).reduce((prev, curr) => prev += curr.toString(16), ''), accessToken: token, - account: { label: userInfo.accountName, id: userInfo.id }, + account: { label: userInfo.accountName, id: userInfo.id, icon: userInfo.avatarUrl ? vscode.Uri.parse(userInfo.avatarUrl) : undefined }, scopes }; + if (!session.account.icon) { + this._sessionsWithoutAvatars.add(session); + } + return session; } public async removeSession(id: string) { diff --git a/extensions/github-authentication/src/githubServer.ts b/extensions/github-authentication/src/githubServer.ts index b8646696a8a..02d44dacb6a 100644 --- a/extensions/github-authentication/src/githubServer.ts +++ b/extensions/github-authentication/src/githubServer.ts @@ -21,7 +21,7 @@ const REDIRECT_URL_INSIDERS = 'https://insiders.vscode.dev/redirect'; export interface IGitHubServer { login(scopes: string, signInProvider?: GitHubSocialSignInProvider, extraAuthorizeParameters?: Record, existingLogin?: string): Promise; logout(session: vscode.AuthenticationSession): Promise; - getUserInfo(token: string): Promise<{ id: string; accountName: string }>; + getUserInfo(token: string): Promise<{ id: string; accountName: string; avatarUrl: string | undefined }>; sendAdditionalTelemetryInfo(session: vscode.AuthenticationSession): Promise; friendlyName: string; } @@ -217,7 +217,7 @@ export class GitHubServer implements IGitHubServer { return vscode.Uri.parse(`${apiUri.scheme}://${apiUri.authority}/api/v3${path}`); } - public async getUserInfo(token: string): Promise<{ id: string; accountName: string }> { + public async getUserInfo(token: string): Promise<{ id: string; accountName: string; avatarUrl: string | undefined }> { let result; try { this._logger.info('Getting user info...'); @@ -237,9 +237,9 @@ export class GitHubServer implements IGitHubServer { if (result.ok) { try { - const json = await result.json() as { id: number; login: string }; + const json = await result.json() as { id: number; login: string; avatar_url?: string }; this._logger.info('Got account info!'); - return { id: `${json.id}`, accountName: json.login }; + return { id: `${json.id}`, accountName: json.login, avatarUrl: json.avatar_url }; } catch (e) { this._logger.error(`Unexpected error parsing response from GitHub: ${e.message ?? e}`); throw e; diff --git a/extensions/github-authentication/src/test/github.test.ts b/extensions/github-authentication/src/test/github.test.ts new file mode 100644 index 00000000000..e3a776d9d17 --- /dev/null +++ b/extensions/github-authentication/src/test/github.test.ts @@ -0,0 +1,43 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import * as vscode from 'vscode'; +import { needsAccountIconLookup, serializeAccountIcon } from '../github'; + +suite('account avatar caching', () => { + test('a pending session needs a lookup, and its serialized no-avatar result no longer needs one', () => { + const pendingSession = { + id: 'session1', + account: { id: 'account1', label: 'Some One' }, + scopes: [], + accessToken: 'token' + }; + const noAvatarIcon = serializeAccountIcon(undefined, true); + const cachedNoAvatarSession = { + id: 'session1', + account: { id: 'account1', label: 'Some One', icon: noAvatarIcon }, + scopes: [], + accessToken: 'token' + }; + + assert.deepStrictEqual( + [needsAccountIconLookup(pendingSession), noAvatarIcon, needsAccountIconLookup(cachedNoAvatarSession)], + [true, null, false] + ); + }); + + test('a resolved avatar URI is serialized as-is and is never replaced by null', () => { + const icon = vscode.Uri.parse('https://example.com/avatar.png'); + + assert.deepStrictEqual(serializeAccountIcon(icon, true), { + scheme: icon.scheme, + authority: icon.authority, + path: icon.path, + query: icon.query, + fragment: icon.fragment + }); + }); +}); diff --git a/extensions/github-authentication/tsconfig.json b/extensions/github-authentication/tsconfig.json index cdbe2a54f36..ff8545f4854 100644 --- a/extensions/github-authentication/tsconfig.json +++ b/extensions/github-authentication/tsconfig.json @@ -19,6 +19,7 @@ "src/**/*", "../../src/vscode-dts/vscode.d.ts", "../../src/vscode-dts/vscode.proposed.authIssuers.d.ts", - "../../src/vscode-dts/vscode.proposed.authProviderSpecific.d.ts" + "../../src/vscode-dts/vscode.proposed.authProviderSpecific.d.ts", + "../../src/vscode-dts/vscode.proposed.authSessionAccountIcon.d.ts" ] } diff --git a/src/vs/platform/extensions/common/extensionsApiProposals.ts b/src/vs/platform/extensions/common/extensionsApiProposals.ts index 73631e61b42..bc632d5321d 100644 --- a/src/vs/platform/extensions/common/extensionsApiProposals.ts +++ b/src/vs/platform/extensions/common/extensionsApiProposals.ts @@ -39,6 +39,9 @@ const _allApiProposals = { authSession: { proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.authSession.d.ts', }, + authSessionAccountIcon: { + proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.authSessionAccountIcon.d.ts', + }, authSessionAudience: { proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.authSessionAudience.d.ts', }, diff --git a/src/vs/sessions/LAYOUT.md b/src/vs/sessions/LAYOUT.md index 450b162be4d..b61bdd29893 100644 --- a/src/vs/sessions/LAYOUT.md +++ b/src/vs/sessions/LAYOUT.md @@ -180,7 +180,7 @@ Approval acknowledgement must use the pending tool call's stable id, not the app ### Account Widget (Right) -Shows the signed-in GitHub profile image (falls back to the account codicon). Clicking opens a combined account and Copilot status panel with sign-in/sign-out and settings actions. +Shows the account profile image, preferring the avatar supplied by the authentication provider for the default account's session, falling back to the public GitHub profile image URL derived from the account name, and finally to the account codicon. Clicking opens a combined account and Copilot status panel with sign-in/sign-out and settings actions. ### Remote Connections (Right) diff --git a/src/vs/sessions/browser/accountTitleBarState.ts b/src/vs/sessions/browser/accountTitleBarState.ts index 5f652575056..e16c6aba37e 100644 --- a/src/vs/sessions/browser/accountTitleBarState.ts +++ b/src/vs/sessions/browser/accountTitleBarState.ts @@ -4,7 +4,9 @@ *--------------------------------------------------------------------------------------------*/ import { Codicon } from '../../base/common/codicons.js'; +import { FileAccess } from '../../base/common/network.js'; import { ThemeIcon } from '../../base/common/themables.js'; +import { URI } from '../../base/common/uri.js'; import { localize } from '../../nls.js'; import { ChatEntitlement, IChatSentiment, IQuotaSnapshot } from '../../workbench/services/chat/common/chatEntitlementService.js'; import { IDefaultAccountService } from '../../platform/defaultAccount/common/defaultAccount.js'; @@ -14,6 +16,11 @@ export interface IResolvedAccountInfo { readonly accountName: string; readonly accountProviderId: string; readonly accountProviderLabel: string; + /** + * The icon (avatar) supplied by the authentication provider for this + * account, if any. + */ + readonly accountIcon?: URI; } /** @@ -32,6 +39,7 @@ export async function resolveAccountInfo( accountName: account.accountName, accountProviderId: account.authenticationProvider.id, accountProviderLabel: account.authenticationProvider.name, + accountIcon: await getSessionAccountIcon(authenticationService, account.authenticationProvider.id, account.sessionId), }; } @@ -42,6 +50,7 @@ export async function resolveAccountInfo( accountName: sessions[0].account.label, accountProviderId: 'github', accountProviderLabel: 'GitHub', + accountIcon: sessions[0].account.icon, }; } } catch { @@ -51,6 +60,20 @@ export async function resolveAccountInfo( return undefined; } +/** + * Looks up the icon (avatar) that the authentication provider supplied for the + * session backing the default account, if any. + */ +async function getSessionAccountIcon(authenticationService: IAuthenticationService, providerId: string, sessionId: string): Promise { + try { + const sessions = await authenticationService.getSessions(providerId); + return sessions.find(session => session.id === sessionId)?.account.icon; + } catch { + // Provider not available yet + return undefined; + } +} + export type AccountTitleBarStateSource = 'account' | 'copilot'; export type AccountTitleBarStateKind = 'default' | 'accent' | 'warning' | 'prominent'; @@ -84,7 +107,11 @@ export interface IAccountTitleBarState { readonly revealLabelOnHover?: boolean; } -export function getAccountProfileImageUrl(accountProviderId: string | undefined, accountName: string | undefined): string | undefined { +export function getAccountProfileImageUrl(accountProviderId: string | undefined, accountName: string | undefined, accountIcon?: URI): string | undefined { + if (accountIcon) { + return FileAccess.uriToBrowserUri(accountIcon).toString(true); + } + if (accountProviderId !== 'github' || !accountName?.trim()) { return undefined; } diff --git a/src/vs/sessions/browser/parts/mobile/mobileTitlebarPart.ts b/src/vs/sessions/browser/parts/mobile/mobileTitlebarPart.ts index ab8cc20f80d..1bbf6fdfe54 100644 --- a/src/vs/sessions/browser/parts/mobile/mobileTitlebarPart.ts +++ b/src/vs/sessions/browser/parts/mobile/mobileTitlebarPart.ts @@ -18,8 +18,9 @@ import { HiddenItemStrategy, MenuWorkbenchToolBar } from '../../../../platform/a import { IMenuService } from '../../../../platform/actions/common/actions.js'; import { fillInActionBarActions } from '../../../../platform/actions/browser/menuEntryActionViewItem.js'; import { ICommandService } from '../../../../platform/commands/common/commands.js'; +import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; import { IDefaultAccountService } from '../../../../platform/defaultAccount/common/defaultAccount.js'; -import { IAuthenticationService } from '../../../../workbench/services/authentication/common/authentication.js'; +import { ACCOUNTS_AVATAR_SETTING, IAuthenticationService } from '../../../../workbench/services/authentication/common/authentication.js'; import { ISessionsService } from '../../../services/sessions/browser/sessionsService.js'; import { ISessionFileChange } from '../../../services/sessions/common/session.js'; import { IsNewChatSessionContext } from '../../../common/contextkeys.js'; @@ -29,6 +30,7 @@ import { ChatEntitlement, ChatEntitlementService, IChatEntitlementService } from import { getAccountTitleBarState, getAccountProfileImageUrl, getAccountTitleBarBadgeKey, resolveAccountInfo } from '../../accountTitleBarState.js'; import { IChatDashboardService } from '../../chatDashboardService.js'; import { MOBILE_OPEN_CHANGES_VIEW_COMMAND_ID } from './contributions/mobileChangesView.js'; +import { URI } from '../../../../base/common/uri.js'; /** * Mobile titlebar — prepended above the workbench grid on phone viewports @@ -82,6 +84,7 @@ export class MobileTitlebarPart extends Disposable { private accountName: string | undefined; private accountProviderId: string | undefined; private accountProviderLabel: string | undefined; + private accountIcon: URI | undefined; private isAccountLoading = true; private accountRequestCounter = 0; private avatarRequestCounter = 0; @@ -109,6 +112,7 @@ export class MobileTitlebarPart extends Disposable { @IMenuService private readonly menuService: IMenuService, @IChatDashboardService private readonly chatDashboardService: IChatDashboardService, @ICommandService private readonly commandService: ICommandService, + @IConfigurationService private readonly configurationService: IConfigurationService, ) { super(); @@ -195,6 +199,11 @@ export class MobileTitlebarPart extends Disposable { this._register(this.chatEntitlementService.onDidChangeSentiment(() => this.renderAccountState())); this._register(this.chatEntitlementService.onDidChangeQuotaExceeded(() => this.renderAccountState())); this._register(this.chatEntitlementService.onDidChangeQuotaRemaining(() => this.renderAccountState())); + this._register(this.configurationService.onDidChangeConfiguration(e => { + if (e.affectsConfiguration(ACCOUNTS_AVATAR_SETTING)) { + this.refreshAvatar(); + } + })); this.refreshAccount(); // Keep the title in sync with the active session @@ -322,6 +331,7 @@ export class MobileTitlebarPart extends Disposable { this.accountName = info?.accountName; this.accountProviderId = info?.accountProviderId; this.accountProviderLabel = info?.accountProviderLabel; + this.accountIcon = info?.accountIcon; this.isAccountLoading = false; this.refreshAvatar(); this.renderAccountState(); @@ -378,7 +388,9 @@ export class MobileTitlebarPart extends Disposable { } private refreshAvatar(): void { - const avatarUrl = getAccountProfileImageUrl(this.accountProviderId, this.accountName); + const avatarUrl = this.configurationService.getValue(ACCOUNTS_AVATAR_SETTING) + ? getAccountProfileImageUrl(this.accountProviderId, this.accountName, this.accountIcon) + : undefined; if (avatarUrl === this.currentAvatarUrl) { return; } diff --git a/src/vs/sessions/contrib/accountMenu/browser/account.contribution.ts b/src/vs/sessions/contrib/accountMenu/browser/account.contribution.ts index a7ffe2a5e7e..5e9a3e23cfd 100644 --- a/src/vs/sessions/contrib/accountMenu/browser/account.contribution.ts +++ b/src/vs/sessions/contrib/accountMenu/browser/account.contribution.ts @@ -40,7 +40,8 @@ import { IsPhoneLayoutContext, SessionHasChangesContext, SessionIsCreatedContext import { IsAuxiliaryWindowContext } from '../../../../workbench/common/contextkeys.js'; import { IAuthenticationAccessService } from '../../../../workbench/services/authentication/browser/authenticationAccessService.js'; import { IAuthenticationUsageService } from '../../../../workbench/services/authentication/browser/authenticationUsageService.js'; -import { IAuthenticationService } from '../../../../workbench/services/authentication/common/authentication.js'; +import { ACCOUNTS_AVATAR_SETTING, IAuthenticationService } from '../../../../workbench/services/authentication/common/authentication.js'; +import { URI } from '../../../../base/common/uri.js'; import { IChatDashboardService } from '../../../browser/chatDashboardService.js'; import { InstantiationType, registerSingleton } from '../../../../platform/instantiation/common/extensions.js'; import { createCodexAccountMenuActions, hasSignedInCodexChatGPTAccount, ICodexAccountService, shouldShowCodexAccount } from '../../../../workbench/services/agentHost/browser/codexAccountService.js'; @@ -180,6 +181,7 @@ class TitleBarAccountWidget extends BaseActionViewItem { private accountName: string | undefined; private accountProviderId: string | undefined; private accountProviderLabel: string | undefined; + private accountIcon: URI | undefined; private isAccountLoading = true; private accountRequestCounter = 0; private avatarRequestCounter = 0; @@ -234,6 +236,9 @@ class TitleBarAccountWidget extends BaseActionViewItem { this.clickPanelDisposable.clear(); this.renderState(); } + if (event.affectsConfiguration(ACCOUNTS_AVATAR_SETTING)) { + this.refreshAvatar(); + } })); // A signed-out user sees either a quiet "Sign In" (the opt-in is on, so signing // in is optional) or a prominent "Agents Signed Out". Re-render so toggling the @@ -288,6 +293,7 @@ class TitleBarAccountWidget extends BaseActionViewItem { this.accountName = info?.accountName; this.accountProviderId = info?.accountProviderId; this.accountProviderLabel = info?.accountProviderLabel; + this.accountIcon = info?.accountIcon; this.isAccountLoading = false; this.refreshAvatar(); this.renderState(); @@ -366,7 +372,9 @@ class TitleBarAccountWidget extends BaseActionViewItem { } private refreshAvatar(): void { - const avatarUrl = getAccountProfileImageUrl(this.accountProviderId, this.accountName); + const avatarUrl = this.configurationService.getValue(ACCOUNTS_AVATAR_SETTING) + ? getAccountProfileImageUrl(this.accountProviderId, this.accountName, this.accountIcon) + : undefined; if (avatarUrl === this.currentAvatarUrl) { return; } diff --git a/src/vs/sessions/contrib/accountMenu/test/browser/accountTitleBarState.test.ts b/src/vs/sessions/contrib/accountMenu/test/browser/accountTitleBarState.test.ts index b25e5630cb9..466e84d43e8 100644 --- a/src/vs/sessions/contrib/accountMenu/test/browser/accountTitleBarState.test.ts +++ b/src/vs/sessions/contrib/accountMenu/test/browser/accountTitleBarState.test.ts @@ -4,9 +4,15 @@ *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; +import { IDefaultAccount } from '../../../../../base/common/defaultAccount.js'; +import { FileAccess } from '../../../../../base/common/network.js'; +import { URI } from '../../../../../base/common/uri.js'; +import { mock } from '../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { IDefaultAccountService } from '../../../../../platform/defaultAccount/common/defaultAccount.js'; +import { AuthenticationSession, IAuthenticationService } from '../../../../../workbench/services/authentication/common/authentication.js'; import { ChatEntitlement } from '../../../../../workbench/services/chat/common/chatEntitlementService.js'; -import { getAccountProfileImageUrl, getAccountTitleBarBadgeKey, getAccountTitleBarState, IAccountTitleBarStateContext } from '../../../../browser/accountTitleBarState.js'; +import { getAccountProfileImageUrl, getAccountTitleBarBadgeKey, getAccountTitleBarState, IAccountTitleBarStateContext, resolveAccountInfo } from '../../../../browser/accountTitleBarState.js'; suite('Sessions - Account Title Bar State', () => { @@ -171,9 +177,61 @@ suite('Sessions - Account Title Bar State', () => { ); }); + test('prefers the account icon supplied by the authentication provider', () => { + assert.strictEqual( + getAccountProfileImageUrl('github', 'mona lisa', URI.parse('https://avatars.githubusercontent.com/u/1?v=4')), + 'https://avatars.githubusercontent.com/u/1?v=4' + ); + assert.strictEqual( + getAccountProfileImageUrl('github-enterprise', 'octocat', URI.parse('https://example.com/avatar.png')), + 'https://example.com/avatar.png' + ); + }); + + test('converts a provider supplied file icon into a browser safe URL', () => { + const icon = URI.file('/home/octocat/avatar.png'); + + assert.strictEqual( + getAccountProfileImageUrl('github', 'octocat', icon), + FileAccess.uriToBrowserUri(icon).toString(true) + ); + }); + test('falls back to the codicon when no GitHub profile image URL is available', () => { assert.strictEqual(getAccountProfileImageUrl(undefined, 'octocat'), undefined); assert.strictEqual(getAccountProfileImageUrl('github-enterprise', 'octocat'), undefined); assert.strictEqual(getAccountProfileImageUrl('github', undefined), undefined); }); + + test('resolves the default account icon by session id, not by label', async () => { + const sessions: AuthenticationSession[] = [ + { id: 'stale-session', accessToken: 'token', scopes: ['scope'], account: { id: 'account', label: 'octocat', icon: URI.parse('https://example.com/stale.png') } }, + { id: 'default-session', accessToken: 'token', scopes: ['scope'], account: { id: 'account', label: 'octocat', icon: URI.parse('https://example.com/default.png') } }, + ]; + const defaultAccountService = new class extends mock() { + override async getDefaultAccount(): Promise { + return { + authenticationProvider: { id: 'github', name: 'GitHub', enterprise: false }, + accountName: 'octocat', + sessionId: 'default-session', + enterprise: false, + }; + } + }; + const authenticationService = new class extends mock() { + override async getSessions(): Promise> { + return sessions; + } + }; + + assert.deepStrictEqual( + await resolveAccountInfo(defaultAccountService, authenticationService), + { + accountName: 'octocat', + accountProviderId: 'github', + accountProviderLabel: 'GitHub', + accountIcon: URI.parse('https://example.com/default.png'), + } + ); + }); }); diff --git a/src/vs/workbench/api/browser/mainThreadAuthentication.ts b/src/vs/workbench/api/browser/mainThreadAuthentication.ts index 2d61963381a..30137e70004 100644 --- a/src/vs/workbench/api/browser/mainThreadAuthentication.ts +++ b/src/vs/workbench/api/browser/mainThreadAuthentication.ts @@ -22,6 +22,7 @@ import { IOpenerService } from '../../../platform/opener/common/opener.js'; import { CancellationError } from '../../../base/common/errors.js'; import { ILogService } from '../../../platform/log/common/log.js'; import { ExtensionHostKind } from '../../services/extensions/common/extensionHostKind.js'; +import { Dto, Proxied } from '../../services/extensions/common/proxyIdentifier.js'; import { IURLService } from '../../../platform/url/common/url.js'; import { DeferredPromise, raceTimeout } from '../../../base/common/async.js'; import { fetchAuthorizationServerMetadata, IAuthorizationTokenResponse } from '../../../base/common/oauth.js'; @@ -49,12 +50,20 @@ export interface AuthenticationGetSessionOptions { authorizationServer?: UriComponents; } +/** + * The account icon is a {@link URI} that does not survive being sent over the RPC boundary, + * so it needs to be revived when sessions are received from the extension host. + */ +export function reviveSessionAccountIcon(session: Dto): AuthenticationSession { + return { ...session, account: { ...session.account, icon: URI.revive(session.account.icon) } }; +} + class MainThreadAuthenticationProvider extends Disposable implements IAuthenticationProvider { readonly onDidChangeSessions: Event; constructor( - protected readonly _proxy: ExtHostAuthenticationShape, + protected readonly _proxy: Proxied, public readonly id: string, public readonly label: string, public readonly supportsMultipleAccounts: boolean, @@ -67,11 +76,12 @@ class MainThreadAuthenticationProvider extends Disposable implements IAuthentica } async getSessions(scopes: string[] | undefined, options: IAuthenticationProviderSessionOptions) { - return this._proxy.$getSessions(this.id, scopes, options); + const sessions = await this._proxy.$getSessions(this.id, scopes, options); + return sessions.map(reviveSessionAccountIcon); } - createSession(scopes: string[], options: IAuthenticationProviderSessionOptions): Promise { - return this._proxy.$createSession(this.id, scopes, options); + async createSession(scopes: string[], options: IAuthenticationProviderSessionOptions): Promise { + return reviveSessionAccountIcon(await this._proxy.$createSession(this.id, scopes, options)); } async removeSession(sessionId: string): Promise { @@ -82,7 +92,7 @@ class MainThreadAuthenticationProvider extends Disposable implements IAuthentica class MainThreadAuthenticationProviderWithChallenges extends MainThreadAuthenticationProvider implements IAuthenticationProvider { constructor( - proxy: ExtHostAuthenticationShape, + proxy: Proxied, id: string, label: string, supportsMultipleAccounts: boolean, @@ -101,18 +111,19 @@ class MainThreadAuthenticationProviderWithChallenges extends MainThreadAuthentic ); } - getSessionsFromChallenges(constraint: IAuthenticationConstraint, options: IAuthenticationProviderSessionOptions): Promise { - return this._proxy.$getSessionsFromChallenges(this.id, constraint, options); + async getSessionsFromChallenges(constraint: IAuthenticationConstraint, options: IAuthenticationProviderSessionOptions): Promise { + const sessions = await this._proxy.$getSessionsFromChallenges(this.id, constraint, options); + return sessions.map(reviveSessionAccountIcon); } - createSessionFromChallenges(constraint: IAuthenticationConstraint, options: IAuthenticationProviderSessionOptions): Promise { - return this._proxy.$createSessionFromChallenges(this.id, constraint, options); + async createSessionFromChallenges(constraint: IAuthenticationConstraint, options: IAuthenticationProviderSessionOptions): Promise { + return reviveSessionAccountIcon(await this._proxy.$createSessionFromChallenges(this.id, constraint, options)); } } @extHostNamedCustomer(MainContext.MainThreadAuthentication) export class MainThreadAuthentication extends Disposable implements MainThreadAuthenticationShape { - private readonly _proxy: ExtHostAuthenticationShape; + private readonly _proxy: Proxied; private readonly _registrations = this._register(new DisposableMap()); private _sentProviderUsageEvents = new Set(); @@ -272,10 +283,14 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu } } - async $sendDidChangeSessions(providerId: string, event: AuthenticationSessionsChangeEvent): Promise { + async $sendDidChangeSessions(providerId: string, event: Dto): Promise { const obj = this._registrations.get(providerId); if (obj instanceof Emitter) { - obj.fire(event); + obj.fire({ + added: event.added?.map(reviveSessionAccountIcon), + removed: event.removed?.map(reviveSessionAccountIcon), + changed: event.changed?.map(reviveSessionAccountIcon) + }); } } @@ -530,7 +545,7 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu return undefined; } - async $getSession(providerId: string, scopeListOrRequest: ReadonlyArray | IAuthenticationWwwAuthenticateRequest, extensionId: string, extensionName: string, options: AuthenticationGetSessionOptions): Promise { + async $getSession(providerId: string, scopeListOrRequest: ReadonlyArray | IAuthenticationWwwAuthenticateRequest, extensionId: string, extensionName: string, options: AuthenticationGetSessionOptions): Promise | undefined> { const scopes = isAuthenticationWwwAuthenticateRequest(scopeListOrRequest) ? scopeListOrRequest.fallbackScopes : scopeListOrRequest; if (scopes) { this.sendClientIdUsageTelemetry(extensionId, providerId, scopes); @@ -545,7 +560,7 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu return session; } - async $getAccounts(providerId: string): Promise> { + async $getAccounts(providerId: string): Promise>> { const accounts = await this.authenticationService.getAccounts(providerId); return accounts; } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 8d3e9aeda4d..52d18657ef8 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -225,9 +225,9 @@ export interface MainThreadAuthenticationShape extends IDisposable { $registerAuthenticationProvider(details: IRegisterAuthenticationProviderDetails): Promise; $unregisterAuthenticationProvider(id: string): Promise; $ensureProvider(id: string): Promise; - $sendDidChangeSessions(providerId: string, event: AuthenticationSessionsChangeEvent): Promise; - $getSession(providerId: string, scopeListOrRequest: ReadonlyArray | IAuthenticationWwwAuthenticateRequest, extensionId: string, extensionName: string, options: AuthenticationGetSessionOptions): Promise; - $getAccounts(providerId: string): Promise>; + $sendDidChangeSessions(providerId: string, event: Dto): Promise; + $getSession(providerId: string, scopeListOrRequest: ReadonlyArray | IAuthenticationWwwAuthenticateRequest, extensionId: string, extensionName: string, options: AuthenticationGetSessionOptions): Promise | undefined>; + $getAccounts(providerId: string): Promise>>; $removeSession(providerId: string, sessionId: string): Promise; $waitForUriHandler(expectedUri: UriComponents): Promise; $showContinueNotification(message: string): Promise; diff --git a/src/vs/workbench/api/common/extHostAuthentication.ts b/src/vs/workbench/api/common/extHostAuthentication.ts index 1926bb30345..f6093339f21 100644 --- a/src/vs/workbench/api/common/extHostAuthentication.ts +++ b/src/vs/workbench/api/common/extHostAuthentication.ts @@ -7,6 +7,7 @@ import type * as vscode from 'vscode'; import * as nls from '../../../nls.js'; import { Emitter, Event } from '../../../base/common/event.js'; import { MainContext, MainThreadAuthenticationShape, ExtHostAuthenticationShape } from './extHost.protocol.js'; +import { Proxied } from '../../services/extensions/common/proxyIdentifier.js'; import { Disposable, ProgressLocation } from './extHostTypes.js'; import { IExtensionDescription, ExtensionIdentifier } from '../../../platform/extensions/common/extensions.js'; import { IAuthenticationGetSessionsOptions, IAuthenticationProviderSessionOptions, INTERNAL_AUTH_PROVIDER_PREFIX, isAuthenticationWwwAuthenticateRequest } from '../../services/authentication/common/authentication.js'; @@ -39,6 +40,14 @@ interface ProviderWithMetadata { options: vscode.AuthenticationProviderOptions; } +/** + * The account icon is a {@link vscode.Uri} that does not survive being sent over the RPC boundary, + * so it needs to be revived when an account is received from the main thread. + */ +export function reviveAccountIcon(account: T): T & { readonly icon?: vscode.Uri } { + return { ...account, icon: URI.revive(account.icon) }; +} + export class ExtHostAuthentication implements ExtHostAuthenticationShape { declare _serviceBrand: undefined; @@ -46,7 +55,7 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { protected readonly _dynamicAuthProviderCtor = DynamicAuthProvider; protected readonly _xaaAuthProviderCtor = XaaifyAuthProvider(DynamicAuthProvider); - private _proxy: MainThreadAuthenticationShape; + private _proxy: Proxied; private _authenticationProviders: Map = new Map(); private _providerOperations = new SequencerByKey(); @@ -124,13 +133,15 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { return await this._getSessionTaskSingler.getOrCreate(singlerKey, async () => { await this._proxy.$ensureProvider(providerId); const extensionName = requestingExtension.displayName || requestingExtension.name; - return this._proxy.$getSession(providerId, scopesOrRequest, extensionId, extensionName, options); + const session = await this._proxy.$getSession(providerId, scopesOrRequest, extensionId, extensionName, options); + return session && { ...session, account: reviveAccountIcon(session.account) }; }); } async getAccounts(providerId: string) { await this._proxy.$ensureProvider(providerId); - return await this._proxy.$getAccounts(providerId); + const accounts = await this._proxy.$getAccounts(providerId); + return accounts.map(account => reviveAccountIcon(account)); } registerAuthenticationProvider(id: string, label: string, provider: vscode.AuthenticationProvider, options?: vscode.AuthenticationProviderOptions): vscode.Disposable { @@ -172,6 +183,9 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { const providerData = this._authenticationProviders.get(providerId); if (providerData) { options.authorizationServer = URI.revive(options.authorizationServer); + if (options.account) { + options.account = reviveAccountIcon(options.account); + } return await providerData.provider.createSession(scopes, options); } @@ -195,6 +209,9 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { const providerData = this._authenticationProviders.get(providerId); if (providerData) { options.authorizationServer = URI.revive(options.authorizationServer); + if (options.account) { + options.account = reviveAccountIcon(options.account); + } return await providerData.provider.getSessions(scopes, options); } @@ -210,6 +227,9 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { // Check if provider supports challenges if (typeof provider.getSessionsFromChallenges === 'function') { options.authorizationServer = URI.revive(options.authorizationServer); + if (options.account) { + options.account = reviveAccountIcon(options.account); + } return await provider.getSessionsFromChallenges(constraint, options); } throw new Error(`Authentication provider with handle: ${providerId} does not support getSessionsFromChallenges`); @@ -227,6 +247,9 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { // Check if provider supports challenges if (typeof provider.createSessionFromChallenges === 'function') { options.authorizationServer = URI.revive(options.authorizationServer); + if (options.account) { + options.account = reviveAccountIcon(options.account); + } return await provider.createSessionFromChallenges(constraint, options); } throw new Error(`Authentication provider with handle: ${providerId} does not support createSessionFromChallenges`); @@ -458,7 +481,7 @@ export class DynamicAuthProvider implements vscode.AuthenticationProvider { @IExtHostInitDataService protected readonly _initData: IExtHostInitDataService, @IExtHostProgress private readonly _extHostProgress: IExtHostProgress, @ILoggerService loggerService: ILoggerService, - protected readonly _proxy: MainThreadAuthenticationShape, + protected readonly _proxy: Proxied, readonly authorizationServer: URI, protected readonly _serverMetadata: IAuthorizationServerMetadata, protected readonly _resourceMetadata: IAuthorizationProtectedResourceMetadata | undefined, diff --git a/src/vs/workbench/api/node/extHostAuthentication.ts b/src/vs/workbench/api/node/extHostAuthentication.ts index 828a91c336f..6060d6b80da 100644 --- a/src/vs/workbench/api/node/extHostAuthentication.ts +++ b/src/vs/workbench/api/node/extHostAuthentication.ts @@ -14,6 +14,7 @@ import { IExtHostWindow } from '../common/extHostWindow.js'; import { IExtHostUrlsService } from '../common/extHostUrls.js'; import { ILoggerService, ILogService } from '../../../platform/log/common/log.js'; import { MainThreadAuthenticationShape } from '../common/extHost.protocol.js'; +import { Proxied } from '../../services/extensions/common/proxyIdentifier.js'; import { IAuthorizationServerMetadata, IAuthorizationProtectedResourceMetadata, IAuthorizationTokenResponse, IAuthorizationDeviceResponse, isAuthorizationDeviceResponse, isAuthorizationTokenResponse, IAuthorizationDeviceTokenErrorResponse, AuthorizationErrorType, AuthorizationDeviceCodeErrorType } from '../../../base/common/oauth.js'; import { Emitter } from '../../../base/common/event.js'; import { raceCancellationError } from '../../../base/common/async.js'; @@ -31,7 +32,7 @@ export class NodeDynamicAuthProvider extends DynamicAuthProvider { initData: IExtHostInitDataService, extHostProgress: IExtHostProgress, loggerService: ILoggerService, - proxy: MainThreadAuthenticationShape, + proxy: Proxied, authorizationServer: URI, serverMetadata: IAuthorizationServerMetadata, resourceMetadata: IAuthorizationProtectedResourceMetadata | undefined, diff --git a/src/vs/workbench/api/test/browser/extHostAuthentication.test.ts b/src/vs/workbench/api/test/browser/extHostAuthentication.test.ts index 450e6e1d231..4f6c3cb750a 100644 --- a/src/vs/workbench/api/test/browser/extHostAuthentication.test.ts +++ b/src/vs/workbench/api/test/browser/extHostAuthentication.test.ts @@ -6,17 +6,18 @@ import assert from 'assert'; import { encodeBase64, VSBuffer } from '../../../../base/common/buffer.js'; import { Emitter } from '../../../../base/common/event.js'; -import { URI } from '../../../../base/common/uri.js'; +import { URI, UriComponents } from '../../../../base/common/uri.js'; import { mock } from '../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; import { ILogger, ILoggerService, NullLogger } from '../../../../platform/log/common/log.js'; import { IAuthenticationProviderSessionOptions } from '../../../services/authentication/common/authentication.js'; -import { DynamicAuthProvider, IAuthorizationToken, TokenStore } from '../../common/extHostAuthentication.js'; +import { DynamicAuthProvider, IAuthorizationToken, reviveAccountIcon, TokenStore } from '../../common/extHostAuthentication.js'; import { MainThreadAuthenticationShape } from '../../common/extHost.protocol.js'; import { IExtHostInitDataService } from '../../common/extHostInitDataService.js'; import { IExtHostProgress } from '../../common/extHostProgress.js'; import { IExtHostUrlsService } from '../../common/extHostUrls.js'; import { IExtHostWindow } from '../../common/extHostWindow.js'; +import { Proxied } from '../../../services/extensions/common/proxyIdentifier.js'; /** Builds a structurally-valid JWT carrying the given claims. */ function jwt(claims: object): string { @@ -80,10 +81,8 @@ suite('DynamicAuthProvider', () => { return new NullLogger(); } }(); - const proxy = new class extends mock() { - override $setSessionsForDynamicAuthProvider(): Promise { - return Promise.resolve(); - } + const proxy = new class extends mock>() { + override $setSessionsForDynamicAuthProvider = (): Promise => Promise.resolve(); }(); const provider = disposables.add(new TestDynamicAuthProvider( new class extends mock() { }(), @@ -128,3 +127,23 @@ suite('DynamicAuthProvider', () => { }); }); }); + +suite('Account Icon Revival', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + const iconComponents: UriComponents = { scheme: 'https', authority: 'example.com', path: '/avatar.png', query: '', fragment: '' }; + + test('reviveAccountIcon revives a present icon into a URI and leaves a missing icon undefined', () => { + const withIcon: { id: string; label: string; icon?: UriComponents } = { id: 'account-with-icon', label: 'Has Icon', icon: iconComponents }; + const withoutIcon: { id: string; label: string; icon?: UriComponents } = { id: 'account-without-icon', label: 'No Icon' }; + + assert.deepStrictEqual( + [reviveAccountIcon(withIcon), reviveAccountIcon(withoutIcon)], + [ + { ...withIcon, icon: URI.from(iconComponents) }, + { ...withoutIcon, icon: undefined } + ] + ); + }); +}); diff --git a/src/vs/workbench/api/test/browser/mainThreadAuthentication.test.ts b/src/vs/workbench/api/test/browser/mainThreadAuthentication.test.ts new file mode 100644 index 00000000000..6d27f0a9c08 --- /dev/null +++ b/src/vs/workbench/api/test/browser/mainThreadAuthentication.test.ts @@ -0,0 +1,41 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { URI, UriComponents } from '../../../../base/common/uri.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { AuthenticationSession } from '../../../services/authentication/common/authentication.js'; +import { Dto } from '../../../services/extensions/common/proxyIdentifier.js'; +import { reviveSessionAccountIcon } from '../../browser/mainThreadAuthentication.js'; + +suite('MainThreadAuthentication', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + const iconComponents: UriComponents = { scheme: 'https', authority: 'example.com', path: '/avatar.png', query: '', fragment: '' }; + + test('reviveSessionAccountIcon revives a session\'s account icon into a URI and leaves a missing icon undefined', () => { + const withIcon: Dto = { + id: 'session-with-icon', + accessToken: 'token', + scopes: ['scope'], + account: { id: 'account-with-icon', label: 'Has Icon', icon: iconComponents } + }; + const withoutIcon: Dto = { + id: 'session-without-icon', + accessToken: 'token', + scopes: ['scope'], + account: { id: 'account-without-icon', label: 'No Icon' } + }; + + assert.deepStrictEqual( + [reviveSessionAccountIcon(withIcon), reviveSessionAccountIcon(withoutIcon)], + [ + { ...withIcon, account: { ...withIcon.account, icon: URI.from(iconComponents) } }, + { ...withoutIcon, account: { ...withoutIcon.account, icon: undefined } } + ] + ); + }); +}); diff --git a/src/vs/workbench/browser/parts/globalCompositeBar.ts b/src/vs/workbench/browser/parts/globalCompositeBar.ts index 0d1395626b6..2187088a06d 100644 --- a/src/vs/workbench/browser/parts/globalCompositeBar.ts +++ b/src/vs/workbench/browser/parts/globalCompositeBar.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import './media/globalCompositeBar.css'; import { localize } from '../../../nls.js'; import { ActionBar, ActionsOrientation } from '../../../base/browser/ui/actionbar/actionbar.js'; import { ACCOUNTS_ACTIVITY_ID, GLOBAL_ACTIVITY_ID } from '../../common/activity.js'; @@ -33,17 +34,20 @@ import { ILogService } from '../../../platform/log/common/log.js'; import { IProductService } from '../../../platform/product/common/productService.js'; import { ISecretStorageService } from '../../../platform/secrets/common/secrets.js'; import { AuthenticationSessionInfo, getCurrentAuthenticationSessionInfo } from '../../services/authentication/browser/authenticationService.js'; -import { AuthenticationSessionAccount, IAuthenticationService, INTERNAL_AUTH_PROVIDER_PREFIX } from '../../services/authentication/common/authentication.js'; +import { ACCOUNTS_AVATAR_SETTING, AuthenticationSessionAccount, IAuthenticationService, INTERNAL_AUTH_PROVIDER_PREFIX } from '../../services/authentication/common/authentication.js'; import { IWorkbenchEnvironmentService } from '../../services/environment/common/environmentService.js'; import { IHoverService } from '../../../platform/hover/browser/hover.js'; import { ILifecycleService, LifecyclePhase } from '../../services/lifecycle/common/lifecycle.js'; import { IUserDataProfileService } from '../../services/userDataProfile/common/userDataProfile.js'; import { DEFAULT_ICON } from '../../services/userDataProfile/common/userDataProfileIcons.js'; import { isString } from '../../../base/common/types.js'; +import { FileAccess } from '../../../base/common/network.js'; +import { URI } from '../../../base/common/uri.js'; import { KeyCode } from '../../../base/common/keyCodes.js'; import { ACTIVITY_BAR_BADGE_BACKGROUND, ACTIVITY_BAR_BADGE_FOREGROUND } from '../../common/theme.js'; import { IBaseActionViewItemOptions } from '../../../base/browser/ui/actionbar/actionViewItems.js'; import { ICommandService } from '../../../platform/commands/common/commands.js'; +import { IDefaultAccountService } from '../../../platform/defaultAccount/common/defaultAccount.js'; import { WORKBENCH_MENU_MOTION_CLASS, workbenchMenuCloseAnimation } from '../actions/menuMotion.js'; import { createCodexAccountMenuActions, ICodexAccountService, shouldShowCodexAccount } from '../../services/agentHost/browser/codexAccountService.js'; @@ -270,6 +274,7 @@ export class AccountsActivityActionViewItem extends AbstractGlobalActivityAction private initialized = false; private sessionFromEmbedder = new Lazy>(() => getCurrentAuthenticationSessionInfo(this.secretStorageService, this.productService)); + private avatarImg: HTMLImageElement | undefined; constructor( contextMenuActionsProvider: () => IAction[], @@ -293,6 +298,7 @@ export class AccountsActivityActionViewItem extends AbstractGlobalActivityAction @IInstantiationService instantiationService: IInstantiationService, @ICommandService private readonly commandService: ICommandService, @ICodexAccountService private readonly codexAccountService: ICodexAccountService, + @IDefaultAccountService private readonly defaultAccountService: IDefaultAccountService, ) { const action = instantiationService.createInstance(CompositeBarAction, { id: ACCOUNTS_ACTIVITY_ID, @@ -308,11 +314,13 @@ export class AccountsActivityActionViewItem extends AbstractGlobalActivityAction private registerListeners(): void { this._register(this.authenticationService.onDidRegisterAuthenticationProvider(async (e) => { await this.addAccountsFromProvider(e.id); + this.updateAvatar(); })); this._register(this.authenticationService.onDidUnregisterAuthenticationProvider((e) => { this.groupedAccounts.delete(e.id); this.problematicProviders.delete(e.id); + this.updateAvatar(); })); this._register(this.authenticationService.onDidChangeSessions(async e => { @@ -328,6 +336,17 @@ export class AccountsActivityActionViewItem extends AbstractGlobalActivityAction this.logService.error(e); } } + this.updateAvatar(); + })); + + this._register(this.configurationService.onDidChangeConfiguration(e => { + if (e.affectsConfiguration(ACCOUNTS_AVATAR_SETTING)) { + this.updateAvatar(); + } + })); + + this._register(this.defaultAccountService.onDidChangeDefaultAccount(() => { + this.updateAvatar(); })); } @@ -358,6 +377,69 @@ export class AccountsActivityActionViewItem extends AbstractGlobalActivityAction } this.initialized = true; + this.updateAvatar(); + } + + override render(container: HTMLElement): void { + super.render(container); + + this.avatarImg = $('img.accounts-avatar') as HTMLImageElement; + this.avatarImg.alt = ''; + this.avatarImg.setAttribute('aria-hidden', 'true'); + this.avatarImg.draggable = false; + this.avatarImg.referrerPolicy = 'no-referrer'; + this.avatarImg.style.display = 'none'; + this.avatarImg.onerror = () => { + this.avatarImg!.style.display = 'none'; + this.label.classList.remove('has-avatar'); + }; + append(this.label, this.avatarImg); + + this.updateAvatar(); + } + + private updateAvatar(): void { + if (!this.avatarImg) { + return; + } + + let avatarIcon: URI | undefined; + if (this.configurationService.getValue(ACCOUNTS_AVATAR_SETTING)) { + avatarIcon = this.getDefaultAccountAvatarIcon(); + if (!avatarIcon) { + for (const accounts of this.groupedAccounts.values()) { + for (const account of accounts) { + if (account.icon) { + avatarIcon = account.icon; + break; + } + } + if (avatarIcon) { + break; + } + } + } + } + + if (avatarIcon) { + this.avatarImg.src = FileAccess.uriToBrowserUri(avatarIcon).toString(true); + this.avatarImg.style.display = ''; + this.label.classList.add('has-avatar'); + } else { + this.avatarImg.removeAttribute('src'); + this.avatarImg.style.display = 'none'; + this.label.classList.remove('has-avatar'); + } + } + + private getDefaultAccountAvatarIcon(): URI | undefined { + const currentDefaultAccount = this.defaultAccountService.currentDefaultAccount; + if (!currentDefaultAccount) { + return undefined; + } + + const accounts = this.groupedAccounts.get(currentDefaultAccount.authenticationProvider.id); + return accounts?.find(account => account.label === currentDefaultAccount.accountName)?.icon; } //#region overrides @@ -554,6 +636,7 @@ export class AccountsActivityActionViewItem extends AbstractGlobalActivityAction if (!canSignOut) { existingAccount.canSignOut = canSignOut; } + existingAccount.icon = account.icon; } else { accounts.push({ ...account, canSignOut }); } @@ -699,6 +782,7 @@ export class SimpleAccountActivityActionViewItem extends AccountsActivityActionV @IInstantiationService instantiationService: IInstantiationService, @ICommandService commandService: ICommandService, @ICodexAccountService codexAccountService: ICodexAccountService, + @IDefaultAccountService defaultAccountService: IDefaultAccountService, ) { super(() => simpleActivityContextMenuActions(storageService, true), { @@ -709,7 +793,7 @@ export class SimpleAccountActivityActionViewItem extends AccountsActivityActionV }), hoverOptions, compact: true, - }, () => undefined, actions => actions, themeService, lifecycleService, hoverService, contextMenuService, menuService, contextKeyService, authenticationService, environmentService, productService, configurationService, keybindingService, secretStorageService, logService, activityService, instantiationService, commandService, codexAccountService); + }, () => undefined, actions => actions, themeService, lifecycleService, hoverService, contextMenuService, menuService, contextKeyService, authenticationService, environmentService, productService, configurationService, keybindingService, secretStorageService, logService, activityService, instantiationService, commandService, codexAccountService, defaultAccountService); } } diff --git a/src/vs/workbench/browser/parts/media/globalCompositeBar.css b/src/vs/workbench/browser/parts/media/globalCompositeBar.css new file mode 100644 index 00000000000..69a4c169336 --- /dev/null +++ b/src/vs/workbench/browser/parts/media/globalCompositeBar.css @@ -0,0 +1,15 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +.monaco-workbench .monaco-action-bar .action-label.has-avatar::before { + display: none; +} + +.monaco-workbench .monaco-action-bar .action-label .accounts-avatar { + width: 1em; + height: 1em; + object-fit: cover; + border-radius: var(--vscode-cornerRadius-circle); +} diff --git a/src/vs/workbench/browser/workbench.contribution.ts b/src/vs/workbench/browser/workbench.contribution.ts index 3c3d1367dcd..4afff4f2dcd 100644 --- a/src/vs/workbench/browser/workbench.contribution.ts +++ b/src/vs/workbench/browser/workbench.contribution.ts @@ -12,6 +12,7 @@ import { Registry } from '../../platform/registry/common/platform.js'; import { ConfigurationKeyValuePairs, ConfigurationMigrationWorkbenchContribution, DynamicWindowConfiguration, DynamicWorkbenchSecurityConfiguration, Extensions, IConfigurationMigrationRegistry, problemsConfigurationNodeBase, windowConfigurationNodeBase, workbenchConfigurationNodeBase } from '../common/configuration.js'; import { WorkbenchPhase, registerWorkbenchContribution2 } from '../common/contributions.js'; import { NotificationsPosition, NotificationsSettings } from '../common/notifications.js'; +import { ACCOUNTS_AVATAR_SETTING } from '../services/authentication/common/authentication.js'; import { CustomEditorLabelService } from '../services/editor/common/customEditorLabelService.js'; import { MOUSE_BACK_FORWARD_NAVIGATION_SETTING } from '../services/history/common/history.js'; import { ActivityBarPosition, EditorActionsLocation, EditorTabsMode, LayoutSettings } from '../services/layout/browser/layoutService.js'; @@ -651,6 +652,11 @@ const registry = Registry.as(ConfigurationExtensions.Con 'default': true, 'description': localize('notificationsButton', "Controls the visibility of the Notifications button in the title bar. Only applies when notifications are positioned at the top right.") }, + [ACCOUNTS_AVATAR_SETTING]: { + 'type': 'boolean', + 'default': true, + 'description': localize('accountsShowAvatar', "Controls whether signed-in account profile images (avatars) are shown in account-related UI, such as the Accounts item in the Activity Bar.") + }, [LayoutSettings.ACTIVITY_BAR_LOCATION]: { 'type': 'string', 'enum': ['default', 'top', 'bottom', 'hidden'], diff --git a/src/vs/workbench/services/authentication/common/authentication.ts b/src/vs/workbench/services/authentication/common/authentication.ts index d940213756f..bd9683498b3 100644 --- a/src/vs/workbench/services/authentication/common/authentication.ts +++ b/src/vs/workbench/services/authentication/common/authentication.ts @@ -13,9 +13,19 @@ import { createDecorator } from '../../../../platform/instantiation/common/insta */ export const INTERNAL_AUTH_PROVIDER_PREFIX = '__'; +/** + * Setting that controls whether the profile image (avatar) of a signed-in account + * is shown in account related UI. + */ +export const ACCOUNTS_AVATAR_SETTING = 'workbench.accounts.showAvatar'; + export interface AuthenticationSessionAccount { label: string; id: string; + /** + * An optional icon for the account. This is typically a URI to a profile image/avatar. + */ + icon?: URI; } export interface AuthenticationSession { diff --git a/src/vs/workbench/test/browser/parts/globalCompositeBar.test.ts b/src/vs/workbench/test/browser/parts/globalCompositeBar.test.ts new file mode 100644 index 00000000000..1308b1f77dd --- /dev/null +++ b/src/vs/workbench/test/browser/parts/globalCompositeBar.test.ts @@ -0,0 +1,128 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { URI } from '../../../../base/common/uri.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { IDefaultAccount } from '../../../../base/common/defaultAccount.js'; +import { AccountsActivityActionViewItem } from '../../../browser/parts/globalCompositeBar.js'; +import { AuthenticationSession, AuthenticationSessionAccount } from '../../../services/authentication/common/authentication.js'; + +interface IUpdateAvatarTestHarness { + avatarImg: HTMLImageElement; + label: HTMLElement; + configurationService: { getValue(): boolean }; + groupedAccounts: Map; + defaultAccountService: { currentDefaultAccount: IDefaultAccount | null }; + getDefaultAccountAvatarIcon(): URI | undefined; +} + +interface IAddOrUpdateAccountTestHarness { + groupedAccounts: Map; + sessionFromEmbedder: { value: Promise }; + authenticationService: { getSessions(): Promise }; +} + +const updateAvatar = Reflect.get(AccountsActivityActionViewItem.prototype, 'updateAvatar') as (this: IUpdateAvatarTestHarness) => void; +const getDefaultAccountAvatarIcon = Reflect.get(AccountsActivityActionViewItem.prototype, 'getDefaultAccountAvatarIcon') as (this: IUpdateAvatarTestHarness) => URI | undefined; +const addOrUpdateAccount = Reflect.get(AccountsActivityActionViewItem.prototype, 'addOrUpdateAccount') as (this: IAddOrUpdateAccountTestHarness, providerId: string, account: AuthenticationSessionAccount) => Promise; + +function createDefaultAccount(providerId: string, accountName: string): IDefaultAccount { + return { + authenticationProvider: { id: providerId, name: providerId, enterprise: false }, + accountName, + sessionId: 'test-session', + enterprise: false, + }; +} + +function createHarness(groupedAccounts: Map, currentDefaultAccount: IDefaultAccount | null): IUpdateAvatarTestHarness { + return { + avatarImg: document.createElement('img'), + label: document.createElement('div'), + configurationService: { getValue: () => true }, + groupedAccounts, + defaultAccountService: { currentDefaultAccount }, + getDefaultAccountAvatarIcon, + }; +} + +suite('AccountsActivityActionViewItem - updateAvatar', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + const firstIcon = URI.parse('https://example.com/first.png'); + const defaultIcon = URI.parse('https://example.com/default.png'); + + function createGroupedAccounts(): Map { + const groupedAccounts = new Map(); + groupedAccounts.set('github', [{ id: 'first-id', label: 'first-account', icon: firstIcon, canSignOut: true }]); + groupedAccounts.set('microsoft', [{ id: 'default-id', label: 'default-account', icon: defaultIcon, canSignOut: true }]); + return groupedAccounts; + } + + test('prefers the current default account avatar over the first account with an icon', () => { + const harness = createHarness(createGroupedAccounts(), createDefaultAccount('microsoft', 'default-account')); + + updateAvatar.call(harness); + + assert.deepStrictEqual( + { src: harness.avatarImg.src, hasAvatarClass: harness.label.classList.contains('has-avatar') }, + { src: defaultIcon.toString(true), hasAvatarClass: true } + ); + }); + + test('falls back to the first account with an icon when there is no default account', () => { + const harness = createHarness(createGroupedAccounts(), null); + + updateAvatar.call(harness); + + assert.deepStrictEqual( + { src: harness.avatarImg.src, hasAvatarClass: harness.label.classList.contains('has-avatar') }, + { src: firstIcon.toString(true), hasAvatarClass: true } + ); + }); + + test('falls back to the first account with an icon when the matching default account has no icon', () => { + const groupedAccounts = createGroupedAccounts(); + groupedAccounts.set('microsoft', [{ id: 'default-id', label: 'default-account', icon: undefined, canSignOut: true }]); + const harness = createHarness(groupedAccounts, createDefaultAccount('microsoft', 'default-account')); + + updateAvatar.call(harness); + + assert.deepStrictEqual( + { src: harness.avatarImg.src, hasAvatarClass: harness.label.classList.contains('has-avatar') }, + { src: firstIcon.toString(true), hasAvatarClass: true } + ); + }); +}); + +suite('AccountsActivityActionViewItem - addOrUpdateAccount', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + function createAddOrUpdateHarness(accounts: (AuthenticationSessionAccount & { canSignOut: boolean })[]): IAddOrUpdateAccountTestHarness { + return { + groupedAccounts: new Map([['github', accounts]]), + sessionFromEmbedder: { value: Promise.resolve(undefined) }, + authenticationService: { getSessions: async () => [] }, + }; + } + + test('updates the icon of an existing account, including clearing a stale one', async () => { + const harness = createAddOrUpdateHarness([{ id: 'account-id', label: 'account', icon: URI.parse('https://example.com/stale.png'), canSignOut: true }]); + + await addOrUpdateAccount.call(harness, 'github', { id: 'account-id', label: 'account', icon: URI.parse('https://example.com/fresh.png') }); + const updated = harness.groupedAccounts.get('github')?.[0].icon; + + await addOrUpdateAccount.call(harness, 'github', { id: 'account-id', label: 'account' }); + const cleared = harness.groupedAccounts.get('github')?.[0].icon; + + assert.deepStrictEqual( + [updated, cleared], + [URI.parse('https://example.com/fresh.png'), undefined] + ); + }); +}); diff --git a/src/vscode-dts/vscode.proposed.authSessionAccountIcon.d.ts b/src/vscode-dts/vscode.proposed.authSessionAccountIcon.d.ts new file mode 100644 index 00000000000..bcee1f9a0f5 --- /dev/null +++ b/src/vscode-dts/vscode.proposed.authSessionAccountIcon.d.ts @@ -0,0 +1,13 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +declare module 'vscode' { + export interface AuthenticationSessionAccountInformation { + /** + * An optional icon for the account. This is typically a URI to a profile image/avatar. + */ + readonly icon?: Uri; + } +}