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

wrap-anti-forgery does not work with lib-noir #82

Closed danjac closed 10 years ago

danjac commented 10 years ago

in my handler.clj:

(def app (middleware/app-handler
           ;;add your application routes here
           [home-routes  restricted-routes app-routes]
           ;;add custom middleware here           
           :middleware [wrap-anti-forgery]
           ;;add access rules here
           ;;each rule should be a vector
           :access-rules [{:redirect "/access-denied"
                       :rule user-access}]))

In my routes:

(form-to [:post "/"]
         (anti-forgery-field)
         ;;; other fields

Consistently I get the "Invalid anti-forgery token" message, which means the middleware is installed ok but not able to compare the session and posted values correctly.

Digging into the anti-forgery code, it appears to look at the session and request :form-params. However, while it's loaded into the session correctly form-params is just nil - so the issue appears to be with lib-noir (somewhere in the middleware stack) as opposed to ring-anti-forgery - for some reason form-params is unavailable at this point. I tried adding wrap-params to the middleware but still no joy.

yogthos commented 10 years ago

It's like the problem here

https://github.com/noir-clojure/lib-noir/blob/master/src/noir/util/middleware.clj#L167

(-> (apply routes app-routes)
      (wrap-request-map)
      (api)
      (with-opts wrap-multipart-params multipart)
      (wrap-middleware middleware)
      (wrap-access-rules access-rules)
      (wrap-noir-validation)
      (wrap-noir-cookies)
      (wrap-noir-flash)
      (wrap-noir-session
        {:store (or store (memory-store mem))}))

as wrap-middleware happens after the api that wraps params. Could you try it with a custom app-handler that looks like this?

(defn app-handler
  [app-routes & {:keys [store multipart middleware access-rules]}]
  (-> (apply routes app-routes)
      (wrap-middleware middleware) ;move middleware to the front of the stack
      (wrap-request-map)
      (api)
      (with-opts wrap-multipart-params multipart)
      (wrap-access-rules access-rules)
      (wrap-noir-validation)
      (wrap-noir-cookies)
      (wrap-noir-flash)
      (wrap-noir-session
        {:store (or store (memory-store mem))})))

if that helps I'll just change the ordering and make a new release

yogthos commented 10 years ago

I did a bit of testing and this does seem to solve the problem. I released a new version 0.6.8 on Clojars let me know if that works for you.

danjac commented 10 years ago

Works beautifully, thank you for the quick response!