JMSSecurityExtraBundle icon indicating copy to clipboard operation
JMSSecurityExtraBundle copied to clipboard

[Feature request] Allow to use an expression when compiling another expression

Open stof opened this issue 12 years ago • 6 comments

I would like to define a custom function implementing some high-level permission check for my app, which would use lower-level functions.

For instance, the expression canUpdate(object) could then translate to (object.canBeUpdatedByParticipants() and isParticipants(object)) or hasPermission(object, 'OWNER') or hasRole("ADMIN")

I currently see 3 solutions:

  • copy all the compiled code of low-level functions I want to call (bad idea as it would duplicate stuff available in other expression function compilers and adds extra maintenance work in our app)
  • build the AST tree and compile it (better but painful as hell)
  • instantiate a parser to turn an expression into the AST and then subcompile it

The third solution looks the right one for this. So unless there is a better way to do this (in which case, I will gladly hear about it), what do you think about adding an helper method in the ExpressionCompiler to subcompile an expression ?

stof avatar Jan 23 '13 12:01 stof

I had the urge to implement something like this myself already. However, a better solution was almost always to defer to the ACL system and let it handle the checks.

In your case, this would also be possible with hasPermission(object, 'EDIT')? So, I'm not sure whether we really need it, or whether deferring to ACL is the way to go.

schmittjoh avatar Jan 23 '13 13:01 schmittjoh

@schmittjoh I'm not using the ACL system in my app. It would require duplicating storage as I would need to register the ownership both in the ACL system and in the doctrine relation between my object and the user (as I need it in some other places, for instance to be able to restrict a Doctrine query based on ownership).

In a previous project, I was doing this using custom voters (which would not be taken into account by hasPermission() and was forced to inject the RoleVoter into my custom voter sometimes to be able to check on roles.

stof avatar Jan 23 '13 13:01 stof

Maybe I'm missing something, but is there a problem with duplicating this information? It looks fine to me.

schmittjoh avatar Jan 23 '13 13:01 schmittjoh

Well, the issue is that we would need to add some logic to keep both in sync. And the ownership is not the only stuff we would need to duplicate. We also have some permissions depending on whether the user is a participant in the contest he is looking at, and this permission will also depend on a flag set on contest by the admin (represented by object.canBeUpdatedByParticipants() above).

Note that my example above is not exactly what we would have in our app as the ownership check would be done by a custom function, not by the ACL system.

stof avatar Jan 23 '13 13:01 stof

It is actually pretty easy to do this in an entity listener without modifying your application logic.

However, I'm also open to expanding expressions. I could imagine that we add something like a pre-compiler step where we expand expressions. Basically, we would use a visitor pattern where each visitor could traverse and modify the AST. On top of this low level system, we could then also add some sugar to make it easier to use.

schmittjoh avatar Jan 23 '13 14:01 schmittjoh

A visitor pattern would be great.

When trying to do some more complex stuff, I figured that the current solutions are not as easy as I thought at first as the replacement has to be done when compiling preconditions too, to be sure they are compiled before compiling the function (see #108 for an issue I faced while doing so). Modifying the AST tree before the compilation would be easier as it would allow doing the replacement once and then letting the compiler handling preconditions and compilation normally.

stof avatar Jan 31 '13 11:01 stof