Merge pull request #1067 from mikegrima/1005

Fix for DB error documented in #1005

env-config/config-local.py

@@ -43,11 +43,7 @@
     'loggers': {
         'security_monkey': {
             'handlers': ['file', 'console'],
-            'level': 'INFO'
-        },
-        'apscheduler': {
-            'handlers': ['file', 'console'],
-            'level': 'WARN'
+            'level': 'DEBUG'
         }
     }
 }

env-config/config.py

@@ -44,11 +44,7 @@
     'loggers': {
         'security_monkey': {
             'handlers': ['file', 'console'],
-            'level': 'WARN'
-        },
-        'apscheduler': {
-            'handlers': ['file', 'console'],
-            'level': 'INFO'
+            'level': 'DEBUG'
         }
     }
 }

migrations/versions/7c54b06e227b_.py

@@ -206,10 +206,9 @@ class Technology(Base):
 
 def upgrade():
     """
-     Need to detect duplicate items.
-     This needs to be done by looking for all items with the same:
-     1. region, name, tech_id, account_id.
-     With these items, pick the one that has the bigger latest_revision_id and delete the others.
+    Need to detect duplicate items.
+    This needs to be done by looking for all items with the same: region, name, tech_id, account_id.
+    With these items, pick the one that has the bigger latest_revision_id and delete the others.
     :return:
     """
     bind = op.get_bind()

security_monkey/auditor.py

@@ -23,7 +23,7 @@
 from six import string_types, text_type
 
 from security_monkey import app, datastore, db
-from security_monkey.watcher import ChangeItem
+from security_monkey.watcher import ChangeItem, ensure_item_has_latest_revision_id
 from security_monkey.common.jinja import get_jinja_env
 from security_monkey.datastore import User, AuditorSettings, Item, ItemAudit, Technology, Account, ItemAuditScore, AccountPatternAuditScore
 from security_monkey.common.utils import send_email
@@ -102,6 +102,7 @@ class Categories:
     # TODO
     # 	INSECURE_CERTIFICATE = 'Insecure Certificate'
 
+
 class Entity:
     """ Entity instances provide a place to map policy elements like s3:my_bucket to the related account. """
     def __init__(self, category, value, account_name=None, account_identifier=None):
@@ -398,10 +399,18 @@ def _load_userids(cls):
         role_results = cls._load_related_items('iamrole')
 
         for item in user_results:
-            add(cls.OBJECT_STORE['userid'], item.latest_config.get('UserId'), item.account.identifier)
+            fixed_item = ensure_item_has_latest_revision_id(item)
+            if not fixed_item:
+                continue
+
+            add(cls.OBJECT_STORE['userid'], fixed_item.latest_config.get('UserId'), fixed_item.account.identifier)
 
         for item in role_results:
-            add(cls.OBJECT_STORE['userid'], item.latest_config.get('RoleId'), item.account.identifier)
+            fixed_item = ensure_item_has_latest_revision_id(item)
+            if not fixed_item:
+                continue
+
+            add(cls.OBJECT_STORE['userid'], fixed_item.latest_config.get('RoleId'), fixed_item.account.identifier)
 
     @classmethod
     def _load_accounts(cls):

security_monkey/tests/core/test_watcher.py

@@ -21,6 +21,7 @@
 
 """
 import datetime
+from datetime import timedelta
 import json
 
 from security_monkey.watcher import Watcher, ChangeItem
@@ -490,3 +491,65 @@ def test_find_deleted_batch(self):
 
             assert item_revision.active
             assert len(ItemAudit.query.filter(ItemAudit.item_id == item_revision.item_id).all()) == 2
+
+    def test_ensure_item_has_latest_revision_id(self):
+        """
+        Test that items always have a proper current revision set.  Otherwise, the item needs to be deleted.
+        :return:
+        """
+        from security_monkey.watchers.iam.iam_role import IAMRole
+        from security_monkey.watcher import ensure_item_has_latest_revision_id
+        from security_monkey.datastore import Datastore
+
+        # Stop the watcher registry from stepping on everyone's toes:
+        import security_monkey.watcher
+        old_watcher_registry = security_monkey.watcher.watcher_registry
+        security_monkey.watcher.watcher_registry = {IAMRole.index: IAMRole}
+
+        # Set everything up:
+        self.setup_batch_db()
+        watcher = IAMRole(accounts=[self.account.name])
+        watcher.current_account = (self.account, 0)
+        watcher.technology = self.technology
+
+        # Test case #1: Create an item in the DB that has no current revision ID:
+        no_revision_item = Item(region="us-east-1", name="NOREVISION", account_id=self.account.id,
+                                tech_id=self.technology.id)
+        db.session.add(no_revision_item)
+        db.session.commit()
+
+        assert db.session.query(Item).filter(Item.name == no_revision_item.name).one()
+
+        # Should delete the item from the DB:
+        result = ensure_item_has_latest_revision_id(no_revision_item)
+        assert not result
+        assert not db.session.query(Item).filter(Item.name == no_revision_item.name).first()
+
+        # Test case #2: Create two item revisions for the given item, but don't attach them to the item.
+        #               After the fixer runs, it should return the item with proper hashes and a proper
+        #               link to the latest version.
+        ds = Datastore()
+        no_revision_item = Item(region="us-east-1", name="NOREVISION", account_id=self.account.id,
+                                tech_id=self.technology.id)
+        db.session.add(no_revision_item)
+        db.session.commit()
+
+        ir_one = ItemRevision(config=ACTIVE_CONF, date_created=datetime.datetime.utcnow(),
+                              item_id=no_revision_item.id)
+        ir_two = ItemRevision(config=ACTIVE_CONF,
+                              date_created=(datetime.datetime.utcnow() - timedelta(days=1)),
+                              item_id=no_revision_item.id)
+
+        db.session.add(ir_one)
+        db.session.add(ir_two)
+        db.session.commit()
+
+        assert len(db.session.query(ItemRevision).filter(ItemRevision.item_id == no_revision_item.id).all()) == 2
+        result = ensure_item_has_latest_revision_id(no_revision_item)
+        assert result
+        assert result.latest_revision_id == ir_one.id
+        assert ds.hash_config(ACTIVE_CONF) == no_revision_item.latest_revision_complete_hash
+        assert ds.durable_hash(ACTIVE_CONF, watcher.ephemeral_paths) == no_revision_item.latest_revision_durable_hash
+
+        # Undo the mock:
+        security_monkey.watcher.watcher_registry = old_watcher_registry

security_monkey/watcher.py

@@ -13,8 +13,8 @@
 from security_monkey.common.PolicyDiff import PolicyDiff
 from security_monkey.common.utils import sub_dict
 from security_monkey import app, datastore
-from security_monkey.datastore import Account, IgnoreListEntry, db
-from security_monkey.datastore import Technology, WatcherConfig, store_exception
+from security_monkey.datastore import Technology, WatcherConfig, store_exception, Account, IgnoreListEntry, db, \
+    ItemRevision, Datastore
 from security_monkey.common.jinja import get_jinja_env
 from security_monkey.alerters.custom_alerter import report_watcher_changes
 
@@ -652,3 +652,62 @@ def save(self, datastore, ephemeral=False):
             new_issues=self.audit_issues,
             ephemeral=ephemeral,
             source_watcher=self.watcher)
+
+
+def ensure_item_has_latest_revision_id(item):
+    """
+    This is a function that will attempt to correct an item that does not have a latest revision id set.
+    There are two outcomes that result:
+    1. If there is a revision with the item id, find the latest revision, and update the item such that it
+       point to that latest revision.
+    2. If not -- then we will treat the item as rancid and delete it from the database.
+    :param item:
+    :return The item if it was fixed -- or None if it was deleted from the DB:
+    """
+    if not item.latest_revision_id:
+        current_revision = db.session.query(ItemRevision).filter(ItemRevision.item_id == item.id)\
+                            .order_by(ItemRevision.date_created.desc()).first()
+
+        if not current_revision:
+            db.session.delete(item)
+            db.session.commit()
+
+            app.logger.error("[?] Item: {name}/{tech}/{account}/{region} does NOT have a latest revision attached, "
+                             "and no current revisions were located. The item has been deleted.".format(
+                                name=item.name,
+                                tech=item.technology.name,
+                                account=item.account.name,
+                                region=item.region))
+
+            return None
+
+        else:
+            # Update the latest revision ID:
+            item.latest_revision_id = current_revision.id
+
+            # Also need to generate the hashes:
+            # 1. Get the watcher class of the item:
+            watcher_cls = watcher_registry[item.technology.name]
+            watcher = watcher_cls(accounts=[item.account.name])
+            ds = Datastore()
+
+            # 2. Generate the hashes:
+            if watcher.honor_ephemerals:
+                ephemeral_paths = watcher.ephemeral_paths
+            else:
+                ephemeral_paths = []
+
+            item.latest_revision_complete_hash = ds.hash_config(current_revision.config)
+            item.latest_revision_durable_hash = ds.durable_hash(current_revision.config, ephemeral_paths)
+
+            db.session.add(item)
+            db.session.commit()
+
+            app.logger.error("[?] Item: {name}/{tech}/{account}/{region} does NOT have a latest revision attached, "
+                             "but a current revision was located. The item has been fixed.".format(
+                                name=item.name,
+                                tech=item.technology.name,
+                                account=item.account.name,
+                                region=item.region))
+
+    return item