Fix re-subscription (#602)

When resubscribing after a reconnected event, the SDK use the original subscription request to re-subscribe.

The Kuzzle.query method mutate request and inject the JWT inside. Meaning that the request saved for the re-subscription already has a JWT inside.

This can be an issue because developers rely on the kuzzle.jwt property of the SDK to know if 1) the token is still valid 2) there are connection information.

Now the request and options parameter of the query method are cloned before modification.

Author: Aschen

doc/7/essentials/events/index.md

@@ -86,6 +86,7 @@ Triggered whenever Kuzzle responds with an error
 **Callback arguments:**
 
 `@param {KuzzleError} error - Error details`
+
 `@param {object} request - Request that caused the error`
 
 ## reconnected

src/Kuzzle.ts

@@ -420,7 +420,9 @@ export class Kuzzle extends KuzzleEventEmitter {
       this.startQueuing();
     }
 
-    this.protocol.addListener('queryError', (err, query) => this.emit('queryError', err, query));
+    this.protocol.addListener('queryError', ({ error, request }) => {
+      this.emit('queryError', { error, request });
+    });
 
     this.protocol.addListener('tokenExpired', () => this.tokenExpired());
 
@@ -456,20 +458,6 @@ export class Kuzzle extends KuzzleEventEmitter {
         this.playQueue();
       }
 
-      if (this.auth.authenticationToken) {
-        return this.auth.checkToken()
-          .then(res => {
-            // shouldn't obtain an error but let's invalidate the token anyway
-            if (!res.valid) {
-              this.auth.authenticationToken = null;
-            }
-          })
-          .catch(() => {
-            this.auth.authenticationToken = null;
-          })
-          .then(() => this.emit('reconnected'));
-      }
-
       this.emit('reconnected');
     });
 
@@ -523,18 +511,21 @@ export class Kuzzle extends KuzzleEventEmitter {
    *    - volatile (object, default: null):
    *        Additional information passed to notifications to other users
    *
-   * @param request
-   * @param options - Optional arguments
+   * @param req
+   * @param opts - Optional arguments
    */
-  query (request: RequestPayload = {}, options: JSONObject = {}): Promise<ResponsePayload> {
-    if (typeof request !== 'object' || Array.isArray(request)) {
-      throw new Error(`Kuzzle.query: Invalid request: ${JSON.stringify(request)}`);
+  query (req: RequestPayload = {}, opts: JSONObject = {}): Promise<ResponsePayload> {
+    if (typeof req !== 'object' || Array.isArray(req)) {
+      throw new Error(`Kuzzle.query: Invalid request: ${JSON.stringify(req)}`);
     }
 
-    if (typeof options !== 'object' || Array.isArray(options)) {
-      throw new Error(`Kuzzle.query: Invalid "options" argument: ${JSON.stringify(options)}`);
+    if (typeof opts !== 'object' || Array.isArray(opts)) {
+      throw new Error(`Kuzzle.query: Invalid "options" argument: ${JSON.stringify(opts)}`);
     }
 
+    const request = JSON.parse(JSON.stringify(req));
+    const options = JSON.parse(JSON.stringify(opts));
+
     if (!request.requestId) {
       request.requestId = uuidv4();
     }
@@ -590,7 +581,7 @@ export class Kuzzle extends KuzzleEventEmitter {
         });
       }
 
-      this.emit('discarded', {request});
+      this.emit('discarded', { request });
       return Promise.reject(new Error(`Unable to execute request: not connected to a Kuzzle server.
 Discarded request: ${JSON.stringify(request)}`));
     }

src/protocols/WebSocket.ts

@@ -150,7 +150,7 @@ export default class WebSocketProtocol extends BaseProtocolRealtime {
             (new Error().stack),
             this.constructor.name);
 
-          this.emit('queryError', error, data);
+          this.emit('queryError', { error, request: data });
         }
       };
 

src/protocols/abstract/Base.ts

