diff --git a/Makefile b/Makefile index 233c7eef..6f49a091 100644 --- a/Makefile +++ b/Makefile @@ -545,6 +545,8 @@ superuser: build-python-base ## Create an admin superuser with password "admin" @echo "$(BOLD)Creating a Django superuser$(RESET)" @$(MANAGE_DB) createsuperuser --email admin@admin.local --password admin @$(MANAGE_DB) createsuperuser --email user1@example.local --password user1 + @echo "$(BOLD)Creating the example.local autojoin domain$(RESET)" + @$(MANAGE_DB) shell -c "from core.models import MailDomain; MailDomain.objects.get_or_create(name='example.local', defaults={'oidc_autojoin': True, 'identity_sync': True})" .PHONY: superuser shell-back: ## open a shell in the backend container diff --git a/README.md b/README.md index b3cea329..6ec82da2 100644 --- a/README.md +++ b/README.md @@ -186,7 +186,7 @@ These examples use [swaks](https://www.jetmore.org/john/code/swaks/), a simple c make start # Send a test message to the MTA-in, which will relay it to the Django MDA. -# The domain must be MESSAGES_TESTDOMAIN (default is example.local) if you want the mailbox created automatically. +# The domain must belong to a MailDomain with oidc_autojoin=True if you want the mailbox created automatically. # You can then read it on the frontend at http://localhost:8900/ (login as user1/user1) and reply to it there. # The replies will then be sent to the mailcatcher on http://localhost:8904/ swaks --to=user1@example.local --server localhost:8910 diff --git a/deploy/env/backend.defaults b/deploy/env/backend.defaults index ec8cf623..f98487d2 100644 --- a/deploy/env/backend.defaults +++ b/deploy/env/backend.defaults @@ -111,8 +111,6 @@ KEYCLOAK_GROUP_PATH_PREFIX=/maildomain- # FRONTEND_LAGAUFRE_WIDGET_CONFIG={"api_url": "", "path": ""} # Messages -MESSAGES_TESTDOMAIN=example.local -MESSAGES_TESTDOMAIN_MAPPING_BASEDOMAIN=example.com MTA_OUT_MODE=relay MTA_OUT_RELAY_HOST=mailcatcher:1025 MDA_API_SECRET=my-shared-secret-mda diff --git a/docs/env.md b/docs/env.md index fd2719c3..0c080c88 100644 --- a/docs/env.md +++ b/docs/env.md @@ -105,8 +105,6 @@ The application uses a new environment file structure with `.defaults` and `.loc | Variable | Default | Description | Required | |----------|---------|-------------|----------| -| `MESSAGES_TESTDOMAIN` | `example.local` | Test domain for development | Dev | -| `MESSAGES_TESTDOMAIN_MAPPING_BASEDOMAIN` | `example.com` | Base domain mapping | Dev | | `MESSAGES_ACCEPT_ALL_EMAILS` | `False` | Accept emails to any domain | Optional | ### DKIM Configuration diff --git a/src/backend/core/authentication/backends.py b/src/backend/core/authentication/backends.py index d1e948f6..0345fa4f 100644 --- a/src/backend/core/authentication/backends.py +++ b/src/backend/core/authentication/backends.py @@ -1,9 +1,7 @@ """Authentication Backends for the messages core app.""" import logging -import re -from django.conf import settings from django.core.exceptions import SuspiciousOperation from lasuite.oidc_login.backends import ( @@ -72,7 +70,6 @@ class OIDCAuthenticationBackend(LaSuiteOIDCAuthenticationBackend): # if sub is absent, try matching on email user = self.get_existing_user(sub, email) - self.create_testdomain() if user: if not user.is_active: @@ -167,16 +164,6 @@ class OIDCAuthenticationBackend(LaSuiteOIDCAuthenticationBackend): except DuplicateEmailError as err: raise SuspiciousOperation(err.message) from err - def create_testdomain(self): - """Create the test domain if it doesn't exist.""" - - # Create the test domain if it doesn't exist - if settings.MESSAGES_TESTDOMAIN: - MailDomain.objects.get_or_create( - name=settings.MESSAGES_TESTDOMAIN, - defaults={"oidc_autojoin": True, "identity_sync": True}, - ) - def should_create_user(self, email): """Check if a user should be created based on the email address.""" @@ -187,11 +174,6 @@ class OIDCAuthenticationBackend(LaSuiteOIDCAuthenticationBackend): if self.get_settings("OIDC_CREATE_USER", True): return True - # MESSAGES_TESTDOMAIN_MAPPING_BASEDOMAIN is a special case of autojoin - testdomain_mapped_email = self.get_testdomain_mapped_email(email) - if testdomain_mapped_email: - return True - # If the email address ends with a domain that has autojoin enabled if MailDomain.objects.filter( name=email.split("@")[1], oidc_autojoin=True @@ -201,36 +183,11 @@ class OIDCAuthenticationBackend(LaSuiteOIDCAuthenticationBackend): # Don't create a user locally return False - def get_testdomain_mapped_email(self, email): - """If it exists, return the mapped email address for the test domain.""" - if not settings.MESSAGES_TESTDOMAIN or not email: - return None - - # Check if the email address ends with the test domain - if not re.search( - r"[@\.]" - + re.escape(settings.MESSAGES_TESTDOMAIN_MAPPING_BASEDOMAIN) - + r"$", - email, - ): - return None - - # => - prefix = email.split("@")[1][ - : -len(settings.MESSAGES_TESTDOMAIN_MAPPING_BASEDOMAIN) - 1 - ] - return ( - email.split("@")[0] - + ("-" + prefix if prefix else "") - + "@" - + settings.MESSAGES_TESTDOMAIN - ) - def autojoin_mailbox(self, user): """Setup autojoin mailbox for user.""" - email = self.get_testdomain_mapped_email(user.email) - if not email and user.email: + email = None + if user.email: # TODO aliases? if MailDomain.objects.filter( name=user.email.split("@")[1], oidc_autojoin=True diff --git a/src/backend/core/mda/inbound.py b/src/backend/core/mda/inbound.py index abd8a795..95a473d4 100644 --- a/src/backend/core/mda/inbound.py +++ b/src/backend/core/mda/inbound.py @@ -36,9 +36,6 @@ def check_local_recipient( # For unit testing, we accept all emails if settings.MESSAGES_ACCEPT_ALL_EMAILS: is_deliverable = True - # MESSAGES_TESTDOMAIN acts as a catch-all, if configured. - elif settings.MESSAGES_TESTDOMAIN == domain_name: - is_deliverable = True else: # Check if the email address exists in the database is_deliverable = models.Mailbox.objects.filter( @@ -68,7 +65,6 @@ def check_local_recipients(email_addresses: list[str]) -> set[str]: Returns a set of email addresses that are deliverable locally. An email is deliverable if: - MESSAGES_ACCEPT_ALL_EMAILS is True (test mode), or - - The domain matches MESSAGES_TESTDOMAIN (catch-all), or - A mailbox exists for that email address """ if not email_addresses: @@ -92,14 +88,6 @@ def check_local_recipients(email_addresses: list[str]) -> set[str]: except ValueError: pass # Invalid email format, not deliverable - # Handle MESSAGES_TESTDOMAIN - acts as catch-all - test_domain = settings.MESSAGES_TESTDOMAIN - if test_domain: - for email, (_, domain) in email_parts.items(): - if domain == test_domain: - deliverable.add(email) - domains.discard(test_domain) - # Query all mailboxes on the relevant domains in a single query if domains: existing_mailboxes = set( diff --git a/src/backend/core/tests/api/test_inbound_mta.py b/src/backend/core/tests/api/test_inbound_mta.py index 662e1e91..2a4f2f06 100644 --- a/src/backend/core/tests/api/test_inbound_mta.py +++ b/src/backend/core/tests/api/test_inbound_mta.py @@ -520,7 +520,6 @@ class TestMTAInboundEmail: class TestMTACheckRecipients: """Test the MTA check recipients endpoint.""" - @override_settings(MESSAGES_TESTDOMAIN="testdomain.com") def test_check_recipients(self, api_client, valid_jwt_token): """Test checking recipients with valid JWT token.""" @@ -532,10 +531,8 @@ class TestMTACheckRecipients: { "addresses": [ "recipient@validdomain.com", - "recipient@testdomain.com", "recipient@invaliddomain.com", "recipient@not.validdomain.com", - "recipient@sub.testdomain.com", ] } ).encode("utf-8") @@ -551,10 +548,8 @@ class TestMTACheckRecipients: assert response.status_code == status.HTTP_200_OK assert response.json() == { "recipient@validdomain.com": True, - "recipient@testdomain.com": True, "recipient@invaliddomain.com": False, "recipient@not.validdomain.com": False, - "recipient@sub.testdomain.com": False, } def test_check_recipients_invalid_token(self, api_client, valid_jwt_token): diff --git a/src/backend/core/tests/authentication/test_backends.py b/src/backend/core/tests/authentication/test_backends.py index a81439a4..b9b5e9a2 100644 --- a/src/backend/core/tests/authentication/test_backends.py +++ b/src/backend/core/tests/authentication/test_backends.py @@ -18,7 +18,6 @@ from core.factories import UserFactory pytestmark = pytest.mark.django_db -@override_settings(MESSAGES_TESTDOMAIN=None) def test_authentication_getter_existing_user_no_email(monkeypatch): """ If an existing user matches the user's info sub, the user should be returned. @@ -39,7 +38,6 @@ def test_authentication_getter_existing_user_no_email(monkeypatch): assert user == db_user -@override_settings(MESSAGES_TESTDOMAIN=None) def test_authentication_getter_existing_user_via_email(monkeypatch): """ If an existing user doesn't match the sub but matches the email, @@ -152,7 +150,6 @@ def test_authentication_getter_existing_user_no_fallback_to_email_no_duplicate( @override_settings( - MESSAGES_TESTDOMAIN=None, OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION=False, OIDC_ALLOW_DUPLICATE_EMAILS=False, OIDC_CREATE_USER=True, @@ -186,7 +183,6 @@ def test_authentication_getter_claims_passwordless_stub_by_email(monkeypatch): assert models.MailboxAccess.objects.filter(user=user, mailbox=mailbox).exists() -@override_settings(MESSAGES_TESTDOMAIN=None) def test_authentication_getter_existing_user_with_email(monkeypatch): """ When the user's info contains an email and targets an existing user, @@ -211,7 +207,6 @@ def test_authentication_getter_existing_user_with_email(monkeypatch): assert user == authenticated_user -@override_settings(MESSAGES_TESTDOMAIN=None) @pytest.mark.parametrize( "first_name, last_name, email", [ @@ -251,7 +246,6 @@ def test_authentication_getter_existing_user_change_fields_sub( assert user.full_name == f"{first_name:s} {last_name:s}" -@override_settings(MESSAGES_TESTDOMAIN=None) @pytest.mark.parametrize( "first_name, last_name, email", [ @@ -308,7 +302,7 @@ def test_authentication_getter_new_user_no_email(monkeypatch): assert user is None -@override_settings(MESSAGES_TESTDOMAIN="example.local") +@override_settings(OIDC_CREATE_USER=True) def test_authentication_getter_new_user_with_email(monkeypatch): """ If no user matches the user's info sub, a user should be created. @@ -362,85 +356,6 @@ def test_authentication_getter_existing_disabled_user_via_email(monkeypatch): assert models.User.objects.count() == 1 -@override_settings( - OIDC_OP_USER_ENDPOINT="http://oidc.endpoint.test/userinfo", - OIDC_USERINFO_ESSENTIAL_CLAIMS=["email", "last_name"], - MESSAGES_TESTDOMAIN="testdomain.bzh", - MESSAGES_TESTDOMAIN_MAPPING_BASEDOMAIN="gouv.fr", -) -def test_authentication_getter_new_user_with_testdomain(monkeypatch): - """ - Check the TESTDOMAIN creation process - """ - - klass = OIDCAuthenticationBackend() - - def get_userinfo_mocked(*args): - return { - "email": "john.doe@sub.gouv.fr", - "last_name": "Doe", - "sub": "123", - } - - monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked) - - user = klass.get_or_create_user( - access_token="test-token", id_token=None, payload=None - ) - - assert models.User.objects.filter(id=user.id).exists() - - assert user.sub == "123" - assert user.full_name == "Doe" - assert user.email == "john.doe@sub.gouv.fr" - - maildomain = models.MailDomain.objects.get(name="testdomain.bzh") - mailbox = models.Mailbox.objects.get(local_part="john.doe-sub", domain=maildomain) - - assert models.Contact.objects.filter( - email="john.doe-sub@testdomain.bzh", mailbox=mailbox - ).exists() - assert models.Mailbox.objects.filter( - local_part="john.doe-sub", domain=maildomain - ).exists() - assert models.MailboxAccess.objects.filter( - mailbox=mailbox, - user=user, - role=models.MailboxRoleChoices.ADMIN, - ).exists() - - -@override_settings( - OIDC_OP_USER_ENDPOINT="http://oidc.endpoint.test/userinfo", - OIDC_USERINFO_ESSENTIAL_CLAIMS=["email", "last_name"], - MESSAGES_TESTDOMAIN="testdomain.bzh", - MESSAGES_TESTDOMAIN_MAPPING_BASEDOMAIN="gouv.fr", -) -def test_authentication_getter_new_user_with_testdomain_no_mapping(monkeypatch): - """ - Check the TESTDOMAIN creation process when email doesn't match - """ - - klass = OIDCAuthenticationBackend() - - def get_userinfo_mocked(*args): - return { - "email": "john.doe@notgouv.fr", - "last_name": "Doe", - "sub": "123", - } - - monkeypatch.setattr(OIDCAuthenticationBackend, "get_userinfo", get_userinfo_mocked) - - user = klass.get_or_create_user( - access_token="test-token", id_token=None, payload=None - ) - - assert user is None - - assert models.User.objects.count() == 0 - - @responses.activate @override_settings( OIDC_OP_TOKEN_ENDPOINT="http://oidc.endpoint.test/token", @@ -449,7 +364,7 @@ def test_authentication_getter_new_user_with_testdomain_no_mapping(monkeypatch): OIDC_STORE_ACCESS_TOKEN=True, OIDC_STORE_REFRESH_TOKEN=True, OIDC_STORE_REFRESH_TOKEN_KEY=Fernet.generate_key(), - MESSAGES_TESTDOMAIN="example.local", + OIDC_CREATE_USER=True, ) def test_authentication_session_tokens(monkeypatch, rf, settings): """ diff --git a/src/backend/core/tests/conftest.py b/src/backend/core/tests/conftest.py index 495673c0..72a2958b 100644 --- a/src/backend/core/tests/conftest.py +++ b/src/backend/core/tests/conftest.py @@ -138,16 +138,3 @@ def _assert_user_event_thread_invariant(request): f"UserEvent invariant broken: {count} row(s) where " "thread_id != thread_event.thread_id" ) - - -# @pytest.fixture -# @pytest.mark.django_db -# def create_testdomain(): -# """Create the TESTDOMAIN.""" -# from core import models -# models.MailDomain.objects.get_or_create( -# name=settings.MESSAGES_TESTDOMAIN, -# defaults={ -# "oidc_autojoin": True -# } -# ) diff --git a/src/backend/core/tests/mda/test_inbound.py b/src/backend/core/tests/mda/test_inbound.py index 37026849..1240ad98 100644 --- a/src/backend/core/tests/mda/test_inbound.py +++ b/src/backend/core/tests/mda/test_inbound.py @@ -467,9 +467,7 @@ class TestDeliverInboundMessage: ).exists() assert models.Message.objects.count() == 1 # Check message was delivered - @override_settings( - MESSAGES_ACCEPT_ALL_EMAILS=False, MESSAGES_TESTDOMAIN="something.else" - ) + @override_settings(MESSAGES_ACCEPT_ALL_EMAILS=False) def test_mailbox_creation_disabled(self, sample_parsed_email, raw_email_data): """Test delivery fails if mailbox doesn't exist and auto-creation is off.""" recipient_addr = "nonexistent@disabled.test" @@ -1059,7 +1057,7 @@ class TestInboundAutoreplyIntegration: assert result is True mock_try_autoreply.assert_not_called() - @override_settings(MESSAGES_ACCEPT_ALL_EMAILS=False, MESSAGES_TESTDOMAIN="") + @override_settings(MESSAGES_ACCEPT_ALL_EMAILS=False) @patch("core.mda.autoreply.try_send_autoreply") def test_autoreply_not_called_on_failed_delivery( self, mock_try_autoreply, sample_parsed_email diff --git a/src/backend/messages/settings.py b/src/backend/messages/settings.py index e8d09fc6..b7d06c5d 100644 --- a/src/backend/messages/settings.py +++ b/src/backend/messages/settings.py @@ -599,15 +599,6 @@ class Base(Configuration): "may", environ_name="MTA_OUT_SMTP_TLS_SECURITY_LEVEL", environ_prefix=None ) - # Test domain settings - MESSAGES_TESTDOMAIN = values.Value( - None, environ_name="MESSAGES_TESTDOMAIN", environ_prefix=None - ) - MESSAGES_TESTDOMAIN_MAPPING_BASEDOMAIN = values.Value( - None, - environ_name="MESSAGES_TESTDOMAIN_MAPPING_BASEDOMAIN", - environ_prefix=None, - ) MESSAGES_ACCEPT_ALL_EMAILS = values.BooleanValue( default=False, environ_name="MESSAGES_ACCEPT_ALL_EMAILS", @@ -1529,6 +1520,21 @@ class Base(Configuration): self.MTA_OUT_RELAY_USERNAME = os.environ.get("MTA_OUT_SMTP_USERNAME") self.MTA_OUT_RELAY_PASSWORD = os.environ.get("MTA_OUT_SMTP_PASSWORD") + # The MESSAGES_TESTDOMAIN feature (catch-all domain + email mapping + + # auto-creation of the test domain) has been removed. Warn loudly if the + # env vars are still configured so operators know they now do nothing. + for removed_var in ( + "MESSAGES_TESTDOMAIN", + "MESSAGES_TESTDOMAIN_MAPPING_BASEDOMAIN", + ): + if os.environ.get(removed_var): + logger.warning( + "[REMOVED] %s is set but no longer used: the TESTDOMAIN " + "feature has been removed. Use a MailDomain with " + "oidc_autojoin=True instead. Please unset this variable.", + removed_var, + ) + if self.MTA_OUT_SMTP_TLS_SECURITY_LEVEL not in {"none", "may", "secure"}: raise ValueError( f"Invalid MTA_OUT_SMTP_TLS_SECURITY_LEVEL: {self.MTA_OUT_SMTP_TLS_SECURITY_LEVEL}"