-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Add feature: refresh MSTeams token before it expires #1230
Conversation
This fixes #1141 |
lib/Teams.js
Outdated
}); | ||
|
||
//use the expires_in value, convert to ms and remove 10 mins in ms | ||
let tokenExpiresIn = 1000 * 60 * configuration.token_expires_in; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case configuration.token_expires_in is not set (undefined) , we should have default value as 60 minutes?
let tokenExpiresIn = 1000 * 60 * (configuration.token_expires_in || 60);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about setting configuration.token_expires_in
in TeamsAPI.js, when we start bot and get the first token ?
https://github.com/howdyai/botkit/blob/master/lib/TeamsAPI.js#L172
configuration.token_expires_in = json.expires_in
Example of json
token - {"token_type":"Bearer","expires_in":3599,"ext_expires_in":0,"access_token":" eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsIng1dCI6IjJLVmN1enFBaWRPTHFXU2FvbDd3Z0ZSR0NZbyIsImtpZCI6IjJLVmN1enFBaWRPTHFXU2FvbDd3Z0ZSR0NZbyJ9.eyJhdWQiOiJodHRwczovL2FwaS5ib3RmcmFtZXdvcmsuY29tIiwiaXNzIjoiaHR0cHM6Ly9zdHMud2luZG93cy5uZXQvZDZkNDk0MjAtZjM5Yi00ZGY3LWExZGMtZDU5YTkzNTg3MWRiLyIsImlhdCI6MTUxMTc1OTExNCwibmJmIjoxNTExNzU5MTE0LCJleHAiOjE1MTE3NjMwMTQsImFpbyI6IlkyTmdZSWhZdVBmYnQ1cUhWd0tuL2JCNDE3ejhDd0E9IiwiYXBwaWQiOiI1ZjVlMDAyMS1lNTNjLTQ1NDktOWE1NC0wNDMyNDAwM2EyM2YiLCJhcHBpZGFjciI6IjEiLCJpZHAiOiJodHRwczovL3N0cy53aW5kb3dzLm5ldC9kNmQ0OTQyMC1mMzliLTRkZjctYTFkYy1kNTlhOTM1ODcxZGIvIiwidGlkIjoiZDZkNDk0MjAtZjM5Yi00ZGY3LWExZGMtZDU5YTkzNTg3MWRiIiwidXRpIjoiLTk4UE1scU5xay1LeWxTaGdTMWFBQSIsInZlciI6IjEuMCJ9.IG_qHoN9WIZDH2hZCGDKW3EYl_7z8G2VwgxXHGWQR_9ZmHAMIUVM8uxqYR8cJTCRwgWOBXSMKS4yKGM8jdvchevJf0xndhvx_iocFun3_TBCWOzIpBWW_vUScNv1W70iM-CcXYvXo9CFHnsM_w7S8DMVov-_p0QEj-U-IHU_sls5hFIDaqpTZtqG_h6ZaK7KmIJtPFcxSM2uf8tUmWL7bdFPTBUzo-rM4yuzV3SewrzWl_EiXccnSEfi3fq5QWnWElwBXNt9VIQ2FqLmob2YHa3B5JLrd7vCtcJGSKZ6yUSphRGTyfq75grwdGrZ1lfPUWfzHVa1hp8vXKvVnSyIyg"}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you know what @narenderpal I forgot to commit the token line hahaha, i'd done exactly that and forgot the commit hahaha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I wrote the tests i'd have caught that too ;) i'll work on adding test coverage soon.
Good point, let me update today tomorrow!
On 25 Jan 2018, at 18:09, Narender Pal <notifications@github.com<mailto:notifications@github.com>> wrote:
@narenderpal commented on this pull request.
________________________________
In lib/Teams.js<#1230 (comment)>:
if (err) {
// this is a fatal error - could not create a Teams API client
throw new Error(err);
}
- });
-
+ //use the expires_in value, convert to ms and remove 10 mins in ms
+ let tokenExpiresIn = 1000 * 60 * configuration.token_expires_in;
In case configuration.token_expires_in is not set (undefined) , we should have default value as 60 minutes?
let tokenExpiresIn = 1000 * 60 * (configuration.token_expires_in || 60);
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#1230 (review)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAYF3OHGaktcMCaeyagG2t5f2bK1874lks5tOMNNgaJpZM4Rs_W_>.
|
lib/Teams.js
Outdated
}); | ||
|
||
//use the expires_in value, convert to ms and remove 10 mins in ms | ||
let tokenExpiresIn = 1000 * 60 * (configuration.token_expires_in || 60); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expires_in unit is seconds, for example the value "3600" will denote that token will expire in one hour (60 * 60).
You may have to change the above line and update tests
let tokenExpiresIn = 1000 * (configuration.token_expires_in || 3600)
Example of json
token - {"token_type":"Bearer","expires_in":3599,"ext_expires_in":0,"access_token":" eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsIng1dCI6IjJLVmN1enFBaWRPTHFXU2FvbDd3Z0ZSR0NZbyIsImtpZCI6IjJLVmN1enFBaWRPTHFXU2FvbDd3Z0ZSR0NZbyJ9.eyJhdWQiOiJodHRwczovL2FwaS5ib3RmcmFtZXdvcmsuY29tIiwiaXNzIjoiaHR0cHM6Ly9zdHMud2luZG93cy5uZXQvZDZkNDk0MjAtZjM5Yi00ZGY3LWExZGMtZDU5YTkzNTg3MWRiLyIsImlhdCI6MTUxMTc1OTExNCwibmJmIjoxNTExNzU5MTE0LCJleHAiOjE1MTE3NjMwMTQsImFpbyI6IlkyTmdZSWhZdVBmYnQ1cUhWd0tuL2JCNDE3ejhDd0E9IiwiYXBwaWQiOiI1ZjVlMDAyMS1lNTNjLTQ1NDktOWE1NC0wNDMyNDAwM2EyM2YiLCJhcHBpZGFjciI6IjEiLCJpZHAiOiJodHRwczovL3N0cy53aW5kb3dzLm5ldC9kNmQ0OTQyMC1mMzliLTRkZjctYTFkYy1kNTlhOTM1ODcxZGIvIiwidGlkIjoiZDZkNDk0MjAtZjM5Yi00ZGY3LWExZGMtZDU5YTkzNTg3MWRiIiwidXRpIjoiLTk4UE1scU5xay1LeWxTaGdTMWFBQSIsInZlciI6IjEuMCJ9.IG_qHoN9WIZDH2hZCGDKW3EYl_7z8G2VwgxXHGWQR_9ZmHAMIUVM8uxqYR8cJTCRwgWOBXSMKS4yKGM8jdvchevJf0xndhvx_iocFun3_TBCWOzIpBWW_vUScNv1W70iM-CcXYvXo9CFHnsM_w7S8DMVov-_p0QEj-U-IHU_sls5hFIDaqpTZtqG_h6ZaK7KmIJtPFcxSM2uf8tUmWL7bdFPTBUzo-rM4yuzV3SewrzWl_EiXccnSEfi3fq5QWnWElwBXNt9VIQ2FqLmob2YHa3B5JLrd7vCtcJGSKZ6yUSphRGTyfq75grwdGrZ1lfPUWfzHVa1hp8vXKvVnSyIyg"}
__test__/lib/Teams.test.js
Outdated
return { | ||
getToken: jest.fn((cb) => { | ||
configuration.token = 'token'; | ||
configuration.token_expires_in = '20'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configuration.token_expires_in value will be in seconds
https://tools.ietf.org/html/rfc6749#section-4.2.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes looks good
@benbrown @peterswimm can this go in? |
this will be included in 0.6.9 |
Awesome! Thanks Ben!
On 29 Jan 2018, at 18:13, Ben Brown <notifications@github.com<mailto:notifications@github.com>> wrote:
this will be included in 0.6.9
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#1230 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAYF3EhOoOAzsMYWFe3MKCjOHQ-OD6Q_ks5tPgoygaJpZM4Rs_W_>.
|
pull in the expires_in value supplied with the token and setup call before the expire to get a new token.