[Service Bus] Bug Fix: Incorrect connection-config in the token crede…

…ntial path (Azure#11250) * create connection context helper * changelog * couple of tests * More tests.. splitting tests for both the paths
AzureSDKAutomation · Sep 16, 2020 · 8190de7 · 8190de7
1 parent f2f446e
commit 8190de7
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 19 deletions.
diff --git a/sdk/servicebus/service-bus/CHANGELOG.md b/sdk/servicebus/service-bus/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 ## 7.0.0-preview.7 (Unreleased)
 
+### New features:
+- `ServiceBusClient` now supports authentication with AAD credentials in the browser(can use `InteractiveBrowserCredential` from `@azure/identity`).
+  [PR 11250](https://github.com/Azure/azure-sdk-for-js/pull/11250)
 
 ## 7.0.0-preview.6 (2020-09-10)
 

diff --git a/sdk/servicebus/service-bus/src/constructorHelpers.ts b/sdk/servicebus/service-bus/src/constructorHelpers.ts
@@ -33,40 +33,55 @@ export interface ServiceBusClientOptions {
 }
 
 /**
- * @param connectionString
- * @param options
  * @internal
  * @ignore
+ *
+ * @param {ConnectionConfig} config
  */
-export function createConnectionContextForConnectionString(
+function validate(config: ConnectionConfig) {
+  // TODO: workaround - core-amqp's validate string-izes "undefined"
+  // the timing of this particular call happens in a spot where we might not have an
+  // entity path so it's perfectly legitimate for it to be empty.
+  config.entityPath = config.entityPath ?? "";
+
+  ConnectionConfig.validate(config);
+}
+
+/**
+ * @internal
+ * @ignore
+ *
+ * @param {string} connectionString
+ * @param {(SharedKeyCredential | TokenCredential)} credential
+ * @param {ServiceBusClientOptions} options
+ */
+export function createConnectionContext(
   connectionString: string,
-  options: ServiceBusClientOptions = {}
+  credential: SharedKeyCredential | TokenCredential,
+  options: ServiceBusClientOptions
 ): ConnectionContext {
   const config = ConnectionConfig.create(connectionString);
 
   config.webSocket = options?.webSocketOptions?.webSocket;
   config.webSocketEndpointPath = "$servicebus/websocket";
   config.webSocketConstructorOptions = options?.webSocketOptions?.webSocketConstructorOptions;
 
-  const credential = SharedKeyCredential.fromConnectionString(connectionString);
-
   validate(config);
   return ConnectionContext.create(config, credential, options);
 }
 
 /**
+ * @param connectionString
+ * @param options
  * @internal
  * @ignore
- *
- * @param {ConnectionConfig} config
  */
-function validate(config: ConnectionConfig) {
-  // TODO: workaround - core-amqp's validate string-izes "undefined"
-  // the timing of this particular call happens in a spot where we might not have an
-  // entity path so it's perfectly legitimate for it to be empty.
-  config.entityPath = config.entityPath ?? "";
-
-  ConnectionConfig.validate(config);
+export function createConnectionContextForConnectionString(
+  connectionString: string,
+  options: ServiceBusClientOptions = {}
+): ConnectionContext {
+  const credential = SharedKeyCredential.fromConnectionString(connectionString);
+  return createConnectionContext(connectionString, credential, options);
 }
 
 /**
@@ -91,10 +106,7 @@ export function createConnectionContextForTokenCredential(
     host += "/";
   }
   const connectionString = `Endpoint=sb://${host};SharedAccessKeyName=defaultKeyName;SharedAccessKey=defaultKeyValue;`;
-  const config = ConnectionConfig.create(connectionString);
-
-  validate(config);
-  return ConnectionContext.create(config, credential, options);
+  return createConnectionContext(connectionString, credential, options);
 }
 
 /**

diff --git a/sdk/servicebus/service-bus/test/internal/serviceBusClient.spec.ts b/sdk/servicebus/service-bus/test/internal/serviceBusClient.spec.ts
@@ -5,6 +5,12 @@ import { extractReceiverArguments, ServiceBusClient } from "../../src/serviceBus
 import chai from "chai";
 import { CreateSessionReceiverOptions } from "../../src/models";
 import { entityPathMisMatchError } from "../../src/util/errors";
+import {
+  createConnectionContextForConnectionString,
+  createConnectionContextForTokenCredential
+} from "../../src/constructorHelpers";
+import { TokenCredential } from "@azure/core-http";
+import { ConnectionContext } from "../../src/connectionContext";
 const assert = chai.assert;
 
 const allLockModes: ("peekLock" | "receiveAndDelete")[] = ["peekLock", "receiveAndDelete"];
@@ -147,4 +153,75 @@ describe("serviceBusClient unit tests", () => {
       }
     });
   });
+
+  describe("Create ConnectionContext helpers", () => {
+    function validateWebsocketInfo(
+      connectionContext: ConnectionContext,
+      providedWebsocketConstructorOptions: any
+    ) {
+      assert.equal(
+        connectionContext.config.webSocketEndpointPath,
+        "$servicebus/websocket",
+        "Unexpected webSocketEndpointPath in the connection config"
+      );
+      assert.equal(
+        connectionContext.config.webSocketConstructorOptions,
+        providedWebsocketConstructorOptions,
+        "Unexpected webSocketEndpointPath in the connection config"
+      );
+    }
+
+    describe("createConnectionContextForConnectionString", () => {
+      it("Websocket endpoint and constructor options are populated in the config", () => {
+        const connString =
+          "Endpoint=sb://a;SharedAccessKeyName=b;SharedAccessKey=c;EntityPath=some-queue";
+        const options = { randomOption: 123 };
+        const connectionContext = createConnectionContextForConnectionString(connString, {
+          webSocketOptions: { webSocketConstructorOptions: options }
+        });
+        validateWebsocketInfo(connectionContext, options);
+      });
+
+      it("undefined entity path is translated to ''", () => {
+        const connString = "Endpoint=sb://a;SharedAccessKeyName=b;SharedAccessKey=c;";
+        const connectionContext = createConnectionContextForConnectionString(connString, {});
+        assert.equal(
+          connectionContext.config.entityPath,
+          "",
+          "Unexpected entityPath in the connection config"
+        );
+      });
+    });
+
+    describe("createConnectionContextForTokenCredential", () => {
+      const pseudoTokenCred: TokenCredential = {
+        async getToken() {
+          return { expiresOnTimestamp: 0, token: "" };
+        }
+      };
+      const endpoint = "endpoint";
+      it("Websocket endpoint and constructor options are populated in the config", () => {
+        const options = { randomOption: 123 };
+        const connectionContext = createConnectionContextForTokenCredential(
+          pseudoTokenCred,
+          endpoint,
+          { webSocketOptions: { webSocketConstructorOptions: options } }
+        );
+        validateWebsocketInfo(connectionContext, options);
+      });
+
+      it("undefined entity path is translated to ''", () => {
+        const connectionContext = createConnectionContextForTokenCredential(
+          pseudoTokenCred,
+          endpoint,
+          {}
+        );
+        assert.equal(
+          connectionContext.config.entityPath,
+          "",
+          "Unexpected entityPath in the connection config"
+        );
+      });
+    });
+  });
 });