From 74c0b30cd88a8232ffccf8cfe6ab8f2ca9478209 Mon Sep 17 00:00:00 2001
From: David Manthey <david.manthey@kitware.com>
Date: Fri, 18 Mar 2022 13:07:28 -0400
Subject: [PATCH] Harden annotation ACL migration code.

Fixes #803.
---
 CHANGELOG.md                                  |  3 +++
 .../models/annotation.py                      |  6 ++++--
 .../test_annotation/test_annotations.py       | 19 ++++++++++++++++++-
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 274049a19..b9f0bdfdb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,9 @@
 - Use orjson instead of ujson for annotations ([#802](../../pull/802))
 - Use simplejpeg for jpeg encoding rather than PIL ([#800](../../pull/800))
 
+### Bug Fixes
+- Harden annotation ACL migration code ([#804](../../pull/804))
+
 ## Version 1.11.2
 
 ### Improvements
diff --git a/girder_annotation/girder_large_image_annotation/models/annotation.py b/girder_annotation/girder_large_image_annotation/models/annotation.py
index 242f7e59d..9a50bd708 100644
--- a/girder_annotation/girder_large_image_annotation/models/annotation.py
+++ b/girder_annotation/girder_large_image_annotation/models/annotation.py
@@ -725,10 +725,12 @@ def _migrateACL(self, annotation):
                 'Could not generate annotation ACL due to missing folder %s', annotation['_id'])
             return annotation
 
-        user = User().load(annotation['creatorId'], force=True)
+        user = None
+        if annotation.get('creatorId'):
+            user = User().load(annotation['creatorId'], force=True)
         if user is None:
             logger.warning(
-                'Could not generate annotation ACL %s due to missing user %s', annotation['_id'])
+                'Could not generate annotation ACL due to missing user %s', annotation['_id'])
             return annotation
 
         self.copyAccessPolicies(item, annotation, save=False)
diff --git a/girder_annotation/test_annotation/test_annotations.py b/girder_annotation/test_annotation/test_annotations.py
index 8a23e18b9..849adb2ac 100644
--- a/girder_annotation/test_annotation/test_annotations.py
+++ b/girder_annotation/test_annotation/test_annotations.py
@@ -637,7 +637,7 @@ def testMigrateAnnotationAccessControl(self, user, admin):
         item = Item().createItem('userItem', user, publicFolder)
         annot = Annotation().createAnnotation(item, admin, sampleAnnotation)
 
-        # assert ACL's work
+        # assert ACLs work
         with pytest.raises(AccessException):
             Annotation().load(annot['_id'], user=user, level=AccessType.WRITE)
 
@@ -713,3 +713,20 @@ def testMigrateAnnotationAccessControlNoUserError(self, user, admin):
             logger.warning.assert_called_once()
         annot = Annotation().load(annot['_id'], force=True)
         assert 'access' not in annot
+
+    def testMigrateAnnotationAccessControlNullUserError(self, user, admin):
+        publicFolder = utilities.namedFolder(admin, 'Public')
+        # create an annotation
+        item = Item().createItem('userItem', user, publicFolder)
+        annot = Annotation().createAnnotation(item, admin, sampleAnnotation)
+
+        # remove the access control properties and save back to the database
+        del annot['access']
+        del annot['public']
+        annot['creatorId'] = None
+        Annotation().save(annot)
+        with mock.patch('girder_large_image_annotation.models.annotation.logger') as logger:
+            Annotation()._migrateDatabase()
+            logger.warning.assert_called_once()
+        annot = Annotation().load(annot['_id'], force=True)
+        assert 'access' not in annot