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

Option to set xhr.withCredentials #103

Closed
aakoshh opened this issue Nov 22, 2017 · 9 comments
Closed

Option to set xhr.withCredentials #103

aakoshh opened this issue Nov 22, 2017 · 9 comments
Labels

Comments

@aakoshh
Copy link

aakoshh commented Nov 22, 2017

I'm facing a scenario where I have to talk to a gRPC server that sits behind a reverse proxy which redirects to ADFS for login and sets a http-only session cookie which it will pick up on subsequent requests and attaches an Authorization header. The problem is that the transports don't set the cookies with the gRPC invocation, so each POST results in a redirect.

What do you think about adding an option somewhere which could set the withCredentials property of the XMLHttpRequest before sending the request?

@aakoshh aakoshh changed the title Option to set xhr.withCredentials Option to set xhr.withCredentials Nov 22, 2017
@fabxc
Copy link

fabxc commented Feb 5, 2018

Looks like the CORS headers set for OPTIONS requests specifically set Access-Control-Allow-Credentials: true but the JS/TS implementation hard-codes the opposite (e.g. here and not allowing to set withCredentials = true in the XMLHTTPRequest.

This seems like an unnecessary restriction that forces annoying constraints on the rest of the setup. Providing a fully custom transport for this seems like a lot of overhead.

@jonnyreeves
Copy link
Contributor

So while we can solve this problem for fetch and xhr based transports, there is nothing that can be done for including cookies on cross-origin WebSocket connections as far as I know.

One workaround would be to make the WebSocketTransport throw an error if you try to construct it in such a way that you request credentials (cookies) be send cross origin (credentials: 'include').

@MOZGIII
Copy link

MOZGIII commented Oct 18, 2018

Why not let the browser deal with that?
I was looking for a way to pass credentials: 'include' to the fetch transport - and found this issue. Our usecase is actually sending the corss-origin request with cerdentials - it's allowd by fetch spec.

@MOZGIII
Copy link

MOZGIII commented Oct 19, 2018

We ended up copying fetch transport from tree to our source code and patching it so it passes include.
In general it would be great to have an ability to configure various transport-specific parameters if we opt out of transport autodetection by pass transport manually. Seems like a good idea to me - it doesn't require a generic interface deduction of any kind, and just provides a flexible way of using that's already in there.

I can compose a sample PR if you like this idea.

@johanbrandhorst
Copy link
Contributor

Sounds good to me, let's get a PR up for discussion.

@MOZGIII
Copy link

MOZGIII commented Oct 19, 2018

@johanbrandhorst please take a look at #260

@MOZGIII
Copy link

MOZGIII commented Oct 19, 2018

And #261.

@jonny-improbable
Copy link
Contributor

This has been resolved for some time now, you can now configure the CrossBrowserTransport.

@maxmcd
Copy link

maxmcd commented Jun 12, 2020

Linking directly for others:

export function CrossBrowserHttpTransport(init: CrossBrowserHttpTransportInit): TransportFactory {
if (detectFetchSupport()) {
const fetchInit: FetchTransportInit = {
credentials: init.withCredentials ? "include" : "same-origin"
};
return FetchReadableStreamTransport(fetchInit);
}
return XhrTransport({ withCredentials: init.withCredentials });
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants