search

microsoftgraph / aspnetcore-connect-sample

[ARCHIVED] This ASP.NET Core MVC sample shows how to connect to Microsoft Graph using delegated permissions and the Azure AD v2.0 (MSAL) endpoint.
MIT License
123 stars 96 forks source link

Transient creation of GraphServiceClient #55

Closed eluchsinger closed 4 years ago

eluchsinger commented 4 years ago

Bug description

Maybe I'm misreading something, but the GraphSdkHelper violates the first paragraph of the documentation on how to create a GraphServiceClient.

The documentation states the following:

You can use a single client instance for the lifetime of the application.

However the HomeController seems to create a new GraphServiceClient with every call to the Index route.

Additional context

Might be related to the issue https://github.com/microsoftgraph/aspnetcore-connect-sample/issues/4 .

mark-szabo commented 4 years ago

Thanks @eluchsinger! I'll migrate to .NET Core 3.1 and refactor this sample in about two weeks time. and will have a look on this. Will keep this issue updated! 🙂

mark-szabo commented 4 years ago

There's a bit of latency, but hopefully my comment will help others later.

So the story here is, that you need to get new tokens for every request as the webserver has no idea which user is actually sending the request. So imagine a situation where user A and B are both using the webapp simultaneously. If A signs in first and we store the authenticated Graph client, when B tries to request some data from Graph, the request will go in the name of A, which is obviously not what we want. So our best option here is to create a newly authenticated Graph client for every request. If you are writing an app where Graph is always accessed through a pre-authenticated client (eg. service client, deamon app, etc.), you can absolutely store the Graph client as a singleton.