From ed5535db92ca9e98bcc09bdc7e9c3574d0a8e1aa Mon Sep 17 00:00:00 2001 From: stevengum <14935595+stevengum@users.noreply.github.com> Date: Tue, 10 Mar 2020 16:50:15 -0700 Subject: [PATCH 1/2] trust reference.serviceUrl if credentials.appId is truthy --- .../botbuilder/src/botFrameworkAdapter.ts | 19 ++++-- .../tests/botFrameworkAdapter.test.js | 66 ++++++++++++++----- 2 files changed, 63 insertions(+), 22 deletions(-) diff --git a/libraries/botbuilder/src/botFrameworkAdapter.ts b/libraries/botbuilder/src/botFrameworkAdapter.ts index 3b868a9d3f..66b7b8db53 100644 --- a/libraries/botbuilder/src/botFrameworkAdapter.ts +++ b/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(this.credentials.appId, audience); + } } const connectorClient = this.createConnectorClientInternal(reference.serviceUrl, credentials); diff --git a/libraries/botbuilder/tests/botFrameworkAdapter.test.js b/libraries/botbuilder/tests/botFrameworkAdapter.test.js index c9ef5b2a9c..84a15fecbe 100644 --- a/libraries/botbuilder/tests/botFrameworkAdapter.test.js +++ b/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(); From cf070c6eb2f8708bb37bb9142657f54f7e20e013 Mon Sep 17 00:00:00 2001 From: Steven Gum <14935595+stevengum@users.noreply.github.com> Date: Wed, 11 Mar 2020 22:26:53 -0700 Subject: [PATCH 2/2] address @stevengum's PR feedback how meta --- libraries/botbuilder/src/botFrameworkAdapter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/botbuilder/src/botFrameworkAdapter.ts b/libraries/botbuilder/src/botFrameworkAdapter.ts index 66b7b8db53..065d6a33ad 100644 --- a/libraries/botbuilder/src/botFrameworkAdapter.ts +++ b/libraries/botbuilder/src/botFrameworkAdapter.ts @@ -283,7 +283,7 @@ export class BotFrameworkAdapter extends BotAdapter implements ExtendedUserToken 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); + credentials = await this.buildCredentials(credentials.appId, audience); } }