Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Streaming] Auth and browser compatibility fixes, linting, refactoring #1338

Merged
merged 5 commits into from
Oct 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 78 additions & 39 deletions libraries/botbuilder/src/botFrameworkAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,10 @@ const US_GOV_OAUTH_ENDPOINT = 'https://api.botframework.azure.us';
// This key is exported internally so that the TeamsActivityHandler will not overwrite any already set InvokeResponses.
export const INVOKE_RESPONSE_KEY: symbol = Symbol('invokeResponse');
const defaultPipeName = 'bfv4.pipes';
const VERSION_PATH:string = '/api/version';
const MESSAGES_PATH:string = '/api/messages';
const GET:string = 'GET';
const POST:string = 'POST';
const VERSION_PATH: string = '/api/version';
const MESSAGES_PATH: string = '/api/messages';
const GET: string = 'GET';
const POST: string = 'POST';

/**
* A [BotAdapter](xref:botbuilder-core.BotAdapter) that can connect a bot to a service endpoint.
Expand Down Expand Up @@ -1000,28 +1000,25 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide
response.statusCode = StatusCodes.BAD_REQUEST;
response.setBody(`Request missing verb and/or path. Verb: ${ request.verb }. Path: ${ request.path }`);
return response;
}
}

if (request.verb.toLocaleUpperCase() === GET && request.path.toLocaleLowerCase() === VERSION_PATH) {
response.statusCode = StatusCodes.OK;
response.setBody({UserAgent: USER_AGENT});
if (request.verb.toLocaleUpperCase() !== POST && request.verb.toLocaleUpperCase() !== GET) {
response.statusCode = StatusCodes.METHOD_NOT_ALLOWED;
response.setBody(`Invalid verb received. Only GET and POST are accepted. Verb: ${ request.verb }`);
}

return response;
if (request.path.toLocaleLowerCase() === VERSION_PATH) {
return await this.handleVersionRequest(request, response);
}

// Convert the StreamingRequest into an activity the Adapter can understand.
let body: Activity;
try {
body = await this.readRequestBodyAsString(request);

} catch (error) {
response.statusCode = StatusCodes.BAD_REQUEST;
response.setBody(`Unable to read request body. Error: ${ error }`);
return response;
}

if (request.verb.toLocaleUpperCase() !== POST) {
response.statusCode = StatusCodes.METHOD_NOT_ALLOWED;
response.setBody(`Method ${ request.verb.toLocaleUpperCase() } not allowed. Expected POST.`);
response.setBody(`Request body missing or malformed: ${ error }`);
return response;
}

Expand All @@ -1031,7 +1028,13 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide
return response;
}

try {
if (request.verb.toLocaleUpperCase() !== POST) {
response.statusCode = StatusCodes.METHOD_NOT_ALLOWED;
response.setBody(`Invalid verb received for ${ request.verb.toLocaleLowerCase() }. Only GET and POST are accepted. Verb: ${ request.verb }`);
return response;
}

try {
let context = new TurnContext(this, body);
await this.runMiddleware(context, this.logic);

Expand All @@ -1057,6 +1060,39 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide
return response;
}

private async handleVersionRequest(request: IReceiveRequest, response: StreamingResponse): Promise<StreamingResponse> {
if (request.verb.toLocaleUpperCase() === GET) {
response.statusCode = StatusCodes.OK;

if (!this.credentials.appId) {
response.setBody({ UserAgent: USER_AGENT });
return response;
}

let token = '';
try {
token = await this.credentials.getToken();

} catch (err) {
/**
* In reality a missing BotToken will cause the channel to close the connection,
* but we still send the response and allow the channel to make that decision
* instead of proactively disconnecting. This allows the channel to know why
* the connection has been closed and make the choice not to make endless reconnection
* attempts that will end up right back here.
*/
console.error(err.message);
}
response.setBody({ UserAgent: USER_AGENT, BotToken: token });

} else {
response.statusCode = StatusCodes.METHOD_NOT_ALLOWED;
response.setBody(`Invalid verb received for path: ${ request.path }. Only GET is accepted. Verb: ${ request.verb }`);
}

return response;
}

/**
* Creates an OAuth API client.
*
Expand Down Expand Up @@ -1114,7 +1150,7 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide
protected checkEmulatingOAuthCards(context: TurnContext): void {
if (!this.isEmulatingOAuthCards &&
context.activity.channelId === 'emulator' &&
(!this.credentials.appId || !this.credentials.appPassword)) {
(!this.credentials.appId)) {
this.isEmulatingOAuthCards = true;
}
}
Expand Down Expand Up @@ -1153,19 +1189,22 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide
return serviceUrl && !serviceUrl.toLowerCase().startsWith('http');
}

private async authenticateConnection(req: WebRequest, appId?: string, appPassword?: string, channelService?: string): Promise<boolean> {
if (!appId || !appPassword) {
private async authenticateConnection(req: WebRequest, channelService?: string): Promise<void> {
if (!this.credentials.appId) {
// auth is disabled
return true;
return;
}

let authHeader: string = req.headers.authorization || req.headers.Authorization || '';
let channelIdHeader: string = req.headers.channelid || req.headers.ChannelId || req.headers.ChannelID || '';
let credentials = new MicrosoftAppCredentials(appId, appPassword);
let credentialProvider = new SimpleCredentialProvider(credentials.appId, credentials.appPassword);
let claims = await JwtTokenValidation.validateAuthHeader(authHeader, credentialProvider, channelService, channelIdHeader);
const authHeader: string = req.headers.authorization || req.headers.Authorization || '';
const channelIdHeader: string = req.headers.channelid || req.headers.ChannelId || req.headers.ChannelID || '';
// Validate the received Upgrade request from the channel.
const claims = await JwtTokenValidation.validateAuthHeader(authHeader, this.credentialsProvider, channelService, channelIdHeader);

// Add serviceUrl from claim to static cache to trigger token refreshes.
const serviceUrl = claims.getClaimValue(AuthenticationConstants.ServiceUrlClaim);
MicrosoftAppCredentials.trustServiceUrl(serviceUrl);

return claims.isAuthenticated;
if (!claims.isAuthenticated) { throw new Error('Unauthorized Access. Request is not authorized'); }
}

/**
Expand Down Expand Up @@ -1203,15 +1242,19 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide

// Restify-specific check.
if (typeof((res as any).claimUpgrade) !== 'function') {
throw new Error("ClaimUpgrade is required for creating WebSocket connection.");
throw new Error('ClaimUpgrade is required for creating WebSocket connection.');
}

const authenticated = await this.authenticateConnection(req, this.settings.appId, this.settings.appPassword, this.settings.channelService);
if (!authenticated) {
try {
await this.authenticateConnection(req, this.settings.channelService);
} catch (err) {
// Set the correct status code for the socket to send back to the channel.
res.status(StatusCodes.UNAUTHORIZED);
return Promise.resolve();
res.send(err.message);
// Re-throw the error so the developer will know what occurred.
throw err;
}

const upgrade = (res as any).claimUpgrade();
const socket = this.webSocketFactory.createWebSocket(req as IncomingMessage, upgrade.socket, upgrade.head);

Expand All @@ -1227,13 +1270,9 @@ export class BotFrameworkAdapter extends BotAdapter implements IUserTokenProvide
await this.streamingServer.start();
}

private async readRequestBodyAsString(request: IReceiveRequest): Promise<Activity> {
try {
let contentStream = request.streams[0];
return await contentStream.readAsJson<Activity>();
} catch (error) {
return Promise.reject(error);
}
private async readRequestBodyAsString(request: IReceiveRequest): Promise<Activity> {
const contentStream = request.streams[0];
return await contentStream.readAsJson<Activity>();
}
}

Expand Down
1 change: 0 additions & 1 deletion libraries/botbuilder/tests/botFrameworkAdapter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const assert = require('assert');
const { TurnContext } = require('botbuilder-core');
const connector = require('botframework-connector');
const { BotFrameworkAdapter } = require('../');
const os = require('os');

const reference = {
activityId: '1234',
Expand Down
10 changes: 5 additions & 5 deletions libraries/botframework-streaming/src/webSocket/nodeWebSocket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ import { Socket } from 'net';
import { Watershed } from 'watershed';
import { ISocket } from '../interfaces/ISocket';

const SHED = new Watershed();

export class NodeWebSocket implements ISocket {
private waterShedSocket: any;
private connected: boolean;
protected watershedShed: Watershed;

/**
* Creates a new instance of the [NodeWebSocket](xref:botframework-streaming.NodeWebSocket) class.
Expand All @@ -25,6 +24,7 @@ export class NodeWebSocket implements ISocket {
public constructor(waterShedSocket?) {
this.waterShedSocket = waterShedSocket;
this.connected = !!waterShedSocket;
this.watershedShed = new Watershed();
}

/**
Expand All @@ -34,7 +34,7 @@ export class NodeWebSocket implements ISocket {
* @param head Buffer
*/
public create(req: IncomingMessage, socket: Socket, head: Buffer): void {
this.waterShedSocket = SHED.accept(req, socket, head);
this.waterShedSocket = this.watershedShed.accept(req, socket, head);
this.connected = true;
}

