[NFR] ACL on router about phalcon

[sergeyklay]

Branch with firewall https://github.com/phalcon/cphalcon/tree/4.0.x-with-firewall

[Jurigag]

You can implement this behaviour on dispatcher level. You need some key, from what get it ? Controller - resource name ? And action - access name ?

[daison12006013]

@Jurigag, yeah that one is existing now, however the implementation seems kind confusing for others, even for me it is confusing to implement.

I could also create a class to do the same thing, to extends the same router, however this thing I think is doable inside the cphalcon itself.

The process is simple compared on the current acl, we're more focused on each routing, either there is 'allowed' or 'denied' keys to determine the roles.

The routing already holds the roles, and upon calling the controller@action, the acl is already registered, what we only need now is to know if the current user is allowed to access the current action.

It is more simpler to implement ACL, I guess, but it is up to the developers which one they want to use.

I am not saying they should remove the existing one, but to wrap a route based acl as well.

---

Thanks @sergeyklay.

[SidRoberts]

FWIW, I know it's not exactly what you're looking for but https://github.com/SidRoberts/phalcon-authmiddleware solves a similar problem.

[Jurigag]

But im just not sure it should be running in controller/router. Dispatcher is imho one good place for it.

So you just wanna router methods to have allowed key, and then it will execute allow() methods for each value ? But with which resource name, and access name ? I can look into it, i already implemented some nice things for ACL in phalcon 2.1.x

[sergeyklay]

@SidRoberts Wow! How about PR to the Awesome Phalcon?

[SidRoberts]

@sergeyklay See phalcon/awesome-phalcon#53. I'll add more to the documentation tonight to include ACL examples.

[Jurigag]

@daison12006013 also whats is current user ? Phalcon dont storing "user" anywhere.

[daison12006013]

@Jurigag, we are not storing anything mate, we are just calling the di 'acl' and injecting the roles on it...

while we need to iterate manually the user roles from the database, and inject those roles inside the acl, no need to provide a user table thing...

[daison12006013]

@SidRoberts that package is so cool for authentication, anyways I have an idea for an alternative way for annotations and events now 😄

[valVk]

Here is my implementation a bit complicated I think.

<?php

namespace APP\Acl;

class Acl {

    private $_reader,
            $_di,
            $_controller,
            $_method,
            $_adminRoleName = 'ROLE_ADMIN',
            $_public = array('index', 'session');


    /**
     * @param \Phalcon\Annotations\Adapter $reader
     * @param $di
     */
    public function __construct(\Phalcon\Annotations\Adapter $reader, $di)
    {
        $this->_reader = $reader;
        //TODO-me create cache clear optional
        xcache_clear_cache(XC_TYPE_VAR, -1);
        xcache_clear_cache(XC_TYPE_PHP, -1);
        $this->_di = $di;
//      $this->_controller = $di->get('dispatcher')->getActiveController();
        $this->_controller = get_class($di->get('dispatcher')->getActiveController());
        $this->_method = $di->get('dispatcher')->getActiveMethod();
    }

    /**
     * @param array $roles
     * @return bool
     */
    public function isAllowed($roles)
    {
        $controllerAllowed = $this->checkController($roles);
        $methodAllowed = $this->checkMethod($roles);

        if (in_array($this->_adminRoleName, $roles)) {
            return true;
        }

        if (true === $controllerAllowed && true === $methodAllowed) {
            return true;
        }

        return false;
    }

    /**
     * @param array $resource
     * @return bool
     */
    public function isPublic($resource)
    {
        return in_array($resource, $this->_public) ? true : false;
    }

    /**
     * @param string $name
     */
    public function setAdminRoleName($name)
    {
        $this->_adminRoleName = $name;
    }

    /**
     * @param array $resources
     */
    public function setPublicResources($resources)
    {
        $this->_public = $resources;
    }

    /**
     * @param array $roles
     * @return bool
     */

    private function checkMethod($roles)
    {
        $methodAllowed = $this->getAccessMethod();
        if (!empty($methodAllowed)) {
            $intersection = array_intersect($roles, $methodAllowed);
            return !empty($intersection) ? true : false;
        }

        return true;
    }

