yakyak / hangupsjs

google hangouts client library for nodejs
226 stars 46 forks source link

fix: Add withCredentials option to requests that need to send cookies #87

Closed Rudloff closed 4 years ago

Rudloff commented 6 years ago

This option is needed when using hangupsjs in a browser environment (with Browserify) because by default the browser does not send the Cookie header (see #85).

This option is totally ignored by Nodejs.

(This won't make hangupsjs work in a standard browser but it allows it to work in a browser-like context without CORS like a Cordova app or an Electron renderer process.)

averissimo commented 6 years ago

I know it was me that proposed this, but what do you think of using it as an option from client?

As in: client = new Client({withCredentials: 'include'})

diff --git a/src/auth.coffee b/src/auth.coffee
index ce01916..fdafb2e 100644
--- a/src/auth.coffee
+++ b/src/auth.coffee
@@ -188,7 +188,7 @@ module.exports = class Auth
                 jar: request.jar jarstore
                 proxy: opts.proxy
                 headers: headers
-                withCredentials: 'include'
+                withCredentials: opts.withCredentials
         .then (res) ->
             return Q.reject NetworkError.forRes(res) unless res.statusCode == 200
             log.debug 'got uberauth'
@@ -201,7 +201,7 @@ module.exports = class Auth
                 uri: MERGE_SESSION
                 jar: request.jar jarstore
                 proxy: opts.proxy
-                withCredentials: 'include'
+                withCredentials: opts.withCredentials
         .then (res) ->
             return Q.reject NetworkError.forRes(res) unless res.statusCode == 200
             log.debug 'request merge session 2/2'
@@ -211,7 +211,7 @@ module.exports = class Auth
                 jar: request.jar jarstore
                 proxy: opts.proxy
                 headers: headers
-                withCredentials: 'include'
+                withCredentials: opts.withCredentials
         .then (res) ->
             return Q.reject NetworkError.forRes(res) unless res.statusCode == 200
             log.debug 'got session cookies'
diff --git a/src/channel.coffee b/src/channel.coffee
index d27051e..8d09531 100644
--- a/src/channel.coffee
+++ b/src/channel.coffee
@@ -66,8 +66,10 @@ MAX_RETRIES = 5

 module.exports = class Channel

-    constructor: (@jarstore, @proxy) ->
+    constructor: (@jarstore, @proxy, @opts) ->
         @pushParser = new PushDataParser()
+        if @opts == undefined
+          @opts = {}

     fetchPvt: =>
         log.debug 'fetching pvt'
@@ -75,7 +77,7 @@ module.exports = class Channel
             method: 'GET'
             uri: "#{ORIGIN_URL}/talkgadget/_/extension-start"
             jar: request.jar @jarstore
-            withCredentials: 'include'
+            withCredentials: @opts.withCredentials
         req(opts).then (res) =>
             data = JSON.parse res.body
             log.debug 'found pvt token', data[1]
@@ -107,7 +109,7 @@ module.exports = class Channel
                     count: 0
                 headers: auth
                 encoding: null # get body as buffer
-                withCredentials: 'include'
+                withCredentials: @opts.withCredentials
             req(opts).then (res) ->
                 # Example format (after parsing JS):
                 # [   [0,["c","SID_HERE","",8]],
diff --git a/src/client.coffee b/src/client.coffee
index 4187a95..9525f03 100644
--- a/src/client.coffee
+++ b/src/client.coffee
@@ -130,7 +130,7 @@ module.exports = class Client extends EventEmitter
         touch @opts.cookiespath unless @opts.jarstore
         @jarstore = @opts.jarstore ? new FileCookieStore(@opts.cookiespath)
         @jar = new CookieJar @jarstore
-        @channel = new Channel @jarstore, @opts.proxy
+        @channel = new Channel @jarstore, @opts.proxy, @opts
         @init = new Init @opts.proxy
         @chatreq = new ChatReq @jarstore, @init, @channel, @opts.proxy
Rudloff commented 6 years ago

I'm not sure there is a need for that since the other possible values wouldn't be useful. The available values are:

averissimo commented 6 years ago

Can you point to the documentation on those options? I could not find it as I was probably looking at the wrong place.

Rudloff commented 6 years ago

Turns out I mixed up this option with the credentials option from the fetch() Web API.

stream-http (which Browserify replaces the http Node library with) only accepts true or false: https://github.com/jhiesey/stream-http#extra-features-compared-to-node So I will update my PR to use withCredentials: true.

Rudloff commented 6 years ago

Any news on this? Please tell if you want me to change something else.