-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Remove session from companion #4394
Comments
@ifedapoolarewaju could I ask your opinion here, please? |
IIRC, cookies were primarily used for displaying/loading authenticated images on Uppy Client. So the use of Cookies are implicit, and maybe better tested if you check that (Google Drive, Dropbox, Instagram, One Drive, etc.) images load correctly despite disabling cookies. I can't remember specifically which of the Providers needed it, but my first guess is Dropbox. PS: So as not to cause a delay, I responded right away based off my fragile vague memory, and haven't done any in-depth look. If this doesn't suffice, I can take a closer look to properly jug my memory. |
@ifedapoolarewaju thanks for the swift response! 🖤 |
I believe the set-cookie logic for thumbnails is here:
it's actually a separate cookie, e.g. |
Initial checklist
Link to runnable example
No response
Steps to reproduce
/list
calls do not contain any cookie. also observe that the responses haveset-cookie
with a different session cookie for each requestExpected behavior
companion should not work because cookies and sessions are not in use
Actual behavior
everything actually works.
this means we can probably remove all session code in companion, as it will simplify companion code and we don't have to worry about things like CORS cookies,
secure
,same-site
etc.The text was updated successfully, but these errors were encountered: