[#6201] expanded the hidden fields check and allow targetting hidden fields in the List API rule
This commit is contained in:
		
							parent
							
								
									2af9b554ad
								
							
						
					
					
						commit
						a8952cfca2
					
				|  | @ -1,6 +1,6 @@ | |||
| ## v0.24.0 (WIP) | ||||
| 
 | ||||
| - Added `@yesterday` and `@tomorrow` date filter macros (@todo docs). | ||||
| - Added `@yesterday` and `@tomorrow` datetime filter macros. | ||||
| 
 | ||||
| - Added `:lower` filter modifier (e.g. `title:lower = "lorem"`). | ||||
| 
 | ||||
|  | @ -25,6 +25,8 @@ | |||
| 
 | ||||
| - Eagerly interrupt waiting for the email alert send in case it takes longer than 15s. | ||||
| 
 | ||||
| - Allowed targetting "Hidden" fields in the List API rule. | ||||
| 
 | ||||
| - ⚠️ Removed the "dry submit" when executing the collections Create API rule | ||||
|     (you can find more details why this change was introduced and how it could affect your app in https://github.com/pocketbase/pocketbase/discussions/6073). | ||||
|     For most users it should be non-breaking change, BUT if you have Create API rules that uses self-references or view counters you may have to adjust them manually. | ||||
|  |  | |||
|  | @ -59,23 +59,27 @@ func recordsList(e *core.RequestEvent) error { | |||
| 		return err | ||||
| 	} | ||||
| 
 | ||||
| 	fieldsResolver := core.NewRecordFieldResolver( | ||||
| 		e.App, | ||||
| 		collection, | ||||
| 		requestInfo, | ||||
| 		// hidden fields are searchable only by superusers
 | ||||
| 		requestInfo.HasSuperuserAuth(), | ||||
| 	) | ||||
| 	query := e.App.RecordQuery(collection) | ||||
| 
 | ||||
| 	searchProvider := search.NewProvider(fieldsResolver). | ||||
| 		Query(e.App.RecordQuery(collection)) | ||||
| 	fieldsResolver := core.NewRecordFieldResolver(e.App, collection, requestInfo, true) | ||||
| 
 | ||||
| 	if !requestInfo.HasSuperuserAuth() && collection.ListRule != nil { | ||||
| 		searchProvider.AddFilter(search.FilterData(*collection.ListRule)) | ||||
| 	if !requestInfo.HasSuperuserAuth() && collection.ListRule != nil && *collection.ListRule != "" { | ||||
| 		expr, err := search.FilterData(*collection.ListRule).BuildExpr(fieldsResolver) | ||||
| 		if err != nil { | ||||
| 			return err | ||||
| 		} | ||||
| 		query.AndWhere(expr) | ||||
| 
 | ||||
| 		// will be applied by the search provider right before executing the query
 | ||||
| 		// fieldsResolver.UpdateQuery(query)
 | ||||
| 	} | ||||
| 
 | ||||
| 	records := []*core.Record{} | ||||
| 	// hidden fields are searchable only by superusers
 | ||||
| 	fieldsResolver.SetAllowHiddenFields(requestInfo.HasSuperuserAuth()) | ||||
| 
 | ||||
| 	searchProvider := search.NewProvider(fieldsResolver).Query(query) | ||||
| 
 | ||||
| 	records := []*core.Record{} | ||||
| 	result, err := searchProvider.ParseAndExec(e.Request.URL.Query().Encode(), &records) | ||||
| 	if err != nil { | ||||
| 		return firstApiError(err, e.BadRequestError("", err)) | ||||
|  | @ -109,7 +113,7 @@ func recordsList(e *core.RequestEvent) error { | |||
| 			len(e.Records) == 0 && | ||||
| 			checkRateLimit(e.RequestEvent, "@pb_list_timing_check_"+collection.Id, listTimingRateLimitRule) != nil { | ||||
| 			e.App.Logger().Debug("Randomized throttle because of too many failed searches", "collectionId", collection.Id) | ||||
| 			randomizedThrottle(100) | ||||
| 			randomizedThrottle(150) | ||||
| 		} | ||||
| 
 | ||||
| 		return e.JSON(http.StatusOK, e.Result) | ||||
|  |  | |||
|  | @ -259,6 +259,47 @@ func TestRecordCrudList(t *testing.T) { | |||
| 				"OnRecordEnrich":       4, | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			Name:   "authenticated regular record that matches the collection list rule with hidden field", | ||||
| 			Method: http.MethodGet, | ||||
| 			URL:    "/api/collections/demo3/records", | ||||
| 			Headers: map[string]string{ | ||||
| 				// clients, test@example.com
 | ||||
| 				"Authorization": "eyJhbGciOiJIUzI1NiJ9.eyJpZCI6ImdrMzkwcWVnczR5NDd3biIsInR5cGUiOiJhdXRoIiwiY29sbGVjdGlvbklkIjoidjg1MXE0cjc5MHJoa25sIiwiZXhwIjoyNTI0NjA0NDYxLCJyZWZyZXNoYWJsZSI6dHJ1ZX0.0ONnm_BsvPRZyDNT31GN1CKUB6uQRxvVvQ-Wc9AZfG0", | ||||
| 			}, | ||||
| 			BeforeTestFunc: func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) { | ||||
| 				col, err := app.FindCollectionByNameOrId("demo3") | ||||
| 				if err != nil { | ||||
| 					t.Fatal(err) | ||||
| 				} | ||||
| 
 | ||||
| 				// mock hidden field
 | ||||
| 				col.Fields.GetByName("title").SetHidden(true) | ||||
| 
 | ||||
| 				col.ListRule = types.Pointer("title ~ 'test'") | ||||
| 
 | ||||
| 				if err = app.Save(col); err != nil { | ||||
| 					t.Fatal(err) | ||||
| 				} | ||||
| 			}, | ||||
| 			ExpectedStatus: 200, | ||||
| 			ExpectedContent: []string{ | ||||
| 				`"page":1`, | ||||
| 				`"perPage":30`, | ||||
| 				`"totalPages":1`, | ||||
| 				`"totalItems":4`, | ||||
| 				`"items":[{`, | ||||
| 				`"id":"1tmknxy2868d869"`, | ||||
| 				`"id":"lcl9d87w22ml6jy"`, | ||||
| 				`"id":"7nwo8tuiatetxdm"`, | ||||
| 				`"id":"mk5fmymtx4wsprk"`, | ||||
| 			}, | ||||
| 			ExpectedEvents: map[string]int{ | ||||
| 				"*":                    0, | ||||
| 				"OnRecordsListRequest": 1, | ||||
| 				"OnRecordEnrich":       4, | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			Name:   "authenticated regular record filtering with a hidden field", | ||||
| 			Method: http.MethodGet, | ||||
|  |  | |||
|  | @ -4,6 +4,7 @@ import ( | |||
| 	"encoding/json" | ||||
| 	"errors" | ||||
| 	"fmt" | ||||
| 	"slices" | ||||
| 	"strconv" | ||||
| 	"strings" | ||||
| 
 | ||||
|  | @ -48,6 +49,26 @@ type RecordFieldResolver struct { | |||
| 	allowHiddenFields bool | ||||
| } | ||||
| 
 | ||||
| // AllowedFields returns a copy of the resolver's allowed fields.
 | ||||
| func (r *RecordFieldResolver) AllowedFields() []string { | ||||
| 	return slices.Clone(r.allowedFields) | ||||
| } | ||||
| 
 | ||||
| // SetAllowedFields replaces the resolver's allowed fields with the new ones.
 | ||||
| func (r *RecordFieldResolver) SetAllowedFields(newAllowedFields []string) { | ||||
| 	r.allowedFields = slices.Clone(newAllowedFields) | ||||
| } | ||||
| 
 | ||||
| // AllowHiddenFields returns whether the current resolver allows filtering hidden fields.
 | ||||
| func (r *RecordFieldResolver) AllowHiddenFields() bool { | ||||
| 	return r.allowHiddenFields | ||||
| } | ||||
| 
 | ||||
| // SetAllowHiddenFields enables or disables hidden fields filtering.
 | ||||
| func (r *RecordFieldResolver) SetAllowHiddenFields(allowHiddenFields bool) { | ||||
| 	r.allowHiddenFields = allowHiddenFields | ||||
| } | ||||
| 
 | ||||
| // NewRecordFieldResolver creates and initializes a new `RecordFieldResolver`.
 | ||||
| func NewRecordFieldResolver( | ||||
| 	app App, | ||||
|  |  | |||
|  | @ -415,6 +415,10 @@ func (r *runner) processActiveProps() (*search.ResolverResult, error) { | |||
| 
 | ||||
| 		field := collection.Fields.GetByName(prop) | ||||
| 
 | ||||
| 		if field != nil && field.GetHidden() && !r.allowHiddenFields { | ||||
| 			return nil, fmt.Errorf("non-filterable field %q", prop) | ||||
| 		} | ||||
| 
 | ||||
| 		// json field -> treat the rest of the props as json path
 | ||||
| 		if field != nil && field.Type() == FieldTypeJSON { | ||||
| 			var jsonPath strings.Builder | ||||
|  | @ -479,6 +483,10 @@ func (r *runner) processActiveProps() (*search.ResolverResult, error) { | |||
| 				return nil, fmt.Errorf("invalid back relation field %q", parts[2]) | ||||
| 			} | ||||
| 
 | ||||
| 			if backField.GetHidden() && !r.allowHiddenFields { | ||||
| 				return nil, fmt.Errorf("non-filterable back relation field %q", backField.GetName()) | ||||
| 			} | ||||
| 
 | ||||
| 			backRelField, ok := backField.(*RelationField) | ||||
| 			if !ok { | ||||
| 				return nil, fmt.Errorf("failed to initialize back relation field %q", backField.GetName()) | ||||
|  |  | |||
|  | @ -3,6 +3,7 @@ package core_test | |||
| import ( | ||||
| 	"encoding/json" | ||||
| 	"regexp" | ||||
| 	"slices" | ||||
| 	"strings" | ||||
| 	"testing" | ||||
| 
 | ||||
|  | @ -12,7 +13,75 @@ import ( | |||
| 	"github.com/pocketbase/pocketbase/tools/search" | ||||
| ) | ||||
| 
 | ||||
| func TestRecordFieldResolverAllowedFields(t *testing.T) { | ||||
| 	t.Parallel() | ||||
| 
 | ||||
| 	app, _ := tests.NewTestApp() | ||||
| 	defer app.Cleanup() | ||||
| 
 | ||||
| 	collection, err := app.FindCollectionByNameOrId("demo1") | ||||
| 	if err != nil { | ||||
| 		t.Fatal(err) | ||||
| 	} | ||||
| 
 | ||||
| 	r := core.NewRecordFieldResolver(app, collection, nil, false) | ||||
| 
 | ||||
| 	fields := r.AllowedFields() | ||||
| 	if len(fields) != 8 { | ||||
| 		t.Fatalf("Expected %d original allowed fields, got %d", 8, len(fields)) | ||||
| 	} | ||||
| 
 | ||||
| 	// change the allowed fields
 | ||||
| 	newFields := []string{"a", "b", "c"} | ||||
| 	expected := slices.Clone(newFields) | ||||
| 	r.SetAllowedFields(newFields) | ||||
| 
 | ||||
| 	// change the new fields to ensure that the slice was cloned
 | ||||
| 	newFields[2] = "d" | ||||
| 
 | ||||
| 	fields = r.AllowedFields() | ||||
| 	if len(fields) != len(expected) { | ||||
| 		t.Fatalf("Expected %d changed allowed fields, got %d", len(expected), len(fields)) | ||||
| 	} | ||||
| 
 | ||||
| 	for i, v := range expected { | ||||
| 		if fields[i] != v { | ||||
| 			t.Errorf("[%d] Expected field %q", i, v) | ||||
| 		} | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| func TestRecordFieldResolverAllowHiddenFields(t *testing.T) { | ||||
| 	t.Parallel() | ||||
| 
 | ||||
| 	app, _ := tests.NewTestApp() | ||||
| 	defer app.Cleanup() | ||||
| 
 | ||||
| 	collection, err := app.FindCollectionByNameOrId("demo1") | ||||
| 	if err != nil { | ||||
| 		t.Fatal(err) | ||||
| 	} | ||||
| 
 | ||||
| 	r := core.NewRecordFieldResolver(app, collection, nil, false) | ||||
| 
 | ||||
| 	allowHiddenFields := r.AllowHiddenFields() | ||||
| 	if allowHiddenFields { | ||||
| 		t.Fatalf("Expected original allowHiddenFields %v, got %v", allowHiddenFields, !allowHiddenFields) | ||||
| 	} | ||||
| 
 | ||||
| 	// change the flag
 | ||||
| 	expected := !allowHiddenFields | ||||
| 	r.SetAllowHiddenFields(expected) | ||||
| 
 | ||||
| 	allowHiddenFields = r.AllowHiddenFields() | ||||
| 	if allowHiddenFields != expected { | ||||
| 		t.Fatalf("Expected changed allowHiddenFields %v, got %v", expected, allowHiddenFields) | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| func TestRecordFieldResolverUpdateQuery(t *testing.T) { | ||||
| 	t.Parallel() | ||||
| 
 | ||||
| 	app, _ := tests.NewTestApp() | ||||
| 	defer app.Cleanup() | ||||
| 
 | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue