symfony2admingenerator / AdmingeneratorGeneratorBundle

(old-legacy) Admingenerator for Symfony2, parse generator.yml files to build classes
http://symfony2admingenerator.org/
MIT License
360 stars 125 forks source link

Conflict with [FrameworkBundle] Added Crontoller::isCsrfTokenValid [SF 2.6.0] #789

Open enkaho opened 10 years ago

enkaho commented 10 years ago

AdmingeneratorGeneratorBundle / Resources / templates / CommonAdmin / csrf_protection.php.twig

cf : https://github.com/symfony/symfony/commit/479c83351cc1ef390ad46787a68e526f50f1c961

ioleo commented 10 years ago

@enkaho thanks for the notice

sescandell commented 10 years ago

Thanks...

Unfortunatelly... the only way I see for now to handle that change is to introduce a BC Break. Does anyone has a (simple) idea to avoid this BC?

I thought about testing the Symfony version and if it is less then 2.5: we don't change anything, otherwise the function will be different, but we still have a BC break for 2.6...

If we strictly want to respect semver, we need to create a new Major version... What's your opinion?

sescandell commented 10 years ago

See branch https://github.com/symfony2admingenerator/AdmingeneratorGeneratorBundle/tree/fix-789

sescandell commented 10 years ago

ping @loostro

Matter here is, depending on your Symfony version, we don't generate the same method name... a kind of BC Break (if you overrided the isCsrfTokenValid()).

But I was wondering something... based on the situation where someone upgrade its Symfony to 2.6, the bundle will not be anymore compatible with it. So, except by a BC Break it will not be able to use this bundle anymore. But given that the developper make an upgrade of Symfony2.6, we can assume he will have to change its customization... so we can easily imagining changing our function and so provide compatibilities with Symfony 2.6+

Otherwise, we'll need to update composer.json and constraint the max version of Symfony to <2.6

I don't have any clear idea about what we can do here... I think the better would be to "introduce" this BC Break (with a very clear explanation). I think impact is limited (I don't think many user override the isCsrfTokenValid() function). Or do we need to create a 2.0.0 version... just for that... that will be complicated to maintain many versions...

ioleo commented 10 years ago

@sescandell We can do two things:

And when next LTS is released (2.7) then merge this patch into repository.

sescandell commented 10 years ago

Sounds like a good idea...

Let's do it

robertfausk commented 9 years ago

@sescandell @loostro What need csrf_token.php.twig to look like? Is there a base template to inherit from? I couldn't find one. I'm in need of upgrade to sf2.6 but the AdminGeneratorBundle causes trouble.

ioleo commented 9 years ago

@robertfausk see this changelog

robertfausk commented 9 years ago

Yeah, I already did. But I have no luck with overriding.

I placed this

{% block csrf_protection_use %}
use Symfony\Component\Security\Core\Exception\InvalidCsrfTokenException;
{% endblock %}

{% block csrf_check_token %}

    /**
     * Check crsf token provided for action
     *
     * @throw InvalidCsrfTokenException if token is invalid
     */
    protected function isCsrfTokenValid($intention)
    {
        $token = $this->getRequest()->request->get('_csrf_token');
        if (!$this->get('form.csrf_provider')->isCsrfTokenValid($intention, $token)) {
            throw new InvalidCsrfTokenException();
        }
    }

{% endblock %}

{% block csrf_action_check_token %}
// Check CSRF token for action
$intention = $this->getRequest()->getRequestUri();
$this->isCsrfTokenValid($intention);
{% endblock %}

{% block csrf_action_check_batch_token %}
// Check CSRF token for batch action
$intention = '{{ namespace_prefix }}_{{ bundle_name ~ (builder.BaseGeneratorName ? '_' ~ builder.BaseGeneratorName :'')}}_batch';
$this->isCsrfTokenValid($intention);
{% endblock %}

in app/Resources/AdmingeneratorGeneratorBundle/templates/CommonAdmin/csrf_protection.php.twig but it doesn't override the template.

I had also no luck with app/Resources/Admingenerator/GeneratorBundle/templates/CommonAdmin/csrf_protection.php.twig like you suggested in https://github.com/symfony2admingenerator/AdmingeneratorGeneratorBundle/issues/789#issuecomment-58965290

sescandell commented 9 years ago

Hi @robertfausk

Actually to "override" a template you need to add your "templates" directory as a config parameter. Take a look to the param templates_dirs https://github.com/symfony2admingenerator/AdmingeneratorGeneratorBundle/blob/master/DependencyInjection/Configuration.php#L72

robertfausk commented 9 years ago

@sescandell Thank you very much. I got it now. Works like a charm ;)

Additionally I had to create one emtpy dir named Doctrine (or DoctrineODM or Propel) to get it working. Also see doc: https://github.com/symfony2admingenerator/AdmingeneratorGeneratorBundle/blob/master/Resources/doc/cookbook/extending-generator-templates.md

sescandell commented 9 years ago

You're right... I forgot to mention this.

bobvandevijver commented 9 years ago

@robertfausk Exactly the reason I've written that documentation: A long time ago I had the same problem ;)

fjbatresv commented 9 years ago

Hello I am trying to edit the templates, i have a SF 2.6, Here is my directory

app
     Resources
          Admingenerator
               GeneratorBundle
                    templates
                         CommonAdmin
                              *templates*
                         Propel

And on my config:

templates_dirs: ["%kernel.root_dir%/../app/Resources/AdmingeneratorGeneratorBundle/templates" ]

Can someone explainme why this does not work properly.

sescandell commented 9 years ago

@fjbatresv if you are working on Windows, add a file in your Propel directory (even if it is empty), and it should work.

fjbatresv commented 9 years ago

@sescandell I am working on a linux distibution and this doesnt work.

sescandell commented 9 years ago

OK...

I think this is because of your path. You should use this: templates_dirs: ["%kernel.root_dir%/../app/Resources/Admingenerator/GeneratorBundle/templates" ]

sebastianlp commented 9 years ago

I think with Symfony 2.7.* this doesn't work any more :(

This is my folder structure:

app
 Resources
  Admingenerator
   GeneratorBundle
    templates
     CommonAdmin
      csrf_protection.php.twig (with proper content)
     Doctrine
      .gitkeep

And my config.yml

admingenerator_generator:
    [...]
    templates_dirs: [ "%kernel.root_dir%/../app/Resources/templates" ]

Am I doing something wrong here?

bobvandevijver commented 9 years ago

@sebastianlp Check if you made every step as described here.

Futhermore, the fix is, if I'm correct, now being developed in the newer version of the Bundle (see here. So, it might actually be time for you to update to the new bundle, also to benefit of new functions.

sescandell commented 9 years ago

@sebastianlp

Your path should be "%kernel.root_dir%/../app/Resources/Admingenerator/GeneratorBundle/templates"

sebastianlp commented 9 years ago

@bobvandevijver , @sescandell Yeah, I forgot to update the answer. I'm using:

admingenerator_generator:
    templates_dirs: [ "%kernel.root_dir%/../app/Resources/Admingenerator/GeneratorBundle/templates" ]

and this is my folder structure: screenshot from 2015-06-22 22 10 00

Thank you guys

bobvandevijver commented 9 years ago

In te docs the directory is mentioned as AdmingeneratorGeneratorBundle, so without the extra subdir per Symfony convention. I also have that and it works, so you could try that.

sescandell commented 9 years ago

OK... I'll make a test on the demo project moving on sf2.7 and let you know.

sebastianlp commented 9 years ago

@bobvandevijver Thank you. I tried both paths, that one in the image and what you mention in the last comment, both with error.

@sescandell Thank you, if it's more convenient to you, I can upload my project

sescandell commented 9 years ago

Working on it.

I'll probably be able to provide you a response tomorrow