diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py index 07a8296f46..c702659762 100644 --- a/gratipay/models/participant/email.py +++ b/gratipay/models/participant/email.py @@ -221,42 +221,74 @@ def _set_primary_email(self, email, cursor): """, dict(email=email, username=self.username)) - def verify_email(self, email, nonce): + def finish_email_verification(self, email, nonce): if '' in (email, nonce): return VERIFICATION_MISSING - r = self.get_email(email) - if r is None: - return VERIFICATION_FAILED - if r.verified: - assert r.nonce is None # and therefore, order of conditions matters - return VERIFICATION_REDUNDANT - if not constant_time_compare(r.nonce, nonce): - return VERIFICATION_FAILED - if (utcnow() - r.verification_start) > EMAIL_HASH_TIMEOUT: - return VERIFICATION_EXPIRED - try: - self.finish_email_verification(email, nonce) - except IntegrityError: - return VERIFICATION_STYMIED - return VERIFICATION_SUCCEEDED + with self.db.get_cursor() as c: + r = self.get_email(email, c) + if r is None: + return VERIFICATION_FAILED + packages = self.get_packages(c, nonce) + if r.verified and not packages: + assert r.nonce is None # and therefore, order of conditions matters + return VERIFICATION_REDUNDANT + if not constant_time_compare(r.nonce, nonce): + return VERIFICATION_FAILED + if (utcnow() - r.verification_start) > EMAIL_HASH_TIMEOUT: + return VERIFICATION_EXPIRED + try: + self.save_email_address(c, email) + self.finish_package_claims(c, *packages) + except IntegrityError: + return VERIFICATION_STYMIED + return VERIFICATION_SUCCEEDED + + + def get_packages(self, cursor, nonce): + """Given a nonce, return :py:class:`Package` objects associated with it. + """ + return cursor.all(""" + SELECT * + FROM packages p + JOIN claims c + ON p.id = c.package_id + WHERE c.nonce=%s + """, (nonce,)) - def finish_email_verification(self, email, nonce): - """Given an email address and nonce, modify the database. + def save_email_address(self, cursor, address): + """Given an email address, modify the database. """ - with self.db.get_cursor() as c: - c.run(""" - UPDATE emails - SET verified=true, verification_end=now(), nonce=NULL - WHERE participant_id=%s - AND address=%s - AND verified IS NULL - """, (self.id, email)) - if not self.email_address: - self.set_primary_email(email, c) + cursor.run(""" + UPDATE emails + SET verified=true, verification_end=now(), nonce=NULL + WHERE participant_id=%s + AND address=%s + AND verified IS NULL + """, (self.id, address)) + if not self.email_address: + self.set_primary_email(address, cursor) + + + def finish_package_claims(self, cursor, *packages): + """Create stub projects if needed and associate them with the packages. + """ + for package in packages: + + # get/create a project for the package + #project = None + + # set ownership of the project + + # associate the two + #package.team_id = project.id + + # log an event + + pass - def get_email(self, email, cursor=None): + def get_email(self, address, cursor=None): """Return a record for a single email address on file for this participant. """ return (cursor or self.db).one(""" @@ -264,7 +296,7 @@ def get_email(self, email, cursor=None): FROM emails WHERE participant_id=%s AND address=%s - """, (self.id, email)) + """, (self.id, address)) def get_emails(self): diff --git a/sql/branch.sql b/sql/branch.sql index 8d39aa1369..8e97919d26 100644 --- a/sql/branch.sql +++ b/sql/branch.sql @@ -9,4 +9,6 @@ BEGIN; , UNIQUE(nonce, package_id) ); + ALTER TABLE packages ADD COLUMN team_id bigint REFERENCES teams(id) ON DELETE RESTRICT; + END; diff --git a/tests/py/test_email.py b/tests/py/test_email.py index 07ce43148a..f9c2af1a7f 100644 --- a/tests/py/test_email.py +++ b/tests/py/test_email.py @@ -25,7 +25,7 @@ def setUp(self): def add(self, participant, address, _flush=False): participant.start_email_verification(address) nonce = participant.get_email(address).nonce - r = participant.verify_email(address, nonce) + r = participant.finish_email_verification(address, nonce) assert r == _email.VERIFICATION_SUCCEEDED if _flush: self.app.email_queue.flush() @@ -52,7 +52,7 @@ def hit_email_spt(self, action, address, user='alice', package_ids=[], should_fa response.render_body({'_': lambda a: a}) return response - def verify_email(self, email, nonce, username='alice', should_fail=False): + def finish_email_verification(self, email, nonce, username='alice', should_fail=False): # Email address is encoded in url. url = '/~%s/emails/verify.html?email2=%s&nonce=%s' url %= (username, encode_for_querystring(email), nonce) @@ -62,7 +62,7 @@ def verify_email(self, email, nonce, username='alice', should_fail=False): def verify_and_change_email(self, old_email, new_email, username='alice', _flush=True): self.hit_email_spt('add-email', old_email) nonce = P(username).get_email(old_email).nonce - self.verify_email(old_email, nonce) + self.finish_email_verification(old_email, nonce) self.hit_email_spt('add-email', new_email) if _flush: self.app.email_queue.flush() @@ -129,15 +129,15 @@ def test_post_too_quickly_is_400(self): assert 'too quickly' in response.body def test_verify_email_without_adding_email(self): - response = self.verify_email('', 'sample-nonce') + response = self.finish_email_verification('', 'sample-nonce') assert 'Bad Info' in response.body def test_verify_email_wrong_nonce(self): self.hit_email_spt('add-email', 'alice@example.com') nonce = 'fake-nonce' - r = self.alice.verify_email('alice@gratipay.com', nonce) + r = self.alice.finish_email_verification('alice@gratipay.com', nonce) assert r == _email.VERIFICATION_FAILED - self.verify_email('alice@example.com', nonce) + self.finish_email_verification('alice@example.com', nonce) expected = None actual = P('alice').email_address assert expected == actual @@ -146,8 +146,8 @@ def test_verify_email_a_second_time_returns_redundant(self): address = 'alice@example.com' self.hit_email_spt('add-email', address) nonce = self.alice.get_email(address).nonce - r = self.alice.verify_email(address, nonce) - r = self.alice.verify_email(address, nonce) + r = self.alice.finish_email_verification(address, nonce) + r = self.alice.finish_email_verification(address, nonce) assert r == _email.VERIFICATION_REDUNDANT def test_verify_email_expired_nonce(self): @@ -159,15 +159,15 @@ def test_verify_email_expired_nonce(self): WHERE participant_id = %s; """, (self.alice.id,)) nonce = self.alice.get_email(address).nonce - r = self.alice.verify_email(address, nonce) + r = self.alice.finish_email_verification(address, nonce) assert r == _email.VERIFICATION_EXPIRED actual = P('alice').email_address assert actual == None - def test_verify_email(self): + def test_finish_email_verification(self): self.hit_email_spt('add-email', 'alice@example.com') nonce = self.alice.get_email('alice@example.com').nonce - self.verify_email('alice@example.com', nonce) + self.finish_email_verification('alice@example.com', nonce) expected = 'alice@example.com' actual = P('alice').email_address assert expected == actual @@ -197,7 +197,7 @@ def test_get_emails(self): def test_verify_email_after_update(self): self.verify_and_change_email('alice@example.com', 'alice@example.net') nonce = self.alice.get_email('alice@example.net').nonce - self.verify_email('alice@example.net', nonce) + self.finish_email_verification('alice@example.net', nonce) expected = 'alice@example.com' actual = P('alice').email_address assert expected == actual @@ -296,7 +296,7 @@ def test_cannot_update_email_to_already_verified(self): with self.assertRaises(EmailTaken): bob.start_email_verification('alice@gratipay.com') nonce = bob.get_email('alice@gratipay.com').nonce - bob.verify_email('alice@gratipay.com', nonce) + bob.finish_email_verification('alice@gratipay.com', nonce) email_alice = P('alice').email_address assert email_alice == 'alice@gratipay.com' @@ -592,7 +592,7 @@ def check(self, *package_names, **kw): def preverify(self, address='alice@example.com'): self.alice.start_email_verification(address) nonce = self.alice.get_email(address).nonce - self.alice.verify_email(address, nonce) + self.alice.finish_email_verification(address, nonce) class VerificationMessage(VerificationBase): @@ -677,3 +677,38 @@ def test_sends_notice_for_unverified_address_and_multiple_packages(self): html, text = self.check('foo', 'bar') assert ' connecting alice@example.com and 2 npm packages ' in html assert ' connecting alice@example.com and 2 npm packages ' in text + + +class FinishEmailVerification(VerificationBase): + + 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' + 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 + + def test_handles_verified_address_and_no_packages(self): + raise NotImplementedError # should error + + def test_handles_verified_address_and_one_package(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 + + def test_handles_verified_address_and_multiple_packages(self): + self.preverify() + raise NotImplementedError + + def test_handles_unverified_address_and_one_package(self): + raise NotImplementedError + + def test_handles_unverified_address_and_multiple_packages(self): + raise NotImplementedError diff --git a/tests/py/test_take_over.py b/tests/py/test_take_over.py index 31e8f13c4e..de35b707e4 100644 --- a/tests/py/test_take_over.py +++ b/tests/py/test_take_over.py @@ -153,7 +153,8 @@ def test_take_over_is_fine_with_identity_info_on_primary(self): TT = self.db.one("SELECT id FROM countries WHERE code='TT'") alice = self.make_participant('alice') alice.start_email_verification('alice@example.com') - alice.verify_email('alice@example.com', alice.get_email('alice@example.com').nonce) + nonce = alice.get_email('alice@example.com').nonce + alice.finish_email_verification('alice@example.com', nonce) alice.store_identity_info(TT, 'nothing-enforced', {}) bob_github = self.make_elsewhere('github', 2, 'bob') @@ -169,7 +170,7 @@ def test_take_over_fails_if_secondary_has_identity_info(self): bob_github = self.make_elsewhere('github', 2, 'bob') bob = bob_github.opt_in('bob')[0].participant bob.start_email_verification('bob@example.com') - bob.verify_email('bob@example.com', bob.get_email('bob@example.com').nonce) + bob.finish_email_verification('bob@example.com', bob.get_email('bob@example.com').nonce) bob.store_identity_info(TT, 'nothing-enforced', {}) pytest.raises(WontTakeOverWithIdentities, alice.take_over, bob_github) @@ -187,11 +188,13 @@ def test_email_addresses_merging(self): alice.start_email_verification('alice@example.com') alice.start_email_verification('alice@example.net') alice.start_email_verification('alice@example.org') - alice.verify_email('alice@example.org', alice.get_email('alice@example.org').nonce) + nonce = alice.get_email('alice@example.org').nonce + alice.finish_email_verification('alice@example.org', nonce) bob_github = self.make_elsewhere('github', 2, 'bob') bob = bob_github.opt_in('bob')[0].participant bob.start_email_verification('alice@example.com') - bob.verify_email('alice@example.com', bob.get_email('alice@example.com').nonce) + nonce = bob.get_email('alice@example.com').nonce + bob.finish_email_verification('alice@example.com', nonce) bob.start_email_verification('alice@example.net') bob.start_email_verification('bob@example.net') alice.take_over(bob_github, have_confirmation=True) diff --git a/www/~/%username/emails/verify.html.spt b/www/~/%username/emails/verify.html.spt index d607ed5a72..c93f8f01f6 100644 --- a/www/~/%username/emails/verify.html.spt +++ b/www/~/%username/emails/verify.html.spt @@ -22,7 +22,7 @@ if participant == user.participant: else: email_address = decode_from_querystring(request.qs.get('email2', ''), default='') nonce = request.qs.get('nonce', '') - result = participant.verify_email(email_address, nonce) + result = participant.finish_email_verification(email_address, nonce) if not participant.email_lang: participant.set_email_lang(request.headers.get("Accept-Language"))