silexphp / Silex

[DEPRECATED -- Use Symfony instead] The PHP micro-framework based on the Symfony Components
https://silex.symfony.com
MIT License
3.58k stars 718 forks source link

Redirect in before method #137

Closed jrivero closed 13 years ago

jrivero commented 13 years ago

Hi!

Is possible use the redirect method in before method? I used the next code idea for protect my routes, but can not get the redirect to work correctly.

$app->before(function() use ($app) {
    if ($app['request']->get('require_authentication')) {   
        if (null === $user = $app['session']->get('user')) {
            return $app->redirect('/login');
                }
    }
});

$app->get('/protected', function ($name) {
    return "Protected route";
})->value('require_authentication', true);
stof commented 13 years ago

In a general case, the return value of a listener is ignored by the Sf2 event dispatcher.

The silex.before and silex.after events are just about executing some code before and after the request in an easy way. If you want more power (needed in your case to set the RedirectResponse early), you need to use the kernel.* events dispatched in the kernel (kernel.request in your case). You can do it by registering a listener in the event dispatcher available through $app['dispatcher']

GromNaN commented 13 years ago

You can throw an exception in your silex.before and manage this exception with the error handler to redirect the user. The accurate exception class is Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException

jrivero commented 13 years ago

@gromNan nice idea!

jrivero commented 13 years ago

The code finally used:

use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;

$app->before(function() use ($app) {
    if ($app['request']->get('require_authentication')) {   
        if (null === $user = $app['session']->get('user')) {
            throw new AccessDeniedHttpException("require auth...");
                }
    }
});

$app->get('/protected', function ($name) {
    return "Protected route";
})->value('require_authentication', true);

$app->error(function (\Exception $e) use ($app) {
    if ($e instanceof AccessDeniedHttpException) {
        return $app->redirect('/login');
    }
});
kbsali commented 13 years ago

I've tried this and it does not seem to work : $app['session'] is NOT yet initialized in the before() filter.

igorw commented 13 years ago

@kbsali did you enable the SessionExtension?

kbsali commented 13 years ago

@igorw : yes, https://github.com/kbsali/silexXliffEditor/blob/master/src/bootstrap.php Is the behavior i'm seeing not normal then?

mgatto commented 13 years ago

I've noticed that I had to call $app['session']->save(); in $app->error() to get anything to persist in $_SESSION. However, the flash message I saved does not appear on the redirected page and is not present at all in $_SESSION there, either. In both cases, $_SESSION is only an empty array.

The expected behavior of $app['session'] and flash messages seemed to work in regular controller routes/actions.

mgatto commented 13 years ago

I faced this as well. I was setting the session lifetime in the config:

<?php
$app->register(new Silex\Extension\SessionExtension(), array(
    'session.storage.options' => array(
        'name' => '_SESS',
        'lifetime' => '3600',
        'path' => '/',
        'domain' => '',
        'secure' => false,
        'httponly' => true
    ),
));
?>

but 'lifetime' was removed from Symfony2: https://github.com/symfony/symfony-standard/commit/9dd067b6fe9c43fda8a740f9c43f5e379ebb3a3a

I removed 'lifetime' from all session extension configs in my controllers and it works now.

igorw commented 13 years ago

Using exceptions is actually kind of a hack here, similar to how the Security component in Symfony2 does it. Not sure if we want to / can support early responses in before(). It's basically a kernel.request which does not deal with the response yet.

So the exception may be an acceptable workaround.

stof commented 13 years ago

@igorw The kernel.request event can set a response, the silex.before event cannot.

igorw commented 13 years ago

Oh yes, in that case we may want to consider allowing setting a response in silex.before. Thoughts?

stof commented 13 years ago

well, he could also use the kernel.request event for this.

fabpot commented 13 years ago

I've "enhanced" the before and after filter. Like what we've done with the error() method, for which you have access to the Exception and the code, the before filter now have access to the Request and can return a Response and the after filter has access to the Response and can tweak it (e40002f08ec7e007f97751099a3e0dce8aeab04c).

Working on this issue, I've also noticed that the before and after filters were run also for sub-requests, which is probably not a good idea. So, they are now only run for the master request (d02fd0be4e636633e20dea319729d67f3564f273).

Last, I have also added the possibility to set the priority for all Silex events (b5c10054219e61ecef945ce88f638b148307ba45).

igorw commented 13 years ago

Nice.

jrivero commented 13 years ago

Thanks Fabien