Skip to content

Commit

Permalink
cherry-pick (4.13): Fix double evaluation and filter sensitive settin…
Browse files Browse the repository at this point in the history
…gs (#3745)

* Fix double evaluation and filter sensitive settings

* refine code

* update
  • Loading branch information
Danieladu authored Jun 15, 2021
1 parent 0ee95d2 commit 1b41fd9
Show file tree
Hide file tree
Showing 12 changed files with 194 additions and 58 deletions.
4 changes: 4 additions & 0 deletions libraries/botbuilder-core/src/activityFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ export class ActivityFactory {
* @param lgResult string result from languageGenerator.
*/
public static fromObject(lgResult: any): Partial<Activity> {
if (lgResult == null) {
return { type: ActivityTypes.Message };
}

if (typeof lgResult === 'string') {
return this.buildActivityFromText(lgResult.trim());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,20 @@
{
"$kind": "Microsoft.SendActivity",
"activity": "${user.profile.name}"
},
{
"$kind": "Microsoft.TextInput",
"property": "user.profile.newname",
"prompt": "Please input the name"
},
{
"$kind": "Microsoft.SetProperty",
"property": "user.profile.result",
"value": "=user.profile.newname"
},
{
"$kind": "Microsoft.SendActivity",
"activity": "${user.profile.result}"
}
]
}
Expand All @@ -111,6 +125,18 @@
{
"$kind": "Microsoft.Test.AssertReply",
"text": "Jack"
},
{
"$kind": "Microsoft.Test.AssertReply",
"text": "Please input the name"
},
{
"$kind": "Microsoft.Test.UserSays",
"text": "=cool"
},
{
"$kind": "Microsoft.Test.AssertReply",
"text": "=cool"
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
},
{
"$kind": "Microsoft.SendActivity",
"activity": "${settings.MicrosoftAppPassword}"
"activity": "${coalesce(settings.MicrosoftAppPassword, 'no-access')}"
},
{
"$kind": "Microsoft.SendActivity",
"activity": "${settings.ApplicationInsightsInstrumentationKey}"
"activity": "${coalesce(settings.ApplicationInsights.InstrumentationKey, 'no-access')}"
}
]
}
Expand All @@ -36,11 +36,11 @@
},
{
"$kind": "Microsoft.Test.AssertReply",
"text": "MICROSOFT_APP_PASSWORD"
"text": "no-access"
},
{
"$kind": "Microsoft.Test.AssertReply",
"text": "00000000-0000-0000-0000-000000000000"
"text": "no-access"
}
]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,17 @@ describe('SettingsStateTests', function () {
resourceExplorer = makeResourceExplorer('SettingsStateTests');
});

process.env['MicrosoftAppId'] = 'MICROSOFT_APP_ID';
process.env['MicrosoftAppPassword'] = 'MICROSOFT_APP_PASSWORD';
process.env['ApplicationInsightsInstrumentationKey'] = '00000000-0000-0000-0000-000000000000';
beforeEach(() => {
process.env['MicrosoftAppId'] = 'MICROSOFT_APP_ID';
process.env['MicrosoftAppPassword'] = 'MICROSOFT_APP_PASSWORD';
process.env['ApplicationInsights:InstrumentationKey'] = '00000000-0000-0000-0000-000000000000';
});

afterEach(() => {
delete process.env['MicrosoftAppId'];
delete process.env['MicrosoftAppPassword'];
delete process.env['ApplicationInsights:InstrumentationKey'];
});

