From 99c74f791d9cdd297dbbcdcae4a498c7faa13834 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Thu, 16 Mar 2023 15:42:04 +0800 Subject: [PATCH 01/23] Update auxiliary auth toke policy --- .../policies/auxiliaryAuthenticationPolicy.ts | 119 ++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts new file mode 100644 index 000000000000..9873056a39d4 --- /dev/null +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts @@ -0,0 +1,119 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { GetTokenOptions, TokenCredential } from "@azure/core-auth"; +import { AzureLogger } from "@azure/logger"; +import { PipelineRequest, PipelineResponse, SendRequest } from "../interfaces"; +import { PipelinePolicy } from "../pipeline"; +import { createTokenCycler } from "../util/tokenCycler"; +import { logger as coreLogger } from "../log"; +import { AuthorizeRequestOptions } from "./bearerTokenAuthenticationPolicy"; + +/** + * The programmatic identifier of the bearerTokenAuthenticationPolicy. + */ +export const auxiliaryAuthenticationPolicyName = "auxiliaryAuthenticationPolicy"; +const AUTHORIZATION_AUXILIARY_HEADER = "x-ms-authorization-auxiliary"; + +/** + * Options to configure the auxiliaryAuthenticationPolicy + */ +export interface AuxiliaryAuthenticationPolicyNameOptions { + /** + * The TokenCredential list that can supply the auxiliary authentication token. + */ + credentials?: TokenCredential[]; + /** + * The scopes for which the auxiliary token applies. + */ + scopes: string | string[]; + /** + * A logger can be sent for debugging purposes. + */ + logger?: AzureLogger; +} + +/** + * Default authorize request handler + */ +async function defaultAuthorizeRequest(options: AuthorizeRequestOptions): Promise { + const { scopes, getAccessToken, request } = options; + const getTokenOptions: GetTokenOptions = { + abortSignal: request.abortSignal, + tracingOptions: request.tracingOptions, + }; + + const accessToken = await getAccessToken(scopes, getTokenOptions); + + if (accessToken) { + return accessToken.token; + } + return null; +} + +function buildAccessTokenCycler(credential: TokenCredential) { + return credential + ? createTokenCycler(credential /* , options */) + : () => Promise.resolve(null); +} + +function formatSingleToken(token: string) { + return `Bearer ${token}`; +} + +/** + * A policy that can request a token from a TokenCredential implementation and + * then apply it to the Authorization header of a request as a Bearer token. + */ +export function auxiliaryAuthenticationPolicy( + options: AuxiliaryAuthenticationPolicyNameOptions +): PipelinePolicy { + const { credentials, scopes } = options; + const logger = options.logger || coreLogger; + const tokenCyclerMap = new Map(); + + return { + name: auxiliaryAuthenticationPolicyName, + async sendRequest(request: PipelineRequest, next: SendRequest): Promise { + if (!request.url.toLowerCase().startsWith("https://")) { + throw new Error( + "Bearer token authentication is not permitted for non-TLS protected (non-https) URLs." + ); + } + + const tokenList: string[] = []; + for (const credential of (credentials ?? [])) { + if (!tokenCyclerMap.has(credential)) { + tokenCyclerMap.set(credential, buildAccessTokenCycler(credential)) + } + const getAccessToken = tokenCyclerMap.get(credential); + const singalAccessToken = await defaultAuthorizeRequest({ + scopes: Array.isArray(scopes) ? scopes : [scopes], + request, + getAccessToken, + logger, + }); + if (singalAccessToken) { + tokenList.push(singalAccessToken); + } + } + + request.headers.set(AUTHORIZATION_AUXILIARY_HEADER, tokenList.map(formatSingleToken).join(",")); + + let response: PipelineResponse; + let error: Error | undefined; + try { + response = await next(request); + } catch (err: any) { + error = err; + response = err.response; + } + + if (error) { + throw error; + } else { + return response; + } + }, + }; +} From 78884f2f7fbd6a877849c106bd21b23d48338cdd Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Thu, 16 Mar 2023 17:06:26 +0800 Subject: [PATCH 02/23] Update the changelog and bump the version --- sdk/core/core-rest-pipeline/CHANGELOG.md | 8 +- sdk/core/core-rest-pipeline/package.json | 2 +- .../review/core-rest-pipeline.api.md | 13 + sdk/core/core-rest-pipeline/src/index.ts | 5 + .../policies/auxiliaryAuthenticationPolicy.ts | 2 +- .../auxiliaryAuthenticationPolicy.spec.ts | 231 ++++++++++++++++++ 6 files changed, 253 insertions(+), 8 deletions(-) create mode 100644 sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts diff --git a/sdk/core/core-rest-pipeline/CHANGELOG.md b/sdk/core/core-rest-pipeline/CHANGELOG.md index 5720c7359eaa..7e422dbd5709 100644 --- a/sdk/core/core-rest-pipeline/CHANGELOG.md +++ b/sdk/core/core-rest-pipeline/CHANGELOG.md @@ -1,14 +1,10 @@ # Release History -## 1.10.3 (Unreleased) +## 1.11.0 (Unreleased) ### Features Added -### Breaking Changes - -### Bugs Fixed - -### Other Changes +- Added AuxiliaryAuthenticationPolicy ## 1.10.2 (2023-03-02) diff --git a/sdk/core/core-rest-pipeline/package.json b/sdk/core/core-rest-pipeline/package.json index 0aca7136a18d..72cfea8cdfb1 100644 --- a/sdk/core/core-rest-pipeline/package.json +++ b/sdk/core/core-rest-pipeline/package.json @@ -1,6 +1,6 @@ { "name": "@azure/core-rest-pipeline", - "version": "1.10.3", + "version": "1.11.0", "description": "Isomorphic client library for making HTTP requests in node.js and browser.", "sdk-type": "client", "main": "dist/index.js", diff --git a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md index e193b03bc959..1d26bad7ff8d 100644 --- a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md +++ b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md @@ -48,6 +48,19 @@ export interface AuthorizeRequestOptions { scopes: string[]; } +// @public +export function auxiliaryAuthenticationPolicy(options: AuxiliaryAuthenticationPolicyNameOptions): PipelinePolicy; + +// @public +export const auxiliaryAuthenticationPolicyName = "auxiliaryAuthenticationPolicy"; + +// @public +export interface AuxiliaryAuthenticationPolicyNameOptions { + credentials?: TokenCredential[]; + logger?: AzureLogger; + scopes: string | string[]; +} + // @public export function bearerTokenAuthenticationPolicy(options: BearerTokenAuthenticationPolicyOptions): PipelinePolicy; diff --git a/sdk/core/core-rest-pipeline/src/index.ts b/sdk/core/core-rest-pipeline/src/index.ts index 516e34327dab..7c392472630e 100644 --- a/sdk/core/core-rest-pipeline/src/index.ts +++ b/sdk/core/core-rest-pipeline/src/index.ts @@ -87,3 +87,8 @@ export { AuthorizeRequestOnChallengeOptions, } from "./policies/bearerTokenAuthenticationPolicy"; export { ndJsonPolicy, ndJsonPolicyName } from "./policies/ndJsonPolicy"; +export { + auxiliaryAuthenticationPolicy, + AuxiliaryAuthenticationPolicyNameOptions, + auxiliaryAuthenticationPolicyName +} from "./policies/auxiliaryAuthenticationPolicy" diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts index 9873056a39d4..1c2ea432e348 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts @@ -77,7 +77,7 @@ export function auxiliaryAuthenticationPolicy( async sendRequest(request: PipelineRequest, next: SendRequest): Promise { if (!request.url.toLowerCase().startsWith("https://")) { throw new Error( - "Bearer token authentication is not permitted for non-TLS protected (non-https) URLs." + "Auxiliary token authentication is not permitted for non-TLS protected (non-https) URLs." ); } diff --git a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts new file mode 100644 index 000000000000..fb2b69eee5fa --- /dev/null +++ b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts @@ -0,0 +1,231 @@ +import { assert } from "chai"; +import * as sinon from "sinon"; +import { AccessToken, TokenCredential } from "@azure/core-auth"; +import { + PipelinePolicy, + PipelineResponse, + SendRequest, + auxiliaryAuthenticationPolicy, + createHttpHeaders, + createPipelineRequest, +} from "../src"; +import { DEFAULT_CYCLER_OPTIONS } from "../src/util/tokenCycler"; + +const { refreshWindowInMs: defaultRefreshWindow } = DEFAULT_CYCLER_OPTIONS; + +describe("AuxiliaryAuthenticationPolicy", function () { + let clock: sinon.SinonFakeTimers; + + beforeEach(() => { + clock = sinon.useFakeTimers(Date.now()); + }); + afterEach(() => { + clock.restore(); + }); + + it("correctly adds an Authentication header with the access token", async function () { + const mockToken = "token"; + const tokenScopes = ["scope1", "scope2"]; + const fakeGetToken = sinon.fake.returns( + Promise.resolve({ token: mockToken, expiresOn: new Date() }) + ); + const mockCredential: TokenCredential = { + getToken: fakeGetToken, + }; + + const request = createPipelineRequest({ url: "https://example.com" }); + const successResponse: PipelineResponse = { + headers: createHttpHeaders(), + request, + status: 200, + }; + const next = sinon.stub, ReturnType>(); + next.resolves(successResponse); + + const auxiTokenAuthPolicy = createAuxiliaryTokenPolicy(tokenScopes, [mockCredential]); + await auxiTokenAuthPolicy.sendRequest(request, next); + + assert( + fakeGetToken.calledWith(tokenScopes, { + abortSignal: undefined, + tracingOptions: undefined, + }), + "fakeGetToken called incorrectly." + ); + assert.strictEqual(request.headers.get("x-ms-authorization-auxiliary"), `Bearer ${mockToken}`); + }); + + it("correctly adds an Authentication header with two access tokens", async function () { + const mockToken1 = "token1", mockToken2 = "token2"; + const tokenScopes = ["scope1", "scope2"]; + const fakeGetToken1 = sinon.fake.returns( + Promise.resolve({ token: mockToken1, expiresOn: new Date() }) + ); + const fakeGetToken2 = sinon.fake.returns( + Promise.resolve({ token: mockToken2, expiresOn: new Date() }) + ); + const mockCredential1: TokenCredential = { + getToken: fakeGetToken1, + }; + const mockCredential2: TokenCredential = { + getToken: fakeGetToken2, + }; + + const request = createPipelineRequest({ url: "https://example.com" }); + const successResponse: PipelineResponse = { + headers: createHttpHeaders(), + request, + status: 200, + }; + const next = sinon.stub, ReturnType>(); + next.resolves(successResponse); + + const auxiTokenAuthPolicy = createAuxiliaryTokenPolicy(tokenScopes, [mockCredential1, mockCredential2]); + await auxiTokenAuthPolicy.sendRequest(request, next); + + assert( + fakeGetToken1.calledWith(tokenScopes, { + abortSignal: undefined, + tracingOptions: undefined, + }), + "fakeGetToken called incorrectly." + ); + assert.strictEqual(request.headers.get("x-ms-authorization-auxiliary"), `Bearer ${mockToken1},Bearer ${mockToken2}`); + }); + + it("only refreshes invalid token during the refresh window", async () => { + const expireDelayMs = defaultRefreshWindow + 5000; + let tokenExpiration = Date.now() + expireDelayMs; + const shortCredential = new MockRefreshAzureCredential(tokenExpiration); + const longCredential = new MockRefreshAzureCredential(Date.now() + expireDelayMs * 3); + + const request = createPipelineRequest({ url: "https://example.com" }); + const successResponse: PipelineResponse = { + headers: createHttpHeaders(), + request, + status: 200, + }; + const next = sinon.stub, ReturnType>(); + next.resolves(successResponse); + + const policy = createAuxiliaryTokenPolicy("test-scope", [shortCredential, longCredential]); + + // The token is cached and remains cached for a bit. + await policy.sendRequest(request, next); + await policy.sendRequest(request, next); + assert.strictEqual(shortCredential.authCount, 1); + assert.strictEqual(longCredential.authCount, 1); + assert.strictEqual(request.headers.get("x-ms-authorization-auxiliary"), `Bearer mock-token,Bearer mock-token`); + + // The token will remain cached until tokenExpiration - testTokenRefreshBufferMs, so in (5000 - 1000) milliseconds. + + // For safe measure, we test the token is still cached a second earlier than the forced refresh expectation. + clock.tick(expireDelayMs - defaultRefreshWindow - 1000); + await policy.sendRequest(request, next); + assert.strictEqual(shortCredential.authCount, 1); + assert.strictEqual(longCredential.authCount, 1); + + // The new token will last for a few minutes again. + tokenExpiration = Date.now() + expireDelayMs; + shortCredential.expiresOnTimestamp = tokenExpiration; + + // Now we wait until it expires: + clock.tick(expireDelayMs + 1000); + await policy.sendRequest(request, next); + assert.strictEqual(shortCredential.authCount, 2); + assert.strictEqual(longCredential.authCount, 1); + }); + + it("credential errors should bubble up", async () => { + const expireDelayMs = 5000; + const startTime = Date.now(); + const tokenExpiration = startTime + expireDelayMs; + const getTokenDelay = 100; + const credential = new MockRefreshAzureCredential(tokenExpiration, getTokenDelay, clock); + + const request = createPipelineRequest({ url: "https://example.com" }); + const successResponse: PipelineResponse = { + headers: createHttpHeaders(), + request, + status: 200, + }; + const next = sinon.stub, ReturnType>(); + next.resolves(successResponse); + + const policy = createAuxiliaryTokenPolicy("test-scope", [credential]); + + credential.shouldThrow = true; + + let error: Error | undefined; + try { + await policy.sendRequest(request, next); + } catch (e: any) { + error = e; + } + assert.equal(error?.message, "Failed to retrieve the token"); + + assert.strictEqual( + credential.authCount, + 1, + "The first authentication attempt should have happened" + ); + }); + + it("throws if the target URI doesn't start with 'https'", async () => { + const expireDelayMs = defaultRefreshWindow + 5000; + const tokenExpiration = Date.now() + expireDelayMs; + const credential = new MockRefreshAzureCredential(tokenExpiration); + + const request = createPipelineRequest({ url: "http://example.com" }); + const policy = createAuxiliaryTokenPolicy("test-scope", [credential]); + const next = sinon.stub, ReturnType>(); + + let error: Error | undefined; + try { + await policy.sendRequest(request, next); + } catch (e: any) { + error = e; + } + + assert.equal( + error?.message, + "Auxiliary token authentication is not permitted for non-TLS protected (non-https) URLs." + ); + }); + + function createAuxiliaryTokenPolicy( + scopes: string | string[], + credentials: TokenCredential[] + ): PipelinePolicy { + return auxiliaryAuthenticationPolicy({ + scopes, + credentials, + }); + } +}); + +class MockRefreshAzureCredential implements TokenCredential { + public authCount = 0; + public shouldThrow: boolean = false; + + constructor( + public expiresOnTimestamp: number, + public getTokenDelay?: number, + public clock?: sinon.SinonFakeTimers + ) { } + + public async getToken(): Promise { + this.authCount++; + + if (this.shouldThrow) { + throw new Error("Failed to retrieve the token"); + } + + // Allowing getToken to take a while + if (this.getTokenDelay && this.clock) { + this.clock.tick(this.getTokenDelay); + } + + return { token: "mock-token", expiresOnTimestamp: this.expiresOnTimestamp }; + } +} From e43ac871220cb384ae58437c159363616d10c024 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Thu, 16 Mar 2023 17:14:37 +0800 Subject: [PATCH 03/23] Update the test name --- .../test/auxiliaryAuthenticationPolicy.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts index fb2b69eee5fa..0a27228f4ab8 100644 --- a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts +++ b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts @@ -23,7 +23,7 @@ describe("AuxiliaryAuthenticationPolicy", function () { clock.restore(); }); - it("correctly adds an Authentication header with the access token", async function () { + it("correctly adds an Authentication header with one access token", async function () { const mockToken = "token"; const tokenScopes = ["scope1", "scope2"]; const fakeGetToken = sinon.fake.returns( From 9ad2d9483b539ec1887c04f39724d8c9e8849105 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Thu, 16 Mar 2023 17:24:44 +0800 Subject: [PATCH 04/23] Update format --- sdk/core/core-rest-pipeline/src/index.ts | 4 ++-- .../policies/auxiliaryAuthenticationPolicy.ts | 17 +++++++--------- .../auxiliaryAuthenticationPolicy.spec.ts | 20 ++++++++++++++----- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/sdk/core/core-rest-pipeline/src/index.ts b/sdk/core/core-rest-pipeline/src/index.ts index 7c392472630e..a1f6a3249cec 100644 --- a/sdk/core/core-rest-pipeline/src/index.ts +++ b/sdk/core/core-rest-pipeline/src/index.ts @@ -90,5 +90,5 @@ export { ndJsonPolicy, ndJsonPolicyName } from "./policies/ndJsonPolicy"; export { auxiliaryAuthenticationPolicy, AuxiliaryAuthenticationPolicyNameOptions, - auxiliaryAuthenticationPolicyName -} from "./policies/auxiliaryAuthenticationPolicy" + auxiliaryAuthenticationPolicyName, +} from "./policies/auxiliaryAuthenticationPolicy"; diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts index 1c2ea432e348..75e714271fe9 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts @@ -52,13 +52,7 @@ async function defaultAuthorizeRequest(options: AuthorizeRequestOptions): Promis } function buildAccessTokenCycler(credential: TokenCredential) { - return credential - ? createTokenCycler(credential /* , options */) - : () => Promise.resolve(null); -} - -function formatSingleToken(token: string) { - return `Bearer ${token}`; + return credential ? createTokenCycler(credential /* , options */) : () => Promise.resolve(null); } /** @@ -82,9 +76,9 @@ export function auxiliaryAuthenticationPolicy( } const tokenList: string[] = []; - for (const credential of (credentials ?? [])) { + for (const credential of credentials ?? []) { if (!tokenCyclerMap.has(credential)) { - tokenCyclerMap.set(credential, buildAccessTokenCycler(credential)) + tokenCyclerMap.set(credential, buildAccessTokenCycler(credential)); } const getAccessToken = tokenCyclerMap.get(credential); const singalAccessToken = await defaultAuthorizeRequest({ @@ -98,7 +92,10 @@ export function auxiliaryAuthenticationPolicy( } } - request.headers.set(AUTHORIZATION_AUXILIARY_HEADER, tokenList.map(formatSingleToken).join(",")); + request.headers.set( + AUTHORIZATION_AUXILIARY_HEADER, + tokenList.map((token) => `Bearer ${token}`).join(",") + ); let response: PipelineResponse; let error: Error | undefined; diff --git a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts index 0a27228f4ab8..01b3dbb85297 100644 --- a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts +++ b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts @@ -56,7 +56,8 @@ describe("AuxiliaryAuthenticationPolicy", function () { }); it("correctly adds an Authentication header with two access tokens", async function () { - const mockToken1 = "token1", mockToken2 = "token2"; + const mockToken1 = "token1", + mockToken2 = "token2"; const tokenScopes = ["scope1", "scope2"]; const fakeGetToken1 = sinon.fake.returns( Promise.resolve({ token: mockToken1, expiresOn: new Date() }) @@ -80,7 +81,10 @@ describe("AuxiliaryAuthenticationPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const auxiTokenAuthPolicy = createAuxiliaryTokenPolicy(tokenScopes, [mockCredential1, mockCredential2]); + const auxiTokenAuthPolicy = createAuxiliaryTokenPolicy(tokenScopes, [ + mockCredential1, + mockCredential2, + ]); await auxiTokenAuthPolicy.sendRequest(request, next); assert( @@ -90,7 +94,10 @@ describe("AuxiliaryAuthenticationPolicy", function () { }), "fakeGetToken called incorrectly." ); - assert.strictEqual(request.headers.get("x-ms-authorization-auxiliary"), `Bearer ${mockToken1},Bearer ${mockToken2}`); + assert.strictEqual( + request.headers.get("x-ms-authorization-auxiliary"), + `Bearer ${mockToken1},Bearer ${mockToken2}` + ); }); it("only refreshes invalid token during the refresh window", async () => { @@ -115,7 +122,10 @@ describe("AuxiliaryAuthenticationPolicy", function () { await policy.sendRequest(request, next); assert.strictEqual(shortCredential.authCount, 1); assert.strictEqual(longCredential.authCount, 1); - assert.strictEqual(request.headers.get("x-ms-authorization-auxiliary"), `Bearer mock-token,Bearer mock-token`); + assert.strictEqual( + request.headers.get("x-ms-authorization-auxiliary"), + `Bearer mock-token,Bearer mock-token` + ); // The token will remain cached until tokenExpiration - testTokenRefreshBufferMs, so in (5000 - 1000) milliseconds. @@ -212,7 +222,7 @@ class MockRefreshAzureCredential implements TokenCredential { public expiresOnTimestamp: number, public getTokenDelay?: number, public clock?: sinon.SinonFakeTimers - ) { } + ) {} public async getToken(): Promise { this.authCount++; From e7e8e148eeba75371ee6976a3c6a2e9c82f8e642 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Thu, 16 Mar 2023 17:42:45 +0800 Subject: [PATCH 05/23] Update the lint fix --- .../test/auxiliaryAuthenticationPolicy.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts index 01b3dbb85297..d5c3c70322fa 100644 --- a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts +++ b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + import { assert } from "chai"; import * as sinon from "sinon"; import { AccessToken, TokenCredential } from "@azure/core-auth"; From 97d37a6c1ecfd31a33ec6f061c85c848b3aadc33 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Thu, 6 Apr 2023 12:20:47 +0800 Subject: [PATCH 06/23] Update sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts Co-authored-by: Jeff Fisher --- .../src/policies/auxiliaryAuthenticationPolicy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts index 75e714271fe9..2bbc2e2af913 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts @@ -81,7 +81,7 @@ export function auxiliaryAuthenticationPolicy( tokenCyclerMap.set(credential, buildAccessTokenCycler(credential)); } const getAccessToken = tokenCyclerMap.get(credential); - const singalAccessToken = await defaultAuthorizeRequest({ + const singleAccessToken = await defaultAuthorizeRequest({ scopes: Array.isArray(scopes) ? scopes : [scopes], request, getAccessToken, From 1ed399a9eb537f270913f9fcfd8e641cb2b2e75f Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Thu, 6 Apr 2023 12:53:20 +0800 Subject: [PATCH 07/23] Update typo issues --- sdk/core/core-rest-pipeline/src/index.ts | 2 +- .../src/policies/auxiliaryAuthenticationPolicy.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/sdk/core/core-rest-pipeline/src/index.ts b/sdk/core/core-rest-pipeline/src/index.ts index a1f6a3249cec..bdc350c75e16 100644 --- a/sdk/core/core-rest-pipeline/src/index.ts +++ b/sdk/core/core-rest-pipeline/src/index.ts @@ -89,6 +89,6 @@ export { export { ndJsonPolicy, ndJsonPolicyName } from "./policies/ndJsonPolicy"; export { auxiliaryAuthenticationPolicy, - AuxiliaryAuthenticationPolicyNameOptions, + AuxiliaryAuthenticationPolicyOptions, auxiliaryAuthenticationPolicyName, } from "./policies/auxiliaryAuthenticationPolicy"; diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts index 2bbc2e2af913..393b0f5fea95 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts @@ -10,7 +10,7 @@ import { logger as coreLogger } from "../log"; import { AuthorizeRequestOptions } from "./bearerTokenAuthenticationPolicy"; /** - * The programmatic identifier of the bearerTokenAuthenticationPolicy. + * The programmatic identifier of the auxiliaryAuthenticationPolicy. */ export const auxiliaryAuthenticationPolicyName = "auxiliaryAuthenticationPolicy"; const AUTHORIZATION_AUXILIARY_HEADER = "x-ms-authorization-auxiliary"; @@ -18,7 +18,7 @@ const AUTHORIZATION_AUXILIARY_HEADER = "x-ms-authorization-auxiliary"; /** * Options to configure the auxiliaryAuthenticationPolicy */ -export interface AuxiliaryAuthenticationPolicyNameOptions { +export interface AuxiliaryAuthenticationPolicyOptions { /** * The TokenCredential list that can supply the auxiliary authentication token. */ @@ -60,7 +60,7 @@ function buildAccessTokenCycler(credential: TokenCredential) { * then apply it to the Authorization header of a request as a Bearer token. */ export function auxiliaryAuthenticationPolicy( - options: AuxiliaryAuthenticationPolicyNameOptions + options: AuxiliaryAuthenticationPolicyOptions ): PipelinePolicy { const { credentials, scopes } = options; const logger = options.logger || coreLogger; @@ -87,8 +87,8 @@ export function auxiliaryAuthenticationPolicy( getAccessToken, logger, }); - if (singalAccessToken) { - tokenList.push(singalAccessToken); + if (singleAccessToken) { + tokenList.push(singleAccessToken); } } From 0cf0f6f03b4f6f57009d84384bef6283a01c86e7 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Thu, 6 Apr 2023 13:14:57 +0800 Subject: [PATCH 08/23] Update the name to MultiTenantAuthenticationPolicy --- .../review/core-rest-pipeline.api.md | 4 ++-- sdk/core/core-rest-pipeline/src/index.ts | 8 ++++---- ...y.ts => multiTenantAuthenticationPolicy.ts} | 18 +++++++++--------- 3 files changed, 15 insertions(+), 15 deletions(-) rename sdk/core/core-rest-pipeline/src/policies/{auxiliaryAuthenticationPolicy.ts => multiTenantAuthenticationPolicy.ts} (83%) diff --git a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md index 1d26bad7ff8d..1601fe90f2c3 100644 --- a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md +++ b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md @@ -49,13 +49,13 @@ export interface AuthorizeRequestOptions { } // @public -export function auxiliaryAuthenticationPolicy(options: AuxiliaryAuthenticationPolicyNameOptions): PipelinePolicy; +export function auxiliaryAuthenticationPolicy(options: AuxiliaryAuthenticationPolicyOptions): PipelinePolicy; // @public export const auxiliaryAuthenticationPolicyName = "auxiliaryAuthenticationPolicy"; // @public -export interface AuxiliaryAuthenticationPolicyNameOptions { +export interface AuxiliaryAuthenticationPolicyOptions { credentials?: TokenCredential[]; logger?: AzureLogger; scopes: string | string[]; diff --git a/sdk/core/core-rest-pipeline/src/index.ts b/sdk/core/core-rest-pipeline/src/index.ts index bdc350c75e16..a7ac8a541cd8 100644 --- a/sdk/core/core-rest-pipeline/src/index.ts +++ b/sdk/core/core-rest-pipeline/src/index.ts @@ -88,7 +88,7 @@ export { } from "./policies/bearerTokenAuthenticationPolicy"; export { ndJsonPolicy, ndJsonPolicyName } from "./policies/ndJsonPolicy"; export { - auxiliaryAuthenticationPolicy, - AuxiliaryAuthenticationPolicyOptions, - auxiliaryAuthenticationPolicyName, -} from "./policies/auxiliaryAuthenticationPolicy"; + multiTenantAuthenticationPolicy, + MultiTenantAuthenticationPolicyOptions, + multiTenantAuthenticationPolicyName, +} from "./policies/multiTenantAuthenticationPolicy"; diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts similarity index 83% rename from sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts rename to sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts index 393b0f5fea95..67d31129d627 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts @@ -10,17 +10,17 @@ import { logger as coreLogger } from "../log"; import { AuthorizeRequestOptions } from "./bearerTokenAuthenticationPolicy"; /** - * The programmatic identifier of the auxiliaryAuthenticationPolicy. + * The programmatic identifier of the multiTenantAuthenticationPolicy. */ -export const auxiliaryAuthenticationPolicyName = "auxiliaryAuthenticationPolicy"; +export const multiTenantAuthenticationPolicyName = "multiTenantAuthenticationPolicy"; const AUTHORIZATION_AUXILIARY_HEADER = "x-ms-authorization-auxiliary"; /** - * Options to configure the auxiliaryAuthenticationPolicy + * Options to configure the multiTenantAuthenticationPolicy */ -export interface AuxiliaryAuthenticationPolicyOptions { +export interface MultiTenantAuthenticationPolicyOptions { /** - * The TokenCredential list that can supply the auxiliary authentication token. + * The TokenCredential list that can supply the multi tenant authentication token. */ credentials?: TokenCredential[]; /** @@ -52,22 +52,22 @@ async function defaultAuthorizeRequest(options: AuthorizeRequestOptions): Promis } function buildAccessTokenCycler(credential: TokenCredential) { - return credential ? createTokenCycler(credential /* , options */) : () => Promise.resolve(null); + return credential ? createTokenCycler(credential) : () => Promise.resolve(null); } /** * A policy that can request a token from a TokenCredential implementation and * then apply it to the Authorization header of a request as a Bearer token. */ -export function auxiliaryAuthenticationPolicy( - options: AuxiliaryAuthenticationPolicyOptions +export function multiTenantAuthenticationPolicy( + options: MultiTenantAuthenticationPolicyOptions ): PipelinePolicy { const { credentials, scopes } = options; const logger = options.logger || coreLogger; const tokenCyclerMap = new Map(); return { - name: auxiliaryAuthenticationPolicyName, + name: multiTenantAuthenticationPolicyName, async sendRequest(request: PipelineRequest, next: SendRequest): Promise { if (!request.url.toLowerCase().startsWith("https://")) { throw new Error( From aee479abe7236075ff386e9c0a96ae600fd90163 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Thu, 6 Apr 2023 15:28:38 +0800 Subject: [PATCH 09/23] Update the policy name and test cases --- .../multiTenantAuthenticationPolicy.ts | 4 +-- ...> multiTenantAuthenticationPolicy.spec.ts} | 26 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) rename sdk/core/core-rest-pipeline/test/{auxiliaryAuthenticationPolicy.spec.ts => multiTenantAuthenticationPolicy.spec.ts} (88%) diff --git a/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts index 67d31129d627..1801a69e6179 100644 --- a/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts @@ -24,7 +24,7 @@ export interface MultiTenantAuthenticationPolicyOptions { */ credentials?: TokenCredential[]; /** - * The scopes for which the auxiliary token applies. + * The scopes for which the Multi tenant token applies. */ scopes: string | string[]; /** @@ -71,7 +71,7 @@ export function multiTenantAuthenticationPolicy( async sendRequest(request: PipelineRequest, next: SendRequest): Promise { if (!request.url.toLowerCase().startsWith("https://")) { throw new Error( - "Auxiliary token authentication is not permitted for non-TLS protected (non-https) URLs." + "Multi tenant token authentication is not permitted for non-TLS protected (non-https) URLs." ); } diff --git a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts b/sdk/core/core-rest-pipeline/test/multiTenantAuthenticationPolicy.spec.ts similarity index 88% rename from sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts rename to sdk/core/core-rest-pipeline/test/multiTenantAuthenticationPolicy.spec.ts index d5c3c70322fa..afbd264d2934 100644 --- a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationPolicy.spec.ts +++ b/sdk/core/core-rest-pipeline/test/multiTenantAuthenticationPolicy.spec.ts @@ -8,7 +8,7 @@ import { PipelinePolicy, PipelineResponse, SendRequest, - auxiliaryAuthenticationPolicy, + multiTenantAuthenticationPolicy, createHttpHeaders, createPipelineRequest, } from "../src"; @@ -16,7 +16,7 @@ import { DEFAULT_CYCLER_OPTIONS } from "../src/util/tokenCycler"; const { refreshWindowInMs: defaultRefreshWindow } = DEFAULT_CYCLER_OPTIONS; -describe("AuxiliaryAuthenticationPolicy", function () { +describe("MultiTenantAuthenticationPolicy", function () { let clock: sinon.SinonFakeTimers; beforeEach(() => { @@ -45,8 +45,8 @@ describe("AuxiliaryAuthenticationPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const auxiTokenAuthPolicy = createAuxiliaryTokenPolicy(tokenScopes, [mockCredential]); - await auxiTokenAuthPolicy.sendRequest(request, next); + const multiTenantTokenAuthPolicy = createMultiTenantAuthenticationPolicy(tokenScopes, [mockCredential]); + await multiTenantTokenAuthPolicy.sendRequest(request, next); assert( fakeGetToken.calledWith(tokenScopes, { @@ -84,11 +84,11 @@ describe("AuxiliaryAuthenticationPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const auxiTokenAuthPolicy = createAuxiliaryTokenPolicy(tokenScopes, [ + const multiTenantAuthenticationPolicy = createMultiTenantAuthenticationPolicy(tokenScopes, [ mockCredential1, mockCredential2, ]); - await auxiTokenAuthPolicy.sendRequest(request, next); + await multiTenantAuthenticationPolicy.sendRequest(request, next); assert( fakeGetToken1.calledWith(tokenScopes, { @@ -118,7 +118,7 @@ describe("AuxiliaryAuthenticationPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const policy = createAuxiliaryTokenPolicy("test-scope", [shortCredential, longCredential]); + const policy = createMultiTenantAuthenticationPolicy("test-scope", [shortCredential, longCredential]); // The token is cached and remains cached for a bit. await policy.sendRequest(request, next); @@ -165,7 +165,7 @@ describe("AuxiliaryAuthenticationPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const policy = createAuxiliaryTokenPolicy("test-scope", [credential]); + const policy = createMultiTenantAuthenticationPolicy("test-scope", [credential]); credential.shouldThrow = true; @@ -190,7 +190,7 @@ describe("AuxiliaryAuthenticationPolicy", function () { const credential = new MockRefreshAzureCredential(tokenExpiration); const request = createPipelineRequest({ url: "http://example.com" }); - const policy = createAuxiliaryTokenPolicy("test-scope", [credential]); + const policy = createMultiTenantAuthenticationPolicy("test-scope", [credential]); const next = sinon.stub, ReturnType>(); let error: Error | undefined; @@ -202,15 +202,15 @@ describe("AuxiliaryAuthenticationPolicy", function () { assert.equal( error?.message, - "Auxiliary token authentication is not permitted for non-TLS protected (non-https) URLs." + "Multi tenant token authentication is not permitted for non-TLS protected (non-https) URLs." ); }); - function createAuxiliaryTokenPolicy( + function createMultiTenantAuthenticationPolicy( scopes: string | string[], credentials: TokenCredential[] ): PipelinePolicy { - return auxiliaryAuthenticationPolicy({ + return multiTenantAuthenticationPolicy({ scopes, credentials, }); @@ -225,7 +225,7 @@ class MockRefreshAzureCredential implements TokenCredential { public expiresOnTimestamp: number, public getTokenDelay?: number, public clock?: sinon.SinonFakeTimers - ) {} + ) { } public async getToken(): Promise { this.authCount++; From 1bbbb24e7361b0b04ce39d45045e27bb584f9bba Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Tue, 25 Apr 2023 15:25:17 +0800 Subject: [PATCH 10/23] Update the test cases and policy code --- .../review/core-rest-pipeline.api.md | 26 +++---- .../multiTenantAuthenticationPolicy.ts | 73 +++++-------------- .../multiTenantAuthenticationPolicy.spec.ts | 10 +-- 3 files changed, 38 insertions(+), 71 deletions(-) diff --git a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md index 1601fe90f2c3..75feb8c3a352 100644 --- a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md +++ b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md @@ -48,19 +48,6 @@ export interface AuthorizeRequestOptions { scopes: string[]; } -// @public -export function auxiliaryAuthenticationPolicy(options: AuxiliaryAuthenticationPolicyOptions): PipelinePolicy; - -// @public -export const auxiliaryAuthenticationPolicyName = "auxiliaryAuthenticationPolicy"; - -// @public -export interface AuxiliaryAuthenticationPolicyOptions { - credentials?: TokenCredential[]; - logger?: AzureLogger; - scopes: string | string[]; -} - // @public export function bearerTokenAuthenticationPolicy(options: BearerTokenAuthenticationPolicyOptions): PipelinePolicy; @@ -185,6 +172,19 @@ export interface LogPolicyOptions { logger?: Debugger; } +// @public (undocumented) +export function multiTenantAuthenticationPolicy(options: MultiTenantAuthenticationPolicyOptions): PipelinePolicy; + +// @public (undocumented) +export const multiTenantAuthenticationPolicyName = "multiTenantAuthenticationPolicy"; + +// @public (undocumented) +export interface MultiTenantAuthenticationPolicyOptions { + credentials?: TokenCredential[]; + logger?: AzureLogger; + scopes: string | string[]; +} + // @public export function ndJsonPolicy(): PipelinePolicy; diff --git a/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts index 1801a69e6179..425d3300b6b6 100644 --- a/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts @@ -5,19 +5,14 @@ import { GetTokenOptions, TokenCredential } from "@azure/core-auth"; import { AzureLogger } from "@azure/logger"; import { PipelineRequest, PipelineResponse, SendRequest } from "../interfaces"; import { PipelinePolicy } from "../pipeline"; -import { createTokenCycler } from "../util/tokenCycler"; +import { AccessTokenGetter, createTokenCycler } from "../util/tokenCycler"; import { logger as coreLogger } from "../log"; import { AuthorizeRequestOptions } from "./bearerTokenAuthenticationPolicy"; -/** - * The programmatic identifier of the multiTenantAuthenticationPolicy. - */ export const multiTenantAuthenticationPolicyName = "multiTenantAuthenticationPolicy"; const AUTHORIZATION_AUXILIARY_HEADER = "x-ms-authorization-auxiliary"; -/** - * Options to configure the multiTenantAuthenticationPolicy - */ + export interface MultiTenantAuthenticationPolicyOptions { /** * The TokenCredential list that can supply the multi tenant authentication token. @@ -33,84 +28,56 @@ export interface MultiTenantAuthenticationPolicyOptions { logger?: AzureLogger; } -/** - * Default authorize request handler - */ -async function defaultAuthorizeRequest(options: AuthorizeRequestOptions): Promise { +type NullableString = string | null | undefined; + +async function sendAuthorizeRequest(options: AuthorizeRequestOptions): Promise { const { scopes, getAccessToken, request } = options; const getTokenOptions: GetTokenOptions = { abortSignal: request.abortSignal, tracingOptions: request.tracingOptions, }; - const accessToken = await getAccessToken(scopes, getTokenOptions); - - if (accessToken) { - return accessToken.token; - } - return null; + return (await getAccessToken(scopes, getTokenOptions))?.token; } -function buildAccessTokenCycler(credential: TokenCredential) { - return credential ? createTokenCycler(credential) : () => Promise.resolve(null); -} - -/** - * A policy that can request a token from a TokenCredential implementation and - * then apply it to the Authorization header of a request as a Bearer token. - */ export function multiTenantAuthenticationPolicy( options: MultiTenantAuthenticationPolicyOptions ): PipelinePolicy { const { credentials, scopes } = options; const logger = options.logger || coreLogger; - const tokenCyclerMap = new Map(); + const tokenCyclerMap = new WeakMap(); return { name: multiTenantAuthenticationPolicyName, async sendRequest(request: PipelineRequest, next: SendRequest): Promise { if (!request.url.toLowerCase().startsWith("https://")) { throw new Error( - "Multi tenant token authentication is not permitted for non-TLS protected (non-https) URLs." + "Multi-tenant token authentication is not permitted for non-TLS protected (non-https) URLs." ); } + if (!credentials || credentials.length === 0) { + return next(request); + } - const tokenList: string[] = []; - for (const credential of credentials ?? []) { + const tokenPromises: Promise[] = []; + for (const credential of credentials) { if (!tokenCyclerMap.has(credential)) { - tokenCyclerMap.set(credential, buildAccessTokenCycler(credential)); + tokenCyclerMap.set(credential, createTokenCycler(credential)); } - const getAccessToken = tokenCyclerMap.get(credential); - const singleAccessToken = await defaultAuthorizeRequest({ + tokenPromises.push(sendAuthorizeRequest({ scopes: Array.isArray(scopes) ? scopes : [scopes], request, - getAccessToken, + getAccessToken: tokenCyclerMap.get(credential)!, logger, - }); - if (singleAccessToken) { - tokenList.push(singleAccessToken); - } + })); } - + const auxiliaryTokens: NullableString[] = await Promise.all(tokenPromises); request.headers.set( AUTHORIZATION_AUXILIARY_HEADER, - tokenList.map((token) => `Bearer ${token}`).join(",") + auxiliaryTokens.filter(token => Boolean(token)).map((token) => `Bearer ${token}`).join(", ") ); - let response: PipelineResponse; - let error: Error | undefined; - try { - response = await next(request); - } catch (err: any) { - error = err; - response = err.response; - } - - if (error) { - throw error; - } else { - return response; - } + return next(request); }, }; } diff --git a/sdk/core/core-rest-pipeline/test/multiTenantAuthenticationPolicy.spec.ts b/sdk/core/core-rest-pipeline/test/multiTenantAuthenticationPolicy.spec.ts index afbd264d2934..cd88f7ed8879 100644 --- a/sdk/core/core-rest-pipeline/test/multiTenantAuthenticationPolicy.spec.ts +++ b/sdk/core/core-rest-pipeline/test/multiTenantAuthenticationPolicy.spec.ts @@ -84,11 +84,11 @@ describe("MultiTenantAuthenticationPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const multiTenantAuthenticationPolicy = createMultiTenantAuthenticationPolicy(tokenScopes, [ + const mockMultiTenantAuthenticationPolicy = createMultiTenantAuthenticationPolicy(tokenScopes, [ mockCredential1, mockCredential2, ]); - await multiTenantAuthenticationPolicy.sendRequest(request, next); + await mockMultiTenantAuthenticationPolicy.sendRequest(request, next); assert( fakeGetToken1.calledWith(tokenScopes, { @@ -99,7 +99,7 @@ describe("MultiTenantAuthenticationPolicy", function () { ); assert.strictEqual( request.headers.get("x-ms-authorization-auxiliary"), - `Bearer ${mockToken1},Bearer ${mockToken2}` + `Bearer ${mockToken1}, Bearer ${mockToken2}` ); }); @@ -127,7 +127,7 @@ describe("MultiTenantAuthenticationPolicy", function () { assert.strictEqual(longCredential.authCount, 1); assert.strictEqual( request.headers.get("x-ms-authorization-auxiliary"), - `Bearer mock-token,Bearer mock-token` + `Bearer mock-token, Bearer mock-token` ); // The token will remain cached until tokenExpiration - testTokenRefreshBufferMs, so in (5000 - 1000) milliseconds. @@ -202,7 +202,7 @@ describe("MultiTenantAuthenticationPolicy", function () { assert.equal( error?.message, - "Multi tenant token authentication is not permitted for non-TLS protected (non-https) URLs." + "Multi-tenant token authentication is not permitted for non-TLS protected (non-https) URLs." ); }); From c5dd52daca3c0d94d996eb7ae84a2e4cfce34af1 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Tue, 25 Apr 2023 15:58:50 +0800 Subject: [PATCH 11/23] Update the documents for this policy --- .../review/core-rest-pipeline.api.md | 6 +++--- .../policies/multiTenantAuthenticationPolicy.ts | 17 ++++++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md index 75feb8c3a352..aad3ca40dff0 100644 --- a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md +++ b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md @@ -172,13 +172,13 @@ export interface LogPolicyOptions { logger?: Debugger; } -// @public (undocumented) +// @public export function multiTenantAuthenticationPolicy(options: MultiTenantAuthenticationPolicyOptions): PipelinePolicy; -// @public (undocumented) +// @public export const multiTenantAuthenticationPolicyName = "multiTenantAuthenticationPolicy"; -// @public (undocumented) +// @public export interface MultiTenantAuthenticationPolicyOptions { credentials?: TokenCredential[]; logger?: AzureLogger; diff --git a/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts index 425d3300b6b6..c5848cca2fa3 100644 --- a/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts @@ -9,17 +9,23 @@ import { AccessTokenGetter, createTokenCycler } from "../util/tokenCycler"; import { logger as coreLogger } from "../log"; import { AuthorizeRequestOptions } from "./bearerTokenAuthenticationPolicy"; +/** + * The programmatic identifier of the multiTenantAuthenticationPolicy. + */ export const multiTenantAuthenticationPolicyName = "multiTenantAuthenticationPolicy"; const AUTHORIZATION_AUXILIARY_HEADER = "x-ms-authorization-auxiliary"; - +/** + * Options to configure the multiTenantAuthenticationPolicy + */ export interface MultiTenantAuthenticationPolicyOptions { /** - * The TokenCredential list that can supply the multi tenant authentication token. + * TokenCredential list used to get token from auxiliary tenants and + * one credential for each tenant the client may need to access */ credentials?: TokenCredential[]; /** - * The scopes for which the Multi tenant token applies. + * Scopes depend on the cloud your application runs in */ scopes: string | string[]; /** @@ -40,6 +46,11 @@ async function sendAuthorizeRequest(options: AuthorizeRequestOptions): Promise Date: Tue, 25 Apr 2023 16:29:41 +0800 Subject: [PATCH 12/23] Update the policy name and CHANGELOG --- sdk/core/core-rest-pipeline/CHANGELOG.md | 3 ++- sdk/core/core-rest-pipeline/src/index.ts | 6 ++--- ...=> auxiliaryAuthenticationHeaderPolicy.ts} | 21 ++++++++-------- ...xiliaryAuthenticationHeaderPolicy.spec.ts} | 24 +++++++++---------- 4 files changed, 28 insertions(+), 26 deletions(-) rename sdk/core/core-rest-pipeline/src/policies/{multiTenantAuthenticationPolicy.ts => auxiliaryAuthenticationHeaderPolicy.ts} (77%) rename sdk/core/core-rest-pipeline/test/{multiTenantAuthenticationPolicy.spec.ts => auxiliaryAuthenticationHeaderPolicy.spec.ts} (88%) diff --git a/sdk/core/core-rest-pipeline/CHANGELOG.md b/sdk/core/core-rest-pipeline/CHANGELOG.md index 6fa0149c21f3..dd994aab85d3 100644 --- a/sdk/core/core-rest-pipeline/CHANGELOG.md +++ b/sdk/core/core-rest-pipeline/CHANGELOG.md @@ -4,6 +4,8 @@ ### Features Added +- Add a policy `auxiliaryAuthenticationHeaderPolicy` for external tokens to `x-ms-authorization-auxiliary` header. This header will be used when creating a cross-tenant application we may need to handle authentication requests for resources that are in different tenants. [PR #25270](https://github.com/Azure/azure-sdk-for-js/pull/25270) + ### Breaking Changes ### Bugs Fixed @@ -12,7 +14,6 @@ ## 1.10.3 (2023-04-06) -- Added AuxiliaryAuthenticationPolicy ### Other Changes - Migrate to use core-util UUID helper [PR# 25413](https://github.com/Azure/azure-sdk-for-js/pull/25413) diff --git a/sdk/core/core-rest-pipeline/src/index.ts b/sdk/core/core-rest-pipeline/src/index.ts index a7ac8a541cd8..452c12ac5b20 100644 --- a/sdk/core/core-rest-pipeline/src/index.ts +++ b/sdk/core/core-rest-pipeline/src/index.ts @@ -88,7 +88,7 @@ export { } from "./policies/bearerTokenAuthenticationPolicy"; export { ndJsonPolicy, ndJsonPolicyName } from "./policies/ndJsonPolicy"; export { - multiTenantAuthenticationPolicy, - MultiTenantAuthenticationPolicyOptions, - multiTenantAuthenticationPolicyName, + auxiliaryAuthenticationHeaderPolicy, + AuxiliaryAuthenticationHeaderPolicyOptions, + auxiliaryAuthenticationHeaderPolicyName, } from "./policies/multiTenantAuthenticationPolicy"; diff --git a/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts similarity index 77% rename from sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts rename to sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts index c5848cca2fa3..2d97e3254008 100644 --- a/sdk/core/core-rest-pipeline/src/policies/multiTenantAuthenticationPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts @@ -10,15 +10,15 @@ import { logger as coreLogger } from "../log"; import { AuthorizeRequestOptions } from "./bearerTokenAuthenticationPolicy"; /** - * The programmatic identifier of the multiTenantAuthenticationPolicy. + * The programmatic identifier of the auxiliaryAuthenticationHeaderPolicy. */ -export const multiTenantAuthenticationPolicyName = "multiTenantAuthenticationPolicy"; +export const auxiliaryAuthenticationHeaderPolicyName = "auxiliaryAuthenticationHeaderPolicy"; const AUTHORIZATION_AUXILIARY_HEADER = "x-ms-authorization-auxiliary"; /** - * Options to configure the multiTenantAuthenticationPolicy + * Options to configure the auxiliaryAuthenticationHeaderPolicy */ -export interface MultiTenantAuthenticationPolicyOptions { +export interface AuxiliaryAuthenticationHeaderPolicyOptions { /** * TokenCredential list used to get token from auxiliary tenants and * one credential for each tenant the client may need to access @@ -47,23 +47,24 @@ async function sendAuthorizeRequest(options: AuthorizeRequestOptions): Promise(); return { - name: multiTenantAuthenticationPolicyName, + name: auxiliaryAuthenticationHeaderPolicyName, async sendRequest(request: PipelineRequest, next: SendRequest): Promise { if (!request.url.toLowerCase().startsWith("https://")) { throw new Error( - "Multi-tenant token authentication is not permitted for non-TLS protected (non-https) URLs." + "Bearer token authentication for auxiliary header is not permitted for non-TLS protected (non-https) URLs." ); } if (!credentials || credentials.length === 0) { diff --git a/sdk/core/core-rest-pipeline/test/multiTenantAuthenticationPolicy.spec.ts b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts similarity index 88% rename from sdk/core/core-rest-pipeline/test/multiTenantAuthenticationPolicy.spec.ts rename to sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts index cd88f7ed8879..dbfd25479932 100644 --- a/sdk/core/core-rest-pipeline/test/multiTenantAuthenticationPolicy.spec.ts +++ b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts @@ -8,7 +8,7 @@ import { PipelinePolicy, PipelineResponse, SendRequest, - multiTenantAuthenticationPolicy, + auxiliaryAuthenticationHeaderPolicy, createHttpHeaders, createPipelineRequest, } from "../src"; @@ -16,7 +16,7 @@ import { DEFAULT_CYCLER_OPTIONS } from "../src/util/tokenCycler"; const { refreshWindowInMs: defaultRefreshWindow } = DEFAULT_CYCLER_OPTIONS; -describe("MultiTenantAuthenticationPolicy", function () { +describe("AuxiliaryAuthenticationHeaderPolicy", function () { let clock: sinon.SinonFakeTimers; beforeEach(() => { @@ -45,8 +45,8 @@ describe("MultiTenantAuthenticationPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const multiTenantTokenAuthPolicy = createMultiTenantAuthenticationPolicy(tokenScopes, [mockCredential]); - await multiTenantTokenAuthPolicy.sendRequest(request, next); + const auxiliaryAuthenticationHeaderPolicy = createAuxiliaryAuthenticationHeaderPolicy(tokenScopes, [mockCredential]); + await auxiliaryAuthenticationHeaderPolicy.sendRequest(request, next); assert( fakeGetToken.calledWith(tokenScopes, { @@ -84,11 +84,11 @@ describe("MultiTenantAuthenticationPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const mockMultiTenantAuthenticationPolicy = createMultiTenantAuthenticationPolicy(tokenScopes, [ + const mockAuxiliaryAuthenticationHeaderPolicy = createAuxiliaryAuthenticationHeaderPolicy(tokenScopes, [ mockCredential1, mockCredential2, ]); - await mockMultiTenantAuthenticationPolicy.sendRequest(request, next); + await mockAuxiliaryAuthenticationHeaderPolicy.sendRequest(request, next); assert( fakeGetToken1.calledWith(tokenScopes, { @@ -118,7 +118,7 @@ describe("MultiTenantAuthenticationPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const policy = createMultiTenantAuthenticationPolicy("test-scope", [shortCredential, longCredential]); + const policy = createAuxiliaryAuthenticationHeaderPolicy("test-scope", [shortCredential, longCredential]); // The token is cached and remains cached for a bit. await policy.sendRequest(request, next); @@ -165,7 +165,7 @@ describe("MultiTenantAuthenticationPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const policy = createMultiTenantAuthenticationPolicy("test-scope", [credential]); + const policy = createAuxiliaryAuthenticationHeaderPolicy("test-scope", [credential]); credential.shouldThrow = true; @@ -190,7 +190,7 @@ describe("MultiTenantAuthenticationPolicy", function () { const credential = new MockRefreshAzureCredential(tokenExpiration); const request = createPipelineRequest({ url: "http://example.com" }); - const policy = createMultiTenantAuthenticationPolicy("test-scope", [credential]); + const policy = createAuxiliaryAuthenticationHeaderPolicy("test-scope", [credential]); const next = sinon.stub, ReturnType>(); let error: Error | undefined; @@ -202,15 +202,15 @@ describe("MultiTenantAuthenticationPolicy", function () { assert.equal( error?.message, - "Multi-tenant token authentication is not permitted for non-TLS protected (non-https) URLs." + "Bearer token authentication for auxiliary header is not permitted for non-TLS protected (non-https) URLs." ); }); - function createMultiTenantAuthenticationPolicy( + function createAuxiliaryAuthenticationHeaderPolicy( scopes: string | string[], credentials: TokenCredential[] ): PipelinePolicy { - return multiTenantAuthenticationPolicy({ + return auxiliaryAuthenticationHeaderPolicy({ scopes, credentials, }); From e1aad772f02981389e58d4a05182bbcf5d98d7f7 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Tue, 25 Apr 2023 16:31:53 +0800 Subject: [PATCH 13/23] Update the index --- .../review/core-rest-pipeline.api.md | 26 +++++++++---------- sdk/core/core-rest-pipeline/src/index.ts | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md index aad3ca40dff0..acb62d6d7dbe 100644 --- a/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md +++ b/sdk/core/core-rest-pipeline/review/core-rest-pipeline.api.md @@ -48,6 +48,19 @@ export interface AuthorizeRequestOptions { scopes: string[]; } +// @public +export function auxiliaryAuthenticationHeaderPolicy(options: AuxiliaryAuthenticationHeaderPolicyOptions): PipelinePolicy; + +// @public +export const auxiliaryAuthenticationHeaderPolicyName = "auxiliaryAuthenticationHeaderPolicy"; + +// @public +export interface AuxiliaryAuthenticationHeaderPolicyOptions { + credentials?: TokenCredential[]; + logger?: AzureLogger; + scopes: string | string[]; +} + // @public export function bearerTokenAuthenticationPolicy(options: BearerTokenAuthenticationPolicyOptions): PipelinePolicy; @@ -172,19 +185,6 @@ export interface LogPolicyOptions { logger?: Debugger; } -// @public -export function multiTenantAuthenticationPolicy(options: MultiTenantAuthenticationPolicyOptions): PipelinePolicy; - -// @public -export const multiTenantAuthenticationPolicyName = "multiTenantAuthenticationPolicy"; - -// @public -export interface MultiTenantAuthenticationPolicyOptions { - credentials?: TokenCredential[]; - logger?: AzureLogger; - scopes: string | string[]; -} - // @public export function ndJsonPolicy(): PipelinePolicy; diff --git a/sdk/core/core-rest-pipeline/src/index.ts b/sdk/core/core-rest-pipeline/src/index.ts index 452c12ac5b20..045b9cd132d4 100644 --- a/sdk/core/core-rest-pipeline/src/index.ts +++ b/sdk/core/core-rest-pipeline/src/index.ts @@ -91,4 +91,4 @@ export { auxiliaryAuthenticationHeaderPolicy, AuxiliaryAuthenticationHeaderPolicyOptions, auxiliaryAuthenticationHeaderPolicyName, -} from "./policies/multiTenantAuthenticationPolicy"; +} from "./policies/auxiliaryAuthenticationHeaderPolicy"; From 05150e15af85e7d80b656a5216242dea5e833d5f Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Tue, 25 Apr 2023 16:33:25 +0800 Subject: [PATCH 14/23] Fix the linter issue --- .../test/auxiliaryAuthenticationHeaderPolicy.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts index dbfd25479932..d352acc088d0 100644 --- a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts +++ b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts @@ -45,8 +45,8 @@ describe("AuxiliaryAuthenticationHeaderPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const auxiliaryAuthenticationHeaderPolicy = createAuxiliaryAuthenticationHeaderPolicy(tokenScopes, [mockCredential]); - await auxiliaryAuthenticationHeaderPolicy.sendRequest(request, next); + const mockAuxiliaryAuthenticationHeaderPolicy = createAuxiliaryAuthenticationHeaderPolicy(tokenScopes, [mockCredential]); + await mockAuxiliaryAuthenticationHeaderPolicy.sendRequest(request, next); assert( fakeGetToken.calledWith(tokenScopes, { From 08e59acc03bc67f379e99a5aa19140a847537961 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Tue, 25 Apr 2023 17:06:35 +0800 Subject: [PATCH 15/23] Update the format --- sdk/core/core-rest-pipeline/CHANGELOG.md | 2 +- .../auxiliaryAuthenticationHeaderPolicy.ts | 23 +++++++++++-------- ...uxiliaryAuthenticationHeaderPolicy.spec.ts | 20 ++++++++++------ 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/sdk/core/core-rest-pipeline/CHANGELOG.md b/sdk/core/core-rest-pipeline/CHANGELOG.md index dd994aab85d3..a996008bf09b 100644 --- a/sdk/core/core-rest-pipeline/CHANGELOG.md +++ b/sdk/core/core-rest-pipeline/CHANGELOG.md @@ -1,6 +1,6 @@ # Release History -## 1.10.4 (Unreleased) +## 1.11.0 (Unreleased) ### Features Added diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts index 2d97e3254008..13f19874c8e2 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts @@ -20,7 +20,7 @@ const AUTHORIZATION_AUXILIARY_HEADER = "x-ms-authorization-auxiliary"; */ export interface AuxiliaryAuthenticationHeaderPolicyOptions { /** - * TokenCredential list used to get token from auxiliary tenants and + * TokenCredential list used to get token from auxiliary tenants and * one credential for each tenant the client may need to access */ credentials?: TokenCredential[]; @@ -48,7 +48,7 @@ async function sendAuthorizeRequest(options: AuthorizeRequestOptions): Promise Boolean(token)).map((token) => `Bearer ${token}`).join(", ") + auxiliaryTokens + .filter((token) => Boolean(token)) + .map((token) => `Bearer ${token}`) + .join(", ") ); return next(request); diff --git a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts index d352acc088d0..d80365a41ebe 100644 --- a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts +++ b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts @@ -45,7 +45,10 @@ describe("AuxiliaryAuthenticationHeaderPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const mockAuxiliaryAuthenticationHeaderPolicy = createAuxiliaryAuthenticationHeaderPolicy(tokenScopes, [mockCredential]); + const mockAuxiliaryAuthenticationHeaderPolicy = createAuxiliaryAuthenticationHeaderPolicy( + tokenScopes, + [mockCredential] + ); await mockAuxiliaryAuthenticationHeaderPolicy.sendRequest(request, next); assert( @@ -84,10 +87,10 @@ describe("AuxiliaryAuthenticationHeaderPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const mockAuxiliaryAuthenticationHeaderPolicy = createAuxiliaryAuthenticationHeaderPolicy(tokenScopes, [ - mockCredential1, - mockCredential2, - ]); + const mockAuxiliaryAuthenticationHeaderPolicy = createAuxiliaryAuthenticationHeaderPolicy( + tokenScopes, + [mockCredential1, mockCredential2] + ); await mockAuxiliaryAuthenticationHeaderPolicy.sendRequest(request, next); assert( @@ -118,7 +121,10 @@ describe("AuxiliaryAuthenticationHeaderPolicy", function () { const next = sinon.stub, ReturnType>(); next.resolves(successResponse); - const policy = createAuxiliaryAuthenticationHeaderPolicy("test-scope", [shortCredential, longCredential]); + const policy = createAuxiliaryAuthenticationHeaderPolicy("test-scope", [ + shortCredential, + longCredential, + ]); // The token is cached and remains cached for a bit. await policy.sendRequest(request, next); @@ -225,7 +231,7 @@ class MockRefreshAzureCredential implements TokenCredential { public expiresOnTimestamp: number, public getTokenDelay?: number, public clock?: sinon.SinonFakeTimers - ) { } + ) {} public async getToken(): Promise { this.authCount++; From 1d3f912e99f717ad09dd0cf4e27bd611910b4a1b Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Wed, 26 Apr 2023 16:56:00 +0800 Subject: [PATCH 16/23] Skip header set if None of the auxiliary tokens are valid --- .../src/policies/auxiliaryAuthenticationHeaderPolicy.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts index 13f19874c8e2..05df70d5ba50 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts @@ -68,6 +68,7 @@ export function auxiliaryAuthenticationHeaderPolicy( ); } if (!credentials || credentials.length === 0) { + logger.info(`Skip ${auxiliaryAuthenticationHeaderPolicyName} due to empty credentials`); return next(request); } @@ -85,11 +86,14 @@ export function auxiliaryAuthenticationHeaderPolicy( }) ); } - const auxiliaryTokens: NullableString[] = await Promise.all(tokenPromises); + const auxiliaryTokens: NullableString[] = (await Promise.all(tokenPromises)).filter((token) => Boolean(token)); + if (!auxiliaryTokens || auxiliaryTokens.length === 0) { + logger.warning(`None of the auxiliary tokens are valid and skip to set ${AUTHORIZATION_AUXILIARY_HEADER} header`); + return next(request); + } request.headers.set( AUTHORIZATION_AUXILIARY_HEADER, auxiliaryTokens - .filter((token) => Boolean(token)) .map((token) => `Bearer ${token}`) .join(", ") ); From 2d9c935312a4a8ca55a7a14b5a5b04627fe3f263 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Wed, 26 Apr 2023 17:05:14 +0800 Subject: [PATCH 17/23] Update the test cases --- .../auxiliaryAuthenticationHeaderPolicy.ts | 12 +++--- ...uxiliaryAuthenticationHeaderPolicy.spec.ts | 40 +++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts index 05df70d5ba50..fb7e9d85b0a9 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts @@ -86,16 +86,18 @@ export function auxiliaryAuthenticationHeaderPolicy( }) ); } - const auxiliaryTokens: NullableString[] = (await Promise.all(tokenPromises)).filter((token) => Boolean(token)); + const auxiliaryTokens: NullableString[] = (await Promise.all(tokenPromises)).filter((token) => + Boolean(token) + ); if (!auxiliaryTokens || auxiliaryTokens.length === 0) { - logger.warning(`None of the auxiliary tokens are valid and skip to set ${AUTHORIZATION_AUXILIARY_HEADER} header`); + logger.warning( + `None of the auxiliary tokens are valid and skip to set ${AUTHORIZATION_AUXILIARY_HEADER} header` + ); return next(request); } request.headers.set( AUTHORIZATION_AUXILIARY_HEADER, - auxiliaryTokens - .map((token) => `Bearer ${token}`) - .join(", ") + auxiliaryTokens.map((token) => `Bearer ${token}`).join(", ") ); return next(request); diff --git a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts index d80365a41ebe..42d27de83193 100644 --- a/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts +++ b/sdk/core/core-rest-pipeline/test/auxiliaryAuthenticationHeaderPolicy.spec.ts @@ -212,6 +212,46 @@ describe("AuxiliaryAuthenticationHeaderPolicy", function () { ); }); + it("should not add auxiliary header if all tokens are invalid", async function () { + const tokenScopes = ["scope1", "scope2"]; + const fakeGetToken1 = sinon.fake.returns( + Promise.resolve({ token: null, expiresOn: new Date() }) + ); + const fakeGetToken2 = sinon.fake.returns( + Promise.resolve({ token: null, expiresOn: new Date() }) + ); + const mockCredential1: TokenCredential = { + getToken: fakeGetToken1, + }; + const mockCredential2: TokenCredential = { + getToken: fakeGetToken2, + }; + + const request = createPipelineRequest({ url: "https://example.com" }); + const successResponse: PipelineResponse = { + headers: createHttpHeaders(), + request, + status: 200, + }; + const next = sinon.stub, ReturnType>(); + next.resolves(successResponse); + + const mockAuxiliaryAuthenticationHeaderPolicy = createAuxiliaryAuthenticationHeaderPolicy( + tokenScopes, + [mockCredential1, mockCredential2] + ); + await mockAuxiliaryAuthenticationHeaderPolicy.sendRequest(request, next); + + assert( + fakeGetToken1.calledWith(tokenScopes, { + abortSignal: undefined, + tracingOptions: undefined, + }), + "fakeGetToken called incorrectly." + ); + assert.isUndefined(request.headers.get("x-ms-authorization-auxiliary")); + }); + function createAuxiliaryAuthenticationHeaderPolicy( scopes: string | string[], credentials: TokenCredential[] From 7bfe2a71058fa9f143af56e834ebf8f4e27d7a95 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Thu, 27 Apr 2023 08:07:25 +0800 Subject: [PATCH 18/23] Update sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts --- .../src/policies/auxiliaryAuthenticationHeaderPolicy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts index fb7e9d85b0a9..dc5bf80034f2 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts @@ -89,7 +89,7 @@ export function auxiliaryAuthenticationHeaderPolicy( const auxiliaryTokens: NullableString[] = (await Promise.all(tokenPromises)).filter((token) => Boolean(token) ); - if (!auxiliaryTokens || auxiliaryTokens.length === 0) { + if (auxiliaryTokens.length === 0) { logger.warning( `None of the auxiliary tokens are valid and skip to set ${AUTHORIZATION_AUXILIARY_HEADER} header` ); From e2e7f7f12c4d65430c2998f9a92e6cc7e1328549 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Sat, 6 May 2023 10:59:19 +0800 Subject: [PATCH 19/23] Update sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts Co-authored-by: Jeff Fisher --- .../src/policies/auxiliaryAuthenticationHeaderPolicy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts index dc5bf80034f2..33abb073081f 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts @@ -91,7 +91,7 @@ export function auxiliaryAuthenticationHeaderPolicy( ); if (auxiliaryTokens.length === 0) { logger.warning( - `None of the auxiliary tokens are valid and skip to set ${AUTHORIZATION_AUXILIARY_HEADER} header` + `None of the auxiliary tokens are valid. ${AUTHORIZATION_AUXILIARY_HEADER} header will not be set.` ); return next(request); } From 8ac016e4ed3bfeddf551378bdba47fd77a653598 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Sat, 6 May 2023 11:02:57 +0800 Subject: [PATCH 20/23] Update sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts Co-authored-by: Jeff Fisher --- .../src/policies/auxiliaryAuthenticationHeaderPolicy.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts index 33abb073081f..79d11133423a 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts @@ -74,14 +74,16 @@ export function auxiliaryAuthenticationHeaderPolicy( const tokenPromises: Promise[] = []; for (const credential of credentials) { - if (!tokenCyclerMap.has(credential)) { - tokenCyclerMap.set(credential, createTokenCycler(credential)); + let getAccessToken = tokenCyclerMap.get(credential); + if (!getAccessToken) { + getAccessToken = createTokenCycler(credential); + tokenCyclerMap.set(credential, getAccessToken); } tokenPromises.push( sendAuthorizeRequest({ scopes: Array.isArray(scopes) ? scopes : [scopes], request, - getAccessToken: tokenCyclerMap.get(credential)!, + getAccessToken, logger, }) ); From 2d9646a1c187ec41b663efe061a602e007785fc0 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Sat, 6 May 2023 11:03:38 +0800 Subject: [PATCH 21/23] Update sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts Co-authored-by: Jeff Fisher --- .../src/policies/auxiliaryAuthenticationHeaderPolicy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts index 79d11133423a..1cdf01354d76 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts @@ -88,7 +88,7 @@ export function auxiliaryAuthenticationHeaderPolicy( }) ); } - const auxiliaryTokens: NullableString[] = (await Promise.all(tokenPromises)).filter((token) => + const auxiliaryTokens = (await Promise.all(tokenPromises)).filter((token): token is string => Boolean(token) ); if (auxiliaryTokens.length === 0) { From 6f110a66a646321a208c84aeb0d3543c8455f856 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Sat, 6 May 2023 11:03:59 +0800 Subject: [PATCH 22/23] Update sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts Co-authored-by: Jeff Fisher --- .../src/policies/auxiliaryAuthenticationHeaderPolicy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts index 1cdf01354d76..f02a07f2209e 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts @@ -43,7 +43,7 @@ async function sendAuthorizeRequest(options: AuthorizeRequestOptions): Promise Date: Sat, 6 May 2023 11:16:08 +0800 Subject: [PATCH 23/23] Update format --- .../auxiliaryAuthenticationHeaderPolicy.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts index f02a07f2209e..6d6f8d9558aa 100644 --- a/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts +++ b/sdk/core/core-rest-pipeline/src/policies/auxiliaryAuthenticationHeaderPolicy.ts @@ -34,9 +34,7 @@ export interface AuxiliaryAuthenticationHeaderPolicyOptions { logger?: AzureLogger; } -type NullableString = string | null | undefined; - -async function sendAuthorizeRequest(options: AuthorizeRequestOptions): Promise { +async function sendAuthorizeRequest(options: AuthorizeRequestOptions): Promise { const { scopes, getAccessToken, request } = options; const getTokenOptions: GetTokenOptions = { abortSignal: request.abortSignal, @@ -68,16 +66,18 @@ export function auxiliaryAuthenticationHeaderPolicy( ); } if (!credentials || credentials.length === 0) { - logger.info(`Skip ${auxiliaryAuthenticationHeaderPolicyName} due to empty credentials`); + logger.info( + `${auxiliaryAuthenticationHeaderPolicyName} header will not be set due to empty credentials.` + ); return next(request); } - const tokenPromises: Promise[] = []; + const tokenPromises: Promise[] = []; for (const credential of credentials) { let getAccessToken = tokenCyclerMap.get(credential); if (!getAccessToken) { - getAccessToken = createTokenCycler(credential); - tokenCyclerMap.set(credential, getAccessToken); + getAccessToken = createTokenCycler(credential); + tokenCyclerMap.set(credential, getAccessToken); } tokenPromises.push( sendAuthorizeRequest({ @@ -88,9 +88,7 @@ export function auxiliaryAuthenticationHeaderPolicy( }) ); } - const auxiliaryTokens = (await Promise.all(tokenPromises)).filter((token): token is string => - Boolean(token) - ); + const auxiliaryTokens = (await Promise.all(tokenPromises)).filter((token) => Boolean(token)); if (auxiliaryTokens.length === 0) { logger.warning( `None of the auxiliary tokens are valid. ${AUTHORIZATION_AUXILIARY_HEADER} header will not be set.`