diff --git a/backend/src/app/auth/oidc.clj b/backend/src/app/auth/oidc.clj index 15b332408..603149d28 100644 --- a/backend/src/app/auth/oidc.clj +++ b/backend/src/app/auth/oidc.clj @@ -460,12 +460,11 @@ (ex/raise :type :restriction :code :profile-blocked)) - (when-let [collector (::audit/collector cfg)] - (audit/submit! collector {:type "command" - :name "login" - :profile-id (:id profile) - :ip-addr (audit/parse-client-ip request) - :props (audit/profile->props profile)})) + (audit/submit! cfg {:type "command" + :name "login-with-password" + :profile-id (:id profile) + :ip-addr (audit/parse-client-ip request) + :props (audit/profile->props profile)}) (->> (redirect-response uri) (sxf request))) diff --git a/backend/src/app/db.clj b/backend/src/app/db.clj index 045ee46a9..f23e1fbec 100644 --- a/backend/src/app/db.clj +++ b/backend/src/app/db.clj @@ -167,7 +167,11 @@ (instance? javax.sql.DataSource v)) (s/def ::pool pool?) +(s/def ::conn some?) + +;; DEPRECATED: to be removed in 1.18 (s/def ::conn-or-pool some?) +(s/def ::pool-or-conn some?) (defn closed? [pool] diff --git a/backend/src/app/loggers/audit.clj b/backend/src/app/loggers/audit.clj index 68aa0b116..1038e8249 100644 --- a/backend/src/app/loggers/audit.clj +++ b/backend/src/app/loggers/audit.clj @@ -20,7 +20,6 @@ [app.loggers.audit.tasks :as-alias tasks] [app.loggers.webhooks :as-alias webhooks] [app.main :as-alias main] - [app.metrics :as mtx] [app.rpc :as-alias rpc] [app.tokens :as tokens] [app.util.retry :as rtry] @@ -30,7 +29,6 @@ [cuerdas.core :as str] [integrant.core :as ig] [lambdaisland.uri :as u] - [promesa.core :as p] [promesa.exec :as px] [yetti.request :as yrq])) @@ -124,7 +122,7 @@ (s/keys :req [::wrk/executor ::db/pool])) (defmethod ig/pre-init-spec ::collector [_] - (s/keys :req [::db/pool ::wrk/executor ::mtx/metrics])) + (s/keys :req [::db/pool ::wrk/executor])) (defmethod ig/init-key ::collector [_ {:keys [::db/pool] :as cfg}] @@ -135,8 +133,8 @@ :else cfg)) -(defn- persist-event! - [pool event] +(defn- handle-event! + [conn-or-pool event] (us/verify! ::event event) (let [params {:id (uuid/next) :name (:name event) @@ -153,7 +151,7 @@ ::rtry/max-retries 6 ::rtry/label "persist-audit-log-event"} (let [now (dt/now)] - (db/insert! pool :audit-log + (db/insert! conn-or-pool :audit-log (-> params (update :props db/tjson) (update :ip-addr db/inet) @@ -172,7 +170,7 @@ :else label) dedupe? (boolean (and batch-key batch-timeout))] - (wrk/submit! ::wrk/conn pool + (wrk/submit! ::wrk/conn conn-or-pool ::wrk/task :process-webhook-event ::wrk/queue :webhooks ::wrk/max-retries 0 @@ -183,16 +181,19 @@ ::webhooks/event (-> params (dissoc :ip-addr) - (dissoc :type))))))) + (dissoc :type))))) + params)) (defn submit! "Submit audit event to the collector." - [{:keys [::wrk/executor ::db/pool] :as collector} params] - (us/assert! ::collector collector) - (->> (px/submit! executor (partial persist-event! pool (d/without-nils params))) - (p/merr (fn [cause] - (l/error :hint "audit: unexpected error processing event" :cause cause) - (p/resolved nil))))) + [{:keys [::wrk/executor] :as cfg} params] + (let [conn (or (::db/conn cfg) (::db/pool cfg))] + (us/assert! ::wrk/executor executor) + (us/assert! ::db/pool-or-conn conn) + (try + (handle-event! conn (d/without-nils params)) + (catch Throwable cause + (l/error :hint "audit: unexpected error processing event" :cause cause))))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; TASK: ARCHIVE diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index 02d33f479..f9a9c5416 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -14,7 +14,6 @@ [app.db :as-alias db] [app.http.client :as-alias http.client] [app.http.session :as-alias http.session] - [app.loggers.audit :as-alias audit] [app.loggers.audit.tasks :as-alias audit.tasks] [app.loggers.webhooks :as-alias webhooks] [app.loggers.zmq :as-alias lzmq] @@ -268,10 +267,8 @@ :github (ig/ref ::oidc.providers/github) :gitlab (ig/ref ::oidc.providers/gitlab) :oidc (ig/ref ::oidc.providers/generic)} - ::audit/collector (ig/ref ::audit/collector) ::http.session/session (ig/ref :app.http.session/manager)} - ;; TODO: revisit the dependencies of this service, looks they are too much unused of them :app.http/router {:assets (ig/ref :app.http.assets/handlers) @@ -324,8 +321,7 @@ :scheduled-executor (ig/ref ::wrk/scheduled-executor)} :app.rpc/methods - {::audit/collector (ig/ref ::audit/collector) - ::http.client/client (ig/ref ::http.client/client) + {::http.client/client (ig/ref ::http.client/client) ::db/pool (ig/ref ::db/pool) ::wrk/executor (ig/ref ::wrk/executor) ::props (ig/ref :app.setup/props) @@ -423,11 +419,6 @@ ::lzmq/receiver {} - ::audit/collector - {::db/pool (ig/ref ::db/pool) - ::wrk/executor (ig/ref ::wrk/executor) - ::mtx/metrics (ig/ref ::mtx/metrics)} - ::audit.tasks/archive {::props (ig/ref :app.setup/props) ::db/pool (ig/ref ::db/pool) diff --git a/backend/src/app/rpc.clj b/backend/src/app/rpc.clj index 9b1f9509a..c2c50dc1c 100644 --- a/backend/src/app/rpc.clj +++ b/backend/src/app/rpc.clj @@ -11,6 +11,7 @@ [app.common.logging :as l] [app.common.spec :as us] [app.common.uuid :as uuid] + [app.config :as cf] [app.db :as db] [app.http :as-alias http] [app.http.client :as-alias http.client] @@ -163,7 +164,8 @@ (defn- wrap-audit [cfg f mdata] - (if-let [collector (::audit/collector cfg)] + (if (or (contains? cf/flags :webhooks) + (contains? cf/flags :audit-log)) (letfn [(handle-audit [params result] (let [resultm (meta result) request (::http/request params) @@ -208,13 +210,14 @@ (::webhooks/event? resultm) false)}] - (audit/submit! collector event))) + (audit/submit! cfg event))) (handle-request [cfg params] (->> (f cfg params) - (p/mcat (fn [result] - (->> (handle-audit params result) - (p/map (constantly result)))))))] + (p/fnly (fn [result cause] + (when-not cause + (handle-audit params result))))))] + (if-not (::audit/skip mdata) (with-meta handle-request mdata) f)) @@ -314,8 +317,7 @@ (s/def ::sprops map?) (defmethod ig/pre-init-spec ::methods [_] - (s/keys :req [::audit/collector - ::http.client/client + (s/keys :req [::http.client/client ::db/pool ::ldap/provider ::wrk/executor] diff --git a/backend/src/app/rpc/commands/auth.clj b/backend/src/app/rpc/commands/auth.clj index e1244dc33..bcd4f1044 100644 --- a/backend/src/app/rpc/commands/auth.clj +++ b/backend/src/app/rpc/commands/auth.clj @@ -348,7 +348,7 @@ :extra-data ptoken}))) (defn register-profile - [{:keys [conn session] :as cfg} {:keys [token] :as params}] + [{:keys [::db/conn session] :as cfg} {:keys [token] :as params}] (let [claims (tokens/verify (::main/props cfg) {:token token :iss :prepared-register}) params (merge params claims) @@ -372,11 +372,10 @@ ;; accordingly. (when-let [id (:profile-id claims)] (db/update! conn :profile {:modified-at (dt/now)} {:id id}) - (when-let [collector (::audit/collector cfg)] - (audit/submit! collector - {:type "fact" - :name "register-profile-retry" - :profile-id id}))) + (audit/submit! cfg + {:type "fact" + :name "register-profile-retry" + :profile-id id})) (cond ;; If invitation token comes in params, this is because the @@ -428,7 +427,7 @@ ::doc/added "1.15"} [{:keys [::db/pool] :as cfg} params] (db/with-atomic [conn pool] - (-> (assoc cfg :conn conn) + (-> (assoc cfg ::db/conn conn) (register-profile params)))) ;; ---- COMMAND: Request Profile Recovery diff --git a/backend/src/app/rpc/commands/teams.clj b/backend/src/app/rpc/commands/teams.clj index 9ab23583c..9f1265ef9 100644 --- a/backend/src/app/rpc/commands/teams.clj +++ b/backend/src/app/rpc/commands/teams.clj @@ -638,10 +638,11 @@ ;; --- Mutation: Create Team Invitation (def sql:upsert-team-invitation - "insert into team_invitation(team_id, email_to, role, valid_until) - values (?, ?, ?, ?) + "insert into team_invitation(id, team_id, email_to, role, valid_until) + values (?, ?, ?, ?, ?) on conflict(team_id, email_to) do - update set role = ?, valid_until = ?, updated_at = now();") + update set role = ?, valid_until = ?, updated_at = now() + returning *") (defn- create-invitation-token [cfg {:keys [profile-id valid-until team-id member-id member-email role]}] @@ -662,16 +663,8 @@ :exp (dt/in-future {:days 30})})) (defn- create-invitation - [{:keys [::conn] :as cfg} {:keys [team profile role email] :as params}] - (let [member (profile/retrieve-profile-data-by-email conn email) - expire (dt/in-future "168h") ;; 7 days - itoken (create-invitation-token cfg {:profile-id (:id profile) - :valid-until expire - :team-id (:id team) - :member-email (or (:email member) email) - :member-id (:id member) - :role role}) - ptoken (create-profile-identity-token cfg profile)] + [{:keys [::db/conn] :as cfg} {:keys [team profile role email] :as params}] + (let [member (profile/retrieve-profile-data-by-email conn email)] (when (and member (not (eml/allow-send-emails? conn member))) (ex/raise :type :validation @@ -686,9 +679,6 @@ :email email :hint "the email you invite has been repeatedly reported as spam or bounce")) - (when (contains? cf/flags :log-invitation-tokens) - (l/trace :hint "invitation token" :token itoken)) - ;; When we have email verification disabled and invitation user is ;; already present in the database, we proceed to add it to the ;; team as-is, without email roundtrip. @@ -709,10 +699,38 @@ (when-not (:is-active member) (db/update! conn :profile {:is-active true} - {:id (:id member)}))) - (do - (db/exec-one! conn [sql:upsert-team-invitation - (:id team) (str/lower email) (name role) expire (name role) expire]) + {:id (:id member)})) + + nil) + (let [id (uuid/next) + expire (dt/in-future "168h") ;; 7 days + invitation (db/exec-one! conn [sql:upsert-team-invitation id + (:id team) (str/lower email) + (name role) expire + (name role) expire]) + updated? (not= id (:id invitation)) + tprops {:profile-id (:id profile) + :invitation-id (:id invitation) + :valid-until expire + :team-id (:id team) + :member-email (:email-to invitation) + :member-id (:id member) + :role role} + itoken (create-invitation-token cfg tprops) + ptoken (create-profile-identity-token cfg profile)] + + (when (contains? cf/flags :log-invitation-tokens) + (l/info :hint "invitation token" :token itoken)) + + (audit/submit! cfg + {:type "action" + :name (if updated? + "update-team-invitation" + "create-team-invitation") + :profile-id (:id profile) + :props (-> (dissoc tprops :profile-id) + (d/without-nils))}) + (eml/send! {::eml/conn conn ::eml/factory eml/invite-to-team :public-uri (cf/get :public-uri) @@ -720,9 +738,9 @@ :invited-by (:fullname profile) :team (:name team) :token itoken - :extra-data ptoken}))) + :extra-data ptoken}) - itoken)) + itoken)))) (s/def ::email ::us/email) (s/def ::emails ::us/set-of-valid-emails) @@ -763,14 +781,14 @@ :code :profile-is-muted :hint "looks like the profile has reported repeatedly as spam or has permanent bounces")) - (let [cfg (assoc cfg ::conn conn) + (let [cfg (assoc cfg ::db/conn conn) invitations (->> emails (map (fn [email] {:email (str/lower email) :team team :profile profile :role role})) - (map (partial create-invitation cfg)))] + (keep (partial create-invitation cfg)))] (with-meta (vec invitations) {::audit/props {:invitations (count invitations)}}))))) @@ -789,7 +807,7 @@ (let [params (assoc params :profile-id profile-id) team (create-team conn params) profile (db/get-by-id conn :profile profile-id) - cfg (assoc cfg ::conn conn)] + cfg (assoc cfg ::db/conn conn)] ;; Create invitations for all provided emails. (->> emails @@ -812,18 +830,16 @@ ::quotes/team-id (:id team) ::quotes/incr (count emails)})) - (-> team - (vary-meta assoc ::audit/props {:invitations (count emails)}) - (rph/with-defer - #(when-let [collector (::audit/collector cfg)] - (audit/submit! collector - {:type "command" - :name "create-team-invitations" - :profile-id profile-id - :props {:emails emails - :role role - :profile-id profile-id - :invitations (count emails)}}))))))) + (audit/submit! cfg + {:type "command" + :name "create-team-invitations" + :profile-id profile-id + :props {:emails emails + :role role + :profile-id profile-id + :invitations (count emails)}}) + + (vary-meta team assoc ::audit/props {:invitations (count emails)})))) ;; --- Query: get-team-invitation-token @@ -885,6 +901,7 @@ (ex/raise :type :validation :code :insufficient-permissions)) - (db/delete! conn :team-invitation - {:team-id team-id :email-to (str/lower email)}) - nil))) + (let [invitation (db/delete! conn :team-invitation + {:team-id team-id + :email-to (str/lower email)})] + (rph/wrap nil {::audit/props {:invitation-id (:id invitation)}}))))) diff --git a/backend/src/app/rpc/commands/verify_token.clj b/backend/src/app/rpc/commands/verify_token.clj index 6eb455f22..379152600 100644 --- a/backend/src/app/rpc/commands/verify_token.clj +++ b/backend/src/app/rpc/commands/verify_token.clj @@ -158,11 +158,10 @@ (let [profile (accept-invitation cfg claims invitation profile)] (-> (assoc claims :state :created) (rph/with-meta {::audit/name "accept-team-invitation" - ::audit/props (merge - (audit/profile->props profile) - {:team-id (:team-id claims) - :role (:role claims)}) - ::audit/profile-id profile-id}))) + ::audit/profile-id (:id profile) + ::audit/props {:team-id (:team-id claims) + :role (:role claims) + :invitation-id (:id invitation)}}))) (ex/raise :type :validation :code :invalid-token @@ -181,12 +180,10 @@ (-> (assoc claims :state :created) (rph/with-transform (session/create-fn session (:id profile))) (rph/with-meta {::audit/name "accept-team-invitation" - ::audit/props (merge - (audit/profile->props profile) - {:team-id (:team-id claims) - :role (:role claims)}) - ::audit/profile-id member-id}))) - + ::audit/profile-id (:id profile) + ::audit/props {:team-id (:team-id claims) + :role (:role claims) + :invitation-id (:id invitation)}}))) {:invitation-token token :iss :team-invitation :redirect-to :auth-register