[pull] master from pocketbase:master

CHANGELOG.md

@@ -1,3 +1,10 @@
+## v0.40.1
+
+- Fixes for some reported regressions related to the `encoding/json/v2` update:
+    - allow mangling invalid UTF8 characters when serializing json data ([#7814](https://github.com/pocketbase/pocketbase/issues/7814))
+    - fixed OAuth2 providers config merge incorrectly replacing the entire slice ([#7815](https://github.com/pocketbase/pocketbase/issues/7815))
+
+
 ## v0.40.0
 
 - Propagate console command errors and recovered panics to `app.Start()` so that the program can exit with non-zero code while still ensuring that `app.OnTerminate` hook was triggered _(responsible for the app graceful shutdown handling)_.

apis/collection_test.go

@@ -1225,6 +1225,71 @@ func TestCollectionUpdate(t *testing.T) {
 				"OnModelValidate":              1,
 			},
 		},
+		{
+			Name:   "add another OAuth2 provider to an auth collection",
+			Method: http.MethodPatch,
+			URL:    "/api/collections/users",
+			Body: strings.NewReader(`{
+				"oauth2": {
+					"providers": [
+						{"name": "apple", "clientId": "a", "clientSecret": "b"},
+						{
+							"pkce": null,
+							"name": "google",
+							"authURL": "",
+							"displayName": "existing",
+							"extra": {}
+						}
+					]
+				}
+			}`),
+			Headers: map[string]string{
+				"Authorization": "eyJhbGciOiJIUzI1NiJ9.eyJpZCI6InN5d2JoZWNuaDQ2cmhtMCIsInR5cGUiOiJhdXRoIiwiY29sbGVjdGlvbklkIjoicGJjXzMxNDI2MzU4MjMiLCJleHAiOjI1MjQ2MDQ0NjEsInJlZnJlc2hhYmxlIjp0cnVlfQ.UXgO3j-0BumcugrFjbd7j0M4MQvbrLggLlcu_YNGjoY",
+			},
+			BeforeTestFunc: func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) {
+				// verify that the collection has google and gitlab as OAuth2 providers
+				users, err := app.FindCollectionByNameOrId("users")
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				if v := len(users.OAuth2.Providers); v != 2 {
+					t.Fatalf("Expected 2 OAuth2 providers, got %d", v)
+				}
+
+				if v := users.OAuth2.Providers[0].Name; v != "gitlab" {
+					t.Fatalf("Expected provider 0 to be %s, got %s", "gitlab", v)
+				}
+
+				if v := users.OAuth2.Providers[1].Name; v != "google" {
+					t.Fatalf("Expected provider 1 to be %s, got %s", "google", v)
+				}
+			},
+			ExpectedStatus: 200,
+			ExpectedContent: []string{
+				`"name":"google"`,
+				`"name":"apple"`,
+				`"displayName":"existing"`,
+				`"clientId":"test"`,
+				`"clientId":"a"`,
+			},
+			NotExpectedContent: []string{
+				`"name":"gitlab"`,
+				`clientSecret`,
+			},
+			ExpectedEvents: map[string]int{
+				"*":                              0,
+				"OnCollectionUpdateRequest":      1,
+				"OnCollectionUpdate":             1,
+				"OnCollectionUpdateExecute":      1,
+				"OnCollectionAfterUpdateSuccess": 1,
+				"OnCollectionValidate":           1,
+				"OnModelUpdate":                  1,
+				"OnModelUpdateExecute":           1,
+				"OnModelAfterUpdateSuccess":      1,
+				"OnModelValidate":                1,
+			},
+		},
 
 		// view
 		// -----------------------------------------------------------

core/collection_model_auth_options.go

@@ -1,6 +1,9 @@
 package core
 
 import (
+	"encoding/json/v2"
+	"errors"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -409,6 +412,71 @@ type OAuth2Config struct {
 	Enabled bool `form:"enabled" json:"enabled"`
 }
 
+// UnmarshalJSON implements the [json.Unmarshaler] interface.
+//
+// The main difference from the standard unmarshalization is that
+// instead of replacing the entire providers config slice, we ensure
+// that partially submitted provider data (e.g. without clientSecret)
+// is merged on per config level based on the provider name
+// (https://github.com/pocketbase/pocketbase/issues/7815).
+func (c *OAuth2Config) UnmarshalJSON(b []byte) error {
+	originalProviders := slices.Clone(c.Providers)
+
+	type alias OAuth2Config
+	err := json.Unmarshal(b, (*alias)(c))
+	if err != nil {
+		return err
+	}
+
+	if len(c.Providers) == 0 {
+		return nil
+	}
+
+	// unmarshal again but this time into a plain array of objects
+	// so that we have only the submitted fields and no zero defaults
+	plain := struct {
+		Providers []map[string]any `json:"providers"`
+	}{}
+	err = json.Unmarshal(b, &plain)
+	if err != nil {
+		return err
+	}
+
+	// no providers were submitted
+	if len(plain.Providers) == 0 {
+		return nil
+	}
+
+	if len(c.Providers) != len(plain.Providers) {
+		return errors.New("the length of the plain unmarshalized providers and the ones from the config doesn't match")
+	}
+
+ProvidersMergeLoop:
+	for i, plain := range plain.Providers {
+		for _, original := range originalProviders {
+			if original.Name == plain["name"] {
+				raw, err := json.Marshal(plain)
+				if err != nil {
+					return err
+				}
+
+				// unmarshal the new plain data on top of the original one
+				err = json.Unmarshal(raw, &original)
+				if err != nil {
+					return err
+				}
+
+				// reassigne to the updated original
+				c.Providers[i] = original
+
+				continue ProvidersMergeLoop
+			}
+		}
+	}
+
+	return nil
+}
+
 // GetProviderConfig returns the first OAuth2ProviderConfig that matches the specified name.
 //
 // Returns false and zero config if no such provider is available in c.Providers.

core/collection_model_auth_options_test.go

@@ -712,6 +712,76 @@ func TestPasswordAuthConfigValidate(t *testing.T) {
 	}
 }
 
+func TestOAuth2ConfigUnmarshalJSON(t *testing.T) {
+	t.Parallel()
+
+	scenarios := []struct {
+		name     string
+		newJSON  string
+		expected string
+	}{
+		{
+			"missing",
+			`{
+				"enabled": true,
+				"mappedFields": {"username": "username_test"}
+			}`,
+			`{"providers":[{"pkce":null,"name":"a","clientId":"a_clientId","clientSecret":"a_clientSecret","authURL":"","tokenURL":"","userInfoURL":"","displayName":"","extra":{}},{"pkce":null,"name":"b","clientId":"b_clientId","clientSecret":"b_clientSecret","authURL":"","tokenURL":"","userInfoURL":"","displayName":"","extra":{}}],"mappedFields":{"id":"","name":"name_test","username":"username_test","avatarURL":""},"enabled":true}`,
+		},
+		{
+			"empty",
+			`{
+				"enabled": true,
+				"mappedFields": {"username": "username_test"},
+				"providers": []
+			}`,
+			`{"providers":[],"mappedFields":{"id":"","name":"name_test","username":"username_test","avatarURL":""},"enabled":true}`,
+		},
+		{
+			"non-empty",
+			`{
+				"enabled": true,
+				"mappedFields": {"username": "username_test"},
+				"providers": [
+					{"name": "c", "clientId": "c_clientId", "clientSecret": "c_clientSecret"},
+					{"name": "a", "displayName": "a_displayName"}
+				]
+			}`,
+			`{"providers":[{"pkce":null,"name":"c","clientId":"c_clientId","clientSecret":"c_clientSecret","authURL":"","tokenURL":"","userInfoURL":"","displayName":"","extra":{}},{"pkce":null,"name":"a","clientId":"a_clientId","clientSecret":"a_clientSecret","authURL":"","tokenURL":"","userInfoURL":"","displayName":"a_displayName","extra":{}}],"mappedFields":{"id":"","name":"name_test","username":"username_test","avatarURL":""},"enabled":true}`,
+		},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.name, func(t *testing.T) {
+			config := core.OAuth2Config{
+				Enabled: false,
+				MappedFields: core.OAuth2KnownFields{
+					Name: "name_test",
+				},
+				Providers: []core.OAuth2ProviderConfig{
+					{Name: "a", ClientId: "a_clientId", ClientSecret: "a_clientSecret"},
+					{Name: "b", ClientId: "b_clientId", ClientSecret: "b_clientSecret"},
+				},
+			}
+
+			err := json.Unmarshal([]byte(s.newJSON), &config)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			raw, err := json.Marshal(config, json.Deterministic(true))
+			if err != nil {
+				t.Fatal(err)
+			}
+			rawStr := string(raw)
+
+			if rawStr != s.expected {
+				t.Fatalf("Expected OAuth2ProviderConfig\n%s\ngot\n%s", s.expected, rawStr)
+			}
+		})
+	}
+}
+
 func TestOAuth2ConfigGetProviderConfig(t *testing.T) {
 	scenarios := []struct {
 		name           string

core/log_model.go

@@ -50,6 +50,8 @@ func (l *Log) DBExport(app App) (map[string]any, error) {
 		result["message"] = l.Message
 	}
 
+	// @todo once added in the standard library consider replacing with
+	// WithByteLimit and WithDepthLimit as suggested in https://github.com/golang/go/issues/56733
 	if len(l.Data) == 0 {
 		result["data"] = l.Data
 	} else {

core/record_model.go

@@ -3,6 +3,7 @@ package core
 import (
 	"bytes"
 	"context"
+	"encoding/json/jsontext"
 	"encoding/json/v2"
 	"errors"
 	"fmt"
@@ -1329,7 +1330,12 @@ func (record *Record) PublicExport() map[string]any {
 //
 // Only the data exported by `PublicExport()` will be serialized.
 func (m Record) MarshalJSON() ([]byte, error) {
-	return json.Marshal(m.PublicExport(), json.Deterministic(true))
+	return json.Marshal(
+		m.PublicExport(),
+		json.Deterministic(true),
+		// to preserve the old jsonv1 behavior in case of invalid data
+		jsontext.AllowInvalidUTF8(true),
+	)
 }
 
 // UnmarshalJSON implements the [json.Unmarshaler] interface.

core/record_model_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"database/sql"
+	"encoding/json/jsontext"
 	"encoding/json/v2"
 	"errors"
 	"fmt"
@@ -1438,7 +1439,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) {
 			false,
 			nil,
 			nil,
-			`{"collectionId":"_pbc_base_123","collectionName":"test_base","expand":{"test":123},"field1":"field_1","field2":"field_2.png","field3":["test1","test2"],"id":"test_id"}`,
+			`{"collectionId":"_pbc_base_123","collectionName":"test_base","expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"id":"test_id"}`,
 		},
 		{
 			"[base] with email visibility",
@@ -1447,7 +1448,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) {
 			false,
 			nil,
 			nil,
-			`{"collectionId":"_pbc_base_123","collectionName":"test_base","expand":{"test":123},"field1":"field_1","field2":"field_2.png","field3":["test1","test2"],"id":"test_id"}`,
+			`{"collectionId":"_pbc_base_123","collectionName":"test_base","expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"id":"test_id"}`,
 		},
 		{
 			"[base] with custom data",
@@ -1456,7 +1457,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) {
 			true,
 			nil,
 			nil,
-			`{"collectionId":"_pbc_base_123","collectionName":"test_base","email":"test_email","emailVisibility":"test_invalid","expand":{"test":123},"field1":"field_1","field2":"field_2.png","field3":["test1","test2"],"id":"test_id","password":"test_passwordHash","tokenKey":"test_tokenKey","unknown":"test_unknown","verified":true}`,
+			`{"collectionId":"_pbc_base_123","collectionName":"test_base","email":"test_email","emailVisibility":"test_invalid","expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"id":"test_id","password":"test_passwordHash","tokenKey":"test_tokenKey","unknown":"test_unknown","verified":true}`,
 		},
 		{
 			"[base] with explicit hide and unhide fields",
@@ -1474,7 +1475,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) {
 			true,
 			nil,
 			[]string{"field5", "@pbInternalAbc", "email", "tokenKey", "unknown"},
-			`{"collectionId":"_pbc_base_123","collectionName":"test_base","email":"test_email","emailVisibility":"test_invalid","expand":{"test":123},"field1":"field_1","field2":"field_2.png","field3":["test1","test2"],"field5":"field_5","id":"test_id","password":"test_passwordHash","tokenKey":"test_tokenKey","unknown":"test_unknown","verified":true}`,
+			`{"collectionId":"_pbc_base_123","collectionName":"test_base","email":"test_email","emailVisibility":"test_invalid","expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"field5":"field_5","id":"test_id","password":"test_passwordHash","tokenKey":"test_tokenKey","unknown":"test_unknown","verified":true}`,
 		},
 
 		// auth
@@ -1485,7 +1486,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) {
 			false,
 			nil,
 			nil,
-			`{"collectionId":"_pbc_auth_123","collectionName":"test_auth","emailVisibility":false,"expand":{"test":123},"field1":"field_1","field2":"field_2.png","field3":["test1","test2"],"id":"test_id","verified":true}`,
+			`{"collectionId":"_pbc_auth_123","collectionName":"test_auth","emailVisibility":false,"expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"id":"test_id","verified":true}`,
 		},
 		{
 			"[auth] with email visibility",
@@ -1494,7 +1495,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) {
 			false,
 			nil,
 			nil,
-			`{"collectionId":"_pbc_auth_123","collectionName":"test_auth","email":"test_email","emailVisibility":false,"expand":{"test":123},"field1":"field_1","field2":"field_2.png","field3":["test1","test2"],"id":"test_id","verified":true}`,
+			`{"collectionId":"_pbc_auth_123","collectionName":"test_auth","email":"test_email","emailVisibility":false,"expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"id":"test_id","verified":true}`,
 		},
 		{
 			"[auth] with custom data",
@@ -1503,7 +1504,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) {
 			true,
 			nil,
 			nil,
-			`{"collectionId":"_pbc_auth_123","collectionName":"test_auth","emailVisibility":false,"expand":{"test":123},"field1":"field_1","field2":"field_2.png","field3":["test1","test2"],"id":"test_id","unknown":"test_unknown","verified":true}`,
+			`{"collectionId":"_pbc_auth_123","collectionName":"test_auth","emailVisibility":false,"expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"id":"test_id","unknown":"test_unknown","verified":true}`,
 		},
 		{
 			"[auth] with explicit hide and unhide fields",
@@ -1521,13 +1522,13 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) {
 			true,
 			nil,
 			[]string{"field5", "@pbInternalAbc", "tokenKey", "unknown", "email"}, // emailVisibility:false has higher priority
-			`{"collectionId":"_pbc_auth_123","collectionName":"test_auth","emailVisibility":false,"expand":{"test":123},"field1":"field_1","field2":"field_2.png","field3":["test1","test2"],"field5":"field_5","id":"test_id","unknown":"test_unknown","verified":true}`,
+			`{"collectionId":"_pbc_auth_123","collectionName":"test_auth","emailVisibility":false,"expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"field5":"field_5","id":"test_id","unknown":"test_unknown","verified":true}`,
 		},
 	}
 
 	data := map[string]any{
 		"id":              "test_id",
-		"field1":          "field_1",
+		"field1":          "field_1\xc3", /* invalid utf8 suffix to test mangling */
 		"field2":          "field_2.png",
 		"field3":          []string{"test1", "test2"},
 		"field4":          "field_4",
@@ -1554,7 +1555,11 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) {
 			m.Unhide(s.unhideFields...)
 			m.Hide(s.hideFields...)
 
-			exportResult, err := json.Marshal(m.PublicExport(), json.Deterministic(true))
+			exportResult, err := json.Marshal(
+				m.PublicExport(),
+				json.Deterministic(true),
+				jsontext.AllowInvalidUTF8(true),
+			)
 			if err != nil {
 				t.Fatal(err)
 			}