Skip to content

Fixed a bug where OAuth cards were opening the file explorer if ngrok… #2155

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

Merged
merged 3 commits into from
Jun 9, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## Fixed
- [client] Added missing content to signed in view of Cosmos DB service dialog and fixed product page link in PR [2150](https://github.com/microsoft/BotFramework-Emulator/pull/2150)
- [docs] Modified CONTRIBUTING.md to include updated information about global dependencies required to build from source in PR [2153](https://github.com/microsoft/BotFramework-Emulator/pull/2153)
- [client] Fixed a bug where trying to open the sign-in link on an OAuth card would open the file explorer if ngrok was not configured in PR [2155](https://github.com/microsoft/BotFramework-Emulator/pull/2155)

## v4.9.0 - 2020 - 05 - 11
## Added
Expand Down
4 changes: 2 additions & 2 deletions packages/app/client/src/hyperlinkHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ export class HyperlinkHandler {
const { TrackEvent } = SharedConstants.Commands.Telemetry;
try {
const parsed = URL.parse(url) || { protocol: '' };
if ((parsed.protocol || '').startsWith('oauth:')) {
if ((parsed.protocol || '').startsWith(SharedConstants.EmulatedOAuthUrlProtocol)) {
this.navigateEmulatedOAuthUrl(url.substring(8));
} else if (parsed.protocol.startsWith('oauthlink:')) {
} else if (parsed.protocol.startsWith(SharedConstants.OAuthUrlProtocol)) {
this.navigateOAuthUrl(url.substring(12));
} else {
this.commandService.remoteCall(TrackEvent, 'app_openLink', { url }).catch(_e => void 0);
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import { createBotFrameworkAuthenticationMiddleware } from '../../handlers/botFr
import { createJsonBodyParserMiddleware } from '../../../utils/jsonBodyParser';

import { mountUserTokenRoutes } from './mountUserTokenRoutes';
import { emulateOAuthCards } from './handlers/emulateOAuthCards';
import { getToken } from './handlers/getToken';
import { signOut } from './handlers/signOut';
import { createTokenResponseHandler } from './handlers/tokenResponse';
Expand Down Expand Up @@ -77,12 +76,6 @@ describe('mountUserTokenRoutes', () => {
getToken
);

expect(emulatorServer.server.post).toHaveBeenCalledWith(
'/api/usertoken/emulateOAuthCards',
botFrameworkAuthentication,
emulateOAuthCards
);

expect(emulatorServer.server.del).toHaveBeenCalledWith(
'/api/usertoken/SignOut',
botFrameworkAuthentication,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import { createBotFrameworkAuthenticationMiddleware } from '../../handlers/botFr
import { createJsonBodyParserMiddleware } from '../../../utils/jsonBodyParser';
import { EmulatorRestServer } from '../../../restServer';

import { emulateOAuthCards } from './handlers/emulateOAuthCards';
import { getToken } from './handlers/getToken';
import { signOut } from './handlers/signOut';
import { createTokenResponseHandler } from './handlers/tokenResponse';
Expand All @@ -49,8 +48,6 @@ export function mountUserTokenRoutes(emulatorServer: EmulatorRestServer) {

server.get('/api/usertoken/GetToken', botFrameworkAuthentication, getBotEndpoint, getToken);

server.post('/api/usertoken/emulateOAuthCards', botFrameworkAuthentication, emulateOAuthCards);

server.del('/api/usertoken/SignOut', botFrameworkAuthentication, getBotEndpoint, signOut);

server.post('/api/usertoken/tokenResponse', jsonBodyParser, createTokenResponseHandler(emulatorServer));
Expand Down
14 changes: 10 additions & 4 deletions packages/app/main/src/server/utils/oauthLinkEncoder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
//

import { AttachmentContentTypes } from '@bfemulator/sdk-shared';
import { SharedConstants } from '@bfemulator/app-shared';

import { OAuthLinkEncoder } from './oauthLinkEncoder';

Expand Down Expand Up @@ -111,15 +112,16 @@ describe('The OauthLinkEncoder', () => {
});
});

it('should throw when an error occurs retrieving the link while calling resolveOAuthCards', async () => {
it('should throw when an error occurs retrieving the link while calling resolveOAuthCards, but should also generate an emulated oauth link', async () => {
ok = false;
statusText = 'oh noes!';
const mockActivity = {
attachments: [
{
contentType: AttachmentContentTypes.oAuthCard,
content: {
buttons: [{ type: 'signin' }],
buttons: [{ type: 'signin', value: undefined }],
connectionName: 'oauth-connection',
},
},
],
Expand All @@ -129,7 +131,11 @@ describe('The OauthLinkEncoder', () => {
await encoder.resolveOAuthCards(mockActivity);
expect(false);
} catch (e) {
expect(e.message).toEqual(statusText);
expect(e.message).toEqual(`Failed to generate an actual sign-in link: Error: ${statusText}`);
expect(mockActivity.attachments[0].content.buttons[0].type).toBe('openUrl');
expect(mockActivity.attachments[0].content.buttons[0].value).toBe(
SharedConstants.EmulatedOAuthUrlProtocol + '//' + 'oauth-connection' + '&&&' + 'testConversation'
);
}
});

Expand All @@ -150,7 +156,7 @@ describe('The OauthLinkEncoder', () => {
await encoder.resolveOAuthCards(mockActivity);
expect(false);
} catch (e) {
expect(e.message).toEqual("I'm in your throw!");
expect(e.message).toEqual(`Failed to generate an actual sign-in link: Error: ${"I'm in your throw!"}`);
}
});

Expand Down
25 changes: 17 additions & 8 deletions packages/app/main/src/server/utils/oauthLinkEncoder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import * as crypto from 'crypto';

import { Attachment, OAuthCard } from 'botframework-schema';
import { SharedConstants } from '@bfemulator/app-shared';
import { AttachmentContentTypes } from '@bfemulator/sdk-shared';
import { Activity } from 'botframework-schema';

Expand All @@ -42,9 +43,6 @@ import { EmulatorRestServer } from '../restServer';
import { uniqueId } from './uniqueId';

export class OAuthLinkEncoder {
public static OAuthUrlProtocol: string = 'oauthlink:';
public static EmulateOAuthCards: boolean = false;

private readonly authorizationHeader: string;
private readonly conversationId: string;
private activity: Activity;
Expand Down Expand Up @@ -75,10 +73,21 @@ export class OAuthLinkEncoder {
const oauthCard: OAuthCard = attachment.content as OAuthCard;
if (oauthCard.buttons && oauthCard.buttons.length === 1) {
const cardAction = oauthCard.buttons[0];
if (cardAction.type === 'signin' && !cardAction.value && !OAuthLinkEncoder.EmulateOAuthCards) {
const link = await this.getSignInLink(oauthCard.connectionName, codeChallenge);
cardAction.value = link;
cardAction.type = 'openUrl';
if (cardAction.type === 'signin' && !cardAction.value) {
// generate a sign-in link for the oauth card and assign it to the button
try {
const link = await this.getSignInLink(oauthCard.connectionName, codeChallenge);
cardAction.value = link;
cardAction.type = 'openUrl';
} catch (e) {
// failed to generate a sign-in link, fall back to an emulated sign-in token
const link =
SharedConstants.EmulatedOAuthUrlProtocol + '//' + oauthCard.connectionName + '&&&' + this.conversationId;
cardAction.value = link;
cardAction.type = 'openUrl';

throw new Error(`Failed to generate an actual sign-in link: ${e}`);
}
}
}
}
Expand Down Expand Up @@ -134,7 +143,7 @@ export class OAuthLinkEncoder {
});
if (response.ok) {
const link = await response.text();
return OAuthLinkEncoder.OAuthUrlProtocol + '//' + link + '&&&' + this.conversationId;
return SharedConstants.OAuthUrlProtocol + '//' + link + '&&&' + this.conversationId;
}
errorMessage = response.statusText;
} catch (e) {
Expand Down
2 changes: 2 additions & 0 deletions packages/app/shared/src/constants/sharedConstants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export const SharedConstants = {
TEMP_BOT_IN_MEMORY_PATH: 'TEMP_BOT_IN_MEMORY',
EDITOR_KEY_PRIMARY,
EDITOR_KEY_SECONDARY,
EmulatedOAuthUrlProtocol: 'oauth:',
OAuthUrlProtocol: 'oauthlink:',

/** Names of commands used in both main and client */
Commands: {
Expand Down