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

fix: fix bot-sso race condition issue #929

Merged
merged 12 commits into from
Jul 12, 2023
Merged
10 changes: 9 additions & 1 deletion bot-sso/commands/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BotCommand } from "../helpers/botCommand";
import { BotCommand, SSOCommand } from "../helpers/botCommand";
import { LearnCommand } from "./learn";
import { ShowUserProfile } from "./showUserProfile";
import { WelcomeCommand } from "./welcome";
eriolchan marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -8,3 +8,11 @@ export const commands: BotCommand[] = [
new ShowUserProfile(),
new WelcomeCommand(),
];

export const SSOCommands: SSOCommand[] = [
new ShowUserProfile(),
];

export const SSOCommandMap: Map<string, any> = new Map(
SSOCommands.map((command) => [command.matchPatterns, command.operationWithSSOToken])
);
2 changes: 1 addition & 1 deletion bot-sso/commands/learn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const rawLearnCard = require("../adaptiveCards/learn.json");
export class LearnCommand extends BotCommand {
constructor() {
super();
this.matchPatterns = [/^\s*learn\s*/];
this.commandMessage = 'learn';
}

validateParameters(parameters: any): boolean {
Expand Down
2 changes: 1 addition & 1 deletion bot-sso/commands/showUserProfile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import oboAuthConfig from "../authConfig";
export class ShowUserProfile extends SSOCommand {
constructor() {
super();
this.matchPatterns = [/^\s*show\s*/];
this.commandMessage = 'show';
this.operationWithSSOToken = this.showUserInfo;
}

Expand Down
2 changes: 1 addition & 1 deletion bot-sso/commands/welcome.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const rawWelcomeCard = require("../adaptiveCards/welcome.json");
export class WelcomeCommand extends BotCommand {
constructor() {
super();
this.matchPatterns = [/^\s*welcome\s*/];
this.commandMessage = 'welcome';
}

async run(parameters: any): Promise<any> {
Expand Down
24 changes: 4 additions & 20 deletions bot-sso/helpers/botCommand.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,16 @@
export type PredicateFunc<T> = (v: T) => boolean;
export type MatchTerm = string | RegExp | PredicateFunc<string>;

export abstract class BotCommand {
public matchPatterns: MatchTerm[];
public commandMessage: string;

abstract run(parameters: any): any;

public validateParameters(parameters: any): boolean {
return true;
}

public expressionMatchesText(userInput: string): RegExpExecArray | boolean {
let matchResult: RegExpExecArray | boolean;
for (const pattern of this.matchPatterns) {
if (typeof pattern == "string") {
matchResult = new RegExp(pattern).exec(userInput);
} else if (pattern instanceof RegExp) {
matchResult = pattern.exec(userInput);
} else {
matchResult = pattern(userInput);
}
if (matchResult) {
return matchResult;
}
}
return false;
public expressionMatchesText(userInput: string): boolean {
return userInput === this.commandMessage;
}

}

export class SSOCommand extends BotCommand {
Expand All @@ -50,7 +35,6 @@ export class SSOCommand extends BotCommand {
async run(parameters: any): Promise<any> {
this.validateParameters(parameters);
const ssoDialog = parameters.ssoDialog;
ssoDialog.setSSOOperation(this.operationWithSSOToken);
await ssoDialog.run(parameters.context, parameters.dialogState);
}
}
48 changes: 24 additions & 24 deletions bot-sso/helpers/ssoDialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
ComponentDialog,
} from "botbuilder-dialogs";
import {
Activity,
ActivityTypes,
Storage,
tokenExchangeOperationName,
Expand All @@ -15,6 +16,7 @@ import { TeamsBotSsoPrompt } from "@microsoft/teamsfx";
import "isomorphic-fetch";
import oboAuthConfig from "../authConfig";
import config from "../config";
import { SSOCommandMap } from "../commands";

const DIALOG_NAME = "SSODialog";
const MAIN_WATERFALL_DIALOG = "MainWaterfallDialog";
Expand All @@ -24,17 +26,13 @@ export class SSODialog extends ComponentDialog {
private requiredScopes: string[] = ["User.Read"]; // hard code the scopes for demo purpose only
private dedupStorage: Storage;
private dedupStorageKeys: string[];
private operationWithSSO: (
arg0: any,
ssoToken: string
) => Promise<any> | undefined;

// Developer controlls the lifecycle of credential provider, as well as the cache in it.
// In this sample the provider is shared in all conversations
constructor(dedupStorage: Storage) {
super(DIALOG_NAME);

const initialLoginEndpoint =`https://${config.botDomain}/auth-start.html` ;
const initialLoginEndpoint = `https://${config.botDomain}/auth-start.html`;

const dialog = new TeamsBotSsoPrompt(
oboAuthConfig,
Expand All @@ -60,16 +58,6 @@ export class SSODialog extends ComponentDialog {
this.dedupStorageKeys = [];
}

setSSOOperation(
handler: (arg0: any, arg1: string) => Promise<any> | undefined
) {
this.operationWithSSO = handler;
}

resetSSOOperation() {
this.operationWithSSO = undefined;
}

/**
* The run method handles the incoming activity (in the form of a DialogContext) and passes it through the dialog system.
* If no dialog is active, it will start the default dialog.
Expand All @@ -87,6 +75,8 @@ export class SSODialog extends ComponentDialog {
}

async ssoStep(stepContext: any) {
const turnContext = stepContext.context as TurnContext;
stepContext.options.commandMessage = this.getActivityText(turnContext.activity);
return await stepContext.beginDialog(TEAMS_SSO_PROMPT_ID);
}

Expand All @@ -102,15 +92,14 @@ export class SSODialog extends ComponentDialog {
async executeOperationWithSSO(stepContext: any) {
const tokenResponse = stepContext.result;
if (!tokenResponse || !tokenResponse.ssoToken) {
await stepContext.context.sendActivity(
"There is an issue while trying to sign you in and retrieve your profile photo, please type \"show\" command to login and consent permissions again."
);
} else {
// Once got ssoToken, run operation that depends on ssoToken
if (this.operationWithSSO) {
await this.operationWithSSO(stepContext.context, tokenResponse.ssoToken);
}
throw new Error("There is an issue while trying to sign you in and run your command. Please try again.");
}
// Once got ssoToken, run operation that depends on ssoToken
const operationWithSSO = SSOCommandMap.get(stepContext.options.commandMessage);
if (!operationWithSSO) {
throw new Error("Can not get sso operation. Please try again.");
}
await operationWithSSO(stepContext.context, tokenResponse.ssoToken);
return await stepContext.endDialog();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like Line 114 is duplicated?

}

Expand All @@ -123,7 +112,6 @@ export class SSODialog extends ComponentDialog {
this.dedupStorageKeys = this.dedupStorageKeys.filter(
(key) => key.indexOf(conversationId) < 0
);
this.resetSSOOperation();
}

// If a user is signed into multiple Teams clients, the Bot might receive a "signin/tokenExchange" from each client.
Expand Down Expand Up @@ -173,4 +161,16 @@ export class SSODialog extends ComponentDialog {
}
return `${channelId}/${conversationId}/${value.id}`;
}

private getActivityText(activity: Activity): string {
let text = activity.text;
const removedMentionText = TurnContext.removeRecipientMention(activity);
if (removedMentionText) {
text = removedMentionText
.toLowerCase()
.replace(/\n|\r\n/g, "")
.trim();
}
return text;
}
}
2 changes: 1 addition & 1 deletion bot-sso/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,4 @@
"nodemon": "^2.0.7",
"shx": "^0.3.3"
}
}
}