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
