diff --git a/requirements/build-requirements.txt b/requirements/build-requirements.txt index f6165c25d..f29b6776d 100644 --- a/requirements/build-requirements.txt +++ b/requirements/build-requirements.txt @@ -9,7 +9,7 @@ pathlib2==2.3.2 --hash=sha256:460e67b14d0574b0529a0017b1eb05d10d9722681e303fec70 python-dateutil==2.7.5 --hash=sha256:f6eb9c17acd5a6954e1a5f2f999a41de3e7e25b6bc41baf6344bd053ec25ceeb python-editor==1.0.3 --hash=sha256:e47dcec4ea883853b8196fbd425b875d7ec791d4ede2e20cfc70b9a25365c65b requests==2.20.0 --hash=sha256:d87b2085783d31d874ac7bc62660e287932aaee7059e80b41b76462eb18d35cc -securedrop-sdk==0.1.0 --hash=sha256:488417f9f08e4c432c81348dfbd5da0e756ded1737ba58b2ffc8f0e703abc1cb +securedrop-sdk==0.1.1 --hash=sha256:a631495acd741ab568410287879c5a3af3ccd38e00a2f3a127cc6b27cba99392 six==1.11.0 --hash=sha256:aa4ad34049ddff178b533062797fd1db9f0038b7c5c2461a7cde2244300b9f3d sqlalchemy==1.3.3 --hash=sha256:bfb4cd0df5802a01acd738a080a19e60ff4700e030d497de273807f9a8bd4a0a urllib3==1.24.3 --hash=sha256:3d440cbb168e2c963d5099232bdb3f7390bf031b6270dad1bc79751698a1399a diff --git a/requirements/dev-requirements.txt b/requirements/dev-requirements.txt index 91d70c2a5..fd032abd1 100644 --- a/requirements/dev-requirements.txt +++ b/requirements/dev-requirements.txt @@ -377,8 +377,8 @@ requests==2.20.0 \ --hash=sha256:99dcfdaaeb17caf6e526f32b6a7b780461512ab3f1d992187801694cba42770c \ --hash=sha256:a84b8c9ab6239b578f22d1c21d51b696dcfe004032bb80ea832398d6909d7279 \ # via -r requirements/requirements.in, securedrop-sdk -securedrop-sdk==0.1.0 \ - --hash=sha256:970fde25e6238e1808ac120951ee972549f4cd7952966dfe29f731bb308cc0d8 \ +securedrop-sdk==0.1.1 \ + --hash=sha256:138ce7a717db519c3c8d19b9475d7660fb7095d7608e8802723e682a7415e677 \ # via -r requirements/requirements.in sip==4.19.8 \ --hash=sha256:09f9a4e6c28afd0bafedb26ffba43375b97fe7207bd1a0d3513f79b7d168b331 \ diff --git a/requirements/requirements.in b/requirements/requirements.in index cbf017322..0e87b5159 100644 --- a/requirements/requirements.in +++ b/requirements/requirements.in @@ -9,7 +9,7 @@ pathlib2==2.3.2 python-dateutil==2.7.5 python-editor==1.0.3 requests==2.20.0 -securedrop-sdk==0.1.0 +securedrop-sdk==0.1.1 six==1.11.0 SQLAlchemy==1.3.3 urllib3==1.24.3 diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 715370f7a..eea6735ff 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -75,8 +75,8 @@ requests==2.20.0 \ --hash=sha256:99dcfdaaeb17caf6e526f32b6a7b780461512ab3f1d992187801694cba42770c \ --hash=sha256:a84b8c9ab6239b578f22d1c21d51b696dcfe004032bb80ea832398d6909d7279 \ # via -r requirements/requirements.in, securedrop-sdk -securedrop-sdk==0.1.0 \ - --hash=sha256:970fde25e6238e1808ac120951ee972549f4cd7952966dfe29f731bb308cc0d8 \ +securedrop-sdk==0.1.1 \ + --hash=sha256:138ce7a717db519c3c8d19b9475d7660fb7095d7608e8802723e682a7415e677 \ # via -r requirements/requirements.in six==1.11.0 \ --hash=sha256:70e8a77beed4562e7f14fe23a786b54f6296e34344c23bc42f07b15018ff98e9 \ diff --git a/securedrop_client/api_jobs/sync.py b/securedrop_client/api_jobs/sync.py index 8abb3a21b..e498cafd6 100644 --- a/securedrop_client/api_jobs/sync.py +++ b/securedrop_client/api_jobs/sync.py @@ -5,7 +5,7 @@ from sqlalchemy.orm.session import Session from securedrop_client.api_jobs.base import ApiJob -from securedrop_client.storage import get_remote_data, update_and_get_user, update_local_storage +from securedrop_client.storage import create_or_update_user, get_remote_data, update_local_storage logger = logging.getLogger(__name__) @@ -42,6 +42,6 @@ def call_api(self, api_client: API, session: Session) -> Any: update_local_storage(session, sources, submissions, replies, self.data_dir) user = api_client.get_current_user() if "uuid" in user and "username" in user and "first_name" in user and "last_name" in user: - update_and_get_user( - user["uuid"], user["username"], user["first_name"], user["last_name"], session, + create_or_update_user( + user["uuid"], user["username"], user["first_name"], user["last_name"], session ) diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index a654dfa05..30913b09d 100644 --- a/securedrop_client/logic.py +++ b/securedrop_client/logic.py @@ -457,11 +457,11 @@ def on_authenticate_success(self, result): """ logger.info("{} successfully logged in".format(self.api.username)) self.gui.hide_login() - user = storage.update_and_get_user( + user = storage.create_or_update_user( self.api.token_journalist_uuid, self.api.username, - self.api.journalist_first_name, - self.api.journalist_last_name, + self.api.first_name, + self.api.last_name, self.session, ) # Clear clipboard contents in case of previously pasted creds diff --git a/securedrop_client/storage.py b/securedrop_client/storage.py index 9851b6d7d..582b3607f 100644 --- a/securedrop_client/storage.py +++ b/securedrop_client/storage.py @@ -299,8 +299,28 @@ def update_replies( source_cache = SourceCache(session) for reply in remote_replies: user = users.get(reply.journalist_uuid) + if not user: - user = find_or_create_user(reply.journalist_uuid, reply.journalist_username, session) + user = create_or_update_user( + reply.journalist_uuid, + reply.journalist_username, + reply.journalist_first_name, + reply.journalist_last_name, + session, + ) + users[reply.journalist_uuid] = user + elif ( + user.username != reply.journalist_username + or user.firstname != reply.journalist_first_name + or user.lastname != reply.journalist_last_name + ): + user = create_or_update_user( + reply.journalist_uuid, + reply.journalist_username, + reply.journalist_first_name, + reply.journalist_last_name, + session, + ) users[reply.journalist_uuid] = user local_reply = local_replies_by_uuid.get(reply.uuid) @@ -357,7 +377,9 @@ def update_replies( session.commit() -def find_or_create_user(uuid: str, username: str, session: Session, commit: bool = True) -> User: +def create_or_update_user( + uuid: str, username: str, firstname: str, lastname: str, session: Session +) -> User: """ Returns a user object representing the referenced journalist UUID. If the user does not already exist in the data, a new instance is created. @@ -366,30 +388,15 @@ def find_or_create_user(uuid: str, username: str, session: Session, commit: bool user = session.query(User).filter_by(uuid=uuid).one_or_none() if not user: - new_user = User(username=username) + new_user = User(username=username, firstname=firstname, lastname=lastname) new_user.uuid = uuid session.add(new_user) - if commit: - session.commit() + session.commit() return new_user if user.username != username: user.username = username - if commit: - session.commit() - - return user - - -def update_and_get_user( - uuid: str, username: str, firstname: str, lastname: str, session: Session -) -> User: - """ - Returns a user object representing the referenced journalist UUID. - If user fields have changed, the db is updated. - """ - user = find_or_create_user(uuid, username, session) - + session.commit() if user.firstname != firstname: user.firstname = firstname session.commit() diff --git a/tests/test_logic.py b/tests/test_logic.py index aaa6f1c3b..fa44971cd 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -219,8 +219,8 @@ def test_Controller_on_authenticate_success(homedir, config, mocker, session_mak co.api = mocker.MagicMock() co.api.token_journalist_uuid = user.uuid co.api.username = user.username - co.api.journalist_first_name = user.firstname - co.api.journalist_last_name = user.lastname + co.api.first_name = user.firstname + co.api.last_name = user.lastname co.resume_queues = mocker.MagicMock() co.on_authenticate_success(True) diff --git a/tests/test_storage.py b/tests/test_storage.py index 7047a718f..821078455 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -15,12 +15,12 @@ import securedrop_client.db from securedrop_client import db from securedrop_client.storage import ( + create_or_update_user, delete_local_source_by_uuid, delete_single_submission_or_reply_on_disk, find_new_files, find_new_messages, find_new_replies, - find_or_create_user, get_file, get_local_files, get_local_messages, @@ -35,7 +35,6 @@ mark_as_not_downloaded, set_message_or_reply_content, source_exists, - update_and_get_user, update_draft_replies, update_file_size, update_files, @@ -95,6 +94,8 @@ def make_remote_reply(source_uuid, journalist_uuid="testymctestface"): filename="1-reply.filename", journalist_uuid=journalist_uuid, journalist_username="test", + journalist_first_name="test", + journalist_last_name="test", file_counter=1, is_deleted_by_source=False, reply_url="test", @@ -683,7 +684,7 @@ def test_update_messages(homedir, mocker): local_user.id = 42 mock_session.query().filter_by().first.return_value = local_source mock_focu = mocker.MagicMock(return_value=local_user) - mocker.patch("securedrop_client.storage.find_or_create_user", mock_focu) + mocker.patch("securedrop_client.storage.create_or_update_user", mock_focu) mock_delete_submission_files = mocker.patch( "securedrop_client.storage.delete_single_submission_or_reply_on_disk" ) @@ -749,8 +750,10 @@ def test_update_replies(homedir, mocker, session): # local database, the other will NOT exist in the local database # (this will be added to the database) remote_reply_update = factory.RemoteReply( - journalist_uuid=journalist.uuid, uuid=local_reply_update.uuid, + journalist_uuid=journalist.uuid, + journalist_first_name="new_name", + journalist_last_name="new_name", source_url="/api/v1/sources/{}".format(source.uuid), file_counter=local_reply_update.file_counter, filename=local_reply_update.filename, @@ -761,6 +764,8 @@ def test_update_replies(homedir, mocker, session): source_url="/api/v1/sources/{}".format(source.uuid), file_counter=factory.REPLY_COUNT + 1, filename="{}-filename.gpg".format(factory.REPLY_COUNT + 1), + journalist_first_name="", + journalist_last_name="", ) remote_replies = [remote_reply_update, remote_reply_create] @@ -867,58 +872,59 @@ def test_update_replies_missing_source(homedir, mocker, session): error_logger.assert_called_once_with(f"No source found for reply {remote_reply.uuid}") -def test_find_or_create_user_existing_uuid(mocker): +def test_create_or_update_user_existing_uuid(mocker): """ Return an existing user object with the referenced uuid. """ mock_session = mocker.MagicMock() mock_user = mocker.MagicMock() mock_user.username = "foobar" + mock_user.firstname = "foobar" + mock_user.lastname = "foobar" mock_session.query().filter_by().one_or_none.return_value = mock_user - assert find_or_create_user("uuid", "foobar", mock_session) == mock_user + assert create_or_update_user("uuid", "username", "fn", "ln", mock_session) == mock_user -def test_find_or_create_user_update_username(mocker, session): +def test_create_or_update_user_update_username(mocker, session): """ Return an existing user object with the updated username. """ user = factory.User(uuid="mock_uuid", username="mock_old_username") session.add(user) - actual_user = find_or_create_user("mock_uuid", "mock_username", session) + actual_user = create_or_update_user("mock_uuid", "mock_username", "fn", "ln", session) assert actual_user == user assert actual_user.username == "mock_username" + assert actual_user.firstname == "fn" + assert actual_user.lastname == "ln" -def test_find_or_create_user_new(mocker): +def test_create_or_update_user_new(mocker): """ Create and return a user object for an unknown username. """ mock_session = mocker.MagicMock() mock_session.query().filter_by().one_or_none.return_value = None - new_user = find_or_create_user("uuid", "unknown", mock_session) + new_user = create_or_update_user("uuid", "unknown", "unknown", "unknown", mock_session) assert new_user.username == "unknown" mock_session.add.assert_called_once_with(new_user) mock_session.commit.assert_called_once_with() -def test_update_and_get_user(mocker, session): +def test_create_or_update_user(mocker, session): """ - Return an existing user object with the updated username. + Return an existing user object with the updated username, firstname, and lastname. """ user = factory.User( uuid="mock_uuid", - username="mock_username", # username is needed for find_or_create_user + username="mock_old_username", # username is needed for create_or_update_user firstname="mock_old_firstname", lastname="mock_old_lastname", ) session.add(user) - find_or_create_user_fn = mocker.patch( - "securedrop_client.storage.find_or_create_user", return_value=user - ) - actual_user = update_and_get_user( + updated_user = create_or_update_user( uuid="mock_uuid", username="mock_username", firstname="mock_firstname", @@ -926,11 +932,9 @@ def test_update_and_get_user(mocker, session): session=session, ) - find_or_create_user_fn.assert_called_with("mock_uuid", "mock_username", session) - assert actual_user == user - assert actual_user.username == "mock_username" - assert actual_user.firstname == "mock_firstname" - assert actual_user.lastname == "mock_lastname" + assert updated_user.username == "mock_username" + assert updated_user.firstname == "mock_firstname" + assert updated_user.lastname == "mock_lastname" def test_find_new_messages(mocker, session):