Skip to content

Commit 9b5c56c

Browse files
Refactor authenticationService.getSessions to match Ext Host API shape (#249913)
Much cleaner and extendable
1 parent 86e8abb commit 9b5c56c

File tree

6 files changed

+24
-13
lines changed

6 files changed

+24
-13
lines changed

src/vs/workbench/api/browser/mainThreadAuthentication.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu
260260

261261
private async doGetSession(providerId: string, scopes: string[], extensionId: string, extensionName: string, options: AuthenticationGetSessionOptions): Promise<AuthenticationSession | undefined> {
262262
const issuer = URI.revive(options.issuer);
263-
const sessions = await this.authenticationService.getSessions(providerId, scopes, options.account, true, issuer);
263+
const sessions = await this.authenticationService.getSessions(providerId, scopes, { account: options.account, issuer }, true);
264264
const provider = this.authenticationService.getProvider(providerId);
265265

266266
// Error cases

src/vs/workbench/api/browser/mainThreadMcp.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ export class MainThreadMcp extends Disposable implements MainThreadMcpShape {
155155
}
156156
providerId = provider.id;
157157
}
158-
const sessions = await this._authenticationService.getSessions(providerId, scopesSupported, undefined, true, issuer);
158+
const sessions = await this._authenticationService.getSessions(providerId, scopesSupported, { issuer }, true);
159159
const accountNamePreference = this.authenticationMcpServersService.getAccountPreference(server.id, providerId);
160160
let matchingAccountPreferenceSession: AuthenticationSession | undefined;
161161
if (accountNamePreference) {

src/vs/workbench/contrib/chat/common/chatEntitlementService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ export class ChatEntitlementRequests extends Disposable {
526526
}
527527

528528
try {
529-
return await this.authenticationService.getSessions(providerId, undefined, preferredAccount);
529+
return await this.authenticationService.getSessions(providerId, undefined, { account: preferredAccount });
530530
} catch (error) {
531531
// ignore - errors can throw if a provider is not registered
532532
}

src/vs/workbench/services/authentication/browser/authenticationService.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { InstantiationType, registerSingleton } from '../../../../platform/insta
1212
import { IProductService } from '../../../../platform/product/common/productService.js';
1313
import { ISecretStorageService } from '../../../../platform/secrets/common/secrets.js';
1414
import { IAuthenticationAccessService } from './authenticationAccessService.js';
15-
import { AuthenticationProviderInformation, AuthenticationSession, AuthenticationSessionAccount, AuthenticationSessionsChangeEvent, IAuthenticationCreateSessionOptions, IAuthenticationProvider, IAuthenticationProviderHostDelegate, IAuthenticationService } from '../common/authentication.js';
15+
import { AuthenticationProviderInformation, AuthenticationSession, AuthenticationSessionAccount, AuthenticationSessionsChangeEvent, IAuthenticationCreateSessionOptions, IAuthenticationGetSessionsOptions, IAuthenticationProvider, IAuthenticationProviderHostDelegate, IAuthenticationService } from '../common/authentication.js';
1616
import { IBrowserWorkbenchEnvironmentService } from '../../environment/browser/environmentService.js';
1717
import { ActivationKind, IExtensionService } from '../../extensions/common/extensions.js';
1818
import { ILogService } from '../../../../platform/log/common/log.js';
@@ -255,18 +255,18 @@ export class AuthenticationService extends Disposable implements IAuthentication
255255
return accounts;
256256
}
257257

258-
async getSessions(id: string, scopes?: string[], account?: AuthenticationSessionAccount, activateImmediate: boolean = false, issuer?: URI): Promise<ReadonlyArray<AuthenticationSession>> {
258+
async getSessions(id: string, scopes?: string[], options?: IAuthenticationGetSessionsOptions, activateImmediate: boolean = false): Promise<ReadonlyArray<AuthenticationSession>> {
259259
const authProvider = this._authenticationProviders.get(id) || await this.tryActivateProvider(id, activateImmediate);
260260
if (authProvider) {
261261
// Check if the issuer is in the list of supported issuers
262-
if (issuer) {
263-
const issuerStr = issuer.toString(true);
262+
if (options?.issuer) {
263+
const issuerStr = options.issuer.toString(true);
264264
// TODO: something is off here...
265265
if (!authProvider.issuers?.some(i => i.toString(true) === issuerStr || match(i.toString(true), issuerStr))) {
266266
throw new Error(`The issuer '${issuerStr}' is not supported by the authentication provider '${id}'.`);
267267
}
268268
}
269-
return await authProvider.getSessions(scopes, { account, issuer });
269+
return await authProvider.getSessions(scopes, { account: options?.account, issuer: options?.issuer });
270270
} else {
271271
throw new Error(`No authentication provider '${id}' is currently registered.`);
272272
}

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,19 @@ export interface IAuthenticationCreateSessionOptions {
5252
issuer?: URI;
5353
}
5454

55+
export interface IAuthenticationGetSessionsOptions {
56+
/**
57+
* The account that is being asked about. If this is passed in, the provider should
58+
* attempt to return the sessions that are only related to this account.
59+
*/
60+
account?: AuthenticationSessionAccount;
61+
/**
62+
* The issuer URI to use for this request. If passed in, first we validate that
63+
* the provider can use this issuer, then it is passed down to the auth provider.
64+
*/
65+
issuer?: URI;
66+
}
67+
5568
export interface AllowedExtension {
5669
id: string;
5770
name: string;
@@ -153,14 +166,12 @@ export interface IAuthenticationService {
153166

154167
/**
155168
* Gets all sessions that satisfy the given scopes from the provider with the given id
156-
* TODO:@TylerLeonhardt Refactor this to use an options bag for account and issuer
157169
* @param id The id of the provider to ask for a session
158170
* @param scopes The scopes for the session
159-
* @param account The account for the session
171+
* @param options Additional options for getting sessions
160172
* @param activateImmediate If true, the provider should activate immediately if it is not already
161-
* @param issuer The issuer for the session
162173
*/
163-
getSessions(id: string, scopes?: string[], account?: AuthenticationSessionAccount, activateImmediate?: boolean, issuer?: URI): Promise<ReadonlyArray<AuthenticationSession>>;
174+
getSessions(id: string, scopes?: string[], options?: IAuthenticationGetSessionsOptions, activateImmediate?: boolean): Promise<ReadonlyArray<AuthenticationSession>>;
164175

165176
/**
166177
* Creates an AuthenticationSession with the given provider and scopes

src/vs/workbench/services/authentication/test/browser/authenticationService.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ suite('AuthenticationService', () => {
249249
},
250250
});
251251
authenticationService.registerAuthenticationProvider(provider.id, provider);
252-
assert.rejects(() => authenticationService.getSessions(provider.id, [], undefined, undefined, URI.parse('https://example.com')));
252+
assert.rejects(() => authenticationService.getSessions(provider.id, [], { issuer: URI.parse('https://example.com') }));
253253
assert.ok(!isCalled);
254254
});
255255

0 commit comments

Comments
 (0)