Permission restriction not working, anyone can post review

[ntmlod]

I have tried to fix it by myself, I add IPermissionRequestor methods so the permissions seems to be complied with from that.
I modify the test that controls the modification of ./changeset page but it's not working.

coderev/web_ui.py

@@ -41 +41 @@
 class CodeReviewerModule(Component):
     """Base component for reviewing changesets."""
 
-    implements(ITemplateProvider, IRequestFilter)
+    implements(ITemplateProvider, IRequestFilter, IPermissionRequestor)
 
     # config options
     statuses = ListOption('codereviewer', 'status_choices',
@@ -68 +68 @@
     def get_templates_dirs(self):
         return []
 
+    # IPermissionRequestor methods
+
+    def get_permission_actions(self):
+    return ['CODEREVIEWER_MODIFY']
+
     # IRequestFilter methods
 
     def pre_process_request(self, req, handler):
@@ -76 +81 @@
     def post_process_request(self, req, template, data, content_type):
         diff_mode = data and 'changeset' in data and \
                     data['changeset'] is False
-        if req.path_info.startswith('/changeset') and not diff_mode:
+        if req.path_info.startswith('/changeset') and not diff_mode and 'CODEREVIEWER_MODIFY' in req.perm:
             changeset = data['changeset']
             repos = changeset.repos
             reponame, rev = repos.reponame, repos.db_rev(changeset.rev)

[ntmlod]

Stupid mistake, I forgot the import at the beginning.

coderev/web_ui.py

@@ -16 +16 @@
 from trac.admin.api import IAdminCommandProvider
 from trac.core import *
 from trac.config import ListOption, Option
+from trac.perm import IPermissionRequestor
 from trac.resource import Resource
 from trac.util.datefmt import format_datetime, from_utimestamp, \
                               to_utimestamp, user_time
@@ -41 +42 @@
 class CodeReviewerModule(Component):
     """Base component for reviewing changesets."""
 
-    implements(ITemplateProvider, IRequestFilter)
+    implements(ITemplateProvider, IRequestFilter, IPermissionRequestor)
 
     # config options
     statuses = ListOption('codereviewer', 'status_choices',
@@ -68 +69 @@
     def get_templates_dirs(self):
         return []
 
+    # IPermissionRequestor methods
+
+    def get_permission_actions(self):
+    return ['CODEREVIEWER_MODIFY']
+
     # IRequestFilter methods
 
     def pre_process_request(self, req, handler):
@@ -76 +82 @@
     def post_process_request(self, req, template, data, content_type):
         diff_mode = data and 'changeset' in data and \
                     data['changeset'] is False
-        if req.path_info.startswith('/changeset') and not diff_mode:
+        if req.path_info.startswith('/changeset') and not diff_mode and 'CODEREVIEWER_MODIFY' in req.perm:
             changeset = data['changeset']
             repos = changeset.repos
             reponame, rev = repos.reponame, repos.db_rev(changeset.rev)

Seems to be working now

[Jun Omae]

I don't think it is needed to implement IPermissionRequestor in CodeReviewerModule because CODEREVIEWER_MODIFY is declared in CodeReviewerSystem.get_permission_actions().

codereviewerplugin/1.0/coderev/web_ui.py

@@ -76 +76 @@
     def post_process_request(self, req, template, data, content_type):
         diff_mode = data and 'changeset' in data and \
                     data['changeset'] is False
-        if req.path_info.startswith('/changeset') and not diff_mode:
+        if req.path_info.startswith('/changeset') and not diff_mode and \
+                'CODEREVIEWER_MODIFY' in req.perm:
             changeset = data['changeset']
             repos = changeset.repos
             reponame, rev = repos.reponame, repos.db_rev(changeset.rev)

[ntmlod]

Replying to Jun Omae:

I don't think it is needed to implement IPermissionRequestor in CodeReviewerModule because CODEREVIEWER_MODIFY is declared in CodeReviewerSystem.get_permission_actions().

codereviewerplugin/1.0/coderev/web_ui.py

@@ -76 +76 @@
     def post_process_request(self, req, template, data, content_type):
         diff_mode = data and 'changeset' in data and \
                     data['changeset'] is False
-        if req.path_info.startswith('/changeset') and not diff_mode:
+        if req.path_info.startswith('/changeset') and not diff_mode and \
+                'CODEREVIEWER_MODIFY' in req.perm:
             changeset = data['changeset']
             repos = changeset.repos
             reponame, rev = repos.reponame, repos.db_rev(changeset.rev)

You're right, my proposal was not much enhanced. The additional test is enough.

[Jun Omae]

In 17013:

CodeReviewerPlugin 1.0.0dev: require CODEREVIEWER_MODIFY action to review (closes #13342)

Initial patch by ntmlod.