diff --git a/extensions/azurecore/package.json b/extensions/azurecore/package.json index c07985cf83..b534ad98ae 100644 --- a/extensions/azurecore/package.json +++ b/extensions/azurecore/package.json @@ -115,20 +115,6 @@ "All" ] }, - "azure.authenticationLibrary": { - "type": "string", - "description": "%config.authenticationLibrary%", - "default": "MSAL", - "enum": [ - "ADAL", - "MSAL" - ], - "enumDescriptions": [ - "Azure Active Directory Authentication Library", - "Microsoft Authentication Library" - ], - "deprecationMessage": "Warning: ADAL has been deprecated, and is scheduled to be removed in a future release. Please use MSAL (default option) instead." - }, "azure.customProviderSettings": { "type": "array", "description": "%config.customProviderSettings%", diff --git a/extensions/azurecore/package.nls.json b/extensions/azurecore/package.nls.json index 00658009af..8566e800ec 100644 --- a/extensions/azurecore/package.nls.json +++ b/extensions/azurecore/package.nls.json @@ -27,7 +27,6 @@ "config.noSystemKeychain": "Disable system keychain integration. Credentials will be stored in a flat file in the user's home directory.", "config.piiLogging": "Should Personally Identifiable Information (PII) be logged in the Azure Accounts output channel and the output channel log file.", "config.loggingLevel": "[Optional] The verbosity of logging for the Azure Accounts extension.", - "config.authenticationLibrary": "The library used for the AAD auth flow. Please restart ADS after changing this option.", "config.customProviderSettings": "Setting containing custom Azure authentication endpoints. Changes to this setting require a restart to take effect.", "config.providerSettingsTitle": "Provider Settings", "config.providerSettingsName": "Cloud Name", diff --git a/extensions/azurecore/src/account-provider/auths/azureAuth.ts b/extensions/azurecore/src/account-provider/auths/azureAuth.ts index 7fab853714..fb64620a5d 100644 --- a/extensions/azurecore/src/account-provider/auths/azureAuth.ts +++ b/extensions/azurecore/src/account-provider/auths/azureAuth.ts @@ -18,11 +18,8 @@ import { import { Deferred } from '../interfaces'; import * as url from 'url'; import * as Constants from '../../constants'; -import { SimpleTokenCache } from '../utils/simpleTokenCache'; import { MemoryDatabase } from '../utils/memoryDatabase'; -import axios, { AxiosRequestConfig, AxiosResponse } from 'axios'; import { Logger } from '../../utils/Logger'; -import * as qs from 'qs'; import { AzureAuthError } from './azureAuthError'; import { AccountInfo, AuthError, AuthenticationResult, InteractionRequiredAuthError, PublicClientApplication } from '@azure/msal-node'; import { HttpClient } from './httpClient'; @@ -43,20 +40,16 @@ export abstract class AzureAuth implements vscode.Disposable { protected readonly clientId: string; protected readonly resources: Resource[]; protected readonly httpClient: HttpClient; - private _authLibrary: string | undefined; constructor( protected readonly metadata: AzureAccountProviderMetadata, - protected readonly tokenCache: SimpleTokenCache, protected readonly msalCacheProvider: MsalCachePluginProvider, protected readonly context: vscode.ExtensionContext, protected clientApplication: PublicClientApplication, protected readonly uriEventEmitter: vscode.EventEmitter, protected readonly authType: AzureAuthType, - public readonly userFriendlyName: string, - public readonly authLibrary: string + public readonly userFriendlyName: string ) { - this._authLibrary = authLibrary; this.loginEndpointUrl = this.metadata.settings.host; this.commonTenant = { @@ -111,38 +104,24 @@ export abstract class AzureAuth implements vscode.Disposable { if (!this.metadata.settings.microsoftResource) { throw new Error(localize('noMicrosoftResource', "Provider '{0}' does not have a Microsoft resource endpoint defined.", this.metadata.displayName)); } - if (this._authLibrary === Constants.AuthLibrary.MSAL) { - const result = await this.loginMsal(this.organizationTenant, this.metadata.settings.microsoftResource); - loginComplete = result.authComplete; - if (!result?.response || !result.response?.account) { - Logger.error(`Authentication failed: ${loginComplete}`); - return { - canceled: false - }; - } - const token: Token = { - token: result.response.accessToken, - key: result.response.account.homeAccountId, - tokenType: result.response.tokenType, - expiresOn: result.response.expiresOn!.getTime() / 1000 + const result = await this.login(this.organizationTenant, this.metadata.settings.microsoftResource); + loginComplete = result.authComplete; + if (!result?.response || !result.response?.account) { + Logger.error(`Authentication failed: ${loginComplete}`); + return { + canceled: false }; - const tokenClaims = result.response.idTokenClaims; - const account = await this.hydrateAccount(token, tokenClaims); - loginComplete?.resolve(); - return account; - } else {// fallback to ADAL as default - const result = await this.loginAdal(this.commonTenant, this.metadata.settings.microsoftResource); - loginComplete = result.authComplete; - if (!result?.response) { - Logger.error('Authentication failed - no response'); - return { - canceled: false - }; - } - const account = await this.hydrateAccount(result.response.accessToken, result.response.tokenClaims); - loginComplete?.resolve(); - return account; } + const token: Token = { + token: result.response.accessToken, + key: result.response.account.homeAccountId, + tokenType: result.response.tokenType, + expiresOn: result.response.expiresOn!.getTime() / 1000 + }; + const tokenClaims = result.response.idTokenClaims; + const account = await this.hydrateAccount(token, tokenClaims); + loginComplete?.resolve(); + return account; } catch (ex) { Logger.error(`Login failed: ${ex}`); if (ex instanceof AzureAuthError) { @@ -162,158 +141,14 @@ export abstract class AzureAuth implements vscode.Disposable { } } - public async refreshAccessAdal(account: AzureAccount): Promise { - // Deprecated account - delete it. - if (account.key.accountVersion !== Constants.AccountVersion) { - account.delete = true; - return account; - } - try { - // There can be multiple home tenants - // We want to return the one that owns the Azure account. - // Not doing so can result in token being issued for the wrong tenant - const tenant = account.properties.owningTenant; - const tokenResult = await this.getAccountSecurityTokenAdal(account, tenant.id, azdata.AzureResource.MicrosoftResourceManagement); - if (!tokenResult) { - account.isStale = true; - return account; - } - - return await this.hydrateAccount(tokenResult, this.getTokenClaims(tokenResult.token)); - } catch (ex) { - if (ex instanceof AzureAuthError) { - void vscode.window.showErrorMessage(ex.message); - Logger.error(`Error refreshing access for account ${account.displayInfo.displayName}`, ex.originalMessageAndException); - } else { - Logger.error(ex); - } - account.isStale = true; - return account; - } - } - public async hydrateAccount(token: Token | AccessToken, tokenClaims: TokenClaims): Promise { let account: azdata.Account; - if (this._authLibrary === Constants.AuthLibrary.MSAL) { - const tenants = await this.getTenantsMsal(token.token, tokenClaims); - account = this.createAccount(tokenClaims, token.key, tenants); - } else { // fallback to ADAL as default - const tenants = await this.getTenantsAdal({ ...token }); - account = this.createAccount(tokenClaims, token.key, tenants); - } + const tenants = await this.getTenants(token.token, tokenClaims); + account = this.createAccount(tokenClaims, token.key, tenants); return account; } - public async getAccountSecurityTokenAdal(account: AzureAccount, tenantId: string, azureResource: azdata.AzureResource): Promise { - if (account.isStale === true) { - Logger.error('Account was stale. No tokens being fetched.'); - return undefined; - } - - const resource = this.resources.find(s => s.azureResourceId === azureResource); - - if (!resource) { - Logger.error(`Unable to find Azure resource ${azureResource}`); - return undefined; - } - - if (!account.properties.owningTenant) { - // Should never happen - throw new AzureAuthError(localize('azure.owningTenantNotFound', "Owning Tenant information not found for account."), 'Owning tenant not found.', undefined); - } - - const tenant = account.properties.owningTenant?.id === tenantId - ? account.properties.owningTenant - : account.properties.tenants.find(t => t.id === tenantId); - - if (!tenant) { - throw new AzureAuthError(localize('azure.tenantNotFound', "Specified tenant with ID '{0}' not found.", tenantId), `Tenant ${tenantId} not found.`, undefined); - } - - const cachedTokens = await this.getSavedTokenAdal(tenant, resource, account.key); - - // Let's check to see if we can just use the cached tokens to return to the user - if (cachedTokens) { - let expiry = Number(cachedTokens.expiresOn); - if (Number.isNaN(expiry)) { - Logger.error('Expiration time was not defined. This is expected on first launch'); - expiry = 0; - } - const currentTime = new Date().getTime() / 1000; - - let accessToken = cachedTokens.accessToken; - let expiresOn = Number(cachedTokens.expiresOn); - const remainingTime = expiry - currentTime; - const maxTolerance = 2 * 60; // two minutes - - if (remainingTime < maxTolerance) { - const result = await this.refreshTokenAdal(tenant, resource, cachedTokens.refreshToken); - if (result) { - accessToken = result.accessToken; - expiresOn = Number(result.expiresOn); - } - } - // Let's just return here. - if (accessToken) { - return { - ...accessToken, - expiresOn: expiresOn, - tokenType: Constants.Bearer - }; - } - } - - // User didn't have any cached tokens, or the cached tokens weren't useful. - // For most users we can use the refresh token from the general microsoft resource to an access token of basically any type of resource we want. - if (!this.metadata.settings.microsoftResource) { - throw new Error(localize('noMicrosoftResource', "Provider '{0}' does not have a Microsoft resource endpoint defined.", this.metadata.displayName)); - } - const baseTokens = await this.getSavedTokenAdal(this.commonTenant, this.metadata.settings.microsoftResource, account.key); - if (!baseTokens) { - Logger.error('User had no base tokens for the basic resource registered. This should not happen and indicates something went wrong with the authentication cycle'); - const msg = localize('azure.noBaseToken', 'Something failed with the authentication, or your tokens have been deleted from the system. Please try adding your account to Azure Data Studio again.'); - account.isStale = true; - throw new AzureAuthError(msg, 'No base token found', undefined); - } - // Let's try to convert the access token type, worst case we'll have to prompt the user to do an interactive authentication. - const result = await this.refreshTokenAdal(tenant, resource, baseTokens.refreshToken); - if (result?.accessToken) { - return { - ...result.accessToken, - expiresOn: Number(result.expiresOn), - tokenType: Constants.Bearer - }; - } - return undefined; - } - - protected abstract loginAdal(tenant: Tenant, resource: Resource): Promise<{ response: OAuthTokenResponse | undefined, authComplete: Deferred }>; - - protected abstract loginMsal(tenant: Tenant, resource: Resource): Promise<{ response: AuthenticationResult | null, authComplete: Deferred }>; - - /** - * Refreshes a token, if a refreshToken is passed in then we use that. If it is not passed in then we will prompt the user for consent. - * @param tenant - * @param resource - * @param refreshToken - * @returns The oauth token response or undefined. Undefined is returned when the user wants to ignore a tenant or chooses not to start the - * re-authentication process for their tenant. - */ - public async refreshTokenAdal(tenant: Tenant, resource: Resource, refreshToken: RefreshToken | undefined): Promise { - Logger.piiSanitized('Refreshing token', [{ name: 'token', objOrArray: refreshToken }], []); - if (refreshToken) { - const postData: RefreshTokenPostData = { - grant_type: 'refresh_token', - client_id: this.clientId, - refresh_token: refreshToken.token, - tenant: tenant.id, - resource: resource.endpoint - }; - return this.getTokenAdal(tenant, resource, postData); - } - return this.handleInteractionRequiredAdal(tenant, resource); - } - + protected abstract login(tenant: Tenant, resource: Resource): Promise<{ response: AuthenticationResult | null, authComplete: Deferred }>; /** * Gets the access token for the correct account and scope from the token cache, if the correct token doesn't exist in the token cache @@ -323,7 +158,7 @@ export abstract class AzureAuth implements vscode.Disposable { * @returns The authentication result, including the access token. * This function returns 'null' instead of 'undefined' by design as the same is returned by MSAL APIs in the flow (e.g. acquireTokenSilent). */ - public async getTokenMsal(accountId: string, azureResource: azdata.AzureResource, tenantId: string): Promise { + public async getToken(accountId: string, azureResource: azdata.AzureResource, tenantId: string): Promise { const resource = this.resources.find(s => s.azureResourceId === azureResource); if (!resource) { @@ -368,7 +203,7 @@ export abstract class AzureAuth implements vscode.Disposable { id: tenantId, displayName: '' }; - return this.handleInteractionRequiredMsal(tenant, resource); + return this.handleInteractionRequired(tenant, resource); } else { if (e.name === 'ClientAuthError') { Logger.verbose('[ClientAuthError] Failed to silently acquire token'); @@ -399,87 +234,10 @@ export abstract class AzureAuth implements vscode.Disposable { return account; } - public async getTokenAdal(tenant: Tenant, resource: Resource, postData: AuthorizationCodePostData | TokenPostData | RefreshTokenPostData): Promise { - Logger.verbose('Fetching token for tenant {0}', tenant.id); - const tokenUrl = `${this.loginEndpointUrl}${tenant.id}/oauth2/token`; - const response = await this.makePostRequest(tokenUrl, postData); + //#region tenant calls - // ADAL is being deprecated so just ignoring these for now - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - Logger.piiSanitized('Token: ', [{ name: 'access token', objOrArray: response.data }, { name: 'refresh token', objOrArray: response.data }], []); - if (response.data.error === 'interaction_required') { - return this.handleInteractionRequiredAdal(tenant, resource); - } - if (response.data.error) { - Logger.error(`Response returned error : ${response.data}`); - throw new AzureAuthError(localize('azure.responseError', "Token retrieval failed with an error. [Open developer tools]({0}) for more details.", 'command:workbench.action.toggleDevTools'), 'Token retrieval failed', undefined); - } - // ADAL is being deprecated so just ignoring these for now - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const accessTokenString = response.data.access_token; - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const refreshTokenString = response.data.refresh_token; - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const expiresOnString = response.data.expires_on; - return this.getTokenHelperAdal(tenant, resource, accessTokenString, refreshTokenString, expiresOnString); - } - - public async getTokenHelperAdal(tenant: Tenant, resource: Resource, accessTokenString: string, refreshTokenString: string, expiresOnString: string): Promise { - if (!accessTokenString) { - const msg = localize('azure.accessTokenEmpty', 'No access token returned from Microsoft OAuth'); - throw new AzureAuthError(msg, 'Access token was empty', undefined); - } - - const tokenClaims: TokenClaims = this.getTokenClaims(accessTokenString); - let userKey: string; - - // Personal accounts don't have an oid when logging into the `common` tenant, but when logging into their home tenant they end up having an oid. - // This makes the key for the same account be different. - // We need to special case personal accounts. - if (tokenClaims.idp === 'live.com') { // Personal account - userKey = tokenClaims.unique_name ?? tokenClaims.email ?? tokenClaims.sub; - } else { - userKey = tokenClaims.home_oid ?? tokenClaims.oid ?? tokenClaims.unique_name ?? tokenClaims.email ?? tokenClaims.sub; - } - - if (!userKey) { - const msg = localize('azure.noUniqueIdentifier', "The user had no unique identifier within AAD"); - throw new AzureAuthError(msg, 'No unique identifier', undefined); - } - - const accessToken: AccessToken = { - token: accessTokenString, - key: userKey - }; - let refreshToken: RefreshToken | undefined = undefined; - - if (refreshTokenString) { - refreshToken = { - token: refreshTokenString, - key: userKey - }; - } - - const result: OAuthTokenResponse = { - accessToken, - refreshToken, - tokenClaims, - expiresOn: expiresOnString - }; - - const accountKey: azdata.AccountKey = { - providerId: this.metadata.id, - accountId: userKey, - authLibrary: this._authLibrary - }; - - await this.saveTokenAdal(tenant, resource, accountKey, result); - - return result; - } - - public async getTenantsMsal(token: string, tokenClaims: TokenClaims): Promise { - const tenantUri = url.resolve(this.metadata.settings.armResource.endpoint, 'tenants?api-version=2020-01-01'); + public async getTenants(token: string, tokenClaims: TokenClaims): Promise { + const tenantUri = url.resolve(this.metadata.settings.armResource.endpoint, 'tenants?api-version=2019-11-01'); try { Logger.verbose(`Fetching tenants with uri: ${tenantUri}`); let tenantList: string[] = []; @@ -526,135 +284,19 @@ export abstract class AzureAuth implements vscode.Disposable { } } - - //#region tenant calls - public async getTenantsAdal(token: AccessToken): Promise { - const tenantUri = url.resolve(this.metadata.settings.armResource.endpoint, 'tenants?api-version=2020-01-01'); - try { - Logger.verbose(`Fetching tenants with uri: ${tenantUri}`); - let tenantList: string[] = []; - const tenantResponse = await this.makeGetRequest(tenantUri, token.token); - if (tenantResponse.status !== 200) { - Logger.error(`Error with tenant response, status: ${tenantResponse.status} | status text: ${tenantResponse.statusText}`); - Logger.error(`Headers: ${JSON.stringify(tenantResponse.headers)}`); - throw new Error('Error with tenant response'); - } - // ADAL is being deprecated so just ignoring these for now - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const tenants: Tenant[] = tenantResponse.data.value.map((tenantInfo: TenantResponse) => { - if (tenantInfo.displayName) { - tenantList.push(tenantInfo.displayName); - } else { - tenantList.push(tenantInfo.tenantId); - Logger.info('Tenant display name found empty: {0}', tenantInfo.tenantId); - } - return { - id: tenantInfo.tenantId, - displayName: tenantInfo.displayName ? tenantInfo.displayName : tenantInfo.tenantId, - userId: token.key, - tenantCategory: tenantInfo.tenantCategory - } as Tenant; - }); - Logger.verbose(`Tenants: ${tenantList}`); - const homeTenantIndex = tenants.findIndex(tenant => tenant.tenantCategory === Constants.HomeCategory); - // remove home tenant from list of tenants - if (homeTenantIndex >= 0) { - const homeTenant = tenants.splice(homeTenantIndex, 1); - tenants.unshift(homeTenant[0]); - } - - return tenants; - } catch (ex) { - Logger.error(`Error fetching tenants :${ex}`); - throw new Error('Error retrieving tenant information'); - } - } - - //#endregion - - //#region token management - private async saveTokenAdal(tenant: Tenant, resource: Resource, accountKey: azdata.AccountKey, { accessToken, refreshToken, expiresOn }: OAuthTokenResponse) { - const msg = localize('azure.cacheErrorAdd', "Error when adding your account to the cache."); - if (!tenant.id || !resource.id) { - Logger.piiSanitized('Tenant ID or resource ID was undefined', [], [], tenant, resource); - throw new AzureAuthError(msg, 'Adding account to cache failed', undefined); - } - try { - Logger.piiSanitized(`Saving access token`, [{ name: 'access_token', objOrArray: accessToken }], []); - await this.tokenCache.saveCredential(`${accountKey.accountId}_access_${resource.id}_${tenant.id}`, JSON.stringify(accessToken)); - Logger.piiSanitized(`Saving refresh token`, [{ name: 'refresh_token', objOrArray: refreshToken }], []); - await this.tokenCache.saveCredential(`${accountKey.accountId}_refresh_${resource.id}_${tenant.id}`, JSON.stringify(refreshToken)); - this.memdb.set(`${accountKey.accountId}_${tenant.id}_${resource.id}`, expiresOn); - } catch (ex) { - Logger.error(ex); - throw new AzureAuthError(msg, 'Adding account to cache failed', ex); - } - } - - public async getSavedTokenAdal(tenant: Tenant, resource: Resource, accountKey: azdata.AccountKey): Promise<{ accessToken: AccessToken, refreshToken: RefreshToken | undefined, expiresOn: string } | undefined> { - const getMsg = localize('azure.cacheErrorGet', "Error when getting your account from the cache"); - const parseMsg = localize('azure.cacheErrorParse', "Error when parsing your account from the cache"); - - if (!tenant.id || !resource.id) { - Logger.piiSanitized('Tenant ID or resource ID was undefined', [], [], tenant, resource); - throw new AzureAuthError(getMsg, 'Getting account from cache failed', undefined); - } - - let accessTokenString: string | undefined = undefined; - let refreshTokenString: string | undefined = undefined; - let expiresOn: string; - try { - Logger.info('Fetching saved token'); - accessTokenString = await this.tokenCache.getCredential(`${accountKey.accountId}_access_${resource.id}_${tenant.id}`); - refreshTokenString = await this.tokenCache.getCredential(`${accountKey.accountId}_refresh_${resource.id}_${tenant.id}`); - expiresOn = this.memdb.get(`${accountKey.accountId}_${tenant.id}_${resource.id}`); - } catch (ex) { - Logger.error(ex); - throw new AzureAuthError(getMsg, 'Getting account from cache failed', ex); - } - - try { - if (!accessTokenString) { - Logger.error('No access token found'); - return undefined; - } - const accessToken: AccessToken = JSON.parse(accessTokenString) as AccessToken; - let refreshToken: RefreshToken | undefined = undefined; - if (refreshTokenString) { - refreshToken = JSON.parse(refreshTokenString) as RefreshToken; - } - Logger.piiSanitized('GetSavedToken ', [{ name: 'access', objOrArray: accessToken }, { name: 'refresh', objOrArray: refreshToken }], [], `expiresOn=${expiresOn}`); - return { - accessToken, refreshToken, expiresOn - }; - } catch (ex) { - Logger.error(ex); - throw new AzureAuthError(parseMsg, 'Parsing account from cache failed', ex); - } - } //#endregion //#region interaction handling - public async handleInteractionRequiredMsal(tenant: Tenant, resource: Resource): Promise { + public async handleInteractionRequired(tenant: Tenant, resource: Resource): Promise { const shouldOpen = await this.askUserForInteraction(tenant, resource); if (shouldOpen) { - const result = await this.loginMsal(tenant, resource); + const result = await this.login(tenant, resource); result?.authComplete?.resolve(); return result?.response; } return null; } - public async handleInteractionRequiredAdal(tenant: Tenant, resource: Resource): Promise { - const shouldOpen = await this.askUserForInteraction(tenant, resource); - if (shouldOpen) { - const result = await this.loginAdal(tenant, resource); - result?.authComplete?.resolve(); - return result?.response; - } - return undefined; - } - /** * Determines whether the account needs to be refreshed based on received error instance * and STS error codes from errorMessage. @@ -802,8 +444,7 @@ export abstract class AzureAuth implements vscode.Disposable { key: { providerId: this.metadata.id, accountId: key, - accountVersion: Constants.AccountVersion, - authLibrary: this._authLibrary + accountVersion: Constants.AccountVersion }, name: displayName, displayInfo: { @@ -830,37 +471,6 @@ export abstract class AzureAuth implements vscode.Disposable { //#endregion //#region network functions - public async makePostRequest(url: string, postData: AuthorizationCodePostData | TokenPostData | DeviceCodeStartPostData | DeviceCodeCheckPostData): Promise> { - const config: AxiosRequestConfig = { - headers: { - 'Content-Type': 'application/x-www-form-urlencoded' - }, - validateStatus: () => true // Never throw - }; - - // Intercept response and print out the response for future debugging - const response = await axios.post(url, qs.stringify(postData), config); - // ADAL is being deprecated so just ignoring these for now - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - Logger.piiSanitized('POST request ', [{ name: 'data', objOrArray: postData }, { name: 'response', objOrArray: response.data }], [], url); - return response; - } - - private async makeGetRequest(url: string, token: string): Promise> { - const config: AxiosRequestConfig = { - headers: { - 'Content-Type': 'application/json', - 'Authorization': `Bearer ${token}` - }, - validateStatus: () => true // Never throw - }; - - const response = await axios.get(url, config); - // ADAL is being deprecated so just ignoring these for now - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - Logger.piiSanitized('GET request ', [{ name: 'response', objOrArray: response.data.value ?? response.data }], [], url,); - return response; - } //#endregion @@ -877,8 +487,7 @@ export abstract class AzureAuth implements vscode.Disposable { protected toBase64UrlEncoding(base64string: string): string { return base64string.replace(/=/g, '').replace(/\+/g, '-').replace(/\//g, '_'); // Need to use base64url encoding } - - public async deleteAllCacheMsal(): Promise { + public async deleteAllCache(): Promise { this.clientApplication.clearCache(); // unlink both cache files @@ -889,30 +498,16 @@ export abstract class AzureAuth implements vscode.Disposable { await this.msalCacheProvider.clearCacheEncryptionKeys(); } - public async deleteAllCacheAdal(): Promise { - const results = await this.tokenCache.findCredentials(''); - - for (let { account } of results) { - await this.tokenCache.clearCredential(account); - } - } - public async clearCredentials(account: azdata.AccountKey): Promise { try { - // remove account based on authLibrary field, accounts added before this field was present will default to - // ADAL method of account removal - if (account.authLibrary === Constants.AuthLibrary.MSAL) { - return await this.deleteAccountCacheMsal(account); - } else { // fallback to ADAL by default - return await this.deleteAccountCacheAdal(account); - } + return await this.deleteAccountCache(account); } catch (ex) { // We need not prompt user for error if token could not be removed from cache. Logger.error('Error when removing token from cache: ', ex); } } - private async deleteAccountCacheMsal(accountKey: azdata.AccountKey): Promise { + private async deleteAccountCache(accountKey: azdata.AccountKey): Promise { const tokenCache = this.clientApplication.getTokenCache(); try { let msalAccount: AccountInfo | null = await this.getAccountFromMsalCache(accountKey.accountId); @@ -927,16 +522,6 @@ export abstract class AzureAuth implements vscode.Disposable { await this.msalCacheProvider.clearAccountFromLocalCache(accountKey.accountId); } - private async deleteAccountCacheAdal(account: azdata.AccountKey): Promise { - const results = await this.tokenCache.findCredentials(account.accountId); - if (!results) { - Logger.error('ADAL: Unable to find account for removal'); - } - for (let { account } of results) { - await this.tokenCache.clearCredential(account); - } - } - public async dispose() { } public async autoOAuthCancelled(): Promise { } diff --git a/extensions/azurecore/src/account-provider/auths/azureAuthCodeGrant.ts b/extensions/azurecore/src/account-provider/auths/azureAuthCodeGrant.ts index 812e9d102e..cb8ef352ad 100644 --- a/extensions/azurecore/src/account-provider/auths/azureAuthCodeGrant.ts +++ b/extensions/azurecore/src/account-provider/auths/azureAuthCodeGrant.ts @@ -3,12 +3,10 @@ * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { AuthorizationCodePostData, AzureAuth, OAuthTokenResponse } from './azureAuth'; +import { AzureAuth } from './azureAuth'; import { AzureAccountProviderMetadata, AzureAuthType, Resource, Tenant } from 'azurecore'; import { Deferred } from '../interfaces'; import * as vscode from 'vscode'; -import * as crypto from 'crypto'; -import { SimpleTokenCache } from '../utils/simpleTokenCache'; import { SimpleWebServer } from '../utils/simpleWebServer'; import { AzureAuthError } from './azureAuthError'; import { Logger } from '../../utils/Logger'; @@ -16,19 +14,12 @@ import * as Constants from '../../constants'; import * as nls from 'vscode-nls'; import * as path from 'path'; import * as http from 'http'; -import * as qs from 'qs'; import { promises as fs } from 'fs'; import { PublicClientApplication, CryptoProvider, AuthorizationUrlRequest, AuthorizationCodeRequest, AuthenticationResult } from '@azure/msal-node'; import { MsalCachePluginProvider } from '../utils/msalCachePlugin'; const localize = nls.loadMessageBundle(); -interface AuthCodeResponse { - authCode: string; - codeVerifier: string; - redirectUri: string; -} - interface CryptoValues { nonce: string; challengeMethod: string; @@ -43,14 +34,12 @@ export class AzureAuthCodeGrant extends AzureAuth { constructor( metadata: AzureAccountProviderMetadata, - tokenCache: SimpleTokenCache, msalCacheProvider: MsalCachePluginProvider, context: vscode.ExtensionContext, uriEventEmitter: vscode.EventEmitter, - clientApplication: PublicClientApplication, - authLibrary: string + clientApplication: PublicClientApplication ) { - super(metadata, tokenCache, msalCacheProvider, context, clientApplication, uriEventEmitter, AzureAuthType.AuthCodeGrant, AzureAuthCodeGrant.USER_FRIENDLY_NAME, authLibrary); + super(metadata, msalCacheProvider, context, clientApplication, uriEventEmitter, AzureAuthType.AuthCodeGrant, AzureAuthCodeGrant.USER_FRIENDLY_NAME); this.cryptoProvider = new CryptoProvider(); this.pkceCodes = { nonce: '', @@ -60,30 +49,13 @@ export class AzureAuthCodeGrant extends AzureAuth { }; } - protected async loginAdal(tenant: Tenant, resource: Resource): Promise<{ response: OAuthTokenResponse | undefined, authComplete: Deferred }> { - let authCompleteDeferred: Deferred; - let authCompletePromise = new Promise((resolve, reject) => authCompleteDeferred = { resolve, reject }); - let authResponse: AuthCodeResponse; - - if (vscode.env.uiKind === vscode.UIKind.Web) { - authResponse = await this.loginWebAdal(tenant, resource); - } else { - authResponse = await this.loginDesktopAdal(tenant, resource, authCompletePromise); - } - - return { - response: await this.getTokenWithAuthorizationCode(tenant, resource, authResponse), - authComplete: authCompleteDeferred! - }; - } - - protected async loginMsal(tenant: Tenant, resource: Resource): Promise<{ response: AuthenticationResult | null, authComplete: Deferred }> { + protected async login(tenant: Tenant, resource: Resource): Promise<{ response: AuthenticationResult | null, authComplete: Deferred }> { let authCompleteDeferred: Deferred; let authCompletePromise = new Promise((resolve, reject) => authCompleteDeferred = { resolve, reject }); let authCodeRequest: AuthorizationCodeRequest; if (vscode.env.uiKind === vscode.UIKind.Web) { - authCodeRequest = await this.loginWebMsal(tenant, resource); + authCodeRequest = await this.loginWeb(tenant, resource); } else { authCodeRequest = await this.loginDesktopMsal(tenant, resource, authCompletePromise); } @@ -101,26 +73,7 @@ export class AzureAuthCodeGrant extends AzureAuth { } } - /** - * Requests an OAuthTokenResponse from Microsoft OAuth - * - * @param tenant - * @param resource - */ - private async getTokenWithAuthorizationCode(tenant: Tenant, resource: Resource, { authCode, redirectUri, codeVerifier }: AuthCodeResponse): Promise { - const postData: AuthorizationCodePostData = { - grant_type: 'authorization_code', - code: authCode, - client_id: this.clientId, - code_verifier: codeVerifier, - redirect_uri: redirectUri, - resource: resource.endpoint - }; - - return this.getTokenAdal(tenant, resource, postData); - } - - private async loginWebMsal(tenant: Tenant, resource: Resource): Promise { + private async loginWeb(tenant: Tenant, resource: Resource): Promise { const callbackUri = await vscode.env.asExternalUri(vscode.Uri.parse(`${vscode.env.uriScheme}://microsoft.azurecore`)); await this.createCryptoValuesMsal(); const port = (callbackUri.authority.match(/:([0-9]*)$/) || [])[1] || (callbackUri.scheme === 'https' ? 443 : 80); @@ -156,36 +109,6 @@ export class AzureAuthCodeGrant extends AzureAuth { } } - private async loginWebAdal(tenant: Tenant, resource: Resource): Promise { - const callbackUri = await vscode.env.asExternalUri(vscode.Uri.parse(`${vscode.env.uriScheme}://microsoft.azurecore`)); - const { nonce, codeVerifier, codeChallenge } = this.createCryptoValuesAdal(); - const port = (callbackUri.authority.match(/:([0-9]*)$/) || [])[1] || (callbackUri.scheme === 'https' ? 443 : 80); - const state = `${port},${encodeURIComponent(nonce)},${encodeURIComponent(callbackUri.query)}`; - - const loginQuery = { - response_type: 'code', - response_mode: 'query', - client_id: this.clientId, - redirect_uri: this.redirectUri, - state, - prompt: Constants.SELECT_ACCOUNT, - code_challenge_method: Constants.S256_CODE_CHALLENGE_METHOD, - code_challenge: codeChallenge, - resource: resource.id - }; - - const signInUrl = `${this.loginEndpointUrl}${tenant.id}/oauth2/authorize?${qs.stringify(loginQuery)}`; - await vscode.env.openExternal(vscode.Uri.parse(signInUrl)); - - const authCode = await this.handleWebResponse(state); - - return { - authCode, - codeVerifier, - redirectUri: this.redirectUri - }; - } - private async handleWebResponse(state: string): Promise { let uriEventListener: vscode.Disposable; return new Promise((resolve: (value: any) => void, reject) => { @@ -262,40 +185,6 @@ export class AzureAuthCodeGrant extends AzureAuth { } } - private async loginDesktopAdal(tenant: Tenant, resource: Resource, authCompletePromise: Promise): Promise { - const server = new SimpleWebServer(); - let serverPort: string; - - try { - serverPort = await server.startup(); - } catch (ex) { - const msg = localize('azure.serverCouldNotStart', 'Server could not start. This could be a permissions error or an incompatibility on your system. You can try enabling device code authentication from settings.'); - throw new AzureAuthError(msg, 'Server could not start', ex); - } - const { nonce, codeVerifier, codeChallenge } = this.createCryptoValuesAdal(); - const state = `${serverPort},${encodeURIComponent(nonce)}`; - const loginQuery = { - response_type: 'code', - response_mode: 'query', - client_id: this.clientId, - redirect_uri: `${this.redirectUri}:${serverPort}/redirect`, - state, - prompt: Constants.SELECT_ACCOUNT, - code_challenge_method: Constants.S256_CODE_CHALLENGE_METHOD, - code_challenge: codeChallenge, - resource: resource.endpoint - }; - const loginUrl = `${this.loginEndpointUrl}${tenant.id}/oauth2/authorize?${qs.stringify(loginQuery)}`; - await vscode.env.openExternal(vscode.Uri.parse(`http://localhost:${serverPort}/signin?nonce=${encodeURIComponent(nonce)}`)); - const authCode = await this.addServerListeners(server, nonce, loginUrl, authCompletePromise); - return { - authCode, - codeVerifier, - redirectUri: `${this.redirectUri}:${serverPort}/redirect` - }; - - } - private async addServerListeners(server: SimpleWebServer, nonce: string, loginUrl: string, authComplete: Promise): Promise { const mediaPath = path.join(this.context.extensionPath, 'media'); @@ -392,18 +281,6 @@ export class AzureAuthCodeGrant extends AzureAuth { }); } - - private createCryptoValuesAdal(): CryptoValues { - const nonce = crypto.randomBytes(16).toString('base64'); - const codeVerifier = this.toBase64UrlEncoding(crypto.randomBytes(32).toString('base64')); - const codeChallenge = this.toBase64UrlEncoding(crypto.createHash('sha256').update(codeVerifier).digest('base64')); - const challengeMethod = ''; - - return { - nonce, challengeMethod, codeVerifier, codeChallenge - }; - } - private async createCryptoValuesMsal(): Promise { this.pkceCodes.nonce = this.cryptoProvider.createNewGuid(); const { verifier, challenge } = await this.cryptoProvider.generatePkceCodes(); diff --git a/extensions/azurecore/src/account-provider/auths/azureDeviceCode.ts b/extensions/azurecore/src/account-provider/auths/azureDeviceCode.ts index 6fb978e405..c1482fb6e5 100644 --- a/extensions/azurecore/src/account-provider/auths/azureDeviceCode.ts +++ b/extensions/azurecore/src/account-provider/auths/azureDeviceCode.ts @@ -7,11 +7,7 @@ import * as azdata from 'azdata'; import * as vscode from 'vscode'; import * as nls from 'vscode-nls'; import { - AzureAuth, - OAuthTokenResponse, - DeviceCodeStartPostData, - DeviceCodeCheckPostData, - + AzureAuth } from './azureAuth'; import { AzureAccountProviderMetadata, @@ -21,46 +17,26 @@ import { } from 'azurecore'; import { Deferred } from '../interfaces'; import { AuthenticationResult, DeviceCodeRequest, PublicClientApplication } from '@azure/msal-node'; -import { SimpleTokenCache } from '../utils/simpleTokenCache'; import { Logger } from '../../utils/Logger'; import { MsalCachePluginProvider } from '../utils/msalCachePlugin'; const localize = nls.loadMessageBundle(); -interface DeviceCodeLogin { // https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-device-code - device_code: string, - expires_in: number; - interval: number; - message: string; - user_code: string; - verification_url: string -} - -interface DeviceCodeLoginResult { - token_type: string, - scope: string, - expires_in: number, - access_token: string, - refresh_token: string, -} - 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, - tokenCache: SimpleTokenCache, msalCacheProvider: MsalCachePluginProvider, context: vscode.ExtensionContext, uriEventEmitter: vscode.EventEmitter, - clientApplication: PublicClientApplication, - authLibrary: string + clientApplication: PublicClientApplication ) { - super(metadata, tokenCache, msalCacheProvider, context, clientApplication, uriEventEmitter, AzureAuthType.DeviceCode, AzureDeviceCode.USER_FRIENDLY_NAME, authLibrary); + super(metadata, msalCacheProvider, context, clientApplication, uriEventEmitter, AzureAuthType.DeviceCode, AzureDeviceCode.USER_FRIENDLY_NAME); this.pageTitle = localize('addAccount', "Add {0} account", this.metadata.displayName); } - protected async loginMsal(tenant: Tenant, resource: Resource): Promise<{ response: AuthenticationResult | null, authComplete: Deferred }> { + protected async login(tenant: Tenant, resource: Resource): Promise<{ response: AuthenticationResult | null, authComplete: Deferred }> { let authCompleteDeferred: Deferred; let authCompletePromise = new Promise((resolve, reject) => authCompleteDeferred = { resolve, reject }); @@ -80,89 +56,11 @@ export class AzureDeviceCode extends AzureAuth { }; } - protected async loginAdal(tenant: Tenant, resource: Resource): Promise<{ response: OAuthTokenResponse, authComplete: Deferred }> { - let authCompleteDeferred: Deferred; - let authCompletePromise = new Promise((resolve, reject) => authCompleteDeferred = { resolve, reject }); - - const uri = `${this.loginEndpointUrl}/${this.commonTenant.id}/oauth2/devicecode`; - const postData: DeviceCodeStartPostData = { - client_id: this.clientId, - resource: resource.endpoint - }; - - const postResult = await this.makePostRequest(uri, postData); - - const initialDeviceLogin: DeviceCodeLogin = postResult.data as DeviceCodeLogin; - - await azdata.accounts.beginAutoOAuthDeviceCode(this.metadata.id, this.pageTitle, initialDeviceLogin.message, initialDeviceLogin.user_code, initialDeviceLogin.verification_url); - - const finalDeviceLogin = await this.setupPolling(initialDeviceLogin); - - const accessTokenString = finalDeviceLogin.access_token; - const refreshTokenString = finalDeviceLogin.refresh_token; - - const currentTime = new Date().getTime() / 1000; - const expiresOn = `${currentTime + finalDeviceLogin.expires_in}`; - - const result = await this.getTokenHelperAdal(tenant, resource, accessTokenString, refreshTokenString, expiresOn); - this.closeOnceComplete(authCompletePromise).catch(Logger.error); - - return { - response: result, - authComplete: authCompleteDeferred! - }; - } - private async closeOnceComplete(promise: Promise): Promise { await promise; azdata.accounts.endAutoOAuthDeviceCode(); } - private setupPolling(info: DeviceCodeLogin): Promise { - const timeoutMessage = localize('azure.timeoutDeviceCode', 'Timed out when waiting for device code login.'); - const fiveMinutes = 5 * 60 * 1000; - - return new Promise((resolve, reject) => { - let timeout: NodeJS.Timer; - - const timer = setInterval(async () => { - const x = await this.checkForResult(info); - if (!x.access_token) { - return; - } - clearTimeout(timeout); - clearInterval(timer); - resolve(x); - }, info.interval * 1000); - - timeout = setTimeout(() => { - clearInterval(timer); - reject(new Error(timeoutMessage)); - }, fiveMinutes); - }); - } - - private async checkForResult(info: DeviceCodeLogin): Promise { - const msg = localize('azure.deviceCodeCheckFail', "Error encountered when trying to check for login results"); - try { - const uri = `${this.loginEndpointUrl}/${this.commonTenant}/oauth2/token`; - const postData: DeviceCodeCheckPostData = { - grant_type: 'urn:ietf:params:oauth:grant-type:device_code', - client_id: this.clientId, - tenant: this.commonTenant.id, - code: info.device_code - }; - - const postResult = await this.makePostRequest(uri, postData); - const result: DeviceCodeLoginResult = postResult.data as DeviceCodeLoginResult; - - return result; - } catch (ex) { - Logger.error('Unexpected error making Azure auth request', 'azureCore.checkForResult', JSON.stringify(ex?.response?.data, undefined, 2)); - throw new Error(msg); - } - } - public override async autoOAuthCancelled(): Promise { return azdata.accounts.endAutoOAuthDeviceCode(); } diff --git a/extensions/azurecore/src/account-provider/azureAccountProvider.ts b/extensions/azurecore/src/account-provider/azureAccountProvider.ts index 6787f20635..5da76cb9f3 100644 --- a/extensions/azurecore/src/account-provider/azureAccountProvider.ts +++ b/extensions/azurecore/src/account-provider/azureAccountProvider.ts @@ -14,12 +14,10 @@ import { } from 'azurecore'; import { Deferred } from './interfaces'; import { AuthenticationResult, PublicClientApplication } from '@azure/msal-node'; -import { SimpleTokenCache } from './utils/simpleTokenCache'; import { Logger } from '../utils/Logger'; import { MultiTenantTokenResponse, Token, AzureAuth } from './auths/azureAuth'; import { AzureAuthCodeGrant } from './auths/azureAuthCodeGrant'; import { AzureDeviceCode } from './auths/azureDeviceCode'; -import { filterAccounts } from '../azureResource/utils'; import * as Constants from '../constants'; import { MsalCachePluginProvider } from './utils/msalCachePlugin'; import { getTenantIgnoreList } from '../utils'; @@ -35,12 +33,10 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp constructor( metadata: AzureAccountProviderMetadata, - tokenCache: SimpleTokenCache, context: vscode.ExtensionContext, clientApplication: PublicClientApplication, private readonly msalCacheProvider: MsalCachePluginProvider, uriEventHandler: vscode.EventEmitter, - private readonly authLibrary: string, private readonly forceDeviceCode: boolean = false ) { this.clientApplication = clientApplication; @@ -48,11 +44,11 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp vscode.workspace.onDidChangeConfiguration((changeEvent) => { const impactProvider = changeEvent.affectsConfiguration(Constants.AccountsAzureAuthSection); if (impactProvider === true) { - this.handleAuthMapping(metadata, tokenCache, context, uriEventHandler); + this.handleAuthMapping(metadata, context, uriEventHandler); } }); - this.handleAuthMapping(metadata, tokenCache, context, uriEventHandler); + this.handleAuthMapping(metadata, context, uriEventHandler); } dispose() { @@ -60,13 +56,10 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp } clearTokenCache(): Thenable { - return this.authLibrary === Constants.AuthLibrary.MSAL - ? this.getAuthMethod().deleteAllCacheMsal() - // fallback to ADAL as default - : this.getAuthMethod().deleteAllCacheAdal(); + return this.getAuthMethod().deleteAllCache(); } - private handleAuthMapping(metadata: AzureAccountProviderMetadata, tokenCache: SimpleTokenCache, context: vscode.ExtensionContext, uriEventHandler: vscode.EventEmitter) { + private handleAuthMapping(metadata: AzureAccountProviderMetadata, context: vscode.ExtensionContext, uriEventHandler: vscode.EventEmitter) { this.authMappings.forEach(m => m.dispose()); this.authMappings.clear(); @@ -75,10 +68,10 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp const deviceCodeMethod: boolean = configuration.get(Constants.AuthType.DeviceCode, false); if (codeGrantMethod === true && !this.forceDeviceCode) { - this.authMappings.set(AzureAuthType.AuthCodeGrant, new AzureAuthCodeGrant(metadata, tokenCache, this.msalCacheProvider, context, uriEventHandler, this.clientApplication, this.authLibrary)); + this.authMappings.set(AzureAuthType.AuthCodeGrant, new AzureAuthCodeGrant(metadata, this.msalCacheProvider, context, uriEventHandler, this.clientApplication)); } if (deviceCodeMethod === true || this.forceDeviceCode) { - this.authMappings.set(AzureAuthType.DeviceCode, new AzureDeviceCode(metadata, tokenCache, this.msalCacheProvider, context, uriEventHandler, this.clientApplication, this.authLibrary)); + this.authMappings.set(AzureAuthType.DeviceCode, new AzureDeviceCode(metadata, this.msalCacheProvider, context, uriEventHandler, this.clientApplication)); } if (codeGrantMethod === false && deviceCodeMethod === false && !this.forceDeviceCode) { console.error('No authentication methods selected'); @@ -110,25 +103,20 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp private async _initialize(storedAccounts: AzureAccount[]): Promise { const accounts: AzureAccount[] = []; Logger.verbose(`Initializing stored accounts ${JSON.stringify(accounts)}`); - const updatedAccounts = filterAccounts(storedAccounts, this.authLibrary); - for (let account of updatedAccounts) { + for (let account of storedAccounts) { const azureAuth = this.getAuthMethod(account); if (!azureAuth) { account.isStale = true; accounts.push(account); } else { account.isStale = false; - if (this.authLibrary === Constants.AuthLibrary.MSAL) { - // Check MSAL Cache before adding account, to mark it as stale if it is not present in cache - const accountInCache = await azureAuth.getAccountFromMsalCache(account.key.accountId); - if (!accountInCache) { - account.isStale = true; - } - accounts.push(account); - - } else { // fallback to ADAL as default - accounts.push(await azureAuth.refreshAccessAdal(account)); + // Check MSAL Cache before adding account, to mark it as stale if it is not present in cache + const accountInCache = await azureAuth.getAccountFromMsalCache(account.key.accountId); + if (!accountInCache) { + account.isStale = true; } + accounts.push(account); + } } this.initComplete.resolve(); @@ -148,53 +136,49 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp const azureAuth = this.getAuthMethod(account); if (azureAuth) { Logger.piiSanitized(`Getting account security token for ${JSON.stringify(account.key)} (tenant ${tenantId}). Auth Method = ${azureAuth.userFriendlyName}`, [], []); - if (this.authLibrary === Constants.AuthLibrary.MSAL) { - try { - // Fetch cached token from local cache if token is available and valid. - let accessToken = await this.msalCacheProvider.getTokenFromLocalCache(account.key.accountId, tenantId, resource); - if (this.isValidToken(accessToken) && - // Ensure MSAL Cache contains user account - (await this.clientApplication.getAllAccounts()).find((accountInfo) => accountInfo.homeAccountId === account.key.accountId)) { - return accessToken; - } // else fallback to fetching a new token. - } catch (e) { - // Log any error and move on to fetching fresh access token. - Logger.info(`Could not fetch access token from cache: ${e}, fetching new access token instead.`); - } - tenantId = tenantId || account.properties.owningTenant.id; - if (getTenantIgnoreList().includes(tenantId)) { - // Tenant found in ignore list, don't fetch access token. - Logger.info(`Tenant ${tenantId} found in the ignore list, authentication will not be attempted. Please remove tenant from setting: '${Constants.AzureTenantConfigFilterSetting}' if you want to re-enable tenant for authentication.`); - throw new TenantIgnoredError(localize('tenantIgnoredError', 'Tenant found in ignore list, authentication not attempted. You can remove tenant {0} from ignore list in settings.json file: {1} if you wish to access resources from this tenant.', tenantId, Constants.AzureTenantConfigFilterSetting)); + try { + // Fetch cached token from local cache if token is available and valid. + let accessToken = await this.msalCacheProvider.getTokenFromLocalCache(account.key.accountId, tenantId, resource); + if (this.isValidToken(accessToken) && + // Ensure MSAL Cache contains user account + (await this.clientApplication.getAllAccounts()).find((accountInfo) => accountInfo.homeAccountId === account.key.accountId)) { + return accessToken; + } // else fallback to fetching a new token. + } catch (e) { + // Log any error and move on to fetching fresh access token. + Logger.info(`Could not fetch access token from cache: ${e}, fetching new access token instead.`); + } + tenantId = tenantId || account.properties.owningTenant.id; + if (getTenantIgnoreList().includes(tenantId)) { + // Tenant found in ignore list, don't fetch access token. + Logger.info(`Tenant ${tenantId} found in the ignore list, authentication will not be attempted. Please remove tenant from setting: '${Constants.AzureTenantConfigFilterSetting}' if you want to re-enable tenant for authentication.`); + throw new TenantIgnoredError(localize('tenantIgnoredError', 'Tenant found in ignore list, authentication not attempted. You can remove tenant {0} from ignore list in settings.json file: {1} if you wish to access resources from this tenant.', tenantId, Constants.AzureTenantConfigFilterSetting)); + } else { + let authResult = await azureAuth.getToken(account.key.accountId, resource, tenantId); + if (this.isAuthenticationResult(authResult) && authResult.account && authResult.account.idTokenClaims) { + const token: Token = { + key: authResult.account.homeAccountId, + token: authResult.accessToken, + tokenType: authResult.tokenType, + expiresOn: authResult.account.idTokenClaims.exp!, + tenantId: tenantId, + resource: resource + }; + try { + await this.msalCacheProvider.writeTokenToLocalCache(token); + } catch (e) { + Logger.error(`Could not save access token to local cache: ${e}, this might cause throttling of AAD requests.`); + } + return token; } else { - let authResult = await azureAuth.getTokenMsal(account.key.accountId, resource, tenantId); - if (this.isAuthenticationResult(authResult) && authResult.account && authResult.account.idTokenClaims) { - const token: Token = { - key: authResult.account.homeAccountId, - token: authResult.accessToken, - tokenType: authResult.tokenType, - expiresOn: authResult.account.idTokenClaims.exp!, - tenantId: tenantId, - resource: resource - }; - try { - await this.msalCacheProvider.writeTokenToLocalCache(token); - } catch (e) { - Logger.error(`Could not save access token to local cache: ${e}, this might cause throttling of AAD requests.`); - } - return token; + Logger.error(`MSAL: getToken call failed: ${authResult}`); + // Throw error with MSAL-specific code/message, else throw generic error message + if (this.isProviderError(authResult)) { + throw new Error(localize('msalTokenError', `{0} occurred when acquiring token. \n{1}`, authResult.errorCode, authResult.errorMessage)); } else { - Logger.error(`MSAL: getToken call failed: ${authResult}`); - // Throw error with MSAL-specific code/message, else throw generic error message - if (this.isProviderError(authResult)) { - throw new Error(localize('msalTokenError', `{0} occurred when acquiring token. \n{1}`, authResult.errorCode, authResult.errorMessage)); - } else { - throw new Error(localize('genericTokenError', 'Failed to get token')); - } + throw new Error(localize('genericTokenError', 'Failed to get token')); } } - } else { // fallback to ADAL as default - return azureAuth.getAccountSecurityTokenAdal(account, tenantId, resource); } } else { account.isStale = true; diff --git a/extensions/azurecore/src/account-provider/azureAccountProviderService.ts b/extensions/azurecore/src/account-provider/azureAccountProviderService.ts index 26e6eb72f5..5db8c879f8 100644 --- a/extensions/azurecore/src/account-provider/azureAccountProviderService.ts +++ b/extensions/azurecore/src/account-provider/azureAccountProviderService.ts @@ -9,7 +9,6 @@ import * as events from 'events'; import * as nls from 'vscode-nls'; import * as vscode from 'vscode'; import { promises as fsPromises } from 'fs'; -import { SimpleTokenCache } from './utils/simpleTokenCache'; import providerSettings from './providerSettings'; import { AzureAccountProvider as AzureAccountProvider } from './azureAccountProvider'; import { AzureAccountProviderMetadata, CacheEncryptionKeys } from 'azurecore'; @@ -46,8 +45,7 @@ export class AzureAccountProviderService implements vscode.Disposable { private _onEncryptionKeysUpdated: vscode.EventEmitter; constructor(private _context: vscode.ExtensionContext, - private _userStoragePath: string, - private _authLibrary: string) { + private _userStoragePath: string) { this._onEncryptionKeysUpdated = new vscode.EventEmitter(); this._disposables.push(vscode.window.registerUriHandler(this._uriEventHandler)); } @@ -163,8 +161,6 @@ export class AzureAccountProviderService implements vscode.Disposable { private async registerAccountProvider(provider: ProviderSettings): Promise { const isSaw: boolean = vscode.env.appName.toLowerCase().indexOf(Constants.Saw) > 0; - const noSystemKeychain = vscode.workspace.getConfiguration(Constants.AzureSection).get(Constants.NoSystemKeyChainSection); - const tokenCacheKey = `azureTokenCache-${provider.metadata.id}`; const tokenCacheKeyMsal = Constants.MSALCacheName; await this.clearOldCacheIfExists(); try { @@ -172,18 +168,10 @@ export class AzureAccountProviderService implements vscode.Disposable { throw new Error('Credential provider not registered'); } - // ADAL Token Cache - let simpleTokenCache = new SimpleTokenCache(tokenCacheKey, this._userStoragePath, noSystemKeychain, this._credentialProvider); - if (this._authLibrary === Constants.AuthLibrary.ADAL) { - await simpleTokenCache.init(); - } - // MSAL Cache Plugin this._cachePluginProvider = new MsalCachePluginProvider(tokenCacheKeyMsal, this._userStoragePath, this._credentialProvider, this._onEncryptionKeysUpdated); - if (this._authLibrary === Constants.AuthLibrary.MSAL) { - // Initialize cache provider and encryption keys - await this._cachePluginProvider.init(); - } + // Initialize cache provider and encryption keys + await this._cachePluginProvider.init(); const msalConfiguration: Configuration = { auth: { @@ -204,8 +192,8 @@ export class AzureAccountProviderService implements vscode.Disposable { this.clientApplication = new PublicClientApplication(msalConfiguration); let accountProvider = new AzureAccountProvider(provider.metadata as AzureAccountProviderMetadata, - simpleTokenCache, this._context, this.clientApplication, this._cachePluginProvider, - this._uriEventHandler, this._authLibrary, isSaw); + this._context, this.clientApplication, this._cachePluginProvider, + this._uriEventHandler, isSaw); this._accountProviders[provider.metadata.id] = accountProvider; this._accountDisposals[provider.metadata.id] = azdata.accounts.registerAccountProvider(provider.metadata, accountProvider); } catch (e) { diff --git a/extensions/azurecore/src/account-provider/utils/fileDatabase.ts b/extensions/azurecore/src/account-provider/utils/fileDatabase.ts deleted file mode 100644 index e9fae2e412..0000000000 --- a/extensions/azurecore/src/account-provider/utils/fileDatabase.ts +++ /dev/null @@ -1,188 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the Source EULA. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { promises as fs, constants as fsConstants } from 'fs'; -import { Logger } from '../../utils/Logger'; - -export type ReadWriteHook = (contents: string, resetOnError?: boolean) => Promise; -const noOpHook: ReadWriteHook = async (contents): Promise => { - return contents; -}; - -export class AlreadyInitializedError extends Error { - -} - -type DbMap = { [key: string]: string }; -export class FileDatabase { - private db: DbMap = {}; - private isDirty = false; - private isSaving = false; - private isInitialized = false; - private saveInterval: NodeJS.Timer | undefined; - - constructor( - private readonly dbPath: string, - private readHook: ReadWriteHook = noOpHook, - private writeHook: ReadWriteHook = noOpHook - ) { - - } - - /** - * Sets a new read hook. Throws AlreadyInitializedError if the database has already started. - * @param hook - */ - public setReadHook(hook: ReadWriteHook): void { - if (this.isInitialized) { - throw new AlreadyInitializedError(); - } - this.readHook = hook; - } - - /** - * Sets a new write hook. - * @param hook - */ - public setWriteHook(hook: ReadWriteHook): void { - this.writeHook = hook; - } - - public async set(key: string, value: string): Promise { - await this.waitForFileSave(); - this.db[key] = value; - this.isDirty = true; - } - - public get(key: string): string { - return this.db[key]; - } - - public async delete(key: string): Promise { - await this.waitForFileSave(); - delete this.db[key]; - this.isDirty = true; - } - - public async clear(): Promise { - await this.waitForFileSave(); - this.db = {}; - this.isDirty = true; - } - - public getPrefix(keyPrefix: string): { key: string, value: string }[] { - return Object.entries(this.db).filter(([key]) => { - return key.startsWith(keyPrefix); - }).map(([key, value]) => { - return { key, value }; - }); - } - - public async deletePrefix(keyPrefix: string): Promise { - await this.waitForFileSave(); - Object.keys(this.db).forEach(s => { - if (s.startsWith(keyPrefix)) { - delete this.db[s]; - } - }); - this.isDirty = true; - } - - public async initialize(): Promise { - this.isInitialized = true; - this.saveInterval = setInterval(() => this.save(), 20 * 1000); - let fileContents: string; - try { - await fs.access(this.dbPath, fsConstants.R_OK | fsConstants.R_OK); - fileContents = await fs.readFile(this.dbPath, { encoding: 'utf8' }); - fileContents = await this.readHook(fileContents, true); - } catch (ex) { - Logger.error(`Error occurred when initializing File Database from file system cache, ADAL cache will be reset: ${ex}`); - await this.createFile(); - this.db = {}; - this.isDirty = true; - return; - } - - try { - this.db = JSON.parse(fileContents) as DbMap; - } catch (ex) { - Logger.error(`Error occurred when reading file database contents as JSON, ADAL cache will be reset: ${ex}`); - await this.createFile(); - this.db = {}; - } - } - - public async shutdown(): Promise { - await this.waitForFileSave(); - if (this.saveInterval) { - clearInterval(this.saveInterval); - } - await this.save(); - } - - /** - * This doesn't need to be called as a timer will automatically call it. - */ - public async save(): Promise { - try { - await this.waitForFileSave(); - if (this.isDirty === false) { - return; - } - - this.isSaving = true; - let contents = JSON.stringify(this.db); - contents = await this.writeHook(contents); - - await fs.writeFile(this.dbPath, contents, { encoding: 'utf8' }); - - this.isDirty = false; - } catch (ex) { - Logger.error(`Error occurred while saving cache contents to file storage, this may cause issues with ADAL cache persistence: ${ex}`); - } finally { - this.isSaving = false; - } - } - - private async waitForFileSave(): Promise { - const cleanupCrew: NodeJS.Timer[] = []; - - const sleepToFail = (time: number): Promise => { - return new Promise((_, reject) => { - const timeout = setTimeout(reject, time); - cleanupCrew.push(timeout); - }); - }; - - const poll = (func: () => boolean): Promise => { - return new Promise(resolve => { - const interval = setInterval(() => { - if (func() === true) { - resolve(); - } - }, 100); - cleanupCrew.push(interval); - }); - }; - - if (this.isSaving) { - const timeout = sleepToFail(5 * 1000); - const check = poll(() => !this.isSaving); - - try { - return await Promise.race([timeout, check]); - } catch (ex) { - throw new Error('Save timed out'); - } finally { - cleanupCrew.forEach(clearInterval); - } - } - } - - private async createFile(): Promise { - return fs.writeFile(this.dbPath, '', { encoding: 'utf8' }); - } -} diff --git a/extensions/azurecore/src/account-provider/utils/fileEncryptionHelper.ts b/extensions/azurecore/src/account-provider/utils/fileEncryptionHelper.ts index fc92b5ccfa..c32f2cb65d 100644 --- a/extensions/azurecore/src/account-provider/utils/fileEncryptionHelper.ts +++ b/extensions/azurecore/src/account-provider/utils/fileEncryptionHelper.ts @@ -6,21 +6,19 @@ import * as azdata from 'azdata'; import * as os from 'os'; import * as crypto from 'crypto'; import * as vscode from 'vscode'; -import { AuthLibrary } from '../../constants'; import * as LocalizedConstants from '../../localizedConstants'; import { Logger } from '../../utils/Logger'; import { CacheEncryptionKeys } from 'azurecore'; export class FileEncryptionHelper { constructor( - private readonly _authLibrary: AuthLibrary, private readonly _credentialService: azdata.CredentialProvider, protected readonly _fileName: string, private readonly _onEncryptionKeysUpdated?: vscode.EventEmitter ) { - this._algorithm = this._authLibrary === AuthLibrary.MSAL ? 'aes-256-cbc' : 'aes-256-gcm'; - this._bufferEncoding = this._authLibrary === AuthLibrary.MSAL ? 'utf16le' : 'hex'; - this._binaryEncoding = this._authLibrary === AuthLibrary.MSAL ? 'base64' : 'hex'; + this._algorithm = 'aes-256-cbc'; + this._bufferEncoding = 'utf16le'; + this._binaryEncoding = 'base64'; } private _algorithm: string; @@ -51,7 +49,7 @@ export class FileEncryptionHelper { } // Emit event with cache encryption keys to send notification to provider services. - if (this._authLibrary === AuthLibrary.MSAL && this._onEncryptionKeysUpdated) { + if (this._onEncryptionKeysUpdated) { this._onEncryptionKeysUpdated.fire(this.getEncryptionKeys()); Logger.verbose('FileEncryptionHelper: Fired encryption keys updated event.'); } @@ -73,9 +71,6 @@ export class FileEncryptionHelper { } const cipherIv = crypto.createCipheriv(this._algorithm, this._keyBuffer!, this._ivBuffer!); let cipherText = `${cipherIv.update(content, 'utf8', this._binaryEncoding)}${cipherIv.final(this._binaryEncoding)}`; - if (this._authLibrary === AuthLibrary.ADAL) { - cipherText += `%${(cipherIv as crypto.CipherGCM).getAuthTag().toString(this._binaryEncoding)}`; - } return cipherText; } @@ -86,14 +81,6 @@ export class FileEncryptionHelper { } let plaintext = content; const decipherIv = crypto.createDecipheriv(this._algorithm, this._keyBuffer!, this._ivBuffer!); - if (this._authLibrary === AuthLibrary.ADAL) { - const split = content.split('%'); - if (split.length !== 2) { - throw new Error('File didn\'t contain the auth tag.'); - } - (decipherIv as crypto.DecipherGCM).setAuthTag(Buffer.from(split[1], this._binaryEncoding)); - plaintext = split[0]; - } return `${decipherIv.update(plaintext, this._binaryEncoding, 'utf8')}${decipherIv.final('utf8')}`; } catch (ex) { Logger.error(`FileEncryptionHelper: Error occurred when decrypting data, IV/KEY will be reset: ${ex}`); @@ -130,7 +117,7 @@ export class FileEncryptionHelper { .then((result) => { status = result; if (result) { - Logger.info(`FileEncryptionHelper: Successfully saved encryption key ${credentialId} for ${this._authLibrary} persistent cache encryption in system credential store.`); + Logger.info(`FileEncryptionHelper: Successfully saved encryption key ${credentialId} persistent cache encryption in system credential store.`); } }, (e => { throw Error(`FileEncryptionHelper: Could not save encryption key: ${credentialId}: ${e}`); diff --git a/extensions/azurecore/src/account-provider/utils/msalCachePlugin.ts b/extensions/azurecore/src/account-provider/utils/msalCachePlugin.ts index 566241d350..a68dd44638 100644 --- a/extensions/azurecore/src/account-provider/utils/msalCachePlugin.ts +++ b/extensions/azurecore/src/account-provider/utils/msalCachePlugin.ts @@ -10,7 +10,7 @@ import * as lockFile from 'lockfile'; import * as path from 'path'; import * as azdata from 'azdata'; import * as vscode from 'vscode'; -import { AccountsClearTokenCacheCommand, AuthLibrary, LocalCacheSuffix, LockFileSuffix } from '../../constants'; +import { AccountsClearTokenCacheCommand, LocalCacheSuffix, LockFileSuffix } from '../../constants'; import { Logger } from '../../utils/Logger'; import { FileEncryptionHelper } from './fileEncryptionHelper'; import { CacheEncryptionKeys } from 'azurecore'; @@ -34,7 +34,7 @@ export class MsalCachePluginProvider { private readonly _credentialService: azdata.CredentialProvider, private readonly _onEncryptionKeysUpdated: vscode.EventEmitter ) { - this._fileEncryptionHelper = new FileEncryptionHelper(AuthLibrary.MSAL, this._credentialService, this._serviceName, this._onEncryptionKeysUpdated); + this._fileEncryptionHelper = new FileEncryptionHelper(this._credentialService, this._serviceName, this._onEncryptionKeysUpdated); this._msalCacheConfiguration = { name: 'MSAL', cacheFilePath: path.join(msalFilePath, this._serviceName), diff --git a/extensions/azurecore/src/account-provider/utils/simpleTokenCache.ts b/extensions/azurecore/src/account-provider/utils/simpleTokenCache.ts deleted file mode 100644 index 128a3d8dad..0000000000 --- a/extensions/azurecore/src/account-provider/utils/simpleTokenCache.ts +++ /dev/null @@ -1,169 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the Source EULA. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ -import * as keytarType from 'keytar'; -import { join, parse } from 'path'; -import { FileDatabase } from './fileDatabase'; -import * as azdata from 'azdata'; -import { FileEncryptionHelper } from './fileEncryptionHelper'; -import { AuthLibrary } from '../../constants'; - -function getSystemKeytar(): Keytar | undefined { - try { - return require('keytar'); - } catch (err) { - console.warn(err); - } - - return undefined; -} - -export type MultipleAccountsResponse = { account: string, password: string }[]; - -// allow-any-unicode-next-line -const separator = '§'; - -async function getFileKeytar(filePath: string, credentialService: azdata.CredentialProvider): Promise { - const fileName = parse(filePath).base; - const fileEncryptionHelper: FileEncryptionHelper = new FileEncryptionHelper(AuthLibrary.ADAL, credentialService, fileName); - const db = new FileDatabase(filePath, fileEncryptionHelper.fileOpener, fileEncryptionHelper.fileSaver); - await db.initialize(); - - const fileKeytar: Keytar = { - async getPassword(service: string, account: string): Promise { - return db.get(`${service}${separator}${account}`); - }, - - async setPassword(service: string, account: string, password: string): Promise { - await db.set(`${service}${separator}${account}`, password); - }, - - async deletePassword(service: string, account: string): Promise { - await db.delete(`${service}${separator}${account}`); - return true; - }, - - async getPasswords(service: string): Promise { - const result = db.getPrefix(`${service}`); - if (!result) { - return []; - } - - return result.map(({ key, value }) => { - return { - account: key.split(separator)[1], - password: value - }; - }); - } - }; - return fileKeytar; -} - - -export type Keytar = { - getPassword: typeof keytarType['getPassword']; - setPassword: typeof keytarType['setPassword']; - deletePassword: typeof keytarType['deletePassword']; - getPasswords: (service: string) => Promise; - findCredentials?: typeof keytarType['findCredentials']; -}; - -export class SimpleTokenCache { - private keytar: Keytar | undefined; - - constructor( - private serviceName: string, - private readonly userStoragePath: string, - private readonly forceFileStorage: boolean = false, - private readonly credentialService: azdata.CredentialProvider, - ) { } - - async init(): Promise { - this.serviceName = this.serviceName.replace(/-/g, '_'); - let keytar: Keytar | undefined; - if (this.forceFileStorage === false) { - keytar = getSystemKeytar(); - - // Add new method to keytar - if (keytar) { - keytar.getPasswords = async (service: string): Promise => { - const [serviceName, accountPrefix] = service.split(separator); - if (serviceName === undefined || accountPrefix === undefined) { - throw new Error('Service did not have separator: ' + service); - } - - const results = await keytar!.findCredentials!(serviceName); - return results.filter(({ account }) => { - return account.startsWith(accountPrefix); - }); - }; - } - } - if (!keytar) { - keytar = await getFileKeytar(join(this.userStoragePath, this.serviceName), this.credentialService); - } - this.keytar = keytar; - } - - async saveCredential(id: string, key: string): Promise { - if (!this.forceFileStorage && key.length > 2500) { // Windows limitation - throw new Error('Key length is longer than 2500 chars'); - } - - if (id.includes(separator)) { - throw new Error('Separator included in ID'); - } - - try { - const keytar = this.getKeytar(); - return await keytar.setPassword(this.serviceName, id, key); - } catch (ex) { - console.warn(`Adding key failed: ${ex}`); - } - } - - async getCredential(id: string): Promise { - try { - const keytar = this.getKeytar(); - const result = await keytar.getPassword(this.serviceName, id); - - if (result === null) { - return undefined; - } - - return result; - } catch (ex) { - console.warn(`Getting key failed: ${ex}`); - return undefined; - } - } - - async clearCredential(id: string): Promise { - try { - const keytar = this.getKeytar(); - return await keytar.deletePassword(this.serviceName, id); - } catch (ex) { - console.warn(`Clearing key failed: ${ex}`); - return false; - } - } - - async findCredentials(prefix: string): Promise<{ account: string, password: string }[]> { - try { - const keytar = this.getKeytar(); - return await keytar.getPasswords(`${this.serviceName}${separator}${prefix}`); - } catch (ex) { - console.warn(`Finding credentials failed: ${ex}`); - return []; - } - } - - private getKeytar(): Keytar { - if (!this.keytar) { - throw new Error('Keytar not initialized'); - } - return this.keytar; - } -} diff --git a/extensions/azurecore/src/azureDataGridProvider.ts b/extensions/azurecore/src/azureDataGridProvider.ts index e2a33a57b8..7dbc0698e9 100644 --- a/extensions/azurecore/src/azureDataGridProvider.ts +++ b/extensions/azurecore/src/azureDataGridProvider.ts @@ -28,15 +28,14 @@ const typesClause = [ ].map(type => `type == "${type}"`).join(' or '); export class AzureDataGridProvider implements azdata.DataGridProvider { - constructor(private _appContext: AppContext, - private readonly authLibrary: string) { } + constructor(private _appContext: AppContext) { } public providerId = constants.dataGridProviderId; public title = loc.azureResourcesGridTitle; public async getDataGridItems() { let accounts: azdata.Account[]; - accounts = azureResourceUtils.filterAccounts(await azdata.accounts.getAllAccounts(), this.authLibrary); + accounts = await azdata.accounts.getAllAccounts(); const items: any[] = []; await Promise.all(accounts.map(async (account) => { await Promise.all(account.properties.tenants.map(async (tenant: { id: string; }) => { diff --git a/extensions/azurecore/src/azureResource/commands.ts b/extensions/azurecore/src/azureResource/commands.ts index f6e8e6f266..27afdcc441 100644 --- a/extensions/azurecore/src/azureResource/commands.ts +++ b/extensions/azurecore/src/azureResource/commands.ts @@ -17,11 +17,11 @@ import { AzureResourceServiceNames } from './constants'; import { AzureAccount, Tenant, azureResource } from 'azurecore'; import { FlatTenantTreeNode } from './tree/flatTenantTreeNode'; import { ConnectionDialogTreeProvider } from './tree/connectionDialogTreeProvider'; -import { AzureResourceErrorMessageUtil, filterAccounts } from './utils'; +import { AzureResourceErrorMessageUtil } from './utils'; import { AzureResourceTenantTreeNode } from './tree/tenantTreeNode'; import { FlatAccountTreeNode } from './tree/flatAccountTreeNode'; -export function registerAzureResourceCommands(appContext: AppContext, azureViewTree: AzureResourceTreeProvider, connectionDialogTree: ConnectionDialogTreeProvider, authLibrary: string): void { +export function registerAzureResourceCommands(appContext: AppContext, azureViewTree: AzureResourceTreeProvider, connectionDialogTree: ConnectionDialogTreeProvider): void { const trees = [azureViewTree, connectionDialogTree]; vscode.commands.registerCommand('azure.resource.startterminal', async (node?: TreeNode) => { try { @@ -35,7 +35,7 @@ export function registerAzureResourceCommands(appContext: AppContext, azureViewT if (node instanceof AzureResourceAccountTreeNode) { azureAccount = node.account; } else { - let accounts = filterAccounts(await azdata.accounts.getAllAccounts(), authLibrary); + let accounts = await azdata.accounts.getAllAccounts(); accounts = accounts.filter(a => a.key.providerId.startsWith('azure')); if (accounts.length === 0) { const signin = localize('azure.signIn', "Sign in"); diff --git a/extensions/azurecore/src/azureResource/tree/connectionDialogTreeProvider.ts b/extensions/azurecore/src/azureResource/tree/connectionDialogTreeProvider.ts index e96f5caa05..cba534582a 100644 --- a/extensions/azurecore/src/azureResource/tree/connectionDialogTreeProvider.ts +++ b/extensions/azurecore/src/azureResource/tree/connectionDialogTreeProvider.ts @@ -13,7 +13,7 @@ import { TreeNode } from '../treeNode'; import { AzureResourceAccountNotSignedInTreeNode } from './accountNotSignedInTreeNode'; import { AzureResourceMessageTreeNode } from '../messageTreeNode'; import { AzureResourceContainerTreeNodeBase } from './baseTreeNodes'; -import { AzureResourceErrorMessageUtil, equals, filterAccounts } from '../utils'; +import { AzureResourceErrorMessageUtil, equals } from '../utils'; import { IAzureResourceTreeChangeHandler } from './treeChangeHandler'; import { Logger } from '../../utils/Logger'; import { AzureAccount } from 'azurecore'; @@ -26,11 +26,10 @@ export class ConnectionDialogTreeProvider implements vscode.TreeDataProvider(); private loadingAccountsPromise: Promise | undefined; - public constructor(private readonly appContext: AppContext, - private readonly authLibrary: string) { + public constructor(private readonly appContext: AppContext) { azdata.accounts.onDidChangeAccounts(async (e: azdata.DidChangeAccountsParams) => { // This event sends it per provider, we need to make sure we get all the azure related accounts - let accounts = filterAccounts(await azdata.accounts.getAllAccounts(), authLibrary); + let accounts = await azdata.accounts.getAllAccounts(); accounts = accounts.filter(a => a.key.providerId.startsWith('azure')); // the onDidChangeAccounts event will trigger in many cases where the accounts didn't actually change // the notifyNodeChanged event triggers a refresh which triggers a getChildren which can trigger this callback @@ -56,11 +55,10 @@ export class ConnectionDialogTreeProvider implements vscode.TreeDataProvider 0) { - let accounts = filterAccounts(this.accounts, this.authLibrary); const accountNodes: FlatAccountTreeNode[] = []; const errorMessages: string[] = []; // We are doing sequential account loading to avoid the Azure request throttling - for (const account of accounts) { + for (const account of this.accounts) { try { const accountNode = new FlatAccountTreeNode(account, this.appContext, this); accountNode.refreshLabel(); @@ -87,7 +85,7 @@ export class ConnectionDialogTreeProvider implements vscode.TreeDataProvider { try { - this.accounts = filterAccounts(await azdata.accounts.getAllAccounts(), this.authLibrary); + this.accounts = await azdata.accounts.getAllAccounts(); // System has been initialized this.setSystemInitialized(); this._onDidChangeTreeData.fire(undefined); diff --git a/extensions/azurecore/src/azureResource/tree/treeProvider.ts b/extensions/azurecore/src/azureResource/tree/treeProvider.ts index 4ce5281213..467e356941 100644 --- a/extensions/azurecore/src/azureResource/tree/treeProvider.ts +++ b/extensions/azurecore/src/azureResource/tree/treeProvider.ts @@ -14,7 +14,7 @@ import { AzureResourceAccountTreeNode } from './accountTreeNode'; import { AzureResourceAccountNotSignedInTreeNode } from './accountNotSignedInTreeNode'; import { AzureResourceMessageTreeNode } from '../messageTreeNode'; import { AzureResourceContainerTreeNodeBase } from './baseTreeNodes'; -import { AzureResourceErrorMessageUtil, equals, filterAccounts } from '../utils'; +import { AzureResourceErrorMessageUtil, equals } from '../utils'; import { IAzureResourceTreeChangeHandler } from './treeChangeHandler'; import { AzureAccount } from 'azurecore'; @@ -25,11 +25,10 @@ export class AzureResourceTreeProvider implements vscode.TreeDataProvider(); private loadingAccountsPromise: Promise | undefined; - public constructor(private readonly appContext: AppContext, - private readonly authLibrary: string) { + public constructor(private readonly appContext: AppContext) { azdata.accounts.onDidChangeAccounts(async (e: azdata.DidChangeAccountsParams) => { // This event sends it per provider, we need to make sure we get all the azure related accounts - let accounts = filterAccounts(await azdata.accounts.getAllAccounts(), authLibrary); + let accounts = await azdata.accounts.getAllAccounts(); accounts = accounts.filter(a => a.key.providerId.startsWith('azure')); // the onDidChangeAccounts event will trigger in many cases where the accounts didn't actually change // the notifyNodeChanged event triggers a refresh which triggers a getChildren which can trigger this callback @@ -59,7 +58,6 @@ export class AzureResourceTreeProvider implements vscode.TreeDataProvider new AzureResourceAccountTreeNode(account, this.appContext, this)); } } else { @@ -72,10 +70,7 @@ export class AzureResourceTreeProvider implements vscode.TreeDataProvider { try { - let accounts = await azdata.accounts.getAllAccounts(); - if (accounts) { - this.accounts = filterAccounts(accounts, this.authLibrary); - } + this.accounts = await azdata.accounts.getAllAccounts(); // System has been initialized this.setSystemInitialized(); this._onDidChangeTreeData.fire(undefined); diff --git a/extensions/azurecore/src/azureResource/utils.ts b/extensions/azurecore/src/azureResource/utils.ts index 27ad2a221c..dd11334691 100644 --- a/extensions/azurecore/src/azureResource/utils.ts +++ b/extensions/azurecore/src/azureResource/utils.ts @@ -674,18 +674,3 @@ export function getProviderMetadataForAccount(account: AzureAccount): AzureAccou return provider.metadata; } - -// Filter accounts based on currently selected Auth Library: -// if the account key is present, filter based on current auth library -// if there is no account key (pre-MSAL account), then it is an ADAL account and -// should be displayed as long as ADAL is the currently selected auth library -export function filterAccounts(accounts: azdata.Account[], authLibrary: string): azdata.Account[] { - let filteredAccounts = accounts.filter(account => { - if (account.key.authLibrary) { - return account.key.authLibrary === authLibrary; - } else { - return authLibrary === Constants.AuthLibrary.ADAL; - } - }); - return filteredAccounts; -} diff --git a/extensions/azurecore/src/constants.ts b/extensions/azurecore/src/constants.ts index 13d371ef8d..9a3b5554b0 100644 --- a/extensions/azurecore/src/constants.ts +++ b/extensions/azurecore/src/constants.ts @@ -9,8 +9,6 @@ export const AccountsSection = 'accounts'; export const AuthSection = 'auth'; -export const AuthenticationLibrarySection = 'authenticationLibrary'; - export const AzureSection = 'azure'; export const AzureAccountProviderCredentials = 'azureAccountProviderCredentials'; @@ -27,8 +25,6 @@ export const AccountsAzureAuthSection = AccountsSection + '.' + AzureSection + ' export const AccountsAzureCloudSection = AccountsSection + '.' + AzureSection + '.' + CloudSection; -export const AzureAuthenticationLibrarySection = AzureSection + '.' + AuthenticationLibrarySection; - export const EnableArcFeaturesSection = 'enableArcFeatures'; export const ServiceName = 'azuredatastudio'; @@ -78,8 +74,6 @@ export const AzureTokenFolderName = 'Azure Accounts'; export const MSALCacheName = 'accessTokenCache'; -export const DefaultAuthLibrary = 'MSAL'; - export const LocalCacheSuffix = '.local'; export const LockFileSuffix = '.lockfile'; @@ -109,14 +103,6 @@ export enum BuiltInCommands { SetContext = 'setContext' } -/** - * AAD Auth library as selected. - */ -export enum AuthLibrary { - MSAL = 'MSAL', - ADAL = 'ADAL' -} - /** * Authentication type as selected. */ diff --git a/extensions/azurecore/src/extension.ts b/extensions/azurecore/src/extension.ts index 2c7019fff5..06fb922401 100644 --- a/extensions/azurecore/src/extension.ts +++ b/extensions/azurecore/src/extension.ts @@ -73,16 +73,6 @@ export async function activate(context: vscode.ExtensionContext): Promise { - if (value === loc.switchMsal) { - await vscode.workspace.getConfiguration(Constants.AzureSection).update(Constants.AuthenticationLibrarySection, Constants.DefaultAuthLibrary, vscode.ConfigurationTarget.Global); - } - }); - } const piiLogging = vscode.workspace.getConfiguration(Constants.AzureSection).get(Constants.piiLogging, false) if (piiLogging) { void vscode.window.showWarningMessage(loc.piiWarning, loc.disable, loc.dismiss).then(async (value) => { @@ -96,18 +86,18 @@ export async function activate(context: vscode.ExtensionContext): Promise; // Create the provider service and activate - let providerService = await initAzureAccountProvider(extensionContext, storagePath, authLibrary!).catch((err) => Logger.error(err)); + let providerService = await initAzureAccountProvider(extensionContext, storagePath).catch((err) => Logger.error(err)); if (providerService) { eventEmitter = providerService.getEncryptionKeysEmitter(); registerAzureServices(appContext); - const azureResourceTree = new AzureResourceTreeProvider(appContext, authLibrary); - const connectionDialogTree = new ConnectionDialogTreeProvider(appContext, authLibrary); + const azureResourceTree = new AzureResourceTreeProvider(appContext); + const connectionDialogTree = new ConnectionDialogTreeProvider(appContext); pushDisposable(vscode.window.registerTreeDataProvider('azureResourceExplorer', azureResourceTree)); pushDisposable(vscode.window.registerTreeDataProvider('connectionDialog/azureResourceExplorer', connectionDialogTree)); pushDisposable(vscode.workspace.onDidChangeConfiguration(e => onDidChangeConfiguration(e))); - registerAzureResourceCommands(appContext, azureResourceTree, connectionDialogTree, authLibrary); - azdata.dataprotocol.registerDataGridProvider(new AzureDataGridProvider(appContext, authLibrary)); + registerAzureResourceCommands(appContext, azureResourceTree, connectionDialogTree); + azdata.dataprotocol.registerDataGridProvider(new AzureDataGridProvider(appContext)); vscode.commands.registerCommand('azure.dataGrid.openInAzurePortal', async (item: azdata.DataGridItem) => { const portalEndpoint = item.portalEndpoint; const subscriptionId = item.subscriptionId; @@ -263,9 +253,9 @@ async function findOrMakeStoragePath() { return storagePath; } -async function initAzureAccountProvider(extensionContext: vscode.ExtensionContext, storagePath: string, authLibrary: string): Promise { +async function initAzureAccountProvider(extensionContext: vscode.ExtensionContext, storagePath: string): Promise { try { - const accountProviderService = new AzureAccountProviderService(extensionContext, storagePath, authLibrary); + const accountProviderService = new AzureAccountProviderService(extensionContext, storagePath); extensionContext.subscriptions.push(accountProviderService); await accountProviderService.activate(); return accountProviderService; @@ -289,12 +279,6 @@ async function onDidChangeConfiguration(e: vscode.ConfigurationChangeEvent): Pro if (e.affectsConfiguration('azure.piiLogging')) { updatePiiLoggingLevel(); } - if (e.affectsConfiguration('azure.authenticationLibrary')) { - if (vscode.workspace.getConfiguration(Constants.AzureSection).get('authenticationLibrary') === 'ADAL') { - void vscode.window.showInformationMessage(loc.deprecatedOption); - } - await utils.displayReloadAds('authenticationLibrary'); - } } function updatePiiLoggingLevel(): void { diff --git a/extensions/azurecore/src/localizedConstants.ts b/extensions/azurecore/src/localizedConstants.ts index be2b14b836..38a5c44bc7 100644 --- a/extensions/azurecore/src/localizedConstants.ts +++ b/extensions/azurecore/src/localizedConstants.ts @@ -69,7 +69,6 @@ export function reloadPrompt(sectionName: string): string { export const reloadPromptCacheClear = localize('azurecore.reloadPromptCacheClear', "Token cache has been cleared successfully, please reload Azure Data Studio."); export const reloadChoice = localize('azurecore.reloadChoice', "Reload Azure Data Studio"); -export const deprecatedOption = localize('azurecore.deprecated', "Warning: ADAL has been deprecated, and is scheduled to be removed in the next release. Please use MSAL instead."); export const piiWarning = localize('azurecore.piiLogging.warning', "Warning: Azure PII Logging is enabled. Enabling this option allows personally identifiable information to be logged and should only be used for debugging purposes."); export const disable = localize('azurecore.disable', 'Disable'); export const dismiss = localize('azurecore.dismiss', 'Dismiss'); diff --git a/extensions/azurecore/src/test/account-provider/auths/azureAuth.test.ts b/extensions/azurecore/src/test/account-provider/auths/azureAuth.test.ts index e26b8f746f..4f79c0f1c6 100644 --- a/extensions/azurecore/src/test/account-provider/auths/azureAuth.test.ts +++ b/extensions/azurecore/src/test/account-provider/auths/azureAuth.test.ts @@ -7,11 +7,11 @@ import * as should from 'should'; import * as TypeMoq from 'typemoq'; import 'mocha'; import { AzureAuthCodeGrant } from '../../../account-provider/auths/azureAuthCodeGrant'; -import { Token, TokenClaims, AccessToken, RefreshToken, OAuthTokenResponse, TokenPostData } from '../../../account-provider/auths/azureAuth'; +import { Token, TokenClaims, AccessToken, RefreshToken } from '../../../account-provider/auths/azureAuth'; import { Tenant, AzureAccount } from 'azurecore'; import providerSettings from '../../../account-provider/providerSettings'; import { AzureResource } from 'azdata'; -import { AxiosResponse } from 'axios'; +import { AuthenticationResult } from '@azure/msal-common'; let azureAuthCodeGrant: TypeMoq.IMock; // let azureDeviceCode: TypeMoq.IMock; @@ -78,208 +78,30 @@ describe('Azure Authentication', function () { }; }); - it('accountHydration should yield a valid account', async function () { - - azureAuthCodeGrant.setup(x => x.getTenantsAdal(mockToken)).returns((): Promise => { - return Promise.resolve([ - mockTenant - ]); - }); - - const response = await azureAuthCodeGrant.object.hydrateAccount(mockToken, mockClaims); - should(response.displayInfo.displayName).be.equal(`${mockClaims.name} - ${mockClaims.email}`, 'Account name should match'); - should(response.displayInfo.userId).be.equal(mockClaims.sub, 'Account ID should match'); - should(response.properties.tenants).be.deepEqual([mockTenant], 'Tenants should match'); - }); - describe('getAccountSecurityToken', function () { - it('should be undefined on stale account', async function () { - mockAccount.isStale = true; - const securityToken = await azureAuthCodeGrant.object.getAccountSecurityTokenAdal(mockAccount, TypeMoq.It.isAny(), TypeMoq.It.isAny()); - should(securityToken).be.undefined(); - }); - it('dont find correct resources', async function () { - const securityToken = await azureAuthCodeGrant.object.getAccountSecurityTokenAdal(mockAccount, TypeMoq.It.isAny(), -1); - should(securityToken).be.undefined(); - }); - it('incorrect tenant', async function () { - await azureAuthCodeGrant.object.getAccountSecurityTokenAdal(mockAccount, 'invalid_tenant', AzureResource.MicrosoftResourceManagement).should.be.rejected(); - }); - - it('token recieved for ossRdbmns resource', async function () { - azureAuthCodeGrant.setup(x => x.getTenantsAdal(mockToken)).returns(() => { - return Promise.resolve([ - mockTenant - ]); - }); - azureAuthCodeGrant.setup(x => x.getTokenHelperAdal(mockTenant, provider.settings.ossRdbmsResource!, TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => { - return Promise.resolve({ - accessToken: mockAccessToken - } as OAuthTokenResponse); - }); - - azureAuthCodeGrant.setup(x => x.refreshTokenAdal(mockTenant, provider.settings.ossRdbmsResource!, mockRefreshToken)).returns((): Promise => { - const mockToken: AccessToken = JSON.parse(JSON.stringify(mockAccessToken)) as AccessToken; - delete (mockToken as any).invalidData; - return Promise.resolve({ - accessToken: mockToken - } as OAuthTokenResponse); - }); - - azureAuthCodeGrant.setup(x => x.getSavedTokenAdal(mockTenant, provider.settings.ossRdbmsResource!, mockAccount.key)).returns((): Promise<{ accessToken: AccessToken, refreshToken: RefreshToken, expiresOn: string }> => { - return Promise.resolve({ - accessToken: mockAccessToken, - refreshToken: mockRefreshToken, - expiresOn: `${(new Date().getTime() / 1000) + (10 * 60)}` - }); - }); - - const securityToken = await azureAuthCodeGrant.object.getAccountSecurityTokenAdal(mockAccount, mockTenant.id, AzureResource.OssRdbms); - should(securityToken?.token).be.equal(mockAccessToken.token, 'Token are not similar'); - - }); - it('saved token exists and can be reused', async function () { delete (mockAccessToken as any).tokenType; - azureAuthCodeGrant.setup(x => x.getSavedTokenAdal(mockTenant, provider.settings.microsoftResource!, mockAccount.key)).returns((): Promise<{ accessToken: AccessToken, refreshToken: RefreshToken, expiresOn: string }> => { + azureAuthCodeGrant.setup(x => x.getToken(mockAccount.key.accountId, AzureResource.MicrosoftResourceManagement, mockTenant.id)).returns((): Promise => { return Promise.resolve({ - accessToken: mockAccessToken, - refreshToken: mockRefreshToken, - expiresOn: `${(new Date().getTime() / 1000) + (10 * 60)}` + authority: 'test', + uniqueId: 'test', + tenantId: 'test', + scopes: ['test'], + account: null, + idToken: 'test', + idTokenClaims: mockClaims, + fromCache: false, + tokenType: 'Bearer', + correlationId: 'test', + accessToken: mockAccessToken.token, + refreshToken: mockRefreshToken.token, + expiresOn: new Date(Date.now()) }); }); - const securityToken = await azureAuthCodeGrant.object.getAccountSecurityTokenAdal(mockAccount, mockTenant.id, AzureResource.MicrosoftResourceManagement); + const securityToken = await azureAuthCodeGrant.object.getToken(mockAccount.key.accountId, AzureResource.MicrosoftResourceManagement, mockTenant.id) as AuthenticationResult; should(securityToken?.tokenType).be.equal('Bearer', 'tokenType should be bearer on a successful getSecurityToken from cache'); }); - - - it('saved token had invalid expiration', async function () { - delete (mockAccessToken as any).tokenType; - (mockAccessToken as any).invalidData = 'this should not exist on response'; - azureAuthCodeGrant.setup(x => x.getSavedTokenAdal(mockTenant, provider.settings.microsoftResource!, mockAccount.key)).returns((): Promise<{ accessToken: AccessToken, refreshToken: RefreshToken, expiresOn: string }> => { - return Promise.resolve({ - accessToken: mockAccessToken, - refreshToken: mockRefreshToken, - expiresOn: 'invalid' - }); - }); - azureAuthCodeGrant.setup(x => x.refreshTokenAdal(mockTenant, provider.settings.microsoftResource!, mockRefreshToken)).returns((): Promise => { - const mockToken: AccessToken = JSON.parse(JSON.stringify(mockAccessToken)) as AccessToken; - delete (mockToken as any).invalidData; - return Promise.resolve({ - accessToken: mockToken - } as OAuthTokenResponse); - }); - const securityToken = await azureAuthCodeGrant.object.getAccountSecurityTokenAdal(mockAccount, mockTenant.id, AzureResource.MicrosoftResourceManagement); - - should((securityToken as any).invalidData).be.undefined(); // Ensure its a new one - should(securityToken?.tokenType).be.equal('Bearer', 'tokenType should be bearer on a successful getSecurityToken from cache'); - - azureAuthCodeGrant.verify(x => x.refreshTokenAdal(mockTenant, provider.settings.microsoftResource!, mockRefreshToken), TypeMoq.Times.once()); - }); - - describe('no saved token', function () { - it('no base token', async function () { - azureAuthCodeGrant.setup(x => x.getSavedTokenAdal(mockTenant, provider.settings.microsoftResource!, mockAccount.key)).returns((): Promise<{ accessToken: AccessToken, refreshToken: RefreshToken, expiresOn: string } | undefined> => { - return Promise.resolve(undefined); - }); - - azureAuthCodeGrant.setup(x => x.getSavedTokenAdal(azureAuthCodeGrant.object.commonTenant, provider.settings.microsoftResource!, mockAccount.key)).returns((): Promise<{ accessToken: AccessToken, refreshToken: RefreshToken, expiresOn: string } | undefined> => { - return Promise.resolve(undefined); - }); - - await azureAuthCodeGrant.object.getAccountSecurityTokenAdal(mockAccount, mockTenant.id, AzureResource.MicrosoftResourceManagement).should.be.rejected(); - }); - - it('base token exists', async function () { - azureAuthCodeGrant.setup(x => x.getSavedTokenAdal(mockTenant, provider.settings.microsoftResource!, mockAccount.key)).returns((): Promise<{ accessToken: AccessToken, refreshToken: RefreshToken, expiresOn: string } | undefined> => { - return Promise.resolve(undefined); - }); - - azureAuthCodeGrant.setup(x => x.getSavedTokenAdal(azureAuthCodeGrant.object.commonTenant, provider.settings.microsoftResource!, mockAccount.key)).returns((): Promise<{ accessToken: AccessToken, refreshToken: RefreshToken, expiresOn: string }> => { - return Promise.resolve({ - accessToken: mockAccessToken, - refreshToken: mockRefreshToken, - expiresOn: '' - }); - }); - delete (mockAccessToken as any).tokenType; - - azureAuthCodeGrant.setup(x => x.refreshTokenAdal(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => { - return Promise.resolve({ - accessToken: mockAccessToken - } as OAuthTokenResponse); - }); - - const securityToken = await azureAuthCodeGrant.object.getAccountSecurityTokenAdal(mockAccount, mockTenant.id, AzureResource.MicrosoftResourceManagement); - should(securityToken?.tokenType).be.equal('Bearer', 'tokenType should be bearer on a successful getSecurityToken from cache'); - }); - }); - - }); - - describe('getToken', function () { - - it('calls handle interaction required', async function () { - azureAuthCodeGrant.setup(x => x.makePostRequest(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => { - return Promise.resolve({ - data: { - error: 'interaction_required' - } - } as AxiosResponse); - }); - - azureAuthCodeGrant.setup(x => x.handleInteractionRequiredAdal(mockTenant, provider.settings.microsoftResource!)).returns(() => { - return Promise.resolve({ - accessToken: mockAccessToken - } as OAuthTokenResponse); - }); - - - const result = await azureAuthCodeGrant.object.getTokenAdal(mockTenant, provider.settings.microsoftResource!, {} as TokenPostData); - - azureAuthCodeGrant.verify(x => x.handleInteractionRequiredAdal(mockTenant, provider.settings.microsoftResource!), TypeMoq.Times.once()); - - should(result?.accessToken).be.deepEqual(mockAccessToken); - }); - - it('unknown error should throw error', async function () { - azureAuthCodeGrant.setup(x => x.makePostRequest(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => { - return Promise.resolve({ - data: { - error: 'unknown error' - } - } as AxiosResponse); - }); - - await azureAuthCodeGrant.object.getTokenAdal(mockTenant, provider.settings.microsoftResource!, {} as TokenPostData).should.be.rejected(); - }); - - it('calls getTokenHelper', async function () { - azureAuthCodeGrant.setup(x => x.makePostRequest(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => { - return Promise.resolve({ - data: { - access_token: mockAccessToken.token, - refresh_token: mockRefreshToken.token, - expires_on: `0` - } - } as AxiosResponse); - }); - - azureAuthCodeGrant.setup(x => x.getTokenHelperAdal(mockTenant, provider.settings.microsoftResource!, TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => { - return Promise.resolve({ - accessToken: mockAccessToken - } as OAuthTokenResponse); - }); - - - const result = await azureAuthCodeGrant.object.getTokenAdal(mockTenant, provider.settings.microsoftResource!, {} as TokenPostData); - - azureAuthCodeGrant.verify(x => x.getTokenHelperAdal(mockTenant, provider.settings.microsoftResource!, TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.once()); - - should(result?.accessToken).be.deepEqual(mockAccessToken); - }); }); }); diff --git a/extensions/azurecore/src/test/account-provider/utils/fileDatabase.test.ts b/extensions/azurecore/src/test/account-provider/utils/fileDatabase.test.ts deleted file mode 100644 index b7d4d3cba3..0000000000 --- a/extensions/azurecore/src/test/account-provider/utils/fileDatabase.test.ts +++ /dev/null @@ -1,99 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the Source EULA. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import * as should from 'should'; -import * as os from 'os'; -import * as path from 'path'; -import * as crypto from 'crypto'; -import { promises as fs } from 'fs'; - -import 'mocha'; - -import { FileDatabase } from '../../../account-provider/utils/fileDatabase'; - -let fileDatabase: FileDatabase; -let fileName: string; - -const k1 = 'k1'; -const v1 = 'v1'; - -const k2 = 'k2'; -const v2 = 'v2'; - -const fakeDB = { - k1: v1, - k2: v2 -}; - -// These tests don't work on Linux systems because gnome-keyring doesn't like running on headless machines. -describe('AccountProvider.FileDatabase', function (): void { - beforeEach(async function (): Promise { - fileName = crypto.randomBytes(4).toString('hex'); - fileDatabase = new FileDatabase(path.join(os.tmpdir(), fileName)); - }); - - it('set, get, and clear', async function (): Promise { - await fileDatabase.initialize(); - await fileDatabase.set(k1, v1); - - let x = fileDatabase.get(k1); - should(x).be.equal(v1); - - await fileDatabase.clear(); - - x = fileDatabase.get(k1); - should(x).be.undefined(); - }); - - it('read the file contents', async function (): Promise { - await fileDatabase.initialize(); - await fileDatabase.set(k1, v1); - - let x = fileDatabase.get(k1); - should(x).be.equal(v1); - - await fileDatabase.shutdown(); - const data = await fs.readFile(path.join(os.tmpdir(), fileName)); - - should(data.toString()).be.equal(JSON.stringify({ k1: v1 })); - }); - - it('delete prefix', async function (): Promise { - await fileDatabase.initialize(); - await Promise.all([fileDatabase.set(k1, v1), fileDatabase.set(k2, v2)]); - - let x = fileDatabase.get(k1); - should(x).be.equal(v1); - - x = fileDatabase.get(k2); - should(x).be.equal(v2); - - await fileDatabase.deletePrefix('k'); - - x = fileDatabase.get(k1); - should(x).be.undefined(); - }); - - it('Test write hook', async function (): Promise { - fileDatabase.setWriteHook(async (contents): Promise => { - should(contents).be.equal(JSON.stringify(fakeDB)); - return contents; - }); - - await fileDatabase.initialize(); - await fileDatabase.set(k1, v1); - await fileDatabase.set(k2, v2); - await fileDatabase.save(); - }); - - it('Test read hook', async function (): Promise { - fileDatabase.setReadHook(async (contents): Promise => { - should(contents).be.equal(JSON.stringify(fakeDB)); - return contents; - }); - await fs.writeFile(path.join(os.tmpdir(), fileName), JSON.stringify(fakeDB)); - await fileDatabase.initialize(); - }); -}); diff --git a/extensions/azurecore/src/test/azureResource/tree/treeProvider.test.ts b/extensions/azurecore/src/test/azureResource/tree/treeProvider.test.ts index 351945ddc1..cd8c0afead 100644 --- a/extensions/azurecore/src/test/azureResource/tree/treeProvider.test.ts +++ b/extensions/azurecore/src/test/azureResource/tree/treeProvider.test.ts @@ -26,42 +26,10 @@ let mockExtensionContext: TypeMoq.IMock; let mockCacheService: TypeMoq.IMock; // Mock test data -const mockAccountAdal1: AzureAccount = { - key: { - accountId: 'mock_account_1', - providerId: 'mock_provider', - authLibrary: 'ADAL' - }, - displayInfo: { - displayName: 'mock_account_1@test.com', - accountType: 'Microsoft', - contextualDisplayName: 'test', - userId: 'test@email.com' - }, - properties: TypeMoq.Mock.ofType().object, - isStale: false -}; -const mockAccountAdal2: AzureAccount = { - key: { - accountId: 'mock_account_2', - providerId: 'mock_provider' - }, - displayInfo: { - displayName: 'mock_account_2@test.com', - accountType: 'Microsoft', - contextualDisplayName: 'test', - userId: 'test@email.com' - }, - properties: TypeMoq.Mock.ofType().object, - isStale: false -}; -const mockAccountsADAL = [mockAccountAdal1, mockAccountAdal2]; - const mockAccountMsal1: AzureAccount = { key: { accountId: 'mock_account_1', - providerId: 'mock_provider', - authLibrary: 'MSAL' + providerId: 'mock_provider' }, displayInfo: { displayName: 'mock_account_1@test.com', @@ -75,8 +43,7 @@ const mockAccountMsal1: AzureAccount = { const mockAccountMsal2: AzureAccount = { key: { accountId: 'mock_account_2', - providerId: 'mock_provider', - authLibrary: 'MSAL' + providerId: 'mock_provider' }, displayInfo: { displayName: 'mock_account_2@test.com', @@ -105,31 +72,10 @@ describe('AzureResourceTreeProvider.getChildren', function (): void { sinon.restore(); }); - it('Should load accounts for ADAL', async function (): Promise { - const getAllAccountsStub = sinon.stub(azdata.accounts, 'getAllAccounts').returns(Promise.resolve(mockAccountsADAL)); - - const treeProvider = new AzureResourceTreeProvider(mockAppContext, 'ADAL'); - - await treeProvider.getChildren(undefined); // Load account promise - const children = await treeProvider.getChildren(undefined); // Actual accounts - - should(getAllAccountsStub.calledOnce).be.true('getAllAccounts should have been called exactly once'); - should(children).Array(); - should(children.length).equal(mockAccountsADAL.length); - - for (let ix = 0; ix < mockAccountsADAL.length; ix++) { - const child = children[ix]; - const account = mockAccountsADAL[ix]; - - should(child).instanceof(AzureResourceAccountTreeNode); - should(child.nodePathValue).equal(`account_${account.key.accountId}`); - } - }); - it('Should load accounts for MSAL', async function (): Promise { const getAllAccountsStub = sinon.stub(azdata.accounts, 'getAllAccounts').returns(Promise.resolve(mockAccountsMSAL)); - const treeProvider = new AzureResourceTreeProvider(mockAppContext, 'MSAL'); + const treeProvider = new AzureResourceTreeProvider(mockAppContext); await treeProvider.getChildren(undefined); // Load account promise const children = await treeProvider.getChildren(undefined); // Actual accounts @@ -147,23 +93,10 @@ describe('AzureResourceTreeProvider.getChildren', function (): void { } }); - it('Should handle when there is no accounts for ADAL', async function (): Promise { - sinon.stub(azdata.accounts, 'getAllAccounts').returns(Promise.resolve([])); - - const treeProvider = new AzureResourceTreeProvider(mockAppContext, 'ADAL'); - treeProvider.isSystemInitialized = true; - - const children = await treeProvider.getChildren(undefined); - - should(children).Array(); - should(children.length).equal(1); - should(children[0]).instanceof(AzureResourceAccountNotSignedInTreeNode); - }); - it('Should handle when there is no accounts for MSAL', async function (): Promise { sinon.stub(azdata.accounts, 'getAllAccounts').returns(Promise.resolve([])); - const treeProvider = new AzureResourceTreeProvider(mockAppContext, 'MSAL'); + const treeProvider = new AzureResourceTreeProvider(mockAppContext); treeProvider.isSystemInitialized = true; const children = await treeProvider.getChildren(undefined); diff --git a/extensions/import/src/common/utils.ts b/extensions/import/src/common/utils.ts index 6ac533e0b1..9b448d6266 100644 --- a/extensions/import/src/common/utils.ts +++ b/extensions/import/src/common/utils.ts @@ -8,35 +8,17 @@ import * as vscode from 'vscode'; const mssqlExtensionConfigName = 'mssql'; const enableSqlAuthenticationProviderConfig = 'enableSqlAuthenticationProvider'; -const azureExtensionConfigName = 'azure'; -const azureAuthenticationLibraryConfig = 'authenticationLibrary'; -const MSAL = 'MSAL'; - /** - * @returns 'True' if MSAL auth library is in use and SQL Auth provider is enabled. + * @returns 'True' if SQL Auth provider is enabled. */ export function isMssqlAuthProviderEnabled(): boolean { - return getAzureAuthenticationLibraryConfig() === MSAL && getEnableSqlAuthenticationProviderConfig(); + return getEnableSqlAuthenticationProviderConfig(); } export function getConfiguration(config: string): vscode.WorkspaceConfiguration { return vscode.workspace.getConfiguration(config); } -/** - * Reads setting 'azure.AuthenticationLibrary' and returns the library name enabled. - * @returns MSAL | ADAL - */ -export function getAzureAuthenticationLibraryConfig(): string { - const config = getConfiguration(azureExtensionConfigName); - if (config) { - return config.get(azureAuthenticationLibraryConfig, MSAL); // default Auth library - } - else { - return MSAL; // default Auth library - } -} - /** * Reads setting 'mssql.enableSqlAuthenticationProvider' and returns true if it's enabled. * @returns True Sql Auth provider is enabled for MSSQL provider. diff --git a/extensions/mssql/src/sqlToolsServer.ts b/extensions/mssql/src/sqlToolsServer.ts index c346772ea0..b4513786c2 100644 --- a/extensions/mssql/src/sqlToolsServer.ts +++ b/extensions/mssql/src/sqlToolsServer.ts @@ -10,7 +10,7 @@ import * as vscode from 'vscode'; import * as azdata from 'azdata'; import * as path from 'path'; import * as azurecore from 'azurecore'; -import { getAzureAuthenticationLibraryConfig, getCommonLaunchArgsAndCleanupOldLogFiles, getConfigTracingLevel, getEnableConnectionPoolingConfig, getEnableSqlAuthenticationProviderConfig, getOrDownloadServer, getParallelMessageProcessingConfig, logDebug, TracingLevel } from './utils'; +import { getCommonLaunchArgsAndCleanupOldLogFiles, getConfigTracingLevel, getEnableConnectionPoolingConfig, getEnableSqlAuthenticationProviderConfig, getOrDownloadServer, getParallelMessageProcessingConfig, logDebug, TracingLevel } from './utils'; import { TelemetryReporter, LanguageClientErrorHandler } from './telemetry'; import { SqlOpsDataClient, ClientOptions } from 'dataprotocol-client'; import { TelemetryFeature, AgentServicesFeature, SerializationFeature, AccountFeature, SqlAssessmentServicesFeature, ProfilerFeature, TableDesignerFeature, ExecutionPlanServiceFeature } from './features'; @@ -99,7 +99,7 @@ export class SqlToolsServer { * @param client SqlOpsDataClient instance */ private async handleEncryptionKeyEventNotification(client: SqlOpsDataClient) { - if (getAzureAuthenticationLibraryConfig() === 'MSAL' && getEnableSqlAuthenticationProviderConfig()) { + if (getEnableSqlAuthenticationProviderConfig()) { let azureCoreApi = await this.getAzureCoreAPI(); let onDidEncryptionKeysChanged = azureCoreApi.onEncryptionKeysUpdated; // Register event listener from Azure Core extension and @@ -163,8 +163,7 @@ function generateServerOptions(logPath: string, executablePath: string): ServerO launchArgs.push('--parallel-message-processing'); } const enableSqlAuthenticationProvider = getEnableSqlAuthenticationProviderConfig(); - const azureAuthLibrary = getAzureAuthenticationLibraryConfig(); - if (azureAuthLibrary === 'MSAL' && enableSqlAuthenticationProvider === true) { + if (enableSqlAuthenticationProvider === true) { launchArgs.push('--enable-sql-authentication-provider'); } const enableConnectionPooling = getEnableConnectionPoolingConfig() diff --git a/extensions/mssql/src/utils.ts b/extensions/mssql/src/utils.ts index 9b7ecac989..b04cfd0a08 100644 --- a/extensions/mssql/src/utils.ts +++ b/extensions/mssql/src/utils.ts @@ -23,8 +23,6 @@ const enableSqlAuthenticationProviderConfig = 'enableSqlAuthenticationProvider'; const enableConnectionPoolingConfig = 'enableConnectionPooling'; const tableDesignerPreloadConfig = 'tableDesigner.preloadDatabaseModel'; -const azureExtensionConfigName = 'azure'; -const azureAuthenticationLibraryConfig = 'authenticationLibrary'; /** * * @returns Whether the current OS is linux or not @@ -68,16 +66,6 @@ export function removeOldLogFiles(logPath: string, prefix: string): JSON { export function getConfiguration(config: string = extensionConfigSectionName): vscode.WorkspaceConfiguration { return vscode.workspace.getConfiguration(config); } -/** - * We need Azure core extension configuration for fetching Authentication Library setting in use. - * This is required for 'enableSqlAuthenticationProvider' to be enabled (as it applies to MSAL only). - * This can be removed in future when ADAL support is dropped. - * @param config Azure core extension configuration section name - * @returns Azure core extension config section - */ -export function getAzureCoreExtConfiguration(config: string = azureExtensionConfigName): vscode.WorkspaceConfiguration { - return vscode.workspace.getConfiguration(config); -} export function getConfigLogFilesRemovalLimit(): number | undefined { let config = getConfiguration(); @@ -157,16 +145,6 @@ export function getParallelMessageProcessingConfig(): boolean { return (azdata.env.quality === azdata.env.AppQuality.dev && setting?.globalValue === undefined && setting?.workspaceValue === undefined) ? true : config[parallelMessageProcessingConfig]; } -export function getAzureAuthenticationLibraryConfig(): string { - const config = getAzureCoreExtConfiguration(); - if (config) { - return config.get(azureAuthenticationLibraryConfig, 'MSAL'); // default Auth library - } - else { - return 'MSAL'; - } -} - /** * Retrieves configuration `mssql:enableSqlAuthenticationProvider` from settings file. * @returns true if setting is enabled in ADS, false otherwise. diff --git a/extensions/types/vscode-mssql.d.ts b/extensions/types/vscode-mssql.d.ts index d31f0987d0..4f04500fee 100644 --- a/extensions/types/vscode-mssql.d.ts +++ b/extensions/types/vscode-mssql.d.ts @@ -801,11 +801,6 @@ declare module 'vscode-mssql' { accountVersion?: any; } - export enum AuthLibrary { - ADAL = 'ADAL', - MSAL = 'MSAL' - } - export enum AzureAuthType { AuthCodeGrant = 0, DeviceCode = 1 diff --git a/src/sql/azdata.proposed.d.ts b/src/sql/azdata.proposed.d.ts index 6c8960c261..6a122bd7f2 100644 --- a/src/sql/azdata.proposed.d.ts +++ b/src/sql/azdata.proposed.d.ts @@ -661,13 +661,6 @@ declare module 'azdata' { type?: ExtensionNodeType; } - export interface AccountKey { - /** - * Auth Library used to add the account - */ - authLibrary?: string; - } - export namespace workspace { /** * Creates and enters a workspace at the specified location diff --git a/src/sql/platform/connection/common/constants.ts b/src/sql/platform/connection/common/constants.ts index 12b419731a..6ce872631b 100644 --- a/src/sql/platform/connection/common/constants.ts +++ b/src/sql/platform/connection/common/constants.ts @@ -26,8 +26,6 @@ export const passwordChars = '***************'; export const enableSqlAuthenticationProviderConfig = 'mssql.enableSqlAuthenticationProvider'; -/** Configuration for Azure Authentication Library */ -export const azureAuthenticationLibraryConfig = 'azure.authenticationLibrary'; /* default authentication type setting name*/ export const defaultAuthenticationType = 'defaultAuthenticationType'; diff --git a/src/sql/platform/telemetry/common/telemetryKeys.ts b/src/sql/platform/telemetry/common/telemetryKeys.ts index 8b704bb30c..f9778e17b9 100644 --- a/src/sql/platform/telemetry/common/telemetryKeys.ts +++ b/src/sql/platform/telemetry/common/telemetryKeys.ts @@ -143,6 +143,5 @@ export const enum NbTelemetryAction { export const enum TelemetryPropertyName { ChartMaxRowCountExceeded = 'chartMaxRowCountExceeded', ConnectionSource = 'connectionSource', - AuthLibrary = 'AuthLibrary' } diff --git a/src/sql/workbench/services/accountManagement/browser/accountDialog.ts b/src/sql/workbench/services/accountManagement/browser/accountDialog.ts index 0022bd3bd6..8de46b33fd 100644 --- a/src/sql/workbench/services/accountManagement/browser/accountDialog.ts +++ b/src/sql/workbench/services/accountManagement/browser/accountDialog.ts @@ -47,7 +47,6 @@ import { Iterable } from 'vs/base/common/iterator'; import { LoadingSpinner } from 'sql/base/browser/ui/loadingSpinner/loadingSpinner'; import { Tenant, TenantListDelegate, TenantListRenderer } from 'sql/workbench/services/accountManagement/browser/tenantListRenderer'; import { IAccountManagementService } from 'sql/platform/accounts/common/interfaces'; -import { ADAL_AUTH_LIBRARY, AuthLibrary, getAuthLibrary } from 'sql/workbench/services/accountManagement/common/utils'; import { defaultButtonStyles } from 'vs/platform/theme/browser/defaultStyles'; export const VIEWLET_ID = 'workbench.view.accountpanel'; @@ -389,14 +388,9 @@ export class AccountDialog extends Modal { this._splitView!.layout(DOM.getContentHeight(this._container!)); // Set the initial items of the list - const authLibrary: AuthLibrary = getAuthLibrary(this._configurationService); - let updatedAccounts: azdata.Account[]; - if (authLibrary) { - updatedAccounts = filterAccounts(newProvider.initialAccounts, authLibrary); - } - providerView.updateAccounts(updatedAccounts); + providerView.updateAccounts(newProvider.initialAccounts); - if ((updatedAccounts.length > 0 && this._splitViewContainer!.hidden) || this._providerViewsMap.size > 1) { + if ((newProvider.initialAccounts.length > 0 && this._splitViewContainer!.hidden) || this._providerViewsMap.size > 1) { this.showSplitView(); } @@ -440,12 +434,7 @@ export class AccountDialog extends Modal { if (!providerMapping || !providerMapping.view) { return; } - const authLibrary: AuthLibrary = getAuthLibrary(this._configurationService); - let updatedAccounts: azdata.Account[]; - if (authLibrary) { - updatedAccounts = filterAccounts(args.accountList, authLibrary); - } - providerMapping.view.updateAccounts(updatedAccounts); + providerMapping.view.updateAccounts(args.accountList); if ((args.accountList.length > 0 && this._splitViewContainer!.hidden) || this._providerViewsMap.size > 1) { this.showSplitView(); @@ -518,18 +507,3 @@ export class AccountDialog extends Modal { v.addAccountAction.run(); } } - -// Filter accounts based on currently selected Auth Library: -// if the account key is present, filter based on current auth library -// if there is no account key (pre-MSAL account), then it is an ADAL account and -// should be displayed as long as ADAL is the currently selected auth library -export function filterAccounts(accounts: azdata.Account[], authLibrary: AuthLibrary): azdata.Account[] { - let filteredAccounts = accounts.filter(account => { - if (account.key.authLibrary) { - return account.key.authLibrary === authLibrary; - } else { - return authLibrary === ADAL_AUTH_LIBRARY; - } - }); - return filteredAccounts; -} diff --git a/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts b/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts index 988c36bdf8..41f538a22f 100644 --- a/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts +++ b/src/sql/workbench/services/accountManagement/browser/accountManagementService.ts @@ -25,11 +25,8 @@ import { ILogService } from 'vs/platform/log/common/log'; import { INotificationService, Severity, INotification } from 'vs/platform/notification/common/notification'; import { Action } from 'vs/base/common/actions'; import { DisposableStore } from 'vs/base/common/lifecycle'; -import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; -import { filterAccounts } from 'sql/workbench/services/accountManagement/browser/accountDialog'; -import { ADAL_AUTH_LIBRARY, MSAL_AUTH_LIBRARY, AuthLibrary, AZURE_AUTH_LIBRARY_CONFIG, getAuthLibrary } from 'sql/workbench/services/accountManagement/common/utils'; import { IAdsTelemetryService } from 'sql/platform/telemetry/common/telemetry'; -import { TelemetryAction, TelemetryError, TelemetryPropertyName, TelemetryView } from 'sql/platform/telemetry/common/telemetryKeys'; +import { TelemetryAction, TelemetryError, TelemetryView } from 'sql/platform/telemetry/common/telemetryKeys'; export class AccountManagementService implements IAccountManagementService { // CONSTANTS /////////////////////////////////////////////////////////// @@ -39,7 +36,6 @@ export class AccountManagementService implements IAccountManagementService { public _providers: { [id: string]: AccountProviderWithMetadata } = {}; public _serviceBrand: undefined; private _accountStore: AccountStore; - private _authLibrary: AuthLibrary; private _accountDialogController?: AccountDialogController; private _autoOAuthDialogController?: AutoOAuthDialogController; private _mementoContext?: Memento; @@ -63,7 +59,6 @@ export class AccountManagementService implements IAccountManagementService { @IOpenerService private _openerService: IOpenerService, @ILogService private readonly _logService: ILogService, @INotificationService private readonly _notificationService: INotificationService, - @IConfigurationService private _configurationService: IConfigurationService, @IAdsTelemetryService private _telemetryService: IAdsTelemetryService ) { this._mementoContext = new Memento(AccountManagementService.ACCOUNT_MEMENTO, this._storageService); @@ -75,12 +70,7 @@ export class AccountManagementService implements IAccountManagementService { this._removeAccountProviderEmitter = new Emitter(); this._updateAccountListEmitter = new Emitter(); - // Determine authentication library in use, to support filtering accounts respectively. - // When this value is changed a restart is required so there isn't a need to dynamically update this value at runtime. - this._authLibrary = getAuthLibrary(this._configurationService); - _storageService.onWillSaveState(() => this.shutdown()); - this.registerListeners(); } private get autoOAuthDialogController(): AutoOAuthDialogController { @@ -152,9 +142,6 @@ export class AccountManagementService implements IAccountManagementService { } else { this._telemetryService.createErrorEvent(TelemetryView.LinkedAccounts, TelemetryError.AddAzureAccountError, accountResult.errorCode, this.getErrorType(accountResult.errorMessage)) - .withAdditionalProperties({ - [TelemetryPropertyName.AuthLibrary]: this._authLibrary - }) .send(); if (accountResult.errorCode && accountResult.errorMessage) { throw new Error(localize('addAccountFailedCodeMessage', `{0} \nError Message: {1}`, accountResult.errorCode, accountResult.errorMessage)); @@ -168,9 +155,6 @@ export class AccountManagementService implements IAccountManagementService { this._logService.error('Adding account failed, no result received.'); this._telemetryService.createErrorEvent(TelemetryView.LinkedAccounts, TelemetryError.AddAzureAccountErrorNoResult, '-1', this.getErrorType()) - .withAdditionalProperties({ - [TelemetryPropertyName.AuthLibrary]: this._authLibrary - }) .send(); throw new Error(genericAccountErrorMessage); } @@ -182,9 +166,6 @@ export class AccountManagementService implements IAccountManagementService { this.spliceModifiedAccount(provider, result.changedAccount); } this._telemetryService.createActionEvent(TelemetryView.LinkedAccounts, TelemetryAction.AddAzureAccount) - .withAdditionalProperties({ - [TelemetryPropertyName.AuthLibrary]: this._authLibrary - }) .send(); this.fireAccountListUpdate(provider, result.accountAdded); } finally { @@ -235,9 +216,6 @@ export class AccountManagementService implements IAccountManagementService { } else { this._telemetryService.createErrorEvent(TelemetryView.LinkedAccounts, TelemetryError.RefreshAzureAccountError, refreshedAccount.errorCode, this.getErrorType(refreshedAccount.errorMessage)) - .withAdditionalProperties({ - [TelemetryPropertyName.AuthLibrary]: this._authLibrary - }) .send(); if (refreshedAccount.errorCode && refreshedAccount.errorMessage) { throw new Error(localize('refreshFailed', `{0} \nError Message: {1}`, refreshedAccount.errorCode, refreshedAccount.errorMessage)); @@ -254,9 +232,6 @@ export class AccountManagementService implements IAccountManagementService { this._logService.error('Refreshing account failed, no result received.'); this._telemetryService.createErrorEvent(TelemetryView.LinkedAccounts, TelemetryError.RefreshAzureAccountErrorNoResult, '-1', this.getErrorType()) - .withAdditionalProperties({ - [TelemetryPropertyName.AuthLibrary]: this._authLibrary - }) .send(); throw new Error(genericAccountErrorMessage); } @@ -281,9 +256,6 @@ export class AccountManagementService implements IAccountManagementService { } this._telemetryService.createActionEvent(TelemetryView.LinkedAccounts, TelemetryAction.RefreshAzureAccount) - .withAdditionalProperties({ - [TelemetryPropertyName.AuthLibrary]: this._authLibrary - }) .send(); this.fireAccountListUpdate(provider, result.accountAdded); return result.changedAccount!; @@ -325,7 +297,6 @@ export class AccountManagementService implements IAccountManagementService { // 3) Update our local cache of accounts return this.doWithProvider(providerId, provider => { return self._accountStore.getAccountsByProvider(provider.metadata.id) - .then(accounts => this._authLibrary ? filterAccounts(accounts, this._authLibrary) : accounts) .then(accounts => { self._providers[providerId].accounts = accounts; return accounts; @@ -337,8 +308,7 @@ export class AccountManagementService implements IAccountManagementService { * Retrieves all the accounts registered with ADS based on auth library in use. */ public getAccounts(): Promise { - return this._accountStore.getAllAccounts() - .then(accounts => this._authLibrary ? filterAccounts(accounts, this._authLibrary) : accounts); + return this._accountStore.getAllAccounts(); } /** @@ -575,15 +545,10 @@ export class AccountManagementService implements IAccountManagementService { }); } - const authLibrary: AuthLibrary = getAuthLibrary(this._configurationService) - let updatedAccounts: azdata.Account[] - if (authLibrary) { - updatedAccounts = filterAccounts(provider.accounts, authLibrary); - } // Step 2) Fire the event let eventArg: UpdateAccountListEventParams = { providerId: provider.metadata.id, - accountList: updatedAccounts ?? provider.accounts + accountList: provider.accounts }; this._updateAccountListEmitter.fire(eventArg); } @@ -598,62 +563,6 @@ export class AccountManagementService implements IAccountManagementService { } } - private registerListeners(): void { - this.disposables.add(this._configurationService.onDidChangeConfiguration(async e => { - if (e.affectsConfiguration(AZURE_AUTH_LIBRARY_CONFIG)) { - const authLibrary: AuthLibrary = getAuthLibrary(this._configurationService); - let accounts = await this._accountStore.getAllAccounts(); - if (accounts) { - let updatedAccounts = await this.filterAndMergeAccounts(accounts, authLibrary); - let eventArg: UpdateAccountListEventParams; - if (updatedAccounts.length > 0) { - updatedAccounts.forEach(account => { - if (account.key.authLibrary === MSAL_AUTH_LIBRARY) { - account.isStale = false; - } - }); - eventArg = { - providerId: updatedAccounts[0].key.providerId, - accountList: updatedAccounts - }; - } else { // default to public cloud if no accounts - eventArg = { - providerId: 'azure_publicCloud', - accountList: updatedAccounts - }; - } - this._updateAccountListEmitter.fire(eventArg); - } - } - })); - } - - // Filters and merges accounts from both authentication libraries - private async filterAndMergeAccounts(accounts: azdata.Account[], currentAuthLibrary: AuthLibrary): Promise { - // Fetch accounts for alternate authenticationLibrary - const altLibrary = currentAuthLibrary === MSAL_AUTH_LIBRARY ? ADAL_AUTH_LIBRARY : MSAL_AUTH_LIBRARY; - const altLibraryAccounts = filterAccounts(accounts, altLibrary); - - // Fetch accounts for current authenticationLibrary - const currentLibraryAccounts = filterAccounts(accounts, currentAuthLibrary); - - // In the list of alternate accounts, check if the accounts are present in the current library cache, - // if not, add the account and mark it stale. The original account is marked as taken so its not picked again. - for (let account of altLibraryAccounts) { - await this.removeAccount(account.key); - if (this.findAccountIndex(currentLibraryAccounts, account) >= 0) { - continue; - } else { - // TODO: Refresh access token for the account if feasible. - account.isStale = true; - account.key.authLibrary = currentAuthLibrary; - currentLibraryAccounts.push(account); - await this.addAccountWithoutPrompt(account); - } - } - return currentLibraryAccounts; - } - public findAccountIndex(accounts: azdata.Account[], accountToFind: azdata.Account): number { let indexToRemove: number = accounts.findIndex(account => { // corner case handling for personal accounts @@ -664,10 +573,6 @@ export class AccountManagementService implements IAccountManagementService { if (accountToFind.key.accountId.includes('.')) { return account.key.accountId === accountToFind!.key.accountId.split('.')[0]; } - // ADAL account added - if (account.key.accountId.includes('.')) { - return account.key.accountId.split('.')[0] === accountToFind!.key.accountId; - } return account.key.accountId === accountToFind!.key.accountId; }); return indexToRemove; diff --git a/src/sql/workbench/services/accountManagement/common/utils.ts b/src/sql/workbench/services/accountManagement/common/utils.ts deleted file mode 100644 index f6f09b1f0e..0000000000 --- a/src/sql/workbench/services/accountManagement/common/utils.ts +++ /dev/null @@ -1,18 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the Source EULA. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; - -export const AZURE_AUTH_LIBRARY_CONFIG = 'azure.authenticationLibrary'; - -export type AuthLibrary = 'ADAL' | 'MSAL'; -export const MSAL_AUTH_LIBRARY: AuthLibrary = 'MSAL'; -export const ADAL_AUTH_LIBRARY: AuthLibrary = 'ADAL'; - -export const DEFAULT_AUTH_LIBRARY: AuthLibrary = MSAL_AUTH_LIBRARY; - -export function getAuthLibrary(configurationService: IConfigurationService): AuthLibrary { - return configurationService.getValue(AZURE_AUTH_LIBRARY_CONFIG) || DEFAULT_AUTH_LIBRARY; -} diff --git a/src/sql/workbench/services/accountManagement/test/browser/accountManagementService.test.ts b/src/sql/workbench/services/accountManagement/test/browser/accountManagementService.test.ts index 852ca2755e..b054a7e63d 100644 --- a/src/sql/workbench/services/accountManagement/test/browser/accountManagementService.test.ts +++ b/src/sql/workbench/services/accountManagement/test/browser/accountManagementService.test.ts @@ -18,7 +18,6 @@ import { EventVerifierSingle } from 'sql/base/test/common/event'; import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService'; import { AccountDialog } from 'sql/workbench/services/accountManagement/browser/accountDialog'; import { Emitter } from 'vs/base/common/event'; -import { TestConfigurationService } from 'sql/platform/connection/test/common/testConfigurationService'; import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService'; // SUITE CONSTANTS ///////////////////////////////////////////////////////// @@ -34,8 +33,7 @@ const noAccountProvider: azdata.AccountProviderMetadata = { const account: azdata.Account = { key: { providerId: hasAccountProvider.id, - accountId: 'testAccount1', - authLibrary: 'MSAL' + accountId: 'testAccount1' }, displayInfo: { displayName: 'Test Account 1', @@ -538,12 +536,11 @@ function getTestState(): AccountManagementState { .returns(() => mockAccountStore.object); const testNotificationService = new TestNotificationService(); - const testConfigurationService = new TestConfigurationService(); const mockTelemetryService = new NullAdsTelemetryService(); // Create the account management service let ams = new AccountManagementService(mockInstantiationService.object, new TestStorageService(), - undefined, undefined, undefined, testNotificationService, testConfigurationService, mockTelemetryService); + undefined, undefined, undefined, testNotificationService, mockTelemetryService); // Wire up event handlers let evUpdate = new EventVerifierSingle(); diff --git a/src/sql/workbench/services/connection/browser/connectionManagementService.ts b/src/sql/workbench/services/connection/browser/connectionManagementService.ts index 59dd9e7d26..0b3a02ccd4 100644 --- a/src/sql/workbench/services/connection/browser/connectionManagementService.ts +++ b/src/sql/workbench/services/connection/browser/connectionManagementService.ts @@ -1062,9 +1062,7 @@ export class ConnectionManagementService extends Disposable implements IConnecti const azureAccounts = accounts.filter(a => a.key.providerId.startsWith('azure')); if (azureAccounts && azureAccounts.length > 0) { let accountId = (connection.authenticationType === Constants.AuthenticationType.AzureMFA || connection.authenticationType === Constants.AuthenticationType.AzureMFAAndUser) ? connection.azureAccount : connection.userName; - // For backwards compatibility with ADAL, we need to check if the account ID matches with tenant Id or just the account ID - // The OR case can be removed once we no longer support ADAL - let account = azureAccounts.find(account => account.key.accountId === accountId || account.key.accountId.split('.')[0] === accountId); + let account = azureAccounts.find(account => account.key.accountId === accountId); if (account) { this._logService.debug(`Getting security token for Azure account ${account.key.accountId}`); if (account.isStale) { diff --git a/src/sql/workbench/services/connection/browser/connectionWidget.ts b/src/sql/workbench/services/connection/browser/connectionWidget.ts index ad93348f99..ba3ddf01e2 100644 --- a/src/sql/workbench/services/connection/browser/connectionWidget.ts +++ b/src/sql/workbench/services/connection/browser/connectionWidget.ts @@ -35,11 +35,9 @@ import Severity from 'vs/base/common/severity'; import { ConnectionStringOptions } from 'sql/platform/capabilities/common/capabilitiesService'; import { isFalsyOrWhitespace } from 'vs/base/common/strings'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; -import { filterAccounts } from 'sql/workbench/services/accountManagement/browser/accountDialog'; import { AuthenticationType, Actions, mssqlApplicationNameOption, applicationName, mssqlProviderName, mssqlCmsProviderName } from 'sql/platform/connection/common/constants'; import { AdsWidget } from 'sql/base/browser/ui/adsWidget'; import { createCSSRule } from 'vs/base/browser/dom'; -import { AuthLibrary, getAuthLibrary } from 'sql/workbench/services/accountManagement/common/utils'; import { adjustForMssqlAppName } from 'sql/platform/connection/common/utils'; import { isMssqlAuthProviderEnabled } from 'sql/workbench/services/connection/browser/utils'; import { RequiredIndicatorClassName } from 'sql/base/browser/ui/label/label'; @@ -711,11 +709,7 @@ export class ConnectionWidget extends lifecycle.Disposable { private async fillInAzureAccountOptions(): Promise { let oldSelection = this._azureAccountDropdown.value; const accounts = await this._accountManagementService.getAccounts(); - const updatedAccounts = accounts.filter(a => a.key.providerId.startsWith('azure')); - const authLibrary: AuthLibrary = getAuthLibrary(this._configurationService); - if (authLibrary) { - this._azureAccountList = filterAccounts(updatedAccounts, authLibrary); - } + this._azureAccountList = accounts.filter(a => a.key.providerId.startsWith('azure')); let accountDropdownOptions: SelectOptionItemSQL[] = this._azureAccountList.map(account => { return { @@ -734,10 +728,7 @@ export class ConnectionWidget extends lifecycle.Disposable { } private updateRefreshCredentialsLink(): void { - // For backwards compatibility with ADAL, we need to check if the account ID matches with tenant Id or just the account ID - // The OR case can be removed once we no longer support ADAL - let chosenAccount = this._azureAccountList.find(account => account.key.accountId === this._azureAccountDropdown.value - || account.key.accountId.split('.')[0] === this._azureAccountDropdown.value); + let chosenAccount = this._azureAccountList.find(account => account.key.accountId === this._azureAccountDropdown.value); if (chosenAccount && chosenAccount.isStale) { this._tableContainer.classList.remove('hide-refresh-link'); } else { @@ -758,10 +749,7 @@ export class ConnectionWidget extends lifecycle.Disposable { await this.fillInAzureAccountOptions(); // If a new account was added find it and select it, otherwise select the first account - // For backwards compatibility with ADAL, we need to check if the account ID matches with tenant Id or just the account ID - // The OR case can be removed once we no longer support ADAL - let newAccount = this._azureAccountList.find(option => !oldAccountIds.some(oldId => oldId === option.key.accountId - || oldId.split('.')[0] === option.key.accountId)); + let newAccount = this._azureAccountList.find(option => !oldAccountIds.some(oldId => oldId === option.key.accountId)); if (newAccount) { this._azureAccountDropdown.selectWithOptionName(newAccount.key.accountId); } else { @@ -773,10 +761,7 @@ export class ConnectionWidget extends lifecycle.Disposable { // Display the tenant select box if needed const hideTenantsClassName = 'hide-azure-tenants'; - // For backwards compatibility with ADAL, we need to check if the account ID matches with tenant Id or just the account ID - // The OR case can be removed once we no longer support ADAL - let selectedAccount = this._azureAccountList.find(account => account.key.accountId === this._azureAccountDropdown.value - || account.key.accountId.split('.')[0] === this._azureAccountDropdown.value); + let selectedAccount = this._azureAccountList.find(account => account.key.accountId === this._azureAccountDropdown.value); if (!selectedAccount && selectFirstByDefault && this._azureAccountList.length > 0) { selectedAccount = this._azureAccountList[0]; } @@ -967,10 +952,7 @@ export class ConnectionWidget extends lifecycle.Disposable { ? connectionInfo.azureAccount : connectionInfo.userName; let account: azdata.Account; if (accountName) { - // For backwards compatibility with ADAL, we need to check if the account ID matches with tenant Id or just the account ID - // The OR case can be removed once we no longer support ADAL - account = this._azureAccountList?.find(account => account.key.accountId === this.getModelValue(accountName) - || account.key.accountId.split('.')[0] === this.getModelValue(accountName)); + account = this._azureAccountList?.find(account => account.key.accountId === this.getModelValue(accountName)); if (account) { if (!account.properties.tenants?.find(tenant => tenant.id === this._azureTenantId)) { this._azureTenantId = account.properties.tenants[0].id; diff --git a/src/sql/workbench/services/connection/browser/utils.ts b/src/sql/workbench/services/connection/browser/utils.ts index 4e988f566b..c857d88874 100644 --- a/src/sql/workbench/services/connection/browser/utils.ts +++ b/src/sql/workbench/services/connection/browser/utils.ts @@ -4,8 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; -import { azureAuthenticationLibraryConfig, enableSqlAuthenticationProviderConfig, mssqlProviderName } from 'sql/platform/connection/common/constants'; -import { MSAL_AUTH_LIBRARY } from 'sql/workbench/services/accountManagement/common/utils'; +import { enableSqlAuthenticationProviderConfig, mssqlProviderName } from 'sql/platform/connection/common/constants'; /** * Reads setting 'mssql.enableSqlAuthenticationProvider' returns true if it's enabled. @@ -15,16 +14,5 @@ import { MSAL_AUTH_LIBRARY } from 'sql/workbench/services/accountManagement/comm * @returns True if provider is MSSQL and Sql Auth provider is enabled. */ export function isMssqlAuthProviderEnabled(provider: string, configService: IConfigurationService | undefined): boolean { - return provider === mssqlProviderName && isMSALAuthLibraryEnabled(configService) && (configService?.getValue(enableSqlAuthenticationProviderConfig) ?? true); -} - -/** - * We need Azure core extension configuration for fetching Authentication Library setting in use. - * This is required for 'enableSqlAuthenticationProvider' to be enabled (as it applies to MSAL only). - * This can be removed in future when ADAL support is dropped. - * @param configService Configuration Service to use. - * @returns true if MSAL_AUTH_LIBRARY is enabled. - */ -export function isMSALAuthLibraryEnabled(configService: IConfigurationService | undefined): boolean { - return configService?.getValue(azureAuthenticationLibraryConfig) === MSAL_AUTH_LIBRARY /*default*/ ?? true; + return provider === mssqlProviderName && (configService?.getValue(enableSqlAuthenticationProviderConfig) ?? true); }