Skip to content

Commit

Permalink
Fix user login when duplicate UserRoleAssociation exists
Browse files Browse the repository at this point in the history
In 3fd5b02 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 #17848
  • Loading branch information
mvdbeek committed Mar 27, 2024
1 parent 2ce2686 commit 50d9d1f
Showing 1 changed file with 9 additions and 5 deletions.
14 changes: 9 additions & 5 deletions lib/galaxy/model/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -746,12 +746,16 @@ def create_private_user_role(self, user):
return self.get_private_user_role(user)

def get_private_user_role(self, user, auto_create=False):
stmt = select(Role).where(
and_(
UserRoleAssociation.user_id == user.id,
Role.id == UserRoleAssociation.role_id,
Role.type == Role.types.PRIVATE,
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:
Expand Down

0 comments on commit 50d9d1f

Please sign in to comment.