From c877a8b30ea42e8a2c1f77eb261c72b298512e22 Mon Sep 17 00:00:00 2001 From: Andrew Eckart Date: Thu, 5 Nov 2020 22:07:54 -0600 Subject: [PATCH 1/6] Add DISABLE_USER_MGMT config value to allow opt-out of ServiceX user management system --- README.rst | 9 +++++++-- app.conf.template | 5 +++++ servicex/resources/servicex_resource.py | 15 ++++++++------- servicex/resources/users/token_refresh.py | 12 +++++++----- servicex/templates/base.html | 2 +- tests/resource_test_base.py | 5 ++++- 6 files changed, 32 insertions(+), 16 deletions(-) diff --git a/README.rst b/README.rst index bb2985af..3f82c579 100644 --- a/README.rst +++ b/README.rst @@ -15,11 +15,16 @@ services: User Management --------------- -If ``ENABLE_AUTH`` is set to True, an identity management system will be -enabled. Users will need to create accounts in order to make requests. +If ``ENABLE_AUTH`` is set to True and `DISABLE_USER_MGMT` is False, +an identity management system will be enabled. +Users will need to create accounts in order to make requests. The system consists of two components: authentication (verification of each user's identity) and authorization (control of access to API resources). +If ``ENABLE_AUTH`` and ``DISABLE_USER_MGMT`` are both set to True, then you +will need to generate your own JWT refresh tokens externally using +``JWT_SECRET_KEY``, and provide them to end users. + Authentication ************** Authentication is currently implemented via `Globus `_, diff --git a/app.conf.template b/app.conf.template index b2d63f97..3bd10dd9 100644 --- a/app.conf.template +++ b/app.conf.template @@ -10,6 +10,11 @@ DOCS_BASE_URL = 'https://servicex.readthedocs.io/en/latest/' # Enable JWT auth on public endpoints ENABLE_AUTH=False +# Disable built-in ServiceX user management system (OAuth via Globus) +# If set to True while ENABLE_AUTH is also True, then you must generate your +# own JWT refresh tokens with identity claims using the JWT_SECRET_KEY +DISABLE_USER_MGMT = False + # Globus configuration - obtained at https://auth.globus.org/v2/web/developers GLOBUS_CLIENT_ID='globus-client-id' GLOBUS_CLIENT_SECRET='globus-client-secret' diff --git a/servicex/resources/servicex_resource.py b/servicex/resources/servicex_resource.py index effb519c..a2fb9bcb 100644 --- a/servicex/resources/servicex_resource.py +++ b/servicex/resources/servicex_resource.py @@ -43,14 +43,15 @@ def _generate_advertised_endpoint(cls, endpoint): @staticmethod def get_requesting_user() -> Optional[UserModel]: """ - :return: User who submitted request for resource. - If auth is enabled, this cannot be None for JWT-protected resources - which are decorated with @auth_required or @admin_required. + :return: ServiceX user who submitted request for resource. + Returns None if auth or user management is disabled. """ - user = None - if current_app.config.get('ENABLE_AUTH'): - user = UserModel.find_by_sub(get_jwt_identity()) - return user + config = current_app.config + if not config.get('ENABLE_AUTH') or config.get('DISABLE_USER_MGMT'): + return None + sub = get_jwt_identity() + if sub is not None: + return UserModel.find_by_sub(sub) @classmethod def _get_app_version(cls): diff --git a/servicex/resources/users/token_refresh.py b/servicex/resources/users/token_refresh.py index d61677ad..a72c10ad 100644 --- a/servicex/resources/users/token_refresh.py +++ b/servicex/resources/users/token_refresh.py @@ -26,6 +26,7 @@ # OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +from flask import current_app from flask_restful import Resource from flask_jwt_extended import (create_access_token, get_raw_jwt, decode_token, jwt_refresh_token_required, get_jwt_identity) @@ -35,11 +36,12 @@ class TokenRefresh(Resource): @jwt_refresh_token_required def post(self): - user = UserModel.find_by_sub(get_jwt_identity()) - claims = get_raw_jwt() - decoded = decode_token(user.refresh_token) - if not claims['jti'] == decoded['jti']: - return {'message': 'Invalid or outdated refresh token'}, 401 + if not current_app.config.get('DISABLE_USER_MGMT'): + user = UserModel.find_by_sub(get_jwt_identity()) + claims = get_raw_jwt() + decoded = decode_token(user.refresh_token) + if not claims['jti'] == decoded['jti']: + return {'message': 'Invalid or outdated refresh token'}, 401 current_user = get_jwt_identity() access_token = create_access_token(identity=current_user) return {'access_token': access_token} diff --git a/servicex/templates/base.html b/servicex/templates/base.html index a6d5391d..1714b6f0 100644 --- a/servicex/templates/base.html +++ b/servicex/templates/base.html @@ -39,7 +39,7 @@ {% endif %} - {% if config['ENABLE_AUTH'] %} + {% if config['ENABLE_AUTH'] and not config['DISABLE_USER_MGMT'] %}
{% if not session['is_authenticated'] %} Sign In diff --git a/tests/resource_test_base.py b/tests/resource_test_base.py index f213e7ab..bd1895cd 100644 --- a/tests/resource_test_base.py +++ b/tests/resource_test_base.py @@ -145,7 +145,10 @@ def mock_requesting_user(self, mocker): mock_user.admin = False mock_user.pending = False mocker.patch( - 'servicex.resources.servicex_resource.UserModel.find_by_sub', + 'servicex.decorators.UserModel.find_by_sub', return_value=mock_user + ) + mocker.patch( + 'servicex.resources.servicex_resource.ServiceXResource.get_requesting_user', return_value=mock_user) return mock_user From 1ee4aeb75b319e98ed39da1ee384ee971db83d8c Mon Sep 17 00:00:00 2001 From: Andrew Eckart Date: Tue, 4 May 2021 23:13:42 -0500 Subject: [PATCH 2/6] Add unit test for ServiceX.get_requesting_user --- tests/resources/test_servicex_resource.py | 35 ++++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/tests/resources/test_servicex_resource.py b/tests/resources/test_servicex_resource.py index b1eeab3c..3c9776d8 100644 --- a/tests/resources/test_servicex_resource.py +++ b/tests/resources/test_servicex_resource.py @@ -27,15 +27,42 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import pkg_resources +from pytest import fixture + +from servicex.resources.servicex_resource import ServiceXResource +from tests.resource_test_base import ResourceTestBase + + +class TestServiceXResource(ResourceTestBase): + module = ServiceXResource.__module__ + + @fixture + def mock_user(self, mocker): + return mocker.patch(f"{self.module}.UserModel.find_by_sub").return_value -class TestServiceXResource: def test_get_app_version_no_servicex_app(self, mocker): mock_get_distribution = mocker.patch( - "servicex.resources.servicex_resource.pkg_resources.get_distribution", - side_effect=pkg_resources.DistributionNotFound(None, None)) + f"{self.module}.pkg_resources.get_distribution", + side_effect=pkg_resources.DistributionNotFound(None, None) + ) - from servicex.resources.servicex_resource import ServiceXResource version = ServiceXResource._get_app_version() mock_get_distribution.assert_called_with("servicex_app") assert version == 'develop' + + def test_get_requesting_user_no_auth(self, client): + with client.application.app_context(): + assert ServiceXResource.get_requesting_user() is None + + def test_get_requesting_user_with_auth(self, mocker, mock_user): + mocker.patch(f"{self.module}.get_jwt_identity").return_value = "abcd" + client = self._test_client(extra_config={'ENABLE_AUTH': True}) + with client.application.app_context(): + assert ServiceXResource.get_requesting_user() == mock_user + + def test_get_requesting_user_no_identity(self, mocker): + mocker.patch(f"{self.module}.get_jwt_identity").return_value = None + client = self._test_client(extra_config={'ENABLE_AUTH': True}) + with client.application.app_context(): + assert ServiceXResource.get_requesting_user() is None From 06d86c9f5977b071cd4d3cc7e3d35c227f95bf89 Mon Sep 17 00:00:00 2001 From: Andrew Eckart Date: Wed, 5 May 2021 00:37:43 -0500 Subject: [PATCH 3/6] Add unit tests for TokenRefresh resource --- tests/resource_test_base.py | 5 +- tests/resources/users/test_token_refresh.py | 55 +++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 tests/resources/users/test_token_refresh.py diff --git a/tests/resource_test_base.py b/tests/resource_test_base.py index bd1895cd..a9d00783 100644 --- a/tests/resource_test_base.py +++ b/tests/resource_test_base.py @@ -28,6 +28,7 @@ from unittest.mock import MagicMock +from flask.testing import FlaskClient from pytest import fixture from servicex import create_app @@ -78,7 +79,7 @@ def _test_client( code_gen_service=MagicMock(CodeGenAdapter), lookup_result_processor=MagicMock(LookupResultProcessor), docker_repo_adapter=None - ): + ) -> FlaskClient: config = ResourceTestBase._app_config() config['TRANSFORMER_MANAGER_ENABLED'] = False config['TRANSFORMER_MANAGER_MODE'] = 'external' @@ -99,7 +100,7 @@ def _test_client( return app.test_client() @fixture - def client(self): + def client(self) -> FlaskClient: return self._test_client() @staticmethod diff --git a/tests/resources/users/test_token_refresh.py b/tests/resources/users/test_token_refresh.py new file mode 100644 index 00000000..82a4b375 --- /dev/null +++ b/tests/resources/users/test_token_refresh.py @@ -0,0 +1,55 @@ +from types import SimpleNamespace +from unittest.mock import MagicMock + +from flask import Response +from pytest import fixture + +from tests.resource_test_base import ResourceTestBase + + +class TestTokenRefresh(ResourceTestBase): + module = 'servicex.resources.users.token_refresh' + endpoint = '/token/refresh' + fake_token = 'abcd' + + @fixture(autouse=True, scope="class") + def unwrap(self): + """Remove the @jwt_refresh_token_required decorator.""" + from servicex.resources.users.token_refresh import TokenRefresh + TokenRefresh.post = TokenRefresh.post.__wrapped__ + + @fixture + def jwt_funcs(self, mocker) -> SimpleNamespace: + m = self.module + patch = mocker.patch + sub = "janedoe@example.com" + return SimpleNamespace( + get_jwt_identity=patch(f"{m}.get_jwt_identity", return_value=sub), + create_access_token=patch(f"{m}.create_access_token", return_value=self.fake_token), + get_raw_jwt=patch(f"{m}.get_raw_jwt", return_value={"jti": "1234"}), + decode_token=mocker.patch(f"{m}.decode_token") + ) + + @fixture + def mock_user(self, mocker) -> MagicMock: + return mocker.patch(f"{self.module}.UserModel.find_by_sub").return_value + + def make_request(self, client): + response: Response = client.post(self.endpoint) + assert response.status_code == 200 + assert response.json == {'access_token': self.fake_token} + + def test_post_valid_refresh_token(self, client, jwt_funcs, mock_user): + jwt_funcs.decode_token.return_value = jwt_funcs.get_raw_jwt.return_value + self.make_request(client) + + def test_post_invalid_refresh_token(self, client, jwt_funcs, mock_user): + jwt_funcs.decode_token.return_value = {"jti": "this value will not match"} + response: Response = client.post(self.endpoint) + assert response.status_code == 401 + assert response.json == {"message": "Invalid or outdated refresh token"} + + def test_post_user_mgmt_disabled(self, jwt_funcs): + client = self._test_client(extra_config={'DISABLE_USER_MGMT': True}) + self.make_request(client) + jwt_funcs.get_jwt_identity.assert_called_once() From 32217fb32e11fc1157f909507c93e63e467b3625 Mon Sep 17 00:00:00 2001 From: Andrew Eckart Date: Wed, 5 May 2021 01:20:05 -0500 Subject: [PATCH 4/6] Show global dashboard link when user management is disabled --- servicex/templates/base.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/servicex/templates/base.html b/servicex/templates/base.html index 1714b6f0..1dbdd05c 100644 --- a/servicex/templates/base.html +++ b/servicex/templates/base.html @@ -34,7 +34,7 @@