Add stop() method to AuthenticationProvider to enable shutting down t…

…imers when disconnecting/exiting

device/core/devdoc/sak_authentication_provider_requirements.md

@@ -46,6 +46,12 @@ sakAuthProvider.on('newTokenAvailable', function (credentials) {
 
 **SRS_NODE_SAK_AUTH_PROVIDER_16_008: [** The `fromConnectionString` method shall extract the credentials from the `connectionString` argument and create a new `SharedAccessKeyAuthenticationProvider` that uses these credentials to generate security tokens. **]**
 
+## stop(): void
+
+**SRS_NODE_SAK_AUTH_PROVIDER_16_012: [** The `stop` method shall clear the token renewal timer if it is running. **]**
+
+**SRS_NODE_SAK_AUTH_PROVIDER_16_013: [** The `stop` method shall simply return if the token renewal timer is not running. **]**
+
 # Generated Security Token
 
 **SRS_NODE_SAK_AUTH_PROVIDER_16_009: [** Every token shall be created with a validity period of `tokenValidTimeInSeconds` if specified when the constructor was called, or 1 hour by default. **]**

device/core/devdoc/sas_authentication_provider_requirements.md

@@ -41,4 +41,8 @@ sasAuthProvider.on('newTokenAvailable', function (credentials) {
 
 **SRS_NODE_SAS_AUTHENTICATION_PROVIDER_16_005: [** The `fromSharedAccessSignature` method shall throw a `ReferenceError` if the `sharedAccessSignature` argument is falsy. **]**
 
-**SRS_NODE_SAS_AUTHENTICATION_PROVIDER_16_006: [** The `fromSharedAccessSignature` shall return a new `SharedAccessSignatureAuthenticationProvider` object initialized with the credentials parsed from the `sharedAccessSignature` argument. **]**
+**SRS_NODE_SAS_AUTHENTICATION_PROVIDER_16_006: [** The `fromSharedAccessSignature` shall return a new `SharedAccessSignatureAuthenticationProvider` object initialized with the credentials parsed from the `sharedAccessSignature` argument. **]**
+
+### stop(): void
+
+**SRS_NODE_SAS_AUTHENTICATION_PROVIDER_16_007: [** The `stop` method shall simply return since there is no timeout or resources to clear. **]**

device/core/package.json

@@ -30,10 +30,10 @@
   "scripts": {
     "lint": "tslint --project . -c ../../tslint.json",
     "build": "tsc",
-    "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot \"test/**/_*_test.js\"",
-    "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot \"test/**/_*_test*.js\"",
-    "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec \"test/**/_*_test.js\"",
-    "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec \"test/**/_*_test*.js\"",
+    "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot \"test/**/_*_test.js\"",
+    "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot \"test/**/_*_test*.js\"",
+    "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec \"test/**/_*_test.js\"",
+    "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec \"test/**/_*_test*.js\"",
     "ci": "npm -s run lint && npm -s run build && npm -s run alltest-min && npm -s run check-cover",
     "test": "npm -s run lint && npm -s run build && npm -s run unittest",
     "check-cover": "istanbul check-coverage --statements 95 --branches 87 --lines 97 --functions 92"

device/core/src/sak_authentication_provider.ts

@@ -66,6 +66,18 @@ export class SharedAccessKeyAuthenticationProvider extends EventEmitter implemen
     }
   }
 
+  /**
+   * Stops the timer used to renew to SAS token.
+   */
+  stop(): void {
+    /*Codes_SRS_NODE_SAK_AUTH_PROVIDER_16_012: [The `stop` method shall clear the token renewal timer if it is running.]*/
+    /*Codes_SRS_NODE_SAK_AUTH_PROVIDER_16_013: [The `stop` method shall simply return if the token renewal timer is not running.]*/
+    if (this._renewalTimeout) {
+      clearTimeout(this._renewalTimeout);
+      this._renewalTimeout = null;
+    }
+  }
+
   protected _sign(resourceUri: string, expiry: number, callback: (err: Error, signature?: string) => void): void {
     callback(null, SharedAccessSignature.create(resourceUri, this._credentials.sharedAccessKeyName, this._credentials.sharedAccessKey, expiry).toString());
   }

device/core/src/sas_authentication_provider.ts

@@ -40,6 +40,14 @@ export class SharedAccessSignatureAuthenticationProvider extends EventEmitter im
     callback(null, this._credentials);
   }
 
+  /**
+   * does nothing and returns - this is part of the token-based authentication provider API but there are no resources to stop/free here.
+   */
+  stop(): void {
+    /*Codes_SRS_NODE_SAS_AUTHENTICATION_PROVIDER_16_007: [The `stop` method shall simply return since there is no timeout or resources to clear.]*/
+    return;
+  }
+
   /**
    * Updates the shared access signature token that transports should use to authenticate. When called, the `SharedAccessSignatureAuthenticationProvider` will emit
    * a `newTokenAvailable` event that the transports can then use to authenticate with the Azure IoT hub instance.

device/core/test/_client_common_testrun.js

@@ -37,7 +37,7 @@ function badConfigTests(opName, Client, Transport, requestFn) {
     client.open(function(err) {
       if (err) {
         test(err);
-        done();
+        client.close(done);
       } else {
         requestFn(client, function (err, res) {
           test(err, res);
@@ -208,10 +208,12 @@ function batchMessageTests(Client, Transport, registry, testName, requestFn) {
 
         requestFn(client, function (err, res) {
           if (err) {
-            done(err);
+            client.close(function () {
+              done(err);
+            });
           } else {
             assert.equal(res.constructor.name, 'MessageEnqueued');
-            done();
+            client.close(done);
           }
         });
       });

device/core/test/_sak_authentication_provider_test.js

@@ -36,6 +36,7 @@ describe('SharedAccessKeyAuthenticationProvider', function () {
         assert.notOk(sasObject.skn);
         assert.isOk(sasObject.sig);
         assert.isTrue(sasObject.se > Date.now() / 1000);
+        sakAuthProvider.stop();
         testCallback();
       });
     });
@@ -67,6 +68,7 @@ describe('SharedAccessKeyAuthenticationProvider', function () {
             assert.isTrue(eventSpy.calledOnce);
             sakAuthProvider.automaticRenewal = false;
             testClock.restore();
+            sakAuthProvider.stop();
             testCallback();
           });
         });
@@ -90,6 +92,7 @@ describe('SharedAccessKeyAuthenticationProvider', function () {
       sakAuthProvider.getDeviceCredentials(function (err, creds) {
         assert.equal(creds.sharedAccessSignature, 'SharedAccessSignature sr=fake.host.name%2Fdevices%2FfakeDeviceId&sig=bYz5R2IFTaejB6pgYOxns2mw6lcuA4VSy8kJbYQp0Sc%3D&se=10');
         testClock.restore();
+        sakAuthProvider.stop();
         testCallback();
       });
     });
@@ -105,7 +108,8 @@ describe('SharedAccessKeyAuthenticationProvider', function () {
       var eventSpy = sinon.spy();
       sakAuthProvider.on('error', function (err) {
         assert.strictEqual(err, fakeError);
-        testCallback();
+          sakAuthProvider.stop();
+          testCallback();
       });
 
       sinon.stub(sakAuthProvider, '_sign').callsArgWith(2, fakeError);
@@ -123,7 +127,8 @@ describe('SharedAccessKeyAuthenticationProvider', function () {
       sinon.stub(sakAuthProvider, '_sign').callsArgWith(2, 'whoops');
       sakAuthProvider.getDeviceCredentials(function(err) {
         assert.equal(err, 'whoops');
-        testCallback();
+          sakAuthProvider.stop();
+          testCallback();
       });
     });
 
@@ -139,6 +144,7 @@ describe('SharedAccessKeyAuthenticationProvider', function () {
       sakAuthProvider._renewToken(function(err, creds) {
         assert.equal(creds.sharedAccessSignature, 'signature');
       });
+      sakAuthProvider.stop();
       testCallback();
     });
 
@@ -201,9 +207,50 @@ describe('SharedAccessKeyAuthenticationProvider', function () {
           assert.strictEqual(creds.moduleId, testConfig.credentials.moduleId);
           assert.strictEqual(creds.host, testConfig.credentials.host);
           assert.strictEqual(creds.sharedAccessKey, testConfig.credentials.sharedAccessKey);
+          sakAuthProvider.stop();
           testCallback();
         });
       });
     });
   });
+
+  describe('stop', function () {
+
+    /*Tests_SRS_NODE_SAK_AUTH_PROVIDER_16_012: [The `stop` method shall clear the token renewal timer if it is running.]*/
+    it('clears the SAS token renewal timeout', function (testCallback) {
+      this.clock = sinon.useFakeTimers();
+      var testClock = this.clock;
+      var fakeCredentials = {
+        deviceId: 'fakeDeviceId',
+        host: 'fake.host.name',
+        sharedAccessKey: 'fakeKey'
+      };
+      var token;
+      var sakAuthProvider = new SharedAccessKeyAuthenticationProvider(fakeCredentials, 10, 1);
+      var eventSpy = sinon.spy();
+      sakAuthProvider.on('newTokenAvailable', eventSpy);
+      /*Tests_SRS_NODE_SAK_AUTH_PROVIDER_16_003: [The `getDeviceCredentials` should call its callback with a `null` first parameter and a `TransportConfig` object as a second parameter, containing the latest valid token it generated.]*/
+      sakAuthProvider.getDeviceCredentials(function () {
+        testClock.tick(11000); // 11 seconds - event should've fired
+        assert.isTrue(eventSpy.calledOnce);
+        sakAuthProvider.stop();
+        testClock.tick(11000);
+        assert.isTrue(eventSpy.calledOnce); // if the timer is still running, the event would've fired twice.
+        testClock.restore();
+        testCallback();
+      });
+    });
+
+    /*Tests_SRS_NODE_SAK_AUTH_PROVIDER_16_013: [The `stop` method shall simply return if the token renewal timer is not running.]*/
+    it('returns and does not crash if the timer is not running', function () {
+      var sakAuthProvider = new SharedAccessKeyAuthenticationProvider({
+        deviceId: 'fakeDeviceId',
+        host: 'fake.host.name',
+        sharedAccessKey: 'fakeKey'
+      }, 10, 1);
+      assert.doesNotThrow(function () {
+        sakAuthProvider.stop();
+      });
+    });
+  });
 });

device/core/test/_sas_authentication_provider_test.js

@@ -4,8 +4,6 @@
 'use strict';
 
 var assert = require('chai').assert;
-var sinon = require('sinon');
-var errors = require('azure-iot-common').errors;
 var SharedAccessSignature = require('azure-iot-common').SharedAccessSignature;
 var SharedAccessSignatureAuthenticationProvider = require('../lib/sas_authentication_provider').SharedAccessSignatureAuthenticationProvider;
 
@@ -117,4 +115,18 @@ describe('SharedAccessSignatureAuthenticationProvider', function () {
       });
     });
   });
+
+  describe('#stop', function () {
+    /*Tests_SRS_NODE_SAS_AUTHENTICATION_PROVIDER_16_007: [The `stop` method shall simply return since there is no timeout or resources to clear.]*/
+    it('returns and does not crash if the timer is not running', function () {
+      var sasAuthProvider = new SharedAccessSignatureAuthenticationProvider({
+        host: 'host.name',
+        deviceId: 'deviceId',
+        sharedAccessSignature: 'sas'
+      }, 10, 1);
+      assert.doesNotThrow(function () {
+        sasAuthProvider.stop();
+      });
+    });
+  })
 });

device/core/test/http_simulated.js

@@ -32,17 +32,21 @@ function SimulatedHttp(authProvider) {
         var sig = SharedAccessSignature.parse(config.sharedAccessSignature);
 
         if (config.host.indexOf('bad') >= 0) {                      // bad host
+          authProvider.stop();
           done(new Error('getaddrinfo ENOTFOUND bad'));
         }
         else if (config.deviceId.indexOf('bad') >= 0) {             // bad policy
+          authProvider.stop();
           done(makeError(404));
         }
         else {
           var cmpSig = (SharedAccessSignature.create(config.host, config.deviceId, 'bad', sig.se)).toString();
           if (config.sharedAccessSignature === cmpSig) {  // bad key
+            authProvider.stop();
             done(makeError(401));
           }
           else {
+            authProvider.stop();
             done(null, new results.MessageEnqueued(new Response(204)));
           }
         }

device/transport/amqp/devdoc/device_amqp_requirements.md

@@ -93,6 +93,8 @@ The `disconnect` method terminates the connection with the Azure IoT Hub instanc
 
 **SRS_NODE_DEVICE_AMQP_16_023: [** The `disconnect` method shall forcefully detach all attached links if a connection error is the causing the transport to be disconnected. **]**
 
+**SRS_NODE_DEVICE_AMQP_16_083: [** When the `amqp` client is disconnected and if token-based authentication is used the `stop` method of the `AuthenticationProvider` shall be called. **]**
+
 ### sendEvent(message, done)
 
 The `sendEvent` method sends an event to the IoT Hub as the device indicated in the constructor argument.

device/transport/amqp/package.json

@@ -30,10 +30,10 @@
   "scripts": {
     "lint": "tslint --project . -c ../../../tslint.json",
     "build": "tsc",
-    "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot test/_*_test.js",
-    "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot test/_*_test*.js",
-    "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec test/_*_test.js",
-    "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec test/_*_test*.js",
+    "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot test/_*_test.js",
+    "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot test/_*_test*.js",
+    "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec test/_*_test.js",
+    "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec test/_*_test*.js",
     "ci": "npm -s run lint && npm -s run build && npm -s run alltest-min && npm -s run check-cover",
     "test": "npm -s run lint && npm -s run build && npm -s run unittest",
     "check-cover": "istanbul check-coverage --statements 93 --branches 84  --lines 95 --functions 93"

device/transport/amqp/src/amqp.ts

@@ -8,7 +8,7 @@ import * as dbg from 'debug';
 const debug = dbg('azure-iot-device-amqp:Amqp');
 import { EventEmitter } from 'events';
 
-import { DeviceTransport, MethodMessage, DeviceMethodResponse, TwinProperties, DeviceClientOptions } from 'azure-iot-device';
+import { DeviceTransport, MethodMessage, DeviceMethodResponse, TwinProperties, DeviceClientOptions, SharedAccessKeyAuthenticationProvider } from 'azure-iot-device';
 import { getUserAgentString } from 'azure-iot-device';
 import { Amqp as BaseAmqpClient, AmqpBaseTransportConfig, translateError, AmqpMessage, SenderLink, ReceiverLink } from 'azure-iot-amqp-base';
 import { endpoint, SharedAccessSignature, errors, results, Message, AuthenticationProvider, AuthenticationType, TransportConfig } from 'azure-iot-common';
@@ -147,6 +147,11 @@ export class Amqp extends EventEmitter implements DeviceTransport {
       states: {
         disconnected: {
           _onEnter: (err, callback) => {
+            /*Codes_SRS_NODE_DEVICE_AMQP_16_083: [When the `amqp` client is disconnected and if token-based authentication is used the `stop` method of the `AuthenticationProvider` shall be called.]*/
+            if (this._authenticationProvider.type === AuthenticationType.Token) {
+              (this._authenticationProvider as SharedAccessKeyAuthenticationProvider).stop();
+            }
+
             if (callback) {
               if (err) {
                 callback(err);

device/transport/amqp/test/_amqp_test.js

@@ -66,6 +66,7 @@ describe('Amqp', function () {
     fakeTokenAuthenticationProvider.type = AuthenticationType.Token;
     fakeTokenAuthenticationProvider.getDeviceCredentials = sinon.stub().callsArgWith(0, null, configWithSAS);
     fakeTokenAuthenticationProvider.updateSharedAccessSignature = sinon.stub();
+    fakeTokenAuthenticationProvider.stop = sinon.stub();
     sinon.spy(fakeTokenAuthenticationProvider, 'on');
 
     fakeX509AuthenticationProvider = {
@@ -703,6 +704,15 @@ describe('Amqp', function () {
           });
         });
       });
+
+      /*Tests_SRS_NODE_DEVICE_AMQP_16_083: [When the `amqp` client is disconnected and if token-based authentication is used the `stop` method of the `AuthenticationProvider` shall be called.]*/
+      it('calls stop on the authentication provider if using token authentication', function (testCallback) {
+        transport.connect(function () {});
+        transport.disconnect(function () {
+          assert.isTrue(fakeTokenAuthenticationProvider.stop.calledTwice); // once when instantiated, once when disconnected
+          testCallback();
+        });
+      });
     });
 
     describe('#updateSharedAccessSignature', function () {

device/transport/http/devdoc/http_requirements.md

@@ -62,6 +62,8 @@ or:
 
 **SRS_NODE_DEVICE_HTTP_16_031: [** The `disconnect` method shall call its callback with a `null` first argument and a `results.Disconnected` second argument after successfully disabling the C2D receiver (if necessary). **]**
 
+**SRS_NODE_DEVICE_HTTP_16_039: [** The `disconnect` method shall call the `stop` method on the `AuthenticationProvider` object if the type of authentication used is "token".  **]**
+
 ### sendEvent(message, done)
 
 The `sendEvent` method sends an event to an IoT hub on behalf of the device indicated in the constructor argument.

device/transport/http/package.json

@@ -26,10 +26,10 @@
   "scripts": {
     "lint": "tslint --project . -c ../../../tslint.json",
     "build": "tsc",
-    "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot test/_*_test.js",
-    "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --exit --reporter dot test/_*_test*.js",
-    "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec test/_*_test.js",
-    "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --exit --reporter spec test/_*_test*.js",
+    "unittest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot test/_*_test.js",
+    "alltest-min": "istanbul cover --report none node_modules/mocha/bin/_mocha -- --reporter dot test/_*_test*.js",
+    "unittest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec test/_*_test.js",
+    "alltest": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter spec test/_*_test*.js",
     "ci": "npm -s run lint && npm -s run build && npm -s run alltest-min && npm -s run check-cover",
     "test": "npm -s run lint && npm -s run build && npm -s run unittest",
     "check-cover": "istanbul check-coverage --statements 92 --branches 82 --lines 93 --functions 91"