Add toggle to turn off evaluate API (#510)

* Add toggle to turn off evaluate API * Added new param info to default.conf * Changed wording of default.conf to match server-config.md * Created separate handler for disabled evaluate api * Fixed security.md formatting * Add new setting to info handler Co-authored-by: Chinmay Gore <[email protected]> Co-authored-by: Chinmay Gore <[email protected]>
tableau · Jul 28, 2021 · bc2c020 · bc2c020
1 parent 938ce3b
commit bc2c020
Show file tree

Hide file tree

Showing 10 changed files with 155 additions and 3 deletions.
diff --git a/docs/security.md b/docs/security.md
@@ -14,3 +14,6 @@ you may want to consider the following as you use TabPy:
   - Install new Python packages which can contain binary code.
   - Execute operating system commands.
   - Open network connections to other servers and download files.
+- Execution of ad-hoc Python scripts can be disabled by turning off the
+  /evaluate endpoint. To disable /evaluate endpoint, set "TABPY_EVALUATE_ENABLE"
+  to false in config file.
diff --git a/docs/server-config.md b/docs/server-config.md
@@ -87,6 +87,8 @@ at [`logging.config` documentation page](https://docs.python.org/3.6/library/log
 - `TABPY_MAX_REQUEST_SIZE_MB` - maximal request size supported by TabPy server
   in Megabytes. All requests of exceeding size are rejected. Default value is
   100 Mb.
+- `TABPY_EVALUATE_ENABLE` - enable evaluate api to execute ad-hoc Python scripts
+  Default value - `true`.
 - `TABPY_EVALUATE_TIMEOUT` - script evaluation timeout in seconds. Default
   value - `30`. This timeout does not apply when evaluating models either
   through the `/query` method, or using the `tabpy.query(...)` syntax with
@@ -125,6 +127,10 @@ settings._
 # Default value is 100 Mb.
 # TABPY_MAX_REQUEST_SIZE_MB = 100
 
+# Enable evaluate api to execute ad-hoc Python scripts
+# Enabled by default. Disabling it will result in 404 error.
+# TABPY_EVALUATE_ENABLE = true
+
 # Configure how long a custom script provided to the /evaluate method
 # will run before throwing a TimeoutError.
 # The value should be a float representing the timeout time in seconds.

diff --git a/tabpy/tabpy_server/app/app.py b/tabpy/tabpy_server/app/app.py
@@ -18,6 +18,7 @@
     EndpointHandler,
     EndpointsHandler,
     EvaluationPlaneHandler,
+    EvaluationPlaneDisabledHandler,
     QueryPlaneHandler,
     ServiceInfoHandler,
     StatusHandler,
@@ -150,7 +151,8 @@ def try_exit(self):
                 ),
                 (
                     self.subdirectory + r"/evaluate",
-                    EvaluationPlaneHandler,
+                    EvaluationPlaneHandler if self.settings[SettingsParameters.EvaluateEnabled]
+                    else EvaluationPlaneDisabledHandler,
                     dict(executor=executor, app=self),
                 ),
                 (
@@ -259,6 +261,8 @@ def _parse_config(self, config_file):
         settings_parameters = [
             (SettingsParameters.Port, ConfigParameters.TABPY_PORT, 9004, None),
             (SettingsParameters.ServerVersion, None, __version__, None),
+            (SettingsParameters.EvaluateEnabled, ConfigParameters.TABPY_EVALUATE_ENABLE,
+             True, parser.getboolean),
             (SettingsParameters.EvaluateTimeout, ConfigParameters.TABPY_EVALUATE_TIMEOUT,
              30, parser.getfloat),
             (SettingsParameters.UploadDir, ConfigParameters.TABPY_QUERY_OBJECT_PATH,

diff --git a/tabpy/tabpy_server/app/app_parameters.py b/tabpy/tabpy_server/app/app_parameters.py
@@ -14,6 +14,7 @@ class ConfigParameters:
     TABPY_LOG_DETAILS = "TABPY_LOG_DETAILS"
     TABPY_STATIC_PATH = "TABPY_STATIC_PATH"
     TABPY_MAX_REQUEST_SIZE_MB = "TABPY_MAX_REQUEST_SIZE_MB"
+    TABPY_EVALUATE_ENABLE = "TABPY_EVALUATE_ENABLE"
     TABPY_EVALUATE_TIMEOUT = "TABPY_EVALUATE_TIMEOUT"
 
 
@@ -34,3 +35,4 @@ class SettingsParameters:
     StaticPath = "static_path"
     MaxRequestSizeInMb = "max_request_size_in_mb"
     EvaluateTimeout = "evaluate_timeout"
+    EvaluateEnabled = "evaluate_enabled"
diff --git a/tabpy/tabpy_server/common/default.conf b/tabpy/tabpy_server/common/default.conf
@@ -25,6 +25,10 @@
 # Default value is 100 Mb.
 # TABPY_MAX_REQUEST_SIZE_MB = 100
 
+# Enable evaluate api to execute ad-hoc Python scripts
+# Enabled by default. Disabling it will result in 404 error.
+# TABPY_EVALUATE_ENABLE = true
+
 # Configure how long a custom script provided to the /evaluate method
 # will run before throwing a TimeoutError.
 # The value should be a float representing the timeout time in seconds.

diff --git a/tabpy/tabpy_server/handlers/__init__.py b/tabpy/tabpy_server/handlers/__init__.py
@@ -3,6 +3,7 @@
 
 from tabpy.tabpy_server.handlers.endpoint_handler import EndpointHandler
 from tabpy.tabpy_server.handlers.endpoints_handler import EndpointsHandler
+from tabpy.tabpy_server.handlers.evaluation_plane_handler import EvaluationPlaneDisabledHandler
 from tabpy.tabpy_server.handlers.evaluation_plane_handler import EvaluationPlaneHandler
 from tabpy.tabpy_server.handlers.query_plane_handler import QueryPlaneHandler
 from tabpy.tabpy_server.handlers.service_info_handler import ServiceInfoHandler

diff --git a/tabpy/tabpy_server/handlers/evaluation_plane_handler.py b/tabpy/tabpy_server/handlers/evaluation_plane_handler.py
@@ -29,6 +29,21 @@ def query(self, name, *args, **kwargs):
         return response.json()
 
 
+class EvaluationPlaneDisabledHandler(BaseHandler):
+    """
+    EvaluationPlaneDisabledHandler responds with error message when ad-hoc scripts have been disabled.
+    """
+
+    def initialize(self, executor, app):
+        super(EvaluationPlaneDisabledHandler, self).initialize(app)
+        self.executor = executor
+
+    @gen.coroutine
+    def post(self):
+        self.error_out(404, "Ad-hoc scripts have been disabled on this analytics extension, please contact your "
+                            "administrator.")
+
+
 class EvaluationPlaneHandler(BaseHandler):
     """
     EvaluationPlaneHandler is responsible for running arbitrary python scripts.

diff --git a/tabpy/tabpy_server/handlers/service_info_handler.py b/tabpy/tabpy_server/handlers/service_info_handler.py
@@ -20,4 +20,5 @@ def get(self):
         info["server_version"] = self.settings[SettingsParameters.ServerVersion]
         info["name"] = self.tabpy_state.name
         info["versions"] = self.settings[SettingsParameters.ApiVersions]
+        info["evaluate_enabled"] = self.settings[SettingsParameters.EvaluateEnabled]
         self.write(json.dumps(info))
diff --git a/tests/unit/server_tests/test_evaluation_plane_handler.py b/tests/unit/server_tests/test_evaluation_plane_handler.py
@@ -2,10 +2,10 @@
 import os
 import tempfile
 
-from argparse import Namespace
+from tornado.testing import AsyncHTTPTestCase
+
 from tabpy.tabpy_server.app.app import TabPyApp
 from tabpy.tabpy_server.handlers.util import hash_password
-from tornado.testing import AsyncHTTPTestCase
 
 
 class TestEvaluationPlainHandlerWithAuth(AsyncHTTPTestCase):
@@ -286,3 +286,116 @@ def test_creds_no_auth_fails(self):
             },
         )
         self.assertEqual(400, response.code)
+
+
+class TestEvaluationPlainHandlerDisabled(AsyncHTTPTestCase):
+    @classmethod
+    def setUpClass(cls):
+        prefix = "__TestEvaluationPlainHandlerDisabled_"
+
+        # create config file
+        cls.config_file = tempfile.NamedTemporaryFile(
+            mode="w+t", prefix=prefix, suffix=".conf", delete=False
+        )
+        cls.config_file.write(
+            "[TabPy]\n"
+            f"TABPY_EVALUATE_ENABLE = false"
+        )
+        cls.config_file.close()
+
+        cls.script = (
+            '{"data":{"_arg1":[2,3],"_arg2":[3,-1]},'
+            '"script":"res=[]\\nfor i in range(len(_arg1)):\\n  '
+            'res.append(_arg1[i] * _arg2[i])\\nreturn res"}'
+        )
+
+    @classmethod
+    def tearDownClass(cls):
+        os.remove(cls.config_file.name)
+
+    def get_app(self):
+        self.app = TabPyApp(self.config_file.name)
+        return self.app._create_tornado_web_app()
+
+    def test_evaluation_disabled_fails(self):
+        response = self.fetch(
+            "/evaluate",
+            method="POST",
+            body=self.script
+        )
+        self.assertEqual(404, response.code)
+
+
+class TestEvaluationPlainHandlerEnabled(AsyncHTTPTestCase):
+    @classmethod
+    def setUpClass(cls):
+        prefix = "__TestEvaluationPlainHandlerEnabled_"
+
+        # create config file
+        cls.config_file = tempfile.NamedTemporaryFile(
+            mode="w+t", prefix=prefix, suffix=".conf", delete=False
+        )
+        cls.config_file.write(
+            "[TabPy]\n"
+            f"TABPY_EVALUATE_ENABLE = true"
+        )
+        cls.config_file.close()
+
+        cls.script = (
+            '{"data":{"_arg1":[2,3],"_arg2":[3,-1]},'
+            '"script":"res=[]\\nfor i in range(len(_arg1)):\\n  '
+            'res.append(_arg1[i] * _arg2[i])\\nreturn res"}'
+        )
+
+    @classmethod
+    def tearDownClass(cls):
+        os.remove(cls.config_file.name)
+
+    def get_app(self):
+        self.app = TabPyApp(self.config_file.name)
+        return self.app._create_tornado_web_app()
+
+    def test_evaluation_enabled(self):
+        response = self.fetch(
+            "/evaluate",
+            method="POST",
+            body=self.script
+        )
+        self.assertEqual(200, response.code)
+
+
+class TestEvaluationPlainHandlerDefault(AsyncHTTPTestCase):
+    @classmethod
+    def setUpClass(cls):
+        prefix = "__TestEvaluationPlainHandlerDefault_"
+
+        # create config file
+        cls.config_file = tempfile.NamedTemporaryFile(
+            mode="w+t", prefix=prefix, suffix=".conf", delete=False
+        )
+        cls.config_file.write(
+            "[TabPy]"
+        )
+        cls.config_file.close()
+
+        cls.script = (
+            '{"data":{"_arg1":[2,3],"_arg2":[3,-1]},'
+            '"script":"res=[]\\nfor i in range(len(_arg1)):\\n  '
+            'res.append(_arg1[i] * _arg2[i])\\nreturn res"}'
+        )
+
+    @classmethod
+    def tearDownClass(cls):
+        os.remove(cls.config_file.name)
+
+    def get_app(self):
+        self.app = TabPyApp(self.config_file.name)
+        return self.app._create_tornado_web_app()
+
+    def test_evaluation_default(self):
+        response = self.fetch(
+            "/evaluate",
+            method="POST",
+            body=self.script
+        )
+        self.assertEqual(200, response.code)
diff --git a/tests/unit/server_tests/test_service_info_handler.py b/tests/unit/server_tests/test_service_info_handler.py
@@ -15,6 +15,7 @@ def _create_expected_info_response(settings, tabpy_state):
         "server_version": settings[SettingsParameters.ServerVersion],
         "name": tabpy_state.name,
         "versions": settings["versions"],
+        "evaluate_enabled": settings["evaluate_enabled"]
     }
 
 
@@ -103,6 +104,7 @@ def test_given_server_with_auth_expect_correct_info_response(self):
             {"authentication": {"methods": {"basic-auth": {}}, "required": True}},
             features,
         )
+        self.assertTrue(actual_response['evaluate_enabled'])
 
 
 class TestServiceInfoHandlerWithoutAuth(BaseTestServiceInfoHandler):
@@ -127,6 +129,7 @@ def test_server_with_no_auth_expect_correct_info_response(self):
         self.assertTrue("features" in v1)
         features = v1["features"]
         self.assertDictEqual({}, features)
+        self.assertTrue(actual_response['evaluate_enabled'])
 
     def test_given_server_with_no_auth_and_password_expect_correct_info_response(self):
         header = {