search

pnp / PnP-Sites-Core

Microsoft 365 Dev PnP Core component (.NET) targeted for increasing developer productivity with CSOM based solutions.
Other
416 stars 643 forks source link

static declaration _authContext in AuthenticationManager #2133

Open mdissel opened 5 years ago

mdissel commented 5 years ago

Which PnP repository should you use to report the issue?

Category

[X] Question [ ] Bug [ ] Enhancement

Environment

[ ] Office 365 / SharePoint Online [ ] SharePoint 2016 [ ] SharePoint 2013

Expected or Desired Behavior

Why is this variable declared as static in the AuthenticationManager class?

private static AuthenticationContext _authContext = null;

https://github.com/SharePoint/PnP-Sites-Core/blob/master/Core/OfficeDevPnP.Core/AuthenticationManager.cs#L54

jansenbe commented 5 years ago

What is the problem you're trying to solve?

mdissel commented 5 years ago

We're connecting within the same appdomain with multiple tenants. A static variable could become a potential hard-to-find bug when the same authContext is re-used with multiple different tenants (in multiple concurrent threads!).

jansenbe commented 5 years ago

I would strongly suggest against having one AuthenticationManager instance being used across tenants...this will lead to other issues, especially in app-only scenarios. What I typically do is create the needed instance on the fly or keep track of the created AuthenticationManager instances and grab the one I need

mdissel commented 5 years ago

that's exactly the reason for my question. Why define this variable as static?