Fix public link path for persistent locks

[PVince81]

Description

When querying lock info on public links, always return a path relative
to the public webdav endpoint.

Related Issue

Fixes #34225

Motivation and Context

How Has This Been Tested?

• Unit tests
  With "/public/share" being the link share:
• TEST: set lock on "/public", query locks on "public.php/webdav/sub" returns lock root "public.php/webdav/"
• TEST: set lock on "/public/share", query locks on "public.php/webdav/sub" returns lock root "public.php/webdav/"
• TEST: set lock on "/public/share/sub", query locks on "public.php/webdav/sub" returns lock root "public.php/webdav/sub"
• TEST: set lock on "/public", set another lock on "public.php/webdav/sub" returns empty lock root ""
• TEST: set lock on "/public/share/sub", set another lock on "public.php/webdav/sub" returns lock root "/sub"

Screenshots (if appropriate):

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:

Open tasks:

• Backport (if applicable set "backport-request" label and remove when the backport was done)

[codecov]

Codecov Report

Merging #34229 into master will decrease coverage by <.01%.
The diff coverage is 45.45%.

@@             Coverage Diff              @@
##             master   #34229      +/-   ##
============================================
- Coverage     64.75%   64.75%   -0.01%     
- Complexity    18362    18364       +2     
============================================
  Files          1198     1198              
  Lines         69533    69540       +7     
  Branches       1281     1281              
============================================
+ Hits          45026    45028       +2     
- Misses        24134    24139       +5     
  Partials        373      373

Flag	Coverage Δ	Complexity Δ
#javascript	53.09% <ø> (ø)	0 <ø> (ø)	⬇️
#phpunit	66.1% <45.45%> (-0.01%)	18364 <0> (+2)

Impacted Files	Coverage Δ	Complexity Δ
apps/dav/lib/Files/FileLocksBackend.php	84.61% <45.45%> (-5.53%)	23 <0> (+2)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c255c6c...793d0d4. Read the comment docs.

[codecov]

Codecov Report

Merging #34229 into master will increase coverage by <.01%.
The diff coverage is 90.9%.

@@             Coverage Diff              @@
##             master   #34229      +/-   ##
============================================
+ Coverage     64.76%   64.76%   +<.01%     
- Complexity    18363    18365       +2     
============================================
  Files          1198     1198              
  Lines         69538    69545       +7     
  Branches       1281     1281              
============================================
+ Hits          45035    45042       +7     
  Misses        24130    24130              
  Partials        373      373

Flag	Coverage Δ	Complexity Δ
#javascript	53.09% <ø> (ø)	0 <ø> (ø)	⬇️
#phpunit	66.11% <90.9%> (ø)	18365 <0> (+2)	⬆️

Impacted Files	Coverage Δ	Complexity Δ
apps/dav/lib/Files/FileLocksBackend.php	91.02% <90.9%> (+0.88%)	23 <0> (+2)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 06e9bf6...207e059. Read the comment docs.

[PVince81]

@davitol FYI see test plan in the section above

@DeepDiver1975 please review

[davitol]

Spotted by @individual-it:

with a PROPFIND a client can find out the locktoken, so a nasty client can overwrite resources even they are locked by an other user.

Steps to reproduce

1. User shares a public link and locks a folder
2. The public does a PROPFIND and gets tould the locktoken
3. Using that token the public can overwrite locked files

[PVince81]

Idea: only expose the lock token if the lock owner is NULL, which refers to a lock that has been created through the public API. if the lock was created by a logged in user, don't reveal it.

@DeepDiver1975 thoughts ?

[PVince81]

ok I think I'll always hide the lock token for public users. there is no UI to unlock there and whoever created the lock already has the token somewhere

the file owner can still unlock with the UI as they have the token

[PVince81]

note: also tested trying to lock "sub" through public webdav while "public" was locked.
the error response also contains the correct non-leaked lock root:

curl -u 6OVGtYzOTEmozQB: -X LOCK http://localhost/owncloud/public.php/webdav/sub -d "<?xml version='1.0' encoding='UTF-8'?><d:lockinfo xmlns:d='DAV:'> <d:lockscope><d:exclusive/></d:lockscope></d:lockinfo>" -H 'Depth: infinite'
<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\ConflictingLock</s:exception>
  <s:message/>
  <d:no-conflicting-lock>
    <d:href/>
  </d:no-conflicting-lock>
</d:error>

[PVince81]

• not sure if bug: lock root as returned in lockdiscovery contains full path with public.php while lock root in conflict exception response contains a relative path

[PVince81]

https://tools.ietf.org/html/rfc4918#section-6.5 says that

Servers MAY make lock tokens publicly readable (e.g., in the DAV:
lockdiscovery property).

which means we should be able to hide them.

However Sabre still returns the lock even when empty... Will need an upstream patch.

[PVince81]

Upstream PR: sabre-io/dav#1130

[individual-it]

@ownclouders rebase

[ownclouders]

Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently ⚠️

[ownclouders]

Automated rebase with GitMate.io was successful! 🎉

[individual-it]

pushed a couple of acceptance tests
@PVince81 @phil-davis @davitol please review test code and expectations

[phil-davis]

@individual-it acceptance tests look good, just little comments

[individual-it]

@phil-davis fixed the things you commented on

Additionally added tests that check the locktoken. See #34229 (comment)
The current tests reflect the current behaviour, as soon this issue is fixed the test need to be adjusted (delete the wrong line and activate the line below)

[PVince81]

stable10: #34267

I forgot to push the solution for #34229 (comment), it will be in a separate PR