From f095ebca91d72a286782b51958efa31263f5aaee Mon Sep 17 00:00:00 2001 From: chua Date: Wed, 20 Apr 2022 03:22:25 +0000 Subject: [PATCH 01/12] fix refresh token expiry lost during refresh --- src/Service.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/Service.js b/src/Service.js index 77ae4933..99545b21 100644 --- a/src/Service.js +++ b/src/Service.js @@ -579,6 +579,9 @@ Service_.prototype.parseToken_ = function(content) { throw new Error('Unknown token format: ' + this.tokenFormat_); } token.granted_time = getTimeInSeconds_(new Date()); + if (token.refresh_expires_in) { + token.refresh_token_expires_in = token.refresh_expires_in; + } return token; }; @@ -608,6 +611,14 @@ Service_.prototype.refresh = function() { if (!newToken.refresh_token) { newToken.refresh_token = token.refresh_token; } + if ( + !newToken.refresh_token_expires_in && + token.refresh_token_expires_in + ) { + newToken.refresh_token_expires_in = + token.refresh_token_expires_in + + (token.granted_time - newToken.granted_time); + } this.saveToken_(newToken); }); }; From 22a3c9d06a039425c4433e4d65b00237be69e6a1 Mon Sep 17 00:00:00 2001 From: chua Date: Wed, 20 Apr 2022 03:40:28 +0000 Subject: [PATCH 02/12] add failing test --- test/test.js | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/test.js b/test/test.js index 57fdd468..a162ecf6 100644 --- a/test/test.js +++ b/test/test.js @@ -436,6 +436,40 @@ describe('Service', () => { done(); }); }); + + it('should retain refresh expiry', () => { + const NOW_SECONDS = OAuth2.getTimeInSeconds_(new Date()); + const ONE_HOUR_AGO_SECONDS = NOW_SECONDS - 360; + var token = { + granted_time: ONE_HOUR_AGO_SECONDS, + expires_in: 100, + refresh_token: 'bar', + refresh_token_expires_in: 720 + }; + var properties = new MockProperties({ + 'oauth2.test': JSON.stringify(token) + }); + + mocks.UrlFetchApp.resultFunction = () => { + return JSON.stringify({ + access_token: 'token' + }); + }; + + OAuth2.createService('test') + .setClientId('abc') + .setClientSecret('def') + .setTokenUrl('http://www.example.com') + .setPropertyStore(properties) + .setLock(new MockLock()) + .refresh(); + + var storedToken = JSON.parse(properties.getProperty('oauth2.test')); + assert.equal(storedToken.access_token, 'token'); + assert.equal(storedToken.refresh_token, 'bar'); + assert.equal(storedToken.granted_time, NOW_SECONDS); + assert.equal(storedToken.refresh_token_expires_in, 360); + }); }); describe('#exchangeGrant_()', () => { From 73efd8038c88af5daadb330baa745de1c9036d94 Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Thu, 28 Apr 2022 15:25:05 +0800 Subject: [PATCH 03/12] revert previous changes to go with a different approach --- src/Service.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/Service.js b/src/Service.js index 99545b21..77ae4933 100644 --- a/src/Service.js +++ b/src/Service.js @@ -579,9 +579,6 @@ Service_.prototype.parseToken_ = function(content) { throw new Error('Unknown token format: ' + this.tokenFormat_); } token.granted_time = getTimeInSeconds_(new Date()); - if (token.refresh_expires_in) { - token.refresh_token_expires_in = token.refresh_expires_in; - } return token; }; @@ -611,14 +608,6 @@ Service_.prototype.refresh = function() { if (!newToken.refresh_token) { newToken.refresh_token = token.refresh_token; } - if ( - !newToken.refresh_token_expires_in && - token.refresh_token_expires_in - ) { - newToken.refresh_token_expires_in = - token.refresh_token_expires_in + - (token.granted_time - newToken.granted_time); - } this.saveToken_(newToken); }); }; From 9a8074859176f89ba6d2af2a51cbd4658729e474 Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Thu, 28 Apr 2022 15:58:00 +0800 Subject: [PATCH 04/12] stab at expires at variables --- src/Service.js | 47 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/src/Service.js b/src/Service.js index 77ae4933..cd443c25 100644 --- a/src/Service.js +++ b/src/Service.js @@ -578,10 +578,30 @@ Service_.prototype.parseToken_ = function(content) { } else { throw new Error('Unknown token format: ' + this.tokenFormat_); } - token.granted_time = getTimeInSeconds_(new Date()); + this.setExpiresAt_(token); return token; }; +/** + * Adds expiresAt annotations on the token. + * @param {string} token A token. + * @private + */ +Service_.prototype.setExpiresAt_ = function(token) { + // granted_time was added in prior versions of this library + var grantedTime = token.granted_time || getTimeInSeconds_(new Date()); + var expiresIn = token.expires_in_sec || token.expires_in || token.expires; + if (expiresIn) { + var expiresAt = grantedTime + Number(expiresIn); + token.expiresAt = expiresAt; + } + var refreshTokenExpiresIn = token.refresh_token_expires_in; + if (refreshTokenExpiresIn) { + var refreshTokenExpiresAt = grantedTime + Number(refreshTokenExpiresIn); + token.refreshTokenExpiresAt = refreshTokenExpiresAt; + } +}; + /** * Refreshes a token that has expired. This is only possible if offline access * was requested when the token was authorized. @@ -608,6 +628,10 @@ Service_.prototype.refresh = function() { if (!newToken.refresh_token) { newToken.refresh_token = token.refresh_token; } + this.setExpiresAt_(token); + if (!token.refreshTokenExpiresAt) { + newToken.refreshTokenExpiresAt = token.refreshTokenExpiresAt; + } this.saveToken_(newToken); }); }; @@ -659,12 +683,19 @@ Service_.prototype.isExpired_ = function(token) { var now = getTimeInSeconds_(new Date()); // Check the authorization token's expiration. - var expiresIn = token.expires_in_sec || token.expires_in || token.expires; - if (expiresIn) { - var expiresTime = token.granted_time + Number(expiresIn); - if (expiresTime - now < Service_.EXPIRATION_BUFFER_SECONDS_) { + if (token.expiresAt) { + if (token.expiresAt - now < Service_.EXPIRATION_BUFFER_SECONDS_) { expired = true; } + } else { + // Previous code path, provided for migration purpose, can be removed later + var expiresIn = token.expires_in_sec || token.expires_in || token.expires; + if (expiresIn) { + var expiresTime = token.granted_time + Number(expiresIn); + if (expiresTime - now < Service_.EXPIRATION_BUFFER_SECONDS_) { + expired = true; + } + } } // Check the ID token's expiration, if it exists. @@ -687,6 +718,12 @@ Service_.prototype.isExpired_ = function(token) { */ Service_.prototype.canRefresh_ = function(token) { if (!token.refresh_token) return false; + this.setExpiresAt_(token); + if (token.refreshTokenExpiresAt) { + return ( + token.refreshTokenExpiresAt - now < Service_.EXPIRATION_BUFFER_SECONDS_ + ); + } var expiresIn = token.refresh_token_expires_in; if (!expiresIn) { return true; From d53fd3096e2ba3343594f295a457ec822154fe18 Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Thu, 28 Apr 2022 16:00:18 +0800 Subject: [PATCH 05/12] fix test and typo --- test/test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/test.js b/test/test.js index a162ecf6..23ee959d 100644 --- a/test/test.js +++ b/test/test.js @@ -467,8 +467,7 @@ describe('Service', () => { var storedToken = JSON.parse(properties.getProperty('oauth2.test')); assert.equal(storedToken.access_token, 'token'); assert.equal(storedToken.refresh_token, 'bar'); - assert.equal(storedToken.granted_time, NOW_SECONDS); - assert.equal(storedToken.refresh_token_expires_in, 360); + assert.equal(storedToken.refreshTokenExpiresAt, NOW_SECONDS + 360); }); }); From cbf79c5ba46df94d79ea996178f9ceb41e2206ea Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Thu, 28 Apr 2022 16:00:33 +0800 Subject: [PATCH 06/12] fix typo --- src/Service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service.js b/src/Service.js index cd443c25..fae7337b 100644 --- a/src/Service.js +++ b/src/Service.js @@ -629,7 +629,7 @@ Service_.prototype.refresh = function() { newToken.refresh_token = token.refresh_token; } this.setExpiresAt_(token); - if (!token.refreshTokenExpiresAt) { + if (token.refreshTokenExpiresAt) { newToken.refreshTokenExpiresAt = token.refreshTokenExpiresAt; } this.saveToken_(newToken); From bdf22835ce3cf19423d0d9c4ad8970e9f7c35218 Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Thu, 28 Apr 2022 16:25:22 +0800 Subject: [PATCH 07/12] clean up code for easier review, minimize changes, follow previus patterns --- src/Service.js | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/Service.js b/src/Service.js index fae7337b..d55b8c1e 100644 --- a/src/Service.js +++ b/src/Service.js @@ -588,6 +588,11 @@ Service_.prototype.parseToken_ = function(content) { * @private */ Service_.prototype.setExpiresAt_ = function(token) { + // handle prior migrations + if (token.expiresAt) { + return; + } + // granted_time was added in prior versions of this library var grantedTime = token.granted_time || getTimeInSeconds_(new Date()); var expiresIn = token.expires_in_sec || token.expires_in || token.expires; @@ -687,14 +692,14 @@ Service_.prototype.isExpired_ = function(token) { if (token.expiresAt - now < Service_.EXPIRATION_BUFFER_SECONDS_) { expired = true; } - } else { - // Previous code path, provided for migration purpose, can be removed later - var expiresIn = token.expires_in_sec || token.expires_in || token.expires; - if (expiresIn) { - var expiresTime = token.granted_time + Number(expiresIn); - if (expiresTime - now < Service_.EXPIRATION_BUFFER_SECONDS_) { - expired = true; - } + } + + // Previous code path, provided for migration purpose, can be removed later + var expiresIn = token.expires_in_sec || token.expires_in || token.expires; + if (expiresIn) { + var expiresTime = token.granted_time + Number(expiresIn); + if (expiresTime - now < Service_.EXPIRATION_BUFFER_SECONDS_) { + expired = true; } } @@ -719,19 +724,13 @@ Service_.prototype.isExpired_ = function(token) { Service_.prototype.canRefresh_ = function(token) { if (!token.refresh_token) return false; this.setExpiresAt_(token); - if (token.refreshTokenExpiresAt) { + if (!token.refreshTokenExpiresAt) { + return false; + } else { return ( token.refreshTokenExpiresAt - now < Service_.EXPIRATION_BUFFER_SECONDS_ ); } - var expiresIn = token.refresh_token_expires_in; - if (!expiresIn) { - return true; - } else { - var expiresTime = token.granted_time + Number(expiresIn); - var now = getTimeInSeconds_(new Date()); - return expiresTime - now > Service_.EXPIRATION_BUFFER_SECONDS_; - } }; /** From f062572569a4a2598a47c8cb57c5fe1509373f5c Mon Sep 17 00:00:00 2001 From: Chris Chua Date: Thu, 28 Apr 2022 16:26:18 +0800 Subject: [PATCH 08/12] add missing var --- src/Service.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Service.js b/src/Service.js index d55b8c1e..db0eb4c0 100644 --- a/src/Service.js +++ b/src/Service.js @@ -727,6 +727,7 @@ Service_.prototype.canRefresh_ = function(token) { if (!token.refreshTokenExpiresAt) { return false; } else { + var now = getTimeInSeconds_(new Date()); return ( token.refreshTokenExpiresAt - now < Service_.EXPIRATION_BUFFER_SECONDS_ ); From ba5c11a47ce9aea69205702e4f81074aea0d6b2f Mon Sep 17 00:00:00 2001 From: chua Date: Tue, 17 May 2022 01:28:16 +0000 Subject: [PATCH 09/12] fix canRefresh logic --- src/Service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service.js b/src/Service.js index db0eb4c0..645cf1e1 100644 --- a/src/Service.js +++ b/src/Service.js @@ -725,7 +725,7 @@ Service_.prototype.canRefresh_ = function(token) { if (!token.refresh_token) return false; this.setExpiresAt_(token); if (!token.refreshTokenExpiresAt) { - return false; + return true; } else { var now = getTimeInSeconds_(new Date()); return ( From e28902c84b876329862c07698745724ba3dec19a Mon Sep 17 00:00:00 2001 From: chua Date: Tue, 17 May 2022 01:49:01 +0000 Subject: [PATCH 10/12] add setExpiresAt tests --- test/test.js | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/test.js b/test/test.js index 23ee959d..df95bd81 100644 --- a/test/test.js +++ b/test/test.js @@ -568,6 +568,37 @@ describe('Service', () => { }); }); + describe('#setExpiresAt_()', () => { + const NOW_SECONDS = OAuth2.getTimeInSeconds_(new Date()); + var service = OAuth2.createService('test') + .setPropertyStore(new MockProperties()) + .setCache(new MockCache()); + + it('should set expires at', () => { + // This is more of an optimization + const token = { + granted_time: NOW_SECONDS, + expires_in_sec: 100 + }; + service.setExpiresAt_(token); + assert.include(token, { + expiresAt: NOW_SECONDS + 100 + }); + }); + + it('should set refresh expires at', () => { + // This is more of an optimization + const token = { + granted_time: NOW_SECONDS, + refresh_token_expires_in: 200 + }; + service.setExpiresAt_(token); + assert.include(token, { + refreshTokenExpiresAt: NOW_SECONDS + 200 + }); + }); + }); + describe('#isExpired_()', () => { const NOW_SECONDS = OAuth2.getTimeInSeconds_(new Date()); const ONE_HOUR_AGO_SECONDS = NOW_SECONDS - 360; From 355fb898d4972ddafc0916f11a356f5ccb7a954f Mon Sep 17 00:00:00 2001 From: chua Date: Tue, 17 May 2022 01:53:24 +0000 Subject: [PATCH 11/12] remove extra comment --- test/test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/test.js b/test/test.js index df95bd81..e33732a8 100644 --- a/test/test.js +++ b/test/test.js @@ -575,7 +575,6 @@ describe('Service', () => { .setCache(new MockCache()); it('should set expires at', () => { - // This is more of an optimization const token = { granted_time: NOW_SECONDS, expires_in_sec: 100 @@ -587,7 +586,6 @@ describe('Service', () => { }); it('should set refresh expires at', () => { - // This is more of an optimization const token = { granted_time: NOW_SECONDS, refresh_token_expires_in: 200 From a38cab955cc414b0fdac083130291b7d5e308fba Mon Sep 17 00:00:00 2001 From: chua Date: Tue, 17 May 2022 02:08:46 +0000 Subject: [PATCH 12/12] add tests and fix typo in logic --- src/Service.js | 2 +- test/test.js | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/Service.js b/src/Service.js index 645cf1e1..b06f317b 100644 --- a/src/Service.js +++ b/src/Service.js @@ -729,7 +729,7 @@ Service_.prototype.canRefresh_ = function(token) { } else { var now = getTimeInSeconds_(new Date()); return ( - token.refreshTokenExpiresAt - now < Service_.EXPIRATION_BUFFER_SECONDS_ + token.refreshTokenExpiresAt - now > Service_.EXPIRATION_BUFFER_SECONDS_ ); } }; diff --git a/test/test.js b/test/test.js index e33732a8..3e84468e 100644 --- a/test/test.js +++ b/test/test.js @@ -597,6 +597,46 @@ describe('Service', () => { }); }); + describe('#canRefresh_()', () => { + const NOW_SECONDS = OAuth2.getTimeInSeconds_(new Date()); + const ONE_HOUR_AGO_SECONDS = NOW_SECONDS - 360; + const ONE_HOUR_LATER_SECONDS = NOW_SECONDS + 360; + var service = OAuth2.createService('test') + .setPropertyStore(new MockProperties()) + .setCache(new MockCache()); + + it('should return false if no refresh token', () => { + const token = {}; + assert.isFalse(service.canRefresh_(token)); + }); + + it('should return true if there is a refresh token', () => { + const token = { + refresh_token: 'bar', + }; + assert.isTrue(service.canRefresh_(token)); + }); + + it('should return true if it is not expired', () => { + const token = { + refresh_token: 'bar', + expiresAt: ONE_HOUR_LATER_SECONDS, + refreshTokenExpiresAt: ONE_HOUR_LATER_SECONDS + }; + console.log('test') + assert.isTrue(service.canRefresh_(token)); + }); + + it('should return false if it is expired', () => { + const token = { + refresh_token: 'bar', + expiresAt: ONE_HOUR_LATER_SECONDS, + refreshTokenExpiresAt: ONE_HOUR_AGO_SECONDS + }; + assert.isFalse(service.canRefresh_(token)); + }); + }); + describe('#isExpired_()', () => { const NOW_SECONDS = OAuth2.getTimeInSeconds_(new Date()); const ONE_HOUR_AGO_SECONDS = NOW_SECONDS - 360;