Fixes for a couple @azure/core-http issues found during arm-* migrati…

…on (Azure#4335) * Provide correct scope name to BearerTokenAuthenticationPolicy * Don't confuse TokenCredential with TokenClientCredentials
daviwil · Jul 17, 2019 · 771614e · 771614e
1 parent 9128a46
commit 771614e
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 5 deletions.
diff --git a/sdk/core/core-auth/src/tokenCredential.ts b/sdk/core/core-auth/src/tokenCredential.ts
@@ -49,5 +49,11 @@ export interface AccessToken {
  * @param credential The assumed TokenCredential to be tested.
  */
 export function isTokenCredential(credential: any): credential is TokenCredential {
-  return credential && typeof credential.getToken === "function";
+  // Check for an object with a 'getToken' function but without
+  // a 'signRequest' function.  We do this check to make sure that
+  // a ServiceClientCredentials implementor (like TokenClientCredentials
+  // in ms-rest-nodeauth) doesn't get mistaken for a TokenCredential.
+  return credential
+    && typeof credential.getToken === "function"
+    && credential.signRequest === undefined;
 }
diff --git a/sdk/core/core-auth/test/index.spec.ts b/sdk/core/core-auth/test/index.spec.ts
@@ -29,4 +29,11 @@ describe("isTokenCredential", function () {
       getToken: true
     }), false);
   });
+
+  it("should return false for an object that has a 'signRequest' field", () => {
+    assert.strictEqual(isTokenCredential({
+      getToken: function () { },
+      signRequest: function() { },
+    }), false);
+  })
 });
diff --git a/sdk/core/core-http/lib/serviceClient.ts b/sdk/core/core-http/lib/serviceClient.ts
@@ -153,7 +153,34 @@ export class ServiceClient {
     if (Array.isArray(options.requestPolicyFactories)) {
       requestPolicyFactories = options.requestPolicyFactories;
     } else {
-      requestPolicyFactories = createDefaultRequestPolicyFactories(credentials, options);
+      let credentialsOrFactory: ServiceClientCredentials | RequestPolicyFactory | undefined = undefined;
+      if (isTokenCredential(credentials)) {
+        // Create a wrapped RequestPolicyFactory here so that we can provide the
+        // correct scope to the BearerTokenAuthenticationPolicy at the first time
+        // one is requested.  This is needed because generated ServiceClient
+        // implementations do not set baseUri until after ServiceClient's constructor
+        // is finished, leaving baseUri empty at the time when it is needed to
+        // build the correct scope name.
+        const wrappedPolicyFactory: () => RequestPolicyFactory = () => {
+          let bearerTokenPolicyFactory: RequestPolicyFactory | undefined = undefined;
+          let serviceClient = this;
+          return {
+            create(nextPolicy: RequestPolicy, options: RequestPolicyOptions): RequestPolicy {
+              if (bearerTokenPolicyFactory === undefined) {
+                bearerTokenPolicyFactory = bearerTokenAuthenticationPolicy(credentials, `${serviceClient.baseUri || ""}/.default`)
+              }
+
+              return bearerTokenPolicyFactory.create(nextPolicy, options);
+            }
+          }
+        };
+
+        credentialsOrFactory = wrappedPolicyFactory();
+      } else {
+        credentialsOrFactory = credentials;
+      }
+
+      requestPolicyFactories = createDefaultRequestPolicyFactories(credentialsOrFactory, options);
       if (options.requestPolicyFactories) {
         const newRequestPolicyFactories: void | RequestPolicyFactory[] = options.requestPolicyFactories(requestPolicyFactories);
         if (newRequestPolicyFactories) {
@@ -395,7 +422,7 @@ function getValueOrFunctionResult(value: undefined | string | ((defaultValue: st
   return result;
 }
 
-function createDefaultRequestPolicyFactories(credentials: ServiceClientCredentials | TokenCredential | RequestPolicyFactory | undefined, options: ServiceClientOptions): RequestPolicyFactory[] {
+function createDefaultRequestPolicyFactories(credentials: ServiceClientCredentials | RequestPolicyFactory | undefined, options: ServiceClientOptions): RequestPolicyFactory[] {
   const factories: RequestPolicyFactory[] = [];
 
   if (options.generateClientRequestIdHeader) {
@@ -405,8 +432,6 @@ function createDefaultRequestPolicyFactories(credentials: ServiceClientCredentia
   if (credentials) {
     if (isRequestPolicyFactory(credentials)) {
       factories.push(credentials);
-    } else if (isTokenCredential(credentials)) {
-      factories.push(bearerTokenAuthenticationPolicy(credentials, "/.default"));
     } else {
       factories.push(signingPolicy(credentials));
     }