Unverified Commit 7867b69f authored by Marius van den Beek's avatar Marius van den Beek Committed by GitHub
Browse files

Merge pull request #18329 from davelopez/24.0_fix_users_api_serialization

[24.0] Fix users API serialization when listing users
parents b208eacf 62049e38
Loading
Loading
Loading
Loading
+3 −0
Original line number Diff line number Diff line
@@ -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
+3 −4
Original line number Diff line number Diff line
@@ -55,12 +55,11 @@ from galaxy.schema.schema import (
    FavoriteObjectsSummary,
    FavoriteObjectType,
    FlexibleUserIdType,
    LimitedUserModel,
    MaybeLimitedUserModel,
    RemoteUserCreationPayload,
    UserBeaconSetting,
    UserCreationPayload,
    UserDeletionPayload,
    UserModel,
)
from galaxy.security.validate_user_input import (
    validate_email,
@@ -202,7 +201,7 @@ class FastAPIUsers:
        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(
@@ -634,7 +633,7 @@ class FastAPIUsers:
        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(
+12 −11
Original line number Diff line number Diff line
@@ -35,6 +35,7 @@ from galaxy.schema.schema import (
    DetailedUserModel,
    FlexibleUserIdType,
    LimitedUserModel,
    MaybeLimitedUserModel,
    UserModel,
)
from galaxy.security.idencoding import IdEncodingHelper
@@ -204,8 +205,8 @@ class UsersService(ServiceBase):
        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 @@ class UsersService(ServiceBase):
                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 @@ class UsersService(ServiceBase):
                    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
+91 −0
Original line number Diff line number Diff line
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