summaryrefslogtreecommitdiffstats
path: root/Http/Firewall
Commit message (Collapse)AuthorAgeFilesLines
* [CS Fix] Consistent coding-style of concatenation operator usageDariusz Górecki2013-04-021-1/+1
|
* merged branch adrienbrault/security-feature (PR #4776)Fabien Potencier2013-03-231-1/+9
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This PR was merged into the master branch. Discussion ---------- [2.2] [Security] Add an option to disable the hasPreviousSession() check in AbstractAuthenticationListener Bug fix: no Feature addition: yes Backwards compatibility break: no Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/adrienbrault/symfony.png?branch=security-feature)](http://travis-ci.org/adrienbrault/symfony) Fixes the following tickets: #3703 Todo: Add this option to the symfony doc security configuration reference License of the code: MIT Documentation PR: N/A As stated in #3703, all authentication listeners that inherit from AbstractAuthenticationListener, only work when a previous session has been created. This PR allows to change the default behavior in the security.yml file. Example: ```yml security: firewalls: secured_area: pattern: ^/demo/secured/ form_login: check_path: /demo/secured/login_check login_path: /demo/secured/login require_previous_session: false # The default value is true logout: path: /demo/secured/logout target: /demo/ #anonymous: ~ #http_basic: # realm: "Secured Demo Area" ``` PS: While removing my old commit, it closed the #4774 PR ... Commits ------- 0562463 [Security] Add an option to disable the hasPreviousSession() check in AbstractAuthenticationListener
| * [Security] Add an option to disable the hasPreviousSession() check in ↵Adrien BRAULT2012-07-091-1/+2
| | | | | | | | AbstractAuthenticationListener
* | Merge branch '2.1' into 2.2Fabien Potencier2013-02-111-2/+4
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * 2.1: added support for the X-Forwarded-For header (closes #6982, closes #7000) fixed the IP address in HttpCache when calling the backend [EventDispatcher] Added assertion. [EventDispathcer] Fix removeListener [DependencyInjection] Add clone for resources which were introduced in 2.1 [DependencyInjection] Allow frozen containers to be dumped to graphviz Fix 'undefined index' error, when entering scope recursively [Security] fixed session creation on login (closes #7011) Add dot character `.` to legal mime subtype regular expression [HttpFoundation] fixed the creation of sub-requests under some circumstancies (closes #6923, closes #6936)
| * | [Security] fixed session creation on login (closes #7011)Adrien Samson2013-02-071-2/+4
| | |
* | | Merge branch '2.1' into 2.2Fabien Potencier2013-02-071-2/+4
|\ \ \ | |/ / | | | | | | | | | | | | | | | * 2.1: [HttpKernel] fixed the creation of the Profiler directory [Security] fixed session creation when none is needed (closes #6917) [FrameworkBundle] removed obsolete comment (see 2e356c1)
| * | [Security] fixed session creation when none is needed (closes #6917)Fabien Potencier2013-02-041-2/+4
| | |
* | | merged branch Seldaek/psr3 (PR #6628)Fabien Potencier2013-01-1013-17/+17
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This PR was merged into the master branch. Commits ------- 67d7423 Remove use of deprecated HttpKernel LoggerInterface dca4528 [HttpKernel] Extend psr/log's NullLogger class 1e5a890 [Monolog] Mark old non-PSR3 methods as deprecated 91a86f8 [HttpKernel][Monolog] Add PSR-3 support to the LoggerInterface Discussion ---------- [HttpKernel][MonologBridge] PSR-3 support This enables PSR-3 support and monolog 1.3+. The first commit is the main part. The rest deals with deprecation of short-hand methods (warn/err/crit/emerg) that are fully expanded in PSR-3 (warning/error/critical/emergency). The downside of deprecating them is that for bundles it's a bit harder to support older and newer versions. If that is too much of a hassle you can drop that for now and cherry pick the first commit. The upside is that it forces people to move towards PSR-3 compatible stuff, which means eventually we could completely drop the LoggerInterface from the framework. In any case I think the documentation should only mention the `Psr\Log\LoggerInterface` and people should start hinting against that. The change should be done in core as well I suppose. Anyway I wanted to throw this out there as it is to get feedback. --------------------------------------------------------------------------- by stof at 2013-01-09T09:15:15Z @Seldaek I also think you should change the typehint to use the PSR LoggerInterface in all classes using the logger --------------------------------------------------------------------------- by Seldaek at 2013-01-09T09:54:55Z OK updated according to all the feedback. I tested it in an app and it still seems to work so there shouldn't be any major issues. --------------------------------------------------------------------------- by Seldaek at 2013-01-09T09:59:55Z @fabpot if you merge please merge also the bundle PR, otherwise it won't be possible to update without conflict. --------------------------------------------------------------------------- by frosas at 2013-01-10T14:59:20Z I'm trying to understand why a `composer update` of a Symfony 2.1.* resulted in a fatal error. Shouldn't a stable version don't break like this? As @olaurendeau points, why Symfony depends 1.* instead of 1.2.*? Or why Monolog 1.3 breaks its public interface (EDIT: I'm not sure about it)? Or why isn't this PR being merged (into branch 2.1) at the same time Monolog 1.3 is released? Please, understand I'm not looking for who to blame, it's just I want to know if this situation is unexpected or if otherwise a `composer update` on a stable branch is not as innocent as it seems. --------------------------------------------------------------------------- by stof at 2013-01-10T15:06:51Z @frosas it cannot be merged into 2.1 as it is a BC break. The 2.1 branch has been updated to forbid Monolog 1.3 already --------------------------------------------------------------------------- by Seldaek at 2013-01-10T15:11:58Z @frosas you can blame me for releasing as 1.3.0 and not 2.0, but technically for monolog this isn't really a BC break, I just added an interface. The problem is due to the way it's used in symfony, it ended up as a fatal error. In any case the situation is now sorted out I think. --------------------------------------------------------------------------- by frosas at 2013-01-10T15:26:43Z @stof now I see this `>=1.0,<1.3-dev` change in the 2.1 branch. Now, shouldn't a new (2.1.7) version be released for all of us not in the dev minimum-stability? @Seldaek then do you see feasible to rely only in X.Y.* versions to avoid this kind of errors? --------------------------------------------------------------------------- by Seldaek at 2013-01-10T15:45:22Z @frosas relying on X.Y.* is painful because you always need to wait until someone updates the constraint to get the new version. Of course using ~1.3 like in this PR means if I fuck up and break BC people will update to it, but that's a less likely occurrence than the alternative I think, so I would rather not use X.Y.* --------------------------------------------------------------------------- by frosas at 2013-01-10T15:50:50Z @Seldaek you are right about this, but I was thinking more in changing it only for the stable versions. EDIT: I mean, how often do you need a new feature in a branch you only apply fixes to? --------------------------------------------------------------------------- by stof at 2013-01-10T15:57:32Z @frosas Monolog and Symfony have separate release cycles. Foorcing Symfony users to use an old version of Monolog until they update to a new version of Symfony whereas the newer Monolog is compatible is a bad idea. Thus, as Monolog keeps BC, it does not maintain bugfix releases for all older versions (just like Twig does too). So it would also forbid you to get the fixes done in newer Monolog versions. The incompatibility between Symfony 2.1 LoggerInterface and PSR-3 (whereas they expect exactly the same behavior and signature for methods with the same name) is unfortunate and is the reason why we get some issues here. --------------------------------------------------------------------------- by frosas at 2013-01-10T16:21:06Z @stof I appreciate you prefer to allow newer versions at the price of having to be constantly monitoring its changes to avoid breaks. Another similar but safer strategy would be to stick to X.Y.* versions and upgrade to X.Y+1.* once the new version integration is tested, but I understand this is discutible in projects as close to Symfony as Monolog. Returning to the issue, what do you say to release this 2.1.7 version? Or is it only me who is having issues here? --------------------------------------------------------------------------- by stof at 2013-01-10T16:26:20Z @frosas a minor release should not break BC when following smeantic versionning (Symfony warned about the fact it is not strictly followed for the first releases of 2.x). But as far as monolog is concerned, 1.3 is BC with 1.2. --------------------------------------------------------------------------- by Seldaek at 2013-01-10T16:49:55Z @frosas sorry I didn't get you still had the problem. I tagged a 2.1.7 of monologbundle which hopefully fixes your issue.
| * | | Remove use of deprecated HttpKernel LoggerInterfaceJordi Boggiano2013-01-0913-13/+13
| | | |
| * | | [Monolog] Mark old non-PSR3 methods as deprecatedJordi Boggiano2013-01-093-4/+4
| | | |
* | | | [Security] Removed `get/setExtraInformation`, added `get/set(Token|User)`Alexander2013-01-071-1/+1
| | | |
* | | | [Security] Fix InsufficientAuthenticationException constructor callsAlexander2013-01-071-1/+3
|/ / /
* | | Fixed most of the docblocks/unused namespacesFlorin Patan2012-12-196-1/+22
| | |
* | | Merge branch '2.1'Fabien Potencier2012-12-111-0/+1
|\ \ \ | |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * 2.1: fixed CS fixed CS [Security] fixed path info encoding (closes #6040, closes #5695) [HttpFoundation] added some tests for the previous merge and removed dead code (closes #6037) Improved Cache-Control header when no-cache is sent removed unneeded comment Fix to allow null values in labels array fix date in changelog removed the Travis icon (as this is not stable enough -- many false positive, closes #6186) Revert "merged branch gajdaw/finder_splfileinfo_fpassthu (PR #4751)" (closes #6224) Fixed a typo Fixed: HeaderBag::parseCacheControl() not parsing quoted zero correctly [Form] Fix const inside an anonymous function [Config] Loader::import must return imported data [DoctrineBridge] Fixed caching in DoctrineType when "choices" or "preferred_choices" is passed [Form] Fixed the default value of "format" in DateType to DateType::DEFAULT_FORMAT if "widget" is not "single_text" [HttpFoundation] fixed a small regression Conflicts: src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php
| * | Merge branch '2.0' into 2.1Fabien Potencier2012-12-111-0/+1
| |\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * 2.0: fixed CS removed the Travis icon (as this is not stable enough -- many false positive, closes #6186) [Config] Loader::import must return imported data [HttpFoundation] fixed a small regression Conflicts: README.md src/Symfony/Bridge/Twig/Extension/FormExtension.php src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/attributes.html.php src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_widget.html.php src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php src/Symfony/Component/Form/Form.php src/Symfony/Component/HttpFoundation/Request.php src/Symfony/Component/HttpFoundation/SessionStorage/PdoSessionStorage.php tests/Symfony/Tests/Bridge/Doctrine/Logger/DbalLoggerTest.php
| | * | fixed CSFabien Potencier2012-12-111-0/+1
| | | |
* | | | Merge branch '2.1'Fabien Potencier2012-11-291-6/+7
|\ \ \ \ | |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * 2.1: (29 commits) [DependencyInjection] fixed composer.json [Validator] Fix typos in validators.ru.xlf Edited some minor grammar and style errors in russian validation file Updated Bulgarian translation [Form] improve error message with a "hasser" hint for PropertyAccessDeniedException [Form] Updated checks for the ICU version from 4.5+ to 4.7+ due to test failures with ICU 4.6 [Form] simplified a test from previous merge Update src/Symfony/Component/Form/Extension/Core/Type/FileType.php fixed CS Xliff with other node than source or target are ignored small fix of #5984 when the container param is not set Filesystem Component mirror symlinked directory fix [Process][Tests] fixed chainedCommandsOutput tests fixed CS Use better default ports in urlRedirectAction Add tests for urlRedirectAction info about session namespace fix upgrade info about locale Update src/Symfony/Component/DomCrawler/Tests/FormTest.php Update src/Symfony/Component/DomCrawler/Form.php ...
| * | | Merge branch '2.0' into 2.1v2.1.4Fabien Potencier2012-11-291-6/+7
| |\ \ \ | | |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * 2.0: [DependencyInjection] fixed composer.json [Form] Updated checks for the ICU version from 4.5+ to 4.7+ due to test failures with ICU 4.6 fixed CS small fix of #5984 when the container param is not set fixed CS Use better default ports in urlRedirectAction Add tests for urlRedirectAction Update src/Symfony/Component/DomCrawler/Tests/FormTest.php Update src/Symfony/Component/DomCrawler/Form.php [Security] remove escape charters from username provided by Digest DigestAuthenticationListener [Security] added test extra for digest authentication fixed CS [Security] Fixed digest authentication [Security] Fixed digest authentication [SecurityBundle] Convert Http method to uppercase in the config Use Norm Data instead of Data Conflicts: src/Symfony/Bridge/Doctrine/Form/EventListener/MergeCollectionListener.php src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php src/Symfony/Component/DependencyInjection/composer.json
| | * | [Security] remove escape charters from username provided by Digest ↵v2.0.19Sebastiaan Stok2012-11-151-1/+1
| | | | | | | | | | | | | | | | DigestAuthenticationListener
| | * | [Security] Fixed digest authenticationVincent Simonin2012-11-151-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | Digest authentication fail if digest parameters contains `=` character or `, ` string. * Support escaped characters
| | * | [Security] Fixed digest authenticationVincent Simonin2012-11-151-5/+6
| | | | | | | | | | | | | | | | Digest authentication fail if digest parameters contains `=` character or `, ` string.
* | | | merged branch acasademont/tweak_userform_security_listener (PR #5824)Fabien Potencier2012-10-281-11/+8
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * | | | [Security] Tweak UsernamePasswordFormAuthenticationListenerAlbert Casademont2012-10-231-11/+8
| | | | | | | | | | | | | | | | | | | | | | | | | - 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
* | | | | Merge branch '2.1'Fabien Potencier2012-10-241-1/+0
|\ \ \ \ \ | |/ / / / |/| / / / | |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * 2.1: added missing use statment (closes #5825) Code cleanup [WebProfilerBundle] Fixed the use of nested macros Removed unused use statements. Nsdocblocks [ConfigDumpReference] avoid notice for variable nodes fixed fallback locale UniqueValidatorTest, Change message on assertions Documented removed _form_is_choice_group function Conflicts: src/Symfony/Bundle/FrameworkBundle/Command/ConfigDumpReferenceCommand.php src/Symfony/Bundle/WebProfilerBundle/Profiler/TemplateManager.php
| * | | Code cleanupv2.1.3Mario A. Alvarez Garcia2012-10-221-1/+0
| | | |
* | | | Merge branch '2.1'Fabien Potencier2012-10-021-1/+1
|\ \ \ \ | |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * 2.1: [2.1] Fix SessionHandlerInterface autoloading Remove executable bit from HttpKernel/DependencyInjection/ConfigurableExtension.php [2.0][http-foundation] Fix Response::getDate method [DoctrineBridge] Require class option for DoctrineType [HttpFoundation] fixed the path to the SensioHandlerInterface class in composer.json Support the new Microsoft URL Rewrite Module for IIS 7.0. @see http://framework.zend.com/issues/browse/ZF-4491 @see http://framework.zend.com/code/revision.php?repname=Zend+Framework&rev=24842 fixed undefined variable hasColorSupport does not take an argument Improve FilterResponseEvent docblocks Response ref
| * | | Merge branch '2.0' into 2.1Fabien Potencier2012-10-021-1/+1
| |\ \ \ | | |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * 2.0: [2.0][http-foundation] Fix Response::getDate method Support the new Microsoft URL Rewrite Module for IIS 7.0. @see http://framework.zend.com/issues/browse/ZF-4491 @see http://framework.zend.com/code/revision.php?repname=Zend+Framework&rev=24842 fixed undefined variable hasColorSupport does not take an argument Improve FilterResponseEvent docblocks Response ref Conflicts: tests/Symfony/Tests/Component/HttpFoundation/ResponseTest.php
| | * | fixed undefined variablev2.0.18James Michael DuPont2012-09-281-1/+1
| | | |
| | * | avoid fatal error on invalid sessionv2.0.17Kris Wallsmith2012-08-071-8/+15
| | | |
| | * | fixed CS (mainly method signatures)v2.0.16Fabien Potencier2012-07-093-3/+3
| | | |
* | | | Update ↵nervo2012-09-231-1/+1
|/ / / | | | | | | | | | | | | src/Symfony/Component/Security/Http/Firewall/UsernamePasswordFormAuthenticationListener.php Better consistency in request method checking (See L.58)
* | | merged 2.0Fabien Potencier2012-08-101-8/+15
| | |
* | | merged branch KaipiYann/Fix-DocBlock-attemptAuthentication (PR #4996)Fabien Potencier2012-07-211-1/+1
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- 134cc84 [Security] Fix DocBlock of attemptAuthentication Discussion ---------- [Security] Fix DocBlock of attemptAuthentication Bug fix: no Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: Todo: - License of the code: MIT Documentation PR: -
| * | | [Security] Fix DocBlock of attemptAuthenticationKaipi Yann2012-07-201-1/+1
| | | | | | | | | | | | | | | | Add Response as possible return type of the method because the method AbstractAuthenticationListener::handle() test if $returnValue is an instance of Response (line 148).
* | | | [Security] Extract default logout success handling logicAlexander2012-07-141-11/+5
| | | |
* | | | ensured that an exception is always converted to an error response (and that ↵Fabien Potencier2012-07-131-1/+0
| | | | | | | | | | | | | | | | we keep the HTTP status code and headers)
* | | | [Security] made sure that we always replace the security access denied ↵Fabien Potencier2012-07-131-4/+2
| | | | | | | | | | | | | | | | exception to an HTTP one
* | | | merged branch asm89/refactor-authentication-success-handling (PR #4599)Fabien Potencier2012-07-092-77/+16
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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).
| * | | | [Security] Fix regression after rebase. Target url should be firewall dependentAlexander2012-07-081-1/+1
| | | | |
| * | | | [Security] Add note to CHANGELOG about refactored authentication ↵Alexander2012-07-081-5/+15
| | | | | | | | | | | | | | | | | | | | 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
| * | | | [Security] Move default authentication failure handling strategy to seperate ↵Alexander2012-07-082-35/+5
| | | | | | | | | | | | | | | | | | | | class [Security] Update configuration for changes regarding default failure handler [Security] Fixes + add AbstractFactory test for failure handler
| * | | | [Security] Move default authentication success handling strategy to seperate ↵Alexander2012-07-082-46/+5
| | |_|/ | |/| | | | | | | | | | class [Security] Update configuration for changes regarding default success handler [Security] Fix + add AbstractFactory test
* | | | fixed CSFabien Potencier2012-07-093-3/+3
|/ / /
* | | [Security] Fix typo in docblockAdrien Brault2012-07-071-1/+1
|/ /
* | [Security] fixed automatic registration of the response listener when ↵Fabien Potencier2012-07-061-4/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | creating the listener This is not a problem with Symfony, but when using the component standalone (Silex for instance), the context listener might be instantiated even if the firewall does not need to be fired. In that case, the handle() method is not called, but the response listener is called, which means that en empty token is stored in the session. For Silex, it means that when authenticated, if you visit a 404 page, you would be disconnected automatically.
* | merged branch uwej711/security_target_path_master (PR #4409)Fabien Potencier2012-07-022-4/+6
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- 8ffaafa Make the session entry for the target url firewall dependent. Discussion ---------- [Security] Make the session entry for the target url firewall dependent. Bug fix: yes Feature addition: no Backwards compatibility break: yes Symfony2 tests pass: yes Fixes the following tickets: License of the code: MIT If there are two firewalls (eg. main and admin), calling an protected admin url will direct you to the login form of the admin. If I ignore this and go to the login form of the main firewall directly I will end up being redirected to the stored admin target url, which will lead me to the admin login form again. --------------------------------------------------------------------------- by travisbot at 2012-05-25T09:33:44Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1431566) (merged 8ffaafa8 into 45849ce3). --------------------------------------------------------------------------- by uwej711 at 2012-06-09T08:05:54Z Doesn't this make sense or did this slip through? Or is there something missing?
| * | Make the session entry for the target url firewall dependent.Uwe Jäger2012-05-252-4/+6
| | | | | | | | | | | | | | | | | | | | | If there are two firewalls (eg. main and admin), calling an protected admin url will direct you to the login form of the admin. If I ignore this and go to the login form of the main firewall directly I will end up being redirected to the stored admin target url. This is not what you usually want to happen.
* | | merged 2.0Fabien Potencier2012-06-201-1/+1
|\ \ \ | | |/ | |/|
| * | tweaked the previous commitFabien Potencier2012-06-161-1/+1
| | |
| * | [Security] Only redirect to urls called with http method GETUwe Jäger2012-06-141-1/+1
| | |
* | | [Security] Check post_only option and request methodHenrik Bjørnskov2012-06-091-0/+12
| |/ |/|
* | merged 2.0Fabien Potencier2012-05-211-2/+2
|\ \ | |/
| * merged branch kepten/ticket_1813 (PR #3551)v2.0.15Fabien Potencier2012-05-211-2/+2
| |\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- a450d00 [HttpFoundation] HTTP Basic authentication is broken with PHP as cgi/fastCGI under Apache Discussion ---------- [HttpFoundation] HTTP Basic authentication is broken with php-cgi under Apache Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: #1813 Todo: - In order to work, add this to the .htaccess: RewriteEngine on RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] RewriteCond %{REQUEST_FILENAME} !-f RewriteRule ^(.*)$ app.php [QSA,L] --------------------------------------------------------------------------- by stof at 2012-03-10T17:34:26Z you should also add a unit test for this --------------------------------------------------------------------------- by kepten at 2012-03-11T15:34:04Z Thanks for the feedback, I committed the changes. --------------------------------------------------------------------------- by stof at 2012-04-04T01:59:53Z @fabpot could you review it ? --------------------------------------------------------------------------- by fabpot at 2012-04-04T07:15:34Z My comments: * `ServerBag` represents what we have in the `$_SERVER` global variables. As such, the code should be moved to the `getHeaders()` method instead like the other tweaks we do for the HTTP headers. * A comment must be added explaining why this is needed and the configuration the user must have to make it work (then remove the Github URLs). * The code should only be executed when `PHP_AUTH_USER` is not available (to not have any overhead when not needed). --------------------------------------------------------------------------- by danielholmes at 2012-04-14T13:27:09Z A quick note on that .htaccess/apache configuration required, if adding to the Symfony SE htaccess file, then it will need to look like this: ``` <IfModule mod_rewrite.c> RewriteEngine On RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] RewriteCond %{REQUEST_FILENAME} !-f RewriteRule ^(.*)$ app.php [QSA,L] </IfModule> ``` NOTE: No **,L** in the Authorization Rewrite as in the original example - it prevents the front controller rewrite from happening --------------------------------------------------------------------------- by towards at 2012-04-20T16:12:49Z @kepten you were faster than me applying @fabpot's comments :) nevertheless part of the bug hunt day I also modified the ServerBag class and tested them on a productive LAMP hosting server using Apache and FastCGI --------------------------------------------------------------------------- by kepten at 2012-04-20T16:15:57Z ok, so is my PR is useless or should I still fix problems? --------------------------------------------------------------------------- by towards at 2012-04-20T16:20:26Z your PR is fine for sure and I don't want to interfere, just wanted to mention that part of the bug hunt day of Symfony I had a go at this PR as an "exercise" but just saw later on that you already fixed the problem, so you can ignore my pushes --------------------------------------------------------------------------- by vicb at 2012-04-20T16:20:36Z I have been working with @towards: your PR is useful, please implement his comments and squash your PR. --------------------------------------------------------------------------- by kepten at 2012-04-20T16:59:07Z never squashed before, is it okay now? :) --------------------------------------------------------------------------- by stof at 2012-04-20T17:21:07Z it is --------------------------------------------------------------------------- by vicb at 2012-05-20T19:57:51Z @fabpot this should be ready to be merged
| | * [HttpFoundation] HTTP Basic authentication is broken with PHP as cgi/fastCGI ↵kepten2012-04-201-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | under Apache Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: #1813 Todo: - In order to work, add this to the .htaccess: RewriteEngine on RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] RewriteCond %{REQUEST_FILENAME} !-f RewriteRule ^(.*)$ app.php [QSA,L]
| * | fixed CSFabien Potencier2012-05-182-11/+11
| | |
| * | fixed phpdoc @param alignmentv2.0.14Fabien Potencier2012-05-152-10/+10
| | |
| * | use SecurityContextInterface instead of SecurityContextv2.0.13Pierre Minnieur2012-03-092-5/+5
| |/
| * fixed CSv2.0.10Fabien Potencier2012-01-171-1/+1
| |
* | fixed CSFabien Potencier2012-05-182-12/+12
| |
* | fixed phpdoc @param alignmentFabien Potencier2012-05-152-11/+11
| |
* | [Security] fixed English grammar in exception messageDouglas Greenshields2012-05-011-1/+1
| |
* | fixed CS (missing or misplaced license blocks)Eriksen Costa2012-04-021-9/+9
| |
* | updated license blocksEriksen Costa2012-03-311-3/+3
| |
* | fixed CSFabien Potencier2012-03-231-2/+2
| |
* | [Security] Log file/line that triggered the AccessDeniedExceptionJordi Boggiano2012-03-231-2/+2
| |
* | [SecurityBundle] Allow switching to the user that is already impersonated ↵Andreas Hucks2012-03-151-6/+10
| | | | | | | | | | | | | | | | | | | | (fix #2554) Disabled exception when switching to the user that is already impersonated, exception is now only thrown when trying to switch to a new user. Added an Excption exception when switching fails because target user does not exist. Added funtional tests for switching users.
* | fixed docblockPierre Minnieur2012-03-071-1/+1
| |
* | use SecurityContextInterface instead of SecurityContextPierre Minnieur2012-03-071-2/+2
| |
* | use SecurityContextInterface instead of SecurityContextPierre Minnieur2012-03-061-2/+2
| |
* | merged branch jmikola/logout-csrf (PR #3007)Fabien Potencier2012-03-052-15/+43
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- 49a8654 [Security] Use LogoutException for invalid CSRF token in LogoutListener a96105e [SecurityBundle] Use assertCount() in tests 4837407 [SecurityBundle] Fix execution of functional tests with different names 66722b3 [SecurityBundle] Templating helpers to generate logout URL's with CSRF tokens aaaa040 [Security] Allow LogoutListener to validate CSRF tokens b1f545b [Security] Refactor LogoutListener constructor to take options c48c775 [SecurityBundle] Add functional test for form login with CSRF token Discussion ---------- [Security] Implement support for CSRF tokens in logout URL's ``` Bug fix: no Feature addition: yes Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: - Todo: - ``` [![Build Status](https://secure.travis-ci.org/jmikola/symfony.png?branch=logout-csrf)](http://travis-ci.org/jmikola/symfony) This derived from #3006 but properly targeting on the master branch. This exposes new configuration options to the logout listener to enable CSRF protection, as already exists for the form login listener. The individual commits and their extended messages should suffice for explaining the logical changes of the PR. In addition to changing LogoutListener, I also created a templating helper to generate logout URL's, which includes a CSRF token if necessary. This may or may not using routing, depending on how the listener is configured since both route names or hard-coded paths are valid options. Additionally, I added unit tests for LogoutListener and functional tests for both CSRF-enabled form logins and the new logout listener work. Kudo's to @henrikbjorn for taking the time to document CSRF validation for form login listeners (see [here](http://henrik.bjrnskov.dk/symfony2-cross-site-request-forgery/)). The [Logout CSRF Protection](http://www.yiiframework.com/wiki/190/logout-csrf-protection/) article on the Yii Framework wiki was also helpful in drafting this. --------------------------------------------------------------------------- by jmikola at 2011-12-31T07:50:31Z Odd that Travis CI reported a build failure for PHP 5.3.2, but both 5.3 and 5.4 passed: http://travis-ci.org/#!/jmikola/symfony/builds/463356 My local machine passes as well. --------------------------------------------------------------------------- by jmikola at 2012-02-06T20:05:30Z @schmittjoh: Please let me know your thoughts on the last commit. I think it would be overkill to add support for another handler service and/or error page just for logout exceptions. Perhaps as an alternative, we might just want to consider an invalid CSRF token on logout imply a false return value for `LogoutListener::requiresLogout()`. That would sacrifice the ability to handle the error separately (which a 403 response allows us), although we could still add logging (currently done in ExceptionListener). --------------------------------------------------------------------------- by jmikola at 2012-02-13T17:41:33Z @schmittjoh: ping --------------------------------------------------------------------------- by fabpot at 2012-02-14T23:36:22Z @jmikola: Instead of merging symfony/master, can you rebase? --------------------------------------------------------------------------- by jmikola at 2012-02-15T00:00:49Z Will do. --------------------------------------------------------------------------- by jmikola at 2012-02-15T00:05:48Z ``` [avocado: symfony] logout-csrf (+9/-216) $ git rebase master First, rewinding head to replay your work on top of it... Applying: [SecurityBundle] Add functional test for form login with CSRF token Applying: [Security] Refactor LogoutListener constructor to take options Applying: [Security] Allow LogoutListener to validate CSRF tokens Applying: [SecurityBundle] Templating helpers to generate logout URL's with CSRF tokens Applying: [SecurityBundle] Fix execution of functional tests with different names Applying: [SecurityBundle] Use assertCount() in tests Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Applying: [Security] Use LogoutException for invalid CSRF token in LogoutListener [avocado: symfony] logout-csrf (+7) $ git st # On branch logout-csrf # Your branch and 'origin/logout-csrf' have diverged, # and have 223 and 9 different commit(s) each, respectively. # nothing to commit (working directory clean) [avocado: symfony] logout-csrf (+7) $ ``` After rebasing, my merge commits disappeared. Is this normal? --------------------------------------------------------------------------- by stof at 2012-02-15T00:15:07Z Are you sure they disappeared ? Diverging from the remote branch is logical (you rewrote the history and so changed the commit id) but are you sure it does not have the commits on top of master ? Try ``git log master..logout-scrf`` If your commut are there, you simply need to force the push for the logout-csrf branch (take care to push only this branch during the force push to avoid messing all others as git won't warn you when asking to force) --------------------------------------------------------------------------- by stof at 2012-02-15T00:17:09Z ah sorry, you talked only about the merge commit. Yeah it is normal. When reapplying your commits on top of master, the merge commit are not kept as you are reapplying the changes linearly on top of the other branch (and deleting the merge commit was the reason why @fabpot asked you to rebase instead of merging btw) --------------------------------------------------------------------------- by jmikola at 2012-02-15T00:18:00Z The merge commits are not present in `git log master..logout-csrf`. Perhaps it used those merge commits when rebasing, as there were definitely conflicts resolved when I originally merged in symfony/master (@fabpot had made his own changes to LogoutListener). I'll force-push the changes to my PR brange. IIRC, GitHub is smart enough to preserve inline diff comments, provided they were made through the PR and not on the original commits. --------------------------------------------------------------------------- by jmikola at 2012-02-15T00:19:38Z That worked well. In the future, I think I'll stick to merging upstream in and then rebasing afterwards. Resolving conflicts is much easier during a merge than interactive rebase. --------------------------------------------------------------------------- by jmikola at 2012-02-23T18:46:13Z @fabpot @schmittjoh: Is there anything else I can do for this PR? I believe the exception was the only outstanding question (see: [this comment](https://github.com/symfony/symfony/pull/3007#issuecomment-3835716)).
| * | [Security] Use LogoutException for invalid CSRF token in LogoutListenerJeremy Mikola2012-02-142-2/+11
| | | | | | | | | | | | | | | | | | On the advice of @schmittjoh, this commit adds a LogoutException class for use by LogoutListener if the CSRF token is invalid. The handling in the Security component's ExceptionListener is modeled after AccessDeniedException, which gets wrapped in an AccessDeniedHttpException in the absence of handler service or error page (I didn't think it was appropriate to re-use those for LogoutException).
| * | [Security] Allow LogoutListener to validate CSRF tokensJeremy Mikola2012-02-141-10/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This adds several new options to the logout listener, modeled after the form_login listener: * csrf_parameter * intention * csrf_provider The "csrf_parameter" and "intention" have default values if omitted. By default, "csrf_provider" is empty and CSRF validation is disabled in LogoutListener (preserving BC). If a service ID is given for "csrf_provider", CSRF validation will be enabled. Invalid tokens will result in an InvalidCsrfTokenException being thrown before any logout handlers are invoked.
| * | [Security] Refactor LogoutListener constructor to take optionsJeremy Mikola2012-02-141-9/+9
| | | | | | | | | | | | This will facilitate adding additional options for CSRF protection. Additionally, a unit test for existing behavior was added.
* | | [Security] added AccessMapInterfaceKris Wallsmith2012-02-152-4/+4
|/ /
* | Authentication(Success|Failure)Handler can now return nullOlivier Dolbeau2012-01-241-2/+6
| |
* | [Security] made the logout path check configurableFabien Potencier2011-12-231-1/+18
| |
* | merged 2.0Fabien Potencier2011-12-181-1/+1
|\ \ | |/
| * fixed CSFabien Potencier2011-12-181-1/+1
| |
* | merged 2.0Fabien Potencier2011-12-131-1/+1
|\ \ | |/
| * [DoctrineBridge] fixed some CSFabien Potencier2011-12-132-1/+2
| |
* | added check for invalid user providersKris Wallsmith2011-12-051-0/+7
| |
* | merged branch jmikola/2.0-LoaderResolverInterface (PR #2785)Fabien Potencier2011-12-051-0/+4
|\ \ | |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- 7c1cbb9 [Config] Use LoaderResolverInterface for type-hinting 48b084e fixed typo 8ad94fb merged branch hhamon/doctrine_bridge_cs (PR #2775) 240796e [Bridge] [Doctrine] fixed coding conventions. 7cfc392 check for session before trying to authentication details 648fae7 merged branch proofek/domcrawlerform-radiodisabled (PR #2768) 3976b7a [DoctrineBridge] fixed CS 9a04783 merged branch beberlei/SecurityEntityRepositoryIdentifierFix (PR #2765) 3c83b89 [DoctrineBridge] Catch user-error when the identifier is not serialized with the User entity. 36c7d03 Fixed GH-2720 - Fix disabled atrribute handling for radio form elements Discussion ---------- [Config] Use LoaderResolverInterface for type-hinting ``` Bug fix: no Feature addition: no Backwards compatibility break: yes Symfony2 tests pass: yes Fixes the following tickets: - ``` I've listed this as a BC break because we're changing the argument type-hint, but I think it's unlikely to affect anyone.
| * check for session before trying to authentication detailsMatt Lehner2011-12-021-0/+4
| |
* | removed unused use statementsFabien Potencier2011-11-241-1/+0
| |
* | merged 2.0Fabien Potencier2011-11-244-4/+0
|\ \ | |/
| * removed unused use statementsFabien Potencier2011-11-244-4/+0
| |
* | merged branch snc/login-referer (PR #2518)Fabien Potencier2011-11-171-1/+1
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- f9a65ba Redirect to default_target_path if use_referer is true and the referer is the login_path. Discussion ---------- Login redirect Bug fix: no Feature addition: no Backwards compatibility break: yes Symfony2 tests pass: yes Redirect to default_target_path if use_referer is true and the referer is the login_path. --------------------------------------------------------------------------- by Seldaek at 2011/10/30 10:52:38 -0700 :+1: --------------------------------------------------------------------------- by stealth35 at 2011/10/30 11:04:16 -0700 @snc BC break ? --------------------------------------------------------------------------- by snc at 2011/10/30 12:11:39 -0700 Well I'm sure it is never intended by a developer to be redirected to the login page after logging in but it could be possible that the controller which displays the login form handles this case, so my change would break it.
| * | Redirect to default_target_path if use_referer is true and the referer is ↵Henrik Westphal2011-10-301-1/+1
| | | | | | | | | | | | the login_path.
* | | merged branch snc/issue-1798-sf21 (PR #2598)Fabien Potencier2011-11-101-2/+1
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- 4d80ebd Remove security token if user was deleted, is disabled or locked to prevent infinite redirect loops to the login path (fixes #1798). Discussion ---------- [2.1] Fix for issue 1798 Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: #1798 This is a simplified PR of #2528 for the master branch.
| * | | Remove security token if user was deleted, is disabled or locked to prevent ↵H. Westphal2011-11-101-0/+6
| | | | | | | | | | | | | | | | infinite redirect loops to the login path (fixes #1798).
* | | | Merge branch '2.0'Fabien Potencier2011-11-101-0/+8
|\ \ \ \ | |/ / / |/| | / | | |/ | |/| | | | | | | * 2.0: Added a class to the logs ol element to prevent hiding it when toggling an exception (fixes #2589). Remove only the security token instead of the session cookie. Clear session cookie if user was deleted, is disabled or locked to prevent infinite redirect loops to the login path (fixes #1798).
| * | Remove only the security token instead of the session cookie.H. Westphal2011-11-011-7/+6
| | |
| * | Clear session cookie if user was deleted, is disabled or locked to prevent ↵H. Westphal2011-10-311-1/+10
| | | | | | | | | | | | infinite redirect loops to the login path (fixes #1798).
* | | merged branch dpb587/patch-sectok (PR #2414)Fabien Potencier2011-11-071-7/+7
|\ \ \ | |_|/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- ab9caa0 [Security] Check for request's session before attempting writes. dabff0e [Security] Support removing tokens from a session. Discussion ---------- [Security] Support removing tokens from a session. Currently there is no way to remove a session's security token without invalidating the entire session and all its data (the ContextListener will only update the session if a token is non-null and non-anonymous). This patch fixes that. I consider this a bug and I found no tests to prove otherwise. Let me know if I'm mistaken. Originally mentioned at https://groups.google.com/d/topic/symfony-devs/ojLvh0WUbfo/discussion Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: - --------------------------------------------------------------------------- by ms937 at 2011/10/24 05:19:21 -0700 This change looks good to me. In fact I'm using similar patch in my app and it works as intended. Also, several other people requested this on the mailing list. Could someone from Symfony team merge this? Thanks.
| * | [Security] Check for request's session before attempting writes.Danny Berger2011-10-251-1/+3
| | |
| * | [Security] Support removing tokens from a session.Danny Berger2011-10-141-9/+7
| | |
* | | merged 2.0Fabien Potencier2011-10-293-6/+0
|\ \ \ | | |/ | |/|
| * | removed unused use statementsFabien Potencier2011-10-293-6/+0
| |/
* | Removed redundant "@return void"-sHelmer Aaviksoo2011-10-071-1/+0
|/
* merged branch helmer/target_path (PR #2228)Fabien Potencier2011-09-251-2/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- 022a9a7 [Security] Make saving target_path extendible Discussion ---------- [Security] Make saving target_path extendible The problem lies in how Security component handles ``target_path`` - the latest request URI is always stored. This can lead to problems in following scenarios: a) The response type of the request is not HTML (think JSON, XML ..) b) The URI matches a route that does not listen to HTTP GET I opened a [PR](https://github.com/symfony/symfony/pull/604) months ago, to partly solve scenario A, which did not make it. Now I am proposing a different solution - user can extend ``ExceptionListener`` and override the logic behind setting the ``target_path`` to match his precise needs. In my simplified scenario, I would be using: ``` protected function setTargetPath(Request $request) { if ($request->isXmlHttpRequest() || 'GET' !== $request->getMethod()) { return; } $request->getSession()->set('_security.target_path', $request->getUri()); } ``` @Seldaek, @schmittjoh, @lsmith77, thoughts? --------------------------------------------------------------------------- by Seldaek at 2011/09/21 02:37:02 -0700 Seems like a better solution for flexibility's sake. Would be quite awesome if you could add a cookbook entry to symfony/symfony-docs about this, otherwise I'm afraid we'll have to explain it over and over again :) --------------------------------------------------------------------------- by helmer at 2011/09/21 03:38:57 -0700 [Cookbook](https://github.com/helmer/symfony-docs/commit/b22c5e666edb2586840884e32f8209425125c30d) entry done. Perhaps though I rushed ahead .. --------------------------------------------------------------------------- by Seldaek at 2011/09/21 03:52:01 -0700 Thanks. You can already do a pull request against symfony-docs, just reference this pull request in it so it's not merged before this is merged.