balderdashy / sails.io.js

Browser SDK for communicating w/ Sails via sockets
https://sailsjs.com/documentation/reference/web-sockets/socket-client
183 stars 118 forks source link

Allow sending headers with socket-based requests. #23

Closed jfrumar closed 8 years ago

jfrumar commented 10 years ago

We use header-based authentication to pass our policies. With the update from Sails 0.10.1 to 0.10.4, the client library changed and our policies were denying socket-based requests. This PR restores the ability to set headers for websocket requests.

Relevant issue: https://github.com/balderdashy/sails/issues/2041

JetFault commented 10 years ago

+1 for this

EduceHealth commented 10 years ago

+1

mikermcneil commented 9 years ago

@jfrumar @JetFault @Grokling this is a great point- however I'm concerned about having the headers object be part of the data (look at what happened w/ consolidate / ejs /etc. with the locals and options)

Could we expand the usage to support an options object as the first argument?

i.e.

io.socket.get({
  url: '/kitten/party',
  headers: {},
  params: {}
}, function (data, JWR){
  // ...  
});

So if the first arg is an object, not a string, it would get interpreted as defined above.

JetFault commented 9 years ago

@mikermcneil Yeah I think thats the better option

npfitz commented 9 years ago

So... Is this a thing that's happening? We rely on http headers for our authentication as well, and being able to set them would be great (even better would be the ability to set them globally for all requests)

dzcpy commented 9 years ago

+1

npfitz commented 9 years ago

So, for anyone who is still looking for this, it turns out you are actually able to send headers via sockets using the request function. We built and angular service around this and it was able to fulfil our needs

theblang commented 9 years ago

@npfitz What version of Sails are you using and what do you mean by the request function?

npfitz commented 9 years ago

I'm using version 0.10.5 I believe, and the request function is part of the io.socket object on the client.

Documentation can be found here: https://github.com/balderdashy/sails-docs/blob/master/reference/websockets/sails.io.js/socket.request.md

On Wed, Jan 21, 2015, 1:21 AM Matt Langston notifications@github.com wrote:

@npfitz https://github.com/npfitz What version of Sails are you using and what do you mean by the request function?

— Reply to this email directly or view it on GitHub https://github.com/balderdashy/sails.io.js/pull/23#issuecomment-70786218 .

theblang commented 9 years ago

@npfitz I'm a little confused, because request isn't in sails.io version 0.10.3, but later versions are for socket.io version 1.x, which wouldn't be compatible with Sails version 0.10.5 would it?

npfitz commented 9 years ago

To be totally honest, I'm not really sure. I do know that I'm using 0.10.5 right now, and that I am able to use the request function to send my stuff with headers.

Would some example code be helpful?

On Tue, Jan 27, 2015, 9:48 PM Matt Langston notifications@github.com wrote:

@npfitz https://github.com/npfitz I'm a little confused, because request isn't in sails.io version 0.10.3, but later versions are for socket.io version 1.x, which wouldn't be compatible with Sails version 0.10.5 would it?

— Reply to this email directly or view it on GitHub https://github.com/balderdashy/sails.io.js/pull/23#issuecomment-71766909 .

robwormald commented 9 years ago

one idea: the new hotness in http-land is 'fetch' : https://fetch.spec.whatwg.org/

we're using github's implementation: https://github.com/github/fetch


//GET by default
fetch('/foos').then(onSuccess,onError);

//crud
fetch('/foos', { method: 'post', data: newFoo, headers: { foo: 'bar' } })

its promise-based, but could conceivably be extended to add an optional callback as the 3rd param. i mention it because it is a really nice API to standardize against (rather than JWR). Happy to roll it if there's any interest.

so socket.fetch becomes the low level API, and socket.get/put/etc just wrap it.

theblang commented 9 years ago

@npfitz Some example code would be amazing! I have actually updated to 0.11-rc9 and the newest version of sails.io, but I still can't get headers to go through.

KirillSuhodolov commented 9 years ago

Any news, guys?

mikermcneil commented 8 years ago

@KirillSuhodolov @mattblang @robwormald @npfitz @andyhu @jfrumar @JetFault @EduceHealth Hey everyone-- this is implemented in Sails v0.12 as follows: