Skip to content

Removed call to users array#10

Closed
GeekTrainer wants to merge 3 commits intoCatalystCode:masterfrom
GeekTrainer:master
Closed

Removed call to users array#10
GeekTrainer wants to merge 3 commits intoCatalystCode:masterfrom
GeekTrainer:master

Conversation

@GeekTrainer
Copy link

I removed the users array. It was declared at the class level, meaning that it would be tied to one server. In addition, it's not needed as we're only using Passport to simplify the authentication process.

@GeekTrainer
Copy link
Author

Made updates to how the resurrect code works. Rather than parsing items out, I simply serialize the address and pass that around as needed.

"botbuilder": "^3.2.3",
"passport": "^0.3.2",
"passport-azure-ad": "2.0.0",
"querystring": "^0.2.0",
Copy link
Member

Choose a reason for hiding this comment

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

kind of confused; are you using the builtin querystring module in your code? (https://nodejs.org/api/querystring.html)

Copy link
Author

Choose a reason for hiding this comment

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

I don't know why I did that. :-) That can be removed.

useAuth: true };

var continueMsg = new builder.Message().address(address).text("signin?authcode=" + authcode + "&code=" + code + "&name=" + req.user.displayName + "&email=" + req.user.email);
bot.receive(continueMsg.toMessage());
Copy link
Collaborator

Choose a reason for hiding this comment

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

In this PR, continueMsg is not set.

} else {
session.replaceDialog('signinPrompt', { invalid: true });
}
session.userData.loginData = JSON.parse(results.response);
Copy link
Collaborator

Choose a reason for hiding this comment

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

The response here is not a json object.

@GeekTrainer
Copy link
Author

Closing this because it doesn't work. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants