Allow additional Sabre plugins in publicwebdav.php by julien-nc · Pull Request #35621 · nextcloud/server

julien-nc · 2022-12-06T13:12:53Z

It could be useful to have additional Sabre plugins in publicwebdav.php. I just added something similar to https://github.com/nextcloud/server/blob/master/apps/dav/appinfo/v1/webdav.php#L80-L82

Concrete use case: in the context of https://github.com/nextcloud/integration_openproject , we want to allow upload to a public share from a browser (from an OpenProject page actually). For that, we need a Sabre plugin to allow the origin website via the CORS headers.

Any strong reason why the OCA\DAV\Connector\Sabre::addPlugin event is not dispatched in publicwebdav.php?

apps/dav/appinfo/v1/publicwebdav.php

PVince81 · 2022-12-06T13:26:45Z

@julien-nc indeed this is long overdue 😄

apps/dav/appinfo/v1/publicwebdav.php

PVince81

👍

please check if it's the correct event dispatcher version / the latest non-deprecated one, if applicable

julien-nc · 2022-12-06T16:28:13Z

@PVince81 Do you mean switching to more modern event management?
There's now a new SabrePublicPluginEvent and we dispatch it with IEventDispatcher::dispatchTyped.

julien-nc · 2022-12-07T09:20:58Z

@PVince81 Are you fine with backporting this to 25, 24 and 23?

PVince81 · 2022-12-07T11:20:26Z

@PVince81 Are you fine with backporting this to 25, 24 and 23?

technically this would count as API addition, so not sure
I'd be fine for 25 as it's still fresh and new, but other releases are a bit too old for that

@AndyScherzinger

AndyScherzinger · 2022-12-07T12:31:46Z

@PVince81 Are you fine with backporting this to 25, 24 and 23?

I'd be fine with backporting since it is an addition and not breaking change, so I#d be okay with it

juliusknorr

Looks good 👍

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>

julien-nc · 2022-12-13T10:18:41Z

/backport to stable25

julien-nc · 2022-12-13T10:20:30Z

Drone failure is not related.

julien-nc · 2022-12-13T10:29:39Z

@PVince81 I agree with @AndyScherzinger, it does not break anything so it can be backported in 23 and 24.
But I'm not very objective since we need this for an app that supports 23 and 24...

technically this would count as API addition, so not sure

I see this as a "fix" for an inconsistency between public and private WebDav endpoints.

What do you think?

PVince81 · 2022-12-13T10:58:50Z

@julien-nc ok, fine

julien-nc · 2022-12-13T11:02:22Z

/backport to stable24

julien-nc · 2022-12-13T11:02:27Z

/backport to stable23

ChristophWurst · 2022-12-15T12:14:58Z

Drone failure is not related.

PHPDoc is needed for OCP\SabrePublicPluginEvent::SabrePublicPluginEvent

well well well

ChristophWurst · 2022-12-15T12:16:46Z

Moreover autoloaders were not updated. Does this code even work?

ChristophWurst · 2022-12-15T12:17:44Z

lib/public/SabrePublicPluginEvent.php

