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 .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from bobbysmith007. Next status will be '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.