From ff979f90d11f6e646203683a5d4b02732ad79e68 Mon Sep 17 00:00:00 2001 From: Amir Omidi Date: Mon, 13 Jul 2020 13:05:00 -0700 Subject: [PATCH] Optional PII logging (#11322) * Start with logger * allow user to enable PII logging * prefix the log --- extensions/azurecore/package.json | 5 +++ extensions/azurecore/package.nls.json | 3 +- .../src/account-provider/auths/azureAuth.ts | 35 ++++++++++-------- .../auths/azureAuthCodeGrant.ts | 15 ++++---- .../account-provider/azureAccountProvider.ts | 5 +-- extensions/azurecore/src/extension.ts | 12 +++++++ extensions/azurecore/src/utils/Logger.ts | 36 +++++++++++++++++++ 7 files changed, 86 insertions(+), 25 deletions(-) create mode 100644 extensions/azurecore/src/utils/Logger.ts diff --git a/extensions/azurecore/package.json b/extensions/azurecore/package.json index 8a0118043f..1a6b101ec0 100644 --- a/extensions/azurecore/package.json +++ b/extensions/azurecore/package.json @@ -92,6 +92,11 @@ "default": true, "description": "%config.noSystemKeychain%", "when": "isLinux || isWeb" + }, + "azure.piiLogging": { + "type": "boolean", + "default": false, + "description": "%config.piiLogging%" } } } diff --git a/extensions/azurecore/package.nls.json b/extensions/azurecore/package.nls.json index 291ff7181f..bcf12a4286 100644 --- a/extensions/azurecore/package.nls.json +++ b/extensions/azurecore/package.nls.json @@ -28,5 +28,6 @@ "config.azureCodeGrantMethod": "Code Grant Method", "config.azureDeviceCodeMethod": "Device Code Method", "config.noSystemKeychain": "Disable system keychain integration. Credentials will be stored in a flat file in the user's home directory.", - "config.enableArcFeatures": "Should features related to Azure Arc be enabled (preview)" + "config.enableArcFeatures": "Should features related to Azure Arc be enabled (preview)", + "config.piiLogging": "Should Personally Identifiable Information (PII) be logged in the console view locally" } diff --git a/extensions/azurecore/src/account-provider/auths/azureAuth.ts b/extensions/azurecore/src/account-provider/auths/azureAuth.ts index 2c3e565c9a..22bfd555d2 100644 --- a/extensions/azurecore/src/account-provider/auths/azureAuth.ts +++ b/extensions/azurecore/src/account-provider/auths/azureAuth.ts @@ -22,6 +22,7 @@ import { import { SimpleTokenCache } from '../simpleTokenCache'; import { MemoryDatabase } from '../utils/memoryDatabase'; +import { Logger } from '../../utils/Logger'; const localize = nls.loadMessageBundle(); export interface AccountKey { @@ -175,7 +176,7 @@ export abstract class AzureAuth implements vscode.Disposable { if (ex.message) { await vscode.window.showErrorMessage(ex.message); } - console.log(ex); + Logger.error(ex); } return oldAccount; } @@ -183,7 +184,7 @@ export abstract class AzureAuth implements vscode.Disposable { public async getSecurityToken(account: azdata.Account, azureResource: azdata.AzureResource): Promise { if (account.isStale === true) { - console.log('Account was stale, no tokens being fetched'); + Logger.log('Account was stale, no tokens being fetched'); return undefined; } @@ -212,7 +213,7 @@ export abstract class AzureAuth implements vscode.Disposable { } else { // No expiration date, assume expired. cachedTokens = undefined; - console.info('Assuming expired token due to no expiration date - this is expected on first launch.'); + Logger.log('Assuming expired token due to no expiration date - this is expected on first launch.'); } } @@ -223,22 +224,22 @@ export abstract class AzureAuth implements vscode.Disposable { const baseToken = await this.getCachedToken(account.key); if (!baseToken) { account.isStale = true; - console.log('Base token was empty, account is stale.'); + Logger.log('Base token was empty, account is stale.'); return undefined; } try { await this.refreshAccessToken(account.key, baseToken.refreshToken, tenant, resource); } catch (ex) { - console.log(`Could not refresh access token for ${JSON.stringify(tenant)} - silently removing the tenant from the user's account.`); - console.log(`Actual error: ${JSON.stringify(ex?.response?.data ?? ex.message ?? ex, undefined, 2)}`); + Logger.log(`Could not refresh access token for ${JSON.stringify(tenant)} - silently removing the tenant from the user's account.`); + Logger.error(`Actual error: ${JSON.stringify(ex?.response?.data ?? ex.message ?? ex, undefined, 2)}`); azureAccount.properties.tenants = azureAccount.properties.tenants.filter(t => t.id !== tenant.id); continue; } cachedTokens = await this.getCachedToken(account.key, resource.id, tenant.id); if (!cachedTokens) { - console.log('Refresh access tokens didn not set cache'); + Logger.log('Refresh access tokens didn not set cache'); return undefined; } } @@ -270,7 +271,7 @@ export abstract class AzureAuth implements vscode.Disposable { } catch (ex) { const msg = localize('azure.cacheErrrorRemove', "Error when removing your account from the cache."); vscode.window.showErrorMessage(msg); - console.error('Error when removing tokens.', ex); + Logger.error('Error when removing tokens.', ex); } } @@ -292,7 +293,7 @@ export abstract class AzureAuth implements vscode.Disposable { return await axios.post(uri, qs.stringify(postData), config); } catch (ex) { - console.log('Unexpected error making Azure auth request', 'azureCore.postRequest', JSON.stringify(ex?.response?.data, undefined, 2)); + Logger.log('Unexpected error making Azure auth request', 'azureCore.postRequest', JSON.stringify(ex?.response?.data, undefined, 2)); throw ex; } } @@ -309,7 +310,7 @@ export abstract class AzureAuth implements vscode.Disposable { return await axios.get(uri, config); } catch (ex) { // Intercept and print error - console.log('Unexpected error making Azure auth request', 'azureCore.getRequest', JSON.stringify(ex?.response?.data, undefined, 2)); + Logger.log('Unexpected error making Azure auth request', 'azureCore.getRequest', JSON.stringify(ex?.response?.data, undefined, 2)); // rethrow error throw ex; } @@ -326,6 +327,7 @@ export abstract class AzureAuth implements vscode.Disposable { const tenantUri = url.resolve(this.metadata.settings.armResource.endpoint, 'tenants?api-version=2019-11-01'); try { const tenantResponse = await this.makeGetRequest(token.token, tenantUri); + Logger.pii('getTenants', tenantResponse.data); const tenants: Tenant[] = tenantResponse.data.value.map((tenantInfo: TenantResponse) => { return { id: tenantInfo.tenantId, @@ -343,7 +345,7 @@ export abstract class AzureAuth implements vscode.Disposable { return tenants; } catch (ex) { - console.log(ex); + Logger.log(ex); throw new Error('Error retrieving tenant information'); } } @@ -357,7 +359,7 @@ export abstract class AzureAuth implements vscode.Disposable { const allSubs: Subscription[] = []; const tokens = await this.getSecurityToken(account, azdata.AzureResource.ResourceManagement); if (!tokens) { - console.log('There were no resource management tokens to retrieve subscriptions from. Account is stale.'); + Logger.log('There were no resource management tokens to retrieve subscriptions from. Account is stale.'); account.isStale = true; } @@ -366,6 +368,7 @@ export abstract class AzureAuth implements vscode.Disposable { const subscriptionUri = url.resolve(this.metadata.settings.armResource.endpoint, 'subscriptions?api-version=2019-11-01'); try { const subscriptionResponse = await this.makeGetRequest(token.token, subscriptionUri); + Logger.pii('getSubscriptions', subscriptionResponse.data); const subscriptions: Subscription[] = subscriptionResponse.data.value.map((subscriptionInfo: SubscriptionResponse) => { return { id: subscriptionInfo.subscriptionId, @@ -375,7 +378,7 @@ export abstract class AzureAuth implements vscode.Disposable { }); allSubs.push(...subscriptions); } catch (ex) { - console.log(ex); + Logger.error(ex); throw new Error('Error retrieving subscription information'); } } @@ -389,6 +392,7 @@ export abstract class AzureAuth implements vscode.Disposable { try { const tokenUrl = `${this.loginEndpointUrl}${tenant}/oauth2/token`; const tokenResponse = await this.makePostRequest(tokenUrl, postData); + Logger.pii(JSON.stringify(tokenResponse.data)); const tokenClaims = this.getTokenClaims(tokenResponse.data.access_token); const accessToken: AccessToken = { @@ -404,6 +408,7 @@ export abstract class AzureAuth implements vscode.Disposable { refreshResponse = { accessToken, refreshToken, tokenClaims, expiresOn }; } catch (ex) { + Logger.pii(JSON.stringify(ex?.response?.data)); if (ex?.response?.data?.error === 'interaction_required') { const shouldOpenLink = await this.openConsentDialog(tenant, resourceId); if (shouldOpenLink === true) { @@ -476,7 +481,7 @@ export abstract class AzureAuth implements vscode.Disposable { const refreshToken = getTokenResponse?.refreshToken; if (!accessToken || !refreshToken) { - console.log('Access or refresh token were undefined'); + Logger.log('Access or refresh token were undefined'); const msg = localize('azure.refreshTokenError', "Error when refreshing your account."); throw new Error(msg); } @@ -499,7 +504,7 @@ export abstract class AzureAuth implements vscode.Disposable { await this.tokenCache.saveCredential(`${account.accountId}_access_${resourceId}_${tenantId}`, JSON.stringify(accessToken)); await this.tokenCache.saveCredential(`${account.accountId}_refresh_${resourceId}_${tenantId}`, JSON.stringify(refreshToken)); } catch (ex) { - console.error('Error when storing tokens.', ex); + Logger.error('Error when storing tokens.', ex); throw new Error(msg); } } diff --git a/extensions/azurecore/src/account-provider/auths/azureAuthCodeGrant.ts b/extensions/azurecore/src/account-provider/auths/azureAuthCodeGrant.ts index 92b1527c33..5180fa92c5 100644 --- a/extensions/azurecore/src/account-provider/auths/azureAuthCodeGrant.ts +++ b/extensions/azurecore/src/account-provider/auths/azureAuthCodeGrant.ts @@ -28,6 +28,7 @@ import { import { SimpleWebServer } from '../utils/simpleWebServer'; import { SimpleTokenCache } from '../simpleTokenCache'; +import { Logger } from '../../utils/Logger'; const localize = nls.loadMessageBundle(); function parseQuery(uri: vscode.Uri) { @@ -82,7 +83,7 @@ export class AzureAuthCodeGrant extends AzureAuth { if (ex.msg) { vscode.window.showErrorMessage(ex.msg); } - console.log(ex); + Logger.error(ex); } if (!accessToken) { @@ -111,7 +112,7 @@ export class AzureAuthCodeGrant extends AzureAuth { } catch (err) { 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.'); vscode.window.showErrorMessage(msg); - console.dir(err); + Logger.error(JSON.stringify(err)); return undefined; } @@ -208,7 +209,7 @@ export class AzureAuthCodeGrant extends AzureAuth { try { await this.setCachedToken({ accountId: accessToken.key, providerId: this.metadata.id }, accessToken, refreshToken); } catch (ex) { - console.log(ex); + Logger.error(ex); if (ex.msg) { vscode.window.showErrorMessage(ex.msg); authCompleteDeferred.reject(ex); @@ -237,7 +238,7 @@ export class AzureAuthCodeGrant extends AzureAuth { try { fileContents = await fs.readFile(filePath); } catch (ex) { - console.error(ex); + Logger.error(ex); res.writeHead(400); res.end(); return; @@ -252,11 +253,11 @@ export class AzureAuthCodeGrant extends AzureAuth { }; server.on('/landing.css', (req, reqUrl, res) => { - sendFile(res, path.join(mediaPath, 'landing.css'), 'text/css; charset=utf-8').catch(console.error); + sendFile(res, path.join(mediaPath, 'landing.css'), 'text/css; charset=utf-8').catch(Logger.error); }); server.on('/SignIn.svg', (req, reqUrl, res) => { - sendFile(res, path.join(mediaPath, 'SignIn.svg'), 'image/svg+xml').catch(console.error); + sendFile(res, path.join(mediaPath, 'SignIn.svg'), 'image/svg+xml').catch(Logger.error); }); server.on('/signin', (req, reqUrl, res) => { @@ -267,7 +268,7 @@ export class AzureAuthCodeGrant extends AzureAuth { res.writeHead(400, { 'content-type': 'text/html' }); res.write(localize('azureAuth.nonceError', "Authentication failed due to a nonce mismatch, please close Azure Data Studio and try again.")); res.end(); - console.error('nonce no match', receivedNonce, nonce); + Logger.error('nonce no match', receivedNonce, nonce); return; } res.writeHead(302, { Location: loginUrl }); diff --git a/extensions/azurecore/src/account-provider/azureAccountProvider.ts b/extensions/azurecore/src/account-provider/azureAccountProvider.ts index ba4630054f..03d382c3f7 100644 --- a/extensions/azurecore/src/account-provider/azureAccountProvider.ts +++ b/extensions/azurecore/src/account-provider/azureAccountProvider.ts @@ -17,6 +17,7 @@ import { SimpleTokenCache } from './simpleTokenCache'; import { AzureAuth, TokenResponse } from './auths/azureAuth'; import { AzureAuthCodeGrant } from './auths/azureAuthCodeGrant'; import { AzureDeviceCode } from './auths/azureDeviceCode'; +import { Logger } from '../utils/Logger'; const localize = nls.loadMessageBundle(); @@ -127,7 +128,7 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp } if (this.authMappings.size === 0) { - console.log('No auth method was enabled.'); + Logger.log('No auth method was enabled.'); vscode.window.showErrorMessage(noAuthAvailable); return { canceled: true }; } @@ -144,7 +145,7 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp const pick = await vscode.window.showQuickPick(options, { canPickMany: false }); if (!pick) { - console.log('No auth method was selected.'); + Logger.log('No auth method was selected.'); vscode.window.showErrorMessage(noAuthSelected); return { canceled: true }; } diff --git a/extensions/azurecore/src/extension.ts b/extensions/azurecore/src/extension.ts index 49f346d10c..a5e98cddb0 100644 --- a/extensions/azurecore/src/extension.ts +++ b/extensions/azurecore/src/extension.ts @@ -42,6 +42,7 @@ import * as azureResourceUtils from './azureResource/utils'; import * as utils from './utils'; import * as loc from './localizedConstants'; import { AzureResourceGroupService } from './azureResource/providers/resourceGroup/resourceGroupService'; +import { Logger } from './utils/Logger'; let extensionContext: vscode.ExtensionContext; @@ -76,6 +77,7 @@ export async function activate(context: vscode.ExtensionContext): Promise console.log(err)); @@ -164,5 +166,15 @@ async function onDidChangeConfiguration(e: vscode.ConfigurationChangeEvent, apiW if (response === loc.reload) { await apiWrapper.executeCommand('workbench.action.reloadWindow'); } + return; + } + + if (e.affectsConfiguration('azure.piiLogging')) { + updatePiiLoggingLevel(apiWrapper); } } + +function updatePiiLoggingLevel(apiWrapper: ApiWrapper) { + const piiLogging: boolean = apiWrapper.getExtensionConfiguration().get('piiLogging'); + Logger.piiLogging = piiLogging; +} diff --git a/extensions/azurecore/src/utils/Logger.ts b/extensions/azurecore/src/utils/Logger.ts new file mode 100644 index 0000000000..2853273fda --- /dev/null +++ b/extensions/azurecore/src/utils/Logger.ts @@ -0,0 +1,36 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the Source EULA. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +export class Logger { + private static _piiLogging: boolean = false; + + static log(msg: any, ...vals: any[]) { + if (vals && vals.length > 0) { + return console.log(msg, vals); + } + console.log(msg); + } + + static error(msg: any, ...vals: any[]) { + if (vals && vals.length > 0) { + return console.error(msg, vals); + } + console.error(msg); + } + + static pii(msg: any, ...vals: any[]) { + if (this.piiLogging) { + Logger.log(msg, vals); + } + } + + public static set piiLogging(val: boolean) { + this._piiLogging = val; + } + + public static get piiLogging(): boolean { + return this._piiLogging; + } +}