From 435776e50b77acd2dc2881dfa9761766562364d1 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Wed, 3 Mar 2021 14:41:05 +0000 Subject: [PATCH] processing challenges even if we already have a token --- .../bearerTokenAuthenticationPolicy.ts | 61 +++++----- ...TokenAuthenticationPolicyChallenge.spec.ts | 105 ++++++++++++++++++ 2 files changed, 134 insertions(+), 32 deletions(-) diff --git a/sdk/core/core-https/src/policies/bearerTokenAuthenticationPolicy.ts b/sdk/core/core-https/src/policies/bearerTokenAuthenticationPolicy.ts index 3a14b686eff3..06277fcbd120 100644 --- a/sdk/core/core-https/src/policies/bearerTokenAuthenticationPolicy.ts +++ b/sdk/core/core-https/src/policies/bearerTokenAuthenticationPolicy.ts @@ -119,46 +119,43 @@ export function bearerTokenAuthenticationPolicy( return request; } - /** - * Uses the challenge parameters to: - * - Prepare the outgoing request (if the `prepareRequest` method has been provided). - * - Send an initial request to receive the challenge if it fails. - * - Process a challenge if the response contains it. - * - Retrieve a token with the challenge information, then re-send the request. - */ - async function challengePolicy( - request: PipelineRequest, - next: SendRequest - ): Promise { - if (prepareRequest) { - await prepareRequest(request); - } - - const response = await next(request); - const challenge = getChallenge(response); - - if (challenge && processChallenge) { - const context = await processChallenge(challenge); - const token = await retrieveToken(request, context); - return next(assignToken(request, token)); - } - return response; - } - return { name: bearerTokenAuthenticationPolicyName, /** - * If there's no cached token and we have challenge options, this policy will try to authenticate using the challenges. - * If there's an access token in the cache, it will avoid any challenge processing. + * If there's no challenge parameter: + * - It will try to retrieve the token using the cache, or the credential's getToken. + * - Then it will try the next policy with or without the retrieved token. + * + * It uses the challenge parameters to: + * - Skip a first attempt to get the token from the credential if there's no cached token, + * since it expects the token to be retrievable only after the challenge. + * - Prepare the outgoing request if the `prepareRequest` method has been provided. + * - Send an initial request to receive the challenge if it fails. + * - Process a challenge if the response contains it. + * - Retrieve a token with the challenge information, then re-send the request. */ async sendRequest(request: PipelineRequest, next: SendRequest): Promise { let accessToken = tokenCache.getCachedToken(); - if (!accessToken && processChallenge) { - return await challengePolicy(request, next); - } else { - const token = await retrieveToken(request); + let token: string | undefined; + if (!accessToken && !processChallenge) { + token = await retrieveToken(request); + request = assignToken(request, token); + } + + if (prepareRequest) { + await prepareRequest(request); + } + + const response = await next(request); + const challenge = getChallenge(response); + + if (challenge && processChallenge) { + const context = await processChallenge(challenge); + const token = await retrieveToken(request, context); return next(assignToken(request, token)); } + + return response; } }; } diff --git a/sdk/core/core-https/test/node/bearerTokenAuthenticationPolicyChallenge.spec.ts b/sdk/core/core-https/test/node/bearerTokenAuthenticationPolicyChallenge.spec.ts index b7174b68460d..876db7f2cdd0 100644 --- a/sdk/core/core-https/test/node/bearerTokenAuthenticationPolicyChallenge.spec.ts +++ b/sdk/core/core-https/test/node/bearerTokenAuthenticationPolicyChallenge.spec.ts @@ -170,4 +170,109 @@ describe("bearerTokenAuthenticationPolicy with challenge", function() { ]); assert.deepEqual(finalSendRequestHeaders, [undefined, `Bearer ${getTokenResponse.token}`]); }); + + it("tests that the challenge is processed even we already had a token", async function() { + const expected = [ + { + scope: "http://localhost/.default", + challengeClaims: JSON.stringify({ + access_token: { foo: "bar" } + }) + }, + { + scope: "http://localhost/.default2", + challengeClaims: JSON.stringify({ + access_token: { foo2: "bar2" } + }) + } + ]; + + const request = createPipelineRequest({ url: "https://example.com" }); + const responses: PipelineResponse[] = [ + { + headers: createHttpHeaders({ + "WWW-Authenticate": `Bearer scope="${expected[0].scope}", claims="${encodeString( + expected[0].challengeClaims + )}"` + }), + request, + status: 401 + }, + { + headers: createHttpHeaders(), + request, + status: 200 + }, + { + headers: createHttpHeaders({ + "WWW-Authenticate": `Bearer scope="${expected[1].scope}", claims="${encodeString( + expected[1].challengeClaims + )}"` + }), + request, + status: 401 + }, + { + headers: createHttpHeaders(), + request, + status: 200 + } + ]; + + const expiresOn = Date.now() + 5000; + const getTokenResponses = [ + { token: "mock-token", expiresOnTimestamp: expiresOn }, + { token: "mock-token2", expiresOnTimestamp: expiresOn } + ]; + const credential = new MockRefreshAzureCredential([...getTokenResponses]); + + const pipeline = createEmptyPipeline(); + const bearerPolicy = bearerTokenAuthenticationPolicy({ + // Intentionally left empty, as it should be replaced by the challenge. + scopes: "", + credential, + challenge: { + processChallenge + } + }); + pipeline.addPolicy(bearerPolicy); + + const finalSendRequestHeaders: (string | undefined)[] = []; + + const testHttpsClient: HttpsClient = { + sendRequest: async (req) => { + finalSendRequestHeaders.push(req.headers.get("Authorization")); + if (responses.length) { + const response = responses.shift()!; + response.request = req; + return response; + } + throw new Error("No responses found"); + } + }; + + await pipeline.sendRequest(testHttpsClient, request); + await pipeline.sendRequest(testHttpsClient, request); + + // Our goal is to test that: + // - After a second challenge was received, we processed it and retrieved the token again. + + assert.equal(credential.authCount, 2); + assert.deepEqual(credential.scopesAndClaims, [ + { + scope: [expected[0].scope], + challengeClaims: expected[0].challengeClaims + }, + { + scope: [expected[1].scope], + challengeClaims: expected[1].challengeClaims + } + ]); + assert.deepEqual(finalSendRequestHeaders, [ + undefined, + `Bearer ${getTokenResponses[0].token}`, + `Bearer ${getTokenResponses[0].token}`, + `Bearer ${getTokenResponses[1].token}` + ]); + }); });