diff --git a/lib/galaxy/managers/citations.py b/lib/galaxy/managers/citations.py index 7a31fa0d5503..a76e38aea9b7 100644 --- a/lib/galaxy/managers/citations.py +++ b/lib/galaxy/managers/citations.py @@ -152,7 +152,7 @@ def to_bibtex(self): try: self.raw_bibtex = self.doi_cache.get_bibtex(self.__doi) except Exception: - log.exception("Failed to fetch bibtex for DOI %s", self.__doi) + log.debug("Failed to fetch bibtex for DOI %s", self.__doi) if self.raw_bibtex is DoiCitation.BIBTEX_UNSET: return f"""@MISC{{{self.__doi}, diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py index d3ff56a8e5ec..097435b4eab2 100644 --- a/lib/galaxy/schema/schema.py +++ b/lib/galaxy/schema/schema.py @@ -349,6 +349,9 @@ class LimitedUserModel(Model): email: Optional[str] = None +MaybeLimitedUserModel = Union[UserModel, LimitedUserModel] + + class DiskUsageUserModel(Model): total_disk_usage: float = TotalDiskUsageField nice_total_disk_usage: str = NiceTotalDiskUsageField diff --git a/lib/galaxy/webapps/base/webapp.py b/lib/galaxy/webapps/base/webapp.py index 0cd502606613..e9624e646185 100644 --- a/lib/galaxy/webapps/base/webapp.py +++ b/lib/galaxy/webapps/base/webapp.py @@ -949,6 +949,10 @@ def get_or_create_default_history(self): self.set_history(history) return history + # Don't create new history if login required and user is anonymous + if self.app.config.require_login and not self.user: + return None + # No suitable history found, create a new one. return self.new_history() diff --git a/lib/galaxy/webapps/galaxy/api/users.py b/lib/galaxy/webapps/galaxy/api/users.py index a5427042c5bd..eb051f63ed97 100644 --- a/lib/galaxy/webapps/galaxy/api/users.py +++ b/lib/galaxy/webapps/galaxy/api/users.py @@ -56,12 +56,11 @@ FavoriteObjectsSummary, FavoriteObjectType, FlexibleUserIdType, - LimitedUserModel, + MaybeLimitedUserModel, RemoteUserCreationPayload, UserBeaconSetting, UserCreationPayload, UserDeletionPayload, - UserModel, ) from galaxy.security.validate_user_input import ( validate_email, @@ -203,7 +202,7 @@ def index_deleted( f_email: Optional[str] = FilterEmailQueryParam, f_name: Optional[str] = FilterNameQueryParam, f_any: Optional[str] = FilterAnyQueryParam, - ) -> List[Union[UserModel, LimitedUserModel]]: + ) -> List[MaybeLimitedUserModel]: return self.service.get_index(trans=trans, deleted=True, f_email=f_email, f_name=f_name, f_any=f_any) @router.post( @@ -651,7 +650,7 @@ def index( f_email: Optional[str] = FilterEmailQueryParam, f_name: Optional[str] = FilterNameQueryParam, f_any: Optional[str] = FilterAnyQueryParam, - ) -> List[Union[UserModel, LimitedUserModel]]: + ) -> List[MaybeLimitedUserModel]: return self.service.get_index(trans=trans, deleted=deleted, f_email=f_email, f_name=f_name, f_any=f_any) @router.get( diff --git a/lib/galaxy/webapps/galaxy/services/users.py b/lib/galaxy/webapps/galaxy/services/users.py index 909a9898f193..0317af35cacc 100644 --- a/lib/galaxy/webapps/galaxy/services/users.py +++ b/lib/galaxy/webapps/galaxy/services/users.py @@ -35,6 +35,7 @@ DetailedUserModel, FlexibleUserIdType, LimitedUserModel, + MaybeLimitedUserModel, UserModel, ) from galaxy.security.idencoding import IdEncodingHelper @@ -204,8 +205,8 @@ def get_index( f_email: Optional[str], f_name: Optional[str], f_any: Optional[str], - ) -> List[Union[UserModel, LimitedUserModel]]: - rval = [] + ) -> List[MaybeLimitedUserModel]: + rval: List[MaybeLimitedUserModel] = [] stmt = select(User) if f_email and (trans.user_is_admin or trans.app.config.expose_user_email): @@ -240,13 +241,12 @@ def get_index( and not trans.app.config.expose_user_email ): if trans.user: - item = trans.user.to_dict() - return [item] + return [UserModel(**trans.user.to_dict())] else: return [] stmt = stmt.filter(User.deleted == false()) for user in trans.sa_session.scalars(stmt).all(): - item = user.to_dict() + user_dict = user.to_dict() # If NOT configured to expose_email, do not expose email UNLESS the user is self, or # the user is an admin if user is not trans.user and not trans.user_is_admin: @@ -255,12 +255,13 @@ def get_index( expose_keys.append("username") if trans.app.config.expose_user_email: expose_keys.append("email") - new_item = {} - for key, value in item.items(): + limited_user = {} + for key, value in user_dict.items(): if key in expose_keys: - new_item[key] = value - item = new_item + limited_user[key] = value + user = LimitedUserModel(**limited_user) + else: + user = UserModel(**user_dict) - # TODO: move into api_values - rval.append(item) + rval.append(user) return rval diff --git a/test/integration/test_users.py b/test/integration/test_users.py new file mode 100644 index 000000000000..d47110bc96c1 --- /dev/null +++ b/test/integration/test_users.py @@ -0,0 +1,91 @@ +from typing import ClassVar + +from galaxy_test.driver import integration_util + +USER_SUMMARY_KEYS = set(["model_class", "id", "email", "username", "deleted", "active", "last_password_change"]) + + +class UsersIntegrationCase(integration_util.IntegrationTestCase): + expose_user_name: ClassVar[bool] + expose_user_email: ClassVar[bool] + expected_regular_user_list_count: ClassVar[int] + expected_limited_user_keys: ClassVar[set] + + @classmethod + def handle_galaxy_config_kwds(cls, config): + super().handle_galaxy_config_kwds(config) + config["expose_user_name"] = cls.expose_user_name + config["expose_user_email"] = cls.expose_user_email + + def setUp(self): + super().setUp() + self._setup_users() + + def _setup_users(self): + self.user = self._get("users/current").json() + self.user2 = self._setup_user("user02@test.gx") + self.user3 = self._setup_user("user03@test.gx") + + def test_admin_index(self): + all_users_response = self._get("users", admin=True) + self._assert_status_code_is(all_users_response, 200) + all_users = all_users_response.json() + assert len(all_users) == 3 + for user in all_users: + self._assert_has_keys(user, *USER_SUMMARY_KEYS) + + def test_user_index(self): + requesting_user_id = self.user["id"] + all_users_response = self._get("users") + self._assert_status_code_is(all_users_response, 200) + all_users = all_users_response.json() + assert len(all_users) == self.expected_regular_user_list_count + + unexpected_user_keys = USER_SUMMARY_KEYS - self.expected_limited_user_keys + for user in all_users: + if user["id"] == requesting_user_id: + # Requesting users should be able to see their own information. + self._assert_has_keys(user, *USER_SUMMARY_KEYS) + continue + # The user should be able to see other users information depending on the configuration. + self._assert_has_keys(user, *self.expected_limited_user_keys) + self._assert_not_has_keys(user, *unexpected_user_keys) + + +class TestExposeUsersIntegration(UsersIntegrationCase): + expose_user_name = True + expose_user_email = True + + # Since we allow to expose user information, all users are returned. + expected_limited_user_keys = set(["id", "username", "email"]) + expected_regular_user_list_count = 3 + + +class TestExposeOnlyUserNameIntegration(UsersIntegrationCase): + expose_user_name = True + expose_user_email = False + + # When only username is exposed, only that field is returned in the user list. + # Since we are exposing user information, all users are returned. + expected_limited_user_keys = set(["id", "username"]) + expected_regular_user_list_count = 3 + + +class TestExposeOnlyUserEmailIntegration(UsersIntegrationCase): + expose_user_name = False + expose_user_email = True + + # When only email is exposed, only that field is returned in the user list. + # Since we are exposing user information, all users are returned. + expected_limited_user_keys = set(["id", "email"]) + expected_regular_user_list_count = 3 + + +class TestUnexposedUsersIntegration(UsersIntegrationCase): + expose_user_name = False + expose_user_email = False + + # Since no user information is exposed, only the current user should be returned. + # And the current user has all fields, so no limited fields. + expected_limited_user_keys = set() + expected_regular_user_list_count = 1