wip jupyter-server#957: refactored ENV_WHITELIST -> ENV_FORWARD; auto…

…matically determine ENV_FORWARD from kernel env stanza

docs/source/config-options.md

@@ -22,7 +22,7 @@ To see the same configuration options at the command line, run the following:
 jupyter enterprisegateway --help-all
 ```
 
-A snapshot of this help appears below for ease of reference on the web.  
+A snapshot of this help appears below for ease of reference on the web.
 
 ```
 Jupyter Enterprise Gateway
@@ -142,14 +142,14 @@ EnterpriseGatewayApp(EnterpriseGatewayConfigMixin, JupyterApp) options
     A value of 0 or less disables dynamic config updates.
     (EG_DYNAMIC_CONFIG_INTERVAL env var)
     Default: 0
---EnterpriseGatewayApp.env_process_whitelist=<list-item-1>...
+--EnterpriseGatewayApp.kernel_env_inherit=<list-item-1>...
     Environment variables allowed to be inherited from the spawning process by
-    the kernel. (EG_ENV_PROCESS_WHITELIST env var)
+    the kernel. (EG_KERNEL_ENV_INHERIT env var)
     Default: []
---EnterpriseGatewayApp.env_whitelist=<list-item-1>...
+--EnterpriseGatewayApp.kernel_env_forward=<list-item-1>...
     Environment variables allowed to be set when a client requests a new kernel.
     Use '*' to allow all environment variables sent in the request.
-    (EG_ENV_WHITELIST env var)
+    (EG_KERNEL_ENV_FORWARD env var)
     Default: []
 --EnterpriseGatewayApp.expose_headers=<Unicode>
     Sets the Access-Control-Expose-Headers header. (EG_EXPOSE_HEADERS env var)
@@ -433,10 +433,10 @@ The following environment variables can be used to influence functionality and a
     in an exception indicating error 403 (Forbidden).  See also EG_PROHIBITED_GIDS.
 
   EG_RESPONSE_IP=None
-    Experimental.  The IP address to use to formulate the response address (with 
-    `EG_RESPONSE_PORT`).  By default, the server's IP is used.  However, we may find 
+    Experimental.  The IP address to use to formulate the response address (with
+    `EG_RESPONSE_PORT`).  By default, the server's IP is used.  However, we may find
     it necessary to use a different IP in cases where the target kernels are external
-    to the Enterprise Gateway server (for example).  It's value may also need to be 
+    to the Enterprise Gateway server (for example).  It's value may also need to be
     set in cases where the computed (default) is not correct for the current topology.
 
   EG_RESPONSE_PORT=8877
@@ -491,7 +491,7 @@ The following environment variables may be useful for troubleshooting:
     should rarely be necessary.
 
   EG_POLL_INTERVAL=0.5
-    The interval (in seconds) to wait before checking poll results again.  
+    The interval (in seconds) to wait before checking poll results again.
 
   EG_REMOVE_CONTAINER=True
     Used by launch_docker.py, indicates whether the kernel's docker container should be

enterprise_gateway/enterprisegatewayapp.py

@@ -147,23 +147,25 @@ def _create_request_handlers(self):
         """Create default Jupyter handlers and redefine them off of the
         base_url path. Assumes init_configurables() has already been called.
         """
-        handlers = []
+        default_auth_handlers = [
+            (r"/login", self.auth_handler_class),
+        ]
+
+        handlers_collection = (
+            default_api_handlers,
+            default_auth_handlers,
+            default_kernel_handlers,
+            default_kernelspec_handlers,
+            default_session_handlers,
+            default_base_handlers,
+        )
 
         # append tuples for the standard kernel gateway endpoints
