[FLOW3-general] RFC: Disable automatic persistence for GET requests

[Sebastian Kurfürst]

Hey Bastian,

thanks for the discussion.  However I don't think we can make this
decision until the release -- seems too quick and not well-enough
thought for me.

> PROS:
> + performance
>
> Usually more than 90% of the requests are readonly. It would probably
> be a huge speedup to avoid all the dirty checks for those.
No, that's wrong. A dirty check *only occurs* if the user calls
$someRepository->update(). When you have a read action without an
update() call, *no dirty checking* at all occurs. If you never called
$someRepository->update(), then $persistenceManager->persistAll() is
essentially a no-op.

> + caching
>
> Caching would be more efficient if we could assume that GET requests
> are stateless. Also it would probably make it much easier to integrate
> 3rd party proxies like varnish.
IMHO that's also not a big problem. We *know* in our framework if we
updated some objects or only did read-only actions; so *we* can send the
appropriate cache-control headers (expiry time, etag, ...). This makes
sure that a "delete" action is *not* cached, but other actions are
cached by varnish or any other proxy.

> + nicer URLs
>
> Currently all links that point to a protected action will - by default
> - have a CSRF token appended, that looks like
> "?__csrfToken=3cb0adda641c1eac31b099cecd7055bd". The main reason why
> we need this is because a GET request could be dangerous.
For me, I like that the csrf token is *always* prepended; because it can
also be that you trigger some "dangerous" actions which do not modify
persistence. Imagine a call to an external web service, modifying state
there.

So, it should be safe by default (== have the csrf token prepended); and
if you are sure the URI does not need it, you can remove it (as it is
done right now).

Furthermore, what we'd need to do is to tag actions as being *only*
accessible via POST- or GET; Imagine an attacker who modifies the
request from POST to GET, removes the HMAC and adds some fields... That
would certainly be possible if you do not tag the actions yourself.


> + HMAC
>
> IMO the HMAC is a really convenient way to add touchless security on a
> granular level. Imagine you can install a user registration package
> and define what fields can be modified only by editing the Fluid
> template..
> One of the problems of our previous implementation was, that it was
> difficult to tell when an HMAC was required. With this change we could
> - by default - require it for all POST/PUT/DELETE requests.
I think that's not really an issue. Instead, we should check the target
of the form, and check if the target action has the HMAC stuff
activated. Taking the decision based on the method is IMHO a shortcut
and an un-expected behavior (people who change their form method from
POST to GET will not expect the HMAC to be gone).


So, my personal bottom line:

  * It's a change which would involve quite many layers; and have quite
    some side-effects at all areas of the framework
  * it's a too-dangerous change before 1.0 final (TODAY is feature
    freeze, remember)
  * We have to watch out to not couple the HTTP behavior too tightly to
    e.g. persistence, as it can always be that users only use parts of
    the framework (e.g call external web services)
  * We should not only think about CRUD; and I think when you discuss
    more involved / complex workflows, want to work with disabled
    JavaScript, etc, this has more drawbacks then benefits.


So, for now (at least) this is a big -2 from my side ;-) I'm still open
for discussion, but please don't modify this anymore for 1.0.

Greets,
Sebastian