Error message from trac-admin is ambiguous when permission can't be removed

[Ryan J Ollos]

The following was discussed this evening on IRC. Suppose I have a default Trac installation and assign an arbitrarily-chosen permission to a new user:

$ trac-admin tracdev permission add user1 WIKI_DELETE

Listing the permissions for that user, it appears they have WIKI_VIEW:

$ trac-admin ../tracdev permission list user1

User   Action                 
------------------------------
user1  BROWSER_VIEW           
user1  CHANGESET_VIEW         
user1  CONFIG_VIEW            
user1  EMAIL_VIEW             
user1  FILE_VIEW              
user1  LOG_VIEW               
user1  MILESTONE_ADMIN        
user1  MILESTONE_CREATE       
user1  MILESTONE_DELETE       
user1  MILESTONE_MODIFY       
user1  MILESTONE_VIEW         
user1  PERMISSION_ADMIN       
user1  PERMISSION_GRANT       
user1  PERMISSION_REVOKE      
user1  REPORT_ADMIN           
user1  REPORT_CREATE          
user1  REPORT_DELETE          
user1  REPORT_MODIFY          
user1  REPORT_SQL_VIEW        
user1  REPORT_VIEW            
user1  ROADMAP_ADMIN          
user1  ROADMAP_VIEW           
user1  SEARCH_VIEW            
user1  TICKET_ADMIN           
user1  TICKET_APPEND          
user1  TICKET_BATCH_MODIFY    
user1  TICKET_CHGPROP         
user1  TICKET_CREATE          
user1  TICKET_EDIT_CC         
user1  TICKET_EDIT_COMMENT    
user1  TICKET_EDIT_DESCRIPTION
user1  TICKET_MODIFY          
user1  TICKET_VIEW            
user1  TIMELINE_VIEW          
user1  TRAC_ADMIN             
user1  VERSIONCONTROL_ADMIN   
user1  WIKI_ADMIN             
user1  WIKI_CREATE            
user1  WIKI_DELETE            
user1  WIKI_MODIFY            
user1  WIKI_RENAME            
user1  WIKI_VIEW              

However, it is not possible to remove the permission:

$ trac-admin ../tracdev permission remove user1 WIKI_VIEW
Error: Cannot remove permission WIKI_VIEW for user user1.

This is because the permission is assigned to the anonymous group:

$ trac-admin ../tracdev permission list anonymous

User       Action                 
----------------------------------
anonymous  BROWSER_VIEW           
anonymous  CHANGESET_VIEW         
anonymous  CONFIG_VIEW            
anonymous  EMAIL_VIEW             
anonymous  FILE_VIEW              
anonymous  LOG_VIEW               
anonymous  MILESTONE_ADMIN        
anonymous  MILESTONE_CREATE       
anonymous  MILESTONE_DELETE       
anonymous  MILESTONE_MODIFY       
anonymous  MILESTONE_VIEW         
anonymous  PERMISSION_ADMIN       
anonymous  PERMISSION_GRANT       
anonymous  PERMISSION_REVOKE      
anonymous  REPORT_ADMIN           
anonymous  REPORT_CREATE          
anonymous  REPORT_DELETE          
anonymous  REPORT_MODIFY          
anonymous  REPORT_SQL_VIEW        
anonymous  REPORT_VIEW            
anonymous  ROADMAP_ADMIN          
anonymous  ROADMAP_VIEW           
anonymous  SEARCH_VIEW            
anonymous  TICKET_ADMIN           
anonymous  TICKET_APPEND          
anonymous  TICKET_BATCH_MODIFY    
anonymous  TICKET_CHGPROP         
anonymous  TICKET_CREATE          
anonymous  TICKET_EDIT_CC         
anonymous  TICKET_EDIT_COMMENT    
anonymous  TICKET_EDIT_DESCRIPTION
anonymous  TICKET_MODIFY          
anonymous  TICKET_VIEW            
anonymous  TIMELINE_VIEW          
anonymous  TRAC_ADMIN             
anonymous  VERSIONCONTROL_ADMIN   
anonymous  WIKI_ADMIN             
anonymous  WIKI_CREATE            
anonymous  WIKI_DELETE            
anonymous  WIKI_MODIFY            
anonymous  WIKI_RENAME            
anonymous  WIKI_VIEW              

We should at least try to provide an indication of why the permission can't be removed from the user

[Ryan J Ollos]

I proposed a simple way to provide a better error message in log:rjollos.git:t11335. It would be ideal to tell the user which meta-permissions or groups are granting the permission that the user is attempting to revoke, but that will require quite a bit more code, so I'll leave that for future work.

[Jun Omae]

Replying to rjollos:

I proposed a simple way to provide a better error message in log:rjollos.git:t11335.

At least, in Japanese (maybe also Chinese), sentences are not concatenated with spaces. I would like to be the following while it might be verbose.

trac/perm.py

@@ -685 +685 @@
                     permsys.revoke_permission(u, a)
                     found = True
             if not found:
-                msg = _("Cannot remove permission %(action)s for user %(user)s.",
-                        action=action, user=user)
                 if user in self.get_user_list() and \
                         action in permsys.get_user_permissions(user):
-                    raise AdminCommandError(msg + " " +
-                        _("The permission %(action)s is granted through "
-                          "a meta-permission or group.", action=action)
-                    )
-                raise AdminCommandError(msg)
+                    msg = _("Cannot remove permission %(action)s for user "
+                            "%(user)s. The permission %(action)s is granted "
+                            "through a meta-permission or group.",
+                            action=action, user=user)
+                else:
+                    msg = _("Cannot remove permission %(action)s for user "
+                            "%(user)s.", action=action, user=user)
+            raise AdminCommandError(msg)
 
     def _do_export(self, filename=None):
         try:

[Ryan J Ollos]

Thanks, I had doubts about the space but forgot to ask about it. Committed to 1.0-stable in [12229] and merged to trunk in [12230].