Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FormTypeCsrfExtension overwrites my Extension's settings #19735

Closed
dragu opened this issue Aug 25, 2016 · 7 comments
Closed

FormTypeCsrfExtension overwrites my Extension's settings #19735

dragu opened this issue Aug 25, 2016 · 7 comments

Comments

@dragu
Copy link

dragu commented Aug 25, 2016

Hi,

I've created an extension disabling CSRFTokens. Unfortunately Symfony\Component\Form\Extension\Csrf\Type\FormTypeCsrfExtension is overwriting my csrf_protection value. If I comment the line

'csrf_protection' => $this->defaultEnabled,

my value is passed to form options and my extension works. While debugging I've changed bool to some string to make sure I see value set by my extension, so I'm pretty sure my extension is ok.

My extension class:

class DisableCSRFExtensionForRestApi extends AbstractTypeExtension
{
    /**
     * @var RequestStack
     */
    private $requestStack;

    /**
     * @var RestApiCallRecognizer
     */
    private $restApiCallRecognizer;

    public function __construct(RequestStack $requestStack, RestApiCallRecognizer $restApiCallRecognizer)
    {
        $this->requestStack = $requestStack;
        $this->restApiCallRecognizer = $restApiCallRecognizer;
    }

    /**
     * @param OptionsResolver $resolver
     */
    public function configureOptions(OptionsResolver $resolver)
    {
        if (!$this->restApiCallRecognizer->isApiCall($this->requestStack->getCurrentRequest())) {
            return;
        }

        $resolver->setDefaults([
            'csrf_protection' => false,
        ]);
    }

    /**
     * @return string
     */
    public function getExtendedType()
    {
        return 'form';
    }
}

My services.yml:

    restapi.form.extension.csrf_disable:
        class: AppBundle\Form\Extension\DisableCSRFExtensionForRestApi
        arguments: [@request_stack, @restapi.call_recognizer]
        tags:
            - { name: form.type_extension, extended_type: Symfony\Component\Form\Extension\Core\Type\FormType, alias: form }

I've tried to change priority of my extension (to -1000, 1000 and 100000) but it didn't help.

@javiereguiluz
Copy link
Member

I can't help you with the form extension problem ... but in case you are not aware of it, you can disable CSRF via config options:

  1. Individually for each form:
use Symfony\Component\OptionsResolver\OptionsResolver;

class TaskType extends AbstractType
{
    // ...

    public function configureOptions(OptionsResolver $resolver)
    {
        $resolver->setDefaults(array(
            // ...
            'csrf_protection' => false,
        ));
    }

    // ...
}
  1. Globally for all forms:
# app/config/config.yml
framework:
    form:
        csrf_protection:
            enabled: false

If you create a new kernel to serve the API, you can easily use the second alternative to disable CSRF for the entire API.

@MacDada
Copy link
Contributor

MacDada commented Aug 29, 2016

you can disable CSRF via config options […] Individually for each form

We decided not to do that, as all forms in REST API don't require CSRF protection.

you can disable CSRF via config options […] globally for all forms

Nope, need that just for the API.

f you create a new kernel to serve the API, you can easily use the second alternative to disable CSRF for the entire API.

Hmmm… worth exploring. But extension should work as well…

@javiereguiluz
Copy link
Member

@MacDada the multi-kernel solution is way easier than it looks (and it will increase the performance of your API because you'll remove lots of unnecessary bundles and features).

There is a pending PR in the symfony-docs repo explaining it: https://github.com/symfony/symfony-docs/pull/6840/files

@dmaicher
Copy link
Contributor

Can you reproduce the issue on a clean symfony standard edition fork? https://github.com/symfony/symfony-standard

@HeahDude
Copy link
Contributor

Unfortunately form type extensions don't support priority.

I suggest you use a normalizer to handle the option outside of the inheritance and extension processes and resolve the option when it is called under condition:

// in your extension
if ($resolver->isDefined('csrf_protection')) {
    $resolver->setNormalizer('crsf_protection', function (Options $options, $csrf) {
        if (!$this->restApiCallRecognizer->isApiCall($this->requestStack->getCurrentRequest())) {
            return $csrf;
        }

        return false;
    });
}

@dragu
Copy link
Author

dragu commented Aug 30, 2016

@dmaicher

https://github.com/dragu/symfony-standard/tree/csrf_extension

@dmaicher
Copy link
Contributor

Thanks for the fork 😄

The problem is indeed the order in which the type extensions are registered. As you load your services.yml within your app's config.yml it gets processed too early and symfony's extension overwrites your defaults again.

Moving your services.yml file into the AppBundle and loading it inside src/AppBundle/DependencyInjection/AppExtension.php works.

But maybe the solution suggested by @HeahDude is also worth looking into 😉

fabpot added a commit that referenced this issue Sep 14, 2016
…pe extension tags (dmaicher)

This PR was merged into the 3.2-dev branch.

Discussion
----------

[FrameworkBundle] add support for prioritizing form type extension tags

| Q             | A
| ------------- | ---
| Branch?       | "master"
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #19735
| License       | MIT
| Doc PR        | symfony/symfony-docs#6958

This PR proposes to add support for `priority` on `form.type_extension` dependecyinjection tags to enable sorting/prioritizing form type extensions.

Issue was mentioned here: #19735

Commits
-------

a3db5f0 [FrameworkBundle] add support for prioritizing form type extension tags
@fabpot fabpot closed this as completed Sep 14, 2016
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this issue Sep 14, 2016
…pe extension tags (dmaicher)

This PR was merged into the 3.2-dev branch.

Discussion
----------

[FrameworkBundle] add support for prioritizing form type extension tags

| Q             | A
| ------------- | ---
| Branch?       | "master"
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #19735
| License       | MIT
| Doc PR        | symfony/symfony-docs#6958

This PR proposes to add support for `priority` on `form.type_extension` dependecyinjection tags to enable sorting/prioritizing form type extensions.

Issue was mentioned here: symfony/symfony#19735

Commits
-------

a3db5f0 [FrameworkBundle] add support for prioritizing form type extension tags
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants