On 2/14/2011 9:43 PM, Steffen Hoffmann wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hello,
this time I'm bringing a recent discussion about permission checking to
your attention. I'm further more asking for advice on how to handle this
in the future.
Summary:
Initially there where two reports related reports against
PrivateTicketsPlugin [1][2] and TracHoursPlugin [3][4] about
incompatible permission check in TracHoursPlugin that caused an error
only if PrivateTicketsPlugin was activated too. This had been meant to
be resolved by two successive changesets [5][6] in TracHoursPlugin. But
later the same issue surfaced again for the combination of
PrivateTicketsPlugin with AnnouncerPlugin [7][8]. This time there was
some more discussion about what's actually going on inside.
Thanks to Odd Simon Simonsen and Ryan J. Ollos it became clear that
recurring issues sprang off from usage of
PermissionSystem.check_permission() [9] without `perm` argument. While
default policies (DefaultPermissionPolicy, LegacyAttachmentPolicy) don't
seem to have a problem with perm=None, PrivateTicketsPolicy does.
perm=None means we're checking for permission actions as a whole.
Known to have delicate semantics (see the whole discussion of #6211
which lead to the current documentation in perm.py; I just notice that
#6211 is still re-opened, another indication that we're not 100% clear
on this issue).
And frankly, the TICKET_VIEW permissions and al. are something of the
past, that we were not able to ditch yet. I'd happily move to short name
actions ('view', 'modify', etc.) on mandatory resources anytime. See
also
http://trac.edgewall.org/wiki/TracDev/Proposals/EvenFinerGrainedPermissions
for "recent" thoughts on future directions for the permission system. If
it's not already written there, I should add that some regex-based
authzpolicy permission system could probably help for speeding up the
thing considerably.
Proper permission checks should be fine-grained and always query
permissions per resource for such realms like tickets and wiki. But this
is not obvious when reading current doc-strings in perm.py . So should
we propose a documentation improvement, or is there another more
appropriate action? After all docs will not prevent more variations of
that issue, while a less tolerant API might do.
According to the current semantic, resource==None is a way to express
whole realm checks; i.e. if you are granted WIKI_VIEW for resource ==
None, that means you *may* have access to more specific wiki resources.
That would actually be the same as 'view' for Resource('wiki') in a
system with short action names and mandatory resources.
Trying to introduce fine-grained permission checks to AnnouncerPlugin
now I'm facing another challenge:
How do I detect the resource identifier of an arbitrary resource?
I don't follow you here. Arbitrary resources are just Resource, right?
Which are built using Resource(realm, id).
So the id of an arbitrary resource r is r.id. For being truly general,
you have to take the eventual .parent resources (and .parent chain
actually), see the tracopt/perm/authz_policy.py module for an example.
I'd be glad to be corrected, if I've overlooked something, but there
seems no way to be sure today. While initially it seems common to have
resource.name (i.e. resource = ticket)
That would be ... surprising, to say the least, knowing that we have
__slots__ = ('realm', 'id', 'version', 'parent') for Resource instances...
Hope this helps,
-- Christian
--
You received this message because you are subscribed to the Google Groups "Trac
Development" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/trac-dev?hl=en.
