From b3c32754cd71856082de8a0270821be2908fca2d Mon Sep 17 00:00:00 2001
From: Carina Antunes <carina.oliveira.antunes@cern.ch>
Date: Fri, 11 Feb 2022 16:06:07 +0000
Subject: [PATCH] audit: improvements

---
 notifications_consumer/auditing.py            |  8 ++--
 .../processors/email/processor.py             |  5 ++-
 .../processors/email_feed/processor.py        |  5 ++-
 .../processors/mattermost/processor.py        |  9 ++--
 .../processors/mattermost/utils.py            | 43 +++++++++++--------
 .../processors/safaripush/processor.py        |  8 ++--
 .../processors/safaripush/utils.py            |  8 ++--
 .../processors/webpush/processor.py           | 10 +++--
 .../processors/webpush/utils.py               | 10 ++---
 scripts/activemq_messages/safaripush.json     |  3 +-
 scripts/docker-send-mattermost.py             |  3 +-
 scripts/docker-send-webpush.py                |  3 +-
 12 files changed, 65 insertions(+), 50 deletions(-)

diff --git a/notifications_consumer/auditing.py b/notifications_consumer/auditing.py
index bfe756b..5934114 100644
--- a/notifications_consumer/auditing.py
+++ b/notifications_consumer/auditing.py
@@ -26,8 +26,8 @@ def audit_notification(notification_id, value, key=None, user_id=None):
     def put():
         client.put(
             (
-                f"/notifications/{notification_id}/{Config.AUDIT_ID}"
-                f"/{'target_users/' + user_id + '/' if user_id else ''}{key}"
+                f"/notifications/{notification_id}/{Config.AUDIT_ID}/{Config.PROCESSOR}"
+                f"/{'targets/' + user_id + '/' if user_id else ''}{key}"
             ),
             json.dumps({"date": datetime.now().strftime("%d/%m/%Y %H:%M:%S"), **value}),
         )
@@ -53,8 +53,8 @@ def get_audit_notification(notification_id, key, user_id=None):
     def get():
         kvs = client.range(
             (
-                f"/notifications/{notification_id}/{Config.AUDIT_ID}"
-                f"/{'target_users/' + user_id + '/' if user_id else ''}{key}"
+                f"/notifications/{notification_id}/{Config.AUDIT_ID}/{Config.PROCESSOR}"
+                f"/{'targets/' + user_id + '/' if user_id else ''}{key}"
             )
         ).kvs
         if kvs:
diff --git a/notifications_consumer/processors/email/processor.py b/notifications_consumer/processors/email/processor.py
index 664d7b3..b78c8d3 100644
--- a/notifications_consumer/processors/email/processor.py
+++ b/notifications_consumer/processors/email/processor.py
@@ -40,6 +40,7 @@ class EmailProcessor(Processor):
                     recipient_email,
                     Config.EMAIL_WHITELIST_GROUP_ID,
                 )
+                audit_notification(kwargs["notification_id"], {"event": "Skipped, not in whitelist"}, recipient_email)
                 return
 
         created_at = kwargs.get("created_at", "")
@@ -77,14 +78,14 @@ class EmailProcessor(Processor):
             context,
         )
 
-        if get_audit_notification(kwargs["notification_id"], self.__id, recipient_email):
+        if get_audit_notification(kwargs["notification_id"], recipient_email):
             logging.warning(
                 "%s is already sent to %s according ot etcd, skipping", kwargs["notification_id"], recipient_email
             )
             return
 
         ret = send_emails([email])
-        audit_notification(kwargs["notification_id"], {"event": "Sent"}, self.__id, recipient_email)
+        audit_notification(kwargs["notification_id"], {"event": "Sent"}, recipient_email)
         return ret
 
     def read_message(self, message: Dict):
diff --git a/notifications_consumer/processors/email_feed/processor.py b/notifications_consumer/processors/email_feed/processor.py
index 47060f8..1aaaa1e 100644
--- a/notifications_consumer/processors/email_feed/processor.py
+++ b/notifications_consumer/processors/email_feed/processor.py
@@ -47,6 +47,7 @@ class EmailFeedProcessor(Processor):
                     recipient_email,
                     Config.EMAIL_WHITELIST_GROUP_ID,
                 )
+                audit_notification(kwargs["notification_id"], {"event": "Skipped, not in whitelist"}, recipient_email)
                 return
 
         title = f'{Config.FEED_TITLE} Summary - {datetime.now().strftime("%d %B %Y")}'
