fix(OCMProvider): Advertise the correct apiVersion

[mickenordin]

Advertise the correct apiVersion

Fixes #52754

[ArtificialOwl]

I am afraid this would break compatibility with older setup,
I have not made an official request to the OCM Team, but apiVersion should be fully ignored and replaced by an optional and informative 'version' entry

[mickenordin]

What older setup is relying on the string 1.0-proposal1 I can't find anything that uses that, anywhere?

[ArtificialOwl]

any version prior to 8b9e7e2#diff-968dd2f309a1906f4e65f96b5e04999c1dfa79ee617fdf5060d5746534f42ca5L227

I would say lower than 28.0.0

[mickenordin]

I would say lower than 28.0.0

Those versions are not supported any more though, are they? And if they are, it seems trivial to backport a fix that bypasses the check? In any case, it is much simpler to fix here, than to try to change the name of a key in the discovery payload in the specification, that has been there from the start and is implemented in reva,ocis,opencloud and seafile already. Any version prior to that fix is also unable to do ocm with any of those providers, so it is not much of a federation support anyway.

[ArtificialOwl]

But this will require patching ...

Do you have a use-case that require apiVersion to be mandatory ?

From my point of view, I do not consider apiVersion as useful. The concept of discovery is to check which features are implemented/configured and a full lock out of basic feature from a remote instance based on the apiVersion is not a viable solution.

[glpatcern]

I have not made an official request to the OCM Team, but apiVersion should be fully ignored and replaced by an optional and informative 'version' entry

Hi there, this was discussed today within the OCM team, and in fact apiVersion is already fully ignored and made available for information only. Not a single OCM operation depends (and ever depended) upon checking the value given as apiVersion - at least in theory, we know that in practice this was used by Nextcloud. Instead, nowadays you can inspect the capabilities to see what to expect from a remote end, as you know, and you are guaranteed that any OCM implementation would provide /shares and webDAV access.

At the same time, if Nextcloud needs to guarantee backwards compatibility prior to v28, then you have to keep that legacy string, and the discussion with @mickenordin was that this PR would have to wait until end of support for versions earlier than v28. Yet, I added that the version field should be removed right away, because it is not compliant to the OCM schema, and remote implementations may fail to parse the payload.

[mickenordin]

I have reset the apiVersion to 1.0-proposal1 now, but retained the removal of the version attribute as per @glpatcern comment above.

[come-nc]

Is there still a point to merging this? It’s only removing version key, which seems useful?

[come-nc]

Is there still a point to merging this? It’s only removing version key, which seems useful?

[ArtificialOwl]

I have reset the apiVersion to 1.0-proposal1 now, but retained the removal of the version attribute as per @glpatcern comment above.

@glpatcern @mickenordin I am deeply sorry I could not spend more time with you to discuss about this...
I am fine removing 'version' but I think it would be nice to have at least one optional entry (set/defined in OCM protocol) that allow each provider to set a custom value about current advancement on the implementation of the protocol. Only for human/debugging purpose.
If 'version' is not good for you, do you think of anything else ?

Regards and Love,

[glpatcern]

@glpatcern @mickenordin I am deeply sorry I could not spend more time with you to discuss about this... I am fine removing 'version' but I think it would be nice to have at least one optional entry (set/defined in OCM protocol) that allow each provider to set a custom value about current advancement on the implementation of the protocol. Only for human/debugging purpose. If 'version' is not good for you, do you think of anything else ?

@ArtificialOwl you're welcome to join the OCM calls :) and a discussion here works as well if you don't have enough time.

IMHO what you are after is the capabilities entry. E.g. once you implement support for enforcing MFA when accessing a remote share (that's Micke's favorite), you can advertise that capability. In CERNBox we advertise invites, webdav-uri (because we support full URIs as share targets), and protocol-object (the OCM 1.1 new structure for multi-protocol), and once we'll implement e.g. the signed requests then we will add that too, along with the publicKey.

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)