Skip to content

Commit 5a4c46c

Browse files
lakhansamanilakhansamani
authored
fix(storage): clear legacy unique email/phone objects name-agnostically (< 2.3.0 upgrades) (#629)
* fix(storage): handle idx_-named unique constraints on email/phone A v1 gorm:"uniqueIndex" can be stored as a UNIQUE *constraint* named idx_<table>_<col> whose backing index cannot be dropped with DROP INDEX ("constraint ... requires it"), and GORM's GetIndexes does not surface constraint-backed indexes — so the previous index-only path missed it and AutoMigrate still aborted with SQLSTATE 42704 (seen in the field on authorizer_otps.phone_number). Add idx_<table>_<col> to the constraint-name drop list so it is removed via DROP CONSTRAINT. Extend the migration test to seed all three real forms: a *_key constraint, an idx_-named constraint, and a standalone unique index. * fix(storage): drop legacy unique email/phone objects name-agnostically The previous approach enumerated known constraint names (<table>_<col>_key, uni_<table>_<col>, idx_<table>_<col>). Any other name — e.g. a custom constraint from a hand-rolled v1 migration — still slipped through and aborted startup with the GORM "DROP CONSTRAINT uni_<table>_<col>" / SQLSTATE 42704 failure. Replace the name guessing with a catalog-driven sweep: read the actual single-column UNIQUE constraint names on users/otps email & phone_number from information_schema.table_constraints (the same source GORM's MigrateColumnUnique reads to decide a column is unique) and DROP CONSTRAINT each by its real name, then drop any standalone single-column UNIQUE index. Guards every < 2.3.0 upgrader regardless of how the constraint was named. Best-effort and non-fatal; sqlite (no information_schema, unaffected) and fresh installs are no-ops. Test now also seeds a custom-named constraint.
1 parent 255d55d commit 5a4c46c

2 files changed

Lines changed: 124 additions & 72 deletions

File tree

‎internal/storage/db/sql/provider.go‎

Lines changed: 91 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -74,71 +74,21 @@ func NewProvider(
7474
return nil, err
7575
}
7676

77-
// v1 declared the email and phone_number columns of authorizer_users and
78-
// authorizer_otps as UNIQUE. v2 keeps plain (non-unique) indexes and enforces
79-
// uniqueness in the application layer (see AddUser/UpdateUser) so behaviour is
80-
// identical across all supported databases and these fields can stay optional
81-
// (email-only or phone-only signups).
77+
// Older Authorizer releases (< 2.3.0) declared the email and phone_number
78+
// columns of authorizer_users and authorizer_otps as UNIQUE. v2 keeps plain
79+
// (non-unique) indexes and enforces uniqueness in the application layer (see
80+
// AddUser/UpdateUser), so behaviour is identical across all supported
81+
// databases and these fields can stay optional (email-only / phone-only
82+
// signups).
8283
//
83-
// On an upgraded SQL database those columns are still unique, so GORM
84-
// AutoMigrate tries to DROP CONSTRAINT "uni_<table>_<col>" — a name that does
85-
// not match what the database actually created — failing with SQLSTATE 42704
86-
// ("constraint does not exist") and aborting the whole migration. Depending on
87-
// the v1 release, the old uniqueness exists either as a CONSTRAINT
88-
// (<table>_<col>_key) or as a standalone UNIQUE INDEX (idx_<table>_<col>), so
89-
// we clear both forms before AutoMigrate. Non-fatal: fresh installs have
90-
// nothing to drop.
91-
staleUniqueColumns := []struct {
92-
model any
93-
table, col string
94-
}{
95-
{&schemas.User{}, "authorizer_users", "email"},
96-
{&schemas.User{}, "authorizer_users", "phone_number"},
97-
{&schemas.OTP{}, "authorizer_otps", "email"},
98-
{&schemas.OTP{}, "authorizer_otps", "phone_number"},
99-
}
100-
101-
// 1) Drop stale unique CONSTRAINTs by their well-known names.
102-
// "<table>_<col>_key" is the Postgres/MySQL default; "uni_<table>_<col>" is
103-
// GORM's default unique-constraint name.
104-
for _, c := range staleUniqueColumns {
105-
for _, name := range []string{c.table + "_" + c.col + "_key", "uni_" + c.table + "_" + c.col} {
106-
if sqlDB.Migrator().HasConstraint(c.model, name) {
107-
if dropErr := sqlDB.Migrator().DropConstraint(c.model, name); dropErr != nil {
108-
deps.Log.Debug().Err(dropErr).Str("constraint", name).Msg("failed to drop stale unique constraint")
109-
}
110-
}
111-
}
112-
}
113-
114-
// 2) Drop stale standalone UNIQUE INDEXes on the same columns (e.g.
115-
// idx_authorizer_otps_phone_number created by a v1 gorm:"uniqueIndex").
116-
// This is name-agnostic: only single-column UNIQUE indexes on email /
117-
// phone_number are removed, so the current schema's non-unique indexes are
118-
// left intact and AutoMigrate recreates anything it needs as non-unique.
119-
uniqueColTargets := map[string]bool{"email": true, "phone_number": true}
120-
for _, model := range []any{&schemas.User{}, &schemas.OTP{}} {
121-
indexes, idxErr := sqlDB.Migrator().GetIndexes(model)
122-
if idxErr != nil {
123-
deps.Log.Debug().Err(idxErr).Msg("failed to list indexes while clearing stale unique indexes")
124-
continue
125-
}
126-
for _, idx := range indexes {
127-
unique, ok := idx.Unique()
128-
if !ok || !unique {
129-
continue
130-
}
131-
cols := idx.Columns()
132-
if len(cols) != 1 || !uniqueColTargets[cols[0]] {
133-
continue
134-
}
135-
if dropErr := sqlDB.Migrator().DropIndex(model, idx.Name()); dropErr != nil {
136-
// Constraint-backed indexes (handled in step 1) can't be dropped
137-
// directly on Postgres — that's expected and harmless here.
138-
deps.Log.Debug().Err(dropErr).Str("index", idx.Name()).Msg("failed to drop stale unique index")
139-
}
140-
}
141-
}
84+
// On an upgraded SQL database those columns are still unique, so GORM 1.25.x
85+
// AutoMigrate reconciles them by issuing DROP CONSTRAINT "uni_<table>_<col>"
86+
// — a fixed name (NamingStrategy.UniqueName) that rarely matches what the old
87+
// DB actually created (authorizer_users_email_key, idx_authorizer_otps_phone_number,
88+
// or any custom name) — failing with "constraint does not exist" (Postgres
89+
// SQLSTATE 42704) and aborting startup. Clear the legacy uniqueness up front,
90+
// name-agnostically, before AutoMigrate runs.
91+
clearLegacyColumnUniqueness(sqlDB, deps.Log)
14292

14393
err = sqlDB.AutoMigrate(&schemas.User{}, &schemas.VerificationRequest{}, &schemas.Session{}, &schemas.Env{}, &schemas.Webhook{}, &schemas.WebhookLog{}, &schemas.EmailTemplate{}, &schemas.OTP{}, &schemas.Authenticator{}, &schemas.SessionToken{}, &schemas.MFASession{}, &schemas.OAuthState{}, &schemas.AuditLog{})
14494
if err != nil {
@@ -189,3 +139,80 @@ func (p *provider) Close() error {
189139
}
190140
return sqlDB.Close()
191141
}
142+
143+
// clearLegacyColumnUniqueness removes any single-column UNIQUE constraint or
144+
// index on the email / phone_number columns of authorizer_users and
145+
// authorizer_otps, regardless of its name. See the call site in NewProvider for
146+
// why this must run before AutoMigrate. Best-effort and non-fatal: fresh
147+
// installs, and catalogs without information_schema (sqlite), simply find
148+
// nothing to drop.
149+
func clearLegacyColumnUniqueness(db *gorm.DB, log *zerolog.Logger) {
150+
targets := map[string]bool{"email": true, "phone_number": true}
151+
tables := []struct {
152+
model any
153+
name string
154+
}{
155+
{&schemas.User{}, "authorizer_users"},
156+
{&schemas.OTP{}, "authorizer_otps"},
157+
}
158+
159+
for _, t := range tables {
160+
// 1) Drop single-column UNIQUE *constraints* by their real names, read
161+
// from the same catalog GORM uses (information_schema). This matches
162+
// ANY name — ..._key, uni_..., idx_..., or custom — which is what
163+
// actually prevents the DROP CONSTRAINT "uni_..." 42704 abort. A
164+
// constraint's backing index cannot be removed with DROP INDEX, so it
165+
// must go via DropConstraint.
166+
var rows []struct {
167+
ConstraintName string `gorm:"column:constraint_name"`
168+
ColumnName string `gorm:"column:column_name"`
169+
}
170+
const q = `SELECT tc.constraint_name AS constraint_name, kcu.column_name AS column_name
171+
FROM information_schema.table_constraints tc
172+
JOIN information_schema.key_column_usage kcu
173+
ON tc.constraint_name = kcu.constraint_name
174+
AND tc.table_schema = kcu.table_schema
175+
AND tc.table_name = kcu.table_name
176+
WHERE tc.constraint_type = 'UNIQUE' AND tc.table_name = ?`
177+
if err := db.Raw(q, t.name).Scan(&rows).Error; err != nil {
178+
// sqlite and other catalogs without information_schema land here; they
179+
// are not affected by the GORM unique-reconciliation behaviour.
180+
log.Debug().Err(err).Str("table", t.name).Msg("skip legacy unique-constraint scan")
181+
} else {
182+
columnsByConstraint := map[string][]string{}
183+
for _, r := range rows {
184+
columnsByConstraint[r.ConstraintName] = append(columnsByConstraint[r.ConstraintName], r.ColumnName)
185+
}
186+
for name, cols := range columnsByConstraint {
187+
if len(cols) == 1 && targets[cols[0]] {
188+
if err := db.Migrator().DropConstraint(t.model, name); err != nil {
189+
log.Debug().Err(err).Str("constraint", name).Msg("failed to drop legacy unique constraint")
190+
}
191+
}
192+
}
193+
}
194+
195+
// 2) Drop any standalone single-column UNIQUE *index* (CREATE UNIQUE
196+
// INDEX, not a constraint) on the same columns. These do not cause the
197+
// abort — GORM reads column-uniqueness from table_constraints only —
198+
// but removing them makes the column genuinely non-unique, matching the
199+
// v2 application-layer model. GetIndexes already excludes
200+
// constraint-backed indexes, so this only sees the standalone form;
201+
// AutoMigrate then recreates the plain non-unique search index.
202+
indexes, err := db.Migrator().GetIndexes(t.model)
203+
if err != nil {
204+
log.Debug().Err(err).Str("table", t.name).Msg("skip legacy unique-index scan")
205+
continue
206+
}
207+
for _, idx := range indexes {
208+
if unique, ok := idx.Unique(); !ok || !unique {
209+
continue
210+
}
211+
if cols := idx.Columns(); len(cols) == 1 && targets[cols[0]] {
212+
if err := db.Migrator().DropIndex(t.model, idx.Name()); err != nil {
213+
log.Debug().Err(err).Str("index", idx.Name()).Msg("failed to drop legacy unique index")
214+
}
215+
}
216+
}
217+
}
218+
}

