diff --git a/pkg/migration/20260627101958.go b/pkg/migration/20260627101958.go new file mode 100644 index 000000000..a34527e66 --- /dev/null +++ b/pkg/migration/20260627101958.go @@ -0,0 +1,125 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package migration + +import ( + "bytes" + "encoding/base64" + "encoding/json" + "strings" + + "src.techknowlogick.com/xormigrate" + "xorm.io/xorm" + "xorm.io/xorm/schemas" +) + +// usersFrontendSettings20260627101958 reads the raw frontend_settings JSON text. +// The json tag makes xorm hand back the stored column text verbatim instead of +// decoding it into a typed value. +type usersFrontendSettings20260627101958 struct { + ID int64 `xorm:"bigint autoincr not null unique pk"` + FrontendSettings string `xorm:"frontend_settings json null"` +} + +func (usersFrontendSettings20260627101958) TableName() string { + return "users" +} + +// legacyFrontendSettingsWhereClause20260627101958 selects rows whose +// frontend_settings is a JSON string ('"…"'). Healthy values are JSON objects +// ('{…}'); only the legacy double-encoded values are stored as JSON strings. +func legacyFrontendSettingsWhereClause20260627101958(dbType schemas.DBType) string { + if dbType == schemas.POSTGRES { + return `frontend_settings IS NOT NULL AND frontend_settings::text LIKE '"%'` + } + return `frontend_settings IS NOT NULL AND frontend_settings LIKE '"%'` +} + +// repairLegacyFrontendSettings20260627101958 reverses the historical double +// encoding of frontend_settings. The pre-fix UpdateUser stored +// json.Marshal(FrontendSettings) back into the interface field, so xorm +// base64-encoded the resulting []byte: a nil value became the JSON string +// "bnVsbA==" (base64 of "null") and a real settings object became the base64 of +// its JSON. +// +// changed reports whether a repair applies; setNull reports the value should +// become SQL NULL; value carries the repaired JSON object otherwise. Only values +// that base64-decode to JSON null or a JSON object are touched, so a legitimate +// string-valued setting is never rewritten. +func repairLegacyFrontendSettings20260627101958(raw string) (value string, setNull, changed bool) { + trimmed := strings.TrimSpace(raw) + if len(trimmed) == 0 || trimmed[0] != '"' { + return "", false, false + } + + var inner string + if err := json.Unmarshal([]byte(trimmed), &inner); err != nil { + return "", false, false + } + + decoded, err := base64.StdEncoding.DecodeString(inner) + if err != nil { + return "", false, false + } + + decoded = bytes.TrimSpace(decoded) + if bytes.Equal(decoded, []byte("null")) { + return "", true, true + } + if len(decoded) > 0 && decoded[0] == '{' && json.Valid(decoded) { + return string(decoded), false, true + } + return "", false, false +} + +func init() { + migrations = append(migrations, &xormigrate.Migration{ + ID: "20260627101958", + Description: "repair double-encoded frontend_settings stored before UpdateUser stopped re-marshalling the column", + Migrate: func(tx *xorm.Engine) error { + users := []*usersFrontendSettings20260627101958{} + if err := tx. + Where(legacyFrontendSettingsWhereClause20260627101958(tx.Dialect().URI().DBType)). + Find(&users); err != nil { + return err + } + + for _, u := range users { + value, setNull, changed := repairLegacyFrontendSettings20260627101958(u.FrontendSettings) + if !changed { + continue + } + + if setNull { + if _, err := tx.Exec("UPDATE users SET frontend_settings = NULL WHERE id = ?", u.ID); err != nil { + return err + } + continue + } + + if _, err := tx.Exec("UPDATE users SET frontend_settings = ? WHERE id = ?", value, u.ID); err != nil { + return err + } + } + + return nil + }, + Rollback: func(tx *xorm.Engine) error { + return nil + }, + }) +} diff --git a/pkg/migration/20260627101958_test.go b/pkg/migration/20260627101958_test.go new file mode 100644 index 000000000..b883c4e00 --- /dev/null +++ b/pkg/migration/20260627101958_test.go @@ -0,0 +1,103 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package migration + +import ( + "encoding/base64" + "testing" + + "xorm.io/xorm/schemas" +) + +func legacyFrontendSettingsRaw20260627101958(jsonValue string) string { + return `"` + base64.StdEncoding.EncodeToString([]byte(jsonValue)) + `"` +} + +func TestRepairLegacyFrontendSettings20260627101958(t *testing.T) { + tests := []struct { + name string + raw string + wantValue string + wantNull bool + wantChange bool + }{ + { + name: "legacy null heals to NULL", + raw: legacyFrontendSettingsRaw20260627101958("null"), + wantNull: true, + wantChange: true, + }, + { + name: "legacy object is decoded back to the object", + raw: legacyFrontendSettingsRaw20260627101958(`{"color_schema":"dark"}`), + wantValue: `{"color_schema":"dark"}`, + wantChange: true, + }, + { + name: "healthy object is left untouched", + raw: `{"color_schema":"dark"}`, + wantChange: false, + }, + { + name: "non-base64 string is left untouched", + raw: `"hello world"`, + wantChange: false, + }, + { + name: "base64 of a scalar is left untouched", + raw: legacyFrontendSettingsRaw20260627101958("123"), + wantChange: false, + }, + { + name: "base64 of an array is left untouched", + raw: legacyFrontendSettingsRaw20260627101958("[]"), + wantChange: false, + }, + { + name: "empty value is left untouched", + raw: "", + wantChange: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + value, setNull, changed := repairLegacyFrontendSettings20260627101958(tt.raw) + if changed != tt.wantChange { + t.Fatalf("changed = %v, want %v", changed, tt.wantChange) + } + if setNull != tt.wantNull { + t.Fatalf("setNull = %v, want %v", setNull, tt.wantNull) + } + if value != tt.wantValue { + t.Fatalf("value = %q, want %q", value, tt.wantValue) + } + }) + } +} + +func TestLegacyFrontendSettingsWhereClause20260627101958(t *testing.T) { + postgres := legacyFrontendSettingsWhereClause20260627101958(schemas.POSTGRES) + if want := `frontend_settings IS NOT NULL AND frontend_settings::text LIKE '"%'`; postgres != want { + t.Fatalf("postgres clause\nwant: %s\ngot: %s", want, postgres) + } + + other := legacyFrontendSettingsWhereClause20260627101958(schemas.SQLITE) + if want := `frontend_settings IS NOT NULL AND frontend_settings LIKE '"%'`; other != want { + t.Fatalf("default clause\nwant: %s\ngot: %s", want, other) + } +} diff --git a/pkg/user/user.go b/pkg/user/user.go index ec165f746..c5283892a 100644 --- a/pkg/user/user.go +++ b/pkg/user/user.go @@ -18,6 +18,7 @@ package user import ( "context" + "encoding/json" "errors" "fmt" "net/mail" @@ -632,28 +633,15 @@ func UpdateUser(s *xorm.Session, user *User, forceOverride bool) (updatedUser *U return nil, &ErrInvalidTimezone{Name: user.Timezone, LoadError: err} } + cols, err := userUpdateColumns(user, forceOverride) + if err != nil { + return nil, err + } + // Update it _, err = s. ID(user.ID). - Cols( - "username", - "email", - "avatar_provider", - "avatar_file_id", - "status", - "name", - "email_reminders_enabled", - "discoverable_by_name", - "discoverable_by_email", - "overdue_tasks_reminders_enabled", - "default_project_id", - "week_start", - "language", - "timezone", - "overdue_tasks_reminders_time", - "frontend_settings", - "extra_settings_links", - ). + Cols(cols...). Update(user) if err != nil { return &User{}, err @@ -668,6 +656,52 @@ func UpdateUser(s *xorm.Session, user *User, forceOverride bool) (updatedUser *U return updatedUser, err } +// userUpdateColumns lists the columns UpdateUser persists and, for settings +// updates, normalises frontend_settings into the JSON-string form xorm stores +// correctly. +// +// frontend_settings is written only when forceOverride is set, which the +// user-settings endpoints pass and which no other caller does. Skipping the +// column elsewhere is deliberate: it is set on no other path, and listing a nil +// interface in Cols makes xorm write NULL, wiping the user's settings on every +// OIDC login. We marshal the value to a string ourselves because xorm passes a +// Go map in an interface{} json column straight to the driver (an error) and +// base64-encodes a []byte (the historical double-encoding bug). +func userUpdateColumns(user *User, forceOverride bool) ([]string, error) { + cols := []string{ + "username", + "email", + "avatar_provider", + "avatar_file_id", + "status", + "name", + "email_reminders_enabled", + "discoverable_by_name", + "discoverable_by_email", + "overdue_tasks_reminders_enabled", + "default_project_id", + "week_start", + "language", + "timezone", + "overdue_tasks_reminders_time", + "extra_settings_links", + } + + if !forceOverride { + return cols, nil + } + + if user.FrontendSettings != nil { + frontendSettingsJSON, err := json.Marshal(user.FrontendSettings) + if err != nil { + return nil, fmt.Errorf("marshal frontend settings: %w", err) + } + user.FrontendSettings = string(frontendSettingsJSON) + } + + return append(cols, "frontend_settings"), nil +} + func SetUserStatus(s *xorm.Session, user *User, status Status) (err error) { _, err = s.Where("id = ?", user.ID). Cols("status"). diff --git a/pkg/user/user_test.go b/pkg/user/user_test.go index 38287c6e0..35c5dd103 100644 --- a/pkg/user/user_test.go +++ b/pkg/user/user_test.go @@ -18,6 +18,8 @@ package user import ( "context" + "database/sql" + "encoding/json" "testing" "code.vikunja.io/api/pkg/db" @@ -475,6 +477,103 @@ func TestUpdateUser(t *testing.T) { require.Error(t, err) assert.True(t, IsErrUserDoesNotExist(err)) }) + t.Run("frontend settings survive profile-only update", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + originalSettings := map[string]any{ + "color_schema": "dark", + } + settingsJSON, err := json.Marshal(originalSettings) + require.NoError(t, err) + _, err = s.Table("users"). + Where("id = ?", 1). + Cols("frontend_settings"). + Update(&struct { + FrontendSettings string `xorm:"frontend_settings"` + }{ + FrontendSettings: string(settingsJSON), + }) + require.NoError(t, err) + + updated, err := UpdateUser(s, &User{ + ID: 1, + Email: "testing@example.com", + }, false) + require.NoError(t, err) + require.Equal(t, map[string]interface{}(originalSettings), updated.FrontendSettings) + + var stored sql.NullString + has, err := s.Table("users"). + Where("id = ?", 1). + Cols("frontend_settings"). + Get(&stored) + require.NoError(t, err) + require.True(t, has) + require.True(t, stored.Valid) + assert.JSONEq(t, string(settingsJSON), stored.String) + }) + t.Run("frontend settings can be saved from request map", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + frontendSettings := map[string]any{ + "color_schema": "dark", + "nested": map[string]any{ + "a": float64(1), + }, + } + + updated, err := UpdateUser(s, &User{ + ID: 1, + FrontendSettings: frontendSettings, + }, true) + require.NoError(t, err) + require.Equal(t, frontendSettings, updated.FrontendSettings) + + var stored sql.NullString + has, err := s.Table("users"). + Where("id = ?", 1). + Cols("frontend_settings"). + Get(&stored) + require.NoError(t, err) + require.True(t, has) + require.True(t, stored.Valid) + assert.JSONEq(t, `{"color_schema":"dark","nested":{"a":1}}`, stored.String) + }) + t.Run("frontend settings can be cleared", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + _, err := s.Table("users"). + Where("id = ?", 1). + Cols("frontend_settings"). + Update(&struct { + FrontendSettings string `xorm:"frontend_settings"` + }{ + FrontendSettings: `{"color_schema":"dark"}`, + }) + require.NoError(t, err) + + updated, err := UpdateUser(s, &User{ + ID: 1, + FrontendSettings: nil, + }, true) + require.NoError(t, err) + require.Nil(t, updated.FrontendSettings) + + var stored sql.NullString + has, err := s.Table("users"). + Where("id = ?", 1). + Cols("frontend_settings"). + Get(&stored) + require.NoError(t, err) + require.True(t, has) + assert.False(t, stored.Valid) + }) } func TestUpdateUserPassword(t *testing.T) {