Hi Ian,
I could see them when I logged in. Did you checked whether you logged in to
the google project? When I was not logged in I was not able to see my
comments also.
But in case if it is still not viewable, I am copying the 3 comments I
added.
*
Ian Boston* 2013/09/18 08:18:57
I think this should be getPrivilegeFromACE. READ, WRITE, DELETE are
privileges
*Reply* *Done*
*(Draft)* 2013/09/18 18:36:19
As per explanation of yours in the mail, read=0x01,write=0x02 and etc are
categorized under Principals. Can you confirm that? On 2013/09/18 08:18:57,
Ian Boston wrote:
Show quoted text
*Edit
*
*Ian Boston* 2013/09/18 08:18:57
I think this impl might be wrong ? Principals are things like ieb,
dishara, I cant see how you could represent those in an int. I think it
should be boolean isUserHasPrincipal(String principal, Set<String>
userPrincipals) { return userPrincipals.contains(principal); }
*Reply* *Done*
*(Draft)* 2013/09/18 18:43:48
According to the mail ACE is something like
<order>_<principal>_<allow|deny> where i.e 0_everyone_allow : 0x01. As per
you, user name is the "principle" ? Initially I did it in that way, but I
thought principle is read, write etc as per last comment of mine, and hence
the values of it can be represent in a integer bitmap. But if READ,WRITE
are priviledges my impl is wrong. Can you please confirm what I think is
correct or not ? On 2013/09/18 08:18:57, Ian Boston wrote:
Show quoted text
*Edit
*
*Ian Boston* 2013/09/18 08:18:57
the int userPrinciples should be a Set<String> userPrincipals
*Reply* *Done*
*(Draft)* 2013/09/19 21:29:51
Can you provide an example set of principals. According to me, it can be
the values of i.e read=0x01, write=0x02 so the set should be <0x01> <0x02>
or a single integer 0x03 which represents above. But as per you I am wrong.
So please correct me. On 2013/09/18 08:18:57, Ian Boston wrote:
Show quoted text
*Edit
**
*
On Fri, Sep 20, 2013 at 4:44 PM, Ian Boston <[email protected]> wrote:
> 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
>
--
Thanks
/Dishara
