Make account access cross client ids (#242721)

The point here is that the user already allowed access to the account for one client id, so that should just apply to any client id that is being used since:
* If we don't actually _have_ an auth token, the user will be asked to log in - so they will see a prompt as expected
* If we _do_ have an auth token, then we rely on extension auth access to gate access to the account

Fixes https://github.com/microsoft/vscode/issues/241526
This commit is contained in:
Tyler James Leonhardt
2025-03-05 11:58:49 -08:00
committed by GitHub
parent 95ab795ff0
commit 9e0461087b
3 changed files with 20 additions and 36 deletions
@@ -6,10 +6,11 @@
import { Disposable, Event, EventEmitter, LogOutputChannel, SecretStorage } from 'vscode';
import { AccountInfo } from '@azure/msal-node';
interface IAccountAccess {
export interface IAccountAccess {
initialize(): Promise<void>;
onDidAccountAccessChange: Event<void>;
isAllowedAccess(account: AccountInfo): boolean;
setAllowedAccess(account: AccountInfo, allowed: boolean): void;
setAllowedAccess(account: AccountInfo, allowed: boolean): Promise<void>;
}
export class ScopedAccountAccess implements IAccountAccess {
@@ -23,11 +24,10 @@ export class ScopedAccountAccess implements IAccountAccess {
constructor(
secretStorage: SecretStorage,
cloudName: string,
clientId: string,
logger: LogOutputChannel,
authoritiesToMigrate?: string[],
migrations?: { clientId: string; authority: string }[],
) {
this._accountAccessSecretStorage = new AccountAccessSecretStorage(secretStorage, cloudName, clientId, logger, authoritiesToMigrate);
this._accountAccessSecretStorage = new AccountAccessSecretStorage(secretStorage, cloudName, logger, migrations);
this._accountAccessSecretStorage.onDidChange(() => this.update());
}
@@ -73,9 +73,8 @@ export class AccountAccessSecretStorage {
constructor(
private readonly _secretStorage: SecretStorage,
private readonly _cloudName: string,
private readonly _clientId: string,
private readonly _logger: LogOutputChannel,
private readonly _authoritiesToMigrate?: string[]
private readonly _migrations?: { clientId: string; authority: string }[],
) {
this._disposable = Disposable.from(
this._onDidChangeEmitter,
@@ -91,7 +90,7 @@ export class AccountAccessSecretStorage {
* TODO: Remove this method after a release with the migration
*/
async initialize(): Promise<void> {
if (!this._authoritiesToMigrate) {
if (!this._migrations) {
return;
}
const current = await this.get();
@@ -101,8 +100,8 @@ export class AccountAccessSecretStorage {
}
try {
const allValues = new Set<string>();
for (const authority of this._authoritiesToMigrate) {
const oldKey = `accounts-${this._cloudName}-${this._clientId}-${authority}`;
for (const { clientId, authority } of this._migrations) {
const oldKey = `accounts-${this._cloudName}-${clientId}-${authority}`;
const value = await this._secretStorage.get(oldKey);
if (value) {
const parsed = JSON.parse(value) as string[];
@@ -10,7 +10,7 @@ import { raceCancellationAndTimeoutError } from '../common/async';
import { SecretStorageCachePlugin } from '../common/cachePlugin';
import { MsalLoggerOptions } from '../common/loggerOptions';
import { ICachedPublicClientApplication } from '../common/publicClientCache';
import { ScopedAccountAccess } from '../common/accountAccess';
import { IAccountAccess } from '../common/accountAccess';
export class CachedPublicClientApplication implements ICachedPublicClientApplication {
// Core properties
@@ -27,7 +27,6 @@ export class CachedPublicClientApplication implements ICachedPublicClientApplica
);
// Broker properties
private readonly _accountAccess = new ScopedAccountAccess(this._secretStorage, this._cloudName, this.clientId, this._logger, this._authoritiesToMigrate);
private readonly _isBrokerAvailable: boolean;
//#region Events
@@ -42,10 +41,9 @@ export class CachedPublicClientApplication implements ICachedPublicClientApplica
constructor(
private readonly _clientId: string,
private readonly _cloudName: string,
private readonly _secretStorage: SecretStorage,
private readonly _accountAccess: IAccountAccess,
private readonly _logger: LogOutputChannel,
private readonly _authoritiesToMigrate?: string[],
) {
const loggerOptions = new MsalLoggerOptions(_logger);
const nativeBrokerPlugin = new NativeBrokerPlugin();
@@ -7,6 +7,7 @@ import { AccountInfo } from '@azure/msal-node';
import { SecretStorage, LogOutputChannel, Disposable, EventEmitter, Memento, Event } from 'vscode';
import { ICachedPublicClientApplication, ICachedPublicClientApplicationManager } from '../common/publicClientCache';
import { CachedPublicClientApplication } from './cachedPublicClientApplication';
import { IAccountAccess, ScopedAccountAccess } from '../common/accountAccess';
export interface IPublicClientApplicationInfo {
clientId: string;
@@ -20,6 +21,7 @@ export class CachedPublicClientApplicationManager implements ICachedPublicClient
private _disposable: Disposable;
private _pcasSecretStorage: PublicClientApplicationsSecretStorage;
private _accountAccess: IAccountAccess | undefined;
private readonly _onDidAccountsChangeEmitter = new EventEmitter<{ added: AccountInfo[]; changed: AccountInfo[]; deleted: AccountInfo[] }>();
readonly onDidAccountsChange = this._onDidAccountsChangeEmitter.event;
@@ -44,9 +46,11 @@ export class CachedPublicClientApplicationManager implements ICachedPublicClient
async initialize() {
this._logger.debug('[initialize] Initializing PublicClientApplicationManager');
let clientIds: string[] | undefined;
let migrations: Map<string, string[]> | undefined;
try {
migrations = await this._getMigrationsPerClientId();
await this._pcasSecretStorage.initialize();
//TODO: Remove this in a version
const migrations = await this._pcasSecretStorage.getOldValue();
this._accountAccess = new ScopedAccountAccess(this._secretStorage, this._cloudName, this._logger, migrations);
clientIds = await this._pcasSecretStorage.get();
} catch (e) {
// data is corrupted
@@ -89,23 +93,6 @@ export class CachedPublicClientApplicationManager implements ICachedPublicClient
this._logger.debug('[initialize] PublicClientApplicationManager initialized');
}
private async _getMigrationsPerClientId(): Promise<Map<string, string[]> | undefined> {
await this._pcasSecretStorage.initialize();
const oldValue = await this._pcasSecretStorage.getOldValue();
// returns a map of clientIds to the authorities found in the old value
if (!oldValue) {
return undefined;
}
const result = new Map<string, string[]>();
for (const { clientId, authority } of oldValue) {
if (!result.has(clientId)) {
result.set(clientId, []);
}
result.get(clientId)?.push(authority);
}
return result;
}
dispose() {
this._disposable.dispose();
Disposable.from(...this._pcaDisposables.values()).dispose();
@@ -117,7 +104,7 @@ export class CachedPublicClientApplicationManager implements ICachedPublicClient
this._logger.debug(`[getOrCreate] [${clientId}] PublicClientApplicationManager cache hit`);
} else {
this._logger.debug(`[getOrCreate] [${clientId}] PublicClientApplicationManager cache miss, creating new PCA...`);
pca = await this._doCreatePublicClientApplication(clientId, refreshTokensToMigrate);
pca = await this._doCreatePublicClientApplication(clientId);
await this._storePublicClientApplications();
this._logger.debug(`[getOrCreate] [${clientId}] PCA created.`);
}
@@ -143,8 +130,8 @@ export class CachedPublicClientApplicationManager implements ICachedPublicClient
return pca;
}
private async _doCreatePublicClientApplication(clientId: string, authoritiesToMigrate?: string[]): Promise<ICachedPublicClientApplication> {
const pca = new CachedPublicClientApplication(clientId, this._cloudName, this._secretStorage, this._logger, authoritiesToMigrate);
private async _doCreatePublicClientApplication(clientId: string): Promise<ICachedPublicClientApplication> {
const pca = new CachedPublicClientApplication(clientId, this._secretStorage, this._accountAccess!, this._logger);
this._pcas.set(clientId, pca);
const disposable = Disposable.from(
pca,