Merge pull request #349 from micahflee/better-filenames

Better filenames

securedrop/db.py

@@ -45,13 +45,19 @@ class Source(Base):
     # sources are "pending" and don't get displayed to journalists until they submit something
     pending = Column(Boolean, default=True)
 
+    # keep track of how many interactions have happened, for filenames
+    interaction_count = Column(Integer, default=0, nullable=False)
+
     def __init__(self, filesystem_id=None, journalist_designation=None):
         self.filesystem_id = filesystem_id
         self.journalist_designation = journalist_designation
 
     def __repr__(self):
         return '<Source %r>' % (self.journalist_designation)
 
+    def journalist_filename(self):
+        valid_chars = 'abcdefghijklmnopqrstuvwxyz1234567890-_'
+        return ''.join([c for c in self.journalist_designation.lower().replace(' ', '_') if c in valid_chars])
 
 class Submission(Base):
     __tablename__ = 'submissions'

securedrop/journalist.py

@@ -72,8 +72,8 @@ def get_docs(sid):
             date=str(datetime.fromtimestamp(os_stat.st_mtime)),
             size=os_stat.st_size,
         ))
-    # sort by date since ordering by filename is meaningless
-    docs.sort(key=lambda x: x['date'])
+    # sort in chronological order
+    docs.sort(key=lambda x: int(x['name'].split('-')[0]))
     return docs
 
 
@@ -138,8 +138,13 @@ def doc(sid, fn):
 @app.route('/reply', methods=('POST',))
 def reply():
     msg = request.form['msg']
+    g.source.interaction_count += 1
+    filename = "{0}-reply.gpg".format(g.source.interaction_count)
+
     crypto_util.encrypt(crypto_util.getkey(g.sid), msg, output=
-                        store.path(g.sid, 'reply-%s.gpg' % uuid.uuid4()))
+                        store.path(g.sid, filename))
+
+    db_session.commit()
     return render_template('reply.html', sid=g.sid,
             codename=g.source.journalist_designation)
 

securedrop/source.py

@@ -143,7 +143,7 @@ def create():
 def lookup():
     replies = []
     for fn in os.listdir(g.loc):
-        if fn.startswith('reply-'):
+        if fn.endswith('-reply.gpg'):
             try:
                 msg = crypto_util.decrypt(g.codename,
                         file(store.path(g.sid, fn)).read()).decode("utf-8")
@@ -192,13 +192,17 @@ def submit():
     strip_metadata = True if 'notclean' in request.form else False
 
     fnames = []
+    journalist_filename = g.source.journalist_filename()
 
     if msg:
-        fnames.append(store.save_message_submission(g.sid, msg))
+        g.source.interaction_count += 1
+        fnames.append(store.save_message_submission(g.sid, g.source.interaction_count,
+            journalist_filename, msg))
         flash("Thanks! We received your message.", "notification")
     if fh:
-        fnames.append(store.save_file_submission(g.sid, fh.filename,
-            fh.stream, fh.content_type, strip_metadata))
+        g.source.interaction_count += 1
+        fnames.append(store.save_file_submission(g.sid, g.source.interaction_count,
+            journalist_filename, fh.filename, fh.stream, fh.content_type, strip_metadata))
         flash("Thanks! We received your document '%s'."
               % fh.filename or '[unnamed]', "notification")
 

securedrop/store.py

@@ -19,7 +19,7 @@
 from werkzeug import secure_filename
 
 VALIDATE_FILENAME = re.compile(
-    "^(reply-)?[a-f0-9-]+(_msg|_doc\.zip|)\.gpg$").match
+    "^(reply-)?[a-z0-9-_]+(-msg|-doc\.zip|)\.gpg$").match
 
 
 class PathException(Exception):
@@ -79,7 +79,7 @@ def get_bulk_archive(filenames):
     return zip_file
 
 
-def save_file_submission(sid, filename, stream, content_type, strip_metadata):
+def save_file_submission(sid, count, journalist_filename, filename, stream, content_type, strip_metadata):
     sanitized_filename = secure_filename(filename)
     clean_file = sanitize_metadata(stream, content_type, strip_metadata)
 
@@ -88,14 +88,14 @@ def save_file_submission(sid, filename, stream, content_type, strip_metadata):
         zf.writestr(sanitized_filename, clean_file.read() if clean_file else stream.read())
     s.reset()
 
-    filename = "%s_doc.zip.gpg" % uuid.uuid4()
+    filename = "{0}-{1}-doc.zip.gpg".format(count, journalist_filename)
     file_loc = path(sid, filename)
     crypto_util.encrypt(config.JOURNALIST_KEY, s, file_loc)
     return filename
 
 
-def save_message_submission(sid, message):
-    filename = "%s_msg.gpg" % uuid.uuid4()
+def save_message_submission(sid, count, journalist_filename, message):
+    filename = "{0}-{1}-msg.gpg".format(count, journalist_filename)
     msg_loc = path(sid, filename)
     crypto_util.encrypt(config.JOURNALIST_KEY, message, msg_loc)
     return filename
@@ -144,3 +144,4 @@ def sanitize_metadata(stream, content_type, strip_metadata):
             t.close()
 
     return s
+

securedrop/tests/unit_tests.py

@@ -323,7 +323,7 @@ def test_bulk_download(self):
         source = Source(sid, crypto_util.display_id())
         db_session.add(source)
         db_session.commit()
-        files = ['abc1_msg.gpg', 'abc2_msg.gpg']
+        files = ['1-abc1-msg.gpg', '2-abc2-msg.gpg']
         filenames = test_setup.setup_test_docs(sid, files)
 
         rv = self.client.post('/bulk', data=dict(
@@ -375,7 +375,7 @@ def test_submit_message(self):
         self.assertEqual(rv.status_code, 200)
         soup = BeautifulSoup(rv.data)
         submission_url = soup.select('ul#submissions li a')[0]['href']
-        self.assertIn("_msg", submission_url)
+        self.assertIn("-msg", submission_url)
         li = soup.select('ul#submissions li')[0]
         self.assertRegexpMatches(li.contents[-1], "\d+ bytes")
 
@@ -448,7 +448,7 @@ def test_submit_file(self):
         self.assertEqual(rv.status_code, 200)
         soup = BeautifulSoup(rv.data)
         submission_url = soup.select('ul#submissions li a')[0]['href']
-        self.assertIn("_doc", submission_url)
+        self.assertIn("-doc", submission_url)
         li = soup.select('ul#submissions li')[0]
         self.assertRegexpMatches(li.contents[-1], "\d+ bytes")
 
@@ -665,6 +665,95 @@ def test_delete_collections(self):
         # 2. "Don't show again" checkbox behavior
         # 2. Correct behavior on "yes" and "no" buttons
 
+    def test_filenames(self):
+        """Test pretty, sequential filenames when source uploads messages and files"""
+        # add a source and submit stuff
+        self.source_app.get('/generate')
+        self.source_app.post('/create')
+        self.helper_filenames_submit()
+
+        # navigate to the collection page
+        rv = self.journalist_app.get('/')
+        soup = BeautifulSoup(rv.data)
+        first_col_url = soup.select('ul#cols > li a')[0]['href']
+        rv = self.journalist_app.get(first_col_url)
+        self.assertEqual(rv.status_code, 200)
+
+        # test filenames and sort order
+        soup = BeautifulSoup(rv.data)
+        submission_filename_re = r'^{0}-[a-z0-9-_]+(-msg|-doc\.zip)\.gpg$'
+        for i, submission_link in enumerate(soup.select('ul#submissions li a')):
+            filename = str(submission_link.contents[0])
+            self.assertTrue(re.match(submission_filename_re.format(i+1), filename))
+
+
+    def test_filenames_delete(self):
+        """Test pretty, sequential filenames when journalist deletes files"""
+        # add a source and submit stuff
+        self.source_app.get('/generate')
+        self.source_app.post('/create')
+        self.helper_filenames_submit()
+
+        # navigate to the collection page
+        rv = self.journalist_app.get('/')
+        soup = BeautifulSoup(rv.data)
+        first_col_url = soup.select('ul#cols > li a')[0]['href']
+        rv = self.journalist_app.get(first_col_url)
+        self.assertEqual(rv.status_code, 200)
+        soup = BeautifulSoup(rv.data)
+
+        # delete file #2
+        self.helper_filenames_delete(soup, 1)
+        rv = self.journalist_app.get(first_col_url)
+        soup = BeautifulSoup(rv.data)
+
+        # test filenames and sort order
+        submission_filename_re = r'^{0}-[a-z0-9-_]+(-msg|-doc\.zip)\.gpg$'
+        filename = str(soup.select('ul#submissions li a')[0].contents[0])
+        self.assertTrue( re.match(submission_filename_re.format(1), filename) )
+        filename = str(soup.select('ul#submissions li a')[1].contents[0])
+        self.assertTrue( re.match(submission_filename_re.format(3), filename) )
+        filename = str(soup.select('ul#submissions li a')[2].contents[0])
+        self.assertTrue( re.match(submission_filename_re.format(4), filename) )
+
+
+    def helper_filenames_submit(self):
+        self.source_app.post('/submit', data=dict(
+            msg="This is a test.",
+            fh=(StringIO(''), ''),
+        ), follow_redirects=True)
+        self.source_app.post('/submit', data=dict(
+            msg="This is a test.",
+            fh=(StringIO('This is a test'), 'test.txt'),
+        ), follow_redirects=True)
+        self.source_app.post('/submit', data=dict(
+            msg="",
+            fh=(StringIO('This is a test'), 'test.txt'),
+        ), follow_redirects=True)
+
+    def helper_filenames_delete(self, soup, i):
+        sid = soup.select('input[name="sid"]')[0]['value']
+        checkbox_values = [soup.select('input[name="doc_names_selected"]')[i]['value']]
+
+        # delete
+        rv = self.journalist_app.post('/bulk', data=dict(
+            sid=sid,
+            action='delete',
+            doc_names_selected=checkbox_values
+        ), follow_redirects=True)
+        self.assertEqual(rv.status_code, 200)
+        self.assertIn("The following file has been selected for <strong>permanent deletion</strong>", rv.data)
+
+        # confirm delete
+        rv = self.journalist_app.post('/bulk', data=dict(
+            sid=sid,
+            action='delete',
+            confirm_delete=1,
+            doc_names_selected=checkbox_values
+        ), follow_redirects=True)
+        self.assertEqual(rv.status_code, 200)
+        self.assertIn("File permanently deleted.", rv.data)
+
 
 class TestStore(unittest.TestCase):
 
@@ -683,7 +772,7 @@ def test_verify(self):
 
     def test_get_zip(self):
         sid = 'EQZGCJBRGISGOTC2NZVWG6LILJBHEV3CINNEWSCLLFTUWZJPKJFECLS2NZ4G4U3QOZCFKTTPNZMVIWDCJBBHMUDBGFHXCQ3R'
-        files = ['abc1_msg.gpg', 'abc2_msg.gpg']
+        files = ['1-abc1-msg.gpg', '2-abc2-msg.gpg']
         filenames = test_setup.setup_test_docs(sid, files)
 
         archive = zipfile.ZipFile(store.get_bulk_archive(filenames))