cs3org / reva

WebDAV/gRPC/HTTP high performance server to link high level clients to storage backends
https://reva.link
Apache License 2.0
167 stars 113 forks source link

Why require providerId to contain a `:`? #1981

Open michielbdejong opened 3 years ago

michielbdejong commented 3 years ago

https://cs3org.github.io/OCM-API/docs.html#/paths/~1shares/post requires providerId to be a string that is unique per provider.

Revad furthermore requires that providerId is of the form StorageId:OpaqueId. Why is this?

When I look at for instance https://github.com/cs3org/reva/blob/v1.11.0/pkg/utils/utils.go#L156-L159 it looks like it might as well have been one unique string instead of two strings with a : between them?

Also, since the OCM API spec does not require this, this will make reva incompatible with other OCM implementations.

butonic commented 3 years ago

Identifier to identify the resource at the provider side. This is unique per provider.

reva uses the StorageId to route requests to the correct storage provider, which in turn then uses the OpaqueId to identify the resource. That was always the case. But for the owncloud ocs and webdav apis we had to glue them together to a single string for legacy reasons. I don't see a conflict with the OCM api, yet. It does not restrict the character set or require a uuid. To other instances the providerid should be an opaque string.

Also reva should be able to use a providerid as is. Is the latter a problem? I havent had the time to dig into the ocm implementation, yet.

michielbdejong commented 3 years ago

Sure, it's fine that reva does that internally, and it's also fine when reva is the provider, then it's at liberty to compose the string in that way.

What breaks is when an unsuspecting provider (unrelated to reva) sends a share over OCM to a reva instance. Then a 400 is thrown, from this line:

https://github.com/cs3org/reva/blob/v1.11.0/internal/grpc/services/ocmcore/ocmcore.go#L113

That's the problem I ran into when testing reva's OCM implementation.

michielbdejong commented 3 years ago

Shares coming into reva are described by a CreateOCMCoreShareRequest. Share coming from reva are described by a requestBody.

The md.StorageId and md.OpaqueId in the requestBody only have meaning at the sending instance, and should never be interpreted at the receiving instance side.