diff --git a/docs/backend/users-and-permissions.md b/docs/backend/users-and-permissions.md index 801a08b..a2f196c 100644 --- a/docs/backend/users-and-permissions.md +++ b/docs/backend/users-and-permissions.md @@ -6,7 +6,7 @@ order: 50 --- # Users and permissions -The admin keeps WinterCMS's backend user model: the `backend_users` and `backend_user_roles` tables, roles with permission grants, and superusers who pass every check. [cabana](../../modules/cabana/README.md) signs administrators in and checks their permissions; the tables are created by the framework migrations that `migrate` runs. +The admin keeps WinterCMS's backend user model: the `backend_users` and `backend_user_roles` tables, roles with permission grants, and superusers who pass every check. [cabana](../../modules/cabana/README.md) signs administrators in and checks their permissions; the tables are created by the framework migrations that `migrate` runs. Administrator emails are unique regardless of case. If a table copied from WinterCMS holds two emails that differ only in case, `migrate` stops and names their logins; change or remove one of them, then run `migrate` again. ## Signing in diff --git a/modules/lagoon/README.md b/modules/lagoon/README.md index d83acb1..0c7db02 100644 --- a/modules/lagoon/README.md +++ b/modules/lagoon/README.md @@ -121,7 +121,7 @@ func (p *Plugin) Migrations() []*gormigrate.Migration { | `lagoon.RollbackLast` | Rolls back the last migration of one plugin. | | `lagoon.Status` | Lists applied migration IDs per plugin as `lagoon.StatusRow` values. | | `lagoon.HistoryTableName` | Returns the gormigrate history table for a plugin ID. | -| `lagoon.BackendAdminMigrations` | Creates the backend user, role and admin token blacklist tables and seeds the system roles. | +| `lagoon.BackendAdminMigrations` | Creates the backend user, role and admin token blacklist tables, seeds the system roles and adds a case-insensitive unique index on `backend_users.email`. The index migration refuses to run while emails differ only in case and names the clashing logins. | | `lagoon.QueueMigrations` | Migrates River's schema to `lagoon.RiverSchemaVersion` and creates the `summer_jobs` job record table. | | `lagoon.QueueHistoryID` | History id of the job-queue set, `summercms.conga`. | | `lagoon.JobsTable` | Name of the job record table, `summer_jobs`. | diff --git a/modules/lagoon/backend_admin_migrations.go b/modules/lagoon/backend_admin_migrations.go index 75ba5cc..229cf45 100644 --- a/modules/lagoon/backend_admin_migrations.go +++ b/modules/lagoon/backend_admin_migrations.go @@ -1,6 +1,9 @@ package lagoon import ( + "fmt" + "strings" + "github.com/go-gormigrate/gormigrate/v2" "gorm.io/gorm" ) @@ -9,10 +12,11 @@ import ( // It is distinct from the frontend jwt_blacklist table. const BackendJWTBlacklistTable = "backend_jwt_blacklist" -// BackendAdminMigrations creates Winter-shaped backend identity tables and -// seeds the developer and publisher system roles. History is isolated under -// the summercms.cabana plugin id. DDL is re-runnable so the system-role seed -// stays idempotent if the history row is removed. +// BackendAdminMigrations creates Winter-shaped backend identity tables, seeds +// the developer and publisher system roles and makes backend user emails +// unique case-insensitively. History is isolated under the summercms.cabana +// plugin id. DDL is re-runnable so the system-role seed stays idempotent if +// the history row is removed. var BackendAdminMigrations = []*gormigrate.Migration{ { ID: "202609240001_backend_admin_identity", @@ -85,4 +89,32 @@ ON CONFLICT (name) DO NOTHING`, return nil }, }, + { + // Emails are matched case-insensitively at login and by admin:create, + // so the table enforces the same rule. Rows copied from Winter may + // hold two emails that differ only in case; the migration refuses to + // run and names them instead of picking an account to drop. + ID: "202610010001_backend_users_email_ci_unique", + Migrate: func(tx *gorm.DB) error { + var dupes []struct { + Email string + Logins string + } + if err := tx.Raw(`SELECT lower(email) AS email, string_agg(login, ', ' ORDER BY id) AS logins +FROM backend_users GROUP BY lower(email) HAVING count(*) > 1 ORDER BY 1`).Scan(&dupes).Error; err != nil { + return err + } + if len(dupes) > 0 { + parts := make([]string, len(dupes)) + for i, d := range dupes { + parts[i] = fmt.Sprintf("%s (logins: %s)", d.Email, d.Logins) + } + return fmt.Errorf("lagoon: backend_users has emails that differ only in case: %s; change or remove the duplicates, then run migrate again", strings.Join(parts, "; ")) + } + return tx.Exec(`CREATE UNIQUE INDEX IF NOT EXISTS backend_users_email_lower_unique ON backend_users (lower(email))`).Error + }, + Rollback: func(tx *gorm.DB) error { + return tx.Exec(`DROP INDEX IF EXISTS backend_users_email_lower_unique`).Error + }, + }, } diff --git a/modules/lagoon/backend_admin_migrations_test.go b/modules/lagoon/backend_admin_migrations_test.go index fef4c72..c460141 100644 --- a/modules/lagoon/backend_admin_migrations_test.go +++ b/modules/lagoon/backend_admin_migrations_test.go @@ -51,9 +51,7 @@ func TestPhase09MigrationsFreshRollback(t *testing.T) { if err != nil { t.Fatal(err) } - if err := admin.RollbackLast(); err != nil { - t.Fatal(err) - } + rollbackBackendAdmin(t, admin) for _, name := range []string{"backend_users", "backend_user_roles", "backend_jwt_blacklist"} { if gdb.Migrator().HasTable(name) { t.Fatalf("%s survived admin rollback", name) @@ -205,9 +203,7 @@ func TestBackendAdminRollback(t *testing.T) { if err != nil { t.Fatal(err) } - if err := m.RollbackLast(); err != nil { - t.Fatal(err) - } + rollbackBackendAdmin(t, m) for _, name := range []string{"backend_users", "backend_user_roles", "backend_jwt_blacklist"} { if gdb.Migrator().HasTable(name) { t.Fatalf("%s survived admin rollback", name) @@ -443,3 +439,71 @@ func assertNoAutoMigrate(t *testing.T) { } } } + +// rollbackBackendAdmin rolls back every BackendAdminMigrations entry, newest +// first, so the identity tables are dropped as a set. +func rollbackBackendAdmin(t *testing.T, m *gormigrate.Gormigrate) { + t.Helper() + for range BackendAdminMigrations { + if err := m.RollbackLast(); err != nil { + t.Fatal(err) + } + } +} + +func TestBackendAdminEmailCaseInsensitiveUnique(t *testing.T) { + db, _ := dedicatedDB(t, "lagoon_admin_email_ci") + gdb, err := Use(t.Context(), db) + if err != nil { + t.Fatal(err) + } + if err := Migrate(gdb, nil); err != nil { + t.Fatal(err) + } + if err := gdb.Exec(`INSERT INTO backend_users (login, email, password) VALUES ('ada', 'Ada@Example.Test', 'x')`).Error; err != nil { + t.Fatal(err) + } + if err := gdb.Exec(`INSERT INTO backend_users (login, email, password) VALUES ('ada-2', 'ada@example.test', 'x')`).Error; err == nil { + t.Fatal("an email differing only in case was accepted") + } +} + +func TestBackendAdminEmailIndexRefusesCaseDuplicates(t *testing.T) { + db, _ := dedicatedDB(t, "lagoon_admin_email_dupes") + gdb, err := Use(t.Context(), db) + if err != nil { + t.Fatal(err) + } + if err := Migrate(gdb, nil); err != nil { + t.Fatal(err) + } + m, err := migrator(gdb, "summercms.cabana", BackendAdminMigrations) + if err != nil { + t.Fatal(err) + } + if err := m.RollbackLast(); err != nil { + t.Fatal(err) + } + // A Winter copy can hold emails that differ only in case. + for _, row := range [][2]string{{"ada", "Ada@Example.Test"}, {"ada-old", "ada@example.test"}} { + if err := gdb.Exec(`INSERT INTO backend_users (login, email, password) VALUES (?, ?, 'x')`, row[0], row[1]).Error; err != nil { + t.Fatal(err) + } + } + err = Migrate(gdb, nil) + if err == nil { + t.Fatal("migrate created the unique email index over case duplicates") + } + if !strings.Contains(err.Error(), "ada@example.test (logins: ada, ada-old)") { + t.Fatalf("error does not name the duplicates: %v", err) + } + if err := gdb.Exec(`UPDATE backend_users SET email = 'ada.old@example.test' WHERE login = 'ada-old'`).Error; err != nil { + t.Fatal(err) + } + if err := Migrate(gdb, nil); err != nil { + t.Fatalf("migrate after resolving duplicates: %v", err) + } + if !indexOn(indexDefs(t, gdb, "backend_users"), "lower(email)") { + t.Fatal("unique lower(email) index missing") + } +}