diff --git a/msal_extensions/__init__.py b/msal_extensions/__init__.py index 14a8c6d..141cfb9 100644 --- a/msal_extensions/__init__.py +++ b/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 diff --git a/msal_extensions/persistence.py b/msal_extensions/persistence.py index e4b6c66..257adad 100644 --- a/msal_extensions/persistence.py +++ b/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,8 +189,14 @@ 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): @@ -177,7 +204,6 @@ def load(self): 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,6 +216,14 @@ 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): @@ -197,19 +231,18 @@ class KeychainPersistence(BasePersistence): 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): diff --git a/msal_extensions/windows.py b/msal_extensions/windows.py index f1e8b59..d534619 100644 --- a/msal_extensions/windows.py +++ b/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) diff --git a/sample/persistence_sample.py b/sample/persistence_sample.py index 74074d3..f5c8c06 100644 --- a/sample/persistence_sample.py +++ b/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 diff --git a/sample/token_cache_sample.py b/sample/token_cache_sample.py index b48e19d..7210efa 100644 --- a/sample/token_cache_sample.py +++ b/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) diff --git a/setup.cfg b/setup.cfg index 80050d9..6b5d8b8 100644 --- a/setup.cfg +++ b/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. diff --git a/tests/test_persistence.py b/tests/test_persistence.py index bbbe155..eab4048 100644 --- a/tests/test_persistence.py +++ b/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'), diff --git a/tox.ini b/tox.ini index ecbab74..8a538bc 100644 --- a/tox.ini +++ b/tox.ini @@ -5,5 +5,7 @@ envlist = py27,py35,py36,py37,py38 deps = pytest passenv = TRAVIS + GITHUB_ACTIONS + commands = pytest