msgraph-sdk-python-core icon indicating copy to clipboard operation
msgraph-sdk-python-core copied to clipboard

Multiple GraphClient instantiations overwriting scopes

Open stephenleeoyo opened this issue 3 years ago • 0 comments

I am writing a custom MS Graph API wrapper package that follows the principle of least privilege, where each module in my package instantiates a GraphClient with its own necessary scopes needed to function properly. I discovered that the GraphClient's __new__ method is preventing two separate modules from each instantiating a GraphClient with their own scopes - rather, they are forced to share the same instance, with the second set of instantiated scopes taking precedence.

To circumvent this, I have been able to dig deep and blend the scopes manually across instantiations, but it's a rather ugly line of code hitting a bunch of protected variables, and I have no certainty the object structure won't change going forward:

class MyClass:
    def __init__(self, credential, **kwargs):
        if GraphClient._GraphClient__instance is not None and 'scopes' in kwargs:
            old = GraphClient._GraphClient__instance.graph_session.adapters.get('https://')._first_middleware.scopes
            kwargs['scopes'] = list(set(kwargs['scopes'] + old))
        self.client = GraphClient(credential=credential, **kwargs)

I don't mind the usage of the shared GraphClient instances, but perhaps providing a built-in option to persist the existing scopes would be a much cleaner solution.

To replicate my example:

from msgraph.core import GraphClient
from azure.identity import DefaultAzureCredential

dac = DefaultAzureCredential()

gc1 = GraphClient(credential=dac, scopes=['user.read'])
gc2 = GraphClient(credential=dac, scopes=['files.readwrite.all'])

print(gc1.graph_session.adapters.get("https://")._first_middleware.scopes)
print(gc2.graph_session.adapters.get("https://")._first_middleware.scopes)

returns

['files.readwrite.all']
['files.readwrite.all']

Note that the user.read scope provided to gc1 has been overwritten.

stephenleeoyo avatar Mar 29 '22 17:03 stephenleeoyo