Review email configuration

.github/workflows/oauth.yaml

@@ -56,20 +56,28 @@ jobs:
             FRACTAL_RUNNER_BACKEND: local
             JWT_SECRET_KEY: jwt_secret_key
             JWT_EXPIRE_SECONDS: 1000
+            # Postgres
             POSTGRES_USER: postgres
             POSTGRES_PASSWORD: postgres
             POSTGRES_DB: fractal_test
             POSTGRES_HOST: localhost
             POSTGRES_PORT: 5432
-            # FRACTAL_EMAIL_SETTINGS and KEY are generated with the following command
-            # `printf "fakepassword\n" | poetry run fractalctl email-settings sender@example.org localhost 1025 test --skip-starttls`
-            FRACTAL_EMAIL_SETTINGS: gAAAAABnYvLgoSeECnrXlv1UoP4D_c9Of0xmwMJVopBA3TIDjOvx6YDVfe2ULz8yGr8Ba5Id8rRLjCXa_Ys8iHjvuniJyvsX0mDrc3IGSoofMEeeSCvYEe4iSWLeb_qTNVNPc4IT2-SLB-F7dEvkwzyAFnEm9dVmApd4_lQLm9_wJoS-tz1Q1K8E1_jJSgpfGgwHaINHICVh1UL_qHjIa3DwFvDPvt32tLLBZTL7oN88A8RCmg00ThIZs4HN7OQkvfninfOiM060Lb-AeNViCVgBX-bIPWZaeQ==
-            FRACTAL_EMAIL_SETTINGS_KEY: 4otDt3R-8p4S97QT0gcUzynCalByypTv01YntqQ9XFk=
-            FRACTAL_EMAIL_RECIPIENTS: recipient1@example.org,recipient2@example.org
+            # Dex (OAuth)
             OAUTH_DEXIDP_CLIENT_ID: client_test_id
             OAUTH_DEXIDP_CLIENT_SECRET: client_test_secret
             OAUTH_DEXIDP_REDIRECT_URL: http://localhost:8001/auth/dexidp/callback/
             OAUTH_DEXIDP_OIDC_CONFIGURATION_ENDPOINT: http://127.0.0.1:5556/dex/.well-known/openid-configuration
+            # Email
+            FRACTAL_EMAIL_SENDER: sender@example.org
+            FRACTAL_EMAIL_SMTP_SERVER: localhost
+            FRACTAL_EMAIL_PORT: 1025
+            FRACTAL_EMAIL_INSTANCE_NAME: test
+            FRACTAL_EMAIL_RECIPIENTS: recipient1@example.org,recipient2@example.org
+            FRACTAL_EMAIL_USE_STARTTLS: false
+            # FRACTAL_EMAIL_PASSWORD and FRACTAL_EMAIL_PASSWORD_KET are generated with the following command
+            # `printf "fakepassword\n" | poetry run fractalctl encrypt-email-password`
+            FRACTAL_EMAIL_PASSWORD: gAAAAABnoQUGHMsDgLkpDtwUtrKtf9T1so44ahEXExGRceAnf097mVY1EbNuMP5fjvkndvwCwBJM7lHoSgKQkZ4VbvO9t3PJZg==
+            FRACTAL_EMAIL_PASSWORD_KEY: lp3j2FVDkzLd0Rklnzg1pHuV9ClCuDE0aGeJfTNCaW4=
           run: |
             fractalctl set-db
             fractalctl start --port 8001 &

fractal_server/__main__.py

@@ -63,45 +63,15 @@
     description="Apply data-migration script to an existing database.",
 )
 
