Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Ian Boston]

On 24 August 2017 at 14:42, Michael Dürig  wrote:

>
>
> On 24.08.17 15:33, Chetan Mehrotra wrote:
>
>> Inside Oak it would have its own version of an AdapterManager,
>>> AdapterFactory. the DataStore would implement an AdapterFactory and
>>> register it with the AdapterManager. The OakConversionService
>>> implementation would then use the AdapterManager to perform the
>>> conversion.
>>> If no AdapterFactory to adapt from JCR Binary to URI existed, then null
>>> would be returned from the OakConversionService.
>>>
>>> Thats no API changes to Blob, binary or anything. No complex
>>> transformation
>>> through multiple layers. No instanceof required and no difference between
>>> Sling and non Sling usage.
>>> It does require an Oak version of the AdapterManager and AdapterFactory
>>> concepts, but does not require anything to implement Adaptable.
>>>
>>
>> Thanks for those details. Much clear now. So with this we need not add
>> adaptTo to all stuff. Instead provide an OakConversionService which
>> converts the Binary to provided type and then have DataStores provide
>> the AdapterFactory.
>>
>> This would indeed avoid any new methods in existing objects and
>> provide a single entry point.
>>
>> +1 for this approach
>>
>
> Yay!



Rough quick PoC at [1] to ilustrate how it might work.

1
https://github.com/apache/jackrabbit-oak/compare/trunk...ieb:OAK-6575?expand=1


>
>
> Michael
>
>
>
>> Chetan Mehrotra
>>
>>
>> On Thu, Aug 24, 2017 at 6:16 AM, Ian Boston  wrote:
>>
>>> Hi,
>>> I am probably not helping as here as there are several layers and I think
>>> they are getting confused between what I am thinking and what you are
>>> thinking.
>>>
>>> I was thinking Oak exposed a service to convert along the lines of the
>>> OSCi
>>> converter service or the OakConversionService suggested earlier. Both
>>> Sling
>>> and other uses of Oak would use it.
>>>
>>> Inside Oak it would have its own version of an AdapterManager,
>>> AdapterFactory. the DataStore would implement an AdapterFactory and
>>> register it with the AdapterManager. The OakConversionService
>>> implementation would then use the AdapterManager to perform the
>>> conversion.
>>> If no AdapterFactory to adapt from JCR Binary to URI existed, then null
>>> would be returned from the OakConversionService.
>>>
>>> Thats no API changes to Blob, binary or anything. No complex
>>> transformation
>>> through multiple layers. No instanceof required and no difference between
>>> Sling and non Sling usage.
>>> It does require an Oak version of the AdapterManager and AdapterFactory
>>> concepts, but does not require anything to implement Adaptable.
>>>
>>> As I showed in the PoC, all the S3 specific implementation fits inside
>>> the
>>> S3DataStore which already does everything required to perform the
>>> conversion. It already goes from Binary -> Blob -> ContentIdentifier ->
>>> S3
>>> Key -> S3 URL by virtue of
>>> ValueImpl.getBlob((Value)jcrBinary).getContentIdentifier() -> convert to
>>> S3key and then signed URL.
>>>
>>> If it would help, I can do a patch to show how it works.
>>> Best Regards
>>> Ian
>>>
>>> On 24 August 2017 at 13:05, Chetan Mehrotra 
>>> wrote:
>>>
>>> No API changes to any existing Oak APIs,
>

 Some API needs to be exposed. Note again Oak does not depend on Sling
 API. Any such integration code is implemented in Sling Base module
 [1]. But that module would still require some API in Oak to provide
 such an adaptor

 The adaptor proposal here is for enabling layers within Oak to allow
 conversion of JCR Binary instance to SignedBinary. Now how this is
 exposed to end user depends on usage context

 Outside Sling
 --

 Check if binary instanceof Oak Adaptable. If yes then cast it and adapt
 it

 import org.apache.jackrabbit.oak.api.Adaptable

 Binary b = ...
 SignedBinary sb  = null
 if (b instanceof Adaptable) {
 sb = ((Adaptable)b).adaptTo(SignedBinary.class);
 }

>>>
>>>
>>> Within Sling
 

 Have an AdapterManager implemented in Sling JCR Base [1] which uses
 above approach

 Chetan Mehrotra
 [1] https://github.com/apache/sling/tree/trunk/bundles/jcr/base


 On Thu, Aug 24, 2017 at 4:55 AM, Ian Boston  wrote:

>  From the javadoc in [1]
>
> "The adaptable object may be any non-null object and is not required to
> implement the Adaptable interface."
>
>
> On 24 August 2017 at 12:54, Ian Boston  wrote:
>
> Hi,
>> That would require javax.jcr.Binary to implement Adaptable, which it
>>
> cant.

> (OakBinary could but it doesnt need to).
>>
>> Using Sling AdapterFactory/AdapterManger javadoc (to be replaced with
>>
> Oaks

> internal version of the same)
>>
>> What is needed is 

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Michael Dürig]

On 24.08.17 15:33, Chetan Mehrotra wrote:

Inside Oak it would have its own version of an AdapterManager,
AdapterFactory. the DataStore would implement an AdapterFactory and
register it with the AdapterManager. The OakConversionService
implementation would then use the AdapterManager to perform the conversion.
If no AdapterFactory to adapt from JCR Binary to URI existed, then null
would be returned from the OakConversionService.

Thats no API changes to Blob, binary or anything. No complex transformation
through multiple layers. No instanceof required and no difference between
Sling and non Sling usage.
It does require an Oak version of the AdapterManager and AdapterFactory
concepts, but does not require anything to implement Adaptable.


Thanks for those details. Much clear now. So with this we need not add
adaptTo to all stuff. Instead provide an OakConversionService which
converts the Binary to provided type and then have DataStores provide
the AdapterFactory.

This would indeed avoid any new methods in existing objects and
provide a single entry point.

+1 for this approach


Yay!

Michael



Chetan Mehrotra


On Thu, Aug 24, 2017 at 6:16 AM, Ian Boston  wrote:

Hi,
I am probably not helping as here as there are several layers and I think
they are getting confused between what I am thinking and what you are
thinking.

I was thinking Oak exposed a service to convert along the lines of the OSCi
converter service or the OakConversionService suggested earlier. Both Sling
and other uses of Oak would use it.

Inside Oak it would have its own version of an AdapterManager,
AdapterFactory. the DataStore would implement an AdapterFactory and
register it with the AdapterManager. The OakConversionService
implementation would then use the AdapterManager to perform the conversion.
If no AdapterFactory to adapt from JCR Binary to URI existed, then null
would be returned from the OakConversionService.

Thats no API changes to Blob, binary or anything. No complex transformation
through multiple layers. No instanceof required and no difference between
Sling and non Sling usage.
It does require an Oak version of the AdapterManager and AdapterFactory
concepts, but does not require anything to implement Adaptable.

As I showed in the PoC, all the S3 specific implementation fits inside the
S3DataStore which already does everything required to perform the
conversion. It already goes from Binary -> Blob -> ContentIdentifier -> S3
Key -> S3 URL by virtue of
ValueImpl.getBlob((Value)jcrBinary).getContentIdentifier() -> convert to
S3key and then signed URL.

If it would help, I can do a patch to show how it works.
Best Regards
Ian

On 24 August 2017 at 13:05, Chetan Mehrotra 
wrote:


No API changes to any existing Oak APIs,


Some API needs to be exposed. Note again Oak does not depend on Sling
API. Any such integration code is implemented in Sling Base module
[1]. But that module would still require some API in Oak to provide
such an adaptor

