[PATCH] Per-configuration permissioning

[Roland Wilczek <r.wilczek@…>]

The following patch and tutorials allows to permit users to

• access build configurations read-only
• modify configurations on a per-configuration-base

Creating and deleting configurations is left unchanged, and requires the general permissions BUILD_CREATE and BUILD_DELETE.

BUILD_MODIFY permits to

• modify all configurations.
• create, delete and update target platforms

The permission to modify a build configuration named foobar to a single user is granted by:

• granting BUILD_LIST to the user
• granting BUILD_MODIFY_foobar to the user

I can think of much nicer (and more capable) solutions, but Bitten's permissioning strategy seems not to really comply with trac's idea. So the capabilities of plugins like Authz Policy? and Extra Permissions Provider? cannot be completely used.

Tutorial

Install package ​configobj. Maybe easy_install configobj does the job.

In the [components] section of trac.ini add:

[components]
tracopt.perm.authz_policy.authzpolicy = enabled
tracopt.perm.config_perm_provider.extrapermissionsprovider = enabled

Create a new section [extra-permissions] in trac.ini if necessary and add:

[extra-permissions]
_perms = BUILD_LIST

Create a section [authz_policy] in trac.ini and define:

[authz_policy]
authz_file = conf/authz.policy

Create a new file authz.policy in /path/to/trac/conf/. (Path and name must comply with that given as authz_file in authz_policy in your trac.ini, of course:

[*]
# To permit a user to administer a build configuration, 
# grant BUILD_LIST to him and permit him to BUILD_MODIFY_configname:
# bob  = BUILD_LIST, BUILD_MODIFY_configurationName
# The user is then able to manage the build configuration's metadata 
# (name, label, description, recipe and repository-mapping).
# Managing target-platforms as well as creating, deleting, activating/deacivating 
# a configuration requires the global permission BUILD_MODIFY.
Tom = BUILD_LIST
Bob = BUILD_LIST, BUILD_MODIFY_BobsConfig
Tim = BUILD_MODIFY_TimsConfig, BUILD_MODIFY_BobsConfig
Max = BUILD_MODIFY

Invoke the patch below.

Restart Apache.

Result:

• Tom has read-only access to all build-configurations within the Admin-panel.
• Bob has read-only access to all build-configurations and is able to modify BobsConfig
• Tim lacks the permission BUILD_LIST. Thus his permission to modify TimsConfig and BobsConfig is revoked. As soon as he is permitted to BUILD_LIST, the permissions will be activated again.
• Max is able to view and modify all build configurations. He is the only one permitted to
  • create new build configurations
  • delete existing build configurations
  • set existing build configurations active|inactive
  • create, modify and delete target platforms

SCM/Bitten/bitten/admin.py

@@ -89 +89 @@
 
 
 class BuildConfigurationsAdminPageProvider(Component):
     """Web administration panel for configuring the build master."""
 
     implements(IAdminPanelProvider)
 
     # IAdminPanelProvider methods
 
     def get_admin_panels(self, req):
-        if req.perm.has_permission('BUILD_MODIFY'):
+        if req.perm.has_permission('BUILD_LIST') or req.perm.has_permission('BUILD_MODIFY'):
             yield ('bitten', 'Builds', 'configs', 'Configurations')
 
     def render_admin_panel(self, req, cat, page, path_info):
         data = {}
 
         # Analyze url
         try:
             config_name, platform_id = path_info.split('/', 1)
         except:
             config_name = path_info
@@ -257 +257 @@
         db = self.env.get_db_cnx()
         for name in sel:
             config = BuildConfig.fetch(self.env, name, db=db)
             if not config:
                 raise TracError('Configuration %r not found' % name)
             config.delete(db=db)
         db.commit()
 
     def _update_config(self, req, config):
         warnings = []
-        req.perm.assert_permission('BUILD_MODIFY')
+        if not req.perm.has_permission('BUILD_MODIFY_' + req.args.get('name')): 
+            req.perm.assert_permission('BUILD_MODIFY')
 
         name = req.args.get('name')
         if not name:
             warnings.append('Missing required field "name".')
         if name and not re.match(r'^[\w.-]+$', name):
             warnings.append('The field "name" may only contain letters, '
                             'digits, periods, or dashes.')
 
         repos = self.env.get_repository(authname=req.authname)
         if not repos:
@@ -342 +343 @@
 
         db = self.env.get_db_cnx()
         for platform_id in sel:
             platform = TargetPlatform.fetch(self.env, platform_id, db=db)
             if not platform:
                 raise TracError('Target platform %r not found' % platform_id)
             platform.delete(db=db)
         db.commit()
 
     def _update_platform(self, req, platform):
+        req.perm.assert_permission('BUILD_MODIFY')
         platform.name = req.args.get('name')
 
         properties = [int(key[9:]) for key in req.args.keys()
                       if key.startswith('property_')]
         properties.sort()
         patterns = [int(key[8:]) for key in req.args.keys()
                     if key.startswith('pattern_')]
         patterns.sort()
         platform.rules = [(req.args.get('property_%d' % property).strip(),
                            req.args.get('pattern_%d' % pattern).strip())

[osimons]

What we really should do is support the resource system fully, and not just partially. I added resource support to Bitten as it was required for attachment support, but I have not yet pushed it into the permission checks.

Supporting permissions properly would mean that we should strive to do checks like:

... 'BUILD_VIEW' in perm(resource) ...

and

... perm(resource).assert_permission('BUILD_MODIFY') ...

That way any permission check in Bitten can be subject to manipulation by regular permission policy plugin - like AuthzPolicy.

It would not require much effort to make sure all checks referred to a resource (where applicable) + change/add some unit tests to verify this. There are only ~40 occurrences of BUILD_* in the repository (all code + tests + documentation), but need to review to make sure we loop all resources in the various views.

I'd rather go that way than change the permission structure itself.

[Roland Wilczek <r.wilczek@…>]

I'd appreciate a better integration of Bitten into trac's resource system a lot!