From 06f47ee93b9472f4dac4b8a92dbfde57b89b2e5b Mon Sep 17 00:00:00 2001 From: KarishmaGhiya Date: Thu, 25 Jul 2024 17:22:20 -0700 Subject: [PATCH] [Identity] Fix PowershellCredential token parsing logic (#30508) --- .../credentials/azurePowerShellCredential.ts | 40 +++++-- .../node/azurePowerShellCredential.spec.ts | 102 ++++++++++++------ 2 files changed, 106 insertions(+), 36 deletions(-) diff --git a/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts b/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts index e7ab4a3a33d1..1cbc2c74e223 100644 --- a/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts +++ b/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts @@ -166,13 +166,8 @@ export class AzurePowerShellCredential implements TokenCredential { ]); const result = results[1]; - try { - return JSON.parse(result); - } catch (e: any) { - throw new Error(`Unable to parse the output of PowerShell. Received output: ${result}`); - } + return parseJsonToken(result); } - throw new Error(`Unable to execute PowerShell. Ensure that it is installed in your system`); } @@ -226,3 +221,36 @@ export class AzurePowerShellCredential implements TokenCredential { }); } } + +/** + * + * @internal + */ +export async function parseJsonToken( + result: string, +): Promise<{ Token: string; ExpiresOn: string }> { + const jsonRegex = /{[^{}]*}/g; + const matches = result.match(jsonRegex); + let resultWithoutToken = result; + if (matches) { + try { + for (const item of matches) { + try { + const jsonContent = JSON.parse(item); + if (jsonContent?.Token) { + resultWithoutToken = resultWithoutToken.replace(item, ""); + if (resultWithoutToken) { + logger.getToken.warning(resultWithoutToken); + } + return jsonContent; + } + } catch (e) { + continue; + } + } + } catch (e: any) { + throw new Error(`Unable to parse the output of PowerShell. Received output: ${result}`); + } + } + throw new Error(`No access token found in the output. Received output: ${result}`); +} diff --git a/sdk/identity/identity/test/internal/node/azurePowerShellCredential.spec.ts b/sdk/identity/identity/test/internal/node/azurePowerShellCredential.spec.ts index b659005b3037..037d6707866b 100644 --- a/sdk/identity/identity/test/internal/node/azurePowerShellCredential.spec.ts +++ b/sdk/identity/identity/test/internal/node/azurePowerShellCredential.spec.ts @@ -5,6 +5,7 @@ import { formatCommand, + parseJsonToken, powerShellErrors, powerShellPublicErrorMessages, } from "../../../src/credentials/azurePowerShellCredential"; @@ -28,7 +29,12 @@ describe("AzurePowerShellCredential", function () { const scope = "https://vault.azure.net/.default"; const tenantIdErrorMessage = "Invalid tenant id provided. You can locate your tenant id by following the instructions listed here: https://learn.microsoft.com/partner-center/find-ids-and-domain-names."; + let sandbox: Sinon.SinonSandbox; + beforeEach(() => { + sandbox = Sinon.createSandbox(); + }); afterEach(() => { + sandbox.restore(); resetCommandStack(); }); @@ -40,8 +46,6 @@ describe("AzurePowerShellCredential", function () { }); it("throws an expected error if the user hasn't logged in through PowerShell", async function () { - const sandbox = Sinon.createSandbox(); - const stub = sandbox.stub(processUtils, "execFile"); stub.onCall(0).returns(Promise.resolve("")); // The first call checks that the command is available. stub.onCall(1).throws(new Error(`Get-AzAccessToken: ${powerShellErrors.login}`)); @@ -58,13 +62,9 @@ describe("AzurePowerShellCredential", function () { assert.ok(error); assert.equal(error?.name, "CredentialUnavailableError"); assert.equal(error?.message, powerShellPublicErrorMessages.login); - - sandbox.restore(); }); it("throws an expected error if the user hasn't installed the Az.Account module", async function () { - const sandbox = Sinon.createSandbox(); - const stub = sandbox.stub(processUtils, "execFile"); stub.onCall(0).returns(Promise.resolve("")); // The first call checks that the command is available. stub.onCall(1).throws(new Error(powerShellErrors.installed)); @@ -81,13 +81,9 @@ describe("AzurePowerShellCredential", function () { assert.ok(error); assert.equal(error?.name, "CredentialUnavailableError"); assert.equal(error?.message, powerShellPublicErrorMessages.installed); - - sandbox.restore(); }); it("throws an expected error if PowerShell isn't installed", async function () { - const sandbox = Sinon.createSandbox(); - const stub = sandbox.stub(processUtils, "execFile"); stub.onCall(0).throws(new Error()); @@ -111,13 +107,9 @@ describe("AzurePowerShellCredential", function () { error?.message, `Error: Unable to execute PowerShell. Ensure that it is installed in your system. To troubleshoot, visit https://aka.ms/azsdk/js/identity/powershellcredential/troubleshoot.`, ); - - sandbox.restore(); }); it("throws an expected error if PowerShell returns something that isn't valid JSON", async function () { - const sandbox = Sinon.createSandbox(); - const stub = sandbox.stub(processUtils, "execFile"); let idx = 0; stub.onCall(idx++).returns(Promise.resolve("")); // The first call checks that the command is available. @@ -137,16 +129,12 @@ describe("AzurePowerShellCredential", function () { assert.equal(error?.name, "CredentialUnavailableError"); assert.equal( error?.message, - `Error: Unable to parse the output of PowerShell. Received output: Not valid JSON. To troubleshoot, visit https://aka.ms/azsdk/js/identity/powershellcredential/troubleshoot.`, + `Error: No access token found in the output. Received output: Not valid JSON. To troubleshoot, visit https://aka.ms/azsdk/js/identity/powershellcredential/troubleshoot.`, ); - - sandbox.restore(); }); if (process.platform === "win32") { it("throws an expected error if PowerShell returns something that isn't valid JSON (Windows PowerShell fallback)", async function () { - const sandbox = Sinon.createSandbox(); - const stub = sandbox.stub(processUtils, "execFile"); let idx = 0; stub.onCall(idx++).throws(new Error()); @@ -167,16 +155,12 @@ describe("AzurePowerShellCredential", function () { assert.equal(error?.name, "CredentialUnavailableError"); assert.equal( error?.message, - `Error: Unable to parse the output of PowerShell. Received output: Not valid JSON. To troubleshoot, visit https://aka.ms/azsdk/js/identity/powershellcredential/troubleshoot.`, + `Error: No access token found in the output. Received output: Not valid JSON. To troubleshoot, visit https://aka.ms/azsdk/js/identity/powershellcredential/troubleshoot.`, ); - - sandbox.restore(); }); } it("authenticates", async function () { - const sandbox = Sinon.createSandbox(); - const tokenResponse = { Token: "token", ExpiresOn: "2021-04-21T20:52:16+00:00", @@ -194,13 +178,9 @@ describe("AzurePowerShellCredential", function () { const token = await credential.getToken(scope); assert.equal(token?.token, tokenResponse.Token); assert.equal(token?.expiresOnTimestamp!, new Date(tokenResponse.ExpiresOn).getTime()); - - sandbox.restore(); }); it("authenticates with tenantId on getToken", async function () { - const sandbox = Sinon.createSandbox(); - const tokenResponse = { Token: "token", ExpiresOn: "2021-04-21T20:52:16+00:00", @@ -218,8 +198,6 @@ describe("AzurePowerShellCredential", function () { const token = await credential.getToken(scope, { tenantId: "TENANT-ID" } as GetTokenOptions); assert.equal(token?.token, tokenResponse.Token); assert.equal(token?.expiresOnTimestamp!, new Date(tokenResponse.ExpiresOn).getTime()); - - sandbox.restore(); }); /** @@ -278,4 +256,68 @@ describe("AzurePowerShellCredential", function () { ); }); } + + it("parses JSON correctly from an output of multiple JSON objects from AzurePowerShell", async function () { + const tokenResponse = { + Token: "token", + ExpiresOn: "2024-07-19T05:35:37+00:00", + TenantId: "***", + UserId: "***", + Type: "Bearer", + }; + const completeResponse = `Note : Go to https://aka.ms/azps-changewarnings for steps to suppress this breaking change warning, and other information on breaking changes in Azure PowerShell. +{"afasf"} +{ +"fsdf": +{ + "Token": "token", + "ExpiresOn": "2024-07-19T05:35:37+00:00", + "TenantId": "***", + "UserId": "***", + "Type": "Bearer" +}, + }`; + const token = await parseJsonToken(completeResponse); + assert.equal(token?.Token, tokenResponse.Token); + }); + + it("parses JSON correctly from an output of errors from AzurePowerShell", async function () { + const tokenResponse = { + Token: "token", + ExpiresOn: "2024-07-19T05:35:37+00:00", + TenantId: "***", + UserId: "***", + Type: "Bearer", + }; + const completeResponse = `AggregateAuthenticationError: ChainedTokenCredential authentication failed. +CredentialUnavailableError: Error: Unable to parse the output of PowerShell. Received output: WARNING: Upcoming breaking changes in the cmdlet 'Get-AzAccessToken' : +The Token property of the output type will be changed from String to SecureString. Add the [-AsSecureString] switch to avoid the impact of this upcoming breaking change. +- The change is expected to take effect in Az version : '13.0.0' +- The change is expected to take effect in Az.Accounts version : '4.0.0' +Note : Go to https://aka.ms/azps-changewarnings for steps to suppress this breaking change warning, and other information on breaking changes in Azure PowerShell. +{ + "Token": "token", + "ExpiresOn": "2024-07-19T05:35:37+00:00", + "TenantId": "***", + "UserId": "***", + "Type": "Bearer" +} + +. To troubleshoot, visit https://aka.ms/azsdk/js/identity/powershellcredential/troubleshoot. +CredentialUnavailableError: Please run 'az login' from a command prompt to authenticate before using this credential. +CredentialUnavailableError: Azure Developer CLI couldn't be found. To mitigate this issue, see the troubleshooting guidelines at https://aka.ms/azsdk/js/identity/azdevclicredential/troubleshoot. +CredentialUnavailableError: EnvironmentCredential is unavailable. No underlying credential could be used. To troubleshoot, visit https://aka.ms/azsdk/js/identity/environmentcredential/troubleshoot. + at /Users/runner/work/1/s/common/temp/node_modules/.pnpm/@azure+identity@4.4.0/node_modules/@azure/identity/src/credentials/chainedTokenCredential.ts:85:23 + at processTicksAndRejections (node:internal/process/task_queues:95:5) + at Object.withSpan (/Users/runner/work/1/s/common/temp/node_modules/.pnpm/@azure+core-tracing@1.1.2/node_modules/@azure/core-tracing/src/tracingClient.ts:70:22) + at ChainedTokenCredential.getToken (/Users/runner/work/1/s/common/temp/node_modules/.pnpm/@azure+identity@4.4.0/node_modules/@azure/identity/src/credentials/chainedTokenCredential.ts:51:23) + at tryGetAccessToken (/Users/runner/work/1/s/sdk/core/core-rest-pipeline/src/util/tokenCycler.ts:71:26) + at beginRefresh (/Users/runner/work/1/s/sdk/core/core-rest-pipeline/src/util/tokenCycler.ts:82:35) + at Object.defaultAuthorizeRequest [as authorizeRequest] (/Users/runner/work/1/s/sdk/core/core-rest-pipeline/src/policies/bearerTokenAuthenticationPolicy.ts:114:23) + at Object.sendRequest (/Users/runner/work/1/s/sdk/core/core-rest-pipeline/src/policies/bearerTokenAuthenticationPolicy.ts:179:7) + at sendRequest (/Users/runner/work/1/s/common/temp/node_modules/.pnpm/@azure-rest+core-client@1.4.0/node_modules/@azure-rest/core-client/src/sendRequest.ts:40:20) + at Context. (/Users/runner/work/1/s/sdk/maps/maps-geolocation-rest/test/public/MapsGeolocation.spec.ts:2:35)`; + const token = await parseJsonToken(completeResponse); + assert.equal(token?.Token, tokenResponse.Token); + }); });