noir-clojure / lib-noir

A set of libraries for ring apps, including stateful sessions.
Eclipse Public License 1.0
483 stars 47 forks source link

:timeout and :timeout-response not working in wrap-noir-session #109

Closed gberenfield closed 9 years ago

gberenfield commented 9 years ago

It seems :timeout and :timeout-response aren't being correctly passed to wrap-noir-session inside noir.util.middleware/app-routes. I'm writing a Luminus-based template webapp with lib-noir 0.9.4 and my sessions always timeout at 30-minutes.

Even with:

:session-options {:timeout (* 60 120)
                  :timeout-response (redirect "/")}

passed to app-handler.

yogthos commented 9 years ago

It might have to do with the change to using ring-defaults. The :session-options should still work, but it's deprecated in favor of using the :session key in ring-defaults config. I'll take a look to see what's preventing the :session-options key from working correctly, meanwhile could you try the following to see if you still experience the issue with the new method:

(def session-defaults
  {:timeout (* 60 120)
   :timeout-response (redirect "/")})

(defn- mk-defaults
       "set to true to enable XSS protection"
       [xss-protection?]
       (-> site-defaults
           (update-in [:session] merge session-defaults)
           (assoc-in [:security :anti-forgery] xss-protection?)))

(def app (app-handler
           ;; add your application routes here
           [home-routes base-routes]
           ;; add custom middleware here
           :middleware (load-middleware)
           :ring-defaults (mk-defaults false)
           ;; add access rules here
           :access-rules []
           ;; serialize/deserialize the following data formats
           ;; available formats:
           ;; :json :json-kw :yaml :yaml-kw :edn :yaml-in-html
           :formats [:json-kw :edn :transit-json]))
yogthos commented 9 years ago

Nothing jumps out at me from cursory glance. The way the timeout is setup is that it the options are passed to noir session here:

(wrap-noir-session
          (update-in
            (or session-options (:session ring-defaults) (:session site-defaults))
            [:store] #(or % (memory-store mem))))

this in turn calls noir.session here:

(defn wrap-noir-session
 "A stateful layer around wrap-session. Options are passed to wrap-session."
 [handler & [{:keys [timeout timeout-response] :as opts}]]
 (let [opts (or (dissoc opts :timeout :timeout-response) {})]
   (if timeout
     (-> handler
        (noir-session)
        (wrap-idle-session-timeout
         {:timeout timeout
          :timeout-response timeout-response})
        (wrap-session opts))
     (-> handler (noir-session) (wrap-session opts)))))

The session-options key should supersede any other parameters when not nil.

gberenfield commented 9 years ago

Trying your first comment worked. I created this app about 1-month ago and see that a new app via lein new luminus foo has the mk-defaults stuff and now uses ring.middleware.defaults.

I agree session-options should take precedence and wonder why it didn't work but am happy to go with the new setup.

Thanks!