Feedback on upcoming permission features

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
Report Content as Inappropriate

Feedback on upcoming permission features

John P. Rouillard
Hi all:

I just wanted to discuss a couple of changes to permissions
that are planned for the next release. One is already in the
repository and the other one I plan on pushing this weekend.

One of the difficulties with roundup is getting the security
model and permissions set up just right. I hope these
changes will make that easier. I have a tracker that uses
both of these features and it made implementing the security
model much easier for me.

The first change, that is in the repository, deals with the
check function assigned to permissions.  The check function
is defined as:

  check(db, userid, itemid)

The problem with this is that you are missing context about the
permission test that you may want to use E.G.:

   is this a search on a property or on the class
   what class is this applied to
   what permission is being checked

Having this info allows you to:

   allow access to a property but denying access to a
       class (more about this case below)

   write a single function that applies similar logic to
       different (Edit and View for example) permissions

   write a function that applies similar logic to multiple

If you change your check function definition to:

  check(db, userid, itemid, **context)

Adding **context provides the following values in the context

   context['property'] the name of the property being
       checked or None if it's a class check.

   context['classname'] the name of the class that is being
       checked (issue, query ....).

   context['permission'] the name of the permission
       (e.g. View, Edit...).

Note this is optional. The three argument form of check
commands will continue to work as they always have. You do
not need to change your schema unless you want this feature.

The second (pending) change is a simpler way to limit
permission tests so they apply only to properties. Note that
permissions defined with a prperty list also apply to
non-property tests.

Define a permission like:

  db.security.addPermission(name='View', klass='user',
    properties=('activity', 'id', 'organisation', 'phone', 'realname',
            'timezone', 'username', 'email', 'alternate_addresses'),
    description="Group admins can have expanded view of other users")

and assign it to a role. This role can view the properties
of the user class, but they *also inherit the ability to
view* the user class.

This makes sense since the usual way to view those
properties is by looking at the user class (to get an index
page) or to look at specific user (e.g. user3). When a user
looks at an index or item page, any properties that are not
listed in the permission above are displayed as [hidden].

However suppose you want to hide the user class entirely
from a role while allowing the role to read just a
particular property.

I wanted to deny access to the user class for the anonymous
user/role (including for its user item).  However I wanted
the anonymous user to have access to the theme property in
its user item.  I wanted attempts by anonymous to look at
tracker/user or tracker/user2 (the anonymous user) to look
like anonymous had no access to the user class at all.

I could change the user templates and change the
tal:condition="context/is_view_ok" to check specifically for
the anonymous user, but that gets out of hand quickly.

I could rewrite the check function to use the new context
argument.  So the check function would look to see if the
user was anonymous and if there was no property specified, I
could deny access.

This could be done, but I also had the feeling that this is
something that should happen at the permission level and not
at a check function.

So I have added a new optional argument to the permission

   props_only = True or False

If this is not specified, it acts as though the value is
False. This keeps the current behavior and permits a user to
access a class if they can access a property on that class.
So you don't need to change anything if you don't want the

For my use case above, I can make the permission apply only
to property checks by using:

  db.security.addPermission(name='View', klass='user',
    properties=['theme'], check=own_record,
    description="User is allowed to view their own theme",
                                       example 1

There is also a method to change the default value of the
props_only. Calling:


in schema.py makes all permissions that include properties
apply only when checking properties. They are never checked
when determining access rights for the class. Using this I
could have written example 1 using:


  db.security.addPermission(name='View', klass='user',
    properties=['theme'], check=own_record,
    description="User is allowed to view their own theme")

  # other permissions with properties defined here will apply only
  # to property access tests


  # permissions with properties defined here will apply for
  # property access tests and for class access tests

This code has not yet made it to the repository, but I am
interested in feedback on both of these methods.

There is one non-optional change. The output of roundup-admin security
will change. Rather than seeing:

 User is allowed to Search queries for creator
      (Search for "query": ['creator'] only)

the final word "only" will only show up with properties if
props_only=True.  Does this break anybody's scripts that use
the security command?

Do these changes address any issues you have had writing
permissions?  Will they make it easier to write permissions?

Feedback welcome.

                                -- rouilj
John Rouillard
My employers don't acknowledge my existence much less my opinions.

Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Roundup-devel mailing list
[hidden email]