Merge branch 'master' into make-sync-continuous

alembic/versions/fb657f2ee8a7_drop_file_original_filename.py

@@ -0,0 +1,71 @@
+"""drop File.original_filename
+
+Revision ID: fb657f2ee8a7
+Revises: 86b01b6290da
+Create Date: 2020-01-23 18:55:09.857324
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+# revision identifiers, used by Alembic.
+revision = "fb657f2ee8a7"
+down_revision = "86b01b6290da"
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    conn = op.get_bind()
+
+    op.rename_table("files", "original_files")
+
+    conn.execute(
+        """
+        CREATE TABLE files (
+                id INTEGER NOT NULL,
+                uuid VARCHAR(36) NOT NULL,
+                filename VARCHAR(255) NOT NULL,
+                file_counter INTEGER NOT NULL,
+                size INTEGER NOT NULL,
+                download_url VARCHAR(255) NOT NULL,
+                is_downloaded BOOLEAN DEFAULT 0 NOT NULL,
+                is_read BOOLEAN DEFAULT 0 NOT NULL,
+                is_decrypted BOOLEAN,
+                source_id INTEGER NOT NULL,
+                CONSTRAINT pk_files PRIMARY KEY (id),
+                CONSTRAINT fk_files_source_id_sources FOREIGN KEY(source_id) REFERENCES sources (id),
+                CONSTRAINT uq_messages_source_id_file_counter UNIQUE (source_id, file_counter),
+                CONSTRAINT uq_files_uuid UNIQUE (uuid),
+                CONSTRAINT files_compare_is_downloaded_vs_is_decrypted CHECK (CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END),
+                CONSTRAINT ck_files_is_downloaded CHECK (is_downloaded IN (0, 1)),
+                CONSTRAINT ck_files_is_read CHECK (is_read IN (0, 1)),
+                CONSTRAINT ck_files_is_decrypted CHECK (is_decrypted IN (0, 1))
+        )
+        """
+    )
+
+    conn.execute(
+        """
+        INSERT INTO files
+        (id, uuid, filename, file_counter, size, download_url, is_downloaded,
+        is_decrypted, is_read, source_id)
+        SELECT id, uuid, filename, file_counter, size, download_url, is_downloaded,
+        is_decrypted, is_read, source_id
+        FROM original_files
+    """
+    )
+
+    op.drop_table("original_files")
+
+
+def downgrade():
+    op.add_column(
+        "files",
+        sa.Column(
+            "original_filename",
+            sa.VARCHAR(length=255),
+            server_default=sa.text("''"),
+            nullable=False,
+        ),
+    )

securedrop_client/api_jobs/downloads.py

@@ -117,14 +117,14 @@ def call_api(self, api_client: API, session: Session) -> Any:
             self._decrypt(os.path.join(self.data_dir, db_object.filename), db_object, session)
             return db_object.uuid
 
-        self._download(api_client, db_object, session)
-        self._decrypt(os.path.join(self.data_dir, db_object.filename), db_object, session)
+        destination = self._download(api_client, db_object, session)
+        self._decrypt(destination, db_object, session)
         return db_object.uuid
 
     def _download(self,
                   api: API,
                   db_object: Union[File, Message, Reply],
-                  session: Session) -> None:
+                  session: Session) -> str:
         '''
         Download the encrypted file. Check file integrity and move it to the data directory before
         marking it as downloaded.
@@ -142,9 +142,12 @@ def _download(self,
                     )
                 raise exception
 
-            shutil.move(download_path, os.path.join(self.data_dir, db_object.filename))
+            destination = db_object.location(self.data_dir)
+            os.makedirs(os.path.dirname(destination), mode=0o700, exist_ok=True)
+            shutil.move(download_path, destination)
             mark_as_downloaded(type(db_object), db_object.uuid, session)
-            logger.info("File downloaded: {}".format(db_object.filename))
+            logger.info("File downloaded to {}".format(destination))
+            return destination
         except BaseError as e:
             logger.debug("Failed to download file: {}".format(db_object.filename))
             raise e
@@ -161,7 +164,9 @@ def _decrypt(self,
             mark_as_decrypted(
                 type(db_object), db_object.uuid, session, original_filename=original_filename
             )
-            logger.info("File decrypted: {}".format(os.path.basename(filepath)))
+            logger.info("File decrypted: {} (decrypted file: {})".format(
+                os.path.basename(filepath), original_filename)
+            )
         except CryptoError as e:
             mark_as_decrypted(type(db_object), db_object.uuid, session, is_decrypted=False)
             logger.debug("Failed to decrypt file: {}".format(os.path.basename(filepath)))
@@ -238,11 +243,21 @@ def call_decrypt(self, filepath: str, session: Session = None) -> str:
         '''
         with NamedTemporaryFile('w+') as plaintext_file:
             self.gpg.decrypt_submission_or_reply(filepath, plaintext_file.name, is_doc=False)
-            set_message_or_reply_content(
-                model_type=Reply,
-                uuid=self.uuid,
-                session=session,
-                content=plaintext_file.read())
+            try:
+                set_message_or_reply_content(
+                    model_type=Reply,
+                    uuid=self.uuid,
+                    session=session,
+                    content=plaintext_file.read())
+            finally:
+                # clean up directory where decryption happened
+                try:
+                    os.rmdir(os.path.dirname(filepath))
+                except Exception as e:
+                    logger.warning(
+                        "Error deleting decryption directory of message %s: %s", self.uuid, e
+                    )
+
         return ""
 
 
@@ -284,12 +299,21 @@ def call_decrypt(self, filepath: str, session: Session = None) -> str:
         The return value is an empty string; messages have no original filename.
         '''
         with NamedTemporaryFile('w+') as plaintext_file:
-            self.gpg.decrypt_submission_or_reply(filepath, plaintext_file.name, is_doc=False)
-            set_message_or_reply_content(
-                model_type=Message,
-                uuid=self.uuid,
-                session=session,
-                content=plaintext_file.read())
+            try:
+                self.gpg.decrypt_submission_or_reply(filepath, plaintext_file.name, is_doc=False)
+                set_message_or_reply_content(
+                    model_type=Message,
+                    uuid=self.uuid,
+                    session=session,
+                    content=plaintext_file.read())
+            finally:
+                # clean up directory where decryption happened
+                try:
+                    os.rmdir(os.path.dirname(filepath))
+                except Exception as e:
+                    logger.warning(
+                        "Error deleting decryption directory of message %s: %s", self.uuid, e
+                    )
         return ""
 
 
@@ -330,8 +354,9 @@ def call_decrypt(self, filepath: str, session: Session = None) -> str:
         the file extensions, e.g. 1-impractical_thing-doc.gz.gpg -> 1-impractical_thing-doc
         '''
         fn_no_ext, _ = os.path.splitext(os.path.splitext(os.path.basename(filepath))[0])
-        plaintext_filepath = os.path.join(self.data_dir, fn_no_ext)
+        plaintext_filepath = os.path.join(os.path.dirname(filepath), fn_no_ext)
         original_filename = self.gpg.decrypt_submission_or_reply(
             filepath, plaintext_filepath, is_doc=True
         )
+        logger.info("""Decrypted file "%s" to "%s" """, filepath, original_filename)
         return original_filename

securedrop_client/crypto.py

@@ -125,8 +125,12 @@ def decrypt_submission_or_reply(self,
 
             # Store the plaintext in the file located at the specified plaintext_filepath
             if is_doc:
-                original_filename = read_gzip_header_filename(out.name)
-                with gzip.open(out.name, 'rb') as infile, open(plaintext_filepath, 'wb') as outfile:
+                original_filename = read_gzip_header_filename(out.name) or plaintext_filepath
+                decrypt_path = os.path.join(
+                    os.path.dirname(filepath),
+                    os.path.basename(original_filename)
+                )
+                with gzip.open(out.name, 'rb') as infile, open(decrypt_path, 'wb') as outfile:
                     shutil.copyfileobj(infile, outfile)
             else:
                 shutil.copy(out.name, plaintext_filepath)
@@ -153,7 +157,7 @@ def fingerprints(self) -> typing.Dict[str, bool]:
         cmd = self._gpg_cmd_base()
         cmd.extend(["--list-public-keys", "--fingerprint", "--with-colons",
                     "--fixed-list-mode", "--list-options", "no-show-photos"])
-        output = subprocess.check_output(cmd, text=True)
+        output = subprocess.check_output(cmd, universal_newlines=True)
 
         fingerprints = {}
         for line in output.splitlines():

securedrop_client/db.py

@@ -63,6 +63,12 @@ def collection(self) -> List:
                                                datetime.datetime(datetime.MINYEAR, 1, 1))))
         return collection
 
+    @property
+    def journalist_filename(self) -> str:
+        valid_chars = 'abcdefghijklmnopqrstuvwxyz1234567890-_'
+        return ''.join([c for c in self.journalist_designation.lower().replace(
+            ' ', '_') if c in valid_chars])
+
 
 class Message(Base):
 
@@ -123,6 +129,18 @@ def __str__(self) -> str:
     def __repr__(self) -> str:
         return '<Message {}>'.format(self.filename)
 
+    def location(self, data_dir: str) -> str:
+        '''
+        Return the full path to the Message's file.
+        '''
+        return os.path.abspath(
+            os.path.join(
+                data_dir,
+                self.source.journalist_filename,
+                os.path.splitext(self.filename)[0] + '.txt'
+            )
+        )
+
 
 class File(Base):
 
@@ -135,15 +153,6 @@ class File(Base):
     uuid = Column(String(36), unique=True, nullable=False)
     filename = Column(String(255), nullable=False)
 
-    # Files from the SecureDrop journalist API are gzipped, then
-    # encrypted with GPG. The gzip header contains the original
-    # filename, which makes it easier for the client to open the file
-    # with the right application. We'll record that filename here
-    # after we've downloaded, decrypted and extracted the file.
-    # If the header does not contain the filename for some reason,
-    # this should be the same as filename.
-    original_filename = Column(String(255), nullable=False, server_default="")
-
     file_counter = Column(Integer, nullable=False)
     size = Column(Integer, nullable=False)
     download_url = Column(String(255), nullable=False)
@@ -179,13 +188,26 @@ def __str__(self) -> str:
         Return something that's a useful string representation of the file.
         """
         if self.is_downloaded:
-            return "File: {}".format(self.original_filename)
+            return "File: {}".format(self.filename)
         else:
             return '<Encrypted file on server>'
 
     def __repr__(self) -> str:
         return '<File {}>'.format(self.filename)
 
+    def location(self, data_dir: str) -> str:
+        '''
+        Return the full path to the File's file.
+        '''
+        return os.path.abspath(
+            os.path.join(
+                data_dir,
+                self.source.journalist_filename,
+                '{}-{}-doc'.format(self.file_counter, self.source.journalist_filename),
+                self.filename
+            )
+        )
+
 
 class Reply(Base):
 
@@ -246,6 +268,18 @@ def __str__(self) -> str:
     def __repr__(self) -> str:
         return '<Reply {}>'.format(self.filename)
 
+    def location(self, data_dir: str) -> str:
+        '''
+        Return the full path to the Reply's file.
+        '''
+        return os.path.abspath(
+            os.path.join(
+                data_dir,
+                self.source.journalist_filename,
+                os.path.splitext(self.filename)[0] + '.txt'
+            )
+        )
+
 
 class DraftReply(Base):
 

securedrop_client/gui/widgets.py

@@ -1897,7 +1897,7 @@ def __init__(
         self.print_button.clicked.connect(self._on_print_clicked)
 
         # File name or default string
-        self.file_name = SecureQLabel(self.file.original_filename)
+        self.file_name = SecureQLabel(self.file.filename)
         self.file_name.setObjectName('file_name')
         self.file_name.installEventFilter(self)
         self.no_file_name = SecureQLabel('ENCRYPTED FILE ON SERVER')
@@ -1952,7 +1952,7 @@ def _on_file_downloaded(self, file_uuid: str) -> None:
         if file_uuid == self.file.uuid:
             self.file = self.controller.get_file(self.file.uuid)
             if self.file.is_downloaded:
-                self.file_name.setText(self.file.original_filename)
+                self.file_name.setText(self.file.filename)
                 self.download_button.hide()
                 self.no_file_name.hide()
                 self.export_button.show()
@@ -1983,10 +1983,11 @@ def _on_export_clicked(self):
         """
         Called when the export button is clicked.
         """
-        if not self.controller.downloaded_file_exists(self.file.uuid):
+        if not self.controller.downloaded_file_exists(self.file):
             return
 
-        dialog = ExportDialog(self.controller, self.file.uuid, self.file.original_filename)
+        dialog = ExportDialog(self.controller, self.file.uuid,
+                              self.file.filename)
         dialog.show()
         dialog.export()
         dialog.exec()
@@ -1996,7 +1997,7 @@ def _on_print_clicked(self):
         """
         Called when the print button is clicked.
         """
-        if not self.controller.downloaded_file_exists(self.file.uuid):
+        if not self.controller.downloaded_file_exists(self.file):
             return
 
         dialog = PrintDialog(self.controller, self.file.uuid)
@@ -2014,7 +2015,7 @@ def _on_left_click(self):
 
         if self.file.is_downloaded:
             # Open the already downloaded file.
-            self.controller.on_file_open(self.file.uuid)
+            self.controller.on_file_open(self.file)
         else:
             if self.controller.api:
                 # Indicate in downloading state... but only after 0.3 seconds (i.e.