Form fields hidden by visitor groups are incorrectly validated

Vote:
 

This is a very specific bug which took me alot of time to find.

We noticed was that some customers were unable to post their forms. They didn't get any error message at all. They just wasn't able to submit. This happen for them when they had previously filled something wrong and had an invalid form. After they corrected their mistake and tried to send it again it didn't work.

More investigations showed that on the 2nd submit the endpoint /EPiServer.Forms/DataSubmit/Submit would say that fields that shouldn't be visible for the user (due to visitor groups) was being validating

The problem was that we had a form actor which in some cases would call CancelSubmit and we had a custom implementation of ActorsExecutingService which removed the form container from cache after submit. We don't know why the form container was removed from cache after submit as it was added in 2018 and the developer has since quit.

To reproduce:

1. Create a form like the image below. Put 2 or more fields in a personalized group and have them all be mandatory. In my example i had visitor groups that matches if the person has visited "About us" or "Search" on the alloy tech demo site.

2. Create a form actor which will call CancelSubmit. For this purpose we'll make this actor only do a cancel and nothing more

public class MyActor : PostSubmissionActorBase, ISyncOrderedSubmissionActor
{
    public override object Run(object input)
    {
        var result = new SubmissionActorResult { CancelSubmit = true, ErrorMessage = "Actor had a problem!" };
        return result;
    }

    public int Order => 1;
}

3. Add a custom implementation to ActorsExecutingService which clears the form container from IContentCacheRemover. 

public class FormsActorsExecutingService : ActorsExecutingService
{
    public override IEnumerable<IPostSubmissionActor> GetFormSubmissionActors(Submission submission,
        FormContainerBlock formContainer, FormIdentity formIden,
        HttpRequest request, HttpResponse response, bool isFormFinalizedSubmission)
    {
        var actors = base.GetFormSubmissionActors(submission, formContainer, formIden, request, response, isFormFinalizedSubmission)?.ToList();

        ServiceLocator.Current.GetInstance<IContentCacheRemover>().Remove(formContainer.Content.ContentLink);
        return actors;
    }
}

4. In ConfigureServices add the new implementation

services.AddSingleton<ActorsExecutingService, FormsActorsExecutingService>();

5. Try posting the form. In this case i first visited "About us" to be included in that visitor group

6. Inspect the response from /EPiServer.Forms/DataSubmit/Submit which looks as we would expect

{
    "isSuccess": false,
    "isProgressiveSubmit": false,
    "redirectUrl": "",
    "message": "Failed to submit.</br>Actor had a problem!",
    "data": null,
    "additionalParams": null
}

7. Try posting the same data again and you'll see that the form itself looks problem free but isn't posting

8. Check the response from /EPiServer.Forms/DataSubmit/Submit again and you'll see this:

{
    "isSuccess": false,
    "isProgressiveSubmit": false,
    "redirectUrl": "",
    "message": null,
    "data": {
        "submissionId": "00000000-0000-0000-0000-000000000000",
        "formLanguage": null,
        "currentStepIndex": 0,
        "isLastestStep": false,
        "validationInfo": [
            {
                "invalidElement": "9655f4a4-df9f-4a0a-9a4c-7c0727db7abe",
                "invalidElementName": "__field_108",
                "invalidElementLabel": "everyoine",
                "validationMessage": "This field is required.",
                "validator": "EPiServer.Forms.Implementation.Validation.RequiredValidator"
            },
            {
                "invalidElement": "df7b99e6-d120-4309-874c-e4cda4435e06",
                "invalidElementName": "__field_109",
                "invalidElementLabel": "search",
                "validationMessage": "This field is required.",
                "validator": "EPiServer.Forms.Implementation.Validation.RequiredValidator"
            }
        ]
    },
    "additionalParams": null
}

Since these fields shouldn't be validated the error makes no sense. I was digging some in the optimizely code using dotPeek as a symbol server and found out that when the form is being validated the code is looking at formContainer.Form.Steps[0].Elements to find the elements it should validate and matches those elements with the incoming form data. On the first post we would get 2 elements in this property and on the second posting we would get 4. So my conclusion is since episerver no longer has the form container in its cache, it will fetch it from the database without taking visitor groups into account which is causing this problem.

For me the solution was to remove the code which evicted the formscontainer form the cache. I would have prefered not to remove it as I'm sure it was added for some reason back in 2018 but i can't figure out what the reason would be. 

Final words: This is of course very specific to this implementation but I feel that the way episerver fetches a content without regards to visitor groups could be a bug that could affect others in other ways than this specifically scenario.

#317143
Feb 14, 2024 12:40
Vote:
 

We're still having this problem even though we corrected what is described in previous post. We can't figure out the pattern for when it happens. Our client is getting alot of complains regarding this from their customers but we haven't been able to find a pattern to when it happens. Sometimes we've been able to see the the incorrect validation but then we try to post again and the problem is gone. 

#320573
Apr 16, 2024 8:36
Vote:
 

This is still occurring with no real pattern to when it happens. I've made breakpoints in DataSubmitController->Submit where i can see that a field it's trying to validate shouldn't be visible since the visitor group that wraps the element isn't active. So why is it being validated? Am I the only one with this problem? The form in question is really huge with lots of dependencies and visitor group logic

#328209
Aug 28, 2024 9:11
Vote:
 

I think I understand why this is happening. Not sure how to fix it though.

When loading a form container in optimizly the FormContainerBlockController will run the following code:

this._formBusinessService.Service.BuildFormModel(currentBlock);

The BuildFormModel take's the optional parameter buildForAllElements. its' default value is false which is what will be used when loading the container.

    public override void BuildFormModel(FormContainerBlock formContainerBlock, bool buildForAllElements = false)
    {
        if (!(formContainerBlock is IContent formContent))
            return;
        Form currentFormModel = new Form(GetFormGuid(formContent))
        {
            Name = formContent.Name,
            Title = formContainerBlock.Title,
            Description = formContainerBlock.Description,
            RedirectUrl = formContainerBlock.RedirectToPage.GetUrlString(this.GetCurrentFormLanguage(formContainerBlock)),
            AllowAnonymousSubmission = formContainerBlock.AllowAnonymousSubmission,
            AllowMultipleSubmission = formContainerBlock.AllowMultipleSubmission,
            AllowExposingDataFeeds = formContainerBlock.AllowExposingDataFeeds,
            ShowNavigationBar = formContainerBlock.ShowNavigationBar,
            FocusOnForm = formContainerBlock.FocusOnForm,
            ShowSummarizedData = formContainerBlock.ShowSummarizedData,
            ConfirmationMessage = formContainerBlock.ConfirmationMessage,
            ResetConfirmationMessage = formContainerBlock.ResetConfirmationMessage,
            Language = this.GetCurrentFormLanguage(formContainerBlock)
        };
        if (formContent is IChangeTrackable changeTrackable)
        {
            currentFormModel.CreatedDate = changeTrackable.Created;
            currentFormModel.ModifiedDate = changeTrackable.Changed;
        }
        currentFormModel.SourceContent = formContainerBlock.Content;
        this.BuildStepsAndElements(ref currentFormModel, formContainerBlock, buildForAllElements);
        formContainerBlock.Form = (IForm)currentFormModel;
    }
    public virtual void BuildStepsAndElements(
        ref Form currentFormModel,
        FormContainerBlock formContainerBlock,
        bool buildForAllElements = false)
    {
        List<IFormStep> formStepList = new List<IFormStep>();
        List<IFormElement> elements = new List<IFormElement>();
        currentFormModel.Steps = formStepList;
        if (formContainerBlock.ElementsArea == null)
            return;
        foreach (ElementBlockBase elementBlockBase in buildForAllElements ? this.GetAllInnerFormElementBlocks(formContainerBlock) : this.GetDisplayableFormElementBlocks(formContainerBlock))
        {
            IFormElement formElement = elementBlockBase.FormElement;
            if (formElement != null)
            {
                formElement.Form = currentFormModel;
                if (formElement is IFormStep)
                {
                    IFormStep formStep = formElement as IFormStep;
                    formStep.Elements = new List<IFormElement>();
                    formStepList.Add(formStep);
                    if (formStepList.Count >= 2)
                    {
                        this.LinkStepAndItsElements(formStepList[formStepList.Count - 2], elements);
                        elements.Clear();
                    }
                    else if (elements.Count > 0)
                    {
                        formStepList.Insert(0, this.CreateVirtualStep(elements));
                        elements.Clear();
                    }
                }
                else
                    elements.Add(formElement);
                if (elementBlockBase is IElementValidatable)
                    formElement.Validators = this._validationService.Service.GetElementValidators(elementBlockBase as IElementValidatable);
            }
        }
        if (formStepList.Count > 0)
        {
            this.LinkStepAndItsElements(formStepList[formStepList.Count - 1], elements);
            elements.Clear();
        }
        if (formStepList.Count >= 1 || elements.Count <= 0)
            return;
        formStepList.Add(this.CreateVirtualStep(elements));
    }

However, if the site would restart for some reason after loading and before submitting it would mean that the validation logic would need to pull the FormContainerBlock from the database. In that case the following code inside FormContainerBlock.cs would run 

    [Ignore]
    [Newtonsoft.Json.JsonIgnore]
    [System.Text.Json.Serialization.JsonIgnore]
    public IForm Form
    {
      get
      {
        if (this._form == null)
          this._formBusinessService.Service.BuildFormModel(this, true);
        return this._form;
      }
      set => this._form = value;
    }

In this code the buildForAllElements are set to true meaning every form element would be used for validation. This doesn't seem right to me. 

  1. Why would you by default want to fetch all elements instead of elements from the context of the user
  2. Why does this sporadically happen in production? Why isn't the previously loaded form data from the FormContainerBlockController being used during validation? 
#328243
Aug 28, 2024 14:12
Vote:
 

I think this is a bug in optimizely. I've remedied is now by overriding PerformDataSubmitAsync in DataSubmissionService and changed this:

        var formIdentity = new FormIdentity
        {
            Guid = formGuid,
            Language = formLanguage
        };
        var formContainer = formIdentity.GetFormBlock();

to this

        var formIdentity = new FormIdentity
        {
            Guid = formGuid,
            Language = formLanguage
        };
        var formContainer = formIdentity.GetFormBlock();
        
        // CUSTOM CODE START
        _formBusinessService.Service.BuildFormModel(formContainer);
        // CUSTOM CODE END

So since GetFormBlock just does a GetContent from the IContentLoader it will use the Form property of the FormContainerBlock content type and load all elements. My line overrides this behaviour and makes sure only the elements for this users context is loaded and validated.

I would have wanted to to this change in the validation logic only and that might actually be a valid way to go. But I'm worried this bug could affect other things during posting so therefor I wanted to add it when the form container is loaded.

This method im overriding has alot of lines of code which isn't optimal to override since things could change in future versions of EPiServer.Forms. I would love some input from an optimizely employee on this

#328327
Aug 29, 2024 9:45
Vote:
 

Hi Andreas Ljungström, since this is a specific case, and not easy to reproduce, I think we better create a Support Case for it, we might need to take a look at your project, log files... before we can decide how to deal with this issue.

#328502
Aug 30, 2024 6:28
Vote:
 

Hi Phu Nguyen, did you read the latest things i've written here? I feel like problem is very reproducable now and I think this is a generic bug. Just wanted to know if there is any reason for this behavior or not

To reproduce:

  1. Create a form with one element that's hidden for you because of a visitor group.
  2. Make the hidden element required
  3. Load the form
  4. Restart the application
  5. Submit the form

This would make sure that the submit doesn't pick up the loaded form from cache and need to reload it and while reloading it, it does so incorrectly. Namely by loading all elements instead of just the user context element which result in an error message that tells you that a form element that isn't even visible for you doesnt validate.

So what i was wondering was if there is anything I'm missing. Why would the submit code want to load all elements instead of just the user context elements?

I've verified this is a problem on an alloy demo site as well

#328504
Edited, Aug 30, 2024 9:04
Vote:
 

Yes thank you Andreas Ljungström, I can easily reproduce the bug using your steps. I created a bug for this, it will appear in the public bug list soon so you can see the progress at
https://world.optimizely.com/support/Bug-list/bug/AFORM-4374

#328505
Aug 30, 2024 9:25
Vote:
 

Hi Phu Nguyen, 

We are still encountering this issue. I successfully replicated it on the Alloy MVC site using the following steps:

1. Install the default Alloy site.
2. Add the latest EPIServer.Forms NuGet package (version 5.10.0 as of 10/18/2024).
3. Create a form with fields for first name, last name, email, and a submit button.
4. Add two additional fields for personalization, ensuring both are marked as required.
5. Create two user accounts.
6. Establish basic audiences with the Criteria Type set to User Profile, using "Username Equals user1" for one and "Username Equals user2" for the other.
7. Apply personalization to the two fields based on the criteria defined in step 6.
8. Create a Standard Page and incorporate the form created earlier.
9. Start the site and load the page in a browser.
10. Log in with user1 to ensure personalized field 1 is displayed.
11. Restart the site.

After filling out the form without reloading the page, the submission failed. Upon checking the network tab, it indicated that personalized field 2 is required.

#331636
Oct 18, 2024 17:31
* You are NOT allowed to include any hyperlinks in the post because your account hasn't associated to your company. User profile should be updated.