[Security] Tweak UsernamePasswordFormAuthenticationListener

- Do not check twice for the only_post condition
- If the expected request is only_post, check only the post variables for the username and password parameters
[2.2] Translatable field type for Propel i18n columns
merged branch kufi/master (PR #5032)

This PR was squashed before being merged into the master branch (closes #5032).

Commits
-------

afba15f [2.2] Translatable field type for Propel i18n columns

Discussion
----------

[2.2] Translatable field type for Propel i18n columns

A field type which allows to automatically generate the correct fields for propels i18n behavior.

Usage example:

     $builder->add('pageI18ns', 'translatable_collection', array(
            'i18n_class' => '\foo\barBundle\Model\PageI18n',
            'languages' => array('de', 'fr'),
            'label' => 'Translations',
            'columns' => array(
                'title' => array(
                    'label' => 'Custom title',
                ),
                'description' => array(
                    'type' => 'textarea'
                )
            )
        ));

With this configuration the field automatically generates the correct fields for the title and description column for the given languages.

---------------------------------------------------------------------------

by stof at 2012-07-24T14:37:27Z

tests are also missing

---------------------------------------------------------------------------

by kufi at 2012-07-27T08:50:05Z

Ok. Moved the Listeners into own classes. Changed the names of the types. Fixed the TranslationCollectionType which now is a Subclass of AbstractType and has the parent collection.

Edit:
The syntax changed slighty for the form:

    $builder->add('pageI18ns', new \Symfony\Bridge\Propel1\Form\Type\TranslationCollectionType(), array(
            'languages' => array('de', 'fr', 'en'),
            'label' => 'Translations',
            'options' => array(
                'data_class' => 'foo\bar\Modell\PageI18n',
                'columns' => array(
                    'title' => array(
                        'label' => 'Custom title',
                    ),
                    'description' => array(
                        'type' => 'textarea'
                    )
                )
            )
        ));

---------------------------------------------------------------------------

by stof at 2012-07-27T08:55:07Z

tests are still missing, and you have some CS issue (which can probably all be fixed by running the [PHP-CS-Fixer](http://cs.sensiolabs.org/) on your classes)

---------------------------------------------------------------------------

by sindro88 at 2012-08-13T13:27:46Z

I followed step by step the implementation but the form type return an error "Could not load type propel1_translation".

---------------------------------------------------------------------------

by kufi at 2012-08-14T06:21:40Z

Could you try again. The problem was that the type propel1_translation_collection relied on a registered form type propel1_translation. I removed this one and replaced it with the actual form class.

---------------------------------------------------------------------------

by sindro88 at 2012-08-14T06:35:33Z

I replaced with the class and now it work, thank you so much!

---------------------------------------------------------------------------

by fabpot at 2012-09-18T16:53:21Z

ping @willdurand

---------------------------------------------------------------------------

by stof at 2012-10-13T17:56:16Z

@willdurand ping

---------------------------------------------------------------------------

by willdurand at 2012-10-23T12:03:22Z

There are a few comments by @stloyd to fix, but I'm :+1: on this PR.

---------------------------------------------------------------------------

by fabpot at 2012-10-23T13:18:59Z

@kufi Can you add a note in the CHANGELOG of the Propel bridge before I merge this PR? Thanks.

---------------------------------------------------------------------------

by kufi at 2012-10-23T13:32:31Z

@fabpot Sure. Does this belong to Version 2.1.0 or the upcoming 2.2.0?

---------------------------------------------------------------------------

by fabpot at 2012-10-23T13:59:04Z

2.2
[Propel1] fixed CHANGELOG
Improve comments in ProfilerController
merged branch raziel057/patch-2 (PR #5847)

This PR was squashed before being merged into the master branch (closes #5847).

Commits
-------

00d2823 Improve comments in ProfilerController

Discussion
----------

Improve comments in ProfilerController

---------------------------------------------------------------------------

by raziel057 at 2012-10-27T21:55:17Z

It's fixed.
moved the secure random class from JMSSecurityExtraBundle to Symfony (closes #3595)
simplified the Prng code
renamed Prng to SecureRandom
moved the secure random dep for remember me as a constructor argument
rename String to StringUtils
added a SecureRandomInterface
fixed CS
fixed tests when OpenSsl is not enabled in PHP, renamed a missnamed test, added missing license doc blocks
merged branch fabpot/prng (PR #4763)

This PR was merged into the master branch.

Commits
-------

aecc9b1 fixed tests when OpenSsl is not enabled in PHP, renamed a missnamed test, added missing license doc blocks
ca567b5 fixed CS
5cdf696 added a SecureRandomInterface
234f725 rename String to StringUtils
5849855 moved the secure random dep for remember me as a constructor argument
248703f renamed Prng to SecureRandom
c0c8972 simplified the Prng code
e5dc7af moved the secure random class from JMSSecurityExtraBundle to Symfony (closes #3595)

Discussion
----------

[2.2][Security] Add a PRNG (closes #3595)

As per #3595, I have moved the secure random class from JMSSecurityExtraBundle to Symfony.

It has more impact than I expected ;)

As you will see, the implementation has been refactored a bit. The most notable change is that Doctrine support has been moved to the bridge with the addition of a proper Doctrine seed provider (Doctrine is not a special case anymore).

The Doctrine configuration has been moved to the DoctrineBundle: doctrine/DoctrineBundle#91

schmittjoh/JMSSecurityExtraBundle#65 removes the code that has been moved.

---------------------------------------------------------------------------

by Seldaek at 2012-07-05T13:26:01Z

I'm all for more security features, and both the String class & the Prng class for wrapping openssl make a lot of sense IMO, but I fail to see the use of the rest.

If we just want a seed to have a fallback in case openssl is missing, I'd rather have a secret in the config.yml than a million classes to store the same secret in the DB. Maybe I'm missing something though? /cc @schmittjoh

---------------------------------------------------------------------------

by schmittjoh at 2012-07-05T16:32:10Z

Having the configuration in different places (SecurityBundle & DoctrineBundle) feels a bit weird. I would prefer an approach similar to ACL, or the user provider/firewall section with factories. The latter being a bit more work to implement and the former potentially asking for complaints about too tight coupling to Doctrine.

Regarding testing, we probably need to move the disableOpenSsl method to the SecureRandom class in order to allow OpenSSL to be disabled for testing and we also need to change the byte generation algorithm to produce the same output for the same starting seed. I agree that it does not make sense to introduce an interface for SecureRandom as only the seed providers should be replaced.

As for the seed itself, it is constantly updated and does not stay the same as in the beginning. Thus, we need a provider that we can write to, and not only read from. I'm also not sure about using OpenSSL on Windows as I have read enough resources which claimed that the entropy on Windows is not always good (including OpenSSL docs). Always using the custom seed provider at least always ensured proper entropy even if OpenSSL's speed issues have been fixed in newer PHP versions.

---------------------------------------------------------------------------

by stof at 2012-07-05T16:44:24Z

@schmittjoh everything is in SecurityBundle now as it does not use a database anymore

---------------------------------------------------------------------------

by stof at 2012-07-05T16:44:59Z

and there is no seed provider anymore either

---------------------------------------------------------------------------

by schmittjoh at 2012-07-05T16:53:39Z

Not having a seed provider is not such a good idea, but having a file-based seed provider is.

---------------------------------------------------------------------------

by Seldaek at 2012-07-05T17:01:18Z

@schmittjoh why would you need to replace the seed provider? Don't you think that people serious about security to the point that they would want a stronger seed provider would enable openssl instead?

---------------------------------------------------------------------------

by stof at 2012-07-05T17:06:50Z

Well, what I meant is that there is no interchangeable provider anymore. The Prng class uses the file directly.

And btw, I think the Prng class should be mockable for tests, so it should either have an interface or not be final (I vote for adding an interface)

---------------------------------------------------------------------------

by jalliot at 2012-07-09T18:46:12Z

@fabpot @schmittjoh What about using more fallbacks for `openssl_random_pseudo_bytes` like in @Seldaek's post ["Unpredictable hashes for humans"](http://seld.be/notes/unpredictable-hashes-for-humans)?
Trying `mcrypt_create_iv` first might also be faster.

---------------------------------------------------------------------------

by Seldaek at 2012-07-10T08:52:46Z

@jalliot I think mcrypt should be after if you make it use /dev/urandom, not 100% sure but openssl is probably higher quality than urandom.

---------------------------------------------------------------------------

by schmittjoh at 2012-07-10T09:12:07Z

The fallback algorithm that I added should be enough (it passes the
statistical randomness tests).

On Tue, Jul 10, 2012 at 10:52 AM, Jordi Boggiano <
reply@reply.github.com
> wrote:

> @jalliot I think mcrypt should be after if you make it use /dev/urandom,
> not 100% sure but openssl is probably higher quality than urandom.
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/4763#issuecomment-6870145
>

---------------------------------------------------------------------------

by stof at 2012-10-13T17:20:06Z

@fabpot please send a PR to the doc so that this can be merged :smiley:

---------------------------------------------------------------------------

by stof at 2012-10-13T17:22:08Z

hmm, actually, some comments have not been taken into account yet so it is not ready to be merged

---------------------------------------------------------------------------

by stof at 2012-10-27T07:14:43Z

you forgot the SecureRandom file

---------------------------------------------------------------------------

by fabpot at 2012-10-27T08:49:54Z

I think I've addressed all the comments. If everyone agree with the current implementation, I'm going to start updating the documentation.

---------------------------------------------------------------------------

by fabpot at 2012-10-27T10:51:15Z

I've fixed the remaining CS issues.

---------------------------------------------------------------------------

by fabpot at 2012-10-28T07:00:31Z

Documentation is here: symfony/symfony-docs#1858
[Security] fixed tests when OpenSSL is not installed
merged branch acasademont/tweak_userform_security_listener (PR #5824)

This PR was merged into the master branch.

Commits
-------

3e58893 [Security] Tweak UsernamePasswordFormAuthenticationListener

Discussion
----------

[Security] Tweak UsernamePasswordFormAuthenticationListener

Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/acasademont/symfony.png)](http://travis-ci.org/acasademont/symfony)
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -

Improvements:

- Do not check twice for the ```only_post``` condition. The condition in the ```attemptAuthentication``` method is useless as this method will never be called if the previous ```requiresAuthentication``` call returns false.
- If the expected request is ```only_post```, check only the POST variables for the username and password parameters. Otherwise, query params and attributes are checked before.
- Use POST instead of post for correctness