0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2025-03-12 07:41:43 -05:00

♻️ Refactor (minor) of http session code

The rationale behind the refactor:
- Make available profile data to other middlewares without
  the need to access to the database (mainly for error reporting).
- Align with codestyle with the rest of internal modules.
- Simplify code.
This commit is contained in:
Andrey Antukh 2022-10-14 13:53:31 +02:00
parent dbe516f725
commit 43ab19f690
10 changed files with 214 additions and 189 deletions

View file

@ -17,6 +17,7 @@
[app.db :as db]
[app.http.client :as http]
[app.http.middleware :as hmw]
[app.http.session :as session]
[app.loggers.audit :as audit]
[app.rpc.queries.profile :as profile]
[app.tokens :as tokens]
@ -423,7 +424,7 @@
(defn- generate-redirect
[{:keys [sprops session audit] :as cfg} request info profile]
(if profile
(let [sxf ((:create session) (:id profile))
(let [sxf (session/create-fn session (:id profile))
token (or (:invitation-token info)
(tokens/generate sprops {:iss :auth
:exp (dt/in-future "15m")
@ -502,14 +503,13 @@
(s/def ::public-uri ::us/not-empty-string)
(s/def ::http-client ::http/client)
(s/def ::session map?)
(s/def ::sprops map?)
(s/def ::providers map?)
(defmethod ig/pre-init-spec ::routes
[_]
(s/keys :req-un [::public-uri
::session
::session/session
::sprops
::http-client
::providers

View file

@ -11,6 +11,7 @@
[app.common.transit :as t]
[app.http.errors :as errors]
[app.http.middleware :as mw]
[app.http.session :as session]
[app.metrics :as mtx]
[app.worker :as wrk]
[clojure.spec.alpha :as s]
@ -123,7 +124,7 @@
(s/def ::oauth map?)
(s/def ::oidc-routes (s/nilable vector?))
(s/def ::rpc-routes (s/nilable vector?))
(s/def ::session map?)
(s/def ::session ::session/session)
(s/def ::storage map?)
(s/def ::ws fn?)
@ -148,13 +149,14 @@
[mw/format-response]
[mw/params]
[mw/parse-request]
[session/middleware-1 session]
[mw/errors errors/handle]
[mw/restrict-methods]]}
["/metrics" {:handler (::mtx/handler metrics)
:allowed-methods #{:get}}]
["/assets" {:middleware [(:middleware session)]}
["/assets" {:middleware [[session/middleware-2 session]]}
["/by-id/:id" {:handler (:objects-handler assets)}]
["/by-file-media-id/:id" {:handler (:file-objects-handler assets)}]
["/by-file-media-id/:id/thumbnail" {:handler (:file-thumbnails-handler assets)}]]
@ -165,12 +167,12 @@
["/sns" {:handler (:awsns-handler cfg)
:allowed-methods #{:post}}]]
["/ws/notifications" {:middleware [(:middleware session)]
["/ws/notifications" {:middleware [[session/middleware-2 session]]
:handler ws
:allowed-methods #{:get}}]
["/api" {:middleware [[mw/cors]
[(:middleware session)]]}
[session/middleware-2 session]]}
["/audit/events" {:handler (:audit-handler cfg)
:allowed-methods #{:post}}]
["/feedback" {:handler feedback

View file

@ -14,6 +14,7 @@
[app.config :as cf]
[app.db :as db]
[app.http.middleware :as mw]
[app.http.session :as session]
[app.rpc.commands.binfile :as binf]
[app.rpc.mutations.files :refer [create-file]]
[app.rpc.queries.profile :as profile]
@ -377,17 +378,15 @@
:code :only-admins-allowed))))))})
(s/def ::session map?)
(defmethod ig/pre-init-spec ::routes [_]
(s/keys :req-un [::db/pool ::wrk/executor ::session]))
(s/keys :req-un [::db/pool ::wrk/executor ::session/session]))
(defmethod ig/init-key ::routes
[_ {:keys [session pool executor] :as cfg}]
[["/readyz" {:middleware [[mw/with-dispatch executor]
[mw/with-config cfg]]
:handler health-handler}]
["/dbg" {:middleware [[(:middleware session)]
["/dbg" {:middleware [[session/middleware-2 session]
[with-authorization pool]
[mw/with-dispatch executor]
[mw/with-config cfg]]}

View file

@ -5,6 +5,7 @@
;; Copyright (c) KALEIDOS INC
(ns app.http.session
(:refer-clojure :exclude [read])
(:require
[app.common.data :as d]
[app.common.logging :as l]
@ -20,6 +21,10 @@
[promesa.exec :as px]
[yetti.request :as yrq]))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; DEFAULTS
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; A default cookie name for storing the session.
(def default-auth-token-cookie-name "auth-token")
@ -33,35 +38,55 @@
;; Default age for automatic session renewal
(def default-renewal-max-age (dt/duration {:hours 6}))
(defprotocol ISessionStore
(read-session [store key])
(write-session [store key data])
(update-session [store data])
(delete-session [store key]))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; PROTOCOLS
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(defn- make-database-store
(defprotocol ISessionManager
(read [_ key])
(decode [_ key])
(write! [_ key data])
(update! [_ data])
(delete! [_ key]))
(s/def ::session #(satisfies? ISessionManager %))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; STORAGE IMPL
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(defn- prepare-session-params
[sprops data]
(let [profile-id (:profile-id data)
user-agent (:user-agent data)
created-at (or (:created-at data) (dt/now))
token (tokens/generate sprops {:iss "authentication"
:iat created-at
:uid profile-id})]
{:user-agent user-agent
:profile-id profile-id
:created-at created-at
:updated-at created-at
:id token}))
(defn- database-manager
[{:keys [pool sprops executor]}]
(reify ISessionStore
(read-session [_ token]
(reify ISessionManager
(read [_ token]
(px/with-dispatch executor
(db/exec-one! pool (sql/select :http-session {:id token}))))
(write-session [_ _ data]
(decode [_ token]
(px/with-dispatch executor
(let [profile-id (:profile-id data)
user-agent (:user-agent data)
created-at (or (:created-at data) (dt/now))
token (tokens/generate sprops {:iss "authentication"
:iat created-at
:uid profile-id})
params {:user-agent user-agent
:profile-id profile-id
:created-at created-at
:updated-at created-at
:id token}]
(db/insert! pool :http-session params))))
(tokens/verify sprops {:token token :iss "authentication"})))
(update-session [_ data]
(write! [_ _ data]
(px/with-dispatch executor
(let [params (prepare-session-params sprops data)]
(db/insert! pool :http-session params)
params)))
(update! [_ data]
(let [updated-at (dt/now)]
(px/with-dispatch executor
(db/update! pool :http-session
@ -69,83 +94,154 @@
{:id (:id data)})
(assoc data :updated-at updated-at))))
(delete-session [_ token]
(delete! [_ token]
(px/with-dispatch executor
(db/delete! pool :http-session {:id token})
nil))))
(defn make-inmemory-store
[{:keys [sprops]}]
(defn inmemory-manager
[{:keys [sprops executor]}]
(let [cache (atom {})]
(reify ISessionStore
(read-session [_ token]
(reify ISessionManager
(read [_ token]
(p/do (get @cache token)))
(write-session [_ _ data]
(p/do
(let [profile-id (:profile-id data)
user-agent (:user-agent data)
created-at (or (:created-at data) (dt/now))
token (tokens/generate sprops {:iss "authentication"
:iat created-at
:uid profile-id})
params {:user-agent user-agent
:created-at created-at
:updated-at created-at
:profile-id profile-id
:id token}]
(decode [_ token]
(px/with-dispatch executor
(tokens/verify sprops {:token token :iss "authentication"})))
(write! [_ _ data]
(p/do
(let [{:keys [token] :as params} (prepare-session-params sprops data)]
(swap! cache assoc token params)
params)))
(update-session [_ data]
(let [updated-at (dt/now)]
(swap! cache update (:id data) assoc :updated-at updated-at)
(assoc data :updated-at updated-at)))
(update! [_ data]
(p/do
(let [updated-at (dt/now)]
(swap! cache update (:id data) assoc :updated-at updated-at)
(assoc data :updated-at updated-at))))
(delete-session [_ token]
(delete! [_ token]
(p/do
(swap! cache dissoc token)
nil)))))
(s/def ::sprops map?)
(defmethod ig/pre-init-spec ::store [_]
(defmethod ig/pre-init-spec ::manager [_]
(s/keys :req-un [::db/pool ::wrk/executor ::sprops]))
(defmethod ig/init-key ::store
(defmethod ig/init-key ::manager
[_ {:keys [pool] :as cfg}]
(if (db/read-only? pool)
(make-inmemory-store cfg)
(make-database-store cfg)))
(inmemory-manager cfg)
(database-manager cfg)))
(defmethod ig/halt-key! ::store
(defmethod ig/halt-key! ::manager
[_ _])
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; MANAGER IMPL
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(declare assign-auth-token-cookie)
(declare assign-authenticated-cookie)
(declare clear-auth-token-cookie)
(declare clear-authenticated-cookie)
(defn create-fn
[manager profile-id]
(fn [request response]
(let [uagent (yrq/get-header request "user-agent")
params {:profile-id profile-id
:user-agent uagent}]
(-> (write! manager nil params)
(p/then (fn [session]
(l/trace :hint "create" :profile-id profile-id)
(-> response
(assign-auth-token-cookie session)
(assign-authenticated-cookie session))))))))
(defn delete-fn
[manager]
(letfn [(delete [{:keys [profile-id] :as request}]
(let [cname (cf/get :auth-token-cookie-name default-auth-token-cookie-name)
cookie (yrq/get-cookie request cname)]
(l/trace :hint "delete" :profile-id profile-id)
(some->> (:value cookie) (delete! manager))))]
(fn [request response]
(p/do
(delete request)
(-> response
(assoc :status 204)
(assoc :body nil)
(clear-auth-token-cookie)
(clear-authenticated-cookie))))))
(def middleware-1
(letfn [(wrap-handler [manager handler request respond raise]
(try
(let [claims (some->> (cf/get :auth-token-cookie-name default-auth-token-cookie-name)
(yrq/get-cookie request)
(decode manager))
request (cond-> request
(some? claims)
(assoc :session-token-claims claims))]
(handler request respond raise))
(catch Throwable _
(handler request respond raise))))]
{:name :session-1
:compile (fn [& _]
(fn [handler manager]
(partial wrap-handler manager handler)))}))
(def middleware-2
(letfn [(wrap-handler [manager handler request respond raise]
(-> (retrieve-session manager request)
(p/finally (fn [session cause]
(cond
(some? cause)
(raise cause)
(nil? session)
(handler request respond raise)
:else
(let [request (-> request
(assoc :profile-id (:profile-id session))
(assoc :session-id (:id session)))
respond (cond-> respond
(renew-session? session)
(wrap-respond manager session))]
(handler request respond raise)))))))
(retrieve-session [manager request]
(let [cname (cf/get :auth-token-cookie-name default-auth-token-cookie-name)
cookie (yrq/get-cookie request cname)]
(some->> (:value cookie) (read manager))))
(renew-session? [{:keys [updated-at] :as session}]
(and (dt/instant? updated-at)
(let [elapsed (dt/diff updated-at (dt/now))]
(neg? (compare default-renewal-max-age elapsed)))))
;; Wrap respond with session renewal code
(wrap-respond [respond manager session]
(fn [response]
(p/let [session (update! manager session)]
(-> response
(assign-auth-token-cookie session)
(assign-authenticated-cookie session)
(respond)))))]
{:name :session-2
:compile (fn [& _]
(fn [handler manager]
(partial wrap-handler manager handler)))}))
;; --- IMPL
(defn- create-session!
[store profile-id user-agent]
(let [params {:user-agent user-agent
:profile-id profile-id}]
(write-session store nil params)))
(defn- update-session!
[store session]
(update-session store session))
(defn- delete-session!
[store {:keys [cookies] :as request}]
(let [name (cf/get :auth-token-cookie-name default-auth-token-cookie-name)]
(when-let [token (get-in cookies [name :value])]
(delete-session store token))))
(defn- retrieve-session
[store request]
(let [cookie-name (cf/get :auth-token-cookie-name default-auth-token-cookie-name)]
(when-let [cookie (yrq/get-cookie request cookie-name)]
(read-session store (:value cookie)))))
(defn assign-auth-token-cookie
(defn- assign-auth-token-cookie
[response {token :id updated-at :updated-at}]
(let [max-age (cf/get :auth-token-cookie-max-age default-cookie-max-age)
created-at (or updated-at (dt/now))
@ -164,7 +260,7 @@
:secure secure?}]
(update response :cookies assoc name cookie)))
(defn assign-authenticated-cookie
(defn- assign-authenticated-cookie
[response {updated-at :updated-at}]
(let [max-age (cf/get :auth-token-cookie-max-age default-cookie-max-age)
created-at (or updated-at (dt/now))
@ -185,96 +281,23 @@
(string? domain)
(update :cookies assoc name cookie))))
(defn clear-auth-token-cookie
(defn- clear-auth-token-cookie
[response]
(let [name (cf/get :auth-token-cookie-name default-auth-token-cookie-name)]
(update response :cookies assoc name {:path "/" :value "" :max-age -1})))
(let [cname (cf/get :auth-token-cookie-name default-auth-token-cookie-name)]
(update response :cookies assoc cname {:path "/" :value "" :max-age -1})))
(defn- clear-authenticated-cookie
[response]
(let [name (cf/get :authenticated-cookie-name default-authenticated-cookie-name)
(let [cname (cf/get :authenticated-cookie-name default-authenticated-cookie-name)
domain (cf/get :authenticated-cookie-domain)]
(cond-> response
(string? domain)
(update :cookies assoc name {:domain domain :path "/" :value "" :max-age -1}))))
(defn- make-middleware
[{:keys [store] :as cfg}]
(letfn [;; Check if time reached for automatic session renewal
(renew-session? [{:keys [updated-at] :as session}]
(and (dt/instant? updated-at)
(let [elapsed (dt/diff updated-at (dt/now))]
(neg? (compare default-renewal-max-age elapsed)))))
;; Wrap respond with session renewal code
(wrap-respond [respond session]
(fn [response]
(p/let [session (update-session! store session)]
(-> response
(assign-auth-token-cookie session)
(assign-authenticated-cookie session)
(respond)))))]
{:name :session
:compile (fn [& _]
(fn [handler]
(fn [request respond raise]
(try
(-> (retrieve-session store request)
(p/finally (fn [session cause]
(cond
(some? cause)
(raise cause)
(nil? session)
(handler request respond raise)
:else
(let [request (-> request
(assoc :profile-id (:profile-id session))
(assoc :session-id (:id session)))
respond (cond-> respond
(renew-session? session)
(wrap-respond session))]
(handler request respond raise))))))
(catch Throwable cause
(raise cause))))))}))
(update :cookies assoc cname {:domain domain :path "/" :value "" :max-age -1}))))
;; --- STATE INIT: SESSION
(s/def ::store #(satisfies? ISessionStore %))
(defmethod ig/pre-init-spec :app.http/session [_]
(s/keys :req-un [::store]))
(defmethod ig/prep-key :app.http/session
[_ cfg]
(d/merge {:buffer-size 128}
(d/without-nils cfg)))
(defmethod ig/init-key :app.http/session
[_ {:keys [store] :as cfg}]
(-> cfg
(assoc :middleware (make-middleware cfg))
(assoc :create (fn [profile-id]
(fn [request response]
(p/let [uagent (yrq/get-header request "user-agent")
session (create-session! store profile-id uagent)]
(-> response
(assign-auth-token-cookie session)
(assign-authenticated-cookie session))))))
(assoc :delete (fn [request response]
(p/do
(delete-session! store request)
(-> response
(assoc :status 204)
(assoc :body nil)
(clear-auth-token-cookie)
(clear-authenticated-cookie)))))))
;; --- STATE INIT: SESSION GC
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; TASK: SESSION GC
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(declare sql:delete-expired)

View file

@ -79,10 +79,7 @@
:app.http/client
{:executor (ig/ref [::default :app.worker/executor])}
:app.http/session
{:store (ig/ref :app.http.session/store)}
:app.http.session/store
:app.http.session/manager
{:pool (ig/ref :app.db/pool)
:sprops (ig/ref :app.setup/props)
:executor (ig/ref [::default :app.worker/executor])}
@ -163,7 +160,7 @@
:sprops (ig/ref :app.setup/props)
:http-client (ig/ref :app.http/client)
:pool (ig/ref :app.db/pool)
:session (ig/ref :app.http/session)
:session (ig/ref :app.http.session/manager)
:public-uri (cf/get :public-uri)
:executor (ig/ref [::default :app.worker/executor])}
@ -171,7 +168,7 @@
:app.http/router
{:assets (ig/ref :app.http.assets/handlers)
:feedback (ig/ref :app.http.feedback/handler)
:session (ig/ref :app.http/session)
:session (ig/ref :app.http.session/manager)
:awsns-handler (ig/ref :app.http.awsns/handler)
:debug-routes (ig/ref :app.http.debug/routes)
:oidc-routes (ig/ref :app.auth.oidc/routes)
@ -188,7 +185,7 @@
{:pool (ig/ref :app.db/pool)
:executor (ig/ref [::worker :app.worker/executor])
:storage (ig/ref :app.storage/storage)
:session (ig/ref :app.http/session)}
:session (ig/ref :app.http.session/manager)}
:app.http.websocket/handler
{:pool (ig/ref :app.db/pool)
@ -217,7 +214,7 @@
:app.rpc/methods
{:pool (ig/ref :app.db/pool)
:session (ig/ref :app.http/session)
:session (ig/ref :app.http.session/manager)
:sprops (ig/ref :app.setup/props)
:metrics (ig/ref :app.metrics/metrics)
:storage (ig/ref :app.storage/storage)

View file

@ -11,12 +11,14 @@
[app.common.spec :as us]
[app.db :as db]
[app.http :as-alias http]
[app.http.session :as-alias session]
[app.loggers.audit :as audit]
[app.metrics :as mtx]
[app.msgbus :as-alias mbus]
[app.rpc.retry :as retry]
[app.rpc.rlimit :as rlimit]
[app.rpc.semaphore :as-alias rsem]
[app.storage :as-alias sto]
[app.util.services :as sv]
[app.util.time :as ts]
[clojure.spec.alpha :as s]
@ -236,13 +238,11 @@
(s/def ::ldap (s/nilable map?))
(s/def ::msgbus ::mbus/msgbus)
(s/def ::public-uri ::us/not-empty-string)
(s/def ::session map?)
(s/def ::storage some?)
(s/def ::sprops map?)
(defmethod ig/pre-init-spec ::methods [_]
(s/keys :req-un [::storage
::session
(s/keys :req-un [::sto/storage
::session/session
::sprops
::audit
::public-uri

View file

@ -13,6 +13,7 @@
[app.config :as cf]
[app.db :as db]
[app.emails :as eml]
[app.http.session :as session]
[app.loggers.audit :as audit]
[app.rpc :as-alias rpc]
[app.rpc.doc :as-alias doc]
@ -135,7 +136,7 @@
profile)]
(with-meta response
{::rpc/transform-response ((:create session) (:id profile))
{::rpc/transform-response (session/create-fn session (:id profile))
::audit/props (audit/profile->props profile)
::audit/profile-id (:id profile)})))))
@ -162,7 +163,7 @@
::doc/added "1.15"}
[{:keys [session] :as cfg} _]
(with-meta {}
{::rpc/transform-response (:delete session)}))
{::rpc/transform-response (session/delete-fn session)}))
;; ---- COMMAND: Recover Profile
@ -403,7 +404,7 @@
token (tokens/generate sprops claims)
resp {:invitation-token token}]
(with-meta resp
{::rpc/transform-response ((:create session) (:id profile))
{::rpc/transform-response (session/create-fn session (:id profile))
::audit/replace-props (audit/profile->props profile)
::audit/profile-id (:id profile)}))
@ -412,7 +413,7 @@
;; we need to mark this session as logged.
(not= "penpot" (:auth-backend profile))
(with-meta (profile/strip-private-attrs profile)
{::rpc/transform-response ((:create session) (:id profile))
{::rpc/transform-response (session/create-fn session (:id profile))
::audit/replace-props (audit/profile->props profile)
::audit/profile-id (:id profile)})
@ -420,7 +421,7 @@
;; to sign in the user directly, without email verification.
(true? is-active)
(with-meta (profile/strip-private-attrs profile)
{::rpc/transform-response ((:create session) (:id profile))
{::rpc/transform-response (session/create-fn session (:id profile))
::audit/replace-props (audit/profile->props profile)
::audit/profile-id (:id profile)})

View file

@ -10,6 +10,7 @@
[app.common.exceptions :as ex]
[app.common.spec :as us]
[app.db :as db]
[app.http.session :as session]
[app.loggers.audit :as-alias audit]
[app.rpc :as-alias rpc]
[app.rpc.commands.auth :as cmd.auth]
@ -63,12 +64,12 @@
:member-email (:email profile))
token (tokens :generate claims)]
(with-meta {:invitation-token token}
{::rpc/transform-response ((:create session) (:id profile))
{::rpc/transform-response (session/create-fn session (:id profile))
::audit/props (:props profile)
::audit/profile-id (:id profile)}))
(with-meta profile
{::rpc/transform-response ((:create session) (:id profile))
{::rpc/transform-response (session/create-fn session (:id profile))
::audit/props (:props profile)
::audit/profile-id (:id profile)})))))

View file

@ -9,6 +9,7 @@
[app.common.exceptions :as ex]
[app.common.spec :as us]
[app.db :as db]
[app.http.session :as session]
[app.loggers.audit :as audit]
[app.rpc :as-alias rpc]
[app.rpc.doc :as-alias doc]
@ -68,7 +69,7 @@
{:id (:id profile)}))
(with-meta claims
{::rpc/transform-response ((:create session) profile-id)
{::rpc/transform-response (session/create-fn session profile-id)
::audit/name "verify-profile-email"
::audit/props (audit/profile->props profile)
::audit/profile-id (:id profile)})))
@ -172,7 +173,7 @@
(let [profile (accept-invitation cfg claims invitation member)]
(with-meta
(assoc claims :state :created)
{::rpc/transform-response ((:create session) (:id profile))
{::rpc/transform-response (session/create-fn session (:id profile))
::audit/name "accept-team-invitation"
::audit/props (merge
(audit/profile->props profile)

View file

@ -12,6 +12,7 @@
[app.config :as cf]
[app.db :as db]
[app.emails :as eml]
[app.http.session :as session]
[app.loggers.audit :as audit]
[app.media :as media]
[app.rpc :as-alias rpc]
@ -278,7 +279,7 @@
{:id profile-id})
(with-meta {}
{::rpc/transform-response (:delete session)}))))
{::rpc/transform-response (session/delete-fn session)}))))
(def sql:owned-teams
"with owner_teams as (
@ -324,7 +325,7 @@
::doc/deprecated "1.15"}
[{:keys [session] :as cfg} _]
(with-meta {}
{::rpc/transform-response (:delete session)}))
{::rpc/transform-response (session/delete-fn session)}))
;; --- MUTATION: Recover Profile