Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change token refresh mechanism to depend on the token expiration time #8082

Merged
merged 11 commits into from
Sep 17, 2020
2 changes: 1 addition & 1 deletion packages/ckeditor-cloud-services-core/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@ckeditor/ckeditor-cloud-services-core",
"version": "22.0.0",
"version": "22.0.1",
pomek marked this conversation as resolved.
Show resolved Hide resolved
"description": "CKEditor Cloud Services Core API.",
"keywords": [
"ckeditor5",
Expand Down
53 changes: 37 additions & 16 deletions packages/ckeditor-cloud-services-core/src/token/token.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@
* @module cloud-services-core/token
*/

/* globals XMLHttpRequest, setInterval, clearInterval */
/* globals XMLHttpRequest, setTimeout, clearTimeout, atob */

import mix from '@ckeditor/ckeditor5-utils/src/mix';
import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';

const DEFAULT_OPTIONS = { refreshInterval: 3600000, autoRefresh: true };
const DEFAULT_OPTIONS = { autoRefresh: true };
const DEFAULT_TOKEN_REFRESH_TIMEOUT_TIME = 3600000;

/**
* Class representing the token used for communication with CKEditor Cloud Services.
Expand All @@ -30,7 +31,6 @@ class Token {
* value is a function it has to match the {@link module:cloud-services-core/token~refreshToken} interface.
* @param {Object} options
* @param {String} [options.initValue] Initial value of the token.
* @param {Number} [options.refreshInterval=3600000] Delay between refreshes. Default 1 hour.
* @param {Boolean} [options.autoRefresh=true] Specifies whether to start the refresh automatically.
*/
constructor( tokenUrlOrRefreshToken, options = DEFAULT_OPTIONS ) {
Expand Down Expand Up @@ -84,10 +84,6 @@ class Token {
*/
init() {
return new Promise( ( resolve, reject ) => {
if ( this._options.autoRefresh ) {
this._startRefreshing();
}

if ( !this.value ) {
this.refreshToken()
.then( resolve )
Expand All @@ -96,6 +92,10 @@ class Token {
return;
}

if ( this._options.autoRefresh ) {
this._registerRefreshTokenTimeout();
}

resolve( this );
} );
}
Expand All @@ -106,33 +106,55 @@ class Token {
*/
refreshToken() {
return this._refresh()
.then( value => this.set( 'value', value ) )
.then( value => {
this.set( 'value', value );
jczapiewski-cksource marked this conversation as resolved.
Show resolved Hide resolved

if ( this._options.autoRefresh ) {
this._registerRefreshTokenTimeout();
}

return this;
jczapiewski-cksource marked this conversation as resolved.
Show resolved Hide resolved
} )
.then( () => this );
}

/**
* Destroys token instance. Stops refreshing.
*/
destroy() {
this._stopRefreshing();
clearTimeout( this._tokenRefreshTimeout );
}

/**
* Starts value refreshing every `refreshInterval` time.
* Registers refresh token timeout for time taken from token.
jczapiewski-cksource marked this conversation as resolved.
Show resolved Hide resolved
*
* @protected
*/
_startRefreshing() {
this._refreshInterval = setInterval( () => this.refreshToken(), this._options.refreshInterval );
_registerRefreshTokenTimeout() {
const tokenRefreshTimeoutTime = this._getTokenRefreshTimeoutTime();

jczapiewski-cksource marked this conversation as resolved.
Show resolved Hide resolved
this._tokenRefreshTimeout = setTimeout( () => {
this.refreshToken();
}, tokenRefreshTimeoutTime );
}

/**
* Stops value refreshing.
* Returns token refresh timeout time calculated from expire time in token payload.
* If token parse fails, the default DEFAULT_TOKEN_REFRESH_TIMEOUT_TIME is returned.
jczapiewski-cksource marked this conversation as resolved.
Show resolved Hide resolved
*
* @returns {Number}
* @protected
*/
_stopRefreshing() {
clearInterval( this._refreshInterval );
_getTokenRefreshTimeoutTime() {
try {
const [ , binaryTokenPayload ] = this.value.split( '.' );
const { exp: tokenExpireTime } = JSON.parse( atob( binaryTokenPayload ) );
const tokenRefreshTimeoutTime = Math.floor( ( tokenExpireTime - Date.now() ) / 2 );

return tokenRefreshTimeoutTime;
} catch ( err ) {
return DEFAULT_TOKEN_REFRESH_TIMEOUT_TIME;
}
}

/**
Expand All @@ -142,7 +164,6 @@ class Token {
* value is a function it has to match the {@link module:cloud-services-core/token~refreshToken} interface.
* @param {Object} options
* @param {String} [options.initValue] Initial value of the token.
* @param {Number} [options.refreshInterval=3600000] Delay between refreshes. Default 1 hour.
* @param {Boolean} [options.autoRefresh=true] Specifies whether to start the refresh automatically.
* @returns {Promise.<module:cloud-services-core/token~Token>}
*/
Expand Down
130 changes: 60 additions & 70 deletions packages/ckeditor-cloud-services-core/tests/token/token.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,48 +84,61 @@ describe( 'Token', () => {
} );
} );

it( 'should start token refresh every 1 hour', done => {
const clock = sinon.useFakeTimers( { toFake: [ 'setInterval' ] } );
it( 'should refresh token with time specified in token `exp` payload property', async () => {
jczapiewski-cksource marked this conversation as resolved.
Show resolved Hide resolved
const clock = sinon.useFakeTimers( { toFake: [ 'setTimeout' ] } );
const tokenInitValue = `header.${ btoa( JSON.stringify( { exp: Date.now() + 3600000 } ) ) }.signature`;

const token = new Token( 'http://token-endpoint', { initValue: 'initValue' } );
const token = new Token( 'http://token-endpoint', { initValue: tokenInitValue } );

token.init()
.then( () => {
clock.tick( 3600000 );
clock.tick( 3600000 );
clock.tick( 3600000 );
clock.tick( 3600000 );
clock.tick( 3600000 );
await token.init();

expect( requests.length ).to.equal( 5 );
await clock.tickAsync( 1800000 );
requests[ 0 ].respond( 200, '', `header.${ btoa( JSON.stringify( { exp: Date.now() + 150000 } ) ) }.signature` );

clock.restore();
await clock.tickAsync( 75000 );
requests[ 1 ].respond( 200, '', `header.${ btoa( JSON.stringify( { exp: Date.now() + 10000 } ) ) }.signature` );

done();
} );
await clock.tickAsync( 5000 );
requests[ 2 ].respond( 200, '', `header.${ btoa( JSON.stringify( { exp: Date.now() + 2000 } ) ) }.signature` );

await clock.tickAsync( 1000 );
requests[ 3 ].respond( 200, '', `header.${ btoa( JSON.stringify( { exp: Date.now() + 300 } ) ) }.signature` );

await clock.tickAsync( 150 );
requests[ 4 ].respond( 200, '', `header.${ btoa( JSON.stringify( { exp: Date.now() + 300 } ) ) }.signature` );

expect( requests.length ).to.equal( 5 );

clock.restore();
} );
} );

describe( 'destroy', () => {
it( 'should stop refreshing the token', () => {
const clock = sinon.useFakeTimers( { toFake: [ 'setInterval', 'clearInterval' ] } );
const token = new Token( 'http://token-endpoint', { initValue: 'initValue' } );
it( 'should stop refreshing the token', async () => {
const clock = sinon.useFakeTimers( { toFake: [ 'setTimeout', 'clearTimeout' ] } );
const tokenInitValue = `header.${ btoa( JSON.stringify( { exp: Date.now() + 3600000 } ) ) }.signature`;

return token.init()
.then( () => {
clock.tick( 3600000 );
clock.tick( 3600000 );
const token = new Token( 'http://token-endpoint', { initValue: tokenInitValue } );

expect( requests.length ).to.equal( 2 );
await token.init();

token.destroy();
await clock.tickAsync( 1800000 );
requests[ 0 ].respond( 200, '', `header.${ btoa( JSON.stringify( { exp: Date.now() + 150000 } ) ) }.signature` );
await clock.tickAsync( 100 );

clock.tick( 3600000 );
clock.tick( 3600000 );
clock.tick( 3600000 );
await clock.tickAsync( 75000 );
requests[ 1 ].respond( 200, '', `header.${ btoa( JSON.stringify( { exp: Date.now() + 10000 } ) ) }.signature` );
await clock.tickAsync( 100 );

expect( requests.length ).to.equal( 2 );
} );
token.destroy();

await clock.tickAsync( 3600000 );
await clock.tickAsync( 3600000 );
await clock.tickAsync( 3600000 );

expect( requests.length ).to.equal( 2 );

clock.restore();
} );
} );

Expand Down Expand Up @@ -202,49 +215,39 @@ describe( 'Token', () => {
} );
} );

describe( '_startRefreshing()', () => {
it( 'should start refreshing', () => {
const clock = sinon.useFakeTimers( { toFake: [ 'setInterval' ] } );
describe( '_registerRefreshTokenTimeout()', () => {
jczapiewski-cksource marked this conversation as resolved.
Show resolved Hide resolved
it( 'should register refresh token timeout and run refresh after that time', async () => {
const clock = sinon.useFakeTimers( { toFake: [ 'setTimeout' ] } );
const tokenInitValue = `header.${ btoa( JSON.stringify( { exp: Date.now() + 3600000 } ) ) }.signature`;

const token = new Token( 'http://token-endpoint', { initValue: 'initValue', autoRefresh: false } );
const token = new Token( 'http://token-endpoint', { initValue: tokenInitValue, autoRefresh: false } );

token._startRefreshing();
token._registerRefreshTokenTimeout();

clock.tick( 3600000 );
clock.tick( 3600000 );
clock.tick( 3600000 );
clock.tick( 3600000 );
clock.tick( 3600000 );
await clock.tickAsync( 1800000 );

expect( requests.length ).to.equal( 5 );
expect( requests.length ).to.equal( 1 );

clock.restore();
} );
} );

describe( '_stopRefreshing()', () => {
it( 'should stop refreshing', done => {
const clock = sinon.useFakeTimers( { toFake: [ 'setInterval', 'clearInterval' ] } );

const token = new Token( 'http://token-endpoint', { initValue: 'initValue' } );

token.init()
.then( () => {
clock.tick( 3600000 );
clock.tick( 3600000 );
clock.tick( 3600000 );
describe( '_getTokenRefreshTimeoutTime', () => {
it( 'should return timeout time based on expiration time in token for valid token', () => {
const tokenInitValue = `header.${ btoa( JSON.stringify( { exp: Date.now() + 3600000 } ) ) }.signature`;
const token = new Token( 'http://token-endpoint', { initValue: tokenInitValue, autoRefresh: false } );

token._stopRefreshing();
const timeoutTime = token._getTokenRefreshTimeoutTime();

clock.tick( 3600000 );
clock.tick( 3600000 );
expect( timeoutTime ).to.eq( 1800000 );
} );

expect( requests.length ).to.equal( 3 );
it( 'should return default refresh timeout time if token parse fails', () => {
jczapiewski-cksource marked this conversation as resolved.
Show resolved Hide resolved
const token = new Token( 'http://token-endpoint', { initValue: 'initValue', autoRefresh: false } );

clock.restore();
const timeoutTime = token._getTokenRefreshTimeoutTime();

done();
} );
expect( timeoutTime ).to.eq( 3600000 );
} );
} );

Expand All @@ -259,18 +262,5 @@ describe( 'Token', () => {

requests[ 0 ].respond( 200, '', 'token-value' );
} );

it( 'should use default options when none passed', done => {
const intervalSpy = sinon.spy( window, 'setInterval' );

Token.create( 'http://token-endpoint' )
.then( () => {
expect( intervalSpy.args[ 0 ][ 1 ] ).to.equal( 3600000 );

done();
} );

requests[ 0 ].respond( 200, '', 'token-value' );
} );
} );
} );