Skip to content

Commit

Permalink
Added error logging to OAuth signin link generation flow.
Browse files Browse the repository at this point in the history
  • Loading branch information
tonyanziano committed Aug 19, 2019
1 parent 508bd95 commit 56cbd07
Show file tree
Hide file tree
Showing 5 changed files with 195 additions and 20 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [main] Added End-to-End tests using Spectron in PR [1696](https://github.com/microsoft/BotFramework-Emulator/pull/1696)
- [main] New Conversation: send a single conversation update activity including bot and user as members added [1709](https://github.com/microsoft/BotFramework-Emulator/pull/1709)
- [app] Consolidated application state store and removed the need for explicit state synchronization between the main and renderer processes in PR [1721](https://github.com/microsoft/BotFramework-Emulator/pull/1721)
- [main] Added logging to OAuth signin link generation flow in PR [1745](https://github.com/microsoft/BotFramework-Emulator/pull/1745)

## Fixed
- [main] Fixed bug where opening a chat via URL was sending two conversation updates in PR [1735](https://github.com/microsoft/BotFramework-Emulator/pull/1735)
Expand Down
30 changes: 30 additions & 0 deletions packages/app/main/src/ngrok.spec.ts
Expand Up @@ -30,6 +30,9 @@
// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
//

import { join } from 'path';

import './fetchProxy';
import { intervals, NgrokInstance } from './ngrok';

Expand Down Expand Up @@ -74,12 +77,18 @@ jest.mock('node-fetch', () => {
};
});

const mockExistsSync = jest.fn(() => true);
jest.mock('fs', () => ({
existsSync: () => mockExistsSync(),
}));

describe('the ngrok ', () => {
const ngrok = new NgrokInstance();

afterEach(() => {
ngrok.kill();
});

it('should spawn ngrok successfully when the happy path is followed', async () => {
const result = await ngrok.connect({
addr: 61914,
Expand Down Expand Up @@ -158,4 +167,25 @@ describe('the ngrok ', () => {
}
expect(threw.toString()).toBe('Error: oh noes!');
});

it('should throw if it failed to find an ngrok executable at the specified path', async () => {
mockExistsSync.mockReturnValueOnce(false);

const path = join('Applications', 'ngrok');
let thrown;
try {
await ngrok.connect({
addr: 61914,
path,
name: 'c87d3e60-266e-11e9-9528-5798e92fee89',
proto: 'http',
});
} catch (e) {
thrown = e;
}

expect(thrown.toString()).toBe(
`Error: Could not find ngrok executable at path: ${path}. Make sure that the correct path to ngrok is configured in the Emulator app settings. Ngrok is required to receive a token from the Bot Framework token service.`
);
});
});
8 changes: 8 additions & 0 deletions packages/app/main/src/ngrok.ts
Expand Up @@ -34,6 +34,7 @@ import { ChildProcess, spawn } from 'child_process';
import { EventEmitter } from 'events';
import { platform } from 'os';
import * as path from 'path';
import { existsSync } from 'fs';
import { clearTimeout, setTimeout } from 'timers';

import { uniqueId } from '@bfemulator/sdk-shared';
Expand Down Expand Up @@ -221,6 +222,13 @@ export class NgrokInstance {
const folder = opts.path ? path.dirname(opts.path) : path.join(__dirname, 'bin');
const args = ['start', '--none', '--log=stdout', `--region=${opts.region}`];
const ngrokPath = path.join(folder, filename);
if (!existsSync(ngrokPath)) {
throw new Error(
`Could not find ngrok executable at path: ${ngrokPath}. ` +
`Make sure that the correct path to ngrok is configured in the Emulator app settings. ` +
`Ngrok is required to receive a token from the Bot Framework token service.`
);
}
const ngrok = spawn(ngrokPath, args, { cwd: folder });
// Errors are emitted instead of throwing since ngrok is a long running process
ngrok.on('error', e => this.ngrokEmitter.emit('error', e));
Expand Down
@@ -0,0 +1,139 @@
//
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license.
//
// Microsoft Bot Framework: http://botframework.com
//
// Bot Framework Emulator Github:
// https://github.com/Microsoft/BotFramwork-Emulator
//
// Copyright (c) Microsoft Corporation
// All rights reserved.
//
// MIT License:
// Permission is hereby granted, free of charge, to any person obtaining
// a copy of this software and associated documentation files (the
// "Software"), to deal in the Software without restriction, including
// without limitation the rights to use, copy, modify, merge, publish,
// distribute, sublicense, and/or sell copies of the Software, and to
// permit persons to whom the Software is furnished to do so, subject to
// the following conditions:
//
// The above copyright notice and this permission notice shall be
// included in all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED ""AS IS"", WITHOUT WARRANTY OF ANY KIND,
// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
//

import { OK } from 'http-status-codes';

import replyToActivity from './replyToActivity';

const mockResolveOAuthCards = jest.fn().mockResolvedValue(true);
jest.mock('../../utils/oauthLinkEncoder', () => {
return jest.fn().mockImplementation(() => {
return { resolveOAuthCards: mockResolveOAuthCards };
});
});

describe('replyToActivity route middleware', () => {
const mockReq: any = {
body: {
id: 'someActivityId',
},
conversation: {
postActivityToUser: jest.fn(() => 'post activity to user response'),
},
headers: {
authorization: 'Bearer <token>',
},
params: {
activityId: 'someOtherActivityId',
conversationId: 'someConversationId',
},
};
const mockRes: any = {
end: jest.fn(() => null),
send: jest.fn(() => null),
};
const mockNext: any = jest.fn(() => null);
const mockBotEmulator: any = {
facilities: {
logger: {
logException: jest.fn(() => null),
},
},
};

beforeEach(() => {
mockResolveOAuthCards.mockClear();
mockReq.conversation.postActivityToUser.mockClear();
mockRes.end.mockClear();
mockRes.send.mockClear();
mockNext.mockClear();
mockBotEmulator.facilities.logger.logException.mockClear();
});

it('should resolve any OAuth cards, post the activity to the user, and send an OK response', async () => {
replyToActivity(mockBotEmulator)(mockReq, mockRes, mockNext);

// since the middleware is not an async function, wait for the async operations to complete
await new Promise(resolve => setTimeout(resolve, 500));

expect(mockReq.conversation.postActivityToUser).toHaveBeenCalledWith({
...mockReq.body,
replyToId: mockReq.params.activityId,
});
expect(mockRes.send).toHaveBeenCalledWith(OK, 'post activity to user response');
expect(mockRes.end).toHaveBeenCalled();
expect(mockNext).toHaveBeenCalled();
});

it('should resolve any OAuth cards, post the activity (with a null id) to the user, and send an OK response', async () => {
mockReq.body.id = undefined;

replyToActivity(mockBotEmulator)(mockReq, mockRes, mockNext);

// since the middleware is not an async function, wait for the async operations to complete
await new Promise(resolve => setTimeout(resolve, 500));

expect(mockReq.conversation.postActivityToUser).toHaveBeenCalledWith({
...mockReq.body,
replyToId: mockReq.params.activityId,
id: null,
});
expect(mockRes.send).toHaveBeenCalledWith(OK, 'post activity to user response');
expect(mockRes.end).toHaveBeenCalled();
expect(mockNext).toHaveBeenCalled();

mockReq.body.id = 'someActivityId';
});

it('should log any exceptions from OAuth signin in generation before posting the activity to the user', async () => {
const ngrokError = new Error('Failed to spawn ngrok');
mockResolveOAuthCards.mockRejectedValueOnce(ngrokError);
replyToActivity(mockBotEmulator)(mockReq, mockRes, mockNext);

// since the middleware is not an async function, wait for the async operations to complete
await new Promise(resolve => setTimeout(resolve, 500));

expect(mockBotEmulator.facilities.logger.logException).toHaveBeenCalledWith('someConversationId', ngrokError);
expect(mockBotEmulator.facilities.logger.logException).toHaveBeenCalledWith(
'someConversationId',
new Error('Falling back to emulated OAuth token.')
);
expect(mockReq.conversation.postActivityToUser).toHaveBeenCalledWith({
...mockReq.body,
replyToId: mockReq.params.activityId,
});
expect(mockRes.send).toHaveBeenCalledWith(OK, 'post activity to user response');
expect(mockRes.end).toHaveBeenCalled();
expect(mockNext).toHaveBeenCalled();
});
});
Expand Up @@ -47,37 +47,34 @@ export default function replyToActivity(botEmulator: BotEmulator) {
const conversationParameters: ConversationAPIPathParameters = req.params;

try {
// TODO: Need to re-enable
// VersionManager.checkVersion(req.header("User-agent"));

activity.id = activity.id || null;
activity.replyToId = req.params.activityId;

// if we found the activity to reply to
// if (!conversation.activities.find((existingActivity, index, obj) => existingActivity.id == activity.replyToId))
// throw createAPIException(HttpStatus.NOT_FOUND, ErrorCodes.BadArgument, "replyToId is not a known activity id");

const continuation = function(): void {
const response: ResourceResponse = (req as any).conversation.postActivityToUser(activity);

res.send(HttpStatus.OK, response);
res.end();
};

const visitor = new OAuthLinkEncoder(
botEmulator,
req.headers.authorization as string,
activity,
conversationParameters.conversationId
);
visitor.resolveOAuthCards(activity).then(
(value?: any) => {
continuation();
},
(_reason: any) => {
const { conversationId } = conversationParameters;
const visitor = new OAuthLinkEncoder(botEmulator, req.headers.authorization as string, activity, conversationId);
visitor
.resolveOAuthCards(activity)
.then(_ => {
continuation();
}
);
})
.catch(
// failed to generate an OAuth signin link
(reason: any) => {
botEmulator.facilities.logger.logException(conversationId, reason);
botEmulator.facilities.logger.logException(
conversationId,
new Error('Falling back to emulated OAuth token.')
);
continuation();
}
);
} catch (err) {
sendErrorResponse(req, res, next, err);
}
Expand Down

0 comments on commit 56cbd07

Please sign in to comment.