Trying to change the group name to an already existing name now fails (#236)

Trying to change the group name to an already existing name now fails

Co-authored-by: Leon Kuchenbecker <[email protected]>
Co-authored-by: root <[email protected]>

Fixes #206

Author: MoritzHahn1337

.vscode/settings.json

@@ -0,0 +1,3 @@
+{
+    "python.pythonPath": "/usr/local/bin/python3.8"
+}

datameta/alembic/versions/20210325_16a09feb3391.py

@@ -0,0 +1,26 @@
+"""make group name unique
+
+Revision ID: 16a09feb3391
+Revises: cfb9cf1b7f6c
+Create Date: 2021-03-25 20:59:08.171642
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '16a09feb3391'
+down_revision = 'cfb9cf1b7f6c'
+branch_labels = None
+depends_on = None
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.create_unique_constraint(op.f('uq_groups_name'), 'groups', ['name'])
+    # ### end Alembic commands ###
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_constraint(op.f('uq_groups_name'), 'groups', type_='unique')
+    # ### end Alembic commands ###

datameta/api/groups.py

@@ -22,6 +22,7 @@
 from .. import security, errors
 from ..resource import resource_by_id, resource_query_by_id, get_identifier
 from sqlalchemy.orm import joinedload
+from sqlalchemy.exc import IntegrityError
 
 from pyramid.httpexceptions import HTTPNoContent, HTTPNotFound, HTTPForbidden, HTTPBadRequest, HTTPGone
 
@@ -118,7 +119,7 @@ def put(request: Request):
     """Change the name of the group"""
 
     group_id = request.matchdict["id"]
-    newGroupName = request.openapi_validated.body["name"]
+    new_group_name = request.openapi_validated.body["name"]
     db = request.dbsession
 
     # Authenticate the user
@@ -129,9 +130,14 @@ def put(request: Request):
     if target_group is None:
         raise HTTPForbidden() # 403 Group ID not found, hidden from the user intentionally
 
-    # Change the group name only if the user is site admin
-    if auth_user.site_admin:
-        target_group.name = newGroupName
+    # Change the group name only if the user is site admin or the admin for the specific group
+    if auth_user.site_admin or (auth_user.group_admin and auth_user.group.uuid == group_id):
+
+        try:
+            target_group.name = new_group_name
+            db.flush()
+        except IntegrityError:
+            raise errors.get_validation_error("A group with that name already exists.")
+        
         return HTTPNoContent()
-    
     raise HTTPForbidden() # 403 Not authorized to change this group name

datameta/models/db.py

@@ -53,7 +53,7 @@ class Group(Base):
     id               = Column(Integer, primary_key=True)
     uuid             = Column(UUID(as_uuid=True), unique=True, default=uuid.uuid4, nullable=False)
     site_id          = Column(String(50), unique=True, nullable=False, index=True)
-    name             = Column(Text, nullable=False)
+    name             = Column(Text, nullable=False, unique=True)
     # Relationships
     user             = relationship('User', back_populates='group')
     submissions      = relationship('Submission', back_populates='group')

tests/integration/test_groupnameupdate.py

@@ -8,51 +8,28 @@
 class GroupNameUpdate(BaseIntegrationTest):
 
     @parameterized.expand([
-        # TEST_NAME                    EXEC_USER           TARGET_GRP       RESP
-        ("foreign_as_admin"          , "admin"           , "group_x_id"   , 204),
-        ("own_as_group_admin"        , "group_x_admin"   , "group_x_id"   , 403),
-        ("foreign_as_group_admin"    , "group_x_admin"   , "group_y_id"   , 403),
-        ("invalid_group"             , "admin"           , "duckburgh"    , 403),
-        ("own_as_regular_user"       , "user_a"          , "group_x_id"   , 403),
-        ("foreign_as_regular_user"   , "user_a"          , "group_y_id"   , 403)
+        # TEST_NAME                    EXEC_USER           TARGET_GRP       NEW_GROUPNAME    RESP
+        ("foreign_as_admin"            , "admin"           , "group_x_id"   , "fancy_group"  , 204),
+        ("own_as_group_admin"          , "group_x_admin"   , "group_x_id"   , "fancy_group"  , 403),
+        ("foreign_as_group_admin"      , "group_x_admin"   , "group_y_id"   , "fancy_group"  , 403),
+        ("invalid_group"               , "admin"           , "duckburgh"    , "fancy_group"  , 403),
+        ("own_as_regular_user"         , "user_a"          , "group_x_id"   , "fancy_group"  , 403),
+        ("foreign_as_regular_user"     , "user_a"          , "group_y_id"   , "fancy_group"  , 403),
+        ("admin_use_existing_groupname", "admin"           , "group_x_id"   , "group_y"      , 400),
+        ("unauthorized"                , ""                , "group_x_id"   , "fancy_group"  , 401),
+        ("admin_expired_token"         , "admin:expired"   , "group_x_id"   , "fancy_group"  , 401),
         ])
-    def test_group_name_update(self, _, executing_user:str, target_group_id:str, expected_response:int):
-        user = self.users[executing_user]
-        request_body = {"name": "new_group_name"}
+    def test_group_name_update(self, _, executing_user:str, target_group_id:str, new_groupname:str, expected_response:int):
+        req_json = {
+            "status": expected_response,
+            "params": {"name": new_groupname}
+        }
+        if executing_user:
+            executing_user, *is_expired = executing_user.split(":")
+            user = self.users[executing_user]
+            req_json["headers"] = user.expired_auth.header if is_expired else user.auth.header
 
         response = self.testapp.put_json(
             f"{base_url}/groups/{target_group_id}",
-            headers = user.auth.header,
-            params = request_body,
-            status = expected_response
-        )
-
-    def test_failure_group_name_update_not_authorised(self, status:int=401):
-        """Testing unsuccessful group name change by unidentified, unauthorized user.
-
-        Expected Response:
-            HTTP 401
-        """
-        request_body = {"name": "fancy_group_name"}
-
-        response = self.testapp.put_json(
-            f"{base_url}/groups/group_y_id",
-            params = request_body,
-            status = status
-        )
-
-    def test_failure_admin_group_name_update_expired_token(self, status:int=401):
-        """Testing unsuccessful group name change by admin user with expired token.
-
-        Expected Response:
-            HTTP 401
-        """
-        user = self.users["admin"]
-        request_body = {"name": "fancy_group_name"}
-
-        response = self.testapp.put_json(
-            f"{base_url}/groups/group_x_id",
-            headers = user.expired_auth.header,
-            params = request_body,
-            status = status
+            **req_json
         )