From efbee458649a703660e3d391a056573a07f45614 Mon Sep 17 00:00:00 2001 From: Kushal Das Date: Wed, 1 Apr 2020 16:14:35 +0530 Subject: [PATCH 1/2] Fixes #5176 Adds code+test to return replies without a journalist We can safely return a reply by a journalist who is deleted from the system. This patch also includes a test case, and dev-data generation script also now adds a reply and then deletes the journalist. --- securedrop/create-dev-data.py | 32 +++++++++++++++++++++---- securedrop/models.py | 17 +++++++++---- securedrop/tests/conftest.py | 17 +++++++++++++ securedrop/tests/test_journalist_api.py | 25 +++++++++++++++++++ securedrop/tests/utils/db_helper.py | 11 +++++++++ 5 files changed, 93 insertions(+), 9 deletions(-) diff --git a/securedrop/create-dev-data.py b/securedrop/create-dev-data.py index 8012e4d10f..fa1609089e 100755 --- a/securedrop/create-dev-data.py +++ b/securedrop/create-dev-data.py @@ -49,29 +49,48 @@ def main(staging=False): test_otp_secret, is_admin=False) + journalist_tobe_deleted = add_test_user("clarkkent", + test_password, + test_otp_secret, + is_admin=False, + first_name="Clark", + last_name="Kent") + # Add test sources and submissions num_sources = int(os.getenv('NUM_SOURCES', 2)) - for _ in range(num_sources): + for i in range(num_sources): + if i == 0: + # For the first source, the journalist who replied will be deleted + create_source_and_submissions(journalist_who_replied=journalist_tobe_deleted) + continue create_source_and_submissions() + # Now let us delete one journalist + db.session.delete(journalist_tobe_deleted) + db.session.commit() -def add_test_user(username, password, otp_secret, is_admin=False): +def add_test_user(username, password, otp_secret, is_admin=False, + first_name="", last_name=""): try: user = Journalist(username=username, password=password, - is_admin=is_admin) + is_admin=is_admin, + first_name=first_name, + last_name=last_name) user.otp_secret = otp_secret db.session.add(user) db.session.commit() print('Test user successfully added: ' 'username={}, password={}, otp_secret={}, is_admin={}' ''.format(username, password, otp_secret, is_admin)) + return user except IntegrityError: print("Test user already added") db.session.rollback() -def create_source_and_submissions(num_submissions=2, num_replies=2): +def create_source_and_submissions(num_submissions=2, num_replies=2, + journalist_who_replied=None): # Store source in database codename = current_app.crypto_util.genrandomid() filesystem_id = current_app.crypto_util.hash_codename(codename) @@ -109,7 +128,10 @@ def create_source_and_submissions(num_submissions=2, num_replies=2): config.JOURNALIST_KEY], current_app.storage.path(source.filesystem_id, fname)) - journalist = Journalist.query.first() + if not journalist_who_replied: + journalist = Journalist.query.first() + else: + journalist = journalist_who_replied reply = Reply(journalist, source, fname) db.session.add(reply) diff --git a/securedrop/models.py b/securedrop/models.py index 48810af001..57781c1058 100644 --- a/securedrop/models.py +++ b/securedrop/models.py @@ -278,6 +278,15 @@ def __repr__(self): def to_json(self): # type: () -> Dict[str, Union[str, int, bool]] + username = "" + first_name = "" + last_name = "" + uuid = "" + if self.journalist: + username = self.journalist.username + first_name = self.journalist.first_name + last_name = self.journalist.last_name + uuid = self.journalist.uuid json_submission = { 'source_url': url_for('api.single_source', source_uuid=self.source.uuid), @@ -286,10 +295,10 @@ def to_json(self): reply_uuid=self.uuid), 'filename': self.filename, 'size': self.size, - 'journalist_username': self.journalist.username, - 'journalist_first_name': self.journalist.first_name, - 'journalist_last_name': self.journalist.last_name, - 'journalist_uuid': self.journalist.uuid, + 'journalist_username': username, + 'journalist_first_name': first_name, + 'journalist_last_name': last_name, + 'journalist_uuid': uuid, 'uuid': self.uuid, 'is_deleted_by_source': self.deleted_by_source, } diff --git a/securedrop/tests/conftest.py b/securedrop/tests/conftest.py index 2518d0abe6..8cc584d18d 100644 --- a/securedrop/tests/conftest.py +++ b/securedrop/tests/conftest.py @@ -218,6 +218,23 @@ def test_files(journalist_app, test_journo): 'replies': source.replies} +@pytest.fixture(scope='function') +def test_files_deleted_journalist(journalist_app, test_journo): + with journalist_app.app_context(): + source, codename = utils.db_helper.init_source() + utils.db_helper.submit(source, 2) + test_journo['journalist'] + juser, _ = utils.db_helper.init_journalist("f", "l", is_admin=False) + utils.db_helper.reply(juser, source, 1) + utils.db_helper.delete_journalist(juser) + return {'source': source, + 'codename': codename, + 'filesystem_id': source.filesystem_id, + 'uuid': source.uuid, + 'submissions': source.submissions, + 'replies': source.replies} + + @pytest.fixture(scope='function') def journalist_api_token(journalist_app, test_journo): with journalist_app.test_client() as app: diff --git a/securedrop/tests/test_journalist_api.py b/securedrop/tests/test_journalist_api.py index d0581b8995..6ce01436c7 100644 --- a/securedrop/tests/test_journalist_api.py +++ b/securedrop/tests/test_journalist_api.py @@ -477,6 +477,31 @@ def test_authorized_user_can_get_single_reply(journalist_app, test_files, test_files['source'].replies[0].size +def test_reply_of_deleted_journalist(journalist_app, + test_files_deleted_journalist, + journalist_api_token): + with journalist_app.test_client() as app: + reply_uuid = test_files_deleted_journalist['source'].replies[0].uuid + uuid = test_files_deleted_journalist['source'].uuid + response = app.get(url_for('api.single_reply', + source_uuid=uuid, + reply_uuid=reply_uuid), + headers=get_api_headers(journalist_api_token)) + + assert response.status_code == 200 + + assert response.json['uuid'] == reply_uuid + assert response.json['journalist_username'] == "" + assert response.json['journalist_uuid'] == "" + assert response.json['journalist_first_name'] == "" + assert response.json['journalist_last_name'] == "" + assert response.json['is_deleted_by_source'] is False + assert response.json['filename'] == \ + test_files_deleted_journalist['source'].replies[0].filename + assert response.json['size'] == \ + test_files_deleted_journalist['source'].replies[0].size + + def test_authorized_user_can_delete_single_submission(journalist_app, test_submissions, journalist_api_token): diff --git a/securedrop/tests/utils/db_helper.py b/securedrop/tests/utils/db_helper.py index bb2f14761a..53ef7afbeb 100644 --- a/securedrop/tests/utils/db_helper.py +++ b/securedrop/tests/utils/db_helper.py @@ -39,6 +39,17 @@ def init_journalist(first_name=None, last_name=None, is_admin=False): return user, user_pw +def delete_journalist(journalist): + """Deletes a journalist from the database. + + :param models.Journalist journalist: The journalist to delete + + :returns: None + """ + db.session.delete(journalist) + db.session.commit() + + def reply(journalist, source, num_replies): """Generates and submits *num_replies* replies to *source* from *journalist*. Returns reply objects as a list. From 7b4484de61ec71a0e9a4d21b523f847004674aa2 Mon Sep 17 00:00:00 2001 From: redshiftzero Date: Wed, 1 Apr 2020 13:48:15 -0400 Subject: [PATCH 2/2] api: reply with username/uuid for deleted journalist users --- securedrop/models.py | 4 ++-- securedrop/tests/test_journalist_api.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/securedrop/models.py b/securedrop/models.py index 57781c1058..b0b5e5ad05 100644 --- a/securedrop/models.py +++ b/securedrop/models.py @@ -278,10 +278,10 @@ def __repr__(self): def to_json(self): # type: () -> Dict[str, Union[str, int, bool]] - username = "" + username = "deleted" first_name = "" last_name = "" - uuid = "" + uuid = "deleted" if self.journalist: username = self.journalist.username first_name = self.journalist.first_name diff --git a/securedrop/tests/test_journalist_api.py b/securedrop/tests/test_journalist_api.py index 6ce01436c7..9a36126a11 100644 --- a/securedrop/tests/test_journalist_api.py +++ b/securedrop/tests/test_journalist_api.py @@ -491,8 +491,8 @@ def test_reply_of_deleted_journalist(journalist_app, assert response.status_code == 200 assert response.json['uuid'] == reply_uuid - assert response.json['journalist_username'] == "" - assert response.json['journalist_uuid'] == "" + assert response.json['journalist_username'] == "deleted" + assert response.json['journalist_uuid'] == "deleted" assert response.json['journalist_first_name'] == "" assert response.json['journalist_last_name'] == "" assert response.json['is_deleted_by_source'] is False