Private ticket permission users can get ticket counts that include tickets they're not allowed to view

[jevans]

A user with private tickets permissions can still query how many tickets meet criteria even if they can't see the tickets listed or view them.

For instance they can type in query?status=!closed&priority=critical to get a count of how many critical defects are open.

I originally wrote this ticket on the PrivateTicketsPlugin (#3674) but heard that it's a problem in Trac core.

[osimons]

Looked into this one, and the Query module and related features such as the TicketQuery macro do not check for permissions. The only place where permission checks do happen is when listing individual tickets in the query_results.html template.

That means:

• As you found, the total does not get adjusted if individual tickets are not permitted to display due to some custom permission
• A format=count for the macro will just fetch the gross count regardless of permissions
• If no tickets in an existing group is allowed for display, we still display the group header and count (should be removed)
• And, no doubt the pagination code also works with the full totals regardless of permissions - haven't looked into this, but when seeing the usage of LIMIT and OFFSET in the Query.execute() method I suppose that also works with gross numbers

For something like the th:PrivateTicketsPlugin where likely only a small percentage of tickets should actually be recognized for a given user, this will no doubt lead to a 'sub-optimal' display...

To fix this, I suggest moving the permission checks into the Query.execute() and Query.count() methods instead of doing it in template. That however looks to be a non-trivial change.

[rblank]

Replying to osimons:

To fix this, I suggest moving the permission checks into the Query.execute() and Query.count() methods instead of doing it in template. That however looks to be a non-trivial change.

This means that the result set of Query.execute() will not be only the result of an SQL query, but also of some extra filtering (in this case, permission checks). This would open the door to solving #7558 quite eaily.

[cboos]

osimons described the implementation issues quite accurately. It's something I've also pondered while working on the pagination and the ticket query macro. At that time, it seemed too costly and complicated to take the permissions into account, for the general case where fine grained ticket permissions are probably not so commonly used. It also meant to rewrite the pagination code completely, as I don't see how the COUNT/OFFSET/LIMIT logic can possibly work if permission checks have to be done, not to mention that fetching all those tickets would also partially defeat the memory optimization goals we had for implementing #216 in the first place.

Just consider the performance costs for a Wiki page containing a matrix of [[TicketQuery(...,format=count)]] macros. If you have a dozen or so of them, care must be taken that the page remains viewable at all, especially considering that #6436 is still not solved.

So I'd say we should have an efficient way to retrieve the tickets when many queries are performed in a request (the above use case), eventually by implementing a ticket cache, then rewrite the pagination logic so that it operates on the subset of tickets that match the filtering conditions (as Remy said, there could be more non-SQL checks than just the permission ones), and see if that's a viable option.

Alternatively, we could just make clear that the numbers shown concern all existing tickets, even non viewable ones. After all, that's what can happen with changesets: you could eventually look at r7000 if you have the permissions, so you know there are at least 6999 more changesets, even if none of them are viewable...