updated changelog formatting and temp moved the admin only rule checks to the record_helpers

CHANGELOG.md

@@ -38,7 +38,6 @@
 
     For better performance and to minimize blocking on hot paths, logs are currently written with
     debounce and on batches:
-
     - 3 seconds after the last debounced log write
     - when the batch threshold is reached (currently 200)
     - right before app termination to attempt saving everything from the existing logs queue

apis/realtime.go

@@ -568,6 +568,10 @@ func (api *realtimeApi) canAccessRecord(
 		return true // no further checks needed
 	}
 
+	if err := checkForAdminOnlyRuleFields(requestInfo); err != nil {
+		return false
+	}
+
 	ruleFunc := func(q *dbx.SelectQuery) error {
 		resolver := resolvers.NewRecordFieldResolver(api.app.Dao(), record.Collection(), requestInfo, false)
 

apis/record_crud.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"log/slog"
 	"net/http"
-	"strings"
 
 	"github.com/labstack/echo/v5"
 	"github.com/pocketbase/dbx"
@@ -43,13 +42,13 @@ func (api *recordApi) list(c echo.Context) error {
 		return NewNotFoundError("", "Missing collection context.")
 	}
 
+	requestInfo := RequestInfo(c)
+
 	// forbid users and guests to query special filter/sort fields
-	if err := api.checkForForbiddenQueryFields(c); err != nil {
+	if err := checkForAdminOnlyRuleFields(requestInfo); err != nil {
 		return err
 	}
 
-	requestInfo := RequestInfo(c)
-
 	if requestInfo.Admin == nil && collection.ListRule == nil {
 		// only admins can access if the rule is nil
 		return NewForbiddenError("Only admins can perform this action.", nil)
@@ -409,21 +408,3 @@ func (api *recordApi) delete(c echo.Context) error {
 		})
 	})
 }
-
-func (api *recordApi) checkForForbiddenQueryFields(c echo.Context) error {
-	admin, _ := c.Get(ContextAdminKey).(*models.Admin)
-	if admin != nil {
-		return nil // admins are allowed to query everything
-	}
-
-	decodedQuery := c.QueryParam(search.FilterQueryParam) + c.QueryParam(search.SortQueryParam)
-	forbiddenFields := []string{"@collection.", "@request."}
-
-	for _, field := range forbiddenFields {
-		if strings.Contains(decodedQuery, field) {
-			return NewForbiddenError("Only admins can filter by @collection and @request query params", nil)
-		}
-	}
-
-	return nil
-}

apis/record_crud_test.go

@@ -43,9 +43,16 @@ func TestRecordCrudList(t *testing.T) {
 			ExpectedContent: []string{`"data":{}`},
 		},
 		{
-			Name:            "public collection but with admin only filter/sort (aka. @collection)",
+			Name:            "public collection but with admin only filter param (aka. @collection, @request, etc.)",
 			Method:          http.MethodGet,
-			Url:             "/api/collections/demo2/records?filter=@collection.demo2.title='test1'",
+			Url:             "/api/collections/demo2/records?filter=%40collection.demo2.title='test1'",
+			ExpectedStatus:  403,
+			ExpectedContent: []string{`"data":{}`},
+		},
+		{
+			Name:            "public collection but with admin only sort param (aka. @collection, @request, etc.)",
+			Method:          http.MethodGet,
+			Url:             "/api/collections/demo2/records?sort=@request.auth.title",
 			ExpectedStatus:  403,
 			ExpectedContent: []string{`"data":{}`},
 		},

apis/record_helpers.go

@@ -314,3 +314,31 @@ func hasAuthManageAccess(
 
 	return findErr == nil
 }
+
+var ruleQueryParams = []string{search.FilterQueryParam, search.SortQueryParam}
+var adminOnlyRuleFields = []string{"@collection.", "@request."}
+
+// @todo consider moving the rules check to the RecordFieldResolver.
+//
+// checkForAdminOnlyRuleFields loosely checks and returns an error if
+// the provided RequestInfo contains rule fields that only the admin can use.
+func checkForAdminOnlyRuleFields(requestInfo *models.RequestInfo) error {
+	if requestInfo.Admin != nil || len(requestInfo.Query) == 0 {
+		return nil // admin or nothing to check
+	}
+
+	for _, param := range ruleQueryParams {
+		v, _ := requestInfo.Query[param].(string)
+		if v == "" {
+			continue
+		}
+
+		for _, field := range adminOnlyRuleFields {
+			if strings.Contains(v, field) {
+				return NewForbiddenError("Only admins can filter by "+field, nil)
+			}
+		}
+	}
+
+	return nil
+}