Re: Feedback on upcoming permission features

Hello John,

This additional functionality is great, I already have respective use cases.

No idea if this breaks something elsewhere, on my plain simple setups it 
does not.

Best Regards,

   Georg Lehner


On 03/14/2017 10:39 AM, John P. Rouillard wrote:
> 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
>        classes
>
> If you change your check function definition to:
>
>   check(db, userid, itemid, **context)
>
> Adding **context provides the following values in the context
> dictionary:
>
>    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'),
>     check=is_this_groups_admin,
>     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
> functions:
>
>    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
> feature.
>
> 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",
>     props_only=True)
>                                        example 1
>
> There is also a method to change the default value of the
> props_only. Calling:
>
>   db.security.set_props_only_default(True)
>
> 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.set_props_only_default(True)
>
>   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
>
>   db.security.set_props_only_default(False)
>
>   # 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-users mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/roundup-users
>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot