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

Commit

Permalink
Support multiple from numbers for system SMS configs (#1312)
Browse files Browse the repository at this point in the history 
This gives system administrators the ability to define a list of available SMS phone numbers. When the system SMS configuration is shared with a realm, the realm can choose the phone number from which they want to send messages.
  • Loading branch information
@sethvargo
sethvargo authored Dec 10, 2020
1 parent 856de9f commit dcdbc54
Show file tree
Hide file tree
Showing 15 changed files with 697 additions and 31 deletions.
113 changes: 101 additions & 12 deletions cmd/server/assets/admin/sms/show.html
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{{define "admin/sms/show"}}

{{$smsConfig := .smsConfig}}
{{$smsFromNumbers := .smsFromNumbers}}

<!doctype html>
<html lang="en">
Expand Down Expand Up @@ -47,25 +48,113 @@ <h1>System SMS config</h1>
</small>
</div>

<div class="form-label-group">
<input type="tel" name="twilio_from_number" id="twilio-from-number" class="form-control text-monospace{{if $smsConfig.ErrorsFor "twilioFromNumber"}} is-invalid{{end}}" autocomplete="new-password"
placeholder="Twilio number" {{if $smsConfig.TwilioFromNumber}}value="{{$smsConfig.TwilioFromNumber}}"{{end}} />
<label for="twilio-from-number">Twilio number</label>
{{template "errorable" $smsConfig.ErrorsFor "twilioFromNumber"}}
<small class="form-text text-muted">
This is the Twilio From Number. Get this value from the Twilio
console. If you plan on sending more than 100 codes per day, we
<strong>strongly recommend</strong> acquiring a toll free number
or SMS short code to reduce the chance that your message will be
flagged as spam.
</small>
<hr />

<p class="small form-text text-muted">
Below are the phone numbers from which messages can be sent.
Purchase these numbers using the Twilio console. After sharing the
system SMS configuration with a realm, they will be able to choose
from these phone numbers. We <strong>strongly recommend</strong>
acquiring a toll free number or SMS short code to reduce the chance
that messages will be flagged as spam.
</p>

<div id="twilio-from-number-template" class="d-none form-row">
<div class="col-sm-3">
<div class="form-label-group">
<input id="template-label" type="text" class="form-control" autocomplete="new-password" placeholder="Label" />
<label>Label</label>
</div>
</div>
<div class="col-sm-9 d-flex">
<div class="form-label-group w-100">
<input id="template-from-number" type="tel" class="form-control text-monospace" autocomplete="new-password" placeholder="Phone number" />
<label>From number</label>
</div>
<a href="#" class="d-inline text-secondary mt-2 ml-3">
<span class="oi oi-circle-x"></span>
</a>
</div>
</div>

<div id="twilio-from-numbers-container" data-twilio-from-numbers="{{$smsFromNumbers | toJSON | toBase64}}">
<p class="text-center">Loading...</p>
</div>

<p>
<small>
<a href="#" id="add-phone-number">
&plus; Add phone number
</a>
</small>
</p>

<button type="submit" class="btn btn-primary btn-block">Update system SMS config</button>
</form>
</div>
</div>
</main>

<script type="text/javascript">
$(function() {
let $container = $('#twilio-from-numbers-container');
let $template = $('#twilio-from-number-template');
let counter = 0;

function addRow(id, label, value) {
let $section = $template.clone();

if (id) {
$('<input>')
.attr('type', 'hidden')
.attr('id', `twilio-from-number-${counter}-id`)
.attr('name', `twilio_from_numbers.${counter}.id`)
.attr('value', id)
.appendTo($section);
}

$section.find('input#template-label')
.prop('required', true)
.attr('id', `twilio-from-number-${counter}-label`)
.attr('name', `twilio_from_numbers.${counter}.label`)
.attr('value', label);

$section.find('input#template-from-number')
.prop('required', true)
.attr('id', `twilio-from-number-${counter}-value`)
.attr('name', `twilio_from_numbers.${counter}.value`)
.attr('value', value);

$section.find('a').click(function(e) {
e.preventDefault();
$section.fadeOut(function() {
$section.remove();
})
});

$section.appendTo($container);
$section.removeClass('d-none');

// Increment counter for next one.
counter++;
}

$('#add-phone-number').click(function(e) {
e.preventDefault();
addRow(null, null, null);
});

// Load existing records.
$container.empty();
let existingRecords = $container.data('twilio-from-numbers');
if (existingRecords) {
let decoded = atob(existingRecords);
$.parseJSON(decoded).forEach(function(record) {
addRow(record.id, record.label, record.value);
});
}
});
</script>
</body>
</html>
{{end}}
33 changes: 30 additions & 3 deletions cmd/server/assets/realmadmin/_form_sms.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

{{$realm := .realm}}
{{$smsConfig := .smsConfig}}
{{$smsFromNumbers := .smsFromNumbers}}
{{$countries := .countries}}

<p class="mb-4">
Expand All @@ -25,14 +26,14 @@

<div class="form-group form-check">
<input type="checkbox" name="use_system_sms_config" id="use-system-sms-config" class="form-check-input" value="1" {{if $realm.UseSystemSMSConfig}} checked{{end}}
data-toggle="collapse" data-target="#sms-form">
data-toggle="collapse" data-target=".sms-system-form">
<label class="form-check-label" for="use-system-sms-config">
Use system SMS configuration
</label>
</div>
{{end}}

<div id="sms-form" class="collapse{{if not $realm.UseSystemSMSConfig}} show{{end}}">
<div class="sms-system-form collapse{{if not $realm.UseSystemSMSConfig}} show{{end}}">
<div class="form-label-group">
<input type="text" name="twilio_account_sid" id="twilio-account-sid" class="form-control text-monospace{{if $smsConfig.ErrorsFor "twilioAccountSid"}} is-invalid{{end}}"
placeholder="Twilio account" {{if $smsConfig.TwilioAccountSid}}value="{{$smsConfig.TwilioAccountSid}}"{{end}} />
Expand Down Expand Up @@ -67,8 +68,34 @@
</div>
</div>

<div class="sms-system-form collapse{{if $realm.UseSystemSMSConfig}} show{{end}}">
<div class="form-label-group">
<select name="sms_from_number_id" id="sms-from-number-id" class="form-control custom-select {{if $realm.ErrorsFor "smsFromNumber"}}is-invalid{{end}}">
<option selected disabled>From number</option>
{{range $smsFromNumber := $smsFromNumbers}}
<option value="{{$smsFromNumber.ID}}" {{selectedIf (eq $realm.SMSFromNumberID $smsFromNumber.ID)}}>{{$smsFromNumber.Label}} &nbsp;&bull;&nbsp; {{$smsFromNumber.Value}}</option>
{{end}}
</select>
{{template "errorable" $realm.ErrorsFor "smsFromNumber"}}
<small class="form-text text-muted">
<p>
This is the phone number from which text messages will originate.
Since you are using the system SMS configuration, you must choose one
of these numbers. To request a new number, contact your system
administrator.
</p>
<p>
<strong>Warning!</strong> These phone numbers may be changed if they
are reported as spam or repeatedly failing to deliver. The phone
number will always correspond to your region, but do not rely on this
exact phone number.
</p>
</small>
</div>
</div>

<div class="form-label-group">
<select name="sms_country" id="sms_country" class="form-control custom-select">
<select name="sms_country" id="sms-country" class="form-control custom-select">
<option selected disabled>SMS country</option>
{{range $name, $value := $countries}}
<option value="{{$value}}"{{if eq $realm.SMSCountry $value}} selected{{end}}>{{$name}}</option>
Expand Down
50 changes: 43 additions & 7 deletions pkg/controller/admin/sms.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,17 @@ import (

// HandleSMSUpdate creates or updates the SMS config.
func (c *Controller) HandleSMSUpdate() http.Handler {
type FormDataFromNumber struct {
ID uint `form:"id"`
Label string `form:"label,required"`
Value string `form:"value,required"`
}

type FormData struct {
TwilioAccountSid string `form:"twilio_account_sid"`
TwilioAuthToken string `form:"twilio_auth_token"`
TwilioFromNumber string `form:"twilio_from_number"`

TwilioFromNumbers []*FormDataFromNumber `form:"twilio_from_numbers"`
}

return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand All @@ -52,29 +59,56 @@ func (c *Controller) HandleSMSUpdate() http.Handler {
smsConfig.IsSystem = true
}

smsFromNumbers, err := c.db.SMSFromNumbers()
if err != nil {
controller.InternalError(w, r, c.h, err)
return
}

// Requested form, stop processing.
if r.Method == http.MethodGet {
c.renderShowSMS(ctx, w, smsConfig)
c.renderShowSMS(ctx, w, smsConfig, smsFromNumbers)
return
}

var form FormData
if err := controller.BindForm(w, r, &form); err != nil {
flash.Error("Failed to process form: %v", err)
c.renderShowSMS(ctx, w, smsConfig)
c.renderShowSMS(ctx, w, smsConfig, smsFromNumbers)
return
}

// Update
// Update Twilio config
smsConfig.ProviderType = sms.ProviderTypeTwilio
smsConfig.TwilioAccountSid = form.TwilioAccountSid
if form.TwilioAuthToken != project.PasswordSentinel {
smsConfig.TwilioAuthToken = form.TwilioAuthToken
}
smsConfig.TwilioFromNumber = form.TwilioFromNumber
if err := c.db.SaveSMSConfig(smsConfig); err != nil {
flash.Error("Failed to save system SMS config: %v", err)
c.renderShowSMS(ctx, w, smsConfig)
c.renderShowSMS(ctx, w, smsConfig, smsFromNumbers)
return
}

// Update from numbers
updatedSMSFromNumbers := make([]*database.SMSFromNumber, 0, len(form.TwilioFromNumbers))
for _, v := range form.TwilioFromNumbers {
// People do weird things in multi-forms. Only accept the value if it's
// completely intact.
if v == nil || v.Label == "" || v.Value == "" {
continue
}

var smsFromNumber database.SMSFromNumber
smsFromNumber.ID = v.ID
smsFromNumber.Label = v.Label
smsFromNumber.Value = v.Value
updatedSMSFromNumbers = append(updatedSMSFromNumbers, &smsFromNumber)
}

if err := c.db.CreateOrUpdateSMSFromNumbers(updatedSMSFromNumbers); err != nil {
flash.Error("Failed to save system SMS from numbers: %s", err)
c.renderShowSMS(ctx, w, smsConfig, smsFromNumbers)
return
}

Expand All @@ -83,9 +117,11 @@ func (c *Controller) HandleSMSUpdate() http.Handler {
})
}

func (c *Controller) renderShowSMS(ctx context.Context, w http.ResponseWriter, smsConfig *database.SMSConfig) {
func (c *Controller) renderShowSMS(ctx context.Context, w http.ResponseWriter,
smsConfig *database.SMSConfig, smsFromNumbers []*database.SMSFromNumber) {
m := controller.TemplateMapFromContext(ctx)
m.Title("SMS - System Admin")
m["smsConfig"] = smsConfig
m["smsFromNumbers"] = smsFromNumbers
c.h.RenderHTML(w, "admin/sms/show", m)
}
46 changes: 40 additions & 6 deletions pkg/controller/admin/sms_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ func TestShowAdminSMS(t *testing.T) {

wantAccountSid := "abc123"
wantAuthToken := "def456"
wantFromNumber := "+11234567890"
wantFromNumber1 := "+11234567890"
wantFromNumber2 := "+99999999999"

if err := chromedp.Run(taskCtx,
// Pre-authenticate the user.
Expand All @@ -80,13 +81,24 @@ func TestShowAdminSMS(t *testing.T) {
// Visit /admin
chromedp.Navigate(`http://`+harness.Server.Addr()+`/admin/sms`),

// Wait for render.
// Wait for render
chromedp.WaitVisible(`body#admin-sms-show`, chromedp.ByQuery),

// Set fields and submit
// Set fields
chromedp.SetValue(`input#twilio-account-sid`, wantAccountSid, chromedp.ByQuery),
chromedp.SetValue(`input#twilio-auth-token`, wantAuthToken, chromedp.ByQuery),
chromedp.SetValue(`input#twilio-from-number`, wantFromNumber, chromedp.ByQuery),

// From number 1
chromedp.Click(`a#add-phone-number`, chromedp.ByQuery),
chromedp.SetValue(`input#twilio-from-number-0-label`, "aaa", chromedp.ByQuery),
chromedp.SetValue(`input#twilio-from-number-0-value`, wantFromNumber1, chromedp.ByQuery),

// From number 2
chromedp.Click(`a#add-phone-number`, chromedp.ByQuery),
chromedp.SetValue(`input#twilio-from-number-1-label`, "zzz", chromedp.ByQuery),
chromedp.SetValue(`input#twilio-from-number-1-value`, wantFromNumber2, chromedp.ByQuery),

// Submit form
chromedp.Submit(`form#sms-form`, chromedp.ByQuery),

// Wait for render.
Expand All @@ -106,7 +118,29 @@ func TestShowAdminSMS(t *testing.T) {
if systemSMSConfig.TwilioAuthToken != wantAuthToken {
t.Errorf("got: %s, want: %s", systemSMSConfig.TwilioAuthToken, wantAuthToken)
}
if systemSMSConfig.TwilioFromNumber != wantFromNumber {
t.Errorf("got: %s, want: %s", systemSMSConfig.TwilioFromNumber, wantFromNumber)

smsFromNumbers, err := harness.Database.SMSFromNumbers()
if err != nil {
t.Fatal(err)
}

if got, want := len(smsFromNumbers), 2; got != want {
t.Fatalf("expected %d to be %d", got, want)
}

aaa := smsFromNumbers[0]
if got, want := aaa.Label, "aaa"; got != want {
t.Errorf("expected %q to be %q", got, want)
}
if got, want := aaa.Value, wantFromNumber1; got != want {
t.Errorf("expected %q to be %q", got, want)
}

zzz := smsFromNumbers[1]
if got, want := zzz.Label, "zzz"; got != want {
t.Errorf("expected %q to be %q", got, want)
}
if got, want := zzz.Value, wantFromNumber2; got != want {
t.Errorf("expected %q to be %q", got, want)
}
}
10 changes: 10 additions & 0 deletions pkg/controller/realmadmin/settings.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func (c *Controller) HandleSettings() http.Handler {
SMS bool `form:"sms"`
UseSystemSMSConfig bool `form:"use_system_sms_config"`
SMSCountry string `form:"sms_country"`
SMSFromNumberID uint `form:"sms_from_number_id"`
TwilioAccountSid string `form:"twilio_account_sid"`
TwilioAuthToken string `form:"twilio_auth_token"`
TwilioFromNumber string `form:"twilio_from_number"`
Expand Down Expand Up @@ -170,6 +171,7 @@ func (c *Controller) HandleSettings() http.Handler {
if form.SMS {
realm.UseSystemSMSConfig = form.UseSystemSMSConfig
realm.SMSCountry = form.SMSCountry
realm.SMSFromNumberID = form.SMSFromNumberID
}

// Email
Expand Down Expand Up @@ -393,6 +395,13 @@ func (c *Controller) renderSettings(
}
}

// Look up the sms from numbers.
smsFromNumbers, err := c.db.SMSFromNumbers()
if err != nil {
controller.InternalError(w, r, c.h, err)
return
}

// Don't pass through the system config to the template - we don't want to
// risk accidentally rendering its ID or values since the realm should never
// see these values. However, we have to go lookup the actual SMS config
Expand Down Expand Up @@ -434,6 +443,7 @@ func (c *Controller) renderSettings(
m.Title("Realm settings")
m["realm"] = realm
m["smsConfig"] = smsConfig
m["smsFromNumbers"] = smsFromNumbers
m["emailConfig"] = emailConfig
m["countries"] = database.Countries
m["testTypes"] = map[string]database.TestType{
Expand Down
Loading

0 comments on commit dcdbc54

Please sign in to comment.