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

The API of the Thumbnails invalidate the Session Cookie on WebDAV on OC9 #22893

Closed
javiergonzper opened this issue Mar 7, 2016 · 16 comments · Fixed by #22901
Closed

The API of the Thumbnails invalidate the Session Cookie on WebDAV on OC9 #22893

javiergonzper opened this issue Mar 7, 2016 · 16 comments · Fixed by #22901
Assignees
Milestone

Comments

@javiergonzper
Copy link

Steps to reproduce

It was discover using during the development of the remote Thumbnails on iOS

Expected behaviour

The server should not return on all the WebDAV 401 error if the cookie was generated using the Thumbnail API

Actual behaviour

If the first request and also the cookie of a session is made on the Thumbnail API the cookie of session do not works properly on the other WebDAV requests because all WebDAV request return a 401 error.

NOTE 1:
On all the WebDAV request we add the Authentication header and the cookies in order to the server prioritize the cookie over the Authentication header but if the cookie has been invalidated the server should use the Authentication header.

NOTE 2:
On OC8.2.2 works as we expected, looks like it start to fail on OC9

Server configuration

ownCloud version:
{"installed":true,"maintenance":false,"version":"9.1.0.0","versionstring":"9.1.0 pre alpha","edition":""}

PS: In my opinion this bug it is important because if we release OC9 with this bug will break the iOS app because the user enter in a loop of "Error credentials" all the time.
If this bug it is not fixed we will must to add a control to not use cookies on the thumbnail requests for versions 9.0... and that it is really ugly

@javiergonzper javiergonzper added this to the 9.0-current milestone Mar 7, 2016
@PVince81
Copy link
Contributor

PVince81 commented Mar 7, 2016

@LukasReschke @rullzer

@PVince81
Copy link
Contributor

PVince81 commented Mar 7, 2016

Can't the iOS app make the first request on an Webdav endpoint to get the cookie from there, before calling the thumbnail API ?

How does the Android app do it ? @davivel

@LukasReschke
Copy link
Member

The app really really really really really really really should only use the cookie getting from the WebDAV endpoint. Cookies from any other endpoint should not be reused. (they all behave kinda differently and some are closed for security reasons...)

@javiergonzper
Copy link
Author

@PVince81 the Android app do not use Cookies yet, because of that this problem was detected only on the iOS one.

@LukasReschke that works for me 👍

@rullzer
Copy link
Contributor

rullzer commented Mar 7, 2016

On a non 9.0 related note... we should move the thumbnail endpoint to webdav as well...

@PVince81
Copy link
Contributor

PVince81 commented Mar 7, 2016

@rullzer do we have a ticket for that ?

@rullzer
Copy link
Contributor

rullzer commented Mar 7, 2016

@PVince81 I remember dicussing it somewhere... but can't find a ticket. @DeepDiver1975 do you know?

@LukasReschke LukasReschke self-assigned this Mar 7, 2016
@LukasReschke
Copy link
Member

I'll add some workarounds for core. But the app should in future make sure only to use cookies from the WebDAV endpoint.

@LukasReschke
Copy link
Member

Workaround in core at #22901.

@javiergonzper Please test as soon as possible. Thanks.

@javiergonzper
Copy link
Author

@LukasReschke so we should not store the cookies of the thumbnails, Share API... ? I will add the conditions to ignore it now

@LukasReschke
Copy link
Member

@javiergonzper I'd keep it as-is for now in the app and just try with the change. That might be less error prone as the change is kinda non-intrusive. Then we can think in the future about streamlining this more.

@javiergonzper
Copy link
Author

@LukasReschke tested and working! 👍
Thank you! 🍻

PS: anyway we should not store the cookies of the APIs? (share, thumbnails...)

@LukasReschke
Copy link
Member

PS: anyway we should not store the cookies of the APIs? (share, thumbnails...)

I'd recommend to only store a cookie if it comes from remote.php and use that then for all other requests as well. :)

@javiergonzper
Copy link
Author

Ok, I will try to add that condition to the store of the cookies 😉

Thank you!

@rperezb
Copy link

rperezb commented Mar 7, 2016

@LukasReschke @PVince81 this feature, remote thumbnails, is under development on the iOS app, not released yet https://github.com/owncloud/ios/tree/thumbnails_fot_not_downloaded_images
On Android, this does not affect because the app doesn't use cookies

@ghost ghost modified the milestones: 9.0.1-current-maintenance, 9.0 Mar 9, 2016
LukasReschke added a commit that referenced this issue Mar 14, 2016
@lock
Copy link

lock bot commented Aug 5, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants