fix(09): WR-11 enforce case-insensitive unique backend user emails
Add a backend admin migration that creates a unique index on lower(backend_users.email). Rows copied from WinterCMS may hold emails that differ only in case, so the migration refuses to run and names the clashing logins instead of choosing an account to drop.
This commit is contained in:
@@ -6,7 +6,7 @@ order: 50
|
|||||||
---
|
---
|
||||||
# Users and permissions
|
# 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
|
## Signing in
|
||||||
|
|
||||||
|
|||||||
@@ -121,7 +121,7 @@ func (p *Plugin) Migrations() []*gormigrate.Migration {
|
|||||||
| `lagoon.RollbackLast` | Rolls back the last migration of one plugin. |
|
| `lagoon.RollbackLast` | Rolls back the last migration of one plugin. |
|
||||||
| `lagoon.Status` | Lists applied migration IDs per plugin as `lagoon.StatusRow` values. |
|
| `lagoon.Status` | Lists applied migration IDs per plugin as `lagoon.StatusRow` values. |
|
||||||
| `lagoon.HistoryTableName` | Returns the gormigrate history table for a plugin ID. |
|
| `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.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.QueueHistoryID` | History id of the job-queue set, `summercms.conga`. |
|
||||||
| `lagoon.JobsTable` | Name of the job record table, `summer_jobs`. |
|
| `lagoon.JobsTable` | Name of the job record table, `summer_jobs`. |
|
||||||
|
|||||||
@@ -1,6 +1,9 @@
|
|||||||
package lagoon
|
package lagoon
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"fmt"
|
||||||
|
"strings"
|
||||||
|
|
||||||
"github.com/go-gormigrate/gormigrate/v2"
|
"github.com/go-gormigrate/gormigrate/v2"
|
||||||
"gorm.io/gorm"
|
"gorm.io/gorm"
|
||||||
)
|
)
|
||||||
@@ -9,10 +12,11 @@ import (
|
|||||||
// It is distinct from the frontend jwt_blacklist table.
|
// It is distinct from the frontend jwt_blacklist table.
|
||||||
const BackendJWTBlacklistTable = "backend_jwt_blacklist"
|
const BackendJWTBlacklistTable = "backend_jwt_blacklist"
|
||||||
|
|
||||||
// BackendAdminMigrations creates Winter-shaped backend identity tables and
|
// BackendAdminMigrations creates Winter-shaped backend identity tables, seeds
|
||||||
// seeds the developer and publisher system roles. History is isolated under
|
// the developer and publisher system roles and makes backend user emails
|
||||||
// the summercms.cabana plugin id. DDL is re-runnable so the system-role seed
|
// unique case-insensitively. History is isolated under the summercms.cabana
|
||||||
// stays idempotent if the history row is removed.
|
// plugin id. DDL is re-runnable so the system-role seed stays idempotent if
|
||||||
|
// the history row is removed.
|
||||||
var BackendAdminMigrations = []*gormigrate.Migration{
|
var BackendAdminMigrations = []*gormigrate.Migration{
|
||||||
{
|
{
|
||||||
ID: "202609240001_backend_admin_identity",
|
ID: "202609240001_backend_admin_identity",
|
||||||
@@ -85,4 +89,32 @@ ON CONFLICT (name) DO NOTHING`,
|
|||||||
return nil
|
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
|
||||||
|
},
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -51,9 +51,7 @@ func TestPhase09MigrationsFreshRollback(t *testing.T) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
if err := admin.RollbackLast(); err != nil {
|
rollbackBackendAdmin(t, admin)
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
for _, name := range []string{"backend_users", "backend_user_roles", "backend_jwt_blacklist"} {
|
for _, name := range []string{"backend_users", "backend_user_roles", "backend_jwt_blacklist"} {
|
||||||
if gdb.Migrator().HasTable(name) {
|
if gdb.Migrator().HasTable(name) {
|
||||||
t.Fatalf("%s survived admin rollback", name)
|
t.Fatalf("%s survived admin rollback", name)
|
||||||
@@ -205,9 +203,7 @@ func TestBackendAdminRollback(t *testing.T) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
if err := m.RollbackLast(); err != nil {
|
rollbackBackendAdmin(t, m)
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
for _, name := range []string{"backend_users", "backend_user_roles", "backend_jwt_blacklist"} {
|
for _, name := range []string{"backend_users", "backend_user_roles", "backend_jwt_blacklist"} {
|
||||||
if gdb.Migrator().HasTable(name) {
|
if gdb.Migrator().HasTable(name) {
|
||||||
t.Fatalf("%s survived admin rollback", 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")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user