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

[#6432] TeamsInfo.GetMemberAsync(...) doesn't work properly in Skill Bot scenario, it returns http 405 error #6443

Merged
merged 2 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 33 additions & 0 deletions libraries/Microsoft.Bot.Builder/ChannelServiceHandlerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,20 @@ public async Task<IList<ChannelAccount>> HandleGetConversationMembersAsync(strin
return await OnGetConversationMembersAsync(claimsIdentity, conversationId, cancellationToken).ConfigureAwait(false);
}

/// <summary>
/// Gets the account of a single conversation member.
/// </summary>
/// <param name="authHeader">The authentication header.</param>
/// <param name="userId">The user id.</param>
/// <param name="conversationId">The conversation Id.</param>
/// <param name="cancellationToken">A cancellation token that can be used by other objects or threads to receive notice of cancellation.</param>
/// <returns>A <see cref="Task{TResult}"/> representing the result of the asynchronous operation.</returns>
public async Task<ChannelAccount> HandleGetConversationMemberAsync(string authHeader, string userId, string conversationId, CancellationToken cancellationToken = default)
{
var claimsIdentity = await AuthenticateAsync(authHeader, cancellationToken).ConfigureAwait(false);
return await OnGetConversationMemberAsync(claimsIdentity, userId, conversationId, cancellationToken).ConfigureAwait(false);
}

/// <summary>
/// Enumerates the members of a conversation one page at a time.
/// </summary>
Expand Down Expand Up @@ -392,6 +406,25 @@ protected virtual Task<IList<ChannelAccount>> OnGetConversationMembersAsync(Clai
throw new NotImplementedException();
}

/// <summary>
/// GetConversationMember() API for Skill.
/// </summary>
/// <remarks>
/// Override this method to get the account of a single conversation member.
///
/// This REST API takes a ConversationId and UserId and returns the ChannelAccount
/// objects representing the member of the conversation.
/// </remarks>
/// <param name="claimsIdentity">claimsIdentity for the bot, should have AudienceClaim, AppIdClaim and ServiceUrlClaim.</param>
/// <param name="userId">User ID.</param>
/// <param name='conversationId'>Conversation ID.</param>
/// <param name='cancellationToken'>The cancellation token.</param>
/// <returns>task for a response.</returns>
protected virtual Task<ChannelAccount> OnGetConversationMemberAsync(ClaimsIdentity claimsIdentity, string userId, string conversationId, CancellationToken cancellationToken = default)
{
throw new NotImplementedException();
}

/// <summary>
/// GetConversationPagedMembers() API for Skill.
/// </summary>
Expand Down
6 changes: 6 additions & 0 deletions libraries/Microsoft.Bot.Builder/Skills/CloudSkillHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,5 +132,11 @@ protected override async Task<ResourceResponse> OnUpdateActivityAsync(ClaimsIden
{
return await _inner.OnUpdateActivityAsync(claimsIdentity, conversationId, activityId, activity, cancellationToken).ConfigureAwait(false);
}

/// <inheritdoc/>
protected override async Task<ChannelAccount> OnGetConversationMemberAsync(ClaimsIdentity claimsIdentity, string userId, string conversationId, CancellationToken cancellationToken = default)
{
return await _inner.OnGetMemberAsync(claimsIdentity, userId, conversationId, cancellationToken).ConfigureAwait(false);
}
}
}
18 changes: 18 additions & 0 deletions libraries/Microsoft.Bot.Builder/Skills/SkillHandlerImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Security.Claims;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Bot.Connector;
using Microsoft.Bot.Connector.Authentication;
using Microsoft.Bot.Schema;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -91,6 +92,23 @@ internal async Task<ResourceResponse> OnUpdateActivityAsync(ClaimsIdentity claim
return resourceResponse ?? new ResourceResponse(Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture));
}

internal async Task<ChannelAccount> OnGetMemberAsync(ClaimsIdentity claimsIdentity, string userId, string conversationId, CancellationToken cancellationToken = default)
{
var skillConversationReference = await GetSkillConversationReferenceAsync(conversationId, cancellationToken).ConfigureAwait(false);
ChannelAccount member = null;

var callback = new BotCallbackHandler(async (turnContext, ct) =>
{
var client = turnContext.TurnState.Get<IConnectorClient>();
var conversationId = turnContext.Activity.Conversation.Id;
member = await ((Conversations)client.Conversations).GetConversationMemberAsync(userId, conversationId, cancellationToken).ConfigureAwait(false);
});

await _adapter.ContinueConversationAsync(claimsIdentity, skillConversationReference.ConversationReference, skillConversationReference.OAuthScope, callback, cancellationToken).ConfigureAwait(false);

return member;
}

private static void ApplySkillActivityToTurnContext(ITurnContext turnContext, Activity activity)
{
// adapter.ContinueConversation() sends an event activity with ContinueConversation in the name.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,19 @@ public virtual async Task<IActionResult> GetConversationMembersAsync(string conv
return new JsonResult(result, HttpHelper.BotMessageSerializerSettings);
}

/// <summary>
/// GetConversationMember.
/// </summary>
/// <param name="userId">User ID.</param>
/// <param name="conversationId">Conversation ID.</param>
/// <returns>The ChannelAccount of the conversation member.</returns>
[HttpGet("v3/conversations/{conversationId}/members/{userId}")]
public virtual async Task<IActionResult> GetConversationMemberAsync(string userId, string conversationId)
{
var result = await _handler.HandleGetConversationMemberAsync(HttpContext.Request.Headers["Authorization"], userId, conversationId).ConfigureAwait(false);
return new JsonResult(result, HttpHelper.BotMessageSerializerSettings);
}

/// <summary>
/// GetConversationPagedMembers.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@

using System;
using System.Collections.Concurrent;
using System.Net.Http;
using System.Security.Claims;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Bot.Builder.Skills;
using Microsoft.Bot.Connector;
using Microsoft.Bot.Connector.Authentication;
using Microsoft.Bot.Schema;
using Moq;
Expand All @@ -19,6 +21,11 @@ public class CloudSkillHandlerTests
{
private static readonly string TestSkillId = Guid.NewGuid().ToString("N");
private static readonly string TestAuthHeader = string.Empty; // Empty since claims extraction is being mocked
private static readonly ChannelAccount TestMember = new ChannelAccount()
{
Id = "userId",
Name = "userName"
};

[Theory]
[InlineData(ActivityTypes.Message, null)]
Expand Down Expand Up @@ -155,6 +162,23 @@ public async void TestUpdateActivityAsync()
Assert.Equal(activity.Text, mockObjects.UpdateActivity.Text);
}

[Fact]
public async void TestGetConversationMemberAsync()
{
// Arrange
var mockObjects = new CloudSkillHandlerTestMocks();
var activity = new Activity(ActivityTypes.Message) { Text = $"Get Member." };
var conversationId = await mockObjects.CreateAndApplyConversationIdAsync(activity);

// Act
var sut = new CloudSkillHandler(mockObjects.Adapter.Object, mockObjects.Bot.Object, mockObjects.ConversationIdFactory, mockObjects.Auth.Object);
var member = await sut.HandleGetConversationMemberAsync(TestAuthHeader, TestMember.Id, conversationId);

// Assert
Assert.Equal(TestMember.Id, member.Id);
Assert.Equal(TestMember.Name, member.Name);
}

/// <summary>
/// Helper class with mocks for adapter, bot and auth needed to instantiate CloudSkillHandler and run tests.
/// This class also captures the turnContext and activities sent back to the bot and the channel so we can run asserts on them.
Expand All @@ -172,6 +196,7 @@ public CloudSkillHandlerTestMocks()
Auth = CreateMockBotFrameworkAuthentication();
Bot = CreateMockBot();
ConversationIdFactory = new TestSkillConversationIdFactory();
Client = CreateMockConnectorClient();
}

public SkillConversationIdFactoryBase ConversationIdFactory { get; }
Expand All @@ -182,6 +207,8 @@ public CloudSkillHandlerTestMocks()

public Mock<IBot> Bot { get; }

public IConnectorClient Client { get; }

// Gets the TurnContext created to call the bot.
public TurnContext TurnContext { get; private set; }

Expand Down Expand Up @@ -242,6 +269,8 @@ private Mock<BotAdapter> CreateMockAdapter()
{
// Create and capture the TurnContext so we can run assertions on it.
TurnContext = new TurnContext(adapter.Object, conv.GetContinuationActivity());
TurnContext.TurnState.Add(Client);

await botCallbackHandler(TurnContext, cancel);
});

Expand Down Expand Up @@ -307,6 +336,21 @@ private Mock<BotFrameworkAuthentication> CreateMockBotFrameworkAuthentication()
});
return auth;
}

private IConnectorClient CreateMockConnectorClient()
{
var httpResponse = new HttpResponseMessage(System.Net.HttpStatusCode.OK)
{
Content = new StringContent(JsonConvert.SerializeObject(TestMember))
};

var httpClient = new Mock<HttpClient>();
httpClient.Setup(x => x.SendAsync(It.IsAny<HttpRequestMessage>(), It.IsAny<CancellationToken>())).ReturnsAsync(httpResponse);

var client = new ConnectorClient(MicrosoftAppCredentials.Empty, httpClient.Object, false);

return client;
}
}

private class TestSkillConversationIdFactory : SkillConversationIdFactoryBase
Expand Down