Merge pull request #804 from girder/harden-acl-migration

Harden annotation ACL migration code.

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

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)

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