metosin / reitit

A fast data-driven routing library for Clojure/Script
https://cljdoc.org/d/metosin/reitit/
Eclipse Public License 1.0
1.42k stars 255 forks source link

`ring.middleware.session/wrap-session` doesn't work with reitit #205

Open ikitommi opened 5 years ago

ikitommi commented 5 years ago

Reitit is a route-first architecture: the middleware/interceptor chain is created and optimized per endpoint and the chain is only applied if a route is matched.

The approach has a caveat: some middleware, like Ring's wrap-session don't work as expected: wrap-session instantiates the default memory store per instance. This means that every route will have their own memory-store. This is a surprising default and should be resolved.

How to fix:

1) mount the wrap-session outside of the router so there is only one instance of the mw for the whole app. There is :middleware option in ring-handler for this:

(require '[reitit.ring :as ring])
(require '[ring.middleware.session :as session])

(defn handler [{session :session}]
  (let [counter (inc (:counter session 0))]
    {:status 200
     :body {:counter counter}
     :session {:counter counter}}))

(def app
  (ring/ring-handler
    (ring/router
      ["/api"
       ["/ping" handler]
       ["/pong" handler]])
    (ring/create-default-handler)
    ;; the middleware on ring-handler runs before routing
    {:middleware [session/wrap-session]}))

2) create a single session store and use it within the routing table (all instances of the session middleware will share the single store).

(require '[ring.middleware.session.memory :as memory])

;; single instance
(def store (memory/memory-store))

;; inside, with shared store
(def app
  (ring/ring-handler
    (ring/router
      ["/api"
       {:middleware [[session/wrap-session {:store store}]]}
       ["/ping" handler]
       ["/pong" handler]])))
  1. Lift the wrap-session into Reitit Middleware & Interceptor and things just work.
(require '[reitit.ring.middleware.session :as session])

(def app
  (ring/ring-handler
    (ring/router
      ["/api"
       {:middleware [(session/session-middleware)]}
       ["/ping" handler]
       ["/pong" handler]])))
ikitommi commented 5 years ago

Happy to take PRs for option 3.

ikitommi commented 5 years ago

original: https://www.reddit.com/r/Clojure/comments/ad228o/sessions_under_the_leiningen_reagent_template/

valerauko commented 5 years ago

2 doesn't work if the store has any mount state in it, it just won't get started

BinitaBharati commented 5 years ago

Thanks ! Number 1 solution worked for me.

erkkikeranen commented 5 years ago

2 doesn't work if the store has any mount state in it, it just won't get started

I managed to work around this by defining the ring-handler as a mount state:

(defstate app :start (ring/ring-handler (ring/router ; and so on

Don't know if there is a problem with this approach.

valerauko commented 5 years ago

I think if you do that the routes get compiled at startup time instead of at compile time, but I may be wrong...

acobster commented 4 years ago

@ikitommi if I go with option 1, but I also use Buddy's session backend in a route-specific middleware, would this issue prevent that from working? Or would sessions still be loaded transparently (i.e., for Buddy and the rest of my code) since wrap-session is wrapping the whole app anyway?

Ramblurr commented 4 years ago

I am also wondering which option to take when using buddy's session backend.

prestancedesign commented 3 years ago

The first solution works with buddy's session backend. Here's a gist of the legacy buddy session example with Reitit replacing Compojure: https://gist.github.com/prestancedesign/e0da3f57d8a07855c2d9fcb26198f0b4

Maybe it can help. Cheers.

allentiak commented 1 year ago

You might want to read this: https://github.com/ferdinand-beyer/reitit-ring-defaults#warning-on-session-middleware

Sleepful commented 9 months ago

Mayhaps we can add this to the docs? It is quite the misdirection to face this while writing some code