diff --git a/emails/verification-notice.spt b/emails/verification-notice.spt new file mode 100644 index 0000000000..d6675cb568 --- /dev/null +++ b/emails/verification-notice.spt @@ -0,0 +1,72 @@ +{{ _("New activity on your account") }} + +[---] text/html +{% if new_email_verified %} +{{ ngettext( "We are connecting the {package_name} npm package to the {username} account on " + "Gratipay. This is a notification sent to {email_address} because that is the " + "primary email address we have on file." + , "We are connecting {n} npm packages to the {username} account on Gratipay. This is " + "a notification sent to {email_address} because that is the primary email address " + "we have on file." + , n=npackages + , package_name=('{}'|safe).format(package_name) + , username=('{0}'|safe).format(username) + , email_address=('{}'|safe).format(email) + ) }} +{% elif npackages > 0 %} +{{ ngettext( "We are connecting {email_address} and the {package_name} npm package to the " + "{username} account on Gratipay. This is a notification sent to {email_address_2} " + "because that is the primary email address we have on file." + , "We are connecting {email_address} and {n} npm packages to the {username} account on " + "Gratipay. This is a notification sent to {email_address_2} because that is the " + "primary email address we have on file." + , n=npackages + , package_name=('{}'|safe).format(package_name) + , username=('{0}'|safe).format(username) + , email_address=('{}'|safe).format(new_email) + , email_address_2=('{}'|safe).format(email) + ) }} +{% else %} +{{ _( "We are connecting {email_address} to the {username} account on Gratipay. This is a " + "notification sent to {email_address_2} because that is the primary email address we have " + "on file." + , username=('{0}'|safe).format(username) + , email_address=('{}'|safe).format(new_email) + , email_address_2=('{}'|safe).format(email) + ) }} + {% endif %} +[---] text/plain +{% if new_email_verified %} +{{ ngettext( "We are connecting the {package_name} npm package to the {username} account on " + "Gratipay. This is a notification sent to {email_address} because that is the " + "primary email address we have on file." + , "We are connecting {n} npm packages to the {username} account on Gratipay. This is " + "a notification sent to {email_address} because that is the primary email address " + "we have on file." + , n=npackages + , package_name=package_name + , username=username + , email_address=email + ) }} +{% elif npackages > 0 %} +{{ ngettext( "We are connecting {email_address} and the {package_name} npm package to the " + "{username} account on Gratipay. This is a notification sent to {email_address_2} " + "because that is the primary email address we have on file." + , "We are connecting {email_address} and {n} npm packages to the {username} account on " + "Gratipay. This is a notification sent to {email_address_2} because that is the " + "primary email address we have on file." + , n=npackages + , package_name=package_name + , username=username + , email_address=new_email + , email_address_2=email + ) }} +{% else %} +{{ _( "We are connecting {email_address} to the {username} account on Gratipay. This is a " + "notification sent to {email_address_2} because that is the primary email address we have " + "on file." + , username=username + , email_address=new_email + , email_address_2=email + ) }} + {% endif %} diff --git a/emails/verification.spt b/emails/verification.spt index 906c24a505..006d850527 100644 --- a/emails/verification.spt +++ b/emails/verification.spt @@ -1,16 +1,63 @@ {{ _("Connect to {0} on Gratipay?", username) }} [---] text/html -{{ _("We've received a request to connect {0} to the {1} account on Gratipay. Sound familiar?", - ('%s'|safe) % email, - ('{0}'|safe).format(username)) }} +{% if new_email_verified %} +{{ ngettext( "We've received a request to connect the {package_name} npm package to the " + "{username} account on Gratipay. Sound familiar?" + , "We've received a request to connect {n} npm packages to the {username} account " + "on Gratipay. Sound familiar?" + , n=npackages + , package_name=('{}'|safe).format(package_name) + , username=('{0}'|safe).format(username) + ) }} +{% elif npackages > 0 %} +{{ ngettext( "We've received a request to connect {email_address} and the {package_name} npm " + "package to the {username} account on Gratipay. Sound familiar?" + , "We've received a request to connect {email_address} and {n} npm packages to the " + "{username} account on Gratipay. Sound familiar?" + , n=npackages + , package_name=('{}'|safe).format(package_name) + , email_address=('{}'|safe).format(new_email) + , username=('{0}'|safe).format(username) + ) }} +{% else %} +{{ _( "We've received a request to connect {email_address} to the {username} account on Gratipay. " + "Sound familiar?" + , email_address=('{}'|safe).format(new_email) + , username=('{0}'|safe).format(username) + ) }} +{% endif %}

