On Sun, Apr 16, 2006 at 08:57:20PM +0200, Christian Boos wrote:
> Christopher Lenz wrote:
> >Am 16.04.2006 um 17:38 schrieb Alec Thomas:
> >>On Sun, Apr 16, 2006 at 04:34:06PM +0200, Christian Boos wrote:
> >>...
> >>>Then, for the "resource" itself, I think we should pass the "type"
> >>>information as well as the "id" only. Relying on the action to
> >>>decide what kind of resource to we handle seems a bit kludgy.
> >>
> >>I see your point, though my reasoning was that the type is implicit in
> >>the permission. However I guess we already have 'meta' permissions like
> >>TRAC_ADMIN, so maybe it won't be as implicit in the future.
> >
> >How about passing the resource *object*, e.g. the Ticket or WikiPage
> >instance?
> >
>
> Of course, that's even better.
My only reservation with this is that I currently like how you can apply
permissions to anything:
req.perm.assert_permission('CONFIG_VIEW', 'config', 'plugins')
Though I guess this we'd just need to create stub objects for anything
under resource control:
class ConfigPage:
""" Represents a configuration page. """
type = 'config'
def __init__(self, id):
self.id = id
> >The current permission system indeed combines resource-type and
> >action... if we start adding actual resources/objects to the
> >permission system, then the resource-type part should be removed,
> >because it's (in theory, at least) part of the resource specification.
>
> In practice all it takes would be that each object class should define a
> 'type' property,
> and 'id' property, e.g.
>
> class WikiPage(object):
> """Represents a wiki page (new or existing)."""
>
> + type = 'wiki'
> +
> def __init__(self, env, name=None, version=None, db=None):
> self.env = env
> - self.name = name
> + self.id = self.name = name
> if name:
> self._fetch(name, version, db)
> else:
>
> ...and so on. Not even a need for a common base class.
Simple and unobtrusive. Although I guess adding a base class wouldn't be
that much pain either, and then we could add some "convenience" methods:
page.has_permission(req, 'WIKI_VIEW')
Although to be honest I'm not sure this is any more convenient than
the current:
req.perm.has_permission('WIKI_VIEW', page) ??
Anyway, go with simple first I think :)
As a convenience, the permission methods could support a shorthand form:
req.perm.has_permission('VIEW', page)
Deriving the prefix from the page.type.
> Well, I'm sure a full fledge "recursive role system" must be worth
> waiting for post-1.0.
> However, if someone comes with a simpler approach for this, which seems
> to be sound,
> simple and appears to be a step forward from what we have, then why not
> adopt it?
That was exactly my motivation for this patch. The idea was to make it
as backwards compatible as humanly possible, while adding good
permission control.
To that end, I'd suggest keeping the current permissions (WIKI_VIEW,
etc.) and the default IPermissionStore system, to lessen the impact on
users.
I do agree though, that a full revamp of the permission system would be
ideal, but very disruptive and a great deal of work.
> >Still, I'd be in favor of you creating a sandbox branch for this work
> >(but only if it doesn't distract you from the workflow work too much ;-)
Sounds good, I'll create it today ;)
> Yes, and targeting it for 0.11...
I'd be +1 for this, obviously. The criteria being:
- Simple, but flexible
- Unobtrusive
- Backwards compatibility
For 1.0, all bets are off (?), but as with Spam control, I think useful
permission control would be better sooner than later.
So, regarding terminology? :) I suggest calling them 'resources', as
'objects' is very generic, and connotative of having some extra
functionality that may not exist. Not everything the security system
applies to will refelect this, eg. about/config pages.
user X has permission to user resource Y
--
Evolution: Taking care of those too stupid to take care of themselves.
_______________________________________________
Trac-dev mailing list
[email protected]
http://lists.edgewall.com/mailman/listinfo/trac-dev
