Core: public link share response incorrectly includes share_folder in file_target

[phil-davis]

Steps to reproduce

From PR #38290 test scenarios: tests/acceptance/features/apiProvisioning-v1/disableUser.feature

  Scenario: getting public link share shared by disabled user using the new public WebDAV API
    Given user "Alice" has been created with default attributes and skeleton files
    And user "Alice" has created a public link share with settings
      | path        | /textfile0.txt |
      | permissions | read           |
    And user "Alice" has been disabled
    When the public downloads the last public shared file using the new public WebDAV API
    Then the HTTP status code should be "404"

  Scenario: getting public link share shared by disabled user using the old public WebDAV API
    Given user "Alice" has been created with default attributes and skeleton files
    And user "Alice" has created a public link share with settings
      | path        | /textfile0.txt |
      | permissions | read           |
    And user "Alice" has been disabled
    When the public downloads the last public shared file using the old public WebDAV API
    Then the HTTP status code should be "200"

And the same for tests/acceptance/features/apiProvisioning-v1/disableUser.feature

Expected behaviour

The results should be consistent. When user "Alice" is disabled, the the public link share that was already made by user "Alice" should either be accessible. or not accessible.

Actual behaviour

~But with the new public WebDAV API it is not accessible, and with the old public WebDAV API it is accessible.~

See comment https://github.com/owncloud/core/issues/38291#issuecomment-759961433 - the root problem seems to be the value of file_target in the response to a public link share create request.

Server configuration

CI with current core master.

[phil-davis]

First, someone needs to decide if a public link share of a now-disabled user should be accessible, or not.

Then someone can make the code do it.

[hodyroff]

The only effect a disabled user shall have is that the user can't login any longer.
This was already discussed when the feature was implemented: https://github.com/owncloud/core/issues/12601

[phil-davis]

So, "Scenario: getting public link share shared by disabled user using the new public WebDAV API" should succeed - get a 200 rather than a 404.

@micbar add to the oC10 project?

If this is easily fixed/made consistent then the test scenarios will be correctly-relevant for OCIS... (otherwise we will need to adjust the test scenarios and have tags... to indicate which scenarios are the correct behaviour and which are demonstrating the current "undesired" oC10 behaviour)

[phil-davis]

Note: we are currently looking a the detail of this - the test scenarios are giving some different results in drone CI than what @saw-jan got locally. So we need to first be sure about what is the reason for the different behaviour.

[saw-jan]

When the public share link is created, we get the following response:
The response should not have /Shares in file_target, because it is a public link (not a "normal" user or group share)

SimpleXMLElement Object
        │ (
        │     [meta] => SimpleXMLElement Object
        │         (
        │             [status] => ok
        │             [statuscode] => 100
        │             [message] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [totalitems] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [itemsperpage] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │         )
        │ 
        │     [data] => SimpleXMLElement Object
        │         (
        │             [id] => 192
        │             [share_type] => 3
        │             [uid_owner] => Alice
        │             [displayname_owner] => Alice Hansen
        │             [permissions] => 1
        │             [stime] => 1610604519
        │             [parent] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [expiration] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [token] => hhke8kKGhbsIhi0
        │             [uid_file_owner] => Alice
        │             [displayname_file_owner] => Alice Hansen
        │             [additional_info_owner] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [additional_info_file_owner] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [path] => /textfile0.txt
        │             [mimetype] => text/plain
        │             [storage_id] => home::Alice
        │             [storage] => 1549
        │             [item_type] => file
        │             [item_source] => 2147627928
        │             [file_source] => 2147627928
        │             [file_parent] => 2147627920
        │             [file_target] => /Shares/textfile0.txt
        │             [name] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [url] => http://172.17.0.1/core/index.php/s/hhke8kKGhbsIhi0
        │             [mail_send] => 0
        │             [attributes] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │         )
        │ 
        │ )

[phil-davis]

@micbar the test code was making use of file_target in the process of constructing the way to access the share. For user and group shares it is all good, because the resource is delivered to the share receiver in their file system at /Shares/textfile0.txt

But for public links the "share" is not delivered into the file system of any "share receiver". It is a free-standing link.

