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

[Manfred Baedke <ma...@gmail.com>]

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)?

[Manfred Baedke <ma...@gmail.com>]

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" <ma...@gmail.com> 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)?

[Manfred Baedke <ma...@gmail.com>]

> 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)?

[Davide Giannella <da...@apache.org>]

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)?

[Angela Schreiber <an...@adobe.com>]

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" <da...@apache.org> 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)?

[Manfred Baedke <ma...@gmail.com>]

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)?

[Davide Giannella <da...@apache.org>]

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)?

[Manfred Baedke <ma...@gmail.com>]

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 Drig 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)?

[Davide Giannella <da...@apache.org>]

On 03/04/2017 15:49, Michael Drig 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)?

[Michael Dürig <md...@apache.org>]

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" <ma...@gmail.com> 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)?

[Angela Schreiber <an...@adobe.com>]

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" <ma...@gmail.com> 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