From dfdb5eca0f38e38cad0b39dbd810c2b3a9e5180d Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Thu, 9 May 2019 15:32:48 -0700 Subject: [PATCH 01/13] Establish Project File Structure (#10) * Establishing most basic file structure. * Promoting msal_extensions folder in file structure. * Parse __init__ for version number. This flow is based on code found here: https://stackoverflow.com/questions/17583443/what-is-the-correct-way-to-share-package-version-with-setup-py-and-the-package/39671214#39671214 However the pattern was expanded slightly to handle String Literal Prefixes as defined here: https://docs.python.org/3/reference/lexical_analysis.html#grammar-token-stringprefix * Exclude tests from bundled packages. * Fixing formatting nit * Removing "tests" from being a package. This prevents "find_packages" from picking it up, even if there is no exclusion set. --- .gitignore | 3 +++ msal_extensions/__init__.py | 1 + setup.py | 19 +++++++++++++++++++ 3 files changed, 23 insertions(+) create mode 100644 msal_extensions/__init__.py create mode 100644 setup.py diff --git a/.gitignore b/.gitignore index 3e759b7..69b1e0e 100644 --- a/.gitignore +++ b/.gitignore @@ -298,6 +298,9 @@ paket-files/ __pycache__/ *.pyc +# Python Auxiliary Tools +*.egg-info/ + # Cake - Uncomment if you are using it # tools/** # !tools/packages.config diff --git a/msal_extensions/__init__.py b/msal_extensions/__init__.py new file mode 100644 index 0000000..f102a9c --- /dev/null +++ b/msal_extensions/__init__.py @@ -0,0 +1 @@ +__version__ = "0.0.1" diff --git a/setup.py b/setup.py new file mode 100644 index 0000000..17d9fca --- /dev/null +++ b/setup.py @@ -0,0 +1,19 @@ +#!/usr/bin/env python + +from setuptools import setup, find_packages +import re, io + +__version__ = re.search( + r'__version__\s*=\s*[rRfFuU]{0,2}[\'"]([^\'"]*)[\'"]', + io.open('msal_extensions/__init__.py', encoding='utf_8_sig').read() + ).group(1) + +setup( + name='msal-extensions', + version=__version__, + packages=find_packages(), + classifiers=[ + 'Development Status :: 2 - Pre-Alpha', + ], + tests_require=['pytest'], +) From 3c2a2825e22ce6c9ec00e4870d73d837e83689a5 Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Wed, 15 May 2019 17:28:04 -0700 Subject: [PATCH 02/13] Prevent entry into a code-block until a file lock can be taken (#11) * Adding a class to prevent entry into a code-block until a file lock can be taken. This replicates logic in the .NET exensions repository found here: https://github.com/AzureAD/microsoft-authentication-extensions-for-dotnet/blob/113ecc725891395dd7212aac82d0681f9a0d60ce/src/Shared/CrossPlatLock.cs It will allow .NET processes and Python processes to not step on eachother's toes. It does add a dependency on two new libraries: psutil and portalocker * Adding class doc-comment. * Remove dependency on psutil * Fixing naming convention * Removing unused constants * Removing dir presence assertion --- msal_extensions/cache_lock.py | 32 ++++++++++++++++++++++++++++++++ setup.py | 3 +++ tests/lock_acquire.py | 28 ++++++++++++++++++++++++++++ tests/test_crossplatlock.py | 18 ++++++++++++++++++ 4 files changed, 81 insertions(+) create mode 100644 msal_extensions/cache_lock.py create mode 100644 tests/lock_acquire.py create mode 100644 tests/test_crossplatlock.py diff --git a/msal_extensions/cache_lock.py b/msal_extensions/cache_lock.py new file mode 100644 index 0000000..9ef90fd --- /dev/null +++ b/msal_extensions/cache_lock.py @@ -0,0 +1,32 @@ +import os +import sys +import errno +import portalocker + + +class CrossPlatLock(object): + """ + Offers a mechanism for waiting until another process is finished interacting with a shared resource. This is + specifically written to interact with a class of the same name in the .NET extensions library. + """ + def __init__(self, lockfile_path): + self._lockpath = lockfile_path + + 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')) + + def __exit__(self, *args): + self._fh.close() + 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 file for itself. + os.remove(self._lockpath) + except PermissionError: + pass + except OSError as ex: + if ex.errno != errno.ENOENT: + raise diff --git a/setup.py b/setup.py index 17d9fca..cbfe080 100644 --- a/setup.py +++ b/setup.py @@ -15,5 +15,8 @@ classifiers=[ 'Development Status :: 2 - Pre-Alpha', ], + install_requires=[ + 'portalocker~=1.0', + ], tests_require=['pytest'], ) diff --git a/tests/lock_acquire.py b/tests/lock_acquire.py new file mode 100644 index 0000000..c1b5b21 --- /dev/null +++ b/tests/lock_acquire.py @@ -0,0 +1,28 @@ +import sys +import os +import time +import datetime +from msal_extensions import CrossPlatLock + + +def main(hold_time): + # type: (datetime.timedelta) -> None + """ + Grabs a lock from a well-known file in order to test the CrossPlatLock class across processes. + :param hold_time: The approximate duration that this process should hold onto the lock. + :return: None + """ + pid = os.getpid() + print('{} starting'.format(pid)) + with CrossPlatLock('./delete_me.lockfile'): + print('{} has acquired the lock'.format(pid)) + time.sleep(hold_time.total_seconds()) + print('{} is releasing the lock'.format(pid)) + print('{} done.'.format(pid)) + + +if __name__ == '__main__': + lock_hold_time = datetime.timedelta(seconds=5) + if len(sys.argv) > 1: + hold_time = datetime.timedelta(seconds=int(sys.argv[1])) + main(lock_hold_time) diff --git a/tests/test_crossplatlock.py b/tests/test_crossplatlock.py new file mode 100644 index 0000000..ea3c9d5 --- /dev/null +++ b/tests/test_crossplatlock.py @@ -0,0 +1,18 @@ +import pytest +from msal_extensions.cache_lock import CrossPlatLock + + +def test_ensure_file_deleted(): + lockfile = './test_lock_1.txt' + + try: + FileNotFoundError + except NameError: + FileNotFoundError = IOError + + with CrossPlatLock(lockfile): + pass + + with pytest.raises(FileNotFoundError): + with open(lockfile): + pass From ff8daaffb9a9272e757d20e9d6d4be86aa0c1bf2 Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Tue, 21 May 2019 14:18:50 -0700 Subject: [PATCH 03/13] Add bare minimum structure for a CI system --- azure-pipelines.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 azure-pipelines.yaml diff --git a/azure-pipelines.yaml b/azure-pipelines.yaml new file mode 100644 index 0000000..b665bbf --- /dev/null +++ b/azure-pipelines.yaml @@ -0,0 +1,8 @@ +resources: + - repo: self + +trigger: + batch: true + branches: + include: + - '*' From bc47535b9ead38fd523031108112ac3b3c27b7bc Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Tue, 21 May 2019 14:33:42 -0700 Subject: [PATCH 04/13] Fix Azure-Pipelines Manifest file name --- azure-pipelines.yaml => azure-pipelines.yml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename azure-pipelines.yaml => azure-pipelines.yml (100%) diff --git a/azure-pipelines.yaml b/azure-pipelines.yml similarity index 100% rename from azure-pipelines.yaml rename to azure-pipelines.yml From 3667b2ad71267e46e7bc3a03044b573ad58101af Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Wed, 22 May 2019 10:45:09 -0700 Subject: [PATCH 05/13] Add TravisCI (#18) * Add TravisCI * Fixing no-newline nit * Auto-complete error * Fix pylint syntax * Remove duplicate linux jobs * Fix pylint violations * Updating suppression mechanism * Fixing Python2 compat bug * Set distro to xenial * Fixing non-Linux TravisCI support I had to follow this guidance: https://docs.travis-ci.com/user/languages/python/#running-python-tests-on-multiple-operating-systems * Add Windows Python2 * Experimentally change chocolately package name * Fix chocolatey version spec * Add Windows 3.5 to matrix * Fix DocString --- .gitignore | 1 + .pylintrc | 3 +++ .travis.yml | 46 +++++++++++++++++++++++++++++++++++ msal_extensions/__init__.py | 1 + msal_extensions/cache_lock.py | 17 +++++++------ tox.ini | 7 ++++++ 6 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 .pylintrc create mode 100644 .travis.yml create mode 100644 tox.ini diff --git a/.gitignore b/.gitignore index 69b1e0e..b9e6255 100644 --- a/.gitignore +++ b/.gitignore @@ -300,6 +300,7 @@ __pycache__/ # Python Auxiliary Tools *.egg-info/ +.tox/ # Cake - Uncomment if you are using it # tools/** diff --git a/.pylintrc b/.pylintrc new file mode 100644 index 0000000..3fa50bb --- /dev/null +++ b/.pylintrc @@ -0,0 +1,3 @@ +[MESSAGES CONTROL] +disable= + useless-object-inheritance diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000..96b68fd --- /dev/null +++ b/.travis.yml @@ -0,0 +1,46 @@ +language: python + +matrix: + fast_finish: true + include: + - python: "2.7" + env: TOXENV=py27 + os: linux + - python: "3.5" + env: TOXENV=py35 + os: linux + - python: "3.6" + env: TOXENV=py36 + os: linux + - python: "3.7" + env: TOXENV=py37 + os: linux + dist: xenial + - name: "Python 3.7 on macOS" + env: TOXENV=py37 + os: osx + osx_image: xcode10.2 + language: shell + - name: "Python 2.7 on Windows" + env: TOXENV=py27 PATH=/c/Python27:/c/Python27/Scripts:$PATH + os: windows + before_install: choco install python2 + language: shell + - name: "Python 3.5 on Windows" + env: TOXENV=py35 PATH=/c/Python35:/c/Python35/Scripts:$PATH + os: windows + before_install: choco install python3 --version 3.5.4 + language: shell + - name: "Python 3.7 on Windows" + env: TOXENV=py37 PATH=/c/Python37:/c/Python37/Scripts:$PATH + os: windows + before_install: choco install python3 --version 3.7.3 + language: shell + +install: + - pip install tox pylint + - pip install . + +script: + - pylint msal_extensions + - tox diff --git a/msal_extensions/__init__.py b/msal_extensions/__init__.py index f102a9c..fcb4bb7 100644 --- a/msal_extensions/__init__.py +++ b/msal_extensions/__init__.py @@ -1 +1,2 @@ +"""Provides auxiliary functionality to the `msal` package.""" __version__ = "0.0.1" diff --git a/msal_extensions/cache_lock.py b/msal_extensions/cache_lock.py index 9ef90fd..2a73f24 100644 --- a/msal_extensions/cache_lock.py +++ b/msal_extensions/cache_lock.py @@ -1,3 +1,4 @@ +"""Provides a mechanism for not competing with other processes interacting with an MSAL cache.""" import os import sys import errno @@ -5,12 +6,13 @@ class CrossPlatLock(object): - """ - Offers a mechanism for waiting until another process is finished interacting with a shared resource. This is - specifically written to interact with a class of the same name in the .NET extensions library. + """Offers a mechanism for waiting until another process is finished interacting with a shared + resource. This is specifically written to interact with a class of the same name in the .NET + extensions library. """ def __init__(self, lockfile_path): self._lockpath = lockfile_path + self._fh = None def __enter__(self): pid = os.getpid() @@ -22,11 +24,10 @@ def __enter__(self): def __exit__(self, *args): self._fh.close() 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 file for itself. + # 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 + # file for itself. os.remove(self._lockpath) - except PermissionError: - pass except OSError as ex: - if ex.errno != errno.ENOENT: + if ex.errno != errno.ENOENT and ex.errno != errno.EACCES: raise diff --git a/tox.ini b/tox.ini new file mode 100644 index 0000000..a442990 --- /dev/null +++ b/tox.ini @@ -0,0 +1,7 @@ +[tox] +envlist = py27,py35,py36,py37 + +[testenv] +deps = pytest +commands = + pytest From b7c303fec9ec81eb711d6f030078ffd1c6edf3ea Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Wed, 22 May 2019 11:39:32 -0700 Subject: [PATCH 06/13] Add DP API Cache (#14) * Add DP API Cache * Removing superfluous underscores in Windows components. * Make DATA_BLOB.raw idempotent * WindowsTokenCache._has_state_changed -> WindowsTokenCache.needs_refresh * Revert "Make DATA_BLOB.raw idempotent" This reverts commit b9becbcd8d1accd77427f71afe825636fa134bc2. Once tested on a Windows box (read, actually tested) it demonstrated that this change causes a seg-fault and crash on Windows. * Broadening scope of lock * Refactoring WindowsDataProtectionAgent constructor * Refactor WindowsTokenCache constructor * Rename Windows read_msal_cache test * Add WindowsTokenCache roundtrip test * Removing unused import * WindowsTokenCache.needs_refresh -> WindowsTokenCache._needs_refresh * Use kwargs for WindowsTokenCache.find * Fixing some pylint errors * Delete C allocated memory when DataBlob is GC'd * Pulling _local_free out of DataBlob.raw Putting LocalFree in `raw` in the first place was a mistake, and represented a fundamental misunderstanding of DataBlob's memory management story. The memory it points at must be managed independently of the DataBlob's lifecycle. --- msal_extensions/windows.py | 189 ++++++++++++++++++++++++++++++++++ setup.py | 1 + tests/test_windows_backend.py | 94 +++++++++++++++++ 3 files changed, 284 insertions(+) create mode 100644 msal_extensions/windows.py create mode 100644 tests/test_windows_backend.py diff --git a/msal_extensions/windows.py b/msal_extensions/windows.py new file mode 100644 index 0000000..7dd7611 --- /dev/null +++ b/msal_extensions/windows.py @@ -0,0 +1,189 @@ +import os +import ctypes +from ctypes import wintypes +import time +import errno +import msal +from .cache_lock import CrossPlatLock + +_local_free = ctypes.windll.kernel32.LocalFree +_memcpy = ctypes.cdll.msvcrt.memcpy +_crypt_protect_data = ctypes.windll.crypt32.CryptProtectData +_crypt_unprotect_data = ctypes.windll.crypt32.CryptUnprotectData +_CRYPTPROTECT_UI_FORBIDDEN = 0x01 + + +class DataBlob(ctypes.Structure): + """A wrapper for interacting with the _CRYPTOAPI_BLOB type and its many aliases. This type is + exposed from Wincrypt.h in XP and above. + + The memory associated with a DataBlob itself does not need to be freed, as the Python runtime + will correctly clean it up. However, depending on the data it points at, it may still need to be + freed. For instance, memory created by ctypes.create_string_buffer is already managed, and needs + to not be freed. However, memory allocated by CryptProtectData and CryptUnprotectData must have + LocalFree called on pbData. + + See documentation for this type at: + https://msdn.microsoft.com/en-us/7a06eae5-96d8-4ece-98cb-cf0710d2ddbd + """ + _fields_ = [("cbData", wintypes.DWORD), ("pbData", ctypes.POINTER(ctypes.c_char))] + + def raw(self): + # type: () -> bytes + cb_data = int(self.cbData) + pb_data = self.pbData + buffer = ctypes.create_string_buffer(cb_data) + _memcpy(buffer, pb_data, cb_data) + return buffer.raw + + +# This code is modeled from a StackOverflow question, which can be found here: +# https://stackoverflow.com/questions/463832/using-dpapi-with-python +class WindowsDataProtectionAgent(object): + + def __init__(self, entropy=None): + # type: (str) -> None + self._entropy_blob = None + if entropy: + entropy_utf8 = entropy.encode('utf-8') + buffer = ctypes.create_string_buffer(entropy_utf8, len(entropy_utf8)) + self._entropy_blob = DataBlob(len(entropy_utf8), buffer) + + def protect(self, message): + # type: (str) -> bytes + + message = message.encode('utf-8') + message_buffer = ctypes.create_string_buffer(message, len(message)) + message_blob = DataBlob(len(message), message_buffer) + result = DataBlob() + + if self._entropy_blob: + entropy = ctypes.byref(self._entropy_blob) + else: + entropy = None + + if _crypt_protect_data( + ctypes.byref(message_blob), + u"python_data", + entropy, + None, + None, + _CRYPTPROTECT_UI_FORBIDDEN, + ctypes.byref(result)): + try: + return result.raw() + finally: + _local_free(result.pbData) + return b'' + + def unprotect(self, cipher_text): + # type: (bytes) -> str + ct_buffer = ctypes.create_string_buffer(cipher_text, len(cipher_text)) + ct_blob = DataBlob(len(cipher_text), ct_buffer) + result = DataBlob() + + if self._entropy_blob: + entropy = ctypes.byref(self._entropy_blob) + else: + entropy = None + + if _crypt_unprotect_data( + ctypes.byref(ct_blob), + None, + entropy, + None, + None, + _CRYPTPROTECT_UI_FORBIDDEN, + ctypes.byref(result) + ): + try: + return result.raw().decode('utf-8') + finally: + _local_free(result.pbData) + return u'' + + +class WindowsTokenCache(msal.SerializableTokenCache): + """A SerializableTokenCache implementation which uses Win32 encryption APIs to protect your + tokens. + """ + def __init__(self, + cache_location=os.path.join( + os.getenv('LOCALAPPDATA'), + '.IdentityService', + 'msal.cache'), + entropy=''): + super(WindowsTokenCache, self).__init__() + + self._cache_location = cache_location + self._lock_location = self._cache_location + '.lockfile' + self._dp_agent = WindowsDataProtectionAgent(entropy=entropy) + self._last_sync = 0 # _last_sync is a Unixtime + + 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. + """ + try: + return self._last_sync < os.path.getmtime(self._cache_location) + except OSError as exp: + if exp.errno != errno.ENOENT: + raise exp + return False + + def add(self, event, **kwargs): + with CrossPlatLock(self._lock_location): + if self._needs_refresh(): + try: + self._read() + except OSError as exp: + if exp.errno != errno.ENOENT: + raise exp + super(WindowsTokenCache, self).add(event, **kwargs) + self._write() + + def update_rt(self, rt_item, new_rt): + with CrossPlatLock(self._lock_location): + if self._needs_refresh(): + try: + self._read() + except OSError as exp: + if exp.errno != errno.ENOENT: + raise exp + super(WindowsTokenCache, self).update_rt(rt_item, new_rt) + self._write() + + def remove_rt(self, rt_item): + with CrossPlatLock(self._lock_location): + if self._needs_refresh(): + try: + self._read() + except OSError as exp: + if exp.errno != errno.ENOENT: + raise exp + super(WindowsTokenCache, self).remove_rt(rt_item) + self._write() + + def find(self, credential_type, **kwargs): + with CrossPlatLock(self._lock_location): + if self._needs_refresh(): + try: + self._read() + except OSError as exp: + if exp.errno != errno.ENOENT: + raise exp + return super(WindowsTokenCache, self).find(credential_type, **kwargs) + + def _write(self): + with open(self._cache_location, 'wb') as handle: + handle.write(self._dp_agent.protect(self.serialize())) + self._last_sync = int(time.time()) + + def _read(self): + with open(self._cache_location, 'rb') as handle: + cipher_text = handle.read() + contents = self._dp_agent.unprotect(cipher_text) + self.deserialize(contents) + self._last_sync = int(time.time()) diff --git a/setup.py b/setup.py index cbfe080..f69bc89 100644 --- a/setup.py +++ b/setup.py @@ -16,6 +16,7 @@ 'Development Status :: 2 - Pre-Alpha', ], install_requires=[ + 'msal~=0.3', 'portalocker~=1.0', ], tests_require=['pytest'], diff --git a/tests/test_windows_backend.py b/tests/test_windows_backend.py new file mode 100644 index 0000000..2999c97 --- /dev/null +++ b/tests/test_windows_backend.py @@ -0,0 +1,94 @@ +import sys +import os +import shutil +import tempfile +import pytest +import uuid +import msal + +if not sys.platform.startswith('win'): + pytest.skip('skipping windows-only tests', allow_module_level=True) +else: + from msal_extensions.windows import WindowsDataProtectionAgent, WindowsTokenCache + + +def test_dpapi_roundtrip_with_entropy(): + subject_without_entropy = WindowsDataProtectionAgent() + subject_with_entropy = WindowsDataProtectionAgent(entropy=uuid.uuid4().hex) + + test_cases = [ + '', + 'lorem ipsum', + 'lorem-ipsum', + '', + uuid.uuid4().hex, + ] + + for tc in test_cases: + ciphered = subject_with_entropy.protect(tc) + assert ciphered != tc + + got = subject_with_entropy.unprotect(ciphered) + assert got == tc + + ciphered = subject_without_entropy.protect(tc) + assert ciphered != tc + + got = subject_without_entropy.unprotect(ciphered) + assert got == tc + + +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`. + """ + cache_locations = [ + os.path.join(os.getenv('LOCALAPPDATA'), '.IdentityService', 'msal.cache'), # this is where it's supposed to be + os.path.join(os.getenv('LOCALAPPDATA'), '.IdentityServices', 'msal.cache'), # There was a miscommunications about whether this was plural or not. + os.path.join(os.getenv('LOCALAPPDATA'), 'msal.cache'), # The earliest most naive builds used this locations. + ] + + found = False + for loc in cache_locations: + try: + with open(loc, mode='rb') as fh: + contents = fh.read() + found = True + break + except FileNotFoundError: + pass + if not found: + pytest.skip('could not find the msal.cache file (try logging in using MSAL)') + + subject = WindowsDataProtectionAgent() + raw = subject.unprotect(contents) + assert raw != "" + + cache = msal.SerializableTokenCache() + cache.deserialize(raw) + access_tokens = cache.find(msal.TokenCache.CredentialType.ACCESS_TOKEN) + assert len(access_tokens) > 0 + + +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.') + + 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) + 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) From fc5b5b70043164728438067195ed0ba60beeb372 Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Wed, 22 May 2019 14:37:46 -0700 Subject: [PATCH 07/13] Fixing all remaining pylint errors in windows.py (#20) * Fixing all remaining pylint errors in windows.py * Fixing variables named buffer so they don't conflict with python2 names. --- msal_extensions/windows.py | 39 +++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/msal_extensions/windows.py b/msal_extensions/windows.py index 7dd7611..ba5870d 100644 --- a/msal_extensions/windows.py +++ b/msal_extensions/windows.py @@ -1,3 +1,4 @@ +"""Implements a Windows Specific TokenCache, and provides auxiliary helper types.""" import os import ctypes from ctypes import wintypes @@ -6,14 +7,14 @@ import msal from .cache_lock import CrossPlatLock -_local_free = ctypes.windll.kernel32.LocalFree -_memcpy = ctypes.cdll.msvcrt.memcpy -_crypt_protect_data = ctypes.windll.crypt32.CryptProtectData -_crypt_unprotect_data = ctypes.windll.crypt32.CryptUnprotectData +_LOCAL_FREE = ctypes.windll.kernel32.LocalFree +_MEMCPY = ctypes.cdll.msvcrt.memcpy +_CRYPT_PROTECT_DATA = ctypes.windll.crypt32.CryptProtectData +_CRYPT_UNPROTECT_DATA = ctypes.windll.crypt32.CryptUnprotectData _CRYPTPROTECT_UI_FORBIDDEN = 0x01 -class DataBlob(ctypes.Structure): +class DataBlob(ctypes.Structure): # pylint: disable=too-few-public-methods """A wrapper for interacting with the _CRYPTOAPI_BLOB type and its many aliases. This type is exposed from Wincrypt.h in XP and above. @@ -30,27 +31,33 @@ class DataBlob(ctypes.Structure): def raw(self): # type: () -> bytes + """Copies the message from the DataBlob in natively allocated memory into Python controlled + memory. + :return A byte array that matches what is stored in native-memory.""" cb_data = int(self.cbData) pb_data = self.pbData - buffer = ctypes.create_string_buffer(cb_data) - _memcpy(buffer, pb_data, cb_data) - return buffer.raw + blob_buffer = ctypes.create_string_buffer(cb_data) + _MEMCPY(blob_buffer, pb_data, cb_data) + return blob_buffer.raw # This code is modeled from a StackOverflow question, which can be found here: # https://stackoverflow.com/questions/463832/using-dpapi-with-python class WindowsDataProtectionAgent(object): + """A mechanism for interacting with the Windows DP API Native library, e.g. Crypt32.dll.""" def __init__(self, entropy=None): # type: (str) -> None self._entropy_blob = None if entropy: entropy_utf8 = entropy.encode('utf-8') - buffer = ctypes.create_string_buffer(entropy_utf8, len(entropy_utf8)) - self._entropy_blob = DataBlob(len(entropy_utf8), buffer) + blob_buffer = ctypes.create_string_buffer(entropy_utf8, len(entropy_utf8)) + self._entropy_blob = DataBlob(len(entropy_utf8), blob_buffer) def protect(self, message): # type: (str) -> bytes + """Encrypts a message. + :return cipher text holding the original message.""" message = message.encode('utf-8') message_buffer = ctypes.create_string_buffer(message, len(message)) @@ -62,7 +69,7 @@ def protect(self, message): else: entropy = None - if _crypt_protect_data( + if _CRYPT_PROTECT_DATA( ctypes.byref(message_blob), u"python_data", entropy, @@ -73,11 +80,13 @@ def protect(self, message): try: return result.raw() finally: - _local_free(result.pbData) + _LOCAL_FREE(result.pbData) return b'' def unprotect(self, cipher_text): # type: (bytes) -> str + """Decrypts cipher text that is provided. + :return The original message hidden in the cipher text.""" ct_buffer = ctypes.create_string_buffer(cipher_text, len(cipher_text)) ct_blob = DataBlob(len(cipher_text), ct_buffer) result = DataBlob() @@ -87,7 +96,7 @@ def unprotect(self, cipher_text): else: entropy = None - if _crypt_unprotect_data( + if _CRYPT_UNPROTECT_DATA( ctypes.byref(ct_blob), None, entropy, @@ -99,7 +108,7 @@ def unprotect(self, cipher_text): try: return result.raw().decode('utf-8') finally: - _local_free(result.pbData) + _LOCAL_FREE(result.pbData) return u'' @@ -166,7 +175,7 @@ def remove_rt(self, rt_item): super(WindowsTokenCache, self).remove_rt(rt_item) self._write() - def find(self, credential_type, **kwargs): + def find(self, credential_type, **kwargs): # pylint: disable=arguments-differ with CrossPlatLock(self._lock_location): if self._needs_refresh(): try: From 61c5d34ce7bcaf53d48a640831560e5e11f70c34 Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Thu, 23 May 2019 14:58:02 -0700 Subject: [PATCH 08/13] Adding default value for LOCALAPPDATA (#19) * Adding default value for LOCALAPPDATA * Responding to review feedback * Add default LOCALAPPDATA to test file as well * Remove reference to Python3-only type in test * Raise an error instead of returning an empty value. * Correct syntax for using winerr parameter * Fixing OSError syntax to not include named arguments * Chaning to IOError to fix Python2 some more * Suppressing remaining error * Editting fallback error-code --- msal_extensions/windows.py | 20 +++++++++++------- tests/test_windows_backend.py | 39 ++++++++++++++++++++++------------- 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/msal_extensions/windows.py b/msal_extensions/windows.py index ba5870d..528d51f 100644 --- a/msal_extensions/windows.py +++ b/msal_extensions/windows.py @@ -8,6 +8,7 @@ from .cache_lock import CrossPlatLock _LOCAL_FREE = ctypes.windll.kernel32.LocalFree +_GET_LAST_ERROR = ctypes.windll.kernel32.GetLastError _MEMCPY = ctypes.cdll.msvcrt.memcpy _CRYPT_PROTECT_DATA = ctypes.windll.crypt32.CryptProtectData _CRYPT_UNPROTECT_DATA = ctypes.windll.crypt32.CryptUnprotectData @@ -81,7 +82,9 @@ def protect(self, message): return result.raw() finally: _LOCAL_FREE(result.pbData) - return b'' + + err_code = _GET_LAST_ERROR() + raise OSError(256, '', '', err_code) def unprotect(self, cipher_text): # type: (bytes) -> str @@ -109,7 +112,8 @@ def unprotect(self, cipher_text): return result.raw().decode('utf-8') finally: _LOCAL_FREE(result.pbData) - return u'' + err_code = _GET_LAST_ERROR() + raise OSError(256, '', '', err_code) class WindowsTokenCache(msal.SerializableTokenCache): @@ -118,7 +122,7 @@ class WindowsTokenCache(msal.SerializableTokenCache): """ def __init__(self, cache_location=os.path.join( - os.getenv('LOCALAPPDATA'), + os.getenv('LOCALAPPDATA', os.path.expanduser('~')), '.IdentityService', 'msal.cache'), entropy=''): @@ -137,7 +141,7 @@ def _needs_refresh(self): """ try: return self._last_sync < os.path.getmtime(self._cache_location) - except OSError as exp: + except IOError as exp: if exp.errno != errno.ENOENT: raise exp return False @@ -147,7 +151,7 @@ def add(self, event, **kwargs): if self._needs_refresh(): try: self._read() - except OSError as exp: + except IOError as exp: if exp.errno != errno.ENOENT: raise exp super(WindowsTokenCache, self).add(event, **kwargs) @@ -158,7 +162,7 @@ def update_rt(self, rt_item, new_rt): if self._needs_refresh(): try: self._read() - except OSError as exp: + except IOError as exp: if exp.errno != errno.ENOENT: raise exp super(WindowsTokenCache, self).update_rt(rt_item, new_rt) @@ -169,7 +173,7 @@ def remove_rt(self, rt_item): if self._needs_refresh(): try: self._read() - except OSError as exp: + except IOError as exp: if exp.errno != errno.ENOENT: raise exp super(WindowsTokenCache, self).remove_rt(rt_item) @@ -180,7 +184,7 @@ def find(self, credential_type, **kwargs): # pylint: disable=arguments-differ if self._needs_refresh(): try: self._read() - except OSError as exp: + except IOError as exp: if exp.errno != errno.ENOENT: raise exp return super(WindowsTokenCache, self).find(credential_type, **kwargs) diff --git a/tests/test_windows_backend.py b/tests/test_windows_backend.py index 2999c97..6ff822e 100644 --- a/tests/test_windows_backend.py +++ b/tests/test_windows_backend.py @@ -1,5 +1,6 @@ import sys import os +import errno import shutil import tempfile import pytest @@ -24,18 +25,24 @@ def test_dpapi_roundtrip_with_entropy(): uuid.uuid4().hex, ] - for tc in test_cases: - ciphered = subject_with_entropy.protect(tc) - assert ciphered != tc + try: + for tc in test_cases: + ciphered = subject_with_entropy.protect(tc) + assert ciphered != tc - got = subject_with_entropy.unprotect(ciphered) - assert got == tc + got = subject_with_entropy.unprotect(ciphered) + assert got == tc - ciphered = subject_without_entropy.protect(tc) - assert ciphered != tc + ciphered = subject_without_entropy.protect(tc) + assert ciphered != tc - got = subject_without_entropy.unprotect(ciphered) - assert got == tc + got = subject_without_entropy.unprotect(ciphered) + assert got == tc + except OSError as exp: + if exp.errno == errno.EIO and os.getenv('TRAVIS_REPO_SLUG'): + pytest.skip('DPAPI tests are known to fail in TravisCI. This effort tracked by ' + 'https://github.com/AzureAD/microsoft-authentication-extentions-for-python' + '/issues/21') def test_read_msal_cache_direct(): @@ -43,10 +50,11 @@ 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`. """ + localappdata_location = os.getenv('LOCALAPPDATA', os.path.expanduser('~')) cache_locations = [ - os.path.join(os.getenv('LOCALAPPDATA'), '.IdentityService', 'msal.cache'), # this is where it's supposed to be - os.path.join(os.getenv('LOCALAPPDATA'), '.IdentityServices', 'msal.cache'), # There was a miscommunications about whether this was plural or not. - os.path.join(os.getenv('LOCALAPPDATA'), 'msal.cache'), # The earliest most naive builds used this locations. + os.path.join(localappdata_location, '.IdentityService', 'msal.cache'), # this is where it's supposed to be + os.path.join(localappdata_location, '.IdentityServices', 'msal.cache'), # There was a miscommunications about whether this was plural or not. + os.path.join(localappdata_location, 'msal.cache'), # The earliest most naive builds used this locations. ] found = False @@ -55,9 +63,12 @@ def test_read_msal_cache_direct(): with open(loc, mode='rb') as fh: contents = fh.read() found = True + break - except FileNotFoundError: - pass + except IOError as exp: + if exp.errno != errno.ENOENT: + raise exp + if not found: pytest.skip('could not find the msal.cache file (try logging in using MSAL)') From 64cb419c685f06f0aaf9f00d983995fa3fe00bbb Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Fri, 24 May 2019 14:48:21 -0700 Subject: [PATCH 09/13] Adopt msal~=0.4 (#23) * Increase version in setup.py * Replacing `remove_rt` and `update_rt` with `modify` * Making argument types consistent. * Formatting fix for pylint --- msal_extensions/windows.py | 18 +++++------------- setup.py | 2 +- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/msal_extensions/windows.py b/msal_extensions/windows.py index 528d51f..0e529f1 100644 --- a/msal_extensions/windows.py +++ b/msal_extensions/windows.py @@ -157,7 +157,7 @@ def add(self, event, **kwargs): super(WindowsTokenCache, self).add(event, **kwargs) self._write() - def update_rt(self, rt_item, new_rt): + def modify(self, credential_type, old_entry, new_key_value_pairs=None): with CrossPlatLock(self._lock_location): if self._needs_refresh(): try: @@ -165,18 +165,10 @@ def update_rt(self, rt_item, new_rt): except IOError as exp: if exp.errno != errno.ENOENT: raise exp - super(WindowsTokenCache, self).update_rt(rt_item, new_rt) - self._write() - - def remove_rt(self, rt_item): - with CrossPlatLock(self._lock_location): - if self._needs_refresh(): - try: - self._read() - except IOError as exp: - if exp.errno != errno.ENOENT: - raise exp - super(WindowsTokenCache, self).remove_rt(rt_item) + super(WindowsTokenCache, self).modify( + credential_type, + old_entry, + new_key_value_pairs=new_key_value_pairs) self._write() def find(self, credential_type, **kwargs): # pylint: disable=arguments-differ diff --git a/setup.py b/setup.py index f69bc89..ce287c8 100644 --- a/setup.py +++ b/setup.py @@ -16,7 +16,7 @@ 'Development Status :: 2 - Pre-Alpha', ], install_requires=[ - 'msal~=0.3', + 'msal~=0.4', 'portalocker~=1.0', ], tests_require=['pytest'], From 6bc51417162e859eccad7d750d13070555331834 Mon Sep 17 00:00:00 2001 From: Martin Strobel Date: Tue, 25 Jun 2019 12:08:56 -0700 Subject: [PATCH 10/13] Add KeyChain Cache, Plain-Text File Cache, and Cache Factory (#15) * Add KeyChain Cache * Use **kwargs in OSXTokenCache.find * Fixing pylint errors * Use CrossPlatLock in OSX Cache * Adding partially implemented cache MUX * Overload modify instead of more specific commands * Squash warnings about duplicate code * Fix pylint error about unnecessary `elif` statements. * Fixing line formatting err from pylint * Add missing doc-strings as noted by pylint * Revert "Squash warnings about duplicate code" This reverts commit f8f61bd08492100ebeef3e3213c01e0c38bfb30c. * Pull duplicated code into single unprotected SerializableTokenCache implementation that can be overriden * Move all token cache implementations into .token_cache Previously, implementations lived in their platform specific files. However, this created an import cycle that pylint was complaining about. * Fixing formatting nit * Mux on generic form of FileTokenCache as fallback plan * Removing redundant test * Fix documentation typo * Making directory creation call in _mkdir_p recursive. * Restructuring what occurs in _read/_write and the other modifiers * Fix kwargs use in constructors * Fix invalid/inaccurate file mode for agnostic backend * Add test for plain file token cache. * Moving error handling out of _read and _write. This accomplishes a couple of things: - Establishes that _read and _write SHOULD error out if they are unable to do their job. This way different places in code base can choose what is and is not an acceptable place to fail a read or write operation. - Keeps the contract that derived class implementers need to fulfill very simple. * Suppress pylint error caused by difference in parameters, which is done strategically with kwargs * Removing duplicated line * Brining all Keychain Error constructors into the same shape. * Don't defer failure of OS module loads * Remove child error-types * Do token cache switching at import time instead of having a separate factory --- msal_extensions/__init__.py | 9 ++ msal_extensions/osx.py | 253 +++++++++++++++++++++++++++++++++ msal_extensions/token_cache.py | 170 ++++++++++++++++++++++ msal_extensions/windows.py | 83 ----------- tests/test_agnostic_backend.py | 54 +++++++ tests/test_macos_backend.py | 45 ++++++ tests/test_windows_backend.py | 3 +- 7 files changed, 533 insertions(+), 84 deletions(-) create mode 100644 msal_extensions/osx.py create mode 100644 msal_extensions/token_cache.py create mode 100644 tests/test_agnostic_backend.py create mode 100644 tests/test_macos_backend.py diff --git a/msal_extensions/__init__.py b/msal_extensions/__init__.py index fcb4bb7..7c5f4a6 100644 --- a/msal_extensions/__init__.py +++ b/msal_extensions/__init__.py @@ -1,2 +1,11 @@ """Provides auxiliary functionality to the `msal` package.""" __version__ = "0.0.1" + +import sys + +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 diff --git a/msal_extensions/osx.py b/msal_extensions/osx.py new file mode 100644 index 0000000..33f85e9 --- /dev/null +++ b/msal_extensions/osx.py @@ -0,0 +1,253 @@ +# pylint: disable=duplicate-code + +"""Implements a macOS specific TokenCache, and provides auxiliary helper types.""" + +import os +import ctypes as _ctypes + +OS_RESULT = _ctypes.c_int32 + + +class KeychainError(OSError): + """The RuntimeError that will be run when a function interacting with Keychain fails.""" + + ACCESS_DENIED = -128 + NO_SUCH_KEYCHAIN = -25294 + NO_DEFAULT = -25307 + ITEM_NOT_FOUND = -25300 + + def __init__(self, exit_status): + super(KeychainError, self).__init__() + self.exit_status = exit_status + # TODO: pylint: disable=fixme + # use SecCopyErrorMessageString to fetch the appropriate message here. + self.message = \ + '{} ' \ + 'see https://opensource.apple.com/source/CarbonHeaders/CarbonHeaders-18.1/MacErrors.h'\ + .format(self.exit_status) + +def _get_native_location(name): + # type: (str) -> str + """ + Fetches the location of a native MacOS library. + :param name: The name of the library to be loaded. + :return: The location of the library on a MacOS filesystem. + """ + return '/System/Library/Frameworks/{0}.framework/{0}'.format(name) + + +# Load native MacOS libraries +_SECURITY = _ctypes.CDLL(_get_native_location('Security')) +_CORE = _ctypes.CDLL(_get_native_location('CoreFoundation')) + + +# Bind CFRelease from native MacOS libraries. +_CORE_RELEASE = _CORE.CFRelease +_CORE_RELEASE.argtypes = ( + _ctypes.c_void_p, +) + +# Bind SecCopyErrorMessageString from native MacOS libraries. +# https://developer.apple.com/documentation/security/1394686-seccopyerrormessagestring?language=objc +_SECURITY_COPY_ERROR_MESSAGE_STRING = _SECURITY.SecCopyErrorMessageString +_SECURITY_COPY_ERROR_MESSAGE_STRING.argtypes = ( + OS_RESULT, + _ctypes.c_void_p +) +_SECURITY_COPY_ERROR_MESSAGE_STRING.restype = _ctypes.c_char_p + +# Bind SecKeychainOpen from native MacOS libraries. +# https://developer.apple.com/documentation/security/1396431-seckeychainopen +_SECURITY_KEYCHAIN_OPEN = _SECURITY.SecKeychainOpen +_SECURITY_KEYCHAIN_OPEN.argtypes = ( + _ctypes.c_char_p, + _ctypes.POINTER(_ctypes.c_void_p) +) +_SECURITY_KEYCHAIN_OPEN.restype = OS_RESULT + +# Bind SecKeychainCopyDefault from native MacOS libraries. +# https://developer.apple.com/documentation/security/1400743-seckeychaincopydefault?language=objc +_SECURITY_KEYCHAIN_COPY_DEFAULT = _SECURITY.SecKeychainCopyDefault +_SECURITY_KEYCHAIN_COPY_DEFAULT.argtypes = ( + _ctypes.POINTER(_ctypes.c_void_p), +) +_SECURITY_KEYCHAIN_COPY_DEFAULT.restype = OS_RESULT + + +# Bind SecKeychainItemFreeContent from native MacOS libraries. +_SECURITY_KEYCHAIN_ITEM_FREE_CONTENT = _SECURITY.SecKeychainItemFreeContent +_SECURITY_KEYCHAIN_ITEM_FREE_CONTENT.argtypes = ( + _ctypes.c_void_p, + _ctypes.c_void_p, +) +_SECURITY_KEYCHAIN_ITEM_FREE_CONTENT.restype = OS_RESULT + +# Bind SecKeychainItemModifyAttributesAndData from native MacOS libraries. +_SECURITY_KEYCHAIN_ITEM_MODIFY_ATTRIBUTES_AND_DATA = \ + _SECURITY.SecKeychainItemModifyAttributesAndData +_SECURITY_KEYCHAIN_ITEM_MODIFY_ATTRIBUTES_AND_DATA.argtypes = ( + _ctypes.c_void_p, + _ctypes.c_void_p, + _ctypes.c_uint32, + _ctypes.c_void_p, +) +_SECURITY_KEYCHAIN_ITEM_MODIFY_ATTRIBUTES_AND_DATA.restype = OS_RESULT + +# Bind SecKeychainFindGenericPassword from native MacOS libraries. +# https://developer.apple.com/documentation/security/1397301-seckeychainfindgenericpassword?language=objc +_SECURITY_KEYCHAIN_FIND_GENERIC_PASSWORD = _SECURITY.SecKeychainFindGenericPassword +_SECURITY_KEYCHAIN_FIND_GENERIC_PASSWORD.argtypes = ( + _ctypes.c_void_p, + _ctypes.c_uint32, + _ctypes.c_char_p, + _ctypes.c_uint32, + _ctypes.c_char_p, + _ctypes.POINTER(_ctypes.c_uint32), + _ctypes.POINTER(_ctypes.c_void_p), + _ctypes.POINTER(_ctypes.c_void_p), +) +_SECURITY_KEYCHAIN_FIND_GENERIC_PASSWORD.restype = OS_RESULT +# Bind SecKeychainAddGenericPassword from native MacOS +# https://developer.apple.com/documentation/security/1398366-seckeychainaddgenericpassword?language=objc +_SECURITY_KEYCHAIN_ADD_GENERIC_PASSWORD = _SECURITY.SecKeychainAddGenericPassword +_SECURITY_KEYCHAIN_ADD_GENERIC_PASSWORD.argtypes = ( + _ctypes.c_void_p, + _ctypes.c_uint32, + _ctypes.c_char_p, + _ctypes.c_uint32, + _ctypes.c_char_p, + _ctypes.c_uint32, + _ctypes.c_char_p, + _ctypes.POINTER(_ctypes.c_void_p), +) +_SECURITY_KEYCHAIN_ADD_GENERIC_PASSWORD.restype = OS_RESULT + + +class Keychain(object): + """Encapsulates the interactions with a particular MacOS Keychain.""" + def __init__(self, filename=None): + # type: (str) -> None + self._ref = _ctypes.c_void_p() + + if filename: + filename = os.path.expanduser(filename) + self._filename = filename.encode('utf-8') + else: + self._filename = None + + def __enter__(self): + if self._filename: + status = _SECURITY_KEYCHAIN_OPEN(self._filename, self._ref) + else: + status = _SECURITY_KEYCHAIN_COPY_DEFAULT(self._ref) + + if status: + raise OSError(status) + return self + + def __exit__(self, *args): + if self._ref: + _CORE_RELEASE(self._ref) + + def get_generic_password(self, service, account_name): + # type: (str, str) -> str + """Fetch the password associated with a particular service and account. + + :param service: The service that this password is associated with. + :param account_name: The account that this password is associated with. + :return: The value of the password associated with the specified service and account. + """ + service = service.encode('utf-8') + account_name = account_name.encode('utf-8') + + length = _ctypes.c_uint32() + contents = _ctypes.c_void_p() + exit_status = _SECURITY_KEYCHAIN_FIND_GENERIC_PASSWORD( + self._ref, + len(service), + service, + len(account_name), + account_name, + length, + contents, + None, + ) + + if exit_status: + raise KeychainError(exit_status=exit_status) + + value = _ctypes.create_string_buffer(length.value) + _ctypes.memmove(value, contents.value, length.value) + _SECURITY_KEYCHAIN_ITEM_FREE_CONTENT(None, contents) + return value.raw.decode('utf-8') + + def set_generic_password(self, service, account_name, value): + # type: (str, str, str) -> None + """Associate a password with a given service and account. + + :param service: The service to associate this password with. + :param account_name: The account to associate this password with. + :param value: The string that should be used as the password. + """ + service = service.encode('utf-8') + account_name = account_name.encode('utf-8') + value = value.encode('utf-8') + + entry = _ctypes.c_void_p() + find_exit_status = _SECURITY_KEYCHAIN_FIND_GENERIC_PASSWORD( + self._ref, + len(service), + service, + len(account_name), + account_name, + None, + None, + entry, + ) + + if not find_exit_status: + modify_exit_status = _SECURITY_KEYCHAIN_ITEM_MODIFY_ATTRIBUTES_AND_DATA( + entry, + None, + len(value), + value, + ) + if modify_exit_status: + raise KeychainError(exit_status=modify_exit_status) + + elif find_exit_status == KeychainError.ITEM_NOT_FOUND: + add_exit_status = _SECURITY_KEYCHAIN_ADD_GENERIC_PASSWORD( + self._ref, + len(service), + service, + len(account_name), + account_name, + len(value), + value, + None + ) + + if add_exit_status: + raise KeychainError(exit_status=add_exit_status) + else: + raise KeychainError(exit_status=find_exit_status) + + def get_internet_password(self, service, username): + # type: (str, str) -> str + """ Fetches a password associated with a domain and username. + NOTE: THIS IS NOT YET IMPLEMENTED + :param service: The website/service that this password is associated with. + :param username: The account that this password is associated with. + :return: The password that was associated with the given service and username. + """ + raise NotImplementedError() + + def set_internet_password(self, service, username, value): + # type: (str, str, str) -> None + """Sets a password associated with a domain and a username. + NOTE: THIS IS NOT YET IMPLEMENTED + :param service: The website/service that this password is associated with. + :param username: The account that this password is associated with. + :param value: The password that should be associated with the given service and username. + """ + raise NotImplementedError() diff --git a/msal_extensions/token_cache.py b/msal_extensions/token_cache.py new file mode 100644 index 0000000..878ebca --- /dev/null +++ b/msal_extensions/token_cache.py @@ -0,0 +1,170 @@ +"""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 + +if sys.platform.startswith('win'): + from .windows import WindowsDataProtectionAgent +elif sys.platform.startswith('darwin'): + from .osx import Keychain + +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. + """ + try: + os.makedirs(path) + except OSError as exp: + if exp.errno == errno.EEXIST and os.path.isdir(path): + pass + else: + raise + + +class FileTokenCache(msal.SerializableTokenCache): + """Implements basic unprotected SerializableTokenCache to a plain-text file.""" + def __init__(self, + cache_location=os.path.join( + os.getenv('LOCALAPPDATA', os.path.expanduser('~')), + '.IdentityService', + 'msal.cache'), + lock_location=None): + super(FileTokenCache, self).__init__() + self._cache_location = cache_location + self._lock_location = lock_location or self._cache_location + '.lockfile' + self._last_sync = 0 # _last_sync is a Unixtime + + 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. + """ + try: + updated = os.path.getmtime(self._cache_location) + return self._last_sync < updated + 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() + + def add(self, event, **kwargs): + 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).add(event, **kwargs) # pylint: disable=duplicate-code + self._write(self.serialize()) + self._last_sync = os.path.getmtime(self._cache_location) + + 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( + 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) + + 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) + + +class UnencryptedTokenCache(FileTokenCache): + """An unprotected token cache to default to when no-platform specific option is available.""" + def __init__(self, **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__(**kwargs) + + +class WindowsTokenCache(FileTokenCache): + """A SerializableTokenCache implementation which uses Win32 encryption APIs to protect your + tokens. + """ + def __init__(self, entropy='', **kwargs): + super(WindowsTokenCache, self).__init__(**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)) + + 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. + """ + + def __init__(self, + service_name='Microsoft.Developer.IdentityService', + account_name='MSALCache', + **kwargs): + super(OSXTokenCache, self).__init__(**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])) diff --git a/msal_extensions/windows.py b/msal_extensions/windows.py index 0e529f1..479c496 100644 --- a/msal_extensions/windows.py +++ b/msal_extensions/windows.py @@ -1,11 +1,6 @@ """Implements a Windows Specific TokenCache, and provides auxiliary helper types.""" -import os import ctypes from ctypes import wintypes -import time -import errno -import msal -from .cache_lock import CrossPlatLock _LOCAL_FREE = ctypes.windll.kernel32.LocalFree _GET_LAST_ERROR = ctypes.windll.kernel32.GetLastError @@ -114,81 +109,3 @@ def unprotect(self, cipher_text): _LOCAL_FREE(result.pbData) err_code = _GET_LAST_ERROR() raise OSError(256, '', '', err_code) - - -class WindowsTokenCache(msal.SerializableTokenCache): - """A SerializableTokenCache implementation which uses Win32 encryption APIs to protect your - tokens. - """ - def __init__(self, - cache_location=os.path.join( - os.getenv('LOCALAPPDATA', os.path.expanduser('~')), - '.IdentityService', - 'msal.cache'), - entropy=''): - super(WindowsTokenCache, self).__init__() - - self._cache_location = cache_location - self._lock_location = self._cache_location + '.lockfile' - self._dp_agent = WindowsDataProtectionAgent(entropy=entropy) - self._last_sync = 0 # _last_sync is a Unixtime - - 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. - """ - try: - return self._last_sync < os.path.getmtime(self._cache_location) - except IOError as exp: - if exp.errno != errno.ENOENT: - raise exp - return False - - def add(self, event, **kwargs): - with CrossPlatLock(self._lock_location): - if self._needs_refresh(): - try: - self._read() - except IOError as exp: - if exp.errno != errno.ENOENT: - raise exp - super(WindowsTokenCache, self).add(event, **kwargs) - self._write() - - def modify(self, credential_type, old_entry, new_key_value_pairs=None): - with CrossPlatLock(self._lock_location): - if self._needs_refresh(): - try: - self._read() - except IOError as exp: - if exp.errno != errno.ENOENT: - raise exp - super(WindowsTokenCache, self).modify( - credential_type, - old_entry, - new_key_value_pairs=new_key_value_pairs) - self._write() - - def find(self, credential_type, **kwargs): # pylint: disable=arguments-differ - with CrossPlatLock(self._lock_location): - if self._needs_refresh(): - try: - self._read() - except IOError as exp: - if exp.errno != errno.ENOENT: - raise exp - return super(WindowsTokenCache, self).find(credential_type, **kwargs) - - def _write(self): - with open(self._cache_location, 'wb') as handle: - handle.write(self._dp_agent.protect(self.serialize())) - self._last_sync = int(time.time()) - - def _read(self): - with open(self._cache_location, 'rb') as handle: - cipher_text = handle.read() - contents = self._dp_agent.unprotect(cipher_text) - self.deserialize(contents) - self._last_sync = int(time.time()) diff --git a/tests/test_agnostic_backend.py b/tests/test_agnostic_backend.py new file mode 100644 index 0000000..1d9c7d0 --- /dev/null +++ b/tests/test_agnostic_backend.py @@ -0,0 +1,54 @@ +import os +import shutil +import tempfile +import pytest +import msal + + +def test_file_token_cache_roundtrip(): + from msal_extensions.token_cache import FileTokenCache + + 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(): + 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) diff --git a/tests/test_macos_backend.py b/tests/test_macos_backend.py new file mode 100644 index 0000000..c0ca8e1 --- /dev/null +++ b/tests/test_macos_backend.py @@ -0,0 +1,45 @@ +import sys +import os +import shutil +import tempfile +import pytest +import uuid +import msal + +if not sys.platform.startswith('darwin'): + pytest.skip('skipping MacOS-only tests', allow_module_level=True) +else: + from msal_extensions.osx import Keychain + from msal_extensions.token_cache import OSXTokenCache + + +def test_keychain_roundtrip(): + with Keychain() as subject: + location, account = "msal_extension_test1", "test_account1" + want = uuid.uuid4().hex + subject.set_generic_password(location, account, want) + got = subject.get_generic_password(location, account) + assert got == want + + +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.') + + 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) + 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) diff --git a/tests/test_windows_backend.py b/tests/test_windows_backend.py index 6ff822e..240b93d 100644 --- a/tests/test_windows_backend.py +++ b/tests/test_windows_backend.py @@ -10,7 +10,8 @@ if not sys.platform.startswith('win'): pytest.skip('skipping windows-only tests', allow_module_level=True) else: - from msal_extensions.windows import WindowsDataProtectionAgent, WindowsTokenCache + from msal_extensions.windows import WindowsDataProtectionAgent + from msal_extensions.token_cache import WindowsTokenCache def test_dpapi_roundtrip_with_entropy(): From 997fdbff33ff94cc849c1e052300f2857e69a058 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 10 Jul 2019 15:30:44 -0700 Subject: [PATCH 11/13] Making cache_location mandatory (#24) This squash will remove all those experimental commits that no longer necessary. --- msal_extensions/token_cache.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/msal_extensions/token_cache.py b/msal_extensions/token_cache.py index 878ebca..fce54b1 100644 --- a/msal_extensions/token_cache.py +++ b/msal_extensions/token_cache.py @@ -33,10 +33,7 @@ def _mkdir_p(path): class FileTokenCache(msal.SerializableTokenCache): """Implements basic unprotected SerializableTokenCache to a plain-text file.""" def __init__(self, - cache_location=os.path.join( - os.getenv('LOCALAPPDATA', os.path.expanduser('~')), - '.IdentityService', - 'msal.cache'), + cache_location, lock_location=None): super(FileTokenCache, self).__init__() self._cache_location = cache_location @@ -121,19 +118,19 @@ def find(self, credential_type, **kwargs): # pylint: disable=arguments-differ class UnencryptedTokenCache(FileTokenCache): """An unprotected token cache to default to when no-platform specific option is available.""" - def __init__(self, **kwargs): + 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__(**kwargs) + super(UnencryptedTokenCache, self).__init__(cache_location, **kwargs) class WindowsTokenCache(FileTokenCache): """A SerializableTokenCache implementation which uses Win32 encryption APIs to protect your tokens. """ - def __init__(self, entropy='', **kwargs): - super(WindowsTokenCache, self).__init__(**kwargs) + def __init__(self, cache_location, entropy='', **kwargs): + super(WindowsTokenCache, self).__init__(cache_location, **kwargs) self._dp_agent = WindowsDataProtectionAgent(entropy=entropy) def _write(self, contents): @@ -152,10 +149,11 @@ class OSXTokenCache(FileTokenCache): """ def __init__(self, + cache_location, service_name='Microsoft.Developer.IdentityService', account_name='MSALCache', **kwargs): - super(OSXTokenCache, self).__init__(**kwargs) + super(OSXTokenCache, self).__init__(cache_location, **kwargs) self._service_name = service_name self._account_name = account_name From 74da5250847d261260c57c78a1001527aef5a3f6 Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 10 Jul 2019 16:46:36 -0700 Subject: [PATCH 12/13] Add release routine --- .travis.yml | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 96b68fd..f3cee97 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,7 @@ matrix: fast_finish: true include: - python: "2.7" - env: TOXENV=py27 + env: TOXENV=py27 PYPI=true os: linux - python: "3.5" env: TOXENV=py35 @@ -44,3 +44,27 @@ install: script: - pylint msal_extensions - tox + +deploy: + - # test pypi + provider: pypi + distributions: "sdist bdist_wheel" + server: https://test.pypi.org/legacy/ + user: "nugetaad" + password: + secure: dpNi6BsZyiAx/gkxJ5Mz6m2yDz2dRGWsSgS5pF+ywNzgHJ6+0e234GyLbSUY5bFeeA7WtOr4is3bxSLB/6tTWDVWdw3TL4FGlDM/54MSLWg8R5bR9PRwO+VU1kvQ03yz+B9mTpzuiwL2e+OSwcwo97jForADzmSRA5OpEq5Z7zAs7WR8J2tyhl+288NwLtKJMVy39UmPl9oifu6/5RfBn7EWLmC7MrMFhHTb2Gj7fJWw4u+5vx9bsQ7ubfiwPbRAtYXLz6wDMtwtFzwme4zZPg5HwWCn0WWlX4b6x7xXirZ7yKsy9iACLgTrLMeAkferrex7f03NFeIDobasML+fLbZufATaL3M97kNGZwulEYNp2+RWyLu/NW6FoZCbS+cSL8HuFnkIDHzEoO56ItMiD9EH47q/NeKgwrrzKjfY+KzaMQOYLlVgCa4WrIeFh5CkwJ4RHrfanMIV2vbEvMxsnHc/mZ+yvgBOFoBNXYN1HEDzEv1NxDPcyt7MBlPUVinEreQaHba7w6qH9Rf0eWgfW2ypBXe+nHaZxQgaGC6J+WGUkzalYQspmHVU4CcuwJa55kuchJs/gbyZKkyK6P8uD5IP6VZiavwZcjWcfvwbZaLeOqzSDVCDMg8M2zYZHoa+6ZR4EgDVW7RvaRvjvvhPTPj5twmLf3YYVJtHIyJSLug= + on: + branch: master + tags: false + condition: $PYPI = "true" + + - # production pypi + provider: pypi + distributions: "sdist bdist_wheel" + user: "nugetaad" + password: + secure: dpNi6BsZyiAx/gkxJ5Mz6m2yDz2dRGWsSgS5pF+ywNzgHJ6+0e234GyLbSUY5bFeeA7WtOr4is3bxSLB/6tTWDVWdw3TL4FGlDM/54MSLWg8R5bR9PRwO+VU1kvQ03yz+B9mTpzuiwL2e+OSwcwo97jForADzmSRA5OpEq5Z7zAs7WR8J2tyhl+288NwLtKJMVy39UmPl9oifu6/5RfBn7EWLmC7MrMFhHTb2Gj7fJWw4u+5vx9bsQ7ubfiwPbRAtYXLz6wDMtwtFzwme4zZPg5HwWCn0WWlX4b6x7xXirZ7yKsy9iACLgTrLMeAkferrex7f03NFeIDobasML+fLbZufATaL3M97kNGZwulEYNp2+RWyLu/NW6FoZCbS+cSL8HuFnkIDHzEoO56ItMiD9EH47q/NeKgwrrzKjfY+KzaMQOYLlVgCa4WrIeFh5CkwJ4RHrfanMIV2vbEvMxsnHc/mZ+yvgBOFoBNXYN1HEDzEv1NxDPcyt7MBlPUVinEreQaHba7w6qH9Rf0eWgfW2ypBXe+nHaZxQgaGC6J+WGUkzalYQspmHVU4CcuwJa55kuchJs/gbyZKkyK6P8uD5IP6VZiavwZcjWcfvwbZaLeOqzSDVCDMg8M2zYZHoa+6ZR4EgDVW7RvaRvjvvhPTPj5twmLf3YYVJtHIyJSLug= + on: + branch: master + tags: true + condition: $PYPI = "true" From 6809f721b3601c620efc70dbd56e0e940fb78fab Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 10 Jul 2019 17:06:47 -0700 Subject: [PATCH 13/13] msal-extensions 0.1.0 --- 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 7c5f4a6..6591075 100644 --- a/msal_extensions/__init__.py +++ b/msal_extensions/__init__.py @@ -1,5 +1,5 @@ """Provides auxiliary functionality to the `msal` package.""" -__version__ = "0.0.1" +__version__ = "0.1.0" import sys