From 1d420940ffeb34c963da19f591ad4eaa801cbf3a Mon Sep 17 00:00:00 2001
From: Chad Whitacre <chad@zetaweb.com>
Date: Mon, 24 Apr 2017 10:16:31 -0400
Subject: [PATCH] Make finish_email_verification concurrency-safe

---
 gratipay/models/package/__init__.py  |  9 ++++++---
 gratipay/models/participant/email.py | 20 ++++++++++++--------
 gratipay/models/team/package.py      |  5 ++++-
 sql/branch.sql                       |  5 ++++-
 tests/py/test_packages.py            | 11 ++++++++++-
 5 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/gratipay/models/package/__init__.py b/gratipay/models/package/__init__.py
index 4514d15ad1..52b3464c0c 100644
--- a/gratipay/models/package/__init__.py
+++ b/gratipay/models/package/__init__.py
@@ -76,10 +76,13 @@ def ensure_team(self, cursor, owner):
                               , owner=owner
                               , _cursor=cursor
                                )
-            cursor.run('UPDATE packages SET team_id=%s WHERE id=%s', (team.id, self.id))
-            self.set_attributes(team_id=team.id)
+            cursor.run('INSERT INTO teams_to_packages (team_id, package_id) '
+                       'VALUES (%s, %s)', (team.id, self.id))
         return team
 
 
     def _load_team(self, cursor):
-        return cursor.one('SELECT t.*::teams FROM teams t WHERE t.id=%s', (self.team_id,))
+        return cursor.one( 'SELECT t.*::teams FROM teams t WHERE t.id='
+                           '(SELECT team_id FROM teams_to_packages tp WHERE tp.package_id=%s)'
+                         , (self.id,)
+                          )
diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py
index 42758213e2..73ee8d317b 100644
--- a/gratipay/models/participant/email.py
+++ b/gratipay/models/participant/email.py
@@ -231,7 +231,7 @@ def finish_email_verification(self, email, nonce):
         if '' in (email.strip(), nonce.strip()):
             return VERIFICATION_MISSING
         with self.db.get_cursor() as cursor:
-            record = self.get_email(email, cursor)
+            record = self.get_email(email, cursor, and_lock=True)
             if record is None:
                 return VERIFICATION_FAILED
             packages = self.get_packages_claiming(cursor, nonce)
@@ -294,15 +294,19 @@ def finish_package_claims(self, cursor, nonce, *packages):
                                )
 
 
-    def get_email(self, address, cursor=None):
+    def get_email(self, address, cursor=None, and_lock=False):
         """Return a record for a single email address on file for this participant.
+
+        :param unicode address: the email address for which to get a record
+        :param Cursor cursor: a database cursor; if ``None``, we'll use ``self.db``
+        :param and_lock: if True, we will acquire a write-lock on the email record before returning
+        :returns: a database record (a named tuple)
+
         """
-        return (cursor or self.db).one("""
-            SELECT *
-              FROM emails
-             WHERE participant_id=%s
-               AND address=%s
-        """, (self.id, address))
+        sql = 'SELECT * FROM emails WHERE participant_id=%s AND address=%s'
+        if and_lock:
+            sql += ' FOR UPDATE'
+        return (cursor or self.db).one(sql, (self.id, address))
 
 
     def get_emails(self):
diff --git a/gratipay/models/team/package.py b/gratipay/models/team/package.py
index dd33f7b76b..50de9a622a 100644
--- a/gratipay/models/team/package.py
+++ b/gratipay/models/team/package.py
@@ -16,4 +16,7 @@ def package(self):
 
 
     def _load_package(self, cursor):
-        return cursor.one('SELECT p.*::packages FROM packages p WHERE p.team_id=%s', (self.id,))
+        return cursor.one( 'SELECT p.*::packages FROM packages p WHERE p.id='
+                           '(SELECT package_id FROM teams_to_packages tp WHERE tp.team_id=%s)'
+                         , (self.id,)
+                          )
diff --git a/sql/branch.sql b/sql/branch.sql
index 8991d23794..4052aa0ec0 100644
--- a/sql/branch.sql
+++ b/sql/branch.sql
@@ -9,6 +9,9 @@ BEGIN;
     , UNIQUE(nonce, package_id)
      );
 
-    ALTER TABLE packages ADD COLUMN team_id bigint UNIQUE REFERENCES teams(id) ON DELETE RESTRICT;
+    CREATE TABLE teams_to_packages
+    ( team_id       bigint UNIQUE REFERENCES teams(id) ON DELETE RESTRICT
+    , package_id    bigint UNIQUE REFERENCES packages(id) ON DELETE RESTRICT
+     );
 
 END;
diff --git a/tests/py/test_packages.py b/tests/py/test_packages.py
index 23f9f10e46..1d48d5e5d9 100644
--- a/tests/py/test_packages.py
+++ b/tests/py/test_packages.py
@@ -51,6 +51,15 @@ def test_team_can_only_be_linked_from_one_package(self):
         bar = self.make_package(name='bar')
         raises( IntegrityError
               , self.db.run
-              , 'UPDATE packages SET team_id=%s WHERE id=%s'
+              , 'INSERT INTO teams_to_packages (team_id, package_id) VALUES (%s, %s)'
               , (team.id, bar.id)
                )
+
+    def test_package_can_only_be_linked_from_one_team(self):
+        alice, package, team = self.test_can_link_to_a_new_team()
+        bar = self.make_team(name='Bar')
+        raises( IntegrityError
+              , self.db.run
+              , 'INSERT INTO teams_to_packages (team_id, package_id) VALUES (%s, %s)'
+              , (bar.id, package.id)
+               )