-        for handler in (
-            default_api_handlers +
-            default_kernel_handlers +
-            default_kernelspec_handlers +
-            default_session_handlers +
-            default_base_handlers
-        ):
-            # Create a new handler pattern rooted at the base_url
-            pattern = url_path_join('/', self.base_url, handler[0])
-            # Some handlers take args, so retain those in addition to the
-            # handler class ref
-            new_handler = tuple([pattern] + list(handler[1:]))
-            handlers.append(new_handler)
-        return handlers
+        return [(
+            url_path_join('/', self.base_url, handler[0]),
+            *handler[1:],
+        ) for handlers in handlers_collection
+        for handler in handlers]
 
     def init_webapp(self):
         """Initializes Tornado web application with uri handlers.
@@ -194,8 +196,8 @@ def init_webapp(self):
             eg_expose_headers=self.expose_headers,
             eg_max_age=self.max_age,
             eg_max_kernels=self.max_kernels,
-            eg_env_process_whitelist=self.env_process_whitelist,
-            eg_env_whitelist=self.env_whitelist,
+            # eg_kernel_env_forward=self.kernel_env_forward,
+            eg_kernel_env_inherit=self.kernel_env_inherit,
             eg_kernel_headers=self.kernel_headers,
             eg_list_kernels=self.list_kernels,
             eg_authorized_users=self.authorized_users,
@@ -205,6 +207,7 @@ def init_webapp(self):
             allow_origin=self.allow_origin,
             # Always allow remote access (has been limited to localhost >= notebook 5.6)
             allow_remote_access=True,
+            login_url=url_path_join('/', self.base_url, '/login'),
             # setting ws_ping_interval value that can allow it to be modified for the purpose of toggling ping mechanism
             # for zmq web-sockets or increasing/decreasing web socket ping interval/timeouts.
             ws_ping_interval=self.ws_ping_interval * 1000

enterprise_gateway/mixins.py

@@ -139,6 +139,24 @@ def write_error(self, status_code, **kwargs):
         self.finish(json.dumps(reply))
 
 
+class KernelEnvMixin:
+    # @property
+    # def kernel_env_forward(self):
+    #     return self.settings['eg_kernel_env_forward']
+
+    @property
+    def kernel_env_keys(self):
+        km = self.settings['kernel_manager']
+        if km.kernel_spec is not None and "env" in km.kernel_spec:
+            return tuple(km.kernel_spec['env'].keys())
+        else:
+            return None
+
+    @property
+    def kernel_env_inherit(self):
+        return self.settings['eg_kernel_env_inherit']
+
+
 class EnterpriseGatewayConfigMixin(Configurable):
     # Server IP / PORT binding
     port_env = 'EG_PORT'
@@ -313,24 +331,24 @@ def default_kernel_name_default(self):
     def list_kernels_default(self):
         return os.getenv(self.list_kernels_env, os.getenv('KG_LIST_KERNELS', 'False')).lower() == 'true'
 
-    env_whitelist_env = 'EG_ENV_WHITELIST'
-    env_whitelist = List(config=True,
-                         help="""Environment variables allowed to be set when a client requests a
-                         new kernel. Use '*' to allow all environment variables sent in the request.
-                         (EG_ENV_WHITELIST env var)""")
+    # kernel_env_forward_env = 'EG_KERNEL_ENV_FORWARD'
+    # kernel_env_forward = List(config=True,
+    #                      help="""Environment variables allowed to be set when a client requests a
+    #                      new kernel. Use '*' to allow all environment variables sent in the request.
+    #                      (EG_KERNEL_ENV_FORWARD env var)""")
 
-    @default('env_whitelist')
-    def env_whitelist_default(self):
-        return os.getenv(self.env_whitelist_env, os.getenv('KG_ENV_WHITELIST', '')).split(',')
+    # @default('kernel_env_forward')
+    # def kernel_env_forward_default(self):
+    #     return os.getenv(self.kernel_env_forward_env, os.getenv('KG_KERNEL_ENV_FORWARD', '')).split(',')
 
-    env_process_whitelist_env = 'EG_ENV_PROCESS_WHITELIST'
-    env_process_whitelist = List(config=True,
+    kernel_env_inherit_env = 'EG_KERNEL_ENV_INHERIT'
+    kernel_env_inherit = List(config=True,
                                  help="""Environment variables allowed to be inherited
-                                 from the spawning process by the kernel. (EG_ENV_PROCESS_WHITELIST env var)""")
+                                 from the spawning process by the kernel. (EG_KERNEL_ENV_INHERIT env var)""")
 
-    @default('env_process_whitelist')
-    def env_process_whitelist_default(self):
-        return os.getenv(self.env_process_whitelist_env, os.getenv('KG_ENV_PROCESS_WHITELIST', '')).split(',')
+    @default('kernel_env_inherit')
+    def kernel_env_inherit_default(self):
+        return os.getenv(self.kernel_env_inherit_env, os.getenv('KG_KERNEL_ENV_INHERIT', '')).split(',')
 
     kernel_headers_env = 'EG_KERNEL_HEADERS'
     kernel_headers = List(config=True,

enterprise_gateway/services/api/swagger.json

@@ -107,13 +107,15 @@
         "tags": [
           "kernelspecs"
         ],
-        "parameters": {
+        "parameters": [
+          {
             "name": "user",
             "required": false,
             "in": "query",
             "description": "When present, kernelspec results will be filtered based on the configured authorization of specified value.",
             "type": "string"
-        },
+          }
+        ],
         "responses": {
           "200": {
             "description": "If no query parameter is specified, all kernel specs will be returned; otherwise the result set is filtered based on the query parameter.",

enterprise_gateway/services/kernels/handlers.py

@@ -9,24 +9,19 @@
 from jupyter_client.jsonutil import date_default
 from tornado import web
 from functools import partial
-from ...mixins import TokenAuthorizationMixin, CORSMixin, JSONErrorsMixin
+from ...mixins import CORSMixin, JSONErrorsMixin, KernelEnvMixin, TokenAuthorizationMixin
 
 
 class MainKernelHandler(TokenAuthorizationMixin,
                         CORSMixin,
                         JSONErrorsMixin,
+                        KernelEnvMixin,
                         jupyter_server_handlers.MainKernelHandler):
     """Extends the jupyter_server main kernel handler with token auth, CORS, and
     JSON errors.
     """
 
-    @property
-    def env_whitelist(self):
-        return self.settings['eg_env_whitelist']
 
-    @property
-    def env_process_whitelist(self):
-        return self.settings['eg_env_process_whitelist']
 
     async def post(self):
         """Overrides the super class method to manage env in the request body.
@@ -50,19 +45,15 @@ async def post(self):
         if model is not None and 'env' in model:
             if not isinstance(model['env'], dict):
                 raise tornado.web.HTTPError(400)
-            # Start with the PATH from the current env. Do not provide the entire environment
-            # which might contain server secrets that should not be passed to kernels.
-            env = {'PATH': os.getenv('PATH', '')}
-            # Whitelist environment variables from current process environment
-            env.update({key: value for key, value in os.environ.items()
-                        if key in self.env_process_whitelist})
-            # Whitelist KERNEL_* args and those allowed by configuration from client.  If all
-            # envs are requested, just use the keys from the payload.
-            env_whitelist = self.env_whitelist
-            if env_whitelist == ['*']:
-                env_whitelist = model['env'].keys()
-            env.update({key: value for key, value in model['env'].items()
-                        if key.startswith('KERNEL_') or key in env_whitelist})
+
+            env = {
+                # always inherit PATH from the gateway env
+                **{'PATH': os.getenv('PATH', '')},
+                # inherit vars from the gateway env if var name in self.env_inherit
+                **{k:v for k, v in os.environ.items() if k in self.kernel_env_inherit},
+                # forward vars from request env if var name starts with KERNEL_ or in self.env_forward
+                **{k:v for k, v in model['env'].items() if k.startswith('KERNEL_') or k in self.kernel_env_keys},
+            }
 
             # If kernel_headers are configured, fetch each of those and include in start request
             kernel_headers = {}

enterprise_gateway/services/kernels/remotemanager.py

@@ -16,7 +16,7 @@
 
 from ..processproxies.processproxy import LocalProcessProxy, RemoteProcessProxy
 from ..sessions.kernelsessionmanager import KernelSessionManager
-from enterprise_gateway.mixins import EnterpriseGatewayConfigMixin
+from ...mixins import EnterpriseGatewayConfigMixin, KernelEnvMixin
 
 
 def get_process_proxy_config(kernelspec):
@@ -284,7 +284,7 @@ def new_kernel_id(self, **kwargs):
         return new_kernel_id(kernel_id_fn=super(RemoteMappingKernelManager, self).new_kernel_id, log=self.log, **kwargs)
 
 
-class RemoteKernelManager(EnterpriseGatewayConfigMixin, AsyncIOLoopKernelManager):
+class RemoteKernelManager(EnterpriseGatewayConfigMixin, KernelEnvMixin, AsyncIOLoopKernelManager):
     """
     Extends the AsyncIOLoopKernelManager used by the RemoteMappingKernelManager.
 
@@ -338,8 +338,8 @@ def _link_dependent_props(self):
                            "port_range",
                            "impersonation_enabled",
                            "max_kernels_per_user",
-                           "env_whitelist",
-                           "env_process_whitelist",
+                           #"kernel_env_forward",
+                           "kernel_env_inherit",
                            "yarn_endpoint",
                            "alt_yarn_endpoint",
                            "yarn_endpoint_security_enabled",
@@ -366,15 +366,15 @@ async def start_kernel(self, **kwargs):
 
     def _capture_user_overrides(self, **kwargs):
         """
-       Make a copy of any whitelist or KERNEL_ env values provided by user.  These will be injected
-       back into the env after the kernelspec env has been applied.  This enables defaulting behavior
-       of the kernelspec env stanza that would have otherwise overridden the user-provided values.
+        Make a copy of any whitelist or KERNEL_ env values provided by user.  These will be injected
+        back into the env after the kernelspec env has been applied.  This enables defaulting behavior
+        of the kernelspec env stanza that would have otherwise overridden the user-provided values.
         """
         env = kwargs.get('env', {})
-        self.user_overrides.update({key: value for key, value in env.items()
-                                    if key.startswith('KERNEL_') or
-                                    key in self.env_process_whitelist or
-                                    key in self.env_whitelist})
+        self.user_overrides.update({k:v for k, v in env.items()
+                                    if k.startswith('KERNEL_') or
+                                    k in self.kernel_env_inherit or
+                                    k in self.kernel_env_keys})
 
     def format_kernel_cmd(self, extra_arguments=None):
         """

enterprise_gateway/tests/test_handlers.py

@@ -24,19 +24,19 @@ def setup_app(self):
         os.environ['JUPYTER_PATH'] = RESOURCES
 
         # These are required for setup of test_kernel_defaults
-        os.environ['EG_ENV_PROCESS_WHITELIST'] = "PROCESS_VAR1,PROCESS_VAR2"
+        os.environ['EG_KERNEL_ENV_INHERIT'] = "PROCESS_VAR1,PROCESS_VAR2"
         os.environ['PROCESS_VAR1'] = "process_var1_override"
 
-        self.app.env_whitelist = ['TEST_VAR', 'OTHER_VAR1', 'OTHER_VAR2']
+        # self.app.kernel_env_forward = ['TEST_VAR', 'OTHER_VAR1', 'OTHER_VAR2']
 
     def tearDown(self):
         """Shuts down the app after test run."""
 
         # Clean out items added to env
         if 'JUPYTER_PATH' in os.environ:
             os.environ.pop('JUPYTER_PATH')
-        if 'EG_ENV_PROCESS_WHITELIST' in os.environ:
-            os.environ.pop('EG_ENV_PROCESS_WHITELIST')
+        if 'EG_KERNEL_ENV_INHERIT' in os.environ:
+            os.environ.pop('EG_KERNEL_ENV_INHERIT')
         if 'PROCESS_VAR1' in os.environ:
             os.environ.pop('PROCESS_VAR1')
 
@@ -501,8 +501,7 @@ def test_json_errors(self):
     @gen_test
     def test_kernel_env(self):
         """Kernel should start with environment vars defined in the request."""
-        # Note: Only envs in request prefixed with KERNEL_ or in env_whitelist (TEST_VAR)
-        # with the exception of KERNEL_GATEWAY - which is "system owned".
+        # env vars are forwarded from request if key starts with KERNEL_ or key in kernel env stanza
         kernel_body = json.dumps({
             'name': 'python',
             'env': {
@@ -530,11 +529,8 @@ def test_kernel_env(self):
 
     @gen_test
     def test_kernel_defaults(self):
-        """Kernel should start with env vars defined in request overriding env vars defined in kernelspec."""
-
-        # Note: Only envs in request prefixed with KERNEL_ or in env_whitelist (OTHER_VAR1, OTHER_VAR2)
-        # with the exception of KERNEL_GATEWAY - which is "system owned" - will be set in kernel env.
-        # Since OTHER_VAR1 is not in the request, its existing value in kernel.json will be used.
+        """Kernel should start with env ars defined in request overriding env vars defined in kernelspec."""
+        # env vars are forwarded from request if key starts with KERNEL_ or key in kernel env stanza
 
         # NOTE: This test requires use of the kernels/kernel_defaults_test/kernel.json file.
         kernel_body = json.dumps({
@@ -686,13 +682,10 @@ def setup_app(self):
         """Configure JUPYTER_PATH so that we can use local kernelspec files for testing.
         """
         super().setup_app()
-        # overwrite env_whitelist
-        self.app.env_whitelist = ['*']
 
     @gen_test
     def test_kernel_wildcard_env(self):
         """Kernel should start with environment vars defined in the request."""
-        # Note: Since env_whitelist == '*', all values should be present.
         kernel_body = json.dumps({
             'name': 'python',
             'env': {