Make hashing independent of notebook content (#1774)

* Make hashing independent of notebook content

Decouple notebook hash from its content

* Testing changes for hash changes

nbgrader/exchange/default/fetch_feedback.py

@@ -48,35 +48,57 @@ def init_src(self):
                     self.coursedir.course_id, assignment_id, timestamp))
             pattern = os.path.join(self.cache_path, submission, "*.ipynb")
             notebooks = glob.glob(pattern)
+
+            # Check if a secret is provided
+            submission_secret = None
+            submission_secret_path = os.path.join(self.cache_path, submission, "submission_secret.txt")
+            if os.path.isfile(submission_secret_path):
+                with open(submission_secret_path) as fh:
+                    submission_secret = fh.read()
+
             for notebook in notebooks:
                 notebook_id = os.path.splitext(os.path.split(notebook)[-1])[0]
-                unique_key = make_unique_key(
-                    self.coursedir.course_id,
-                    assignment_id,
-                    notebook_id,
-                    student_id,
-                    timestamp)
-
-                # Look for the feedback using new-style of feedback
-                self.log.debug("Unique key is: {}".format(unique_key))
-                nb_hash = notebook_hash(notebook, unique_key)
-                feedbackpath = os.path.join(self.outbound_path, '{0}.html'.format(nb_hash))
-                if os.path.exists(feedbackpath):
-                    self.feedback_files.append((notebook_id, timestamp, feedbackpath))
-                    self.log.info(
-                        "Found feedback for '{}/{}/{}' submitted at {}".format(
-                            self.coursedir.course_id, assignment_id, notebook_id, timestamp))
-                    continue
-
-                # If it doesn't exist, try the legacy hashing
-                nb_hash = notebook_hash(notebook)
-                feedbackpath = os.path.join(self.outbound_path, '{0}.html'.format(nb_hash))
-                if os.path.exists(feedbackpath):
-                    self.feedback_files.append((notebook_id, timestamp, feedbackpath))
-                    self.log.warning(
-                        "Found legacy feedback for '{}/{}/{}' submitted at {}".format(
-                            self.coursedir.course_id, assignment_id, notebook_id, timestamp))
-                    continue
+
+                # If a secret is provided, use that
+                # If not, fall back to using make_unique_key
+                if submission_secret:
+                    nb_hash = notebook_hash(secret=submission_secret, notebook_id=notebook_id)
+                    feedbackpath = os.path.join(self.outbound_path, '{0}.html'.format(nb_hash))
+                    if os.path.exists(feedbackpath):
+                        self.feedback_files.append((notebook_id, timestamp, feedbackpath))
+                        self.log.info(
+                            "Found feedback for '{}/{}/{}' submitted at {}".format(
+                                self.coursedir.course_id, assignment_id, notebook_id, timestamp))
+                        continue
+
+                else:
+
+                    unique_key = make_unique_key(
+                        self.coursedir.course_id,
+                        assignment_id,
+                        notebook_id,
+                        student_id,
+                        timestamp)
+
+                    # Try legacy hashing 1
+                    nb_hash = notebook_hash(notebook, unique_key)
+                    feedbackpath = os.path.join(self.outbound_path, '{0}.html'.format(nb_hash))
+                    if os.path.exists(feedbackpath):
+                        self.feedback_files.append((notebook_id, timestamp, feedbackpath))
+                        self.log.info(
+                            "Found feedback for '{}/{}/{}' submitted at {}".format(
+                                self.coursedir.course_id, assignment_id, notebook_id, timestamp))
+                        continue
+
+                    # If it doesn't exist, try legacy hashing 2
+                    nb_hash = notebook_hash(notebook)
+                    feedbackpath = os.path.join(self.outbound_path, '{0}.html'.format(nb_hash))
+                    if os.path.exists(feedbackpath):
+                        self.feedback_files.append((notebook_id, timestamp, feedbackpath))
+                        self.log.warning(
+                            "Found legacy feedback for '{}/{}/{}' submitted at {}".format(
+                                self.coursedir.course_id, assignment_id, notebook_id, timestamp))
+                        continue
 
                 # If we reached here, then there's no feedback available
                 self.log.warning(

nbgrader/exchange/default/list.py

@@ -128,23 +128,36 @@ def parse_assignments(self):
                     local_feedback_checksum = None
 
                 # Also look to see if there is feedback available to fetch.
-                unique_key = make_unique_key(
-                    info['course_id'],
-                    info['assignment_id'],
-                    nb_info['notebook_id'],
-                    info['student_id'],
-                    info['timestamp'])
-                self.log.debug("Unique key is: {}".format(unique_key))
-                nb_hash = notebook_hash(notebook, unique_key)
-                exchange_feedback_path = os.path.join(
-                    self.root, info['course_id'], 'feedback', '{0}.html'.format(nb_hash))
-                has_exchange_feedback = os.path.isfile(exchange_feedback_path)
-                if not has_exchange_feedback:
-                    # Try looking for legacy feedback.
-                    nb_hash = notebook_hash(notebook)
+
+                # Check if a secret is provided
+                # If not, fall back to using make_unique_key
+                submission_secret_path = os.path.join(path, "submission_secret.txt")
+                if os.path.isfile(submission_secret_path):
+                    with open(submission_secret_path) as fh:
+                        submission_secret = fh.read()
+                    nb_hash = notebook_hash(secret=submission_secret, notebook_id=nb_info["notebook_id"])
+                    exchange_feedback_path = os.path.join(
+                        self.root, info['course_id'], 'feedback', '{0}.html'.format(nb_hash))
+                    has_exchange_feedback = os.path.isfile(exchange_feedback_path)
+                else:
+                    unique_key = make_unique_key(
+                        info['course_id'],
+                        info['assignment_id'],
+                        nb_info['notebook_id'],
+                        info['student_id'],
+                        info['timestamp'])
+                    self.log.debug("Unique key is: {}".format(unique_key))
+                    nb_hash = notebook_hash(notebook, unique_key)
                     exchange_feedback_path = os.path.join(
                         self.root, info['course_id'], 'feedback', '{0}.html'.format(nb_hash))
                     has_exchange_feedback = os.path.isfile(exchange_feedback_path)
+                    if not has_exchange_feedback:
+                        # Try looking for legacy feedback.
+                        nb_hash = notebook_hash(notebook)
+                        exchange_feedback_path = os.path.join(
+                            self.root, info['course_id'], 'feedback', '{0}.html'.format(nb_hash))
+                        has_exchange_feedback = os.path.isfile(exchange_feedback_path)
+
                 if has_exchange_feedback:
                     exchange_feedback_checksum = _checksum(exchange_feedback_path)
                 else:

nbgrader/exchange/default/release_feedback.py

@@ -61,21 +61,12 @@ def copy_files(self):
                 continue
 
             feedback_dir = os.path.split(html_file)[0]
-            submission_dir = self.coursedir.format_path(
-                self.coursedir.submitted_directory, student_id,
-                self.coursedir.assignment_id)
 
             timestamp = open(os.path.join(feedback_dir, 'timestamp.txt')).read()
-            nbfile = os.path.join(submission_dir, "{}.ipynb".format(notebook_id))
-            unique_key = make_unique_key(
-                self.coursedir.course_id,
-                self.coursedir.assignment_id,
-                notebook_id,
-                student_id,
-                timestamp)
+            with open(os.path.join(feedback_dir, "submission_secret.txt")) as fh:
+                submission_secret = fh.read()
 
-            self.log.debug("Unique key is: {}".format(unique_key))
-            checksum = notebook_hash(nbfile, unique_key)
+            checksum = notebook_hash(secret=submission_secret, notebook_id=notebook_id)
             dest = os.path.join(self.dest_path, "{}.html".format(checksum))
 
             self.log.info("Releasing feedback for student '{}' on assignment '{}/{}/{}' ({})".format(

nbgrader/exchange/default/submit.py

@@ -1,5 +1,6 @@
 import base64
 import os
+import secrets
 from stat import (
     S_IRUSR, S_IWUSR, S_IXUSR,
     S_IRGRP, S_IWGRP, S_IXGRP,
@@ -121,6 +122,7 @@ def check_filename_diff(self):
 
     def copy_files(self):
         self.init_release()
+        submission_secret = secrets.token_hex(64)
 
         dest_path = os.path.join(self.inbound_path, self.assignment_filename)
         if self.add_random_string:
@@ -136,6 +138,8 @@ def copy_files(self):
         self.do_copy(self.src_path, dest_path)
         with open(os.path.join(dest_path, "timestamp.txt"), "w") as fh:
             fh.write(self.timestamp)
+        with open(os.path.join(dest_path, "submission_secret.txt"), "w") as fh:
+            fh.write(submission_secret)
         self.set_perms(
             dest_path,
             fileperms=(S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH),
@@ -153,6 +157,8 @@ def copy_files(self):
         self.do_copy(self.src_path, cache_path)
         with open(os.path.join(cache_path, "timestamp.txt"), "w") as fh:
             fh.write(self.timestamp)
+        with open(os.path.join(cache_path, "submission_secret.txt"), "w") as fh:
+            fh.write(submission_secret)
 
         self.log.info("Submitted as: {} {} {}".format(
             self.coursedir.course_id, self.coursedir.assignment_id, str(self.timestamp)

nbgrader/tests/apps/files/submission_secret.txt

@@ -0,0 +1 @@
+e73a3cd47d1836ad99f5edbd1d4fb40799ea38d98a6d81a32b976e03fd79c7cbed81f51f857a532d8b23e702de3af4f03fbb4b3a69059dcae657dbcdc811d66b

nbgrader/tests/apps/files/submission_secret2.txt

@@ -0,0 +1 @@
+c03680201cf00012e5255077f14764b21c1e0bf88e43c5abda536be235d906bd89b4284411d85c489eef40f575a66d67be958231cc2a8fb48c8ed9aefe84bba3

nbgrader/tests/apps/test_api.py

@@ -768,22 +768,26 @@ def test_release_feedback(self, api, course_dir, db, exchange):
         api.generate_assignment("ps1")
         self._copy_file(join("files", "submitted-changed.ipynb"), join(course_dir, "submitted", "foo", "ps1", "p1.ipynb"))
         self._copy_file(join("files", "timestamp.txt"), join(course_dir, "submitted", "foo", "ps1", "timestamp.txt"))
+        self._copy_file(join("files", "submission_secret.txt"),
+                        join(course_dir, "submitted", "foo", "ps1", "submission_secret.txt"))
         api.autograde("ps1", "foo")
         api.generate_feedback("ps1", "foo")
         result = api.release_feedback("ps1", "foo")
         assert result["success"]
         assert os.path.isdir(join(exchange, "abc101", "feedback"))
-        assert os.path.exists(join(exchange, "abc101", "feedback", "c600ef68c434c3d136bb5e68ea874169.html"))
+        assert os.path.exists(join(exchange, "abc101", "feedback", "a2eea9fae1cfd3376a5ed6b7aac0bdc2.html"))
         # add another assignment
         self._copy_file(join("files", "submitted-unchanged.ipynb"), join(course_dir, "source", "ps2", "p2.ipynb"))
         api.generate_assignment("ps2")
         self._copy_file(join("files", "submitted-unchanged.ipynb"), join(course_dir, "submitted", "foo", "ps2", "p2.ipynb"))
         self._copy_file(join("files", "timestamp.txt"), join(course_dir, "submitted", "foo", "ps2", "timestamp.txt"))
+        self._copy_file(join("files", "submission_secret.txt"),
+                        join(course_dir, "submitted", "foo", "ps2", "submission_secret.txt"))
         api.autograde("ps2", "foo")
         api.generate_feedback("ps2", "foo")
         result = api.release_feedback("ps2", "foo")
         assert result["success"]
-        assert os.path.exists(join(exchange, "abc101", "feedback", "e190e1f234b633832f2069f4f8a3a680.html"))
+        assert os.path.exists(join(exchange, "abc101", "feedback", "6a0713045d217697b2ae0ab6b49fa1fe.html"))
 
     @notwindows
     def test_fetch_feedback(self, api, course_dir, db, cache):
@@ -793,8 +797,11 @@ def test_fetch_feedback(self, api, course_dir, db, cache):
         cachepath = join(cache, "abc101", "foo+ps1+{}".format(timestamp))
         self._copy_file(join("files", "submitted-changed.ipynb"), join(cachepath, "p1.ipynb"))
         self._copy_file(join("files", "timestamp.txt"), join(cachepath, "timestamp.txt"))
+        self._copy_file(join("files", "submission_secret.txt"), join(cachepath, "submission_secret.txt"))
         self._copy_file(join("files", "submitted-changed.ipynb"), join(course_dir, "submitted", "foo", "ps1", "p1.ipynb"))
         self._copy_file(join("files", "timestamp.txt"), join(course_dir, "submitted", "foo", "ps1", "timestamp.txt"))
+        self._copy_file(join("files", "submission_secret.txt"),
+                        join(course_dir, "submitted", "foo", "ps1", "submission_secret.txt"))
         api.autograde("ps1", "foo")
         api.generate_feedback("ps1", "foo")
         api.release_feedback("ps1", "foo")
@@ -808,8 +815,11 @@ def test_fetch_feedback(self, api, course_dir, db, cache):
         cachepath = join(cache, "abc101", "foo+ps2+{}".format(timestamp))
         self._copy_file(join("files", "submitted-unchanged.ipynb"), join(cachepath, "p2.ipynb"))
         self._copy_file(join("files", "timestamp.txt"), join(cachepath, "timestamp.txt"))
+        self._copy_file(join("files", "submission_secret.txt"), join(cachepath, "submission_secret.txt"))
         self._copy_file(join("files", "submitted-unchanged.ipynb"), join(course_dir, "submitted", "foo", "ps2", "p2.ipynb"))
         self._copy_file(join("files", "timestamp.txt"), join(course_dir, "submitted", "foo", "ps2", "timestamp.txt"))
+        self._copy_file(join("files", "submission_secret.txt"),
+                        join(course_dir, "submitted", "foo", "ps2", "submission_secret.txt"))
         api.autograde("ps2", "foo")
         api.generate_feedback("ps2", "foo")
         api.release_feedback("ps2", "foo")

nbgrader/tests/apps/test_nbgrader_list.py

@@ -215,6 +215,7 @@ def test_list_inbound(self, exchange, cache, course_dir):
         self._submit("ps1", exchange, cache)
         filename, = os.listdir(os.path.join(exchange, "abc101", "inbound"))
         timestamp = filename.split("+")[2]
+        print(self._list(exchange, cache, "ps1", flags=["--inbound"]))
         assert self._list(exchange, cache, "ps1", flags=["--inbound"]) == dedent(
             """
             [ListApp | INFO] Submitted assignments: