vlucas / bulletphp

A resource-oriented micro PHP framework
http://bulletphp.com
BSD 3-Clause "New" or "Revised" License
418 stars 50 forks source link

Short-circuit path handling? #65

Open acicali opened 9 years ago

acicali commented 9 years ago

I'm attempting to add authentication to my bullet-based web service. I'd like to be able to handle ALL requests and load a template for users that fail authentication, but this is proving more difficult than I expected.

I authenticate all requests (even those that might otherwise result in a 404). Also, I authenticate via headers - the request must include valid ('X-Session-Username' AND 'X-Session-Password') OR ('X-Session-Token'). Lastly, I don't do any redirection - all URIs should return a 403 without proper authentication.

The only way I can see to handle this "globally" is to use the 'before' event, but that won't stop the actual route from running.

Am I missing something?

acicali commented 9 years ago

Just to elaborate a little, I see issue https://github.com/vlucas/bulletphp/issues/36 which seems to tackle the same issue but with a redirect. I can almost achieve the desired results with the code below, but it's not very elegant and definitely not DRY.

//----------------------------------------------------
// Create bullet instance, providing the views path
//----------------------------------------------------
$app = new Bullet\App([
    'template' => [
        'path' => APPPATH.'views/'
    ]
]);

//----------------------------------------------------
// Empty path serves the documentation
//----------------------------------------------------
$app->path('/',
    function($request) use($app){
        return $app->template('documentation')
            ->set(Documentation::create());
    }
);

//----------------------------------------------------
// All other paths require authentication
//----------------------------------------------------
if(! AUTHENTICATED){
    $app->on('before',
        function($request, $response) use($app){
            return $app->response(403, 
                $app->template('error')->set([
                    'status'    => '403',
                    'message'   => 'Forbidden'
                ])->content()
            )->send();
        }
    );

    echo $app->run( // <-- "SHORT CIRCUIT" the routing.
        new Bullet\Request()
    );

    exit;
}

//----------------------------------------------------
// Non-empty path checks for a matching controller
//----------------------------------------------------
$app->param('Valid::controller',
    function($request, $controller) use($app){
        require(CONTROLLERS.$controller.'.php');
    }
);

//----------------------------------------------------
// Otherwise load the 404 template
//----------------------------------------------------
$app->on(404,
    function($request, $response) use($app){
        return $response->content(
            $app->template('error')->set([
                'status'    => 404,
                'message'   => "I tried really hard to find what you're looking for, but it just isn't there."
            ])->content()
        );
    }
);

//----------------------------------------------------
// Run the above routing, return the response
//----------------------------------------------------
echo $app->run(
    new Bullet\Request()
);

This approach still sends a 404 status and appends "Not Found" to the response body, since there's no actual matching route. Certainly an authentication failure could bypass Bullet and manually load a view at the top of the app, but that defeats the purpose :)

vlucas commented 9 years ago

Hmmm. There does seem to be room for a new feature here. Do you have any recommendations on syntax or exactly what you need? I'd definitely like to build something into Bullet that makes this easier.

vlucas commented 9 years ago

Maybe there just needs to be a way to tell Bullet the request is "done", and to send what's in the current response object?

acicali commented 9 years ago

I was just perusing Bullet/App to see how such a feature would be implemented, but I'm not sure if I have enough experience to even make a good suggestion here :)

Issue https://github.com/vlucas/bulletphp/issues/36 mentions a done() method, although I'm not sure that clearly describes its purpose. Also, would this method still call the 'after' and 'beforeResponseHandler' events? Finally, would it ignore the HTTP method of the request?

Potentially, a new response method end() might fit the API.

$app->on('before',
    function($request, $response) use($app){
        return $app->response(403, 
            $app->template('error')->set([
                'status'    => '403',
                'message'   => "You've attempted to access a resource without permission."
            ])->content()
        )->end();
    }
);

It would have to somehow cause subsequent calls to path() and param() to be ignored. Does that make sense?

vlucas commented 9 years ago

Yes - it makes perfect sense - basically a way to shortcut out of the route parsing step from inside a route and just return the current response object as-is.

acicali commented 9 years ago

It seems the existing $response->send() method should probably do this. Would that cause too much breakage?

acicali commented 9 years ago

I'm not ultra educated on the HTTP protocol. Reading the spec, I see that clients MUST be prepared to handle one or more 1xx status codes, but isn't that the only status range where that case applies?

Maybe $response->send() should look something like this?

public function send()
{
    if(strpos($this->_status, 1) === false)
    {
        die($this);
    }

    echo $this;
}

This would allow the server to send multiple statuses (in the case of 1xx), but finalize the response otherwise.

For my use case, changing echo to die() achieves the desired behavior.

acicali commented 9 years ago

Correction,

Changing echo to die() only achieves the desired behavior when send() is followed by:

echo $app->run(null);
exit;
vlucas commented 9 years ago

I think basically you just have to do:

$response->send();
exit;

And you can achieve what you want. It's not terribly elegant, but it works.