[core] bearerTokenAuthenticationPolicy shouldn't sign HTTP URLs (#15785)

Following our approach in other languages, for example, Python’s: https://github.com/Azure/azure-sdk-for-python/blob/d27b0f6a49afa85d99994f052188d192a10c5f02/sdk/containerregistry/azure-containerregistry/azure/containerregistry/_helpers.py#L81 Fixes #14311
Azure · Jun 18, 2021 · f99402d · f99402d
1 parent a955ff2
commit f99402d
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 19 deletions.
diff --git a/sdk/core/core-http/src/policies/bearerTokenAuthenticationPolicy.ts b/sdk/core/core-http/src/policies/bearerTokenAuthenticationPolicy.ts
@@ -240,6 +240,12 @@ export function bearerTokenAuthenticationPolicy(
     }
 
     public async sendRequest(webResource: WebResourceLike): Promise<HttpOperationResponse> {
+      if (!webResource.url.toLowerCase().startsWith("https://")) {
+        throw new Error(
+          "Bearer token authentication is not permitted for non-TLS protected (non-https) URLs."
+        );
+      }
+
       const { token } = await getToken({
         abortSignal: webResource.abortSignal,
         tracingOptions: {

diff --git a/sdk/core/core-http/test/policies/bearerTokenAuthenticationPolicyTests.ts b/sdk/core/core-http/test/policies/bearerTokenAuthenticationPolicyTests.ts
@@ -183,8 +183,30 @@ describe("BearerTokenAuthenticationPolicy", function() {
     assert.equal(expireDelayMs + 2 * getTokenDelay, Date.now() - startTime, exceptionMessage);
   });
 
-  function createRequest(operationSpec?: OperationSpec): WebResource {
-    const request = new WebResource();
+  it("throws if the target URI doesn't start with 'https'", async () => {
+    const expiresOn = Date.now() + 1000 * 60; // One minute later.
+    const credential = new MockRefreshAzureCredential(expiresOn);
+    const request = createRequest(undefined, "http://azure.com");
+    const policy = createBearerTokenPolicy("test-scope", credential);
+
+    let error: Error | undefined;
+    try {
+      await policy.sendRequest(request);
+    } catch (e) {
+      error = e;
+    }
+
+    assert.equal(
+      error?.message,
+      "Bearer token authentication is not permitted for non-TLS protected (non-https) URLs."
+    );
+  });
+
+  function createRequest(
+    operationSpec?: OperationSpec,
+    uri: string = "https://azure.com"
+  ): WebResource {
+    const request = new WebResource(uri);
     request.operationSpec = operationSpec;
     return request;
   }

diff --git a/sdk/core/core-http/test/serviceClientTests.ts b/sdk/core/core-http/test/serviceClientTests.ts
@@ -44,7 +44,7 @@ describe("ServiceClient", function() {
 
     const testOperationSpec: OperationSpec = {
       httpMethod: "GET",
-      baseUrl: "httpbin.org",
+      baseUrl: "https://bin.org",
       serializer: new Serializer(),
       headerParameters: [
         {
@@ -217,7 +217,7 @@ describe("ServiceClient", function() {
       },
       {
         httpMethod: "GET",
-        baseUrl: "httpbin.org",
+        baseUrl: "https://bin.org",
         serializer: new Serializer(),
         headerParameters: [
           {
@@ -271,7 +271,7 @@ describe("ServiceClient", function() {
       {},
       {
         httpMethod: "GET",
-        baseUrl: "httpbin.org",
+        baseUrl: "https://bin.org",
         serializer: new Serializer(),
         headerParameters: [],
         responses: {
@@ -385,7 +385,7 @@ describe("ServiceClient", function() {
   });
 
   it("should serialize collection with empty multi query parameter", async function() {
-    await testSendOperationRequest([], QueryCollectionFormat.Multi, true, "httpbin.org");
+    await testSendOperationRequest([], QueryCollectionFormat.Multi, true, "https://bin.org");
   });
 
   it("should apply withCredentials to requests", async function() {
@@ -406,7 +406,7 @@ describe("ServiceClient", function() {
       {
         serializer: new Serializer(),
         httpMethod: "GET",
-        baseUrl: "httpbin.org",
+        baseUrl: "https://bin.org",
         responses: { 200: {} }
       }
     );
@@ -423,7 +423,7 @@ describe("ServiceClient", function() {
       {
         serializer: new Serializer(),
         httpMethod: "GET",
-        baseUrl: "httpbin.org",
+        baseUrl: "https://bin.org",
         responses: { 200: {} }
       }
     );
@@ -454,7 +454,7 @@ describe("ServiceClient", function() {
       {
         serializer: new Serializer(),
         httpMethod: "GET",
-        baseUrl: "httpbin.org",
+        baseUrl: "https://bin.org",
         responses: {
           200: {
             bodyMapper: {
@@ -494,7 +494,7 @@ describe("ServiceClient", function() {
       {
         serializer: new Serializer(),
         httpMethod: "GET",
-        baseUrl: "httpbin.org",
+        baseUrl: "https://bin.org",
         responses: {}
       }
     );
@@ -521,7 +521,7 @@ describe("ServiceClient", function() {
       {
         serializer: new Serializer(),
         httpMethod: "GET",
-        baseUrl: "httpbin.org",
+        baseUrl: "https://bin.org",
         responses: {}
       }
     );
@@ -547,7 +547,7 @@ describe("ServiceClient", function() {
       {
         serializer: new Serializer(),
         httpMethod: "GET",
-        baseUrl: "httpbin.org",
+        baseUrl: "https://bin.org",
         responses: {}
       }
     );
@@ -576,7 +576,7 @@ describe("ServiceClient", function() {
       {
         serializer: new Serializer(),
         httpMethod: "GET",
-        baseUrl: "httpbin.org",
+        baseUrl: "https://bin.org",
         responses: {}
       }
     );
@@ -607,7 +607,7 @@ describe("ServiceClient", function() {
       {
         serializer: new Serializer(),
         httpMethod: "GET",
-        baseUrl: "httpbin.org",
+        baseUrl: "https://bin.org",
         responses: {}
       }
     );
@@ -1960,7 +1960,7 @@ describe("ServiceClient", function() {
           bodyMapper: BodyMapper
         }
       },
-      baseUrl: "httpbin.org",
+      baseUrl: "https://bin.org",
       serializer
     };
 
@@ -2032,7 +2032,7 @@ async function testSendOperationRequest(
     },
     {
       httpMethod: "GET",
-      baseUrl: "httpbin.org",
+      baseUrl: "https://bin.org",
       serializer: new Serializer(),
       queryParameters: [
         {

diff --git a/sdk/core/core-rest-pipeline/src/policies/bearerTokenAuthenticationPolicy.ts b/sdk/core/core-rest-pipeline/src/policies/bearerTokenAuthenticationPolicy.ts
@@ -155,6 +155,12 @@ export function bearerTokenAuthenticationPolicy(
      * - Retrieve a token with the challenge information, then re-send the request.
      */
     async sendRequest(request: PipelineRequest, next: SendRequest): Promise<PipelineResponse> {
+      if (!request.url.toLowerCase().startsWith("https://")) {
+        throw new Error(
+          "Bearer token authentication is not permitted for non-TLS protected (non-https) URLs."
+        );
+      }
+
       await callbacks.authorizeRequest({
         scopes: Array.isArray(scopes) ? scopes : [scopes],
         request,

diff --git a/sdk/core/core-rest-pipeline/test/bearerTokenAuthenticationPolicy.spec.ts b/sdk/core/core-rest-pipeline/test/bearerTokenAuthenticationPolicy.spec.ts
@@ -223,6 +223,28 @@ describe("BearerTokenAuthenticationPolicy", function() {
     assert.equal(expireDelayMs + 2 * getTokenDelay, Date.now() - startTime, exceptionMessage);
   });
 
+  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 = createBearerTokenPolicy("test-scope", credential);
+    const next = sinon.stub<Parameters<SendRequest>, ReturnType<SendRequest>>();
+
+    let error: Error | undefined;
+    try {
+      await policy.sendRequest(request, next);
+    } catch (e) {
+      error = e;
+    }
+
+    assert.equal(
+      error?.message,
+      "Bearer token authentication is not permitted for non-TLS protected (non-https) URLs."
+    );
+  });
+
   function createBearerTokenPolicy(
     scopes: string | string[],
     credential: TokenCredential

diff --git a/sdk/monitor/monitor-query/test/internal/unit/logsQueryClient.unittest.spec.ts b/sdk/monitor/monitor-query/test/internal/unit/logsQueryClient.unittest.spec.ts
@@ -29,12 +29,12 @@ describe("LogsQueryClient unit tests", () => {
     };
 
     const client = new LogsQueryClient(tokenCredential, {
-      endpoint: "customEndpoint1",
+      endpoint: "https://customEndpoint1",
       scopes: "customScopes1"
     });
 
-    assert.equal(client["_logAnalytics"].$host, "customEndpoint1");
-    assert.equal(client["_logAnalytics"]["baseUri"], "customEndpoint1");
+    assert.equal(client["_logAnalytics"].$host, "https://customEndpoint1");
+    assert.equal(client["_logAnalytics"]["baseUri"], "https://customEndpoint1");
 
     try {
       await client.queryLogs("workspaceId", "query", Durations.last5Minutes);