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

Cors origin allowed list check #37716

Open
aleixq opened this issue Apr 13, 2023 · 4 comments · May be fixed by #40537
Open

Cors origin allowed list check #37716

aleixq opened this issue Apr 13, 2023 · 4 comments · May be fixed by #40537
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap developer experience enhancement integration security

Comments

@aleixq
Copy link

aleixq commented Apr 13, 2023

I think is useful to be able to operate with data from Nextcloud from other websites cms such as moodle, drupal, wp's while using different domains. The app webappassword lets use the DAV resources, it uses the sabre pluggable system to modify the response checking against a list of allowed domains. But I cannot find a way to use share api from different domain as files_sharing app is not providing neither the necessary preflight OPTIONS route, nor the @CORS annotation in the relevant controller methods. Also using it will allow every domain to gain access, so I think it may open security breach in sites.

workaround for parts of

It's discussed also in digital-blueprint/webapppassword#1 , but I cannot find a way to listen for kernel-response events like https://symfony.com/doc/current/reference/events.html#kernel-response (as using symfony events is discouraged). So the workaround I end up using to be able to make requests from other domain is extending the shareapi controller to modify the response( https://gitlab.com/communia/files_sharing_webapppassword information about it is in app issue tracker: ). Of course, feedback about the ugliness of this workaround is welcome.

proposal

Maybe nextcloud could provide it natively, modifying the existing behavior in https://github.com/nextcloud/server/blob/master/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php#L116 to use a similar logic as I am using in the checkOrigin trait (https://gitlab.com/communia/files_sharing_webapppassword/-/blob/main/lib/Controller/AccessControl.php#L25 ) .

One thing that will remain is how to define somewhere the automatic adding of preflighted OPTIONS route if cors is used...

@aleixq aleixq added 0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement labels Apr 13, 2023
@aleixq
Copy link
Author

aleixq commented Apr 28, 2023

Adding an application to be used as frontend to set the allowed origins, it's in separated repo at: Communia/cors_origin_filter_settings . (also reported in PR)

@szaimen
Copy link
Contributor

szaimen commented Apr 28, 2023

Hi, did you know about this app?
https://apps.nextcloud.com/apps/webapppassword

@aleixq
Copy link
Author

aleixq commented Apr 28, 2023

Hi, yes, but it has some limitations, as it only covers webdav and with a commit I have recently contributed it also adds share api support.
But as I said it has some limitations, it needs to reimplement every controller methods of the desired endpoint, so we(me and main developer) are agree in digital-blueprint/webapppassword#61 (comment) that a nextcloud native way of offfering this will be better.
That's why I work on this proposal

@aleixq
Copy link
Author

aleixq commented Apr 29, 2023

Just note also that PR #37896 together with https://github.com/Communia/cors_origin_filter_settings would also make easier an enhanced filepicker such as one asked for in #2028 and #3131 , or the implemented one in https://github.com/julien-nc/nextcloud-webdav-filepicker that needs curl to workaround the cors problem (see more in https://github.com/julien-nc/nextcloud-webdav-filepicker#create-nextcloud-share-links )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap developer experience enhancement integration security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants