Security fixes

ptmd/api/queries/files/delete.py

@@ -3,6 +3,7 @@
 
 from flask import jsonify, Response
 
+from ptmd.logger import LOGGER
 from ptmd.database.models import File
 from ptmd.api.queries.utils import check_role
 
@@ -23,4 +24,5 @@ def delete_file(file_id: int) -> tuple[Response, int]:
  file.remove()
  return jsonify({"message": f"File {file_id} was successfully deleted."}), 200
  except PermissionError as e:
- return jsonify({"message": str(e)}), 403
+ LOGGER.error("Permission Error: %s" % (str(e)))
+ return jsonify({"message": 'You do not have permission to perform this action.'}), 403

ptmd/api/queries/files/isa.py

@@ -3,6 +3,7 @@
 
 from flask import jsonify, Response
 
+from ptmd.logger import LOGGER
 from ptmd.lib.isa import convert_file_to_isa
 from ptmd.api.queries.utils import check_role
 
@@ -17,7 +18,9 @@ def convert_to_isa(file_id: int) -> tuple[Response, int]:
  try:
  response: dict = convert_file_to_isa(file_id)[0]
  except ValueError as e:
- return jsonify({'message': str(e)}), 400
+ LOGGER.error("ValueError: %s" % (str(e)))
+ return jsonify({'message': 'File conversion failed.'}), 400
  except FileNotFoundError as e:
- return jsonify({'message': str(e)}), 404
+ LOGGER.error("File not found: %s" % (str(e)))
+ return jsonify({'message': 'File not found.'}), 404
  return jsonify(response), 200

ptmd/api/queries/files/register.py

@@ -8,6 +8,7 @@
 from flask import jsonify, Response, request
 from flask_jwt_extended import get_current_user
 
+from ptmd.logger import LOGGER
 from ptmd.config import session
 from ptmd.database import Organisation, File, get_shipped_file
 from ptmd.lib import BatchUpdater, GoogleDriveConnector
@@ -60,7 +61,7 @@
  file_data: dict[str, str] | None = connector.upload_file(organisation.gdrive_id, filepath, filename)
  remove(filepath)
  if not file_data:
- raise ValueError(f"File '{file_id}' could not be uploaded.")
+ raise ValueError(f"File '{file_id}' could not be uploaded.") # TODO: Test this
 
  file: File = File(gdrive_id=file_data['id'],
  name=filename,
@@ -71,8 +72,11 @@
  session.commit()
  msg: str = f'file {file_id} was successfully created with internal id {file.file_id}'
  return jsonify({"message": msg, "file": dict(file)}), 201
- except Exception as e:
+ except ValueError as e:
  return jsonify({"message": str(e)}), 400
+ except Exception as e:
+ LOGGER.error("Registration error: %s" % (str(e)))
+ return jsonify({"message": 'An unexpected error occurred.'}), 500
 
 
 def change_batch(new_batch: str, species: str, filepath: str, filename: str) -> tuple[Response, int] | str:

ptmd/api/queries/files/shipment.py

@@ -4,6 +4,7 @@
 
 from flask import jsonify, Response, request
 
+from ptmd.logger import LOGGER
 from ptmd.database import File, get_shipped_file
 from ptmd.database.queries.users import email_admins_file_shipped
 from ptmd.api.queries.utils import check_role
@@ -34,7 +35,8 @@ def ship_data(file_id: int) -> tuple[Response, int]:
  except PermissionError:
  return jsonify({'message': f'File {file_id} could not be locked but has been sent anyway'}), 200
  except ValueError as e:
- return jsonify({'message': str(e)}), 400
+ LOGGER.error("Value error: %s" % (str(e)))
+ return jsonify({'message': 'File is not in the correct state.'}), 400
  except Exception:
  session.rollback()
  return jsonify({'message': 'An unknown error occurred. Sorry.'}), 500
@@ -58,9 +60,11 @@ def receive_data(file_id: int) -> tuple[Response, int]:
  except BatchError as e:
  return e.serialize()
  except PermissionError as e:
- return jsonify({'message': str(e)}), 403
+ LOGGER.error("User is not an admin: %s" % (str(e)))
+ return jsonify({'message': 'You do not have permission to perform this action.'}), 403
  except ValueError as e:
- return jsonify({'message': str(e)}), 400
+ LOGGER.error("File not in correct state: %s" % (str(e)))
+ return jsonify({'message': 'File is not in the correct state.'}), 400
 
 
 def validate_batch(file_id: int, new_batch: str | None = None) -> File:

ptmd/api/queries/users.py

@@ -11,6 +11,7 @@
 from sqlalchemy.exc import IntegrityError
 from jsonschema import Draft4Validator as Validator
 
+from ptmd.logger import LOGGER
 from ptmd.config import session
 from ptmd.const import CREATE_USER_SCHEMA_PATH
 from ptmd.database import login_user, get_token, User, Token, Organisation
@@ -144,7 +145,8 @@ def enable_account(token: str) -> tuple[Response, int]:
  user.set_role('enabled')
  return jsonify(msg="Account enabled. An email has been to an admin to validate your account."), 200
  except Exception as e:
- return jsonify(msg=str(e)), 400
+ LOGGER.error("Account not enabled: %s" % (str(e)))
+ return jsonify(msg='Failed to enable your account. Please contact an admin for assistance.'), 400
 
 
 @check_role(role='admin')

tests/test_api/test_queries/test_files/test_isa.py

@@ -29,7 +29,7 @@ def test_convert_error_400(self, mock_jwt_verify_flask, mock_jwt_verify_utils, m
  mock_convert.side_effect = ValueError('A 400 error.')
  with app.test_client() as client:
  response = client.get('/api/files/1/isa', headers=HEADERS)
- self.assertEqual(response.json, {'message': 'A 400 error.'})
+ self.assertEqual(response.json, {'message': 'File conversion failed.'})
  self.assertEqual(response.status_code, 400)
 
  def test_convert_success(self, mock_jwt_verify_flask, mock_jwt_verify_utils, mock_user):

tests/test_api/test_queries/test_files/test_register.py

@@ -266,3 +266,35 @@ def test_change_batch_error_412_success(self, mock_shipped_file, mock_batch_upda
  mock_rm.assert_not_called()
  mock_batch_updated.assert_called_with(batch='AA', filepath='filepath')
  self.assertEqual(response, 'filenameAA')
+
+ @patch('ptmd.api.queries.files.register.session')
+ @patch('ptmd.api.queries.files.register.GoogleDriveConnector', return_value=MockGoogleDriveError())
+ @patch('flask_jwt_extended.view_decorators.verify_jwt_in_request')
+ @patch('ptmd.api.queries.files.register.File')
+ @patch('ptmd.api.queries.utils.verify_jwt_in_request')
+ @patch('ptmd.api.queries.utils.get_current_user')
+ @patch('ptmd.api.queries.files.register.Organisation')
+ @patch('ptmd.api.queries.files.register.get_current_user')
+ @patch('ptmd.api.queries.files.register.extract_data_from_spreadsheet')
+ @patch('ptmd.api.queries.files.register.remove')
+ @patch('ptmd.api.queries.files.register.get_shipped_file')
+ def test_register_file_exception_upload(self, mocked_shipped_file, mock_rm, mock_data, mock_user, mock_organisation,
+ mock_get_user, mock_jwt_in_request, mock_file,
+ mock_verify_jwt, mock_gdrive, mock_session):
+ mock_get_user().role = 'admin'
+ mock_file.return_value.file_id = '123'
+ mocked_shipped_file.return_value = None
+
+ mock_organisation.query.filter().first.return_value.gdrive_id = '123'
+ mock_organisation.query.filter().first.return_value.name = 'test'
+ mock_user().id = 1
+ mocked_shipped_file.side_effect = Exception()
+
+ with app.test_client() as client:
+ with patch('ptmd.api.queries.files.register.jsonify') as mock_jsonify:
+ external_file = {'file_id': '123', 'batch': 'AA', 'organism': 'human', 'partner': 'partner'}
+ response = client.post('/api/files/register',
+ headers={'Authorization': f'Bearer {123}', **HEADERS},
+ data=json_dumps(external_file))
+ mock_jsonify.assert_called_once_with({'message': 'An unexpected error occurred.'})
+ self.assertEqual(response.status_code, 500)

tests/test_api/test_queries/test_files/test_shipment.py

@@ -47,7 +47,7 @@ def ship_samples(self, at):
  mock_user().role = 'admin'
  with app.test_client() as client:
  response = client.post('/api/files/1/ship', headers=HEADERS, json={})
- self.assertEqual(response.json, {'message': 'A value error.'})
+ self.assertEqual(response.json, {'message': 'File is not in the correct state.'})
  self.assertEqual(response.status_code, 400)
 
  @patch('ptmd.api.queries.files.shipment.validate_batch')
@@ -143,7 +143,7 @@ def shipment_was_received(self, at):
  mock_user().role = 'admin'
  with app.test_client() as client:
  response = client.post('/api/files/1/receive', headers=HEADERS, json={})
- self.assertEqual(response.json, {'message': 'A value error.'})
+ self.assertEqual(response.json, {'message': 'File is not in the correct state.'})
  self.assertEqual(response.status_code, 400)
 
  @patch('ptmd.api.queries.files.shipment.validate_batch')

tests/test_api/test_queries/test_users.py

@@ -254,13 +254,19 @@ def test_enable_account_errors(self, mock_session_commit, mock_session_delete,
  response = client.get('/api/users/enable/2', headers={'Authorization': f'Bearer {123}', **HEADERS})
  mock_session_commit.assert_called_once()
  mock_session_delete.assert_called_once_with(mocked_token.query.filter().first.return_value)
- self.assertEqual(response.json, {'msg': 'Token expired'})
+ self.assertEqual(response.json,
+ {
+ 'msg': 'Failed to enable your account. Please contact an admin for assistance.'
+ })
  self.assertEqual(response.status_code, 400)
 
  mocked_token.query.filter().first.return_value = None
  with app.test_client() as client:
  response = client.get('/api/users/enable/2', headers={'Authorization': f'Bearer {123}', **HEADERS})
- self.assertEqual(response.json, {'msg': 'Invalid token'})
+ self.assertEqual(response.json,
+ {
+ 'msg': 'Failed to enable your account. Please contact an admin for assistance.'
+ })
  self.assertEqual(response.status_code, 400)
 
  @patch('ptmd.api.queries.users.User')