From 04c45d14c8a2b531a4f869c355b76b2fd260af22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Rodr=C3=ADguez?= Date: Fri, 22 Oct 2021 17:42:56 -0400 Subject: [PATCH] [Identity] Re-enabling the Service Fabric MSI (#17579) * [Identity] Re-enabling the Service Fabric MSI * Update managedIdentityCredential.spec.ts * CHANGELOG entry * Update fabricMsi.ts * Update fabricMsi.ts --- sdk/identity/identity/CHANGELOG.md | 2 ++ .../managedIdentityCredential/fabricMsi.ts | 10 +++++-- .../managedIdentityCredential/index.ts | 5 ++-- .../managedIdentityCredential/utils.ts | 8 +++++- sdk/identity/identity/test/httpRequests.ts | 28 +++++++++---------- .../node/managedIdentityCredential.spec.ts | 7 ++--- 6 files changed, 34 insertions(+), 26 deletions(-) diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md index b2442704d7ed..4b99ff80b5e1 100644 --- a/sdk/identity/identity/CHANGELOG.md +++ b/sdk/identity/identity/CHANGELOG.md @@ -4,6 +4,8 @@ ### Features Added +- The `ManagedIdentityCredential` now supports the Service Fabric environment. + ### Breaking Changes ### Bugs Fixed diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/fabricMsi.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/fabricMsi.ts index 21198a222803..135eeda19295 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/fabricMsi.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/fabricMsi.ts @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +import https from "https"; import { createHttpHeaders, PipelineRequestOptions } from "@azure/core-rest-pipeline"; import { AccessToken, GetTokenOptions } from "@azure/core-auth"; import { MSI, MSIConfiguration } from "./models"; @@ -87,7 +88,7 @@ export const fabricMsi: MSI = { configuration: MSIConfiguration, getTokenOptions: GetTokenOptions = {} ): Promise { - const { identityClient, scopes, clientId } = configuration; + const { scopes, identityClient, clientId } = configuration; logger.info( [ @@ -103,7 +104,12 @@ export const fabricMsi: MSI = { identityClient, prepareRequestOptions(scopes, clientId), expiresInParser, - getTokenOptions + getTokenOptions, + new https.Agent({ + // This is necessary because Service Fabric provides a self-signed certificate. + // The alternative path is to verify the certificate using the IDENTITY_SERVER_THUMBPRINT env variable. + rejectUnauthorized: false + }) ); } }; diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts index 5e42e897965a..f43680ee6c81 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts @@ -14,6 +14,7 @@ import { imdsMsi } from "./imdsMsi"; import { MSI } from "./models"; import { arcMsi } from "./arcMsi"; import { tokenExchangeMsi } from "./tokenExchangeMsi"; +import { fabricMsi } from "./fabricMsi"; const logger = credentialLogger("ManagedIdentityCredential"); @@ -74,9 +75,7 @@ export class ManagedIdentityCredential implements TokenCredential { return this.cachedMSI; } - // "fabricMsi" can't be added yet because our HTTPs pipeline doesn't allow skipping the SSL verification step, - // which is necessary since Service Fabric only provides self-signed certificates on their Identity Endpoint. - const MSIs = [appServiceMsi2017, cloudShellMsi, arcMsi, tokenExchangeMsi(), imdsMsi]; + const MSIs = [fabricMsi, appServiceMsi2017, cloudShellMsi, arcMsi, tokenExchangeMsi(), imdsMsi]; for (const msi of MSIs) { if (await msi.isAvailable(scopes, this.identityClient, clientId, getTokenOptions)) { diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts index 8b430c7efb72..cd42c22df699 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts @@ -3,6 +3,7 @@ import { AccessToken, GetTokenOptions } from "@azure/core-auth"; import { PipelineRequestOptions, createPipelineRequest } from "@azure/core-rest-pipeline"; +import { Agent } from "http"; import { IdentityClient } from "../../client/identityClient"; import { DefaultScopeSuffix } from "./constants"; import { MSIExpiresInParser } from "./models"; @@ -38,7 +39,8 @@ export async function msiGenericGetToken( identityClient: IdentityClient, requestOptions: PipelineRequestOptions, expiresInParser: MSIExpiresInParser | undefined, - getTokenOptions: GetTokenOptions = {} + getTokenOptions: GetTokenOptions = {}, + agent?: Agent ): Promise { const request = createPipelineRequest({ abortSignal: getTokenOptions.abortSignal, @@ -46,6 +48,10 @@ export async function msiGenericGetToken( allowInsecureConnection: true }); + if (agent) { + request.agent = agent; + } + const tokenResponse = await identityClient.sendTokenRequest(request, expiresInParser); return (tokenResponse && tokenResponse.accessToken) || null; } diff --git a/sdk/identity/identity/test/httpRequests.ts b/sdk/identity/identity/test/httpRequests.ts index cc22b88e075c..6c2002336f0c 100644 --- a/sdk/identity/identity/test/httpRequests.ts +++ b/sdk/identity/identity/test/httpRequests.ts @@ -172,23 +172,21 @@ export async function prepareIdentityTests({ const totalOptions: http.RequestOptions[] = []; try { - sandbox.replace( - providerObject, - "request", - (options: string | URL | http.RequestOptions, resolve: any) => { - totalOptions.push(options as http.RequestOptions); + const fakeRequest = (options: string | URL | http.RequestOptions, resolve: any) => { + totalOptions.push(options as http.RequestOptions); - const { response, error } = responses.shift()!; - if (error) { - throw error; - } else { - resolve(responseToIncomingMessage(response!)); - } - const request = createRequest(); - spies.push(sandbox.spy(request, "end")); - return request; + const { response, error } = responses.shift()!; + if (error) { + throw error; + } else { + resolve(responseToIncomingMessage(response!)); } - ); + const request = createRequest(); + spies.push(sandbox.spy(request, "end")); + return request; + }; + sandbox.replace(providerObject, "request", fakeRequest); + sandbox.replace(providerObject.Agent.prototype as any, "request", fakeRequest); } catch (e) { console.debug( "Failed to replace the request. This might be expected if you're running multiple sendCredentialRequests() calls." diff --git a/sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts b/sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts index fe902f117f7f..789144ff9967 100644 --- a/sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts +++ b/sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts @@ -447,10 +447,7 @@ describe("ManagedIdentityCredential", function() { } }); - // "fabricMsi" isn't part of the ManagedIdentityCredential MSIs yet - // because our HTTPs pipeline doesn't allow skipping the SSL verification step, - // which is necessary since Service Fabric only provides self-signed certificates on their Identity Endpoint. - it.skip("sends an authorization request correctly in an Azure Fabric environment", async () => { + it("sends an authorization request correctly in an Azure Fabric environment", async () => { // Trigger App Service behavior by setting environment variables process.env.IDENTITY_ENDPOINT = "https://endpoint"; process.env.IDENTITY_HEADER = "secret"; @@ -463,7 +460,7 @@ describe("ManagedIdentityCredential", function() { credential: new ManagedIdentityCredential("client"), secureResponses: [ createResponse(200, { - token: "token", + access_token: "token", expires_on: 1 }) ]