@@ -75,14 +76,14 @@ class EmailFeedProcessor(Processor):
             context,
         )
 
-        if get_audit_notification(kwargs["notification_id"], self.__id, recipient_email):
+        if get_audit_notification(kwargs["notification_id"], recipient_email):
             logging.warning(
                 "%s is already sent to %s according ot etcd, skipping", kwargs["notification_id"], recipient_email
             )
             return
 
         ret = send_emails([email])
-        audit_notification(kwargs["notification_id"], {"event": "Sent"}, self.__id, recipient_email)
+        audit_notification(kwargs["notification_id"], {"event": "Sent"}, recipient_email)
         return ret
 
     def read_message(self, message: Dict):
diff --git a/notifications_consumer/processors/mattermost/processor.py b/notifications_consumer/processors/mattermost/processor.py
index ad6b989..2a0e426 100644
--- a/notifications_consumer/processors/mattermost/processor.py
+++ b/notifications_consumer/processors/mattermost/processor.py
@@ -27,8 +27,9 @@ class MattermostProcessor(Processor):
         """Process the message and send a mattermost notification."""
         logging.debug("%s - status:running - kwargs:%r", self, kwargs)
 
-        device_token = kwargs["devicetoken"]
-
+        device_token = kwargs["device_token"]
+        email = kwargs["email"]
+        device_id = kwargs["device_id"]
         subject = f'[{kwargs["channel_name"]}] - {kwargs["summary"]}'
         wpmessage = create_message(
             subject,
@@ -37,14 +38,14 @@ class MattermostProcessor(Processor):
             kwargs["img_url"],
         )
 
-        if get_audit_notification(kwargs["notification_id"], self.__id, device_token):
+        if get_audit_notification(kwargs["notification_id"], device_id, email):
             logging.warning(
                 "%s is already sent to %s according ot etcd, skipping", kwargs["notification_id"], device_token
             )
             return
 
         ret = send_message(wpmessage, device_token)
-        audit_notification(kwargs["notification_id"], {"event": "Sent"}, self.__id, device_token)
+        audit_notification(kwargs["notification_id"], {"event": "Sent", "device_token": device_token}, device_id, email)
         return ret
 
     def read_message(self, message: Dict):
diff --git a/notifications_consumer/processors/mattermost/utils.py b/notifications_consumer/processors/mattermost/utils.py
index 5060e6e..4396626 100644
--- a/notifications_consumer/processors/mattermost/utils.py
+++ b/notifications_consumer/processors/mattermost/utils.py
@@ -4,9 +4,10 @@ import logging
 import re
 import unicodedata
 
-from notifications_consumer.config import Config
 from mattermostdriver import Driver
 
+from notifications_consumer.config import Config
+
 
 def create_message(summary: str, body: str, url: str, image: str):
     """Create json blob to be sent via Mattermost."""
@@ -25,24 +26,28 @@ def create_message(summary: str, body: str, url: str, image: str):
     return blob
 
 
-def send_message(msg: str, devicetoken: str):
+def send_message(msg: str, device_token: str):
     """Send the message blob."""
-    logging.debug("mattermost send_message %s to %s", msg, devicetoken)
-    mmApi = Driver({
-        'url': Config.MATTERMOST_SERVER,
-        'token': Config.MATTERMOST_TOKEN,
-        'scheme': 'https',
-        'port': 443,
-        'basepath': '/api/v4',
-    })
+    logging.debug("mattermost send_message %s to %s", msg, device_token)
+    mmApi = Driver(
+        {
+            "url": Config.MATTERMOST_SERVER,
+            "token": Config.MATTERMOST_TOKEN,
+            "scheme": "https",
+            "port": 443,
+            "basepath": "/api/v4",
+        }
+    )
     mmApi.login()
-    notificationsUser = mmApi.users.get_user(user_id='me')
-    targetUser = mmApi.users.get_user_by_email(devicetoken)
-    if not (notificationsUser['id'] and targetUser['id']):
-        logging.error("Mattermost error unkwown user %s", devicetoken)
+    notificationsUser = mmApi.users.get_user(user_id="me")
+    targetUser = mmApi.users.get_user_by_email(device_token)
+    if not (notificationsUser["id"] and targetUser["id"]):
+        logging.error("Mattermost error unkwown user %s", device_token)
         return
