Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

Commit

Permalink
Require region codes are globally unique
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sethvargo committed Sep 22, 2020
1 parent 0525fd6 commit de0f01d
Show file tree
Hide file tree
Showing 9 changed files with 179 additions and 65 deletions.
4 changes: 1 addition & 3 deletions cmd/server/assets/realmadmin/_form_abuse_prevention.html
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions cmd/server/assets/realmadmin/_form_codes.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
5 changes: 4 additions & 1 deletion cmd/server/assets/realmadmin/_form_general.html
Original file line number Diff line number Diff line change
Expand Up @@ -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}}"
Expand All @@ -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"}}
Expand All @@ -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>

Expand Down
1 change: 1 addition & 0 deletions cmd/server/assets/realmadmin/_form_security.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion cmd/server/assets/realmadmin/_form_sms.html
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
84 changes: 32 additions & 52 deletions pkg/controller/realmadmin/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down
25 changes: 24 additions & 1 deletion pkg/database/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -486,3 +486,26 @@ func withRetries(ctx context.Context, f retry.RetryFunc) error {

return retry.Do(ctx, b, f)
}

func stringValue(s *string) string {
if s == nil {
return ""
}
return *s
}

func stringPtr(s string) *string {
if s == "" {
return nil
}
return &s
}

func setStringPtrValue(in **string, out string) {
if out == "" {
*in = nil
return
}

*in = &out
}
91 changes: 91 additions & 0 deletions pkg/database/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
},
})
}

Expand Down
31 changes: 24 additions & 7 deletions pkg/database/realm.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,20 +80,26 @@ 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"`
LongCodeDuration DurationSeconds `gorm:"type:bigint; not null; default: 86400"` // default 24h
// 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"`

Expand Down Expand Up @@ -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)
Expand All @@ -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 == "" {
Expand Down

0 comments on commit de0f01d

Please sign in to comment.