The adaptor proposal here is for enabling layers within Oak to allow
conversion of JCR Binary instance to SignedBinary. Now how this is
exposed to end user depends on usage context

Outside Sling
--

Check if binary instanceof Oak Adaptable. If yes then cast it and adapt it

import org.apache.jackrabbit.oak.api.Adaptable

Binary b = ...
SignedBinary sb  = null
if (b instanceof Adaptable) {
sb = ((Adaptable)b).adaptTo(SignedBinary.class);
}




Within Sling


Have an AdapterManager implemented in Sling JCR Base [1] which uses
above approach

Chetan Mehrotra
[1] https://github.com/apache/sling/tree/trunk/bundles/jcr/base


On Thu, Aug 24, 2017 at 4:55 AM, Ian Boston  wrote:

 From the javadoc in [1]

"The adaptable object may be any non-null object and is not required to
implement the Adaptable interface."


On 24 August 2017 at 12:54, Ian Boston  wrote:


Hi,
That would require javax.jcr.Binary to implement Adaptable, which it

cant.

(OakBinary could but it doesnt need to).

Using Sling AdapterFactory/AdapterManger javadoc (to be replaced with

Oaks

internal version of the same)

What is needed is an AdapterFactory for javax.jcr.Binary to SignedBinary
provided by the S3DataStore itself.

Since javax.jcr.Binary cant extend Adaptable, its not possible to call
binary.adaptTo(SignedBinary.class) without a cast, hence,
the call is done via the AdapterManager[1]

SignedBinary signedBinary =  adapterManager.getAdapter(binary,
SignedBinary.class);

---
You could just jump to
URI uri =  adapterManager.getAdapter(binary, URI.class);

No API changes to any existing Oak APIs,

Best Regards
Ian


1 https://sling.apache.org/apidocs/sling5/org/apache/sling/api/adapter/
AdapterManager.html



On 24 August 2017 at 12:38, Chetan Mehrotra 
wrote:


various layers involved. The bit I don't understand is how the

adaptable

pattern would make those go away. To me that 

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Chetan Mehrotra]

> Inside Oak it would have its own version of an AdapterManager,
> AdapterFactory. the DataStore would implement an AdapterFactory and
> register it with the AdapterManager. The OakConversionService
> implementation would then use the AdapterManager to perform the conversion.
> If no AdapterFactory to adapt from JCR Binary to URI existed, then null
> would be returned from the OakConversionService.
>
> Thats no API changes to Blob, binary or anything. No complex transformation
> through multiple layers. No instanceof required and no difference between
> Sling and non Sling usage.
> It does require an Oak version of the AdapterManager and AdapterFactory
> concepts, but does not require anything to implement Adaptable.

Thanks for those details. Much clear now. So with this we need not add
adaptTo to all stuff. Instead provide an OakConversionService which
converts the Binary to provided type and then have DataStores provide
the AdapterFactory.

This would indeed avoid any new methods in existing objects and
provide a single entry point.

+1 for this approach

Chetan Mehrotra


On Thu, Aug 24, 2017 at 6:16 AM, Ian Boston  wrote:
> Hi,
> I am probably not helping as here as there are several layers and I think
> they are getting confused between what I am thinking and what you are
> thinking.
>
> I was thinking Oak exposed a service to convert along the lines of the OSCi
> converter service or the OakConversionService suggested earlier. Both Sling
> and other uses of Oak would use it.
>
> Inside Oak it would have its own version of an AdapterManager,
> AdapterFactory. the DataStore would implement an AdapterFactory and
> register it with the AdapterManager. The OakConversionService
> implementation would then use the AdapterManager to perform the conversion.
> If no AdapterFactory to adapt from JCR Binary to URI existed, then null
> would be returned from the OakConversionService.
>
> Thats no API changes to Blob, binary or anything. No complex transformation
> through multiple layers. No instanceof required and no difference between
> Sling and non Sling usage.
> It does require an Oak version of the AdapterManager and AdapterFactory
> concepts, but does not require anything to implement Adaptable.
>
> As I showed in the PoC, all the S3 specific implementation fits inside the
> S3DataStore which already does everything required to perform the
> conversion. It already goes from Binary -> Blob -> ContentIdentifier -> S3
> Key -> S3 URL by virtue of
> ValueImpl.getBlob((Value)jcrBinary).getContentIdentifier() -> convert to
> S3key and then signed URL.
>
> If it would help, I can do a patch to show how it works.
> Best Regards
> Ian
>
> On 24 August 2017 at 13:05, Chetan Mehrotra 
> wrote:
>
>> > No API changes to any existing Oak APIs,
>>
>> Some API needs to be exposed. Note again Oak does not depend on Sling
>> API. Any such integration code is implemented in Sling Base module
>> [1]. But that module would still require some API in Oak to provide
>> such an adaptor
>>
>> The adaptor proposal here is for enabling layers within Oak to allow
>> conversion of JCR Binary instance to SignedBinary. Now how this is
>> exposed to end user depends on usage context
>>
>> Outside Sling
>> --
>>
>> Check if binary instanceof Oak Adaptable. If yes then cast it and adapt it
>>
>> import org.apache.jackrabbit.oak.api.Adaptable
>>
>> Binary b = ...
>> SignedBinary sb  = null
>> if (b instanceof Adaptable) {
>>sb = ((Adaptable)b).adaptTo(SignedBinary.class);
>> }
>
>
>> Within Sling
>> 
>>
>> Have an AdapterManager implemented in Sling JCR Base [1] which uses
>> above approach
>>
>> Chetan Mehrotra
>> [1] https://github.com/apache/sling/tree/trunk/bundles/jcr/base
>>
>>
>> On Thu, Aug 24, 2017 at 4:55 AM, Ian Boston  wrote:
>> > From the javadoc in [1]
>> >
>> > "The adaptable object may be any non-null object and is not required to
>> > implement the Adaptable interface."
>> >
>> >
>> > On 24 August 2017 at 12:54, Ian Boston  wrote:
>> >
>> >> Hi,
>> >> That would require javax.jcr.Binary to implement Adaptable, which it
>> cant.
>> >> (OakBinary could but it doesnt need to).
>> >>
>> >> Using Sling AdapterFactory/AdapterManger javadoc (to be replaced with
>> Oaks
>> >> internal version of the same)
>> >>
>> >> What is needed is an AdapterFactory for javax.jcr.Binary to SignedBinary
>> >> provided by the S3DataStore itself.
>> >>
>> >> Since javax.jcr.Binary cant extend Adaptable, its not possible to call
>> >> binary.adaptTo(SignedBinary.class) without a cast, hence,
>> >> the call is done via the AdapterManager[1]
>> >>
>> >> SignedBinary signedBinary =  adapterManager.getAdapter(binary,
>> >> SignedBinary.class);
>> >>
>> >> ---
>> >> You could just jump to
>> >> URI uri =  adapterManager.getAdapter(binary, URI.class);
>> >>
>> >> No API changes to any existing Oak APIs,
>> >>
>> >> Best Regards
>> >> Ian
>> >>

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Ian Boston]

Hi,
I am probably not helping as here as there are several layers and I think
they are getting confused between what I am thinking and what you are
thinking.

I was thinking Oak exposed a service to convert along the lines of the OSCi
converter service or the OakConversionService suggested earlier. Both Sling
and other uses of Oak would use it.

Inside Oak it would have its own version of an AdapterManager,
AdapterFactory. the DataStore would implement an AdapterFactory and
register it with the AdapterManager. The OakConversionService
implementation would then use the AdapterManager to perform the conversion.
If no AdapterFactory to adapt from JCR Binary to URI existed, then null
would be returned from the OakConversionService.

