I think it's better to add this check directly on the ObjectIdentity::fromDomainObject method so you don't have to do this check everytime you call this method. Also, if we wouldn't add this check inside this method, we should throw an exception for this case so we prevent unexpected behaviour like the described before. What do you think? I'll wait for your feedback about this to make the pull request.
I've added this check and a corresponding unit test: https://github.com/comfortablynumb/symfony/tree/acl-fix Thanks. On Thu, Mar 24, 2011 at 5:27 PM, Johannes Schmitt <[email protected]>wrote: > It would indeed be a good addition. Could you add something like > > if (!$domainObject instanceof ObjectIdentityInterface) { > $domainObject = > $oidRetrievalStrategy->getObjectIdentity($domainObject); > } > > to the AclVoter class; please also add a unit test to verify this behavior. > > Thanks, > Johannes > > > > On Thu, Mar 24, 2011 at 4:03 PM, Gustavo Adrian < > [email protected]> wrote: > >> Can anyone verify this? if this is indeed a bug then I'll create a pull >> request. If not, what I am doing wrong? >> >> >> Thanks. >> >> On Wed, Mar 23, 2011 at 4:55 PM, Gustavo Adrian < >> [email protected]> wrote: >> >>> Ok, I think I've found the reason for this behaviour. On the >>> "ObjectIdentityRetrievalStrategy" class the "getObjectIdentity" method >>> >>> public function getObjectIdentity($domainObject) >>> { >>> try { >>> return ObjectIdentity::fromDomainObject($domainObject); >>> } catch (InvalidDomainObjectException $failed) { >>> return null; >>> } >>> } >>> >>> So, if it already receives an ObjectIdentity instance (like it does if I >>> pass a new ObjectIdentity instance to the isGranted method of the >>> SecurityContext object), it pass it anyway to the fromDomainObject method, >>> so it returns an ObjectIdentity instance for the original ObjectIdentity I >>> passed to the isGranted method. If I change the method to look like: >>> >>> public function getObjectIdentity($domainObject) >>> { >>> try { >>> if ( get_class( $domainObject ) !== >>> 'Symfony\Component\Security\Acl\Domain\ObjectIdentity' ) { >>> return ObjectIdentity::fromDomainObject($domainObject); >>> } else { >>> return $domainObject; >>> } >>> } catch (InvalidDomainObjectException $failed) { >>> return null; >>> } >>> } >>> >>> It works like I want and it successfully denies me the creation of a >>> Comment object. What I still don't know if I'm doing the right thing when I >>> try to check If a user has a class-scope permission to CREATE a Comment, >>> like in the example I've shown before. >>> >>> >>> >>> Thanks :) >>> >>> On Wed, Mar 23, 2011 at 4:44 PM, Gustavo Adrian < >>> [email protected]> wrote: >>> >>>> Another thing that it recently came up is this. Having no ACL or even an >>>> entry for this ObjectIdentity, if I try this: >>>> >>>> // $comment domain object was loaded from DB >>>> >>>> if ( $securityContext->isGranted( 'UPDATE', $comment ) === false ) >>>> { >>>> throw new AccessDeniedException( 'Some error message' ); >>>> } >>>> >>>> It successfully denies me to update the object as expected. >>>> >>>> Logs show this: >>>> >>>> 2011-03-24T00:01:45-03:00 INFO: SELECT a.ancestor_id >>>> FROM acl_object_identities o >>>> INNER JOIN acl_classes c ON c.id = o.class_id >>>> INNER JOIN acl_object_identity_ancestors a ON >>>> a.object_identity_id = o.id >>>> WHERE ((o.object_identifier = '24435' AND c.class_type = >>>> 'MyCommentClass')) ([]) >>>> 2011-03-24T00:01:45-03:00 DEBUG: No ACL found for the object identity. >>>> Voting to deny access. >>>> >>>> Just what I was expecting. But When I try to create a new object: >>>> >>>> // A new ObjectIdentity representing the Comment class for class-scope >>>> ACE's >>>> $commentClassOID = new ObjectIdentity( -1, 'MyCommentClass' ); >>>> >>>> if ( $securityContext->isGranted( 'CREATE', $commentClassOID ) === false >>>> ) >>>> { >>>> throw new AccessDeniedException( 'Some error message' ); >>>> } >>>> >>>> It doesn't throw an exception, and the logs show this: >>>> >>>> 2011-03-24T00:05:12-03:00 DEBUG: Object identity unavailable. Voting to >>>> grant access >>>> >>>> What I don't know is what's the default behaviour of the ACL feature if >>>> it doesn't have an ACL for an ObjectIdentity, or if it doesn't have any >>>> object identity on the DB referring to the ObjectIdentity passed to the >>>> isGrant method. So I still don't know which of both cases is wrong. >>>> >>>> >>>> >>>> Thanks in advance! >>>> >>>> On Wed, Mar 23, 2011 at 12:14 PM, Gustavo Adrian < >>>> [email protected]> wrote: >>>> >>>>> Hi Johannes, >>>>> >>>>> 1) My scenario is the following: >>>>> >>>>> a BusinessUnit has many Departments >>>>> a Department has many Users >>>>> >>>>> So, having this example tree: >>>>> >>>>> BusinessUnit 1 >>>>> |_ Department 1 >>>>> |_ User 1 >>>>> |_ User 2 >>>>> |_ Deparment 2 >>>>> |_ User 3 >>>>> >>>>> What I would like is to define rules in BusinessUnit's, and inherit >>>>> them from Departments. The same with my Users. I'd like them to inherit >>>>> their rules from Departments. So, if I want an ALLOW rule for the VIEW >>>>> permission in some "Article" class, I'd create a class-scope ACE for that >>>>> rule on BusinessUnit 1, and then every child of BusinessUnit 1 would >>>>> inherit >>>>> from it. So, for this case, I'd create an ACL for BusinessUnit 1, create >>>>> all >>>>> the ACEs I want in it (in this case, an ALLOW rule for the VIEW >>>>> class-scope >>>>> permission for the "Article" class) , and then create an ACL without ACEs >>>>> (or with specific ACEs if I want, for example, to DENY the previous ALLOW >>>>> permission) for the other entities, and inherit the ACL's to create a tree >>>>> just like my entity tree. For example, create an ACL for Department 1 and >>>>> make it inherit from the BusinessUnit 1's ACL I recently created. Then >>>>> create an ACL for User 1, and make it inherit from the ACL of Deparment 1, >>>>> etc. Is this scenario valid? having Department 1, to inherit permissions >>>>> from BusinessUnit 1, all I need to do is to create an ACL for Department 1 >>>>> and make it inherit from the ACL of BusinessUnit 1, and only create ACE's >>>>> for Deparment 1's ACL in case I want to customize the inherited >>>>> permissions? >>>>> >>>>> 2) Great! I'll give it a try today. >>>>> >>>>> 3) That seems really useful. audit_success being true means that it >>>>> will call the AuditLogger when someone is granted for that rule? so, if >>>>> "granting" is true and audit_success is true, it will call my AuditLogger >>>>> when someone call the "isGranted" method? the same goes for granting being >>>>> false and audit_failure true? >>>>> >>>>> >>>>> >>>>> Thanks a lot for your help Johannes :) >>>>> >>>>> >>>>> >>>>> On Tue, Mar 22, 2011 at 6:16 PM, Johannes Schmitt < >>>>> [email protected]> wrote: >>>>> >>>>>> 1) Yes, granting specifies whether the entry is granting (true) or >>>>>> denying (false). Depending on the order of ACL entries, and the SIDs to >>>>>> which the ACEs have been assigned, this can be used to implement >>>>>> scenarios >>>>>> like "grant access to all users, but for this specific user deny access". >>>>>> >>>>>> 2) The index is the index of the entry in the array. You can retrieve >>>>>> the array via ->getXXXAces() method, and then loop over it to get the >>>>>> correct index for your entry. Do not call methods directly on the ACE >>>>>> instances, these changes will not be persisted to the database. >>>>>> >>>>>> 3) Since the order in which the entries are checked is significant, >>>>>> "ace_order" is used to preserve the order of the entries in the array. >>>>>> audit_success/_failure are only relevant if you have set-up an >>>>>> AuditLogger >>>>>> which then can log if someone has been granted permissions through the >>>>>> ACL >>>>>> system, based on which entry permissions was granted, to which object >>>>>> permission was granted, etc. The AuditLogger is only called if the value >>>>>> of >>>>>> audit_success/_failure is true. >>>>>> >>>>>> Kind regards, >>>>>> Johannes >>>>>> >>>>>> >>>>>> On Tue, Mar 22, 2011 at 9:42 PM, Gustavo Adrian < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> After implementing class-scope permissions thanks to Christophe's >>>>>>> tip, I now have more questions :) >>>>>>> >>>>>>> I've been looking at the ACE schema and I see a couple of interesting >>>>>>> things that I'd like to know how to use them. >>>>>>> >>>>>>> 1) First I see a "granting" boolean field and a "grant_strategy" >>>>>>> field (with value "all" in my case in each row) on the "acl_entries" >>>>>>> table. >>>>>>> Looking then at the MutableAclInterface I see that the method >>>>>>> "insertClassAce" has these signature: >>>>>>> >>>>>>> *function insertClassAce(SecurityIdentityInterface $sid, $mask, >>>>>>> $index = 0, $granting = true, $strategy = null);* >>>>>>> >>>>>>> Does the $granting field allow me to use something like a DENY >>>>>>> permission if I pass "false" to it? if that's the case, could I use it >>>>>>> to >>>>>>> DENY a privilege that has an ALLOW permission somewhere up on the >>>>>>> hierarchy? >>>>>>> >>>>>>> 2) I've noticed that I can insert multiple Class Ace's using the same >>>>>>> object identity and the same security identity, but it increases the >>>>>>> "ace_order" for each row I insert. I have a form for each type of >>>>>>> entity on >>>>>>> my app to give class-scope permissions to specific users. Each time a >>>>>>> user >>>>>>> updates them, how can I update the corresponding ACE? I see in the >>>>>>> MutableAclInterface that it has a updateClassAce method, but it needs an >>>>>>> $index parameter and I don't know what it is. Is this the primary key >>>>>>> of the >>>>>>> ACE? if this is the case, should I query myself the DB using the class >>>>>>> ID >>>>>>> and the SID ID to retrieve the corresponding ACE's PK or is there a >>>>>>> method >>>>>>> to update ACE's based on the SID and OID that I haven't found? >>>>>>> >>>>>>> 3) What's the purpose of the following fields on the "acl_entries" >>>>>>> table?: ace_order, audit_success, audit_failure. I've seen the >>>>>>> AuditableAclInterface. How methods like updateClassAuditing (with >>>>>>> $auditSuccess and $auditFailure parameters) works? >>>>>>> >>>>>>> >>>>>>> >>>>>>> Thanks in advance and sorry if I asked too many questions, but this >>>>>>> is a wonderful feature and I'd love to know all its possibilities :) >>>>>>> >>>>>>> On Mon, Mar 21, 2011 at 4:26 PM, Gustavo Adrian < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> Great! even easier than I thought. I'm beginning to love this >>>>>>>> feature :) >>>>>>>> >>>>>>>> >>>>>>>> Thanks a lot! >>>>>>>> >>>>>>>> >>>>>>>> On Mon, Mar 21, 2011 at 4:22 PM, Christophe COEVOET >>>>>>>> <[email protected]>wrote: >>>>>>>> >>>>>>>>> Le 21/03/2011 20:11, Gustavo Adrian a écrit : >>>>>>>>> >>>>>>>>> Hi all, >>>>>>>>>> >>>>>>>>>> I'm starting to implement the ACL feature on my app and, for what >>>>>>>>>> I''ve read and what I've already implemented, I must say: it's >>>>>>>>>> BEAUTIFUL. >>>>>>>>>> Great job guys. It's just what I was looking for :) >>>>>>>>>> >>>>>>>>>> I have one question about this wonderful feature: >>>>>>>>>> >>>>>>>>>> 1) I've already implemented object scope permissions using the >>>>>>>>>> ObjectIdentifier class. What I don't know is: How to grant a class >>>>>>>>>> scope and >>>>>>>>>> class-field scope permissions to a user? I didn't find how to do it >>>>>>>>>> in the >>>>>>>>>> docs and I didn't find in the API something like ClassIdentifier. >>>>>>>>>> Which is >>>>>>>>>> the right way to grant a class scope permission to a user? Do I have >>>>>>>>>> to >>>>>>>>>> check this permissions the same way I do it with object-scope >>>>>>>>>> permissions? >>>>>>>>>> ($securityContext->isGranted( 'UPDATE', $objectIdentity ) ) >>>>>>>>>> >>>>>>>>> >>>>>>>>> the key to create a class Ace is to use the insertClassAce method: >>>>>>>>> >>>>>>>>> $oid = new ObjectIdentity('whatever_you_want', 'Class\\Name'); >>>>>>>>> $acl = $provider->createAcl($oid); >>>>>>>>> $sid = UserSecurityIdentity::fromAccount($user); >>>>>>>>> $acl->insertClassAce($sid, MaskBuilder::MASK_OWNER); >>>>>>>>> >>>>>>>>> And then your user will have OWNER rights for all instances of the >>>>>>>>> Class\Name class. The way to check does not change. >>>>>>>>> >>>>>>>>> Regards >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Christophe | Stof >>>>>>>>> >>>>>>>>> -- >>>>>>>>> If you want to report a vulnerability issue on symfony, please send >>>>>>>>> it to security at symfony-project.com >>>>>>>>> >>>>>>>>> You received this message because you are subscribed to the Google >>>>>>>>> Groups "symfony users" group. >>>>>>>>> To post to this group, send email to >>>>>>>>> [email protected] >>>>>>>>> To unsubscribe from this group, send email to >>>>>>>>> [email protected] >>>>>>>>> For more options, visit this group at >>>>>>>>> http://groups.google.com/group/symfony-users?hl=en >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> -- >>>>>>> If you want to report a vulnerability issue on symfony, please send >>>>>>> it to security at symfony-project.com >>>>>>> >>>>>>> You received this message because you are subscribed to the Google >>>>>>> Groups "symfony users" group. >>>>>>> To post to this group, send email to [email protected] >>>>>>> To unsubscribe from this group, send email to >>>>>>> [email protected] >>>>>>> For more options, visit this group at >>>>>>> http://groups.google.com/group/symfony-users?hl=en >>>>>>> >>>>>> >>>>>> -- >>>>>> If you want to report a vulnerability issue on symfony, please send it >>>>>> to security at symfony-project.com >>>>>> >>>>>> You received this message because you are subscribed to the Google >>>>>> Groups "symfony users" group. >>>>>> To post to this group, send email to [email protected] >>>>>> To unsubscribe from this group, send email to >>>>>> [email protected] >>>>>> For more options, visit this group at >>>>>> http://groups.google.com/group/symfony-users?hl=en >>>>>> >>>>> >>>>> >>>> >>> >> -- >> If you want to report a vulnerability issue on symfony, please send it to >> security at symfony-project.com >> >> You received this message because you are subscribed to the Google >> Groups "symfony users" group. >> To post to this group, send email to [email protected] >> To unsubscribe from this group, send email to >> [email protected] >> For more options, visit this group at >> http://groups.google.com/group/symfony-users?hl=en >> > > -- > If you want to report a vulnerability issue on symfony, please send it to > security at symfony-project.com > > You received this message because you are subscribed to the Google > Groups "symfony users" group. > To post to this group, send email to [email protected] > To unsubscribe from this group, send email to > [email protected] > For more options, visit this group at > http://groups.google.com/group/symfony-users?hl=en > -- If you want to report a vulnerability issue on symfony, please send it to security at symfony-project.com You received this message because you are subscribed to the Google Groups "symfony users" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/symfony-users?hl=en
