kallithea Changeset - abc1ada59076

Changeset - abc1ada59076

Parent rev.

Child rev.

[Not reviewed]

default

0 1 0

Søren Løvborg - 10 years ago 2016-04-19 18:03:30
sorenl@unity3d.com

notifications: untangle notification access check

This removes a broken permission check when viewing notifications (the
HasRepoPermissionAny object was created, but never actually called with
a repo_name argument as required). It would be non-trivial to actually
implement the check, as notifications don't track their repository
relationship explicitly, and even then, it's unclear why it would
make sense to allow a repository admin to see notifications to
other users.

It was never a vulnerability, due to a subsequent (and much stricter)
ownership check, which remains but has been untangled for readability.
In short, this changeset is a pure refactoring, except that specifying
a non-existent notification ID will now produce error 404, not 403.

1 file changed with 7 insertions and 11 deletions:

kallithea/controllers/admin/notifications.py

0 comments (0 inline, 0 general)

kallithea/controllers/admin/notifications.py

➞

Show inline comments

@@ @@ -147,28 +147,24 @@ class NotificationsController(BaseContro @@
     def show(self, notification_id, format='html'):
         """GET /_admin/notifications/id: Show a specific item"""
         # url('notification', notification_id=ID)
         c.user = self.authuser
         no = Notification.get(notification_id)
         notification = Notification.get_or_404(notification_id)
         owner = any(un.user.user_id == c.authuser.user_id
                     for un in no.notifications_to_users)
         repo_admin = h.HasRepoPermissionAny('repository.admin')
         if no and (h.HasPermissionAny('hg.admin')() or repo_admin or owner):
             unotification = NotificationModel() \
-                            .get_user_notification(c.user.user_id, no)
+            .get_user_notification(self.authuser.user_id, notification)
             # if this association to user is not valid, we don't want to show
             # this message
             if unotification is not None:
         if unotification is None:
             raise HTTPForbidden()
                 if not unotification.read:
                     unotification.mark_as_read()
                     Session().commit()
                 c.notification = no
         c.notification = notification
         c.user = self.authuser
                 return render('admin/notifications/show_notification.html')
         raise HTTPForbidden()
     def edit(self, notification_id, format='html'):
         """GET /_admin/notifications/id/edit: Form to edit an existing item"""
         # url('edit_notification', notification_id=ID)

0 comments (0 inline, 0 general)