@@ -146,7 +146,7 @@ Discarded request: ${JSON.stringify(request)}`));
           error = response.error;
         }
 
-        this.emit('queryError', error, request);
+        this.emit('queryError', { error, request });
 
         if (request.action !== 'logout' && error.id === 'security.token.invalid') {
           this.emit('tokenExpired');

test/kuzzle/connect.test.js

@@ -1,9 +1,7 @@
-const
-  should = require('should'),
-  sinon = require('sinon'),
-  ProtocolMock = require('../mocks/protocol.mock'),
-  generateJwt = require('../mocks/generateJwt.mock'),
-  { Kuzzle } = require('../../src/Kuzzle');
+const should = require('should');
+const sinon = require('sinon');
+const ProtocolMock = require('../mocks/protocol.mock');
+const { Kuzzle } = require('../../src/Kuzzle');
 
 describe('Kuzzle connect', () => {
 
@@ -96,42 +94,6 @@ describe('Kuzzle connect', () => {
         });
     });
 
-    it('should keep a valid JWT at reconnection', () => {
-      const
-        jwt = generateJwt(),
-        kuzzle = new Kuzzle(protocols.somewhereagain);
-
-      kuzzle.auth.checkToken = sinon.stub().resolves({
-        valid: true
-      });
-      kuzzle.jwt = jwt;
-
-      return kuzzle.connect()
-        .then(() => {
-          should(kuzzle.auth.checkToken).be.calledOnce();
-
-          should(kuzzle.jwt).be.eql(jwt);
-        });
-    });
-
-    it('should empty the JWT at reconnection if it has expired', () => {
-      const
-        jwt = generateJwt(),
-        kuzzle = new Kuzzle(protocols.somewhereagain);
-
-      kuzzle.auth.checkToken = sinon.stub().resolves({
-        valid: false
-      });
-      kuzzle.jwt = jwt;
-
-      return kuzzle.connect()
-        .then(() => {
-          should(kuzzle.auth.checkToken).be.calledOnce();
-
-          should(kuzzle.jwt).be.null();
-        });
-    });
-
     it('should register listeners upon receiving a "disconnect" event', () => {
       const
         kuzzle = new Kuzzle(protocols.somewhere),

test/kuzzle/protocol.test.js

@@ -1,9 +1,8 @@
-const
-  should = require('should'),
-  sinon = require('sinon'),
-  ProtocolMock = require('../mocks/protocol.mock'),
-  generateJwt = require('../mocks/generateJwt.mock'),
-  { Kuzzle } = require('../../src/Kuzzle');
+const should = require('should');
+const sinon = require('sinon');
+const ProtocolMock = require('../mocks/protocol.mock');
+const generateJwt = require('../mocks/generateJwt.mock');
+const { Kuzzle } = require('../../src/Kuzzle');
 
 describe('Kuzzle protocol methods', () => {
   let kuzzle;
@@ -25,17 +24,20 @@ describe('Kuzzle protocol methods', () => {
 
   describe('#events', () => {
     it('should propagate protocol "queryError" events', () => {
-      const
-        eventStub = sinon.stub(),
-        error = {message: 'foo-bar'},
-        query = {foo: 'bar'};
+      const eventStub = sinon.stub();
+      const error = { message: 'foo-bar' };
+      const request = { foo: 'bar' };
+
       kuzzle.connect();
       kuzzle.addListener('queryError', eventStub);
 
-      kuzzle.protocol.emit('queryError', error, query);
+      kuzzle.protocol.emit('queryError', { error, request });
 
       should(eventStub).be.calledOnce();
-      should(eventStub).be.calledWithMatch({message: 'foo-bar'}, {foo: 'bar'});
+      should(eventStub).be.calledWithMatch({
+        error: { message: 'foo-bar' },
+        request: { foo: 'bar'}
+      });
     });
 
     it('should propagate protocol "tokenExpired" events', () => {

test/kuzzle/query.test.js

@@ -191,7 +191,7 @@ describe('Kuzzle query management', () => {
       should(kuzzle.protocol.query).not.be.called();
       should(eventStub)
         .be.calledOnce()
-        .be.calledWith({request});
+        .be.calledWithMatch({ request });
 
       should(kuzzle._offlineQueue.length).be.eql(1);
     });
@@ -217,7 +217,7 @@ describe('Kuzzle query management', () => {
           should(kuzzle._offlineQueue.length).eql(0);
           should(eventStub)
             .be.calledOnce()
-            .be.calledWith({request});
+            .be.calledWithMatch({ request });
         });
     });
 
@@ -233,7 +233,14 @@ describe('Kuzzle query management', () => {
           should(kuzzle.protocol.query)
             .be.not.be.called();
         });
+    });
+
+    it('should clone the original request', async () => {
+      const request = { controller: 'server', action: 'now' };
+
+      await kuzzle.query(request);
 
+      should(request).be.eql({ controller: 'server', action: 'now' });
     });
   });
 });

test/protocol/WebSocket.test.js

@@ -313,9 +313,9 @@ describe('WebSocket networking module', () => {
 
     let expectedError;
     websocket.on('discarded', cb);
-    websocket.on('queryError', (error, data) => {
+    websocket.on('queryError', ({ error, request }) => {
       expectedError = error;
-      cb2(error, data);
+      cb2(error, request);
     });
     websocket.connect();