From 5dde39eb371b4253a55042002337d840e1997be2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Thu, 21 Apr 2022 18:40:24 +0200 Subject: [PATCH 01/13] Prevent free users from creating reverse-alias --- app/api/views/alias.py | 55 ++----- app/dashboard/views/alias_contact_manager.py | 134 ++++++++++-------- app/errors.py | 35 ++++- app/models.py | 11 ++ ...2_050317_98040e190381_add_flags_to_user.py | 29 ++++ pytest.ini | 2 +- .../dashboard/alias_contact_manager.html | 6 +- tests/api/test_alias.py | 58 +++++++- 8 files changed, 224 insertions(+), 106 deletions(-) create mode 100644 migrations/versions/2022_050317_98040e190381_add_flags_to_user.py diff --git a/app/api/views/alias.py b/app/api/views/alias.py index 8c1a4575..eef24ba1 100644 --- a/app/api/views/alias.py +++ b/app/api/views/alias.py @@ -1,6 +1,4 @@ from deprecated import deprecated -from flanker.addresslib import address -from flanker.addresslib.address import EmailAddress from flask import g from flask import jsonify from flask import request @@ -17,15 +15,16 @@ from app.api.serializer import ( get_alias_info_v2, get_alias_infos_with_pagination_v3, ) +from app.dashboard.views.alias_contact_manager import create_contact from app.dashboard.views.alias_log import get_alias_log from app.db import Session -from app.email_utils import ( - generate_reply_email, +from app.errors import ( + CannotCreateContactForReverseAlias, + ErrContactErrorUpgradeNeeded, + ErrContactAlreadyExists, + ErrAddressInvalid, ) -from app.errors import CannotCreateContactForReverseAlias -from app.log import LOG from app.models import Alias, Contact, Mailbox, AliasMailbox -from app.utils import sanitize_email @deprecated @@ -407,50 +406,26 @@ def create_contact_route(alias_id): Output: 201 if success 409 if contact already added - - """ data = request.get_json() if not data: return jsonify(error="request body cannot be empty"), 400 - user = g.user alias: Alias = Alias.get(alias_id) - if alias.user_id != user.id: + if alias.user_id != g.user.id: return jsonify(error="Forbidden"), 403 - contact_addr = data.get("contact") - - if not contact_addr: - return jsonify(error="Contact cannot be empty"), 400 - - full_address: EmailAddress = address.parse(contact_addr) - if not full_address: - return jsonify(error=f"invalid contact email {contact_addr}"), 400 - - contact_name, contact_email = full_address.display_name, full_address.address - - contact_email = sanitize_email(contact_email, not_lower=True) - - # already been added - contact = Contact.get_by(alias_id=alias.id, website_email=contact_email) - if contact: - return jsonify(**serialize_contact(contact, existed=True)), 200 + contact_address = data.get("contact") try: - contact = Contact.create( - user_id=alias.user_id, - alias_id=alias.id, - website_email=contact_email, - name=contact_name, - reply_email=generate_reply_email(contact_email, user), - ) - except CannotCreateContactForReverseAlias: - return jsonify(error="You can't create contact for a reverse alias"), 400 - - LOG.d("create reverse-alias for %s %s", contact_addr, alias) - Session.commit() + contact = create_contact(g.user, alias, contact_address) + except ErrContactErrorUpgradeNeeded as err: + return jsonify(error=err.error_for_user()), 403 + except (ErrAddressInvalid, CannotCreateContactForReverseAlias) as err: + return jsonify(error=err.error_for_user()), 400 + except ErrContactAlreadyExists as err: + return jsonify(**serialize_contact(err.contact, existed=True)), 200 return jsonify(**serialize_contact(contact)), 201 diff --git a/app/dashboard/views/alias_contact_manager.py b/app/dashboard/views/alias_contact_manager.py index adaf8694..dbe78ea1 100644 --- a/app/dashboard/views/alias_contact_manager.py +++ b/app/dashboard/views/alias_contact_manager.py @@ -16,9 +16,14 @@ from app.email_utils import ( generate_reply_email, parse_full_address, ) -from app.errors import CannotCreateContactForReverseAlias +from app.errors import ( + CannotCreateContactForReverseAlias, + ErrContactErrorUpgradeNeeded, + ErrAddressInvalid, + ErrContactAlreadyExists, +) from app.log import LOG -from app.models import Alias, Contact, EmailLog +from app.models import Alias, Contact, EmailLog, User def email_validator(): @@ -44,6 +49,47 @@ def email_validator(): return _check +def create_contact( + user: User, alias: Alias, contact_address: str, fail_if_already_exist: bool = True +) -> Contact: + if not contact_address: + raise ErrAddressInvalid("Empty address") + try: + contact_name, contact_email = parse_full_address(contact_address) + except ValueError: + raise ErrAddressInvalid(contact_address) + + if not is_valid_email(contact_email): + raise ErrAddressInvalid(contact_email) + + contact = Contact.get_by(alias_id=alias.id, website_email=contact_email) + if contact: + if fail_if_already_exist: + raise ErrContactAlreadyExists(contact) + return contact + + if not user.is_premium() and user.flags & User.FLAG_FREE_DISABLE_CREATE_ALIAS > 0: + raise ErrContactErrorUpgradeNeeded() + + contact = Contact.create( + user_id=alias.user_id, + alias_id=alias.id, + website_email=contact_email, + name=contact_name, + reply_email=generate_reply_email(contact_email, user), + ) + + LOG.d( + "create reverse-alias for %s %s, reverse alias:%s", + contact_address, + alias, + contact.reply_email, + ) + Session.commit() + + return contact + + class NewContactForm(FlaskForm): email = StringField( "Email", validators=[validators.DataRequired(), email_validator()] @@ -150,6 +196,21 @@ def get_contact_infos( return ret +def delete_contact(alias: Alias, contact_id: int): + contact = Contact.get(contact_id) + + if not contact: + flash("Unknown error. Refresh the page", "warning") + elif contact.alias_id != alias.id: + flash("You cannot delete reverse-alias", "warning") + else: + delete_contact_email = contact.website_email + Contact.delete(contact_id) + Session.commit() + + flash(f"Reverse-alias for {delete_contact_email} has been deleted", "success") + + @dashboard_bp.route("/alias_contact_manager//", methods=["GET", "POST"]) @login_required def alias_contact_manager(alias_id): @@ -179,45 +240,18 @@ def alias_contact_manager(alias_id): if request.method == "POST": if request.form.get("form-name") == "create": if new_contact_form.validate(): - contact_addr = new_contact_form.email.data.strip() - + contact_address = new_contact_form.email.data.strip() try: - contact_name, contact_email = parse_full_address(contact_addr) - except Exception: - flash(f"{contact_addr} is invalid", "error") + contact = create_contact(current_user, alias, contact_address) + except ( + ErrContactErrorUpgradeNeeded, + ErrAddressInvalid, + ErrContactAlreadyExists, + CannotCreateContactForReverseAlias, + ) as excp: + flash(excp.error_for_user(), "error") return redirect(request.url) - - if not is_valid_email(contact_email): - flash(f"{contact_email} is invalid", "error") - return redirect(request.url) - - contact = Contact.get_by(alias_id=alias.id, website_email=contact_email) - # already been added - if contact: - flash(f"{contact_email} is already added", "error") - return redirect(request.url) - - try: - contact = Contact.create( - user_id=alias.user_id, - alias_id=alias.id, - website_email=contact_email, - name=contact_name, - reply_email=generate_reply_email(contact_email, current_user), - ) - except CannotCreateContactForReverseAlias: - flash("You can't create contact for a reverse alias", "error") - return redirect(request.url) - - LOG.d( - "create reverse-alias for %s %s, reverse alias:%s", - contact_addr, - alias, - contact.reply_email, - ) - Session.commit() - flash(f"Reverse alias for {contact_addr} is created", "success") - + flash(f"Reverse alias for {contact_address} is created", "success") return redirect( url_for( "dashboard.alias_contact_manager", @@ -227,27 +261,7 @@ def alias_contact_manager(alias_id): ) elif request.form.get("form-name") == "delete": contact_id = request.form.get("contact-id") - contact = Contact.get(contact_id) - - if not contact: - flash("Unknown error. Refresh the page", "warning") - return redirect( - url_for("dashboard.alias_contact_manager", alias_id=alias_id) - ) - elif contact.alias_id != alias.id: - flash("You cannot delete reverse-alias", "warning") - return redirect( - url_for("dashboard.alias_contact_manager", alias_id=alias_id) - ) - - delete_contact_email = contact.website_email - Contact.delete(contact_id) - Session.commit() - - flash( - f"Reverse-alias for {delete_contact_email} has been deleted", "success" - ) - + delete_contact(alias, contact_id) return redirect( url_for("dashboard.alias_contact_manager", alias_id=alias_id) ) diff --git a/app/errors.py b/app/errors.py index f75e83d1..9d14d272 100644 --- a/app/errors.py +++ b/app/errors.py @@ -3,6 +3,10 @@ class SLException(Exception): super_str = super().__str__() return f"{type(self).__name__} {super_str}" + def error_for_user(self) -> str: + """By default send the exception errror to the user. Should be overloaded by the child exceptions""" + return str(self) + class AliasInTrashError(SLException): """raised when alias is deleted before""" @@ -25,7 +29,8 @@ class SubdomainInTrashError(SLException): class CannotCreateContactForReverseAlias(SLException): """raised when a contact is created that has website_email=reverse_alias of another contact""" - pass + def error_for_user(self) -> str: + return "You can't create contact for a reverse alias" class NonReverseAliasInReplyPhase(SLException): @@ -60,3 +65,31 @@ class MailSentFromReverseAlias(SLException): class ProtonPartnerNotSetUp(SLException): pass + + +class ErrContactErrorUpgradeNeeded(SLException): + """raised when user cannot create a contact because the plan doesn't allow it""" + + def error_for_user(self) -> str: + return f"Please upgrade to premium to create reverse-alias" + + +class ErrAddressInvalid(SLException): + """raised when an address is invalid""" + + def __init__(self, address: str): + self.address = address + + def error_for_user(self) -> str: + return f"{self.address} is not a valid email address" + + +class ErrContactAlreadyExists(SLException): + """raised when a contact already exists""" + + # TODO: type-hint this as a contact when models are almost dataclasses and don't import errors + def __init__(self, contact): + self.contact = contact + + def error_for_user(self) -> str: + return f"{self.contact.website_email} is already added" diff --git a/app/models.py b/app/models.py index d0bb193a..e57a8d08 100644 --- a/app/models.py +++ b/app/models.py @@ -292,6 +292,9 @@ class Fido(Base, ModelMixin): class User(Base, ModelMixin, UserMixin, PasswordOracle): __tablename__ = "users" + + FLAG_FREE_DISABLE_CREATE_ALIAS = 1 + email = sa.Column(sa.String(256), unique=True, nullable=False) name = sa.Column(sa.String(128), nullable=True) @@ -490,6 +493,14 @@ class User(Base, ModelMixin, UserMixin, PasswordOracle): ), ) + # bitwise flags. Allow for future expansion + flags = sa.Column( + sa.BigInteger, + default=FLAG_FREE_DISABLE_CREATE_ALIAS, + server_default="0", + nullable=False, + ) + @property def directory_quota(self): return min( diff --git a/migrations/versions/2022_050317_98040e190381_add_flags_to_user.py b/migrations/versions/2022_050317_98040e190381_add_flags_to_user.py new file mode 100644 index 00000000..b32733c4 --- /dev/null +++ b/migrations/versions/2022_050317_98040e190381_add_flags_to_user.py @@ -0,0 +1,29 @@ +"""add flags to user + +Revision ID: 98040e190381 +Revises: 0aaad1740797 +Create Date: 2022-05-03 17:31:58.559032 + +""" +import sqlalchemy_utils +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '98040e190381' +down_revision = '0aaad1740797' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('users', sa.Column('flags', sa.BigInteger(), server_default='0', nullable=False)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('users', 'flags') + # ### end Alembic commands ### diff --git a/pytest.ini b/pytest.ini index 3d362baf..c0f5472c 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,5 +1,5 @@ [pytest] -addopts = +xaddopts = --cov --cov-config coverage.ini --cov-report=html:htmlcov diff --git a/templates/dashboard/alias_contact_manager.html b/templates/dashboard/alias_contact_manager.html index 386a8989..5061406c 100644 --- a/templates/dashboard/alias_contact_manager.html +++ b/templates/dashboard/alias_contact_manager.html @@ -64,7 +64,11 @@
Where do you want to send the email?
- + {% if alias.user.is_premium() %} + + {% else %} + + {% endif %} diff --git a/tests/api/test_alias.py b/tests/api/test_alias.py index d69ea018..1d770baf 100644 --- a/tests/api/test_alias.py +++ b/tests/api/test_alias.py @@ -1,11 +1,12 @@ from flask import url_for +import arrow from app.config import PAGE_LIMIT from app.db import Session from app.email_utils import is_reverse_alias from app.models import User, ApiKey, Alias, Contact, EmailLog, Mailbox from tests.api.utils import get_new_user_and_api_key -from tests.utils import login +from tests.utils import login, random_domain def test_get_aliases_error_without_pagination(flask_client): @@ -527,6 +528,57 @@ def test_create_contact_route(flask_client): assert r.json["existed"] +def test_create_contact_route_invalid_alias(flask_client): + user, api_key = get_new_user_and_api_key() + other_user, other_api_key = get_new_user_and_api_key() + + alias = Alias.create_new_random(other_user) + Session.commit() + + r = flask_client.post( + url_for("api.create_contact_route", alias_id=alias.id), + headers={"Authentication": api_key.code}, + json={"contact": "First Last "}, + ) + + assert r.status_code == 403 + + +def test_create_contact_route_free_users(flask_client): + user, api_key = get_new_user_and_api_key() + + alias = Alias.create_new_random(user) + Session.commit() + # On trial, should be ok + r = flask_client.post( + url_for("api.create_contact_route", alias_id=alias.id), + headers={"Authentication": api_key.code}, + json={"contact": f"First Last "}, + ) + assert r.status_code == 201 + + # End trial but allow via flags for older free users + user.trial_end = arrow.now() + user.flags = 0 + Session.commit() + r = flask_client.post( + url_for("api.create_contact_route", alias_id=alias.id), + headers={"Authentication": api_key.code}, + json={"contact": f"First Last "}, + ) + assert r.status_code == 201 + + # End trial and disallow for new free users + user.flags = User.FLAG_FREE_DISABLE_CREATE_ALIAS + Session.commit() + r = flask_client.post( + url_for("api.create_contact_route", alias_id=alias.id), + headers={"Authentication": api_key.code}, + json={"contact": f"First Last "}, + ) + assert r.status_code == 403 + + def test_create_contact_route_empty_contact_address(flask_client): user = login(flask_client) alias = Alias.filter_by(user_id=user.id).first() @@ -537,7 +589,7 @@ def test_create_contact_route_empty_contact_address(flask_client): ) assert r.status_code == 400 - assert r.json["error"] == "Contact cannot be empty" + assert r.json["error"] == "Empty address is not a valid email address" def test_create_contact_route_invalid_contact_email(flask_client): @@ -550,7 +602,7 @@ def test_create_contact_route_invalid_contact_email(flask_client): ) assert r.status_code == 400 - assert r.json["error"] == "invalid contact email @gmail.com" + assert r.json["error"] == "@gmail.com is not a valid email address" def test_delete_contact(flask_client): From 19e30eaf0ab1b80ea6133a5376724c81480bea3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Thu, 12 May 2022 13:42:53 +0200 Subject: [PATCH 02/13] New migration --- ..._088f23324464_add_flags_to_the_user_model.py} | 12 ++++++------ scripts/new-migration.sh | 16 ++++++++++++---- 2 files changed, 18 insertions(+), 10 deletions(-) rename migrations/versions/{2022_050317_98040e190381_add_flags_to_user.py => 2022_051213_088f23324464_add_flags_to_the_user_model.py} (75%) mode change 100644 => 100755 scripts/new-migration.sh diff --git a/migrations/versions/2022_050317_98040e190381_add_flags_to_user.py b/migrations/versions/2022_051213_088f23324464_add_flags_to_the_user_model.py similarity index 75% rename from migrations/versions/2022_050317_98040e190381_add_flags_to_user.py rename to migrations/versions/2022_051213_088f23324464_add_flags_to_the_user_model.py index b32733c4..5d33e6a4 100644 --- a/migrations/versions/2022_050317_98040e190381_add_flags_to_user.py +++ b/migrations/versions/2022_051213_088f23324464_add_flags_to_the_user_model.py @@ -1,8 +1,8 @@ -"""add flags to user +"""add flags to the user model -Revision ID: 98040e190381 -Revises: 0aaad1740797 -Create Date: 2022-05-03 17:31:58.559032 +Revision ID: 088f23324464 +Revises: e866ad0e78e1 +Create Date: 2022-05-12 13:32:30.898367 """ import sqlalchemy_utils @@ -11,8 +11,8 @@ import sqlalchemy as sa # revision identifiers, used by Alembic. -revision = '98040e190381' -down_revision = '0aaad1740797' +revision = '088f23324464' +down_revision = 'e866ad0e78e1' branch_labels = None depends_on = None diff --git a/scripts/new-migration.sh b/scripts/new-migration.sh old mode 100644 new mode 100755 index 5c9f710f..c0f0023d --- a/scripts/new-migration.sh +++ b/scripts/new-migration.sh @@ -2,9 +2,17 @@ # To run it: # sh scripts/new-migration.sh +container_name=sl-db-new-migration + +if [ "$#" -lt "1" ]; then + echo "What is this migration for?" + exit 1 +fi +reason="$@" + # create a postgres database for SimpleLogin -docker rm -f sl-db -docker run -p 25432:5432 --name sl-db -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=sl -d postgres:13 +docker rm -f ${container_name} +docker run -p 25432:5432 --name ${container_name} -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=sl -d postgres:13 # sleep a little bit for the db to be ready sleep 3 @@ -13,7 +21,7 @@ sleep 3 env DB_URI=postgresql://postgres:postgres@127.0.0.1:25432/sl poetry run alembic upgrade head # generate the migration script. -env DB_URI=postgresql://postgres:postgres@127.0.0.1:25432/sl poetry run alembic revision --autogenerate +env DB_URI=postgresql://postgres:postgres@127.0.0.1:25432/sl poetry run alembic revision --autogenerate -m "$reason" # remove the db -docker rm -f sl-db +docker rm -f ${container_name} From caff70ea38fe73e09ef4836c0c05e6d9c3b1df78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Thu, 12 May 2022 16:35:51 +0200 Subject: [PATCH 03/13] Set global config to enable/disable feature --- app/dashboard/views/alias_contact_manager.py | 17 ++++++++++++++++- tests/api/test_alias.py | 15 ++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/app/dashboard/views/alias_contact_manager.py b/app/dashboard/views/alias_contact_manager.py index dbe78ea1..14e139bb 100644 --- a/app/dashboard/views/alias_contact_manager.py +++ b/app/dashboard/views/alias_contact_manager.py @@ -26,6 +26,19 @@ from app.log import LOG from app.models import Alias, Contact, EmailLog, User +# TODO: Move this to a scoped config once the global config gets scoped. +# This allows this config to be modified from tests so we can test both scenarios. +# By default allow users to create contacts +class AllowFreeUsersToCreateContacts: + allow: bool = True + + def set(self, allow: bool): + self.allow = allow + + +allow_free_users_to_create_contacts = AllowFreeUsersToCreateContacts() + + def email_validator(): """validate email address. Handle both only email and email with name: - ab@cd.com @@ -68,7 +81,9 @@ def create_contact( raise ErrContactAlreadyExists(contact) return contact - if not user.is_premium() and user.flags & User.FLAG_FREE_DISABLE_CREATE_ALIAS > 0: + if not allow_free_users_to_create_contacts.allow and ( + not user.is_premium() and user.flags & User.FLAG_FREE_DISABLE_CREATE_ALIAS > 0 + ): raise ErrContactErrorUpgradeNeeded() contact = Contact.create( diff --git a/tests/api/test_alias.py b/tests/api/test_alias.py index 1d770baf..068c5973 100644 --- a/tests/api/test_alias.py +++ b/tests/api/test_alias.py @@ -2,6 +2,9 @@ from flask import url_for import arrow from app.config import PAGE_LIMIT +from app.dashboard.views.alias_contact_manager import ( + allow_free_users_to_create_contacts, +) from app.db import Session from app.email_utils import is_reverse_alias from app.models import User, ApiKey, Alias, Contact, EmailLog, Mailbox @@ -568,7 +571,7 @@ def test_create_contact_route_free_users(flask_client): ) assert r.status_code == 201 - # End trial and disallow for new free users + # End trial and disallow for new free users. Config should allow it user.flags = User.FLAG_FREE_DISABLE_CREATE_ALIAS Session.commit() r = flask_client.post( @@ -576,7 +579,17 @@ def test_create_contact_route_free_users(flask_client): headers={"Authentication": api_key.code}, json={"contact": f"First Last "}, ) + assert r.status_code == 201 + + # Set the global config to disable free users from create contacts + allow_free_users_to_create_contacts.set(False) + r = flask_client.post( + url_for("api.create_contact_route", alias_id=alias.id), + headers={"Authentication": api_key.code}, + json={"contact": f"First Last "}, + ) assert r.status_code == 403 + allow_free_users_to_create_contacts.set(True) def test_create_contact_route_empty_contact_address(flask_client): From 8b3dc765fa5c8bb6e4e8c6762cd549b14697d854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Thu, 12 May 2022 18:21:19 +0200 Subject: [PATCH 04/13] Revert pytest.ini --- pytest.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest.ini b/pytest.ini index c0f5472c..3d362baf 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,5 +1,5 @@ [pytest] -xaddopts = +addopts = --cov --cov-config coverage.ini --cov-report=html:htmlcov From 4d07bc9d3104c99f0ca806f212dd319c309f64b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Thu, 12 May 2022 18:30:46 +0200 Subject: [PATCH 05/13] Moved global flag to config --- app/config.py | 2 ++ app/dashboard/views/alias_contact_manager.py | 25 +++++--------- tests/api/test_alias.py | 35 +++++++++----------- 3 files changed, 26 insertions(+), 36 deletions(-) diff --git a/app/config.py b/app/config.py index 72eec144..fdbbe5c1 100644 --- a/app/config.py +++ b/app/config.py @@ -464,3 +464,5 @@ def setup_nameservers(): NAMESERVERS = setup_nameservers() + +DISABLE_CREATE_CONTACTS_FOR_FREE_USERS = False diff --git a/app/dashboard/views/alias_contact_manager.py b/app/dashboard/views/alias_contact_manager.py index 14e139bb..caa6488f 100644 --- a/app/dashboard/views/alias_contact_manager.py +++ b/app/dashboard/views/alias_contact_manager.py @@ -8,7 +8,7 @@ from flask_wtf import FlaskForm from sqlalchemy import and_, func, case from wtforms import StringField, validators, ValidationError -from app.config import PAGE_LIMIT +from app import config from app.dashboard.base import dashboard_bp from app.db import Session from app.email_utils import ( @@ -26,19 +26,6 @@ from app.log import LOG from app.models import Alias, Contact, EmailLog, User -# TODO: Move this to a scoped config once the global config gets scoped. -# This allows this config to be modified from tests so we can test both scenarios. -# By default allow users to create contacts -class AllowFreeUsersToCreateContacts: - allow: bool = True - - def set(self, allow: bool): - self.allow = allow - - -allow_free_users_to_create_contacts = AllowFreeUsersToCreateContacts() - - def email_validator(): """validate email address. Handle both only email and email with name: - ab@cd.com @@ -81,7 +68,7 @@ def create_contact( raise ErrContactAlreadyExists(contact) return contact - if not allow_free_users_to_create_contacts.allow and ( + if config.DISABLE_CREATE_CONTACTS_FOR_FREE_USERS and ( not user.is_premium() and user.flags & User.FLAG_FREE_DISABLE_CREATE_ALIAS > 0 ): raise ErrContactErrorUpgradeNeeded() @@ -196,7 +183,11 @@ def get_contact_infos( ], else_=Contact.created_at, ) - q = q.order_by(latest_activity.desc()).limit(PAGE_LIMIT).offset(page * PAGE_LIMIT) + q = ( + q.order_by(latest_activity.desc()) + .limit(config.PAGE_LIMIT) + .offset(page * config.PAGE_LIMIT) + ) ret = [] for contact, latest_email_log, nb_reply, nb_forward in q: @@ -293,7 +284,7 @@ def alias_contact_manager(alias_id): ) contact_infos = get_contact_infos(alias, page, query=query) - last_page = len(contact_infos) < PAGE_LIMIT + last_page = len(contact_infos) < config.PAGE_LIMIT nb_contact = Contact.filter(Contact.alias_id == alias.id).count() # if highlighted contact isn't included, fetch it diff --git a/tests/api/test_alias.py b/tests/api/test_alias.py index 068c5973..b120c00d 100644 --- a/tests/api/test_alias.py +++ b/tests/api/test_alias.py @@ -1,10 +1,7 @@ from flask import url_for import arrow -from app.config import PAGE_LIMIT -from app.dashboard.views.alias_contact_manager import ( - allow_free_users_to_create_contacts, -) +from app import config from app.db import Session from app.email_utils import is_reverse_alias from app.models import User, ApiKey, Alias, Contact, EmailLog, Mailbox @@ -46,17 +43,17 @@ def test_get_aliases_with_pagination(flask_client): api_key = ApiKey.create(user.id, "for test") Session.commit() - # create more aliases than PAGE_LIMIT - for _ in range(PAGE_LIMIT + 1): + # create more aliases than config.PAGE_LIMIT + for _ in range(config.PAGE_LIMIT + 1): Alias.create_new_random(user) Session.commit() - # get aliases on the 1st page, should return PAGE_LIMIT aliases + # get aliases on the 1st page, should return config.PAGE_LIMIT aliases r = flask_client.get( url_for("api.get_aliases", page_id=0), headers={"Authentication": api_key.code} ) assert r.status_code == 200 - assert len(r.json["aliases"]) == PAGE_LIMIT + assert len(r.json["aliases"]) == config.PAGE_LIMIT # assert returned field for a in r.json["aliases"]: @@ -71,7 +68,7 @@ def test_get_aliases_with_pagination(flask_client): assert "note" in a # get aliases on the 2nd page, should return 2 aliases - # as the total number of aliases is PAGE_LIMIT +2 + # as the total number of aliases is config.PAGE_LIMIT +2 # 1 alias is created when user is created r = flask_client.get( url_for("api.get_aliases", page_id=1), headers={"Authentication": api_key.code} @@ -90,7 +87,7 @@ def test_get_aliases_query(flask_client): api_key = ApiKey.create(user.id, "for test") Session.commit() - # create more aliases than PAGE_LIMIT + # create more aliases than config.PAGE_LIMIT Alias.create_new(user, "prefix1") Alias.create_new(user, "prefix2") Session.commit() @@ -281,7 +278,7 @@ def test_alias_activities(flask_client): ) Session.commit() - for _ in range(int(PAGE_LIMIT / 2)): + for _ in range(int(config.PAGE_LIMIT / 2)): EmailLog.create( contact_id=contact.id, is_reply=True, @@ -289,7 +286,7 @@ def test_alias_activities(flask_client): alias_id=contact.alias_id, ) - for _ in range(int(PAGE_LIMIT / 2) + 2): + for _ in range(int(config.PAGE_LIMIT / 2) + 2): EmailLog.create( contact_id=contact.id, blocked=True, @@ -303,7 +300,7 @@ def test_alias_activities(flask_client): ) assert r.status_code == 200 - assert len(r.json["activities"]) == PAGE_LIMIT + assert len(r.json["activities"]) == config.PAGE_LIMIT for ac in r.json["activities"]: assert ac["from"] assert ac["to"] @@ -456,7 +453,7 @@ def test_alias_contacts(flask_client): Session.commit() # create some alias log - for i in range(PAGE_LIMIT + 1): + for i in range(config.PAGE_LIMIT + 1): contact = Contact.create( website_email=f"marketing-{i}@example.com", reply_email=f"reply-{i}@a.b", @@ -476,7 +473,7 @@ def test_alias_contacts(flask_client): r = flask_client.get(f"/api/aliases/{alias.id}/contacts?page_id=0") assert r.status_code == 200 - assert len(r.json["contacts"]) == PAGE_LIMIT + assert len(r.json["contacts"]) == config.PAGE_LIMIT for ac in r.json["contacts"]: assert ac["creation_date"] assert ac["creation_timestamp"] @@ -582,14 +579,14 @@ def test_create_contact_route_free_users(flask_client): assert r.status_code == 201 # Set the global config to disable free users from create contacts - allow_free_users_to_create_contacts.set(False) + config.DISABLE_CREATE_CONTACTS_FOR_FREE_USERS = True r = flask_client.post( url_for("api.create_contact_route", alias_id=alias.id), headers={"Authentication": api_key.code}, json={"contact": f"First Last "}, ) assert r.status_code == 403 - allow_free_users_to_create_contacts.set(True) + config.DISABLE_CREATE_CONTACTS_FOR_FREE_USERS = False def test_create_contact_route_empty_contact_address(flask_client): @@ -644,11 +641,11 @@ def test_delete_contact(flask_client): def test_get_alias(flask_client): user, api_key = get_new_user_and_api_key() - # create more aliases than PAGE_LIMIT + # create more aliases than config.PAGE_LIMIT alias = Alias.create_new_random(user) Session.commit() - # get aliases on the 1st page, should return PAGE_LIMIT aliases + # get aliases on the 1st page, should return config.PAGE_LIMIT aliases r = flask_client.get( url_for("api.get_alias", alias_id=alias.id), headers={"Authentication": api_key.code}, From 9066116b7ebc0aaa333dac307550b7a56dc63033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Thu, 12 May 2022 18:33:13 +0200 Subject: [PATCH 06/13] Simplified method --- app/dashboard/views/alias_contact_manager.py | 9 +++------ tests/api/test_alias.py | 1 + 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/app/dashboard/views/alias_contact_manager.py b/app/dashboard/views/alias_contact_manager.py index caa6488f..f5c560ca 100644 --- a/app/dashboard/views/alias_contact_manager.py +++ b/app/dashboard/views/alias_contact_manager.py @@ -8,6 +8,7 @@ from flask_wtf import FlaskForm from sqlalchemy import and_, func, case from wtforms import StringField, validators, ValidationError +# Need to import directly from config to allow modification from the tests from app import config from app.dashboard.base import dashboard_bp from app.db import Session @@ -49,9 +50,7 @@ def email_validator(): return _check -def create_contact( - user: User, alias: Alias, contact_address: str, fail_if_already_exist: bool = True -) -> Contact: +def create_contact(user: User, alias: Alias, contact_address: str) -> Contact: if not contact_address: raise ErrAddressInvalid("Empty address") try: @@ -64,9 +63,7 @@ def create_contact( contact = Contact.get_by(alias_id=alias.id, website_email=contact_email) if contact: - if fail_if_already_exist: - raise ErrContactAlreadyExists(contact) - return contact + raise ErrContactAlreadyExists(contact) if config.DISABLE_CREATE_CONTACTS_FOR_FREE_USERS and ( not user.is_premium() and user.flags & User.FLAG_FREE_DISABLE_CREATE_ALIAS > 0 diff --git a/tests/api/test_alias.py b/tests/api/test_alias.py index b120c00d..b66e5e65 100644 --- a/tests/api/test_alias.py +++ b/tests/api/test_alias.py @@ -1,6 +1,7 @@ from flask import url_for import arrow +# Need to import directly from config to allow modification from the tests from app import config from app.db import Session from app.email_utils import is_reverse_alias From 39b035a1230af02dde737f8abc1e0c280adb2686 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Thu, 12 May 2022 18:36:12 +0200 Subject: [PATCH 07/13] Added docs --- app/dashboard/views/alias_contact_manager.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/dashboard/views/alias_contact_manager.py b/app/dashboard/views/alias_contact_manager.py index f5c560ca..8f755292 100644 --- a/app/dashboard/views/alias_contact_manager.py +++ b/app/dashboard/views/alias_contact_manager.py @@ -51,6 +51,13 @@ def email_validator(): def create_contact(user: User, alias: Alias, contact_address: str) -> Contact: + """ + Create a contact for a user. Can be restricted for new free users by enabling DISABLE_CREATE_CONTACTS_FOR_FREE_USERS. + Can throw exceptions: + - ErrAddressInvalid + - ErrContactAlreadyExists + - ErrContactUpgradeNeeded - If DISABLE_CREATE_CONTACTS_FOR_FREE_USERS this exception will be raised for new free users + """ if not contact_address: raise ErrAddressInvalid("Empty address") try: From bfb1ae637173c131d21b3840576e0e1a7b89d69e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Thu, 12 May 2022 18:42:16 +0200 Subject: [PATCH 08/13] PR comments --- app/dashboard/views/alias_contact_manager.py | 2 ++ app/errors.py | 2 +- scripts/new-migration.sh | 8 +------- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/app/dashboard/views/alias_contact_manager.py b/app/dashboard/views/alias_contact_manager.py index 8f755292..9df31ca8 100644 --- a/app/dashboard/views/alias_contact_manager.py +++ b/app/dashboard/views/alias_contact_manager.py @@ -25,6 +25,7 @@ from app.errors import ( ) from app.log import LOG from app.models import Alias, Contact, EmailLog, User +from app.utils import sanitize_email def email_validator(): @@ -65,6 +66,7 @@ def create_contact(user: User, alias: Alias, contact_address: str) -> Contact: except ValueError: raise ErrAddressInvalid(contact_address) + contact_email = sanitize_email(contact_email) if not is_valid_email(contact_email): raise ErrAddressInvalid(contact_email) diff --git a/app/errors.py b/app/errors.py index 9d14d272..ede8ac2d 100644 --- a/app/errors.py +++ b/app/errors.py @@ -88,7 +88,7 @@ class ErrContactAlreadyExists(SLException): """raised when a contact already exists""" # TODO: type-hint this as a contact when models are almost dataclasses and don't import errors - def __init__(self, contact): + def __init__(self, contact: "Contact"): # noqa: F821 self.contact = contact def error_for_user(self) -> str: diff --git a/scripts/new-migration.sh b/scripts/new-migration.sh index c0f0023d..da11a756 100755 --- a/scripts/new-migration.sh +++ b/scripts/new-migration.sh @@ -4,12 +4,6 @@ container_name=sl-db-new-migration -if [ "$#" -lt "1" ]; then - echo "What is this migration for?" - exit 1 -fi -reason="$@" - # create a postgres database for SimpleLogin docker rm -f ${container_name} docker run -p 25432:5432 --name ${container_name} -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=sl -d postgres:13 @@ -21,7 +15,7 @@ sleep 3 env DB_URI=postgresql://postgres:postgres@127.0.0.1:25432/sl poetry run alembic upgrade head # generate the migration script. -env DB_URI=postgresql://postgres:postgres@127.0.0.1:25432/sl poetry run alembic revision --autogenerate -m "$reason" +env DB_URI=postgresql://postgres:postgres@127.0.0.1:25432/sl poetry run alembic revision --autogenerate $@ # remove the db docker rm -f ${container_name} From 0c896100a4efd5efaa630ff275cfa719e8d5e416 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Thu, 12 May 2022 18:46:42 +0200 Subject: [PATCH 09/13] Update html --- app/dashboard/views/alias_contact_manager.py | 14 +++++++++++--- templates/dashboard/alias_contact_manager.html | 2 +- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/dashboard/views/alias_contact_manager.py b/app/dashboard/views/alias_contact_manager.py index 9df31ca8..15a1a353 100644 --- a/app/dashboard/views/alias_contact_manager.py +++ b/app/dashboard/views/alias_contact_manager.py @@ -51,6 +51,15 @@ def email_validator(): return _check +def user_can_create_contacts(user: User) -> bool: + if user.is_premium(): + return True + return ( + config.DISABLE_CREATE_CONTACTS_FOR_FREE_USERS + and user.flags & User.FLAG_FREE_DISABLE_CREATE_ALIAS > 0 + ) + + def create_contact(user: User, alias: Alias, contact_address: str) -> Contact: """ Create a contact for a user. Can be restricted for new free users by enabling DISABLE_CREATE_CONTACTS_FOR_FREE_USERS. @@ -74,9 +83,7 @@ def create_contact(user: User, alias: Alias, contact_address: str) -> Contact: if contact: raise ErrContactAlreadyExists(contact) - if config.DISABLE_CREATE_CONTACTS_FOR_FREE_USERS and ( - not user.is_premium() and user.flags & User.FLAG_FREE_DISABLE_CREATE_ALIAS > 0 - ): + if not user_can_create_contacts(user): raise ErrContactErrorUpgradeNeeded() contact = Contact.create( @@ -312,4 +319,5 @@ def alias_contact_manager(alias_id): last_page=last_page, query=query, nb_contact=nb_contact, + can_create_contacts=user_can_create_contacts(current_user), ) diff --git a/templates/dashboard/alias_contact_manager.html b/templates/dashboard/alias_contact_manager.html index 5061406c..a26370e4 100644 --- a/templates/dashboard/alias_contact_manager.html +++ b/templates/dashboard/alias_contact_manager.html @@ -64,7 +64,7 @@
Where do you want to send the email?
- {% if alias.user.is_premium() %} + {% if can_create_contacts %} {% else %} From 6e948408c6af4e982e0a8b5685979d452f511024 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Thu, 12 May 2022 18:50:27 +0200 Subject: [PATCH 10/13] Updated api docs --- docs/api.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/api.md b/docs/api.md index 04e2373c..ae6bb357 100644 --- a/docs/api.md +++ b/docs/api.md @@ -600,6 +600,14 @@ Return 200 and `existed=true` if contact is already added. } ``` +It can return 403 with an error if the user cannot create reverse alias. + +``json +{ + "error": "Please upgrade to create a reverse-alias" +} +``` + ### Mailbox endpoints #### GET /api/v2/mailboxes From 75dd20ebccf3c96781dfe68c9aa5904b8f393e10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Thu, 12 May 2022 19:01:04 +0200 Subject: [PATCH 11/13] Fix condition --- app/dashboard/views/alias_contact_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/dashboard/views/alias_contact_manager.py b/app/dashboard/views/alias_contact_manager.py index 15a1a353..bcd81f45 100644 --- a/app/dashboard/views/alias_contact_manager.py +++ b/app/dashboard/views/alias_contact_manager.py @@ -55,8 +55,8 @@ def user_can_create_contacts(user: User) -> bool: if user.is_premium(): return True return ( - config.DISABLE_CREATE_CONTACTS_FOR_FREE_USERS - and user.flags & User.FLAG_FREE_DISABLE_CREATE_ALIAS > 0 + not config.DISABLE_CREATE_CONTACTS_FOR_FREE_USERS + or user.flags & User.FLAG_FREE_DISABLE_CREATE_ALIAS == 0 ) From 52cd9d2692516b766385b21aca1b1cc88a3e0be9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Thu, 12 May 2022 19:02:06 +0200 Subject: [PATCH 12/13] Simplify condition --- app/dashboard/views/alias_contact_manager.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/dashboard/views/alias_contact_manager.py b/app/dashboard/views/alias_contact_manager.py index bcd81f45..5f478d9b 100644 --- a/app/dashboard/views/alias_contact_manager.py +++ b/app/dashboard/views/alias_contact_manager.py @@ -54,10 +54,9 @@ def email_validator(): def user_can_create_contacts(user: User) -> bool: if user.is_premium(): return True - return ( - not config.DISABLE_CREATE_CONTACTS_FOR_FREE_USERS - or user.flags & User.FLAG_FREE_DISABLE_CREATE_ALIAS == 0 - ) + if user.flags & User.FLAG_FREE_DISABLE_CREATE_ALIAS == 0: + return True + return not config.DISABLE_CREATE_CONTACTS_FOR_FREE_USERS def create_contact(user: User, alias: Alias, contact_address: str) -> Contact: From 7235de8e733452f9e533b3b4869db8b6beacd4d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Fri, 13 May 2022 13:02:26 +0200 Subject: [PATCH 13/13] HTML formatting --- templates/dashboard/alias_contact_manager.html | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/templates/dashboard/alias_contact_manager.html b/templates/dashboard/alias_contact_manager.html index a26370e4..620bada7 100644 --- a/templates/dashboard/alias_contact_manager.html +++ b/templates/dashboard/alias_contact_manager.html @@ -64,11 +64,11 @@
Where do you want to send the email?
- {% if can_create_contacts %} - - {% else %} - - {% endif %} + {% if can_create_contacts %} + + {% else %} + + {% endif %}