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

port: update and delete activities from skill handler #2912

Merged
merged 27 commits into from
Oct 21, 2020
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
261b9f9
Properly use RoleTypes
Oct 15, 2020
eced079
Add skill roletype, cast to bandaid compiler bug?
Oct 15, 2020
cff5ef6
Port anonymous skill changes
Oct 15, 2020
dc4f5c0
ChannelServiceHandler test
Oct 15, 2020
d8ceb82
AppCredentials tests
Oct 15, 2020
9683ace
SkillsValidation tests
Oct 15, 2020
373b8e4
Merge branch 'main' into jpg/skills-without-appid-password
Oct 19, 2020
a875d5c
Fix ClaimsIdentity constructor
Oct 19, 2020
250e226
Merge branch 'main' into jpg/skills-without-appid-password
Oct 20, 2020
e92ced2
Update/Delete activities from skills
Oct 19, 2020
9bf35dd
Merge branch 'main' into jpg/skills-without-appid-password
joshgummersall Oct 20, 2020
4dec9a9
Merge branch 'main' into jpg/skills-without-appid-password
joshgummersall Oct 20, 2020
e06b3f0
Merge branch 'main' into jpg/skills-without-appid-password
Oct 20, 2020
142740a
Merge branch 'jpg/skills-without-appid-password' into jpg/skills-upda…
Oct 20, 2020
af560a1
Add comment describing `isAuthenticated` override
Oct 20, 2020
234d6a0
Merge branch 'jpg/skills-without-appid-password' into jpg/skills-upda…
Oct 20, 2020
8516f3d
Fake response with activityId
Oct 20, 2020
b0b7e2d
Stgum/q fixes (#2932)
stevengum Oct 21, 2020
9b354e6
Merge branch 'main' into jpg/skills-without-appid-password
Oct 21, 2020
c487802
Fix skill validation tests
Oct 21, 2020
7bb5958
Fix assert message
Oct 21, 2020
e3ad74d
Fix test label
Oct 21, 2020
6676576
Merge branch 'jpg/skills-without-appid-password' into jpg/skills-upda…
Oct 21, 2020
4e3976e
Merge branch 'main' into jpg/skills-update-delete-activities
Oct 21, 2020
53f5cd9
Cleanup
Oct 21, 2020
fecdd05
Merge branch 'main' into jpg/skills-update-delete-activities
Oct 21, 2020
4a2795d
Merge branch 'main' into jpg/skills-update-delete-activities
joshgummersall Oct 21, 2020
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
3 changes: 2 additions & 1 deletion libraries/botbuilder-core/src/transcriptLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
ActivityTypes,
ConversationReference,
ResourceResponse,
RoleTypes,
} from 'botframework-schema';
import { Middleware } from './middlewareSet';
import { TurnContext } from './turnContext';
Expand Down Expand Up @@ -45,7 +46,7 @@ export class TranscriptLoggerMiddleware implements Middleware {
// log incoming activity at beginning of turn
if (context.activity) {
if (!context.activity.from.role) {
context.activity.from.role = 'user';
context.activity.from.role = RoleTypes.User;
}

this.logActivity(transcript, this.cloneActivity(context.activity));
Expand Down
2 changes: 1 addition & 1 deletion libraries/botbuilder/src/botFrameworkAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1702,7 +1702,7 @@ export class BotFrameworkAdapter
* Override this in a derived class to modify how the adapter creates a turn context.
*/
protected createContext(request: Partial<Activity>): TurnContext {
return new TurnContext(this as any, request);
return new TurnContext(this, request);
}

/**
Expand Down
52 changes: 12 additions & 40 deletions libraries/botbuilder/src/botFrameworkHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import axios from 'axios';
import { Activity, BotFrameworkClient, InvokeResponse } from 'botbuilder-core';
import { Activity, BotFrameworkClient, ChannelAccount, InvokeResponse, RoleTypes } from 'botbuilder-core';
import {
AuthenticationConstants,
AppCredentials,
Expand Down Expand Up @@ -58,33 +58,6 @@ export class BotFrameworkHttpClient implements BotFrameworkClient {
* @param conversationId A conversation ID to use for the conversation with the skill.
* @param activity Activity to forward.
*/
public async postActivity<T>(
fromBotId: string,
toBotId: string,
toUrl: string,
serviceUrl: string,
conversationId: string,
activity: Activity
): Promise<InvokeResponse<T>>;
public async postActivity(
fromBotId: string,
toBotId: string,
toUrl: string,
serviceUrl: string,
conversationId: string,
activity: Activity
): Promise<InvokeResponse>;
/**
* Forwards an activity to another bot.
* @template T The type of body in the [InvokeResponse](xref:botbuilder-core.InvokeResponse).
* @param fromBotId The MicrosoftAppId of the bot sending the activity.
* @param toBotId The MicrosoftAppId of the bot receiving the activity.
* @param toUrl The URL of the bot receiving the activity.
* @param serviceUrl The callback Url for the skill host.
* @param conversationId A conversation ID to use for the conversation with the skill.
* @param activity [Activity](xref:botframework-schema.Activity) to forward.
* @returns A `Promise` representing the [InvokeResponse](xref:botbuilder-core.InvokeResponse) for the operation.
*/
public async postActivity<T = any>(
fromBotId: string,
toBotId: string,
Expand Down Expand Up @@ -117,6 +90,7 @@ export class BotFrameworkHttpClient implements BotFrameworkClient {
const originalServiceUrl = activity.serviceUrl;
const originalRelatesTo = activity.relatesTo;
const originalRecipient = activity.recipient;

try {
activity.relatesTo = {
serviceUrl: activity.serviceUrl,
Expand All @@ -139,10 +113,11 @@ export class BotFrameworkHttpClient implements BotFrameworkClient {

// Fixes: https://github.com/microsoft/botframework-sdk/issues/5785
if (!activity.recipient) {
activity.recipient = {} as any;
activity.recipient = {} as ChannelAccount;
}
activity.recipient.role = RoleTypes.Skill;

const config = {
const config: { headers: Record<string, string>; validateStatus: () => boolean } = {
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
Expand All @@ -152,13 +127,11 @@ export class BotFrameworkHttpClient implements BotFrameworkClient {
};

if (token) {
config.headers['Authorization'] = `Bearer ${token}`;
config.headers.Authorization = `Bearer ${token}`;
}

const response = await axios.post(toUrl, activity, config);
const invokeResponse: InvokeResponse<T> = { status: response.status, body: response.data };

return invokeResponse;
const response = await axios.post<T>(toUrl, activity, config);
return { status: response.status, body: response.data };
} finally {
// Restore activity properties.
activity.conversation.id = originalConversationId;
Expand All @@ -177,14 +150,13 @@ export class BotFrameworkHttpClient implements BotFrameworkClient {
*/
protected async buildCredentials(appId: string, oAuthScope?: string): Promise<AppCredentials> {
const appPassword = await this.credentialProvider.getAppPassword(appId);
let appCredentials;
if (JwtTokenValidation.isGovernment(this.channelService)) {
appCredentials = new MicrosoftAppCredentials(appId, appPassword, undefined, oAuthScope);
const appCredentials = new MicrosoftAppCredentials(appId, appPassword, undefined, oAuthScope);
appCredentials.oAuthEndpoint = GovernmentConstants.ToChannelFromBotLoginUrl;
return appCredentials;
} else {
appCredentials = new MicrosoftAppCredentials(appId, appPassword, undefined, oAuthScope);
return new MicrosoftAppCredentials(appId, appPassword, undefined, oAuthScope);
}
return appCredentials;
}

/**
Expand All @@ -206,7 +178,7 @@ export class BotFrameworkHttpClient implements BotFrameworkClient {
}

// Credentials not found in cache, build them
appCredentials = (await this.buildCredentials(appId, oAuthScope)) as MicrosoftAppCredentials;
appCredentials = await this.buildCredentials(appId, oAuthScope);

// Cache the credentials for later use
BotFrameworkHttpClient.appCredentialMapCache.set(cacheKey, appCredentials);
Expand Down
38 changes: 19 additions & 19 deletions libraries/botbuilder/src/channelServiceHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
ClaimsIdentity,
ICredentialProvider,
JwtTokenValidation,
SkillValidation,
} from 'botframework-connector';
import { StatusCodeError } from './statusCodeError';

Expand Down Expand Up @@ -597,27 +598,26 @@ export class ChannelServiceHandler {
* @private
*/
private async authenticate(authHeader: string): Promise<ClaimsIdentity> {
try {
if (!authHeader) {
const isAuthDisable = this.credentialProvider.isAuthenticationDisabled();
if (isAuthDisable) {
// In the scenario where Auth is disabled, we still want to have the
// IsAuthenticated flag set in the ClaimsIdentity. To do this requires
// adding in an empty claim.
return new ClaimsIdentity([], false);
}
if (!authHeader) {
const isAuthDisabled = await this.credentialProvider.isAuthenticationDisabled();
if (!isAuthDisabled) {
throw new StatusCodeError(StatusCodes.UNAUTHORIZED);
}

return await JwtTokenValidation.validateAuthHeader(
authHeader,
this.credentialProvider,
this.channelService,
'unknown',
undefined,
this.authConfig
);
} catch (err) {
throw new StatusCodeError(StatusCodes.UNAUTHORIZED);
// In the scenario where Auth is disabled, we still want to have the
// IsAuthenticated flag set in the ClaimsIdentity. To do this requires
// adding in an empty claim.
// Since ChannelServiceHandler calls are always a skill callback call, we set the skill claim too.
return SkillValidation.createAnonymousSkillClaim();
}

return JwtTokenValidation.validateAuthHeader(
authHeader,
this.credentialProvider,
this.channelService,
'unknown',
undefined,
this.authConfig
);
}
}
22 changes: 11 additions & 11 deletions libraries/botbuilder/src/eventFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ export class EventFactory {
* @param handoffContext Agent hub-specific context.
* @param transcript Transcript of the conversation.
*/
public static createHandoffInitiation(
public static createHandoffInitiation<T = unknown>(
context: TurnContext,
handoffContext: any,
handoffContext: T,
transcript?: Transcript
): Activity {
if (!context) {
Expand All @@ -30,7 +30,7 @@ export class EventFactory {
const handoffEvent = this.createHandoffEvent(
HandoffEventNames.InitiateHandoff,
handoffContext,
context.activity.conversation
context.activity.conversation as ConversationAccount // TODO(joshgummersall) why is this necessary?
);

handoffEvent.from = context.activity.from;
Expand Down Expand Up @@ -66,18 +66,18 @@ export class EventFactory {
throw new TypeError('EventFactory.createHandoffStatus(): missing state.');
}

const value: any = { state, message };

const handoffEvent = this.createHandoffEvent(HandoffEventNames.HandoffStatus, value, conversation);

return handoffEvent;
return this.createHandoffEvent(HandoffEventNames.HandoffStatus, { state, message }, conversation);
}

/**
* @private
*/
private static createHandoffEvent(name: string, value: any, conversation: ConversationAccount): Activity {
const handoffEvent: Activity = {} as any;
private static createHandoffEvent<T = unknown>(
name: string,
value: T,
conversation: ConversationAccount
): Activity {
const handoffEvent: Partial<Activity> = {};

handoffEvent.name = name;
handoffEvent.value = value;
Expand All @@ -90,7 +90,7 @@ export class EventFactory {
handoffEvent.attachments = [];
handoffEvent.entities = [];

return handoffEvent;
return handoffEvent as Activity;
}
}

Expand Down
Loading