Re: What's the contract of QueryBuilder.impersonates(String name)?

2017-04-05 Thread Davide Giannella
On 04/04/2017 13:42, Angela Schreiber wrote:
> and what do we gain with that? except for the fact that api consumers have
> to create an Principal instance from a name?

I thought that the consumer already had the Principal at hand and from
there extracting the name rather than having just a name as a String. It
would then make sense to have the String exposed method.

However I think we should act in two directions: the first as you
pointed out the javadoc improvement and the second, IMO, it should be
the QueryEngine responsible to escape the string where and when needed.

Davide




Re: What's the contract of QueryBuilder.impersonates(String name)?

2017-04-04 Thread Manfred Baedke

hi

and what do we gain with that? except for the fact that api consumers have
to create an Principal instance from a name?
not sure if that makes sense... i'd rather just clarify the API contract
in the javadoc.

angela


You're right. I didn't realize that the intended overload would have the 
same signature.





Re: What's the contract of QueryBuilder.impersonates(String name)?

2017-04-04 Thread Angela Schreiber
hi

and what do we gain with that? except for the fact that api consumers have
to create an Principal instance from a name?
not sure if that makes sense... i'd rather just clarify the API contract
in the javadoc.

angela

On 04/04/17 14:32, "Davide Giannella"  wrote:

>On 04/04/2017 11:05, Manfred Baedke wrote:
>> Yes, but now we'd break all existing implementations.
>
>True. We could though, create an overload that receives the principal
>and deprecate the other method for the 1.8 timeframe. By 1.10 we delete
>the deprecated method.
>
>D.
>
>



Re: What's the contract of QueryBuilder.impersonates(String name)?

2017-04-04 Thread Manfred Baedke

On 4/4/2017 2:32 PM, Davide Giannella wrote:

On 04/04/2017 11:05, Manfred Baedke wrote:

Yes, but now we'd break all existing implementations.

True. We could though, create an overload that receives the principal
and deprecate the other method for the 1.8 timeframe. By 1.10 we delete
the deprecated method.

Sensible.


Re: What's the contract of QueryBuilder.impersonates(String name)?

2017-04-04 Thread Davide Giannella
On 04/04/2017 11:05, Manfred Baedke wrote:
> Yes, but now we'd break all existing implementations. 

True. We could though, create an overload that receives the principal
and deprecate the other method for the 1.8 timeframe. By 1.10 we delete
the deprecated method.

D.




Re: What's the contract of QueryBuilder.impersonates(String name)?

2017-04-04 Thread Manfred Baedke

Yes, but now we'd break all existing implementations.

On 4/4/2017 11:09 AM, Davide Giannella wrote:

On 03/04/2017 15:49, Michael Dürig wrote:

Confirmed, this is principle name. At least this is what it was built
for in Jackrabbit 2. The string passed is escaped vis
org.apache.jackrabbit.oak.commons.QueryUtils#escapeForQuery

Just for the sake of discussion. Wouldn't it be better to just pass in
the Principal itself and let the QueryBuilder implementation to deal
with all the requirements of the actual QueryBuilder itself? Escaping
and what ever other things it may be fancying of?

D.






Re: What's the contract of QueryBuilder.impersonates(String name)?

2017-04-04 Thread Davide Giannella
On 03/04/2017 15:49, Michael Dürig wrote:
> Confirmed, this is principle name. At least this is what it was built
> for in Jackrabbit 2. The string passed is escaped vis
> org.apache.jackrabbit.oak.commons.QueryUtils#escapeForQuery

Just for the sake of discussion. Wouldn't it be better to just pass in
the Principal itself and let the QueryBuilder implementation to deal
with all the requirements of the actual QueryBuilder itself? Escaping
and what ever other things it may be fancying of?

D.




Re: What's the contract of QueryBuilder.impersonates(String name)?

2017-04-03 Thread Michael Dürig


Confirmed, this is principle name. At least this is what it was built 
for in Jackrabbit 2. The string passed is escaped vis 
org.apache.jackrabbit.oak.commons.QueryUtils#escapeForQuery




Michael

On 03.04.17 16:36, Angela Schreiber wrote:

Hi

I don't know how Michael intended it to work originally. Given the fact
that the impersonation setup is established and evaluated using principals
I would expect it to be a principal name, which in the default
implementation just can be any string value.

Kind regards
Angela

On 03/04/17 16:14, "Manfred Baedke"  wrote:


Hi all,

Can anyone clarify the contract of the method
org.apache.jackrabbit.api.security.user.QueryBuilder.impersonate(String
name)?
According to the JavaDoc, the parameter is the "name of an
authorizable". But the interface
org.apache.jackrabbit.api.security.user.Authorizable doesn't have a
name, just an id and a principal (which in turn has a name).
If a principal name is expected here (which seems to be the case
according to the implementations), then it needs to be specified if the
caller has to do any necessary escaping: if the user in question is e.g.
an LDAP user, it's principal name may contain backslash characters.

Best regards,
Manfred




Re: What's the contract of QueryBuilder.impersonates(String name)?

2017-04-03 Thread Manfred Baedke

Hi Angela,

Thanks. So we would expect the caller to do the escaping, wouldn't we?

Best regards,
Manfred

On 4/3/2017 4:36 PM, Angela Schreiber wrote:

Hi

I don't know how Michael intended it to work originally. Given the fact
that the impersonation setup is established and evaluated using principals
I would expect it to be a principal name, which in the default
implementation just can be any string value.

Kind regards
Angela

On 03/04/17 16:14, "Manfred Baedke"  wrote:


Hi all,

Can anyone clarify the contract of the method
org.apache.jackrabbit.api.security.user.QueryBuilder.impersonate(String
name)?
According to the JavaDoc, the parameter is the "name of an
authorizable". But the interface
org.apache.jackrabbit.api.security.user.Authorizable doesn't have a
name, just an id and a principal (which in turn has a name).
If a principal name is expected here (which seems to be the case
according to the implementations), then it needs to be specified if the
caller has to do any necessary escaping: if the user in question is e.g.
an LDAP user, it's principal name may contain backslash characters.

Best regards,
Manfred




Re: What's the contract of QueryBuilder.impersonates(String name)?

2017-04-03 Thread Angela Schreiber
Hi 

I don't know how Michael intended it to work originally. Given the fact
that the impersonation setup is established and evaluated using principals
I would expect it to be a principal name, which in the default
implementation just can be any string value.

Kind regards
Angela

On 03/04/17 16:14, "Manfred Baedke"  wrote:

>Hi all,
>
>Can anyone clarify the contract of the method
>org.apache.jackrabbit.api.security.user.QueryBuilder.impersonate(String
>name)?
>According to the JavaDoc, the parameter is the "name of an
>authorizable". But the interface
>org.apache.jackrabbit.api.security.user.Authorizable doesn't have a
>name, just an id and a principal (which in turn has a name).
>If a principal name is expected here (which seems to be the case
>according to the implementations), then it needs to be specified if the
>caller has to do any necessary escaping: if the user in question is e.g.
>an LDAP user, it's principal name may contain backslash characters.
>
>Best regards,
>Manfred