Skip to content

Commit

Permalink
fix: add accept header (#3983) (#3984)
Browse files Browse the repository at this point in the history
* Add accept header to connectorclient outgoing calls

* fix accept header test

* Simplify accept header test

* update accept header failure message

* Add accept header to connectorFactoryImpl connector client options

* fix spacing

* lint
  • Loading branch information
EricDahlvang authored Nov 18, 2021
1 parent ed1831b commit 7b0ff53
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 6 deletions.
17 changes: 14 additions & 3 deletions libraries/botbuilder/src/botFrameworkAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import * as z from 'zod';
import { BotFrameworkHttpAdapter } from './botFrameworkHttpAdapter';
import { ConnectorClientBuilder, Request, Response, ResponseT, WebRequest, WebResponse } from './interfaces';
import { HttpClient, userAgentPolicy } from '@azure/ms-rest-js';
import { HttpClient, RequestPolicyFactory, userAgentPolicy } from '@azure/ms-rest-js';
import { INodeBufferT, INodeSocketT, LogicT } from './zod';
import { arch, release, type } from 'os';
import { delay, retry } from 'botbuilder-stdlib';
Expand Down Expand Up @@ -1514,6 +1514,17 @@ export class BotFrameworkAdapter
value: `${USER_AGENT}${userAgent ?? ''}`,
});

const acceptHeader: RequestPolicyFactory = {
create:(nextPolicy) => ({
sendRequest: (httpRequest)=>{
if(!httpRequest.headers.contains('accept')) {
httpRequest.headers.set('accept', '*/*');
}
return nextPolicy.sendRequest(httpRequest);
}
})
};

// Resolve any user request policy factories, then include our user agent via a factory policy
options.requestPolicyFactories = (defaultRequestPolicyFactories) => {
let defaultFactories = [];
Expand All @@ -1530,11 +1541,11 @@ export class BotFrameworkAdapter

// If the user has supplied custom factories, allow them to optionally set user agent
// before we do.
defaultFactories = [...defaultFactories, setUserAgent];
defaultFactories = [...defaultFactories, acceptHeader, setUserAgent];
} else {
// In the case that there are no user supplied factories, inject our user agent as
// the first policy to ensure none of the default policies override it.
defaultFactories = [setUserAgent, ...defaultRequestPolicyFactories];
defaultFactories = [acceptHeader, setUserAgent, ...defaultRequestPolicyFactories];
}

return defaultFactories;
Expand Down
28 changes: 28 additions & 0 deletions libraries/botbuilder/tests/botFrameworkAdapter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,34 @@ describe('BotFrameworkAdapter', function () {
});
});

it('ConnectorClient should add requestPolicyFactory for accept header', async function () {
let hasAcceptHeader = false;
const mockNextPolicy = {
create: (innerPolicy) => ({
}),
sendRequest: (httpRequest) => {
return {};
}
};
const client = new BotFrameworkAdapter().createConnectorClient('https://localhost')
var length = client._requestPolicyFactories.length;
for (var i = 0; i < length; i++) {
var mockHttp = {
headers: new HttpHeaders()
};

var result = client._requestPolicyFactories[i].create(mockNextPolicy);

result.sendRequest(mockHttp);
if(mockHttp.headers.get("accept") == "*/*") {
hasAcceptHeader = true;
break;
}
}

assert(hasAcceptHeader, 'accept header from connector client should be */*');
});

it('createConnectorClientWithIdentity should throw without identity', async function () {
const adapter = new BotFrameworkAdapter();
await assert.rejects(
Expand Down
17 changes: 14 additions & 3 deletions libraries/botframework-connector/src/auth/connectorFactoryImpl.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { getDefaultUserAgentValue, userAgentPolicy } from '@azure/ms-rest-js';
import { getDefaultUserAgentValue, RequestPolicyFactory, userAgentPolicy } from '@azure/ms-rest-js';
import { ConnectorClient } from '../connectorApi/connectorClient';
import { ConnectorClientOptions } from '../connectorApi/models';
import { ConnectorFactory } from './connectorFactory';
Expand Down Expand Up @@ -50,6 +50,17 @@ export class ConnectorFactoryImpl extends ConnectorFactory {
value: `${USER_AGENT}${userAgent ?? ''}`,
});

const acceptHeader: RequestPolicyFactory = {
create: (nextPolicy) => ({
sendRequest: (httpRequest) => {
if (!httpRequest.headers.contains('accept')) {
httpRequest.headers.set('accept', '*/*');
}
return nextPolicy.sendRequest(httpRequest);
},
}),
};

// Resolve any user request policy factories, then include our user agent via a factory policy
options.requestPolicyFactories = (defaultRequestPolicyFactories) => {
let defaultFactories = [];
Expand All @@ -66,11 +77,11 @@ export class ConnectorFactoryImpl extends ConnectorFactory {

// If the user has supplied custom factories, allow them to optionally set user agent
// before we do.
defaultFactories = [...defaultFactories, setUserAgent];
defaultFactories = [...defaultFactories, setUserAgent, acceptHeader];
} else {
// In the case that there are no user supplied factories, inject our user agent as
// the first policy to ensure none of the default policies override it.
defaultFactories = [setUserAgent, ...defaultRequestPolicyFactories];
defaultFactories = [acceptHeader, setUserAgent, ...defaultRequestPolicyFactories];
}

return defaultFactories;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const {
PasswordServiceClientCredentialFactory,
SkillValidation,
} = require('..');
const { HttpHeaders } = require('@azure/ms-rest-js');

describe('BotFrameworkAuthenticationFactory', function () {
it('should create anonymous BotFrameworkAuthentication', function () {
Expand Down Expand Up @@ -122,6 +123,7 @@ describe('BotFrameworkAuthenticationFactory', function () {
assert.strictEqual(connectorFactory.credentialFactory, credsFactory);

const connectorClient = await connectorFactory.create(HOST_SERVICE_URL, HOST_AUDIENCE);
assertHasAcceptHeader(connectorClient);
assert.strictEqual(connectorClient.credentials.appId, APP_ID);
assert.strictEqual(connectorClient.credentials.appPassword, APP_PASSWORD);
assert.strictEqual(connectorClient.credentials.oAuthScope, HOST_AUDIENCE);
Expand All @@ -132,4 +134,31 @@ describe('BotFrameworkAuthenticationFactory', function () {
assert.strictEqual(userTokenClient.client.credentials.appPassword, APP_PASSWORD);
});
});

function assertHasAcceptHeader(client) {
let hasAcceptHeader = false;
const mockNextPolicy = {
create: (_) => ({}),
sendRequest: (_) => {
return {};
},
};

const length = client._requestPolicyFactories.length;
for (let i = 0; i < length; i++) {
const mockHttp = {
headers: new HttpHeaders(),
};

const result = client._requestPolicyFactories[i].create(mockNextPolicy);

result.sendRequest(mockHttp);
if (mockHttp.headers.get('accept') == '*/*') {
hasAcceptHeader = true;
break;
}
}

assert(hasAcceptHeader, 'accept header from connector client should be */*');
}
});

0 comments on commit 7b0ff53

Please sign in to comment.