Skip initial permission check on setName method for shared mounts

[JammingBen]

Description

Because renaming a mount should always be possible (see https://github.com/owncloud/enterprise/issues/4582#issuecomment-851328502).

Related Issue

• Fixes https://github.com/owncloud/enterprise/issues/4582
• fixes Rename option is visible, even though user only has share permissions #30325

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:
• Changelog item, see TEMPLATE

[JammingBen]

It seems that the check can be removed safely. This makes it possible to always rename a share regardless of its permissions. Not a 100% sure though, any opinions or thoughts @mrow4a ?

Another thing to mention: we still have the same issue with mounted storages. The UI shows the "rename"-action for a storage, but renaming results in an error. @jvillafanez You've dealt with a similar issue lately if I remember correctly? Something related to WND?

[jvillafanez]

Yes. #38709 is just for SMB. WND has similar changes too. I don't know if we want to make similar changes for all the storages.

[jvillafanez]

Could we check instead if the node refers to a shared mount? Something like "if node is not a shared mount and not writable, then throw forbidden exception" It seems more reasonable than just removing the permission check.

[JammingBen]

@jvillafanez

I don't know if we want to make similar changes for all the storages.

Depends on the solution I would say. According to Patrick, a mount point should always be allowed to be renamed. Your fix is heading in a different direction if I understood correctly, I assume because it was an easy fix for the problem? So that would be something for the future.

Could we check instead if the node refers to a shared mount? Something like "if node is not a shared mount and not writable, then throw forbidden exception" It seems more reasonable than just removing the permission check.

Good point, I updated the PR.

@phil-davis How do we handle the acceptance test in https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToRoot/moveReceivedShareOc10Issue30325.feature? Do we adjust it, or better remove entirely?

[phil-davis]

How do we handle the acceptance test in https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToRoot/moveReceivedShareOc10Issue30325.feature? Do we adjust it, or better remove entirely?

Remove it, and search for issue-30325 tags. Remove the skipOnOcV10 tags from the "good" scenarios. Those should now pass.

The same sort of thing is here: https://github.com/owncloud/core/blob/master/tests/acceptance/features/webUIRenameFolders/renameFoldersOc10Issue30325.feature

[phil-davis]

Code and test changes LGTM

[jvillafanez]

Little detail. Code looks good anyway.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information