Merge pull request #8522 from OpenMined/hotfix/rename_email_credentials

Replace email_token -> username/password credentials

packages/grid/backend/grid/core/config.py

@@ -71,8 +71,6 @@ def sentry_dsn_can_be_blank(cls, v: str) -> Optional[str]:
     SMTP_TLS: bool = True
     SMTP_PORT: Optional[int] = None
     SMTP_HOST: Optional[str] = None
-    SMTP_USER: Optional[str] = None
-    SMTP_PASSWORD: Optional[str] = None
     EMAILS_FROM_EMAIL: Optional[EmailStr] = None
     EMAILS_FROM_NAME: Optional[str] = None
 
@@ -144,7 +142,8 @@ def get_emails_enabled(cls, v: bool, values: Dict[str, Any]) -> bool:
     SINGLE_CONTAINER_MODE: bool = str_to_bool(os.getenv("SINGLE_CONTAINER_MODE", False))
     CONSUMER_SERVICE_NAME: Optional[str] = os.getenv("CONSUMER_SERVICE_NAME")
     INMEMORY_WORKERS: bool = str_to_bool(os.getenv("INMEMORY_WORKERS", True))
-    EMAIL_TOKEN: str = os.getenv("EMAIL_TOKEN", "")
+    SMTP_USERNAME: str = os.getenv("SMTP_USERNAME", "")
+    SMTP_PASSWORD: str = os.getenv("SMTP_PASSWORD", "")
 
     TEST_MODE: bool = (
         True if os.getenv("TEST_MODE", "false").lower() == "true" else False

packages/grid/backend/grid/core/node.py

@@ -94,5 +94,6 @@ def seaweedfs_config() -> SeaweedFSConfig:
     queue_config=queue_config,
     migrate=True,
     in_memory_workers=settings.INMEMORY_WORKERS,
-    email_token=settings.EMAIL_TOKEN,
+    smtp_username=settings.SMTP_USERNAME,
+    smtp_password=settings.SMTP_PASSWORD,
 )

packages/hagrid/hagrid/cli.py

@@ -323,11 +323,18 @@ def clean(location: str) -> None:
     help="Container image tag to use",
 )
 @click.option(
-    "--email-token",
+    "--smtp-username",
     default=None,
     required=False,
     type=str,
-    help="Token used to auth in email server and enable notification via emails",
+    help="Username used to auth in email server and enable notification via emails",
+)
+@click.option(
+    "--smtp-password",
+    default=None,
+    required=False,
+    type=str,
+    help="Password used to auth in email server and enable notification via emails",
 )
 @click.option(
     "--build-src",
@@ -1316,7 +1323,8 @@ def create_launch_cmd(
     else:
         parsed_kwargs["node_side_type"] = NodeSideType.HIGH_SIDE.value
 
-    parsed_kwargs["email_token"] = kwargs["email_token"]
+    parsed_kwargs["smtp_username"] = kwargs["smtp_username"]
+    parsed_kwargs["smtp_password"] = kwargs["smtp_password"]
 
     parsed_kwargs["enable_warnings"] = not kwargs["no_warnings"]
 
@@ -2165,7 +2173,8 @@ def create_launch_docker_cmd(
 
     single_container_mode = kwargs["deployment_type"] == "single_container"
     in_mem_workers = kwargs.get("in_mem_workers")
-    email_token = kwargs.get("email_token")
+    smtp_username = kwargs.get("smtp_username")
+    smtp_password = kwargs.get("smtp_password")
 
     enable_oblv = bool(kwargs["oblv"])
     print("  - NAME: " + str(snake_name))
@@ -2230,7 +2239,8 @@ def create_launch_docker_cmd(
         "NODE_SIDE_TYPE": kwargs["node_side_type"],
         "SINGLE_CONTAINER_MODE": single_container_mode,
         "INMEMORY_WORKERS": in_mem_workers,
-        "EMAIL_TOKEN": email_token,
+        "SMTP_USERNAME": smtp_username,
+        "SMTP_PASSWORD": smtp_password,
     }
 
     if "trace" in kwargs and kwargs["trace"] is True:

packages/syft/src/syft/client/client.py

@@ -684,6 +684,12 @@ def notifications(self) -> Optional[APIModule]:
             return self.api.services.notifications
         return None
 
+    @property
+    def notifier(self) -> Optional[APIModule]:
+        if self.api.has_service("notifier"):
+            return self.api.services.notifier
+        return None
+
     @property
     def peers(self) -> Optional[Union[List[NodePeer], SyftError]]:
         if self.api.has_service("network"):

packages/syft/src/syft/node/node.py

@@ -310,6 +310,8 @@ def __init__(
         dev_mode: bool = False,
         migrate: bool = False,
         in_memory_workers: bool = True,
+        smtp_username: Optional[str] = None,
+        smtp_password: Optional[str] = None,
         email_token: Optional[str] = None,
     ):
         # 🟡 TODO 22: change our ENV variable format and default init args to make this
@@ -397,18 +399,9 @@ def __init__(
             node=self,
         )
 
-        if not email_token:
-            NotifierService.init_notifier(
-                node=self,
-                active=False,
-                email_token=None,
-            )
-        else:
-            NotifierService.init_notifier(
-                node=self,
-                active=True,
-                email_token=email_token,
-            )
+        NotifierService.init_notifier(
+            node=self, email_password=smtp_password, email_username=smtp_username
+        )
 
         self.client_cache = {}
         if isinstance(node_type, str):

packages/syft/src/syft/service/notifier/notifier.py

@@ -24,49 +24,57 @@
 from .notifier_enums import NOTIFIERS
 from .smtp_client import SMTPClient
 
+DEFAULT_EMAIL_SERVER = "smtp.postmarkapp.com"
+
 
 class BaseNotifier:
+    EMAIL_SERVER = DEFAULT_EMAIL_SERVER
+
     def send(
         self, target: SyftVerifyKey, notification: Notification
     ) -> Union[SyftSuccess, SyftError]:
-        pass
+        return SyftError(message="Not implemented")
 
 
 class EmailNotifier(BaseNotifier):
+    smtp_client = SMTPClient
+    username: str
+    password: str
     server: str
-    token: Optional[str]
-    smtp_client: SMTPClient
+    port: int
 
     def __init__(
         self,
-        token: Optional[str],
-        server: str = "smtp.postmarkapp.com",
+        username: str,
+        password: str,
+        server: str = DEFAULT_EMAIL_SERVER,
+        port: int = 587,
     ) -> None:
-        self.token = token
+        self.username = username
+        self.password = password
         self.server = server
+        self.port = port
         self.smtp_client = SMTPClient(
-            smtp_server=self.server, smtp_port=587, access_token=self.token
+            server=self.server,
+            port=self.port,
+            username=self.username,
+            password=self.password,
         )
 
-    @staticmethod
+    @classmethod
     def check_credentials(
-        server: str,
-        port: int,
-        token: Optional[str] = None,
-        username: Optional[str] = None,
-        password: Optional[str] = None,
-    ) -> bool:
-        if token:
-            return SMTPClient(
-                smtp_server=server, smtp_port=port, access_token=token
-            ).check_credentials()
-        else:
-            return SMTPClient(
-                smtp_server=server,
-                smtp_port=port,
-                username=username,
-                password=password,
-            ).check_credentials()
+        cls,
+        username: str,
+        password: str,
+        server: str = DEFAULT_EMAIL_SERVER,
+        port: int = 587,
+    ) -> Result[Ok, Err]:
+        return cls.smtp_client.check_credentials(
+            server=server,
+            port=port,
+            username=username,
+            password=password,
+        )
 
     def send(self, node: AbstractNode, notification: Notification) -> Result[Ok, Err]:
         try:
@@ -88,8 +96,10 @@ def send(self, node: AbstractNode, notification: Notification) -> Result[Ok, Err
                 sender=sender_email, receiver=receiver_email, subject=subject, body=body
             )
             return Ok("Email sent successfully!")
-        except Exception as e:
-            return Err(f"Error: unable to send email: {e}")
+        except Exception:
+            return Err(
+                "Some notifications failed to be delivered. Please check the health of the mailing server."
+            )
 
 
 @serializable()
@@ -99,12 +109,9 @@ class NotifierSettings(SyftObject):
     __repr_attrs__ = [
         "active",
         "email_enabled",
-        "sms_enabled",
-        "slack_enabled",
-        "app_enabled",
     ]
     active: bool = False
-    # Flag to identify which notification is enable
+    # Flag to identify which notification is enabled
     # For now, consider only the email notification
     # In future, Admin, must be able to have a better
     # control on diff notifications.
@@ -120,7 +127,11 @@ class NotifierSettings(SyftObject):
         NOTIFIERS.APP: False,
     }
 
-    email_token: Optional[str] = ""
+    email_server: Optional[str] = DEFAULT_EMAIL_SERVER
+    email_port: Optional[int] = 587
+    email_username: Optional[str] = ""
+    email_password: Optional[str] = ""
+    email_subscribers = set()
 
     @property
     def email_enabled(self) -> bool:
@@ -138,9 +149,18 @@ def slack_enabled(self) -> bool:
     def app_enabled(self) -> bool:
         return self.notifiers_status[NOTIFIERS.APP]
 
-    def valid_email_credentials(self, token: str) -> bool:
+    def validate_email_credentials(
+        self,
+        username: str,
+        password: str,
+        server: Optional[str] = None,
+        port: Optional[int] = None,
+    ) -> Result[Ok, Err]:
         return self.notifiers[NOTIFIERS.EMAIL].check_credentials(
-            server="smtp.postmarkapp.com", port=587, token=token
+            server=server if server else self.email_server,
+            port=port if port else self.email_port,
+            username=username,
+            password=password,
         )
 
     def send_notifications(
@@ -158,13 +178,13 @@ def send_notifications(
         return Ok("Notification sent successfully!")
 
     def select_notifiers(self, notification: Notification) -> List[BaseNotifier]:
-        """This method allow us to check which notification is enabled and return the
-        notifier object to be used to send the notification.
+        """
+        Return a list of the notifiers enabled for the given notification"
 
         Args:
             notification (Notification): The notification object
         Returns:
-            List[BaseNotifier]: A list of notifier objects
+            List[BaseNotifier]: A list of enabled notifier objects
         """
         notifier_objs = []
         for notifier_type in notification.notifier_types:
@@ -176,7 +196,9 @@ def select_notifiers(self, notification: Notification) -> List[BaseNotifier]:
                 # If notifier is email, we need to pass the token
                 if notifier_type == NOTIFIERS.EMAIL:
                     notifier_objs.append(
-                        self.notifiers[notifier_type](token=self.email_token)
+                        self.notifiers[notifier_type](
+                            username=self.email_username, password=self.email_password
+                        )
                     )
                 # If notifier is not email, we just create the notifier object
                 # TODO: Add the other notifiers, and its auth methods