Introducing Kiota Authentication #590
Conversation
baywet
left a comment
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.
| * @callback - The anonymous callback function which takes a single param | ||
| */ | ||
| export type AuthProvider = (done: AuthProviderCallback) => void; | ||
| export interface AuthProviderCallback { |
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)
| * To get the access token | ||
| * @returns The promise that resolves to an access token | ||
| */ | ||
| public getAuthorizationToken(): Promise<string> { |
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
| this.updateRequestOptions(options); | ||
| const customHosts = this.config?.customHosts; | ||
| const customHosts = this.config?.customHosts; | ||
| const requestInfo = new RequestInformation(); |
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
| * @public | ||
| * @constructor | ||
| * Creates an instance of AuthenticationHandler | ||
| * @param {AuthenticationProvider} authenticationProvider - The authentication provider for the authentication handler |
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