From 0ade0405f560c277f8213c150538dbaefb9e1e94 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 23 Feb 2022 11:49:25 +0100 Subject: [PATCH 1/2] :bug: Fix feedback and audit-log http handlers --- backend/src/app/http/feedback.clj | 58 ++++++++++++--------- backend/src/app/loggers/audit.clj | 84 +++++++++++++++++-------------- backend/src/app/main.clj | 5 +- 3 files changed, 82 insertions(+), 65 deletions(-) diff --git a/backend/src/app/http/feedback.clj b/backend/src/app/http/feedback.clj index 8b0938bbe..a402b4bd1 100644 --- a/backend/src/app/http/feedback.clj +++ b/backend/src/app/http/feedback.clj @@ -14,48 +14,56 @@ [app.db :as db] [app.emails :as eml] [app.rpc.queries.profile :as profile] + [app.worker :as wrk] [clojure.spec.alpha :as s] - [integrant.core :as ig])) + [integrant.core :as ig] + [promesa.core :as p] + [promesa.exec :as px])) -(declare send-feedback) +(declare ^:private send-feedback) +(declare ^:private handler) (defmethod ig/pre-init-spec ::handler [_] - (s/keys :req-un [::db/pool])) + (s/keys :req-un [::db/pool ::wrk/executor])) (defmethod ig/init-key ::handler - [_ {:keys [pool] :as scfg}] + [_ {:keys [executor] :as cfg}] (let [ftoken (cf/get :feedback-token ::no-token) - enabled (contains? cf/flags :user-feedback)] - (fn [{:keys [profile-id] :as request}] - (let [token (get-in request [:headers "x-feedback-token"]) - params (d/merge (:params request) - (:body-params request))] + enabled? (contains? cf/flags :user-feedback)] + (if enabled? + (fn [request respond raise] + (-> (px/submit! executor #(handler cfg request)) + (p/then' respond) + (p/catch raise))) + (fn [_ _ raise] + (raise (ex/error :type :validation + :code :feedback-disabled + :hint "feedback module is disabled")))))) - (when-not enabled - (ex/raise :type :validation - :code :feedback-disabled - :hint "feedback module is disabled")) +(defn- handler + [{:keys [pool] :as cfg} {:keys [profile-id] :as request}] + (let [ftoken (cf/get :feedback-token ::no-token) + token (get-in request [:headers "x-feedback-token"]) + params (d/merge (:params request) + (:body-params request))] + (cond + (uuid? profile-id) + (let [profile (profile/retrieve-profile-data pool profile-id) + params (assoc params :from (:email profile))] + (send-feedback pool profile params)) - (cond - (uuid? profile-id) - (let [profile (profile/retrieve-profile-data pool profile-id) - params (assoc params :from (:email profile))] - (when-not (:is-muted profile) - (send-feedback pool profile params))) + (= token ftoken) + (send-feedback cfg nil params)) - (= token ftoken) - (send-feedback scfg nil params)) - - {:status 204 :body ""})))) + {:status 204 :body ""})) (s/def ::content ::us/string) (s/def ::from ::us/email) (s/def ::subject ::us/string) - (s/def ::feedback (s/keys :req-un [::from ::subject ::content])) -(defn send-feedback +(defn- send-feedback [pool profile params] (let [params (us/conform ::feedback params) destination (cf/get :feedback-destination)] diff --git a/backend/src/app/loggers/audit.clj b/backend/src/app/loggers/audit.clj index 1c113bb14..a7154c03c 100644 --- a/backend/src/app/loggers/audit.clj +++ b/backend/src/app/loggers/audit.clj @@ -22,6 +22,8 @@ [clojure.core.async :as a] [clojure.spec.alpha :as s] [cuerdas.core :as str] + [promesa.core :as p] + [promesa.exec :as px] [integrant.core :as ig] [lambdaisland.uri :as u] [promesa.exec :as px])) @@ -83,49 +85,55 @@ (defmethod ig/init-key ::http-handler [_ {:keys [executor pool] :as cfg}] - (if (db/read-only? pool) + (if (or (db/read-only? pool) (not (contains? cf/flags :audit-log))) (do - (l/warn :hint "audit log http handler disabled, db is read-only") - (constantly {:status 204 :body ""})) - (fn [{:keys [params profile-id] :as request}] - (when (contains? cf/flags :audit-log) - (let [events (->> (:events params) - (remove #(not= profile-id (:profile-id %))) - (us/conform ::frontend-events)) - ip-addr (parse-client-ip request) - cfg (-> cfg - (assoc :source "frontend") - (assoc :events events) - (assoc :ip-addr ip-addr))] + (l/warn :hint "audit log http handler disabled or db is read-only") + (fn [_ respond _] + (respond {:status 204 :body ""}))) - (px/run! executor #(persist-http-events cfg)))) - {:status 204 :body ""}))) + + (letfn [(handler [{:keys [params profile-id] :as request}] + (let [events (->> (:events params) + (remove #(not= profile-id (:profile-id %))) + (us/conform ::frontend-events)) + + ip-addr (parse-client-ip request) + cfg (-> cfg + (assoc :source "frontend") + (assoc :events events) + (assoc :ip-addr ip-addr))] + (persist-http-events cfg))) + + (handle-error [cause] + (let [xdata (ex-data cause)] + (if (= :spec-validation (:code xdata)) + (l/error ::l/raw (str "spec validation on persist-events:\n" (us/pretty-explain xdata))) + (l/error :hint "error on persist-events" :cause cause))))] + + (fn [request respond raise] + ;; Fire and forget, log error in case of errro + (-> (px/submit! executor #(handler request)) + (p/catch handle-error)) + + (respond {:status 204 :body ""}))))) (defn- persist-http-events [{:keys [pool events ip-addr source] :as cfg}] - (try - (let [columns [:id :name :source :type :tracked-at :profile-id :ip-addr :props :context] - prepare-xf (map (fn [event] - [(uuid/next) - (:name event) - source - (:type event) - (:timestamp event) - (:profile-id event) - (db/inet ip-addr) - (db/tjson (:props event)) - (db/tjson (d/without-nils (:context event)))])) - events (us/conform ::events events)] - (when (seq events) - (->> (into [] prepare-xf events) - (db/insert-multi! pool :audit-log columns)))) - (catch Throwable e - (let [xdata (ex-data e)] - (if (= :spec-validation (:code xdata)) - (l/error ::l/raw (str "spec validation on persist-events:\n" - (:explain xdata))) - (l/error :hint "error on persist-events" - :cause e)))))) + (let [columns [:id :name :source :type :tracked-at :profile-id :ip-addr :props :context] + prepare-xf (map (fn [event] + [(uuid/next) + (:name event) + source + (:type event) + (:timestamp event) + (:profile-id event) + (db/inet ip-addr) + (db/tjson (:props event)) + (db/tjson (d/without-nils (:context event)))])) + events (us/conform ::events events)] + (when (seq events) + (->> (into [] prepare-xf events) + (db/insert-multi! pool :audit-log columns))))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Collector diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index 6430a8e4e..b8464c39f 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -135,7 +135,8 @@ :signature-max-age (dt/duration {:hours 24 :minutes 5})} :app.http.feedback/handler - {:pool (ig/ref :app.db/pool)} + {:pool (ig/ref :app.db/pool) + :executor (ig/ref [::default :app.worker/executor])} :app.http.oauth/handler {:rpc (ig/ref :app.rpc/rpc) @@ -280,7 +281,7 @@ :app.loggers.audit/http-handler {:pool (ig/ref :app.db/pool) - :executor (ig/ref [::default :app.worker/executor])} + :executor (ig/ref [::worker :app.worker/executor])} :app.loggers.audit/collector {:pool (ig/ref :app.db/pool) From 0b0ae756a383c5c023a31910970877f241d5bd00 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 23 Feb 2022 11:49:25 +0100 Subject: [PATCH 2/2] :bug: Minor fix on audit http handler --- backend/src/app/loggers/audit.clj | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/backend/src/app/loggers/audit.clj b/backend/src/app/loggers/audit.clj index a7154c03c..60ef42abd 100644 --- a/backend/src/app/loggers/audit.clj +++ b/backend/src/app/loggers/audit.clj @@ -81,7 +81,7 @@ (s/keys :req-un [::type ::name ::props ::timestamp ::profile-id] :opt-un [::context])) -(s/def ::frontend-events (s/every ::event)) +(s/def ::frontend-events (s/every ::frontend-event)) (defmethod ig/init-key ::http-handler [_ {:keys [executor pool] :as cfg}] @@ -129,8 +129,7 @@ (:profile-id event) (db/inet ip-addr) (db/tjson (:props event)) - (db/tjson (d/without-nils (:context event)))])) - events (us/conform ::events events)] + (db/tjson (d/without-nils (:context event)))]))] (when (seq events) (->> (into [] prepare-xf events) (db/insert-multi! pool :audit-log columns)))))