-    dmObject = mmApi.channels.create_direct_message_channel(options=[notificationsUser['id'], targetUser['id']])
-    mmApi.posts.create_post(options={
-        'channel_id': dmObject['id'],
-        'message': msg,
-    })
+    dmObject = mmApi.channels.create_direct_message_channel(options=[notificationsUser["id"], targetUser["id"]])
+    mmApi.posts.create_post(
+        options={
+            "channel_id": dmObject["id"],
+            "message": msg,
+        }
+    )
diff --git a/notifications_consumer/processors/safaripush/processor.py b/notifications_consumer/processors/safaripush/processor.py
index d0495f4..77f2b5d 100644
--- a/notifications_consumer/processors/safaripush/processor.py
+++ b/notifications_consumer/processors/safaripush/processor.py
@@ -27,7 +27,9 @@ class SafariPushProcessor(Processor):
         """Process the message and send a push notification."""
         logging.debug("%s - status:running - kwargs:%r", self, kwargs)
 
-        device_token = kwargs["devicetoken"]
+        device_token = kwargs["device_token"]
+        email = kwargs["email"]
+        device_id = kwargs["device_id"]
 
         subject = f'[{kwargs["channel_name"]}] - {kwargs["summary"]}'
         spmessage = create_message(
@@ -37,14 +39,14 @@ class SafariPushProcessor(Processor):
             kwargs["img_url"],
         )
 
-        if get_audit_notification(kwargs["notification_id"], self.__id, device_token):
+        if get_audit_notification(kwargs["notification_id"], device_id, email):
             logging.warning(
                 "%s is already sent to %s according ot etcd, skipping", kwargs["notification_id"], device_token
             )
             return
 
         ret = send_message(spmessage, device_token)
-        audit_notification(kwargs["notification_id"], {"event": "Sent"}, self.__id, device_token)
+        audit_notification(kwargs["notification_id"], {"event": "Sent", "device_token": device_token}, device_id, email)
         return ret
 
     def read_message(self, message: Dict):
diff --git a/notifications_consumer/processors/safaripush/utils.py b/notifications_consumer/processors/safaripush/utils.py
index c70c32f..658d6ad 100644
--- a/notifications_consumer/processors/safaripush/utils.py
+++ b/notifications_consumer/processors/safaripush/utils.py
@@ -39,14 +39,14 @@ def create_message(summary: str, body: str, url: str, image: str):
     return spmessage
 
 
-def send_message(payload: Payload, devicetoken: str):
+def send_message(payload: Payload, device_token: str):
     """Send the message Payload.
 
     :param payload: apns2 Payload object
-    :param devicetoken: Device token
+    :param device_token: Device token
     """
-    logging.debug("safaripush send_message to %s", devicetoken)
+    logging.debug("safaripush send_message to %s", device_token)
     topic = Config.APPLE_SAFARI_PUSH_WEBSITEPUSHID
     client = APNsClient(Config.APPLE_SAFARI_PUSH_CERT_KEY_FILE_PATH, use_sandbox=False, use_alternative_port=False)
-    client.send_notification(devicetoken, payload, topic)
+    client.send_notification(device_token, payload, topic)
     logging.debug("Safari message sent")
diff --git a/notifications_consumer/processors/webpush/processor.py b/notifications_consumer/processors/webpush/processor.py
index 5065637..a194449 100644
--- a/notifications_consumer/processors/webpush/processor.py
+++ b/notifications_consumer/processors/webpush/processor.py
@@ -27,8 +27,10 @@ class WebPushProcessor(Processor):
         """Process the message and send a push notification."""
         logging.debug("%s - status:running - kwargs:%r", self, kwargs)
 
-        device_token = kwargs["devicetoken"]
+        device_token = kwargs["device_token"]
         encoding = kwargs.get("encoding", None)
+        email = kwargs["email"]
+        device_id = kwargs["device_id"]
 
         subject = f'[{kwargs["channel_name"]}] - {kwargs["summary"]}'
         wpmessage = create_message(
@@ -38,14 +40,14 @@ class WebPushProcessor(Processor):
             kwargs["img_url"],
         )
 
-        if get_audit_notification(kwargs["notification_id"], self.__id, device_token):
+        if get_audit_notification(kwargs["notification_id"], device_id, email):
             logging.warning(
-                "%s is already sent to %s according ot etcd, skipping", kwargs["notification_id"], device_token
+                "%s is already sent to %s according ot etcd, skipping", kwargs["notification_id"], device_id
             )
             return
 
         ret = send_message(wpmessage, device_token, encoding)
