Skip to content

Conversation

@VicDeo
Copy link
Member

@VicDeo VicDeo commented Aug 15, 2019

resubmit of #34260

Description

Return an absolute URL for OCM WebDav discovery endpoint and change the storage to work with absolute discovery URLs

Note: It makes OCM-based sharing in backward incompatible without patching apps/files_sharing/lib/External/Storage.php

Related Issue

Motivation and Context

To comply with OCM 1.0.0-proposal1 spec

How Has This Been Tested?

open URL owncloud.tld/ocm-provider/

expected

{"enabled":true,"apiVersion":"1.0-proposal1",
"endPoint":"http:\/\/owncloud.tld\/index.php\/apps\/federatedfilesharing",
"shareTypes":[{"name":"file",
"protocols":{"webdav":"http:\/\/owncloud.tld\/public.php\/webdav\/"}}]}

actual

{"enabled":true,"apiVersion":"1.0-proposal1",
"endPoint":"http:\/\/owncloud.tld\/index.php\/apps\/federatedfilesharing",
"shareTypes":[{"name":"file",
"protocols":{"webdav":"\/public.php\/webdav\/"}}]}

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@VicDeo VicDeo added this to the backlog milestone Aug 15, 2019
@VicDeo VicDeo self-assigned this Aug 15, 2019
@micbar
Copy link
Contributor

micbar commented Aug 27, 2019

@VicDeo Could you check on the failing acceptance test?

@VicDeo
Copy link
Member Author

VicDeo commented Oct 3, 2019

@micbar:

Could you check on the failing acceptance test?

None of OC/NC support absolute URL for OCM WebDav endpoint.
Support is implemented in this PR but Federated sharing is tested against daily that still doesn't support it

@phil-davis
Copy link
Contributor

Support is implemented in this PR but Federated sharing is tested against daily that still doesn't support it

The CI here is old. If you rebase then it will run the webUISharingExternal acceptance test suite with a federated server running 10.2.1 and again with a federated server running daily-master-qa - probably they will both fail?

If there are fails against a 10.2.1 federated server, then that means that if this change is released, e.g. in 10.4.0, and someone is using federation and upgrades one of their servers to 10.4.0, then various federated connections to/from the 10.2.1 server are going to have trouble.

Doesn't there have to be a way that the new code can still talk remotely to an older server?

Please tell me if I have not understood correctly.

@VicDeo
Copy link
Member Author

VicDeo commented Oct 3, 2019

@phil-davis everything is correct.

OTOH it could be also a mistake in the spec. The spec is still in a "proposal" state and I have no feedback to my question so far
cs3org/OCM-API#37 (comment)

What we can do to be future-proof is to extract the part using absolute URL in storage (apps/files_sharing/lib/External/Storage.php) and merge it separately.
Thus by the time apps/federatedfilesharing/lib/Controller/OcmController.php is updated to expose an absolute URL instead of relative one the storage will need no changes to support it

@VicDeo VicDeo force-pushed the make-webdavurl-absolute-2 branch from 726657d to 262c5b0 Compare May 12, 2020 10:31
@VicDeo
Copy link
Member Author

VicDeo commented Jul 1, 2020

Well, proposal has been merged...
We need to consider pushing this forward in near future.
The storage change has been moved into #37621

@VicDeo VicDeo force-pushed the make-webdavurl-absolute-2 branch from 262c5b0 to f156250 Compare July 1, 2020 12:14
@VicDeo VicDeo force-pushed the make-webdavurl-absolute-2 branch from f156250 to be715af Compare September 16, 2020 11:17
@DeepDiver1975
Copy link
Member

old aka dead -> close

@DeepDiver1975 DeepDiver1975 deleted the make-webdavurl-absolute-2 branch January 4, 2023 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OCM Returned endpoint relative

5 participants