Throw http 401 rather than redirect to 'login'

[nover]

Hi!

I'm currently using SLIM 2 for creating a rest api which needs authentication and authorization - and decided to give your plugin a spin.

However, I find it suboptimal that the authentication performs a redirect to 'login' when the user is not logged in - it would be much nicer if it just returned a 401, to show the consumers of the API that authentication is required.

Perhaps it could be configurable in the Bootstrapper? Like, adding an configuration array as a 4'th optional argument

$acl = new \My\ApiAcl();
$authBootstrap = new Bootstrap($app, $adapter, $acl, array(
   'disableAuthRedirect'  => true
));
$authBootstrap->bootstrap();

Or similar :)

[jeremykendall]

However, I find it suboptimal that the authentication performs a redirect to 'login' when the user is not logged in - it would be much nicer if it just returned a 401

Great point, @nover. I'm positively swamped with work at this point. Do you feel comfortable putting together a pull request for this?

[nover]

Yeah, definitely - I looked around the code and it shouldn't be too hard. Do you want it configurable like suggested or always "401" ? :)

[jeremykendall]

Awesome! Thanks!

Do you want it configurable like suggested or always "401" ?

I'd love for it to be configurable. In retrospect, throwing a 401 might have been best, but making it configurable means this will be a backwards compatible change.

[jeremykendall]

FYI, if you haven't seen it yet, you might look at the halt() route helper:

The Slim application’s halt() method will immediately return an HTTP response with a given status code and body.

[nover]

Cool thanks for the hint - I'll take a look at it!

[jeremykendall]

@nover FYI, I'm working on this as a BC breaking change for the first beta version. It's gonna be really nice. Thanks for the feature request!

[nover]

@jeremykendall Great! Glad to hear that - and sorry that I never got around to looking at the issue as promised.

[jeremykendall]

@nover No worries! It only took me 1/4 years :-)

I'll close this once I've released 0.0.7-alpha.

[LorenzBischof]

How would I add the old behavior again?

[jeremykendall]

@Lorenzbi You don't. I decided I @nover's idea was so much better, and more appropriate, that I pulled out the old behavior entirely.

The new way to handle unauthorized requests is with the Slim custom error handler, like so:

// Handle the possible 403 the middleware can throw
$app->error(function (\Exception $e) use ($app) {
    if ($e instanceof HttpForbiddenException) {
        return $app->render('403.twig', array('e' => $e), 403);
    }
    if ($e instanceof HttpUnauthorizedException) {
        return $app->redirectTo('login');
    }
    // You should handle other exceptions here, not throw them
    throw $e;
});

[cGuille]

Hello!

I just used a slightly customized version of the custom error handler you provided. I wanted to give you some feedback about this choice.

I was disappointed to see that, since the Slim custom error handler is ignored in debug mode, I could not get login redirections in debug mode. I think this is bad: you get different behaviours depending on whether you are in debug mode or not. Moreover, the fact that the custom error handler is ignored in debug mode lets me think that its intended purpose is only to manage the rendering of errors, not really handling them with some kind of business logic.

I don't know Slim well, but I do not think it provides a clean way to handle the errors thrown by a middleware. So maybe unauthorized requests should not be considered as errors (and thus not result in the throwing of an uncaught exception in the auth middleware).

Could we imagine an optional handler that we would pass to slim auth, telling what to do with unauthorized requests? It would throw an HttpUnauthorizedException by default but this behaviour would be customizable.