trust reference.serviceUrl if credentials.appId is truthy (#1889)

* trust reference.serviceUrl if credentials.appId is truthy

* address @stevengum's PR feedback

how meta

Author: stevengum

libraries/botbuilder/src/botFrameworkAdapter.ts

@@ -272,12 +272,19 @@ export class BotFrameworkAdapter extends BotAdapter implements ExtendedUserToken
 
         let credentials: AppCredentials = this.credentials;
 
-        // If the provided OAuthScope doesn't match the current one on the instance's credentials, create
-        // a new AppCredentials with the correct OAuthScope.
-        if (credentials.oAuthScope !== audience) {
-            // The BotFrameworkAdapter JavaScript implementation supports one Bot per instance, so get
-            // the botAppId from the credentials.
-            credentials = await this.buildCredentials(this.credentials.appId, audience);
+        // For authenticated flows (where the bot has an AppId), the ConversationReference's serviceUrl needs
+        // to be trusted for the bot to acquire a token when sending activities to the conversation.
+        // For anonymous flows, the serviceUrl should not be trusted.
+        if (credentials.appId) {
+            AppCredentials.trustServiceUrl(reference.serviceUrl);
+
+            // If the provided OAuthScope doesn't match the current one on the instance's credentials, create
+            // a new AppCredentials with the correct OAuthScope.
+            if (credentials.oAuthScope !== audience) {
+                // The BotFrameworkAdapter JavaScript implementation supports one Bot per instance, so get
+                // the botAppId from the credentials.
+                credentials = await this.buildCredentials(credentials.appId, audience);
+            }
         }
 
         const connectorClient = this.createConnectorClientInternal(reference.serviceUrl, credentials);

libraries/botbuilder/tests/botFrameworkAdapter.test.js

@@ -467,22 +467,6 @@ describe(`BotFrameworkAdapter`, function () {
         });
     });
 
-    it(`should continueConversation().`, function (done) {
-        let called = false;
-        const adapter = new AdapterUnderTest();
-        adapter.continueConversation(reference, (context) => {
-            assert(context, `context not passed.`);
-            assert(context.activity, `context has no request.`);
-            assert(context.activity.type === 'event', `request has invalid type.`);
-            assert(context.activity.from && context.activity.from.id === reference.user.id, `request has invalid from.id.`);
-            assert(context.activity.recipient && context.activity.recipient.id === reference.bot.id, `request has invalid recipient.id.`);
-            called = true;
-        }).then(() => {
-            assert(called, `bot logic not called.`);
-            done();
-        });
-    });
-
     it(`should createConversation().`, function (done) {
         let called = false;
         const adapter = new AdapterUnderTest();
@@ -1198,6 +1182,56 @@ describe(`BotFrameworkAdapter`, function () {
     });
 
     describe('continueConversation', function() {
+        it(`should succeed.`, function (done) {
+            let called = false;
+            const adapter = new AdapterUnderTest();
+            adapter.continueConversation(reference, (context) => {
+                assert(context, `context not passed.`);
+                assert(context.activity, `context has no request.`);
+                assert(context.activity.type === 'event', `request has invalid type.`);
+                assert(context.activity.from && context.activity.from.id === reference.user.id, `request has invalid from.id.`);
+                assert(context.activity.recipient && context.activity.recipient.id === reference.bot.id, `request has invalid recipient.id.`);
+                called = true;
+            }).then(() => {
+                assert(called, `bot logic not called.`);
+                done();
+            });
+        });
+
+        it(`should not trust reference.serviceUrl if there is no AppId on the credentials.`, function (done) {
+            let called = false;
+            const adapter = new AdapterUnderTest();
+            adapter.continueConversation(reference, (context) => {
+                assert(context, `context not passed.`);
+                assert(context.activity, `context has no request.`);
+                assert(context.activity.type === 'event', `request has invalid type.`);
+                assert(context.activity.from && context.activity.from.id === reference.user.id, `request has invalid from.id.`);
+                assert(context.activity.recipient && context.activity.recipient.id === reference.bot.id, `request has invalid recipient.id.`);
+                assert(!MicrosoftAppCredentials.isTrustedServiceUrl('https://example.org/channel'));
+                called = true;
+            }).then(() => {
+                assert(called, `bot logic not called.`);
+                done();
+            });
+        });
+
+        it(`should trust reference.serviceUrl if there is an AppId on the credentials.`, function (done) {
+            let called = false;
+            const adapter = new AdapterUnderTest({ appId: '2id', appPassword: '2pw' });
+            adapter.continueConversation(reference, (context) => {
+                assert(context, `context not passed.`);
+                assert(context.activity, `context has no request.`);
+                assert(context.activity.type === 'event', `request has invalid type.`);
+                assert(context.activity.from && context.activity.from.id === reference.user.id, `request has invalid from.id.`);
+                assert(context.activity.recipient && context.activity.recipient.id === reference.bot.id, `request has invalid recipient.id.`);
+                assert(MicrosoftAppCredentials.isTrustedServiceUrl('https://example.org/channel'));
+                called = true;
+            }).then(() => {
+                assert(called, `bot logic not called.`);
+                done();
+            });
+        });
+
         it(`should work with oAuthScope and logic passed in.`, async () => {
             let called = false;
             const adapter = new AdapterUnderTest();