Add proper audit log for invitations

This commit is contained in:
Andrey Antukh 2023-02-23 16:26:31 +01:00
parent 475b6ff6e0
commit f0c0e5e43a
8 changed files with 105 additions and 95 deletions

View file

@ -460,12 +460,11 @@
(ex/raise :type :restriction (ex/raise :type :restriction
:code :profile-blocked)) :code :profile-blocked))
(when-let [collector (::audit/collector cfg)] (audit/submit! cfg {:type "command"
(audit/submit! collector {:type "command" :name "login-with-password"
:name "login" :profile-id (:id profile)
:profile-id (:id profile) :ip-addr (audit/parse-client-ip request)
:ip-addr (audit/parse-client-ip request) :props (audit/profile->props profile)})
:props (audit/profile->props profile)}))
(->> (redirect-response uri) (->> (redirect-response uri)
(sxf request))) (sxf request)))

View file

@ -167,7 +167,11 @@
(instance? javax.sql.DataSource v)) (instance? javax.sql.DataSource v))
(s/def ::pool pool?) (s/def ::pool pool?)
(s/def ::conn some?)
;; DEPRECATED: to be removed in 1.18
(s/def ::conn-or-pool some?) (s/def ::conn-or-pool some?)
(s/def ::pool-or-conn some?)
(defn closed? (defn closed?
[pool] [pool]

View file

@ -20,7 +20,6 @@
[app.loggers.audit.tasks :as-alias tasks] [app.loggers.audit.tasks :as-alias tasks]
[app.loggers.webhooks :as-alias webhooks] [app.loggers.webhooks :as-alias webhooks]
[app.main :as-alias main] [app.main :as-alias main]
[app.metrics :as mtx]
[app.rpc :as-alias rpc] [app.rpc :as-alias rpc]
[app.tokens :as tokens] [app.tokens :as tokens]
[app.util.retry :as rtry] [app.util.retry :as rtry]
@ -30,7 +29,6 @@
[cuerdas.core :as str] [cuerdas.core :as str]
[integrant.core :as ig] [integrant.core :as ig]
[lambdaisland.uri :as u] [lambdaisland.uri :as u]
[promesa.core :as p]
[promesa.exec :as px] [promesa.exec :as px]
[yetti.request :as yrq])) [yetti.request :as yrq]))
@ -124,7 +122,7 @@
(s/keys :req [::wrk/executor ::db/pool])) (s/keys :req [::wrk/executor ::db/pool]))
(defmethod ig/pre-init-spec ::collector [_] (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 (defmethod ig/init-key ::collector
[_ {:keys [::db/pool] :as cfg}] [_ {:keys [::db/pool] :as cfg}]
@ -135,8 +133,8 @@
:else :else
cfg)) cfg))
(defn- persist-event! (defn- handle-event!
[pool event] [conn-or-pool event]
(us/verify! ::event event) (us/verify! ::event event)
(let [params {:id (uuid/next) (let [params {:id (uuid/next)
:name (:name event) :name (:name event)
@ -153,7 +151,7 @@
::rtry/max-retries 6 ::rtry/max-retries 6
::rtry/label "persist-audit-log-event"} ::rtry/label "persist-audit-log-event"}
(let [now (dt/now)] (let [now (dt/now)]
(db/insert! pool :audit-log (db/insert! conn-or-pool :audit-log
(-> params (-> params
(update :props db/tjson) (update :props db/tjson)
(update :ip-addr db/inet) (update :ip-addr db/inet)
@ -172,7 +170,7 @@
:else label) :else label)
dedupe? (boolean (and batch-key batch-timeout))] 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/task :process-webhook-event
::wrk/queue :webhooks ::wrk/queue :webhooks
::wrk/max-retries 0 ::wrk/max-retries 0
@ -183,16 +181,19 @@
::webhooks/event ::webhooks/event
(-> params (-> params
(dissoc :ip-addr) (dissoc :ip-addr)
(dissoc :type))))))) (dissoc :type)))))
params))
(defn submit! (defn submit!
"Submit audit event to the collector." "Submit audit event to the collector."
[{:keys [::wrk/executor ::db/pool] :as collector} params] [{:keys [::wrk/executor] :as cfg} params]
(us/assert! ::collector collector) (let [conn (or (::db/conn cfg) (::db/pool cfg))]
(->> (px/submit! executor (partial persist-event! pool (d/without-nils params))) (us/assert! ::wrk/executor executor)
(p/merr (fn [cause] (us/assert! ::db/pool-or-conn conn)
(l/error :hint "audit: unexpected error processing event" :cause cause) (try
(p/resolved nil))))) (handle-event! conn (d/without-nils params))
(catch Throwable cause
(l/error :hint "audit: unexpected error processing event" :cause cause)))))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; TASK: ARCHIVE ;; TASK: ARCHIVE

View file

@ -14,7 +14,6 @@
[app.db :as-alias db] [app.db :as-alias db]
[app.http.client :as-alias http.client] [app.http.client :as-alias http.client]
[app.http.session :as-alias http.session] [app.http.session :as-alias http.session]
[app.loggers.audit :as-alias audit]
[app.loggers.audit.tasks :as-alias audit.tasks] [app.loggers.audit.tasks :as-alias audit.tasks]
[app.loggers.webhooks :as-alias webhooks] [app.loggers.webhooks :as-alias webhooks]
[app.loggers.zmq :as-alias lzmq] [app.loggers.zmq :as-alias lzmq]
@ -268,10 +267,8 @@
:github (ig/ref ::oidc.providers/github) :github (ig/ref ::oidc.providers/github)
:gitlab (ig/ref ::oidc.providers/gitlab) :gitlab (ig/ref ::oidc.providers/gitlab)
:oidc (ig/ref ::oidc.providers/generic)} :oidc (ig/ref ::oidc.providers/generic)}
::audit/collector (ig/ref ::audit/collector)
::http.session/session (ig/ref :app.http.session/manager)} ::http.session/session (ig/ref :app.http.session/manager)}
;; TODO: revisit the dependencies of this service, looks they are too much unused of them ;; TODO: revisit the dependencies of this service, looks they are too much unused of them
:app.http/router :app.http/router
{:assets (ig/ref :app.http.assets/handlers) {:assets (ig/ref :app.http.assets/handlers)
@ -324,8 +321,7 @@
:scheduled-executor (ig/ref ::wrk/scheduled-executor)} :scheduled-executor (ig/ref ::wrk/scheduled-executor)}
:app.rpc/methods :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) ::db/pool (ig/ref ::db/pool)
::wrk/executor (ig/ref ::wrk/executor) ::wrk/executor (ig/ref ::wrk/executor)
::props (ig/ref :app.setup/props) ::props (ig/ref :app.setup/props)
@ -423,11 +419,6 @@
::lzmq/receiver ::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 ::audit.tasks/archive
{::props (ig/ref :app.setup/props) {::props (ig/ref :app.setup/props)
::db/pool (ig/ref ::db/pool) ::db/pool (ig/ref ::db/pool)

View file

@ -11,6 +11,7 @@
[app.common.logging :as l] [app.common.logging :as l]
[app.common.spec :as us] [app.common.spec :as us]
[app.common.uuid :as uuid] [app.common.uuid :as uuid]
[app.config :as cf]
[app.db :as db] [app.db :as db]
[app.http :as-alias http] [app.http :as-alias http]
[app.http.client :as-alias http.client] [app.http.client :as-alias http.client]
@ -163,7 +164,8 @@
(defn- wrap-audit (defn- wrap-audit
[cfg f mdata] [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] (letfn [(handle-audit [params result]
(let [resultm (meta result) (let [resultm (meta result)
request (::http/request params) request (::http/request params)
@ -208,13 +210,14 @@
(::webhooks/event? resultm) (::webhooks/event? resultm)
false)}] false)}]
(audit/submit! collector event))) (audit/submit! cfg event)))
(handle-request [cfg params] (handle-request [cfg params]
(->> (f cfg params) (->> (f cfg params)
(p/mcat (fn [result] (p/fnly (fn [result cause]
(->> (handle-audit params result) (when-not cause
(p/map (constantly result)))))))] (handle-audit params result))))))]
(if-not (::audit/skip mdata) (if-not (::audit/skip mdata)
(with-meta handle-request mdata) (with-meta handle-request mdata)
f)) f))
@ -314,8 +317,7 @@
(s/def ::sprops map?) (s/def ::sprops map?)
(defmethod ig/pre-init-spec ::methods [_] (defmethod ig/pre-init-spec ::methods [_]
(s/keys :req [::audit/collector (s/keys :req [::http.client/client
::http.client/client
::db/pool ::db/pool
::ldap/provider ::ldap/provider
::wrk/executor] ::wrk/executor]

View file

@ -348,7 +348,7 @@
:extra-data ptoken}))) :extra-data ptoken})))
(defn register-profile (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}) (let [claims (tokens/verify (::main/props cfg) {:token token :iss :prepared-register})
params (merge params claims) params (merge params claims)
@ -372,11 +372,10 @@
;; accordingly. ;; accordingly.
(when-let [id (:profile-id claims)] (when-let [id (:profile-id claims)]
(db/update! conn :profile {:modified-at (dt/now)} {:id id}) (db/update! conn :profile {:modified-at (dt/now)} {:id id})
(when-let [collector (::audit/collector cfg)] (audit/submit! cfg
(audit/submit! collector {:type "fact"
{:type "fact" :name "register-profile-retry"
:name "register-profile-retry" :profile-id id}))
:profile-id id})))
(cond (cond
;; If invitation token comes in params, this is because the ;; If invitation token comes in params, this is because the
@ -428,7 +427,7 @@
::doc/added "1.15"} ::doc/added "1.15"}
[{:keys [::db/pool] :as cfg} params] [{:keys [::db/pool] :as cfg} params]
(db/with-atomic [conn pool] (db/with-atomic [conn pool]
(-> (assoc cfg :conn conn) (-> (assoc cfg ::db/conn conn)
(register-profile params)))) (register-profile params))))
;; ---- COMMAND: Request Profile Recovery ;; ---- COMMAND: Request Profile Recovery

