Skip to content

Commit 3c40060

Browse files
joaomorenojoaomoreno
authored
Revert "Have AccessService also take into consideration the ProductService and adopt it in ManageTrustedExtensionsForAccountActionImpl" (microsoft#252139)
Revert "Have AccessService also take into consideration the ProductService an…" This reverts commit e760e15.
1 parent d6d5034 commit 3c40060

File tree

6 files changed

+112
-658
lines changed

6 files changed

+112
-658
lines changed

‎src/vs/workbench/contrib/authentication/browser/actions/manageTrustedExtensionsForAccountAction.ts

Lines changed: 106 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@ import { Action2 } from '../../../../../platform/actions/common/actions.js';
1212
import { ICommandService } from '../../../../../platform/commands/common/commands.js';
1313
import { IDialogService } from '../../../../../platform/dialogs/common/dialogs.js';
1414
import { IInstantiationService, ServicesAccessor } from '../../../../../platform/instantiation/common/instantiation.js';
15+
import { IProductService } from '../../../../../platform/product/common/productService.js';
1516
import { IQuickInputService, IQuickPickItem, IQuickPickSeparator } from '../../../../../platform/quickinput/common/quickInput.js';
17+
import { IAuthenticationAccessService } from '../../../../services/authentication/browser/authenticationAccessService.js';
18+
import { IAuthenticationUsageService } from '../../../../services/authentication/browser/authenticationUsageService.js';
1619
import { AllowedExtension, IAuthenticationService } from '../../../../services/authentication/common/authentication.js';
17-
import { IAuthenticationQueryService, IAccountQuery } from '../../../../services/authentication/common/authenticationQuery.js';
1820
import { IExtensionService } from '../../../../services/extensions/common/extensions.js';
1921

2022
export class ManageTrustedExtensionsForAccountAction extends Action2 {
@@ -40,110 +42,133 @@ interface TrustedExtensionsQuickPickItem extends IQuickPickItem {
4042

4143
class ManageTrustedExtensionsForAccountActionImpl {
4244
constructor(
45+
@IProductService private readonly _productService: IProductService,
4346
@IExtensionService private readonly _extensionService: IExtensionService,
4447
@IDialogService private readonly _dialogService: IDialogService,
4548
@IQuickInputService private readonly _quickInputService: IQuickInputService,
4649
@IAuthenticationService private readonly _authenticationService: IAuthenticationService,
47-
@IAuthenticationQueryService private readonly _authenticationQueryService: IAuthenticationQueryService,
50+
@IAuthenticationUsageService private readonly _authenticationUsageService: IAuthenticationUsageService,
51+
@IAuthenticationAccessService private readonly _authenticationAccessService: IAuthenticationAccessService,
4852
@ICommandService private readonly _commandService: ICommandService
4953
) { }
5054

5155
async run(options?: { providerId: string; accountLabel: string }) {
52-
const accountQuery = await this._resolveAccountQuery(options?.providerId, options?.accountLabel);
53-
if (!accountQuery) {
56+
const { providerId, accountLabel } = await this._resolveProviderAndAccountLabel(options?.providerId, options?.accountLabel);
57+
if (!providerId || !accountLabel) {
5458
return;
5559
}
5660

57-
const items = await this._getItems(accountQuery);
61+
const items = await this._getItems(providerId, accountLabel);
5862
if (!items.length) {
5963
return;
6064
}
61-
const picker = this._createQuickPick(accountQuery);
65+
const disposables = new DisposableStore();
66+
const picker = this._createQuickPick(disposables, providerId, accountLabel);
6267
picker.items = items;
6368
picker.selectedItems = items.filter((i): i is TrustedExtensionsQuickPickItem => i.type !== 'separator' && !!i.picked);
6469
picker.show();
6570
}
6671

67-
//#region Account Query Resolution
68-
69-
private async _resolveAccountQuery(providerId: string | undefined, accountLabel: string | undefined): Promise<IAccountQuery | undefined> {
70-
if (providerId && accountLabel) {
71-
return this._authenticationQueryService.provider(providerId).account(accountLabel);
72-
}
73-
74-
const accounts = await this._getAllAvailableAccounts();
75-
const pick = await this._quickInputService.pick(accounts, {
76-
placeHolder: localize('pickAccount', "Pick an account to manage trusted extensions for"),
77-
matchOnDescription: true,
78-
});
79-
80-
return pick ? this._authenticationQueryService.provider(pick.providerId).account(pick.label) : undefined;
81-
}
72+
private async _resolveProviderAndAccountLabel(providerId: string | undefined, accountLabel: string | undefined) {
73+
if (!providerId || !accountLabel) {
74+
const accounts = new Array<{ providerId: string; providerLabel: string; accountLabel: string }>();
75+
for (const id of this._authenticationService.getProviderIds()) {
76+
const providerLabel = this._authenticationService.getProvider(id).label;
77+
const sessions = await this._authenticationService.getSessions(id);
78+
const uniqueAccountLabels = new Set<string>();
79+
for (const session of sessions) {
80+
if (!uniqueAccountLabels.has(session.account.label)) {
81+
uniqueAccountLabels.add(session.account.label);
82+
accounts.push({ providerId: id, providerLabel, accountLabel: session.account.label });
83+
}
84+
}
85+
}
8286

83-
private async _getAllAvailableAccounts() {
84-
const accounts = [];
85-
for (const providerId of this._authenticationService.getProviderIds()) {
86-
const provider = this._authenticationService.getProvider(providerId);
87-
const sessions = await this._authenticationService.getSessions(providerId);
88-
const uniqueLabels = new Set<string>();
89-
90-
for (const session of sessions) {
91-
if (!uniqueLabels.has(session.account.label)) {
92-
uniqueLabels.add(session.account.label);
93-
accounts.push({
94-
providerId,
95-
label: session.account.label,
96-
description: provider.label
97-
});
87+
const pick = await this._quickInputService.pick(
88+
accounts.map(account => ({
89+
providerId: account.providerId,
90+
label: account.accountLabel,
91+
description: account.providerLabel
92+
})),
93+
{
94+
placeHolder: localize('pickAccount', "Pick an account to manage trusted extensions for"),
95+
matchOnDescription: true,
9896
}
97+
);
98+
99+
if (pick) {
100+
providerId = pick.providerId;
101+
accountLabel = pick.label;
102+
} else {
103+
return { providerId: undefined, accountLabel: undefined };
99104
}
100105
}
101-
return accounts;
106+
return { providerId, accountLabel };
102107
}
103108

104-
//#endregion
105-
106-
//#region Item Retrieval and Quick Pick Creation
107-
108-
private async _getItems(accountQuery: IAccountQuery) {
109-
const allowedExtensions = accountQuery.extensions().getAllowedExtensions();
110-
const extensionIdToDisplayName = new Map<string, string>();
111-
112-
// Get display names for all allowed extensions
109+
private async _getItems(providerId: string, accountLabel: string) {
110+
let allowedExtensions = this._authenticationAccessService.readAllowedExtensions(providerId, accountLabel);
111+
// only include extensions that are installed
113112
const resolvedExtensions = await Promise.all(allowedExtensions.map(ext => this._extensionService.getExtension(ext.id)));
114-
resolvedExtensions.forEach((resolved, i) => {
115-
if (resolved) {
116-
extensionIdToDisplayName.set(allowedExtensions[i].id, resolved.displayName || resolved.name);
113+
allowedExtensions = resolvedExtensions
114+
.map((ext, i) => ext ? allowedExtensions[i] : undefined)
115+
.filter(ext => !!ext);
116+
const trustedExtensionAuthAccess = this._productService.trustedExtensionAuthAccess;
117+
const trustedExtensionIds =
118+
// Case 1: trustedExtensionAuthAccess is an array
119+
Array.isArray(trustedExtensionAuthAccess)
120+
? trustedExtensionAuthAccess
121+
// Case 2: trustedExtensionAuthAccess is an object
122+
: typeof trustedExtensionAuthAccess === 'object'
123+
? trustedExtensionAuthAccess[providerId] ?? []
124+
: [];
125+
for (const extensionId of trustedExtensionIds) {
126+
const allowedExtension = allowedExtensions.find(ext => ext.id === extensionId);
127+
if (!allowedExtension) {
128+
// Add the extension to the allowedExtensions list
129+
const extension = await this._extensionService.getExtension(extensionId);
130+
if (extension) {
131+
allowedExtensions.push({
132+
id: extensionId,
133+
name: extension.displayName || extension.name,
134+
allowed: true,
135+
trusted: true
136+
});
137+
}
138+
} else {
139+
// Update the extension to be allowed
140+
allowedExtension.allowed = true;
141+
allowedExtension.trusted = true;
117142
}
118-
});
119-
120-
// Filter out extensions that are not currently installed and enrich with display names
121-
const filteredExtensions = allowedExtensions
122-
.filter(ext => extensionIdToDisplayName.has(ext.id))
123-
.map(ext => {
124-
const usage = accountQuery.extension(ext.id).getUsage();
125-
return {
126-
...ext,
127-
// Use the extension display name from the extension service
128-
name: extensionIdToDisplayName.get(ext.id)!,
129-
lastUsed: usage.length > 0 ? Math.max(...usage.map(u => u.lastUsed)) : ext.lastUsed
130-
};
131-
});
143+
}
132144

133-
if (!filteredExtensions.length) {
145+
if (!allowedExtensions.length) {
134146
this._dialogService.info(localize('noTrustedExtensions', "This account has not been used by any extensions."));
135147
return [];
136148
}
137149

138-
const trustedExtensions = filteredExtensions.filter(e => e.trusted);
139-
const otherExtensions = filteredExtensions.filter(e => !e.trusted);
150+
const usages = this._authenticationUsageService.readAccountUsages(providerId, accountLabel);
151+
const trustedExtensions = [];
152+
const otherExtensions = [];
153+
for (const extension of allowedExtensions) {
154+
const usage = usages.find(usage => extension.id === usage.extensionId);
155+
extension.lastUsed = usage?.lastUsed;
156+
if (extension.trusted) {
157+
trustedExtensions.push(extension);
158+
} else {
159+
otherExtensions.push(extension);
160+
}
161+
}
162+
140163
const sortByLastUsed = (a: AllowedExtension, b: AllowedExtension) => (b.lastUsed || 0) - (a.lastUsed || 0);
141164

142-
return [
165+
const items = [
143166
...otherExtensions.sort(sortByLastUsed).map(this._toQuickPickItem),
144167
{ type: 'separator', label: localize('trustedExtensions', "Trusted by Microsoft") } satisfies IQuickPickSeparator,
145168
...trustedExtensions.sort(sortByLastUsed).map(this._toQuickPickItem)
146169
];
170+
171+
return items;
147172
}
148173

149174
private _toQuickPickItem(extension: AllowedExtension): TrustedExtensionsQuickPickItem {
@@ -171,39 +196,38 @@ class ManageTrustedExtensionsForAccountActionImpl {
171196
};
172197
}
173198

174-
private _createQuickPick(accountQuery: IAccountQuery) {
175-
const disposableStore = new DisposableStore();
199+
private _createQuickPick(disposableStore: DisposableStore, providerId: string, accountLabel: string) {
176200
const quickPick = disposableStore.add(this._quickInputService.createQuickPick<TrustedExtensionsQuickPickItem>({ useSeparators: true }));
177-
178-
// Configure quick pick
179201
quickPick.canSelectMany = true;
180202
quickPick.customButton = true;
181203
quickPick.customLabel = localize('manageTrustedExtensions.cancel', 'Cancel');
204+
182205
quickPick.title = localize('manageTrustedExtensions', "Manage Trusted Extensions");
183206
quickPick.placeholder = localize('manageExtensions', "Choose which extensions can access this account");
184207

185-
// Set up event handlers
186208
disposableStore.add(quickPick.onDidAccept(() => {
187209
const updatedAllowedList = quickPick.items
188210
.filter((item): item is TrustedExtensionsQuickPickItem => item.type !== 'separator')
189211
.map(i => i.extension);
190212

191213
const allowedExtensionsSet = new Set(quickPick.selectedItems.map(i => i.extension));
192-
for (const extension of updatedAllowedList) {
193-
const allowed = allowedExtensionsSet.has(extension);
194-
accountQuery.extension(extension.id).setAccessAllowed(allowed, extension.name);
195-
}
214+
updatedAllowedList.forEach(extension => {
215+
extension.allowed = allowedExtensionsSet.has(extension);
216+
});
217+
this._authenticationAccessService.updateAllowedExtensions(providerId, accountLabel, updatedAllowedList);
196218
quickPick.hide();
197219
}));
198220

199-
disposableStore.add(quickPick.onDidHide(() => disposableStore.dispose()));
200-
disposableStore.add(quickPick.onDidCustom(() => quickPick.hide()));
221+
disposableStore.add(quickPick.onDidHide(() => {
222+
disposableStore.dispose();
223+
}));
224+
225+
disposableStore.add(quickPick.onDidCustom(() => {
226+
quickPick.hide();
227+
}));
201228
disposableStore.add(quickPick.onDidTriggerItemButton(e =>
202-
this._commandService.executeCommand('_manageAccountPreferencesForExtension', e.item.extension.id, accountQuery.providerId)
229+
this._commandService.executeCommand('_manageAccountPreferencesForExtension', e.item.extension.id, providerId)
203230
));
204-
205231
return quickPick;
206232
}
207-
208-
//#endregion
209233
}

‎src/vs/workbench/services/authentication/browser/authenticationAccessService.ts

Lines changed: 1 addition & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -75,34 +75,6 @@ export class AuthenticationAccessService extends Disposable implements IAuthenti
7575
}
7676
} catch (err) { }
7777

78-
// Add trusted extensions from product.json if they're not already in the list
79-
const trustedExtensionAuthAccess = this._productService.trustedExtensionAuthAccess;
80-
const trustedExtensionIds =
81-
// Case 1: trustedExtensionAuthAccess is an array
82-
Array.isArray(trustedExtensionAuthAccess)
83-
? trustedExtensionAuthAccess
84-
// Case 2: trustedExtensionAuthAccess is an object
85-
: typeof trustedExtensionAuthAccess === 'object'
86-
? trustedExtensionAuthAccess[providerId] ?? []
87-
: [];
88-
89-
for (const extensionId of trustedExtensionIds) {
90-
const existingExtension = trustedExtensions.find(extension => extension.id === extensionId);
91-
if (!existingExtension) {
92-
// Add new trusted extension (name will be set by caller if they have extension info)
93-
trustedExtensions.push({
94-
id: extensionId,
95-
name: extensionId, // Default to ID, caller can update with proper name
96-
allowed: true,
97-
trusted: true
98-
});
99-
} else {
100-
// Update existing extension to be trusted
101-
existingExtension.allowed = true;
102-
existingExtension.trusted = true;
103-
}
104-
}
105-
10678
return trustedExtensions;
10779
}
10880

@@ -114,16 +86,9 @@ export class AuthenticationAccessService extends Disposable implements IAuthenti
11486
allowList.push(extension);
11587
} else {
11688
allowList[index].allowed = extension.allowed;
117-
// Update name if provided and not already set to a proper name
118-
if (extension.name && extension.name !== extension.id && allowList[index].name !== extension.name) {
119-
allowList[index].name = extension.name;
120-
}
12189
}
12290
}
123-
124-
// Filter out trusted extensions before storing - they should only come from product.json, not user storage
125-
const userManagedExtensions = allowList.filter(extension => !extension.trusted);
126-
this._storageService.store(`${providerId}-${accountName}`, JSON.stringify(userManagedExtensions), StorageScope.APPLICATION, StorageTarget.USER);
91+
this._storageService.store(`${providerId}-${accountName}`, JSON.stringify(allowList), StorageScope.APPLICATION, StorageTarget.USER);
12792
this._onDidChangeExtensionSessionAccess.fire({ providerId, accountName });
12893
}
12994

‎src/vs/workbench/services/authentication/browser/authenticationQueryService.ts

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,6 @@ class AccountExtensionQuery extends BaseQuery implements IAccountExtensionQuery
122122
const preferredAccount = this.queryService.authenticationExtensionsService.getAccountPreference(this.extensionId, this.providerId);
123123
return preferredAccount === this.accountName;
124124
}
125-
126-
isTrusted(): boolean {
127-
const allowedExtensions = this.queryService.authenticationAccessService.readAllowedExtensions(this.providerId, this.accountName);
128-
const extension = allowedExtensions.find(ext => ext.id === this.extensionId);
129-
return extension?.trusted === true;
130-
}
131125
}
132126

133127
/**
@@ -232,29 +226,9 @@ class AccountExtensionsQuery extends BaseQuery implements IAccountExtensionsQuer
232226
super(providerId, queryService);
233227
}
234228

235-
getAllowedExtensions(): { id: string; name: string; allowed?: boolean; lastUsed?: number; trusted?: boolean }[] {
229+
getAllowedExtensionIds(): string[] {
236230
const allowedExtensions = this.queryService.authenticationAccessService.readAllowedExtensions(this.providerId, this.accountName);
237-
const usages = this.queryService.authenticationUsageService.readAccountUsages(this.providerId, this.accountName);
238-
239-
return allowedExtensions
240-
.filter(ext => ext.allowed !== false)
241-
.map(ext => {
242-
// Find the most recent usage for this extension
243-
const extensionUsages = usages.filter(usage => usage.extensionId === ext.id);
244-
const lastUsed = extensionUsages.length > 0 ? Math.max(...extensionUsages.map(u => u.lastUsed)) : undefined;
245-
246-
// Check if trusted through the extension query
247-
const extensionQuery = new AccountExtensionQuery(this.providerId, this.accountName, ext.id, this.queryService);
248-
const trusted = extensionQuery.isTrusted();
249-
250-
return {
251-
id: ext.id,
252-
name: ext.name,
253-
allowed: ext.allowed,
254-
lastUsed,
255-
trusted
256-
};
257-
});
231+
return allowedExtensions.filter(ext => ext.allowed !== false).map(ext => ext.id);
258232
}
259233

260234
allowAccess(extensionIds: string[]): void {

‎src/vs/workbench/services/authentication/common/authenticationQuery.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -125,12 +125,6 @@ export interface IAccountExtensionQuery extends IBaseQuery {
125125
* Check if this account is the preferred account for this extension
126126
*/
127127
isPreferred(): boolean;
128-
129-
/**
130-
* Check if this extension is trusted (defined in product.json)
131-
* @returns True if the extension is trusted, false otherwise
132-
*/
133-
isTrusted(): boolean;
134128
}
135129

136130
/**
@@ -200,10 +194,10 @@ export interface IAccountExtensionsQuery extends IBaseQuery {
200194
readonly accountName: string;
201195

202196
/**
203-
* Get all extensions that have access to this account with their trusted state
204-
* @returns Array of objects containing extension data including trusted state
197+
* Get all extension IDs that have access to this account
198+
* @returns Array of extension IDs
205199
*/
206-
getAllowedExtensions(): { id: string; name: string; allowed?: boolean; lastUsed?: number; trusted?: boolean }[];
200+
getAllowedExtensionIds(): string[];
207201

208202
/**
209203
* Grant access to this account for all specified extensions

0 commit comments

Comments
 (0)