From 223443fa1a32c9cc02246ee8377faee287c3b43c Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 13 Aug 2024 16:47:30 +0200 Subject: [PATCH] Tests and code refactoring. --- .../src/token/token.ts | 71 ++++---- .../tests/token/token.js | 153 ++++++++++++++++++ 2 files changed, 183 insertions(+), 41 deletions(-) diff --git a/packages/ckeditor5-cloud-services/src/token/token.ts b/packages/ckeditor5-cloud-services/src/token/token.ts index d9e7db9aa18..88e230b2a51 100644 --- a/packages/ckeditor5-cloud-services/src/token/token.ts +++ b/packages/ckeditor5-cloud-services/src/token/token.ts @@ -9,7 +9,7 @@ /* globals XMLHttpRequest, setTimeout, clearTimeout, atob */ -import { ObservableMixin, CKEditorError } from 'ckeditor5/src/utils.js'; +import { ObservableMixin, CKEditorError, logWarning } from 'ckeditor5/src/utils.js'; import type { TokenUrl } from '../cloudservicesconfig.js'; const DEFAULT_OPTIONS = { autoRefresh: true }; @@ -37,10 +37,15 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { */ private _refresh: () => Promise; + /** + * Cached token options. + */ private _options: { initValue?: string; autoRefresh: boolean }; + /** + * `setTimeout()` id for a token refresh when {@link module:cloud-services/token/token~TokenOptions auto refresh} is enabled. + */ private _tokenRefreshTimeout?: ReturnType; - private _tokenFailedRefreshTimeout?: ReturnType; /** * Creates `Token` instance. @@ -104,12 +109,14 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { * Refresh token method. Useful in a method form as it can be override in tests. */ public refreshToken(): Promise { + const autoRefresh = this._options.autoRefresh; + return this._refresh() .then( value => { this._validateTokenValue( value ); this.set( 'value', value ); - if ( this._options.autoRefresh ) { + if ( autoRefresh ) { this._registerRefreshTokenTimeout(); } @@ -117,18 +124,30 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { } ) .catch( err => { /** - * TODO + * You will see this warning when the CKEditor {@link module:cloud-services/token/token~Token token} could not be refreshed. + * This may be a result of a network error, a token endpoint (server) error, or an invalid + * {@link module:cloud-services/cloudservicesconfig~CloudServicesConfig#tokenUrl token URL configuration}. + * + * If this warning repeats, please make sure that the configuration is correct and that the token + * endpoint is up and running. {@link module:cloud-services/cloudservicesconfig~CloudServicesConfig#tokenUrl Learn more} + * about token configuration. + * + * **Note:** If the {@link module:cloud-services/token/token~TokenOptions auto refresh} is enabled, attempts to refresh + * the token will be made until destroyed. * * @error token-refresh-failed + * @param autoRefresh Whether the token will keep auto refreshing. */ - console.warn( 'token-refresh-failed: TODO' ); + logWarning( 'token-refresh-failed', { autoRefresh } ); // If the refresh failed, keep trying to refresh the token. Failing to do so will eventually // lead to the disconnection from the RTC service and the editing session (and potential data loss // if the user keeps editing). - this._registerFailedRefreshTokenTimeout(); + if ( autoRefresh ) { + this._registerRefreshTokenTimeout( TOKEN_FAILED_REFRESH_TIMEOUT_TIME ); + } - return err; + throw err; } ); } @@ -136,7 +155,7 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { * Destroys token instance. Stops refreshing. */ public destroy(): void { - this._clearRefreshTimeouts(); + clearTimeout( this._tokenRefreshTimeout ); } /** @@ -168,36 +187,16 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { /** * Registers a refresh token timeout for the time taken from token. */ - private _registerRefreshTokenTimeout() { - const tokenRefreshTimeoutTime = this._getTokenRefreshTimeoutTime(); + private _registerRefreshTokenTimeout( timeoutTime?: number ) { + const tokenRefreshTimeoutTime = timeoutTime || this._getTokenRefreshTimeoutTime(); - this._clearRefreshTimeouts(); + clearTimeout( this._tokenRefreshTimeout ); this._tokenRefreshTimeout = setTimeout( () => { - console.log( 'Refreshing token due to expiry time...' ); - this.refreshToken(); }, tokenRefreshTimeoutTime ); } - /** - * TODO - */ - private _registerFailedRefreshTokenTimeout() { - this._clearRefreshTimeouts(); - - this._tokenFailedRefreshTimeout = setTimeout( () => { - console.log( 'Refreshing token after a failure...' ); - - this.refreshToken() - .then( () => { - // If refresh was successful, the logic will switch to the default timeout (if enabled) - // based on token expiry time and this timeout will no longer be needed. - clearTimeout( this._tokenFailedRefreshTimeout ); - } ); - }, TOKEN_FAILED_REFRESH_TIMEOUT_TIME ); - } - /** * Returns token refresh timeout time calculated from expire time in the token payload. * @@ -208,8 +207,6 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { const [ , binaryTokenPayload ] = this.value!.split( '.' ); const { exp: tokenExpireTime } = JSON.parse( atob( binaryTokenPayload ) ); - console.log( 'Token expiry time', new Date( tokenExpireTime * 1000 ) ); - if ( !tokenExpireTime ) { return DEFAULT_TOKEN_REFRESH_TIMEOUT_TIME; } @@ -233,14 +230,6 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { return token.init(); } - - /** - * TODO - */ - private _clearRefreshTimeouts() { - clearTimeout( this._tokenRefreshTimeout ); - clearTimeout( this._tokenFailedRefreshTimeout ); - } } /** diff --git a/packages/ckeditor5-cloud-services/tests/token/token.js b/packages/ckeditor5-cloud-services/tests/token/token.js index abbc18641d5..9a26522a4f2 100644 --- a/packages/ckeditor5-cloud-services/tests/token/token.js +++ b/packages/ckeditor5-cloud-services/tests/token/token.js @@ -7,10 +7,13 @@ import Token from '../../src/token/token.js'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror.js'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils.js'; describe( 'Token', () => { let requests; + testUtils.createSinonSandbox(); + beforeEach( () => { requests = []; @@ -241,6 +244,8 @@ describe( 'Token', () => { } ); it( 'should throw an error if the returned token is wrapped in additional quotes', done => { + testUtils.sinon.stub( console, 'warn' ); + const tokenValue = getTestTokenValue(); const token = new Token( 'http://token-endpoint', { autoRefresh: false } ); @@ -258,6 +263,8 @@ describe( 'Token', () => { } ); it( 'should throw an error if the returned token is not a valid JWT token', done => { + testUtils.sinon.stub( console, 'warn' ); + const token = new Token( 'http://token-endpoint', { autoRefresh: false } ); token.refreshToken() @@ -335,6 +342,152 @@ describe( 'Token', () => { expect( error ).to.equal( 'Custom error occurred' ); } ); } ); + + describe( 'refresh failure handling', () => { + let clock; + + beforeEach( () => { + clock = sinon.useFakeTimers( { + toFake: [ 'setTimeout', 'clearTimeout' ] + } ); + + testUtils.sinon.stub( console, 'warn' ); + } ); + + afterEach( () => { + clock.restore(); + } ); + + it( 'should log a warning in the console', () => { + const tokenInitValue = getTestTokenValue(); + const token = new Token( 'http://token-endpoint', { initValue: tokenInitValue, autoRefresh: false } ); + const promise = token.refreshToken(); + + requests[ 0 ].error(); + + return promise.then( () => { + throw new Error( 'Promise should fail' ); + }, () => { + sinon.assert.calledWithMatch( console.warn, 'token-refresh-failed', { autoRefresh: false } ); + } ); + } ); + + it( 'should attempt to periodically refresh the token', async () => { + const tokenInitValue = getTestTokenValue(); + const token = new Token( 'http://token-endpoint', { initValue: tokenInitValue, autoRefresh: true } ); + const promise = token.refreshToken(); + + requests[ 0 ].error(); + + return promise + .then( async () => { + throw new Error( 'Promise should fail' ); + } ) + .catch( async err => { + expect( err ).to.match( /Network Error/ ); + + await clock.tickAsync( '05' ); + expect( requests.length ).to.equal( 2 ); + + requests[ 1 ].error(); + + await clock.tickAsync( '05' ); + expect( requests.length ).to.equal( 3 ); + + requests[ 2 ].error(); + + await clock.tickAsync( '05' ); + expect( requests.length ).to.equal( 4 ); + } ); + } ); + + it( 'should restore the regular refresh interval after a successfull refresh', () => { + const tokenInitValue = getTestTokenValue(); + const token = new Token( 'http://token-endpoint', { initValue: tokenInitValue, autoRefresh: true } ); + const promise = token.refreshToken(); + + requests[ 0 ].error(); + + return promise + .then( async () => { + throw new Error( 'Promise should fail' ); + } ) + .catch( async err => { + expect( err ).to.match( /Network Error/ ); + + await clock.tickAsync( '05' ); + expect( requests.length ).to.equal( 2 ); + + requests[ 1 ].respond( 200, '', getTestTokenValue( 20 ) ); + + await clock.tickAsync( '05' ); + // Switched to 10s interval because refresh was successful. + expect( requests.length ).to.equal( 2 ); + + await clock.tickAsync( '05' ); + expect( requests.length ).to.equal( 3 ); + + requests[ 2 ].respond( 200, '', getTestTokenValue( 20 ) ); + + await clock.tickAsync( '10' ); + expect( requests.length ).to.equal( 4 ); + } ); + } ); + + it( 'should not auto-refresh after a failure if options.autoRefresh option is false', () => { + const tokenInitValue = getTestTokenValue(); + const token = new Token( 'http://token-endpoint', { initValue: tokenInitValue, autoRefresh: false } ); + const promise = token.refreshToken(); + + requests[ 0 ].error(); + + return promise + .then( async () => { + throw new Error( 'Promise should fail' ); + } ) + .catch( async err => { + expect( err ).to.match( /Network Error/ ); + + await clock.tickAsync( '05' ); + expect( requests.length ).to.equal( 1 ); + + await clock.tickAsync( '10' ); + expect( requests.length ).to.equal( 1 ); + } ); + } ); + + it( 'should clear any queued refresh upon manual refreshToken() call to avoid duplicated refreshes', () => { + const tokenInitValue = getTestTokenValue(); + const token = new Token( 'http://token-endpoint', { initValue: tokenInitValue, autoRefresh: true } ); + const promise = token.refreshToken(); + + requests[ 0 ].error(); + + return promise + .then( async () => { + throw new Error( 'Promise should fail' ); + } ) + .catch( async err => { + expect( err ).to.match( /Network Error/ ); + + await clock.tickAsync( '05' ); + expect( requests.length ).to.equal( 2 ); + + token.refreshToken(); + token.refreshToken(); + token.refreshToken(); + + requests[ 1 ].error(); + requests[ 2 ].error(); + requests[ 3 ].error(); + requests[ 4 ].error(); + + await clock.tickAsync( '05' ); + + expect( requests.length ).to.equal( 6 ); + } ); + } ); + } ); } ); describe( 'static create()', () => {