Skip to content

Commit

Permalink
Make portalocker optional
Browse files Browse the repository at this point in the history
  • Loading branch information
rayluo committed Jan 23, 2025
1 parent 7a5d39e commit 4b90cc8
Show file tree
Hide file tree
Showing 16 changed files with 120 additions and 29 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -334,3 +334,5 @@ ASALocalRun/
.mfractor/

.eggs/
.env
Session.vim
8 changes: 4 additions & 4 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -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,
Expand All @@ -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", "--"]
Expand Down
5 changes: 2 additions & 3 deletions docker_run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions msal_extensions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,5 @@
KeychainPersistence,
LibsecretPersistence,
)
from .cache_lock import CrossPlatLock
from .token_cache import PersistedTokenCache
from .token_cache import PersistedTokenCache, CrossPlatLock, LockError

5 changes: 4 additions & 1 deletion msal_extensions/cache_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
62 changes: 62 additions & 0 deletions msal_extensions/filelock.py
Original file line number Diff line number Diff line change
@@ -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

4 changes: 1 addition & 3 deletions msal_extensions/libsecret.py
Original file line number Diff line number Diff line change
Expand Up @@ -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", ...}
Expand Down
8 changes: 7 additions & 1 deletion msal_extensions/token_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
6 changes: 5 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
)
Empty file added tests/__init__.py
Empty file.
7 changes: 4 additions & 3 deletions tests/cache_file_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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)


Expand Down
12 changes: 12 additions & 0 deletions tests/http_client.py
Original file line number Diff line number Diff line change
@@ -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()

20 changes: 12 additions & 8 deletions tests/test_agnostic_backend.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion tests/test_cache_lock_file_perf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion tests/test_crossplatlock.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import pytest
from msal_extensions.cache_lock import CrossPlatLock
from msal_extensions import CrossPlatLock


def test_ensure_file_deleted():
Expand All @@ -10,6 +10,7 @@ def test_ensure_file_deleted():
except NameError:
FileNotFoundError = IOError

print("Testing with {}".format(CrossPlatLock))
with CrossPlatLock(lockfile):
pass

Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ passenv =
GITHUB_ACTIONS

commands =
pytest
{posargs:pytest --color=yes}

[testenv:lint]
deps =
Expand Down

0 comments on commit 4b90cc8

Please sign in to comment.