Fix a few problems with new Azure auth (#9760)

* Fix a few problems

* Fix bug

* One resource per line

* Dispose before cleaning

* Dispose the event handler

* Dispose webserver
This commit is contained in:
Amir Omidi
2020-03-27 13:18:27 -07:00
committed by GitHub
parent 52f7ab121d
commit a9240f38f7
7 changed files with 64 additions and 32 deletions

View File

@@ -91,7 +91,7 @@ export interface TokenClaims { // https://docs.microsoft.com/en-us/azure/active-
export type TokenRefreshResponse = { accessToken: AccessToken, refreshToken: RefreshToken, tokenClaims: TokenClaims };
export abstract class AzureAuth {
export abstract class AzureAuth implements vscode.Disposable {
protected readonly memdb = new MemoryDatabase();
protected readonly WorkSchoolAccountType: string = 'work_school';
@@ -110,6 +110,7 @@ export abstract class AzureAuth {
protected readonly metadata: AzureAccountProviderMetadata,
protected readonly tokenCache: SimpleTokenCache,
protected readonly context: vscode.ExtensionContext,
protected readonly uriEventEmitter: vscode.EventEmitter<vscode.Uri>,
protected readonly authType: AzureAuthType,
public readonly userFriendlyName: string
) {
@@ -119,8 +120,11 @@ export abstract class AzureAuth {
this.clientId = this.metadata.settings.clientId;
this.resources = [
this.metadata.settings.armResource, this.metadata.settings.sqlResource,
this.metadata.settings.graphResource, this.metadata.settings.ossRdbmsResource
this.metadata.settings.armResource,
this.metadata.settings.sqlResource,
this.metadata.settings.graphResource,
this.metadata.settings.ossRdbmsResource,
this.metadata.settings.azureKeyVaultResource
];
this.scopes = [...this.metadata.settings.scopes];
@@ -131,6 +135,7 @@ export abstract class AzureAuth {
public abstract async autoOAuthCancelled(): Promise<void>;
public dispose() { }
public async refreshAccess(account: azdata.Account): Promise<azdata.Account> {
const response = await this.getCachedToken(account.key);

View File

@@ -30,12 +30,6 @@ import { SimpleWebServer } from '../utils/simpleWebServer';
import { SimpleTokenCache } from '../simpleTokenCache';
const localize = nls.loadMessageBundle();
class UriEventHandler extends vscode.EventEmitter<vscode.Uri> implements vscode.UriHandler {
public handleUri(uri: vscode.Uri) {
this.fire(uri);
}
}
function parseQuery(uri: vscode.Uri) {
return uri.query.split('&').reduce((prev: any, current) => {
const queryString = current.split('=');
@@ -52,14 +46,14 @@ interface AuthCodeResponse {
export class AzureAuthCodeGrant extends AzureAuth {
private static readonly USER_FRIENDLY_NAME: string = localize('azure.azureAuthCodeGrantName', "Azure Auth Code Grant");
private server: SimpleWebServer;
private readonly _uriHandler: UriEventHandler;
constructor(metadata: AzureAccountProviderMetadata,
constructor(
metadata: AzureAccountProviderMetadata,
tokenCache: SimpleTokenCache,
context: vscode.ExtensionContext) {
super(metadata, tokenCache, context, AzureAuthType.AuthCodeGrant, AzureAuthCodeGrant.USER_FRIENDLY_NAME);
this._uriHandler = new UriEventHandler();
vscode.window.registerUriHandler(this._uriHandler);
context: vscode.ExtensionContext,
uriEventEmitter: vscode.EventEmitter<vscode.Uri>,
) {
super(metadata, tokenCache, context, uriEventEmitter, AzureAuthType.AuthCodeGrant, AzureAuthCodeGrant.USER_FRIENDLY_NAME);
}
public async autoOAuthCancelled(): Promise<void> {
@@ -146,7 +140,7 @@ export class AzureAuthCodeGrant extends AzureAuth {
public async handleCodeResponse(state: string): Promise<string> {
let uriEventListener: vscode.Disposable;
return new Promise((resolve: (value: any) => void, reject) => {
uriEventListener = this._uriHandler.event(async (uri: vscode.Uri) => {
uriEventListener = this.uriEventEmitter.event(async (uri: vscode.Uri) => {
try {
const query = parseQuery(uri);
const code = query.code;
@@ -316,4 +310,8 @@ export class AzureAuthCodeGrant extends AzureAuth {
return this.getToken(postData);
}
public dispose() {
this.server?.shutdown().catch(console.error);
}
}

View File

@@ -46,10 +46,13 @@ export class AzureDeviceCode extends AzureAuth {
private static readonly USER_FRIENDLY_NAME: string = localize('azure.azureDeviceCodeAuth', "Azure Device Code");
private readonly pageTitle: string;
constructor(metadata: AzureAccountProviderMetadata,
constructor(
metadata: AzureAccountProviderMetadata,
tokenCache: SimpleTokenCache,
context: vscode.ExtensionContext) {
super(metadata, tokenCache, context, AzureAuthType.AuthCodeGrant, AzureDeviceCode.USER_FRIENDLY_NAME);
context: vscode.ExtensionContext,
uriEventEmitter: vscode.EventEmitter<vscode.Uri>,
) {
super(metadata, tokenCache, context, uriEventEmitter, AzureAuthType.AuthCodeGrant, AzureDeviceCode.USER_FRIENDLY_NAME);
this.pageTitle = localize('addAccount', "Add {0} account", this.metadata.displayName);
}

View File

@@ -20,7 +20,7 @@ import { AzureDeviceCode } from './auths/azureDeviceCode';
const localize = nls.loadMessageBundle();
export class AzureAccountProvider implements azdata.AccountProvider {
export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disposable {
private static readonly CONFIGURATION_SECTION = 'accounts.azure.auth';
private readonly authMappings = new Map<AzureAuthType, AzureAuth>();
private initComplete: Deferred<void>;
@@ -30,23 +30,29 @@ export class AzureAccountProvider implements azdata.AccountProvider {
metadata: AzureAccountProviderMetadata,
tokenCache: SimpleTokenCache,
context: vscode.ExtensionContext,
uriEventHandler: vscode.EventEmitter<vscode.Uri>,
private readonly forceDeviceCode: boolean = false
) {
vscode.workspace.onDidChangeConfiguration((changeEvent) => {
const impact = changeEvent.affectsConfiguration(AzureAccountProvider.CONFIGURATION_SECTION);
if (impact === true) {
this.handleAuthMapping(metadata, tokenCache, context);
this.handleAuthMapping(metadata, tokenCache, context, uriEventHandler);
}
});
this.handleAuthMapping(metadata, tokenCache, context);
this.handleAuthMapping(metadata, tokenCache, context, uriEventHandler);
}
dispose() {
this.authMappings.forEach(x => x.dispose());
}
clearTokenCache(): Thenable<void> {
return this.getAuthMethod().deleteAllCache();
}
private handleAuthMapping(metadata: AzureAccountProviderMetadata, tokenCache: SimpleTokenCache, context: vscode.ExtensionContext) {
private handleAuthMapping(metadata: AzureAccountProviderMetadata, tokenCache: SimpleTokenCache, context: vscode.ExtensionContext, uriEventHandler: vscode.EventEmitter<vscode.Uri>) {
this.authMappings.forEach(m => m.dispose());
this.authMappings.clear();
const configuration = vscode.workspace.getConfiguration(AzureAccountProvider.CONFIGURATION_SECTION);
@@ -54,10 +60,10 @@ export class AzureAccountProvider implements azdata.AccountProvider {
const deviceCodeMethod: boolean = configuration.get('deviceCode');
if (codeGrantMethod === true && !this.forceDeviceCode) {
this.authMappings.set(AzureAuthType.AuthCodeGrant, new AzureAuthCodeGrant(metadata, tokenCache, context));
this.authMappings.set(AzureAuthType.AuthCodeGrant, new AzureAuthCodeGrant(metadata, tokenCache, context, uriEventHandler));
}
if (deviceCodeMethod === true || this.forceDeviceCode) {
this.authMappings.set(AzureAuthType.DeviceCode, new AzureDeviceCode(metadata, tokenCache, context));
this.authMappings.set(AzureAuthType.DeviceCode, new AzureDeviceCode(metadata, tokenCache, context, uriEventHandler));
}
}

View File

@@ -15,6 +15,12 @@ import * as loc from '../localizedConstants';
let localize = nls.loadMessageBundle();
class UriEventHandler extends vscode.EventEmitter<vscode.Uri> implements vscode.UriHandler {
public handleUri(uri: vscode.Uri) {
this.fire(uri);
}
}
export class AzureAccountProviderService implements vscode.Disposable {
// CONSTANTS ///////////////////////////////////////////////////////////////
private static CommandClearTokenCache = 'accounts.clearTokenCache';
@@ -22,12 +28,14 @@ export class AzureAccountProviderService implements vscode.Disposable {
private static CredentialNamespace = 'azureAccountProviderCredentials';
// MEMBER VARIABLES ////////////////////////////////////////////////////////
private _disposables: vscode.Disposable[] = [];
private _accountDisposals: { [accountProviderId: string]: vscode.Disposable };
private _accountProviders: { [accountProviderId: string]: azdata.AccountProvider };
private _credentialProvider: azdata.CredentialProvider;
private _configChangePromiseChain: Thenable<void>;
private _currentConfig: vscode.WorkspaceConfiguration;
private _event: events.EventEmitter;
private readonly _uriEventHandler: UriEventHandler;
constructor(private _context: vscode.ExtensionContext, private _userStoragePath: string) {
this._accountDisposals = {};
@@ -35,6 +43,9 @@ export class AzureAccountProviderService implements vscode.Disposable {
this._configChangePromiseChain = Promise.resolve();
this._currentConfig = null;
this._event = new events.EventEmitter();
this._uriEventHandler = new UriEventHandler();
this._disposables.push(vscode.window.registerUriHandler(this._uriEventHandler));
}
// PUBLIC METHODS //////////////////////////////////////////////////////
@@ -65,7 +76,14 @@ export class AzureAccountProviderService implements vscode.Disposable {
});
}
public dispose() { }
public dispose() {
while (this._disposables.length) {
const item = this._disposables.pop();
if (item) {
item.dispose();
}
}
}
// PRIVATE HELPERS /////////////////////////////////////////////////////
private onClearTokenCache(): Thenable<void> {
@@ -133,7 +151,7 @@ export class AzureAccountProviderService implements vscode.Disposable {
await simpleTokenCache.init();
const isSaw: boolean = vscode.env.appName.toLowerCase().indexOf('saw') > 0;
let accountProvider = new AzureAccountProvider(provider.metadata as AzureAccountProviderMetadata, simpleTokenCache, this._context, isSaw);
let accountProvider = new AzureAccountProvider(provider.metadata as AzureAccountProviderMetadata, simpleTokenCache, this._context, this._uriEventHandler, isSaw);
this._accountProviders[provider.metadata.id] = accountProvider;
this._accountDisposals[provider.metadata.id] = azdata.accounts.registerAccountProvider(provider.metadata, accountProvider);

View File

@@ -39,8 +39,9 @@ const publicAzureSettings: ProviderSettings = {
azureResourceId: AzureResource.OssRdbms
},
azureKeyVaultResource: {
id: 'https://vault.azure.net',
endpoint: 'https://vault.azure.net'
id: 'vault',
endpoint: 'https://vault.azure.net',
azureResourceId: AzureResource.AzureKeyVault
},
redirectUri: 'https://vscode-redirect.azurewebsites.net/',
scopes: [
@@ -82,8 +83,9 @@ const usGovAzureSettings: ProviderSettings = {
azureResourceId: AzureResource.OssRdbms
},
azureKeyVaultResource: {
id: 'https://vault.usgovcloudapi.net',
endpoint: 'https://vault.usgovcloudapi.net'
id: 'vault',
endpoint: 'https://vault.usgovcloudapi.net',
azureResourceId: AzureResource.AzureKeyVault
},
redirectUri: 'https://vscode-redirect.azurewebsites.net/',
scopes: [

View File

@@ -48,7 +48,7 @@ describe('AccountProvider.AzureAuth', function (): void {
beforeEach(async function (): Promise<void> {
const tokenCache = new SimpleTokenCache('testTokenService', os.tmpdir(), true, new CredentialsTestProvider());
await tokenCache.init();
baseAuth = new BasicAzureAuth(providerSettings[0].metadata, tokenCache, undefined, AzureAuthType.AuthCodeGrant, 'Auth Code Grant');
baseAuth = new BasicAzureAuth(providerSettings[0].metadata, tokenCache, undefined, undefined, AzureAuthType.AuthCodeGrant, 'Auth Code Grant');
});
it('Basic token set and get', async function (): Promise<void> {