phalcon / cphalcon

High performance, full-stack PHP framework delivered as a C extension.
https://phalcon.io
BSD 3-Clause "New" or "Revised" License
10.79k stars 1.96k forks source link

[NFR] Modules to ACL #582

Closed Swader closed 6 years ago

Swader commented 11 years ago

Please consider adding getModuleName() to Dispatcher. It already has getControllerName and getActionName, and would be very helpful in custom ACL issues on multi-module applications.

Also consider adding Modules to ACL resources somehow. Right now resources are registered in the form of $controller => $actionArray, but there is no way to register a module such that a specific user role (admin) has access to a specific module in its entirety (admin module, for example).

niden commented 11 years ago

@Swader We had a discussion about the ACL NFR mentioned above

It is a bit more complicated to offer this kind of functionality. Effectively the module itself would need to be treated as a resource itself so that roles can be attached to it. Since each module has its own dispatcher we will need to do some magic in the background to ensure that the ACL works as expected in that kind of scenario.

Of course the quick and easy solution would be to have an ACL for each module - which can be done with the current functionality.

niden commented 11 years ago

Assigning this to 1.2.0. If we get some time before that we will address it.

Swader commented 11 years ago

Thanks, good to know. Will do workaround until then.

tasmaniski commented 11 years ago

+1 adding Modules to ACL resources

phalcon commented 11 years ago

We can use "resources" as an analogy for "controllers", what is the correct term for "modules" in an Acl?

Swader commented 11 years ago
  1. Leave it at "modules".
  2. Add a resource type flag: Resource::MODULE vs Resource::CONTROLLER
  3. Alter Resources such that they are created as a combination encompassing all three. E.g.,
new Resource(array('module' => 'admin', 'controller' => '*', 'action' => '*')), 

or

new Resource(array('module' => '*', 'controller' => 'management', 'action' => '*'));

or

new Resource(array('module' => 'frontend', 'controller' => '*', 'action' => 'my_*'));

etc.

I'm heavily in favor of the first one.

tasmaniski commented 11 years ago

I'm not sure there is an official name. Many frameworks haven't ACL in 3 levels. But the two levels are not sufficient.

@Swader Third option seems interesting .. Name "modules" would be quite ok.

pdeszynski commented 11 years ago

As for me, there shouldn't be modules at all in ACL. Each module should define it's own ACL resources to encapsulate behaviour and do not share globally ACL configuration. Additionally ACL really often acts differently in each module, but making it exactly the same in every module could lead to security risks (for e.g. you will for sure do Admin module a closed system, whether your main application might be an open system). So these are for me arguments against this feature.

tasmaniski commented 11 years ago

"Additionally ACL really often acts differently in each module, but making it exactly the same in every module could lead to security risks"

I see no reason why your requirements should not meet. You can set different permissions for every module. If you have an application that has more than two modules(Admin&Default), ACL in three level is required.

A lot of people have this issues eg. http://stackoverflow.com/a/7139565/949273 http://stackoverflow.com/search?q=set+module+in+ACL

pdeszynski commented 11 years ago

@tasmaniski I wouldn't still use ACL that's crossmodule one. ZF which is mostly shown in these links works differently than Phalcon in case of modules - all of them are fired no matter in which one request will finish. In Phalcon only module which servers response for current request is fired. This allows you to easily bootstrap ACL per current module, not to waste resources or by mistake overwrite permissions. This would be preferable approach at least that I would go.

As for current ACL as a resource cannot a "Module-Controller" be taken into consideration as in many frameworks? ACL could be build with resources as module:controller. I just never saw an abstraction like module in ACL, it's always about permissions attached to an object, but I don't deny that they couldn't be created.

tasmaniski commented 11 years ago

Yes, you're right. Maybe it's better to define ACL in module. I have not tried that, seems logical.

iforp commented 11 years ago

-1 for issue The rules should be set for each module separately.

hesalx commented 11 years ago

Maybe the problem is in understanding of modules. Now they are some kind of "separated application beyond a single domain name". But many developers want "separated entities in a single application". Sounds different, yes? As for me, I want to have forum as a natural essential part of my web application using its models, forward to its controllers and etc. Yes, I have tried modules. And understood it is not I want. I had to play with namespaces, events and router to separate forum from other parts. And now all that is complicated and hard to deal with (but not so bad as if forum is dispersed throughout the application).

What they want is the HMVC-architecture. And modules, as they're now, don't provide such thing. It should be something with shared dispatcher, router and acl (and some others maybe as default, or by developer choice as well).

