From cccff9b8567355035fd17e0f0db6a3a3a90a2912 Mon Sep 17 00:00:00 2001 From: Aaron Louie Date: Thu, 11 Jun 2020 11:29:58 -0400 Subject: [PATCH] Fixes broken unit tests. But still broken. --- Pipfile.lock | 36 +++++----- config/default.py | 2 +- crc/api.yml | 18 ++--- crc/api/user.py | 56 +++++++++------ crc/models/user.py | 8 +-- tests/base_test.py | 1 - tests/test_authentication.py | 130 ++++++++++++++++++++++++----------- 7 files changed, 156 insertions(+), 95 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index f8ab746b..fb38d03c 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -104,17 +104,17 @@ }, "celery": { "hashes": [ - "sha256:9ae2e73b93cc7d6b48b56aaf49a68c91752d0ffd7dfdcc47f842ca79a6f13eae", - "sha256:c2037b6a8463da43b19969a0fc13f9023ceca6352b4dd51be01c66fbbb13647e" + "sha256:c3f4173f83ceb5a5c986c5fdaefb9456de3b0729a72a5776e46bd405fda7b647", + "sha256:d1762d6065522879f341c3d67c2b9fe4615eb79756d59acb1434601d4aca474b" ], - "version": "==4.4.4" + "version": "==4.4.5" }, "certifi": { "hashes": [ - "sha256:1d987a998c75633c40847cc966fcf5904906c920a7f17ef374f5aa4282abd304", - "sha256:51fcb31174be6e6664c5f69e3e1691a2d72a1a12e90f872cbdb1567eb47b6519" + "sha256:5ad7e9a056d25ffa5082862e36f119f7f7cec6457fa07ee2f8c339814b80c9b1", + "sha256:9cd41137dc19af6a5e03b630eefe7d1f458d964d406342dd3edf625839b944cc" ], - "version": "==2020.4.5.1" + "version": "==2020.4.5.2" }, "cffi": { "hashes": [ @@ -285,11 +285,11 @@ }, "flask-marshmallow": { "hashes": [ - "sha256:6e6aec171b8e092e0eafaf035ff5b8637bf3a58ab46f568c4c1bab02f2a3c196", - "sha256:a1685536e7ab5abdc712bbc1ac1a6b0b50951a368502f7985e7d1c27b3c21e59" + "sha256:1da1e6454a56a3e15107b987121729f152325bdef23f3df2f9b52bbd074af38e", + "sha256:aefc1f1d96256c430a409f08241bab75ffe97e5d14ac5d1f000764e39bf4873a" ], "index": "pypi", - "version": "==0.12.0" + "version": "==0.13.0" }, "flask-migrate": { "hashes": [ @@ -359,10 +359,10 @@ }, "inflection": { "hashes": [ - "sha256:32a5c3341d9583ec319548b9015b7fbdf8c429cbcb575d326c33ae3a0e90d52c", - "sha256:9a15d3598f01220e93f2207c432cfede50daff53137ce660fb8be838ef1ca6cc" + "sha256:88b101b2668a1d81d6d72d4c2018e53bc6c7fc544c987849da1c7f77545c3bc9", + "sha256:f576e85132d34f5bf7df5183c2c6f94cfb32e528f53065345cf71329ba0b8924" ], - "version": "==0.4.0" + "version": "==0.5.0" }, "itsdangerous": { "hashes": [ @@ -751,11 +751,11 @@ }, "sphinx": { "hashes": [ - "sha256:779a519adbd3a70fc7c468af08c5e74829868b0a5b34587b33340e010291856c", - "sha256:ea64df287958ee5aac46be7ac2b7277305b0381d213728c3a49d8bb9b8415807" + "sha256:1c445320a3310baa5ccb8d957267ef4a0fc930dc1234db5098b3d7af14fbb242", + "sha256:7d3d5087e39ab5a031b75588e9859f011de70e213cd0080ccbc28079fb0786d1" ], "index": "pypi", - "version": "==3.0.4" + "version": "==3.1.0" }, "sphinxcontrib-applehelp": { "hashes": [ @@ -990,10 +990,10 @@ }, "wcwidth": { "hashes": [ - "sha256:980fbf4f3c196c0f329cdcd1e84c554d6a211f18e252e525a0cf4223154a41d6", - "sha256:edbc2b718b4db6cdf393eefe3a420183947d6aa312505ce6754516f458ff8830" + "sha256:79375666b9954d4a1a10739315816324c3e73110af9d0e102d906fdb0aec009f", + "sha256:8c6b5b6ee1360b842645f336d9e5d68c55817c26d3050f46b235ef2bc650e48f" ], - "version": "==0.2.3" + "version": "==0.2.4" }, "zipp": { "hashes": [ diff --git a/config/default.py b/config/default.py index 44e6cb3d..93e4a933 100644 --- a/config/default.py +++ b/config/default.py @@ -29,7 +29,7 @@ SQLALCHEMY_DATABASE_URI = environ.get( 'SQLALCHEMY_DATABASE_URI', default="postgresql://%s:%s@%s:%s/%s" % (DB_USER, DB_PASSWORD, DB_HOST, DB_PORT, DB_NAME) ) -TOKEN_AUTH_TTL_HOURS = int(environ.get('TOKEN_AUTH_TTL_HOURS', default=4)) +TOKEN_AUTH_TTL_HOURS = float(environ.get('TOKEN_AUTH_TTL_HOURS', default=24)) TOKEN_AUTH_SECRET_KEY = environ.get('TOKEN_AUTH_SECRET_KEY', default="Shhhh!!! This is secret! And better darn well not show up in prod.") FRONTEND_AUTH_CALLBACK = environ.get('FRONTEND_AUTH_CALLBACK', default="http://localhost:4200/session") SWAGGER_AUTH_KEY = environ.get('SWAGGER_AUTH_KEY', default="SWAGGER") diff --git a/crc/api.yml b/crc/api.yml index a5f92212..64f6086a 100644 --- a/crc/api.yml +++ b/crc/api.yml @@ -115,7 +115,7 @@ paths: delete: operationId: crc.api.study.delete_study security: - - jwt_admin: ['secret'] + - auth_admin: ['secret'] summary: Removes the given study completely. tags: - Studies @@ -218,7 +218,7 @@ paths: put: operationId: crc.api.workflow.update_workflow_specification security: - - jwt_admin: ['secret'] + - auth_admin: ['secret'] summary: Modifies an existing workflow specification with the given parameters. tags: - Workflow Specifications @@ -237,7 +237,7 @@ paths: delete: operationId: crc.api.workflow.delete_workflow_specification security: - - jwt_admin: ['secret'] + - auth_admin: ['secret'] summary: Removes an existing workflow specification tags: - Workflow Specifications @@ -284,7 +284,7 @@ paths: post: operationId: crc.api.workflow.add_workflow_spec_category security: - - jwt_admin: ['secret'] + - auth_admin: ['secret'] summary: Creates a new workflow spec category with the given parameters. tags: - Workflow Specification Category @@ -323,7 +323,7 @@ paths: put: operationId: crc.api.workflow.update_workflow_spec_category security: - - jwt_admin: ['secret'] + - auth_admin: ['secret'] summary: Modifies an existing workflow spec category with the given parameters. tags: - Workflow Specification Category @@ -342,7 +342,7 @@ paths: delete: operationId: crc.api.workflow.delete_workflow_spec_category security: - - jwt_admin: ['secret'] + - auth_admin: ['secret'] summary: Removes an existing workflow spec category tags: - Workflow Specification Category @@ -543,7 +543,7 @@ paths: put: operationId: crc.api.file.set_reference_file security: - - jwt_admin: ['secret'] + - auth_admin: ['secret'] summary: Update the contents of a named reference file. tags: - Files @@ -603,7 +603,7 @@ paths: delete: operationId: crc.api.workflow.delete_workflow security: - - jwt_admin: ['secret'] + - auth_admin: ['secret'] summary: Removes an existing workflow tags: - Workflows and Tasks @@ -924,7 +924,7 @@ components: scheme: bearer bearerFormat: JWT x-bearerInfoFunc: crc.api.user.verify_token - jwt_admin: + auth_admin: type: http scheme: bearer bearerFormat: JWT diff --git a/crc/api/user.py b/crc/api/user.py index 3cf13c9f..0786fbc9 100644 --- a/crc/api/user.py +++ b/crc/api/user.py @@ -4,8 +4,7 @@ from flask import g, request from crc import app, db from crc.api.common import ApiError from crc.models.user import UserModel, UserModelSchema -from crc.services.ldap_service import LdapService, LdapModel, LdapUserInfo -from crc.services.approval_service import ApprovalService +from crc.services.ldap_service import LdapService, LdapModel """ .. module:: crc.api.user @@ -31,7 +30,8 @@ def verify_token(token=None): print('=== verify_token ===') print('_is_production()', _is_production()) - failure_error = ApiError("invalid_token", "Unable to decode the token you provided. Please re-authenticate", status_code=403) + failure_error = ApiError("invalid_token", "Unable to decode the token you provided. Please re-authenticate", + status_code=403) if not _is_production(): g.user = UserModel.query.first() @@ -62,7 +62,8 @@ def verify_token(token=None): return token_info else: - ApiError("no_user", "User not found. Please login via the frontend app before accessing this feature.", status_code=403) + ApiError("no_user", "User not found. Please login via the frontend app before accessing this feature.", + status_code=403) raise failure_error @@ -81,7 +82,6 @@ def verify_token_admin(token=None): print('=== verify_token_admin ===') print('_is_production()', _is_production()) - # If this is production, check that the user is in the list of admins if _is_production(): uid = _get_request_uid(request) @@ -101,8 +101,8 @@ def get_current_user(): def login( - uid=None, - redirect_url=None, + uid=None, + redirect_url=None, ): """ In non-production environment, provides an endpoint for end-to-end system testing that allows the system @@ -175,7 +175,7 @@ def sso(): return response -def _handle_login(user_info: LdapUserInfo, redirect_url=None): +def _handle_login(user_info: LdapModel, redirect_url=None): """ On successful login, adds user to database if the user is not already in the system, then returns the frontend auth callback URL, with auth token appended. @@ -187,22 +187,10 @@ def _handle_login(user_info: LdapUserInfo, redirect_url=None): Returns: Response. 302 - Redirects to the frontend auth callback URL, with auth token appended. """ + print('=== _handle_login ===') print('user_info', user_info) - user = db.session.query(UserModel).filter(UserModel.uid == user_info.uid).first() - - if user is None: - # Add new user - user = UserModel() - - user.uid = user_info.uid - user.display_name = user_info.display_name - user.email_address = user_info.email_address - user.affiliation = user_info.affiliation - user.title = user_info.title - - db.session.add(user) - db.session.commit() + user = _upsert_user(user_info) # Return the frontend auth callback URL, with auth token appended. auth_token = user.encode_auth_token().decode() @@ -217,11 +205,35 @@ def _handle_login(user_info: LdapUserInfo, redirect_url=None): return auth_token +def _upsert_user(user_info): + user = db.session.query(UserModel).filter(UserModel.uid == user_info.uid).first() + + if user is None: + # Add new user + user = UserModel() + else: + user = db.session.query(UserModel).filter(UserModel.uid == user_info.uid).with_for_update().first() + + user.uid = user_info.uid + user.display_name = user_info.display_name + user.email_address = user_info.email_address + user.affiliation = user_info.affiliation + user.title = user_info.title + + db.session.add(user) + db.session.commit() + return user + + def _get_request_uid(req): uid = None if _is_production(): + if 'user' in g and g.user is not None: + print('g.user.uid', g.user.uid) + return g.user.uid + print('req.headers', req.headers) uid = req.headers.get("Uid") if not uid: diff --git a/crc/models/user.py b/crc/models/user.py index d9ee8f72..67e85967 100644 --- a/crc/models/user.py +++ b/crc/models/user.py @@ -27,7 +27,7 @@ class UserModel(db.Model): Generates the Auth Token :return: string """ - hours = int(app.config['TOKEN_AUTH_TTL_HOURS']) + hours = float(app.config['TOKEN_AUTH_TTL_HOURS']) payload = { 'exp': datetime.datetime.utcnow() + datetime.timedelta(hours=hours, minutes=0, seconds=0), 'iat': datetime.datetime.utcnow(), @@ -36,7 +36,7 @@ class UserModel(db.Model): return jwt.encode( payload, app.config.get('TOKEN_AUTH_SECRET_KEY'), - algorithm='HS256' + algorithm='HS256', ) @staticmethod @@ -50,9 +50,9 @@ class UserModel(db.Model): payload = jwt.decode(auth_token, app.config.get('TOKEN_AUTH_SECRET_KEY'), algorithms='HS256') return payload except jwt.ExpiredSignatureError: - raise ApiError('token_expired', 'The Authentication token you provided expired, and must be renewed.') + raise ApiError('token_expired', 'The Authentication token you provided expired and must be renewed.') except jwt.InvalidTokenError: - raise ApiError('token_invalid', 'The Authentication token you provided. You need a new token. ') + raise ApiError('token_invalid', 'The Authentication token you provided is invalid. You need a new token. ') class UserModelSchema(SQLAlchemyAutoSchema): diff --git a/tests/base_test.py b/tests/base_test.py index 07a1bd37..5c9e7dc1 100644 --- a/tests/base_test.py +++ b/tests/base_test.py @@ -95,7 +95,6 @@ class BaseTest(unittest.TestCase): def tearDown(self): ExampleDataLoader.clean_db() - session.flush() self.auths = {} def logged_in_headers(self, user=None, redirect_url='http://some/frontend/url'): diff --git a/tests/test_authentication.py b/tests/test_authentication.py index ff720b57..d18614df 100644 --- a/tests/test_authentication.py +++ b/tests/test_authentication.py @@ -1,9 +1,10 @@ import json -from datetime import timezone, datetime +from calendar import timegm +from datetime import timezone, datetime, timedelta from tests.base_test import BaseTest from crc import db, app -from crc.models.study import StudySchema +from crc.models.study import StudySchema, StudyModel from crc.models.user import UserModel from crc.models.protocol_builder import ProtocolBuilderStatus @@ -16,11 +17,29 @@ class TestAuthentication(BaseTest): super().tearDown() def test_auth_token(self): + # Save the orginal timeout setting + orig_ttl = float(app.config['TOKEN_AUTH_TTL_HOURS']) + self.load_example_data() + + # Set the timeout to something else + new_ttl = 4.0 + app.config['TOKEN_AUTH_TTL_HOURS'] = new_ttl user = UserModel(uid="dhf8r") - auth_token = user.encode_auth_token() - self.assertTrue(isinstance(auth_token, bytes)) - self.assertEqual("dhf8r", user.decode_auth_token(auth_token).get("sub")) + expected_exp_1 = timegm((datetime.utcnow() + timedelta(hours=new_ttl)).utctimetuple()) + auth_token_1 = user.encode_auth_token() + self.assertTrue(isinstance(auth_token_1, bytes)) + self.assertEqual("dhf8r", user.decode_auth_token(auth_token_1).get("sub")) + actual_exp_1 = user.decode_auth_token(auth_token_1).get("exp") + self.assertTrue(expected_exp_1 - 1000 <= actual_exp_1 <= expected_exp_1 + 1000) + + # Set the timeout back to where it was + app.config['TOKEN_AUTH_TTL_HOURS'] = orig_ttl + expected_exp_2 = timegm((datetime.utcnow() + timedelta(hours=new_ttl)).utctimetuple()) + auth_token_2 = user.encode_auth_token() + self.assertTrue(isinstance(auth_token_2, bytes)) + actual_exp_2 = user.decode_auth_token(auth_token_1).get("exp") + self.assertTrue(expected_exp_2 - 1000 <= actual_exp_2 <= expected_exp_2 + 1000) def test_non_production_auth_creates_user(self): new_uid = 'lb3dp' ## Assure this user id is in the fake responses from ldap. @@ -48,12 +67,14 @@ class TestAuthentication(BaseTest): self.assertTrue(str.startswith(rv_2.location, redirect_url)) def test_production_auth_creates_user(self): + # Switch production mode on app.config['PRODUCTION'] = True - new_uid = 'lb3dp' # This user is in the test ldap system. self.load_example_data() - user = db.session.query(UserModel).filter(UserModel.uid == new_uid).first() + + new_uid = 'lb3dp' # This user is in the test ldap system. + user = db.session.query(UserModel).filter_by(uid=new_uid).first() self.assertIsNone(user) redirect_url = 'http://worlds.best.website/admin' headers = dict(Uid=new_uid) @@ -61,7 +82,7 @@ class TestAuthentication(BaseTest): rv = self.app.get('v1.0/login', follow_redirects=False, headers=headers) self.assert_success(rv) - user = db.session.query(UserModel).filter(UserModel.uid == new_uid).first() + user = db.session.query(UserModel).filter_by(uid=new_uid).first() self.assertIsNotNone(user) self.assertEqual(new_uid, user.uid) self.assertEqual("Laura Barnes", user.display_name) @@ -70,6 +91,14 @@ class TestAuthentication(BaseTest): # Switch production mode back off app.config['PRODUCTION'] = False + db.session.flush() + db.session.flush() + db.session.flush() + db.session.flush() + db.session.flush() + db.session.flush() + db.session.flush() + db.session.flush() def test_current_user_status(self): self.load_example_data() @@ -84,49 +113,65 @@ class TestAuthentication(BaseTest): rv = self.app.get('/v1.0/user', headers=self.logged_in_headers(user, redirect_url='http://omg.edu/lolwut')) self.assert_success(rv) - def test_admin_only_endpoints(self): + def test_admin_can_access_admin_only_endpoints(self): + # Switch production mode on app.config['PRODUCTION'] = True + self.load_example_data() admin_uids = app.config['ADMIN_UIDS'] self.assertGreater(len(admin_uids), 0) + admin_uid = admin_uids[0] + self.assertEqual(admin_uid, 'dhf8r') # This user is in the test ldap system. + admin_headers = dict(Uid=admin_uid) - for uid in admin_uids: - admin_headers = dict(Uid=uid) + rv = self.app.get('v1.0/login', follow_redirects=False, headers=admin_headers) + self.assert_success(rv) - rv = self.app.get( - 'v1.0/login', - follow_redirects=False, - headers=admin_headers - ) - self.assert_success(rv) + admin_user = db.session.query(UserModel).filter(UserModel.uid == admin_uid).first() + self.assertIsNotNone(admin_user) + self.assertEqual(admin_uid, admin_user.uid) - admin_user = db.session.query(UserModel).filter_by(uid=uid).first() - self.assertIsNotNone(admin_user) + admin_study = self._make_fake_study(admin_uid) - admin_study = self._make_fake_study(uid) + admin_token_headers = dict(Authorization='Bearer ' + admin_user.encode_auth_token().decode()) - rv_add_study = self.app.post( - '/v1.0/study', - content_type="application/json", - headers=admin_headers, - data=json.dumps(StudySchema().dump(admin_study)) - ) - self.assert_success(rv_add_study, 'Admin user should be able to add a study') + rv_add_study = self.app.post( + '/v1.0/study', + content_type="application/json", + headers=admin_token_headers, + data=json.dumps(StudySchema().dump(admin_study)), + follow_redirects=False + ) + self.assert_success(rv_add_study, 'Admin user should be able to add a study') - new_study = json.loads(rv.get_data(as_text=True)) + new_admin_study = json.loads(rv_add_study.get_data(as_text=True)) + db_admin_study = db.session.query(StudyModel).filter_by(id=new_admin_study['id']).first() + self.assertIsNotNone(db_admin_study) - rv_del_study = self.app.delete( - '/v1.0/study/%i' % new_study.id, - follow_redirects=False, - headers=admin_headers - ) - self.assert_success(rv_del_study, 'Admin user should be able to delete a study') + rv_del_study = self.app.delete( + '/v1.0/study/%i' % db_admin_study.id, + follow_redirects=False, + headers=admin_token_headers + ) + self.assert_success(rv_del_study, 'Admin user should be able to delete a study') + # Switch production mode back off + app.config['PRODUCTION'] = False + + def test_nonadmin_cannot_access_admin_only_endpoints(self): + # Switch production mode on + app.config['PRODUCTION'] = True + + self.load_example_data() # Non-admin user should not be able to delete a study non_admin_uid = 'lb3dp' + admin_uids = app.config['ADMIN_UIDS'] + self.assertGreater(len(admin_uids), 0) + self.assertNotIn(non_admin_uid, admin_uids) + non_admin_headers = dict(Uid=non_admin_uid) rv = self.app.get( @@ -138,24 +183,29 @@ class TestAuthentication(BaseTest): non_admin_user = db.session.query(UserModel).filter_by(uid=non_admin_uid).first() self.assertIsNotNone(non_admin_user) + + non_admin_token_headers = dict(Authorization='Bearer ' + non_admin_user.encode_auth_token().decode()) + non_admin_study = self._make_fake_study(non_admin_uid) rv_add_study = self.app.post( '/v1.0/study', content_type="application/json", - headers=non_admin_headers, + headers=non_admin_token_headers, data=json.dumps(StudySchema().dump(non_admin_study)) ) self.assert_success(rv_add_study, 'Non-admin user should be able to add a study') - new_study = json.loads(rv.get_data(as_text=True)) + new_non_admin_study = json.loads(rv_add_study.get_data(as_text=True)) + db_non_admin_study = db.session.query(StudyModel).filter_by(id=new_non_admin_study['id']).first() + self.assertIsNotNone(db_non_admin_study) - rv_del_study = self.app.delete( - '/v1.0/study/%i' % new_study.id, + rv_non_admin_del_study = self.app.delete( + '/v1.0/study/%i' % db_non_admin_study.id, follow_redirects=False, - headers=non_admin_headers + headers=non_admin_token_headers ) - self.assert_failure(rv_del_study, 'Non-admin user should not be able to delete a study') + self.assert_failure(rv_non_admin_del_study, 401) # Switch production mode back off app.config['PRODUCTION'] = False