Minor App Config API tweaks requested by @mikestone14 (#681)

* Minor App Config API tweaks requested by @mikestone14

* Refactored mail test into separate method, implemented code review changes

server/datastore/mysql/app_configs.go

@@ -45,7 +45,7 @@ func (d *Datastore) SaveAppConfig(info *kolide.AppConfig) error {
 			smtp_password,
 			smtp_verify_ssl_certs,
 			smtp_enable_start_tls,
-			smtp_disabled
+			smtp_enabled
 		)
 		VALUES( 1, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )
 		ON DUPLICATE KEY UPDATE
@@ -64,7 +64,7 @@ func (d *Datastore) SaveAppConfig(info *kolide.AppConfig) error {
 			smtp_password = VALUES(smtp_password),
 			smtp_verify_ssl_certs = VALUES(smtp_verify_ssl_certs),
 			smtp_enable_start_tls = VALUES(smtp_enable_start_tls),
-			smtp_disabled = VALUES(smtp_disabled)
+			smtp_enabled = VALUES(smtp_enabled)
 	`
 
 	_, err := d.db.Exec(insertStatement,
@@ -83,7 +83,7 @@ func (d *Datastore) SaveAppConfig(info *kolide.AppConfig) error {
 		info.SMTPPassword,
 		info.SMTPVerifySSLCerts,
 		info.SMTPEnableStartTLS,
-		info.SMTPDisabled,
+		info.SMTPEnabled,
 	)
 
 	return err

server/datastore/mysql/migrations/20161118193812_CreateTableAppConfigs.go

@@ -28,7 +28,7 @@ func Up_20161118193812(tx *sql.Tx) error {
 		"`smtp_password` VARCHAR(255) NOT NULL DEFAULT ''," +
 		"`smtp_verify_ssl_certs` TINYINT(1) NOT NULL DEFAULT TRUE, " +
 		"`smtp_enable_start_tls` TINYINT(1) NOT NULL DEFAULT TRUE, " +
-		"`smtp_disabled` TINYINT(1) NOT NULL DEFAULT FALSE, " +
+		"`smtp_enabled` TINYINT(1) NOT NULL DEFAULT FALSE, " +
 		"PRIMARY KEY (`id`)" +
 		") ENGINE=InnoDB DEFAULT CHARSET=utf8;"
 	if _, err := tx.Exec(sqlStatement); err != nil {

server/kolide/app.go

@@ -18,6 +18,15 @@ type AppConfigService interface {
 	ModifyAppConfig(ctx context.Context, p AppConfigPayload) (info *AppConfig, err error)
 }
 
+// SMTP settings names returned from API, these map to SMTPAuthType and
+// SMTPAuthMethod
+const (
+	AuthMethodNameCramMD5        = "authmethod_cram_md5"
+	AuthMethodNamePlain          = "authmethod_plain"
+	AuthTypeNameUserNamePassword = "authtype_username_password"
+	AuthTypeNameNone             = "authtype_none"
+)
+
 type SMTPAuthType int
 
 const (
@@ -25,6 +34,17 @@ const (
 	AuthTypeNone
 )
 
+func (a SMTPAuthType) String() string {
+	switch a {
+	case AuthTypeUserNamePassword:
+		return AuthTypeNameUserNamePassword
+	case AuthTypeNone:
+		return AuthTypeNameNone
+	default:
+		return ""
+	}
+}
+
 type SMTPAuthMethod int
 
 const (
@@ -32,6 +52,17 @@ const (
 	AuthMethodCramMD5
 )
 
+func (m SMTPAuthMethod) String() string {
+	switch m {
+	case AuthMethodPlain:
+		return AuthMethodNamePlain
+	case AuthMethodCramMD5:
+		return AuthMethodNameCramMD5
+	default:
+		return ""
+	}
+}
+
 // AppConfig holds configuration about the Kolide application.
 // AppConfig data can be managed by a Kolide API user.
 type AppConfig struct {
@@ -68,7 +99,7 @@ type AppConfig struct {
 	// SMTPEnableStartTLS detects of TLS is enabled on mail server and starts to use it (default true)
 	SMTPEnableStartTLS bool `db:"smtp_enable_start_tls"`
 	// SMTPDisabled if user sets this to TRUE emails will not be sent from the application
-	SMTPDisabled bool `db:"smtp_disabled"`
+	SMTPEnabled bool `db:"smtp_enabled"`
 	// SMTPLastError contains error information if email test fails, it's not persisted
 	SMTPLastError string
 }
@@ -97,7 +128,7 @@ type SMTPSettings struct {
 	// SMTPPort port SMTP server will use
 	SMTPPort uint `json:"port"`
 	// SMTPAuthenticationType type of authentication for SMTP
-	SMTPAuthenticationType SMTPAuthType `json:"authentication_type"`
+	SMTPAuthenticationType string `json:"authentication_type"`
 	// SMTPUserName must be provided if SMTPAuthenticationType is UserNamePassword
 	SMTPUserName string `json:"user_name"`
 	// SMTPPassword must be provided if SMTPAuthenticationType is UserNamePassword
@@ -105,38 +136,38 @@ type SMTPSettings struct {
 	// SMTPEnableSSLTLS whether to use SSL/TLS for SMTP
 	SMTPEnableTLS bool `json:"enable_ssl_tls"`
 	// SMTPAuthenticationMethod authentication method smtp server will use
-	SMTPAuthenticationMethod SMTPAuthMethod `json:"authentication_method"`
+	SMTPAuthenticationMethod string `json:"authentication_method"`
 
 	// SMTPDomain optional domain for SMTP
-	SMTPDomain string `json:"domain,omitempty"`
+	SMTPDomain string `json:"domain"`
 	// SMTPVerifySSLCerts defaults to true but can be turned off if self signed
 	// SSL certs are used by the SMTP server
 	SMTPVerifySSLCerts bool `json:"verify_ssl_certs"`
 	// SMTPEnableStartTLS detects of TLS is enabled on mail server and starts to use it (default true)
 	SMTPEnableStartTLS bool `json:"enable_start_tls"`
 	// SMTPDisabled if user sets this to TRUE emails will not be sent from the application
-	SMTPDisabled bool `json:"email_disabled"`
+	SMTPEnabled bool `json:"email_enabled"`
 }
 
 // AppConfigPayload contains request/response format of
 // the AppConfig endpoints.
 type AppConfigPayload struct {
-	OrgInfo        *OrgInfo        `json:"org_info,omitempty"`
-	ServerSettings *ServerSettings `json:"server_settings,omitempty"`
-	SMTPSettings   *SMTPSettings   `json:"smtp_settings,omitempty"`
+	OrgInfo        *OrgInfo        `json:"org_info"`
+	ServerSettings *ServerSettings `json:"server_settings"`
+	SMTPSettings   *SMTPSettings   `json:"smtp_settings"`
 	// SMTPTest is a flag that if set will cause the server to test email configuration
 	SMTPTest *bool `json:"smtp_test,omitempty"`
 }
 
 // OrgInfo contains general info about the organization using Kolide.
 type OrgInfo struct {
-	OrgName    *string `json:"org_name,omitempty"`
-	OrgLogoURL *string `json:"org_logo_url,omitempty"`
+	OrgName    *string `json:"org_name"`
+	OrgLogoURL *string `json:"org_logo_url"`
 }
 
 // ServerSettings contains general settings about the kolide App.
 type ServerSettings struct {
-	KolideServerURL *string `json:"kolide_server_url,omitempty"`
+	KolideServerURL *string `json:"kolide_server_url"`
 }
 
 type OrderDirection int

server/mail/mail.go

@@ -20,6 +20,23 @@ func NewDevService() kolide.MailService {
 type mailService struct{}
 type devMailService struct{}
 
+type sender interface {
+	sendMail(e kolide.Email, msg []byte) error
+}
+
+func Test(mailer kolide.MailService, e kolide.Email) error {
+	mailBody, err := getMessageBody(e)
+	if err != nil {
+		return err
+	}
+
+	if svc, ok := mailer.(sender); ok {
+		return svc.sendMail(e, mailBody)
+	}
+
+	return nil
+}
+
 const (
 	PortSSL = 465
 	PortTLS = 587
@@ -38,20 +55,29 @@ func getMessageBody(e kolide.Email) ([]byte, error) {
 }
 
 func (dm devMailService) SendEmail(e kolide.Email) error {
-	if !e.Config.SMTPDisabled {
-		if e.Config.SMTPConfigured {
-			msg, err := getMessageBody(e)
-			if err != nil {
-				return err
-			}
-			fmt.Printf(string(msg))
+	if !e.Config.SMTPEnabled {
+		return fmt.Errorf("email disabled")
+	}
+	if e.Config.SMTPConfigured {
+		msg, err := getMessageBody(e)
+		if err != nil {
+			return err
 		}
+		fmt.Printf(string(msg))
 	}
 	return nil
 }
 
+func (dm devMailService) sendMail(e kolide.Email, msg []byte) error {
+	fmt.Println(string(msg))
+	return nil
+}
+
 func (m mailService) SendEmail(e kolide.Email) error {
-	if !e.Config.SMTPDisabled && e.Config.SMTPConfigured {
+	if !e.Config.SMTPEnabled {
+		return fmt.Errorf("email disabled")
+	}
+	if e.Config.SMTPConfigured {
 		msg, err := getMessageBody(e)
 		if err != nil {
 			return err

server/mail/mail_test.go

@@ -35,6 +35,7 @@ var testFunctions = [...]func(*testing.T, kolide.MailService){
 	testSMTPPlainAuth,
 	testSMTPSkipVerify,
 	testSMTPNoAuth,
+	testMailTest,
 }
 
 func TestMail(t *testing.T) {
@@ -53,7 +54,7 @@ func testSMTPPlainAuth(t *testing.T, mailer kolide.MailService) {
 		To:      []string{"john@kolide.co"},
 		Config: &kolide.AppConfig{
 			SMTPConfigured:           true,
-			SMTPDisabled:             false,
+			SMTPEnabled:              true,
 			SMTPAuthenticationType:   kolide.AuthTypeUserNamePassword,
 			SMTPAuthenticationMethod: kolide.AuthMethodPlain,
 			SMTPUserName:             "bob",
@@ -80,7 +81,7 @@ func testSMTPSkipVerify(t *testing.T, mailer kolide.MailService) {
 		To:      []string{"john@kolide.co"},
 		Config: &kolide.AppConfig{
 			SMTPConfigured:           true,
-			SMTPDisabled:             false,
+			SMTPEnabled:              true,
 			SMTPAuthenticationType:   kolide.AuthTypeUserNamePassword,
 			SMTPAuthenticationMethod: kolide.AuthMethodPlain,
 			SMTPUserName:             "bob",
@@ -107,7 +108,7 @@ func testSMTPNoAuth(t *testing.T, mailer kolide.MailService) {
 		To:      []string{"bob@foo.com"},
 		Config: &kolide.AppConfig{
 			SMTPConfigured:         true,
-			SMTPDisabled:           false,
+			SMTPEnabled:            true,
 			SMTPAuthenticationType: kolide.AuthTypeNone,
 			SMTPEnableTLS:          true,
 			SMTPVerifySSLCerts:     true,
@@ -123,3 +124,26 @@ func testSMTPNoAuth(t *testing.T, mailer kolide.MailService) {
 	err := mailer.SendEmail(mail)
 	assert.Nil(t, err)
 }
+
+func testMailTest(t *testing.T, mailer kolide.MailService) {
+	mail := kolide.Email{
+		Subject: "test tester",
+		To:      []string{"bob@foo.com"},
+		Config: &kolide.AppConfig{
+			SMTPConfigured:         true,
+			SMTPEnabled:            true,
+			SMTPAuthenticationType: kolide.AuthTypeNone,
+			SMTPEnableTLS:          true,
+			SMTPVerifySSLCerts:     true,
+			SMTPPort:               1025,
+			SMTPServer:             "localhost",
+			SMTPSenderAddress:      "kolide@kolide.com",
+		},
+		Mailer: &kolide.SMTPTestMailer{
+			KolideServerURL: "https://localhost:8080",
+		},
+	}
+	err := Test(mailer, mail)
+	assert.Nil(t, err)
+
+}

server/service/endpoint_appconfig_test.go

@@ -54,6 +54,7 @@ func testModifyAppConfig(t *testing.T, r *testResource) {
 		SMTPEnableTLS:          true,
 		SMTPVerifySSLCerts:     true,
 		SMTPEnableStartTLS:     true,
+		SMTPEnabled:            true,
 	}
 	payload := fromAppConfig(config)
 	payload.SMTPTest = new(bool)
@@ -89,6 +90,7 @@ func testModifyAppConfigWithValidationFail(t *testing.T, r *testResource) {
 		SMTPEnableTLS:          true,
 		SMTPVerifySSLCerts:     true,
 		SMTPEnableStartTLS:     true,
+		SMTPEnabled:            true,
 	}
 	payload := fromAppConfig(config)
 	payload.SMTPTest = new(bool)
@@ -107,9 +109,9 @@ func testModifyAppConfigWithValidationFail(t *testing.T, r *testResource) {
 	err = json.NewDecoder(resp.Body).Decode(&validationErrors)
 	require.Nil(t, err)
 	assert.Equal(t, "Validation Failed", validationErrors.Message)
-	assert.Equal(t, 2, len(validationErrors.Errors))
+	require.Equal(t, 2, len(validationErrors.Errors))
 	assert.Equal(t, "kolide_server_url", validationErrors.Errors[0].Name)
-	assert.Equal(t, "url scheme must be https", validationErrors.Errors[0].Reason)
+	assert.Equal(t, "missing", validationErrors.Errors[0].Reason)
 	assert.Equal(t, "smtp_server", validationErrors.Errors[1].Name)
 	assert.Equal(t, "required argument", validationErrors.Errors[1].Reason)
 	// verify no changes are not saved if validation fails

server/service/service_appconfig.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/kolide/kolide-ose/server/contexts/viewer"
 	"github.com/kolide/kolide-ose/server/kolide"
+	"github.com/kolide/kolide-ose/server/mail"
 	"golang.org/x/net/context"
 )
 
@@ -30,7 +31,7 @@ func (svc service) ModifyAppConfig(ctx context.Context, p kolide.AppConfigPayloa
 		return nil, err
 	}
 	newConfig := fromPayload(p, *oldConfig)
-	if p.SMTPSettings != nil && !p.SMTPSettings.SMTPDisabled {
+	if p.SMTPSettings != nil && p.SMTPSettings.SMTPEnabled {
 		oldSettings := smtpSettingsFromAppConfig(oldConfig)
 		// anything changed?
 		if !reflect.DeepEqual(oldSettings, p.SMTPSettings) {
@@ -38,15 +39,17 @@ func (svc service) ModifyAppConfig(ctx context.Context, p kolide.AppConfigPayloa
 			if !ok {
 				return nil, errNoContext
 			}
+
 			testMail := kolide.Email{
 				Subject: "Hello from Kolide",
 				To:      []string{vc.User.Email},
 				Mailer: &kolide.SMTPTestMailer{
 					KolideServerURL: newConfig.KolideServerURL,
 				},
+				Config: newConfig,
 			}
-			// test mail set SMTPConfigured so we know if we can send mail
-			err = svc.mailService.SendEmail(testMail)
+
+			err = mail.Test(svc.mailService, testMail)
 			if err != nil {
 				// if the provided SMTP parameters don't work with the targeted SMTP server
 				// capture the error and return it to the front end so that GUI can
@@ -79,10 +82,18 @@ func fromPayload(p kolide.AppConfigPayload, config kolide.AppConfig) *kolide.App
 		config.KolideServerURL = *p.ServerSettings.KolideServerURL
 	}
 	if p.SMTPSettings != nil {
-		config.SMTPAuthenticationMethod = p.SMTPSettings.SMTPAuthenticationMethod
-		config.SMTPAuthenticationType = p.SMTPSettings.SMTPAuthenticationType
+		if p.SMTPSettings.SMTPAuthenticationMethod == kolide.AuthMethodNameCramMD5 {
+			config.SMTPAuthenticationMethod = kolide.AuthMethodCramMD5
+		} else {
+			config.SMTPAuthenticationMethod = kolide.AuthMethodPlain
+		}
+		if p.SMTPSettings.SMTPAuthenticationType == kolide.AuthTypeNameUserNamePassword {
+			config.SMTPAuthenticationType = kolide.AuthTypeUserNamePassword
+		} else {
+			config.SMTPAuthenticationType = kolide.AuthTypeNone
+		}
 		config.SMTPConfigured = p.SMTPSettings.SMTPConfigured
-		config.SMTPDisabled = p.SMTPSettings.SMTPDisabled
+		config.SMTPEnabled = p.SMTPSettings.SMTPEnabled
 		config.SMTPDomain = p.SMTPSettings.SMTPDomain
 		config.SMTPEnableStartTLS = p.SMTPSettings.SMTPEnableStartTLS
 		config.SMTPEnableTLS = p.SMTPSettings.SMTPEnableTLS
@@ -102,15 +113,15 @@ func smtpSettingsFromAppConfig(config *kolide.AppConfig) *kolide.SMTPSettings {
 		SMTPSenderAddress:        config.SMTPSenderAddress,
 		SMTPServer:               config.SMTPServer,
 		SMTPPort:                 config.SMTPPort,
-		SMTPAuthenticationType:   config.SMTPAuthenticationType,
+		SMTPAuthenticationType:   config.SMTPAuthenticationType.String(),
 		SMTPUserName:             config.SMTPUserName,
 		SMTPPassword:             config.SMTPPassword,
 		SMTPEnableTLS:            config.SMTPEnableTLS,
-		SMTPAuthenticationMethod: config.SMTPAuthenticationMethod,
+		SMTPAuthenticationMethod: config.SMTPAuthenticationMethod.String(),
 		SMTPDomain:               config.SMTPDomain,
 		SMTPVerifySSLCerts:       config.SMTPVerifySSLCerts,
 		SMTPEnableStartTLS:       config.SMTPEnableStartTLS,
-		SMTPDisabled:             config.SMTPDisabled,
+		SMTPEnabled:              config.SMTPEnabled,
 	}
 }
 

server/service/util_test.go

@@ -26,7 +26,7 @@ func createTestAppConfig(t *testing.T, ds kolide.Datastore) *kolide.AppConfig {
 		OrgLogoURL:             "https://tyrell.com/image.png",
 		KolideServerURL:        "https://kolide.tyrell.com",
 		SMTPConfigured:         true,
-		SMTPDisabled:           false,
+		SMTPEnabled:            true,
 		SMTPSenderAddress:      "kolide@tyrell.com",
 		SMTPServer:             "smtp.tyrell.com",
 		SMTPPort:               587,

server/service/validation_appconfig.go

@@ -7,34 +7,30 @@ import (
 
 func (mw validationMiddleware) ModifyAppConfig(ctx context.Context, p kolide.AppConfigPayload) (*kolide.AppConfig, error) {
 	invalid := &invalidArgumentError{}
-	if p.ServerSettings == nil {
-		invalid.Append("server_settings", "missing")
-	}
-	if p.ServerSettings != nil && p.ServerSettings.KolideServerURL == nil {
+
+	if p.ServerSettings.KolideServerURL == nil || *p.ServerSettings.KolideServerURL == "" {
 		invalid.Append("kolide_server_url", "missing")
 	}
-	if p.ServerSettings != nil && p.ServerSettings.KolideServerURL != nil {
+	if p.ServerSettings.KolideServerURL != nil && *p.ServerSettings.KolideServerURL != "" {
 		if err := validateKolideServerURL(*p.ServerSettings.KolideServerURL); err != nil {
 			invalid.Append("kolide_server_url", err.Error())
 		}
 	}
-	if p.SMTPSettings == nil {
-		invalid.Append("smtp_settings", "missing")
-	}
-	if p.SMTPSettings != nil && !p.SMTPSettings.SMTPDisabled {
+
+	if p.SMTPSettings.SMTPEnabled {
 		if p.SMTPSettings.SMTPSenderAddress != "" {
 			invalid.Append("smtp_sender_address", "required argument")
 		}
 		if p.SMTPSettings.SMTPServer == "" {
 			invalid.Append("smtp_server", "required argument")
 		}
-		if p.SMTPSettings.SMTPAuthenticationType != kolide.AuthTypeUserNamePassword &&
-			p.SMTPSettings.SMTPAuthenticationType != kolide.AuthTypeNone {
+		if p.SMTPSettings.SMTPAuthenticationType != kolide.AuthTypeNameUserNamePassword &&
+			p.SMTPSettings.SMTPAuthenticationType != kolide.AuthTypeNameNone {
 			invalid.Append("smtp_authentication_type", "invalid value")
 		}
-		if p.SMTPSettings.SMTPAuthenticationType == kolide.AuthTypeUserNamePassword {
-			if p.SMTPSettings.SMTPAuthenticationMethod != kolide.AuthMethodCramMD5 &&
-				p.SMTPSettings.SMTPAuthenticationMethod != kolide.AuthMethodPlain {
+		if p.SMTPSettings.SMTPAuthenticationType == kolide.AuthTypeNameUserNamePassword {
+			if p.SMTPSettings.SMTPAuthenticationMethod != kolide.AuthMethodNameCramMD5 &&
+				p.SMTPSettings.SMTPAuthenticationMethod != kolide.AuthMethodNamePlain {
 				invalid.Append("smtp_authentication_method", "invalid value")
 			}
 			if p.SMTPSettings.SMTPUserName == "" {