Repository: google/timesketch
Issue: Views are not validated for expected attributes

Views should be validated for expected attributes. The filter format will change over time, and there should be a way to ensure that old filters are updated with missing attributes.


Changed files:
timesketch/api/v1/resources.py, timesketch/models/sketch.py, timesketch/ui/views/sketch.py, timesketch/lib/testlib.py, timesketch/models/sketch_test.py

diff --git a/timesketch/api/v1/resources.py b/timesketch/api/v1/resources.py
index 531e231f..a080de2f 100644
--- a/timesketch/api/v1/resources.py
+++ b/timesketch/api/v1/resources.py
@@ -372,6 +372,12 @@ class ViewResource(ResourceMixin, Resource):
         # belongs to the current_user
         if view.name == u'' and view.user != current_user:
             abort(HTTP_STATUS_CODE_FORBIDDEN)
+
+        # Make sure we have all expected attributes in the query filter.
+        view.query_filter = view.validate_filter()
+        db_session.add(view)
+        db_session.commit()
+
         return self.to_json(view)
 
     @login_required
diff --git a/timesketch/lib/testlib.py b/timesketch/lib/testlib.py
index 5392b0eb..239751b2 100644
--- a/timesketch/lib/testlib.py
+++ b/timesketch/lib/testlib.py
@@ -13,6 +13,8 @@
 # limitations under the License.
 """This module contains common test utilities for Timesketch."""
 
+import json
+
 from flask_testing import TestCase
 
 from timesketch import create_app
@@ -295,8 +297,8 @@ class BaseTest(TestCase):
             A view (instance of timesketch.models.sketch.View)
         """
         view = View(
-            name=name, query_string=name, query_filter=u'', user=user,
-            sketch=sketch)
+            name=name, query_string=name, query_filter=json.dumps(dict()),
+            user=user, sketch=sketch)
         self._commit_to_database(view)
         return view
 
diff --git a/timesketch/models/sketch.py b/timesketch/models/sketch.py
index dc62e127..a5cec5da 100644
--- a/timesketch/models/sketch.py
+++ b/timesketch/models/sketch.py
@@ -13,6 +13,8 @@
 # limitations under the License.
 """This module implements the models for the Timesketch core system."""
 
+import json
+
 from sqlalchemy import Column
 from sqlalchemy import ForeignKey
 from sqlalchemy import Integer
@@ -175,6 +177,48 @@ class View(AccessControlMixin, LabelMixin, StatusMixin, CommentMixin,
         self.query_string = query_string
         self.query_filter = query_filter
 
+    def validate_filter(self, query_filter=None):
+        """Validate the Query Filter.
+
+        Make sure that we have all expected attributes in the query filter
+        json string. The filter dictionary evolves over time and this function
+        is used to update all filters.
+
+        Args:
+            query_filter: The query filter (JSON format or dictionary)
+
+        Returns:
+            query_filter: Query filter dictionary serialized to JSON
+
+        """
+        DEFAULT_LIMIT = 40  # Number of resulting documents to return
+        DEFAULT_VALUES = {
+            u'time_start': None,
+            u'time_end': None,
+            u'limit': DEFAULT_LIMIT,
+            u'indices': [],
+            u'exclude': [],
+            u'order': u'asc'
+        }
+        # If not provided, get the saved filter from the view
+        if not query_filter:
+            query_filter = self.query_filter
+
+        # Make sure we have the filter as a dictionary
+        if not isinstance(query_filter, dict):
+            filter_dict = json.loads(query_filter)
+        else:
+            filter_dict = query_filter
+
+        # Get all missing attributes and set them to their default value
+        missing_attributes = list(
+            set(DEFAULT_VALUES.keys()) - set(filter_dict.keys())
+        )
+        for key in missing_attributes:
+            filter_dict[key] = DEFAULT_VALUES[key]
+
+        return json.dumps(filter_dict, ensure_ascii=False)
+
 
 class Event(LabelMixin, StatusMixin, CommentMixin, BaseModel):
     """Implements the Event model."""
diff --git a/timesketch/models/sketch_test.py b/timesketch/models/sketch_test.py
index 5191e090..374cdf01 100644
--- a/timesketch/models/sketch_test.py
+++ b/timesketch/models/sketch_test.py
@@ -13,6 +13,8 @@
 # limitations under the License.
 """Tests for the sketch models."""
 
+import json
+
 from timesketch.models.sketch import Sketch
 from timesketch.models.sketch import Timeline
 from timesketch.models.sketch import SearchIndex
@@ -89,3 +91,23 @@ class SketchModelTest(ModelBaseTest):
         ])
         self._test_db_object(
             expected_result=expected_result, model_cls=Event)
+
+    def test_validate_filter(self):
+        """
+         Test the query filter validation.
+         """
+        DEFAULT_LIMIT = 40
+        default_values = {
+            u'time_start': None,
+            u'time_end': None,
+            u'limit': DEFAULT_LIMIT,
+            u'indices': [],
+            u'exclude': [],
+            u'order': u'asc'
+        }
+        test_filter = dict(indices=[])
+        test_filter_json = json.dumps(test_filter)
+        validated_filter_dict = self.view1.validate_filter(test_filter)
+        validated_filter_json = self.view1.validate_filter(test_filter_json)
+        self.assertDictEqual(json.loads(validated_filter_dict), default_values)
+        self.assertDictEqual(json.loads(validated_filter_json), default_values)
diff --git a/timesketch/ui/views/sketch.py b/timesketch/ui/views/sketch.py
index 6f424140..edf76c59 100644
--- a/timesketch/ui/views/sketch.py
+++ b/timesketch/ui/views/sketch.py
@@ -171,7 +171,6 @@ def explore(sketch_id, view_id=None):
     Returns:
         Template with context.
     """
-    DEFAULT_LIMIT = 100  # Number of events to return.
     save_view = False  # If the view should be saved to the database.
     sketch = Sketch.query.get_with_acl(sketch_id)
     sketch_timelines = [t.searchindex.index_name for t in sketch.timelines]
@@ -196,13 +195,12 @@ def explore(sketch_id, view_id=None):
             return abort(HTTP_STATUS_CODE_NOT_FOUND)
     else:
         view = sketch.get_user_view(current_user)
-
-    if not view:
-        query_filter = dict(indices=sketch_timelines, limit=DEFAULT_LIMIT)
-        view = View(
-            user=current_user, name=u'', sketch=sketch, query_string=u'*',
-            query_filter=json.dumps(query_filter, ensure_ascii=False))
-        save_view = True
+        if not view:
+            query_filter = view.validate_filter(dict(indices=sketch_timelines))
+            view = View(
+                user=current_user, name=u'', sketch=sketch, query_string=u'*',
+                query_filter=query_filter)
+            save_view = True
 
     if url_query:
         view.query_string = url_query
@@ -213,13 +211,7 @@ def explore(sketch_id, view_id=None):
             query_filter[u'indices'] = [url_index]
         if url_limit:
             query_filter[u'limit'] = url_limit
-        view.query_filter = json.dumps(query_filter, ensure_ascii=False)
-        save_view = True
-
-    query_filter = json.loads(view.query_filter)
-    if not query_filter.get(u'limit', None):
-        query_filter[u'limit'] = DEFAULT_LIMIT
-        view.query_filter = json.dumps(query_filter, ensure_ascii=False)
+        view.query_filter = view.validate_filter(query_filter)
         save_view = True
 
     if save_view: