lock the _mfas and _otps delete api rule, fixed flaky tests, fixed jsvm types example

CHANGELOG.md

@@ -1,3 +1,19 @@
+## v0.23.0-rc8 (WIP)
+
+> [!CAUTION]
+> **This is a prerelease intended for test and experimental purposes only!**
+
+- Lock the `_otps` and `_mfas` system collections Delete API rule for superusers only.
+
+- Reassign in the JSVM executors the global `$app` variable with the hook scoped `e.app` value to minimize the risk of a deadlock when a hook or middleware is wrapped in a transaction.
+
+- Reuse the OAuth2 created user record pointer to ensure that all its following hooks operate on the same record instance.
+
+- Added tags support for the `OnRecordFileToken` hook.
+
+- Added more detailed godoc for the collection fields and `core.App`.
+
+
 ## v0.23.0-rc7
 
 > [!CAUTION]

apis/record_crud_mfa_test.go

@@ -170,7 +170,7 @@ func TestRecordCrudMFADelete(t *testing.T) {
 					t.Fatal(err)
 				}
 			},
-			ExpectedStatus:  404,
+			ExpectedStatus:  403,
 			ExpectedContent: []string{`"data":{}`},
 			ExpectedEvents:  map[string]int{"*": 0},
 		},
@@ -187,7 +187,7 @@ func TestRecordCrudMFADelete(t *testing.T) {
 					t.Fatal(err)
 				}
 			},
-			ExpectedStatus:  404,
+			ExpectedStatus:  403,
 			ExpectedContent: []string{`"data":{}`},
 			ExpectedEvents:  map[string]int{"*": 0},
 		},
@@ -204,6 +204,23 @@ func TestRecordCrudMFADelete(t *testing.T) {
 					t.Fatal(err)
 				}
 			},
+			ExpectedStatus:  403,
+			ExpectedContent: []string{`"data":{}`},
+			ExpectedEvents:  map[string]int{"*": 0},
+		},
+		{
+			Name:   "superusers auth",
+			Method: http.MethodDelete,
+			URL:    "/api/collections/" + core.CollectionNameMFAs + "/records/user1_0",
+			Headers: map[string]string{
+				// superusers, test@example.com
+				"Authorization": "eyJhbGciOiJIUzI1NiJ9.eyJpZCI6InN5d2JoZWNuaDQ2cmhtMCIsInR5cGUiOiJhdXRoIiwiY29sbGVjdGlvbklkIjoiX3BiY18zMzIzODY2MzM5IiwiZXhwIjoyNTI0NjA0NDYxLCJyZWZyZXNoYWJsZSI6dHJ1ZX0.v_bMAygr6hXPwD2DpPrFpNQ7dd68Q3pGstmYAsvNBJg",
+			},
+			BeforeTestFunc: func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) {
+				if err := tests.StubMFARecords(app); err != nil {
+					t.Fatal(err)
+				}
+			},
 			ExpectedStatus: 204,
 			ExpectedEvents: map[string]int{
 				"*":                          0,

apis/record_crud_otp_test.go

@@ -170,12 +170,12 @@ func TestRecordCrudOTPDelete(t *testing.T) {
 					t.Fatal(err)
 				}
 			},
-			ExpectedStatus:  404,
+			ExpectedStatus:  403,
 			ExpectedContent: []string{`"data":{}`},
 			ExpectedEvents:  map[string]int{"*": 0},
 		},
 		{
-			Name:   "non-owner",
+			Name:   "non-owner auth",
 			Method: http.MethodDelete,
 			URL:    "/api/collections/" + core.CollectionNameOTPs + "/records/user1_0",
 			Headers: map[string]string{
@@ -187,12 +187,12 @@ func TestRecordCrudOTPDelete(t *testing.T) {
 					t.Fatal(err)
 				}
 			},
-			ExpectedStatus:  404,
+			ExpectedStatus:  403,
 			ExpectedContent: []string{`"data":{}`},
 			ExpectedEvents:  map[string]int{"*": 0},
 		},
 		{
-			Name:   "owner",
+			Name:   "owner regular auth",
 			Method: http.MethodDelete,
 			URL:    "/api/collections/" + core.CollectionNameOTPs + "/records/user1_0",
 			Headers: map[string]string{
@@ -204,6 +204,23 @@ func TestRecordCrudOTPDelete(t *testing.T) {
 					t.Fatal(err)
 				}
 			},
+			ExpectedStatus:  403,
+			ExpectedContent: []string{`"data":{}`},
+			ExpectedEvents:  map[string]int{"*": 0},
+		},
+		{
+			Name:   "superusers auth",
+			Method: http.MethodDelete,
+			URL:    "/api/collections/" + core.CollectionNameOTPs + "/records/user1_0",
+			Headers: map[string]string{
+				// superusers, test@example.com
+				"Authorization": "eyJhbGciOiJIUzI1NiJ9.eyJpZCI6InN5d2JoZWNuaDQ2cmhtMCIsInR5cGUiOiJhdXRoIiwiY29sbGVjdGlvbklkIjoiX3BiY18zMzIzODY2MzM5IiwiZXhwIjoyNTI0NjA0NDYxLCJyZWZyZXNoYWJsZSI6dHJ1ZX0.v_bMAygr6hXPwD2DpPrFpNQ7dd68Q3pGstmYAsvNBJg",
+			},
+			BeforeTestFunc: func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) {
+				if err := tests.StubOTPRecords(app); err != nil {
+					t.Fatal(err)
+				}
+			},
 			ExpectedStatus: 204,
 			ExpectedEvents: map[string]int{
 				"*":                          0,

apis/record_helpers.go

@@ -256,13 +256,13 @@ func EnrichRecords(e *core.RequestEvent, records []*core.Record, defaultExpands
 
 	return triggerRecordEnrichHooks(e.App, info, records, func() error {
 		expands := defaultExpands
-		if param := e.Request.URL.Query().Get(expandQueryParam); param != "" {
+		if param := info.Query[expandQueryParam]; param != "" {
 			expands = append(expands, strings.Split(param, ",")...)
 		}
 
 		err := defaultEnrichRecords(e.App, info, records, expands...)
 		if err != nil {
-			// only log as it is not critical
+			// only log because it is not critical
 			e.App.Logger().Warn("failed to apply default enriching", "error", err)
 		}
 
@@ -270,8 +270,6 @@ func EnrichRecords(e *core.RequestEvent, records []*core.Record, defaultExpands
 	})
 }
 
-var iterate func(record *core.Record) error
-
 type iterator[T any] struct {
 	items []T
 	index int
@@ -297,6 +295,7 @@ func triggerRecordEnrichHooks(app core.App, requestInfo *core.RequestInfo, recor
 	event.App = app
 	event.RequestInfo = requestInfo
 
+	var iterate func(record *core.Record) error
 	iterate = func(record *core.Record) error {
 		if record == nil {
 			return nil
@@ -350,6 +349,7 @@ func defaultEnrichRecords(app core.App, requestInfo *core.RequestInfo, records [
 
 // expandFetch is the records fetch function that is used to expand related records.
 func expandFetch(app core.App, originalRequestInfo *core.RequestInfo) core.ExpandFetchFunc {
+	// shallow clone the provided request info to set an "expand" context
 	requestInfoClone := *originalRequestInfo
 	requestInfoPtr := &requestInfoClone
 	requestInfoPtr.Context = core.RequestInfoContextExpand

apis/record_helpers_test.go

@@ -23,6 +23,14 @@ func TestEnrichRecords(t *testing.T) {
 	app, _ := tests.NewTestApp()
 	defer app.Cleanup()
 
+	freshRecords := func(records []*core.Record) []*core.Record {
+		result := make([]*core.Record, len(records))
+		for i, r := range records {
+			result[i] = r.Fresh()
+		}
+		return result
+	}
+
 	user, err := app.FindAuthRecordByEmail("users", "test@example.com")
 	if err != nil {
 		t.Fatal(err)
@@ -77,7 +85,7 @@ func TestEnrichRecords(t *testing.T) {
 		{
 			name:           "[emailVisibility] guest",
 			auth:           nil,
-			records:        usersRecords,
+			records:        freshRecords(usersRecords),
 			queryExpand:    "",
 			defaultExpands: nil,
 			expected: []string{
@@ -91,7 +99,7 @@ func TestEnrichRecords(t *testing.T) {
 		{
 			name:           "[emailVisibility] owner",
 			auth:           user,
-			records:        usersRecords,
+			records:        freshRecords(usersRecords),
 			queryExpand:    "",
 			defaultExpands: nil,
 			expected: []string{
@@ -103,7 +111,7 @@ func TestEnrichRecords(t *testing.T) {
 		{
 			name:           "[emailVisibility] manager",
 			auth:           user,
-			records:        nologinRecords,
+			records:        freshRecords(nologinRecords),
 			queryExpand:    "",
 			defaultExpands: nil,
 			expected: []string{
@@ -115,7 +123,7 @@ func TestEnrichRecords(t *testing.T) {
 		{
 			name:           "[emailVisibility] superuser",
 			auth:           superuser,
-			records:        nologinRecords,
+			records:        freshRecords(nologinRecords),
 			queryExpand:    "",
 			defaultExpands: nil,
 			expected: []string{
@@ -127,7 +135,7 @@ func TestEnrichRecords(t *testing.T) {
 		{
 			name:           "[emailVisibility + expand] recursive auth rule checks (regular user)",
 			auth:           user,
-			records:        demo1Records,
+			records:        freshRecords(demo1Records),
 			queryExpand:    "",
 			defaultExpands: []string{"rel_many"},
 			expected: []string{
@@ -144,7 +152,7 @@ func TestEnrichRecords(t *testing.T) {
 		{
 			name:           "[emailVisibility + expand] recursive auth rule checks (superuser)",
 			auth:           superuser,
-			records:        demo1Records,
+			records:        freshRecords(demo1Records),
 			queryExpand:    "",
 			defaultExpands: []string{"rel_many"},
 			expected: []string{
@@ -164,7 +172,7 @@ func TestEnrichRecords(t *testing.T) {
 		{
 			name:           "[expand] guest (query)",
 			auth:           nil,
-			records:        usersRecords,
+			records:        freshRecords(usersRecords),
 			queryExpand:    "rel",
 			defaultExpands: nil,
 			expected: []string{
@@ -180,7 +188,7 @@ func TestEnrichRecords(t *testing.T) {
 		{
 			name:           "[expand] guest (default expands)",
 			auth:           nil,
-			records:        usersRecords,
+			records:        freshRecords(usersRecords),
 			queryExpand:    "",
 			defaultExpands: []string{"rel"},
 			expected: []string{
@@ -193,7 +201,7 @@ func TestEnrichRecords(t *testing.T) {
 		{
 			name:           "[expand] @request.context=expand check",
 			auth:           nil,
-			records:        demo5Records,
+			records:        freshRecords(demo5Records),
 			queryExpand:    "rel_one",
 			defaultExpands: []string{"rel_many"},
 			expected: []string{

migrations/1640988000_init.go

@@ -51,6 +51,8 @@ func init() {
 				[[created]]    TEXT DEFAULT (strftime('%Y-%m-%d %H:%M:%fZ')) NOT NULL,
 				[[updated]]    TEXT DEFAULT (strftime('%Y-%m-%d %H:%M:%fZ')) NOT NULL
 			);
+
+			CREATE INDEX IF NOT EXISTS idx__collections_type on {{_collections}} ([[type]]);
 		`).Execute()
 		if execerr != nil {
 			return fmt.Errorf("_collections exec error: %w", execerr)
@@ -122,7 +124,6 @@ func createMFAsCollection(txApp core.App) error {
 	ownerRule := "@request.auth.id != '' && recordRef = @request.auth.id && collectionRef = @request.auth.collectionId"
 	col.ListRule = types.Pointer(ownerRule)
 	col.ViewRule = types.Pointer(ownerRule)
-	col.DeleteRule = types.Pointer(ownerRule)
 
 	col.Fields.Add(&core.TextField{
 		Name:     "collectionRef",
@@ -162,7 +163,6 @@ func createOTPsCollection(txApp core.App) error {
 	ownerRule := "@request.auth.id != '' && recordRef = @request.auth.id && collectionRef = @request.auth.collectionId"
 	col.ListRule = types.Pointer(ownerRule)
 	col.ViewRule = types.Pointer(ownerRule)
-	col.DeleteRule = types.Pointer(ownerRule)
 
 	col.Fields.Add(&core.TextField{
 		Name:     "collectionRef",

migrations/1717233557_v0.23_migrate2.go

@@ -0,0 +1,35 @@
+package migrations
+
+import (
+	"github.com/pocketbase/pocketbase/core"
+)
+
+// note: this migration will be deleted in future version
+
+func init() {
+	core.SystemMigrations.Register(func(txApp core.App) error {
+		_, err := txApp.DB().NewQuery("CREATE INDEX IF NOT EXISTS idx__collections_type on {{_collections}} ([[type]]);").Execute()
+		if err != nil {
+			return err
+		}
+
+		// reset mfas and otps delete rule
+		collectionNames := []string{core.CollectionNameMFAs, core.CollectionNameOTPs}
+		for _, name := range collectionNames {
+			col, err := txApp.FindCollectionByNameOrId(name)
+			if err != nil {
+				return err
+			}
+
+			if col.DeleteRule != nil {
+				col.DeleteRule = nil
+				err = txApp.SaveNoValidate(col)
+				if err != nil {
+					return err
+				}
+			}
+		}
+
+		return nil
+	}, nil)
+}

plugins/jsvm/binds.go

@@ -55,6 +55,8 @@ func hooksBinds(app core.App, loader *goja.Runtime, executors *vmsPool) {
 
 		// register the hook to the loader
 		loader.Set(jsName, func(callback string, tags ...string) {
+			// overwrite the global $app with the hook scoped instance
+			callback = `function(e) { $app = e.app; return (` + callback + `).call(undefined, e) }`
 			pr := goja.MustCompile("", "{("+callback+").apply(undefined, __args)}", true)
 
 			tagsAsValues := make([]reflect.Value, len(tags))
@@ -74,6 +76,7 @@ func hooksBinds(app core.App, loader *goja.Runtime, executors *vmsPool) {
 				}
 
 				err := executors.run(func(executor *goja.Runtime) error {
+					executor.Set("$app", goja.Undefined())
 					executor.Set("__args", handlerArgs)
 					res, err := executor.RunProgram(pr)
 					executor.Set("__args", goja.Undefined())
@@ -189,6 +192,7 @@ func wrapHandlerFunc(executors *vmsPool, handler goja.Value) (func(*core.Request
 
 		wrappedHandler := func(e *core.RequestEvent) error {
 			return executors.run(func(executor *goja.Runtime) error {
+				executor.Set("$app", e.App) // overwrite the global $app with the hook scoped instance
 				executor.Set("__args", []any{e})
 				res, err := executor.RunProgram(pr)
 				executor.Set("__args", goja.Undefined())
@@ -245,6 +249,7 @@ func wrapMiddlewares(executors *vmsPool, rawMiddlewares ...goja.Value) ([]*hook.
 				Priority: v.priority,
 				Func: func(e *core.RequestEvent) error {
 					return executors.run(func(executor *goja.Runtime) error {
+						executor.Set("$app", e.App) // overwrite the global $app with the hook scoped instance
 						executor.Set("__args", []any{e})
 						res, err := executor.RunProgram(pr)
 						executor.Set("__args", goja.Undefined())
@@ -266,6 +271,7 @@ func wrapMiddlewares(executors *vmsPool, rawMiddlewares ...goja.Value) ([]*hook.
 			wrappedMiddlewares[i] = &hook.Handler[*core.RequestEvent]{
 				Func: func(e *core.RequestEvent) error {
 					return executors.run(func(executor *goja.Runtime) error {
+						executor.Set("$app", e.App) // overwrite the global $app with the hook scoped instance
 						executor.Set("__args", []any{e})
 						res, err := executor.RunProgram(pr)
 						executor.Set("__args", goja.Undefined())

plugins/jsvm/internal/types/types.go

@@ -217,7 +217,7 @@ declare function sleep(milliseconds: number): void;
  * ` + "```" + `js
  * const records = arrayOf(new Record)
  *
- * $app.dao().recordQuery("articles").limit(10).all(records)
+ * $app.recordQuery("articles").limit(10).all(records)
  * ` + "```" + `
  *
  * @group PocketBase
@@ -279,7 +279,7 @@ declare class Context implements context.Context {
  * Record model class.
  *
  * ` + "```" + `js
- * const collection = $app.dao().findCollectionByNameOrId("article")
+ * const collection = $app.findCollectionByNameOrId("article")
  *
  * const record = new Record(collection, {
  *     title: "Lorem ipsum"

ui/.env

@@ -11,4 +11,4 @@ PB_DOCS_URL            = "https://pocketbase.io/docs/"
 PB_JS_SDK_URL          = "https://github.com/pocketbase/js-sdk"
 PB_DART_SDK_URL        = "https://github.com/pocketbase/dart-sdk"
 PB_RELEASES            = "https://github.com/pocketbase/pocketbase/releases"
-PB_VERSION             = "v0.23.0-rc7"
+PB_VERSION             = "v0.23.0-rc8"