diff --git a/emails/signin_link.spt b/emails/signin_link.spt new file mode 100644 index 0000000000..dc4b23fbc7 --- /dev/null +++ b/emails/signin_link.spt @@ -0,0 +1,15 @@ +{{ _("Sign in to Gratipay") }} + +[---] text/html +{{ _( "Click the button below to sign in to Gratipay. " + "This link will expire in 1 hour and can only be used once.") }} +
+
+{{ _("Sign in to Gratipay") }} + +[---] text/plain + +{{ _( "Click the link below to sign in to Gratipay. " + "This link will expire in 1 hour and can only be used once.") }} + +{{ signin_link }} diff --git a/emails/signup_link.spt b/emails/signup_link.spt new file mode 100644 index 0000000000..7e8d18b5b3 --- /dev/null +++ b/emails/signup_link.spt @@ -0,0 +1,15 @@ +{{ _("Finish creating your account on Gratipay") }} + +[---] text/html +{{ _( "Click the button below to create an account on Gratipay. " + "This link will expire in 1 hour and can only be used once.") }} +
+
+{{ _("Create an account on Gratipay") }} + +[---] text/plain + +{{ _( "Click the link below to create an account on Gratipay. " + "This link will expire in 1 hour and can only be used once.") }} + +{{ signup_link }} diff --git a/gratipay/email.py b/gratipay/email.py index b851634521..4f0e1a4e22 100644 --- a/gratipay/email.py +++ b/gratipay/email.py @@ -55,13 +55,17 @@ def _have_ses(self, env): and env.aws_ses_default_region - def put(self, to, template, _user_initiated=True, **context): + def put(self, to, template, _user_initiated=True, email=None, **context): """Put an email message on the queue. - :param Participant to: the participant to send the email message to + :param Participant to: the participant to send the email message to. + In cases where an email is not linked to a participant, this can be + ``None``. :param unicode template: the name of the template to use when rendering the email, corresponding to a filename in ``emails/`` without the file extension + :param unicode email: The email address to send this message to. If not + provided, the ``to`` participant's primary email is used. :param bool _user_initiated: user-initiated emails are throttled; system-initiated messages don't count against throttling :param dict context: the values to use when rendering the template @@ -73,19 +77,57 @@ def put(self, to, template, _user_initiated=True, **context): :returns: ``None`` """ + + assert to or email # Either participant or email address required. + with self.db.get_cursor() as cursor: + participant_id = to.id if to else None cursor.run(""" INSERT INTO email_queue - (participant, spt_name, context, user_initiated) - VALUES (%s, %s, %s, %s) - """, (to.id, template, pickle.dumps(context), _user_initiated)) + (participant, + email_address, + spt_name, + context, + user_initiated) + VALUES (%s, %s, %s, %s, %s) + """, (participant_id, email, template, pickle.dumps(context), _user_initiated)) + if _user_initiated: - n = cursor.one('SELECT count(*) FROM email_queue ' - 'WHERE participant=%s AND user_initiated', (to.id,)) - if n > self.allow_up_to: + nqueued = self._get_nqueued(cursor, to, email) + if nqueued > self.allow_up_to: raise Throttled() + def _get_nqueued(self, cursor, participant, email_address): + """Returns the number of messages already queued for the given + participant or email address. Prefers participant if provided, falls + back to email_address otherwise. + + :param Participant participant: The participant to check queued messages + for. + + :param unicode email_address: The email address to check queued messages + for. + + :returns number of queued messages + """ + + if participant: + return cursor.one(""" + SELECT COUNT(*) + FROM email_queue + WHERE user_initiated + AND participant=%s + """, (participant.id, )) + else: + return cursor.one(""" + SELECT COUNT(*) + FROM email_queue + WHERE user_initiated + AND email_address=%s + """, (email_address, )) + + def flush(self): """Load messages queued for sending, and send them. """ @@ -142,22 +184,35 @@ def _prepare_email_message_for_ses(self, rec): #. ``participant.email_address``. """ - to = Participant.from_id(rec.participant) + participant = Participant.from_id(rec.participant) spt = self._email_templates[rec.spt_name] context = pickle.loads(rec.context) - context['participant'] = to - context['username'] = to.username + email = rec.email_address or participant.email_address + + # Previously, email_address was stored in the 'email' key on `context` + # and not in the `email_address` field. Let's handle that case so that + # old emails don't suffer + # + # TODO: Remove this once we're sure old emails have gone out. + email = context.get('email', email) + + if not email: + return None + + context['email'] = email + if participant: + context['participant'] = participant + context['username'] = participant.username context['button_style'] = ( "color: #fff; text-decoration:none; display:inline-block; " "padding: 0 15px; background: #396; white-space: nowrap; " "font: normal 14px/40px Arial, sans-serif; border-radius: 3px" ) context.setdefault('include_unsubscribe', True) - email = context.setdefault('email', to.email_address) - if not email: - return None - langs = i18n.parse_accept_lang(to.email_lang or 'en') + + accept_lang = (participant and participant.email_lang) or 'en' + langs = i18n.parse_accept_lang(accept_lang) locale = i18n.match_lang(langs) i18n.add_helpers_to_context(self.tell_sentry, context, locale) context['escape'] = lambda s: s @@ -172,7 +227,12 @@ def render(t, context): message = {} message['Source'] = 'Gratipay Support ' message['Destination'] = {} - message['Destination']['ToAddresses'] = ["%s <%s>" % (to.username, email)] # "Name " + if participant: + # "username " + destination = "%s <%s>" % (participant.username, email) + else: + destination = email + message['Destination']['ToAddresses'] = [destination] message['Message'] = {} message['Message']['Subject'] = {} message['Message']['Subject']['Data'] = spt['subject'].render(context).strip() diff --git a/gratipay/exceptions.py b/gratipay/exceptions.py index cece213bdc..8db8dab7ac 100644 --- a/gratipay/exceptions.py +++ b/gratipay/exceptions.py @@ -7,23 +7,23 @@ from gratipay.utils.i18n import LocalizedErrorResponse -class ProblemChangingUsername(Exception): +class ProblemWithUsername(Exception): def __str__(self): return self.msg.format(self.args[0]) -class UsernameIsEmpty(ProblemChangingUsername): +class UsernameIsEmpty(ProblemWithUsername): msg = "You need to provide a username!" -class UsernameTooLong(ProblemChangingUsername): +class UsernameTooLong(ProblemWithUsername): msg = "The username '{}' is too long." -class UsernameContainsInvalidCharacters(ProblemChangingUsername): +class UsernameContainsInvalidCharacters(ProblemWithUsername): msg = "The username '{}' contains invalid characters." -class UsernameIsRestricted(ProblemChangingUsername): +class UsernameIsRestricted(ProblemWithUsername): msg = "The username '{}' is restricted." -class UsernameAlreadyTaken(ProblemChangingUsername): +class UsernameAlreadyTaken(ProblemWithUsername): msg = "The username '{}' is already taken." diff --git a/gratipay/models/__init__.py b/gratipay/models/__init__.py index 4ae554ec56..eef0010e1d 100644 --- a/gratipay/models/__init__.py +++ b/gratipay/models/__init__.py @@ -172,12 +172,14 @@ def _check_no_team_balances(cursor): def _check_orphans(cursor): """ Finds participants that - * does not have corresponding elsewhere account + * do not have a verified email address (i.e. did not signup via email) + * do not have corresponding elsewhere account * have not been absorbed by other participant - These are broken because new participants arise from elsewhere - and elsewhere is detached only by take over which makes a note - in absorptions if it removes the last elsewhere account. + These are broken because participants without an email attached arise from + elsewhere (signup via third-party providers), and elsewhere is detached + only by take over which makes a note in absorptions if it removes the last + elsewhere account. Especially bad case is when also claimed_time is set because there must have been elsewhere account attached and used to sign in. @@ -185,10 +187,11 @@ def _check_orphans(cursor): https://github.com/gratipay/gratipay.com/issues/617 """ orphans = cursor.all(""" - select username - from participants - where not exists (select * from elsewhere where elsewhere.participant=username) - and not exists (select * from absorptions where archived_as=username) + SELECT username + FROM participants + WHERE participants.email_address IS NULL + AND NOT EXISTS (SELECT * FROM elsewhere WHERE participant=username) + AND NOT EXISTS (SELECT * FROM absorptions WHERE archived_as=username) """) assert len(orphans) == 0, "missing elsewheres: {}".format(list(orphans)) diff --git a/gratipay/models/account_elsewhere.py b/gratipay/models/account_elsewhere.py index 7fc35c64ba..2fda0079a5 100644 --- a/gratipay/models/account_elsewhere.py +++ b/gratipay/models/account_elsewhere.py @@ -15,7 +15,7 @@ import xmltodict import gratipay -from gratipay.exceptions import ProblemChangingUsername +from gratipay.exceptions import ProblemWithUsername from gratipay.security.crypto import constant_time_compare from gratipay.utils.username import safely_reserve_a_username @@ -232,7 +232,7 @@ def opt_in(self, desired_username): user.participant.set_as_claimed() try: user.participant.change_username(desired_username) - except ProblemChangingUsername: + except ProblemWithUsername: pass if user.participant.is_closed: user.participant.update_is_closed(False) diff --git a/gratipay/models/participant/__init__.py b/gratipay/models/participant/__init__.py index 90f8256c62..57de0fc07f 100644 --- a/gratipay/models/participant/__init__.py +++ b/gratipay/models/participant/__init__.py @@ -80,6 +80,28 @@ def with_random_username(cls): username = safely_reserve_a_username(cursor) return cls.from_username(username) + @classmethod + def with_email_and_username(cls, email, username): + """Return a new participant with given email and username + """ + username = username and username.strip() + + cls._validate_username(username) + + with cls.db.get_cursor() as cursor: + try: + participant = cursor.one(""" + INSERT INTO participants (username, username_lower, claimed_time) + VALUES (%s, %s, CURRENT_TIMESTAMP) + RETURNING participants.*::participants + """, (username, username.lower())) + except IntegrityError: + raise UsernameAlreadyTaken(username) + + participant.add_verified_email(email, cursor) + + return participant + @classmethod def from_id(cls, id): """Return an existing participant based on id. @@ -102,6 +124,20 @@ def from_session_token(cls, token): return participant + @classmethod + def from_email(cls, email_address): + """Return an existing participant based on email. + """ + return cls.db.one(""" + + SELECT participants.*::participants + FROM participants + JOIN emails ON emails.participant_id = participants.id + WHERE emails.address=%s + AND emails.verified IS true + + """, (email_address, )) + @classmethod def _from_thing(cls, thing, value): assert thing in ("id", "username_lower", "session_token", "api_key") @@ -483,9 +519,16 @@ def delete_elsewhere(self, platform, user_id): user_id = unicode(user_id) with self.db.get_cursor() as c: accounts = self.get_elsewhere_logins(c) - assert len(accounts) > 0 - if len(accounts) == 1 and accounts[0] == (platform, user_id): - raise LastElsewhere() + + # A user who signed up via a third-party provider might not have + # and email attached. They must maintain at least one elsewhere + # account until they provide an email. + assert self.email_address or (len(accounts) > 0) + + is_last = len(accounts) == 1 and accounts[0] == (platform, user_id) + if is_last and not self.email_address: + raise LastElsewhereAndNoEmail() + c.one(""" DELETE FROM elsewhere WHERE participant=%s @@ -826,6 +869,28 @@ def insert_into_communities(self, is_member, name, slug): """, locals()) + @classmethod + def _validate_username(cls, username): + """Raises a ``ProblemWithUsername`` exception if the username is not valid""" + + if not username: + raise UsernameIsEmpty(username) + + if len(username) > USERNAME_MAX_SIZE: + raise UsernameTooLong(username) + + if set(username) - ASCII_ALLOWED_IN_USERNAME: + raise UsernameContainsInvalidCharacters(username) + + lowercased = username.lower() + + # Don't allow any username which is the name of a + # file existing on the web_root folder. + for name in (lowercased, lowercased + '.spt'): + if name in gratipay.RESTRICTED_USERNAMES: + raise UsernameIsRestricted(username) + + def change_username(self, suggested): """Raise Response or return None. @@ -836,23 +901,10 @@ def change_username(self, suggested): # TODO: reconsider allowing unicode usernames suggested = suggested and suggested.strip() - if not suggested: - raise UsernameIsEmpty(suggested) - - if len(suggested) > USERNAME_MAX_SIZE: - raise UsernameTooLong(suggested) - - if set(suggested) - ASCII_ALLOWED_IN_USERNAME: - raise UsernameContainsInvalidCharacters(suggested) + self._validate_username(suggested) lowercased = suggested.lower() - # Don't allow any username which is the name of a - # file existing on the web_root folder. - for name in (lowercased, lowercased + '.spt'): - if name in gratipay.RESTRICTED_USERNAMES: - raise UsernameIsRestricted(suggested) - if suggested != self.username: try: # Will raise IntegrityError if the desired username is taken. @@ -864,12 +916,12 @@ def change_username(self, suggested): , values=dict(username=suggested) ) ) - actual = c.one( "UPDATE participants " - "SET username=%s, username_lower=%s " - "WHERE username=%s " - "RETURNING username, username_lower" - , (suggested, lowercased, self.username) - ) + actual = c.one(""" + UPDATE participants + SET username=%s, username_lower=%s + WHERE username=%s + RETURNING username, username_lower + """, (suggested, lowercased, self.username)) except IntegrityError: raise UsernameAlreadyTaken(suggested) @@ -1387,7 +1439,7 @@ def __nonzero__(self): A, B, C = self._all return A or C -class LastElsewhere(Exception): pass +class LastElsewhereAndNoEmail(Exception): pass class NonexistingElsewhere(Exception): pass diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py index 97864d4153..c4d261f93b 100644 --- a/gratipay/models/participant/email.py +++ b/gratipay/models/participant/email.py @@ -190,6 +190,15 @@ def get_email_verification_nonce(self, c, email): return nonce + def add_verified_email(self, email, cursor): + cursor.run(""" + INSERT INTO emails (participant_id, address, verified) + VALUES (%s, %s, true) + """, (self.id, email)) + + if not self.email_address: + self.set_primary_email(email, cursor) + def set_primary_email(self, email, cursor=None): """Set the primary email address for the participant. diff --git a/gratipay/security/authentication/__init__.py b/gratipay/security/authentication/__init__.py new file mode 100644 index 0000000000..d02742db1b --- /dev/null +++ b/gratipay/security/authentication/__init__.py @@ -0,0 +1,4 @@ +"""Gratipay authentication module. +""" + +from __future__ import absolute_import, division, print_function, unicode_literals diff --git a/gratipay/security/authentication/email.py b/gratipay/security/authentication/email.py new file mode 100644 index 0000000000..899e86c499 --- /dev/null +++ b/gratipay/security/authentication/email.py @@ -0,0 +1,60 @@ +import datetime +import uuid + +from aspen.utils import utcnow + + +class VerificationResult(object): + def __init__(self, name): + self.name = name + + def __repr__(self): + return "" % self.name + __str__ = __repr__ + +#: Signal that the nonce doesn't exist in our database +NONCE_INVALID = VerificationResult('Invalid') + +#: Signal that the nonce exists, but has expired +NONCE_EXPIRED = VerificationResult('Expired') + +#: Signal that the nonce exists, and is valid +NONCE_VALID = VerificationResult('Valid') + +#: Time for nonce to expire +NONCE_EXPIRY_MINUTES = 60 + + +def create_nonce(db, email_address): + nonce = str(uuid.uuid4()) + db.run(""" + INSERT INTO email_auth_nonces (email_address, nonce) + VALUES (%(email_address)s, %(nonce)s) + """, locals()) + + return nonce + + +def verify_nonce(db, email_address, nonce): + record = db.one(""" + SELECT email_address, ctime + FROM email_auth_nonces + WHERE nonce = %(nonce)s + AND email_address = %(email_address)s + """, locals(), back_as=dict) + + if not record: + return NONCE_INVALID + + if utcnow() - record['ctime'] > datetime.timedelta(minutes=NONCE_EXPIRY_MINUTES): + return NONCE_EXPIRED + + return NONCE_VALID + + +def invalidate_nonce(db, email_address, nonce): + db.run(""" + DELETE FROM email_auth_nonces + WHERE nonce = %(nonce)s + AND email_address = %(email_address)s + """, locals()) diff --git a/gratipay/security/authentication.py b/gratipay/security/authentication/website_helpers.py similarity index 99% rename from gratipay/security/authentication.py rename to gratipay/security/authentication/website_helpers.py index 7dee71e79e..80801f8b7b 100644 --- a/gratipay/security/authentication.py +++ b/gratipay/security/authentication/website_helpers.py @@ -9,7 +9,6 @@ from gratipay.security.crypto import constant_time_compare from gratipay.security.user import User, SESSION - ANON = User() def _get_user_via_api_key(api_key): @@ -56,11 +55,6 @@ def _turn_off_csrf(request): request.headers.cookie['csrf_token'] = csrf_token request.headers['X-CSRF-TOKEN'] = csrf_token -def start_user_as_anon(): - """Make sure we always have a user object, regardless of exceptions during authentication. - """ - return {'user': ANON} - def authenticate_user_if_possible(request, user): """This signs the user in. """ @@ -86,3 +80,8 @@ def add_auth_to_response(response, request=None, user=ANON): if SESSION in request.headers.cookie: if not user.ANON: user.keep_signed_in(response.headers.cookie) + +def start_user_as_anon(): + """Make sure we always have a user object, regardless of exceptions during authentication. + """ + return {'user': ANON} diff --git a/gratipay/website.py b/gratipay/website.py index 5fb41c000f..fc5e4eddde 100644 --- a/gratipay/website.py +++ b/gratipay/website.py @@ -8,7 +8,8 @@ from aspen.website import Website as BaseWebsite from . import utils, security, typecasting, version -from .security import authentication, csrf +from .security import csrf +from .security.authentication import website_helpers as auth_helpers 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 @@ -86,8 +87,8 @@ def modify_algorithm(self, tell_sentry): utils.use_tildes_for_participants, algorithm['redirect_to_base_url'], i18n.set_up_i18n, - authentication.start_user_as_anon, - authentication.authenticate_user_if_possible, + auth_helpers.start_user_as_anon, + auth_helpers.authenticate_user_if_possible, security.only_allow_certain_methods, csrf.extract_token_from_cookie, csrf.reject_forgeries, @@ -106,7 +107,7 @@ def modify_algorithm(self, tell_sentry): algorithm['get_response_for_exception'], set_version_header, - authentication.add_auth_to_response, + auth_helpers.add_auth_to_response, csrf.add_token_to_response, http_caching.add_caching_to_response, security.add_headers_to_response, diff --git a/js/gratipay.js b/js/gratipay.js index 38f0ccb3a8..f3b9e9fb2d 100644 --- a/js/gratipay.js +++ b/js/gratipay.js @@ -14,7 +14,7 @@ Gratipay.init = function() { Gratipay.warnOffUsersFromDeveloperConsole(); Gratipay.adaptToLongUsernames(); Gratipay.forms.initCSRF(); - Gratipay.signIn.wireUpButton(); + Gratipay.signIn.wireUp(); Gratipay.signOut(); Gratipay.payments.initSupportGratipay(); Gratipay.tabs.init(); diff --git a/js/gratipay/sign-in.js b/js/gratipay/sign-in.js index a068b8b9d3..1a56c75b0e 100644 --- a/js/gratipay/sign-in.js +++ b/js/gratipay/sign-in.js @@ -1,17 +1,39 @@ Gratipay.signIn = {}; +Gratipay.signIn.wireUp = function() { + Gratipay.signIn.wireUpButton(); + Gratipay.signIn.wireUpEmailInput(); +} + Gratipay.signIn.wireUpButton = function() { $('.sign-in button').click(Gratipay.signIn.openSignInOrSignUpModal); } -Gratipay.signIn.openSignInToContinueModal = function () { +Gratipay.signIn.wireUpEmailInput = function() { + $('#sign-in-modal form.email-form').submit(function(e) { + e.preventDefault(); + jQuery.ajax({ + url: '/auth/send-link.json', + type: 'POST', + data: { + 'email_address': $(this).find('input').val() + }, + success: function(data) { + Gratipay.notification(data.message, 'success'); + }, + error: Gratipay.error + }); + }); +} + +Gratipay.signIn.openSignInToContinueModal = function() { Gratipay.signIn.replaceTextInModal('sign-in-to-continue'); - Gratipay.modal.open('#sign-in-modal'); + Gratipay.signIn.openModalAndFocusInput(); } -Gratipay.signIn.openSignInOrSignUpModal = function () { +Gratipay.signIn.openSignInOrSignUpModal = function() { Gratipay.signIn.replaceTextInModal('sign-in-or-sign-up'); - Gratipay.modal.open('#sign-in-modal'); + Gratipay.signIn.openModalAndFocusInput(); } Gratipay.signIn.replaceTextInModal = function(dataKey) { @@ -20,3 +42,8 @@ Gratipay.signIn.replaceTextInModal = function(dataKey) { $(this).text(textToReplace); }); } + +Gratipay.signIn.openModalAndFocusInput = function() { + Gratipay.modal.open('#sign-in-modal'); + $('#sign-in-modal input').focus(); +} diff --git a/js/gratipay/sign-up.js b/js/gratipay/sign-up.js new file mode 100644 index 0000000000..89922396a9 --- /dev/null +++ b/js/gratipay/sign-up.js @@ -0,0 +1,24 @@ +Gratipay.signUp = {}; + +Gratipay.signUp.wireUp = function() { + $('.signup-form').submit(function(e) { + e.preventDefault(); + jQuery.ajax({ + url: '/auth/signup.json', + type: 'POST', + data: { + 'email': $(this).find('input[name="email"]').val(), + 'username': $(this).find('input[name="username"]').val(), + 'nonce': $(this).find('input[name="nonce"]').val(), + }, + success: function(data) { + Gratipay.notification(data.message, 'success'); + + // Let's reload the verification page, so that the + // user is signed in + setTimeout(function() { window.location.reload() }, 1000); + }, + error: Gratipay.error + }); + }) +} diff --git a/scss/components/modal.scss b/scss/components/modal.scss index 8141681cb9..7bd59836f3 100644 --- a/scss/components/modal.scss +++ b/scss/components/modal.scss @@ -46,7 +46,7 @@ text-align: center; display: none; background: transparentize($light-brown, 0.3); - z-index: 1000; + z-index: 900; } .modal { diff --git a/scss/components/sign_in.scss b/scss/components/sign_in.scss index ff16a87fed..2fc7d09856 100644 --- a/scss/components/sign_in.scss +++ b/scss/components/sign_in.scss @@ -58,8 +58,15 @@ } } + .email-form input { + color: $black; + height: auto; + padding: 10px; + } + .auth-links { margin-top: 5px; + margin-bottom: 10px; } .auth-links li { @@ -71,14 +78,27 @@ } } - .auth-links button { + .auth-links button, .email-form button { text-align: left; background: $green; color: white; - padding: 15px 45px 15px 15px; position: relative; font: normal 14px $Ideal; + &:hover { + background: $darker-green; + } + } + + .email-form button { + padding: 15px 15px 15px 15px; + margin: 15px 0px 20px 0px; + text-align: center; + } + + .auth-links button { + padding: 15px 45px 15px 15px; // 30px for icon on the right + span { vertical-align: middle; } @@ -95,10 +115,6 @@ span.icon.github { @include has-icon("github"); } span.icon.openstreetmap { @include has-icon("openstreetmap"); } span.icon.bitbucket { @include has-icon("bitbucket"); } - - &:hover { - background: $darker-green; - } } } diff --git a/scss/elements/elements.scss b/scss/elements/elements.scss index af16b1de24..19bbf609f3 100644 --- a/scss/elements/elements.scss +++ b/scss/elements/elements.scss @@ -88,12 +88,12 @@ textarea { } input { - &[type="text"], &[type="number"], &[type="password"] { + &[type="text"], &[type="number"], &[type="password"], &[type="email"] { @include border-radius(3px); - height: 22px; border: 1px solid $brown; padding: 0 4px; line-height: 22px; + box-shadow: 0px 0px 2px $light-gray; } &[type="number"] { @@ -101,6 +101,14 @@ input { } } +input.large { + &[type="text"], &[type="number"], &[type="password"], &[type="email"] { + padding: 10px; + border: 1px solid $brown; + line-height: 22px; + } +} + table.simple { margin: 0 auto 1em; th, td { diff --git a/scss/pages/signup.scss b/scss/pages/signup.scss new file mode 100644 index 0000000000..819c4bd914 --- /dev/null +++ b/scss/pages/signup.scss @@ -0,0 +1,16 @@ +.signup-form { + text-align: center; + + p { + margin-bottom: 2em; + } + + button { + background: $green; + color: white; + + &:hover { + background: $darker-green; + } + } +} diff --git a/sql/branch.sql b/sql/branch.sql new file mode 100644 index 0000000000..5d5bc9e36e --- /dev/null +++ b/sql/branch.sql @@ -0,0 +1,20 @@ +BEGIN; + -- In some cases, we don't have a participant linked to emails + ALTER TABLE email_queue ALTER COLUMN participant DROP NOT NULL; + + -- Email address to send emails to. If not provided, participant's primary email will be used. + ALTER TABLE email_queue ADD COLUMN email_address text; + + ALTER TABLE email_queue ADD CONSTRAINT email_or_participant_required + CHECK ((participant IS NOT NULL) OR (email_address IS NOT NULL)); +END; + +BEGIN; + CREATE TABLE email_auth_nonces + ( id serial PRIMARY KEY + , email_address text NOT NULL + , nonce text NOT NULL + , ctime timestamp with time zone NOT NULL DEFAULT CURRENT_TIMESTAMP + , UNIQUE (nonce) + ); +END; diff --git a/templates/nonce-verification-failed.html b/templates/nonce-verification-failed.html new file mode 100644 index 0000000000..ee1701fa52 --- /dev/null +++ b/templates/nonce-verification-failed.html @@ -0,0 +1,16 @@ +{% if result == NONCE_EXPIRED %} +

