diff --git a/open_city_profile/graphene.py b/open_city_profile/graphene.py index e4bef01c..765c527e 100644 --- a/open_city_profile/graphene.py +++ b/open_city_profile/graphene.py @@ -184,33 +184,30 @@ def __init_subclass_with_meta__( class AllowedDataFieldsMiddleware: - def resolve(self, next, root, info, **kwargs): if getattr(root, "check_allowed_data_fields", False): field_name = to_snake_case(getattr(info, "field_name", "")) - service = getattr(info.context, "service", None) - if not service: - if settings.ENABLE_ALLOWED_DATA_FIELDS_RESTRICTION: - raise ServiceNotIdentifiedError("Service not identified") - - logging.warning( - "Allowed data field exception would occur: Service not identified. Field name: %s", - field_name, - ) - - return next(root, info, **kwargs) if not root.is_field_allowed_for_service(field_name, service): - if settings.ENABLE_ALLOWED_DATA_FIELDS_RESTRICTION: - raise FieldNotAllowedError( - "Field is not allowed for service.", field_name=field_name + if service: + if settings.ENABLE_ALLOWED_DATA_FIELDS_RESTRICTION: + raise FieldNotAllowedError( + "Field is not allowed for service.", field_name=field_name + ) + + logging.warning( + "Allowed data field exception would occur: Field (%s) is not allowed for service %s.", + field_name, + info.context.service, ) + else: + if settings.ENABLE_ALLOWED_DATA_FIELDS_RESTRICTION: + raise ServiceNotIdentifiedError("Service not identified") - logging.warning( - "Allowed data field exception would occur. Field (%s) is not allowed for service %s.", - field_name, - info.context.service, - ) + logging.warning( + "Allowed data field exception would occur: Service not identified. Field name: %s", + field_name, + ) return next(root, info, **kwargs) diff --git a/profiles/models.py b/profiles/models.py index 7fb8fcc0..a78acbb9 100644 --- a/profiles/models.py +++ b/profiles/models.py @@ -52,13 +52,14 @@ class AllowedDataFieldsMixin: check_allowed_data_fields = True @classmethod - def is_field_allowed_for_service(cls, field_name: str, service: Service): - if not service: - raise ValueError("No service identified") - + def is_field_allowed_for_service(cls, field_name: str, service: Service = None): + # Always allow certain fields, regardless of the service if field_name in cls.always_allow_fields: return True + if not service: + return False + allowed_data_fields = service.allowed_data_fields.values_list( "field_name", flat=True ) diff --git a/profiles/tests/test_allowed_data_fields_mixin.py b/profiles/tests/test_allowed_data_fields_mixin.py new file mode 100644 index 00000000..b8ca442b --- /dev/null +++ b/profiles/tests/test_allowed_data_fields_mixin.py @@ -0,0 +1,43 @@ +import pytest + +from profiles.models import AllowedDataFieldsMixin, Profile +from services.models import Service + + +@pytest.mark.django_db +def test_field_allowed_when_in_always_allow_fields(monkeypatch): + monkeypatch.setattr(AllowedDataFieldsMixin, "always_allow_fields", ["id"]) + service = Service.objects.create(name="Test Service") + + assert AllowedDataFieldsMixin.is_field_allowed_for_service("id", service) is True + + +def test_field_allowed_when_service_is_none_and_in_always_allow_fields(monkeypatch): + monkeypatch.setattr(AllowedDataFieldsMixin, "always_allow_fields", ["id"]) + + assert AllowedDataFieldsMixin.is_field_allowed_for_service("id", None) is True + + +def test_field_not_allowed_when_service_is_not_defined(monkeypatch): + monkeypatch.setattr(AllowedDataFieldsMixin, "always_allow_fields", []) + + assert AllowedDataFieldsMixin.is_field_allowed_for_service("id", None) is False + + +@pytest.mark.django_db +def test_field_allowed_when_in_allowed_data_fields(monkeypatch): + monkeypatch.setattr(Profile, "always_allow_fields", []) + monkeypatch.setattr(Profile, "allowed_data_fields_map", {"id": ("id",)}) + service = Service.objects.create(name="Test Service") + service.allowed_data_fields.create(field_name="id") + + assert Profile.is_field_allowed_for_service("id", service) is True + + +@pytest.mark.django_db +def test_field_not_allowed_when_not_in_allowed_data_fields(monkeypatch): + monkeypatch.setattr(Profile, "always_allow_fields", []) + monkeypatch.setattr(Profile, "allowed_data_fields_map", {}) + service = Service.objects.create(name="Test Service") + + assert Profile.is_field_allowed_for_service("id", service) is False