diff --git a/slackhealthbot/core/exceptions.py b/slackhealthbot/core/exceptions.py index 2398133c..78992f0d 100644 --- a/slackhealthbot/core/exceptions.py +++ b/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. + """ diff --git a/slackhealthbot/data/repositories/sqlalchemyfitbitrepository.py b/slackhealthbot/data/repositories/sqlalchemyfitbitrepository.py index 1e7e1c61..69c04fc8 100644 --- a/slackhealthbot/data/repositories/sqlalchemyfitbitrepository.py +++ b/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( diff --git a/slackhealthbot/data/repositories/sqlalchemywithingsrepository.py b/slackhealthbot/data/repositories/sqlalchemywithingsrepository.py index 42099835..cb4b7052 100644 --- a/slackhealthbot/data/repositories/sqlalchemywithingsrepository.py +++ b/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, diff --git a/slackhealthbot/routers/fitbit.py b/slackhealthbot/routers/fitbit.py index 3ac8bd49..7db38590 100644 --- a/slackhealthbot/routers/fitbit.py +++ b/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) diff --git a/slackhealthbot/routers/withings.py b/slackhealthbot/routers/withings.py index b985da7b..8ae65a15 100644 --- a/slackhealthbot/routers/withings.py +++ b/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) diff --git a/tests/routes/test_fitbit_routes.py b/tests/routes/test_fitbit_routes.py index c5b06621..8c19fa62 100644 --- a/tests/routes/test_fitbit_routes.py +++ b/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 diff --git a/tests/routes/test_withings_routes.py b/tests/routes/test_withings_routes.py index 1131d069..de7dca6b 100644 --- a/tests/routes/test_withings_routes.py +++ b/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