-# fractalctl email-settings
-email_settings_parser = subparsers.add_parser(
-    "email-settings",
-    description=(
-        "Generate valid values for environment variables "
-        "`FRACTAL_EMAIL_SETTINGS` and `FRACTAL_EMAIL_SETTINGS_KEY`."
-    ),
-)
-email_settings_parser.add_argument(
-    "sender",
-    type=str,
-    help="Email of the sender",
-)
-email_settings_parser.add_argument(
-    "server",
-    type=str,
-    help="SMPT server used to send emails",
-)
-email_settings_parser.add_argument(
-    "port",
-    type=int,
-    help="Port of the SMPT server",
-)
-email_settings_parser.add_argument(
-    "instance",
-    type=str,
-    help="Name of the Fractal instance sending emails",
-)
-email_settings_parser.add_argument(
-    "--skip-starttls",
-    action="store_true",
-    default=False,
-    help="If set, skip the execution of `starttls` when sending emails",
+# fractalctl encrypt-email-password
+encrypt_email_password_parser = subparsers.add_parser(
+    "encrypt-email-password",
+    description="TBD",
 )
 
 
 def save_openapi(dest="openapi.json"):
     from fractal_server.main import start_application
-    import json
 
     app = start_application()
     openapi_schema = app.openapi()
@@ -227,31 +197,15 @@ def _slugify_version(raw_version: str) -> str:
     current_update_db_data_module.fix_db()
 
 
-def print_mail_settings(
-    sender: str,
-    server: str,
-    port: int,
-    instance: str,
-    skip_starttls: bool,
-):
+def print_encrypted_password():
     from cryptography.fernet import Fernet
 
-    password = input(f"Insert email password for sender '{sender}': ")
+    password = input("Insert email password: ").encode("utf-8")
     key = Fernet.generate_key().decode("utf-8")
-    fractal_mail_settings = json.dumps(
-        dict(
-            sender=sender,
-            password=password,
-            smtp_server=server,
-            port=port,
-            instance_name=instance,
-            use_starttls=(not skip_starttls),
-        )
-    ).encode("utf-8")
-    email_settings = Fernet(key).encrypt(fractal_mail_settings).decode("utf-8")
+    encrypted_password = Fernet(key).encrypt(password).decode("utf-8")
 
-    print(f"\nFRACTAL_EMAIL_SETTINGS: {email_settings}")
-    print(f"FRACTAL_EMAIL_SETTINGS_KEY: {key}")
+    print(f"\nFRACTAL_EMAIL_PASSWORD: {encrypted_password}")
+    print(f"FRACTAL_EMAIL_PASSWORD_KEY: {key}")
 
 
 def run():
@@ -270,14 +224,8 @@ def run():
             port=args.port,
             reload=args.reload,
         )
-    elif args.cmd == "email-settings":
-        print_mail_settings(
-            sender=args.sender,
-            server=args.server,
-            port=args.port,
-            instance=args.instance,
-            skip_starttls=args.skip_starttls,
-        )
+    elif args.cmd == "encrypt-email-password":
+        print_encrypted_password()
     else:
         sys.exit(f"Error: invalid command '{args.cmd}'.")
 

fractal_server/config.py

@@ -11,7 +11,6 @@
 # <exact-lab.it> under contract with Liberali Lab from the Friedrich Miescher
 # Institute for Biomedical Research and Pelkmans Lab from the University of
 # Zurich.
-import json
 import logging
 import shutil
 import sys
@@ -47,6 +46,7 @@
         password: Sender password
         instance_name: Name of SMTP server instance
         use_starttls: Using or not security protocol
+        use_login: TBD  # FIXME
     """
 
     sender: EmailStr
@@ -56,6 +56,7 @@
     password: str
     instance_name: str
     use_starttls: bool
+    use_login: bool
 
 
 class FractalConfigurationError(RuntimeError):
@@ -406,7 +407,7 @@
     """
 
     @root_validator(pre=True)
-    def check_tasks_python(cls, values) -> None:
+    def check_tasks_python(cls, values):
         """
         Perform multiple checks of the Python-interpreter variables.
 
@@ -416,7 +417,6 @@
             `sys.executable` and set the corresponding
             `FRACTAL_TASKS_PYTHON_X_Y` (and unset all others).
         """
-
         # `FRACTAL_TASKS_PYTHON_X_Y` variables can only be absolute paths
         for version in ["3_9", "3_10", "3_11", "3_12"]:
             key = f"FRACTAL_TASKS_PYTHON_{version}"
@@ -575,66 +575,95 @@
     ###########################################################################
     # SMTP SERVICE
     ###########################################################################
-    FRACTAL_EMAIL_SETTINGS: Optional[str] = None
+
+    FRACTAL_EMAIL_SENDER: Optional[str] = None
+    """
+    TBD
+    """
+    FRACTAL_EMAIL_PASSWORD: Optional[str] = None
     """
-    Encrypted version of settings dictionary, with keys `sender`, `password`,
-    `smtp_server`, `port`, `instance_name`, `use_starttls`.
+    TBD
     """
-    FRACTAL_EMAIL_SETTINGS_KEY: Optional[str] = None
+    FRACTAL_EMAIL_PASSWORD_KEY: Optional[str] = None
     """
     Key value for `cryptography.fernet` decrypt
     """
