diff --git a/app/README.md b/app/README.md index b98f317..731a744 100644 --- a/app/README.md +++ b/app/README.md @@ -84,7 +84,7 @@ For email gurus, we have chosen 1024 key length instead of 2048 for DNS simplici ### DNS -Please note that DNS changes could take up to 24 hours to propagate. In practice, it's a lot faster though (~1 minute or so in our test). In DNS setup, we usually use domain with a trailing dot (`.`) at the end to to force using absolute domain. +Please note that DNS changes could take up to 24 hours to propagate. In practice, it's a lot faster though (~1 minute or so in our test). In DNS setup, we usually use domain with a trailing dot (`.`) at the end to force using absolute domain. #### MX record diff --git a/app/app/account_linking.py b/app/app/account_linking.py index 56b1da8..2f42aa0 100644 --- a/app/app/account_linking.py +++ b/app/app/account_linking.py @@ -6,6 +6,7 @@ from typing import Optional import arrow from arrow import Arrow from newrelic import agent +from psycopg2.errors import UniqueViolation from sqlalchemy import or_ from app.db import Session @@ -160,15 +161,55 @@ class ClientMergeStrategy(ABC): class NewUserStrategy(ClientMergeStrategy): def process(self) -> LinkResult: - # Will create a new SL User with a random password canonical_email = canonicalize_email(self.link_request.email) - new_user = User.create( - email=canonical_email, - name=self.link_request.name, - password=random_string(20), - activated=True, - from_partner=self.link_request.from_partner, + try: + # Will create a new SL User with a random password + new_user = User.create( + email=canonical_email, + name=self.link_request.name, + password=random_string(20), + activated=True, + from_partner=self.link_request.from_partner, + ) + self.create_partner_user(new_user) + Session.commit() + + if not new_user.created_by_partner: + send_welcome_email(new_user) + + agent.record_custom_event( + "PartnerUserCreation", {"partner": self.partner.name} + ) + + return LinkResult( + user=new_user, + strategy=self.__class__.__name__, + ) + except UniqueViolation: + return self.create_missing_link(canonical_email) + + def create_missing_link(self, canonical_email: str): + # If there's a unique key violation due to race conditions try to create only the partner if needed + partner_user = PartnerUser.get_by( + external_user_id=self.link_request.external_user_id, + partner_id=self.partner.id, ) + if partner_user is None: + # Get the user by canonical email and if not by normal email + user = User.get_by(email=canonical_email) or User.get_by( + email=self.link_request.email + ) + if not user: + raise RuntimeError( + "Tried to create only partner on UniqueViolation but cannot find the user" + ) + partner_user = self.create_partner_user(user) + Session.commit() + return LinkResult( + user=partner_user.user, strategy=ExistingUnlinkedUserStrategy.__name__ + ) + + def create_partner_user(self, new_user: User): partner_user = create_partner_user( user=new_user, partner_id=self.partner.id, @@ -182,17 +223,7 @@ class NewUserStrategy(ClientMergeStrategy): partner_user, self.link_request.plan, ) - Session.commit() - - if not new_user.created_by_partner: - send_welcome_email(new_user) - - agent.record_custom_event("PartnerUserCreation", {"partner": self.partner.name}) - - return LinkResult( - user=new_user, - strategy=self.__class__.__name__, - ) + return partner_user class ExistingUnlinkedUserStrategy(ClientMergeStrategy): diff --git a/app/app/alias_suffix.py b/app/app/alias_suffix.py index fbcbff2..35485fd 100644 --- a/app/app/alias_suffix.py +++ b/app/app/alias_suffix.py @@ -58,7 +58,7 @@ def verify_prefix_suffix( # alias_domain must be either one of user custom domains or built-in domains if alias_domain not in user.available_alias_domains(alias_options=alias_options): - LOG.e("wrong alias suffix %s, user %s", alias_suffix, user) + LOG.i("wrong alias suffix %s, user %s", alias_suffix, user) return False # SimpleLogin domain case: @@ -75,17 +75,17 @@ def verify_prefix_suffix( and not config.DISABLE_ALIAS_SUFFIX ): if not alias_domain_prefix.startswith("."): - LOG.e("User %s submits a wrong alias suffix %s", user, alias_suffix) + LOG.i("User %s submits a wrong alias suffix %s", user, alias_suffix) return False else: if alias_domain not in user_custom_domains: if not config.DISABLE_ALIAS_SUFFIX: - LOG.e("wrong alias suffix %s, user %s", alias_suffix, user) + LOG.i("wrong alias suffix %s, user %s", alias_suffix, user) return False if alias_domain not in available_sl_domains: - LOG.e("wrong alias suffix %s, user %s", alias_suffix, user) + LOG.i("wrong alias suffix %s, user %s", alias_suffix, user) return False return True diff --git a/app/app/api/views/new_custom_alias.py b/app/app/api/views/new_custom_alias.py index 0e21815..6cb78c2 100644 --- a/app/app/api/views/new_custom_alias.py +++ b/app/app/api/views/new_custom_alias.py @@ -153,7 +153,8 @@ def new_custom_alias_v3(): if not isinstance(data, dict): return jsonify(error="request body does not follow the required format"), 400 - alias_prefix = data.get("alias_prefix", "").strip().lower().replace(" ", "") + alias_prefix_data = data.get("alias_prefix", "") or "" + alias_prefix = alias_prefix_data.strip().lower().replace(" ", "") signed_suffix = data.get("signed_suffix", "") or "" signed_suffix = signed_suffix.strip() diff --git a/app/app/dashboard/views/alias_contact_manager.py b/app/app/dashboard/views/alias_contact_manager.py index 088442a..501910b 100644 --- a/app/app/dashboard/views/alias_contact_manager.py +++ b/app/app/dashboard/views/alias_contact_manager.py @@ -227,7 +227,10 @@ def alias_contact_manager(alias_id): page = 0 if request.args.get("page"): - page = int(request.args.get("page")) + try: + page = int(request.args.get("page")) + except ValueError: + pass query = request.args.get("query") or "" diff --git a/app/app/dashboard/views/index.py b/app/app/dashboard/views/index.py index 26f2ab4..c9f2499 100644 --- a/app/app/dashboard/views/index.py +++ b/app/app/dashboard/views/index.py @@ -71,7 +71,10 @@ def index(): page = 0 if request.args.get("page"): - page = int(request.args.get("page")) + try: + page = int(request.args.get("page")) + except ValueError: + pass highlight_alias_id = None if request.args.get("highlight_alias_id"): diff --git a/app/app/dashboard/views/notification.py b/app/app/dashboard/views/notification.py index e22c8e2..405747a 100644 --- a/app/app/dashboard/views/notification.py +++ b/app/app/dashboard/views/notification.py @@ -43,7 +43,10 @@ def notification_route(notification_id): def notifications_route(): page = 0 if request.args.get("page"): - page = int(request.args.get("page")) + try: + page = int(request.args.get("page")) + except ValueError: + pass notifications = ( Notification.filter_by(user_id=current_user.id) diff --git a/app/app/models.py b/app/app/models.py index 702e875..721f95d 100644 --- a/app/app/models.py +++ b/app/app/models.py @@ -565,6 +565,7 @@ class User(Base, ModelMixin, UserMixin, PasswordOracle): "ix_users_activated_trial_end_lifetime", activated, trial_end, lifetime ), sa.Index("ix_users_delete_on", delete_on), + sa.Index("ix_users_default_mailbox_id", default_mailbox_id), ) @property @@ -1666,6 +1667,11 @@ class Alias(Base, ModelMixin): custom_domain = Alias.get_custom_domain(email) if custom_domain: new_alias.custom_domain_id = custom_domain.id + else: + custom_domain = CustomDomain.get(kw["custom_domain_id"]) + # If it comes from a custom domain created from partner. Mark it as created from partner + if custom_domain is not None and custom_domain.partner_id is not None: + new_alias.flags = (new_alias.flags or 0) | Alias.FLAG_PARTNER_CREATED Session.add(new_alias) DailyMetric.get_or_create_today_metric().nb_alias += 1 @@ -2069,7 +2075,11 @@ class Contact(Base, ModelMixin): class EmailLog(Base, ModelMixin): __tablename__ = "email_log" - __table_args__ = (Index("ix_email_log_created_at", "created_at"),) + __table_args__ = ( + Index("ix_email_log_created_at", "created_at"), + Index("ix_email_log_mailbox_id", "mailbox_id"), + Index("ix_email_log_bounced_mailbox_id", "bounced_mailbox_id"), + ) user_id = sa.Column( sa.ForeignKey(User.id, ondelete="cascade"), nullable=False, index=True @@ -2747,7 +2757,10 @@ class Mailbox(Base, ModelMixin): generic_subject = sa.Column(sa.String(78), nullable=True) - __table_args__ = (sa.UniqueConstraint("user_id", "email", name="uq_mailbox_user"),) + __table_args__ = ( + sa.UniqueConstraint("user_id", "email", name="uq_mailbox_user"), + sa.Index("ix_mailbox_pgp_finger_print", "pgp_finger_print"), + ) user = orm.relationship(User, foreign_keys=[user_id]) diff --git a/app/migrations/versions/2024_111315_842ac670096e_add_missing_indices_on_user_and_mailbox.py b/app/migrations/versions/2024_111315_842ac670096e_add_missing_indices_on_user_and_mailbox.py new file mode 100644 index 0000000..1cef47a --- /dev/null +++ b/app/migrations/versions/2024_111315_842ac670096e_add_missing_indices_on_user_and_mailbox.py @@ -0,0 +1,30 @@ +"""add missing indices on user and mailbox + +Revision ID: 842ac670096e +Revises: bc9aa210efa3 +Create Date: 2024-11-13 15:55:28.798506 + +""" +import sqlalchemy_utils +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '842ac670096e' +down_revision = 'bc9aa210efa3' +branch_labels = None +depends_on = None + + +def upgrade(): + with op.get_context().autocommit_block(): + op.create_index('ix_mailbox_pgp_finger_print', 'mailbox', ['pgp_finger_print'], unique=False) + op.create_index('ix_users_default_mailbox_id', 'users', ['default_mailbox_id'], unique=False) + # ### end Alembic commands ### + + +def downgrade(): + with op.get_context().autocommit_block(): + op.drop_index('ix_users_default_mailbox_id', table_name='users') + op.drop_index('ix_mailbox_pgp_finger_print', table_name='mailbox') diff --git a/app/migrations/versions/2024_111410_12274da2299f_add_missing_indices_on_email_log.py b/app/migrations/versions/2024_111410_12274da2299f_add_missing_indices_on_email_log.py new file mode 100644 index 0000000..96de714 --- /dev/null +++ b/app/migrations/versions/2024_111410_12274da2299f_add_missing_indices_on_email_log.py @@ -0,0 +1,29 @@ +"""add missing indices on email log + +Revision ID: 12274da2299f +Revises: 842ac670096e +Create Date: 2024-11-14 10:27:20.371191 + +""" +import sqlalchemy_utils +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '12274da2299f' +down_revision = '842ac670096e' +branch_labels = None +depends_on = None + + +def upgrade(): + with op.get_context().autocommit_block(): + op.create_index('ix_email_log_bounced_mailbox_id', 'email_log', ['bounced_mailbox_id'], unique=False) + op.create_index('ix_email_log_mailbox_id', 'email_log', ['mailbox_id'], unique=False) + + +def downgrade(): + with op.get_context().autocommit_block(): + op.drop_index('ix_email_log_mailbox_id', table_name='email_log') + op.drop_index('ix_email_log_bounced_mailbox_id', table_name='email_log') diff --git a/app/tests/models/test_alias.py b/app/tests/models/test_alias.py index c11b596..c50dd7d 100644 --- a/app/tests/models/test_alias.py +++ b/app/tests/models/test_alias.py @@ -1,5 +1,5 @@ from app.db import Session -from app.models import Alias, Mailbox, AliasMailbox, User +from app.models import Alias, Mailbox, AliasMailbox, User, CustomDomain from tests.utils import create_new_user, random_email @@ -29,3 +29,23 @@ def test_alias_create_from_partner_flags_also_the_user(): flush=True, ) assert alias.user.flags & User.FLAG_CREATED_ALIAS_FROM_PARTNER > 0 + + +def test_alias_create_from_partner_domain_flags_the_alias(): + user = create_new_user() + domain = CustomDomain.create( + domain=random_email(), + verified=True, + user_id=user.id, + partner_id=1, + ) + Session.flush() + email = random_email() + alias = Alias.create( + user_id=user.id, + email=email, + mailbox_id=user.default_mailbox_id, + custom_domain_id=domain.id, + flush=True, + ) + assert alias.flags & Alias.FLAG_PARTNER_CREATED > 0 diff --git a/app/tests/test_account_linking.py b/app/tests/test_account_linking.py index 0c64416..a452423 100644 --- a/app/tests/test_account_linking.py +++ b/app/tests/test_account_linking.py @@ -144,6 +144,21 @@ def test_login_case_from_web(): assert audit_logs[0].action == UserAuditLogAction.LinkAccount.value +def test_new_user_strategy_create_missing_link(): + email = random_email() + user = User.create(email, commit=True) + nus = NewUserStrategy( + link_request=random_link_request( + email=user.email, external_user_id=random_string(), from_partner=False + ), + user=None, + partner=get_proton_partner(), + ) + result = nus.create_missing_link(user.email) + assert result.user.id == user.id + assert result.strategy == ExistingUnlinkedUserStrategy.__name__ + + def test_get_strategy_existing_sl_user(): email = random_email() user = User.create(email, commit=True)