search

girder / girder_web_components

Reusable Javascript and VueJS components for interacting with a Girder server.
https://gwc.girder.org
Apache License 2.0
16 stars 9 forks source link

Bugfix/with credentials #227

Closed subdavis closed 4 years ago

subdavis commented 4 years ago

client-side support needed to use changes from https://github.com/girder/girder/issues/2905

Edit: wrong reference. https://github.com/girder/girder/pull/644

subdavis commented 4 years ago

This fix isn't perfect. It causes login to girder client as a side-effect, and if you log out of girder client, anything in your GWC app that relied on the cross-origin cookie will break until you log out and in again.

So this is really only half a fix.

zachmullen commented 4 years ago

It causes login to girder client as a side-effect

This seems like maybe a feature more than a bug, but either way it's OK behavior. Most users of Girder web apps aren't flipping back and forth between different client apps and logging in and out repeatedly.

subdavis commented 4 years ago

@zachmullen something I didn't consider. This will break GWC in all cases where Girder's CORS policy doesn't include an exact domain match.

For example, GWC will refuse to authenticate now for Access-Control-Allow-Origin = *.

That's definitely not what we want, so this should be optional and probably off by default.

zachmullen commented 4 years ago

Good catch. Yes, let's make it optional. It probably makes sense on either the method or the class, but for practicality I'd vote to put the option on the class since that will make it so that downstream apps can configure it in their main module where the REST client is usually instantiated, which seems like where process.env references ought to be concentrated, rather than scattered throughout the code.