So I think that the problem to be fixed is that, in the case of public link shares, file_target should not have share_folder prepended.

I guess that the real-world clients are not trying to use file_target when they display the public link full URL, and when they do "add to your ownCloud" etc. And so this problem has not been noticed "in the wild".

[saw-jan]

with OCIS, it looks fine
response from OCIS server

SimpleXMLElement Object
        │ (
        │     [meta] => SimpleXMLElement Object
        │         (
        │             [status] => ok
        │             [statuscode] => 200
        │             [message] => OK
        │         )
        │ 
        │     [data] => SimpleXMLElement Object
        │         (
        │             [id] => cIWZOQnlJtVtyVu
        │             [share_type] => 3
        │             [uid_owner] => Alice
        │             [displayname_owner] => Alice Hansen
        │             [permissions] => 1
        │             [stime] => 1610606808
        │             [parent] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [expiration] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [token] => hkgMFlwxWUIOUNp
        │             [uid_file_owner] => Alice
        │             [displayname_file_owner] => Alice Hansen
        │             [additional_info_owner] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [additional_info_file_owner] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [state] => 0
        │             [path] => /textfile0.txt
        │             [item_type] => file
        │             [mimetype] => text/plain
        │             [storage_id] => 1284d238-aa92-42ce-bdc4-0b0000009157
        │             [storage] => 0
        │             [item_source] => MTI4NGQyMzgtYWE5Mi00MmNlLWJkYzQtMGIwMDAwMDA5MTU3OjAxZGZkY2MwLTkyMGItNDIyYy05ZTUwLTZmOTJkMWVhN2VkYQ==
        │             [file_source] => MTI4NGQyMzgtYWE5Mi00MmNlLWJkYzQtMGIwMDAwMDA5MTU3OjAxZGZkY2MwLTkyMGItNDIyYy05ZTUwLTZmOTJkMWVhN2VkYQ==
        │             [file_parent] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [file_target] => /textfile0.txt
        │             [share_with_additional_info] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [mail_send] => 0
        │             [name] => SimpleXMLElement Object
        │                 (
        │                 )
        │ 
        │             [url] => https://localhost:9200/#/s/hkgMFlwxWUIOUNp
        │         )
        │ 
        │ )

[saw-jan]

In OC10 server, file_target contains /Shares in the response only if OC is set to receive shares inside Shares folder.
i.e. In config.php

...
'share_folder' => 'Shares',
...

Otherwise, the response is identical to OCIS server

[phil-davis]

It looks easy to fix in lib/private/Share20/Manager.php createShare()
PR incoming...

[VicDeo]

@phil-davis I think lib/private/Share20/DefaultShareProvider.php create() is a better place (line 751 or so)

But I'm not sure if this is a correct approach...

[phil-davis]

@phil-davis I think lib/private/Share20/DefaultShareProvider.php create() is a better place (line 751 or so)

But I'm not sure if this is a correct approach...

@VicDeo please review PR #38294 - I edited the place where share_folder is being prepended.

[VicDeo]

@phil-davis So the initially reported inconsistency was related to the usage of file_target in tests, wasn't it?

[phil-davis]

@phil-davis So the initially reported inconsistency was related to the usage of file_target in tests, wasn't it?

We happened to notice this because we were adding more public link share tests, and we were using the url plus file_target from the response to then access the public link. That did not work because there was a "bonus" /Shares in file_target

I don't think that existing real clients make use of file_target when working with public link shares. There is a path field in the response, which is correct and is probably what they use. IMO that is why this problem has not been reported by clients.

[VicDeo]

@phil-davis In addition to https://github.com/owncloud/core/pull/38295 we might need to have a repair step fixing file_target for all existing public links. That's why I was asking.

[phil-davis]

@phil-davis In addition to #38295 we might need to have a repair step fixing file_target for all existing public links. That's why I was asking.

OK - is that stored for the share (not calculated on-the-fly)?

A repair step should be a NOOP for most production installs - if share_folder is not set, then there is nothing to repair. If share_folder is set, then the repair only needs to scan through the public link shares.