Release 0.4.0

msal_extensions/__init__.py

@@ -1,5 +1,5 @@
 """Provides auxiliary functionality to the `msal` package."""
-__version__ = "0.3.1"
+__version__ = "0.4.0"
 
 import sys
 

msal_extensions/persistence.py

@@ -9,6 +9,7 @@
 import abc
 import os
 import errno
+import hashlib
 import logging
 import sys
 try:
@@ -50,21 +51,36 @@ def _mkdir_p(path):
         else:
             raise
 
+def _auto_hash(input_string):
+    return hashlib.sha256(input_string.encode('utf-8')).hexdigest()
+
 
 # We do not aim to wrap every os-specific exception.
-# Here we define only the most common one,
-# otherwise caller would need to catch os-specific persistence exceptions.
-class PersistenceNotFound(IOError):  # Use IOError rather than OSError as base,
+# Here we standardize only the most common ones,
+# otherwise caller would need to catch os-specific underlying exceptions.
+class PersistenceError(IOError):  # Use IOError rather than OSError as base,
+    """The base exception for persistence."""
         # because historically an IOError was bubbled up and expected.
         # https://github.com/AzureAD/microsoft-authentication-extensions-for-python/blob/0.2.2/msal_extensions/token_cache.py#L38
         # Now we want to maintain backward compatibility even when using Python 2.x
         # It makes no difference in Python 3.3+ where IOError is an alias of OSError.
+    def __init__(self, err_no=None, message=None, location=None):  # pylint: disable=useless-super-delegation
+        super(PersistenceError, self).__init__(err_no, message, location)
+
+
+class PersistenceNotFound(PersistenceError):
     """This happens when attempting BasePersistence.load() on a non-existent persistence instance"""
     def __init__(self, err_no=None, message=None, location=None):
         super(PersistenceNotFound, self).__init__(
-            err_no or errno.ENOENT,
-            message or "Persistence not found",
-            location)
+            err_no=errno.ENOENT,
+            message=message or "Persistence not found",
+            location=location)
+
+class PersistenceEncryptionError(PersistenceError):
+    """This could be raised by persistence.save()"""
+
+class PersistenceDecryptionError(PersistenceError):
+    """This could be raised by persistence.load()"""
 
 
 class BasePersistence(ABC):
@@ -101,6 +117,11 @@ def get_location(self):
         raise NotImplementedError
 
 
+def _open(location):
+    return os.open(location, os.O_RDWR | os.O_CREAT | os.O_TRUNC, 0o600)
+        # The 600 seems no-op on NTFS/Windows, and that is fine
+
+
 class FilePersistence(BasePersistence):
     """A generic persistence, storing data in a plain-text file"""
 
@@ -113,7 +134,7 @@ def __init__(self, location):
     def save(self, content):
         # type: (str) -> None
         """Save the content into this persistence"""
-        with open(self._location, 'w+') as handle:  # pylint: disable=unspecified-encoding
+        with os.fdopen(_open(self._location), 'w+') as handle:
             handle.write(content)
 
     def load(self):
@@ -168,16 +189,21 @@ def __init__(self, location, entropy=''):
 
     def save(self, content):
         # type: (str) -> None
-        data = self._dp_agent.protect(content)
-        with open(self._location, 'wb+') as handle:
+        try:
+            data = self._dp_agent.protect(content)
+        except OSError as exception:
+            raise PersistenceEncryptionError(
+                err_no=getattr(exception, "winerror", None),  # Exists in Python 3 on Windows
+                message="Encryption failed: {}. Consider disable encryption.".format(exception),  # pylint: disable=consider-using-f-string
+                )
+        with os.fdopen(_open(self._location), 'wb+') as handle:
             handle.write(data)
 
     def load(self):
         # type: () -> str
         try:
             with open(self._location, 'rb') as handle:
                 data = handle.read()
-            return self._dp_agent.unprotect(data)
         except EnvironmentError as exp:  # EnvironmentError in Py 2.7 works across platform
             if exp.errno == errno.ENOENT:
                 raise PersistenceNotFound(
@@ -190,26 +216,33 @@ def load(self):
                 "DPAPI error likely caused by file content not previously encrypted. "
                 "App developer should migrate by calling save(plaintext) first.")
             raise
+        try:
+            return self._dp_agent.unprotect(data)
+        except OSError as exception:
+            raise PersistenceDecryptionError(
+                err_no=getattr(exception, "winerror", None),  # Exists in Python 3 on Windows
+                message="Decryption failed: {}. You may have to delete the file.".format(exception),  # pylint: disable=consider-using-f-string
+                location=self._location,
+                )
 
 
 class KeychainPersistence(BasePersistence):
     """A generic persistence with data stored in,
     and protected by native Keychain libraries on OSX"""
     is_encrypted = True
 