Thats no API changes to Blob, binary or anything. No complex transformation
through multiple layers. No instanceof required and no difference between
Sling and non Sling usage.
It does require an Oak version of the AdapterManager and AdapterFactory
concepts, but does not require anything to implement Adaptable.

As I showed in the PoC, all the S3 specific implementation fits inside the
S3DataStore which already does everything required to perform the
conversion. It already goes from Binary -> Blob -> ContentIdentifier -> S3
Key -> S3 URL by virtue of
ValueImpl.getBlob((Value)jcrBinary).getContentIdentifier() -> convert to
S3key and then signed URL.

If it would help, I can do a patch to show how it works.
Best Regards
Ian

On 24 August 2017 at 13:05, Chetan Mehrotra 
wrote:

> > No API changes to any existing Oak APIs,
>
> Some API needs to be exposed. Note again Oak does not depend on Sling
> API. Any such integration code is implemented in Sling Base module
> [1]. But that module would still require some API in Oak to provide
> such an adaptor
>
> The adaptor proposal here is for enabling layers within Oak to allow
> conversion of JCR Binary instance to SignedBinary. Now how this is
> exposed to end user depends on usage context
>
> Outside Sling
> --
>
> Check if binary instanceof Oak Adaptable. If yes then cast it and adapt it
>
> import org.apache.jackrabbit.oak.api.Adaptable
>
> Binary b = ...
> SignedBinary sb  = null
> if (b instanceof Adaptable) {
>sb = ((Adaptable)b).adaptTo(SignedBinary.class);
> }


> Within Sling
> 
>
> Have an AdapterManager implemented in Sling JCR Base [1] which uses
> above approach
>
> Chetan Mehrotra
> [1] https://github.com/apache/sling/tree/trunk/bundles/jcr/base
>
>
> On Thu, Aug 24, 2017 at 4:55 AM, Ian Boston  wrote:
> > From the javadoc in [1]
> >
> > "The adaptable object may be any non-null object and is not required to
> > implement the Adaptable interface."
> >
> >
> > On 24 August 2017 at 12:54, Ian Boston  wrote:
> >
> >> Hi,
> >> That would require javax.jcr.Binary to implement Adaptable, which it
> cant.
> >> (OakBinary could but it doesnt need to).
> >>
> >> Using Sling AdapterFactory/AdapterManger javadoc (to be replaced with
> Oaks
> >> internal version of the same)
> >>
> >> What is needed is an AdapterFactory for javax.jcr.Binary to SignedBinary
> >> provided by the S3DataStore itself.
> >>
> >> Since javax.jcr.Binary cant extend Adaptable, its not possible to call
> >> binary.adaptTo(SignedBinary.class) without a cast, hence,
> >> the call is done via the AdapterManager[1]
> >>
> >> SignedBinary signedBinary =  adapterManager.getAdapter(binary,
> >> SignedBinary.class);
> >>
> >> ---
> >> You could just jump to
> >> URI uri =  adapterManager.getAdapter(binary, URI.class);
> >>
> >> No API changes to any existing Oak APIs,
> >>
> >> Best Regards
> >> Ian
> >>
> >>
> >> 1 https://sling.apache.org/apidocs/sling5/org/apache/sling/api/adapter/
> >> AdapterManager.html
> >>
> >>
> >>
> >> On 24 August 2017 at 12:38, Chetan Mehrotra 
> >> wrote:
> >>
> >>> > various layers involved. The bit I don't understand is how the
> adaptable
> >>> > pattern would make those go away. To me that pattern is just another
> >>> way to
> >>> > implement this but it would also need to deal with all those layers.
> >>>
> >>> Yes this adapter support would need to be implement at all layers.
> >>>
> >>> So call to
> >>> 1. binary.adaptTo(SignedBinary.class) //binary is JCR Binary
> >>> 2. results in blob.adaptTo(SignedBinary.class) //blob is Oak Blob.
> >>> Blob interface would extend adaptable
> >>> 3. results in SegmentBlob delegating to BlobStoreBlob which
> >>> 4. delegates to BlobStore // Here just passing the BlobId
> >>> 5. which delegates to DataStoreBlobStore
> >>> 6. which delegates to S3DataStore
> >>> 7. which returns the SignedBinary implementation
> >>>
> >>> However adapter support would allow us to make this instance of check
> >>> extensible. Otherwise we would be hardcoding instance of check to
> >>> SignedBinary at each of the above place though those layers need not
> >>> be aware of SignedBinary support (its 

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Michael Dürig]

On 24.08.17 14:47, Chetan Mehrotra wrote:

Which circles back to my initial concern: "According to YAGNI we should stick with 
instance of checks unless we already have a somewhat clear picture of future 
extensions."


I thought that with all those discussion around JCR Usecases for past
some time we have an agreement for such cases (specially UC3 and UC4).
Hence the push for this approach to enable further work on them going
forward.


I wasn't referring to those use cases but to the adapter pattern. That 
pattern as it was proposed here is far more general than what is 
required for the problem at hand. It represents a shift in paradigm of 
what we used to do in Oak. That's why I asked for a "somewhat clear 
picture of future extensions". Introducing the adapter pattern in its 
full generality would IMO even deserve an own discussion thread instead 
of being piggy backed on the secure URL discussion.


Michael




Chetan Mehrotra


On Thu, Aug 24, 2017 at 5:41 AM, Michael Dürig  wrote:



On 24.08.17 14:32, Chetan Mehrotra wrote:


Why not just add a method Blob.getSignedURI()? This would be inline with
getReference() and what we have done with ReferenceBinary.



Can be done. But later if we decide to support adapting to say
FileChannel [1] then would we be adding that to Blob. Though it may
not be related to different Blob types.

Having adaptable support allows to extend this later with minimal changes.



Which circles back to my initial concern: "According to YAGNI we should
stick with instance of checks unless we already have a somewhat clear
picture of future extensions."

Michael




Chetan Mehrotra
[1] https://wiki.apache.org/jackrabbit/JCR%20Binary%20Usecase#UC4


On Thu, Aug 24, 2017 at 5:25 AM, Michael Dürig  wrote:




On 24.08.17 13:38, Chetan Mehrotra wrote:



various layers involved. The bit I don't understand is how the
adaptable
pattern would make those go away. To me that pattern is just another
way
to
implement this but it would also need to deal with all those layers.




Yes this adapter support would need to be implement at all layers.

So call to
1. binary.adaptTo(SignedBinary.class) //binary is JCR Binary
2. results in blob.adaptTo(SignedBinary.class) //blob is Oak Blob.
Blob interface would extend adaptable




Why not just add a method Blob.getSignedURI()? This would be inline with
getReference() and what we have done with ReferenceBinary.

Michael



3. results in SegmentBlob delegating to BlobStoreBlob which
4. delegates to BlobStore // Here just passing the BlobId
5. which delegates to DataStoreBlobStore
6. which delegates to S3DataStore
7. which returns the SignedBinary implementation

However adapter support would allow us to make this instance of check
extensible. Otherwise we would be hardcoding instance of check to
SignedBinary at each of the above place though those layers need not
be aware of SignedBinary support (its specific to S3 impl)

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Chetan Mehrotra]

> Which circles back to my initial concern: "According to YAGNI we should stick 
> with instance of checks unless we already have a somewhat clear picture of 
> future extensions."

I thought that with all those discussion around JCR Usecases for past
some time we have an agreement for such cases (specially UC3 and UC4).
Hence the push for this approach to enable further work on them going
forward.


