Commit Graph

474 Commits

Author SHA1 Message Date
Alexander
5e6c06fc70 [Security] Remove hard dependency on $providerKey for default auth success handler 2012-07-12 12:03:58 +02:00
Christian Raue
20ae6931fa added missing dot 2012-07-12 03:11:01 +03:00
Fabien Potencier
7f9fd11fd0 merged branch asm89/refactor-authentication-success-handling (PR #4599)
Commits
-------

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

Discussion
----------

[Security] Refactor authentication success handling

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

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

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

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

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

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

+1 from me

@fabpot, what so you think?

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

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

Can you add a note in the CHANGELOG? Thanks.

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

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

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

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

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

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

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

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

👍 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](9e9519913d/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 8ffaafa867 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).
2012-07-09 15:29:00 +02:00
Fabien Potencier
d100ffaf76 fixed CS 2012-07-09 14:54:20 +02:00
Fabien Potencier
03d22b74ec fixed CS (mainly method signatures) 2012-07-09 14:43:50 +02:00
Alexander
bb138dadb3 [Security] Fix regression after rebase. Target url should be firewall dependent 2012-07-08 19:59:10 +02:00
Alexander
eb19f2c9e3 [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
2012-07-08 19:59:10 +02:00
Alexander
f9d5606f3f [Security] Update AuthenticationFailureHandlerInterface docblock. Never return null 2012-07-08 19:59:10 +02:00
Alexander
915704c071 [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
2012-07-08 19:59:10 +02:00
Alexander
c6aa392df7 [Security] Move default authentication success handling strategy to seperate class
[Security] Update configuration for changes regarding default success handler
[Security] Fix + add AbstractFactory test
2012-07-08 19:59:10 +02:00
Adrien Brault
0a4fd43e0e [Security] Fix typo in docblock 2012-07-07 20:42:10 +03:00
Fabien Potencier
1cb9a74057 [Security] fixed automatic registration of the response listener when 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.
2012-07-06 15:59:19 +02:00
Fabien Potencier
67a69ea357 [Security] updated CHANGELOG 2012-07-02 19:29:27 +02:00
Fabien Potencier
637aaacccb merged branch uwej711/security_target_path_master (PR #4409)
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?
2012-07-02 19:27:21 +02:00
Fabien Potencier
6dd342c351 fixed previous merge 2012-07-01 23:25:00 +02:00
Fabien Potencier
d2b5208a77 merged branch dlsniper/session-start-fix (PR #4541)
Commits
-------

f72ba0a Fixed detection of an active session

Discussion
----------

[WIP][HttpFoundation][Session] Fixed detection of an active session

Bug fix: yes
Feature addition: no
Backwards compatibility break: not sure
Symfony2 tests pass: no
Fixes the following tickets: #4529
Todo: Fix failing tests
License of the code: MIT
Documentation PR: ~

This fixes the problem when the session variable inside $request now has always data in it as it's now more powerful but this introduces the problem that the old way of detecting if a session is started or not doesn't work anymore.

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

by travisbot at 2012-06-09T21:53:17Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1578839) (merged 9ae13e12 into 6266b72d).

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

by drak at 2012-06-10T01:57:59Z

Sessions should be started implicitly. The SF auto_start config parameter controls the session listener to start the session.

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

by dlsniper at 2012-06-11T06:46:02Z

So this patch is correct then and I should continue the work on it?

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

by drak at 2012-06-11T07:51:39Z

@dlsniper - no it's not correct.  The session should not be auto-started like this, @fabpot and I recently discussed it.

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

by dlsniper at 2012-06-11T07:52:55Z

@Drak, ok I'll remove the patch for auto_start then but the fix for start would still stand, right?

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

by drak at 2012-06-12T18:40:35Z

@dlsniper - I have no objection to the rest of the PR except for the autostart stuff.  I've annotated for clarity :)

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

by travisbot at 2012-06-12T19:51:12Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1604158) (merged 3499980e into 37550d23).

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

by travisbot at 2012-06-12T19:52:00Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1604166) (merged dcc73071 into 37550d23).

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

by dlsniper at 2012-06-12T19:56:51Z

Seems Travis doesn't like the squashing of commits that I've did but the PR does pass the normal tests.
@drak is this good for merging now?

Thanks :)

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

by dlsniper at 2012-06-13T09:05:09Z

@fabpot this can be merged safely, I've just applied the patch on my production application and the patch is ok, it's just travis failing.

Thanks

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

by travisbot at 2012-06-13T09:23:46Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1608735) (merged 1a6eabd2 into 37550d23).

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

