You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Chetan Mehrotra <ch...@gmail.com> on 2017/08/23 11:25:37 UTC

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

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

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

Posted by Ian Boston <ie...@tfd.co.uk>.
Somehow the following between me and Chetan slipped off list, so copying it
back here to continue on list, probably my fault, apologies.

----------------

Hi,

On 23 August 2017 at 13:29, Chetan Mehrotra <ch...@gmail.com>
 wrote:

> > >> Can we make that more generic, not relying on Binary?
>
> Was being conservative there and hence not proposed a generic
> adaptable api. But +1 for making it generic
>
> > adapting from something already Adaptable would work.
> > Sling Resource might be a good option, since a) its adaptable, and b) its
> > resolvable and already available to the code that would use this.
>
> So far Oak does not have a dependency on Sling API and this api should
> be usable in non Sling setups also
>


In that case, Oak can't implement the AdapterFactory interface either or
implement Adaptables. AdaptableBinary should
strictly speaking extend Adaptable if it is to implement adaptTo and be
used in a Sling based deployment.

Since the AdapterFactory and Adaptable interface are not available, the
benefits are not available to Oak.

Hence, why not simply use  binaryProp instanceof SignedBinary ?

Oak would have to provide multiple versions of the Binary implementation,
differing depending on if or not a SignedBinary was appropriate. That might
be dependent on the properties of the node or one of its parents. It would
certainly be dependent on if the Binary was in S3 (or equiv) or NodeStore
(<16K).



>
> > > SingedBinary sb = resource.adaptTo(SignedBinary.class);
>
> At Oak level we do not have resource. What we have is JCR Binary
> interface which does not extend Adaptable
>

ok, understood.


>
> > Also, As a caller I could specify 20 years, or 30s. Oak should decide
> what
> > the ttl is, not leave it to the caller.
>
> As mentioned earlier implementation can enforce an upper limit. My
> take was that different usecase require different ttls. For e.g. if
> the asset needs to be shared for some workflow processing then it may
> require longer ttl. If other think its not useful we can drop that
>

If a long ttl is possible for certain use cases, then what is to stop
someone using that long ttl where it is not appropriate (publishing it to
public html).?
The ttl should be configurable by the deployer, but not by the caller.

OAK-6575 was only really thinking about requests being converted into
redirects that would be used immediately.

If something like workflow (or any client) needed direct access some time
later, it should authenticate against Oak, be issued a fresh signed url as
a redirect and access the binary using its own signed url.

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

Best Regards
Ian




>
> Chetan Mehrotra





On 23 August 2017 at 13:22, Ian Boston <ie...@tfd.co.uk> wrote:

> Hi,
>
> On 23 August 2017 at 13:06, Julian Reschke <ju...@gmx.de> wrote:
>
>> 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 <AdapterType> The generic type to which this binary is
>>> adapted
>>>       *            to
>>>       * @param type The Class object of the target type
>>>       * @return The adapter target or <code>null</code> if the binary
>>> cannot
>>>       *         adapt to the requested type
>>>       */
>>>      <AdapterType> AdapterType adaptTo(Class<AdapterType> type);
>>> }
>>>
>>
>> Can we make that more generic, not relying on Binary?
>
>
>
> adapting from something already Adaptable would work.
> Sling Resource might be a good option, since a) its adaptable, and b) its
> resolvable and already available to the code that would use this.
>
>
>
>
>>
>>
>> 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);
>>> }
>>>
>>>
>
> This breaks the Adaptable pattern, which should not require intermediate
> interfaces.
> If the implementation needs instanceof we can drop the Adaptable pattern
> and just use APIs.
>
> if (binProp instanceof SignedBinary) {
>      response.sendRedirect(((SingedBinary)binProp).getURL());
> }
>
> better would be
>
> SingedBinary sb = resource.adaptTo(SignedBinary.class);
> if ( sb != null ) { // oak might decide for this invocation a signed
> binary is not appropriate, and so return null.
>   response.sendRedirect(sb.getURL());
> }
>
> If the DS didn't have a AdapterFactory implemented, null will be returned.
> Optionally implementing an interface on a call by call basis requires more
> work, potentially, than adapting.
>
>
>
>
>> Where SignedBinary is one of the supported adaptables.
>>>
>>> public interface SignedBinary {
>>>
>>>      URL getUrl(int ttl, TimeUnit unit)
>>> }
>>>
>>
>> Use URI, not URL.
>>
>
> Is there something so wrong with an immutable String in this case ?
>
> Also, As a caller I could specify 20 years, or 30s. Oak should decide what
> the ttl is, not leave it to the caller.
>
> Best Regards
> Ian
>
>
>> ...
>>>
>>
>> Best regards, Julian
>>
>
>

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

Posted by Amit Jain <am...@ieee.org>.
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 <chetan.mehrotra@gmail.com
> 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 <AdapterType> The generic type to which this type is adapted
>      *            to
>      * @param type The Class object of the target type
>      * @return The adapter target or <code>null</code> if the type cannot
>      *         adapt to the requested type
>      */
>     <AdapterType> AdapterType adaptTo(Class<AdapterType> 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
> <ch...@gmail.com> 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.

Posted by Ian Boston <ie...@tfd.co.uk>.
On 24 August 2017 at 14:42, Michael Dürig <md...@apache.org> 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 <ie...@tfd.co.uk> 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 <ch...@gmail.com>
>>> 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 <ie...@tfd.co.uk> 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 <ie...@tfd.co.uk> 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 <
>>>>>> chetan.mehrotra@gmail.com>
>>>>>> 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.

Posted by Michael Dürig <md...@apache.org>.

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 <ie...@tfd.co.uk> 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 <ch...@gmail.com>
>> 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 <ie...@tfd.co.uk> 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 <ie...@tfd.co.uk> 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 <ch...@gmail.com>
>>>>> 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.

Posted by Chetan Mehrotra <ch...@gmail.com>.
> 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 <ie...@tfd.co.uk> 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 <ch...@gmail.com>
> 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 <ie...@tfd.co.uk> 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 <ie...@tfd.co.uk> 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 <ch...@gmail.com>
>> >> 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.

Posted by Ian Boston <ie...@tfd.co.uk>.
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 <ch...@gmail.com>
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 <ie...@tfd.co.uk> 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 <ie...@tfd.co.uk> 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 <ch...@gmail.com>
> >> 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.

Posted by Chetan Mehrotra <ch...@gmail.com>.
> 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 <ie...@tfd.co.uk> 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 <ie...@tfd.co.uk> 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 <ch...@gmail.com>
>> 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.

Posted by Ian Boston <ie...@tfd.co.uk>.
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 <ie...@tfd.co.uk> 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 <ch...@gmail.com>
> 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.

Posted by Michael Dürig <md...@apache.org>.

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.

Posted by Ian Boston <ie...@tfd.co.uk>.
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 <ch...@gmail.com>
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.

Posted by Michael Dürig <md...@apache.org>.

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 <md...@apache.org> 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 <md...@apache.org> 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.

Posted by Chetan Mehrotra <ch...@gmail.com>.
> 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 <md...@apache.org> 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 <md...@apache.org> 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.

Posted by Michael Dürig <md...@apache.org>.

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 <md...@apache.org> 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.

Posted by Chetan Mehrotra <ch...@gmail.com>.
> 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.

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


On Thu, Aug 24, 2017 at 5:25 AM, Michael Dürig <md...@apache.org> 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.

Posted by Michael Dürig <md...@apache.org>.

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.

Posted by Chetan Mehrotra <ch...@gmail.com>.
> 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.

Posted by Michael Dürig <md...@apache.org>.
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> AdapterType adaptTo(Binary b, Class<AdapterType> 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.

Posted by Chetan Mehrotra <ch...@gmail.com>.
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> AdapterType adaptTo(Binary b, Class<AdapterType> 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.

Posted by Ian Boston <ie...@tfd.co.uk>.
On 24 August 2017 at 09:16, Michael Dürig <md...@apache.org> wrote:

>
>
> On 24.08.17 09:27, Ian Boston wrote:
>
>> On 24 August 2017 at 08:18, Michael Dürig <md...@apache.org> 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> T convertTo(Object source, Class<T> 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> T convertTo(Object source, Class<T> 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.

Posted by Tommaso Teofili <to...@gmail.com>.
Il giorno gio 24 ago 2017 alle ore 10:16 Michael Dürig <md...@apache.org>
ha scritto:

>
>
> On 24.08.17 09:27, Ian Boston wrote:
> > On 24 August 2017 at 08:18, Michael Dürig <md...@apache.org> 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> T convertTo(Object source, Class<T> 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.

Posted by Michael Dürig <md...@apache.org>.

On 24.08.17 09:27, Ian Boston wrote:
> On 24 August 2017 at 08:18, Michael Dürig <md...@apache.org> 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> T convertTo(Object source, Class<T> 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.

Posted by Chetan Mehrotra <ch...@gmail.com>.
> 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 <ie...@tfd.co.uk> wrote:
> Hi,
>
> On 24 August 2017 at 10:20, Julian Sedding <js...@gmail.com> wrote:
>
>> Hi
>>
>> On Thu, Aug 24, 2017 at 9:27 AM, Ian Boston <ie...@tfd.co.uk> wrote:
>> > On 24 August 2017 at 08:18, Michael Dürig <md...@apache.org> 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> T convertTo(Object source, Class<T> 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.

Posted by Ian Boston <ie...@tfd.co.uk>.
Hi,

On 24 August 2017 at 10:20, Julian Sedding <js...@gmail.com> wrote:

> Hi
>
> On Thu, Aug 24, 2017 at 9:27 AM, Ian Boston <ie...@tfd.co.uk> wrote:
> > On 24 August 2017 at 08:18, Michael Dürig <md...@apache.org> 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> T convertTo(Object source, Class<T> 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.

Posted by Julian Sedding <js...@gmail.com>.
Hi

On Thu, Aug 24, 2017 at 9:27 AM, Ian Boston <ie...@tfd.co.uk> wrote:
> On 24 August 2017 at 08:18, Michael Dürig <md...@apache.org> 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> T convertTo(Object source, Class<T> 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.

Regards
Julian

[0] https://github.com/osgi/design/blob/05cd5cf03d4b6f8a512886eae472a6b6fde594b0/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.

Posted by Ian Boston <ie...@tfd.co.uk>.
On 24 August 2017 at 08:18, Michael Dürig <md...@apache.org> 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> T convertTo(Object source, Class<T> 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.

Posted by Ian Boston <ie...@tfd.co.uk>.
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 <ch...@gmail.com>
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.

Posted by Chetan Mehrotra <ch...@gmail.com>.
> 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.

Posted by Michael Dürig <md...@apache.org>.

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.

Posted by Chetan Mehrotra <ch...@gmail.com>.
> 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.

Posted by Michael Dürig <md...@apache.org>.
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 <AdapterType> The generic type to which this type is adapted
>       *            to
>       * @param type The Class object of the target type
>       * @return The adapter target or <code>null</code> if the type cannot
>       *         adapt to the requested type
>       */
>      <AdapterType> AdapterType adaptTo(Class<AdapterType> 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
> <ch...@gmail.com> 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.

Posted by Chetan Mehrotra <ch...@gmail.com>.
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 <AdapterType> The generic type to which this type is adapted
     *            to
     * @param type The Class object of the target type
     * @return The adapter target or <code>null</code> if the type cannot
     *         adapt to the requested type
     */
    <AdapterType> AdapterType adaptTo(Class<AdapterType> 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
<ch...@gmail.com> 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.

Posted by Chetan Mehrotra <ch...@gmail.com>.
> 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.

Posted by Julian Reschke <ju...@gmx.de>.
On 2017-08-23 14:22, Ian Boston wrote:
> ...
> This breaks the Adaptable pattern, which should not require intermediate
> interfaces.
> If the implementation needs instanceof we can drop the Adaptable pattern
> and just use APIs.
> ...

Not really. It depends on how many extensions there might be in the 
future. Using instanceOf doesn't work well if there may multiple 
extensions in the future thatcan be combined.

...we should have a generic "Adaptable" pattern in Oak for a long time...

Best regards, Julian

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

Posted by Ian Boston <ie...@tfd.co.uk>.
Hi,

On 23 August 2017 at 13:06, Julian Reschke <ju...@gmx.de> wrote:

> 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 <AdapterType> The generic type to which this binary is
>> adapted
>>       *            to
>>       * @param type The Class object of the target type
>>       * @return The adapter target or <code>null</code> if the binary
>> cannot
>>       *         adapt to the requested type
>>       */
>>      <AdapterType> AdapterType adaptTo(Class<AdapterType> type);
>> }
>>
>
> Can we make that more generic, not relying on Binary?



adapting from something already Adaptable would work.
Sling Resource might be a good option, since a) its adaptable, and b) its
resolvable and already available to the code that would use this.




>
>
> 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);
>> }
>>
>>

This breaks the Adaptable pattern, which should not require intermediate
interfaces.
If the implementation needs instanceof we can drop the Adaptable pattern
and just use APIs.

if (binProp instanceof SignedBinary) {
     response.sendRedirect(((SingedBinary)binProp).getURL());
}

better would be

SingedBinary sb = resource.adaptTo(SignedBinary.class);
if ( sb != null ) { // oak might decide for this invocation a signed binary
is not appropriate, and so return null.
  response.sendRedirect(sb.getURL());
}

If the DS didn't have a AdapterFactory implemented, null will be returned.
Optionally implementing an interface on a call by call basis requires more
work, potentially, than adapting.




> Where SignedBinary is one of the supported adaptables.
>>
>> public interface SignedBinary {
>>
>>      URL getUrl(int ttl, TimeUnit unit)
>> }
>>
>
> Use URI, not URL.
>

Is there something so wrong with an immutable String in this case ?

Also, As a caller I could specify 20 years, or 30s. Oak should decide what
the ttl is, not leave it to the caller.

Best Regards
Ian


> ...
>>
>
> Best regards, Julian
>

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

Posted by Julian Reschke <ju...@gmx.de>.
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 <AdapterType> The generic type to which this binary is adapted
>       *            to
>       * @param type The Class object of the target type
>       * @return The adapter target or <code>null</code> if the binary cannot
>       *         adapt to the requested type
>       */
>      <AdapterType> AdapterType adaptTo(Class<AdapterType> 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.

Posted by Chetan Mehrotra <ch...@gmail.com>.
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 <AdapterType> The generic type to which this binary is adapted
     *            to
     * @param type The Class object of the target type
     * @return The adapter target or <code>null</code> if the binary cannot
     *         adapt to the requested type
     */
    <AdapterType> AdapterType adaptTo(Class<AdapterType> 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
<ch...@gmail.com> 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