Merge pull request #69 from caarmen/handle-webhook-notif-unknown-user

Handle webhook notifications for unknown users

slackhealthbot/core/exceptions.py

@@ -2,3 +2,9 @@ class UserLoggedOutException(Exception):
     """
     Raised when we are unable to refresh an oauth access token
     """
+
+
+class UnknownUserException(Exception):
+    """
+    Raised when we fail to find a user.
+    """

slackhealthbot/data/repositories/sqlalchemyfitbitrepository.py

@@ -3,6 +3,7 @@
 from sqlalchemy import and_, desc, func, select, update
 from sqlalchemy.ext.asyncio import AsyncSession
 
+from slackhealthbot.core.exceptions import UnknownUserException
 from slackhealthbot.core.models import OAuthFields
 from slackhealthbot.data.database import models
 from slackhealthbot.domain.localrepository.localfitbitrepository import (
@@ -130,7 +131,9 @@ async def get_user_by_fitbit_userid(
                 .join(models.User.fitbit)
                 .where(models.FitbitUser.oauth_userid == fitbit_userid)
             )
-        ).one()
+        ).one_or_none()
+        if not user:
+            raise UnknownUserException
         return User(
             identity=UserIdentity(
                 fitbit_userid=user.fitbit.oauth_userid,
@@ -236,7 +239,9 @@ async def get_sleep_by_fitbit_userid(
                     models.FitbitUser.oauth_userid == fitbit_userid
                 )
             )
-        ).one()
+        ).one_or_none()
+        if not fitbit_user:
+            raise UnknownUserException
         if not fitbit_user or not fitbit_user.last_sleep_end_time:
             return None
         return SleepData(

slackhealthbot/data/repositories/sqlalchemywithingsrepository.py

@@ -3,6 +3,7 @@
 from sqlalchemy import select, update
 from sqlalchemy.ext.asyncio import AsyncSession
 
+from slackhealthbot.core.exceptions import UnknownUserException
 from slackhealthbot.core.models import OAuthFields
 from slackhealthbot.data.database import models
 from slackhealthbot.domain.localrepository.localwithingsrepository import (
@@ -126,7 +127,9 @@ async def get_user_by_withings_userid(
                 .join(models.User.withings)
                 .where(models.WithingsUser.oauth_userid == withings_userid)
             )
-        ).one()
+        ).one_or_none()
+        if not user:
+            raise UnknownUserException
         return User(
             identity=UserIdentity(
                 withings_userid=user.withings.oauth_userid,

slackhealthbot/routers/fitbit.py

@@ -4,7 +4,7 @@
 from fastapi import APIRouter, Depends, Request, Response, status
 from pydantic import BaseModel
 
-from slackhealthbot.core.exceptions import UserLoggedOutException
+from slackhealthbot.core.exceptions import UnknownUserException, UserLoggedOutException
 from slackhealthbot.domain.localrepository.localfitbitrepository import (
     LocalFitbitRepository,
 )
@@ -141,4 +141,7 @@ async def fitbit_notification_webhook(
                 fitbit_userid=notification.ownerId,
             )
             break
+        except UnknownUserException:
+            logging.info("fitbit_notification_webhook: unknown user")
+            return Response(status_code=status.HTTP_404_NOT_FOUND)
     return Response(status_code=status.HTTP_204_NO_CONTENT)

slackhealthbot/routers/withings.py

@@ -3,7 +3,7 @@
 from fastapi import APIRouter, Depends, Request, Response, status
 from pydantic import BaseModel
 
-from slackhealthbot.core.exceptions import UserLoggedOutException
+from slackhealthbot.core.exceptions import UnknownUserException, UserLoggedOutException
 from slackhealthbot.domain.localrepository.localwithingsrepository import (
     LocalWithingsRepository,
 )
@@ -124,6 +124,9 @@ async def withings_notification_webhook(
                 slack_repo=slack_repo,
                 withings_userid=notification.userid,
             )
+        except UnknownUserException:
+            logging.info("withings_notification_webhook: unknown user")
+            return Response(status_code=status.HTTP_404_NOT_FOUND)
     else:
         logging.info("Ignoring duplicate withings notification")
     return Response(status_code=status.HTTP_204_NO_CONTENT)

tests/routes/test_fitbit_routes.py

@@ -370,3 +370,38 @@ async def test_duplicate_sleep_notification(
     # Then we don't post to stack a second time
     assert sleep_request.call_count == 1
     assert slack_request.call_count == 1
+
+
+@pytest.mark.parametrize(
+    argnames=["collectionType"],
+    argvalues=[
+        ["activities"],
+        ["sleep"],
+    ],
+)
+def test_notification_unknown_user(
+    client: TestClient,
+    collectionType: str,
+):
+    """
+    Given some data in the db
+    When we receive a fitbit notification for an unknown user
+    Then the webhook returns the expected error.
+    """
+    # When we receive a fitbit notification for an unknown user
+    with client:
+        response = client.post(
+            "/fitbit-notification-webhook/",
+            content=json.dumps(
+                [
+                    {
+                        "ownerId": "UNKNOWNUSER",
+                        "date": "2023-05-12",
+                        "collectionType": collectionType,
+                    }
+                ]
+            ),
+        )
+
+    # Then the webhook returns the expected error.
+    assert response.status_code == status.HTTP_404_NOT_FOUND

tests/routes/test_withings_routes.py

@@ -223,3 +223,26 @@ async def test_duplicate_weight_notification(
     # Then we don't post to stack a second time
     assert weight_request.call_count == 1
     assert slack_request.call_count == 1
+
+
+def test_notification_unknown_user(
+    client: TestClient,
+):
+    """
+    Given some data in the db
+    When we receive a withings notification for an unknown user
+    Then the webhook returns the expected error.
+    """
+    # When we receive a withings notification for an unknown user
+    with client:
+        response = client.post(
+            "/withings-notification-webhook/",
+            data={
+                "userid": "UNKNOWNUSER",
+                "startdate": 1683894606,
+                "enddate": 1686570821,
+            },
+        )
+
+    # Then the webhook returns the expected error.
+    assert response.status_code == status.HTTP_404_NOT_FOUND