Add basic authentication in the server (#1670)

* Add basic authentication in the server

A pattern for creating authentication methods is introduced, with three
authentication types defined:
  none - no authentication
  session - like the current session based auth used for user annotations
  test - used to test the login/logout process end to end

The config endpoint now returns informations about the authentication, like if
the user is authenticated and their username.  The redirect uri's for login and
logout are also returned if the authentication type requires login

This is the first a several PRs for authentication.

*. Update server tests to avoid hardcoded ports

test_api and test_nan_rest now use a common function for starting a test server,
than will initially choose a random port.

server/test/__init__.py

@@ -86,15 +86,14 @@ def random_string(n):
     return "".join(random.choice(string.ascii_letters) for _ in range(n))
 
 
-@contextmanager
-def test_server(command_line_args=[], app_config=None):
-    """A context to run the cellxgene server.
+def start_test_server(command_line_args=[], app_config=None):
+    """
     Command line arguments can be passed in, as well as an app_config.
     This function is meant to be used like this, for example:
 
     with test_server(...) as server:
-       r = requests.get(f"{server}/...")
-       // check r
+        r = requests.get(f"{server}/...")
+        // check r
 
     where the server can be accessed within the context, and is terminated when
     the context is exited.
@@ -104,7 +103,8 @@ def test_server(command_line_args=[], app_config=None):
     yaml config file, which this server will read and parse.
     """
 
-    port = int(os.environ.get("CXG_SERVER_PORT", DEFAULT_SERVER_PORT))
+    start = random.randint(DEFAULT_SERVER_PORT, 2**16 - 1)
+    port = int(os.environ.get("CXG_SERVER_PORT", start))
     port = find_available_port("localhost", port)
     command = ["cellxgene", "--no-upgrade-check", "launch", "--verbose", "--port=%d" % port] + command_line_args
 
@@ -128,10 +128,25 @@ def test_server(command_line_args=[], app_config=None):
     if tempdir:
         tempdir.cleanup()
 
+    return ps, server
+
+
+def stop_test_server(ps):
+    try:
+        ps.terminate()
+    except ProcessLookupError:
+        pass
+
+
+@contextmanager
+def test_server(command_line_args=[], app_config=None):
+    """A context to run the cellxgene server."""
+
+    ps, server = start_test_server(command_line_args, app_config)
     try:
         yield server
     finally:
         try:
-            ps.terminate()
+            stop_test_server(ps)
         except ProcessLookupError:
             pass

server/test/test_api.py

@@ -2,7 +2,6 @@ import shutil
 import time
 import unittest
 from http import HTTPStatus
-from subprocess import Popen
 
 import pandas as pd
 import requests
@@ -11,6 +10,7 @@ import server.test.decode_fbs as decode_fbs
 from server.data_common.matrix_loader import MatrixDataType
 from server.test import data_with_tmp_annotations, make_fbs, PROJECT_ROOT
 from server.test.test_datasets.fixtures import pbmc3k_colors
+from server.test import start_test_server, stop_test_server
 
 
 BAD_FILTER = {"filter": {"obs": {"annotation_value": [{"name": "xyz"}]}}}
@@ -21,9 +21,6 @@ BAD_FILTER = {"filter": {"obs": {"annotation_value": [{"name": "xyz"}]}}}
 class EndPoints(object):
     ANNOTATIONS_ENABLED = True
 
-    def setUp(self):
-        self.session = requests.Session()
-
     def test_initialize(self):
         endpoint = "schema"
         url = f"{self.URL_BASE}{endpoint}"
@@ -308,13 +305,13 @@ class EndPoints(object):
     def test_static(self):
         endpoint = "static"
         file = "assets/favicon.ico"
-        url = f"{self.LOCAL_URL}{endpoint}/{file}"
+        url = f"{self.server}/{endpoint}/{file}"
         result = self.session.get(url)
         self.assertEqual(result.status_code, HTTPStatus.OK)
 
-    @staticmethod
-    def _setUpClass(child_class, start_command):
-        child_class.ps = Popen(start_command)
+    def _setupClass(child_class, command_line):
+        child_class.ps, child_class.server = start_test_server(command_line)
+        child_class.URL_BASE = f"{child_class.server}/api/v0.2/"
         child_class.session = requests.Session()
         for i in range(90):
             try:
@@ -323,13 +320,6 @@ class EndPoints(object):
             except requests.exceptions.ConnectionError:
                 time.sleep(1)
 
-    @staticmethod
-    def _tearDownClass(child_class):
-        try:
-            child_class.ps.terminate()
-        except ProcessLookupError:
-            pass
-
 
 class EndPointsAnnotations(EndPoints):
     def test_get_schema_existing_writable(self):
@@ -385,32 +375,19 @@ class EndPointsAnnotations(EndPoints):
 class EndPointsAnndata(unittest.TestCase, EndPoints):
     """Test Case for endpoints"""
 
-    PORT = 5010
-    LOCAL_URL = f"http://127.0.0.1:{PORT}/"
-    VERSION = "v0.2"
-    URL_BASE = f"{LOCAL_URL}api/{VERSION}/"
     ANNOTATIONS_ENABLED = False
 
     @classmethod
     def setUpClass(cls):
-        cls._setUpClass(
-            cls,
-            [
-                "cellxgene",
-                "--no-upgrade-check",
-                "launch",
-                f"{PROJECT_ROOT}/example-dataset/pbmc3k.h5ad",
-                "--disable-annotations",
-                "--verbose",
-                "--experimental-enable-reembedding",
-                "--port",
-                str(cls.PORT),
-            ],
-        )
+        cls._setupClass(cls, [
+            f"{PROJECT_ROOT}/example-dataset/pbmc3k.h5ad",
+            "--disable-annotations",
+            "--experimental-enable-reembedding",
+        ])
 
     @classmethod
     def tearDownClass(cls):
-        cls._tearDownClass(cls)
+        stop_test_server(cls.ps)
 
     @property
     def annotations_enabled(self):
@@ -420,98 +397,57 @@ class EndPointsAnndata(unittest.TestCase, EndPoints):
 class EndPointsCxg(unittest.TestCase, EndPoints):
     """Test Case for endpoints"""
 
-    PORT = 5011
-    LOCAL_URL = f"http://127.0.0.1:{PORT}/"
-    VERSION = "v0.2"
-    URL_BASE = f"{LOCAL_URL}api/{VERSION}/"
     ANNOTATIONS_ENABLED = False
 
     @classmethod
     def setUpClass(cls):
-        cls._setUpClass(
-            cls,
-            [
-                "cellxgene",
-                "--no-upgrade-check",
-                "launch",
-                f"{PROJECT_ROOT}/server/test/test_datasets/pbmc3k.cxg",
-                "--disable-annotations",
-                "--verbose",
-                "--port",
-                str(cls.PORT),
-            ],
-        )
+        cls._setupClass(cls, [
+            f"{PROJECT_ROOT}/server/test/test_datasets/pbmc3k.cxg",
+            "--disable-annotations",
+        ])
 
     @classmethod
     def tearDownClass(cls):
-        cls._tearDownClass(cls)
+        stop_test_server(cls.ps)
 
 
 class EndPointsAnndataAnnotations(unittest.TestCase, EndPointsAnnotations):
     """Test Case for endpoints"""
 
-    PORT = 5012
-    LOCAL_URL = f"http://127.0.0.1:{PORT}/"
-    VERSION = "v0.2"
-    URL_BASE = f"{LOCAL_URL}api/{VERSION}/"
     ANNOTATIONS_ENABLED = True
-    MATRIX_DATA_TYPE = MatrixDataType.H5AD
 
     @classmethod
     def setUpClass(cls):
         cls.data, cls.tmp_dir, cls.annotations = data_with_tmp_annotations(
             MatrixDataType.H5AD, annotations_fixture=True
         )
-        cls._setUpClass(
-            cls,
-            [
-                "cellxgene",
-                "--no-upgrade-check",
-                "launch",
-                "--annotations-file",
-                cls.annotations.output_file,
-                "--verbose",
-                "--port",
-                str(cls.PORT),
-                cls.data.get_location(),
-            ],
-        )
+        cls._setupClass(cls, [
+            "--annotations-file",
+            cls.annotations.output_file,
+            cls.data.get_location(),
+        ])
 
     @classmethod
     def tearDownClass(cls):
         shutil.rmtree(cls.tmp_dir)
-        cls._tearDownClass(cls)
+        stop_test_server(cls.ps)
 
 
 class EndPointsCxgAnnotations(unittest.TestCase, EndPointsAnnotations):
     """Test Case for endpoints"""
 
-    PORT = 5013
-    LOCAL_URL = f"http://127.0.0.1:{PORT}/"
-    VERSION = "v0.2"
-    URL_BASE = f"{LOCAL_URL}api/{VERSION}/"
     ANNOTATIONS_ENABLED = True
-    MATRIX_DATA_TYPE = MatrixDataType.CXG
 
     @classmethod
     def setUpClass(cls):
         cls.data, cls.tmp_dir, cls.annotations = data_with_tmp_annotations(MatrixDataType.CXG, annotations_fixture=True)
-        cls._setUpClass(
-            cls,
-            [
-                "cellxgene",
-                "--no-upgrade-check",
-                "launch",
-                "--annotations-file",
-                cls.annotations.output_file,
-                "--verbose",
-                "--port",
-                str(cls.PORT),
-                cls.data.get_location(),
-            ],
-        )
+        cls._setupClass(cls, [
+            "--annotations-file",
+            cls.annotations.output_file,
+            cls.data.get_location(),
+        ])
 
     @classmethod
     def tearDownClass(cls):
         shutil.rmtree(cls.tmp_dir)
-        cls._tearDownClass(cls)
+        stop_test_server(cls.ps)

server/test/test_auth.py

@@ -0,0 +1,142 @@
+import unittest
+from server.common.app_config import AppConfig
+from server.test import PROJECT_ROOT, test_server
+import requests
+
+
+class AuthTest(unittest.TestCase):
+    def test_auth_none(self):
+        c = AppConfig()
+        c.update_server_config(
+            authentication__type=None, multi_dataset__dataroot=f"{PROJECT_ROOT}/server/test/test_datasets"
+        )
+        c.update_default_dataset_config(user_annotations__enable=False)
+
+        c.complete_config()
+
+        with test_server(app_config=c) as server:
+            session = requests.Session()
+            r = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config")
+            data_config = r.json()
+            assert "authentication" not in data_config["config"]
+
+    def test_auth_session(self):
+        c = AppConfig()
+        c.update_server_config(
+            authentication__type="session", multi_dataset__dataroot=f"{PROJECT_ROOT}/server/test/test_datasets"
+        )
+        c.update_default_dataset_config(user_annotations__enable=True)
+        c.complete_config()
+
+        with test_server(app_config=c) as server:
+            session = requests.Session()
+            r = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config")
+            data_config = r.json()
+            assert data_config["config"]["authentication"]["is_authenticated"]
+            assert not data_config["config"]["authentication"]["requires_client_login"]
+            assert data_config["config"]["authentication"]["username"] == "anonymous"
+
+    def test_auth_test(self):
+        c = AppConfig()
+        c.update_server_config(authentication__type="test")
+        c.update_server_config(
+            multi_dataset__dataroot=dict(
+                a1=dict(dataroot=f"{PROJECT_ROOT}/server/test/test_datasets", base_url="auth"),
+                a2=dict(dataroot=f"{PROJECT_ROOT}/server/test/test_datasets", base_url="no-auth"),
+            )
+        )
+
+        # specialize the configs
+        c.add_dataroot_config("a1", app__authentication_enable=True, user_annotations__enable=True)
+        c.add_dataroot_config("a2", app__authentication_enable=False, user_annotations__enable=False)
+
+        c.complete_config()
+
+        with test_server(app_config=c) as server:
+            session = requests.Session()
+
+            # auth datasets
+            r = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config")
+            data_config = r.json()
+            assert not data_config["config"]["authentication"]["is_authenticated"]
+            assert data_config["config"]["authentication"]["requires_client_login"]
+            assert data_config["config"]["authentication"]["username"] is None
+            assert data_config["config"]["parameters"]["annotations"]
+
+            login_uri = data_config["config"]["authentication"]["login"]
+            logout_uri = data_config["config"]["authentication"]["logout"]
+
+            assert login_uri == "/login?dataset=auth/pbmc3k.cxg"
+            assert logout_uri == "/logout?dataset=auth/pbmc3k.cxg"
+
+            r = session.get(f"{server}/{login_uri}")
+            # check that the login redirect worked
+            assert r.history[0].status_code == 302
+            assert r.url == f"{server}/auth/pbmc3k.cxg/"
+
+            r = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config")
+            data_config = r.json()
+            assert data_config["config"]["authentication"]["is_authenticated"]
+            assert data_config["config"]["authentication"]["username"] == "test_account"
+            assert data_config["config"]["parameters"]["annotations"]
+
+            r = session.get(f"{server}/{logout_uri}")
+            # check that the logout redirect worked
+            assert r.history[0].status_code == 302
+            assert r.url == f"{server}/auth/pbmc3k.cxg/"
+            r = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config")
+            data_config = r.json()
+            assert not data_config["config"]["authentication"]["is_authenticated"]
+            assert data_config["config"]["authentication"]["username"] is None
+            assert data_config["config"]["parameters"]["annotations"]
+
+            # no-auth datasets
+            r = session.get(f"{server}/no-auth/pbmc3k.cxg/api/v0.2/config")
+            data_config = r.json()
+            assert "authentication" not in data_config["config"]
+            assert not data_config["config"]["parameters"]["annotations"]
+
+    def test_auth_test_single(self):
+        c = AppConfig()
+        c.update_server_config(
+            authentication__type="test",
+            single_dataset__datapath=f"{PROJECT_ROOT}/server/test/test_datasets/pbmc3k.cxg")
+
+        c.complete_config()
+
+        with test_server(app_config=c) as server:
+            session = requests.Session()
+
+            r = session.get(f"{server}/api/v0.2/config")
+            data_config = r.json()
+            assert not data_config["config"]["authentication"]["is_authenticated"]
+            assert data_config["config"]["authentication"]["requires_client_login"]
+            assert data_config["config"]["authentication"]["username"] is None
+            assert data_config["config"]["parameters"]["annotations"]
+
+            login_uri = data_config["config"]["authentication"]["login"]
+            logout_uri = data_config["config"]["authentication"]["logout"]
+
+            assert login_uri == "/login"
+            assert logout_uri == "/logout"
+
+            r = session.get(f"{server}/{login_uri}")
+            # check that the login redirect worked
+            assert r.history[0].status_code == 302
+            assert r.url == f"{server}/"
+
+            r = session.get(f"{server}/api/v0.2/config")
+            data_config = r.json()
+            assert data_config["config"]["authentication"]["is_authenticated"]
+            assert data_config["config"]["authentication"]["username"] == "test_account"
+            assert data_config["config"]["parameters"]["annotations"]
+
+            r = session.get(f"{server}/{logout_uri}")
+            # check that the logout redirect worked
+            assert r.history[0].status_code == 302
+            assert r.url == f"{server}/"
+            r = session.get(f"{server}/api/v0.2/config")
+            data_config = r.json()
+            assert not data_config["config"]["authentication"]["is_authenticated"]
+            assert data_config["config"]["authentication"]["username"] is None
+            assert data_config["config"]["parameters"]["annotations"]

server/test/test_nan_rest.py

@@ -1,17 +1,13 @@
 from http import HTTPStatus
-from subprocess import Popen
 import unittest
-import time
 import math
+from server.test import start_test_server, stop_test_server
 
 import server.test.decode_fbs as decode_fbs
 
 import requests
 
-LOCAL_URL = "http://127.0.0.1:5006/"
 VERSION = "v0.2"
-URL_BASE = f"{LOCAL_URL}api/{VERSION}/"
-
 BAD_FILTER = {"filter": {"obs": {"annotation_value": [{"name": "xyz"}]}}}
 
 
@@ -20,33 +16,25 @@ class WithNaNs(unittest.TestCase):
 
     @classmethod
     def setUpClass(cls):
-        cls.ps = Popen(["cellxgene", "launch", "test/test_datasets/nan.h5ad", "--verbose", "--port", "5006"])
-        session = requests.Session()
-        for i in range(90):
-            try:
-                session.get(f"{URL_BASE}schema")
-            except requests.exceptions.ConnectionError:
-                time.sleep(1)
+        cls.ps, cls.server = start_test_server(["test/test_datasets/nan.h5ad"])
 
     @classmethod
     def tearDownClass(cls):
-        try:
-            cls.ps.terminate()
-        except ProcessLookupError:
-            pass
+        stop_test_server(cls.ps)
 
     def setUp(self):
         self.session = requests.Session()
+        self.url_base = f"{self.server}/api/{VERSION}/"
 
     def test_initialize(self):
         endpoint = "schema"
-        url = f"{URL_BASE}{endpoint}"
+        url = f"{self.url_base}{endpoint}"
         result = self.session.get(url)
         self.assertEqual(result.status_code, HTTPStatus.OK)
 
     def test_data(self):
         endpoint = "data/var"
-        url = f"{URL_BASE}{endpoint}"
+        url = f"{self.url_base}{endpoint}"
         filter = {"filter": {"var": {"index": [[0, 20]]}}}
         result = self.session.put(url, json=filter)
         self.assertEqual(result.status_code, HTTPStatus.OK)
@@ -56,7 +44,7 @@ class WithNaNs(unittest.TestCase):
 
     def test_annotation_obs(self):
         endpoint = "annotations/obs"
-        url = f"{URL_BASE}{endpoint}"
+        url = f"{self.url_base}{endpoint}"
         result = self.session.get(url)
         self.assertEqual(result.status_code, HTTPStatus.OK)
         self.assertEqual(result.headers["Content-Type"], "application/octet-stream")
@@ -65,7 +53,7 @@ class WithNaNs(unittest.TestCase):
 
     def test_annotation_var(self):
         endpoint = "annotations/var"
-        url = f"{URL_BASE}{endpoint}"
+        url = f"{self.url_base}{endpoint}"
         result = self.session.get(url)
         self.assertEqual(result.status_code, HTTPStatus.OK)
         self.assertEqual(result.headers["Content-Type"], "application/octet-stream")