From b6b0791e3afa3ffce6479e7bfa4e84f66f983c50 Mon Sep 17 00:00:00 2001
From: Seth Vargo <seth@sethvargo.com>
Date: Mon, 21 Sep 2020 17:23:31 -0400
Subject: [PATCH] Require region codes are globally unique

This is required for the ENX redirector, but we've also always made the
assumption that these need to be globally unique.

Note that this requires changing the column to citext (for
case-insensitive matching) and to NULL out the default of '', since ''
is a duplicate of ''.

It also handles any existing duplicates by appending -N to them before
applying the database-level unique constraint.
---
 .../realmadmin/_form_abuse_prevention.html    |  4 +-
 cmd/server/assets/realmadmin/_form_codes.html |  1 +
 .../assets/realmadmin/_form_general.html      |  5 +-
 .../assets/realmadmin/_form_security.html     |  1 +
 cmd/server/assets/realmadmin/_form_sms.html   |  2 +-
 pkg/controller/realmadmin/settings.go         | 84 +++++++----------
 pkg/database/database.go                      | 18 +++-
 pkg/database/migrations.go                    | 91 +++++++++++++++++++
 pkg/database/realm.go                         | 31 +++++--
 9 files changed, 172 insertions(+), 65 deletions(-)

diff --git a/cmd/server/assets/realmadmin/_form_abuse_prevention.html b/cmd/server/assets/realmadmin/_form_abuse_prevention.html
index 08e32ab2d..db2f717b2 100644
--- a/cmd/server/assets/realmadmin/_form_abuse_prevention.html
+++ b/cmd/server/assets/realmadmin/_form_abuse_prevention.html
@@ -4,6 +4,7 @@
 
 <form method="POST" action="/realm/settings#abuse-prevention" class="floating-form">
   {{ .csrfField }}
+  <input type="hidden" name="abuse_prevention" value="1" />
 
   <p>
     Abuse prevention uses the historical record of your realm's past
@@ -18,9 +19,6 @@
   </p>
 
   <div class="form-group form-check">
-    <!-- This forces the form to send a value of false (instead of no value at
-    all), when abuse prevention is unchecked. -->
-    <input type="hidden" name="abuse_prevention_enabled" value="0" />
     <input type="checkbox" name="abuse_prevention_enabled" id="abuse-prevention-enabled" class="form-check-input" value="1" {{if $realm.AbusePreventionEnabled}} checked{{end}}>
     <label class="form-check-label" for="abuse-prevention-enabled">
       Enable abuse prevention
diff --git a/cmd/server/assets/realmadmin/_form_codes.html b/cmd/server/assets/realmadmin/_form_codes.html
index 8b5b9c0bb..a9d1f53bd 100644
--- a/cmd/server/assets/realmadmin/_form_codes.html
+++ b/cmd/server/assets/realmadmin/_form_codes.html
@@ -10,6 +10,7 @@
 
 <form method="POST" action="/realm/settings#codes" class="floating-form">
   {{ .csrfField }}
+  <input type="hidden" name="codes" value="1" />
 
   <div class="form-group">
     <label>Allowed test types</label>
diff --git a/cmd/server/assets/realmadmin/_form_general.html b/cmd/server/assets/realmadmin/_form_general.html
index 9a61a0e96..ef7900d24 100644
--- a/cmd/server/assets/realmadmin/_form_general.html
+++ b/cmd/server/assets/realmadmin/_form_general.html
@@ -9,6 +9,7 @@
 
 <form method="POST" action="/realm/settings#general" class="floating-form">
   {{ .csrfField }}
+  <input type="hidden" name="general" value="1" />
 
   <div class="form-label-group">
     <input type="text" name="name" id="name" class="form-control{{if $realm.ErrorsFor "name"}} is-invalid{{end}}"
@@ -22,11 +23,12 @@
     <small class="form-text text-muted">
       The realm name is displayed on the realm selection page and in the header
       when selected. Choose a descriptive name that your team will recognize.
+      This value must be globally unique in the system.
     </small>
   </div>
 
   <div class="form-label-group">
-    <input type="text" name="region_code" id="region-code" class="form-control{{if $realm.ErrorsFor "regionCode"}} is-invalid{{end}}"
+    <input type="text" name="region_code" id="region-code" class="form-control text-uppercase{{if $realm.ErrorsFor "regionCode"}} is-invalid{{end}}"
       value="{{$realm.RegionCode}}" placeholder="Region code" />
     <label for="region-code">Region code</label>
     {{if $realm.ErrorsFor "regionCode"}}
@@ -41,6 +43,7 @@
       <a href="https://en.wikipedia.org/wiki/List_of_ISO_3166_country_codes">ISO
       3166-1 country codes and ISO 3166-2 subdivision codes</a> where
       applicable. For example, Washington state would be <code>US-WA</code>.
+      This value must globally unique in the system.
     </small>
   </div>
 
diff --git a/cmd/server/assets/realmadmin/_form_security.html b/cmd/server/assets/realmadmin/_form_security.html
index eafa425bd..9ce885ebd 100644
--- a/cmd/server/assets/realmadmin/_form_security.html
+++ b/cmd/server/assets/realmadmin/_form_security.html
@@ -8,6 +8,7 @@
 
 <form method="POST" action="/realm/settings#security" class="floating-form">
   {{ .csrfField }}
+  <input type="hidden" name="security" value="1" />
 
   <div class="form-group">
     <label for="email-verified-mode">Email verification</label>
diff --git a/cmd/server/assets/realmadmin/_form_sms.html b/cmd/server/assets/realmadmin/_form_sms.html
index 6f8091258..598141eda 100644
--- a/cmd/server/assets/realmadmin/_form_sms.html
+++ b/cmd/server/assets/realmadmin/_form_sms.html
@@ -11,7 +11,7 @@
 
 <form method="POST" action="/realm/settings#sms" class="floating-form">
   {{ .csrfField }}
-  <input type="hidden" name="twilio_sms_config" value="1" />
+  <input type="hidden" name="sms" value="1" />
 
   <div class="form-label-group">
     <input type="text" name="twilio_account_sid" id="twilio-account-sid" class="form-control text-monospace"
diff --git a/pkg/controller/realmadmin/settings.go b/pkg/controller/realmadmin/settings.go
index b35aeb33c..d4216dd6a 100644
--- a/pkg/controller/realmadmin/settings.go
+++ b/pkg/controller/realmadmin/settings.go
@@ -46,29 +46,33 @@ func init() {
 
 func (c *Controller) HandleSettings() http.Handler {
 	type FormData struct {
+		General        bool   `form:"general"`
 		Name           string `form:"name"`
 		RegionCode     string `form:"region_code"`
 		WelcomeMessage string `form:"welcome_message"`
 
+		Codes                 bool              `form:"codes"`
 		AllowedTestTypes      database.TestType `form:"allowed_test_types"`
-		RequireDate           *bool             `form:"require_date"`
+		RequireDate           bool              `form:"require_date"`
 		CodeLength            uint              `form:"code_length"`
 		CodeDurationMinutes   int64             `form:"code_duration"`
 		LongCodeLength        uint              `form:"long_code_length"`
 		LongCodeDurationHours int64             `form:"long_code_duration"`
 		SMSTextTemplate       string            `form:"sms_text_template"`
 
-		TwilioSMSConfig  bool   `form:"twilio_sms_config"`
+		SMS              bool   `form:"sms"`
 		TwilioAccountSid string `form:"twilio_account_sid"`
 		TwilioAuthToken  string `form:"twilio_auth_token"`
 		TwilioFromNumber string `form:"twilio_from_number"`
 
-		MFAMode                     *int16 `form:"mfa_mode"`
-		EmailVerifiedMode           *int16 `form:"email_verified_mode"`
-		PasswordRotationPeriodDays  uint   `form:"password_rotation_period_days"`
-		PasswordRotationWarningDays uint   `form:"password_rotation_warning_days"`
+		Security                    bool  `form:"security"`
+		MFAMode                     int16 `form:"mfa_mode"`
+		EmailVerifiedMode           int16 `form:"email_verified_mode"`
+		PasswordRotationPeriodDays  uint  `form:"password_rotation_period_days"`
+		PasswordRotationWarningDays uint  `form:"password_rotation_warning_days"`
 
-		AbusePreventionEnabled     *bool   `form:"abuse_prevention_enabled"`
+		AbusePrevention            bool    `form:"abuse_prevention"`
+		AbusePreventionEnabled     bool    `form:"abuse_prevention_enabled"`
 		AbusePreventionLimitFactor float32 `form:"abuse_prevention_limit_factor"`
 		AbusePreventionBurst       uint64  `form:"abuse_prevention_burst"`
 	}
@@ -110,59 +114,35 @@ func (c *Controller) HandleSettings() http.Handler {
 		}
 
 		// General
-		if v := form.Name; v != "" {
-			realm.Name = v
-		}
-		if v := form.RegionCode; v != "" {
-			realm.RegionCode = v
-		}
-		if v := form.WelcomeMessage; v != "" {
-			realm.WelcomeMessage = v
+		if form.General {
+			realm.Name = form.Name
+			realm.RegionCode = form.RegionCode
+			realm.WelcomeMessage = form.WelcomeMessage
 		}
 
 		// Codes
-		if v := form.AllowedTestTypes; v > 0 {
-			realm.AllowedTestTypes = v
-		}
-		if v := form.RequireDate; v != nil {
-			realm.RequireDate = *v
-		}
-		if v := form.CodeLength; v > 0 {
-			realm.CodeLength = v
-		}
-		if v := form.CodeDurationMinutes; v > 0 {
-			realm.CodeDuration.Duration = time.Duration(v) * time.Minute
-		}
-		if v := form.LongCodeLength; v > 0 {
-			realm.LongCodeLength = v
-		}
-		if v := form.LongCodeDurationHours; v > 0 {
-			realm.LongCodeDuration.Duration = time.Duration(v) * time.Minute
-		}
-		if v := form.SMSTextTemplate; v != "" {
-			realm.SMSTextTemplate = v
+		if form.Codes {
+			realm.AllowedTestTypes = form.AllowedTestTypes
+			realm.RequireDate = form.RequireDate
+			realm.CodeLength = form.CodeLength
+			realm.CodeDuration.Duration = time.Duration(form.CodeDurationMinutes) * time.Minute
+			realm.LongCodeLength = form.LongCodeLength
+			realm.LongCodeDuration.Duration = time.Duration(form.LongCodeDurationHours) * time.Hour
+			realm.SMSTextTemplate = form.SMSTextTemplate
 		}
 
 		// Security
-		if v := form.EmailVerifiedMode; v != nil {
-			realm.EmailVerifiedMode = database.AuthRequirement(*v)
-		}
-		if v := form.MFAMode; v != nil {
-			realm.MFAMode = database.AuthRequirement(*v)
-		}
-		if v := form.PasswordRotationPeriodDays; v > 0 {
-			realm.PasswordRotationPeriodDays = v
-		}
-		if v := form.PasswordRotationWarningDays; v > 0 {
-			realm.PasswordRotationWarningDays = v
+		if form.Security {
+			realm.EmailVerifiedMode = database.AuthRequirement(form.EmailVerifiedMode)
+			realm.MFAMode = database.AuthRequirement(form.MFAMode)
+			realm.PasswordRotationPeriodDays = form.PasswordRotationPeriodDays
+			realm.PasswordRotationWarningDays = form.PasswordRotationWarningDays
 		}
 
 		// Abuse prevention
-		if v := form.AbusePreventionEnabled; v != nil {
-			realm.AbusePreventionEnabled = *v
-		}
-		if v := form.AbusePreventionLimitFactor; v > 0 {
-			realm.AbusePreventionLimitFactor = v
+		if form.AbusePrevention {
+			realm.AbusePreventionEnabled = form.AbusePreventionEnabled
+			realm.AbusePreventionLimitFactor = form.AbusePreventionLimitFactor
 		}
 
 		// Save
@@ -172,7 +152,7 @@ func (c *Controller) HandleSettings() http.Handler {
 			return
 		}
 
-		if form.TwilioSMSConfig {
+		if form.SMS {
 			// Process SMS settings
 			smsConfig, err := realm.SMSConfig(c.db)
 			if err != nil && !database.IsNotFound(err) {
diff --git a/pkg/database/database.go b/pkg/database/database.go
index c380d1d74..3475446a3 100644
--- a/pkg/database/database.go
+++ b/pkg/database/database.go
@@ -415,7 +415,7 @@ func callbackKMSEncrypt(ctx context.Context, keyManager keys.KeyManager, keyID,
 	}
 }
 
-// callback HMAC alters HMACs the value with the given key before saving.
+// callbackHMAC alters HMACs the value with the given key before saving.
 func callbackHMAC(ctx context.Context, hashFunc func(string) (string, error), table, column string) func(scope *gorm.Scope) {
 	return func(scope *gorm.Scope) {
 		// Do nothing if not the target table
@@ -486,3 +486,19 @@ func withRetries(ctx context.Context, f retry.RetryFunc) error {
 
 	return retry.Do(ctx, b, f)
 }
+
+// stringValue gets the value of the string pointer, returning "" for nil.
+func stringValue(s *string) string {
+	if s == nil {
+		return ""
+	}
+	return *s
+}
+
+// stringPtr converts the string value to a pointer, returning nil for "".
+func stringPtr(s string) *string {
+	if s == "" {
+		return nil
+	}
+	return &s
+}
diff --git a/pkg/database/migrations.go b/pkg/database/migrations.go
index 9fa0b4bda..b99da011d 100644
--- a/pkg/database/migrations.go
+++ b/pkg/database/migrations.go
@@ -1131,6 +1131,97 @@ func (db *Database) getMigrations(ctx context.Context) *gormigrate.Gormigrate {
 				return nil
 			},
 		},
+		{
+			ID: "00049-MakeRegionCodeUnique",
+			Migrate: func(tx *gorm.DB) error {
+				sqls := []string{
+					// Make region code case insensitive and unique.
+					"ALTER TABLE realms ALTER COLUMN region_code TYPE CITEXT",
+					"ALTER TABLE realms ALTER COLUMN region_code DROP DEFAULT",
+					"ALTER TABLE realms ALTER COLUMN region_code DROP NOT NULL",
+				}
+				for _, sql := range sqls {
+					if err := tx.Exec(sql).Error; err != nil {
+						return err
+					}
+				}
+
+				// Make any existing empty string region codes to NULL. Without this,
+				// the new unique constraint will fail.
+				if err := tx.Exec("UPDATE realms SET region_code = NULL WHERE TRIM(region_code) = ''").Error; err != nil {
+					return err
+				}
+
+				// Make all region codes uppercase.
+				if err := tx.Exec("UPDATE realms SET region_code = UPPER(region_code) WHERE region_code IS NOT NULL").Error; err != nil {
+					return err
+				}
+
+				// Find any existing duplicate region codes - this could be combined
+				// into a much larger SQL statement with the next thing, but I'm
+				// optimizing for readability here.
+				var dupRegionCodes []string
+				if err := tx.Model(&Realm{}).
+					Unscoped().
+					Select("UPPER(region_code) AS region_code").
+					Where("region_code IS NOT NULL").
+					Group("region_code").
+					Having("COUNT(*) > 1").
+					Pluck("region_code", &dupRegionCodes).
+					Error; err != nil {
+					return err
+				}
+
+				// Update any duplicate regions to not be duplicate anymore.
+				for _, dupRegionCode := range dupRegionCodes {
+					logger.Warn("de-duplicating region code %q", dupRegionCode)
+
+					// I call this the "Microsoft method". For each duplicate realm,
+					// append -N, starting with 1. If there are 3 realms with the region
+					// code "PA", their new values will be "PA", "PA-1", and "PA-2"
+					// respectively.
+					sql := `
+						UPDATE
+							realms
+						SET region_code = CONCAT(realms.region_code, '-', (z-1)::text)
+						FROM (
+							SELECT
+								id,
+								region_code,
+								ROW_NUMBER() OVER (ORDER BY id ASC) AS z
+							FROM realms
+							WHERE UPPER(region_code) = UPPER($1)
+						) AS sq
+						WHERE realms.id = sq.id AND sq.z > 1`
+					if err := tx.Exec(sql, dupRegionCode).Error; err != nil {
+						return err
+					}
+				}
+
+				sqls = []string{
+					// There's already a runtime constraint and validation on names, this
+					// is just an extra layer of protection at the database layer.
+					"ALTER TABLE realms ALTER COLUMN name SET NOT NULL",
+
+					// Alter the unique index on realm names to be a column constraint.
+					"DROP INDEX IF EXISTS uix_realms_name",
+					"ALTER TABLE realms ADD CONSTRAINT uix_realms_name UNIQUE (name)",
+
+					// Now finally add a unique constraint on region codes.
+					"ALTER TABLE realms ADD CONSTRAINT uix_realms_region_code UNIQUE (region_code)",
+				}
+
+				for _, sql := range sqls {
+					if err := tx.Exec(sql).Error; err != nil {
+						return err
+					}
+				}
+				return nil
+			},
+			Rollback: func(tx *gorm.DB) error {
+				return nil
+			},
+		},
 	})
 }
 
diff --git a/pkg/database/realm.go b/pkg/database/realm.go
index de8abad8e..b2784d17e 100644
--- a/pkg/database/realm.go
+++ b/pkg/database/realm.go
@@ -80,8 +80,19 @@ type Realm struct {
 	// Name is the name of the realm.
 	Name string `gorm:"type:varchar(200);unique_index"`
 
+	// RegionCode is both a display attribute and required field for ENX. To
+	// handle NULL and uniqueness, the field is converted from it's ptr type to a
+	// concrete type in callbacks. Do not modify RegionCodePtr directly.
+	RegionCode    string  `gorm:"-"`
+	RegionCodePtr *string `gorm:"column:region_code; type:varchar(10);"`
+
+	// WelcomeMessage is arbitrary realm-defined data to display to users after
+	// selecting this realm. If empty, nothing is displayed. The format is
+	// markdown. Do not modify WelcomeMessagePtr directly.
+	WelcomeMessage    string  `gorm:"-"`
+	WelcomeMessagePtr *string `gorm:"column:welcome_message; type:text;"`
+
 	// Code configuration
-	RegionCode       string          `gorm:"type:varchar(10); not null; default: ''"`
 	CodeLength       uint            `gorm:"type:smallint; not null; default: 8"`
 	CodeDuration     DurationSeconds `gorm:"type:bigint; not null; default: 900"` // default 15m (in seconds)
 	LongCodeLength   uint            `gorm:"type:smallint; not null; default: 16"`
@@ -89,11 +100,6 @@ type Realm struct {
 	// SMS Content
 	SMSTextTemplate string `gorm:"type:varchar(400); not null; default: 'This is your Exposure Notifications Verification code: [longcode] Expires in [longexpires] hours'"`
 
-	// WelcomeMessage is arbitrary realm-defined data to display to users after
-	// selecting this realm. If empty, nothing is displayed. The format is
-	// markdown.
-	WelcomeMessage string `gorm:"type:text"`
-
 	// MFAMode represents the mode for Multi-Factor-Authorization requirements for the realm.
 	MFAMode AuthRequirement `gorm:"type:smallint; not null; default: 0"`
 
@@ -187,6 +193,14 @@ func (r *Realm) SigningKeyID() string {
 	return fmt.Sprintf("realm-%d", r.ID)
 }
 
+// AfterFind runs after a realm is found.
+func (r *Realm) AfterFind(tx *gorm.DB) error {
+	r.RegionCode = stringValue(r.RegionCodePtr)
+	r.WelcomeMessage = stringValue(r.WelcomeMessagePtr)
+
+	return nil
+}
+
 // BeforeSave runs validations. If there are errors, the save fails.
 func (r *Realm) BeforeSave(tx *gorm.DB) error {
 	r.Name = strings.TrimSpace(r.Name)
@@ -195,10 +209,13 @@ func (r *Realm) BeforeSave(tx *gorm.DB) error {
 	}
 
 	r.RegionCode = strings.ToUpper(strings.TrimSpace(r.RegionCode))
-
 	if len(r.RegionCode) > 10 {
 		r.AddError("regionCode", "cannot be more than 10 characters")
 	}
+	r.RegionCodePtr = stringPtr(r.RegionCode)
+
+	r.WelcomeMessage = strings.TrimSpace(r.WelcomeMessage)
+	r.WelcomeMessagePtr = stringPtr(r.WelcomeMessage)
 
 	if r.EnableENExpress {
 		if r.RegionCode == "" {