merged branch asm89/refactor-authentication-success-handling (PR #4599)

Commits
-------

bb138da [Security] Fix regression after rebase. Target url should be firewall dependent
eb19f2c [Security] Add note to CHANGELOG about refactored authentication failure/success handling [Security] Various CS + doc fixes [Security] Exception when authentication failure/success handlers do not return a response [Security] Add authors + fix docblock
f9d5606 [Security] Update AuthenticationFailureHandlerInterface docblock. Never return null
915704c [Security] Move default authentication failure handling strategy to seperate class [Security] Update configuration for changes regarding default failure handler [Security] Fixes + add AbstractFactory test for failure handler
c6aa392 [Security] Move default authentication success handling strategy to seperate class [Security] Update configuration for changes regarding default success handler [Security] Fix + add AbstractFactory test

Discussion
----------

[Security] Refactor authentication success handling

Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/asm89/symfony.png?branch=refactor-authentication-success-handling)](http://travis-ci.org/asm89/symfony)
License of the code: MIT

This PR extracts the default authentication success handling to its own class as discussed in #4553. In the end the PR will basically revert #3183 (as suggested by @schmittjoh) and fix point one of #838.

There are a few noticeable changes in this PR:
- This implementation changes the constructor signature of the `AbstractAuthentictionListener` and `UsernamePasswordFormAuthenticationListener` by making the `AuthenticationSuccessHandler` mandatory (BC break). If this WIP is approved I will refactor the failure handling logic too and then this will also move one place in the constructor
- This PR reverts the change of making the returning of a `Response` optional in the `AuthenticationSuccessHandlerInterface`. Developers can now extend the default behavior themselves

@schmittjoh Any suggestions? Or a +1 to do the failure logic too?

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

by schmittjoh at 2012-06-17T23:53:07Z

+1 from me

@fabpot, what so you think?

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

by fabpot at 2012-06-19T08:15:48Z

Can you add a note in the CHANGELOG? Thanks.

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

by asm89 at 2012-06-19T10:22:20Z

I will, but I'll first do the same for the failure logic.

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

by travisbot at 2012-06-21T08:03:14Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1671555) (merged 17c8f66f into 55c6df99).

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

by asm89 at 2012-06-21T08:45:38Z

:+1: thank you @stof. I think this is good to go now.

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

by travisbot at 2012-06-21T08:50:28Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1671817) (merged 8982c769 into 55c6df99).

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

by asm89 at 2012-06-21T14:23:58Z

@schmittjoh @fabpot The `LogoutListener` currently throws an exception when the successhandler doesn't return a `Response` ([link](https://github.com/symfony/symfony/blob/9e9519913d2c5e2bef96070bcb9106e1e389c3bd/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php#L101)). Should this code check for this too?

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

by schmittjoh at 2012-06-21T14:26:49Z

Yes, this code was removed, but needs to be re-added here as well.

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

by travisbot at 2012-06-21T15:08:59Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1674437) (merged 5afa240d into 55c6df99).

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

by asm89 at 2012-06-26T06:01:02Z

@fabpot Can you make a final decision on this? If you decide on point 3, this code can be merged.  I agree with the arguments of @stof about the option handling and it 'only' being a BC break for direct users of the security component. I even think these direct users should be really careful anyway, since the behavior of the success and failurehandlers now change back to how they acted in 2.0.

Now I am thinking about it, can't the optional parameters of this class move to setters anyway? That will make it cleaner to extend.

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

by asm89 at 2012-06-28T10:29:50Z

ping @fabpot

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

by fabpot at 2012-06-28T17:23:02Z

I'm ok with option 1 (the BC break). After doing the last changes, can you squash your commits before I merge? Thanks.

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

by asm89 at 2012-07-06T21:59:54Z

@fabpot I rebased the PR, added the authors and also ported the fix that was done in 8ffaafa86741a03ecb2f91e3d67802f4c6baf36b to be contained in the default success handler. I also squashed all the CS and 'small blabla fix' commits. Is it ok now?

Edit: travisbot will probably say that the tests in this PR fail, but that is because current master fails on form things

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

by asm89 at 2012-07-08T18:53:05Z

