Skip to content

Commit

Permalink
fix(device:sak_auth_prov): Start timer and emit events only when requ…
Browse files Browse the repository at this point in the history 
…ested
  • Loading branch information
Pierre Cauchois committed Jun 26, 2018
1 parent d8a4bb3 commit 734bd35
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 42 deletions.
4 changes: 2 additions & 2 deletions device/core/devdoc/sak_authentication_provider_requirements.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ sakAuthProvider.on('newTokenAvailable', function (credentials) {

**SRS_NODE_SAK_AUTH_PROVIDER_16_001: [** The `constructor` shall create the initial token value using the `credentials` parameter. **]**

**SRS_NODE_SAK_AUTH_PROVIDER_16_002: [** The `constructor` shall start a timer that will automatically renew the token every (`tokenValidTimeInSeconds` - `tokenRenewalMarginInSeconds`) seconds if specified, or 45 minutes by default. **]**

**SRS_NODE_SAK_AUTH_PROVIDER_16_011: [** The `constructor` shall throw an `ArgumentError` if the `tokenRenewalMarginInSeconds` is less than or equal `tokenValidTimeInSeconds`. **]**

## getDeviceCredentials(callback: (err: Error, credentials: TransportConfig) => void): void

**SRS_NODE_SAK_AUTH_PROVIDER_16_002: [** The `getDeviceCredentials` method shall start a timer that will automatically renew the token every (`tokenValidTimeInSeconds` - `tokenRenewalMarginInSeconds`) seconds if specified, or 45 minutes by default. **]**

**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. **]**

## newTokenAvailable event
Expand Down
3 changes: 1 addition & 2 deletions device/core/src/iotedge_authentication_provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ export class IotEdgeAuthenticationProvider extends SharedAccessKeyAuthentication
gatewayHostName: _authConfig && _authConfig.gatewayHostName
},
tokenValidTimeInSeconds,
tokenRenewalMarginInSeconds,
true
tokenRenewalMarginInSeconds
);

// Codes_SRS_NODE_IOTEDGED_AUTHENTICATION_PROVIDER_13_001: [ The constructor shall throw a ReferenceError if the _authConfig parameter is falsy. ]
Expand Down
66 changes: 38 additions & 28 deletions device/core/src/sak_authentication_provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export class SharedAccessKeyAuthenticationProvider extends EventEmitter implemen
* @param tokenValidTimeInSeconds [optional] The number of seconds for which a token is supposed to be valid.
* @param tokenRenewalMarginInSeconds [optional] The number of seconds before the end of the validity period during which the `SharedAccessKeyAuthenticationProvider` should renew the token.
*/
constructor(credentials: TransportConfig, tokenValidTimeInSeconds?: number, tokenRenewalMarginInSeconds?: number, startTimerOnNextTick?: boolean) {
constructor(credentials: TransportConfig, tokenValidTimeInSeconds?: number, tokenRenewalMarginInSeconds?: number) {
super();
/*Codes_SRS_NODE_SAK_AUTH_PROVIDER_16_001: [The `constructor` shall create the initial token value using the `credentials` parameter.]*/
this._credentials = credentials;
Expand All @@ -42,16 +42,6 @@ export class SharedAccessKeyAuthenticationProvider extends EventEmitter implemen
if (this._tokenValidTimeInSeconds <= this._tokenRenewalMarginInSeconds) {
throw new errors.ArgumentError('tokenRenewalMarginInSeconds must be less than tokenValidTimeInSeconds');
}

/*Codes_SRS_NODE_SAK_AUTH_PROVIDER_16_002: [The `constructor` shall start a timer that will automatically renew the token every (`tokenValidTimeInSeconds` - `tokenRenewalMarginInSeconds`) seconds if specified, or 45 minutes by default.]*/

if (startTimerOnNextTick) {
// we kick off the initial token creation during the next tick because if this code is being run from a sub-class then
// there might be additional initialization that needs to occur before we are ready to sign tokens
setImmediate(() => this._renewToken());
} else {
this._renewToken();
}
}

/**
Expand All @@ -61,7 +51,15 @@ export class SharedAccessKeyAuthenticationProvider extends EventEmitter implemen
*/
getDeviceCredentials(callback: (err: Error, credentials?: TransportConfig) => void): void {
if (this._shouldRenewToken()) {
this._renewToken(callback);
this._renewToken((err, creds) => {
if (err) {
callback(err);
} else {
/*Codes_SRS_NODE_SAK_AUTH_PROVIDER_16_002: [The `getDeviceCredentials` method shall start a timer that will automatically renew the token every (`tokenValidTimeInSeconds` - `tokenRenewalMarginInSeconds`) seconds if specified, or 45 minutes by default.]*/
this._scheduleNextExpiryTimeout();
callback(null, creds);
}
});
} else {
/*Codes_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.]*/
callback(null, this._credentials);
Expand All @@ -81,11 +79,7 @@ export class SharedAccessKeyAuthenticationProvider extends EventEmitter implemen
return (this._currentTokenExpiryTimeInSeconds - currentTimeInSeconds) < this._tokenRenewalMarginInSeconds;
}

private _renewToken(callback?: (err: Error, credentials?: TransportConfig) => void): void {
if (this._renewalTimeout) {
clearTimeout(this._renewalTimeout);
}

private _renewToken(callback: (err: Error, credentials?: TransportConfig) => void): void {
/*Codes_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.]*/
const newExpiry = Math.floor(Date.now() / 1000) + this._tokenValidTimeInSeconds;

Expand All @@ -104,27 +98,43 @@ export class SharedAccessKeyAuthenticationProvider extends EventEmitter implemen
const resourceUri = encodeUriComponentStrict(resourceString);
this._sign(resourceUri, newExpiry, (err, signature) => {
if (err) {
if (callback) {
callback(err);
} else {
this.emit('error', err);
}
callback(err);
} else {
this._currentTokenExpiryTimeInSeconds = newExpiry;
this._credentials.sharedAccessSignature = signature;

const nextRenewalTimeout = (this._tokenValidTimeInSeconds - this._tokenRenewalMarginInSeconds) * 1000;
this._renewalTimeout = setTimeout(() => this._renewToken(), nextRenewalTimeout);
callback(null, this._credentials);
}
});
}

private _expiryTimerHandler(): void {
if (this._renewalTimeout) {
clearTimeout(this._renewalTimeout);
this._renewalTimeout = null;
}

this._renewToken((err) => {
if (!err) {
this._scheduleNextExpiryTimeout();
/*Codes_SRS_NODE_SAK_AUTH_PROVIDER_16_005: [Every time a new token is created, the `newTokenAvailable` event shall be fired with the updated credentials.]*/
this.emit('newTokenAvailable', this._credentials);

if (callback) {
callback(null, this._credentials);
}
} else {
this.emit('error', err);
}
});
}

private _scheduleNextExpiryTimeout(): void {
if (this._renewalTimeout) {
clearTimeout(this._renewalTimeout);
this._renewalTimeout = null;
}

const nextRenewalTimeout = (this._tokenValidTimeInSeconds - this._tokenRenewalMarginInSeconds) * 1000;
this._renewalTimeout = setTimeout(() => this._expiryTimerHandler(), nextRenewalTimeout);
}

/**
* Creates a new `SharedAccessKeyAuthenticationProvider` from a connection string
*
Expand Down
22 changes: 12 additions & 10 deletions device/core/test/_sak_authentication_provider_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('SharedAccessKeyAuthenticationProvider', function () {
});
});

/*Tests_SRS_NODE_SAK_AUTH_PROVIDER_16_002: [The `constructor` shall start a timer that will automatically renew the token every (`tokenValidTimeInSeconds` - `tokenRenewalMarginInSeconds`) seconds if specified, or 45 minutes by default.]*/
/*Tests_SRS_NODE_SAK_AUTH_PROVIDER_16_002: [The `getDeviceCredentials` method shall start a timer that will automatically renew the token every (`tokenValidTimeInSeconds` - `tokenRenewalMarginInSeconds`) seconds if specified, or 45 minutes by default.]*/
it('starts a timer to renew the token', function (testCallback) {
this.clock = sinon.useFakeTimers();
var testClock = this.clock;
Expand Down Expand Up @@ -94,23 +94,25 @@ describe('SharedAccessKeyAuthenticationProvider', function () {
});
});

it('_renewToken propagates error via event if _sign fails', function (testCallback) {
it('emits an error if _sign fails while automatically renewing on token timeout', function (testCallback) {
var fakeCredentials = {
deviceId: 'fakeDeviceId',
host: 'fake.host.name',
sharedAccessKey: 'fakeKey'
};
var fakeError = new Error('whoops');
var sakAuthProvider = new SharedAccessKeyAuthenticationProvider(fakeCredentials, 10, 1);
var eventSpy = sinon.spy();
sakAuthProvider.on('error', eventSpy);
sakAuthProvider.on('error', function (err) {
assert.strictEqual(err, fakeError);
testCallback();
});

sinon.stub(sakAuthProvider, '_sign').callsArgWith(2, 'whoops');
sakAuthProvider._renewToken();
assert.isTrue(eventSpy.calledOnce);
testCallback();
sinon.stub(sakAuthProvider, '_sign').callsArgWith(2, fakeError);
sakAuthProvider._expiryTimerHandler();
});

it('_renewToken propagates error via callback if _sign fails', function (testCallback) {
it('getDeviceCredentials propagates error via callback if _sign fails', function (testCallback) {
var fakeCredentials = {
deviceId: 'fakeDeviceId',
host: 'fake.host.name',
Expand All @@ -119,10 +121,10 @@ describe('SharedAccessKeyAuthenticationProvider', function () {
var sakAuthProvider = new SharedAccessKeyAuthenticationProvider(fakeCredentials, 10, 1);

sinon.stub(sakAuthProvider, '_sign').callsArgWith(2, 'whoops');
sakAuthProvider._renewToken(function(err) {
sakAuthProvider.getDeviceCredentials(function(err) {
assert.equal(err, 'whoops');
testCallback();
});
testCallback();
});

it('_renewToken provides result via callback', function (testCallback) {
Expand Down

0 comments on commit 734bd35

Please sign in to comment.