Merge pull request #110 from AzureAD/persistence-factory

Persistence factory

Author: rayluo

msal_extensions/__init__.py

@@ -1,20 +1,13 @@
 """Provides auxiliary functionality to the `msal` package."""
 __version__ = "0.3.1"
 
-import sys
-
 from .persistence import (
     FilePersistence,
+    build_encrypted_persistence,
     FilePersistenceWithDataProtection,
     KeychainPersistence,
     LibsecretPersistence,
     )
 from .cache_lock import CrossPlatLock
 from .token_cache import PersistedTokenCache
 
-if sys.platform.startswith('win'):
-    from .token_cache import WindowsTokenCache as TokenCache
-elif sys.platform.startswith('darwin'):
-    from .token_cache import OSXTokenCache as TokenCache
-else:
-    from .token_cache import FileTokenCache as TokenCache

msal_extensions/persistence.py

@@ -83,6 +83,22 @@ class PersistenceDecryptionError(PersistenceError):
     """This could be raised by persistence.load()"""
 
 
+def build_encrypted_persistence(location):
+    """Build a suitable encrypted persistence instance based your current OS.
+
+    If you do not need encryption, then simply use ``FilePersistence`` constructor.
+    """
+    # Does not (yet?) support fallback_to_plaintext flag,
+    # because the persistence on Windows and macOS do not support built-in trial_run().
+    if sys.platform.startswith('win'):
+        return FilePersistenceWithDataProtection(location)
+    if sys.platform.startswith('darwin'):
+        return KeychainPersistence(location)
+    if sys.platform.startswith('linux'):
+        return LibsecretPersistence(location)
+    raise RuntimeError("Unsupported platform: {}".format(sys.platform))  # pylint: disable=consider-using-f-string
+
+
 class BasePersistence(ABC):
     """An abstract persistence defining the common interface of this family"""
 

msal_extensions/token_cache.py

@@ -1,15 +1,12 @@
 """Generic functions and types for working with a TokenCache that is not platform specific."""
 import os
-import warnings
 import time
 import logging
 
 import msal
 
 from .cache_lock import CrossPlatLock
-from .persistence import (
-    _mkdir_p, PersistenceNotFound, FilePersistence,
-    FilePersistenceWithDataProtection, KeychainPersistence)
+from .persistence import _mkdir_p, PersistenceNotFound
 
 
 logger = logging.getLogger(__name__)
@@ -89,35 +86,3 @@ def find(self, credential_type, **kwargs):  # pylint: disable=arguments-differ
                 return super(PersistedTokenCache, self).find(credential_type, **kwargs)
         return []  # Not really reachable here. Just to keep pylint happy.
 
-
-class FileTokenCache(PersistedTokenCache):
-    """A token cache which uses plain text file to store your tokens."""
-    def __init__(self, cache_location, **ignored):  # pylint: disable=unused-argument
-        warnings.warn("You are using an unprotected token cache", RuntimeWarning)
-        warnings.warn("Use PersistedTokenCache(...) instead", DeprecationWarning)
-        super(FileTokenCache, self).__init__(FilePersistence(cache_location))
-
-UnencryptedTokenCache = FileTokenCache  # For backward compatibility
-
-
-class WindowsTokenCache(PersistedTokenCache):
-    """A token cache which uses Windows DPAPI to encrypt your tokens."""
-    def __init__(
-            self, cache_location, entropy='',
-            **ignored):  # pylint: disable=unused-argument
-        warnings.warn("Use PersistedTokenCache(...) instead", DeprecationWarning)
-        super(WindowsTokenCache, self).__init__(
-            FilePersistenceWithDataProtection(cache_location, entropy=entropy))
-
-
-class OSXTokenCache(PersistedTokenCache):
-    """A token cache which uses native Keychain libraries to encrypt your tokens."""
-    def __init__(self,
-                 cache_location,
-                 service_name='Microsoft.Developer.IdentityService',
-                 account_name='MSALCache',
-                 **ignored):  # pylint: disable=unused-argument
-        warnings.warn("Use PersistedTokenCache(...) instead", DeprecationWarning)
-        super(OSXTokenCache, self).__init__(
-            KeychainPersistence(cache_location, service_name, account_name))
-

sample/persistence_sample.py

@@ -1,32 +1,21 @@
-import sys
 import logging
 import json
 
-from msal_extensions import *
+from msal_extensions import build_encrypted_persistence, FilePersistence, CrossPlatLock
 
 
 def build_persistence(location, fallback_to_plaintext=False):
     """Build a suitable persistence instance based your current OS"""
-    if sys.platform.startswith('win'):
-        return FilePersistenceWithDataProtection(location)
-    if sys.platform.startswith('darwin'):
-        return KeychainPersistence(location)
-    if sys.platform.startswith('linux'):
-        try:
-            return LibsecretPersistence(
-                # By using same location as the fall back option below,
-                # this would override the unencrypted data stored by the
-                # fall back option.  It is probably OK, or even desirable
-                # (in order to aggressively wipe out plain-text persisted data),
-                # unless there would frequently be a desktop session and
-                # a remote ssh session being active simultaneously.
-                location,
-                )
-        except:  # pylint: disable=bare-except
-            if not fallback_to_plaintext:
-                raise
-            logging.warning("Encryption unavailable. Opting in to plain text.")
-    return FilePersistence(location)
+    # Note: This sample stores both encrypted persistence and plaintext persistence
+    # into same location, therefore their data would likely override with each other.
+    try:
+        return build_encrypted_persistence(location)
+    except:  # pylint: disable=bare-except
+        # Known issue: Currently, only Linux
+        if not fallback_to_plaintext:
+            raise
+        logging.warning("Encryption unavailable. Opting in to plain text.")
+        return FilePersistence(location)
 
 persistence = build_persistence("storage.bin", fallback_to_plaintext=False)
 print("Type of persistence: {}".format(persistence.__class__.__name__))

sample/token_cache_sample.py

@@ -2,31 +2,21 @@
 import logging
 import json
 
-from msal_extensions import *
+from msal_extensions import build_encrypted_persistence, FilePersistence
 
 
 def build_persistence(location, fallback_to_plaintext=False):
     """Build a suitable persistence instance based your current OS"""
-    if sys.platform.startswith('win'):
-        return FilePersistenceWithDataProtection(location)
-    if sys.platform.startswith('darwin'):
-        return KeychainPersistence(location)
-    if sys.platform.startswith('linux'):
-        try:
-            return LibsecretPersistence(
-                # By using same location as the fall back option below,
-                # this would override the unencrypted data stored by the
-                # fall back option.  It is probably OK, or even desirable
-                # (in order to aggressively wipe out plain-text persisted data),
-                # unless there would frequently be a desktop session and
-                # a remote ssh session being active simultaneously.
-                location,
-                )
-        except:  # pylint: disable=bare-except
-            if not fallback_to_plaintext:
-                raise
-            logging.exception("Encryption unavailable. Opting in to plain text.")
-    return FilePersistence(location)
+    # Note: This sample stores both encrypted persistence and plaintext persistence
+    # into same location, therefore their data would likely override with each other.
+    try:
+        return build_encrypted_persistence(location)
+    except:  # pylint: disable=bare-except
+        # Known issue: Currently, only Linux
+        if not fallback_to_plaintext:
+            raise
+        logging.warning("Encryption unavailable. Opting in to plain text.")
+        return FilePersistence(location)
 
 persistence = build_persistence("token_cache.bin")
 print("Type of persistence: {}".format(persistence.__class__.__name__))

tests/test_agnostic_backend.py

@@ -34,18 +34,16 @@ def _test_token_cache_roundtrip(cache):
     assert token1['access_token'] == token2['access_token']
 
 def test_file_token_cache_roundtrip(temp_location):
-    from msal_extensions.token_cache import FileTokenCache
-    _test_token_cache_roundtrip(FileTokenCache(temp_location))
+    _test_token_cache_roundtrip(PersistedTokenCache(FilePersistence(temp_location)))
 
-def test_current_platform_cache_roundtrip_with_alias_class(temp_location):
-    from msal_extensions import TokenCache
-    _test_token_cache_roundtrip(TokenCache(temp_location))
+def test_current_platform_cache_roundtrip_with_persistence_builder(temp_location):
+    _test_token_cache_roundtrip(PersistedTokenCache(build_encrypted_persistence(temp_location)))
 
 def test_persisted_token_cache(temp_location):
     _test_token_cache_roundtrip(PersistedTokenCache(FilePersistence(temp_location)))
 
 def test_file_not_found_error_is_not_raised():
     persistence = FilePersistence('non_existing_file')
-    cache = PersistedTokenCache(persistence=persistence)
+    cache = PersistedTokenCache(persistence)
     # An exception raised here will fail the test case as it is supposed to be a NO-OP
     cache.find('')

tests/test_macos_backend.py

@@ -10,7 +10,8 @@
     pytest.skip('skipping MacOS-only tests', allow_module_level=True)
 else:
     from msal_extensions.osx import Keychain
-    from msal_extensions.token_cache import OSXTokenCache
+    from msal_extensions.token_cache import PersistedTokenCache
+    from msal_extensions.persistence import KeychainPersistence
 
 
 def test_keychain_roundtrip():
@@ -26,12 +27,12 @@ def test_osx_token_cache_roundtrip():
     client_id = os.getenv('AZURE_CLIENT_ID')
     client_secret = os.getenv('AZURE_CLIENT_SECRET')
     if not (client_id and client_secret):
-        pytest.skip('no credentials present to test OSXTokenCache round-trip with.')
+        pytest.skip('no credentials present to test PersistedTokenCache round-trip with.')
 
     test_folder = tempfile.mkdtemp(prefix="msal_extension_test_osx_token_cache_roundtrip")
     cache_file = os.path.join(test_folder, 'msal.cache')
     try:
-        subject = OSXTokenCache(cache_location=cache_file)
+        subject = PersistedTokenCache(KeychainPersistence(cache_file))
         app = msal.ConfidentialClientApplication(
             client_id=client_id,
             client_credential=client_secret,

tests/test_windows_backend.py

@@ -11,7 +11,8 @@
     pytest.skip('skipping windows-only tests', allow_module_level=True)
 else:
     from msal_extensions.windows import WindowsDataProtectionAgent
-    from msal_extensions.token_cache import WindowsTokenCache
+    from msal_extensions.token_cache import PersistedTokenCache
+    from msal_extensions.persistence import FilePersistenceWithDataProtection
 
 
 def test_dpapi_roundtrip_with_entropy():
@@ -48,8 +49,7 @@ def test_dpapi_roundtrip_with_entropy():
 
 def test_read_msal_cache_direct():
     """
-    This loads and unprotects an MSAL cache directly, only using the DataProtectionAgent. It is not meant to test the
-    wrapper `WindowsTokenCache`.
+    This loads and unprotects an MSAL cache directly, only using the DataProtectionAgent.
     """
     localappdata_location = os.getenv('LOCALAPPDATA', os.path.expanduser('~'))
     cache_locations = [
@@ -87,12 +87,12 @@ def test_windows_token_cache_roundtrip():
     client_id = os.getenv('AZURE_CLIENT_ID')
     client_secret = os.getenv('AZURE_CLIENT_SECRET')
     if not (client_id and client_secret):
-        pytest.skip('no credentials present to test WindowsTokenCache round-trip with.')
+        pytest.skip('no credentials present to test PersistedTokenCache round-trip with.')
 
     test_folder = tempfile.mkdtemp(prefix="msal_extension_test_windows_token_cache_roundtrip")
     cache_file = os.path.join(test_folder, 'msal.cache')
     try:
-        subject = WindowsTokenCache(cache_location=cache_file)
+        subject = PersistedTokenCache(FilePersistenceWithDataProtection(cache_file))
         app = msal.ConfidentialClientApplication(
             client_id=client_id,
             client_credential=client_secret,