Skip to content

Commit

Permalink
[#6432] TeamsInfo.GetMemberAsync(...) doesn't work properly in Skill …
Browse files Browse the repository at this point in the history
…Bot scenario, it returns http 405 error (#6443)

* Implement GetMemberAsync for skills

* Add unit test
  • Loading branch information
ceciliaavila authored Aug 30, 2022
1 parent 4b182ca commit f732e1b
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 0 deletions.
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
44 changes: 44 additions & 0 deletions tests/Microsoft.Bot.Builder.Tests/Skills/CloudSkillHandlerTests.cs
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

0 comments on commit f732e1b

Please sign in to comment.