diff --git a/core/bundles/models.py b/core/bundles/models.py index fc8c90a61..46f5f51d9 100644 --- a/core/bundles/models.py +++ b/core/bundles/models.py @@ -11,8 +11,10 @@ class Bundle: - def __init__(self, root, repo_version, params=None, verbose=False, requested_url=None): # pylint: disable=too-many-arguments + def __init__( # pylint: disable=too-many-arguments + self, root, repo_version, params=None, verbose=False, requested_url=None, user=None): params = params or {} + self.user = user self.repo_version = repo_version self.params = params self.verbose = verbose @@ -96,7 +98,14 @@ def set_cascade_direction(self): self.reverse = self.params[CASCADE_DIRECTION_PARAM] in TRUTHY def set_omit_if_exists_in(self): - self.omit_if_exists_in = self.params.get(OMIT_IF_EXISTS_IN, None) or None + """A repo the user can't view is ignored, as if the expression didn't resolve.""" + omit_if_exists_in = self.params.get(OMIT_IF_EXISTS_IN, None) or None + if omit_if_exists_in: + from core.common.models import ConceptContainerModel + repo, _ = ConceptContainerModel.resolve_expression_to_version(omit_if_exists_in) + if get(repo, 'id') and not repo.has_view_access(self.user): + omit_if_exists_in = None + self.omit_if_exists_in = omit_if_exists_in def set_include_self(self): if INCLUDE_SELF in self.params: @@ -183,8 +192,9 @@ def cascade_as_hierarchy(self): self.root, context={'request': {'query_params': self.params}}).data def set_entries(self): - self.entries += self.get_concept_serializer()(self.concepts, many=True).data - self.entries += self.get_mapping_serializer()(self.mappings, many=True).data + context = {'user': self.user} + self.entries += self.get_concept_serializer()(self.concepts, many=True, context=context).data + self.entries += self.get_mapping_serializer()(self.mappings, many=True, context=context).data def get_mapping_serializer(self): from core.mappings.models import Mapping @@ -210,7 +220,8 @@ def clone( # pylint: disable=too-many-arguments params=parameters, verbose=is_verbose, repo_version=clone_from_source, - requested_url=requested_url + requested_url=requested_url, + user=user ) bundle.set_cascade_parameters() _parameters = {} diff --git a/core/code_systems/tests.py b/core/code_systems/tests.py index c1bf49b1b..ff5384f11 100644 --- a/core/code_systems/tests.py +++ b/core/code_systems/tests.py @@ -7,6 +7,7 @@ from core.code_systems.converter import CodeSystemConverter from core.code_systems.serializers import CodeSystemDetailSerializer from core.code_systems.views import CodeSystemLookupNotFoundError +from core.common.constants import ACCESS_TYPE_NONE from core.common.tests import OCLTestCase from core.concepts.models import Concept from core.concepts.tests.factories import ConceptFactory @@ -259,6 +260,67 @@ def test_lookup_for_code_system_with_new_canonical(self): {'name': 'version', 'valueString': self.org_source_v2.version}, {'name': 'display', 'valueString': self.concept_1.display_name}]})) + @patch('core.sources.models.index_source_concepts', Mock(__name__='index_source_concepts')) + @patch('core.sources.models.index_source_mappings', Mock(__name__='index_source_mappings')) + def test_private_code_system_answers_as_missing_for_non_viewers(self): # pylint: disable=too-many-locals + private_org = OrganizationFactory() + private_source = OrganizationSourceFactory( + organization=private_org, canonical_url='/private/url', public_access=ACCESS_TYPE_NONE) + concept = ConceptFactory(parent=private_source, names=1, names__name='private_concept_name') + Source.persist_new_version(OrganizationSourceFactory.build( + version='v1', mnemonic=private_source.mnemonic, organization=private_org, + public_access=ACCESS_TYPE_NONE), private_source.created_by) + member = UserProfileFactory() + private_org.members.add(member) + outsider = {'HTTP_AUTHORIZATION': 'Token ' + self.user_token} + validate_url = f'/fhir/CodeSystem/$validate-code/?url=/private/url&code={concept.mnemonic}' + lookup_url = '/fhir/CodeSystem/$lookup/?system={}&code={}' + not_found = self.client.get(lookup_url.format('/unknown/url', concept.mnemonic)) + self.assertEqual(not_found.status_code, 400) + + public_concept = ConceptFactory() + selector = (f'&org={public_concept.parent.organization.mnemonic}&source={public_concept.parent.mnemonic}' + f'&concept={public_concept.mnemonic}') + for headers in [{}, outsider]: + # without a code, a code system the requester can't view still answers as one that doesn't exist + for system_url in ['/private/url', '/unknown/url']: + response = self.client.get(f'/fhir/CodeSystem/$validate-code/?url={system_url}{selector}', **headers) + self.assertFalse(response.data['parameter'][0]['valueBoolean'], (system_url, headers)) + for url in [validate_url, f'{validate_url}&display={concept.display_name}']: + response = self.client.get(url, **headers) + self.assertEqual(response.status_code, 200, (url, headers)) + self.assertFalse(response.data['parameter'][0]['valueBoolean'], (url, headers)) + for code in [concept.mnemonic, 'missing']: + response = self.client.get(lookup_url.format('/private/url', code), **headers) + self.assertEqual(response.status_code, 400, (code, headers)) + self.assertEqual(response.content, not_found.content, (code, headers)) + + member_headers = {'HTTP_AUTHORIZATION': 'Token ' + member.get_token()} + response = self.client.get(validate_url, **member_headers) + self.assertTrue(response.data['parameter'][0]['valueBoolean']) + response = self.client.get(lookup_url.format('/private/url', concept.mnemonic), **member_headers) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['parameter'][2]['valueString'], concept.display_name) + + @patch('core.sources.models.index_source_concepts', Mock(__name__='index_source_concepts')) + @patch('core.sources.models.index_source_mappings', Mock(__name__='index_source_mappings')) + def test_private_code_system_does_not_hide_public_one_with_same_url(self): + private_source = UserSourceFactory(user=self.user, public_access=ACCESS_TYPE_NONE, canonical_url='/shared/url') + Source.persist_new_version(UserSourceFactory.build( + version='v1', mnemonic=private_source.mnemonic, user=self.user), private_source.created_by) + public_source = OrganizationSourceFactory(canonical_url='/shared/url') + concept = ConceptFactory(parent=public_source) + Source.persist_new_version(OrganizationSourceFactory.build( + version='v1', mnemonic=public_source.mnemonic, organization=public_source.organization), + public_source.created_by) + + response = self.client.get(f'/fhir/CodeSystem/$lookup/?system=/shared/url&code={concept.mnemonic}') + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['parameter'][0]['valueString'], public_source.mnemonic) + + response = self.client.get(f'/fhir/CodeSystem/$validate-code/?url=/shared/url&code={concept.mnemonic}') + self.assertTrue(response.data['parameter'][0]['valueBoolean']) + @patch('core.sources.models.index_source_concepts', Mock(__name__='index_source_concepts')) @patch('core.sources.models.index_source_mappings', Mock(__name__='index_source_mappings')) def test_post_code_system_without_concepts(self): diff --git a/core/code_systems/views.py b/core/code_systems/views.py index a03bd3ab7..52b8672ac 100644 --- a/core/code_systems/views.py +++ b/core/code_systems/views.py @@ -1,7 +1,7 @@ import logging from django.db.models import F -from rest_framework.exceptions import ValidationError, NotAuthenticated +from rest_framework.exceptions import ValidationError, NotAuthenticated, PermissionDenied from core.bundles.serializers import FHIRBundleSerializer from core.code_systems.serializers import CodeSystemDetailSerializer, \ @@ -18,6 +18,16 @@ logger = logging.getLogger('oclapi') +def get_viewable_repo(repos, user): + """The first of these repo versions the user can view, skipping the rest as if they didn't exist. Its HEAD, + which governs access to its concepts, must be viewable too.""" + for repo in repos.order_by('id'): + head = repo.head + if head and repo.has_view_access(user) and head.has_view_access(user): + return repo + return None + + class CodeSystemListView(SourceListView): serializer_class = CodeSystemDetailSerializer @@ -96,14 +106,22 @@ def get_queryset(self): else: source = source.filter(is_latest_version=True).exclude(version=HEAD) + source = get_viewable_repo(source, self.request.user) if source: - queryset = queryset.filter(sources=source.first(), mnemonic=code) + queryset = queryset.filter(sources=source, mnemonic=code) if not queryset.exists(): raise CodeSystemLookupNotFoundError(code) return queryset raise CodeSystemLookupNotFoundError() + def get_object(self, queryset=None): + """A code the requester can't view gives the same answer as a code system that doesn't exist.""" + try: + return super().get_object(queryset) + except (NotAuthenticated, PermissionDenied) as ex: + raise CodeSystemLookupNotFoundError() from ex + def get_serializer(self, instance=None): # pylint: disable=arguments-differ if instance: return ParametersSerializer.from_concept(instance) @@ -147,19 +165,19 @@ def get_queryset(self): url = IdentifierSerializer.convert_fhir_url_to_ocl_uri(url, 'sources') source = Source.objects.filter(uri=url) - if not source: - return queryset.none() - if version: source = source.filter(version=version) else: source = source.filter(is_latest_version=True).exclude(version=HEAD) + source = get_viewable_repo(source, self.request.user) + if not source: + return queryset.none() if code: if not source: return queryset.none() - queryset = queryset.filter(sources=source.first(), mnemonic=code) + queryset = queryset.filter(sources=source, mnemonic=code) if display and queryset: instance = queryset.first() @@ -187,7 +205,7 @@ def get_object(self, queryset=None): if instance: try: self.check_object_permissions(self.request, instance) - except NotAuthenticated: + except (NotAuthenticated, PermissionDenied): return { 'parameter': [ {'name': 'result', 'valueBoolean': False} diff --git a/core/collections/models.py b/core/collections/models.py index f403d2b8e..ffb013656 100644 --- a/core/collections/models.py +++ b/core/collections/models.py @@ -877,6 +877,12 @@ def get_concept_cascade_params(self, system_version=None): 'reverse', 'omit_if_exists_in']: if attr in self.cascade: cascade_params[attr] = get(self.cascade, attr) + omit_if_exists_in = cascade_params.get('omit_if_exists_in') + if omit_if_exists_in: + # a repo the reference's creator can't view is ignored, as if the expression didn't resolve + repo, _ = ConceptContainerModel.resolve_expression_to_version(omit_if_exists_in) + if get(repo, 'id') and not repo.has_view_access(self.created_by): + cascade_params.pop('omit_if_exists_in') map_types = get(self.cascade, 'map_types', None) exclude_map_types = get(self.cascade, 'exclude_map_types', None) return_map_types = get(self.cascade, 'return_map_types', None) diff --git a/core/common/mixins.py b/core/common/mixins.py index 04760a764..f085aa6a2 100644 --- a/core/common/mixins.py +++ b/core/common/mixins.py @@ -23,13 +23,14 @@ from core.common.constants import HEAD, ACCESS_TYPE_NONE, INCLUDE_FACETS, \ LIST_DEFAULT_LIMIT, HTTP_COMPRESS_HEADER, CSV_DEFAULT_LIMIT, FACETS_ONLY, INCLUDE_RETIRED_PARAM, \ SEARCH_STATS_ONLY, INCLUDE_SEARCH_STATS, UPDATED_BY_USERNAME_PARAM, CHECKSUM_STANDARD_HEADER, \ - CHECKSUM_SMART_HEADER, SEARCH_LATEST_REPO_VERSION, SAME_STANDARD_CHECKSUM_ERROR, ACCESS_TYPE_VIEW + CHECKSUM_SMART_HEADER, SEARCH_LATEST_REPO_VERSION, SAME_STANDARD_CHECKSUM_ERROR, ACCESS_TYPE_VIEW, \ + INCLUDE_PARENT_CONCEPTS, INCLUDE_CHILD_CONCEPTS from core.common.permissions import HasPrivateAccess, HasOwnership, CanViewConceptDictionary, \ CanViewConceptDictionaryVersion, CanEditConceptDictionary from core.users.constants import LIST_UNPAGINATED_PERMISSION from .checksums import ChecksumModel from .exceptions import Http403, UnpaginatedListLimitReached -from .utils import write_csv_to_s3, get_csv_from_s3, get_query_params_from_url_string, compact_dict_by_values, \ +from .utils import write_csv_to_s3, get_query_params_from_url_string, compact_dict_by_values, \ to_owner_uri, parse_updated_since_param, get_export_service, to_int, get_truthy_values, generate_temp_version, \ canonical_url_to_url_and_version, decode_string, to_parent_kwargs_from_uri from ..concepts.constants import PERSIST_CLONE_ERROR @@ -181,9 +182,13 @@ def __get_cached_data_if_any(self, request): return key_body, cache.get(key_body) or None, key_headers, cache.get(key_headers) or None def __can_cache(self): - # facets/search-stats are header driven and change the response shape, but are not part of the cache key + # facets/search-stats are header driven and change the response shape, but are not part of the cache key; + # included parent/child concepts are filtered per requester, so they can't be shared + params = self.request.query_params return (self.is_repo_version_children_request_without_any_search() and not self.should_include_facets() and not self.should_include_search_stats() and + params.get(INCLUDE_PARENT_CONCEPTS) not in TRUTHY and + params.get(INCLUDE_CHILD_CONCEPTS) not in TRUTHY and get(self, 'parent_resource.is_latest_version', False)) def list(self, request, *args, **kwargs): # pylint:disable=too-many-locals,too-many-branches,too-many-statements @@ -328,15 +333,10 @@ def get_object_ids(self): return map(lambda o: o.id, self.object_list[0:100]) def get_csv(self, request, queryset=None): - filename, url, prepare_new_file, is_member = None, None, True, False - - parent = None # TODO: fix this for parent (owner) - - if parent: - prepare_new_file = False - user = request.query_params.get('user', None) - is_member = self._is_member(parent, user) - + """ + Builds the CSV from the view's own queryset, so the view's access checks apply, and stores it in a folder + of the requester's own: one requester's export is never served to another. + """ try: path = request.__dict__.get('_request').path filename = '_'.join(compact(path.split('/'))).replace('.', '_') @@ -346,28 +346,14 @@ def get_csv(self, request, queryset=None): except Exception: # pylint: disable=broad-except kwargs = {} - if filename and prepare_new_file: - url = get_csv_from_s3(filename, is_member) - - if not url: - queryset = queryset or self._get_query_set_from_view(is_member) - data = self.get_csv_rows(queryset) if hasattr(self, 'get_csv_rows') else queryset.values() - url = write_csv_to_s3(data, is_member, **kwargs) + if queryset is None: + queryset = self.get_queryset()[0:CSV_DEFAULT_LIMIT] + data = self.get_csv_rows(queryset) if hasattr(self, 'get_csv_rows') else queryset.values() + requester = request.user.id if request.user.is_authenticated else 'anonymous' + url = write_csv_to_s3(data, requester, **kwargs) return Response({'url': url}, status=200) - @staticmethod - def _is_member(parent, requesting_user): - if not parent or type(parent).__name__ in ['UserProfile', 'Organization']: - return False - - owner = parent.owner - return owner.members.filter(username=requesting_user).exists() if type(owner).__name__ == 'Organization' else \ - requesting_user == parent.created_by - - def _get_query_set_from_view(self, is_member): - return self.get_queryset() if is_member else self.get_queryset()[0:CSV_DEFAULT_LIMIT] - class PathWalkerMixin: """ @@ -655,7 +641,7 @@ class Meta: abstract = True @classmethod - def load_list_fields(cls, instances, fields, collection=None): + def load_list_fields(cls, instances, fields, collection=None, user=None): """Loads, for a whole page at once, what the serialized fields would otherwise query row by row.""" fields = set(fields) prefetch_related_objects(instances, *cls.LIST_PREFETCHES, *fields.intersection(cls.FIELD_PREFETCHES)) @@ -663,11 +649,12 @@ def load_list_fields(cls, instances, fields, collection=None): 'version_url': cls.load_latest_version_uris, 'previous_version_url': cls.load_previous_version_uris, 'source_versions': cls.load_source_versions, - 'collection_versions': cls.load_collection_versions, } for field, loader in loaders.items(): if field in fields: loader(instances) + if 'collection_versions' in fields: + cls.load_collection_versions(instances, user) reads_latest_source_version = fields & set(cls.LATEST_SOURCE_VERSION_FIELDS) if collection: # there latest_source_version shows the collection's resolved version instead @@ -703,13 +690,14 @@ def load_source_versions(cls, instances): instance._source_versions = uris.get(instance.id, []) # pylint: disable=protected-access @classmethod - def load_collection_versions(cls, instances): - """Sets each row's _collection_versions: the uris of the collection versions whose expansions hold it.""" + def load_collection_versions(cls, instances, user): + """Sets each row's _collection_versions: the uris of the user-viewable collection versions holding it.""" key = f'{cls._meta.model_name}_id' uris = {} - for row_id, uri in cls.expansion_set.through.objects.filter( + for row_id, uri in cls.filter_by_viewable_collection_version(cls.expansion_set.through.objects.filter( **{f'{key}__in': [instance.id for instance in instances]} - ).exclude(expansion__collection_version__version=HEAD).values_list(key, 'expansion__collection_version__uri'): + ).exclude(expansion__collection_version__version=HEAD), user, 'expansion__').values_list( + key, 'expansion__collection_version__uri'): uris.setdefault(row_id, set()).add(uri) for instance in instances: instance._collection_versions = sorted(uris.get(instance.id, ())) # pylint: disable=protected-access @@ -874,12 +862,24 @@ def source_versions(self): return self._source_versions return list(self.sources.exclude(version=HEAD).order_by('created_at').values_list('uri', flat=True)) - @property - def collection_versions(self): + def get_collection_versions(self, user): + """URIs of the released collection versions holding this resource that the user can view.""" if hasattr(self, '_collection_versions'): # loaded for a whole page by load_list_fields return self._collection_versions - return sorted(set(self.expansion_set.exclude( - collection_version__version=HEAD).values_list('collection_version__uri', flat=True))) + expansions = self.filter_by_viewable_collection_version( + self.expansion_set.exclude(collection_version__version=HEAD), user) + return sorted(set(expansions.values_list('collection_version__uri', flat=True))) + + @staticmethod + def filter_by_viewable_collection_version(expansions, user, prefix=''): + """Expansions of collection versions the user can view: public ones, their own and their orgs' (staff: all).""" + if get(user, 'is_staff'): + return expansions + criteria = ~Q(**{f'{prefix}collection_version__public_access': ACCESS_TYPE_NONE}) + if get(user, 'is_authenticated'): + criteria |= Q(**{f'{prefix}collection_version__user_id': user.id}) | Q( + **{f'{prefix}collection_version__organization__members__id': user.id}) + return expansions.filter(criteria) @property def versions(self): diff --git a/core/common/models.py b/core/common/models.py index 782cde907..bb000080c 100644 --- a/core/common/models.py +++ b/core/common/models.py @@ -1035,6 +1035,15 @@ def has_view_access(self, user): return bool(user and user.is_authenticated and self.has_parent_edit_access(user)) + def get_view_access_criteria(self, user_field): + """has_view_access as a query on a UserProfile foreign key, e.g. the user who saved a row.""" + if self.public_can_view: + return Q() + criteria = Q(**{f'{user_field}__is_staff': True}) + if self.organization_id: + return criteria | Q(**{f'{user_field}__organizations__id': self.organization_id}) + return criteria | Q(**{f'{user_field}_id': self.user_id}) + @staticmethod def get_version_url_kwarg(): return 'version' diff --git a/core/common/search.py b/core/common/search.py index 65788f133..3497d889b 100644 --- a/core/common/search.py +++ b/core/common/search.py @@ -17,17 +17,23 @@ from core.common.utils import is_url_encoded_string +# Exact-case owner field: `owner` is lowercase-normalized, and owner names that differ only in case belong to +# different users or orgs. +EXACT_OWNER_FIELD = 'owner_text.keyword' + + def get_visible_repo_criteria(user): - """ES filter for concepts/mappings in repos the user can view: public ones, their own and their orgs'. + """ES filter for concepts in repos the user can view: public ones, their own and their orgs'. None for staff, who can view everything.""" if get(user, 'is_staff'): return None should = [Q('term', public_can_view=True)] if get(user, 'is_authenticated'): - should.append(Q('bool', must=[Q('term', owner_type='User'), Q('term', owner=user.username.lower())])) - org_mnemonics = [mnemonic.lower() for mnemonic in user.organizations.values_list('mnemonic', flat=True)] + should.append(Q('bool', must=[Q('term', owner_type='User'), Q('term', **{EXACT_OWNER_FIELD: user.username})])) + org_mnemonics = list(user.organizations.values_list('mnemonic', flat=True)) if org_mnemonics: - should.append(Q('bool', must=[Q('term', owner_type='Organization'), Q('terms', owner=org_mnemonics)])) + should.append(Q('bool', must=[ + Q('term', owner_type='Organization'), Q('terms', **{EXACT_OWNER_FIELD: org_mnemonics})])) return Q('bool', should=should, minimum_should_match=1) diff --git a/core/common/serializers.py b/core/common/serializers.py index a2c7bb20d..14ec15fcc 100644 --- a/core/common/serializers.py +++ b/core/common/serializers.py @@ -27,6 +27,12 @@ class TaskSerializer(Serializer): # pylint: disable=abstract-method pass +class CollectionVersionsMixin: # pylint: disable=too-few-public-methods + def get_collection_versions(self, obj): + # a $cascade bundle passes its requester as `user` + return obj.get_collection_versions(get(self, 'context.request.user') or get(self, 'context.user')) + + class ReadSerializerMixin: """ Mixin for serializer which does not update or create resources. """ @@ -254,7 +260,8 @@ class SourceChildPageSerializer(ListSerializer): # pylint: disable=abstract-met def to_representation(self, data): instances = list(data.all() if isinstance(data, BaseManager) else data) self.child.Meta.model.load_list_fields( - instances, self.child.fields.keys(), get_collection_in_scope(self.context)) + instances, self.child.fields.keys(), get_collection_in_scope(self.context), + get(self.context, 'request.user') or get(self.context, 'user')) return super().to_representation(instances) diff --git a/core/common/swagger_parameters.py b/core/common/swagger_parameters.py index bfbfbb28f..a46b70279 100644 --- a/core/common/swagger_parameters.py +++ b/core/common/swagger_parameters.py @@ -78,10 +78,12 @@ ) last_login_before_param = openapi.Parameter( - LAST_LOGIN_BEFORE_PARAM, openapi.IN_QUERY, description='YYYY-MM-DD (optional)', type=openapi.TYPE_STRING) + LAST_LOGIN_BEFORE_PARAM, openapi.IN_QUERY, description='YYYY-MM-DD (optional, staff only)', + type=openapi.TYPE_STRING) last_login_since_param = openapi.Parameter( - LAST_LOGIN_SINCE_PARAM, openapi.IN_QUERY, description='YYYY-MM-DD (optional)', type=openapi.TYPE_STRING) + LAST_LOGIN_SINCE_PARAM, openapi.IN_QUERY, description='YYYY-MM-DD (optional, staff only)', + type=openapi.TYPE_STRING) date_joined_before_param = openapi.Parameter( DATE_JOINED_BEFORE_PARAM, openapi.IN_QUERY, description='YYYY-MM-DD (optional)', type=openapi.TYPE_STRING) diff --git a/core/common/utils.py b/core/common/utils.py index 097b74dd3..5ebdce68e 100644 --- a/core/common/utils.py +++ b/core/common/utils.py @@ -43,7 +43,8 @@ def cd_temp(): return cwd -def write_csv_to_s3(data, is_owner, **kwargs): # pragma: no cover +def write_csv_to_s3(data, requester, **kwargs): # pragma: no cover + """Uploads `data` as a zipped CSV to the requester's own downloads folder and returns its URL.""" cwd = cd_temp() csv_file = csv_file_for(data, **kwargs) csv_file.close() @@ -51,7 +52,7 @@ def write_csv_to_s3(data, is_owner, **kwargs): # pragma: no cover with zipfile.ZipFile(zip_file_name, 'w', zipfile.ZIP_DEFLATED) as zip_file: zip_file.write(csv_file.name) - key = get_downloads_path(is_owner) + zip_file.filename + key = f'downloads/reader/{requester}/' + zip_file.filename get_export_service().upload_file( key=key, file_path=os.path.abspath(zip_file.filename), binary=True, metadata={'ContentType': 'application/zip'}, headers={'content-type': 'application/zip'} @@ -69,20 +70,6 @@ def compact_dict_by_values(_dict): return copied_dict -def get_downloads_path(is_owner): # pragma: no cover - return 'downloads/creator/' if is_owner else 'downloads/reader/' - - -def get_csv_from_s3(filename, is_owner): # pragma: no cover - filename = get_downloads_path(is_owner) + filename + '.csv.zip' - - export_service = get_export_service() - if export_service.exists(filename): - return export_service.url_for(filename) - - return None - - def reverse_resource(resource, viewname, args=None, kwargs=None, **extra): """ Generate the URL for the view specified as viewname of the object specified as resource. diff --git a/core/common/views.py b/core/common/views.py index 346ecfeef..ac5a4bed7 100644 --- a/core/common/views.py +++ b/core/common/views.py @@ -29,7 +29,7 @@ CANONICAL_URL_REQUEST_PARAM, CHECKSUMS_PARAM, ACCESS_TYPE_NONE from core.common.exceptions import Http400 from core.common.mixins import PathWalkerMixin -from core.common.search import CustomESSearch +from core.common.search import CustomESSearch, EXACT_OWNER_FIELD from core.common.serializers import RootSerializer from core.common.swagger_parameters import all_resource_query_param from core.common.throttling import ThrottleUtil @@ -630,7 +630,8 @@ def get_extras_searchable_fields_from_query_params(self): for key, value in query_params.items(): if key.startswith('extras.') and not key.startswith('extras.exists') and not key.startswith('extras.exact'): parts = key.split('extras.') - value = value.replace('/', '\\/').replace('-', '_') + # escape `:` so the query_string value can only match this extras field + value = value.replace('\\', '\\\\').replace('/', '\\/').replace(':', '\\:').replace('-', '_') result['extras.' + parts[1].replace('.', '__')] = f"*{value}*" return result @@ -732,6 +733,38 @@ def __should_query_latest_version(self): return (not collection or collection.startswith('!')) and (not version or version.startswith('!')) + def is_members_view(self): + """/user/orgs//: lists across all the requester's orgs.""" + return self.get_view_name() in ['Organization Collection List', 'Organization Source List', + 'Organization Repo List', 'Organization Url Registry List'] + + def get_scope_owner_criteria(self): + """Private content in the requester's own scope (their user, or an org of theirs) is shown, but the owner and + source filters match names in any case, and owner names that differ only in case are other owners: their + private content is excluded (concepts by exact owner name; for other documents, such as mappings and repos, + all private content is excluded when such an owner exists). None when there's nothing to exclude.""" + if (not (self.is_source_child_document_model() or self.is_concept_container_document_model() or + self.is_repo_document_model()) or + self.request.user.is_staff or 'collection' in self.kwargs or not self.is_user_scope()): + return None + org = self.kwargs.get('org') + owner = org or self.kwargs.get('user') or self.request.user.username + public = Q('term', public_can_view=True) + orgs = list(self.request.user.organizations.values_list('mnemonic', flat=True)) if ( + self.is_members_view()) else ([org] if org else None) + if self.is_concept_document(): + return public | Q('term', **{EXACT_OWNER_FIELD: owner}) + if orgs is not None: + from core.orgs.models import Organization + twin_criteria = models.Q() + for mnemonic in orgs: + twin_criteria |= models.Q(mnemonic__iexact=mnemonic) + has_twin = bool(orgs) and Organization.objects.filter(twin_criteria).exclude(mnemonic__in=orgs).exists() + else: + from core.users.models import UserProfile + has_twin = UserProfile.objects.filter(username__iexact=owner).exclude(username=owner).exists() + return public if has_twin else None + def __apply_common_search_filters(self, ignore_retired_filter=False, force=False): results = None if not force and not self.should_perform_es_search(): @@ -769,9 +802,10 @@ def __apply_common_search_filters(self, ignore_retired_filter=False, force=False if not ignore_retired_filter and self._should_exclude_retired_from_search_results(): results = results.filter('term', retired=False) - include_private = self._should_include_private() - if not include_private: - results = results.filter(self.get_public_criteria()) + visibility_criteria = self.get_scope_owner_criteria() if self._should_include_private() else ( + self.get_public_criteria()) + if visibility_criteria is not None: + results = results.filter(visibility_criteria) faceted_criterion = self.get_faceted_criterion() if faceted_criterion: results = results.filter(faceted_criterion) @@ -889,8 +923,7 @@ def __get_search_results(self, ignore_retired_filter=False, sort=True, highlight is_authenticated = user.is_authenticated username = user.username - is_members_view = self.get_view_name() in ['Organization Collection List', 'Organization Source List', - 'Organization Repo List', 'Organization Url Registry List'] + is_members_view = self.is_members_view() if self.is_owner_document_model(): kwargs_filters = self.kwargs.copy() if self.user_is_self and is_authenticated: diff --git a/core/concepts/models.py b/core/concepts/models.py index 6cd515472..b29d11e85 100644 --- a/core/concepts/models.py +++ b/core/concepts/models.py @@ -655,7 +655,7 @@ def create_new_version_for( if has_parent_concept_uris_attr: parent_concept_uris = cls.get_viewable_parent_uris( - parent_concept_uris or [], user, prev_latest.parent_concept_urls if prev_latest else []) + parent_concept_uris or [], user, lambda: prev_latest.parent_concept_urls if prev_latest else []) else: parent_concept_uris = list(prev_latest.parent_concept_urls) if prev_latest else [] @@ -682,22 +682,21 @@ def create_new_version_for( @staticmethod def get_viewable_parent_uris(uris, user, linked_uris=None): """Only parents in repos the user can view can be linked. Parents already linked are kept, - and so are not-yet-existing ones pointing into a repo the user can view (linked later by the hierarchy task).""" + and so are not-yet-existing ones pointing into a repo the user can view (linked later by the hierarchy task). + `linked_uris` is a callable returning those, called only when some parent isn't viewable.""" from core.common.utils import to_parent_uri from core.sources.models import Source - linked_uris = set(linked_uris or []) - new_uris = [uri for uri in uris if uri not in linked_uris] viewable_uris = set() can_view_source = {} existing_uris = set() - for concept in Concept.objects.filter(uri__in=new_uris).select_related('parent') if new_uris else []: + for concept in Concept.objects.filter(uri__in=uris).select_related('parent'): existing_uris.add(concept.uri) if concept.parent_id not in can_view_source: can_view_source[concept.parent_id] = concept.parent.has_view_access(user) if can_view_source[concept.parent_id]: viewable_uris.add(concept.uri) repo_view_by_uri = {} - for uri in new_uris: + for uri in uris: if uri in existing_uris: continue repo_uri = to_parent_uri(uri) @@ -706,7 +705,9 @@ def get_viewable_parent_uris(uris, user, linked_uris=None): repo_view_by_uri[repo_uri] = bool(repo and repo.has_view_access(user)) if repo_view_by_uri[repo_uri]: viewable_uris.add(uri) - return [uri for uri in uris if uri in linked_uris or uri in viewable_uris] + if linked_uris and any(uri not in viewable_uris for uri in uris): + viewable_uris |= set(linked_uris()) + return [uri for uri in uris if uri in viewable_uris] def set_parent_concepts_from_uris(self, create_parent_version=True): parent_concepts = get(self, '_parent_concepts', []) @@ -1284,16 +1285,16 @@ def validate_locales_limit(names, descriptions): def get_unidirectional_mappings_for_collection(self, collection_url, collection_version=HEAD): from core.mappings.models import Mapping return Mapping.objects.filter( - from_concept__uri__icontains=drop_version(self.uri), - expansion_set__collection_version__uri__icontains=collection_url, + from_concept__uri__startswith=drop_version(self.uri), + expansion_set__collection_version__uri__startswith=collection_url, expansion_set__collection_version__version=collection_version ) def get_indirect_mappings_for_collection(self, collection_url, collection_version=HEAD): from core.mappings.models import Mapping return Mapping.objects.filter( - to_concept__uri__icontains=drop_version(self.uri), - expansion_set__collection_version__uri__icontains=collection_url, + to_concept__uri__startswith=drop_version(self.uri), + expansion_set__collection_version__uri__startswith=collection_url, expansion_set__collection_version__version=collection_version ) @@ -1343,15 +1344,17 @@ def get_latest_versions_for_queryset(concepts_qs): def update_mappings(self): # Updates mappings where mapping.to_concept or mapping.from_concepts matches concept's mnemonic and parent + # only mappings whose last saver can view this concept's repo; others keep the external code, as on save from core.mappings.models import Mapping parent_uris = self.parent.identity_uris + can_view = self.parent.get_view_access_criteria('updated_by') Mapping.objects.filter( - to_concept_code=self.mnemonic, to_source_url__in=parent_uris, to_concept__isnull=True + can_view, to_concept_code=self.mnemonic, to_source_url__in=parent_uris, to_concept__isnull=True ).update(to_concept=self) Mapping.objects.filter( - from_concept_code=self.mnemonic, from_source_url__in=parent_uris, from_concept__isnull=True + can_view, from_concept_code=self.mnemonic, from_source_url__in=parent_uris, from_concept__isnull=True ).update(from_concept=self) @classmethod diff --git a/core/concepts/search.py b/core/concepts/search.py index fc7bb2bdb..8c7c03b15 100644 --- a/core/concepts/search.py +++ b/core/concepts/search.py @@ -3,7 +3,7 @@ from core.common.constants import FACET_SIZE, HEAD from core.common.lexical_variants import LexicalVariantDictionary -from core.common.search import CustomESFacetedSearch, CustomESSearch +from core.common.search import CustomESFacetedSearch, CustomESSearch, EXACT_OWNER_FIELD from core.common.utils import get_embeddings, is_canonical_uri from core.concepts.models import Concept @@ -124,6 +124,8 @@ class ConceptFuzzySearch: # pragma: no cover ['concept_class', 'datatype', 0], ] fuzzy_fields = ['name', 'synonyms'] + # `owner` and `source` are lowercase-normalized; names that differ only in case are different repos + exact_repo_fields = {'owner': EXACT_OWNER_FIELD, 'source': 'source_text.keyword'} @staticmethod def get_target_repo(repo_url): @@ -436,7 +438,10 @@ def get_filter_criteria(cls, data, include_retired, repo_params, repo_url, addit must_clauses = [] repo_params = repo_params or cls.get_target_repo_params(repo_url) for field, value in repo_params.items(): - must_clauses.append(Q('match', **{field: value})) + if field in cls.exact_repo_fields: + must_clauses.append(Q('term', **{cls.exact_repo_fields[field]: value})) + else: + must_clauses.append(Q('match', **{field: value})) if not include_retired: must_clauses.append(Q('match', retired=False)) for field in cls.filter_fields: diff --git a/core/concepts/serializers.py b/core/concepts/serializers.py index 6c77557e3..9a9fc8493 100644 --- a/core/concepts/serializers.py +++ b/core/concepts/serializers.py @@ -10,7 +10,7 @@ INCLUDE_RETIRED_PARAM, ACCESS_TYPE_NONE from core.common.fields import EncodedDecodedCharField from core.common.serializers import AbstractResourceSerializer, SourceChildPageSerializer, \ - SourceVersionFieldMixin, get_collection_in_scope + SourceVersionFieldMixin, CollectionVersionsMixin, get_collection_in_scope from core.common.utils import to_parent_uri_from_kwargs, get_truthy_values from core.concepts.models import Concept, ConceptName @@ -18,6 +18,14 @@ TRUTHY = get_truthy_values() +def get_collection_uri_from_view(view, request): + """The URI of the collection in the view's URL, with /user/ resolved to the requester.""" + kwargs = get(view, 'kwargs') or {} + if get(view, 'user_is_self'): + kwargs = {**kwargs, 'user': get(request, 'user.username')} + return to_parent_uri_from_kwargs(kwargs) + + def get_viewable_concepts(queryset, request): """Concepts from repos the requester can view.""" user = get(request, 'user') @@ -237,7 +245,7 @@ def get_mappings(self, obj): serializer_class = MappingMinimalSerializer if is_mapping_brief else MappingDetailSerializer is_collection = 'collection' in self.view_kwargs collection_version = self.view_kwargs.get('version', HEAD) if is_collection else None - parent_uri = to_parent_uri_from_kwargs(self.view_kwargs) if is_collection else None + parent_uri = get_collection_uri_from_view(get(context, 'view'), request) if is_collection else None if self.include_indirect_mappings: mappings = obj.get_bidirectional_mappings_for_collection( parent_uri, collection_version @@ -333,10 +341,10 @@ class Meta: ) -class ConceptVersionListSerializer(ConceptListSerializer): +class ConceptVersionListSerializer(CollectionVersionsMixin, ConceptListSerializer): previous_version_url = CharField(read_only=True, source='prev_version_uri') source_versions = ListField(read_only=True) - collection_versions = ListField(read_only=True) + collection_versions = SerializerMethodField() class Meta: model = Concept @@ -575,7 +583,7 @@ def get_child_concept_urls(obj): return list(uris) -class ConceptVersionDetailSerializer(SourceVersionFieldMixin, ModelSerializer): +class ConceptVersionDetailSerializer(CollectionVersionsMixin, SourceVersionFieldMixin, ModelSerializer): type = CharField(source='resource_type') uuid = CharField(source='id') id = EncodedDecodedCharField(source='mnemonic') @@ -600,7 +608,7 @@ class ConceptVersionDetailSerializer(SourceVersionFieldMixin, ModelSerializer): parent_concept_urls = ListField(read_only=True) child_concept_urls = ListField(read_only=True) source_versions = ListField(read_only=True) - collection_versions = ListField(read_only=True) + collection_versions = SerializerMethodField() references = SerializerMethodField() latest_source_version = SerializerMethodField() @@ -666,7 +674,8 @@ def get_mappings(self, obj): context = get(self, 'context') is_collection = 'collection' in self.view_kwargs collection_version = self.view_kwargs.get('version', HEAD) if is_collection else None - parent_uri = to_parent_uri_from_kwargs(self.view_kwargs) if is_collection else None + parent_uri = get_collection_uri_from_view( + get(context, 'view'), get(context, 'request')) if is_collection else None if self.include_indirect_mappings: mappings = obj.get_bidirectional_mappings_for_collection( parent_uri, collection_version diff --git a/core/concepts/tests/tests.py b/core/concepts/tests/tests.py index 513191090..aea0c8a3b 100644 --- a/core/concepts/tests/tests.py +++ b/core/concepts/tests/tests.py @@ -32,6 +32,7 @@ from core.concepts.validators import ValidatorSpecifier from core.mappings.models import Mapping from core.mappings.tests.factories import MappingFactory +from core.orgs.tests.factories import OrganizationFactory from core.sources.tests.factories import OrganizationSourceFactory from core.users.models import UserProfile from core.users.tests.factories import UserProfileFactory @@ -397,7 +398,6 @@ def test_match_concepts_access_denied_403(self): def test_match_concepts_consumption_attributed_from_event_metadata(self, _filter_queryset_mock): from core.capabilities.models import UsageEvent from core.map_projects.tests.factories import MapProjectFactory - from core.orgs.tests.factories import OrganizationFactory user = UserProfileFactory() user.groups.add(Group.objects.get(name=PREVIEW_GROUP_NAME)) org = OrganizationFactory() @@ -422,7 +422,6 @@ def test_match_concepts_ignores_map_project_id_the_user_does_not_own(self, _filt # usage still charges the right user's quota, it just stays unattributed. from core.capabilities.models import UsageEvent from core.map_projects.tests.factories import MapProjectFactory - from core.orgs.tests.factories import OrganizationFactory user = UserProfileFactory() user.groups.add(Group.objects.get(name=PREVIEW_GROUP_NAME)) other_user = UserProfileFactory() @@ -2760,9 +2759,17 @@ def test_get_unidirectional_mappings_for_collection(self): expansion = ExpansionFactory(collection_version=collection) expansion.mappings.add(mapping) + twin_mapping = MappingFactory(from_concept=concept1, to_concept=concept2, parent=source) + twin = OrganizationCollectionFactory( + organization=OrganizationFactory(mnemonic=collection.organization.mnemonic.upper()), + mnemonic=collection.mnemonic.upper()) + ExpansionFactory(collection_version=twin).mappings.add(twin_mapping) + results = concept1.get_unidirectional_mappings_for_collection(collection.uri) self.assertIn(mapping.id, results.values_list('id', flat=True)) + # a collection whose URL differs only in case is another collection + self.assertNotIn(twin_mapping.id, results.values_list('id', flat=True)) def test_get_indirect_mappings_for_collection(self): source = OrganizationSourceFactory() @@ -2773,9 +2780,17 @@ def test_get_indirect_mappings_for_collection(self): expansion = ExpansionFactory(collection_version=collection) expansion.mappings.add(mapping) + twin_mapping = MappingFactory(from_concept=concept1, to_concept=concept2, parent=source) + twin = OrganizationCollectionFactory( + organization=OrganizationFactory(mnemonic=collection.organization.mnemonic.upper()), + mnemonic=collection.mnemonic.upper()) + ExpansionFactory(collection_version=twin).mappings.add(twin_mapping) + results = concept2.get_indirect_mappings_for_collection(collection.uri) self.assertIn(mapping.id, results.values_list('id', flat=True)) + # a collection whose URL differs only in case is another collection + self.assertNotIn(twin_mapping.id, results.values_list('id', flat=True)) def test_get_hierarchy_concept_urls_versioned(self): source = OrganizationSourceFactory() diff --git a/core/concepts/urls.py b/core/concepts/urls.py index 1a0004a9f..ae9c6f59f 100644 --- a/core/concepts/urls.py +++ b/core/concepts/urls.py @@ -1,5 +1,4 @@ from django.urls import path -from django.views.decorators.cache import cache_page from core.concepts.feeds import ConceptFeed from . import views @@ -8,9 +7,7 @@ path('', views.ConceptListView.as_view(), name='concept-list'), path( 'lookup/', - cache_page( - timeout=60 * 60 * 24, key_prefix='cache_lookup' - )(views.ConceptLookupValuesView.as_view()), + views.cache_public_source_lookup(views.ConceptLookupValuesView.as_view()), name='concept-lookup-list' ), path( diff --git a/core/concepts/views.py b/core/concepts/views.py index 711c22ddd..41fec4cc6 100644 --- a/core/concepts/views.py +++ b/core/concepts/views.py @@ -6,6 +6,7 @@ from django.db import transaction from django.db.models import F, Q as DjangoQ from django.http import Http404 +from django.views.decorators.cache import cache_page from drf_yasg import openapi from drf_yasg.utils import swagger_auto_schema from elasticsearch_dsl import Q @@ -44,7 +45,7 @@ match_brief_param, match_encoder_model_param, match_reranker_param, match_offset_param) from core.common.tasks import delete_concept, make_hierarchy from core.common.throttling import ThrottleUtil -from core.common.utils import (to_parent_uri_from_kwargs, generate_temp_version, get_truthy_values, to_int, +from core.common.utils import (generate_temp_version, get_truthy_values, to_int, drop_version, get_falsy_values, parse_id, get_event_metadata) from core.common.views import SourceChildCommonBaseView, SourceChildExtrasView, \ SourceChildExtraRetrieveUpdateDestroyView, BaseAPIView @@ -60,7 +61,8 @@ ConceptDetailSerializer, ConceptListSerializer, ConceptDescriptionSerializer, ConceptNameSerializer, ConceptVersionDetailSerializer, ConceptVersionListSerializer, ConceptSummarySerializer, ConceptMinimalSerializer, - ConceptChildrenSerializer, ConceptParentsSerializer, ConceptLookupListSerializer, ConceptChecksumSerializer) + ConceptChildrenSerializer, ConceptParentsSerializer, ConceptLookupListSerializer, ConceptChecksumSerializer, + get_viewable_concepts, get_collection_uri_from_view) from core.mappings.serializers import MappingListSerializer from core.sources.clone_limits import clone_limit_error_detail, clone_lock, get_clone_budget from core.sources.models import CloneError, CloneLimitExceeded @@ -104,7 +106,28 @@ def set_parent_resource(self, __pop=True): self.kwargs['parent_resource'] = self.parent_resource = parent_resource -# this is a cached view (expiry 24 hours) +def cache_public_source_lookup(view_func): + """Caches lookups of public sources only: a cached response is served to anyone, + without the view's access check.""" + cached_view_func = cache_page(timeout=60 * 60 * 24, key_prefix='cache_lookup')(view_func) + + def wrapper(request, *args, **kwargs): + from core.sources.models import Source + owner_filters = None + if 'org' in kwargs: + owner_filters = {'organization__mnemonic': kwargs['org']} + elif 'user' in kwargs: + owner_filters = {'user__username': kwargs['user']} + source = Source.get_version(kwargs['source'], HEAD, owner_filters) if owner_filters and kwargs.get( + 'source') else None + if source and source.public_can_view: + return cached_view_func(request, *args, **kwargs) + return view_func(request, *args, **kwargs) + + return wrapper + + +# this is a cached view for public sources (expiry 24 hours) # used for TermBrowser forms lookup values -- map-types/locales/datatypes/etc class ConceptLookupValuesView(ListAPIView, BaseAPIView): # pragma: no cover serializer_class = ConceptLookupListSerializer @@ -125,7 +148,7 @@ def set_parent_resource(self): def get_queryset(self): self.set_parent_resource() - if self.parent_resource: + if self.parent_resource and self.parent_resource.has_view_access(self.request.user): queryset = self.parent_resource.concepts_set.filter(id=F('versioned_object_id')) if self.is_verbose(): queryset = queryset.prefetch_related('names') @@ -323,10 +346,11 @@ def get_queryset(self): instance = self.get_object() from core.collections.models import Collection - return Collection.objects.filter(id__in=instance.expansion_set.filter( + expansions = Concept.filter_by_viewable_collection_version(instance.expansion_set.filter( collection_version__organization_id=instance.parent.organization_id, collection_version__user_id=instance.parent.user_id - ).values_list('collection_version_id', flat=True)) + ), self.request.user) + return Collection.objects.filter(id__in=expansions.values_list('collection_version_id', flat=True)) def get(self, request, *args, **kwargs): return self.list(request, *args, **kwargs) @@ -518,12 +542,19 @@ def get_object(self, queryset=None): omit_if_exists_in_param, equivalency_map_types_param ] ) + def set_cascade_repo(self): + """The repo the cascade runs over, from the URL. Query params can pick the root concept from another repo, + so this one's view access is checked too.""" + self.set_parent_resource(False) + if self.parent_resource and not self.parent_resource.has_view_access(self.request.user): + raise Http404() + def get(self, request, **kwargs): # pylint: disable=unused-argument instance = self.get_object() - self.set_parent_resource(False) + self.set_cascade_repo() bundle = Bundle( root=instance, params=self.request.query_params, verbose=self.is_verbose(), - repo_version=self.parent_resource, requested_url=self.request.get_full_path() + repo_version=self.parent_resource, requested_url=self.request.get_full_path(), user=self.request.user ) bundle.cascade() return Response(BundleSerializer(bundle, context={'request': request}).data) @@ -541,7 +572,7 @@ def post(self, request, **kwargs): # pylint: disable=unused-argument } """ clone_to_source = self.get_clone_to_source() - self.set_parent_resource(False) + self.set_cascade_repo() budget = get_clone_budget(request.user) parameters = dict(request.data.get('parameters') or {}) parameters.pop('resource_budget', None) @@ -571,6 +602,7 @@ def get_clone_to_source(self): class ConceptChildrenView(ConceptBaseView, ListWithHeadersMixin): + is_searchable = False # a search would list the repo's concepts, not this concept's hierarchy serializer_class = ConceptChildrenSerializer default_qs_sort_attr = 'mnemonic' @@ -583,13 +615,14 @@ def get_queryset(self): raise Http404() self.check_object_permissions(self.request, instance) - return instance.child_concept_queryset() + return get_viewable_concepts(instance.child_concept_queryset(), self.request) def get(self, request, *args, **kwargs): return self.list(request, *args, **kwargs) class ConceptParentsView(ConceptBaseView, ListWithHeadersMixin): + is_searchable = False # a search would list the repo's concepts, not this concept's hierarchy serializer_class = ConceptParentsSerializer def get_queryset(self): @@ -601,7 +634,7 @@ def get_queryset(self): raise Http404() self.check_object_permissions(self.request, instance) - return instance.parent_concept_queryset() + return get_viewable_concepts(instance.parent_concept_queryset(), self.request) def get(self, request, *args, **kwargs): return self.list(request, *args, **kwargs) @@ -668,7 +701,7 @@ def get_queryset(self): include_indirect_mappings = self.request.query_params.get(INCLUDE_INVERSE_MAPPINGS_PARAM, 'false') in TRUTHY is_collection = 'collection' in self.kwargs collection_version = self.kwargs.get('version', HEAD) if is_collection else None - parent_uri = to_parent_uri_from_kwargs(self.kwargs) if is_collection else None + parent_uri = get_collection_uri_from_view(self, self.request) if is_collection else None if include_indirect_mappings: mappings_queryset = concept.get_bidirectional_mappings_for_collection( parent_uri, collection_version @@ -1151,11 +1184,19 @@ def get_repo_params(is_semantic, target_repo_params, target_repo_url, user=None) raise Http400(f'Unable to resolve "target_repo_url": "{target_repo_url}"') repo_params = ConceptFuzzySearch.get_repo_params(repo) elif target_repo_params: - repos = MetadataToConceptsListView.get_target_repos_from_params(target_repo_params) - if not repos.exists() or not all(target_repo.has_view_access(user) for target_repo in repos): + repos = list(MetadataToConceptsListView.get_target_repos_from_params(target_repo_params)) + if not repos or not all(target_repo.has_view_access(user) for target_repo in repos): raise Http400(f'Unable to resolve "target_repo": "{target_repo_params}"') - repo = repos.first() - repo_params = target_repo_params + repo = repos[0] + # search the repos that were checked, by their stored names: target_repo's own values may differ + # in case or key names (e.g. "version", "org"), which the search filters would read differently + repo_params = {'owner': repo.parent.mnemonic, 'source': repo.mnemonic} + owner_types = {target_repo.parent.resource_type for target_repo in repos} + if len(owner_types) == 1: + repo_params['owner_type'] = owner_types.pop() + version = get(target_repo_params, 'source_version') or get(target_repo_params, 'version') + if version: + repo_params['source_version'] = version else: raise Http400(f'Unable to resolve "target_repo_url": "{target_repo_url}"') if is_semantic and not repo.has_semantic_match_algorithm: diff --git a/core/events/migrations/0011_private_follow_events.py b/core/events/migrations/0011_private_follow_events.py new file mode 100644 index 000000000..d99952073 --- /dev/null +++ b/core/events/migrations/0011_private_follow_events.py @@ -0,0 +1,45 @@ +import re + +from django.db import migrations + +REPO_URI = re.compile(r'^/(?:orgs|users)/[^/]+/(?:sources|collections)/[^/]+/') + + +def get_private_uris(model, uris, **kwargs): + # 'Edit' is retired and reads as 'View' + return { + uri for uri, public_access in model.objects.filter(uri__in=uris, **kwargs).values_list('uri', 'public_access') + if (public_access or '').lower() not in ('view', 'edit') + } + + +# Followed/Unfollowed events of an org or repo the public can't view (or of content in such a repo) are private. +def make_follow_events_of_private_objects_private(apps, schema_editor): # pylint: disable=unused-argument + events = apps.get_model('events', 'Event').objects.filter(event_type__in=['Followed', 'Unfollowed'], public=True) + urls = set(filter(None, events.values_list('referenced_object_url', flat=True))) + repo_uris = {url: match.group(0) for url in urls if (match := REPO_URI.match(url))} + + private_uris = get_private_uris(apps.get_model('orgs', 'Organization'), urls) + for app_label, model_name in [('sources', 'Source'), ('collections', 'Collection')]: + model = apps.get_model(app_label, model_name) + # a followed repo version by its own access, anything else under a repo by its HEAD's + private_uris |= get_private_uris(model, urls) + private_uris |= get_private_uris(model, set(repo_uris.values()), version='HEAD') + + private_urls = [url for url in urls if url in private_uris or repo_uris.get(url) in private_uris] + if private_urls: + events.filter(referenced_object_url__in=private_urls).update(public=False) + + +class Migration(migrations.Migration): + + dependencies = [ + ('events', '0010_auto_20250910_0142'), + ('orgs', '0022_retire_public_edit_access'), + ('sources', '0046_retire_public_edit_access'), + ('collections', '0071_retire_public_edit_access'), + ] + + operations = [ + migrations.RunPython(make_follow_events_of_private_objects_private, migrations.RunPython.noop), + ] diff --git a/core/events/models.py b/core/events/models.py index 6074afe20..b7ad856e3 100644 --- a/core/events/models.py +++ b/core/events/models.py @@ -40,12 +40,19 @@ def get_events_for_following(cls, following_queryset, private=False, **event_kwa queryset = cls.objects.none() for following in following_queryset: events = following.following.events.filter(**event_kwargs) - # private events only for objects the follower can still view - if not private or not following.follower.can_view(following.following): + if not private or not cls.can_follower_view_private_events(following.follower, following.following): events = events.filter(public=True) queryset = queryset.union(events) return queryset + @staticmethod + def can_follower_view_private_events(follower, followed): + """Private events of an org or repo go to followers who can view it. A user's private events are about + orgs and repos the follower may not view, so only staff get them (org members get them in the orgs scope).""" + if followed.__class__.__name__ == 'UserProfile': + return follower.is_staff + return follower.can_view(followed) + @classmethod def get_user_organization_events(cls, user, private=False): org_uris = list(user.organizations.values_list('uri', flat=True)) diff --git a/core/events/serializers.py b/core/events/serializers.py index 257a05a6d..eb46111b6 100644 --- a/core/events/serializers.py +++ b/core/events/serializers.py @@ -1,3 +1,4 @@ +from pydash import get from rest_framework import serializers from core.events.models import Event @@ -8,6 +9,7 @@ class EventSerializer(serializers.ModelSerializer): actor = UserListSerializer(read_only=True) referenced_object = serializers.SerializerMethodField() object = serializers.SerializerMethodField() + description = serializers.SerializerMethodField() class Meta: model = Event @@ -16,19 +18,27 @@ class Meta: 'actor', 'public', 'url', 'created_at' ) - @staticmethod - def get_referenced_object(obj): + def can_view(self, instance): + """An event's flag is set when it's recorded; the object shown is live, so its access is checked now.""" + from core.users.models import UserProfile + return UserProfile.can_user_view(get(self, 'context.request.user'), instance) + + def get_referenced_object(self, obj): referenced_object = obj.referenced_object if referenced_object: - data = referenced_object.get_brief_serializer()(referenced_object).data + data = referenced_object.get_brief_serializer()(referenced_object).data if self.can_view( + referenced_object) else None else: data = obj._referenced_object return data - @staticmethod - def get_object(obj): + def get_description(self, obj): + referenced_object = obj.referenced_object + return obj.event_type if referenced_object and not self.can_view(referenced_object) else obj.description + + def get_object(self, obj): resource_object = obj.object data = None - if resource_object: + if resource_object and self.can_view(resource_object): data = resource_object.get_brief_serializer()(resource_object).data return data diff --git a/core/events/tests/tests.py b/core/events/tests/tests.py index f9c54153e..3b4ed7015 100644 --- a/core/events/tests/tests.py +++ b/core/events/tests/tests.py @@ -1,3 +1,6 @@ +import importlib + +from django.apps import apps from django.utils import timezone from core.common.tests import OCLTestCase @@ -109,3 +112,35 @@ def test_clean_fields(self): self.user_event.clean_fields() self.assertEqual(self.user_event.public, False) + + +class PrivateFollowEventsMigrationTest(OCLTestCase): + def test_migration_makes_follow_events_of_private_objects_private(self): # pylint: disable=too-many-locals + migration = importlib.import_module('core.events.migrations.0011_private_follow_events') + follower = UserProfileFactory() + private_org = OrganizationFactory(public_access='None') + public_org = OrganizationFactory() + private_source = OrganizationSourceFactory(organization=public_org, public_access='None') + public_source = OrganizationSourceFactory(organization=private_org) + private_concept = ConceptFactory(parent=private_source) + public_concept = ConceptFactory(parent=public_source) + private_version = OrganizationSourceFactory( + organization=public_source.organization, mnemonic=public_source.mnemonic, version='v1', + public_access='None') + private_version.refresh_from_db() + urls = [ + follower.url, private_org.url, public_org.url, private_source.url, public_source.url, + private_concept.url, public_concept.url, '/orgs/deleted-org/', private_version.uri + ] + events = { + (event_type, url): Event.objects.create( + object_url=follower.url, referenced_object_url=url, event_type=event_type, actor=follower) + for url in urls for event_type in ['Followed', 'Unfollowed', 'Created'] + } + + migration.make_follow_events_of_private_objects_private(apps, None) + + private_urls = [private_org.url, private_source.url, private_concept.url, private_version.uri] + for (event_type, url), event in events.items(): + event.refresh_from_db() + self.assertEqual(event.public, event_type == 'Created' or url not in private_urls, (event_type, url)) diff --git a/core/events/views.py b/core/events/views.py index 0ef104dda..94ae20801 100644 --- a/core/events/views.py +++ b/core/events/views.py @@ -18,6 +18,7 @@ class EventsView(BaseAPIView, ListWithHeadersMixin): is_searchable = False + allow_csv_export = False # an export would skip the serializer's live-object checks permission_classes = (AllowAny,) default_qs_sort_attr = '-created_at' serializer_class = EventSerializer diff --git a/core/graphql/queries.py b/core/graphql/queries.py index e96b6ac7e..3f1e9a0ea 100644 --- a/core/graphql/queries.py +++ b/core/graphql/queries.py @@ -14,8 +14,10 @@ from strawberry.exceptions import GraphQLError from core.common.constants import HEAD +from core.common.search import get_visible_repo_criteria from core.concepts.documents import ConceptDocument from core.concepts.models import Concept +from core.concepts.serializers import get_viewable_concepts from core.mappings.models import Mapping from core.sources.models import Source @@ -67,7 +69,8 @@ class ConceptSearchResult: ) -async def resolve_source_version(org: str, source: str, version: Optional[str]) -> Source: +async def resolve_source_version(org: str, source: str, version: Optional[str], user) -> Source: + """The org's source version; one the user cannot view is reported as not found.""" filters = {'organization__mnemonic': org} target_version = version or HEAD instance = await sync_to_async(Source.get_version)(source, target_version, filters) @@ -75,7 +78,7 @@ async def resolve_source_version(org: str, source: str, version: Optional[str]) if not instance and version is None: instance = await sync_to_async(Source.find_latest_released_version_by)({**filters, 'mnemonic': source}) - if not instance: + if not instance or not await sync_to_async(instance.has_view_access)(user): raise GraphQLError( f"Source '{source}' with version '{version or 'HEAD'}' was not found for org '{org}'." ) @@ -103,13 +106,17 @@ def build_mapping_prefetch(source_version: Source) -> Prefetch: return Prefetch('mappings_from', queryset=mapping_qs, to_attr='graphql_mappings') -def build_global_mapping_prefetch() -> Prefetch: +def build_global_mapping_prefetch(user) -> Prefetch: + """Mappings from each concept, in repositories the user can view.""" + mapping_qs = Mapping.objects.filter( + from_concept_id__isnull=False, + is_active=True, + retired=False, + ) + if not user.is_staff: + mapping_qs = Mapping.apply_user_criteria(mapping_qs, user) mapping_qs = ( - Mapping.objects.filter( - from_concept_id__isnull=False, - is_active=True, - retired=False, - ) + mapping_qs .select_related('to_source', 'to_concept', 'to_concept__parent') .order_by('map_type', 'to_concept_code', 'to_concept__mnemonic') .distinct() @@ -354,6 +361,7 @@ def concept_ids_from_es( query: str, source_version: Optional[Source], pagination: Optional[dict], + user, ) -> Optional[tuple[list[int], int]]: trimmed = query.strip() if not trimmed: @@ -368,6 +376,9 @@ def concept_ids_from_es( else: search = search.filter('term', source_version=source_version.version) search = search.filter('term', retired=False) + visible_repo_criteria = get_visible_repo_criteria(user) + if visible_repo_criteria: + search = search.filter(visible_repo_criteria) should_queries = [ ES_Q('match', id={'query': trimmed, 'boost': 6, 'operator': 'AND'}), @@ -425,14 +436,15 @@ async def concepts_for_ids( return await sync_to_async(list)(qs), total -async def concepts_for_query( +async def concepts_for_query( # pylint: disable=too-many-arguments base_qs, query: str, source_version: Source, pagination: Optional[dict], mapping_prefetch: Prefetch, + user, ) -> tuple[List[Concept], int]: - es_result = await sync_to_async(concept_ids_from_es)(query, source_version, pagination) + es_result = await sync_to_async(concept_ids_from_es)(query, source_version, pagination, user) if es_result is not None: concept_ids, total = es_result if not concept_ids: @@ -488,16 +500,17 @@ async def concepts( # pylint: disable=too-many-arguments,too-many-locals raise GraphQLError('Either conceptIds or query must be provided.') pagination = normalize_pagination(page, limit) + user = info.context.user if org and source: - source_version = await resolve_source_version(org, source, version) + source_version = await resolve_source_version(org, source, version, user) base_qs = build_base_queryset(source_version) mapping_prefetch = build_mapping_prefetch(source_version) else: - # Global search across all repositories + # Global search across all repositories the user can view source_version = None - base_qs = Concept.objects.filter(is_active=True, retired=False) - mapping_prefetch = build_global_mapping_prefetch() + base_qs = get_viewable_concepts(Concept.objects.filter(is_active=True, retired=False), info.context) + mapping_prefetch = build_global_mapping_prefetch(user) if concept_ids_param: concepts, total = await concepts_for_ids(base_qs, concept_ids_param, pagination, mapping_prefetch) @@ -508,6 +521,7 @@ async def concepts( # pylint: disable=too-many-arguments,too-many-locals source_version, pagination, mapping_prefetch, + user, ) serialized = await sync_to_async(serialize_concepts)(concepts) diff --git a/core/graphql/tests/test_concepts_from_source.py b/core/graphql/tests/test_concepts_from_source.py index c4caf4013..83d30765e 100644 --- a/core/graphql/tests/test_concepts_from_source.py +++ b/core/graphql/tests/test_concepts_from_source.py @@ -1,11 +1,15 @@ import json import os +import uuid from unittest import mock from django.test import override_settings +from rest_framework.authtoken.models import Token -from core.common.constants import HEAD +from core.common.constants import ACCESS_TYPE_NONE, HEAD from core.common.tests import OCLTestCase +from core.concepts.documents import ConceptDocument +from core.concepts.models import Concept from core.concepts.tests.factories import ( ConceptDescriptionFactory, ConceptFactory, @@ -103,17 +107,36 @@ def setUp(self): self.concept1.extras = {**(self.concept1.extras or {}), 'is_set': False} self.concept1.save(update_fields=['extras']) - def _execute(self, query: str, variables: dict): + def _post(self, query: str, variables: dict, user=None): + auth_header = auth_header_for_token(Token.objects.get_or_create(user=user)[0]) if user else self.auth_header response = self.client.post( '/graphql/', data=json.dumps({'query': query, 'variables': variables}), content_type='application/json', - HTTP_AUTHORIZATION=self.auth_header, + HTTP_AUTHORIZATION=auth_header, ) - payload = response.json() + return response.status_code, response.json() + + def _execute(self, query: str, variables: dict, user=None): + status, payload = self._post(query, variables, user) if 'errors' in payload: self.fail(payload['errors']) - return response.status_code, payload['data'] + return status, payload['data'] + + def _create_private_source(self, source_mnemonic='GQLPRIVATE', concept_name='Private concept'): + """A private org source with one concept and a mapping from the public concept1, and an org member.""" + private_org = OrganizationFactory(mnemonic='GQLPRIVORG', public_access=ACCESS_TYPE_NONE) + private_source = OrganizationSourceFactory( + organization=private_org, mnemonic=source_mnemonic, version=HEAD, public_access=ACCESS_TYPE_NONE) + private_concept = ConceptFactory( + parent=private_source, mnemonic='gql-private-1', public_access=ACCESS_TYPE_NONE, + names=[ConceptNameFactory.build(name=concept_name, locale='en', locale_preferred=True)]) + MappingFactory( + parent=private_source, from_concept=self.concept1, to_concept=private_concept, map_type='Narrower Than', + public_access=ACCESS_TYPE_NONE) + member, _ = create_user_with_token(username='graphql-private-member', super_user=self.super_user) + private_org.members.add(member) + return private_source, private_concept, member def tearDown(self): if self._old_async_flag is None: @@ -557,3 +580,83 @@ def test_serialize_concepts_includes_metadata_for_retired_concepts(self): self.assertEqual(serialized.metadata.updated_by, retired_user.username) self.assertEqual(serialized.metadata.created_at, format_datetime_for_api(retired.created_at)) self.assertEqual(serialized.metadata.updated_at, format_datetime_for_api(retired.updated_at)) + + def test_private_source_is_reported_as_not_found(self): + private_source, private_concept, member = self._create_private_source() + query = """ + query ($org: String, $source: String, $conceptIds: [String!]) { + concepts(org: $org, source: $source, conceptIds: $conceptIds) { + totalCount + results { conceptId } + } + } + """ + variables = {'org': 'GQLPRIVORG', 'source': private_source.mnemonic, 'conceptIds': [private_concept.mnemonic]} + + _, missing = self._post(query, {**variables, 'source': 'NOSUCHSOURCE'}) + _, private = self._post(query, variables) + + self.assertIsNone(private['data']) + self.assertEqual( + [error['message'] for error in private['errors']], + [error['message'].replace('NOSUCHSOURCE', private_source.mnemonic) for error in missing['errors']]) + for user in [member, self.super_user]: + _, data = self._execute(query, variables, user) + self.assertEqual(data['concepts']['totalCount'], 1) + self.assertEqual(data['concepts']['results'][0]['conceptId'], private_concept.mnemonic) + + def test_global_concept_ids_lookup_lists_only_viewable_concepts_and_mappings(self): + _, private_concept, member = self._create_private_source() + query = """ + query ($conceptIds: [String!]) { + concepts(conceptIds: $conceptIds) { + totalCount + results { conceptId mappings { mapType } } + } + } + """ + variables = {'conceptIds': [self.concept1.mnemonic, private_concept.mnemonic]} + + for user, concept_ids, map_types in [ + (None, {self.concept1.mnemonic}, {'Same As'}), + (member, {self.concept1.mnemonic, private_concept.mnemonic}, {'Same As', 'Narrower Than'}), + (self.super_user, {self.concept1.mnemonic, private_concept.mnemonic}, {'Same As', 'Narrower Than'})]: + _, data = self._execute(query, variables, user) + results = data['concepts']['results'] + self.assertEqual({result['conceptId'] for result in results}, concept_ids, user) + self.assertEqual(data['concepts']['totalCount'], len(results), user) + self.assertEqual( + {mapping['mapType'] for result in results for mapping in result['mappings']}, map_types, user) + + @mock.patch('core.graphql.queries.fallback_db_search', side_effect=AssertionError('expected an ES search')) + def test_query_search_lists_and_counts_only_viewable_concepts(self, _fallback_mock): + self.patch_concept_es_mapping_for_ci() + token = f'zyq{uuid.uuid4().hex[:10]}' + public_concept = ConceptFactory( + parent=self.source, mnemonic='gql-public-1', + names=[ConceptNameFactory.build(name=f'{token} public', locale='en', locale_preferred=True)]) + # a private source with the same name as the public one, in another org + _, private_concept, member = self._create_private_source(self.source.mnemonic, f'{token} private') + ConceptDocument().update( + Concept.objects.filter(mnemonic__in=[public_concept.mnemonic, private_concept.mnemonic]), refresh=True) + query = """ + query ($org: String, $source: String, $text: String!) { + concepts(org: $org, source: $source, query: $text) { + totalCount + results { conceptId } + } + } + """ + + for user, concept_ids in [ + (None, {public_concept.mnemonic}), + (member, {public_concept.mnemonic, private_concept.mnemonic}), + (self.super_user, {public_concept.mnemonic, private_concept.mnemonic})]: + _, data = self._execute(query, {'text': token}, user) + results = data['concepts']['results'] + self.assertEqual({result['conceptId'] for result in results}, concept_ids, user) + self.assertEqual(data['concepts']['totalCount'], len(results), user) + + _, data = self._execute( + query, {'org': self.organization.mnemonic, 'source': self.source.mnemonic, 'text': token}) + self.assertEqual(data['concepts']['totalCount'], 1) diff --git a/core/graphql/tests/test_query_helpers.py b/core/graphql/tests/test_query_helpers.py index e0148904b..3d3d99c9b 100644 --- a/core/graphql/tests/test_query_helpers.py +++ b/core/graphql/tests/test_query_helpers.py @@ -250,7 +250,7 @@ def test_resolve_source_version_and_base_queries(self): ) with patch('core.graphql.queries.Source.get_version', return_value=self.source): success = async_to_sync(resolve_source_version)( - self.organization.mnemonic, self.source.mnemonic, None + self.organization.mnemonic, self.source.mnemonic, None, self.audit_user ) self.assertEqual(success, self.source) @@ -258,17 +258,17 @@ def test_resolve_source_version_and_base_queries(self): 'core.graphql.queries.Source.find_latest_released_version_by', return_value=fallback_only ): resolved = async_to_sync(resolve_source_version)( - self.organization.mnemonic, fallback_only.mnemonic, None + self.organization.mnemonic, fallback_only.mnemonic, None, self.audit_user ) self.assertEqual(resolved, fallback_only) with self.assertRaises(GraphQLError): async_to_sync(resolve_source_version)( - self.organization.mnemonic, 'missing-source', 'v-does-not-exist' + self.organization.mnemonic, 'missing-source', 'v-does-not-exist', self.audit_user ) base_qs = build_base_queryset(self.source) mapping_prefetch = build_mapping_prefetch(self.source) - global_prefetch = build_global_mapping_prefetch() + global_prefetch = build_global_mapping_prefetch(self.audit_user) self.assertIsNotNone(mapping_prefetch) self.assertIsNotNone(global_prefetch) @@ -287,7 +287,7 @@ def test_resolve_source_version_error_path_and_pagination_defaults(self): 'core.graphql.queries.Source.find_latest_released_version_by', return_value=None ): with self.assertRaises(GraphQLError): - async_to_sync(resolve_source_version)('ORG', 'SRC', None) + async_to_sync(resolve_source_version)('ORG', 'SRC', None, self.audit_user) self.assertIsNone(normalize_pagination(None, None)) self.assertFalse(has_next(10, None)) @@ -387,7 +387,7 @@ def test_datatype_helpers(self): self.assertIsNone(format_datetime_for_api(None)) def test_concept_ids_from_es_paths(self): - ids, total = concept_ids_from_es(' ', self.source, None) + ids, total = concept_ids_from_es(' ', self.source, None, self.audit_user) self.assertEqual(ids, []) self.assertEqual(total, 0) @@ -426,12 +426,12 @@ def execute(self): 'core.graphql.queries.ConceptDocument.search', return_value=FakeSearch([self.concept1.id, self.concept2.id]), ): - ids, total = concept_ids_from_es('search text', self.source, {'start': 0, 'end': 1}) + ids, total = concept_ids_from_es('search text', self.source, {'start': 0, 'end': 1}, self.audit_user) self.assertEqual(ids, [self.concept1.id]) self.assertEqual(total, 2) with patch('core.graphql.queries.ConceptDocument.search', side_effect=Exception('boom')): - self.assertIsNone(concept_ids_from_es('text', self.source, None)) + self.assertIsNone(concept_ids_from_es('text', self.source, None, self.audit_user)) def test_fallback_and_concepts_queries(self): base_qs = build_base_queryset(self.source) @@ -453,20 +453,20 @@ def test_fallback_and_concepts_queries(self): with patch('core.graphql.queries.concept_ids_from_es', return_value=([self.concept2.id], 1)): concepts, total = async_to_sync(concepts_for_query)( - base_qs, 'anything', self.source, None, mapping_prefetch + base_qs, 'anything', self.source, None, mapping_prefetch, self.audit_user ) self.assertEqual(total, 1) self.assertEqual(concepts[0].id, self.concept2.id) with patch('core.graphql.queries.concept_ids_from_es', return_value=None): concepts, total = async_to_sync(concepts_for_query)( - base_qs, 'UTIL', self.source, normalize_pagination(1, 1), mapping_prefetch + base_qs, 'UTIL', self.source, normalize_pagination(1, 1), mapping_prefetch, self.audit_user ) self.assertGreaterEqual(total, 1) with patch('core.graphql.queries.concept_ids_from_es', return_value=([], 2)): concepts, total = async_to_sync(concepts_for_query)( - base_qs, 'UTIL', self.source, None, mapping_prefetch + base_qs, 'UTIL', self.source, None, mapping_prefetch, self.audit_user ) self.assertEqual(total, 2) self.assertEqual(concepts, []) @@ -480,7 +480,7 @@ def test_query_concepts_auth_and_results(self): with self.assertRaises(GraphQLError): async_to_sync(Query().concepts)(info_invalid, query='test') - info_valid = SimpleNamespace(context=SimpleNamespace(auth_status='valid')) + info_valid = SimpleNamespace(context=SimpleNamespace(auth_status='valid', user=self.audit_user)) with self.assertRaises(GraphQLError): async_to_sync(Query().concepts)(info_valid) diff --git a/core/importers/models.py b/core/importers/models.py index c06b2af23..1c41e76bd 100644 --- a/core/importers/models.py +++ b/core/importers/models.py @@ -228,6 +228,8 @@ def parse(self): def process(self): if not self.exists(): + if Organization.objects.filter(mnemonic__iexact=self.data['mnemonic']).exists(): + return FAILED # org names are unique regardless of case org = Organization.objects.create(**self.data) org.members.add(org.created_by) if org: diff --git a/core/importers/tests.py b/core/importers/tests.py index be4080cc0..a169d34df 100644 --- a/core/importers/tests.py +++ b/core/importers/tests.py @@ -2087,6 +2087,14 @@ def test_organization_importer_delete_permission_denied(self): self.assertEqual(importer.delete(), PERMISSION_DENIED) self.assertTrue(Organization.objects.filter(mnemonic=org.mnemonic).exists()) + def test_organization_importer_does_not_create_org_whose_name_differs_only_in_case(self): + OrganizationFactory(mnemonic='TwinOrg') + importer = OrganizationImporter({'id': 'twinorg', 'name': 'Twin'}, UserProfileFactory()) + importer.parse() + + self.assertEqual(importer.process(), FAILED) + self.assertFalse(Organization.objects.filter(mnemonic='twinorg').exists()) + def test_organization_importer_delete_not_found(self): importer = OrganizationImporter({'id': 'DoesNotExist'}, UserProfileFactory()) self.assertEqual(importer.delete(), NOT_FOUND) diff --git a/core/integration_tests/tests_access_control.py b/core/integration_tests/tests_access_control.py index 4d7781f27..47c0f3883 100644 --- a/core/integration_tests/tests_access_control.py +++ b/core/integration_tests/tests_access_control.py @@ -1,3 +1,7 @@ +import csv +import io +import zipfile + from mock import patch, Mock from core.client_configs.models import ClientConfig @@ -9,13 +13,14 @@ from core.common.tests import OCLAPITestCase, OCLTestCase from core.concepts.models import Concept from core.concepts.tests.factories import ConceptFactory, ConceptNameFactory, ConceptDescriptionFactory -from core.map_projects.tests.factories import MapProjectFactory +from core.map_projects.tests.factories import MapProjectFactory, AutomatchRunFactory from core.orgs.models import Organization from core.orgs.tests.factories import OrganizationFactory from core.pins.models import Pin from core.sources.documents import SourceDocument from core.sources.models import Source from core.sources.tests.factories import OrganizationSourceFactory, UserSourceFactory +from core.tasks.models import Task from core.url_registry.factories import GlobalURLRegistryFactory, OrganizationURLRegistryFactory, UserURLRegistryFactory from core.url_registry.models import URLRegistry from core.users.models import UserProfile @@ -553,6 +558,70 @@ def test_members_and_staff_see_private_orgs(self): self.list_orgs(self.member.uri + 'orgs/', self.other_member), all_orgs) +class CsvExportAccessTest(AccessTestMixin, OCLAPITestCase): + """A ?csv=true export holds only rows the requester can list, and one requester's export is never served to + another.""" + def setUp(self): + super().setUp() + self.exports = {} + export_service = Mock(upload_file=self.upload, url_for=lambda key: 'https://exports.test/' + key) + patcher = patch('core.common.utils.get_export_service', return_value=export_service) + patcher.start() + self.addCleanup(patcher.stop) + + def upload(self, key, file_path=None, **kwargs): # pylint: disable=unused-argument + with open(file_path, 'rb') as file: + self.exports[key] = file.read() + + def export(self, url, user, column): + """The `column` values of the CSV export at `url` for `user`.""" + response = self.request('get', url, user) + self.assertEqual(response.status_code, 200) + with zipfile.ZipFile(io.BytesIO(self.exports[response.data['url'].removeprefix('https://exports.test/')])) \ + as zip_file, zip_file.open(zip_file.namelist()[0]) as csv_file: + return sorted(row[column] for row in csv.DictReader(io.TextIOWrapper(csv_file, 'utf-8-sig'))) + + def test_org_list(self): + private_org = OrganizationFactory(public_access=ACCESS_TYPE_NONE) + + self.assertIn(private_org.mnemonic, self.export('/orgs/?csv=true', self.staff, 'mnemonic')) + for url in ['/orgs/?csv=true', '/orgs/?csv=true&type=Organization']: + mnemonics = self.export(url, None, 'mnemonic') + self.assertIn(self.org.mnemonic, mnemonics) + self.assertNotIn(private_org.mnemonic, mnemonics) + + def test_task_list(self): + Task.objects.create(name='csv-owner-task', created_by=self.owner) + url = self.owner.uri + 'tasks/?csv=true' + + self.assertEqual(self.export(url, self.owner, 'name'), ['csv-owner-task']) + self.assertEqual(self.export(url, self.outsider, 'name'), []) + + def test_map_project_list(self): + MapProjectFactory(user=self.owner, organization=None, public_access=ACCESS_TYPE_NONE, name='csv-project') + url = self.owner.uri + 'map-projects/?csv=true&verbose=true' + + self.assertEqual(self.export(url, self.owner, 'name'), ['csv-project']) + self.assertEqual(self.export(url, None, 'name'), []) + + def test_repo_concepts(self): + source = UserSourceFactory(user=self.owner, public_access=ACCESS_TYPE_NONE) + concept = ConceptFactory(parent=source, mnemonic='csv-concept') + url = source.concepts_url + '?csv=true&limit=1000' + + self.assertEqual(self.export(url, self.owner, 'mnemonic'), [concept.mnemonic]) + self.assertEqual(self.request('get', url, self.outsider).status_code, 403) + + def test_auto_match_run_list_checks_the_project(self): + project = MapProjectFactory(user=self.owner, organization=None, public_access=ACCESS_TYPE_NONE) + run = AutomatchRunFactory(map_project=project) + url = f'{self.owner.uri}map-projects/{project.id}/auto-match-runs/?csv=true' + + self.assertEqual(self.export(url, self.owner, 'id'), [str(run.id)]) + self.assertEqual(self.request('get', url, self.outsider).status_code, 403) + self.assertEqual(self.request('get', url, None).status_code, 401) + + class OrganizationExtrasAccessTest(AccessTestMixin, OCLAPITestCase): def setUp(self): super().setUp() diff --git a/core/integration_tests/tests_concepts.py b/core/integration_tests/tests_concepts.py index eda91604a..95c389dc7 100644 --- a/core/integration_tests/tests_concepts.py +++ b/core/integration_tests/tests_concepts.py @@ -3314,6 +3314,26 @@ def test_repo_version_concepts_are_served_from_cache(self): self.assertEqual(response.status_code, 200) self.assertEqual(response.data, [{'id': 'FromCache'}]) + def test_repo_version_concepts_with_hierarchy_includes_are_not_cached(self): + body_key, _ = self.source_v1.get_concepts_cache_keys() + + for param in ['includeParentConcepts', 'includeChildConcepts']: + key = f'{body_key}?{param}=true' + url = f'/orgs/{self.org}/sources/MySource/v1/concepts/?{param}=true' + + response = self.client.get(url) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data[0]['id'], 'MyConcept1') + self.assertIsNone(cache.get(key)) + + # the included concepts are filtered per requester, so a cached response is never served + cache.set(key, [{'id': 'FromCache'}]) + response = self.client.get(url) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data[0]['id'], 'MyConcept1') + def test_repo_version_concepts_facets_are_cached(self): body_key, _ = self.source_v1.get_concepts_cache_keys() facets_key = body_key + '?facetsOnly=true' diff --git a/core/integration_tests/tests_cross_repo_access.py b/core/integration_tests/tests_cross_repo_access.py index bdb763a77..cb6de1ca2 100644 --- a/core/integration_tests/tests_cross_repo_access.py +++ b/core/integration_tests/tests_cross_repo_access.py @@ -3,11 +3,15 @@ from mock import patch from core.bundles.models import Bundle +from core.collections.models import CollectionReference +from core.collections.tests.factories import OrganizationCollectionFactory, ExpansionFactory, UserCollectionFactory from core.common.constants import ACCESS_TYPE_NONE from core.common.tests import OCLAPITestCase from core.concepts.models import Concept from core.concepts.tests.factories import ConceptFactory, ConceptNameFactory +from core.mappings.tests.factories import MappingFactory from core.orgs.tests.factories import OrganizationFactory +from core.sources.models import Source from core.sources.tests.factories import OrganizationSourceFactory, UserSourceFactory from core.users.models import UserProfile from core.users.tests.factories import UserProfileFactory @@ -169,6 +173,54 @@ def test_include_parent_concepts_hides_private_parents(self): self.assertEqual( sorted(concept['id'] for concept in response.data['parent_concepts']), ['public', 'secret']) + def test_viewable_parent_uris_reads_linked_parents_only_when_a_parent_is_not_viewable(self): + def not_needed(): + raise AssertionError('linked parents were read') + + with self.assertNumQueries(0): + self.assertEqual(Concept.get_viewable_parent_uris([], self.outsider, not_needed), []) + self.assertEqual( + Concept.get_viewable_parent_uris([self.public_concept.uri], self.outsider, not_needed), + [self.public_concept.uri]) + for linked, expected in [ + ([self.private_concept.uri], [self.private_concept.uri, self.public_concept.uri]), + ([], [self.public_concept.uri])]: + self.assertEqual( + Concept.get_viewable_parent_uris( + [self.private_concept.uri, self.public_concept.uri], self.outsider, lambda linked=linked: linked), + expected) + + def test_parents_and_children_lists_hide_private_concepts(self): + child = self.persist_child('listedchild', [self.private_concept.uri, self.public_concept.uri]) + private_child = Concept.persist_new({ + **factory.build(dict, FACTORY_CLASS=ConceptFactory), 'mnemonic': 'privchild', + 'parent': self.private_source, 'public_access': ACCESS_TYPE_NONE, + 'names': [ConceptNameFactory.build(locale='en', locale_preferred=True)], + 'parent_concept_urls': [self.public_concept.uri] + }, self.member) + self.assertEqual(private_child.errors, {}) + + for user, parents, children in [ + (None, ['public'], ['listedchild']), + (self.outsider, ['public'], ['listedchild']), + (self.member, ['public', 'secret'], ['listedchild', 'privchild']), + (self.admin, ['public', 'secret'], ['listedchild', 'privchild'])]: + headers = {'HTTP_AUTHORIZATION': 'Token ' + user.get_token()} if user else {} + + response = self.client.get(child.uri + 'parents/', **headers) + self.assertEqual(response.status_code, 200) + self.assertEqual(sorted(concept['id'] for concept in response.data), parents, user) + + response = self.client.get(self.public_concept.uri + 'children/', **headers) + self.assertEqual(response.status_code, 200) + self.assertEqual(sorted(concept['id'] for concept in response.data), children, user) + + # these lists aren't searchable: a search would list the repo's concepts instead + for url, expected in [(child.uri + 'parents/?q=secret', parents), + (self.public_concept.uri + 'children/?q=privchild', children)]: + response = self.client.get(url, **headers) + self.assertEqual(sorted(concept['id'] for concept in response.data), expected, (user, url)) + class MappingTargetAccessTest(CrossRepoAccessBaseTest): def test_outsider_mapping_to_private_concept_is_external(self): @@ -228,6 +280,55 @@ def test_member_mapping_to_private_concept_is_linked(self): self.assertEqual(mapping.to_concept.versioned_object_id, self.private_concept.id) self.assertEqual(mapping.to_source_id, self.private_source.id) + def map_to(self, user, source, to_concept_url): + from_concept = ConceptFactory(parent=source) + response = self.client.post( + source.mappings_url, + {'map_type': 'SAME-AS', 'from_concept_url': from_concept.uri, 'to_concept_url': to_concept_url}, + HTTP_AUTHORIZATION='Token ' + user.get_token(), + format='json' + ) + self.assertEqual(response.status_code, 201, response.data) + return source.mappings.get(from_concept__versioned_object_id=from_concept.id, id=F('versioned_object_id')) + + def test_concept_created_later_is_linked_only_to_mappings_whose_saver_can_view_it(self): + member_source = OrganizationSourceFactory( + organization=self.private_org, mnemonic='MemberMapSource', public_access=ACCESS_TYPE_NONE) + outsider_mapping = self.map_to(self.outsider, self.outsider_source, self.private_source.uri + 'concepts/later/') + member_mapping = self.map_to(self.member, member_source, self.private_source.uri + 'concepts/later/') + public_mapping = self.map_to(self.outsider, self.outsider_source, self.public_source.uri + 'concepts/later/') + + for source in [self.private_source, self.public_source]: + response = self.client.post( + source.concepts_url, self.concept_payload('later'), + HTTP_AUTHORIZATION='Token ' + self.admin.get_token(), format='json') + self.assertEqual(response.status_code, 201, response.data) + + outsider_mapping.refresh_from_db() + self.assertIsNone(outsider_mapping.to_concept_id) + self.assertEqual(outsider_mapping.to_concept_code, 'later') + for mapping, source in [(member_mapping, self.private_source), (public_mapping, self.public_source)]: + mapping.refresh_from_db() + self.assertEqual(mapping.to_concept.parent_id, source.id) + + def test_source_created_later_is_linked_only_to_mappings_whose_saver_can_view_it(self): + member_source = OrganizationSourceFactory( + organization=self.private_org, mnemonic='MemberMapSource', public_access=ACCESS_TYPE_NONE) + outsider_mapping = self.map_to(self.outsider, self.outsider_source, '/orgs/PrivOrg/sources/NewPriv/concepts/x/') + member_mapping = self.map_to(self.member, member_source, '/orgs/PrivOrg/sources/NewPriv/concepts/x/') + + errors = Source.persist_new( + OrganizationSourceFactory.build( + organization=self.private_org, mnemonic='NewPriv', public_access=ACCESS_TYPE_NONE), + self.member, parent_resource=self.private_org) + self.assertEqual(errors, {}) + new_source = Source.objects.get(mnemonic='NewPriv', version='HEAD') + + outsider_mapping.refresh_from_db() + self.assertIsNone(outsider_mapping.to_source_id) + member_mapping.refresh_from_db() + self.assertEqual(member_mapping.to_source_id, new_source.id) + def test_edit_by_user_who_cannot_view_target_keeps_link(self): from_concept = ConceptFactory(parent=self.outsider_source, mnemonic='from') response = self.client.post( @@ -254,3 +355,85 @@ def test_edit_by_user_who_cannot_view_target_keeps_link(self): mapping.refresh_from_db() self.assertEqual(mapping.to_concept_id, linked_concept_id) self.assertEqual(mapping.to_source_id, linked_source_id) + + +class OmitIfExistsInAccessTest(CrossRepoAccessBaseTest): + def setUp(self): + super().setUp() + self.omitted = ConceptFactory(parent=self.public_source, mnemonic='omitted') + self.mapping = MappingFactory( + parent=self.public_source, from_concept=self.public_concept, to_concept=self.omitted) + self.private_collection = OrganizationCollectionFactory( + organization=self.private_org, mnemonic='PrivColl', public_access=ACCESS_TYPE_NONE) + expansion = ExpansionFactory(collection_version=self.private_collection) + self.private_collection.expansion_uri = expansion.uri + self.private_collection.save() + expansion.concepts.add(self.omitted.get_latest_version()) + + def cascade(self, user, omit_if_exists_in): + headers = {'HTTP_AUTHORIZATION': 'Token ' + user.get_token()} if user else {} + response = self.client.get( + self.public_concept.uri + '$cascade/?omitIfExistsIn=' + omit_if_exists_in, **headers) + self.assertEqual(response.status_code, 200) + return sorted(entry['url'] for entry in response.data['entry']) + + def test_cascade_ignores_omit_repo_the_requester_cannot_view(self): + everything = sorted([self.public_concept.uri, self.mapping.uri, self.omitted.uri]) + self.assertEqual(self.cascade(None, '/orgs/PrivOrg/collections/NoSuchColl/'), everything) + + for user in [None, self.outsider]: + self.assertEqual(self.cascade(user, self.private_collection.uri), everything, user) + + for user in [self.member, self.admin]: + self.assertEqual( + self.cascade(user, self.private_collection.uri), sorted([self.public_concept.uri, self.mapping.uri]), + user) + + def test_reference_ignores_omit_repo_its_creator_cannot_view(self): + for user, kept in [(self.outsider, False), (self.member, True), (self.admin, True), (None, False)]: + reference = CollectionReference( + expression=self.public_source.uri, + cascade={'method': 'sourcetoconcepts', 'omit_if_exists_in': self.private_collection.uri}, + created_by=user) + + self.assertEqual('omit_if_exists_in' in reference.get_concept_cascade_params(), kept, user) + + +class CascadeRepoAccessTest(CrossRepoAccessBaseTest): + def test_cascade_and_clone_over_a_repo_the_requester_cannot_view_are_not_found(self): + MappingFactory(parent=self.private_source, from_concept=self.private_concept, to_concept=self.public_concept) + # the root concept comes from the query params, the repo to cascade over from the URL + url = (f'{self.private_concept.uri}$cascade/?org={self.public_source.organization.mnemonic}' + f'&source={self.public_source.mnemonic}&concept={self.public_concept.mnemonic}&reverse=true') + + self.assertEqual(self.client.get(url).status_code, 404) + self.assertEqual(self.client.get(url, HTTP_AUTHORIZATION='Token ' + self.outsider_token).status_code, 404) + response = self.client.get(url, HTTP_AUTHORIZATION='Token ' + self.member.get_token()) + self.assertEqual(response.status_code, 200) + self.assertIn(self.private_concept.uri, str(response.data)) + + response = self.client.post( + url.replace('$cascade/', '$clone/'), + {'source_uri': self.outsider_source.uri, 'parameters': {'reverse': True}}, + HTTP_AUTHORIZATION='Token ' + self.outsider_token, format='json') + self.assertEqual(response.status_code, 404) + + +class CollectionConceptMappingsTest(CrossRepoAccessBaseTest): + def test_concept_mappings_in_a_collection_through_the_current_user_route(self): + mapping = MappingFactory( + parent=self.public_source, from_concept=self.public_concept, to_concept=self.public_concept) + collection = UserCollectionFactory(user=self.outsider, mnemonic='MyColl') + expansion = ExpansionFactory(collection_version=collection) + collection.expansion_uri = expansion.uri + collection.save() + expansion.concepts.add(self.public_concept.get_latest_version()) + expansion.mappings.add(mapping.get_latest_version()) + + for url in [f'/users/{self.outsider.username}/collections/MyColl/', '/user/collections/MyColl/']: + for param in ['includeMappings', 'includeInverseMappings']: + response = self.client.get( + f'{url}concepts/public/?{param}=true', HTTP_AUTHORIZATION='Token ' + self.outsider_token) + + self.assertEqual(response.status_code, 200, (url, param)) + self.assertEqual({data['url'] for data in response.data['mappings']}, {mapping.url}, (url, param)) diff --git a/core/integration_tests/tests_events.py b/core/integration_tests/tests_events.py index 2b9b4fc86..d10e0a875 100644 --- a/core/integration_tests/tests_events.py +++ b/core/integration_tests/tests_events.py @@ -172,11 +172,11 @@ def test_get_following_events(self): # pylint:disable=too-many-statements,too-m # 1. Mario's event of joining Gotham Gazette # 2. Penguin's event of joining Gotham # 3. Joker's event of joining Gotham - # 4. Bruce's follow of Gotham Gazette Crime Deptt - # 5. Bruce's follow of Gotham Gazette - # 6. Joker's event of following Bruce - # 7. OCL's event of creation of Gotham Gazette - self.assertEqual(len(response.data), 7) + # 4. Bruce's follow of Gotham Gazette + # 5. Joker's event of following Bruce + # 6. OCL's event of creation of Gotham Gazette + # Not Bruce's follow of the private Gotham Gazette Crime Deptt: that event is private + self.assertEqual(len(response.data), 6) self.assertEqual(response.data[0]['object']['url'], mario.url) self.assertEqual(response.data[0]['referenced_object']['url'], gotham_gazette.url) self.assertEqual(response.data[1]['object']['url'], penguin.url) @@ -184,13 +184,11 @@ def test_get_following_events(self): # pylint:disable=too-many-statements,too-m self.assertEqual(response.data[2]['object']['url'], joker.url) self.assertEqual(response.data[2]['referenced_object']['url'], gotham.url) self.assertEqual(response.data[3]['object']['url'], bruce.url) - self.assertEqual(response.data[3]['referenced_object']['url'], gotham_gazette_crime_repo.url) - self.assertEqual(response.data[4]['object']['url'], bruce.url) - self.assertEqual(response.data[4]['referenced_object']['url'], gotham_gazette.url) - self.assertEqual(response.data[5]['object']['url'], joker.url) - self.assertEqual(response.data[5]['referenced_object']['url'], bruce.url) - self.assertEqual(response.data[6]['object']['url'], '/users/ocladmin/') - self.assertEqual(response.data[6]['referenced_object']['url'], gotham_gazette.url) + self.assertEqual(response.data[3]['referenced_object']['url'], gotham_gazette.url) + self.assertEqual(response.data[4]['object']['url'], joker.url) + self.assertEqual(response.data[4]['referenced_object']['url'], bruce.url) + self.assertEqual(response.data[5]['object']['url'], '/users/ocladmin/') + self.assertEqual(response.data[5]['referenced_object']['url'], gotham_gazette.url) response = self.client.get( '/users/bruce/events/?scopes=following', @@ -201,12 +199,12 @@ def test_get_following_events(self): # pylint:disable=too-many-statements,too-m # 1. Mario's event of joining Gotham Gazette # 2. Penguin's event of joining Gotham # 3. Joker's event of joining Gotham - # 4. Bruce's follow of Gotham Gazette Crime Deptt - # 5. Bruce's follow of Gotham Gazette - # 6. Joker's event of following Bruce - # 7. OCL's event of creation of Gotham Gazette - # Not the private creation of Gotham Gazette Crime Deptt: Bruce can't view that repo - self.assertEqual(len(response.data), 7) + # 4. Bruce's follow of Gotham Gazette + # 5. Joker's event of following Bruce + # 6. OCL's event of creation of Gotham Gazette + # Not the private creation of Gotham Gazette Crime Deptt, nor Bruce's (private) follow of it: + # Bruce can't view that repo + self.assertEqual(len(response.data), 6) self.assertEqual(response.data[0]['object']['url'], mario.url) self.assertEqual(response.data[0]['referenced_object']['url'], gotham_gazette.url) self.assertEqual(response.data[1]['object']['url'], penguin.url) @@ -214,13 +212,11 @@ def test_get_following_events(self): # pylint:disable=too-many-statements,too-m self.assertEqual(response.data[2]['object']['url'], joker.url) self.assertEqual(response.data[2]['referenced_object']['url'], gotham.url) self.assertEqual(response.data[3]['object']['url'], bruce.url) - self.assertEqual(response.data[3]['referenced_object']['url'], gotham_gazette_crime_repo.url) - self.assertEqual(response.data[4]['object']['url'], bruce.url) - self.assertEqual(response.data[4]['referenced_object']['url'], gotham_gazette.url) - self.assertEqual(response.data[5]['object']['url'], joker.url) - self.assertEqual(response.data[5]['referenced_object']['url'], bruce.url) - self.assertEqual(response.data[6]['object']['url'], '/users/ocladmin/') - self.assertEqual(response.data[6]['referenced_object']['url'], gotham_gazette.url) + self.assertEqual(response.data[3]['referenced_object']['url'], gotham_gazette.url) + self.assertEqual(response.data[4]['object']['url'], joker.url) + self.assertEqual(response.data[4]['referenced_object']['url'], bruce.url) + self.assertEqual(response.data[5]['object']['url'], '/users/ocladmin/') + self.assertEqual(response.data[5]['referenced_object']['url'], gotham_gazette.url) mario.follow(gotham_gazette_crime_repo) response = self.client.get( @@ -235,6 +231,80 @@ def test_get_following_events(self): # pylint:disable=too-many-statements,too-m ] self.assertIn(gotham_gazette_crime_repo.url, created_urls) + def test_get_following_events_without_followed_users_private_events(self): + followed = UserProfileFactory(username='followed') + follower = UserProfileFactory(username='follower') + member = UserProfileFactory(username='member') + staff = UserProfileFactory(username='staff', is_staff=True) + private_org = OrganizationFactory(mnemonic='private-org', public_access='None') + private_org.members.add(followed, member) + joined_private_org_event = EventFactory( + object_url=followed.url, referenced_object_url=private_org.url, actor=followed, event_type='Joined', + public=False) + for user in [follower, member, staff]: + user.follow(followed) + + def get_event_ids(user, scopes): + response = self.client.get( + f'/users/{user.username}/events/?scopes={scopes}', HTTP_AUTHORIZATION=f'Token {user.get_token()}') + self.assertEqual(response.status_code, 200) + return [event['id'] for event in response.data] + + self.assertNotIn(joined_private_org_event.id, get_event_ids(follower, 'following')) + self.assertNotIn(joined_private_org_event.id, get_event_ids(follower, 'all')) + self.assertNotIn(joined_private_org_event.id, get_event_ids(member, 'following')) + self.assertIn(joined_private_org_event.id, get_event_ids(member, 'orgs')) + self.assertIn(joined_private_org_event.id, get_event_ids(staff, 'following')) + + def test_get_follow_events_of_private_objects(self): + private_org = OrganizationFactory(mnemonic='private-org', public_access='None') + private_source = OrganizationSourceFactory(organization=private_org, public_access='None') + public_org = OrganizationFactory(mnemonic='public-org') + member = UserProfileFactory(username='member') + private_org.members.add(member) + outsider = UserProfileFactory(username='outsider') + staff = UserProfileFactory(username='staff', is_staff=True) + for followed in [private_org, private_source, public_org]: + member.follow(followed) + + def get_followed_urls(user=None): + headers = {'HTTP_AUTHORIZATION': f'Token {user.get_token()}'} if user else {} + response = self.client.get('/users/member/events/', **headers) + self.assertEqual(response.status_code, 200) + return sorted(event['referenced_object']['url'] for event in response.data) + + self.assertEqual(get_followed_urls(), [public_org.url]) + self.assertEqual(get_followed_urls(outsider), [public_org.url]) + for user in [member, staff]: + self.assertEqual(get_followed_urls(user), sorted([private_org.url, private_source.url, public_org.url])) + + def test_get_events_show_objects_only_to_requesters_who_can_view_them_now(self): + org = OrganizationFactory(mnemonic='turned-private-org') + source = OrganizationSourceFactory(organization=org, mnemonic='turned-private') + member = UserProfileFactory(username='turned-member') + org.members.add(member) + follower = UserProfileFactory(username='turned-follower') + follower.follow(source) + source.public_access = 'None' + source.save() + + def get_events(user=None, params=''): + headers = {'HTTP_AUTHORIZATION': f'Token {user.get_token()}'} if user else {} + response = self.client.get(f'/users/turned-follower/events/{params}', **headers) + self.assertEqual(response.status_code, 200) + return response.data + + # the event was public when recorded; the source it shows is live, so it's shown only to those who can view it + for user in [None, follower]: + events = get_events(user) + self.assertEqual([event['referenced_object'] for event in events], [None], user) + self.assertEqual([event['description'] for event in events], ['Followed'], user) + # no CSV export, which would skip these checks + self.assertEqual(get_events(user, '?csv=true'), events, user) + events = get_events(member) + self.assertEqual([event['referenced_object']['url'] for event in events], [source.url]) + self.assertNotEqual(events[0]['description'], 'Followed') + def test_get_orgs_events(self): # pylint:disable=too-many-statements,too-many-locals alfred = UserProfileFactory(username='alfred', first_name='Alfred', last_name='Pennyworth') bruce = UserProfileFactory(username='bruce', first_name='Bruce', last_name='Wayne') @@ -407,7 +477,8 @@ def test_get_all_events(self): # pylint:disable=too-many-statements,too-many-lo '/users/bruce/events/?scopes=all' ) self.assertEqual(response.status_code, 200) - self.assertEqual(len(response.data), 12) + # not Bruce's follow of the private Gotham Gazette Crime Unit: that event is private + self.assertEqual(len(response.data), 11) self.assertEqual(response.data[0]['object']['url'], mario.url) self.assertEqual(response.data[0]['referenced_object']['url'], gotham_gazette.url) self.assertEqual(response.data[1]['object']['url'], penguin.url) @@ -421,17 +492,15 @@ def test_get_all_events(self): # pylint:disable=too-many-statements,too-many-lo self.assertEqual(response.data[5]['object']['url'], bruce.url) self.assertEqual(response.data[5]['referenced_object']['url'], wayne_corp.url) self.assertEqual(response.data[6]['object']['url'], bruce.url) - self.assertEqual(response.data[6]['referenced_object']['url'], gotham_gazette_crime_repo.url) - self.assertEqual(response.data[7]['object']['url'], bruce.url) - self.assertEqual(response.data[7]['referenced_object']['url'], gotham_gazette.url) - self.assertEqual(response.data[8]['object']['url'], joker.url) - self.assertEqual(response.data[8]['referenced_object']['url'], bruce.url) + self.assertEqual(response.data[6]['referenced_object']['url'], gotham_gazette.url) + self.assertEqual(response.data[7]['object']['url'], joker.url) + self.assertEqual(response.data[7]['referenced_object']['url'], bruce.url) + self.assertEqual(response.data[8]['object']['url'], '/users/ocladmin/') + self.assertEqual(response.data[8]['referenced_object']['url'], gotham_gazette.url) self.assertEqual(response.data[9]['object']['url'], '/users/ocladmin/') - self.assertEqual(response.data[9]['referenced_object']['url'], gotham_gazette.url) + self.assertEqual(response.data[9]['referenced_object']['url'], gotham.url) self.assertEqual(response.data[10]['object']['url'], '/users/ocladmin/') - self.assertEqual(response.data[10]['referenced_object']['url'], gotham.url) - self.assertEqual(response.data[11]['object']['url'], '/users/ocladmin/') - self.assertEqual(response.data[11]['referenced_object']['url'], wayne_corp.url) + self.assertEqual(response.data[10]['referenced_object']['url'], wayne_corp.url) response = self.client.get( '/users/bruce/events/?scopes=all', # all = orgs + following @@ -439,8 +508,9 @@ def test_get_all_events(self): # pylint:disable=too-many-statements,too-many-lo ) self.assertEqual(response.status_code, 200) bruce_all_events = response.data - # not the private creation of Gotham Gazette Crime Unit: Bruce follows it but can't view it - self.assertEqual(len(bruce_all_events), 15) + # not the private creation of Gotham Gazette Crime Unit, nor Bruce's (private) follow of it: + # Bruce follows it but can't view it + self.assertEqual(len(bruce_all_events), 14) self.assertEqual(response.data[0]['object']['url'], mario.url) self.assertEqual(response.data[0]['referenced_object']['url'], gotham_gazette.url) self.assertEqual(response.data[1]['object']['url'], penguin.url) @@ -458,19 +528,17 @@ def test_get_all_events(self): # pylint:disable=too-many-statements,too-many-lo self.assertEqual(response.data[7]['object']['url'], bruce.url) self.assertEqual(response.data[7]['referenced_object']['url'], bat_cave.url) self.assertEqual(response.data[8]['object']['url'], bruce.url) - self.assertEqual(response.data[8]['referenced_object']['url'], gotham_gazette_crime_repo.url) - self.assertEqual(response.data[9]['object']['url'], bruce.url) - self.assertEqual(response.data[9]['referenced_object']['url'], gotham_gazette.url) - self.assertEqual(response.data[10]['object']['url'], joker.url) - self.assertEqual(response.data[10]['referenced_object']['url'], bruce.url) + self.assertEqual(response.data[8]['referenced_object']['url'], gotham_gazette.url) + self.assertEqual(response.data[9]['object']['url'], joker.url) + self.assertEqual(response.data[9]['referenced_object']['url'], bruce.url) + self.assertEqual(response.data[10]['object']['url'], '/users/ocladmin/') + self.assertEqual(response.data[10]['referenced_object']['url'], gotham_gazette.url) self.assertEqual(response.data[11]['object']['url'], '/users/ocladmin/') - self.assertEqual(response.data[11]['referenced_object']['url'], gotham_gazette.url) + self.assertEqual(response.data[11]['referenced_object']['url'], gotham.url) self.assertEqual(response.data[12]['object']['url'], '/users/ocladmin/') - self.assertEqual(response.data[12]['referenced_object']['url'], gotham.url) + self.assertEqual(response.data[12]['referenced_object']['url'], bat_cave.url) self.assertEqual(response.data[13]['object']['url'], '/users/ocladmin/') - self.assertEqual(response.data[13]['referenced_object']['url'], bat_cave.url) - self.assertEqual(response.data[14]['object']['url'], '/users/ocladmin/') - self.assertEqual(response.data[14]['referenced_object']['url'], wayne_corp.url) + self.assertEqual(response.data[13]['referenced_object']['url'], wayne_corp.url) response = self.client.get( '/users/bruce/events/?scopes=orgs,following', diff --git a/core/integration_tests/tests_public_read_access.py b/core/integration_tests/tests_public_read_access.py index 52a2b2fe9..222f7051d 100644 --- a/core/integration_tests/tests_public_read_access.py +++ b/core/integration_tests/tests_public_read_access.py @@ -1,11 +1,16 @@ -from core.collections.tests.factories import OrganizationCollectionFactory +from django.core.cache import cache +from django.test import override_settings + +from core.collections.tests.factories import OrganizationCollectionFactory, ExpansionFactory from core.common.constants import ACCESS_TYPE_NONE from core.common.feeds import FeedFilterMixin, MAX_LIMIT, DEFAULT_LIMIT from core.common.tests import OCLAPITestCase from core.concepts.models import Concept from core.concepts.tests.factories import ConceptFactory +from core.mappings.tests.factories import MappingFactory from core.orgs.tests.factories import OrganizationFactory from core.sources.tests.factories import OrganizationSourceFactory +from core.users.models import UserProfile from core.users.tests.factories import UserProfileFactory @@ -24,6 +29,80 @@ def setUp(self): self.public_concept = ConceptFactory(parent=self.public_source, mnemonic='shown') +@override_settings(CACHES={'default': {'BACKEND': 'django.core.cache.backends.locmem.LocMemCache'}}) +class ConceptLookupAccessTest(PrivateRepoReadAccessBaseTest): + def setUp(self): + super().setUp() + cache.clear() + + def lookup(self, source, user=None): + headers = {'HTTP_AUTHORIZATION': 'Token ' + user.get_token()} if user else {} + return self.client.get(source.uri + 'concepts/lookup/', **headers) + + def test_private_source_lookup_only_for_members(self): + response = self.lookup(self.private_source, self.member) + + self.assertEqual(response.status_code, 200) + self.assertEqual([concept['id'] for concept in response.data], ['hidden']) + + # a member's response must not be served to others from the cache + for user in [None, self.outsider]: + response = self.lookup(self.private_source, user) + + self.assertEqual(response.status_code, 404) + + def test_public_source_lookup_for_anyone(self): + for user in [None, self.outsider, self.member]: + response = self.lookup(self.public_source, user) + + self.assertEqual(response.status_code, 200) + self.assertEqual([concept['id'] for concept in response.data], ['shown']) + + +class CollectionVersionsAccessTest(PrivateRepoReadAccessBaseTest): + def setUp(self): + super().setUp() + self.mapping = MappingFactory( + parent=self.public_source, from_concept=self.public_concept, to_concept=self.public_concept) + self.collection_urls = {} + for mnemonic, public_access in [('PublicColl', 'View'), ('PrivateColl', ACCESS_TYPE_NONE)]: + collection = OrganizationCollectionFactory( + organization=self.org, mnemonic=mnemonic, version='v1', public_access=public_access) + expansion = ExpansionFactory(collection_version=collection) + expansion.concepts.add(self.public_concept, self.public_concept.get_latest_version()) + expansion.mappings.add(self.mapping, self.mapping.get_latest_version()) + collection.refresh_from_db() + self.collection_urls[mnemonic] = collection.uri + + def get(self, url, user): + headers = {'HTTP_AUTHORIZATION': 'Token ' + user.get_token()} if user else {} + response = self.client.get(url, **headers) + self.assertEqual(response.status_code, 200, url) + return response.data + + def test_private_collection_versions_only_for_members_and_staff(self): + public_only = [self.collection_urls['PublicColl']] + both = sorted(self.collection_urls.values()) + admin = UserProfile.objects.get(username='ocladmin') + + for user, expected in [(None, public_only), (self.outsider, public_only), (self.member, both), (admin, both)]: + for resource in [self.public_concept, self.mapping]: + version = resource.get_latest_version() + + self.assertEqual(self.get(version.uri, user)['collection_versions'], expected, (user, resource)) + versions = self.get(resource.uri + 'versions/?includeCollectionVersions=true', user) + self.assertEqual( + [data['collection_versions'] for data in versions if data['uuid'] == str(version.id)], + [expected], (user, resource)) + self.assertEqual( + sorted(data['version_url'] for data in self.get(resource.uri + 'collection-versions/', user)), + expected, (user, resource)) + + entries = self.get(self.public_concept.uri + '$cascade/?verbose=true', user)['entry'] + self.assertEqual( + [entry['collection_versions'] for entry in entries if entry['type'] == 'Mapping'], [expected], user) + + class FeedAccessTest(PrivateRepoReadAccessBaseTest): def test_source_feed_only_for_public_sources(self): self.assertEqual(self.client.get(self.private_source.uri + 'atom/').status_code, 404) diff --git a/core/integration_tests/tests_search_resolution_access.py b/core/integration_tests/tests_search_resolution_access.py index af6aeb963..5335a7589 100644 --- a/core/integration_tests/tests_search_resolution_access.py +++ b/core/integration_tests/tests_search_resolution_access.py @@ -4,8 +4,16 @@ from core.common.constants import ACCESS_TYPE_NONE from core.common.search import get_visible_repo_criteria from core.common.tests import OCLAPITestCase, PREVIEW_GROUP_NAME +from core.concepts.documents import ConceptDocument +from core.concepts.models import Concept from core.concepts.search import ConceptFuzzySearch +from core.concepts.tests.factories import ConceptFactory +from core.mappings.documents import MappingDocument +from core.mappings.models import Mapping +from core.mappings.tests.factories import MappingFactory from core.orgs.tests.factories import OrganizationFactory +from core.sources.documents import SourceDocument +from core.sources.models import Source from core.sources.tests.factories import OrganizationSourceFactory from core.url_registry.factories import GlobalURLRegistryFactory from core.users.models import UserProfile @@ -35,9 +43,11 @@ def test_criteria(self): member = get_visible_repo_criteria(self.member).to_dict() self.assertEqual(member['bool']['minimum_should_match'], 1) self.assertIn({'bool': {'must': [ - {'term': {'owner_type': 'User'}}, {'term': {'owner': 'resolvemember'}}]}}, member['bool']['should']) + {'term': {'owner_type': 'User'}}, {'term': {'owner_text.keyword': 'resolvemember'}}]}}, + member['bool']['should']) self.assertIn({'bool': {'must': [ - {'term': {'owner_type': 'Organization'}}, {'terms': {'owner': ['resolveorg']}}]}}, member['bool']['should']) + {'term': {'owner_type': 'Organization'}}, {'terms': {'owner_text.keyword': ['ResolveOrg']}}]}}, + member['bool']['should']) outsider = get_visible_repo_criteria(self.outsider).to_dict() self.assertEqual(len(outsider['bool']['should']), 2) @@ -65,14 +75,13 @@ def test_match_search_is_limited_to_visible_repos(self, get_repo_params_mock): additional_filter_criterion = search_mock.call_args[0][8] self.assertEqual(additional_filter_criterion, get_visible_repo_criteria(self.outsider)) - def match(self, user, target_repo_url, target_repo=None): - data = {'rows': [{'name': 'foo'}]} + def match(self, user, target_repo_url, target_repo=None, row=None, url='/concepts/$match/'): # pylint: disable=too-many-arguments + data = {'rows': [row or {'name': 'foo'}]} if target_repo_url: data['target_repo_url'] = target_repo_url if target_repo: data['target_repo'] = target_repo - return self.client.post( - '/concepts/$match/', data, HTTP_AUTHORIZATION='Token ' + user.get_token(), format='json') + return self.client.post(url, data, HTTP_AUTHORIZATION='Token ' + user.get_token(), format='json') def test_match_target_repo_needs_view_access(self): preview = Group.objects.get(name=PREVIEW_GROUP_NAME) @@ -102,6 +111,43 @@ def test_match_target_repo_needs_view_access(self): self.match(user, target_repo_url, target_repo) self.assertEqual(search_mock.call_args[0][2], private_target_repo) + def test_match_target_repo_is_searched_by_its_stored_names(self): + self.member.groups.add(Group.objects.get(name=PREVIEW_GROUP_NAME)) + + with patch.object(ConceptFuzzySearch, 'search', side_effect=StopSearch) as search_mock: + with self.assertRaises(StopSearch): + self.match( + self.member, None, + {'owner': 'ResolveOrg', 'owner_type': 'org', 'source': 'ResolveSource', 'version': 'HEAD'}) + self.assertEqual( + search_mock.call_args[0][2], + {'owner': 'ResolveOrg', 'owner_type': 'Organization', 'source': 'ResolveSource', 'source_version': 'HEAD'}) + + def test_match_tells_apart_owners_whose_names_differ_only_in_case(self): + self.patch_concept_es_mapping_for_ci() + self.outsider.groups.add(Group.objects.get(name=PREVIEW_GROUP_NAME)) + own_org = OrganizationFactory(mnemonic='resolveorg', public_access=ACCESS_TYPE_NONE) + own_org.members.add(self.outsider) + own_source = OrganizationSourceFactory( + organization=own_org, mnemonic='ResolveSource', public_access=ACCESS_TYPE_NONE) + for source in [own_source, self.private_source]: + concept = ConceptFactory(parent=source, mnemonic='case-collision', public_access=ACCESS_TYPE_NONE) + source.concepts.add(concept.get_latest_version()) + ConceptDocument().update(Concept.objects.filter(mnemonic='case-collision'), refresh=True) + + for target_repo_url, target_repo in [ + (own_source.uri, None), + (None, {'owner': 'resolveorg', 'owner_type': 'Organization', 'source': 'ResolveSource'})]: + response = self.match( + self.outsider, target_repo_url, target_repo, row={'id': 'case-collision'}, + url='/concepts/$match/?limit=10') + + self.assertEqual(response.status_code, 200) + self.assertEqual( + {result['url'] for result in response.data[0]['results']}, + {'/orgs/resolveorg/sources/ResolveSource/concepts/case-collision/'}, + (target_repo_url, target_repo)) + def test_match_head_target_repo_with_bare_url_searches_head(self): # the HEAD version_url is the bare repo URL, which resolves to the latest released version OrganizationSourceFactory( @@ -116,6 +162,55 @@ def test_match_head_target_repo_with_bare_url_searches_head(self): self.assertEqual(search_mock.call_args[0][2]['source_version'], 'HEAD') +class RepoSearchOwnerCaseTest(SearchResolutionAccessBaseTest): + def test_repo_search_excludes_private_content_of_owner_whose_name_differs_only_in_case(self): + self.patch_concept_es_mapping_for_ci() + own_org = OrganizationFactory(mnemonic='resolveorg', public_access=ACCESS_TYPE_NONE) + own_org.members.add(self.outsider) + own_source = OrganizationSourceFactory( + organization=own_org, mnemonic='ResolveSource', public_access=ACCESS_TYPE_NONE) + for source in [own_source, self.private_source]: + concept = ConceptFactory(parent=source, mnemonic='case-twin', public_access=ACCESS_TYPE_NONE) + source.concepts.add(concept.get_latest_version()) + mapping = MappingFactory( + parent=source, from_concept=concept, to_concept=concept, public_access=ACCESS_TYPE_NONE) + source.mappings.add(mapping.get_latest_version()) + ConceptDocument().update(Concept.objects.filter(mnemonic='case-twin'), refresh=True) + MappingDocument().update(Mapping.objects.filter(parent__mnemonic='ResolveSource'), refresh=True) + + def search(resource): + response = self.client.get( + f'{own_source.uri}{resource}/?sortAsc=last_update&limit=100', + HTTP_AUTHORIZATION='Token ' + self.outsider.get_token()) + self.assertEqual(response.status_code, 200) + return {result['url'].split('/sources/')[0] for result in response.data} + + self.assertEqual(search('concepts'), {'/orgs/resolveorg'}) + # mappings have no exact-case owner field: with such an owner, only public mappings are searched + self.assertEqual(search('mappings'), set()) + + +class RepoListSearchOwnerCaseTest(SearchResolutionAccessBaseTest): + def test_repo_list_search_excludes_private_repos_of_owner_whose_name_differs_only_in_case(self): + own_org = OrganizationFactory(mnemonic='resolveorg', public_access=ACCESS_TYPE_NONE) + own_org.members.add(self.outsider) + own_source = OrganizationSourceFactory(organization=own_org, mnemonic='ResolveSource') + SourceDocument().update(Source.objects.filter(mnemonic='ResolveSource'), refresh=True) + + for url in ['/orgs/resolveorg/sources/?q=ResolveSource', '/user/orgs/sources/?q=ResolveSource']: + response = self.client.get(url, HTTP_AUTHORIZATION='Token ' + self.outsider.get_token()) + + self.assertEqual(response.status_code, 200) + self.assertEqual([source['url'] for source in response.data], [own_source.uri], url) + + def test_org_names_are_unique_regardless_of_case(self): + response = self.client.post( + '/orgs/', {'id': 'RESOLVEORG', 'name': 'Twin'}, HTTP_AUTHORIZATION='Token ' + self.outsider.get_token(), + format='json') + + self.assertEqual(response.status_code, 400) + + class ResolveReferenceAccessTest(SearchResolutionAccessBaseTest): def resolve(self, user): response = self.client.post( diff --git a/core/integration_tests/tests_users.py b/core/integration_tests/tests_users.py index b5406a63f..e12f56858 100644 --- a/core/integration_tests/tests_users.py +++ b/core/integration_tests/tests_users.py @@ -517,6 +517,49 @@ def test_get_staff_flag_filters_only_for_staff(self): self.assertEqual(response.status_code, 200) self.assertEqual([user['username'] for user in response.data], ['ocladmin']) + def test_get_extras_filter_matches_only_the_extras_field(self): + regular_user = UserProfileFactory(username='extrasregularuser', extras={'xdept': 'xcardiology'}) + member = UserProfileFactory(username='extrasprivatemember') + private_org = OrganizationFactory(mnemonic='extrasprivateorg', public_access=ACCESS_TYPE_NONE) + private_org.members.add(member) + UserProfileDocument().update([regular_user, member]) + headers = {'HTTP_AUTHORIZATION': 'Token ' + regular_user.get_token()} + + response = self.client.get('/users/', {'extras.xdept': 'cardio'}, format='json', **headers) + + self.assertEqual(response.status_code, 200) + self.assertEqual([user['username'] for user in response.data], ['extrasregularuser']) + + for value in ['zzqq OR is_staff:true OR zzqq', 'zzqq OR org:extrasprivateorg OR zzqq', + 'zzqq OR is_staff\\:true OR zzqq']: + response = self.client.get('/users/', {'extras.xdept': value}, format='json', **headers) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data, []) + + def test_get_last_login_filters_only_for_staff(self): + old_login_user = UserProfileFactory(username='oldloginuser', last_login='2020-01-01T00:00:00Z') + recent_login_user = UserProfileFactory(username='recentloginuser', last_login='2024-01-01T00:00:00Z') + all_users = ['ocladmin', 'oldloginuser', 'recentloginuser'] + + for headers in [{}, {'HTTP_AUTHORIZATION': 'Token ' + old_login_user.get_token()}]: + for filter_param in ['lastLoginSince=2022-01-01', 'lastLoginBefore=2022-01-01']: + response = self.client.get(f'/users/?{filter_param}', format='json', **headers) + + self.assertEqual(response.status_code, 200) + self.assertEqual(sorted(user['username'] for user in response.data), all_users) + + headers = {'HTTP_AUTHORIZATION': 'Token ' + self.superuser.get_token()} + response = self.client.get('/users/?lastLoginSince=2022-01-01', format='json', **headers) + + self.assertEqual(response.status_code, 200) + self.assertEqual([user['username'] for user in response.data], [recent_login_user.username]) + + response = self.client.get('/users/?lastLoginBefore=2022-01-01', format='json', **headers) + + self.assertEqual(response.status_code, 200) + self.assertEqual([user['username'] for user in response.data], [old_login_user.username]) + def test_get_verbose_plan_fields_only_for_self_and_staff(self): regular_user = UserProfileFactory(username='planfieldsregular') other_user = UserProfileFactory(username='planfieldsother') @@ -1270,6 +1313,45 @@ def test_get(self): self.assertEqual(response.status_code, 200) self.assertEqual(len(response.data), 0) + def test_get_only_follows_the_requester_can_view(self): + private_org = OrganizationFactory(mnemonic='private-followed', public_access=ACCESS_TYPE_NONE) + private_source = OrganizationSourceFactory(organization=private_org, public_access=ACCESS_TYPE_NONE) + public_org = OrganizationFactory(mnemonic='public-followed') + member = UserProfileFactory(username='private-member') + private_org.members.add(member) + outsider = UserProfileFactory(username='private-outsider') + staff = UserProfileFactory(username='private-staff', is_staff=True) + for followed in [private_org, private_source, public_org]: + member.follow(followed) + all_urls = sorted([private_org.url, private_source.url, public_org.url]) + + def get_followed_urls(url, user=None): + headers = {'HTTP_AUTHORIZATION': 'Token ' + user.get_token()} if user else {} + response = self.client.get(url, **headers) + self.assertEqual(response.status_code, 200) + follows = response.data + if url.startswith('/users/?'): + follows = next(user for user in follows if user['username'] == member.username)['following'] + elif 'includeFollowing' in url: + follows = follows['following'] + return sorted(follow['object']['url'] for follow in follows) + + following_url = f'/users/{member.username}/following/' + detail_url = f'/users/{member.username}/?includeFollowing=true' + list_url = '/users/?verbose=true&includeFollowing=true' + for url in [following_url, list_url]: + self.assertEqual(get_followed_urls(url), [public_org.url]) + for url in [following_url, detail_url, list_url]: + self.assertEqual(get_followed_urls(url, outsider), [public_org.url]) + self.assertEqual(get_followed_urls(url, member), all_urls) + self.assertEqual(get_followed_urls(url, staff), all_urls) + + # the user themselves too, once they lose access + private_org.members.remove(member) + for url in [following_url, detail_url, list_url]: + self.assertEqual(get_followed_urls(url, member), [public_org.url]) + self.assertEqual(get_followed_urls(url, staff), all_urls) + def test_post(self): follower = UserProfileFactory(username='follower') followed = UserProfileFactory(username='followed') diff --git a/core/mappings/serializers.py b/core/mappings/serializers.py index df6d17cd8..35dd4767f 100644 --- a/core/mappings/serializers.py +++ b/core/mappings/serializers.py @@ -8,7 +8,7 @@ INCLUDE_SOURCE_VERSIONS, INCLUDE_COLLECTION_VERSIONS, INCLUDE_VERBOSE_REFERENCES from core.common.fields import EncodedDecodedCharField from core.common.serializers import AbstractResourceSerializer, SourceChildPageSerializer, \ - SourceVersionFieldMixin, get_collection_in_scope + SourceVersionFieldMixin, CollectionVersionsMixin, get_collection_in_scope from core.common.utils import get_truthy_values from core.concepts.serializers import ConceptDetailSerializer from core.mappings.models import Mapping @@ -139,10 +139,10 @@ class Meta: ) -class MappingVersionListSerializer(MappingListSerializer): +class MappingVersionListSerializer(CollectionVersionsMixin, MappingListSerializer): previous_version_url = CharField(read_only=True, source='prev_version_uri') source_versions = ListField(read_only=True) - collection_versions = ListField(read_only=True) + collection_versions = SerializerMethodField() class Meta: model = Mapping @@ -317,10 +317,10 @@ def get_checksums(obj): return obj.get_checksums() -class MappingVersionDetailSerializer(MappingDetailSerializer): +class MappingVersionDetailSerializer(CollectionVersionsMixin, MappingDetailSerializer): previous_version_url = CharField(read_only=True, source='prev_version_uri') source_versions = ListField(read_only=True) - collection_versions = ListField(read_only=True) + collection_versions = SerializerMethodField() class Meta: model = Mapping diff --git a/core/mappings/views.py b/core/mappings/views.py index e02c8aef3..4a058a975 100644 --- a/core/mappings/views.py +++ b/core/mappings/views.py @@ -278,10 +278,11 @@ def get_queryset(self): instance = self.get_object() from core.collections.models import Collection - return Collection.objects.filter(id__in=instance.expansion_set.filter( + expansions = Mapping.filter_by_viewable_collection_version(instance.expansion_set.filter( collection_version__organization_id=instance.parent.organization_id, collection_version__user_id=instance.parent.user_id - ).values_list('collection_version_id', flat=True)) + ), self.request.user) + return Collection.objects.filter(id__in=expansions.values_list('collection_version_id', flat=True)) def get(self, request, *args, **kwargs): return self.list(request, *args, **kwargs) diff --git a/core/orgs/models.py b/core/orgs/models.py index fb0898d00..33caba85b 100644 --- a/core/orgs/models.py +++ b/core/orgs/models.py @@ -1,6 +1,7 @@ from django.contrib.contenttypes.fields import GenericRelation from django.core.validators import RegexValidator from django.db import models, transaction +from pydash import get from core.client_configs.models import ClientConfig from core.common.checksums import ChecksumModel @@ -79,6 +80,16 @@ def get_by_username(cls, username): def get_public(cls): return cls.objects.filter(public_access=ACCESS_TYPE_VIEW) + @classmethod + def get_visible_to(cls, user, queryset): + """Limits `queryset` to the orgs `user` can view: public orgs and the user's own. Staff can view all.""" + if get(user, 'is_superuser') or get(user, 'is_staff'): + return queryset + visible = cls.get_public() + if get(user, 'is_authenticated'): + visible = visible | cls.get_by_username(user.username) + return queryset.filter(id__in=visible.values('id')) + def save(self, *args, force_insert=False, force_update=False, using=None, update_fields=None): """ The creator becomes a member when the org is created. Later saves leave the members alone, so editing diff --git a/core/orgs/serializers.py b/core/orgs/serializers.py index c6002ecc3..b12f8db37 100644 --- a/core/orgs/serializers.py +++ b/core/orgs/serializers.py @@ -48,7 +48,7 @@ class Meta: def prepare_object(self, validated_data): user = self.context['request'].user mnemonic = validated_data.get('mnemonic', None) - if Organization.objects.filter(mnemonic=mnemonic).exists(): + if Organization.objects.filter(mnemonic__iexact=mnemonic).exists(): # unique regardless of case self._errors['mnemonic'] = f'Organization with mnemonic {mnemonic} already exists.' return Organization() organization = Organization(name=validated_data.get('name'), mnemonic=mnemonic) diff --git a/core/orgs/views.py b/core/orgs/views.py index e129a4eba..28175aab2 100644 --- a/core/orgs/views.py +++ b/core/orgs/views.py @@ -51,7 +51,7 @@ def get_queryset(self): username = get(self.request.user, 'username') if username: - self.queryset = self.get_visible(Organization.get_by_username(username)) + self.queryset = Organization.get_visible_to(self.request.user, Organization.get_by_username(username)) elif self.request.user.is_anonymous: self.queryset = Organization.get_public() elif self.request.user.is_superuser or self.request.user.is_staff: @@ -70,16 +70,6 @@ def get_queryset(self): return self.queryset.distinct() - def get_visible(self, queryset): - """Limits `queryset` to the orgs the requester can view: public orgs and the requester's own.""" - user = self.request.user - if user.is_superuser or user.is_staff: - return queryset - visible = Organization.get_public() - if user.is_authenticated: - visible = visible | Organization.get_by_username(user.username) - return queryset.filter(id__in=visible.values('id')) - def get_serializer_class(self): if self.request.method == 'GET' and self.is_verbose(): return OrganizationDetailSerializer diff --git a/core/pins/models.py b/core/pins/models.py index 742e97094..de0387178 100644 --- a/core/pins/models.py +++ b/core/pins/models.py @@ -73,6 +73,18 @@ def get_resource(cls, resource_type, resource_id): return None + @staticmethod + def get_visible_to(queryset, request): + """Limits `queryset` to the pins whose org (for org pins) and pinned resource the requester can view.""" + from core.common.permissions import CanViewConceptDictionary + permission = CanViewConceptDictionary() + + def can_view(obj): # a user pin has no org; a pin whose resource is gone shows no resource + return obj is None or permission.has_object_permission(request, None, obj) + + return queryset.filter( + id__in=[pin.id for pin in queryset if can_view(pin.organization) and can_view(pin.resource)]) + @property def parent(self): if self.organization_id: diff --git a/core/pins/permissions.py b/core/pins/permissions.py index 01788010f..a6db981f4 100644 --- a/core/pins/permissions.py +++ b/core/pins/permissions.py @@ -5,8 +5,8 @@ class CanEditPins(BasePermission): """ - Anyone can read pins. A user's pins can be changed by that user, an org's pins by its members, and any pin by - staff. Checked against the user or org the pins belong to. + Anyone can read the pins they can view (Pin.get_visible_to). A user's pins can be changed by that user, an + org's pins by its members, and any pin by staff. Checked against the user or org the pins belong to. """ def has_object_permission(self, request, view, obj): if request.method in SAFE_METHODS: diff --git a/core/pins/tests.py b/core/pins/tests.py index 6f86b7b7a..b8d9610b5 100644 --- a/core/pins/tests.py +++ b/core/pins/tests.py @@ -2,12 +2,14 @@ from mock import Mock from core.collections.tests.factories import OrganizationCollectionFactory +from core.common.constants import ACCESS_TYPE_NONE from core.common.tests import OCLTestCase, OCLAPITestCase from core.orgs.models import Organization from core.orgs.tests.factories import OrganizationFactory from core.pins.models import Pin from core.sources.models import Source from core.sources.tests.factories import OrganizationSourceFactory +from core.users.models import UserProfile from core.users.tests.factories import UserProfileFactory @@ -222,6 +224,57 @@ def test_get_200_with_creator_pins(self): sorted([source.uri, collection.uri]) ) + def test_get_200_only_pins_the_requester_can_view(self): + private_org = OrganizationFactory(public_access=ACCESS_TYPE_NONE) + public_org = OrganizationFactory() + owner = UserProfileFactory(organizations=[private_org, public_org]) + private_source = OrganizationSourceFactory(organization=private_org, public_access=ACCESS_TYPE_NONE) + public_source = OrganizationSourceFactory(organization=public_org) + private_user_pin = owner.pins.create(resource=private_source) + owner.pins.create(resource=public_source) + owner.pins.create(resource=private_org) + public_org.pins.create(resource=private_source) + public_org.pins.create(resource=public_source) + private_org.pins.create(resource=public_source) + admin = UserProfile.objects.get(username='ocladmin') + + def get_pin_uris(url, token=None): + headers = {'HTTP_AUTHORIZATION': 'Token ' + token} if token else {} + response = self.client.get(url, format='json', **headers) + self.assertEqual(response.status_code, 200) + return sorted(pin['resource_uri'] for pin in response.data) + + for token in [None, UserProfileFactory().get_token()]: + self.assertEqual(get_pin_uris(owner.uri + 'pins/', token), [public_source.uri]) + self.assertEqual(get_pin_uris(public_org.uri + 'pins/', token), [public_source.uri]) + self.assertEqual(get_pin_uris(private_org.uri + 'pins/', token), []) + headers = {'HTTP_AUTHORIZATION': 'Token ' + token} if token else {} + response = self.client.get( + owner.uri + f'pins/{private_user_pin.id}/', format='json', **headers) + self.assertEqual(response.status_code, 404) + + for token in [owner.get_token(), admin.get_token()]: + self.assertEqual( + get_pin_uris(owner.uri + 'pins/', token), + sorted([private_source.uri, public_source.uri, private_org.uri])) + self.assertEqual( + get_pin_uris(public_org.uri + 'pins/', token), sorted([private_source.uri, public_source.uri])) + self.assertEqual(get_pin_uris(private_org.uri + 'pins/', token), [public_source.uri]) + + private_org.members.remove(owner) + self.assertEqual(get_pin_uris(owner.uri + 'pins/', owner.get_token()), [public_source.uri]) + response = self.client.get( + owner.uri + '?includePins=true', HTTP_AUTHORIZATION='Token ' + owner.get_token(), format='json') + self.assertEqual([pin['resource_uri'] for pin in response.data['pins']], [public_source.uri]) + + # an update would return the resource, so it's refused; the owner can still delete the pin + pin_url = owner.uri + f'pins/{private_user_pin.id}/' + response = self.client.patch( + pin_url, {'order': 0}, HTTP_AUTHORIZATION='Token ' + owner.get_token(), format='json') + self.assertEqual(response.status_code, 404) + response = self.client.delete(pin_url, HTTP_AUTHORIZATION='Token ' + owner.get_token(), format='json') + self.assertEqual(response.status_code, 204) + def test_post_201(self): source = OrganizationSourceFactory() org = OrganizationFactory() diff --git a/core/pins/views.py b/core/pins/views.py index f4c8edd19..620da2a8d 100644 --- a/core/pins/views.py +++ b/core/pins/views.py @@ -60,9 +60,13 @@ def get_queryset(self): criteria = Q(**filters) if self.should_include_creator_pins() and 'org' not in self.kwargs: criteria |= Q(created_by_id=self.request.user.id, user_id__isnull=True) - return Pin.objects.filter( + queryset = Pin.objects.filter( criteria ).select_related('organization', 'user').prefetch_related('resource') + # an update returns the pinned resource, so only a pin's owner deleting it skips the visibility filter + if self.request.method != 'DELETE': + queryset = Pin.get_visible_to(queryset, self.request) + return queryset class PinListView(PinBaseView, ListAPIView): diff --git a/core/sources/models.py b/core/sources/models.py index bf1b09b8d..079a473ba 100644 --- a/core/sources/models.py +++ b/core/sources/models.py @@ -245,16 +245,18 @@ def update_mappings(self): from core.mappings.models import Mapping from core.mappings.documents import MappingDocument + # only mappings whose last saver can view this repo; others keep the external URL, as on save uris = self.identity_uris + can_view = self.get_view_access_criteria('updated_by') to_queryset = Mapping.objects.filter(to_source_url__in=uris) from_queryset = Mapping.objects.filter(from_source_url__in=uris) - to_queryset.filter(to_source_id__isnull=True).update(to_source=self) - from_queryset.filter(from_source_id__isnull=True).update(from_source=self) + to_queryset.filter(can_view, to_source_id__isnull=True).update(to_source=self) + from_queryset.filter(can_view, from_source_id__isnull=True).update(from_source=self) Mapping.batch_index(to_queryset.filter(to_source=self), MappingDocument) - Mapping.batch_index(from_queryset.filter(to_source=self), MappingDocument) + Mapping.batch_index(from_queryset.filter(from_source=self), MappingDocument) def is_hierarchy_root_belonging_to_self(self): hierarchy_root = self.hierarchy_root diff --git a/core/users/models.py b/core/users/models.py index f09337594..f2e729ac0 100644 --- a/core/users/models.py +++ b/core/users/models.py @@ -423,15 +423,29 @@ def is_member_of_org(self, org_mnemonic): def can_view(self, obj): """Whether this user can view a user, an org, a repo or content inside a repo.""" + return self.can_user_view(self, obj) + + @staticmethod + def can_user_view(user, obj): + """Whether `user` (anonymous or None for the public) can view a user, an org, a repo or content in a repo.""" from core.orgs.models import Organization - if self.is_staff or isinstance(obj, UserProfile): + if getattr(user, 'is_staff', False) or isinstance(obj, UserProfile): return True if isinstance(obj, Organization): - return obj.public_can_view or obj.is_member(self) + return obj.public_can_view or bool(user and user.is_authenticated and obj.is_member(user)) repo = obj if hasattr(obj, 'has_view_access') else ( getattr(obj, 'collection_version', None) or getattr(obj, 'collection', None) or getattr(obj, 'parent', None)) - return bool(repo and hasattr(repo, 'has_view_access') and repo.has_view_access(self)) + return bool(repo and hasattr(repo, 'has_view_access') and repo.has_view_access(user)) + + def get_following_visible_to(self, viewer): + """This user's follows, without those of objects `viewer` can't view (the user too, once they lose access). + Staff get them all.""" + follows = self.following.prefetch_related('following') + if getattr(viewer, 'is_staff', False): + return follows + hidden_ids = [follow.id for follow in follows if not self.can_user_view(viewer, follow.following)] + return follows.exclude(id__in=hidden_ids) if hidden_ids else follows def follow(self, following): self.following.create(following_id=following.id, following_type=ContentType.objects.get_for_model(following)) @@ -442,6 +456,7 @@ def follow(self, following): event_type=Event.FOLLOWED, object_url=self.url, referenced_object_url=following.url, + public=self.can_user_view(None, following), # private for objects the public can't view ) def unfollow(self, following): @@ -454,6 +469,7 @@ def unfollow(self, following): event_type=Event.UNFOLLOWED, object_url=self.url, referenced_object_url=following.url, + public=self.can_user_view(None, following), ) def set_groups(self, groups, verify=True, _save=True): diff --git a/core/users/serializers.py b/core/users/serializers.py index 2c9f5b9f4..d42e81468 100644 --- a/core/users/serializers.py +++ b/core/users/serializers.py @@ -18,7 +18,7 @@ TRUTHY = get_truthy_values() PRIVATE_USER_FIELDS = ( - 'email', 'last_login', 'is_staff', 'is_superuser', 'auth_groups', 'permissions', 'capabilities') + 'email', 'last_login', 'is_staff', 'is_superuser', 'auth_groups', 'permissions', 'capabilities', 'pins') def can_view_private_user_fields(request, user): @@ -30,7 +30,7 @@ def can_view_private_user_fields(request, user): class PrivateUserFieldsMixin: - """Email, account flags and plan fields are only for the user themselves and staff.""" + """Email, account flags, plan fields and pins are only for the user themselves and staff.""" def to_representation(self, instance): data = super().to_representation(instance) if not can_view_private_user_fields(get(self, 'context.request'), instance): @@ -204,7 +204,7 @@ class UserDetailSerializer(PrivateUserFieldsMixin, AbstractResourceSerializer): deactivated_at = serializers.DateTimeField(read_only=True) pins = serializers.SerializerMethodField() followers = FollowerSerializer(many=True, read_only=True) - following = FollowingSerializer(many=True, read_only=True) + following = serializers.SerializerMethodField() class Meta: model = UserProfile @@ -245,9 +245,12 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) def get_subscribed_orgs(self, obj): + """The user's orgs that the requester can view.""" if self.include_subscribed_orgs: + from core.orgs.models import Organization from core.orgs.serializers import OrganizationListSerializer - return OrganizationListSerializer(obj.organizations.all(), many=True).data + orgs = Organization.get_visible_to(get(self, 'context.request.user'), obj.organizations.all()) + return OrganizationListSerializer(orgs, many=True).data return None @@ -267,12 +270,19 @@ def get_capabilities(self, obj): return CapabilitySerializer(obj.capabilities, many=True, context={'user': obj}).data def get_pins(self, obj): - if self.include_pins: + request = get(self, 'context.request') + if self.include_pins and can_view_private_user_fields(request, obj): + from core.pins.models import Pin from core.pins.serializers import PinSerializer - return PinSerializer(obj.pins.prefetch_related('resource').all(), many=True).data + return PinSerializer( + Pin.get_visible_to(obj.pins.prefetch_related('resource').all(), request), many=True).data return None + def get_following(self, obj): + """Only follows of objects the requester can view.""" + return FollowingSerializer(obj.get_following_visible_to(get(self, 'context.request.user')), many=True).data + def validate_username(self, value): from core.services.auth.core import AuthService # Under SSO, Keycloak owns usernames and OIDC sign-in matches accounts on them, so no caller can change one. diff --git a/core/users/tests/tests.py b/core/users/tests/tests.py index b0e51976e..1be213874 100644 --- a/core/users/tests/tests.py +++ b/core/users/tests/tests.py @@ -12,6 +12,7 @@ from core.common.tasks import send_user_verification_email, send_user_reset_password_email from core.common.tests import OCLTestCase, OCLAPITestCase, PREVIEW_GROUP_NAME from core.orgs.models import Organization +from core.orgs.tests.factories import OrganizationFactory from core.sources.tests.factories import OrganizationSourceFactory from core.users.constants import USER_OBJECT_TYPE, OCL_SERVERS_GROUP, MAPPER_USE_PERMISSION, \ MAPPER_AI_ASSISTANT_PERMISSION, MAPPER_SCISPACY_PERMISSION, STAFF_GROUP, SUPERADMIN_GROUP @@ -304,6 +305,46 @@ def test_user_without_preview_group_has_no_mapper_permission(self): self.assertFalse(user.has_perm(MAPPER_USE_PERMISSION)) + def test_can_user_view(self): + from django.contrib.auth.models import AnonymousUser + from core.concepts.tests.factories import ConceptFactory + member, outsider, staff = UserProfileFactory(), UserProfileFactory(), UserProfileFactory(is_staff=True) + private_org = OrganizationFactory(public_access=ACCESS_TYPE_NONE) + private_org.members.add(member) + private_source = OrganizationSourceFactory(organization=private_org, public_access=ACCESS_TYPE_NONE) + private_concept = ConceptFactory(parent=private_source) + public_source = OrganizationSourceFactory(organization=private_org) + + for obj in [private_org, private_source, private_concept]: + for user in [None, AnonymousUser(), outsider]: + self.assertFalse(UserProfile.can_user_view(user, obj)) + self.assertFalse(outsider.can_view(obj)) + for user in [member, staff]: + self.assertTrue(UserProfile.can_user_view(user, obj)) + self.assertTrue(user.can_view(obj)) + for obj in [public_source, member]: + for user in [None, AnonymousUser(), outsider]: + self.assertTrue(UserProfile.can_user_view(user, obj)) + + def test_follow_events_of_private_objects_are_private(self): + follower = UserProfileFactory() + followed_user = UserProfileFactory() + private_source = OrganizationSourceFactory(public_access=ACCESS_TYPE_NONE) + public_source = OrganizationSourceFactory() + + for followed in [followed_user, private_source, public_source]: + follower.follow(followed) + follower.unfollow(followed) + + self.assertEqual( + sorted(follower.events.values_list('event_type', 'referenced_object_url', 'public')), + sorted([ + ('Followed', followed_user.url, True), ('Unfollowed', followed_user.url, True), + ('Followed', private_source.url, False), ('Unfollowed', private_source.url, False), + ('Followed', public_source.url, True), ('Unfollowed', public_source.url, True), + ]) + ) + class TokenAuthenticationViewTest(OCLAPITestCase): def test_post(self): @@ -769,6 +810,41 @@ def test_user_detail_plan_fields_only_for_self_and_staff(self): self.assertIn( {'name': 'mapper.match_operations', 'limit': 100, 'used': 0}, response.data['capabilities']) + def test_user_subscribed_orgs_and_pins_only_as_visible_to_requester(self): + public_org = OrganizationFactory(mnemonic='visorgspublic') + private_org = OrganizationFactory(mnemonic='visorgsprivate', public_access=ACCESS_TYPE_NONE) + user = UserProfileFactory(username='visorgsuser', organizations=[public_org, private_org]) + co_member = UserProfileFactory(username='visorgscomember', organizations=[private_org]) + other_user = UserProfileFactory(username='visorgsother') + private_source = OrganizationSourceFactory(organization=private_org, public_access=ACCESS_TYPE_NONE) + user.pins.create(resource=private_source) + params = 'includeSubscribedOrgs=true&includePins=true' + url = f'/users/{user.username}/?{params}' + + for requester, orgs in [(other_user, ['visorgspublic']), (co_member, ['visorgsprivate', 'visorgspublic'])]: + response = self.client.get(url, HTTP_AUTHORIZATION='Token ' + requester.get_token()) + + self.assertEqual(response.status_code, 200) + self.assertEqual(sorted(org['id'] for org in response.data['subscribed_orgs']), orgs) + self.assertNotIn('pins', response.data) + + for token in [user.get_token(), self.admin_token]: + response = self.client.get(url, HTTP_AUTHORIZATION='Token ' + token) + + self.assertEqual(response.status_code, 200) + self.assertEqual( + sorted(org['id'] for org in response.data['subscribed_orgs']), ['visorgsprivate', 'visorgspublic']) + self.assertEqual([pin['resource_uri'] for pin in response.data['pins']], [private_source.uri]) + + response = self.client.get(f'/users/?verbose=true&{params}') + + self.assertEqual(response.status_code, 200) + users = {user['username']: user for user in response.data} + self.assertEqual([org['id'] for org in users[user.username]['subscribed_orgs']], ['visorgspublic']) + self.assertEqual([org['id'] for org in users[co_member.username]['subscribed_orgs']], []) + for user_data in users.values(): + self.assertNotIn('pins', user_data) + @patch('core.users.models.UserProfile.get_all_permissions', return_value=set()) @patch('core.users.models.UserProfile.get_capability_usage', return_value=0) def test_user_detail_does_not_compute_plan_for_other_users(self, usage_mock, permissions_mock): diff --git a/core/users/views.py b/core/users/views.py index d5945f142..e4eb50c26 100644 --- a/core/users/views.py +++ b/core/users/views.py @@ -218,8 +218,10 @@ def get_faceted_filters(self, split=False, params=None, additional_fields=None, def get_queryset(self): updated_since = parse_updated_since_param(self.request.query_params) updated_by = self.request.query_params.get(UPDATED_BY_USERNAME_PARAM, None) - last_login_since = self.request.query_params.get(LAST_LOGIN_SINCE_PARAM, None) - last_login_before = self.request.query_params.get(LAST_LOGIN_BEFORE_PARAM, None) + last_login_since = last_login_before = None + if self.request.user.is_staff: # last login is staff-only account data + last_login_since = self.request.query_params.get(LAST_LOGIN_SINCE_PARAM, None) + last_login_before = self.request.query_params.get(LAST_LOGIN_BEFORE_PARAM, None) date_joined_since = self.request.query_params.get(DATE_JOINED_SINCE_PARAM, None) date_joined_before = self.request.query_params.get(DATE_JOINED_BEFORE_PARAM, None) if updated_since: @@ -620,7 +622,7 @@ class UserFollowingListView(AbstractFollowerFollowedView, ListAPIView): serializer_class = FollowingSerializer def get_queryset(self): - return self.get_object().following.filter() + return self.get_object().get_following_visible_to(self.request.user) def post(self, request, *args, **kwargs): # pylint: disable=unused-argument follower = self.get_object() diff --git a/core/value_sets/tests.py b/core/value_sets/tests.py index 7315b51b1..f68e35f21 100644 --- a/core/value_sets/tests.py +++ b/core/value_sets/tests.py @@ -1,7 +1,7 @@ from mock.mock import patch, Mock from core.collections.models import CollectionReference, Collection -from core.collections.tests.factories import OrganizationCollectionFactory, ExpansionFactory +from core.collections.tests.factories import OrganizationCollectionFactory, ExpansionFactory, UserCollectionFactory from core.common.constants import ACCESS_TYPE_NONE from core.common.tests import OCLAPITestCase from core.concepts.documents import ConceptDocument @@ -581,6 +581,72 @@ def test_validate_code_private_value_set_only_for_members_and_staff(self): self.assertEqual(response.status_code, 200, url) self.assertEqual(response.data['parameter'][0]['valueBoolean'], expected, (url, headers)) + def add_reference(self, collection, concept): + if not collection.expansion_uri: + collection.expansion_uri = ExpansionFactory(collection_version=collection).uri + collection.save() + collection.add_references([ + CollectionReference( + expression=concept.uri, collection=collection, code=concept.mnemonic, + system=concept.parent.uri, version='v2' + ), + ]) + + def test_validate_code_reads_only_value_sets_the_requester_can_view(self): + self.add_reference(self.collection, self.concept_1) + private_org = OrganizationFactory() + member = UserProfileFactory() + private_org.members.add(member) + private_collection = OrganizationCollectionFactory( + organization=private_org, mnemonic='priv', public_access=ACCESS_TYPE_NONE) + self.add_reference(private_collection, self.concept_2) + self.add_reference(OrganizationCollectionFactory(organization=private_org, mnemonic='pub'), self.concept_1) + owner = UserProfileFactory() + self.add_reference( + UserCollectionFactory(user=owner, mnemonic='upriv', public_access=ACCESS_TYPE_NONE), self.concept_2) + query = f'system=http://some/url&systemVersion={self.org_source_v2.version}&code=' + private_org_params = f'org={private_org.mnemonic}&collection=priv' + + for headers in [{}, {'HTTP_AUTHORIZATION': 'Token ' + self.user_token}]: + for url in [ + f'/orgs/{self.org.mnemonic}/ValueSet/c1/$validate-code/?{private_org_params}&{query}', + f'/orgs/{private_org.mnemonic}/ValueSet/pub/$validate-code/?collection=priv&{query}', + f'/fhir/ValueSet/upriv/$validate-code/?{query}', + f'/fhir/ValueSet/upriv/$validate-code/?user={owner.username}&{query}', + f'/fhir/ValueSet/c1/$validate-code/?user={owner.username}&collection=upriv&{query}', + ]: + response = self.client.get(url + self.concept_2.mnemonic, **headers) + self.assertEqual(response.status_code, 200, (url, headers)) + self.assertFalse(response.data['parameter'][0]['valueBoolean'], (url, headers)) + + # Without `url`, the global route checks the code system alone; other params don't pick a value set + for concept in [self.concept_1, self.concept_2]: + response = self.client.get( + f'/fhir/ValueSet/$validate-code/?{private_org_params}&{query}{concept.mnemonic}', **headers) + self.assertTrue(response.data['parameter'][0]['valueBoolean'], (concept.mnemonic, headers)) + + for url, headers in [ + (f'/orgs/{private_org.mnemonic}/ValueSet/priv/$validate-code/?{query}', member), + (f'/fhir/ValueSet/priv/$validate-code/?{query}', member), + (f'/fhir/ValueSet/upriv/$validate-code/?{query}', owner), + ]: + response = self.client.get( + url + self.concept_2.mnemonic, HTTP_AUTHORIZATION='Token ' + headers.get_token()) + self.assertTrue(response.data['parameter'][0]['valueBoolean'], url) + response = self.client.get(f'/fhir/ValueSet/c1/$validate-code/?{query}{self.concept_1.mnemonic}') + self.assertTrue(response.data['parameter'][0]['valueBoolean']) + + def test_validate_code_with_display_for_code_not_in_value_set(self): + self.add_reference(self.collection, self.concept_1) + + response = self.client.get( + f'/orgs/{self.org.mnemonic}/ValueSet/c1/$validate-code/' + f'?system=http://some/url&systemVersion={self.org_source_v2.version}&code=missing&display=x' + ) + + self.assertEqual(response.status_code, 200) + self.assertFalse(response.data['parameter'][0]['valueBoolean']) + def test_expand(self): self.client.post( f'/users/{self.user.mnemonic}/ValueSet/', diff --git a/core/value_sets/views.py b/core/value_sets/views.py index c9432c3be..b18bab53b 100644 --- a/core/value_sets/views.py +++ b/core/value_sets/views.py @@ -2,17 +2,16 @@ from django.core.exceptions import ValidationError from rest_framework import status -from rest_framework.exceptions import PermissionDenied from rest_framework.response import Response from core.bundles.serializers import FHIRBundleSerializer -from core.code_systems.views import CodeSystemValidateCodeView +from core.code_systems.views import CodeSystemValidateCodeView, get_viewable_repo from core.collections.models import Collection from core.collections.views import CollectionListView, CollectionRetrieveUpdateDestroyView, \ CollectionVersionExpansionsView -from core.common.constants import HEAD +from core.common.constants import HEAD, LATEST from core.common.fhir_helpers import translate_fhir_query -from core.concepts.views import ConceptRetrieveUpdateDestroyView +from core.concepts.models import Concept from core.sources.models import Source from core.value_sets.serializers import ValueSetDetailSerializer, \ ValueSetExpansionParametersSerializer, ValueSetExpansionSerializer @@ -52,32 +51,37 @@ def get_detail_serializer(self, obj): class ValueSetValidateCodeView(CodeSystemValidateCodeView): - INVALID_RESULT = {'parameter': [{'name': 'result', 'valueBoolean': False}]} - - def get_value_sets(self): - value_sets = [] - url = self.get_parameters().get('url') - if url: - value_sets.append(Collection.objects.filter( - canonical_url=url, is_latest_version=True).exclude(version=HEAD).first()) - collection = self.kwargs.get('collection') - if collection: - owner_filters = {'organization__mnemonic': self.kwargs['org']} if 'org' in self.kwargs else { - 'user__username': self.kwargs.get('user')} - value_sets.append(Collection.get_version(collection, self.kwargs.get('version', HEAD), owner_filters)) - return [value_set for value_set in value_sets if value_set] - - def get_object(self, queryset=None): - """A value set the requester can't view gives the same answer as a code that isn't in it.""" - if any(not value_set.has_view_access(self.request.user) for value_set in self.get_value_sets()): - return self.INVALID_RESULT - try: - return super().get_object(queryset) - except PermissionDenied: - return self.INVALID_RESULT + CONTAINER_PARAMS = ('user', 'org', 'source', 'collection', 'version') + + def get_route_value_set_versions(self): + """Ids of the route's collection versions the requester can view, narrowed by a `version` query param. The + owner-less route reads every collection with that id the requester can view.""" + user = self.request.user + heads = Collection.objects.filter( + mnemonic=self.kwargs['collection'], version=HEAD).select_related('organization') + if 'org' in self.kwargs: + heads = heads.filter(organization__mnemonic=self.kwargs['org']) + elif 'user' in self.kwargs: + heads = heads.filter(user__username=self.kwargs['user']) + version = self.request.query_params.get('version') + version_ids = [] + for head in heads: + has_private_access = bool(user.is_authenticated and head.has_parent_edit_access(user)) + if not has_private_access and not head.public_can_view: + continue + versions = head.versions.only('id', 'public_access') + if version == LATEST: + versions = versions.filter(released=True)[:1] + elif version: + versions = versions.filter(version=version) + version_ids += [repo.id for repo in versions if has_private_access or repo.public_can_view] + return version_ids def get_queryset(self): - queryset = super(ConceptRetrieveUpdateDestroyView, self).get_queryset() + """Reads only the value sets the request names (`url` and the route's collection), never ones other query + params pick; a value set that doesn't resolve or the requester can't view reads nothing.""" + params = {key: value for key, value in self.params.items() if key not in self.CONTAINER_PARAMS} + queryset = Concept.get_base_queryset(params) parameters = self.get_parameters() @@ -88,28 +92,33 @@ def get_queryset(self): system_version = parameters.get('systemVersion') if url: - collection = Collection.objects.filter(canonical_url=url).exclude(version=HEAD)\ - .filter(is_latest_version=True) + collection = get_viewable_repo( + Collection.objects.filter(canonical_url=url, is_latest_version=True).exclude(version=HEAD), + self.request.user) if not collection: return queryset.none() - queryset = queryset.filter(references__collection=collection.first()) + queryset = queryset.filter(references__collection=collection) + + if 'collection' in self.kwargs: + version_ids = self.get_route_value_set_versions() + if not version_ids: + return queryset.none() + queryset = queryset.filter(expansion_set__collection_version__in=version_ids) if code and system: concept_source = Source.objects.filter(canonical_url=system) - if not concept_source: - return queryset.none() if system_version: concept_source = concept_source.filter(version=system_version) else: concept_source = concept_source.filter(is_latest_version=True).exclude(version=HEAD) - if concept_source: - queryset = queryset.filter(sources=concept_source.first(), mnemonic=code) - else: + concept_source = get_viewable_repo(concept_source, self.request.user) + if not concept_source: return queryset.none() + queryset = queryset.filter(sources=concept_source, mnemonic=code) if display: instance = queryset.first() - if display not in (instance.name, instance.display_name): + if not instance or display not in (instance.name, instance.display_name): return queryset.none() return queryset