Skip to content
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

Confusion with @microsoft/microsoft-graph-client #479

Closed
tboulis opened this issue Jan 23, 2024 · 12 comments
Closed

Confusion with @microsoft/microsoft-graph-client #479

tboulis opened this issue Jan 23, 2024 · 12 comments

Comments

@tboulis
Copy link

tboulis commented Jan 23, 2024

Your NPM page is https://www.npmjs.com/package/@microsoft/msgraph-sdk-JAVASCRIPT
Your github page URL is https://github.com/microsoftgraph/msgraph-sdk-TYPESCRIPT
Your github repo name is microsoftgraph/msgraph-sdk-TYPESCRIPT.

And your documentation about setting up an Authentication provider points to this Microsoft docs page which references a completely different package, for which the:

NPM page is https://www.npmjs.com/package/@microsoft/microsoft-graph-client
Github page is https://github.com/microsoftgraph/msgraph-sdk-JAVASCRIPT
Github repo is microsoftgraph/msgraph-sdk-JAVASCRIPT

Im trying use to your app with our msal.ConfidentialClientApplication but with no luck, the authProviders have different types (callback for @microsoft/microsoft-graph-client and class for your app)

I hope you can see why I am confused 😅

@baywet
Copy link
Member

baywet commented Jan 29, 2024

Hi @tboulis
Thanks for trying this SDK and for reaching out.
We're still in very early preview days, and trying to figure out the best approach here.
So far, here are our plans.

package name repo name notes
@microsoft/microsoft-graph-client microsoftgraph/msgraph-sdk-javascript the historical and legacy client, will be replaced over time by this new set of packages, the repository name will be renamed as well to free up the name
@microsoft/msgraph-sdk-javascript this repo npm package is deprecated, older attempt at delivering a fluent API
@microsoft/msgraph-sdk-core microsoftgraph/msgraph-sdk-typescript-core core functionality shared by the fluent API packages
@microsoft/msgraph-sdk this repo base (empty) client, and all the models for the v1 API
@microsoft/msgraph-beta-sdk microsoftgraph/msgraph-beta-sdk-typescript base (empty) client, and all the models for the beta API
@microsoft/msgraph-sdk-users (and many others) this repo v1 fluent API for all APIs under /users
@microsoft/msgraph-beta-sdk-users (and many others) microsoftgraph/msgraph-beta-sdk-typescript beta fluent API for all APIs under /users

The idea currently being:

  1. install @microsoft/msgraph-sdk this will get you the models and the an empty client.
  2. install @microsoft/msgraph-sdk-users (or another one), this will add the fluent API for all operations under /users.
  3. start writing your application

Over time, we'll most likely rename the repositories from microsoft/msgraph-sdk-typescript to microsoft/msgraph-sdk-javascript to reduce confusion, but this will only ever happen when the work here is ready for general availability.

I hope this clarifies the current state of things and future plans.
Let us know if you have questions/feedback.

@tboulis
Copy link
Author

tboulis commented Jan 29, 2024

That's awesome @baywet , thank you clarifying this 🙏

@tboulis tboulis closed this as completed Jan 29, 2024
@1oglop1
Copy link

1oglop1 commented Jul 23, 2024

@baywet Could you please pin this issue or put the table in the repo README? I'm wasting a lot of time figuring out where I am supposed to get the Client object.

When all I want is to call graph API with my user.

  const tokenCredential = identity.getDefaultAzureCredential();

  const options: TokenCredentialAuthenticationProviderOptions = {
    scopes: [
      // '.default',
      "https://graph.microsoft.com/.default",
    ],
    getTokenOptions: {},
  };

  // Create an instance of the TokenCredentialAuthenticationProvider by passing the tokenCredential instance and options to the constructor
  const authProvider = new TokenCredentialAuthenticationProvider(tokenCredential, options);

  const requestAdapter = new FetchRequestAdapter(authProvider); // Type mismatch - likely due to legacy @microsoft/microsoft-graph-client
  const graphClient = Client.initWithMiddleware({ authProvider: authProvider });

  const jane = await graphServiceClient.users.byUserId("[email protected]").get();
  

the code here imports some Client but what it is. Nobody knows.

The API I am trying to call is https://graph.microsoft.com/beta/servicePrincipals/" + $servicePrincipalId + "/synchronization/jobs" but I am just unable to understand the order of this SDK to download the right one.

@baywet
Copy link
Member

baywet commented Jul 23, 2024

@1oglop1 where did you get this client initialization code from? You shouldn't need the Client.InitWith... anymore.
See this documentation

Also, if you want to call beta endpoints, you need to install the beta SDK instead documentation

@1oglop1
Copy link

1oglop1 commented Jul 24, 2024

@baywet
It is not just about beta endpoints.

I tried to view the documentation, but this is my main beef with Microsoft docs, one page with incomplete examples leads to another page with incomplete examples.
It would be way easier if there were complete executable examples.

Example for this case:

this documentation ( link from )

For an example of how to get an authentication provider, see [choose a Microsoft Graph authentication provider](https://docs.microsoft.com/graph/sdks/choose-authentication-providers?tabs=typescript).

Shows incomplete an example and delegates the creation of authProvider to another page

import { FetchRequestAdapter } from "@microsoft/kiota-http-fetchlibrary";
import { createGraphServiceClient } from "@microsoft/msgraph-sdk";
import "@microsoft/msgraph-sdk-users";

const requestAdapter = new FetchRequestAdapter(authProvider);
const graphServiceClient = createGraphServiceClient(requestAdapter);

const jane = await graphServiceClient.users.byUserId("[email protected]").get();

When the user clicks on the link choose a Microsoft Graph authentication provider.
They are redirected to the page with another incomplete example, that contains an incompatible type with this SDK.

// @azure/identity
const credential = new InteractiveBrowserCredential({
  tenantId: 'YOUR_TENANT_ID',
  clientId: 'YOUR_CLIENT_ID',
  redirectUri: 'http://localhost',
});

// @microsoft/microsoft-graph-client/authProviders/azureTokenCredentials
const authProvider = new TokenCredentialAuthenticationProvider(credential, {
  scopes: ['User.Read'],
});

const graphClient = Client.initWithMiddleware({ authProvider: authProvider });

As a complete newcomer to AzureSDKs it took me several hours to put the pieces together to authenticate as "me" just by running az login to get this working.

Working code example (click to unfold)
import * as identity from "@azure/identity";
import * as gc from "@microsoft/microsoft-graph-client";
import {
  TokenCredentialAuthenticationProvider,
  TokenCredentialAuthenticationProviderOptions,
} from "@microsoft/microsoft-graph-client/authProviders/azureTokenCredentials";

(async () => {

  const tokenCredential = identity.getDefaultAzureCredential();

  const options: TokenCredentialAuthenticationProviderOptions = {
    scopes: [
      // '.default',
      "https://graph.microsoft.com/.default",
    ],
    getTokenOptions: {},
  };
  const authProvider = new TokenCredentialAuthenticationProvider(tokenCredential, options);
  const client = gc.Client.initWithMiddleware({
    authProvider: authProvider,
  });

  const subscriptionId = "1234-uuid"; // azure subscription id
  const resourceGroup = "DefaultResourceGroup-WEU";
  const partnerTopic = "msGraphGroups2";
  const location = "westeurope";

  const resp = await client.api('/subscriptions')
  .get();
  console.log(resp);
})();

When I tried using this SDK, I got just as confused as I was at the start because the authProvider type needed by the requestAdapter doesn't match what's in the docs.

Say I overcome the authentication problem, try this (new) SDK. Here's my resolution process:

  1. read the docs for the synchronization https://learn.microsoft.com/en-us/graph/api/synchronization-synchronization-list-jobs?view=graph-rest-1.0&tabs=http
  2. Find the url the client has to execute GET /servicePrincipals/{id}/synchronization/jobs/
  3. find the correct packages msgraph-sdk-*

Now step 3. is also slightly confusing due to API structure. Based on my experience with other SDKs I'd be looking for the SDK named msgraph-sdk-servicePrincipals, because that's the resource the endpoint starts with.
However, this one does not contain the call to the correct endpoint. And somehow I need to find out that it is the msgraph-sdk-applications I need. - the question is where do I learn this fact if there isn't a complete executable example?

IMO the authors of the docs assumed a higher level of familiarity with Microsoft APIs which is often not the case and I missed several clues which caused me a cognitive overhead.

Minor rants:

  • The implicit behaviour of import "@microsoft/msgraph-sdk-users"; which injects the code in the main client.
  • Forcing the sdk user to do null checks where an error would be more appropriate - I have never encountered where get_by_id would return undefined instead of HTTP (404) error.
  const requestAdapter = new FetchRequestAdapter(authProvider); // Type mismatch - likely due to legacy @microsoft/microsoft-graph-client
  const graphClient = createGraphServiceClient(requestAdapter);
  const appRequest = graphClient.applications.byApplicationId("");
  const result = await appRequest.get()  // why is this undefined instead of error?

I hope this write-up shows the missing cognitive links, please let me know if there is anything difficult to understand (English is not my first language). This situation evokes 15 standards (xkcd) feelings and I am a bit frustrated by the inability to do a seemingly simple thing.

@baywet
Copy link
Member

baywet commented Jul 25, 2024

Thank you so much for this amazing feedback!

Yes I can see where the confusion comes from here.

the documentation Shows incomplete an example and delegates the creation of authProvider to another page

This links points at samples for the old SDK, I think we put the link here assuming the samples would get updated upon general availability, but until then it creates more confusion than anything. @jasonjoh do you think it'd be possible to add a set of "TypeScript (Preview)" tabs on this page and link to that instead from here? (or rename the current TypeScript tab to javascript to be consistent with the rest of the documentation and add a new TypeScript tab)

When I tried using this SDK, I got just as confused as I was at the start because the authProvider

Yes the mix between the two generations of SDKs created more confusion than anything. We changed the approach from the initial previews of this new SDK. And they are now entirely separate products to avoid any further confusion. Sorry the auth docs lead you back to the older SDK.

Say I overcome the authentication problem, try this (new) SDK. Here's my resolution process

Yes the operations docs only have a JavaScript tab at this moment, snippets for this new TypeScript preview SDK is still work to be done. microsoftgraph/microsoft-graph-devx-api#1939

Based on my experience with other SDKs I'd be looking for the SDK named msgraph-sdk-servicePrincipals, because that's the resource the endpoint starts with.
However, this one does not contain the call to the correct endpoint. And somehow I need to find out that it is the msgraph-sdk-applications I need. - the question is where do I learn this fact if there isn't a complete executable example?

I'm not sure I understand why you need applications package for an operation under service principals at this stage?

const appRequest = graphClient.applications.byApplicationId("");
const result = await appRequest.get() // why is this undefined instead of error?

This is most likely a bug, the SDK should have complained about passing an empty string to begin with. Can you file a separate issue for that please?

Note: the error probably comes from a lack of validation here

@jasonjoh
Copy link
Member

I'm concerned that will introduce more confusion by using different SDKs across JS/TS. We've also adopted the policy that our docs will reference GA versions of SDKs only. During preview, it might make sense to remove the link back to our docs using the GA SDK and just put some inline examples of a few of the more common auth providers to help folks get started.

@baywet
Copy link
Member

baywet commented Jul 25, 2024

Thanks for your input @jasonjoh
@1oglop1 can you create another additional issue for the readme to remove the link and add the additional snippets here for authentication please?

@baywet
Copy link
Member

baywet commented Jul 25, 2024

Thanks for creating the first issue @1oglop1
Can you also create one for that please?

const appRequest = graphClient.applications.byApplicationId("");
const result = await appRequest.get() // why is this undefined instead of error?

This is most likely a bug, the SDK should have complained about passing an empty string to begin with.
Note: the error probably comes from a lack of validation here

@1oglop1
Copy link

1oglop1 commented Jul 29, 2024

Sorry for my late reply, I noticed I did not hit the button 😮‍💨 🌴

I'm not sure I understand why you need applications package for an operation under service principals at this stage?

Yes, that is exactly my point, I should not need applications package IMO. However, I do not think there is much we can do about it.

The endpoint GET /servicePrincipals/{id}/synchronization/jobs/ is represented as applicatons ->

export interface RestartPostRequestBody extends AdditionalDataHolder, BackedModel, Parsable {

But when you look at the REST API documentation it follows the structure from here:
https://learn.microsoft.com/en-us/graph/api/synchronization-synchronizationjob-provisionondemand?view=graph-rest-1.0&tabs=http
image


This is most likely a bug, the SDK should have complained about passing an empty string to begin with. Can you file a separate issue for that please?
Note: the error probably comes from a lack of validation here

I would not worry about that, I've never executed the code, only type completion so that I can explore the API via using the client instead of searching docs.


@baywet thank you for #628! 🙌
This was the missing link, now I should be able to use the managed identity and this

// @azure/identity
const credential = identity.getDefaultAzureCredential();
// @microsoft/kiota-authentication-azure
const authProvider = new AzureIdentityAuthenticationProvider(credential, ["https://graph.microsoft.com/.default"]);

@baywet
Copy link
Member

baywet commented Jul 29, 2024

Thank you for the additional information .
Yes, the table of content for the documentation can be a bit confusing at times as it doesn't map exactly with the structure of the API.
This is outside of the scope of this repository though.
I'm glad that the additional documentation for the authentication providers has helped you solve your issue.
I still think that we need to fix the input validation issue I talked about.
Is this something that you want to follow up on or should I do it on my own?

@1oglop1
Copy link

1oglop1 commented Jul 30, 2024

@baywet
I was confident that the input validation was not a problem so dug deeper into the SDK and created #632.
Let's continue the discussion there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants