Skip to content

Updates to Facebook Adapter #4123

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

Merged
merged 2 commits into from
Jun 19, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook
{
/// <summary>
/// BotAdapter to allow for handling Facebook App payloads and responses via the Facebook API.
/// </summary>
public class FacebookAdapter : BotAdapter, IBotFrameworkHttpAdapter
{
private const string HubModeSubscribe = "subscribe";
Expand All @@ -33,6 +36,7 @@ public class FacebookAdapter : BotAdapter, IBotFrameworkHttpAdapter
/// </summary>
private readonly FacebookClientWrapper _facebookClient;
private readonly ILogger _logger;
private readonly FacebookAdapterOptions _options;

/// <summary>
/// Initializes a new instance of the <see cref="FacebookAdapter"/> class using configuration settings.
Expand All @@ -46,19 +50,21 @@ public class FacebookAdapter : BotAdapter, IBotFrameworkHttpAdapter
/// </remarks>
/// <param name="logger">The logger this adapter should use.</param>
public FacebookAdapter(IConfiguration configuration, ILogger logger = null)
: this(new FacebookClientWrapper(new FacebookAdapterOptions(configuration["FacebookVerifyToken"], configuration["FacebookAppSecret"], configuration["FacebookAccessToken"])), logger)
: this(new FacebookAdapterOptions(configuration["FacebookVerifyToken"], configuration["FacebookAppSecret"], configuration["FacebookAccessToken"]), logger)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="FacebookAdapter"/> class using an existing Facebook client.
/// </summary>
/// <param name="facebookClient">A Facebook API interface.</param>
/// <param name="options">Options for the Facebook Adapter.</param>
/// <param name="logger">The logger this adapter should use.</param>
/// <exception cref="ArgumentNullException"><paramref name="facebookClient"/> is null.</exception>
public FacebookAdapter(FacebookClientWrapper facebookClient, ILogger logger = null)
/// <param name="facebookClient">Client used to interact with the Facebook API.</param>
/// <exception cref="ArgumentNullException"><paramref name="options"/> is null.</exception>
public FacebookAdapter(FacebookAdapterOptions options, ILogger logger = null, FacebookClientWrapper facebookClient = null)
Copy link
Contributor

@mrivera-ms mrivera-ms Jun 18, 2020

Choose a reason for hiding this comment

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

Does this break backward compatibility? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but we are OK as this is a preview package so this is part of the pre GA changes.

{
_facebookClient = facebookClient ?? throw new ArgumentNullException(nameof(facebookClient));
_options = options ?? throw new ArgumentNullException(nameof(options));
_facebookClient = facebookClient ?? new FacebookClientWrapper(options);
_logger = logger ?? NullLogger.Instance;
}

Expand Down Expand Up @@ -219,7 +225,7 @@ public override async Task ContinueConversationAsync(ClaimsIdentity claimsIdenti
/// <exception cref="AuthenticationException">The webhook receives message with invalid signature.</exception>
public async Task ProcessAsync(HttpRequest httpRequest, HttpResponse httpResponse, IBot bot, CancellationToken cancellationToken = default)
{
if (httpRequest.Query["hub.mode"] == HubModeSubscribe)
if (httpRequest.Query["hub.mode"] == HubModeSubscribe && _options.VerifyIncomingRequests)
{
await _facebookClient.VerifyWebhookAsync(httpRequest, httpResponse, cancellationToken).ConfigureAwait(false);
return;
Expand All @@ -232,7 +238,7 @@ public async Task ProcessAsync(HttpRequest httpRequest, HttpResponse httpRespons
stringifiedBody = await sr.ReadToEndAsync().ConfigureAwait(false);
}

if (!_facebookClient.VerifySignature(httpRequest, stringifiedBody))
if (!_facebookClient.VerifySignature(httpRequest, stringifiedBody) && _options.VerifyIncomingRequests)
{
await FacebookHelper.WriteAsync(httpResponse, HttpStatusCode.Unauthorized, string.Empty, Encoding.UTF8, cancellationToken).ConfigureAwait(false);
throw new AuthenticationException("WARNING: Webhook received message with invalid signature. Potential malicious behavior!");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook
{
/// <summary>
/// Options class for Facebook Adapter.
/// </summary>
public class FacebookAdapterOptions
{
/// <summary>
Expand Down Expand Up @@ -56,6 +59,13 @@ public FacebookAdapterOptions(string verifyToken, string appSecret, string acces
/// <value>The access token.</value>
public string FacebookAccessToken { get; set; }

/// <summary>
/// Gets or sets a value indicating whether incoming requests should be verified.
/// Should be set to true in Production but can be set to false for testing or development purposes.
/// </summary>
/// <value>The flag to indicate if incoming requests should be verified.</value>
public bool VerifyIncomingRequests { get; set; } = true;

/// <summary>
/// Throws a <see cref="NotImplementedException"/> exception in all cases.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook
{
/// <summary>
/// Client for interacting with the Facebook API.
/// </summary>
public class FacebookClientWrapper
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents
{
/// <summary>
/// Inner Facebook Attachment payload that can be sent as part of a Facebook Attachment on a Facebook message.
/// </summary>
public class AttachmentPayload
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents
{
/// <summary>
/// Facebook Attachment object that can be sent as part of a Facebook message.
/// </summary>
public class FacebookAttachment
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents
{
/// <summary>
/// Represents a Facebook Bot User object.
/// </summary>
public class FacebookBotUser
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents
{
/// <summary>
/// Represents a Facebook Message Entry.
/// </summary>
public class FacebookEntry
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents
{
/// <summary>
/// Represents a Facebook Post Back.
/// </summary>
public class FacebookPostBack
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents
{
/// <summary>
/// Facebook Quick Reply object that can be sent as part of a Facebook message.
/// </summary>
public class FacebookQuickReply
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents
{
/// <summary>
/// Facebook Recipient object used as part of a Facebook message.
/// </summary>
public class FacebookRecipient
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents
{
/// <summary>
/// Represents the incoming object received from Facebook and processed by the adapter.
/// </summary>
public class FacebookResponseEvent
{
/// <summary>
Expand All @@ -21,6 +24,10 @@ public class FacebookResponseEvent
/// <value>Array containing event data.</value>
public List<FacebookEntry> Entry { get; } = new List<FacebookEntry>();

/// <summary>
/// Gets the flag to determine if the Entry property should be serialized.
/// </summary>
/// <returns>True if Entry count is greater than zero.</returns>
public bool ShouldSerializeEntry()
{
return Entry.Count > 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents
{
/// <summary>
/// Represents the response object received from Facebook API when a message is sent.
/// </summary>
public class FacebookResponseOk
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents.Handover
{
/// <summary>
/// Event object sent to Facebook when requesting to pass thread control to another app.
/// </summary>
public class FacebookPassThreadControl : FacebookThreadControl
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents.Handover
{
/// <summary>
/// Event object sent to Facebook when requesting to take thread control from another app.
/// </summary>
public class FacebookTakeThreadControl : FacebookThreadControl
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents.Handover
{
/// <summary>
/// Base event object for thread control request payloads.
/// </summary>
public class FacebookThreadControl
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents.Handover
{
/// <summary>
/// Constants used as part of the Facebook handover protocol.
/// </summary>
public static class HandoverConstants
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents.Templates
{
/// <summary>
/// Facebook button object.
/// </summary>
public class Button
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook.FacebookEvents.Templates
{
/// <summary>
/// Default action template.
/// </summary>
public class DefaultAction
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook
{
/// <summary>
/// Helper class for converting between Bot Framework objects and Facebook API objects.
/// </summary>
public static class FacebookHelper
{
/// <summary>
Expand Down Expand Up @@ -58,7 +61,7 @@ public static FacebookMessage ActivityToFacebook(Activity activity)
}
}

if (activity.Attachments != null && activity.Attachments.Count > 0)
if (activity.Attachments != null && activity.Attachments.Count > 0 && facebookMessage.Message != null)
{
var payload = JsonConvert.DeserializeObject<AttachmentPayload>(JsonConvert.SerializeObject(
activity.Attachments[0].Content,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

namespace Microsoft.Bot.Builder.Adapters.Facebook
{
/// <summary>
/// Facebook message object used when sending messages via the Facebook API.
/// </summary>
public class Message
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@
<DebugSymbols>true</DebugSymbols>
</PropertyGroup>

<PropertyGroup>
<!-- These documentation warnings excludes should be removed as part of https://github.com/microsoft/botbuilder-dotnet/issues/4052 -->
<NoWarn>$(NoWarn);CS1591</NoWarn>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Bot.Builder.Integration.AspNet.Core" Condition=" '$(IsBuildServer)' == '' " Version="$(LocalPackageVersion)" />
<PackageReference Include="Microsoft.Bot.Builder.Integration.AspNet.Core" Condition=" '$(IsBuildServer)' != '' " Version="$(ReleasePackageVersion)" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
<PackageId>Microsoft.Bot.Builder.Adapters.Slack</PackageId>
<Description>Library for connecting bots with Slack.</Description>
<Summary>This library implements C# classes for the Slack adapter.</Summary>
<!-- The SlackAPI package isn't signed, so supress the warning. There seems to not be a way to supress this for ONLY SlackAPI. -->
<NoWarn>$(NoWarn),CS8002</NoWarn>
</PropertyGroup>

<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
Expand Down
Loading