Refactor mail config settings (#614)

- Updating testing to check this
- Allow custom env files to be passed to `create_app`

pydatalab/docs/config.md

@@ -55,7 +55,8 @@ We are looking for ways around this in the future.
 #### Email magic links
 
 To support sign-in via email magic-links, you must currently provide additional configuration for authorized SMTP server.
-The SMTP server must be configured via the settings [`EMAIL_AUTH_SMTP_SETTINGS`][pydatalab.config.ServerConfig.EMAIL_AUTH_SMTP_SETTINGS], with expected values `MAIL_SERVER`, `MAIL_USER`, `MAIL_PASSWORD`, `MAIL_DEFAULT_SENDER`, `MAIL_PORT` and `MAIL_USE_TLS`, following the environment variables described in the [Flask-Mail documentation](https://flask-mail.readthedocs.io/en/latest/#configuring-flask-mail).
+The SMTP server must be configured via the settings [`EMAIL_AUTH_SMTP_SETTINGS`][pydatalab.config.ServerConfig.EMAIL_AUTH_SMTP_SETTINGS], with expected values `MAIL_SERVER`, `MAIL_USER`, `MAIL_DEFAULT_SENDER`, `MAIL_PORT` and `MAIL_USE_TLS`, following the environment variables described in the [Flask-Mail documentation](https://flask-mail.readthedocs.io/en/latest/#configuring-flask-mail).
+The `MAIL_PASSWORD` setting should then be provided via a `.env` file.
 
 Third-party options could include [SendGrid](https://sendgrid.com/), which can be configured to use the `MAIL_USER` `apikey` with an appropriate API key, after verifying ownership of the `MAIL_DEFAULT_SENDER` address via DNS (see [the SendGrid documentation](https://sendgrid.com/en-us/blog/sending-emails-from-python-flask-applications-with-twilio-sendgrid) for an example configuration).
 

pydatalab/pydatalab/config.py

@@ -111,8 +111,10 @@ class SMTPSettings(BaseModel):
 
     MAIL_SERVER: str = Field("127.0.0.1", description="The SMTP server to use for sending emails.")
     MAIL_PORT: int = Field(587, description="The port to use for the SMTP server.")
-    MAIL_USERNAME: str = Field("", description="The username to use for the SMTP server.")
-    MAIL_PASSWORD: str = Field("", description="The password to use for the SMTP server.")
+    MAIL_USERNAME: str = Field(
+        "",
+        description="The username to use for the SMTP server. Will use the externally provided `MAIL_PASSWORD` environment variable for authentication.",
+    )
     MAIL_USE_TLS: bool = Field(True, description="Whether to use TLS for the SMTP connection.")
     MAIL_DEFAULT_SENDER: str = Field(
         "", description="The email address to use as the sender for emails."
@@ -195,8 +197,8 @@ class ServerConfig(BaseSettings):
         description="A list of domains for which user's will be able to register accounts if they have a matching email address. Setting the value to `None` will allow any email addresses at any domain to register an account, otherwise the default `[]` will not allow any email addresses.",
     )
 
-    EMAIL_AUTH_SMTP_SETTINGS: SMTPSettings = Field(
-        SMTPSettings(),
+    EMAIL_AUTH_SMTP_SETTINGS: Optional[SMTPSettings] = Field(
+        None,
         description="A dictionary containing SMTP settings for sending emails for account registration.",
     )
 
@@ -249,7 +251,6 @@ def validate_identifier_prefix(cls, v, values):
         The app startup will test for this value and should also warn aggressively that this is unset.
 
         """
-
         if values.get("TESTING") or v is None:
             return "test"
 

pydatalab/pydatalab/main.py

@@ -27,7 +27,23 @@ def _warn_startup_settings(app):
     """
 
     if CONFIG.EMAIL_AUTH_SMTP_SETTINGS is None:
-        LOGGER.warning("No email auth SMTP settings provided, email registration will not work.")
+        LOGGER.warning(
+            "No email auth SMTP settings provided, email registration will not be enabled."
+        )
+    else:
+        if app.config["MAIL_SERVER"] and not app.config.get("MAIL_PASSWORD"):
+            LOGGER.critical(
+                "CONFIG.EMAIL_AUTH_SMTP_SETTINGS.MAIL_SERVER was set to '%s' but no `MAIL_PASSWORD` was provided. "
+                "This can be passed in a `.env` file (as `MAIL_PASSWORD`) or as an environment variable.",
+                app.config["MAIL_SERVER"],
+            )
+        if not app.config["MAIL_DEFAULT_SENDER"]:
+            LOGGER.critical(
+                "CONFIG.EMAIL_AUTH_SMTP_SETTINGS.MAIL_DEFAULT_SENDER is not set in the config. "
+                "Email authentication may not work correctly."
+                "This can be set in the config above or equivalently via `MAIL_DEFAULT_SENDER` in a `.env` file, "
+                "or as an environment variable."
+            )
 
     if CONFIG.IDENTIFIER_PREFIX == "test":
         LOGGER.critical(
@@ -73,7 +89,9 @@ def _warn_startup_settings(app):
         )
 
 
-def create_app(config_override: Dict[str, Any] | None = None) -> Flask:
+def create_app(
+    config_override: Dict[str, Any] | None = None, env_file: pathlib.Path | None = None
+) -> Flask:
     """Create the main `Flask` app with the given config.
 
     Parameters:
@@ -93,8 +111,18 @@ def create_app(config_override: Dict[str, Any] | None = None) -> Flask:
         CONFIG.update(config_override)
 
     app.config.update(CONFIG.dict())
+
+    # This value will still be overwritten by any dotenv values
     app.config["MAIL_DEBUG"] = CONFIG.TESTING
-    app.config.update(dotenv_values())
+
+    # percolate datalab mail settings up to the `MAIL_` env vars/app config
+    # for use by Flask Mail
+    if CONFIG.EMAIL_AUTH_SMTP_SETTINGS is not None:
+        mail_settings = CONFIG.EMAIL_AUTH_SMTP_SETTINGS.dict()
+        for key in mail_settings:
+            app.config[key] = mail_settings[key]
+
+    app.config.update(dotenv_values(dotenv_path=env_file))
 
     LOGGER.info("Starting app with Flask app.config: %s", app.config)
     _warn_startup_settings(app)

pydatalab/pydatalab/send_email.py

@@ -2,7 +2,7 @@
 
 from flask_mail import Mail, Message
 
-from pydatalab.config import CONFIG
+from pydatalab.logger import LOGGER
 
 MAIL = Mail()
 
@@ -20,14 +20,14 @@ def send_mail(recipient: str, subject: str, body: str):
         body (str): The body of the email.
 
     """
-
-    if CONFIG.EMAIL_AUTH_SMTP_SETTINGS is None:
-        raise RuntimeError("No SMTP settings configured.")
+    LOGGER.debug("Sending email to %s", recipient)
 
     message = Message(
-        sender=CONFIG.EMAIL_AUTH_SMTP_SETTINGS.MAIL_DEFAULT_SENDER,
+        sender="[email protected]",
         recipients=[recipient],
         body=body,
         subject=subject,
     )
+    MAIL.connect()
     MAIL.send(message)
+    LOGGER.debug("Email sent to %s", recipient)

pydatalab/tests/test_config.py

@@ -2,7 +2,7 @@
 
 import pytest
 
-from pydatalab.config import ServerConfig
+from pydatalab.config import ServerConfig, SMTPSettings
 from pydatalab.main import create_app
 
 
@@ -42,5 +42,43 @@ def test_config_override():
 
 def test_validators():
     # check bad prefix
-    with pytest.raises(RuntimeError):
-        _ = ServerConfig(IDENTIFIER_PREFIX="this prefix is way way too long")
+    with pytest.raises(
+        RuntimeError, match="Identifier prefix must be less than 12 characters long,"
+    ):
+        _ = ServerConfig(IDENTIFIER_PREFIX="this prefix is way way too long", TESTING=False)
+
+
+def test_mail_settings_combinations(tmpdir):
+    """Tests that the config file mail settings get passed
+    correctly to the flask settings, and that additional
+    overrides can be provided as environment variables.
+    """
+
+    from pydatalab.config import CONFIG
+
+    CONFIG.update(
+        {
+            "EMAIL_AUTH_SMTP_SETTINGS": SMTPSettings(
+                MAIL_SERVER="example.com",
+                MAIL_DEFAULT_SENDER="[email protected]",
+                MAIL_PORT=587,
+                MAIL_USE_TLS=True,
+                MAIL_USERNAME="user",
+            )
+        }
+    )
+
+    app = create_app()
+    assert app.config["MAIL_SERVER"] == "example.com"
+    assert app.config["MAIL_DEFAULT_SENDER"] == "[email protected]"
+    assert app.config["MAIL_PORT"] == 587
+    assert app.config["MAIL_USE_TLS"] is True
+    assert app.config["MAIL_USERNAME"] == "user"
+
+    # write temporary .env file and check that it overrides the config
+    env_file = Path(tmpdir.join(".env"))
+    env_file.write_text("MAIL_PASSWORD=password\n[email protected]")
+
+    app = create_app(env_file=env_file)
+    assert app.config["MAIL_PASSWORD"] == "password"
+    assert app.config["MAIL_DEFAULT_SENDER"] == "[email protected]"