Edgewall Software

Changes between Initial Version and Version 1 of Ticket #11069, comment 1


Ignore:
Timestamp:
Jul 30, 2013, 8:47:36 PM (11 years ago)
Author:
Ryan J Ollos

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #11069, comment 1

    initial v1  
    33 * Users with `PERMISSION_REVOKE` can revoke **any** permission, so we don't have symmetry with `PERMISSION_GRANT`. The patch includes a change to disable the checkboxes and prevent users from revoking permissions that they don't themselves have. I could imagine throwing this part away if it is deemed too complex. However, consider that a user with `PERMISSION_REVOKE` could remove `TRAC_ADMIN` permission from an administrator, even if they themselves are not an administrator.
    44 * To add a subject to a group, the user must have all of the permissions that belong to that group. If not, on the first permission check that fails, an error such as ''TICKET_CREATE privileges are required to perform this operation. You don't have the required permissions.'' will be raised. This seems a bit ambiguous and we might want to improve on the message that is displayed. I think this would typically be confusing to a user as to why they can't add a subject to a group.
    5  * The logging panel was missing a permission check for `render_admin_panel` when compared to the `render_admin_panel` method for most of the other panels. However, it appears this permission check is not necessary because `render_admin_panel` won't be called unless `get_admin_panels` yields a tuple, and a permissions check is always done before yielding in `get_admin_panels` (see [/browser/trunk/trac/admin/web_ui.py?rev=11054&marks=72-75#L53 here] and [/browser/trunk/trac/admin/web_ui.py?rev=11054&marks=169#L163 here]). Therefore I removed the permission checks for all `render_admin_panels` methods rather than modifying the permissions checks for fine-grained checking. The ticket admin panels don't perform permission checks in `render_admin_panel` or `_render_admin_panel` either, presumably for the reason I've described here.
     5 * The logging panel was missing a permission check for `render_admin_panel` when compared to the `render_admin_panel` method for most of the other panels. However, it appears this permission check is not necessary because `render_admin_panel` won't be called unless `get_admin_panels` yields a tuple, and a permissions check is always done before yielding in `get_admin_panels` ([/browser/trunk/trac/admin/web_ui.py?rev=11054&marks=169#L163 get panels] -> [/browser/trunk/trac/admin/web_ui.py?rev=11054&marks=209-210#L201 check perm] -> [/browser/trunk/trac/admin/web_ui.py?rev=11054&marks=89,123-124#L88 render panels]). Therefore I removed the permission checks for all `render_admin_panels` methods rather than modifying the permissions checks for fine-grained checking. The ticket admin panels don't perform permission checks in `render_admin_panel` or `_render_admin_panel` either, presumably for the reason I've described here.
    66 *  The milestone on the Admin Milestone panel are **not** filtered by fine-grained permissions checks, and the panel is only shown if the user has the coarse-grained `MILESTONE_VIEW` permission. I spent some time looking at this, and while I feel that we might want to do fine-grained permission checks on the milestones, it seems like this patch is getting overly complex and I need some other opinions on the matter. I didn't dive into doing fine-grained permission checks on the repositories for the same reason (and because it was making my head spin).
    77