-    def __init__(self, signal_location, service_name, account_name):
+    def __init__(self, signal_location, service_name=None, account_name=None):
         """Initialization could fail due to unsatisfied dependency.
 
         :param signal_location: See :func:`persistence.LibsecretPersistence.__init__`
         """
-        if not (service_name and account_name):  # It would hang on OSX
-            raise ValueError("service_name and account_name are required")
         from .osx import Keychain, KeychainError  # pylint: disable=import-outside-toplevel
         self._file_persistence = FilePersistence(signal_location)  # Favor composition
         self._Keychain = Keychain  # pylint: disable=invalid-name
         self._KeychainError = KeychainError  # pylint: disable=invalid-name
-        self._service_name = service_name
-        self._account_name = account_name
+        default_service_name = "msal-extensions"  # This is also our package name
+        self._service_name = service_name or default_service_name
+        self._account_name = account_name or _auto_hash(signal_location)
 
     def save(self, content):
         with self._Keychain() as locker:
@@ -247,7 +280,7 @@ class LibsecretPersistence(BasePersistence):
     and protected by native libsecret libraries on Linux"""
     is_encrypted = True
 
-    def __init__(self, signal_location, schema_name, attributes, **kwargs):
+    def __init__(self, signal_location, schema_name=None, attributes=None, **kwargs):
         """Initialization could fail due to unsatisfied dependency.
 
         :param string signal_location:
@@ -262,7 +295,8 @@ def __init__(self, signal_location, schema_name, attributes, **kwargs):
         from .libsecret import (  # This uncertain import is deferred till runtime
             LibSecretAgent, trial_run)
         trial_run()
-        self._agent = LibSecretAgent(schema_name, attributes, **kwargs)
+        self._agent = LibSecretAgent(
+            schema_name or _auto_hash(signal_location), attributes or {}, **kwargs)
         self._file_persistence = FilePersistence(signal_location)  # Favor composition
 
     def save(self, content):

msal_extensions/windows.py

@@ -39,6 +39,15 @@ def raw(self):
         _MEMCPY(blob_buffer, pb_data, cb_data)
         return blob_buffer.raw
 
+_err_description = {
+    # Keys came from real world observation, values came from winerror.h (https://errors)
+    -2146893813: "Key not valid for use in specified state.",
+    -2146892987: "The requested operation cannot be completed. "
+        "The computer must be trusted for delegation and "
+        "the current user account must be configured to allow delegation. "
+        "See also https://docs.microsoft.com/en-us/windows/security/threat-protection/security-policy-settings/enable-computer-and-user-accounts-to-be-trusted-for-delegation",
+    13: "The data is invalid",
+    }
 
 # This code is modeled from a StackOverflow question, which can be found here:
 # https://stackoverflow.com/questions/463832/using-dpapi-with-python
@@ -82,7 +91,7 @@ def protect(self, message):
                 _LOCAL_FREE(result.pbData)
 
         err_code = _GET_LAST_ERROR()
-        raise OSError(256, '', '', err_code)
+        raise OSError(None, _err_description.get(err_code), None, err_code)
 
     def unprotect(self, cipher_text):
         # type: (bytes) -> str
@@ -111,4 +120,4 @@ def unprotect(self, cipher_text):
             finally:
                 _LOCAL_FREE(result.pbData)
         err_code = _GET_LAST_ERROR()
-        raise OSError(256, '', '', err_code)
+        raise OSError(None, _err_description.get(err_code), None, err_code)

sample/persistence_sample.py

@@ -10,7 +10,7 @@ def build_persistence(location, fallback_to_plaintext=False):
     if sys.platform.startswith('win'):
         return FilePersistenceWithDataProtection(location)
     if sys.platform.startswith('darwin'):
-        return KeychainPersistence(location, "my_service_name", "my_account_name")
+        return KeychainPersistence(location)
     if sys.platform.startswith('linux'):
         try:
             return LibsecretPersistence(
@@ -21,8 +21,6 @@ def build_persistence(location, fallback_to_plaintext=False):
                 # unless there would frequently be a desktop session and
                 # a remote ssh session being active simultaneously.
                 location,
-                schema_name="my_schema_name",
-                attributes={"my_attr1": "foo", "my_attr2": "bar"},
                 )
         except:  # pylint: disable=bare-except
             if not fallback_to_plaintext:
@@ -31,6 +29,7 @@ def build_persistence(location, fallback_to_plaintext=False):
     return FilePersistence(location)
 
 persistence = build_persistence("storage.bin", fallback_to_plaintext=False)
