From 84ebe5aa299c8a77e1cbc4c0e74607b2f19f2ebf Mon Sep 17 00:00:00 2001
From: shrish-browserstack <shrish@browserstack.com>
Date: Fri, 23 Oct 2026 16:30:09 +0530
Subject: [PATCH] fix(access): check group admin rights before updating owners

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 Access/group_helper.py            |  9 +++++
 Access/tests/test_group_helper.py | 57 +++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/Access/group_helper.py b/Access/group_helper.py
index a21298fc..ab3b0145 100644
--- a/Access/group_helper.py
+++ b/Access/group_helper.py
@@ -248,6 +248,15 @@ def update_owners(request, group_name):
         }
         return context
 
+    if not request.user.user.is_allowed_admin_actions_on_group(group):
+        logger.debug("Permission denied, requester is non owner")
+        return {
+            "error": {
+                "error_msg": NON_OWNER_PERMISSION_DENIED_ERROR["error_msg"],
+                "msg": NON_OWNER_PERMISSION_DENIED_ERROR["msg"],
+            }
+        }
+
     logger.debug(
         "updating owners for group "
         + group.name
diff --git a/Access/tests/test_group_helper.py b/Access/tests/test_group_helper.py
index 788a3d90..67a6de8b 100644
--- a/Access/tests/test_group_helper.py
+++ b/Access/tests/test_group_helper.py
@@ -448,3 +448,60 @@ def test_add_user_to_group(mocker, test_name, post_data, expected_output):
 
     context = group_helper.add_user_to_group(request)
     assert expected_output in str(context)
+
+
+def _update_owners_request(mocker, is_admin_of_group):
+    request = mocker.MagicMock()
+    request.user.username = "requester"
+    request.user.user.email = "requester@example.com"
+    request.user.user.is_allowed_admin_actions_on_group.return_value = is_admin_of_group
+    request.POST = QueryDict("owners=member@example.com")
+    return request
+
+
+def test_update_owners_rejects_non_owner(mocker):
+    group = mocker.MagicMock()
+    group.name = testGroupName
+    mocker.patch(
+        "Access.models.GroupV2.get_active_group_by_name", return_value=group
+    )
+    send_mail = mocker.patch(
+        "Access.group_helper.notifications.send_group_owners_update_mail"
+    )
+    request = _update_owners_request(mocker, is_admin_of_group=False)
+
+    context = group_helper.update_owners(request, testGroupName)
+
+    assert context == {
+        "error": {
+            "error_msg": group_helper.NON_OWNER_PERMISSION_DENIED_ERROR["error_msg"],
+            "msg": group_helper.NON_OWNER_PERMISSION_DENIED_ERROR["msg"],
+        }
+    }
+    request.user.user.is_allowed_admin_actions_on_group.assert_called_once_with(group)
+    group.get_all_approved_members.assert_not_called()
+    send_mail.assert_not_called()
+
+
+def test_update_owners_allows_owner(mocker):
+    member = mocker.MagicMock()
+    member.user.email = "member@example.com"
+    member.is_owner = False
+    group = mocker.MagicMock()
+    group.name = testGroupName
+    group.get_all_approved_members.return_value.exclude.return_value = [member]
+    mocker.patch(
+        "Access.models.GroupV2.get_active_group_by_name", return_value=group
+    )
+    mocker.patch("Access.group_helper.transaction.atomic")
+    send_mail = mocker.patch(
+        "Access.group_helper.notifications.send_group_owners_update_mail"
+    )
+    request = _update_owners_request(mocker, is_admin_of_group=True)
+
+    context = group_helper.update_owners(request, testGroupName)
+
+    assert context == {"notification": "Owner's updated"}
+    assert member.is_owner is True
+    member.save.assert_called_once()
+    send_mail.assert_called_once()