microsoftgraph / msgraph-sdk-javascript

Microsoft Graph client library for JavaScript
https://graph.microsoft.com
MIT License
745 stars 225 forks source link

setup default valid hosts for graph in graph core #1139

Open baywet opened 1 year ago

baywet commented 1 year ago

related to https://github.com/microsoft/kiota/issues/1063 related to https://github.com/microsoft/kiota-typescript/pull/505

The original value for the default valid hostnames was removed from kiota authentication libraries to make them API agnostic and easier to use with other APIs.

Since the initialization of the client is a bit different here compared with other languages, I'm opening this issue to discuss the best approach to set those default values in graph core. In other languages the approach was to make derived authentication and access token providers that would set the default.

Original default values:

new Set<string>([
      "graph.microsoft.com",
      "graph.microsoft.us",
      "dod-graph.microsoft.us",
      "graph.microsoft.de",
      "microsoftgraph.chinacloudapi.cn",
      "canary.graph.microsoft.com",
    ]
nikithauc commented 1 year ago

We already have this

export const GRAPH_URLS = new Set<string>(["graph.microsoft.com", "graph.microsoft.us", "dod-graph.microsoft.us", "graph.microsoft.de", "microsoftgraph.chinacloudapi.cn", "canary.graph.microsoft.com"]);

https://github.com/microsoftgraph/msgraph-sdk-javascript/blob/597accef46f205e5bbfc27ee3cac9ceb5cefd592/src/Constants.ts#L28

nikithauc commented 1 year ago

Currently this constant set is used to append or remove an auth header and telemetry header.

baywet commented 1 year ago

Thanks for the information. How do you suggest we inject that value in the authentication providers (spfx, az identity,...) ? derive and overload or some other approach?

nikithauc commented 1 year ago

https://github.com/microsoftgraph/msgraph-sdk-javascript/blob/65310a885e08a33c77d62279340a64f40d38150c/src/GraphRequestUtil.ts#L132

Passing it at the time of initialization client instance.

baywet commented 1 year ago

oh so it's already being done? perfect then. If you could just confirm and close the issue please.