diff --git a/backend/src/app/email.clj b/backend/src/app/email.clj index bae1d22075..3f3216e511 100644 --- a/backend/src/app/email.clj +++ b/backend/src/app/email.clj @@ -305,7 +305,14 @@ second `params` provides the email data and optionally `::factory` and `::reuse-conn`. When `::reuse-conn` is true the submission reuses the caller's existing `::db/conn` (from cfg) instead of - acquiring a fresh one from the pool." + acquiring a fresh one from the pool. + + NOTE: this renders the email before submitting, so the full subject + and body are stored as job params. Prefer `send-template` for + templated emails: it stores only the template id plus the raw data + and renders inside the worker. `send` is kept for raw (non + templated) emails and for draining jobs submitted in the old + format." [cfg {:keys [::reuse-conn ::factory] :as params}] (when (and reuse-conn (nil? (::db/conn cfg))) (ex/raise :type :validation @@ -593,6 +600,110 @@ :id ::comment-notification :schema schema:comment-notification)) +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; TEMPLATED SEND (render inside the worker) +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +;; Registry of template id (the same string stored as `:id` on the +;; rendered email) to factory. The queue only carries the id plus the +;; raw template data; the worker resolves the factory here and +;; renders. `::template` params on `send-template` accept the +;; qualified keyword (e.g. `::invite-to-team`); only its name is +;; stored, which is plain-JSON safe (namespaced keyword values would +;; lose their namespace when encoded). +(def templates + {"feedback" user-feedback + "register" register + "password-recovery" password-recovery + "password-changed" password-changed + "change-email" change-email + "invite-to-team" invite-to-team + "invite-to-organization" invite-to-organization + "organization-setup-sso" organization-setup-sso + "renewal-notice" renewal-notice + "join-team" join-team + "request-file-access" request-file-access + "request-file-access-yourpenpot" request-file-access-yourpenpot + "request-file-access-yourpenpot-view" request-file-access-yourpenpot-view + "request-team-access" request-team-access + "comment-mention" comment-mention + "comment-thread" comment-thread + "comment-notification" comment-notification}) + +(def ^:private schema:templated-params + [:map {:title "Templated Email Params"} + [:template (into [:enum] (sort (keys templates)))] + [:to [:or ::sm/email [::sm/vec ::sm/email]]] + [:reply-to {:optional true} ::sm/email] + [:from {:optional true} ::sm/email] + [:lang {:optional true} ::sm/text] + [:priority {:optional true} [:enum :high :low]] + [:extra-data {:optional true} ::sm/text] + [:team-id {:optional true} ::sm/uuid] + [:file-id {:optional true} ::sm/uuid] + [:page-id {:optional true} ::sm/uuid] + [:profile {:optional true} + [:map + [:id ::sm/uuid]]] + [:attachments {:optional true} + [:map-of :string :string]]]) + +(defn send-template + "Schedule a templated email to be sent asynchronously using the + unified jobs machinery. Unlike `send`, this does NOT render the + email: it stores only the template id plus the raw template data as + `:send-templated-mail` job params, and the worker renders and sends + the email. The `::template` param is the qualified template keyword + (e.g. `::invite-to-team`); `::reuse-conn` behaves like in `send`." + [cfg {:keys [::template ::reuse-conn] :as params}] + (when (and reuse-conn (nil? (::db/conn cfg))) + (ex/raise :type :validation + :code :missing-connection + :hint "reuse-conn requires the caller cfg to carry ::db/conn (call inside a transaction)")) + (let [template-id (when (some? template) (name template))] + (when (nil? template-id) + (ex/raise :type :validation + :code :missing-template + :hint "template is mandatory for templated emails")) + (when-not (contains? templates template-id) + (ex/raise :type :validation + :code :unknown-email-template + :hint (str/ffmt "unknown email template: %", template-id))) + (let [recipe (-> params + (dissoc ::reuse-conn ::template) + ;; Present-but-nil extras (e.g. `:file-id` when there is + ;; no file) would fail the job schema, while the old + ;; inline render simply ignored them. + (d/without-nils))] + (jobs/submit (cond-> (dissoc cfg ::db/conn) + reuse-conn (assoc ::db/conn (::db/conn cfg))) + {::jobs/name :send-templated-mail + ::jobs/delay 0 + ::jobs/max-retries 4 + ::jobs/priority 200 + ::jobs/params (cond-> (assoc recipe :template template-id) + ;; `:public-uri` arrives as a URI record + ;; from config, which plain JSON cannot + ;; roundtrip; the templates only need its + ;; string form. + (some? (:public-uri recipe)) + (update :public-uri str))})))) + +(defmethod ig/init-key ::templated-job-def + [_ {sendmail ::sendmail}] + {::jobs/name :send-templated-mail + ::jobs/schema schema:templated-params + ::jobs/handler (fn [context params] + (let [template-id (:template params) + factory (get templates template-id)] + (when-not factory + (ex/raise :type :internal + :code :unknown-email-template + :template-id template-id)) + (sendmail context (factory (dissoc params :template))))) + ::jobs/decoder (sm/decoder schema:templated-params sm/json-transformer) + ::jobs/validator (sm/validator schema:templated-params)}) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; BOUNCE/COMPLAINS HELPERS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index 7476f9bfb2..2c4e7fbad1 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -622,24 +622,25 @@ ::setup/shared-keys (ig/ref ::setup/shared-keys)} ::jobs/defs - {:sendmail (ig/ref ::email/job-def) - :delete-object (ig/ref :app.tasks.delete-object/job-def) - :demo-purge (ig/ref :app.tasks.demo-purge/job-def) - :run-webhook (ig/ref ::webhooks/run-webhook-job-def) - :process-webhook-event (ig/ref ::webhooks/process-webhook-event-job-def) - :file-gc (ig/ref :app.tasks.file-gc/job-def) - :offload-file-data (ig/ref :app.tasks.offload-file-data/job-def) - :objects-gc (ig/ref :app.tasks.objects-gc/job-def) - :export-binfile (ig/ref :app.tasks.export-binfile/job-def) - :import-binfile (ig/ref :app.tasks.import-binfile/job-def) - :storage-gc-deleted (ig/ref ::sto.gc-deleted/job-def) - :storage-gc-touched (ig/ref ::sto.gc-touched/job-def) - :storage-pending-gc (ig/ref ::sto.pending-gc/job-def) - :jobs-gc (ig/ref :app.jobs.gc/job-def) - :telemetry (ig/ref :app.tasks.telemetry/job-def) - :session-gc (ig/ref ::session/job-def) - :file-gc-scheduler (ig/ref :app.tasks.file-gc-scheduler/job-def) - :audit-log-consumer-nexus (ig/ref :app.loggers.audit.consumer-task/nexus-job-def) + {:sendmail (ig/ref ::email/job-def) + :send-templated-mail (ig/ref ::email/templated-job-def) + :delete-object (ig/ref :app.tasks.delete-object/job-def) + :demo-purge (ig/ref :app.tasks.demo-purge/job-def) + :run-webhook (ig/ref ::webhooks/run-webhook-job-def) + :process-webhook-event (ig/ref ::webhooks/process-webhook-event-job-def) + :file-gc (ig/ref :app.tasks.file-gc/job-def) + :offload-file-data (ig/ref :app.tasks.offload-file-data/job-def) + :objects-gc (ig/ref :app.tasks.objects-gc/job-def) + :export-binfile (ig/ref :app.tasks.export-binfile/job-def) + :import-binfile (ig/ref :app.tasks.import-binfile/job-def) + :storage-gc-deleted (ig/ref ::sto.gc-deleted/job-def) + :storage-gc-touched (ig/ref ::sto.gc-touched/job-def) + :storage-pending-gc (ig/ref ::sto.pending-gc/job-def) + :jobs-gc (ig/ref :app.jobs.gc/job-def) + :telemetry (ig/ref :app.tasks.telemetry/job-def) + :session-gc (ig/ref ::session/job-def) + :file-gc-scheduler (ig/ref :app.tasks.file-gc-scheduler/job-def) + :audit-log-consumer-nexus (ig/ref :app.loggers.audit.consumer-task/nexus-job-def) :audit-log-consumer-admin-console (ig/ref :app.loggers.audit.consumer-task/admin-console-job-def)} ::jobs/request-pool @@ -650,6 +651,10 @@ {::email/sendmail (ig/ref ::email/sendmail) ::mtx/metrics (ig/ref ::mtx/metrics)} + :app.email/templated-job-def + {::email/sendmail (ig/ref ::email/sendmail) + ::mtx/metrics (ig/ref ::mtx/metrics)} + :app.tasks.delete-object/job-def {::db/pool (ig/ref ::db/pool) ::sto/storage (ig/ref ::sto/storage) diff --git a/backend/src/app/rpc/commands/auth.clj b/backend/src/app/rpc/commands/auth.clj index 549d24a3ba..ad96391c5b 100644 --- a/backend/src/app/rpc/commands/auth.clj +++ b/backend/src/app/rpc/commands/auth.clj @@ -202,11 +202,11 @@ (update-password conn))] (when profile (login-lockout/clear-attempts! cfg (:id profile)) - (eml/send cfg {::eml/reuse-conn true - ::eml/factory eml/password-changed - :public-uri (cf/get :public-uri) - :to (:email profile) - :name (:fullname profile)}))) + (eml/send-template cfg {::eml/reuse-conn true + ::eml/template ::eml/password-changed + :public-uri (cf/get :public-uri) + :to (:email profile) + :name (:fullname profile)}))) nil)) @@ -485,13 +485,13 @@ {:iss :profile-identity :profile-id (:id profile) :exp (ct/in-future {:days 30})})] - (eml/send cfg {::eml/reuse-conn true - ::eml/factory eml/register - :public-uri (cf/get :public-uri) - :to (:email profile) - :name (:fullname profile) - :token vtoken - :extra-data ptoken})))) + (eml/send-template cfg {::eml/reuse-conn true + ::eml/template ::eml/register + :public-uri (cf/get :public-uri) + :to (:email profile) + :name (:fullname profile) + :token vtoken + :extra-data ptoken})))) (defn register-profile [{:keys [::db/conn] :as cfg} {:keys [token] :as params}] @@ -654,13 +654,13 @@ {:iss :profile-identity :profile-id (:id profile) :exp (ct/in-future {:days 30})})] - (eml/send cfg {::eml/reuse-conn true - ::eml/factory eml/password-recovery - :public-uri (cf/get :public-uri) - :to (:email profile) - :token (:token profile) - :name (:fullname profile) - :extra-data ptoken}) + (eml/send-template cfg {::eml/reuse-conn true + ::eml/template ::eml/password-recovery + :public-uri (cf/get :public-uri) + :to (:email profile) + :token (:token profile) + :name (:fullname profile) + :extra-data ptoken}) nil))] (let [profile (->> (profile/clean-email email) diff --git a/backend/src/app/rpc/commands/comments.clj b/backend/src/app/rpc/commands/comments.clj index fbd460cbb2..555291901b 100644 --- a/backend/src/app/rpc/commands/comments.clj +++ b/backend/src/app/rpc/commands/comments.clj @@ -113,10 +113,10 @@ (doseq [mention comment-mentions] (let [{:keys [fullname email props]} (get team-users mention)] (when (mention-email? props) - (eml/send + (eml/send-template cfg {::eml/reuse-conn true - ::eml/factory eml/comment-mention + ::eml/template ::eml/comment-mention :public-uri (cf/get :public-uri) :to email :name fullname @@ -129,10 +129,10 @@ (doseq [mention thread-mentions] (let [{:keys [fullname email props]} (get team-users mention)] (when (mention-email? props) - (eml/send + (eml/send-template cfg {::eml/reuse-conn true - ::eml/factory eml/comment-thread + ::eml/template ::eml/comment-thread :public-uri (cf/get :public-uri) :to email :name fullname @@ -145,10 +145,10 @@ (doseq [user-id notificate-users-ids] (let [{:keys [id fullname email props]} (get team-users user-id)] (when (notification-email? id (:owner-id thread) props) - (eml/send + (eml/send-template cfg {::eml/reuse-conn true - ::eml/factory eml/comment-notification + ::eml/template ::eml/comment-notification :public-uri (cf/get :public-uri) :to email :name fullname diff --git a/backend/src/app/rpc/commands/feedback.clj b/backend/src/app/rpc/commands/feedback.clj index 7ff4076da7..f97e1c01ab 100644 --- a/backend/src/app/rpc/commands/feedback.clj +++ b/backend/src/app/rpc/commands/feedback.clj @@ -57,16 +57,16 @@ (d/without-nils {"error-report.txt" (:error-report params)})] - (eml/send cfg {::eml/reuse-conn true - ::eml/factory eml/user-feedback - :to destination - :reply-to (:email profile) - :email (:email profile) - :attachments attachments + (eml/send-template cfg {::eml/reuse-conn true + ::eml/template ::eml/user-feedback + :to destination + :reply-to (:email profile) + :email (:email profile) + :attachments attachments - :feedback-subject (:subject params) - :feedback-type (:type params "not-specified") - :feedback-content (:content params) - :feedback-error-href (:error-href params) - :profile profile}) + :feedback-subject (:subject params) + :feedback-type (:type params "not-specified") + :feedback-content (:content params) + :feedback-error-href (:error-href params) + :profile (select-keys profile [:id :fullname :email])}) nil)) diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index cefd382b72..ca3f4a1c09 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -225,11 +225,11 @@ (update-profile-password! cfg (assoc profile :password password)) - (eml/send cfg {::eml/reuse-conn true - ::eml/factory eml/password-changed - :public-uri (cf/get :public-uri) - :to (:email profile) - :name (:fullname profile)}) + (eml/send-template cfg {::eml/reuse-conn true + ::eml/template ::eml/password-changed + :public-uri (cf/get :public-uri) + :to (:email profile) + :name (:fullname profile)}) (->> (rph/get-request params) (session/get-session) @@ -461,14 +461,14 @@ :email (:email profile) :hint "looks like the email has spam complaint reports")) - (eml/send cfg {::eml/reuse-conn true - ::eml/factory eml/change-email - :public-uri (cf/get :public-uri) - :to (:email profile) - :name (:fullname profile) - :pending-email email - :token token - :extra-data ptoken}) + (eml/send-template cfg {::eml/reuse-conn true + ::eml/template ::eml/change-email + :public-uri (cf/get :public-uri) + :to (:email profile) + :name (:fullname profile) + :pending-email email + :token token + :extra-data ptoken}) nil)) ;; --- MUTATION: Update Profile Props diff --git a/backend/src/app/rpc/commands/teams_invitations.clj b/backend/src/app/rpc/commands/teams_invitations.clj index 6cabb06ec5..7c1edc3e06 100644 --- a/backend/src/app/rpc/commands/teams_invitations.clj +++ b/backend/src/app/rpc/commands/teams_invitations.clj @@ -291,24 +291,24 @@ (not recent?)) (if organization (when (contains? cf/flags :admin-console) - (eml/send cfg {::eml/reuse-conn true - ::eml/factory eml/invite-to-organization - :public-uri (cf/get :public-uri) - :to email - :invited-by (:fullname profile) - :user-name (:fullname member) - :organization organization - :token itoken - :extra-data ptoken})) - (eml/send cfg {::eml/reuse-conn true - ::eml/factory eml/invite-to-team - :public-uri (cf/get :public-uri) - :to email - :invited-by (:fullname profile) - :team (:name team) - :organization (:organization team) - :token itoken - :extra-data ptoken}))) + (eml/send-template cfg {::eml/reuse-conn true + ::eml/template ::eml/invite-to-organization + :public-uri (cf/get :public-uri) + :to email + :invited-by (:fullname profile) + :user-name (:fullname member) + :organization organization + :token itoken + :extra-data ptoken})) + (eml/send-template cfg {::eml/reuse-conn true + ::eml/template ::eml/invite-to-team + :public-uri (cf/get :public-uri) + :to email + :invited-by (:fullname profile) + :team (:name team) + :organization (:organization team) + :token itoken + :extra-data ptoken}))) itoken))))) @@ -355,13 +355,13 @@ (db/delete! conn :team-invitation {:team-id team-id :email-to (:email member)}) - (eml/send cfg {::eml/reuse-conn true - ::eml/factory eml/join-team - :public-uri (cf/get :public-uri) - :to (:email member) - :invited-by (:fullname profile) - :team (:name team) - :team-id (:id team)}))) + (eml/send-template cfg {::eml/reuse-conn true + ::eml/template ::eml/join-team + :public-uri (cf/get :public-uri) + :to (:email member) + :invited-by (:fullname profile) + :team (:name team) + :team-id (:id team)}))) (def ^:private sql:valid-access-request-profiles "SELECT p.id, p.email, p.is_blocked @@ -809,30 +809,30 @@ (teams/check-email-spam conn (:email team-owner) true) (let [request (upsert-team-access-request cfg team-id profile-id) - factory (cond - (and (some? file) (:is-default team) is-viewer) - eml/request-file-access-yourpenpot-view + template (cond + (and (some? file) (:is-default team) is-viewer) + ::eml/request-file-access-yourpenpot-view - (and (some? file) (:is-default team)) - eml/request-file-access-yourpenpot + (and (some? file) (:is-default team)) + ::eml/request-file-access-yourpenpot - (some? file) - eml/request-file-access + (some? file) + ::eml/request-file-access - :else - eml/request-team-access)] + :else + ::eml/request-team-access)] - (eml/send cfg {::eml/reuse-conn true - ::eml/factory factory - :public-uri (cf/get :public-uri) - :to (:email team-owner) - :requested-by (:fullname requester) - :requested-by-email (:email requester) - :team-name (:name team) - :team-id team-id - :file-name (:name file) - :file-id file-id - :page-id (:page-id file)}) + (eml/send-template cfg {::eml/reuse-conn true + ::eml/template template + :public-uri (cf/get :public-uri) + :to (:email team-owner) + :requested-by (:fullname requester) + :requested-by-email (:email requester) + :team-name (:name team) + :team-id team-id + :file-name (:name file) + :file-id file-id + :page-id (:page-id file)}) (with-meta {:request request} {::audit/props {:request 1}})))) diff --git a/backend/src/app/rpc/management/nitrate.clj b/backend/src/app/rpc/management/nitrate.clj index ac18a6c10d..e44fa5f9f7 100644 --- a/backend/src/app/rpc/management/nitrate.clj +++ b/backend/src/app/rpc/management/nitrate.clj @@ -800,14 +800,14 @@ RETURNING id, deleted_at;") (:fullname (profile/get-profile cfg profile-id)) (str/trim user-name))] (db/tx-run! cfg (fn [tx-cfg] - (eml/send tx-cfg {::eml/reuse-conn true - ::eml/factory eml/renewal-notice - :public-uri (cf/get :public-uri) - :to user-email - :user-name user-name - :renewal-date renewal-date - :estimated-amount amount-str - :organizations organizations})))) + (eml/send-template tx-cfg {::eml/reuse-conn true + ::eml/template ::eml/renewal-notice + :public-uri (cf/get :public-uri) + :to user-email + :user-name user-name + :renewal-date renewal-date + :estimated-amount amount-str + :organizations organizations})))) nil) ;; API: exists-organization-team-invitations-for-non-members / diff --git a/backend/src/app/rpc/nitrate/emails_helper.clj b/backend/src/app/rpc/nitrate/emails_helper.clj index 926ef68c29..af6687a180 100644 --- a/backend/src/app/rpc/nitrate/emails_helper.clj +++ b/backend/src/app/rpc/nitrate/emails_helper.clj @@ -55,11 +55,11 @@ [{:keys [::db/conn] :as cfg} organization-name {:keys [email profile]}] (when (or (nil? profile) (eml/allow-send-emails? conn profile)) - (eml/send cfg {::eml/reuse-conn true - ::eml/factory eml/organization-setup-sso - :public-uri (cf/get :public-uri) - :to email - :organization-name organization-name}))) + (eml/send-template cfg {::eml/reuse-conn true + ::eml/template ::eml/organization-setup-sso + :public-uri (cf/get :public-uri) + :to email + :organization-name organization-name}))) (defn- get-organization-sso-notify-recipients "Unique organization members and pending organization/team invitees for SSO activation emails." diff --git a/backend/test/backend_tests/email_sending_test.clj b/backend/test/backend_tests/email_sending_test.clj index b49f675a56..052747a4be 100644 --- a/backend/test/backend_tests/email_sending_test.clj +++ b/backend/test/backend_tests/email_sending_test.clj @@ -142,3 +142,120 @@ (assoc email ::emails/reuse-conn true))))] (t/testing "the email job is submitted through the caller conn" (t/is (= "sendmail" (:name (th/db-get :job {:id job-id}))))))) + +(defn- templated-job-def + [sendmail] + (ig/init-key ::emails/templated-job-def + {::emails/sendmail sendmail})) + +(defn- submit-templated + [job-def params] + (db/tx-run! {::db/pool th/*pool*} + (fn [cfg] + (emails/send-template (assoc cfg + ::jobs/defs {:send-templated-mail job-def} + ::mtx/metrics (get th/*system* :app.metrics/metrics)) + (assoc params ::emails/reuse-conn true))))) + +(def ^:private invite-to-team-recipe + {:to "invitee@example.com" + :public-uri (cf/get :public-uri) + :invited-by "Owner User" + :team "Design Team" + :token "test-token" + :organization {:name "Acme Inc" + :sso-active false}}) + +(t/deftest templated-registry-covers-all-factories + (t/is (= #{"feedback" "register" "password-recovery" "password-changed" + "change-email" "invite-to-team" "invite-to-organization" + "organization-setup-sso" "renewal-notice" "join-team" + "request-file-access" "request-file-access-yourpenpot" + "request-file-access-yourpenpot-view" "request-team-access" + "comment-mention" "comment-thread" "comment-notification"} + (set (keys emails/templates))))) + +(t/deftest send-template-validation + (t/testing "missing template fails fast" + (t/is (thrown-with-msg? clojure.lang.ExceptionInfo #"template" + (emails/send-template {} {:to "a@example.com"})))) + (t/testing "unknown template fails fast" + (t/is (thrown-with-msg? clojure.lang.ExceptionInfo #"unknown email template" + (emails/send-template {} {:to "a@example.com" + ::emails/template ::nope})))) + (t/testing "reuse-conn without a caller connection fails fast" + (t/is (thrown-with-msg? clojure.lang.ExceptionInfo #"reuse-conn" + (emails/send-template {} {:to "a@example.com" + ::emails/reuse-conn true + ::emails/template ::emails/register}))))) + +(t/deftest send-template-stores-recipe-not-rendered-mail + (let [job-def (templated-job-def (fn [_] nil)) + job-id (submit-templated job-def (assoc invite-to-team-recipe + ::emails/template ::emails/invite-to-team)) + row (th/db-get :job {:id job-id}) + params (jobs/decode-params job-def (:params row)) + rendered (emails/render emails/invite-to-team invite-to-team-recipe)] + (t/testing "the job lands on the new queue" + (t/is (= "send-templated-mail" (:name row)))) + (t/testing "stored params carry the template id and no rendered content" + (t/is (= "invite-to-team" (:template params))) + (t/is (not (contains? params :subject))) + (t/is (not (contains? params :body)))) + (t/testing "stored params are smaller than the rendered html alone" + (let [stored-size (count (str (:params row))) + rendered-html (count (get-in rendered [:body "text/html"]))] + (t/is (< stored-size rendered-html)))) + (t/testing "the worker renders the same mail from the stored recipe" + (t/is (= rendered ((get emails/templates (:template params)) + (dissoc params :template))))))) + +(t/deftest send-template-roundtrips-uuids-and-profile + (let [sent (atom nil) + job-def (templated-job-def (fn [_ email] (reset! sent email))) + team-id (random-uuid) + recipe {:to "member@example.com" + :public-uri (cf/get :public-uri) + :invited-by "Owner User" + :team "Design Team" + :team-id team-id} + job-id (submit-templated job-def (assoc recipe + ::emails/template ::emails/join-team)) + params (jobs/decode-params job-def (:params (th/db-get :job {:id job-id})))] + (t/testing "uuids survive the plain-JSON queue roundtrip typed" + (t/is (uuid? (:team-id params))) + (t/is (= team-id (:team-id params)))) + (t/testing "the handler renders and delivers the mail" + ((::jobs/handler job-def) nil params) + (t/is (= (:subject (emails/render emails/join-team recipe)) + (:subject @sent))) + (t/is (= "member@example.com" (:to @sent))))) + (let [sent (atom nil) + job-def (templated-job-def (fn [_ email] (reset! sent email))) + recipe {:to "dest@example.com" + :reply-to "user@example.com" + :email "user@example.com" + :attachments {"error-report.txt" "trace"} + :feedback-subject "subj" + :feedback-type "bug" + :feedback-content "content" + :profile {:id (random-uuid) :fullname "User" :email "user@example.com"}} + job-id (submit-templated job-def (assoc recipe + ::emails/template ::emails/feedback)) + params (jobs/decode-params job-def (:params (th/db-get :job {:id job-id})))] + (t/testing "nested profile id survives the roundtrip typed" + (t/is (uuid? (get-in params [:profile :id])))) + (t/testing "the handler renders the feedback mail" + ((::jobs/handler job-def) nil params) + (t/is (= (:subject (emails/render emails/user-feedback recipe)) + (:subject @sent)))))) + +(t/deftest send-templated-mail-handler-rejects-unknown-template + (let [job-def (templated-job-def (fn [_] nil)) + code (try + ((::jobs/handler job-def) nil {:template "nope" + :to "a@example.com"}) + nil + (catch clojure.lang.ExceptionInfo e + (-> e ex-data :code)))] + (t/is (= :unknown-email-template code)))) diff --git a/backend/test/backend_tests/rpc_feedback_test.clj b/backend/test/backend_tests/rpc_feedback_test.clj index c9147e6649..63bff4c143 100644 --- a/backend/test/backend_tests/rpc_feedback_test.clj +++ b/backend/test/backend_tests/rpc_feedback_test.clj @@ -51,9 +51,9 @@ cf/config (assoc cf/config :user-feedback-destination "fb@example.com")] (let [profile (th/create-profile* 1 {}) captured (atom nil)] - (with-redefs [eml/send (fn [cfg params] - (reset! captured {:cfg cfg :params params}) - nil)] + (with-redefs [eml/send-template (fn [cfg params] + (reset! captured {:cfg cfg :params params}) + nil)] (let [{:keys [error]} (th/command! {::th/type :send-user-feedback ::rpc/profile-id (:id profile) :subject "s" diff --git a/backend/test/backend_tests/rpc_management_nitrate_test.clj b/backend/test/backend_tests/rpc_management_nitrate_test.clj index be36f84680..9124669b27 100644 --- a/backend/test/backend_tests/rpc_management_nitrate_test.clj +++ b/backend/test/backend_tests/rpc_management_nitrate_test.clj @@ -54,7 +54,7 @@ (t/deftest create-and-update-organization-invitations-audit-props (let [owner-id-ref (atom nil)] - (with-mocks [email-mock {:target 'app.email/send :return nil} + (with-mocks [email-mock {:target 'app.email/send-template :return nil} audit-mock {:target 'app.loggers.audit/submit :return nil} nitrate-mock {:target 'app.nitrate/call :return (fn [_cfg method params] @@ -103,7 +103,7 @@ (t/deftest invite-to-organization-rejects-non-owner (let [organization-summary-ref (atom nil)] - (with-mocks [email-mock {:target 'app.email/send :return nil} + (with-mocks [email-mock {:target 'app.email/send-template :return nil} nitrate-mock {:target 'app.nitrate/call :return (fn [_cfg method _params] (when (= method :get-organization-summary) @@ -131,7 +131,7 @@ (t/is (not (:called? @email-mock))))))) (t/deftest invite-to-organization-rejects-unknown-organization - (with-mocks [email-mock {:target 'app.email/send :return nil} + (with-mocks [email-mock {:target 'app.email/send-template :return nil} nitrate-mock {:target 'app.nitrate/call :return nil}] (let [profile (th/create-profile* 105 {:is-active true}) organization-id (uuid/random) @@ -150,7 +150,7 @@ (t/deftest invite-to-organization-uses-authoritative-branding (let [organization-summary-ref (atom nil)] - (with-mocks [email-mock {:target 'app.email/send :return nil} + (with-mocks [email-mock {:target 'app.email/send-template :return nil} nitrate-mock {:target 'app.nitrate/call :return (fn [_cfg method _params] (when (= method :get-organization-summary) @@ -1835,7 +1835,7 @@ nil)) nil))} ;; --- Email mock: capture sent emails --- - email-mock {:target 'app.email/send + email-mock {:target 'app.email/send-template :return (fn [_cfg params] (swap! sent conj params) nil)}] ;; --- Setup: create profiles, team, organization-summary --- @@ -1905,7 +1905,7 @@ emails)) (doseq [email-params @sent] (t/is (= organization-name (:organization-name email-params))) - (t/is (= eml/organization-setup-sso (::eml/factory email-params))))))))) + (t/is (= ::eml/organization-setup-sso (::eml/template email-params))))))))) (t/deftest notify-organization-sso-change-skips-email-when-not-active (let [sent (atom []) @@ -1913,7 +1913,7 @@ :organization-id (uuid/random) :updated-props false :announce-activation false}] - (with-redefs [eml/send (fn [_cfg params] (swap! sent conj params))] + (with-redefs [eml/send-template (fn [_cfg params] (swap! sent conj params))] (th/management-command! params)) (t/is (empty? @sent)))) @@ -2115,7 +2115,7 @@ (t/deftest send-renewal-email-falls-back-to-profile-fullname-when-name-is-nil ;; `nil` user-name means "no override": the RPC must look up the ;; account owner's real name instead of sending a blank greeting. - (with-mocks [email-mock {:target 'app.email/send :return nil} + (with-mocks [email-mock {:target 'app.email/send-template :return nil} nitrate-mock {:target 'app.nitrate/call :return nil}] (let [profile (th/create-profile* 1 {:is-active true :fullname "Nitrate User"}) out (th/management-command! (send-renewal-email-params profile nil))] @@ -2126,7 +2126,7 @@ (t/deftest send-renewal-email-keeps-explicit-empty-name ;; An explicit "" means the caller deliberately wants no name shown ;; and must not be replaced by the profile's fullname. - (with-mocks [email-mock {:target 'app.email/send :return nil} + (with-mocks [email-mock {:target 'app.email/send-template :return nil} nitrate-mock {:target 'app.nitrate/call :return nil}] (let [profile (th/create-profile* 1 {:is-active true :fullname "Nitrate User"}) out (th/management-command! (send-renewal-email-params profile ""))] @@ -2137,7 +2137,7 @@ (t/deftest send-renewal-email-treats-blank-name-as-empty ;; A blank name is trimmed and follows the same path as "": no name ;; is shown and the profile's fullname is not used. - (with-mocks [email-mock {:target 'app.email/send :return nil} + (with-mocks [email-mock {:target 'app.email/send-template :return nil} nitrate-mock {:target 'app.nitrate/call :return nil}] (let [profile (th/create-profile* 1 {:is-active true :fullname "Nitrate User"}) out (th/management-command! (send-renewal-email-params profile " "))] diff --git a/backend/test/backend_tests/rpc_nitrate_test.clj b/backend/test/backend_tests/rpc_nitrate_test.clj index b50d5dd8ac..3ab91be93e 100644 --- a/backend/test/backend_tests/rpc_nitrate_test.clj +++ b/backend/test/backend_tests/rpc_nitrate_test.clj @@ -1197,7 +1197,7 @@ :sso-active? true :set-team-params set-team-params}) teams/initialize-user-in-organization (fn [& _] nil) - eml/send (fn [_cfg params] (swap! sent conj params))] + eml/send-template (fn [_cfg params] (swap! sent conj params))] (let [out (th/command! {::th/type :add-team-to-organization ::rpc/profile-id (:id owner) :team-id (:id team) @@ -1214,7 +1214,7 @@ (t/is (= #{"member302@example.com"} emails)) (doseq [email-params @sent] (t/is (= organization-name (:organization-name email-params))) - (t/is (= eml/organization-setup-sso (::eml/factory email-params))))))) + (t/is (= ::eml/organization-setup-sso (::eml/template email-params))))))) (t/deftest add-team-to-organization-deletes-external-invitations-for-unregistered-users (let [owner (th/create-profile* 305 {:is-active true @@ -1328,7 +1328,7 @@ :team-id (:id team) :sso-active? false}) teams/initialize-user-in-organization (fn [& _] nil) - eml/send (fn [_cfg params] (swap! sent conj params))] + eml/send-template (fn [_cfg params] (swap! sent conj params))] (let [out (th/command! {::th/type :add-team-to-organization ::rpc/profile-id (:id owner) :team-id (:id team) diff --git a/backend/test/backend_tests/rpc_profile_test.clj b/backend/test/backend_tests/rpc_profile_test.clj index b0cda30dd8..da519eaf86 100644 --- a/backend/test/backend_tests/rpc_profile_test.clj +++ b/backend/test/backend_tests/rpc_profile_test.clj @@ -509,7 +509,7 @@ (t/is (= "mtma" (:penpot/mtm-campaign props))))))) (t/deftest prepare-register-and-register-profile-2 - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [current-token (atom nil)] ;; PREPARE REGISTER (let [data {::th/type :prepare-register-profile @@ -563,7 +563,7 @@ (t/is (= 1 (:call-count @mock)))))))) (t/deftest prepare-register-and-register-profile-3 - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [current-token (atom nil)] ;; PREPARE REGISTER (let [data {::th/type :prepare-register-profile @@ -611,7 +611,7 @@ ;; When disable-email-verification is set and the profile is inactive ;; (e.g. created before the flag was set), re-registering should be ;; rejected with :email-already-exists. - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (with-redefs [app.config/flags #{:registration :login-with-password}] (let [current-token (atom nil)] ;; PREPARE REGISTER: first attempt (no profile exists yet) @@ -699,7 +699,7 @@ (t/deftest prepare-register-with-invitation-and-disabled-registration (with-redefs [app.config/flags #{:login-with-password :email-verification}] - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [email "invited@example.com" fixture (create-invitation-fixture email (ct/in-future "48h")) itoken (create-test-invitation-token fixture email) @@ -840,7 +840,7 @@ ;; verify-email mail with the invitation token EMBEDDED into the ;; verify-email JWE (so the team-invitation flow can resume after ;; the user clicks the email link). - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [itoken (tokens/generate th/*system* {:iss :team-invitation :exp (ct/in-future "48h") @@ -1181,7 +1181,7 @@ "prepare-register must not embed existing profile id of an anonymous caller")))) (t/deftest register-profile-with-invitation-must-not-take-over-existing-account - (with-mocks [_mock {:target 'app.email/send :return nil}] + (with-mocks [_mock {:target 'app.email/send-template :return nil}] (let [;; Victim profile exists but is not yet active (e.g. registered ;; but has not clicked the verification link). This is the ;; remaining attack surface after fix 1b: `prepare-register` @@ -1287,7 +1287,7 @@ (t/is (true? (:is-active reloaded)))))) (t/deftest email-change-request - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [profile (th/create-profile* 1) pool (:app.db/pool th/*system*) data {::th/type :request-email-change @@ -1327,7 +1327,7 @@ (t/deftest email-change-request-without-smtp - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (with-redefs [app.config/flags #{}] (let [profile (th/create-profile* 1) pool (:app.db/pool th/*system*) @@ -1343,7 +1343,7 @@ (t/deftest request-profile-recovery - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [profile1 (th/create-profile* 1 {:is-active false}) profile2 (th/create-profile* 2 {:is-active true}) pool (:app.db/pool th/*system*) @@ -1401,7 +1401,7 @@ (t/deftest update-profile-password - (with-mocks [_ {:target 'app.email/send :return nil}] + (with-mocks [_ {:target 'app.email/send-template :return nil}] (let [profile (th/create-profile* 1) data {::th/type :update-profile-password ::rpc/profile-id (:id profile) @@ -1413,7 +1413,7 @@ (t/deftest update-profile-password-bad-old-password - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [profile (th/create-profile* 1) data {::th/type :update-profile-password ::rpc/profile-id (:id profile) @@ -1700,7 +1700,7 @@ (t/is (th/ex-of-code? (:error out) :weak-password)))) (t/deftest update-profile-password-sends-notification - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [profile (th/create-profile* 1) data {::th/type :update-profile-password ::rpc/profile-id (:id profile) @@ -1710,14 +1710,14 @@ (t/is (nil? (:error out))) (t/is (nil? (:result out))) (t/is (= 1 (:call-count @mock))) - (let [[_cfg {:keys [::eml/factory :to :name]}] (first (:call-args-list @mock))] - (t/is (= eml/password-changed factory)) + (let [[_cfg {:keys [::eml/template :to :name]}] (first (:call-args-list @mock))] + (t/is (= ::eml/password-changed template)) (t/is (= (:email profile) to)) (t/is (= (:fullname profile) name)))))) (t/deftest update-profile-password-sends-notification-for-first-password - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [profile (th/create-profile* 1 {:password "!"}) data {::th/type :update-profile-password ::rpc/profile-id (:id profile) @@ -1726,14 +1726,14 @@ (t/is (nil? (:error out))) (t/is (nil? (:result out))) (t/is (= 1 (:call-count @mock))) - (let [[_cfg {:keys [::eml/factory :to :name]}] (first (:call-args-list @mock))] - (t/is (= eml/password-changed factory)) + (let [[_cfg {:keys [::eml/template :to :name]}] (first (:call-args-list @mock))] + (t/is (= ::eml/password-changed template)) (t/is (= (:email profile) to)) (t/is (= (:fullname profile) name)))))) (t/deftest recover-profile-sends-notification - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [profile (th/create-profile* 1) token (tokens/generate th/*system* {:iss :password-recovery @@ -1745,8 +1745,8 @@ out (th/command! data)] (t/is (nil? (:error out))) (t/is (= 1 (:call-count @mock))) - (let [[_cfg {:keys [::eml/factory :to :name]}] (first (:call-args-list @mock))] - (t/is (= eml/password-changed factory)) + (let [[_cfg {:keys [::eml/template :to :name]}] (first (:call-args-list @mock))] + (t/is (= ::eml/password-changed template)) (t/is (= (:email profile) to)) (t/is (= (:fullname profile) name)))))) diff --git a/backend/test/backend_tests/rpc_team_test.clj b/backend/test/backend_tests/rpc_team_test.clj index ed0a12fb5b..1feaabb654 100644 --- a/backend/test/backend_tests/rpc_team_test.clj +++ b/backend/test/backend_tests/rpc_team_test.clj @@ -31,7 +31,7 @@ (t/use-fixtures :each th/database-reset) (t/deftest create-team-invitations - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [profile1 (th/create-profile* 1 {:is-active true}) profile2 (th/create-profile* 2 {:is-active true}) profile3 (th/create-profile* 3 {:is-active true :is-muted true}) @@ -110,7 +110,7 @@ (t/is (= :member-is-muted (:code edata)))))))) (t/deftest create-and-update-team-invitations-include-organization-props - (with-mocks [email-mock {:target 'app.email/send :return nil} + (with-mocks [email-mock {:target 'app.email/send-template :return nil} audit-mock {:target 'app.loggers.audit/submit :return nil}] (let [owner (th/create-profile* 101 {:is-active true}) invitee (th/create-profile* 102 {:is-active true}) @@ -171,7 +171,7 @@ (t/is (false? (get-in create-plain [:props :invitee-already-organization-member]))))))) (t/deftest create-team-invitations-blacklisted-domain - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [profile1 (th/create-profile* 1 {:is-active true}) team (th/create-team* 1 {:profile-id (:id profile1)}) data {::th/type :create-team-invitations @@ -211,7 +211,7 @@ (t/is (= 1 (:call-count @mock)))))))) (t/deftest create-team-invitations-with-request-access - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [profile1 (th/create-profile* 1 {:is-active true}) requester (th/create-profile* 2 {:is-active true :email "requester@example.com"}) @@ -248,7 +248,7 @@ (t/deftest create-team-invitations-with-request-access-2 - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [profile1 (th/create-profile* 1 {:is-active true}) requester (th/create-profile* 2 {:is-active true :email "requester@example.com"}) @@ -311,7 +311,7 @@ (t/deftest invitation-tokens - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [profile1 (th/create-profile* 1 {:is-active true}) profile2 (th/create-profile* 2 {:is-active true}) @@ -623,7 +623,7 @@ (t/is (not-any? #(= "accept-organization-invitation" (:name %)) events)))))) (t/deftest create-team-invitations-with-email-verification-disabled - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [profile1 (th/create-profile* 1 {:is-active true}) profile2 (th/create-profile* 2 {:is-active true}) profile3 (th/create-profile* 3 {:is-active true :is-muted true}) @@ -965,7 +965,7 @@ (t/is (= (:name team) (:team-name props))))))) (t/deftest create-team-access-request - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [owner (th/create-profile* 1 {:is-active true :email "owner@bar.com"}) requester (th/create-profile* 3 {:is-active true :email "requester@bar.com"}) team (th/create-team* 1 {:profile-id (:id owner)}) @@ -1012,7 +1012,7 @@ (t/deftest create-team-access-request-owner-muted - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [owner (th/create-profile* 1 {:is-active true :is-muted true :email "owner@bar.com"}) requester (th/create-profile* 2 {:is-active true :email "requester@bar.com"}) team (th/create-team* 1 {:profile-id (:id owner)}) @@ -1031,7 +1031,7 @@ (t/deftest create-team-access-request-requester-muted - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [owner (th/create-profile* 1 {:is-active true :email "owner@bar.com"}) requester (th/create-profile* 2 {:is-active true :is-muted true :email "requester@bar.com"}) team (th/create-team* 1 {:profile-id (:id owner)}) @@ -1057,7 +1057,7 @@ (t/deftest create-team-access-request-owner-bounce - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [owner (th/create-profile* 1 {:is-active true :email "owner@bar.com"}) requester (th/create-profile* 2 {:is-active true :email "requester@bar.com"}) team (th/create-team* 1 {:profile-id (:id owner)}) @@ -1085,7 +1085,7 @@ (t/is (= "private" (:email edata))))))) (t/deftest create-team-access-request-requester-bounce - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [owner (th/create-profile* 1 {:is-active true :email "owner@bar.com"}) requester (th/create-profile* 2 {:is-active true :email "requester@bar.com"}) team (th/create-team* 1 {:profile-id (:id owner)}) @@ -1142,7 +1142,7 @@ (t/is (th/success? out))))) (t/deftest create-team-invitations-email-cooldown - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [profile1 (th/create-profile* 1 {:is-active true}) team (th/create-team* 1 {:profile-id (:id profile1)}) @@ -1276,7 +1276,7 @@ ;; --- T7-F-01: Role ceiling in team invitations --- (t/deftest admin-cannot-create-invitation-with-owner-role - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [owner (th/create-profile* 1 {:is-active true}) admin (th/create-profile* 2 {:is-active true}) team (th/create-team* 1 {:profile-id (:id owner)})] @@ -1300,7 +1300,7 @@ (t/is (= 0 (:call-count @mock))))))) (t/deftest admin-cannot-create-invitation-with-owner-role-invitations-format - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [owner (th/create-profile* 1 {:is-active true}) admin (th/create-profile* 2 {:is-active true}) team (th/create-team* 1 {:profile-id (:id owner)})] @@ -1323,7 +1323,7 @@ (t/is (= 0 (:call-count @mock))))))) (t/deftest admin-cannot-update-invitation-role-to-owner - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [owner (th/create-profile* 1 {:is-active true}) admin (th/create-profile* 2 {:is-active true}) team (th/create-team* 1 {:profile-id (:id owner)})] @@ -1357,7 +1357,7 @@ (t/is (th/ex-of-code? (:error out) :cant-promote-to-owner)))))) (t/deftest owner-can-create-invitation-with-owner-role - (with-mocks [mock {:target 'app.email/send :return nil}] + (with-mocks [mock {:target 'app.email/send-template :return nil}] (let [owner (th/create-profile* 1 {:is-active true}) team (th/create-team* 1 {:profile-id (:id owner)})] @@ -1954,7 +1954,7 @@ ;; The team is created by an internal call, so both the command event and ;; the manual event must carry its id for a projection to key it. (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil} - email-mock {:target 'app.email/send :return nil}] + email-mock {:target 'app.email/send-template :return nil}] (with-redefs [cf/flags (conj cf/flags :audit-log)] (let [profile (th/create-profile* 1 {:is-active true}) out (th/command-raw! {::th/type :create-team-with-invitations