+    FRACTAL_EMAIL_SMTP_SERVER: Optional[str] = None
+    """
+    TBD
+    """
+    FRACTAL_EMAIL_PORT: Optional[int] = None
+    """
+    TBD
+    """
+    FRACTAL_EMAIL_INSTANCE_NAME: Optional[str] = None
+    """
+    TBD
+    """
     FRACTAL_EMAIL_RECIPIENTS: Optional[str] = None
     """
     List of email receivers, separated with commas
     """
+    FRACTAL_EMAIL_USE_STARTTLS: Optional[bool] = True
+    """
+    TBD
+    """
+    FRACTAL_EMAIL_USE_LOGIN: Optional[bool] = True
+    """
+    TBD
+    """
+
+    @root_validator(pre=True)
+    def validate_email_settings(cls, values):
+        email_values = {k: v for k, v in values.items() if "EMAIL" in k}
+        if email_values:
+
+            def assert_key(key: str):
+                if key not in email_values:
+                    raise ValueError(f"Missing '{key}'")
+
+            assert_key("FRACTAL_EMAIL_SENDER")
+            assert_key("FRACTAL_EMAIL_SMTP_SERVER")
+            assert_key("FRACTAL_EMAIL_PORT")
+            assert_key("FRACTAL_EMAIL_INSTANCE_NAME")
+            assert_key("FRACTAL_EMAIL_RECIPIENTS")
+            if email_values.get("FRACTAL_EMAIL_USE_LOGIN") is not False and (
+                "FRACTAL_EMAIL_PASSWORD" not in email_values
+                or "FRACTAL_EMAIL_PASSWORD_KEY" not in email_values
+            ):
+                raise ValueError(
+                    "'FRACTAL_EMAIL_USE_LOGIN' is True but "
+                    "'FRACTAL_EMAIL_PASSWORD' or 'FRACTAL_EMAIL_PASSWORD_KEY' "
+                    "are provided."
+                )
+        return values
 
     @property
     def MAIL_SETTINGS(self) -> Optional[MailSettings]:
-        if (
-            self.FRACTAL_EMAIL_SETTINGS is not None
-            and self.FRACTAL_EMAIL_SETTINGS_KEY is not None
-            and self.FRACTAL_EMAIL_RECIPIENTS is not None
-        ):
-            smpt_settings = (
-                Fernet(self.FRACTAL_EMAIL_SETTINGS_KEY)
-                .decrypt(self.FRACTAL_EMAIL_SETTINGS)
-                .decode("utf-8")
-            )
-            recipients = self.FRACTAL_EMAIL_RECIPIENTS.split(",")
-            mail_settings = MailSettings(
-                **json.loads(smpt_settings), recipients=recipients
-            )
-            return mail_settings
-        elif not all(
-            [
-                self.FRACTAL_EMAIL_RECIPIENTS is None,
-                self.FRACTAL_EMAIL_SETTINGS_KEY is None,
-                self.FRACTAL_EMAIL_SETTINGS is None,
-            ]
-        ):
-            raise ValueError(
-                "You must set all SMPT config variables: "
-                f"{self.FRACTAL_EMAIL_SETTINGS=}, "
-                f"{self.FRACTAL_EMAIL_RECIPIENTS=}, "
-                f"{self.FRACTAL_EMAIL_SETTINGS_KEY=}, "
-            )
+        if self.FRACTAL_EMAIL_SENDER is None:
+            return None
+        password = (
+            Fernet(self.FRACTAL_EMAIL_PASSWORD_KEY)
+            .decrypt(self.FRACTAL_EMAIL_PASSWORD)
+            .decode("utf-8")
+        )
+        recipients = self.FRACTAL_EMAIL_RECIPIENTS.split(",")
+        mail_settings = MailSettings(
+            sender=self.FRACTAL_EMAIL_SENDER,
+            password=password,
+            recipients=recipients,
+            smtp_server=self.FRACTAL_EMAIL_SMTP_SERVER,
+            port=self.FRACTAL_EMAIL_PORT,
+            instance_name=self.FRACTAL_EMAIL_INSTANCE_NAME,
+            use_starttls=self.FRACTAL_EMAIL_USE_STARTTLS,
+            use_login=self.FRACTAL_EMAIL_USE_LOGIN,
+        )
+        return mail_settings
 
     ###########################################################################
     # BUSINESS LOGIC
     ###########################################################################
 
