From 61a657b0cdfebdd90ee4c2b2edcd980e123984a4 Mon Sep 17 00:00:00 2001 From: Sean Wood Date: Tue, 20 Feb 2024 08:38:09 +0000 Subject: [PATCH] Allow setting email app to use SSL instead of TLS (#3911) # What this PR does Adds flexibility of the method of encryption in the SMTP email app. Some email servers are configured to use port 465 (intrinsic TLS) which requires `EMAIL_USE_SSL` instead of `EMAIL_USE_TLS`. ## Which issue(s) this PR fixes Fixes https://github.com/grafana/oncall/issues/1044 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --------- Co-authored-by: Joey Orlando Co-authored-by: Joey Orlando --- CHANGELOG.md | 1 + engine/apps/base/models/live_setting.py | 2 + engine/apps/base/utils.py | 11 ++++ engine/apps/email/tasks.py | 1 + engine/settings/base.py | 1 + helm/oncall/templates/_env.tpl | 11 +++- .../integrations_deployment_test.yaml.snap | 2 + helm/oncall/tests/smtp_env_test.yaml | 53 +++++++++++++++++++ helm/oncall/values.yaml | 1 + 9 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 helm/oncall/tests/smtp_env_test.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a9cc6cf..9c19181c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Support prescribed labels ([#3848](https://github.com/grafana/oncall/pull/3848)) - Add status change trigger type to webhooks ([#3920](https://github.com/grafana/oncall/pull/3920)) +- Add `EMAIL_USE_SSL` environment setting by @WoodyWoodsta ([#3911](https://github.com/grafana/oncall/pull/3911)) ### Fixed diff --git a/engine/apps/base/models/live_setting.py b/engine/apps/base/models/live_setting.py index 48c3ac06..f6e8b7e5 100644 --- a/engine/apps/base/models/live_setting.py +++ b/engine/apps/base/models/live_setting.py @@ -39,6 +39,7 @@ class LiveSetting(models.Model): "EMAIL_HOST_USER", "EMAIL_HOST_PASSWORD", "EMAIL_USE_TLS", + "EMAIL_USE_SSL", "EMAIL_FROM_ADDRESS", "INBOUND_EMAIL_ESP", "INBOUND_EMAIL_DOMAIN", @@ -79,6 +80,7 @@ class LiveSetting(models.Model): "EMAIL_HOST_USER": "SMTP server user", "EMAIL_HOST_PASSWORD": "SMTP server password", "EMAIL_USE_TLS": "SMTP enable/disable TLS", + "EMAIL_USE_SSL": "SMTP enable/disable SSL. Should be used mutually exclusively with EMAIL_USE_TLS.", "EMAIL_FROM_ADDRESS": "Email address used to send emails. If not specified, EMAIL_HOST_USER will be used.", "INBOUND_EMAIL_DOMAIN": "Inbound email domain", "INBOUND_EMAIL_ESP": ( diff --git a/engine/apps/base/utils.py b/engine/apps/base/utils.py index ec15ac41..71263612 100644 --- a/engine/apps/base/utils.py +++ b/engine/apps/base/utils.py @@ -1,5 +1,6 @@ import json import re +import typing from urllib.parse import urlparse import phonenumbers @@ -40,6 +41,8 @@ class LiveSettingValidator: "TWILIO_API_KEY_SECRET", ) + EMAIL_SSL_TLS_ERROR_MSG = "Cannot set Email (SMTP) to use SSL and TLS at the same time" + def __init__(self, live_setting): self.live_setting = live_setting @@ -157,6 +160,14 @@ class LiveSettingValidator: _, err = CloudConnector.sync_with_cloud(grafana_oncall_token) return err + @classmethod + def _check_email_use_tls(cls, email_use_tls: bool) -> typing.Optional[str]: + return cls.EMAIL_SSL_TLS_ERROR_MSG if live_settings.EMAIL_USE_SSL is True and email_use_tls is True else None + + @classmethod + def _check_email_use_ssl(cls, email_use_ssl: bool) -> typing.Optional[str]: + return cls.EMAIL_SSL_TLS_ERROR_MSG if live_settings.EMAIL_USE_TLS is True and email_use_ssl is True else None + @staticmethod def _is_email_valid(email): return re.match(r"^[^@]+@[^@]+\.[^@]+$", email) diff --git a/engine/apps/email/tasks.py b/engine/apps/email/tasks.py index 1b0b7e12..685eb808 100644 --- a/engine/apps/email/tasks.py +++ b/engine/apps/email/tasks.py @@ -95,6 +95,7 @@ def notify_user_async(user_pk, alert_group_pk, notification_policy_pk): username=live_settings.EMAIL_HOST_USER, password=live_settings.EMAIL_HOST_PASSWORD, use_tls=live_settings.EMAIL_USE_TLS, + use_ssl=live_settings.EMAIL_USE_SSL, fail_silently=False, timeout=5, ) diff --git a/engine/settings/base.py b/engine/settings/base.py index f3a96eb6..3ad446c3 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -746,6 +746,7 @@ EMAIL_HOST_USER = os.getenv("EMAIL_HOST_USER") EMAIL_HOST_PASSWORD = os.getenv("EMAIL_HOST_PASSWORD") EMAIL_PORT = getenv_integer("EMAIL_PORT", 587) EMAIL_USE_TLS = getenv_boolean("EMAIL_USE_TLS", True) +EMAIL_USE_SSL = getenv_boolean("EMAIL_USE_SSL", False) EMAIL_FROM_ADDRESS = os.getenv("EMAIL_FROM_ADDRESS") EMAIL_NOTIFICATIONS_LIMIT = getenv_integer("EMAIL_NOTIFICATIONS_LIMIT", 200) diff --git a/helm/oncall/templates/_env.tpl b/helm/oncall/templates/_env.tpl index 68502eab..6821d2a9 100644 --- a/helm/oncall/templates/_env.tpl +++ b/helm/oncall/templates/_env.tpl @@ -601,6 +601,13 @@ when broker.type != rabbitmq, we do not need to include rabbitmq environment var {{- end }} {{- define "snippet.oncall.smtp.env" -}} + {{- $smtpTLS:=.Values.oncall.smtp.tls | default true | toString | title | quote }} + {{- $smtpSSL:=.Values.oncall.smtp.ssl | default false | toString | title | quote }} + {{- if eq $smtpTLS "\"True\"" }} + {{- if eq $smtpSSL "\"True\"" }} + {{- fail "cannot set Email (SMTP) to use SSL and TLS at the same time" }} + {{- end }} + {{- end }} - name: FEATURE_EMAIL_INTEGRATION_ENABLED value: {{ .Values.oncall.smtp.enabled | toString | title | quote }} {{- if .Values.oncall.smtp.enabled }} @@ -617,7 +624,9 @@ when broker.type != rabbitmq, we do not need to include rabbitmq environment var key: smtp-password optional: true - name: EMAIL_USE_TLS - value: {{ .Values.oncall.smtp.tls | default true | toString | title | quote }} + value: {{ $smtpTLS }} +- name: EMAIL_USE_SSL + value: {{ $smtpSSL }} - name: EMAIL_FROM_ADDRESS value: {{ .Values.oncall.smtp.fromEmail | quote }} - name: EMAIL_NOTIFICATIONS_LIMIT diff --git a/helm/oncall/tests/__snapshot__/integrations_deployment_test.yaml.snap b/helm/oncall/tests/__snapshot__/integrations_deployment_test.yaml.snap index b2b31638..89efb16b 100644 --- a/helm/oncall/tests/__snapshot__/integrations_deployment_test.yaml.snap +++ b/helm/oncall/tests/__snapshot__/integrations_deployment_test.yaml.snap @@ -49,6 +49,8 @@ detached_integrations.enabled=true -> should create integrations deployment: optional: true - name: EMAIL_USE_TLS value: "True" + - name: EMAIL_USE_SSL + value: "False" - name: EMAIL_FROM_ADDRESS value: null - name: EMAIL_NOTIFICATIONS_LIMIT diff --git a/helm/oncall/tests/smtp_env_test.yaml b/helm/oncall/tests/smtp_env_test.yaml new file mode 100644 index 00000000..e2e0cf9d --- /dev/null +++ b/helm/oncall/tests/smtp_env_test.yaml @@ -0,0 +1,53 @@ +suite: test SMTP/Email envs for deployments +templates: + - engine/deployment.yaml +release: + name: oncall +tests: + - it: smtp.ssl -> mutually exclusive with smtp.tls default + set: + oncall.smtp: + ssl: "True" + asserts: + - failedTemplate: + errorMessage: cannot set Email (SMTP) to use SSL and TLS at the same time + - it: smtp.ssl -> mutually exclusive with smtp.tls set + set: + oncall.smtp: + tls: "True" + ssl: "True" + asserts: + - failedTemplate: + errorMessage: cannot set Email (SMTP) to use SSL and TLS at the same time + - it: smtp.ssl -> mutually exclusive with smtp.tls disabled + set: + oncall.smtp: + tls: "False" + ssl: "True" + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: EMAIL_USE_TLS + value: "False" + - contains: + path: spec.template.spec.containers[0].env + content: + name: EMAIL_USE_SSL + value: "True" + - it: smtp.tls -> mutually exclusive with smtp.ssl disabled + set: + oncall.smtp: + tls: "True" + ssl: "False" + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: EMAIL_USE_TLS + value: "True" + - contains: + path: spec.template.spec.containers[0].env + content: + name: EMAIL_USE_SSL + value: "False" diff --git a/helm/oncall/values.yaml b/helm/oncall/values.yaml index 957d7fb4..55a4b79f 100644 --- a/helm/oncall/values.yaml +++ b/helm/oncall/values.yaml @@ -341,6 +341,7 @@ oncall: username: ~ password: ~ tls: ~ + ssl: ~ fromEmail: ~ exporter: enabled: false