-        audit_notification(kwargs["notification_id"], {"event": "Sent"}, self.__id, device_token)
+        audit_notification(kwargs["notification_id"], {"event": "Sent", "device_token": device_token}, device_id, email)
         return ret
 
     def read_message(self, message: Dict):
diff --git a/notifications_consumer/processors/webpush/utils.py b/notifications_consumer/processors/webpush/utils.py
index 0ae03e9..8021228 100644
--- a/notifications_consumer/processors/webpush/utils.py
+++ b/notifications_consumer/processors/webpush/utils.py
@@ -27,13 +27,13 @@ def create_message(summary: str, body: str, url: str, image: str):
     return wpmessage
 
 
-def send_message(msg: str, devicetoken: str, encoding: str = None):
+def send_message(msg: str, device_token: str, encoding: str = None):
     """Send the message blob."""
-    logging.debug("webpush send_message (custom encoding=%s) %s to %s", encoding, msg, devicetoken)
+    logging.debug("webpush send_message (custom encoding=%s) %s to %s", encoding, msg, device_token)
     try:
         if encoding == "aesgcm":
             webpush(
-                json.loads(devicetoken),
+                json.loads(device_token),
                 json.dumps(msg),
                 content_encoding=encoding,
                 vapid_private_key=Config.VAPID_PRIVATEKEY,
@@ -42,7 +42,7 @@ def send_message(msg: str, devicetoken: str, encoding: str = None):
             return
 
         webpush(
-            json.loads(devicetoken),
+            json.loads(device_token),
             json.dumps(msg),
             vapid_private_key=Config.VAPID_PRIVATEKEY,
             vapid_claims={"sub": f"mailto:{Config.VAPID_EMAIL}"},
@@ -54,6 +54,6 @@ def send_message(msg: str, devicetoken: str, encoding: str = None):
             raise
         # if 410 error the device should be removed form Device table, so logging
         if "410" in ex.message:
-            logging.warning("Webpush CLEANUP needed for device %s", devicetoken)
+            logging.warning("Webpush CLEANUP needed for device %s", device_token)
         else:
             logging.error("Webpush error %s", ex.message)
diff --git a/scripts/activemq_messages/safaripush.json b/scripts/activemq_messages/safaripush.json
index ebf5e56..8a7c527 100644
--- a/scripts/activemq_messages/safaripush.json
+++ b/scripts/activemq_messages/safaripush.json
@@ -4,5 +4,6 @@
     "summary": "sub test EO",
     "link": "http://cds.cern.ch/record/2687667",
     "img_url": "http://cds.cern.ch/record/2687667/files/CLICtd.png?subformat=icon-640",
-    "devicetoken": "FD276F2A142090371ECEAAA368FB99C675577194DAA3F2968DBEB3F10FA4D60F"
+    "device_token": "FD276F2A142090371ECEAAA368FB99C675577194DAA3F2968DBEB3F10FA4D60F",
+    "email": "emmanuel.ormancey@cern.ch",
 }
diff --git a/scripts/docker-send-mattermost.py b/scripts/docker-send-mattermost.py
index 53640fc..5bff4c8 100644
--- a/scripts/docker-send-mattermost.py
+++ b/scripts/docker-send-mattermost.py
@@ -10,7 +10,8 @@ message_body = r"""{
 "summary": "sub test EO",
 "link": "http://cds.cern.ch/record/2687667",
 "img_url": "http://cds.cern.ch/record/2687667/files/CLICtd.png?subformat=icon-640",
-"devicetoken": "emmanuel.ormancey@cern.ch"
+"device_token": "emmanuel.ormancey@cern.ch",
+"email": "emmanuel.ormancey@cern.ch"
  }"""
 conn.send(body=message_body, destination="/queue/np.mattermost", headers={"persistent": "true"})
 conn.disconnect()
diff --git a/scripts/docker-send-webpush.py b/scripts/docker-send-webpush.py
index af40b22..0f3c296 100644
--- a/scripts/docker-send-webpush.py
+++ b/scripts/docker-send-webpush.py
@@ -10,7 +10,8 @@ message_body = r"""{
 "summary": "sub test EO",
 "link": "http://cds.cern.ch/record/2687667",
 "img_url": "http://cds.cern.ch/record/2687667/files/CLICtd.png?subformat=icon-640",
-"devicetoken": ""
+"device_token": "",
+"email": "emmanuel.ormancey@cern.ch"
  }"""
 conn.send(body=message_body, destination="/queue/np.webpush", headers={"persistent": "true"})
 conn.disconnect()
-- 
GitLab