Adapt for SMTP without auth (GSI-949) (#17)

* Update config so credentials are optional

* Move server connection into cm function

* Bump version from 1.1.3 -> 1.2.0

* Allow login values to be None

.pyproject_generation/pyproject_custom.toml

@@ -1,6 +1,6 @@
 [project]
 name = "ns"
-version = "1.1.3"
+version = "1.2.0"
 description = "The Notification Service (NS) handles notification kafka events."
 dependencies = [
     "typer>=0.12",

README.md

@@ -33,21 +33,21 @@ We recommend using the provided Docker container.
 
 A pre-build version is available at [docker hub](https://hub.docker.com/repository/docker/ghga/notification-service):
 ```bash
-docker pull ghga/notification-service:1.1.3
+docker pull ghga/notification-service:1.2.0
 ```
 
 Or you can build the container yourself from the [`./Dockerfile`](./Dockerfile):
 ```bash
 # Execute in the repo's root dir:
-docker build -t ghga/notification-service:1.1.3 .
+docker build -t ghga/notification-service:1.2.0 .
 ```
 
 For production-ready deployment, we recommend using Kubernetes, however,
 for simple use cases, you could execute the service using docker
 on a single server:
 ```bash
 # The entrypoint is preconfigured:
-docker run -p 8080:8080 ghga/notification-service:1.1.3 --help
+docker run -p 8080:8080 ghga/notification-service:1.2.0 --help
 ```
 
 If you prefer not to use containers, you may install the service from source:
@@ -131,9 +131,21 @@ The service requires the following configuration parameters:
 
 - **`smtp_port`** *(integer)*: The port for the mail server connection.
 
-- **`login_user`** *(string)*: The login username or email.
+- **`login_user`**: The login username or email. Default: `null`.
 
-- **`login_password`** *(string, format: password)*: The login password.
+  - **Any of**
+
+    - *string*
+
+    - *null*
+
+- **`login_password`**: The login password. Default: `null`.
+
+  - **Any of**
+
+    - *string, format: password*
+
+    - *null*
 
 - **`use_starttls`** *(boolean)*: Boolean flag indicating the use of STARTTLS. Default: `true`.
 

config_schema.json

@@ -97,16 +97,32 @@
       "type": "integer"
     },
     "login_user": {
+      "anyOf": [
+        {
+          "type": "string"
+        },
+        {
+          "type": "null"
+        }
+      ],
+      "default": null,
       "description": "The login username or email",
-      "title": "Login User",
-      "type": "string"
+      "title": "Login User"
     },
     "login_password": {
+      "anyOf": [
+        {
+          "format": "password",
+          "type": "string",
+          "writeOnly": true
+        },
+        {
+          "type": "null"
+        }
+      ],
+      "default": null,
       "description": "The login password",
-      "format": "password",
-      "title": "Login Password",
-      "type": "string",
-      "writeOnly": true
+      "title": "Login Password"
     },
     "use_starttls": {
       "default": true,
@@ -199,8 +215,6 @@
     "from_address",
     "smtp_host",
     "smtp_port",
-    "login_user",
-    "login_password",
     "notification_event_topic",
     "notification_event_type",
     "kafka_servers"

pyproject.toml

@@ -21,7 +21,7 @@ classifiers = [
     "Intended Audience :: Developers",
 ]
 name = "ns"
-version = "1.1.3"
+version = "1.2.0"
 description = "The Notification Service (NS) handles notification kafka events."
 dependencies = [
     "typer>=0.12",

src/ns/adapters/outbound/smtp_client.py

@@ -17,11 +17,14 @@
 """Contains the smtp client adapter"""
 
 import logging
-import smtplib
 import ssl
+from collections.abc import Generator
+from contextlib import contextmanager
 from email.message import EmailMessage
+from smtplib import SMTP, SMTPAuthenticationError, SMTPException
+from typing import Self
 
-from pydantic import Field, SecretStr
+from pydantic import Field, SecretStr, model_validator
 from pydantic_settings import BaseSettings
 
 from ns.ports.outbound.smtp_client import SmtpClientPort
@@ -38,12 +41,28 @@ class SmtpClientConfig(BaseSettings):
     smtp_port: int = Field(
         default=..., description="The port for the mail server connection"
     )
-    login_user: str = Field(default=..., description="The login username or email")
-    login_password: SecretStr = Field(default=..., description="The login password")
+    login_user: str | None = Field(
+        default=None, description="The login username or email"
+    )
+    login_password: SecretStr | None = Field(
+        default=None, description="The login password"
+    )
     use_starttls: bool = Field(
         default=True, description="Boolean flag indicating the use of STARTTLS"
     )
 
+    @model_validator(mode="after")
+    def validate_prefix(self) -> Self:
+        """Enforce setting both the user/pwd or leaving both unconfigured (`None`)"""
+        msg = "Login_user and login_password must be configured together or not at all."
+        nones = [
+            _
+            for _ in filter(lambda x: x is None, [self.login_user, self.login_password])
+        ]
+        if nones and len(nones) == 1:
+            raise ValueError(msg)
+        return self
+
 
 class SmtpClient(SmtpClientPort):
     """Concrete implementation of an SmtpClientPort"""
@@ -52,27 +71,40 @@ def __init__(self, *, config: SmtpClientConfig):
         """Assign config, which should contain all needed info"""
         self._config = config
 
+    @contextmanager
+    def get_connection(self) -> Generator[SMTP, None, None]:
+        """Establish a connection to the SMTP server"""
+        with SMTP(self._config.smtp_host, self._config.smtp_port) as server:
+            yield server
+
     def send_email_message(self, message: EmailMessage):
         """Send an email message.
 
-        Creates an ssl security context if configured, then log in with the configured
-        credentials and send the provided email message.
+        Creates an ssl security context if configured, then logs in with the configured
+        credentials and sends the provided email message.
+        In the case that username and password are `None`, authentication will not be
+        performed.
         """
         try:
-            with smtplib.SMTP(self._config.smtp_host, self._config.smtp_port) as server:
+            with self.get_connection() as server:
                 if self._config.use_starttls:
                     # create ssl security context per Python's Security considerations
                     context = ssl.create_default_context()
                     server.starttls(context=context)
-                try:
-                    server.login(
-                        self._config.login_user,
-                        self._config.login_password.get_secret_value(),
-                    )
-                except smtplib.SMTPAuthenticationError as err:
-                    login_error = self.FailedLoginError()
-                    log.critical(login_error)
-                    raise login_error from err
+
+                if (
+                    self._config.login_user is not None
+                    and self._config.login_password is not None
+                ):
+                    try:
+                        server.login(
+                            self._config.login_user,
+                            self._config.login_password.get_secret_value(),
+                        )
+                    except SMTPAuthenticationError as err:
+                        login_error = self.FailedLoginError()
+                        log.critical(login_error)
+                        raise login_error from err
 
                 # check for a connection
                 if server.noop()[0] != 250:
@@ -81,7 +113,7 @@ def send_email_message(self, message: EmailMessage):
                     raise connection_error
 
                 server.send_message(msg=message)
-        except smtplib.SMTPException as exc:
+        except SMTPException as exc:
             error = self.GeneralSmtpException(error_info=exc.args[0])
             log.error(error, extra={"error_info": exc.args[0]})
             raise error from exc

tests/fixtures/server.py

@@ -29,7 +29,7 @@
 class Authenticator:
     """Basic authenticator so we can test error handling for failed authentication"""
 
-    def __init__(self, user: str, password: str):
+    def __init__(self, user: str | None, password: str | None):
         self._user = user
         self._password = password
 
@@ -38,10 +38,9 @@ def __call__(self, server, session, envelope, mechanism, auth_data):
         login = str(auth_data.login, encoding="utf-8")
         password = str(auth_data.password, encoding="utf-8")
 
-        if login == self._user and password == self._password:
-            return AuthResult(success=True)
+        authenticated = login == self._user and password == self._password
 
-        return AuthResult(success=False, handled=False)
+        return AuthResult(success=authenticated, handled=False)
 
 
 class CustomHandler(Sink):
@@ -111,7 +110,8 @@ def __init__(self, *, config: Config):
         """Assign config"""
         self._config = config
         self.login = self._config.login_user
-        self.password = self._config.login_password.get_secret_value()
+        password = self._config.login_password
+        self.password = password.get_secret_value() if password else None
 
     def _record_email(
         self, *, expected_email: EmailMessage, controller: Controller

tests/test_basic.py

@@ -17,13 +17,17 @@
 
 import html
 import json
+import smtplib
+from contextlib import contextmanager
+from email.message import EmailMessage
 from hashlib import sha256
 from typing import cast
+from unittest.mock import Mock
 
 import pytest
 from hexkit.correlation import correlation_id_var
 
-from ns.adapters.outbound.smtp_client import SmtpClient
+from ns.adapters.outbound.smtp_client import SmtpClient, SmtpClientConfig
 from ns.core.models import NotificationRecord
 from ns.core.notifier import Notifier
 from tests.fixtures.joint import JointFixture
@@ -54,10 +58,7 @@ def correlation_id_fixture():
     correlation_id_var.reset(token)
 
 
-@pytest.mark.parametrize(
-    "notification_details",
-    [sample_notification],
-)
+@pytest.mark.parametrize("notification_details", [sample_notification])
 async def test_email_construction(
     joint_fixture: JointFixture,
     notification_details,
@@ -97,8 +98,74 @@ async def test_email_construction(
     assert html_content.strip() == expected_html
 
 
+async def test_bad_login_config():
+    """Verify that we get an error if credentials are misconfigured"""
+    with pytest.raises(ValueError):
+        SmtpClientConfig(
+            smtp_host="127.0.0.1",
+            smtp_port=587,
+            login_user="test",
+            login_password=None,
+        )
+
+    with pytest.raises(ValueError):
+        SmtpClientConfig(
+            smtp_host="127.0.0.1",
+            smtp_port=587,
+            login_user=None,
+            login_password="test",  # type: ignore
+        )
+
+
+@pytest.mark.parametrize(
+    "username, password",
+    [
+        ("[email protected]", "passw0rd"),
+        ("[email protected]", ""),
+        ("", "passw0rd"),
+        (None, None),
+    ],
+    ids=["WithUsernameAndPassword", "BlankPassword", "BlankUsername", "NoAuth"],
+)
+async def test_smtp_authentication(username: str | None, password: str | None):
+    """Verify that authentication is only used when credentials are configured."""
+    config = SmtpClientConfig(
+        smtp_host="127.0.0.1",
+        smtp_port=587,
+        login_user=username,
+        login_password=password,  # type: ignore
+    )
+
+    mock_server = Mock(spec=smtplib.SMTP)
+    mock_server.noop.side_effect = lambda: (250, b"")
+
+    smtp_client = SmtpClient(config=config)
+
+    @contextmanager
+    def get_mock_server():
+        yield mock_server
+
+    smtp_client.get_connection = get_mock_server  # type: ignore [method-assign]
+
+    message = EmailMessage()
+    message["To"] = "[email protected]"
+    message["Subject"] = "Test"
+    message["From"] = "[email protected]"
+
+    smtp_client.send_email_message(message)
+
+    # Verify that 'login' is only called when credentials are set
+    if username is not None and password is not None:
+        mock_server.login.assert_called()
+    else:
+        mock_server.login.assert_not_called()
+
+    # Verify that 'send_message' is called regardless
+    mock_server.send_message.assert_called()
+
+
 async def test_failed_authentication(joint_fixture: JointFixture):
-    """Change login credentials so authentication fails."""
+    """Test that we raise the expected error when auth fails on a secured SMTP server."""
     # Cast notifier type
     joint_fixture.notifier = cast(Notifier, joint_fixture.notifier)