From b0f027d27a39f93e8bbc60e86c0ccbcef68e4aa2 Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Sun, 10 Dec 2023 21:06:02 +0200 Subject: [PATCH] updated changelog formatting and temp moved the admin only rule checks to the record_helpers --- CHANGELOG.md | 47 ++++++++++++++++++++-------------------- apis/realtime.go | 4 ++++ apis/record_crud.go | 25 +++------------------ apis/record_crud_test.go | 11 ++++++++-- apis/record_helpers.go | 28 ++++++++++++++++++++++++ 5 files changed, 67 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1d2a45a..afc14eb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,36 +38,35 @@ 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 + - 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 Some notable log related changes: - - ⚠️ Bumped the minimum required Go version to 1.21. + - ⚠️ Bumped the minimum required Go version to 1.21. - - ⚠️ Removed `_requests` table in favor of the generalized `_logs`. - _Note that existing logs will be deleted!_ + - ⚠️ Removed `_requests` table in favor of the generalized `_logs`. + _Note that existing logs will be deleted!_ - - ⚠️ Renamed the following `Dao` log methods: - ```go - Dao.RequestQuery(...) -> Dao.LogQuery(...) - Dao.FindRequestById(...) -> Dao.FindLogById(...) - Dao.RequestsStats(...) -> Dao.LogsStats(...) - Dao.DeleteOldRequests(...) -> Dao.DeleteOldLogs(...) - Dao.SaveRequest(...) -> Dao.SaveLog(...) - ``` - - ⚠️ Removed `app.IsDebug()` and the `--debug` flag. - This was done to avoid the confusion with the new logger and its debug severity level. - If you want to store debug logs you can set `-4` as min log level from the Admin UI. + - ⚠️ Renamed the following `Dao` log methods: + ```go + Dao.RequestQuery(...) -> Dao.LogQuery(...) + Dao.FindRequestById(...) -> Dao.FindLogById(...) + Dao.RequestsStats(...) -> Dao.LogsStats(...) + Dao.DeleteOldRequests(...) -> Dao.DeleteOldLogs(...) + Dao.SaveRequest(...) -> Dao.SaveLog(...) + ``` + - ⚠️ Removed `app.IsDebug()` and the `--debug` flag. + This was done to avoid the confusion with the new logger and its debug severity level. + If you want to store debug logs you can set `-4` as min log level from the Admin UI. - - Refactored Admin UI Logs: - - Added new logs table listing. - - Added log settings option to toggle the IP logging for the activity logger. - - Added log settings option to specify a minimum log level. - - Added controls to export individual or bulk selected logs as json. - - Other minor improvements and fixes. + - Refactored Admin UI Logs: + - Added new logs table listing. + - Added log settings option to toggle the IP logging for the activity logger. + - Added log settings option to specify a minimum log level. + - Added controls to export individual or bulk selected logs as json. + - Other minor improvements and fixes. - Added new `filesystem/System.Copy(src, dest)` method to copy existing files from one location to another. _This is usually useful when duplicating records with `file` field(s) programmatically._ diff --git a/apis/realtime.go b/apis/realtime.go index 9467a755..13ef37f3 100644 --- a/apis/realtime.go +++ b/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) diff --git a/apis/record_crud.go b/apis/record_crud.go index 18e3e627..106570e1 100644 --- a/apis/record_crud.go +++ b/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 -} diff --git a/apis/record_crud_test.go b/apis/record_crud_test.go index 0abeeaf1..d1e56135 100644 --- a/apis/record_crud_test.go +++ b/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":{}`}, }, diff --git a/apis/record_helpers.go b/apis/record_helpers.go index aae28d44..bb1bb2fe 100644 --- a/apis/record_helpers.go +++ b/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 +}