Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Commit

Permalink
Clean up error handling
Browse files Browse the repository at this point in the history
- use LocalizedErrorResponse to deduplicate error messages
- don't include email address in error message to reduce spoofing
  surface
  • Loading branch information
chadwhitacre committed Apr 3, 2017
1 parent 86ccce4 commit 2e979de
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 35 deletions.
30 changes: 18 additions & 12 deletions gratipay/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from __future__ import print_function, unicode_literals

from aspen import Response
from gratipay.utils.i18n import LocalizedErrorResponse


class ProblemChangingUsername(Exception):
Expand All @@ -27,31 +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):
msg = "The email address '{}' is not on file for this package."
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):
Expand Down
10 changes: 5 additions & 5 deletions gratipay/models/participant/email.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ 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(email)
raise EmailNotOnFile()

owner_id = c.one("""
SELECT participant_id
Expand All @@ -98,17 +98,17 @@ def validate_email_verification_request(self, c, email, *packages):

if owner_id:
if owner_id != self.id:
raise EmailTaken(email)
raise EmailTaken()
elif packages:
pass # allow reverify if claiming packages
else:
raise EmailAlreadyVerified(email)
raise EmailAlreadyVerified()

if len(self.get_emails()) > 9:
if owner_id and owner_id == self.id and packages:
pass # they're using an already-verified email to verify packages
else:
raise TooManyEmailAddresses(email)
raise TooManyEmailAddresses()


def get_email_verification_link(self, c, email, *packages):
Expand Down Expand Up @@ -197,7 +197,7 @@ def update_email(self, email):
"""Set the email address for the participant.
"""
if not getattr(self.get_email(email), 'verified', False):
raise EmailNotVerified(email)
raise EmailNotVerified()
username = self.username
with self.db.get_cursor() as c:
self.app.add_event( c
Expand Down
8 changes: 6 additions & 2 deletions gratipay/utils/i18n.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,13 @@ def set_locale():

class LocalizedErrorResponse(Response):

def __init__(self, code, lazy_body, **kw):
def __repr__(self):
return "<{}: {}>".format(self.__class__.__name__, self)

def __init__(self, code=400, lazy_body=None, **kw):
Response.__init__(self, code, '', **kw)
self.lazy_body = lazy_body
if lazy_body:
self.lazy_body = lazy_body

def render_body(self, state):
f = self.lazy_body
Expand Down
7 changes: 5 additions & 2 deletions tests/py/test_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from gratipay.exceptions import CannotRemovePrimaryEmail, EmailTaken, EmailNotVerified
from gratipay.exceptions import TooManyEmailAddresses, Throttled, EmailAlreadyVerified
from gratipay.exceptions import EmailNotOnFile
from gratipay.exceptions import EmailNotOnFile, ProblemChangingEmail
from gratipay.testing import P, Harness
from gratipay.testing.email import QueuedEmailHarness, SentEmailHarness
from gratipay.models.participant import email as _email
Expand Down Expand Up @@ -36,7 +36,10 @@ def hit_email_spt(self, action, address, user='alice', 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)
response = f('/~alice/emails/modify.json', data, auth_as=user, **headers)
if issubclass(response.__class__, (Throttled, ProblemChangingEmail)):
response.render_body({'_': lambda a: a})
return response

def verify_email(self, email, nonce, username='alice', should_fail=False):
# Email address is encoded in url.
Expand Down
16 changes: 2 additions & 14 deletions www/~/%username/emails/modify.json.spt
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,8 @@ if not participant.email_lang:

msg = None
if action in ('add-email', 'resend'):
try:
participant.start_email_verification(address)
except EmailTaken:
raise Response(400, _( "{email_address} is already linked to a different Gratipay account."
, email_address=address
))
except EmailAlreadyVerified:
raise Response(400, _( "You have already added and verified {email_address}."
, email_address=address
))
except Throttled:
raise Response(400, _("You've initiated too many emails too quickly. Please try again in a minute or two."))
else:
msg = _("A verification email has been sent to {email_address}.", email_address=address)
participant.start_email_verification(address)
msg = _("Check your inbox for a verification email.")
elif action == 'set-primary':
participant.update_email(address)
elif action == 'remove':
Expand Down

0 comments on commit 2e979de

Please sign in to comment.