Expand Down Expand Up @@ -62,7 +62,7 @@ export class NodeWebSocket implements ISocket {
*/
public async connect(serverAddress, port = 8082): Promise<void> {
// Following template from https://github.com/joyent/node-watershed#readme
const wskey = SHED.generateKey();
const wskey = this.watershedShed.generateKey();
const options = {
port: port,
hostname: serverAddress,
Expand All @@ -75,7 +75,7 @@ export class NodeWebSocket implements ISocket {
const req = request(options);
req.end();
req.on('upgrade', function(res, socket, head): void {
SHED.connect(res, socket, head, wskey);
this.watershedShed.connect(res, socket, head, wskey);
});

this.connected = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@ import { Socket } from 'net';
import * as WebSocket from 'ws';
import * as crypto from 'crypto';

const WS_SERVER = new WebSocket.Server({ noServer: true });

// Taken from watershed, these needs to be investigated.
const NONCE_LENGTH = 16;

export class WsNodeWebSocket implements ISocket {
private wsSocket: WebSocket;
private connected: boolean;
protected wsServer: WebSocket.Server;

/**
* Creates a new instance of the [WsNodeWebSocket](xref:botframework-streaming.WsNodeWebSocket) class.
Expand All @@ -29,6 +28,7 @@ export class WsNodeWebSocket implements ISocket {
public constructor(wsSocket?: WebSocket) {
this.wsSocket = wsSocket;
this.connected = !!wsSocket;
this.wsServer = new WebSocket.Server({ noServer: true });
}

/**
Expand All @@ -40,7 +40,7 @@ export class WsNodeWebSocket implements ISocket {
public async create(req: IncomingMessage, socket: Socket, head: Buffer): Promise<void> {
return new Promise<void>((resolve, reject) => {
try {
WS_SERVER.handleUpgrade(req, socket, head, (websocket) => {
this.wsServer.handleUpgrade(req, socket, head, (websocket) => {
this.wsSocket = websocket;
this.connected = true;
resolve();
Expand Down Expand Up @@ -90,7 +90,7 @@ export class WsNodeWebSocket implements ISocket {
req.on('upgrade', (res, socket, head): void => {
// @types/ws does not contain the signature for completeUpgrade
// https://github.com/websockets/ws/blob/0a612364e69fc07624b8010c6873f7766743a8e3/lib/websocket-server.js#L269
(WS_SERVER as any).completeUpgrade(wskey, undefined, res, socket, head, (websocket): void => {
(this.wsServer as any).completeUpgrade(wskey, undefined, res, socket, head, (websocket): void => {
this.wsSocket = websocket;
this.connected = true;
});
Expand Down