summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* 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.
* | Merge branch '2.0'Fabien Potencier2011-11-141-1/+5
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | * 2.0: [HttpKernel] fixed Content-Length header when using ESI tags (closes #2623) [HttpFoundation] added an exception to MimeTypeGuesser::guess() when no guesser are available (closes #2636) [Security] fixed HttpUtils::checkRequestPath() to not catch all exceptions (closes #2637) [DoctrineBundle] added missing default parameters, needed to setup and use DBAL without ORM [Transation] Fix grammar. [TwigBundle] Fix trace to not show 'in at line' when file/line are empty.
| * | [Security] fixed HttpUtils::checkRequestPath() to not catch all exceptions ↵v2.0.6Fabien Potencier2011-11-141-1/+5
| | | | | | | | | | | | (closes #2637)
* | | 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).
| * | merged branch snc/issue-1798 (PR #2528)Fabien Potencier2011-11-101-0/+8
| |\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- f9befb6 Remove only the security token instead of the session cookie. 348bccb Clear session cookie if user was deleted, is disabled or locked to prevent infinite redirect loops to the login path (fixes #1798). Discussion ---------- Fix for issue 1798 Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Clear session cookie if user was deleted, is disabled or locked to prevent infinite redirect loops to the login path (fixes #1798). --------------------------------------------------------------------------- by snc at 2011/11/01 04:01:49 -0700 @stof I have changed the code so that it only removes the token... do we still need any hook support? --------------------------------------------------------------------------- by stof at 2011/11/01 04:07:17 -0700 well, the hook is for your own use case but it would be for 2.1 only anyway, not for 2.0 --------------------------------------------------------------------------- by snc at 2011/11/07 15:11:52 -0800 Now that #2414 is merged to 2.1, this could be simplified for the master branch...
| | * | 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 2.0Fabien Potencier2011-11-081-1/+1
|\ \ \ \ | |/ / /
| * | | [Security] Fix checkRequestPath doc; closes #2323Jeremy Mikola2011-11-071-1/+1
| | | |
* | | | 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
| | | | |
* | | | | [Security] made exceptions thrown by the user checker and the ↵Fabien Potencier2011-11-071-11/+19
| | | | | | | | | | | | | | | | | | | | checkAuthentication() method use the hideUserNotFoundExceptions flag
* | | | | merged 2.0Fabien Potencier2011-11-075-6/+6
|\ \ \ \ \ | | |/ / / | |/| | |
| * | | | merged branch igorw/a-user-interface (PR #2555)Fabien Potencier2011-11-035-6/+6
| |\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- ffa537c replace occurences of "an UserInteface" with "a UserInterface" Discussion ---------- replace occurences of "an UserInteface" with "a UserInterface"
| | * | | | replace occurences of "an UserInteface" with "a UserInterface"Igor Wiedler2011-11-035-6/+6
| | | | | |
| * | | | | bumped Symfony version to 2.0.6-DEVFabien Potencier2011-11-021-1/+1
| |/ / / /
| * | | | bumped Symfony version in composer.json files to 2.0.5v2.0.5Fabien Potencier2011-11-021-1/+1
| | | | |
* | | | | merged 2.0Fabien Potencier2011-11-011-1/+5
|\ \ \ \ \ | |/ / / / | | | | / | |_|_|/ |/| | |
| * | | updated composer.json files to contain information about autoloading and ↵Fabien Potencier2011-11-011-1/+5
| | |/ | |/| | | | | | | target dirs
* | | Merge branch '2.0'Fabien Potencier2011-10-291-1/+1
|\ \ \ | |/ / | | | | | | | | | * 2.0: fixed CS
| * | fixed CSFabien Potencier2011-10-291-1/+1
| | |
* | | merged 2.0Fabien Potencier2011-10-2917-26/+0
|\ \ \ | |/ /
| * | removed unused use statementsFabien Potencier2011-10-2917-26/+0
| |/
* | moved management of the locale from the Session class to the Request classFabien Potencier2011-10-081-5/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The locale management does not require sessions anymore. In the Symfony2 spirit, the locale should be part of your URLs. If this is the case (via the special _locale request attribute), Symfony will store it in the request (getLocale()). This feature is now also configurable/replaceable at will as everything is now managed by the new LocaleListener event listener. How to upgrade: The default locale configuration has been moved from session to the main configuration: Before: framework: session: default_locale: en After: framework: default_locale: en Whenever you want to get the current locale, call getLocale() on the request (was on the session before).
* | Removed redundant "@return void"-sHelmer Aaviksoo2011-10-0733-102/+0
| |
* | [Security] changed a RuntimeException to LogicException for consistencies ↵Fabien Potencier2011-10-031-1/+1
| | | | | | | | between the different Token classes (closes #2310)
* | updated composer.json for 2.1Fabien Potencier2011-09-291-8/+8
|/
* [composer] add composer.jsonv2.0.4Igor Wiedler2011-09-271-0/+31
|
* 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.
* Fixed the creation of the subrequestsChristophe Coevoet2011-09-181-0/+3
| | | | | | The subrequest must be created using an absolute path to keep the informations about the host and the base path. Closes #2168
* Added missing method to HTTP Digest entry pointStefano Sala2011-09-061-0/+10
|
* merged branch Abhoryo/patch-1 (PR #1956)v2.0.1Fabien Potencier2011-08-231-2/+11
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- e9d2a67 CS 3a64b08 Search in others user providers when a user is not found in the first user provider and throws the right exception. Discussion ---------- Chain user provider doesn't search in all user providers I commit these changes because Chain user provider doesn't search in all user providers. Example with the Acme/DemoBundle: // security.yml ... providers: chain_provider: providers: [in_memory, in_memory_extend] in_memory_extend: users: admin2: { password: adminpass2, roles: [ 'ROLE_ADMIN' ] } in_memory: users: user: { password: userpass, roles: [ 'ROLE_USER' ] } ... firewalls: ... secured_area: pattern: ^/demo/secured/ provider: chain_provider OR in_memory_extend ... We can see these logs : security.INFO: User "admin2" has been authenticated successfully [] [] security.DEBUG: Write SecurityContext in the session [] [] security.DEBUG: Read SecurityContext from the session [] [] security.DEBUG: Reloading user from user provider. [] [] security.WARNING: Username "admin2" could not be found. [] [] The new code search in others user providers when a user is not found in the first user provider and throws the right exception. --------------------------------------------------------------------------- by lsmith77 at 2011/08/14 12:20:04 -0700 I wonder if it should be a provider option to continue on a failed user lookup. I can see cases where you really dont want to iterate over all providers and others where you do. --------------------------------------------------------------------------- by Abhoryo at 2011/08/14 17:27:16 -0700 If someone need a provider like you describe, he can create one. Here we talk about a chain user provider. Doc : [using-multiple-user-providers](http://symfony.com/doc/current/book/security.html#using-multiple-user-providers) We can read in the doc: "The chain_provider will, in turn, try to load the user from both the in_memory and user_db providers." But its not the case right now.
| * CSAbhoryo2011-08-141-3/+4
| |
| * Search in others user providers when a user is not found in the first user ↵Abhoryo2011-08-141-2/+10
| | | | | | | | provider and throws the right exception.
* | increased visibility of httpUtils propertyJohannes Schmitt2011-08-181-1/+2
| |
* | merged branch aboks/acl_voter (PR #1954)Fabien Potencier2011-08-141-6/+5
|\ \ | |/ |/| | | | | | | | | | | | | | | | | | | | | | | Commits ------- 09c41d3 [Security] Fixed incorrect merge of two modifications (53f5c23c and 85199677) to AclVoter Discussion ---------- [Security] Fixed incorrect merge of two modifications to AclVoter It seems two modifications to `AclVoter` (53f5c23c and 85199677) have been merged incorrectly, leading to a method call on an object that is known to be `null` and a fatal error when running the tests
| * [Security] Fixed incorrect merge of two modifications (53f5c23c and ↵Arnout Boks2011-08-131-6/+5
| | | | | | | | 85199677) to AclVoter
* | Revert "expanded namespaces within phpdoc (special for PhpStorm)"Fabien Potencier2011-08-133-5/+5
|/ | | | This reverts commit 6e7439e73ade26bca01edc193c836b35aaa91ffe.
* expanded namespaces within phpdoc (special for PhpStorm)realmfoo2011-08-103-5/+5
|
* merge from masterrealmfoo2011-08-1094-310/+544
|\
| * Using the $status parameter instead of fixed value when creating a ↵v2.0.0Henrik Westphal2011-07-241-1/+1
| | | | | | | | RedirectResponse.
| * merged branch schmittjoh/httpUtilFixes (PR #1739)Fabien Potencier2011-07-222-15/+47
| |\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- eae6a77 fixed wrong case d0a175b fixes #1659 f300ede fixes several bugs a4f05ac added some tests Discussion ---------- Http util fixes Fixes several bugs in the http utils. Please don't add anymore features without sufficient tests. Especially for the Security\Http namespace, regressions are very likely otherwise. --------------------------------------------------------------------------- by fabpot at 2011/07/19 22:37:26 -0700 Tests do not pass for me: There were 2 errors: 1) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testLoginLogoutProcedure with data set #0 ('en') InvalidArgumentException: The current node list is empty. .../src/Symfony/Component/DomCrawler/Crawler.php:604 .../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:16 2) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testLoginLogoutProcedure with data set #1 ('de') InvalidArgumentException: The current node list is empty. .../src/Symfony/Component/DomCrawler/Crawler.php:604 .../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:16 -- There were 4 failures: 1) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testAccessRestrictedResource with data set #0 ('en') Failed asserting that two strings are equal. --- Expected +++ Actual @@ @@ -http://localhost/en/login +http://localhost/login .../src/Symfony/Bundle/Securitybundle/Tests/Functional/WebTestCase.php:22 .../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:38 2) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testAccessRestrictedResource with data set #1 ('de') Failed asserting that two strings are equal. --- Expected +++ Actual @@ @@ -http://localhost/de/login +http://localhost/login .../src/Symfony/Bundle/Securitybundle/Tests/Functional/WebTestCase.php:22 .../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:38 3) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testAccessRestrictedResourceWithForward with data set #0 ('en') HTTP/1.0 302 Found Cache-Control: no-cache Content-Length: 299 Content-Type: text/html; charset=UTF-8 Date: Wed, 20 Jul 2011 05:36:27 GMT Location: http://localhost/login Set-Cookie: PHPSESSID=11c9c6a7e7620e13bddef223a5ba46d9; path=/; domain= <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <meta http-equiv="refresh" content="1;url=http://localhost/login" /> </head> <body> Redirecting to <a href="http://localhost/login">http://localhost/login</a>. </body> </html> Failed asserting that <integer:0> matches expected <integer:1>. .../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:50 4) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testAccessRestrictedResourceWithForward with data set #1 ('de') HTTP/1.0 302 Found Cache-Control: no-cache Content-Length: 299 Content-Type: text/html; charset=UTF-8 Date: Wed, 20 Jul 2011 05:36:28 GMT Location: http://localhost/login Set-Cookie: PHPSESSID=2bbe63786a088471ade3717917f4ba4f; path=/; domain= <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <meta http-equiv="refresh" content="1;url=http://localhost/login" /> </head> <body> Redirecting to <a href="http://localhost/login">http://localhost/login</a>. </body> </html> Failed asserting that <integer:0> matches expected <integer:1>. .../src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php:50 --------------------------------------------------------------------------- by schmittjoh at 2011/07/19 23:47:29 -0700 I fixed a wrong case, but I couldn't reproduce the other errors (tested on Ubuntu). My guess is that the temporary directory on your machine couldn't be deleted for some reason, and the test runs with the configuration of some of the previous tests. --------------------------------------------------------------------------- by fabpot at 2011/07/20 00:28:41 -0700 That does not make any difference for me. For instance, in `LocalizedRoutesAsPathTest::testLoginLogoutProcedure()`, the first request to `'/'.$locale.'/login'` returns the following Response: <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> <meta http-equiv="refresh" content="1;url=http://localhost/login" /> </head> <body> Redirecting to <a href="http://localhost/login">http://localhost/login</a>. </body> </html> --------------------------------------------------------------------------- by schmittjoh at 2011/07/20 00:31:34 -0700 That's weird, did you make sure that the temporary directory does not exist? ``rm -Rf /tmp/StandardFormLogin/`` On Wed, Jul 20, 2011 at 9:28 AM, fabpot < reply@reply.github.com>wrote: > That does not make any difference for me. For instance, in > `LocalizedRoutesAsPathTest::testLoginLogoutProcedure()`, the first request > to `'/'.$locale.'/login'` returns the following Response: > > <html> > <head> > <meta http-equiv="Content-Type" content="text/html; > charset=utf-8" /> > <meta http-equiv="refresh" content="1;url= > http://localhost/login" /> > </head> > <body> > Redirecting to <a href="http://localhost/login"> > http://localhost/login</a>. > </body> > </html> > > -- > Reply to this email directly or view it on GitHub: > https://github.com/symfony/symfony/pull/1739#issuecomment-1613504 > --------------------------------------------------------------------------- by fabpot at 2011/07/20 00:33:40 -0700 Yes, I've just checked and the directory does not exist. --------------------------------------------------------------------------- by schmittjoh at 2011/07/20 00:39:55 -0700 Sorry, I can't reproduce it on Ubuntu and unless someone wants to sponsor me a Mac, there is not much I can do.
| | * fixes #1659Johannes Schmitt2011-07-191-0/+5
| | |
| | * fixes several bugsJohannes Schmitt2011-07-191-15/+42
| | |
| * | [Security] change a comparison to use a strict comparisonFabien Potencier2011-07-221-1/+1
| |/
| * DoctrineAclCache unserialize sets the acl to the wrong fieldGeoffrey Tran2011-07-151-1/+1
| | | | | | | | | | | | Upon unserialize of the acl, the acl is currently set to the id field which should be a string. Currently it passes the acl object into the id field which causes the following error. Warning: Illegal offset type in isset or empty in Symfony/Component/Security/Acl/Dbal/AclProvider.php line 404
| * merged branch schmittjoh/abstractAuthenticationListener (PR #1683)Fabien Potencier2011-07-131-8/+8
| |\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- 29e4063 [Security] changed order of checks to check for more specific things first Discussion ---------- [Security] changed order of checks
| | * [Security] changed order of checks to check for more specific things firstJohannes Schmitt2011-07-131-8/+8
| | |
| * | [Security] fixes #1329Johannes Schmitt2011-07-131-1/+1
| | |
| * | Revert "merged branch yktd26/master (PR #1673)"Fabien Potencier2011-07-131-2/+2
| | | | | | | | | | | | | | | This reverts commit af70ac8d777873c49347ac828a817a400006cbea, reversing changes made to c881379fe7b849c4262b6b47af2834c94393a130.
| * | merged branch yktd26/master (PR #1673)Fabien Potencier2011-07-131-2/+2
| |\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- 26ff05b fixes #1538 Discussion ---------- fixes #1538 Constructor of Symfony\Component\Security\Acl\Domain\RoleSecurityIdentity -------------------------------------------------------------------------------------------------------- currently it check if the argument is instance of Symfony\Component\Security\Core\Role\Role by ``if ($role instanceof Role)`` Maybe it should be changed to ``if ($role instanceof RoleInterface)`` Because if we use another Role class which implements RoleInterface it dosen't work when we check access, it will throw a *NoAceFoundException* when vote
| | * | fixes #1538yktd262011-07-131-2/+2
| | | |
| * | | [Security] Moved EntityUserProvider to Doctrine Bridgemarc.weistroff2011-07-131-85/+0
| |/ /
| * | Added a AccessDeniedHttpException to wrap the AccessDeniedException.Christophe Coevoet2011-07-111-5/+6
| | | | | | | | | | | | See #1631
| * | [Security] fixed redirection URLs when using {_locale} in the patternFabien Potencier2011-07-111-0/+13
| | |
| * | [Security] tweaked previous commitFabien Potencier2011-07-051-0/+1
| | |
| * | [Security] removed a hackFabien Potencier2011-07-051-9/+14
| | |
| * | [Security] reverted change from previous mergeFabien Potencier2011-07-041-0/+6
| | |
| * | merged branch Herzult/testSecurity (PR #1447)Fabien Potencier2011-07-043-10/+2
| |\ \ | | |/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- 164aea4 [Security] Add tests for the channel listener d51cbc0 [Security] Remove useless attribute in basic authentication listener & test it 91e6dc9 [Security] Add tests for the anonymous authentication listener 3c2affb [Security] Update access listener constructor's prototype and add tests 81afd77 [Security] Add tests for the firewall map aa6ae33 [Security] Remove useless attribute & var in firewall Discussion ---------- Test security --------------------------------------------------------------------------- by lsmith77 at 2011/06/29 13:41:07 -0700 @schmittjoh is probably the person to review this change ..
| | * [Security] Remove useless attribute in basic authentication listener & test itAntoine Hérault2011-06-261-6/+0
| | |
| | * [Security] Update access listener constructor's prototype and add testsAntoine Hérault2011-06-261-2/+2
| | |
| | * [Security] Remove useless attribute & var in firewallAntoine Hérault2011-06-251-2/+0
| | |
| * | [Security] Fix http retry authentication entry pointAntoine Hérault2011-06-251-1/+1
| | |
| * | [Security] Fix http form authentication entry pointAntoine Hérault2011-06-251-1/+1
| | |
| * | [Security] Fix http digest authentication entry pointAntoine Hérault2011-06-251-1/+1
| |/
| * [Security] Fix http basic authentication entry pointAntoine Hérault2011-06-251-1/+1
| |
| * fixed CSFabien Potencier2011-06-231-1/+1
| |
| * [Security] added an HttpUtils class to manage logic related to Requests and ↵Fabien Potencier2011-06-226-30/+135
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Responses This change removes the need for the {_locale} hack. Now, all paths in the Security component can be: * An absolute path (/login) * An absolute URL (http://symfony.com/login) * A route name (login) So, if you want to use a path that includes a global parameter (like _locale), use a route instead of a path.
| * Renamed core.* events to kernel.* and CoreEvents to KernelEventsJordi Boggiano2011-06-216-11/+8
| |
| * [Security] renamed UserProviderInterface::loadUser() to refreshUser()Fabien Potencier2011-06-165-7/+7
| |
| * made some tweaks to error levelsFabien Potencier2011-06-154-7/+7
| |
| * [Security] reverted some changes from previous mergeFabien Potencier2011-06-152-5/+2
| |
| * merged branch kaiwa/loglevel (PR #1073)Fabien Potencier2011-06-1511-19/+22
| |\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits ------- cdf4b6a Checked log levels a45d3ee Reverted last commit 529381b ControllerNotFound: Changed log level from info to error. Also moved throw exception code block up, to prevent the message from beeing logged multiple times. 7c29e88 Changed log level of "Matched route ..." message from info to debug dca09fd Changed log level of "Using Controller ..." message from info to debug Discussion ---------- Log levels Just wanted to ask if the log level INFO is still correct for these messages? As there are only four log levels left (DEBUG, INFO, WARNING, ERROR), DEBUG might be the more appropriate level for these messages now. Let me give an example: An application is logging user actions (maybe to database) in order to assure comprehensibility, e. g. "User %s deleted post %d", "User %s written a message to user %s". These are not warnings of course, so the only suitable log level is INFO. But they will be thrown together with these very common (at least two per request?) "Using controller..." and "Matched route..." messages when choosing INFO as log level. --------------------------------------------------------------------------- by Seldaek at 2011/05/24 07:13:18 -0700 Agreed, this stuff is framework debug information. --------------------------------------------------------------------------- by fabpot at 2011/05/24 08:53:24 -0700 Why do you want to change these two specific ones? The framework uses the INFO level at other places too. Is it a good idea to say that the framework only logs with DEBUG? --------------------------------------------------------------------------- by stof at 2011/05/24 09:12:53 -0700 Doctrine logs at the INFO level too and I think it is useful to keep it as INFO. Being able to see the queries without having all DEBUG messages of the event dispatcher and security components is useful IMO. --------------------------------------------------------------------------- by Seldaek at 2011/05/25 02:30:24 -0700 Yeah, that's true, maybe we just need to reintroduce (again, meh:) NOTICE between INFO and WARNING. @kaiwa Of course the other way could be that you just add your DB handler to the app logger stack. That could be done in a onCoreRequest listener or such, basically you'd have to call `->pushHandler($yourDBHandler)` on the `monolog.logger.app` service. That way your messages will flow to it, but it won't receive noise from the framework stuff since those log on monolog.logger.request and other log channels. --------------------------------------------------------------------------- by fabpot at 2011/05/25 02:48:26 -0700 @Seldaek: I don't think we need another level. We just need to come up with a standard rules about the usage of each level. Adapted from log4j: * ERROR: Other runtime errors or unexpected conditions. * WARN: Use of deprecated APIs, poor use of API, 'almost' errors, other runtime that are undesirable or unexpected, but not necessarily "wrong" (unable to write to the profiler DB, ). * INFO: Interesting runtime events (security infos like the fact the user is logged-in or not, SQL logs, ...). * DEBUG: Detailed information on the flow through the system (route match, security flow infos like the fact that a token was found or that remember-me cookie is found, ...). What do you think? --------------------------------------------------------------------------- by stloyd at 2011/05/25 02:53:38 -0700 +1 for this standard (also this PR can be merged then), but we should review code for other "wrong" log levels usage (if everyone accept this standard) --------------------------------------------------------------------------- by fabpot at 2011/05/25 02:55:07 -0700 I won't merge this PR before all occurrences of the logger calls have been reviewed carefully and changed to the right level. --------------------------------------------------------------------------- by kaiwa at 2011/05/25 02:58:44 -0700 @fabpot: Just noticed these two occurring for every request in my log file. You are right, there are other places where this changes must be applied if we will change the log level. @stof: Hmm, i see. It is not possible to set the logger separately for each bundle, is it? That maybe would solve the problem. If somebody is interested in seeing the queries, he could set the log handler level to DEBUG for doctrine bundle, but still use INFO for the framwork itself. Plus he could even define a different output file or a completely different handler. I'm not sure if something like that is possible already (?) or realizable at all... just came into my mind. --------------------------------------------------------------------------- by Seldaek at 2011/05/25 03:01:07 -0700 Just FYI, from Monolog\Logger (which has CRITICAL and ALERT): * Debug messages const DEBUG = 100; * Messages you usually don't want to see const INFO = 200; * Exceptional occurences that are not errors * This is typically the logging level you want to use const WARNING = 300; * Errors const ERROR = 400; * Critical conditions (component unavailable, etc.) const CRITICAL = 500; * Action must be taken immediately (entire service down) * Should trigger alert by sms, email, etc. const ALERT = 550; The values kind of match http error codes too, 4xx are expected errors that are not really important (404s etc) and 5xx are server errors that you'd better fix ASAP. I'm ok with the descriptions, but I think alert and critical should be included too. I'll probably update Monolog docblocks to match whatever ends up in the docs. --------------------------------------------------------------------------- by Seldaek at 2011/05/25 03:03:21 -0700 @kaiwa you can do a lot, but not from the default monolog configuration entry, I'm not sure if we can really make that fully configurable without having a giant config mess. Please refer to my [comment above](https://github.com/symfony/symfony/pull/1073#issuecomment-1234316) to see how you could solve it. Maybe @fabpot has an idea how to make this more usable though. --------------------------------------------------------------------------- by stof at 2011/05/25 03:19:43 -0700 @Seldaek the issue is that the different logging channels are only know in the compiler pass, not in the DI extension. So changing the level in the extension is really hard IMO. Thus, the handlers are shared between the different logging channels (needed to open the log file only once for instance, or to send a single mail instead of one per channel) and the level is handled in the handlers, not the logger. I'm +1 for the standard, by adding the distinction between 400 and 500 status calls using ERROR and CRITICAL (which is already the case in the code). @kaiwa do you have time to review the calls to the logger between DEBUG and INFO or do you prefer I do it ? For instance, the Security component currently logs all message at DEBUG level and some of them should be INFO. --------------------------------------------------------------------------- by kaiwa at 2011/05/25 04:31:04 -0700 @stof ok i'll do that --------------------------------------------------------------------------- by kaiwa at 2011/05/25 12:22:51 -0700 Need some help :) I came across `ControllerNameParser::handleControllerNotFoundException()` which leads to redundant log messages currently: >[2011-05-25 20:53:16] request.INFO: Unable to find controller "AppBaseBundle:Blog" - class "App\BaseBundle\Controller\BlogController" does not exist. >[2011-05-25 20:53:16] request.ERROR: InvalidArgumentException: Unable to find controller "AppBaseBundle:Blog" - class "App\BaseBundle\Controller\BlogController" does not exist. (uncaught exception) at /home/ruth/symfony3/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerNameParser.php line 87 Is it necessary to call `$this->logger->info($log);` if the InvalidArgumentException will be logged anyway? --------------------------------------------------------------------------- by stof at 2011/05/25 12:39:22 -0700 Well, the issue is that the ControllerNameParser logs messages and then uses them to throw an exception. I guess the logging call should be removed as it is redundant with the one of the ExceptionListener. @fabpot thoughts ? --------------------------------------------------------------------------- by kaiwa at 2011/05/27 11:39:25 -0700 I checked all debug, info and log calls. Sometimes it is hard to distinguish between the levels, so it would be great if someone reviews @cdf4b6a. @stof, maybe you want to take a look? --------------------------------------------------------------------------- by kaiwa at 2011/05/31 12:52:07 -0700 @stof, thanks for your comments. I added some replies above, please let me know your suggestions. --------------------------------------------------------------------------- by stof at 2011/05/31 14:04:22 -0700 @kaiwa As I said before, all the security logging calls should be DEBUG (most of them) or INFO (the one syaing that authentication succeeded for instance), but not WARN or ERROR as the exception don't go outside the firewall.
| | * Checked log levelskaiwa2011-05-2711-19/+22
| | |
| * | fixed CSFabien Potencier2011-06-143-4/+4
| | |
| * | fixed CSFabien Potencier2011-06-131-1/+1
| | |
| * | made logoutPath localizable as wellNed Schwartz2011-06-101-1/+3
| | |
| * | storing localized targetPath in a string as opposed to updating the attributeNed Schwartz2011-06-101-4/+3
| | |
| * | In the spirit of 882a8e3f09c602a6f0ed3b5bd20e8d4688331500 allow for ↵Ned Schwartz2011-06-101-0/+2
| | | | | | | | | | | | localized logout target url
| * | fixed CSFabien Potencier2011-06-0868-68/+68
| | |
| * | fixed CSFabien Potencier2011-06-081-0/+2
| | |
| * | [Security] fixed sub-requests creation (closes #1212)Fabien Potencier2011-06-083-3/+5
| | |
| * | Added the support of the locale in the login path and the check pathChristophe Coevoet2011-06-062-7/+10
| | |
| * | [Security/Http] removed irrelevant codeJohannes M. Schmitt2011-06-031-2/+0
| | |
| * | [Security/Core] added missing method to interfaceJohannes M. Schmitt2011-06-011-0/+6
| | |
| * | added missing license headersFabien Potencier2011-05-3113-0/+118
| | |
| * | merged origin/masterFabien Potencier2011-05-311-1/+1
| |\ \
| | * | [Security] fixed wrong function callFabien Potencier2011-05-301-1/+1
| | | |
| * | | merged origin/masterFabien Potencier2011-05-3010-45/+79
| |\ \ \ | | |/ /
| | * | [Security] fixes a possible bug when username is an integerJohannes M. Schmitt2011-05-301-1/+1
| | | |
| | * | [Security] fixes a regression in the AclVoterJohannes M. Schmitt2011-05-301-1/+1
| | | |
| | * | Merge branch 'master' of http://github.com/symfony/symfony into securityJohannes M. Schmitt2011-05-304-11/+16
| | |\ \
| | * | | Revert "revert exception message"Johannes Schmitt2011-05-281-1/+1
| | | | | | | | | | | | | | | | | | | | This reverts commit b637a3190d897e1b6076878ed69d2e4b3a58158d.
| | * | | Merge branch 'security' of github.com:schmittjoh/symfony into securityJohannes Schmitt2011-05-283-3/+3
| | |\ \ \
| | | * | | added a few finalsJohannes M. Schmitt2011-05-243-3/+3
| | | | | |
| | * | | | Merge branch 'master' of git://github.com/symfony/symfony into securityJohannes Schmitt2011-05-282-3/+4
| | |\ \ \ \ | | | |/ / / | | |/| | |
generated by cgit v1.1 at 2025-05-30 22:02:46 +0000