From d445f511d85bdb9b08deb89d8d61b45a810f3805 Mon Sep 17 00:00:00 2001 From: hoang Date: Mon, 28 Sep 2026 12:24:50 +0700 Subject: [PATCH] Fix missing validations for owner in Permission APIs We missed validating the assignment or removal of owner permissions via APIs, unlike the checks in Promgen Web. To address this, we've created validators in validators.py and ensured both Promgen Web and Promgen APIs use them. Consequently, the existing similar validations in views.py will be removed. --- promgen/rest_v2.py | 4 ++-- promgen/validators.py | 17 +++++++++++++++++ promgen/views.py | 25 ++++++++++--------------- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/promgen/rest_v2.py b/promgen/rest_v2.py index c7005c3c9..062c65abe 100644 --- a/promgen/rest_v2.py +++ b/promgen/rest_v2.py @@ -206,10 +206,9 @@ def assign_user(self, request, id): serializer.is_valid(raise_exception=True) user = User.objects.get(id=serializer.validated_data["id"]) - if not user.is_active: - raise ValidationError({"detail": "Cannot assign permissions to an inactive user."}) content_type = ContentType.objects.get_for_model(object) permission = content_type.model + "_" + serializer.validated_data["role"].lower() + validators.validate_assign_perm(user, object, permission) user_object_perm = assign_perm(permission, user, object) return Response( serializers.UserObjectPermissionSerializer(user_object_perm).data, @@ -285,6 +284,7 @@ def remove_user(self, request, id, user_id): request.query_params.get("remove_sub_permissions", "true").lower() == "true" ) user = User.objects.get(id=user_id) + validators.validate_remove_perm(user, self.get_object()) self.remove_perm(user, remove_sub_permissions) return Response(status=HTTPStatus.NO_CONTENT) diff --git a/promgen/validators.py b/promgen/validators.py index 89758ce6b..ebecfe1c6 100644 --- a/promgen/validators.py +++ b/promgen/validators.py @@ -6,6 +6,7 @@ from dateutil import parser from django.core.exceptions import ValidationError from django.core.validators import RegexValidator, URLValidator +from django.utils.translation import gettext as _ # See definition of duration field # https://prometheus.io/docs/prometheus/latest/configuration/configuration/#configuration-file @@ -102,3 +103,19 @@ def validate_utf8(value): value.encode("utf-8").decode("utf-8") except (UnicodeEncodeError, UnicodeDecodeError): raise ValidationError("Invalid UTF-8 string.") + + +def validate_assign_perm(user, object, permission): + if not user.is_active: + raise ValidationError(_("Cannot assign permissions to an inactive user.")) + if user == object.owner and permission not in ["service_admin", "project_admin"]: + raise ValidationError( + _("Cannot assign permission for the owner. The owner must have the ADMIN role.") + ) + + +def validate_remove_perm(user, object): + if user == object.owner: + raise ValidationError( + _("Cannot remove permissions for the owner. Please transfer ownership first.") + ) diff --git a/promgen/views.py b/promgen/views.py index d63e8d16b..08cdd03fc 100644 --- a/promgen/views.py +++ b/promgen/views.py @@ -17,6 +17,7 @@ from django.contrib.auth.mixins import LoginRequiredMixin from django.contrib.auth.models import User from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ValidationError from django.core.paginator import EmptyPage, Paginator from django.db import transaction from django.db.models import Count, Prefetch, Q @@ -52,6 +53,7 @@ from promgen.forms import GroupMemberForm, UserPermissionForm from promgen.mixins import PromgenGuardianPermissionMixin from promgen.shortcuts import resolve_domain +from promgen.validators import validate_assign_perm, validate_remove_perm logger = logging.getLogger(__name__) @@ -1956,15 +1958,10 @@ def post(self, request): if "user" == permission_type: user = User.objects.get_by_natural_key(request.POST["username"]) - # Prevent changing permissions for the owner of the object - if user == obj.owner and request.POST["permission"] not in self.permission_required: - messages.warning( - request, - _( - "Cannot assign permission for the owner. " - "The owner must have the ADMIN role." - ), - ) + try: + validate_assign_perm(user, obj, permission) + except ValidationError as e: + messages.error(request, e.message) return redirect(request.POST["next"]) assign_perm(permission, user, obj) @@ -2006,12 +2003,10 @@ def post(self, request): if "user" == permission_type: user = User.objects.get_by_natural_key(request.POST["username"]) - # Prevent removing permissions for the owner of the object - if user == obj.owner: - messages.warning( - request, - _("Cannot remove permissions for the owner. Please transfer ownership first."), - ) + try: + validate_remove_perm(user, obj) + except ValidationError as e: + messages.error(request, e.message) return redirect(request.POST["next"]) self.delete_perm(user)