From bd3eab8c340c6b465ddb50bafa169aebc5d9de8d Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Mon, 18 Nov 2024 11:02:21 +0100
Subject: [PATCH] fix(filters): validate filter expression when creating or
 updating filter

Resolves https://github.com/go-vikunja/vikunja/issues/354
---
 pkg/models/saved_filters.go      | 10 ++++
 pkg/models/saved_filters_test.go | 90 +++++++++++++++++++++++---------
 2 files changed, 74 insertions(+), 26 deletions(-)

diff --git a/pkg/models/saved_filters.go b/pkg/models/saved_filters.go
index 5b27450d3..4d17f0535 100644
--- a/pkg/models/saved_filters.go
+++ b/pkg/models/saved_filters.go
@@ -123,6 +123,11 @@ func (sf *SavedFilter) toProject() *Project {
 // @Failure 500 {object} models.Message "Internal error"
 // @Router /filters [put]
 func (sf *SavedFilter) Create(s *xorm.Session, auth web.Auth) (err error) {
+	_, err = getTaskFiltersFromFilterString(sf.Filters.Filter, sf.Filters.FilterTimezone)
+	if err != nil {
+		return
+	}
+
 	sf.OwnerID = auth.GetID()
 	sf.ID = 0
 	_, err = s.Insert(sf)
@@ -190,6 +195,11 @@ func (sf *SavedFilter) Update(s *xorm.Session, _ web.Auth) error {
 		sf.Filters = origFilter.Filters
 	}
 
+	_, err = getTaskFiltersFromFilterString(sf.Filters.Filter, sf.Filters.FilterTimezone)
+	if err != nil {
+		return err
+	}
+
 	_, err = s.
 		Where("id = ?", sf.ID).
 		Cols(
diff --git a/pkg/models/saved_filters_test.go b/pkg/models/saved_filters_test.go
index 0fa2932f6..619abd12d 100644
--- a/pkg/models/saved_filters_test.go
+++ b/pkg/models/saved_filters_test.go
@@ -46,34 +46,56 @@ func TestSavedFilter_getFilterIDFromProjectID(t *testing.T) {
 }
 
 func TestSavedFilter_Create(t *testing.T) {
-	db.LoadAndAssertFixtures(t)
-	s := db.NewSession()
-	defer s.Close()
+	t.Run("empty filter", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
 
-	sf := &SavedFilter{
-		Title:       "test",
-		Description: "Lorem Ipsum dolor sit amet",
-		Filters:     &TaskCollection{}, // Empty filter
-	}
+		sf := &SavedFilter{
+			Title:       "test",
+			Description: "Lorem Ipsum dolor sit amet",
+			Filters:     &TaskCollection{}, // Empty filter
+		}
 
-	u := &user.User{ID: 1}
-	err := sf.Create(s, u)
-	require.NoError(t, err)
-	assert.Equal(t, u.ID, sf.OwnerID)
-	err = s.Commit()
-	require.NoError(t, err)
-	vals := map[string]interface{}{
-		"title":       "'test'",
-		"description": "'Lorem Ipsum dolor sit amet'",
-		"filters":     "'{\"sort_by\":null,\"order_by\":null,\"filter\":\"\",\"filter_include_nulls\":false}'",
-		"owner_id":    1,
-	}
-	// Postgres can't compare json values directly, see https://dba.stackexchange.com/a/106290/210721
-	if db.Type() == schemas.POSTGRES {
-		vals["filters::jsonb"] = vals["filters"].(string) + "::jsonb"
-		delete(vals, "filters")
-	}
-	db.AssertExists(t, "saved_filters", vals, true)
+		u := &user.User{ID: 1}
+		err := sf.Create(s, u)
+		require.NoError(t, err)
+		assert.Equal(t, u.ID, sf.OwnerID)
+		err = s.Commit()
+		require.NoError(t, err)
+		vals := map[string]interface{}{
+			"title":       "'test'",
+			"description": "'Lorem Ipsum dolor sit amet'",
+			"filters":     "'{\"sort_by\":null,\"order_by\":null,\"filter\":\"\",\"filter_include_nulls\":false}'",
+			"owner_id":    1,
+		}
+		// Postgres can't compare json values directly, see https://dba.stackexchange.com/a/106290/210721
+		if db.Type() == schemas.POSTGRES {
+			vals["filters::jsonb"] = vals["filters"].(string) + "::jsonb"
+			delete(vals, "filters")
+		}
+		db.AssertExists(t, "saved_filters", vals, true)
+	})
+	t.Run("invalid filter string", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+
+		sf := &SavedFilter{
+			Title:       "test",
+			Description: "Lorem Ipsum dolor sit amet",
+			Filters: &TaskCollection{
+				Filter: "foo = value",
+			},
+		}
+
+		u := &user.User{ID: 1}
+		err := sf.Create(s, u)
+		require.Error(t, err)
+		db.AssertMissing(t, "saved_filters", map[string]interface{}{
+			"title": "test",
+		})
+	})
 }
 
 func TestSavedFilter_ReadOne(t *testing.T) {
@@ -134,6 +156,22 @@ func TestSavedFilter_Update(t *testing.T) {
 			"is_favorite": true,
 		}, false)
 	})
+	t.Run("invalid filter string", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+
+		sf := &SavedFilter{
+			ID:          1,
+			Title:       "NewTitle",
+			Description: "", // Explicitly reset the description
+			Filters: &TaskCollection{
+				Filter: "foo = bar",
+			},
+		}
+		err := sf.Update(s, &user.User{ID: 1})
+		require.Error(t, err)
+	})
 }
 
 func TestSavedFilter_Delete(t *testing.T) {