processing challenges even if we already have a token

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<PipelineResponse> {
-    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<PipelineResponse> {
       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;
     }
   };
 }

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}`
+    ]);
+  });
 });