diff --git a/kobo/apps/organizations/permissions.py b/kobo/apps/organizations/permissions.py index f37dc36757..02bfe7c1de 100644 --- a/kobo/apps/organizations/permissions.py +++ b/kobo/apps/organizations/permissions.py @@ -30,21 +30,31 @@ def has_object_permission(self, request, view, obj): class HasOrgRolePermission(IsOrgAdminPermission): + + def has_object_permission(self, request, view, obj): + if super().has_object_permission(request, view, obj): + return True + return request.method in permissions.SAFE_METHODS + + +class OrganizationNestedHasOrgRolePermission(HasOrgRolePermission): def has_permission(self, request, view): + if not super().has_permission(request, view): return False - organization = Organization.objects.filter( - id=view.kwargs.get('organization_id') - ).first() - if organization and not self.has_object_permission( - request, view, organization - ): - return False - return True + try: + organization = Organization.objects.get( + id=view.kwargs.get('organization_id') + ) + except Organization.DoesNotExist: + raise Http404 + + return super().has_object_permission(request, view, organization) def has_object_permission(self, request, view, obj): - obj = obj if isinstance(obj, Organization) else obj.organization - if super().has_object_permission(request, view, obj): - return True - return request.method in permissions.SAFE_METHODS + """ + The object check is always performed on the parent (organization) and + is validated in `has_permission()`. Therefore, this method always returns True. + """ + return True diff --git a/kobo/apps/organizations/tests/test_organizations_api.py b/kobo/apps/organizations/tests/test_organizations_api.py index 1761b96e8b..8fd6e09eaa 100644 --- a/kobo/apps/organizations/tests/test_organizations_api.py +++ b/kobo/apps/organizations/tests/test_organizations_api.py @@ -169,6 +169,70 @@ def test_api_response_includes_is_mmo_with_override_and_subscription( self.assertEqual(response.data['is_mmo'], True) +@ddt +class OrganizationDetailAPITestCase(BaseTestCase): + + fixtures = ['test_data'] + URL_NAMESPACE = URL_NAMESPACE + + def setUp(self): + self.someuser = User.objects.get(username='someuser') + self.organization = self.someuser.organization + self.organization.mmo_override = True + self.organization.save(update_fields=['mmo_override']) + + # anotheruser is an admin of someuser's organization + self.anotheruser = User.objects.get(username='anotheruser') + self.organization.add_user(self.anotheruser, is_admin=True) + + # alice is a regular member of someuser's organization + self.alice = User.objects.create_user( + username='alice', password='alice', email='alice@alice.com' + ) + self.organization.add_user(self.alice, is_admin=False) + + # bob is external to someuser's organization + self.bob = User.objects.create_user( + username='bob', password='bob', email='bob@bob.com' + ) + + @data( + ('someuser', status.HTTP_200_OK), + ('anotheruser', status.HTTP_200_OK), + ('alice', status.HTTP_403_FORBIDDEN), + ('bob', status.HTTP_404_NOT_FOUND), + ) + @unpack + def test_asset_usage(self, username, expected_status_code): + user = User.objects.get(username=username) + self.client.force_login(user) + + url = reverse( + self._get_endpoint('organizations-asset-usage'), + kwargs={'id': self.organization.id} + ) + response = self.client.get(url) + assert response.status_code == expected_status_code + + @data( + ('someuser', status.HTTP_200_OK), + ('anotheruser', status.HTTP_200_OK), + ('alice', status.HTTP_200_OK), + ('bob', status.HTTP_404_NOT_FOUND), + ) + @unpack + def test_service_usage(self, username, expected_status_code): + user = User.objects.get(username=username) + self.client.force_login(user) + + url = reverse( + self._get_endpoint('organizations-service-usage'), + kwargs={'id': self.organization.id} + ) + response = self.client.get(url) + assert response.status_code == expected_status_code + + class BaseOrganizationAssetApiTestCase(BaseAssetTestCase): """ This test suite (e.g. classes which inherit from this one) does not cover diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py index b8677f7b97..f45950fa57 100644 --- a/kobo/apps/organizations/views.py +++ b/kobo/apps/organizations/views.py @@ -1,5 +1,3 @@ -from django.conf import settings -from django.contrib.postgres.aggregates import ArrayAgg from django.db.models import ( QuerySet, Case, @@ -10,7 +8,7 @@ ) from django.db.models.expressions import Exists from django.utils.http import http_date -from rest_framework import status, viewsets +from rest_framework import viewsets from rest_framework.decorators import action from rest_framework.request import Request from rest_framework.response import Response @@ -26,10 +24,13 @@ from kpi.utils.object_permission import get_database_user from kpi.views.v2.asset import AssetViewSet from .models import Organization, OrganizationOwner, OrganizationUser -from .permissions import HasOrgRolePermission, IsOrgAdminPermission +from .permissions import ( + HasOrgRolePermission, + IsOrgAdminPermission, + OrganizationNestedHasOrgRolePermission, +) from .serializers import OrganizationSerializer, OrganizationUserSerializer from ..accounts.mfa.models import MfaMethod -from ..stripe.constants import ACTIVE_STRIPE_STATUSES class OrganizationAssetViewSet(AssetViewSet): @@ -74,7 +75,7 @@ class OrganizationViewSet(viewsets.ModelViewSet): """ Organizations are groups of users with assigned permissions and configurations - - Organization admins can manage the organization and it's membership + - Organization admins can manage the organization and its membership - Connect to authentication mechanisms and enforce policy - Create teams and projects under the organization """ @@ -153,16 +154,13 @@ def service_usage(self, request, pk=None, *args, **kwargs): > } ### CURRENT ENDPOINT """ - - context = { - 'organization_id': kwargs.get('id', None), - **self.get_serializer_context(), - } + self.get_object() # check permissions serializer = ServiceUsageSerializer( get_database_user(request.user), - context=context, + context=self.get_serializer_context(), ) + response = Response( data=serializer.data, headers={ @@ -172,7 +170,7 @@ def service_usage(self, request, pk=None, *args, **kwargs): return response - @action(detail=True, methods=['get']) + @action(detail=True, methods=['get'], permission_classes=[IsOrgAdminPermission]) def asset_usage(self, request, pk=None, *args, **kwargs): """ ## Organization Asset Usage Tracker @@ -212,31 +210,11 @@ def asset_usage(self, request, pk=None, *args, **kwargs): ### CURRENT ENDPOINT """ - org_id = kwargs.get('id', None) - # Check if the organization exists and if the user is the owner - try: - organization = Organization.objects.prefetch_related('organization_users__user').filter( - id=org_id, owner__organization_user__user_id=request.user.id, - ).annotate( - user_ids=ArrayAgg('organization_users__user_id') - )[0] - except IndexError: - return Response( - {'error': 'There was a problem finding the organization.'}, - status=status.HTTP_400_BAD_REQUEST, - ) + organization = self.get_object() - # default to showing all users for this org - asset_users = organization.user_ids - # if Stripe is enabled, check that the org is on a plan that supports Organization features - if settings.STRIPE_ENABLED and not Organization.objects.filter( - id=org_id, - djstripe_customers__subscriptions__status__in=ACTIVE_STRIPE_STATUSES, - djstripe_customers__subscriptions__items__price__product__metadata__has_key='plan_type', - djstripe_customers__subscriptions__items__price__product__metadata__plan_type='enterprise', - ).exists(): - # No active subscription that supports multiple users, just get this user's data - asset_users = [request.user.id] + user_id = get_database_user(request.user).pk + if organization.is_mmo: + user_id = organization.owner_user_object.pk assets = ( Asset.objects.only( @@ -247,7 +225,7 @@ def asset_usage(self, request, pk=None, *args, **kwargs): ) .select_related('owner') .filter( - owner__in=asset_users, + owner_id=user_id, asset_type=ASSET_TYPE_SURVEY, ) ) @@ -418,7 +396,7 @@ class OrganizationMemberViewSet(viewsets.ModelViewSet): in updates. """ serializer_class = OrganizationUserSerializer - permission_classes = [HasOrgRolePermission] + permission_classes = [OrganizationNestedHasOrgRolePermission] http_method_names = ['get', 'patch', 'delete'] lookup_field = 'user__username' @@ -437,7 +415,7 @@ def get_queryset(self): organization_user=OuterRef('pk') ).values('pk') - # Annotate with role based on organization ownership and admin status + # Annotate with the role based on organization ownership and admin status queryset = OrganizationUser.objects.filter( organization_id=organization_id ).select_related('user__extra_details').annotate( diff --git a/kobo/apps/stripe/tests/test_organization_usage.py b/kobo/apps/stripe/tests/test_organization_usage.py index a349361bff..ee622c45a3 100644 --- a/kobo/apps/stripe/tests/test_organization_usage.py +++ b/kobo/apps/stripe/tests/test_organization_usage.py @@ -83,19 +83,6 @@ def setUp(self): def tearDown(self): cache.clear() - def test_usage_doesnt_include_org_users_without_subscription(self): - """ - Test that the endpoint *only* returns usage for the logged-in user - if they don't have a subscription that includes Organizations. - """ - response = self.client.get(self.detail_url) - # without a plan, the user should only see their usage - assert response.data['total_submission_count']['all_time'] == self.expected_submissions_single - assert response.data['total_submission_count']['current_month'] == self.expected_submissions_single - assert response.data['total_storage_bytes'] == ( - self.expected_file_size() * self.expected_submissions_single - ) - def test_usage_for_plans_with_org_access(self): """ Test that the endpoint aggregates usage for each user in the organization @@ -112,22 +99,6 @@ def test_usage_for_plans_with_org_access(self): self.expected_file_size() * self.expected_submissions_multi ) - def test_doesnt_include_org_users_with_invalid_plan(self): - """ - Test that the endpoint *doesn't* aggregates usage for the organization - when subscribed to a product that doesn't include org access - """ - - generate_plan_subscription(self.organization) - - response = self.client.get(self.detail_url) - # without the proper subscription, the user should only see their usage - assert response.data['total_submission_count']['current_month'] == self.expected_submissions_single - assert response.data['total_submission_count']['all_time'] == self.expected_submissions_single - assert response.data['total_storage_bytes'] == ( - self.expected_file_size() * self.expected_submissions_single - ) - @pytest.mark.performance def test_endpoint_speed(self): # get the average request time for 10 hits to the endpoint @@ -425,12 +396,12 @@ def test_nonexistent_organization(self): non_org_id = 'lkdjalkfewkl' url_with_non_org_id = f'{self.url}{non_org_id}/asset_usage/' response = self.client.get(url_with_non_org_id) - assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.status_code == status.HTTP_404_NOT_FOUND def test_user_not_member_of_organization(self): self.client.force_login(self.someuser) response = self.client.get(self.detail_url) - assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.status_code == status.HTTP_404_NOT_FOUND def test_successful_retrieval(self): generate_mmo_subscription(self.organization) diff --git a/kobo/apps/trackers/tests/submission_utils.py b/kobo/apps/trackers/tests/submission_utils.py index 91c1d2f288..21aa89c17b 100644 --- a/kobo/apps/trackers/tests/submission_utils.py +++ b/kobo/apps/trackers/tests/submission_utils.py @@ -38,10 +38,11 @@ def _get_uid(count): return uids for idx, user in enumerate(users): + real_owner = user.organization.owner_user_object assets = assets + baker.make( Asset, content=content_source_asset, - owner=user, + owner=real_owner, asset_type='survey', name='test', uid=iter(_get_uid(assets_per_user)), diff --git a/kobo/apps/trackers/utils.py b/kobo/apps/trackers/utils.py index 1e7325127f..1c70ac1f3f 100644 --- a/kobo/apps/trackers/utils.py +++ b/kobo/apps/trackers/utils.py @@ -71,7 +71,7 @@ def get_organization_usage(organization: Organization, usage_type: UsageType) -> Get the used amount for a given organization and usage type """ usage_calc = ServiceUsageCalculator( - organization.owner.organization_user.user, organization, disable_cache=True + organization.owner.organization_user.user, disable_cache=True ) usage = usage_calc.get_nlp_usage_by_type(USAGE_LIMIT_MAP[usage_type]) diff --git a/kpi/filters.py b/kpi/filters.py index 81bcd96ca1..a54cb47461 100644 --- a/kpi/filters.py +++ b/kpi/filters.py @@ -433,6 +433,7 @@ class SearchFilter(filters.BaseFilterBackend): """ def filter_queryset(self, request, queryset, view): + try: q = request.query_params['q'] except AttributeError: diff --git a/kpi/serializers/v2/service_usage.py b/kpi/serializers/v2/service_usage.py index 36adab02ff..f5694a8c5c 100644 --- a/kpi/serializers/v2/service_usage.py +++ b/kpi/serializers/v2/service_usage.py @@ -1,7 +1,6 @@ from rest_framework import serializers from rest_framework.fields import empty -from kobo.apps.organizations.models import Organization from kobo.apps.organizations.utils import ( get_monthly_billing_dates, get_yearly_billing_dates, @@ -111,14 +110,7 @@ class ServiceUsageSerializer(serializers.Serializer): def __init__(self, instance=None, data=empty, **kwargs): super().__init__(instance=instance, data=data, **kwargs) - organization = None - organization_id = self.context.get('organization_id', None) - if organization_id: - organization = Organization.objects.filter( - organization_users__user_id=instance.id, - id=organization_id, - ).first() - self.calculator = ServiceUsageCalculator(instance, organization) + self.calculator = ServiceUsageCalculator(user=instance) def get_current_month_end(self, user): return self.calculator.current_month_end.isoformat() diff --git a/kpi/tests/test_usage_calculator.py b/kpi/tests/test_usage_calculator.py index 82c3afc6f7..6b4e3c2e57 100644 --- a/kpi/tests/test_usage_calculator.py +++ b/kpi/tests/test_usage_calculator.py @@ -162,7 +162,7 @@ def setUp(self): self.add_submissions(count=5) def test_disable_cache(self): - calculator = ServiceUsageCalculator(self.anotheruser, None, disable_cache=True) + calculator = ServiceUsageCalculator(self.anotheruser, disable_cache=True) nlp_usage_A = calculator.get_nlp_usage_counters() self.add_nlp_trackers() nlp_usage_B = calculator.get_nlp_usage_counters() @@ -176,7 +176,7 @@ def test_disable_cache(self): ) def test_nlp_usage_counters(self): - calculator = ServiceUsageCalculator(self.anotheruser, None) + calculator = ServiceUsageCalculator(self.anotheruser) nlp_usage = calculator.get_nlp_usage_counters() assert nlp_usage['asr_seconds_current_month'] == 4586 assert nlp_usage['asr_seconds_all_time'] == 4728 @@ -184,7 +184,7 @@ def test_nlp_usage_counters(self): assert nlp_usage['mt_characters_all_time'] == 6726 def test_no_data(self): - calculator = ServiceUsageCalculator(self.someuser, None) + calculator = ServiceUsageCalculator(self.someuser) nlp_usage = calculator.get_nlp_usage_counters() submission_counters = calculator.get_submission_counters() @@ -203,7 +203,7 @@ def test_organization_setup(self): organization.add_user(user=self.someuser, is_admin=True) generate_mmo_subscription(organization) - calculator = ServiceUsageCalculator(self.someuser, organization) + calculator = ServiceUsageCalculator(self.someuser) submission_counters = calculator.get_submission_counters() assert submission_counters['current_month'] == 5 assert submission_counters['all_time'] == 5 @@ -220,11 +220,11 @@ def test_organization_setup(self): assert calculator.get_nlp_usage_by_type(USAGE_LIMIT_MAP['seconds']) == 4586 def test_storage_usage(self): - calculator = ServiceUsageCalculator(self.anotheruser, None) + calculator = ServiceUsageCalculator(self.anotheruser) assert calculator.get_storage_usage() == 5 * self.expected_file_size() def test_submission_counters(self): - calculator = ServiceUsageCalculator(self.anotheruser, None) + calculator = ServiceUsageCalculator(self.anotheruser) submission_counters = calculator.get_submission_counters() assert submission_counters['current_month'] == 5 assert submission_counters['all_time'] == 5 diff --git a/kpi/utils/usage_calculator.py b/kpi/utils/usage_calculator.py index 12ea46b8dc..8e1d6f9566 100644 --- a/kpi/utils/usage_calculator.py +++ b/kpi/utils/usage_calculator.py @@ -1,5 +1,4 @@ from json import dumps, loads -from typing import Optional from django.apps import apps from django.conf import settings @@ -13,7 +12,6 @@ get_monthly_billing_dates, get_yearly_billing_dates, ) -from kobo.apps.stripe.constants import ACTIVE_STRIPE_STATUSES from kpi.utils.cache import CachedClass, cached_class_property @@ -23,38 +21,21 @@ class ServiceUsageCalculator(CachedClass): def __init__( self, user: User, - organization: Optional['Organization'], disable_cache: bool = False, ): self.user = user - self.organization = organization self._cache_available = not disable_cache - self._user_ids = [user.pk] - self._user_id_query = self._filter_by_user([user.pk]) - if organization and settings.STRIPE_ENABLED: - # If the user is in an organization and has an enterprise plan, get all org - # users we evaluate this queryset instead of using it as a subquery. It's - # referencing fields from the auth_user tables on kpi *and* kobocat, - # making getting results in a single query not feasible until those tables - # are combined. - user_ids = list( - User.objects.filter( - organizations_organization__id=organization.id, - organizations_organization__djstripe_customers__subscriptions__status__in=ACTIVE_STRIPE_STATUSES, # noqa: E501 - organizations_organization__djstripe_customers__subscriptions__items__price__product__metadata__has_key='plan_type', # noqa: E501 - organizations_organization__djstripe_customers__subscriptions__items__price__product__metadata__plan_type='enterprise', # noqa: E501 - ).values_list('pk', flat=True)[: settings.ORGANIZATION_USER_LIMIT] - ) - if user_ids: - self._user_ids = user_ids - self._user_id_query = self._filter_by_user(user_ids) + self._user_id = user.pk + self.organization = user.organization + if self.organization.is_mmo: + self._user_id = self.organization.owner_user_object.pk now = timezone.now() self.current_month_start, self.current_month_end = get_monthly_billing_dates( - organization + self.organization ) self.current_year_start, self.current_year_end = get_yearly_billing_dates( - organization + self.organization ) self.current_month_filter = Q(date__range=[self.current_month_start, now]) self.current_year_filter = Q(date__range=[self.current_year_start, now]) @@ -94,7 +75,7 @@ def get_nlp_usage_counters(self): NLPUsageCounter.objects.only( 'date', 'total_asr_seconds', 'total_mt_characters' ) - .filter(self._user_id_query) + .filter(user_id=self._user_id) .aggregate( asr_seconds_current_year=Coalesce( Sum('total_asr_seconds', filter=self.current_year_filter), 0 @@ -132,7 +113,7 @@ def get_storage_usage(self): xforms = ( XForm.objects.only('attachment_storage_bytes', 'id') .exclude(pending_delete=True) - .filter(self._user_id_query) + .filter(user_id=self._user_id) ) total_storage_bytes = xforms.aggregate( @@ -152,7 +133,7 @@ def get_submission_counters(self): """ submission_count = ( DailyXFormSubmissionCounter.objects.only('counter', 'user_id') - .filter(self._user_id_query) + .filter(user_id=self._user_id) .aggregate( all_time=Coalesce(Sum('counter'), 0), current_year=Coalesce( @@ -175,9 +156,3 @@ def _get_cache_hash(self): return f'user-{self.user.id}' else: return f'organization-{self.organization.id}' - - def _filter_by_user(self, user_ids: list) -> Q: - """ - Turns a list of user ids into a query object to filter by - """ - return Q(user_id__in=user_ids) diff --git a/kpi/views/v2/service_usage.py b/kpi/views/v2/service_usage.py index ef3a16b925..40b9533e34 100644 --- a/kpi/views/v2/service_usage.py +++ b/kpi/views/v2/service_usage.py @@ -1,4 +1,3 @@ -# coding: utf-8 from rest_framework import ( renderers, viewsets, @@ -12,7 +11,9 @@ class ServiceUsageViewSet(viewsets.GenericViewSet): """ + ⚠️ Deprecated ## Service Usage Tracker +

Tracks the total usage of different services for the logged-in user

Tracks the submissions and NLP seconds/characters for the current month/year/all time

Tracks the current total storage used