That's the problem. Phalcon users try to transform modules in something they aren't.

Swader commented 11 years ago

I suppose it is all in the habits. On my last job, I've worked in zend for some 3 years. Zend has exactly this - modules are resources like any other and can be bound to a role (or multiple roles). So this approach is nothing but natural to me.

A use case is a "backend" module for maintainers of the site only. Another is a cms module for handling content creation. An administrator should be able to mess with the whole site, a content writer should be able to only access the cms. Thus, in mockish code,

    $acl->addResources(
        array(
            'admin' =>... ,
            'cms' =>... 
            'frontend' =>... 
        )
    ); 
    $acl->addRoles('admin', 'writer');
    $acl->bind('admin', '*');
    $acl->bind('writer', array('frontend', 'cms');

It all makes perfect sense to me, but I understand the confusion from people not used to this approach.

Eavu notifications@github.com wrote:

Maybe the problem is in understanding of modules. Now they are some kind of "separated application beyond a single domain name". But many developers want "separated entities in a single application". Sounds different, yes? As for me, I want to have forum as a natural essential part of my web application using its models, forward to its controllers and etc. Yes, I have tried modules. And understood it is not I want. I had to play with namespaces, events and router to separate forum from other parts. And now all that is complicated and hard to deal with (but not so bad as if forum is dispersed throughout the application).

What they want is the HMVC-architecture. And modules, as they're now, don't provide such thing. It should be something with shared dispatcher, router and acl (and some others maybe as default, or by developer choice as well).

That's the problem. Phalcon users try to transform modules in something they aren't.


Reply to this email directly or view it on GitHub: https://github.com/phalcon/cphalcon/issues/582#issuecomment-21224211

Sent from my Android phone with K-9 Mail. Please excuse my brevity.

bendo01 commented 10 years ago

+1 adding Modules to ACL resources

Slyvampy commented 10 years ago

Is this something that is ever going to be considered? currently having most of the controllers in its own library rather than modules is a pain. I put everything into modules and then realised that there is no functionality to use the phalcon ACL so I have two options, either do not use modules or do not use phalcon's ACL.?

An update would be good :D

cornfeedhobo commented 10 years ago

@Slyvampy I am not sure that it ever should be. I have a really nice solution to a multi-module ACL that I would be glad to share. It builds the ACL for just the module being used and uses a multi-dimensional array to construct everything. I am honestly surprised my way is not in the docs. It seems like the only logical approach.

Swader commented 10 years ago

@cornfeed do tell. I'll take it up another notch - I'll pay you to write a tutorial on making module based ACL happen in Phalcon, if your solution is approachable enough. Get in touch via +BrunoSkvorc, @bitfalls or bruno.skvorc@sitepoint.com

cornfeedhobo commented 10 years ago

@Swader Email sent. Please take a look and tell me what you think. Essentially, I see the value in per-module ACLs but I want a central place to manage them (database or array). This is the best compromise. Since the ACL should be hooking into the dispatcher, you can get the current module being called and load only the portion of the array that is relevant. Wrap it all in some caching and it is pretty clean in my opinion. Let me know what you think before I post it to github.

KoriSeng commented 9 years ago

https://github.com/7thcubic/cphalcon/tree/2.0.0_database_module_acl https://github.com/7thcubic/cphalcon/tree/2.0.0_module_acl

Help me test it and let me know if i missed anything. It seems to be working correctly.

Need some rewrite to make it nicer though imo, open for suggestions.

wgalarcon commented 9 years ago

Resources as modules create acl? Help

Slyvampy commented 9 years ago

I ended up implementing a Zend Framework approach with the resource array 'module:controller'=> array('method') to get around it.

kornerita commented 7 years ago

Hi,

The \Phalcon\Mvc\ModuleDefinitionInterface requires a registerServices method to register it's services.

On my implementation of this I get the acl from the DI and add the Resources, Roles and permissions for the module there.

i.e.: Module.php

class Module implements ModuleDefinitionInterface { 

    public function registerServices($dependencyInjector) {
        $acl = $dependencyInjector->get('acl'); /* @var $acl \Phalcon\Acl\AdapterInterface */
        $acl->addRole('module_role');
        $acl->addResource('Module/IndexController', ['index']);
        $acl->allow('module_role', 'Module/IndexController', 'index');
    }

    // ...
niden commented 5 years ago

https://github.com/phalcon/cphalcon/pull/14078