it('SettingsTest', async () => {
await TestUtils.runTestScript(resourceExplorer, 'SettingsStateTests_SettingsTest');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/

import {
ValueExpression,
BoolExpression,
Expand All @@ -23,7 +24,7 @@ import {
DialogConfiguration,
} from 'botbuilder-dialogs';
import { DialogExpression } from '../expressions';
import { replaceJsonRecursively } from '../jsonExtensions';
import { evaluateExpression } from '../jsonExtensions';
import { DialogExpressionConverter } from '../converters';

export interface BaseInvokeDialogConfiguration extends DialogConfiguration {
Expand Down Expand Up @@ -140,13 +141,7 @@ export class BaseInvokeDialog<O extends object = {}>

for (const key in bindingOptions) {
const bindingValue = bindingOptions[key];
let value = new ValueExpression(bindingValue).getValue(dc.state);

if (value) {
value = replaceJsonRecursively(dc.state, value);
}

boundOptions[key] = value;
boundOptions[key] = evaluateExpression(dc.state, new ValueExpression(bindingValue));
}

return boundOptions;
Expand Down
11 changes: 4 additions & 7 deletions libraries/botbuilder-dialogs-adaptive/src/actions/endDialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/

import {
BoolExpression,
BoolExpressionConverter,
Expand All @@ -20,7 +21,7 @@ import {
DialogContext,
DialogTurnResult,
} from 'botbuilder-dialogs';
import { replaceJsonRecursively } from '../jsonExtensions';
import { evaluateExpression } from '../jsonExtensions';

export interface EndDialogConfiguration extends DialogConfiguration {
value?: unknown | ValueExpression;
Expand Down Expand Up @@ -77,11 +78,7 @@ export class EndDialog<O extends object = {}> extends Dialog<O> implements EndDi
}

if (this.value) {
let value = this.value.getValue(dc.state);

if (value) {
value = replaceJsonRecursively(dc.state, value);
}
const value = evaluateExpression(dc.state, this.value);

return await this.endParentDialog(dc, value);
}
Expand All @@ -92,7 +89,7 @@ export class EndDialog<O extends object = {}> extends Dialog<O> implements EndDi
/**
* Ends the parent [Dialog](xref:botbuilder-dialogs.Dialog).
* @param dc The [DialogContext](xref:botbuilder-dialogs.DialogContext) for the current turn of conversation.
* @param result Optional. Value returned from the dialog that was called. The type
* @param result Optional. Value returned from the dialog that was called. The type
* of the value returned is dependent on the child dialog.
* @returns A `Promise` representing the asynchronous operation.
*/
Expand Down
17 changes: 8 additions & 9 deletions libraries/botbuilder-dialogs-adaptive/src/actions/httpRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/
import fetch from 'node-fetch';
import { Response, Headers } from 'node-fetch';

import {
BoolExpression,
BoolExpressionConverter,
Expand All @@ -27,7 +28,7 @@ import {
DialogTurnResult,
DialogConfiguration,
} from 'botbuilder-dialogs';
import { replaceJsonRecursively } from '../jsonExtensions';
import { evaluateExpression } from '../jsonExtensions';

type HeadersInput = Record<string, string>;
type HeadersOutput = Record<string, StringExpression>;
Expand Down Expand Up @@ -275,14 +276,12 @@ export class HttpRequest<O extends object = {}> extends Dialog<O> implements Htt
instanceHeaders['Content-Type'] = contentType;

let instanceBody: string;
if (this.body) {
const body = this.body.getValue(dc.state);
if (body) {
if (typeof body === 'string') {
instanceBody = body;
} else {
instanceBody = JSON.stringify(replaceJsonRecursively(dc.state, Object.assign({}, body)));
}
const body = evaluateExpression(dc.state, this.body);
if (body) {
if (typeof body === 'string') {
instanceBody = body;
} else {
instanceBody = JSON.stringify(Object.assign({}, body));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
Expression,
} from 'adaptive-expressions';
import { StringUtils } from 'botbuilder';

import {
Converter,
ConverterFactory,
Expand All @@ -21,7 +22,7 @@ import {
DialogContext,
DialogTurnResult,
} from 'botbuilder-dialogs';
import { replaceJsonRecursively } from '../jsonExtensions';
import { evaluateExpression } from '../jsonExtensions';

type AssignmentInput<T> = {
property: string;
Expand Down Expand Up @@ -105,11 +106,8 @@ export class SetProperties<O extends object = {}> extends Dialog<O> implements S

for (let i = 0; i < this.assignments.length; i++) {
const assignment = this.assignments[i];
let value = assignment.value.getValue(dc.state);

if (value) {
value = replaceJsonRecursively(dc.state, value);
}
const value = evaluateExpression(dc.state, assignment.value);

const property = assignment.property.getValue(dc.state);
dc.state.setValue(property, value);
Expand Down
10 changes: 3 additions & 7 deletions libraries/botbuilder-dialogs-adaptive/src/actions/setProperty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/

import {
BoolExpression,
BoolExpressionConverter,
Expand All @@ -22,7 +23,7 @@ import {
DialogContext,
DialogTurnResult,
} from 'botbuilder-dialogs';
import { replaceJsonRecursively } from '../jsonExtensions';
import { evaluateExpression } from '../jsonExtensions';

export interface SetPropertyConfiguration extends DialogConfiguration {
property?: string | Expression | StringExpression;
Expand Down Expand Up @@ -106,13 +107,8 @@ export class SetProperty<O extends object = {}> extends Dialog<O> implements Set
throw new Error(`${this.id}: no 'value' expression specified.`);
}

// Evaluate expression and save value
const property = this.property.getValue(dc.state);
let value = this.value.getValue(dc.state);

if (value) {
value = replaceJsonRecursively(dc.state, value);
}
const value = evaluateExpression(dc.state, this.value);

dc.state.setValue(property, value);

Expand Down
10 changes: 10 additions & 0 deletions libraries/botbuilder-dialogs-adaptive/src/jsonExtensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,13 @@ export function replaceJsonRecursively(state: DialogStateManager, unit: object):

return unit;
}

/**
* Evaluate ValueExpression according the value type.
* @param state Input ValueExpression
* @param valExpr A scope for looking up variables.
* @returns Deep data binding result.
*/
export function evaluateExpression(state: DialogStateManager, valExpr: ValueExpression): any {
return valExpr.expressionText == null ? replaceJsonRecursively(state, valExpr.value) : valExpr.getValue(state);
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,19 @@ class Node {
* SettingsMemoryScope maps "settings" -> dc.context.turnState['settings']
*/
export class SettingsMemoryScope extends MemoryScope {
private static readonly blockingList = [
'MicrosoftAppPassword',
'cosmosDb:authKey',
'blobStorage:connectionString',
'BlobsStorage:connectionString',
'CosmosDbPartitionedStorage:authKey',
'applicationInsights:connectionString',
'applicationInsights:InstrumentationKey',
'runtimeSettings:telemetry:options:connectionString',
'runtimeSettings:telemetry:options:instrumentationKey',
'runtimeSettings:features:blobTranscript:connectionString',
];

/**
* Initializes a new instance of the [SettingsMemoryScope](xref:botbuilder-dialogs.SettingsMemoryScope) class.
*
Expand Down Expand Up @@ -76,7 +89,9 @@ export class SettingsMemoryScope extends MemoryScope {

public async load(dc: DialogContext): Promise<void> {
if (this.initialSettings) {
dc.context.turnState.set(ScopePath.settings, this.initialSettings);
// filter initialSettings
const filteredSettings = SettingsMemoryScope.filterSettings(this.initialSettings);
dc.context.turnState.set(ScopePath.settings, filteredSettings);
}

await super.load(dc);
Expand All @@ -100,7 +115,8 @@ export class SettingsMemoryScope extends MemoryScope {
);
}

return settings;
// filter env configuration settings
return this.filterSettings(settings);
}

/**
Expand Down Expand Up @@ -205,4 +221,31 @@ export class SettingsMemoryScope extends MemoryScope {
return result;
}, {});
}

private static filterSettings(settings: Record<string, unknown>): Record<string, unknown> {
const result = Object.assign({}, settings);
this.blockingList.forEach((path) => this.deletePropertyPath(result, path));
return result;
}

private static deletePropertyPath(obj, path: string): void {
if (!obj || !path?.length) {
return;
}

const pathArray = path.split(':');

for (let i = 0; i < pathArray.length - 1; i++) {
const realKey = Object.keys(obj).find((key) => key.toLowerCase() === pathArray[i].toLowerCase());
obj = obj[realKey];

if (typeof obj === 'undefined') {
return;
}
}

const lastPath = pathArray.pop().toLowerCase();
const lastKey = Object.keys(obj).find((key) => key.toLowerCase() === lastPath);
delete obj[lastKey];
}
}
Loading

0 comments on commit 1b41fd9

Please sign in to comment.