-
-
Notifications
You must be signed in to change notification settings - Fork 96
[feature] Added REST API endpoints for organization memberships #543 #555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
d22fa25
37b3fea
088d287
6a21b37
22c705c
9a0c2f6
2f69eb4
fb7b0fc
a4489e5
50a5348
36be96f
d06ea00
679eca1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -213,6 +213,40 @@ def to_internal_value(self, data): | |
| return super().to_internal_value(data) | ||
|
|
||
|
|
||
| class OrganizationMembershipSerializer(ValidatedModelSerializer): | ||
| exclude_validation = ("user", "organization") | ||
| id = serializers.UUIDField(read_only=True) | ||
| organization = OrgUserCustomPrimarykeyRelatedField() | ||
|
|
||
| class Meta: | ||
| model = OrganizationUser | ||
| fields = ("id", "organization", "is_admin", "created", "modified") | ||
|
|
||
| def validate(self, data): | ||
| data["user"] = self.context["user"] | ||
| if self.instance is None: | ||
| organization = data.get("organization") | ||
| if ( | ||
| organization is not None | ||
| and self.Meta.model.objects.filter( | ||
| user=data["user"], organization=organization | ||
| ).exists() | ||
| ): | ||
| raise serializers.ValidationError( | ||
| { | ||
| "organization": _( | ||
| "The user is already a member of this organization." | ||
|
BHARATH0153 marked this conversation as resolved.
|
||
| ) | ||
| } | ||
| ) | ||
| return super().validate(data) | ||
|
|
||
|
|
||
| class OrganizationMembershipDetailSerializer(OrganizationMembershipSerializer): | ||
| class Meta(OrganizationMembershipSerializer.Meta): | ||
| extra_kwargs = {"organization": {"read_only": True}} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The detail serializer does not make Please explicitly declare the detail field as read-only, or keep it writable and reject a value that differs from the URL organization. Add a regression test for an owner membership as well. Severity: P1 |
||
|
|
||
|
|
||
| class BaseSuperUserSerializer(ValidatedModelSerializer): | ||
| _skip_validation_fields = [ | ||
| "groups", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,9 @@ | |
| from django.urls import reverse | ||
| from django.utils.translation import gettext_lazy as _ | ||
| from drf_yasg.utils import swagger_auto_schema | ||
| from organizations.exceptions import OwnershipRequired | ||
| from rest_framework.authtoken.views import ObtainAuthToken | ||
| from rest_framework.exceptions import ValidationError | ||
| from rest_framework.generics import ( | ||
| GenericAPIView, | ||
| ListCreateAPIView, | ||
|
|
@@ -32,6 +34,8 @@ | |
| EmailAddressSerializer, | ||
| GroupSerializer, | ||
| OrganizationDetailSerializer, | ||
| OrganizationMembershipDetailSerializer, | ||
| OrganizationMembershipSerializer, | ||
| OrganizationSerializer, | ||
| PasswordChangeSerializer, | ||
| PasswordResetSerializer, | ||
|
|
@@ -331,6 +335,83 @@ def get_object(self): | |
| return obj | ||
|
|
||
|
|
||
| class BaseOrganizationMembershipView(ProtectedAPIMixin, FilterByParent, GenericAPIView): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The parent lookup and organization-scoping logic duplicates Please move the shared parent-scoping and serializer-context behavior into a common base or mixin, then reuse it from both endpoint families. Severity: P3 |
||
| model = OrganizationUser | ||
| serializer_class = OrganizationMembershipSerializer | ||
|
|
||
| def get_queryset(self): | ||
| if getattr(self, "swagger_fake_view", False): | ||
| return OrganizationUser.objects.none() | ||
| qs = OrganizationUser.objects.select_related("organization", "user") | ||
| if not self.request.user.is_superuser: | ||
| qs = qs.filter(organization_id__in=self.request.user.organizations_managed) | ||
| return qs | ||
|
|
||
| def initial(self, *args, **kwargs): | ||
| super().initial(*args, **kwargs) | ||
| self.assert_parent_exists() | ||
|
|
||
| def get_parent_queryset(self): | ||
| qs = User.objects.filter(pk=self.kwargs["pk"]) | ||
| if self.request.user.is_superuser: | ||
| return qs | ||
| return self.get_organization_queryset(qs) | ||
|
|
||
| def get_organization_queryset(self, qs): | ||
| orgs = self.request.user.organizations_managed | ||
| app_label = User._meta.app_config.label | ||
| filter_kwargs = { | ||
| "is_superuser": False, | ||
| f"{app_label}_organizationuser__organization_id__in": orgs, | ||
| } | ||
| return qs.filter(**filter_kwargs).distinct() | ||
|
BHARATH0153 marked this conversation as resolved.
|
||
|
|
||
| def get_serializer_context(self): | ||
| if getattr(self, "swagger_fake_view", False): | ||
| return None | ||
| context = super().get_serializer_context() | ||
| context["user"] = self.get_parent_queryset().first() | ||
| return context | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
|
|
||
| class OrganizationMembershipListCreateView( | ||
| BaseOrganizationMembershipView, ListCreateAPIView | ||
| ): | ||
| pagination_class = OpenWispPagination | ||
|
|
||
| def get_queryset(self): | ||
| if getattr(self, "swagger_fake_view", False): | ||
| return OrganizationUser.objects.none() | ||
| return super().get_queryset().filter(user_id=self.kwargs["pk"]) | ||
|
BHARATH0153 marked this conversation as resolved.
|
||
|
|
||
|
|
||
| class OrganizationMembershipDetailView( | ||
| BaseOrganizationMembershipView, RetrieveUpdateDestroyAPIView | ||
| ): | ||
| def get_serializer_class(self): | ||
| return OrganizationMembershipDetailSerializer | ||
|
|
||
| def get_object(self): | ||
| queryset = self.filter_queryset(self.get_queryset()) | ||
| queryset = queryset.filter(user_id=self.kwargs["pk"]) | ||
| filter_kwargs = { | ||
| "organization_id": self.kwargs["org_id"], | ||
| } | ||
| obj = get_object_or_404(queryset, **filter_kwargs) | ||
| self.check_object_permissions(self.request, obj) | ||
| return obj | ||
|
|
||
| def update(self, request, *args, **kwargs): | ||
| kwargs["partial"] = True | ||
| return super().update(request, *args, **kwargs) | ||
|
BHARATH0153 marked this conversation as resolved.
BHARATH0153 marked this conversation as resolved.
Comment on lines
+404
to
+406
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This turns Please restore standard DRF Severity: P2 |
||
|
|
||
| def destroy(self, request, *args, **kwargs): | ||
| try: | ||
| return super().destroy(request, *args, **kwargs) | ||
| except OwnershipRequired as error: | ||
| raise ValidationError(str(error)) | ||
|
|
||
|
|
||
| obtain_auth_token = ObtainAuthTokenView.as_view() | ||
| password_reset = PasswordResetView.as_view() | ||
| password_reset_confirm = PasswordResetConfirmView.as_view() | ||
|
|
@@ -344,3 +425,5 @@ def get_object(self): | |
| password_change = PasswordChangeView.as_view() | ||
| email_update = EmailUpdateView.as_view() | ||
| email_list = EmailListCreateView.as_view() | ||
| organization_membership_list = OrganizationMembershipListCreateView.as_view() | ||
| organization_membership_detail = OrganizationMembershipDetailView.as_view() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new API documentation lists the paths but omits their authorization rules. A client cannot tell that superusers have global access, while organization managers need the relevant model permission, are limited to organizations they manage, and cannot manage superusers.
Please add a concise note documenting these permission and scope restrictions.
Severity: P3