FIX Tighten routing rule for userforms ping action

[tiller1010]

… UserDefinedForm

Closes #1223

Summary

• Checks for "Fields" method
• Check if the dataRecord is an instance of UserDefinedForm. If not, return 404.

[GuySartorelli]

Can you please describe the perceived problem this change is intended to solve? I can't think of a scenario where this would be necessary?

[tiller1010]

@GuySartorelli This is to prevent emergencies from visiting "/UserDefinedFormController"
This only occurs from the root url, visiting "/any-page-url/UserDefinedFormController" returns a 404 instead:

Versus

[michalkleiner]

It probably shouldn't error on any level of the structure, but why do you need to visit the URL of the controller? Does it happen when you submit a form that you made to be your homepage?

[GuySartorelli]

After a quick play around, this happens even without any userform on the home page. There's a route in _config/routes.yml that is defined as such:

SilverStripe\Control\Director:
  rules:
    UserDefinedFormController//$Action: SilverStripe\UserForms\Control\UserDefinedFormController

So, first step would be to find out what this route is even doing there in the first place.
Second step would be to see if there's a way to do that without needing a global route, which just feels wrong.
If that's not possible, then an approach like the one in this PR could be considered, but it feels like we're hitting a nail with a screwdriver here.

[NightJar]

The premise of this PR is that a UserDefinedController is being loaded as the controller for a page that does not have a UserForm on it.

The editor extension is missing. Simple as that.

@tiller1010 I would suggest checking your HomePage::$controller_name config (I'm making assumption on naming here, but hopefully you get the idea).

So, first step would be to find out what this route is even doing there in the first place.

@GuySartorelli
The UserDefinedFormController route is to allow for ping actions, in order that someone doing a bazillion field form over 5 "pages" (javascript show/hide events) doesn't have the security token time out on them. The same as the CMS does.

The confusion stems from the route being for a Page type controller, where no page is defined because it's a direct route. However the route URL controller defines "The page for / is [...::class]", so the ContentController subclass can happily load a data model for itself (this is an assumption - I've not checked... that or it's using a placebo "null record" - thus SiteTree, not HomePage or whatever). This is perhaps incorrect behaviour, but is not the root cause of this issue. The controller is not designed to be called directly.

If any commit should result from this error, I would guess that the route should probably more accurately be:

SilverStripe\Control\Director:
  rules:
    UserDefinedFormController/ping:
      Controller: SilverStripe\UserForms\Control\UserDefinedFormController
      Action: ping

---

I missed at the beginning of my reply that the controller is being called directly, with no action. What is the reason for this? I don't think it should happen. The second part of my reply deals more directly to that point.

[GuySartorelli]

Of all of these changes, only the one in routes.yml seems necessary - that on its own resolves the issue.

[GuySartorelli]

I've also changed the target branch to 5.15. Please reset the commits so that this PR has only one commit in it, which is the change being made to routes.yml.

[GuySartorelli]

I've reset the commits and made the requested changes. Someone else will need to review this now, though.

I've made these changes

[emteknetnz]

Tested locally, I can still create and submit userforms

Had a look around, seems reasonable to limit this to just ping

ping is used here - https://github.com/silverstripe/silverstripe-userforms/blob/6/client/src/bundles/UserForms.js#L757