AuthzPolicy will fail silently if ConfigObj is not available

[Dirk Stöcker]

When python-configobj is not available, the AuthzPolicy fails without any notice (except a log entry). In the default config that means, that all pages are accessible and any restrictions are void. This is VERY dangerous.

Immediate Fix:

/usr/lib/python2.7/site-packages/tracopt/perm/authz_policy.py

@@ -139 +139 @@
 
     def check_permission(self, action, username, resource, perm):
         if ConfigObj is None:
-            self.log.error('configobj package not found')
-            return None
+            self.log.error('AuthzPolicy: configobj package not found')
+            return False # never silently fail!
 
         if self.authz_file and not self.authz_mtime or \
                 os.path.getmtime(self.get_authz_file()) > self.authz_mtime:

Also the setup.py should show clearly, that AuthzPolicy requires python-configobj to make the problem obvious.

[Ryan J Ollos]

Related:

• After #11272: if the authz file can't be parsed or can't be found, a ConfigurationError is raised.
• After #10285: If AuthzPolicy is added to [trac] permission_policies but the component is not enabled or fails to load a ConfigurationError will be raised.

If [authz_policy] authz_file is not specified in trac.ini but AuthzPolicy is active, there is currently no error. We should probably raise a ConfigurationError in this case.

[Ryan J Ollos]

Proposed changes can be found in rjollos.git:t11293.

[Ryan J Ollos]

rjollos.git:1c203a85 results in a ConfigurationError being raised immediately when tracopt.perm.authz_policy is enabled, and before any other steps are taken to configure the installation for authz permissions. It seems better to only raise the ConfigurationError when AuthzPolicy is in the list of active permission_policies. So the behavior I propose is, allow tracopt.perm.authz_policy to be enabled but don't perform any error checks unless AuthzPolicy is in the list of active permission_policies. Once AuthzPolicy is active, we are very strict about failing with a ConfigurationError if the authz policy won't be enforced due to a configuration error or dependency not being loaded. Furthermore, after #10285, if AuthzPolicy is active but tracopt.perm.authz_policy is not enabled, a ConfigurationError will be raised, and this also extends to the case of raising a ConfigurationError if a permission policy in permission_policies is misspelled.

In rjollos.git:t11293.2 another case is covered by raising a ConfigurationError if the authz_file is empty.

[Ryan J Ollos]

Committed to 1.0-stable in [12037:12039]. Merged to trunk in [12040].

[Jun Omae]

I got ConfigurationError and ERROR: The authz file is empty. in trac.log when I temporarily comment out (or remove) all settings in authzpolicy file like the following. I feel the behavior is strange and confused me.

# [admin:ticket/versions]
# foobar = TRAC_ADMIN
#
# [repository:babel@*]
# foobar = !BROWSER_VIEW, !FILE_VIEW

I think we should allow authzpolicy file to be empty.

[Ryan J Ollos]

Allowing an empty file sounds fine to me.

[Jun Omae]

I've pushed changes which allows an empty authz policy in [12812].

I mentioned authzpolicy doesn't work for default repository in comment:10:ticket:10961. I think the behavior is a defect. Proposed fix can be found in jomae.git@t11293.4.