From ef31431f3a02bcdf2aaee3afb4b1f8e007cc5d3d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 8 Aug 2024 15:53:27 +0200 Subject: [PATCH 1/4] WIP --- .../src/token/token.ts | 53 +++++++++++++++++-- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-cloud-services/src/token/token.ts b/packages/ckeditor5-cloud-services/src/token/token.ts index 996470b8fe4..d9e7db9aa18 100644 --- a/packages/ckeditor5-cloud-services/src/token/token.ts +++ b/packages/ckeditor5-cloud-services/src/token/token.ts @@ -13,7 +13,8 @@ import { ObservableMixin, CKEditorError } from 'ckeditor5/src/utils.js'; import type { TokenUrl } from '../cloudservicesconfig.js'; const DEFAULT_OPTIONS = { autoRefresh: true }; -const DEFAULT_TOKEN_REFRESH_TIMEOUT_TIME = 3600000; +const DEFAULT_TOKEN_REFRESH_TIMEOUT_TIME = 3600000; // 1 hour +const TOKEN_FAILED_REFRESH_TIMEOUT_TIME = 5000; // 5 seconds /** * Class representing the token used for communication with CKEditor Cloud Services. @@ -39,6 +40,7 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { private _options: { initValue?: string; autoRefresh: boolean }; private _tokenRefreshTimeout?: ReturnType; + private _tokenFailedRefreshTimeout?: ReturnType; /** * Creates `Token` instance. @@ -112,6 +114,21 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { } return this as InitializedToken; + } ) + .catch( err => { + /** + * TODO + * + * @error token-refresh-failed + */ + console.warn( 'token-refresh-failed: TODO' ); + + // 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(); + + return err; } ); } @@ -119,7 +136,7 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { * Destroys token instance. Stops refreshing. */ public destroy(): void { - clearTimeout( this._tokenRefreshTimeout ); + this._clearRefreshTimeouts(); } /** @@ -154,13 +171,33 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { private _registerRefreshTokenTimeout() { const tokenRefreshTimeoutTime = this._getTokenRefreshTimeoutTime(); - clearTimeout( this._tokenRefreshTimeout ); + this._clearRefreshTimeouts(); 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. * @@ -171,6 +208,8 @@ 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; } @@ -194,6 +233,14 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { return token.init(); } + + /** + * TODO + */ + private _clearRefreshTimeouts() { + clearTimeout( this._tokenRefreshTimeout ); + clearTimeout( this._tokenFailedRefreshTimeout ); + } } /** From 223443fa1a32c9cc02246ee8377faee287c3b43c Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 13 Aug 2024 16:47:30 +0200 Subject: [PATCH 2/4] 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()', () => { From c1bd2cf8ae0c3615457589096e6ffbd8d8ec7d3d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 13 Aug 2024 17:04:54 +0200 Subject: [PATCH 3/4] Docs: Updated Token and tokenUrl API docs. --- .../src/cloudservicesconfig.ts | 3 +++ .../src/token/token.ts | 18 +++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-cloud-services/src/cloudservicesconfig.ts b/packages/ckeditor5-cloud-services/src/cloudservicesconfig.ts index e11b87a6826..cae31040d1a 100644 --- a/packages/ckeditor5-cloud-services/src/cloudservicesconfig.ts +++ b/packages/ckeditor5-cloud-services/src/cloudservicesconfig.ts @@ -88,6 +88,9 @@ export interface CloudServicesConfig { * } ) * ``` * + * If the request to the token endpoint fails, the editor will call the token request function every 5 seconds in attempt + * to refresh the token. + * * You can find more information about token endpoints in the * {@glink @cs guides/easy-image/quick-start#create-token-endpoint Cloud Services - Quick start} * and {@glink @cs developer-resources/security/token-endpoint Cloud Services - Token endpoint} documentation. diff --git a/packages/ckeditor5-cloud-services/src/token/token.ts b/packages/ckeditor5-cloud-services/src/token/token.ts index 88e230b2a51..6bdace506d8 100644 --- a/packages/ckeditor5-cloud-services/src/token/token.ts +++ b/packages/ckeditor5-cloud-services/src/token/token.ts @@ -17,8 +17,9 @@ const DEFAULT_TOKEN_REFRESH_TIMEOUT_TIME = 3600000; // 1 hour const TOKEN_FAILED_REFRESH_TIMEOUT_TIME = 5000; // 5 seconds /** - * Class representing the token used for communication with CKEditor Cloud Services. - * Value of the token is retrieving from the specified URL and is refreshed every 1 hour by default. + * The class representing the token used for communication with CKEditor Cloud Services. + * The value of the token is retrieved from the specified URL and refreshed every 1 hour by default. + * If the token retrieval fails, the token will automatically retry in 5 seconds intervals. */ export default class Token extends /* #__PURE__ */ ObservableMixin() { /** @@ -106,7 +107,13 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { } /** - * Refresh token method. Useful in a method form as it can be override in tests. + * Refresh token method. Useful in a method form as it can be overridden in tests. + * + * This method will be invoked periodically based on the token expiry date after first call to keep the token up-to-date + * (requires {@link module:cloud-services/token/token~TokenOptions auto refresh option} to be set). + * + * If the token refresh fails, the method will retry in 5 seconds intervals until success or until the token gets + * {@link #destroy destroyed}. */ public refreshToken(): Promise { const autoRefresh = this._options.autoRefresh; @@ -132,8 +139,9 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { * 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. + * **Note:** If the token's {@link module:cloud-services/token/token~TokenOptions auto refresh option} is enabled, + * attempts to refresh will be made until success or token's + * {@link module:cloud-services/token/token~Token#destroy destruction}. * * @error token-refresh-failed * @param autoRefresh Whether the token will keep auto refreshing. From 348d08fc2c624a60530128c034ffa4553f5e8148 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 13 Aug 2024 17:07:34 +0200 Subject: [PATCH 4/4] Docs. --- packages/ckeditor5-cloud-services/src/token/token.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-cloud-services/src/token/token.ts b/packages/ckeditor5-cloud-services/src/token/token.ts index 6bdace506d8..b560a6b1078 100644 --- a/packages/ckeditor5-cloud-services/src/token/token.ts +++ b/packages/ckeditor5-cloud-services/src/token/token.ts @@ -112,7 +112,7 @@ export default class Token extends /* #__PURE__ */ ObservableMixin() { * This method will be invoked periodically based on the token expiry date after first call to keep the token up-to-date * (requires {@link module:cloud-services/token/token~TokenOptions auto refresh option} to be set). * - * If the token refresh fails, the method will retry in 5 seconds intervals until success or until the token gets + * If the token refresh fails, the method will retry in 5 seconds intervals until success or the token gets * {@link #destroy destroyed}. */ public refreshToken(): Promise {