Skip to content

Commit

Permalink
Add permission checks to markNotificationAsRead + deleteNotification (#…
Browse files Browse the repository at this point in the history
…1654)

### Feature or Bugfix
<!-- please choose -->
- 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.
  • Loading branch information
noah-paige authored and dlpzx committed Nov 6, 2024
1 parent f029f45 commit 768119e
Showing 1 changed file with 27 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -88,13 +112,15 @@ 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
session.commit()
return True

@staticmethod
@NotificationAccess.is_recipient
def delete_notification(session, notificationUri):
notification = session.query(models.Notification).get(notificationUri)
if notification:
Expand Down

0 comments on commit 768119e

Please sign in to comment.