Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add email notification setting to User #8525

Merged
merged 10 commits into from
Feb 27, 2024
13 changes: 9 additions & 4 deletions packages/syft/src/syft/service/notifier/notifier.py
Expand Up @@ -77,9 +77,14 @@ def send(
try:
user_service = context.node.get_service("userservice")

receiver_email = user_service.get_by_verify_key(
notification.to_user_verify_key
).email
receiver = user_service.get_by_verify_key(notification.to_user_verify_key)

if not receiver.notifications_enabled[NOTIFIERS.EMAIL]:
return Ok(
"Email notifications are disabled for this user."
) # TODO: Should we return an error here?

receiver_email = receiver.email

subject = notification.email_template.email_title(
notification, context=context
Expand Down Expand Up @@ -191,7 +196,7 @@ def select_notifiers(self, notification: Notification) -> List[BaseNotifier]:
self.notifiers_status[notifier_type]
and self.notifiers[notifier_type] is not None
):
# If notifier is email, we need to pass the token
# If notifier is email, we need to pass the parameters
if notifier_type == NOTIFIERS.EMAIL:
notifier_objs.append(
self.notifiers[notifier_type](
Expand Down
66 changes: 17 additions & 49 deletions packages/syft/src/syft/service/notifier/notifier_service.py
Expand Up @@ -5,6 +5,7 @@
from typing import Union

# third party
from pydantic import EmailStr
from result import Err
from result import Ok
from result import Result
Expand Down Expand Up @@ -109,11 +110,6 @@ def turn_on(
username=email_username, password=email_password
)

if not email_sender and not notifier.email_sender:
return SyftError(
message="You must provide a sender email address to enable notifications."
)

if validation_result.is_err():
return SyftError(
message="Invalid SMTP credentials. Please check your username and password."
Expand All @@ -122,7 +118,19 @@ def turn_on(
notifier.email_password = email_password
notifier.email_username = email_username

# Email sender verification
if not email_sender and not notifier.email_sender:
return SyftError(
message="You must provide a sender email address to enable notifications."
)

if email_sender:
try:
EmailStr.validate(email_sender)
except ValueError:
return SyftError(
message="Invalid sender email address. Please check your email address."
)
notifier.email_sender = email_sender

notifier.active = True
Expand Down Expand Up @@ -170,34 +178,8 @@ def activate(
Activate email notifications for the authenticated user.
This will only work if the domain owner has enabled notifications.
"""

# TODO: user credentials should not be used to get the notifier settings
# TODO: WARNING THIS IS A POTENTIAL SECURITY RISK (ONLY FOR DEVELOPMENT PURPOSES)

admin_key = self.stash.admin_verify_key()

result = self.stash.get(credentials=admin_key)
print(result)

if result.is_err():
return SyftError(message=result.err())

notifier = result.ok()

user_key = context.credentials

if user_key in notifier.email_subscribers:
return SyftSuccess(
message="Notifications are already activated for this user."
)

notifier.email_subscribers.add(user_key)

# TODO: user credentials should not be used to update the notifier settings
result = self.stash.update(credentials=admin_key, settings=notifier)
if result.is_err():
return SyftError(message=result.err())
return SyftSuccess(message="Notifications enabled successfully.")
user_service = context.node.get_service("userservice")
return user_service.enable_notifications(context)

@service_method(
path="notifier.deactivate",
Expand All @@ -211,22 +193,8 @@ def deactivate(
"""Deactivate email notifications for the authenticated user
This will only work if the domain owner has enabled notifications.
"""

# TODO: WARNING THIS IS A POTENTIAL SECURITY RISK (ONLY FOR DEVELOPMENT PURPOSES)
admin_key = self.stash.admin_verify_key()
result = self.stash.get(credentials=admin_key)

if result.is_err():
return SyftError(message=result.err())

notifier = result.ok()
user_key = context.credentials
notifier.email_subscribers.remove(user_key)

result = self.stash.update(credentials=admin_key, settings=notifier)
if result.is_err():
return SyftError(message=result.err())
return SyftSuccess(message="Notifications disabled successfully.")
user_service = context.node.get_service("userservice")
return user_service.disable_notifications(context)

@staticmethod
def init_notifier(
Expand Down
13 changes: 7 additions & 6 deletions packages/syft/src/syft/service/notifier/notifier_stash.py
Expand Up @@ -29,24 +29,25 @@ class NotifierStash(BaseStash):
name=NotifierSettings.__canonical_name__, object_type=NotifierSettings
)

# Usual Stash Implementation
# The only diff is: We need to be sure that
# this will act as a Singleton (as SettingsStash).

def __init__(self, store: DocumentStore) -> None:
super().__init__(store=store)

def admin_verify_key(self) -> SyftVerifyKey:
return self.partition.root_verify_key

# TODO: should this method behave like a singleton?
def get(self, credentials: SyftVerifyKey) -> Result[NotifierSettings, Err]:
"""Get Settings"""
result = self.get_all(credentials)
if result.is_ok():
settings = result.ok()
if len(settings) == 0:
return Ok(None)
result = settings[0]
return Ok(
None
) # TODO: Stash shouldn't be empty after init. Return Err instead?
result = settings[
0
] # TODO: Should we check if theres more than one? => Report corruption
return Ok(result)
else:
return Err(message=result.err())
Expand Down