Fix the wrong CSRF token storage being wired

[Toflar]

Q	A
Fixed issues	Fixes #1491
Docs PR or issue	-

If not specified explicitly, Symfony will autowire to the default CSRF token storage which is the session one :)

[aschempp]

Can you explain what the change does?

[aschempp]

Can you explain how the change works?

[Toflar]

What do you mean by explain? You want me to add a comment in the services.yml or what should I do? It just wires the correct service, I don‘t know what else to say?🤷‍♂️

[aschempp]

I had to read up on the Symfony docs 😇

So the issue is exactly the reason people argument that you should not use autowiring in a bundle. Now I wonder:

1. why do we use autowiring here, but „forbid“ it for everything else?
2. should we „solve“ the autowiring issue by creating our own „subclass“ of the CsrfTokenStorage?

[Toflar]

1. why do we use autowiring here, but „forbid“ it for everything else?

I was asking myself the same, I didn't understand why you guys did that.

2. should we „solve“ the autowiring issue by creating our own „subclass“ of the `CsrfTokenStorage`?

How would that help? Was that maybe a mistake and you meant the TokenStorageInterface?

[leofeyer]

1. why do we use autowiring here, but „forbid“ it for everything else?

I was asking myself the same, I didn't understand why you guys did that.

Me neither. Who added this anyway?

[fritzmg]

Hm, not really sure. I added the controller.service_arguments tag in ad125f7 . Is that what makes the subscribed services "autowired"? How did it work before that?

[aschempp]

I was asking myself the same, I didn't understand why you guys did that.

We‘re using Symfony‘s AbstractController which uses autowiring by default. Probably because controllers usually live in the app and not bundles.
However, it‘s currently good to use a service locator for lazy loading, because our controller has way too many actions (which is a bad thing).

How would that help? Was that maybe a mistake and you meant the TokenStorageInterface?

Not really. We can add an interface as we, but the current issue is because our service uses a „default“ Symfony class. We‘d need to override/extend that to make autowiring work.

[aschempp]

I‘ve added „up for discussion“ because we probably want to talk about it tomorrow on Mumble.

[leofeyer]

As discussed in Mumble on April, 9th, we want to merge the PR to fix the issue.

[Toflar]

Sorry, accidentally deleted the branch.

[leofeyer]

Thank you @Toflar.