From 768119eeebe825bc323ffee5dcfeceb2130a5b52 Mon Sep 17 00:00:00 2001 From: Noah Paige <69586985+noah-paige@users.noreply.github.com> Date: Mon, 28 Oct 2024 11:32:51 -0400 Subject: [PATCH] Add permission checks to markNotificationAsRead + deleteNotification (#1654) ### Feature or Bugfix - Bugfix ### Detail - Add check to ensure user is a recipient of a Notification before allowing to mark as read or delete ### Relates ### Security Please answer the questions below briefly where applicable, or write `N/A`. Based on [OWASP 10](https://owasp.org/Top10/en/). - Does this PR introduce or modify any input fields or queries - this includes fetching data from storage outside the application (e.g. a database, an S3 bucket)? - Is the input sanitized? - What precautions are you taking before deserializing the data you consume? - Is injection prevented by parametrizing queries? - Have you ensured no `eval` or similar functions are used? - Does this PR introduce any functionality or component that requires authorization? - How have you ensured it respects the existing AuthN/AuthZ mechanisms? - Are you logging failed auth attempts? - Are you using or adding any cryptographic features? - Do you use a standard proven implementations? - Are the used keys controlled by the customer? Where are they stored? - Are you introducing any new policies/roles/users? - Have you used the least-privilege principle? How? By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. --- .../db/notification_repositories.py | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/backend/dataall/modules/notifications/db/notification_repositories.py b/backend/dataall/modules/notifications/db/notification_repositories.py index 34ff7d1c6..4dd3b159a 100644 --- a/backend/dataall/modules/notifications/db/notification_repositories.py +++ b/backend/dataall/modules/notifications/db/notification_repositories.py @@ -3,7 +3,31 @@ from sqlalchemy import func, and_, or_ from dataall.modules.notifications.db import notification_models as models -from dataall.base.db import paginate +from dataall.base.db import paginate, exceptions +from dataall.base.context import get_context +from functools import wraps + + +class NotificationAccess: + @staticmethod + def is_recipient(f): + @wraps(f) + def wrapper(*args, **kwds): + uri = kwds.get('notificationUri') + if not uri: + raise KeyError(f"{f.__name__} doesn't have parameter uri.") + context = get_context() + with context.db_engine.scoped_session() as session: + notification = session.query(models.Notification).get(uri) + if notification and (notification.recipient in context.groups + [context.username]): + return f(*args, **kwds) + else: + raise exceptions.UnauthorizedOperation( + action='UPDATE NOTIFICATION', + message=f'User {context.username} is not the recipient user/group of the notification {uri}', + ) + + return wrapper class NotificationRepository: @@ -88,6 +112,7 @@ def count_deleted_notifications(session, username, groups): return int(count) @staticmethod + @NotificationAccess.is_recipient def read_notification(session, notificationUri): notification = session.query(models.Notification).get(notificationUri) notification.is_read = True @@ -95,6 +120,7 @@ def read_notification(session, notificationUri): return True @staticmethod + @NotificationAccess.is_recipient def delete_notification(session, notificationUri): notification = session.query(models.Notification).get(notificationUri) if notification: