From 3f9ca2b2efbd548248a50e3d0530c0b73635ef2d Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Tue, 1 Oct 2024 10:34:14 -0400
Subject: [PATCH 01/18] Do not require private role name = user.email in tests

---
 test/unit/data/model/db/test_security.py | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/test/unit/data/model/db/test_security.py b/test/unit/data/model/db/test_security.py
index 9a6db937a86b..13c6aa9419fe 100644
--- a/test/unit/data/model/db/test_security.py
+++ b/test/unit/data/model/db/test_security.py
@@ -13,7 +13,7 @@
 @pytest.fixture
 def make_user_and_role(session, make_user, make_role, make_user_role_association):
     """
-    Each user created in Galaxy is assumed to have a private role, such that role.name == user.email.
+    Each user created in Galaxy is assumed to have a private role, such that role.type == Role.types.PRIVATE.
     Since we are testing user/group/role associations here, to ensure the correct state of the test database,
     we need to ensure that a user is never created without a corresponding private role.
     Therefore, we use this fixture instead of make_user (which only creates a user).
@@ -21,7 +21,7 @@ def make_user_and_role(session, make_user, make_role, make_user_role_association
 
     def f(**kwd):
         user = make_user()
-        private_role = make_role(name=user.email, type=Role.types.PRIVATE)
+        private_role = make_role(type=Role.types.PRIVATE)
         make_user_role_association(user, private_role)
         return user, private_role
 
@@ -31,15 +31,8 @@ def f(**kwd):
 def test_private_user_role_assoc_not_affected_by_setting_user_roles(session, make_user_and_role):
     # Create user with a private role
     user, private_role = make_user_and_role()
-    assert user.email == private_role.name
     verify_user_associations(user, [], [private_role])  # the only existing association is with the private role
 
-    # Update users's email so it's no longer the same as the private role's name.
-    user.email = user.email + "updated"
-    session.add(user)
-    session.commit()
-    assert user.email != private_role.name
-
     # Delete user roles
     GalaxyRBACAgent(session).set_user_group_and_role_associations(user, role_ids=[])
     # association with private role is preserved
@@ -49,15 +42,8 @@ def test_private_user_role_assoc_not_affected_by_setting_user_roles(session, mak
 def test_private_user_role_assoc_not_affected_by_setting_role_users(session, make_user_and_role):
     # Create user with a private role
     user, private_role = make_user_and_role()
-    assert user.email == private_role.name
     verify_user_associations(user, [], [private_role])  # the only existing association is with the private role
 
-    # Update users's email
-    user.email = user.email + "updated"
-    session.add(user)
-    session.commit()
-    assert user.email != private_role.name
-
     # Update role users
     GalaxyRBACAgent(session).set_role_user_and_group_associations(private_role, user_ids=[])
     # association of private role with user is preserved

From b0194ffa88be2f2adb194c045f37e3d82e251525 Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Tue, 1 Oct 2024 14:43:38 -0400
Subject: [PATCH 02/18] Update remote user private role bug fix from 2009

---
 lib/galaxy/managers/users.py | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py
index 6e50241ab3d9..fc8644d1ba4e 100644
--- a/lib/galaxy/managers/users.py
+++ b/lib/galaxy/managers/users.py
@@ -658,11 +658,8 @@ def get_or_create_remote_user(self, remote_user_email):
             remote_user_email = remote_user_email.lower()
         user = get_user_by_email(self.session(), remote_user_email, self.app.model.User)
         if user:
-            # GVK: June 29, 2009 - This is to correct the behavior of a previous bug where a private
-            # role and default user / history permissions were not set for remote users.  When a
-            # remote user authenticates, we'll look for this information, and if missing, create it.
-            if not self.app.security_agent.get_private_user_role(user):
-                self.app.security_agent.create_private_user_role(user)
+            # Ensure a private role and default permissions are set for remote users (remote user creation bug existed prior to 2009)
+            self.app.security_agent.get_private_user_role(user, auto_create=True)
             if self.app_type == "galaxy":
                 if not user.default_permissions:
                     self.app.security_agent.user_set_default_permissions(user)

From 7ff2633b8f53f5961f02683acd9728f10e0e58e7 Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Tue, 1 Oct 2024 15:55:26 -0400
Subject: [PATCH 03/18] Remove breakpoint() comment

---
 lib/galaxy/model/security.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/galaxy/model/security.py b/lib/galaxy/model/security.py
index a5bc222e2d00..f6936b16a4db 100644
--- a/lib/galaxy/model/security.py
+++ b/lib/galaxy/model/security.py
@@ -1526,7 +1526,6 @@ def _set_user_roles(self, user, role_ids):
         else:
             delete_stmt = delete_stmt.where(UserRoleAssociation.role_id != private_role.id)
         role_ids = self._filter_private_roles(role_ids)
-        # breakpoint()
 
         insert_values = [{"user_id": user.id, "role_id": role_id} for role_id in role_ids]
         self._set_associations(user, UserRoleAssociation, delete_stmt, insert_values)

From 6349b381de026aa3b041643d4a82d5d684f80be9 Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Thu, 3 Oct 2024 14:59:22 -0400
Subject: [PATCH 04/18] Factor out roles service from roles api

---
 lib/galaxy/webapps/galaxy/api/roles.py      | 38 ++++---------
 lib/galaxy/webapps/galaxy/services/roles.py | 59 +++++++++++++++++++++
 2 files changed, 68 insertions(+), 29 deletions(-)
 create mode 100644 lib/galaxy/webapps/galaxy/services/roles.py

diff --git a/lib/galaxy/webapps/galaxy/api/roles.py b/lib/galaxy/webapps/galaxy/api/roles.py
index 166860c92ffd..55f6d9bfece2 100644
--- a/lib/galaxy/webapps/galaxy/api/roles.py
+++ b/lib/galaxy/webapps/galaxy/api/roles.py
@@ -7,22 +7,18 @@
 from fastapi import Body
 
 from galaxy.managers.context import ProvidesUserContext
-from galaxy.managers.roles import RoleManager
-from galaxy.schema.fields import (
-    DecodedDatabaseIdField,
-    Security,
-)
+from galaxy.schema.fields import DecodedDatabaseIdField
 from galaxy.schema.schema import (
     RoleDefinitionModel,
     RoleListResponse,
     RoleModelResponse,
 )
-from galaxy.webapps.base.controller import url_for
 from galaxy.webapps.galaxy.api import (
     depends,
     DependsOnTrans,
     Router,
 )
+from galaxy.webapps.galaxy.services.roles import RolesService
 
 log = logging.getLogger(__name__)
 
@@ -32,48 +28,32 @@
 router = Router(tags=["roles"])
 
 
-def role_to_model(role):
-    item = role.to_dict(view="element")
-    role_id = Security.security.encode_id(role.id)
-    item["url"] = url_for("role", id=role_id)
-    return RoleModelResponse(**item)
-
-
 @router.cbv
 class FastAPIRoles:
-    role_manager: RoleManager = depends(RoleManager)
+    service: RolesService = depends(RolesService)
 
     @router.get("/api/roles")
     def index(self, trans: ProvidesUserContext = DependsOnTrans) -> RoleListResponse:
-        roles = self.role_manager.list_displayable_roles(trans)
-        return RoleListResponse(root=[role_to_model(r) for r in roles])
+        return self.service.get_index(trans=trans)
 
     @router.get("/api/roles/{id}")
     def show(self, id: DecodedDatabaseIdField, trans: ProvidesUserContext = DependsOnTrans) -> RoleModelResponse:
-        role = self.role_manager.get(trans, id)
-        return role_to_model(role)
+        return self.service.show(trans, id)
 
     @router.post("/api/roles", require_admin=True)
     def create(
         self, trans: ProvidesUserContext = DependsOnTrans, role_definition_model: RoleDefinitionModel = Body(...)
     ) -> RoleModelResponse:
-        role = self.role_manager.create_role(trans, role_definition_model)
-        return role_to_model(role)
+        return self.service.create(trans, role_definition_model)
 
     @router.delete("/api/roles/{id}", require_admin=True)
     def delete(self, id: DecodedDatabaseIdField, trans: ProvidesUserContext = DependsOnTrans) -> RoleModelResponse:
-        role = self.role_manager.get(trans, id)
-        role = self.role_manager.delete(trans, role)
-        return role_to_model(role)
+        return self.service.delete(trans, id)
 
     @router.post("/api/roles/{id}/purge", require_admin=True)
     def purge(self, id: DecodedDatabaseIdField, trans: ProvidesUserContext = DependsOnTrans) -> RoleModelResponse:
-        role = self.role_manager.get(trans, id)
-        role = self.role_manager.purge(trans, role)
-        return role_to_model(role)
+        return self.service.purge(trans, id)
 
     @router.post("/api/roles/{id}/undelete", require_admin=True)
     def undelete(self, id: DecodedDatabaseIdField, trans: ProvidesUserContext = DependsOnTrans) -> RoleModelResponse:
-        role = self.role_manager.get(trans, id)
-        role = self.role_manager.undelete(trans, role)
-        return role_to_model(role)
+        return self.service.undelete(trans, id)
diff --git a/lib/galaxy/webapps/galaxy/services/roles.py b/lib/galaxy/webapps/galaxy/services/roles.py
new file mode 100644
index 000000000000..afa34fd05a82
--- /dev/null
+++ b/lib/galaxy/webapps/galaxy/services/roles.py
@@ -0,0 +1,59 @@
+from galaxy.managers.context import ProvidesUserContext
+from galaxy.managers.roles import RoleManager
+from galaxy.schema.fields import (
+    DecodedDatabaseIdField,
+    Security,
+)
+from galaxy.schema.schema import (
+    RoleDefinitionModel,
+    RoleListResponse,
+    RoleModelResponse,
+)
+from galaxy.security.idencoding import IdEncodingHelper
+from galaxy.webapps.base.controller import url_for
+from galaxy.webapps.galaxy.services.base import ServiceBase
+
+
+def role_to_model(role):
+    item = role.to_dict(view="element")
+    role_id = Security.security.encode_id(role.id)
+    item["url"] = url_for("role", id=role_id)
+    return RoleModelResponse(**item)
+
+
+class RolesService(ServiceBase):
+
+    def __init__(
+        self,
+        security: IdEncodingHelper,
+        role_manager: RoleManager,
+    ):
+        super().__init__(security)
+        self.role_manager = role_manager
+
+    def get_index(self, trans: ProvidesUserContext) -> RoleListResponse:
+        roles = self.role_manager.list_displayable_roles(trans)
+        return RoleListResponse(root=[role_to_model(r) for r in roles])
+
+    def show(self, trans: ProvidesUserContext, id: DecodedDatabaseIdField) -> RoleModelResponse:
+        role = self.role_manager.get(trans, id)
+        return role_to_model(role)
+
+    def create(self, trans: ProvidesUserContext, role_definition_model: RoleDefinitionModel):
+        role = self.role_manager.create_role(trans, role_definition_model)
+        return role_to_model(role)
+
+    def delete(self, trans: ProvidesUserContext, id: DecodedDatabaseIdField) -> RoleModelResponse:
+        role = self.role_manager.get(trans, id)
+        role = self.role_manager.delete(trans, role)
+        return role_to_model(role)
+
+    def purge(self, trans: ProvidesUserContext, id: DecodedDatabaseIdField) -> RoleModelResponse:
+        role = self.role_manager.get(trans, id)
+        role = self.role_manager.purge(trans, role)
+        return role_to_model(role)
+
+    def undelete(self, trans: ProvidesUserContext, id: DecodedDatabaseIdField) -> RoleModelResponse:
+        role = self.role_manager.get(trans, id)
+        role = self.role_manager.undelete(trans, role)
+        return role_to_model(role)

From d04327b3259ac4eb4baa64293ff543c4fa937c39 Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Thu, 3 Oct 2024 10:04:20 -0400
Subject: [PATCH 05/18] Add users api endpoint for user-roles

---
 lib/galaxy/webapps/galaxy/api/users.py      | 14 ++++++++++++++
 lib/galaxy/webapps/galaxy/services/users.py |  7 +++++++
 2 files changed, 21 insertions(+)

diff --git a/lib/galaxy/webapps/galaxy/api/users.py b/lib/galaxy/webapps/galaxy/api/users.py
index d3d4ea4dcda4..81c5b374c4da 100644
--- a/lib/galaxy/webapps/galaxy/api/users.py
+++ b/lib/galaxy/webapps/galaxy/api/users.py
@@ -57,6 +57,7 @@
     FlexibleUserIdType,
     MaybeLimitedUserModel,
     RemoteUserCreationPayload,
+    RoleListResponse,
     UserBeaconSetting,
     UserCreationPayload,
     UserDeletionPayload,
@@ -734,6 +735,19 @@ def send_activation_email(
         if not self.service.user_manager.send_activation_email(trans, user.email, user.username):
             raise exceptions.MessageException("Unable to send activation email.")
 
+    @router.get(
+        "/api/users/{user_id}/roles",
+        name="get user roles",
+        description="Return a collection of roles associated with this user. Only admins can see user roles.",
+        require_admin=True,
+    )
+    def get_user_roles(
+        self,
+        user_id: UserIdPathParam,
+        trans: ProvidesUserContext = DependsOnTrans,
+    ) -> RoleListResponse:
+        return self.service.get_user_roles(trans=trans, user_id=user_id)
+
 
 class UserAPIController(BaseGalaxyAPIController, UsesTagsMixin, BaseUIController, UsesFormDefinitionsMixin):
     service: UsersService = depends(UsersService)
diff --git a/lib/galaxy/webapps/galaxy/services/users.py b/lib/galaxy/webapps/galaxy/services/users.py
index 84c1f4a12978..1d5705b34bac 100644
--- a/lib/galaxy/webapps/galaxy/services/users.py
+++ b/lib/galaxy/webapps/galaxy/services/users.py
@@ -30,6 +30,7 @@
     FlexibleUserIdType,
     LimitedUserModel,
     MaybeLimitedUserModel,
+    RoleListResponse,
     UserModel,
 )
 from galaxy.security.idencoding import IdEncodingHelper
@@ -37,6 +38,7 @@
     async_task_summary,
     ServiceBase,
 )
+from galaxy.webapps.galaxy.services.roles import role_to_model
 
 
 class UsersService(ServiceBase):
@@ -248,3 +250,8 @@ def get_index(
             else:
                 rval.append(UserModel(**user_dict))
         return rval
+
+    def get_user_roles(self, trans, user_id):
+        user = self.get_user(trans, user_id)
+        roles = [ura.role for ura in user.roles]
+        return RoleListResponse(root=[role_to_model(r) for r in roles])

From b6616708e1666cc89cd379e33a58bfbd71326d11 Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Thu, 3 Oct 2024 15:04:09 -0400
Subject: [PATCH 06/18] Change how a populator gets a user's private role

Because we no longer can match role name to user email
---
 lib/galaxy_test/base/populators.py | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/galaxy_test/base/populators.py b/lib/galaxy_test/base/populators.py
index a31c5fe73956..1b7d77c762e8 100644
--- a/lib/galaxy_test/base/populators.py
+++ b/lib/galaxy_test/base/populators.py
@@ -138,6 +138,8 @@
 
 SKIP_FLAKEY_TESTS_ON_ERROR = os.environ.get("GALAXY_TEST_SKIP_FLAKEY_TESTS_ON_ERROR", None)
 
+PRIVATE_ROLE_TYPE = "private"
+
 
 def flakey(method):
     @wraps(method)
@@ -1237,11 +1239,13 @@ def user_id(self) -> str:
         return users[0]["id"]
 
     def user_private_role_id(self) -> str:
-        user_email = self.user_email()
-        roles = self.get_roles()
-        users_roles = [r for r in roles if r["name"] == user_email]
-        assert len(users_roles) == 1, f"Did not find exactly one role for email {user_email} - {users_roles}"
-        role = users_roles[0]
+        userid = self.user_id()
+        response = self._get(f"users/{userid}/roles", admin=True)
+        assert response.status_code == 200
+        roles = response.json()
+        private_roles = [r for r in roles if r["type"] == PRIVATE_ROLE_TYPE]
+        assert len(private_roles) == 1, f"Did not find exactly one private role for user {userid} - {private_roles}"
+        role = private_roles[0]
         assert "id" in role, role
         return role["id"]
 

From 347da39aec4bce91e55341caa6385a1cc4b47c14 Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Thu, 3 Oct 2024 15:04:53 -0400
Subject: [PATCH 07/18] Add API test for new user-roles endpoint

---
 lib/galaxy_test/api/test_users.py | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lib/galaxy_test/api/test_users.py b/lib/galaxy_test/api/test_users.py
index 8e081fd2389b..21c5c7fca473 100644
--- a/lib/galaxy_test/api/test_users.py
+++ b/lib/galaxy_test/api/test_users.py
@@ -7,6 +7,7 @@
 )
 from galaxy_test.base.populators import (
     DatasetPopulator,
+    PRIVATE_ROLE_TYPE,
     skip_without_tool,
 )
 
@@ -20,6 +21,7 @@
 
 
 class TestUsersApi(ApiTestCase):
+
     @requires_admin
     @requires_new_user
     def test_index(self):
@@ -356,3 +358,12 @@ def test_manage_beacon_settings(self):
         response = self._get(f"users/{user_id}/beacon")
         user_beacon_settings = response.json()
         assert user_beacon_settings["enabled"]
+
+    @requires_admin
+    @requires_new_user
+    def test_user_roles(self):
+        user = self._setup_user(TEST_USER_EMAIL)
+        response = self._get(f"users/{user['id']}/roles", admin=True)
+        user_roles = response.json()
+        assert len(user_roles) == 1
+        assert user_roles[0]["type"] == PRIVATE_ROLE_TYPE

From 7f2c2a62e2a1aa858c91ccca96c825ad99069b25 Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Tue, 8 Oct 2024 14:08:00 -0400
Subject: [PATCH 08/18] Remove unique constraint from role name, add not null
 constraint

---
 lib/galaxy/model/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py
index b56d0f517b62..6226a68b26f0 100644
--- a/lib/galaxy/model/__init__.py
+++ b/lib/galaxy/model/__init__.py
@@ -3749,7 +3749,7 @@ class Role(Base, Dictifiable, RepresentById):
     id: Mapped[int] = mapped_column(primary_key=True)
     create_time: Mapped[datetime] = mapped_column(default=now, nullable=True)
     update_time: Mapped[datetime] = mapped_column(default=now, onupdate=now, nullable=True)
-    name: Mapped[Optional[str]] = mapped_column(String(255), index=True, unique=True)
+    name: Mapped[str] = mapped_column(String(255), index=True)
     description: Mapped[Optional[str]] = mapped_column(TEXT)
     type: Mapped[Optional[str]] = mapped_column(String(40), index=True)
     deleted: Mapped[Optional[bool]] = mapped_column(index=True, default=False)

From a83da4d54c9539c846c90395ef2df4a306ee1b16 Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Tue, 8 Oct 2024 14:20:23 -0400
Subject: [PATCH 09/18] Add migration: remove unique constraint from role name,
 add not null constraint

---
 ...remove_unique_constraint_from_role_name.py | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 lib/galaxy/model/migrations/alembic/versions_gxy/9a5207190a4d_remove_unique_constraint_from_role_name.py

diff --git a/lib/galaxy/model/migrations/alembic/versions_gxy/9a5207190a4d_remove_unique_constraint_from_role_name.py b/lib/galaxy/model/migrations/alembic/versions_gxy/9a5207190a4d_remove_unique_constraint_from_role_name.py
new file mode 100644
index 000000000000..2f88c66cdabe
--- /dev/null
+++ b/lib/galaxy/model/migrations/alembic/versions_gxy/9a5207190a4d_remove_unique_constraint_from_role_name.py
@@ -0,0 +1,40 @@
+"""Remove unique constraint from role name, add not null constraint
+
+Revision ID: 9a5207190a4d
+Revises: 7ffd33d5d144
+Create Date: 2024-10-08 14:08:28.418055
+
+"""
+
+from galaxy.model.database_object_names import build_index_name
+from galaxy.model.migrations.util import (
+    alter_column,
+    create_index,
+    drop_index,
+    transaction,
+)
+
+# revision identifiers, used by Alembic.
+revision = "9a5207190a4d"
+down_revision = "7ffd33d5d144"
+branch_labels = None
+depends_on = None
+
+
+table_name = "role"
+column_name = "name"
+index_name = build_index_name(table_name, [column_name])
+
+
+def upgrade():
+    with transaction():
+        drop_index(index_name, table_name)
+        alter_column(table_name, column_name, nullable=False)
+        create_index(index_name, table_name, [column_name])
+
+
+def downgrade():
+    with transaction():
+        drop_index(index_name, table_name)
+        alter_column(table_name, column_name, nullable=True)
+        create_index(index_name, table_name, [column_name], unique=True)

From 8ee271992a386d277551183f29502cd776b73c56 Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Tue, 8 Oct 2024 14:47:34 -0400
Subject: [PATCH 10/18] Assign default role name based on role type

Now that role name doesn't have to be unique, we don't want to pass args
like "private role" or "shared role" on role creation.
---
 lib/galaxy/model/__init__.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py
index 6226a68b26f0..27c31bdc6759 100644
--- a/lib/galaxy/model/__init__.py
+++ b/lib/galaxy/model/__init__.py
@@ -3768,8 +3768,12 @@ class types(str, Enum):
         ADMIN = "admin"
         SHARING = "sharing"
 
+    @staticmethod
+    def default_name(role_type):
+        return f"{role_type.value} role"
+
     def __init__(self, name=None, description=None, type=types.SYSTEM, deleted=False):
-        self.name = name
+        self.name = name or Role.default_name(type)
         self.description = description
         self.type = type
         self.deleted = deleted

From bb199f8203924047810a0649cd12a06931235b2e Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Tue, 8 Oct 2024 14:49:37 -0400
Subject: [PATCH 11/18] Do not specify name when creating a sharing role

---
 lib/galaxy/model/security.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/galaxy/model/security.py b/lib/galaxy/model/security.py
index f6936b16a4db..5bbba36c582d 100644
--- a/lib/galaxy/model/security.py
+++ b/lib/galaxy/model/security.py
@@ -1024,7 +1024,7 @@ def privately_share_dataset(self, dataset, users=None):
         self.set_dataset_permission(dataset, {self.permitted_actions.DATASET_ACCESS: [sharing_role]})
 
     def _create_sharing_role(self, users):
-        sharing_role = Role(name=f"Sharing role for: {', '.join(u.email for u in users)}", type=Role.types.SHARING)
+        sharing_role = Role(type=Role.types.SHARING)
         self.sa_session.add(sharing_role)
         with transaction(self.sa_session):
             self.sa_session.commit()
@@ -1807,3 +1807,7 @@ def is_foreign_key_violation(error):
         # If this is a PostgreSQL foreign key error, then error.orig is an instance of psycopg2.errors.ForeignKeyViolation
         # and should have an attribute `pgcode` = 23503.
         return int(getattr(error.orig, "pgcode", -1)) == 23503
+
+
+def role_name_by_type(type):
+    return f"{type} role"

From 5a3f7aa5937883f4d40839fc0310debf522be118 Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Tue, 8 Oct 2024 15:54:34 -0400
Subject: [PATCH 12/18] Fix unit test: check for correct role name

---
 test/unit/data/test_galaxy_mapping.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/test/unit/data/test_galaxy_mapping.py b/test/unit/data/test_galaxy_mapping.py
index 60c9c3116942..6bf94bf21721 100644
--- a/test/unit/data/test_galaxy_mapping.py
+++ b/test/unit/data/test_galaxy_mapping.py
@@ -458,11 +458,11 @@ def test_workflows(self):
     def test_role_creation(self):
         security_agent = GalaxyRBACAgent(self.model.session)
 
-        def check_private_role(private_role, email):
+        def check_private_role(private_role, user):
             assert private_role.type == model.Role.types.PRIVATE
             assert len(private_role.users) == 1
-            assert private_role.name == email
-            assert private_role.description == "Private Role for " + email
+            assert private_role.users[0].user is user
+            assert private_role.name == model.Role.default_name(model.Role.types.PRIVATE)
 
         email = "rule_user_1@example.com"
         u = model.User(email=email, password="password")
@@ -472,7 +472,7 @@ def check_private_role(private_role, email):
         assert role is None
         role = security_agent.create_private_user_role(u)
         assert role is not None
-        check_private_role(role, email)
+        check_private_role(role, u)
 
         email = "rule_user_2@example.com"
         u = model.User(email=email, password="password")
@@ -481,12 +481,12 @@ def check_private_role(private_role, email):
         assert role is None
         role = security_agent.get_private_user_role(u, auto_create=True)
         assert role is not None
-        check_private_role(role, email)
+        check_private_role(role, u)
 
         # make sure re-running auto_create doesn't break things
         role = security_agent.get_private_user_role(u, auto_create=True)
         assert role is not None
-        check_private_role(role, email)
+        check_private_role(role, u)
 
     def test_private_share_role(self):
         security_agent = GalaxyRBACAgent(self.model.session)

From 31c6e6427bb199c223cc556e66accd538715eea6 Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Tue, 8 Oct 2024 15:58:43 -0400
Subject: [PATCH 13/18] Do not use user's email in private role name,
 description

Reason: decouple user email from private role naming: emails can be
changed or redacted; user id in user-role-association + role type is
sufficient to tie a user to a private role.

The description (i.e., "this is a private role for a user" is inferrable
from the role name ("private role"), which is assigned by default.
---
 lib/galaxy/model/__init__.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py
index 27c31bdc6759..b268f8bfc607 100644
--- a/lib/galaxy/model/__init__.py
+++ b/lib/galaxy/model/__init__.py
@@ -1228,10 +1228,7 @@ def is_authenticated(self):
 
     def attempt_create_private_role(self):
         session = object_session(self)
-        role_name = self.email
-        role_desc = f"Private Role for {self.email}"
-        role_type = Role.types.PRIVATE
-        role = Role(name=role_name, description=role_desc, type=role_type)
+        role = Role(type=Role.types.PRIVATE)
         assoc = UserRoleAssociation(self, role)
         session.add(assoc)
         with transaction(session):

From a5cef0c118e245f38ad7ef03ba08748a4d469153 Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Tue, 8 Oct 2024 16:52:41 -0400
Subject: [PATCH 14/18] Modify make_role fixture to accommodate downgrading in
 migration tests

---
 test/unit/data/model/conftest.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/test/unit/data/model/conftest.py b/test/unit/data/model/conftest.py
index f49454266001..a3daec99bc2e 100644
--- a/test/unit/data/model/conftest.py
+++ b/test/unit/data/model/conftest.py
@@ -375,6 +375,14 @@ def f(**kwd):
 @pytest.fixture
 def make_role(session):
     def f(**kwd):
+        # We must specify `name` because after removing the unique constraint
+        # from role.name (migration 9a5207190a4d) and setting up a default name
+        # generation for roles that do not receive a name argument that does
+        # not generate unique names, any migration unit tests that use
+        # this fixture AFTER DOWNGRADING (like # test_migrations.py::test_349dd9d9aac9)
+        # would break due to violating that constraint (restored via
+        # downgrading) without setting name.
+        kwd["name"] = kwd.get("name") or random_str()
         model = m.Role(**kwd)
         write_to_db(session, model)
         return model

From 1ae35a8cee41082e0098104c40ddf9f36766080c Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Wed, 9 Oct 2024 10:06:04 -0400
Subject: [PATCH 15/18] Fix grammar typo in schema

---
 lib/galaxy/schema/schema.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py
index a2cb9ef72272..8abc6d8c9e8a 100644
--- a/lib/galaxy/schema/schema.py
+++ b/lib/galaxy/schema/schema.py
@@ -3117,7 +3117,7 @@ class LibraryAvailablePermissions(Model):
     roles: List[BasicRoleModel] = Field(
         ...,
         title="Roles",
-        description="A list available roles that can be assigned to a particular permission.",
+        description="A list containing available roles that can be assigned to a particular permission.",
     )
     page: int = Field(
         ...,

From 3ab63ccba4f0d8ad85896caaa9666184d1819b99 Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Wed, 9 Oct 2024 10:53:57 -0400
Subject: [PATCH 16/18] Do not retrieve roles based on string matching

Reasons: not needed, query is ambiguos after decoupling user emails from role names
---
 lib/galaxy/model/security.py                  |  9 +--------
 lib/galaxy/webapps/galaxy/api/folders.py      |  4 ----
 lib/galaxy/webapps/galaxy/api/libraries.py    |  4 ----
 .../webapps/galaxy/services/libraries.py      |  3 +--
 .../galaxy/services/library_folders.py        |  3 +--
 lib/galaxy_test/api/test_libraries.py         | 19 -------------------
 6 files changed, 3 insertions(+), 39 deletions(-)

diff --git a/lib/galaxy/model/security.py b/lib/galaxy/model/security.py
index 5bbba36c582d..869f9052c550 100644
--- a/lib/galaxy/model/security.py
+++ b/lib/galaxy/model/security.py
@@ -128,7 +128,7 @@ def get_roles_for_action(self, item, action):
                 roles.append(item_permission.role)
         return roles
 
-    def get_valid_roles(self, trans, item, query=None, page=None, page_limit=None, is_library_access=False):
+    def get_valid_roles(self, trans, item, page=None, page_limit=None, is_library_access=False):
         """
         This method retrieves the list of possible roles that user can select
         in the item permissions form. Admins can select any role so the
@@ -138,11 +138,6 @@ def get_valid_roles(self, trans, item, query=None, page=None, page_limit=None, i
         sharing roles and any public role (not private and not sharing).
         """
         roles = []
-        if query not in [None, ""]:
-            query = query.strip().replace("_", "/_").replace("%", "/%").replace("/", "//")
-            search_query = f"{query}%"
-        else:
-            search_query = None
         # Limit the query only to get the page needed
         if page is not None and page_limit is not None:
             limit = page * page_limit
@@ -164,8 +159,6 @@ def get_valid_roles(self, trans, item, query=None, page=None, page_limit=None, i
             else:
                 # User is not an admin but the configuration exposes all private roles to all users.
                 stmt = select(Role).where(and_(Role.deleted == false(), Role.type == Role.types.PRIVATE))
-            if search_query:
-                stmt = stmt.where(Role.name.like(search_query, escape="/"))
 
             count_stmt = select(func.count()).select_from(stmt)
             total_count = trans.sa_session.scalar(count_stmt)
diff --git a/lib/galaxy/webapps/galaxy/api/folders.py b/lib/galaxy/webapps/galaxy/api/folders.py
index 60f7dde8611b..952073169346 100644
--- a/lib/galaxy/webapps/galaxy/api/folders.py
+++ b/lib/galaxy/webapps/galaxy/api/folders.py
@@ -123,9 +123,6 @@ def get_permissions(
         page_limit: int = Query(
             default=10, title="Page Limit", description="The maximum number of permissions per page when paginating."
         ),
-        q: Optional[str] = Query(
-            None, title="Query", description="Optional search text to retrieve only the roles matching this query."
-        ),
     ) -> Union[LibraryFolderCurrentPermissions, LibraryAvailablePermissions]:
         """Gets the current or available permissions of a particular library.
         The results can be paginated and additionally filtered by a query."""
@@ -135,7 +132,6 @@ def get_permissions(
             scope,
             page,
             page_limit,
-            q,
         )
 
     @router.post(
diff --git a/lib/galaxy/webapps/galaxy/api/libraries.py b/lib/galaxy/webapps/galaxy/api/libraries.py
index 7a814ee09f2a..41ef814a94d6 100644
--- a/lib/galaxy/webapps/galaxy/api/libraries.py
+++ b/lib/galaxy/webapps/galaxy/api/libraries.py
@@ -172,9 +172,6 @@ def get_permissions(
         page_limit: int = Query(
             default=10, title="Page Limit", description="The maximum number of permissions per page when paginating."
         ),
-        q: Optional[str] = Query(
-            None, title="Query", description="Optional search text to retrieve only the roles matching this query."
-        ),
     ) -> Union[LibraryCurrentPermissions, LibraryAvailablePermissions]:
         """Gets the current or available permissions of a particular library.
         The results can be paginated and additionally filtered by a query."""
@@ -185,7 +182,6 @@ def get_permissions(
             is_library_access,
             page,
             page_limit,
-            q,
         )
 
     @router.post(
diff --git a/lib/galaxy/webapps/galaxy/services/libraries.py b/lib/galaxy/webapps/galaxy/services/libraries.py
index 708f2bfb10f7..bd290d73d000 100644
--- a/lib/galaxy/webapps/galaxy/services/libraries.py
+++ b/lib/galaxy/webapps/galaxy/services/libraries.py
@@ -134,7 +134,6 @@ def get_permissions(
         is_library_access: Optional[bool] = False,
         page: int = 1,
         page_limit: int = 10,
-        query: Optional[str] = None,
     ) -> Union[LibraryCurrentPermissions, LibraryAvailablePermissions]:
         """Load all permissions for the given library id and return it.
 
@@ -167,7 +166,7 @@ def get_permissions(
         #  Return roles that are available to select.
         elif scope == LibraryPermissionScope.available:
             roles, total_roles = trans.app.security_agent.get_valid_roles(
-                trans, library, query, page, page_limit, is_library_access
+                trans, library, page, page_limit, is_library_access
             )
 
             return_roles = []
diff --git a/lib/galaxy/webapps/galaxy/services/library_folders.py b/lib/galaxy/webapps/galaxy/services/library_folders.py
index bc560c849238..da57e6a4ec8a 100644
--- a/lib/galaxy/webapps/galaxy/services/library_folders.py
+++ b/lib/galaxy/webapps/galaxy/services/library_folders.py
@@ -84,7 +84,6 @@ def get_permissions(
         scope: Optional[LibraryPermissionScope] = LibraryPermissionScope.current,
         page: int = 1,
         page_limit: int = 10,
-        query: Optional[str] = None,
     ) -> Union[LibraryFolderCurrentPermissions, LibraryAvailablePermissions]:
         """
         Load all permissions for the given folder id and return it.
@@ -113,7 +112,7 @@ def get_permissions(
             return LibraryFolderCurrentPermissions.model_construct(**current_permissions)
         #  Return roles that are available to select.
         elif scope == LibraryPermissionScope.available:
-            roles, total_roles = trans.app.security_agent.get_valid_roles(trans, folder, query, page, page_limit)
+            roles, total_roles = trans.app.security_agent.get_valid_roles(trans, folder, page, page_limit)
             return_roles = []
             for role in roles:
                 return_roles.append(BasicRoleModel(id=role.id, name=role.name, type=role.type))
diff --git a/lib/galaxy_test/api/test_libraries.py b/lib/galaxy_test/api/test_libraries.py
index ba0bd7850870..42f0c0f6b0e5 100644
--- a/lib/galaxy_test/api/test_libraries.py
+++ b/lib/galaxy_test/api/test_libraries.py
@@ -170,25 +170,6 @@ def test_get_library_available_permissions(self):
         available_role_ids = [role["id"] for role in available["roles"]]
         assert role_id in available_role_ids
 
-    @requires_new_library
-    def test_get_library_available_permissions_with_query(self):
-        library = self.library_populator.new_library("GetAvailablePermissionWithQueryTestLibrary")
-        library_id = library["id"]
-
-        with self._different_user():
-            email = self.library_populator.user_email()
-
-        # test at least 2 user roles should be available now
-        available = self.library_populator.get_permissions(library_id, scope="available")
-        available_role_ids = [role["id"] for role in available["roles"]]
-        assert len(available_role_ids) > 1
-
-        # test query for specific role/email
-        available = self.library_populator.get_permissions(library_id, scope="available", q=email)
-        available_role_emails = [role["name"] for role in available["roles"]]
-        assert available["total"] == 1
-        assert available_role_emails[0] == email
-
     @requires_new_library
     def test_create_library_dataset_bootstrap_user(self, library_name="private_dataset", wait=True):
         library = self.library_populator.new_private_library(library_name)

From 146a290206a5be6134fee3d6b8f7cde4306ca48e Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Wed, 9 Oct 2024 15:43:13 -0400
Subject: [PATCH 17/18] Fix set of role names

- Add user email to represent user's private role
- Exclude generic role names
---
 lib/galaxy/files/__init__.py | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/galaxy/files/__init__.py b/lib/galaxy/files/__init__.py
index b13b2e1db8c8..d2c08059548d 100644
--- a/lib/galaxy/files/__init__.py
+++ b/lib/galaxy/files/__init__.py
@@ -396,7 +396,15 @@ def preferences(self):
     def role_names(self) -> Set[str]:
         """The set of role names of this user."""
         user = self.trans.user
-        return {ura.role.name for ura in user.roles} if user else set()
+        role_names = {ura.role.name for ura in user.roles} if user else set()
+        # Exclude generic role names
+        # TODO refactor to use Role.default_name (can't import Role)
+        sharing_role = "sharing role"
+        private_role = "private role"
+        role_names = role_names - {sharing_role, private_role}
+        # Add user email to identify their private role
+        role_names.add(user.email)
+        return role_names
 
     @property
     def group_names(self) -> Set[str]:

From 84a30a2ea8ee299ef23aa92dd89652804979162a Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Wed, 9 Oct 2024 16:35:34 -0400
Subject: [PATCH 18/18] Rebuild client schema

---
 client/src/api/schema/schema.ts | 70 ++++++++++++++++++++++++++++++---
 1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts
index 51148bebc8d7..73bb0a38d2a0 100644
--- a/client/src/api/schema/schema.ts
+++ b/client/src/api/schema/schema.ts
@@ -4646,6 +4646,26 @@ export interface paths {
         patch?: never;
         trace?: never;
     };
+    "/api/users/{user_id}/roles": {
+        parameters: {
+            query?: never;
+            header?: never;
+            path?: never;
+            cookie?: never;
+        };
+        /**
+         * Get User Roles
+         * @description Return a collection of roles associated with this user. Only admins can see user roles.
+         */
+        get: operations["get_user_roles_api_users__user_id__roles_get"];
+        put?: never;
+        post?: never;
+        delete?: never;
+        options?: never;
+        head?: never;
+        patch?: never;
+        trace?: never;
+    };
     "/api/users/{user_id}/send_activation_email": {
         parameters: {
             query?: never;
@@ -13041,7 +13061,7 @@ export interface components {
             page_limit: number;
             /**
              * Roles
-             * @description A list available roles that can be assigned to a particular permission.
+             * @description A list containing available roles that can be assigned to a particular permission.
              */
             roles: components["schemas"]["BasicRoleModel"][];
             /**
@@ -21262,8 +21282,6 @@ export interface operations {
                 page?: number;
                 /** @description The maximum number of permissions per page when paginating. */
                 page_limit?: number;
-                /** @description Optional search text to retrieve only the roles matching this query. */
-                q?: string | null;
             };
             header?: {
                 /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
@@ -28096,8 +28114,6 @@ export interface operations {
                 page?: number;
                 /** @description The maximum number of permissions per page when paginating. */
                 page_limit?: number;
-                /** @description Optional search text to retrieve only the roles matching this query. */
-                q?: string | null;
             };
             header?: {
                 /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
@@ -33297,6 +33313,50 @@ export interface operations {
             };
         };
     };
+    get_user_roles_api_users__user_id__roles_get: {
+        parameters: {
+            query?: never;
+            header?: {
+                /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
+                "run-as"?: string | null;
+            };
+            path: {
+                /** @description The ID of the user. */
+                user_id: string;
+            };
+            cookie?: never;
+        };
+        requestBody?: never;
+        responses: {
+            /** @description Successful Response */
+            200: {
+                headers: {
+                    [name: string]: unknown;
+                };
+                content: {
+                    "application/json": components["schemas"]["RoleListResponse"];
+                };
+            };
+            /** @description Request Error */
+            "4XX": {
+                headers: {
+                    [name: string]: unknown;
+                };
+                content: {
+                    "application/json": components["schemas"]["MessageExceptionModel"];
+                };
+            };
+            /** @description Server Error */
+            "5XX": {
+                headers: {
+                    [name: string]: unknown;
+                };
+                content: {
+                    "application/json": components["schemas"]["MessageExceptionModel"];
+                };
+            };
+        };
+    };
     send_activation_email_api_users__user_id__send_activation_email_post: {
         parameters: {
             query?: never;