botframework-sdk icon indicating copy to clipboard operation
botframework-sdk copied to clipboard

[SSO] Support SSO on host bots

Open lzc850612 opened this issue 5 years ago • 7 comments
trafficstars

Issue

To support scenario of host bot (VA) support SSO for skills under it, host bot should handle OAuth card and do the token exchange and send the exchangeable token back to skill. This should be supported from the SDK so bot developer shouldn't have to do anything additional to support this scenario.

Sample code: within SkillHandler:

protected override async Task<ResourceResponse> **OnSendToConversationAsync**(ClaimsIdentity claimsIdentity, string conversationId, Activity activity, CancellationToken cancellationToken = default(CancellationToken))
        {
            if (await **InterceptOAuthCards**(claimsIdentity, conversationId, activity).ConfigureAwait(false))
            {
                return new ResourceResponse(Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture));
            }

            return await base.OnSendToConversationAsync(claimsIdentity, conversationId, activity, cancellationToken).ConfigureAwait(false);
        }

private async Task<bool> **InterceptOAuthCards**(ClaimsIdentity claimsIdentity, string conversationId, Activity activity)
        {
            if (activity.Attachments != null)
            {
                foreach (var attachment in activity.Attachments.Where(a => a?.ContentType == OAuthCard.ContentType))
                {
                    var targetSkill = GetCallingSkill(claimsIdentity);

                    if (targetSkill != null)
                    { 
                        var oauthCard = ((JObject)attachment.Content).ToObject<OAuthCard>();

                        if (oauthCard.TokenExchangeResource != null && _tokenExchangeConfig.ProviderId == oauthCard.TokenExchangeResource.ProviderId)
                        {
                            using (var context = new TurnContext(_adapter, activity))
                            {
                                context.TurnState.Add<IIdentity>("BotIdentity", claimsIdentity);

                                // AAD token exchange
                                var result = await _tokenExchangeProvider.ExchangeTokenAsync(
                                    context,
                                    _tokenExchangeConfig.ConnectionName,
                                    new TokenExchangeRequest() { Uri = oauthCard.TokenExchangeResource.Uri }).ConfigureAwait(false);

                                if (!string.IsNullOrEmpty(result.Token))
                                {
                                    // Send an Invoke back to the Skill
                                    return await SendTokenExchangeInvokeToSkill(activity, oauthCard.TokenExchangeResource.Id, result.Token, oauthCard.ConnectionName, targetSkill, default(CancellationToken)).ConfigureAwait(false);
                                }

                                return false;
                            }
                        }
                    }
                }
            }
            return false;
        }

Proposed change

Describe the proposed solution

Component Impact

Describe which components need to be updated

Customer Impact

Describe the impact on customers

Tracking Status

Dotnet SDK

  • [ ] PR
  • [ ] Merged

Javascript SDK

  • [ ] PR
  • [ ] Merged

Java SDK

  • [ ] PR
  • [ ] Merged

Python SDK

  • [ ] PR
  • [ ] Merged

Emulator

  • [ ] PR
  • [ ] Merged

Samples

  • [ ] PR
  • [ ] Merged

Docs

  • [ ] PR
  • [ ] Merged

Tools

  • [ ] PR
  • [ ] Merged

[dcr]

lzc850612 avatar Mar 03 '20 19:03 lzc850612

This is to support the reverse SSO scenario, @carlosscastro and @lzc850612 to pair program on this early R10.

carlosscastro avatar Apr 20 '20 18:04 carlosscastro

@darrenj and I discussed that given the low risk and the fact that there is a workaround (and R10 being pretty booked), we are pushing this to R11. @lzc850612 if you are passionate about this, you could bring it back in and implement, but the thing is not only implementation: this needs to be implemented in js, c# and python and verified in all 3 languages.

carlosscastro avatar Jul 13 '20 15:07 carlosscastro

@lzc850612 is booked and cannot implement this in R11 (and I'll be on leave soon). Moving to R12 and we'll plan ahead to make sure we have owners for this then.

carlosscastro avatar Oct 05 '20 16:10 carlosscastro

@carlosscastro I can chip in for this work in R12 once I get up to speed. I'd like to ensure that this feature lands.

stevengum avatar Oct 05 '20 18:10 stevengum

Moving to re-assess priority in R13.

carlosscastro avatar Feb 03 '21 18:02 carlosscastro

In R13 triage this one did not make the cut line. We'll reconsider in R14. There is definitely value in not having users add the code above. However we are also making progress in adaptive runtime and built in SSO, which may change the course of this item.

carlosscastro avatar Mar 23 '21 14:03 carlosscastro

Now, the SSO docs provide guidance on how to use SSO without writing code using the adaptive runtime, so that is a big step forward iin this direction. In R15 we'll hopefully finally be confident to bake that code into the SDK. @EricDahlvang FYI

carlosscastro avatar Jun 14 '21 15:06 carlosscastro