{{ _("Link expired") }}

+

{{ _( "This link has expired. Please generate a new one.") }}

+ {# TODO: Add form for email right here? #} +{% else %} {# NONCE_INVALID #} +

{{ _("Bad Info") }}

+

+ {{ _( "Sorry, that's a bad link.") }} + +

+ + {{ _("If you think this is a mistake, please contact {a}support@gratipay.com.{_a}" + , a=(''|safe) + , _a=''|safe) }} +

+{% endif %} diff --git a/templates/sign-in-modal.html b/templates/sign-in-modal.html index 065254129c..7f60e4da7a 100644 --- a/templates/sign-in-modal.html +++ b/templates/sign-in-modal.html @@ -27,9 +27,21 @@
+ + + +

+ {{ _('Alternatively, use a third-party provider:') }}

    {% for platform in website.signin_platforms %} diff --git a/tests/py/security/authentication/test_email_signin.py b/tests/py/security/authentication/test_email_signin.py new file mode 100644 index 0000000000..4e16a697d2 --- /dev/null +++ b/tests/py/security/authentication/test_email_signin.py @@ -0,0 +1,73 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import, division, print_function, unicode_literals +from uuid import UUID + +from gratipay.security.authentication.email import create_nonce, verify_nonce, invalidate_nonce +from gratipay.security.authentication.email import NONCE_VALID, NONCE_INVALID, NONCE_EXPIRED +from gratipay.testing import Harness + +class TestCreateNonce(Harness): + + def setUp(self): + alice = self.make_participant('alice') + self.add_and_verify_email(alice, 'alice@gratipay.com') + + def _fetch_rec_from_db(self, nonce): + return self.db.one("SELECT * FROM email_auth_nonces WHERE nonce = %s", (nonce, ), back_as=dict) + + def test_inserts_into_db(self): + nonce = create_nonce(self.db, 'alice@gratipay.com') + rec = self._fetch_rec_from_db(nonce) + + assert rec['nonce'] == nonce + assert rec['email_address'] == 'alice@gratipay.com' + + def test_nonce_is_valid_uuid(self): + nonce = create_nonce(self.db, 'alice@gratipay.com') + + assert UUID(nonce, version=4).__class__ == UUID + + +class TestVerifyNonce(Harness): + + def setUp(self): + alice = self.make_participant('alice') + self.add_and_verify_email(alice, 'alice@gratipay.com') + + def test_valid_nonce(self): + nonce = create_nonce(self.db, 'alice@gratipay.com') + assert verify_nonce(self.db, 'alice@gratipay.com', nonce) == NONCE_VALID + + def test_expired_nonce(self): + nonce = create_nonce(self.db, 'alice@gratipay.com') + self.db.run("UPDATE email_auth_nonces SET ctime = ctime - interval '1 day'") + assert verify_nonce(self.db, 'alice@gratipay.com', nonce) == NONCE_EXPIRED + + def test_invalid_nonce(self): + create_nonce(self.db, 'alice@gratipay.com') + assert verify_nonce(self.db, 'alice@gratipay.com', "dummy_nonce") == NONCE_INVALID + +class TestInvalidateNonce(Harness): + + def setUp(self): + alice = self.make_participant('alice') + self.add_and_verify_email(alice, 'alice@gratipay.com') + + def test_deletes_nonce(self): + nonce = create_nonce(self.db, 'alice@gratipay.com') + invalidate_nonce(self.db, 'alice@gratipay.com', nonce) + + assert verify_nonce(self.db, 'alice@gratipay.com', nonce) == NONCE_INVALID + + def test_only_deletes_one_nonce(self): + nonce1 = create_nonce(self.db, 'alice@gratipay.com') + nonce2 = create_nonce(self.db, 'alice@gratipay.com') + invalidate_nonce(self.db, 'alice@gratipay.com', nonce1) + + assert verify_nonce(self.db, 'alice@gratipay.com', nonce1) == NONCE_INVALID + assert verify_nonce(self.db, 'alice@gratipay.com', nonce2) == NONCE_VALID + + def test_tolerates_invalidated_nonce(self): + nonce = create_nonce(self.db, 'alice@gratipay.com') + invalidate_nonce(self.db, 'alice@gratipay.com', nonce) + invalidate_nonce(self.db, 'alice@gratipay.com', nonce) # Should not throw an error diff --git a/tests/py/test_email.py b/tests/py/test_email.py index c162824f27..3fefbb0488 100644 --- a/tests/py/test_email.py +++ b/tests/py/test_email.py @@ -1,364 +1,63 @@ from __future__ import absolute_import, division, print_function, unicode_literals -import json -import Queue -import sys -import threading import time -import urllib import mock from pytest import raises -from gratipay.exceptions import CannotRemovePrimaryEmail, EmailTaken, EmailNotVerified -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 NPM, 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 +from gratipay.exceptions import Throttled +from gratipay.testing import Harness +from gratipay.testing.email import SentEmailHarness, QueuedEmailHarness -class Alice(QueuedEmailHarness): +class TestPut(QueuedEmailHarness): 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 - result = participant.finish_email_verification(address, nonce) - assert result == (_email.VERIFICATION_SUCCEEDED, [], None) - if _flush: - self.app.email_queue.flush() - - -class TestEndpoints(Alice): - - 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 - - # 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 hit_verify_spt(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) - f = self.client.GxT if should_fail else self.client.GET - return f(url, auth_as=username) - - 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.hit_verify_spt(old_email, nonce) - self.hit_email_spt('add-email', new_email) - if _flush: - self.app.email_queue.flush() - - def test_participant_can_start_email_verification(self): - response = self.hit_email_spt('add-email', 'alice@gratipay.com') - assert json.loads(response.body) == 'Check your inbox for a verification link.' - - 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() - assert last_email['to'] == 'alice ' - expected = "We've received a request to connect alice@gratipay.com to the alice account" - assert expected in last_email['body_text'] + self.alice = self.make_participant('alice', claimed_time='now', email_address='alice@example.com') - def test_email_address_is_encoded_in_sent_verification_link(self): - address = 'alice@gratipay.com' - encoded = encode_for_querystring(address) - self.hit_email_spt('add-email', address) + def test_put_with_only_email(self): + self.app.email_queue.put(None, 'base', email='dummy@example.com') last_email = self.get_last_email() - assert "~alice/emails/verify.html?email2="+encoded in last_email['body_text'] + assert last_email['to'] == 'dummy@example.com' - def test_verification_email_doesnt_contain_unsubscribe(self): - self.hit_email_spt('add-email', 'alice@gratipay.com') - last_email = self.get_last_email() - assert "To stop receiving" not in last_email['body_text'] + def test_put_with_only_participant(self): + self.app.email_queue.put(self.alice, 'base') - 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() - self.app.email_queue.flush() - assert last_email['to'] == 'alice ' - expected = "We are connecting alice2@example.com to the alice account on Gratipay" - assert expected in last_email['body_text'] + # Should pickup primary email + assert self.get_last_email()['to'] == 'alice ' - def test_post_anon_returns_401(self): - response = self.hit_email_spt('add-email', 'anon@example.com', user=None, should_fail=True) - assert response.code == 401 - - def test_post_with_no_at_symbol_is_400(self): - response = self.hit_email_spt('add-email', 'gratipay.com', should_fail=True) - assert response.code == 400 - - def test_post_with_no_period_symbol_is_400(self): - response = self.hit_email_spt('add-email', 'test@gratipay', should_fail=True) - assert response.code == 400 - - def test_post_with_long_address_is_okay(self): - response = self.hit_email_spt('add-email', ('a'*242) + '@example.com') - assert response.code == 200 - - def test_post_with_looooong_address_is_400(self): - response = self.hit_email_spt('add-email', ('a'*243) + '@example.com', should_fail=True) - assert response.code == 400 - - def test_post_too_quickly_is_400(self): - self.hit_email_spt('add-email', 'alice@example.com') - self.hit_email_spt('add-email', 'alice+a@example.com') - self.hit_email_spt('add-email', 'alice+b@example.com') - response = self.hit_email_spt('add-email', 'alice+c@example.com', should_fail=True) - assert response.code == 400 - assert 'too quickly' in response.body - - def test_verify_email_without_adding_email(self): - response = self.hit_verify_spt('', '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' - result = self.alice.finish_email_verification('alice@gratipay.com', nonce) - assert result == (_email.VERIFICATION_FAILED, None, None) - self.hit_verify_spt('alice@example.com', nonce) - expected = None - actual = P('alice').email_address - assert expected == actual - - 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 - self.alice.finish_email_verification(address, nonce) - result = self.alice.finish_email_verification(address, nonce) - assert result == (_email.VERIFICATION_REDUNDANT, None, None) - - def test_verify_email_expired_nonce_fails(self): - address = 'alice@example.com' - self.hit_email_spt('add-email', address) - self.db.run(""" - UPDATE emails - SET verification_start = (now() - INTERVAL '25 hours') - WHERE participant_id = %s; - """, (self.alice.id,)) - nonce = self.alice.get_email(address).nonce - result = self.alice.finish_email_verification(address, nonce) - assert result == (_email.VERIFICATION_FAILED, None, None) - actual = P('alice').email_address - assert actual == None - - def test_finish_email_verification(self): - self.hit_email_spt('add-email', 'alice@example.com') - nonce = self.alice.get_email('alice@example.com').nonce - assert self.hit_verify_spt('alice@example.com', nonce).code == 200 - assert P('alice').email_address == 'alice@example.com' - - def test_empty_email_fails(self): - for empty in ('', ' '): - result = self.alice.finish_email_verification(empty, 'foobar') - 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, None) - - def test_email_verification_is_backwards_compatible(self): - """Test email verification still works with unencoded email in verification link. - """ - self.hit_email_spt('add-email', 'alice@example.com') - nonce = self.alice.get_email('alice@example.com').nonce - url = '/~alice/emails/verify.html?email=alice@example.com&nonce='+nonce - self.client.GET(url, auth_as='alice') - expected = 'alice@example.com' - actual = P('alice').email_address - assert expected == actual - - def test_verified_email_is_not_changed_after_update(self): - self.verify_and_change_email('alice@example.com', 'alice@example.net') - expected = 'alice@example.com' - actual = P('alice').email_address - assert expected == actual - - def test_get_emails(self): - self.verify_and_change_email('alice@example.com', 'alice@example.net') - emails = self.alice.get_emails() - assert len(emails) == 2 - - 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.hit_verify_spt('alice@example.net', nonce) - expected = 'alice@example.com' - actual = P('alice').email_address - assert expected == actual - - 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 - - def test_emails_page_shows_emails(self): - self.verify_and_change_email('alice@example.com', 'alice@example.net') - body = self.client.GET("/~alice/emails/", auth_as="alice").body - assert 'alice@example.com' in body - assert 'alice@example.net' in body - - def test_set_primary(self): - self.verify_and_change_email('alice@example.com', 'alice@example.net') - self.verify_and_change_email('alice@example.net', 'alice@example.org') - self.hit_email_spt('set-primary', 'alice@example.com') - - def test_cannot_set_primary_to_unverified(self): - with self.assertRaises(EmailNotVerified): - self.hit_email_spt('set-primary', 'alice@example.com') - - def test_remove_email(self): - # Can remove unverified - self.hit_email_spt('add-email', 'alice@example.com') - self.hit_email_spt('remove', 'alice@example.com') - - # Can remove verified - self.verify_and_change_email('alice@example.com', 'alice@example.net') - self.verify_and_change_email('alice@example.net', 'alice@example.org') - self.hit_email_spt('remove', 'alice@example.net') - - # Cannot remove primary - with self.assertRaises(CannotRemovePrimaryEmail): - self.hit_email_spt('remove', 'alice@example.com') - - - 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_if_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 test_package_reverification_succeeds_if_package_is_already_claimed_by_self(self): - foo = self.make_package() - self.claim_package('alice', foo) - response = self.hit_email_spt( 'start-verification' - , 'alice@example.com' - , package_ids=[foo.id] - ) - assert response.code == 200 - - def test_package_verification_fails_if_package_is_already_claimed_by_other(self): - self.make_participant('bob', claimed_time='now', email_address='bob@example.com') - foo = self.make_package(emails=['alice@example.com', 'bob@example.com']) - self.claim_package('bob', foo) - response = self.hit_email_spt( 'start-verification' - , 'alice@example.com' - , package_ids=[foo.id] - , should_fail=True - ) - assert response.code == 400 - - -class TestFunctions(Alice): - - def test_cannot_update_email_to_already_verified(self): - bob = self.make_participant('bob', claimed_time='now') - self.add(self.alice, 'alice@gratipay.com') - - with self.assertRaises(EmailTaken): - bob.start_email_verification('alice@gratipay.com') - nonce = bob.get_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_html_escaping(self): - 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_put_with_participant_and_email(self): + self.app.email_queue.put(self.alice, 'base', email='alice2@example.com') - def test_npm_package_name_is_handled_safely(self): - foo = self.make_package(name=' + {{ super() }} +{% endblock %} diff --git a/www/~/%username/delete-elsewhere.json.spt b/www/~/%username/delete-elsewhere.json.spt index f338e6eeed..6976480893 100644 --- a/www/~/%username/delete-elsewhere.json.spt +++ b/www/~/%username/delete-elsewhere.json.spt @@ -1,5 +1,5 @@ from aspen import Response -from gratipay.models.participant import LastElsewhere, NonexistingElsewhere +from gratipay.models.participant import LastElsewhereAndNoEmail, NonexistingElsewhere [-----------------------------------------------------------------------------] if user.ANON: raise Response(403) @@ -10,10 +10,16 @@ user_id = request.body["user_id"] try: user.participant.delete_elsewhere(platform, user_id) -except LastElsewhere: - raise Response(400, "Cannot delete last login account. If you want to merge this login account to a different Gratipay account: log out of this account; log into the other account using another method; and then add this authentication provider to it. The two Gratipay accounts will be merged.") +except LastElsewhereAndNoEmail: + msg = _("Cannot delete last login account since you don't have an email " + "address attached. If you want to merge this login account to a " + "different Gratipay account: log out of this account; log into the " + "other account using another method; and then add this " + "authentication provider to it. The two Gratipay accounts " + "will be merged.") + raise Response(400, msg) except NonexistingElsewhere: - raise Response(400, "Account does not exist.") + raise Response(400, _("Account does not exist.")) [---] application/json via json_dump { "msg": "OK" } diff --git a/www/~/%username/username.json.spt b/www/~/%username/username.json.spt index 89a24d7fc2..2c9fc247af 100644 --- a/www/~/%username/username.json.spt +++ b/www/~/%username/username.json.spt @@ -1,5 +1,5 @@ from aspen import Response, log_dammit -from gratipay.exceptions import ProblemChangingUsername +from gratipay.exceptions import ProblemWithUsername from gratipay.models.participant import Participant from gratipay.utils import get_participant @@ -12,7 +12,7 @@ try: old_username = participant.username new_username = participant.change_username(request.body['username']) log_dammit("user with username %s has become username %s" % (old_username, new_username)) -except ProblemChangingUsername, e: +except ProblemWithUsername, e: raise Response(400, unicode(e)) [---] application/json via json_dump