I rebased the PR, tests are green now: [![Build Status](https://secure.travis-ci.org/asm89/symfony.png?branch=refactor-authentication-success-handling)](http://travis-ci.org/asm89/symfony).
[Form] Output a more usable error when PropertyPath has tried to find adders and getters, but failed to find them
merged branch SamsonIT/error_explaining_lack_of_adder_remover (PR #4777)

Commits
-------

1fa22d9 [Form] Output a more usable error when PropertyPath has tried to find adders and getters, but failed to find them

Discussion
----------

[Form] Output a more usable error when PropertyPath has tried to find ad...

...ders and getters, but failed to find them

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -

I've refactored the writeProperty method of propertypath in order to supply a better error message when writing has failed.

The writeProperty method itself now finds singulars (if a singular was not passed) for the private findAdderAndRemover method which allowed for some duplicate code to be removed and since the writeProperty now holds this data, it can provide a more verbose exception message.

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

by bschussek at 2012-07-09T13:54:35Z

Apart from the typo this PR looks good.

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

by Burgov at 2012-07-09T14:01:04Z

fixed&squashed
[Form] Unified rendering of errors for nested elements
merged branch bamarni/master (PR #4792)

Commits
-------

6c9c2ec [ClassCollectionLoader] fixed comment striping on classes in global namespace

Discussion
----------

[ClassCollectionLoader] fixed comment striping on classes in global namespace

Comments aren't striped when the class is in the global namespace, this adds a fake namespace to let the fixNamespaceDeclaration method handling the formating, so we can remove a method.

I've also put dev as minimum policy in composer, otherwise I couldn't install dependencies.
merged branch frosas/start-session-on-generate-csrf-token (PR #4781)

Commits
-------

6d94f3e Ensure there is a session before getting the session id

Discussion
----------

[Form] Ensure there is a session before getting the session id

Solves "The CSRF token is invalid. Please try to resubmit the form" error when a form is generated before the session is started.

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

by fabpot at 2012-07-09T10:23:32Z

Adding a CSRF token only makes sense if you are on a page with a "user". If not (and if you don't use HTTP auth or whatever), then there is no need for a CSRF token.

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

by frosas at 2012-07-09T14:42:40Z

This PR doesn't change any logic on whether a CSRF token is added or not, it just fixes a bug when a token is requested.
[Form] Fixed: ChoiceType omits the "empty_value" option if the choices contain an empty element
merged branch bschussek/issue3864 (PR #4801)

Commits
-------

040ba8f [Form] Fixed: ChoiceType omits the "empty_value" option if the choices contain an empty element

Discussion
----------

[Form] Fixed: ChoiceType omits the "empty_value" option if the choices contain an empty element

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3854, #3864
Todo: -
merged branch gajdaw/finder_splfileinfo_fpassthu (PR #4751)

Commits
-------

b4d7a7e [Component][Finder][SplFileInfo] file_get_contents=>fpassthru

Discussion
----------

[Component][Finder][SplFileInfo] file_get_contents=>fpassthru

Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: https://github.com/symfony/symfony/pull/4335/files#r1016152
Todo: -
License of the code: MIT
Documentation PR: -
fixed CS
Update to the "Upgrade to v2.1" documentation: the way HTML attributes are passed for the `form_label` function has changed.
Simpler sentence concerning upgrade of the form_label function
[Form] Refactored processing of the "empty_value" option in DateType
merged branch michaelperrin/upgrade_to_21 (PR #4802)

Commits
-------

81eb722 Simpler sentence concerning upgrade of the form_label function
f6f1bb5 Update to the "Upgrade to v2.1" documentation: the way HTML attributes are passed for the `form_label` function has changed.

Discussion
----------

Update upgrade to 2.1 documentation concerning form_label

The way HTML attributes are passed for the `form_label` function has changed since this commit :
https://github.com/symfony/symfony/commit/167e64f79992f6366108fde1548e2a2c9552d3c6

I also made a PR for the documentation:
https://github.com/symfony/symfony-docs/pull/1542
merged branch everzet/twig-paths (PR #4649)

Commits
-------

dbeff69 [TwigBundle] added support for custom loader paths

Discussion
----------

[TwigBundle] added support for custom loader paths

Before this commit, there was no ability to specify custom
search paths for Twig loader. Lets say we have twig templates
outside bundles directories (parts of the domain logic, not
application) - we want to be able to load them.

This commit adds `loader_paths` parameter to twig config,
which is used to set custom paths to the loader.

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

by travisbot at 2012-06-25T09:50:44Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1699654) (merged dbeff697 into 03c8d4d2).

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

by vicb at 2012-06-26T06:14:30Z

You also need to support xml in the configuration and update the xsd file.

edit: adding some DI unit tests is probably a good idea.

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

by everzet at 2012-06-26T08:49:20Z

@vicb agree, was just a fast stabbing ;)

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

by fabpot at 2012-06-28T14:06:02Z

I'm +1. Can you "finish" the PR?

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

by fabpot at 2012-07-02T13:23:33Z

@everzet If you don't have time, I can do the remaining work.
[TwigBundle] added a new paths setting to allow configuration of more paths for the filesystem loader (refs #4649)
[Form] Fixed display of empty values in DateType, TimeType and DateTimeType if they are not required
[Validator] fixed error message for dates like 2012-02-31 (closes #4223)
Merge branch '2.0'

* 2.0:
  [Validator] fixed error message for dates like 2012-02-31 (closes #4223)
merged branch bschussek/issue4596 (PR #4804)

Commits
-------

e6b5595 [Form] Fixed display of empty values in DateType, TimeType and DateTimeType if they are not required
14e293f [Form] Refactored processing of the "empty_value" option in DateType

Discussion
----------

[Form] Fixed display of empty values in DateType, TimeType and DateTimeType

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4596
Todo: -