-
-
Notifications
You must be signed in to change notification settings - Fork 572
Ability to block domains when receiving emails #2 #2384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
4c44d2a
04b776e
af682dd
08b5ff9
6065d5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| from app.models import BlockedDomain | ||
|
|
||
|
|
||
| def is_domain_blocked(user_id: int, domain: str) -> bool: | ||
| """checks whether the provided domain is blocked for a given user""" | ||
| return BlockedDomain.filter_by(user_id=user_id, domain=domain).first() is not None |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
| ALIAS_RANDOM_SUFFIX_LENGTH, | ||
| CONNECT_WITH_PROTON, | ||
| ) | ||
| from app.custom_domain_utils import sanitize_domain, can_blocked_domain_be_used | ||
| from app.dashboard.base import dashboard_bp | ||
| from app.db import Session | ||
| from app.extensions import limiter | ||
|
|
@@ -39,6 +40,7 @@ | |
| PartnerSubscription, | ||
| UnsubscribeBehaviourEnum, | ||
| UserAliasDeleteAction, | ||
| BlockedDomain, | ||
| ) | ||
| from app.proton.proton_unlink import can_unlink_proton_account | ||
| from app.utils import ( | ||
|
|
@@ -81,6 +83,12 @@ def setting(): | |
| else: | ||
| pending_email = None | ||
|
|
||
| blocked_domains = ( | ||
| BlockedDomain.filter_by(user_id=current_user.id) | ||
| .order_by(BlockedDomain.created_at.desc()) | ||
| .all() | ||
| ) | ||
|
|
||
| if request.method == "POST": | ||
| if not csrf_form.validate(): | ||
| flash("Invalid request", "warning") | ||
|
|
@@ -284,6 +292,63 @@ def setting(): | |
| return redirect(url_for("dashboard.setting")) | ||
| Session.commit() | ||
| flash("Your preference has been updated", "success") | ||
| elif request.form.get("form-name") == "blocked-domains-add": | ||
| domain = request.form.get("domain-name") | ||
|
|
||
| if not domain: | ||
| flash("Domain name is required", "error") | ||
| return redirect(url_for("dashboard.setting")) | ||
|
|
||
| new_domain = sanitize_domain(domain) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If domain is None this will fail
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If domain is None it will fail with an exception |
||
| domain_forbidden_cause = can_blocked_domain_be_used( | ||
| current_user, new_domain | ||
| ) | ||
|
|
||
| if domain_forbidden_cause: | ||
| flash(domain_forbidden_cause.message(new_domain), "error") | ||
| return redirect(url_for("dashboard.setting")) | ||
|
|
||
| BlockedDomain.create(user_id=current_user.id, domain=new_domain) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add an user audit log here to keep track on what happened. |
||
|
|
||
| LOG.i( | ||
| f"A new blocked domain [{new_domain}] was added for user [{current_user.id}]" | ||
| ) | ||
|
|
||
| Session.commit() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missign user audit log on the action done |
||
| flash(f"Added blocked domain [{domain}]", "success") | ||
| return redirect(url_for("dashboard.setting")) | ||
| elif request.form.get("form-name") == "blocked-domains-remove": | ||
| domain_id = request.form.get("domain_id") | ||
| domain_name = request.form.get("domain_name") | ||
|
|
||
| if not domain_id: | ||
| flash("Domain Id is missing", "error") | ||
| return redirect(url_for("dashboard.setting")) | ||
|
|
||
| if not domain_name: | ||
| flash("Domain Name is missing", "error") | ||
| return redirect(url_for("dashboard.setting")) | ||
|
|
||
| domain = BlockedDomain.get(domain_id) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If domain_id is none this will crash
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If domain_id is None it will fail with an exception |
||
| if not domain or domain.user_id != current_user.id: | ||
| LOG.e( | ||
| f"Blocked domain with id [{domain_id}] not found or not owned by user [{current_user.id}] therefore couldn't be deleted" | ||
| ) | ||
| flash( | ||
| "Blocked domain not found or the user doesn't have access to it", | ||
| "error", | ||
| ) | ||
| return redirect(url_for("dashboard.setting")) | ||
|
|
||
| BlockedDomain.delete(domain.id) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any user seems to be able to delete any domain.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I now validate if the current user is the owner of the domain before deleting it, let me know if that looks ok deleted = (
BlockedDomain.filter_by(
id=domain_id_int,
user_id=current_user.id,
).delete()
)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any user can delete any domain |
||
|
|
||
| LOG.i( | ||
| f"A blocked domain [{domain}] was deleted by user [{current_user.id}]" | ||
| ) | ||
|
|
||
| Session.commit() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. Missing an user audit log. |
||
| flash(f"Deleted blocked domain [{domain_name}]", "success") | ||
| return redirect(url_for("dashboard.setting")) | ||
|
|
||
| manual_sub = ManualSubscription.get_by(user_id=current_user.id) | ||
| apple_sub = AppleSubscription.get_by(user_id=current_user.id) | ||
|
|
@@ -318,4 +383,5 @@ def setting(): | |
| ALIAS_RAND_SUFFIX_LENGTH=ALIAS_RANDOM_SUFFIX_LENGTH, | ||
| connect_with_proton=CONNECT_WITH_PROTON, | ||
| can_unlink_proton_account=can_unlink_proton_account(current_user), | ||
| blocked_domains=blocked_domains, | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,6 +61,7 @@ | |
| change_alias_status, | ||
| get_alias_recipient_name, | ||
| ) | ||
| from app.blocked_domain_utils import is_domain_blocked | ||
| from app.config import ( | ||
| EMAIL_DOMAIN, | ||
| URL, | ||
|
|
@@ -613,6 +614,21 @@ def handle_forward(envelope, msg: Message, rcpt_to: str) -> List[Tuple[bool, str | |
| handle_email_sent_to_ourself(alias, addr, msg, user) | ||
| return [(True, status.E209)] | ||
|
|
||
| mail_from_domain = get_email_domain_part(mail_from) | ||
| if is_domain_blocked(user.id, mail_from_domain): | ||
| LOG.i( | ||
| f"Email [{mail_from}] was ignored for the user [{user.id}] because of a blocked domain [{mail_from_domain}]" | ||
| ) | ||
| # by default return 2** instead of 5** to allow user to receive emails again when domain is unblocked | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a log here so that we can track when an email is dropped in the logs. |
||
| res_status = status.E200 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a log entry here to mark that the email has been dropped. |
||
| if user.block_behaviour == BlockBehaviourEnum.return_5xx: | ||
| LOG.i( | ||
| f"Email [{mail_from}] was rejected for the user [{user.id}] because of a blocked domain [{mail_from_domain}]" | ||
| ) | ||
| res_status = status.E502 | ||
|
|
||
| return [(True, res_status)] | ||
|
|
||
| from_header = get_header_unicode(msg[headers.FROM]) | ||
| LOG.d("Create or get contact for from_header:%s", from_header) | ||
| contact = get_or_create_contact(from_header, envelope.mail_from, alias) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| """empty message | ||
|
|
||
| Revision ID: c18048c40ed9 | ||
| Revises: 3ee37864eb67 | ||
| Create Date: 2025-10-10 20:29:32.701784 | ||
|
|
||
| """ | ||
| import sqlalchemy as sa | ||
| import sqlalchemy_utils | ||
| from alembic import op | ||
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision = 'c18048c40ed9' | ||
| down_revision = '3ee37864eb67' | ||
| branch_labels = None | ||
| depends_on = None | ||
|
|
||
|
|
||
| def upgrade(): | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.create_table('blocked_domain', | ||
| sa.Column('id', sa.Integer(), autoincrement=True, nullable=False), | ||
| sa.Column('created_at', sqlalchemy_utils.types.arrow.ArrowType(), nullable=False), | ||
| sa.Column('updated_at', sqlalchemy_utils.types.arrow.ArrowType(), nullable=True), | ||
| sa.Column('domain', sa.String(length=128), nullable=False), | ||
| sa.Column('user_id', sa.Integer(), nullable=False), | ||
| sa.ForeignKeyConstraint(['user_id'], ['users.id'], ondelete='cascade'), | ||
| sa.PrimaryKeyConstraint('id'), | ||
| sa.UniqueConstraint('user_id', 'domain', name='uq_blocked_domain') | ||
| ) | ||
| # ### end Alembic commands ### | ||
|
|
||
|
|
||
| def downgrade(): | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.drop_table('blocked_domain') | ||
| # ### end Alembic commands ### |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this changed. This would allow users to create a custom domain with an invalid domain or an SL domain.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the original code I modified the existing and created 2 separate functions (
can_domain_be_used,can_custom_domain_be_usedandcan_blocked_domain_be_usedrespectively).Because of that I also had to change this to call the function
can_custom_domain_be_usedto still include all the previously executed checks.But inside of that function I call the
can_domain_be_usedwhich should check for the domain validity and SL domain as well and if that reason is notNone, then return with that error.Currently in this branch that call is in line 119: