add permissions to control user of rest and xmlrpc API interfaces.

rouilj · rouilj · commit 8bbf8caa79b9 · 2019-09-27T23:29:59.000-04:00
issue2551058: Add new permissions: 'Rest Access' and 'Xmlrpc Access'
to allow per-user access control to rest and xmlrpc interfaces using
roles.

Updated all schemas to add these new perms to all authenticated roles.

Error conditions in handle_xmlrpc were not working right in manual
testing. I tried to make it a little better, but I don't actually
understand how the fault xmlrpc object is supposed to be used. So I
may have messed something up. I'll try to ping the people who wrote
the xmlrpc code to have them review.
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -91,7 +91,9 @@ Features:
 - issue2551061: Add rudimentary experimental support for JSON Web
   Tokens to allow delegation of limited access rights to third
   parties. See doc/rest.txt for details and intent. (John Rouillard)
- 
+- issue2551058: Add new permissions: 'Rest Access' and 'Xmlrpc Access'
+  to allow per-user access control to rest and xmlrpc interfaces using
+  roles.
 
 Fixed:
 
diff --git a/doc/rest.txt b/doc/rest.txt
@@ -12,8 +12,8 @@ Introduction
 
 After the last 1.6.0 Release, a REST-API developed in 2015 during a
 Google Summer of Code (GSOC) by Chau Nguyen, supervised by Ezio
-Melotti was integrated. The code was updated by John Rouillard and
-Ralf Schlatterbeck to fix some shortcomings and provide the necessary
+Melotti was integrated. The code was updated by Ralf Schlatterbeck
+and John Rouillard to fix some shortcomings and provide the necessary
 functions for a single page web application, e.g. etag support,
 pagination, field embedding among others.
 
@@ -23,6 +23,15 @@ Enabling the REST API
 The REST API can be disabled in the ``[web]`` section of ``config.ini``
 via the variable ``enable_rest`` which is ``yes`` by default.
 
+Users have to be authorized to use the rest api. The user must have
+"Rest Access" permission. To add this to the "User" role change
+schema.py to add::
+
+    db.security.addPermissionToRole('User', 'Rest Access')
+
+This is usually included near where other permissions like "Web Access"
+or "Email Access" are assigned.
+
 The REST api is reached via the ``/rest/`` endpoint of the tracker
 URL. Partial URLs paths below (not starting with https) will have
 /rest removed for brevity.
@@ -1386,6 +1395,7 @@ proper authorization::
             properties=('id', 'times'),
             description="Allow editing timelog for issue", props_only=False)
    db.security.addPermissionToRole("User:timelog", perm)
+   db.security.addPermissionToRole('User:timelog', 'Rest Access')
 
 Then role is named to work with the jwt issue rest call. Starting the role
 name with ``User:`` allows the jwt issue code to create a token with
diff --git a/doc/upgrading.txt b/doc/upgrading.txt
@@ -50,6 +50,20 @@ https://pypi.org/project/MySQL-python/ is still supported, it is
 recommended to switch to the updated module from
 https://pypi.org/project/mysqlclient/.
 
+XMLRPC Access Role
+------------------
+
+A new permission has been added to control access to the XMLRPC
+endpoint. If the user doesn't have the new "Xmlrpc Access" permission,
+they will not be able to log in using the /xmlrpc end point.  To add
+this new permission to the "User" role you should change your
+tracker's schema.py and add::
+
+    db.security.addPermissionToRole('User', 'Xmlrpc Access')
+
+This is usually included near where other permissions like "Web Access"
+or "Email Access" are assigned.
+
 Python 3 support
 ----------------
 
diff --git a/doc/xmlrpc.txt b/doc/xmlrpc.txt
@@ -23,12 +23,33 @@ Enabling XML-RPC server
 -----------------------
 There are two ways to run the XML-RPC interface:
 
+  through roundup itself
+
   stand alone roundup-xmlrpc-server
 
-  through roundup itself
+
+through roundup
+---------------
+The XML-RPC service is available from the roundup HTTP server under
+/xmlrpc.
+
+To enable this set ``enable_xmlrpc`` to ``yes`` in the ``[web]``
+section of the ``config.ini`` file in your tracker.
+
+Each user that needs access must include the "Xmlrpc Access" role. To
+add this new permission to the "User" role you should change your
+schema.py to add::
+
+    db.security.addPermissionToRole('User', 'Xmlrpc Access')
+
+This is usually included near where other permissions like "Web Access"
+or "Email Access" are assigned.
 
 stand alone roundup-xmlrpc-server
 ---------------------------------
+Using roundup to access the xmlrpc interface is preferred. Roundup
+provides better control over who can use the interface.
+
 The Roundup XML-RPC standalone server must be started before remote clients can access the
 tracker via XML-RPC. ``roundup-xmlrpc-server`` is installed in the scripts
 directory alongside ``roundup-server`` and roundup-admin``. When invoked, the
@@ -39,11 +60,6 @@ location of the tracker instance must be specified.
 The default port is ``8000``. An alternative port can be specified with the
 ``--port`` switch.
 
-through roundup
----------------
-In addition to running a stand alone server described above, the
-XML-RPC service is available from the roundup HTTP server.
-
 security consideration
 ----------------------
 Note that the current ``roundup-xmlrpc-server`` implementation does not
diff --git a/roundup/cgi/client.py b/roundup/cgi/client.py
@@ -62,6 +62,14 @@ def initialiseSecurity(security):
         description="User may access the web interface")
     security.addPermissionToRole('Admin', p)
 
+    p = security.addPermission(name="Rest Access",
+        description="User may access the rest interface")
+    security.addPermissionToRole('Admin', p)
+
+    p = security.addPermission(name="Xmlrpc Access",
+        description="User may access the xmlrpc interface")
+    security.addPermissionToRole('Admin', p)
+
     # doing Role stuff through the web - make sure Admin can
     # TODO: deprecate this and use a property-based control
     p = security.addPermission(name="Web Roles",
@@ -497,9 +505,22 @@ def handle_xmlrpc(self):
             self.determine_user()
         except LoginError as msg:
             output = xmlrpc_.client.dumps(
-                xmlrpc_.client.Fault(1, "%s:%s" % (exc_type, exc_value)),
+                xmlrpc_.client.Fault(401, "%s" % msg),
                 allow_none=True)
+            self.setHeader("Content-Type", "text/xml")
+            self.setHeader("Content-Length", str(len(output)))
+            self.write(s2b(output))
+            return
 
+        if not self.db.security.hasPermission('Xmlrpc Access', self.userid):
+            output = xmlrpc_.client.dumps(
+                xmlrpc_.client.Fault(403, "Forbidden"),
+                allow_none=True)
+            self.setHeader("Content-Type", "text/xml")
+            self.setHeader("Content-Length", str(len(output)))
+            self.write(s2b(output))
+            return
+        
         self.check_anonymous_access()
 
         try:
@@ -544,6 +565,11 @@ def handle_rest(self):
             self.write(output)
             return
 
+        if not self.db.security.hasPermission('Rest Access', self.userid):
+            self.response_code = 403
+            self.write(s2b('{ "error": { "status": 403, "msg": "Forbidden." } }'))
+            return
+        
         self.check_anonymous_access()
 
         try:
diff --git a/share/roundup/templates/classic/schema.py b/share/roundup/templates/classic/schema.py
@@ -89,6 +89,8 @@
 # Give the regular users access to the web and email interface
 db.security.addPermissionToRole('User', 'Web Access')
 db.security.addPermissionToRole('User', 'Email Access')
+db.security.addPermissionToRole('User', 'Rest Access')
+db.security.addPermissionToRole('User', 'Xmlrpc Access')
 
 # Assign the access and edit Permissions for issue, file and message
 # to regular users now
diff --git a/share/roundup/templates/devel/schema.py b/share/roundup/templates/devel/schema.py
@@ -205,7 +205,9 @@
 for r in 'User', 'Developer', 'Coordinator':
     db.security.addPermissionToRole(r, 'Web Access')
     db.security.addPermissionToRole(r, 'Email Access')
-
+    db.security.addPermissionToRole(r, 'Rest Access')
+    db.security.addPermissionToRole(r, 'Xmlrpc Access')
+    
 ##########################
 # User permissions
 ##########################
diff --git a/share/roundup/templates/jinja2/schema.py b/share/roundup/templates/jinja2/schema.py
@@ -89,6 +89,8 @@
 # Give the regular users access to the web and email interface
 db.security.addPermissionToRole('User', 'Web Access')
 db.security.addPermissionToRole('User', 'Email Access')
+db.security.addPermissionToRole('User', 'Rest Access')
+db.security.addPermissionToRole('User', 'Xmlrpc Access')
 
 # Assign the access and edit Permissions for issue, file and message
 # to regular users now
diff --git a/share/roundup/templates/minimal/schema.py b/share/roundup/templates/minimal/schema.py
@@ -29,6 +29,8 @@
 # Give the regular users access to the web and email interface
 db.security.addPermissionToRole('User', 'Web Access')
 db.security.addPermissionToRole('User', 'Email Access')
+db.security.addPermissionToRole('User', 'Rest Access')
+db.security.addPermissionToRole('User', 'Xmlrpc Access')
 
 # May users view other user information?
 # Comment these lines out if you don't want them to
diff --git a/share/roundup/templates/responsive/schema.py b/share/roundup/templates/responsive/schema.py
@@ -204,7 +204,10 @@
 for r in 'User', 'Developer', 'Coordinator':
     db.security.addPermissionToRole(r, 'Web Access')
     db.security.addPermissionToRole(r, 'Email Access')
+    db.security.addPermissionToRole(r, 'Rest Access')
+    db.security.addPermissionToRole(r, 'Xmlrpc Access')
 
+    
 ##########################
 # User permissions
 ##########################
diff --git a/test/rest_common.py b/test/rest_common.py
@@ -116,6 +116,19 @@ def setUp(self):
                        description="Allow jwt access to email",
                        props_only=False)
         self.db.security.addPermissionToRole("User:email", jwt_perms)
+        self.db.security.addPermissionToRole("User:email", "Rest Access")
+
+        # add set of roles for testing jwt's.
+        # this is like the user:email role, but it missing access to the rest endpoint.
+        self.db.security.addRole(name="User:emailnorest",
+                        description="allow email by jwt")
+        jwt_perms = self.db.security.addPermission(name='View',
+                       klass='user',
+                       properties=('id', 'realname', 'address', 'username'),
+                       description="Allow jwt access to email but forget to allow rest",
+                       props_only=False)
+        self.db.security.addPermissionToRole("User:emailnorest", jwt_perms)
+
 
         if jwt:
             # must be 32 chars in length minimum (I think this is at least
@@ -188,6 +201,12 @@ def setUp(self):
             self.jwt['user:email'] = b2s(jwt.encode(self.claim['user:email'], secret,
                                           algorithm='HS256'))
 
+            # generate valid claim with limited user:emailnorest role
+            self.claim['user:emailnorest'] = copy(claim)
+            self.claim['user:emailnorest']['roles'] = [ "user:emailnorest" ]
+            self.jwt['user:emailnorest'] = b2s(jwt.encode(self.claim['user:emailnorest'], secret,
+                                          algorithm='HS256'))
+
         self.db.tx_Source = 'web'
 
         self.db.issue.addprop(tx_Source=hyperdb.String())
@@ -3013,6 +3032,7 @@ def wh(s):
 
     @skip_jwt
     def test_user_email_jwt(self):
+        '''tests "Rest Access" permission present case'''
         # self.dummy_client.main() closes database, so
         # we need a new test with setup called for each test
         out = []
@@ -3078,6 +3098,69 @@ def wh(s):
         self.assertTrue('address' in json_dict['data']['collection'][1])
         self.assertTrue('address' in json_dict['data']['collection'][2])
 
+    @skip_jwt
+    def test_user_emailnorest_jwt(self):
+        '''tests "Rest Access" permission missing case'''
+        # self.dummy_client.main() closes database, so
+        # we need a new test with setup called for each test
+        out = []
+        def wh(s):
+            out.append(s)
+
+        secret = self.db.config.WEB_JWT_SECRET
+
+        # verify library and tokens are correct
+        self.assertRaises(jwt.exceptions.InvalidTokenError,
+                          jwt.decode, self.jwt['expired'],
+                          secret,  algorithms=['HS256'],
+                          audience=self.db.config.TRACKER_WEB,
+                          issuer=self.db.config.TRACKER_WEB)
+
+        result = jwt.decode(self.jwt['user'],
+                            secret,  algorithms=['HS256'],
+                            audience=self.db.config.TRACKER_WEB,
+                            issuer=self.db.config.TRACKER_WEB)
+        self.assertEqual(self.claim['user'],result)
+
+        result = jwt.decode(self.jwt['user:email'],
+                            secret,  algorithms=['HS256'],
+                            audience=self.db.config.TRACKER_WEB,
+                            issuer=self.db.config.TRACKER_WEB)
+        self.assertEqual(self.claim['user:email'],result)
+
+        # set environment for all jwt tests
+        env = {
+            'PATH_INFO': 'rest/data/user',
+            'HTTP_HOST': 'localhost',
+            'TRACKER_NAME': 'rounduptest',
+            "REQUEST_METHOD": "GET"
+        }
+        self.dummy_client = client.Client(self.instance, MockNull(), env,
+                                          [], None)
+        self.dummy_client.db = self.db
+        self.dummy_client.request.headers.get = self.get_header
+        self.empty_form = cgi.FieldStorage()
+        self.terse_form = cgi.FieldStorage()
+        self.terse_form.list = [
+            cgi.MiniFieldStorage('@verbose', '0'),
+        ]
+        self.dummy_client.form = cgi.FieldStorage()
+        self.dummy_client.form.list = [
+            cgi.MiniFieldStorage('@fields', 'username,address'),
+        ]
+        # accumulate json output for further analysis
+        self.dummy_client.write = wh
+
+        # set up for limited user:email role token
+        env['HTTP_AUTHORIZATION'] = 'bearer %s'%self.jwt['user:emailnorest']
+        self.dummy_client.main()
+        json_dict = json.loads(b2s(out[0]))
+        # user will be joe id 3 as auth works
+        self.assertTrue('1', self.db.getuid())
+        { "error": { "status": 403, "msg": "Forbidden." } }
+        self.assertTrue('error' in json_dict)
+        self.assertTrue(json_dict['error']['status'], 403)
+        self.assertTrue(json_dict['error']['msg'], "Forbidden.")
 
     @skip_jwt
     def test_disabled_jwt(self):