-    def check_fractal_mail_settings(self):
-        """
-        Checks that the mail settings are properly set.
-        """
-        try:
-            self.MAIL_SETTINGS
-        except Exception as e:
-            raise FractalConfigurationError(
-                f"Invalid email configuration settings. Original error: {e}"
-            )
-
     def check_db(self) -> None:
         """
         Checks that db environment variables are properly set.
@@ -740,7 +769,6 @@
 
         self.check_db()
         self.check_runner()
-        self.check_fractal_mail_settings()
 
     def get_sanitized(self) -> dict:
         def _must_be_sanitized(string) -> bool:

fractal_server/main.py

@@ -68,7 +68,7 @@ def check_settings() -> None:
     logger = set_logger("fractal_server_settings")
     logger.debug("Fractal Settings:")
     for key, value in settings.dict().items():
-        if any(s in key.upper() for s in ["PASSWORD", "SECRET"]):
+        if any(s in key.upper() for s in ["PASSWORD", "SECRET", "KEY"]):
             value = "*****"
         logger.debug(f"  {key}: {value}")
     reset_logger_handlers(logger)

tests/no_version/test_commands.py

@@ -103,8 +103,7 @@ def test_email_settings():
 
     cmd = (
         'printf "mypassword\n" | '
-        "poetry run fractalctl email-settings "
-        "sender@test.org localhost 1234 exact --skip-starttls"
+        "poetry run fractalctl encrypt-email-password"
     )
     res = subprocess.run(
         cmd,
@@ -113,6 +112,6 @@ def test_email_settings():
         cwd=FRACTAL_SERVER_DIR,
         shell=True,
     )
-    assert "FRACTAL_EMAIL_SETTINGS" in res.stdout
-    assert "FRACTAL_EMAIL_SETTINGS_KEY" in res.stdout
+    assert "FRACTAL_EMAIL_PASSWORD" in res.stdout
+    assert "FRACTAL_EMAIL_PASSWORD_KEY" in res.stdout
     assert not res.stderr

tests/no_version/test_email.py

@@ -7,17 +7,18 @@
 
 async def test_server_not_available(override_settings_factory, db, caplog):
     override_settings_factory(
-        FRACTAL_EMAIL_SETTINGS=(
-            "gAAAAABnYvLgoSeECnrXlv1UoP4D_c9Of0xmwMJVopBA3TIDjOvx6YDVfe2ULz8yG"
-            "r8Ba5Id8rRLjCXa_Ys8iHjvuniJyvsX0mDrc3IGSoofMEeeSCvYEe4iSWLeb_qTNV"
-            "NPc4IT2-SLB-F7dEvkwzyAFnEm9dVmApd4_lQLm9_wJoS-tz1Q1K8E1_jJSgpfGgw"
-            "HaINHICVh1UL_qHjIa3DwFvDPvt32tLLBZTL7oN88A8RCmg00ThIZs4HN7OQkvfni"
-            "nfOiM060Lb-AeNViCVgBX-bIPWZaeQ=="
+        FRACTAL_EMAIL_PASSWORD=(
+            "gAAAAABnoQUGHMsDgLkpDtwUtrKtf9T1so44ahEXExGRceAnf097mVY1EbNuMP5fj"
+            "vkndvwCwBJM7lHoSgKQkZ4VbvO9t3PJZg=="
         ),
-        FRACTAL_EMAIL_SETTINGS_KEY=(
-            "4otDt3R-8p4S97QT0gcUzynCalByypTv01YntqQ9XFk="
+        FRACTAL_EMAIL_PASSWORD_KEY=(
+            "lp3j2FVDkzLd0Rklnzg1pHuV9ClCuDE0aGeJfTNCaW4="
         ),
         FRACTAL_EMAIL_RECIPIENTS="test@example.org",
+        FRACTAL_EMAIL_SENDER="fractal@fractal.fractal",
+        FRACTAL_EMAIL_SMTP_SERVER="localhost",
+        FRACTAL_EMAIL_PORT="1234",
+        FRACTAL_EMAIL_INSTANCE_NAME="fractal",
     )
     user = UserOAuth(
         email="user@example.org",
@@ -48,6 +49,5 @@ async def test_server_not_available(override_settings_factory, db, caplog):
 
     async with contextlib.asynccontextmanager(get_user_manager)() as um:
         await um.on_after_register(user)
-
     assert "ERROR sending notification email" in caplog.text
     assert "[Errno 111] Connection refused" in caplog.text