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

Metadata must be set a editable for PROPPATCH #41390

Merged
merged 1 commit into from Nov 14, 2023

Conversation

ArtificialOwl
Copy link
Member

@ArtificialOwl ArtificialOwl commented Nov 11, 2023

With the implementation of metadata and its compatibility with webdav PROPPATCH request, and to avoid unexpected behavior, 2 things needs to be implemented:

  • manage a list of remotely editable metadata,
  • compare edit permissions on metadata and actual rights on file for initiator

throws exception on fail, might be better to just log a warning but keep the process running ?

@ArtificialOwl ArtificialOwl added the 2. developing Work in progress label Nov 11, 2023
@ArtificialOwl ArtificialOwl added this to the Nextcloud 28 milestone Nov 11, 2023
@ArtificialOwl ArtificialOwl added the 3. to review Waiting for reviews label Nov 11, 2023
* Disable modification of the displayname property for files and
* folders via PROPPATCH. See PROPFIND for more information.
*/
$propPatch->handle(self::DISPLAYNAME_PROPERTYNAME, function ($displayName) {

Check notice

Code scanning / Psalm

MissingClosureParamType Note

Parameter $displayName has no provided type
apps/dav/lib/Connector/Sabre/FilesPlugin.php Fixed Show fixed Hide fixed
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/limit-proppatch-metadata branch from 8235cdc to 9cac531 Compare November 11, 2023 11:56
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/limit-proppatch-metadata branch from 9cac531 to 76b44a5 Compare November 11, 2023 12:10
@ArtificialOwl
Copy link
Member Author

cypress error not related

@blizzz blizzz mentioned this pull request Nov 13, 2023
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/limit-proppatch-metadata branch 2 times, most recently from 1ebc2e1 to e25ebe6 Compare November 13, 2023 17:09
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/limit-proppatch-metadata branch 2 times, most recently from 11395c9 to 8e4f8e9 Compare November 13, 2023 17:38
@ArtificialOwl
Copy link
Member Author

cypress error does not seems related

@ArtificialOwl ArtificialOwl force-pushed the enh/noid/limit-proppatch-metadata branch 3 times, most recently from edd8494 to b9f5552 Compare November 14, 2023 08:12
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/limit-proppatch-metadata branch 2 times, most recently from 61a26fc to a460d61 Compare November 14, 2023 11:01
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
@blizzz blizzz force-pushed the enh/noid/limit-proppatch-metadata branch from a460d61 to 847c687 Compare November 14, 2023 13:38
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small improvements needed on phpdoc for the EDIT_* enum

* @param string $key metadata key
*
* @inheritDoc
* @return int edit permission
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @return int edit permission
* @return IMetadataValueWrapper::EDIT_* edit permission level

@@ -133,7 +134,20 @@ public function getKnownMetadata(): IFilesMetadata;
* @param string $key metadata key
* @param string $type metadata type
* @param bool $indexed TRUE if metadata can be search
* @param int $editPermission remote edit permission via Webdav PROPPATCH
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param int $editPermission remote edit permission via Webdav PROPPATCH
* @param IMetadataValueWrapper::EDIT_* $editPermission remote edit permission level via Webdav PROPPATCH

* (Webdav PROPPATCH)
*
* @param string $key metadata key
* @param int $permission remote edit permission
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param int $permission remote edit permission
* @param IMetadataValueWrapper::EDIT_* $permission remote edit permission level

*
* @param string $key metadata key
*
* @return int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @return int
* @return IMetadataValueWrapper::EDIT_* edit permission level

* set remote edit permission
* (Webdav PROPPATCH)
*
* @param int $permission edit permission
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param int $permission edit permission
* @param IMetadataValueWrapper::EDIT_* $permission edit permission level

* get remote edit permission
* (Webdav PROPPATCH)
*
* @return int edit permission
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @return int edit permission
* @return IMetadataValueWrapper::EDIT_* edit permission level

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me do this on a separated PR, post merge

@AndyScherzinger AndyScherzinger merged commit 93ea634 into master Nov 14, 2023
48 of 50 checks passed
@AndyScherzinger AndyScherzinger deleted the enh/noid/limit-proppatch-metadata branch November 14, 2023 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress 3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants