Conversation
baywet
left a comment
There was a problem hiding this comment.
thanks for starting this! It's great to see things starting to line up! Lots of work ahead but it's starting to take shape! Two minor comments on my side.
There was a problem hiding this comment.
What's the value of this interface since we already have access token provider in kiota? (or will have soon at least)
There was a problem hiding this comment.
note, should probably be using the allowed host validator and check for https
| * @returns The promise that resolves to an access token | ||
| */ | ||
| public async getAuthorizationToken(url: string): Promise<string> { | ||
| if (!url || !this.allowedHostsValidator.isUrlHostValid(url)) { |
src/GraphRequest.ts
Outdated
There was a problem hiding this comment.
assuming we're manipulating a BaseBearerTokenAuthenticationProvider and this provides a property for the access token provider, you shouldn't have to instantiate a request information at all for arbitrary requests
There was a problem hiding this comment.
param name and description needs to be updated
| const scopes = ["User.Read", "Mail.Send"]; | ||
|
|
||
| const graphClient = Client.init({ | ||
| authenticationTokenProvider: new AzureIdentityAuthenticationProvider(deviceCodeCredentials, scopes), |
There was a problem hiding this comment.
| authenticationTokenProvider: new AzureIdentityAuthenticationProvider(deviceCodeCredentials, scopes), | |
| authenticationProvider: new AzureIdentityAuthenticationProvider(deviceCodeCredentials, scopes), |
|
|
||
| ```Shell | ||
| npm install @microsoft/msgraph-sdk-javascript-core --save | ||
| npm install @microsoft/msgraph-sdk-javascript-types --save-dev |
There was a problem hiding this comment.
are we still going to chip the type separately at this point?
There was a problem hiding this comment.
My feeling tells me a core user should have access to the Types. Especially with the work we are doing with the api() method that would reuse the same pattern than the rest of our chained factories. I think a user should be able to benefit from our types without having to download all of our package.
| const scopes = ["User.Read", "Mail.Send"]; | ||
|
|
||
| const graphClient = Client.init({ | ||
| authenticationTokenProvider: new AzureIdentityAuthenticationProvider(deviceCodeCredentials, scopes), |
There was a problem hiding this comment.
| authenticationTokenProvider: new AzureIdentityAuthenticationProvider(deviceCodeCredentials, scopes), | |
| authenticationProvider: new AzureIdentityAuthenticationProvider(deviceCodeCredentials, scopes), |
|
isn't this one superseded by #712 at this point? lots of duplicate changes |
|
The changes here are contains in #712 |
This PR is WIP.
client.api("")request using Kiota Authentication.Graph.IAuthenticationProviderand modified code to align withKiota.IAuthenticationProviderandKiota.IAccessTokenProvider.SimpleAuthenticationProvideras suggested by @sebastienlevert aligning with MGT SimpleProvider. We could move this to Kiota too.https://github.com/microsoftgraph/microsoft-graph-toolkit/blob/ec1a5bf8cbe0f76f4aea9da8ad425b144f5c6572/packages/mgt-element/src/providers/SimpleProvider.ts#L19
fixes #536