Merge pull request #4922 from penpot/niwinz-staging-inet

✨ Ip Addr parsing and audit log context forwarding fixes

backend/src/app/auth/oidc.clj

@@ -26,6 +26,7 @@
    [app.rpc.commands.profile :as profile]
    [app.setup :as-alias setup]
    [app.tokens :as tokens]
+   [app.util.inet :as inet]
    [app.util.json :as json]
    [app.util.time :as dt]
    [buddy.sign.jwk :as jwk]
@@ -571,10 +572,10 @@
             props   (audit/profile->props profile)
             context (d/without-nils {:external-session-id (:external-session-id info)})]
 
-        (audit/submit! cfg {::audit/type "command"
+        (audit/submit! cfg {::audit/type "action"
                             ::audit/name "login-with-oidc"
                             ::audit/profile-id (:id profile)
-                            ::audit/ip-addr (audit/parse-client-ip request)
+                            ::audit/ip-addr (inet/parse-request request)
                             ::audit/props props
                             ::audit/context context})
 

backend/src/app/loggers/audit.clj

@@ -21,28 +21,18 @@
    [app.rpc :as-alias rpc]
    [app.rpc.retry :as rtry]
    [app.setup :as-alias setup]
+   [app.util.inet :as inet]
    [app.util.services :as-alias sv]
    [app.util.time :as dt]
    [app.worker :as wrk]
    [clojure.spec.alpha :as s]
    [cuerdas.core :as str]
-   [integrant.core :as ig]
+   [integrant.core :as ig]))
-   [ring.request :as rreq]))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; HELPERS
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
-(defn parse-client-ip
-  [request]
-  (let [ip-addr (or (some-> (rreq/get-header request "x-forwarded-for") (str/split ",") first)
-                    (rreq/get-header request "x-real-ip")
-                    (some-> (rreq/remote-addr request) str))
-        ip-addr (-> ip-addr
-                    (str/split ":" 2)
-                    (first))]
-    ip-addr))
-
 (defn extract-utm-params
   "Extracts additional data from params and namespace them under
   `penpot` ns."
@@ -90,17 +80,20 @@
          (remove #(contains? reserved-props (key %))))
         props))
 
-(defn params->context
+(defn event-from-rpc-params
-  "Extract default context properties from RPC params object"
+  "Create a base event skeleton with pre-filled some important
+  data that can be extracted from RPC params object"
   [params]
-  (d/without-nils
+  (let [context {:external-session-id (::rpc/external-session-id params)
-   {:external-session-id (::rpc/external-session-id params)
+                 :external-event-origin (::rpc/external-event-origin params)
-    :event-origin (::rpc/external-event-origin params)
+                 :triggered-by (::rpc/handler-name params)}]
-    :triggered-by (::rpc/handler-name params)}))
+    {::type "action"
+     ::profile-id (::rpc/profile-id params)
+     ::ip-addr (::rpc/ip-addr params)
+     ::context (d/without-nils context)}))
 
 ;; --- SPECS
 
-
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; COLLECTOR
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
@@ -167,14 +160,16 @@
                       (assoc :external-session-id session-id)
                       (assoc :external-event-origin event-origin)
                       (assoc :access-token-id (some-> token-id str))
-                      (d/without-nils))]
+                      (d/without-nils))
+
+        ip-addr   (inet/parse-request request)]
 
     {::type (or (::type resultm)
                 (::rpc/type cfg))
      ::name (or (::name resultm)
                 (::sv/name mdata))
      ::profile-id profile-id
-     ::ip-addr (some-> request parse-client-ip)
+     ::ip-addr ip-addr
      ::props props
      ::context context
 
@@ -202,7 +197,7 @@
                 :name (::name event)
                 :type (::type event)
                 :profile-id (::profile-id event)
-                :ip-addr (::ip-addr event "0.0.0.0")
+                :ip-addr (::ip-addr event)
                 :context (::context event {})
                 :props (::props event {})
                 :source "backend"}
@@ -246,8 +241,7 @@
                        (assoc :created-at tnow)
                        (update :tracked-at #(or % tnow))
                        (assoc :props {})
-                       (assoc :context {})
+                       (assoc :context {}))]
-                       (assoc :ip-addr "0.0.0.0"))]
         (append-audit-entry! cfg params)))
 
     (when (and (contains? cf/flags :webhooks)

backend/src/app/rpc.clj

@@ -29,6 +29,7 @@
    [app.rpc.rlimit :as rlimit]
    [app.setup :as-alias setup]
    [app.storage :as-alias sto]
+   [app.util.inet :as inet]
    [app.util.services :as sv]
    [app.util.time :as dt]
    [clojure.spec.alpha :as s]
@@ -81,7 +82,9 @@
 (defn- get-external-event-origin
   [request]
   (when-let [origin (rreq/get-header request "x-event-origin")]
-    (when-not (> (count origin) 256)
+    (when-not (or (> (count origin) 256)
+                  (= origin "null")
+                  (str/blank? origin))
       origin)))
 
 (defn- rpc-handler
@@ -93,11 +96,13 @@
         profile-id   (or (::session/profile-id request)
                          (::actoken/profile-id request))
 
+        ip-addr      (inet/parse-request request)
         session-id   (get-external-session-id request)
         event-origin (get-external-event-origin request)
 
         data         (-> params
                          (assoc ::handler-name handler-name)
+                         (assoc ::ip-addr ip-addr)
                          (assoc ::request-at (dt/now))
                          (assoc ::external-session-id session-id)
                          (assoc ::external-event-origin event-origin)

backend/src/app/rpc/commands/audit.clj

@@ -14,11 +14,12 @@
    [app.config :as cf]
    [app.db :as db]
    [app.http :as-alias http]
-   [app.loggers.audit :as audit]
+   [app.loggers.audit :as-alias audit]
    [app.rpc :as-alias rpc]
    [app.rpc.climit :as-alias climit]
    [app.rpc.doc :as-alias doc]
    [app.rpc.helpers :as rph]
+   [app.util.inet :as inet]
    [app.util.services :as sv]
    [app.util.time :as dt]))
 
@@ -61,7 +62,7 @@
 (defn- handle-events
   [{:keys [::db/pool]} {:keys [::rpc/profile-id events] :as params}]
   (let [request (-> params meta ::http/request)
-        ip-addr (audit/parse-client-ip request)
+        ip-addr (inet/parse-request request)
         tnow    (dt/now)
         xform   (comp
                  (map (fn [event]

backend/src/app/rpc/commands/management.clj

@@ -413,15 +413,13 @@
                                   {:modified-at (dt/now)}
                                   {:id project-id})
 
-                      (let [props   (audit/clean-props params)
+                      (let [props (audit/clean-props params)]
-                            context (audit/params->context params)]
                         (doseq [file-id result]
-                          (audit/submit! cfg
+                          (let [props (assoc props :id file-id)
-                                         {::audit/type "action"
+                                event (-> (audit/event-from-rpc-params params)
-                                          ::audit/name "create-file"
+                                          (assoc ::audit/name "create-file")
-                                          ::audit/profile-id profile-id
+                                          (assoc ::audit/props props))]
-                                          ::audit/props (assoc props :id file-id)
+                            (audit/submit! cfg event))))
-                                          ::audit/context context})))
 
                       result))))
 

backend/src/app/rpc/commands/teams.clj

@@ -787,18 +787,15 @@
           (l/info :hint "invitation token" :token itoken))
 
 
-        (let [props   (-> (dissoc tprops :profile-id)
+        (let [props  (-> (dissoc tprops :profile-id)
-                          (audit/clean-props))
+                         (audit/clean-props))
-              context (audit/params->context params)]
+              evname (if updated?
-
+                       "update-team-invitation"
-          (audit/submit! cfg
+                       "create-team-invitation")
-                         {::audit/type "action"
+              event (-> (audit/event-from-rpc-params params)
-                          ::audit/name (if updated?
+                        (assoc ::audit/name evname)
-                                         "update-team-invitation"
+                        (assoc ::audit/props props))]
-                                         "create-team-invitation")
+          (audit/submit! cfg event))
-                          ::audit/profile-id (:id profile)
-                          ::audit/props props
-                          ::audit/context context}))
 
         (eml/send! {::eml/conn conn
                     ::eml/factory eml/invite-to-team
@@ -882,62 +879,51 @@
 (sv/defmethod ::create-team-with-invitations
   {::doc/added "1.17"
    ::sm/params schema:create-team-with-invitations}
-  [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id emails role name] :as params}]
+  [cfg {:keys [::rpc/profile-id emails role name] :as params}]
-  (db/with-atomic [conn pool]
 
-    (let [features (-> (cfeat/get-enabled-features cf/flags)
+  (db/tx-run! cfg
-                       (cfeat/check-client-features! (:features params)))
+              (fn [{:keys [::db/conn] :as cfg}]
+                (let [features (-> (cfeat/get-enabled-features cf/flags)
+                                   (cfeat/check-client-features! (:features params)))
 
-          params   (-> params
+                      params   (-> params
-                       (assoc :profile-id profile-id)
+                                   (assoc :profile-id profile-id)
-                       (assoc :features features))
+                                   (assoc :features features))
 
-          cfg      (assoc cfg ::db/conn conn)
+                      cfg      (assoc cfg ::db/conn conn)
-          team     (create-team cfg params)
+                      team     (create-team cfg params)
-          profile  (db/get-by-id conn :profile profile-id)
+                      profile  (db/get-by-id conn :profile profile-id)
-          emails   (into #{} (map profile/clean-email) emails)
+                      emails   (into #{} (map profile/clean-email) emails)]
-          context  (audit/params->context params)]
 
-      ;; Create invitations for all provided emails.
+                  (let [props {:name name :features features}
-      (->> emails
+                        event (-> (audit/event-from-rpc-params params)
-           (map (fn [email]
+                                  (assoc ::audit/name "create-team")
-                  (-> params
+                                  (assoc ::audit/props props))]
-                      (assoc :team team)
+                    (audit/submit! cfg event))
-                      (assoc :profile profile)
-                      (assoc :email email)
-                      (assoc :role role))))
-           (run! (partial create-invitation cfg)))
 
-      (run! (partial quotes/check-quote! conn)
+                  ;; Create invitations for all provided emails.
-            (list {::quotes/id ::quotes/teams-per-profile
+                  (->> emails
-                   ::quotes/profile-id profile-id}
+                       (map (fn [email]
-                  {::quotes/id ::quotes/invitations-per-team
+                              (-> params
-                   ::quotes/profile-id profile-id
+                                  (assoc :team team)
-                   ::quotes/team-id (:id team)
+                                  (assoc :profile profile)
-                   ::quotes/incr (count emails)}
+                                  (assoc :email email)
-                  {::quotes/id ::quotes/profiles-per-team
+                                  (assoc :role role))))
-                   ::quotes/profile-id profile-id
+                       (run! (partial create-invitation cfg)))
-                   ::quotes/team-id (:id team)
-                   ::quotes/incr (count emails)}))
 
-      (audit/submit! cfg
+                  (run! (partial quotes/check-quote! conn)
-                     {::audit/type "action"
+                        (list {::quotes/id ::quotes/teams-per-profile
-                      ::audit/name "create-team"
+                               ::quotes/profile-id profile-id}
-                      ::audit/profile-id profile-id
+                              {::quotes/id ::quotes/invitations-per-team
-                      ::audit/props {:name name
+                               ::quotes/profile-id profile-id
-                                     :features features}
+                               ::quotes/team-id (:id team)
-                      ::audit/context context})
+                               ::quotes/incr (count emails)}
+                              {::quotes/id ::quotes/profiles-per-team
+                               ::quotes/profile-id profile-id
+                               ::quotes/team-id (:id team)
+                               ::quotes/incr (count emails)}))
 
-      (audit/submit! cfg
+                  (vary-meta team assoc ::audit/props {:invitations (count emails)})))))
-                     {::audit/type "command"
-                      ::audit/name "create-team-invitations"
-                      ::audit/profile-id profile-id
-                      ::audit/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
 

backend/src/app/rpc/commands/verify_token.clj

@@ -169,19 +169,15 @@
         ;; if we have logged-in user and it matches the invitation we proceed
         ;; with accepting the invitation and joining the current profile to the
         ;; invited team.
-        (let [context (audit/params->context params)
+        (let [props {:team-id (:team-id claims)
-              props   {:team-id (:team-id claims)
+                     :role (:role claims)
-                       :role (:role claims)
+                     :invitation-id (:id invitation)}
-                       :invitation-id (:id invitation)}]
+              event (-> (audit/event-from-rpc-params params)
+                        (assoc ::audit/name "accept-team-invitation")
+                        (assoc ::audit/props props))]
 
           (accept-invitation cfg claims invitation profile)
-          (audit/submit! cfg
+          (audit/submit! cfg event)
-                         {::audit/type "action"
-                          ::audit/name "accept-team-invitation"
-                          ::audit/profile-id profile-id
-                          ::audit/props props
-                          ::audit/context context})
-
           (assoc claims :state :created))
 
         (ex/raise :type :validation

backend/src/app/rpc/rlimit.clj

@@ -51,12 +51,12 @@
    [app.common.uuid :as uuid]
    [app.config :as cf]
    [app.http :as-alias http]
-   [app.loggers.audit :refer [parse-client-ip]]
    [app.redis :as rds]
    [app.redis.script :as-alias rscript]
    [app.rpc :as-alias rpc]
    [app.rpc.helpers :as rph]
    [app.rpc.rlimit.result :as-alias lresult]
+   [app.util.inet :as inet]
    [app.util.services :as-alias sv]
    [app.util.time :as dt]
    [app.worker :as wrk]
@@ -215,7 +215,7 @@
   [{:keys [::rpc/profile-id] :as params}]
   (let [request (-> params meta ::http/request)]
     (or profile-id
-        (some-> request parse-client-ip)
+        (some-> request inet/parse-request)
         uuid/zero)))
 
 (defn process-request!

backend/src/app/util/inet.clj

@@ -0,0 +1,37 @@
+;; This Source Code Form is subject to the terms of the Mozilla Public
+;; License, v. 2.0. If a copy of the MPL was not distributed with this
+;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;;
+;; Copyright (c) KALEIDOS INC
+
+(ns app.util.inet
+  "INET addr parsing and validation helpers"
+  (:require
+   [cuerdas.core :as str]
+   [ring.request :as rreq])
+  (:import
+   com.google.common.net.InetAddresses
+   java.net.InetAddress))
+
+(defn valid?
+  [s]
+  (InetAddresses/isInetAddress s))
+
+(defn normalize
+  [s]
+  (try
+    (let [addr (InetAddresses/forString s)]
+      (.getHostAddress ^InetAddress addr))
+    (catch Throwable _cause
+      nil)))
+
+(defn parse-request
+  [request]
+  (or (some-> (rreq/get-header request "x-real-ip")
+              (normalize))
+      (some-> (rreq/get-header request "x-forwarded-for")
+              (str/split #"\s*,\s*")
+              (first)
+              (normalize))
+      (some-> (rreq/remote-addr request)
+              (normalize))))

backend/src/app/util/websocket.clj

@@ -11,7 +11,7 @@
    [app.common.logging :as l]
    [app.common.transit :as t]
    [app.common.uuid :as uuid]
-   [app.loggers.audit :refer [parse-client-ip]]
+   [app.util.inet :as inet]
    [app.util.time :as dt]
    [promesa.exec :as px]
    [promesa.exec.csp :as sp]
@@ -84,7 +84,7 @@
         output-ch  (sp/chan :buf output-buff-size)
         hbeat-ch   (sp/chan :buf (sp/sliding-buffer 6))
         close-ch   (sp/chan)
-        ip-addr    (parse-client-ip request)
+        ip-addr    (inet/parse-request request)
         uagent     (rreq/get-header request "user-agent")
         id         (uuid/next)
         state      (atom {})

backend/test/backend_tests/rpc_audit_test.clj

@@ -28,7 +28,8 @@
     ring.request/Request
     (get-header [_ name]
       (case name
-        "x-forwarded-for" "127.0.0.44"))))
+        "x-forwarded-for" "127.0.0.44"
+        "x-real-ip" "127.0.0.43"))))
 
 (t/deftest push-events-1
   (with-redefs [app.config/flags #{:audit-log}]
@@ -46,6 +47,7 @@
                              :profile-id (:id prof)
                              :timestamp (dt/now)
                              :type "action"}]}
+
           params  (with-meta params
                     {:app.http/request http-request})