diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py index 9de81ff572..a5e456dabe 100644 --- a/gratipay/models/participant/email.py +++ b/gratipay/models/participant/email.py @@ -225,21 +225,21 @@ def _set_primary_email(self, email, cursor): def finish_email_verification(self, email, nonce): if '' in (email, nonce): return VERIFICATION_MISSING - with self.db.get_cursor() as c: - r = self.get_email(email, c) - if r is None: + with self.db.get_cursor() as cursor: + record = self.get_email(email, cursor) + if record is None: return VERIFICATION_FAILED - packages = self.get_packages_claiming(c, nonce) - if r.verified and not packages: - assert r.nonce is None # and therefore, order of conditions matters + packages = self.get_packages_claiming(cursor, nonce) + if record.verified and not packages: + assert record.nonce is None # and therefore, order of conditions matters return VERIFICATION_REDUNDANT - if not constant_time_compare(r.nonce, nonce): + if not constant_time_compare(record.nonce, nonce): return VERIFICATION_FAILED - if (utcnow() - r.verification_start) > EMAIL_HASH_TIMEOUT: + if (utcnow() - record.verification_start) > EMAIL_HASH_TIMEOUT: return VERIFICATION_EXPIRED try: - self.save_email_address(c, email) - self.finish_package_claims(c, *packages) + self.finish_package_claims(cursor, nonce, *packages) + self.save_email_address(cursor, email) except IntegrityError: return VERIFICATION_STYMIED return VERIFICATION_SUCCEEDED @@ -272,9 +272,10 @@ def save_email_address(self, cursor, address): self.set_primary_email(address, cursor) - def finish_package_claims(self, cursor, *packages): + def finish_package_claims(self, cursor, nonce, *packages): """Create teams if needed and associate them with the packages. """ + cursor.run('DELETE FROM claims WHERE nonce=%s', (nonce,)) package_ids = [] for package in packages: package.get_or_create_linked_team(cursor, self) diff --git a/tests/py/test_email.py b/tests/py/test_email.py index f9c2af1a7f..691087bad3 100644 --- a/tests/py/test_email.py +++ b/tests/py/test_email.py @@ -2,8 +2,8 @@ import json import sys - import urllib + from pytest import raises from gratipay.exceptions import CannotRemovePrimaryEmail, EmailTaken, EmailNotVerified @@ -11,6 +11,7 @@ from gratipay.exceptions import EmailNotOnFile, ProblemChangingEmail from gratipay.testing import P, Harness from gratipay.testing.email import QueuedEmailHarness, SentEmailHarness +from gratipay.models.package import Package from gratipay.models.participant import email as _email from gratipay.utils import encode_for_querystring from gratipay.cli import queue_branch_email as _queue_branch_email @@ -679,36 +680,59 @@ def test_sends_notice_for_unverified_address_and_multiple_packages(self): assert ' connecting alice@example.com and 2 npm packages ' in text -class FinishEmailVerification(VerificationBase): +class PackageLinking(VerificationBase): + + address = 'alice@example.com' def start(self, address, *package_names): packages = [self.make_package(name=name, emails=[address]) for name in package_names] self.alice.start_email_verification(address, *packages) return self.alice.get_email(address).nonce - def test_handles_new_address(self): - address = 'alice@example.com' + def check(self, *package_names): + nonce = self.start(self.address, *package_names) + retval = self.alice.finish_email_verification(self.address, nonce) + assert retval == _email.VERIFICATION_SUCCEEDED + assert self.alice.email_address == P('alice').email_address == self.address + for name in package_names: + package = Package.from_names('npm', name) + assert package.team.package == package + + + def test_preverify_preverifies(self): assert self.alice.email_address is None - self.alice.finish_email_verification(address, self.start(address)) - assert self.alice.email_address == P('alice').email_address == address + self.preverify() + assert self.alice.email_address == self.address - def test_handles_verified_address_and_no_packages(self): - raise NotImplementedError # should error - def test_handles_verified_address_and_one_package(self): + def test_unverified_address_and_no_packages_succeeds(self): + assert self.check() + + def test_unverified_address_and_one_package_succeeds(self): + assert self.check('foo') + + def test_unverified_address_and_multiple_packages_succeeds(self): + assert self.check('foo', 'bar') + + def test_verified_address_and_no_packages_is_a_no_go(self): self.preverify() - address = 'alice@example.com' - assert self.alice.email_address == address - self.alice.finish_email_verification(address, self.start(address, 'foo')) - assert self.alice.email_address == P('alice').email_address == address - raise NotImplementedError # assert we connect the package + raises(EmailAlreadyVerified, self.check) - def test_handles_verified_address_and_multiple_packages(self): + def test_verified_address_and_one_package_succeeds(self): self.preverify() - raise NotImplementedError + assert self.check('foo') - def test_handles_unverified_address_and_one_package(self): - raise NotImplementedError + def test_verified_address_and_multiple_packages_succeeds(self): + self.preverify() + assert self.check('foo', 'bar') - def test_handles_unverified_address_and_multiple_packages(self): - raise NotImplementedError + + def test_while_we_are_at_it_that_packages_have_unique_teams_that_survive_comparison(self): + self.test_handles_verified_address_and_multiple_packages() + + foo = Package.from_names('npm', 'foo') + bar = Package.from_names('npm', 'bar') + + assert foo.team == foo.team + assert bar.team == bar.team + assert foo.team != bar.team