‎internal/storage/db/sql/provider_migration_test.go‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -187,23 +187,48 @@ func TestStaleUniqueConstraintMigration(t *testing.T) {
187187
p1, err := NewProvider(cfg, deps)
188188
require.NoError(t, err)
189189

190-
// Simulate a v1 database: a UNIQUE CONSTRAINT on users.email and a
191-
// standalone UNIQUE INDEX on otps.phone_number — the two real-world
192-
// forms reported in the field. Idempotent so reruns against the shared
193-
// test DB don't fail on pre-existing objects.
194-
require.NoError(t, p1.db.Exec(`ALTER TABLE authorizer_users DROP CONSTRAINT IF EXISTS authorizer_users_email_key`).Error)
195-
require.NoError(t, p1.db.Exec(`ALTER TABLE authorizer_users ADD CONSTRAINT authorizer_users_email_key UNIQUE (email)`).Error)
196-
require.NoError(t, p1.db.Exec(`DROP INDEX IF EXISTS idx_authorizer_otps_phone_number`).Error)
197-
require.NoError(t, p1.db.Exec(`CREATE UNIQUE INDEX idx_authorizer_otps_phone_number ON authorizer_otps (phone_number)`).Error)
190+
// Simulate a v1 database, covering every real-world stale form so the
191+
// name-agnostic cleanup is exercised end-to-end:
192+
// - users.email: UNIQUE CONSTRAINT "authorizer_users_email_key"
193+
// (Postgres default for a gorm:"unique" tag)
194+
// - users.phone_number: UNIQUE CONSTRAINT "my_legacy_phone_uq"
195+
// (arbitrary/custom name — only a catalog-driven
196+
// drop, not name enumeration, can catch this)
197+
// - otps.phone_number: UNIQUE CONSTRAINT "idx_authorizer_otps_phone_number"
198+
// (idx_-named constraint; backing index can NOT
199+
// be dropped with DROP INDEX — field-reported case)
200+
// - otps.email: standalone UNIQUE INDEX "idx_authorizer_otps_email"
201+
// Idempotent so reruns against the shared test DB don't clash.
202+
for _, stmt := range []string{
203+
`ALTER TABLE authorizer_users DROP CONSTRAINT IF EXISTS authorizer_users_email_key`,
204+
`ALTER TABLE authorizer_users ADD CONSTRAINT authorizer_users_email_key UNIQUE (email)`,
205+
`ALTER TABLE authorizer_users DROP CONSTRAINT IF EXISTS my_legacy_phone_uq`,
206+
`ALTER TABLE authorizer_users ADD CONSTRAINT my_legacy_phone_uq UNIQUE (phone_number)`,
207+
`ALTER TABLE authorizer_otps DROP CONSTRAINT IF EXISTS idx_authorizer_otps_phone_number`,
208+
`DROP INDEX IF EXISTS idx_authorizer_otps_phone_number`,
209+
`ALTER TABLE authorizer_otps ADD CONSTRAINT idx_authorizer_otps_phone_number UNIQUE (phone_number)`,
210+
`DROP INDEX IF EXISTS idx_authorizer_otps_email`,
211+
`CREATE UNIQUE INDEX idx_authorizer_otps_email ON authorizer_otps (email)`,
212+
} {
213+
require.NoError(t, p1.db.Exec(stmt).Error, stmt)
214+
}
198215
require.True(t, p1.db.Migrator().HasConstraint(&schemas.User{}, "authorizer_users_email_key"),
199216
"precondition: stale unique constraint seeded")
217+
require.True(t, p1.db.Migrator().HasConstraint(&schemas.User{}, "my_legacy_phone_uq"),
218+
"precondition: custom-named unique constraint seeded")
219+
require.True(t, p1.db.Migrator().HasConstraint(&schemas.OTP{}, "idx_authorizer_otps_phone_number"),
220+
"precondition: idx_-named unique constraint seeded")
200221

201222
// Second boot must clear the stale uniqueness and complete migration.
202223
p2, err := NewProvider(cfg, deps)
203224
require.NoError(t, err, "startup must not abort with SQLSTATE 42704")
204225

205226
assert.False(t, p2.db.Migrator().HasConstraint(&schemas.User{}, "authorizer_users_email_key"),
206227
"stale unique constraint should be dropped")
228+
assert.False(t, p2.db.Migrator().HasConstraint(&schemas.User{}, "my_legacy_phone_uq"),
229+
"custom-named unique constraint should be dropped")
230+
assert.False(t, p2.db.Migrator().HasConstraint(&schemas.OTP{}, "idx_authorizer_otps_phone_number"),
231+
"stale idx_-named unique constraint should be dropped")
207232

208233
// The search index is preserved, just no longer unique: AutoMigrate
209234
// recreates idx_authorizer_otps_phone_number as a non-unique index.

0 commit comments

Comments
 (0)