by travisbot at 2012-06-13T09:28:26Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1608758) (merged 4e3a93c8 into 37550d23).

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

by dlsniper at 2012-06-13T09:29:28Z

I've noticed that this is failing, I'll fix it later on today.

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

by travisbot at 2012-06-13T15:14:01Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1611541) (merged 5504c4b7 into 37550d23).

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

by drak at 2012-06-13T15:23:47Z

It's possible that other tests are failing not related to this PR. Run the tests on the current master, and try rebasing your branch to the current master also.

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

by dlsniper at 2012-06-13T15:44:22Z

I've just reminded why this is failing on builds, I can't do them locally because of this:
```
Installing dev dependencies
Your requirements could not be solved to an installable set of packages.

        Problems:
                - Problem caused by:
                        - Installation request for doctrine/orm [>= 2.2.0.0, < 2.4.0.0-dev]: Satisfiable by [doctrine/orm-2.2.2, doctrine/orm-2.2.1, doctrine/orm-2.2.0, doctrine/orm-2.2.x-dev, doctrine/orm-2.3.x-dev].
```

I'll try and install this somehow and see what's wrong with it.

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

by mvrhov at 2012-06-13T18:08:58Z

@dlsniper: as @stof said to me this should be resolved in latest versions of composer, but it seems that is not. The problem is that composer cannot figure out that you are on dev-master if you try to instal dev. dependencies on feature branch. Take a look at the .travis.yml file on how to do a proper dev vendors install.
cc @Seldaek

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

by dlsniper at 2012-06-13T23:08:53Z

@mvrhov Thanks for pointing this out.

@drak I still got two tests not passing but I'm not sure how to fix them as adding $session->start() will either fail with the message that the session has already been started, the headers_sent() call which returns true. Any help with them will be greatly appreciated. Thanks!

Here is what the HttpKernel tests are returning:
```
There were 2 failures:

1) Symfony\Component\HttpKernel\Tests\EventListener\LocaleListenerTest::testDefaultLocaleWithSession
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'es'
+'fr'

/var/www/symfony-orig/src/Symfony/Component/HttpKernel/Tests/EventListener/LocaleListenerTest.php:51

2) Symfony\Component\HttpKernel\Tests\EventListener\LocaleListenerTest::testLocaleFromRequestAttribute
Expectation failed for method name is equal to <string:set> when invoked 1 time(s).
Method was expected to be called 1 times, actually called 0 times.

FAILURES!
Tests: 263, Assertions: 1025, Failures: 2, Skipped: 10.
```

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

by travisbot at 2012-06-13T23:42:59Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1614883) (merged 1004b7c0 into c07e9163).

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

by travisbot at 2012-06-13T23:53:06Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1614897) (merged f72ba0a2 into c07e9163).

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

by dlsniper at 2012-06-16T20:14:41Z

@stof / @vicb Hi, do either of you think that you can either point me out to the right direction for fixing this either ping someone else for home help as @drak doesn't seem available for this and at the moment I'm pretty much clueless in what direction I should take this fix.

Thanks!

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

by dlsniper at 2012-06-19T14:16:29Z

ping @fabpot Can you please provide some input on this one as I'm a bit stuck and seems noone else is available.

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

by drak at 2012-06-20T10:24:43Z

fyi - I'll be able to look again in a few days

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

by fabpot at 2012-07-01T07:53:28Z

I'm +1 to add the `isStarted()` method, but -1 for the change of `Request::hasSession`.

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

by drak at 2012-07-01T09:06:15Z

@fabpot, I agree. `hasSession()` should not be changed, it's semantically incorrect to make it return effectively "hasActiveSession".
2012-07-01 23:20:45 +02:00
Fabien Potencier
42212394cb fixed phpdoc 2012-06-26 12:06:05 +02:00
Fabien Potencier
21e2f29eb1 [Security] simplified some code 2012-06-26 11:30:41 +02:00
Fabien Potencier
16a0af1262 [Security] changed the HttpUtils constructor to tak both a UrlGenerator and a UrlMatcher instead of a Router (to make it useable by Silex) 2012-06-26 11:18:35 +02:00
Fabien Potencier
55c6df995f merged 2.0 2012-06-20 21:33:33 +02:00
Fabien Potencier
15ae25b2e2 merged branch Maks3w/patch-4 (PR #4616)
Commits
-------

fa050b7 [Security] Change return value in DocBlock

Discussion
----------

[Security] Change return value in DocBlock

Change the return value for avoid confusions.

```php
if (!$user instanceof UserInterface) {
    throw new AuthenticationServiceException('retrieveUser() must return a UserInterface.');
}
```

[UserAuthenticationProvider.php#L67](https://github.com/symfony/symfony/tree/2.0/src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php#L67)

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

by travisbot at 2012-06-19T21:03:07Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1659148) (merged fa050b77 into e78a7bae).
2012-06-20 21:15:34 +02:00
Victor Berchet
680b83c6d3 [Security] Allow "0" as a password 2012-06-20 20:42:55 +02:00
Douglas Greenshields
d4b0a9caaa [Security] fixed exception message in EncoderFactory 2012-06-20 12:10:08 +01:00
Maks
fa050b77af [Security] Change return value in DocBlock
Change the return value for avoid confusions.
2012-06-19 23:27:38 +03:00
Fabien Potencier
faccd25c0f [Security] removed test that fail on PHP 5.3.3 2012-06-18 14:34:17 +02:00
Fabien Potencier
61a9345e6f [Security] fixed some unit tests for PHP 5.3.3 (see https://bugs.php.net/bug.php?id=53727) 2012-06-18 14:22:33 +02:00
Alexander
0be11f4d9d [Security] Fix retrieval of encoder when configured for concrete classes 2012-06-18 12:01:09 +02:00
Alexander
024cb91c5b [Security] Add failing testcases for EncoderFactory 2012-06-18 12:00:44 +02:00
Fabien Potencier
0b8b76bfef [Security] allowed class names to be passed as an argument to EncoderFactoryInterface::getEncoder() 2012-06-18 08:12:50 +02:00
Fabien Potencier
cd0aa3781b tweaked the previous commit 2012-06-16 18:27:53 +02:00
Uwe Jäger
3ce8227a9b [Security] Only redirect to urls called with http method GET 2012-06-14 13:45:10 +02:00
Florin Patan
f72ba0a27d Fixed detection of an active session 2012-06-14 01:48:01 +03:00
Fabien Potencier
77839690d9 changed getName() to name on all Reflection* object calls (fixes #4555, refs https://bugs.php.net/bug.php?id=61384) 2012-06-12 14:23:19 +02:00
Fabien Potencier
ba16a51d37 changed getName() to name on all Reflection* object calls (fixes #4555, refs https://bugs.php.net/bug.php?id=61384) 2012-06-12 13:59:42 +02:00
Fabien Potencier
0995b1f28b moved the UserPassword validator from the security bundle to the security component to make it reusable outside the full-stack framework 2012-06-10 21:58:31 +02:00
Henrik Bjørnskov
6a01d3dd54 [Security] Check post_only option and request method 2012-06-09 16:16:43 +02:00
Uwe Jäger
8ffaafa867 Make the session entry for the target url firewall dependent.
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.
2012-05-25 10:47:31 +02:00
Fabien Potencier
aa3e1a3b8c merged 2.0 2012-05-21 16:05:28 +02:00
Fabien Potencier
87bb3661fc merged branch kepten/ticket_1813 (PR #3551)
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
2012-05-21 11:09:36 +02:00
Christophe Coevoet
cdfb0b19d2 Changed composer constraint to allow Doctrine 2.3 too 2012-05-20 22:28:43 +02:00
Fabien Potencier
9b7aab5e94 merged 2.0 2012-05-20 18:16:37 +02:00
Fabien Potencier
d48d88a05a merged 2.0 2012-05-19 09:24:03 +02:00
Fabien Potencier
3bdf52a16a fixed CS 2012-05-18 19:42:42 +02:00
Fabien Potencier
e173d79e34 fixed CS 2012-05-18 19:37:58 +02:00
Fabien Potencier
41621e42e9 fixed phpdoc @param alignment 2012-05-15 22:19:31 +02:00
Fabien Potencier
ce9791246b fixed phpdoc @param alignment 2012-05-15 18:56:32 +02:00
Fabien Potencier
4c7261e3b1 merged 2.0 2012-05-11 18:14:47 +02:00
Fabien Potencier
3719c70870 updated minimum PHP version to 5.3.3
5.3.3 has some interesting fixes and this is the version used by
Redhat 6 and Debian 6
2012-05-07 10:29:11 +02:00
Fabien Potencier
8eea5c3d5f merged branch shieldo/fix_exception_message (PR #4173)
Commits
-------

69e0451 [Security] fixed English grammar in exception message

Discussion
----------

[Security] fixed English grammar in exception message
2012-05-01 18:03:29 +02:00
Fabien Potencier
5bed5f3c2c merged branch willdurand/fix-components (PR #4155)
Commits
-------

c195957 [Components] Tests/Autoloading fixes

Discussion
----------

Fix components

See #4141

----
This PR:

* configures each component to use composer to manage "dev" dependencies instead of env variables;
* adds phpunit configuration file on Filesystem component;
* fixes READMEs.

It's mergeable without any problems, but I would recommend to wait a fix in Composer in order to use `self.version` in `require`/`require-dev` sections.

Note: I kept `suggest` sections because it makes sense but this PR doesn't aim to provide useful explanations for each entry. It could be another PR, not that one.

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

by willdurand at 2012-04-30T20:43:13Z

@fabpot I reviewed each component, one by one. Now `phpunit` always works, even if tests are skipped. A simple `composer install --dev` allows to run the complete test suite. Each commit is well separated from the others. I guess, everything is ok now.

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

by Tobion at 2012-04-30T20:47:00Z

Please squash, as it makes no sense to have the same commit for each component.

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

by fabpot at 2012-05-01T14:26:11Z

Can you squash your commits before I merge? Thanks.

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

by willdurand at 2012-05-01T14:29:38Z

done

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

by fabpot at 2012-05-01T15:48:25Z

It does not seem that the commits are squashed.

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

by willdurand at 2012-05-01T15:54:08Z

done
2012-05-01 17:59:34 +02:00
William DURAND
c1959571ac [Components] Tests/Autoloading fixes
* Switched to Composer to manage "dev" dependencies
* Fixed READMEs
* Excluded vendor in phpunit.xml.dist files
* Fixed message in bootstrap.php files
* Added autoloader for the component itself
2012-05-01 17:51:41 +02:00
Douglas Greenshields
69e0451143 [Security] fixed English grammar in exception message 2012-05-01 14:45:52 +01:00
Fabien Potencier
26f933e7bd fixed CS 2012-05-01 15:23:48 +02:00
Hugo Hamon
9ac8d43dd1 [Security] fixed phpdoc. 2012-05-01 13:15:31 +01:00
Hugo Hamon
a3ecea3ed3 [Security] added some missing phpdoc for AbstractToken::setUser() and UsernamePasswordToken::__construct() methods. 2012-05-01 13:13:14 +01:00
Fabien Potencier
9fbf8555f0 Revert "merged branch Seldaek/master (PR #4133)"
This reverts commit 00e7a94a8c, reversing
changes made to a01dec00f4.
2012-04-27 19:55:40 +02:00
Jordi Boggiano
00c4267726 Update branch aliases 2012-04-27 12:47:50 +02:00
Fabien Potencier
6c0c38c718 [Security] added CHANGELOG 2012-04-26 22:30:56 +02:00
kepten
a450d002f2 [HttpFoundation] HTTP Basic authentication is broken with PHP as cgi/fastCGI 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]
2012-04-20 18:39:29 +02:00
Jordan Alliot
6483d88f69 [Security][ACL] Fixed ObjectIdentity::fromDomainObject and UserSecurityIdentity::from(Account|Token) when working with proxies
Backported ClassUtils class from Doctrine Common 2.2
Fixes #2611, #2056, #2048, #2035
2012-04-12 00:40:59 +02:00
Eriksen Costa
2cac50d8a9 fixed CS (missing or misplaced license blocks) 2012-04-02 00:52:14 -03:00
Eriksen Costa
013f998bb8 updated license blocks 2012-03-31 18:00:32 -03:00
Victor Berchet
234ce4df9e [PhpUnit] Fix the path to the boostrap files in the components 2012-03-30 13:49:28 +02:00
Fabien Potencier
fea6b79acd moved component and bridge unit tests to the src/ directory
This is the first step to make each Symfony Component and Bridge self-contained.
2012-03-29 08:37:22 +02:00
Fabien Potencier
def4323dc8 fixed CS 2012-03-23 12:47:42 +01:00
Jordi Boggiano
da0bd7d0b1 [Security] Log file/line that triggered the AccessDeniedException 2012-03-23 11:42:13 +01:00
Andreas Hucks
a98d554472 [SecurityBundle] Allow switching to the user that is already impersonated (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.
2012-03-15 01:50:14 -07:00
Fabien Potencier
673bbb8a8e fixed CS 2012-03-11 18:00:25 +01:00
Victor Berchet
6d27aecb02 [SecurityBundle] Improve the init:acl command 2012-03-11 15:50:58 +01:00
Johannes M. Schmitt
e8094589f0 [Security]replaced acl:init command with postGenerateSchema listener 2012-03-11 14:52:28 +01:00
Fabien Potencier
1a7ba03dbc merged branch Engerim/patch-1 (PR #3538)
Commits
-------

3fd9003 Update src/Symfony/Component/Security/Acl/Permission/MaskBuilder.php

Discussion
----------

Changed return Tags from PermissionBuilder to MaskBuilder
2012-03-11 09:19:24 +01:00
Christophe Coevoet
919eee4c4b [Security] Regenerated the ACL SQL schema with the latest Doctrine version 2012-03-11 03:29:45 +01:00
Alexander Miehe
3fd9003301 Update src/Symfony/Component/Security/Acl/Permission/MaskBuilder.php 2012-03-09 10:17:07 +01:00
Pierre Minnieur
0c9b2d47b0 use SecurityContextInterface instead of SecurityContext 2012-03-09 10:08:43 +01:00
Pierre Minnieur
bfb5547fa0 fixed docblock 2012-03-07 12:59:25 +01:00
Pierre Minnieur
bf75212fbc use SecurityContextInterface instead of SecurityContext 2012-03-07 12:58:57 +01:00
Pierre Minnieur
498b4b61b5 use SecurityContextInterface instead of SecurityContext 2012-03-06 17:40:30 +01:00
Fabien Potencier
294b57e1b1 merged branch jmikola/logout-csrf (PR #3007)
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)).
2012-03-05 16:12:24 +01:00
Fabien Potencier
ddeac9a9ea [Security] added support for the remember_me parameter in the query (closes #3460) 2012-03-02 21:45:57 +01:00
Jordi Boggiano
bafcaafbe6 Removed version field 2012-02-27 09:59:20 +01:00
Jordi Boggiano
f9d9dc7ce9 Add branch-alias for composer 2012-02-25 03:26:20 +01:00
Fabien Potencier
f373085928 merged 2.0 2012-02-22 18:59:56 +01:00
marc.weistroff
89868f7901 Updated LICENSE files copyright 2012-02-22 10:10:37 +01:00
Kris Wallsmith
1e8236cfb3 [Security] added AccessMapInterface 2012-02-15 14:14:40 -08:00
Jeremy Mikola
49a8654cb8 [Security] Use LogoutException for invalid CSRF token in LogoutListener
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).
2012-02-14 19:03:52 -05:00
Jeremy Mikola
aaaa04003d [Security] Allow LogoutListener to validate CSRF tokens
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.
2012-02-14 19:03:51 -05:00
Jeremy Mikola
b1f545b677 [Security] Refactor LogoutListener constructor to take options
This will facilitate adding additional options for CSRF protection. Additionally, a unit test for existing behavior was added.
2012-02-14 19:03:51 -05:00
Toni Uebernickel
dbd3a1bcc0 allow unsetting parentAcl on MutableAclInterface 2012-01-30 14:16:44 +01:00
Olivier Dolbeau
ed9c34822b Authentication(Success|Failure)Handler can now return null 2012-01-24 17:57:22 +01:00
Fabien Potencier
c290d829a7 merged branch gustavopiltcher/master (PR #2858)
Commits
-------

4a797df Oracle issues
81d73bb Oracle issues
2316b21 Oracle issues
315bfc4 just update
b20b15b Oracle 10 issues

Discussion
----------

Oracle issues

updated with some adjustments required by stof

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

by fabpot at 2011-12-13T07:24:12Z

@schmittjoh: Can you have a look at this PR?

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

by fabpot at 2011-12-24T08:19:37Z

Can you squash your commit before I merge your PR? Thanks.
2012-01-22 10:45:17 +01:00
Fabien Potencier
9c3c53a5c1 merged 2.0 2012-01-17 11:23:18 +01:00
Fabien Potencier
51ecb3c07b fixed CS 2012-01-17 10:56:02 +01:00
Toni Uebernickel
360f5745cb fix expected return type of MutableAclProvider 2012-01-16 15:24:43 +01:00
Gordon Franke
f3c413d156 add missing class var; add phpdocs 2012-01-16 12:15:36 +01:00
Dariusz Górecki
d7ffeb5844 Add some more tests, and enforce boolean return value of interface implementations. 2012-01-10 21:55:05 +01:00
Dariusz Górecki
9d3a49f065 When method name is hasUserChanged the return boolean should be true
(to match question semantics) and false when user has not changed,
this commits inverts return statements.
2012-01-10 21:55:05 +01:00
Dariusz Górecki
c57b528cca Add note about AdvancedUserInterface. 2012-01-10 21:55:05 +01:00
Dariusz Górecki
3682f62a07 Refactor isUserChanged to hasUserChanged 2012-01-10 21:55:05 +01:00
Dariusz Górecki
56db4a1d26 Change names to Equatable 2012-01-10 21:55:04 +01:00
Dariusz Górecki
680b1086e9 Suggested fixes ;) 2012-01-10 21:55:04 +01:00