From 5d22932a0b7c8067ea22e06ba337fc2fc7abbd72 Mon Sep 17 00:00:00 2001 From: Abhidnya Date: Fri, 10 Apr 2020 13:45:21 -0700 Subject: [PATCH 1/6] Adding explicit unlocking using portalocker (#43) --- msal_extensions/cache_lock.py | 20 +++++++++++++------- setup.py | 2 +- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/msal_extensions/cache_lock.py b/msal_extensions/cache_lock.py index 2a73f24..1adcb5d 100644 --- a/msal_extensions/cache_lock.py +++ b/msal_extensions/cache_lock.py @@ -12,17 +12,23 @@ class CrossPlatLock(object): """ def __init__(self, lockfile_path): self._lockpath = lockfile_path - self._fh = None + self._lock = portalocker.Lock( + lockfile_path, + mode='wb+', + # In posix systems, we HAVE to use LOCK_EX(exclusive lock) bitwise ORed + # with LOCK_NB(non-blocking) to avoid blocking on lock acquisition. + # More information here: + # https://docs.python.org/3/library/fcntl.html#fcntl.lockf + flags=portalocker.LOCK_EX | portalocker.LOCK_NB, + buffering=0) def __enter__(self): - pid = os.getpid() - - self._fh = open(self._lockpath, 'wb+', buffering=0) - portalocker.lock(self._fh, portalocker.LOCK_EX) - self._fh.write('{} {}'.format(pid, sys.argv[0]).encode('utf-8')) + file_handle = self._lock.__enter__() + file_handle.write('{} {}'.format(os.getpid(), sys.argv[0]).encode('utf-8')) + return file_handle def __exit__(self, *args): - self._fh.close() + self._lock.__exit__(*args) try: # Attempt to delete the lockfile. In either of the failure cases enumerated below, it is # likely that another process has raced this one and ended up clearing or locking the diff --git a/setup.py b/setup.py index ce2845c..4c742a7 100644 --- a/setup.py +++ b/setup.py @@ -18,7 +18,7 @@ package_data={'': ['LICENSE']}, install_requires=[ 'msal>=0.4.1,<2.0.0', - 'portalocker~=1.0', + 'portalocker~=1.6', ], tests_require=['pytest'], ) From 770860633961d342c1aa4896272333d9495e6982 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 25 Mar 2020 22:32:23 -0700 Subject: [PATCH 2/6] Proof-of-Concept: an encryption wrapper based on libsecret --- .pylintrc | 1 + .travis.yml | 18 +++++- msal_extensions/libsecret.py | 120 +++++++++++++++++++++++++++++++++++ setup.py | 1 + 4 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 msal_extensions/libsecret.py diff --git a/.pylintrc b/.pylintrc index 3fa50bb..ed97d8b 100644 --- a/.pylintrc +++ b/.pylintrc @@ -1,3 +1,4 @@ [MESSAGES CONTROL] disable= + trailing-newlines, useless-object-inheritance diff --git a/.travis.yml b/.travis.yml index c315191..31a5e21 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,20 +6,35 @@ matrix: - python: "2.7" env: TOXENV=py27 PYPI=true os: linux + before_install: + - sudo apt update + - sudo apt install python-dev libgirepository1.0-dev libcairo2-dev gir1.2-secret-1 - python: "3.5" env: TOXENV=py35 os: linux + before_install: + - sudo apt update + - sudo apt install python3-dev libgirepository1.0-dev libcairo2-dev gir1.2-secret-1 - python: "3.6" env: TOXENV=py36 os: linux + before_install: + - sudo apt update + - sudo apt install python3-dev libgirepository1.0-dev libcairo2-dev gir1.2-secret-1 - python: "3.7" env: TOXENV=py37 os: linux dist: xenial + before_install: + - sudo apt update + - sudo apt install python3-dev libgirepository1.0-dev libcairo2-dev gir1.2-secret-1 - python: "3.8" env: TOXENV=py38 os: linux dist: xenial + before_install: + - sudo apt update + - sudo apt install python3-dev libgirepository1.0-dev libcairo2-dev gir1.2-secret-1 - name: "Python 3.7 on macOS" env: TOXENV=py37 os: osx @@ -46,7 +61,8 @@ install: - pip install . script: - - pylint msal_extensions + - # Difficult to get .pylintrc working on both Python 2 & 3, and we don't have to + - if [ "$TOXENV" = "py37"]; then pylint msal_extensions; fi - tox deploy: diff --git a/msal_extensions/libsecret.py b/msal_extensions/libsecret.py new file mode 100644 index 0000000..fa64a94 --- /dev/null +++ b/msal_extensions/libsecret.py @@ -0,0 +1,120 @@ +"""Implements a Linux specific TokenCache, and provides auxiliary helper types. + +This module depends on PyGObject. But `pip install pygobject` would typically fail, +until you install its dependencies first. For example, on a Debian Linux, you need:: + + sudo apt install libgirepository1.0-dev libcairo2-dev python3-dev gir1.2-secret-1 + pip install pygobject + +Alternatively, you could skip Cairo & PyCairo, but you still need to do all these +(derived from https://gitlab.gnome.org/GNOME/pygobject/-/issues/395):: + + sudo apt install libgirepository1.0-dev python3-dev gir1.2-secret-1 + pip install wheel + PYGOBJECT_WITHOUT_PYCAIRO=1 pip install --no-build-isolation pygobject +""" +import logging + +import gi # https://pygobject.readthedocs.io/en/latest/getting_started.html + +# pylint: disable=no-name-in-module +gi.require_version("Secret", "1") # Would require a package gir1.2-secret-1 +# pylint: disable=wrong-import-position +from gi.repository import Secret # Would require a package gir1.2-secret-1 + + +logger = logging.getLogger(__name__) + +class LibSecretAgent(object): + """A loader/saver built on top of low-level libsecret""" + # Inspired by https://developer.gnome.org/libsecret/unstable/py-examples.html + def __init__( # pylint: disable=too-many-arguments + self, + schema_name, + attributes, # {"name": "value", ...} + label="", # Helpful when visualizing secrets by other viewers + attribute_types=None, # {name: SchemaAttributeType, ...} + collection=None, # None means default collection + ): # pylint: disable=bad-continuation + """This agent is built on top of lower level libsecret API. + + Content stored via libsecret is associated with a bunch of attributes. + + :param string schema_name: + Attributes would conceptually follow an existing schema. + But this class will do it in the other way around, + by automatically deriving a schema based on your attributes. + However, you will still need to provide a schema_name. + load() and save() will only operate on data with matching schema_name. + + :param dict attributes: + Attributes are key-value pairs, represented as a Python dict here. + They will be used to filter content during load() and save(). + Their arbitrary keys are strings. + Their arbitrary values can MEAN strings, integers and booleans, + but are always represented as strings, according to upstream sample: + https://developer.gnome.org/libsecret/0.18/py-store-example.html + + :param string label: + It will not be used during data lookup and filtering. + It is only helpful when/if you visualize secrets by other viewers. + + :param dict attribute_types: + Each key is the name of your each attribute. + The corresponding value will be one of the following three: + + * Secret.SchemaAttributeType.STRING + * Secret.SchemaAttributeType.INTEGER + * Secret.SchemaAttributeType.BOOLEAN + + But if all your attributes are Secret.SchemaAttributeType.STRING, + you do not need to provide this types definition at all. + + :param collection: + The default value `None` means default collection. + """ + self._collection = collection + self._attributes = attributes or {} + self._label = label + self._schema = Secret.Schema.new(schema_name, Secret.SchemaFlags.NONE, { + k: (attribute_types or {}).get(k, Secret.SchemaAttributeType.STRING) + for k in self._attributes}) + + def save(self, data): + """Store data. Returns a boolean of whether operation was successful.""" + return Secret.password_store_sync( + self._schema, self._attributes, self._collection, self._label, + data, None) + + def load(self): + """Load a password in the secret service, return None when found nothing""" + return Secret.password_lookup_sync(self._schema, self._attributes, None) + + def clear(self): + """Returns a boolean of whether any passwords were removed""" + return Secret.password_clear_sync(self._schema, self._attributes, None) + + +def trial_run(): + """This trial run will raise an exception if libsecret is not functioning. + + Even after you installed all the dependencies so that your script can start, + or even if your previous run was successful, your script could fail next time, + for example when it will be running inside a headless SSH session. + + You do not have to do trial_run. The exception would also be raised by save(). + """ + try: + agent = LibSecretAgent("Test Schema", {"attr1": "foo", "attr2": "bar"}) + payload = "Test Data" + agent.save(payload) # It would fail when running inside an SSH session + assert agent.load() == payload # This line is probably not reachable + agent.clear() + except (gi.repository.GLib.Error, AssertionError): + message = ( + "libsecret did not perform properly. Please refer to " + "https://github.com/AzureAD/microsoft-authentication-extensions-for-python/wiki/Encryption-on-Linux") # pylint: disable=line-too-long + logger.exception(message) # This log contains trace stack for debugging + logger.warning(message) # This is visible by default + raise + diff --git a/setup.py b/setup.py index 4c742a7..c0ffab6 100644 --- a/setup.py +++ b/setup.py @@ -19,6 +19,7 @@ install_requires=[ 'msal>=0.4.1,<2.0.0', 'portalocker~=1.6', + "pygobject>=3,<4;platform_system=='Linux'", ], tests_require=['pytest'], ) From c8f4774a8bac627be5c54f90dc4fd5cb6726ff08 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 3 Apr 2020 12:52:15 -0700 Subject: [PATCH 3/6] A generic persistence layer, some provides data encryption Remove dummy and use Path(...).touch() instead Document schema_name and attributes Change getmtime() to time_last_modified() Rename 2 location into signal_location, with docs Refactor test cases using pytest.mark.skipif --- msal_extensions/__init__.py | 8 ++ msal_extensions/persistence.py | 201 +++++++++++++++++++++++++++++++++ sample/persistence_sample.py | 45 ++++++++ setup.py | 1 + tests/test_persistence.py | 54 +++++++++ tox.ini | 2 + 6 files changed, 311 insertions(+) create mode 100644 msal_extensions/persistence.py create mode 100644 sample/persistence_sample.py create mode 100644 tests/test_persistence.py diff --git a/msal_extensions/__init__.py b/msal_extensions/__init__.py index fbd898c..82143e0 100644 --- a/msal_extensions/__init__.py +++ b/msal_extensions/__init__.py @@ -3,6 +3,14 @@ import sys +from .persistence import ( + FilePersistence, + FilePersistenceWithDataProtection, + KeychainPersistence, + LibsecretPersistence, + ) +from .cache_lock import CrossPlatLock + if sys.platform.startswith('win'): from .token_cache import WindowsTokenCache as TokenCache elif sys.platform.startswith('darwin'): diff --git a/msal_extensions/persistence.py b/msal_extensions/persistence.py new file mode 100644 index 0000000..2ddf432 --- /dev/null +++ b/msal_extensions/persistence.py @@ -0,0 +1,201 @@ +"""A generic persistence layer, optionally encrypted on Windows, OSX, and Linux. + +Should a certain encryption is unavailable, exception will be raised at run-time, +rather than at import time. + +By successfully creating and using a certain persistence object, +app developer would naturally know whether the data are protected by encryption. +""" +import abc +import os +import errno +try: + from pathlib import Path # Built-in in Python 3 +except: + from pathlib2 import Path # An extra lib for Python 2 + + +try: + ABC = abc.ABC +except AttributeError: # Python 2.7, abc exists, but not ABC + ABC = abc.ABCMeta("ABC", (object,), {"__slots__": ()}) # type: ignore + + +def _mkdir_p(path): + """Creates a directory, and any necessary parents. + + This implementation based on a Stack Overflow question that can be found here: + https://stackoverflow.com/questions/600268/mkdir-p-functionality-in-python + + If the path provided is an existing file, this function raises an exception. + :param path: The directory name that should be created. + """ + if not path: + return # NO-OP + try: + os.makedirs(path) + except OSError as exp: + if exp.errno == errno.EEXIST and os.path.isdir(path): + pass + else: + raise + + +class BasePersistence(ABC): + """An abstract persistence defining the common interface of this family""" + + is_encrypted = False # Default to False. To be overridden by sub-classes. + + @abc.abstractmethod + def save(self, content): + # type: (str) -> None + """Save the content into this persistence""" + raise NotImplementedError + + @abc.abstractmethod + def load(self): + # type: () -> str + """Load content from this persistence""" + raise NotImplementedError + + @abc.abstractmethod + def time_last_modified(self): + """Get the last time when this persistence has been modified""" + raise NotImplementedError + + @abc.abstractmethod + def get_location(self): + """Return the file path which this persistence stores (meta)data into""" + raise NotImplementedError + + +class FilePersistence(BasePersistence): + """A generic persistence, storing data in a plain-text file""" + + def __init__(self, location): + if not location: + raise ValueError("Requires a file path") + self._location = os.path.expanduser(location) + _mkdir_p(os.path.dirname(self._location)) + + def save(self, content): + # type: (str) -> None + """Save the content into this persistence""" + with open(self._location, 'w+') as handle: + handle.write(content) + + def load(self): + # type: () -> str + """Load content from this persistence""" + with open(self._location, 'r') as handle: + return handle.read() + + def time_last_modified(self): + return os.path.getmtime(self._location) + + def touch(self): + """To touch this file-based persistence without writing content into it""" + Path(self._location).touch() # For os.path.getmtime() to work + + def get_location(self): + return self._location + + +class FilePersistenceWithDataProtection(FilePersistence): + """A generic persistence with data stored in a file, + protected by Win32 encryption APIs on Windows""" + is_encrypted = True + + def __init__(self, location, entropy=''): + """Initialization could fail due to unsatisfied dependency""" + # pylint: disable=import-outside-toplevel + from .windows import WindowsDataProtectionAgent + self._dp_agent = WindowsDataProtectionAgent(entropy=entropy) + super(FilePersistenceWithDataProtection, self).__init__(location) + + def save(self, content): + super(FilePersistenceWithDataProtection, self).save( + self._dp_agent.protect(content)) + + def load(self): + return self._dp_agent.unprotect( + super(FilePersistenceWithDataProtection, self).load()) + + +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): + """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 # pylint: disable=import-outside-toplevel + self._file_persistence = FilePersistence(signal_location) # Favor composition + self._Keychain = Keychain # pylint: disable=invalid-name + self._service_name = service_name + self._account_name = account_name + + def save(self, content): + with self._Keychain() as locker: + locker.set_generic_password( + self._service_name, self._account_name, content) + self._file_persistence.touch() # For time_last_modified() + + def load(self): + with self._Keychain() as locker: + return locker.get_generic_password( + self._service_name, self._account_name) + + def time_last_modified(self): + return self._file_persistence.time_last_modified() + + def get_location(self): + return self._file_persistence.get_location() + + +class LibsecretPersistence(BasePersistence): + """A generic persistence with data stored in, + and protected by native libsecret libraries on Linux""" + is_encrypted = True + + def __init__(self, signal_location, schema_name, attributes, **kwargs): + """Initialization could fail due to unsatisfied dependency. + + :param string signal_location: + Besides saving the real payload into encrypted storage, + this class will also touch this signal file. + Applications may listen a FileSystemWatcher.Changed event for reload. + https://docs.microsoft.com/en-us/dotnet/api/system.io.filesystemwatcher.changed?view=netframework-4.8#remarks + :param string schema_name: See :func:`libsecret.LibSecretAgent.__init__` + :param dict attributes: See :func:`libsecret.LibSecretAgent.__init__` + """ + # pylint: disable=import-outside-toplevel + from .libsecret import ( # This uncertain import is deferred till runtime + LibSecretAgent, trial_run) + trial_run() + self._agent = LibSecretAgent(schema_name, attributes, **kwargs) + self._file_persistence = FilePersistence(signal_location) # Favor composition + + def save(self, content): + if self._agent.save(content): + self._file_persistence.touch() # For time_last_modified() + + def load(self): + return self._agent.load() + + def time_last_modified(self): + return self._file_persistence.time_last_modified() + + def get_location(self): + return self._file_persistence.get_location() + +# We could also have a KeyringPersistence() which can then be used together +# with a FilePersistence to achieve +# https://github.com/AzureAD/microsoft-authentication-extensions-for-python/issues/12 +# But this idea is not pursued at this time. + diff --git a/sample/persistence_sample.py b/sample/persistence_sample.py new file mode 100644 index 0000000..dc24435 --- /dev/null +++ b/sample/persistence_sample.py @@ -0,0 +1,45 @@ +import sys +import logging +import json + +from msal_extensions import * + + +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, "my_service_name", "my_account_name") + 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, + schema_name="my_schema_name", + attributes={"my_attr1": "foo", "my_attr2": "bar"}, + ) + except: # pylint: disable=bare-except + if not fallback_to_plaintext: + raise + logging.exception("Encryption unavailable. Opting in to plain text.") + return FilePersistence(location) + +persistence = build_persistence("storage.bin") +print("Is this persistence encrypted?", persistence.is_encrypted) + +data = { # It can be anything, here we demonstrate an arbitrary json object + "foo": "hello world", + "bar": "", + "service_principle_1": "blah blah...", + } + +with CrossPlatLock("my_another_lock.txt"): + persistence.save(json.dumps(data)) + assert json.loads(persistence.load()) == data + diff --git a/setup.py b/setup.py index c0ffab6..52b8fb9 100644 --- a/setup.py +++ b/setup.py @@ -19,6 +19,7 @@ install_requires=[ 'msal>=0.4.1,<2.0.0', 'portalocker~=1.6', + "pathlib2;python_version<'3.0'", "pygobject>=3,<4;platform_system=='Linux'", ], tests_require=['pytest'], diff --git a/tests/test_persistence.py b/tests/test_persistence.py new file mode 100644 index 0000000..0ec369b --- /dev/null +++ b/tests/test_persistence.py @@ -0,0 +1,54 @@ +import os +import sys +import shutil +import tempfile +import logging + +import pytest + +from msal_extensions.persistence import * + + +is_running_on_travis_ci = 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")) + +@pytest.fixture +def temp_location(): + test_folder = tempfile.mkdtemp(prefix="test_persistence_roundtrip") + yield os.path.join(test_folder, 'persistence.bin') + shutil.rmtree(test_folder, ignore_errors=True) + +def _test_persistence_roundtrip(persistence): + payload = 'arbitrary content' + persistence.save(payload) + assert persistence.load() == payload + +def test_file_persistence(temp_location): + _test_persistence_roundtrip(FilePersistence(temp_location)) + +@pytest.mark.skipif( + 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)) + +@pytest.mark.skipif( + 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")) + +@pytest.mark.skipif( + 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"}, + )) + diff --git a/tox.ini b/tox.ini index 3317e47..ecbab74 100644 --- a/tox.ini +++ b/tox.ini @@ -3,5 +3,7 @@ envlist = py27,py35,py36,py37,py38 [testenv] deps = pytest +passenv = + TRAVIS commands = pytest From c4091f7752c55d1596cfb139c1c6432819f040e2 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Mon, 30 Mar 2020 21:08:07 -0700 Subject: [PATCH 4/6] Rewrite entire token_cache.py --- .pylintrc | 2 + msal_extensions/__init__.py | 3 +- msal_extensions/token_cache.py | 173 ++++++++++----------------------- sample/token_cache_sample.py | 39 ++++++++ tests/test_agnostic_backend.py | 78 +++++++-------- 5 files changed, 131 insertions(+), 164 deletions(-) create mode 100644 sample/token_cache_sample.py diff --git a/.pylintrc b/.pylintrc index ed97d8b..bd618db 100644 --- a/.pylintrc +++ b/.pylintrc @@ -1,4 +1,6 @@ [MESSAGES CONTROL] +good-names= + logger disable= trailing-newlines, useless-object-inheritance diff --git a/msal_extensions/__init__.py b/msal_extensions/__init__.py index 82143e0..148b696 100644 --- a/msal_extensions/__init__.py +++ b/msal_extensions/__init__.py @@ -10,10 +10,11 @@ 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 UnencryptedTokenCache as TokenCache + from .token_cache import FileTokenCache as TokenCache diff --git a/msal_extensions/token_cache.py b/msal_extensions/token_cache.py index 4d419d2..bf55448 100644 --- a/msal_extensions/token_cache.py +++ b/msal_extensions/token_cache.py @@ -1,156 +1,89 @@ """Generic functions and types for working with a TokenCache that is not platform specific.""" import os -import sys import warnings import time import errno -import msal -from .cache_lock import CrossPlatLock +import logging -if sys.platform.startswith('win'): - from .windows import WindowsDataProtectionAgent -elif sys.platform.startswith('darwin'): - from .osx import Keychain +import msal -def _mkdir_p(path): - """Creates a directory, and any necessary parents. +from .cache_lock import CrossPlatLock +from .persistence import ( + _mkdir_p, FilePersistence, + FilePersistenceWithDataProtection, KeychainPersistence) - This implementation based on a Stack Overflow question that can be found here: - https://stackoverflow.com/questions/600268/mkdir-p-functionality-in-python - If the path provided is an existing file, this function raises an exception. - :param path: The directory name that should be created. - """ - try: - os.makedirs(path) - except OSError as exp: - if exp.errno == errno.EEXIST and os.path.isdir(path): - pass - else: - raise +logger = logging.getLogger(__name__) +class PersistedTokenCache(msal.SerializableTokenCache): + """A token cache using given persistence layer, coordinated by a file lock.""" -class FileTokenCache(msal.SerializableTokenCache): - """Implements basic unprotected SerializableTokenCache to a plain-text file.""" - def __init__(self, - cache_location, - lock_location=None): - super(FileTokenCache, self).__init__() - self._cache_location = cache_location - self._lock_location = lock_location or self._cache_location + '.lockfile' + def __init__(self, persistence, lock_location=None): + super(PersistedTokenCache, self).__init__() + self._lock_location = ( + os.path.expanduser(lock_location) if lock_location + else persistence.get_location() + ".lockfile") + _mkdir_p(os.path.dirname(self._lock_location)) + self._persistence = persistence self._last_sync = 0 # _last_sync is a Unixtime + self.is_encrypted = persistence.is_encrypted - self._cache_location = os.path.expanduser(self._cache_location) - self._lock_location = os.path.expanduser(self._lock_location) - - _mkdir_p(os.path.dirname(self._lock_location)) - _mkdir_p(os.path.dirname(self._cache_location)) - - def _needs_refresh(self): - # type: () -> Bool - """ - Inspects the file holding the encrypted TokenCache to see if a read is necessary. - :return: True if there are changes not reflected in memory, False otherwise. - """ + def _reload_if_necessary(self): + # type: () -> None + """Reload cache from persistence layer, if necessary""" try: - updated = os.path.getmtime(self._cache_location) - return self._last_sync < updated + if self._last_sync < self._persistence.time_last_modified(): + self.deserialize(self._persistence.load()) + self._last_sync = time.time() except IOError as exp: if exp.errno != errno.ENOENT: - raise exp - return False - - def _write(self, contents): - # type: (str) -> None - """Handles actually committing the serialized form of this TokenCache to persisted storage. - For types derived of this, class that will be a file, which has the ability to track a last - modified time. - - :param contents: The serialized contents of a TokenCache - """ - with open(self._cache_location, 'w+') as handle: - handle.write(contents) - - def _read(self): - # type: () -> str - """Fetches the contents of a file and invokes deserialization.""" - with open(self._cache_location, 'r') as handle: - return handle.read() + raise + # Otherwise, from cache's perspective, a nonexistent file is a NO-OP def modify(self, credential_type, old_entry, new_key_value_pairs=None): with CrossPlatLock(self._lock_location): - if self._needs_refresh(): - try: - self.deserialize(self._read()) - except IOError as exp: - if exp.errno != errno.ENOENT: - raise - super(FileTokenCache, self).modify( + self._reload_if_necessary() + super(PersistedTokenCache, self).modify( credential_type, old_entry, new_key_value_pairs=new_key_value_pairs) - self._write(self.serialize()) - self._last_sync = os.path.getmtime(self._cache_location) + self._persistence.save(self.serialize()) + self._last_sync = time.time() def find(self, credential_type, **kwargs): # pylint: disable=arguments-differ with CrossPlatLock(self._lock_location): - if self._needs_refresh(): - try: - self.deserialize(self._read()) - except IOError as exp: - if exp.errno != errno.ENOENT: - raise - self._last_sync = time.time() - return super(FileTokenCache, self).find(credential_type, **kwargs) + self._reload_if_necessary() + return super(PersistedTokenCache, self).find(credential_type, **kwargs) -class UnencryptedTokenCache(FileTokenCache): - """An unprotected token cache to default to when no-platform specific option is available.""" - def __init__(self, cache_location, **kwargs): - warnings.warn("You are using an unprotected token cache, " - "because an encrypted option is not available for {}".format(sys.platform), - RuntimeWarning) - super(UnencryptedTokenCache, self).__init__(cache_location, **kwargs) +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(FileTokenCache): - """A SerializableTokenCache implementation which uses Win32 encryption APIs to protect your - tokens. - """ - def __init__(self, cache_location, entropy='', **kwargs): - super(WindowsTokenCache, self).__init__(cache_location, **kwargs) - self._dp_agent = WindowsDataProtectionAgent(entropy=entropy) - def _write(self, contents): - with open(self._cache_location, 'wb') as handle: - handle.write(self._dp_agent.protect(contents)) +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)) - def _read(self): - with open(self._cache_location, 'rb') as handle: - cipher_text = handle.read() - return self._dp_agent.unprotect(cipher_text) - - -class OSXTokenCache(FileTokenCache): - """A SerializableTokenCache implementation which uses native Keychain libraries to protect your - tokens. - """ +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', - **kwargs): - super(OSXTokenCache, self).__init__(cache_location, **kwargs) - self._service_name = service_name - self._account_name = account_name - - def _read(self): - with Keychain() as locker: - return locker.get_generic_password(self._service_name, self._account_name) - - def _write(self, contents): - with Keychain() as locker: - locker.set_generic_password(self._service_name, self._account_name, contents) - with open(self._cache_location, "w+") as handle: - handle.write('{} {}'.format(os.getpid(), sys.argv[0])) + **ignored): # pylint: disable=unused-argument + warnings.warn("Use PersistedTokenCache(...) instead", DeprecationWarning) + super(OSXTokenCache, self).__init__( + KeychainPersistence(cache_location, service_name, account_name)) + diff --git a/sample/token_cache_sample.py b/sample/token_cache_sample.py new file mode 100644 index 0000000..b48e19d --- /dev/null +++ b/sample/token_cache_sample.py @@ -0,0 +1,39 @@ +import sys +import logging +import json + +from msal_extensions import * + + +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, "my_service_name", "my_account_name") + 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, + schema_name="my_schema_name", + attributes={"my_attr1": "foo", "my_attr2": "bar"}, + ) + except: # pylint: disable=bare-except + if not fallback_to_plaintext: + raise + logging.exception("Encryption unavailable. Opting in to plain text.") + return FilePersistence(location) + +persistence = build_persistence("token_cache.bin") +print("Is this persistence encrypted?", persistence.is_encrypted) + +cache = PersistedTokenCache(persistence) +# Now you can use it in an msal application like this: +# app = msal.PublicClientApplication("my_client_id", token_cache=cache) + diff --git a/tests/test_agnostic_backend.py b/tests/test_agnostic_backend.py index 1d9c7d0..4619391 100644 --- a/tests/test_agnostic_backend.py +++ b/tests/test_agnostic_backend.py @@ -1,54 +1,46 @@ import os import shutil import tempfile -import pytest +import sys + import msal +import pytest +from msal_extensions import * -def test_file_token_cache_roundtrip(): - from msal_extensions.token_cache import FileTokenCache +@pytest.fixture +def temp_location(): + test_folder = tempfile.mkdtemp(prefix="test_token_cache_roundtrip") + yield os.path.join(test_folder, 'token_cache.bin') + shutil.rmtree(test_folder, ignore_errors=True) + + +def _test_token_cache_roundtrip(cache): 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 FileTokenCache round-trip with.') - - test_folder = tempfile.mkdtemp(prefix="msal_extension_test_file_token_cache_roundtrip") - cache_file = os.path.join(test_folder, 'msal.cache') - try: - subject = FileTokenCache(cache_location=cache_file) - app = msal.ConfidentialClientApplication( - client_id=client_id, - client_credential=client_secret, - token_cache=subject) - desired_scopes = ['https://graph.microsoft.com/.default'] - token1 = app.acquire_token_for_client(scopes=desired_scopes) - os.utime(cache_file, None) # Mock having another process update the cache. - token2 = app.acquire_token_silent(scopes=desired_scopes, account=None) - assert token1['access_token'] == token2['access_token'] - finally: - shutil.rmtree(test_folder, ignore_errors=True) - - -def test_current_platform_cache_roundtrip(): + pytest.skip('no credentials present to test TokenCache round-trip with.') + + app = msal.ConfidentialClientApplication( + client_id=client_id, + client_credential=client_secret, + token_cache=cache) + desired_scopes = ['https://graph.microsoft.com/.default'] + token1 = app.acquire_token_for_client(scopes=desired_scopes) + os.utime( # Mock having another process update the cache + cache._persistence.get_location(), None) + token2 = app.acquire_token_silent(scopes=desired_scopes, account=None) + 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)) + +def test_current_platform_cache_roundtrip_with_alias_class(temp_location): from msal_extensions import TokenCache - 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 FileTokenCache round-trip with.') - - test_folder = tempfile.mkdtemp(prefix="msal_extension_test_file_token_cache_roundtrip") - cache_file = os.path.join(test_folder, 'msal.cache') - try: - subject = TokenCache(cache_location=cache_file) - app = msal.ConfidentialClientApplication( - client_id=client_id, - client_credential=client_secret, - token_cache=subject) - desired_scopes = ['https://graph.microsoft.com/.default'] - token1 = app.acquire_token_for_client(scopes=desired_scopes) - os.utime(cache_file, None) # Mock having another process update the cache. - token2 = app.acquire_token_silent(scopes=desired_scopes, account=None) - assert token1['access_token'] == token2['access_token'] - finally: - shutil.rmtree(test_folder, ignore_errors=True) + _test_token_cache_roundtrip(TokenCache(temp_location)) + +def test_persisted_token_cache(temp_location): + _test_token_cache_roundtrip(PersistedTokenCache(FilePersistence(temp_location))) + From 88d37b7c433192655d3f063ea61ac718df650d3c Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Fri, 3 Apr 2020 17:57:58 -0700 Subject: [PATCH 5/6] Dunno why this was passing previously but not now --- msal_extensions/cache_lock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msal_extensions/cache_lock.py b/msal_extensions/cache_lock.py index 1adcb5d..eeeb25e 100644 --- a/msal_extensions/cache_lock.py +++ b/msal_extensions/cache_lock.py @@ -34,6 +34,6 @@ def __exit__(self, *args): # likely that another process has raced this one and ended up clearing or locking the # file for itself. os.remove(self._lockpath) - except OSError as ex: + except OSError as ex: # pylint: disable=invalid-name if ex.errno != errno.ENOENT and ex.errno != errno.EACCES: raise From 578d0b545e4b6b55019330d4f2a679b85c321e36 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 15 Apr 2020 11:30:13 -0700 Subject: [PATCH 6/6] Update __init__.py --- msal_extensions/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msal_extensions/__init__.py b/msal_extensions/__init__.py index 148b696..1d573a0 100644 --- a/msal_extensions/__init__.py +++ b/msal_extensions/__init__.py @@ -1,5 +1,5 @@ """Provides auxiliary functionality to the `msal` package.""" -__version__ = "0.1.3" +__version__ = "0.2.0" import sys