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

[OAuth]do not set signInLink to empty when OAuthAppCredentials is set #3564

Merged
merged 1 commit into from
Mar 15, 2020

Conversation

lzc850612
Copy link
Contributor

When OAuthAppCredentials is passed in as part OAuthPromptSettings, do not set SignInLink to be empty as Emulator/other first party channels will ignore this and get a SignInLink without the additional OAuthAppCredentials setting.

if (turnContext.Activity.IsFromStreamingConnection() ||
(turnContext.TurnState.Get<ClaimsIdentity>(BotAdapter.BotIdentityKey) is ClaimsIdentity botIdentity && SkillValidation.IsSkillClaim(botIdentity.Claims)))
(turnContext.TurnState.Get<ClaimsIdentity>(BotAdapter.BotIdentityKey) is ClaimsIdentity botIdentity && SkillValidation.IsSkillClaim(botIdentity.Claims)) ||
Copy link
Member

Choose a reason for hiding this comment

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

This 'if' should become a method given how complex it is, but I'm OK doing that for R9

Copy link
Contributor

@tomlm tomlm left a comment

Choose a reason for hiding this comment

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

:shipit:

@carlosscastro carlosscastro merged commit 9e6d2b6 into master Mar 15, 2020
@carlosscastro carlosscastro deleted the tedlee/fix-oauth-credentials branch March 15, 2020 23:57
carlosscastro added a commit that referenced this pull request Mar 15, 2020
[OAuth]do not set signInLink to empty when OAuthAppCredentials is set
carlosscastro added a commit that referenced this pull request Mar 16, 2020
[OAuth]do not set signInLink to empty when OAuthAppCredentials is set
cleemullins pushed a commit that referenced this pull request Mar 16, 2020
…3571)

[OAuth]do not set signInLink to empty when OAuthAppCredentials is set
garypretty added a commit that referenced this pull request Apr 16, 2020
* Fix AdaptiveTesting to have correct preview labels

* Proactive auth: trust incoming token url conditionally (#3533)

* Add unit test for CancelAllDialogs and fix eventName and eventValue could be null (#3536)

* fix lack of a generic typed method for posting activities to skills (#3539) (#3542)

* fix lack of a generic typed method for posting activities to skills

* remove duplicated code

Co-authored-by: Swagat Mishra <swagatm@microsoft.com>

Co-authored-by: swagat mishra <swagatmishra2007@gmail.com>
Co-authored-by: Swagat Mishra <swagatm@microsoft.com>

* EoC should only be handled when coming from parent in RunAsync (#3540) (#3543)

* Added check to only cancel dialogs in RunAsync when EoC comes from a parent bot (root or skill).
* Applied URI prefix to caller ID as per OBI spec
https://github.com/microsoft/botframework-obi/blob/master/protocols/botframework-activity/botframework-activity.md#bot-calling-skill

(cherry picked from commit 3730963)

* Merge pull request #3564 from microsoft/tedlee/fix-oauth-credentials (#3571)

[OAuth]do not set signInLink to empty when OAuthAppCredentials is set

* Address QnAMaker / recognizer telemetry across dialogs / adaptive

* Fixes following merge from master

* Added comments to the TrackRecognizerResult method on the Recognizer class.

* More comments.

Co-authored-by: Tom Laird-McConnell <tomlm@microsoft.com>
Co-authored-by: Carlos Castro <carlosscastro@users.noreply.github.com>
Co-authored-by: Steven Gum <14935595+stevengum@users.noreply.github.com>
Co-authored-by: swagat mishra <swagatmishra2007@gmail.com>
Co-authored-by: Swagat Mishra <swagatm@microsoft.com>
Co-authored-by: Gabo Gilabert <gabog@users.noreply.github.com>
chrimc62 pushed a commit that referenced this pull request Jul 15, 2020
[OAuth]do not set signInLink to empty when OAuthAppCredentials is set
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.

None yet

3 participants