Add a unit test triggering a bug in IP autologin, and fix the bug

Contains_eager must be used when an explicit join has already been added
to the query. If not, it will just end up in a cross product. In that
case, when loading participations and users, many rows are returned, one
for every user. If the participation's user isn't in the identity map, a
new one will be created with its fields initialized using the data of
the first result row, even if it's for another user.

Author: lw

cms/server/contest/authentication.py

@@ -38,7 +38,7 @@
 import logging
 from datetime import timedelta
 
-from sqlalchemy.orm import contains_eager
+from sqlalchemy.orm import contains_eager, joinedload
 
 from cms import config
 from cms.db import Participation, User
@@ -259,7 +259,7 @@ def _authenticate_request_by_ip_address(sql_session, contest, ip_address):
     ip_network = ipaddress.ip_network((ip_address, ip_address.max_prefixlen))
 
     participations = sql_session.query(Participation) \
-        .options(contains_eager(Participation.user)) \
+        .options(joinedload(Participation.user)) \
         .filter(Participation.contest == contest) \
         .filter(Participation.ip.any(ip_network))
 

cmstestsuite/unit_tests/server/contest/authentication_test.py

@@ -50,20 +50,32 @@ class TestValidateLogin(DatabaseMixin, unittest.TestCase):
     def setUp(self):
         super(TestValidateLogin, self).setUp()
         self.timestamp = make_datetime()
+        self.add_contest()
         self.contest = self.add_contest(allow_password_authentication=True)
+        self.add_user(username="otheruser")
         self.user = self.add_user(
             username="myuser", password=build_password("mypass"))
         self.participation = self.add_participation(
             contest=self.contest, user=self.user)
 
     def assertSuccess(self, username, password, ip_address):
+        # We had an issue where due to a misuse of contains_eager we ended up
+        # with the wrong user attached to the participation. This only happens
+        # if the correct user isn't already in the identity map, which is what
+        # these lines trigger.
+        self.session.flush()
+        self.session.expire(self.user)
+        self.session.expire(self.contest)
+
         authenticated_participation, cookie = validate_login(
             self.session, self.contest, self.timestamp,
             username, password, ipaddress.ip_address(ip_address))
 
         self.assertIsNotNone(authenticated_participation)
         self.assertIsNotNone(cookie)
         self.assertIs(authenticated_participation, self.participation)
+        self.assertIs(authenticated_participation.user, self.user)
+        self.assertIs(authenticated_participation.contest, self.contest)
 
     def assertFailure(self, username, password, ip_address):
         authenticated_participation, cookie = validate_login(
@@ -152,7 +164,9 @@ class TestAuthenticateRequest(DatabaseMixin, unittest.TestCase):
     def setUp(self):
         super(TestAuthenticateRequest, self).setUp()
         self.timestamp = make_datetime()
+        self.add_contest()
         self.contest = self.add_contest()
+        self.add_user(username="otheruser")
         self.user = self.add_user(
             username="myuser", password=build_password("mypass"))
         self.participation = self.add_participation(
@@ -176,10 +190,23 @@ def assertSuccess(self, **kwargs):
         # autologin or thanks to the cookie) and return the cookie that should
         # be set (or None, if it should be cleared/left unset).
         # The arguments are the same as those of attempt_authentication.
+
+        # We had an issue where due to a misuse of contains_eager we ended up
+        # with the wrong user attached to the participation. This only happens
+        # if the correct user isn't already in the identity map, which is what
+        # these lines trigger.
+        self.session.flush()
+        self.session.expire(self.user)
+        self.session.expire(self.contest)
+
         authenticated_participation, cookie = \
             self.attempt_authentication(**kwargs)
+
         self.assertIsNotNone(authenticated_participation)
         self.assertIs(authenticated_participation, self.participation)
+        self.assertIs(authenticated_participation.user, self.user)
+        self.assertIs(authenticated_participation.contest, self.contest)
+
         return cookie
 
     def assertSuccessAndCookieRefreshed(self, **kwargs):