Skip to content

Commit

Permalink
[Streaming] Auth and browser compatibility fixes, linting, refactoring (
Browse files Browse the repository at this point in the history 
#1338)

* patch auth in streaming to send token to channel

* apply PR feedback for BotFrameworkAdapter

* move ws server construction to wsNodeWebSocket constructor for browser compat

* cleanup and cache serviceUrl in authenticateConnection
  • Loading branch information
@stevengum
stevengum authored Oct 23, 2019
1 parent d38f878 commit 5e5cf06
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 49 deletions.
117 changes: 78 additions & 39 deletions libraries/botbuilder/src/botFrameworkAdapter.ts
View file
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
View file
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
View file
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
8 changes: 4 additions & 4 deletions libraries/botframework-streaming/src/webSocket/wsNodeWebSocket.ts
View file
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

0 comments on commit 5e5cf06

Please sign in to comment.