search

galaxyproject / galaxy

Data intensive science for everyone.
https://galaxyproject.org
Other
1.42k stars 1.01k forks source link

Redirect from data sources sometimes loses session cookies #11374

Closed mvdbeek closed 2 years ago

mvdbeek commented 3 years ago

I don't understand what is going on there, but if you go to e.g. Get Data -> UCSC main table browser and you select send to Galaxy, sometimes the request is executed with sessioncookie, and sometimes without.

If the sessioncookie is not set users are logged out, which is especially confusing considering it's not always super clear that users aren't logged in.

mvdbeek commented 3 years ago

ok, the cookie is filtered out because it doesn't set SameSite=None (and secure=true in my case).

Screenshot 2021-02-15 at 12 29 19

I suppose we could add a special cookie just for the tool_runner path that is SameSite=None

mvdbeek commented 3 years ago

Weird though that this is inconsistent and works on every 2nd request

dannon commented 3 years ago

@mvdbeek This is one AlexO saw, too, but I couldn't reproduce on my end; I thought it was maybe browser addons or something at the time?

mvdbeek commented 3 years ago

The explanation given by Chrome makes sense to me, the request seems to be a cross site request (redirect), and therefore the cookie gets filtered out. I haven't managed to reproduce on firefox. I am a little surprised how this could actually work, assuming all browsers properly implement SameSite

mvdbeek commented 3 years ago

But of course now everything is working for me again ...

mvdbeek commented 3 years ago

Ah, no there it is again, just confused myself, thought I was still logged in ... 😆

jdavcs commented 3 years ago

To confirm, this happens to me. I can't identify a pattern; first it happened consistently (not every 2nd request), now it happened once and stopped. Now it happens again. It may be connected to requesting different data, but I only have anecdotal evidence - nothing systematic. Discovered in the course of 21.01 release testing.

jdavcs commented 3 years ago

Same happens on usegalaxy.eu

jennaj commented 3 years ago

Happened again at usegalaxy.org

jennaj commented 3 years ago

From testing yesterday, could reproduce still with Chrome, but not Firefox or Safari on Mac OSX. Maybe that helps to find out what is going wrong.

@hexylena Should add this to the GTN for next week's training? The 101 uses this function and that is a pre-requisite for many (most?) other tutorials. Not sure about the best placement. Over time it happens, then it doesn't, then happens again. Might be a UCSC issue.

mvdbeek commented 3 years ago

This is totally our issue, this shouldn't be working on any modern browser AFAICT, but I likely won't have bandwidth to look into this before GCC.

hexylena commented 3 years ago

@jennaj if it was a month ago, I would've said "let's cut out the UCSC portion of the tutorials and use fixed copies of the data because that's better for reproducibility anyway" but since it's so close, I think the only option is fixing it, or warning people a lot in those tutorials :( it's unfortunate, I wish there were better options but we're stuck for now

dannon commented 3 years ago

It's chrome only though, right? The actual 'bug' is just that chrome's actually enforcing this on a redirect now? (so, is recommending using anything but chrome a better option than attempting to scramble to patch main, even if we do have time for a fix?)

mvdbeek commented 3 years ago

A proper fix would be a scoped token, right ? I don't think we should accept cookies that authenticate users from redirects ?

jennaj commented 3 years ago

Just dropping current status in here: Reproducible as of now re session lost when using chrome to get data from UCSC TB

Oh, slightly different behavior .. weird

  1. first query: logged out, but the output is sent back to a new unnamed history in the original account
  2. second query: log in under a new tab, that new unnamed history is the latest. repeat the query and am not logged out, and the output is added back to that same history

https://usegalaxy.org/u/jen-galaxyproject/h/unnamed-history

Maybe are more clues about how/what to fix. Certainly will help with end-user Qs if nothing else

screenshots

Screen Shot 2021-08-24 at 12 18 37 PM Screen Shot 2021-08-24 at 12 18 45 PM
hexylena commented 3 years ago

Reproducible as of November, we're seeing it in a large course with @shiltemann and the proteomics community.

Takadonet commented 2 years ago

Potential solution is proposed by @ericenns at https://github.com/galaxyproject/galaxy/issues/11066

That is the solution we are using in-house at the moment for our Galaxy instances when using IRIDA as a data source. A permanent solution needs to be implemented.

hexylena commented 2 years ago

@natefoo @gmauro @Slugger70 what do y'all reckon is the chance to get the workaround described by https://github.com/galaxyproject/galaxy/issues/11066 live for Smörgåsbord 2? Otherwise I think we should remove tutorials which use UCSC for the introductory day, fine either way, just want to make an administrative decision

mvdbeek commented 2 years ago

Should be fixed in https://github.com/galaxyproject/galaxy/pull/14038. Once we've got that on test or main and we confirm it works as expected (what with it sometimes working and sometimes not) we can backport this as far as necessary.