interaction between session and anti-forgery middleware

49 views
Skip to first unread message

Xinhui ZENG

unread,
Nov 21, 2019, 6:10:13 PM11/21/19
to Ring
My application uses the site-defaults
(wrap-defaults app site-defaults)
 
What puzzles me is that is:
when my app responds with :session data like {:status :root}, my next inbound request will not have {:ring.middleware.anti-forgery/anti-forgery-token "xyz"} under :session.
when my app responds without :session data, my next inbound request will have {:ring.middleware.anti-forgery/anti-forgery-token "xyz"} under :session.
and I see that in both cases the {"ring-session" {:value val}} stays the same in request.

What could it be? I looked into source code but still not able to figure this out.

(deftype SessionStrategy []
  strategy/Strategy
  (get-token [this request]
    (or (session-token request)
        (random/base64 60)))
  (valid-token? [_ request token]
    (when-let [stored-token (session-token request)]
      (crypto/eq? token stored-token)))
  (write-token [this request response token]
    (let [old-token (session-token request)]
      (if (= old-token token)
        response
        (-> response
            (assoc :session (:session response (:session request)))
            (assoc-in
              [:session :ring.middleware.anti-forgery/anti-forgery-token]
              token))))))

(deftype MemoryStore [session-map]
  SessionStore
  (read-session [_ key]
    (@session-map key))
  (write-session [_ key data]
    (let [key (or key (str (UUID/randomUUID)))]
      (swap! session-map assoc key data)
      key))
  (delete-session [_ key]
    (swap! session-map dissoc key)
    nil))

James Reeves

unread,
Nov 21, 2019, 6:14:39 PM11/21/19
to Ring
If you want to modify the session without ovewriting it, you need to take the current value from the request map, modify it, then put it in the response map.

For example:

  (defn handler [{:keys [session]}]
    {:status 200, :session (assoc session :x 1), :body "Test"})


--
You received this message because you are subscribed to the Google Groups "Ring" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ring-clojure...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ring-clojure/2e700abc-0aec-4474-a853-ed8bebe3ffc5%40googlegroups.com.


--
James Reeves

Xinhui ZENG

unread,
Nov 21, 2019, 6:47:01 PM11/21/19
to Ring
That's it. Thank you for the super quick response.

Could you also point me to the part of code which determines this behavior, is it the anti-forgery middleware? Cause the different in response really is just extra {:ring.middleware.anti-forgery/anti-forgery-token "xyz"} in :session that I can tell.
To unsubscribe from this group and stop receiving emails from it, send an email to ring-c...@googlegroups.com.


--
James Reeves

James Reeves

unread,
Nov 21, 2019, 7:03:02 PM11/21/19
to Ring
It's the session middleware that determines the behaviour of the :session key. See: https://github.com/ring-clojure/ring/blob/master/ring-core/src/ring/middleware/session.clj#L52

To unsubscribe from this group and stop receiving emails from it, send an email to ring-clojure...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ring-clojure/2a12a8fd-31fe-4522-b5d3-b706ea4ab38f%40googlegroups.com.


--
James Reeves

Xinhui ZENG

unread,
Nov 21, 2019, 11:55:34 PM11/21/19
to Ring
Thanks for the tip. I think I got it this time.
Essentially every middleware and handler behind `session` middleware in the processing flow can save their info under :session key in response map which got saved/restored by `session` middleware.
What I didn't realize was that anti-forgery's own stuff can be affected by middleware/handler behind it.
How do you think about this code, is it correct to always attach the current token to response map?
 
(deftype SessionStrategy []
  strategy/Strategy
  (get-token [this request]
    (or (session-token request)
        (random/base64 60)))
  (valid-token? [_ request token]
    (when-let [stored-token (session-token request)]
      (crypto/eq? token stored-token)))
  (write-token [this request response token]
    (let [old-token (session-token request)]
      (if (= old-token token (session-token response))
        response
        (-> response
            (assoc :session (:session response (:session request)))
            (assoc-in
              [:session :ring.middleware.anti-forgery/anti-forgery-token]
              token))))))

Off topic: here is how I dump the req/resp maps left and right to troubleshoot. It's just nice that source code is available I just need to save it to my project and reference this. What's the better way?

(defn- zeng [handler prompt]
  (fn [req]
    (do (prn (format "\033[1;31m===== enter %s\033[m" prompt)) (pp/pprint req))
    (let [result (handler req)]
      (do (prn (format "\033[1;32m===== exit %s\033[m" prompt)) (pp/pprint result))
      result)))
(defn wrap-defaults
  [handler config]
  (-> handler
      (zeng "handler")
      (wrap anti/wrap-anti-forgery     (get-in config [:security :anti-forgery] false))
      (zeng "anti-f") ...))



--
James Reeves

James Reeves

unread,
Nov 22, 2019, 9:04:36 AM11/22/19
to Ring
On Fri, 22 Nov 2019 at 04:55, Xinhui ZENG <zen...@gmail.com> wrote:
Thanks for the tip. I think I got it this time.
Essentially every middleware and handler behind `session` middleware in the processing flow can save their info under :session key in response map which got saved/restored by `session` middleware.
What I didn't realize was that anti-forgery's own stuff can be affected by middleware/handler behind it.
How do you think about this code, is it correct to always attach the current token to response map?

I don't understand what you're aiming to achieve with the code you posted.

Off topic: here is how I dump the req/resp maps left and right to troubleshoot. It's just nice that source code is available I just need to save it to my project and reference this. What's the better way?

That's a reasonable way of checking the request and response. You could also use a debugger, if your editor/IDE supports one.

--
James Reeves

Xinhui ZENG

unread,
Nov 22, 2019, 10:49:41 AM11/22/19
to ring-c...@googlegroups.com
On Fri, Nov 22, 2019 at 9:04 AM James Reeves <ja...@booleanknot.com> wrote:
On Fri, 22 Nov 2019 at 04:55, Xinhui ZENG <zen...@gmail.com> wrote:
Thanks for the tip. I think I got it this time.
Essentially every middleware and handler behind `session` middleware in the processing flow can save their info under :session key in response map which got saved/restored by `session` middleware.
What I didn't realize was that anti-forgery's own stuff can be affected by middleware/handler behind it.
How do you think about this code, is it correct to always attach the current token to response map?

I don't understand what you're aiming to achieve with the code you posted.

Perhaps it's clearer to see them side by side. The extra test completes the idea that `anti-forgery` will take care of the it's own stuff: the injection of :ring.middleware.anti-forgery/anti-forgery-token into :session of a resp map.
Three scenarios:
a) When `anti-forgery-token` is not present in :session of a req map:
this part of test `(= old-token token ...)` fails and at that point middleware will proceed to inject `anti-forgery-token` into :session of a resp map.
b) When `anti-forgery-token` is not present in :session of a resp map:
this is what got me to start this conversation. Is it reasonable in your opinion to inject the token when the token is missing from resp map?
c) When `anti-forgery-token` is present in :session of a resp map but its value is different than the existing token:
there's no valid reason application handler changes the value, is there? I feel the middleware should replace the value with what the middleware has generated earlier.
the extra test `(= ... ... (session-token response))` takes care of both scenarios b) and c)

image.png
Off topic: here is how I dump the req/resp maps left and right to troubleshoot. It's just nice that source code is available I just need to save it to my project and reference this. What's the better way?

That's a reasonable way of checking the request and response. You could also use a debugger, if your editor/IDE supports one.

I'm using emacs/cider but am not aware of a better way to debug web app when all things are async in nature. ring-mocks maybe or what else is there?
 
--
James Reeves

--
You received this message because you are subscribed to the Google Groups "Ring" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ring-clojure...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ring-clojure/CALg24jSJXKoJ8bqB%3D8MG9QbC5ZYccUkq1NXYxmcfFDu1en9WaA%40mail.gmail.com.

James Reeves

unread,
Nov 22, 2019, 11:14:12 AM11/22/19
to Ring

On Fri, 22 Nov 2019 at 15:49, Xinhui ZENG <zen...@gmail.com> wrote:
On Fri, Nov 22, 2019 at 9:04 AM James Reeves <ja...@booleanknot.com> wrote:

I don't understand what you're aiming to achieve with the code you posted.

Perhaps it's clearer to see them side by side. The extra test completes the idea that `anti-forgery` will take care of the it's own stuff: the injection of :ring.middleware.anti-forgery/anti-forgery-token into :session of a resp map.
Three scenarios:
a) When `anti-forgery-token` is not present in :session of a req map:
this part of test `(= old-token token ...)` fails and at that point middleware will proceed to inject `anti-forgery-token` into :session of a resp map.
b) When `anti-forgery-token` is not present in :session of a resp map:
this is what got me to start this conversation. Is it reasonable in your opinion to inject the token when the token is missing from resp map?

It's an interesting idea, but I don't think it's a route we should go down. It's a slippery slope to try and automatically correct or work around developer mistakes. In the best case, you're encouraging people to use sessions incorrectly; in the worst case, you're introducing behaviour that will break existing systems.

However, I think it is worth considering how to make sessions easier to work with, or improving the documentation on sessions.

--
James Reeves

Xinhui ZENG

unread,
Nov 22, 2019, 1:10:37 PM11/22/19
to ring-c...@googlegroups.com
I agree with everything you say except I feel the extra test should not break stuff because while the data under :session is shared but this particular one `anti-forgery-token` only belongs to anti-forgery middleware working like a reserved key. The test (= old-token token) of existing code does the same work updating `anti-forgery-token` regardless of what :session in a resp map says. This is due to in principle we have layered middleware/handler approach in ring and different than the misuse of how :session should be updated.
 
--
James Reeves

--
You received this message because you are subscribed to the Google Groups "Ring" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ring-clojure...@googlegroups.com.

Xinhui ZENG

unread,
Nov 22, 2019, 1:14:11 PM11/22/19
to ring-c...@googlegroups.com
On Fri, Nov 22, 2019 at 1:10 PM Xinhui ZENG <zen...@gmail.com> wrote:


On Fri, Nov 22, 2019 at 11:14 AM James Reeves <ja...@booleanknot.com> wrote:

On Fri, 22 Nov 2019 at 15:49, Xinhui ZENG <zen...@gmail.com> wrote:
On Fri, Nov 22, 2019 at 9:04 AM James Reeves <ja...@booleanknot.com> wrote:

I don't understand what you're aiming to achieve with the code you posted.

Perhaps it's clearer to see them side by side. The extra test completes the idea that `anti-forgery` will take care of the it's own stuff: the injection of :ring.middleware.anti-forgery/anti-forgery-token into :session of a resp map.
Three scenarios:
a) When `anti-forgery-token` is not present in :session of a req map:
this part of test `(= old-token token ...)` fails and at that point middleware will proceed to inject `anti-forgery-token` into :session of a resp map.
b) When `anti-forgery-token` is not present in :session of a resp map:
this is what got me to start this conversation. Is it reasonable in your opinion to inject the token when the token is missing from resp map?

It's an interesting idea, but I don't think it's a route we should go down. It's a slippery slope to try and automatically correct or work around developer mistakes. In the best case, you're encouraging people to use sessions incorrectly; in the worst case, you're introducing behaviour that will break existing systems.

However, I think it is worth considering how to make sessions easier to work with, or improving the documentation on sessions.

I agree with everything you say except I feel the extra test should not break stuff because while the data under :session is shared but this particular one `anti-forgery-token` only belongs to anti-forgery middleware working like a reserved key. The test (= old-token token) of existing code does the same work updating `anti-forgery-token` regardless of what :session in a resp map says. This is due to in principle we have layered middleware/handler approach in ring and different than the misuse of how :session should be updated.
 
The extra test does cost extra work to be done to update almost every response, versus just the first response in a http session. Hmm....

James Reeves

unread,
Nov 22, 2019, 1:27:09 PM11/22/19
to Ring
On Fri, 22 Nov 2019 at 18:10, Xinhui ZENG <zen...@gmail.com> wrote:
On Fri, Nov 22, 2019 at 11:14 AM James Reeves <ja...@booleanknot.com> wrote:
On Fri, 22 Nov 2019 at 15:49, Xinhui ZENG <zen...@gmail.com> wrote:
b) When `anti-forgery-token` is not present in :session of a resp map:
this is what got me to start this conversation. Is it reasonable in your opinion to inject the token when the token is missing from resp map?

It's an interesting idea, but I don't think it's a route we should go down. It's a slippery slope to try and automatically correct or work around developer mistakes. In the best case, you're encouraging people to use sessions incorrectly; in the worst case, you're introducing behaviour that will break existing systems.

However, I think it is worth considering how to make sessions easier to work with, or improving the documentation on sessions.
 
I agree with everything you say except I feel the extra test should not break stuff because while the data under :session is shared but this particular one `anti-forgery-token` only belongs to anti-forgery middleware working like a reserved key. The test (= old-token token) of existing code does the same work updating `anti-forgery-token` regardless of what :session in a resp map says. This is due to in principle we have layered middleware/handler approach in ring and different than the misuse of how :session should be updated.

That is true... though it's not quite as simple as just checking the response. The response session can be:

1. A map, in which case the session is replaced
2. nil, in which case the session is deleted
3. The key doesn't exist, in which case the session is unchanged

So it's a little trickier than it first appears.

However, let me think about it a bit. You've made a good case.

--
James Reeves
Reply all
Reply to author
Forward
0 new messages