+print("Type of persistence: {}".format(persistence.__class__.__name__))
 print("Is this persistence encrypted?", persistence.is_encrypted)
 
 data = {  # It can be anything, here we demonstrate an arbitrary json object

sample/token_cache_sample.py

@@ -10,7 +10,7 @@ def build_persistence(location, fallback_to_plaintext=False):
     if sys.platform.startswith('win'):
         return FilePersistenceWithDataProtection(location)
     if sys.platform.startswith('darwin'):
-        return KeychainPersistence(location, "my_service_name", "my_account_name")
+        return KeychainPersistence(location)
     if sys.platform.startswith('linux'):
         try:
             return LibsecretPersistence(
@@ -21,8 +21,6 @@ def build_persistence(location, fallback_to_plaintext=False):
                 # unless there would frequently be a desktop session and
                 # a remote ssh session being active simultaneously.
                 location,
-                schema_name="my_schema_name",
-                attributes={"my_attr1": "foo", "my_attr2": "bar"},
                 )
         except:  # pylint: disable=bare-except
             if not fallback_to_plaintext:
@@ -31,6 +29,7 @@ def build_persistence(location, fallback_to_plaintext=False):
     return FilePersistence(location)
 
 persistence = build_persistence("token_cache.bin")
+print("Type of persistence: {}".format(persistence.__class__.__name__))
 print("Is this persistence encrypted?", persistence.is_encrypted)
 
 cache = PersistedTokenCache(persistence)

setup.cfg

@@ -9,3 +9,4 @@ project_urls =    Changelog = https://github.com/AzureAD/microsoft-authenticatio
 classifiers =
     License :: OSI Approved :: MIT License
     Development Status :: 4 - Beta
+description = Microsoft Authentication Library extensions (MSAL EX) provides a persistence API that can save your data on disk, encrypted on Windows, macOS and Linux. Concurrent data access will be coordinated by a file lock mechanism.

tests/test_persistence.py

@@ -9,11 +9,18 @@
 from msal_extensions.persistence import *
 
 
-is_running_on_travis_ci = bool(  # (WTF) What-The-Finding:
+def _is_env_var_defined(env_var):
+    return bool(  # (WTF) What-The-Finding:
     # The bool(...) is necessary, otherwise skipif(...) would treat "true" as
     # string conditions and then raise an undefined "true" exception.
     # https://docs.pytest.org/en/latest/historical-notes.html#string-conditions
-    os.getenv("TRAVIS"))
+        os.getenv(env_var))
+
+
+# Note: If you use tox, remember to pass them through via tox.ini
+# https://tox.wiki/en/latest/example/basic.html#passing-down-environment-variables
+is_running_on_travis_ci = _is_env_var_defined("TRAVIS")
+is_running_on_github_ci = _is_env_var_defined("GITHUB_ACTIONS")
 
 @pytest.fixture
 def temp_location():
@@ -42,7 +49,13 @@ def test_nonexistent_file_persistence(temp_location):
     is_running_on_travis_ci or not sys.platform.startswith('win'),
     reason="Requires Windows Desktop")
 def test_file_persistence_with_data_protection(temp_location):
-    _test_persistence_roundtrip(FilePersistenceWithDataProtection(temp_location))
+    try:
+        _test_persistence_roundtrip(FilePersistenceWithDataProtection(temp_location))
+    except PersistenceDecryptionError:
+        if is_running_on_github_ci or is_running_on_travis_ci:
+            logging.warning("DPAPI tends to fail on Windows VM. Run this on your desktop to double check.")
+        else:
+            raise
 
 @pytest.mark.skipif(
     is_running_on_travis_ci or not sys.platform.startswith('win'),
@@ -54,8 +67,7 @@ def test_nonexistent_file_persistence_with_data_protection(temp_location):
     not sys.platform.startswith('darwin'),
     reason="Requires OSX. Whether running on TRAVIS CI does not seem to matter.")
 def test_keychain_persistence(temp_location):
-    _test_persistence_roundtrip(KeychainPersistence(
-        temp_location, "my_service_name", "my_account_name"))
+    _test_persistence_roundtrip(KeychainPersistence(temp_location))
 
 @pytest.mark.skipif(
     not sys.platform.startswith('darwin'),
@@ -69,11 +81,7 @@ def test_nonexistent_keychain_persistence(temp_location):
     is_running_on_travis_ci or not sys.platform.startswith('linux'),
     reason="Requires Linux Desktop. Headless or SSH session won't work.")
 def test_libsecret_persistence(temp_location):
-    _test_persistence_roundtrip(LibsecretPersistence(
-        temp_location,
-        "my_schema_name",
-        {"my_attr_1": "foo", "my_attr_2": "bar"},
-        ))
+    _test_persistence_roundtrip(LibsecretPersistence(temp_location))
 
 @pytest.mark.skipif(
     is_running_on_travis_ci or not sys.platform.startswith('linux'),

tox.ini

@@ -5,5 +5,7 @@ envlist = py27,py35,py36,py37,py38
 deps = pytest
 passenv =
     TRAVIS
+    GITHUB_ACTIONS
+
 commands =
     pytest