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; 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]: 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) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index b56d0f517b62..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): @@ -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 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) diff --git a/lib/galaxy/model/security.py b/lib/galaxy/model/security.py index a5bc222e2d00..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) @@ -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" 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( ..., 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/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/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/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/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) 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]) 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) 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 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"] 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 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 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)