+ */
+namespace OCP;
+
+class SabrePublicPluginEvent extends SabrePluginEvent {


The naming rule was not followed: https://docs.nextcloud.com/server/18/developer_manual/app/events.html#naming-scheme

PVince81 · 2022-12-16T10:41:00Z

need manual backport + include #35789

PVince81 · 2022-12-20T07:47:23Z

stable25: #35817
stable24: #35818
stable23: #35819

julien-nc added enhancement 3. to review Waiting for reviews feature: dav labels Dec 6, 2022

julien-nc added this to the Nextcloud 26 milestone Dec 6, 2022

julien-nc requested review from PVince81 and juliusknorr December 6, 2022 13:12

julien-nc mentioned this pull request Dec 6, 2022

Add Sabre plugin to whitelist the OP instance in the CORS headers nextcloud/integration_openproject#285

Closed

github-advanced-security bot found potential problems Dec 6, 2022

View reviewed changes

apps/dav/appinfo/v1/publicwebdav.php Fixed Show fixed Hide fixed

apps/dav/appinfo/v1/publicwebdav.php Fixed Show fixed Hide fixed

apps/dav/appinfo/v1/publicwebdav.php Fixed Show fixed Hide fixed

PVince81 reviewed Dec 6, 2022

View reviewed changes

apps/dav/appinfo/v1/publicwebdav.php Outdated Show resolved Hide resolved

julien-nc force-pushed the enh/noid/sabre-plugin-publicwebdav branch from 2a982e3 to 22e02ef Compare December 6, 2022 13:33

julien-nc requested a review from PVince81 December 6, 2022 13:36

github-advanced-security bot found potential problems Dec 6, 2022

View reviewed changes

apps/dav/appinfo/v1/publicwebdav.php Fixed Show fixed Hide fixed

apps/dav/appinfo/v1/publicwebdav.php Fixed Show fixed Hide fixed

apps/dav/appinfo/v1/publicwebdav.php Fixed Show fixed Hide fixed

julien-nc force-pushed the enh/noid/sabre-plugin-publicwebdav branch from 22e02ef to c0703ed Compare December 6, 2022 14:00

github-advanced-security bot found potential problems Dec 6, 2022

View reviewed changes

apps/dav/appinfo/v1/publicwebdav.php Fixed Show fixed Hide fixed

apps/dav/appinfo/v1/publicwebdav.php Fixed Show fixed Hide fixed

apps/dav/appinfo/v1/publicwebdav.php Fixed Show fixed Hide fixed

PVince81 approved these changes Dec 6, 2022

View reviewed changes

This comment was marked as resolved.

Sign in to view

juliusknorr approved these changes Dec 7, 2022

View reviewed changes

julien-nc added 2 commits December 12, 2022 13:56

allow additional Sabre plugins in publicwebdav.php

da9ffe4

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>

use modern event dispatch

2d860d2

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>

julien-nc force-pushed the enh/noid/sabre-plugin-publicwebdav branch from 1d9b4bc to 2d860d2 Compare December 12, 2022 13:04

backportbot-nextcloud bot added the backport-request label Dec 13, 2022

julien-nc merged commit 104a109 into master Dec 13, 2022

julien-nc deleted the enh/noid/sabre-plugin-publicwebdav branch December 13, 2022 10:20

backportbot-nextcloud bot mentioned this pull request Dec 13, 2022

[stable25] Allow additional Sabre plugins in publicwebdav.php #35749

Closed

This was referenced Dec 13, 2022

[stable23] Allow additional Sabre plugins in publicwebdav.php #35751

Closed

[stable24] Allow additional Sabre plugins in publicwebdav.php #35752

Closed

ChristophWurst reviewed Dec 15, 2022

View reviewed changes

ChristophWurst mentioned this pull request Dec 15, 2022

Make the LoginController strict #35784

Merged

6 tasks

julien-nc mentioned this pull request Dec 15, 2022

Rename SabrePublicPluginEvent to BeforeSabrePubliclyLoadedEvent #35789

Merged

This was referenced Dec 20, 2022

[stable23] Allow additional Sabre plugins in publicwebdav.php #35819

Closed

[stable24] Allow additional Sabre plugins in publicwebdav.php #35818

Merged

[stable25] Allow additional Sabre plugins in publicwebdav.php #35817

Merged

skjnldsv removed the backport-request label Jan 15, 2024

Uh oh!

Conversation

julien-nc commented Dec 6, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PVince81 commented Dec 6, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PVince81 left a comment

Choose a reason for hiding this comment

Uh oh!

julien-nc commented Dec 6, 2022

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

julien-nc commented Dec 7, 2022

Uh oh!

This comment was marked as resolved.

PVince81 commented Dec 7, 2022

Uh oh!

AndyScherzinger commented Dec 7, 2022

Uh oh!

juliusknorr left a comment

Choose a reason for hiding this comment

Uh oh!

julien-nc commented Dec 13, 2022

Uh oh!

julien-nc commented Dec 13, 2022

Uh oh!

julien-nc commented Dec 13, 2022

Uh oh!

PVince81 commented Dec 13, 2022

Uh oh!

julien-nc commented Dec 13, 2022

Uh oh!

julien-nc commented Dec 13, 2022

Uh oh!

ChristophWurst commented Dec 15, 2022

Uh oh!

ChristophWurst commented Dec 15, 2022

Uh oh!

ChristophWurst Dec 15, 2022

Choose a reason for hiding this comment

Uh oh!

PVince81 commented Dec 16, 2022

Uh oh!

PVince81 commented Dec 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants