diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py index 49c74219c1..05656653dd 100644 --- a/gratipay/models/participant/email.py +++ b/gratipay/models/participant/email.py @@ -233,31 +233,36 @@ def _set_primary_email(self, email, cursor): def finish_email_verification(self, email, nonce): - """Given an email address and a nonce as strings, return a tuple: - (a ``VERIFICATION_*`` constant, and a list of packages for - ``VERIFICATION_SUCCEEDED`` or ``None`` otherwise) + """Given an email address and a nonce as strings, return a tuple: (a + ``VERIFICATION_*`` constant, a list of packages for + ``VERIFICATION_SUCCEEDED`` or ``None`` otherwise, and a boolean + indicating whether the participant's PayPal address was updated) """ if '' in (email.strip(), nonce.strip()): - return VERIFICATION_FAILED, None + return VERIFICATION_FAILED, None, None with self.db.get_cursor() as cursor: record = self.get_email(email, cursor, and_lock=True) if record is None: - return VERIFICATION_FAILED, None + return VERIFICATION_FAILED, None, None 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, None + return VERIFICATION_REDUNDANT, None, None if not constant_time_compare(record.nonce, nonce): - return VERIFICATION_FAILED, None + return VERIFICATION_FAILED, None, None if (utcnow() - record.verification_start) > EMAIL_HASH_TIMEOUT: - return VERIFICATION_FAILED, None + return VERIFICATION_FAILED, None, None try: + paypal_updated = False if packages: self.finish_package_claims(cursor, nonce, *packages) self.save_email_address(cursor, email) + if packages and not self.get_payout_routes(good_only=True): + self.set_paypal_address(email, cursor) + paypal_updated = True except IntegrityError: - return VERIFICATION_STYMIED, None - return VERIFICATION_SUCCEEDED, packages + return VERIFICATION_STYMIED, None, None + return VERIFICATION_SUCCEEDED, packages, paypal_updated def get_packages_claiming(self, cursor, nonce): diff --git a/tests/py/test_email.py b/tests/py/test_email.py index 9c8fbb5216..a46ca637ad 100644 --- a/tests/py/test_email.py +++ b/tests/py/test_email.py @@ -30,7 +30,7 @@ def add(self, participant, address, _flush=False): participant.start_email_verification(address) nonce = participant.get_email(address).nonce result = participant.finish_email_verification(address, nonce) - assert result == (_email.VERIFICATION_SUCCEEDED, []) + assert result == (_email.VERIFICATION_SUCCEEDED, [], False) if _flush: self.app.email_queue.flush() @@ -145,7 +145,7 @@ def test_verify_email_wrong_nonce(self): self.hit_email_spt('add-email', 'alice@example.com') nonce = 'fake-nonce' result = self.alice.finish_email_verification('alice@gratipay.com', nonce) - assert result == (_email.VERIFICATION_FAILED, None) + assert result == (_email.VERIFICATION_FAILED, None, None) self.hit_verify_spt('alice@example.com', nonce) expected = None actual = P('alice').email_address @@ -157,7 +157,7 @@ def test_verify_email_a_second_time_returns_redundant(self): nonce = self.alice.get_email(address).nonce self.alice.finish_email_verification(address, nonce) result = self.alice.finish_email_verification(address, nonce) - assert result == (_email.VERIFICATION_REDUNDANT, None) + assert result == (_email.VERIFICATION_REDUNDANT, None, None) def test_verify_email_expired_nonce_fails(self): address = 'alice@example.com' @@ -169,7 +169,7 @@ def test_verify_email_expired_nonce_fails(self): """, (self.alice.id,)) nonce = self.alice.get_email(address).nonce result = self.alice.finish_email_verification(address, nonce) - assert result == (_email.VERIFICATION_FAILED, None) + assert result == (_email.VERIFICATION_FAILED, None, None) actual = P('alice').email_address assert actual == None @@ -182,12 +182,12 @@ def test_finish_email_verification(self): def test_empty_email_fails(self): for empty in ('', ' '): result = self.alice.finish_email_verification(empty, 'foobar') - assert result == (_email.VERIFICATION_FAILED, None) + assert result == (_email.VERIFICATION_FAILED, None, None) def test_empty_nonce_fails(self): for empty in ('', ' '): result = self.alice.finish_email_verification('foobar', empty) - assert result == (_email.VERIFICATION_FAILED, None) + assert result == (_email.VERIFICATION_FAILED, None, None) def test_email_verification_is_backwards_compatible(self): """Test email verification still works with unencoded email in verification link. @@ -550,13 +550,13 @@ def test_finishing_verification_clears_competing_claims_and_emails(self): assert emails() == [self.alice.id, bob.id] result = self.alice.finish_email_verification('alice@example.com', anonce) - assert result == (_email.VERIFICATION_SUCCEEDED, [foo]) + assert result == (_email.VERIFICATION_SUCCEEDED, [foo], True) assert claims() == {} assert emails() == [self.alice.id] result = bob.finish_email_verification('alice@example.com', bnonce) - assert result == (_email.VERIFICATION_FAILED, None) + assert result == (_email.VERIFICATION_FAILED, None, None) class RemoveEmail(Alice): @@ -740,7 +740,7 @@ def check(self, *package_names): # email? packages = [Package.from_names(NPM, name) for name in package_names] - assert result == (_email.VERIFICATION_SUCCEEDED, packages) + assert result == (_email.VERIFICATION_SUCCEEDED, packages, bool(packages)) assert self.alice.email_address == P('alice').email_address == self.address # database? @@ -802,11 +802,11 @@ def test_bob_cannot_steal_a_package_claim_from_alice(self): bob = self.make_participant('bob', claimed_time='now') bob.start_email_verification(self.address, foo) result = bob.finish_email_verification(self.address, nonce) # using alice's nonce, even! - assert result == (_email.VERIFICATION_FAILED, None) + assert result == (_email.VERIFICATION_FAILED, None, None) assert len(bob.get_teams()) == 0 result = self.alice.finish_email_verification(self.address, nonce) - assert result == (_email.VERIFICATION_SUCCEEDED, [foo]) + assert result == (_email.VERIFICATION_SUCCEEDED, [foo], True) teams = self.alice.get_teams() assert len(teams) == 1 assert teams[0].package == foo @@ -823,6 +823,15 @@ def test_while_we_are_at_it_that_packages_have_unique_teams_that_survive_compari assert foo.team != bar.team + def test_finishing_email_verification_with_preexisting_paypal_doesnt_update_paypal(self): + self.add_and_verify_email(self.alice, self.address) + self.alice.set_paypal_address(self.address) + nonce = self.start(self.address, 'foo') + result = self.alice.finish_email_verification(self.address, nonce) + foo = Package.from_names('npm', 'foo') + assert result == (_email.VERIFICATION_SUCCEEDED, [foo], False) + + def test_finishing_email_verification_is_thread_safe(self): foo = self.make_package() self.alice.start_email_verification(self.address, foo) @@ -857,5 +866,5 @@ def monkey(self, *a, **kw): finally: Package.get_or_create_linked_team = old_get_or_create_linked_team - assert results[a.ident] == (_email.VERIFICATION_SUCCEEDED, [foo]) - assert results[b.ident] == (_email.VERIFICATION_REDUNDANT, None) + assert results[a.ident] == (_email.VERIFICATION_SUCCEEDED, [foo], True) + assert results[b.ident] == (_email.VERIFICATION_REDUNDANT, None, None) diff --git a/www/~/%username/emails/verify.html.spt b/www/~/%username/emails/verify.html.spt index c3a70fe436..14fd8e93f0 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, packages = participant.finish_email_verification(email_address, nonce) + result, packages, paypal_updated = participant.finish_email_verification(email_address, nonce) project_list = None if packages is None else [p.team for p in packages] if not participant.email_lang: participant.set_email_lang(request.headers.get("Accept-Language"))