fix auth bug, streaming dependencies, add tests (#1498)

libraries/botbuilder/src/botFrameworkAdapter.ts

@@ -1085,7 +1085,8 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide
         if (!credentials) throw new Error('invalid credentials');
 
         let client: ConnectorClient = context.turnState.get(this.ConnectorClientKey);
-        if (!client) {
+        // Inspect the retrieved client to confirm that the serviceUrl is correct, if it isn't, create a new one.
+        if (!client || client['baseUri'] !== serviceUrl) {
             client = this.createConnectorClientInternal(serviceUrl, credentials);
         }
 
@@ -1098,7 +1099,7 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide
      * @param appId 
      * @param oAuthScope 
      */
-    protected async buildCredentials(appId: string, oAuthScope: string = ''): Promise<AppCredentials> {
+    protected async buildCredentials(appId: string, oAuthScope?: string): Promise<AppCredentials> {
         // There is no cache for AppCredentials in JS as opposed to C#.
         // Instead of retrieving an AppCredentials from the Adapter instance, generate a new one
         const appPassword = await this.credentialsProvider.getAppPassword(appId);

libraries/botbuilder/src/botFrameworkHttpClient.ts

@@ -110,12 +110,11 @@ export class BotFrameworkHttpClient {
 
         const appPassword = await this.credentialProvider.getAppPassword(appId);
         if (JwtTokenValidation.isGovernment(this.channelService)) {
-            appCredentials = new MicrosoftAppCredentials(appId, appPassword, this.channelService);
+            appCredentials = new MicrosoftAppCredentials(appId, appPassword, this.channelService, oAuthScope);
             appCredentials.oAuthEndpoint = GovernmentConstants.ToChannelFromBotLoginUrl;
             appCredentials.oAuthScope = GovernmentConstants.ToChannelFromBotOAuthScope;
         } else {
-            appCredentials = new MicrosoftAppCredentials(appId, appPassword, this.channelService);
-            appCredentials.oAuthScope = !oAuthScope ? AuthenticationConstants.ToChannelFromBotOAuthScope : oAuthScope;
+            appCredentials = new MicrosoftAppCredentials(appId, appPassword, this.channelService, oAuthScope);
         }
 
         // Cache the credentials for later use

libraries/botbuilder/src/skills/skillHandler.ts

@@ -148,11 +148,11 @@ export class SkillHandler extends ChannelServiceHandler {
             const adapter: BotFrameworkAdapter = (context.adapter as BotFrameworkAdapter);
             // Cache the ClaimsIdentity and ConnectorClient on the context so that it's available inside of the bot's logic.
             context.turnState.set(adapter.BotIdentityKey, claimsIdentity);
-            const client = await adapter.createConnectorClientWithIdentity(activity.serviceUrl, claimsIdentity);
-            context.turnState.set(adapter.ConnectorClientKey, client);
             context.turnState.set(this.SkillConversationReferenceKey, skillConversationReference);
+            activity = TurnContext.applyConversationReference(activity, conversationReference) as Activity;
+            const client = adapter.createConnectorClient(activity.serviceUrl);
+            context.turnState.set(adapter.ConnectorClientKey, client);
 
-            TurnContext.applyConversationReference(activity, conversationReference);
             context.activity.id = replyToActivityId;
             switch (activity.type) {
                 case ActivityTypes.EndOfConversation:

libraries/botbuilder/src/streaming/streamingHttpClient.ts

@@ -6,7 +6,7 @@
  * Licensed under the MIT License.
  */
 
-import { WebResource, HttpOperationResponse, HttpClient } from 'botframework-connector/node_modules/@azure/ms-rest-js';
+import { WebResource, HttpOperationResponse, HttpClient } from '@azure/ms-rest-js';
 import { IStreamingTransportServer, StreamingRequest } from 'botframework-streaming';
 
 export class StreamingHttpClient implements HttpClient {

libraries/botbuilder/tests/botFrameworkAdapter.test.js

@@ -1,7 +1,7 @@
 const assert = require('assert');
-const { TurnContext } = require('botbuilder-core');
+const { ActivityTypes, TurnContext } = require('botbuilder-core');
 const connector = require('botframework-connector');
-const { AuthenticationConstants, CertificateAppCredentials } = require('botframework-connector');
+const { AuthenticationConstants, CertificateAppCredentials, ConnectorClient, MicrosoftAppCredentials } = require('botframework-connector');
 const { BotFrameworkAdapter } = require('../');
 
 const reference = {
@@ -257,13 +257,25 @@ describe(`BotFrameworkAdapter`, function () {
         });
     });
 
-    it(`should createConnectorClient().`, function (done) {
-        const req = new MockRequest(incomingMessage);
-        const adapter = new AdapterUnderTest();
-        const client = adapter.testCreateConnectorClient(reference.serviceUrl);
-        assert(client, `client not returned.`);
-        assert(client.conversations, `invalid client returned.`);
-        done();
+    describe('get/create ConnectorClient methods', () => {
+        it(`should createConnectorClient().`, function (done) {
+            const req = new MockRequest(incomingMessage);
+            const adapter = new AdapterUnderTest();
+            const client = adapter.testCreateConnectorClient(reference.serviceUrl);
+            assert(client, `client not returned.`);
+            assert(client.conversations, `invalid client returned.`);
+            done();
+        });
+
+        it('getOrCreateConnectorClient should create a new client if the cached serviceUrl does not match the provided one', () => {
+            const adapter = new BotFrameworkAdapter();
+            const context = new TurnContext(adapter, { type: ActivityTypes.Message, text: 'hello', serviceUrl: 'http://bing.com' });
+            const cc = new ConnectorClient(new MicrosoftAppCredentials('', ''), {baseUri: 'http://bing.com'});
+            context.turnState.set(adapter.ConnectorClientKey, cc);
+
+            const client = adapter.getOrCreateConnectorClient(context, 'https://botframework.com', adapter.credentials);
+            assert.notEqual(client.baseUri, cc.baseUri);
+        });
     });
 
     it(`should processActivity().`, function (done) {
@@ -732,15 +744,15 @@ describe(`BotFrameworkAdapter`, function () {
     });
 
     // This unit test doesn't work anymore because client.UserAgentInfo was removed, so we can't inspect the user agent string
-    // it(`should create a User-Agent header with the same info as the host machine.`, function (done) {
-    //     const adapter = new BotFrameworkAdapter();
-    //     const client = adapter.createConnectorClient('https://example.com');
-    //     //const userAgentHeader = client.userAgentInfo.value;
-    //     const pjson = require('../package.json');
-    //     const userAgent = 'Microsoft-BotFramework/3.1 BotBuilder/' + pjson.version + ' (Node.js,Version=' + process.version + '; ' + os.type() + ' ' + os.release() + '; ' + os.arch() + ')';
-    //     // assert(userAgentHeader.includes(userAgent), `ConnectorClient doesn't have user-agent header created by BotFrameworkAdapter or header is incorrect.`);
-    //     done();
-    // });
+    xit(`should create a User-Agent header with the same info as the host machine.`, function (done) {
+        const adapter = new BotFrameworkAdapter();
+        const client = adapter.createConnectorClient('https://example.com');
+        //const userAgentHeader = client.userAgentInfo.value;
+        const pjson = require('../package.json');
+        const userAgent = 'Microsoft-BotFramework/3.1 BotBuilder/' + pjson.version + ' (Node.js,Version=' + process.version + '; ' + os.type() + ' ' + os.release() + '; ' + os.arch() + ')';
+        // assert(userAgentHeader.includes(userAgent), `ConnectorClient doesn't have user-agent header created by BotFrameworkAdapter or header is incorrect.`);
+        done();
+    });
 
     it(`should set openIdMetadata property on ChannelValidation`, function (done) {
         const testEndpoint = "http://rainbows.com";

libraries/botframework-connector/src/auth/appCredentials.ts

@@ -30,23 +30,32 @@ export abstract class AppCredentials implements msrest.ServiceClientCredentials
     public appId: string;
 
     public oAuthEndpoint: string;
-    public oAuthScope: string = AuthenticationConstants.ToBotFromChannelTokenIssuer;
-    public readonly tokenCacheKey: string;
+    private _oAuthScope: string;
+    public tokenCacheKey: string;
     protected refreshingToken: Promise<adal.TokenResponse> | null = null;
     protected readonly authenticationContext: adal.AuthenticationContext;
 
-    constructor(appId: string, channelAuthTenant?: string) {
+    constructor(appId: string, channelAuthTenant?: string, oAuthScope: string = AuthenticationConstants.ToBotFromChannelTokenIssuer) {
         this.appId = appId;
         const tenant = channelAuthTenant && channelAuthTenant.length > 0
             ? channelAuthTenant
             : AuthenticationConstants.DefaultChannelAuthTenant;
         this.oAuthEndpoint = AuthenticationConstants.ToChannelFromBotLoginUrlPrefix + tenant;
-        this.tokenCacheKey = `${ appId }${ this.oAuthScope }-cache`;
+        this.oAuthScope = oAuthScope;
         // aadApiVersion is set to '1.5' to avoid the "spn:" concatenation on the audience claim
         // For more info, see https://github.com/AzureAD/azure-activedirectory-library-for-nodejs/issues/128
         this.authenticationContext = new adal.AuthenticationContext(this.oAuthEndpoint, true, undefined, '1.5');
     }
 
+    public get oAuthScope(): string {
+        return this._oAuthScope
+    }
+
+    public set oAuthScope(value: string) {
+        this._oAuthScope = value;
+        this.tokenCacheKey = `${ this.appId }${ this.oAuthScope }-cache`;
+    }
+
     /**
      * Adds the host of service url to trusted hosts.
      * If expiration time is not provided, the expiration date will be current (utc) date + 1 day.

libraries/botframework-connector/src/auth/certificateAppCredentials.ts

@@ -16,8 +16,8 @@ export class CertificateAppCredentials extends AppCredentials {
     public certificateThumbprint: string;
     public certificatePrivateKey: string;
 
-    constructor(appId: string, certificateThumbprint: string, certificatePrivateKey: string, channelAuthTenant?: string) {
-        super(appId, channelAuthTenant);
+    constructor(appId: string, certificateThumbprint: string, certificatePrivateKey: string, channelAuthTenant?: string, oAuthScope?: string) {
+        super(appId, channelAuthTenant, oAuthScope);
         this.certificateThumbprint = certificateThumbprint;
         this.certificatePrivateKey = certificatePrivateKey;
     }

libraries/botframework-connector/src/auth/microsoftAppCredentials.ts

@@ -16,12 +16,8 @@ export class MicrosoftAppCredentials extends AppCredentials {
     public appPassword: string;
 
     constructor(appId: string, appPassword: string, channelAuthTenant?: string, oAuthScope?: string) {
-        super(appId, channelAuthTenant);
+        super(appId, channelAuthTenant, oAuthScope);
         this.appPassword = appPassword;
-
-        // AppCredentials.oAuthScope has an initial an initial value of AuthenticationConstants.ToBotFromChannelTokenIssuer,
-        // Only change it if there is an actual value provided in the constructor for MicrosoftAppCredentials.
-        this.oAuthScope = oAuthScope ? oAuthScope : this.oAuthScope;
     }
 
     protected async refreshToken(): Promise<adal.TokenResponse> {

libraries/botframework-connector/tests/appCredentials.test.js

@@ -32,18 +32,34 @@ describe('AppCredentials', () => {
         strictEqual(certCreds.certificatePrivateKey, CERT_KEY);
 
         const msAppCreds = new MicrosoftAppCredentials(APP_ID, APP_PASSWORD);
-        strictEqual(msAppCreds.appId, '2cd87869-38a0-4182-9251-d056e8f0ac24');
+        strictEqual(msAppCreds.appId, APP_ID);
         strictEqual(msAppCreds.appPassword, APP_PASSWORD);
     });
 
     describe('MicrosoftAppCredentials', () => {
         it('should set oAuthScope when passed in the constructor', () => {
             const oAuthScope = 'oAuthScope';
-            const tokenGenerator = new MicrosoftAppCredentials('2cd87869-38a0-4182-9251-d056e8f0ac24', undefined, undefined, oAuthScope);
+            const tokenGenerator = new MicrosoftAppCredentials(APP_ID, undefined, undefined, oAuthScope);
             strictEqual(tokenGenerator.oAuthScope, oAuthScope);
         });
+
+        it('should set update the tokenCacheKey when oAuthScope is set after construction', () => {
+            const tokenGenerator = new MicrosoftAppCredentials(APP_ID);
+            strictEqual(tokenGenerator.tokenCacheKey, `${APP_ID}${AuthenticationConstants.ToBotFromChannelTokenIssuer}-cache`);
+
+            const oAuthScope = 'oAuthScope';
+            tokenGenerator.oAuthScope = oAuthScope;
+            strictEqual(tokenGenerator.tokenCacheKey, `${APP_ID}${oAuthScope}-cache`);
+
+            /* CertificateAppCredentials */
+            const certCreds = new CertificateAppCredentials(APP_ID, CERT_THUMBPRINT, CERT_KEY);
+            strictEqual(certCreds.tokenCacheKey, `${APP_ID}${AuthenticationConstants.ToBotFromChannelTokenIssuer}-cache`);
+            certCreds.oAuthScope = oAuthScope;
+            strictEqual(certCreds.tokenCacheKey, `${APP_ID}${oAuthScope}-cache`);
+        });
+
         it('should fail to get a token with an appId and no appPassword', async () => {
-            const tokenGenerator = new MicrosoftAppCredentials('2cd87869-38a0-4182-9251-d056e8f0ac24');
+            const tokenGenerator = new MicrosoftAppCredentials(APP_ID);
             try {
                 await tokenGenerator.getToken(true);
                 fail('Should not have successfully retrieved token.');

libraries/botframework-streaming/package.json

@@ -22,13 +22,13 @@
   "main": "lib/index.js",
   "typings": "lib/index.d.js",
   "dependencies": {
+    "@types/ws": "^6.0.3",
     "uuid": "^3.3.2",
     "ws": "^7.1.2"
   },
   "devDependencies": {
     "@types/chai": "^4.1.7",
     "@types/node": "^10.12.18",
-    "@types/ws": "^6.0.3",
     "@typescript-eslint/eslint-plugin": "^1.10.2",
     "@typescript-eslint/parser": "^1.10.2",
     "chai": "^4.2.0",