From 38d58bdf26d9f5d181853388cb8f2aa53129ef4a Mon Sep 17 00:00:00 2001 From: Tim DiLauro Date: Thu, 14 Sep 2023 21:43:59 -0400 Subject: [PATCH] Add ability to switch off patron basic token authentication. --- api/authentication/basic_token.py | 4 +- api/authenticator.py | 4 +- core/config.py | 26 ++++++++++++- core/util/__init__.py | 33 ++++++++++++++++ tests/api/test_authenticator.py | 51 +++++++++++++++++++++---- tests/api/test_config.py | 38 +++++++++++++++++++ tests/core/util/test_util.py | 63 +++++++++++++++++++++++++++++++ 7 files changed, 208 insertions(+), 11 deletions(-) diff --git a/api/authentication/basic_token.py b/api/authentication/basic_token.py index 19b59e73c5..6e445c8aeb 100644 --- a/api/authentication/basic_token.py +++ b/api/authentication/basic_token.py @@ -23,6 +23,8 @@ class BasicTokenAuthenticationProvider(AuthenticationProvider): It is a companion to the basic authentication, and has no meaning without it. """ + FLOW_TYPE = "http://thepalaceproject.org/authtype/basic-token" + def __init__( self, _db: Session, @@ -105,7 +107,7 @@ def remote_patron_lookup(self, _db): @property def flow_type(self) -> str: - return "http://thepalaceproject.org/authtype/basic-token" + return self.FLOW_TYPE @classmethod def description(cls) -> str: diff --git a/api/authenticator.py b/api/authenticator.py index f030bb26dc..11d655b0e1 100644 --- a/api/authenticator.py +++ b/api/authenticator.py @@ -401,7 +401,9 @@ def register_basic_auth_provider( ): raise CannotLoadConfiguration("Two basic auth providers configured") self.basic_auth_provider = provider - if self.library is not None: + # TODO: We can remove the configuration test once + # basic token authentication is fully deployed. + if self.library is not None and Configuration.basic_token_auth_is_enabled(): self.access_token_authentication_provider = ( BasicTokenAuthenticationProvider( self._db, self.library, self.basic_auth_provider diff --git a/core/config.py b/core/config.py index 5a9cf07b46..9744a2ece1 100644 --- a/core/config.py +++ b/core/config.py @@ -11,7 +11,7 @@ # from this module, alongside CannotLoadConfiguration. from core.exceptions import IntegrationException -from .util import LanguageCodes +from .util import LanguageCodes, ansible_boolean from .util.datetime_helpers import to_utc, utc_now @@ -40,6 +40,10 @@ class Configuration(ConfigurationConstants): DATABASE_TEST_ENVIRONMENT_VARIABLE = "SIMPLIFIED_TEST_DATABASE" DATABASE_PRODUCTION_ENVIRONMENT_VARIABLE = "SIMPLIFIED_PRODUCTION_DATABASE" + # TODO: We can remove this variable once basic token authentication is fully deployed. + # Patron token authentication enabled switch. + BASIC_TOKEN_AUTH_ENABLED_ENVVAR = "SIMPLIFIED_ENABLE_BASIC_TOKEN_AUTH" + # Environment variables for Firebase Cloud Messaging (FCM) service account key FCM_CREDENTIALS_FILE_ENVIRONMENT_VARIABLE = "SIMPLIFIED_FCM_CREDENTIALS_FILE" FCM_CREDENTIALS_JSON_ENVIRONMENT_VARIABLE = "SIMPLIFIED_FCM_CREDENTIALS_JSON" @@ -207,6 +211,26 @@ def database_url(cls): logging.info("Connecting to database: %s" % url_obj.__to_string__()) return url + # TODO: We can remove this method once basic token authentication is fully deployed. + @classmethod + def basic_token_auth_is_enabled(cls) -> bool: + """Is basic token authentication enabled? + + Return False, if the variable is unset or is an empty string. + Raises CannotLoadConfiguration, if the setting is invalid. + :raise CannotLoadConfiguration: If the setting contains an unsupported value. + """ + try: + return ansible_boolean( + os.environ.get(cls.BASIC_TOKEN_AUTH_ENABLED_ENVVAR), + label=cls.BASIC_TOKEN_AUTH_ENABLED_ENVVAR, + default=False, + ) + except (TypeError, ValueError) as e: + raise CannotLoadConfiguration( + f"Invalid value for {cls.BASIC_TOKEN_AUTH_ENABLED_ENVVAR} environment variable." + ) from e + @classmethod def fcm_credentials(cls) -> Dict[str, str]: """Returns a dictionary containing Firebase Cloud Messaging credentials. diff --git a/core/util/__init__.py b/core/util/__init__.py index eb8cee21e5..c3fd91159e 100644 --- a/core/util/__init__.py +++ b/core/util/__init__.py @@ -582,6 +582,39 @@ def chunks(lst, chunk_size, start_index=0): yield lst[i : i + chunk_size] +def ansible_boolean( + value: Optional[str | bool], + label: Optional[str] = None, + default: Optional[bool] = None, +) -> bool: + """Map Ansible "truthy" and "falsy" values to a Python boolean. + + :param value: The value from which to map. + :param label: Optional name or label associated with the value. + :param default: Default result if value is empty string or None. + """ + _value_label = f"Value of '{label}'" if label else "Value" + if default is not None and not isinstance(default, bool): + raise TypeError("'default' must be a boolean, when specified.") + if isinstance(value, bool): + return value + if value is None or value == "": + if default is not None: + return default + raise ValueError( + f"{_value_label} must be non-null and non-empty if no default is specified." + ) + if not isinstance(value, str): + raise TypeError(f"{_value_label} must be a string or boolean.") + + if value.upper() in ("TRUE", "T", "ON", "YES", "Y", "1"): + return True + if value.upper() in ("FALSE", "F", "OFF", "NO", "N", "0"): + return False + + raise ValueError(f"{_value_label} does not map to True or False.") + + class ValuesMeta(type): """Metaclass to allow operators on simple constants defining classes""" diff --git a/tests/api/test_authenticator.py b/tests/api/test_authenticator.py index 34c2a7a5ca..dae4a296bc 100644 --- a/tests/api/test_authenticator.py +++ b/tests/api/test_authenticator.py @@ -31,6 +31,7 @@ Keyboards, LibraryIdentifierRestriction, ) +from api.authentication.basic_token import BasicTokenAuthenticationProvider from api.authenticator import ( Authenticator, BaseSAMLAuthenticationProvider, @@ -923,8 +924,13 @@ def test_authenticated_patron_bearer( assert response == "foo" assert saml.authenticated_patron.call_count == 1 + # TODO: We can remove this patch once basic token authentication is fully deployed. + @patch.object(Configuration, "basic_token_auth_is_enabled", return_value=True) def test_authenticated_patron_bearer_access_token( - self, db: DatabaseTransactionFixture, mock_basic: MockBasicFixture + self, + basic_token_auth_is_enabled_mock, + db: DatabaseTransactionFixture, + mock_basic: MockBasicFixture, ): basic = mock_basic() authenticator = LibraryAuthenticator( @@ -950,8 +956,13 @@ def test_authenticated_patron_unsupported_mechanism( ) assert UNSUPPORTED_AUTHENTICATION_MECHANISM == problem + # TODO: We can remove this patch once basic token authentication is fully deployed. + @patch.object(Configuration, "basic_token_auth_is_enabled", return_value=True) def test_get_credential_from_header( - self, db: DatabaseTransactionFixture, mock_basic: MockBasicFixture + self, + basic_token_auth_is_enabled_mock, + db: DatabaseTransactionFixture, + mock_basic: MockBasicFixture, ): basic = mock_basic() @@ -983,12 +994,21 @@ def test_get_credential_from_header( credential = Authorization(auth_type="bearer", token=token) assert authenticator.get_credential_from_header(credential) == "passworx" + @pytest.mark.parametrize( + "token_auth_enabled, auth_count", + [ + [True, 2], + [False, 1], + ], + ) def test_create_authentication_document( self, db: DatabaseTransactionFixture, mock_basic: MockBasicFixture, announcement_fixture: AnnouncementFixture, library_fixture: LibraryFixture, + token_auth_enabled: bool, + auth_count: int, ): class MockAuthenticator(LibraryAuthenticator): """Mock the _geographic_areas method.""" @@ -1003,11 +1023,16 @@ def _geographic_areas(cls, library): library_settings = library_fixture.settings(library) basic = mock_basic() library.name = "A Fabulous Library" - authenticator = MockAuthenticator( - _db=db.session, - library=library, - basic_auth_provider=basic, - ) + # TODO: We can remove this patch once basic token authentication is fully deployed. + with patch.object( + Configuration, "basic_token_auth_is_enabled" + ) as token_auth_enabled_method: + token_auth_enabled_method.return_value = token_auth_enabled + authenticator = MockAuthenticator( + _db=db.session, + library=library, + basic_auth_provider=basic, + ) def annotate_authentication_document(library, doc, url_for): doc["modified"] = "Kilroy was here" @@ -1103,7 +1128,17 @@ def annotate_authentication_document(library, doc, url_for): # The main thing we need to test is that the # authentication sub-documents are assembled properly and # placed in the right position. - [token_doc, basic_doc] = doc["authentication"] + # TODO: token doc will be here only when correct environment variable set to true. + # If basic token auth is enabled, then there should be two authentication + # mechanisms and the first should be for token auth. + authenticators = doc["authentication"] + assert auth_count > 0 + assert auth_count == len(authenticators) + # TODO: We can remove this `if` block/restructure once basic token authentication is fully deployed. + if token_auth_enabled: + token_doc = authenticators[0] + assert BasicTokenAuthenticationProvider.FLOW_TYPE == token_doc["type"] + basic_doc = authenticators[auth_count - 1] expect_basic = basic.authentication_flow_document(db.session) assert expect_basic == basic_doc diff --git a/tests/api/test_config.py b/tests/api/test_config.py index 76a00f3bb5..ac50a16dc0 100644 --- a/tests/api/test_config.py +++ b/tests/api/test_config.py @@ -1,6 +1,7 @@ import json import os from collections import Counter +from contextlib import nullcontext as does_not_raise from unittest.mock import patch import pytest @@ -196,3 +197,40 @@ def test_fcm_credentials(self, notifications_files_fixture): match=r"Cannot parse value of FCM credential environment variable .* as JSON.", ): Configuration.fcm_credentials() + + @pytest.mark.parametrize( + "env_var_value, expected_result, raises_exception", + [ + ["true", True, False], + ["True", True, False], + [None, False, False], + ["", False, False], + ["false", False, False], + ["False", False, False], + ["3", None, True], + ["X", None, True], + ], + ) + @patch.object(os, "environ", new=dict()) + def test_basic_token_auth_is_enabled( + self, env_var_value, expected_result, raises_exception + ): + env_var = Configuration.BASIC_TOKEN_AUTH_ENABLED_ENVVAR + + # Simulate an unset environment variable with the `None` value. + if env_var_value is None: + del os.environ[env_var] + else: + os.environ[env_var] = env_var_value + + expected_exception = ( + pytest.raises( + CannotLoadConfiguration, + match=f"Invalid value for {env_var} environment variable.", + ) + if raises_exception + else does_not_raise() + ) + + with expected_exception: + assert expected_result == Configuration.basic_token_auth_is_enabled() diff --git a/tests/core/util/test_util.py b/tests/core/util/test_util.py index 2bb66cb2d6..4110d7cad7 100644 --- a/tests/core/util/test_util.py +++ b/tests/core/util/test_util.py @@ -14,6 +14,7 @@ MetadataSimilarity, MoneyUtility, TitleProcessor, + ansible_boolean, english_bigrams, fast_query_count, slugify, @@ -478,3 +479,65 @@ def test_parse( def test_parsing_bad_value_raises_valueerror(self, bad_value): with pytest.raises(ValueError): MoneyUtility.parse(bad_value) + + +class TestAnsibleBoolean: + _truthy_values = ["TRUE", "T", "ON", "YES", "Y", "1"] + _falsy_values = ["FALSE", "F", "OFF", "NO", "N", "0"] + # Values are case-insensitive. + TRUTHY = [True] + _truthy_values + [v.lower() for v in _truthy_values] + FALSY = [False] + _falsy_values + [v.lower() for v in _falsy_values] + MISSING = [None, ""] + + @pytest.mark.parametrize( + "expected_result, example_values, default_value", + [ + [True, TRUTHY, False], + [True, TRUTHY, True], + [True, TRUTHY, None], + [True, MISSING, True], + [False, FALSY, False], + [False, FALSY, True], + [False, FALSY, None], + [False, MISSING, False], + ], + ) + def test_ansible_boolean_true_or_false( + self, expected_result, example_values, default_value + ): + for value in example_values: + assert expected_result == ansible_boolean(value, default=default_value) + assert expected_result == ansible_boolean( + value, default=default_value, label="some label" + ) + + @pytest.mark.parametrize( + "example_value, default_value, expected_exception, expected_message", + [ + ["TRUE", "", TypeError, "'default' must be a boolean, when specified"], + ["TRUE", "X", TypeError, "'default' must be a boolean, when specified"], + ["TRUE", 0, TypeError, "'default' must be a boolean, when specified"], + ["TRUE", "TRUE", TypeError, "'default' must be a boolean, when specified"], + [1, None, TypeError, "must be a string"], + [3.3, None, TypeError, "must be a string"], + ["!", None, ValueError, "does not map to True or False"], + ["x", None, ValueError, "does not map to True or False"], + [ + None, + None, + ValueError, + "must be non-null and non-empty if no default is specified", + ], + [ + "", + None, + ValueError, + "must be non-null and non-empty if no default is specified", + ], + ], + ) + def test_ansible_boolean_exceptions( + self, example_value, default_value, expected_exception, expected_message + ): + with pytest.raises(expected_exception, match=expected_message): + ansible_boolean(example_value, default=default_value)