Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow full paths #1739

Merged
merged 10 commits into from
Sep 13, 2021
Merged

Allow full paths #1739

merged 10 commits into from
Sep 13, 2021

Conversation

butonic
Copy link
Contributor

@butonic butonic commented May 27, 2021

A continuation of #1605, labkode#183 and #1691.

@phil-davis @individual-it I will add the failing tests to the expected failures because there is now a new scenario which is not covered by the oc10 tests. When using a global namespace on top of eos it is possible to do a PROPFIND on /eos/users/e/ when setting the share_prefix="/", files_namespace="/" and webdav_namespace="/" in the ocs/ocdav config sections.

The implentation in this PR allows using global paths, but when a share is not mounted it will leave the internal path, exactly like oc10. It leaks the internal path ... yes, just like oc10 ... so we need to fix it anyway. This will be addressed with the spaces api. For now I'll add the failing test cases to expected failures.

When the spaces API is in place we can clean this up more elegantly because shares will be accessed via the new /dav/spaces endpoint.

The faling tests expect the shares to be mounted in the users home. But the config is actually setting up a reva instance that jails users into home AND a shares folder, so I think unless an acceptance test contains:

    And the administrator has set the default folder for received shares to "Shares"
    And auto-accept shares has been disabled

it will fail, which makes sense.

@butonic
Copy link
Contributor Author

butonic commented May 28, 2021

maybe we can use the share_prefix to trigger trimming the paths when they have not been accepted, yet. That might let some tests pass...

@butonic
Copy link
Contributor Author

butonic commented May 28, 2021

After discussing this with @phil-davis and @individual-it we will introduce a new expose_global_namespace flag to trigger exposing global paths.

@butonic
Copy link
Contributor Author

butonic commented May 28, 2021

Looking at the test results 81a3b1d actually sooms to work without an additional flag:

  • When shares are jailed into the users home the webdav url to access them is https://domain.tld/webdav/Shares/<sharename>. This is the default ocis configuration and we set share_prefix='/Shares' so the ocs list shares with me endpoint constructs paths by using the /Shares prefix and the basename of the shared resource.
  • cernBox uses a global namespace where the webdav url to access a share is, eg. https://domain.tld/webdav/users/m/marie/path/to/share. Setting share_prefix='/' will make the ocs list shares with me endpoint just use the path returned from the gateway as is: global.

Note that to jails users into their home ocis also has to set the ocdav files_namespace=/users and webdav_namespace=/home.
cernBox uses files_namespace=/ (I think ... maybe they need /users here as well) and webdav_namespace=/ to expose the global namespace on the webdav endpoints.

We are working on a owncloud share storage provider that will use an oc10 database to mount shares exactly like in oc10. We need this for migration purposes to run oc10 and ocis in parallel. We will then move towards the spaces api to give clients a more direct way to talk to the correct storage providers.

@butonic
Copy link
Contributor Author

butonic commented May 28, 2021

@ishank011 let me know if this produces the paths you need for the global namespace.

@butonic
Copy link
Contributor Author

butonic commented May 28, 2021

@ishank011 @labkode what is that licence check complaining about? AFAICT this can be merged when it has been tested in the global namespace.

@labkode
Copy link
Member

labkode commented May 28, 2021

@butonic CI is green! @ishank011 can you please give another pair of eyes?

@labkode labkode requested a review from ishank011 May 28, 2021 13:20
@labkode
Copy link
Member

labkode commented Jul 2, 2021

@labkode @ishank011 re-check if part of fork

@labkode
Copy link
Member

labkode commented Jul 26, 2021

@butonic can you rebase this one?

@butonic
Copy link
Contributor Author

butonic commented Jul 28, 2021

@labkode @ishank011 @micbar rebased and dropped cosmetic changes to the expected failures files

@butonic butonic force-pushed the allow-full-paths branch 4 times, most recently from 2e694b0 to 0efb2c8 Compare August 10, 2021 14:31
@butonic
Copy link
Contributor Author

butonic commented Aug 18, 2021

@labkode can you check this out?

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@labkode labkode merged commit afa06f7 into cs3org:master Sep 13, 2021
glpatcern pushed a commit to glpatcern/reva that referenced this pull request Sep 23, 2021
@butonic butonic deleted the allow-full-paths branch October 1, 2021 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants