Merge pull request #2186 from DSD-DBS/project-access-policy

fix: Add uniqueness constraint for project users

backend/capellacollab/alembic/versions/16a64401737c_add_uniqueness_constraint_for_project_.py

@@ -0,0 +1,93 @@
+# SPDX-FileCopyrightText: Copyright DB InfraGO AG and contributors
+# SPDX-License-Identifier: Apache-2.0
+
+"""Add uniqueness constraint for project users
+
+Revision ID: 16a64401737c
+Revises: 8731ac0b284e
+Create Date: 2025-02-10 16:19:52.570927
+
+"""
+
+import logging
+
+import sqlalchemy as sa
+from alembic import op
+
+LOGGER = logging.getLogger(__name__)
+
+# revision identifiers, used by Alembic.
+revision = "16a64401737c"
+down_revision = "8731ac0b284e"
+branch_labels = None
+depends_on = None
+
+t_project_user_association = sa.Table(
+    "project_user_association",
+    sa.MetaData(),
+    sa.Column("user_id", sa.Integer),
+    sa.Column("project_id", sa.Integer),
+    sa.Column("role", sa.String),
+    sa.Column("permission", sa.String),
+)
+
+
+def upgrade():
+    bind = op.get_bind()
+    insp = sa.inspect(bind)
+    constraint = insp.get_pk_constraint("project_user_association")
+
+    if (
+        constraint["name"]
+        and "project_user_association_pkey" == constraint["name"]
+    ):
+        # The database looks correct, nothing to do
+        return
+
+    project_user_combinations = []
+    users_to_create = {}
+
+    for project_user in (
+        bind.execute(sa.select(t_project_user_association)).mappings().all()
+    ):
+        project_user_tuple = (
+            project_user["user_id"],
+            project_user["project_id"],
+        )
+        if project_user_tuple in project_user_combinations:
+            # User is part of the project multiple times
+
+            LOGGER.info("Found duplicated user %s", project_user_tuple)
+
+            bind.execute(
+                sa.delete(t_project_user_association)
+                .where(
+                    t_project_user_association.c.user_id
+                    == project_user["user_id"]
+                )
+                .where(
+                    t_project_user_association.c.project_id
+                    == project_user["project_id"]
+                )
+            )
+            users_to_create[project_user_tuple] = project_user
+        else:
+            project_user_combinations.append(project_user_tuple)
+
+    for user in users_to_create.values():
+        LOGGER.info("Recreating user: %s", user)
+        bind.execute(
+            t_project_user_association.insert().values(
+                user_id=user["user_id"],
+                project_id=user["project_id"],
+                role=user["role"],
+                permission=user["permission"],
+            )
+        )
+
+    LOGGER.info("Creating primary key 'project_user_association_pkey'")
+    op.create_primary_key(
+        "project_user_association_pkey",
+        "project_user_association",
+        ["user_id", "project_id"],
+    )

backend/capellacollab/projects/crud.py

@@ -48,8 +48,12 @@ def get_common_projects_for_users(
     user1: users_models.DatabaseUser,
     user2: users_models.DatabaseUser,
 ) -> abc.Sequence[models.DatabaseProject]:
-    user1_table = orm.aliased(project_users_models.ProjectUserAssociation)
-    user2_table = orm.aliased(project_users_models.ProjectUserAssociation)
+    user1_table = orm.aliased(
+        project_users_models.DatabaseProjectUserAssociation
+    )
+    user2_table = orm.aliased(
+        project_users_models.DatabaseProjectUserAssociation
+    )
 
     return (
         db.execute(

backend/capellacollab/projects/models.py

@@ -22,7 +22,9 @@
     from capellacollab.projects.tools.models import (
         DatabaseProjectToolAssociation,
     )
-    from capellacollab.projects.users.models import ProjectUserAssociation
+    from capellacollab.projects.users.models import (
+        DatabaseProjectUserAssociation,
+    )
 
 
 class UserMetadata(core_pydantic.BaseModel):
@@ -134,7 +136,7 @@ class DatabaseProject(database.Base):
         default=ProjectType.GENERAL
     )
 
-    users: orm.Mapped[list[ProjectUserAssociation]] = orm.relationship(
+    users: orm.Mapped[list[DatabaseProjectUserAssociation]] = orm.relationship(
         default_factory=list, back_populates="project"
     )
     tokens: orm.Mapped[list[DatabaseProjectPATAssociation]] = orm.relationship(

backend/capellacollab/projects/permissions/permissions.py

@@ -33,7 +33,7 @@ def inherit_global_permissions(
 
 def derive_project_permissions_from_role(
     project: projects_models.DatabaseProject,
-    project_user: projects_users_models.ProjectUserAssociation | None,
+    project_user: projects_users_models.DatabaseProjectUserAssociation | None,
     user: users_models.DatabaseUser,
 ) -> models.ProjectUserScopes:
     read_only_permissions = models.ProjectUserScopes(

backend/capellacollab/projects/users/crud.py

@@ -13,19 +13,19 @@ def get_project_user_association_or_raise(
     db: orm.Session,
     project: projects_models.DatabaseProject,
     user: users_models.DatabaseUser,
-) -> models.ProjectUserAssociation:
+) -> models.DatabaseProjectUserAssociation:
     return db.execute(
-        sa.select(models.ProjectUserAssociation)
-        .where(models.ProjectUserAssociation.project == project)
-        .where(models.ProjectUserAssociation.user == user)
+        sa.select(models.DatabaseProjectUserAssociation)
+        .where(models.DatabaseProjectUserAssociation.project == project)
+        .where(models.DatabaseProjectUserAssociation.user == user)
     ).scalar_one()
 
 
 def get_project_user_association(
     db: orm.Session,
     project: projects_models.DatabaseProject,
     user: users_models.DatabaseUser,
-) -> models.ProjectUserAssociation | None:
+) -> models.DatabaseProjectUserAssociation | None:
     try:
         return get_project_user_association_or_raise(db, project, user)
     except exc.NoResultFound:
@@ -38,8 +38,8 @@ def add_user_to_project(
     user: users_models.DatabaseUser,
     role: models.ProjectUserRole,
     permission: models.ProjectUserPermission,
-) -> models.ProjectUserAssociation:
-    association = models.ProjectUserAssociation(
+) -> models.DatabaseProjectUserAssociation:
+    association = models.DatabaseProjectUserAssociation(
         role=role,
         permission=permission,
         project=project,
@@ -55,7 +55,7 @@ def change_role_of_user_in_project(
     project: projects_models.DatabaseProject,
     user: users_models.DatabaseUser,
     role: models.ProjectUserRole,
-) -> models.ProjectUserAssociation:
+) -> models.DatabaseProjectUserAssociation:
     association = get_project_user_association_or_raise(db, project, user)
     association.role = role
     db.commit()
@@ -67,7 +67,7 @@ def change_permission_of_user_in_project(
     project: projects_models.DatabaseProject,
     user: users_models.DatabaseUser,
     permission: models.ProjectUserPermission,
-) -> models.ProjectUserAssociation:
+) -> models.DatabaseProjectUserAssociation:
     association = get_project_user_association_or_raise(db, project, user)
     association.permission = permission
     db.commit()
@@ -80,9 +80,9 @@ def delete_user_from_project(
     user: users_models.DatabaseUser,
 ):
     db.execute(
-        sa.delete(models.ProjectUserAssociation)
-        .where(models.ProjectUserAssociation.user == user)
-        .where(models.ProjectUserAssociation.project == project)
+        sa.delete(models.DatabaseProjectUserAssociation)
+        .where(models.DatabaseProjectUserAssociation.user == user)
+        .where(models.DatabaseProjectUserAssociation.project == project)
     )
     db.commit()
 
@@ -91,17 +91,17 @@ def delete_users_from_project(
     db: orm.Session, project: projects_models.DatabaseProject
 ):
     db.execute(
-        sa.delete(models.ProjectUserAssociation).where(
-            models.ProjectUserAssociation.project_id == project.id
+        sa.delete(models.DatabaseProjectUserAssociation).where(
+            models.DatabaseProjectUserAssociation.project_id == project.id
         )
     )
     db.commit()
 
 
 def delete_projects_for_user(db: orm.Session, user_id: int):
     db.execute(
-        sa.delete(models.ProjectUserAssociation).where(
-            models.ProjectUserAssociation.user_id == user_id
+        sa.delete(models.DatabaseProjectUserAssociation).where(
+            models.DatabaseProjectUserAssociation.user_id == user_id
         )
     )
     db.commit()

backend/capellacollab/projects/users/models.py

@@ -54,7 +54,7 @@ class PatchProjectUser(core_pydantic.BaseModel):
     reason: str
 
 
-class ProjectUserAssociation(database.Base):
+class DatabaseProjectUserAssociation(database.Base):
     __tablename__ = "project_user_association"
 
     user_id: orm.Mapped[int] = orm.mapped_column(

backend/capellacollab/projects/users/routes.py

@@ -33,15 +33,15 @@
 def check_user_not_in_project(
     project: projects_models.DatabaseProject, user: users_models.DatabaseUser
 ):
-    if user in project.users:
+    if user in [user.user for user in project.users]:
         raise exceptions.ProjectUserAlreadyExistsError(user.name, project.slug)
 
 
 def get_project_user_association_or_raise(
     db: orm.Session,
     project: projects_models.DatabaseProject,
     user: users_models.DatabaseUser,
-) -> models.ProjectUserAssociation | models.ProjectUser:
+) -> models.DatabaseProjectUserAssociation | models.ProjectUser:
     if project_user := crud.get_project_user_association(db, project, user):
         return project_user
 
@@ -66,7 +66,7 @@ def get_current_project_user(
         projects_injectables.get_existing_project
     ),
     db: orm.Session = fastapi.Depends(database.get_db),
-) -> models.ProjectUserAssociation | models.ProjectUser:
+) -> models.DatabaseProjectUserAssociation | models.ProjectUser:
     """Get the current project users"""
     if user.role == users_models.Role.ADMIN:
         return models.ProjectUser(
@@ -130,7 +130,7 @@ def add_user_to_project(
         users_injectables.get_own_user
     ),
     db: orm.Session = fastapi.Depends(database.get_db),
-) -> models.ProjectUserAssociation:
+) -> models.DatabaseProjectUserAssociation:
     if not (
         user := users_crud.get_user_by_name(db, post_project_user.username)
     ):

backend/capellacollab/settings/modelsources/t4c/instance/repositories/crud.py

@@ -106,10 +106,12 @@ def _get_user_write_t4c_repositories(
         .where(projects_models.DatabaseProject.is_archived.is_(False))
         .join(projects_models.DatabaseProject.users)
         .where(
-            projects_users_models.ProjectUserAssociation.permission
+            projects_users_models.DatabaseProjectUserAssociation.permission
             == projects_users_models.ProjectUserPermission.WRITE
         )
-        .where(projects_users_models.ProjectUserAssociation.user == user)
+        .where(
+            projects_users_models.DatabaseProjectUserAssociation.user == user
+        )
         .distinct()
     )
 

backend/capellacollab/users/models.py

@@ -17,7 +17,9 @@
 
 if t.TYPE_CHECKING:
     from capellacollab.events.models import DatabaseUserHistoryEvent
-    from capellacollab.projects.users.models import ProjectUserAssociation
+    from capellacollab.projects.users.models import (
+        DatabaseProjectUserAssociation,
+    )
     from capellacollab.sessions.models import DatabaseSession
     from capellacollab.users.tokens.models import DatabaseUserToken
 
@@ -172,8 +174,8 @@ class DatabaseUser(database.Base):
         default=datetime.datetime.now(datetime.UTC)
     )
 
-    projects: orm.Mapped[list[ProjectUserAssociation]] = orm.relationship(
-        default_factory=list, back_populates="user"
+    projects: orm.Mapped[list[DatabaseProjectUserAssociation]] = (
+        orm.relationship(default_factory=list, back_populates="user")
     )
     sessions: orm.Mapped[list[DatabaseSession]] = orm.relationship(
         default_factory=list, back_populates="owner"

backend/tests/projects/test_projects_permissions.py

@@ -68,7 +68,7 @@ def test_route():
 def test_project_permission_validation_injectable_passes(
     id_token: str,
     project: projects_models.DatabaseProject,
-    project_user: projects_users_models.ProjectUserAssociation,
+    project_user: projects_users_models.DatabaseProjectUserAssociation,
 ):
     """Test that the project permission validation passes if permissions are sufficient"""
     project_user.role = projects_users_models.ProjectUserRole.MANAGER

backend/tests/projects/test_projects_users_routes.py

@@ -21,7 +21,7 @@ def test_assign_read_write_permission_when_adding_manager(
     project: projects_models.DatabaseProject,
 ):
     response = client.post(
-        f"/api/v1/projects/{project.slug}/users/",
+        f"/api/v1/projects/{project.slug}/users",
         json={
             "role": projects_users_models.ProjectUserRole.MANAGER.value,
             "permission": projects_users_models.ProjectUserPermission.READ.value,
@@ -173,3 +173,72 @@ def test_get_current_project_user_as_admin(
     assert response.status_code == 200
     assert response.json()["role"] == "administrator"
     assert response.json()["permission"] == "write"
+
+
+def test_get_project_users(
+    db: orm.Session,
+    client: testclient.TestClient,
+    project: projects_models.DatabaseProject,
+    user2: users_models.DatabaseUser,
+    admin: users_models.DatabaseUser,
+):
+    projects_users_crud.add_user_to_project(
+        db,
+        project=project,
+        user=user2,
+        role=projects_users_models.ProjectUserRole.MANAGER,
+        permission=projects_users_models.ProjectUserPermission.WRITE,
+    )
+
+    another_user = users_crud.create_user(db, "another_user", "another_user")
+    projects_users_crud.add_user_to_project(
+        db,
+        project=project,
+        user=another_user,
+        role=projects_users_models.ProjectUserRole.MANAGER,
+        permission=projects_users_models.ProjectUserPermission.WRITE,
+    )
+
+    response = client.get(f"/api/v1/projects/{project.slug}/users")
+
+    assert len(response.json()) == 4
+    usernames = [user["user"]["name"] for user in response.json()]
+
+    assert admin.name in usernames
+    assert user2.name in usernames
+    assert another_user.name in usernames
+
+
+@pytest.mark.usefixtures("admin")
+def test_fail_to_add_existing_user(
+    client: testclient.TestClient,
+    project: projects_models.DatabaseProject,
+    user2: users_models.DatabaseUser,
+):
+    """Try to add a user twice to a project"""
+    response = client.post(
+        f"/api/v1/projects/{project.slug}/users",
+        json={
+            "role": projects_users_models.ProjectUserRole.MANAGER.value,
+            "permission": projects_users_models.ProjectUserPermission.READ.value,
+            "username": user2.name,
+            "reason": "",
+        },
+    )
+
+    assert response.status_code == 200
+
+    response = client.post(
+        f"/api/v1/projects/{project.slug}/users",
+        json={
+            "role": projects_users_models.ProjectUserRole.MANAGER.value,
+            "permission": projects_users_models.ProjectUserPermission.READ.value,
+            "username": user2.name,
+            "reason": "",
+        },
+    )
+
+    assert response.status_code == 409
+    assert (
+        response.json()["detail"]["err_code"] == "PROJECT_USER_ALREADY_EXISTS"
+    )