Mercurial > kallithea
changeset 5875:abc1ada59076
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.
| author | Søren Løvborg <sorenl@unity3d.com> |
|---|---|
| date | Tue, 19 Apr 2016 18:03:30 +0200 |
| parents | b103f41a4954 |
| children | ea02c8b2b529 |
| files | kallithea/controllers/admin/notifications.py |
| diffstat | 1 files changed, 13 insertions(+), 17 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/controllers/admin/notifications.py Fri Feb 05 21:19:44 2016 +0100 +++ b/kallithea/controllers/admin/notifications.py Tue Apr 19 18:03:30 2016 +0200 @@ -147,27 +147,23 @@ 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) + unotification = NotificationModel() \ + .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 not unotification.read: - unotification.mark_as_read() - Session().commit() - c.notification = no + # if this association to user is not valid, we don't want to show + # this message + if unotification is None: + raise HTTPForbidden() - return render('admin/notifications/show_notification.html') + if not unotification.read: + unotification.mark_as_read() + Session().commit() - raise HTTPForbidden() + c.notification = notification + c.user = self.authuser + return render('admin/notifications/show_notification.html') def edit(self, notification_id, format='html'): """GET /_admin/notifications/id/edit: Form to edit an existing item"""