Umbraco.Forms.Issues icon indicating copy to clipboard operation
Umbraco.Forms.Issues copied to clipboard

Prevent users defining an email workflow that allows the form's sender email to be defined as that entered by the user

Open RachBreeze opened this issue 10 months ago • 5 comments

We have clients which add an email field on the form eg emailAddress and then set this as the Sender Email in the email workflow, an example is shown below

image

Whilst this makes sense from a content manager's perspective (they can just reply to the email the form's workflow sends).

The website is actually spoofing the email address and this means some commonly used email clients are rejecting the email from being sent.

The correct fix is when someone defines the email workflow, prevent them from entering any fields used in the form definition to be used as a value in the Sender Email field is this something you could look at please?

RachBreeze avatar Apr 23 '24 15:04 RachBreeze

Are you able to use this configuration feature to achieve this end? In particular, hiding the "Sender Email" field such that it'll use the site wide configured value.

AndyButland avatar Apr 23 '24 16:04 AndyButland

Hi @AndyButland

Thanks for getting back so quickly. This wouldn't allow the content manager to use different sender email addresses (there are instances for this too), for example from "[email protected]" and from "[email protected]"

RachBreeze avatar Apr 23 '24 16:04 RachBreeze

OK, will have a think about it. Not 100% sure it should be default behaviour as I suppose it could be that people would want to use the feature your editors are employing, but it could be a configuration option.

I had a look into the existing validation notifications thinking they could be used, but it's not ideal given that the UI allows you to save forms and workflows together, but the underlying services are separate. This means the WorkflowSavingNotification and FormSavingNotification only know about the workflow and form respectively, and not each other.

As some sort of a solution currently you could hook into the WorkflowSavingNotification, check for the specific workflow and setting and empty the value if it contains something starting with { and ending with }. That would at least stop the situation happening and the workflow would fall back to using the Umbraco-wide configured sender address. But that would be a "silent" change as I don't see a way currently to bubble that up to the UI (only cancellations of FormSavingNotifications do this).

AndyButland avatar Apr 24 '24 05:04 AndyButland

Something like this:

using Umbraco.Cms.Core.Events;
using Umbraco.Forms.Core.Models;
using Umbraco.Forms.Core.Providers;
using Umbraco.Forms.Core.Services.Notifications;

namespace Umbraco.Forms.Testsite;

public class WorkflowSavingNotificationHandler : INotificationHandler<WorkflowSavingNotification>
{
    private readonly WorkflowCollection _workflowCollection;

    public WorkflowSavingNotificationHandler(WorkflowCollection workflowCollection) => _workflowCollection = workflowCollection;

    public void Handle(WorkflowSavingNotification notification)
    {
        foreach (Workflow workflow in notification.SavedEntities)
        {
            Core.WorkflowType workflowTYpe = _workflowCollection[workflow.WorkflowTypeId];
            if (workflowTYpe.Id == Guid.Parse(Core.Constants.WorkflowTypes.SendRazorEmail))
            {
                var senderEmailValue = workflow.Settings["SenderEmail"];
                if (senderEmailValue.StartsWith("{") && senderEmailValue.EndsWith("}"))
                {
                    workflow.Settings["SenderEmail"] = string.Empty;
                }
            }
        }
    }
}

AndyButland avatar Apr 24 '24 05:04 AndyButland

Hi @AndyButland Thanks for the quick response.

I understand the issue re validation (now you've explained it :-) ) but also this will be impacting a lot of Umbraco Forms users, as SMTP servers tighten their policies around spoofing and it would be really good if it could be resolved in Umbraco Forms rather than handled as a "Training issue" or a package/ agency implementation

RachBreeze avatar Apr 24 '24 06:04 RachBreeze

Makes sense, thanks.... I've added the validation now so we'll have this in the next releases.

image

AndyButland avatar Jun 19 '24 06:06 AndyButland

Hi @AndyButland

That's perfect thank you

RachBreeze avatar Jun 19 '24 07:06 RachBreeze