From 69e159916e895a0197d272ab9b2907303123dc9a Mon Sep 17 00:00:00 2001 From: Ben MR Date: Fri, 17 Sep 2021 11:47:52 -0700 Subject: [PATCH] [zh2311] Remove auth (#2427) * remove auth * Remove all auth code zh2311 * lint * remove auth from e2e tests conf --- .github/workflows/compatibility_tests.yml | 1 - backend/common/errors.py | 3 - backend/server/Makefile | 2 +- backend/server/app/app.py | 29 ------ backend/server/app/session.py | 13 +++ backend/server/auth/__init__.py | 5 - backend/server/auth/auth.py | 91 ------------------- backend/server/auth/auth_none.py | 27 ------ backend/server/auth/auth_session.py | 39 -------- backend/server/auth/auth_test.py | 73 --------------- .../common/annotations/local_file_csv.py | 16 +--- backend/server/common/config/base_config.py | 4 +- backend/server/common/config/client_config.py | 38 -------- .../server/common/config/dataset_config.py | 10 -- backend/server/common/config/server_config.py | 21 ----- backend/server/common/rest.py | 7 +- backend/server/converters/schema/remix.py | 2 + backend/server/default_config.py | 18 ---- backend/server/requirements-dev.txt | 1 - .../test/fixtures/dataset_config_outline.py | 2 - .../test/fixtures/server_config_outline.py | 3 - .../test/test_server/unit/auth/__init__.py | 0 .../test/test_server/unit/auth/test_auth.py | 89 ------------------ .../unit/common/config/__init__.py | 6 -- .../unit/common/config/test_dataset_config.py | 18 +--- .../unit/common/config/test_server_config.py | 11 +-- client/__tests__/e2e/test_config.yaml | 8 -- 27 files changed, 28 insertions(+), 509 deletions(-) create mode 100644 backend/server/app/session.py delete mode 100644 backend/server/auth/__init__.py delete mode 100644 backend/server/auth/auth.py delete mode 100644 backend/server/auth/auth_none.py delete mode 100644 backend/server/auth/auth_session.py delete mode 100644 backend/server/auth/auth_test.py delete mode 100644 backend/test/test_server/unit/auth/__init__.py delete mode 100644 backend/test/test_server/unit/auth/test_auth.py diff --git a/.github/workflows/compatibility_tests.yml b/.github/workflows/compatibility_tests.yml index e3e3ba8f..190dbd29 100644 --- a/.github/workflows/compatibility_tests.yml +++ b/.github/workflows/compatibility_tests.yml @@ -9,7 +9,6 @@ on: env: JEST_ENV: prod - CXG_AUTH_TYPE: none jobs: docker-build: diff --git a/backend/common/errors.py b/backend/common/errors.py index 56075016..de6ea445 100644 --- a/backend/common/errors.py +++ b/backend/common/errors.py @@ -41,9 +41,6 @@ define_request_exception( ) define_request_exception("ExceedsLimitError", "Raised when an HTTP request exceeds a limit/quota") define_request_exception("ColorFormatException", "Raised when color helper functions encounter an unknown color format") -define_request_exception( - "AuthenticationError", "Raised when there is an authentication error", default_status_code=HTTPStatus.UNAUTHORIZED -) define_request_exception( "AnnotationCategoryNameError", diff --git a/backend/server/Makefile b/backend/server/Makefile index bd34ec30..feb164b7 100644 --- a/backend/server/Makefile +++ b/backend/server/Makefile @@ -9,7 +9,7 @@ clean: .PHONY: unit-test unit-test: PYTHONWARNINGS=ignore:ResourceWarning coverage run \ - --source=app,auth,cli,common,compute,converters,data_anndata,data_common \ + --source=app,cli,common,compute,converters,data_anndata,data_common \ --omit=.coverage,venv \ -m unittest discover \ --start-directory ../test/test_server/unit \ diff --git a/backend/server/app/app.py b/backend/server/app/app.py index 38a7aac4..bc283cc9 100644 --- a/backend/server/app/app.py +++ b/backend/server/app/app.py @@ -1,7 +1,6 @@ import datetime import logging from functools import wraps -from http import HTTPStatus from flask import ( Flask, @@ -81,18 +80,6 @@ def handle_request_exception(error): return common_rest.abort_and_log(error.status_code, error.message, loglevel=logging.INFO, include_exc_info=True) -def requires_authentication(func): - @wraps(func) - def wrapped_function(self, *args, **kwargs): - auth = current_app.auth - if auth.is_user_authenticated(): - return func(self, *args, **kwargs) - else: - return make_response("not authenticated", HTTPStatus.UNAUTHORIZED) - - return wrapped_function - - def rest_get_data_adaptor(func): @wraps(func) def wrapped_function(self): @@ -128,20 +115,12 @@ class ConfigAPI(Resource): return common_rest.config_get(current_app.app_config, data_adaptor) -class UserInfoAPI(Resource): - @cache_control_always(no_store=True) - @rest_get_data_adaptor - def get(self, data_adaptor): - return common_rest.userinfo_get(current_app.app_config, data_adaptor) - - class AnnotationsObsAPI(Resource): @cache_control(public=True, max_age=ONE_WEEK) @rest_get_data_adaptor def get(self, data_adaptor): return common_rest.annotations_obs_get(request, data_adaptor) - @requires_authentication @cache_control(no_store=True) @rest_get_data_adaptor def put(self, data_adaptor): @@ -194,7 +173,6 @@ class GenesetsAPI(Resource): def get(self, data_adaptor): return common_rest.genesets_get(request, data_adaptor) - @requires_authentication @cache_control(no_store=True) @rest_get_data_adaptor def put(self, data_adaptor): @@ -233,7 +211,6 @@ def get_api_dataroot_resources(bp_dataroot): # Initialization routes add_resource(SchemaAPI, "/schema") add_resource(ConfigAPI, "/config") - add_resource(UserInfoAPI, "/userinfo") # Data routes add_resource(AnnotationsObsAPI, "/annotations/obs") add_resource(AnnotationsVarAPI, "/annotations/var") @@ -288,9 +265,3 @@ class Server: self.app.data_adaptor = server_config.data_adaptor self.app.app_config = app_config - - auth = server_config.auth - self.app.auth = auth - if auth.requires_client_login(): - auth.add_url_rules(self.app) - auth.complete_setup(self.app) diff --git a/backend/server/app/session.py b/backend/server/app/session.py new file mode 100644 index 00000000..933369b5 --- /dev/null +++ b/backend/server/app/session.py @@ -0,0 +1,13 @@ +from uuid import uuid4 + +from flask.sessions import SessionMixin + +CXGUID = "cxguid" + + +def get_user_id(session: SessionMixin) -> str: + """ Gets a session-persistent user id. Creates one in the Flask session if non-extant """ + if CXGUID not in session: + session[CXGUID] = uuid4().hex + session.permanent = True + return session[CXGUID] diff --git a/backend/server/auth/__init__.py b/backend/server/auth/__init__.py deleted file mode 100644 index 33b5ad2b..00000000 --- a/backend/server/auth/__init__.py +++ /dev/null @@ -1,5 +0,0 @@ -# import the built in auth types so they can be registered - -import backend.server.auth.auth_none # noqa: F401 -import backend.server.auth.auth_test # noqa: F401 -import backend.server.auth.auth_session # noqa: F401 diff --git a/backend/server/auth/auth.py b/backend/server/auth/auth.py deleted file mode 100644 index 03184e8d..00000000 --- a/backend/server/auth/auth.py +++ /dev/null @@ -1,91 +0,0 @@ -from abc import ABC, abstractmethod - - -class AuthTypeBase(ABC): - """Base type for all authentication types.""" - - def __init__(self): - super().__init__() - - @abstractmethod - def is_valid_authentication_type(self): - """Return True if the auth type is valid, e.g. it can return userinfo and username. - (AuthTypeNone is the only one type that returns False)""" - pass - - def requires_client_login(self): - """Return True if the user needs to login from the client (e.g. Login button is shown)""" - return False - - @abstractmethod - def complete_setup(self, app): - """complete any setup that may be needed by this auth type. The Flask app is passed in. - This is the last auth function called before the server starts to run.""" - pass - - @abstractmethod - def is_user_authenticated(self): - """Return True if the user is authenticated""" - pass - - @abstractmethod - def get_user_id(self): - """Return the id for this user (string)""" - pass - - @abstractmethod - def get_user_name(self): - """Return the name of the user (string)""" - pass - - @abstractmethod - def get_user_email(self): - """Return the name of the user (string)""" - pass - - def get_user_picture(self): - """Return the location to the user's picture""" - return None - - -class AuthTypeClientBase(AuthTypeBase): - """Base type for all authentication types that require the client to login""" - - def __init__(self): - super().__init__() - - def requires_client_login(self): - return True - - @abstractmethod - def add_url_rules(self, selfapp): - """Add url rules to the app (like /login, /logout, etc)""" - pass - - @abstractmethod - def get_login_url(self, data_adaptor): - """Return the url for the login route""" - pass - - @abstractmethod - def get_logout_url(self, data_adaptor): - """Return the url for the logout route""" - pass - - -class AuthTypeFactory: - """Factory class to create an authentication type""" - - auth_types = {} - - @staticmethod - def register(name, auth_type): - assert issubclass(auth_type, AuthTypeBase) - AuthTypeFactory.auth_types[name] = auth_type - - @staticmethod - def create(name, app_config): - auth_type = AuthTypeFactory.auth_types.get(name) - if auth_type is None: - return None - return auth_type(app_config) diff --git a/backend/server/auth/auth_none.py b/backend/server/auth/auth_none.py deleted file mode 100644 index 92b7ce0d..00000000 --- a/backend/server/auth/auth_none.py +++ /dev/null @@ -1,27 +0,0 @@ -from backend.server.auth.auth import AuthTypeBase, AuthTypeFactory - - -class AuthTypeNone(AuthTypeBase): - def __init__(self, app_config): - super().__init__() - - def is_valid_authentication_type(self): - return False - - def complete_setup(self, app): - pass - - def is_user_authenticated(self): - return True - - def get_user_id(self): - return None - - def get_user_name(self): - return None - - def get_user_email(self): - return None - - -AuthTypeFactory.register(None, AuthTypeNone) diff --git a/backend/server/auth/auth_session.py b/backend/server/auth/auth_session.py deleted file mode 100644 index e177a8bc..00000000 --- a/backend/server/auth/auth_session.py +++ /dev/null @@ -1,39 +0,0 @@ -from backend.server.auth.auth import AuthTypeBase, AuthTypeFactory -from flask import session -from uuid import uuid4 - - -class AuthTypeSession(AuthTypeBase): - """Session based authentication. The user is always logged. The user id is a random number - associated with the session. This is a good choice for desktop servers.""" - - # key in the session token for userid - CXGUID = "cxguid" - - def __init__(self, app_config): - super().__init__() - - def is_valid_authentication_type(self): - return True - - def complete_setup(self, app): - pass - - def is_user_authenticated(self): - # always authenticated - return True - - def get_user_id(self): - if self.CXGUID not in session: - session[self.CXGUID] = uuid4().hex - session.permanent = True - return session[self.CXGUID] - - def get_user_name(self): - return "anonymous" - - def get_user_email(self): - return None - - -AuthTypeFactory.register("session", AuthTypeSession) diff --git a/backend/server/auth/auth_test.py b/backend/server/auth/auth_test.py deleted file mode 100644 index cda8e34c..00000000 --- a/backend/server/auth/auth_test.py +++ /dev/null @@ -1,73 +0,0 @@ -from backend.server.auth.auth import AuthTypeClientBase, AuthTypeFactory -from flask import session, request, redirect - - -class AuthTypeTest(AuthTypeClientBase): - """An authentication type for testing client based logins. When the login route is accessed - the user is automatically logged in with a default or configured username""" - - # key in session token with userid and username - CXGUID = "cxguid_test" - CXGUNAME = "cxguname_test" - CXGUEMAIL = "cxguemail_test" - CXGUPICTURE = "cxgupicture_test" - - def __init__(self, app_config): - super().__init__() - self.user_name = "test_account" - self.user_id = "id0001" - self.user_email = "test_account@test.com" - self.user_picture = None - - def is_valid_authentication_type(self): - return True - - def requires_client_login(self): - return True - - def add_url_rules(self, app): - app.add_url_rule("/login", "login", self.login, methods=["GET"]) - app.add_url_rule("/logout", "logout", self.logout, methods=["GET"]) - - def complete_setup(self, app): - pass - - def is_user_authenticated(self): - return self.CXGUID in session - - def get_user_id(self): - return session.get(self.CXGUID) - - def get_user_name(self): - return session.get(self.CXGUNAME) - - def get_user_email(self): - return session.get(self.CXGUEMAIL) - - def get_user_picture(self): - return session.get(self.CXGUPICTURE) - - def login(self): - args = request.args - return_to = args.get("dataset", "/") - session[self.CXGUID] = args.get("userid", self.user_id) - session[self.CXGUNAME] = args.get("username", self.user_name) - session[self.CXGUEMAIL] = args.get("email", self.user_email) - session[self.CXGUPICTURE] = args.get("picture", self.user_picture) - return redirect(return_to) - - def logout(self): - session.clear() - return_to = request.args.get("dataset", "/") - return redirect(return_to) - - def get_login_url(self, data_adaptor): - """Return the url for the login route""" - return "/login" - - def get_logout_url(self, data_adaptor): - """Return the url for the logout route""" - return "/logout" - - -AuthTypeFactory.register("test", AuthTypeTest) diff --git a/backend/server/common/annotations/local_file_csv.py b/backend/server/common/annotations/local_file_csv.py index 3856dafc..2ca8ab99 100644 --- a/backend/server/common/annotations/local_file_csv.py +++ b/backend/server/common/annotations/local_file_csv.py @@ -6,9 +6,10 @@ from datetime import datetime from hashlib import blake2b import pandas as pd -from flask import session, has_request_context, current_app +from flask import session from backend.server import __version__ as cellxgene_version +from backend.server.app.session import get_user_id from backend.server.common.annotations.annotations import Annotations from backend.common.genesets import read_gene_sets_tidycsv from backend.common.errors import AnnotationsError, ObsoleteRequest @@ -60,10 +61,6 @@ class AnnotationsLocalFile(Annotations): def read_labels(self, data_adaptor): self.check_user_annotations_enabled() # raises - if has_request_context(): - if not current_app.auth.is_user_authenticated(): - return pd.DataFrame() - fname = self._get_celllabels_filename(data_adaptor) with self.label_lock: if fname is not None and os.path.exists(fname) and os.path.getsize(fname) > 0: @@ -111,10 +108,6 @@ class AnnotationsLocalFile(Annotations): self.last_labels = df def read_gene_sets(self, data_adaptor, context=None): - if has_request_context(): - if not current_app.auth.is_user_authenticated(): - return ({}, self.last_geneset_tid) - fname = self._get_genesets_filename(data_adaptor) gene_sets = {} tid = None @@ -177,7 +170,7 @@ class AnnotationsLocalFile(Annotations): Return a short hash that weakly identifies the user and dataset. Used to create safe annotations output file names. """ - uid = current_app.auth.get_user_id() or "" + uid = get_user_id(session) id = (uid + data_adaptor.get_location()).encode() idhash = base64.b32encode(blake2b(id, digest_size=5).digest()).decode("utf-8") return idhash @@ -275,7 +268,6 @@ class AnnotationsLocalFile(Annotations): params["annotations-data-collection-is-read-only"] = not self.user_annotations_enabled() params["annotations-data-collection-name"] = collection - if current_app.auth.is_user_authenticated(): - params["annotations-user-data-idhash"] = self._get_userdata_idhash(data_adaptor) + params["annotations-user-data-idhash"] = self._get_userdata_idhash(data_adaptor) parameters.update(params) diff --git a/backend/server/common/config/base_config.py b/backend/server/common/config/base_config.py index ca383edf..1c4922f3 100644 --- a/backend/server/common/config/base_config.py +++ b/backend/server/common/config/base_config.py @@ -22,13 +22,13 @@ class BaseConfig(object): def create_mapping(self, config): """ Create a dictionary where the keys are the name of attributes (using double underscore convention) - For example: authentication__type + For example: app__host The values are a tuple, - the first item of the tuple is a tuple of path elements (location in config 'tree') - the second item is the value of the config parameter - For example: (('authentication', 'type'), 'session')) + For example: (("app", "host"), "session")) """ config_copy = copy.deepcopy(config) mapping = {} diff --git a/backend/server/common/config/client_config.py b/backend/server/common/config/client_config.py index f0f3ebfa..8cb02c33 100644 --- a/backend/server/common/config/client_config.py +++ b/backend/server/common/config/client_config.py @@ -9,7 +9,6 @@ def get_client_config(app_config, data_adaptor): server_config = app_config.server_config dataset_config = data_adaptor.dataset_config annotation = dataset_config.user_annotations - auth = server_config.auth # FIXME The current set of config is not consistently presented: # we have camalCase, hyphen-text, and underscore_text @@ -79,41 +78,4 @@ def get_client_config(app_config, data_adaptor): "diffexp_cellcount_max": server_config.limits__diffexp_cellcount_max, } - if dataset_config.app__authentication_enable and auth.is_valid_authentication_type(): - config["authentication"] = { - "requires_client_login": auth.requires_client_login(), - } - if auth.requires_client_login(): - config["authentication"].update( - { - # Todo why are these stored on the data_adaptor? - "login": auth.get_login_url(data_adaptor), - "logout": auth.get_logout_url(data_adaptor), - } - ) - return client_config - - -def get_client_userinfo(app_config, data_adaptor): - """ - Return the userinfo as required by the /userinfo REST route - """ - - server_config = app_config.server_config - dataset_config = data_adaptor.dataset_config - auth = server_config.auth - - # make sure the configuration has been checked. - app_config.check_config() - - if dataset_config.app__authentication_enable and auth.is_valid_authentication_type(): - userinfo = {} - userinfo["userinfo"] = { - "is_authenticated": auth.is_user_authenticated(), - "username": auth.get_user_name(), - "user_id": auth.get_user_id(), - "email": auth.get_user_email(), - "picture": auth.get_user_picture(), - } - return userinfo diff --git a/backend/server/common/config/dataset_config.py b/backend/server/common/config/dataset_config.py index 0f5cebe2..b77049c2 100644 --- a/backend/server/common/config/dataset_config.py +++ b/backend/server/common/config/dataset_config.py @@ -16,7 +16,6 @@ class DatasetConfig(BaseConfig): try: self.app__scripts = default_config["app"]["scripts"] self.app__inline_scripts = default_config["app"]["inline_scripts"] - self.app__authentication_enable = default_config["app"]["authentication_enable"] self.presentation__max_categories = default_config["presentation"]["max_categories"] self.presentation__custom_colors = default_config["presentation"]["custom_colors"] @@ -65,7 +64,6 @@ class DatasetConfig(BaseConfig): def handle_app(self): self.validate_correct_type_of_configuration_attribute("app__scripts", list) self.validate_correct_type_of_configuration_attribute("app__inline_scripts", list) - self.validate_correct_type_of_configuration_attribute("app__authentication_enable", bool) # scripts can be string (filename) or dict (attributes). Convert string to dict. scripts = [] @@ -100,14 +98,6 @@ class DatasetConfig(BaseConfig): ) self.validate_correct_type_of_configuration_attribute("user_annotations__gene_sets__readonly", bool) - if self.user_annotations__enable or not self.user_annotations__gene_sets__readonly: - server_config = self.app_config.server_config - if not self.app__authentication_enable: - raise ConfigurationError("user annotations requires authentication to be enabled") - if not server_config.auth.is_valid_authentication_type(): - auth_type = server_config.authentication__type - raise ConfigurationError(f"authentication method {auth_type} is not compatible with user annotations") - # Must always have an annotations instance to support genesets. User annotation (cell labels) are optional # as are writable gene sets if self.user_annotations__type == "local_file_csv": diff --git a/backend/server/common/config/server_config.py b/backend/server/common/config/server_config.py index e7b2d2d8..cb1846f1 100644 --- a/backend/server/common/config/server_config.py +++ b/backend/server/common/config/server_config.py @@ -4,7 +4,6 @@ import warnings from os.path import basename from urllib.parse import urlparse -from backend.server.auth.auth import AuthTypeFactory from backend.server.common.config.base_config import BaseConfig from backend.server.common.config import DEFAULT_SERVER_PORT, BIG_FILE_SIZE_THRESHOLD from backend.common.utils.data_locator import discover_s3_region_name @@ -29,11 +28,6 @@ class ServerConfig(BaseConfig): self.app__flask_secret_key = default_config["app"]["flask_secret_key"] self.app__generate_cache_control_headers = default_config["app"]["generate_cache_control_headers"] - self.authentication__type = default_config["authentication"]["type"] - self.authentication__insecure_test_environment = default_config["authentication"][ - "insecure_test_environment" - ] - self.single_dataset__datapath = default_config["single_dataset"]["datapath"] self.single_dataset__obs_names = default_config["single_dataset"]["obs_names"] self.single_dataset__var_names = default_config["single_dataset"]["var_names"] @@ -52,13 +46,9 @@ class ServerConfig(BaseConfig): self.data_adaptor = None - # The authentication object - self.auth = None - def complete_config(self, context): self.handle_app(context) self.handle_data_source() - self.handle_authentication() self.handle_data_locator() self.handle_adaptor() # may depend on data_locator self.handle_single_dataset(context) # may depend on adaptor @@ -106,17 +96,6 @@ class ServerConfig(BaseConfig): if not self.app__verbose: sys.tracebacklimit = 0 - def handle_authentication(self): - self.validate_correct_type_of_configuration_attribute("authentication__type", (type(None), str)) - self.validate_correct_type_of_configuration_attribute("authentication__insecure_test_environment", bool) - - if self.authentication__type == "test" and not self.authentication__insecure_test_environment: - raise ConfigurationError("Test auth can only be used in an insecure test environment") - - self.auth = AuthTypeFactory.create(self.authentication__type, self) - if self.auth is None: - raise ConfigurationError(f"Unknown authentication type: {self.authentication__type}") - def handle_data_locator(self): self.validate_correct_type_of_configuration_attribute("data_locator__s3__region_name", (type(None), bool, str)) if self.data_locator__s3__region_name is True: diff --git a/backend/server/common/rest.py b/backend/server/common/rest.py index 4beade4e..bfb3ba2e 100644 --- a/backend/server/common/rest.py +++ b/backend/server/common/rest.py @@ -8,7 +8,7 @@ import json from flask import make_response, jsonify, current_app, abort from werkzeug.urls import url_unquote -from backend.server.common.config.client_config import get_client_config, get_client_userinfo +from backend.server.common.config.client_config import get_client_config from backend.common.constants import Axis, DiffExpMode, JSON_NaN_to_num_warning_msg from backend.common.errors import ( FilterError, @@ -126,11 +126,6 @@ def config_get(app_config, data_adaptor): return make_response(jsonify(config), HTTPStatus.OK) -def userinfo_get(app_config, data_adaptor): - config = get_client_userinfo(app_config, data_adaptor) - return make_response(jsonify(config), HTTPStatus.OK) - - def annotations_obs_get(request, data_adaptor): fields = request.args.getlist("annotation-name", None) num_columns_requested = len(data_adaptor.get_obs_keys()) if len(fields) == 0 else len(fields) diff --git a/backend/server/converters/schema/remix.py b/backend/server/converters/schema/remix.py index 5cc60746..aba878b1 100644 --- a/backend/server/converters/schema/remix.py +++ b/backend/server/converters/schema/remix.py @@ -216,12 +216,14 @@ def fixup_gene_symbols(adata, fixup_config): return fixup_adata + def _strip_version(adata): """Remove version information from the AnnData object.""" if "version" in adata.uns_keys(): del adata.uns["version"] + def apply_schema(source_h5ad, remix_config, output_filename): try: diff --git a/backend/server/default_config.py b/backend/server/default_config.py index 9dc15f16..49453747 100644 --- a/backend/server/default_config.py +++ b/backend/server/default_config.py @@ -12,13 +12,6 @@ server: flask_secret_key: null generate_cache_control_headers: false - authentication: - # The authentication types may be "none" or "session" - # none: No authentication support, features like user_annotations must not be enabled. - # session: A session based userid is automatically generated. (no params needed) - type: session - insecure_test_environment: false - single_dataset: # If datapath is set, then cellxgene with serve a single dataset located at datapath. datapath: null @@ -52,9 +45,6 @@ dataset: # Inline scripts are a list of file names, where the contents of the file will be injected into the index. inline_scripts: [] - # allow authentication support - authentication_enable: true - presentation: max_categories: 1000 custom_colors: true @@ -118,14 +108,6 @@ external: # - key: db_uri # path: [dataset, user_annotations, db_uri] # required: true - # - name: my_auth_secret - # values: - # - key: client_secret - # path: [server, authentication, client_secret] - # required: true - # - key: client_id - # path: [server, authentication, client_id] - # required: true aws_secrets_manager: region: null diff --git a/backend/server/requirements-dev.txt b/backend/server/requirements-dev.txt index ae94747a..2823f465 100644 --- a/backend/server/requirements-dev.txt +++ b/backend/server/requirements-dev.txt @@ -1,4 +1,3 @@ -Authlib>=0.14.3 black bumpversion>=0.5 codecov>=2.0.15 diff --git a/backend/test/fixtures/dataset_config_outline.py b/backend/test/fixtures/dataset_config_outline.py index ff8a6e03..10346d28 100644 --- a/backend/test/fixtures/dataset_config_outline.py +++ b/backend/test/fixtures/dataset_config_outline.py @@ -4,8 +4,6 @@ dataset: scripts: {scripts} #list of strs (filenames) or dicts containing keys inline_scripts: {inline_scripts} #list of strs (filenames) - authentication_enable: {authentication_enable} - presentation: max_categories: {max_categories} custom_colors: {custom_colors} diff --git a/backend/test/fixtures/server_config_outline.py b/backend/test/fixtures/server_config_outline.py index cb4fa085..178c1134 100644 --- a/backend/test/fixtures/server_config_outline.py +++ b/backend/test/fixtures/server_config_outline.py @@ -8,9 +8,6 @@ f"""server: force_https: {force_https} flask_secret_key: {flask_secret_key} generate_cache_control_headers: {generate_cache_control_headers} - authentication: - type: {auth_type} - insecure_test_environment: {insecure_test_environment} single_dataset: datapath: {dataset_datapath} diff --git a/backend/test/test_server/unit/auth/__init__.py b/backend/test/test_server/unit/auth/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/backend/test/test_server/unit/auth/test_auth.py b/backend/test/test_server/unit/auth/test_auth.py deleted file mode 100644 index 2db0c3ec..00000000 --- a/backend/test/test_server/unit/auth/test_auth.py +++ /dev/null @@ -1,89 +0,0 @@ -import unittest - -import requests - -from backend.server.common.config.app_config import AppConfig -from backend.test.test_server.unit import test_server -from backend.test import H5AD_FIXTURE - - -class AuthTest(unittest.TestCase): - def setUp(self): - self.dataset_datapath = H5AD_FIXTURE - - def test_auth_none(self): - app_config = AppConfig() - app_config.update_server_config(app__flask_secret_key="secret") - app_config.update_server_config(authentication__type=None, single_dataset__datapath=self.dataset_datapath) - app_config.update_dataset_config(user_annotations__enable=False, user_annotations__gene_sets__readonly=True) - - app_config.complete_config() - - with test_server(app_config=app_config) as server: - session = requests.Session() - config = session.get(f"{server}/api/v0.2/config").json() - userinfo = session.get(f"{server}/api/v0.2/userinfo").json() - self.assertNotIn("authentication", config["config"]) - self.assertIsNone(userinfo) - - def test_auth_session(self): - app_config = AppConfig() - app_config.update_server_config(app__flask_secret_key="secret") - app_config.update_server_config(authentication__type="session", single_dataset__datapath=self.dataset_datapath) - app_config.update_dataset_config(user_annotations__enable=True) - app_config.complete_config() - with test_server(app_config=app_config) as server: - session = requests.Session() - config = session.get(f"{server}/api/v0.2/config").json() - userinfo = session.get(f"{server}/api/v0.2/userinfo").json() - - self.assertFalse(config["config"]["authentication"]["requires_client_login"]) - self.assertTrue(userinfo["userinfo"]["is_authenticated"]) - self.assertEqual(userinfo["userinfo"]["username"], "anonymous") - - def test_auth_test_single(self): - app_config = AppConfig() - app_config.update_server_config(app__flask_secret_key="secret") - app_config.update_server_config( - authentication__type="test", - single_dataset__datapath=self.dataset_datapath, - authentication__insecure_test_environment=True, - ) - - app_config.complete_config() - - with test_server(app_config=app_config) as server: - session = requests.Session() - config = session.get(f"{server}/api/v0.2/config").json() - userinfo = session.get(f"{server}/api/v0.2/userinfo").json() - self.assertFalse(userinfo["userinfo"]["is_authenticated"]) - self.assertIsNone(userinfo["userinfo"]["username"]) - self.assertTrue(config["config"]["authentication"]["requires_client_login"]) - self.assertTrue(config["config"]["parameters"]["annotations"]) - - login_uri = config["config"]["authentication"]["login"] - logout_uri = config["config"]["authentication"]["logout"] - - self.assertEqual(login_uri, "/login") - self.assertEqual(logout_uri, "/logout") - - response = session.get(f"{server}/{login_uri}") - # check that the login redirect worked - self.assertEqual(response.history[0].status_code, 302) - self.assertEqual(response.url, f"{server}/") - - config = session.get(f"{server}/api/v0.2/config").json() - userinfo = session.get(f"{server}/api/v0.2/userinfo").json() - self.assertTrue(userinfo["userinfo"]["is_authenticated"]) - self.assertEqual(userinfo["userinfo"]["username"], "test_account") - self.assertTrue(config["config"]["parameters"]["annotations"]) - - response = session.get(f"{server}/{logout_uri}") - # check that the logout redirect worked - self.assertEqual(response.history[0].status_code, 302) - self.assertEqual(response.url, f"{server}/") - config = session.get(f"{server}/api/v0.2/config").json() - userinfo = session.get(f"{server}/api/v0.2/userinfo").json() - self.assertFalse(userinfo["userinfo"]["is_authenticated"]) - self.assertIsNone(userinfo["userinfo"]["username"]) - self.assertTrue(config["config"]["parameters"]["annotations"]) diff --git a/backend/test/test_server/unit/common/config/__init__.py b/backend/test/test_server/unit/common/config/__init__.py index 371038a2..12c1ee3a 100644 --- a/backend/test/test_server/unit/common/config/__init__.py +++ b/backend/test/test_server/unit/common/config/__init__.py @@ -33,7 +33,6 @@ class ConfigTests(unittest.TestCase): force_https="false", flask_secret_key="secret", generate_cache_control_headers="false", - auth_type="session", insecure_test_environment="false", index="false", allowed_matrix_types=[], @@ -69,7 +68,6 @@ class ConfigTests(unittest.TestCase): force_https="false", flask_secret_key="secret", generate_cache_control_headers="false", - auth_type="session", index="false", allowed_matrix_types=[], max_cached_datasets=5, @@ -85,7 +83,6 @@ class ConfigTests(unittest.TestCase): diffexp_cellcount_max="null", scripts=[], inline_scripts=[], - authentication_enable="true", max_categories=1000, custom_colors="true", enable_users_annotations="true", @@ -117,7 +114,6 @@ class ConfigTests(unittest.TestCase): force_https=force_https, flask_secret_key=flask_secret_key, generate_cache_control_headers=generate_cache_control_headers, - auth_type=auth_type, index=index, allowed_matrix_types=allowed_matrix_types, max_cached_datasets=max_cached_datasets, @@ -136,7 +132,6 @@ class ConfigTests(unittest.TestCase): dataset_config = self.custom_dataset_config( scripts=scripts, inline_scripts=inline_scripts, - authentication_enable=authentication_enable, max_categories=max_categories, custom_colors=custom_colors, enable_users_annotations=enable_users_annotations, @@ -172,7 +167,6 @@ class ConfigTests(unittest.TestCase): self, scripts=[], inline_scripts=[], - authentication_enable="true", max_categories=1000, custom_colors="true", enable_users_annotations="true", diff --git a/backend/test/test_server/unit/common/config/test_dataset_config.py b/backend/test/test_server/unit/common/config/test_dataset_config.py index 2f923246..b60a5f62 100644 --- a/backend/test/test_server/unit/common/config/test_dataset_config.py +++ b/backend/test/test_server/unit/common/config/test_dataset_config.py @@ -46,7 +46,7 @@ class TestDatasetConfig(ConfigTests): mock_check_attrs.side_effect = BaseConfig.validate_correct_type_of_configuration_attribute() self.dataset_config.complete_config(self.context) self.assertIsNotNone(self.config.server_config.data_adaptor) - self.assertEqual(mock_check_attrs.call_count, 17) + self.assertEqual(mock_check_attrs.call_count, 16) def test_app_sets_script_vars(self): config = self.get_config(scripts=["path/to/script"]) @@ -71,26 +71,16 @@ class TestDatasetConfig(ConfigTests): with self.assertRaises(ConfigurationError): config.dataset_config.handle_app() - def test_handle_user_annotations_ensures_auth_is_enabled_with_valid_auth_type(self): - config = self.get_config(enable_users_annotations="true", authentication_enable="false") - config.server_config.complete_config(self.context) - with self.assertRaises(ConfigurationError): - config.dataset_config.handle_user_annotations(self.context) - - config = self.get_config(enable_users_annotations="true", authentication_enable="true", auth_type="pretend") - with self.assertRaises(ConfigurationError): - config.server_config.complete_config(self.context) - def test_handle_user_annotations__instantiates_user_annotations_class_correctly(self): config = self.get_config( - enable_users_annotations="true", authentication_enable="true", annotation_type="local_file_csv" + enable_users_annotations="true", annotation_type="local_file_csv" ) config.server_config.complete_config(self.context) config.dataset_config.handle_user_annotations(self.context) self.assertIsInstance(config.dataset_config.user_annotations, AnnotationsLocalFile) config = self.get_config( - enable_users_annotations="true", authentication_enable="true", annotation_type="NOT_REAL" + enable_users_annotations="true", annotation_type="NOT_REAL" ) config.server_config.complete_config(self.context) with self.assertRaises(ConfigurationError): @@ -98,7 +88,7 @@ class TestDatasetConfig(ConfigTests): def test_handle_local_file_csv_annotations__sets_dir_if_not_passed_in(self): config = self.get_config( - enable_users_annotations="true", authentication_enable="true", annotation_type="local_file_csv" + enable_users_annotations="true", annotation_type="local_file_csv" ) config.server_config.complete_config(self.context) config.dataset_config.handle_local_file_csv_annotations(self.context) diff --git a/backend/test/test_server/unit/common/config/test_server_config.py b/backend/test/test_server/unit/common/config/test_server_config.py index 647e29fe..181ad5e2 100644 --- a/backend/test/test_server/unit/common/config/test_server_config.py +++ b/backend/test/test_server/unit/common/config/test_server_config.py @@ -49,7 +49,7 @@ class TestServerConfig(ConfigTests): def test_complete_config_checks_all_attr(self, mock_check_attrs): mock_check_attrs.side_effect = BaseConfig.validate_correct_type_of_configuration_attribute() self.server_config.complete_config(self.context) - self.assertEqual(mock_check_attrs.call_count, 21) + self.assertEqual(mock_check_attrs.call_count, 19) def test_handle_app__throws_error_if_port_doesnt_exist(self): config = self.get_config(port=99999999) @@ -111,12 +111,3 @@ class TestServerConfig(ConfigTests): config.update_from_config_file(file_name) with self.assertRaises(ConfigurationError): config.server_config.handle_single_dataset(self.context) - - def test_test_auth_only_in_insecure(self): - - config = self.get_config(auth_type="test") - with self.assertRaises(ConfigurationError): - config.complete_config() - - config.update_server_config(authentication__insecure_test_environment=True) - config.complete_config() diff --git a/client/__tests__/e2e/test_config.yaml b/client/__tests__/e2e/test_config.yaml index cda11f0d..4085e204 100644 --- a/client/__tests__/e2e/test_config.yaml +++ b/client/__tests__/e2e/test_config.yaml @@ -3,14 +3,6 @@ server: force_https: true port: 5005 - authentication: - # The authentication types may be "none", "session", "oauth" - # none: No authentication support, features like user_annotations must not be enabled. - # session: A session based userid is automatically generated. (no params needed) - # oauth: oauth2 is used for authentication; parameters are defined in params_oauth. - type: session - insecure_test_environment: true - dataset: presentation: max_categories: 1000