Skip to content

Conversation

@PVince81
Copy link
Contributor

Whenever outgoing shares are disabled, still allow ajax requests to make
it possible to use the Webdav interface in the public link page.

Please note that disabling outgoing shares isn't strong anyway as
someone could abuse the ajax endpoints to access files anyway. To
properly disable remote sharing, public link sharing must be disabled
too.

@oparoz that would also fix the text preview, was it still using Webdav ?

This will be required when the files UI uses Webdav.

Please review @LukasReschke @icewind1991 @MorrisJobke @schiesbn @th3fallen

@PVince81
Copy link
Contributor Author

Required by #16902 and the text preview #16464

@PVince81
Copy link
Contributor Author

PVince81 commented Aug 6, 2015

@LukasReschke @oparoz what do you think ?

@oparoz
Copy link
Contributor

oparoz commented Aug 6, 2015

I think it feels strange if core and apps have to use an endpoint located in files_sharing to access public files since it's a completely different URL from the standard one, but at the same times, it makes sense since there is no public page without sharing enabled.

Isn't there a lot of code duplication between the private and public endpoints? That would be my only concern. Apart from that this fix should cover most situations, if it works through proxies and firewalls. 👍

@PVince81
Copy link
Contributor Author

PVince81 commented Aug 7, 2015

It would indeed be nice to have the same endpoints for private and public. There was an old idea of creating virtual users for public links but I haven't heard of it since a long time.

@oparoz
Copy link
Contributor

oparoz commented Aug 7, 2015

I think this can't happen as long as there isn't a token decoder in core (as opposed to in an app).

@PVince81 PVince81 force-pushed the publicwebdav-allowajaxwhendisabled branch from f79d88b to 8e15043 Compare August 26, 2015 15:46
@scrutinizer-notifier
Copy link

A new inspection was created.

@PVince81
Copy link
Contributor Author

Rebased

@oparoz
Copy link
Contributor

oparoz commented Sep 15, 2015

We need one more vote: @DeepDiver1975 @Xenopathic @nickvergessen

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it should have some kind of "same origin" check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Usually I think we can't completely block remote access to existing public shares since people can already access them in the web UI, this is only giving them another view. And this also only applies for cases where people mounted the public shares directly but then the admin decided to disable federation afterwards.

@LukasReschke what do you think ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@LukasReschke what do you think ?

@LukasReschke ;)

Copy link
Member

Choose a reason for hiding this comment

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

Acceptable.

Whenever outgoing shares are disabled, still allow ajax requests to make
it possible to use the Webdav interface in the public link page.

Please note that disabling outgoing shares isn't strong anyway as
someone could abuse the ajax endpoints to access files anyway. To
properly disable remote sharing, public link sharing must be disabled
too.
@PVince81 PVince81 force-pushed the publicwebdav-allowajaxwhendisabled branch from 8e15043 to c8a6fea Compare September 22, 2015 10:20
@MorrisJobke
Copy link
Contributor

This will be required when the files UI uses Webdav.

@PVince81 Do we want to ship this already with 8.2 and then move the files UI to WebDAV in 9.0 but be able to use this on other (federated cloud sharing) 8.2 instances already properly?

cc @nickvergessen @LukasReschke Review please :)

@PVince81
Copy link
Contributor Author

@MorrisJobke if we only care about files UI with Webdav, 9.0 is fine.
However this fix also helps fixing #16464 but might need additional changes to make the text preview send the proper header.

@LukasReschke
Copy link
Member

👍

@PVince81
Copy link
Contributor Author

Dammit, I set "to release" too early. Still missing a second thumbs up.

@MorrisJobke
Copy link
Contributor

Dammit, I set "to release" too early. Still missing a second thumbs up.

#17601 (comment) ;)

@MorrisJobke
Copy link
Contributor

I'm also fine with this 👍

DeepDiver1975 added a commit that referenced this pull request Sep 29, 2015
…abled

Allow ajax requests on public webdav interface
@DeepDiver1975 DeepDiver1975 merged commit 580a961 into master Sep 29, 2015
@DeepDiver1975 DeepDiver1975 deleted the publicwebdav-allowajaxwhendisabled branch September 29, 2015 11:21
@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants