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

The cached _graphClient instance in GraphSDKHelper is not used #4

Closed John-Garland closed 6 years ago

John-Garland commented 7 years ago

The _graphClient field in GraphSDK helper appears to be there in order to cache the reference between executions of the GetAuthenticatedClient method. Except that the private field is only ever referenced from this function, and is generated new for every single function call. Either the call needs to be changed to check it against null and then create the instance if null, or the variable can be moved from a field to a local function variable.

jamescro commented 7 years ago

Thanks for spotting this. That field is a vestige of an earlier version of the sample, and we're no longer following that pattern. We'll remove it in the next week or two.

mark-szabo commented 6 years ago

Hi @dotnetgator, in theory this should have fixed too with the new MSAL implementation. Please have a look and reopen if you find the issue valid.