{{ _("Yes, proceed!") }} [---] text/plain -{{ _("We've received a request to connect {0} to the {1} account on Gratipay. Sound familiar?", - email, username) }} +{% if new_email_verified %} +{{ ngettext( "We've received a request to connect the {package_name} npm package to the " + "{username} account on Gratipay. Sound familiar?" + , "We've received a request to connect {n} npm packages to the {username} account " + "on Gratipay. Sound familiar?" + , n=npackages + , package_name=package_name + , username=username + ) }} +{% elif npackages > 0 %} +{{ ngettext( "We've received a request to connect {email_address} and the {package_name} npm " + "package to the {username} account on Gratipay. Sound familiar?" + , "We've received a request to connect {email_address} and {n} npm packages to the " + "{username} account on Gratipay. Sound familiar?" + , n=npackages + , package_name=package_name + , email_address=new_email + , username=username + ) }} +{% else %} +{{ _( "We've received a request to connect {email_address} to the {username} account on Gratipay. " + "Sound familiar?" + , email_address=new_email + , username=username + ) }} +{% endif %} {{ _("Follow this link to finish connecting your email:") }} diff --git a/emails/verification_notice.spt b/emails/verification_notice.spt deleted file mode 100644 index b48abd72ca..0000000000 --- a/emails/verification_notice.spt +++ /dev/null @@ -1,13 +0,0 @@ -{{ _("Connecting {0} to {1} on Gratipay.", new_email, username) }} - -[---] text/html -{{ _("We are connecting {0} to the {1} account on Gratipay. This is a notification " - "sent to {2} because that is the primary email address we have on file.", - ('%s'|safe) % new_email, - ('{0}'|safe).format(username), - ('%s'|safe) % email) }} - -[---] text/plain -{{ _("We are connecting {0} to the {1} account on Gratipay. This is a notification " - "sent to {2} because that is the primary email address we have on file.", - new_email, username, email) }} diff --git a/error.spt b/error.spt index d07b35a3e7..1c69853e8a 100644 --- a/error.spt +++ b/error.spt @@ -2,8 +2,7 @@ from __future__ import absolute_import, division, print_function, unicode_litera from aspen.http import status_strings -from gratipay.utils import LazyResponse -from gratipay.utils.i18n import HTTP_ERRORS +from gratipay.utils.i18n import HTTP_ERRORS, LocalizedErrorResponse [----------------------------------------] @@ -19,7 +18,7 @@ try: except Exception as e: website.tell_sentry(e, state) -if isinstance(response, LazyResponse): +if isinstance(response, LocalizedErrorResponse): response.render_body(state) err = response.body if code == 500 and not err: diff --git a/gratipay/elsewhere/__init__.py b/gratipay/elsewhere/__init__.py index e125c46136..87e7524355 100644 --- a/gratipay/elsewhere/__init__.py +++ b/gratipay/elsewhere/__init__.py @@ -17,7 +17,7 @@ from requests_oauthlib import OAuth1Session, OAuth2Session from gratipay.elsewhere._extractors import not_available -from gratipay.utils import LazyResponse +from gratipay.utils.i18n import LocalizedErrorResponse ACTIONS = {'opt-in', 'connect'} @@ -137,13 +137,13 @@ def msg(_, to_age): return _("You've consumed your quota of requests, you can try again in {0}.", to_age(reset)) else: return _("You're making requests too fast, please try again later.") - raise LazyResponse(status, msg) + raise LocalizedErrorResponse(status, msg) if status != 200: log('{} api responded with {}:\n{}'.format(self.name, status, response.text) , level=logging.ERROR) msg = lambda _: _("{0} returned an error, please try again later.", self.display_name) - raise LazyResponse(502, msg) + raise LocalizedErrorResponse(502, msg) return response diff --git a/gratipay/exceptions.py b/gratipay/exceptions.py index 983a4a4f18..6888e97259 100644 --- a/gratipay/exceptions.py +++ b/gratipay/exceptions.py @@ -4,7 +4,7 @@ from __future__ import print_function, unicode_literals -from aspen import Response +from gratipay.utils.i18n import LocalizedErrorResponse class ProblemChangingUsername(Exception): @@ -27,28 +27,37 @@ class UsernameAlreadyTaken(ProblemChangingUsername): msg = "The username '{}' is already taken." -class ProblemChangingEmail(Response): - def __init__(self, *args): - Response.__init__(self, 400, self.msg.format(*args)) +class ProblemChangingEmail(LocalizedErrorResponse): + pass class EmailAlreadyVerified(ProblemChangingEmail): - msg = "{} is already verified for this Gratipay account." + def lazy_body(self, _): + return _("You have already added and verified that address.") class EmailTaken(ProblemChangingEmail): - msg = "{} is already connected to a different Gratipay account." + def lazy_body(self, _): + return _("That address is already linked to a different Gratipay account.") class CannotRemovePrimaryEmail(ProblemChangingEmail): - msg = "You cannot remove your primary email address." + def lazy_body(self, _): + return _("You cannot remove your primary email address.") + +class EmailNotOnFile(ProblemChangingEmail): + def lazy_body(self, _): + return _("That email address is not on file for this package.") class EmailNotVerified(ProblemChangingEmail): - msg = "The email address '{}' is not verified." + def lazy_body(self, _): + return _("That email address is not verified.") class TooManyEmailAddresses(ProblemChangingEmail): - msg = "You've reached the maximum number of email addresses we allow." + def lazy_body(self, _): + return _("You've reached the maximum number of email addresses we allow.") -class Throttled(Exception): - msg = "You've initiated too many emails too quickly. Please try again in a minute or two." +class Throttled(LocalizedErrorResponse): + def lazy_body(self, _): + return _("You've initiated too many emails too quickly. Please try again in a minute or two.") class ProblemChangingNumber(Exception): @@ -78,3 +87,4 @@ def __str__(self): return "Negative balance not allowed in this context." class NotWhitelisted(Exception): pass +class NoPackages(Exception): pass diff --git a/gratipay/models/package/__init__.py b/gratipay/models/package/__init__.py index 571e2cb971..d15579fca4 100644 --- a/gratipay/models/package/__init__.py +++ b/gratipay/models/package/__init__.py @@ -1,6 +1,9 @@ # -*- coding: utf-8 -*- from __future__ import absolute_import, division, print_function, unicode_literals +import uuid + +from gratipay.models.team import Team from postgres.orm import Model @@ -10,6 +13,14 @@ class Package(Model): """Represent a gratipackage. :-) + + Packages are entities on open source package managers; `npm + `_ is the only one we support so far. Each package + on npm has a page on Gratipay with an URL of the form ``/on/npm/foo/``. + Packages can be claimed by Gratipay participants, at which point we create + a :py:class:`~gratipay.models.team.Team` for them under the hood so they + can start accepting payments. + """ typname = 'packages' @@ -25,6 +36,13 @@ def __ne__(self, other): return self.id != other.id + @property + def url_path(self): + """The path part of the URL for this package on Gratipay. + """ + return '/on/{}/{}/'.format(self.package_manager, self.name) + + # Constructors # ============ @@ -42,8 +60,45 @@ def from_names(cls, package_manager, name): "WHERE package_manager=%s and name=%s", (package_manager, name)) - # Emails - # ====== + @property + def team(self): + """A computed attribute, the :py:class:`~gratipay.models.team.Team` + linked to this package if there is one, otherwise ``None``. Makes a + database call. + """ + return self._load_team(self.db) + + + def ensure_team(self, cursor, owner): + """Given a db cursor and :py:class:`Participant`, insert into ``teams`` if need be. + """ + if self._load_team(cursor): + return + + def slug_options(): + yield self.name + for i in range(1, 10): + yield '{}-{}'.format(self.name, i) + yield uuid.uuid4().hex + + for slug in slug_options(): + if cursor.one('SELECT count(*) FROM teams WHERE slug=%s', (slug,)) > 0: + continue + team = Team.insert( slug=slug + , slug_lower=slug.lower() + , name=slug + , homepage='' + , product_or_service='' + , owner=owner + , _cursor=cursor + ) + cursor.run('INSERT INTO teams_to_packages (team_id, package_id) ' + 'VALUES (%s, %s)', (team.id, self.id)) + break + - def send_confirmation_email(self, address): - pass + def _load_team(self, cursor): + 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 38ca03bc32..73ee8d317b 100644 --- a/gratipay/models/participant/email.py +++ b/gratipay/models/participant/email.py @@ -9,7 +9,7 @@ import gratipay from gratipay.exceptions import EmailAlreadyVerified, EmailTaken, CannotRemovePrimaryEmail -from gratipay.exceptions import EmailNotVerified, TooManyEmailAddresses +from gratipay.exceptions import EmailNotVerified, TooManyEmailAddresses, EmailNotOnFile, NoPackages from gratipay.security.crypto import constant_time_compare from gratipay.utils import encode_for_querystring @@ -41,140 +41,272 @@ class Email(object): """ - def add_email(self, email): + def start_email_verification(self, email, *packages): """Add an email address for a participant. This is called when adding a new email address, and when resending the verification email for an unverified email address. :param unicode email: the email address to add + :param gratipay.models.package.Package packages: packages to optionally + also verify ownership of :returns: ``None`` :raises EmailAlreadyVerified: if the email is already verified for - this participant + this participant (unless they're claiming packages) :raises EmailTaken: if the email is verified for a different participant + :raises EmailNotOnFile: if the email address is not on file for any of + the packages :raises TooManyEmailAddresses: if the participant already has 10 emails :raises Throttled: if the participant adds too many emails too quickly """ + with self.db.get_cursor() as c: + self.validate_email_verification_request(c, email, *packages) + link = self.get_email_verification_link(c, email, *packages) + + verified_emails = self.get_verified_email_addresses() + kwargs = dict( npackages=len(packages) + , package_name=packages[0].name if packages else '' + , new_email=email + , new_email_verified=email in verified_emails + , link=link + , include_unsubscribe=False + ) + self.app.email_queue.put(self, 'verification', email=email, **kwargs) + if self.email_address and self.email_address != email: + self.app.email_queue.put( self + , 'verification-notice' + + # Don't count this one against their sending quota. + # It's going to their own verified address, anyway. + , _user_initiated=False + + , **kwargs + ) - # Check that this address isn't already verified - owner = self.db.one(""" - SELECT p.username - FROM emails e INNER JOIN participants p - ON e.participant_id = p.id - WHERE e.address = %(email)s - AND e.verified IS true - """, locals()) - if owner: - if owner == self.username: - raise EmailAlreadyVerified(email) + + def validate_email_verification_request(self, c, email, *packages): + """Given a cursor, email, and packages, return ``None`` or raise. + """ + if not all(email in p.emails for p in packages): + raise EmailNotOnFile() + + owner_id = c.one(""" + SELECT participant_id + FROM emails + WHERE address = %(email)s + AND verified IS true + """, dict(email=email)) + + if owner_id: + if owner_id != self.id: + raise EmailTaken() + elif packages: + pass # allow reverify if claiming packages else: - raise EmailTaken(email) + raise EmailAlreadyVerified() if len(self.get_emails()) > 9: - raise TooManyEmailAddresses(email) + if owner_id and owner_id == self.id and packages: + pass # they're using an already-verified email to verify packages + else: + raise TooManyEmailAddresses() + + + def get_email_verification_link(self, c, email, *packages): + """Get a link to complete an email verification workflow. + + :param Cursor c: the cursor to use + :param unicode email: the email address to be verified + :param packages: :py:class:`~gratipay.models.package.Package` objects + for which a successful verification will also entail verification of + ownership of the package + + :returns: a URL by which to complete the verification process + + """ + self.app.add_event( c + , 'participant' + , dict(id=self.id, action='add', values=dict(email=email)) + ) + nonce = self.get_email_verification_nonce(c, email) + if packages: + self.start_package_claims(c, nonce, *packages) + link = "{base_url}/~{username}/emails/verify.html?email2={encoded_email}&nonce={nonce}" + return link.format( base_url=gratipay.base_url + , username=self.username_lower + , encoded_email=encode_for_querystring(email) + , nonce=nonce + ) + + + def get_email_verification_nonce(self, c, email): + """Given a cursor and email address, return a verification nonce. + """ nonce = str(uuid.uuid4()) - verification_start = utcnow() - - try: - with self.db.get_cursor() as c: - self.app.add_event(c, 'participant', dict(id=self.id, action='add', values=dict(email=email))) - c.run(""" - INSERT INTO emails - (address, nonce, verification_start, participant_id) - VALUES (%s, %s, %s, %s) - """, (email, nonce, verification_start, self.id)) - except IntegrityError: - nonce = self.db.one(""" + existing = c.one( 'SELECT * FROM emails WHERE address=%s AND participant_id=%s' + , (email, self.id) + ) # can't use eafp here because of cursor error handling + + if existing is None: + + # Not in the table yet. This should throw an IntegrityError if the + # address is verified for a different participant. + + c.run( "INSERT INTO emails (participant_id, address, nonce) VALUES (%s, %s, %s)" + , (self.id, email, nonce) + ) + else: + + # Already in the table. Restart verification. Henceforth, old links + # will fail. + + if existing.nonce: + c.run('DELETE FROM claims WHERE nonce=%s', (existing.nonce,)) + c.run(""" UPDATE emails - SET verification_start=%s + SET nonce=%s + , verification_start=now() WHERE participant_id=%s AND address=%s - AND verified IS NULL - RETURNING nonce - """, (verification_start, self.id, email)) - if not nonce: - return self.add_email(email) - - base_url = gratipay.base_url - username = self.username_lower - encoded_email = encode_for_querystring(email) - link = "{base_url}/~{username}/emails/verify.html?email2={encoded_email}&nonce={nonce}" - self.app.email_queue.put( self - , 'verification' - , email=email - , link=link.format(**locals()) - , include_unsubscribe=False - ) - if self.email_address: - self.app.email_queue.put( self - , 'verification_notice' - , new_email=email - , include_unsubscribe=False + """, (nonce, self.id, email)) - # Don't count this one against their sending quota. - # It's going to their own verified address, anyway. - , _user_initiated=False - ) + return nonce - def update_email(self, email): - """Set the email address for the participant. + def start_package_claims(self, c, nonce, *packages): + """Takes a cursor, nonce and list of packages, inserts into ``claims`` + and returns ``None`` (or raise :py:exc:`NoPackages`). """ - if not getattr(self.get_email(email), 'verified', False): - raise EmailNotVerified(email) - username = self.username - with self.db.get_cursor() as c: - self.app.add_event(c, 'participant', dict(id=self.id, action='set', values=dict(primary_email=email))) - c.run(""" - UPDATE participants - SET email_address=%(email)s - WHERE username=%(username)s - """, locals()) + if not packages: + raise NoPackages() + + # We want to make a single db call to insert all claims, so we need to + # do a little SQL construction. Do it in such a way that we still avoid + # Python string interpolation (~= SQLi vector). + + extra_sql, values = [], [] + for p in packages: + extra_sql.append('(%s, %s)') + values += [nonce, p.id] + c.run('INSERT INTO claims (nonce, package_id) VALUES' + ', '.join(extra_sql), values) + self.app.add_event( c + , 'participant' + , dict( id=self.id + , action='start-claim' + , values=dict(package_ids=[p.id for p in packages]) + ) + ) + + + def set_primary_email(self, email, cursor=None): + """Set the primary email address for the participant. + """ + if cursor: + self._set_primary_email(email, cursor) + else: + with self.db.get_cursor() as cursor: + self._set_primary_email(email, cursor) self.set_attributes(email_address=email) - def verify_email(self, email, nonce): - if '' in (email, nonce): + def _set_primary_email(self, email, cursor): + if not getattr(self.get_email(email, cursor), 'verified', False): + raise EmailNotVerified() + self.app.add_event( cursor + , 'participant' + , dict(id=self.id, action='set', values=dict(primary_email=email)) + ) + cursor.run(""" + UPDATE participants + SET email_address=%(email)s + WHERE username=%(username)s + """, dict(email=email, username=self.username)) + + + def finish_email_verification(self, email, nonce): + if '' in (email.strip(), nonce.strip()): 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.db.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)) - except IntegrityError: - return VERIFICATION_STYMIED + with self.db.get_cursor() as cursor: + record = self.get_email(email, cursor, and_lock=True) + if record is None: + return VERIFICATION_FAILED + 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(record.nonce, nonce): + return VERIFICATION_FAILED + if (utcnow() - record.verification_start) > EMAIL_HASH_TIMEOUT: + return VERIFICATION_EXPIRED + try: + self.finish_package_claims(cursor, nonce, *packages) + self.save_email_address(cursor, email) + except IntegrityError: + return VERIFICATION_STYMIED + return VERIFICATION_SUCCEEDED + + + def get_packages_claiming(self, cursor, nonce): + """Given a nonce, return :py:class:`~gratipay.models.package.Package` + objects associated with it. + """ + return cursor.all(""" + SELECT p.*::packages + FROM packages p + JOIN claims c + ON p.id = c.package_id + WHERE c.nonce=%s + """, (nonce,)) + + def save_email_address(self, cursor, address): + """Given an email address, modify the database. + """ + 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.update_email(email) - return VERIFICATION_SUCCEEDED + self.set_primary_email(address, cursor) + + 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.ensure_team(cursor, self) + package_ids.append(package.id) + self.app.add_event( cursor + , 'participant' + , dict( id=self.id + , action='finish-claim' + , values=dict(package_ids=package_ids) + ) + ) - def get_email(self, email): + + 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 self.db.one(""" - SELECT * - FROM emails - WHERE participant_id=%s - AND address=%s - """, (self.id, email)) + 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): @@ -202,7 +334,10 @@ def remove_email(self, address): if address == self.email_address: raise CannotRemovePrimaryEmail() with self.db.get_cursor() as c: - self.app.add_event(c, 'participant', dict(id=self.id, action='remove', values=dict(email=address))) + self.app.add_event( c + , 'participant' + , dict(id=self.id, action='remove', values=dict(email=address)) + ) c.run("DELETE FROM emails WHERE participant_id=%s AND address=%s", (self.id, address)) diff --git a/gratipay/models/team/__init__.py b/gratipay/models/team/__init__.py index 4f371a3d1e..24fb869b48 100644 --- a/gratipay/models/team/__init__.py +++ b/gratipay/models/team/__init__.py @@ -6,15 +6,17 @@ from decimal import Decimal import requests -from aspen import json, log -from gratipay.exceptions import InvalidTeamName +from aspen import json, log, Response from postgres.orm import Model from .available import Available from .closing import Closing from .membership import Membership +from .package import Package from .takes import Takes from .tip_migration import TipMigration +from ...exceptions import InvalidTeamName +from ...utils import canonicalize # Should have at least one letter. @@ -36,7 +38,7 @@ def slugize(name): return slug -class Team(Model, Available, Closing, Membership, Takes, TipMigration): +class Team(Model, Available, Closing, Membership, Package, Takes, TipMigration): """Represent a Gratipay team. """ @@ -70,6 +72,13 @@ def __ne__(self, other): nreceiving_from = 0 + @property + def url_path(self): + """The path part of the URL for this team on Gratipay. + """ + return '/{}/'.format(self.slug) + + # Constructors # ============ @@ -98,9 +107,10 @@ def _from_thing(cls, thing, value): @classmethod def insert(cls, owner, **fields): + cursor = fields.pop('_cursor') if '_cursor' in fields else None fields['slug_lower'] = fields['slug'].lower() fields['owner'] = owner.username - return cls.db.one(""" + return (cursor or cls.db).one(""" INSERT INTO teams (slug, slug_lower, name, homepage, @@ -306,6 +316,7 @@ def update_receiving(self, cursor=None): , ndistributing_to=r.ndistributing_to ) + @property def status(self): return { None: 'unreviewed' @@ -313,6 +324,7 @@ def status(self): , True: 'approved' }[self.is_approved] + def to_dict(self): return { 'homepage': self.homepage, @@ -367,3 +379,35 @@ def load_image(self, size): with self.db.get_connection() as c: image = c.lobject(oid, mode='rb').read() return image + + +def cast(path_part, state): + """This is an Aspen typecaster. Given a slug and a state dict, raise + Response or return Team. + """ + redirect = state['website'].redirect + request = state['request'] + user = state['user'] + slug = path_part + qs = request.line.uri.querystring + + try: + team = Team.from_slug(slug) + except: + raise Response(400, 'bad slug') + + if team is None: + # Try to redirect to a Participant. + from gratipay.models.participant import Participant # avoid circular import + participant = Participant.from_username(slug) + if participant is not None: + qs = '?' + request.qs.raw if request.qs.raw else '' + redirect('/~' + request.path.raw[1:] + qs) + raise Response(404) + + canonicalize(redirect, request.line.uri.path.raw, '/', team.slug, slug, qs) + + if team.is_closed and not user.ADMIN: + raise Response(410) + + return team diff --git a/gratipay/models/team/package.py b/gratipay/models/team/package.py new file mode 100644 index 0000000000..50de9a622a --- /dev/null +++ b/gratipay/models/team/package.py @@ -0,0 +1,22 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import, division, print_function, unicode_literals + + +class Package(object): + """A :py:class:`~gratipay.models.team.Team` can be associated with :py:class:`Package`. + """ + + @property + def package(self): + """A computed attribute, the + :py:class:`~gratipay.models.package.Package` linked to this team if + there is one, otherwise ``None``. Makes a database call. + """ + return self._load_package(self.db) + + + def _load_package(self, cursor): + 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/gratipay/testing/harness.py b/gratipay/testing/harness.py index 93c0c6f15f..295cf08ce8 100644 --- a/gratipay/testing/harness.py +++ b/gratipay/testing/harness.py @@ -17,8 +17,9 @@ from gratipay.exceptions import NoSelfTipping, NoTippee, BadAmount from gratipay.models.account_elsewhere import AccountElsewhere from gratipay.models.exchange_route import ExchangeRoute -from gratipay.models.team import Team +from gratipay.models.package import NPM, Package from gratipay.models.participant import Participant, MAX_TIP, MIN_TIP +from gratipay.models.team import Team from gratipay.security import user from gratipay.testing.vcr import use_cassette from psycopg2 import IntegrityError, InternalError @@ -199,14 +200,15 @@ def make_team(self, *a, **kw): return team - def make_package(self, package_manager='npm', name='foo', description='Foo', + def make_package(self, package_manager=NPM, name='foo', description='Foo', emails=['alice@example.com']): """Factory for packages. """ - return self.db.one( 'INSERT INTO packages (package_manager, name, description, emails) ' - 'VALUES (%s, %s, %s, %s) RETURNING *' - , (package_manager, name, description, emails) - ) + self.db.run( 'INSERT INTO packages (package_manager, name, description, emails) ' + 'VALUES (%s, %s, %s, %s) RETURNING *' + , (package_manager, name, description, emails) + ) + return Package.from_names(NPM, name) def make_participant(self, username, **kw): diff --git a/gratipay/typecasting.py b/gratipay/typecasting.py new file mode 100644 index 0000000000..8300d73cf6 --- /dev/null +++ b/gratipay/typecasting.py @@ -0,0 +1,37 @@ +# -*- coding: utf-8 -*- +"""Fork of aspen.typecasting to clean some stuff up. Upstream it, ya? +""" +from __future__ import absolute_import, division, print_function, unicode_literals + +from dependency_injection import resolve_dependencies + + +def cast(website, request, state): + """Implement typecasting (differently from stock Aspen). + + When matching paths, Aspen looks for ``/%foo/`` and then foo is a variable + with the value in the URL path, so ``/bar/`` would end up with + ``foo='bar'``. + + There's a dictionary at ``website.typecasters`` that maps variable names to + functions, dependency-injectable as with ``website.algorithm`` + (state-chain) functions. If an entry exists in ``typecasters`` for a given + path variable, then the value of ``path[part]`` is replaced with the result + of calling the function. + + Before calling your cast function, we add an additional value to the state + dict at ``path_part``: the URL path part that matched, as a string. That is + user input, so handle it carefully. It's your job to raise + ``Response(40x)`` if it's bad input. + + """ + typecasters = website.typecasters + path = request.line.uri.path + + for part in path.keys(): + if part not in typecasters: + continue + state['path_part'] = path[part] + path.popall(part) + func = typecasters[part] + path[part] = func(*resolve_dependencies(func, state).as_args) diff --git a/gratipay/utils/__init__.py b/gratipay/utils/__init__.py index 2e3b004b0e..621ae36ade 100644 --- a/gratipay/utils/__init__.py +++ b/gratipay/utils/__init__.py @@ -9,7 +9,6 @@ from aspen import Response, json from aspen.utils import to_rfc822, utcnow -from dependency_injection import resolve_dependencies from postgres.cursors import SimpleCursorBase import gratipay @@ -35,6 +34,15 @@ def dict_to_querystring(mapping): def _munge(website, request, url_prefix, fs_prefix): + """Given website and requests objects along with URL and filesystem + prefixes, redirect or modify the request. The idea here is that sometimes + for various reasons the dispatcher can't handle a mapping, so this is a + hack to rewrite the URL to help the dispatcher map to the filesystem. + + If you access the filesystem version directly through the web, we redirect + you to the URL version. If you access the URL version as desired, then we + rewrite so we can find it on the filesystem. + """ if request.path.raw.startswith(fs_prefix): to = url_prefix + request.path.raw[len(fs_prefix):] if request.qs.raw: @@ -113,35 +121,6 @@ def get_participant(state, restrict=True, resolve_unclaimed=True): return participant -def get_team(state): - """Given a Request, raise Response or return Team. - """ - redirect = state['website'].redirect - request = state['request'] - user = state['user'] - slug = request.line.uri.path['team'] - qs = request.line.uri.querystring - - from gratipay.models.team import Team # avoid circular import - team = Team.from_slug(slug) - - if team is None: - # Try to redirect to a Participant. - from gratipay.models.participant import Participant # avoid circular import - participant = Participant.from_username(slug) - if participant is not None: - qs = '?' + request.qs.raw if request.qs.raw else '' - redirect('/~' + request.path.raw[1:] + qs) - raise Response(404) - - canonicalize(redirect, request.line.uri.path.raw, '/', team.slug, slug, qs) - - if team.is_closed and not user.ADMIN: - raise Response(410) - - return team - - def encode_for_querystring(s): """Given a unicode, return a unicode that's safe for transport across a querystring. """ @@ -267,17 +246,6 @@ def to_javascript(obj): return json.dumps(obj).replace('".format(self.__class__.__name__, self) + + def __init__(self, code=400, lazy_body=None, **kw): + Response.__init__(self, code, '', **kw) + if lazy_body: + self.lazy_body = lazy_body + + def render_body(self, state): + f = self.lazy_body + self.body = f(*resolve_dependencies(f, state).as_args) diff --git a/gratipay/website.py b/gratipay/website.py index 77b9bb57b9..d0834e5166 100644 --- a/gratipay/website.py +++ b/gratipay/website.py @@ -6,10 +6,11 @@ import aspen from aspen.website import Website as BaseWebsite -from . import utils, security, version +from . import utils, security, typecasting, version from .security import authentication, csrf from .utils import erase_cookie, http_caching, i18n, set_cookie, set_version_header, timer from .renderers import csv_dump, jinja2_htmlescaped, eval_, scss +from .models import team class Website(BaseWebsite): @@ -20,6 +21,7 @@ def __init__(self, app): BaseWebsite.__init__(self) self.app = app self.version = version.get_version() + self.configure_typecasters() self.configure_renderers() # TODO Can't do remaining config here because of lingering wireup @@ -35,6 +37,10 @@ def init_even_more(self): self.monkey_patch_response() + def configure_typecasters(self): + self.typecasters['team'] = team.cast + + def configure_renderers(self): self.renderer_default = 'unspecified' # require explicit renderer, to avoid escaping bugs @@ -87,7 +93,7 @@ def modify_algorithm(self, tell_sentry): http_caching.get_etag_for_file if self.cache_static else noop, http_caching.try_to_serve_304 if self.cache_static else noop, - algorithm['apply_typecasters_to_path'], + typecasting.cast, algorithm['get_resource_for_request'], algorithm['extract_accept_from_request'], algorithm['get_response_for_resource'], diff --git a/js/gratipay/packages.js b/js/gratipay/packages.js index 675a0c4614..4fabc116d3 100644 --- a/js/gratipay/packages.js +++ b/js/gratipay/packages.js @@ -3,7 +3,7 @@ Gratipay.packages = {}; Gratipay.packages.post = function(e) { e.preventDefault(); var $this = $(this); - var action = 'add-email-and-claim-package'; + var action = 'start-verification'; var package_id = $('input[name=package_id]').val(); var email = $('input[name=email]:checked').val(); diff --git a/sql/branch.sql b/sql/branch.sql new file mode 100644 index 0000000000..4052aa0ec0 --- /dev/null +++ b/sql/branch.sql @@ -0,0 +1,17 @@ +BEGIN; + + ALTER TABLE emails ADD CONSTRAINT emails_nonce_key UNIQUE (nonce); + CREATE TABLE claims + ( nonce text NOT NULL REFERENCES emails(nonce) ON DELETE CASCADE + ON UPDATE RESTRICT + , package_id bigint NOT NULL REFERENCES packages(id) ON DELETE RESTRICT + ON UPDATE RESTRICT + , UNIQUE(nonce, package_id) + ); + + 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_close.py b/tests/py/test_close.py index b05e765358..43c68698b6 100644 --- a/tests/py/test_close.py +++ b/tests/py/test_close.py @@ -208,7 +208,7 @@ def test_clears_personal_information(self): , taking=40 ) alice.upsert_statement('en', 'not forgetting to be awesome!') - alice.add_email('alice@example.net') + alice.start_email_verification('alice@example.net') with self.db.get_cursor() as cursor: alice.clear_personal_information(cursor) diff --git a/tests/py/test_email.py b/tests/py/test_email.py index a6c7f64049..9cd85ad1db 100644 --- a/tests/py/test_email.py +++ b/tests/py/test_email.py @@ -1,14 +1,19 @@ from __future__ import absolute_import, division, print_function, unicode_literals import json +import Queue import sys +import threading +import urllib from pytest import raises from gratipay.exceptions import CannotRemovePrimaryEmail, EmailTaken, EmailNotVerified -from gratipay.exceptions import TooManyEmailAddresses, Throttled -from gratipay.testing import P +from gratipay.exceptions import TooManyEmailAddresses, Throttled, EmailAlreadyVerified +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 @@ -20,16 +25,42 @@ def setUp(self): QueuedEmailHarness.setUp(self) self.alice = self.make_participant('alice', claimed_time='now') + def add(self, participant, address, _flush=False): + participant.start_email_verification(address) + nonce = participant.get_email(address).nonce + r = participant.finish_email_verification(address, nonce) + assert r == _email.VERIFICATION_SUCCEEDED + if _flush: + self.app.email_queue.flush() + class TestEndpoints(Alice): - def hit_email_spt(self, action, address, user='alice', should_fail=False): + def hit_email_spt(self, action, address, user='alice', package_ids=[], should_fail=False): f = self.client.PxST if should_fail else self.client.POST - data = {'action': action, 'address': address} - headers = {b'HTTP_ACCEPT_LANGUAGE': b'en'} - return f('/~alice/emails/modify.json', data, auth_as=user, **headers) - def verify_email(self, email, nonce, username='alice', should_fail=False): + # Aspen's test client should really support URL-encoding POST data for + # us, but it doesn't (it only supports multipart, which I think maybe + # doesn't work because of other Aspen bugs around multiple package_id + # values in the same POST body in that case?), so let's do that + # ourselves. + + data = [ ('action', action) + , ('address', address) + ] + [('package_id', str(p)) for p in package_ids] + body = urllib.urlencode(data) + + response = f( '/~alice/emails/modify.json' + , body=body + , content_type=b'application/x-www-form-urlencoded' + , auth_as=user + , HTTP_ACCEPT_LANGUAGE=b'en' + ) + if issubclass(response.__class__, (Throttled, ProblemChangingEmail)): + response.render_body({'_': lambda a: a}) + return response + + 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) @@ -39,17 +70,16 @@ 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() - def test_participant_can_add_email(self): + def test_participant_can_start_email_verification(self): response = self.hit_email_spt('add-email', 'alice@gratipay.com') - actual = json.loads(response.body) - assert actual + assert json.loads(response.body) == 'Check your inbox for a verification link.' - def test_adding_email_sends_verification_email(self): + def test_starting_email_verification_triggers_verification_email(self): self.hit_email_spt('add-email', 'alice@gratipay.com') assert self.count_email_messages() == 1 last_email = self.get_last_email() @@ -69,7 +99,7 @@ def test_verification_email_doesnt_contain_unsubscribe(self): last_email = self.get_last_email() assert "To stop receiving" not in last_email['body_text'] - def test_adding_second_email_sends_verification_notice(self): + def test_verifying_second_email_sends_verification_notice(self): self.verify_and_change_email('alice1@example.com', 'alice2@example.com', _flush=False) assert self.count_email_messages() == 3 last_email = self.get_last_email() @@ -107,15 +137,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 @@ -124,8 +154,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): @@ -137,18 +167,26 @@ 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) - expected = 'alice@example.com' - actual = P('alice').email_address - assert expected == actual + assert self.finish_email_verification('alice@example.com', nonce).code == 200 + assert P('alice').email_address == 'alice@example.com' + + def test_empty_email_results_in_missing(self): + for empty in ('', ' '): + result = self.alice.finish_email_verification(empty, 'foobar') + assert result == _email.VERIFICATION_MISSING + + def test_empty_nonce_results_in_missing(self): + for empty in ('', ' '): + result = self.alice.finish_email_verification('foobar', empty) + assert result == _email.VERIFICATION_MISSING def test_email_verification_is_backwards_compatible(self): """Test email verification still works with unencoded email in verification link. @@ -175,17 +213,17 @@ 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 - def test_nonce_is_reused_when_resending_email(self): + def test_nonce_is_not_reused_when_resending_email(self): self.hit_email_spt('add-email', 'alice@example.com') nonce1 = self.alice.get_email('alice@example.com').nonce self.hit_email_spt('resend', 'alice@example.com') nonce2 = self.alice.get_email('alice@example.com').nonce - assert nonce1 == nonce2 + assert nonce1 != nonce2 def test_emails_page_shows_emails(self): self.verify_and_change_email('alice@example.com', 'alice@example.net') @@ -217,96 +255,128 @@ def test_remove_email(self): self.hit_email_spt('remove', 'alice@example.com') -class TestFunctions(Alice): + def test_participant_can_verify_a_package_along_with_email(self): + foo = self.make_package(name='foo', emails=['alice@gratipay.com']) + response = self.hit_email_spt( 'start-verification' + , 'alice@gratipay.com' + , package_ids=[foo.id] + ) + assert json.loads(response.body) == 'Check your inbox for a verification link.' + assert self.db.all('select package_id from claims order by package_id') == [foo.id] + + def test_participant_cant_verify_packages_with_add_email_or_resend(self): + foo = self.make_package(name='foo', emails=['alice@gratipay.com']) + for action in ('add-email', 'resend'): + assert self.hit_email_spt( action + , 'alice@gratipay.com' + , package_ids=[foo.id] + , should_fail=True + ).code == 400 + + def test_participant_can_verify_multiple_packages_along_with_email(self): + package_ids = [self.make_package(name=name, emails=['alice@gratipay.com']).id + for name in ('foo', 'bar', 'baz', 'buz')] + response = self.hit_email_spt( 'start-verification' + , 'alice@gratipay.com' + , package_ids=package_ids + ) + assert json.loads(response.body) == 'Check your inbox for a verification link.' + assert self.db.all('select package_id from claims order by package_id') == package_ids + + def test_package_verification_fails_if_email_not_listed(self): + foo = self.make_package() + response = self.hit_email_spt( 'start-verification' + , 'bob@gratipay.com' + , package_ids=[foo.id] + , should_fail=True + ) + assert response.code == 400 + assert self.db.all('select package_id from claims order by package_id') == [] + + def test_package_verification_fails_package_id_is_garbage(self): + response = self.hit_email_spt( 'start-verification' + , 'bob@gratipay.com' + , package_ids=['cheese monkey'] + , should_fail=True + ) + assert response.code == 400 + assert self.db.all('select package_id from claims order by package_id') == [] - def add_and_verify(self, participant, address): - participant.add_email('alice@gratipay.com') - nonce = participant.get_email('alice@gratipay.com').nonce - r = participant.verify_email('alice@gratipay.com', nonce) - assert r == _email.VERIFICATION_SUCCEEDED + +class TestFunctions(Alice): def test_cannot_update_email_to_already_verified(self): bob = self.make_participant('bob', claimed_time='now') - self.add_and_verify(self.alice, 'alice@gratipay.com') + self.add(self.alice, 'alice@gratipay.com') with self.assertRaises(EmailTaken): - bob.add_email('alice@gratipay.com') + 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' - def test_cannot_add_too_many_emails(self): - self.alice.add_email('alice@gratipay.com') - self.alice.add_email('alice@gratipay.net') - self.alice.add_email('alice@gratipay.org') - self.app.email_queue.flush() - self.alice.add_email('alice@gratipay.co.uk') - self.alice.add_email('alice@gratipay.io') - self.alice.add_email('alice@gratipay.co') - self.app.email_queue.flush() - self.alice.add_email('alice@gratipay.eu') - self.alice.add_email('alice@gratipay.asia') - self.alice.add_email('alice@gratipay.museum') - self.app.email_queue.flush() - self.alice.add_email('alice@gratipay.py') - with self.assertRaises(TooManyEmailAddresses): - self.alice.add_email('alice@gratipay.coop') - def test_html_escaping(self): - self.alice.add_email("foo'bar@example.com") + self.alice.start_email_verification("foo'bar@example.com") last_email = self.get_last_email() assert 'foo'bar' in last_email['body_html'] assert ''' not in last_email['body_text'] + def test_npm_package_name_is_handled_safely(self): + foo = self.make_package(name='