Live component force post requests

[hepisec]

Q	A
Bug fix?	no
New feature?	yes
Tickets	Fix #1208
License	MIT

When your component model may contain sensitive data, you probably don't want your data being transferred via GET requests, because then the data might leak to browser history, server logs etc.

With this PR you can add forcePost: true to #[AsLiveComponent] and your component will always use POST requests to talk to the backend.

#[AsLiveComponent(forcePost: true)]
class MySensitiveComponent
{
// ...
}

[weaverryan]

Thanks for getting this rolling - that's awesome! Might it make sense to have the setting not as an attribute but on the #[AsLiveComponent] attribute? If you have a component that requires POST for some reason, then it will ALWAYS need to be POSTed - it shouldn't need to have a data-force-post passed each time.

[hepisec]

I tried that first as this was your first suggestion. But I was not able to pass the property to the frontend yet. Would LiveComponentHydrator be the right place to do so?

[weaverryan]

But I was not able to pass the property to the frontend yet. Would LiveComponentHydrator be the right place to do so?

Try https://github.com/symfony/ux/blob/2.x/src/LiveComponent/src/Util/LiveControllerAttributesCreator.php

If we added a new forcePost: true option to AsLiveComponent, we should be able to get it in there. Let me know if you need help - I can dig in a bit further. Ultimately, we'll add an attribute that would map to a new "value" in the Live stimulus controller. Then the JS part shouldn't be too much different :)

[smnandre]

Should we make that a default ?

I agree with @hepisec, and i'm even ready to say that any data sent to the server should be in POST...

[hepisec]

I've added support for #[AsLiveComponent(forcePost: true)] to the PR. I leave the decision for the default value up to you. I'd like to see the security considerations covered in the documentation.

[smnandre]

Is there something checking the request is sent in POST in the backend side ?

I think somehow the LiveComponentSubscriber should be updated no ?

[hepisec]

I've added a check for forcePost in LiveComponentSubscriber::onKernelController.

[weaverryan]

After talking with a few people, I think we should reverse your PR. We should make components use POST by default, with an option to opt into GET. The reason is the one given in your description. And, more generally, if you think of a live component re-render as a type of "form submit" (and, if the user is changing a prop value... then they likely are filling out a form in one way or another), then form submits should almost always be POST (Symfony forms, for example, default to POST).

Could you change the option from forcePost to method and default it to post? We could throw an exception in the attribute's constructor if some value other than get or post` were passed.

[hepisec]

I'll update the PR in the next days to have POST by default, as suggested by @weaverryan

[WebMamba]

Thanks a lot for your work @hepisec! 🧡

[hepisec]

I've finished the work on this issue. As POST is now the new default method and GET is only allowed when #[AsLiveComponent(method: 'get')] is set, I had to update several tests / fixtures to work with the new default method.

Thanks everyone so far, I'm looking forward to your feedback.

[weaverryan]

Minor notes - this is looking good. Thanks for the patience on the slow review - but this is a very important PR!

[weaverryan]

Friendly ping to you @hepisec :). Let us know if you have time to do the final steps! Thanks!

[hepisec]

I'm starting right now.

[hepisec]

It should be done now. The failed tests seem to be related to TwigComponent and are 8.2 only.

I wish you all happy holidays.

[weaverryan]

Hey @hepisec!

Sorry for the delay - this is ready to merge. Can you rebase this please?

Thanks!

[hepisec]

Rebase done. Looking forward to my first finished contribution to symfony/ux :-)

[weaverryan]

Thank you 1000 for this @hepisec!