Modify

Opened 6 years ago

Closed 3 years ago

Last modified 2 years ago

#3828 closed defect (fixed)

Inefficient Permission Check (Was:Permission Problem with internal tickets)

Reported by: watho@… Owned by: bobbysmith007
Priority: normal Component: TimingAndEstimationPlugin
Severity: normal Keywords:
Cc: watho@… Trac Release: 0.11

Description

Setting a ticket to internal leads to error message: Error forbidden, Permission TICKET_VIEW needed.

I can only access the tickets as admin.

I'm using Trac 0.12dev r7557

Attachments (1)

t-e-blackmagic.patch (1.0 KB) - added by mixedpuppy 5 years ago.
blackmagic fix for checkbox fields

Download all attachments as: .zip

Change History (12)

comment:1 Changed 6 years ago by watho

  • Cc watho@… added

comment:2 Changed 6 years ago by bobbysmith007

(In [4358]) re #3828 fixed a bug in ticket policy where it should have been checking for a permission or a group, but was only checking for a permission

comment:3 Changed 6 years ago by bobbysmith007

  • Resolution set to fixed
  • Status changed from new to closed

If you were using a group name instead of a permission name, this would not have worked. This has been fixed now and we check for group membership or permission.

If you want to set what group/permission is used you can do so in the trac.ini file.

[ticket]
internalgroup = PermissionOrGroupHere

If you still experience the error after upgrading please reopen. Thanks for the bug report,

Russ

comment:4 Changed 6 years ago by jodok

  • Resolution fixed deleted
  • Status changed from closed to reopened

that's very inefficient as it calls the def get_all_permissions(self): on the authentication backend.

and e.g. in an ldap environment it says:

        """Retrieve the permissions for all users from the LDAP directory"""
        # do not use the cache as this method is only used for administration
        # tasks, not for runtime

i disabled the possibility to check for groups as it took > 10 seconds to do this check. probably there is a smarter way to check for that?

comment:5 Changed 6 years ago by bobbysmith007

  • Summary changed from Permission Problem with internal tickets to Inefficien Permission Check (Was:Permission Problem with internal tickets)

comment:6 Changed 6 years ago by bobbysmith007

  • Summary changed from Inefficien Permission Check (Was:Permission Problem with internal tickets) to Inefficient Permission Check (Was:Permission Problem with internal tickets)

comment:7 Changed 5 years ago by mixedpuppy

This happens for me as well, but for a slightly different reason. I set internal.value=0 in trac.ini, but the form has '1' as the value (if the field gets hidden). The result is that any non-TRAC_ADMIN user submitting a ticket ends up creating a ticket with the 'internal' flag on, then get denied viewing the ticket they just submitted.

Changed 5 years ago by mixedpuppy

blackmagic fix for checkbox fields

comment:8 Changed 5 years ago by mixedpuppy

The patch I just added properly deals with checkbox values when hiding a field. There is probably more that should be done, but this fixes the initial problem for me.

repro for my issue:

in trac.ini set internal.value=0
as a user who has TICKET_CREATE and TICKET_VIEW permissions, but not admin or "internal" permission, create a new ticket.

You will get the error message from the description of this ticket.

comment:9 Changed 5 years ago by bobbysmith007

Thanks for the patch, but in response to #4400 I added a patch just like this on friday: [5348], so you may want to update to the latest version of this plugin.

I will attempt to reproduce your problem and figure out what is wrong... Your problem seems to be somewhat different than the one this ticket is about. I will assess it futher, when I can test the error.

Still no word on efficient permission check.

HTH, Russ

comment:10 Changed 3 years ago by bobbysmith007

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [10626]) ver 1.1.8b

when searching for groups, dont query all permissions unless we are
using the default store (LDAP store takes a long time to return all
permissions).

fix #3828

comment:11 Changed 2 years ago by rjollos

I see that you also implemented the same fix as in #3222 as part of [10626], so mixed-case group names should be supported by your plugin after [10626] (action.islower() -> not action.isupper()).

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.