From 79b2249e69b6d877433728826ec89618a59650a2 Mon Sep 17 00:00:00 2001 From: Zachary Wasserman Date: Fri, 7 Sep 2018 15:37:35 -0700 Subject: [PATCH] Allow update of settings page without enabling SMTP (#1903) Fixes #1871 --- .../admin/AppConfigForm/AppConfigForm.jsx | 30 ++++++++++++------- .../forms/admin/AppConfigForm/validate.js | 11 ++----- .../admin/AppConfigForm/validate.tests.js | 4 ++- frontend/kolide/entities/config.tests.js | 1 - frontend/kolide/helpers.js | 3 +- frontend/kolide/helpers.tests.js | 2 +- .../Admin/AppSettingsPage/AppSettingsPage.jsx | 2 +- frontend/test/stubs.js | 2 -- server/kolide/app.go | 3 ++ server/service/endpoint_appconfig.go | 1 + server/service/endpoint_appconfig_test.go | 1 + server/service/service_appconfig.go | 11 +++++-- 12 files changed, 43 insertions(+), 28 deletions(-) diff --git a/frontend/components/forms/admin/AppConfigForm/AppConfigForm.jsx b/frontend/components/forms/admin/AppConfigForm/AppConfigForm.jsx index ae04df2359..76b7d47c29 100644 --- a/frontend/components/forms/admin/AppConfigForm/AppConfigForm.jsx +++ b/frontend/components/forms/admin/AppConfigForm/AppConfigForm.jsx @@ -24,7 +24,7 @@ const formFields = [ 'authentication_method', 'authentication_type', 'domain', 'enable_ssl_tls', 'enable_start_tls', 'kolide_server_url', 'org_logo_url', 'org_name', 'osquery_enroll_secret', 'password', 'port', 'sender_address', 'server', 'user_name', 'verify_ssl_certs', 'idp_name', 'entity_id', - 'issuer_uri', 'idp_image_url', 'metadata', 'metadata_url', 'enable_sso', + 'issuer_uri', 'idp_image_url', 'metadata', 'metadata_url', 'enable_sso', 'enable_smtp', ]; const Header = ({ showAdvancedOptions }) => { const CaratIcon = ; @@ -58,6 +58,7 @@ class AppConfigForm extends Component { metadata_url: formFieldInterface.isRequired, idp_name: formFieldInterface.isRequired, enable_sso: formFieldInterface.isRequired, + enable_smtp: formFieldInterface.isRequired, }).isRequired, handleSubmit: PropTypes.func.isRequired, smtpConfigured: PropTypes.bool.isRequired, @@ -185,6 +186,15 @@ class AppConfigForm extends Component {

SAML Single Sign On Options

+ +
+ + Enable Single Sign On + +
+
A URL that references the identity provider metadata.

-
- - Enable Single Sign On - -
-

SMTP Options STATUS: {smtpConfigured ? 'CONFIGURED' : 'NOT CONFIGURED'}

+
+ + Enable SMTP + +
+
- User SSL/TLS to connect (recommended) + Use SSL/TLS to connect (recommended)
diff --git a/frontend/components/forms/admin/AppConfigForm/validate.js b/frontend/components/forms/admin/AppConfigForm/validate.js index f9c2856eba..e0ec9dffd1 100644 --- a/frontend/components/forms/admin/AppConfigForm/validate.js +++ b/frontend/components/forms/admin/AppConfigForm/validate.js @@ -1,8 +1,4 @@ -import { size, some } from 'lodash'; - -import APP_CONSTANTS from 'app_constants'; - -const { APP_SETTINGS } = APP_CONSTANTS; +import { size } from 'lodash'; export default (formData) => { const errors = {}; @@ -10,6 +6,7 @@ export default (formData) => { authentication_type: authType, kolide_server_url: kolideServerUrl, org_name: orgName, + enable_smtp: enableSMTP, password: smtpPassword, sender_address: smtpSenderAddress, server: smtpServer, @@ -42,9 +39,7 @@ export default (formData) => { errors.org_name = 'Organization Name must be present'; } - if (some([smtpSenderAddress, smtpServer, smtpUserName]) || - (smtpPassword && smtpPassword !== APP_SETTINGS.FAKE_PASSWORD) || - (smtpServerPort !== APP_SETTINGS.DEFAULT_SMTP_PORT)) { + if (enableSMTP) { if (!smtpSenderAddress) { errors.sender_address = 'SMTP Sender Address must be present'; } diff --git a/frontend/components/forms/admin/AppConfigForm/validate.tests.js b/frontend/components/forms/admin/AppConfigForm/validate.tests.js index 61dc98ebd7..abedfa4967 100644 --- a/frontend/components/forms/admin/AppConfigForm/validate.tests.js +++ b/frontend/components/forms/admin/AppConfigForm/validate.tests.js @@ -8,6 +8,7 @@ describe('AppConfigForm - validations', () => { authentication_type: 'username_password', kolide_server_url: 'https://gnar.dog', sender_address: 'hi@gnar.dog', + enable_smtp: true, server: '192.168.99.100', port: '1025', user_name: 'gnardog', @@ -117,9 +118,10 @@ describe('AppConfigForm - validations', () => { }); }); - it('does not validate smtp config if only password and port are present and they are defaults', () => { + it('does not validate smtp config if smtp not enabled', () => { const formData = { ...validFormData, + enable_smtp: false, user_name: '', server: '', sender_address: '', diff --git a/frontend/kolide/entities/config.tests.js b/frontend/kolide/entities/config.tests.js index 6b240c3d6b..c9499186a5 100644 --- a/frontend/kolide/entities/config.tests.js +++ b/frontend/kolide/entities/config.tests.js @@ -44,7 +44,6 @@ describe('Kolide - API client (config)', () => { authentication_method: 'authmethod_plain', verify_ssl_certs: true, enable_start_tls: true, - email_enabled: false, }; const configData = helpers.formatConfigDataForServer(formData); const request = configMocks.update.valid(bearerToken, configData); diff --git a/frontend/kolide/helpers.js b/frontend/kolide/helpers.js index 084ed23d4e..5c05605531 100644 --- a/frontend/kolide/helpers.js +++ b/frontend/kolide/helpers.js @@ -76,8 +76,9 @@ export const formatConfigDataForServer = (config) => { const orgInfoAttrs = pick(config, ['org_logo_url', 'org_name']); const serverSettingsAttrs = pick(config, ['kolide_server_url', 'osquery_enroll_secret']); const smtpSettingsAttrs = pick(config, [ - 'authentication_method', 'authentication_type', 'domain', 'email_enabled', 'enable_ssl_tls', + 'authentication_method', 'authentication_type', 'domain', 'enable_ssl_tls', 'enable_start_tls', 'password', 'port', 'sender_address', 'server', 'user_name', 'verify_ssl_certs', + 'enable_smtp', ]); const ssoSettingsAttrs = pick(config, ['entity_id', 'issuer_uri', 'idp_image_url', 'metadata', 'metadata_url', 'idp_name', 'enable_sso', diff --git a/frontend/kolide/helpers.tests.js b/frontend/kolide/helpers.tests.js index 99148b4c2e..503b906741 100644 --- a/frontend/kolide/helpers.tests.js +++ b/frontend/kolide/helpers.tests.js @@ -25,6 +25,7 @@ describe('Kolide API - helpers', () => { kolide_server_url: '', configured: false, domain: '', + smtp_enabled: true, sender_address: '', server: '', port: 587, @@ -35,7 +36,6 @@ describe('Kolide API - helpers', () => { authentication_method: 'authmethod_plain', verify_ssl_certs: true, enable_start_tls: true, - email_enabled: false, }; it('splits config into categories for the server', () => { diff --git a/frontend/pages/Admin/AppSettingsPage/AppSettingsPage.jsx b/frontend/pages/Admin/AppSettingsPage/AppSettingsPage.jsx index 99cf6859d1..cee2c2aeda 100644 --- a/frontend/pages/Admin/AppSettingsPage/AppSettingsPage.jsx +++ b/frontend/pages/Admin/AppSettingsPage/AppSettingsPage.jsx @@ -62,7 +62,7 @@ class AppSettingsPage extends Component { return false; } - const formData = { ...appConfig }; + const formData = { ...appConfig, enable_smtp: smtpConfigured }; return (
diff --git a/frontend/test/stubs.js b/frontend/test/stubs.js index 009584bb45..ab7615ed8f 100644 --- a/frontend/test/stubs.js +++ b/frontend/test/stubs.js @@ -27,7 +27,6 @@ export const configStub = { authentication_method: 'authmethod_plain', verify_ssl_certs: true, enable_start_tls: true, - email_enabled: false, }, }; @@ -47,7 +46,6 @@ export const flatConfigStub = { authentication_method: 'authmethod_plain', verify_ssl_certs: true, enable_start_tls: true, - email_enabled: false, }; export const hostStub = { diff --git a/server/kolide/app.go b/server/kolide/app.go index b6d5541345..bb4e9c0ddc 100644 --- a/server/kolide/app.go +++ b/server/kolide/app.go @@ -165,6 +165,9 @@ type SSOSettingsPayload struct { // SMTPSettingsPayload is part of the AppConfigPayload which defines the wire representation // of the app config endpoints type SMTPSettingsPayload struct { + // SMTPEnabled indicates whether the user has selected that SMTP is + // enabled in the UI. + SMTPEnabled *bool `json:"enable_smtp"` // SMTPConfigured is a flag that indicates if smtp has been successfully // tested with the settings provided by an admin user. SMTPConfigured *bool `json:"configured"` diff --git a/server/service/endpoint_appconfig.go b/server/service/endpoint_appconfig.go index 0776f22e7f..25edfd1015 100644 --- a/server/service/endpoint_appconfig.go +++ b/server/service/endpoint_appconfig.go @@ -105,6 +105,7 @@ func smtpSettingsFromAppConfig(config *kolide.AppConfig) *kolide.SMTPSettingsPay authType := config.SMTPAuthenticationType.String() authMethod := config.SMTPAuthenticationMethod.String() return &kolide.SMTPSettingsPayload{ + SMTPEnabled: &config.SMTPConfigured, SMTPConfigured: &config.SMTPConfigured, SMTPSenderAddress: &config.SMTPSenderAddress, SMTPServer: &config.SMTPServer, diff --git a/server/service/endpoint_appconfig_test.go b/server/service/endpoint_appconfig_test.go index 64920ee46f..ed10612be4 100644 --- a/server/service/endpoint_appconfig_test.go +++ b/server/service/endpoint_appconfig_test.go @@ -61,6 +61,7 @@ func testModifyAppConfig(t *testing.T, r *testResource) { } payload := appConfigPayloadFromAppConfig(config) payload.SMTPTest = new(bool) + *payload.SMTPSettings.SMTPEnabled = true var buffer bytes.Buffer err := json.NewEncoder(&buffer).Encode(payload) diff --git a/server/service/service_appconfig.go b/server/service/service_appconfig.go index bda6c5fa23..f922861915 100644 --- a/server/service/service_appconfig.go +++ b/server/service/service_appconfig.go @@ -84,10 +84,15 @@ func (svc service) ModifyAppConfig(ctx context.Context, p kolide.AppConfigPayloa config := appConfigFromAppConfigPayload(p, *oldAppConfig) if p.SMTPSettings != nil { - if err = svc.SendTestEmail(ctx, config); err != nil { - return nil, err + enabled := p.SMTPSettings.SMTPEnabled + if (enabled == nil && oldAppConfig.SMTPConfigured) || (enabled != nil && *enabled) { + if err = svc.SendTestEmail(ctx, config); err != nil { + return nil, err + } + config.SMTPConfigured = true + } else if enabled != nil && !*enabled { + config.SMTPConfigured = false } - config.SMTPConfigured = true } if err := svc.ds.SaveAppConfig(config); err != nil {