From 0eb6853c6ec2c50fc1826ebf9e66f66c76750ab5 Mon Sep 17 00:00:00 2001 From: Koen Bollen Date: Fri, 11 Oct 2024 16:32:53 +0200 Subject: [PATCH 1/4] Introduce new, required, access control options Previously this library has been insecure by default, this change will make one of the new access control options required. --- README.md | 6 +- examples/basic_test.go | 15 ++++- examples/readme_test.go | 5 +- filter/converter.go | 47 +++++++++++-- filter/converter_test.go | 127 ++++++++++++++++++++++++++++++----- filter/errors.go | 14 ++++ filter/options.go | 62 ++++++++++++++--- fuzz/fuzz_test.go | 3 +- integration/postgres_test.go | 12 ++-- 9 files changed, 245 insertions(+), 46 deletions(-) create mode 100644 filter/errors.go diff --git a/README.md b/README.md index c49a078..52f32c8 100644 --- a/README.md +++ b/README.md @@ -32,8 +32,12 @@ import ( func main() { // Create a converter with options: + // - WithAllowAllColumns: allow all columns to be filtered. // - WithArrayDriver: to convert arrays to the correct driver type, required when using lib/pq - converter := filter.NewConverter(filter.WithArrayDriver(pq.Array)) + converter, err := filter.NewConverter(filter.WithAllowAllColumns(), filter.WithArrayDriver(pq.Array)) + if err != nil { + // handle error + } // Convert a filter query to a WHERE clause and values: input := []byte(`{"title": "Jurassic Park"}`) diff --git a/examples/basic_test.go b/examples/basic_test.go index d7a3374..9d706d9 100644 --- a/examples/basic_test.go +++ b/examples/basic_test.go @@ -9,7 +9,10 @@ import ( func ExampleNewConverter() { // Remeber to use `filter.WithArrayDriver(pg.Array)` when using github.com/lib/pq - converter := filter.NewConverter(filter.WithNestedJSONB("meta", "created_at", "updated_at")) + converter, err := filter.NewConverter(filter.WithNestedJSONB("meta", "created_at", "updated_at")) + if err != nil { + // handle error + } mongoFilterQuery := `{ "name": "John", @@ -30,7 +33,10 @@ func ExampleNewConverter() { } func ExampleNewConverter_emptyfilter() { - converter := filter.NewConverter(filter.WithEmptyCondition("TRUE")) // The default is FALSE if you don't change it. + converter, err := filter.NewConverter(filter.WithAllowAllColumns(), filter.WithEmptyCondition("TRUE")) // The default is FALSE if you don't change it. + if err != nil { + // handle error + } mongoFilterQuery := `{}` conditions, _, err := converter.Convert([]byte(mongoFilterQuery), 1) @@ -44,7 +50,10 @@ func ExampleNewConverter_emptyfilter() { } func ExampleNewConverter_nonIsolatedConditions() { - converter := filter.NewConverter() + converter, err := filter.NewConverter(filter.WithAllowAllColumns()) + if err != nil { + // handle error + } mongoFilterQuery := `{ "$or": [ diff --git a/examples/readme_test.go b/examples/readme_test.go index 6a60354..7851cd6 100644 --- a/examples/readme_test.go +++ b/examples/readme_test.go @@ -8,7 +8,10 @@ import ( func ExampleNewConverter_readme() { // Remeber to use `filter.WithArrayDriver(pg.Array)` when using github.com/lib/pq - converter := filter.NewConverter(filter.WithNestedJSONB("meta", "created_at", "updated_at")) + converter, err := filter.NewConverter(filter.WithNestedJSONB("meta", "created_at", "updated_at")) + if err != nil { + // handle error + } mongoFilterQuery := `{ "$and": [ diff --git a/filter/converter.go b/filter/converter.go index 1684290..c3ef104 100644 --- a/filter/converter.go +++ b/filter/converter.go @@ -30,9 +30,12 @@ const defaultPlaceholderName = "__filter_placeholder" // Converter converts MongoDB filter queries to SQL conditions and values. Use [filter.NewConverter] to create a new instance. type Converter struct { - nestedColumn string - nestedExemptions []string - arrayDriver func(a any) interface { + allowAllColumns bool + allowedColumns []string + disallowedColumns []string + nestedColumn string + nestedExemptions []string + arrayDriver func(a any) interface { driver.Valuer sql.Scanner } @@ -45,16 +48,23 @@ type Converter struct { // NewConverter creates a new [Converter] with optional nested JSONB field mapping. // // Note: When using https://github.com/lib/pq, the [filter.WithArrayDriver] should be set to pq.Array. -func NewConverter(options ...Option) *Converter { +func NewConverter(options ...Option) (*Converter, error) { converter := &Converter{ // don't set defaults, use the once.Do in #Convert() } + seenAccessOption := false for _, option := range options { - if option != nil { - option(converter) + if option.f != nil { + option.f(converter) } + if option.isAccessOption { + seenAccessOption = true + } + } + if !seenAccessOption { + return nil, ErrNoAccessOption } - return converter + return converter, nil } // Convert converts a MongoDB filter query into SQL conditions and values. @@ -165,6 +175,9 @@ func (c *Converter) convertFilter(filter map[string]any, paramIndex int) (string if !isValidPostgresIdentifier(key) { return "", nil, fmt.Errorf("invalid column name: %s", key) } + if !c.isColumnAllowed(key) { + return "", nil, ColumnNotAllowedError{Column: key} + } switch v := value.(type) { case map[string]any: @@ -346,6 +359,26 @@ func (c *Converter) columnName(column string) string { return fmt.Sprintf(`%q->>'%s'`, c.nestedColumn, column) } +func (c *Converter) isColumnAllowed(column string) bool { + for _, disallowed := range c.disallowedColumns { + if disallowed == column { + return false + } + } + if c.allowAllColumns { + return true + } + if c.nestedColumn != "" { + return true + } + for _, allowed := range c.allowedColumns { + if allowed == column { + return true + } + } + return false +} + func (c *Converter) isNestedColumn(column string) bool { if c.nestedColumn == "" { return false diff --git a/filter/converter_test.go b/filter/converter_test.go index d9f3c79..e8db1d0 100644 --- a/filter/converter_test.go +++ b/filter/converter_test.go @@ -11,7 +11,10 @@ import ( func ExampleNewConverter() { // Remeber to use `filter.WithArrayDriver(pg.Array)` when using github.com/lib/pq - converter := filter.NewConverter(filter.WithNestedJSONB("meta", "created_at", "updated_at")) + converter, err := filter.NewConverter(filter.WithNestedJSONB("meta", "created_at", "updated_at")) + if err != nil { + // handle error + } mongoFilterQuery := `{ "name": "John", @@ -33,7 +36,7 @@ func ExampleNewConverter() { func TestConverter_Convert(t *testing.T) { tests := []struct { name string - option filter.Option + option []filter.Option input string conditions string values []any @@ -73,7 +76,7 @@ func TestConverter_Convert(t *testing.T) { }, { "nested jsonb single value", - filter.WithNestedJSONB("meta"), + []filter.Option{filter.WithNestedJSONB("meta")}, `{"name": "John"}`, `("meta"->>'name' = $1)`, []any{"John"}, @@ -81,7 +84,7 @@ func TestConverter_Convert(t *testing.T) { }, { "nested jsonb multi value", - filter.WithNestedJSONB("meta", "created_at", "updated_at"), + []filter.Option{filter.WithNestedJSONB("meta", "created_at", "updated_at")}, `{"created_at": {"$gte": "2020-01-01T00:00:00Z"}, "name": "John", "role": "admin"}`, `(("created_at" >= $1) AND ("meta"->>'name' = $2) AND ("meta"->>'role' = $3))`, []any{"2020-01-01T00:00:00Z", "John", "admin"}, @@ -296,7 +299,7 @@ func TestConverter_Convert(t *testing.T) { }, { "null jsonb column", - filter.WithNestedJSONB("meta"), + []filter.Option{filter.WithNestedJSONB("meta")}, `{"name": null}`, `(jsonb_path_match(meta, 'exists($.name)') AND "meta"->>'name' IS NULL)`, nil, @@ -312,7 +315,7 @@ func TestConverter_Convert(t *testing.T) { }, { "not $exists jsonb column", - filter.WithNestedJSONB("meta"), + []filter.Option{filter.WithNestedJSONB("meta")}, `{"name": {"$exists": false}}`, `(NOT jsonb_path_match(meta, 'exists($.name)'))`, nil, @@ -320,7 +323,7 @@ func TestConverter_Convert(t *testing.T) { }, { "$exists jsonb column", - filter.WithNestedJSONB("meta"), + []filter.Option{filter.WithNestedJSONB("meta")}, `{"name": {"$exists": true}}`, `(jsonb_path_match(meta, 'exists($.name)'))`, nil, @@ -344,7 +347,7 @@ func TestConverter_Convert(t *testing.T) { }, { "$elemMatch on jsonb column", - filter.WithNestedJSONB("meta"), + []filter.Option{filter.WithNestedJSONB("meta")}, `{"name": {"$elemMatch": {"$eq": "John"}}}`, `EXISTS (SELECT 1 FROM jsonb_array_elements("meta"->'name') AS __filter_placeholder WHERE ("__filter_placeholder"::text = $1))`, []any{"John"}, @@ -352,7 +355,7 @@ func TestConverter_Convert(t *testing.T) { }, { "$elemMatch with $gt", - filter.WithPlaceholderName("__placeholder"), + []filter.Option{filter.WithAllowAllColumns(), filter.WithPlaceholderName("__placeholder")}, `{"age": {"$elemMatch": {"$gt": 18}}}`, `EXISTS (SELECT 1 FROM unnest("age") AS __placeholder WHERE ("__placeholder"::text > $1))`, []any{float64(18)}, @@ -360,7 +363,7 @@ func TestConverter_Convert(t *testing.T) { }, { "numeric comparison bug with jsonb column", - filter.WithNestedJSONB("meta"), + []filter.Option{filter.WithNestedJSONB("meta")}, `{"foo": {"$gt": 0}}`, `(("meta"->>'foo')::numeric > $1)`, []any{float64(0)}, @@ -368,7 +371,7 @@ func TestConverter_Convert(t *testing.T) { }, { "numeric comparison against null with jsonb column", - filter.WithNestedJSONB("meta"), + []filter.Option{filter.WithNestedJSONB("meta")}, `{"foo": {"$gt": null}}`, `("meta"->>'foo' > $1)`, []any{nil}, @@ -392,7 +395,7 @@ func TestConverter_Convert(t *testing.T) { }, { "compare two jsonb fields", - filter.WithNestedJSONB("meta"), + []filter.Option{filter.WithNestedJSONB("meta")}, `{"foo": {"$eq": {"$field": "bar"}}}`, `("meta"->>'foo' = "meta"->>'bar')`, nil, @@ -400,7 +403,7 @@ func TestConverter_Convert(t *testing.T) { }, { "compare two jsonb fields with numeric comparison", - filter.WithNestedJSONB("meta"), + []filter.Option{filter.WithNestedJSONB("meta")}, `{"foo": {"$lt": {"$field": "bar"}}}`, `(("meta"->>'foo')::numeric < ("meta"->>'bar')::numeric)`, nil, @@ -408,7 +411,7 @@ func TestConverter_Convert(t *testing.T) { }, { "compare two fields with simple expression", - filter.WithNestedJSONB("meta", "foo"), + []filter.Option{filter.WithNestedJSONB("meta", "foo")}, `{"foo": {"$field": "bar"}}`, `("foo" = "meta"->>'bar')`, nil, @@ -426,7 +429,13 @@ func TestConverter_Convert(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c := filter.NewConverter(tt.option) + if tt.option == nil { + tt.option = []filter.Option{filter.WithAllowAllColumns()} + } + c, err := filter.NewConverter(tt.option...) + if err != nil { + t.Fatal(err) + } conditions, values, err := c.Convert([]byte(tt.input), 1) if err != nil && (tt.err == nil || err.Error() != tt.err.Error()) { t.Errorf("Converter.Convert() error = %v, wantErr %v", err, tt.err) @@ -447,7 +456,7 @@ func TestConverter_Convert(t *testing.T) { } func TestConverter_Convert_startAtParameterIndex(t *testing.T) { - c := filter.NewConverter() + c, _ := filter.NewConverter(filter.WithAllowAllColumns()) conditions, values, err := c.Convert([]byte(`{"name": "John", "password": "secret"}`), 10) if err != nil { t.Fatal(err) @@ -476,7 +485,7 @@ func TestConverter_Convert_startAtParameterIndex(t *testing.T) { } func TestConverter_WithEmptyCondition(t *testing.T) { - c := filter.NewConverter(filter.WithEmptyCondition("TRUE")) + c, _ := filter.NewConverter(filter.WithAllowAllColumns(), filter.WithEmptyCondition("TRUE")) conditions, values, err := c.Convert([]byte(`{}`), 1) if err != nil { t.Fatal(err) @@ -490,6 +499,8 @@ func TestConverter_WithEmptyCondition(t *testing.T) { } func TestConverter_NoConstructor(t *testing.T) { + t.Skip() // this is currently not supported since we introduced the access control options + c := &filter.Converter{} conditions, values, err := c.Convert([]byte(`{"name": "John"}`), 1) if err != nil { @@ -524,3 +535,85 @@ func TestConverter_CopyReference(t *testing.T) { t.Errorf("Converter.Convert() conditions = %v, want %v", conditions, want) } } + +func TestConverter_RequireAccessControl(t *testing.T) { + if _, err := filter.NewConverter(); err != filter.ErrNoAccessOption { + t.Errorf("NewConverter() error = %v, want %v", err, filter.ErrNoAccessOption) + } + if _, err := filter.NewConverter(filter.WithPlaceholderName("___test___")); err != filter.ErrNoAccessOption { + t.Errorf("NewConverter() error = %v, want %v", err, filter.ErrNoAccessOption) + } + if _, err := filter.NewConverter(filter.WithAllowAllColumns()); err != nil { + t.Errorf("NewConverter() error = %v, want no error", err) + } + if _, err := filter.NewConverter(filter.WithAllowColumns("name", "map")); err != nil { + t.Errorf("NewConverter() error = %v, want no error", err) + } + if _, err := filter.NewConverter(filter.WithAllowColumns()); err != nil { + t.Errorf("NewConverter() error = %v, want no error", err) + } + if _, err := filter.NewConverter(filter.WithNestedJSONB("meta", "created_at", "updated_at")); err != nil { + t.Errorf("NewConverter() error = %v, want no error", err) + } + if _, err := filter.NewConverter(filter.WithDisallowColumns("password")); err != nil { + t.Errorf("NewConverter() error = %v, want no error", err) + } +} + +func TestConverter_AccessControl(t *testing.T) { + f := func(in string, wantErr error, options ...filter.Option) func(t *testing.T) { + t.Helper() + return func(t *testing.T) { + t.Helper() + c := &filter.Converter{} + if options != nil { + c, _ = filter.NewConverter(options...) + // requirement of access control is tested above. + } + q, _, err := c.Convert([]byte(in), 1) + t.Log(in, "->", q, err) + if wantErr == nil && err != nil { + t.Fatalf("no error returned, expected error: %v", err) + } else if wantErr != nil && err == nil { + t.Fatalf("expected error: %v", wantErr) + } else if wantErr != nil && wantErr.Error() != err.Error() { + t.Fatalf("error mismatch: %v != %v", err, wantErr) + } + } + } + + no := func(c string) error { return filter.ColumnNotAllowedError{Column: c} } + + t.Run("allow all, single root field", + f(`{"name":"John"}`, nil, filter.WithAllowAllColumns())) + t.Run("allow name, single allowed root field", + f(`{"name":"John"}`, nil, filter.WithAllowColumns("name"))) + t.Run("allow name, single disallowed root field", + f(`{"password":"hacks"}`, no("password"), filter.WithAllowColumns("name"))) + t.Run("allowed meta, single allowed nested field", + f(`{"map":"de_dust"}`, nil, filter.WithNestedJSONB("meta", "created_at"))) + t.Run("allowed nested excemption, single allowed field", + f(`{"created_at":"de_dust"}`, nil, filter.WithNestedJSONB("meta", "created_at"))) + t.Run("multi allow, single allowed root field", + f(`{"name":"John"}`, nil, filter.WithAllowColumns("name", "email"))) + t.Run("multi allow, two allowed root fields", + f(`{"name":"John", "email":"test@example.org"}`, nil, filter.WithAllowColumns("name", "email"))) + t.Run("multi allow, mixes access", + f(`{"name":"John", "password":"hacks"}`, no("password"), filter.WithAllowColumns("name", "email"))) + t.Run("multi allow, mixes access", + f(`{"name":"John", "password":"hacks"}`, no("password"), filter.WithAllowColumns("name", "email"))) + t.Run("allowed basic $and", + f(`{"$and": [{"name": "John"}, {"version": 3}]}`, nil, filter.WithAllowColumns("name", "version"))) + t.Run("disallowed basic $and", + f(`{"$and": [{"name": "John"}, {"version": 3}]}`, no("version"), filter.WithAllowColumns("name"))) + t.Run("allow all but one", + f(`{"name": "John"}`, nil, filter.WithAllowAllColumns(), filter.WithDisallowColumns("password"))) + t.Run("allow all but one, failing", + f(`{"$and": [{"name": "John"}, {"password": "hacks"}]}`, no("password"), filter.WithAllowAllColumns(), filter.WithDisallowColumns("password"))) + t.Run("nested but disallow password, allow exception", + f(`{"created_at": "1"}`, nil, filter.WithNestedJSONB("meta", "created_at"), filter.WithDisallowColumns("password"))) + t.Run("nested but disallow password, allow nested", + f(`{"map": "de_dust"}`, nil, filter.WithNestedJSONB("meta", "created_at"), filter.WithDisallowColumns("password"))) + t.Run("nested but disallow password, disallow", + f(`{"password": "hacks"}`, no("password"), filter.WithNestedJSONB("meta", "created_at"), filter.WithDisallowColumns("password"))) +} diff --git a/filter/errors.go b/filter/errors.go new file mode 100644 index 0000000..480e173 --- /dev/null +++ b/filter/errors.go @@ -0,0 +1,14 @@ +package filter + +import "fmt" + +// ErrNoAccessOption is returned when no access options are provided to NewConverter. +var ErrNoAccessOption = fmt.Errorf("NewConverter: need atleast one of the access options: WithAllowAllColumns, WithAllowColumns, WithNestedJSONB") + +type ColumnNotAllowedError struct { + Column string +} + +func (e ColumnNotAllowedError) Error() string { + return fmt.Sprintf("column not allowed: %s", e.Column) +} diff --git a/filter/options.go b/filter/options.go index 691adae..32de467 100644 --- a/filter/options.go +++ b/filter/options.go @@ -5,7 +5,40 @@ import ( "database/sql/driver" ) -type Option func(*Converter) +type Option struct { + f func(*Converter) + isAccessOption bool +} + +// WithAllowAllColumns is the option to allow all columns in the query. +func WithAllowAllColumns() Option { + return Option{ + f: func(c *Converter) { + c.allowAllColumns = true + }, + isAccessOption: true, + } +} + +// WithAllowColumns is an option to allow only the specified columns in the query. +func WithAllowColumns(columns ...string) Option { + return Option{ + f: func(c *Converter) { + c.allowedColumns = append(c.allowedColumns, columns...) + }, + isAccessOption: true, + } +} + +// WithDisallowColumns is an option to disallow the specified columns in the query. +func WithDisallowColumns(columns ...string) Option { + return Option{ + f: func(c *Converter) { + c.disallowedColumns = append(c.disallowedColumns, columns...) + }, + isAccessOption: true, + } +} // WithNestedJSONB is an option to specify the column name that contains the nested // JSONB object. (e.g. you have a column named `metadata` that contains a nested @@ -18,9 +51,12 @@ type Option func(*Converter) // // c := filter.NewConverter(filter.WithNestedJSONB("metadata", "created_at", "updated_at")) func WithNestedJSONB(column string, exemption ...string) Option { - return func(c *Converter) { - c.nestedColumn = column - c.nestedExemptions = exemption + return Option{ + f: func(c *Converter) { + c.nestedColumn = column + c.nestedExemptions = exemption + }, + isAccessOption: true, } } @@ -35,8 +71,10 @@ func WithArrayDriver(f func(a any) interface { driver.Valuer sql.Scanner }) Option { - return func(c *Converter) { - c.arrayDriver = f + return Option{ + f: func(c *Converter) { + c.arrayDriver = f + }, } } @@ -45,8 +83,10 @@ func WithArrayDriver(f func(a any) interface { // // The default value is `FALSE`, because it's the safer choice in most cases. func WithEmptyCondition(condition string) Option { - return func(c *Converter) { - c.emptyCondition = condition + return Option{ + f: func(c *Converter) { + c.emptyCondition = condition + }, } } @@ -54,7 +94,9 @@ func WithEmptyCondition(condition string) Option { // used in the generated SQL query. This name should not be used in the database // or any JSONB column. func WithPlaceholderName(name string) Option { - return func(c *Converter) { - c.placeholderName = name + return Option{ + f: func(c *Converter) { + c.placeholderName = name + }, } } diff --git a/fuzz/fuzz_test.go b/fuzz/fuzz_test.go index eef71bf..59a1518 100644 --- a/fuzz/fuzz_test.go +++ b/fuzz/fuzz_test.go @@ -52,12 +52,13 @@ func FuzzConverter(f *testing.F) { f.Fuzz(func(t *testing.T, in string, jsonb bool) { options := []filter.Option{ + filter.WithAllowAllColumns(), filter.WithArrayDriver(pq.Array), } if jsonb { options = append(options, filter.WithNestedJSONB("meta")) } - c := filter.NewConverter(options...) + c, _ := filter.NewConverter(options...) conditions, _, err := c.Convert([]byte(in), 1) if err == nil && conditions != "" { j, err := pg_query.ParseToJSON("SELECT * FROM test WHERE 1 AND " + conditions) diff --git a/integration/postgres_test.go b/integration/postgres_test.go index 9ec14ac..0943f02 100644 --- a/integration/postgres_test.go +++ b/integration/postgres_test.go @@ -41,7 +41,7 @@ func TestIntegration_ReadmeExample(t *testing.T) { t.Fatal(err) } - c := filter.NewConverter( + c, _ := filter.NewConverter( filter.WithArrayDriver(pq.Array), filter.WithNestedJSONB("customData", "password", "playerCount"), ) @@ -120,7 +120,7 @@ func TestIntegration_InAny_PQ(t *testing.T) { t.Fatal(err) } - c := filter.NewConverter(filter.WithArrayDriver(pq.Array)) + c, _ := filter.NewConverter(filter.WithAllowAllColumns(), filter.WithArrayDriver(pq.Array)) in := `{ "role": { "$in": ["guest", "user"] } }` @@ -185,7 +185,7 @@ func TestIntegration_InAny_PGX(t *testing.T) { t.Fatal(err) } - c := filter.NewConverter() + c, _ := filter.NewConverter() in := `{ "role": { "$in": ["guest", "user"] } }` @@ -445,7 +445,7 @@ func TestIntegration_BasicOperators(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c := filter.NewConverter(filter.WithArrayDriver(pq.Array), filter.WithNestedJSONB("metadata", "name", "level", "class", "mount", "items", "parents")) + c, _ := filter.NewConverter(filter.WithArrayDriver(pq.Array), filter.WithNestedJSONB("metadata", "name", "level", "class", "mount", "items", "parents")) conditions, values, err := c.Convert([]byte(tt.input), 1) if err != nil { t.Fatal(err) @@ -522,7 +522,7 @@ func TestIntegration_NestedJSONB(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c := filter.NewConverter(filter.WithArrayDriver(pq.Array), filter.WithNestedJSONB("metadata", "name", "level", "class")) + c, _ := filter.NewConverter(filter.WithArrayDriver(pq.Array), filter.WithNestedJSONB("metadata", "name", "level", "class")) conditions, values, err := c.Convert([]byte(tt.input), 1) if err != nil { t.Fatal(err) @@ -590,7 +590,7 @@ func TestIntegration_Logic(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c := filter.NewConverter(filter.WithArrayDriver(pq.Array), filter.WithNestedJSONB("metadata", "name", "level", "class")) + c, _ := filter.NewConverter(filter.WithArrayDriver(pq.Array), filter.WithNestedJSONB("metadata", "name", "level", "class")) conditions, values, err := c.Convert([]byte(tt.input), 1) if err != nil { t.Fatal(err) From a0d43386e0593593b3610e28daa0b5a111a2fff5 Mon Sep 17 00:00:00 2001 From: Koen Bollen Date: Fri, 18 Oct 2024 10:41:28 +0200 Subject: [PATCH 2/4] Fix intergration test. --- integration/postgres_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/postgres_test.go b/integration/postgres_test.go index 0943f02..3c027f6 100644 --- a/integration/postgres_test.go +++ b/integration/postgres_test.go @@ -185,7 +185,7 @@ func TestIntegration_InAny_PGX(t *testing.T) { t.Fatal(err) } - c, _ := filter.NewConverter() + c, _ := filter.NewConverter(filter.WithAllowAllColumns()) in := `{ "role": { "$in": ["guest", "user"] } }` From 9d5f1cf1ed97ed24af217aecf91faa1b32d10a72 Mon Sep 17 00:00:00 2001 From: Koen Bollen Date: Fri, 18 Oct 2024 10:43:56 +0200 Subject: [PATCH 3/4] Change and reenable test for no constructor usage. --- filter/converter_test.go | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/filter/converter_test.go b/filter/converter_test.go index e8db1d0..e863f31 100644 --- a/filter/converter_test.go +++ b/filter/converter_test.go @@ -499,21 +499,9 @@ func TestConverter_WithEmptyCondition(t *testing.T) { } func TestConverter_NoConstructor(t *testing.T) { - t.Skip() // this is currently not supported since we introduced the access control options - c := &filter.Converter{} - conditions, values, err := c.Convert([]byte(`{"name": "John"}`), 1) - if err != nil { - t.Fatal(err) - } - if want := `("name" = $1)`; conditions != want { - t.Errorf("Converter.Convert() conditions = %v, want %v", conditions, want) - } - if !reflect.DeepEqual(values, []any{"John"}) { - t.Errorf("Converter.Convert() values = %v, want %v", values, []any{"John"}) - } - conditions, values, err = c.Convert([]byte(``), 1) + conditions, values, err := c.Convert([]byte(``), 1) if err != nil { t.Fatal(err) } @@ -523,6 +511,11 @@ func TestConverter_NoConstructor(t *testing.T) { if len(values) != 0 { t.Errorf("Converter.Convert() values = %v, want nil", values) } + + _, _, err = c.Convert([]byte(`{"name": "John"}`), 1) + if err == nil { + t.Fatal("expected error, got nil") + } } func TestConverter_CopyReference(t *testing.T) { From a23010095035b7c7d6c9feb0551033ef1821b965 Mon Sep 17 00:00:00 2001 From: Koen Bollen Date: Fri, 18 Oct 2024 11:02:29 +0200 Subject: [PATCH 4/4] Format functests a bit better --- filter/converter_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/filter/converter_test.go b/filter/converter_test.go index e863f31..59e9f48 100644 --- a/filter/converter_test.go +++ b/filter/converter_test.go @@ -579,34 +579,49 @@ func TestConverter_AccessControl(t *testing.T) { t.Run("allow all, single root field", f(`{"name":"John"}`, nil, filter.WithAllowAllColumns())) + t.Run("allow name, single allowed root field", f(`{"name":"John"}`, nil, filter.WithAllowColumns("name"))) + t.Run("allow name, single disallowed root field", f(`{"password":"hacks"}`, no("password"), filter.WithAllowColumns("name"))) + t.Run("allowed meta, single allowed nested field", f(`{"map":"de_dust"}`, nil, filter.WithNestedJSONB("meta", "created_at"))) + t.Run("allowed nested excemption, single allowed field", f(`{"created_at":"de_dust"}`, nil, filter.WithNestedJSONB("meta", "created_at"))) + t.Run("multi allow, single allowed root field", f(`{"name":"John"}`, nil, filter.WithAllowColumns("name", "email"))) + t.Run("multi allow, two allowed root fields", f(`{"name":"John", "email":"test@example.org"}`, nil, filter.WithAllowColumns("name", "email"))) + t.Run("multi allow, mixes access", f(`{"name":"John", "password":"hacks"}`, no("password"), filter.WithAllowColumns("name", "email"))) + t.Run("multi allow, mixes access", f(`{"name":"John", "password":"hacks"}`, no("password"), filter.WithAllowColumns("name", "email"))) + t.Run("allowed basic $and", f(`{"$and": [{"name": "John"}, {"version": 3}]}`, nil, filter.WithAllowColumns("name", "version"))) + t.Run("disallowed basic $and", f(`{"$and": [{"name": "John"}, {"version": 3}]}`, no("version"), filter.WithAllowColumns("name"))) + t.Run("allow all but one", f(`{"name": "John"}`, nil, filter.WithAllowAllColumns(), filter.WithDisallowColumns("password"))) + t.Run("allow all but one, failing", f(`{"$and": [{"name": "John"}, {"password": "hacks"}]}`, no("password"), filter.WithAllowAllColumns(), filter.WithDisallowColumns("password"))) + t.Run("nested but disallow password, allow exception", f(`{"created_at": "1"}`, nil, filter.WithNestedJSONB("meta", "created_at"), filter.WithDisallowColumns("password"))) + t.Run("nested but disallow password, allow nested", f(`{"map": "de_dust"}`, nil, filter.WithNestedJSONB("meta", "created_at"), filter.WithDisallowColumns("password"))) + t.Run("nested but disallow password, disallow", f(`{"password": "hacks"}`, no("password"), filter.WithNestedJSONB("meta", "created_at"), filter.WithDisallowColumns("password"))) }