    /**
     * @param array $roles
     * @return bool
     */
    private function checkController($roles)
    {
        $controllerAllowed = $this->getAccessController();
        if (!empty($controllerAllowed)) {
            $intersection = array_intersect($roles, $controllerAllowed);
            return !empty($intersection) ? true : false;
        }

        return true;
    }

    /**
     * Checks if method allowed for access
     * @return bool
     */
    private function getAccessMethod()
    {
        $annotations = $this->_reader->getMethod($this->_controller, $this->_method);

        if (!empty($annotations)) {
            if ($annotations->has('Allow')) {
                $annotation = $annotations->get('Allow');
                return $annotation->getArguments();
            }
        }

        return false;
    }

    /**
     * Checks if controller allowed for access
     * @return bool
     */
    private function getAccessController()
    {
        $reflector = $this->_reader->get($this->_controller);
        $annotations = $reflector->getClassAnnotations();
        if (!empty($annotations)) {
            foreach ($annotations as $annotation) {
                if ($annotations->has('Allow')) {
                    return $annotation->getArguments();
                }
            }
        }

        return false;
    }
}

<?php

namespace APP\Acl;

use \Phalcon\Events\Event;
use \Phalcon\Mvc\Dispatcher;
use Phalcon\Mvc\User\Component;

class Gate extends Component {

    /**
     * Execute before the router so we can determine if this is a provate controller, and must be authenticated, or not
     * public controller that is open to all.
     *
     * @param Dispatcher $dispatcher
     * @return boolean
     */
    public function beforeExecuteRoute(Event $event, Dispatcher $dispatcher)
    {

        $moduleName = $dispatcher->getModuleName();
        $identity = $this->auth->getIdentity();
        if (!$this->acl->isPublic($dispatcher->getControllerName()) && !is_array($identity)) {

            $this->flashSession->notice('Please login to take action');

            $this->response->redirect(array('for' => 'homepage'));
            return;
        }

        if ($moduleName == 'backend' && !$this->auth->isGranted('ROLE_ADMIN')) {
            $this->flashSession->notice('You don\'t have access to this module: private');

            return $this->response->redirect(array('for' => 'homepage'));
        }

        if (!$this->acl->isAllowed($this->auth->getRoles())) {

            $this->flashSession->notice('You don\'t have access to this module: private');

            return $this->response->redirect(array('for' => 'homepage'));
        }

        return true;
    }
} 

//service php

/**
 * Access Control List
 */
$di->set('acl', function () use ($di) {
    $acl = new \APP\Acl\Acl(new \Phalcon\Annotations\Adapter\Xcache(), $di);
    $acl->setPublicResources(array('index', 'session'));
    return $acl;
});

I'm using modules so for each module I did folowing

<?php

namespace APP\Backend;

use APP\Acl\Gate;

class Module
{

    public function registerAutoloaders()
    {

        $loader = new \Phalcon\Loader();

        $loader->registerNamespaces(array(
            'APP\Backend\Controllers' => realpath(__DIR__ . '/../../modules/backend/controllers/'),
            'APP\Models' => '../app/models/',
        ));

        $loader->register();
    }

    /**
     * Register the services here to make them general or register in the ModuleDefinition to make them module-specific
     */
    public function registerServices($di)
    {

/*      $dispatcher = $di->get('dispatcher');
        $eventManager = new \Phalcon\Events\Manager();
        $eventManager->attach('dispatch', new Gate());

        $dispatcher->setEventsManager($eventManager);

        $dispatcher->setDefaultNamespace("APP\\Backend\\Controllers\\");

        $di->get('view')->setViewsDir(realpath(__DIR__ . '/../../modules/backend/views/'.$di->get('siteLayout')));*/

        $dispatcher = $di->get('dispatcher');

        $eventManager = $di->getShared('eventsManager');

        $eventManager->attach('dispatch:beforeExecuteRoute', new Gate());

        $dispatcher->setEventsManager($eventManager);

        $dispatcher->setDefaultNamespace("APP\\Backend\\Controllers\\");

        $di->get('view')->setViewsDir(realpath(__DIR__ . '/../../modules/backend/views/'.$di->get('siteLayout'))."/");
    }

}

than for controller I did something like that

<?php

namespace APP\Frontend\Controllers;
use APP\Models\Entity\Users;

/**
 *
 * Class DashboardController
 * @package APP\Frontend\Controllers
 */
class DashboardController extends ControllerBase
{
    /**
     * Show user dashboard
     *
     * @Allow('ROLE_USER')
     */
    public function indexAction()
    {
        $user = $this->auth->getIdentity();
        $userModel = Users::findFirst($user['id']);

        $this->view->clubs = $userModel->UsersClubs;
    }
}

[Jurigag]

Okay. Now i get the idea. Its cool and fine. I can create some code to be build-in in phalcon for it. But i just dont know one thing:

$identity = $this->auth->getIdentity();

Then it depends from the developer where he will store this Identity, phalcon dont have any built-in auth/security etc service. Both version with annotations and without it can be nice, but from where get this user/identity ? That would need to implement some built-in auth service in phalcon to be working out of box, just configure and here you go, we can already use just RoleAware interface i already made. But how about auth service ? Just set/get user object from session ?

[sergeyklay]

Just use events:

$dispatcher = new Dispatcher;
$em         = new Phalcon\Events\Manager;

$em->attach('dispatch', new My\Awesome\Acl);

$dispatcher->setEventsManager($em);

Then, throw:

$this->getEventsManager()->fire(
    'dispatch:beforeException',
    $dispatcher,
    new Phalcon\Mvc\Dispatcher\Exception
);

by using My\Awesome\Acl::beforeDispatch if resource not alowed

[Jurigag]

Well the same way i guess we could create some method for getting user.

$awesomeAcl = new My\Awesome\Acl();
$awesomeAcl->setUserDefinition(function(){
    // here code for returning user from some service done by developer or whatever
});

$em->attach('dispatch', $awesomeAcl);

?

Also as longer i think, the annotations would be nice, but im not sure that we should define allowed roles while defining routes. Maybe in some configuration of our class/component.

Well how about some new service like Firewall ? While we only set it in di, set how we want to use it - by annotations or while adding routes, set annotation adapter(if we use annotations), method for events manager(fire events when allow/disallow), and function for defineing "user definition" which return role as string, or object impelemting RoleAware and phalcon will do the rest ?

I will create some code and do PR with it.

[josefguenther]

By the way, this issue will get in your way: phalcon/cphalcon#11378

It would need to be fixed before adding stuff to the router array.

[Jurigag]

Oh right, i totally forgot about this bug :/ Well, maybe it will be problem - it will inject for example allowed array into params in dispatcher.

[Jurigag]

Well i am working on some built-in firewall class. Should i still make it for 2.1 or add it later in 2.2 ? @sergeyklay

Defineing it would be something like this:

use Phalcon\Annotations\Adapter\Apc as ApcReader;

$di->set('dispatcher',function(){
    $dispatcher = new Dispatcher();
    $eventsManager = new EventsManager();
    $firewall = new Firewall("acl"); // name of acl service in container, also you can add events manager to firewall to handle three events - beforeCheck, afterCheck, beforeException
    $firewall->setUseAnnotations(true, new ApcReader()); // another way i wanted to use firewall was when defining allowed/denied roles when add routes to application, but dont know about it with this bug above
    $firewall->setRoleClosure(function(){
        return Di::getDefault()->get('session')->get('user'); // returning string(role name) or object implementing RoleAware
    });
    $firewall->setDefaultAccess(Acl::Allow);
    $eventsManager->attach('dispatcher:beforeExecuteRoute',$firewall);
    $dispatcher->setEventsManager($eventsManager);
    return $dispatcher;
});

And we could use @Allow() or @Deny() both on controller and action with array of roles or single role as string. Also default access(no annotation) would be what we set earlier. If we dont set any parameter in annotation then all roles all allowed/denied.

Also it's firstly checking access to controller, then to action, if we dont have access to controller then it's checking action - im not sure about it, maybe we should just return no access if we don't have access to controller/no defined access(and default access is deny ?) and if there is no defined access(but default access is allow) then check action ?

Also maybe only configuration by annotations would be enough ? @daison12006013 @valVk @SidRoberts

[daison12006013]

@Jurigag, awesome! I have a question, does this supports multiple roles?

I see this:

$firewall->setRoleClosure(function(){
        return Di::getDefault()->get('session')->get('user');
    });

is the returned data from session is array of roles?

[sergeyklay]

Should i still make it for 2.1 or add it later in 2.2 ?

2.1 coming soon. It will be LTS release

setRoleClosure

not the best idea to call methods in a similar way

[Jurigag]

@daison12006013 Well you mean one user can have multiple roles ? Well phalcon's acl is not supporting it(or is it ?) so i don't see a point in supporting this in firewall.

[daison12006013]

Logically for business requirements, a user can have multiple roles and a role can have multiple users.

Might be better to update your firewall setRoleClosure to support multiple injected roles? Is it possible?

Scenario:
What if I have multiple roles, such as an Supervisor and Client Support, I must have the ability to access Client Support(creation of ticket, frontliner), and Supervisor(creationg, soft deletion, updating of tickets). Which the supervisor could be a frontliner too. Let's assume their tasks is our controller action.

---

You mentioned having @Allow() and @Deny() that supports array of roles, how about the setRoleClosure?

[Jurigag]

And how it's suppose to work if we pass multiple roles ? Check if every role have access to this action/controller or only one of them ? You can just make Supervisor to inherit Client Support role and that's it.

Phalcon's ACL don't support passing array of roles in isRole or isAllowed for example.

well @Allow() and @Deny() is pretty obvious that need's to support array of roles, cuz you can have many roles that will access some certain action. But you can almost always make role that inherit some other role, user don't have to have multiple roles. If I will add this in firewall, then in phalcon's acl it's needed too.

[daison12006013]

Yes it is possible, I once tried using the existing acl.

[Jurigag]

No it's not, you have to pass string, not array:
https://github.com/phalcon/cphalcon/blob/master/phalcon/acl/adapter/memory.zep#L498
https://github.com/phalcon/cphalcon/blob/master/phalcon/acl/adapter/memory.zep#L235

Until those methods will accept arrays i will not implement arrays here. You can use method inheritance to have always one role per user.

It's gonna using acl for checking if certain role exist in application.

Or maybe should make it as a whole seperated component without using acl(at least when using annotations) ? Then maybe i will add what's you want.

Also possibly i will add option to use firewall without annotations using just phalcon's acl.

[daison12006013]

@Jurigag, you're talking about the checker function? ofcourse that must be a single checker, what I am talking about is this mate:

https://github.com/phalcon/cphalcon/blob/master/phalcon/acl/adapter/memory.zep#L158

which you could call it as many times as you want to add a role from current request.

[Jurigag]

But im not gonna add a roles from current request. All roles will be defined in acl - as always, that's why you have provide name of acl service to check if role exists, current user role(single string or object implementing RoleAware) will be returned from function defined in:

$firewall->setRoleClosure()

And for it will be doing check. I dont see anywhere in phalcon that you check array of roles if they allowed. So how it's should be working in firewall ?

I don't see why it suppose to return array, and how to check later this array ? Check if any role from @Allow or @Deny is in there ?

[daison12006013]

That's the problem, because my feature request will be based on route, which triggers the resource Controller and Action.

So the only plan is to iterate the roles assigned to the resource,

then in the dispatcher event we should probably check all the roles assigned to the resource, and let's check if we triggered the addRole() before the route request.

---

Let us say we initially added a role from the current request. We assigned superuser and user. NOTE: this is global, always initialized.

So a new request came example.com/user/lists, now this route requires a superuser, administrator access. In the event dispatcher we're comparing each initialized roles which we only have [superuser, user].

Now back to story, the route requires a [superuser, administrator], we then iterate one-by-one until we were able to match the initialized roles, returning true if allowed, else throw an exception \AccessNotAllowedException.

---

Example:

The first is the initialized role(s), second is the route, third is allowed role(s), fourth is the return logic.

[user]
GET|POST example.com/user/1/delete
[administrator, superuser]
= NOT ALLOWED found none

[user, superuser]
GET|POST example.com/user/1/edit
[administrator, superuser]
= ALLOWED found superuser

[user, superuser]
POST example.com/reports/dtr-to-excel/generate
[user]
= ALLOWED found user

[user, superuser]
POST example.com/tickets/1/cancel
[user, superuser]
= ALLOWED found 2 user and superuser

[administrator]
POST example.com/tickets/1/cancel
[user, superuser]
= NOT ALLOWED found none

Is this sample doable in Phalcon? It is much more easier though to assign roles.

[Jurigag]

Let us say we initially added a role from the current request. We assigned superuser and user. NOTE: this is global, always initialized.

What you mean it's global ? Why it's global ? Role depends on the user, not a global stuff. Why you just don't implement use role inheritance and that's it ? I can add functionnality you want to iterate passed array of roles, just function in $firewall->setRoleClosure() would need to return array of roles, but it would only work if you are using annotation configuration. If you for example would want to switch to phalcon's acl then only string or object passed as RoleAware would be possible.

Well i guess i would add option to use firewall independent from ACL, and acl as configuration option, so there will be two(or three mechanisms to set firewall):

• If you want to use annotation you don't even need registered acl in service and added roles, they will be just getted from function which you defined in setRoleClosure(), in this case you can use array of roles.
• If you want to use phalcon's acl then you obviously need it to register in service and add all roles etc, cuz role returned from setRoleClosure() will be checked if it exists, in this case you can't use array of roles beacause isAllowed() and isRole() only allow string, also here you could possibly use user defined function in phalcon's acl(added in 2.1.x) where you can add custom logic after model is binded if you are using model binding(well need to wait for such an event beacause currently there is no such an event, then firewall would work for example on afterBinding instead of beforeExecuteRoute). In this option the controller name would be resource name and action name would be access. Not sure what to do if there will be two controllers with the same name - then we have a problem ?
• Well third option is to define allowed/denied roles when adding routes, like:

$router->add(
    "/documentation/{chapter}/{name}.{type:[a-z]+}",
    array(
        "controller" => "documentation",
        "action"     => "show",
        "denied"    => ["SOMEROLE"],
        "allowed"    => ["SOMEROLE2"]
    )

But not sure about this 3rd option, beacause it's gonna be harded to accomplish than other two. And not sure it's necessary.

[daison12006013]

@Jurigag, what I mean about the initialized roles, those are roles came from a user's roles in the database table, in which it will be stored in the session, now it is always global because every request we're iterating the session roles and calling the addRole() in Phalcon ACL.

Now the remaining process is the one I've requested, to implement the 'roles' thing in the route and an internal process in the dispatcher to check 'roles' key if exists.

About the route allowed/denied in the routes, that's my last resort if this isn't doable on Phalcon side, then I'll just do a PHP side implementation.

[Jurigag]

I don't get it. I know you can addRole() in phalcon acl as array but you CANT CHECK THEM if they are allowed or this array of roles exist. So if we want to use firewall in annotation method the best way will be just get rid of phalcon acl in this case, and just get roles from function defined in setRoleClosure() and check if any role from it is allowed to controller/action. Easy and simple.

Or maybe you want @Allow and @Deny annotations to build whole acl service ? Then i kind of see point of calling addRole() on phalcon's acl.

[daison12006013]

Yeah I think we should not rely on Phalcon ACL itself for this as it has a different approach, I'm looking forward with your firewall though.

Hoping that to be added 2.1 also.

[Jurigag]

@sergeyklay

setRoleClosure()

how it should be called ? maybe setRoleCallback or setUserCallback ? Or something else ?

Code is pretty much finished, need to test it, write tests and do pr.

And also im waiting for adding afterBiding event to dispatcher, to be able for firewall to be running after model is binded so it can be passed to user defined function in acl(so we can really have nice firewall with custom logic - for example if something is created by user or don't).

Also all is done in one class. What you guys think about splitting it to two classes:
Phalcon\Firewall\Adapter\Annotation and Phalcon\Firewall\Adapter\Acl ?

[Jurigag]

Okay, there is PR:

phalcon/cphalcon#11590

Let's move discussion here, beacause i asked there some questions.

[Jurigag]

I will create proper pr for 3.1.0 for this as soon branch will be created. It will be similar as this pr but i will add ability to use it with micro too.

[Jurigag]

Well i plan to add Firewall namespace to Phalcon 4, which will allow you do achieve this functionallity. It won't be exactly on router however. It will be simply - set rules in acl or annotations in your controllers. Attach Firewall class as listener to Dispatcher and done pretty much.

[niden]

phalcon/cphalcon#14078

[ruudboon]

Reopening this. See phalcon/cphalcon#14631