Modify

Opened 16 years ago

Closed 13 years ago

Last modified 13 years ago

#3828 closed defect (fixed)

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

Reported by: Thomas Wagner Owned by: Russ Tyndall
Priority: normal Component: TimingAndEstimationPlugin
Severity: normal Keywords:
Cc: Thomas Wagner 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 Shane Caraveo 16 years ago.
blackmagic fix for checkbox fields

Download all attachments as: .zip

Change History (12)

comment:1 Changed 16 years ago by Thomas Wagner

Cc: Thomas Wagner added; anonymous removed

comment:2 Changed 16 years ago by Russ Tyndall

(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 16 years ago by Russ Tyndall

Resolution: fixed
Status: newclosed

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 16 years ago by Jodok Batlogg

Resolution: fixed
Status: closedreopened

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 16 years ago by Russ Tyndall

Summary: Permission Problem with internal ticketsInefficien Permission Check (Was:Permission Problem with internal tickets)

comment:6 Changed 16 years ago by Russ Tyndall

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

comment:7 Changed 16 years ago by Shane Caraveo

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 16 years ago by Shane Caraveo

Attachment: t-e-blackmagic.patch added

blackmagic fix for checkbox fields

comment:8 Changed 16 years ago by Shane Caraveo

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 16 years ago by Russ Tyndall

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 13 years ago by Russ Tyndall

Resolution: fixed
Status: reopenedclosed

(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 13 years ago by Ryan J Ollos

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()).

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Russ Tyndall.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


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

 
Note: See TracTickets for help on using tickets.