From 9520117914d5e831e3f5e57b6adb493f72d9ef59 Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Fri, 4 Sep 2026 17:24:55 +0800 Subject: [PATCH] =?UTF-8?q?feat=E2=9C=A8:=20normalize=20invalid=20data=20s?= =?UTF-8?q?copes=20on=20existing=20installs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The seed fix only reaches new installations. An install that imported the old db.sql already has an administrator with an empty data_scope, and after the fail-closed change that account sees nothing. The migration rewrites any value outside "1".."5" to "1", which is the behaviour those rows had before. Plain SQL rather than the frozen migration models, per the rule that migrations after 1786700003000 must not use them. Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx --- ...1786700005000_normalize_role_data_scope.go | 49 +++++++ ...00005000_normalize_role_data_scope_test.go | 122 ++++++++++++++++++ 2 files changed, 171 insertions(+) create mode 100644 cmd/migrate/migration/version/1786700005000_normalize_role_data_scope.go create mode 100644 cmd/migrate/migration/version/1786700005000_normalize_role_data_scope_test.go diff --git a/cmd/migrate/migration/version/1786700005000_normalize_role_data_scope.go b/cmd/migrate/migration/version/1786700005000_normalize_role_data_scope.go new file mode 100644 index 00000000..ec7dbd3a --- /dev/null +++ b/cmd/migrate/migration/version/1786700005000_normalize_role_data_scope.go @@ -0,0 +1,49 @@ +package version + +import ( + "runtime" + + "gorm.io/gorm" + + "go-admin/cmd/migrate/migration" + common "go-admin/common/models" +) + +// Normalize sys_role.data_scope to one of the five values +// actions.Permission recognizes, ahead of PRD 006 F14/H2 making its +// unrecognized-scope branch fail closed instead of fail open. +// +// Before that change, an empty or unrecognized data_scope fell into +// Permission's default branch, which returned the query untouched - exactly +// the same SQL as data_scope "1" (全部数据权限). The seed data shipped +// precisely that: config/db.sql's built-in admin role (role_id 1) carries an +// empty data_scope rather than "1". Once the default starts matching no +// rows instead, that role would silently lose all visibility everywhere +// actions.Permission is used, the moment a deployment turns EnableDP on. +// +// Rewriting every value outside {1,2,3,4,5} to "1" keeps each such role's +// effective visibility exactly what it already was - a role that intended a +// tighter scope was never getting it under the old fail-open default either, +// so this does not tighten anything a deployment was relying on. Whether to +// tighten it further is left to whoever owns that role. +func init() { + _, fileName, _, _ := runtime.Caller(0) + migration.Migrate.SetVersion(migration.GetFilename(fileName), _1786700005000NormalizeRoleDataScope) +} + +func _1786700005000NormalizeRoleDataScope(db *gorm.DB, version string) error { + return db.Transaction(func(tx *gorm.DB) error { + if err := normalizeRoleDataScope(tx); err != nil { + return err + } + return tx.Create(&common.Migration{Version: version}).Error + }) +} + +// normalizeRoleDataScope is split out so tests can run it against a database +// that only has sys_role, without also standing up sys_migration. +func normalizeRoleDataScope(tx *gorm.DB) error { + return tx.Exec( + "UPDATE sys_role SET data_scope = '1' WHERE data_scope NOT IN ('1', '2', '3', '4', '5')", + ).Error +} diff --git a/cmd/migrate/migration/version/1786700005000_normalize_role_data_scope_test.go b/cmd/migrate/migration/version/1786700005000_normalize_role_data_scope_test.go new file mode 100644 index 00000000..c85ea2c8 --- /dev/null +++ b/cmd/migrate/migration/version/1786700005000_normalize_role_data_scope_test.go @@ -0,0 +1,122 @@ +package version + +import ( + "testing" + + "github.com/glebarez/sqlite" + "gorm.io/gorm" +) + +type roleDataScopeRow struct { + RoleId int `gorm:"column:role_id;primaryKey;autoIncrement"` + DataScope string `gorm:"column:data_scope"` +} + +func (roleDataScopeRow) TableName() string { return "sys_role" } + +func openRoleTable(t *testing.T) *gorm.DB { + t.Helper() + + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + if err != nil { + t.Fatalf("open: %v", err) + } + if err := db.AutoMigrate(&roleDataScopeRow{}); err != nil { + t.Fatalf("migrate: %v", err) + } + return db +} + +// The migration exists because the shipped admin role is exactly this case: +// config/db.sql's role_id 1 carries an empty data_scope. Reproduces the seed +// data literally rather than a made-up example. +func TestNormalizesTheEmptyDataScopeTheSeedDataShips(t *testing.T) { + db := openRoleTable(t) + if err := db.Create(&roleDataScopeRow{RoleId: 1, DataScope: ""}).Error; err != nil { + t.Fatalf("seed: %v", err) + } + + if err := normalizeRoleDataScope(db); err != nil { + t.Fatalf("migrate: %v", err) + } + + var row roleDataScopeRow + if err := db.First(&row, 1).Error; err != nil { + t.Fatalf("read back: %v", err) + } + if row.DataScope != "1" { + t.Fatalf("data_scope = %q, want %q", row.DataScope, "1") + } +} + +// The five recognized values must survive untouched - this migration +// normalizes what Permission cannot make sense of, not what it already can. +func TestLeavesRecognizedScopesAlone(t *testing.T) { + db := openRoleTable(t) + valid := []string{"1", "2", "3", "4", "5"} + for i, scope := range valid { + if err := db.Create(&roleDataScopeRow{RoleId: i + 1, DataScope: scope}).Error; err != nil { + t.Fatalf("seed %d: %v", i, err) + } + } + + if err := normalizeRoleDataScope(db); err != nil { + t.Fatalf("migrate: %v", err) + } + + var rows []roleDataScopeRow + if err := db.Order("role_id").Find(&rows).Error; err != nil { + t.Fatalf("read back: %v", err) + } + for i, row := range rows { + if row.DataScope != valid[i] { + t.Errorf("role %d: data_scope = %q, want %q (untouched)", row.RoleId, row.DataScope, valid[i]) + } + } +} + +// A garbage value (not just empty) must be normalized the same way as empty - +// both are "not one of the five", and the migration's WHERE clause has to +// catch both. +func TestNormalizesGarbageScopesToo(t *testing.T) { + db := openRoleTable(t) + if err := db.Create(&roleDataScopeRow{RoleId: 1, DataScope: "6"}).Error; err != nil { + t.Fatalf("seed: %v", err) + } + + if err := normalizeRoleDataScope(db); err != nil { + t.Fatalf("migrate: %v", err) + } + + var row roleDataScopeRow + if err := db.First(&row, 1).Error; err != nil { + t.Fatalf("read back: %v", err) + } + if row.DataScope != "1" { + t.Fatalf("data_scope = %q, want %q", row.DataScope, "1") + } +} + +// Running it twice must be safe: it is a plain UPDATE, not DDL, but +// sys_migration only records success once, and an operator who reruns +// `migrate` on a partially-applied database has to be able to trust that. +func TestNormalizeRoleDataScopeIsRepeatable(t *testing.T) { + db := openRoleTable(t) + if err := db.Create(&roleDataScopeRow{RoleId: 1, DataScope: ""}).Error; err != nil { + t.Fatalf("seed: %v", err) + } + + for i := 0; i < 3; i++ { + if err := normalizeRoleDataScope(db); err != nil { + t.Fatalf("migrate %d: %v", i, err) + } + } + + var row roleDataScopeRow + if err := db.First(&row, 1).Error; err != nil { + t.Fatalf("read back: %v", err) + } + if row.DataScope != "1" { + t.Fatalf("data_scope = %q, want %q", row.DataScope, "1") + } +}