Merge branch 'release_24.0' into release_24.1

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},

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

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()
 

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(

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

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