Hi, Did you publish your replies to my comments at [1]. I cant see anything other than what I said. Be sure to click on each comment to exand it to full size.
Ian 1 https://codereview.appspot.com/13396052/diff/1/main/cassandra/src/main/java/org/apache/sling/cassandra/resource/provider/security/AccessControlUtil.java On 19 September 2013 22:38, Dishara Wijewardana <[email protected]> wrote: > Hi Ian, > If you find some time, appreciate if you can add some reply to the comments > on the code review basically regarding the issues you raised in the > algorithm and the conflict of principals and privileges as per my > understanding. I will cleanup the code with your clarification in this > weekend. > > > On Thu, Sep 19, 2013 at 12:16 AM, Dishara Wijewardana < > [email protected]> wrote: > >> Hi Ian, >> Thank you very much for the valuable feedbacks as always. I added some >> comments to clarify couple of cases. Appreciate your response, so that I >> can proceed with the improvements. >> >> >> On Wed, Sep 18, 2013 at 1:19 PM, Ian Boston <[email protected]> wrote: >> >>> Hi Dishara, >>> Looking good, I have some comments, so I did a code review at [1] >>> Ian >>> >>> 1 https://codereview.appspot.com/13396052/ >>> >>> >>> On 18 September 2013 04:13, Dishara Wijewardana <[email protected]> >>> wrote: >>> > Hi Ian, >>> > Sorry for the delay of updating the thread. I had to to some >>> > experiment(writing dummy tests iteratively) to figure out what you >>> exactly >>> > meant. And finally was able to implement what you said. I have commited >>> the >>> > src under a new package called "security". Currently it is a util class >>> > which is executable and we can test. >>> > >>> > Now i.e following can be done and returns accurate results. >>> > >>> > String path = "/content/cassandra/p2/c2"; >>> > String policy="0_dishara_allow :0x01 "; >>> > >>> > accessControlUtil.addACE(path,policy); >>> > >>> > int results[] = accessControlUtil.buildAtLevel(0x04,path); >>> > System.out.println("GRANT" +results[0]); >>> > System.out.println("DENY" +results[1]); >>> > >>> > >>> > >>> > >>> > >>> > On Mon, Sep 16, 2013 at 2:28 PM, Ian Boston <[email protected]> wrote: >>> > >>> >> Hi, >>> >> >>> >> Yes you could store the ACL (ordered list of ACE's) with the resource >>> >> itself, although you will then have to add additional code to protect >>> >> access to that property which will complicate the CassandraProvider, >>> >> which is why I was sugesting that you do ACL storage in a completely >>> >> separate Column Family. >>> >> >>> >> If you choose to do it that way (storing ACLs with the resource), then >>> >> you might consider storing the ACL as a JSON string. >>> >> >>> >> By default, permissions inherit from the permissions on the parent >>> >> resource, so there is no need to create an ACL on creation, but the >>> >> implementation of the ResourceAccessGate will need compute access >>> >> control including parent ACLs. >>> >> >>> >> How you implement ACL crud is entirely upto you. I suggest you use >>> >> Hector directly to do this. >>> >> >>> >> This will make your CassandraResourceAccessGate completely separate >>> >> from you CassandraResourceProvider. >>> >> >>> >> Best Regards >>> >> Ian >>> >> >>> >> On 13 September 2013 13:21, Dishara Wijewardana < >>> [email protected]> >>> >> wrote: >>> >> > On Thu, Sep 12, 2013 at 8:37 PM, Ian Boston <[email protected]> wrote: >>> >> > >>> >> >> Hi >>> >> >> >>> >> >> On 12 September 2013 13:24, Dishara Wijewardana < >>> >> [email protected]> >>> >> >> wrote: >>> >> >> > Hi Ian >>> >> >> > >>> >> >> > >>> >> >> > On Thu, Sep 12, 2013 at 1:50 PM, Ian Boston <[email protected]> >>> wrote: >>> >> >> > >>> >> >> >> Hi Dishara, >>> >> >> >> To make the Cassandra Resource Provider really useful I think we >>> need >>> >> >> >> to add access control. I think the best way of doing this is to >>> >> borrow >>> >> >> >> some concepts from Jackrabbit access control. >>> >> >> >> >>> >> >> >> >>> >> >> > The following algorithms, and etc does sling already have any >>> >> >> > implementation of it. If so I can reuse them. Since sling has few >>> >> >> providers >>> >> >> > I believe they probably have some common interface. >>> >> >> >>> >> >> >>> >> >> >>> >> >> There is not, or at least, not in a way that is exposed. There is an >>> >> >> implementation inside Jackrabbit, but it is tightly coupled to >>> >> >> Jackrabbit and more complex than the simple system below. >>> >> >> >>> >> >> Once implemented, this access control system may be exposed as a >>> >> >> ResourceAccessGate, but since (IIRC) imposing anything other than >>> read >>> >> >> access control has not been done, the CassandraResourceProvider may >>> >> >> use the implementation directly to impose write and delete. >>> >> >> >>> >> >> > >>> >> >> > >>> >> >> >> Take a deep breath, and you will see why I left this till last. >>> >> >> >> >>> >> >> >> I think we should provide path base access control, which >>> inherits >>> >> >> >> from parent resources in the path. At every level there is a an >>> >> >> >> ordered list of access control entries each access control entry >>> >> (ACE) >>> >> >> >> being either an allow entry or a deny entry. What is allowed or >>> >> denied >>> >> >> >> is defined in a 32bit bitmap with each bit representing 1 >>> permission, >>> >> >> >> so we can have upto 32 permissions. Each ACE specifies a single >>> >> >> >> principal. So an ACL consists of a ordered list of ACE's each one >>> >> >> >> bound to a principal. >>> >> >> >> >>> >> >> >> A user has a set of principals, so to resolve the ACL at any one >>> path >>> >> >> >> for a user the global ACL is filtered to contain only the ACE's >>> with >>> >> >> >> principals that the user has. >>> >> >> >> >>> >> >> >> Computing a final access control bitmap for a user at a location >>> >> >> >> requires ordered processing of all the ACEs relevant to the user >>> at >>> >> >> >> the current path and then all ancestors. >>> >> >> >> >>> >> >> >> The pseudo algorithm to calculate the a grant bitmap and a deny >>> >> bitmap >>> >> >> >> at any level is: >>> >> >> >> >>> >> >> >> function getCurrentLevelBitmaps(currentPath): >>> >> >> >> int grants = 0; >>> >> >> >> int denies = 0; >>> >> >> >> for all ACEs in the ACL at the currentPath: >>> >> >> >> if the user has the principal of the current ACE: >>> >> >> >> int toGrant = 0; >>> >> >> >> int toDeny = 0; >>> >> >> >> if the ACE is a grant: >>> >> >> >> toGrant = the ACE bitmap; >>> >> >> >> else: >>> >> >> >> toDeny = the ACE bitmap; >>> >> >> >> toGrant = toGrant & ~denies; >>> >> >> >> toDeny = toDeny & ~grants; >>> >> >> >> grants = grants | toGrant; >>> >> >> >> denied = denies | toDenies; >>> >> >> >> return (grants, denies); >>> >> >> >> >>> >> >> > >>> >> >> > - Can you please tell me how to calculate the ACE bitmap ? >>> >> >> >>> >> >> That is just the 32bit integer stored in the cassandra column >>> >> >> representing the grant or deny for the principal >>> >> >> >>> >> >> eg >>> >> >> Cassandra rowID : base64(sha1(/content/cassandra/foo/bar )) >>> >> >> Columns: 0_everyone_allow : 0x01, 1_admin_allow : 0x07 >>> >> >> >>> >> >> Allows everyone read and admin everything at the path /cassandra >>> >> >> >>> >> >> > - Also I will be more clear if you can provide a sample value for >>> the >>> >> >> input >>> >> >> > and output of this function ? i.e When currentPath= >>> >> >> > /content/cassandra/foo/bar it returns grants=? denies=? some >>> actual >>> >> >> values >>> >> >> > just for my understanding. >>> >> >> >>> >> >> Using the above for a random user (ie has the everyone principal): >>> >> >> grants = 0x01 >>> >> >> denies - 0x0 >>> >> >> >>> >> >> For the following values >>> >> >> Cassandra rowID : base64(sha1(/content/cassandra/foo/bar )) >>> >> >> Columns: 0_everyone_allow : 0x01, 1_admin_allow : 0x07, 2_ieb_deny : >>> >> 0x01 >>> >> >> >>> >> >> results in >>> >> >> dishara : grants = 0x01, denies = 0x00 >>> >> >> ieb : grants = 0x00, denies = 0x01 >>> >> >> admin: grants = 0x07, denies = 0x00 >>> >> >> >>> >> >> >>> >> >> >>> >> >> > >>> >> >> > >>> >> >> >> To combine what is granted at the child level with what is >>> granted at >>> >> >> >> a parent level we need to mask the parent level with the deny at >>> the >>> >> >> >> child level. >>> >> >> >> >>> >> >> >> eg >>> >> >> >> toGrant = grantedAtParent & ~denies; >>> >> >> >> toDeny = deniedAtParent & ~grants; >>> >> >> >> grants = grants | toGrant; >>> >> >> >> denied = denies | toDenies; >>> >> >> >> >>> >> >> >> The simplest way of achieving this is to use recursion again in >>> >> pseudo >>> >> >> >> code: >>> >> >> >> >>> >> >> >> function buildAtLevel(): >>> >> >> >> if not root level: >>> >> >> >> (grantedAtParent, deniedAtParent) = >>> >> >> >> buildAtLevel(getParentLevel(currentLevel)); >>> >> >> >> (grants, denies) = >>> getCurrentLevelBitmaps(currentLevel); >>> >> >> >> toGrant = grantedAtParent & ~denies; >>> >> >> >> toDeny = deniedAtParent & ~grants; >>> >> >> >> grants = grants | toGrant; >>> >> >> >> denied = denies | toDenies; >>> >> >> >> return (grants, denied); >>> >> >> >> >>> >> >> >> >>> >> >> >> There are some optimisations you can apply here, and there are >>> plenty >>> >> >> >> of opportunities to cache intermediate bitmaps in memory. Just >>> >> caching >>> >> >> >> the ACL reduces resolution to bitwise operations. >>> >> >> >> >>> >> >> >> Principals >>> >> >> >> ---------------- >>> >> >> >> Initially keep it simple. >>> >> >> >> >>> >> >> >> read = 0x01 >>> >> >> >> write = 0x02 >>> >> >> >> delete = 0x04 >>> >> >> >> >>> >> >> >> Storage of ACLs. >>> >> >> >> ------------------------- >>> >> >> >> I suggest you store ACLs in their own Column Family, where the >>> rowID >>> >> is >>> >> >> >> base64(sha1(path)) or whatever path -> rowid encoding you have >>> >> >> currently. >>> >> >> >> >>> >> >> >> IIRC Cassandra columns come out in the natural order of Strings >>> >> >> >> <order>_<principal>_<allow|deny> and the value is the bitmap of >>> >> >> >> permissions. >>> >> >> >> >>> >> >> >> - If I understand you correctly is >>> >> <order>_<principal>_<allow|deny> is >>> >> >> > one ACE ? >>> >> >> >>> >> >> yes >>> >> >> >>> >> >> If per row there can be a ACL, there should be one additional >>> >> >> > column by default called "ACL" and it will have a comma separated >>> >> string >>> >> >> > which are set of ACEs. >>> >> >> >>> >> >> not necessary as the order of columns is returned in the natural >>> >> >> string sort order (IIRC) so 0_* will be the first followed by 1_ >>> >> >> >>> >> >> However, if thats not the case a column called ACL will be required >>> to >>> >> >> list the ACLs in order. >>> >> >> >>> >> >> Correct me if I am wrong. >>> >> >> > - Who stores these ACEs ? any API? >>> >> >> >>> >> >> Not determined yet. It would need an API able to modify ACLs >>> >> >> >>> >> > >>> >> > OK, I am trying to understand. Can you please clarify following. >>> >> > >>> >> > To start with, as I feel I should start storing data to cassandra >>> with an >>> >> > updated column which stores a string which is a comma separated ACE >>> list. >>> >> > i.e when the provider stores a resource at >>> /content/cassandra/foo/bar >>> >> the >>> >> > ACL column can be empty. Does permissions added upon node creation ? >>> Or >>> >> > there is some interface,and when it get called, (path,ACE-list) I >>> will >>> >> > update the row with the new value for the ACL column. If that is the >>> case >>> >> > permission should be granted to a path only of the path exists. OR >>> upon >>> >> > node creation does it suppose to store parent resolved ACE list by >>> >> > default. If that is so it will be more complex for Provider and have >>> to >>> >> > change that to do so. >>> >> > >>> >> > >>> >> > >>> >> >> > - i.e <order> is a auto increment number we have to do a >>> additional >>> >> read >>> >> >> > before storing ACE to check what is the last number for <order>. >>> >> >> >>> >> >> >>> >> >> Ok, that indicates having an ACE column might be better, which would >>> >> >> indicate that no count is required. >>> >> >> >>> >> >> Best Regards >>> >> >> Ian >>> >> >> >>> >> >> > >>> >> >> > Where <order> is 000 to 999 ( I really doubt that a single ACL >>> will >>> >> >> >> have 1000 ACEs ever) >>> >> >> >> >>> >> >> >> Once you have this working, we can wire it into the >>> ResourceProvider >>> >> >> >> or another Sling API. >>> >> >> >> >>> >> >> >> Does that make sense ? >>> >> >> >> Ian >>> >> >> >> >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > -- >>> >> >> > Thanks >>> >> >> > /Dishara >>> >> >> >>> >> > >>> >> > >>> >> > >>> >> > -- >>> >> > Thanks >>> >> > /Dishara >>> >> >>> > >>> > >>> > >>> > -- >>> > Thanks >>> > /Dishara >>> >> >> >> >> -- >> Thanks >> /Dishara >> > > > > -- > Thanks > /Dishara
