diff --git a/.gitignore b/.gitignore index 1b806a6..8bf087c 100644 --- a/.gitignore +++ b/.gitignore @@ -334,3 +334,5 @@ ASALocalRun/ .mfractor/ .eggs/ +.env +Session.vim diff --git a/Dockerfile b/Dockerfile index 8b506dd..1304b97 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,7 @@ # TODO: Can this Dockerfile use multi-stage build? +# https://testdriven.io/tips/6da2d9c9-8849-4386-b7f9-13b28514ded8/ # Final size 690MB. (It would be 1.16 GB if started with python:3 as base) -FROM python:3.12-slim +FROM python:3.13-slim # Install Generic PyGObject (sans GTK) #The following somehow won't work: @@ -9,7 +10,6 @@ RUN apt-get update && apt-get install -y \ libcairo2-dev \ libgirepository1.0-dev \ python3-dev -RUN pip install "pygobject>=3,<4" # Install MSAL Extensions dependencies # Don't know how to get container talk to dbus on host, @@ -19,10 +19,10 @@ RUN apt-get install -y \ gnome-keyring # Not strictly necessary, but we include a pytest (which is only 3MB) to facilitate testing. -RUN pip install "pytest>=6,<7" +RUN pip install "pygobject>=3,<4" "pytest>=6,<7" # Install MSAL Extensions. Upgrade the pinned version number to trigger a new image build. -RUN pip install "msal-extensions==1.1" +RUN pip install "msal-extensions==1.2" # This setup is inspired from https://github.com/jaraco/keyring#using-keyring-on-headless-linux-systems-in-a-docker-container ENTRYPOINT ["dbus-run-session", "--"] diff --git a/docker_run.sh b/docker_run.sh index 04914d7..c1ac96f 100755 --- a/docker_run.sh +++ b/docker_run.sh @@ -6,11 +6,10 @@ docker build -t $IMAGE_NAME - < Dockerfile echo "==== Integration Test for Persistence on Linux (libsecret) ====" echo "After seeing the bash prompt, run the following to test encryption on Linux:" echo " pip install -e ." -echo " pytest -s tests/chosen_test_file.py" -echo "Note that you probably need to set up ENV VAR for the test cases to run" +echo " pytest --capture=no -s tests/chosen_test_file.py" +echo "Note: It will test portalocker-based lock when portalocker is installed, or test file-based lock otherwise." docker run --rm -it \ --privileged \ - --env-file .env \ -w /home -v $PWD:/home \ $IMAGE_NAME \ $1 diff --git a/msal_extensions/__init__.py b/msal_extensions/__init__.py index a9a7283..384f5fb 100644 --- a/msal_extensions/__init__.py +++ b/msal_extensions/__init__.py @@ -8,6 +8,5 @@ KeychainPersistence, LibsecretPersistence, ) -from .cache_lock import CrossPlatLock -from .token_cache import PersistedTokenCache +from .token_cache import PersistedTokenCache, CrossPlatLock, LockError diff --git a/msal_extensions/cache_lock.py b/msal_extensions/cache_lock.py index 5f61413..e5ddf2b 100644 --- a/msal_extensions/cache_lock.py +++ b/msal_extensions/cache_lock.py @@ -5,12 +5,15 @@ import time import logging -import portalocker +import portalocker # pylint: disable=import-error logger = logging.getLogger(__name__) +LockError = portalocker.exceptions.LockException + + 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 diff --git a/msal_extensions/filelock.py b/msal_extensions/filelock.py new file mode 100644 index 0000000..2c02247 --- /dev/null +++ b/msal_extensions/filelock.py @@ -0,0 +1,62 @@ +"""A cross-process lock based on exclusive creation of a given file name""" +import os +import sys +import errno +import time +import logging + + +logger = logging.getLogger(__name__) + + +class LockError(RuntimeError): + """It will be raised when unable to obtain a lock""" + + +class CrossPlatLock(object): + """This implementation relies only on ``open(..., 'x')``""" + def __init__(self, lockfile_path): + self._lockpath = lockfile_path + + def __enter__(self): + self._create_lock_file('{} {}'.format( + os.getpid(), + sys.argv[0], + ).encode('utf-8')) # pylint: disable=consider-using-f-string + return self + + def _create_lock_file(self, content): + timeout = 5 + check_interval = 0.25 + current_time = getattr(time, "monotonic", time.time) + timeout_end = current_time() + timeout + while timeout_end > current_time(): + try: + with open(self._lockpath, 'xb') as lock_file: # pylint: disable=unspecified-encoding + lock_file.write(content) + return None # Happy path + except ValueError: # This needs to be the first clause, for Python 2 to hit it + raise LockError("Python 2 does not support atomic creation of file") + except FileExistsError: # Only Python 3 will reach this clause + logger.debug( + "Process %d found existing lock file, will retry after %f second", + os.getpid(), check_interval) + time.sleep(check_interval) + raise LockError( + "Unable to obtain lock, despite trying for {} second(s). " + "You may want to manually remove the stale lock file {}".format( + timeout, + self._lockpath, + )) + + def __exit__(self, *args): + try: + os.remove(self._lockpath) + except OSError as ex: # pylint: disable=invalid-name + if ex.errno in (errno.ENOENT, errno.EACCES): + # Probably another process has raced this one + # and ended up clearing or locking the file for itself. + logger.debug("Unable to remove lock file") + else: + raise + diff --git a/msal_extensions/libsecret.py b/msal_extensions/libsecret.py index 5f81753..08c7028 100644 --- a/msal_extensions/libsecret.py +++ b/msal_extensions/libsecret.py @@ -40,9 +40,7 @@ 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 - # pylint: disable=too-many-positional-arguments + def __init__( # pylint: disable=too-many-arguments,too-many-positional-arguments self, schema_name, attributes, # {"name": "value", ...} diff --git a/msal_extensions/token_cache.py b/msal_extensions/token_cache.py index 93e8565..348424d 100644 --- a/msal_extensions/token_cache.py +++ b/msal_extensions/token_cache.py @@ -5,7 +5,13 @@ import msal -from .cache_lock import CrossPlatLock +try: # It needs portalocker + from .cache_lock import ( # pylint: disable=unused-import + CrossPlatLock, + LockError, # We don't use LockError in this file, but __init__.py uses it. + ) +except ImportError: # Falls back to file-based lock + from .filelock import CrossPlatLock, LockError # pylint: disable=unused-import from .persistence import _mkdir_p, PersistenceNotFound diff --git a/setup.py b/setup.py index d16e5d8..6d43f08 100644 --- a/setup.py +++ b/setup.py @@ -20,10 +20,14 @@ python_requires=">=3.7", install_requires=[ 'msal>=1.29,<2', # Use TokenCache.search() from MSAL Python 1.29+ - 'portalocker<4,>=1.4', ## We choose to NOT define a hard dependency on this. # "pygobject>=3,<4;platform_system=='Linux'", ], + extras_require={ + "portalocker": [ + 'portalocker<4,>=1.4', + ], + }, tests_require=['pytest'], ) diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/cache_file_generator.py b/tests/cache_file_generator.py index 4164314..377d2db 100644 --- a/tests/cache_file_generator.py +++ b/tests/cache_file_generator.py @@ -14,9 +14,10 @@ import sys import time -from portalocker import exceptions +from msal_extensions import FilePersistence, CrossPlatLock, LockError -from msal_extensions import FilePersistence, CrossPlatLock + +print("Testing with {}".format(CrossPlatLock)) def _acquire_lock_and_write_to_cache(cache_location, sleep_interval): @@ -31,7 +32,7 @@ def _acquire_lock_and_write_to_cache(cache_location, sleep_interval): time.sleep(sleep_interval) data += "> " + str(os.getpid()) + "\n" cache_accessor.save(data) - except exceptions.LockException as e: + except LockError as e: logging.warning("Unable to acquire lock %s", e) diff --git a/tests/http_client.py b/tests/http_client.py new file mode 100644 index 0000000..1cee3c5 --- /dev/null +++ b/tests/http_client.py @@ -0,0 +1,12 @@ +class MinimalResponse(object): # Not for production use + def __init__(self, requests_resp=None, status_code=None, text=None, headers=None): + self.status_code = status_code or requests_resp.status_code + self.text = text if text is not None else requests_resp.text + self.headers = {} if headers is None else headers + self._raw_resp = requests_resp + + def raise_for_status(self): + if self._raw_resp is not None: # Turns out `if requests.response` won't work + # cause it would be True when 200<=status<400 + self._raw_resp.raise_for_status() + diff --git a/tests/test_agnostic_backend.py b/tests/test_agnostic_backend.py index 565de36..d241a2d 100644 --- a/tests/test_agnostic_backend.py +++ b/tests/test_agnostic_backend.py @@ -1,12 +1,15 @@ +import json import os import shutil import tempfile +from unittest.mock import patch import sys import msal import pytest from msal_extensions import * +from .http_client import MinimalResponse @pytest.fixture @@ -16,18 +19,19 @@ def temp_location(): shutil.rmtree(test_folder, ignore_errors=True) def _test_token_cache_roundtrip(persistence): - 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 TokenCache round-trip with.') - desired_scopes = ['https://graph.microsoft.com/.default'] apps = [ # Multiple apps sharing same persistence msal.ConfidentialClientApplication( - client_id, client_credential=client_secret, + "fake_client_id", client_credential="fake_client_secret", token_cache=PersistedTokenCache(persistence)) for i in range(2)] - token1 = apps[0].acquire_token_for_client(scopes=desired_scopes) - assert token1["token_source"] == "identity_provider", "Initial token should come from IdP" + with patch.object(apps[0].http_client, "post", return_value=MinimalResponse( + status_code=200, text=json.dumps({ + "token_type": "Bearer", + "access_token": "app token", + "expires_in": 3600, + }))) as mocked_post: + token1 = apps[0].acquire_token_for_client(scopes=desired_scopes) + assert token1["token_source"] == "identity_provider", "Initial token should come from IdP" token2 = apps[1].acquire_token_for_client(scopes=desired_scopes) # Hit token cache in MSAL 1.23+ assert token2["token_source"] == "cache", "App2 should hit cache written by app1" assert token1['access_token'] == token2['access_token'], "Cache should hit" diff --git a/tests/test_cache_lock_file_perf.py b/tests/test_cache_lock_file_perf.py index 8bc1a1c..a86bf82 100644 --- a/tests/test_cache_lock_file_perf.py +++ b/tests/test_cache_lock_file_perf.py @@ -5,7 +5,7 @@ import pytest -from cache_file_generator import _acquire_lock_and_write_to_cache +from .cache_file_generator import _acquire_lock_and_write_to_cache @pytest.fixture diff --git a/tests/test_crossplatlock.py b/tests/test_crossplatlock.py index ea3c9d5..4aac516 100644 --- a/tests/test_crossplatlock.py +++ b/tests/test_crossplatlock.py @@ -1,5 +1,5 @@ import pytest -from msal_extensions.cache_lock import CrossPlatLock +from msal_extensions import CrossPlatLock def test_ensure_file_deleted(): @@ -10,6 +10,7 @@ def test_ensure_file_deleted(): except NameError: FileNotFoundError = IOError + print("Testing with {}".format(CrossPlatLock)) with CrossPlatLock(lockfile): pass diff --git a/tox.ini b/tox.ini index 2f44068..93ef1c2 100644 --- a/tox.ini +++ b/tox.ini @@ -7,7 +7,7 @@ passenv = GITHUB_ACTIONS commands = - pytest + {posargs:pytest --color=yes} [testenv:lint] deps =