Skip to content

Commit

Permalink
Fix comments for PR 5892 (#6206)
Browse files Browse the repository at this point in the history
### Motivation and context
Applied comments:
- [x] Aligning ORGANIZATION_OPEN_API_PARAMETERS
#5892 (comment)
- [x] Moving ORGANIZATION_OPEN_API_PARAMETERS
#5892 (comment)
~~- [ ] Moving CustomerAutoSchema
#5892 (comment) this
uncritical comment that cannot be done easily, [see
answer](#5892 (comment))
- [x] Raise error if cannot get `organization_id` for objects
#5892 (comment)
- [x] Multiply fields for `iam_organization_field`
#5892 (comment)

Co-authored-by: Maxim Zhiltsov <zhiltsov.max35@gmail.com>
  • Loading branch information
Kirill Sizov and zhiltsov-max authored May 31, 2023
1 parent 21503b3 commit 2584b96
Show file tree
Hide file tree
Showing 18 changed files with 208 additions and 107 deletions.
4 changes: 2 additions & 2 deletions cvat/apps/engine/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -546,9 +546,9 @@ def create(cls, **kwargs):
@property
def organization_id(self):
if self.project is not None:
return self.project.organization.id
return self.project.organization_id
if self.task is not None:
return self.task.organization.id
return self.task.organization_id
return None

class Meta:
Expand Down
30 changes: 4 additions & 26 deletions cvat/apps/engine/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

import textwrap
from typing import Type
from rest_framework import serializers
from drf_spectacular.utils import OpenApiParameter

from drf_spectacular.extensions import OpenApiSerializerExtension
from drf_spectacular.plumbing import force_instance, build_basic_type
from drf_spectacular.types import OpenApiTypes
from drf_spectacular.plumbing import build_basic_type, force_instance
from drf_spectacular.serializers import PolymorphicProxySerializerExtension
from drf_spectacular.types import OpenApiTypes
from rest_framework import serializers


def _copy_serializer(
Expand Down Expand Up @@ -229,27 +229,5 @@ class CloudStorageReadSerializerExtension(_CloudStorageSerializerExtension):
class CloudStorageWriteSerializerExtension(_CloudStorageSerializerExtension):
target_class = 'cvat.apps.engine.serializers.CloudStorageWriteSerializer'

ORGANIZATION_OPEN_API_PARAMETERS = [
OpenApiParameter(
name='org',
type=str,
required=False,
location=OpenApiParameter.QUERY,
description="Organization unique slug",
),
OpenApiParameter(
name='org_id',
type=int,
required=False,
location=OpenApiParameter.QUERY,
description="Organization identifier",
),
OpenApiParameter(
name='X-Organization',
type=str,
required=False,
location=OpenApiParameter.HEADER
),
]

__all__ = [] # No public symbols here
7 changes: 2 additions & 5 deletions cvat/apps/engine/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
CloudStorageReadSerializer, DatasetFileSerializer,
ProjectFileSerializer, TaskFileSerializer, CloudStorageContentSerializer)
from cvat.apps.engine.view_utils import get_cloud_storage_for_import_or_export
from cvat.apps.engine.schema import ORGANIZATION_OPEN_API_PARAMETERS

from utils.dataset_manifest import ImageManifestManager
from cvat.apps.engine.utils import (
Expand All @@ -79,6 +78,7 @@
from cvat.apps.iam.permissions import (CloudStoragePermission,
CommentPermission, IssuePermission, JobPermission, LabelPermission, ProjectPermission,
TaskPermission, UserPermission)
from cvat.apps.iam.filters import ORGANIZATION_OPEN_API_PARAMETERS
from cvat.apps.engine.cache import MediaCache
from cvat.apps.events.handlers import handle_annotations_patch
from cvat.apps.engine.view_utils import tus_chunk_action
Expand Down Expand Up @@ -1824,10 +1824,7 @@ class LabelViewSet(viewsets.GenericViewSet, mixins.ListModelMixin,
'project__organization'
).all()

# NOTE: This filter works incorrectly for this view
# it requires task__organization OR project__organization check.
# Thus, we rely on permission-based filtering
iam_organization_field = None
iam_organization_field = ('task__organization', 'project__organization')

search_fields = ('name', 'parent')
filter_fields = list(search_fields) + ['id', 'type', 'color', 'parent_id']
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/events/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@


from cvat.apps.iam.permissions import EventsPermission
from cvat.apps.iam.filters import ORGANIZATION_OPEN_API_PARAMETERS
from cvat.apps.events.serializers import ClientEventsSerializer
from cvat.apps.engine.log import vlogger
from cvat.apps.engine.schema import ORGANIZATION_OPEN_API_PARAMETERS
from .export import export

class EventsViewSet(viewsets.ViewSet):
Expand Down
111 changes: 79 additions & 32 deletions cvat/apps/iam/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,65 @@
# SPDX-License-Identifier: MIT

from rest_framework.filters import BaseFilterBackend
from django.db.models import Q
from collections.abc import Iterable

from drf_spectacular.utils import OpenApiParameter

ORGANIZATION_OPEN_API_PARAMETERS = [
OpenApiParameter(
name='org',
type=str,
required=False,
location=OpenApiParameter.QUERY,
description="Organization unique slug",
),
OpenApiParameter(
name='org_id',
type=int,
required=False,
location=OpenApiParameter.QUERY,
description="Organization identifier",
),
OpenApiParameter(
name='X-Organization',
type=str,
required=False,
location=OpenApiParameter.HEADER,
description="Organization unique slug",
),
]

class OrganizationFilterBackend(BaseFilterBackend):
organization_slug = 'org'
organization_slug_description = 'Organization unique slug'
organization_id = 'org_id'
organization_id_description = 'Organization identifier'
organization_slug_header = 'X-Organization'

def _parameter_is_provided(self, request):
for parameter in ORGANIZATION_OPEN_API_PARAMETERS:
if parameter.location == 'header' and parameter.name in request.headers:
return True
elif parameter.location == 'query' and parameter.name in request.query_params:
return True
return False

def _construct_filter_query(self, organization_fields, org_id):
if isinstance(organization_fields, str):
return Q(**{organization_fields: org_id})

if isinstance(organization_fields, Iterable):
# we select all db records where AT LEAST ONE organization field is equal org_id
operation = Q.OR

if org_id is None:
# but to get all non-org objects we need select db records where ALL organization fields are None
operation = Q.AND

filter_query = Q()
for org_field in organization_fields:
filter_query.add(Q(**{org_field: org_id}), operation)

return filter_query

return Q()


def filter_queryset(self, request, queryset, view):
# Filter works only for "list" requests and allows to return
Expand All @@ -24,40 +76,35 @@ def filter_queryset(self, request, queryset, view):
if org:
visibility = {'organization': org.id}

elif not org and (
self.organization_slug in request.query_params
or self.organization_id in request.query_params
or self.organization_slug_header in request.headers
):
elif not org and self._parameter_is_provided(request):
visibility = {'organization': None}

if visibility:
visibility[view.iam_organization_field] = visibility.pop('organization')
return queryset.filter(**visibility).distinct()
org_id = visibility.pop("organization")
query = self._construct_filter_query(view.iam_organization_field, org_id)

return queryset.filter(query).distinct()

return queryset

def get_schema_operation_parameters(self, view):
if not view.iam_organization_field or view.detail:
return []

return [
{
'name': self.organization_slug,
'in': 'query',
'description': self.organization_slug_description,
'schema': {'type': 'string'},
},
{
'name': self.organization_id,
'in': 'query',
'description': self.organization_id_description,
'schema': {'type': 'integer'},
},
{
'name': self.organization_slug_header,
'in': 'header',
'description': self.organization_slug_description,
'schema': {'type': 'string'},
},
]
parameters = []
for parameter in ORGANIZATION_OPEN_API_PARAMETERS:
parameter_type = None

if parameter.type == int:
parameter_type = 'integer'
elif parameter.type == str:
parameter_type = 'string'

parameters.append({
'name': parameter.name,
'in': parameter.location,
'description': parameter.description,
'schema': {'type': parameter_type}
})

return parameters
30 changes: 21 additions & 9 deletions cvat/apps/iam/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@
from attrs import define, field
from django.conf import settings
from django.db.models import Q
from rest_framework.exceptions import ValidationError, PermissionDenied
from rest_framework.exceptions import PermissionDenied, ValidationError
from rest_framework.permissions import BasePermission

from cvat.apps.engine.models import (CloudStorage, Issue, Job, Label, Project,
Task)
from cvat.apps.organizations.models import Membership, Organization
from cvat.apps.engine.models import CloudStorage, Label, Project, Task, Job, Issue
from cvat.apps.webhooks.models import WebhookTypeChoice
from cvat.utils.http import make_requests_session

Expand Down Expand Up @@ -56,14 +57,24 @@ def get_organization(request, obj):
return obj

if obj:
if organization_id := getattr(obj, "organization_id", None):
try:
return Organization.objects.get(id=organization_id)
except Organization.DoesNotExist:
try:
organization_id = getattr(obj, 'organization_id')
except AttributeError as exc:
# Skip initialization of organization for those objects that don't related with organization
view = request.parser_context.get('view')
if view and view.basename in ('user', 'function', 'request'):
return None
return None

return request.iam_context["organization"]
raise exc

try:
return Organization.objects.get(id=organization_id)
except Organization.DoesNotExist:
return None



return request.iam_context['organization']

def get_membership(request, organization):
if organization is None:
Expand All @@ -80,12 +91,13 @@ def get_iam_context(request, obj):
membership = get_membership(request, organization)

if organization and not request.user.is_superuser and membership is None:
raise PermissionDenied({"message": "You should be an active member in the organization"})
raise PermissionDenied({'message': 'You should be an active member in the organization'})

return {
'user_id': request.user.id,
'group_name': request.iam_context['privilege'],
'org_id': getattr(organization, 'id', None),
'org_slug': getattr(organization, 'slug', None),
'org_owner_id': getattr(organization.owner, 'id', None)
if organization else None,
'org_role': getattr(membership, 'role', None),
Expand Down
6 changes: 4 additions & 2 deletions cvat/apps/iam/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@

import re
import textwrap
from drf_spectacular.openapi import AutoSchema

from drf_spectacular.authentication import SessionScheme, TokenScheme
from drf_spectacular.extensions import OpenApiAuthenticationExtension
from drf_spectacular.authentication import TokenScheme, SessionScheme
from drf_spectacular.openapi import AutoSchema


class SignatureAuthenticationScheme(OpenApiAuthenticationExtension):
Expand Down Expand Up @@ -95,3 +96,4 @@ def get_operation_id(self):
tokenized_path.append('formatted')

return '_'.join([tokenized_path[0]] + [action] + tokenized_path[1:])

2 changes: 1 addition & 1 deletion cvat/apps/lambda_manager/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@
from cvat.apps.engine.frame_provider import FrameProvider
from cvat.apps.engine.models import Job, ShapeType, SourceType, Task
from cvat.apps.engine.serializers import LabeledDataSerializer
from cvat.apps.engine.schema import ORGANIZATION_OPEN_API_PARAMETERS
from cvat.utils.http import make_requests_session
from cvat.apps.iam.permissions import LambdaPermission
from cvat.apps.iam.filters import ORGANIZATION_OPEN_API_PARAMETERS


class LambdaType(Enum):
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/organizations/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@

from drf_spectacular.utils import OpenApiResponse, extend_schema, extend_schema_view
from cvat.apps.engine.mixins import PartialUpdateModelMixin
from cvat.apps.engine.schema import ORGANIZATION_OPEN_API_PARAMETERS

from cvat.apps.iam.permissions import (
InvitationPermission, MembershipPermission, OrganizationPermission)
from cvat.apps.iam.filters import ORGANIZATION_OPEN_API_PARAMETERS
from .models import Invitation, Membership, Organization

from .serializers import (
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/webhooks/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
from rest_framework.permissions import SAFE_METHODS
from rest_framework.response import Response

from cvat.apps.engine.schema import ORGANIZATION_OPEN_API_PARAMETERS
from cvat.apps.engine.view_utils import list_action, make_paginated_response
from cvat.apps.iam.permissions import WebhookPermission
from cvat.apps.iam.filters import ORGANIZATION_OPEN_API_PARAMETERS

from .event_type import AllEvents, OrganizationEvents, ProjectEvents
from .models import Webhook, WebhookDelivery, WebhookTypeChoice
Expand Down
Loading

0 comments on commit 2584b96

Please sign in to comment.