Unverified Commit 50d9d1f4 authored by mvdbeek's avatar mvdbeek
Browse files

Fix user login when duplicate UserRoleAssociation exists

In https://github.com/galaxyproject/galaxy/commit/3fd5b0252f840dbb92aaebf31b9d21034b21c578 we changed the sqlalchemy query construct `one_or_none`
method to use the core statement `.scalar_one_or_none`, which bypasses
the identity map. Since there are a handful of legacy accounts with
duplicate UserRoleAssociation rows on usegalaxy.{org,eu} the
deduplication that occured via the entity map meant that no exception
was raised, while for the core level construct this happened.

Fortunately it's easy enough to simply add a distinct
statement to return to the old behavior.

Here's an ipython session to prove this:

"""
In [4]: ura = sa_session.query(UserRoleAssociation).filter_by(user_id=1, role_id=1).all()

In [5]: ura
Out[5]: [<galaxy.model.UserRoleAssociation(1) at 0x165809190>]

In [6]: new_ura = UserRoleAssociation(sa_session.get(User, 1), sa_session.get(Role, 1))

In [7]: sa_session.add(new_ura)

In [8]: sa_session.flush()

In [9]:         role = (
   ...:             sa_session.query(Role)
   ...:             .filter(
   ...:                 and_(
   ...:                     UserRoleAssociation.table.c.user_id == 1,
   ...:                     Role.id == UserRoleAssociation.table.c.role_id,
   ...:                     Role.type == Role.types.PRIVATE,
   ...:                 )
   ...:             )
   ...:             .one_or_none()
   ...:         )

In [10]:         stmt = select(Role).where(
    ...:             and_(
    ...:                 UserRoleAssociation.user_id == 1,
    ...:                 Role.id == UserRoleAssociation.role_id,
    ...:                 Role.type == Role.types.PRIVATE,
    ...:             )
    ...:         )
    ...:         role = sa_session.execute(stmt).scalar_one_or_none()
---------------------------------------------------------------------------
MultipleResultsFound                      Traceback (most recent call last)
Cell In[10], line 8
      1 stmt = select(Role).where(
      2     and_(
      3         UserRoleAssociation.user_id == 1,
   (...)
      6     )
      7 )
----> 8 role = sa_session.execute(stmt).scalar_one_or_none()

File ~/src/galaxy/.venv/lib/python3.11/site-packages/sqlalchemy/engine/result.py:1225, in Result.scalar_one_or_none(self)
   1212 def scalar_one_or_none(self):
   1213     """Return exactly one scalar result or ``None``.
   1214
   1215     This is equivalent to calling :meth:`_engine.Result.scalars` and
   (...)
   1223
   1224     """
-> 1225     return self._only_one_row(
   1226         raise_for_second_row=True, raise_for_none=False, scalar=True
   1227     )

File ~/src/galaxy/.venv/lib/python3.11/site-packages/sqlalchemy/engine/result.py:614, in ResultInternal._only_one_row(self, raise_for_second_row, raise_for_none, scalar)
    612     if next_row is not _NO_ROW:
    613         self._soft_close(hard=True)
--> 614         raise exc.MultipleResultsFound(
    615             "Multiple rows were found when exactly one was required"
    616             if raise_for_none
    617             else "Multiple rows were found when one or none "
    618             "was required"
    619         )
    620 else:
    621     next_row = _NO_ROW

MultipleResultsFound: Multiple rows were found when one or none was required

In [11]:         stmt = select(Role).where(
    ...:             and_(
    ...:                 UserRoleAssociation.user_id == 1,
    ...:                 Role.id == UserRoleAssociation.role_id,
    ...:                 Role.type == Role.types.PRIVATE,
    ...:             )
    ...:         ).distinct()
    ...:         role = sa_session.execute(stmt).scalar_one_or_none()
"""

I think that's what the sqlalchemy docs for [Query.one_or_none](https://docs.sqlalchemy.org/en/14/orm/query.html#sqlalchemy.orm.Query.one_or_none) mean to
communicate with

> Returns None if the query selects no rows. Raises sqlalchemy.orm.exc.MultipleResultsFound if multiple object identities are returned, or if multiple rows are returned for a query that returns only scalar values as opposed to full identity-mapped entities.

Fixes https://github.com/galaxyproject/galaxy/issues/17848
parent 2ce26862
Loading
Loading
Loading
Loading
+9 −5
Original line number Diff line number Diff line
@@ -746,13 +746,17 @@ class GalaxyRBACAgent(RBACAgent):
        return self.get_private_user_role(user)

    def get_private_user_role(self, user, auto_create=False):
        stmt = select(Role).where(
        stmt = (
            select(Role)
            .where(
                and_(
                    UserRoleAssociation.user_id == user.id,
                    Role.id == UserRoleAssociation.role_id,
                    Role.type == Role.types.PRIVATE,
                )
            )
            .distinct()
        )
        role = self.sa_session.execute(stmt).scalar_one_or_none()
        if not role:
            if auto_create: