Also im guessing that firewall should have methods to get controller name, action, roles, module name, to make them be available in handleException i guess ?

@sergeyklay @daison12006013 @valVk @SidRoberts

Could you guys look into it and comment about things i mentioned when you have time and i will update it ? All of you were writing in ACL on router issue. I think it needs discussion before adding it to phalcon in any time. I just need some feedback and will do the rest.

Major 👍 A very useful feature to have. Thanks for implementing it.

It looks more something for the incubator than the core. Keeping this open for discussion

Well, not everyone knows about incubator and using it, if it would be in core and docs then someone will know that there is such behavior. I guess there are already similar thing in incubator, so no reason for adding this to it.

I will test it later if there is any more reason for having this in core(like performance boosts).

Also most of frameworks have built-in firewall or something like this. I think phalcon would need it to built-in. If someone doesn't like how it's gonna function - they he can look for different solutions or implement ones himself.

This might belong under the Phalcon\Security\Firewall namespace, since it's a security feature and the namespace is mostly unused. But then again I also think the ACL should be under security too.

@sergeyklay I would say it belongs in the core, because:

1. It's a very useful feature, which can save a considerable amount of work in doing a rather common task of checking the user role before executing a controller action. Most web apps have some form of usergroups or roles, at least an admin with added permissions. This feature makes securing an admin panel, for example, a single annotation line, which is a great developer experience.
2. we already have many features in the core that are much more situational than securing access to routes, for example we have the image adapters which I doubt are used by more than 1% of the users of the framework.
3. This feature doesn't add considerable bloat. It is only 2 additional classes and one interface.
4. Most other frameworks have something similar. For example http://symfony.com/doc/current/components/security/firewall.html . Therefore many new users coming to the framework may expect to find a similar feature.

This is really a helpful, upon checking the test sample, it's kind a more easy way to handle rather than building a set of ACL, this firewall feature is awesome I guess. 👍 @Jurigag

I don't say NO.

I just want to discuss this issue in more detail. I don't like the idea of having this feature in the core. Frankly speaking, I have spoken to @andresgutierrez, and he also thinks that this feature must remain in the Incubator. Your suggestion is too optional. Let's just discuss the use case of your feature. Let's take opinion by @niden, @xboston or @carvajaldiazeduar.

Maybe, I don't understand something. Show me a real example where it can be used. Explain why the framework will be poor without this feature at the core level.

I'll chime in as well. First, I think @Green-Cat said most of it in #11590 (comment), already.

I don't think the framework will be worse-off without it, but I do feel it's an extension to the existing ACL and therefor ok to have in the core. Providing more native features in the framework is not a bad thing, I also always understood the incubator as a place where functionality lives for a while until it gets ported to the framework definitely. Moving it there would by that reasoning simply postpone the merging.

I think the only real arguments that can be brought against it are:

• Phalcon doesn't aim to be a full-stack framework so we shouldn't implement "non-core" features. Which to my knowledge is false, Phalcon does aim to be that, in the long run, attested by the copious amount of libraries already available like Image, Security, Crypt, Queue, etc.
• The functionality already exists. It doesn't exactly though, the ACL we have is different.

@rianorie
Actually it's not really extension to acl, there is only option to be able to use it with acl, (but you can as well use it without acl, only as annotations) so we can pretty much change names(or even dont if we name resources as controller prefixes) and add firewall and we ready to go.

I already done extension to acl in this commit: https://github.com/phalcon/cphalcon/pull/11277/files

That's why firewall can work too on afterBinding(and why also added this event in this PR) and you can add some custom logic to acl and operate on objects and for example have something like this:

function someAction(Article $article)

And if you will add some custom logic to acl(from 2.1.x) you can do that, firewall will check for example if current user can edit this article beacause it's added by him or don't(for example)

I'll clarify, I meant it can serve as an extension, as in, provide extra functionality together with the existing ACL. I didn't mean they were actually related as such.

Yea but it doesn't changing anything in ACL, acl remains the same as it is. Only one problem maybe is that i thought that controller prefix as resource name is fine, but maybe someone else have already other naming convention and he would need to change it to work with built-in firewall.

@daison12006013 @valVk check annotations firewall, beacause you guys wanted it, not sure if it's working how it supposed(or you need) but i think it's enough and logic

@Jurigag, sorry seems like I late to the party. I'll take a look into this.

I agree that this feature have to live in incubator and later after some time if it'll need than it have to be implemented to the core.

In the same time I belive that this functionality have to be in the core, because this is the very usefull thing in full stack frameworks.

Well if you need use cases then in like most of the CRM/CMS systems there are roles, and mostly some roles have access to some action, some don't and it would be nice to have some functionality in core which is used pretty often. At least more then for example Image adapters or queue.

I pretty much need this functionality in all my 4 projects im working on. Queue im using actually in one of them for sending emails(beacause it's sending emails to group of people so it would slow down response if done when sending response).

Just so you know guys

$ ls -al ext/modules/
total 300828
drwxr-xr-x 2 root root      4096 Apr 13 17:59 .
drwxrwxr-x 9 klay klay      4096 Apr 13 17:59 ..
-rwxr-xr-x 1 root root 308032250 Apr 13 17:59 phalcon.so

308 MB

@sergeyklay it's only on php 5.6 like that, on php 7 for example is much smaller. Still most of hostings that have phalcon support have higher size. And VPS have moslty 20++ gbs.

That's a 293M module that PHP needs to load every single time.. It's not just about the diskspace.

Wtf ? It's loaded only once on php startup bro. You can check that easly by starting for example php5-fpm or apache and then delete phalcon.so - application will still work unitl you restart it.

If php would need to load 293M module every single time then phalcon would be useless.

How can it be 300Mb whereas in Phalcon 2.0.9 is 4.6Mb ?

@ogarbe because some people forget that Phalcon is framework but not all-in-one

my question is what have change which add 295Mb in the compiled code ??? (the 300Mb code is perhaps the debug one ?)

Yes, 300 mb is debug one(compiled on debug version of php). Phalcon 2.1.x on non-debug is 4.6 mb.

On production you should like never use debug version beacause it will slow down app.

Write php -v next time before checking size xD

About this component - i will add afterBinding tests and some more methods to firewall to access active controller etc so we don't have to access dispatcher in beforeException after quick thinking i think that this will duplicate code, i will just add method to return dispatcher. Then i will rebase it. I don't know still about the things i wrote in list. I will propose something on my own then:

• i will put this under Phalcon\Mvc\Dispatcher namespace beacause it requires Phalcon\Mvc\Dispatcher and uses it
• if it's multi module application there will be option to set that we are using multi-module application and option to set seperator between module name and controller prefix(default :) so it will look for resource in acl like ModuleName:ControllerPrefix

But i don't even know i should spend a time on it beacause as i see there is no need for this (?).

Maybe when i will add afterBinding tests then you will get the idea beaucse it will clean up code pretty nicely. And acutally make a binding viable option for CRM systems.

Okay. Added the rest of tests and things i wrote here lately and rebased.

Especially see tests with afterBinding beacause it gives you nice ability to have clean code.

If anyone have any idea or something to add that it should work in diffrent way maybe(annotations firewall or something else) then tell me.

As i see there is like no chance for this in 2.1.0 ? How about 2.2.x ?

most likely 2.2.x

Fine with me. Well until 2.1.0 is released then leave it as open so more people can see this and write some idea or comment if have any.

3.1.x ? Actually will need to change branch for it, well when it will be created then mention me. So i don't forget about this.

@Green-Cat well im planning to add ability to use it with Phalcon\Mvc\Micro as well. So i would need to add it in other namepsace. So Phalcon\Firewall or Phalcon\Security\Firwall ? @sergeyklay

I prefer Phalcon\Firewall

I think this definitely belongs in incubator as it's controversial and adds performance overhead as well. Needs a lot more discussion prior to being included.

So in php it will be even slower - and this won't be used out of box. You would need to create class and attach it to dispatcher/micro.

I just in every project have checking if user can access something/execute action, it's silly to have this in incubator imho. Isn't why phalcon is written in c/zephir - to make common things faster ?