Merge pull request #18777 from jdavcs/dev_assoc_handling2

Backend handling of setting user-role, user-group, and group-role associations

lib/galaxy/app.py

@@ -655,7 +655,7 @@ def __init__(self, configure_logging=True, use_converters=True, use_display_appl
         # Load security policy.
         self.security_agent = self.model.security_agent
         self.host_security_agent = galaxy.model.security.HostAgent(
-            model=self.security_agent.model, permitted_actions=self.security_agent.permitted_actions
+            self.security_agent.sa_session, permitted_actions=self.security_agent.permitted_actions
         )
 
         # We need the datatype registry for running certain tasks that modify HDAs, and to build the registry we need

lib/galaxy/managers/groups.py

@@ -13,8 +13,6 @@
 from galaxy.managers.context import ProvidesAppContext
 from galaxy.model import Group
 from galaxy.model.base import transaction
-from galaxy.model.db.role import get_roles_by_ids
-from galaxy.model.db.user import get_users_by_ids
 from galaxy.model.scoped_session import galaxy_scoped_session
 from galaxy.schema.fields import Security
 from galaxy.schema.groups import (
@@ -54,13 +52,11 @@ def create(self, trans: ProvidesAppContext, payload: GroupCreatePayload):
 
         group = model.Group(name=name)
         sa_session.add(group)
-        user_ids = payload.user_ids
-        users = get_users_by_ids(sa_session, user_ids)
-        role_ids = payload.role_ids
-        roles = get_roles_by_ids(sa_session, role_ids)
-        trans.app.security_agent.set_entity_group_associations(groups=[group], roles=roles, users=users)
-        with transaction(sa_session):
-            sa_session.commit()
+
+        trans.app.security_agent.set_group_user_and_role_associations(
+            group, user_ids=payload.user_ids, role_ids=payload.role_ids
+        )
+        sa_session.commit()
 
         encoded_id = Security.security.encode_id(group.id)
         item = group.to_dict(view="element")
@@ -88,23 +84,12 @@ def update(self, trans: ProvidesAppContext, group_id: int, payload: GroupUpdateP
         if name := payload.name:
             self._check_duplicated_group_name(sa_session, name)
             group.name = name
-            sa_session.add(group)
-
-        users = None
-        if payload.user_ids is not None:
-            users = get_users_by_ids(sa_session, payload.user_ids)
-
-        roles = None
-        if payload.role_ids is not None:
-            roles = get_roles_by_ids(sa_session, payload.role_ids)
+            sa_session.commit()
 
-        self._app.security_agent.set_entity_group_associations(
-            groups=[group], roles=roles, users=users, delete_existing_assocs=False
+        self._app.security_agent.set_group_user_and_role_associations(
+            group, user_ids=payload.user_ids, role_ids=payload.role_ids
         )
 
-        with transaction(sa_session):
-            sa_session.commit()
-
         encoded_id = Security.security.encode_id(group.id)
         item = group.to_dict(view="element")
         item["url"] = self._url_for(trans, "show_group", group_id=encoded_id)

lib/galaxy/model/__init__.py

@@ -849,14 +849,6 @@ class User(Base, Dictifiable, RepresentById):
     all_notifications: Mapped[List["UserNotificationAssociation"]] = relationship(
         back_populates="user", cascade_backrefs=False
     )
-    non_private_roles: Mapped[List["UserRoleAssociation"]] = relationship(
-        viewonly=True,
-        primaryjoin=(
-            lambda: (User.id == UserRoleAssociation.user_id)
-            & (UserRoleAssociation.role_id == Role.id)
-            & not_(Role.name == User.email)
-        ),
-    )
 
     preferences: AssociationProxy[Any]
 
@@ -2967,10 +2959,11 @@ def __init__(self, name=None):
 
 class UserGroupAssociation(Base, RepresentById):
     __tablename__ = "user_group_association"
+    __table_args__ = (UniqueConstraint("user_id", "group_id"),)
 
     id: Mapped[int] = mapped_column(primary_key=True)
-    user_id: Mapped[int] = mapped_column(ForeignKey("galaxy_user.id"), index=True, nullable=True)
-    group_id: Mapped[int] = mapped_column(ForeignKey("galaxy_group.id"), index=True, nullable=True)
+    user_id: Mapped[int] = mapped_column(ForeignKey("galaxy_user.id"), index=True)
+    group_id: Mapped[int] = mapped_column(ForeignKey("galaxy_group.id"), index=True)
     create_time: Mapped[datetime] = mapped_column(default=now, nullable=True)
     update_time: Mapped[datetime] = mapped_column(default=now, onupdate=now, nullable=True)
     user: Mapped["User"] = relationship(back_populates="groups")
@@ -3685,10 +3678,11 @@ class HistoryUserShareAssociation(Base, UserShareAssociation):
 
 class UserRoleAssociation(Base, RepresentById):
     __tablename__ = "user_role_association"
+    __table_args__ = (UniqueConstraint("user_id", "role_id"),)
 
     id: Mapped[int] = mapped_column(primary_key=True)
-    user_id: Mapped[int] = mapped_column(ForeignKey("galaxy_user.id"), index=True, nullable=True)
-    role_id: Mapped[int] = mapped_column(ForeignKey("role.id"), index=True, nullable=True)
+    user_id: Mapped[int] = mapped_column(ForeignKey("galaxy_user.id"), index=True)
+    role_id: Mapped[int] = mapped_column(ForeignKey("role.id"), index=True)
     create_time: Mapped[datetime] = mapped_column(default=now, nullable=True)
     update_time: Mapped[datetime] = mapped_column(default=now, onupdate=now, nullable=True)
 
@@ -3703,10 +3697,11 @@ def __init__(self, user, role):
 
 class GroupRoleAssociation(Base, RepresentById):
     __tablename__ = "group_role_association"
+    __table_args__ = (UniqueConstraint("group_id", "role_id"),)
 
     id: Mapped[int] = mapped_column(primary_key=True)
-    group_id: Mapped[int] = mapped_column(ForeignKey("galaxy_group.id"), index=True, nullable=True)
-    role_id: Mapped[int] = mapped_column(ForeignKey("role.id"), index=True, nullable=True)
+    group_id: Mapped[int] = mapped_column(ForeignKey("galaxy_group.id"), index=True)
+    role_id: Mapped[int] = mapped_column(ForeignKey("role.id"), index=True)
     create_time: Mapped[datetime] = mapped_column(default=now, nullable=True)
     update_time: Mapped[datetime] = mapped_column(default=now, onupdate=now, nullable=True)
     group: Mapped["Group"] = relationship(back_populates="roles")

lib/galaxy/model/mapping.py

@@ -97,7 +97,7 @@ def _build_model_mapping(engine, map_install_models, thread_local_log) -> Galaxy
         model_modules.append(tool_shed_install)
 
     model_mapping = GalaxyModelMapping(model_modules, engine)
-    model_mapping.security_agent = GalaxyRBACAgent(model_mapping)
+    model_mapping.security_agent = GalaxyRBACAgent(model_mapping.session)
     model_mapping.thread_local_log = thread_local_log
     return model_mapping
 

lib/galaxy/model/migrations/alembic/versions_gxy/13fe10b8e35b_add_not_null_constraints_to_user_group_.py

@@ -0,0 +1,42 @@
+"""Add not-null constraints to user_group_association
+
+Revision ID: 13fe10b8e35b
+Revises: 56ddf316dbd0
+Create Date: 2024-09-09 21:26:26.032842
+
+"""
+
+from alembic import op
+
+from galaxy.model.migrations.data_fixes.association_table_fixer import UserGroupAssociationNullFix
+from galaxy.model.migrations.util import (
+    alter_column,
+    transaction,
+)
+
+# revision identifiers, used by Alembic.
+revision = "13fe10b8e35b"
+down_revision = "56ddf316dbd0"
+branch_labels = None
+depends_on = None
+
+table_name = "user_group_association"
+
+
+def upgrade():
+    with transaction():
+        _remove_records_with_nulls()
+        alter_column(table_name, "user_id", nullable=False)
+        alter_column(table_name, "group_id", nullable=False)
+
+
+def downgrade():
+    with transaction():
+        alter_column(table_name, "user_id", nullable=True)
+        alter_column(table_name, "group_id", nullable=True)
+
+
+def _remove_records_with_nulls():
+    """Remove associations having null as user_id or group_id"""
+    connection = op.get_bind()
+    UserGroupAssociationNullFix(connection).run()

lib/galaxy/model/migrations/alembic/versions_gxy/1fdd615f2cdb_add_not_null_constraints_to_user_role_.py

@@ -0,0 +1,42 @@
+"""Add not-null constraints to user_role_association
+
+Revision ID: 1fdd615f2cdb
+Revises: 349dd9d9aac9
+Create Date: 2024-09-09 21:28:11.987054
+
+"""
+
+from alembic import op
+
+from galaxy.model.migrations.data_fixes.association_table_fixer import UserRoleAssociationNullFix
+from galaxy.model.migrations.util import (
+    alter_column,
+    transaction,
+)
+
+# revision identifiers, used by Alembic.
+revision = "1fdd615f2cdb"
+down_revision = "349dd9d9aac9"
+branch_labels = None
+depends_on = None
+
+table_name = "user_role_association"
+
+
+def upgrade():
+    with transaction():
+        _remove_records_with_nulls()
+        alter_column(table_name, "user_id", nullable=False)
+        alter_column(table_name, "role_id", nullable=False)
+
+
+def downgrade():
+    with transaction():
+        alter_column(table_name, "user_id", nullable=True)
+        alter_column(table_name, "role_id", nullable=True)
+
+
+def _remove_records_with_nulls():
+    """Remove associations having null as user_id or role_id"""
+    connection = op.get_bind()
+    UserRoleAssociationNullFix(connection).run()

lib/galaxy/model/migrations/alembic/versions_gxy/25b092f7938b_add_not_null_constraints_to_group_role_.py

@@ -0,0 +1,42 @@
+"""Add not-null constraints to group_role_association
+
+Revision ID: 25b092f7938b
+Revises: 9ef6431f3a4e
+Create Date: 2024-09-09 16:17:26.652865
+
+"""
+
+from alembic import op
+
+from galaxy.model.migrations.data_fixes.association_table_fixer import GroupRoleAssociationNullFix
+from galaxy.model.migrations.util import (
+    alter_column,
+    transaction,
+)
+
+# revision identifiers, used by Alembic.
+revision = "25b092f7938b"
+down_revision = "9ef6431f3a4e"
+branch_labels = None
+depends_on = None
+
+table_name = "group_role_association"
+
+
+def upgrade():
+    with transaction():
+        _remove_records_with_nulls()
+        alter_column(table_name, "group_id", nullable=True)
+        alter_column(table_name, "role_id", nullable=False)
+
+
+def downgrade():
+    with transaction():
+        alter_column(table_name, "group_id", nullable=True)
+        alter_column(table_name, "role_id", nullable=True)
+
+
+def _remove_records_with_nulls():
+    """Remove associations having null as group_id or role_id"""
+    connection = op.get_bind()
+    GroupRoleAssociationNullFix(connection).run()

lib/galaxy/model/migrations/alembic/versions_gxy/349dd9d9aac9_add_unique_constraint_to_user_role_assoc.py

@@ -0,0 +1,45 @@
+"""Add unique constraint to user_role_association
+
+Revision ID: 349dd9d9aac9
+Revises: 1cf595475b58
+Create Date: 2024-09-09 16:14:58.278850
+
+"""
+
+from alembic import op
+
+from galaxy.model.migrations.data_fixes.association_table_fixer import UserRoleAssociationDuplicateFix
+from galaxy.model.migrations.util import (
+    create_unique_constraint,
+    drop_constraint,
+    transaction,
+)
+
+# revision identifiers, used by Alembic.
+revision = "349dd9d9aac9"
+down_revision = "1cf595475b58"
+branch_labels = None
+depends_on = None
+
+table_name = "user_role_association"
+constraint_column_names = ["user_id", "role_id"]
+unique_constraint_name = (
+    "user_role_association_user_id_key"  # This is what the model's naming convention will generate.
+)
+
+
+def upgrade():
+    with transaction():
+        _remove_duplicate_records()
+        create_unique_constraint(unique_constraint_name, table_name, constraint_column_names)
+
+
+def downgrade():
+    with transaction():
+        drop_constraint(unique_constraint_name, table_name)
+
+
+def _remove_duplicate_records():
+    """Remove duplicate associations"""
+    connection = op.get_bind()
+    UserRoleAssociationDuplicateFix(connection).run()

lib/galaxy/model/migrations/alembic/versions_gxy/56ddf316dbd0_add_unique_constraint_to_user_group_.py

@@ -0,0 +1,45 @@
+"""Add unique constraint to user_group_association
+
+Revision ID: 56ddf316dbd0
+Revises: 1fdd615f2cdb
+Create Date: 2024-09-09 16:10:37.081834
+
+"""
+
+from alembic import op
+
+from galaxy.model.migrations.data_fixes.association_table_fixer import UserGroupAssociationDuplicateFix
+from galaxy.model.migrations.util import (
+    create_unique_constraint,
+    drop_constraint,
+    transaction,
+)
+
+# revision identifiers, used by Alembic.
+revision = "56ddf316dbd0"
+down_revision = "1fdd615f2cdb"
+branch_labels = None
+depends_on = None
+
+table_name = "user_group_association"
+constraint_column_names = ["user_id", "group_id"]
+unique_constraint_name = (
+    "user_group_association_user_id_key"  # This is what the model's naming convention will generate.
+)
+
+
+def upgrade():
+    with transaction():
+        _remove_duplicate_records()
+        create_unique_constraint(unique_constraint_name, table_name, constraint_column_names)
+
+
+def downgrade():
+    with transaction():
+        drop_constraint(unique_constraint_name, table_name)
+
+
+def _remove_duplicate_records():
+    """Remove duplicate associations"""
+    connection = op.get_bind()
+    UserGroupAssociationDuplicateFix(connection).run()

lib/galaxy/model/migrations/alembic/versions_gxy/9ef6431f3a4e_add_unique_constraint_to_group_role_.py

@@ -0,0 +1,45 @@
+"""Add unique constraint to group_role_association
+
+Revision ID: 9ef6431f3a4e
+Revises: 13fe10b8e35b
+Create Date: 2024-09-09 15:01:20.426534
+
+"""
+
+from alembic import op
+
+from galaxy.model.migrations.data_fixes.association_table_fixer import GroupRoleAssociationDuplicateFix
+from galaxy.model.migrations.util import (
+    create_unique_constraint,
+    drop_constraint,
+    transaction,
+)
+
+# revision identifiers, used by Alembic.
+revision = "9ef6431f3a4e"
+down_revision = "13fe10b8e35b"
+branch_labels = None
+depends_on = None
+
+table_name = "group_role_association"
+constraint_column_names = ["group_id", "role_id"]
+unique_constraint_name = (
+    "group_role_association_group_id_key"  # This is what the model's naming convention will generate.
+)
+
+
+def upgrade():
+    with transaction():
+        _remove_duplicate_records()
+        create_unique_constraint(unique_constraint_name, table_name, constraint_column_names)
+
+
+def downgrade():
+    with transaction():
+        drop_constraint(unique_constraint_name, table_name)
+
+
+def _remove_duplicate_records():
+    """Remove duplicate associations"""
+    connection = op.get_bind()
+    GroupRoleAssociationDuplicateFix(connection).run()