Use AAD Token for Authentication #362
Conversation
| @@ -0,0 +1,33 @@ | |||
| <!-- | |||
| @@ -0,0 +1,9 @@ | |||
| # Microsoft Open Source Code of Conduct | |||
| @@ -0,0 +1,45 @@ | |||
| ## Purpose | |||
samples/acquire-token/.gitignore
Outdated
| @@ -0,0 +1,349 @@ | |||
| ## Ignore Visual Studio temporary files, build results, and | |||
|
|
||
| ### Pre-requisites | ||
|
|
||
| 1. Open PowerShell (On Windows, press `Windows-R` and type `PowerShell` in the search window) |
There was a problem hiding this comment.
this only worked on Windows? What about other platforms?
| @@ -0,0 +1,76 @@ | |||
| # Contributing to [project-title] | |||
src/directLine.ts
Outdated
| @@ -558,9 +564,12 @@ export class DirectLine implements IBotConnection { | |||
| } else { | |||
| return this.startConversation().do(conversation => { | |||
| this.conversationId = conversation.conversationId; | |||
| this.token = this.secret || conversation.token; | |||
| // Don't invoke acquireToken(), coulf fetch anew AAD token | |||
src/directLine.ts
Outdated
| @@ -664,7 +675,7 @@ export class DirectLine implements IBotConnection { | |||
| } | |||
|
|
|||
| private refreshTokenLoop() { | |||
| this.tokenRefreshSubscription = Observable.interval(intervalRefreshToken, this.services.scheduler) | |||
| this.tokenRefreshSubscription = Observable.interval(3000, this.services.scheduler) | |||
| @@ -558,9 +564,12 @@ export class DirectLine implements IBotConnection { | |||
| } else { | |||
| return this.startConversation().do(conversation => { | |||
| this.conversationId = conversation.conversationId; | |||
| this.token = this.secret || conversation.token; | |||
| // Don't invoke acquireToken(), coulf fetch anew AAD token | |||
| this.token = (this.acquireToken ? this.token : this.secret) || conversation.token; | |||
There was a problem hiding this comment.
Why are we setting the secret as the token ever? This isn't a practice we should follow, right?
There was a problem hiding this comment.
Yeah, that doesn't look good. However, ABS API expects either a token or secret in the Authentication header. Maybe we should use a variable name that suits both.
| this.secret = options.secret; | ||
| this.token = options.secret || options.token; | ||
| this.token = this.acquireToken ? this.acquireToken() : options.secret || options.token; |
There was a problem hiding this comment.
I'm concerned about the async nature of this function. What if the caller needs to do some HTTP call or database operation? I'm not sure this will work.
There was a problem hiding this comment.
The client calls webchat after fetching the token atleast once. The WebSite code logic keeps refreshing the token. Yes, that code is async. Sample is in this doc.
There was a problem hiding this comment.
this code doesn't look to be async, is what I mean. Can you write a test that exercises this to demonstrate the behavior you mention?
A new policy, DisableLocalAuth, was added to Azure Bot Service to enable bot resource owners to enforce bots in particular subscriptions to explicitly use AAD token instead of secret.
This PR is to update DirectLine client to be able to support using AAD token.
Follow this doc if you would like to test the feature.