View file

@ -638,10 +638,11 @@
;; --- Mutation: Create Team Invitation ;; --- Mutation: Create Team Invitation
(def sql:upsert-team-invitation (def sql:upsert-team-invitation
"insert into team_invitation(team_id, email_to, role, valid_until) "insert into team_invitation(id, team_id, email_to, role, valid_until)
values (?, ?, ?, ?) values (?, ?, ?, ?, ?)
on conflict(team_id, email_to) do 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 (defn- create-invitation-token
[cfg {:keys [profile-id valid-until team-id member-id member-email role]}] [cfg {:keys [profile-id valid-until team-id member-id member-email role]}]
@ -662,16 +663,8 @@
:exp (dt/in-future {:days 30})})) :exp (dt/in-future {:days 30})}))
(defn- create-invitation (defn- create-invitation
[{:keys [::conn] :as cfg} {:keys [team profile role email] :as params}] [{:keys [::db/conn] :as cfg} {:keys [team profile role email] :as params}]
(let [member (profile/retrieve-profile-data-by-email conn email) (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)]
(when (and member (not (eml/allow-send-emails? conn member))) (when (and member (not (eml/allow-send-emails? conn member)))
(ex/raise :type :validation (ex/raise :type :validation
@ -686,9 +679,6 @@
:email email :email email
:hint "the email you invite has been repeatedly reported as spam or bounce")) :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 ;; When we have email verification disabled and invitation user is
;; already present in the database, we proceed to add it to the ;; already present in the database, we proceed to add it to the
;; team as-is, without email roundtrip. ;; team as-is, without email roundtrip.
@ -709,10 +699,38 @@
(when-not (:is-active member) (when-not (:is-active member)
(db/update! conn :profile (db/update! conn :profile
{:is-active true} {:is-active true}
{:id (:id member)}))) {:id (:id member)}))
(do
(db/exec-one! conn [sql:upsert-team-invitation nil)
(:id team) (str/lower email) (name role) expire (name role) expire]) (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/send! {::eml/conn conn
::eml/factory eml/invite-to-team ::eml/factory eml/invite-to-team
:public-uri (cf/get :public-uri) :public-uri (cf/get :public-uri)
@ -720,9 +738,9 @@
:invited-by (:fullname profile) :invited-by (:fullname profile)
:team (:name team) :team (:name team)
:token itoken :token itoken
:extra-data ptoken}))) :extra-data ptoken})
itoken)) itoken))))
(s/def ::email ::us/email) (s/def ::email ::us/email)
(s/def ::emails ::us/set-of-valid-emails) (s/def ::emails ::us/set-of-valid-emails)
@ -763,14 +781,14 @@
:code :profile-is-muted :code :profile-is-muted
:hint "looks like the profile has reported repeatedly as spam or has permanent bounces")) :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 invitations (->> emails
(map (fn [email] (map (fn [email]
{:email (str/lower email) {:email (str/lower email)
:team team :team team
:profile profile :profile profile
:role role})) :role role}))
(map (partial create-invitation cfg)))] (keep (partial create-invitation cfg)))]
(with-meta (vec invitations) (with-meta (vec invitations)
{::audit/props {:invitations (count invitations)}}))))) {::audit/props {:invitations (count invitations)}})))))
@ -789,7 +807,7 @@
(let [params (assoc params :profile-id profile-id) (let [params (assoc params :profile-id profile-id)
team (create-team conn params) team (create-team conn params)
profile (db/get-by-id conn :profile profile-id) 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. ;; Create invitations for all provided emails.
(->> emails (->> emails
@ -812,18 +830,16 @@
::quotes/team-id (:id team) ::quotes/team-id (:id team)
::quotes/incr (count emails)})) ::quotes/incr (count emails)}))
(-> team (audit/submit! cfg
(vary-meta assoc ::audit/props {:invitations (count emails)}) {:type "command"
(rph/with-defer :name "create-team-invitations"
#(when-let [collector (::audit/collector cfg)] :profile-id profile-id
(audit/submit! collector :props {:emails emails
{:type "command" :role role
:name "create-team-invitations" :profile-id profile-id
:profile-id profile-id :invitations (count emails)}})
:props {:emails emails
:role role (vary-meta team assoc ::audit/props {:invitations (count emails)}))))
:profile-id profile-id
:invitations (count emails)}})))))))
;; --- Query: get-team-invitation-token ;; --- Query: get-team-invitation-token
@ -885,6 +901,7 @@
(ex/raise :type :validation (ex/raise :type :validation
:code :insufficient-permissions)) :code :insufficient-permissions))
(db/delete! conn :team-invitation (let [invitation (db/delete! conn :team-invitation
{:team-id team-id :email-to (str/lower email)}) {:team-id team-id
nil))) :email-to (str/lower email)})]
(rph/wrap nil {::audit/props {:invitation-id (:id invitation)}})))))

View file

@ -158,11 +158,10 @@
(let [profile (accept-invitation cfg claims invitation profile)] (let [profile (accept-invitation cfg claims invitation profile)]
(-> (assoc claims :state :created) (-> (assoc claims :state :created)
(rph/with-meta {::audit/name "accept-team-invitation" (rph/with-meta {::audit/name "accept-team-invitation"
::audit/props (merge ::audit/profile-id (:id profile)
(audit/profile->props profile) ::audit/props {:team-id (:team-id claims)
{:team-id (:team-id claims) :role (:role claims)
:role (:role claims)}) :invitation-id (:id invitation)}})))
::audit/profile-id profile-id})))
(ex/raise :type :validation (ex/raise :type :validation
:code :invalid-token :code :invalid-token
@ -181,12 +180,10 @@
(-> (assoc claims :state :created) (-> (assoc claims :state :created)
(rph/with-transform (session/create-fn session (:id profile))) (rph/with-transform (session/create-fn session (:id profile)))
(rph/with-meta {::audit/name "accept-team-invitation" (rph/with-meta {::audit/name "accept-team-invitation"
::audit/props (merge ::audit/profile-id (:id profile)
(audit/profile->props profile) ::audit/props {:team-id (:team-id claims)
{:team-id (:team-id claims) :role (:role claims)
:role (:role claims)}) :invitation-id (:id invitation)}})))
::audit/profile-id member-id})))
{:invitation-token token {:invitation-token token
:iss :team-invitation :iss :team-invitation
:redirect-to :auth-register :redirect-to :auth-register