diff --git a/open_city_profile/graphene.py b/open_city_profile/graphene.py index 6bad7d7a..27b4c2dc 100644 --- a/open_city_profile/graphene.py +++ b/open_city_profile/graphene.py @@ -5,12 +5,14 @@ from django.conf import settings from django.forms import MultipleChoiceField from django_filters import MultipleChoiceFilter +from graphene.utils.str_converters import to_snake_case from graphene_django import DjangoObjectType from graphene_django.forms.converter import convert_form_field from graphene_django.types import ALL_FIELDS from graphql_sync_dataloaders import SyncDataLoader from parler.models import TranslatableModel +from open_city_profile.exceptions import FieldNotAllowedError, ServiceNotIdentifiedError from profiles.loaders import ( addresses_by_profile_id_loader, emails_by_profile_id_loader, @@ -178,3 +180,20 @@ def __init_subclass_with_meta__( _meta=_meta, **options, ) + + +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", "")) + + if not getattr(info.context, "service", False): + raise ServiceNotIdentifiedError("Service not identified") + + if not root.is_field_allowed_for_service(field_name, info.context.service): + raise FieldNotAllowedError( + "Field is not allowed for service.", field_name=field_name + ) + + return next(root, info, **kwargs) diff --git a/open_city_profile/settings.py b/open_city_profile/settings.py index b71615e2..96f7cbc5 100644 --- a/open_city_profile/settings.py +++ b/open_city_profile/settings.py @@ -288,6 +288,7 @@ "SCHEMA": "open_city_profile.schema.schema", "MIDDLEWARE": [ # NOTE: Graphene runs its middlewares in reverse order! + "open_city_profile.graphene.AllowedDataFieldsMiddleware", "open_city_profile.graphene.JWTMiddleware", "open_city_profile.graphene.GQLDataLoaders", ], diff --git a/open_city_profile/tests/conftest.py b/open_city_profile/tests/conftest.py index b3849328..d7f81563 100644 --- a/open_city_profile/tests/conftest.py +++ b/open_city_profile/tests/conftest.py @@ -22,7 +22,8 @@ UserFactory, ) from open_city_profile.views import GraphQLView -from services.tests.factories import ServiceFactory +from services.models import Service +from services.tests.factories import AllowedDataFieldFactory, ServiceFactory _not_provided = object() @@ -34,6 +35,7 @@ def execute( auth_token_payload=None, service=_not_provided, context=None, + allowed_data_fields: list[str] = None, **kwargs, ): """ @@ -63,9 +65,18 @@ def execute( context.service = None if service is _not_provided: - context.service = ServiceFactory(name="profile", is_profile_service=True) + service = Service.objects.filter(is_profile_service=True).first() + if not service: + service = ServiceFactory(name="profile", is_profile_service=True) + + context.service = service elif service: context.service = service + if allowed_data_fields: + for field_name in allowed_data_fields: + context.service.allowed_data_fields.add( + AllowedDataFieldFactory(field_name=field_name) + ) return super().execute( *args, diff --git a/open_city_profile/tests/graphql_test_helpers.py b/open_city_profile/tests/graphql_test_helpers.py index 1dc99890..42eb59f6 100644 --- a/open_city_profile/tests/graphql_test_helpers.py +++ b/open_city_profile/tests/graphql_test_helpers.py @@ -6,13 +6,19 @@ from django.utils.crypto import get_random_string from jose import jwt -from services.tests.factories import ServiceClientIdFactory +from services.tests.factories import ( + AllowedDataFieldFactory, + ServiceClientIdFactory, + ServiceConnectionFactory, +) from .conftest import get_unix_timestamp_now from .keys import rsa_key AUDIENCE = getattr(settings, "OIDC_API_TOKEN_AUTH")["AUDIENCE"] ISSUER = getattr(settings, "OIDC_API_TOKEN_AUTH")["ISSUER"] +if isinstance(ISSUER, list): + ISSUER = ISSUER[0] CONFIG_URL = f"{ISSUER}/.well-known/openid-configuration" JWKS_URL = f"{ISSUER}/jwks" @@ -129,6 +135,18 @@ def do_graphql_call_as_user( service_client_id = ServiceClientIdFactory( service__service_type=None, service__is_profile_service=True ) + if getattr(user, "profile", None): + ServiceConnectionFactory( + profile=user.profile, service=service_client_id.service + ) + service_client_id.service.allowed_data_fields.add( + AllowedDataFieldFactory(field_name="name"), + AllowedDataFieldFactory(field_name="address"), + AllowedDataFieldFactory(field_name="email"), + AllowedDataFieldFactory(field_name="phone"), + AllowedDataFieldFactory(field_name="personalidentitycode"), + ) + elif service: service_client_id = service.client_ids.first() diff --git a/profiles/models.py b/profiles/models.py index 5a451497..1f6c31e4 100644 --- a/profiles/models.py +++ b/profiles/models.py @@ -8,7 +8,7 @@ from encrypted_fields import fields from enumfields import EnumField -from services.models import ServiceConnection +from services.models import Service, ServiceConnection from users.models import User from utils.fields import ( NullToEmptyCharField, @@ -28,7 +28,50 @@ ) -class Profile(UUIDModel, SerializableMixin): +class AllowedDataFieldsMixin: + """ + Mixin class for checking allowed data fields per service. + + `allowed_data_fields_map` is a dictionary where the key is the `field_name` of the allowed data field + `allowed_data_fields.json` and the value is an iterable of django model's field names that the `field_name` + describes. For example, if the `field_name` is `name`, the value could be `("first_name", "last_name")`. + e.g: + allowed_data_fields_map = { + "name": ("first_name", "last_name", "nickname"), + "personalidentitycode": ("national_identification_number",), + "address": ("address", "postal_code", "city", "country_code") + } + + `always_allow_fields`: Since connections are not defined in `allowed_data_fields.json` they should be + defined here. If the field is connection and the node does not inherit this mixin the data will be available + to all services. + """ + + allowed_data_fields_map = {} + always_allow_fields = ["id", "service_connections"] + 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") + + if field_name in cls.always_allow_fields: + return True + + allowed_data_fields = service.allowed_data_fields.values_list( + "field_name", flat=True + ) + return any( + field_name in cls.allowed_data_fields_map.get(allowed_data_field, []) + for allowed_data_field in allowed_data_fields + ) + + class Meta: + abstract = True + + +class Profile(UUIDModel, SerializableMixin, AllowedDataFieldsMixin): user = models.OneToOneField(User, on_delete=models.PROTECT, null=True, blank=True) first_name = NullToEmptyCharField(max_length=150, blank=True, db_index=True) last_name = NullToEmptyCharField(max_length=150, blank=True, db_index=True) @@ -63,6 +106,24 @@ class Meta: ) audit_log = True + # AllowedDataField configs + allowed_data_fields_map = { + "name": ( + "first_name", + "last_name", + "nickname", + ), + "email": ("emails", "primary_email"), + "phone": ("phones", "primary_phone"), + "address": ("addresses", "primary_address"), + "personalidentitycode": ("sensitivedata",), + } + always_allow_fields = AllowedDataFieldsMixin.always_allow_fields + [ + "verified_personal_information", + "language", + "contact_method", + ] + def resolve_profile(self): return self @@ -178,7 +239,7 @@ def get_national_identification_number_hash_key(): return settings.SALT_NATIONAL_IDENTIFICATION_NUMBER -class VerifiedPersonalInformation(SerializableMixin): +class VerifiedPersonalInformation(SerializableMixin, AllowedDataFieldsMixin): profile = models.OneToOneField( Profile, on_delete=models.CASCADE, related_name="verified_personal_information" ) @@ -237,6 +298,18 @@ class VerifiedPersonalInformation(SerializableMixin): ) audit_log = True + allowed_data_fields_map = { + "name": ("first_name", "last_name", "given_name"), + "personalidentitycode": ("national_identification_number",), + "address": ( + "municipality_of_residence", + "municipality_of_residence_number", + "permanent_address", + "temporary_address", + "permanent_foreign_address", + ), + } + class Meta: permissions = [ ( diff --git a/profiles/tests/test_gql_claim_profile_mutation.py b/profiles/tests/test_gql_claim_profile_mutation.py index 93c89706..d040e98e 100644 --- a/profiles/tests/test_gql_claim_profile_mutation.py +++ b/profiles/tests/test_gql_claim_profile_mutation.py @@ -61,7 +61,7 @@ def test_user_can_claim_claimable_profile_without_existing_profile( } } } - executed = user_gql_client.execute(query) + executed = user_gql_client.execute(query, allowed_data_fields=["name"]) assert "errors" not in executed assert dict(executed["data"]) == expected_data @@ -98,6 +98,42 @@ def test_user_can_claim_claimable_profile_without_existing_profile( """ +def test_user_cant_get_fields_not_allowed_when_claiming_a_profile(user_gql_client): + profile = ProfileWithPrimaryEmailFactory(user=None) + claim_token = ClaimTokenFactory(profile=profile) + + t = Template( + """ + mutation { + claimProfile( + input: { + token: "${claimToken}", + profile: { + firstName: "Joe", + nickname: "Joey" + } + } + ) { + profile { + id + firstName + lastName + nickname + sensitivedata { + ssn + } + } + } + } + """ + ) + query = t.substitute(claimToken=claim_token.token) + executed = user_gql_client.execute(query, allowed_data_fields=["name"]) + + assert "errors" in executed + assert_match_error_code(executed, "FIELD_NOT_ALLOWED_ERROR") + + def test_can_not_change_primary_email_to_non_primary(user_gql_client): profile = ProfileFactory(user=None) email = EmailFactory(profile=profile, primary=True) @@ -112,7 +148,9 @@ def test_can_not_change_primary_email_to_non_primary(user_gql_client): }, } - executed = user_gql_client.execute(CLAIM_PROFILE_MUTATION, variables=variables) + executed = user_gql_client.execute( + CLAIM_PROFILE_MUTATION, variables=variables, allowed_data_fields=["email"] + ) assert_match_error_code(executed, "PROFILE_MUST_HAVE_PRIMARY_EMAIL") @@ -128,6 +166,7 @@ def test_can_not_delete_primary_email(user_gql_client): "token": str(claim_token.token), "profileInput": {"removeEmails": email_deletes}, }, + allowed_data_fields=["email"], ) assert_match_error_code(executed, "PROFILE_MUST_HAVE_PRIMARY_EMAIL") @@ -172,6 +211,7 @@ def test_changing_an_email_address_marks_it_unverified( CLAIM_PROFILE_MUTATION, variables=variables, execution_context_class=execution_context_class, + allowed_data_fields=["email"], ) assert "errors" not in executed assert executed["data"] == expected_data @@ -189,7 +229,9 @@ def execute_query(self, user_gql_client, profile_input): "profileInput": profile_input, } - return user_gql_client.execute(CLAIM_PROFILE_MUTATION, variables=variables) + return user_gql_client.execute( + CLAIM_PROFILE_MUTATION, variables=variables, allowed_data_fields=["email"] + ) def test_user_cannot_claim_claimable_profile_if_token_expired(user_gql_client): @@ -223,7 +265,7 @@ def test_user_cannot_claim_claimable_profile_if_token_expired(user_gql_client): """ ) query = t.substitute(claimToken=expired_claim_token.token) - executed = user_gql_client.execute(query) + executed = user_gql_client.execute(query, allowed_data_fields=["name"]) assert "errors" in executed assert executed["errors"][0]["extensions"]["code"] == TOKEN_EXPIRED_ERROR @@ -237,7 +279,9 @@ def test_using_non_existing_token_produces_an_object_does_not_exist_error( variables = { "token": non_existing_token, } - executed = user_gql_client.execute(CLAIM_PROFILE_MUTATION, variables=variables) + executed = user_gql_client.execute( + CLAIM_PROFILE_MUTATION, variables=variables, allowed_data_fields=["email"] + ) assert_match_error_code(executed, "OBJECT_DOES_NOT_EXIST_ERROR") @@ -270,7 +314,7 @@ def test_user_cannot_claim_claimable_profile_with_existing_profile(user_gql_clie """ ) query = t.substitute(claimToken=claim_token.token) - executed = user_gql_client.execute(query) + executed = user_gql_client.execute(query, allowed_data_fields=["name"]) assert "errors" in executed assert ( diff --git a/profiles/tests/test_gql_claimable_profile_query.py b/profiles/tests/test_gql_claimable_profile_query.py index 059d7037..5ac0358e 100644 --- a/profiles/tests/test_gql_claimable_profile_query.py +++ b/profiles/tests/test_gql_claimable_profile_query.py @@ -33,7 +33,7 @@ def test_can_query_claimable_profile_with_token(user_gql_client): "lastName": profile.last_name, } } - executed = user_gql_client.execute(query) + executed = user_gql_client.execute(query, allowed_data_fields=["name"]) assert "errors" not in executed assert dict(executed["data"]) == expected_data diff --git a/profiles/tests/test_gql_create_my_profile_mutation.py b/profiles/tests/test_gql_create_my_profile_mutation.py index 8dc7358c..c25a2e19 100644 --- a/profiles/tests/test_gql_create_my_profile_mutation.py +++ b/profiles/tests/test_gql_create_my_profile_mutation.py @@ -84,7 +84,9 @@ def test_normal_user_can_create_profile( primary=str(email_is_primary).lower(), ssn=ssn, ) - executed = user_gql_client.execute(mutation) + executed = user_gql_client.execute( + mutation, allowed_data_fields=["email", "name", "personalidentitycode"] + ) assert "errors" not in executed assert executed["data"] == expected_data @@ -115,10 +117,46 @@ def test_normal_user_can_create_profile_with_no_email(user_gql_client, email_dat expected_data = {"createMyProfile": {"profile": {"emails": {"edges": []}}}} - executed = user_gql_client.execute(mutation) + executed = user_gql_client.execute(mutation, allowed_data_fields=["email"]) assert executed["data"] == expected_data +def test_cant_query_fields_not_allowed_in_create_mutation(user_gql_client, email_data): + mutation = """ + mutation { + createMyProfile( + input: { + profile: {} + } + ) { + profile { + firstName + emails { + edges { + node { + email, + emailType, + primary, + verified + } + } + } + } + } + } + """ + + expected_data = { + "profile": {"firstName": user_gql_client.user.first_name, "emails": None} + } + + executed = user_gql_client.execute(mutation, allowed_data_fields=["name"]) + + assert "errors" in executed + assert expected_data == executed["data"]["createMyProfile"] + assert_match_error_code(executed, "FIELD_NOT_ALLOWED_ERROR") + + class TestProfileInputValidation(ProfileInputValidationBase): def execute_query(self, user_gql_client, profile_input): mutation = """ diff --git a/profiles/tests/test_gql_create_or_update_profile_with_verified_personal_information_mutation.py b/profiles/tests/test_gql_create_or_update_profile_with_verified_personal_information_mutation.py index ac5156c7..08eb9895 100644 --- a/profiles/tests/test_gql_create_or_update_profile_with_verified_personal_information_mutation.py +++ b/profiles/tests/test_gql_create_or_update_profile_with_verified_personal_information_mutation.py @@ -41,7 +41,7 @@ def execute_mutation(input_data, gql_client): } """ - return gql_client.execute(query, variables={"input": input_data}, service=None) + return gql_client.execute(query, variables={"input": input_data}) def execute_successful_mutation(input_data, gql_client): diff --git a/profiles/tests/test_gql_create_or_update_user_profile_mutation.py b/profiles/tests/test_gql_create_or_update_user_profile_mutation.py index 9db71829..04051646 100644 --- a/profiles/tests/test_gql_create_or_update_user_profile_mutation.py +++ b/profiles/tests/test_gql_create_or_update_user_profile_mutation.py @@ -35,13 +35,13 @@ def execute_mutation(input_data, gql_client): input: $input, ) { profile { - id, + id } } } """ - return gql_client.execute(query, variables={"input": input_data}, service=None) + return gql_client.execute(query, variables={"input": input_data}) def execute_successful_mutation(input_data, gql_client): diff --git a/profiles/tests/test_gql_create_profile_mutation.py b/profiles/tests/test_gql_create_profile_mutation.py index d75e69ac..67b7bc09 100644 --- a/profiles/tests/test_gql_create_profile_mutation.py +++ b/profiles/tests/test_gql_create_profile_mutation.py @@ -146,7 +146,9 @@ def test_staff_user_can_create_a_profile( } } } - executed = user_gql_client.execute(query, service=service) + executed = user_gql_client.execute( + query, service=service, allowed_data_fields=["name", "phone", "email"] + ) assert executed["data"] == expected_data @@ -229,7 +231,9 @@ def test_staff_user_with_sensitive_data_service_accesss_can_create_a_profile_wit "profile": {"firstName": "John", "sensitivedata": {"ssn": "121282-123E"}} } } - executed = user_gql_client.execute(query, service=service) + executed = user_gql_client.execute( + query, service=service, allowed_data_fields=["name", "personalidentitycode"] + ) assert executed["data"] == expected_data @@ -280,6 +284,7 @@ def test_staff_user_cannot_create_a_profile_with_sensitive_data_without_sensitiv assert executed["errors"][0]["message"] == _( "You do not have permission to perform this action." ) + assert executed["data"]["createProfile"] is None class TestProfileInputValidation(ProfileInputValidationBase): diff --git a/profiles/tests/test_gql_federation.py b/profiles/tests/test_gql_federation.py index e2c1f500..eeb76698 100644 --- a/profiles/tests/test_gql_federation.py +++ b/profiles/tests/test_gql_federation.py @@ -149,7 +149,10 @@ def test_owner_can_resolve_profile_entity( "_entities": [{"id": profile._global_id, "firstName": profile.first_name}] } executed = user_gql_client.execute( - ENTITY_QUERY, variables=variables, service=service if with_service else None + ENTITY_QUERY, + variables=variables, + service=service if with_service else None, + allowed_data_fields=["name"] if with_service else None, ) if with_service and with_serviceconnection: @@ -208,7 +211,7 @@ def test_staff_user_can_resolve_profile_entity( "_entities": [{"id": profile._global_id, "firstName": profile.first_name}] } executed = user_gql_client.execute( - ENTITY_QUERY, variables=variables, service=service + ENTITY_QUERY, variables=variables, service=service, allowed_data_fields=["name"] ) if with_serviceconnection: diff --git a/profiles/tests/test_gql_my_profile_query.py b/profiles/tests/test_gql_my_profile_query.py index 2b79339c..cf1fcf50 100644 --- a/profiles/tests/test_gql_my_profile_query.py +++ b/profiles/tests/test_gql_my_profile_query.py @@ -2,7 +2,11 @@ from open_city_profile.tests import to_graphql_name from open_city_profile.tests.asserts import assert_match_error_code -from services.tests.factories import ServiceConnectionFactory +from services.tests.factories import ( + AllowedDataFieldFactory, + ServiceConnectionFactory, + ServiceFactory, +) from .conftest import VERIFIED_PERSONAL_INFORMATION_ADDRESS_TYPES from .factories import ( @@ -16,7 +20,7 @@ ) -def test_normal_user_can_query_emails(user_gql_client): +def test_normal_user_can_query_emails(user_gql_client, service): profile = ProfileWithPrimaryEmailFactory(user=user_gql_client.user) email = profile.emails.first() @@ -50,11 +54,14 @@ def test_normal_user_can_query_emails(user_gql_client): } } } - executed = user_gql_client.execute(query) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="email")) + ServiceConnectionFactory(profile=profile, service=service) + + executed = user_gql_client.execute(query, service=service) assert dict(executed["data"]) == expected_data -def test_normal_user_can_query_phones(user_gql_client): +def test_normal_user_can_query_phones(user_gql_client, service): profile = ProfileWithPrimaryEmailFactory(user=user_gql_client.user) phone = PhoneFactory(profile=profile) @@ -88,11 +95,14 @@ def test_normal_user_can_query_phones(user_gql_client): } } } - executed = user_gql_client.execute(query) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="phone")) + ServiceConnectionFactory(profile=profile, service=service) + + executed = user_gql_client.execute(query, service=service) assert dict(executed["data"]) == expected_data -def test_normal_user_can_query_addresses(user_gql_client): +def test_normal_user_can_query_addresses(user_gql_client, service): profile = ProfileWithPrimaryEmailFactory(user=user_gql_client.user) address = AddressFactory(profile=profile) @@ -126,12 +136,15 @@ def test_normal_user_can_query_addresses(user_gql_client): } } } - executed = user_gql_client.execute(query) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="address")) + ServiceConnectionFactory(profile=profile, service=service) + + executed = user_gql_client.execute(query, service=service) assert dict(executed["data"]) == expected_data def test_normal_user_can_query_primary_contact_details( - user_gql_client, execution_context_class + user_gql_client, execution_context_class, service ): profile = ProfileFactory(user=user_gql_client.user) phone = PhoneFactory(profile=profile, primary=True) @@ -181,8 +194,15 @@ def test_normal_user_can_query_primary_contact_details( }, } } + service.allowed_data_fields.add( + AllowedDataFieldFactory(field_name="phone"), + AllowedDataFieldFactory(field_name="email"), + AllowedDataFieldFactory(field_name="address"), + ) + ServiceConnectionFactory(profile=profile, service=service) + executed = user_gql_client.execute( - query, execution_context_class=execution_context_class + query, execution_context_class=execution_context_class, service=service ) assert dict(executed["data"]) == expected_data @@ -218,31 +238,56 @@ class TestProfileWithVerifiedPersonalInformation: } """ - @staticmethod - def _execute_query(gql_client, loa="substantial"): + @pytest.fixture(autouse=True) + def setup_data(self, db, user_gql_client): + self.user = user_gql_client.user + self.client = user_gql_client + self.service = ServiceFactory(is_profile_service=True) + self.profile = ProfileFactory(user=user_gql_client.user) + ServiceConnectionFactory(profile=self.profile, service=self.service) + self._add_allowed_data_fields_to_service(self.service) + + def _create_allowed_data_fields(self): + self.allowed_name = AllowedDataFieldFactory(field_name="name") + self.allowed_address = AllowedDataFieldFactory(field_name="address") + self.allowed_personal_identity_code = AllowedDataFieldFactory( + field_name="personalidentitycode" + ) + + def _add_allowed_data_fields_to_service(self, service): + if not getattr(self, "allowed_name", None): + self._create_allowed_data_fields() + + service.allowed_data_fields.add( + self.allowed_name, + self.allowed_address, + self.allowed_personal_identity_code, + ) + + def _execute_query(self, loa="substantial", service=None): token_payload = { "loa": loa, } - return gql_client.execute( + kwargs = {"service": self.service} + if service: + kwargs["service"] = service + + return self.client.execute( TestProfileWithVerifiedPersonalInformation.QUERY, auth_token_payload=token_payload, + **kwargs, ) - def test_when_verified_personal_infomation_does_not_exist_returns_null( - self, user_gql_client - ): - ProfileFactory(user=user_gql_client.user) - - executed = self._execute_query(user_gql_client) + def test_when_verified_personal_information_does_not_exist_returns_null(self): + executed = self._execute_query() assert "errors" not in executed assert executed["data"]["myProfile"]["verifiedPersonalInformation"] is None - def test_normal_user_can_query_verified_personal_information(self, user_gql_client): - profile = ProfileFactory(user=user_gql_client.user) + def test_normal_user_can_query_verified_personal_information(self): verified_personal_information = VerifiedPersonalInformationFactory( - profile=profile + profile=self.profile ) permanent_address = verified_personal_information.permanent_address @@ -279,20 +324,17 @@ def test_normal_user_can_query_verified_personal_information(self, user_gql_clie } } - executed = self._execute_query(user_gql_client) + executed = self._execute_query() assert executed["data"] == expected_data @pytest.mark.parametrize( "address_type", VERIFIED_PERSONAL_INFORMATION_ADDRESS_TYPES ) - def test_when_address_does_not_exist_returns_null( - self, address_type, user_gql_client - ): - profile = ProfileFactory(user=user_gql_client.user) - VerifiedPersonalInformationFactory(profile=profile, **{address_type: None}) + def test_when_address_does_not_exist_returns_null(self, address_type): + VerifiedPersonalInformationFactory(profile=self.profile, **{address_type: None}) - executed = self._execute_query(user_gql_client) + executed = self._execute_query() assert "errors" not in executed @@ -305,11 +347,10 @@ def test_when_address_does_not_exist_returns_null( assert isinstance(received_address, dict) @pytest.mark.parametrize("loa", ["substantial", "high"]) - def test_high_enough_level_of_assurance_gains_access(self, loa, user_gql_client): - profile = ProfileFactory(user=user_gql_client.user) - VerifiedPersonalInformationFactory(profile=profile) + def test_high_enough_level_of_assurance_gains_access(self, loa): + VerifiedPersonalInformationFactory(profile=self.profile) - executed = self._execute_query(user_gql_client, loa) + executed = self._execute_query(loa) assert not hasattr(executed, "errors") assert isinstance( @@ -317,26 +358,24 @@ def test_high_enough_level_of_assurance_gains_access(self, loa, user_gql_client) ) @pytest.mark.parametrize("loa", [None, "low", "unknown"]) - def test_too_low_level_of_assurance_denies_access(self, loa, user_gql_client): - profile = ProfileFactory(user=user_gql_client.user) - VerifiedPersonalInformationFactory(profile=profile) + def test_too_low_level_of_assurance_denies_access(self, loa): + VerifiedPersonalInformationFactory(profile=self.profile) - executed = self._execute_query(user_gql_client, loa) + executed = self._execute_query(loa) assert_match_error_code(executed, "PERMISSION_DENIED_ERROR") assert executed["data"]["myProfile"]["verifiedPersonalInformation"] is None @pytest.mark.parametrize("with_serviceconnection", (True, False)) - def test_service_connection_required( - self, user_gql_client, service, with_serviceconnection - ): - profile = ProfileFactory(user=user_gql_client.user) - VerifiedPersonalInformationFactory(profile=profile) + def test_service_connection_required(self, with_serviceconnection): + service = ServiceFactory() + self._add_allowed_data_fields_to_service(service) + VerifiedPersonalInformationFactory(profile=self.profile) if with_serviceconnection: - ServiceConnectionFactory(profile=profile, service=service) + ServiceConnectionFactory(profile=self.profile, service=service) - executed = user_gql_client.execute( + executed = self.client.execute( TestProfileWithVerifiedPersonalInformation.QUERY, auth_token_payload={"loa": "substantial"}, service=service, @@ -372,6 +411,7 @@ def test_querying_non_existent_profile_doesnt_return_errors(user_gql_client, ser def test_normal_user_can_query_their_own_profile( user_gql_client, service, with_service, with_serviceconnection ): + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) profile = ProfileFactory(user=user_gql_client.user) if with_serviceconnection: ServiceConnectionFactory(profile=profile, service=service) @@ -399,8 +439,15 @@ def test_normal_user_can_query_their_own_profile( assert executed["data"]["myProfile"] is None -def test_normal_user_can_query_their_own_profiles_sensitivedata(user_gql_client): +def test_normal_user_can_query_their_own_profiles_sensitivedata( + user_gql_client, service +): + service.allowed_data_fields.add( + AllowedDataFieldFactory(field_name="name"), + AllowedDataFieldFactory(field_name="personalidentitycode"), + ) profile = ProfileFactory(user=user_gql_client.user) + ServiceConnectionFactory(profile=profile, service=service) sensitive_data = SensitiveDataFactory(profile=profile) query = """ @@ -419,5 +466,113 @@ def test_normal_user_can_query_their_own_profiles_sensitivedata(user_gql_client) "sensitivedata": {"ssn": sensitive_data.ssn}, } } - executed = user_gql_client.execute(query) + executed = user_gql_client.execute(query, service=service) assert dict(executed["data"]) == expected_data + + +def test_my_profile_results_error_if_querying_fields_not_allowed( + user_gql_client, service +): + profile = ProfileFactory(user=user_gql_client.user) + ServiceConnectionFactory(profile=profile, service=service) + + query = """ + { + myProfile { + firstName + lastName + sensitivedata { + ssn + } + } + } + """ + executed = user_gql_client.execute(query, service=service) + assert_match_error_code(executed, "FIELD_NOT_ALLOWED_ERROR") + assert executed["data"]["myProfile"] is None + + +def test_my_profile_results_error_if_querying_fields_not_allowed_and_shows_allowed( + user_gql_client, service +): + profile = ProfileFactory(user=user_gql_client.user) + ServiceConnectionFactory(profile=profile, service=service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) + + query = """ + { + myProfile { + firstName + lastName + sensitivedata { + ssn + } + } + } + """ + executed = user_gql_client.execute(query, service=service) + assert_match_error_code(executed, "FIELD_NOT_ALLOWED_ERROR") + assert executed["data"]["myProfile"]["firstName"] == profile.first_name + assert executed["data"]["myProfile"]["lastName"] == profile.last_name + assert executed["data"]["myProfile"]["sensitivedata"] is None + + +def test_my_profile_checks_allowed_data_fields_for_single_query( + user_gql_client, service +): + profile = ProfileFactory(user=user_gql_client.user) + ServiceConnectionFactory(profile=profile, service=service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) + + query = """ + { + myProfile { + firstName + lastName + sensitivedata { + ssn + } + } + } + """ + executed = user_gql_client.execute(query, service=service) + assert_match_error_code(executed, "FIELD_NOT_ALLOWED_ERROR") + assert executed["data"]["myProfile"]["firstName"] == profile.first_name + assert executed["data"]["myProfile"]["lastName"] == profile.last_name + assert executed["data"]["myProfile"]["sensitivedata"] is None + + +def test_my_profile_checks_allowed_data_fields_for_multiple_queries( + user_gql_client, service +): + profile = ProfileFactory(user=user_gql_client.user) + ServiceConnectionFactory(profile=profile, service=service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) + + query = """ + { + _service { + __typename + } + myProfile { + firstName + lastName + sensitivedata { + ssn + } + } + services { + edges { + node { + name + } + } + } + } + """ + executed = user_gql_client.execute(query, service=service) + assert_match_error_code(executed, "FIELD_NOT_ALLOWED_ERROR") + assert executed["data"]["myProfile"]["firstName"] == profile.first_name + assert executed["data"]["myProfile"]["lastName"] == profile.last_name + assert executed["data"]["myProfile"]["sensitivedata"] is None + assert executed["data"]["services"] is None diff --git a/profiles/tests/test_gql_profile_query.py b/profiles/tests/test_gql_profile_query.py index 75efbf72..3b157fde 100644 --- a/profiles/tests/test_gql_profile_query.py +++ b/profiles/tests/test_gql_profile_query.py @@ -7,7 +7,7 @@ from open_city_profile.consts import OBJECT_DOES_NOT_EXIST_ERROR from open_city_profile.tests.asserts import assert_match_error_code -from services.tests.factories import ServiceConnectionFactory +from services.tests.factories import AllowedDataFieldFactory, ServiceConnectionFactory from ..schema import ProfileNode from .factories import SensitiveDataFactory @@ -44,6 +44,7 @@ def test_staff_user_can_query_a_profile_connected_to_service_he_is_admin_of( user = user_gql_client.user user.groups.add(group) assign_perm("can_view_profiles", group, service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) # serviceType is included in query just to ensure that it has NO affect t = Template( @@ -74,6 +75,7 @@ def test_staff_user_cannot_query_a_profile_without_id( user = user_gql_client.user user.groups.add(group) assign_perm("can_view_profiles", group, service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) query = """ { @@ -98,6 +100,9 @@ def test_staff_user_cannot_query_a_profile_without_a_connection_to_their_service other_service = service_factory() ServiceConnectionFactory(profile=profile, service=other_service) + adf_name = AllowedDataFieldFactory(field_name="name") + other_service.allowed_data_fields.add(adf_name) + staff_user_service.allowed_data_fields.add(adf_name) t = Template( """ @@ -127,6 +132,9 @@ def test_staff_user_cannot_query_sensitive_data_with_only_profile_permissions( user = user_gql_client.user user.groups.add(group) assign_perm("can_view_profiles", group, service) + service.allowed_data_fields.add( + AllowedDataFieldFactory(field_name="personalidentitycode") + ) t = Template( """ @@ -158,6 +166,9 @@ def test_staff_user_can_query_sensitive_data_with_given_permissions( user.groups.add(group) assign_perm("can_view_profiles", group, service) assign_perm("can_view_sensitivedata", group, service) + service.allowed_data_fields.add( + AllowedDataFieldFactory(field_name="personalidentitycode") + ) t = Template( """ @@ -188,6 +199,9 @@ def test_staff_receives_null_sensitive_data_if_it_does_not_exist( user.groups.add(group) assign_perm("can_view_profiles", group, service) assign_perm("can_view_sensitivedata", group, service) + service.allowed_data_fields.add( + AllowedDataFieldFactory(field_name="personalidentitycode") + ) t = Template( """ @@ -229,6 +243,7 @@ def test_staff_user_needs_required_permission_to_access_verified_personal_inform ServiceConnectionFactory( profile=profile_with_verified_personal_information, service=service ) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) user = user_gql_client.user user.groups.add(group) @@ -273,3 +288,83 @@ def test_staff_user_needs_required_permission_to_access_verified_personal_inform else: assert_match_error_code(executed, "PERMISSION_DENIED_ERROR") assert executed["data"] == {"profile": {"verifiedPersonalInformation": None}} + + +def test_profile_checks_allowed_data_fields_for_single_query( + user_gql_client, service, profile, group +): + user = user_gql_client.user + user.groups.add(group) + assign_perm("can_view_profiles", group, service) + assign_perm("can_view_sensitivedata", group, service) + ServiceConnectionFactory(profile=profile, service=service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) + + query = """ + { + profile(id: "%s") { + firstName + lastName + sensitivedata { + ssn + } + } + } + """ % relay.Node.to_global_id( + ProfileNode._meta.name, profile.id + ) + + executed = user_gql_client.execute(query, service=service) + assert_match_error_code(executed, "FIELD_NOT_ALLOWED_ERROR") + assert executed["data"]["profile"]["firstName"] == profile.first_name + assert executed["data"]["profile"]["lastName"] == profile.last_name + assert executed["data"]["profile"]["sensitivedata"] is None + + +def test_my_profile_checks_allowed_data_fields_for_multiple_queries( + user_gql_client, service, profile, group +): + user = user_gql_client.user + user.groups.add(group) + assign_perm("can_view_profiles", group, service) + assign_perm("can_view_sensitivedata", group, service) + ServiceConnectionFactory(profile=profile, service=service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) + + query = """ + { + myProfile { + firstName + lastName + sensitivedata { + ssn + } + } + _service { + __typename + } + profile(id: "%s") { + firstName + lastName + sensitivedata { + ssn + } + } + services { + edges { + node { + name + } + } + } + } + """ % relay.Node.to_global_id( + ProfileNode._meta.name, profile.id + ) + + executed = user_gql_client.execute(query, service=service) + assert_match_error_code(executed, "FIELD_NOT_ALLOWED_ERROR") + assert executed["data"]["profile"]["firstName"] == profile.first_name + assert executed["data"]["profile"]["lastName"] == profile.last_name + assert executed["data"]["profile"]["sensitivedata"] is None + assert executed["data"]["services"] is None diff --git a/profiles/tests/test_gql_profile_with_access_token_query.py b/profiles/tests/test_gql_profile_with_access_token_query.py index 1df87249..ea714874 100644 --- a/profiles/tests/test_gql_profile_with_access_token_query.py +++ b/profiles/tests/test_gql_profile_with_access_token_query.py @@ -26,7 +26,9 @@ def test_anonymous_user_can_retrieve_a_profile_with_temporary_read_access_token( profile = ProfileFactory() token = TemporaryReadAccessTokenFactory(profile=profile) - executed = anon_user_gql_client.execute(self.query(token.token)) + executed = anon_user_gql_client.execute( + self.query(token.token), allowed_data_fields=["name"] + ) assert "errors" not in executed actual_profile = executed["data"]["profileWithAccessToken"] diff --git a/profiles/tests/test_gql_profiles_query.py b/profiles/tests/test_gql_profiles_query.py index bf87f65d..4ebc956c 100644 --- a/profiles/tests/test_gql_profiles_query.py +++ b/profiles/tests/test_gql_profiles_query.py @@ -8,7 +8,7 @@ from open_city_profile.tests import to_graphql_name from open_city_profile.tests.asserts import assert_match_error_code from profiles.enums import AddressType, EmailType, PhoneType -from services.tests.factories import ServiceConnectionFactory +from services.tests.factories import AllowedDataFieldFactory, ServiceConnectionFactory from .factories import ( AddressFactory, @@ -41,6 +41,7 @@ def test_normal_user_can_not_query_profiles(user_gql_client, service): def test_admin_user_can_query_profiles(superuser_gql_client, profile, service): ServiceConnectionFactory(profile=profile, service=service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) query = """ { @@ -73,6 +74,47 @@ def test_admin_user_can_query_profiles(superuser_gql_client, profile, service): assert executed["data"] == expected_data +def test_admin_user_cant_query_fields_that_are_not_allowed( + superuser_gql_client, profile, service +): + ServiceConnectionFactory(profile=profile, service=service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) + + query = """ + { + profiles { + edges { + node { + firstName + lastName + nickname + emails { edges { node { email } } } + } + } + } + } + """ + + expected_data = { + "profiles": { + "edges": [ + { + "node": { + "firstName": profile.first_name, + "lastName": profile.last_name, + "nickname": profile.nickname, + "emails": None, + } + } + ] + } + } + executed = superuser_gql_client.execute(query, service=service) + assert executed["data"] == expected_data + assert "errors" in executed + assert_match_error_code(executed, "FIELD_NOT_ALLOWED_ERROR") + + def test_staff_user_with_group_access_can_query_profiles( user_gql_client, profile, group, service ): @@ -80,6 +122,7 @@ def test_staff_user_with_group_access_can_query_profiles( user = user_gql_client.user user.groups.add(group) assign_perm("can_view_profiles", group, service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) # serviceType is included in query just to ensure that it has NO affect query = """ @@ -154,6 +197,7 @@ def test_staff_user_can_filter_profiles_by_a_field( profile_1, profile_2 = ProfileFactory.create_batch(2) ServiceConnectionFactory(profile=profile_1, service=service) ServiceConnectionFactory(profile=profile_2, service=service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) user = user_gql_client.user user.groups.add(group) assign_perm("can_view_profiles", group, service) @@ -189,6 +233,7 @@ def test_staff_user_filter_profiles_by_verified_personal_information_permissions vpi = VerifiedPersonalInformationFactory() ServiceConnectionFactory(profile=vpi.profile, service=service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) user = user_gql_client.user user.groups.add(group) @@ -235,6 +280,7 @@ def test_staff_user_can_sort_profiles(user_gql_client, group, service): ) ServiceConnectionFactory(profile=profile_1, service=service) ServiceConnectionFactory(profile=profile_2, service=service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) user = user_gql_client.user user.groups.add(group) assign_perm("can_view_profiles", group, service) @@ -298,6 +344,7 @@ def test_staff_user_can_sort_profiles_by_custom_fields( EmailFactory(profile=profile_2, email="bryan.tester@example.com", primary=True) ServiceConnectionFactory(profile=profile_1, service=service) ServiceConnectionFactory(profile=profile_2, service=service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) user = user_gql_client.user user.groups.add(group) assign_perm("can_view_profiles", group, service) @@ -658,6 +705,7 @@ def test_staff_user_can_paginate_profiles( ): ServiceConnectionFactory(profile=profile, service=service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) user = user_gql_client.user user.groups.add(group) assign_perm("can_view_profiles", group, service) @@ -699,12 +747,15 @@ def test_staff_user_with_group_access_can_query_only_profiles_he_has_access_to( entitled_profile = ProfileFactory() entitled_service = service_factory() + adf_name = AllowedDataFieldFactory(field_name="name") ServiceConnectionFactory(profile=entitled_profile, service=entitled_service) assign_perm("can_view_profiles", group, entitled_service) + entitled_service.allowed_data_fields.add(adf_name) unentitled_profile = ProfileFactory() unentitled_service = service_factory() ServiceConnectionFactory(profile=unentitled_profile, service=unentitled_service) + unentitled_service.allowed_data_fields.add(adf_name) query = """ { diff --git a/profiles/tests/test_gql_relay_ordering.py b/profiles/tests/test_gql_relay_ordering.py index c1d55523..616be10e 100644 --- a/profiles/tests/test_gql_relay_ordering.py +++ b/profiles/tests/test_gql_relay_ordering.py @@ -61,8 +61,14 @@ """ -def test_addresses_are_ordered_first_by_primary_then_by_id(user_gql_client): +def test_addresses_are_ordered_first_by_primary_then_by_id(user_gql_client, service): profile = ProfileFactory(user=user_gql_client.user) + ServiceConnectionFactory(profile=profile, service=service) + service.allowed_data_fields.add( + AllowedDataFieldFactory(field_name="address"), + AllowedDataFieldFactory(field_name="email"), + AllowedDataFieldFactory(field_name="phone"), + ) first_address = AddressFactory(profile=profile, primary=False) primary_address = AddressFactory(profile=profile, primary=True) second_address = AddressFactory(profile=profile, primary=False) @@ -74,12 +80,18 @@ def test_addresses_are_ordered_first_by_primary_then_by_id(user_gql_client): ) ) - executed = user_gql_client.execute(QUERY) + executed = user_gql_client.execute(QUERY, service=service) assert executed["data"]["myProfile"]["addresses"]["edges"] == expected_edges -def test_emails_are_ordered_first_by_primary_then_by_id(user_gql_client): +def test_emails_are_ordered_first_by_primary_then_by_id(user_gql_client, service): profile = ProfileFactory(user=user_gql_client.user) + ServiceConnectionFactory(profile=profile, service=service) + service.allowed_data_fields.add( + AllowedDataFieldFactory(field_name="address"), + AllowedDataFieldFactory(field_name="email"), + AllowedDataFieldFactory(field_name="phone"), + ) first_email = EmailFactory(profile=profile, primary=False) primary_email = EmailFactory(profile=profile, primary=True) second_email = EmailFactory(profile=profile, primary=False) @@ -91,12 +103,18 @@ def test_emails_are_ordered_first_by_primary_then_by_id(user_gql_client): ) ) - executed = user_gql_client.execute(QUERY) + executed = user_gql_client.execute(QUERY, service=service) assert executed["data"]["myProfile"]["emails"]["edges"] == expected_edges -def test_phones_are_ordered_first_by_primary_then_by_id(user_gql_client): +def test_phones_are_ordered_first_by_primary_then_by_id(user_gql_client, service): profile = ProfileFactory(user=user_gql_client.user) + ServiceConnectionFactory(profile=profile, service=service) + service.allowed_data_fields.add( + AllowedDataFieldFactory(field_name="address"), + AllowedDataFieldFactory(field_name="email"), + AllowedDataFieldFactory(field_name="phone"), + ) first_phone = PhoneFactory(profile=profile, primary=False) primary_phone = PhoneFactory(profile=profile, primary=True) second_phone = PhoneFactory(profile=profile, primary=False) @@ -108,7 +126,7 @@ def test_phones_are_ordered_first_by_primary_then_by_id(user_gql_client): ) ) - executed = user_gql_client.execute(QUERY) + executed = user_gql_client.execute(QUERY, service=service) assert executed["data"]["myProfile"]["phones"]["edges"] == expected_edges diff --git a/profiles/tests/test_gql_update_my_profile_mutation.py b/profiles/tests/test_gql_update_my_profile_mutation.py index 8a5d4a7a..a8fd189d 100644 --- a/profiles/tests/test_gql_update_my_profile_mutation.py +++ b/profiles/tests/test_gql_update_my_profile_mutation.py @@ -100,7 +100,9 @@ def test_update_profile( email_type=email_data["email_type"], primary=str(email_data["primary"]).lower(), ) - executed = user_gql_client.execute(mutation, service=service) + executed = user_gql_client.execute( + mutation, service=service, allowed_data_fields=["name", "email"] + ) if with_serviceconnection: assert executed["data"] == expected_data @@ -152,7 +154,7 @@ def test_update_profile_without_email(user_gql_client, profile_data): } mutation = t.substitute(nickname=profile_data["nickname"]) - executed = user_gql_client.execute(mutation) + executed = user_gql_client.execute(mutation, allowed_data_fields=["name", "email"]) assert executed["data"] == expected_data @@ -299,7 +301,7 @@ def test_add_email(user_gql_client, email_data): email_type=email_data["email_type"], primary=str(not email_data["primary"]).lower(), ) - executed = user_gql_client.execute(mutation) + executed = user_gql_client.execute(mutation, allowed_data_fields=["email"]) assert dict(executed["data"]) == expected_data @@ -358,7 +360,9 @@ def test_update_email( ] executed = user_gql_client.execute( - EMAILS_MUTATION, variables={"profileInput": {"updateEmails": email_updates}} + EMAILS_MUTATION, + variables={"profileInput": {"updateEmails": email_updates}}, + allowed_data_fields=["email"], ) if succeeds: @@ -399,7 +403,9 @@ def test_can_not_update_email_of_another_profile(user_gql_client): } ] executed = user_gql_client.execute( - EMAILS_MUTATION, variables={"profileInput": {"updateEmails": email_updates}} + EMAILS_MUTATION, + variables={"profileInput": {"updateEmails": email_updates}}, + allowed_data_fields=["email"], ) assert executed["data"]["updateMyProfile"] is None @@ -445,7 +451,9 @@ def test_change_primary_email_to_another_one(user_gql_client): {"id": to_global_id(type="EmailNode", id=email.id), "primary": True} ] executed = user_gql_client.execute( - EMAILS_MUTATION, variables={"profileInput": {"updateEmails": email_updates}} + EMAILS_MUTATION, + variables={"profileInput": {"updateEmails": email_updates}}, + allowed_data_fields=["email"], ) assert dict(executed["data"]) == expected_data @@ -458,7 +466,9 @@ def test_can_not_change_primary_email_to_non_primary(user_gql_client): {"id": to_global_id(type="EmailNode", id=email.id), "primary": False} ] executed = user_gql_client.execute( - EMAILS_MUTATION, variables={"profileInput": {"updateEmails": email_updates}} + EMAILS_MUTATION, + variables={"profileInput": {"updateEmails": email_updates}}, + allowed_data_fields=["email"], ) assert_match_error_code(executed, "PROFILE_MUST_HAVE_PRIMARY_EMAIL") @@ -469,7 +479,9 @@ def test_can_not_delete_primary_email(user_gql_client): email_deletes = [to_global_id(type="EmailNode", id=email.id)] executed = user_gql_client.execute( - EMAILS_MUTATION, variables={"profileInput": {"removeEmails": email_deletes}} + EMAILS_MUTATION, + variables={"profileInput": {"removeEmails": email_deletes}}, + allowed_data_fields=["email"], ) assert_match_error_code(executed, "PROFILE_MUST_HAVE_PRIMARY_EMAIL") @@ -502,6 +514,7 @@ def test_can_replace_a_primary_email_with_a_newly_created_one( "updateEmails": email_updates, } }, + allowed_data_fields=["email"], ) new_primary_email = Email.objects.get(email=email_data["email"]) @@ -571,7 +584,9 @@ def test_changing_an_email_address_marks_it_unverified(user_gql_client): {"id": to_global_id("EmailNode", email.id), "email": new_email_value} ] executed = user_gql_client.execute( - EMAILS_MUTATION, variables={"profileInput": {"updateEmails": email_updates}} + EMAILS_MUTATION, + variables={"profileInput": {"updateEmails": email_updates}}, + allowed_data_fields=["email"], ) assert dict(executed["data"]) == expected_data @@ -599,7 +614,9 @@ def test_remove_email(global_id_type, global_id_id, succeeds, user_gql_client): to_global_id(type=global_id_type, id=global_id_id), ] executed = user_gql_client.execute( - EMAILS_MUTATION, variables={"profileInput": {"removeEmails": email_deletes}} + EMAILS_MUTATION, + variables={"profileInput": {"removeEmails": email_deletes}}, + allowed_data_fields=["email"], ) if succeeds: @@ -659,7 +676,9 @@ def test_remove_all_emails_if_they_are_not_primary(user_gql_client): to_global_id(type="EmailNode", id=email2.id), ] executed = user_gql_client.execute( - EMAILS_MUTATION, variables={"profileInput": {"removeEmails": email_deletes}} + EMAILS_MUTATION, + variables={"profileInput": {"removeEmails": email_deletes}}, + allowed_data_fields=["email"], ) assert executed["data"] == expected_data @@ -740,6 +759,7 @@ def test_add_phone(user_gql_client, phone_data): ] } }, + allowed_data_fields=["phone"], ) phone = profile.phones.get() @@ -800,6 +820,7 @@ def test_update_phone( ] } }, + allowed_data_fields=["phone"], ) if succeeds: @@ -844,6 +865,7 @@ def test_can_not_update_phone_of_another_profile(user_gql_client): ] } }, + allowed_data_fields=["phone"], ) assert executed["data"]["updateMyProfile"] is None @@ -876,6 +898,7 @@ def test_remove_phone(global_id_type, global_id_id, succeeds, user_gql_client): "removePhones": [to_global_id(type=global_id_type, id=global_id_id)] } }, + allowed_data_fields=["phone"], ) if succeeds: @@ -911,7 +934,9 @@ def create_profile(self, user): def execute_query(self, user_gql_client, profile_input): return user_gql_client.execute( - PHONES_MUTATION, variables={"profileInput": profile_input} + PHONES_MUTATION, + variables={"profileInput": profile_input}, + allowed_data_fields=["phone"], ) @@ -961,6 +986,7 @@ def test_add_address(user_gql_client, address_data): ] } }, + allowed_data_fields=["address"], ) address = profile.addresses.get() @@ -1027,6 +1053,7 @@ def test_update_address( ] } }, + allowed_data_fields=["address"], ) if succeeds: @@ -1108,6 +1135,7 @@ def test_remove_address(global_id_type, global_id_id, succeeds, user_gql_client) "removeAddresses": [to_global_id(type=global_id_type, id=global_id_id)] } }, + allowed_data_fields=["address"], ) if succeeds: @@ -1239,7 +1267,9 @@ def test_change_primary_contact_details( primary="true", ) executed = user_gql_client.execute( - mutation, execution_context_class=execution_context_class + mutation, + execution_context_class=execution_context_class, + allowed_data_fields=["email", "phone", "address"], ) assert "errors" not in executed assert dict(executed["data"]) == expected_data @@ -1285,5 +1315,48 @@ def test_update_sensitive_data(user_gql_client): } } } - executed = user_gql_client.execute(query) + executed = user_gql_client.execute( + query, allowed_data_fields=["name", "personalidentitycode"] + ) + assert dict(executed["data"]) == expected_data + + +def test_update_profile_does_not_reveal_fields_not_allowed(user_gql_client): + profile = ProfileWithPrimaryEmailFactory(user=user_gql_client.user) + SensitiveDataFactory(profile=profile, ssn="010199-1234") + + t = Template( + """ + mutation { + updateMyProfile( + input: { + profile: { + nickname: "${nickname}" + } + } + ) { + profile { + nickname + sensitivedata { + ssn + } + } + } + } + """ + ) + + data = {"nickname": "Larry"} + + query = t.substitute(**data) + + expected_data = { + "updateMyProfile": { + "profile": { + "nickname": data["nickname"], + "sensitivedata": None, + } + } + } + executed = user_gql_client.execute(query, allowed_data_fields=["name"]) assert dict(executed["data"]) == expected_data diff --git a/profiles/tests/test_gql_update_profile_mutation.py b/profiles/tests/test_gql_update_profile_mutation.py index c32854a9..811c901d 100644 --- a/profiles/tests/test_gql_update_profile_mutation.py +++ b/profiles/tests/test_gql_update_profile_mutation.py @@ -152,7 +152,17 @@ def test_staff_user_can_update_a_profile( } } } - executed = user_gql_client.execute(query, service=service) + executed = user_gql_client.execute( + query, + service=service, + allowed_data_fields=[ + "name", + "email", + "phone", + "personalidentitycode", + "address", + ], + ) if with_serviceconnection: assert executed["data"] == expected_data @@ -207,6 +217,7 @@ def test_can_not_change_primary_email_to_non_primary(user_gql_client, service): "updateEmails": email_updates, } }, + allowed_data_fields=["email"], ) assert_match_error_code(executed, "PROFILE_MUST_HAVE_PRIMARY_EMAIL") @@ -227,6 +238,7 @@ def test_can_not_delete_primary_email(user_gql_client, service): "removeEmails": email_deletes, } }, + allowed_data_fields=["email"], ) assert_match_error_code(executed, "PROFILE_MUST_HAVE_PRIMARY_EMAIL") @@ -267,7 +279,10 @@ def test_changing_an_email_address_marks_it_unverified(user_gql_client, service) } executed = user_gql_client.execute( - EMAILS_MUTATION, service=service, variables=variables + EMAILS_MUTATION, + service=service, + variables=variables, + allowed_data_fields=["email"], ) assert executed["data"] == expected_data @@ -311,7 +326,10 @@ def test_when_keycloak_returns_conflict_on_update_changes_are_reverted( } } executed = user_gql_client.execute( - EMAILS_MUTATION, service=service, variables=variables + EMAILS_MUTATION, + service=service, + variables=variables, + allowed_data_fields=["email"], ) assert_match_error_code(executed, "DATA_CONFLICT_ERROR") @@ -333,7 +351,10 @@ def execute_query(self, user_gql_client, profile_input): variables = {"profileInput": profile_input} return user_gql_client.execute( - EMAILS_MUTATION, service=service, variables=variables + EMAILS_MUTATION, + service=service, + variables=variables, + allowed_data_fields=["email"], ) diff --git a/profiles/tests/test_profiles_graphql_authentication.py b/profiles/tests/test_profiles_graphql_authentication.py index dbe073a1..a24a7b0b 100644 --- a/profiles/tests/test_profiles_graphql_authentication.py +++ b/profiles/tests/test_profiles_graphql_authentication.py @@ -6,6 +6,7 @@ do_graphql_call, do_graphql_call_as_user, ) +from services.tests.factories import AllowedDataFieldFactory def test_presenting_a_valid_access_token_grants_access(profile, live_server): @@ -28,6 +29,7 @@ def test_jwt_claims_are_usable_in_field_resolvers( service_connection_factory( profile=profile_with_verified_personal_information, service=service ) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) user_uuid = profile_with_verified_personal_information.user.uuid claims = {"sub": str(user_uuid), "loa": loa, "azp": service_client_id.client_id} @@ -55,6 +57,7 @@ def test_determine_service_from_the_azp_claim( user = profile.user user.groups.add(group) assign_perm("can_view_profiles", group, service) + service.allowed_data_fields.add(AllowedDataFieldFactory(field_name="name")) claims = {"sub": str(user.uuid), "azp": service_client_id.client_id} query = """