[WIP] Decoupling user email from role name

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;

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]:

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)

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):
@@ -3749,7 +3746,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)
@@ -3768,8 +3765,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

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)

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)
@@ -1024,7 +1017,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()
@@ -1526,7 +1519,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)
@@ -1808,3 +1800,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"

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(
         ...,

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(

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(

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)