Chetan Mehrotra


On Thu, Aug 24, 2017 at 5:41 AM, Michael Dürig  wrote:
>
>
> On 24.08.17 14:32, Chetan Mehrotra wrote:
>>>
>>> Why not just add a method Blob.getSignedURI()? This would be inline with
>>> getReference() and what we have done with ReferenceBinary.
>>
>>
>> Can be done. But later if we decide to support adapting to say
>> FileChannel [1] then would we be adding that to Blob. Though it may
>> not be related to different Blob types.
>>
>> Having adaptable support allows to extend this later with minimal changes.
>
>
> Which circles back to my initial concern: "According to YAGNI we should
> stick with instance of checks unless we already have a somewhat clear
> picture of future extensions."
>
> Michael
>
>
>>
>> Chetan Mehrotra
>> [1] https://wiki.apache.org/jackrabbit/JCR%20Binary%20Usecase#UC4
>>
>>
>> On Thu, Aug 24, 2017 at 5:25 AM, Michael Dürig  wrote:
>>>
>>>
>>>
>>> On 24.08.17 13:38, Chetan Mehrotra wrote:
>
>
> various layers involved. The bit I don't understand is how the
> adaptable
> pattern would make those go away. To me that pattern is just another
> way
> to
> implement this but it would also need to deal with all those layers.



 Yes this adapter support would need to be implement at all layers.

 So call to
 1. binary.adaptTo(SignedBinary.class) //binary is JCR Binary
 2. results in blob.adaptTo(SignedBinary.class) //blob is Oak Blob.
 Blob interface would extend adaptable
>>>
>>>
>>>
>>> Why not just add a method Blob.getSignedURI()? This would be inline with
>>> getReference() and what we have done with ReferenceBinary.
>>>
>>> Michael
>>>
>>>
 3. results in SegmentBlob delegating to BlobStoreBlob which
 4. delegates to BlobStore // Here just passing the BlobId
 5. which delegates to DataStoreBlobStore
 6. which delegates to S3DataStore
 7. which returns the SignedBinary implementation

 However adapter support would allow us to make this instance of check
 extensible. Otherwise we would be hardcoding instance of check to
 SignedBinary at each of the above place though those layers need not
 be aware of SignedBinary support (its specific to S3 impl)
>>>
>>>
>>>
>>>
>

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Michael Dürig]

On 24.08.17 14:32, Chetan Mehrotra wrote:

Why not just add a method Blob.getSignedURI()? This would be inline with 
getReference() and what we have done with ReferenceBinary.


Can be done. But later if we decide to support adapting to say
FileChannel [1] then would we be adding that to Blob. Though it may
not be related to different Blob types.

Having adaptable support allows to extend this later with minimal changes.


Which circles back to my initial concern: "According to YAGNI we should 
stick with instance of checks unless we already have a somewhat clear 
picture of future extensions."


Michael




Chetan Mehrotra
[1] https://wiki.apache.org/jackrabbit/JCR%20Binary%20Usecase#UC4


On Thu, Aug 24, 2017 at 5:25 AM, Michael Dürig  wrote:



On 24.08.17 13:38, Chetan Mehrotra wrote:


various layers involved. The bit I don't understand is how the adaptable
pattern would make those go away. To me that pattern is just another way
to
implement this but it would also need to deal with all those layers.



Yes this adapter support would need to be implement at all layers.

So call to
1. binary.adaptTo(SignedBinary.class) //binary is JCR Binary
2. results in blob.adaptTo(SignedBinary.class) //blob is Oak Blob.
Blob interface would extend adaptable



Why not just add a method Blob.getSignedURI()? This would be inline with
getReference() and what we have done with ReferenceBinary.

Michael



3. results in SegmentBlob delegating to BlobStoreBlob which
4. delegates to BlobStore // Here just passing the BlobId
5. which delegates to DataStoreBlobStore
6. which delegates to S3DataStore
7. which returns the SignedBinary implementation

However adapter support would allow us to make this instance of check
extensible. Otherwise we would be hardcoding instance of check to
SignedBinary at each of the above place though those layers need not
be aware of SignedBinary support (its specific to S3 impl)

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Michael Dürig]

On 24.08.17 13:54, Ian Boston wrote:

You could just jump to
URI uri =  adapterManager.getAdapter(binary, URI.class);

No API changes to any existing Oak APIs,


+1, I think this is what we should aim for.

Michael

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Chetan Mehrotra]

> No API changes to any existing Oak APIs,

Some API needs to be exposed. Note again Oak does not depend on Sling
API. Any such integration code is implemented in Sling Base module
[1]. But that module would still require some API in Oak to provide
such an adaptor

The adaptor proposal here is for enabling layers within Oak to allow
conversion of JCR Binary instance to SignedBinary. Now how this is
exposed to end user depends on usage context

Outside Sling
--

Check if binary instanceof Oak Adaptable. If yes then cast it and adapt it

import org.apache.jackrabbit.oak.api.Adaptable

Binary b = ...
SignedBinary sb  = null
if (b instanceof Adaptable) {
   sb = ((Adaptable)b).adaptTo(SignedBinary.class);
}

Within Sling


Have an AdapterManager implemented in Sling JCR Base [1] which uses
above approach

Chetan Mehrotra
[1] https://github.com/apache/sling/tree/trunk/bundles/jcr/base


On Thu, Aug 24, 2017 at 4:55 AM, Ian Boston  wrote:
> From the javadoc in [1]
>
> "The adaptable object may be any non-null object and is not required to
> implement the Adaptable interface."
>
>
> On 24 August 2017 at 12:54, Ian Boston  wrote:
>
>> Hi,
>> That would require javax.jcr.Binary to implement Adaptable, which it cant.
>> (OakBinary could but it doesnt need to).
>>
>> Using Sling AdapterFactory/AdapterManger javadoc (to be replaced with Oaks
>> internal version of the same)
>>
>> What is needed is an AdapterFactory for javax.jcr.Binary to SignedBinary
>> provided by the S3DataStore itself.
>>
>> Since javax.jcr.Binary cant extend Adaptable, its not possible to call
>> binary.adaptTo(SignedBinary.class) without a cast, hence,
>> the call is done via the AdapterManager[1]
>>
>> SignedBinary signedBinary =  adapterManager.getAdapter(binary,
>> SignedBinary.class);
>>
>> ---
>> You could just jump to
>> URI uri =  adapterManager.getAdapter(binary, URI.class);
>>
>> No API changes to any existing Oak APIs,
>>
>> Best Regards
>> Ian
>>
>>
>> 1 https://sling.apache.org/apidocs/sling5/org/apache/sling/api/adapter/
>> AdapterManager.html
>>
>>
>>
>> On 24 August 2017 at 12:38, Chetan Mehrotra 
>> wrote:
>>
>>> > various layers involved. The bit I don't understand is how the adaptable
>>> > pattern would make those go away. To me that pattern is just another
>>> way to
>>> > implement this but it would also need to deal with all those layers.
>>>
>>> Yes this adapter support would need to be implement at all layers.
>>>
>>> So call to
>>> 1. binary.adaptTo(SignedBinary.class) //binary is JCR Binary
>>> 2. results in blob.adaptTo(SignedBinary.class) //blob is Oak Blob.
>>> Blob interface would extend adaptable
>>> 3. results in SegmentBlob delegating to BlobStoreBlob which
>>> 4. delegates to BlobStore // Here just passing the BlobId
>>> 5. which delegates to DataStoreBlobStore
>>> 6. which delegates to S3DataStore
>>> 7. which returns the SignedBinary implementation
>>>
>>> However adapter support would allow us to make this instance of check
>>> extensible. Otherwise we would be hardcoding instance of check to
>>> SignedBinary at each of the above place though those layers need not
>>> be aware of SignedBinary support (its specific to S3 impl)
>>>
>>> Chetan Mehrotra
>>>
>>
>>

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Ian Boston]

>From the javadoc in [1]

"The adaptable object may be any non-null object and is not required to
implement the Adaptable interface."


On 24 August 2017 at 12:54, Ian Boston  wrote:

> Hi,
> That would require javax.jcr.Binary to implement Adaptable, which it cant.
> (OakBinary could but it doesnt need to).
>
> Using Sling AdapterFactory/AdapterManger javadoc (to be replaced with Oaks
> internal version of the same)
>
> What is needed is an AdapterFactory for javax.jcr.Binary to SignedBinary
> provided by the S3DataStore itself.
>
> Since javax.jcr.Binary cant extend Adaptable, its not possible to call
> binary.adaptTo(SignedBinary.class) without a cast, hence,
> the call is done via the AdapterManager[1]
>
> SignedBinary signedBinary =  adapterManager.getAdapter(binary,
> SignedBinary.class);
>
> ---
> You could just jump to
> URI uri =  adapterManager.getAdapter(binary, URI.class);
>
> No API changes to any existing Oak APIs,
>
> Best Regards
> Ian
>
>
> 1 https://sling.apache.org/apidocs/sling5/org/apache/sling/api/adapter/
> AdapterManager.html
>
>
>
> On 24 August 2017 at 12:38, Chetan Mehrotra 
> wrote:
>
>> > various layers involved. The bit I don't understand is how the adaptable
>> > pattern would make those go away. To me that pattern is just another
>> way to
>> > implement this but it would also need to deal with all those layers.
>>
>> Yes this adapter support would need to be implement at all layers.
>>
>> So call to
>> 1. binary.adaptTo(SignedBinary.class) //binary is JCR Binary
>> 2. results in blob.adaptTo(SignedBinary.class) //blob is Oak Blob.
>> Blob interface would extend adaptable
>> 3. results in SegmentBlob delegating to BlobStoreBlob which
>> 4. delegates to BlobStore // Here just passing the BlobId
>> 5. which delegates to DataStoreBlobStore
>> 6. which delegates to S3DataStore
>> 7. which returns the SignedBinary implementation
>>
>> However adapter support would allow us to make this instance of check
>> extensible. Otherwise we would be hardcoding instance of check to
>> SignedBinary at each of the above place though those layers need not
>> be aware of SignedBinary support (its specific to S3 impl)
>>
>> Chetan Mehrotra
>>
>
>

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Ian Boston]

Hi,
That would require javax.jcr.Binary to implement Adaptable, which it cant.
(OakBinary could but it doesnt need to).

Using Sling AdapterFactory/AdapterManger javadoc (to be replaced with Oaks
internal version of the same)

What is needed is an AdapterFactory for javax.jcr.Binary to SignedBinary
provided by the S3DataStore itself.

Since javax.jcr.Binary cant extend Adaptable, its not possible to call
binary.adaptTo(SignedBinary.class) without a cast, hence,
the call is done via the AdapterManager[1]

SignedBinary signedBinary =  adapterManager.getAdapter(binary,
SignedBinary.class);

---
You could just jump to
URI uri =  adapterManager.getAdapter(binary, URI.class);

No API changes to any existing Oak APIs,

Best Regards
Ian


1
https://sling.apache.org/apidocs/sling5/org/apache/sling/api/adapter/AdapterManager.html



On 24 August 2017 at 12:38, Chetan Mehrotra 
wrote:

> > various layers involved. The bit I don't understand is how the adaptable
> > pattern would make those go away. To me that pattern is just another way
> to
> > implement this but it would also need to deal with all those layers.
>
> Yes this adapter support would need to be implement at all layers.
>
> So call to
> 1. binary.adaptTo(SignedBinary.class) //binary is JCR Binary
> 2. results in blob.adaptTo(SignedBinary.class) //blob is Oak Blob.
> Blob interface would extend adaptable
> 3. results in SegmentBlob delegating to BlobStoreBlob which
> 4. delegates to BlobStore // Here just passing the BlobId
> 5. which delegates to DataStoreBlobStore
> 6. which delegates to S3DataStore
> 7. which returns the SignedBinary implementation
>
> However adapter support would allow us to make this instance of check
> extensible. Otherwise we would be hardcoding instance of check to
> SignedBinary at each of the above place though those layers need not
> be aware of SignedBinary support (its specific to S3 impl)
>
> Chetan Mehrotra
>

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Chetan Mehrotra]

> 1. Figure out how to surface a signed URL from the DataStore to the
> level of the JCR (or Oak) API.
> 2. Provide OSGi glue inside Sling, possibly exposing the signed URL it
> via adaptTo().

Thats sums up the requirement well. Most of proposal here is for #1.
Once we have that implemented #2 can be done in Sling side
Chetan Mehrotra


On Thu, Aug 24, 2017 at 3:06 AM, Ian Boston  wrote:
> Hi,
>
> On 24 August 2017 at 10:20, Julian Sedding  wrote:
>
>> Hi
>>
>> On Thu, Aug 24, 2017 at 9:27 AM, Ian Boston  wrote:
>> > On 24 August 2017 at 08:18, Michael Dürig  wrote:
>> >
>> >>
>> >>
>> >> URI uri = ((OakValueFactory) valueFactory).getSignedURI(binProp);
>> >>
>> >>
>> > +1
>> >
>> > One point
>> > Users in Sling dont know abou Oak, they know about JCR.
>>
>> I think this issue should be solved in two steps:
>>
>> 1. Figure out how to surface a signed URL from the DataStore to the
>> level of the JCR (or Oak) API.
>> 2. Provide OSGi glue inside Sling, possibly exposing the signed URL it
>> via adaptTo().
>>
>> >
>> > URI uri = ((OakValueFactory)
>> > valueFactory).getSignedURI(jcrNode.getProperty("jcr:data"));
>> >
>> > No new APIs, let OakValueFactory work it out and return null if it cant
>> do
>> > it. It should also handle a null parameter.
>> > (I assume OakValueFactory already exists)
>> >
>> > If you want to make it extensible
>> >
>> >  T convertTo(Object source, Class target);
>> >
>> > used as
>> >
>> > URI uri = ((OakValueFactory)
>> > valueFactory). convertTo(jcrNode.getProperty("jcr:data"), URI.class);
>>
>> There is an upcoming OSGi Spec for a Converter service (RFC 215 Object
>> Conversion, also usable outside of OSGI)[0]. It has an implementation
>> in Felix, but afaik no releases so far.
>>
>> A generic Converter would certainly help with decoupling. Basically
>> the S3-DataStore could register an appropriate conversion, hiding all
>> implementation details.
>>
>
> Sounds like a good fit.
> +1
>
> Best Regards
> Ian
>
>
>>
>> Regards
>> Julian
>>
>> [0] https://github.com/osgi/design/blob/05cd5cf03d4b6f8a512886eae472a6
>> b6fde594b0/rfcs/rfc0215/rfc-0215-object-conversion.pdf
>>
>> >
>> > The user doesnt know or need to know the URI is signed, it needs a URI
>> that
>> > can be resolved.
>> > Oak wants it to be signed.
>> >
>> > Best Regards
>> > Ian
>> >
>> >
>> >
>> >> Michael
>> >>
>> >>
>> >>
>> >>
>> >>> A rough sketch of any alternative proposal would be helpful to decide
>> >>> how to move forward
>> >>>
>> >>> Chetan Mehrotra
>> >>>
>> >>>
>>

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Chetan Mehrotra]

> various layers involved. The bit I don't understand is how the adaptable
> pattern would make those go away. To me that pattern is just another way to
> implement this but it would also need to deal with all those layers.

Yes this adapter support would need to be implement at all layers.

So call to
1. binary.adaptTo(SignedBinary.class) //binary is JCR Binary
2. results in blob.adaptTo(SignedBinary.class) //blob is Oak Blob.
Blob interface would extend adaptable
3. results in SegmentBlob delegating to BlobStoreBlob which
4. delegates to BlobStore // Here just passing the BlobId
5. which delegates to DataStoreBlobStore
6. which delegates to S3DataStore
7. which returns the SignedBinary implementation

However adapter support would allow us to make this instance of check
extensible. Otherwise we would be hardcoding instance of check to
SignedBinary at each of the above place though those layers need not
be aware of SignedBinary support (its specific to S3 impl)

Chetan Mehrotra

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Ian Boston]

Hi,

On 24 August 2017 at 10:20, Julian Sedding  wrote:

> Hi
>
> On Thu, Aug 24, 2017 at 9:27 AM, Ian Boston  wrote:
> > On 24 August 2017 at 08:18, Michael Dürig  wrote:
> >
> >>
> >>
> >> URI uri = ((OakValueFactory) valueFactory).getSignedURI(binProp);
> >>
> >>
> > +1
> >
> > One point
> > Users in Sling dont know abou Oak, they know about JCR.
>
> I think this issue should be solved in two steps:
>
> 1. Figure out how to surface a signed URL from the DataStore to the
> level of the JCR (or Oak) API.
> 2. Provide OSGi glue inside Sling, possibly exposing the signed URL it
> via adaptTo().
>
> >
> > URI uri = ((OakValueFactory)
> > valueFactory).getSignedURI(jcrNode.getProperty("jcr:data"));
> >
> > No new APIs, let OakValueFactory work it out and return null if it cant
> do
> > it. It should also handle a null parameter.
> > (I assume OakValueFactory already exists)
> >
> > If you want to make it extensible
> >
> >  T convertTo(Object source, Class target);
> >
> > used as
> >
> > URI uri = ((OakValueFactory)
> > valueFactory). convertTo(jcrNode.getProperty("jcr:data"), URI.class);
>
> There is an upcoming OSGi Spec for a Converter service (RFC 215 Object
> Conversion, also usable outside of OSGI)[0]. It has an implementation
> in Felix, but afaik no releases so far.
>
> A generic Converter would certainly help with decoupling. Basically
> the S3-DataStore could register an appropriate conversion, hiding all
> implementation details.
>

Sounds like a good fit.
+1

Best Regards
Ian


>
> Regards
> Julian
>
> [0] https://github.com/osgi/design/blob/05cd5cf03d4b6f8a512886eae472a6
> b6fde594b0/rfcs/rfc0215/rfc-0215-object-conversion.pdf
>
> >
> > The user doesnt know or need to know the URI is signed, it needs a URI
> that
> > can be resolved.
> > Oak wants it to be signed.
> >
> > Best Regards
> > Ian
> >
> >
> >
> >> Michael
> >>
> >>
> >>
> >>
> >>> A rough sketch of any alternative proposal would be helpful to decide
> >>> how to move forward
> >>>
> >>> Chetan Mehrotra
> >>>
> >>>
>

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Michael Dürig]

I understand the difficulties involved with implementing this due to the 
various layers involved. The bit I don't understand is how the adaptable 
pattern would make those go away. To me that pattern is just another way 
to implement this but it would also need to deal with all those layers.


Michael

On 24.08.17 11:08, Chetan Mehrotra wrote:

As explained in previous mail adaptable pattern requirement is to
enable such a support within Oak itself. due to multiple layers
involved.


If it doesnt exist then perhaps Oak could add a Service interface that
deals with conversions, rather than expose a second adaptable pattern in
Sling, or require type casting and instanceof.


We can expose such a service also if that helps. That service
implementation internally would anyway have to use adaptable pattern.

public class OakConversionService{
  AdapterType adaptTo(Binary b, Class type) {
 if (b instanceof Adaptable) {
 return (type)b.adaptTo(type);
 }
 return null
 }
}

So just another level of abstraction.

Chetan Mehrotra

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Tommaso Teofili]

Il giorno gio 24 ago 2017 alle ore 10:16 Michael Dürig 
ha scritto:

>
>
> On 24.08.17 09:27, Ian Boston wrote:
> > On 24 August 2017 at 08:18, Michael Dürig  wrote:
> >
> >>
> >>
> >> URI uri = ((OakValueFactory) valueFactory).getSignedURI(binProp);
> >>
> >>
> > +1
> >
> > One point
> > Users in Sling dont know abou Oak, they know about JCR.
> >
> > URI uri = ((OakValueFactory)
> > valueFactory).getSignedURI(jcrNode.getProperty("jcr:data"));
> >
> > No new APIs, let OakValueFactory work it out and return null if it cant
> do
> > it. It should also handle a null parameter.
> > (I assume OakValueFactory already exists)
>
> No, OakValueFactory does not exist as API (yet). But adding it would be
> more inline with how we approached the Oak API traditionally.
>
> I'm not against introducing the adaptable pattern but would like to
> understand whether there is concrete enough use cases beyond the current
> one to warrant it.
>

+1, currently I much prefer such a concrete approach over the adaptable;
not that it's bad per se, just I am not sure there'll be other use cases.

That said, although I understand the use cases and requirements, this still
seems to me a break into the fundamental Oak design, sorry.
It's not I'm totally against it, it's just that perhaps we should rethink
some of our design if it can't stick to our requirements, e.g. provide an
API for accessing binaries at the storage layer, which we expect to be used
by consumers having the right privileges by decorating it with something
similar to a JWT [1] token, emitted by the repo.

my 2 cents,
Tommaso

[1] : https://jwt.io/


>
> Michael
>
> >
> > If you want to make it extensible
> >
> >  T convertTo(Object source, Class target);
> >
> > used as
> >
> > URI uri = ((OakValueFactory)
> > valueFactory). convertTo(jcrNode.getProperty("jcr:data"), URI.class);
> >
> > The user doesnt know or need to know the URI is signed, it needs a URI
> that
> > can be resolved.
> > Oak wants it to be signed.
> >
> > Best Regards
> > Ian
> >
> >
> >
> >> Michael
> >>
> >>
> >>
> >>
> >>> A rough sketch of any alternative proposal would be helpful to decide
> >>> how to move forward
> >>>
> >>> Chetan Mehrotra
> >>>
> >>>
> >
>

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Chetan Mehrotra]

As explained in previous mail adaptable pattern requirement is to
enable such a support within Oak itself. due to multiple layers
involved.

> If it doesnt exist then perhaps Oak could add a Service interface that
> deals with conversions, rather than expose a second adaptable pattern in
> Sling, or require type casting and instanceof.

We can expose such a service also if that helps. That service
implementation internally would anyway have to use adaptable pattern.

public class OakConversionService{
 AdapterType adaptTo(Binary b, Class type) {
if (b instanceof Adaptable) {
return (type)b.adaptTo(type);
}
return null
}
}

So just another level of abstraction.

Chetan Mehrotra

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Ian Boston]

On 24 August 2017 at 09:16, Michael Dürig  wrote:

>
>
> On 24.08.17 09:27, Ian Boston wrote:
>
>> On 24 August 2017 at 08:18, Michael Dürig  wrote:
>>
>>
>>>
>>> URI uri = ((OakValueFactory) valueFactory).getSignedURI(binProp);
>>>
>>>
>>> +1
>>
>> One point
>> Users in Sling dont know abou Oak, they know about JCR.
>>
>> URI uri = ((OakValueFactory)
>> valueFactory).getSignedURI(jcrNode.getProperty("jcr:data"));
>>
>> No new APIs, let OakValueFactory work it out and return null if it cant do
>> it. It should also handle a null parameter.
>> (I assume OakValueFactory already exists)
>>
>
> No, OakValueFactory does not exist as API (yet). But adding it would be
> more inline with how we approached the Oak API traditionally.
>
>
If it doesnt exist then perhaps Oak could add a Service interface that
deals with conversions, rather than expose a second adaptable pattern in
Sling, or require type casting and instanceof.  How Oak implements those
conversions is upto Oak.

eg

public interface OakConversionService  {
  T convertTo(Object source, Class target);
}


implemented as an OSGi Service used as eg

@Reference
private OakConversionService conversionService;


public void doFilter(...) {
...
   URL u = conversionService.convertTo(jcrBinary, URI.class);

}

Best Regards
Ian


> I'm not against introducing the adaptable pattern but would like to
> understand whether there is concrete enough use cases beyond the current
> one to warrant it.
>
> Michael
>
>
>
>> If you want to make it extensible
>>
>>  T convertTo(Object source, Class target);
>>
>> used as
>>
>> URI uri = ((OakValueFactory)
>> valueFactory). convertTo(jcrNode.getProperty("jcr:data"), URI.class);
>>
>> The user doesnt know or need to know the URI is signed, it needs a URI
>> that
>> can be resolved.
>> Oak wants it to be signed.
>>
>> Best Regards
>> Ian
>>
>>
>>
>> Michael
>>>
>>>
>>>
>>>
>>> A rough sketch of any alternative proposal would be helpful to decide
 how to move forward

 Chetan Mehrotra



>>

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Michael Dürig]

On 24.08.17 09:27, Ian Boston wrote:

On 24 August 2017 at 08:18, Michael Dürig  wrote:




URI uri = ((OakValueFactory) valueFactory).getSignedURI(binProp);



+1

One point
Users in Sling dont know abou Oak, they know about JCR.

URI uri = ((OakValueFactory)
valueFactory).getSignedURI(jcrNode.getProperty("jcr:data"));

No new APIs, let OakValueFactory work it out and return null if it cant do
it. It should also handle a null parameter.
(I assume OakValueFactory already exists)


No, OakValueFactory does not exist as API (yet). But adding it would be 
more inline with how we approached the Oak API traditionally.


I'm not against introducing the adaptable pattern but would like to 
understand whether there is concrete enough use cases beyond the current 
one to warrant it.


Michael



If you want to make it extensible

 T convertTo(Object source, Class target);

used as

URI uri = ((OakValueFactory)
valueFactory). convertTo(jcrNode.getProperty("jcr:data"), URI.class);

The user doesnt know or need to know the URI is signed, it needs a URI that
can be resolved.
Oak wants it to be signed.

Best Regards
Ian




Michael





A rough sketch of any alternative proposal would be helpful to decide
how to move forward

Chetan Mehrotra

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Ian Boston]

The datastore should understand how to go from Blob -> URI.

In the case of S3 it does and uses Blob.getContentId().

If the datastore doesnt know how to do it, then its not supported by the
datastore.

You might need a DataStore.getSignedURI(Blob b) method.

On 24 August 2017 at 08:27, Chetan Mehrotra 
wrote:

> > Fair point. So this is more about dynamic adaptability than future
> > extendibility. But AFIU this could still be achieved without the full
> > adaptable machinery:
> >
> > if (binProp instanceOf SignableBin) {
> >   URI uri = ((SignableBin) binProp).getSignedURI();
> >   if (uri != null) {
> > // resolve URI etc.
> >   }
> > }
>
> This would be tricky. The current logic is like below.
>
> 1. Oak JCR BinaryImpl holds a ValueImpl
> 2. ValueImpl -> PropertyState -> Blob
> 3. From Blob following paths are possible
>- Blob -> SegmentBlob -> BlobStoreBlob -> DataRecord -> S3DataRecord
>- Blob -> ArrayBasedBlob
>- Blob ... MongoBlob
>
> So at JCR level where we have a PropertyState we cannot determine if
> the Blob provided by it can provide a signed binary without adding
> such instance of check at each place. Hence the adaptor based proposal
>
> Chetan Mehrotra
>

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Ian Boston]

On 24 August 2017 at 08:18, Michael Dürig  wrote:

>
>
> URI uri = ((OakValueFactory) valueFactory).getSignedURI(binProp);
>
>
+1

One point
Users in Sling dont know abou Oak, they know about JCR.

URI uri = ((OakValueFactory)
valueFactory).getSignedURI(jcrNode.getProperty("jcr:data"));

No new APIs, let OakValueFactory work it out and return null if it cant do
it. It should also handle a null parameter.
(I assume OakValueFactory already exists)

If you want to make it extensible

 T convertTo(Object source, Class target);

used as

URI uri = ((OakValueFactory)
valueFactory). convertTo(jcrNode.getProperty("jcr:data"), URI.class);

The user doesnt know or need to know the URI is signed, it needs a URI that
can be resolved.
Oak wants it to be signed.

Best Regards
Ian



> Michael
>
>
>
>
>> A rough sketch of any alternative proposal would be helpful to decide
>> how to move forward
>>
>> Chetan Mehrotra
>>
>>

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Chetan Mehrotra]

> Fair point. So this is more about dynamic adaptability than future
> extendibility. But AFIU this could still be achieved without the full
> adaptable machinery:
>
> if (binProp instanceOf SignableBin) {
>   URI uri = ((SignableBin) binProp).getSignedURI();
>   if (uri != null) {
> // resolve URI etc.
>   }
> }

This would be tricky. The current logic is like below.

1. Oak JCR BinaryImpl holds a ValueImpl
2. ValueImpl -> PropertyState -> Blob
3. From Blob following paths are possible
   - Blob -> SegmentBlob -> BlobStoreBlob -> DataRecord -> S3DataRecord
   - Blob -> ArrayBasedBlob
   - Blob ... MongoBlob

So at JCR level where we have a PropertyState we cannot determine if
the Blob provided by it can provide a signed binary without adding
such instance of check at each place. Hence the adaptor based proposal

Chetan Mehrotra

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Michael Dürig]

On 24.08.17 09:06, Chetan Mehrotra wrote:

I think the discussion about the adapter pattern is orthogonal to the binary


For me its tied to how you are going to implement this support.
Adaptable patterns is one way based on my current understand of Oak
design.

At level of ValueFactory.getBinary we do not know if the Blob can
provide a signed url. Its deep down in the layer JCR Binary -> Oak
Blob -> DataStoreBlob -> S3DataStore DataRecord. So each Blob cannot
provided a signed url and it depends on backing DataStore. This can be
easily supported via adaptor pattern where JCR layer tries to adapt
and then final backing BlobStore impl decides to provide the adaption
implementation.

I do not see how instance of checks can be expressed across all these layers


Fair point. So this is more about dynamic adaptability than future 
extendibility. But AFIU this could still be achieved without the full 
adaptable machinery:


if (binProp instanceOf SignableBin) {
  URI uri = ((SignableBin) binProp).getSignedURI();
  if (uri != null) {
// resolve URI etc.
  }
}

Or alternatively something along the lines of:

URI uri = ((OakValueFactory) valueFactory).getSignedURI(binProp);


Michael




A rough sketch of any alternative proposal would be helpful to decide
how to move forward

Chetan Mehrotra

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Chetan Mehrotra]

> I think the discussion about the adapter pattern is orthogonal to the binary

For me its tied to how you are going to implement this support.
Adaptable patterns is one way based on my current understand of Oak
design.

At level of ValueFactory.getBinary we do not know if the Blob can
provide a signed url. Its deep down in the layer JCR Binary -> Oak
Blob -> DataStoreBlob -> S3DataStore DataRecord. So each Blob cannot
provided a signed url and it depends on backing DataStore. This can be
easily supported via adaptor pattern where JCR layer tries to adapt
and then final backing BlobStore impl decides to provide the adaption
implementation.

I do not see how instance of checks can be expressed across all these layers

A rough sketch of any alternative proposal would be helpful to decide
how to move forward

Chetan Mehrotra

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Michael Dürig]

Hi,

I think the discussion about the adapter pattern is orthogonal to the 
binary issue. According to YAGNI we should stick with instance of checks 
unless we already have a somewhat clear picture of future extensions.


Michael

On 24.08.17 07:28, Chetan Mehrotra wrote:

Based on the feedback so far below is revised proposal

1. Define a new Adaptable interface in 'org.apache.jackrabbit.oak.api'

public interface Adaptable {

 /**
  * Adapts the binary to another type
  *
  * @param  The generic type to which this type is adapted
  *to
  * @param type The Class object of the target type
  * @return The adapter target or null if the type cannot
  * adapt to the requested type
  */
  AdapterType adaptTo(Class type);
}

2. Have the binary implementation in Oak implement Adaptable
3. Have a minimal implementation in Oak on line of Sling Adaptor support [1]

For current usecase we would provide an adaptation to SignedBinary

public interface SignedBinary {

 URI getUri()
}

Chetan Mehrotra

[1] 
https://github.com/apache/sling/tree/trunk/bundles/api/src/main/java/org/apache/sling/api/adapter


On Wed, Aug 23, 2017 at 10:04 PM, Chetan Mehrotra
 wrote:

Hence, why not simply use  binaryProp instanceof SignedBinary ?


As Julian mentioned it would make it tricky to support multiple
extensions with various permutations. Having adapter support for
simplify the implementation


No client should be issued a signed url that could be used in the distant
(relatively) future bypassing fresh ACL constraints saved to Oak.


Fair point. Then lets drop the ttl paramater

Chetan Mehrotra

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Amit Jain]

Hi,

+1 from my side for the broad contours as proposed above.

Thanks
Amit

On Thu, Aug 24, 2017 at 10:58 AM, Chetan Mehrotra  wrote:

> Based on the feedback so far below is revised proposal
>
> 1. Define a new Adaptable interface in 'org.apache.jackrabbit.oak.api'
>
> public interface Adaptable {
>
> /**
>  * Adapts the binary to another type
>  *
>  * @param  The generic type to which this type is adapted
>  *to
>  * @param type The Class object of the target type
>  * @return The adapter target or null if the type cannot
>  * adapt to the requested type
>  */
>  AdapterType adaptTo(Class type);
> }
>
> 2. Have the binary implementation in Oak implement Adaptable
> 3. Have a minimal implementation in Oak on line of Sling Adaptor support
> [1]
>
> For current usecase we would provide an adaptation to SignedBinary
>
> public interface SignedBinary {
>
> URI getUri()
> }
>
> Chetan Mehrotra
>
> [1] https://github.com/apache/sling/tree/trunk/bundles/api/
> src/main/java/org/apache/sling/api/adapter
>
>
> On Wed, Aug 23, 2017 at 10:04 PM, Chetan Mehrotra
>  wrote:
> >> Hence, why not simply use  binaryProp instanceof SignedBinary ?
> >
> > As Julian mentioned it would make it tricky to support multiple
> > extensions with various permutations. Having adapter support for
> > simplify the implementation
> >
> >> No client should be issued a signed url that could be used in the
> distant
> >> (relatively) future bypassing fresh ACL constraints saved to Oak.
> >
> > Fair point. Then lets drop the ttl paramater
> >
> > Chetan Mehrotra
>

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Julian Reschke]

On 2017-08-23 13:39, Chetan Mehrotra wrote:

Below is one possible sketch of the proposed api. We introduce a new
AdaptableBinary which allows adapting a Binary to some other form.

API
===

public interface AdaptableBinary {

 /**
  * Adapts the binary to another type
  *
  * @param  The generic type to which this binary is adapted
  *to
  * @param type The Class object of the target type
  * @return The adapter target or null if the binary cannot
  * adapt to the requested type
  */
  AdapterType adaptTo(Class type);
}


Can we make that more generic, not relying on Binary?


Usage
=

Binary binProp = node.getProperty("jcr:data").getBinary();

//Check if Binary is of type AdaptableBinary
if (binProp instanceof AdaptableBinary){
 AdaptableBinary adaptableBinary = (AdaptableBinary) binProp;
 SignedBinary url = adaptableBinary.adaptTo(SignedBinary.class);
}

Where SignedBinary is one of the supported adaptables.

public interface SignedBinary {

 URL getUrl(int ttl, TimeUnit unit)
}


Use URI, not URL.


...


Best regards, Julian

Re: OAK-6575 - Provide a secure external URL to a DataStore binary.

[Chetan Mehrotra]

Below is one possible sketch of the proposed api. We introduce a new
AdaptableBinary which allows adapting a Binary to some other form.

API
===

public interface AdaptableBinary {

/**
 * Adapts the binary to another type
 *
 * @param  The generic type to which this binary is adapted
 *to
 * @param type The Class object of the target type
 * @return The adapter target or null if the binary cannot
 * adapt to the requested type
 */
 AdapterType adaptTo(Class type);
}



Usage
=

Binary binProp = node.getProperty("jcr:data").getBinary();

//Check if Binary is of type AdaptableBinary
if (binProp instanceof AdaptableBinary){
AdaptableBinary adaptableBinary = (AdaptableBinary) binProp;
SignedBinary url = adaptableBinary.adaptTo(SignedBinary.class);
}

Where SignedBinary is one of the supported adaptables.

public interface SignedBinary {

URL getUrl(int ttl, TimeUnit unit)
}

The user can specify ttl. The implementation may enforce an upper
bound on the allowed ttl.

This proposal is meant to provide base. If we agree on the general
approach then we can decide further details like

1. Under which package to expose AdaptableBinary

Proposal 'org.apache.jackrabbit.oak.jcr.binary'. We would also later
possibly need an AdaptableBlob for Oak layer

2. Under which package to expose SignedBinary

Proposal 'org.apache.jackrabbit.oak.api.blob' in oak-api

Thoughts?
Chetan Mehrotra


On Wed, Aug 23, 2017 at 4:25 AM, Chetan Mehrotra
 wrote:
> Recently we had internal discussion for Ian's requirement in OAK-6575.
> See issue for complete details. In brief
>
> 1. Need a way to provide a signed url [1] for Blobs stored in Oak if
> they are stored in S3
> 2. The url would only be created if the user can access the Binary.
> 3.  The url would only be valid for certain time
>
> To meet this requirement various approaches were suggested like using
> Adaptable pattern in Sling, or having a new api in Binary object.
>
> Would follow up with a sketch for such an API
>
> Chetan Mehrotra
> [1] 
> http://docs.aws.amazon.com/AmazonS3/latest/dev/ShareObjectPreSignedURL.html