coder · SasSwart · Oct 14, 2024 · Oct 3, 2024 · Oct 5, 2024 · Oct 5, 2024
diff --git a/.github/workflows/typos.toml b/.github/workflows/typos.toml
@@ -41,4 +41,6 @@ extend-exclude = [
 	"tailnet/testdata/**",
 	"site/src/pages/SetupPage/countries.tsx",
 	"provisioner/terraform/testdata/**",
+	# notifications' golden files confuse the detector because of quoted-printable encoding
+	"coderd/notifications/testdata/**"
 ]
diff --git a/coderd/database/migrations/000263_consistent_notification_initiator_naming.down.sql b/coderd/database/migrations/000263_consistent_notification_initiator_naming.down.sql
@@ -0,0 +1,55 @@
+-- UserAccountCreated
+UPDATE notification_templates
+SET
+	body_template = E'Hi {{.UserName}},\n\n' ||
+					E'New user account **{{.Labels.created_account_name}}** has been created.\n\n' ||
+					-- Mention the real name of the user who created the account:
+					E'This new user account was created for **{{.Labels.created_account_user_name}}** by **{{.Labels.account_creator}}**.'
+WHERE
+	id = '4e19c0ac-94e1-4532-9515-d1801aa283b2';
+
+-- UserAccountDeleted
+UPDATE notification_templates
+SET
+	body_template = E'Hi {{.UserName}},\n\n' ||
+					E'User account **{{.Labels.deleted_account_name}}** has been deleted.\n\n' ||
+					-- Mention the real name of the user who deleted the account:
+					E'The deleted account belonged to **{{.Labels.deleted_account_user_name}}** and was deleted by **{{.Labels.account_deleter_user_name}}**.'
+WHERE
+	id = 'f44d9314-ad03-4bc8-95d0-5cad491da6b6';
+
+-- UserAccountSuspended
+UPDATE notification_templates
+SET
+	body_template = E'Hi {{.UserName}},\n\n' || -- Add a \n
+					E'User account **{{.Labels.suspended_account_name}}** has been suspended.\n\n' ||
+					-- Mention the real name of the user who suspended the account:
+  					E'The newly suspended account belongs to **{{.Labels.suspended_account_user_name}}** and was suspended by **{{.Labels.account_suspender_user_name}}**.'
+WHERE
+	id = 'b02ddd82-4733-4d02-a2d7-c36f3598997d';
+
+-- YourAccountSuspended
+UPDATE notification_templates
+SET
+	body_template = E'Hi {{.UserName}},\n\n' || -- Add a \n
+					E'Your account **{{.Labels.suspended_account_name}}** has been suspended by **{{.Labels.account_suspender_user_name}}**.'
+WHERE
+	id = '6a2f0609-9b69-4d36-a989-9f5925b6cbff';
+
+
+-- UserAccountActivated
+UPDATE notification_templates
+SET
+	body_template = E'Hi {{.UserName}},\n\n' || -- Add a \n
+					E'User account **{{.Labels.activated_account_name}}** has been activated.\n\n' ||
+					E'The newly activated account belongs to **{{.Labels.activated_account_user_name}}** and was activated by **{{.Labels.account_activator_user_name}}**.'
+WHERE
+	id = '9f5af851-8408-4e73-a7a1-c6502ba46689';
+
+-- YourAccountActivated
+UPDATE notification_templates
+SET
+	body_template = E'Hi {{.UserName}},\n\n' || -- Add a \n
+					E'Your account **{{.Labels.activated_account_name}}** has been activated by **{{.Labels.account_activator_user_name}}**.'
+WHERE
+	id = '1a6a6bea-ee0a-43e2-9e7c-eabdb53730e4';
diff --git a/coderd/database/migrations/000263_consistent_notification_initiator_naming.up.sql b/coderd/database/migrations/000263_consistent_notification_initiator_naming.up.sql
@@ -0,0 +1,57 @@
+-- UserAccountCreated
+UPDATE notification_templates
+SET
+	body_template = E'Hi {{.UserName}},\n\n' ||
+					E'New user account **{{.Labels.created_account_name}}** has been created.\n\n' ||
+					-- Use the conventional initiator label:
+					E'This new user account was created for **{{.Labels.created_account_user_name}}** by **{{.Labels.initiator}}**.'
+WHERE
+	id = '4e19c0ac-94e1-4532-9515-d1801aa283b2';
+
+-- UserAccountDeleted
+UPDATE notification_templates
+SET
+	body_template = E'Hi {{.UserName}},\n\n' ||
+					E'User account **{{.Labels.deleted_account_name}}** has been deleted.\n\n' ||
+					-- Use the conventional initiator label:
+					E'The deleted account belonged to **{{.Labels.deleted_account_user_name}}** and was deleted by **{{.Labels.initiator}}**.'
+WHERE
+	id = 'f44d9314-ad03-4bc8-95d0-5cad491da6b6';
+
+-- UserAccountSuspended
+UPDATE notification_templates
+SET
+	body_template = E'Hi {{.UserName}},\n\n' || -- Add a \n
+					E'User account **{{.Labels.suspended_account_name}}** has been suspended.\n\n' ||
+					-- Use the conventional initiator label:
+  					E'The newly suspended account belongs to **{{.Labels.suspended_account_user_name}}** and was suspended by **{{.Labels.initiator}}**.'
+WHERE
+	id = 'b02ddd82-4733-4d02-a2d7-c36f3598997d';
+
+-- YourAccountSuspended
+UPDATE notification_templates
+SET
+	body_template = E'Hi {{.UserName}},\n\n' || -- Add a \n
+					-- Use the conventional initiator label:
+					E'Your account **{{.Labels.suspended_account_name}}** has been suspended by **{{.Labels.initiator}}**.'
+WHERE
+	id = '6a2f0609-9b69-4d36-a989-9f5925b6cbff';
+
+-- UserAccountActivated
+UPDATE notification_templates
+SET
+	body_template = E'Hi {{.UserName}},\n\n' || -- Add a \n
+					E'User account **{{.Labels.activated_account_name}}** has been activated.\n\n' ||
+					-- Use the conventional initiator label:
+					E'The newly activated account belongs to **{{.Labels.activated_account_user_name}}** and was activated by **{{.Labels.initiator}}**.'
+WHERE
+	id = '9f5af851-8408-4e73-a7a1-c6502ba46689';
+
+-- YourAccountActivated
+UPDATE notification_templates
+SET
+	body_template = E'Hi {{.UserName}},\n\n' || -- Add a \n
+					-- Use the conventional initiator label:
+					E'Your account **{{.Labels.activated_account_name}}** has been activated by **{{.Labels.initiator}}**.'
+WHERE
+	id = '1a6a6bea-ee0a-43e2-9e7c-eabdb53730e4';
diff --git a/coderd/notifications/dispatch/smtp_test.go b/coderd/notifications/dispatch/smtp_test.go
@@ -2,11 +2,8 @@ package dispatch_test
 
 import (
 	"bytes"
-	"crypto/tls"
-	_ "embed"
 	"fmt"
 	"log"
-	"net"
 	"sync"
 	"testing"
 
@@ -22,6 +19,7 @@ import (
 	"github.com/coder/serpent"
 
 	"github.com/coder/coder/v2/coderd/notifications/dispatch"
+	"github.com/coder/coder/v2/coderd/notifications/dispatch/smtptest"
 	"github.com/coder/coder/v2/coderd/notifications/types"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/testutil"
@@ -47,9 +45,9 @@ func TestSMTP(t *testing.T) {
 		subject = "This is the subject"
 		body    = "This is the body"
 
-		caFile   = "fixtures/ca.crt"
-		certFile = "fixtures/server.crt"
-		keyFile  = "fixtures/server.key"
+		caFile   = "smtptest/fixtures/ca.crt"
+		certFile = "smtptest/fixtures/server.crt"
+		keyFile  = "smtptest/fixtures/server.key"
 	)
 
 	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true, IgnoredErrorIs: []error{}}).Leveled(slog.LevelDebug)
@@ -125,7 +123,7 @@ func TestSMTP(t *testing.T) {
 
 				Auth: codersdk.NotificationsEmailAuthConfig{
 					Username:     username,
-					PasswordFile: "fixtures/password.txt",
+					PasswordFile: "smtptest/fixtures/password.txt",
 				},
 			},
 			toAddrs:          []string{to},
@@ -341,14 +339,14 @@ func TestSMTP(t *testing.T) {
 			cfg: codersdk.NotificationsEmailConfig{
 				TLS: codersdk.NotificationsEmailTLSConfig{
 					CAFile:   caFile,
-					CertFile: "fixtures/nope.cert",
+					CertFile: "smtptest/fixtures/nope.cert",
 					KeyFile:  keyFile,
 				},
 			},
 			// not using full error message here since it differs on *nix and Windows:
 			// *nix: no such file or directory
 			// Windows: The system cannot find the file specified.
-			expectedErr: "open fixtures/nope.cert:",
+			expectedErr: "open smtptest/fixtures/nope.cert:",
 			retryable:   true,
 		},
 		{
@@ -358,13 +356,13 @@ func TestSMTP(t *testing.T) {
 				TLS: codersdk.NotificationsEmailTLSConfig{
 					CAFile:   caFile,
 					CertFile: certFile,
-					KeyFile:  "fixtures/nope.key",
+					KeyFile:  "smtptest/fixtures/nope.key",
 				},
 			},
 			// not using full error message here since it differs on *nix and Windows:
 			// *nix: no such file or directory
 			// Windows: The system cannot find the file specified.
-			expectedErr: "open fixtures/nope.key:",
+			expectedErr: "open smtptest/fixtures/nope.key:",
 			retryable:   true,
 		},
 		/**
@@ -417,7 +415,7 @@ func TestSMTP(t *testing.T) {
 
 			tc.cfg.ForceTLS = serpent.Bool(tc.useTLS)
 
-			backend := NewBackend(Config{
+			backend := smtptest.NewBackend(smtptest.Config{
 				AuthMechanisms: tc.authMechs,
 
 				AcceptedIdentity: tc.cfg.Auth.Identity.String(),
@@ -428,7 +426,7 @@ func TestSMTP(t *testing.T) {
 			})
 
 			// Create a mock SMTP server which conditionally listens for plain or TLS connections.
-			srv, listen, err := createMockSMTPServer(backend, tc.useTLS)
+			srv, listen, err := smtptest.CreateMockSMTPServer(backend, tc.useTLS)
 			require.NoError(t, err)
 			t.Cleanup(func() {
 				// We expect that the server has already been closed in the test
@@ -460,7 +458,7 @@ func TestSMTP(t *testing.T) {
 
 			// Wait for the server to become pingable.
 			require.Eventually(t, func() bool {
-				cl, err := pingClient(listen, tc.useTLS, tc.cfg.TLS.StartTLS.Value())
+				cl, err := smtptest.PingClient(listen, tc.useTLS, tc.cfg.TLS.StartTLS.Value())
 				if err != nil {
 					t.Logf("smtp not yet dialable: %s", err)
 					return false
@@ -522,19 +520,3 @@ func TestSMTP(t *testing.T) {
 		})
 	}
 }
-
-func pingClient(listen net.Listener, useTLS bool, startTLS bool) (*smtp.Client, error) {
-	tlsCfg := &tls.Config{
-		// nolint:gosec // It's a test.
-		InsecureSkipVerify: true,
-	}
-
-	switch {
-	case useTLS:
-		return smtp.DialTLS(listen.Addr().String(), tlsCfg)
-	case startTLS:
-		return smtp.DialStartTLS(listen.Addr().String(), tlsCfg)
-	default:
-		return smtp.Dial(listen.Addr().String())
-	}
-}
diff --git a/...d/notifications/dispatch/fixtures/ca.conf → ...ations/dispatch/smtptest/fixtures/ca.conf b/...d/notifications/dispatch/fixtures/ca.conf → ...ations/dispatch/smtptest/fixtures/ca.conf
diff --git a/...rd/notifications/dispatch/fixtures/ca.crt → ...cations/dispatch/smtptest/fixtures/ca.crt b/...rd/notifications/dispatch/fixtures/ca.crt → ...cations/dispatch/smtptest/fixtures/ca.crt
diff --git a/...rd/notifications/dispatch/fixtures/ca.key → ...cations/dispatch/smtptest/fixtures/ca.key b/...rd/notifications/dispatch/fixtures/ca.key → ...cations/dispatch/smtptest/fixtures/ca.key
diff --git a/...rd/notifications/dispatch/fixtures/ca.srl → ...cations/dispatch/smtptest/fixtures/ca.srl b/...rd/notifications/dispatch/fixtures/ca.srl → ...cations/dispatch/smtptest/fixtures/ca.srl
diff --git a/...tifications/dispatch/fixtures/generate.sh → ...ns/dispatch/smtptest/fixtures/generate.sh b/...tifications/dispatch/fixtures/generate.sh → ...ns/dispatch/smtptest/fixtures/generate.sh
diff --git a/...ifications/dispatch/fixtures/password.txt → ...s/dispatch/smtptest/fixtures/password.txt b/...ifications/dispatch/fixtures/password.txt → ...s/dispatch/smtptest/fixtures/password.txt
diff --git a/...tifications/dispatch/fixtures/server.conf → ...ns/dispatch/smtptest/fixtures/server.conf b/...tifications/dispatch/fixtures/server.conf → ...ns/dispatch/smtptest/fixtures/server.conf
diff --git a/...otifications/dispatch/fixtures/server.crt → ...ons/dispatch/smtptest/fixtures/server.crt b/...otifications/dispatch/fixtures/server.crt → ...ons/dispatch/smtptest/fixtures/server.crt
diff --git a/...otifications/dispatch/fixtures/server.csr → ...ons/dispatch/smtptest/fixtures/server.csr b/...otifications/dispatch/fixtures/server.csr → ...ons/dispatch/smtptest/fixtures/server.csr
diff --git a/...otifications/dispatch/fixtures/server.key → ...ons/dispatch/smtptest/fixtures/server.key b/...otifications/dispatch/fixtures/server.key → ...ons/dispatch/smtptest/fixtures/server.key
diff --git a/...tifications/dispatch/fixtures/v3_ext.conf → ...ns/dispatch/smtptest/fixtures/v3_ext.conf b/...tifications/dispatch/fixtures/v3_ext.conf → ...ns/dispatch/smtptest/fixtures/v3_ext.conf
diff --git a/.../notifications/dispatch/smtp_util_test.go → ...notifications/dispatch/smtptest/server.go b/.../notifications/dispatch/smtp_util_test.go → ...notifications/dispatch/smtptest/server.go
@@ -1,4 +1,4 @@
-package dispatch_test
+package smtptest
 
 import (
 	"crypto/tls"
@@ -162,7 +162,7 @@ func (*Session) Reset() {}
 func (*Session) Logout() error { return nil }
 
 // nolint:revive // Yes, useTLS is a control flag.
-func createMockSMTPServer(be *Backend, useTLS bool) (*smtp.Server, net.Listener, error) {
+func CreateMockSMTPServer(be *Backend, useTLS bool) (*smtp.Server, net.Listener, error) {
 	// nolint:gosec
 	tlsCfg := &tls.Config{
 		GetCertificate: readCert,
@@ -203,3 +203,19 @@ func readCert(_ *tls.ClientHelloInfo) (*tls.Certificate, error) {
 
 	return &crt, nil
 }
+
+func PingClient(listen net.Listener, useTLS bool, startTLS bool) (*smtp.Client, error) {
+	tlsCfg := &tls.Config{
+		// nolint:gosec // It's a test.
+		InsecureSkipVerify: true,
+	}
+
+	switch {
+	case useTLS:
+		return smtp.DialTLS(listen.Addr().String(), tlsCfg)
+	case startTLS:
+		return smtp.DialStartTLS(listen.Addr().String(), tlsCfg)
+	default:
+		return smtp.Dial(listen.Addr().String())
+	}
+}